Ensure safeguarding flags before Profile student calls

abcampo-iry · abcampo-iry · commit bb08ab1b3203 · 2026-06-12T12:02:27.000+02:00
diff --git a/app/controllers/api/school_members_controller.rb b/app/controllers/api/school_members_controller.rb
@@ -6,8 +6,6 @@ class SchoolMembersController < ApiController
     load_and_authorize_resource :school
     authorize_resource :school_member, class: false
 
-    before_action :create_safeguarding_flags
-
     def index
       result = SchoolMember::List.call(school: @school, token: current_user.token)
 
@@ -18,34 +16,5 @@ def index
         render json: { error: result[:error] }, status: :unprocessable_content
       end
     end
-
-    private
-
-    def create_safeguarding_flags
-      create_teacher_safeguarding_flag
-      create_owner_safeguarding_flag
-    end
-
-    def create_teacher_safeguarding_flag
-      return unless current_user.school_teacher?(@school)
-
-      ProfileApiClient.create_safeguarding_flag(
-        token: current_user.token,
-        flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher],
-        email: current_user.email,
-        school_id: @school.id
-      )
-    end
-
-    def create_owner_safeguarding_flag
-      return unless current_user.school_owner?(@school)
-
-      ProfileApiClient.create_safeguarding_flag(
-        token: current_user.token,
-        flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner],
-        email: current_user.email,
-        school_id: @school.id
-      )
-    end
   end
 end
diff --git a/app/controllers/api/school_students_controller.rb b/app/controllers/api/school_students_controller.rb
@@ -10,8 +10,6 @@ class SchoolStudentsController < ApiController
     load_and_authorize_resource :school
     authorize_resource :school_student, class: false
 
-    before_action :create_safeguarding_flags
-
     def index
       result = SchoolStudent::List.call(school: @school, token: current_user.token)
 
@@ -183,32 +181,5 @@ def school_students_params
     def student_ids_params
       params.fetch(:student_ids, [])
     end
-
-    def create_safeguarding_flags
-      create_teacher_safeguarding_flag
-      create_owner_safeguarding_flag
-    end
-
-    def create_teacher_safeguarding_flag
-      return unless current_user.school_teacher?(@school)
-
-      ProfileApiClient.create_safeguarding_flag(
-        token: current_user.token,
-        flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher],
-        email: current_user.email,
-        school_id: @school.id
-      )
-    end
-
-    def create_owner_safeguarding_flag
-      return unless current_user.school_owner?(@school)
-
-      ProfileApiClient.create_safeguarding_flag(
-        token: current_user.token,
-        flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner],
-        email: current_user.email,
-        school_id: @school.id
-      )
-    end
   end
 end
diff --git a/app/jobs/create_students_job.rb b/app/jobs/create_students_job.rb
@@ -31,7 +31,9 @@ def self.attempt_perform_later(school_id:, students:, token:)
   end
 
   def perform(school_id:, students:, token:)
+    school = School.find(school_id)
     decrypted_students = StudentHelpers.decrypt_students(students)
+    SafeguardingFlagService.create_for_token(token:, school:)
     responses = ProfileApiClient.create_school_students(token:, students: decrypted_students, school_id:)
     return if responses[:created].blank?
 
diff --git a/app/services/safeguarding_flag_service.rb b/app/services/safeguarding_flag_service.rb
@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+class SafeguardingFlagService
+  class << self
+    def create_for_school_roles(user:, school:)
+      return if user.blank? || school.blank?
+
+      roles = []
+      roles << :teacher if user.school_teacher?(school)
+      roles << :owner if user.school_owner?(school)
+
+      create_for_roles(token: user.token, email: user.email, school:, roles:)
+    end
+
+    def create_for_token(token:, school:)
+      return if token.blank? || school.blank?
+
+      create_for_school_roles(user: User.from_token(token:), school:)
+    end
+
+    def create_for_roles(token:, email:, school:, roles:)
+      return if token.blank? || email.blank? || school.blank?
+
+      Array(roles).each do |role|
+        flag = ProfileApiClient::SAFEGUARDING_FLAGS[role.to_sym]
+        next if flag.blank?
+
+        ProfileApiClient.create_safeguarding_flag(
+          token:,
+          flag:,
+          email:,
+          school_id: school.id
+        )
+      end
+    end
+  end
+end
diff --git a/app/services/student_removal_service.rb b/app/services/student_removal_service.rb
@@ -34,7 +34,10 @@ def remove_students
           student_roles.destroy_all
 
           # Remove from profile if requested - inside transaction so it can be rolled back
