Make NCES ID mandatory for US schools (#638)

Mohamed-RPF · jamiebenstead · web-flow · commit 284caefe6061 · 2026-01-09T10:45:56.000Z
# Make NCES ID mandatory for US schools Add conditional presence validation for US NCES ID (district_nces_id) fields, with format validation and scoped uniqueness to allow reuse after school rejection. This improves validation error handling so users receive specific, actionable error messages instead of generic form errors. ## Status Closes: https://github.com/orgs/RaspberryPiFoundation/projects/51/views/11?pane=issue&itemId=120513360&issue=RaspberryPiFoundation%7Cdigital-editor-issues%7C780 Related to: (standalone url) ## Points for consideration: ### Security - Partial unique indexes prevent duplicate entries among active (non-rejected) schools - Format validation enforces strict digit patterns (5-6 digits for [URN](https://get-information-schools.service.gov.uk/glossary), 12 digits for [NCES ID](https://nces.ed.gov/ccd/psadd.asp)) - Case-insensitive uniqueness check prevents bypass attempts ## What's changed? ### Database: - Converted `reference` index to partial unique index (`WHERE rejected_at IS NULL`) - Converted `district_nces_id` index to partial unique index (`WHERE rejected_at IS NULL`) - This allows rejected schools to release their identifiers for reuse by legitimate schools ### Model (School): - Added conditional presence validation: `reference` required for UK (`country_code == 'GB'`) - Added conditional presence validation: `district_nces_id` required for US (`country_code == 'US'`) - Added format validation: URN must be 5-6 digits, NCES ID must be 12 digits - Added scoped uniqueness: only active (non-rejected) schools are checked for duplicates - Added `united_kingdom?` and `united_states?` helper methods ### API Error Responses: | Field | Validation | Error Message | |-------|------------|---------------| | `reference` | presence | `"can't be blank"` | | `reference` | uniqueness | `"has already been taken"` | | `reference` | format | `"must be 5-6 digits (e.g., 100000)"` | | `district_nces_id` | presence | `"can't be blank"` | | `district_nces_id` | uniqueness | `"has already been taken"` | | `district_nces_id` | format | `"must be 12 digits (e.g., 010000000001)"` | ### Tests: - 16 comprehensive validation tests covering: - Conditional presence (UK/US specific) - Format validation (valid/invalid patterns) - Scoped uniqueness (allows reuse after rejection) - Updated factory to include valid `reference` for default GB schools --------- Co-authored-by: Jamie Benstead <57325966+jamiebenstead@users.noreply.github.com> Co-authored-by: Jamie Benstead <jamie.benstead@gmail.com>
diff --git a/Gemfile.lock b/Gemfile.lock
@@ -558,6 +558,7 @@ GEM
 PLATFORMS
   aarch64-linux
   arm64-darwin-24
+  arm64-darwin-25
   x86_64-linux
 
 DEPENDENCIES
diff --git a/app/controllers/api/schools_controller.rb b/app/controllers/api/schools_controller.rb
@@ -22,7 +22,10 @@ def create
         @school = result[:school]
         render :show, formats: [:json], status: :created
       else
-        render json: { error: result[:error] }, status: :unprocessable_entity
+        render json: {
+          error: result[:error],
+          error_types: result[:error_types]
+        }, status: :unprocessable_entity
       end
     end
 
diff --git a/app/jobs/school_import_job.rb b/app/jobs/school_import_job.rb
@@ -128,7 +128,10 @@ def build_school_params(school_data)
       creator_agree_authority: true,
       creator_agree_terms_and_conditions: true,
       creator_agree_responsible_safeguarding: true,
-      user_origin: 'experience_cs'
+      user_origin: 'experience_cs',
+      district_name: school_data[:district_name],
+      district_nces_id: school_data[:district_nces_id],
+      school_roll_number: school_data[:school_roll_number]
     }.compact
   end
 
diff --git a/app/models/school.rb b/app/models/school.rb
@@ -16,12 +16,21 @@ class School < ApplicationRecord
   validates :address_line_1, presence: true
   validates :municipality, presence: true
   validates :country_code, presence: true, inclusion: { in: ISO3166::Country.codes }
