Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ap-4463: poc refactor provider_dwp_override flow #6002

Closed
wants to merge 7 commits into from
Closed

ap-4463: poc refactor provider_dwp_override flow #6002

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4fc8fc0
ap-4463: refactor confirm dwp non passported step
kmahern Nov 16, 2023
50e6d50
ap-4463: refactor check_client_details step
kmahern Nov 20, 2023
7ba2366
ap-4463: refactor received_benefits_confirmations step
kmahern Nov 20, 2023
2a51d07
ap-4463: refactor has_evidence_of_benefits step
kmahern Nov 20, 2023
768cb82
ap-4463: add ProviderDWPOveride namespace to break steps into collect…
kmahern Nov 22, 2023
844ad12
ap-4463: use keyword arguments for step definitions
kmahern Nov 22, 2023
1b18607
ap-4463: use custom_matcher for testing flow steps
kmahern Nov 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 4 additions & 40 deletions app/services/flow/flows/provider_dwp_override.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,46 +2,10 @@ module Flow
module Flows
class ProviderDWPOverride < FlowSteps
STEPS = {
confirm_dwp_non_passported_applications: {
path: ->(application) { urls.providers_legal_aid_application_confirm_dwp_non_passported_applications_path(application) },
forward: lambda do |application, confirm_dwp_non_passported|
if confirm_dwp_non_passported
application.change_state_machine_type("NonPassportedStateMachine")
:about_financial_means
else
application.change_state_machine_type("PassportedStateMachine")
:check_client_details
end
end,
},
check_client_details: {
path: ->(application) { urls.providers_legal_aid_application_check_client_details_path(application) },
forward: :received_benefit_confirmations,
},
received_benefit_confirmations: {
path: ->(application) { urls.providers_legal_aid_application_received_benefit_confirmation_path(application) },
forward: lambda do |application, has_benefit|
if has_benefit
application.change_state_machine_type("PassportedStateMachine")
:has_evidence_of_benefits
else
application.change_state_machine_type("NonPassportedStateMachine")
:about_financial_means
end
end,
},
has_evidence_of_benefits: {
path: ->(application) { urls.providers_legal_aid_application_has_evidence_of_benefit_path(application) },
forward: lambda do |application, has_evidence_of_benefit|
if has_evidence_of_benefit
application.change_state_machine_type("PassportedStateMachine")
application.used_delegated_functions? ? :substantive_applications : :capital_introductions
else
application.change_state_machine_type("NonPassportedStateMachine")
:about_financial_means
end
end,
},
confirm_dwp_non_passported_applications: Flow::Steps::ConfirmDWPNonPassportedApplicationsStep,
kmahern marked this conversation as resolved.
Show resolved Hide resolved
check_client_details: Flow::Steps::CheckClientDetailsStep,
received_benefit_confirmations: Flow::Steps::ReceivedBenefitConfirmationsStep,
has_evidence_of_benefits: Flow::Steps::HasEvidenceOfBenefitsStep,
}.freeze
end
end
Expand Down
9 changes: 9 additions & 0 deletions app/services/flow/steps/check_client_details_step.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module Flow
module Steps
CheckClientDetailsStep = Step.new(
kmahern marked this conversation as resolved.
Show resolved Hide resolved
->(application) { urls.providers_legal_aid_application_check_client_details_path(application) },
:received_benefit_confirmations,
nil,
)
end
end
17 changes: 17 additions & 0 deletions app/services/flow/steps/confirm_dwp_non_passported_applications_step.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
module Flow
module Steps
ConfirmDWPNonPassportedApplicationsStep = Step.new(
->(application) { urls.providers_legal_aid_application_confirm_dwp_non_passported_applications_path(application) },
lambda do |application, confirm_dwp_non_passported|
if confirm_dwp_non_passported
application.change_state_machine_type("NonPassportedStateMachine")
:about_financial_means
else
application.change_state_machine_type("PassportedStateMachine")
:check_client_details
end
end,
nil,
)
end
end
17 changes: 17 additions & 0 deletions app/services/flow/steps/has_evidence_of_benefits_step.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
module Flow
module Steps
HasEvidenceOfBenefitsStep = Step.new(
->(application) { urls.providers_legal_aid_application_has_evidence_of_benefit_path(application) },
lambda do |application, has_evidence_of_benefits|
if has_evidence_of_benefits
application.change_state_machine_type("PassportedStateMachine")
application.used_delegated_functions? ? :substantive_applications : :capital_introductions
else
application.change_state_machine_type("NonPassportedStateMachine")
:about_financial_means
end
end,
nil,
)
end
end
17 changes: 17 additions & 0 deletions app/services/flow/steps/received_benefit_confirmations_step.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
module Flow
module Steps
ReceivedBenefitConfirmationsStep = Step.new(
->(application) { urls.providers_legal_aid_application_received_benefit_confirmation_path(application) },
lambda do |application, has_benefit|
if has_benefit
application.change_state_machine_type("PassportedStateMachine")
:has_evidence_of_benefits
else
application.change_state_machine_type("NonPassportedStateMachine")
:about_financial_means
end
end,
nil,
)
end
end
9 changes: 9 additions & 0 deletions app/services/flow/steps/step.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module Flow
module Steps
def self.urls
Rails.application.routes.url_helpers
end

