From 390c74ef5a7706fd5950adea76f1c38d8800668c Mon Sep 17 00:00:00 2001
From: maebeale <maebeale@users.noreply.github.com>
Date: Mon, 22 Jun 2026 08:17:30 -0400
Subject: [PATCH] Clarify payment "Revert" as removing an allocation

The revert action only offsets an allocation and returns its amount to
the payment's unallocated balance, but the bare "revert" label and a
silent greyed-out row left users guessing what it did. Reframe it as
"Remove" with a consequence-stating confirm, explicit Removed/Reversal
badges, and a one-line explainer, plus light polish on the payments
index and detail views.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 app/views/payments/index.html.erb           |  2 +-
 app/views/payments/payment_results.html.erb | 13 +++--
 app/views/payments/show.html.erb            | 58 +++++++++++++--------
 3 files changed, 45 insertions(+), 28 deletions(-)

diff --git a/app/views/payments/index.html.erb b/app/views/payments/index.html.erb
index 76596d4c54..3227c2ccb2 100644
--- a/app/views/payments/index.html.erb
+++ b/app/views/payments/index.html.erb
@@ -1,7 +1,7 @@
 <div class="max-w-6xl mx-auto">
   <div class="flex justify-between items-center mb-6">
     <h1 class="text-2xl font-bold">Payments</h1>
-    <div class="flex gap-2"><%= link_to "Add Cash Payment", new_payment_path(type: "CashPayment"), class: "btn btn-secondary" %><%= link_to "Add Check Payment", new_payment_path(type: "CheckPayment"), class: "btn btn-secondary" %><%= link_to "Generate Stripe Link", new_checkout_link_payments_path, class: "btn btn-secondary" %></div>
+    <div class="flex gap-2"><%= link_to "Add cash payment", new_payment_path(type: "CashPayment"), class: "btn btn-secondary" %><%= link_to "Add check payment", new_payment_path(type: "CheckPayment"), class: "btn btn-secondary" %><%= link_to "Generate Stripe link", new_checkout_link_payments_path, class: "btn btn-secondary" %></div>
   </div>
 
   <%= render 'search_boxes' %>
diff --git a/app/views/payments/payment_results.html.erb b/app/views/payments/payment_results.html.erb
index 93cc2c6c02..dadcaef3a4 100644
--- a/app/views/payments/payment_results.html.erb
+++ b/app/views/payments/payment_results.html.erb
@@ -1,6 +1,6 @@
 <%= turbo_frame_tag :payment_results do %>
   <% if @payments.any? %>
-    <div class="overflow-x-auto animate-fade blur-on-submit">
+    <div class="bg-white border border-gray-200 rounded-lg shadow-sm overflow-x-auto animate-fade blur-on-submit">
       <table class="min-w-full divide-y divide-gray-200">
         <thead class="bg-gray-50">
           <tr>
@@ -14,11 +14,11 @@
         </thead>
         <tbody class="bg-white divide-y divide-gray-200">
           <% @payments.each do |payment| %>
-            <tr>
+            <tr class="hover:bg-gray-50 transition-colors">
               <td class="px-6 py-4 whitespace-nowrap text-sm text-gray-900"><%= payment.type.underscore.titleize.gsub(" Payment", "").gsub("External Processor", "Stripe") %></td>
               <td class="px-6 py-4 whitespace-nowrap text-sm text-gray-900"><%= payment.payer.name %></td>
               <td class="px-6 py-4 whitespace-nowrap text-sm text-gray-900"><%= dollars_from_cents(payment.amount_cents) %></td>
-              <td class="px-6 py-4 whitespace-nowrap text-sm text-gray-900"><%= dollars_from_cents(payment.amount_cents_remaining) %></td>
+              <td class="px-6 py-4 whitespace-nowrap text-sm <%= payment.amount_cents_remaining > 0 ? "text-amber-700 font-medium" : "text-gray-500" %>"><%= dollars_from_cents(payment.amount_cents_remaining) %></td>
               <td class="px-6 py-4 whitespace-nowrap text-sm text-gray-500"><%= payment.created_at.strftime("%B %d, %Y at %I:%M %p") %></td>
               <td class="px-6 py-4 whitespace-nowrap text-right text-sm font-medium"><%= link_to "View", payment_path(payment), class: "text-primary hover:text-primary-dark", data: { turbo_frame: "_top"} %></td>
             </tr>
