From 22989fdff2449201a6d1a19b4763c1eae41d665a Mon Sep 17 00:00:00 2001 From: Joel Sugarman Date: Tue, 12 Nov 2024 16:48:35 +0000 Subject: [PATCH] AP-5496: Standardise currency handling accross the app MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Certain currency/amount fields accepted humanized monetary values while others did not. Some would accept but clean `£` and `,` values such that `£1,000` would become `1000` while others would error. Yet others would ignore `£` and `,` values for validation purposes but when peristing this data would not, resulting in, for example, `£2,000` become a stored value of `2.0`. This PR standardise the approach, removing `£` and `,` values prior to saving across regular incomes, regular outgoings, cash incomes, cash outgoings, state benefits, housing benefit, student finance and mandatory and discretionary capital disregards monetary amount fields. In shouldbe noted that some forms validate currency, which cleans/ignores `£` and `,` chars, while others do not validate this way. This has not been changed. --- .../capital_disregards/add_details_form.rb | 4 + .../providers/means/state_benefit_form.rb | 4 + .../providers/regular_transaction_form.rb | 10 +- .../base_student_finance_form.rb | 4 + .../check_answers/_cash_payments.html.erb | 3 +- .../check_answers/_student_finance.html.erb | 2 +- .../check_your_means_answers.feature | 2 +- .../check_your_means_answers.feature | 2 +- .../enter_humanized_currency_amounts.feature | 141 ++++++++++++++++++ .../bank_statement_upload_steps.rb | 1 + .../step_definitions/civil_journey_steps.rb | 8 + .../add_details_form_spec.rb | 21 ++- .../means/housing_benefit_form_spec.rb | 15 ++ .../means/regular_income_form_spec.rb | 15 ++ .../means/regular_outgoings_form_spec.rb | 15 ++ .../means/state_benefit_form_spec.rb | 18 +++ 16 files changed, 254 insertions(+), 11 deletions(-) create mode 100644 features/providers/regressions/enter_humanized_currency_amounts.feature diff --git a/app/forms/providers/means/capital_disregards/add_details_form.rb b/app/forms/providers/means/capital_disregards/add_details_form.rb index f61384d80d..414db1e41a 100644 --- a/app/forms/providers/means/capital_disregards/add_details_form.rb +++ b/app/forms/providers/means/capital_disregards/add_details_form.rb @@ -41,6 +41,10 @@ def date_received @date_received = attributes[:date_received] = date_received_fields.form_date end + def attributes_to_clean + [:amount] + end + private def exclude_from_model diff --git a/app/forms/providers/means/state_benefit_form.rb b/app/forms/providers/means/state_benefit_form.rb index c1713d3903..0b14359f2d 100644 --- a/app/forms/providers/means/state_benefit_form.rb +++ b/app/forms/providers/means/state_benefit_form.rb @@ -19,6 +19,10 @@ def frequency_options RegularTransaction.frequencies_for(state_benefit_transaction_type) end + def attributes_to_clean + %i[amount] + end + private def state_benefit_transaction_type diff --git a/app/forms/providers/regular_transaction_form.rb b/app/forms/providers/regular_transaction_form.rb index 8f85038ffc..d977583fee 100644 --- a/app/forms/providers/regular_transaction_form.rb +++ b/app/forms/providers/regular_transaction_form.rb @@ -13,7 +13,7 @@ def initialize(params = {}) @legal_aid_application = params.delete(:legal_aid_application) @transaction_type_ids = params["transaction_type_ids"] || existing_transaction_type_ids owner.present? - assign_regular_transaction_attributes + assign_existing_regular_transaction_attributes super end @@ -96,7 +96,7 @@ def existing_transaction_type_ids .pluck(:transaction_type_id) end - def assign_regular_transaction_attributes + def assign_existing_regular_transaction_attributes regular_transactions.each do |transaction| transaction_type = transaction.transaction_type public_send(:"#{transaction_type.name}_amount=", transaction.amount) @@ -168,7 +168,7 @@ def none_and_another_checkbox_checked? def all_regular_transactions_valid regular_transactions.each do |transaction| transaction_type = transaction.transaction_type - transaction.amount = public_send(:"#{transaction_type.name}_amount") + transaction.amount = clean_amount(public_send(:"#{transaction_type.name}_amount")) transaction.frequency = public_send(:"#{transaction_type.name}_frequency") next if transaction.valid? @@ -182,6 +182,10 @@ def all_regular_transactions_valid end end + def clean_amount(amount) + amount.to_s.tr("£,", "") + end + def add_regular_transaction_error_to_form(transaction_type, error) attribute = if error.attribute.in?(%i[amount frequency]) :"#{transaction_type}_#{error.attribute}" diff --git a/app/forms/student_finances/base_student_finance_form.rb b/app/forms/student_finances/base_student_finance_form.rb index feaabe3dad..5e378a0b56 100644 --- a/app/forms/student_finances/base_student_finance_form.rb +++ b/app/forms/student_finances/base_student_finance_form.rb @@ -10,5 +10,9 @@ class BaseStudentFinanceForm < BaseForm def student_finance? student_finance.eql?("true") end + + def attributes_to_clean + %i[student_finance_amount] + end end end diff --git a/app/views/shared/check_answers/_cash_payments.html.erb b/app/views/shared/check_answers/_cash_payments.html.erb index 79167d8d2d..e45dfafdf3 100644 --- a/app/views/shared/check_answers/_cash_payments.html.erb +++ b/app/views/shared/check_answers/_cash_payments.html.erb @@ -1,6 +1,5 @@ <% read_only = false unless local_assigns.key?(:read_only) %> - -
+

