868 add school email domain api#878
Conversation
b46e372 to
4ea2fbf
Compare
Test coverage92.04% line coverage reported by SimpleCov. |
4ea2fbf to
3159b2a
Compare
There was a problem hiding this comment.
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) andPOST /api/schools/:school_id/school_email_domains(create) endpoints. - Adds a
SchoolEmailDomain::Createconcept 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.
3159b2a to
1ef1f88
Compare
| end | ||
| response | ||
| rescue ActiveRecord::RecordInvalid => e | ||
| record = response[:school_email_domain] || e.record |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I've made this changed and initialised with nil since response[:school_email_domain] should be a single record
1ef1f88 to
511affb
Compare
86c56f1 to
e24d6af
Compare
There was a problem hiding this comment.
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.
Add SchoolEmailDomainsController and school_email_domains route. Return a list of domain strings. Update CanCan ability to allow access only for teachers and owners. Co-authored-by: Cursor <cursoragent@cursor.com>
Add POST create to SchoolEmailDomainsController and route. Add SchoolEmailDomain::Create concept to persist domains and sync the full list with Profile.
a1bc812 to
27cc95f
Compare
mwtrew
left a comment
There was a problem hiding this comment.
Looking good overall, but I think we'll need to do something about the problem in this thread.
mwtrew
left a comment
There was a problem hiding this comment.
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.
Serialise concurrent creates per school so Profile always receives a complete domain list.
| response | ||
| rescue StandardError => e | ||
| Sentry.capture_exception(e) # Send unexpected/Profile errors to Sentry | ||
| response[:error] = e.message |
| rescue StandardError => e | ||
| Sentry.capture_exception(e) # Send unexpected/Profile errors to Sentry | ||
| response[:error] = e.message | ||
| response[:error_code] = 'profile_sync_failed' |
The spec tests PG directly, rather than our code, and its complicated structure requires local overrides to RSpec configuration. Full discussion, with considerations of other ways of testing the lock, is recorded on the PR (#878): #878 (comment)

Status
Description
A REST API that allows school owners and teachers to create and list SchoolEmailDomains.
What's changed?