diff --git a/app/controllers/api/school_members_controller.rb b/app/controllers/api/school_members_controller.rb index aef765752..3c71593fe 100644 --- a/app/controllers/api/school_members_controller.rb +++ b/app/controllers/api/school_members_controller.rb @@ -32,7 +32,8 @@ def create_teacher_safeguarding_flag ProfileApiClient.create_safeguarding_flag( token: current_user.token, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], - email: current_user.email + email: current_user.email, + school_id: @school.id ) end @@ -42,7 +43,8 @@ def create_owner_safeguarding_flag ProfileApiClient.create_safeguarding_flag( token: current_user.token, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], - email: current_user.email + email: current_user.email, + school_id: @school.id ) end end diff --git a/app/controllers/api/school_students_controller.rb b/app/controllers/api/school_students_controller.rb index a6512b65e..14c1fedef 100644 --- a/app/controllers/api/school_students_controller.rb +++ b/app/controllers/api/school_students_controller.rb @@ -195,7 +195,8 @@ def create_teacher_safeguarding_flag ProfileApiClient.create_safeguarding_flag( token: current_user.token, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], - email: current_user.email + email: current_user.email, + school_id: @school.id ) end @@ -205,7 +206,8 @@ def create_owner_safeguarding_flag ProfileApiClient.create_safeguarding_flag( token: current_user.token, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], - email: current_user.email + email: current_user.email, + school_id: @school.id ) end end diff --git a/app/services/school_onboarding_service.rb b/app/services/school_onboarding_service.rb index ea82e439e..2bdb9926a 100644 --- a/app/services/school_onboarding_service.rb +++ b/app/services/school_onboarding_service.rb @@ -14,6 +14,11 @@ def onboard(token:) ProfileApiClient.create_school(token:, id: school.id, code: school.code) end + rescue ProfileApiClient::UnauthorizedError + # Do not log noise to sentry. + # TODO: consider returning a separate error here to distinguish from other errors and return 401 from the API, not 422 + Rails.logger.warn { "Failed to onboard school #{@school.id}: user is unauthorized" } + false rescue StandardError => e Sentry.capture_exception(e) Rails.logger.error { "Failed to onboard school #{@school.id}: #{e.message}" } diff --git a/lib/profile_api_client.rb b/lib/profile_api_client.rb index bd70fe796..c550da352 100644 --- a/lib/profile_api_client.rb +++ b/lib/profile_api_client.rb @@ -8,7 +8,7 @@ class ProfileApiClient # rubocop:disable Naming/MethodName School = Data.define(:id, :schoolCode, :updatedAt, :createdAt, :discardedAt) - SafeguardingFlag = Data.define(:id, :userId, :flag, :email, :createdAt, :updatedAt, :discardedAt) + SafeguardingFlag = Data.define(:id, :userId, :schoolId, :flag, :email, :createdAt, :updatedAt, :discardedAt) Student = Data.define(:id, :schoolId, :name, :username, :createdAt, :updatedAt, :discardedAt, :email, :ssoProviders) # rubocop:enable Naming/MethodName @@ -27,6 +27,8 @@ def initialize(errors) end end + class UnauthorizedError < Error; end + class UnexpectedResponse < Error attr_reader :response_status, :response_headers, :response_body @@ -50,6 +52,7 @@ def create_school(token:, id:, code:) } end + unauthorized!(response) raise UnexpectedResponse, response unless response.status == 201 School.new(**response.body) @@ -58,6 +61,7 @@ def create_school(token:, id:, code:) def school_student(token:, school_id:, student_id:) response = connection(token).get("/api/v1/schools/#{school_id}/students/#{student_id}") + unauthorized!(response) raise UnexpectedResponse, response unless response.status == 200 build_student(response.body) @@ -70,6 +74,7 @@ def list_school_students(token:, school_id:, student_ids:) request.body = student_ids end + unauthorized!(response) raise UnexpectedResponse, response unless response.status == 200 response.body.map { |attrs| build_student(attrs) } @@ -86,6 +91,7 @@ def create_school_student(token:, username:, password:, name:, school_id:) }] end + unauthorized!(response) raise UnexpectedResponse, response unless response.status == 201 response.body.deep_symbolize_keys @@ -103,6 +109,7 @@ def validate_school_students(token:, students:, school_id:) request.headers['Content-Type'] = 'application/json' end + unauthorized!(response) raise UnexpectedResponse, response unless response.status == 200 rescue Faraday::UnprocessableEntityError => e raise Student422Error, JSON.parse(e.response_body)['errors'] @@ -119,6 +126,7 @@ def create_school_students(token:, students:, school_id:, preflight: false) request.headers['Content-Type'] = 'application/json' end + unauthorized!(response) raise UnexpectedResponse, response unless [200, 201].include?(response.status) response.body.deep_symbolize_keys @@ -136,6 +144,7 @@ def create_school_students_sso(token:, students:, school_id:) request.headers['Content-Type'] = 'application/json' end + unauthorized!(response) raise UnexpectedResponse, response unless [200, 201].include?(response.status) response.body.map(&:deep_symbolize_keys) @@ -154,6 +163,7 @@ def update_school_student(token:, school_id:, student_id:, name: nil, username: }.compact end + unauthorized!(response) raise UnexpectedResponse, response unless response.status == 200 build_student(response.body) @@ -166,28 +176,32 @@ def delete_school_student(token:, school_id:, student_id:) response = connection(token).delete("/api/v1/schools/#{school_id}/students/#{student_id}") + unauthorized!(response) raise UnexpectedResponse, response unless response.status == 204 end def safeguarding_flags(token:) response = connection(token).get('/api/v1/safeguarding-flags') + unauthorized!(response) raise UnexpectedResponse, response unless response.status == 200 response.body.map { |flag| SafeguardingFlag.new(**flag.symbolize_keys) } end - def create_safeguarding_flag(token:, flag:, email:) + def create_safeguarding_flag(token:, flag:, email:, school_id:) response = connection(token).post('/api/v1/safeguarding-flags') do |request| - request.body = { flag:, email: } + request.body = { flag:, email:, schoolId: school_id } end + unauthorized!(response) raise UnexpectedResponse, response unless [201, 303].include?(response.status) end def delete_safeguarding_flag(token:, flag:) response = connection(token).delete("/api/v1/safeguarding-flags/#{flag}") + unauthorized!(response) raise UnexpectedResponse, response unless response.status == 204 end @@ -197,7 +211,7 @@ def connection(token) Faraday.new(ENV.fetch('IDENTITY_URL')) do |faraday| faraday.request :json faraday.response :json - faraday.response :raise_error + faraday.response :raise_error, allowed_statuses: [401] faraday.headers = { 'Accept' => 'application/json', 'Authorization' => "Bearer #{token}", @@ -229,5 +243,10 @@ def build_student(attrs) Student.new(**symbolized_attrs) end + + def unauthorized!(response) + # The API is only available to verified non-student users that are over 13. Others get a 401. + raise UnauthorizedError, 'Profile API unauthorized' if response.status == 401 + end end end diff --git a/spec/features/school/creating_a_school_spec.rb b/spec/features/school/creating_a_school_spec.rb index ca5a4b69f..487e4635c 100644 --- a/spec/features/school/creating_a_school_spec.rb +++ b/spec/features/school/creating_a_school_spec.rb @@ -4,12 +4,12 @@ RSpec.describe 'Creating a school', type: :request do before do - authenticated_in_hydra_as(owner) + authenticated_in_hydra_as(user) + stub_profile_api_create_school end let(:headers) { { Authorization: UserProfileMock::TOKEN } } - let(:school) { create(:school) } - let(:owner) { create(:owner, school:) } + let(:user) { create(:user) } let(:params) do { diff --git a/spec/features/school_member/listing_school_members_spec.rb b/spec/features/school_member/listing_school_members_spec.rb index 714a97d56..7ad5e9474 100644 --- a/spec/features/school_member/listing_school_members_spec.rb +++ b/spec/features/school_member/listing_school_members_spec.rb @@ -94,12 +94,12 @@ it 'creates the school owner safeguarding flag' do get("/api/schools/#{school.id}/students", headers:) - expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], email: owner.email) + 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 get("/api/schools/#{school.id}/students", headers:) - expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: owner.email) + 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 it "responds with nil attributes for students if the user profile doesn't exist" do @@ -136,7 +136,7 @@ authenticated_in_hydra_as(teacher) get("/api/schools/#{school.id}/students", headers:) - expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], email: owner.email) + expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], email: owner.email, school_id: school.id) end it 'creates the school teacher safeguarding flag when the user is a school teacher' do @@ -144,6 +144,6 @@ authenticated_in_hydra_as(teacher) get("/api/schools/#{school.id}/students", headers:) - expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: teacher.email) + expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: teacher.email, school_id: school.id) end end diff --git a/spec/features/school_student/creating_a_batch_of_school_students_spec.rb b/spec/features/school_student/creating_a_batch_of_school_students_spec.rb index 3310870fb..31c386cd5 100644 --- a/spec/features/school_student/creating_a_batch_of_school_students_spec.rb +++ b/spec/features/school_student/creating_a_batch_of_school_students_spec.rb @@ -56,12 +56,12 @@ it 'creates the school owner safeguarding flag' do post("/api/schools/#{school.id}/students/batch", headers:, params:) - expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], email: owner.email) + 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 post("/api/schools/#{school.id}/students/batch", headers:, params:) - expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: owner.email) + 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 it 'responds 202 Accepted' do @@ -113,7 +113,7 @@ authenticated_in_hydra_as(teacher) post("/api/schools/#{school.id}/students/batch", headers:, params:) - expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], email: owner.email) + expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], email: owner.email, school_id: school.id) end it 'creates the school teacher safeguarding flag when the user is a school-teacher' do @@ -121,7 +121,7 @@ authenticated_in_hydra_as(teacher) post("/api/schools/#{school.id}/students/batch", headers:, params:) - expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: teacher.email) + expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: teacher.email, school_id: school.id) end it 'responds 422 Unprocessable Entity when params are invalid' do diff --git a/spec/features/school_student/creating_a_school_student_spec.rb b/spec/features/school_student/creating_a_school_student_spec.rb index bb01c2bce..c41a40dba 100644 --- a/spec/features/school_student/creating_a_school_student_spec.rb +++ b/spec/features/school_student/creating_a_school_student_spec.rb @@ -25,12 +25,12 @@ it 'creates the school owner safeguarding flag' do post("/api/schools/#{school.id}/students", headers:, params:) - expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], email: owner.email) + 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 post("/api/schools/#{school.id}/students", headers:, params:) - expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: owner.email) + 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 it 'responds 204 No Content' do @@ -51,7 +51,7 @@ authenticated_in_hydra_as(teacher) post("/api/schools/#{school.id}/students", headers:, params:) - expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], email: owner.email) + expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], email: owner.email, school_id: school.id) end it 'creates the school teacher safeguarding flag when the user is a school teacher' do @@ -59,7 +59,7 @@ authenticated_in_hydra_as(teacher) post("/api/schools/#{school.id}/students", headers:, params:) - expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: teacher.email) + expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: teacher.email, school_id: school.id) end it 'responds 400 Bad Request when params are missing' do 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 04f188574..ce34d2912 100644 --- a/spec/features/school_student/deleting_a_school_student_spec.rb +++ b/spec/features/school_student/deleting_a_school_student_spec.rb @@ -22,12 +22,12 @@ it 'creates the school owner safeguarding flag' do 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) + 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:) - expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: owner.email) + 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 it 'responds 200 OK' do @@ -61,7 +61,7 @@ authenticated_in_hydra_as(teacher) 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[:owner], email: owner.email) + expect(ProfileApiClient).not_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 when logged in as a teacher' do @@ -69,7 +69,7 @@ authenticated_in_hydra_as(teacher) 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: teacher.email) + expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: teacher.email, school_id: school.id) end it 'responds 403 Forbidden when the user is a school-student' do diff --git a/spec/features/school_student/listing_school_students_spec.rb b/spec/features/school_student/listing_school_students_spec.rb index b5e5831fe..94dc581ad 100644 --- a/spec/features/school_student/listing_school_students_spec.rb +++ b/spec/features/school_student/listing_school_students_spec.rb @@ -22,12 +22,12 @@ it 'creates the school owner safeguarding flag' do get("/api/schools/#{school.id}/students", headers:) - expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], email: owner.email) + 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 get("/api/schools/#{school.id}/students", headers:) - expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: owner.email) + 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 it 'responds 200 OK when the user is a school-teacher' do @@ -43,7 +43,7 @@ authenticated_in_hydra_as(teacher) get("/api/schools/#{school.id}/students", headers:) - expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], email: owner.email) + expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], email: owner.email, school_id: school.id) end it 'creates the school teacher safeguarding flag when the user is a school teacher' do @@ -51,7 +51,7 @@ authenticated_in_hydra_as(teacher) get("/api/schools/#{school.id}/students", headers:) - expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: teacher.email) + expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: teacher.email, school_id: school.id) end it 'responds with the school students JSON' do diff --git a/spec/features/school_student/updating_a_school_student_spec.rb b/spec/features/school_student/updating_a_school_student_spec.rb index 16c9c7421..22fac6fc2 100644 --- a/spec/features/school_student/updating_a_school_student_spec.rb +++ b/spec/features/school_student/updating_a_school_student_spec.rb @@ -27,12 +27,12 @@ it 'creates the school owner safeguarding flag' do put("/api/schools/#{school.id}/students/#{student_id}", headers:, params:) - expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], email: owner.email) + 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 put("/api/schools/#{school.id}/students/#{student_id}", headers:, params:) - expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: owner.email) + 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 it 'responds 204 No Content' do @@ -53,7 +53,7 @@ authenticated_in_hydra_as(teacher) put("/api/schools/#{school.id}/students/#{student_id}", headers:, params:) - expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], email: owner.email) + expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], email: owner.email, school_id: school.id) end it 'creates the school teacher safeguarding flag when the user is a school teacher' do @@ -61,7 +61,7 @@ authenticated_in_hydra_as(teacher) put("/api/schools/#{school.id}/students/#{student_id}", headers:, params:) - expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: teacher.email) + expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: teacher.email, school_id: school.id) end it 'responds 401 Unauthorized when no token is given' do diff --git a/spec/lib/profile_api_client_spec.rb b/spec/lib/profile_api_client_spec.rb index 16403a9ed..37340a8c4 100644 --- a/spec/lib/profile_api_client_spec.rb +++ b/spec/lib/profile_api_client_spec.rb @@ -125,7 +125,8 @@ def create_school email: 'user@example.com', createdAt: '2024-07-01T12:49:18.926Z', updatedAt: '2024-07-01T12:49:18.926Z', - discardedAt: nil + discardedAt: nil, + schoolId: SecureRandom.uuid } expected = ProfileApiClient::SafeguardingFlag.new(**flag) stub_request(:get, list_safeguarding_flags_url) @@ -144,6 +145,7 @@ def list_safeguarding_flags subject(:create_safeguarding_flag_response) { create_safeguarding_flag } let(:flag) { 'school:owner' } + let(:school_id) { SecureRandom.uuid } let(:create_safeguarding_flag_url) { "#{api_url}/api/v1/safeguarding-flags" } before do @@ -155,7 +157,7 @@ def list_safeguarding_flags it 'sends the safeguarding flag in the request body' do create_safeguarding_flag_response - expect(WebMock).to have_requested(:post, create_safeguarding_flag_url).with(body: { flag:, email: 'user@example.com' }.to_json) + expect(WebMock).to have_requested(:post, create_safeguarding_flag_url).with(body: { flag:, email: 'user@example.com', schoolId: school_id }.to_json) end it 'returns empty body if created successfully' do @@ -176,7 +178,7 @@ def list_safeguarding_flags private def create_safeguarding_flag - described_class.create_safeguarding_flag(token:, flag:, email: 'user@example.com') + described_class.create_safeguarding_flag(token:, flag:, email: 'user@example.com', school_id:) end end diff --git a/spec/services/school_onboarding_service_spec.rb b/spec/services/school_onboarding_service_spec.rb index e00a34970..d5ca907a0 100644 --- a/spec/services/school_onboarding_service_spec.rb +++ b/spec/services/school_onboarding_service_spec.rb @@ -54,6 +54,32 @@ end end + describe 'when Profile API returns unauthorized' do + before do + allow(ProfileApiClient).to receive(:create_school).and_raise(ProfileApiClient::UnauthorizedError) + allow(Sentry).to receive(:capture_exception) + end + + it 'does not create owner role' do + service.onboard(token:) + expect(school_creator).not_to be_school_owner(school) + end + + it 'does not create teacher role' do + service.onboard(token:) + expect(school_creator).not_to be_school_teacher(school) + end + + it 'does not capture the error in Sentry' do + service.onboard(token:) + expect(Sentry).not_to have_received(:capture_exception) + end + + it 'returns false' do + expect(service.onboard(token:)).to be(false) + end + end + describe 'when teacher and owner roles cannot be created because they already have a role in another school' do let(:another_school) { create(:school) } diff --git a/spec/support/profile_api_mock.rb b/spec/support/profile_api_mock.rb index 61ddb5e1b..bbfa938a3 100644 --- a/spec/support/profile_api_mock.rb +++ b/spec/support/profile_api_mock.rb @@ -30,6 +30,19 @@ def stub_profile_api_create_school_student(user_id: SecureRandom.uuid) allow(ProfileApiClient).to receive(:create_school_student).and_return(created: [user_id]) end + def stub_profile_api_create_school(id: SecureRandom.uuid, code: '99-12-34') + now = Time.current.to_fs(:iso8601) # rubocop:disable Naming/VariableNumber + allow(ProfileApiClient).to receive(:create_school).and_return( + ProfileApiClient::School.new( + id:, + schoolCode: code, + updatedAt: now, + createdAt: now, + discardedAt: nil + ) + ) + end + def stub_profile_api_create_school_students(user_ids: [SecureRandom.uuid]) allow(ProfileApiClient).to receive(:create_school_students).and_return(created: [user_ids.join(', ')]) end diff --git a/spec/support/shared_examples/profile_api_client_examples.rb b/spec/support/shared_examples/profile_api_client_examples.rb index bd471b7e5..03b0d15dc 100644 --- a/spec/support/shared_examples/profile_api_client_examples.rb +++ b/spec/support/shared_examples/profile_api_client_examples.rb @@ -42,10 +42,25 @@ RSpec.shared_examples 'a request that handles standard HTTP errors' do |http_method, url:| let(:expected_url) { instance_exec(&url) } - it 'raises faraday exception for 4xx and 5xx responses' do - stub_request(http_method, expected_url).to_return(status: 401) + it 'raises faraday exception for 4xx responses' do + stub_request(http_method, expected_url).to_return(status: 403) expect { subject }.to raise_error(Faraday::Error) end + + it 'raises faraday exception for 5xx responses' do + stub_request(http_method, expected_url).to_return(status: 500) + expect { subject }.to raise_error(Faraday::Error) + end + + it 'does not raise faraday exception for 401' do + stub_request(http_method, expected_url).to_return(status: 401) + expect { subject }.not_to raise_error(Faraday::Error) + end + + it 'raises UnauthorizedError on 401' do + stub_request(http_method, expected_url).to_return(status: 401) + expect { subject }.to raise_error(ProfileApiClient::UnauthorizedError) + end end RSpec.shared_examples 'a request that handles an unexpected response status' do |http_method, url:, status:|