<%= t(".#{type}_heading", individual_with_determiner:) %>

diff --git a/app/views/shared/check_answers/_student_finance.html.erb b/app/views/shared/check_answers/_student_finance.html.erb index 44b30f594e..e6110ce4f7 100644 --- a/app/views/shared/check_answers/_student_finance.html.erb +++ b/app/views/shared/check_answers/_student_finance.html.erb @@ -1,4 +1,4 @@ -
+

<%= t(".heading") %>

diff --git a/features/providers/bank_statement_upload/check_your_means_answers.feature b/features/providers/bank_statement_upload/check_your_means_answers.feature index 926b25666a..99cf9618a5 100644 --- a/features/providers/bank_statement_upload/check_your_means_answers.feature +++ b/features/providers/bank_statement_upload/check_your_means_answers.feature @@ -88,7 +88,7 @@ Feature: Bank statement upload check your answers And I click "Save and continue" Then I should be on the "check_income_answers" page showing "Check your answers" - When I click Check Your Answers Change link for "applicant student finance" + When I click Check Your Answers Change link for applicant 'student_finance' And I choose "Yes" And I enter amount "5000" diff --git a/features/providers/partner_means_assessment/bank_statement_upload/check_your_means_answers.feature b/features/providers/partner_means_assessment/bank_statement_upload/check_your_means_answers.feature index a97a52642b..f4a6a1002f 100644 --- a/features/providers/partner_means_assessment/bank_statement_upload/check_your_means_answers.feature +++ b/features/providers/partner_means_assessment/bank_statement_upload/check_your_means_answers.feature @@ -88,7 +88,7 @@ Feature: Bank statement upload check your answers And I click "Save and continue" Then I should be on the "check_income_answers" page showing "Check your answers" - When I click Check Your Answers Change link for "partner student finance" + When I click Check Your Answers Change link for partner 'student_finance' And I choose "Yes" And I enter amount "5000" diff --git a/features/providers/regressions/enter_humanized_currency_amounts.feature b/features/providers/regressions/enter_humanized_currency_amounts.feature new file mode 100644 index 0000000000..bb4588c146 --- /dev/null +++ b/features/providers/regressions/enter_humanized_currency_amounts.feature @@ -0,0 +1,141 @@ +@javascript +Feature: Entering humanized monetary amounts on various forms + Scenario: I can enter humanized monetary amounts like 1,000 for cash income + Given csrf is enabled + And I have completed a non-passported employed application for "client" with bank statements as far as the end of the means income section + Then I should be on the "check_income_answers" page showing "Check your answers" + + When I click Check Your Answers Change link for applicant 'cash_income' + And I select "Maintenance payments from a former partner" + And I fill "aggregated-cash-income-maintenance-in1-field" with "£2,654.33" + And I fill "aggregated-cash-income-maintenance-in2-field" with "£3,654.33" + And I fill "aggregated-cash-income-maintenance-in3-field" with "£4,654.33" + + When I click 'Save and continue' + Then I should be on the 'check_income_answers' page showing 'Check your answers' + And I should see "£2,654.33" + And I should see "£3,654.33" + And I should see "£4,654.33" + + Scenario: I can enter humanized monetary amounts like 1,000 for cash outgoings and housing benefit + Given csrf is enabled + And I have completed a non-passported employed application for "client" with bank statements as far as the end of the means income section + Then I should be on the "check_income_answers" page showing "Check your answers" + + When I click Check Your Answers Change link for applicant 'cash_outgoings' + And I select "Housing payments" + And I fill "aggregated-cash-outgoings-rent-or-mortgage1-field" with "£2,275.43" + And I fill "aggregated-cash-outgoings-rent-or-mortgage2-field" with "£3,275.43" + And I fill "aggregated-cash-outgoings-rent-or-mortgage3-field" with "£4,275.43" + + When I click 'Save and continue' + Then I should be on the 'housing_benefits' page showing "Does your client get Housing Benefit?" + + When I choose "Yes" + And I fill "providers-means-housing-benefit-form-housing-benefit-amount-field" with "£1,322.55" + And I choose "Monthly" + + When I click 'Save and continue' + Then I should be on the 'check_income_answers' page showing 'Check your answers' + And I should see "£2,275.43" + And I should see "£3,275.43" + And I should see "£4,275.43" + And I should see "£1,322.55" + + Scenario: I can enter humanized monetary amounts like 1,000 for state benefits + Given csrf is enabled + And I have completed a non-passported employed application for "client" with bank statements as far as the end of the means income section + Then I should be on the "check_income_answers" page showing "Check your answers" + + When I click Check Your Answers Change link for applicant 'state_benefits' + And I choose "Yes" + And I click 'Save and continue' + + Then I should be on a page with title "Add benefit, charitable or government payment details" + And I fill 'regular-transaction-description-field' with "my government handout" + And I fill 'regular-transaction-amount-field' with "£1,222,333.44" + And I choose "Every week" + + When I click 'Save and continue' + Then I should be on the 'add_other_state_benefits' page showing 'You added 1 benefit, charitable or government payment' + And I should see "£1,222,333.44" + + Scenario: I can enter humanized monetary amounts like 1,000 for student finance + Given csrf is enabled + And I have completed a non-passported employed application for "client" with bank statements as far as the end of the means income section + Then I should be on the 'check_income_answers' page showing 'Check your answers' + + When I click Check Your Answers Change link for applicant 'student_finance' + Then I should be on a page with title "Does your client get student finance?" + And I choose "Yes" + And I fill 'applicant-student-finance-amount-field' with "£5,432.11" + + When I click 'Save and continue' + Then I should be on the 'check_income_answers' page showing 'Check your answers' + And I should see "£5,432.11" + + Scenario: I can enter humanized monetary amounts like 1,000 for regular income + Given csrf is enabled + And I have completed a non-passported employed application for "client" with bank statements as far as the end of the means income section + Then I should be on the 'check_income_answers' page showing 'Check your answers' + + When I click Check Your Answers Change link for "Payments your client receives" + Then I should be on a page with title "Which of these payments does your client get?" + And I select "Financial help from friends or family" + And I fill "Friends or family" with "£1,112.33" + And I choose the "Monthly" frequency for "Friends or family" + + When I click 'Save and continue' + Then I should be on a page with title "Select payments your client receives in cash" + And I select "My client receives none of these payments in cash" + And I click 'Save and continue' + Then I should be on the 'check_income_answers' page showing 'Check your answers' + And I should see "£1,112.33" + + Scenario: I can enter humanized monetary amounts like 1,000 for regular outgoings and housing benefit + Given csrf is enabled + And I have completed a non-passported employed application for "client" with bank statements as far as the end of the means income section + Then I should be on the 'check_income_answers' page showing 'Check your answers' + + When I click Check Your Answers Change link for "Payments your client makes" + Then I should be on a page with title "Which of these payments does your client pay?" + And I select "Maintenance payments to a former partner" + And I fill "Maintenance out" with "£2,322.22" + And I choose the "Monthly" frequency for "Maintenance out" + + When I click 'Save and continue' + Then I should be on a page with title "Select payments your client pays in cash" + + When I select "None of the above" + And I click 'Save and continue' + Then I should be on a page with title "Does your client get Housing Benefit?" + + When I click 'Save and continue' + Then I should be on the 'check_income_answers' page showing 'Check your answers' + And I should see "£2,322.22" + + Scenario: I can enter humanized monetary amounts like 1,000 for mandatory capital disregards + Given the feature flag for means_test_review_a is enabled + And the MTR-A start date is in the past + And I have completed the income and capital sections of a non-passported application with bank statement uploads post-MTRA + When I am viewing the means capital check your answers page + + When I click link "Change Budgeting Advances" + And I fill "capital-disregard-amount-field" with "£1,654.33" + + When I click 'Save and continue' + Then I should be on the 'check_capital_answers' page showing 'Check your answers' + And I should see "£1,654.33" + + Scenario: I can enter humanized monetary amounts like 1,000 for discretionary capital disregards + Given the feature flag for means_test_review_a is enabled + And the MTR-A start date is in the past + And I have completed the income and capital sections of a non-passported application with bank statement uploads post-MTRA + When I am viewing the means capital check your answers page + + When I click link "Change Compensation, damages or ex-gratia payments for personal harm" + And I fill "capital-disregard-amount-field" with "£2,321.11" + + When I click 'Save and continue' + Then I should be on the 'check_capital_answers' page showing 'Check your answers' + And I should see "£2,321.11" diff --git a/features/step_definitions/bank_statement_upload_steps.rb b/features/step_definitions/bank_statement_upload_steps.rb index 35813b7938..255f91e56b 100644 --- a/features/step_definitions/bank_statement_upload_steps.rb +++ b/features/step_definitions/bank_statement_upload_steps.rb @@ -33,6 +33,7 @@ :legal_aid_application, :with_proceedings, :with_employed_applicant, + :with_maintenance_in_category, :with_rent_or_mortgage_regular_transaction, :with_housing_benefit_regular_transaction, :with_savings_amount, diff --git a/features/step_definitions/civil_journey_steps.rb b/features/step_definitions/civil_journey_steps.rb index c3f81f7eec..ce9c48fa7a 100644 --- a/features/step_definitions/civil_journey_steps.rb +++ b/features/step_definitions/civil_journey_steps.rb @@ -1139,6 +1139,14 @@ end end +Given("I click Check Your Answers Change link for partner {string}") do |question| + question_id = question.parameterize(separator: "_") + + within "#app-check-your-answers__partner__#{question_id}" do + click_on("Change") + end +end + Given("I click Check Your Answers Change link for {string}") do |question| question_id = question.parameterize(separator: "_") diff --git a/spec/forms/providers/means/capital_disregards/add_details_form_spec.rb b/spec/forms/providers/means/capital_disregards/add_details_form_spec.rb index a98a51c09c..047d81a546 100644 --- a/spec/forms/providers/means/capital_disregards/add_details_form_spec.rb +++ b/spec/forms/providers/means/capital_disregards/add_details_form_spec.rb @@ -32,9 +32,24 @@ end it "updates the capital_disregard" do - expect(application.capital_disregards.first.amount).to eq 123 - expect(application.capital_disregards.first.account_name).to eq "Barclays" - expect(application.capital_disregards.first.date_received).to eq Date.new(2024, 2, 1) + expect(application.capital_disregards.first) + .to have_attributes( + amount: 123, + account_name: "Barclays", + date_received: Date.new(2024, 2, 1), + ) + end + + context "with humanized monetary value" do + let(:amount) { "£1,244.55" } + + it "is valid" do + expect(form).to be_valid + end + + it "saves the monetary result" do + expect(application.capital_disregards.first.amount).to eq(1_244.55) + end end context "when amount is missing" do diff --git a/spec/forms/providers/means/housing_benefit_form_spec.rb b/spec/forms/providers/means/housing_benefit_form_spec.rb index 8d84abd0b1..31e90563dc 100644 --- a/spec/forms/providers/means/housing_benefit_form_spec.rb +++ b/spec/forms/providers/means/housing_benefit_form_spec.rb @@ -454,6 +454,21 @@ ) end + it "cleans the housing benefit regular transactions amount of humanized characters" do + legal_aid_application = create(:legal_aid_application, :with_applicant) + transaction_type = create(:transaction_type, :housing_benefit) + params = { + "transaction_type_ids" => transaction_type.id, + "housing_benefit_amount" => "£1,543.66", + "housing_benefit_frequency" => "weekly", + legal_aid_application:, + } + form = described_class.new(params) + form.save + + expect(legal_aid_application.regular_transactions.first).to have_attributes(amount: 1_543.66) + end + context "when a housing benefit regular transaction already exists" do it "does not create another legal aid application transaction type" do legal_aid_application = create(:legal_aid_application, :with_applicant) diff --git a/spec/forms/providers/means/regular_income_form_spec.rb b/spec/forms/providers/means/regular_income_form_spec.rb index 1ef05a68a0..de2098915e 100644 --- a/spec/forms/providers/means/regular_income_form_spec.rb +++ b/spec/forms/providers/means/regular_income_form_spec.rb @@ -597,6 +597,21 @@ outgoing_cash_transaction, ) end + + it "cleans the regular transaction amount of humanized characters" do + legal_aid_application = create(:legal_aid_application, :with_applicant) + pension = create(:transaction_type, :pension) + params = { + "transaction_type_ids" => ["", pension.id], + "pension_amount" => "£2,333.66", + "pension_frequency" => "monthly", + }.merge(legal_aid_application:) + + form = described_class.new(params) + form.save + + expect(legal_aid_application.regular_transactions.first).to have_attributes(amount: 2_333.66) + end end end end diff --git a/spec/forms/providers/means/regular_outgoings_form_spec.rb b/spec/forms/providers/means/regular_outgoings_form_spec.rb index 9a4ff28492..d690e1263e 100644 --- a/spec/forms/providers/means/regular_outgoings_form_spec.rb +++ b/spec/forms/providers/means/regular_outgoings_form_spec.rb @@ -470,6 +470,21 @@ .to contain_exactly([rent_or_mortgage.id, 250.50, "weekly"], [child_care.id, 100, "monthly"]) end + it "cleans the regular transaction amount of humanized characters" do + legal_aid_application = create(:legal_aid_application, :with_applicant) + rent_or_mortgage = create(:transaction_type, :rent_or_mortgage) + params = { + "transaction_type_ids" => ["", rent_or_mortgage.id], + "rent_or_mortgage_amount" => "£2,333.55", + "rent_or_mortgage_frequency" => "monthly", + }.merge(legal_aid_application:) + + form = described_class.new(params) + form.save + + expect(legal_aid_application.regular_transactions.first).to have_attributes(amount: 2_333.55) + end + it "destroys any existing housing benefit transactions if housing " \ "payments are not selected" do legal_aid_application = create(:legal_aid_application, :with_applicant) diff --git a/spec/forms/providers/means/state_benefit_form_spec.rb b/spec/forms/providers/means/state_benefit_form_spec.rb index 762c44f703..ac0b70fe19 100644 --- a/spec/forms/providers/means/state_benefit_form_spec.rb +++ b/spec/forms/providers/means/state_benefit_form_spec.rb @@ -62,6 +62,12 @@ it { expect(validation).to be false } end + context "when the amount is humanized monetary value" do + let(:amount) { "£1,000" } + + it { expect(validation).to be true } + end + context "when the frequency is not valid" do let(:frequency) { "NOT-A-FREQUENCY" } @@ -93,6 +99,18 @@ expect(legal_aid_application.regular_transactions.first.description).to eq "New State Benefit" end end + + context "with humanized monetary value" do + let(:model) { RegularTransaction.new(legal_aid_application:, transaction_type_id: transaction_type.id) } + let(:amount) { "£1,244.55" } + + it "saves the new transaction" do + params[:model] = model + save_form + + expect(legal_aid_application.regular_transactions.first.amount).to eq(1_244.55) + end + end end end end