Fix account deletion cascade bug (#1644)

* Fix account deletion cascade bug

* Rubocop fixes

app/controllers/accounts_controller.rb

@@ -4,8 +4,8 @@ class AccountsController < ApplicationController
   before_action :set_account, only: %i[sync]
 
   def index
-    @manual_accounts = Current.family.accounts.where(scheduled_for_deletion: false).manual.alphabetically
-    @plaid_items = Current.family.plaid_items.where(scheduled_for_deletion: false).ordered
+    @manual_accounts = Current.family.accounts.manual.alphabetically
+    @plaid_items = Current.family.plaid_items.ordered
   end
 
   def summary

app/models/account/transaction.rb

@@ -6,8 +6,8 @@ class Account::Transaction < ApplicationRecord
   has_many :taggings, as: :taggable, dependent: :destroy
   has_many :tags, through: :taggings
 
-  has_one :transfer_as_inflow, class_name: "Transfer", foreign_key: "inflow_transaction_id", dependent: :restrict_with_exception
-  has_one :transfer_as_outflow, class_name: "Transfer", foreign_key: "outflow_transaction_id", dependent: :restrict_with_exception
+  has_one :transfer_as_inflow, class_name: "Transfer", foreign_key: "inflow_transaction_id", dependent: :destroy
+  has_one :transfer_as_outflow, class_name: "Transfer", foreign_key: "outflow_transaction_id", dependent: :destroy
 
   accepts_nested_attributes_for :taggings, allow_destroy: true
 

app/views/accounts/_account.html.erb

@@ -8,32 +8,47 @@
       </div>
 
       <div>
-        <%= link_to account.name, account, class: [(account.is_active ? "text-gray-900" : "text-gray-400"), "text-sm font-medium hover:underline"], data: { turbo_frame: "_top" } %>
-        <% if account.has_issues? %>
-          <div class="text-sm flex items-center gap-1 text-error">
-            <%= lucide_icon "alert-octagon", class: "shrink-0 w-4 h-4" %>
-            <%= tag.span t(".has_issues") %>
-            <%= link_to t(".troubleshoot"), issue_path(account.issues.first), class: "underline", data: { turbo_frame: :drawer } %>
-          </div>
+        <% if account.scheduled_for_deletion? %>
+          <p class="text-sm font-medium text-gray-900">
+            <span>
+              <%= account.name %>
+            </span>
+            <span class="text-red-500 animate-pulse">
+              (deletion in progress...)
+            </span>
+          </p>
+        <% else %>
+          <%= link_to account.name, account, class: [(account.is_active ? "text-gray-900" : "text-gray-400"), "text-sm font-medium hover:underline"], data: { turbo_frame: "_top" } %>
+          <% if account.has_issues? %>
+            <div class="text-sm flex items-center gap-1 text-error">
+              <%= lucide_icon "alert-octagon", class: "shrink-0 w-4 h-4" %>
+              <%= tag.span t(".has_issues") %>
+              <%= link_to t(".troubleshoot"), issue_path(account.issues.first), class: "underline", data: { turbo_frame: :drawer } %>
+            </div>
+          <% end %>
         <% end %>
       </div>
 
-      <%= link_to edit_account_path(account, return_to: return_to), data: { turbo_frame: :modal }, class: "group-hover/account:flex hidden hover:opacity-80 items-center justify-center" do %>
-        <%= lucide_icon "pencil-line", class: "w-4 h-4 text-gray-500" %>
+      <% unless account.scheduled_for_deletion? %>
+        <%= link_to edit_account_path(account, return_to: return_to), data: { turbo_frame: :modal }, class: "group-hover/account:flex hidden hover:opacity-80 items-center justify-center" do %>
+          <%= lucide_icon "pencil-line", class: "w-4 h-4 text-gray-500" %>
+        <% end %>
       <% end %>
     </div>
     <div class="flex items-center gap-8">
       <p class="text-sm font-medium <%= account.is_active ? "text-gray-900" : "text-gray-400" %>">
         <%= format_money account.balance_money %>
       </p>
 
-      <%= form_with model: account,
+      <% unless account.scheduled_for_deletion? %>
+        <%= form_with model: account,
                     namespace: account.id,
                     data: { controller: "auto-submit-form", turbo_frame: "_top" } do |form| %>
-        <div class="relative inline-block select-none">
-          <%= form.check_box :is_active, { class: "sr-only peer", data: { "auto-submit-form-target": "auto" } } %>
-          <%= form.label :is_active, "&nbsp;".html_safe, class: "maybe-switch" %>
-        </div>
+          <div class="relative inline-block select-none">
+            <%= form.check_box :is_active, { class: "sr-only peer", data: { "auto-submit-form-target": "auto" } } %>
+            <%= form.label :is_active, "&nbsp;".html_safe, class: "maybe-switch" %>
+          </div>
+        <% end %>
       <% end %>
     </div>
   </div>

app/views/plaid_items/_plaid_item.html.erb

@@ -17,7 +17,12 @@
         </div>
 
         <div class="pl-1 text-sm">
-          <%= tag.p plaid_item.name, class: "font-medium text-gray-900" %>
+          <div class="flex items-center gap-2">
+            <%= tag.p plaid_item.name, class: "font-medium text-gray-900" %>
+            <% if plaid_item.scheduled_for_deletion? %>
+              <p class="text-red-500 text-sm animate-pulse">(deletion in progress...)</p>
+            <% end %>
+          </div>
           <% if plaid_item.syncing? %>
             <div class="text-gray-500 flex items-center gap-1">
               <%= lucide_icon "loader", class: "w-4 h-4 animate-pulse" %>
@@ -37,7 +42,7 @@
       </div>
 
       <div class="flex items-center gap-2">
-        <%= button_to sync_plaid_item_path(plaid_item), disabled: plaid_item.syncing?, class: "disabled:text-gray-400 text-gray-900 flex hover:text-gray-800 items-center text-sm font-medium hover:underline" do %>
+        <%= button_to sync_plaid_item_path(plaid_item), disabled: plaid_item.syncing? || plaid_item.scheduled_for_deletion?, class: "disabled:text-gray-400 text-gray-900 flex hover:text-gray-800 items-center text-sm font-medium hover:underline" do %>
           <%= lucide_icon "refresh-cw", class: "w-4 h-4" %>
         <% end %>
 
@@ -46,6 +51,7 @@
             <%= button_to plaid_item_path(plaid_item),
                       method: :delete,
                       class: "block w-full py-2 px-3 space-x-2 text-red-600 hover:bg-red-50 flex items-center rounded-lg",
+                      disabled: plaid_item.syncing? || plaid_item.scheduled_for_deletion?,
                       data: {
                         turbo_confirm: {
                           title: t(".confirm_title"),
@@ -62,15 +68,17 @@
       </div>
     </summary>
 
-    <div class="space-y-4 mt-4">
-      <% if plaid_item.accounts.any? %>
-        <%= render "accounts/index/account_groups", accounts: plaid_item.accounts %>
-      <% else %>
-        <div class="p-4 flex flex-col gap-3 items-center justify-center">
-          <p class="text-gray-900 font-medium text-sm"><%= t(".no_accounts_title") %></p>
-          <p class="text-gray-500 text-sm"><%= t(".no_accounts_description") %></p>
-        </div>
-      <% end %>
-    </div>
+    <% unless plaid_item.scheduled_for_deletion? %>
+      <div class="space-y-4 mt-4">
+        <% if plaid_item.accounts.any? %>
+          <%= render "accounts/index/account_groups", accounts: plaid_item.accounts %>
+        <% else %>
+          <div class="p-4 flex flex-col gap-3 items-center justify-center">
+            <p class="text-gray-900 font-medium text-sm"><%= t(".no_accounts_title") %></p>
+            <p class="text-gray-500 text-sm"><%= t(".no_accounts_description") %></p>
+          </div>
+        <% end %>
+      </div>
+    <% end %>
   </details>
 <% end %>

test/models/account_test.rb

@@ -8,6 +8,12 @@ class AccountTest < ActiveSupport::TestCase
     @family = families(:dylan_family)
   end
 
+  test "can destroy" do
+    assert_difference "Account.count", -1 do
+      @account.destroy
+    end
+  end
+
   test "groups accounts by type" do
     result = @family.accounts.by_group(period: Period.all)
     assets = result[:assets]

test/models/transfer_test.rb

@@ -8,6 +8,12 @@ class TransferTest < ActiveSupport::TestCase
     @inflow = account_transactions(:transfer_in)
   end
 
+  test "transfer destroyed if either transaction is destroyed" do
+    assert_difference [ "Transfer.count", "Account::Transaction.count", "Account::Entry.count" ], -1 do
+      @outflow.entry.destroy
+    end
+  end
+
   test "auto matches transfers" do
     outflow_entry = create_transaction(date: 1.day.ago.to_date, account: accounts(:depository), amount: 500)
     inflow_entry = create_transaction(date: Date.current, account: accounts(:credit_card), amount: -500)