diff --git a/app/controllers/api/school_members_controller.rb b/app/controllers/api/school_members_controller.rb index 9c5156fc1..7aa0cc489 100644 --- 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 index 299f5eda3..ba67dde6f 100644 --- 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/controllers/concerns/identifiable.rb b/app/controllers/concerns/identifiable.rb index d707a61fb..82f1b60b6 100644 --- a/app/controllers/concerns/identifiable.rb +++ b/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 diff --git a/app/jobs/create_students_job.rb b/app/jobs/create_students_job.rb index 7dc5688ca..06170c135 100644 --- 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 new file mode 100644 index 000000000..da9b18c85 --- /dev/null +++ b/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 diff --git a/app/services/student_removal_service.rb b/app/services/student_removal_service.rb index 1c3b1c767..c4d3bed53 100644 --- a/app/services/student_removal_service.rb +++ b/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 diff --git a/lib/concepts/class_member/operations/list.rb b/lib/concepts/class_member/operations/list.rb index 9f588b8ec..7ef3ac3f4 100644 --- a/lib/concepts/class_member/operations/list.rb +++ b/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 diff --git a/lib/concepts/school_member/list.rb b/lib/concepts/school_member/list.rb index ade14601a..7402ea2a0 100644 --- a/lib/concepts/school_member/list.rb +++ b/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 diff --git a/lib/concepts/school_student/create.rb b/lib/concepts/school_student/create.rb index 2954c4fb4..4dc5a5209 100644 --- 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 index 8a04f94bc..e1c18dd5d 100644 --- 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 index 3f06ac1f6..1b406848a 100644 --- 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 index 4080f40ed..357aba0d4 100644 --- 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 index 46f44be71..c3745085c 100644 --- 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 index 09b51c928..30de52c2e 100644 --- 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 index e13171426..ae5f54ad2 100644 --- 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| @@ -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:) diff --git a/spec/concepts/school_member/list_spec.rb b/spec/concepts/school_member/list_spec.rb index 96670af8d..bb7a20690 100644 --- 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:) } @@ -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:) diff --git a/spec/concepts/school_student/create_batch_sso_spec.rb b/spec/concepts/school_student/create_batch_sso_spec.rb index 8aa0ae391..b7bea3982 100644 --- 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 index 202b3815b..abb19c7f3 100644 --- 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 index 32c26daba..5476a417b 100644 --- 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 index 5015cd99e..26711738c 100644 --- 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 index b24d0152c..be6a7f662 100644 --- 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 index a45af5978..8903c58a9 100644 --- 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 index f5ad035ba..d99b4b1ae 100644 --- 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 index 52e672d88..a4541a17d 100644 --- 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 index f94800067..7a419063b 100644 --- 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 index ccb0c0b98..9c54515bc 100644 --- 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 index ce34d2912..d96969fbe 100644 --- 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 index bc4a76193..7b6d1b007 100644 --- 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 index 97ebc000c..d3548edd1 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -193,6 +193,7 @@ before do stub_profile_api_list_school_students(school:, student_attributes:) + allow(SafeguardingFlagService).to receive(:create_for_token) end it 'returns User instances for the current scope' do @@ -227,6 +228,7 @@ before do stub_profile_api_list_school_students(school:, student_attributes:) + allow(SafeguardingFlagService).to receive(:create_for_token) end it 'returns an array of class members paired with their User instance' do @@ -264,6 +266,7 @@ before do stub_profile_api_list_school_students(school:, student_attributes:) + allow(SafeguardingFlagService).to receive(:create_for_token) end it 'returns the class member paired with their User instance' do diff --git a/spec/requests/projects/remix_spec.rb b/spec/requests/projects/remix_spec.rb index 80bf8a177..1275a8639 100644 --- a/spec/requests/projects/remix_spec.rb +++ b/spec/requests/projects/remix_spec.rb @@ -28,6 +28,7 @@ before do authenticated_in_hydra_as(owner) + stub_profile_api_create_safeguarding_flag end describe '#index' do diff --git a/spec/requests/projects/show_spec.rb b/spec/requests/projects/show_spec.rb index 74c515392..f5aa0241b 100644 --- a/spec/requests/projects/show_spec.rb +++ b/spec/requests/projects/show_spec.rb @@ -13,6 +13,7 @@ before do authenticated_in_hydra_as(teacher) stub_profile_api_list_school_students(school:, student_attributes: [{ name: 'Joe Bloggs' }]) + stub_profile_api_create_safeguarding_flag end context 'when loading own project' do diff --git a/spec/services/safeguarding_flag_service_spec.rb b/spec/services/safeguarding_flag_service_spec.rb new file mode 100644 index 000000000..497ee9384 --- /dev/null +++ b/spec/services/safeguarding_flag_service_spec.rb @@ -0,0 +1,140 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe SafeguardingFlagService do + let(:school) { create(:school) } + let(:token) { UserProfileMock::TOKEN } + + before do + RequestStore.store[:safeguarding_flag_users_by_token] = {} + allow(ProfileApiClient).to receive(:create_safeguarding_flag) + end + + describe '.create_for_school_roles' do + context 'when the user is a school owner' do + let(:user) { create(:owner, school:, token:) } + + it 'creates the school owner safeguarding flag' do + described_class.create_for_school_roles(user:, school:) + + expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with( + token: user.token, + flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], + email: user.email, + school_id: school.id + ) + end + end + + context 'when the user is a school teacher' do + let(:user) { create(:teacher, school:, token:) } + + it 'creates the school teacher safeguarding flag' do + described_class.create_for_school_roles(user:, school:) + + expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with( + token: user.token, + flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], + email: user.email, + school_id: school.id + ) + end + end + + context 'when the user is a school student' do + let(:user) { create(:student, school:, token:) } + + it 'does not create a safeguarding flag' do + described_class.create_for_school_roles(user:, school:) + + expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag) + end + end + end + + describe '.create_for_token' do + let(:user) { create(:teacher, school:, token:) } + + before do + allow(User).to receive(:from_token).with(token:).and_return(user) + allow(described_class).to receive(:request_store_active?).and_return(false) + end + + it 'creates flags for the user identified by the token' do + described_class.create_for_token(token:, school:) + + expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with( + token:, + flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], + email: user.email, + school_id: school.id + ) + end + + it 'uses a user cached for the token' do + allow(described_class).to receive(:request_store_active?).and_return(true) + RequestStore.store[:safeguarding_flag_users_by_token][token] = user + + described_class.create_for_token(token:, school:) + + expect(User).not_to have_received(:from_token) + expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with( + token:, + flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], + email: user.email, + school_id: school.id + ) + end + + it 'caches the user after looking it up' do + allow(described_class).to receive(:request_store_active?).and_return(true) + + 2.times { described_class.create_for_token(token:, school:) } + + expect(User).to have_received(:from_token).once + end + + it 'does not cache token lookups outside an active request store' do + 2.times { described_class.create_for_token(token:, school:) } + + expect(User).to have_received(:from_token).twice + end + + it 'does not look up a user without a token' do + described_class.create_for_token(token: nil, school:) + + expect(User).not_to have_received(:from_token) + end + end + + describe '.create_for_roles' do + it 'creates each requested teacher or owner flag' do + described_class.create_for_roles( + token:, + email: 'user@example.com', + school:, + roles: %i[owner teacher student] + ) + + expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with( + token:, + flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], + email: 'user@example.com', + school_id: school.id + ) + expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with( + token:, + flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], + email: 'user@example.com', + school_id: school.id + ) + end + + it 'does not create flags without an email' do + described_class.create_for_roles(token:, email: nil, school:, roles: %i[owner teacher]) + + expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag) + end + end +end diff --git a/spec/services/student_removal_service_spec.rb b/spec/services/student_removal_service_spec.rb index ec493d5c9..64b462389 100644 --- a/spec/services/student_removal_service_spec.rb +++ b/spec/services/student_removal_service_spec.rb @@ -13,6 +13,7 @@ before do allow(ProfileApiClient).to receive(:delete_school_student) + allow(SafeguardingFlagService).to receive(:create_for_token) create(:class_student, student_id: student.id, school_class: school_class) end @@ -145,6 +146,16 @@ expect(ProfileApiClient).to have_received(:delete_school_student).with(token: token, school_id: school.id, student_id: student.id) end + it 'creates the safeguarding flag once when removing multiple students from Profile' do + token = 'abc123' + second_student = create(:student, school: school) + service = described_class.new(students: [student.id, second_student.id], school: school, remove_from_profile: true, token: token) + + service.remove_students + + expect(SafeguardingFlagService).to have_received(:create_for_token).with(token: token, school: school).once + end + it 'does not call ProfileApiClient if remove_from_profile is false' do service = described_class.new(students: [student.id], school: school, remove_from_profile: false, token: 'token') service.remove_students