diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 00c2fc12dc..fc7987fa72 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -12,7 +12,7 @@ RSpec/AnyInstance: - 'spec/requests/admin/legal_aid_applications/submissions_controller_spec.rb' - 'spec/requests/providers/uploaded_evidence_collection_spec.rb' - 'spec/requests/providers/use_ccms_controller_spec.rb' - - 'spec/requests/saml_sessions_spec.rb' + - 'spec/requests/saml_sessions_controller_spec.rb' - 'spec/services/ccms/attribute_configuration_spec.rb' - 'spec/services/ccms/manual_review_determiner_spec.rb' - 'spec/services/ccms/payload_generators/entity_attributes_generator_spec.rb' diff --git a/app/models/provider.rb b/app/models/provider.rb index 73bb9c1285..e665b51eab 100644 --- a/app/models/provider.rb +++ b/app/models/provider.rb @@ -21,7 +21,7 @@ def update_details end def update_details_directly - ProviderDetailsCreator.call(self) + PDA::ProviderDetailsCreator.call(self) end def sca_permissions? diff --git a/app/services/provider_after_login_service.rb b/app/services/provider_after_login_service.rb index 498b50058f..1afe45d901 100644 --- a/app/services/provider_after_login_service.rb +++ b/app/services/provider_after_login_service.rb @@ -27,11 +27,11 @@ def check_provider_details_api end def call_provider_details_api - ProviderDetailsCreator.call(@provider) + PDA::ProviderDetailsCreator.call(@provider) @provider.clear_invalid_login! - rescue ProviderDetailsRetriever::ApiRecordNotFoundError + rescue PDA::ProviderDetailsRetriever::ApiRecordNotFoundError @provider.update!(invalid_login_details: "api_details_user_not_found") - rescue ProviderDetailsRetriever::ApiError + rescue PDA::ProviderDetailsRetriever::ApiError @provider.update!(invalid_login_details: "provider_details_api_error") end end diff --git a/app/services/provider_details_retriever.rb b/app/services/provider_details_retriever.rb index 9dd7a61d8b..ec080098b8 100644 --- a/app/services/provider_details_retriever.rb +++ b/app/services/provider_details_retriever.rb @@ -13,14 +13,18 @@ def initialize(username) end def call + # :nocov: Rails.logger.info "**** Using #{self.class} to retrieve Provider Details" unless Rails.env.test? + # :nocov: provider_details end private def provider_details + # :nocov: raise_record_not_found_error if response.is_a?(Net::HTTPNotFound) + # :nocov: raise_error unless response.is_a?(Net::HTTPOK) @@ -50,6 +54,8 @@ def raise_error end def raise_record_not_found_error + # :nocov: raise ApiRecordNotFoundError, "Retrieval Failed: #{response.message} (#{response.code}) #{response.body}" + # :nocov: end end diff --git a/helm_deploy/apply-for-legal-aid/values-uat.yaml b/helm_deploy/apply-for-legal-aid/values-uat.yaml index f9f5bc8517..59de1240e2 100644 --- a/helm_deploy/apply-for-legal-aid/values-uat.yaml +++ b/helm_deploy/apply-for-legal-aid/values-uat.yaml @@ -103,7 +103,7 @@ laa_portal: idpSsoTargetUrl: "https://portal.uat.legalservices.gov.uk/oamfed/idp/samlv20" idpSloTargetUrl: "https://portal.uat.legalservices.gov.uk/oam/server/logout" idpCertFingerprintAlgorithm: "" - mockSaml: "true" + mockSaml: "false" provider_details: url: "https://ccms-pda.stg.legalservices.gov.uk/api/providerDetails" diff --git a/spec/models/provider_spec.rb b/spec/models/provider_spec.rb index 99ee25ca90..f140251406 100644 --- a/spec/models/provider_spec.rb +++ b/spec/models/provider_spec.rb @@ -7,7 +7,7 @@ describe "#update_details" do context "when firm exists" do it "does not call provider details creator immediately" do - expect(ProviderDetailsCreator).not_to receive(:call).with(provider) + expect(PDA::ProviderDetailsCreator).not_to receive(:call).with(provider) provider.update_details end diff --git a/spec/requests/saml_sessions_spec.rb b/spec/requests/saml_sessions_controller_spec.rb similarity index 77% rename from spec/requests/saml_sessions_spec.rb rename to spec/requests/saml_sessions_controller_spec.rb index 744ae8e2e6..1f33982255 100644 --- a/spec/requests/saml_sessions_spec.rb +++ b/spec/requests/saml_sessions_controller_spec.rb @@ -1,13 +1,15 @@ require "rails_helper" require "sidekiq/testing" -RSpec.describe "SamlSessionsController" do +RSpec.describe SamlSessionsController do let(:firm) { create(:firm, offices: [office]) } let(:office) { create(:office) } let(:provider) { create(:provider, firm:, selected_office: office, offices: [office], username:) } let(:username) { "bob the builder" } - let(:provider_details_api_url) { "#{Rails.configuration.x.provider_details.url}#{username.gsub(' ', '%20')}" } - let(:provider_details_api_reponse) { api_response.to_json } + let(:firm_id) { "3959183" } + let(:provider_details_api_url) { "#{Rails.configuration.x.pda.url}/provider-users/#{username.gsub(' ', '%20')}/provider-offices" } + let(:provider_firms_api_url) { "#{Rails.configuration.x.pda.url}/provider-firms/#{firm_id}/provider-offices" } + let(:provider_details_api_response) { api_response.to_json } let(:enable_mock_saml) { false } before do @@ -48,7 +50,8 @@ before do allow_any_instance_of(Warden::Proxy).to receive(:authenticate!).and_return(provider) - stub_request(:get, provider_details_api_url).to_return(body: provider_details_api_reponse, status:) + stub_request(:get, provider_details_api_url).to_return(body: provider_details_api_response, status:) + stub_request(:get, provider_firms_api_url).to_return(body: firm_response.to_json, status:) end context "when on staging or production" do @@ -61,16 +64,15 @@ let(:provider) { create(:provider, :created_by_devise, :with_ccms_apply_role, username:) } it "calls the Provider details api" do - expect(ProviderDetailsCreator).to receive(:call).with(provider).and_call_original + expect(PDA::ProviderDetailsCreator).to receive(:call).with(provider).and_call_original post_request end it "updates the record with firm and offices" do post_request - firm = Firm.find_by(ccms_id: raw_details_response[:providerFirmId]) - expect(provider.firm_id).to eq firm.id - expect(firm.offices.size).to eq 1 - expect(firm.offices.first.ccms_id).to eq raw_details_response[:providerOffices].first[:id].to_s + expect(provider.firm.ccms_id).to eq "10835831" + expect(provider.firm.offices.size).to eq 1 + expect(provider.firm.offices.first.ccms_id).to eq "34406595" end context "when mock_saml is activated" do @@ -96,7 +98,7 @@ before { allow(Rails.configuration.x.laa_portal).to receive(:mock_saml).and_return(false) } it "does not call the ProviderDetailsCreator" do - expect(ProviderDetailsCreator).not_to receive(:call) + expect(PDA::ProviderDetailsCreator).not_to receive(:call) post_request end @@ -112,12 +114,12 @@ end context "and the provider does not exist on Provider details api" do - let(:api_response) { raw_404_response } - let(:status) { 404 } + let(:api_response) { blank_response } + let(:status) { 204 } let(:provider) { create(:provider, :created_by_devise, :with_ccms_apply_role, username:) } it "calls the Provider details creator" do - expect(ProviderDetailsCreator).to receive(:call).with(provider).and_call_original + expect(PDA::ProviderDetailsCreator).to receive(:call).with(provider).and_call_original post_request end @@ -154,7 +156,7 @@ it "uses a worker to update details" do ProviderDetailsCreatorWorker.clear expect(ProviderDetailsCreatorWorker).to receive(:perform_async).with(provider.id).and_call_original - expect(ProviderDetailsCreator).to receive(:call).with(provider).and_call_original + expect(PDA::ProviderDetailsCreator).to receive(:call).with(provider).and_call_original post_request ProviderDetailsCreatorWorker.drain end @@ -172,16 +174,15 @@ let(:provider) { create(:provider, :created_by_devise, :with_ccms_apply_role, username:) } it "calls the Provider details api" do - expect(ProviderDetailsCreator).to receive(:call).with(provider).and_call_original + expect(PDA::ProviderDetailsCreator).to receive(:call).with(provider).and_call_original post_request end it "updates the record with firm and offices" do post_request - firm = Firm.find_by(ccms_id: raw_details_response[:providerFirmId]) - expect(provider.firm_id).to eq firm.id - expect(firm.offices.size).to eq 1 - expect(firm.offices.first.ccms_id).to eq raw_details_response[:providerOffices].first[:id].to_s + expect(provider.firm.ccms_id).to eq "10835831" + expect(provider.firm.offices.size).to eq 1 + expect(provider.firm.offices.first.ccms_id).to eq "34406595" end it "displays the start page" do @@ -200,7 +201,7 @@ it "does not schedule a job to update the provider details" do expect(provider).to receive(:update_details).and_call_original expect(ProviderDetailsCreatorWorker).not_to receive(:perform_async).with(provider.id) - expect(ProviderDetailsCreator).not_to receive(:call).with(provider) + expect(PDA::ProviderDetailsCreator).not_to receive(:call).with(provider) post_request end end @@ -221,34 +222,38 @@ def raw_details_response { - providerFirmId: 22_381, - contactUserId: 29_562, - contacts: [ - { - id: 568_352, - name: "SALLYCORNHILL", - }, + firm: { + firmId: 3_959_183, + ccmsFirmId: 10_835_831, + # trimmed for stub usage + }, + user: { + # trimmed for stub usage + }, + officeCodes: [ { - id: 2_017_809, - name: username, - }, - ], - providerOffices: [ - { - id: 81_693, - name: "Test1 and Co", + firmOfficeId: 145_434, + ccmsFirmOfficeId: 34_406_595, + # trimmed for stub usage }, ], } end - def raw_404_response + def firm_response { - timestamp: "2020-10-27T12:15:13.639+0000", - status: 404, - error: "Not Found", - message: "No records found for [bob%20the%20builder]", - path: "/api/providerDetails/bob%20the%20builder", + firm: { + firmId: 3_959_183, + ccmsFirmId: 10_835_831, + # trimmed for stub usage + }, + offices: [ + { + firmOfficeId: 145_434, + ccmsFirmOfficeId: 34_406_595, + # trimmed for stub usage + }, + ], } end diff --git a/spec/services/provider_after_login_service_spec.rb b/spec/services/provider_after_login_service_spec.rb index ff84010f55..f84cf9c0e9 100644 --- a/spec/services/provider_after_login_service_spec.rb +++ b/spec/services/provider_after_login_service_spec.rb @@ -21,7 +21,7 @@ let(:provider) { create(:provider, :created_by_devise, :with_ccms_apply_role, invalid_login_details: "provider_details_api_error") } context "and the provider cannot be found on Provider Details API" do - before { allow(ProviderDetailsCreator).to receive(:call).and_raise(ProviderDetailsRetriever::ApiRecordNotFoundError) } + before { allow(PDA::ProviderDetailsCreator).to receive(:call).and_raise(PDA::ProviderDetailsRetriever::ApiRecordNotFoundError) } it "updates the provider invalid login details" do call @@ -30,7 +30,7 @@ end context "and the provider found on Provider Details API" do - before { allow(ProviderDetailsCreator).to receive(:call).with(provider) } + before { allow(PDA::ProviderDetailsCreator).to receive(:call).with(provider) } it "does not update invalid login details" do call @@ -39,7 +39,7 @@ end context "and the provider details API not available" do - before { allow(ProviderDetailsCreator).to receive(:call).and_raise(ProviderDetailsRetriever::ApiError) } + before { allow(PDA::ProviderDetailsCreator).to receive(:call).and_raise(PDA::ProviderDetailsRetriever::ApiError) } it "updates the provider invalid login details" do call