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. %> +<%= t("goals.form.fields.target_months_hint") %>
+