Skip to content

Commit e953dd7

Browse files
jamiebensteadjamiebenstead
committed
Add validation and tests
1 parent dae0327 commit e953dd7

File tree

3 files changed

+76
-61
lines changed

3 files changed

+76
-61
lines changed

‎app/models/school.rb‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,20 @@ class School < ApplicationRecord
1717
validates :municipality, presence: true
1818
validates :country_code, presence: true, inclusion: { in: ISO3166::Country.codes }
1919
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') }
20+
uniqueness: { conditions: -> { where(rejected_at: nil) }, case_sensitive: false, allow_blank: true },
21+
format: { with: /\A\d{5,6}\z/, allow_nil: true, message: I18n.t('validations.school.reference') },
22+
if: :united_kingdom?
2323
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') }
24+
uniqueness: { conditions: -> { where(rejected_at: nil) }, case_sensitive: false, allow_blank: true },
25+
format: { with: /\A\d{12}\z/, allow_nil: true, message: I18n.t('validations.school.district_nces_id') },
26+
presence: true,
27+
if: :united_states?
28+
validates :district_name, presence: true, if: :united_states?
2729
validates :school_roll_number,
28-
uniqueness: { conditions: -> { where(rejected_at: nil) }, case_sensitive: false, allow_nil: true },
29-
presence: { if: :ireland? },
30-
format: { with: /\A[0-9]+[A-Z]+\z/, allow_nil: true, message: I18n.t('validations.school.school_roll_number') }
30+
uniqueness: { conditions: -> { where(rejected_at: nil) }, case_sensitive: false, allow_blank: true },
31+
format: { with: /\A[0-9]+[A-Z]+\z/, allow_nil: true, message: I18n.t('validations.school.school_roll_number') },
32+
presence: true,
33+
if: :ireland?
3134
validates :creator_id, presence: true, uniqueness: true
3235
validates :creator_agree_authority, presence: true, acceptance: true
3336
validates :creator_agree_terms_and_conditions, presence: true, acceptance: true

‎spec/jobs/school_import_job_spec.rb‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,7 @@
2626
address_line_1: '456 Oak Ave',
2727
municipality: 'Boston',
2828
country_code: 'US',
29-
owner_email: 'owner2@example.com',
30-
district_nces_id: '010000000002'
29+
owner_email: 'owner2@example.com'
3130
}
3231
]
3332
end
@@ -126,8 +125,7 @@
126125
'address_line_1' => '123 Main St',
127126
'municipality' => 'Springfield',
128127
'country_code' => 'us',
129-
'owner_email' => 'owner1@example.com',
130-
'district_nces_id' => '010000000001'
128+
'owner_email' => 'owner1@example.com'
131129
}
132130
]
133131
end

‎spec/models/school_spec.rb‎

Lines changed: 62 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
let(:student) { create(:student, school:) }
77
let(:teacher) { create(:teacher, school:) }
88
let(:school) { create(:school, creator_id: SecureRandom.uuid) }
9+
let!(:us_school) { create(:school, country_code: 'US', district_name: 'Some District', district_nces_id: '010000000001', creator_id: SecureRandom.uuid) }
10+
let!(:ireland_school) { create(:school, country_code: 'IE', school_roll_number: '01572D', creator_id: SecureRandom.uuid) }
911

1012
describe 'associations' do
1113
it 'has many classes' do
@@ -138,11 +140,10 @@
138140
expect(school).to be_valid
139141
end
140142

141-
it 'requires reference for UK schools' do
143+
it 'does not requires reference for UK schools' do
142144
school.country_code = 'GB'
143145
school.reference = nil
144-
expect(school).not_to be_valid
145-
expect(school.errors[:reference]).to include("can't be blank")
146+
expect(school).to be_valid
146147
end
147148

148149
it 'requires references to be unique if provided' do
@@ -191,48 +192,67 @@
191192
expect { new_school.save! }.not_to raise_error
192193
end
193194

194-
it 'does not require a district_nces_id for non-US schools' do
195+
it 'does not require a district_nces_id for UK schools' do
195196
school.country_code = 'GB'
196197
school.district_nces_id = nil
197198
expect(school).to be_valid
198199
end
199200

200-
it 'requires district_nces_id for US schools' do
201-
school.country_code = 'US'
201+
it 'does not require a district_nces_id for CA schools' do
202+
school.country_code = 'CA'
203+
school.district_name = 'Some District'
202204
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+
expect(school).to be_valid
205206
end
206207

207-
it 'requires district_nces_id to be unique if provided' do
208-
school.district_nces_id = '010000000001'
209-
school.save!
208+
it 'requires district_nces_id for US schools' do
209+
us_school.district_nces_id = nil
210+
expect(us_school).not_to be_valid
211+
expect(us_school.errors[:district_nces_id]).to include("can't be blank")
212+
end
213+
214+
it 'requires district_name for US schools' do
215+
us_school.district_name = nil
216+
expect(us_school).not_to be_valid
217+
expect(us_school.errors[:district_name]).to include("can't be blank")
218+
end
219+
220+
it 'does not require district_name for non-US schools' do
221+
school.district_name = nil
222+
expect(school).to be_valid
223+
end
224+
225+
it 'does not require district_name for CA schools' do
226+
school.country_code = 'CA'
227+
school.district_name = nil
228+
expect(school).to be_valid
229+
end
210230

