Skip to content

Commit

Permalink
Merge pull request #7453 from ministryofjustice/ap-5177/switch-to-new…
Browse files Browse the repository at this point in the history
…-pda

AP-5177: Switch to new PDA after login
  • Loading branch information
colinbruce authored Nov 26, 2024
2 parents a2ca0d6 + c3aabb0 commit 9cf2c13
Show file tree
Hide file tree
Showing 8 changed files with 62 additions and 51 deletions.
2 changes: 1 addition & 1 deletion .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
2 changes: 1 addition & 1 deletion app/models/provider.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def update_details
end

def update_details_directly
ProviderDetailsCreator.call(self)
PDA::ProviderDetailsCreator.call(self)
end

def sca_permissions?
Expand Down
6 changes: 3 additions & 3 deletions app/services/provider_after_login_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 6 additions & 0 deletions app/services/provider_details_retriever.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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
2 changes: 1 addition & 1 deletion helm_deploy/apply-for-legal-aid/values-uat.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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: "<idp-cert-fingerprint-alg-goes-here>"
mockSaml: "true"
mockSaml: "false"

provider_details:
url: "https://ccms-pda.stg.legalservices.gov.uk/api/providerDetails"
Expand Down
2 changes: 1 addition & 1 deletion spec/models/provider_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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

Expand All @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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

Expand Down
6 changes: 3 additions & 3 deletions spec/services/provider_after_login_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down

0 comments on commit 9cf2c13

Please sign in to comment.