Step = Struct.new(:path, :forward, :check_answers)
kmahern marked this conversation as resolved.
Show resolved Hide resolved
end
end
13 changes: 0 additions & 13 deletions spec/requests/providers/check_client_details_controller_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,17 +102,4 @@
end
end
end

describe "PATCH /providers/applications/:legal_aid_application_id/check_client_details" do
subject(:patch_request) { patch "/providers/applications/#{application_id}/check_client_details" }

before do
login_as application.provider
patch_request
end

it "continues to the received benefit confirmations page" do
expect(response).to redirect_to(providers_legal_aid_application_received_benefit_confirmation_path(application))
end
end
end
25 changes: 0 additions & 25 deletions spec/requests/providers/confirm_dwp_non_passported_applications_controller_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,16 +108,6 @@
patch_request
end

it "displays the about financial means page" do
patch_request
expect(response).to redirect_to(providers_legal_aid_application_about_financial_means_path(application))
end

it "uses the non-passported state machine" do
patch_request
expect(application.reload.state_machine_proxy.type).to eq "NonPassportedStateMachine"
end

it "calls the HMRC::CreateResponsesService" do
patch_request
expect(HMRC::CreateResponsesService).to have_received(:call).once
Expand Down Expand Up @@ -150,21 +140,11 @@
expect(application.reload.state).to eq "overriding_dwp_result"
end

it "displays the check_client_details page" do
patch_request
expect(response).to redirect_to providers_legal_aid_application_check_client_details_path(application)
end

it "does not update the partner shared benefit field" do
patch_request
expect(partner.reload.shared_benefit_with_applicant).to be false
end

it "uses the passported state machine" do
patch_request
expect(application.reload.state_machine_proxy.type).to eq "PassportedStateMachine"
end

it "calls the HMRC::CreateResponsesService" do
patch_request
expect(HMRC::CreateResponsesService).to have_received(:call)
Expand Down Expand Up @@ -196,11 +176,6 @@
expect(partner.reload.shared_benefit_with_applicant).to be true
end

it "displays the check_client_details page" do
patch_request
expect(response).to redirect_to providers_legal_aid_application_check_client_details_path(application)
end

it "uses the passported state machine" do
patch_request
expect(application.reload.state_machine_proxy.type).to eq "PassportedStateMachine"
Expand Down
24 changes: 0 additions & 24 deletions spec/requests/providers/has_evidence_of_benefits_controller_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,37 +88,13 @@
expect(dwp_override.has_evidence_of_benefit).to be true
end

it "redirects to the upload substantive application page" do
expect(response).to redirect_to(providers_legal_aid_application_substantive_application_path(legal_aid_application))
end

context "when the provider has not used delegated functions" do
let(:legal_aid_application) { create(:legal_aid_application, :with_dwp_override, :applicant_details_checked) }

it "redirects to the upload capital introductions page" do
expect(response).to redirect_to(providers_legal_aid_application_capital_introduction_path(legal_aid_application))
end
end

it "updates the state machine type" do
expect(legal_aid_application.reload.state_machine).to be_a PassportedStateMachine
end

context "when the provider chose no" do
let(:has_evidence_of_benefit) { "false" }

it "updates the dwp_override model" do
dwp_override = legal_aid_application.reload.dwp_override
expect(dwp_override.has_evidence_of_benefit).to be false
end

it "redirects to the about financial means page" do
expect(response).to redirect_to(providers_legal_aid_application_about_financial_means_path(legal_aid_application))
end

it "updates the state machine type" do
expect(legal_aid_application.reload.state_machine).to be_a NonPassportedStateMachine
end
end

context "when the provider has chosen nothing" do
Expand Down
10 changes: 0 additions & 10 deletions spec/requests/providers/received_benefit_confirmations_controller_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,6 @@
expect { patch_request }.to change(DWPOverride, :count).by(-1)
end
end

