diff --git a/app/models/investment_statement.rb b/app/models/investment_statement.rb index bb7a836a9..da70d40d2 100644 --- a/app/models/investment_statement.rb +++ b/app/models/investment_statement.rb @@ -14,11 +14,9 @@ class InvestmentStatement # Get totals for a specific period def totals(period: Period.current_month) - trades_in_period = family.trades - .joins(:entry) - .where(entries: { date: period.date_range, account_id: investment_account_ids }) + account_ids = investment_account_ids - result = totals_query(trades_scope: trades_in_period) + result = totals_query(account_ids: account_ids, date_range: period.date_range) PeriodTotals.new( contributions: Money.new(result[:contributions], family.currency), @@ -305,12 +303,17 @@ class InvestmentStatement @investment_account_ids ||= investment_accounts.pluck(:id) end - def totals_query(trades_scope:) - sql_hash = Digest::MD5.hexdigest(trades_scope.to_sql) + def totals_query(account_ids:, date_range:) + if account_ids.empty? + return Totals.new(family, account_ids: account_ids, date_range: date_range).call + end + + account_ids_hash = Digest::MD5.hexdigest(account_ids.sort.join(",")) Rails.cache.fetch([ - "investment_statement", "totals_query", family.id, user&.id, sql_hash, family.entries_cache_version - ]) { Totals.new(family, trades_scope: trades_scope).call } + "investment_statement", "totals_query", family.id, user&.id, + account_ids_hash, date_range.begin, date_range.end, family.entries_cache_version + ]) { Totals.new(family, account_ids: account_ids, date_range: date_range).call } end def monetizable_currency diff --git a/app/models/investment_statement/totals.rb b/app/models/investment_statement/totals.rb index 65af7be6f..9d8efffa1 100644 --- a/app/models/investment_statement/totals.rb +++ b/app/models/investment_statement/totals.rb @@ -1,10 +1,13 @@ class InvestmentStatement::Totals - def initialize(family, trades_scope:) + def initialize(family, account_ids:, date_range:) @family = family - @trades_scope = trades_scope + @account_ids = account_ids + @date_range = date_range end def call + return empty_result if @account_ids.empty? + result = ActiveRecord::Base.connection.select_one(query_sql) { @@ -17,6 +20,16 @@ class InvestmentStatement::Totals end private + def empty_result + { + contributions: 0, + withdrawals: 0, + dividends: 0, + interest: 0, + trades_count: 0 + } + end + def query_sql ActiveRecord::Base.sanitize_sql_array([ aggregation_sql, @@ -27,30 +40,36 @@ class InvestmentStatement::Totals # Aggregate trades by direction (buy vs sell) # Buys (qty > 0) = contributions (cash going out to buy securities) # Sells (qty < 0) = withdrawals (cash coming in from selling securities) + # Missing FX rates preserve InvestmentStatement's existing 1:1 fallback. + # + # account_ids is already scoped to the family's visible (draft/active) + # investment accounts, so the query trusts that input and skips a join back + # to accounts for family/status filtering. def aggregation_sql <<~SQL SELECT - COALESCE(SUM(CASE WHEN t.qty > 0 THEN ABS(ae.amount * COALESCE(er.rate, 1)) ELSE 0 END), 0) as contributions, - COALESCE(SUM(CASE WHEN t.qty < 0 THEN ABS(ae.amount * COALESCE(er.rate, 1)) ELSE 0 END), 0) as withdrawals, - COUNT(t.id) as trades_count - FROM (#{@trades_scope.to_sql}) t - JOIN entries ae ON ae.entryable_id = t.id AND ae.entryable_type = 'Trade' - JOIN accounts a ON a.id = ae.account_id + COALESCE(SUM(CASE WHEN trades.qty > 0 THEN ABS(entries.amount * COALESCE(er.rate, 1)) ELSE 0 END), 0) as contributions, + COALESCE(SUM(CASE WHEN trades.qty < 0 THEN ABS(entries.amount * COALESCE(er.rate, 1)) ELSE 0 END), 0) as withdrawals, + COUNT(trades.id) as trades_count + FROM entries + JOIN trades ON trades.id = entries.entryable_id AND entries.entryable_type = 'Trade' LEFT JOIN exchange_rates er ON ( - er.date = ae.date AND - er.from_currency = ae.currency AND + er.date = entries.date AND + er.from_currency = entries.currency AND er.to_currency = :target_currency ) - WHERE a.family_id = :family_id - AND a.status IN ('draft', 'active') - AND ae.excluded = false + WHERE entries.account_id IN (:account_ids) + AND entries.date BETWEEN :start_date AND :end_date + AND entries.excluded = false SQL end def sql_params { - family_id: @family.id, - target_currency: @family.currency + target_currency: @family.currency, + account_ids: @account_ids, + start_date: @date_range.begin, + end_date: @date_range.end } end end diff --git a/db/migrate/20260608160000_add_investment_totals_lookup_index_to_entries.rb b/db/migrate/20260608160000_add_investment_totals_lookup_index_to_entries.rb new file mode 100644 index 000000000..df9e42b6b --- /dev/null +++ b/db/migrate/20260608160000_add_investment_totals_lookup_index_to_entries.rb @@ -0,0 +1,11 @@ +class AddInvestmentTotalsLookupIndexToEntries < ActiveRecord::Migration[7.2] + disable_ddl_transaction! + + def change + add_index :entries, + [ :account_id, :date, :entryable_id ], + name: "index_entries_on_investment_totals_lookup", + where: "entryable_type = 'Trade' AND excluded = false", + algorithm: :concurrently + end +end diff --git a/db/schema.rb b/db/schema.rb index 8c4274404..4c0be7fcf 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_06_07_071000) do +ActiveRecord::Schema[7.2].define(version: 2026_06_08_160000) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -636,6 +636,7 @@ ActiveRecord::Schema[7.2].define(version: 2026_06_07_071000) do t.boolean "import_locked", default: false, null: false t.uuid "parent_entry_id" t.index "lower((name)::text)", name: "index_entries_on_lower_name" + t.index ["account_id", "date", "entryable_id"], name: "index_entries_on_investment_totals_lookup", where: "(((entryable_type)::text = 'Trade'::text) AND (excluded = false))" 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" diff --git a/test/models/investment_statement_test.rb b/test/models/investment_statement_test.rb index 064553969..12c34e10f 100644 --- a/test/models/investment_statement_test.rb +++ b/test/models/investment_statement_test.rb @@ -222,6 +222,50 @@ class InvestmentStatementTest < ActiveSupport::TestCase assert_in_delta 5.0, trend.percent, 0.1 end + test "totals skips cache when there are no investment accounts" do + Rails.cache.expects(:fetch).never + + totals = @statement.totals(period: Period.current_month) + + assert_equal Money.new(0, "USD"), totals.contributions + assert_equal Money.new(0, "USD"), totals.withdrawals + assert_equal Money.new(0, "USD"), totals.dividends + assert_equal Money.new(0, "USD"), totals.interest + assert_equal 0, totals.trades_count + end + + test "totals aggregate directly from trade entries" do + period = Period.custom(start_date: Date.current.beginning_of_month, end_date: Date.current) + shared_user = users(:new_email) + investment_account = create_investment_account(balance: 500) + hidden_account = create_investment_account(balance: 500) + investment_account.share_with!(shared_user, permission: "read_only", include_in_finances: true) + + create_trade(account: investment_account, qty: 2, amount: 120, date: period.start_date) + create_trade(account: investment_account, qty: -1, amount: -40, date: period.start_date + 1.day) + create_trade(account: investment_account, qty: 1, amount: 999, date: period.start_date - 1.day) + create_trade(account: hidden_account, qty: 1, amount: 9999, date: period.start_date) + + statement = InvestmentStatement.new(@family, user: shared_user) + totals = nil + queries = capture_sql_queries { totals = statement.totals(period: period) } + + assert_equal Money.new(120, "USD"), totals.contributions + assert_equal Money.new(40, "USD"), totals.withdrawals + assert_equal 2, totals.trades_count + + aggregate_queries = queries.grep(/SUM\(CASE WHEN trades\.qty > 0/) + assert_equal 1, aggregate_queries.size + assert_includes aggregate_queries.first, "FROM entries JOIN trades" + assert_includes aggregate_queries.first, "entries.entryable_type = 'Trade'" + assert_includes aggregate_queries.first, "entries.account_id IN" + assert_includes aggregate_queries.first, "entries.excluded = false" + assert_no_match(/FROM \(SELECT "trades"\.\*/, aggregate_queries.first) + # account_ids is pre-scoped to the family's visible accounts, so the + # aggregate trusts that input and no longer joins back to accounts. + assert_no_match(/JOIN accounts/, aggregate_queries.first) + end + private def create_investment_account(balance:, cash_balance: 0, currency: "USD") @family.accounts.create!( @@ -232,4 +276,19 @@ class InvestmentStatementTest < ActiveSupport::TestCase accountable: Investment.new ) end + + def create_trade(account:, qty:, amount:, date:) + account.entries.create!( + name: "Trade #{SecureRandom.hex(3)}", + amount: amount, + date: date, + currency: account.currency, + entryable: Trade.new( + security: Security.create!(ticker: "T#{SecureRandom.hex(2)}", name: "Test Security"), + qty: qty, + price: amount.to_d.abs / qty.to_d.abs, + currency: account.currency + ) + ) + end end diff --git a/test/support/sql_query_capture.rb b/test/support/sql_query_capture.rb new file mode 100644 index 000000000..ed1b1f1c0 --- /dev/null +++ b/test/support/sql_query_capture.rb @@ -0,0 +1,17 @@ +module SqlQueryCapture + def capture_sql_queries + queries = [] + callback = lambda do |_name, _started, _finished, _unique_id, payload| + next if payload[:cached] + next if %w[SCHEMA TRANSACTION].include?(payload[:name]) + + queries << payload[:sql].squish + end + + ActiveSupport::Notifications.subscribed(callback, "sql.active_record") do + yield + end + + queries + end +end diff --git a/test/test_helper.rb b/test/test_helper.rb index b70459708..d73d3706d 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -26,6 +26,7 @@ require "webmock/minitest" require "rack/test" require "tempfile" require "uri" +require Rails.root.join("test/support/sql_query_capture").to_s VCR.configure do |config| config.cassette_library_dir = "test/vcr_cassettes" @@ -85,6 +86,8 @@ module ActiveSupport # test so each starts from the rolled-back DB state. setup { Setting.clear_cache } + include SqlQueryCapture + # Add more helper methods to be used by all tests here... def sign_in(user) post sessions_path, params: { email: user.email, password: user_password_test }