Skip to content

Commit

Permalink
AP-3563: Fix error message and form value rendering
Browse files Browse the repository at this point in the history
This fixes an underlying bug that causes the behaviour described
in the bug ticket AP-3563. Namely, that while the amount for any
cash income or outgoing type is validated it is not done in the way
common to other pages or forms. This has resulted in a difference
whereby comma separated monetayr values, `1,000`, are invalid but
where being rendered as a value when the form errors and rerenders.

This has been corrected to emulate the functionality seen in
form objects with monetary amount fields.

1. It now "cleans" the monetary amount params to remove `£` and `,`.

2. When rendering the form the field value is no longer rendered as a
"number to currency" value with no unit - which was rendering commas
for numeric values over 3 digits long.
  • Loading branch information
jsugarman committed Nov 14, 2024
1 parent 883ea0d commit c6aa0de
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 2 deletions.
13 changes: 12 additions & 1 deletion app/models/base_aggregated_cash_transaction.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,22 @@ def transaction_date(month_number)
end

def update_cash_attributes(params)
params.each do |key, value|
cleaned_params = clean_attributes(params)
cleaned_params.each do |key, value|
__send__(:"#{key}=", value)
end
end

def clean_attributes(params)
params.each.with_object({}) do |(k, v), new_hash|
new_hash[k] = cash_transaction_amount_field?(k) ? v.to_s.tr("£,", "") : v
end
end

def cash_transaction_amount_field?(param_name)
self.class.cash_transaction_categories.map(&:to_s).any? { |category| param_name.start_with?(category) }
end

def save_cash_transaction_records
self.class.cash_transaction_categories.each do |category|
CashTransaction.where(
Expand Down
2 changes: 1 addition & 1 deletion app/views/shared/partials/_revealing_checkbox.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
<% end %>
<%= form.govuk_text_field "#{name}#{number}",
label: { text: model.period(number) },
value: gds_number_to_currency(model.__send__(:"#{name}#{number}"), unit: ""),
value: model.__send__(:"#{name}#{number}"),
prefix_text: defined?(input_prefix) ? input_prefix : nil,
form_group: { classes: error_class },
width: "one-third" %>
Expand Down
9 changes: 9 additions & 0 deletions spec/models/aggregated_cash_income_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,15 @@
end
end

context "with valid comma separate monetary params" do
let(:params) { valid_params.merge(benefits1: "1,222,222.33") }

it "creates the expected cash income records" do
expect { call_update }.to change(CashTransaction, :count).by(6)
expect(CashTransaction.pluck(:amount)).to include(1_222_222.33)
end
end

context "with invalid params" do
context "and non-numeric values" do
let(:params) { non_numeric_params }
Expand Down
9 changes: 9 additions & 0 deletions spec/models/aggregated_cash_outgoings_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,15 @@ def error_msg(name, month)
end
end

context "with valid comma separate monetary params" do
let(:params) { valid_params.merge(rent_or_mortgage1: "1,222,222.33") }

it "creates the expected cash income records" do
expect { call_update }.to change(CashTransaction, :count).by(6)
expect(CashTransaction.pluck(:amount)).to include(1_222_222.33)
end
end

context "with invalid params" do
context "and non-numeric values" do
let(:params) { non_numeric_params }
Expand Down

0 comments on commit c6aa0de

Please sign in to comment.