From e09d79ce25bb14585592eabdb6a1329367cf5080 Mon Sep 17 00:00:00 2001
From: Guillem Arias <accounts@gariasf.com>
Date: Mon, 11 May 2026 16:09:51 +0200
Subject: [PATCH] feat(savings_goals/show): scrollable contributions list +
 hide pace card for reached goals
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

D10 — Drop the silent .limit(50) on the contributions query and put the
list in a max-h-[480px] scrollable container. Goals with many
contributions now show all of them without truncation, but the page
stays compact via the inner scroll.

Polish — reached / completed goals no longer render the combo monthly-
pace card. After the goal is hit, comparing actual vs required pace is
moot (and target $0/mo · Above target pace was awkward filler). Only the
Total contributions card remains in the stat row.

D12 (clickable contribution rows) deferred — adding a dedicated
contribution detail/edit route adds enough scope to warrant its own
ticket. The per-row delete X already covers the only mutation people
need from this view.
---
 app/controllers/savings_goals_controller.rb |  2 +-
 app/views/savings_goals/show.html.erb       | 50 ++++++++++++---------
 2 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/app/controllers/savings_goals_controller.rb b/app/controllers/savings_goals_controller.rb
index a4c4327bf..e35a2c3ab 100644
--- a/app/controllers/savings_goals_controller.rb
+++ b/app/controllers/savings_goals_controller.rb
@@ -20,7 +20,7 @@ class SavingsGoalsController < ApplicationController
   end
 
   def show
-    @contributions = @savings_goal.savings_contributions.includes(:account).chronological.limit(50)
+    @contributions = @savings_goal.savings_contributions.includes(:account).chronological
     @funding_breakdown = funding_breakdown_for(@savings_goal)
     @stats = stats_for(@savings_goal)
   end
diff --git a/app/views/savings_goals/show.html.erb b/app/views/savings_goals/show.html.erb
index e96301c73..323e7b0e7 100644
--- a/app/views/savings_goals/show.html.erb
+++ b/app/views/savings_goals/show.html.erb
@@ -202,29 +202,33 @@
     <% end %>
   </section>
 
-  <%# Stat row — combo pace card + contributions count %>
-  <section class="grid grid-cols-1 md:grid-cols-3 gap-3">
-    <%# Combo: Avg vs Target pace %>
-    <div class="md:col-span-2 bg-container rounded-xl shadow-border-xs px-5 py-4">
-      <p class="text-[11px] text-secondary mb-2"><%= t(".stats.monthly_pace") %></p>
-      <div class="flex items-baseline gap-2">
-        <p class="text-2xl font-medium text-primary tabular-nums"><%= Money.new(@stats[:avg_monthly], @savings_goal.currency).format %></p>
-        <p class="text-sm text-subdued tabular-nums">/mo</p>
-        <% if @savings_goal.monthly_target_amount %>
-          <p class="text-sm text-subdued tabular-nums">· <%= t(".stats.target_of", amount: Money.new(@savings_goal.monthly_target_amount, @savings_goal.currency).format) %></p>
+  <%# Stat row — combo pace card + contributions count. Reached goals
+      hide the pace combo since the comparison is moot. %>
+  <% goal_reached = @savings_goal.completed? || @savings_goal.status == :reached %>
+  <section class="grid grid-cols-1 <%= goal_reached ? "" : "md:grid-cols-3" %> gap-3">
+    <% unless goal_reached %>
+      <%# Combo: Avg vs Target pace %>
+      <div class="md:col-span-2 bg-container rounded-xl shadow-border-xs px-5 py-4">
+        <p class="text-[11px] text-secondary mb-2"><%= t(".stats.monthly_pace") %></p>
+        <div class="flex items-baseline gap-2">
+          <p class="text-2xl font-medium text-primary tabular-nums"><%= Money.new(@stats[:avg_monthly], @savings_goal.currency).format %></p>
+          <p class="text-sm text-subdued tabular-nums">/mo</p>
+          <% if @savings_goal.monthly_target_amount && @savings_goal.monthly_target_amount.to_d.positive? %>
+            <p class="text-sm text-subdued tabular-nums">· <%= t(".stats.target_of", amount: Money.new(@savings_goal.monthly_target_amount, @savings_goal.currency).format) %></p>
+          <% end %>
+        </div>
+        <% if @savings_goal.monthly_target_amount && @savings_goal.monthly_target_amount.to_d.positive? %>
+          <% delta = @savings_goal.monthly_target_amount.to_d - @stats[:avg_monthly].to_d %>
+          <% if delta.positive? %>
+            <p class="text-xs text-warning mt-1 tabular-nums"><%= t(".stats.behind_by", amount: Money.new(delta, @savings_goal.currency).format) %></p>
+          <% else %>
+            <p class="text-xs text-success mt-1 tabular-nums"><%= t(".stats.above_target_pace") %></p>
+          <% end %>
+        <% else %>
+          <p class="text-xs text-subdued mt-1"><%= t(".stats.no_required_pace") %></p>
         <% end %>
       </div>
-      <% if @savings_goal.monthly_target_amount %>
-        <% delta = @savings_goal.monthly_target_amount.to_d - @stats[:avg_monthly].to_d %>
-        <% if delta.positive? %>
-          <p class="text-xs text-warning mt-1 tabular-nums"><%= t(".stats.behind_by", amount: Money.new(delta, @savings_goal.currency).format) %></p>
-        <% else %>
-          <p class="text-xs text-success mt-1 tabular-nums"><%= t(".stats.above_target_pace") %></p>
-        <% end %>
-      <% else %>
-        <p class="text-xs text-subdued mt-1"><%= t(".stats.no_required_pace") %></p>
-      <% end %>
-    </div>
+    <% end %>
 
     <%# Total contributions %>
     <div class="bg-container rounded-xl shadow-border-xs px-5 py-4">
@@ -241,7 +245,9 @@
         <h3 class="text-sm font-medium text-primary"><%= t(".contributions_heading") %></h3>
         <span class="ml-2 text-xs text-subdued tabular-nums"><%= @contributions.size %></span>
       </div>
-      <%= render "contributions_list", contributions: @contributions %>
+      <div class="max-h-[480px] overflow-y-auto">
+        <%= render "contributions_list", contributions: @contributions %>
+      </div>
     </div>
 
     <div class="bg-container rounded-xl shadow-border-xs p-5">