it "continue to the has_evidence_of_benefit page" do
patch_request
expect(response).to redirect_to(providers_legal_aid_application_has_evidence_of_benefit_path(application))
end
end

context "when none of these selected" do
Expand All @@ -101,11 +96,6 @@
expect { patch_request }.not_to change(DWPOverride, :count)
end

it "continue to the about financial means page" do
patch_request
expect(response).to redirect_to(providers_legal_aid_application_about_financial_means_path(application))
end

it "transitions the application state to applicant details checked" do
patch_request
expect(application.reload.state).to eq "applicant_details_checked"
Expand Down
29 changes: 29 additions & 0 deletions spec/services/flow/steps/check_client_details_step_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
require "rails_helper"

RSpec.describe Flow::Steps::CheckClientDetailsStep do
let(:legal_aid_application) { create(:legal_aid_application) }

describe "#path" do
subject(:path) { described_class.path.call(legal_aid_application) }

it "returns the check_client_details_path when called" do
expect(path).to eq "/providers/applications/#{legal_aid_application.id}/check_client_details?locale=en"
end
end

describe "#forward" do
subject(:forward) { described_class.forward }

it "returns the about_financial_means step when called" do
expect(forward).to eq(:received_benefit_confirmations)
end
end

describe "#check_answers" do
subject(:check_answers) { described_class.check_answers }

it "returns nil" do
expect(check_answers).to be_nil
end
end
Copy link
Contributor

@jsugarman jsugarman Nov 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could use a generic custom matcher since we are going to do a lot of these

quick example:

# spec/support/have_flow_step_args.rb

RSpec::Matchers.define :have_flow_step_args do |expected|
  results = { path: nil, forward: nil, check_answers: nil }

  match do |actual|
    results[:path] = actual.path.call(legal_aid_application) == expected[:path]
    results[:forward] = actual.forward == expected[:forward]
    results[:check_answers] = actual.check_answers == expected[:check_answers]

    results.values.all?(true)
  end

  failure_message do |actual|
    message = ""
    message += "expected #{actual.path.call(legal_aid_application)} to equal #{expected[:path]}\n" if actual.path.call(legal_aid_application) != expected[:path]
    message += "expected #{actual.forward} to equal #{expected[:forward]}\n" if actual.forward != expected[:forward]
    message += "expected #{actual.check_answers} to equal #{expected[:check_answers]}\n" if actual.check_answers != expected[:check_answers]
    message
  end

  description do
    "have matching flow step arguments"
  end
end

And use:

  it "has expected flow step" do
    expect(described_class).to have_flow_step_args(path: "/providers/applications/#{legal_aid_application.id}/check_client_details?locale=en",
                                                   forward: :reeived_benefit_confirmations,
                                                   check_answers: nil)
  end

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added a commit to add a custom_matcher similar to the above. To make it generic, I have had to use a common name for the second paramater that is sometimes passed to the step lambda in the specs - I have called it args, but am not sure whether this is confusing?

end
51 changes: 51 additions & 0 deletions spec/services/flow/steps/confirm_dwp_non_passported_applications_step_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
require "rails_helper"

RSpec.describe Flow::Steps::ConfirmDWPNonPassportedApplicationsStep do
let(:legal_aid_application) { create(:legal_aid_application) }

describe "#path" do
subject(:path) { described_class.path.call(legal_aid_application) }

it "returns the confirm_dwp_non_passported_path when called" do
expect(path).to eq "/providers/applications/#{legal_aid_application.id}/confirm_dwp_non_passported_applications?locale=en"
end
end

describe "#forward" do
subject(:forward) { described_class.forward.call(legal_aid_application, confirm_dwp_non_passported) }

let(:confirm_dwp_non_passported) { true }

context "when the provider confirms the application is non passported" do
it "returns the about_financial_means step when called" do
expect(forward).to eq(:about_financial_means)
end

it "sets the application's state machine to be the non passported state machine" do
forward
expect(legal_aid_application.state_machine_proxy.type).to eq "NonPassportedStateMachine"
end
end

context "when the provider confirms the application is passported" do
let(:confirm_dwp_non_passported) { false }

it "returns the check_client_details step when called" do
expect(forward).to eq(:check_client_details)
end

it "sets the application's state machine to be the passported state machine" do
forward
expect(legal_aid_application.state_machine_proxy.type).to eq "PassportedStateMachine"
end
end
end

describe "#check_answers" do
subject(:check_answers) { described_class.check_answers }

it "returns nil" do
expect(check_answers).to be_nil
end
end
end
Loading