diff --git a/app/javascript/controllers/goal_kind_controller.js b/app/javascript/controllers/goal_kind_controller.js index a68234498..4975e14ad 100644 --- a/app/javascript/controllers/goal_kind_controller.js +++ b/app/javascript/controllers/goal_kind_controller.js @@ -1,10 +1,16 @@ import { Controller } from "@hotwired/stimulus" -// A maintained reserve has no deadline: it is a level to hold, not something -// due on a date. Hiding the target-date field keeps a stale value from being +// A maintained reserve has no deadline: it is a target balance to hold, not +// something due on a date. Hiding the target-date field keeps a stale value from being // submitted and driving a pace the goal does not have. export default class extends Controller { - static targets = ["radio", "dateField", "modeField", "modeSelect", "monthsField", "amountField"] + static targets = ["radio", "dateField", "modeField", "modeSelect", "monthsField", "amountField", "amountDerived"] + + static values = { + amountLabel: String, + balanceLabel: String, + derivable: Boolean, + } connect() { this.refresh() @@ -26,16 +32,42 @@ export default class extends Controller { this.monthsFieldTargets.forEach((field) => field.classList.toggle("hidden", !months)) if (!months) this.#clearInputs(this.monthsFieldTargets) - // In months mode the floor is derived from the family's spending, not - // chosen. Shown, because it is the figure the user is saving against, but - // not editable — the model overwrites a typed one anyway, and a field that - // silently discards what you put in it is worse than one you cannot type - // into. + // Three states. A one-off saves toward a target amount; a fixed reserve + // holds a target balance you type; a months-based reserve holds one worked + // out from spending, so the input gives way to the figure itself — a + // greyed-out input still reads as something to fill in, beside the months + // that are the actual question. + // + // Only where there IS spending to work one out from, though. With none the + // model keeps whatever was typed, so the field has to stay: it is then the + // only way to give the reserve a target at all. + const derived = months && this.derivableValue + this.amountFieldTargets.forEach((field) => field.classList.toggle("hidden", derived)) + this.amountDerivedTargets.forEach((field) => field.classList.toggle("hidden", !derived)) + + // Disabled, not merely hidden. A `required` input is still validated by + // the browser while `display: none`, which then blocks a submit it cannot + // show the error on — the reserve became impossible to create. Disabling + // also keeps the typed figure out of the params, so what lands is the + // figure the model derived. this.amountFieldTargets.forEach((field) => { - field.querySelectorAll("input").forEach((input) => { - input.readOnly = months - input.classList.toggle("opacity-60", months) - }) + field.querySelectorAll("input").forEach((input) => { input.disabled = derived }) + }) + + // Only the wording changes. Assigning `textContent` would replace every + // child of the label, and the label carries the required-field asterisk in + // a span of its own — wiped on the first `refresh()`, which runs on connect + // for every goal form, with nothing to put it back. + const label = maintained ? this.balanceLabelValue : this.amountLabelValue + this.amountFieldTargets.forEach((field) => { + const el = field.querySelector(".form-field__label") + if (!el || !label) return + + const wording = [...el.childNodes].find( + (node) => node.nodeType === Node.TEXT_NODE && node.textContent.trim(), + ) + if (wording) wording.textContent = label + else el.prepend(document.createTextNode(label)) }) } diff --git a/app/models/goal.rb b/app/models/goal.rb index fb3d2f8a9..fda5f954a 100644 --- a/app/models/goal.rb +++ b/app/models/goal.rb @@ -457,8 +457,22 @@ class Goal < ApplicationRecord computed end + # Whether a months-based target could be worked out AT ALL right now. The + # form has to know before the user has picked anything, so this deliberately + # ignores the goal's own kind, mode and months: it answers "is there spending + # history to derive from", nothing else. With none, the typed amount is the + # only way to set a target, so the form must keep offering the field. + def months_target_derivable? + return false if family.nil? || currency.blank? + + median = median_monthly_expense + return false unless median.positive? + + convert_to_goal_currency(median).to_d.positive? + end + private - # The floor this reserve should hold, or nil when it cannot be computed. + # The family's median monthly spend, in FAMILY currency. # # ⚠️ The account scope is passed EXPLICITLY, and that is the whole point # of this method. IncomeStatement's constructor does `user || Current.user` @@ -468,11 +482,19 @@ class Goal < ApplicationRecord # whole family: derived from a viewer's slice of the accounts it would # change depending on who last triggered it. The rollover chain hit # exactly this and had to be pinned the same way. + # Deliberately not memoized. The refresh job derives again on an instance + # it has already saved through, and a median frozen on first read would + # hand it back the figure it started with. + def median_monthly_expense + IncomeStatement.new(family, accounts: family.accounts.visible.included_in_reports) + .median_expense(interval: "month").to_d + end + + # The balance this reserve should hold, or nil when it cannot be computed. def months_of_expenses_amount return nil unless maintained? && months_of_expenses_target? && target_months.to_i.positive? - statement = IncomeStatement.new(family, accounts: family.accounts.visible.included_in_reports) - median = statement.median_expense(interval: "month").to_d + median = median_monthly_expense return nil unless median.positive? computed = (median * target_months).round(2) diff --git a/app/views/goals/_form.html.erb b/app/views/goals/_form.html.erb index 4b832d6ab..1cdd3949f 100644 --- a/app/views/goals/_form.html.erb +++ b/app/views/goals/_form.html.erb @@ -8,7 +8,10 @@ data-goal-form-currency-value="<%= Current.family.primary_currency_code %>" data-goal-form-require-account-value="<%= !goal.persisted? %>" data-goal-form-suggested-with-date-value="<%= t("goals.form.suggested_with_date") %>" - data-goal-form-suggested-no-date-value="<%= t("goals.form.suggested_no_date") %>"> + data-goal-form-suggested-no-date-value="<%= t("goals.form.suggested_no_date") %>" + data-goal-kind-amount-label-value="<%= t("goals.form.fields.target_amount") %>" + data-goal-kind-balance-label-value="<%= t("goals.form.fields.target_balance") %>" + data-goal-kind-derivable-value="<%= goal.months_target_derivable? %>"> <%= styled_form_with model: goal, url: goal.persisted? ? goal_path(goal) : goals_path, method: goal.persisted? ? :patch : :post, @@ -76,6 +79,23 @@ amount_data: { goal_form_target: "amountInput", action: "input->goal-form#suggestedChanged" } %>

