From 8548703651bb91ff58c2c09200ffc4f7acc41e42 Mon Sep 17 00:00:00 2001 From: Guillem Arias Date: Thu, 14 May 2026 19:43:29 +0200 Subject: [PATCH] refactor(goals/show): move projection_summary + catch_up_delta to model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The show template carried a 17-line `if/elsif` chain computing `projection_summary` inline, plus a `Money.new([…, 0].max, …)` expression building the catch-up delta on the fly. CLAUDE.md's "skinny controllers, fat models" convention pushes both onto Goal. - `Goal#projection_summary`: returns the localized, `html_safe`-aware string for the chart subtitle and the chart's `aria-description`. Memoized so the two callsites in show.html.erb share one computation. - `Goal#catch_up_delta_money`: clamped-at-zero monthly delta between pace and the required monthly target. Used by the catch-up callout body. Previously the view computed `Money.new([req - pace, 0].max, currency)` — same math, but duplicated inline. show.html.erb drops both blocks and reads `@goal.projection_summary` / `@goal.catch_up_delta_money` directly. Also: V15 — the celebration card used `bg-green-500/10` directly. Swap to `bg-success/10` (DS semantic token, same Tailwind-4 alpha syntax DS::Alert already uses) so the celebration palette tracks the rest of the success surface. --- app/models/goal.rb | 38 +++++++++++++++++++++++++++++++++++ app/views/goals/show.html.erb | 32 +++++------------------------ 2 files changed, 43 insertions(+), 27 deletions(-) diff --git a/app/models/goal.rb b/app/models/goal.rb index 5f9005c6e..b57abcf2c 100644 --- a/app/models/goal.rb +++ b/app/models/goal.rb @@ -249,6 +249,44 @@ class Goal < ApplicationRecord any_connected_account? ? "goals.show.pledge_just_transferred" : "goals.show.pledge_just_saved" end + # Single source of truth for the projection-chart subtitle / chart-aria + # description. Used to live inline in show.html.erb as a 17-line if/elsif + # chain. Returns an `html_safe` string when it picks the `_html` variant. + def projection_summary + return @projection_summary if defined?(@projection_summary) + + @projection_summary = + if completed? || progress_percent >= 100 + I18n.t("goals.show.projection.reached") + elsif target_date.nil? + I18n.t("goals.show.projection.no_target_date") + elsif monthly_target_amount && pace.to_d < monthly_target_amount.to_d + I18n.t( + "goals.show.projection.behind", + current: Money.new(pace, currency).format, + required: Money.new(monthly_target_amount, currency).format + ) + elsif pace.positive? + months = (remaining_amount.to_d / pace.to_d).ceil + I18n.t( + "goals.show.projection.on_track_html", + date: (Date.current >> months.to_i).strftime("%b %Y") + ) + else + I18n.t("goals.show.projection.no_pace") + end + end + + # Monthly extra needed beyond the current pace to hit the target on time. + # Clamps at zero — never asks the user to "make up" a deficit they're + # already ahead of. + def catch_up_delta_money + return Money.new(0, currency) if monthly_target_amount.nil? + + delta = [ monthly_target_amount.to_d - pace.to_d, 0 ].max + Money.new(delta, currency) + end + private def balance_series_values return [] if linked_accounts.empty? diff --git a/app/views/goals/show.html.erb b/app/views/goals/show.html.erb index d73918117..ab1b8cedf 100644 --- a/app/views/goals/show.html.erb +++ b/app/views/goals/show.html.erb @@ -136,14 +136,12 @@ <% end %> <% elsif @goal.status == :behind && @goal.monthly_target_amount %> <% catch_up_money = Money.new(@goal.monthly_target_amount, @goal.currency) %> - <% catch_up_pace_money = @goal.pace_money %> - <% catch_up_delta_money = Money.new([ @goal.monthly_target_amount.to_d - @goal.pace.to_d, 0 ].max, @goal.currency) %> <%= render DS::Alert.new(variant: "warning", title: t("goals.show.catch_up.title", amount: catch_up_money.format)) do %>

<% if @goal.target_date %> - <%= t("goals.show.catch_up.body_with_date", avg: catch_up_pace_money.format, delta: catch_up_delta_money.format, date: I18n.l(@goal.target_date, format: :long)) %> + <%= t("goals.show.catch_up.body_with_date", avg: @goal.pace_money.format, delta: @goal.catch_up_delta_money.format, date: I18n.l(@goal.target_date, format: :long)) %> <% else %> - <%= t("goals.show.catch_up.body", avg: catch_up_pace_money.format, delta: catch_up_delta_money.format) %> + <%= t("goals.show.catch_up.body", avg: @goal.pace_money.format, delta: @goal.catch_up_delta_money.format) %> <% end %>

@@ -176,26 +174,6 @@

- <% - pace = @goal.pace - required = @goal.monthly_target_amount - projection_summary = if @goal.completed? || @goal.progress_percent >= 100 - t("goals.show.projection.reached") - elsif @goal.target_date.nil? - t("goals.show.projection.no_target_date") - elsif required && pace.to_d < required.to_d - t("goals.show.projection.behind", - current: Money.new(pace, @goal.currency).format, - required: Money.new(required, @goal.currency).format) - elsif pace.positive? - months = (@goal.remaining_amount.to_d / pace.to_d).ceil - t("goals.show.projection.on_track_html", - date: (Date.current >> months.to_i).strftime("%b %Y")) - else - t("goals.show.projection.no_pace") - end - %> - <% if @goal.archived? || @goal.paused? %>
@@ -210,7 +188,7 @@
<% elsif @goal.completed? || @goal.status == :reached %>
-
+
<%= icon("party-popper", size: "2xl", color: "success") %>

<%= t(".celebration.heading") %>

@@ -252,7 +230,7 @@

<%= t(".projection.heading") %>

-

<%= projection_summary %>

+

<%= @goal.projection_summary %>

<% projection_color = @goal.status == :on_track ? "var(--color-green-600)" : "var(--color-yellow-600)" %>
@@ -272,7 +250,7 @@ data-controller="goal-projection-chart" data-goal-projection-chart-data-value="<%= @goal.projection_payload.to_json %>" data-goal-projection-chart-aria-label-value="<%= t("goals.show.projection.aria_label", name: @goal.name) %>" - data-goal-projection-chart-aria-description-value="<%= strip_tags(projection_summary) %>">
+ data-goal-projection-chart-aria-description-value="<%= strip_tags(@goal.projection_summary) %>">
<% if @goal.target_date.nil? %>
<%= icon("calendar-plus", size: "sm") %>