Ensure safeguarding flags before Profile student calls (#866)

## Status

- Closes
RaspberryPiFoundation/digital-editor-issues#1475

## What's changed?

This change ensures safeguarding flags are created before editor-api
calls Profile’s protected school-student endpoints.

Instead of relying on individual controllers to create the flags, flag
creation now happens at the `SchoolStudent` / Profile API boundary. This
covers controller and non-controller callers, including class member
listing, project student-name lookups, SSO student imports, batch
student creation jobs, and student removal.

The duplicated safeguarding flag logic has been removed from
`SchoolStudentsController` and `SchoolMembersController`, and a new
`SafeguardingFlagService` centralizes owner/teacher flag creation.

Member listing now also propagates `SchoolStudent::List` failures
instead of treating failed student lookups as empty lists. This prevents
school/class member endpoints from returning successful but incomplete
responses when safeguarding flag creation or Profile API calls fail.

## Steps to perform after deploying to production

_If the production environment requires any extra work after this PR has
been deployed detail it here. This could be running a Rake task, a
migration, or upgrading a Gem. That kind of thing._

Author: abcampo-iry

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,13 @@ module Identifiable
 
   def load_current_user
     token = request.headers['Authorization']
-    @current_user = User.from_token(token:) if token
+    return if token.blank?
+
+    @current_user = User.from_token(token:)
+    return if @current_user.blank?
+    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

@@ -12,6 +12,8 @@ def initialize(students:, school:, remove_from_profile: false, token: nil)
   def remove_students
     results = []
 
+    ensure_safeguarding_flag if remove_from_profile?
+
     @students.each do |user_id|
       student_roles = Role.student.where(user_id:, school_id: @school.id)
       if student_roles.empty?
@@ -33,8 +35,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 +45,18 @@ def remove_students
     end
     results
   end
+
+  private
+
+  def delete_from_profile(user_id)
+    ProfileApiClient.delete_school_student(token: @token, school_id: @school.id, student_id: user_id)
+  end
+
+  def ensure_safeguarding_flag
+    SafeguardingFlagService.create_for_token(token: @token, school: @school)
+  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)