@@ -26,8 +26,11 @@
         </tbody>
       </table>
     </div>
+    <div class="pagination flex justify-center mt-6"><%= tailwind_paginate @payments %></div>
   <% else %>
-    <p class="text-gray-500 text-center py-6">There are no payments that match your search. Please try again.</p>
+    <div class="bg-white border border-gray-200 rounded-lg shadow-sm py-12 text-center">
+      <i class="fa-solid fa-receipt text-3xl text-gray-300"></i>
+      <p class="mt-3 text-gray-500">There are no payments that match your search. Please try again.</p>
+    </div>
   <% end %>
-  <div class="pagination flex justify-center mt-6"><%= tailwind_paginate @payments %></div>
 <% end %>
diff --git a/app/views/payments/show.html.erb b/app/views/payments/show.html.erb
index dabd0078f8..8435725a41 100644
--- a/app/views/payments/show.html.erb
+++ b/app/views/payments/show.html.erb
@@ -36,7 +36,7 @@
     </div>
     <% if @payment.check_number.present? %>
       <div>
-        <dt class="text-sm font-medium text-gray-500">Check Number</dt>
+        <dt class="text-sm font-medium text-gray-500">Check number</dt>
         <dd class="mt-1 text-sm text-gray-900"><%= @payment.check_number %></dd>
       </div>
     <% end %>
@@ -48,7 +48,7 @@
     <% end %>
     <% if @payment.pay_charge_id.present? %>
       <div>
-        <dt class="text-sm font-medium text-gray-500">Pay Charge ID</dt>
+        <dt class="text-sm font-medium text-gray-500">Pay charge ID</dt>
         <dd class="mt-1 text-sm text-gray-900"><%= @payment.pay_charge_id %></dd>
       </div>
     <% end %>
@@ -92,43 +92,57 @@
   </dl>
   <% if @allocations.any? %>
     <div class="mt-8">
-      <h2 class="text-lg font-medium mb-4">Allocations</h2>
+      <div class="flex items-baseline justify-between mb-1">
+        <h2 class="text-lg font-medium">Allocations</h2>
+      </div>
+      <p class="text-sm text-gray-500 mb-4">Removing an allocation returns its amount to this payment's unallocated balance.</p>
       <div class="bg-white border border-gray-200 rounded-xl shadow-sm overflow-hidden">
         <table class="min-w-full divide-y divide-gray-200">
           <thead class="bg-gray-50">
             <tr>
-              <th class="px-4 py-3 text-left text-xs font-medium text-gray-500 uppercase">Allocated To</th>
+              <th class="px-4 py-3 text-left text-xs font-medium text-gray-500 uppercase">Allocated to</th>
               <th class="px-4 py-3 text-left text-xs font-medium text-gray-500 uppercase">Amount</th>
-              <th class="px-4 py-3 text-left text-xs font-medium text-gray-500 uppercase">Date/Time</th>
+              <th class="px-4 py-3 text-left text-xs font-medium text-gray-500 uppercase">Date/time</th>
               <th class="px-4 py-3 text-right text-xs font-medium text-gray-500 uppercase">Actions</th>
             </tr>
           </thead>
           <tbody class="divide-y divide-gray-200">
             <% @allocations.each do |allocation| %>
-              <% row_class = if allocation.amount.to_i < 0
-                  'bg-red-50'
-                elsif allocation.reverted_id.present?
-                  'bg-gray-100 border border-gray-300 opacity-60'
+              <% is_reversal = allocation.amount.to_i < 0 %>
+              <% is_removed = allocation.reverted_id.present? %>
+              <% row_class = if is_reversal
+                  "bg-amber-50"
+                elsif is_removed
+                  "bg-gray-50 text-gray-400"
                 end %>
               <tr class="<%= row_class %>">
