From 70e7a5f2d63d3197dae4f53515e645e17cc17aa0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 23 Nov 2025 10:39:02 +0000 Subject: [PATCH 01/10] Initial plan From 52588784d0c1cad7fd0cd7baa6b6c512a8053bf4 Mon Sep 17 00:00:00 2001 From: Josh Waldrep Date: Sat, 10 Jan 2026 19:48:04 -0500 Subject: [PATCH 02/10] Add investment activity detection, labels, and exclusions - Introduced `InvestmentActivityDetector` to mark internal investment activity as excluded from cashflow and assign appropriate labels. - Added `exclude_from_cashflow` flag to `entries` and `investment_activity_label` to `transactions` with migrations. - Implemented rake tasks to backfill and clear investment activity labels. - Updated `PlaidAccount::Investments::TransactionsProcessor` to map Plaid transaction types to labels. - Included comprehensive test coverage for new functionality. --- app/controllers/reports_controller.rb | 10 +- app/controllers/transactions_controller.rb | 4 +- app/models/account/provider_import_adapter.rb | 11 +- app/models/income_statement/category_stats.rb | 3 +- app/models/income_statement/family_stats.rb | 3 +- app/models/income_statement/totals.rb | 7 +- app/models/investment_activity_detector.rb | 346 ++++++++++++++++++ app/models/lunchflow_account/processor.rb | 27 ++ .../investments/transactions_processor.rb | 25 +- app/models/plaid_account/processor.rb | 40 ++ app/models/simplefin_account/processor.rb | 27 ++ app/models/simplefin_entry/processor.rb | 10 +- app/models/transaction.rb | 29 +- app/models/transaction/search.rb | 8 +- app/models/transfer.rb | 4 + app/views/transactions/_transaction.html.erb | 13 + app/views/transactions/show.html.erb | 53 ++- config/locales/views/transactions/en.yml | 11 + ...0120000_add_investment_cashflow_support.rb | 13 + ...vestment_activity_label_to_transactions.rb | 8 + db/schema.rb | 62 +--- lib/tasks/investment_labels.rake | 185 ++++++++++ test/models/income_statement_test.rb | 66 ++++ .../investment_activity_detector_test.rb | 299 +++++++++++++++ test/models/transaction_test.rb | 33 ++ test/models/transfer_test.rb | 20 + 26 files changed, 1235 insertions(+), 82 deletions(-) create mode 100644 app/models/investment_activity_detector.rb create mode 100644 db/migrate/20260110120000_add_investment_cashflow_support.rb create mode 100644 db/migrate/20260110180000_add_investment_activity_label_to_transactions.rb create mode 100644 lib/tasks/investment_labels.rake create mode 100644 test/models/investment_activity_detector_test.rb diff --git a/app/controllers/reports_controller.rb b/app/controllers/reports_controller.rb index 91593f801..842ff50e8 100644 --- a/app/controllers/reports_controller.rb +++ b/app/controllers/reports_controller.rb @@ -338,7 +338,7 @@ class ReportsController < ApplicationController .joins(:entry) .joins(entry: :account) .where(accounts: { family_id: Current.family.id, status: [ "draft", "active" ] }) - .where(entries: { entryable_type: "Transaction", excluded: false, date: @period.date_range }) + .where(entries: { entryable_type: "Transaction", excluded: false, exclude_from_cashflow: false, date: @period.date_range }) .where.not(kind: [ "funds_movement", "one_time", "cc_payment" ]) .includes(entry: :account, category: []) @@ -350,7 +350,7 @@ class ReportsController < ApplicationController .joins(:entry) .joins(entry: :account) .where(accounts: { family_id: Current.family.id, status: [ "draft", "active" ] }) - .where(entries: { entryable_type: "Trade", excluded: false, date: @period.date_range }) + .where(entries: { entryable_type: "Trade", excluded: false, exclude_from_cashflow: false, date: @period.date_range }) .includes(entry: :account, category: []) # Get sort parameters @@ -519,7 +519,7 @@ class ReportsController < ApplicationController .joins(:entry) .joins(entry: :account) .where(accounts: { family_id: Current.family.id, status: [ "draft", "active" ] }) - .where(entries: { entryable_type: "Transaction", excluded: false, date: @period.date_range }) + .where(entries: { entryable_type: "Transaction", excluded: false, exclude_from_cashflow: false, date: @period.date_range }) .where.not(kind: [ "funds_movement", "one_time", "cc_payment" ]) .includes(entry: :account, category: []) @@ -556,7 +556,7 @@ class ReportsController < ApplicationController .joins(:entry) .joins(entry: :account) .where(accounts: { family_id: Current.family.id, status: [ "draft", "active" ] }) - .where(entries: { entryable_type: "Transaction", excluded: false, date: @period.date_range }) + .where(entries: { entryable_type: "Transaction", excluded: false, exclude_from_cashflow: false, date: @period.date_range }) .where.not(kind: [ "funds_movement", "one_time", "cc_payment" ]) .includes(entry: :account, category: []) @@ -567,7 +567,7 @@ class ReportsController < ApplicationController .joins(:entry) .joins(entry: :account) .where(accounts: { family_id: Current.family.id, status: [ "draft", "active" ] }) - .where(entries: { entryable_type: "Trade", excluded: false, date: @period.date_range }) + .where(entries: { entryable_type: "Trade", excluded: false, exclude_from_cashflow: false, date: @period.date_range }) .includes(entry: :account, category: []) # Group by category, type, and month diff --git a/app/controllers/transactions_controller.rb b/app/controllers/transactions_controller.rb index a0fc9aaee..03dcbc2db 100644 --- a/app/controllers/transactions_controller.rb +++ b/app/controllers/transactions_controller.rb @@ -217,8 +217,8 @@ class TransactionsController < ApplicationController def entry_params entry_params = params.require(:entry).permit( - :name, :date, :amount, :currency, :excluded, :notes, :nature, :entryable_type, - entryable_attributes: [ :id, :category_id, :merchant_id, :kind, { tag_ids: [] } ] + :name, :date, :amount, :currency, :excluded, :exclude_from_cashflow, :notes, :nature, :entryable_type, + entryable_attributes: [ :id, :category_id, :merchant_id, :kind, :investment_activity_label, { tag_ids: [] } ] ) nature = entry_params.delete(:nature) diff --git a/app/models/account/provider_import_adapter.rb b/app/models/account/provider_import_adapter.rb index e74d1f5e6..31d65a6ae 100644 --- a/app/models/account/provider_import_adapter.rb +++ b/app/models/account/provider_import_adapter.rb @@ -18,8 +18,9 @@ class Account::ProviderImportAdapter # @param notes [String, nil] Optional transaction notes/memo # @param pending_transaction_id [String, nil] Plaid's linking ID for pending→posted reconciliation # @param extra [Hash, nil] Optional provider-specific metadata to merge into transaction.extra + # @param investment_activity_label [String, nil] Optional activity type label (e.g., "Buy", "Dividend") # @return [Entry] The created or updated entry - def import_transaction(external_id:, amount:, currency:, date:, name:, source:, category_id: nil, merchant: nil, notes: nil, pending_transaction_id: nil, extra: nil) + def import_transaction(external_id:, amount:, currency:, date:, name:, source:, category_id: nil, merchant: nil, notes: nil, pending_transaction_id: nil, extra: nil, investment_activity_label: nil) raise ArgumentError, "external_id is required" if external_id.blank? raise ArgumentError, "source is required" if source.blank? @@ -114,6 +115,14 @@ class Account::ProviderImportAdapter entry.transaction.extra = existing.deep_merge(incoming) entry.transaction.save! end + + # Set investment activity label if provided and not already set + if investment_activity_label.present? && entry.entryable.is_a?(Transaction) + if entry.transaction.investment_activity_label.blank? + entry.transaction.update!(investment_activity_label: investment_activity_label) + end + end + entry.save! # AFTER save: For NEW posted transactions, check for fuzzy matches to SUGGEST (not auto-claim) diff --git a/app/models/income_statement/category_stats.rb b/app/models/income_statement/category_stats.rb index 3bc91b839..31679a662 100644 --- a/app/models/income_statement/category_stats.rb +++ b/app/models/income_statement/category_stats.rb @@ -47,8 +47,9 @@ class IncomeStatement::CategoryStats er.to_currency = :target_currency ) WHERE a.family_id = :family_id - AND t.kind NOT IN ('funds_movement', 'one_time', 'cc_payment') + AND t.kind NOT IN ('funds_movement', 'one_time', 'cc_payment', 'investment_contribution') AND ae.excluded = false + AND ae.exclude_from_cashflow = false AND (t.extra -> 'simplefin' ->> 'pending')::boolean IS DISTINCT FROM true AND (t.extra -> 'plaid' ->> 'pending')::boolean IS DISTINCT FROM true GROUP BY c.id, period, CASE WHEN ae.amount < 0 THEN 'income' ELSE 'expense' END diff --git a/app/models/income_statement/family_stats.rb b/app/models/income_statement/family_stats.rb index c2a3c8f8e..c96f13b35 100644 --- a/app/models/income_statement/family_stats.rb +++ b/app/models/income_statement/family_stats.rb @@ -44,8 +44,9 @@ class IncomeStatement::FamilyStats er.to_currency = :target_currency ) WHERE a.family_id = :family_id - AND t.kind NOT IN ('funds_movement', 'one_time', 'cc_payment') + AND t.kind NOT IN ('funds_movement', 'one_time', 'cc_payment', 'investment_contribution') AND ae.excluded = false + AND ae.exclude_from_cashflow = false AND (t.extra -> 'simplefin' ->> 'pending')::boolean IS DISTINCT FROM true AND (t.extra -> 'plaid' ->> 'pending')::boolean IS DISTINCT FROM true GROUP BY period, CASE WHEN ae.amount < 0 THEN 'income' ELSE 'expense' END diff --git a/app/models/income_statement/totals.rb b/app/models/income_statement/totals.rb index 355212486..2b5bd07b7 100644 --- a/app/models/income_statement/totals.rb +++ b/app/models/income_statement/totals.rb @@ -69,8 +69,9 @@ class IncomeStatement::Totals er.from_currency = ae.currency AND er.to_currency = :target_currency ) - WHERE at.kind NOT IN ('funds_movement', 'one_time', 'cc_payment') + WHERE at.kind NOT IN ('funds_movement', 'one_time', 'cc_payment', 'investment_contribution') AND ae.excluded = false + AND ae.exclude_from_cashflow = false AND a.family_id = :family_id AND a.status IN ('draft', 'active') GROUP BY c.id, c.parent_id, CASE WHEN ae.amount < 0 THEN 'income' ELSE 'expense' END; @@ -95,8 +96,9 @@ class IncomeStatement::Totals er.from_currency = ae.currency AND er.to_currency = :target_currency ) - WHERE at.kind NOT IN ('funds_movement', 'one_time', 'cc_payment') + WHERE at.kind NOT IN ('funds_movement', 'one_time', 'cc_payment', 'investment_contribution') AND ae.excluded = false + AND ae.exclude_from_cashflow = false AND a.family_id = :family_id AND a.status IN ('draft', 'active') GROUP BY c.id, c.parent_id, CASE WHEN ae.amount < 0 THEN 'income' ELSE 'expense' END @@ -126,6 +128,7 @@ class IncomeStatement::Totals WHERE a.family_id = :family_id AND a.status IN ('draft', 'active') AND ae.excluded = false + AND ae.exclude_from_cashflow = false AND ae.date BETWEEN :start_date AND :end_date GROUP BY c.id, c.parent_id, CASE WHEN ae.amount < 0 THEN 'income' ELSE 'expense' END, CASE WHEN t.category_id IS NULL THEN true ELSE false END SQL diff --git a/app/models/investment_activity_detector.rb b/app/models/investment_activity_detector.rb new file mode 100644 index 000000000..c030d9cdf --- /dev/null +++ b/app/models/investment_activity_detector.rb @@ -0,0 +1,346 @@ +# Detects internal investment activity (fund swaps, reinvestments) by comparing +# holdings snapshots between syncs and marks matching transactions as excluded +# from cashflow. This is provider-agnostic and works with any holdings data. +# +# Usage: +# detector = InvestmentActivityDetector.new(account) +# detector.detect_and_mark_internal_activity(current_holdings, recent_transactions) +# +class InvestmentActivityDetector + def initialize(account) + @account = account + end + + # Class method for inferring activity label from description and amount + # without needing a full detector instance + # @param name [String] Transaction name/description + # @param amount [Numeric] Transaction amount + # @param account [Account, nil] Optional account for context (e.g., retirement plan detection) + # @return [String, nil] Activity label or nil if unknown + def self.infer_label_from_description(name, amount, account = nil) + new(nil).send(:infer_from_description, name, amount, account) + end + + # Call this after syncing transactions for an investment/crypto account + # @param current_holdings [Array] Array of holding objects/hashes from provider + # @param recent_transactions [Array] Recently imported transactions + def detect_and_mark_internal_activity(current_holdings, recent_transactions) + return unless @account.investment? || @account.crypto? + return if current_holdings.blank? + + previous_snapshot = @account.holdings_snapshot_data || [] + + # Find holdings changes that indicate buys/sells + changes = detect_holdings_changes(previous_snapshot, current_holdings) + + # Match changes to transactions and mark them as excluded + changes.each do |change| + matched_entry = find_matching_entry(change, recent_transactions) + next unless matched_entry + + transaction = matched_entry.entryable + + # Only auto-set if not already manually set by user (respect user overrides) + unless matched_entry.locked?(:exclude_from_cashflow) + matched_entry.update!(exclude_from_cashflow: true) + matched_entry.lock_attr!(:exclude_from_cashflow) + + Rails.logger.info( + "InvestmentActivityDetector: Auto-excluded entry #{matched_entry.id} " \ + "(#{matched_entry.name}) as internal #{change[:type]} of #{change[:symbol] || change[:description]}" + ) + end + + # Set activity label if not already set + if transaction.is_a?(Transaction) && transaction.investment_activity_label.blank? + label = infer_activity_label(matched_entry, change[:type]) + transaction.update!(investment_activity_label: label) if label.present? + end + end + + # Store current snapshot for next comparison + save_holdings_snapshot(current_holdings) + end + + private + + # Infer activity label from change type and transaction description + def infer_activity_label(entry, change_type) + # If we know it's a buy or sell from holdings comparison + return "Buy" if change_type == :buy + return "Sell" if change_type == :sell + + # Otherwise try to infer from description + infer_from_description(entry) + end + + # Infer activity label from transaction description + # Can be called with an Entry or with name/amount directly + # @param entry_or_name [Entry, String] Entry object or transaction name + # @param amount [Numeric, nil] Transaction amount (required if entry_or_name is String) + # @param account [Account, nil] Optional account for context (e.g., retirement plan detection) + def infer_from_description(entry_or_name, amount = nil, account = nil) + if entry_or_name.respond_to?(:name) + description = (entry_or_name.name || "").upcase + amount = entry_or_name.amount || 0 + account ||= entry_or_name.try(:account) + else + description = (entry_or_name || "").upcase + amount ||= 0 + end + + # Check if this is a retirement plan account (401k, 403b, etc.) + account_name = (account&.name || "").upcase + retirement_indicators = %w[401K 403B RETIREMENT TOTALSOURCE NETBENEFITS] + retirement_phrases = [ "SAVINGS PLAN", "THRIFT PLAN", "PENSION" ] + is_retirement_plan = retirement_indicators.any? { |ind| account_name.include?(ind) } || + retirement_phrases.any? { |phrase| account_name.include?(phrase) } + + # Check for sweep/money market patterns (but NOT money market FUND purchases) + # INVESTOR CL indicates this is a money market fund, not a sweep + sweep_patterns = %w[SWEEP SETTLEMENT] + money_market_sweep = description.include?("MONEY MARKET") && !description.include?("INVESTOR") + common_money_market_tickers = %w[VMFXX SPAXX FDRXX SWVXX SPRXX] + + if sweep_patterns.any? { |p| description.include?(p) } || + money_market_sweep || + common_money_market_tickers.any? { |t| description == t } + return amount.positive? ? "Sweep Out" : "Sweep In" + end + + # Check for likely interest/dividend on money market funds + # Small amounts (under $5) on money market funds are typically interest income + money_market_fund_patterns = %w[MONEY\ MARKET VMFXX SPAXX FDRXX SWVXX SPRXX VUSXX] + is_money_market_fund = money_market_fund_patterns.any? { |p| description.include?(p) } + + if is_money_market_fund && amount.abs < 5 + # Small money market amounts are interest, not buys/sells + return "Interest" + end + + # Check for dividend patterns + if description == "CASH" || description.include?("DIVIDEND") || + description.include?("DISTRIBUTION") + return "Dividend" + end + + # Check for interest + return "Interest" if description.include?("INTEREST") + + # Check for fees + return "Fee" if description.include?("FEE") || description.include?("CHARGE") + + # Check for reinvestment + return "Reinvestment" if description.include?("REINVEST") + + # Check for exchange/conversion + return "Exchange" if description.include?("EXCHANGE") || description.include?("CONVERSION") + + # Check for contribution patterns + return "Contribution" if description.include?("CONTRIBUTION") || description.include?("DEPOSIT") + + # Check for withdrawal patterns + return "Withdrawal" if description.include?("WITHDRAWAL") || description.include?("DISBURSEMENT") + + # Check for fund names that indicate buy/sell activity + # Positive amount = money out from account perspective = buying securities + # Negative amount = money in = selling securities + fund_patterns = %w[ + INDEX FUND ADMIRAL ETF SHARES TRUST + VANGUARD FIDELITY SCHWAB ISHARES SPDR + 500\ INDEX TOTAL\ MARKET GROWTH BOND + ] + + # Common fund ticker patterns + fund_ticker_patterns = %w[ + VFIAX VTSAX VXUS VBTLX VTIAX VTTVX + VTI VOO VGT VIG VYM VGIT + FXAIX FZROX FSKAX FBALX + SWTSX SWPPX SCHD SCHX + SPY QQQ IVV AGG + IBIT GBTC ETHE + ] + + is_fund_transaction = fund_patterns.any? { |p| description.include?(p) } || + fund_ticker_patterns.any? { |t| description.include?(t) } + + if is_fund_transaction + if is_retirement_plan && amount.negative? + # Negative amount in retirement plan = payroll contribution buying shares + return "Contribution" + else + return amount.positive? ? "Buy" : "Sell" + end + end + + nil # Unknown - user can set manually + end + + def detect_holdings_changes(previous, current) + changes = [] + + current.each do |holding| + prev = find_previous_holding(previous, holding) + + if prev.nil? + # New holding appeared = BUY + changes << { + type: :buy, + symbol: holding_symbol(holding), + description: holding_description(holding), + shares: holding_shares(holding), + cost_basis: holding_cost_basis(holding), + created_at: holding_created_at(holding) + } + elsif holding_shares(holding) > prev_shares(prev) + # Shares increased = BUY + changes << { + type: :buy, + symbol: holding_symbol(holding), + description: holding_description(holding), + shares_delta: holding_shares(holding) - prev_shares(prev), + cost_basis_delta: holding_cost_basis(holding) - prev_cost_basis(prev) + } + elsif holding_shares(holding) < prev_shares(prev) + # Shares decreased = SELL + changes << { + type: :sell, + symbol: holding_symbol(holding), + description: holding_description(holding), + shares_delta: prev_shares(prev) - holding_shares(holding) + } + end + end + + # Check for holdings that completely disappeared = SELL ALL + previous.each do |prev| + unless current.any? { |h| same_holding?(h, prev) } + changes << { + type: :sell, + symbol: prev_symbol(prev), + description: prev_description(prev), + shares: prev_shares(prev) + } + end + end + + changes + end + + def find_matching_entry(change, transactions) + transactions.each do |txn| + entry = txn.respond_to?(:entry) ? txn.entry : txn + next unless entry + next if entry.exclude_from_cashflow? # Already excluded + + # Match by cost_basis amount (for buys with known cost) + if change[:cost_basis].present? && change[:cost_basis].to_d > 0 + amount_diff = (entry.amount.to_d.abs - change[:cost_basis].to_d.abs).abs + return entry if amount_diff < 0.01 + end + + # Match by cost_basis delta (for additional buys) + if change[:cost_basis_delta].present? && change[:cost_basis_delta].to_d > 0 + amount_diff = (entry.amount.to_d.abs - change[:cost_basis_delta].to_d.abs).abs + return entry if amount_diff < 0.01 + end + + # Match by description containing security name/symbol + entry_desc = entry.name&.downcase || "" + + if change[:symbol].present? + return entry if entry_desc.include?(change[:symbol].downcase) + end + + if change[:description].present? + # Match first few words of description for fuzzy matching + desc_words = change[:description].downcase.split.first(3).join(" ") + return entry if desc_words.present? && entry_desc.include?(desc_words) + end + end + + nil + end + + def find_previous_holding(previous, current) + symbol = holding_symbol(current) + return previous.find { |p| prev_symbol(p) == symbol } if symbol.present? + + # Fallback to description matching if no symbol + desc = holding_description(current) + previous.find { |p| prev_description(p) == desc } if desc.present? + end + + def same_holding?(current, previous) + current_symbol = holding_symbol(current) + prev_sym = prev_symbol(previous) + + if current_symbol.present? && prev_sym.present? + current_symbol == prev_sym + else + holding_description(current) == prev_description(previous) + end + end + + def save_holdings_snapshot(holdings) + snapshot_data = holdings.map do |h| + { + "symbol" => holding_symbol(h), + "description" => holding_description(h), + "shares" => holding_shares(h).to_s, + "cost_basis" => holding_cost_basis(h).to_s, + "market_value" => holding_market_value(h).to_s + } + end + + @account.update!( + holdings_snapshot_data: snapshot_data, + holdings_snapshot_at: Time.current + ) + end + + # Normalize access - holdings could be AR objects or hashes from different providers + def holding_symbol(h) + h.try(:symbol) || h.try(:ticker) || h["symbol"] || h[:symbol] || h["ticker"] || h[:ticker] + end + + def holding_description(h) + h.try(:description) || h.try(:name) || h["description"] || h[:description] || h["name"] || h[:name] + end + + def holding_shares(h) + val = h.try(:shares) || h.try(:qty) || h["shares"] || h[:shares] || h["qty"] || h[:qty] + val.to_d + end + + def holding_cost_basis(h) + val = h.try(:cost_basis) || h["cost_basis"] || h[:cost_basis] + val.to_d + end + + def holding_market_value(h) + val = h.try(:market_value) || h.try(:amount) || h["market_value"] || h[:market_value] || h["amount"] || h[:amount] + val.to_d + end + + def holding_created_at(h) + h.try(:created_at) || h["created"] || h[:created] || h["created_at"] || h[:created_at] + end + + # Previous snapshot accessor methods (snapshot is always a hash) + def prev_symbol(p) + p["symbol"] || p[:symbol] + end + + def prev_description(p) + p["description"] || p[:description] + end + + def prev_shares(p) + (p["shares"] || p[:shares]).to_d + end + + def prev_cost_basis(p) + (p["cost_basis"] || p[:cost_basis]).to_d + end +end diff --git a/app/models/lunchflow_account/processor.rb b/app/models/lunchflow_account/processor.rb index b9c6b2184..d82301dbd 100644 --- a/app/models/lunchflow_account/processor.rb +++ b/app/models/lunchflow_account/processor.rb @@ -74,10 +74,37 @@ class LunchflowAccount::Processor return unless [ "Investment", "Crypto" ].include?(lunchflow_account.current_account&.accountable_type) LunchflowAccount::Investments::HoldingsProcessor.new(lunchflow_account).process + + # Detect and mark internal investment activity (fund swaps, reinvestments) + detect_internal_investment_activity rescue => e report_exception(e, "holdings") end + def detect_internal_investment_activity + account = lunchflow_account.current_account + return unless account&.investment? || account&.crypto? + + # Get current holdings from raw payload + current_holdings = lunchflow_account.raw_holdings_payload || [] + return if current_holdings.blank? + + # Get recent transactions (last 30 days to catch any we might have missed) + recent_transactions = account.entries + .joins("INNER JOIN transactions ON transactions.id = entries.entryable_id AND entries.entryable_type = 'Transaction'") + .where(date: 30.days.ago.to_date..Date.current) + .where(exclude_from_cashflow: false) + .map(&:entryable) + .compact + + InvestmentActivityDetector.new(account).detect_and_mark_internal_activity( + current_holdings, + recent_transactions + ) + rescue => e + Rails.logger.warn("InvestmentActivityDetector failed for Lunchflow account #{lunchflow_account.id}: #{e.message}") + end + def report_exception(error, context) Sentry.capture_exception(error) do |scope| scope.set_tags( diff --git a/app/models/plaid_account/investments/transactions_processor.rb b/app/models/plaid_account/investments/transactions_processor.rb index 922a3f2a6..ded2a473f 100644 --- a/app/models/plaid_account/investments/transactions_processor.rb +++ b/app/models/plaid_account/investments/transactions_processor.rb @@ -1,6 +1,23 @@ class PlaidAccount::Investments::TransactionsProcessor SecurityNotFoundError = Class.new(StandardError) + # Map Plaid investment transaction types to activity labels + PLAID_TYPE_TO_LABEL = { + "buy" => "Buy", + "sell" => "Sell", + "cancel" => "Cancelled", + "cash" => "Cash", + "fee" => "Fee", + "transfer" => "Transfer", + "dividend" => "Dividend", + "interest" => "Interest", + "contribution" => "Contribution", + "withdrawal" => "Withdrawal", + "dividend reinvestment" => "Reinvestment", + "spin off" => "Other", + "split" => "Other" + }.freeze + def initialize(plaid_account, security_resolver:) @plaid_account = plaid_account @security_resolver = security_resolver @@ -68,10 +85,16 @@ class PlaidAccount::Investments::TransactionsProcessor currency: transaction["iso_currency_code"], date: transaction["date"], name: transaction["name"], - source: "plaid" + source: "plaid", + investment_activity_label: label_from_plaid_type(transaction) ) end + def label_from_plaid_type(transaction) + plaid_type = transaction["type"]&.downcase + PLAID_TYPE_TO_LABEL[plaid_type] || plaid_type&.titleize + end + def transactions plaid_account.raw_investments_payload["transactions"] || [] end diff --git a/app/models/plaid_account/processor.rb b/app/models/plaid_account/processor.rb index 4faead9d9..e282d5e9d 100644 --- a/app/models/plaid_account/processor.rb +++ b/app/models/plaid_account/processor.rb @@ -103,10 +103,50 @@ class PlaidAccount::Processor def process_investments PlaidAccount::Investments::TransactionsProcessor.new(plaid_account, security_resolver: security_resolver).process PlaidAccount::Investments::HoldingsProcessor.new(plaid_account, security_resolver: security_resolver).process + + # Detect and mark internal investment activity (fund swaps, reinvestments) + # Note: Plaid already creates Trade entries for buy/sell, but this catches cash transactions + detect_internal_investment_activity rescue => e report_exception(e) end + def detect_internal_investment_activity + account = AccountProvider.find_by(provider: plaid_account)&.account + return unless account&.investment? || account&.crypto? + + # Get current holdings from raw payload + raw_holdings = plaid_account.raw_investments_payload&.dig("holdings") || [] + return if raw_holdings.blank? + + # Transform to common format + current_holdings = raw_holdings.map do |h| + security = security_resolver.resolve(h["security_id"]) + { + "symbol" => security&.ticker, + "description" => security&.name, + "shares" => h["quantity"], + "cost_basis" => h["cost_basis"], + "market_value" => h["institution_value"] + } + end + + # Get recent transactions (last 30 days to catch any we might have missed) + recent_transactions = account.entries + .joins("INNER JOIN transactions ON transactions.id = entries.entryable_id AND entries.entryable_type = 'Transaction'") + .where(date: 30.days.ago.to_date..Date.current) + .where(exclude_from_cashflow: false) + .map(&:entryable) + .compact + + InvestmentActivityDetector.new(account).detect_and_mark_internal_activity( + current_holdings, + recent_transactions + ) + rescue => e + Rails.logger.warn("InvestmentActivityDetector failed for Plaid account #{plaid_account.id}: #{e.message}") + end + def process_liabilities case [ plaid_account.plaid_type, plaid_account.plaid_subtype ] when [ "credit", "credit card" ] diff --git a/app/models/simplefin_account/processor.rb b/app/models/simplefin_account/processor.rb index 569f515cd..68b679772 100644 --- a/app/models/simplefin_account/processor.rb +++ b/app/models/simplefin_account/processor.rb @@ -151,10 +151,37 @@ class SimplefinAccount::Processor return unless simplefin_account.current_account&.accountable_type == "Investment" SimplefinAccount::Investments::TransactionsProcessor.new(simplefin_account).process SimplefinAccount::Investments::HoldingsProcessor.new(simplefin_account).process + + # Detect and mark internal investment activity (fund swaps, reinvestments) + detect_internal_investment_activity rescue => e report_exception(e, "investments") end + def detect_internal_investment_activity + account = simplefin_account.current_account + return unless account&.investment? || account&.crypto? + + # Get current holdings from raw payload + current_holdings = simplefin_account.raw_holdings_payload || [] + return if current_holdings.blank? + + # Get recent transactions (last 30 days to catch any we might have missed) + recent_transactions = account.entries + .joins("INNER JOIN transactions ON transactions.id = entries.entryable_id AND entries.entryable_type = 'Transaction'") + .where(date: 30.days.ago.to_date..Date.current) + .where(exclude_from_cashflow: false) + .map(&:entryable) + .compact + + InvestmentActivityDetector.new(account).detect_and_mark_internal_activity( + current_holdings, + recent_transactions + ) + rescue => e + Rails.logger.warn("InvestmentActivityDetector failed for account #{simplefin_account.current_account&.id}: #{e.message}") + end + def process_liabilities case simplefin_account.current_account&.accountable_type when "CreditCard" diff --git a/app/models/simplefin_entry/processor.rb b/app/models/simplefin_entry/processor.rb index db4b5689b..8e6cd88f5 100644 --- a/app/models/simplefin_entry/processor.rb +++ b/app/models/simplefin_entry/processor.rb @@ -18,7 +18,8 @@ class SimplefinEntry::Processor source: "simplefin", merchant: merchant, notes: notes, - extra: extra_metadata + extra: extra_metadata, + investment_activity_label: inferred_activity_label ) end @@ -204,4 +205,11 @@ class SimplefinEntry::Processor end parts.presence&.join(" | ") end + + # Infer investment activity label from transaction description + # Only returns a label for investment/crypto accounts + def inferred_activity_label + return nil unless account&.investment? || account&.crypto? + InvestmentActivityDetector.infer_label_from_description(name, amount, account) + end end diff --git a/app/models/transaction.rb b/app/models/transaction.rb index 70ca49da0..42e3a2677 100644 --- a/app/models/transaction.rb +++ b/app/models/transaction.rb @@ -16,9 +16,23 @@ class Transaction < ApplicationRecord funds_movement: "funds_movement", # Movement of funds between accounts, excluded from budget analytics cc_payment: "cc_payment", # A CC payment, excluded from budget analytics (CC payments offset the sum of expense transactions) loan_payment: "loan_payment", # A payment to a Loan account, treated as an expense in budgets - one_time: "one_time" # A one-time expense/income, excluded from budget analytics + one_time: "one_time", # A one-time expense/income, excluded from budget analytics + investment_contribution: "investment_contribution" # Transfer to investment/crypto account, included in budget as investment expense } + # Labels for internal investment activity (auto-exclude from cashflow) + # Only internal shuffling should be excluded, not contributions/dividends/withdrawals + INTERNAL_ACTIVITY_LABELS = %w[Buy Sell Reinvestment Exchange].freeze + + # All valid investment activity labels (for UI dropdown) + ACTIVITY_LABELS = [ + "Buy", "Sell", "Sweep In", "Sweep Out", "Dividend", "Reinvestment", + "Interest", "Fee", "Transfer", "Contribution", "Withdrawal", "Exchange", "Other" + ].freeze + + after_save :sync_exclude_from_cashflow_with_activity_label, + if: :saved_change_to_investment_activity_label? + # Pending transaction scopes - filter based on provider pending flags in extra JSONB # Works with any provider that stores pending status in extra["provider_name"]["pending"] scope :pending, -> { @@ -145,4 +159,17 @@ class Transaction < ApplicationRecord FamilyMerchantAssociation.where(family: family, merchant: merchant).delete_all end + + # Sync exclude_from_cashflow based on activity label + # Internal activities (Buy, Sell, etc.) should be excluded from cashflow + def sync_exclude_from_cashflow_with_activity_label + return unless entry&.account&.investment? || entry&.account&.crypto? + return if entry.locked?(:exclude_from_cashflow) # Respect user's manual setting + + should_exclude = INTERNAL_ACTIVITY_LABELS.include?(investment_activity_label) + + if entry.exclude_from_cashflow != should_exclude + entry.update!(exclude_from_cashflow: should_exclude) + end + end end diff --git a/app/models/transaction/search.rb b/app/models/transaction/search.rb index 6c401e0f0..3ece18e45 100644 --- a/app/models/transaction/search.rb +++ b/app/models/transaction/search.rb @@ -49,8 +49,8 @@ class Transaction::Search Rails.cache.fetch("transaction_search_totals/#{cache_key_base}") do result = transactions_scope .select( - "COALESCE(SUM(CASE WHEN entries.amount >= 0 AND transactions.kind NOT IN ('funds_movement', 'cc_payment') THEN ABS(entries.amount * COALESCE(er.rate, 1)) ELSE 0 END), 0) as expense_total", - "COALESCE(SUM(CASE WHEN entries.amount < 0 AND transactions.kind NOT IN ('funds_movement', 'cc_payment') THEN ABS(entries.amount * COALESCE(er.rate, 1)) ELSE 0 END), 0) as income_total", + "COALESCE(SUM(CASE WHEN entries.amount >= 0 AND transactions.kind NOT IN ('funds_movement', 'cc_payment', 'investment_contribution') AND entries.exclude_from_cashflow = false THEN ABS(entries.amount * COALESCE(er.rate, 1)) ELSE 0 END), 0) as expense_total", + "COALESCE(SUM(CASE WHEN entries.amount < 0 AND transactions.kind NOT IN ('funds_movement', 'cc_payment', 'investment_contribution') AND entries.exclude_from_cashflow = false THEN ABS(entries.amount * COALESCE(er.rate, 1)) ELSE 0 END), 0) as income_total", "COUNT(entries.id) as transactions_count" ) .joins( @@ -100,14 +100,14 @@ class Transaction::Search if parent_category_ids.empty? query = query.left_joins(:category).where( "categories.name IN (?) OR ( - categories.id IS NULL AND (transactions.kind NOT IN ('funds_movement', 'cc_payment')) + categories.id IS NULL AND (transactions.kind NOT IN ('funds_movement', 'cc_payment', 'investment_contribution')) )", categories ) else query = query.left_joins(:category).where( "categories.name IN (?) OR categories.parent_id IN (?) OR ( - categories.id IS NULL AND (transactions.kind NOT IN ('funds_movement', 'cc_payment')) + categories.id IS NULL AND (transactions.kind NOT IN ('funds_movement', 'cc_payment', 'investment_contribution')) )", categories, parent_category_ids ) diff --git a/app/models/transfer.rb b/app/models/transfer.rb index 93de0a068..d2dcbf667 100644 --- a/app/models/transfer.rb +++ b/app/models/transfer.rb @@ -16,6 +16,10 @@ class Transfer < ApplicationRecord def kind_for_account(account) if account.loan? "loan_payment" + elsif account.credit_card? + "cc_payment" + elsif account.investment? || account.crypto? + "investment_contribution" elsif account.liability? "cc_payment" else diff --git a/app/views/transactions/_transaction.html.erb b/app/views/transactions/_transaction.html.erb index ec89bb308..a4593a56d 100644 --- a/app/views/transactions/_transaction.html.erb +++ b/app/views/transactions/_transaction.html.erb @@ -78,6 +78,19 @@ <% end %> + <% if entry.exclude_from_cashflow? %> + + <%= icon "eye-off", size: "sm", color: "current" %> + + <% end %> + + <%# Investment activity label badge %> + <% if transaction.investment_activity_label.present? %> + "> + <%= transaction.investment_activity_label %> + + <% end %> + <%# Pending indicator %> <% if transaction.pending? %> "> diff --git a/app/views/transactions/show.html.erb b/app/views/transactions/show.html.erb index 0b6397494..85e770509 100644 --- a/app/views/transactions/show.html.erb +++ b/app/views/transactions/show.html.erb @@ -194,8 +194,8 @@ data: { controller: "auto-submit-form" } do |f| %>
-

