feat(goals): call a reserve's amount what the rest of the app calls it (#3230)

* fix(goals): let a months-of-expenses reserve be created at all

The mode could not be used from the UI. The form makes the amount field
read-only in months mode — correctly, since the figure is derived — so the
form submits it empty. `target_amount` is required and positive, and
validations run before every save callback, so the derivation that fills it
never got the chance. Creation came back 422 with "can't be blank" on a
field the user is not allowed to type in.

Reproduced through the controller before changing anything: response 422,
no goal created.

Every existing test set `target_amount` explicitly, which is why the model
looked healthy — the gap was entirely on the path a user actually takes.

The derivation moves to `before_validation`, where a derived value belongs:
it is computed, then validated like any other. The dirty-state predicates
change with it, since `will_save_change_to_*` describes a save that has not
been decided on yet at that point.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016GTNba5qE5NwzaHzbp27ye

* test(goals): move the new tests out of the private section

Review flagged them as never running. They do — Rails' `test` macro goes
through `define_method` from a class method, which defines a public method
whatever the surrounding visibility, and `-n` confirms Minitest picks both
up. Verified before touching anything: 2 runs, 7 assertions.

Moved anyway. My insertion targeted the file's last `private` rather than
its first, so they landed among the helper methods, where they read as a
mistake whether or not they behave like one — three separate reviewers have
now stopped on it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016GTNba5qE5NwzaHzbp27ye

* test(goals): put the page tests where nobody has to check they run

Review on #3229. The four page tests sat between `private` and a later
`public`, and every reader so far has stopped to work out whether they run.
They do — Rails' `test` macro calls `define_method` from a class method, and a
method defined that way is public whatever the surrounding visibility — but a
test whose behaviour has to be reasoned about is a test nobody trusts. They
move above the first `private`, where the question does not come up.

The second `private` goes with them: everything between it and the first was
already private, so it did nothing.

The fixed-amount test also gained the assertion it was missing. It named the
guard it was protecting and then checked only the status, so a 422 arriving
for any other reason would have kept it green. It now asserts the error the
form actually puts in front of the user; flipping that paragraph's condition
makes it fail, which is the point.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016GTNba5qE5NwzaHzbp27ye

* feat(goals): settle on target balance, the term this kind of app uses

A reserve holds a balance rather than reaching an amount, and the page had
three words for that one idea: "floor" twice, "level" seven times, and a
field labelled "Target amount". The mode selector said "How the floor is
set" three lines above a field called "Target amount".

They are all replaced by **target balance** / **solde cible**.

That is the term this kind of application uses, and it keeps the noun the
rest of the page already leans on — "target" appears 55 times here. My
first pass invented "Level to hold", which was internally tidy and standard
nowhere: it fought 55 uses of a word that was not actually wrong. A target
need not be a finish line; a target balance is one you hold.

In months mode the balance also stops pretending to be a field. It is
worked out from spending, so it is shown as a result with a line saying
where it comes from — "Worked out when you save" on a goal that has none
yet, and the balance it currently holds when editing one. That removes the
`readOnly` toggle, which existed only to stop people typing into something
that should not have been an input.

Three smaller corrections while in the file:

- `exceeds_earmark` had the actor backwards in both languages. The account
  does not earmark; the goal earmarks on the account.
- The French `not_active` was a comma splice, where the file already uses a
  colon for that construction.
- "se recomplète" is not standard French; a reserve "se reconstitue", and
  "ce qui lui manque" reads better than "son manque".

A test asserts neither locale still says floor or niveau, so the three
vocabularies cannot quietly come back.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016GTNba5qE5NwzaHzbp27ye

* fix(goals): stop a hidden field blocking the reserve it belongs to

Replacing the read-only amount input with a hidden one left it `required`,
and a required input is still validated by the browser while `display: none`.
Submitting a months-based reserve was refused over a field the user could not
see, and could not have filled in either — the reserve became impossible to
create, which is the very thing the previous change set out to fix.

Disabled rather than hidden, so it is barred from validation and its value
stays out of the params, letting the derived figure land.

The field also has to come back when there is nothing to derive from: with no
spending history the model keeps whatever was typed, so the typed amount is
then the only way to set a target at all. The form now asks the family that
question up front and keeps the field where the answer is no.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016GTNba5qE5NwzaHzbp27ye

* fix(goals): stop the label swap deleting the required marker

Review on #3230. `_money_field` puts the required-field asterisk inside the
label, in a span of its own. Swapping the wording with `textContent = label`
replaces every child of that label, so the asterisk went with it — and
`refresh()` runs on connect, so this fired on every goal form, one-off and
fixed reserve included, not only the derived-months case this PR is about.
Nothing put it back for the life of the page.

Only the wording changes now: the label's text node is rewritten and the span
left alone.

A system test covers it, because nothing short of a browser can. It fails on
the old code at the first assertion, before anything is clicked, which is
where the bug actually landed.

Also from review: the months derivation asked for the median twice, building
an IncomeStatement each time. It reads it once now — the method stays
un-memoized, which is what the refresh job needs.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016GTNba5qE5NwzaHzbp27ye

---------

Signed-off-by: Juan José Mata <juanjo.mata@gmail.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Juan José Mata <juanjo.mata@gmail.com>
This commit is contained in:
buzzromain
2026-08-28 07:32:30 +02:00
committed by GitHub
co-authored by Claude Opus 5 Juan José Mata
parent 390ed50616
commit 3a4e92c6f8
8 changed files with 231 additions and 34 deletions
+51
View File
@@ -783,6 +783,57 @@ class GoalsControllerTest < ActionDispatch::IntegrationTest
assert_no_match(/already used|déjà utilisés/, label)
end
# A reserve holds a balance rather than reaching an amount. "Target balance"
# is what this kind of app calls that, and it keeps the noun the rest of the
# page already uses 55 times — rather than inventing a "level" or a "floor"
# that nothing else in the domain says.
test "the form carries both the amount and the balance wording" do
unclaimed_account("Vocab Pot")
get new_goal_url
assert_response :success
assert_includes response.body, I18n.t("goals.form.fields.target_amount")
assert_includes response.body, I18n.t("goals.form.fields.target_balance")
end
# In months mode the balance is worked out, not typed. A read-only input
# still reads as something to fill in, beside the months that are the real
# question, so the figure is presented as a result instead.
test "the form carries the derived balance as a result, not a field" do
unclaimed_account("Derived Pot")
get new_goal_url
assert_response :success
assert_select "[data-goal-kind-target=amountDerived]", 1
assert_includes response.body, I18n.t("goals.form.fields.target_balance_pending")
end
test "editing a months reserve shows the balance it currently holds" do
account = unclaimed_account("Existing Pot")
IncomeStatement.any_instance.stubs(:median_expense).returns(500)
goal = @user.family.goals.create!(
name: "Precaution", target_amount: 3_000, currency: "USD", kind: "maintained",
target_mode: "months_of_expenses", target_months: 6
) { |g| g.goal_accounts.build(account: account, allocated_amount: 3_000) }
get edit_goal_url(goal)
assert_response :success
assert_includes response.body, goal.target_amount_money.format(precision: 0)
end
# One vocabulary, not three. "level" and "floor" said the same thing as
# "target balance" in different words, on the same page.
test "the goals copy settles on one word for a reserve's balance" do
%i[en fr].each do |locale|
copy = YAML.load_file(Rails.root.join("config/locales/views/goals/#{locale}.yml")).to_s
assert_no_match(/\bfloor\b|\bniveau\b/i, copy, "#{locale} still mixes vocabularies")
end
end
private
def spent_goal_for_display