Skip to content

Commit 980647f

Browse files
committed
Make URN mandatory for UK schools and NCES ID mandatory for US schools
1 parent 2a6ab27 commit 980647f

File tree

6 files changed

+136
-9
lines changed

6 files changed

+136
-9
lines changed

app/models/school.rb

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,14 @@ class School < ApplicationRecord
1616
validates :address_line_1, presence: true
1717
validates :municipality, presence: true
1818
validates :country_code, presence: true, inclusion: { in: ISO3166::Country.codes }
19-
validates :reference, uniqueness: { case_sensitive: false, allow_nil: true }, presence: false
20-
validates :district_nces_id, uniqueness: { case_sensitive: false, allow_nil: true }, presence: false
19+
validates :reference,
20+
uniqueness: { conditions: -> { where(rejected_at: nil) }, case_sensitive: false, allow_nil: true },
21+
presence: { if: :united_kingdom? },
22+
format: { with: /\A\d{5,6}\z/, allow_nil: true, message: I18n.t('validations.school.reference') }
23+
validates :district_nces_id,
24+
uniqueness: { conditions: -> { where(rejected_at: nil) }, case_sensitive: false, allow_nil: true },
25+
presence: { if: :united_states? },
26+
format: { with: /\A\d{12}\z/, allow_nil: true, message: I18n.t('validations.school.district_nces_id') }
2127
validates :creator_id, presence: true, uniqueness: true
2228
validates :creator_agree_authority, presence: true, acceptance: true
2329
validates :creator_agree_terms_and_conditions, presence: true, acceptance: true
@@ -118,6 +124,14 @@ def should_format_uk_postal_code?
118124
country_code == 'GB' && postal_code.to_s.length >= 5
119125
end
120126

127+
def united_kingdom?
128+
country_code == 'GB'
129+
end
130+
131+
def united_states?
132+
country_code == 'US'
133+
end
134+
121135
def format_uk_postal_code
122136
cleaned_postal_code = postal_code.delete(' ')
123137
# insert a space as the third-from-last character in the postcode, eg. SW1A1AA -> SW1A 1AA

config/locales/en.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ en:
1414
validations:
1515
school:
1616
website: "must be a valid URL"
17+
reference: "must be 5-6 digits (e.g., 100000)"
18+
district_nces_id: "must be 12 digits (e.g., 010000000001)"
1719
invitation:
1820
email_address: "'%<value>s' is invalid"
1921
activerecord:
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
# frozen_string_literal: true
2+
3+
class ChangeReferenceAndNcesIdIndexesToPartial < ActiveRecord::Migration[7.2]
4+
def change
5+
# Convert reference (UK URN) index to partial index
6+
# This allows rejected schools to release their URN for reuse
7+
remove_index :schools, :reference
8+
add_index :schools, :reference, unique: true, where: 'rejected_at IS NULL'
9+
10+
# Convert district_nces_id (US NCES ID) index to partial index
11+
# This allows rejected schools to release their NCES ID for reuse
12+
remove_index :schools, :district_nces_id
13+
add_index :schools, :district_nces_id, unique: true, where: 'rejected_at IS NULL'
14+
end
15+
end
16+

db/schema.rb

Lines changed: 5 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

spec/factories/school.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
address_line_1 { 'Address Line 1' }
88
municipality { 'Greater London' }
99
country_code { 'GB' }
10+
sequence(:reference) { |n| format('%06d', 100000 + n) }
1011
creator_id { SecureRandom.uuid }
1112
creator_agree_authority { true }
1213
creator_agree_terms_and_conditions { true }

spec/models/school_spec.rb

Lines changed: 96 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -132,21 +132,113 @@
132132
expect(school).to be_valid
133133
end
134134

135-
it 'does not require a reference' do
136-
create(:school, id: SecureRandom.uuid, reference: nil)
137-
135+
it 'does not require a reference for non-UK schools' do
136+
school.country_code = 'IE'
138137
school.reference = nil
139138
expect(school).to be_valid
140139
end
141140

