From f09c97275cfbc8e4ef06b4ca623e92b5b127ed6a Mon Sep 17 00:00:00 2001 From: Jamie Benstead Date: Thu, 12 Feb 2026 11:19:29 +0000 Subject: [PATCH 01/12] Remove validation and old tests --- app/models/teacher_invitation.rb | 7 ------- lib/concepts/school_student/create.rb | 5 ++--- spec/concepts/school_student/create_spec.rb | 9 --------- spec/concepts/school_teacher/invite_spec.rb | 9 --------- 4 files changed, 2 insertions(+), 28 deletions(-) diff --git a/app/models/teacher_invitation.rb b/app/models/teacher_invitation.rb index bb058c570..c93a67195 100644 --- a/app/models/teacher_invitation.rb +++ b/app/models/teacher_invitation.rb @@ -6,7 +6,6 @@ class TeacherInvitation < ApplicationRecord belongs_to :school validates :email_address, format: { with: EmailValidator.regexp, message: I18n.t('validations.invitation.email_address') } - validate :school_is_verified after_create_commit :send_invitation_email encrypts :email_address @@ -16,12 +15,6 @@ class TeacherInvitation < ApplicationRecord private - def school_is_verified - return if school.verified? - - errors.add(:school, 'is not verified') - end - def send_invitation_email InvitationMailer.with(invitation: self).invite_teacher.deliver_later end diff --git a/lib/concepts/school_student/create.rb b/lib/concepts/school_student/create.rb index 0d4d3925c..01e2463d9 100644 --- a/lib/concepts/school_student/create.rb +++ b/lib/concepts/school_student/create.rb @@ -22,7 +22,7 @@ def create_student(school, school_student_params, token) password = DecryptionHelpers.decrypt_password(encrypted_password) name = school_student_params.fetch(:name) - validate(school:, username:, password:, name:) + validate(username:, password:, name:) response = ProfileApiClient.create_school_student(token:, username:, password:, name:, school_id:) user_id = response[:created].first @@ -30,8 +30,7 @@ def create_student(school, school_student_params, token) user_id end - def validate(school:, username:, password:, name:) - raise ArgumentError, 'school is not verified' unless school.verified? + def validate(username:, password:, name:) raise ArgumentError, "username '#{username}' is invalid" if username.blank? raise ArgumentError, "password '#{password}' is invalid" if password.size < 8 raise ArgumentError, "name '#{name}' is invalid" if name.blank? diff --git a/spec/concepts/school_student/create_spec.rb b/spec/concepts/school_student/create_spec.rb index 4d2d87c5a..202b3815b 100644 --- a/spec/concepts/school_student/create_spec.rb +++ b/spec/concepts/school_student/create_spec.rb @@ -76,15 +76,6 @@ end end - context 'when the school is not verified' do - let(:school) { create(:school) } - - it 'returns the error message in the operation response' do - response = described_class.call(school:, school_student_params:, token:) - expect(response[:error]).to match(/school is not verified/) - end - end - context 'when the student cannot be created in profile api because of a 422 response' do let(:error) { { 'message' => "something's up with the username" } } let(:exception) { ProfileApiClient::Student422Error.new(error) } diff --git a/spec/concepts/school_teacher/invite_spec.rb b/spec/concepts/school_teacher/invite_spec.rb index 43439010b..31f851c3f 100644 --- a/spec/concepts/school_teacher/invite_spec.rb +++ b/spec/concepts/school_teacher/invite_spec.rb @@ -49,13 +49,4 @@ expect(Sentry).to have_received(:capture_exception).with(kind_of(StandardError)) end end - - context 'when the school is not verified' do - let(:school) { create(:school) } - - it 'returns the error message in the operation response' do - response = described_class.call(school:, school_teacher_params:, token:) - expect(response[:error]).to match(/School is not verified/) - end - end end From 36edf7f50dac3255dc2b51efb3466c639ed33519 Mon Sep 17 00:00:00 2001 From: Jamie Benstead Date: Thu, 12 Feb 2026 13:02:23 +0000 Subject: [PATCH 02/12] Change test to allow unverified schools --- spec/models/teacher_invitation_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/models/teacher_invitation_spec.rb b/spec/models/teacher_invitation_spec.rb index d77a33762..54df0ade6 100644 --- a/spec/models/teacher_invitation_spec.rb +++ b/spec/models/teacher_invitation_spec.rb @@ -18,11 +18,11 @@ expect(invitation).not_to be_valid end - it 'is invalid with an unverified school' do + it 'is valid with an unverified school' do school = build(:school, verified_at: nil) invitation = build(:teacher_invitation, school:) - expect(invitation).not_to be_valid + expect(invitation).to be_valid end it 'sends an invitation email after create' do From 1e5a266ab73c4d36fa554647014542da79210592 Mon Sep 17 00:00:00 2001 From: Jamie Benstead Date: Thu, 12 Feb 2026 13:02:41 +0000 Subject: [PATCH 03/12] Add test in around unverified schools --- spec/concepts/school_teacher/invite_spec.rb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/spec/concepts/school_teacher/invite_spec.rb b/spec/concepts/school_teacher/invite_spec.rb index 31f851c3f..7d6790f24 100644 --- a/spec/concepts/school_teacher/invite_spec.rb +++ b/spec/concepts/school_teacher/invite_spec.rb @@ -4,7 +4,7 @@ RSpec.describe SchoolTeacher::Invite, type: :unit do let(:token) { UserProfileMock::TOKEN } - let(:school) { create(:verified_school) } + let(:school) { create(:school) } let(:teacher_id) { SecureRandom.uuid } let(:school_teacher_params) do @@ -49,4 +49,13 @@ expect(Sentry).to have_received(:capture_exception).with(kind_of(StandardError)) end end + + context 'when the school is not verified' do + let(:school) { create(:school) } + + it 'does not return an error message in the operation response' do + response = described_class.call(school:, school_teacher_params:, token:) + expect(response[:error]).to be_blank + end + end end From f2c22bd4c5703eaa8c209ea16ce5240e3ec8653e Mon Sep 17 00:00:00 2001 From: Jamie Benstead Date: Thu, 19 Feb 2026 10:37:21 +0000 Subject: [PATCH 04/12] Create roles if do not exist when verifying school --- app/services/school_verification_service.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/services/school_verification_service.rb b/app/services/school_verification_service.rb index aaa3b7559..b9dffd9b9 100644 --- a/app/services/school_verification_service.rb +++ b/app/services/school_verification_service.rb @@ -12,6 +12,10 @@ def verify(token: nil) School.transaction do school.verify! + creator_id = school.creator_id + Role.owner.find_or_create_by!(user_id: creator_id, school:) + Role.teacher.find_or_create_by!(user_id: creator_id, school:) + # TODO: Remove this line, once the feature flag is retired success = FeatureFlags.immediate_school_onboarding? || SchoolOnboardingService.new(school).onboard(token: token) From 72faadb7310ef548ae114b2186d1d00ea6dfbd4b Mon Sep 17 00:00:00 2001 From: Matthew Trew Date: Thu, 19 Feb 2026 12:03:34 +0000 Subject: [PATCH 05/12] Only update roles in Immediate Onboarding branch --- app/services/school_verification_service.rb | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/app/services/school_verification_service.rb b/app/services/school_verification_service.rb index b9dffd9b9..00d0d6fae 100644 --- a/app/services/school_verification_service.rb +++ b/app/services/school_verification_service.rb @@ -12,12 +12,16 @@ def verify(token: nil) School.transaction do school.verify! - creator_id = school.creator_id - Role.owner.find_or_create_by!(user_id: creator_id, school:) - Role.teacher.find_or_create_by!(user_id: creator_id, school:) - - # TODO: Remove this line, once the feature flag is retired - success = FeatureFlags.immediate_school_onboarding? || SchoolOnboardingService.new(school).onboard(token: token) + if FeatureFlags.immediate_school_onboarding? + # For backwards compatibility with pre-Immediate Onboarding unverified schools, we need to create the roles + # if they don't exist - this didn't happen at the time the school was created. + creator_id = school.creator_id + Role.owner.find_or_create_by!(user_id: creator_id, school:) + Role.teacher.find_or_create_by!(user_id: creator_id, school:) + success = true + else + success = SchoolOnboardingService.new(school).onboard(token: token) + end # TODO: Remove this line, once the feature flag is retired raise ActiveRecord::Rollback unless success From e07f43a66668e485c01558a2ba4d376b826a229f Mon Sep 17 00:00:00 2001 From: Matthew Trew Date: Thu, 19 Feb 2026 13:33:27 +0000 Subject: [PATCH 06/12] Update tests for verification when Immediate Onboarding enabled --- spec/services/school_verification_service_spec.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/spec/services/school_verification_service_spec.rb b/spec/services/school_verification_service_spec.rb index 93ea0289b..e353f0d1f 100644 --- a/spec/services/school_verification_service_spec.rb +++ b/spec/services/school_verification_service_spec.rb @@ -36,6 +36,16 @@ it 'returns true' do expect(service.verify).to be(true) end + + it 'grants the creator the owner role for the school' do + service.verify(token:) + expect(school_creator).to be_school_owner(school) + end + + it 'grants the creator the teacher role for the school' do + service.verify(token:) + expect(school_creator).to be_school_teacher(school) + end end describe 'when school cannot be saved' do From 313435326f34366c3c4b9a93c6eccc48b2ff41b9 Mon Sep 17 00:00:00 2001 From: Jamie Benstead Date: Thu, 19 Feb 2026 14:40:37 +0000 Subject: [PATCH 07/12] Add back in validate wrapped in feature flag, add tests --- app/models/teacher_invitation.rb | 7 +++++++ lib/concepts/school_student/create.rb | 12 +++++++++-- spec/concepts/school_student/create_spec.rb | 23 +++++++++++++++++++++ spec/concepts/school_teacher/invite_spec.rb | 23 ++++++++++++++++++--- 4 files changed, 60 insertions(+), 5 deletions(-) diff --git a/app/models/teacher_invitation.rb b/app/models/teacher_invitation.rb index c93a67195..68576b5b7 100644 --- a/app/models/teacher_invitation.rb +++ b/app/models/teacher_invitation.rb @@ -6,6 +6,7 @@ class TeacherInvitation < ApplicationRecord belongs_to :school validates :email_address, format: { with: EmailValidator.regexp, message: I18n.t('validations.invitation.email_address') } + validate :school_is_verified, unless: -> { FeatureFlags.immediate_school_onboarding? } after_create_commit :send_invitation_email encrypts :email_address @@ -15,6 +16,12 @@ class TeacherInvitation < ApplicationRecord private + def school_is_verified + return if school.verified? + + errors.add(:school, 'is not verified') + end + def send_invitation_email InvitationMailer.with(invitation: self).invite_teacher.deliver_later end diff --git a/lib/concepts/school_student/create.rb b/lib/concepts/school_student/create.rb index 01e2463d9..1c0745d00 100644 --- a/lib/concepts/school_student/create.rb +++ b/lib/concepts/school_student/create.rb @@ -22,7 +22,12 @@ def create_student(school, school_student_params, token) password = DecryptionHelpers.decrypt_password(encrypted_password) name = school_student_params.fetch(:name) - validate(username:, password:, name:) + validate( + username:, + password:, + name:, + school: (FeatureFlags.immediate_school_onboarding? ? nil : school) + ) response = ProfileApiClient.create_school_student(token:, username:, password:, name:, school_id:) user_id = response[:created].first @@ -30,10 +35,13 @@ def create_student(school, school_student_params, token) user_id end - def validate(username:, password:, name:) + def validate(username:, password:, name:, school: nil) raise ArgumentError, "username '#{username}' is invalid" if username.blank? raise ArgumentError, "password '#{password}' is invalid" if password.size < 8 raise ArgumentError, "name '#{name}' is invalid" if name.blank? + + return unless school + raise ArgumentError, 'school must be verified' unless school.verified? end end end diff --git a/spec/concepts/school_student/create_spec.rb b/spec/concepts/school_student/create_spec.rb index 202b3815b..1981f0a00 100644 --- a/spec/concepts/school_student/create_spec.rb +++ b/spec/concepts/school_student/create_spec.rb @@ -76,6 +76,29 @@ end end + context 'when the school is not verified' do + let(:school) { create(:school) } # not verified by default + + context 'when immediate_school_onboarding is FALSE' do + it 'returns the error message in the operation response' do + ClimateControl.modify(ENABLE_IMMEDIATE_SCHOOL_ONBOARDING: 'false') do + response = described_class.call(school:, school_student_params:, token:) + expect(response[:error]).to match(/school must be verified/) + end + end + end + + context 'when immediate_school_onboarding is TRUE' do + it 'does not error due to school verification' do + ClimateControl.modify(ENABLE_IMMEDIATE_SCHOOL_ONBOARDING: 'true') do + response = described_class.call(school:, school_student_params:, token:) + + expect(response[:error]).to be_nil + end + end + end + end + context 'when the student cannot be created in profile api because of a 422 response' do let(:error) { { 'message' => "something's up with the username" } } let(:exception) { ProfileApiClient::Student422Error.new(error) } diff --git a/spec/concepts/school_teacher/invite_spec.rb b/spec/concepts/school_teacher/invite_spec.rb index 7d6790f24..3e8ece23c 100644 --- a/spec/concepts/school_teacher/invite_spec.rb +++ b/spec/concepts/school_teacher/invite_spec.rb @@ -53,9 +53,26 @@ context 'when the school is not verified' do let(:school) { create(:school) } - it 'does not return an error message in the operation response' do - response = described_class.call(school:, school_teacher_params:, token:) - expect(response[:error]).to be_blank + context 'when immediate_school_onboarding is FALSE' do + # before do + # allow(FeatureFlags).to receive(:immediate_school_onboarding?).and_return(false) + # end + + it 'does return an error message in the operation response' do + ClimateControl.modify(ENABLE_IMMEDIATE_SCHOOL_ONBOARDING: 'false') do + response = described_class.call(school:, school_teacher_params:, token:) + expect(response[:error]).to match(/is not verified/) + end + end + end + + context 'when immediate_school_onboarding is TRUE' do + it 'does not return an error message in the operation response' do + ClimateControl.modify(ENABLE_IMMEDIATE_SCHOOL_ONBOARDING: 'true') do + response = described_class.call(school:, school_teacher_params:, token:) + expect(response[:error]).to be_blank + end + end end end end From 263457cd0b9a262c3878b0b75ab0bfc38a902224 Mon Sep 17 00:00:00 2001 From: Jamie Benstead Date: Thu, 19 Feb 2026 14:53:49 +0000 Subject: [PATCH 08/12] Remove commented out code --- spec/concepts/school_teacher/invite_spec.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/spec/concepts/school_teacher/invite_spec.rb b/spec/concepts/school_teacher/invite_spec.rb index 3e8ece23c..91d599cd5 100644 --- a/spec/concepts/school_teacher/invite_spec.rb +++ b/spec/concepts/school_teacher/invite_spec.rb @@ -54,10 +54,6 @@ let(:school) { create(:school) } context 'when immediate_school_onboarding is FALSE' do - # before do - # allow(FeatureFlags).to receive(:immediate_school_onboarding?).and_return(false) - # end - it 'does return an error message in the operation response' do ClimateControl.modify(ENABLE_IMMEDIATE_SCHOOL_ONBOARDING: 'false') do response = described_class.call(school:, school_teacher_params:, token:) From 4deca608dd213265a06bf7f4673a70f5a7aebfe3 Mon Sep 17 00:00:00 2001 From: Jamie Benstead Date: Thu, 19 Feb 2026 15:46:00 +0000 Subject: [PATCH 09/12] Change to use verified school --- spec/concepts/school_teacher/invite_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/concepts/school_teacher/invite_spec.rb b/spec/concepts/school_teacher/invite_spec.rb index 91d599cd5..231d8cbda 100644 --- a/spec/concepts/school_teacher/invite_spec.rb +++ b/spec/concepts/school_teacher/invite_spec.rb @@ -4,7 +4,7 @@ RSpec.describe SchoolTeacher::Invite, type: :unit do let(:token) { UserProfileMock::TOKEN } - let(:school) { create(:school) } + let(:school) { create(:verified_school) } let(:teacher_id) { SecureRandom.uuid } let(:school_teacher_params) do From 8ac2fb792d1327ba2c9f44ba9bc034d98c125c37 Mon Sep 17 00:00:00 2001 From: Jamie Benstead Date: Thu, 19 Feb 2026 15:48:28 +0000 Subject: [PATCH 10/12] Add tests around feature flag --- spec/models/teacher_invitation_spec.rb | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/spec/models/teacher_invitation_spec.rb b/spec/models/teacher_invitation_spec.rb index 54df0ade6..d1706e08e 100644 --- a/spec/models/teacher_invitation_spec.rb +++ b/spec/models/teacher_invitation_spec.rb @@ -18,11 +18,22 @@ expect(invitation).not_to be_valid end - it 'is valid with an unverified school' do - school = build(:school, verified_at: nil) - invitation = build(:teacher_invitation, school:) + it 'is valid with an unverified school and immediate_school_onboarding is TRUE' do + ClimateControl.modify(ENABLE_IMMEDIATE_SCHOOL_ONBOARDING: 'true') do + school = build(:school, verified_at: nil) + invitation = build(:teacher_invitation, school:) - expect(invitation).to be_valid + expect(invitation).to be_valid + end + end + + it 'is invalid with an unverified school and immediate_school_onboarding is FALSE' do + ClimateControl.modify(ENABLE_IMMEDIATE_SCHOOL_ONBOARDING: 'false') do + school = build(:school, verified_at: nil) + invitation = build(:teacher_invitation, school:) + + expect(invitation).not_to be_valid + end end it 'sends an invitation email after create' do From c29357423bd92503c1870ef0581d53f298f2b87c Mon Sep 17 00:00:00 2001 From: Matthew Trew Date: Fri, 20 Feb 2026 15:12:16 +0000 Subject: [PATCH 11/12] Revert f2c22b 72faad --- app/services/school_verification_service.rb | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/app/services/school_verification_service.rb b/app/services/school_verification_service.rb index 00d0d6fae..aaa3b7559 100644 --- a/app/services/school_verification_service.rb +++ b/app/services/school_verification_service.rb @@ -12,16 +12,8 @@ def verify(token: nil) School.transaction do school.verify! - if FeatureFlags.immediate_school_onboarding? - # For backwards compatibility with pre-Immediate Onboarding unverified schools, we need to create the roles - # if they don't exist - this didn't happen at the time the school was created. - creator_id = school.creator_id - Role.owner.find_or_create_by!(user_id: creator_id, school:) - Role.teacher.find_or_create_by!(user_id: creator_id, school:) - success = true - else - success = SchoolOnboardingService.new(school).onboard(token: token) - end + # TODO: Remove this line, once the feature flag is retired + success = FeatureFlags.immediate_school_onboarding? || SchoolOnboardingService.new(school).onboard(token: token) # TODO: Remove this line, once the feature flag is retired raise ActiveRecord::Rollback unless success From 4318708ec7569fe087da602c9bbbdc75ab652a97 Mon Sep 17 00:00:00 2001 From: Matthew Trew Date: Fri, 20 Feb 2026 15:58:19 +0000 Subject: [PATCH 12/12] Revert "Update tests for verification when Immediate Onboarding enabled" This reverts commit e07f43a66668e485c01558a2ba4d376b826a229f. --- spec/services/school_verification_service_spec.rb | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/spec/services/school_verification_service_spec.rb b/spec/services/school_verification_service_spec.rb index e353f0d1f..93ea0289b 100644 --- a/spec/services/school_verification_service_spec.rb +++ b/spec/services/school_verification_service_spec.rb @@ -36,16 +36,6 @@ it 'returns true' do expect(service.verify).to be(true) end - - it 'grants the creator the owner role for the school' do - service.verify(token:) - expect(school_creator).to be_school_owner(school) - end - - it 'grants the creator the teacher role for the school' do - service.verify(token:) - expect(school_creator).to be_school_teacher(school) - end end describe 'when school cannot be saved' do