From 293de4ab1dc23c2d32d7abf7145cd9d3ba91d3b8 Mon Sep 17 00:00:00 2001 From: Pete Simonovic <69108995+PetarSimonovic@users.noreply.github.com> Date: Fri, 1 May 2026 11:07:19 +0100 Subject: [PATCH 1/2] Add update_school_email_domains to ProfileApiClient Allow the client to PATCH a collection of domains --- lib/profile_api_client.rb | 24 ++++++++-- spec/lib/profile_api_client_spec.rb | 71 ++++++++++++++++++++++++++--- spec/support/profile_api_mock.rb | 3 +- 3 files changed, 87 insertions(+), 11 deletions(-) diff --git a/lib/profile_api_client.rb b/lib/profile_api_client.rb index 1a64efb23..0e5551d54 100644 --- a/lib/profile_api_client.rb +++ b/lib/profile_api_client.rb @@ -7,7 +7,7 @@ class ProfileApiClient }.freeze # rubocop:disable Naming/MethodName - School = Data.define(:id, :schoolCode, :updatedAt, :createdAt, :discardedAt) + School = Data.define(:id, :schoolCode, :studentEmailDomains, :updatedAt, :createdAt, :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 @@ -51,13 +51,14 @@ def check_auth(token:) false end - def create_school(token:, id:, code:) - return { 'id' => id, 'schoolCode' => code } if ENV['BYPASS_OAUTH'].present? + def create_school(token:, id:, code:, school_email_domains: []) + return { 'id' => id, 'schoolCode' => code, 'studentEmailDomains' => school_email_domains } if ENV['BYPASS_OAUTH'].present? response = connection(token).post('/api/v1/schools') do |request| request.body = { id:, - schoolCode: code + schoolCode: code, + studentEmailDomains: school_email_domains } end @@ -214,6 +215,21 @@ def delete_safeguarding_flag(token:, flag:) raise UnexpectedResponse, response unless response.status == 204 end + def update_school_email_domains(token:, school_id:, school_email_domains: []) + return { 'id' => school_id, 'studentEmailDomains' => school_email_domains } if ENV['BYPASS_OAUTH'].present? + + response = connection(token).patch("/api/v1/schools/#{school_id}") do |request| + request.body = { + studentEmailDomains: school_email_domains + } + end + + unauthorized!(response) + raise UnexpectedResponse, response unless response.status == 201 + + School.new(**response.body) + end + private def connection(token) diff --git a/spec/lib/profile_api_client_spec.rb b/spec/lib/profile_api_client_spec.rb index 3a2c40def..b4b8f62cb 100644 --- a/spec/lib/profile_api_client_spec.rb +++ b/spec/lib/profile_api_client_spec.rb @@ -58,7 +58,7 @@ stub_request(:post, create_school_url) .to_return( status: 201, - body: '{"id":"","schoolCode":"","updatedAt":"","createdAt":"","discardedAt":""}', + body: '{"id":"","schoolCode":"","updatedAt":"","createdAt":"","discardedAt":"","studentEmailDomains":[]}', headers: { 'content-type' => 'application/json' } ) end @@ -67,14 +67,15 @@ it_behaves_like 'a request that handles standard HTTP errors', :post, url: -> { create_school_url } it_behaves_like 'a request that handles an unexpected response status', :post, url: -> { "#{api_url}/api/v1/schools" }, status: 200 - it 'sends the school id and code in the request body as json' do + it 'sends the school id, code and studentEmailDomains in the request body as json' do create_school_response - expected_body = { id: school.id, schoolCode: school.code }.to_json + expected_body = { id: school.id, schoolCode: school.code, studentEmailDomains: [] }.to_json expect(WebMock).to have_requested(:post, create_school_url).with(body: expected_body) end it 'returns the created school if successful' do - data = { id: 'id', schoolCode: 'code', updatedAt: '2024-07-09T10:31:13.196Z', createdAt: '2024-07-09T10:31:13.196Z', discardedAt: nil } + data = { id: 'id', schoolCode: 'code', updatedAt: '2024-07-09T10:31:13.196Z', createdAt: '2024-07-09T10:31:13.196Z', discardedAt: nil, + studentEmailDomains: [] } expected = ProfileApiClient::School.new(**data) stub_request(:post, create_school_url) .to_return(status: 201, body: data.to_json, headers: { 'Content-Type' => 'application/json' }) @@ -91,8 +92,8 @@ 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 } + it 'returns the id, code and email domains of the school supplied' do + expected = { 'id' => school.id, 'schoolCode' => school.code, 'studentEmailDomains' => [] } expect(create_school_response).to eq(expected) end end @@ -558,4 +559,62 @@ def school_student described_class.school_student(token:, school_id: school.id, student_id:) end end + + describe '.update_school_email_domains' do + subject(:update_school_email_domains_response) { update_school_email_domains } + + let(:school) { build(:school, id: SecureRandom.uuid, code: SecureRandom.uuid) } + let(:update_school_email_domains_url) { "#{api_url}/api/v1/schools/#{school.id}" } + let(:school_email_domains) { ['student.example.edu', 'mail.example.org'] } + + before do + stub_request(:patch, update_school_email_domains_url) + .to_return( + status: 201, + body: '{"id":"","schoolCode":"","updatedAt":"","createdAt":"","discardedAt":"","studentEmailDomains":[]}', + headers: { 'content-type' => 'application/json' } + ) + end + + it_behaves_like 'an authenticated JSON API request', :patch, url: -> { update_school_email_domains_url } + it_behaves_like 'a request that handles standard HTTP errors', :patch, url: -> { update_school_email_domains_url } + it_behaves_like 'a request that handles an unexpected response status', :patch, url: -> { "#{api_url}/api/v1/schools/#{school.id}" }, status: 200 + + it 'sends studentEmailDomains in the JSON body' do + update_school_email_domains_response + expected_body = { studentEmailDomains: school_email_domains }.to_json + expect(WebMock).to have_requested(:patch, update_school_email_domains_url).with(body: expected_body) + end + + it 'returns a school if successful' do + data = { id: 'id', schoolCode: 'code', updatedAt: '2024-07-09T10:31:13.196Z', createdAt: '2024-07-09T10:31:13.196Z', discardedAt: nil, + studentEmailDomains: school_email_domains } + expected = ProfileApiClient::School.new(**data) + stub_request(:patch, update_school_email_domains_url) + .to_return(status: 201, body: data.to_json, headers: { 'Content-Type' => 'application/json' }) + expect(update_school_email_domains_response).to eq(expected) + end + + describe 'when BYPASS_OAUTH is true' do + before do + allow(ENV).to receive(:[]).with('BYPASS_OAUTH').and_return(true) + end + + it 'does not make a request to Profile API' do + update_school_email_domains_response + expect(WebMock).not_to have_requested(:patch, update_school_email_domains_url) + end + + it 'returns the id and email domains of the school supplied' do + expected = { 'id' => school.id, 'studentEmailDomains' => school_email_domains } + expect(update_school_email_domains_response).to eq(expected) + end + end + + private + + def update_school_email_domains + described_class.update_school_email_domains(token:, school_id: school.id, school_email_domains:) + end + end end diff --git a/spec/support/profile_api_mock.rb b/spec/support/profile_api_mock.rb index bbfa938a3..db337f56e 100644 --- a/spec/support/profile_api_mock.rb +++ b/spec/support/profile_api_mock.rb @@ -30,12 +30,13 @@ 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') + def stub_profile_api_create_school(id: SecureRandom.uuid, code: '99-12-34', student_email_domains: []) now = Time.current.to_fs(:iso8601) # rubocop:disable Naming/VariableNumber allow(ProfileApiClient).to receive(:create_school).and_return( ProfileApiClient::School.new( id:, schoolCode: code, + studentEmailDomains: student_email_domains, updatedAt: now, createdAt: now, discardedAt: nil From 9570e79b518d5b000ec4e67925ba1ac61b6038a5 Mon Sep 17 00:00:00 2001 From: Pete Simonovic <69108995+PetarSimonovic@users.noreply.github.com> Date: Fri, 1 May 2026 11:09:51 +0100 Subject: [PATCH 2/2] Create UpdateSchoolEmailDomainsJob Set up a job to update Profile's school email domains --- app/jobs/update_school_email_domains_job.rb | 41 +++++++++++ .../update_school_email_domains_job_spec.rb | 70 +++++++++++++++++++ 2 files changed, 111 insertions(+) create mode 100644 app/jobs/update_school_email_domains_job.rb create mode 100644 spec/jobs/update_school_email_domains_job_spec.rb diff --git a/app/jobs/update_school_email_domains_job.rb b/app/jobs/update_school_email_domains_job.rb new file mode 100644 index 000000000..506cafdd0 --- /dev/null +++ b/app/jobs/update_school_email_domains_job.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +class UpdateSchoolEmailDomainsJob < ApplicationJob + class ConcurrencyExceededForSchool < StandardError; end + + include GoodJob::ActiveJobExtensions::Concurrency + + # Serialize runs per school (eg user edits domains while batch or another job is updating Profile). + good_job_control_concurrency_with( + perform_limit: 1, + key: -> { "#{self.class.name}/#{concurrency_key_id}" } + ) + + retry_on StandardError, wait: :polynomially_longer, attempts: 3 do |_job, e| + Sentry.capture_exception(e) + raise e + end + + # Don't retry... + rescue_from ConcurrencyExceededForSchool do |e| + Rails.logger.error "Only one job per school can be enqueued at a time: #{concurrency_key_id}" + Sentry.capture_exception(e) + raise e + end + + queue_as :update_school_email_domains_job + + def self.attempt_perform_later(school_id:, school_email_domains:, token:) + perform_later(school_id:, school_email_domains:, token:) + end + + def perform(school_id:, school_email_domains:, token:) + ProfileApiClient.update_school_email_domains(token:, school_id:, school_email_domains:) + end + + private + + def concurrency_key_id + arguments.first&.with_indifferent_access&.dig(:school_id) + end +end diff --git a/spec/jobs/update_school_email_domains_job_spec.rb b/spec/jobs/update_school_email_domains_job_spec.rb new file mode 100644 index 000000000..598d61324 --- /dev/null +++ b/spec/jobs/update_school_email_domains_job_spec.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe UpdateSchoolEmailDomainsJob do + include ActiveJob::TestHelper + + let(:token) { UserProfileMock::TOKEN } + let(:school) { create(:verified_school) } + let(:school_email_domains) { ['student.example.edu', 'mail.example.org'] } + let(:profile_api_base_url) { 'http://profile.com' } + let(:profile_api_key) { 'api-key' } + let(:update_school_email_domains_path) { "/api/v1/schools/#{school.id}" } + let(:update_school_email_domains_url) { "#{profile_api_base_url}#{update_school_email_domains_path}" } + + around do |example| + ClimateControl.modify( + IDENTITY_URL: profile_api_base_url, + PROFILE_API_KEY: profile_api_key + ) do + example.run + end + end + + before do + stub_request(:patch, update_school_email_domains_url) + .to_return( + status: 201, + body: { + id: school.id, + schoolCode: '99-12-34', + updatedAt: '2024-07-09T10:31:13.196Z', + createdAt: '2024-07-09T10:31:13.196Z', + discardedAt: nil, + studentEmailDomains: school_email_domains + }.to_json, + headers: { 'Content-Type' => 'application/json' } + ) + end + + after do + GoodJob::Job.delete_all + end + + describe 'GoodJob concurrency' do + it 'uses a per-school concurrency key' do + job = described_class.new + job.arguments = [{ school_id: school.id, school_email_domains:, token: }] + + expect(job.good_job_concurrency_key).to eq("#{described_class.name}/#{school.id}") + end + + it 'uses a different concurrency key for a different school' do + other_school = create(:verified_school) + job_a = described_class.new + job_a.arguments = [{ school_id: school.id, school_email_domains:, token: }] + job_b = described_class.new + job_b.arguments = [{ school_id: other_school.id, school_email_domains:, token: }] + + expect(job_a.good_job_concurrency_key).not_to eq(job_b.good_job_concurrency_key) + end + end + + it 'PATCHes student email domains to the Profile API' do + described_class.perform_now(token:, school_id: school.id, school_email_domains:) + expect(WebMock).to have_requested(:patch, update_school_email_domains_url).with( + body: { studentEmailDomains: school_email_domains }.to_json + ) + end +end