-          ProfileApiClient.delete_school_student(token: @token, school_id: @school.id, student_id: user_id) if @remove_from_profile && @token.present?
+          if @remove_from_profile && @token.present?
+            SafeguardingFlagService.create_for_token(token: @token, school: @school)
+            ProfileApiClient.delete_school_student(token: @token, school_id: @school.id, student_id: user_id)
+          end
         end
       rescue StandardError => e
         result[:error] = "#{e.class}: #{e.message}"
diff --git a/lib/concepts/school_student/create.rb b/lib/concepts/school_student/create.rb
@@ -28,6 +28,7 @@ def create_student(school, school_student_params, token)
           name:
         )
 
+        SafeguardingFlagService.create_for_token(token:, school:)
         response = ProfileApiClient.create_school_student(token:, username:, password:, name:, school_id:)
         user_id = response[:created].first
         Role.student.create!(school:, user_id:)
diff --git a/lib/concepts/school_student/create_batch_sso.rb b/lib/concepts/school_student/create_batch_sso.rb
@@ -15,7 +15,7 @@ class << self
       def call(school:, school_students_params:, current_user:)
         response = OperationResponse.new
         response[:school_students] = []
-        response[:school_students] = create_batch_sso(school, school_students_params, current_user.token)
+        response[:school_students] = create_batch_sso(school, school_students_params, current_user)
         response
       rescue ValidationError => e
         response[:error] = "Error creating one or more students - see 'errors' key for details"
@@ -31,8 +31,10 @@ def call(school:, school_students_params:, current_user:)
 
       private
 
-      def create_batch_sso(school, students, token)
+      def create_batch_sso(school, students, current_user)
         students = normalize_student_params(students)
+        token = current_user.token
+        SafeguardingFlagService.create_for_school_roles(user: current_user, school:)
         responses = ProfileApiClient.create_school_students_sso(token:, students:, school_id: school.id)
 
         create_student_roles(school, responses)
diff --git a/lib/concepts/school_student/delete.rb b/lib/concepts/school_student/delete.rb
@@ -16,6 +16,7 @@ def call(school:, student_id:, token:)
       private
 
       def delete_student(school, student_id, token)
+        SafeguardingFlagService.create_for_token(token:, school:)
         ProfileApiClient.delete_school_student(token:, student_id:, school_id: school.id)
       end
     end
diff --git a/lib/concepts/school_student/list.rb b/lib/concepts/school_student/list.rb
@@ -19,6 +19,7 @@ def list_students(school, token, student_ids)
         student_ids ||= Role.student.where(school:).map(&:user_id)
         return [] if student_ids.empty?
 
+        SafeguardingFlagService.create_for_token(token:, school:)
         students = ProfileApiClient.list_school_students(token:, school_id: school.id, student_ids:)
         students.map do |student|
           User.new(student.to_h.slice(:id, :username, :name, :email).merge(sso_providers: student.ssoProviders))
diff --git a/lib/concepts/school_student/update.rb b/lib/concepts/school_student/update.rb
@@ -39,6 +39,7 @@ def update_student(school, student_id, school_student_params, token)
         validate(username:, password:, name:)
 
         # Prevent updating SSO students (students with ssoProviders present)
