From 3570f1e395844b9ba018e11d233d99b33fa5828b 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 and student finance 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. --- .../providers/means/state_benefit_form.rb | 4 + .../providers/regular_transaction_form.rb | 6 +- .../base_student_finance_form.rb | 4 + .../check_answers/_cash_payments.html.erb | 3 +- .../check_answers/_student_finance.html.erb | 2 +- config/locales/en/activemodel.yml | 12 +- .../check_your_means_answers.feature | 2 +- .../check_your_means_answers.feature | 2 +- .../enter_humanized_currency_amounts.feature | 115 ++++++++++++++++++ .../bank_statement_upload_steps.rb | 1 + .../step_definitions/civil_journey_steps.rb | 8 ++ .../means/housing_benefit_form_spec.rb | 23 ++++ .../means/regular_income_form_spec.rb | 21 ++++ .../means/regular_outgoings_form_spec.rb | 21 ++++ .../means/state_benefit_form_spec.rb | 18 +++ 15 files changed, 230 insertions(+), 12 deletions(-) create mode 100644 features/providers/regressions/enter_humanized_currency_amounts.feature 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..db20795ecd 100644 --- a/app/forms/providers/regular_transaction_form.rb +++ b/app/forms/providers/regular_transaction_form.rb @@ -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 d5b9b6ce64..8ec0204b6a 100644 --- a/app/forms/student_finances/base_student_finance_form.rb +++ b/app/forms/student_finances/base_student_finance_form.rb @@ -6,5 +6,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/config/locales/en/activemodel.yml b/config/locales/en/activemodel.yml index 822e84972a..11c0516ed5 100644 --- a/config/locales/en/activemodel.yml +++ b/config/locales/en/activemodel.yml @@ -156,9 +156,9 @@ en: inclusion: Select yes if your client receives student finance student_finance_amount: blank: Enter the total amount of student finance this academic year - too_many_decimals: &student_error Student loan amount must be an amount of money, like 10,000 - not_a_number: *student_error - greater_than_or_equal_to: *student_error + too_many_decimals: Student loan amount cannot include more than 2 decimal places + not_a_number: Student loan amount must be an amount of money, like 60,000 + greater_than_or_equal_to: Student loan amount must 0 or more applied_previously: inclusion: Select yes if your client has applied for civil legal aid before previous_reference: @@ -360,9 +360,9 @@ en: inclusion: Select yes if your partner receives student finance student_finance_amount: blank: Enter the total amount of student finance this academic year - too_many_decimals: &student_error Student loan amount must be an amount of money, like 10,000 - not_a_number: *student_error - greater_than_or_equal_to: *student_error + too_many_decimals: Student loan amount cannot include more than 2 decimal places + not_a_number: Student loan amount must be an amount of money, like 60,000 + greater_than_or_equal_to: Student loan amount must 0 or more base: none_selected: Select one or more employment types or None of the above if not employed none_and_another_option_selected: If you select 'None of the above', you cannot select any of the other options 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..c3f531c88b --- /dev/null +++ b/features/providers/regressions/enter_humanized_currency_amounts.feature @@ -0,0 +1,115 @@ +@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" 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 1a5b07802c..35cf212163 100644 --- a/features/step_definitions/civil_journey_steps.rb +++ b/features/step_definitions/civil_journey_steps.rb @@ -1135,6 +1135,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/housing_benefit_form_spec.rb b/spec/forms/providers/means/housing_benefit_form_spec.rb index 8d84abd0b1..8a0571f289 100644 --- a/spec/forms/providers/means/housing_benefit_form_spec.rb +++ b/spec/forms/providers/means/housing_benefit_form_spec.rb @@ -454,6 +454,29 @@ ) 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 + + regular_transactions = legal_aid_application.regular_transactions + expect(regular_transactions.count).to eq 1 + expect(regular_transactions.first).to have_attributes( + legal_aid_application:, + transaction_type:, + amount: 1_543.66, + frequency: "weekly", + ) + 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..e9eed9a218 100644 --- a/spec/forms/providers/means/regular_income_form_spec.rb +++ b/spec/forms/providers/means/regular_income_form_spec.rb @@ -597,6 +597,27 @@ 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( + legal_aid_application:, + transaction_type: pension, + amount: 2_333.66, + frequency: "monthly", + ) + 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..a3f5da6328 100644 --- a/spec/forms/providers/means/regular_outgoings_form_spec.rb +++ b/spec/forms/providers/means/regular_outgoings_form_spec.rb @@ -470,6 +470,27 @@ .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( + legal_aid_application:, + transaction_type: rent_or_mortgage, + amount: 2_333.55, + frequency: "monthly", + ) + 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..cdfcfd9219 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 be(1_244.55) + end + end end end end