Ensure safeguarding flags before Profile student calls

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

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

app/controllers/concerns/identifiable.rb

@@ -10,6 +10,12 @@ module Identifiable
 
   def load_current_user
     token = request.headers['Authorization']
-    @current_user = User.from_token(token:) if token
+    return unless token
+
+    @current_user = User.from_token(token:)
+    return unless RequestStore.respond_to?(:active?) && RequestStore.active?
+
+    RequestStore.store[:safeguarding_flag_users_by_token] ||= {}
+    RequestStore.store[:safeguarding_flag_users_by_token][token] = @current_user
   end
 end

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?
 

app/services/safeguarding_flag_service.rb

@@ -0,0 +1,48 @@
+# frozen_string_literal: true
+
+class SafeguardingFlagService
+  class << self
+    def create_for_school_roles(user:, school:)
+      return if user.blank? || school.blank?
+
+      create_for_roles(token: user.token, email: user.email, school:, roles: user.school_roles(school))
+    end
+
+    def create_for_token(token:, school:)
+      return if token.blank? || school.blank?
+
+      create_for_school_roles(user: user_for_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
+
+    private
+
+    def user_for_token(token)
+      return User.from_token(token:) unless request_store_active?
+
+      cache = RequestStore.store[:safeguarding_flag_users_by_token] ||= {}
+      return cache[token] if cache.key?(token)
+
+      cache[token] = User.from_token(token:)
+    end
+
+    def request_store_active?
+      RequestStore.respond_to?(:active?) && RequestStore.active?
+    end
+  end
+end

app/services/student_removal_service.rb

@@ -33,8 +33,8 @@ def remove_students
           # Remove roles
           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?
+          # Keep local DB changes uncommitted until Profile confirms deletion.
+          delete_from_profile(user_id) if remove_from_profile?
         end
       rescue StandardError => e
         result[:error] = "#{e.class}: #{e.message}"
@@ -43,4 +43,22 @@ def remove_students
     end
     results
   end
+
+  private
+
+  def delete_from_profile(user_id)
+    ensure_safeguarding_flag
+    ProfileApiClient.delete_school_student(token: @token, school_id: @school.id, student_id: user_id)
+  end
+
+  def ensure_safeguarding_flag
+    return if @safeguarding_flag_created
+
+    SafeguardingFlagService.create_for_token(token: @token, school: @school)
+    @safeguarding_flag_created = true
+  end
+
+  def remove_from_profile?
+    @remove_from_profile && @token.present?
+  end
 end

lib/concepts/class_member/operations/list.rb

@@ -10,7 +10,13 @@ def call(school_class:, class_students:, token:)
         begin
           school = school_class.school
           student_ids = class_students.pluck(:student_id)
-          students = SchoolStudent::List.call(school:, token:, student_ids:).fetch(:school_students, [])
+          students_response = SchoolStudent::List.call(school:, token:, student_ids:)
+          if students_response.failure?
+            response[:error] = students_response[:error]
+            return response
+          end
+
+          students = students_response.fetch(:school_students, [])
           class_students.each do |member|
             member.student = students.find { |student| student.id == member.student_id }
           end

lib/concepts/school_member/list.rb

@@ -15,7 +15,13 @@ def call(school:, token:)
         response[:school_members] = []
 
         begin
-          students = fetch_students(school:, token:)
+          students_response = fetch_students(school:, token:)
+          if students_response.failure?
+            response[:error] = students_response[:error]
+            return response
+          end
+
+          students = build_student_members(students_response.fetch(:school_students, []))
           teachers = fetch_teachers(school:)
           owners = fetch_owners(school:)
 
@@ -36,11 +42,13 @@ def call(school:, token:)
       private
 
       def fetch_students(school:, token:)
-        student_roles = Role.student.where(school:)
+        return OperationResponse[school_students: []] unless Role.student.exists?(school:)
 
-        students_response = student_roles.any? ? SchoolStudent::List.call(school:, token:).fetch(:school_students, []) : []
+        SchoolStudent::List.call(school:, token:)
+      end
 
-        students_response.map do |student|
+      def build_student_members(students)
+        students.map do |student|
           SchoolMember.new(student.id, student.name, student.username, student.email, :student, student.sso_providers)
         end
       end

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:)

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)

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

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))

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,

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)

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|
@@ -68,8 +72,23 @@
       expect(Sentry).to have_received(:capture_exception).with(instance_of(StandardError))
     end
 
+    it 'propagates student listing operation errors' do
+      students.each do |student|
+        create(:class_student, school_class:, student_id: student.id)
+      end
+      allow(SchoolStudent::List).to receive(:call).and_return(
+        OperationResponse[error: 'Error listing school students: Some API error']
+      )
+
+      response = described_class.call(school_class:, class_students:, token:)
+
+      expect(response.failure?).to be(true)
+      expect(response[:error]).to eq('Error listing school students: Some API error')
+      expect(response[:class_members]).to eq([])
+    end
+
     it 'returns an empty array when no ids match' do
-      allow(SchoolStudent::List).to receive(:call).and_return({ school_students: [] })
+      allow(SchoolStudent::List).to receive(:call).and_return(OperationResponse[school_students: []])
       allow(SchoolTeacher::List).to receive(:call).and_return({ school_teachers: [] })
 
       response = described_class.call(school_class:, class_students:, token:)

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:) }
@@ -110,8 +114,20 @@
       expect(Sentry).to have_received(:capture_exception).with(instance_of(StandardError))
     end
 
+    it 'propagates student listing operation errors' do
+      allow(SchoolStudent::List).to receive(:call).and_return(
+        OperationResponse[error: 'Error listing school students: Some API error']
+      )
+
+      response = described_class.call(school:, token:)
+
+      expect(response.failure?).to be(true)
+      expect(response[:error]).to eq('Error listing school students: Some API error')
+      expect(response[:school_members]).to eq([])
+    end
+
     it 'returns an empty array when no ids match' do
-      allow(SchoolStudent::List).to receive(:call).and_return({ school_students: [] })
+      allow(SchoolStudent::List).to receive(:call).and_return(OperationResponse[school_students: []])
       allow(SchoolTeacher::List).to receive(:call).and_return({ school_teachers: [] })
 
       response = described_class.call(school:, token:)

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 },