Store school email domains

[PetarSimonovic]

Status

Closes #1325

What's changed?

• Add a school_email_domains table
• Add a SchoolEmailDomain model
• Perform minimal formatting on a domain to strip leading @ and lowercase the string
• Update association on School
• Add and update specs

[Copilot]

Pull request overview

Introduces first-class storage for school email domains by adding a dedicated table/model and wiring it into the School domain model, with accompanying specs.

Changes:

• Added school_email_domains table with a composite unique index per school.
• Added SchoolEmailDomain model with before_validation normalization (strip leading @, downcase).
• Updated School to has_many :school_email_domains and added/updated model specs.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
spec/models/school_spec.rb	Adds association + dependent-destroy coverage for school_email_domains.
spec/models/school_email_domain_spec.rb	Adds model spec coverage for domain normalization behavior.
db/schema.rb	Reflects new school_email_domains table and foreign key.
db/migrate/20260420104937_create_school_email_domains.rb	Creates the new school_email_domains table and composite unique index.
app/models/school_email_domain.rb	Introduces the new model and domain normalization callback.
app/models/school.rb	Adds has_many :school_email_domains, dependent: :destroy.

---

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

[github-actions]

Test coverage

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

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

---

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

[fspeirs]

Overall this looks like the right approach. I have one consideration about validating domains - I feel like it would be helpful to be stricter in checking that the user hasn't typo'ed their domain or misunderstood what we are asking for.

[fspeirs]

Directionally, quite correct. Just some sharpening of the normalisation approach in school_email_domain and a request for a testing method in school.rb. The lack of a testing method is non-blocking if it's coming in a separate PR.

[fspeirs]

Issue: I think this code could be much simpler and doesn't have to expressly handle specific things like uri_host_if_http_url. Instead, I suggest that we just try to parse a URI and, if successful take the host portion:

def normalise_domain
  return if domain.blank?
  value = domain.strip.downcase
  # Add a scheme unless it already has one, so URI can parse it
  value = "http://#{value}" unless value =~ %r{\A[a-z][a-z0-9+\-.]*://}i
  
  uri = URI.parse(value)

  host = uri.host
  return if host.blank?

  if PublicSuffix.valid?(host)
    self.domain = host
  else
    errors.add(:domain, "is not a valid domain")
  end
rescue URI::InvalidURIError
  errors.add(:domain, "is not a valid domain")
end

This is kind of the approach you're taking in uri_host_if_http_url and pulling it up to be the main way we normalise domains: (a) can we parse it with URI::parse and secondly is it a valid suffix.

[PetarSimonovic]

Thanks for this Fraser.

I've implemented your changes and taken account of Päivi's comment below

[fspeirs]

Issue: Callers (including my work) would like a validator method on this called something like: valid_domain?(candidate_domain) which returns true if candidate_domain is registered for this school.

This would allow testing domains presented by registering users without exposing the internal implementation of school_email_domains.

[PetarSimonovic]

I've added a validator method to School using a straightforward school_email_domains.exists?

I don't think we need to optimise for batch domain checks, since we're validating one student at a time

If we ever did need to perform some sort of batch validation, we could revisit this and perhaps pluck then memoise the domains

[peconia]

The regex we use in profile side to validate is '^\\s*(?:[A-Za-z0-9](?:[A-Za-z0-9-]{0,61}[A-Za-z0-9])?\\.)+[A-Za-z]{2,63}\\s*$'

While it is similar, we would reject things that had a dash in the wrong place, like example.-wrong.edu, but that would be approved by this one. I also wonder about things that are domains but not email domains, like having www. or ftp. prefix to the domain. Maybe that will come down to making sure the UI is clear in what is expected, and the API level can validate some things too.

[PetarSimonovic]

@fspeirs and @peconia I've grouped the invalid domains together so it might be easier to check edge cases like the one that Päivi mentioned

[PetarSimonovic]

@peconia we're validating against the regex that Profile uses