868 add school email domain api

[PetarSimonovic]

Status

• Closes Add school email domain API #868

Description

A REST API that allows school owners and teachers to create and list SchoolEmailDomains.

• Owners and teachers can list/create; students and users from other schools cannot
• Domains are validated (FQDN format, URI, registrable public suffix) with translated error messages
• New domains are synced to Profile; the DB transaction rolls back if Profile sync fails
• Delete is out of scope for this PR

What's changed?

• GET /api/schools/:school_id/school_email_domains — list domains
• POST /api/schools/:school_id/school_email_domains — create a domain
• Added concept with Profile sync + rollback on failure
• Validation with translated error messages
• Specs at validator, model, concept, and feature levels

[github-actions]

Test coverage

91.93% line coverage reported by SimpleCov.
Run: https://github.com/RaspberryPiFoundation/editor-api/actions/runs/28161351896

[Copilot]

Pull request overview

Adds a REST API for managing a school’s allowed student email domains, intended for use by authorised school owners/teachers and kept in sync with the Profile service.

Changes:

• Introduces GET /api/schools/:school_id/school_email_domains (list) and POST /api/schools/:school_id/school_email_domains (create) endpoints.
• Adds a SchoolEmailDomain::Create concept that persists the domain and syncs the school’s full domain list to Profile (rolling back on sync failure).
• Extends authorisation and i18n error messages, plus adds factories and request/unit specs.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
spec/support/profile_api_mock.rb	Adds a stub helper for Profile “update school email domains” calls in specs.
spec/features/school_email_domain/listing_school_email_domains_spec.rb	Request specs for listing domains with auth/unauth coverage.
spec/features/school_email_domain/creating_school_email_domains_spec.rb	Request specs for creating domains, validation errors, and Profile failure handling.
spec/factories/school_email_domain.rb	Factory for SchoolEmailDomain.
spec/concepts/school_email_domain/create_spec.rb	Unit coverage for the SchoolEmailDomain::Create concept.
lib/concepts/school_email_domain/operations/create.rb	Implements the create + Profile sync operation with rollback on failure.
config/routes.rb	Wires nested school email domain routes under schools.
config/locales/en.yml	Adds translated validation messages for domain validation error codes.
app/models/ability.rb	Grants school owners/teachers ability to read/create school email domains.
app/controllers/api/school_email_domains_controller.rb	Adds the API controller for index/create with CanCan authorisation.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[mwtrew]

Might be worth fixing this after all - I don't expect it to be a problem but it's a trivial change. It's similar to what's been done here.

[PetarSimonovic]

I've made this changed and initialised with nil since response[:school_email_domain] should be a single record

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.

^{Reviewed by Cursor Bugbot for commit e24d6af. Configure here.}

[mwtrew]

Looking good overall, but I think we'll need to do something about the problem in this thread.

[mwtrew]

Looks good, thanks for those changes.

If you'd like to look into using an advisory lock that would be great, but I think what you have will work for now.

[PetarSimonovic]

Happy to remove/shorten this comment if the point about identical key values is overkill

[mwtrew]

Yep, I think the comment can go

[mwtrew]

Could we also check that the lock was released at the end?

[PetarSimonovic]

I considered a couple of approaches to check that the lock is released and I'm not sure it's possible to do this in a simple way for a couple of reasons.

1. Ask postgres

I thought the simplest would be to query the DB to see if any matching locks remain after the transaction.

But Rspec is configured to use_transactional_fixtures so I think the spec example itself is wrapped in a transaction. The advisory lock is only released once the spec has concluded so it's still present in the test.

2. pg_try_advisory_xact_lock

We could use pg_try_advisory_xact_lock to see if we can obtain a lock after our transaction has run. A false result would mean it was "still locked"

But within a given session I don't think we can ever get a false result: pg_try_advisory_xact_lock either acquires the lock, or returns true because the session already holds it.

Cursor suggestion

Cursor made the following suggestion about how we could practically check whether the lock has released. I'm happy to pursue it if it looks like a valid approach.

Use a hook in rails_helper.rb to disable transactional fixtures per-example:

Then create a new Thread to check out its own connection from the pool, which is a distinct PG session, so it can actually contend for the lock.

[mwtrew]

Interesting, didn't know that about Rspec tests. Yep, I was thinking of the first option as well.

I don't know if you've been able to test the contention situation manually - if you have and it works I think that's good enough, but if not it would be ideal to try it via a test like Cursor has suggested.