From 89cfac4d9546186da412076550b099b4de302206 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Wed, 7 May 2025 21:35:05 -0400 Subject: [PATCH 1/3] Fix Plaid cash balance double counting --- app/models/balance/reverse_calculator.rb | 8 +++++++- app/models/plaid_investment_sync.rb | 18 ++++++++++++++++++ .../models/balance/reverse_calculator_test.rb | 19 +++++++++++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-) diff --git a/app/models/balance/reverse_calculator.rb b/app/models/balance/reverse_calculator.rb index 4c124ced..78a372cf 100644 --- a/app/models/balance/reverse_calculator.rb +++ b/app/models/balance/reverse_calculator.rb @@ -21,7 +21,13 @@ class Balance::ReverseCalculator < Balance::BaseCalculator if valuation.present? @balances << build_balance(date, previous_cash_balance, holdings_value) else - @balances << build_balance(date, current_cash_balance, holdings_value) + # If date is today, we don't distinguish cash vs. total since provider's are inconsistent with treatment + # of the cash component. Instead, just set the balance equal to the "total value" reported by the provider + if date == Date.current + @balances << build_balance(date, account.balance, 0) + else + @balances << build_balance(date, current_cash_balance, holdings_value) + end end current_cash_balance = previous_cash_balance diff --git a/app/models/plaid_investment_sync.rb b/app/models/plaid_investment_sync.rb index 489b0ca1..8a95d1e2 100644 --- a/app/models/plaid_investment_sync.rb +++ b/app/models/plaid_investment_sync.rb @@ -11,6 +11,7 @@ class PlaidInvestmentSync @securities = securities PlaidAccount.transaction do + normalize_cash_balance! sync_transactions! sync_holdings! end @@ -19,6 +20,23 @@ class PlaidInvestmentSync private attr_reader :transactions, :holdings, :securities + # Plaid considers "brokerage cash" and "cash equivalent holdings" to all be part of "cash balance" + # Internally, we DO NOT. + # Maybe clearly distinguishes between "brokerage cash" vs. "holdings (i.e. invested cash)" + # For this reason, we must back out cash + cash equivalent holdings from the reported cash balance to avoid double counting + def normalize_cash_balance! + non_cash_holdings = holdings.reject do |h| + _internal_security, plaid_security = get_security(h.security_id, securities) + plaid_security&.type == "cash" + end + + non_cash_holdings_value = non_cash_holdings.sum { |h| h.quantity * h.institution_price } + + plaid_account.account.update!( + cash_balance: plaid_account.account.cash_balance - non_cash_holdings_value + ) + end + def sync_transactions! transactions.each do |transaction| security, plaid_security = get_security(transaction.security_id, securities) diff --git a/test/models/balance/reverse_calculator_test.rb b/test/models/balance/reverse_calculator_test.rb index 29711702..38ede057 100644 --- a/test/models/balance/reverse_calculator_test.rb +++ b/test/models/balance/reverse_calculator_test.rb @@ -120,4 +120,23 @@ class Balance::ReverseCalculatorTest < ActiveSupport::TestCase assert_equal expected, calculated end + + test "uses provider reported holdings and cash value on current day" do + aapl = securities(:aapl) + + # Implied holdings value of $1,000 from provider + @account.update!(cash_balance: 19000, balance: 20000) + + # Create a holding that differs in value from provider ($2,000 vs. the $1,000 reported by provider) + Holding.create!(date: Date.current, account: @account, security: aapl, qty: 10, price: 100, amount: 2000, currency: "USD") + Holding.create!(date: 1.day.ago.to_date, account: @account, security: aapl, qty: 10, price: 100, amount: 2000, currency: "USD") + + # Today reports the provider value. Yesterday, provider won't give us any data, so we MUST look at the generated holdings value + # to calculate the end balance ($19,000 cash + $2,000 holdings = $21,000 total value) + expected = [ 21000, 20000 ] + + calculated = Balance::ReverseCalculator.new(@account).calculate.sort_by(&:date).map(&:balance) + + assert_equal expected, calculated + end end -- 2.53.0 From 9c7bd588cdb0d9e83efd9efb73f9ef0cda3937ed Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Thu, 8 May 2025 11:24:40 -0400 Subject: [PATCH 2/3] Fix today's cash balance --- app/models/balance/reverse_calculator.rb | 2 +- app/models/plaid_investment_sync.rb | 10 +++++----- app/views/accounts/show/_chart.html.erb | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/models/balance/reverse_calculator.rb b/app/models/balance/reverse_calculator.rb index 78a372cf..6a2ba70b 100644 --- a/app/models/balance/reverse_calculator.rb +++ b/app/models/balance/reverse_calculator.rb @@ -24,7 +24,7 @@ class Balance::ReverseCalculator < Balance::BaseCalculator # If date is today, we don't distinguish cash vs. total since provider's are inconsistent with treatment # of the cash component. Instead, just set the balance equal to the "total value" reported by the provider if date == Date.current - @balances << build_balance(date, account.balance, 0) + @balances << build_balance(date, account.cash_balance, account.balance - account.cash_balance) else @balances << build_balance(date, current_cash_balance, holdings_value) end diff --git a/app/models/plaid_investment_sync.rb b/app/models/plaid_investment_sync.rb index 8a95d1e2..34f0d7e4 100644 --- a/app/models/plaid_investment_sync.rb +++ b/app/models/plaid_investment_sync.rb @@ -25,15 +25,15 @@ class PlaidInvestmentSync # Maybe clearly distinguishes between "brokerage cash" vs. "holdings (i.e. invested cash)" # For this reason, we must back out cash + cash equivalent holdings from the reported cash balance to avoid double counting def normalize_cash_balance! - non_cash_holdings = holdings.reject do |h| - _internal_security, plaid_security = get_security(h.security_id, securities) - plaid_security&.type == "cash" + excludable_cash_holdings = holdings.select do |h| + internal_security, plaid_security = get_security(h.security_id, securities) + internal_security.present? && (plaid_security&.is_cash_equivalent || plaid_security&.type == "cash") end - non_cash_holdings_value = non_cash_holdings.sum { |h| h.quantity * h.institution_price } + excludable_cash_holdings_value = excludable_cash_holdings.sum { |h| h.quantity * h.institution_price } plaid_account.account.update!( - cash_balance: plaid_account.account.cash_balance - non_cash_holdings_value + cash_balance: plaid_account.account.cash_balance - excludable_cash_holdings_value ) end diff --git a/app/views/accounts/show/_chart.html.erb b/app/views/accounts/show/_chart.html.erb index 1661554c..08e67e84 100644 --- a/app/views/accounts/show/_chart.html.erb +++ b/app/views/accounts/show/_chart.html.erb @@ -28,7 +28,7 @@ Period.as_options, { selected: period.key }, data: { "auto-submit-form-target": "auto" }, - class: "bg-container border border-secondary font-medium rounded-lg px-3 py-2 text-sm pr-7 cursor-pointer text-primary focus:outline-hidden focus:ring-0" %> + class: "bg-container border border-secondary rounded-lg px-3 py-2 text-sm pr-7 cursor-pointer text-primary focus:outline-hidden focus:ring-0" %> <% end %> -- 2.53.0 From 993c147aa9510b859b825b77ddeefe76f9e70c1e Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Thu, 8 May 2025 12:20:53 -0400 Subject: [PATCH 3/3] Simplify balance trends in activity view --- app/models/balance/trend_calculator.rb | 84 +++----------------- app/views/accounts/show/_activity.html.erb | 7 +- app/views/transactions/_transaction.html.erb | 27 +++---- app/views/valuations/_valuation.html.erb | 4 +- 4 files changed, 27 insertions(+), 95 deletions(-) diff --git a/app/models/balance/trend_calculator.rb b/app/models/balance/trend_calculator.rb index 5fb8b406..b088d022 100644 --- a/app/models/balance/trend_calculator.rb +++ b/app/models/balance/trend_calculator.rb @@ -5,90 +5,26 @@ class Balance::TrendCalculator BalanceTrend = Struct.new(:trend, :cash, keyword_init: true) - class << self - def for(entries) - return nil if entries.blank? - - account = entries.first.account - - date_range = entries.minmax_by(&:date) - min_entry_date, max_entry_date = date_range.map(&:date) - - # In case view is filtered and there are entry gaps, refetch all entries in range - all_entries = account.entries.where(date: min_entry_date..max_entry_date).chronological.to_a - balances = account.balances.where(date: (min_entry_date - 1.day)..max_entry_date).chronological.to_a - holdings = account.holdings.where(date: (min_entry_date - 1.day)..max_entry_date).to_a - - new(all_entries, balances, holdings) - end - end - - def initialize(entries, balances, holdings) - @entries = entries + def initialize(balances) @balances = balances - @holdings = holdings end - def trend_for(entry) - intraday_balance = nil - intraday_cash_balance = nil + def trend_for(date) + balance = @balances.find { |b| b.date == date } + prior_balance = @balances.find { |b| b.date == date - 1.day } - start_of_day_balance = balances.find { |b| b.date == entry.date - 1.day && b.currency == entry.currency } - end_of_day_balance = balances.find { |b| b.date == entry.date && b.currency == entry.currency } - - return BalanceTrend.new(trend: nil) if start_of_day_balance.blank? || end_of_day_balance.blank? - - todays_holdings_value = holdings.select { |h| h.date == entry.date }.sum(&:amount) - - prior_balance = start_of_day_balance.balance - prior_cash_balance = start_of_day_balance.cash_balance - current_balance = nil - current_cash_balance = nil - - todays_entries = entries.select { |e| e.date == entry.date } - - todays_entries.each_with_index do |e, idx| - if e.valuation? - current_balance = e.amount - current_cash_balance = e.amount - else - multiplier = e.account.liability? ? 1 : -1 - balance_change = e.trade? ? 0 : multiplier * e.amount - cash_change = multiplier * e.amount - - current_balance = prior_balance + balance_change - current_cash_balance = prior_cash_balance + cash_change - end - - if e.id == entry.id - # Final entry should always match the end-of-day balances - if idx == todays_entries.size - 1 - intraday_balance = end_of_day_balance.balance - intraday_cash_balance = end_of_day_balance.cash_balance - else - intraday_balance = current_balance - intraday_cash_balance = current_cash_balance - end - - break - else - prior_balance = current_balance - prior_cash_balance = current_cash_balance - end - end - - return BalanceTrend.new(trend: nil) unless intraday_balance.present? + return BalanceTrend.new(trend: nil) unless balance.present? BalanceTrend.new( trend: Trend.new( - current: Money.new(intraday_balance, entry.currency), - previous: Money.new(prior_balance, entry.currency), - favorable_direction: entry.account.favorable_direction + current: Money.new(balance.balance, balance.currency), + previous: Money.new(prior_balance.balance, balance.currency), + favorable_direction: balance.account.favorable_direction ), - cash: Money.new(intraday_cash_balance, entry.currency), + cash: Money.new(balance.cash_balance, balance.currency), ) end private - attr_reader :entries, :balances, :holdings + attr_reader :balances end diff --git a/app/views/accounts/show/_activity.html.erb b/app/views/accounts/show/_activity.html.erb index 00bf610d..704c6789 100644 --- a/app/views/accounts/show/_activity.html.erb +++ b/app/views/accounts/show/_activity.html.erb @@ -77,10 +77,11 @@
- <% calculator = Balance::TrendCalculator.for(@entries) %> + <% calculator = Balance::TrendCalculator.new(@account.balances) %> + <%= entries_by_date(@entries) do |entries| %> - <% entries.each do |entry| %> - <%= render entry, balance_trend: calculator&.trend_for(entry), view_ctx: "account" %> + <% entries.each_with_index do |entry, index| %> + <%= render entry, balance_trend: index == 0 ? calculator.trend_for(entry.date) : nil, view_ctx: "account" %> <% end %> <% end %>
diff --git a/app/views/transactions/_transaction.html.erb b/app/views/transactions/_transaction.html.erb index 1ad74980..d031134f 100644 --- a/app/views/transactions/_transaction.html.erb +++ b/app/views/transactions/_transaction.html.erb @@ -4,12 +4,11 @@ <%= turbo_frame_tag dom_id(entry) do %> <%= turbo_frame_tag dom_id(transaction) do %> -
"> -
"> +
<%= check_box_tag dom_id(entry, "selection"), disabled: transaction.transfer?, class: "checkbox checkbox--light", @@ -58,7 +57,7 @@ <% end %>
- -