feat: add School Roll Number field for Ireland schools Add optional s… #632

Mohamed-RPF · 2025-11-28T10:57:54Z

Add School Roll Number field for Ireland schools

Add optional school_roll_number field with alphanumeric validation, following the same pattern as UK URN and US district fields. Includes database migration, API endpoints, admin dashboard integration, and comprehensive test coverage.

Status

Closes: https://github.com/orgs/RaspberryPiFoundation/projects/51/views/11?pane=issue&itemId=132315256&issue=RaspberryPiFoundation%7Cdigital-editor-issues%7C925

Related to: https://github.com/RaspberryPiFoundation/editor-standalone/pull/686

Points for consideration:

Security

Field has unique index to prevent duplicate entries
Input validation enforces alphanumeric format (numbers followed by letters)
Case-insensitive uniqueness check prevents bypass attempts

Performance

Database index added on school_roll_number column for efficient lookups
Optional field (nullable) - no impact on existing schools

What's changed?

Database:

Added school_roll_number string column to schools table
Added unique index on school_roll_number

Model (School):

Added validation: uniqueness (case-insensitive), alphanumeric format
Added normalisation callback to handle blank values
Added I18n error message for validation

API:

Added school_roll_number to permitted parameters in SchoolsController
Included school_roll_number in JSON API responses

Admin Dashboard:

Added field to admin show page, form, and attributes list
Admins can view School Roll Numbers

Tests:

8 comprehensive validation tests (optional, uniqueness, format validation, normalisation)
Updated factory to support the new field

Locales:

Added validation message to config/locales/en.yml

…chool_roll_number field with alphanumeric validation, following the same pattern as UK URN and US district fields. Includes database migration, API endpoints, admin dashboard integration, and comprehensive test coverage.

Copilot

Pull request overview

This PR adds an optional school_roll_number field for Ireland schools, following the established pattern used for UK reference numbers (URN) and US district fields. The implementation includes database migration with unique indexing, model validation with alphanumeric format enforcement, API endpoint integration, admin dashboard support, and comprehensive model-level test coverage.

Key changes:

Added school_roll_number column to schools table with unique index for efficient lookups and duplicate prevention
Implemented case-insensitive uniqueness validation and alphanumeric format requirement (numbers followed by letters)
Integrated field throughout the API stack (REST endpoints, JSON responses, admin dashboard)

Reviewed changes

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

Show a summary per file

File	Description
`db/migrate/20251128102719_add_school_roll_number_to_schools.rb`	Migration adding school_roll_number column with unique index
`db/schema.rb`	Schema update reflecting new column and index
`app/models/school.rb`	Validation rules (uniqueness, format), normalization callback for blank values
`app/controllers/api/schools_controller.rb`	Added school_roll_number to permitted parameters
`app/views/api/schools/_school.json.jbuilder`	Included school_roll_number in JSON API responses
`app/dashboards/school_dashboard.rb`	Added field to admin dashboard attributes (show, form)
`config/locales/en.yml`	Added validation error message for format requirements
`spec/models/school_spec.rb`	8 comprehensive tests covering optional field, uniqueness, format validation, normalization
`spec/factories/school.rb`	Added school_roll_number attribute with nil default
`spec/features/my_school/showing_my_school_spec.rb`	Updated expected JSON fields to include school_roll_number

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

app/models/school.rb

spec/models/school_spec.rb

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

spec/models/school_spec.rb

Copilot

Pull request overview

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

loiswells97 · 2025-12-01T16:37:14Z

config/locales/en.yml

  validations:
    school:
      website: "must be a valid URL"
+      school_roll_number: "must be alphanumeric (e.g., 01572D)"


not sure this is the best error message, as the tests show that we are precluding strings that contain either all letters or all numbers, which are still alphanumeric

Also, on this, where are you thinking that this error message will be surfaced? Because we need to ensure that it's still translatable on the frontend

Looking closer the backend validation messages aren't currently shown inline to users, they see a generic 'Please check the fields' alert instead. The frontend handles user facing error messages with translations (similar to how the website field works). I'll update the backend message to be more accurate for logging/API purposes: "must be numbers followed by letters (e.g., 01572D)" which matches the actual Irish School Roll Number format from the official government database. I'll follow the existing pattern used for the website URL field and have a translatable key on the frontend.

I think most likely we'll want to be catching this on the frontend anyway

app/models/school.rb

db/migrate/20251128102719_add_school_roll_number_to_schools.rb

spec/models/school_spec.rb

loiswells97 · 2025-12-01T16:51:22Z

app/models/school.rb

+  validates :school_roll_number,
+            uniqueness: { case_sensitive: false, allow_nil: true },
+            presence: false,
+            format: { with: /\A[0-9]+[A-Z]+\z/, allow_nil: true, message: I18n.t('validations.school.school_roll_number') }


do we have a reference anywhere that shows us what the format of an Irish school roll number should be so we can verify this regex is correct?

