diff --git a/app/controllers/api/school_students_controller.rb b/app/controllers/api/school_students_controller.rb index 4faec95b9..83abd59d3 100644 --- a/app/controllers/api/school_students_controller.rb +++ b/app/controllers/api/school_students_controller.rb @@ -5,6 +5,7 @@ class SchoolStudentsController < ApiController before_action :authorize_user 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) @@ -62,5 +63,28 @@ def destroy def school_student_params params.require(:school_student).permit(:username, :password, :name) 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] + ) + 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] + ) + end end end diff --git a/app/services/school_verification_service.rb b/app/services/school_verification_service.rb index 711dde4ab..38b6f4ff1 100644 --- a/app/services/school_verification_service.rb +++ b/app/services/school_verification_service.rb @@ -13,7 +13,7 @@ def verify(token:) school.verify! Role.owner.create!(user_id: school.creator_id, school:) Role.teacher.create!(user_id: school.creator_id, school:) - ProfileApiClient.create_school(token:, school:) + ProfileApiClient.create_school(token:, id: school.id, code: school.code) end rescue StandardError => e Sentry.capture_exception(e) diff --git a/lib/concepts/school_student/create.rb b/lib/concepts/school_student/create.rb index f6649be29..6bb8e4129 100644 --- a/lib/concepts/school_student/create.rb +++ b/lib/concepts/school_student/create.rb @@ -5,7 +5,11 @@ class Create class << self def call(school:, school_student_params:, token:) response = OperationResponse.new - create_student(school, school_student_params, token) + response[:student_id] = create_student(school, school_student_params, token) + response + rescue ProfileApiClient::CreateStudent422Error => e + Sentry.capture_exception(e) + response[:error] = "Error creating school student: #{e.error}" response rescue StandardError => e Sentry.capture_exception(e) @@ -16,14 +20,17 @@ def call(school:, school_student_params:, token:) private def create_student(school, school_student_params, token) - organisation_id = school.id + school_id = school.id username = school_student_params.fetch(:username) password = school_student_params.fetch(:password) name = school_student_params.fetch(:name) validate(school:, username:, password:, name:) - ProfileApiClient.create_school_student(token:, username:, password:, name:, organisation_id:) + response = ProfileApiClient.create_school_student(token:, username:, password:, name:, school_id:) + user_id = response[:created].first + Role.student.create!(school:, user_id:) + user_id end def validate(school:, username:, password:, name:) diff --git a/lib/concepts/school_student/create_batch.rb b/lib/concepts/school_student/create_batch.rb index 5329466a6..388b2b193 100644 --- a/lib/concepts/school_student/create_batch.rb +++ b/lib/concepts/school_student/create_batch.rb @@ -23,7 +23,7 @@ def create_batch(school, uploaded_file, token) validate(school:, sheet:) non_header_rows_with_content(sheet:).each do |name, username, password| - ProfileApiClient.create_school_student(token:, username:, password:, name:, organisation_id: school.id) + ProfileApiClient.create_school_student(token:, username:, password:, name:, school_id: school.id) end end diff --git a/lib/profile_api_client.rb b/lib/profile_api_client.rb index c898b0a4c..ad5a7e8d3 100644 --- a/lib/profile_api_client.rb +++ b/lib/profile_api_client.rb @@ -1,23 +1,48 @@ # frozen_string_literal: true class ProfileApiClient + SAFEGUARDING_FLAGS = { + teacher: 'school:teacher', + owner: 'school:owner' + }.freeze + + class Error < StandardError; end + + class CreateStudent422Error < Error + DEFAULT_ERROR = 'unknown error' + ERRORS = { + 'ERR_USER_EXISTS' => 'username has already been taken', + 'ERR_INVALID' => 'unknown validation error', + 'ERR_INVALID_PASSWORD' => 'password is invalid', + 'ERR_UNKNOWN' => DEFAULT_ERROR + }.freeze + + attr_reader :username, :error + + def initialize(error) + @username = error['username'] + @error = ERRORS.fetch(error['error'], DEFAULT_ERROR) + + super "Student not created in Profile API (status code 422, username '#{@username}', error '#{@error}')" + end + end + class << self # TODO: Replace with HTTP requests once the profile API has been built. - def create_school(token:, school:) - return { 'id' => school.id, 'schoolCode' => school.code } if ENV['BYPASS_OAUTH'].present? + def create_school(token:, id:, code:) + return { 'id' => id, 'schoolCode' => code } if ENV['BYPASS_OAUTH'].present? - response = connection.post('/api/v1/schools') do |request| - apply_default_headers(request, token) + response = connection(token).post('/api/v1/schools') do |request| request.body = { - id: school.id, - schoolCode: school.code - }.to_json + id:, + schoolCode: code + } end - raise "School not created in Profile API. HTTP response code: #{response.status}" unless response.status == 201 + raise "School not created in Profile API (status code #{response.status})" unless response.status == 201 - JSON.parse(response.body) + response.body end # The API should enforce these constraints: @@ -142,19 +167,24 @@ def list_school_students(token:, organisation_id:) # The API should respond: # - 404 Not Found if the user doesn't exist # - 422 Unprocessable if the constraints are not met - def create_school_student(token:, username:, password:, name:, organisation_id:) + # rubocop:disable Metrics/AbcSize + def create_school_student(token:, username:, password:, name:, school_id:) return nil if token.blank? - _ = username - _ = password - _ = name - _ = organisation_id + response = connection(token).post("/api/v1/schools/#{school_id}/students") do |request| + request.body = [{ + name: name.strip, + username: username.strip, + password: password.strip + }] + end - # TODO: We should make Faraday raise a Ruby error for a non-2xx status - # code so that SchoolStudent::Create propagates the error in the response. - response = {} - response.deep_symbolize_keys + raise CreateStudent422Error, response.body['errors'].first if response.status == 422 + raise "Student not created in Profile API (status code #{response.status})" unless response.status == 201 + + response.body.deep_symbolize_keys end + # rubocop:enable Metrics/AbcSize # The API should enforce these constraints: # - The token has the school-owner or school-teacher role for the given organisation ID @@ -198,18 +228,46 @@ def delete_school_student(token:, student_id:, organisation_id:) response.deep_symbolize_keys end - private + def safeguarding_flags(token:) + response = connection(token).get('/api/v1/safeguarding-flags') - def connection - Faraday.new(ENV.fetch('IDENTITY_URL')) + unless response.status == 200 + raise "Safeguarding flags cannot be retrieved from Profile API (status code #{response.status})" + end + + response.body.map(&:deep_symbolize_keys) + end + + def create_safeguarding_flag(token:, flag:) + response = connection(token).post('/api/v1/safeguarding-flags') do |request| + request.body = { flag: } + end + + return if response.status == 201 || response.status == 303 + + raise "Safeguarding flag not created in Profile API (status code #{response.status})" end - def apply_default_headers(request, token) - request.headers['Accept'] = 'application/json' - request.headers['Authorization'] = "Bearer #{token}" - request.headers['Content-Type'] = 'application/json' - request.headers['X-API-KEY'] = ENV.fetch('PROFILE_API_KEY') - request + def delete_safeguarding_flag(token:, flag:) + response = connection(token).delete("/api/v1/safeguarding-flags/#{flag}") + + return if response.status == 204 + + raise "Safeguarding flag not deleted from Profile API (status code #{response.status})" + end + + private + + def connection(token) + Faraday.new(ENV.fetch('IDENTITY_URL')) do |faraday| + faraday.request :json + faraday.response :json + faraday.headers = { + 'Accept' => 'application/json', + 'Authorization' => "Bearer #{token}", + 'X-API-KEY' => ENV.fetch('PROFILE_API_KEY') + } + end end end end diff --git a/spec/concepts/school_student/create_batch_spec.rb b/spec/concepts/school_student/create_batch_spec.rb index 0cba73b3b..ba82fd3c1 100644 --- a/spec/concepts/school_student/create_batch_spec.rb +++ b/spec/concepts/school_student/create_batch_spec.rb @@ -21,7 +21,7 @@ # TODO: Replace with WebMock assertion once the profile API has been built. expect(ProfileApiClient).to have_received(:create_school_student) - .with(token:, username: 'jane123', password: 'secret123', name: 'Jane Doe', organisation_id: school.id) + .with(token:, username: 'jane123', password: 'secret123', name: 'Jane Doe', school_id: school.id) end it "makes a profile API call to create John Doe's account" do @@ -29,7 +29,7 @@ # TODO: Replace with WebMock assertion once the profile API has been built. expect(ProfileApiClient).to have_received(:create_school_student) - .with(token:, username: 'john123', password: 'secret456', name: 'John Doe', organisation_id: school.id) + .with(token:, username: 'john123', password: 'secret456', name: 'John Doe', school_id: school.id) end context 'when an .xlsx file is provided' do diff --git a/spec/concepts/school_student/create_spec.rb b/spec/concepts/school_student/create_spec.rb index 240c5d811..395c07eb0 100644 --- a/spec/concepts/school_student/create_spec.rb +++ b/spec/concepts/school_student/create_spec.rb @@ -5,6 +5,7 @@ RSpec.describe SchoolStudent::Create, type: :unit do let(:token) { UserProfileMock::TOKEN } let(:school) { create(:verified_school) } + let(:user_id) { SecureRandom.uuid } let(:school_student_params) do { @@ -15,7 +16,7 @@ end before do - stub_profile_api_create_school_student + stub_profile_api_create_school_student(user_id:) end it 'returns a successful operation response' do @@ -28,13 +29,23 @@ # TODO: Replace with WebMock assertion once the profile API has been built. expect(ProfileApiClient).to have_received(:create_school_student) - .with(token:, username: 'student-to-create', password: 'at-least-8-characters', name: 'School Student', organisation_id: school.id) + .with(token:, username: 'student-to-create', password: 'at-least-8-characters', name: 'School Student', school_id: school.id) + end + + it 'creates a role associating the student with the school' do + described_class.call(school:, school_student_params:, token:) + expect(Role.student.where(school:, user_id:)).to exist + end + + it 'returns the ID of the student created in Profile API' do + response = described_class.call(school:, school_student_params:, token:) + expect(response[:student_id]).to eq(user_id) end context 'when creation fails' do let(:school_student_params) do { - username: ' ', + username: '', password: 'at-least-8-characters', name: 'School Student' } @@ -56,7 +67,7 @@ it 'returns the error message in the operation response' do response = described_class.call(school:, school_student_params:, token:) - expect(response[:error]).to match(/username ' ' is invalid/) + expect(response[:error]).to match(/username '' is invalid/) end it 'sent the exception to Sentry' do @@ -73,4 +84,30 @@ expect(response[:error]).to match(/school is not verified/) end end + + context 'when the student cannot be created in profile api because of a 422 response' do + let(:error) { { 'username' => 'username', 'error' => 'ERR_USER_EXISTS' } } + let(:exception) { ProfileApiClient::CreateStudent422Error.new(error) } + + before do + allow(ProfileApiClient).to receive(:create_school_student).and_raise(exception) + end + + it 'adds a useful error message' do + response = described_class.call(school:, school_student_params:, token:) + expect(response[:error]).to eq('Error creating school student: username has already been taken') + end + end + + context 'when the student cannot be created in profile api because of a response other than 422' do + before do + allow(ProfileApiClient).to receive(:create_school_student) + .and_raise('Student not created in Profile API (status code 401)') + end + + it 'adds a useful error message' do + response = described_class.call(school:, school_student_params:, token:) + expect(response[:error]).to eq('Error creating school student: Student not created in Profile API (status code 401)') + end + 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 ffd8bb3f9..50dd724e3 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 @@ -6,6 +6,7 @@ before do authenticated_in_hydra_as(owner) stub_profile_api_create_school_student + stub_profile_api_create_safeguarding_flag end let(:headers) { { Authorization: UserProfileMock::TOKEN } } @@ -15,6 +16,16 @@ let(:file) { fixture_file_upload('students.csv') } + it 'creates the school owner safeguarding flag' do + post("/api/schools/#{school.id}/students/batch", headers:, params: { file: }) + expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner]) + end + + it 'does not create the school teacher safeguarding flag' do + post("/api/schools/#{school.id}/students/batch", headers:, params: { file: }) + expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher]) + end + it 'responds 204 No Content' do post("/api/schools/#{school.id}/students/batch", headers:, params: { file: }) expect(response).to have_http_status(:no_content) @@ -28,6 +39,22 @@ expect(response).to have_http_status(:no_content) end + it 'does not create the school owner safeguarding flag when the user is a school-teacher' do + teacher = create(:teacher, school:) + authenticated_in_hydra_as(teacher) + + post("/api/schools/#{school.id}/students/batch", headers:, params: { file: }) + expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner]) + end + + it 'creates the school teacher safeguarding flag when the user is a school-teacher' do + teacher = create(:teacher, school:) + authenticated_in_hydra_as(teacher) + + post("/api/schools/#{school.id}/students/batch", headers:, params: { file: }) + expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher]) + end + it 'responds 422 Unprocessable Entity when params are invalid' do post("/api/schools/#{school.id}/students/batch", headers:, params: {}) expect(response).to have_http_status(:unprocessable_entity) 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 852a9d022..29d18bf64 100644 --- a/spec/features/school_student/creating_a_school_student_spec.rb +++ b/spec/features/school_student/creating_a_school_student_spec.rb @@ -6,6 +6,7 @@ before do authenticated_in_hydra_as(owner) stub_profile_api_create_school_student + stub_profile_api_create_safeguarding_flag end let(:headers) { { Authorization: UserProfileMock::TOKEN } } @@ -22,6 +23,16 @@ } end + 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]) + 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]) + end + it 'responds 204 No Content' do post("/api/schools/#{school.id}/students", headers:, params:) expect(response).to have_http_status(:no_content) @@ -35,6 +46,22 @@ expect(response).to have_http_status(:no_content) end + it 'does not create the school owner safeguarding flag when the user is a school teacher' do + teacher = create(:teacher, school:) + 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]) + end + + it 'creates the school teacher safeguarding flag when the user is a school teacher' do + teacher = create(:teacher, school:) + 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]) + end + it 'responds 400 Bad Request when params are missing' do post("/api/schools/#{school.id}/students", headers:) expect(response).to have_http_status(:bad_request) 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 10fc11d77..2053daeba 100644 --- a/spec/features/school_student/deleting_a_school_student_spec.rb +++ b/spec/features/school_student/deleting_a_school_student_spec.rb @@ -6,6 +6,7 @@ before do authenticated_in_hydra_as(owner) stub_profile_api_delete_school_student + stub_profile_api_create_safeguarding_flag end let(:headers) { { Authorization: UserProfileMock::TOKEN } } @@ -13,6 +14,16 @@ let(:student_id) { SecureRandom.uuid } let(:owner) { create(:owner, school:) } + 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]) + 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]) + end + it 'responds 204 No Content' do delete("/api/schools/#{school.id}/students/#{student_id}", headers:) expect(response).to have_http_status(:no_content) @@ -39,6 +50,22 @@ expect(response).to have_http_status(:forbidden) end + it 'does not create the school owner safeguarding flag when logged in as a teacher' do + teacher = create(:teacher, school:) + 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]) + end + + it 'does not create the school teacher safeguarding flag when logged in as a teacher' do + teacher = create(:teacher, school:) + 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]) + end + it 'responds 403 Forbidden when the user is a school-student' do student = create(:student, school:) authenticated_in_hydra_as(student) diff --git a/spec/features/school_student/listing_school_students_spec.rb b/spec/features/school_student/listing_school_students_spec.rb index 413c34830..af6262126 100644 --- a/spec/features/school_student/listing_school_students_spec.rb +++ b/spec/features/school_student/listing_school_students_spec.rb @@ -7,6 +7,7 @@ authenticated_in_hydra_as(owner) stub_profile_api_list_school_students(user_id: student.id) stub_user_info_api_for(student) + stub_profile_api_create_safeguarding_flag end let(:headers) { { Authorization: UserProfileMock::TOKEN } } @@ -19,6 +20,16 @@ expect(response).to have_http_status(:ok) end + 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]) + 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]) + end + it 'responds 200 OK when the user is a school-teacher' do teacher = create(:teacher, school:) authenticated_in_hydra_as(teacher) @@ -27,6 +38,22 @@ expect(response).to have_http_status(:ok) end + it 'does not create the school owner safeguarding flag when the user is a school teacher' do + teacher = create(:teacher, school:) + 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]) + end + + it 'creates the school teacher safeguarding flag when the user is a school teacher' do + teacher = create(:teacher, school:) + 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]) + end + it 'responds with the school students JSON' do get("/api/schools/#{school.id}/students", headers:) data = JSON.parse(response.body, symbolize_names: true) 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 1ecccd8a7..37e5cf9c6 100644 --- a/spec/features/school_student/updating_a_school_student_spec.rb +++ b/spec/features/school_student/updating_a_school_student_spec.rb @@ -6,6 +6,7 @@ before do authenticated_in_hydra_as(owner) stub_profile_api_update_school_student + stub_profile_api_create_safeguarding_flag end let(:headers) { { Authorization: UserProfileMock::TOKEN } } @@ -23,6 +24,16 @@ } end + 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]) + 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]) + end + it 'responds 204 No Content' do put("/api/schools/#{school.id}/students/#{student_id}", headers:, params:) expect(response).to have_http_status(:no_content) @@ -36,6 +47,22 @@ expect(response).to have_http_status(:no_content) end + it 'does not create the school owner safeguarding flag when the user is a school teacher' do + teacher = create(:teacher, school:) + 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]) + end + + it 'creates the school teacher safeguarding flag when the user is a school teacher' do + teacher = create(:teacher, school:) + 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]) + end + it 'responds 401 Unauthorized when no token is given' do put("/api/schools/#{school.id}/students/#{student_id}", params:) expect(response).to have_http_status(:unauthorized) diff --git a/spec/lib/profile_api_client_spec.rb b/spec/lib/profile_api_client_spec.rb index 3926de7c6..920b6c91f 100644 --- a/spec/lib/profile_api_client_spec.rb +++ b/spec/lib/profile_api_client_spec.rb @@ -3,68 +3,92 @@ require 'rails_helper' RSpec.describe ProfileApiClient do + let(:api_url) { 'http://example.com' } + let(:api_key) { 'api-key' } + let(:token) { SecureRandom.uuid } + + before do + allow(ENV).to receive(:fetch).with('IDENTITY_URL').and_return(api_url) + allow(ENV).to receive(:fetch).with('PROFILE_API_KEY').and_return(api_key) + end + + describe ProfileApiClient::CreateStudent422Error do + subject(:exception) { described_class.new(error) } + + let(:error_code) { 'ERR_USER_EXISTS' } + let(:error) { { 'username' => 'username', 'error' => error_code } } + + it 'includes status code, username and translated error code in the message' do + expect(exception.message).to eq("Student not created in Profile API (status code 422, username 'username', error 'username has already been taken')") + end + + context "when the error isn't recognised" do + let(:error_code) { 'unrecognised-code' } + + it 'includes a default error message' do + expect(exception.message).to match(/error 'unknown error'/) + end + end + end + describe '.create_school' do - let(:api_url) { 'http://example.com' } - let(:api_key) { 'api-key' } - let(:token) { SecureRandom.uuid } let(:school) { build(:school, id: SecureRandom.uuid, code: SecureRandom.uuid) } + let(:create_school_url) { "#{api_url}/api/v1/schools" } before do - stub_request(:post, "#{api_url}/api/v1/schools").to_return(status: 201, body: '{}') - allow(ENV).to receive(:fetch).with('IDENTITY_URL').and_return(api_url) - allow(ENV).to receive(:fetch).with('PROFILE_API_KEY').and_return(api_key) + stub_request(:post, create_school_url).to_return(status: 201, body: '{}') end it 'makes a request to the profile api host' do - described_class.create_school(token:, school:) - expect(WebMock).to have_requested(:post, "#{api_url}/api/v1/schools") + create_school + expect(WebMock).to have_requested(:post, create_school_url) end it 'includes token in the authorization request header' do - described_class.create_school(token:, school:) - expect(WebMock).to have_requested(:post, "#{api_url}/api/v1/schools").with(headers: { authorization: "Bearer #{token}" }) + create_school + expect(WebMock).to have_requested(:post, create_school_url).with(headers: { authorization: "Bearer #{token}" }) end it 'includes the profile api key in the x-api-key request header' do - described_class.create_school(token:, school:) - expect(WebMock).to have_requested(:post, "#{api_url}/api/v1/schools").with(headers: { 'x-api-key' => api_key }) + create_school + expect(WebMock).to have_requested(:post, create_school_url).with(headers: { 'x-api-key' => api_key }) end it 'sets content-type of request to json' do - described_class.create_school(token:, school:) - expect(WebMock).to have_requested(:post, "#{api_url}/api/v1/schools").with(headers: { 'content-type' => 'application/json' }) + create_school + expect(WebMock).to have_requested(:post, create_school_url).with(headers: { 'content-type' => 'application/json' }) end it 'sets accept header to json' do - described_class.create_school(token:, school:) - expect(WebMock).to have_requested(:post, "#{api_url}/api/v1/schools").with(headers: { 'accept' => 'application/json' }) + create_school + expect(WebMock).to have_requested(:post, create_school_url).with(headers: { 'accept' => 'application/json' }) end it 'sends the school id and code in the request body as json' do - described_class.create_school(token:, school:) + create_school expected_body = { id: school.id, schoolCode: school.code }.to_json - expect(WebMock).to have_requested(:post, "#{api_url}/api/v1/schools").with(body: expected_body) + expect(WebMock).to have_requested(:post, create_school_url).with(body: expected_body) end it 'returns the created school if successful' do data = { 'id' => 'school-id', 'schoolCode' => 'school-code' } - stub_request(:post, "#{api_url}/api/v1/schools") - .to_return(status: 201, body: data.to_json) - expect(described_class.create_school(token:, school:)).to eq(data) + stub_request(:post, create_school_url) + .to_return(status: 201, body: data.to_json, headers: { 'Content-Type' => 'application/json' }) + expect(create_school).to eq(data) end it 'raises exception if anything other than a 201 status code is returned' do - stub_request(:post, "#{api_url}/api/v1/schools") + stub_request(:post, create_school_url) .to_return(status: 200) - expect { described_class.create_school(token:, school:) }.to raise_error(RuntimeError) + expect { create_school }.to raise_error(RuntimeError) end it 'includes details of underlying response when exception is raised' do - stub_request(:post, "#{api_url}/api/v1/schools") + stub_request(:post, create_school_url) .to_return(status: 401) - expect { described_class.create_school(token:, school:) }.to raise_error('School not created in Profile API. HTTP response code: 401') + expect { create_school }.to raise_error('School not created in Profile API (status code 401)') end describe 'when BYPASS_OAUTH is true' do @@ -73,14 +97,288 @@ end it 'does not make a request to Profile API' do - described_class.create_school(token:, school:) - expect(WebMock).not_to have_requested(:post, "#{api_url}/api/v1/schools") + create_school + expect(WebMock).not_to have_requested(:post, create_school_url) end it 'returns the id and code of the school supplied' do expected = { 'id' => school.id, 'schoolCode' => school.code } - expect(described_class.create_school(token:, school:)).to eq(expected) + expect(create_school).to eq(expected) end end + + private + + def create_school + described_class.create_school(token:, id: school.id, code: school.code) + end + end + + describe '.safeguarding_flags' do + let(:list_safeguarding_flags_url) { "#{api_url}/api/v1/safeguarding-flags" } + + before do + stub_request(:get, list_safeguarding_flags_url).to_return(status: 200, body: '[]', headers: { 'content-type' => 'application/json' }) + end + + it 'makes a request to the profile api host' do + list_safeguarding_flags + expect(WebMock).to have_requested(:get, list_safeguarding_flags_url) + end + + it 'includes token in the authorization request header' do + list_safeguarding_flags + expect(WebMock).to have_requested(:get, list_safeguarding_flags_url).with(headers: { authorization: "Bearer #{token}" }) + end + + it 'includes the profile api key in the x-api-key request header' do + list_safeguarding_flags + expect(WebMock).to have_requested(:get, list_safeguarding_flags_url).with(headers: { 'x-api-key' => api_key }) + end + + it 'sets accept header to json' do + list_safeguarding_flags + expect(WebMock).to have_requested(:get, list_safeguarding_flags_url).with(headers: { 'accept' => 'application/json' }) + end + + # rubocop:disable RSpec/ExampleLength + it 'returns list of safeguarding flags if successful' do + flags = [ + { + id: '7ac79585-e187-4d2f-bf0c-a1cbe72ecc9a', + userId: '583ba872-b16e-46e1-9f7d-df89d267550d', + flag: 'school:owner', + isActive: 'true', + createdAt: '2024-07-01T12:49:18.926Z', + updatedAt: '2024-07-01T12:49:18.926Z' + } + ] + stub_request(:get, list_safeguarding_flags_url) + .to_return(status: 200, body: flags.to_json, headers: { 'content-type' => 'application/json' }) + expect(list_safeguarding_flags).to eq(flags) + end + # rubocop:enable RSpec/ExampleLength + + it 'raises exception if anything other than a 200 status code is returned' do + stub_request(:get, list_safeguarding_flags_url) + .to_return(status: 201) + + expect { list_safeguarding_flags }.to raise_error(RuntimeError) + end + + it 'includes details of underlying response when exception is raised' do + stub_request(:get, list_safeguarding_flags_url) + .to_return(status: 401) + + expect { list_safeguarding_flags }.to raise_error('Safeguarding flags cannot be retrieved from Profile API (status code 401)') + end + + private + + def list_safeguarding_flags + described_class.safeguarding_flags(token:) + end + end + + describe '.create_safeguarding_flag' do + let(:flag) { 'school:owner' } + let(:create_safeguarding_flag_url) { "#{api_url}/api/v1/safeguarding-flags" } + + before do + stub_request(:post, create_safeguarding_flag_url).to_return(status: 201, body: '{}', headers: { 'content-type' => 'application/json' }) + end + + it 'makes a request to the profile api host' do + create_safeguarding_flag + expect(WebMock).to have_requested(:post, create_safeguarding_flag_url) + end + + it 'includes token in the authorization request header' do + create_safeguarding_flag + expect(WebMock).to have_requested(:post, create_safeguarding_flag_url).with(headers: { authorization: "Bearer #{token}" }) + end + + it 'includes the profile api key in the x-api-key request header' do + create_safeguarding_flag + expect(WebMock).to have_requested(:post, create_safeguarding_flag_url).with(headers: { 'x-api-key' => api_key }) + end + + it 'sets content-type of request to json' do + create_safeguarding_flag + expect(WebMock).to have_requested(:post, create_safeguarding_flag_url).with(headers: { 'content-type' => 'application/json' }) + end + + it 'sets accept header to json' do + create_safeguarding_flag + expect(WebMock).to have_requested(:post, create_safeguarding_flag_url).with(headers: { 'accept' => 'application/json' }) + end + + it 'sends the safeguarding flag in the request body' do + create_safeguarding_flag + expect(WebMock).to have_requested(:post, create_safeguarding_flag_url).with(body: { flag: }.to_json) + end + + it 'returns empty body if created successfully' do + stub_request(:post, create_safeguarding_flag_url) + .to_return(status: 201) + expect(create_safeguarding_flag).to be_nil + end + + it 'returns empty body if 303 response returned to indicate that the flag already exists' do + stub_request(:post, create_safeguarding_flag_url) + .to_return(status: 303) + expect(create_safeguarding_flag).to be_nil + end + + it 'raises exception if anything other than a 201 or 303 status code is returned' do + stub_request(:post, create_safeguarding_flag_url) + .to_return(status: 200) + + expect { create_safeguarding_flag }.to raise_error(RuntimeError) + end + + it 'includes details of underlying response when exception is raised' do + stub_request(:post, create_safeguarding_flag_url) + .to_return(status: 401) + + expect { create_safeguarding_flag }.to raise_error('Safeguarding flag not created in Profile API (status code 401)') + end + + def create_safeguarding_flag + described_class.create_safeguarding_flag(token:, flag:) + end + end + + describe '.delete_safeguarding_flag' do + let(:flag) { 'school:owner' } + let(:delete_safeguarding_flag_url) { "#{api_url}/api/v1/safeguarding-flags/#{flag}" } + + before do + stub_request(:delete, delete_safeguarding_flag_url).to_return(status: 204, body: '') + end + + it 'makes a request to the profile api host' do + delete_safeguarding_flag + expect(WebMock).to have_requested(:delete, delete_safeguarding_flag_url) + end + + it 'includes token in the authorization request header' do + delete_safeguarding_flag + expect(WebMock).to have_requested(:delete, delete_safeguarding_flag_url).with(headers: { authorization: "Bearer #{token}" }) + end + + it 'includes the profile api key in the x-api-key request header' do + delete_safeguarding_flag + expect(WebMock).to have_requested(:delete, delete_safeguarding_flag_url).with(headers: { 'x-api-key' => api_key }) + end + + it 'sets accept header to json' do + delete_safeguarding_flag + expect(WebMock).to have_requested(:delete, delete_safeguarding_flag_url).with(headers: { 'accept' => 'application/json' }) + end + + it 'returns empty body if successful' do + stub_request(:delete, delete_safeguarding_flag_url) + .to_return(status: 204, body: '') + expect(delete_safeguarding_flag).to be_nil + end + + it 'raises exception if anything other than a 204 status code is returned' do + stub_request(:delete, delete_safeguarding_flag_url) + .to_return(status: 200) + + expect { delete_safeguarding_flag }.to raise_error(RuntimeError) + end + + it 'includes details of underlying response when exception is raised' do + stub_request(:delete, delete_safeguarding_flag_url) + .to_return(status: 401) + + expect { delete_safeguarding_flag }.to raise_error('Safeguarding flag not deleted from Profile API (status code 401)') + end + + def delete_safeguarding_flag + described_class.delete_safeguarding_flag(token:, flag:) + end + end + + describe '.create_school_student' do + let(:username) { 'username' } + let(:password) { 'password' } + let(:name) { 'name' } + let(:school) { build(:school, id: SecureRandom.uuid) } + let(:create_students_url) { "#{api_url}/api/v1/schools/#{school.id}/students" } + + before do + stub_request(:post, create_students_url).to_return(status: 201, body: '{}', headers: { 'content-type' => 'application/json' }) + end + + it 'makes a request to the profile api host' do + create_school_student + expect(WebMock).to have_requested(:post, create_students_url) + end + + it 'includes token in the authorization request header' do + create_school_student + expect(WebMock).to have_requested(:post, create_students_url).with(headers: { authorization: "Bearer #{token}" }) + end + + it 'includes the profile api key in the x-api-key request header' do + create_school_student + expect(WebMock).to have_requested(:post, create_students_url).with(headers: { 'x-api-key' => api_key }) + end + + it 'sets content-type of request to json' do + create_school_student + expect(WebMock).to have_requested(:post, create_students_url).with(headers: { 'content-type' => 'application/json' }) + end + + it 'sets accept header to json' do + create_school_student + expect(WebMock).to have_requested(:post, create_students_url).with(headers: { 'accept' => 'application/json' }) + end + + it 'sends the student details in the request body' do + create_school_student + expect(WebMock).to have_requested(:post, create_students_url).with(body: [{ name:, username:, password: }].to_json) + end + + it 'returns the id of the created student(s) if successful' do + response = { created: ['student-id'] } + stub_request(:post, create_students_url) + .to_return(status: 201, body: response.to_json, headers: { 'content-type' => 'application/json' }) + expect(create_school_student).to eq(response) + end + + it 'raises 422 exception if 422 status code is returned' do + response = { errors: [username: 'username', error: 'ERR_USER_EXISTS'] } + stub_request(:post, create_students_url) + .to_return(status: 422, body: response.to_json, headers: { 'content-type' => 'application/json' }) + + expect { create_school_student }.to raise_error(ProfileApiClient::CreateStudent422Error) + .with_message("Student not created in Profile API (status code 422, username 'username', error 'username has already been taken')") + end + + it 'raises exception if anything other that 201 status code is returned' do + stub_request(:post, create_students_url) + .to_return(status: 200) + + expect { create_school_student }.to raise_error('Student not created in Profile API (status code 200)') + end + + context 'when there are extraneous leading and trailing spaces in the student params' do + let(:username) { ' username ' } + let(:password) { ' password ' } + let(:name) { ' name ' } + + it 'strips the extraneous spaces' do + create_school_student + expect(WebMock).to have_requested(:post, create_students_url).with(body: [{ name: 'name', username: 'username', password: 'password' }].to_json) + end + end + + def create_school_student + described_class.create_school_student(token:, username:, password:, name:, school_id: school.id) + end end end diff --git a/spec/services/school_verification_service_spec.rb b/spec/services/school_verification_service_spec.rb index 628b3ccc2..28c4dd3ae 100644 --- a/spec/services/school_verification_service_spec.rb +++ b/spec/services/school_verification_service_spec.rb @@ -44,7 +44,7 @@ it 'creates the school in Profile API' do service.verify(token:) - expect(ProfileApiClient).to have_received(:create_school).with(token:, school:) + expect(ProfileApiClient).to have_received(:create_school).with(token:, id: school.id, code: school.code) end it 'returns true' do diff --git a/spec/support/profile_api_mock.rb b/spec/support/profile_api_mock.rb index 25d680075..3e75d2576 100644 --- a/spec/support/profile_api_mock.rb +++ b/spec/support/profile_api_mock.rb @@ -27,8 +27,8 @@ def stub_profile_api_list_school_students(user_id:) allow(ProfileApiClient).to receive(:list_school_students).and_return(ids: [user_id]) end - def stub_profile_api_create_school_student - allow(ProfileApiClient).to receive(:create_school_student) + 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_update_school_student @@ -38,4 +38,8 @@ def stub_profile_api_update_school_student def stub_profile_api_delete_school_student allow(ProfileApiClient).to receive(:delete_school_student) end + + def stub_profile_api_create_safeguarding_flag + allow(ProfileApiClient).to receive(:create_safeguarding_flag) + end end