141+
it 'requires reference for UK schools' do
142+
school.country_code = 'GB'
143+
school.reference = nil
144+
expect(school).not_to be_valid
145+
expect(school.errors[:reference]).to include("can't be blank")
146+
end
147+
142148
it 'requires references to be unique if provided' do
149+
school.reference = '100000'
150+
school.save!
151+
152+
duplicate_school = build(:school, reference: '100000')
153+
expect(duplicate_school).not_to be_valid
154+
end
155+
156+
it 'accepts a valid reference format (5-6 digits)' do
157+
school.reference = '100000'
158+
expect(school).to be_valid
159+
end
160+
161+
it 'accepts a 5-digit reference' do
162+
school.reference = '20000'
163+
expect(school).to be_valid
164+
end
165+
166+
it 'rejects a reference with non-digit characters' do
143167
school.reference = 'URN-123'
168+
expect(school).not_to be_valid
169+
expect(school.errors[:reference]).to include('must be 5-6 digits (e.g., 100000)')
170+
end
171+
172+
it 'rejects a reference with too few digits' do
173+
school.reference = '1234'
174+
expect(school).not_to be_valid
175+
expect(school.errors[:reference]).to include('must be 5-6 digits (e.g., 100000)')
176+
end
177+
178+
it 'rejects a reference with too many digits' do
179+
school.reference = '1234567'
180+
expect(school).not_to be_valid
181+
expect(school.errors[:reference]).to include('must be 5-6 digits (e.g., 100000)')
182+
end
183+
184+
it 'allows reference reuse when original school is rejected' do
185+
school.reference = '100000'
144186
school.save!
187+
school.reject
188+
189+
new_school = build(:school, reference: '100000')
190+
expect(new_school).to be_valid
191+
expect { new_school.save! }.not_to raise_error
192+
end
193+
194+
it 'does not require a district_nces_id for non-US schools' do
195+
school.country_code = 'GB'
196+
school.district_nces_id = nil
197+
expect(school).to be_valid
198+
end
145199

146-
duplicate_school = build(:school, reference: 'urn-123')
200+
it 'requires district_nces_id for US schools' do
201+
school.country_code = 'US'
202+
school.district_nces_id = nil
203+
expect(school).not_to be_valid
204+
expect(school.errors[:district_nces_id]).to include("can't be blank")
205+
end
206+
207+
it 'requires district_nces_id to be unique if provided' do
208+
school.district_nces_id = '010000000001'
209+
school.save!
210+
211+
duplicate_school = build(:school, district_nces_id: '010000000001')
147212
expect(duplicate_school).not_to be_valid
148213
end
149214

215+
it 'accepts a valid district_nces_id format (12 digits)' do
216+
school.district_nces_id = '010000000001'
217+
expect(school).to be_valid
218+
end
219+
220+
it 'rejects a district_nces_id with non-digit characters' do
221+
school.district_nces_id = '01000000000A'
222+
expect(school).not_to be_valid
223+
expect(school.errors[:district_nces_id]).to include('must be 12 digits (e.g., 010000000001)')
224+
end
225+
226+
it 'rejects a district_nces_id with wrong length' do
227+
school.district_nces_id = '12345678901'
228+
expect(school).not_to be_valid
229+
expect(school.errors[:district_nces_id]).to include('must be 12 digits (e.g., 010000000001)')
230+
end
231+
232+
it 'allows district_nces_id reuse when original school is rejected' do
233+
school.district_nces_id = '010000000001'
234+
school.save!
235+
school.reject
236+
237+
new_school = build(:school, district_nces_id: '010000000001')
238+
expect(new_school).to be_valid
239+
expect { new_school.save! }.not_to raise_error
240+
end
241+
150242
it 'requires an address_line_1' do
151243
school.address_line_1 = ' '
152244
expect(school).not_to be_valid

0 commit comments

Comments
 (0)