From c1d98fe73bc4fd3307d15b1b7df411c9b8d0510c Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Tue, 15 Jul 2025 11:42:41 -0400 Subject: [PATCH 01/29] Start and end balance anchors for historical account balances (#2455) * Add kind field to valuation * Fix schema conflict * Add kind to valuation * Scaffold opening balance manager * Opening balance manager implementation * Update account import to use opening balance manager + tests * Update account to use opening balance manager * Fix test assertions, usage of current balance manager * Lint fixes * Add Opening Balance manager, add tests to forward calculator * Add credit card to "all cash" designation * Simplify valuation model * Add current balance manager with tests * Add current balance logic to reverse calculator and plaid sync * Tweaks to initial calc logic * Ledger testing helper, tweak assertions for reverse calculator * Update test assertions * Extract balance transformer, simplify calculators * Algo simplifications * Final tweaks to calculators * Cleanup * Fix error, propagate sync errors up to parent * Update migration script, valuation naming --- app/models/account.rb | 25 +- app/models/account/anchorable.rb | 52 +++ app/models/account/balance_updater.rb | 2 +- app/models/account/current_balance_manager.rb | 86 ++++ app/models/account/linkable.rb | 1 + app/models/account/opening_balance_manager.rb | 99 +++++ app/models/account_import.rb | 16 +- app/models/balance/base_calculator.rb | 82 ++++ app/models/balance/forward_calculator.rb | 91 ++-- app/models/balance/reverse_calculator.rb | 114 ++--- app/models/balance/sync_cache.rb | 4 +- app/models/balance/trend_calculator.rb | 2 +- app/models/concerns/syncable.rb | 2 +- app/models/demo/generator.rb | 10 +- app/models/plaid_account/processor.rb | 7 + app/models/valuation.rb | 12 +- app/models/valuation/name.rb | 10 +- .../20250710225721_add_valuation_kind.rb | 5 + db/schema.rb | 3 +- lib/tasks/data_migration.rake | 43 ++ .../credit_cards_controller_test.rb | 4 +- test/controllers/loans_controller_test.rb | 4 +- test/controllers/vehicles_controller_test.rb | 4 +- test/fixtures/imports.yml | 5 + test/fixtures/valuations.yml | 4 +- .../account/current_balance_manager_test.rb | 153 +++++++ test/models/account/entry_test.rb | 2 +- .../account/opening_balance_manager_test.rb | 252 ++++++++++++ test/models/account_import_test.rb | 92 +++++ .../models/balance/forward_calculator_test.rb | 388 ++++++++++++++---- .../models/balance/reverse_calculator_test.rb | 325 ++++++++++----- test/models/plaid_account/processor_test.rb | 83 +++- test/models/valuation/name_test.rb | 45 ++ test/support/entries_test_helper.rb | 41 +- test/support/ledger_testing_helper.rb | 152 +++++++ 35 files changed, 1884 insertions(+), 336 deletions(-) create mode 100644 app/models/account/anchorable.rb create mode 100644 app/models/account/current_balance_manager.rb create mode 100644 app/models/account/opening_balance_manager.rb create mode 100644 app/models/balance/base_calculator.rb create mode 100644 db/migrate/20250710225721_add_valuation_kind.rb create mode 100644 test/models/account/current_balance_manager_test.rb create mode 100644 test/models/account/opening_balance_manager_test.rb create mode 100644 test/models/account_import_test.rb create mode 100644 test/support/ledger_testing_helper.rb diff --git a/app/models/account.rb b/app/models/account.rb index 222062b41d2..1217c5fefe9 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -1,6 +1,5 @@ class Account < ApplicationRecord - include Syncable, Monetizable, Chartable, Linkable, Enrichable - include AASM + include AASM, Syncable, Monetizable, Chartable, Linkable, Enrichable, Anchorable validates :name, :balance, :currency, presence: true @@ -59,26 +58,14 @@ class << self def create_and_sync(attributes) attributes[:accountable_attributes] ||= {} # Ensure accountable is created, even if empty account = new(attributes.merge(cash_balance: attributes[:balance])) - initial_balance = attributes.dig(:accountable_attributes, :initial_balance)&.to_d || 0 + initial_balance = attributes.dig(:accountable_attributes, :initial_balance)&.to_d transaction do - # Create 2 valuations for new accounts to establish a value history for users to see - account.entries.build( - name: Valuation.build_current_anchor_name(account.accountable_type), - date: Date.current, - amount: account.balance, - currency: account.currency, - entryable: Valuation.new - ) - account.entries.build( - name: Valuation.build_opening_anchor_name(account.accountable_type), - date: 1.day.ago.to_date, - amount: initial_balance, - currency: account.currency, - entryable: Valuation.new - ) - account.save! + + manager = Account::OpeningBalanceManager.new(account) + result = manager.set_opening_balance(balance: initial_balance || account.balance) + raise result.error if result.error end account.sync_later diff --git a/app/models/account/anchorable.rb b/app/models/account/anchorable.rb new file mode 100644 index 00000000000..750fb067219 --- /dev/null +++ b/app/models/account/anchorable.rb @@ -0,0 +1,52 @@ +# All accounts are "anchored" with start/end valuation records, with transactions, +# trades, and reconciliations between them. +module Account::Anchorable + extend ActiveSupport::Concern + + included do + include Monetizable + + monetize :opening_balance + end + + def set_opening_anchor_balance(**opts) + opening_balance_manager.set_opening_balance(**opts) + end + + def opening_anchor_date + opening_balance_manager.opening_date + end + + def opening_anchor_balance + opening_balance_manager.opening_balance + end + + def has_opening_anchor? + opening_balance_manager.has_opening_anchor? + end + + def set_current_anchor_balance(balance) + current_balance_manager.set_current_balance(balance) + end + + def current_anchor_balance + current_balance_manager.current_balance + end + + def current_anchor_date + current_balance_manager.current_date + end + + def has_current_anchor? + current_balance_manager.has_current_anchor? + end + + private + def opening_balance_manager + @opening_balance_manager ||= Account::OpeningBalanceManager.new(self) + end + + def current_balance_manager + @current_balance_manager ||= Account::CurrentBalanceManager.new(self) + end +end diff --git a/app/models/account/balance_updater.rb b/app/models/account/balance_updater.rb index a8100d9b279..0eb9a7894ec 100644 --- a/app/models/account/balance_updater.rb +++ b/app/models/account/balance_updater.rb @@ -18,7 +18,7 @@ def update end valuation_entry = account.entries.valuations.find_or_initialize_by(date: date) do |entry| - entry.entryable = Valuation.new + entry.entryable = Valuation.new(kind: "reconciliation") end valuation_entry.amount = balance diff --git a/app/models/account/current_balance_manager.rb b/app/models/account/current_balance_manager.rb new file mode 100644 index 00000000000..243032134f1 --- /dev/null +++ b/app/models/account/current_balance_manager.rb @@ -0,0 +1,86 @@ +class Account::CurrentBalanceManager + InvalidOperation = Class.new(StandardError) + + Result = Struct.new(:success?, :changes_made?, :error, keyword_init: true) + + def initialize(account) + @account = account + end + + def has_current_anchor? + current_anchor_valuation.present? + end + + # Our system should always make sure there is a current anchor, and that it is up to date. + # The fallback is provided for backwards compatibility, but should not be relied on since account.balance is a "cached/derived" value. + def current_balance + if current_anchor_valuation + current_anchor_valuation.entry.amount + else + Rails.logger.warn "No current balance anchor found for account #{account.id}. Using cached balance instead, which may be out of date." + account.balance + end + end + + def current_date + if current_anchor_valuation + current_anchor_valuation.entry.date + else + Date.current + end + end + + def set_current_balance(balance) + # A current balance anchor implies there is an external data source that will keep it updated. Since manual accounts + # are tracked by the user, a current balance anchor is not appropriate. + raise InvalidOperation, "Manual accounts cannot set current balance anchor. Set opening balance or use a reconciliation instead." if account.manual? + + if current_anchor_valuation + changes_made = update_current_anchor(balance) + Result.new(success?: true, changes_made?: changes_made, error: nil) + else + create_current_anchor(balance) + Result.new(success?: true, changes_made?: true, error: nil) + end + end + + private + attr_reader :account + + def current_anchor_valuation + @current_anchor_valuation ||= account.valuations.current_anchor.includes(:entry).first + end + + def create_current_anchor(balance) + account.entries.create!( + date: Date.current, + name: Valuation.build_current_anchor_name(account.accountable_type), + amount: balance, + currency: account.currency, + entryable: Valuation.new(kind: "current_anchor") + ) + end + + def update_current_anchor(balance) + changes_made = false + + ActiveRecord::Base.transaction do + # Update associated entry attributes + entry = current_anchor_valuation.entry + + if entry.amount != balance + entry.amount = balance + changes_made = true + end + + if entry.date != Date.current + entry.date = Date.current + changes_made = true + end + + entry.save! if entry.changed? + end + + changes_made + end +end diff --git a/app/models/account/linkable.rb b/app/models/account/linkable.rb index ee0871bde21..76b41bb1011 100644 --- a/app/models/account/linkable.rb +++ b/app/models/account/linkable.rb @@ -15,4 +15,5 @@ def linked? def unlinked? !linked? end + alias_method :manual?, :unlinked? end diff --git a/app/models/account/opening_balance_manager.rb b/app/models/account/opening_balance_manager.rb new file mode 100644 index 00000000000..95597cdaaf3 --- /dev/null +++ b/app/models/account/opening_balance_manager.rb @@ -0,0 +1,99 @@ +class Account::OpeningBalanceManager + Result = Struct.new(:success?, :changes_made?, :error, keyword_init: true) + + def initialize(account) + @account = account + end + + def has_opening_anchor? + opening_anchor_valuation.present? + end + + # Most accounts should have an opening anchor. If not, we derive the opening date from the oldest entry date + def opening_date + return opening_anchor_valuation.entry.date if opening_anchor_valuation.present? + + [ + account.entries.valuations.order(:date).first&.date, + account.entries.where.not(entryable_type: "Valuation").order(:date).first&.date&.prev_day + ].compact.min || Date.current + end + + def opening_balance + opening_anchor_valuation&.entry&.amount || 0 + end + + def set_opening_balance(balance:, date: nil) + resolved_date = date || default_date + + # Validate date is before oldest entry + if date && oldest_entry_date && resolved_date >= oldest_entry_date + return Result.new(success?: false, changes_made?: false, error: "Opening balance date must be before the oldest entry date") + end + + if opening_anchor_valuation.nil? + create_opening_anchor( + balance: balance, + date: resolved_date + ) + Result.new(success?: true, changes_made?: true, error: nil) + else + changes_made = update_opening_anchor(balance: balance, date: date) + Result.new(success?: true, changes_made?: changes_made, error: nil) + end + end + + private + attr_reader :account + + def opening_anchor_valuation + @opening_anchor_valuation ||= account.valuations.opening_anchor.includes(:entry).first + end + + def oldest_entry_date + @oldest_entry_date ||= account.entries.minimum(:date) + end + + def default_date + if oldest_entry_date + [ oldest_entry_date - 1.day, 2.years.ago.to_date ].min + else + 2.years.ago.to_date + end + end + + def create_opening_anchor(balance:, date:) + account.entries.create!( + date: date, + name: Valuation.build_opening_anchor_name(account.accountable_type), + amount: balance, + currency: account.currency, + entryable: Valuation.new( + kind: "opening_anchor" + ) + ) + end + + def update_opening_anchor(balance:, date: nil) + changes_made = false + + ActiveRecord::Base.transaction do + # Update associated entry attributes + entry = opening_anchor_valuation.entry + + if entry.amount != balance + entry.amount = balance + changes_made = true + end + + if date.present? && entry.date != date + entry.date = date + changes_made = true + end + + entry.save! if entry.changed? + end + + changes_made + end +end diff --git a/app/models/account_import.rb b/app/models/account_import.rb index 4836ce550f7..0ffdcec4cd4 100644 --- a/app/models/account_import.rb +++ b/app/models/account_import.rb @@ -1,4 +1,6 @@ class AccountImport < Import + OpeningBalanceError = Class.new(StandardError) + def import! transaction do rows.each do |row| @@ -15,13 +17,13 @@ def import! account.save! - account.entries.create!( - amount: row.amount, - currency: row.currency, - date: 2.years.ago.to_date, - name: Valuation.build_opening_anchor_name(account.accountable_type), - entryable: Valuation.new - ) + manager = Account::OpeningBalanceManager.new(account) + result = manager.set_opening_balance(balance: row.amount.to_d) + + # Re-raise since we should never have an error here + if result.error + raise OpeningBalanceError, result.error + end end end end diff --git a/app/models/balance/base_calculator.rb b/app/models/balance/base_calculator.rb new file mode 100644 index 00000000000..3360bcec4b6 --- /dev/null +++ b/app/models/balance/base_calculator.rb @@ -0,0 +1,82 @@ +class Balance::BaseCalculator + attr_reader :account + + def initialize(account) + @account = account + end + + def calculate + raise NotImplementedError, "Subclasses must implement this method" + end + + private + def sync_cache + @sync_cache ||= Balance::SyncCache.new(account) + end + + def holdings_value_for_date(date) + holdings = sync_cache.get_holdings(date) + holdings.sum(&:amount) + end + + def derive_cash_balance_on_date_from_total(total_balance:, date:) + if balance_type == :investment + total_balance - holdings_value_for_date(date) + elsif balance_type == :cash + total_balance + else + 0 + end + end + + def derive_cash_balance(cash_balance, date) + entries = sync_cache.get_entries(date) + + if balance_type == :non_cash + 0 + else + cash_balance + signed_entry_flows(entries) + end + end + + def derive_non_cash_balance(non_cash_balance, date, direction: :forward) + entries = sync_cache.get_entries(date) + # Loans are a special case (loan payment reducing principal, which is non-cash) + if balance_type == :non_cash && account.accountable_type == "Loan" + non_cash_balance + signed_entry_flows(entries) + elsif balance_type == :investment + # For reverse calculations, we need the previous day's holdings + target_date = direction == :forward ? date : date.prev_day + holdings_value_for_date(target_date) + else + non_cash_balance + end + end + + def signed_entry_flows(entries) + raise NotImplementedError, "Directional calculators must implement this method" + end + + def balance_type + case account.accountable_type + when "Depository", "CreditCard" + :cash + when "Property", "Vehicle", "OtherAsset", "Loan", "OtherLiability" + :non_cash + when "Investment", "Crypto" + :investment + else + raise "Unknown account type: #{account.accountable_type}" + end + end + + def build_balance(date:, cash_balance:, non_cash_balance:) + Balance.new( + account_id: account.id, + date: date, + balance: non_cash_balance + cash_balance, + cash_balance: cash_balance, + currency: account.currency + ) + end +end diff --git a/app/models/balance/forward_calculator.rb b/app/models/balance/forward_calculator.rb index 4e6f2d5c4d8..bd9272b7a43 100644 --- a/app/models/balance/forward_calculator.rb +++ b/app/models/balance/forward_calculator.rb @@ -1,61 +1,66 @@ -class Balance::ForwardCalculator - attr_reader :account - - def initialize(account) - @account = account - end - +class Balance::ForwardCalculator < Balance::BaseCalculator def calculate Rails.logger.tagged("Balance::ForwardCalculator") do - calculate_balances - end - end - - private - def calculate_balances - current_cash_balance = 0 - next_cash_balance = nil - - @balances = [] + start_cash_balance = derive_cash_balance_on_date_from_total( + total_balance: account.opening_anchor_balance, + date: account.opening_anchor_date + ) + start_non_cash_balance = account.opening_anchor_balance - start_cash_balance - account.start_date.upto(Date.current).each do |date| - entries = sync_cache.get_entries(date) - holdings = sync_cache.get_holdings(date) - holdings_value = holdings.sum(&:amount) - valuation = sync_cache.get_valuation(date) + calc_start_date.upto(calc_end_date).map do |date| + valuation = sync_cache.get_reconciliation_valuation(date) - next_cash_balance = if valuation - valuation.amount - holdings_value + if valuation + end_cash_balance = derive_cash_balance_on_date_from_total( + total_balance: valuation.amount, + date: date + ) + end_non_cash_balance = valuation.amount - end_cash_balance else - calculate_next_balance(current_cash_balance, entries, direction: :forward) + end_cash_balance = derive_end_cash_balance(start_cash_balance: start_cash_balance, date: date) + end_non_cash_balance = derive_end_non_cash_balance(start_non_cash_balance: start_non_cash_balance, date: date) end - @balances << build_balance(date, next_cash_balance, holdings_value) + output_balance = build_balance( + date: date, + cash_balance: end_cash_balance, + non_cash_balance: end_non_cash_balance + ) + + # Set values for the next iteration + start_cash_balance = end_cash_balance + start_non_cash_balance = end_non_cash_balance - current_cash_balance = next_cash_balance + output_balance end + end + end - @balances + private + def calc_start_date + account.opening_anchor_date end - def sync_cache - @sync_cache ||= Balance::SyncCache.new(account) + def calc_end_date + [ account.entries.order(:date).last&.date, account.holdings.order(:date).last&.date ].compact.max || Date.current end - def build_balance(date, cash_balance, holdings_value) - Balance.new( - account_id: account.id, - date: date, - balance: holdings_value + cash_balance, - cash_balance: cash_balance, - currency: account.currency - ) + # Negative entries amount on an "asset" account means, "account value has increased" + # Negative entries amount on a "liability" account means, "account debt has decreased" + # Positive entries amount on an "asset" account means, "account value has decreased" + # Positive entries amount on a "liability" account means, "account debt has increased" + def signed_entry_flows(entries) + entry_flows = entries.sum(&:amount) + account.asset? ? -entry_flows : entry_flows + end + + # Derives cash balance, starting from the start-of-day, applying entries in forward to get the end-of-day balance + def derive_end_cash_balance(start_cash_balance:, date:) + derive_cash_balance(start_cash_balance, date) end - def calculate_next_balance(prior_balance, transactions, direction: :forward) - flows = transactions.sum(&:amount) - negated = direction == :forward ? account.asset? : account.liability? - flows *= -1 if negated - prior_balance + flows + # Derives non-cash balance, starting from the start-of-day, applying entries in forward to get the end-of-day balance + def derive_end_non_cash_balance(start_non_cash_balance:, date:) + derive_non_cash_balance(start_non_cash_balance, date, direction: :forward) end end diff --git a/app/models/balance/reverse_calculator.rb b/app/models/balance/reverse_calculator.rb index 52a05608486..1e75d5e44ee 100644 --- a/app/models/balance/reverse_calculator.rb +++ b/app/models/balance/reverse_calculator.rb @@ -1,71 +1,79 @@ -class Balance::ReverseCalculator - attr_reader :account - - def initialize(account) - @account = account - end - +class Balance::ReverseCalculator < Balance::BaseCalculator def calculate Rails.logger.tagged("Balance::ReverseCalculator") do - calculate_balances - end - end - - private - def calculate_balances - current_cash_balance = account.cash_balance - previous_cash_balance = nil + # Since it's a reverse sync, we're starting with the "end of day" balance components and + # calculating backwards to derive the "start of day" balance components. + end_cash_balance = derive_cash_balance_on_date_from_total( + total_balance: account.current_anchor_balance, + date: account.current_anchor_date + ) + end_non_cash_balance = account.current_anchor_balance - end_cash_balance - @balances = [] + # Calculates in reverse-chronological order (End of day -> Start of day) + account.current_anchor_date.downto(account.opening_anchor_date).map do |date| + if use_opening_anchor_for_date?(date) + end_cash_balance = derive_cash_balance_on_date_from_total( + total_balance: account.opening_anchor_balance, + date: date + ) + end_non_cash_balance = account.opening_anchor_balance - end_cash_balance - Date.current.downto(account.start_date).map do |date| - entries = sync_cache.get_entries(date) - holdings = sync_cache.get_holdings(date) - holdings_value = holdings.sum(&:amount) - valuation = sync_cache.get_valuation(date) + start_cash_balance = end_cash_balance + start_non_cash_balance = end_non_cash_balance - previous_cash_balance = if valuation - valuation.amount - holdings_value + build_balance( + date: date, + cash_balance: end_cash_balance, + non_cash_balance: end_non_cash_balance + ) else - calculate_next_balance(current_cash_balance, entries, direction: :reverse) - end + start_cash_balance = derive_start_cash_balance(end_cash_balance: end_cash_balance, date: date) + start_non_cash_balance = derive_start_non_cash_balance(end_non_cash_balance: end_non_cash_balance, date: date) - if valuation.present? - @balances << build_balance(date, previous_cash_balance, holdings_value) - else - # 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.cash_balance, account.balance - account.cash_balance) - else - @balances << build_balance(date, current_cash_balance, holdings_value) - end - end + # Even though we've just calculated "start" balances, we set today equal to end of day, then use those + # in our next iteration (slightly confusing, but just the nature of a "reverse" sync) + output_balance = build_balance( + date: date, + cash_balance: end_cash_balance, + non_cash_balance: end_non_cash_balance + ) + + end_cash_balance = start_cash_balance + end_non_cash_balance = start_non_cash_balance - current_cash_balance = previous_cash_balance + output_balance + end end + end + end + + private - @balances + # Negative entries amount on an "asset" account means, "account value has increased" + # Negative entries amount on a "liability" account means, "account debt has decreased" + # Positive entries amount on an "asset" account means, "account value has decreased" + # Positive entries amount on a "liability" account means, "account debt has increased" + def signed_entry_flows(entries) + entry_flows = entries.sum(&:amount) + account.asset? ? entry_flows : -entry_flows end - def sync_cache - @sync_cache ||= Balance::SyncCache.new(account) + # Reverse syncs are a bit different than forward syncs because we do not allow "reconciliation" valuations + # to be used at all. This is primarily to keep the code and the UI easy to understand. For a more detailed + # explanation, see the test suite. + def use_opening_anchor_for_date?(date) + account.has_opening_anchor? && date == account.opening_anchor_date end - def build_balance(date, cash_balance, holdings_value) - Balance.new( - account_id: account.id, - date: date, - balance: holdings_value + cash_balance, - cash_balance: cash_balance, - currency: account.currency - ) + # Alias method, for algorithmic clarity + # Derives cash balance, starting from the end-of-day, applying entries in reverse to get the start-of-day balance + def derive_start_cash_balance(end_cash_balance:, date:) + derive_cash_balance(end_cash_balance, date) end - def calculate_next_balance(prior_balance, transactions, direction: :forward) - flows = transactions.sum(&:amount) - negated = direction == :forward ? account.asset? : account.liability? - flows *= -1 if negated - prior_balance + flows + # Alias method, for algorithmic clarity + # Derives non-cash balance, starting from the end-of-day, applying entries in reverse to get the start-of-day balance + def derive_start_non_cash_balance(end_non_cash_balance:, date:) + derive_non_cash_balance(end_non_cash_balance, date, direction: :reverse) end end diff --git a/app/models/balance/sync_cache.rb b/app/models/balance/sync_cache.rb index aed2b64e72b..be2eaa19ff0 100644 --- a/app/models/balance/sync_cache.rb +++ b/app/models/balance/sync_cache.rb @@ -3,8 +3,8 @@ def initialize(account) @account = account end - def get_valuation(date) - converted_entries.find { |e| e.date == date && e.valuation? } + def get_reconciliation_valuation(date) + converted_entries.find { |e| e.date == date && e.valuation? && e.valuation.reconciliation? } end def get_holdings(date) diff --git a/app/models/balance/trend_calculator.rb b/app/models/balance/trend_calculator.rb index b088d022cd6..990a8339229 100644 --- a/app/models/balance/trend_calculator.rb +++ b/app/models/balance/trend_calculator.rb @@ -18,7 +18,7 @@ def trend_for(date) BalanceTrend.new( trend: Trend.new( current: Money.new(balance.balance, balance.currency), - previous: Money.new(prior_balance.balance, balance.currency), + previous: prior_balance.present? ? Money.new(prior_balance.balance, balance.currency) : nil, favorable_direction: balance.account.favorable_direction ), cash: Money.new(balance.cash_balance, balance.currency), diff --git a/app/models/concerns/syncable.rb b/app/models/concerns/syncable.rb index 739d53812f9..298aa620759 100644 --- a/app/models/concerns/syncable.rb +++ b/app/models/concerns/syncable.rb @@ -47,7 +47,7 @@ def broadcast_sync_complete end def sync_error - latest_sync&.error + latest_sync&.error || latest_sync&.children&.map(&:error)&.compact&.first end def last_synced_at diff --git a/app/models/demo/generator.rb b/app/models/demo/generator.rb index e12e5b852a3..b530b273691 100644 --- a/app/models/demo/generator.rb +++ b/app/models/demo/generator.rb @@ -1174,7 +1174,7 @@ def reconcile_balances!(family) # Property valuations (these accounts are valued, not transaction-driven) @home.entries.create!( - entryable: Valuation.new, + entryable: Valuation.new(kind: "current_anchor"), amount: 350_000, name: Valuation.build_current_anchor_name(@home.accountable_type), currency: "USD", @@ -1183,7 +1183,7 @@ def reconcile_balances!(family) # Vehicle valuations (these depreciate over time) @honda_accord.entries.create!( - entryable: Valuation.new, + entryable: Valuation.new(kind: "current_anchor"), amount: 18_000, name: Valuation.build_current_anchor_name(@honda_accord.accountable_type), currency: "USD", @@ -1191,7 +1191,7 @@ def reconcile_balances!(family) ) @tesla_model3.entries.create!( - entryable: Valuation.new, + entryable: Valuation.new(kind: "current_anchor"), amount: 4_500, name: Valuation.build_current_anchor_name(@tesla_model3.accountable_type), currency: "USD", @@ -1199,7 +1199,7 @@ def reconcile_balances!(family) ) @jewelry.entries.create!( - entryable: Valuation.new, + entryable: Valuation.new(kind: "reconciliation"), amount: 2000, name: Valuation.build_reconciliation_name(@jewelry.accountable_type), currency: "USD", @@ -1207,7 +1207,7 @@ def reconcile_balances!(family) ) @personal_loc.entries.create!( - entryable: Valuation.new, + entryable: Valuation.new(kind: "reconciliation"), amount: 800, name: Valuation.build_reconciliation_name(@personal_loc.accountable_type), currency: "USD", diff --git a/app/models/plaid_account/processor.rb b/app/models/plaid_account/processor.rb index 6c999911b8b..5b16f90dbae 100644 --- a/app/models/plaid_account/processor.rb +++ b/app/models/plaid_account/processor.rb @@ -51,6 +51,13 @@ def process_account! ) account.save! + + # Create or update the current balance anchor valuation for event-sourced ledger + # Note: This is a partial implementation. In the future, we'll introduce HoldingValuation + # to properly track the holdings vs. cash breakdown, but for now we're only tracking + # the total balance in the current anchor. The cash_balance field on the account model + # is still being used for the breakdown. + account.set_current_anchor_balance(balance_calculator.balance) end end diff --git a/app/models/valuation.rb b/app/models/valuation.rb index fe3febcc2ba..f80226be31f 100644 --- a/app/models/valuation.rb +++ b/app/models/valuation.rb @@ -1,6 +1,12 @@ class Valuation < ApplicationRecord include Entryable + enum :kind, { + reconciliation: "reconciliation", + opening_anchor: "opening_anchor", + current_anchor: "current_anchor" + }, validate: true, default: "reconciliation" + class << self def build_reconciliation_name(accountable_type) Valuation::Name.new("reconciliation", accountable_type).to_s @@ -14,10 +20,4 @@ def build_current_anchor_name(accountable_type) Valuation::Name.new("current_anchor", accountable_type).to_s end end - - # TODO: Remove this method when `kind` column is added to valuations table - # This is a temporary implementation until the database migration is complete - def kind - "reconciliation" - end end diff --git a/app/models/valuation/name.rb b/app/models/valuation/name.rb index 6b442876183..79398cdeaaf 100644 --- a/app/models/valuation/name.rb +++ b/app/models/valuation/name.rb @@ -20,11 +20,11 @@ def to_s def opening_anchor_name case accountable_type - when "Property" + when "Property", "Vehicle" "Original purchase price" when "Loan" "Original principal" - when "Investment" + when "Investment", "Crypto", "OtherAsset" "Opening account value" else "Opening balance" @@ -33,11 +33,11 @@ def opening_anchor_name def current_anchor_name case accountable_type - when "Property" + when "Property", "Vehicle" "Current market value" when "Loan" "Current loan balance" - when "Investment" + when "Investment", "Crypto", "OtherAsset" "Current account value" else "Current balance" @@ -46,7 +46,7 @@ def current_anchor_name def recon_name case accountable_type - when "Property", "Investment" + when "Property", "Investment", "Vehicle", "Crypto", "OtherAsset" "Manual value update" when "Loan" "Manual principal update" diff --git a/db/migrate/20250710225721_add_valuation_kind.rb b/db/migrate/20250710225721_add_valuation_kind.rb new file mode 100644 index 00000000000..e6b80702ce2 --- /dev/null +++ b/db/migrate/20250710225721_add_valuation_kind.rb @@ -0,0 +1,5 @@ +class AddValuationKind < ActiveRecord::Migration[7.2] + def change + add_column :valuations, :kind, :string, default: "reconciliation", null: false + end +end diff --git a/db/schema.rb b/db/schema.rb index 8484bac88e1..56d7ba097b1 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.2].define(version: 2025_07_02_173231) do +ActiveRecord::Schema[7.2].define(version: 2025_07_10_225721) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -779,6 +779,7 @@ t.datetime "created_at", null: false t.datetime "updated_at", null: false t.jsonb "locked_attributes", default: {} + t.string "kind", default: "reconciliation", null: false end create_table "vehicles", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| diff --git a/lib/tasks/data_migration.rake b/lib/tasks/data_migration.rake index 509e033c866..6d3a14fd798 100644 --- a/lib/tasks/data_migration.rake +++ b/lib/tasks/data_migration.rake @@ -111,4 +111,47 @@ namespace :data_migration do puts "✅ Duplicate security migration complete." end + + desc "Migrate account valuation anchors" + # 2025-07-10: Set opening_anchor kinds for valuations to support event-sourced ledger model. + # Manual accounts get their oldest valuation marked as opening_anchor, which acts as the + # starting balance for the account. Current anchors are only used for Plaid accounts. + task migrate_account_valuation_anchors: :environment do + puts "==> Migrating account valuation anchors..." + + manual_accounts = Account.manual.includes(valuations: :entry) + total_accounts = manual_accounts.count + accounts_processed = 0 + opening_anchors_set = 0 + + manual_accounts.find_each do |account| + accounts_processed += 1 + + # Find oldest account entry + oldest_entry = account.entries + .order("date ASC, created_at ASC") + .first + + # Check if it's a valuation that isn't already an anchor + if oldest_entry && oldest_entry.valuation? + derived_valuation_name = Valuation.build_opening_anchor_name(account.accountable_type) + + Account.transaction do + oldest_entry.valuation.update!(kind: "opening_anchor") + oldest_entry.update!(name: derived_valuation_name) + end + opening_anchors_set += 1 + end + + if accounts_processed % 100 == 0 + puts "[#{accounts_processed}/#{total_accounts}] Processed #{accounts_processed} accounts..." + end + rescue => e + puts "ERROR processing account #{account.id}: #{e.message}" + end + + puts "✅ Account valuation anchor migration complete." + puts " Processed: #{accounts_processed} accounts" + puts " Opening anchors set: #{opening_anchors_set}" + end end diff --git a/test/controllers/credit_cards_controller_test.rb b/test/controllers/credit_cards_controller_test.rb index 6a270156c03..5fb0ec524b8 100644 --- a/test/controllers/credit_cards_controller_test.rb +++ b/test/controllers/credit_cards_controller_test.rb @@ -11,8 +11,8 @@ class CreditCardsControllerTest < ActionDispatch::IntegrationTest test "creates with credit card details" do assert_difference -> { Account.count } => 1, -> { CreditCard.count } => 1, - -> { Valuation.count } => 2, - -> { Entry.count } => 2 do + -> { Valuation.count } => 1, + -> { Entry.count } => 1 do post credit_cards_path, params: { account: { name: "New Credit Card", diff --git a/test/controllers/loans_controller_test.rb b/test/controllers/loans_controller_test.rb index ec590363cc9..e12a27057c1 100644 --- a/test/controllers/loans_controller_test.rb +++ b/test/controllers/loans_controller_test.rb @@ -11,8 +11,8 @@ class LoansControllerTest < ActionDispatch::IntegrationTest test "creates with loan details" do assert_difference -> { Account.count } => 1, -> { Loan.count } => 1, - -> { Valuation.count } => 2, - -> { Entry.count } => 2 do + -> { Valuation.count } => 1, + -> { Entry.count } => 1 do post loans_path, params: { account: { name: "New Loan", diff --git a/test/controllers/vehicles_controller_test.rb b/test/controllers/vehicles_controller_test.rb index bb7df9c68c6..37cea18d867 100644 --- a/test/controllers/vehicles_controller_test.rb +++ b/test/controllers/vehicles_controller_test.rb @@ -11,8 +11,8 @@ class VehiclesControllerTest < ActionDispatch::IntegrationTest test "creates with vehicle details" do assert_difference -> { Account.count } => 1, -> { Vehicle.count } => 1, - -> { Valuation.count } => 2, - -> { Entry.count } => 2 do + -> { Valuation.count } => 1, + -> { Entry.count } => 1 do post vehicles_path, params: { account: { name: "Vehicle", diff --git a/test/fixtures/imports.yml b/test/fixtures/imports.yml index 366bb6d9d4c..b017253272e 100644 --- a/test/fixtures/imports.yml +++ b/test/fixtures/imports.yml @@ -7,3 +7,8 @@ trade: family: dylan_family type: TradeImport status: pending + +account: + family: dylan_family + type: AccountImport + status: pending diff --git a/test/fixtures/valuations.yml b/test/fixtures/valuations.yml index 21aeae24cb2..27891bd4eb4 100644 --- a/test/fixtures/valuations.yml +++ b/test/fixtures/valuations.yml @@ -1,2 +1,2 @@ -one: { } -two: { } \ No newline at end of file +one: + kind: reconciliation diff --git a/test/models/account/current_balance_manager_test.rb b/test/models/account/current_balance_manager_test.rb new file mode 100644 index 00000000000..d48eb927bce --- /dev/null +++ b/test/models/account/current_balance_manager_test.rb @@ -0,0 +1,153 @@ +require "test_helper" + +class Account::CurrentBalanceManagerTest < ActiveSupport::TestCase + setup do + @connected_account = accounts(:connected) # Connected account - can update current balance + @manual_account = accounts(:depository) # Manual account - cannot update current balance + end + + test "when no existing anchor, creates new anchor" do + manager = Account::CurrentBalanceManager.new(@connected_account) + + assert_difference -> { @connected_account.entries.count } => 1, + -> { @connected_account.valuations.count } => 1 do + result = manager.set_current_balance(1000) + + assert result.success? + assert result.changes_made? + assert_nil result.error + end + + current_anchor = @connected_account.valuations.current_anchor.first + assert_not_nil current_anchor + assert_equal 1000, current_anchor.entry.amount + assert_equal "current_anchor", current_anchor.kind + + entry = current_anchor.entry + assert_equal 1000, entry.amount + assert_equal Date.current, entry.date + assert_equal "Current balance", entry.name # Depository type returns "Current balance" + end + + test "updates existing anchor" do + # First create a current anchor + manager = Account::CurrentBalanceManager.new(@connected_account) + result = manager.set_current_balance(1000) + assert result.success? + + current_anchor = @connected_account.valuations.current_anchor.first + original_id = current_anchor.id + original_entry_id = current_anchor.entry.id + + # Travel to tomorrow to ensure date change + travel_to Date.current + 1.day do + # Now update it + assert_no_difference -> { @connected_account.entries.count } do + assert_no_difference -> { @connected_account.valuations.count } do + result = manager.set_current_balance(2000) + assert result.success? + assert result.changes_made? + end + end + + current_anchor.reload + assert_equal original_id, current_anchor.id # Same valuation record + assert_equal original_entry_id, current_anchor.entry.id # Same entry record + assert_equal 2000, current_anchor.entry.amount + assert_equal Date.current, current_anchor.entry.date # Should be updated to current date + end + end + + test "when manual account, raises InvalidOperation error" do + manager = Account::CurrentBalanceManager.new(@manual_account) + + error = assert_raises(Account::CurrentBalanceManager::InvalidOperation) do + manager.set_current_balance(1000) + end + + assert_equal "Manual accounts cannot set current balance anchor. Set opening balance or use a reconciliation instead.", error.message + + # Verify no current anchor was created + assert_nil @manual_account.valuations.current_anchor.first + end + + test "when no changes made, returns success with no changes made" do + # First create a current anchor + manager = Account::CurrentBalanceManager.new(@connected_account) + result = manager.set_current_balance(1000) + assert result.success? + assert result.changes_made? + + # Try to set the same value on the same date + result = manager.set_current_balance(1000) + + assert result.success? + assert_not result.changes_made? + assert_nil result.error + end + + test "updates only amount when balance changes" do + manager = Account::CurrentBalanceManager.new(@connected_account) + + # Create initial anchor + result = manager.set_current_balance(1000) + assert result.success? + + current_anchor = @connected_account.valuations.current_anchor.first + original_date = current_anchor.entry.date + + # Update only the balance + result = manager.set_current_balance(1500) + assert result.success? + assert result.changes_made? + + current_anchor.reload + assert_equal 1500, current_anchor.entry.amount + assert_equal original_date, current_anchor.entry.date # Date should remain the same if on same day + end + + test "updates date when called on different day" do + manager = Account::CurrentBalanceManager.new(@connected_account) + + # Create initial anchor + result = manager.set_current_balance(1000) + assert result.success? + + current_anchor = @connected_account.valuations.current_anchor.first + original_amount = current_anchor.entry.amount + + # Travel to tomorrow and update with same balance + travel_to Date.current + 1.day do + result = manager.set_current_balance(1000) + assert result.success? + assert result.changes_made? # Should be true because date changed + + current_anchor.reload + assert_equal original_amount, current_anchor.entry.amount + assert_equal Date.current, current_anchor.entry.date # Should be updated to new current date + end + end + + test "current_balance returns balance from current anchor" do + manager = Account::CurrentBalanceManager.new(@connected_account) + + # Create a current anchor + manager.set_current_balance(1500) + + # Should return the anchor's balance + assert_equal 1500, manager.current_balance + + # Update the anchor + manager.set_current_balance(2500) + + # Should return the updated balance + assert_equal 2500, manager.current_balance + end + + test "current_balance falls back to account balance when no anchor exists" do + manager = Account::CurrentBalanceManager.new(@connected_account) + + # When no current anchor exists, should fall back to account.balance + assert_equal @connected_account.balance, manager.current_balance + end +end diff --git a/test/models/account/entry_test.rb b/test/models/account/entry_test.rb index 1cc6b478fc1..dba43ba9bc7 100644 --- a/test/models/account/entry_test.rb +++ b/test/models/account/entry_test.rb @@ -17,7 +17,7 @@ class EntryTest < ActiveSupport::TestCase existing_valuation = entries :valuation new_valuation = Entry.new \ - entryable: Valuation.new, + entryable: Valuation.new(kind: "reconciliation"), account: existing_valuation.account, date: existing_valuation.date, # invalid currency: existing_valuation.currency, diff --git a/test/models/account/opening_balance_manager_test.rb b/test/models/account/opening_balance_manager_test.rb new file mode 100644 index 00000000000..67becb60a22 --- /dev/null +++ b/test/models/account/opening_balance_manager_test.rb @@ -0,0 +1,252 @@ +require "test_helper" + +class Account::OpeningBalanceManagerTest < ActiveSupport::TestCase + setup do + @depository_account = accounts(:depository) + @investment_account = accounts(:investment) + end + + test "when no existing anchor, creates new anchor" do + manager = Account::OpeningBalanceManager.new(@depository_account) + + assert_difference -> { @depository_account.entries.count } => 1, + -> { @depository_account.valuations.count } => 1 do + result = manager.set_opening_balance( + balance: 1000, + date: 1.year.ago.to_date + ) + + assert result.success? + assert result.changes_made? + assert_nil result.error + end + + opening_anchor = @depository_account.valuations.opening_anchor.first + assert_not_nil opening_anchor + assert_equal 1000, opening_anchor.entry.amount + assert_equal "opening_anchor", opening_anchor.kind + + entry = opening_anchor.entry + assert_equal 1000, entry.amount + assert_equal 1.year.ago.to_date, entry.date + assert_equal "Opening balance", entry.name + end + + test "when no existing anchor, creates with provided balance" do + # Test with Depository account (should default to balance) + depository_manager = Account::OpeningBalanceManager.new(@depository_account) + + assert_difference -> { @depository_account.valuations.count } => 1 do + result = depository_manager.set_opening_balance(balance: 2000) + assert result.success? + assert result.changes_made? + end + + depository_anchor = @depository_account.valuations.opening_anchor.first + assert_equal 2000, depository_anchor.entry.amount + + # Test with Investment account (should default to 0) + investment_manager = Account::OpeningBalanceManager.new(@investment_account) + + assert_difference -> { @investment_account.valuations.count } => 1 do + result = investment_manager.set_opening_balance(balance: 5000) + assert result.success? + assert result.changes_made? + end + + investment_anchor = @investment_account.valuations.opening_anchor.first + assert_equal 5000, investment_anchor.entry.amount + end + + test "when no existing anchor and no date provided, provides default based on account type" do + # Test with recent entry (less than 2 years ago) + @depository_account.entries.create!( + date: 30.days.ago.to_date, + name: "Test transaction", + amount: 100, + currency: "USD", + entryable: Transaction.new + ) + + manager = Account::OpeningBalanceManager.new(@depository_account) + + assert_difference -> { @depository_account.valuations.count } => 1 do + result = manager.set_opening_balance(balance: 1500) + assert result.success? + assert result.changes_made? + end + + opening_anchor = @depository_account.valuations.opening_anchor.first + # Default should be MIN(1 day before oldest entry, 2 years ago) = 2 years ago + assert_equal 2.years.ago.to_date, opening_anchor.entry.date + + # Test with old entry (more than 2 years ago) + loan_account = accounts(:loan) + loan_account.entries.create!( + date: 3.years.ago.to_date, + name: "Old transaction", + amount: 100, + currency: "USD", + entryable: Transaction.new + ) + + loan_manager = Account::OpeningBalanceManager.new(loan_account) + + assert_difference -> { loan_account.valuations.count } => 1 do + result = loan_manager.set_opening_balance(balance: 5000) + assert result.success? + assert result.changes_made? + end + + loan_anchor = loan_account.valuations.opening_anchor.first + # Default should be MIN(3 years ago - 1 day, 2 years ago) = 3 years ago - 1 day + assert_equal (3.years.ago.to_date - 1.day), loan_anchor.entry.date + + # Test with account that has no entries + property_account = accounts(:property) + manager_no_entries = Account::OpeningBalanceManager.new(property_account) + + assert_difference -> { property_account.valuations.count } => 1 do + result = manager_no_entries.set_opening_balance(balance: 3000) + assert result.success? + assert result.changes_made? + end + + opening_anchor_no_entries = property_account.valuations.opening_anchor.first + # Default should be 2 years ago when no entries exist + assert_equal 2.years.ago.to_date, opening_anchor_no_entries.entry.date + end + + test "updates existing anchor" do + # First create an opening anchor + manager = Account::OpeningBalanceManager.new(@depository_account) + result = manager.set_opening_balance( + balance: 1000, + date: 6.months.ago.to_date + ) + assert result.success? + + opening_anchor = @depository_account.valuations.opening_anchor.first + original_id = opening_anchor.id + original_entry_id = opening_anchor.entry.id + + # Now update it + assert_no_difference -> { @depository_account.entries.count } do + assert_no_difference -> { @depository_account.valuations.count } do + result = manager.set_opening_balance( + balance: 2000, + date: 8.months.ago.to_date + ) + assert result.success? + assert result.changes_made? + end + end + + opening_anchor.reload + assert_equal original_id, opening_anchor.id # Same valuation record + assert_equal original_entry_id, opening_anchor.entry.id # Same entry record + assert_equal 2000, opening_anchor.entry.amount + assert_equal 2000, opening_anchor.entry.amount + assert_equal 8.months.ago.to_date, opening_anchor.entry.date + end + + test "when existing anchor and no date provided, only update balance" do + # First create an opening anchor + manager = Account::OpeningBalanceManager.new(@depository_account) + result = manager.set_opening_balance( + balance: 1000, + date: 3.months.ago.to_date + ) + assert result.success? + + opening_anchor = @depository_account.valuations.opening_anchor.first + + # Update without providing date + result = manager.set_opening_balance(balance: 1500) + assert result.success? + assert result.changes_made? + + opening_anchor.reload + assert_equal 1500, opening_anchor.entry.amount + end + + test "when existing anchor and updating balance only, preserves original date" do + # First create an opening anchor with specific date + manager = Account::OpeningBalanceManager.new(@depository_account) + original_date = 4.months.ago.to_date + result = manager.set_opening_balance( + balance: 1000, + date: original_date + ) + assert result.success? + + opening_anchor = @depository_account.valuations.opening_anchor.first + + # Update without providing date + result = manager.set_opening_balance(balance: 2500) + assert result.success? + assert result.changes_made? + + opening_anchor.reload + assert_equal 2500, opening_anchor.entry.amount + assert_equal original_date, opening_anchor.entry.date # Should remain unchanged + end + + test "when date is equal to or greater than account's oldest entry, returns error result" do + # Create an entry with a specific date + oldest_date = 60.days.ago.to_date + @depository_account.entries.create!( + date: oldest_date, + name: "Test transaction", + amount: 100, + currency: "USD", + entryable: Transaction.new + ) + + manager = Account::OpeningBalanceManager.new(@depository_account) + + # Try to set opening balance on the same date as oldest entry + result = manager.set_opening_balance( + balance: 1000, + date: oldest_date + ) + + assert_not result.success? + assert_not result.changes_made? + assert_equal "Opening balance date must be before the oldest entry date", result.error + + # Try to set opening balance after the oldest entry + result = manager.set_opening_balance( + balance: 1000, + date: oldest_date + 1.day + ) + + assert_not result.success? + assert_not result.changes_made? + assert_equal "Opening balance date must be before the oldest entry date", result.error + + # Verify no opening anchor was created + assert_nil @depository_account.valuations.opening_anchor.first + end + + test "when no changes made, returns success with no changes made" do + # First create an opening anchor + manager = Account::OpeningBalanceManager.new(@depository_account) + result = manager.set_opening_balance( + balance: 1000, + date: 2.months.ago.to_date + ) + assert result.success? + assert result.changes_made? + + # Try to set the same values + result = manager.set_opening_balance( + balance: 1000, + date: 2.months.ago.to_date + ) + + assert result.success? + assert_not result.changes_made? + assert_nil result.error + end +end diff --git a/test/models/account_import_test.rb b/test/models/account_import_test.rb new file mode 100644 index 00000000000..29204c0fd87 --- /dev/null +++ b/test/models/account_import_test.rb @@ -0,0 +1,92 @@ +require "test_helper" + +class AccountImportTest < ActiveSupport::TestCase + include ActiveJob::TestHelper, ImportInterfaceTest + + setup do + @subject = @import = imports(:account) + end + + test "import creates accounts with valuations" do + import_csv = <<~CSV + type,name,amount,currency + depository,Main Checking,1000.00,USD + depository,Savings Account,5000.00,USD + CSV + + @import.update!( + raw_file_str: import_csv, + entity_type_col_label: "type", + name_col_label: "name", + amount_col_label: "amount", + currency_col_label: "currency" + ) + + @import.generate_rows_from_csv + + # Create mappings for account types + @import.mappings.create! key: "depository", value: "Depository", type: "Import::AccountTypeMapping" + + @import.reload + + # Store initial counts + initial_account_count = Account.count + initial_entry_count = Entry.count + initial_valuation_count = Valuation.count + + # Perform the import + @import.publish + + # Check if import succeeded + if @import.failed? + fail "Import failed with error: #{@import.error}" + end + + assert_equal "complete", @import.status + + # Check the differences + assert_equal initial_account_count + 2, Account.count, "Expected 2 new accounts" + assert_equal initial_entry_count + 2, Entry.count, "Expected 2 new entries" + assert_equal initial_valuation_count + 2, Valuation.count, "Expected 2 new valuations" + + # Verify accounts were created correctly + accounts = @import.accounts.order(:name) + assert_equal [ "Main Checking", "Savings Account" ], accounts.pluck(:name) + assert_equal [ 1000.00, 5000.00 ], accounts.map { |a| a.balance.to_f } + + # Verify valuations were created with correct fields + accounts.each do |account| + valuation = account.valuations.last + assert_not_nil valuation + assert_equal "opening_anchor", valuation.kind + assert_equal account.balance, valuation.entry.amount + end + end + + test "column_keys returns expected keys" do + assert_equal %i[entity_type name amount currency], @import.column_keys + end + + test "required_column_keys returns expected keys" do + assert_equal %i[name amount], @import.required_column_keys + end + + test "mapping_steps returns account type mapping" do + assert_equal [ Import::AccountTypeMapping ], @import.mapping_steps + end + + test "dry_run returns expected counts" do + @import.rows.create!( + entity_type: "depository", + name: "Test Account", + amount: "1000.00", + currency: "USD" + ) + + assert_equal({ accounts: 1 }, @import.dry_run) + end + + test "max_row_count is limited to 50" do + assert_equal 50, @import.max_row_count + end +end diff --git a/test/models/balance/forward_calculator_test.rb b/test/models/balance/forward_calculator_test.rb index 05215c259f7..b6eb2d119a0 100644 --- a/test/models/balance/forward_calculator_test.rb +++ b/test/models/balance/forward_calculator_test.rb @@ -1,129 +1,349 @@ require "test_helper" +# The "forward calculator" is used for all **manual** accounts where balance tracking is done through entries and NOT from an external data provider. class Balance::ForwardCalculatorTest < ActiveSupport::TestCase - include EntriesTestHelper + include LedgerTestingHelper - setup do - @account = families(:empty).accounts.create!( - name: "Test", - balance: 20000, - cash_balance: 20000, - currency: "USD", - accountable: Investment.new + # ------------------------------------------------------------------------------------------------ + # General tests for all account types + # ------------------------------------------------------------------------------------------------ + + # When syncing forwards, we don't care about the account balance. We generate everything based on entries, starting from 0. + test "no entries sync" do + account = create_account_with_ledger( + account: { type: Depository, balance: 20000, cash_balance: 20000, currency: "USD" }, + entries: [] ) - end - test "balance generation respects user timezone and last generated date is current user date" do - # Simulate user in EST timezone - Time.use_zone("America/New_York") do - # Set current time to 1am UTC on Jan 5, 2025 - # This would be 8pm EST on Jan 4, 2025 (user's time, and the last date we should generate balances for) - travel_to Time.utc(2025, 01, 05, 1, 0, 0) + assert_equal 0, account.balances.count + + calculated = Balance::ForwardCalculator.new(account).calculate + + assert_calculated_ledger_balances( + calculated_data: calculated, + expected_balances: [ + [ Date.current, { balance: 0, cash_balance: 0 } ] + ] + ) + end - # Create a valuation for Jan 3, 2025 - create_valuation(account: @account, date: "2025-01-03", amount: 17000) + # Our system ensures all manual accounts have an opening anchor (for UX), but we should be able to handle a missing anchor by starting at 0 (i.e. "fresh account with no history") + test "account without opening anchor starts at zero balance" do + account = create_account_with_ledger( + account: { type: Depository, balance: 20000, cash_balance: 20000, currency: "USD" }, + entries: [ + { type: "transaction", date: 2.days.ago.to_date, amount: -1000 } + ] + ) - expected = [ [ "2025-01-02", 0 ], [ "2025-01-03", 17000 ], [ "2025-01-04", 17000 ] ] - calculated = Balance::ForwardCalculator.new(@account).calculate + calculated = Balance::ForwardCalculator.new(account).calculate - assert_equal expected, calculated.map { |b| [ b.date.to_s, b.balance ] } - end + # Since we start at 0, this transaction (inflow) simply increases balance from 0 -> 1000 + assert_calculated_ledger_balances( + calculated_data: calculated, + expected_balances: [ + [ 3.days.ago.to_date, { balance: 0, cash_balance: 0 } ], + [ 2.days.ago.to_date, { balance: 1000, cash_balance: 1000 } ] + ] + ) end - # When syncing forwards, we don't care about the account balance. We generate everything based on entries, starting from 0. - test "no entries sync" do - assert_equal 0, @account.balances.count + test "reconciliation valuation sets absolute balance before applying subsequent transactions" do + account = create_account_with_ledger( + account: { type: Depository, balance: 20000, cash_balance: 20000, currency: "USD" }, + entries: [ + { type: "reconciliation", date: 3.days.ago.to_date, balance: 18000 }, + { type: "transaction", date: 2.days.ago.to_date, amount: -1000 } + ] + ) - expected = [ 0, 0 ] - calculated = Balance::ForwardCalculator.new(@account).calculate + calculated = Balance::ForwardCalculator.new(account).calculate - assert_equal expected, calculated.map(&:balance) + # First valuation sets balance to 18000, then transaction increases balance to 19000 + assert_calculated_ledger_balances( + calculated_data: calculated, + expected_balances: [ + [ 3.days.ago.to_date, { balance: 18000, cash_balance: 18000 } ], + [ 2.days.ago.to_date, { balance: 19000, cash_balance: 19000 } ] + ] + ) end - test "valuations sync" do - create_valuation(account: @account, date: 4.days.ago.to_date, amount: 17000) - create_valuation(account: @account, date: 2.days.ago.to_date, amount: 19000) + test "cash-only accounts (depository, credit card) use valuations where cash balance equals total balance" do + [ Depository, CreditCard ].each do |account_type| + account = create_account_with_ledger( + account: { type: account_type, balance: 10000, cash_balance: 10000, currency: "USD" }, + entries: [ + { type: "opening_anchor", date: 3.days.ago.to_date, balance: 17000 }, + { type: "reconciliation", date: 2.days.ago.to_date, balance: 18000 } + ] + ) + + calculated = Balance::ForwardCalculator.new(account).calculate + + assert_calculated_ledger_balances( + calculated_data: calculated, + expected_balances: [ + [ 3.days.ago.to_date, { balance: 17000, cash_balance: 17000 } ], + [ 2.days.ago.to_date, { balance: 18000, cash_balance: 18000 } ] + ] + ) + end + end - expected = [ 0, 17000, 17000, 19000, 19000, 19000 ] - calculated = Balance::ForwardCalculator.new(@account).calculate.sort_by(&:date).map(&:balance) + test "non-cash accounts (property, loan) use valuations where cash balance is always zero" do + [ Property, Loan ].each do |account_type| + account = create_account_with_ledger( + account: { type: account_type, balance: 10000, cash_balance: 10000, currency: "USD" }, + entries: [ + { type: "opening_anchor", date: 3.days.ago.to_date, balance: 17000 }, + { type: "reconciliation", date: 2.days.ago.to_date, balance: 18000 } + ] + ) + + calculated = Balance::ForwardCalculator.new(account).calculate + + assert_calculated_ledger_balances( + calculated_data: calculated, + expected_balances: [ + [ 3.days.ago.to_date, { balance: 17000, cash_balance: 0.0 } ], + [ 2.days.ago.to_date, { balance: 18000, cash_balance: 0.0 } ] + ] + ) + end + end + + test "mixed accounts (investment) use valuations where cash balance is total minus holdings" do + account = create_account_with_ledger( + account: { type: Investment, balance: 10000, cash_balance: 10000, currency: "USD" }, + entries: [ + { type: "opening_anchor", date: 3.days.ago.to_date, balance: 17000 }, + { type: "reconciliation", date: 2.days.ago.to_date, balance: 18000 } + ] + ) - assert_equal expected, calculated + # Without holdings, cash balance equals total balance + calculated = Balance::ForwardCalculator.new(account).calculate + + assert_calculated_ledger_balances( + calculated_data: calculated, + expected_balances: [ + [ 3.days.ago.to_date, { balance: 17000, cash_balance: 17000 } ], + [ 2.days.ago.to_date, { balance: 18000, cash_balance: 18000 } ] + ] + ) end - test "transactions sync" do - create_transaction(account: @account, date: 4.days.ago.to_date, amount: -500) # income - create_transaction(account: @account, date: 2.days.ago.to_date, amount: 100) # expense + # ------------------------------------------------------------------------------------------------ + # All Cash accounts (Depository, CreditCard) + # ------------------------------------------------------------------------------------------------ + + test "transactions on depository accounts affect cash balance" do + account = create_account_with_ledger( + account: { type: Depository, balance: 20000, cash_balance: 20000, currency: "USD" }, + entries: [ + { type: "opening_anchor", date: 5.days.ago.to_date, balance: 20000 }, + { type: "transaction", date: 4.days.ago.to_date, amount: -500 }, # income + { type: "transaction", date: 2.days.ago.to_date, amount: 100 } # expense + ] + ) - expected = [ 0, 500, 500, 400, 400, 400 ] - calculated = Balance::ForwardCalculator.new(@account).calculate.sort_by(&:date).map(&:balance) + calculated = Balance::ForwardCalculator.new(account).calculate - assert_equal expected, calculated + assert_calculated_ledger_balances( + calculated_data: calculated, + expected_balances: [ + [ 5.days.ago.to_date, { balance: 20000, cash_balance: 20000 } ], + [ 4.days.ago.to_date, { balance: 20500, cash_balance: 20500 } ], + [ 3.days.ago.to_date, { balance: 20500, cash_balance: 20500 } ], + [ 2.days.ago.to_date, { balance: 20400, cash_balance: 20400 } ] + ] + ) end - test "multi-entry sync" do - create_transaction(account: @account, date: 8.days.ago.to_date, amount: -5000) - create_valuation(account: @account, date: 6.days.ago.to_date, amount: 17000) - create_transaction(account: @account, date: 6.days.ago.to_date, amount: -500) - create_transaction(account: @account, date: 4.days.ago.to_date, amount: -500) - create_valuation(account: @account, date: 3.days.ago.to_date, amount: 17000) - create_transaction(account: @account, date: 1.day.ago.to_date, amount: 100) - expected = [ 0, 5000, 5000, 17000, 17000, 17500, 17000, 17000, 16900, 16900 ] - calculated = Balance::ForwardCalculator.new(@account).calculate.sort_by(&:date).map(&:balance) + test "transactions on credit card accounts affect cash balance inversely" do + account = create_account_with_ledger( + account: { type: CreditCard, balance: 10000, cash_balance: 10000, currency: "USD" }, + entries: [ + { type: "opening_anchor", date: 5.days.ago.to_date, balance: 1000 }, + { type: "transaction", date: 4.days.ago.to_date, amount: -500 }, # CC payment + { type: "transaction", date: 2.days.ago.to_date, amount: 100 } # expense + ] + ) + + calculated = Balance::ForwardCalculator.new(account).calculate - assert_equal expected, calculated + assert_calculated_ledger_balances( + calculated_data: calculated, + expected_balances: [ + [ 5.days.ago.to_date, { balance: 1000, cash_balance: 1000 } ], + [ 4.days.ago.to_date, { balance: 500, cash_balance: 500 } ], + [ 3.days.ago.to_date, { balance: 500, cash_balance: 500 } ], + [ 2.days.ago.to_date, { balance: 600, cash_balance: 600 } ] + ] + ) end - test "multi-currency sync" do - ExchangeRate.create! date: 1.day.ago.to_date, from_currency: "EUR", to_currency: "USD", rate: 1.2 + test "depository account with transactions and balance reconciliations" do + account = create_account_with_ledger( + account: { type: Depository, balance: 20000, cash_balance: 20000, currency: "USD" }, + entries: [ + { type: "opening_anchor", date: 10.days.ago.to_date, balance: 20000 }, + { type: "transaction", date: 8.days.ago.to_date, amount: -5000 }, + { type: "reconciliation", date: 6.days.ago.to_date, balance: 17000 }, + { type: "transaction", date: 6.days.ago.to_date, amount: -500 }, + { type: "transaction", date: 4.days.ago.to_date, amount: -500 }, + { type: "reconciliation", date: 3.days.ago.to_date, balance: 17000 }, + { type: "transaction", date: 1.day.ago.to_date, amount: 100 } + ] + ) - create_transaction(account: @account, date: 3.days.ago.to_date, amount: -100, currency: "USD") - create_transaction(account: @account, date: 2.days.ago.to_date, amount: -300, currency: "USD") + calculated = Balance::ForwardCalculator.new(account).calculate + + assert_calculated_ledger_balances( + calculated_data: calculated, + expected_balances: [ + [ 10.days.ago.to_date, { balance: 20000, cash_balance: 20000 } ], + [ 9.days.ago.to_date, { balance: 20000, cash_balance: 20000 } ], + [ 8.days.ago.to_date, { balance: 25000, cash_balance: 25000 } ], + [ 7.days.ago.to_date, { balance: 25000, cash_balance: 25000 } ], + [ 6.days.ago.to_date, { balance: 17000, cash_balance: 17000 } ], + [ 5.days.ago.to_date, { balance: 17000, cash_balance: 17000 } ], + [ 4.days.ago.to_date, { balance: 17500, cash_balance: 17500 } ], + [ 3.days.ago.to_date, { balance: 17000, cash_balance: 17000 } ], + [ 2.days.ago.to_date, { balance: 17000, cash_balance: 17000 } ], + [ 1.day.ago.to_date, { balance: 16900, cash_balance: 16900 } ] + ] + ) + end - # Transaction in different currency than the account's main currency - create_transaction(account: @account, date: 1.day.ago.to_date, amount: -500, currency: "EUR") # €500 * 1.2 = $600 + test "accounts with transactions in multiple currencies convert to the account currency" do + account = create_account_with_ledger( + account: { type: Depository, balance: 20000, cash_balance: 20000, currency: "USD" }, + entries: [ + { type: "opening_anchor", date: 4.days.ago.to_date, balance: 100 }, + { type: "transaction", date: 3.days.ago.to_date, amount: -100 }, + { type: "transaction", date: 2.days.ago.to_date, amount: -300 }, + # Transaction in different currency than the account's main currency + { type: "transaction", date: 1.day.ago.to_date, amount: -500, currency: "EUR" } # €500 * 1.2 = $600 + ], + exchange_rates: [ + { date: 1.day.ago.to_date, from: "EUR", to: "USD", rate: 1.2 } + ] + ) - expected = [ 0, 100, 400, 1000, 1000 ] - calculated = Balance::ForwardCalculator.new(@account).calculate.sort_by(&:date).map(&:balance) + calculated = Balance::ForwardCalculator.new(account).calculate - assert_equal expected, calculated + assert_calculated_ledger_balances( + calculated_data: calculated, + expected_balances: [ + [ 4.days.ago.to_date, { balance: 100, cash_balance: 100 } ], + [ 3.days.ago.to_date, { balance: 200, cash_balance: 200 } ], + [ 2.days.ago.to_date, { balance: 500, cash_balance: 500 } ], + [ 1.day.ago.to_date, { balance: 1100, cash_balance: 1100 } ] + ] + ) end - test "holdings and trades sync" do - aapl = securities(:aapl) + # A loan is a special case where despite being a "non-cash" account, it is typical to have "payment" transactions that reduce the loan principal (non cash balance) + test "loan payment transactions affect non cash balance" do + account = create_account_with_ledger( + account: { type: Loan, balance: 10000, cash_balance: 0, currency: "USD" }, + entries: [ + { type: "opening_anchor", date: 2.days.ago.to_date, balance: 20000 }, + # "Loan payment" of $2000, which reduces the principal + # TODO: We'll eventually need to calculate which portion of the txn was "interest" vs. "principal", but for now we'll just assume it's all principal + # since we don't have a first-class way to track interest payments yet. + { type: "transaction", date: 1.day.ago.to_date, amount: -2000 } + ] + ) + + calculated = Balance::ForwardCalculator.new(account).calculate - # Account starts at a value of $5000 - create_valuation(account: @account, date: 2.days.ago.to_date, amount: 5000) + assert_calculated_ledger_balances( + calculated_data: calculated, + expected_balances: [ + [ 2.days.ago.to_date, { balance: 20000, cash_balance: 0 } ], + [ 1.day.ago.to_date, { balance: 18000, cash_balance: 0 } ] + ] + ) + end - # Share purchase reduces cash balance by $1000, but keeps overall balance same - create_trade(aapl, account: @account, qty: 10, date: 1.day.ago.to_date, price: 100) + test "non cash accounts can only use valuations and transactions will be recorded but ignored for balance calculation" do + [ Property, Vehicle, OtherAsset, OtherLiability ].each do |account_type| + account = create_account_with_ledger( + account: { type: account_type, balance: 10000, cash_balance: 10000, currency: "USD" }, + entries: [ + { type: "opening_anchor", date: 3.days.ago.to_date, balance: 500000 }, + + # Will be ignored for balance calculation due to account type of non-cash + { type: "transaction", date: 2.days.ago.to_date, amount: -50000 } + ] + ) + + calculated = Balance::ForwardCalculator.new(account).calculate + + assert_calculated_ledger_balances( + calculated_data: calculated, + expected_balances: [ + [ 3.days.ago.to_date, { balance: 500000, cash_balance: 0 } ], + [ 2.days.ago.to_date, { balance: 500000, cash_balance: 0 } ] + ] + ) + end + end - Holding.create!(date: 1.day.ago.to_date, account: @account, security: aapl, qty: 10, price: 100, amount: 1000, currency: "USD") - Holding.create!(date: Date.current, account: @account, security: aapl, qty: 10, price: 100, amount: 1000, currency: "USD") + # ------------------------------------------------------------------------------------------------ + # Hybrid accounts (Investment, Crypto) - these have both cash and non-cash balance components + # ------------------------------------------------------------------------------------------------ + + # A transaction increases/decreases cash balance (i.e. "deposits" and "withdrawals") + # A trade increases/decreases cash balance (i.e. "buys" and "sells", which consume/add "brokerage cash" and create/destroy "holdings") + # A valuation can set both cash and non-cash balances to "override" investment account value. + # Holdings are calculated separately and fed into the balance calculator; treated as "non-cash" + test "investment account calculates balance from transactions and trades and treats holdings as non-cash, additive to balance" do + account = create_account_with_ledger( + account: { type: Investment, balance: 10000, cash_balance: 10000, currency: "USD" }, + entries: [ + # Account starts with brokerage cash of $5000 and no holdings + { type: "opening_anchor", date: 3.days.ago.to_date, balance: 5000 }, + # Share purchase reduces cash balance by $1000, but keeps overall balance same + { type: "trade", date: 1.day.ago.to_date, ticker: "AAPL", qty: 10, price: 100 } + ], + holdings: [ + # Holdings calculator will calculate $1000 worth of holdings + { date: 1.day.ago.to_date, ticker: "AAPL", qty: 10, price: 100, amount: 1000 }, + { date: Date.current, ticker: "AAPL", qty: 10, price: 100, amount: 1000 } + ] + ) # Given constant prices, overall balance (account value) should be constant # (the single trade doesn't affect balance; it just alters cash vs. holdings composition) - expected = [ 0, 5000, 5000, 5000 ] - calculated = Balance::ForwardCalculator.new(@account).calculate.sort_by(&:date).map(&:balance) - - assert_equal expected, calculated + calculated = Balance::ForwardCalculator.new(account).calculate + + assert_calculated_ledger_balances( + calculated_data: calculated, + expected_balances: [ + [ 3.days.ago.to_date, { balance: 5000, cash_balance: 5000 } ], + [ 2.days.ago.to_date, { balance: 5000, cash_balance: 5000 } ], + [ 1.day.ago.to_date, { balance: 5000, cash_balance: 4000 } ], + [ Date.current, { balance: 5000, cash_balance: 4000 } ] + ] + ) end - # Balance calculator is entirely reliant on HoldingCalculator and respects whatever holding records it creates. - test "holdings are additive to total balance" do - aapl = securities(:aapl) - - # Account starts at a value of $5000 - create_valuation(account: @account, date: 2.days.ago.to_date, amount: 5000) + private - # Even though there are no trades in the history, the calculator will still add the holdings to the total balance - Holding.create!(date: 1.day.ago.to_date, account: @account, security: aapl, qty: 10, price: 100, amount: 1000, currency: "USD") - Holding.create!(date: Date.current, account: @account, security: aapl, qty: 10, price: 100, amount: 1000, currency: "USD") + def assert_balances(calculated_data:, expected_balances:) + # Sort calculated data by date to ensure consistent ordering + sorted_data = calculated_data.sort_by(&:date) - # Start at zero, then valuation of $5000, then tack on $1000 of holdings for remaining 2 days - expected = [ 0, 5000, 6000, 6000 ] - calculated = Balance::ForwardCalculator.new(@account).calculate.sort_by(&:date).map(&:balance) + # Extract actual values as [date, { balance:, cash_balance: }] + actual_balances = sorted_data.map do |b| + [ b.date, { balance: b.balance, cash_balance: b.cash_balance } ] + end - assert_equal expected, calculated - end + assert_equal expected_balances, actual_balances + end end diff --git a/test/models/balance/reverse_calculator_test.rb b/test/models/balance/reverse_calculator_test.rb index 6d73aea84cb..a9348220610 100644 --- a/test/models/balance/reverse_calculator_test.rb +++ b/test/models/balance/reverse_calculator_test.rb @@ -1,142 +1,279 @@ require "test_helper" class Balance::ReverseCalculatorTest < ActiveSupport::TestCase - include EntriesTestHelper + include LedgerTestingHelper - setup do - @account = families(:empty).accounts.create!( - name: "Test", - balance: 20000, - cash_balance: 20000, - currency: "USD", - accountable: Investment.new + # When syncing backwards, we start with the account balance and generate everything from there. + test "when missing anchor and no entries, falls back to cached account balance" do + account = create_account_with_ledger( + account: { type: Depository, balance: 20000, cash_balance: 20000, currency: "USD" }, + entries: [] ) - end - # When syncing backwards, we start with the account balance and generate everything from there. - test "no entries sync" do - assert_equal 0, @account.balances.count + assert_equal 20000, account.balance - expected = [ @account.balance, @account.balance ] - calculated = Balance::ReverseCalculator.new(@account).calculate + calculated = Balance::ReverseCalculator.new(account).calculate - assert_equal expected, calculated.map(&:balance) + assert_calculated_ledger_balances( + calculated_data: calculated, + expected_balances: [ + [ Date.current, { balance: 20000, cash_balance: 20000 } ] + ] + ) end - test "balance generation respects user timezone and last generated date is current user date" do - # Simulate user in EST timezone - Time.use_zone("America/New_York") do - # Set current time to 1am UTC on Jan 5, 2025 - # This would be 8pm EST on Jan 4, 2025 (user's time, and the last date we should generate balances for) - travel_to Time.utc(2025, 01, 05, 1, 0, 0) - - create_valuation(account: @account, date: "2025-01-03", amount: 17000) - - expected = [ [ "2025-01-02", 17000 ], [ "2025-01-03", 17000 ], [ "2025-01-04", @account.balance ] ] - calculated = Balance::ReverseCalculator.new(@account).calculate + # An artificial constraint we put on the reverse sync because it's confusing in both the code and the UI + # to think about how an absolute "Valuation" affects balances when syncing backwards. Furthermore, since + # this is typically a Plaid sync, we expect Plaid to provide us the history. + # Note: while "reconciliation" valuations don't affect balance, `current_anchor` and `opening_anchor` do. + test "reconciliation valuations do not affect balance for reverse syncs" do + account = create_account_with_ledger( + account: { type: Depository, balance: 20000, cash_balance: 20000, currency: "USD" }, + entries: [ + { type: "current_anchor", date: Date.current, balance: 20000 }, + { type: "reconciliation", date: 1.day.ago, balance: 17000 }, # Ignored + { type: "reconciliation", date: 2.days.ago, balance: 17000 }, # Ignored + { type: "opening_anchor", date: 4.days.ago, balance: 15000 } + ] + ) - assert_equal expected, calculated.sort_by(&:date).map { |b| [ b.date.to_s, b.balance ] } - end + calculated = Balance::ReverseCalculator.new(account).calculate + + # The "opening anchor" works slightly differently than most would expect. Since it's an artificial + # value provided by the user to set the date/balance of the start of the account, we must assume + # that there are "missing" entries following it. Because of this, we cannot "carry forward" this value + # like we do for a "forward sync". We simply sync backwards normally, then set the balance on opening + # date equal to this anchor. This is not "ideal", but is a constraint put on us since we cannot guarantee + # a 100% full entries history. + assert_calculated_ledger_balances( + calculated_data: calculated, + expected_balances: [ + [ Date.current, { balance: 20000, cash_balance: 20000 } ], # Current anchor + [ 1.day.ago, { balance: 20000, cash_balance: 20000 } ], + [ 2.days.ago, { balance: 20000, cash_balance: 20000 } ], + [ 3.days.ago, { balance: 20000, cash_balance: 20000 } ], + [ 4.days.ago, { balance: 15000, cash_balance: 15000 } ] # Opening anchor + ] + ) end - test "valuations sync" do - create_valuation(account: @account, date: 4.days.ago.to_date, amount: 17000) - create_valuation(account: @account, date: 2.days.ago.to_date, amount: 19000) + # Investment account balances are made of two components: cash and holdings. + test "anchors on investment accounts calculate cash balance dynamically based on holdings value" do + account = create_account_with_ledger( + account: { type: Investment, balance: 20000, cash_balance: 10000, currency: "USD" }, + entries: [ + { type: "current_anchor", date: Date.current, balance: 20000 }, # "Total account value is $20,000 today" + { type: "opening_anchor", date: 1.day.ago, balance: 15000 } # "Total account value was $15,000 at the start of the account" + ], + holdings: [ + { date: Date.current, ticker: "AAPL", qty: 100, price: 100, amount: 10000 }, + { date: 1.day.ago, ticker: "AAPL", qty: 100, price: 100, amount: 10000 } + ] + ) - expected = [ 17000, 17000, 19000, 19000, 20000, 20000 ] - calculated = Balance::ReverseCalculator.new(@account).calculate.sort_by(&:date).map(&:balance) + calculated = Balance::ReverseCalculator.new(account).calculate - assert_equal expected, calculated + assert_calculated_ledger_balances( + calculated_data: calculated, + expected_balances: [ + [ Date.current, { balance: 20000, cash_balance: 10000 } ], # Since $10,000 of holdings, cash has to be $10,000 to reach $20,000 total value + [ 1.day.ago, { balance: 15000, cash_balance: 5000 } ] # Since $10,000 of holdings, cash has to be $5,000 to reach $15,000 total value + ] + ) end - test "transactions sync" do - create_transaction(account: @account, date: 4.days.ago.to_date, amount: -500) # income - create_transaction(account: @account, date: 2.days.ago.to_date, amount: 100) # expense + test "transactions on depository accounts affect cash balance" do + account = create_account_with_ledger( + account: { type: Depository, balance: 20000, cash_balance: 20000, currency: "USD" }, + entries: [ + { type: "current_anchor", date: Date.current, balance: 20000 }, + { type: "transaction", date: 4.days.ago, amount: -500 }, # income + { type: "transaction", date: 2.days.ago, amount: 100 } # expense + ] + ) - expected = [ 19600, 20100, 20100, 20000, 20000, 20000 ] - calculated = Balance::ReverseCalculator.new(@account).calculate.sort_by(&:date).map(&:balance) + calculated = Balance::ReverseCalculator.new(account).calculate + + assert_calculated_ledger_balances( + calculated_data: calculated, + expected_balances: [ + [ Date.current, { balance: 20000, cash_balance: 20000 } ], # Current balance + [ 1.day.ago, { balance: 20000, cash_balance: 20000 } ], # No change + [ 2.days.ago, { balance: 20000, cash_balance: 20000 } ], # After expense (+100) + [ 3.days.ago, { balance: 20100, cash_balance: 20100 } ], # Before expense + [ 4.days.ago, { balance: 20100, cash_balance: 20100 } ], # After income (-500) + [ 5.days.ago, { balance: 19600, cash_balance: 19600 } ] # After income (-500) + ] + ) + end - assert_equal expected, calculated + test "transactions on credit card accounts affect cash balance inversely" do + account = create_account_with_ledger( + account: { type: CreditCard, balance: 2000, cash_balance: 2000, currency: "USD" }, + entries: [ + { type: "current_anchor", date: Date.current, balance: 2000 }, + { type: "transaction", date: 2.days.ago, amount: 100 }, # expense (increases cash balance) + { type: "transaction", date: 4.days.ago, amount: -500 } # CC payment (reduces cash balance) + ] + ) + + calculated = Balance::ReverseCalculator.new(account).calculate + + # Reversed order: showing how we work backwards + assert_calculated_ledger_balances( + calculated_data: calculated, + expected_balances: [ + [ Date.current, { balance: 2000, cash_balance: 2000 } ], # Current balance + [ 1.day.ago, { balance: 2000, cash_balance: 2000 } ], # No change + [ 2.days.ago, { balance: 2000, cash_balance: 2000 } ], # After expense (+100) + [ 3.days.ago, { balance: 1900, cash_balance: 1900 } ], # Before expense + [ 4.days.ago, { balance: 1900, cash_balance: 1900 } ], # After CC payment (-500) + [ 5.days.ago, { balance: 2400, cash_balance: 2400 } ] + ] + ) end - test "multi-entry sync" do - create_transaction(account: @account, date: 8.days.ago.to_date, amount: -5000) - create_valuation(account: @account, date: 6.days.ago.to_date, amount: 17000) - create_transaction(account: @account, date: 6.days.ago.to_date, amount: -500) - create_transaction(account: @account, date: 4.days.ago.to_date, amount: -500) - create_valuation(account: @account, date: 3.days.ago.to_date, amount: 17000) - create_transaction(account: @account, date: 1.day.ago.to_date, amount: 100) + # A loan is a special case where despite being a "non-cash" account, it is typical to have "payment" transactions that reduce the loan principal (non cash balance) + test "loan payment transactions affect non cash balance" do + account = create_account_with_ledger( + account: { type: Loan, balance: 198000, cash_balance: 0, currency: "USD" }, + entries: [ + { type: "current_anchor", date: Date.current, balance: 198000 }, + # "Loan payment" of $2000, which reduces the principal + # TODO: We'll eventually need to calculate which portion of the txn was "interest" vs. "principal", but for now we'll just assume it's all principal + # since we don't have a first-class way to track interest payments yet. + { type: "transaction", date: 1.day.ago.to_date, amount: -2000 } + ] + ) + + calculated = Balance::ReverseCalculator.new(account).calculate - expected = [ 12000, 17000, 17000, 17000, 16500, 17000, 17000, 20100, 20000, 20000 ] - calculated = Balance::ReverseCalculator.new(@account).calculate.sort_by(&:date).map(&:balance) + assert_calculated_ledger_balances( + calculated_data: calculated, + expected_balances: [ + [ Date.current, { balance: 198000, cash_balance: 0 } ], + [ 1.day.ago, { balance: 198000, cash_balance: 0 } ], + [ 2.days.ago, { balance: 200000, cash_balance: 0 } ] + ] + ) + end - assert_equal expected, calculated + test "non cash accounts can only use valuations and transactions will be recorded but ignored for balance calculation" do + [ Property, Vehicle, OtherAsset, OtherLiability ].each do |account_type| + account = create_account_with_ledger( + account: { type: account_type, balance: 1000, cash_balance: 0, currency: "USD" }, + entries: [ + { type: "current_anchor", date: Date.current, balance: 1000 }, + + # Will be ignored for balance calculation due to account type of non-cash + { type: "transaction", date: 1.day.ago, amount: -100 } + ] + ) + + calculated = Balance::ReverseCalculator.new(account).calculate + + assert_calculated_ledger_balances( + calculated_data: calculated, + expected_balances: [ + [ Date.current, { balance: 1000, cash_balance: 0 } ], + [ 1.day.ago, { balance: 1000, cash_balance: 0 } ], + [ 2.days.ago, { balance: 1000, cash_balance: 0 } ] + ] + ) + end end # When syncing backwards, trades from the past should NOT affect the current balance or previous balances. # They should only affect the *cash* component of the historical balances test "holdings and trades sync" do - aapl = securities(:aapl) - # Account starts with $20,000 total value, $19,000 cash, $1,000 in holdings - @account.update!(cash_balance: 19000, balance: 20000) - - # Bought 10 AAPL shares 1 day ago, so cash is $19,000, $1,000 in holdings, total value is $20,000 - create_trade(aapl, account: @account, qty: 10, date: 1.day.ago.to_date, price: 100) + account = create_account_with_ledger( + account: { type: Investment, balance: 20000, cash_balance: 19000, currency: "USD" }, + entries: [ + { type: "current_anchor", date: Date.current, balance: 20000 }, + # Bought 10 AAPL shares 1 day ago, so cash is $19,000, $1,000 in holdings, total value is $20,000 + { type: "trade", date: 1.day.ago.to_date, ticker: "AAPL", qty: 10, price: 100 } + ], + holdings: [ + { date: Date.current, ticker: "AAPL", qty: 10, price: 100, amount: 1000 }, + { date: 1.day.ago.to_date, ticker: "AAPL", qty: 10, price: 100, amount: 1000 } + ] + ) - Holding.create!(date: Date.current, account: @account, security: aapl, qty: 10, price: 100, amount: 1000, currency: "USD") - Holding.create!(date: 1.day.ago.to_date, account: @account, security: aapl, qty: 10, price: 100, amount: 1000, currency: "USD") + calculated = Balance::ReverseCalculator.new(account).calculate # Given constant prices, overall balance (account value) should be constant # (the single trade doesn't affect balance; it just alters cash vs. holdings composition) - expected = [ 20000, 20000, 20000 ] - calculated = Balance::ReverseCalculator.new(@account).calculate.sort_by(&:date).map(&:balance) - - assert_equal expected, calculated + assert_calculated_ledger_balances( + calculated_data: calculated, + expected_balances: [ + [ Date.current, { balance: 20000, cash_balance: 19000 } ], # Current: $19k cash + $1k holdings (anchor) + [ 1.day.ago.to_date, { balance: 20000, cash_balance: 19000 } ], # After trade: $19k cash + $1k holdings + [ 2.days.ago.to_date, { balance: 20000, cash_balance: 20000 } ] # At first, account is 100% cash, no holdings (no trades) + ] + ) end # A common scenario with Plaid is they'll give us holding records for today, but no trade history for some of them. # This is because they only supply 2 years worth of historical data. Our system must properly handle this. test "properly calculates balances when a holding has no trade history" do - aapl = securities(:aapl) - msft = securities(:msft) - # Account starts with $20,000 total value, $19,000 cash, $1,000 in holdings ($500 AAPL, $500 MSFT) - @account.update!(cash_balance: 19000, balance: 20000) - - # A holding *with* trade history (5 shares of AAPL, purchased 1 day ago, results in 2 holdings) - Holding.create!(date: Date.current, account: @account, security: aapl, qty: 5, price: 100, amount: 500, currency: "USD") - Holding.create!(date: 1.day.ago.to_date, account: @account, security: aapl, qty: 5, price: 100, amount: 500, currency: "USD") - create_trade(aapl, account: @account, qty: 5, date: 1.day.ago.to_date, price: 100) - - # A holding *without* trade history (5 shares of MSFT, no trade history, results in 1 holding) - # We assume if no history is provided, this holding has existed since beginning of account - Holding.create!(date: Date.current, account: @account, security: msft, qty: 5, price: 100, amount: 500, currency: "USD") - Holding.create!(date: 1.day.ago.to_date, account: @account, security: msft, qty: 5, price: 100, amount: 500, currency: "USD") - Holding.create!(date: 2.days.ago.to_date, account: @account, security: msft, qty: 5, price: 100, amount: 500, currency: "USD") + account = create_account_with_ledger( + account: { type: Investment, balance: 20000, cash_balance: 19000, currency: "USD" }, + entries: [ + { type: "current_anchor", date: Date.current, balance: 20000 }, + # A holding *with* trade history (5 shares of AAPL, purchased 1 day ago) + { type: "trade", date: 1.day.ago.to_date, ticker: "AAPL", qty: 5, price: 100 } + ], + holdings: [ + # AAPL holdings + { date: Date.current, ticker: "AAPL", qty: 5, price: 100, amount: 500 }, + { date: 1.day.ago.to_date, ticker: "AAPL", qty: 5, price: 100, amount: 500 }, + # MSFT holdings without trade history - Balance calculator doesn't care how the holdings were created. It just reads them and assumes they are accurate. + { date: Date.current, ticker: "MSFT", qty: 5, price: 100, amount: 500 }, + { date: 1.day.ago.to_date, ticker: "MSFT", qty: 5, price: 100, amount: 500 }, + { date: 2.days.ago.to_date, ticker: "MSFT", qty: 5, price: 100, amount: 500 } + ] + ) - expected = [ 20000, 20000, 20000 ] - calculated = Balance::ReverseCalculator.new(@account).calculate.sort_by(&:date).map(&:balance) + calculated = Balance::ReverseCalculator.new(account).calculate - assert_equal expected, calculated + assert_calculated_ledger_balances( + calculated_data: calculated, + expected_balances: [ + [ Date.current, { balance: 20000, cash_balance: 19000 } ], # Current: $19k cash + $1k holdings ($500 MSFT, $500 AAPL) + [ 1.day.ago.to_date, { balance: 20000, cash_balance: 19000 } ], # After AAPL trade: $19k cash + $1k holdings + [ 2.days.ago.to_date, { balance: 20000, cash_balance: 19500 } ] # Before AAPL trade: $19.5k cash + $500 MSFT + ] + ) 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) + account = create_account_with_ledger( + account: { type: Investment, balance: 20000, cash_balance: 19000, currency: "USD" }, + entries: [ + { type: "current_anchor", date: Date.current, balance: 20000 }, + { type: "opening_anchor", date: 2.days.ago, balance: 15000 } + ], + holdings: [ + # Create holdings that differ in value from provider ($2,000 vs. the $1,000 reported by provider) + { date: Date.current, ticker: "AAPL", qty: 10, price: 100, amount: 2000 }, + { date: 1.day.ago, ticker: "AAPL", qty: 10, price: 100, amount: 2000 } + ] + ) - assert_equal expected, calculated + calculated = Balance::ReverseCalculator.new(account).calculate + + assert_calculated_ledger_balances( + calculated_data: calculated, + expected_balances: [ + # No matter what, we force current day equal to the "anchor" balance (what provider gave us), and let "cash" float based on holdings value + # This ensures the user sees the same top-line number reported by the provider (even if it creates a discrepancy in the cash balance) + [ Date.current, { balance: 20000, cash_balance: 18000 } ], + [ 1.day.ago, { balance: 20000, cash_balance: 18000 } ], + [ 2.days.ago, { balance: 15000, cash_balance: 15000 } ] # Opening anchor sets absolute balance + ] + ) end end diff --git a/test/models/plaid_account/processor_test.rb b/test/models/plaid_account/processor_test.rb index ec75296dbec..ba6a002f086 100644 --- a/test/models/plaid_account/processor_test.rb +++ b/test/models/plaid_account/processor_test.rb @@ -94,10 +94,21 @@ class PlaidAccount::ProcessorTest < ActiveSupport::TestCase test "calculates balance using BalanceCalculator for investment accounts" do @plaid_account.update!(plaid_type: "investment") - PlaidAccount::Investments::BalanceCalculator.any_instance.expects(:balance).returns(1000).once + # Balance is called twice: once for account.balance and once for set_current_balance + PlaidAccount::Investments::BalanceCalculator.any_instance.expects(:balance).returns(1000).twice PlaidAccount::Investments::BalanceCalculator.any_instance.expects(:cash_balance).returns(1000).once PlaidAccount::Processor.new(@plaid_account).process + + # Verify that the balance was set correctly + account = @plaid_account.account + assert_equal 1000, account.balance + assert_equal 1000, account.cash_balance + + # Verify current balance anchor was created with correct value + current_anchor = account.valuations.current_anchor.first + assert_not_nil current_anchor + assert_equal 1000, current_anchor.entry.amount end test "processes credit liability data" do @@ -142,6 +153,76 @@ class PlaidAccount::ProcessorTest < ActiveSupport::TestCase PlaidAccount::Processor.new(@plaid_account).process end + test "creates current balance anchor when processing account" do + expect_default_subprocessor_calls + + # Clear out accounts to start fresh + Account.destroy_all + + @plaid_account.update!( + plaid_id: "test_plaid_id", + plaid_type: "depository", + plaid_subtype: "checking", + current_balance: 1500, + available_balance: 1500, + currency: "USD", + name: "Test Account with Anchor", + mask: "1234" + ) + + assert_difference "Account.count", 1 do + assert_difference "Entry.count", 1 do + assert_difference "Valuation.count", 1 do + PlaidAccount::Processor.new(@plaid_account).process + end + end + end + + account = Account.order(created_at: :desc).first + assert_equal 1500, account.balance + + # Verify current balance anchor was created + current_anchor = account.valuations.current_anchor.first + assert_not_nil current_anchor + assert_equal "current_anchor", current_anchor.kind + assert_equal 1500, current_anchor.entry.amount + assert_equal Date.current, current_anchor.entry.date + assert_equal "Current balance", current_anchor.entry.name + end + + test "updates existing current balance anchor when reprocessing" do + # First process creates the account and anchor + expect_default_subprocessor_calls + PlaidAccount::Processor.new(@plaid_account).process + + account = @plaid_account.account + original_anchor = account.valuations.current_anchor.first + assert_not_nil original_anchor + original_anchor_id = original_anchor.id + original_entry_id = original_anchor.entry.id + original_balance = original_anchor.entry.amount + + # Update the plaid account balance + @plaid_account.update!(current_balance: 2500) + + # Expect subprocessor calls again for the second processing + expect_default_subprocessor_calls + + # Reprocess should update the existing anchor + assert_no_difference "Valuation.count" do + assert_no_difference "Entry.count" do + PlaidAccount::Processor.new(@plaid_account).process + end + end + + # Verify the anchor was updated + original_anchor.reload + assert_equal original_anchor_id, original_anchor.id + assert_equal original_entry_id, original_anchor.entry.id + assert_equal 2500, original_anchor.entry.amount + assert_not_equal original_balance, original_anchor.entry.amount + end + private def expect_investment_product_processor_calls PlaidAccount::Investments::TransactionsProcessor.any_instance.expects(:process).once diff --git a/test/models/valuation/name_test.rb b/test/models/valuation/name_test.rb index 7fa41ccac26..feed97eae28 100644 --- a/test/models/valuation/name_test.rb +++ b/test/models/valuation/name_test.rb @@ -17,6 +17,21 @@ class Valuation::NameTest < ActiveSupport::TestCase assert_equal "Opening account value", name.to_s end + test "generates opening anchor name for Vehicle" do + name = Valuation::Name.new("opening_anchor", "Vehicle") + assert_equal "Original purchase price", name.to_s + end + + test "generates opening anchor name for Crypto" do + name = Valuation::Name.new("opening_anchor", "Crypto") + assert_equal "Opening account value", name.to_s + end + + test "generates opening anchor name for OtherAsset" do + name = Valuation::Name.new("opening_anchor", "OtherAsset") + assert_equal "Opening account value", name.to_s + end + test "generates opening anchor name for other account types" do name = Valuation::Name.new("opening_anchor", "Depository") assert_equal "Opening balance", name.to_s @@ -38,6 +53,21 @@ class Valuation::NameTest < ActiveSupport::TestCase assert_equal "Current account value", name.to_s end + test "generates current anchor name for Vehicle" do + name = Valuation::Name.new("current_anchor", "Vehicle") + assert_equal "Current market value", name.to_s + end + + test "generates current anchor name for Crypto" do + name = Valuation::Name.new("current_anchor", "Crypto") + assert_equal "Current account value", name.to_s + end + + test "generates current anchor name for OtherAsset" do + name = Valuation::Name.new("current_anchor", "OtherAsset") + assert_equal "Current account value", name.to_s + end + test "generates current anchor name for other account types" do name = Valuation::Name.new("current_anchor", "Depository") assert_equal "Current balance", name.to_s @@ -54,6 +84,21 @@ class Valuation::NameTest < ActiveSupport::TestCase assert_equal "Manual value update", name.to_s end + test "generates recon name for Vehicle" do + name = Valuation::Name.new("reconciliation", "Vehicle") + assert_equal "Manual value update", name.to_s + end + + test "generates recon name for Crypto" do + name = Valuation::Name.new("reconciliation", "Crypto") + assert_equal "Manual value update", name.to_s + end + + test "generates recon name for OtherAsset" do + name = Valuation::Name.new("reconciliation", "OtherAsset") + assert_equal "Manual value update", name.to_s + end + test "generates recon name for Loan" do name = Valuation::Name.new("reconciliation", "Loan") assert_equal "Manual principal update", name.to_s diff --git a/test/support/entries_test_helper.rb b/test/support/entries_test_helper.rb index a4f2013f828..35e5450ff0f 100644 --- a/test/support/entries_test_helper.rb +++ b/test/support/entries_test_helper.rb @@ -15,17 +15,50 @@ def create_transaction(attributes = {}) Entry.create! entry_defaults.merge(entry_attributes) end + def create_opening_anchor_valuation(account:, balance:, date:) + create_valuation( + account: account, + kind: "opening_anchor", + amount: balance, + date: date + ) + end + + def create_reconciliation_valuation(account:, balance:, date:) + create_valuation( + account: account, + kind: "reconciliation", + amount: balance, + date: date + ) + end + + def create_current_anchor_valuation(account:, balance:, date: Date.current) + create_valuation( + account: account, + kind: "current_anchor", + amount: balance, + date: date + ) + end + def create_valuation(attributes = {}) + entry_attributes = attributes.except(:kind) + valuation_attributes = attributes.slice(:kind) + + account = attributes[:account] || accounts(:depository) + amount = attributes[:amount] || 5000 + entry_defaults = { - account: accounts(:depository), + account: account, name: "Valuation", date: 1.day.ago.to_date, currency: "USD", - amount: 5000, - entryable: Valuation.new + amount: amount, + entryable: Valuation.new({ kind: "reconciliation" }.merge(valuation_attributes)) } - Entry.create! entry_defaults.merge(attributes) + Entry.create! entry_defaults.merge(entry_attributes) end def create_trade(security, account:, qty:, date:, price: nil, currency: "USD") diff --git a/test/support/ledger_testing_helper.rb b/test/support/ledger_testing_helper.rb new file mode 100644 index 00000000000..6ae71678bda --- /dev/null +++ b/test/support/ledger_testing_helper.rb @@ -0,0 +1,152 @@ +module LedgerTestingHelper + def create_account_with_ledger(account:, entries: [], exchange_rates: [], security_prices: [], holdings: []) + # Clear all exchange rates and security prices to ensure clean test environment + ExchangeRate.destroy_all + Security::Price.destroy_all + + # Create account with specified attributes + account_attrs = account.except(:type) + account_type = account[:type] + + # Create the account + created_account = families(:empty).accounts.create!( + name: "Test Account", + accountable: account_type.new, + **account_attrs + ) + + # Set up exchange rates if provided + exchange_rates.each do |rate_data| + ExchangeRate.create!( + date: rate_data[:date], + from_currency: rate_data[:from], + to_currency: rate_data[:to], + rate: rate_data[:rate] + ) + end + + # Set up security prices if provided + security_prices.each do |price_data| + security = Security.find_or_create_by!(ticker: price_data[:ticker]) do |s| + s.name = price_data[:ticker] + end + + Security::Price.create!( + security: security, + date: price_data[:date], + price: price_data[:price], + currency: created_account.currency + ) + end + + # Create entries in the order they were specified + entries.each do |entry_data| + case entry_data[:type] + when "current_anchor", "opening_anchor", "reconciliation" + # Create valuation entry + created_account.entries.create!( + name: "Valuation", + date: entry_data[:date], + amount: entry_data[:balance], + currency: entry_data[:currency] || created_account.currency, + entryable: Valuation.new(kind: entry_data[:type]) + ) + when "transaction" + # Use account currency if not specified + currency = entry_data[:currency] || created_account.currency + + created_account.entries.create!( + name: "Transaction", + date: entry_data[:date], + amount: entry_data[:amount], + currency: currency, + entryable: Transaction.new + ) + when "trade" + # Find or create security + security = Security.find_or_create_by!(ticker: entry_data[:ticker]) do |s| + s.name = entry_data[:ticker] + end + + # Use account currency if not specified + currency = entry_data[:currency] || created_account.currency + + trade = Trade.new( + qty: entry_data[:qty], + security: security, + price: entry_data[:price], + currency: currency + ) + + created_account.entries.create!( + name: "Trade", + date: entry_data[:date], + amount: entry_data[:qty] * entry_data[:price], + currency: currency, + entryable: trade + ) + end + end + + # Create holdings if provided + holdings.each do |holding_data| + # Find or create security + security = Security.find_or_create_by!(ticker: holding_data[:ticker]) do |s| + s.name = holding_data[:ticker] + end + + Holding.create!( + account: created_account, + security: security, + date: holding_data[:date], + qty: holding_data[:qty], + price: holding_data[:price], + amount: holding_data[:amount], + currency: holding_data[:currency] || created_account.currency + ) + end + + created_account + end + + def assert_calculated_ledger_balances(calculated_data:, expected_balances:) + # Convert expected balances to a hash for easier lookup + expected_hash = expected_balances.to_h do |date, balance_data| + [ date.to_date, balance_data ] + end + + # Get all unique dates from both calculated and expected data + all_dates = (calculated_data.map(&:date) + expected_hash.keys).uniq.sort + + # Check each date + all_dates.each do |date| + calculated_balance = calculated_data.find { |b| b.date == date } + expected = expected_hash[date] + + if expected + assert calculated_balance, "Expected balance for #{date} but none was calculated" + + if expected[:balance] + assert_equal expected[:balance], calculated_balance.balance.to_d, + "Balance mismatch for #{date}" + end + + if expected[:cash_balance] + assert_equal expected[:cash_balance], calculated_balance.cash_balance.to_d, + "Cash balance mismatch for #{date}" + end + else + assert_nil calculated_balance, "Unexpected balance calculated for #{date}" + end + end + + # Verify we got all expected dates + expected_dates = expected_hash.keys.sort + calculated_dates = calculated_data.map(&:date).sort + + expected_dates.each do |date| + assert_includes calculated_dates, date, + "Expected balance for #{date} was not in calculated data" + end + end +end From 89cc64418e5202d8121018955b80b6965c00a1fb Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Tue, 15 Jul 2025 18:58:40 -0400 Subject: [PATCH 02/29] Add confirmation dialog for balance reconciliation creates and updates (#2457) --- app/controllers/valuations_controller.rb | 40 ++++++++++----- app/models/account.rb | 4 +- app/models/account/balance_updater.rb | 14 ++++-- app/models/investment.rb | 15 ++++++ app/views/entries/_selection_bar.html.erb | 2 +- .../_confirmation_contents.html.erb | 49 +++++++++++++++++++ app/views/valuations/_form.html.erb | 17 ------- app/views/valuations/confirm_create.html.erb | 20 ++++++++ app/views/valuations/confirm_update.html.erb | 19 +++++++ app/views/valuations/new.html.erb | 15 +++++- app/views/valuations/show.html.erb | 27 +++++++--- config/routes.rb | 5 +- .../controllers/valuations_controller_test.rb | 2 - 13 files changed, 180 insertions(+), 49 deletions(-) create mode 100644 app/views/valuations/_confirmation_contents.html.erb delete mode 100644 app/views/valuations/_form.html.erb create mode 100644 app/views/valuations/confirm_create.html.erb create mode 100644 app/views/valuations/confirm_update.html.erb diff --git a/app/controllers/valuations_controller.rb b/app/controllers/valuations_controller.rb index 90aa1da0c86..85f3413ab3b 100644 --- a/app/controllers/valuations_controller.rb +++ b/app/controllers/valuations_controller.rb @@ -1,15 +1,24 @@ class ValuationsController < ApplicationController include EntryableResource, StreamExtensions + def confirm_create + @account = Current.family.accounts.find(params.dig(:entry, :account_id)) + @entry = @account.entries.build(entry_params.merge(currency: @account.currency)) + + render :confirm_create + end + + def confirm_update + @entry = Current.family.entries.find(params[:id]) + @account = @entry.account + @entry.assign_attributes(entry_params.merge(currency: @account.currency)) + + render :confirm_update + end + def create account = Current.family.accounts.find(params.dig(:entry, :account_id)) - - result = account.update_balance( - balance: entry_params[:amount], - date: entry_params[:date], - currency: entry_params[:currency], - notes: entry_params[:notes] - ) + result = perform_balance_update(account, entry_params.merge(currency: account.currency)) if result.success? @success_message = result.updated? ? "Balance updated" : "No changes made. Account is already up to date." @@ -25,12 +34,7 @@ def create end def update - result = @entry.account.update_balance( - date: @entry.date, - balance: entry_params[:amount], - currency: entry_params[:currency], - notes: entry_params[:notes] - ) + result = perform_balance_update(@entry.account, entry_params.merge(currency: @entry.currency, existing_valuation_id: @entry.id)) if result.success? @entry.reload @@ -59,4 +63,14 @@ def entry_params params.require(:entry) .permit(:date, :amount, :currency, :notes) end + + def perform_balance_update(account, params) + account.update_balance( + balance: params[:amount], + date: params[:date], + currency: params[:currency], + notes: params[:notes], + existing_valuation_id: params[:existing_valuation_id] + ) + end end diff --git a/app/models/account.rb b/app/models/account.rb index 1217c5fefe9..837d6e57e7b 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -115,8 +115,8 @@ def current_holdings end - def update_balance(balance:, date: Date.current, currency: nil, notes: nil) - Account::BalanceUpdater.new(self, balance:, currency:, date:, notes:).update + def update_balance(balance:, date: Date.current, currency: nil, notes: nil, existing_valuation_id: nil) + Account::BalanceUpdater.new(self, balance:, currency:, date:, notes:, existing_valuation_id:).update end def start_date diff --git a/app/models/account/balance_updater.rb b/app/models/account/balance_updater.rb index 0eb9a7894ec..1c1b1556e81 100644 --- a/app/models/account/balance_updater.rb +++ b/app/models/account/balance_updater.rb @@ -1,10 +1,11 @@ class Account::BalanceUpdater - def initialize(account, balance:, currency: nil, date: Date.current, notes: nil) + def initialize(account, balance:, currency: nil, date: Date.current, notes: nil, existing_valuation_id: nil) @account = account @balance = balance.to_d @currency = currency @date = date.to_date @notes = notes + @existing_valuation_id = existing_valuation_id end def update @@ -17,10 +18,15 @@ def update account.save! end - valuation_entry = account.entries.valuations.find_or_initialize_by(date: date) do |entry| - entry.entryable = Valuation.new(kind: "reconciliation") + valuation_entry = if existing_valuation_id + account.entries.find(existing_valuation_id) + else + account.entries.valuations.find_or_initialize_by(date: date) do |entry| + entry.entryable = Valuation.new(kind: "reconciliation") + end end + valuation_entry.date = date valuation_entry.amount = balance valuation_entry.currency = currency if currency.present? valuation_entry.name = Valuation.build_reconciliation_name(account.accountable_type) @@ -37,7 +43,7 @@ def update end private - attr_reader :account, :balance, :currency, :date, :notes + attr_reader :account, :balance, :currency, :date, :notes, :existing_valuation_id Result = Struct.new(:success?, :updated?, :error_message) diff --git a/app/models/investment.rb b/app/models/investment.rb index 4e4c25c86b7..10d879e4313 100644 --- a/app/models/investment.rb +++ b/app/models/investment.rb @@ -28,4 +28,19 @@ def icon "line-chart" end end + + def holdings_value_for_date(date) + # Find the most recent holding for each security on or before the given date + # Using a subquery to get the max date for each security + account.holdings + .where(currency: account.currency) + .where("date <= ?", date) + .where("(security_id, date) IN ( + SELECT security_id, MAX(date) as max_date + FROM holdings + WHERE account_id = ? AND date <= ? + GROUP BY security_id + )", account.id, date) + .sum(:amount) + end end diff --git a/app/views/entries/_selection_bar.html.erb b/app/views/entries/_selection_bar.html.erb index 1c633bdedb6..a1faca091a8 100644 --- a/app/views/entries/_selection_bar.html.erb +++ b/app/views/entries/_selection_bar.html.erb @@ -6,7 +6,7 @@
- <%= form_with url: transactions_bulk_deletion_path, data: { turbo_confirm: true, turbo_frame: "_top" } do %> + <%= form_with url: transactions_bulk_deletion_path, data: { turbo_confirm: CustomConfirm.for_resource_deletion("entry").to_data_attribute, turbo_frame: "_top" } do %> diff --git a/app/views/valuations/_confirmation_contents.html.erb b/app/views/valuations/_confirmation_contents.html.erb new file mode 100644 index 00000000000..c3dc37247a2 --- /dev/null +++ b/app/views/valuations/_confirmation_contents.html.erb @@ -0,0 +1,49 @@ +
+ <% if account.investment? %> + <% holdings_value = account.investment.holdings_value_for_date(entry.date) %> + <% brokerage_cash = entry.amount - holdings_value %> + +

This will <%= action_verb %> the account value on <%= entry.date.strftime("%B %d, %Y") %> to:

+ +
+
+ Total account value + <%= entry.amount_money.format %> +
+
+ Holdings value + <%= Money.new(holdings_value, account.currency).format %> +
+
+ Brokerage cash + "><%= Money.new(brokerage_cash, account.currency).format %> +
+
+ <% else %> +

<%= action_verb.capitalize %> + <% if account.depository? %> + account balance + <% elsif account.credit_card? %> + credit card balance + <% elsif account.loan? %> + loan balance + <% elsif account.property? %> + property value + <% elsif account.vehicle? %> + vehicle value + <% elsif account.crypto? %> + crypto balance + <% elsif account.other_asset? %> + asset value + <% elsif account.other_liability? %> + liability balance + <% else %> + balance + <% end %> + on <%= entry.date.strftime("%B %d, %Y") %> to + <%= entry.amount_money.format %>. +

+ <% end %> + +

All future transactions and balances will be recalculated based on this <%= is_update ? "change" : "update" %>.

+
\ No newline at end of file diff --git a/app/views/valuations/_form.html.erb b/app/views/valuations/_form.html.erb deleted file mode 100644 index 5429f3a7051..00000000000 --- a/app/views/valuations/_form.html.erb +++ /dev/null @@ -1,17 +0,0 @@ -<%# locals: (entry:, error_message:) %> - -<%= styled_form_with model: entry, url: valuations_path, class: "space-y-4" do |form| %> - <%= form.hidden_field :account_id %> - - <% if error_message.present? %> - <%= render AlertComponent.new(message: error_message, variant: :error) %> - <% end %> - -
- <%= form.hidden_field :name, value: "Balance update" %> - <%= form.date_field :date, label: true, required: true, value: Date.current, min: Entry.min_supported_date, max: Date.current %> - <%= form.money_field :amount, label: t(".amount"), required: true %> -
- - <%= form.submit t(".submit") %> -<% end %> diff --git a/app/views/valuations/confirm_create.html.erb b/app/views/valuations/confirm_create.html.erb new file mode 100644 index 00000000000..75646450603 --- /dev/null +++ b/app/views/valuations/confirm_create.html.erb @@ -0,0 +1,20 @@ +<%= render DialogComponent.new do |dialog| %> + <% dialog.with_header(title: "Confirm new balance") %> + <% dialog.with_body do %> + <%= styled_form_with model: @entry, url: valuations_path, class: "space-y-4", data: { turbo: false } do |form| %> + <%= form.hidden_field :account_id %> + <%= form.hidden_field :date %> + <%= form.hidden_field :amount %> + <%= form.hidden_field :currency %> + <%= form.hidden_field :notes %> + + <%= render "confirmation_contents", + account: @account, + entry: @entry, + action_verb: "set", + is_update: false %> + + <%= form.submit "Confirm" %> + <% end %> + <% end %> +<% end %> diff --git a/app/views/valuations/confirm_update.html.erb b/app/views/valuations/confirm_update.html.erb new file mode 100644 index 00000000000..43c67c373d2 --- /dev/null +++ b/app/views/valuations/confirm_update.html.erb @@ -0,0 +1,19 @@ +<%= render DialogComponent.new do |dialog| %> + <% dialog.with_header(title: "Update balance") %> + <% dialog.with_body do %> + <%= styled_form_with model: @entry, url: valuation_path(@entry), method: :patch, class: "space-y-4", data: { turbo: false } do |form| %> + <%= form.hidden_field :date %> + <%= form.hidden_field :amount %> + <%= form.hidden_field :currency %> + <%= form.hidden_field :notes %> + + <%= render "confirmation_contents", + account: @account, + entry: @entry, + action_verb: "update", + is_update: true %> + + <%= form.submit "Update" %> + <% end %> + <% end %> +<% end %> \ No newline at end of file diff --git a/app/views/valuations/new.html.erb b/app/views/valuations/new.html.erb index 82102f16af5..bfa8d5e20c5 100644 --- a/app/views/valuations/new.html.erb +++ b/app/views/valuations/new.html.erb @@ -1,6 +1,19 @@ <%= render DialogComponent.new do |dialog| %> <% dialog.with_header(title: t(".title")) %> <% dialog.with_body do %> - <%= render "form", entry: @entry, error_message: @error_message %> + <%= styled_form_with model: @entry, url: confirm_create_valuations_path, class: "space-y-4" do |form| %> + <%= form.hidden_field :account_id %> + + <% if @error_message.present? %> + <%= render AlertComponent.new(message: @error_message, variant: :error) %> + <% end %> + +
+ <%= form.date_field :date, label: true, required: true, value: Date.current, min: Entry.min_supported_date, max: Date.current %> + <%= form.money_field :amount, label: t(".amount"), required: true, disable_currency: true %> +
+ + <%= form.submit t(".submit") %> + <% end %> <% end %> <% end %> diff --git a/app/views/valuations/show.html.erb b/app/views/valuations/show.html.erb index 6e96dcaa146..b4156e393c6 100644 --- a/app/views/valuations/show.html.erb +++ b/app/views/valuations/show.html.erb @@ -15,18 +15,25 @@ <% dialog.with_section(title: t(".overview"), open: true) do %>
<%= styled_form_with model: entry, - url: entry_path(entry), - class: "space-y-2", - data: { controller: "auto-submit-form" } do |f| %> + url: confirm_update_valuation_path(entry), + method: :post, + data: { turbo_frame: :modal }, + class: "space-y-4" do |f| %> <%= f.date_field :date, label: t(".date_label"), - max: Date.current, - "data-auto-submit-form-target": "auto" %> + max: Date.current %> <%= f.money_field :amount, label: t(".amount"), - auto_submit: true, disable_currency: true %> + +
+ <%= render ButtonComponent.new( + text: "Update value", + variant: :primary, + type: "submit" + ) %> +
<% end %>
<% end %> @@ -34,9 +41,13 @@ <% dialog.with_section(title: t(".details")) do %>
<%= styled_form_with model: entry, - url: entry_path(entry), + url: valuation_path(entry), + method: :patch, class: "space-y-2", data: { controller: "auto-submit-form" } do |f| %> + <%= f.hidden_field :date, value: entry.date %> + <%= f.hidden_field :amount, value: entry.amount %> + <%= f.hidden_field :currency, value: entry.currency %> <%= f.text_area :notes, label: t(".note_label"), placeholder: t(".note_placeholder"), @@ -59,7 +70,7 @@ entry_path(entry), method: :delete, class: "rounded-lg px-3 py-2 text-red-500 text-sm font-medium border border-secondary", - data: { turbo_confirm: true, turbo_frame: "_top" } %> + data: { turbo_confirm: CustomConfirm.for_resource_deletion("value update").to_data_attribute, turbo_frame: "_top" } %>
<% end %> diff --git a/config/routes.rb b/config/routes.rb index f04142873b9..3dd1d7f1899 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -110,7 +110,10 @@ resources :holdings, only: %i[index new show destroy] resources :trades, only: %i[show new create update destroy] - resources :valuations, only: %i[show new create update destroy] + resources :valuations, only: %i[show new create update destroy] do + post :confirm_create, on: :collection + post :confirm_update, on: :member + end namespace :transactions do resource :bulk_deletion, only: :create diff --git a/test/controllers/valuations_controller_test.rb b/test/controllers/valuations_controller_test.rb index 52c62ad4cde..4746eb323ea 100644 --- a/test/controllers/valuations_controller_test.rb +++ b/test/controllers/valuations_controller_test.rb @@ -15,7 +15,6 @@ class ValuationsControllerTest < ActionDispatch::IntegrationTest post valuations_url, params: { entry: { amount: account.balance + 100, - currency: "USD", date: Date.current.to_s, account_id: account.id } @@ -37,7 +36,6 @@ class ValuationsControllerTest < ActionDispatch::IntegrationTest patch valuation_url(@entry), params: { entry: { amount: 20000, - currency: "USD", date: Date.current } } From 52333e3fa69d61fb69f329fadec9899393fa88b6 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Wed, 16 Jul 2025 11:31:47 -0400 Subject: [PATCH 03/29] Add reconciliation manager (#2459) * Add reconciliation manager * Fix notes editing --- app/controllers/valuations_controller.rb | 53 +++++---- app/models/account.rb | 2 +- app/models/account/reconcileable.rb | 16 +++ app/models/account/reconciliation_manager.rb | 90 +++++++++++++++ app/models/investment.rb | 15 --- .../_confirmation_contents.html.erb | 14 ++- app/views/valuations/confirm_create.html.erb | 13 +-- app/views/valuations/confirm_update.html.erb | 15 ++- app/views/valuations/show.html.erb | 5 +- .../controllers/valuations_controller_test.rb | 11 +- .../account/reconciliation_manager_test.rb | 103 ++++++++++++++++++ 11 files changed, 273 insertions(+), 64 deletions(-) create mode 100644 app/models/account/reconcileable.rb create mode 100644 app/models/account/reconciliation_manager.rb create mode 100644 test/models/account/reconciliation_manager_test.rb diff --git a/app/controllers/valuations_controller.rb b/app/controllers/valuations_controller.rb index 85f3413ab3b..5234c0beaaa 100644 --- a/app/controllers/valuations_controller.rb +++ b/app/controllers/valuations_controller.rb @@ -5,6 +5,12 @@ def confirm_create @account = Current.family.accounts.find(params.dig(:entry, :account_id)) @entry = @account.entries.build(entry_params.merge(currency: @account.currency)) + @reconciliation_dry_run = @entry.account.create_reconciliation( + balance: entry_params[:amount], + date: entry_params[:date], + dry_run: true + ) + render :confirm_create end @@ -13,19 +19,28 @@ def confirm_update @account = @entry.account @entry.assign_attributes(entry_params.merge(currency: @account.currency)) + @reconciliation_dry_run = @entry.account.update_reconciliation( + @entry, + balance: entry_params[:amount], + date: entry_params[:date], + dry_run: true + ) + render :confirm_update end def create account = Current.family.accounts.find(params.dig(:entry, :account_id)) - result = perform_balance_update(account, entry_params.merge(currency: account.currency)) - if result.success? - @success_message = result.updated? ? "Balance updated" : "No changes made. Account is already up to date." + result = account.create_reconciliation( + balance: entry_params[:amount], + date: entry_params[:date], + ) + if result.success? respond_to do |format| - format.html { redirect_back_or_to account_path(account), notice: @success_message } - format.turbo_stream { stream_redirect_back_or_to(account_path(account), notice: @success_message) } + format.html { redirect_back_or_to account_path(account), notice: "Account updated" } + format.turbo_stream { stream_redirect_back_or_to(account_path(account), notice: "Account updated") } end else @error_message = result.error_message @@ -34,13 +49,22 @@ def create end def update - result = perform_balance_update(@entry.account, entry_params.merge(currency: @entry.currency, existing_valuation_id: @entry.id)) + # Notes updating is independent of reconciliation, just a simple CRUD operation + @entry.update!(notes: entry_params[:notes]) if entry_params[:notes].present? - if result.success? + if entry_params[:date].present? && entry_params[:amount].present? + result = @entry.account.update_reconciliation( + @entry, + balance: entry_params[:amount], + date: entry_params[:date], + ) + end + + if result.nil? || result.success? @entry.reload respond_to do |format| - format.html { redirect_back_or_to account_path(@entry.account), notice: result.updated? ? "Balance updated" : "No changes made. Account is already up to date." } + format.html { redirect_back_or_to account_path(@entry.account), notice: "Entry updated" } format.turbo_stream do render turbo_stream: [ turbo_stream.replace( @@ -60,17 +84,6 @@ def update private def entry_params - params.require(:entry) - .permit(:date, :amount, :currency, :notes) - end - - def perform_balance_update(account, params) - account.update_balance( - balance: params[:amount], - date: params[:date], - currency: params[:currency], - notes: params[:notes], - existing_valuation_id: params[:existing_valuation_id] - ) + params.require(:entry).permit(:date, :amount, :notes) end end diff --git a/app/models/account.rb b/app/models/account.rb index 837d6e57e7b..684736ce763 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -1,5 +1,5 @@ class Account < ApplicationRecord - include AASM, Syncable, Monetizable, Chartable, Linkable, Enrichable, Anchorable + include AASM, Syncable, Monetizable, Chartable, Linkable, Enrichable, Anchorable, Reconcileable validates :name, :balance, :currency, presence: true diff --git a/app/models/account/reconcileable.rb b/app/models/account/reconcileable.rb new file mode 100644 index 00000000000..bad855a92b4 --- /dev/null +++ b/app/models/account/reconcileable.rb @@ -0,0 +1,16 @@ +module Account::Reconcileable + extend ActiveSupport::Concern + + def create_reconciliation(balance:, date:, dry_run: false) + reconciliation_manager.reconcile_balance(balance: balance, date: date, dry_run: dry_run) + end + + def update_reconciliation(existing_valuation_entry, balance:, date:, dry_run: false) + reconciliation_manager.reconcile_balance(balance: balance, date: date, existing_valuation_entry: existing_valuation_entry, dry_run: dry_run) + end + + private + def reconciliation_manager + @reconciliation_manager ||= Account::ReconciliationManager.new(self) + end +end diff --git a/app/models/account/reconciliation_manager.rb b/app/models/account/reconciliation_manager.rb new file mode 100644 index 00000000000..e1300143d81 --- /dev/null +++ b/app/models/account/reconciliation_manager.rb @@ -0,0 +1,90 @@ +class Account::ReconciliationManager + attr_reader :account + + def initialize(account) + @account = account + end + + # Reconciles balance by creating a Valuation entry. If existing valuation is provided, it will be updated instead of creating a new one. + def reconcile_balance(balance:, date: Date.current, dry_run: false, existing_valuation_entry: nil) + old_balance_components = old_balance_components(reconciliation_date: date, existing_valuation_entry: existing_valuation_entry) + prepared_valuation = prepare_reconciliation(balance, date, existing_valuation_entry) + + unless dry_run + prepared_valuation.save! + account.sync_later + end + + ReconciliationResult.new( + success?: true, + old_cash_balance: old_balance_components[:cash_balance], + old_balance: old_balance_components[:balance], + new_cash_balance: derived_cash_balance(date: date, total_balance: prepared_valuation.amount), + new_balance: prepared_valuation.amount, + error_message: nil + ) + rescue => e + ReconciliationResult.new( + success?: false, + error_message: e.message + ) + end + + private + # Returns before -> after OR error message + ReconciliationResult = Struct.new( + :success?, + :old_cash_balance, + :old_balance, + :new_cash_balance, + :new_balance, + :error_message, + keyword_init: true + ) + + def prepare_reconciliation(balance, date, existing_valuation) + valuation_record = existing_valuation || + account.entries.valuations.find_by(date: date) || # In case of conflict, where existing valuation is not passed as arg, but one exists + account.entries.build( + name: Valuation.build_reconciliation_name(account.accountable_type), + entryable: Valuation.new(kind: "reconciliation") + ) + + valuation_record.assign_attributes( + date: date, + amount: balance, + currency: account.currency + ) + + valuation_record + end + + def derived_cash_balance(date:, total_balance:) + balance_components_for_reconciliation_date = get_balance_components_for_date(date) + + return nil unless balance_components_for_reconciliation_date[:balance] && balance_components_for_reconciliation_date[:cash_balance] + + # We calculate the existing non-cash balance, which for investments would represents "holdings" for the date of reconciliation + # Since the user is setting "total balance", we have to subtract the existing non-cash balance from the total balance to get the new cash balance + existing_non_cash_balance = balance_components_for_reconciliation_date[:balance] - balance_components_for_reconciliation_date[:cash_balance] + + total_balance - existing_non_cash_balance + end + + def old_balance_components(reconciliation_date:, existing_valuation_entry: nil) + if existing_valuation_entry + get_balance_components_for_date(existing_valuation_entry.date) + else + get_balance_components_for_date(reconciliation_date) + end + end + + def get_balance_components_for_date(date) + balance_record = account.balances.find_by(date: date, currency: account.currency) + + { + cash_balance: balance_record&.cash_balance, + balance: balance_record&.balance + } + end +end diff --git a/app/models/investment.rb b/app/models/investment.rb index 10d879e4313..4e4c25c86b7 100644 --- a/app/models/investment.rb +++ b/app/models/investment.rb @@ -28,19 +28,4 @@ def icon "line-chart" end end - - def holdings_value_for_date(date) - # Find the most recent holding for each security on or before the given date - # Using a subquery to get the max date for each security - account.holdings - .where(currency: account.currency) - .where("date <= ?", date) - .where("(security_id, date) IN ( - SELECT security_id, MAX(date) as max_date - FROM holdings - WHERE account_id = ? AND date <= ? - GROUP BY security_id - )", account.id, date) - .sum(:amount) - end end diff --git a/app/views/valuations/_confirmation_contents.html.erb b/app/views/valuations/_confirmation_contents.html.erb index c3dc37247a2..19d2ff5f8e2 100644 --- a/app/views/valuations/_confirmation_contents.html.erb +++ b/app/views/valuations/_confirmation_contents.html.erb @@ -1,14 +1,16 @@ +<%# locals: (account:, entry:, reconciliation_dry_run:, is_update:, action_verb:) %> +
<% if account.investment? %> - <% holdings_value = account.investment.holdings_value_for_date(entry.date) %> - <% brokerage_cash = entry.amount - holdings_value %> + <% holdings_value = reconciliation_dry_run.new_balance - reconciliation_dry_run.new_cash_balance %> + <% brokerage_cash = reconciliation_dry_run.new_cash_balance %>

This will <%= action_verb %> the account value on <%= entry.date.strftime("%B %d, %Y") %> to:

Total account value - <%= entry.amount_money.format %> + <%= Money.new(reconciliation_dry_run.new_balance, account.currency).format %>
Holdings value @@ -20,7 +22,7 @@
<% else %> -

<%= action_verb.capitalize %> +

<%= action_verb.capitalize %> <% if account.depository? %> account balance <% elsif account.credit_card? %> @@ -40,10 +42,10 @@ <% else %> balance <% end %> - on <%= entry.date.strftime("%B %d, %Y") %> to + on <%= entry.date.strftime("%B %d, %Y") %> to <%= entry.amount_money.format %>.

<% end %>

All future transactions and balances will be recalculated based on this <%= is_update ? "change" : "update" %>.

-
\ No newline at end of file + diff --git a/app/views/valuations/confirm_create.html.erb b/app/views/valuations/confirm_create.html.erb index 75646450603..3e3a4a90acc 100644 --- a/app/views/valuations/confirm_create.html.erb +++ b/app/views/valuations/confirm_create.html.erb @@ -1,17 +1,16 @@ <%= render DialogComponent.new do |dialog| %> <% dialog.with_header(title: "Confirm new balance") %> <% dialog.with_body do %> - <%= styled_form_with model: @entry, url: valuations_path, class: "space-y-4", data: { turbo: false } do |form| %> + <%= styled_form_with model: @entry, url: valuations_path, class: "space-y-4" do |form| %> <%= form.hidden_field :account_id %> <%= form.hidden_field :date %> <%= form.hidden_field :amount %> - <%= form.hidden_field :currency %> - <%= form.hidden_field :notes %> - <%= render "confirmation_contents", - account: @account, - entry: @entry, - action_verb: "set", + <%= render "confirmation_contents", + reconciliation_dry_run: @reconciliation_dry_run, + account: @account, + entry: @entry, + action_verb: "set", is_update: false %> <%= form.submit "Confirm" %> diff --git a/app/views/valuations/confirm_update.html.erb b/app/views/valuations/confirm_update.html.erb index 43c67c373d2..c24e27cf158 100644 --- a/app/views/valuations/confirm_update.html.erb +++ b/app/views/valuations/confirm_update.html.erb @@ -1,19 +1,18 @@ <%= render DialogComponent.new do |dialog| %> <% dialog.with_header(title: "Update balance") %> <% dialog.with_body do %> - <%= styled_form_with model: @entry, url: valuation_path(@entry), method: :patch, class: "space-y-4", data: { turbo: false } do |form| %> + <%= styled_form_with model: @entry, url: valuation_path(@entry), method: :patch, class: "space-y-4", data: { turbo_frame: :_top } do |form| %> <%= form.hidden_field :date %> <%= form.hidden_field :amount %> - <%= form.hidden_field :currency %> - <%= form.hidden_field :notes %> - <%= render "confirmation_contents", - account: @account, - entry: @entry, - action_verb: "update", + <%= render "confirmation_contents", + reconciliation_dry_run: @reconciliation_dry_run, + account: @account, + entry: @entry, + action_verb: "update", is_update: true %> <%= form.submit "Update" %> <% end %> <% end %> -<% end %> \ No newline at end of file +<% end %> diff --git a/app/views/valuations/show.html.erb b/app/views/valuations/show.html.erb index b4156e393c6..3cd44f9f151 100644 --- a/app/views/valuations/show.html.erb +++ b/app/views/valuations/show.html.erb @@ -44,10 +44,7 @@ url: valuation_path(entry), method: :patch, class: "space-y-2", - data: { controller: "auto-submit-form" } do |f| %> - <%= f.hidden_field :date, value: entry.date %> - <%= f.hidden_field :amount, value: entry.amount %> - <%= f.hidden_field :currency, value: entry.currency %> + data: { controller: "auto-submit-form", auto_submit_form_trigger_event_value: "blur" } do |f| %> <%= f.text_area :notes, label: t(".note_label"), placeholder: t(".note_placeholder"), diff --git a/test/controllers/valuations_controller_test.rb b/test/controllers/valuations_controller_test.rb index 4746eb323ea..7827906b8f5 100644 --- a/test/controllers/valuations_controller_test.rb +++ b/test/controllers/valuations_controller_test.rb @@ -8,7 +8,7 @@ class ValuationsControllerTest < ActionDispatch::IntegrationTest @entry = entries(:valuation) end - test "creates entry with basic attributes" do + test "can create reconciliation" do account = accounts(:investment) assert_difference [ "Entry.count", "Valuation.count" ], 1 do @@ -35,8 +35,9 @@ class ValuationsControllerTest < ActionDispatch::IntegrationTest assert_no_difference [ "Entry.count", "Valuation.count" ] do patch valuation_url(@entry), params: { entry: { - amount: 20000, - date: Date.current + amount: 22000, + date: Date.current, + notes: "Test notes" } } end @@ -44,5 +45,9 @@ class ValuationsControllerTest < ActionDispatch::IntegrationTest assert_enqueued_with job: SyncJob assert_redirected_to account_url(@entry.account) + + @entry.reload + assert_equal 22000, @entry.amount + assert_equal "Test notes", @entry.notes end end diff --git a/test/models/account/reconciliation_manager_test.rb b/test/models/account/reconciliation_manager_test.rb new file mode 100644 index 00000000000..0e7a39a9e53 --- /dev/null +++ b/test/models/account/reconciliation_manager_test.rb @@ -0,0 +1,103 @@ +require "test_helper" + +class Account::ReconciliationManagerTest < ActiveSupport::TestCase + setup do + @account = accounts(:investment) + @manager = Account::ReconciliationManager.new(@account) + end + + test "new reconciliation" do + @account.balances.create!( + date: Date.current, + balance: 1000, + cash_balance: 500, + currency: @account.currency + ) + + result = @manager.reconcile_balance(balance: 1200, date: Date.current) + + assert_equal 1200, result.new_balance + assert_equal 700, result.new_cash_balance # Non cash stays the same since user is valuing the entire account balance + assert_equal 1000, result.old_balance + assert_equal 500, result.old_cash_balance + assert_equal true, result.success? + end + + test "updates existing reconciliation without date change" do + @account.balances.create!(date: Date.current, balance: 1000, cash_balance: 500, currency: @account.currency) + + # Existing reconciliation entry + existing_entry = @account.entries.create!(name: "Test", amount: 1000, date: Date.current, entryable: Valuation.new(kind: "reconciliation"), currency: @account.currency) + + result = @manager.reconcile_balance(balance: 1200, date: Date.current, existing_valuation_entry: existing_entry) + + assert_equal 1200, result.new_balance + assert_equal 700, result.new_cash_balance # Non cash stays the same since user is valuing the entire account balance + assert_equal 1000, result.old_balance + assert_equal 500, result.old_cash_balance + assert_equal true, result.success? + end + + test "updates existing reconciliation with date and amount change" do + @account.balances.create!(date: 5.days.ago, balance: 1000, cash_balance: 500, currency: @account.currency) + @account.balances.create!(date: Date.current, balance: 1200, cash_balance: 700, currency: @account.currency) + + # Existing reconciliation entry (5 days ago) + existing_entry = @account.entries.create!(name: "Test", amount: 1000, date: 5.days.ago, entryable: Valuation.new(kind: "reconciliation"), currency: @account.currency) + + # Should update and change date for existing entry; not create a new one + assert_no_difference "Valuation.count" do + # "Update valuation from 5 days ago to today, set balance from 1000 to 1500" + result = @manager.reconcile_balance(balance: 1500, date: Date.current, existing_valuation_entry: existing_entry) + + assert_equal true, result.success? + + # Reconciliation + assert_equal 1500, result.new_balance # Equal to new valuation amount + assert_equal 1000, result.new_cash_balance # Get non-cash balance today (1200 - 700 = 500). Then subtract this from new valuation (1500 - 500 = 1000) + + # Prior valuation + assert_equal 1000, result.old_balance # This is the balance from the old valuation, NOT the date we're reconciling to + assert_equal 500, result.old_cash_balance + end + end + + test "handles date conflicts" do + @account.balances.create!( + date: Date.current, + balance: 1000, + cash_balance: 1000, + currency: @account.currency + ) + + # Existing reconciliation entry + @account.entries.create!( + name: "Test", + amount: 1000, + date: Date.current, + entryable: Valuation.new(kind: "reconciliation"), + currency: @account.currency + ) + + # Doesn't pass existing_valuation_entry, but reconciliation manager should recognize its the same date and update the existing entry + assert_no_difference "Valuation.count" do + result = @manager.reconcile_balance(balance: 1200, date: Date.current) + + assert result.success? + assert_equal 1200, result.new_balance + end + end + + test "dry run does not persist or sync account" do + @account.balances.create!(date: Date.current, balance: 1000, cash_balance: 500, currency: @account.currency) + + assert_no_difference "Valuation.count" do + @manager.reconcile_balance(balance: 1200, date: Date.current, dry_run: true) + end + + assert_difference "Valuation.count", 1 do + @account.expects(:sync_later).once + @manager.reconcile_balance(balance: 1200, date: Date.current) + end + end +end From 3eea5a98910a5265bdbfc90e63d717fd3db666c0 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Thu, 17 Jul 2025 06:49:56 -0400 Subject: [PATCH 04/29] Add auto-update strategies for current balance on manual accounts (#2460) * Add auto-update strategies for current balance on manual accounts * Remove deprecated BalanceUpdater, replace with new methods --- .../concerns/accountable_resource.rb | 3 +- app/controllers/properties_controller.rb | 4 +- app/models/account.rb | 24 +- app/models/account/anchorable.rb | 10 +- app/models/account/balance_updater.rb | 53 ---- app/models/account/current_balance_manager.rb | 73 +++++- app/models/account/reconcileable.rb | 8 +- app/models/account/reconciliation_manager.rb | 1 - app/models/balance/base_calculator.rb | 23 +- app/models/plaid_account/processor.rb | 2 +- .../controllers/properties_controller_test.rb | 8 +- .../account/current_balance_manager_test.rb | 235 +++++++++++++++--- .../account/reconciliation_manager_test.rb | 3 +- 13 files changed, 311 insertions(+), 136 deletions(-) delete mode 100644 app/models/account/balance_updater.rb diff --git a/app/controllers/concerns/accountable_resource.rb b/app/controllers/concerns/accountable_resource.rb index 9daa0ae2f82..a508764d5d4 100644 --- a/app/controllers/concerns/accountable_resource.rb +++ b/app/controllers/concerns/accountable_resource.rb @@ -45,12 +45,13 @@ def create def update # Handle balance update if provided if account_params[:balance].present? - result = @account.update_balance(balance: account_params[:balance], currency: account_params[:currency]) + result = @account.set_current_balance(account_params[:balance].to_d) unless result.success? @error_message = result.error_message render :edit, status: :unprocessable_entity return end + @account.sync_later end # Update remaining account attributes diff --git a/app/controllers/properties_controller.rb b/app/controllers/properties_controller.rb index 8b2ec062ce3..7a1db5de3eb 100644 --- a/app/controllers/properties_controller.rb +++ b/app/controllers/properties_controller.rb @@ -37,10 +37,10 @@ def balances end def update_balances - result = @account.update_balance(balance: balance_params[:balance], currency: balance_params[:currency]) + result = @account.set_current_balance(balance_params[:balance].to_d) if result.success? - @success_message = result.updated? ? "Balance updated successfully." : "No changes made. Account is already up to date." + @success_message = "Balance updated successfully." if @account.active? render :balances diff --git a/app/models/account.rb b/app/models/account.rb index 684736ce763..6a21c3e341c 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -114,11 +114,6 @@ def current_holdings .order(amount: :desc) end - - def update_balance(balance:, date: Date.current, currency: nil, notes: nil, existing_valuation_id: nil) - Account::BalanceUpdater.new(self, balance:, currency:, date:, notes:, existing_valuation_id:).update - end - def start_date first_entry_date = entries.minimum(:date) || Date.current first_entry_date - 1.day @@ -146,4 +141,23 @@ def short_subtype_label def long_subtype_label accountable_class.long_subtype_label_for(subtype) || accountable_class.display_name end + + # The balance type determines which "component" of balance is being tracked. + # This is primarily used for balance related calculations and updates. + # + # "Cash" = "Liquid" + # "Non-cash" = "Illiquid" + # "Investment" = A mix of both, including brokerage cash (liquid) and holdings (illiquid) + def balance_type + case accountable_type + when "Depository", "CreditCard" + :cash + when "Property", "Vehicle", "OtherAsset", "Loan", "OtherLiability" + :non_cash + when "Investment", "Crypto" + :investment + else + raise "Unknown account type: #{accountable_type}" + end + end end diff --git a/app/models/account/anchorable.rb b/app/models/account/anchorable.rb index 750fb067219..bb797c8515a 100644 --- a/app/models/account/anchorable.rb +++ b/app/models/account/anchorable.rb @@ -10,7 +10,9 @@ module Account::Anchorable end def set_opening_anchor_balance(**opts) - opening_balance_manager.set_opening_balance(**opts) + result = opening_balance_manager.set_opening_balance(**opts) + sync_later if result.success? + result end def opening_anchor_date @@ -25,8 +27,10 @@ def has_opening_anchor? opening_balance_manager.has_opening_anchor? end - def set_current_anchor_balance(balance) - current_balance_manager.set_current_balance(balance) + def set_current_balance(balance) + result = current_balance_manager.set_current_balance(balance) + sync_later if result.success? + result end def current_anchor_balance diff --git a/app/models/account/balance_updater.rb b/app/models/account/balance_updater.rb deleted file mode 100644 index 1c1b1556e81..00000000000 --- a/app/models/account/balance_updater.rb +++ /dev/null @@ -1,53 +0,0 @@ -class Account::BalanceUpdater - def initialize(account, balance:, currency: nil, date: Date.current, notes: nil, existing_valuation_id: nil) - @account = account - @balance = balance.to_d - @currency = currency - @date = date.to_date - @notes = notes - @existing_valuation_id = existing_valuation_id - end - - def update - return Result.new(success?: true, updated?: false) unless requires_update? - - Account.transaction do - if date == Date.current - account.balance = balance - account.currency = currency if currency.present? - account.save! - end - - valuation_entry = if existing_valuation_id - account.entries.find(existing_valuation_id) - else - account.entries.valuations.find_or_initialize_by(date: date) do |entry| - entry.entryable = Valuation.new(kind: "reconciliation") - end - end - - valuation_entry.date = date - valuation_entry.amount = balance - valuation_entry.currency = currency if currency.present? - valuation_entry.name = Valuation.build_reconciliation_name(account.accountable_type) - valuation_entry.notes = notes if notes.present? - valuation_entry.save! - end - - account.sync_later - - Result.new(success?: true, updated?: true) - rescue => e - message = Rails.env.development? ? e.message : "Unable to update account values. Please try again." - Result.new(success?: false, updated?: false, error_message: message) - end - - private - attr_reader :account, :balance, :currency, :date, :notes, :existing_valuation_id - - Result = Struct.new(:success?, :updated?, :error_message) - - def requires_update? - date != Date.current || account.balance != balance || account.currency != currency - end -end diff --git a/app/models/account/current_balance_manager.rb b/app/models/account/current_balance_manager.rb index 243032134f1..2b1ba630948 100644 --- a/app/models/account/current_balance_manager.rb +++ b/app/models/account/current_balance_manager.rb @@ -31,22 +31,77 @@ def current_date end def set_current_balance(balance) - # A current balance anchor implies there is an external data source that will keep it updated. Since manual accounts - # are tracked by the user, a current balance anchor is not appropriate. - raise InvalidOperation, "Manual accounts cannot set current balance anchor. Set opening balance or use a reconciliation instead." if account.manual? - - if current_anchor_valuation - changes_made = update_current_anchor(balance) - Result.new(success?: true, changes_made?: changes_made, error: nil) + if account.linked? + result = set_current_balance_for_linked_account(balance) else - create_current_anchor(balance) - Result.new(success?: true, changes_made?: true, error: nil) + result = set_current_balance_for_manual_account(balance) end + + # Update cache field so changes appear immediately to the user + account.update!(balance: balance) + + result + rescue => e + Result.new(success?: false, changes_made?: false, error: e.message) end private attr_reader :account + def opening_balance_manager + @opening_balance_manager ||= Account::OpeningBalanceManager.new(account) + end + + def reconciliation_manager + @reconciliation_manager ||= Account::ReconciliationManager.new(account) + end + + # Manual accounts do not manage the `current_anchor` valuation (otherwise, user would need to continually update it, which is bad UX) + # Instead, we use a combination of "auto-update strategies" to set the current balance according to the user's intent. + # + # The "auto-update strategies" are: + # 1. Value tracking - If the account has a reconciliation already, we assume they are tracking the account value primarily with reconciliations, so we append a new one + # 2. Transaction adjustment - If the account doesn't have recons, we assume user is tracking with transactions, so we adjust the opening balance with a delta until it + # gets us to the desired balance. This ensures we don't append unnecessary reconciliations to the account, which "reset" the value from that + # date forward (not user's intent). + # + # For more documentation on these auto-update strategies, see the test cases. + def set_current_balance_for_manual_account(balance) + # If we're dealing with a cash account that has no reconciliations, use "Transaction adjustment" strategy (update opening balance to "back in" to the desired current balance) + if account.balance_type == :cash && account.valuations.reconciliation.empty? + adjust_opening_balance_with_delta(new_balance: balance, old_balance: account.balance) + else + existing_reconciliation = account.entries.valuations.find_by(date: Date.current) + + result = reconciliation_manager.reconcile_balance(balance: balance, date: Date.current, existing_valuation_entry: existing_reconciliation) + + # Normalize to expected result format + Result.new(success?: result.success?, changes_made?: true, error: result.error_message) + end + end + + def adjust_opening_balance_with_delta(new_balance:, old_balance:) + delta = new_balance - old_balance + + result = opening_balance_manager.set_opening_balance(balance: account.opening_anchor_balance + delta) + + # Normalize to expected result format + Result.new(success?: result.success?, changes_made?: true, error: result.error) + end + + # Linked accounts manage "current balance" via the special `current_anchor` valuation. + # This is NOT a user-facing feature, and is primarily used in "processors" while syncing + # linked account data (e.g. via Plaid) + def set_current_balance_for_linked_account(balance) + if current_anchor_valuation + changes_made = update_current_anchor(balance) + Result.new(success?: true, changes_made?: changes_made, error: nil) + else + create_current_anchor(balance) + Result.new(success?: true, changes_made?: true, error: nil) + end + end + def current_anchor_valuation @current_anchor_valuation ||= account.valuations.current_anchor.includes(:entry).first end diff --git a/app/models/account/reconcileable.rb b/app/models/account/reconcileable.rb index bad855a92b4..b88052366d9 100644 --- a/app/models/account/reconcileable.rb +++ b/app/models/account/reconcileable.rb @@ -2,11 +2,15 @@ module Account::Reconcileable extend ActiveSupport::Concern def create_reconciliation(balance:, date:, dry_run: false) - reconciliation_manager.reconcile_balance(balance: balance, date: date, dry_run: dry_run) + result = reconciliation_manager.reconcile_balance(balance: balance, date: date, dry_run: dry_run) + sync_later if result.success? + result end def update_reconciliation(existing_valuation_entry, balance:, date:, dry_run: false) - reconciliation_manager.reconcile_balance(balance: balance, date: date, existing_valuation_entry: existing_valuation_entry, dry_run: dry_run) + result = reconciliation_manager.reconcile_balance(balance: balance, date: date, existing_valuation_entry: existing_valuation_entry, dry_run: dry_run) + sync_later if result.success? + result end private diff --git a/app/models/account/reconciliation_manager.rb b/app/models/account/reconciliation_manager.rb index e1300143d81..aac821b260a 100644 --- a/app/models/account/reconciliation_manager.rb +++ b/app/models/account/reconciliation_manager.rb @@ -12,7 +12,6 @@ def reconcile_balance(balance:, date: Date.current, dry_run: false, existing_val unless dry_run prepared_valuation.save! - account.sync_later end ReconciliationResult.new( diff --git a/app/models/balance/base_calculator.rb b/app/models/balance/base_calculator.rb index 3360bcec4b6..92ef5d3e7f2 100644 --- a/app/models/balance/base_calculator.rb +++ b/app/models/balance/base_calculator.rb @@ -20,9 +20,9 @@ def holdings_value_for_date(date) end def derive_cash_balance_on_date_from_total(total_balance:, date:) - if balance_type == :investment + if account.balance_type == :investment total_balance - holdings_value_for_date(date) - elsif balance_type == :cash + elsif account.balance_type == :cash total_balance else 0 @@ -32,7 +32,7 @@ def derive_cash_balance_on_date_from_total(total_balance:, date:) def derive_cash_balance(cash_balance, date) entries = sync_cache.get_entries(date) - if balance_type == :non_cash + if account.balance_type == :non_cash 0 else cash_balance + signed_entry_flows(entries) @@ -42,9 +42,9 @@ def derive_cash_balance(cash_balance, date) def derive_non_cash_balance(non_cash_balance, date, direction: :forward) entries = sync_cache.get_entries(date) # Loans are a special case (loan payment reducing principal, which is non-cash) - if balance_type == :non_cash && account.accountable_type == "Loan" + if account.balance_type == :non_cash && account.accountable_type == "Loan" non_cash_balance + signed_entry_flows(entries) - elsif balance_type == :investment + elsif account.balance_type == :investment # For reverse calculations, we need the previous day's holdings target_date = direction == :forward ? date : date.prev_day holdings_value_for_date(target_date) @@ -57,19 +57,6 @@ def signed_entry_flows(entries) raise NotImplementedError, "Directional calculators must implement this method" end - def balance_type - case account.accountable_type - when "Depository", "CreditCard" - :cash - when "Property", "Vehicle", "OtherAsset", "Loan", "OtherLiability" - :non_cash - when "Investment", "Crypto" - :investment - else - raise "Unknown account type: #{account.accountable_type}" - end - end - def build_balance(date:, cash_balance:, non_cash_balance:) Balance.new( account_id: account.id, diff --git a/app/models/plaid_account/processor.rb b/app/models/plaid_account/processor.rb index 5b16f90dbae..b42bdf3b6a7 100644 --- a/app/models/plaid_account/processor.rb +++ b/app/models/plaid_account/processor.rb @@ -57,7 +57,7 @@ def process_account! # to properly track the holdings vs. cash breakdown, but for now we're only tracking # the total balance in the current anchor. The cash_balance field on the account model # is still being used for the breakdown. - account.set_current_anchor_balance(balance_calculator.balance) + account.set_current_balance(balance_calculator.balance) end end diff --git a/test/controllers/properties_controller_test.rb b/test/controllers/properties_controller_test.rb index b5f1305ff80..34f76734dba 100644 --- a/test/controllers/properties_controller_test.rb +++ b/test/controllers/properties_controller_test.rb @@ -71,10 +71,6 @@ class PropertiesControllerTest < ActionDispatch::IntegrationTest test "updates balances tab" do original_balance = @account.balance - # Mock the update_balance method to return a successful result - Account::BalanceUpdater::Result.any_instance.stubs(:success?).returns(true) - Account::BalanceUpdater::Result.any_instance.stubs(:updated?).returns(true) - patch update_balances_property_path(@account), params: { account: { balance: 600000, @@ -116,9 +112,7 @@ class PropertiesControllerTest < ActionDispatch::IntegrationTest end test "balances update handles validation errors" do - # Mock update_balance to return a failure result - Account::BalanceUpdater::Result.any_instance.stubs(:success?).returns(false) - Account::BalanceUpdater::Result.any_instance.stubs(:error_message).returns("Invalid balance") + Account.any_instance.stubs(:set_current_balance).returns(OpenStruct.new(success?: false, error_message: "Invalid balance")) patch update_balances_property_path(@account), params: { account: { diff --git a/test/models/account/current_balance_manager_test.rb b/test/models/account/current_balance_manager_test.rb index d48eb927bce..0d7b914beca 100644 --- a/test/models/account/current_balance_manager_test.rb +++ b/test/models/account/current_balance_manager_test.rb @@ -2,15 +2,185 @@ class Account::CurrentBalanceManagerTest < ActiveSupport::TestCase setup do - @connected_account = accounts(:connected) # Connected account - can update current balance - @manual_account = accounts(:depository) # Manual account - cannot update current balance + @family = families(:empty) + @linked_account = accounts(:connected) end - test "when no existing anchor, creates new anchor" do - manager = Account::CurrentBalanceManager.new(@connected_account) + # ------------------------------------------------------------------------------------------------- + # Manual account current balance management + # + # Manual accounts do not manage `current_anchor` valuations and have "auto-update strategies" to set the current balance. + # ------------------------------------------------------------------------------------------------- - assert_difference -> { @connected_account.entries.count } => 1, - -> { @connected_account.valuations.count } => 1 do + test "when one or more reconciliations exist, append new reconciliation to represent the current balance" do + account = @family.accounts.create!( + name: "Test", + balance: 1000, + cash_balance: 1000, + currency: "USD", + accountable: Depository.new + ) + + # A reconciliation tells us that the user is tracking this account's value with balance-only updates + account.entries.create!( + date: 30.days.ago.to_date, + name: "First manual recon valuation", + amount: 1200, + currency: "USD", + entryable: Valuation.new(kind: "reconciliation") + ) + + manager = Account::CurrentBalanceManager.new(account) + + assert_equal 1, account.valuations.count + + # Here, we assume user is once again "overriding" the balance to 1400 + manager.set_current_balance(1400) + + today_valuation = account.entries.valuations.find_by(date: Date.current) + + assert_equal 2, account.valuations.count + assert_equal 1400, today_valuation.amount + + assert_equal 1400, account.balance + end + + test "all manual non cash accounts append reconciliations for current balance updates" do + [ Property, Vehicle, OtherAsset, Loan, OtherLiability ].each do |account_type| + account = @family.accounts.create!( + name: "Test", + balance: 1000, + cash_balance: 1000, + currency: "USD", + accountable: account_type.new + ) + + manager = Account::CurrentBalanceManager.new(account) + + assert_equal 0, account.valuations.count + + manager.set_current_balance(1400) + + assert_equal 1, account.valuations.count + + today_valuation = account.entries.valuations.find_by(date: Date.current) + + assert_equal 1400, today_valuation.amount + assert_equal 1400, account.balance + end + end + + # Scope: Depository, CreditCard only (i.e. all-cash accounts) + # + # If a user has an opening balance (valuation) for their manual *Depository* or *CreditCard* account and has 1+ transactions, the intent of + # "updating current balance" typically means that their start balance is incorrect. We follow that user intent + # by default and find the delta required, and update the opening balance so that the timeline reflects this current balance + # + # The purpose of this is so we're not cluttering up their timeline with "balance reconciliations" that reset the balance + # on the current date. Our goal is to keep the timeline with as few "Valuations" as possible. + # + # If we ever build a UI that gives user options, this test expectation may require some updates, but for now this + # is the least surprising outcome. + test "when no reconciliations exist on cash accounts, adjust opening balance with delta until it gets us to the desired balance" do + account = @family.accounts.create!( + name: "Test", + balance: 900, # the balance after opening valuation + transaction have "synced" (1000 - 100 = 900) + cash_balance: 900, + currency: "USD", + accountable: Depository.new + ) + + account.entries.create!( + date: 1.year.ago.to_date, + name: "Test opening valuation", + amount: 1000, + currency: "USD", + entryable: Valuation.new(kind: "opening_anchor") + ) + + account.entries.create!( + date: 10.days.ago.to_date, + name: "Test expense transaction", + amount: 100, + currency: "USD", + entryable: Transaction.new + ) + + # What we're asserting here: + # 1. User creates the account with an opening balance of 1000 + # 2. User creates a transaction of 100, which then reduces the balance to 900 (the current balance value on account above) + # 3. User requests "current balance update" back to 1000, which was their intention + # 4. We adjust the opening balance by the delta (100) to 1100, which is the new opening balance, so that the transaction + # of 100 reduces it down to 1000, which is the current balance they intended. + assert_equal 1, account.valuations.count + assert_equal 1, account.transactions.count + + # No new valuation is appended; we're just adjusting the opening valuation anchor + assert_no_difference "account.entries.count" do + manager = Account::CurrentBalanceManager.new(account) + manager.set_current_balance(1000) + end + + opening_valuation = account.valuations.find_by(kind: "opening_anchor") + + assert_equal 1100, opening_valuation.entry.amount + assert_equal 1000, account.balance + end + + # (SEE ABOVE TEST FOR MORE DETAILED EXPLANATION) + # Same assertions as the test above, but Credit Card accounts are liabilities, which means expenses increase balance; not decrease + test "when no reconciliations exist on credit card accounts, adjust opening balance with delta until it gets us to the desired balance" do + account = @family.accounts.create!( + name: "Test", + balance: 1100, # the balance after opening valuation + transaction have "synced" (1000 + 100 = 1100) (expenses increase balance) + cash_balance: 1100, + currency: "USD", + accountable: CreditCard.new + ) + + account.entries.create!( + date: 1.year.ago.to_date, + name: "Test opening valuation", + amount: 1000, + currency: "USD", + entryable: Valuation.new(kind: "opening_anchor") + ) + + account.entries.create!( + date: 10.days.ago.to_date, + name: "Test expense transaction", + amount: 100, + currency: "USD", + entryable: Transaction.new + ) + + assert_equal 1, account.valuations.count + assert_equal 1, account.transactions.count + + assert_no_difference "account.entries.count" do + manager = Account::CurrentBalanceManager.new(account) + manager.set_current_balance(1000) + end + + opening_valuation = account.valuations.find_by(kind: "opening_anchor") + + assert_equal 900, opening_valuation.entry.amount + assert_equal 1000, account.balance + end + + # ------------------------------------------------------------------------------------------------- + # Linked account current balance management + # + # Linked accounts manage "current balance" via the special `current_anchor` valuation. + # This is NOT a user-facing feature, and is primarily used in "processors" while syncing + # linked account data (e.g. via Plaid) + # ------------------------------------------------------------------------------------------------- + + test "when no existing anchor for linked account, creates new anchor" do + manager = Account::CurrentBalanceManager.new(@linked_account) + + assert_difference -> { @linked_account.entries.count } => 1, + -> { @linked_account.valuations.count } => 1 do result = manager.set_current_balance(1000) assert result.success? @@ -18,7 +188,7 @@ class Account::CurrentBalanceManagerTest < ActiveSupport::TestCase assert_nil result.error end - current_anchor = @connected_account.valuations.current_anchor.first + current_anchor = @linked_account.valuations.current_anchor.first assert_not_nil current_anchor assert_equal 1000, current_anchor.entry.amount assert_equal "current_anchor", current_anchor.kind @@ -27,23 +197,25 @@ class Account::CurrentBalanceManagerTest < ActiveSupport::TestCase assert_equal 1000, entry.amount assert_equal Date.current, entry.date assert_equal "Current balance", entry.name # Depository type returns "Current balance" + + assert_equal 1000, @linked_account.balance end - test "updates existing anchor" do + test "updates existing anchor for linked account" do # First create a current anchor - manager = Account::CurrentBalanceManager.new(@connected_account) + manager = Account::CurrentBalanceManager.new(@linked_account) result = manager.set_current_balance(1000) assert result.success? - current_anchor = @connected_account.valuations.current_anchor.first + current_anchor = @linked_account.valuations.current_anchor.first original_id = current_anchor.id original_entry_id = current_anchor.entry.id # Travel to tomorrow to ensure date change travel_to Date.current + 1.day do # Now update it - assert_no_difference -> { @connected_account.entries.count } do - assert_no_difference -> { @connected_account.valuations.count } do + assert_no_difference -> { @linked_account.entries.count } do + assert_no_difference -> { @linked_account.valuations.count } do result = manager.set_current_balance(2000) assert result.success? assert result.changes_made? @@ -56,24 +228,13 @@ class Account::CurrentBalanceManagerTest < ActiveSupport::TestCase assert_equal 2000, current_anchor.entry.amount assert_equal Date.current, current_anchor.entry.date # Should be updated to current date end - end - - test "when manual account, raises InvalidOperation error" do - manager = Account::CurrentBalanceManager.new(@manual_account) - - error = assert_raises(Account::CurrentBalanceManager::InvalidOperation) do - manager.set_current_balance(1000) - end - - assert_equal "Manual accounts cannot set current balance anchor. Set opening balance or use a reconciliation instead.", error.message - # Verify no current anchor was created - assert_nil @manual_account.valuations.current_anchor.first + assert_equal 2000, @linked_account.balance end test "when no changes made, returns success with no changes made" do # First create a current anchor - manager = Account::CurrentBalanceManager.new(@connected_account) + manager = Account::CurrentBalanceManager.new(@linked_account) result = manager.set_current_balance(1000) assert result.success? assert result.changes_made? @@ -84,16 +245,18 @@ class Account::CurrentBalanceManagerTest < ActiveSupport::TestCase assert result.success? assert_not result.changes_made? assert_nil result.error + + assert_equal 1000, @linked_account.balance end test "updates only amount when balance changes" do - manager = Account::CurrentBalanceManager.new(@connected_account) + manager = Account::CurrentBalanceManager.new(@linked_account) # Create initial anchor result = manager.set_current_balance(1000) assert result.success? - current_anchor = @connected_account.valuations.current_anchor.first + current_anchor = @linked_account.valuations.current_anchor.first original_date = current_anchor.entry.date # Update only the balance @@ -104,16 +267,18 @@ class Account::CurrentBalanceManagerTest < ActiveSupport::TestCase current_anchor.reload assert_equal 1500, current_anchor.entry.amount assert_equal original_date, current_anchor.entry.date # Date should remain the same if on same day + + assert_equal 1500, @linked_account.balance end test "updates date when called on different day" do - manager = Account::CurrentBalanceManager.new(@connected_account) + manager = Account::CurrentBalanceManager.new(@linked_account) # Create initial anchor result = manager.set_current_balance(1000) assert result.success? - current_anchor = @connected_account.valuations.current_anchor.first + current_anchor = @linked_account.valuations.current_anchor.first original_amount = current_anchor.entry.amount # Travel to tomorrow and update with same balance @@ -126,10 +291,12 @@ class Account::CurrentBalanceManagerTest < ActiveSupport::TestCase assert_equal original_amount, current_anchor.entry.amount assert_equal Date.current, current_anchor.entry.date # Should be updated to new current date end + + assert_equal 1000, @linked_account.balance end test "current_balance returns balance from current anchor" do - manager = Account::CurrentBalanceManager.new(@connected_account) + manager = Account::CurrentBalanceManager.new(@linked_account) # Create a current anchor manager.set_current_balance(1500) @@ -142,12 +309,16 @@ class Account::CurrentBalanceManagerTest < ActiveSupport::TestCase # Should return the updated balance assert_equal 2500, manager.current_balance + + assert_equal 2500, @linked_account.balance end test "current_balance falls back to account balance when no anchor exists" do - manager = Account::CurrentBalanceManager.new(@connected_account) + manager = Account::CurrentBalanceManager.new(@linked_account) # When no current anchor exists, should fall back to account.balance - assert_equal @connected_account.balance, manager.current_balance + assert_equal @linked_account.balance, manager.current_balance + + assert_equal @linked_account.balance, @linked_account.balance end end diff --git a/test/models/account/reconciliation_manager_test.rb b/test/models/account/reconciliation_manager_test.rb index 0e7a39a9e53..794c2cc5942 100644 --- a/test/models/account/reconciliation_manager_test.rb +++ b/test/models/account/reconciliation_manager_test.rb @@ -88,7 +88,7 @@ class Account::ReconciliationManagerTest < ActiveSupport::TestCase end end - test "dry run does not persist or sync account" do + test "dry run does not persist account" do @account.balances.create!(date: Date.current, balance: 1000, cash_balance: 500, currency: @account.currency) assert_no_difference "Valuation.count" do @@ -96,7 +96,6 @@ class Account::ReconciliationManagerTest < ActiveSupport::TestCase end assert_difference "Valuation.count", 1 do - @account.expects(:sync_later).once @manager.reconcile_balance(balance: 1200, date: Date.current) end end From 8c97c9d31a391b889d558a04cca468c05aa7bb9d Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Fri, 18 Jul 2025 05:52:18 -0400 Subject: [PATCH 05/29] Consolidate and simplify account pages (#2462) * Remove ScrollFocusable * Consolidate and simplify account pages * Lint fixes * Fix tab param initialization * Remove stale files * Remove stale route, make accountable routes clearer --- .../UI/account/chart.html.erb} | 42 +++++++---- app/components/UI/account/chart.rb | 72 +++++++++++++++++++ app/components/UI/account_page.html.erb | 29 ++++++++ app/components/UI/account_page.rb | 45 ++++++++++++ app/components/application_component.rb | 4 ++ app/controllers/accounts_controller.rb | 25 +++++-- .../concerns/accountable_resource.rb | 19 ++--- app/controllers/concerns/scroll_focusable.rb | 21 ------ app/controllers/transactions_controller.rb | 9 +-- .../controllers/focus_record_controller.js | 21 ------ app/views/accounts/_chart_loader.html.erb | 7 -- app/views/accounts/chart.html.erb | 22 ------ app/views/accounts/show.html.erb | 6 ++ app/views/accounts/show/_activity.html.erb | 2 +- app/views/accounts/show/_header.html.erb | 32 ++++----- app/views/accounts/show/_loading.html.erb | 3 - app/views/accounts/show/_tab.html.erb | 9 --- app/views/accounts/show/_tabs.html.erb | 17 ----- app/views/accounts/show/_template.html.erb | 27 ------- app/views/budget_categories/show.html.erb | 2 +- app/views/credit_cards/show.html.erb | 6 -- app/views/cryptos/show.html.erb | 1 - app/views/depositories/show.html.erb | 1 - app/views/investments/show.html.erb | 7 -- .../_holdings.html.erb} | 0 app/views/loans/show.html.erb | 6 -- app/views/loans/{ => tabs}/_overview.html.erb | 0 app/views/other_assets/show.html.erb | 1 - app/views/other_liabilities/show.html.erb | 1 - app/views/properties/show.html.erb | 7 -- .../properties/{ => tabs}/_overview.html.erb | 0 app/views/transactions/_transaction.html.erb | 7 +- app/views/transactions/index.html.erb | 2 +- app/views/vehicles/show.html.erb | 6 -- .../vehicles/{ => tabs}/_overview.html.erb | 0 config/routes.rb | 25 +++---- test/controllers/accounts_controller_test.rb | 17 +++-- .../credit_cards_controller_test.rb | 2 +- test/controllers/loans_controller_test.rb | 2 +- test/controllers/vehicles_controller_test.rb | 4 +- .../accountable_resource_interface_test.rb | 12 ---- 41 files changed, 252 insertions(+), 269 deletions(-) rename app/{views/accounts/show/_chart.html.erb => components/UI/account/chart.html.erb} (51%) create mode 100644 app/components/UI/account/chart.rb create mode 100644 app/components/UI/account_page.html.erb create mode 100644 app/components/UI/account_page.rb create mode 100644 app/components/application_component.rb delete mode 100644 app/controllers/concerns/scroll_focusable.rb delete mode 100644 app/javascript/controllers/focus_record_controller.js delete mode 100644 app/views/accounts/_chart_loader.html.erb delete mode 100644 app/views/accounts/chart.html.erb create mode 100644 app/views/accounts/show.html.erb delete mode 100644 app/views/accounts/show/_loading.html.erb delete mode 100644 app/views/accounts/show/_tab.html.erb delete mode 100644 app/views/accounts/show/_tabs.html.erb delete mode 100644 app/views/accounts/show/_template.html.erb delete mode 100644 app/views/credit_cards/show.html.erb delete mode 100644 app/views/cryptos/show.html.erb delete mode 100644 app/views/depositories/show.html.erb delete mode 100644 app/views/investments/show.html.erb rename app/views/investments/{_holdings_tab.html.erb => tabs/_holdings.html.erb} (100%) delete mode 100644 app/views/loans/show.html.erb rename app/views/loans/{ => tabs}/_overview.html.erb (100%) delete mode 100644 app/views/other_assets/show.html.erb delete mode 100644 app/views/other_liabilities/show.html.erb delete mode 100644 app/views/properties/show.html.erb rename app/views/properties/{ => tabs}/_overview.html.erb (100%) delete mode 100644 app/views/vehicles/show.html.erb rename app/views/vehicles/{ => tabs}/_overview.html.erb (100%) diff --git a/app/views/accounts/show/_chart.html.erb b/app/components/UI/account/chart.html.erb similarity index 51% rename from app/views/accounts/show/_chart.html.erb rename to app/components/UI/account/chart.html.erb index 00506be2718..ff54a5789b8 100644 --- a/app/views/accounts/show/_chart.html.erb +++ b/app/components/UI/account/chart.html.erb @@ -1,32 +1,28 @@ -<%# locals: (account:, tooltip: nil, chart_view: nil, **args) %> - -<% period = @period || Period.last_30_days %> -<% default_value_title = account.asset? ? t(".balance") : t(".owed") %> -
- <%= tag.p account.investment? ? "Total value" : default_value_title, class: "text-sm font-medium text-secondary" %> + <%= tag.p title, class: "text-sm font-medium text-secondary" %> <% if account.investment? %> - <%= render "investments/value_tooltip", balance: account.balance_money, holdings: account.balance_money - account.cash_balance_money, cash: account.cash_balance_money %> + <%= render "investments/value_tooltip", balance: account.balance_money, holdings: holdings_value_money, cash: account.cash_balance_money %> <% end %>
- <%= tag.p format_money(account.balance_money), class: "text-primary text-3xl font-medium truncate" %> - <% if account.currency != Current.family.currency %> - <%= tag.p format_money(account.balance_money.exchange_to(Current.family.currency, fallback_rate: 1)), class: "text-sm font-medium text-secondary" %> + <%= tag.p view_balance_money.format, class: "text-primary text-3xl font-medium truncate" %> + + <% if converted_balance_money %> + <%= tag.p converted_balance_money.format, class: "text-sm font-medium text-secondary" %> <% end %>
- <%= form_with url: request.path, method: :get, data: { controller: "auto-submit-form" } do |form| %> + <%= form_with url: account_path(account), method: :get, data: { controller: "auto-submit-form" } do |form| %>
- <% if chart_view.present? %> + <% if account.investment? %> <%= form.select :chart_view, [["Total value", "balance"], ["Holdings", "holdings_balance"], ["Cash", "cash_balance"]], - { selected: chart_view }, + { selected: view }, class: "bg-container border border-secondary rounded-lg text-sm pr-7 cursor-pointer text-primary focus:outline-hidden focus:ring-0", data: { "auto-submit-form-target": "auto" } %> <% end %> @@ -40,7 +36,23 @@ <% end %>
- <%= turbo_frame_tag dom_id(account, :chart_details), src: chart_account_path(account, period: period.key, chart_view: chart_view) do %> - <%= render "accounts/chart_loader" %> + <%= turbo_frame_tag dom_id(@account, :chart_details) do %> +
+ <%= render partial: "shared/trend_change", locals: { trend: trend, comparison_label: period.comparison_label } %> +
+ +
+ <% if series.any? %> +
+ <% else %> +
+

No data available

+
+ <% end %> +
<% end %>
diff --git a/app/components/UI/account/chart.rb b/app/components/UI/account/chart.rb new file mode 100644 index 00000000000..1e58529aa5b --- /dev/null +++ b/app/components/UI/account/chart.rb @@ -0,0 +1,72 @@ +class UI::Account::Chart < ApplicationComponent + attr_reader :account + + def initialize(account:, period: nil, view: nil) + @account = account + @period = period + @view = view + end + + def period + @period ||= Period.last_30_days + end + + def holdings_value_money + account.balance_money - account.cash_balance_money + end + + def view_balance_money + case view + when "balance" + account.balance_money + when "holdings_balance" + holdings_value_money + when "cash_balance" + account.cash_balance_money + end + end + + def title + case account.accountable_type + when "Investment", "Crypto" + case view + when "balance" + "Total account value" + when "holdings_balance" + "Holdings value" + when "cash_balance" + "Cash value" + end + when "Property", "Vehicle" + "Estimated #{account.accountable_type.humanize.downcase} value" + when "CreditCard", "OtherLiability" + "Debt balance" + when "Loan" + "Remaining principal balance" + else + "Balance" + end + end + + def foreign_currency? + account.currency != account.family.currency + end + + def converted_balance_money + return nil unless foreign_currency? + + account.balance_money.exchange_to(account.family.currency, fallback_rate: 1) + end + + def view + @view ||= "balance" + end + + def series + account.balance_series(period: period, view: view) + end + + def trend + series.trend + end +end diff --git a/app/components/UI/account_page.html.erb b/app/components/UI/account_page.html.erb new file mode 100644 index 00000000000..7b02d30f324 --- /dev/null +++ b/app/components/UI/account_page.html.erb @@ -0,0 +1,29 @@ +<%= turbo_stream_from account %> + +<%= turbo_frame_tag dom_id(account, :container) do %> + <%= tag.div class: "space-y-4 pb-32" do %> + <%= render "accounts/show/header", account: account, title: title, subtitle: subtitle %> + + <%= render UI::Account::Chart.new(account: account, period: chart_period, view: chart_view) %> + +
+ <% if tabs.count > 1 %> + <%= render TabsComponent.new(active_tab: active_tab, url_param_key: "tab") do |tabs_container| %> + <% tabs_container.with_nav(classes: "max-w-fit") do |nav| %> + <% tabs.each do |tab| %> + <% nav.with_btn(id: tab, label: tab.to_s.humanize, classes: "px-6") %> + <% end %> + <% end %> + + <% tabs.each do |tab| %> + <% tabs_container.with_panel(tab_id: tab) do %> + <%= render tab_partial_name(tab), account: account %> + <% end %> + <% end %> + <% end %> + <% else %> + <%= render tab_partial_name(tabs.first), account: account %> + <% end %> +
+ <% end %> +<% end %> diff --git a/app/components/UI/account_page.rb b/app/components/UI/account_page.rb new file mode 100644 index 00000000000..159ed56fbb7 --- /dev/null +++ b/app/components/UI/account_page.rb @@ -0,0 +1,45 @@ +class UI::AccountPage < ApplicationComponent + attr_reader :account, :chart_view, :chart_period + + def initialize(account:, chart_view: nil, chart_period: nil, active_tab: nil) + @account = account + @chart_view = chart_view + @chart_period = chart_period + @active_tab = active_tab + end + + def title + account.name + end + + def subtitle + return nil unless account.property? + + account.property.address + end + + def active_tab + tabs.find { |tab| tab == @active_tab&.to_sym } || tabs.first + end + + def tabs + case account.accountable_type + when "Investment" + [ :activity, :holdings ] + when "Property", "Vehicle", "Loan" + [ :activity, :overview ] + else + [ :activity ] + end + end + + def tab_partial_name(tab) + case tab + when :activity + "accounts/show/activity" + when :holdings, :overview + # Accountable is responsible for implementing the partial in the correct folder + "#{account.accountable_type.downcase.pluralize}/tabs/#{tab}" + end + end +end diff --git a/app/components/application_component.rb b/app/components/application_component.rb new file mode 100644 index 00000000000..37cb953da10 --- /dev/null +++ b/app/components/application_component.rb @@ -0,0 +1,4 @@ +class ApplicationComponent < ViewComponent::Base + # These don't work as expected with helpers.turbo_frame_tag, etc., so we include them here + include Turbo::FramesHelper, Turbo::StreamsHelper +end diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index c1804637189..7bf4470c2ee 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -1,5 +1,5 @@ class AccountsController < ApplicationController - before_action :set_account, only: %i[sync chart sparkline toggle_active] + before_action :set_account, only: %i[sync sparkline toggle_active show destroy] include Periodable def index @@ -9,6 +9,15 @@ def index render layout: "settings" end + def show + @chart_view = params[:chart_view] || "balance" + @tab = params[:tab] + @q = params.fetch(:q, {}).permit(:search) + entries = @account.entries.search(@q).reverse_chronological + + @pagy, @entries = pagy(entries, limit: params[:per_page] || "10") + end + def sync unless @account.syncing? @account.sync_later @@ -17,11 +26,6 @@ def sync redirect_to account_path(@account) end - def chart - @chart_view = params[:chart_view] || "balance" - render layout: "application" - end - def sparkline etag_key = @account.family.build_cache_key("#{@account.id}_sparkline", invalidate_on_data_updates: true) @@ -42,6 +46,15 @@ def toggle_active redirect_to accounts_path end + def destroy + if @account.linked? + redirect_to account_path(@account), alert: "Cannot delete a linked account" + else + @account.destroy_later + redirect_to accounts_path, notice: "Account scheduled for deletion" + end + end + private def family Current.family diff --git a/app/controllers/concerns/accountable_resource.rb b/app/controllers/concerns/accountable_resource.rb index a508764d5d4..3b06ff1632d 100644 --- a/app/controllers/concerns/accountable_resource.rb +++ b/app/controllers/concerns/accountable_resource.rb @@ -2,9 +2,9 @@ module AccountableResource extend ActiveSupport::Concern included do - include ScrollFocusable, Periodable + include Periodable - before_action :set_account, only: [ :show, :edit, :update, :destroy ] + before_action :set_account, only: [ :show, :edit, :update ] before_action :set_link_options, only: :new end @@ -27,9 +27,7 @@ def show @q = params.fetch(:q, {}).permit(:search) entries = @account.entries.search(@q).reverse_chronological - set_focused_record(entries, params[:focused_record_id]) - - @pagy, @entries = pagy(entries, limit: params[:per_page] || "10", params: ->(params) { params.except(:focused_record_id) }) + @pagy, @entries = pagy(entries, limit: params[:per_page] || "10") end def edit @@ -63,16 +61,7 @@ def update end @account.lock_saved_attributes! - redirect_back_or_to @account, notice: t("accounts.update.success", type: accountable_type.name.underscore.humanize) - end - - def destroy - if @account.linked? - redirect_to account_path(@account), alert: "Cannot delete a linked account" - else - @account.destroy_later - redirect_to accounts_path, notice: t("accounts.destroy.success", type: accountable_type.name.underscore.humanize) - end + redirect_back_or_to account_path(@account), notice: t("accounts.update.success", type: accountable_type.name.underscore.humanize) end private diff --git a/app/controllers/concerns/scroll_focusable.rb b/app/controllers/concerns/scroll_focusable.rb deleted file mode 100644 index 7eb47a1b39f..00000000000 --- a/app/controllers/concerns/scroll_focusable.rb +++ /dev/null @@ -1,21 +0,0 @@ -module ScrollFocusable - extend ActiveSupport::Concern - - def set_focused_record(record_scope, record_id, default_per_page: 10) - return unless record_id.present? - - @focused_record = record_scope.find_by(id: record_id) - - record_index = record_scope.pluck(:id).index(record_id) - - return unless record_index - - page_of_focused_record = (record_index / (params[:per_page]&.to_i || default_per_page)) + 1 - - if params[:page]&.to_i != page_of_focused_record - ( - redirect_to(url_for(page: page_of_focused_record, focused_record_id: record_id)) - ) - end - end -end diff --git a/app/controllers/transactions_controller.rb b/app/controllers/transactions_controller.rb index 1c260d8feea..9243df84d03 100644 --- a/app/controllers/transactions_controller.rb +++ b/app/controllers/transactions_controller.rb @@ -1,5 +1,5 @@ class TransactionsController < ApplicationController - include ScrollFocusable, EntryableResource + include EntryableResource before_action :store_params!, only: :index @@ -21,12 +21,7 @@ def index :transfer_as_inflow, :transfer_as_outflow ) - @pagy, @transactions = pagy(base_scope, limit: per_page, params: ->(p) { p.except(:focused_record_id) }) - - # No performance penalty by default. Only runs queries if the record is set. - if params[:focused_record_id].present? - set_focused_record(base_scope, params[:focused_record_id], default_per_page: per_page) - end + @pagy, @transactions = pagy(base_scope, limit: per_page) end def clear_filter diff --git a/app/javascript/controllers/focus_record_controller.js b/app/javascript/controllers/focus_record_controller.js deleted file mode 100644 index 0cc3fc9a64a..00000000000 --- a/app/javascript/controllers/focus_record_controller.js +++ /dev/null @@ -1,21 +0,0 @@ -import { Controller } from "@hotwired/stimulus"; - -// Connects to data-controller="focus-record" -export default class extends Controller { - static values = { - id: String, - }; - - connect() { - const element = document.getElementById(this.idValue); - - if (element) { - element.scrollIntoView({ behavior: "smooth" }); - - // Remove the focused_record_id parameter from URL - const url = new URL(window.location); - url.searchParams.delete("focused_record_id"); - window.history.replaceState({}, "", url); - } - } -} diff --git a/app/views/accounts/_chart_loader.html.erb b/app/views/accounts/_chart_loader.html.erb deleted file mode 100644 index b080329f5a7..00000000000 --- a/app/views/accounts/_chart_loader.html.erb +++ /dev/null @@ -1,7 +0,0 @@ -
-
-
- -
-
-
diff --git a/app/views/accounts/chart.html.erb b/app/views/accounts/chart.html.erb deleted file mode 100644 index 11dcbaac01f..00000000000 --- a/app/views/accounts/chart.html.erb +++ /dev/null @@ -1,22 +0,0 @@ -<% series = @account.balance_series(period: @period, view: @chart_view) %> -<% trend = series.trend %> - -<%= turbo_frame_tag dom_id(@account, :chart_details) do %> -
- <%= render partial: "shared/trend_change", locals: { trend: trend, comparison_label: @period.comparison_label } %> -
- -
- <% if series.any? %> -
- <% else %> -
-

<%= t(".data_not_available") %>

-
- <% end %> -
-<% end %> diff --git a/app/views/accounts/show.html.erb b/app/views/accounts/show.html.erb new file mode 100644 index 00000000000..a24e8b0b89f --- /dev/null +++ b/app/views/accounts/show.html.erb @@ -0,0 +1,6 @@ +<%= render UI::AccountPage.new( + account: @account, + chart_view: @chart_view, + chart_period: @period, + active_tab: @tab + ) %> diff --git a/app/views/accounts/show/_activity.html.erb b/app/views/accounts/show/_activity.html.erb index ab65dd4c464..3c454f2d0b2 100644 --- a/app/views/accounts/show/_activity.html.erb +++ b/app/views/accounts/show/_activity.html.erb @@ -1,7 +1,7 @@ <%# locals: (account:) %> <%= turbo_frame_tag dom_id(account, "entries") do %> -
+
<%= tag.h2 t(".title"), class: "font-medium text-lg" %> <% unless @account.plaid_account_id.present? %> diff --git a/app/views/accounts/show/_header.html.erb b/app/views/accounts/show/_header.html.erb index 51d3b253c41..06bcc3e2852 100644 --- a/app/views/accounts/show/_header.html.erb +++ b/app/views/accounts/show/_header.html.erb @@ -1,36 +1,30 @@ -<%# locals: (account:, title: nil, subtitle: nil) %> +<%# locals: (account:, title:, subtitle: nil) %>
- <% content = yield %> +
+ <%= render "accounts/logo", account: account %> - <% if content.present? %> - <%= content %> - <% else %> -
- <%= render "accounts/logo", account: account %> - -
-
-
-

"><%= title || account.name %>

- <% if account.draft? %> - <%= render LinkComponent.new( +
+
+
+

"><%= title %>

+ <% if account.draft? %> + <%= render LinkComponent.new( text: "Complete setup", href: edit_account_path(account), variant: :outline, size: :sm, frame: :modal ) %> - <% end %> -
- <% if subtitle.present? %> -

<%= subtitle %>

<% end %>
+ <% if subtitle.present? %> +

<%= subtitle %>

+ <% end %>
- <% end %> +
<% if Rails.env.development? || self_hosted? %> diff --git a/app/views/accounts/show/_loading.html.erb b/app/views/accounts/show/_loading.html.erb deleted file mode 100644 index 26167412338..00000000000 --- a/app/views/accounts/show/_loading.html.erb +++ /dev/null @@ -1,3 +0,0 @@ -
-

Loading account...

-
diff --git a/app/views/accounts/show/_tab.html.erb b/app/views/accounts/show/_tab.html.erb deleted file mode 100644 index 000f9743943..00000000000 --- a/app/views/accounts/show/_tab.html.erb +++ /dev/null @@ -1,9 +0,0 @@ -<%# locals: (account:, key:, is_selected:) %> - -<%= link_to key.titleize, - account_path(account, tab: key), - data: { turbo: false }, - class: [ - "px-2 py-1.5 rounded-md border border-transparent", - "bg-container shadow-xs border-alpha-black-50": is_selected - ] %> diff --git a/app/views/accounts/show/_tabs.html.erb b/app/views/accounts/show/_tabs.html.erb deleted file mode 100644 index 169fe0d45a4..00000000000 --- a/app/views/accounts/show/_tabs.html.erb +++ /dev/null @@ -1,17 +0,0 @@ -<%# locals: (account:, tabs:) %> - -<% active_tab = tabs.find { |tab| tab[:key] == params[:tab] } || tabs.first %> - -<%= render TabsComponent.new(active_tab: active_tab[:key], url_param_key: "tab") do |tabs_container| %> - <% tabs_container.with_nav(classes: "max-w-fit") do |nav| %> - <% tabs.each do |tab| %> - <% nav.with_btn(id: tab[:key], label: tab[:key].humanize, classes: "px-6") %> - <% end %> - <% end %> - - <% tabs.each do |tab| %> - <% tabs_container.with_panel(tab_id: tab[:key]) do %> - <%= tab[:contents] %> - <% end %> - <% end %> -<% end %> diff --git a/app/views/accounts/show/_template.html.erb b/app/views/accounts/show/_template.html.erb deleted file mode 100644 index efc7ea5a17d..00000000000 --- a/app/views/accounts/show/_template.html.erb +++ /dev/null @@ -1,27 +0,0 @@ -<%# locals: (account:, header: nil, chart: nil, chart_view: nil, tabs: nil) %> - -<%= turbo_stream_from account %> - -<%= turbo_frame_tag dom_id(account, :container) do %> - <%= tag.div class: "space-y-4 pb-32" do %> - <% if header.present? %> - <%= header %> - <% else %> - <%= render "accounts/show/header", account: account %> - <% end %> - - <% if chart.present? %> - <%= chart %> - <% else %> - <%= render "accounts/show/chart", account: account, chart_view: chart_view %> - <% end %> - -
- <% if tabs.present? %> - <%= tabs %> - <% else %> - <%= render "accounts/show/activity", account: account %> - <% end %> -
- <% end %> -<% end %> diff --git a/app/views/budget_categories/show.html.erb b/app/views/budget_categories/show.html.erb index 53b0624037d..d6e3ea1b9b7 100644 --- a/app/views/budget_categories/show.html.erb +++ b/app/views/budget_categories/show.html.erb @@ -107,7 +107,7 @@ <%= transaction.entry.date.strftime("%b %d") %>

<%= link_to transaction.entry.name, - transactions_path(focused_record_id: transaction.id), + transactions_path, class: "text-primary hover:underline", data: { turbo_frame: :_top } %>
diff --git a/app/views/credit_cards/show.html.erb b/app/views/credit_cards/show.html.erb deleted file mode 100644 index 4ba5252b19b..00000000000 --- a/app/views/credit_cards/show.html.erb +++ /dev/null @@ -1,6 +0,0 @@ -<%= render "accounts/show/template", - account: @account, - tabs: render("accounts/show/tabs", account: @account, tabs: [ - { key: "activity", contents: render("accounts/show/activity", account: @account) }, - { key: "overview", contents: render("credit_cards/overview", account: @account) }, - ]) %> diff --git a/app/views/cryptos/show.html.erb b/app/views/cryptos/show.html.erb deleted file mode 100644 index 0c3e6e97444..00000000000 --- a/app/views/cryptos/show.html.erb +++ /dev/null @@ -1 +0,0 @@ -<%= render "accounts/show/template", account: @account %> diff --git a/app/views/depositories/show.html.erb b/app/views/depositories/show.html.erb deleted file mode 100644 index 0c3e6e97444..00000000000 --- a/app/views/depositories/show.html.erb +++ /dev/null @@ -1 +0,0 @@ -<%= render "accounts/show/template", account: @account %> diff --git a/app/views/investments/show.html.erb b/app/views/investments/show.html.erb deleted file mode 100644 index 84be862918e..00000000000 --- a/app/views/investments/show.html.erb +++ /dev/null @@ -1,7 +0,0 @@ -<%= render "accounts/show/template", - account: @account, - chart_view: @chart_view, - tabs: render("accounts/show/tabs", account: @account, tabs: [ - { key: "activity", contents: render("accounts/show/activity", account: @account) }, - { key: "holdings", contents: render("investments/holdings_tab", account: @account) }, - ]) %> diff --git a/app/views/investments/_holdings_tab.html.erb b/app/views/investments/tabs/_holdings.html.erb similarity index 100% rename from app/views/investments/_holdings_tab.html.erb rename to app/views/investments/tabs/_holdings.html.erb diff --git a/app/views/loans/show.html.erb b/app/views/loans/show.html.erb deleted file mode 100644 index 55bffa52b09..00000000000 --- a/app/views/loans/show.html.erb +++ /dev/null @@ -1,6 +0,0 @@ -<%= render "accounts/show/template", - account: @account, - tabs: render("accounts/show/tabs", account: @account, tabs: [ - { key: "activity", contents: render("accounts/show/activity", account: @account) }, - { key: "overview", contents: render("loans/overview", account: @account) }, - ]) %> diff --git a/app/views/loans/_overview.html.erb b/app/views/loans/tabs/_overview.html.erb similarity index 100% rename from app/views/loans/_overview.html.erb rename to app/views/loans/tabs/_overview.html.erb diff --git a/app/views/other_assets/show.html.erb b/app/views/other_assets/show.html.erb deleted file mode 100644 index 0c3e6e97444..00000000000 --- a/app/views/other_assets/show.html.erb +++ /dev/null @@ -1 +0,0 @@ -<%= render "accounts/show/template", account: @account %> diff --git a/app/views/other_liabilities/show.html.erb b/app/views/other_liabilities/show.html.erb deleted file mode 100644 index 0c3e6e97444..00000000000 --- a/app/views/other_liabilities/show.html.erb +++ /dev/null @@ -1 +0,0 @@ -<%= render "accounts/show/template", account: @account %> diff --git a/app/views/properties/show.html.erb b/app/views/properties/show.html.erb deleted file mode 100644 index e6f8c34fd9e..00000000000 --- a/app/views/properties/show.html.erb +++ /dev/null @@ -1,7 +0,0 @@ -<%= render "accounts/show/template", - account: @account, - header: render("accounts/show/header", account: @account, subtitle: @account.property.address), - tabs: render("accounts/show/tabs", account: @account, tabs: [ - { key: "activity", contents: render("accounts/show/activity", account: @account) }, - { key: "overview", contents: render("properties/overview", account: @account) }, - ]) %> diff --git a/app/views/properties/_overview.html.erb b/app/views/properties/tabs/_overview.html.erb similarity index 100% rename from app/views/properties/_overview.html.erb rename to app/views/properties/tabs/_overview.html.erb diff --git a/app/views/transactions/_transaction.html.erb b/app/views/transactions/_transaction.html.erb index d6807adbc66..b603e305ce1 100644 --- a/app/views/transactions/_transaction.html.erb +++ b/app/views/transactions/_transaction.html.erb @@ -4,10 +4,7 @@ <%= turbo_frame_tag dom_id(entry) do %> <%= turbo_frame_tag dom_id(transaction) do %> -
- <%= entry.excluded ? "opacity-50 text-gray-400" : "" %>"> +
">
"> <%= check_box_tag dom_id(entry, "selection"), @@ -81,7 +78,7 @@ <% else %> <%= link_to entry.account.name, - account_path(entry.account, tab: "transactions", focused_record_id: entry.id), + account_path(entry.account, tab: "transactions"), data: { turbo_frame: "_top" }, class: "hover:underline" %> <% end %> diff --git a/app/views/transactions/index.html.erb b/app/views/transactions/index.html.erb index 149849493eb..4e9bcf82495 100644 --- a/app/views/transactions/index.html.erb +++ b/app/views/transactions/index.html.erb @@ -1,4 +1,4 @@ -
+

Transactions

diff --git a/app/views/vehicles/show.html.erb b/app/views/vehicles/show.html.erb deleted file mode 100644 index 555cd9269a2..00000000000 --- a/app/views/vehicles/show.html.erb +++ /dev/null @@ -1,6 +0,0 @@ -<%= render "accounts/show/template", - account: @account, - tabs: render("accounts/show/tabs", account: @account, tabs: [ - { key: "activity", contents: render("accounts/show/activity", account: @account) }, - { key: "overview", contents: render("vehicles/overview", account: @account) }, - ]) %> diff --git a/app/views/vehicles/_overview.html.erb b/app/views/vehicles/tabs/_overview.html.erb similarity index 100% rename from app/views/vehicles/_overview.html.erb rename to app/views/vehicles/tabs/_overview.html.erb diff --git a/config/routes.rb b/config/routes.rb index 3dd1d7f1899..e2817432454 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -150,10 +150,9 @@ end end - resources :accounts, only: %i[index new], shallow: true do + resources :accounts, only: %i[index new show destroy], shallow: true do member do post :sync - get :chart get :sparkline patch :toggle_active end @@ -161,17 +160,13 @@ # Convenience routes for polymorphic paths # Example: account_path(Account.new(accountable: Depository.new)) => /depositories/123 - direct :account do |model, options| - route_for model.accountable_name, model, options - end - direct :edit_account do |model, options| route_for "edit_#{model.accountable_name}", model, options end - resources :depositories, except: :index - resources :investments, except: :index - resources :properties, except: :index do + resources :depositories, only: %i[new create edit update] + resources :investments, only: %i[new create edit update] + resources :properties, only: %i[new create edit update] do member do get :balances patch :update_balances @@ -180,12 +175,12 @@ patch :update_address end end - resources :vehicles, except: :index - resources :credit_cards, except: :index - resources :loans, except: :index - resources :cryptos, except: :index - resources :other_assets, except: :index - resources :other_liabilities, except: :index + resources :vehicles, only: %i[new create edit update] + resources :credit_cards, only: %i[new create edit update] + resources :loans, only: %i[new create edit update] + resources :cryptos, only: %i[new create edit update] + resources :other_assets, only: %i[new create edit update] + resources :other_liabilities, only: %i[new create edit update] resources :securities, only: :index diff --git a/test/controllers/accounts_controller_test.rb b/test/controllers/accounts_controller_test.rb index ba0b937e592..ec26ef49f00 100644 --- a/test/controllers/accounts_controller_test.rb +++ b/test/controllers/accounts_controller_test.rb @@ -11,18 +11,25 @@ class AccountsControllerTest < ActionDispatch::IntegrationTest assert_response :success end + test "should get show" do + get account_url(@account) + assert_response :success + end + test "should sync account" do post sync_account_url(@account) assert_redirected_to account_url(@account) end - test "should get chart" do - get chart_account_url(@account) - assert_response :success - end - test "should get sparkline" do get sparkline_account_url(@account) assert_response :success end + + test "destroys account" do + delete account_url(@account) + assert_redirected_to accounts_path + assert_enqueued_with job: DestroyJob + assert_equal "Account scheduled for deletion", flash[:notice] + end end diff --git a/test/controllers/credit_cards_controller_test.rb b/test/controllers/credit_cards_controller_test.rb index 5fb0ec524b8..d19db6512cb 100644 --- a/test/controllers/credit_cards_controller_test.rb +++ b/test/controllers/credit_cards_controller_test.rb @@ -48,7 +48,7 @@ class CreditCardsControllerTest < ActionDispatch::IntegrationTest test "updates with credit card details" do assert_no_difference [ "Account.count", "CreditCard.count" ] do - patch account_path(@account), params: { + patch credit_card_path(@account), params: { account: { name: "Updated Credit Card", balance: 2000, diff --git a/test/controllers/loans_controller_test.rb b/test/controllers/loans_controller_test.rb index e12a27057c1..47809400c03 100644 --- a/test/controllers/loans_controller_test.rb +++ b/test/controllers/loans_controller_test.rb @@ -46,7 +46,7 @@ class LoansControllerTest < ActionDispatch::IntegrationTest test "updates with loan details" do assert_no_difference [ "Account.count", "Loan.count" ] do - patch account_path(@account), params: { + patch loan_path(@account), params: { account: { name: "Updated Loan", balance: 45000, diff --git a/test/controllers/vehicles_controller_test.rb b/test/controllers/vehicles_controller_test.rb index 37cea18d867..55aa89cff76 100644 --- a/test/controllers/vehicles_controller_test.rb +++ b/test/controllers/vehicles_controller_test.rb @@ -45,7 +45,7 @@ class VehiclesControllerTest < ActionDispatch::IntegrationTest test "updates with vehicle details" do assert_no_difference [ "Account.count", "Vehicle.count" ] do - patch account_path(@account), params: { + patch vehicle_path(@account), params: { account: { name: "Updated Vehicle", balance: 28000, @@ -64,7 +64,7 @@ class VehiclesControllerTest < ActionDispatch::IntegrationTest } end - assert_redirected_to @account + assert_redirected_to account_path(@account) assert_equal "Vehicle account updated", flash[:notice] assert_enqueued_with(job: SyncJob) end diff --git a/test/interfaces/accountable_resource_interface_test.rb b/test/interfaces/accountable_resource_interface_test.rb index ad5f50796d8..5ad9598e120 100644 --- a/test/interfaces/accountable_resource_interface_test.rb +++ b/test/interfaces/accountable_resource_interface_test.rb @@ -14,16 +14,4 @@ module AccountableResourceInterfaceTest get edit_account_url(@account) assert_response :success end - - test "renders accountable page" do - get account_url(@account) - assert_response :success - end - - test "destroys account" do - delete account_url(@account) - assert_redirected_to accounts_path - assert_enqueued_with job: DestroyJob - assert_equal "#{@account.accountable_name.underscore.humanize} account scheduled for deletion", flash[:notice] - end end From d5b147f2cd6a64740301440d27da6a68256f10fc Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Fri, 18 Jul 2025 08:19:44 -0400 Subject: [PATCH 06/29] Add indexes to core models (#2464) * [claudesquad] update from 'add-indexes-to-core-models' on 18 Jul 25 08:03 EDT * [claudesquad] update from 'add-indexes-to-core-models' on 18 Jul 25 08:09 EDT --- ...250718120146_add_indexes_to_core_models.rb | 20 +++++++++++++++++++ db/schema.rb | 11 +++++++++- 2 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20250718120146_add_indexes_to_core_models.rb diff --git a/db/migrate/20250718120146_add_indexes_to_core_models.rb b/db/migrate/20250718120146_add_indexes_to_core_models.rb new file mode 100644 index 00000000000..bac38c8c158 --- /dev/null +++ b/db/migrate/20250718120146_add_indexes_to_core_models.rb @@ -0,0 +1,20 @@ +class AddIndexesToCoreModels < ActiveRecord::Migration[7.2] + def change + # Accounts table indexes + add_index :accounts, [ :family_id, :status ] + add_index :accounts, :status + add_index :accounts, :currency + + # Balances table indexes + add_index :balances, [ :account_id, :date ], order: { date: :desc } + + # Entries table indexes + add_index :entries, [ :account_id, :date ] + add_index :entries, :date + add_index :entries, :entryable_type + add_index :entries, "lower(name)", name: "index_entries_on_lower_name" + + # Transfers table indexes + add_index :transfers, :status + end +end diff --git a/db/schema.rb b/db/schema.rb index 56d7ba097b1..3b839c95f0e 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.2].define(version: 2025_07_10_225721) do +ActiveRecord::Schema[7.2].define(version: 2025_07_18_120146) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -37,11 +37,14 @@ t.string "status", default: "active" t.index ["accountable_id", "accountable_type"], name: "index_accounts_on_accountable_id_and_accountable_type" t.index ["accountable_type"], name: "index_accounts_on_accountable_type" + t.index ["currency"], name: "index_accounts_on_currency" t.index ["family_id", "accountable_type"], name: "index_accounts_on_family_id_and_accountable_type" t.index ["family_id", "id"], name: "index_accounts_on_family_id_and_id" + t.index ["family_id", "status"], name: "index_accounts_on_family_id_and_status" t.index ["family_id"], name: "index_accounts_on_family_id" t.index ["import_id"], name: "index_accounts_on_import_id" t.index ["plaid_account_id"], name: "index_accounts_on_plaid_account_id" + t.index ["status"], name: "index_accounts_on_status" end create_table "active_storage_attachments", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| @@ -113,6 +116,7 @@ t.datetime "updated_at", null: false t.decimal "cash_balance", precision: 19, scale: 4, default: "0.0" t.index ["account_id", "date", "currency"], name: "index_account_balances_on_account_id_date_currency_unique", unique: true + t.index ["account_id", "date"], name: "index_balances_on_account_id_and_date", order: { date: :desc } t.index ["account_id"], name: "index_balances_on_account_id" end @@ -215,7 +219,11 @@ t.boolean "excluded", default: false t.string "plaid_id" t.jsonb "locked_attributes", default: {} + t.index "lower((name)::text)", name: "index_entries_on_lower_name" + t.index ["account_id", "date"], name: "index_entries_on_account_id_and_date" t.index ["account_id"], name: "index_entries_on_account_id" + t.index ["date"], name: "index_entries_on_date" + t.index ["entryable_type"], name: "index_entries_on_entryable_type" t.index ["import_id"], name: "index_entries_on_import_id" end @@ -741,6 +749,7 @@ t.index ["inflow_transaction_id", "outflow_transaction_id"], name: "idx_on_inflow_transaction_id_outflow_transaction_id_8cd07a28bd", unique: true t.index ["inflow_transaction_id"], name: "index_transfers_on_inflow_transaction_id" t.index ["outflow_transaction_id"], name: "index_transfers_on_outflow_transaction_id" + t.index ["status"], name: "index_transfers_on_status" end create_table "users", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| From ab6fdbbb6806e870362f44728f88f0c82831ff10 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Fri, 18 Jul 2025 08:30:00 -0400 Subject: [PATCH 07/29] Component namespacing (#2463) * [claudesquad] update from 'component-namespacing' on 18 Jul 25 07:23 EDT * [claudesquad] update from 'component-namespacing' on 18 Jul 25 07:30 EDT * Update stimulus controller references to use namespace * Fix remaining tests --- .../alert.html.erb} | 0 .../{alert_component.rb => DS/alert.rb} | 2 +- .../button.html.erb} | 0 .../{button_component.rb => DS/button.rb} | 2 +- .../buttonish.rb} | 4 +-- .../dialog.html.erb} | 2 +- .../{dialog_component.rb => DS/dialog.rb} | 20 ++++++------- app/components/{ => DS}/dialog_controller.js | 0 .../disclosure.html.erb} | 0 .../disclosure.rb} | 2 +- .../filled_icon.html.erb} | 0 .../filled_icon.rb} | 2 +- .../link.html.erb} | 0 .../{link_component.rb => DS/link.rb} | 2 +- .../menu.html.erb} | 8 +++--- .../{menu_component.rb => DS/menu.rb} | 8 +++--- app/components/{ => DS}/menu_controller.js | 0 .../menu_item.html.erb} | 0 .../menu_item.rb} | 2 +- .../{tab_component.rb => DS/tab.rb} | 2 +- app/components/DS/tabs.html.erb | 18 ++++++++++++ .../{tabs_component.rb => DS/tabs.rb} | 6 ++-- .../{tabs/nav_component.rb => DS/tabs/nav.rb} | 4 +-- .../panel_component.rb => DS/tabs/panel.rb} | 2 +- app/components/{ => DS}/tabs_controller.js | 0 .../toggle.html.erb} | 0 .../{toggle_component.rb => DS/toggle.rb} | 2 +- app/components/UI/account_page.html.erb | 2 +- app/components/design_system_component.rb | 2 ++ app/components/tabs_component.html.erb | 18 ------------ app/helpers/application_helper.rb | 2 +- app/helpers/styled_form_builder.rb | 4 +-- app/views/accounts/_account.html.erb | 4 +-- .../accounts/_account_sidebar_tabs.html.erb | 8 +++--- app/views/accounts/_account_type.html.erb | 2 +- .../accounts/_accountable_group.html.erb | 4 +-- app/views/accounts/_empty.html.erb | 2 +- app/views/accounts/_form.html.erb | 2 +- app/views/accounts/_logo.html.erb | 2 +- app/views/accounts/index.html.erb | 2 +- app/views/accounts/new.html.erb | 2 +- app/views/accounts/new/_container.html.erb | 4 +-- app/views/accounts/show/_activity.html.erb | 2 +- app/views/accounts/show/_header.html.erb | 2 +- app/views/accounts/show/_menu.html.erb | 2 +- .../_budget_category.html.erb | 2 +- .../_confirm_button.html.erb | 2 +- .../budget_categories/_no_categories.html.erb | 4 +-- app/views/budget_categories/show.html.erb | 4 +-- app/views/budgets/_budget_donut.html.erb | 6 ++-- app/views/budgets/_budget_header.html.erb | 8 +++--- .../budgets/_over_allocation_warning.html.erb | 2 +- app/views/budgets/_picker.html.erb | 2 +- app/views/budgets/edit.html.erb | 2 +- app/views/budgets/show.html.erb | 4 +-- app/views/categories/_category.html.erb | 2 +- app/views/categories/_menu.html.erb | 2 +- app/views/categories/edit.html.erb | 2 +- app/views/categories/index.html.erb | 8 +++--- app/views/categories/new.html.erb | 2 +- app/views/category/deletions/new.html.erb | 6 ++-- app/views/category/dropdowns/_row.html.erb | 2 +- app/views/category/dropdowns/show.html.erb | 2 +- app/views/chats/_chat.html.erb | 2 +- app/views/chats/_chat_nav.html.erb | 4 +-- app/views/chats/_error.html.erb | 2 +- app/views/chats/index.html.erb | 2 +- app/views/credit_cards/_overview.html.erb | 2 +- app/views/credit_cards/edit.html.erb | 2 +- app/views/credit_cards/new.html.erb | 2 +- app/views/cryptos/edit.html.erb | 2 +- app/views/cryptos/new.html.erb | 2 +- app/views/depositories/edit.html.erb | 2 +- app/views/depositories/new.html.erb | 2 +- .../doorkeeper/authorizations/error.html.erb | 2 +- .../doorkeeper/authorizations/new.html.erb | 4 +-- .../_family_merchant.html.erb | 2 +- app/views/family_merchants/edit.html.erb | 2 +- app/views/family_merchants/index.html.erb | 4 +-- app/views/family_merchants/new.html.erb | 2 +- app/views/holdings/_cash.html.erb | 2 +- app/views/holdings/show.html.erb | 2 +- app/views/import/cleans/show.html.erb | 2 +- app/views/import/configurations/show.html.erb | 4 +-- app/views/import/confirms/_mappings.html.erb | 6 ++-- app/views/import/uploads/show.html.erb | 2 +- app/views/imports/_empty.html.erb | 2 +- app/views/imports/_failure.html.erb | 2 +- app/views/imports/_import.html.erb | 2 +- app/views/imports/_importing.html.erb | 4 +-- app/views/imports/_ready.html.erb | 2 +- app/views/imports/_revert_failure.html.erb | 2 +- app/views/imports/_success.html.erb | 2 +- app/views/imports/index.html.erb | 2 +- app/views/imports/new.html.erb | 2 +- app/views/investments/edit.html.erb | 2 +- app/views/investments/new.html.erb | 2 +- app/views/invitations/new.html.erb | 2 +- app/views/layouts/application.html.erb | 4 +-- app/views/layouts/imports.html.erb | 4 +-- .../layouts/shared/_confirm_dialog.html.erb | 4 +-- app/views/layouts/wizard.html.erb | 4 +-- app/views/loans/edit.html.erb | 2 +- app/views/loans/new.html.erb | 2 +- app/views/loans/tabs/_overview.html.erb | 2 +- app/views/mfa/backup_codes.html.erb | 2 +- app/views/onboardings/_logout.html.erb | 2 +- app/views/onboardings/goals.html.erb | 2 +- app/views/onboardings/trial.html.erb | 28 +++++++++---------- app/views/other_assets/edit.html.erb | 2 +- app/views/other_assets/new.html.erb | 2 +- app/views/other_liabilities/edit.html.erb | 2 +- app/views/other_liabilities/new.html.erb | 2 +- app/views/pages/dashboard.html.erb | 4 +-- .../pages/dashboard/_balance_sheet.html.erb | 2 +- .../pages/dashboard/_cashflow_sankey.html.erb | 4 +-- .../_no_accounts_graph_placeholder.html.erb | 4 +-- .../pages/redis_configuration_error.html.erb | 4 +-- app/views/plaid_items/_plaid_item.html.erb | 4 +-- app/views/properties/_form_alert.html.erb | 4 +-- app/views/properties/address.html.erb | 4 +-- app/views/properties/balances.html.erb | 4 +-- app/views/properties/edit.html.erb | 4 +-- app/views/properties/new.html.erb | 4 +-- app/views/properties/tabs/_overview.html.erb | 2 +- .../rule/conditions/_condition_group.html.erb | 2 +- app/views/rules/_category_rule_cta.html.erb | 4 +-- app/views/rules/_form.html.erb | 6 ++-- app/views/rules/_rule.html.erb | 2 +- app/views/rules/confirm.html.erb | 4 +-- app/views/rules/edit.html.erb | 2 +- app/views/rules/index.html.erb | 8 +++--- app/views/rules/new.html.erb | 2 +- app/views/settings/_settings_nav.html.erb | 4 +-- app/views/settings/api_keys/created.html.erb | 6 ++-- .../api_keys/created.turbo_stream.erb | 6 ++-- app/views/settings/api_keys/new.html.erb | 4 +-- app/views/settings/api_keys/show.html.erb | 18 ++++++------ app/views/settings/billings/show.html.erb | 6 ++-- app/views/settings/profiles/show.html.erb | 10 +++---- app/views/settings/securities/show.html.erb | 4 +-- app/views/subscriptions/upgrade.html.erb | 6 ++-- app/views/tag/deletions/new.html.erb | 6 ++-- app/views/tags/_tag.html.erb | 2 +- app/views/tags/edit.html.erb | 2 +- app/views/tags/index.html.erb | 6 ++-- app/views/tags/new.html.erb | 2 +- app/views/trades/_header.html.erb | 2 +- app/views/trades/_trade.html.erb | 2 +- app/views/trades/new.html.erb | 2 +- app/views/trades/show.html.erb | 2 +- app/views/transactions/_form.html.erb | 2 +- app/views/transactions/_transaction.html.erb | 2 +- .../transactions/bulk_updates/new.html.erb | 10 +++---- app/views/transactions/index.html.erb | 8 +++--- app/views/transactions/new.html.erb | 2 +- .../transactions/searches/_form.html.erb | 5 ++-- .../transactions/searches/_menu.html.erb | 8 +++--- .../filters/_merchant_filter.html.erb | 2 +- .../searches/filters/_tag_filter.html.erb | 2 +- app/views/transactions/show.html.erb | 6 ++-- app/views/transfer_matches/new.html.erb | 2 +- app/views/transfers/new.html.erb | 2 +- app/views/transfers/show.html.erb | 2 +- app/views/users/_user_menu.html.erb | 2 +- app/views/valuations/_valuation.html.erb | 2 +- app/views/valuations/confirm_create.html.erb | 2 +- app/views/valuations/confirm_update.html.erb | 2 +- app/views/valuations/new.html.erb | 4 +-- app/views/valuations/show.html.erb | 6 ++-- app/views/vehicles/edit.html.erb | 2 +- app/views/vehicles/new.html.erb | 2 +- app/views/vehicles/tabs/_overview.html.erb | 2 +- .../previews/alert_component_preview.rb | 2 +- .../previews/button_component_preview.rb | 6 ++-- .../previews/dialog_component_preview.rb | 4 +-- .../previews/disclosure_component_preview.rb | 2 +- .../previews/filled_icon_component_preview.rb | 4 +-- .../previews/link_component_preview.rb | 10 +++---- .../previews/menu_component_preview.rb | 6 ++-- .../previews/toggle_component_preview.rb | 2 +- test/system/accounts_test.rb | 2 +- 182 files changed, 322 insertions(+), 321 deletions(-) rename app/components/{alert_component.html.erb => DS/alert.html.erb} (100%) rename app/components/{alert_component.rb => DS/alert.rb} (97%) rename app/components/{button_component.html.erb => DS/button.html.erb} (100%) rename app/components/{button_component.rb => DS/button.rb} (95%) rename app/components/{buttonish_component.rb => DS/buttonish.rb} (96%) rename app/components/{dialog_component.html.erb => DS/dialog.html.erb} (92%) rename app/components/{dialog_component.rb => DS/dialog.rb} (77%) rename app/components/{ => DS}/dialog_controller.js (100%) rename app/components/{disclosure_component.html.erb => DS/disclosure.html.erb} (100%) rename app/components/{disclosure_component.rb => DS/disclosure.rb} (82%) rename app/components/{filled_icon_component.html.erb => DS/filled_icon.html.erb} (100%) rename app/components/{filled_icon_component.rb => DS/filled_icon.rb} (97%) rename app/components/{link_component.html.erb => DS/link.html.erb} (100%) rename app/components/{link_component.rb => DS/link.rb} (94%) rename app/components/{menu_component.html.erb => DS/menu.html.erb} (59%) rename app/components/{menu_component.rb => DS/menu.rb} (81%) rename app/components/{ => DS}/menu_controller.js (100%) rename app/components/{menu_item_component.html.erb => DS/menu_item.html.erb} (100%) rename app/components/{menu_item_component.rb => DS/menu_item.rb} (97%) rename app/components/{tab_component.rb => DS/tab.rb} (75%) create mode 100644 app/components/DS/tabs.html.erb rename app/components/{tabs_component.rb => DS/tabs.rb} (94%) rename app/components/{tabs/nav_component.rb => DS/tabs/nav.rb} (87%) rename app/components/{tabs/panel_component.rb => DS/tabs/panel.rb} (69%) rename app/components/{ => DS}/tabs_controller.js (100%) rename app/components/{toggle_component.html.erb => DS/toggle.html.erb} (100%) rename app/components/{toggle_component.rb => DS/toggle.rb} (95%) create mode 100644 app/components/design_system_component.rb delete mode 100644 app/components/tabs_component.html.erb diff --git a/app/components/alert_component.html.erb b/app/components/DS/alert.html.erb similarity index 100% rename from app/components/alert_component.html.erb rename to app/components/DS/alert.html.erb diff --git a/app/components/alert_component.rb b/app/components/DS/alert.rb similarity index 97% rename from app/components/alert_component.rb rename to app/components/DS/alert.rb index 47c348f86cc..22241133fef 100644 --- a/app/components/alert_component.rb +++ b/app/components/DS/alert.rb @@ -1,4 +1,4 @@ -class AlertComponent < ViewComponent::Base +class DS::Alert < DesignSystemComponent def initialize(message:, variant: :info) @message = message @variant = variant diff --git a/app/components/button_component.html.erb b/app/components/DS/button.html.erb similarity index 100% rename from app/components/button_component.html.erb rename to app/components/DS/button.html.erb diff --git a/app/components/button_component.rb b/app/components/DS/button.rb similarity index 95% rename from app/components/button_component.rb rename to app/components/DS/button.rb index 36600a3ce83..a253c04c71c 100644 --- a/app/components/button_component.rb +++ b/app/components/DS/button.rb @@ -2,7 +2,7 @@ # An extension to `button_to` helper. All options are passed through to the `button_to` helper with some additional # options available. -class ButtonComponent < ButtonishComponent +class DS::Button < DS::Buttonish attr_reader :confirm def initialize(confirm: nil, **opts) diff --git a/app/components/buttonish_component.rb b/app/components/DS/buttonish.rb similarity index 96% rename from app/components/buttonish_component.rb rename to app/components/DS/buttonish.rb index 4bbb2882356..b83557b17d0 100644 --- a/app/components/buttonish_component.rb +++ b/app/components/DS/buttonish.rb @@ -1,4 +1,4 @@ -class ButtonishComponent < ViewComponent::Base +class DS::Buttonish < DesignSystemComponent VARIANTS = { primary: { container_classes: "text-inverse bg-inverse hover:bg-inverse-hover disabled:bg-gray-500 theme-dark:disabled:bg-gray-400", @@ -71,7 +71,7 @@ def initialize(variant: :primary, size: :md, href: nil, text: nil, icon: nil, ic end def call - raise NotImplementedError, "ButtonishComponent is an abstract class and cannot be instantiated directly." + raise NotImplementedError, "Buttonish is an abstract class and cannot be instantiated directly." end def container_classes(override_classes = nil) diff --git a/app/components/dialog_component.html.erb b/app/components/DS/dialog.html.erb similarity index 92% rename from app/components/dialog_component.html.erb rename to app/components/DS/dialog.html.erb index ac4cd07fcb0..ca8f86b7d5d 100644 --- a/app/components/dialog_component.html.erb +++ b/app/components/DS/dialog.html.erb @@ -1,7 +1,7 @@ <%= wrapper_element do %> <%= tag.dialog class: "w-full h-full bg-transparent theme-dark:backdrop:bg-alpha-black-900 backdrop:bg-overlay #{drawer? ? "lg:p-3" : "lg:p-1"}", **merged_opts do %> <%= tag.div class: dialog_outer_classes do %> - <%= tag.div class: dialog_inner_classes, data: { dialog_target: "content" } do %> + <%= tag.div class: dialog_inner_classes, data: { DS__dialog_target: "content" } do %>
<% if header? %> <%= header %> diff --git a/app/components/dialog_component.rb b/app/components/DS/dialog.rb similarity index 77% rename from app/components/dialog_component.rb rename to app/components/DS/dialog.rb index 942468723da..3385003c112 100644 --- a/app/components/dialog_component.rb +++ b/app/components/DS/dialog.rb @@ -1,9 +1,9 @@ -class DialogComponent < ViewComponent::Base +class DS::Dialog < DesignSystemComponent renders_one :header, ->(title: nil, subtitle: nil, hide_close_icon: false, **opts, &block) do content_tag(:header, class: "px-4 flex flex-col gap-2", **opts) do title_div = content_tag(:div, class: "flex items-center justify-between gap-2") do title = content_tag(:h2, title, class: class_names("font-medium text-primary", drawer? ? "text-lg" : "")) if title - close_icon = render ButtonComponent.new(variant: "icon", class: "ml-auto", icon: "x", tabindex: "-1", data: { action: "dialog#close" }) unless hide_close_icon + close_icon = render DS::Button.new(variant: "icon", class: "ml-auto", icon: "x", tabindex: "-1", data: { action: "DS--dialog#close" }) unless hide_close_icon safe_join([ title, close_icon ].compact) end @@ -19,16 +19,16 @@ class DialogComponent < ViewComponent::Base renders_many :actions, ->(cancel_action: false, **button_opts) do merged_opts = if cancel_action - button_opts.merge(type: "button", data: { action: "modal#close" }) + button_opts.merge(type: "button", data: { action: "DS--dialog#close" }) else button_opts end - render ButtonComponent.new(**merged_opts) + render DS::Button.new(**merged_opts) end renders_many :sections, ->(title:, **disclosure_opts, &block) do - render DisclosureComponent.new(title: title, align: :right, **disclosure_opts) do + render DS::Disclosure.new(title: title, align: :right, **disclosure_opts) do block.call end end @@ -99,11 +99,11 @@ def merged_opts merged_opts = opts.dup data = merged_opts.delete(:data) || {} - data[:controller] = [ "dialog", "hotkey", data[:controller] ].compact.join(" ") - data[:dialog_auto_open_value] = auto_open - data[:dialog_reload_on_close_value] = reload_on_close - data[:action] = [ "mousedown->dialog#clickOutside", data[:action] ].compact.join(" ") - data[:hotkey] = "esc:dialog#close" + data[:controller] = [ "DS--dialog", "hotkey", data[:controller] ].compact.join(" ") + data[:DS__dialog_auto_open_value] = auto_open + data[:DS__dialog_reload_on_close_value] = reload_on_close + data[:action] = [ "mousedown->DS--dialog#clickOutside", data[:action] ].compact.join(" ") + data[:hotkey] = "esc:DS--dialog#close" merged_opts[:data] = data merged_opts diff --git a/app/components/dialog_controller.js b/app/components/DS/dialog_controller.js similarity index 100% rename from app/components/dialog_controller.js rename to app/components/DS/dialog_controller.js diff --git a/app/components/disclosure_component.html.erb b/app/components/DS/disclosure.html.erb similarity index 100% rename from app/components/disclosure_component.html.erb rename to app/components/DS/disclosure.html.erb diff --git a/app/components/disclosure_component.rb b/app/components/DS/disclosure.rb similarity index 82% rename from app/components/disclosure_component.rb rename to app/components/DS/disclosure.rb index 5aba01b20eb..d301d671ed9 100644 --- a/app/components/disclosure_component.rb +++ b/app/components/DS/disclosure.rb @@ -1,4 +1,4 @@ -class DisclosureComponent < ViewComponent::Base +class DS::Disclosure < DesignSystemComponent renders_one :summary_content attr_reader :title, :align, :open, :opts diff --git a/app/components/filled_icon_component.html.erb b/app/components/DS/filled_icon.html.erb similarity index 100% rename from app/components/filled_icon_component.html.erb rename to app/components/DS/filled_icon.html.erb diff --git a/app/components/filled_icon_component.rb b/app/components/DS/filled_icon.rb similarity index 97% rename from app/components/filled_icon_component.rb rename to app/components/DS/filled_icon.rb index 1f9032436ae..73113a0bce1 100644 --- a/app/components/filled_icon_component.rb +++ b/app/components/DS/filled_icon.rb @@ -1,4 +1,4 @@ -class FilledIconComponent < ViewComponent::Base +class DS::FilledIcon < DesignSystemComponent attr_reader :icon, :text, :hex_color, :size, :rounded, :variant VARIANTS = %i[default text surface container inverse].freeze diff --git a/app/components/link_component.html.erb b/app/components/DS/link.html.erb similarity index 100% rename from app/components/link_component.html.erb rename to app/components/DS/link.html.erb diff --git a/app/components/link_component.rb b/app/components/DS/link.rb similarity index 94% rename from app/components/link_component.rb rename to app/components/DS/link.rb index 4bbe10e5e44..209f86b09f6 100644 --- a/app/components/link_component.rb +++ b/app/components/DS/link.rb @@ -1,6 +1,6 @@ # An extension to `link_to` helper. All options are passed through to the `link_to` helper with some additional # options available. -class LinkComponent < ButtonishComponent +class DS::Link < DS::Buttonish attr_reader :frame VARIANTS = VARIANTS.reverse_merge( diff --git a/app/components/menu_component.html.erb b/app/components/DS/menu.html.erb similarity index 59% rename from app/components/menu_component.html.erb rename to app/components/DS/menu.html.erb index 00f19b2ad99..d4f0ea8d833 100644 --- a/app/components/menu_component.html.erb +++ b/app/components/DS/menu.html.erb @@ -1,17 +1,17 @@ -<%= tag.div data: { controller: "menu", menu_placement_value: placement, menu_offset_value: offset, testid: testid } do %> +<%= tag.div data: { controller: "DS--menu", DS__menu_placement_value: placement, DS__menu_offset_value: offset, testid: testid } do %> <% if variant == :icon %> - <%= render ButtonComponent.new(variant: "icon", icon: icon_vertical ? "more-vertical" : "more-horizontal", data: { menu_target: "button" }) %> + <%= render DS::Button.new(variant: "icon", icon: icon_vertical ? "more-vertical" : "more-horizontal", data: { DS__menu_target: "button" }) %> <% elsif variant == :button %> <%= button %> <% elsif variant == :avatar %> - <% end %> -