Add class code to the school class model (#521)

closes #518

---------

Co-authored-by: create-issue-branch[bot] <53036503+create-issue-branch[bot]@users.noreply.github.com>
Co-authored-by: Lois Wells <[email protected]>
Co-authored-by: Lois Wells <[email protected]>

Author: create-issue-branch[bot]

app/models/school.rb

@@ -54,7 +54,7 @@ def rejected?
   def verify!
     attempts = 0
     begin
-      update!(verified_at: Time.zone.now, code: SchoolCodeGenerator.generate)
+      update!(verified_at: Time.zone.now, code: ForEducationCodeGenerator.generate)
     rescue ActiveRecord::RecordInvalid => e
       raise unless e.record.errors[:code].include?('has already been taken') && attempts <= 5
 

app/models/school_class.rb

@@ -9,7 +9,11 @@ class SchoolClass < ApplicationRecord
 
   scope :with_teachers, ->(user_id) { joins(:teachers).where(teachers: { id: user_id }) }
 
+  before_validation :assign_class_code, on: :create
+
   validates :name, presence: true
+  validates :code, uniqueness: { scope: :school_id }, presence: true, format: { with: /\d\d-\d\d-\d\d/, allow_nil: false }
+  validate :code_cannot_be_changed
   validate :school_class_has_at_least_one_teacher
 
   has_paper_trail(
@@ -36,11 +40,30 @@ def with_teachers
     [self, User.from_userinfo(ids: teacher_ids)]
   end
 
+  def assign_class_code
+    return if code.present?
+
+    5.times do
+      self.code = ForEducationCodeGenerator.generate
+      return if code_is_unique_within_school
+    end
+
+    errors.add(:code, 'could not be generated')
+  end
+
   private
 
   def school_class_has_at_least_one_teacher
     return if teachers.present?
 
     errors.add(:teachers, 'must have at least one teacher')
   end
+
+  def code_cannot_be_changed
+    errors.add(:code, 'cannot be changed after verification') if code_was.present? && code_changed?
+  end
+
+  def code_is_unique_within_school
+    code.present? && SchoolClass.where(code:, school:).none?
+  end
 end

app/views/api/school_classes/show.json.jbuilder

@@ -8,6 +8,7 @@ json.call(
   :description,
   :school_id,
   :name,
+  :code,
   :created_at,
   :updated_at
 )

db/migrate/20250307113438_add_school_class_code.rb

@@ -0,0 +1,6 @@
+class AddSchoolClassCode < ActiveRecord::Migration[7.1]
+  def change
+    add_column :school_classes, :code, :string
+    add_index :school_classes, [:code, :school_id], unique: true
+  end
+end

db/migrate/20250307113710_assign_code_to_existing_classes.rb

@@ -0,0 +1,12 @@
+class AssignCodeToExistingClasses < ActiveRecord::Migration[7.1]
+  def up
+    SchoolClass.find_each do |school_class|
+      school_class.assign_class_code
+      school_class.save!
+    end
+  end
+
+  def down
+    SchoolClass.update_all(code: nil)
+  end
+end

db/schema.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-class SchoolCodeGenerator
+class ForEducationCodeGenerator
   MAX_CODE = 1_000_000
 
   def self.generate

lib/school_code_generator.rb lib/for_education_code_generator.rb

@@ -14,6 +14,6 @@
 
   factory :verified_school, parent: :school do
     verified_at { Time.current }
-    code { SchoolCodeGenerator.generate }
+    code { ForEducationCodeGenerator.generate }
   end
 end

spec/factories/school.rb

@@ -3,6 +3,7 @@
 FactoryBot.define do
   factory :school_class do
     sequence(:name) { |n| "Class #{n}" }
+    code { ForEducationCodeGenerator.generate }
 
     transient do
       teacher_ids { [SecureRandom.uuid] }

spec/factories/school_class.rb

@@ -42,6 +42,13 @@
     expect(data[:name]).to eq('Test School Class')
   end
 
+  it 'includes the school class code in the response' do
+    get("/api/schools/#{school.id}/classes/#{school_class.id}", headers:)
+    data = JSON.parse(response.body, symbolize_names: true)
+
+    expect(data[:code]).to eq(school_class.code)
+  end
+
   it 'responds with the teacher JSON' do
     get("/api/schools/#{school.id}/classes/#{school_class.id}", headers:)
     data = JSON.parse(response.body, symbolize_names: true)

spec/features/school_class/showing_a_school_class_spec.rb

@@ -2,11 +2,11 @@
 
 require 'rails_helper'
 
-RSpec.describe SchoolCodeGenerator do
+RSpec.describe ForEducationCodeGenerator do
   describe '.generate' do
     it 'uses Random#rand to generate a random number up to the maximum' do
       random = instance_double(Random)
-      allow(random).to receive(:rand).with(SchoolCodeGenerator::MAX_CODE).and_return(123)
+      allow(random).to receive(:rand).with(ForEducationCodeGenerator::MAX_CODE).and_return(123)
       allow(Random).to receive(:new).and_return(random)
 
       expect(described_class.generate).to eq('00-01-23')

spec/lib/school_code_generator_spec.rb spec/lib/for_education_code_generator_spec.rb

@@ -63,6 +63,42 @@
       school_class.name = ' '
       expect(school_class).to be_invalid
     end
+
+    it 'assigns class code before validating' do
+      school_class.code = nil
+      school_class.valid?
+      expect(school_class.code).to match(/\d\d-\d\d-\d\d/)
+    end
+
+    it 'requires a unique class code within the same school' do
+      school_class.save!
+      school_class_with_duplicate_code = build(:school_class, school: school_class.school, code: school_class.code)
+      school_class_with_duplicate_code.valid?
+      expect(school_class_with_duplicate_code.errors[:code]).to include('has already been taken')
+    end
+
+    it 'permits a duplicate class code in a different school' do
+      school_class.save!
+      school_class_with_duplicate_code = build(:school_class, school: build(:school), code: school_class.code)
+      expect(school_class_with_duplicate_code).to be_valid
+    end
+
+    it 'requires a valid class code format' do
+      school_class.code = 'invalid'
+      expect(school_class).to be_invalid
+    end
+
+    it 'accepts a valid class code format' do
+      school_class.code = '12-34-56'
+      expect(school_class).to be_valid
+    end
+
+    it 'does not allow the class code to be changed' do
+      school_class.code = '12-34-56'
+      school_class.save!
+      school_class.code = '65-43-21'
+      expect(school_class).to be_invalid
+    end
   end
 
   describe '.teachers' do
@@ -141,6 +177,44 @@
     end
   end
 
+  describe '#assign_class_code' do
+    it 'assigns a class code if not already present' do
+      school_class = build(:school_class, code: nil, school:)
+      school_class.assign_class_code
+      expect(school_class.code).to match(/\d\d-\d\d-\d\d/)
+    end
+
+    it 'does not assign a class code if already present' do
+      school_class = build(:school_class, code: '12-34-56', school:)
+      school_class.assign_class_code
+      expect(school_class.code).to eq('12-34-56')
+    end
+
+    it 'retries 5 times if the school code is not unique within the school' do
+      school_class = create(:school_class, code: '12-34-56', school:)
+      allow(ForEducationCodeGenerator).to receive(:generate).and_return(*([school_class.code] * 4), '00-00-00')
+      another_school_class = create(:school_class, school: school_class.school, code: nil)
+      another_school_class.assign_class_code
+      expect(another_school_class.code).to eq('00-00-00')
+    end
+
+    it 'raises adds error if unique code cannot be generated in 5 retries' do
+      school_class = create(:school_class, code: '12-34-56', school:)
+      allow(ForEducationCodeGenerator).to receive(:generate).and_return(*([school_class.code] * 5))
+      another_school_class = build(:school_class, school: school_class.school, code: nil)
+      another_school_class.assign_class_code
+      expect(another_school_class.errors[:code]).to include('could not be generated')
+    end
+
+    it 'does not add error if class code shared by class from another school' do
+      school_class = create(:school_class, code: '12-34-56', school:)
+      allow(ForEducationCodeGenerator).to receive(:generate).and_return(school_class.code)
+      another_school_class = build(:school_class, school: build(:school), code: nil)
+      another_school_class.assign_class_code
+      expect(another_school_class.errors[:code]).to be_empty
+    end
+  end
+
   describe 'auditing' do
     subject(:school_class) { create(:school_class, teacher_ids: [teacher.id], school:) }
 

spec/models/school_class_spec.rb

@@ -309,22 +309,22 @@
     end
 
     it 'uses the school code generator to generates and set the code' do
-      allow(SchoolCodeGenerator).to receive(:generate).and_return('00-00-00')
+      allow(ForEducationCodeGenerator).to receive(:generate).and_return('00-00-00')
       school.verify!
       expect(school.code).to eq('00-00-00')
     end
 
     it 'retries 5 times if the school code is not unique' do
       school.verify!
-      allow(SchoolCodeGenerator).to receive(:generate).and_return(school.code, school.code, school.code, school.code, '00-00-00')
+      allow(ForEducationCodeGenerator).to receive(:generate).and_return(school.code, school.code, school.code, school.code, '00-00-00')
       another_school = create(:school)
       another_school.verify!
       expect(another_school.code).to eq('00-00-00')
     end
 
     it 'raises exception if unique code cannot be generated in 5 retries' do
       school.verify!
-      allow(SchoolCodeGenerator).to receive(:generate).and_return(school.code, school.code, school.code, school.code, school.code)
+      allow(ForEducationCodeGenerator).to receive(:generate).and_return(school.code, school.code, school.code, school.code, school.code)
       another_school = create(:school)
       expect { another_school.verify! }.to raise_error(ActiveRecord::RecordInvalid)
     end