+        SafeguardingFlagService.create_for_token(token:, school:)
         student = ProfileApiClient.school_student(
           token: token,
           school_id: school.id,
diff --git a/lib/concepts/school_student/validate_batch.rb b/lib/concepts/school_student/validate_batch.rb
@@ -33,6 +33,7 @@ def call(school:, students:, token:)
 
       def validate_batch(school:, students:, token:)
         decrypted_students = StudentHelpers.decrypt_students(students)
+        SafeguardingFlagService.create_for_token(token:, school:)
         ProfileApiClient.validate_school_students(token:, students: decrypted_students, school_id: school.id)
       rescue ProfileApiClient::Student422Error => e
         handle_student422_error(e.errors)
diff --git a/spec/concepts/class_member/list_spec.rb b/spec/concepts/class_member/list_spec.rb
@@ -13,6 +13,10 @@
   let(:student_ids) { students.map(&:id) }
   let(:teacher_ids) { [teacher.id] }
 
+  before do
+    allow(SafeguardingFlagService).to receive(:create_for_token)
+  end
+
   context 'with students and a teacher' do
     before do
       student_ids.each do |student_id|
diff --git a/spec/concepts/school_member/list_spec.rb b/spec/concepts/school_member/list_spec.rb
@@ -11,6 +11,10 @@
   let(:student_ids) { students.map(&:id) }
   let(:teacher_ids) { [teacher.id] }
 
+  before do
+    allow(SafeguardingFlagService).to receive(:create_for_token)
+  end
+
   context 'with a mixture of students' do
     let(:sso_student) { create(:student, :sso, school:) }
     let(:standard_student) { create(:student, school:) }
diff --git a/spec/concepts/school_student/create_batch_sso_spec.rb b/spec/concepts/school_student/create_batch_sso_spec.rb
@@ -19,6 +19,10 @@
     ]
   end
 
+  before do
+    allow(SafeguardingFlagService).to receive(:create_for_school_roles)
+  end
+
   context 'when SSO creation succeeds' do
     let(:user_ids) { [SecureRandom.uuid, SecureRandom.uuid] }
 
@@ -39,6 +43,12 @@
         .with(token: current_user.token, students: school_students_params, school_id: school.id)
     end
 
+    it 'ensures the current user has a safeguarding flag before creating students' do
+      described_class.call(school:, school_students_params:, current_user:)
+
+      expect(SafeguardingFlagService).to have_received(:create_for_school_roles).with(user: current_user, school:)
+    end
+
     it 'transforms nil values to empty strings before API call' do
       params_with_nils = [
         { name: 'Test Student 1', email: nil },
diff --git a/spec/concepts/school_student/create_spec.rb b/spec/concepts/school_student/create_spec.rb
@@ -17,6 +17,7 @@
 
   before do
     stub_profile_api_create_school_student(user_id:)
+    allow(SafeguardingFlagService).to receive(:create_for_token)
   end
 
   it 'returns a successful operation response' do
diff --git a/spec/concepts/school_student/delete_spec.rb b/spec/concepts/school_student/delete_spec.rb
@@ -9,6 +9,7 @@
 
   before do
     stub_profile_api_delete_school_student
+    allow(SafeguardingFlagService).to receive(:create_for_token)
   end
 
   it 'returns a successful operation response' do
diff --git a/spec/concepts/school_student/list_spec.rb b/spec/concepts/school_student/list_spec.rb
@@ -7,6 +7,10 @@
   let(:school) { create(:school) }
   let(:students) { create_list(:student, 3, school:) }
 
+  before do
+    allow(SafeguardingFlagService).to receive(:create_for_token)
+  end
+
   context 'without student_ids' do
     before do
       student_attributes = students.map do |student|
@@ -31,6 +35,12 @@
       )
     end
 
+    it 'ensures the current user has a safeguarding flag before listing students' do
+      described_class.call(school:, token:)
+
+      expect(SafeguardingFlagService).to have_received(:create_for_token).with(token:, school:)
+    end
+
     it 'returns a school students JSON array' do
       response = described_class.call(school:, token:)
       expect(response[:school_students].size).to eq(3)
diff --git a/spec/concepts/school_student/update_spec.rb b/spec/concepts/school_student/update_spec.rb
@@ -18,6 +18,7 @@
   before do
     stub_profile_api_school_student # This is the call made to determine if it's an SSO student
     stub_profile_api_update_school_student
+    allow(SafeguardingFlagService).to receive(:create_for_token)
   end
 
   it 'returns a successful operation response' do
diff --git a/spec/concepts/school_student/validate_batch_spec.rb b/spec/concepts/school_student/validate_batch_spec.rb
@@ -6,6 +6,10 @@
   let(:school) { create(:verified_school) }
   let(:token) { UserProfileMock::TOKEN }
 