211-
duplicate_school = build(:school, district_nces_id: '010000000001')
231+
it 'requires district_nces_id to be unique if provided' do
232+
duplicate_school = build(:school, country_code: 'US', district_nces_id: '010000000001')
212233
expect(duplicate_school).not_to be_valid
213234
end
214235

215236
it 'accepts a valid district_nces_id format (12 digits)' do
216-
school.district_nces_id = '010000000001'
217-
expect(school).to be_valid
237+
us_school.district_nces_id = '010000000001'
238+
expect(us_school).to be_valid
218239
end
219240

220241
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)')
242+
us_school.district_nces_id = '01000000000A'
243+
expect(us_school).not_to be_valid
244+
expect(us_school.errors[:district_nces_id]).to include('must be 12 digits (e.g., 010000000001)')
224245
end
225246

226247
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)')
248+
us_school.district_nces_id = '12345678901'
249+
expect(us_school).not_to be_valid
250+
expect(us_school.errors[:district_nces_id]).to include('must be 12 digits (e.g., 010000000001)')
230251
end
231252

232253
it 'allows district_nces_id reuse when original school is rejected' do
233-
school.district_nces_id = '010000000001'
234-
school.save!
235-
school.reject
254+
us_school.district_nces_id = '010000000001'
255+
us_school.reject
236256

237257
new_school = build(:school, district_nces_id: '010000000001')
238258
expect(new_school).to be_valid
@@ -246,46 +266,42 @@
246266
end
247267

248268
it 'requires school_roll_number for Ireland schools' do
249-
school.country_code = 'IE'
250-
school.school_roll_number = nil
251-
expect(school).not_to be_valid
252-
expect(school.errors[:school_roll_number]).to include("can't be blank")
269+
ireland_school.school_roll_number = nil
270+
expect(ireland_school).not_to be_valid
271+
expect(ireland_school.errors[:school_roll_number]).to include("can't be blank")
253272
end
254273

255274
it 'requires school_roll_number to be unique if provided' do
256-
school.school_roll_number = '01572D'
257-
school.save!
258-
259-
duplicate_school = build(:school, school_roll_number: '01572d')
275+
duplicate_school = build(:school, school_roll_number: '01572D', country_code: 'IE')
260276
expect(duplicate_school).not_to be_valid
261277
end
262278

263279
it 'accepts a valid alphanumeric school_roll_number' do
264-
school.school_roll_number = '01572D'
265-
expect(school).to be_valid
280+
ireland_school.school_roll_number = '01572D'
281+
expect(ireland_school).to be_valid
266282
end
267283

268284
it 'accepts a school_roll_number with one or more letters' do
269-
school.school_roll_number = '12345ABC'
270-
expect(school).to be_valid
285+
ireland_school.school_roll_number = '12345ABC'
286+
expect(ireland_school).to be_valid
271287
end
272288

273289
it 'rejects a school_roll_number with only numbers' do
274-
school.school_roll_number = '01572'
275-
expect(school).not_to be_valid
276-
expect(school.errors[:school_roll_number]).to include('must be numbers followed by letters (e.g., 01572D)')
290+
ireland_school.school_roll_number = '01572'
291+
expect(ireland_school).not_to be_valid
292+
expect(ireland_school.errors[:school_roll_number]).to include('must be numbers followed by letters (e.g., 01572D)')
277293
end
278294

279295
it 'rejects a school_roll_number with only letters' do
280-
school.school_roll_number = 'ABCDE'
281-
expect(school).not_to be_valid
282-
expect(school.errors[:school_roll_number]).to include('must be numbers followed by letters (e.g., 01572D)')
296+
ireland_school.school_roll_number = 'ABCDE'
297+
expect(ireland_school).not_to be_valid
298+
expect(ireland_school.errors[:school_roll_number]).to include('must be numbers followed by letters (e.g., 01572D)')
283299
end
284300

285301
it 'rejects a school_roll_number with special characters' do
286-
school.school_roll_number = '01572-D'
287-
expect(school).not_to be_valid
288-
expect(school.errors[:school_roll_number]).to include('must be numbers followed by letters (e.g., 01572D)')
302+
ireland_school.school_roll_number = '01572-D'
303+
expect(ireland_school).not_to be_valid
304+
expect(ireland_school.errors[:school_roll_number]).to include('must be numbers followed by letters (e.g., 01572D)')
289305
end
290306

291307
it 'normalizes blank school_roll_number to nil' do
@@ -301,11 +317,9 @@
301317
end
302318

303319
it 'allows school_roll_number reuse when original school is rejected' do
304-
school.school_roll_number = '01572D'
305-
school.save!
306-
school.reject
320+
ireland_school.reject
307321

308-
new_school = build(:school, school_roll_number: '01572D')
322+
new_school = build(:school, school_roll_number: '01572D', country_code: 'IE')
309323
expect(new_school).to be_valid
310324
expect { new_school.save! }.not_to raise_error
311325
end

0 commit comments

Comments
 (0)