mt-1.5 text-xs text-destructive" data-goal-form-target="amountError"><%= t("goals.form.errors.amount_required") %>

+ + <%# In months mode the balance is worked out, not typed. A read-only + input still reads as something to fill in, sitting beside the months + that are the actual question — so it is shown as what it is. Only + when there is spending to work it out from, though: with none, the + controller leaves the field above in place and this stays hidden. %> +
<%= f.date_field :target_date, label: t("goals.form.fields.target_date"), diff --git a/config/locales/views/goals/en.yml b/config/locales/views/goals/en.yml index de0543d71..5a709b14d 100644 --- a/config/locales/views/goals/en.yml +++ b/config/locales/views/goals/en.yml @@ -112,7 +112,7 @@ en: account_required: This goal is funded by several accounts — say which one the money came out of. account_not_linked: That account does not fund this goal. not_active: This goal is no longer active, so nothing can be recorded against it. - exceeds_earmark: That is more than this account has earmarked for the goal. + exceeds_earmark: That is more than the goal has earmarked on this account. transaction_already_claimed: That outflow has already been attributed. transaction_not_found: That outflow is no longer available to attribute. no_linked_account: This goal has no funding account left. @@ -131,8 +131,8 @@ en: reopen: Reopen goal delete: Delete reserve_shortfall: - heading: Reserve below its level - body: "You are holding %{saved} of %{target}. Add %{missing} to bring it back to its floor." + heading: Reserve below its target balance + body: "You are holding %{saved} of %{target}. Add %{missing} to bring it back to its target balance." record_pledge_cta: Record pledge pledge_just_transferred: Log a transfer you made pledge_just_saved: Log money you set aside @@ -146,7 +146,7 @@ en: funding_last_30d: last 30d funding_last_90d: last 90d status_callout: - depleted: "%{amount} short of its level — top it up to bring the reserve back" + depleted: "%{amount} below its target balance — top it up to bring the reserve back" behind: "save %{amount}/mo more to catch up" behind_covered: "pending pledges close the gap" on_track: "reaches goal around %{date}" @@ -182,7 +182,7 @@ en: legend_saved: Saved legend_projection: Projection legend_required: Required - reserve: "A reserve holds a level, so there is no finish line to project." + reserve: "A reserve holds a target balance, so there is no finish line to project." reached: You've hit the target. No projection needed. no_target_date: No target date set. Set one to project a finish line. no_pace: No deposits yet. Add money to a linked account to start a projection. @@ -217,7 +217,7 @@ en: restore_cta: Restore goal celebration: heading_reserve: Reserve intact - body_reserve: "Your reserve is at its full level, %{saved} of %{target}. Nothing to do — it stays earmarked." + body_reserve: "Your reserve is at its target balance, %{saved} of %{target}. Nothing to do — it stays earmarked." heading: Goal reached. Nice work. body_reached: "You've hit your target at %{saved} of %{target}. It is not closed yet — the money is still earmarked for it." close_cta: Close this goal @@ -302,7 +302,7 @@ en: hint: Save toward something, then close it when you spend it. label: One-off goal maintained: - hint: A level to keep, like an emergency fund. Never closed. + hint: A balance to keep, like an emergency fund. Never closed. label: Reserve to maintain create: Create goal save: Save changes @@ -317,12 +317,15 @@ en: prorata: "Your goals on this account come to {total} for a balance of {balance} — they will progress pro rata." whole_balance: This account will fund whatever is left after the other earmarks. fields: - target_mode: How the floor is set + target_mode: How the target balance is set target_months: Months of expenses target_months_hint: Recomputed on the 1st of each month from your median monthly spending. name: Name name_placeholder: Emergency fund, House down payment… target_amount: Target amount + target_balance: "Target balance" + target_balance_pending: "Worked out when you save" + target_balance_hint: "From your median monthly spending over the months above. Refreshed each month." target_date: Target date color: Color notes: Notes (optional) diff --git a/config/locales/views/goals/fr.yml b/config/locales/views/goals/fr.yml index 2b338813e..f0e41b46a 100644 --- a/config/locales/views/goals/fr.yml +++ b/config/locales/views/goals/fr.yml @@ -35,7 +35,7 @@ fr: months_of_expenses: Un nombre de mois de dépenses kinds: maintained: - hint: Un niveau à tenir, comme une épargne de précaution. Jamais clôturée. + hint: Un solde à maintenir, comme une épargne de précaution. Jamais clôturée. label: Réserve à maintenir one_off: hint: Épargner pour quelque chose, puis clôturer une fois la dépense faite. @@ -50,7 +50,7 @@ fr: prorata: "Vos objectifs sur ce compte totalisent {total} pour un solde de {balance} : ils progresseront au prorata." whole_balance: Ce compte financera le solde restant après les autres affectations. fields: - target_mode: Comment le seuil est fixé + target_mode: Comment le solde cible est fixé target_months: Mois de dépenses target_months_hint: Recalculé le 1er de chaque mois à partir de vos dépenses mensuelles médianes. color: Couleur @@ -63,6 +63,9 @@ fr: notes: Notes (facultatif) notes_placeholder: Un rappel pour plus tard… target_amount: Montant cible + target_balance: "Solde cible" + target_balance_pending: "Calculé à l'enregistrement" + target_balance_hint: "D'après vos dépenses mensuelles médianes sur le nombre de mois ci-dessus. Recalculé chaque mois." target_date: Date cible whole_balance: Solde total save: Enregistrer les modifications @@ -189,12 +192,12 @@ fr: success: "%{amount} enregistrés comme utilisés." errors: non_positive: Indiquez un montant supérieur à zéro. - maintained: Une réserve se ponctionne et se recomplète, elle ne se dépense pas — son manque apparaît déjà sur l'objectif. + maintained: "Une réserve se ponctionne et se reconstitue, elle ne se dépense pas — ce qui lui manque apparaît déjà sur l'objectif." exceeds_target: C'est plus que ce que cet objectif visait à épargner. account_required: Cet objectif est financé par plusieurs comptes — indiquez celui d'où vient l'argent. account_not_linked: Ce compte ne finance pas cet objectif. - not_active: Cet objectif n'est plus actif, rien ne peut y être enregistré. - exceeds_earmark: C'est plus que ce que ce compte a affecté à l'objectif. + not_active: "Cet objectif n'est plus actif : rien ne peut y être enregistré." + exceeds_earmark: "C'est plus que ce que l'objectif a affecté sur ce compte." transaction_already_claimed: Cette sortie a déjà été attribuée. transaction_not_found: Cette sortie n'est plus disponible. no_linked_account: Cet objectif n'a plus de compte de financement. @@ -213,7 +216,7 @@ fr: body: Rythme actuel %{avg}/mois · requis %{required}/mois pour atteindre votre objectif. title: Épargnez %{amount}/mois de plus pour rattraper le retard celebration: - body_reserve: "Votre réserve a atteint son niveau cible, %{saved} sur %{target}. Rien à faire — elle reste affectée." + body_reserve: "Votre réserve est à son solde cible, %{saved} sur %{target}. Rien à faire — elle reste affectée." heading_reserve: Réserve constituée body_reached: "Vous avez atteint votre cible, à %{saved} sur %{target}. Il n'est pas encore clôturé — l'argent lui reste affecté." close_cta: Clôturer cet objectif @@ -290,15 +293,15 @@ fr: no_pace: Aucun dépôt pour le moment. Ajoutez de l'argent sur un compte lié pour démarrer la projection. no_target_date: Aucune date cible définie. Définissez-en une pour projeter la fin. on_track_html: Au rythme actuel, vous atteindrez cet objectif vers le %{date}. - reserve: "Une réserve maintient un niveau : il n'y a pas d'échéance à projeter." + reserve: "Une réserve maintient un solde cible : il n'y a pas d'échéance à projeter." reached: Vous avez atteint l'objectif. Aucune projection nécessaire. today_marker: Aujourd'hui tooltip_projected: 'Projeté : %{amount}' tooltip_saved: 'Épargné : %{amount}' tooltip_target_relation: "%{percent}% de la cible de %{target}" reserve_shortfall: - heading: Réserve sous son niveau - body: "Vous détenez %{saved} sur %{target}. Complétez de %{missing} pour la ramener à son niveau." + heading: Réserve sous son solde cible + body: "Vous détenez %{saved} sur %{target}. Complétez de %{missing} pour la ramener à son solde cible." record_pledge_cta: Enregistrer une promesse reopen: Réouvrir l'objectif resume: Reprendre @@ -312,7 +315,7 @@ fr: saved: Épargné to_go: "%{amount} restant(s)" status_callout: - depleted: "il manque %{amount} — complétez pour ramener la réserve à son niveau" + depleted: "il manque %{amount} — complétez pour ramener la réserve à son solde cible" behind: épargnez %{amount}/mois de plus pour rattraper le retard behind_covered: les promesses en attente comblent le retard no_target_date: définissez une date cible pour projeter une date de fin diff --git a/test/controllers/goals_controller_test.rb b/test/controllers/goals_controller_test.rb index 750e11e38..0bffa4316 100644 --- a/test/controllers/goals_controller_test.rb +++ b/test/controllers/goals_controller_test.rb @@ -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 diff --git a/test/models/goal_test.rb b/test/models/goal_test.rb index 06d5d26a1..209d3e1c1 100644 --- a/test/models/goal_test.rb +++ b/test/models/goal_test.rb @@ -779,6 +779,39 @@ class GoalTest < ActiveSupport::TestCase assert_equal BigDecimal("1000"), goal.reload.target_amount.to_d end + # The form asks this BEFORE the user has picked a kind or a number of months, + # to decide whether to keep offering the amount field. So it must answer for + # the family alone, not for what this goal happens to be right now. + test "derivability is answered for the family, whatever the goal is set to" do + IncomeStatement.any_instance.stubs(:median_expense).returns(BigDecimal("500")) + goal = reserve_goal(balance: 3_000, target: 1_000) + + assert goal.months_target_derivable?, + "a one-off with no months set still has spending to derive from" + end + + # With nothing to derive from, the typed amount is the only way to give the + # reserve a target. The form has to be told, or it hides the only field that + # would let the user do it. + test "a family with no spending history can derive nothing" do + IncomeStatement.any_instance.stubs(:median_expense).returns(BigDecimal("0")) + goal = reserve_goal(balance: 3_000, target: 1_000) + + assert_not goal.months_target_derivable? + end + + test "derivability follows the goal's currency, not the family's" do + IncomeStatement.any_instance.stubs(:median_expense).returns(BigDecimal("500")) + goal = reserve_goal(balance: 3_000, target: 1_000) + goal.stubs(:currency).returns("EUR") + Money.any_instance.stubs(:exchange_to).raises( + Money::ConversionError.new(from_currency: "USD", to_currency: "EUR", date: Date.current) + ) + + assert_not goal.months_target_derivable?, + "no rate for the day means no figure to show, so the field must stay" + end + test "account free_to_earmark subtracts non-archived fixed earmarks" do account = Account.create!(family: @family, accountable: Depository.new, name: "Headroom Savings", currency: "USD", balance: 5_000) @family.goals.create!(name: "Earmarker", target_amount: 10_000, currency: "USD") do |g| diff --git a/test/system/goals_form_test.rb b/test/system/goals_form_test.rb new file mode 100644 index 000000000..0985d4b00 --- /dev/null +++ b/test/system/goals_form_test.rb @@ -0,0 +1,33 @@ +require "application_system_test_case" + +# The goal form swaps the amount field's label between "Target amount" and +# "Target balance" as the kind changes. The label also carries the +# required-field asterisk, in a span of its own, so how that swap is made +# decides whether the asterisk survives it — and only a browser can say. +class GoalsFormTest < ApplicationSystemTestCase + setup do + @user = users(:family_admin) + # Goals sit behind the preview gate; without it the visit redirects to the + # dashboard and the form never renders. + @user.update!(preferences: (@user.preferences || {}).merge("preview_features_enabled" => true)) + sign_in @user + end + + test "the required asterisk survives the label swap" do + visit new_goal_path + + # This first check is the one that bites: `refresh()` runs on connect, so a + # swap that replaced the label's children has already wiped the asterisk by + # the time the page is idle — on every goal form, one-off included, with + # nothing to put it back. + label = find(".form-field__label", text: I18n.t("goals.form.fields.target_amount")) + assert label.has_css?("span", text: "*"), + "the required marker was gone before anything was even clicked" + + choose I18n.t("goals.form.kinds.maintained.label") + + swapped = find(".form-field__label", text: I18n.t("goals.form.fields.target_balance")) + assert swapped.has_css?("span", text: "*"), + "the label swap deleted the required marker" + end +end