-  validates :reference, uniqueness: { case_sensitive: false, allow_nil: true }, presence: false
-  validates :district_nces_id, uniqueness: { case_sensitive: false, allow_nil: true }, presence: false
+  validates :reference,
+            uniqueness: { conditions: -> { where(rejected_at: nil) }, case_sensitive: false, allow_blank: true, message: I18n.t('validations.school.reference_urn_exists') },
+            format: { with: /\A\d{5,6}\z/, allow_nil: true, message: I18n.t('validations.school.reference') },
+            if: :united_kingdom?
+  validates :district_nces_id,
+            uniqueness: { conditions: -> { where(rejected_at: nil) }, case_sensitive: false, allow_blank: true, message: I18n.t('validations.school.district_nces_id_exists') },
+            format: { with: /\A\d{12}\z/, allow_nil: true, message: I18n.t('validations.school.district_nces_id') },
+            presence: true,
+            if: :united_states?
+  validates :district_name, presence: true, if: :united_states?
   validates :school_roll_number,
-            uniqueness: { conditions: -> { where(rejected_at: nil) }, case_sensitive: false, allow_nil: true },
-            presence: { if: :ireland? },
-            format: { with: /\A[0-9]+[A-Z]+\z/, allow_nil: true, message: I18n.t('validations.school.school_roll_number') }
+            uniqueness: { conditions: -> { where(rejected_at: nil) }, case_sensitive: false, allow_blank: true, message: I18n.t('validations.school.school_roll_number_exists') },
+            format: { with: /\A[0-9]+[A-Z]+\z/, allow_nil: true, message: I18n.t('validations.school.school_roll_number') },
+            presence: true,
+            if: :ireland?
   validates :creator_id, presence: true, uniqueness: true
   validates :creator_agree_authority, presence: true, acceptance: true
   validates :creator_agree_terms_and_conditions, presence: true, acceptance: true
@@ -129,6 +138,14 @@ def should_format_uk_postal_code?
     country_code == 'GB' && postal_code.to_s.length >= 5
   end
 
+  def united_kingdom?
+    country_code == 'GB'
+  end
+
+  def united_states?
+    country_code == 'US'
+  end
+
   def ireland?
     country_code == 'IE'
   end
diff --git a/config/locales/en.yml b/config/locales/en.yml
@@ -15,7 +15,12 @@ en:
   validations:
     school:
       website: "must be a valid URL"
+      reference: "must be 5-6 digits (e.g., 100000)"
+      reference_urn_exists: "URN number already exists"
+      district_nces_id: "must be 12 digits (e.g., 010000000001)"
+      district_nces_id_exists: "NCES ID already exists"
       school_roll_number: "must be numbers followed by letters (e.g., 01572D)"
+      school_roll_number_exists: "School roll number already exists"
     invitation:
       email_address: "'%<value>s' is invalid"
   activerecord:
diff --git a/db/migrate/20251208134354_change_reference_and_nces_id_indexes_to_partial.rb b/db/migrate/20251208134354_change_reference_and_nces_id_indexes_to_partial.rb
@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+
+class ChangeReferenceAndNcesIdIndexesToPartial < ActiveRecord::Migration[7.2]
+  def change
+    # Convert reference (UK URN) index to partial index
+    # This allows rejected schools to release their URN for reuse
+    remove_index :schools, :reference
+    add_index :schools, :reference, unique: true, where: 'rejected_at IS NULL'
+
+    # Convert district_nces_id (US NCES ID) index to partial index
+    # This allows rejected schools to release their NCES ID for reuse
+    remove_index :schools, :district_nces_id
+    add_index :schools, :district_nces_id, unique: true, where: 'rejected_at IS NULL'
+  end
+end
+
diff --git a/db/schema.rb b/db/schema.rb
diff --git a/lib/concepts/school/operations/create.rb b/lib/concepts/school/operations/create.rb
@@ -11,6 +11,8 @@ def call(school_params:, creator_id:)
       rescue StandardError => e
         Sentry.capture_exception(e)
         response[:error] = response[:school].errors
+        response[:error_types] = response[:school].errors.details
+
         response
       end
 
