Skip to content

Commit

Permalink
AP-5496: Standardise currency handling accross the app
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jsugarman committed Nov 12, 2024
1 parent d1f9741 commit 3570f1e
Show file tree
Hide file tree
Showing 15 changed files with 230 additions and 12 deletions.
4 changes: 4 additions & 0 deletions app/forms/providers/means/state_benefit_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion app/forms/providers/regular_transaction_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand All @@ -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}"
Expand Down
4 changes: 4 additions & 0 deletions app/forms/student_finances/base_student_finance_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
3 changes: 1 addition & 2 deletions app/views/shared/check_answers/_cash_payments.html.erb
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
<% read_only = false unless local_assigns.key?(:read_only) %>

<div class="govuk-grid-row">
<div class="govuk-grid-row" id="app-check-your-answers__<%= individual.to_s.downcase %>__cash_<%= type.to_s.downcase %>">
<div class="govuk-grid-column-two-thirds">
<h3 class="govuk-heading-m"><%= t(".#{type}_heading", individual_with_determiner:) %></h3>
</div>
Expand Down
2 changes: 1 addition & 1 deletion app/views/shared/check_answers/_student_finance.html.erb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<div class="govuk-grid-row" id="app-check-your-answers__<%= individual.class.to_s.downcase %>_student_finance">
<div class="govuk-grid-row" id="app-check-your-answers__<%= individual.class.to_s.downcase %>__student_finance">
<div class="govuk-grid-column-two-thirds">
<h3 class="govuk-heading-m"><%= t(".heading") %></h3>
</div>
Expand Down
12 changes: 6 additions & 6 deletions config/locales/en/activemodel.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"

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

Expand Down
Original file line number Diff line number Diff line change
@@ -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"
1 change: 1 addition & 0 deletions features/step_definitions/bank_statement_upload_steps.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
8 changes: 8 additions & 0 deletions features/step_definitions/civil_journey_steps.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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: "_")

Expand Down
23 changes: 23 additions & 0 deletions spec/forms/providers/means/housing_benefit_form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
21 changes: 21 additions & 0 deletions spec/forms/providers/means/regular_income_form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 21 additions & 0 deletions spec/forms/providers/means/regular_outgoings_form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
18 changes: 18 additions & 0 deletions spec/forms/providers/means/state_benefit_form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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" }

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

0 comments on commit 3570f1e

Please sign in to comment.