From 74bb980271e8d5bf526671a11c56420d7645bc2c Mon Sep 17 00:00:00 2001 From: buzzromain <18685603+buzzromain@users.noreply.github.com> Date: Wed, 26 Aug 2026 22:02:57 +0200 Subject: [PATCH] feat(goals): a reserve measured in months of spending, not a fixed sum (#3180) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(goals): a reserve measured in months of spending, not a fixed sum "Six months of expenses" is the way people actually describe an emergency fund, and it is a moving number: what covered six months last January does not cover six months today. A reserve pinned to a figure typed once drifts quietly out of date, and the drift always runs the wrong way — the bar reads full while the cover shrinks. `target_amount` stays the single source of truth, rewritten monthly by RefreshMaintainedGoalTargetsJob. That is the whole architectural decision here. An `effective_target_amount` would have been the obvious shape and the wrong one: `remaining_amount`, `progress_percent`, `Goal.summary_for`, the ring, the card and every future caller would each have had to learn which target to read. None of them change. The job refuses to write more often than it writes, on purpose: - a family with no spending history yet computes a floor of zero, which would both violate the `target_amount > 0` constraint and read to the user as "your reserve is complete". The previous target stands. - a figure identical to the current one is not rewritten, so a reserve does not collect a fresh updated_at every month for nothing. - a write that fails validation leaves the target alone and is recorded through DebugLogEntry, not just the application log: a reserve frozen at a stale floor is invisible to the user, who has no reason to suspect the number stopped moving. The job reads the family's spending, not a member's view. IncomeStatement falls back to Current.user when nobody says otherwise, which in a background job is nobody — so the scope is the whole family, and the number is the same whoever is looking. That is deliberate, and matches how the rollover chain had to be pinned. `target_months` is refused outside a months-mode reserve rather than tolerated: a number nothing reads would sit there looking meaningful, and the job would skip it for reasons no one could see. schema.rb is hand-edited again — verified against a real migration on a throwaway database, structures identical. The dumper on this Rails version also rewrites every check-constraint cast, so the new constraint is written in the file's existing style rather than the dumper's. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DJ1npaGEHr6t2HW1rYZdt4 * fix(goals): pin the reserve calculation to the family, and get it right on day one Review of the previous commit raised two things, and they are the same thing seen from either end. `IncomeStatement.new(family)` looked family-wide but was only so by accident. Its constructor falls back to `Current.user`, and eligible_accounts narrows to that user's accounts when one is present. The single caller was a background job, where nobody is current — so the figure was correct for the reason that it happened to be computed nowhere else. `target_amount` belongs to the whole family: derived from a viewer's slice of the accounts, it would have started moving with whoever last triggered it. This is the same fallback that made the budget rollover carry depend on its reader. The account scope is now passed explicitly, so the calculation is safe whatever calls it. That mattered immediately, because the second point required a new caller. A reserve created as "6 months of expenses" had no floor computed until the 1st of the following month: the user chose the mode, guessed an amount, and lived with a wrong target for up to a month. The feature's first impression was its least convincing moment. The floor is now computed on creation, and whenever the mode or the number of months changes — but never on an unrelated save, so the monthly job keeps owning the cadence and renaming a goal cannot silently move a financial figure. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DJ1npaGEHr6t2HW1rYZdt4 * fix(goals): keep a months-based floor derived, and in the right currency Review on #3180. The median comes back in FAMILY currency and `target_amount` is stored in the GOAL's, so a EUR reserve in a USD family read a 3,000 dollar floor as 3,000 euros — and rewrote it that way every month, silently. Converted now, and when there is no rate for the day the previous target stands: the same safe failure the method already took for a family with no spending history, because a stale floor beats a wrong one. The callback also skipped a target_amount edit, so the form could persist an arbitrary figure under a "six months of expenses" label until the next monthly refresh. It runs on that edit now and overwrites it — and when there is nothing to derive from, restores what the reserve already had rather than accepting the typed figure. The form marks the field read-only in that mode. The model does not depend on it, but a field that silently discards what you type is worse than one you cannot type into. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016GTNba5qE5NwzaHzbp27ye --------- Co-authored-by: Claude Opus 5 --- app/controllers/goals_controller.rb | 4 +- .../controllers/goal_kind_controller.js | 49 ++++-- .../refresh_maintained_goal_targets_job.rb | 48 ++++++ app/models/goal.rb | 126 ++++++++++++++ app/views/goals/_form.html.erb | 23 ++- config/locales/models/goal/en.yml | 5 + config/locales/models/goal/fr.yml | 5 + config/locales/views/goals/en.yml | 6 + config/locales/views/goals/fr.yml | 6 + config/schedule.yml | 6 + ...20260824140000_add_target_mode_to_goals.rb | 14 ++ db/schema.rb | 3 + ...efresh_maintained_goal_targets_job_test.rb | 102 +++++++++++ test/models/goal_test.rb | 158 ++++++++++++++++++ 14 files changed, 540 insertions(+), 15 deletions(-) create mode 100644 app/jobs/refresh_maintained_goal_targets_job.rb create mode 100644 db/migrate/20260824140000_add_target_mode_to_goals.rb create mode 100644 test/jobs/refresh_maintained_goal_targets_job_test.rb diff --git a/app/controllers/goals_controller.rb b/app/controllers/goals_controller.rb index 5d2b67f6b..bac6bfd31 100644 --- a/app/controllers/goals_controller.rb +++ b/app/controllers/goals_controller.rb @@ -200,11 +200,11 @@ class GoalsController < ApplicationController end def goal_params - params.require(:goal).permit(:name, :target_amount, :target_date, :color, :icon, :notes, :kind) + params.require(:goal).permit(:name, :target_amount, :target_date, :color, :icon, :notes, :kind, :target_mode, :target_months) end def goal_update_params - params.require(:goal).permit(:name, :target_amount, :target_date, :color, :icon, :notes, :kind) + params.require(:goal).permit(:name, :target_amount, :target_date, :color, :icon, :notes, :kind, :target_mode, :target_months) end def lookup_accounts(ids) diff --git a/app/javascript/controllers/goal_kind_controller.js b/app/javascript/controllers/goal_kind_controller.js index 95aaec26a..a68234498 100644 --- a/app/javascript/controllers/goal_kind_controller.js +++ b/app/javascript/controllers/goal_kind_controller.js @@ -4,7 +4,7 @@ import { Controller } from "@hotwired/stimulus" // 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"] + static targets = ["radio", "dateField", "modeField", "modeSelect", "monthsField", "amountField"] connect() { this.refresh() @@ -12,18 +12,43 @@ export default class extends Controller { refresh() { const maintained = this.radioTargets.some((radio) => radio.checked && radio.value === "maintained") - this.dateFieldTargets.forEach((field) => field.classList.toggle("hidden", maintained)) - if (maintained) { - this.dateFieldTargets.forEach((field) => { - const input = field.querySelector("input") - if (!input) return - input.value = "" - // Assigning `value` fires nothing, so the pace suggestion bound to - // this input's action kept showing a monthly figure derived from a - // deadline the goal no longer has. - input.dispatchEvent(new Event("input", { bubbles: true })) + this.dateFieldTargets.forEach((field) => field.classList.toggle("hidden", maintained)) + if (maintained) this.#clearInputs(this.dateFieldTargets) + + // The target mode is a reserve's business only; a one-off is always a + // fixed amount. Reset it on the way out so a one-off cannot be saved + // carrying a months mode nothing would ever refresh. + this.modeFieldTargets.forEach((field) => field.classList.toggle("hidden", !maintained)) + if (!maintained && this.hasModeSelectTarget) this.modeSelectTarget.value = "fixed" + + const months = maintained && this.hasModeSelectTarget && this.modeSelectTarget.value === "months_of_expenses" + 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. + this.amountFieldTargets.forEach((field) => { + field.querySelectorAll("input").forEach((input) => { + input.readOnly = months + input.classList.toggle("opacity-60", months) }) - } + }) + } + + #clearInputs(fields) { + fields.forEach((field) => { + const input = field.querySelector("input") + if (!input) return + + input.value = "" + // Assigning `value` fires nothing, so the pace suggestion bound to this + // input's action kept showing a monthly figure derived from a deadline + // the goal no longer has. + input.dispatchEvent(new Event("input", { bubbles: true })) + }) } } diff --git a/app/jobs/refresh_maintained_goal_targets_job.rb b/app/jobs/refresh_maintained_goal_targets_job.rb new file mode 100644 index 000000000..69dc41755 --- /dev/null +++ b/app/jobs/refresh_maintained_goal_targets_job.rb @@ -0,0 +1,48 @@ +# Keeps "N months of expenses" reserves honest. Their floor is a moving +# number: what covered six months last January does not cover six months +# today. Runs on the 1st of each month, after a full month of spending has +# landed. +# +# Writes `target_amount` rather than deriving a target on read, so every +# aggregate that already reads it keeps working — see Goal::TARGET_MODES. +class RefreshMaintainedGoalTargetsJob < ApplicationJob + queue_as :scheduled + + def perform + scope = Goal.where(kind: "maintained", target_mode: "months_of_expenses") + .where.not(state: Goal::RELEASED_STATES) + + scope.find_each do |goal| + refresh(goal) + end + end + + private + def refresh(goal) + previous = goal.target_amount.to_d + updated = goal.refresh_target_from_expenses! + + return if updated.nil? || updated == previous + + Rails.logger.info( + "RefreshMaintainedGoalTargetsJob: goal #{goal.id} target #{previous} -> #{updated}" + ) + rescue ActiveRecord::RecordInvalid => e + # The reserve keeps the target it had. Surfaced in the support UI + # rather than only the application log: a reserve quietly frozen at a + # stale floor is invisible to the user, who has no reason to suspect + # the figure stopped moving. + DebugLogEntry.capture( + category: "goals", + level: "error", + message: "Could not refresh maintained goal target: #{e.message}", + source: self.class.name, + family: goal.family, + metadata: { + goal_id: goal.id, + target_months: goal.target_months, + previous_target_amount: previous.to_s + } + ) + end +end diff --git a/app/models/goal.rb b/app/models/goal.rb index 0343a45b2..5ea22052c 100644 --- a/app/models/goal.rb +++ b/app/models/goal.rb @@ -34,6 +34,24 @@ class Goal < ApplicationRecord # before_save (not before_validation) so it only mutates on persistence, not # on every valid? call — a goal can be inspected without its basis flipping. before_save :default_progress_basis_for_investment + # A reserve measured in months is derived, not typed: computing it only in + # the monthly job would leave a brand-new one wrong until the 1st, so the + # feature's first impression would be its least convincing moment. Fired on + # creation and whenever the inputs change — never on an unrelated save, so + # the job keeps owning the monthly cadence and renaming a goal cannot + # silently move a financial figure. + # A target_amount edit is in the list because in this mode the amount is + # derived, not typed: without it the form could persist an arbitrary figure + # under a "six months of expenses" label until the next monthly refresh. + before_save :apply_months_of_expenses_target, + if: -> { + months_of_expenses_target? && ( + new_record? || + will_save_change_to_target_months? || + will_save_change_to_target_mode? || + will_save_change_to_target_amount? + ) + } validate :must_have_at_least_one_linked_account validate :linked_accounts_must_be_fundable @@ -107,7 +125,19 @@ class Goal < ApplicationRecord # is even allowed, and how they sort. KINDS = %w[one_off maintained].freeze + # How a reserve's floor is expressed. "6 months of expenses" is a moving + # number — what covers six months in January is not what covers six months + # in December — so RefreshMaintainedGoalTargetsJob rewrites `target_amount` + # monthly. `target_amount` stays the single source of truth on purpose: + # every aggregate that reads it (remaining_amount, progress_percent, + # Goal.summary_for, the ring, the card) keeps working untouched, where an + # effective_target_amount would have to be threaded through all of them. + TARGET_MODES = %w[fixed months_of_expenses].freeze + validates :kind, inclusion: { in: KINDS } + validates :target_mode, inclusion: { in: TARGET_MODES } + validates :target_months, numericality: { only_integer: true, greater_than: 0 }, allow_nil: true + validate :months_target_requires_a_reserve # Display order for active (non-completed/non-archived) goals: whatever # needs money first, then on-track, then open-ended, then the reserves that @@ -402,6 +432,66 @@ class Goal < ApplicationRecord kind == "maintained" end + def months_of_expenses_target? + target_mode == "months_of_expenses" + end + + # Recomputes this reserve's floor from the family's median monthly spend. + # Returns the new amount when it wrote one, nil when it deliberately did + # not — a family with no spending history yet, or a figure that would + # violate the `target_amount > 0` check constraint. Leaving the previous + # target standing is the safe failure: it is a number the user has been + # saving against, where zero would silently declare the reserve complete. + def refresh_target_from_expenses! + computed = months_of_expenses_amount + return nil if computed.nil? || computed == target_amount.to_d + + update!(target_amount: computed) + computed + end + + private + # The floor this reserve should hold, or nil when it cannot be computed. + # + # ⚠️ The account scope is passed EXPLICITLY, and that is the whole point + # of this method. IncomeStatement's constructor does `user || Current.user` + # and narrows to that user's accounts, so calling it bare gives a + # family-wide figure only by accident — when no user happens to be + # current, i.e. from a background job. `target_amount` is shared by the + # 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. + 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 + return nil unless median.positive? + + computed = (median * target_months).round(2) + return nil unless computed.positive? + + # The median comes back in FAMILY currency; `target_amount` is stored in + # the GOAL's. A EUR reserve in a USD family would otherwise read a 3,000 + # dollar floor as 3,000 euros, and rewrite it that way every month. + converted = convert_to_goal_currency(computed) + converted&.positive? ? converted : nil + end + + # nil when there is no rate for the day. That is the same safe failure as + # a family with no spending history: the previous target stands, because + # it is a number the user has been saving against and a wrong one is worse + # than a stale one. + def convert_to_goal_currency(amount) + return amount if currency == family.currency + + Money.new(amount, family.currency).exchange_to(currency).amount.round(2) + rescue Money::ConversionError + nil + end + + public + # Market value of the goal's backing (balance basis), regardless of the # progress basis — the "what it's worth today" figure shown next to # contributions on an investment-backed goal. @@ -1016,6 +1106,42 @@ class Goal < ApplicationRecord update_columns(**attrs) end + # Leaves whatever the user typed when the median cannot be computed: the + # presence + positivity validations still apply, so a family with no + # spending history is asked for a figure rather than blocked. + # + # Runs on a target_amount edit too, and overwrites it. In this mode the + # floor is derived, not typed — the form disables the field, but the + # invariant cannot depend on the form: a goal left saying + # "six months of expenses" while holding a figure someone typed is + # untrue on its face, and would stay untrue until the next monthly run. + def apply_months_of_expenses_target + computed = months_of_expenses_amount + return self.target_amount = computed if computed + + # Nothing to derive from and a typed figure on its way in: keep the one + # the reserve already had. Same reasoning as the refresh job — a stale + # floor beats a wrong one, and this one would be wearing a label saying + # it was computed. + self.target_amount = target_amount_in_database if will_save_change_to_target_amount? && !new_record? + end + + # target_months only means something for a reserve on the months basis. + # Allowing it elsewhere would leave a number nothing reads, which the + # refresh job would then look at and skip for reasons no one could see. + def months_target_requires_a_reserve + return if target_mode == "fixed" && target_months.blank? + return if months_of_expenses_target? && maintained? && target_months.present? + + if months_of_expenses_target? && !maintained? + errors.add(:target_mode, :months_requires_maintained) + elsif months_of_expenses_target? + errors.add(:target_months, :blank) + else + errors.add(:target_months, :only_with_months_mode) + end + end + # Cleared after every AASM transition. The state column drives the # display_status / projection_summary memos; without this the same instance # keeps returning the pre-transition value if a controller calls archive! / diff --git a/app/views/goals/_form.html.erb b/app/views/goals/_form.html.erb index 45195af57..4b832d6ab 100644 --- a/app/views/goals/_form.html.erb +++ b/app/views/goals/_form.html.erb @@ -46,8 +46,29 @@

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

+ <%# Only meaningful for a reserve, and only in months mode — the Stimulus + controller shows it accordingly. The amount field stays visible either + way: in months mode it shows what the job last computed, which is the + figure the user is actually saving against — read-only there, because + in that mode it is derived rather than chosen. %> + + + +
-
+
<%= f.money_field :target_amount, label: t("goals.form.fields.target_amount"), hide_currency: true, diff --git a/config/locales/models/goal/en.yml b/config/locales/models/goal/en.yml index cf1272f5d..6f10e8061 100644 --- a/config/locales/models/goal/en.yml +++ b/config/locales/models/goal/en.yml @@ -18,6 +18,11 @@ en: base: at_least_one_linked_account_required: Pick at least one account to fund this goal. whole_account_conflict_on_restore: "Can't restore this goal: “%{account_name}” is now fully earmarked for “%{goal_name}”. Enter an amount on one of them first." + target_mode: + months_requires_maintained: Only a reserve you maintain can have a target set in months of expenses. + target_months: + blank: Say how many months of expenses this reserve should cover. + only_with_months_mode: Months of expenses only apply when the target is set that way. linked_accounts: must_be_fundable: All linked accounts must be cash or investment accounts. currency_mismatch: All linked accounts must share the same currency. diff --git a/config/locales/models/goal/fr.yml b/config/locales/models/goal/fr.yml index 2040ae58b..c6b547a52 100644 --- a/config/locales/models/goal/fr.yml +++ b/config/locales/models/goal/fr.yml @@ -25,6 +25,11 @@ fr: below_consumed: Impossible de fixer une cible inférieure à ce que cet objectif a déjà enregistré comme dépensé. currency: locked_after_linked: Impossible de modifier la devise après que l'objectif a été lié à des comptes. + target_mode: + months_requires_maintained: Seule une réserve à maintenir peut avoir une cible exprimée en mois de dépenses. + target_months: + blank: Indiquez combien de mois de dépenses cette réserve doit couvrir. + only_with_months_mode: Les mois de dépenses ne s'appliquent que si la cible est exprimée ainsi. linked_accounts: currency_mismatch: Tous les comptes liés doivent partager la même devise. must_be_fundable: Tous les comptes liés doivent être des comptes de trésorerie ou d'investissement. diff --git a/config/locales/views/goals/en.yml b/config/locales/views/goals/en.yml index 017d828bd..00f8a7a19 100644 --- a/config/locales/views/goals/en.yml +++ b/config/locales/views/goals/en.yml @@ -291,6 +291,9 @@ en: one: Last pledge matched 1 day ago other: "Last pledge matched %{count} days ago" form: + target_modes: + fixed: A fixed amount + months_of_expenses: A number of months of expenses kinds: one_off: hint: Save toward something, then close it when you spend it. @@ -311,6 +314,9 @@ 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_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 diff --git a/config/locales/views/goals/fr.yml b/config/locales/views/goals/fr.yml index 4695869a2..b204d86e3 100644 --- a/config/locales/views/goals/fr.yml +++ b/config/locales/views/goals/fr.yml @@ -30,6 +30,9 @@ fr: errors: not_found: Cet objectif n'a pas pu être trouvé. Il a peut-être été supprimé. form: + target_modes: + fixed: Un montant fixe + 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. @@ -47,6 +50,9 @@ 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_months: Mois de dépenses + target_months_hint: Recalculé le 1er de chaque mois à partir de vos dépenses mensuelles médianes. color: Couleur earmark_for: Affecter un montant pour %{account} earmark_hint: Laissez le montant vide pour dédier la totalité du solde de ce compte. diff --git a/config/schedule.yml b/config/schedule.yml index 795ccab7e..11b617459 100644 --- a/config/schedule.yml +++ b/config/schedule.yml @@ -66,3 +66,9 @@ sweep_expired_goal_pledges: class: "SweepExpiredGoalPledgesJob" queue: "scheduled" description: "Marks goal pledges that passed their 7-day window as expired" + +refresh_maintained_goal_targets: + cron: "0 3 1 * *" # 3:00 AM on the 1st of each month + class: "RefreshMaintainedGoalTargetsJob" + queue: "scheduled" + description: "Recomputes the floor of reserves expressed in months of expenses" diff --git a/db/migrate/20260824140000_add_target_mode_to_goals.rb b/db/migrate/20260824140000_add_target_mode_to_goals.rb new file mode 100644 index 000000000..283b8a03d --- /dev/null +++ b/db/migrate/20260824140000_add_target_mode_to_goals.rb @@ -0,0 +1,14 @@ +class AddTargetModeToGoals < ActiveRecord::Migration[7.2] + def change + # "6 months of expenses" is a moving target: the amount that covers six + # months in January is not the one that covers six months in December. + # `target_amount` stays the single source of truth — a monthly job + # rewrites it — so every existing aggregate (remaining_amount, + # progress_percent, Goal.summary_for) keeps working untouched, with no + # effective_target_amount that every caller would have to know about. + add_column :goals, :target_mode, :string, null: false, default: "fixed" + add_column :goals, :target_months, :integer + add_check_constraint :goals, "target_mode IN ('fixed','months_of_expenses')", + name: "chk_goals_target_mode_enum" + end +end diff --git a/db/schema.rb b/db/schema.rb index 9e539914d..738b9e2db 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -885,6 +885,8 @@ ActiveRecord::Schema[7.2].define(version: 2026_08_25_120000) do t.decimal "consumed_amount", precision: 19, scale: 4, default: "0.0", null: false t.datetime "completed_at" t.string "kind", default: "one_off", null: false + t.string "target_mode", default: "fixed", null: false + t.integer "target_months" t.index ["family_id", "state"], name: "index_goals_on_family_id_and_state" t.index ["family_id"], name: "index_goals_on_family_id" t.check_constraint "char_length(name::text) <= 255", name: "chk_savings_goals_name_length" @@ -893,6 +895,7 @@ ActiveRecord::Schema[7.2].define(version: 2026_08_25_120000) do t.check_constraint "progress_basis::text = ANY (ARRAY['balance'::character varying, 'contributions'::character varying]::text[])", name: "chk_goals_progress_basis_enum" t.check_constraint "state::text = ANY (ARRAY['active'::character varying, 'paused'::character varying, 'completed'::character varying, 'archived'::character varying]::text[])", name: "chk_savings_goals_state_enum" t.check_constraint "target_amount > 0::numeric", name: "chk_savings_goals_target_amount_positive" + t.check_constraint "target_mode::text = ANY (ARRAY['fixed'::character varying, 'months_of_expenses'::character varying]::text[])", name: "chk_goals_target_mode_enum" end create_table "holdings", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| diff --git a/test/jobs/refresh_maintained_goal_targets_job_test.rb b/test/jobs/refresh_maintained_goal_targets_job_test.rb new file mode 100644 index 000000000..c9fb21696 --- /dev/null +++ b/test/jobs/refresh_maintained_goal_targets_job_test.rb @@ -0,0 +1,102 @@ +require "test_helper" + +class RefreshMaintainedGoalTargetsJobTest < ActiveJob::TestCase + setup do + @family = families(:dylan_family) + end + + test "a reserve's floor follows the median monthly spend" do + goal = reserve(months: 6, target: 1_000) + stub_median(500) + + RefreshMaintainedGoalTargetsJob.perform_now + + assert_equal BigDecimal("3000"), goal.reload.target_amount.to_d + end + + # A family with no spending history yet would compute a floor of zero, + # which violates the target_amount > 0 constraint and would read as "your + # reserve is complete". Keep the number the user has been saving against. + test "a median of zero leaves the target standing" do + goal = reserve(months: 6, target: 1_000) + stub_median(0) + + RefreshMaintainedGoalTargetsJob.perform_now + + assert_equal BigDecimal("1000"), goal.reload.target_amount.to_d + end + + test "fixed-target reserves and one-off goals are left alone" do + fixed = reserve(months: nil, target: 2_000, mode: "fixed") + one_off = @family.goals.create!( + name: "Trip", target_amount: 800, currency: "USD" + ) { |g| g.goal_accounts.build(account: account_for("Trip")) } + stub_median(500) + + RefreshMaintainedGoalTargetsJob.perform_now + + assert_equal BigDecimal("2000"), fixed.reload.target_amount.to_d + assert_equal BigDecimal("800"), one_off.reload.target_amount.to_d + end + + test "an archived reserve is not refreshed" do + goal = reserve(months: 6, target: 1_000) + goal.archive! + stub_median(500) + + RefreshMaintainedGoalTargetsJob.perform_now + + assert_equal BigDecimal("1000"), goal.reload.target_amount.to_d + end + + test "a target already equal to the computed figure is not rewritten" do + goal = reserve(months: 6, target: 3_000) + stub_median(500) + + assert_no_changes -> { goal.reload.updated_at } do + RefreshMaintainedGoalTargetsJob.perform_now + end + end + + # A reserve silently frozen at a stale floor is invisible to the user, so + # the failure belongs in the support UI, not only in the application log. + test "a write that fails validation is recorded for support" do + goal = reserve(months: 6, target: 1_000) + stub_median(500) + Goal.any_instance.stubs(:update!).raises( + ActiveRecord::RecordInvalid.new(goal) + ) + + assert_difference -> { DebugLogEntry.where(category: "goals").count }, 1 do + RefreshMaintainedGoalTargetsJob.perform_now + end + + assert_equal BigDecimal("1000"), goal.reload.target_amount.to_d + end + + private + def account_for(name) + Account.create!( + family: @family, accountable: Depository.new, + name: "#{name} Pot", currency: "USD", balance: 1_000 + ) + end + + # `target` is the figure the reserve STARTS from, so it is written past + # the before_save that computes a months-mode floor on creation — these + # tests are about what the job does to an existing target, not about + # where that target came from. + def reserve(months:, target:, mode: "months_of_expenses", name: "Emergency") + goal = @family.goals.create!( + name: name, target_amount: target, currency: "USD", + kind: "maintained", target_mode: mode, target_months: months + ) { |g| g.goal_accounts.build(account: account_for(name)) } + + goal.update_column(:target_amount, target) + goal.reload + end + + def stub_median(amount) + IncomeStatement.any_instance.stubs(:median_expense).returns(BigDecimal(amount.to_s)) + end +end diff --git a/test/models/goal_test.rb b/test/models/goal_test.rb index 212351b6d..6bd5d1b68 100644 --- a/test/models/goal_test.rb +++ b/test/models/goal_test.rb @@ -677,6 +677,108 @@ class GoalTest < ActiveSupport::TestCase assert_equal BigDecimal("2000"), Goal.find(goal.id).remaining_amount.to_d end + # --- Lot B3b: a floor expressed in months of expenses --- + + test "months mode is only for a reserve, and needs a number of months" do + goal = reserve_goal(balance: 6_000, target: 6_000) + + goal.target_mode = "months_of_expenses" + assert_not goal.valid?, "months mode without target_months must be refused" + + goal.target_months = 6 + assert goal.valid?, goal.errors.full_messages.to_sentence + + goal.kind = "one_off" + assert_not goal.valid?, "a one-off goal has no months-of-expenses floor" + end + + test "target_months without months mode is refused" do + goal = reserve_goal(balance: 6_000, target: 6_000) + goal.target_months = 6 + + assert_not goal.valid? + end + + # target_amount stays the source of truth — the aggregates that read it must + # keep working with no knowledge of the mode. + test "refreshing the floor moves every figure that reads target_amount" do + goal = reserve_goal(balance: 3_000, target: 1_000) + goal.update!(target_mode: "months_of_expenses", target_months: 6) + IncomeStatement.any_instance.stubs(:median_expense).returns(BigDecimal("500")) + + assert_equal BigDecimal("3000"), goal.refresh_target_from_expenses! + + refreshed = Goal.find(goal.id) + assert_equal BigDecimal("3000"), refreshed.target_amount.to_d + assert_equal :funded, refreshed.status + assert_equal 100, refreshed.progress_percent + end + + # A family with no spending history computes a floor of zero, which would + # both break the target_amount > 0 constraint and read as "your reserve is + # complete". The figure the user has been saving against stands. + test "a zero median leaves the floor untouched" do + IncomeStatement.any_instance.stubs(:median_expense).returns(BigDecimal("500")) + goal = reserve_goal(balance: 3_000, target: 1_000) + goal.update!(target_mode: "months_of_expenses", target_months: 6) + assert_equal BigDecimal("3000"), goal.reload.target_amount.to_d + + IncomeStatement.any_instance.stubs(:median_expense).returns(BigDecimal("0")) + + assert_nil goal.refresh_target_from_expenses! + assert_equal BigDecimal("3000"), goal.reload.target_amount.to_d + end + + # A reserve created in months mode must be right immediately: waiting for + # the 1st would make the feature's first impression its least convincing. + test "creating a months-mode reserve computes its floor straight away" do + IncomeStatement.any_instance.stubs(:median_expense).returns(BigDecimal("500")) + account = Account.create!( + family: @family, accountable: Depository.new, + name: "Fresh Reserve Pot", currency: "USD", balance: 100 + ) + + goal = @family.goals.create!( + name: "Fresh reserve", target_amount: 1, currency: "USD", + kind: "maintained", target_mode: "months_of_expenses", target_months: 6 + ) { |g| g.goal_accounts.build(account: account) } + + assert_equal BigDecimal("3000"), goal.reload.target_amount.to_d + end + + test "changing the number of months recomputes the floor at once" do + IncomeStatement.any_instance.stubs(:median_expense).returns(BigDecimal("500")) + goal = reserve_goal(balance: 100, target: 1) + goal.update!(target_mode: "months_of_expenses", target_months: 6) + assert_equal BigDecimal("3000"), goal.reload.target_amount.to_d + + goal.update!(target_months: 3) + assert_equal BigDecimal("1500"), goal.reload.target_amount.to_d + end + + # The monthly job owns the cadence: an unrelated edit must not quietly move + # a financial figure the user has been saving against. + test "renaming a months-mode reserve leaves its floor alone" do + IncomeStatement.any_instance.stubs(:median_expense).returns(BigDecimal("500")) + goal = reserve_goal(balance: 100, target: 1) + goal.update!(target_mode: "months_of_expenses", target_months: 6) + + IncomeStatement.any_instance.unstub(:median_expense) + IncomeStatement.any_instance.stubs(:median_expense).returns(BigDecimal("900")) + goal.update!(name: "Renamed reserve") + + assert_equal BigDecimal("3000"), goal.reload.target_amount.to_d, + "only the job, or a change of months, may move the floor" + end + + test "a fixed reserve ignores the refresh entirely" do + goal = reserve_goal(balance: 3_000, target: 1_000) + IncomeStatement.any_instance.stubs(:median_expense).returns(BigDecimal("500")) + + assert_nil goal.refresh_target_from_expenses! + assert_equal BigDecimal("1000"), goal.reload.target_amount.to_d + 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| @@ -1049,6 +1151,51 @@ class GoalTest < ActiveSupport::TestCase assert_not reserve.needs_attention? end + # --- months-of-expenses review follow-ups --- + + # The median comes back in FAMILY currency; target_amount is stored in the + # GOAL's. A EUR reserve in a USD family would otherwise read a 3,000 dollar + # floor as 3,000 euros, and rewrite it that way every month. + test "a reserve in another currency gets its floor converted" do + account = Account.create!(family: @family, accountable: Depository.new, + name: "EUR pot", currency: "EUR", balance: 1_000) + goal = @family.goals.create!( + name: "Precaution", target_amount: 1_000, currency: "EUR", + kind: "maintained", target_mode: "months_of_expenses", target_months: 6 + ) { |g| g.goal_accounts.build(account: account, allocated_amount: 1_000) } + + # 500/month family currency x 6 months = 3,000, at a rate of 0.9 = 2,700. + IncomeStatement.any_instance.stubs(:median_expense).returns(500) + Money.any_instance.stubs(:exchange_to).returns(Money.new(2_700, "EUR")) + + goal.update!(target_amount: 1) + + assert_equal 2_700, goal.reload.target_amount.to_d + end + + # The floor is derived in this mode. Without this the edit form could + # persist an arbitrary figure under a "six months of expenses" label until + # the next monthly refresh. + test "a typed amount cannot stand in for a derived floor" do + goal = months_based_reserve + IncomeStatement.any_instance.stubs(:median_expense).returns(500) + + goal.update!(target_amount: 99) + + assert_equal 3_000, goal.reload.target_amount.to_d + end + + # Nothing to derive from and a typed figure on its way in: a stale floor + # beats a wrong one wearing a computed label. + test "with no spending history a typed amount does not replace the floor" do + goal = months_based_reserve + IncomeStatement.any_instance.stubs(:median_expense).returns(0) + + goal.update!(target_amount: 99) + + assert_equal 3_000, goal.reload.target_amount.to_d + end + private # Its own account, so the shared-pool haircut does not make the frozen @@ -1066,6 +1213,17 @@ class GoalTest < ActiveSupport::TestCase Account.create!(family: @family, accountable: Depository.new, name: "Pot #{SecureRandom.hex(3)}", currency: @family.currency, balance: balance) + end + + def months_based_reserve + account = Account.create!(family: @family, accountable: Depository.new, + name: "Reserve pot #{SecureRandom.hex(3)}", + currency: @family.currency, balance: 3_000) + IncomeStatement.any_instance.stubs(:median_expense).returns(500) + @family.goals.create!( + name: "Precaution", target_amount: 3_000, currency: @family.currency, + kind: "maintained", target_mode: "months_of_expenses", target_months: 6 + ) { |g| g.goal_accounts.build(account: account, allocated_amount: 3_000) } end # A fresh account: the fixtures deliberately carry three goals holding # whole-account links on `depository`, a legacy overlap the exclusivity