diff --git a/lib/tasks/seeds_helper.rb b/lib/tasks/seeds_helper.rb
@@ -16,15 +16,23 @@ module SeedsHelper
   def create_school(creator_id, school_id = nil)
     School.find_or_create_by!(creator_id:, id: school_id) do |school|
       Rails.logger.info 'Seeding a school...'
+      country_code = Faker::Address.country_code
       school.name = Faker::Educator.secondary_school
       school.website = Faker::Internet.url(scheme: 'https')
       school.address_line_1 = Faker::Address.street_address
       school.municipality = Faker::Address.city
-      school.country_code = Faker::Address.country_code
+      school.country_code = country_code
       school.creator_id = creator_id
       school.creator_agree_authority = true
       school.creator_agree_terms_and_conditions = true
       school.creator_agree_to_ux_contact = true
+      # Country-specific required fields
+      school.reference = format('%06d', rand(100_000..999_999)) if country_code == 'GB'
+      if country_code == 'US'
+        school.district_nces_id = format('%012d', rand(10**12))
+        school.district_name = Faker::Address.community
+      end
+      school.school_roll_number = "#{rand(10_000..99_999)}#{('A'..'Z').to_a.sample}" if country_code == 'IE'
     end
   end
 
diff --git a/spec/concepts/school/create_spec.rb b/spec/concepts/school/create_spec.rb
@@ -10,6 +10,7 @@
       address_line_1: 'Address Line 1',
       municipality: 'Greater London',
       country_code: 'GB',
+      reference: '100000',
       creator_agree_authority: true,
       creator_agree_terms_and_conditions: true,
       creator_agree_to_ux_contact: true,
diff --git a/spec/factories/school.rb b/spec/factories/school.rb
@@ -7,6 +7,7 @@
     address_line_1 { 'Address Line 1' }
     municipality { 'Greater London' }
     country_code { 'GB' }
+    sequence(:reference) { |n| format('%06d', 100_000 + n) }
     school_roll_number { nil }
     creator_id { SecureRandom.uuid }
     creator_agree_authority { true }
diff --git a/spec/features/admin/schools_spec.rb b/spec/features/admin/schools_spec.rb
@@ -79,7 +79,7 @@
 
     describe 'when the school is in the United States and has a postal code' do
       before do
-        school.update(country_code: 'US', postal_code: '90210')
+        school.update(country_code: 'US', postal_code: '90210', district_name: 'Some District', district_nces_id: '010000000001', reference: nil)
         get admin_school_path(school)
       end
 
diff --git a/spec/features/school/creating_a_school_spec.rb b/spec/features/school/creating_a_school_spec.rb
@@ -19,6 +19,7 @@
         address_line_1: 'Address Line 1',
         municipality: 'Greater London',
         country_code: 'GB',
+        reference: '100000',
         creator_agree_authority: true,
         creator_agree_terms_and_conditions: true,
         creator_agree_to_ux_contact: true,
diff --git a/spec/jobs/school_import_job_spec.rb b/spec/jobs/school_import_job_spec.rb
@@ -17,15 +17,19 @@
           address_line_1: '123 Main St',
           municipality: 'Springfield',
           country_code: 'US',
-          owner_email: 'owner1@example.com'
+          owner_email: 'owner1@example.com',
+          district_name: 'Some District',
+          district_nces_id: '010000000001'
         },
         {
           name: 'Test School 2',
           website: 'https://test2.example.com',
           address_line_1: '456 Oak Ave',
           municipality: 'Boston',
           country_code: 'US',
-          owner_email: 'owner2@example.com'
+          owner_email: 'owner2@example.com',
+          district_name: 'Other District',
+          district_nces_id: '010000000002'
         }
       ]
     end
@@ -124,7 +128,9 @@
             'address_line_1' => '123 Main St',
             'municipality' => 'Springfield',
             'country_code' => 'us',
-            'owner_email' => 'owner1@example.com'
+            'owner_email' => 'owner1@example.com',
+            'district_name' => 'Some District',
+            'district_nces_id' => '010000000001'
           }
         ]
       end
diff --git a/spec/models/school_spec.rb b/spec/models/school_spec.rb