Exclude

-

Excluded transactions will be removed from budgeting calculations and reports.

+

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

+

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

<%= f.toggle :excluded, { data: { auto_submit_form_target: "auto" } } %> @@ -203,6 +203,55 @@ <% end %>
+
+ <%= styled_form_with model: @entry, + url: transaction_path(@entry), + class: "p-3", + data: { controller: "auto-submit-form" } do |f| %> +
+
+

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

+

+ <% if @entry.account.investment? || @entry.account.crypto? %> + <%= t(".exclude_from_cashflow_description_investment") %> + <% else %> + <%= t(".exclude_from_cashflow_description") %> + <% end %> +

+
+ + <%= f.toggle :exclude_from_cashflow, { data: { auto_submit_form_target: "auto" } } %> +
+ <% end %> +
+ + <% if @entry.account.investment? || @entry.account.crypto? %> +
+ <%= styled_form_with model: @entry, + url: transaction_path(@entry), + class: "p-3", + data: { controller: "auto-submit-form" } do |f| %> + <%= f.fields_for :entryable do |ef| %> +
+
+

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

+

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

+
+ + <%= ef.select :investment_activity_label, + options_for_select( + [["—", nil]] + Transaction::ACTIVITY_LABELS.map { |l| [l, l] }, + @entry.entryable.investment_activity_label + ), + { label: false }, + { class: "form-field__input border border-secondary rounded-lg px-3 py-1.5 max-w-40 text-sm", + data: { auto_submit_form_target: "auto" } } %> +
+ <% end %> + <% end %> +
+ <% end %> +
<%= styled_form_with model: @entry, url: transaction_path(@entry), diff --git a/config/locales/views/transactions/en.yml b/config/locales/views/transactions/en.yml index 717d146b1..0a393d835 100644 --- a/config/locales/views/transactions/en.yml +++ b/config/locales/views/transactions/en.yml @@ -31,6 +31,13 @@ en: balances, and cannot be undone. delete_title: Delete transaction details: Details + exclude: Exclude + exclude_description: Excluded transactions will be removed from budgeting calculations and reports. + exclude_from_cashflow: Exclude from Cashflow + exclude_from_cashflow_description: Hide from income/expense reports and Sankey chart. Useful for transactions you don't want in cashflow analysis. + exclude_from_cashflow_description_investment: Hide from income/expense reports and Sankey chart. Use for internal investment activity like fund swaps, reinvestments, or money market sweeps. + activity_type: Activity Type + activity_type_description: Type of investment activity (Buy, Sell, Dividend, etc.). Auto-detected or set manually. mark_recurring: Mark as Recurring mark_recurring_subtitle: Track this as a recurring transaction. Amount variance is automatically calculated from past 6 months of similar transactions. mark_recurring_title: Recurring Transaction @@ -48,9 +55,13 @@ en: potential_duplicate_description: This pending transaction may be the same as the posted transaction below. If so, merge them to avoid double-counting. merge_duplicate: Yes, merge them keep_both: No, keep both + loan_payment: Loan Payment + transfer: Transfer transaction: pending: Pending pending_tooltip: Pending transaction — may change when posted + excluded_from_cashflow_tooltip: Excluded from cashflow reports + activity_type_tooltip: Investment activity type possible_duplicate: Duplicate? potential_duplicate_tooltip: This may be a duplicate of another transaction review_recommended: Review diff --git a/db/migrate/20260110120000_add_investment_cashflow_support.rb b/db/migrate/20260110120000_add_investment_cashflow_support.rb new file mode 100644 index 000000000..26cffbc55 --- /dev/null +++ b/db/migrate/20260110120000_add_investment_cashflow_support.rb @@ -0,0 +1,13 @@ +class AddInvestmentCashflowSupport < ActiveRecord::Migration[7.2] + def change + # Flag for excluding from cashflow (user-controllable) + # Used for internal investment activity like fund swaps + add_column :entries, :exclude_from_cashflow, :boolean, default: false, null: false + add_index :entries, :exclude_from_cashflow + + # Holdings snapshot for comparison (provider-agnostic) + # Used to detect internal investment activity by comparing holdings between syncs + add_column :accounts, :holdings_snapshot_data, :jsonb + add_column :accounts, :holdings_snapshot_at, :datetime + end +end diff --git a/db/migrate/20260110180000_add_investment_activity_label_to_transactions.rb b/db/migrate/20260110180000_add_investment_activity_label_to_transactions.rb new file mode 100644 index 000000000..658900bc2 --- /dev/null +++ b/db/migrate/20260110180000_add_investment_activity_label_to_transactions.rb @@ -0,0 +1,8 @@ +class AddInvestmentActivityLabelToTransactions < ActiveRecord::Migration[7.2] + def change + # Label for investment activity type (Buy, Sell, Sweep In, Dividend, etc.) + # Provides human-readable context for why a transaction is excluded from cashflow + add_column :transactions, :investment_activity_label, :string + add_index :transactions, :investment_activity_label + end +end diff --git a/db/schema.rb b/db/schema.rb index a4b314661..04b9920ae 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: 2026_01_12_065106) do +ActiveRecord::Schema[7.2].define(version: 2026_01_10_180000) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -476,22 +476,6 @@ ActiveRecord::Schema[7.2].define(version: 2026_01_12_065106) do t.index ["merchant_id"], name: "index_family_merchant_associations_on_merchant_id" end - create_table "flipper_features", force: :cascade do |t| - t.string "key", null: false - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.index ["key"], name: "index_flipper_features_on_key", unique: true - end - - create_table "flipper_gates", force: :cascade do |t| - t.string "feature_key", null: false - t.string "key", null: false - t.text "value" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.index ["feature_key", "key", "value"], name: "index_flipper_gates_on_feature_key_and_key_and_value", unique: true - end - create_table "holdings", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| t.uuid "account_id", null: false t.uuid "security_id", null: false @@ -505,8 +489,6 @@ ActiveRecord::Schema[7.2].define(version: 2026_01_12_065106) do t.string "external_id" t.decimal "cost_basis", precision: 19, scale: 4 t.uuid "account_provider_id" - t.string "cost_basis_source" - t.boolean "cost_basis_locked", default: false, null: false t.index ["account_id", "external_id"], name: "idx_holdings_on_account_id_external_id_unique", unique: true, where: "(external_id IS NOT NULL)" t.index ["account_id", "security_id", "date", "currency"], name: "idx_on_account_id_security_id_date_currency_5323e39f8b", unique: true t.index ["account_id"], name: "index_holdings_on_account_id" @@ -816,8 +798,6 @@ ActiveRecord::Schema[7.2].define(version: 2026_01_12_065106) do t.datetime "last_authenticated_at" t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.string "issuer" - t.index ["issuer"], name: "index_oidc_identities_on_issuer" t.index ["provider", "uid"], name: "index_oidc_identities_on_provider_and_uid", unique: true t.index ["user_id"], name: "index_oidc_identities_on_user_id" end @@ -1073,38 +1053,6 @@ ActiveRecord::Schema[7.2].define(version: 2026_01_12_065106) do t.index ["status"], name: "index_simplefin_items_on_status" end - create_table "sso_audit_logs", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| - t.uuid "user_id" - t.string "event_type", null: false - t.string "provider" - t.string "ip_address" - t.string "user_agent" - t.jsonb "metadata", default: {}, null: false - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.index ["created_at"], name: "index_sso_audit_logs_on_created_at" - t.index ["event_type"], name: "index_sso_audit_logs_on_event_type" - t.index ["user_id", "created_at"], name: "index_sso_audit_logs_on_user_id_and_created_at" - t.index ["user_id"], name: "index_sso_audit_logs_on_user_id" - end - - create_table "sso_providers", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| - t.string "strategy", null: false - t.string "name", null: false - t.string "label", null: false - t.string "icon" - t.boolean "enabled", default: true, null: false - t.string "issuer" - t.string "client_id" - t.string "client_secret" - t.string "redirect_uri" - t.jsonb "settings", default: {}, null: false - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.index ["enabled"], name: "index_sso_providers_on_enabled" - t.index ["name"], name: "index_sso_providers_on_name", unique: true - end - create_table "subscriptions", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| t.uuid "family_id", null: false t.string "status", null: false @@ -1181,14 +1129,7 @@ ActiveRecord::Schema[7.2].define(version: 2026_01_12_065106) do t.string "currency" t.jsonb "locked_attributes", default: {} t.uuid "category_id" - t.decimal "realized_gain", precision: 19, scale: 4 - t.decimal "cost_basis_amount", precision: 19, scale: 4 - t.string "cost_basis_currency" - t.integer "holding_period_days" - t.string "realized_gain_confidence" - t.string "realized_gain_currency" t.index ["category_id"], name: "index_trades_on_category_id" - t.index ["realized_gain"], name: "index_trades_on_realized_gain_not_null", where: "(realized_gain IS NOT NULL)" t.index ["security_id"], name: "index_trades_on_security_id" end @@ -1339,7 +1280,6 @@ ActiveRecord::Schema[7.2].define(version: 2026_01_12_065106) do add_foreign_key "sessions", "users" add_foreign_key "simplefin_accounts", "simplefin_items" add_foreign_key "simplefin_items", "families" - add_foreign_key "sso_audit_logs", "users" add_foreign_key "subscriptions", "families" add_foreign_key "syncs", "syncs", column: "parent_id" add_foreign_key "taggings", "tags" diff --git a/lib/tasks/investment_labels.rake b/lib/tasks/investment_labels.rake new file mode 100644 index 000000000..31741574a --- /dev/null +++ b/lib/tasks/investment_labels.rake @@ -0,0 +1,185 @@ +# frozen_string_literal: true + +# Backfill investment activity labels for existing transactions +# +# Usage examples: +# # Preview (dry run) - show what labels would be set +# bin/rails 'sure:investments:backfill_labels[dry_run=true]' +# +# # Execute the backfill for all investment/crypto accounts +# bin/rails 'sure:investments:backfill_labels[dry_run=false]' +# +# # Backfill for a specific account +# bin/rails 'sure:investments:backfill_labels[account_id=8b46387c-5aa4-4a92-963a-4392c10999c9,dry_run=false]' +# +# # Force re-label already-labeled transactions +# bin/rails 'sure:investments:backfill_labels[dry_run=false,force=true]' + +namespace :sure do + namespace :investments do + desc "Backfill activity labels for existing investment transactions. Args: account_id (optional), dry_run=true, force=false" + task :backfill_labels, [ :account_id, :dry_run, :force ] => :environment do |_, args| + # Support named args (key=value) - parse all positional args for key=value pairs + kv = {} + [ args[:account_id], args[:dry_run], args[:force] ].each do |raw| + next unless raw.is_a?(String) && raw.include?("=") + k, v = raw.split("=", 2) + kv[k.to_s] = v + end + + # Only use positional args if they don't contain "=" (otherwise they're named args in wrong position) + positional_account_id = args[:account_id] unless args[:account_id].to_s.include?("=") + positional_dry_run = args[:dry_run] unless args[:dry_run].to_s.include?("=") + positional_force = args[:force] unless args[:force].to_s.include?("=") + + account_id = (kv["account_id"] || positional_account_id).presence + dry_raw = (kv["dry_run"] || positional_dry_run).to_s.downcase + force_raw = (kv["force"] || positional_force).to_s.downcase + force = %w[true yes 1].include?(force_raw) + + # Default to dry_run=true unless explicitly disabled + dry_run = if dry_raw.blank? + true + elsif %w[1 true yes y].include?(dry_raw) + true + elsif %w[0 false no n].include?(dry_raw) + false + else + puts({ ok: false, error: "invalid_argument", message: "dry_run must be one of: true/yes/1 or false/no/0" }.to_json) + exit 1 + end + + # Build account scope + accounts = if account_id.present? + Account.where(id: account_id) + else + Account.where(accountable_type: %w[Investment Crypto]) + end + + if accounts.none? + puts({ ok: false, error: "no_accounts", message: "No investment/crypto accounts found" }.to_json) + exit 1 + end + + total_processed = 0 + total_labeled = 0 + total_skipped = 0 + total_errors = 0 + + accounts.find_each do |account| + # Skip non-investment/crypto accounts if processing all + next unless account.investment? || account.crypto? + + acct_processed = 0 + acct_labeled = 0 + acct_skipped = 0 + acct_errors = 0 + + # Find transactions (optionally include already-labeled if force=true) + entries = account.entries + .joins("INNER JOIN transactions ON transactions.id = entries.entryable_id AND entries.entryable_type = 'Transaction'") + + unless force + entries = entries.where("transactions.investment_activity_label IS NULL OR transactions.investment_activity_label = ''") + end + + entries.find_each do |entry| + acct_processed += 1 + total_processed += 1 + + begin + transaction = entry.transaction + current_label = transaction.investment_activity_label + label = InvestmentActivityDetector.infer_label_from_description(entry.name, entry.amount, account) + + # Skip if no label can be inferred + if label.blank? + acct_skipped += 1 + total_skipped += 1 + next + end + + # Skip if label unchanged (when force=true) + if current_label == label + acct_skipped += 1 + total_skipped += 1 + next + end + + if dry_run + if current_label.present? + puts " [DRY RUN] Would relabel '#{entry.name}' (#{entry.amount}) from '#{current_label}' to '#{label}'" + else + puts " [DRY RUN] Would label '#{entry.name}' (#{entry.amount}) as '#{label}'" + end + else + transaction.update!(investment_activity_label: label) + if current_label.present? + puts " Relabeled '#{entry.name}' (#{entry.amount}) from '#{current_label}' to '#{label}'" + else + puts " Labeled '#{entry.name}' (#{entry.amount}) as '#{label}'" + end + end + acct_labeled += 1 + total_labeled += 1 + rescue => e + acct_errors += 1 + total_errors += 1 + puts({ error: e.class.name, message: e.message, entry_id: entry.id }.to_json) + end + end + + puts({ account_id: account.id, account_name: account.name, accountable_type: account.accountable_type, processed: acct_processed, labeled: acct_labeled, skipped: acct_skipped, errors: acct_errors, dry_run: dry_run, force: force }.to_json) + end + + puts({ ok: true, total_processed: total_processed, total_labeled: total_labeled, total_skipped: total_skipped, total_errors: total_errors, dry_run: dry_run }.to_json) + end + + desc "Clear all investment activity labels (for testing). Args: account_id (required), dry_run=true" + task :clear_labels, [ :account_id, :dry_run ] => :environment do |_, args| + kv = {} + [ args[:account_id], args[:dry_run] ].each do |raw| + next unless raw.is_a?(String) && raw.include?("=") + k, v = raw.split("=", 2) + kv[k.to_s] = v + end + + # Only use positional args if they don't contain "=" + positional_account_id = args[:account_id] unless args[:account_id].to_s.include?("=") + positional_dry_run = args[:dry_run] unless args[:dry_run].to_s.include?("=") + + account_id = (kv["account_id"] || positional_account_id).presence + dry_raw = (kv["dry_run"] || positional_dry_run).to_s.downcase + + unless account_id.present? + puts({ ok: false, error: "usage", message: "Provide account_id" }.to_json) + exit 1 + end + + dry_run = if dry_raw.blank? + true + elsif %w[1 true yes y].include?(dry_raw) + true + else + false + end + + account = Account.find(account_id) + + count = account.entries + .joins("INNER JOIN transactions ON transactions.id = entries.entryable_id AND entries.entryable_type = 'Transaction'") + .where("transactions.investment_activity_label IS NOT NULL AND transactions.investment_activity_label != ''") + .count + + if dry_run + puts({ ok: true, message: "Would clear #{count} labels", dry_run: true }.to_json) + else + Transaction.joins(:entry) + .where(entries: { account_id: account_id }) + .where("investment_activity_label IS NOT NULL AND investment_activity_label != ''") + .update_all(investment_activity_label: nil) + puts({ ok: true, message: "Cleared #{count} labels", dry_run: false }.to_json) + end + end + end +end diff --git a/test/models/income_statement_test.rb b/test/models/income_statement_test.rb index b152917c6..ba5fa3e81 100644 --- a/test/models/income_statement_test.rb +++ b/test/models/income_statement_test.rb @@ -285,4 +285,70 @@ class IncomeStatementTest < ActiveSupport::TestCase assert_equal 5, totals.transactions_count assert_equal Money.new(1050, @family.currency), totals.expense_money # 900 + 150 end + + # NEW TESTS: exclude_from_cashflow Feature + test "excludes transactions with exclude_from_cashflow flag from totals" do + # Create an expense transaction and mark it as excluded from cashflow + excluded_entry = create_transaction(account: @checking_account, amount: 250, category: @groceries_category) + excluded_entry.update!(exclude_from_cashflow: true) + + income_statement = IncomeStatement.new(@family) + totals = income_statement.totals(date_range: Period.last_30_days.date_range) + + # Should NOT include the excluded transaction + assert_equal 4, totals.transactions_count # Only original 4 transactions + assert_equal Money.new(1000, @family.currency), totals.income_money + assert_equal Money.new(900, @family.currency), totals.expense_money + end + + test "excludes income transactions with exclude_from_cashflow flag" do + # Create income and mark as excluded from cashflow + excluded_income = create_transaction(account: @checking_account, amount: -500, category: @income_category) + excluded_income.update!(exclude_from_cashflow: true) + + income_statement = IncomeStatement.new(@family) + totals = income_statement.totals(date_range: Period.last_30_days.date_range) + + # Should NOT include the excluded income + assert_equal 4, totals.transactions_count + assert_equal Money.new(1000, @family.currency), totals.income_money # Original income only + assert_equal Money.new(900, @family.currency), totals.expense_money + end + + test "excludes investment_contribution transactions from income statement" do + # Create a transfer to investment account (marked as investment_contribution) + investment_contribution = create_transaction( + account: @checking_account, + amount: 1000, + category: nil, + kind: "investment_contribution" + ) + + income_statement = IncomeStatement.new(@family) + totals = income_statement.totals(date_range: Period.last_30_days.date_range) + + # investment_contribution should be excluded (it's in the exclusion list) + assert_equal 4, totals.transactions_count # Only original 4 transactions + assert_equal Money.new(1000, @family.currency), totals.income_money + assert_equal Money.new(900, @family.currency), totals.expense_money + end + + test "exclude_from_cashflow works with median calculations" do + # Clear existing transactions + Entry.joins(:account).where(accounts: { family_id: @family.id }).destroy_all + + # Create expenses: 100, 200, 300 + create_transaction(account: @checking_account, amount: 100, category: @groceries_category) + create_transaction(account: @checking_account, amount: 200, category: @groceries_category) + excluded_entry = create_transaction(account: @checking_account, amount: 300, category: @groceries_category) + + # Exclude the 300 transaction from cashflow + excluded_entry.update!(exclude_from_cashflow: true) + + income_statement = IncomeStatement.new(@family) + + # Median should only consider non-excluded transactions (100, 200) + # Monthly total = 300, so median = 300.0 + assert_equal 300.0, income_statement.median_expense(interval: "month") + end end diff --git a/test/models/investment_activity_detector_test.rb b/test/models/investment_activity_detector_test.rb new file mode 100644 index 000000000..68ea44ac2 --- /dev/null +++ b/test/models/investment_activity_detector_test.rb @@ -0,0 +1,299 @@ +require "test_helper" + +class InvestmentActivityDetectorTest < ActiveSupport::TestCase + include EntriesTestHelper + + setup do + @family = families(:empty) + @investment_account = @family.accounts.create!( + name: "Brokerage", + balance: 10000, + cash_balance: 2000, + currency: "USD", + accountable: Investment.new + ) + @detector = InvestmentActivityDetector.new(@investment_account) + end + + test "detects new holding purchase and marks matching transaction" do + # Create a transaction that matches a new holding purchase + entry = create_transaction( + account: @investment_account, + amount: 1000, + name: "Buy VFIAX" + ) + transaction = entry.transaction + + # Simulate holdings snapshot showing a new holding + current_holdings = [ + { "symbol" => "VFIAX", "cost_basis" => 1000.0, "shares" => 10 } + ] + + # No previous snapshot + @investment_account.update!(holdings_snapshot_data: nil, holdings_snapshot_at: nil) + + @detector.detect_and_mark_internal_activity(current_holdings, [ transaction ]) + + entry.reload + assert entry.exclude_from_cashflow?, "Transaction matching new holding should be excluded from cashflow" + end + + test "detects holding sale and marks matching transaction" do + # Set up previous holdings + previous_holdings = [ + { "symbol" => "VFIAX", "cost_basis" => 2000.0, "shares" => 20 } + ] + @investment_account.update!( + holdings_snapshot_data: previous_holdings, + holdings_snapshot_at: 1.day.ago + ) + + # Create a transaction for the sale proceeds (negative = inflow) + entry = create_transaction( + account: @investment_account, + amount: -1000, + name: "Sell VFIAX" + ) + transaction = entry.transaction + + # Current holdings show reduced position + current_holdings = [ + { "symbol" => "VFIAX", "cost_basis" => 1000.0, "shares" => 10 } + ] + + @detector.detect_and_mark_internal_activity(current_holdings, [ transaction ]) + + entry.reload + assert entry.exclude_from_cashflow?, "Transaction matching holding sale should be excluded from cashflow" + end + + test "respects locked exclude_from_cashflow attribute" do + # Create a transaction and lock the attribute + entry = create_transaction( + account: @investment_account, + amount: 1000, + name: "Buy VFIAX" + ) + transaction = entry.transaction + + # User explicitly set to NOT exclude (and locked it) + entry.update!(exclude_from_cashflow: false) + entry.lock_attr!(:exclude_from_cashflow) + + current_holdings = [ + { "symbol" => "VFIAX", "cost_basis" => 1000.0, "shares" => 10 } + ] + + @detector.detect_and_mark_internal_activity(current_holdings, [ transaction ]) + + entry.reload + assert_not entry.exclude_from_cashflow?, "Locked attribute should not be overwritten" + end + + test "updates holdings snapshot after detection" do + current_holdings = [ + { "symbol" => "VFIAX", "cost_basis" => 1000.0, "shares" => 10 }, + { "symbol" => "IBIT", "cost_basis" => 500.0, "shares" => 5 } + ] + + @detector.detect_and_mark_internal_activity(current_holdings, []) + + @investment_account.reload + # Snapshot is normalized with string values and additional fields + snapshot = @investment_account.holdings_snapshot_data + assert_equal 2, snapshot.size + assert_equal "VFIAX", snapshot[0]["symbol"] + assert_equal "1000.0", snapshot[0]["cost_basis"] + assert_equal "10.0", snapshot[0]["shares"] + assert_equal "IBIT", snapshot[1]["symbol"] + assert_not_nil @investment_account.holdings_snapshot_at + end + + test "matches transaction by cost_basis amount within tolerance" do + entry = create_transaction( + account: @investment_account, + amount: 1000.005, # Very close - within 0.01 tolerance + name: "Investment purchase" + ) + transaction = entry.transaction + + # Holding with cost basis close to transaction amount (within 0.01) + current_holdings = [ + { "symbol" => "VFIAX", "cost_basis" => 1000.0, "shares" => 10 } + ] + + @detector.detect_and_mark_internal_activity(current_holdings, [ transaction ]) + + entry.reload + assert entry.exclude_from_cashflow?, "Should match transaction within tolerance" + end + + test "does not mark unrelated transactions" do + # Create a regular expense transaction + entry = create_transaction( + account: @investment_account, + amount: 50, + name: "Account fee" + ) + transaction = entry.transaction + + # Holdings that don't match + current_holdings = [ + { "symbol" => "VFIAX", "cost_basis" => 1000.0, "shares" => 10 } + ] + + @detector.detect_and_mark_internal_activity(current_holdings, [ transaction ]) + + entry.reload + assert_not entry.exclude_from_cashflow?, "Unrelated transaction should not be excluded" + end + + test "works with crypto accounts" do + crypto_account = @family.accounts.create!( + name: "Crypto Wallet", + balance: 5000, + currency: "USD", + accountable: Crypto.new + ) + detector = InvestmentActivityDetector.new(crypto_account) + + entry = create_transaction( + account: crypto_account, + amount: 1000, + name: "Buy BTC" + ) + transaction = entry.transaction + + current_holdings = [ + { "symbol" => "BTC", "cost_basis" => 1000.0, "shares" => 0.02 } + ] + + detector.detect_and_mark_internal_activity(current_holdings, [ transaction ]) + + entry.reload + assert entry.exclude_from_cashflow?, "Should work with crypto accounts" + end + + test "handles empty holdings gracefully" do + entry = create_transaction( + account: @investment_account, + amount: 1000, + name: "Some transaction" + ) + transaction = entry.transaction + + # Should not raise, just do nothing + assert_nothing_raised do + @detector.detect_and_mark_internal_activity([], [ transaction ]) + end + + entry.reload + assert_not entry.exclude_from_cashflow? + end + + test "handles nil holdings gracefully" do + entry = create_transaction( + account: @investment_account, + amount: 1000, + name: "Some transaction" + ) + transaction = entry.transaction + + assert_nothing_raised do + @detector.detect_and_mark_internal_activity(nil, [ transaction ]) + end + + entry.reload + assert_not entry.exclude_from_cashflow? + end + + test "sets Buy label for new holding purchase" do + entry = create_transaction( + account: @investment_account, + amount: 1000, + name: "Some investment" + ) + transaction = entry.transaction + + current_holdings = [ + { "symbol" => "VFIAX", "cost_basis" => 1000.0, "shares" => 10 } + ] + + @detector.detect_and_mark_internal_activity(current_holdings, [ transaction ]) + + transaction.reload + assert_equal "Buy", transaction.investment_activity_label + end + + test "sets Sell label for holding sale" do + previous_holdings = [ + { "symbol" => "VFIAX", "cost_basis" => 2000.0, "shares" => 20 } + ] + @investment_account.update!( + holdings_snapshot_data: previous_holdings, + holdings_snapshot_at: 1.day.ago + ) + + entry = create_transaction( + account: @investment_account, + amount: -1000, + name: "VFIAX Sale" + ) + transaction = entry.transaction + + current_holdings = [ + { "symbol" => "VFIAX", "cost_basis" => 1000.0, "shares" => 10 } + ] + + @detector.detect_and_mark_internal_activity(current_holdings, [ transaction ]) + + transaction.reload + assert_equal "Sell", transaction.investment_activity_label + end + + test "infers Sweep In label from money market description" do + entry = create_transaction( + account: @investment_account, + amount: -500, + name: "VANGUARD FEDERAL MONEY MARKET" + ) + transaction = entry.transaction + + # Call with empty holdings but simulate it being a sweep + # This tests the infer_from_description fallback + current_holdings = [ + { "symbol" => "VMFXX", "cost_basis" => 500.0, "shares" => 500 } + ] + + @detector.detect_and_mark_internal_activity(current_holdings, [ transaction ]) + + transaction.reload + # Should be either "Buy" (from holdings match) or "Sweep In" (from description) + assert transaction.investment_activity_label.present? + end + + test "infers Dividend label from CASH description" do + entry = create_transaction( + account: @investment_account, + amount: -50, + name: "CASH" + ) + transaction = entry.transaction + + # No holdings change, but description-based inference + current_holdings = [ + { "symbol" => "VFIAX", "cost_basis" => 1000.0, "shares" => 10 } + ] + @investment_account.update!( + holdings_snapshot_data: current_holdings, + holdings_snapshot_at: 1.day.ago + ) + + @detector.detect_and_mark_internal_activity(current_holdings, [ transaction ]) + + # Since there's no holdings change, no label gets set via holdings match + # But if we manually test the infer_from_description method... + label = @detector.send(:infer_from_description, entry) + assert_equal "Dividend", label + end +end diff --git a/test/models/transaction_test.rb b/test/models/transaction_test.rb index 7b5f2faeb..3d7b3236b 100644 --- a/test/models/transaction_test.rb +++ b/test/models/transaction_test.rb @@ -18,4 +18,37 @@ class TransactionTest < ActiveSupport::TestCase assert_not transaction.pending? end + + test "investment_contribution is a valid kind" do + transaction = Transaction.new(kind: "investment_contribution") + + assert_equal "investment_contribution", transaction.kind + assert transaction.investment_contribution? + end + + test "all transaction kinds are valid" do + valid_kinds = %w[standard funds_movement cc_payment loan_payment one_time investment_contribution] + + valid_kinds.each do |kind| + transaction = Transaction.new(kind: kind) + assert_equal kind, transaction.kind, "#{kind} should be a valid transaction kind" + end + end + + test "INTERNAL_ACTIVITY_LABELS contains expected labels" do + assert_includes Transaction::INTERNAL_ACTIVITY_LABELS, "Buy" + assert_includes Transaction::INTERNAL_ACTIVITY_LABELS, "Sell" + assert_includes Transaction::INTERNAL_ACTIVITY_LABELS, "Reinvestment" + assert_includes Transaction::INTERNAL_ACTIVITY_LABELS, "Exchange" + end + + test "ACTIVITY_LABELS contains all valid labels" do + assert_includes Transaction::ACTIVITY_LABELS, "Buy" + assert_includes Transaction::ACTIVITY_LABELS, "Sell" + assert_includes Transaction::ACTIVITY_LABELS, "Sweep In" + assert_includes Transaction::ACTIVITY_LABELS, "Sweep Out" + assert_includes Transaction::ACTIVITY_LABELS, "Dividend" + assert_includes Transaction::ACTIVITY_LABELS, "Interest" + assert_includes Transaction::ACTIVITY_LABELS, "Fee" + end end diff --git a/test/models/transfer_test.rb b/test/models/transfer_test.rb index 331638165..e47d200d2 100644 --- a/test/models/transfer_test.rb +++ b/test/models/transfer_test.rb @@ -104,4 +104,24 @@ class TransferTest < ActiveSupport::TestCase Transfer.create!(inflow_transaction: inflow_entry2.transaction, outflow_transaction: outflow_entry.transaction) end end + + test "kind_for_account returns investment_contribution for investment accounts" do + assert_equal "investment_contribution", Transfer.kind_for_account(accounts(:investment)) + end + + test "kind_for_account returns investment_contribution for crypto accounts" do + assert_equal "investment_contribution", Transfer.kind_for_account(accounts(:crypto)) + end + + test "kind_for_account returns loan_payment for loan accounts" do + assert_equal "loan_payment", Transfer.kind_for_account(accounts(:loan)) + end + + test "kind_for_account returns cc_payment for credit card accounts" do + assert_equal "cc_payment", Transfer.kind_for_account(accounts(:credit_card)) + end + + test "kind_for_account returns funds_movement for depository accounts" do + assert_equal "funds_movement", Transfer.kind_for_account(accounts(:depository)) + end end From 96022cbe9a76324eb32a946e0ee706e560a3a9a3 Mon Sep 17 00:00:00 2001 From: Josh Waldrep Date: Sat, 10 Jan 2026 20:03:51 -0500 Subject: [PATCH 03/10] Update `PlaidAccount::Processor` to use resolved security from response - Refactored to extract `security` from `security_resolver.resolve` response for better clarity and consistency. --- app/models/plaid_account/processor.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/models/plaid_account/processor.rb b/app/models/plaid_account/processor.rb index e282d5e9d..3dc09c710 100644 --- a/app/models/plaid_account/processor.rb +++ b/app/models/plaid_account/processor.rb @@ -121,7 +121,8 @@ class PlaidAccount::Processor # Transform to common format current_holdings = raw_holdings.map do |h| - security = security_resolver.resolve(h["security_id"]) + response = security_resolver.resolve(plaid_security_id: h["security_id"]) + security = response.security { "symbol" => security&.ticker, "description" => security&.name, From e5fbdfb593dcb1c5ec5fbd8993b94a8f853d26f8 Mon Sep 17 00:00:00 2001 From: LPW Date: Mon, 12 Jan 2026 08:05:46 -0500 Subject: [PATCH 04/10] Add cost basis source tracking with manual override and lock protection (#623) * Add cost basis tracking and management to holdings - Added migration to introduce `cost_basis_source` and `cost_basis_locked` fields to `holdings`. - Implemented backfill for existing holdings to set `cost_basis_source` based on heuristics. - Introduced `Holding::CostBasisReconciler` to manage cost basis resolution logic. - Added user interface components for editing and locking cost basis in holdings. - Updated `materializer` to integrate reconciliation logic and respect locked holdings. - Extended tests for cost basis-related workflows to ensure accuracy and reliability. * Fix cost basis calculation in holdings controller - Ensure `cost_basis` is converted to decimal for accurate arithmetic. - Fix conditional check to properly validate positive `cost_basis`. * Improve cost basis validation and error handling in holdings controller - Allow zero as a valid cost basis for gifted/inherited shares. - Add error handling with user feedback for invalid cost basis values. --------- Co-authored-by: Josh Waldrep --- db/schema.rb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/db/schema.rb b/db/schema.rb index 04b9920ae..067d2c9f6 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: 2026_01_10_180000) do +ActiveRecord::Schema[7.2].define(version: 2026_01_12_065106) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -489,6 +489,8 @@ ActiveRecord::Schema[7.2].define(version: 2026_01_10_180000) do t.string "external_id" t.decimal "cost_basis", precision: 19, scale: 4 t.uuid "account_provider_id" + t.string "cost_basis_source" + t.boolean "cost_basis_locked", default: false, null: false t.index ["account_id", "external_id"], name: "idx_holdings_on_account_id_external_id_unique", unique: true, where: "(external_id IS NOT NULL)" t.index ["account_id", "security_id", "date", "currency"], name: "idx_on_account_id_security_id_date_currency_5323e39f8b", unique: true t.index ["account_id"], name: "index_holdings_on_account_id" @@ -1129,7 +1131,14 @@ ActiveRecord::Schema[7.2].define(version: 2026_01_10_180000) do t.string "currency" t.jsonb "locked_attributes", default: {} t.uuid "category_id" + t.decimal "realized_gain", precision: 19, scale: 4 + t.decimal "cost_basis_amount", precision: 19, scale: 4 + t.string "cost_basis_currency" + t.integer "holding_period_days" + t.string "realized_gain_confidence" + t.string "realized_gain_currency" t.index ["category_id"], name: "index_trades_on_category_id" + t.index ["realized_gain"], name: "index_trades_on_realized_gain_not_null", where: "(realized_gain IS NOT NULL)" t.index ["security_id"], name: "index_trades_on_security_id" end From 307a8bb760456961d1cd7a8ddf9f7fbf5b7ee51a Mon Sep 17 00:00:00 2001 From: Josh Waldrep Date: Mon, 12 Jan 2026 09:19:09 -0500 Subject: [PATCH 05/10] Localize investment activity labels and improve transaction processing - Replaced hardcoded activity labels with `I18n` translations for better localization. - Updated `transactions` views to display localized labels dynamically. - Fixed `InvestmentActivityDetector` to enhance dividend detection. - Refined `Account::ProviderImportAdapter` to prevent unnecessary updates and ensure transactional consistency. - Improved error handling and feedback in rake tasks for invalid arguments. --- app/models/account/provider_import_adapter.rb | 3 ++- app/models/investment_activity_detector.rb | 5 +++-- app/views/transactions/_transaction.html.erb | 2 +- app/views/transactions/show.html.erb | 6 +++--- config/locales/views/transactions/en.yml | 16 ++++++++++++++++ lib/tasks/investment_labels.rake | 6 +++++- 6 files changed, 30 insertions(+), 8 deletions(-) diff --git a/app/models/account/provider_import_adapter.rb b/app/models/account/provider_import_adapter.rb index 31d65a6ae..dac0da8d4 100644 --- a/app/models/account/provider_import_adapter.rb +++ b/app/models/account/provider_import_adapter.rb @@ -119,11 +119,12 @@ class Account::ProviderImportAdapter # Set investment activity label if provided and not already set if investment_activity_label.present? && entry.entryable.is_a?(Transaction) if entry.transaction.investment_activity_label.blank? - entry.transaction.update!(investment_activity_label: investment_activity_label) + entry.transaction.assign_attributes(investment_activity_label: investment_activity_label) end end entry.save! + entry.transaction.save! if entry.transaction.changed? # AFTER save: For NEW posted transactions, check for fuzzy matches to SUGGEST (not auto-claim) # This handles tip adjustments where auto-matching is too risky diff --git a/app/models/investment_activity_detector.rb b/app/models/investment_activity_detector.rb index c030d9cdf..2ef6c2f71 100644 --- a/app/models/investment_activity_detector.rb +++ b/app/models/investment_activity_detector.rb @@ -119,8 +119,9 @@ class InvestmentActivityDetector end # Check for dividend patterns - if description == "CASH" || description.include?("DIVIDEND") || - description.include?("DISTRIBUTION") + # "CASH" alone typically indicates dividend payout in brokerage feeds (only for inflows) + if description.include?("DIVIDEND") || description.include?("DISTRIBUTION") || + (description == "CASH" && amount < 0) return "Dividend" end diff --git a/app/views/transactions/_transaction.html.erb b/app/views/transactions/_transaction.html.erb index a4593a56d..c645d515d 100644 --- a/app/views/transactions/_transaction.html.erb +++ b/app/views/transactions/_transaction.html.erb @@ -87,7 +87,7 @@ <%# Investment activity label badge %> <% if transaction.investment_activity_label.present? %> "> - <%= transaction.investment_activity_label %> + <%= t("transactions.activity_labels.#{transaction.investment_activity_label.parameterize(separator: '_')}") %> <% end %> diff --git a/app/views/transactions/show.html.erb b/app/views/transactions/show.html.erb index 85e770509..4af991f1a 100644 --- a/app/views/transactions/show.html.erb +++ b/app/views/transactions/show.html.erb @@ -240,7 +240,7 @@ <%= ef.select :investment_activity_label, options_for_select( - [["—", nil]] + Transaction::ACTIVITY_LABELS.map { |l| [l, l] }, + [["—", nil]] + Transaction::ACTIVITY_LABELS.map { |l| [t("transactions.activity_labels.#{l.parameterize(separator: '_')}"), l] }, @entry.entryable.investment_activity_label ), { label: false }, @@ -260,8 +260,8 @@ <%= f.fields_for :entryable do |ef| %>
-

One-time <%= @entry.amount.negative? ? "Income" : "Expense" %>

-

One-time transactions will be excluded from certain budgeting calculations and reports to help you see what's really important.

+

<%= t(".one_time_title", type: @entry.amount.negative? ? t("transactions.form.income") : t("transactions.form.expense")) %>

+

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

<%= ef.toggle :kind, { diff --git a/config/locales/views/transactions/en.yml b/config/locales/views/transactions/en.yml index 0a393d835..267adce43 100644 --- a/config/locales/views/transactions/en.yml +++ b/config/locales/views/transactions/en.yml @@ -38,6 +38,22 @@ en: exclude_from_cashflow_description_investment: Hide from income/expense reports and Sankey chart. Use for internal investment activity like fund swaps, reinvestments, or money market sweeps. activity_type: Activity Type activity_type_description: Type of investment activity (Buy, Sell, Dividend, etc.). Auto-detected or set manually. + one_time_title: One-time %{type} + one_time_description: One-time transactions will be excluded from certain budgeting calculations and reports to help you see what's really important. + activity_labels: + buy: Buy + sell: Sell + sweep_in: Sweep In + sweep_out: Sweep Out + dividend: Dividend + reinvestment: Reinvestment + interest: Interest + fee: Fee + transfer: Transfer + contribution: Contribution + withdrawal: Withdrawal + exchange: Exchange + other: Other mark_recurring: Mark as Recurring mark_recurring_subtitle: Track this as a recurring transaction. Amount variance is automatically calculated from past 6 months of similar transactions. mark_recurring_title: Recurring Transaction diff --git a/lib/tasks/investment_labels.rake b/lib/tasks/investment_labels.rake index 31741574a..9cefae7c9 100644 --- a/lib/tasks/investment_labels.rake +++ b/lib/tasks/investment_labels.rake @@ -78,6 +78,7 @@ namespace :sure do # Find transactions (optionally include already-labeled if force=true) entries = account.entries .joins("INNER JOIN transactions ON transactions.id = entries.entryable_id AND entries.entryable_type = 'Transaction'") + .includes(:entryable) unless force entries = entries.where("transactions.investment_activity_label IS NULL OR transactions.investment_activity_label = ''") @@ -160,8 +161,11 @@ namespace :sure do true elsif %w[1 true yes y].include?(dry_raw) true - else + elsif %w[0 false no n].include?(dry_raw) false + else + puts({ ok: false, error: "invalid_argument", message: "dry_run must be one of: true/yes/1 or false/no/0" }.to_json) + exit 1 end account = Account.find(account_id) From cfda5a6d3d2644ff96477389d1f0a1394840510e Mon Sep 17 00:00:00 2001 From: Josh Waldrep Date: Mon, 12 Jan 2026 11:13:49 -0500 Subject: [PATCH 06/10] Remove `InvestmentActivityDetector` and related functionality - Deleted the `InvestmentActivityDetector` and associated tests. - Removed rake tasks for backfilling and clearing investment activity labels. - Simplified transaction processing in `SimplefinEntry::Processor` by removing inferred activity label logic. - Added new rule `SetInvestmentActivityLabel` for setting labels using rules. - Updated `Rule::Registry::TransactionResource` to include the new rule executor. --- app/models/investment_activity_detector.rb | 347 ------------------ app/models/lunchflow_account/processor.rb | 27 -- app/models/plaid_account/processor.rb | 41 --- .../set_investment_activity_label.rb | 31 ++ .../rule/registry/transaction_resource.rb | 1 + app/models/simplefin_account/processor.rb | 27 -- app/models/simplefin_entry/processor.rb | 10 +- app/models/transaction.rb | 20 - lib/tasks/investment_labels.rake | 189 ---------- .../investment_activity_detector_test.rb | 299 --------------- test/models/rule/action_test.rb | 32 ++ test/models/transaction_test.rb | 7 - 12 files changed, 65 insertions(+), 966 deletions(-) delete mode 100644 app/models/investment_activity_detector.rb create mode 100644 app/models/rule/action_executor/set_investment_activity_label.rb delete mode 100644 lib/tasks/investment_labels.rake delete mode 100644 test/models/investment_activity_detector_test.rb diff --git a/app/models/investment_activity_detector.rb b/app/models/investment_activity_detector.rb deleted file mode 100644 index 2ef6c2f71..000000000 --- a/app/models/investment_activity_detector.rb +++ /dev/null @@ -1,347 +0,0 @@ -# Detects internal investment activity (fund swaps, reinvestments) by comparing -# holdings snapshots between syncs and marks matching transactions as excluded -# from cashflow. This is provider-agnostic and works with any holdings data. -# -# Usage: -# detector = InvestmentActivityDetector.new(account) -# detector.detect_and_mark_internal_activity(current_holdings, recent_transactions) -# -class InvestmentActivityDetector - def initialize(account) - @account = account - end - - # Class method for inferring activity label from description and amount - # without needing a full detector instance - # @param name [String] Transaction name/description - # @param amount [Numeric] Transaction amount - # @param account [Account, nil] Optional account for context (e.g., retirement plan detection) - # @return [String, nil] Activity label or nil if unknown - def self.infer_label_from_description(name, amount, account = nil) - new(nil).send(:infer_from_description, name, amount, account) - end - - # Call this after syncing transactions for an investment/crypto account - # @param current_holdings [Array] Array of holding objects/hashes from provider - # @param recent_transactions [Array] Recently imported transactions - def detect_and_mark_internal_activity(current_holdings, recent_transactions) - return unless @account.investment? || @account.crypto? - return if current_holdings.blank? - - previous_snapshot = @account.holdings_snapshot_data || [] - - # Find holdings changes that indicate buys/sells - changes = detect_holdings_changes(previous_snapshot, current_holdings) - - # Match changes to transactions and mark them as excluded - changes.each do |change| - matched_entry = find_matching_entry(change, recent_transactions) - next unless matched_entry - - transaction = matched_entry.entryable - - # Only auto-set if not already manually set by user (respect user overrides) - unless matched_entry.locked?(:exclude_from_cashflow) - matched_entry.update!(exclude_from_cashflow: true) - matched_entry.lock_attr!(:exclude_from_cashflow) - - Rails.logger.info( - "InvestmentActivityDetector: Auto-excluded entry #{matched_entry.id} " \ - "(#{matched_entry.name}) as internal #{change[:type]} of #{change[:symbol] || change[:description]}" - ) - end - - # Set activity label if not already set - if transaction.is_a?(Transaction) && transaction.investment_activity_label.blank? - label = infer_activity_label(matched_entry, change[:type]) - transaction.update!(investment_activity_label: label) if label.present? - end - end - - # Store current snapshot for next comparison - save_holdings_snapshot(current_holdings) - end - - private - - # Infer activity label from change type and transaction description - def infer_activity_label(entry, change_type) - # If we know it's a buy or sell from holdings comparison - return "Buy" if change_type == :buy - return "Sell" if change_type == :sell - - # Otherwise try to infer from description - infer_from_description(entry) - end - - # Infer activity label from transaction description - # Can be called with an Entry or with name/amount directly - # @param entry_or_name [Entry, String] Entry object or transaction name - # @param amount [Numeric, nil] Transaction amount (required if entry_or_name is String) - # @param account [Account, nil] Optional account for context (e.g., retirement plan detection) - def infer_from_description(entry_or_name, amount = nil, account = nil) - if entry_or_name.respond_to?(:name) - description = (entry_or_name.name || "").upcase - amount = entry_or_name.amount || 0 - account ||= entry_or_name.try(:account) - else - description = (entry_or_name || "").upcase - amount ||= 0 - end - - # Check if this is a retirement plan account (401k, 403b, etc.) - account_name = (account&.name || "").upcase - retirement_indicators = %w[401K 403B RETIREMENT TOTALSOURCE NETBENEFITS] - retirement_phrases = [ "SAVINGS PLAN", "THRIFT PLAN", "PENSION" ] - is_retirement_plan = retirement_indicators.any? { |ind| account_name.include?(ind) } || - retirement_phrases.any? { |phrase| account_name.include?(phrase) } - - # Check for sweep/money market patterns (but NOT money market FUND purchases) - # INVESTOR CL indicates this is a money market fund, not a sweep - sweep_patterns = %w[SWEEP SETTLEMENT] - money_market_sweep = description.include?("MONEY MARKET") && !description.include?("INVESTOR") - common_money_market_tickers = %w[VMFXX SPAXX FDRXX SWVXX SPRXX] - - if sweep_patterns.any? { |p| description.include?(p) } || - money_market_sweep || - common_money_market_tickers.any? { |t| description == t } - return amount.positive? ? "Sweep Out" : "Sweep In" - end - - # Check for likely interest/dividend on money market funds - # Small amounts (under $5) on money market funds are typically interest income - money_market_fund_patterns = %w[MONEY\ MARKET VMFXX SPAXX FDRXX SWVXX SPRXX VUSXX] - is_money_market_fund = money_market_fund_patterns.any? { |p| description.include?(p) } - - if is_money_market_fund && amount.abs < 5 - # Small money market amounts are interest, not buys/sells - return "Interest" - end - - # Check for dividend patterns - # "CASH" alone typically indicates dividend payout in brokerage feeds (only for inflows) - if description.include?("DIVIDEND") || description.include?("DISTRIBUTION") || - (description == "CASH" && amount < 0) - return "Dividend" - end - - # Check for interest - return "Interest" if description.include?("INTEREST") - - # Check for fees - return "Fee" if description.include?("FEE") || description.include?("CHARGE") - - # Check for reinvestment - return "Reinvestment" if description.include?("REINVEST") - - # Check for exchange/conversion - return "Exchange" if description.include?("EXCHANGE") || description.include?("CONVERSION") - - # Check for contribution patterns - return "Contribution" if description.include?("CONTRIBUTION") || description.include?("DEPOSIT") - - # Check for withdrawal patterns - return "Withdrawal" if description.include?("WITHDRAWAL") || description.include?("DISBURSEMENT") - - # Check for fund names that indicate buy/sell activity - # Positive amount = money out from account perspective = buying securities - # Negative amount = money in = selling securities - fund_patterns = %w[ - INDEX FUND ADMIRAL ETF SHARES TRUST - VANGUARD FIDELITY SCHWAB ISHARES SPDR - 500\ INDEX TOTAL\ MARKET GROWTH BOND - ] - - # Common fund ticker patterns - fund_ticker_patterns = %w[ - VFIAX VTSAX VXUS VBTLX VTIAX VTTVX - VTI VOO VGT VIG VYM VGIT - FXAIX FZROX FSKAX FBALX - SWTSX SWPPX SCHD SCHX - SPY QQQ IVV AGG - IBIT GBTC ETHE - ] - - is_fund_transaction = fund_patterns.any? { |p| description.include?(p) } || - fund_ticker_patterns.any? { |t| description.include?(t) } - - if is_fund_transaction - if is_retirement_plan && amount.negative? - # Negative amount in retirement plan = payroll contribution buying shares - return "Contribution" - else - return amount.positive? ? "Buy" : "Sell" - end - end - - nil # Unknown - user can set manually - end - - def detect_holdings_changes(previous, current) - changes = [] - - current.each do |holding| - prev = find_previous_holding(previous, holding) - - if prev.nil? - # New holding appeared = BUY - changes << { - type: :buy, - symbol: holding_symbol(holding), - description: holding_description(holding), - shares: holding_shares(holding), - cost_basis: holding_cost_basis(holding), - created_at: holding_created_at(holding) - } - elsif holding_shares(holding) > prev_shares(prev) - # Shares increased = BUY - changes << { - type: :buy, - symbol: holding_symbol(holding), - description: holding_description(holding), - shares_delta: holding_shares(holding) - prev_shares(prev), - cost_basis_delta: holding_cost_basis(holding) - prev_cost_basis(prev) - } - elsif holding_shares(holding) < prev_shares(prev) - # Shares decreased = SELL - changes << { - type: :sell, - symbol: holding_symbol(holding), - description: holding_description(holding), - shares_delta: prev_shares(prev) - holding_shares(holding) - } - end - end - - # Check for holdings that completely disappeared = SELL ALL - previous.each do |prev| - unless current.any? { |h| same_holding?(h, prev) } - changes << { - type: :sell, - symbol: prev_symbol(prev), - description: prev_description(prev), - shares: prev_shares(prev) - } - end - end - - changes - end - - def find_matching_entry(change, transactions) - transactions.each do |txn| - entry = txn.respond_to?(:entry) ? txn.entry : txn - next unless entry - next if entry.exclude_from_cashflow? # Already excluded - - # Match by cost_basis amount (for buys with known cost) - if change[:cost_basis].present? && change[:cost_basis].to_d > 0 - amount_diff = (entry.amount.to_d.abs - change[:cost_basis].to_d.abs).abs - return entry if amount_diff < 0.01 - end - - # Match by cost_basis delta (for additional buys) - if change[:cost_basis_delta].present? && change[:cost_basis_delta].to_d > 0 - amount_diff = (entry.amount.to_d.abs - change[:cost_basis_delta].to_d.abs).abs - return entry if amount_diff < 0.01 - end - - # Match by description containing security name/symbol - entry_desc = entry.name&.downcase || "" - - if change[:symbol].present? - return entry if entry_desc.include?(change[:symbol].downcase) - end - - if change[:description].present? - # Match first few words of description for fuzzy matching - desc_words = change[:description].downcase.split.first(3).join(" ") - return entry if desc_words.present? && entry_desc.include?(desc_words) - end - end - - nil - end - - def find_previous_holding(previous, current) - symbol = holding_symbol(current) - return previous.find { |p| prev_symbol(p) == symbol } if symbol.present? - - # Fallback to description matching if no symbol - desc = holding_description(current) - previous.find { |p| prev_description(p) == desc } if desc.present? - end - - def same_holding?(current, previous) - current_symbol = holding_symbol(current) - prev_sym = prev_symbol(previous) - - if current_symbol.present? && prev_sym.present? - current_symbol == prev_sym - else - holding_description(current) == prev_description(previous) - end - end - - def save_holdings_snapshot(holdings) - snapshot_data = holdings.map do |h| - { - "symbol" => holding_symbol(h), - "description" => holding_description(h), - "shares" => holding_shares(h).to_s, - "cost_basis" => holding_cost_basis(h).to_s, - "market_value" => holding_market_value(h).to_s - } - end - - @account.update!( - holdings_snapshot_data: snapshot_data, - holdings_snapshot_at: Time.current - ) - end - - # Normalize access - holdings could be AR objects or hashes from different providers - def holding_symbol(h) - h.try(:symbol) || h.try(:ticker) || h["symbol"] || h[:symbol] || h["ticker"] || h[:ticker] - end - - def holding_description(h) - h.try(:description) || h.try(:name) || h["description"] || h[:description] || h["name"] || h[:name] - end - - def holding_shares(h) - val = h.try(:shares) || h.try(:qty) || h["shares"] || h[:shares] || h["qty"] || h[:qty] - val.to_d - end - - def holding_cost_basis(h) - val = h.try(:cost_basis) || h["cost_basis"] || h[:cost_basis] - val.to_d - end - - def holding_market_value(h) - val = h.try(:market_value) || h.try(:amount) || h["market_value"] || h[:market_value] || h["amount"] || h[:amount] - val.to_d - end - - def holding_created_at(h) - h.try(:created_at) || h["created"] || h[:created] || h["created_at"] || h[:created_at] - end - - # Previous snapshot accessor methods (snapshot is always a hash) - def prev_symbol(p) - p["symbol"] || p[:symbol] - end - - def prev_description(p) - p["description"] || p[:description] - end - - def prev_shares(p) - (p["shares"] || p[:shares]).to_d - end - - def prev_cost_basis(p) - (p["cost_basis"] || p[:cost_basis]).to_d - end -end diff --git a/app/models/lunchflow_account/processor.rb b/app/models/lunchflow_account/processor.rb index d82301dbd..b9c6b2184 100644 --- a/app/models/lunchflow_account/processor.rb +++ b/app/models/lunchflow_account/processor.rb @@ -74,37 +74,10 @@ class LunchflowAccount::Processor return unless [ "Investment", "Crypto" ].include?(lunchflow_account.current_account&.accountable_type) LunchflowAccount::Investments::HoldingsProcessor.new(lunchflow_account).process - - # Detect and mark internal investment activity (fund swaps, reinvestments) - detect_internal_investment_activity rescue => e report_exception(e, "holdings") end - def detect_internal_investment_activity - account = lunchflow_account.current_account - return unless account&.investment? || account&.crypto? - - # Get current holdings from raw payload - current_holdings = lunchflow_account.raw_holdings_payload || [] - return if current_holdings.blank? - - # Get recent transactions (last 30 days to catch any we might have missed) - recent_transactions = account.entries - .joins("INNER JOIN transactions ON transactions.id = entries.entryable_id AND entries.entryable_type = 'Transaction'") - .where(date: 30.days.ago.to_date..Date.current) - .where(exclude_from_cashflow: false) - .map(&:entryable) - .compact - - InvestmentActivityDetector.new(account).detect_and_mark_internal_activity( - current_holdings, - recent_transactions - ) - rescue => e - Rails.logger.warn("InvestmentActivityDetector failed for Lunchflow account #{lunchflow_account.id}: #{e.message}") - end - def report_exception(error, context) Sentry.capture_exception(error) do |scope| scope.set_tags( diff --git a/app/models/plaid_account/processor.rb b/app/models/plaid_account/processor.rb index 3dc09c710..4faead9d9 100644 --- a/app/models/plaid_account/processor.rb +++ b/app/models/plaid_account/processor.rb @@ -103,51 +103,10 @@ class PlaidAccount::Processor def process_investments PlaidAccount::Investments::TransactionsProcessor.new(plaid_account, security_resolver: security_resolver).process PlaidAccount::Investments::HoldingsProcessor.new(plaid_account, security_resolver: security_resolver).process - - # Detect and mark internal investment activity (fund swaps, reinvestments) - # Note: Plaid already creates Trade entries for buy/sell, but this catches cash transactions - detect_internal_investment_activity rescue => e report_exception(e) end - def detect_internal_investment_activity - account = AccountProvider.find_by(provider: plaid_account)&.account - return unless account&.investment? || account&.crypto? - - # Get current holdings from raw payload - raw_holdings = plaid_account.raw_investments_payload&.dig("holdings") || [] - return if raw_holdings.blank? - - # Transform to common format - current_holdings = raw_holdings.map do |h| - response = security_resolver.resolve(plaid_security_id: h["security_id"]) - security = response.security - { - "symbol" => security&.ticker, - "description" => security&.name, - "shares" => h["quantity"], - "cost_basis" => h["cost_basis"], - "market_value" => h["institution_value"] - } - end - - # Get recent transactions (last 30 days to catch any we might have missed) - recent_transactions = account.entries - .joins("INNER JOIN transactions ON transactions.id = entries.entryable_id AND entries.entryable_type = 'Transaction'") - .where(date: 30.days.ago.to_date..Date.current) - .where(exclude_from_cashflow: false) - .map(&:entryable) - .compact - - InvestmentActivityDetector.new(account).detect_and_mark_internal_activity( - current_holdings, - recent_transactions - ) - rescue => e - Rails.logger.warn("InvestmentActivityDetector failed for Plaid account #{plaid_account.id}: #{e.message}") - end - def process_liabilities case [ plaid_account.plaid_type, plaid_account.plaid_subtype ] when [ "credit", "credit card" ] diff --git a/app/models/rule/action_executor/set_investment_activity_label.rb b/app/models/rule/action_executor/set_investment_activity_label.rb new file mode 100644 index 000000000..21e421292 --- /dev/null +++ b/app/models/rule/action_executor/set_investment_activity_label.rb @@ -0,0 +1,31 @@ +class Rule::ActionExecutor::SetInvestmentActivityLabel < Rule::ActionExecutor + def label + "Set investment activity label" + end + + def type + "select" + end + + def options + Transaction::ACTIVITY_LABELS.map { |l| [ l, l ] } + end + + def execute(transaction_scope, value: nil, ignore_attribute_locks: false, rule_run: nil) + return 0 unless Transaction::ACTIVITY_LABELS.include?(value) + + scope = transaction_scope + + unless ignore_attribute_locks + scope = scope.enrichable(:investment_activity_label) + end + + count_modified_resources(scope) do |txn| + txn.enrich_attribute( + :investment_activity_label, + value, + source: "rule" + ) + end + end +end diff --git a/app/models/rule/registry/transaction_resource.rb b/app/models/rule/registry/transaction_resource.rb index d051b5837..fac1d6667 100644 --- a/app/models/rule/registry/transaction_resource.rb +++ b/app/models/rule/registry/transaction_resource.rb @@ -20,6 +20,7 @@ class Rule::Registry::TransactionResource < Rule::Registry Rule::ActionExecutor::SetTransactionTags.new(rule), Rule::ActionExecutor::SetTransactionMerchant.new(rule), Rule::ActionExecutor::SetTransactionName.new(rule), + Rule::ActionExecutor::SetInvestmentActivityLabel.new(rule), Rule::ActionExecutor::ExcludeTransaction.new(rule) ] diff --git a/app/models/simplefin_account/processor.rb b/app/models/simplefin_account/processor.rb index 68b679772..569f515cd 100644 --- a/app/models/simplefin_account/processor.rb +++ b/app/models/simplefin_account/processor.rb @@ -151,37 +151,10 @@ class SimplefinAccount::Processor return unless simplefin_account.current_account&.accountable_type == "Investment" SimplefinAccount::Investments::TransactionsProcessor.new(simplefin_account).process SimplefinAccount::Investments::HoldingsProcessor.new(simplefin_account).process - - # Detect and mark internal investment activity (fund swaps, reinvestments) - detect_internal_investment_activity rescue => e report_exception(e, "investments") end - def detect_internal_investment_activity - account = simplefin_account.current_account - return unless account&.investment? || account&.crypto? - - # Get current holdings from raw payload - current_holdings = simplefin_account.raw_holdings_payload || [] - return if current_holdings.blank? - - # Get recent transactions (last 30 days to catch any we might have missed) - recent_transactions = account.entries - .joins("INNER JOIN transactions ON transactions.id = entries.entryable_id AND entries.entryable_type = 'Transaction'") - .where(date: 30.days.ago.to_date..Date.current) - .where(exclude_from_cashflow: false) - .map(&:entryable) - .compact - - InvestmentActivityDetector.new(account).detect_and_mark_internal_activity( - current_holdings, - recent_transactions - ) - rescue => e - Rails.logger.warn("InvestmentActivityDetector failed for account #{simplefin_account.current_account&.id}: #{e.message}") - end - def process_liabilities case simplefin_account.current_account&.accountable_type when "CreditCard" diff --git a/app/models/simplefin_entry/processor.rb b/app/models/simplefin_entry/processor.rb index 8e6cd88f5..db4b5689b 100644 --- a/app/models/simplefin_entry/processor.rb +++ b/app/models/simplefin_entry/processor.rb @@ -18,8 +18,7 @@ class SimplefinEntry::Processor source: "simplefin", merchant: merchant, notes: notes, - extra: extra_metadata, - investment_activity_label: inferred_activity_label + extra: extra_metadata ) end @@ -205,11 +204,4 @@ class SimplefinEntry::Processor end parts.presence&.join(" | ") end - - # Infer investment activity label from transaction description - # Only returns a label for investment/crypto accounts - def inferred_activity_label - return nil unless account&.investment? || account&.crypto? - InvestmentActivityDetector.infer_label_from_description(name, amount, account) - end end diff --git a/app/models/transaction.rb b/app/models/transaction.rb index 42e3a2677..d1f65a704 100644 --- a/app/models/transaction.rb +++ b/app/models/transaction.rb @@ -20,19 +20,12 @@ class Transaction < ApplicationRecord investment_contribution: "investment_contribution" # Transfer to investment/crypto account, included in budget as investment expense } - # Labels for internal investment activity (auto-exclude from cashflow) - # Only internal shuffling should be excluded, not contributions/dividends/withdrawals - INTERNAL_ACTIVITY_LABELS = %w[Buy Sell Reinvestment Exchange].freeze - # All valid investment activity labels (for UI dropdown) ACTIVITY_LABELS = [ "Buy", "Sell", "Sweep In", "Sweep Out", "Dividend", "Reinvestment", "Interest", "Fee", "Transfer", "Contribution", "Withdrawal", "Exchange", "Other" ].freeze - after_save :sync_exclude_from_cashflow_with_activity_label, - if: :saved_change_to_investment_activity_label? - # Pending transaction scopes - filter based on provider pending flags in extra JSONB # Works with any provider that stores pending status in extra["provider_name"]["pending"] scope :pending, -> { @@ -159,17 +152,4 @@ class Transaction < ApplicationRecord FamilyMerchantAssociation.where(family: family, merchant: merchant).delete_all end - - # Sync exclude_from_cashflow based on activity label - # Internal activities (Buy, Sell, etc.) should be excluded from cashflow - def sync_exclude_from_cashflow_with_activity_label - return unless entry&.account&.investment? || entry&.account&.crypto? - return if entry.locked?(:exclude_from_cashflow) # Respect user's manual setting - - should_exclude = INTERNAL_ACTIVITY_LABELS.include?(investment_activity_label) - - if entry.exclude_from_cashflow != should_exclude - entry.update!(exclude_from_cashflow: should_exclude) - end - end end diff --git a/lib/tasks/investment_labels.rake b/lib/tasks/investment_labels.rake deleted file mode 100644 index 9cefae7c9..000000000 --- a/lib/tasks/investment_labels.rake +++ /dev/null @@ -1,189 +0,0 @@ -# frozen_string_literal: true - -# Backfill investment activity labels for existing transactions -# -# Usage examples: -# # Preview (dry run) - show what labels would be set -# bin/rails 'sure:investments:backfill_labels[dry_run=true]' -# -# # Execute the backfill for all investment/crypto accounts -# bin/rails 'sure:investments:backfill_labels[dry_run=false]' -# -# # Backfill for a specific account -# bin/rails 'sure:investments:backfill_labels[account_id=8b46387c-5aa4-4a92-963a-4392c10999c9,dry_run=false]' -# -# # Force re-label already-labeled transactions -# bin/rails 'sure:investments:backfill_labels[dry_run=false,force=true]' - -namespace :sure do - namespace :investments do - desc "Backfill activity labels for existing investment transactions. Args: account_id (optional), dry_run=true, force=false" - task :backfill_labels, [ :account_id, :dry_run, :force ] => :environment do |_, args| - # Support named args (key=value) - parse all positional args for key=value pairs - kv = {} - [ args[:account_id], args[:dry_run], args[:force] ].each do |raw| - next unless raw.is_a?(String) && raw.include?("=") - k, v = raw.split("=", 2) - kv[k.to_s] = v - end - - # Only use positional args if they don't contain "=" (otherwise they're named args in wrong position) - positional_account_id = args[:account_id] unless args[:account_id].to_s.include?("=") - positional_dry_run = args[:dry_run] unless args[:dry_run].to_s.include?("=") - positional_force = args[:force] unless args[:force].to_s.include?("=") - - account_id = (kv["account_id"] || positional_account_id).presence - dry_raw = (kv["dry_run"] || positional_dry_run).to_s.downcase - force_raw = (kv["force"] || positional_force).to_s.downcase - force = %w[true yes 1].include?(force_raw) - - # Default to dry_run=true unless explicitly disabled - dry_run = if dry_raw.blank? - true - elsif %w[1 true yes y].include?(dry_raw) - true - elsif %w[0 false no n].include?(dry_raw) - false - else - puts({ ok: false, error: "invalid_argument", message: "dry_run must be one of: true/yes/1 or false/no/0" }.to_json) - exit 1 - end - - # Build account scope - accounts = if account_id.present? - Account.where(id: account_id) - else - Account.where(accountable_type: %w[Investment Crypto]) - end - - if accounts.none? - puts({ ok: false, error: "no_accounts", message: "No investment/crypto accounts found" }.to_json) - exit 1 - end - - total_processed = 0 - total_labeled = 0 - total_skipped = 0 - total_errors = 0 - - accounts.find_each do |account| - # Skip non-investment/crypto accounts if processing all - next unless account.investment? || account.crypto? - - acct_processed = 0 - acct_labeled = 0 - acct_skipped = 0 - acct_errors = 0 - - # Find transactions (optionally include already-labeled if force=true) - entries = account.entries - .joins("INNER JOIN transactions ON transactions.id = entries.entryable_id AND entries.entryable_type = 'Transaction'") - .includes(:entryable) - - unless force - entries = entries.where("transactions.investment_activity_label IS NULL OR transactions.investment_activity_label = ''") - end - - entries.find_each do |entry| - acct_processed += 1 - total_processed += 1 - - begin - transaction = entry.transaction - current_label = transaction.investment_activity_label - label = InvestmentActivityDetector.infer_label_from_description(entry.name, entry.amount, account) - - # Skip if no label can be inferred - if label.blank? - acct_skipped += 1 - total_skipped += 1 - next - end - - # Skip if label unchanged (when force=true) - if current_label == label - acct_skipped += 1 - total_skipped += 1 - next - end - - if dry_run - if current_label.present? - puts " [DRY RUN] Would relabel '#{entry.name}' (#{entry.amount}) from '#{current_label}' to '#{label}'" - else - puts " [DRY RUN] Would label '#{entry.name}' (#{entry.amount}) as '#{label}'" - end - else - transaction.update!(investment_activity_label: label) - if current_label.present? - puts " Relabeled '#{entry.name}' (#{entry.amount}) from '#{current_label}' to '#{label}'" - else - puts " Labeled '#{entry.name}' (#{entry.amount}) as '#{label}'" - end - end - acct_labeled += 1 - total_labeled += 1 - rescue => e - acct_errors += 1 - total_errors += 1 - puts({ error: e.class.name, message: e.message, entry_id: entry.id }.to_json) - end - end - - puts({ account_id: account.id, account_name: account.name, accountable_type: account.accountable_type, processed: acct_processed, labeled: acct_labeled, skipped: acct_skipped, errors: acct_errors, dry_run: dry_run, force: force }.to_json) - end - - puts({ ok: true, total_processed: total_processed, total_labeled: total_labeled, total_skipped: total_skipped, total_errors: total_errors, dry_run: dry_run }.to_json) - end - - desc "Clear all investment activity labels (for testing). Args: account_id (required), dry_run=true" - task :clear_labels, [ :account_id, :dry_run ] => :environment do |_, args| - kv = {} - [ args[:account_id], args[:dry_run] ].each do |raw| - next unless raw.is_a?(String) && raw.include?("=") - k, v = raw.split("=", 2) - kv[k.to_s] = v - end - - # Only use positional args if they don't contain "=" - positional_account_id = args[:account_id] unless args[:account_id].to_s.include?("=") - positional_dry_run = args[:dry_run] unless args[:dry_run].to_s.include?("=") - - account_id = (kv["account_id"] || positional_account_id).presence - dry_raw = (kv["dry_run"] || positional_dry_run).to_s.downcase - - unless account_id.present? - puts({ ok: false, error: "usage", message: "Provide account_id" }.to_json) - exit 1 - end - - dry_run = if dry_raw.blank? - true - elsif %w[1 true yes y].include?(dry_raw) - true - elsif %w[0 false no n].include?(dry_raw) - false - else - puts({ ok: false, error: "invalid_argument", message: "dry_run must be one of: true/yes/1 or false/no/0" }.to_json) - exit 1 - end - - account = Account.find(account_id) - - count = account.entries - .joins("INNER JOIN transactions ON transactions.id = entries.entryable_id AND entries.entryable_type = 'Transaction'") - .where("transactions.investment_activity_label IS NOT NULL AND transactions.investment_activity_label != ''") - .count - - if dry_run - puts({ ok: true, message: "Would clear #{count} labels", dry_run: true }.to_json) - else - Transaction.joins(:entry) - .where(entries: { account_id: account_id }) - .where("investment_activity_label IS NOT NULL AND investment_activity_label != ''") - .update_all(investment_activity_label: nil) - puts({ ok: true, message: "Cleared #{count} labels", dry_run: false }.to_json) - end - end - end -end diff --git a/test/models/investment_activity_detector_test.rb b/test/models/investment_activity_detector_test.rb deleted file mode 100644 index 68ea44ac2..000000000 --- a/test/models/investment_activity_detector_test.rb +++ /dev/null @@ -1,299 +0,0 @@ -require "test_helper" - -class InvestmentActivityDetectorTest < ActiveSupport::TestCase - include EntriesTestHelper - - setup do - @family = families(:empty) - @investment_account = @family.accounts.create!( - name: "Brokerage", - balance: 10000, - cash_balance: 2000, - currency: "USD", - accountable: Investment.new - ) - @detector = InvestmentActivityDetector.new(@investment_account) - end - - test "detects new holding purchase and marks matching transaction" do - # Create a transaction that matches a new holding purchase - entry = create_transaction( - account: @investment_account, - amount: 1000, - name: "Buy VFIAX" - ) - transaction = entry.transaction - - # Simulate holdings snapshot showing a new holding - current_holdings = [ - { "symbol" => "VFIAX", "cost_basis" => 1000.0, "shares" => 10 } - ] - - # No previous snapshot - @investment_account.update!(holdings_snapshot_data: nil, holdings_snapshot_at: nil) - - @detector.detect_and_mark_internal_activity(current_holdings, [ transaction ]) - - entry.reload - assert entry.exclude_from_cashflow?, "Transaction matching new holding should be excluded from cashflow" - end - - test "detects holding sale and marks matching transaction" do - # Set up previous holdings - previous_holdings = [ - { "symbol" => "VFIAX", "cost_basis" => 2000.0, "shares" => 20 } - ] - @investment_account.update!( - holdings_snapshot_data: previous_holdings, - holdings_snapshot_at: 1.day.ago - ) - - # Create a transaction for the sale proceeds (negative = inflow) - entry = create_transaction( - account: @investment_account, - amount: -1000, - name: "Sell VFIAX" - ) - transaction = entry.transaction - - # Current holdings show reduced position - current_holdings = [ - { "symbol" => "VFIAX", "cost_basis" => 1000.0, "shares" => 10 } - ] - - @detector.detect_and_mark_internal_activity(current_holdings, [ transaction ]) - - entry.reload - assert entry.exclude_from_cashflow?, "Transaction matching holding sale should be excluded from cashflow" - end - - test "respects locked exclude_from_cashflow attribute" do - # Create a transaction and lock the attribute - entry = create_transaction( - account: @investment_account, - amount: 1000, - name: "Buy VFIAX" - ) - transaction = entry.transaction - - # User explicitly set to NOT exclude (and locked it) - entry.update!(exclude_from_cashflow: false) - entry.lock_attr!(:exclude_from_cashflow) - - current_holdings = [ - { "symbol" => "VFIAX", "cost_basis" => 1000.0, "shares" => 10 } - ] - - @detector.detect_and_mark_internal_activity(current_holdings, [ transaction ]) - - entry.reload - assert_not entry.exclude_from_cashflow?, "Locked attribute should not be overwritten" - end - - test "updates holdings snapshot after detection" do - current_holdings = [ - { "symbol" => "VFIAX", "cost_basis" => 1000.0, "shares" => 10 }, - { "symbol" => "IBIT", "cost_basis" => 500.0, "shares" => 5 } - ] - - @detector.detect_and_mark_internal_activity(current_holdings, []) - - @investment_account.reload - # Snapshot is normalized with string values and additional fields - snapshot = @investment_account.holdings_snapshot_data - assert_equal 2, snapshot.size - assert_equal "VFIAX", snapshot[0]["symbol"] - assert_equal "1000.0", snapshot[0]["cost_basis"] - assert_equal "10.0", snapshot[0]["shares"] - assert_equal "IBIT", snapshot[1]["symbol"] - assert_not_nil @investment_account.holdings_snapshot_at - end - - test "matches transaction by cost_basis amount within tolerance" do - entry = create_transaction( - account: @investment_account, - amount: 1000.005, # Very close - within 0.01 tolerance - name: "Investment purchase" - ) - transaction = entry.transaction - - # Holding with cost basis close to transaction amount (within 0.01) - current_holdings = [ - { "symbol" => "VFIAX", "cost_basis" => 1000.0, "shares" => 10 } - ] - - @detector.detect_and_mark_internal_activity(current_holdings, [ transaction ]) - - entry.reload - assert entry.exclude_from_cashflow?, "Should match transaction within tolerance" - end - - test "does not mark unrelated transactions" do - # Create a regular expense transaction - entry = create_transaction( - account: @investment_account, - amount: 50, - name: "Account fee" - ) - transaction = entry.transaction - - # Holdings that don't match - current_holdings = [ - { "symbol" => "VFIAX", "cost_basis" => 1000.0, "shares" => 10 } - ] - - @detector.detect_and_mark_internal_activity(current_holdings, [ transaction ]) - - entry.reload - assert_not entry.exclude_from_cashflow?, "Unrelated transaction should not be excluded" - end - - test "works with crypto accounts" do - crypto_account = @family.accounts.create!( - name: "Crypto Wallet", - balance: 5000, - currency: "USD", - accountable: Crypto.new - ) - detector = InvestmentActivityDetector.new(crypto_account) - - entry = create_transaction( - account: crypto_account, - amount: 1000, - name: "Buy BTC" - ) - transaction = entry.transaction - - current_holdings = [ - { "symbol" => "BTC", "cost_basis" => 1000.0, "shares" => 0.02 } - ] - - detector.detect_and_mark_internal_activity(current_holdings, [ transaction ]) - - entry.reload - assert entry.exclude_from_cashflow?, "Should work with crypto accounts" - end - - test "handles empty holdings gracefully" do - entry = create_transaction( - account: @investment_account, - amount: 1000, - name: "Some transaction" - ) - transaction = entry.transaction - - # Should not raise, just do nothing - assert_nothing_raised do - @detector.detect_and_mark_internal_activity([], [ transaction ]) - end - - entry.reload - assert_not entry.exclude_from_cashflow? - end - - test "handles nil holdings gracefully" do - entry = create_transaction( - account: @investment_account, - amount: 1000, - name: "Some transaction" - ) - transaction = entry.transaction - - assert_nothing_raised do - @detector.detect_and_mark_internal_activity(nil, [ transaction ]) - end - - entry.reload - assert_not entry.exclude_from_cashflow? - end - - test "sets Buy label for new holding purchase" do - entry = create_transaction( - account: @investment_account, - amount: 1000, - name: "Some investment" - ) - transaction = entry.transaction - - current_holdings = [ - { "symbol" => "VFIAX", "cost_basis" => 1000.0, "shares" => 10 } - ] - - @detector.detect_and_mark_internal_activity(current_holdings, [ transaction ]) - - transaction.reload - assert_equal "Buy", transaction.investment_activity_label - end - - test "sets Sell label for holding sale" do - previous_holdings = [ - { "symbol" => "VFIAX", "cost_basis" => 2000.0, "shares" => 20 } - ] - @investment_account.update!( - holdings_snapshot_data: previous_holdings, - holdings_snapshot_at: 1.day.ago - ) - - entry = create_transaction( - account: @investment_account, - amount: -1000, - name: "VFIAX Sale" - ) - transaction = entry.transaction - - current_holdings = [ - { "symbol" => "VFIAX", "cost_basis" => 1000.0, "shares" => 10 } - ] - - @detector.detect_and_mark_internal_activity(current_holdings, [ transaction ]) - - transaction.reload - assert_equal "Sell", transaction.investment_activity_label - end - - test "infers Sweep In label from money market description" do - entry = create_transaction( - account: @investment_account, - amount: -500, - name: "VANGUARD FEDERAL MONEY MARKET" - ) - transaction = entry.transaction - - # Call with empty holdings but simulate it being a sweep - # This tests the infer_from_description fallback - current_holdings = [ - { "symbol" => "VMFXX", "cost_basis" => 500.0, "shares" => 500 } - ] - - @detector.detect_and_mark_internal_activity(current_holdings, [ transaction ]) - - transaction.reload - # Should be either "Buy" (from holdings match) or "Sweep In" (from description) - assert transaction.investment_activity_label.present? - end - - test "infers Dividend label from CASH description" do - entry = create_transaction( - account: @investment_account, - amount: -50, - name: "CASH" - ) - transaction = entry.transaction - - # No holdings change, but description-based inference - current_holdings = [ - { "symbol" => "VFIAX", "cost_basis" => 1000.0, "shares" => 10 } - ] - @investment_account.update!( - holdings_snapshot_data: current_holdings, - holdings_snapshot_at: 1.day.ago - ) - - @detector.detect_and_mark_internal_activity(current_holdings, [ transaction ]) - - # Since there's no holdings change, no label gets set via holdings match - # But if we manually test the infer_from_description method... - label = @detector.send(:infer_from_description, entry) - assert_equal "Dividend", label - end -end diff --git a/test/models/rule/action_test.rb b/test/models/rule/action_test.rb index db3933ea0..bcb8846af 100644 --- a/test/models/rule/action_test.rb +++ b/test/models/rule/action_test.rb @@ -100,4 +100,36 @@ class Rule::ActionTest < ActiveSupport::TestCase assert_equal new_name, transaction.reload.entry.name end end + + test "set_investment_activity_label" do + # Does not modify transactions that are locked (user edited them) + @txn1.lock_attr!(:investment_activity_label) + + action = Rule::Action.new( + rule: @transaction_rule, + action_type: "set_investment_activity_label", + value: "Dividend" + ) + + action.apply(@rule_scope) + + assert_nil @txn1.reload.investment_activity_label + + [ @txn2, @txn3 ].each do |transaction| + assert_equal "Dividend", transaction.reload.investment_activity_label + end + end + + test "set_investment_activity_label ignores invalid values" do + action = Rule::Action.new( + rule: @transaction_rule, + action_type: "set_investment_activity_label", + value: "InvalidLabel" + ) + + result = action.apply(@rule_scope) + + assert_equal 0, result + assert_nil @txn1.reload.investment_activity_label + end end diff --git a/test/models/transaction_test.rb b/test/models/transaction_test.rb index 3d7b3236b..3f0f903b0 100644 --- a/test/models/transaction_test.rb +++ b/test/models/transaction_test.rb @@ -35,13 +35,6 @@ class TransactionTest < ActiveSupport::TestCase end end - test "INTERNAL_ACTIVITY_LABELS contains expected labels" do - assert_includes Transaction::INTERNAL_ACTIVITY_LABELS, "Buy" - assert_includes Transaction::INTERNAL_ACTIVITY_LABELS, "Sell" - assert_includes Transaction::INTERNAL_ACTIVITY_LABELS, "Reinvestment" - assert_includes Transaction::INTERNAL_ACTIVITY_LABELS, "Exchange" - end - test "ACTIVITY_LABELS contains all valid labels" do assert_includes Transaction::ACTIVITY_LABELS, "Buy" assert_includes Transaction::ACTIVITY_LABELS, "Sell" From 582eda999ba93d94cc02e867fa55d639d941bac1 Mon Sep 17 00:00:00 2001 From: Josh Waldrep Date: Mon, 12 Jan 2026 15:10:33 -0500 Subject: [PATCH 07/10] Remove `exclude_from_cashflow` flag and consolidate logic into `excluded` toggle - Removed `exclude_from_cashflow` attribute across models, controllers, and views. - Updated queries to rely solely on the `excluded` flag for filtering transactions and entries. - Simplified migration by consolidating `exclude_from_cashflow` functionality into the existing `excluded` toggle. - Refactored related tests to remove outdated logic and ensured compatibility with the updated implementation. --- app/controllers/accounts_controller.rb | 2 +- app/controllers/reports_controller.rb | 10 ++-- app/controllers/transactions_controller.rb | 2 +- app/models/income_statement/category_stats.rb | 1 - app/models/income_statement/family_stats.rb | 1 - app/models/income_statement/totals.rb | 9 ++-- app/models/transaction/search.rb | 4 +- app/views/transactions/_transaction.html.erb | 6 --- app/views/transactions/show.html.erb | 22 --------- config/locales/views/transactions/en.yml | 4 -- ...0120000_add_investment_cashflow_support.rb | 10 +--- test/models/income_statement_test.rb | 48 ------------------- 12 files changed, 13 insertions(+), 106 deletions(-) diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index b71318b62..1524b25d0 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -36,7 +36,7 @@ class AccountsController < ApplicationController @chart_view = params[:chart_view] || "balance" @tab = params[:tab] @q = params.fetch(:q, {}).permit(:search, status: []) - entries = @account.entries.where(excluded: false).search(@q).reverse_chronological + entries = @account.entries.search(@q).reverse_chronological @pagy, @entries = pagy(entries, limit: params[:per_page] || "10") diff --git a/app/controllers/reports_controller.rb b/app/controllers/reports_controller.rb index 842ff50e8..91593f801 100644 --- a/app/controllers/reports_controller.rb +++ b/app/controllers/reports_controller.rb @@ -338,7 +338,7 @@ class ReportsController < ApplicationController .joins(:entry) .joins(entry: :account) .where(accounts: { family_id: Current.family.id, status: [ "draft", "active" ] }) - .where(entries: { entryable_type: "Transaction", excluded: false, exclude_from_cashflow: false, date: @period.date_range }) + .where(entries: { entryable_type: "Transaction", excluded: false, date: @period.date_range }) .where.not(kind: [ "funds_movement", "one_time", "cc_payment" ]) .includes(entry: :account, category: []) @@ -350,7 +350,7 @@ class ReportsController < ApplicationController .joins(:entry) .joins(entry: :account) .where(accounts: { family_id: Current.family.id, status: [ "draft", "active" ] }) - .where(entries: { entryable_type: "Trade", excluded: false, exclude_from_cashflow: false, date: @period.date_range }) + .where(entries: { entryable_type: "Trade", excluded: false, date: @period.date_range }) .includes(entry: :account, category: []) # Get sort parameters @@ -519,7 +519,7 @@ class ReportsController < ApplicationController .joins(:entry) .joins(entry: :account) .where(accounts: { family_id: Current.family.id, status: [ "draft", "active" ] }) - .where(entries: { entryable_type: "Transaction", excluded: false, exclude_from_cashflow: false, date: @period.date_range }) + .where(entries: { entryable_type: "Transaction", excluded: false, date: @period.date_range }) .where.not(kind: [ "funds_movement", "one_time", "cc_payment" ]) .includes(entry: :account, category: []) @@ -556,7 +556,7 @@ class ReportsController < ApplicationController .joins(:entry) .joins(entry: :account) .where(accounts: { family_id: Current.family.id, status: [ "draft", "active" ] }) - .where(entries: { entryable_type: "Transaction", excluded: false, exclude_from_cashflow: false, date: @period.date_range }) + .where(entries: { entryable_type: "Transaction", excluded: false, date: @period.date_range }) .where.not(kind: [ "funds_movement", "one_time", "cc_payment" ]) .includes(entry: :account, category: []) @@ -567,7 +567,7 @@ class ReportsController < ApplicationController .joins(:entry) .joins(entry: :account) .where(accounts: { family_id: Current.family.id, status: [ "draft", "active" ] }) - .where(entries: { entryable_type: "Trade", excluded: false, exclude_from_cashflow: false, date: @period.date_range }) + .where(entries: { entryable_type: "Trade", excluded: false, date: @period.date_range }) .includes(entry: :account, category: []) # Group by category, type, and month diff --git a/app/controllers/transactions_controller.rb b/app/controllers/transactions_controller.rb index 03dcbc2db..753b77300 100644 --- a/app/controllers/transactions_controller.rb +++ b/app/controllers/transactions_controller.rb @@ -217,7 +217,7 @@ class TransactionsController < ApplicationController def entry_params entry_params = params.require(:entry).permit( - :name, :date, :amount, :currency, :excluded, :exclude_from_cashflow, :notes, :nature, :entryable_type, + :name, :date, :amount, :currency, :excluded, :notes, :nature, :entryable_type, entryable_attributes: [ :id, :category_id, :merchant_id, :kind, :investment_activity_label, { tag_ids: [] } ] ) diff --git a/app/models/income_statement/category_stats.rb b/app/models/income_statement/category_stats.rb index 31679a662..f4eb2815f 100644 --- a/app/models/income_statement/category_stats.rb +++ b/app/models/income_statement/category_stats.rb @@ -49,7 +49,6 @@ class IncomeStatement::CategoryStats WHERE a.family_id = :family_id AND t.kind NOT IN ('funds_movement', 'one_time', 'cc_payment', 'investment_contribution') AND ae.excluded = false - AND ae.exclude_from_cashflow = false AND (t.extra -> 'simplefin' ->> 'pending')::boolean IS DISTINCT FROM true AND (t.extra -> 'plaid' ->> 'pending')::boolean IS DISTINCT FROM true GROUP BY c.id, period, CASE WHEN ae.amount < 0 THEN 'income' ELSE 'expense' END diff --git a/app/models/income_statement/family_stats.rb b/app/models/income_statement/family_stats.rb index c96f13b35..48b0d9507 100644 --- a/app/models/income_statement/family_stats.rb +++ b/app/models/income_statement/family_stats.rb @@ -46,7 +46,6 @@ class IncomeStatement::FamilyStats WHERE a.family_id = :family_id AND t.kind NOT IN ('funds_movement', 'one_time', 'cc_payment', 'investment_contribution') AND ae.excluded = false - AND ae.exclude_from_cashflow = false AND (t.extra -> 'simplefin' ->> 'pending')::boolean IS DISTINCT FROM true AND (t.extra -> 'plaid' ->> 'pending')::boolean IS DISTINCT FROM true GROUP BY period, CASE WHEN ae.amount < 0 THEN 'income' ELSE 'expense' END diff --git a/app/models/income_statement/totals.rb b/app/models/income_statement/totals.rb index 2b5bd07b7..758ae6be3 100644 --- a/app/models/income_statement/totals.rb +++ b/app/models/income_statement/totals.rb @@ -71,8 +71,7 @@ class IncomeStatement::Totals ) WHERE at.kind NOT IN ('funds_movement', 'one_time', 'cc_payment', 'investment_contribution') AND ae.excluded = false - AND ae.exclude_from_cashflow = false - AND a.family_id = :family_id + AND a.family_id = :family_id AND a.status IN ('draft', 'active') GROUP BY c.id, c.parent_id, CASE WHEN ae.amount < 0 THEN 'income' ELSE 'expense' END; SQL @@ -98,8 +97,7 @@ class IncomeStatement::Totals ) WHERE at.kind NOT IN ('funds_movement', 'one_time', 'cc_payment', 'investment_contribution') AND ae.excluded = false - AND ae.exclude_from_cashflow = false - AND a.family_id = :family_id + AND a.family_id = :family_id AND a.status IN ('draft', 'active') GROUP BY c.id, c.parent_id, CASE WHEN ae.amount < 0 THEN 'income' ELSE 'expense' END SQL @@ -128,8 +126,7 @@ class IncomeStatement::Totals WHERE a.family_id = :family_id AND a.status IN ('draft', 'active') AND ae.excluded = false - AND ae.exclude_from_cashflow = false - AND ae.date BETWEEN :start_date AND :end_date + AND ae.date BETWEEN :start_date AND :end_date GROUP BY c.id, c.parent_id, CASE WHEN ae.amount < 0 THEN 'income' ELSE 'expense' END, CASE WHEN t.category_id IS NULL THEN true ELSE false END SQL end diff --git a/app/models/transaction/search.rb b/app/models/transaction/search.rb index 3ece18e45..e46a66472 100644 --- a/app/models/transaction/search.rb +++ b/app/models/transaction/search.rb @@ -49,8 +49,8 @@ class Transaction::Search Rails.cache.fetch("transaction_search_totals/#{cache_key_base}") do result = transactions_scope .select( - "COALESCE(SUM(CASE WHEN entries.amount >= 0 AND transactions.kind NOT IN ('funds_movement', 'cc_payment', 'investment_contribution') AND entries.exclude_from_cashflow = false THEN ABS(entries.amount * COALESCE(er.rate, 1)) ELSE 0 END), 0) as expense_total", - "COALESCE(SUM(CASE WHEN entries.amount < 0 AND transactions.kind NOT IN ('funds_movement', 'cc_payment', 'investment_contribution') AND entries.exclude_from_cashflow = false THEN ABS(entries.amount * COALESCE(er.rate, 1)) ELSE 0 END), 0) as income_total", + "COALESCE(SUM(CASE WHEN entries.amount >= 0 AND transactions.kind NOT IN ('funds_movement', 'cc_payment', 'investment_contribution') THEN ABS(entries.amount * COALESCE(er.rate, 1)) ELSE 0 END), 0) as expense_total", + "COALESCE(SUM(CASE WHEN entries.amount < 0 AND transactions.kind NOT IN ('funds_movement', 'cc_payment', 'investment_contribution') THEN ABS(entries.amount * COALESCE(er.rate, 1)) ELSE 0 END), 0) as income_total", "COUNT(entries.id) as transactions_count" ) .joins( diff --git a/app/views/transactions/_transaction.html.erb b/app/views/transactions/_transaction.html.erb index c645d515d..9c52d6acd 100644 --- a/app/views/transactions/_transaction.html.erb +++ b/app/views/transactions/_transaction.html.erb @@ -78,12 +78,6 @@ <% end %> - <% if entry.exclude_from_cashflow? %> - - <%= icon "eye-off", size: "sm", color: "current" %> - - <% end %> - <%# Investment activity label badge %> <% if transaction.investment_activity_label.present? %> "> diff --git a/app/views/transactions/show.html.erb b/app/views/transactions/show.html.erb index 4af991f1a..3d8910dcb 100644 --- a/app/views/transactions/show.html.erb +++ b/app/views/transactions/show.html.erb @@ -203,28 +203,6 @@ <% end %>
-
- <%= styled_form_with model: @entry, - url: transaction_path(@entry), - class: "p-3", - data: { controller: "auto-submit-form" } do |f| %> -
-
-

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

-

- <% if @entry.account.investment? || @entry.account.crypto? %> - <%= t(".exclude_from_cashflow_description_investment") %> - <% else %> - <%= t(".exclude_from_cashflow_description") %> - <% end %> -

-
- - <%= f.toggle :exclude_from_cashflow, { data: { auto_submit_form_target: "auto" } } %> -
- <% end %> -
- <% if @entry.account.investment? || @entry.account.crypto? %>
<%= styled_form_with model: @entry, diff --git a/config/locales/views/transactions/en.yml b/config/locales/views/transactions/en.yml index 267adce43..3b295b2f4 100644 --- a/config/locales/views/transactions/en.yml +++ b/config/locales/views/transactions/en.yml @@ -33,9 +33,6 @@ en: details: Details exclude: Exclude exclude_description: Excluded transactions will be removed from budgeting calculations and reports. - exclude_from_cashflow: Exclude from Cashflow - exclude_from_cashflow_description: Hide from income/expense reports and Sankey chart. Useful for transactions you don't want in cashflow analysis. - exclude_from_cashflow_description_investment: Hide from income/expense reports and Sankey chart. Use for internal investment activity like fund swaps, reinvestments, or money market sweeps. activity_type: Activity Type activity_type_description: Type of investment activity (Buy, Sell, Dividend, etc.). Auto-detected or set manually. one_time_title: One-time %{type} @@ -76,7 +73,6 @@ en: transaction: pending: Pending pending_tooltip: Pending transaction — may change when posted - excluded_from_cashflow_tooltip: Excluded from cashflow reports activity_type_tooltip: Investment activity type possible_duplicate: Duplicate? potential_duplicate_tooltip: This may be a duplicate of another transaction diff --git a/db/migrate/20260110120000_add_investment_cashflow_support.rb b/db/migrate/20260110120000_add_investment_cashflow_support.rb index 26cffbc55..39765f4f1 100644 --- a/db/migrate/20260110120000_add_investment_cashflow_support.rb +++ b/db/migrate/20260110120000_add_investment_cashflow_support.rb @@ -1,13 +1,5 @@ class AddInvestmentCashflowSupport < ActiveRecord::Migration[7.2] + # No-op: exclude_from_cashflow was consolidated into the existing 'excluded' toggle def change - # Flag for excluding from cashflow (user-controllable) - # Used for internal investment activity like fund swaps - add_column :entries, :exclude_from_cashflow, :boolean, default: false, null: false - add_index :entries, :exclude_from_cashflow - - # Holdings snapshot for comparison (provider-agnostic) - # Used to detect internal investment activity by comparing holdings between syncs - add_column :accounts, :holdings_snapshot_data, :jsonb - add_column :accounts, :holdings_snapshot_at, :datetime end end diff --git a/test/models/income_statement_test.rb b/test/models/income_statement_test.rb index ba5fa3e81..e4af61d66 100644 --- a/test/models/income_statement_test.rb +++ b/test/models/income_statement_test.rb @@ -286,35 +286,6 @@ class IncomeStatementTest < ActiveSupport::TestCase assert_equal Money.new(1050, @family.currency), totals.expense_money # 900 + 150 end - # NEW TESTS: exclude_from_cashflow Feature - test "excludes transactions with exclude_from_cashflow flag from totals" do - # Create an expense transaction and mark it as excluded from cashflow - excluded_entry = create_transaction(account: @checking_account, amount: 250, category: @groceries_category) - excluded_entry.update!(exclude_from_cashflow: true) - - income_statement = IncomeStatement.new(@family) - totals = income_statement.totals(date_range: Period.last_30_days.date_range) - - # Should NOT include the excluded transaction - assert_equal 4, totals.transactions_count # Only original 4 transactions - assert_equal Money.new(1000, @family.currency), totals.income_money - assert_equal Money.new(900, @family.currency), totals.expense_money - end - - test "excludes income transactions with exclude_from_cashflow flag" do - # Create income and mark as excluded from cashflow - excluded_income = create_transaction(account: @checking_account, amount: -500, category: @income_category) - excluded_income.update!(exclude_from_cashflow: true) - - income_statement = IncomeStatement.new(@family) - totals = income_statement.totals(date_range: Period.last_30_days.date_range) - - # Should NOT include the excluded income - assert_equal 4, totals.transactions_count - assert_equal Money.new(1000, @family.currency), totals.income_money # Original income only - assert_equal Money.new(900, @family.currency), totals.expense_money - end - test "excludes investment_contribution transactions from income statement" do # Create a transfer to investment account (marked as investment_contribution) investment_contribution = create_transaction( @@ -332,23 +303,4 @@ class IncomeStatementTest < ActiveSupport::TestCase assert_equal Money.new(1000, @family.currency), totals.income_money assert_equal Money.new(900, @family.currency), totals.expense_money end - - test "exclude_from_cashflow works with median calculations" do - # Clear existing transactions - Entry.joins(:account).where(accounts: { family_id: @family.id }).destroy_all - - # Create expenses: 100, 200, 300 - create_transaction(account: @checking_account, amount: 100, category: @groceries_category) - create_transaction(account: @checking_account, amount: 200, category: @groceries_category) - excluded_entry = create_transaction(account: @checking_account, amount: 300, category: @groceries_category) - - # Exclude the 300 transaction from cashflow - excluded_entry.update!(exclude_from_cashflow: true) - - income_statement = IncomeStatement.new(@family) - - # Median should only consider non-excluded transactions (100, 200) - # Monthly total = 300, so median = 300.0 - assert_equal 300.0, income_statement.median_expense(interval: "month") - end end From 2f20d715a492e4e18cec8084a79fc4a961c59e61 Mon Sep 17 00:00:00 2001 From: Josh Waldrep Date: Mon, 12 Jan 2026 15:26:17 -0500 Subject: [PATCH 08/10] Remove exclude_from_cashflow column from schema This column was never meant to be added - the migration is a no-op. The schema.rb was incorrectly committed with this column during rebase. Co-Authored-By: Claude Opus 4.5 --- db/schema.rb | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 51 insertions(+), 2 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index 067d2c9f6..ad548839c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -342,14 +342,12 @@ ActiveRecord::Schema[7.2].define(version: 2026_01_12_065106) do t.jsonb "locked_attributes", default: {} t.string "external_id" t.string "source" - t.boolean "exclude_from_cashflow", default: false, null: false 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", "source", "external_id"], name: "index_entries_on_account_source_and_external_id", unique: true, where: "((external_id IS NOT NULL) AND (source IS NOT NULL))" 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 ["exclude_from_cashflow"], name: "index_entries_on_exclude_from_cashflow" t.index ["import_id"], name: "index_entries_on_import_id" end @@ -476,6 +474,22 @@ ActiveRecord::Schema[7.2].define(version: 2026_01_12_065106) do t.index ["merchant_id"], name: "index_family_merchant_associations_on_merchant_id" end + create_table "flipper_features", force: :cascade do |t| + t.string "key", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["key"], name: "index_flipper_features_on_key", unique: true + end + + create_table "flipper_gates", force: :cascade do |t| + t.string "feature_key", null: false + t.string "key", null: false + t.text "value" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["feature_key", "key", "value"], name: "index_flipper_gates_on_feature_key_and_key_and_value", unique: true + end + create_table "holdings", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| t.uuid "account_id", null: false t.uuid "security_id", null: false @@ -800,6 +814,8 @@ ActiveRecord::Schema[7.2].define(version: 2026_01_12_065106) do t.datetime "last_authenticated_at" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.string "issuer" + t.index ["issuer"], name: "index_oidc_identities_on_issuer" t.index ["provider", "uid"], name: "index_oidc_identities_on_provider_and_uid", unique: true t.index ["user_id"], name: "index_oidc_identities_on_user_id" end @@ -1055,6 +1071,38 @@ ActiveRecord::Schema[7.2].define(version: 2026_01_12_065106) do t.index ["status"], name: "index_simplefin_items_on_status" end + create_table "sso_audit_logs", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| + t.uuid "user_id" + t.string "event_type", null: false + t.string "provider" + t.string "ip_address" + t.string "user_agent" + t.jsonb "metadata", default: {}, null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["created_at"], name: "index_sso_audit_logs_on_created_at" + t.index ["event_type"], name: "index_sso_audit_logs_on_event_type" + t.index ["user_id", "created_at"], name: "index_sso_audit_logs_on_user_id_and_created_at" + t.index ["user_id"], name: "index_sso_audit_logs_on_user_id" + end + + create_table "sso_providers", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| + t.string "strategy", null: false + t.string "name", null: false + t.string "label", null: false + t.string "icon" + t.boolean "enabled", default: true, null: false + t.string "issuer" + t.string "client_id" + t.string "client_secret" + t.string "redirect_uri" + t.jsonb "settings", default: {}, null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["enabled"], name: "index_sso_providers_on_enabled" + t.index ["name"], name: "index_sso_providers_on_name", unique: true + end + create_table "subscriptions", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| t.uuid "family_id", null: false t.string "status", null: false @@ -1289,6 +1337,7 @@ ActiveRecord::Schema[7.2].define(version: 2026_01_12_065106) do add_foreign_key "sessions", "users" add_foreign_key "simplefin_accounts", "simplefin_items" add_foreign_key "simplefin_items", "families" + add_foreign_key "sso_audit_logs", "users" add_foreign_key "subscriptions", "families" add_foreign_key "syncs", "syncs", column: "parent_id" add_foreign_key "taggings", "tags" From 308a4ab048d036d70756251202cfce6cf118e8e1 Mon Sep 17 00:00:00 2001 From: Josh Waldrep Date: Mon, 12 Jan 2026 16:04:53 -0500 Subject: [PATCH 09/10] Refactor Plaid transaction type mapping and improve label handling - Updated `PLAID_TYPE_TO_LABEL` in `TransactionsProcessor` to consolidate labels ("Cancel" and "Cash" now mapped to "Other"). - Adjusted `label_from_plaid_type` to return "Other" as the default fallback. - Enhanced tests to include additional valid activity labels and ensure label consistency. - Minor fixes to locale keys for transaction views. --- .../investments/transactions_processor.rb | 7 ++-- app/models/transaction.rb | 2 +- config/locales/views/transactions/en.yml | 35 +++++++++---------- test/models/transaction_test.rb | 6 ++++ 4 files changed, 28 insertions(+), 22 deletions(-) diff --git a/app/models/plaid_account/investments/transactions_processor.rb b/app/models/plaid_account/investments/transactions_processor.rb index ded2a473f..d90657ce6 100644 --- a/app/models/plaid_account/investments/transactions_processor.rb +++ b/app/models/plaid_account/investments/transactions_processor.rb @@ -2,11 +2,12 @@ class PlaidAccount::Investments::TransactionsProcessor SecurityNotFoundError = Class.new(StandardError) # Map Plaid investment transaction types to activity labels + # All values must be valid Transaction::ACTIVITY_LABELS PLAID_TYPE_TO_LABEL = { "buy" => "Buy", "sell" => "Sell", - "cancel" => "Cancelled", - "cash" => "Cash", + "cancel" => "Other", + "cash" => "Other", "fee" => "Fee", "transfer" => "Transfer", "dividend" => "Dividend", @@ -92,7 +93,7 @@ class PlaidAccount::Investments::TransactionsProcessor def label_from_plaid_type(transaction) plaid_type = transaction["type"]&.downcase - PLAID_TYPE_TO_LABEL[plaid_type] || plaid_type&.titleize + PLAID_TYPE_TO_LABEL[plaid_type] || "Other" end def transactions diff --git a/app/models/transaction.rb b/app/models/transaction.rb index d1f65a704..e1a86afb5 100644 --- a/app/models/transaction.rb +++ b/app/models/transaction.rb @@ -17,7 +17,7 @@ class Transaction < ApplicationRecord cc_payment: "cc_payment", # A CC payment, excluded from budget analytics (CC payments offset the sum of expense transactions) loan_payment: "loan_payment", # A payment to a Loan account, treated as an expense in budgets one_time: "one_time", # A one-time expense/income, excluded from budget analytics - investment_contribution: "investment_contribution" # Transfer to investment/crypto account, included in budget as investment expense + investment_contribution: "investment_contribution" # Transfer to investment/crypto account, excluded from budget analytics } # All valid investment activity labels (for UI dropdown) diff --git a/config/locales/views/transactions/en.yml b/config/locales/views/transactions/en.yml index 3b295b2f4..a53c28d85 100644 --- a/config/locales/views/transactions/en.yml +++ b/config/locales/views/transactions/en.yml @@ -51,25 +51,24 @@ en: withdrawal: Withdrawal exchange: Exchange other: Other - mark_recurring: Mark as Recurring - mark_recurring_subtitle: Track this as a recurring transaction. Amount variance is automatically calculated from past 6 months of similar transactions. - mark_recurring_title: Recurring Transaction - merchant_label: Merchant - name_label: Name - nature: Type - none: "(none)" - note_label: Notes - note_placeholder: Enter a note - overview: Overview - settings: Settings - tags_label: Tags - uncategorized: "(uncategorized)" - potential_duplicate_title: Possible duplicate detected - potential_duplicate_description: This pending transaction may be the same as the posted transaction below. If so, merge them to avoid double-counting. - merge_duplicate: Yes, merge them - keep_both: No, keep both loan_payment: Loan Payment - transfer: Transfer + mark_recurring: Mark as Recurring + mark_recurring_subtitle: Track this as a recurring transaction. Amount variance is automatically calculated from past 6 months of similar transactions. + mark_recurring_title: Recurring Transaction + merchant_label: Merchant + name_label: Name + nature: Type + none: "(none)" + note_label: Notes + note_placeholder: Enter a note + overview: Overview + settings: Settings + tags_label: Tags + uncategorized: "(uncategorized)" + potential_duplicate_title: Possible duplicate detected + potential_duplicate_description: This pending transaction may be the same as the posted transaction below. If so, merge them to avoid double-counting. + merge_duplicate: Yes, merge them + keep_both: No, keep both transaction: pending: Pending pending_tooltip: Pending transaction — may change when posted diff --git a/test/models/transaction_test.rb b/test/models/transaction_test.rb index 3f0f903b0..9064f6b97 100644 --- a/test/models/transaction_test.rb +++ b/test/models/transaction_test.rb @@ -41,7 +41,13 @@ class TransactionTest < ActiveSupport::TestCase assert_includes Transaction::ACTIVITY_LABELS, "Sweep In" assert_includes Transaction::ACTIVITY_LABELS, "Sweep Out" assert_includes Transaction::ACTIVITY_LABELS, "Dividend" + assert_includes Transaction::ACTIVITY_LABELS, "Reinvestment" assert_includes Transaction::ACTIVITY_LABELS, "Interest" assert_includes Transaction::ACTIVITY_LABELS, "Fee" + assert_includes Transaction::ACTIVITY_LABELS, "Transfer" + assert_includes Transaction::ACTIVITY_LABELS, "Contribution" + assert_includes Transaction::ACTIVITY_LABELS, "Withdrawal" + assert_includes Transaction::ACTIVITY_LABELS, "Exchange" + assert_includes Transaction::ACTIVITY_LABELS, "Other" end end From 5c300989a7142ad8c61fcc8e578c74f8854d9b2b Mon Sep 17 00:00:00 2001 From: Josh Waldrep Date: Mon, 12 Jan 2026 16:17:36 -0500 Subject: [PATCH 10/10] Remove unused `loan_payment` key from transaction locale --- config/locales/views/transactions/en.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/config/locales/views/transactions/en.yml b/config/locales/views/transactions/en.yml index a53c28d85..ae7bb1cb3 100644 --- a/config/locales/views/transactions/en.yml +++ b/config/locales/views/transactions/en.yml @@ -51,7 +51,6 @@ en: withdrawal: Withdrawal exchange: Exchange other: Other - loan_payment: Loan Payment mark_recurring: Mark as Recurring mark_recurring_subtitle: Track this as a recurring transaction. Amount variance is automatically calculated from past 6 months of similar transactions. mark_recurring_title: Recurring Transaction