diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 52555c7c4c..c9e38e71e6 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_controller_spec.rb' + - 'spec/requests/saml_sessions_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 e665b51eab..73bb9c1285 100644 --- a/app/models/provider.rb +++ b/app/models/provider.rb @@ -21,7 +21,7 @@ def update_details end def update_details_directly - PDA::ProviderDetailsCreator.call(self) + 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 1afe45d901..498b50058f 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 - PDA::ProviderDetailsCreator.call(@provider) + ProviderDetailsCreator.call(@provider) @provider.clear_invalid_login! - rescue PDA::ProviderDetailsRetriever::ApiRecordNotFoundError + rescue ProviderDetailsRetriever::ApiRecordNotFoundError @provider.update!(invalid_login_details: "api_details_user_not_found") - rescue PDA::ProviderDetailsRetriever::ApiError + rescue 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 ec080098b8..9dd7a61d8b 100644 --- a/app/services/provider_details_retriever.rb +++ b/app/services/provider_details_retriever.rb @@ -13,18 +13,14 @@ 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) @@ -54,8 +50,6 @@ 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/spec/models/provider_spec.rb b/spec/models/provider_spec.rb index f140251406..99ee25ca90 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(PDA::ProviderDetailsCreator).not_to receive(:call).with(provider) + expect(ProviderDetailsCreator).not_to receive(:call).with(provider) provider.update_details end diff --git a/spec/requests/saml_sessions_controller_spec.rb b/spec/requests/saml_sessions_spec.rb similarity index 77% rename from spec/requests/saml_sessions_controller_spec.rb rename to spec/requests/saml_sessions_spec.rb index 1f33982255..744ae8e2e6 100644 --- a/spec/requests/saml_sessions_controller_spec.rb +++ b/spec/requests/saml_sessions_spec.rb @@ -1,15 +1,13 @@ 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(: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(:provider_details_api_url) { "#{Rails.configuration.x.provider_details.url}#{username.gsub(' ', '%20')}" } + let(:provider_details_api_reponse) { api_response.to_json } let(:enable_mock_saml) { false } before do @@ -50,8 +48,7 @@ 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_response, status:) - stub_request(:get, provider_firms_api_url).to_return(body: firm_response.to_json, status:) + stub_request(:get, provider_details_api_url).to_return(body: provider_details_api_reponse, status:) end context "when on staging or production" do @@ -64,15 +61,16 @@ let(:provider) { create(:provider, :created_by_devise, :with_ccms_apply_role, username:) } it "calls the Provider details api" do - expect(PDA::ProviderDetailsCreator).to receive(:call).with(provider).and_call_original + expect(ProviderDetailsCreator).to receive(:call).with(provider).and_call_original post_request end it "updates the record with firm and offices" do post_request - 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" + 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 end context "when mock_saml is activated" do @@ -98,7 +96,7 @@ before { allow(Rails.configuration.x.laa_portal).to receive(:mock_saml).and_return(false) } it "does not call the ProviderDetailsCreator" do - expect(PDA::ProviderDetailsCreator).not_to receive(:call) + expect(ProviderDetailsCreator).not_to receive(:call) post_request end @@ -114,12 +112,12 @@ end context "and the provider does not exist on Provider details api" do - let(:api_response) { blank_response } - let(:status) { 204 } + let(:api_response) { raw_404_response } + let(:status) { 404 } let(:provider) { create(:provider, :created_by_devise, :with_ccms_apply_role, username:) } it "calls the Provider details creator" do - expect(PDA::ProviderDetailsCreator).to receive(:call).with(provider).and_call_original + expect(ProviderDetailsCreator).to receive(:call).with(provider).and_call_original post_request end @@ -156,7 +154,7 @@ it "uses a worker to update details" do ProviderDetailsCreatorWorker.clear expect(ProviderDetailsCreatorWorker).to receive(:perform_async).with(provider.id).and_call_original - expect(PDA::ProviderDetailsCreator).to receive(:call).with(provider).and_call_original + expect(ProviderDetailsCreator).to receive(:call).with(provider).and_call_original post_request ProviderDetailsCreatorWorker.drain end @@ -174,15 +172,16 @@ let(:provider) { create(:provider, :created_by_devise, :with_ccms_apply_role, username:) } it "calls the Provider details api" do - expect(PDA::ProviderDetailsCreator).to receive(:call).with(provider).and_call_original + expect(ProviderDetailsCreator).to receive(:call).with(provider).and_call_original post_request end it "updates the record with firm and offices" do post_request - 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" + 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 end it "displays the start page" do @@ -201,7 +200,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(PDA::ProviderDetailsCreator).not_to receive(:call).with(provider) + expect(ProviderDetailsCreator).not_to receive(:call).with(provider) post_request end end @@ -222,38 +221,34 @@ def raw_details_response { - firm: { - firmId: 3_959_183, - ccmsFirmId: 10_835_831, - # trimmed for stub usage - }, - user: { - # trimmed for stub usage - }, - officeCodes: [ + providerFirmId: 22_381, + contactUserId: 29_562, + contacts: [ + { + id: 568_352, + name: "SALLYCORNHILL", + }, { - firmOfficeId: 145_434, - ccmsFirmOfficeId: 34_406_595, - # trimmed for stub usage + id: 2_017_809, + name: username, + }, + ], + providerOffices: [ + { + id: 81_693, + name: "Test1 and Co", }, ], } end - def firm_response + def raw_404_response { - 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 - }, - ], + 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", } end diff --git a/spec/services/provider_after_login_service_spec.rb b/spec/services/provider_after_login_service_spec.rb index f84cf9c0e9..ff84010f55 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(PDA::ProviderDetailsCreator).to receive(:call).and_raise(PDA::ProviderDetailsRetriever::ApiRecordNotFoundError) } + before { allow(ProviderDetailsCreator).to receive(:call).and_raise(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(PDA::ProviderDetailsCreator).to receive(:call).with(provider) } + before { allow(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(PDA::ProviderDetailsCreator).to receive(:call).and_raise(PDA::ProviderDetailsRetriever::ApiError) } + before { allow(ProviderDetailsCreator).to receive(:call).and_raise(ProviderDetailsRetriever::ApiError) } it "updates the provider invalid login details" do call