+  before do
+    allow(SafeguardingFlagService).to receive(:create_for_token)
+  end
+
   context 'when all students are valid' do
     let(:valid_students) do
       [
diff --git a/spec/features/class_member/creating_a_batch_of_class_members_spec.rb b/spec/features/class_member/creating_a_batch_of_class_members_spec.rb
@@ -9,6 +9,10 @@
   let(:students) { create_list(:student, 3, school:) }
   let(:teacher) { create(:teacher, school:) }
 
+  before do
+    stub_profile_api_create_safeguarding_flag
+  end
+
   context 'with valid params' do
     let(:student_attributes) do
       students.map do |student|
diff --git a/spec/features/class_member/creating_a_class_member_spec.rb b/spec/features/class_member/creating_a_class_member_spec.rb
@@ -8,11 +8,12 @@
   let(:school) { create(:school) }
   let(:student) { create(:student, school:, name: 'School Student', username: 'school-student') }
   let(:teacher) { create(:teacher, school:) }
+  let(:owner) { create(:owner, school:) }
 
   before do
-    owner = create(:owner, school:)
     authenticated_in_hydra_as(owner)
     stub_user_info_api_for(student)
+    stub_profile_api_create_safeguarding_flag
   end
 
   context 'with valid params' do
diff --git a/spec/features/class_member/listing_class_members_spec.rb b/spec/features/class_member/listing_class_members_spec.rb
@@ -17,6 +17,7 @@
       { id: student.id, name: student.name, username: student.username, email: nil }
     end
     stub_profile_api_list_school_students(school:, student_attributes:)
+    stub_profile_api_create_safeguarding_flag
 
     students.each do |student|
       create(:class_student, student_id: student.id, school_class:)
diff --git a/spec/features/school_class/importing_a_school_class_spec.rb b/spec/features/school_class/importing_a_school_class_spec.rb
@@ -6,6 +6,7 @@
   before do
     authenticated_in_hydra_as(teacher)
     stub_user_info_api_for(teacher)
+    stub_profile_api_create_safeguarding_flag
   end
 
   let(:headers) { { Authorization: UserProfileMock::TOKEN } }
diff --git a/spec/features/school_student/deleting_a_school_student_spec.rb b/spec/features/school_student/deleting_a_school_student_spec.rb
@@ -21,12 +21,14 @@
   end
 
   it 'creates the school owner safeguarding flag' do
-    delete("/api/schools/#{school.id}/students/#{student_id}", headers:)
+    student = create(:student, school:)
+    delete("/api/schools/#{school.id}/students/#{student.id}", headers:)
     expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], email: owner.email, school_id: school.id)
   end
 
   it 'does not create the school teacher safeguarding flag' do
-    delete("/api/schools/#{school.id}/students/#{student_id}", headers:)
+    student = create(:student, school:)
+    delete("/api/schools/#{school.id}/students/#{student.id}", headers:)
     expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: owner.email, school_id: school.id)
   end
 
diff --git a/spec/jobs/create_students_job_spec.rb b/spec/jobs/create_students_job_spec.rb
@@ -27,6 +27,7 @@
 
   before do
     stub_profile_api_create_school_students(user_ids: [user_id])
+    allow(SafeguardingFlagService).to receive(:create_for_token)
   end
 
   after do
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
diff --git a/spec/requests/projects/remix_spec.rb b/spec/requests/projects/remix_spec.rb
diff --git a/spec/requests/projects/show_spec.rb b/spec/requests/projects/show_spec.rb
diff --git a/spec/services/safeguarding_flag_service_spec.rb b/spec/services/safeguarding_flag_service_spec.rb
diff --git a/spec/services/student_removal_service_spec.rb b/spec/services/student_removal_service_spec.rb

Original file line number	Diff line number	Diff line change
`@@ -28,6 +28,7 @@ def create_student(school, school_student_params, token)`
`28`	`28`	`name:`
`29`	`29`	`)`
`30`	`30`
	`31`	`+ SafeguardingFlagService.create_for_token(token:, school:)`
`31`	`32`	`response = ProfileApiClient.create_school_student(token:, username:, password:, name:, school_id:)`
`32`	`33`	`user_id = response[:created].first`
`33`	`34`	`Role.student.create!(school:, user_id:)`