From 0cb13c6dfe1538e43313e4b0627a3fdab645e341 Mon Sep 17 00:00:00 2001 From: Chris Zetter <253059100+zetter-rpf@users.noreply.github.com> Date: Tue, 19 May 2026 16:15:01 +0100 Subject: [PATCH 1/3] Remove school response This isn't used anywhere and makes the code brittle as it will fail if new attributes are added to profile --- lib/profile_api_client.rb | 5 ++--- spec/lib/profile_api_client_spec.rb | 10 ++++------ spec/support/profile_api_mock.rb | 14 ++------------ 3 files changed, 8 insertions(+), 21 deletions(-) diff --git a/lib/profile_api_client.rb b/lib/profile_api_client.rb index 44215e4a6..36103da05 100644 --- a/lib/profile_api_client.rb +++ b/lib/profile_api_client.rb @@ -7,7 +7,6 @@ class ProfileApiClient }.freeze # rubocop:disable Naming/MethodName - 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 @@ -65,7 +64,7 @@ def create_school(token:, id:, code:, school_email_domains: []) unauthorized!(response) raise UnexpectedResponse, response unless response.status == 201 - School.new(**response.body) + true end def school_student(token:, school_id:, student_id:) @@ -227,7 +226,7 @@ def update_school_email_domains(token:, school_id:, school_email_domains: []) unauthorized!(response) raise UnexpectedResponse, response unless response.status == 200 - School.new(**response.body) + true end private diff --git a/spec/lib/profile_api_client_spec.rb b/spec/lib/profile_api_client_spec.rb index 1a69d740c..890b050e4 100644 --- a/spec/lib/profile_api_client_spec.rb +++ b/spec/lib/profile_api_client_spec.rb @@ -73,13 +73,12 @@ expect(WebMock).to have_requested(:post, create_school_url).with(body: expected_body) end - it 'returns the created school if successful' do + it 'returns true 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: [] } - expected = ProfileApiClient::School.new(**data) stub_request(:post, create_school_url) .to_return(status: 201, body: data.to_json, headers: { 'Content-Type' => 'application/json' }) - expect(create_school_response).to eq(expected) + expect(create_school_response).to be(true) end describe 'when BYPASS_OAUTH is true' do @@ -586,13 +585,12 @@ def school_student expect(WebMock).to have_requested(:patch, update_school_email_domains_url).with(body: expected_body) end - it 'returns a school if successful' do + it 'returns true 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: 200, body: data.to_json, headers: { 'Content-Type' => 'application/json' }) - expect(update_school_email_domains_response).to eq(expected) + expect(update_school_email_domains_response).to be(true) end describe 'when BYPASS_OAUTH is true' do diff --git a/spec/support/profile_api_mock.rb b/spec/support/profile_api_mock.rb index db337f56e..b3fae6cc5 100644 --- a/spec/support/profile_api_mock.rb +++ b/spec/support/profile_api_mock.rb @@ -30,18 +30,8 @@ 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', 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 - ) - ) + def stub_profile_api_create_school + allow(ProfileApiClient).to receive(:create_school).and_return(true) end def stub_profile_api_create_school_students(user_ids: [SecureRandom.uuid]) From 031173dd98805eb7fdfc6cd708425d092eea40cb Mon Sep 17 00:00:00 2001 From: Chris Zetter <253059100+zetter-rpf@users.noreply.github.com> Date: Tue, 19 May 2026 16:15:32 +0100 Subject: [PATCH 2/3] Remove unused safeguarding flags It looks like were not using this, so remove it --- lib/profile_api_client.rb | 10 -------- spec/lib/profile_api_client_spec.rb | 37 ----------------------------- 2 files changed, 47 deletions(-) diff --git a/lib/profile_api_client.rb b/lib/profile_api_client.rb index 36103da05..05a621d02 100644 --- a/lib/profile_api_client.rb +++ b/lib/profile_api_client.rb @@ -7,7 +7,6 @@ class ProfileApiClient }.freeze # rubocop:disable Naming/MethodName - 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 @@ -189,15 +188,6 @@ def delete_school_student(token:, school_id:, student_id:) 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:, school_id:) response = connection(token).post('/api/v1/safeguarding-flags') do |request| request.body = { flag:, email:, schoolId: school_id } diff --git a/spec/lib/profile_api_client_spec.rb b/spec/lib/profile_api_client_spec.rb index 890b050e4..0fbcc3cfc 100644 --- a/spec/lib/profile_api_client_spec.rb +++ b/spec/lib/profile_api_client_spec.rb @@ -104,43 +104,6 @@ def create_school end end - describe '.safeguarding_flags' do - subject(:safeguarding_flags_response) { list_safeguarding_flags } - - 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_behaves_like 'an authenticated API request', :get, url: -> { list_safeguarding_flags_url } - it_behaves_like 'a request that handles standard HTTP errors', :get, url: -> { list_safeguarding_flags_url } - it_behaves_like 'a request that handles an unexpected response status', :get, url: -> { list_safeguarding_flags_url }, status: 201 - - it 'returns list of safeguarding flags if successful' do - flag = { - id: '7ac79585-e187-4d2f-bf0c-a1cbe72ecc9a', - userId: '583ba872-b16e-46e1-9f7d-df89d267550d', - flag: 'school:owner', - email: 'user@example.com', - createdAt: '2024-07-01T12:49:18.926Z', - updatedAt: '2024-07-01T12:49:18.926Z', - discardedAt: nil, - schoolId: SecureRandom.uuid - } - expected = ProfileApiClient::SafeguardingFlag.new(**flag) - stub_request(:get, list_safeguarding_flags_url) - .to_return(status: 200, body: [flag].to_json, headers: { 'content-type' => 'application/json' }) - expect(safeguarding_flags_response).to eq([expected]) - end - - private - - def list_safeguarding_flags - described_class.safeguarding_flags(token:) - end - end - describe '.create_safeguarding_flag' do subject(:create_safeguarding_flag_response) { create_safeguarding_flag } From 2c57635dcb212591de2b69878430d99ee4e380c4 Mon Sep 17 00:00:00 2001 From: Chris Zetter <253059100+zetter-rpf@users.noreply.github.com> Date: Tue, 19 May 2026 16:29:21 +0100 Subject: [PATCH 3/3] Don't error if new keys are added to student response It's common when developing with APIs to add in new keys to the response if this happens we don't want this code to error. --- lib/profile_api_client.rb | 4 +++- spec/lib/profile_api_client_spec.rb | 9 +++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/lib/profile_api_client.rb b/lib/profile_api_client.rb index 05a621d02..cbd45d8e1 100644 --- a/lib/profile_api_client.rb +++ b/lib/profile_api_client.rb @@ -255,7 +255,9 @@ def build_student(attrs) symbolized_attrs[:email] ||= nil symbolized_attrs[:ssoProviders] ||= [] - Student.new(**symbolized_attrs) + allowed_keys = ProfileApiClient::Student.members + + Student.new(**symbolized_attrs.slice(*allowed_keys)) end def unauthorized!(response) diff --git a/spec/lib/profile_api_client_spec.rb b/spec/lib/profile_api_client_spec.rb index 0fbcc3cfc..0b9dfb446 100644 --- a/spec/lib/profile_api_client_spec.rb +++ b/spec/lib/profile_api_client_spec.rb @@ -515,6 +515,15 @@ def update_school_student expect(school_student_response).to eq(expected) end + it 'returns student even if response contains unexpected keys' do + data = { id: student_id, schoolId: school.id, name: 'name', username: 'username', email: 'test@example.com', ssoProviders: [], createdAt: '', updatedAt: '', discardedAt: '' } + response = data.merge({ unexpectedKey: 'unexpectedValue' }) + expected = ProfileApiClient::Student.new(**data) + stub_request(:get, student_url) + .to_return(status: 200, body: response.to_json, headers: { 'content-type' => 'application/json' }) + expect(school_student_response).to eq(expected) + end + private def school_student