mirror of
https://github.com/we-promise/sure.git
synced 2026-09-06 23:24:21 +00:00
* 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>
149 lines
6.2 KiB
Ruby
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
|