The 3rd AC in the ticket mentions 'Given the "School Roll Number" field is visible, when I enter a valid alpha-numeric value (e.g., 01572D), then the data is accepted and saved with the application.'

Is that example meant to show what the precise format will be? I was thinking more of something official

These are some examples from the gov.ie website: 20375I, 20374G, 20378O, 20371A

I checked public sources and there’s no official published spec for the roll-number format.
In the Department’s datasets, roll numbers consistently appear as 5 digits followed by an uppercase letter (e.g. 01572D).

okay great, thanks for looking into this 👍

… reapplication with the same roll number after a school is rejected

…reuse. Aligns database constraint with application validation by only enforcing uniqueness where rejected_at IS NULL

…d-api

…l presence validation so school_roll_number is required when country_code is IE, following existing pattern for UK postal codes.

danhalson

LGTM

db/migrate/20251204132605_change_school_roll_number_index_to_partial.rb

…d-api

cla-bot bot added the cla-signed label Nov 28, 2025

Mohamed-RPF added 3 commits November 28, 2025 12:44

CircleCl build fix: rubocop

efe6780

test fix: add school_roll_number to the expected JSON fields in the test

da5cab5

Mohamed-RPF force-pushed the 925-verification-flow-add-roll-number-ireland-api branch from 0725649 to da5cab5 Compare November 28, 2025 12:44

Mohamed-RPF marked this pull request as ready for review November 28, 2025 13:09

adrian-rpf requested a review from Copilot November 28, 2025 16:48

Copilot started reviewing on behalf of adrian-rpf November 28, 2025 16:48 View session

Copilot finished reviewing on behalf of adrian-rpf November 28, 2025 16:52

Copilot AI reviewed Nov 28, 2025

View reviewed changes

app/models/school.rb Show resolved Hide resolved

spec/models/school_spec.rb Show resolved Hide resolved

Mohamed-RPF added 2 commits December 1, 2025 09:23

fix: normalize school_roll_number to uppercase before validation

d9b33fe

Rubocop style issue fix

05481ed

Mohamed-RPF requested a review from Copilot December 1, 2025 09:51

Copilot started reviewing on behalf of Mohamed-RPF December 1, 2025 09:52 View session

Copilot finished reviewing on behalf of Mohamed-RPF December 1, 2025 09:54

Copilot AI reviewed Dec 1, 2025

View reviewed changes

spec/models/school_spec.rb Show resolved Hide resolved

fix: normalize school_roll_number to uppercase before validation

8fe2f36

jamiebenstead requested a review from Copilot December 1, 2025 10:26

Copilot started reviewing on behalf of jamiebenstead December 1, 2025 10:26 View session

Copilot finished reviewing on behalf of jamiebenstead December 1, 2025 10:30

Copilot AI reviewed Dec 1, 2025

View reviewed changes

jamiebenstead previously approved these changes Dec 1, 2025

View reviewed changes

loiswells97 reviewed Dec 1, 2025

View reviewed changes

app/models/school.rb Show resolved Hide resolved

loiswells97 reviewed Dec 1, 2025

View reviewed changes

db/migrate/20251128102719_add_school_roll_number_to_schools.rb Show resolved Hide resolved

loiswells97 reviewed Dec 1, 2025

View reviewed changes

spec/models/school_spec.rb Show resolved Hide resolved

loiswells97 reviewed Dec 1, 2025

View reviewed changes

fix: clarify school_roll_number validation message for backend logging

6a375d2

Mohamed-RPF dismissed jamiebenstead’s stale review via 6a375d2 December 2, 2025 13:11

Scope school_roll_number uniqueness to non-rejected schools and allow…

da062e6

… reapplication with the same roll number after a school is rejected

Mohamed-RPF added 4 commits December 4, 2025 13:34

Change school_roll_number index to partial index for rejected school …

dba5faf

…reuse. Aligns database constraint with application validation by only enforcing uniqueness where rejected_at IS NULL

Merge branch 'main' into 925-verification-flow-add-roll-number-irelan…

84d8def

…d-api

Remove redundant assignment in school_roll_number test

fb572cc

Make school_roll_number mandatory for Ireland schools. Add conditiona…

4abee07

…l presence validation so school_roll_number is required when country_code is IE, following existing pattern for UK postal codes.

Mohamed-RPF requested a review from loiswells97 December 8, 2025 17:20

danhalson approved these changes Dec 9, 2025

View reviewed changes

db/migrate/20251204132605_change_school_roll_number_index_to_partial.rb Show resolved Hide resolved

Merge branch 'main' into 925-verification-flow-add-roll-number-irelan…

116dd1d

…d-api

feat: add School Roll Number field for Ireland schools Add optional s… #632

Are you sure you want to change the base?

feat: add School Roll Number field for Ireland schools Add optional s… #632

Conversation

Mohamed-RPF commented Nov 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Status

Points for consideration:

Security

Performance

What's changed?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mohamed-RPF Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danhalson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Mohamed-RPF commented Nov 28, 2025 •

edited

Loading

Mohamed-RPF Dec 2, 2025 •

edited

Loading