Files
sure/test/models/assistant/function/create_goal_test.rb
T
b6029c1e28 feat(goals): show what each account still has room to earmark (#3166)
* feat(goals): show what each account still has room to earmark

`Account#free_to_earmark` has existed, unused, since earmarks shipped —
its own comment said the UI was a follow-up. This is that follow-up, and
the wording is the substance of it.

It does not say "over-allocated". `free_to_earmark` is negative for as
long as the saving is unfinished, which is the normal condition of anyone
with goals in progress: a 6,000 account backing two goals of 5,000 gives
−4,000 and is a perfectly correct setup. A warning phrased as a fault
would fire permanently and teach people to ignore it. The message states
the consequence instead — the goals come to X for a balance of Y, so they
progress pro rata — and is never styled as an error.

The trap is the goal being edited. `goal_earmarked_total` counts every
goal including that one, so reopening a goal that earmarks 5,000 on a
6,000 account shows 1,000 of headroom, and re-entering the same 5,000
trips a message about a setup the user has not touched.
`earmarked_by_other_goals` excludes it, and only when it is persisted —
a goal being created has nothing to exclude.

The pool is read once per render and passed down, never per account: the
form lists every fundable account the user can see. A test counts the
query and fails at two.

The Stimulus controller is its own, with 3 targets. goal_form_controller
is at 10 against the 7 the project guidelines suggest, needs none of this
state, and is untouched.

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

* fix(goals): read the typed amount strictly, and format it in the app's locale

Addresses review feedback on #3166.

`Number.parseFloat` accepts prefixes, so "500abc" became 500, and the bare
comma-to-dot swap turned a thousands-separated "1,500" into 1.5. Either way the
preview described an amount the user had not typed — and the second case is a
habit from another locale, not a typo, so it would have gone unnoticed. The
value now has to match a complete number before anything is computed.

`Intl.NumberFormat(undefined, ...)` let the BROWSER pick the locale, so a
French user on an English-locale browser read separators and symbol placement
matching nothing else on the page. The amounts cannot be formatted server-side
— they change with every keystroke — so the server passes `I18n.locale` and the
client applies it. That puts the decision where the rest of the app's
formatting already lives.

bin/rails test: 6954 runs, 0 failures. RuboCop, erb_lint and biome clean.

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

* fix(goals): let the assistant create a second goal on a claimed account

Review on #3166. The function always built whole-account links and had no
way to express an earmark, so once exclusivity landed, asking for a second
goal on an account another goal already claimed came back as a bare
`validation_failed` — while the account list still advertised the account
as available. A common request became an unexplained refusal.

Three changes, and the list is the important one: it now says what is left
on each account and which are claimed in full, because the assistant
reasons from that list and had no way to know otherwise.

`earmarks` is an optional map of account name to amount, so the assistant
can reserve a slice rather than the whole balance. Accounts left out keep
the previous behaviour and take whatever is spare.

The refusal is named before the save — `account_claimed_in_full`, with the
account names — so the assistant gets a reason it can act on and ask about,
rather than a validation message it can only relay. Checked after the
currency check, which is the more fundamental of the two.

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

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

The merge of `main` into this branch landed #3176's tests between
`count_pool_queries` and the helpers below it, inside the `private`
section and at the wrong indentation. `ci / lint` has been failing on
`Layout/IndentationConsistency` since.

They still ran — `test` is a class method, so `private` does not hide them
— which is why the unit job stayed green while lint went red.

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>
2026-08-26 21:12:06 +02:00

149 lines
6.2 KiB
Ruby

require "test_helper"
class Assistant::Function::CreateGoalTest < ActiveSupport::TestCase
setup do
@user = users(:family_admin)
@family = @user.family
@depository = accounts(:depository)
@fn = Assistant::Function::CreateGoal.new(@user)
end
test "to_definition returns valid JSON shape" do
definition = @fn.to_definition
assert_equal "create_goal", definition[:name]
assert_kind_of String, definition[:description]
assert_equal "object", definition[:params_schema][:type]
assert_includes definition[:params_schema][:required], "name"
assert_includes definition[:params_schema][:required], "target_amount"
assert_includes definition[:params_schema][:required], "linked_account_names"
end
test "creates a goal with linked accounts" do
# A fresh account, not one the goal fixtures already claim in full:
# GoalAccount refuses a second whole-balance link on a contested account,
# and this function has no way to pass an earmark.
unclaimed = Account.create!(
family: @family, accountable: Depository.new,
name: "Vacation Savings", currency: "USD", balance: 2_000
)
assert_difference -> { Goal.count } => 1,
-> { GoalAccount.count } => 1 do
result = @fn.call(
"name" => "Vacation",
"target_amount" => 1500,
"target_date" => 3.months.from_now.to_date.iso8601,
"linked_account_names" => [ unclaimed.name ]
)
assert result[:success]
assert_match(/Vacation/, result[:message])
assert result[:url].present?
assert_equal "USD", result[:currency]
end
end
test "soft error when name is missing" do
result = @fn.call("target_amount" => 100, "linked_account_names" => [ @depository.name ])
assert_equal false, result[:success]
assert_equal "name_required", result[:error]
end
test "soft error when target_amount is zero" do
result = @fn.call("name" => "X", "target_amount" => 0, "linked_account_names" => [ @depository.name ])
assert_equal false, result[:success]
assert_equal "target_amount_invalid", result[:error]
end
test "soft error when no linked accounts" do
result = @fn.call("name" => "X", "target_amount" => 100, "linked_account_names" => [])
assert_equal false, result[:success]
assert_equal "no_linked_accounts", result[:error]
assert_kind_of Array, result[:available_accounts]
assert(result[:available_accounts].all? { |a| a.is_a?(Hash) && a.key?(:name) })
end
test "soft error when account name doesn't match" do
result = @fn.call("name" => "X", "target_amount" => 100, "linked_account_names" => [ "Nonexistent Account" ])
assert_equal false, result[:success]
assert_equal "unknown_accounts", result[:error]
assert_includes result[:unknown_names], "Nonexistent Account"
end
test "soft error when currencies differ across linked accounts" do
eur = Account.create!(family: @family, accountable: Depository.new, name: "EUR Account", currency: "EUR", balance: 100)
result = @fn.call(
"name" => "Mixed",
"target_amount" => 100,
"linked_account_names" => [ @depository.name, eur.name ]
)
assert_equal false, result[:success]
assert_equal "currency_mismatch", result[:error]
end
test "scopes to the user's family" do
other_family = Family.create!(name: "Other", currency: "USD", locale: "en", country: "US", timezone: "UTC")
Account.create!(family: other_family, accountable: Depository.new, name: "Foreign Checking", currency: "USD", balance: 100)
result = @fn.call(
"name" => "X",
"target_amount" => 100,
"linked_account_names" => [ "Foreign Checking" ]
)
assert_equal false, result[:success]
assert_equal "unknown_accounts", result[:error]
end
# --- Review follow-up (#3166) ---
# The function always built whole-account links, so once exclusivity landed a
# second goal on the same account failed with a generic validation error —
# while the account list still advertised it as available. A common request
# ("save for a holiday too") became an unexplained refusal.
test "an account another goal claims in full is refused with a reason, not a validation failure" do
account = Account.create!(family: @family, accountable: Depository.new,
name: "Claimed Pot", currency: "USD", balance: 5_000)
@family.goals.create!(name: "Precaution", target_amount: 5_000, currency: "USD") do |g|
g.goal_accounts.build(account: account)
end
result = @fn.call("name" => "Holiday", "target_amount" => 1_000,
"linked_account_names" => [ account.name ])
assert_equal false, result[:success]
assert_equal "account_claimed_in_full", result[:error]
assert_includes result[:claimed_account_names], account.name
end
test "the same account is accepted once an earmark says how much to take" do
account = Account.create!(family: @family, accountable: Depository.new,
name: "Claimed Pot", currency: "USD", balance: 5_000)
@family.goals.create!(name: "Precaution", target_amount: 5_000, currency: "USD") do |g|
g.goal_accounts.build(account: account)
end
result = @fn.call("name" => "Holiday", "target_amount" => 1_000,
"linked_account_names" => [ account.name ],
"earmarks" => { account.name => 1_000 })
assert_equal true, result[:success]
assert_equal 1_000, Goal.find(result[:goal_id]).goal_accounts.first.allocated_amount.to_d
end
# The list is what the assistant reasons from; without this it had no way to
# know an account could not be taken whole.
test "the account list says what is left and what is already claimed" do
account = Account.create!(family: @family, accountable: Depository.new,
name: "Claimed Pot", currency: "USD", balance: 5_000)
@family.goals.create!(name: "Precaution", target_amount: 5_000, currency: "USD") do |g|
g.goal_accounts.build(account: account)
end
result = @fn.call("name" => "X", "target_amount" => 100, "linked_account_names" => [])
listed = result[:available_accounts].find { |a| a[:name] == account.name }
assert listed[:claimed_in_full]
assert listed.key?(:free_to_earmark)
end
end