-                <td class="px-4 py-3 text-sm text-gray-900">
-                  <%= link_to allocations_path(allocatable_sgid: allocation.allocatable.to_sgid.to_s) do %>
-                    <% if allocation.allocatable_type == "EventRegistration" && allocation.allocatable.present? %>
-                      <%= allocation.allocatable.registrant.full_name %>
-                      —
-                      <%= allocation.allocatable.event.title %>
-                    <% else %>
-                      <%= allocation.allocatable_type.underscore.titleize %>
+                <td class="px-4 py-3 text-sm">
+                  <div class="flex items-center gap-2">
+                    <%= link_to allocations_path(allocatable_sgid: allocation.allocatable.to_sgid.to_s), class: "#{'line-through' if is_removed}" do %>
+                      <% if allocation.allocatable_type == "EventRegistration" && allocation.allocatable.present? %>
+                        <%= allocation.allocatable.registrant.full_name %>
+                        —
+                        <%= allocation.allocatable.event.title %>
+                      <% else %>
+                        <%= allocation.allocatable_type.underscore.humanize %>
+                      <% end %>
                     <% end %>
-                  <% end %>
+                    <% if is_reversal %>
+                      <span class="inline-flex items-center gap-1 rounded-full text-xs font-medium border px-2 py-0.5 bg-amber-50 text-amber-700 border-amber-200"><i class="fa-solid fa-rotate-left"></i> Reversal</span>
+                    <% elsif is_removed %>
+                      <span class="inline-flex items-center gap-1 rounded-full text-xs font-medium border px-2 py-0.5 bg-gray-100 text-gray-500 border-gray-300"><i class="fa-solid fa-xmark"></i> Removed</span>
+                    <% end %>
+                  </div>
                 </td>
 
-                <td class="px-4 py-3 text-sm text-gray-900"><%= dollars_from_cents(allocation.amount) %></td>
+                <td class="px-4 py-3 text-sm <%= is_reversal ? 'text-amber-700' : 'text-gray-900' %>"><%= dollars_from_cents(allocation.amount) %></td>
                 <td class="px-4 py-3 text-sm text-gray-500"><%= allocation.created_at.strftime("%B %d, %Y at %I:%M %p") %></td>
 
                 <td class="px-4 py-3 text-right text-sm font-medium">
-                  <% unless allocation.reverted? || allocation.amount.to_i < 0 %>
-                    <%= button_to "revert", revert_allocation_path(allocation), method: :post, class: "cursor-pointer text-danger hover:text-danger-dark", data: { turbo_confirm: "are you sure you want to revert this allocation?" } %>
+                  <% unless is_removed || is_reversal %>
+                    <%= button_to revert_allocation_path(allocation), method: :post, class: "inline-flex items-center gap-1.5 cursor-pointer text-danger hover:text-danger-dark", data: { turbo_confirm: "Remove this allocation? The #{dollars_from_cents(allocation.amount)} will be returned to this payment's unallocated balance." } do %>
+                      <i class="fa-solid fa-trash-can"></i> Remove
+                    <% end %>
                   <% end %>
                 </td>
               </tr>
@@ -149,7 +163,7 @@
               <th class="px-4 py-3 text-left text-xs font-medium text-gray-500 uppercase">Recipient</th>
               <th class="px-4 py-3 text-left text-xs font-medium text-gray-500 uppercase">Method</th>
               <th class="px-4 py-3 text-left text-xs font-medium text-gray-500 uppercase">Amount</th>
-              <th class="px-4 py-3 text-left text-xs font-medium text-gray-500 uppercase">Date/Time</th>
+              <th class="px-4 py-3 text-left text-xs font-medium text-gray-500 uppercase">Date/time</th>
             </tr>
           </thead>
           <tbody class="divide-y divide-gray-200">