Fix: Purge stale holdings from accounts during sync (#1954)

* Fix: Purge stale holdings from accounts during sync

* Fix typo

* Prevent Plaid holding deletions

app/controllers/account/holdings_controller.rb

@@ -9,9 +9,12 @@ class Account::HoldingsController < ApplicationController
   end
 
   def destroy
+    if @holding.account.plaid_account_id.present?
+      flash[:alert] = "You cannot delete this holding"
+    else
       @holding.destroy_holding_and_entries!
-
       flash[:notice] = t(".success")
+    end
 
     respond_to do |format|
       format.html { redirect_back_or_to account_path(@holding.account) }

app/controllers/plaid_items_controller.rb

@@ -22,7 +22,7 @@ class PlaidItemsController < ApplicationController
     end
 
     respond_to do |format|
-      format.html { redirect_to accounts_path }
+      format.html { redirect_back_or_to accounts_path }
       format.json { head :ok }
     end
   end

app/models/account/syncer.rb

@@ -10,11 +10,11 @@ class Account::Syncer
     holdings = sync_holdings
     balances = sync_balances(holdings)
     account.reload
-    update_account_info(balances, holdings) unless account.plaid_account_id.present?
+    update_account_info(balances, holdings) unless plaid_sync?
     convert_records_to_family_currency(balances, holdings) unless account.currency == account.family.currency
 
     # Enrich if user opted in or if we're syncing transactions from a Plaid account on the hosted app
-    if account.family.data_enrichment_enabled? || (account.plaid_account_id.present? && Rails.application.config.app_mode.hosted?)
+    if account.family.data_enrichment_enabled? || (plaid_sync? && Rails.application.config.app_mode.hosted?)
       account.enrich_data
     else
       Rails.logger.info("Data enrichment is disabled, skipping enrichment for account #{account.id}")
@@ -41,15 +41,13 @@ class Account::Syncer
 
     def sync_holdings
       calculator = Account::HoldingCalculator.new(account)
-      calculated_holdings = calculator.calculate(reverse: account.plaid_account_id.present?)
+      calculated_holdings = calculator.calculate(reverse: plaid_sync?)
 
       current_time = Time.now
 
       Account.transaction do
         load_holdings(calculated_holdings)
-
-        # Purge outdated holdings
-        account.holdings.delete_by("date < ? OR security_id NOT IN (?)", account_start_date, calculated_holdings.map(&:security_id))
+        purge_outdated_holdings unless plaid_sync?
       end
 
       calculated_holdings
@@ -57,13 +55,11 @@ class Account::Syncer
 
     def sync_balances(holdings)
       calculator = Account::BalanceCalculator.new(account, holdings: holdings)
-      calculated_balances = calculator.calculate(reverse: account.plaid_account_id.present?, start_date: start_date)
+      calculated_balances = calculator.calculate(reverse: plaid_sync?, start_date: start_date)
 
       Account.transaction do
         load_balances(calculated_balances)
-
-        # Purge outdated balances
-        account.balances.delete_by("date < ?", account_start_date)
+        purge_outdated_balances
       end
 
       calculated_balances
@@ -131,4 +127,23 @@ class Account::Syncer
         unique_by: %i[account_id security_id date currency]
       )
     end
+
+    def purge_outdated_balances
+      account.balances.delete_by("date < ?", account_start_date)
+    end
+
+    def plaid_sync?
+      account.plaid_account_id.present?
+    end
+
+    def purge_outdated_holdings
+      portfolio_security_ids = account.entries.account_trades.map { |entry| entry.entryable.security_id }.uniq
+
+      # If there are no securities in the portfolio, delete all holdings
+      if portfolio_security_ids.empty?
+        account.holdings.delete_all
+      else
+        account.holdings.delete_by("date < ? OR security_id NOT IN (?)", account_start_date, portfolio_security_ids)
+      end
+    end
 end

app/views/account/holdings/index.html.erb

@@ -21,12 +21,12 @@
       </div>
 
       <div class="rounded-lg bg-white shadow-border-xs">
-        <% if @account.current_holdings.any? %>
         <%= render "account/holdings/cash", account: @account %>
+
         <%= render "account/holdings/ruler" %>
+
+        <% if @account.current_holdings.any? %>
           <%= render partial: "account/holdings/holding", collection: @account.current_holdings, spacer_template: "ruler" %>
-        <% else %>
-          <p class="text-secondary text-sm p-4"><%= t(".no_holdings") %></p>
         <% end %>
       </div>
     </div>

app/views/account/holdings/show.html.erb

@@ -87,6 +87,7 @@
       </div>
     </details>
 
+    <% unless @holding.account.plaid_account_id.present? %>
       <details class="group space-y-2" open>
         <summary class="flex list-none items-center justify-between rounded-xl px-3 py-2 text-xs font-medium uppercase text-secondary bg-gray-25 focus-visible:outline-hidden">
           <h4><%= t(".settings") %></h4>
@@ -108,5 +109,6 @@
           </div>
         </div>
       </details>
+    <% end %>
   </div>
 <% end %>

test/models/account/syncer_test.rb

@@ -17,6 +17,8 @@ class Account::SyncerTest < ActiveSupport::TestCase
     @account.family.update! currency: "USD"
     @account.update! currency: "EUR"
 
+    @account.entries.create!(date: 1.day.ago.to_date, currency: "EUR", amount: 500, name: "Buy AAPL", entryable: Account::Trade.new(security: securities(:aapl), qty: 10, price: 50, currency: "EUR"))
+
     ExchangeRate.create!(date: 1.day.ago.to_date, from_currency: "EUR", to_currency: "USD", rate: 1.2)
     ExchangeRate.create!(date: Date.current, from_currency: "EUR", to_currency: "USD", rate: 2)