diff --git a/app/models/balance/chart_series_builder.rb b/app/models/balance/chart_series_builder.rb index c988651b7..034f54c67 100644 --- a/app/models/balance/chart_series_builder.rb +++ b/app/models/balance/chart_series_builder.rb @@ -136,13 +136,22 @@ class Balance::ChartSeriesBuilder LIMIT 1 ) last_bal ON TRUE LEFT JOIN LATERAL ( - SELECT er.rate - FROM exchange_rates er - WHERE er.from_currency = accounts.currency - AND er.to_currency = :target_currency - AND er.date <= d.date - ORDER BY er.date DESC - LIMIT 1 + SELECT COALESCE( + (SELECT er.rate + FROM exchange_rates er + WHERE er.from_currency = accounts.currency + AND er.to_currency = :target_currency + AND er.date <= d.date + ORDER BY er.date DESC + LIMIT 1), + (SELECT er.rate + FROM exchange_rates er + WHERE er.from_currency = accounts.currency + AND er.to_currency = :target_currency + AND er.date > d.date + ORDER BY er.date ASC + LIMIT 1) + ) AS rate ) er ON TRUE WHERE accounts.id = ANY(array[:account_ids]::uuid[]) GROUP BY d.date diff --git a/app/models/balance_sheet/account_totals.rb b/app/models/balance_sheet/account_totals.rb index 31708f241..2a1f63761 100644 --- a/app/models/balance_sheet/account_totals.rb +++ b/app/models/balance_sheet/account_totals.rb @@ -27,37 +27,56 @@ class BalanceSheet::AccountTotals @visible_accounts ||= family.accounts.visible.with_attached_logo end + # Wraps each account in an AccountRow with its converted balance and sync status. def account_rows - @account_rows ||= query.map do |account_row| + @account_rows ||= accounts.map do |account| AccountRow.new( - account: account_row, - converted_balance: account_row.converted_balance, - is_syncing: sync_status_monitor.account_syncing?(account_row) + account: account, + converted_balance: converted_balance_for(account), + is_syncing: sync_status_monitor.account_syncing?(account) ) end end + # Returns the cache key for storing visible account IDs, invalidated on data updates. def cache_key family.build_cache_key( - "balance_sheet_account_rows", + "balance_sheet_account_ids", invalidate_on_data_updates: true ) end - def query - @query ||= Rails.cache.fetch(cache_key) do - visible_accounts - .joins(ActiveRecord::Base.sanitize_sql_array([ - "LEFT JOIN exchange_rates ON exchange_rates.date = ? AND accounts.currency = exchange_rates.from_currency AND exchange_rates.to_currency = ?", - Date.current, - family.currency - ])) - .select( - "accounts.*", - "SUM(accounts.balance * COALESCE(exchange_rates.rate, 1)) as converted_balance" - ) - .group(:classification, :accountable_type, :id) - .to_a + # Loads visible accounts, caching their IDs to speed up subsequent requests. + # On cache miss, loads records once and writes IDs; on hit, filters by cached IDs. + def accounts + @accounts ||= begin + ids = Rails.cache.read(cache_key) + + if ids + visible_accounts.where(id: ids).to_a + else + records = visible_accounts.to_a + Rails.cache.write(cache_key, records.map(&:id)) + records + end end end + + # Batch-fetches today's exchange rates for all foreign currencies present in accounts. + # @return [Hash{String => Numeric}] currency code to rate mapping + def exchange_rates + @exchange_rates ||= begin + foreign_currencies = accounts.filter_map { |a| a.currency if a.currency != family.currency } + ExchangeRate.rates_for(foreign_currencies, to: family.currency, date: Date.current) + end + end + + # Converts an account's balance to the family's currency using pre-fetched exchange rates. + # @return [BigDecimal] balance in the family's currency + def converted_balance_for(account) + return account.balance if account.currency == family.currency + + rate = exchange_rates[account.currency] + account.balance * rate + end end diff --git a/app/models/concerns/accountable.rb b/app/models/concerns/accountable.rb index 3b764c245..9324e34ab 100644 --- a/app/models/concerns/accountable.rb +++ b/app/models/concerns/accountable.rb @@ -62,15 +62,21 @@ module Accountable self.name.pluralize.titleize end + # Sums the balances of all active accounts of this type, converting foreign currencies to the family's currency. + # @return [BigDecimal] total balance in the family's currency def balance_money(family) - family.accounts - .active - .joins(sanitize_sql_array([ - "LEFT JOIN exchange_rates ON exchange_rates.date = :current_date AND accounts.currency = exchange_rates.from_currency AND exchange_rates.to_currency = :family_currency", - { current_date: Date.current.to_s, family_currency: family.currency } - ])) - .where(accountable_type: self.name) - .sum("accounts.balance * COALESCE(exchange_rates.rate, 1)") + accounts = family.accounts.active.where(accountable_type: self.name).to_a + + foreign_currencies = accounts.filter_map { |a| a.currency if a.currency != family.currency } + rates = ExchangeRate.rates_for(foreign_currencies, to: family.currency, date: Date.current) + + accounts.sum(BigDecimal(0)) { |account| + if account.currency == family.currency + account.balance + else + account.balance * (rates[account.currency] || 1) + end + } end end diff --git a/app/models/exchange_rate/provided.rb b/app/models/exchange_rate/provided.rb index c4e894d26..8a3f88af3 100644 --- a/app/models/exchange_rate/provided.rb +++ b/app/models/exchange_rate/provided.rb @@ -8,10 +8,24 @@ module ExchangeRate::Provided registry.get_provider(provider.to_sym) end + # Maximum number of days to look back for a cached rate before calling the provider. + NEAREST_RATE_LOOKBACK_DAYS = 5 + def find_or_fetch_rate(from:, to:, date: Date.current, cache: true) rate = find_by(from_currency: from, to_currency: to, date: date) return rate if rate.present? + # Reuse the nearest recently-cached rate before hitting the provider. + # Providers like Yahoo Finance return the most recent trading-day rate + # (e.g. Friday for a Saturday request) and save it under that date, so + # subsequent requests for the weekend date always miss the exact lookup + # and trigger redundant API calls. + nearest = where(from_currency: from, to_currency: to) + .where(date: (date - NEAREST_RATE_LOOKBACK_DAYS)..date) + .order(date: :desc) + .first + return nearest if nearest.present? + return nil unless provider.present? # No provider configured (some self-hosted apps) response = provider.fetch_exchange_rate(from: from, to: to, date: date) @@ -39,6 +53,15 @@ module ExchangeRate::Provided rate end + # Batch-fetches exchange rates for multiple source currencies. + # Returns a hash mapping each currency to its numeric rate, defaulting to 1 when unavailable. + def rates_for(currencies, to:, date: Date.current) + currencies.uniq.each_with_object({}) do |currency, map| + rate = find_or_fetch_rate(from: currency, to: to, date: date) + map[currency] = rate&.rate || 1 + end + end + # @return [Integer] The number of exchange rates synced def import_provider_rates(from:, to:, start_date:, end_date:, clear_cache: false) unless provider.present? diff --git a/db/migrate/20260217120000_add_composite_index_on_accounts_family_status_type.rb b/db/migrate/20260217120000_add_composite_index_on_accounts_family_status_type.rb new file mode 100644 index 000000000..3cf20a596 --- /dev/null +++ b/db/migrate/20260217120000_add_composite_index_on_accounts_family_status_type.rb @@ -0,0 +1,9 @@ +class AddCompositeIndexOnAccountsFamilyStatusType < ActiveRecord::Migration[7.2] + disable_ddl_transaction! + + def change + add_index :accounts, [ :family_id, :status, :accountable_type ], + name: "index_accounts_on_family_id_status_accountable_type", + algorithm: :concurrently + end +end diff --git a/db/schema.rb b/db/schema.rb index fe8f6523b..cd9839dce 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_02_11_120001) do +ActiveRecord::Schema[7.2].define(version: 2026_02_17_120000) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -56,6 +56,7 @@ ActiveRecord::Schema[7.2].define(version: 2026_02_11_120001) do t.index ["currency"], name: "index_accounts_on_currency" t.index ["family_id", "accountable_type"], name: "index_accounts_on_family_id_and_accountable_type" t.index ["family_id", "id"], name: "index_accounts_on_family_id_and_id" + t.index ["family_id", "status", "accountable_type"], name: "index_accounts_on_family_id_status_accountable_type" t.index ["family_id", "status"], name: "index_accounts_on_family_id_and_status" t.index ["family_id"], name: "index_accounts_on_family_id" t.index ["import_id"], name: "index_accounts_on_import_id" diff --git a/test/models/balance/chart_series_builder_test.rb b/test/models/balance/chart_series_builder_test.rb index b80d51bca..7e180ef36 100644 --- a/test/models/balance/chart_series_builder_test.rb +++ b/test/models/balance/chart_series_builder_test.rb @@ -52,11 +52,11 @@ class Balance::ChartSeriesBuilderTest < ActiveSupport::TestCase ) # Only 1 rate in DB. We'll be missing the first and last days in the series. - # This rate should be applied to 1 day ago and today, but not 2 days ago (will fall back to 1) + # This rate should be applied to all days: LOCF for future dates, nearest future rate for earlier dates. ExchangeRate.create!(date: 1.day.ago.to_date, from_currency: "USD", to_currency: "EUR", rate: 2) expected = [ - 1000, # No rate available, so fall back to 1:1 conversion (1000 USD = 1000 EUR) + 2000, # No prior rate, so use nearest future rate (2:1 from 1 day ago): 1000 * 2 = 2000 2200, # Rate available, so use 2:1 conversion (1100 USD = 2200 EUR) 2400 # Rate NOT available, but LOCF will use the last available rate, so use 2:1 conversion (1200 USD = 2400 EUR) ] diff --git a/test/models/exchange_rate_test.rb b/test/models/exchange_rate_test.rb index 021b4edf4..53e037dd6 100644 --- a/test/models/exchange_rate_test.rb +++ b/test/models/exchange_rate_test.rb @@ -67,4 +67,32 @@ class ExchangeRateTest < ActiveSupport::TestCase assert_nil ExchangeRate.find_or_fetch_rate(from: "USD", to: "EUR", date: Date.current, cache: true) end + + test "reuses nearest cached rate within lookback window instead of calling provider" do + # Simulate a rate saved under Friday's date when Saturday is requested + friday = 1.day.ago.to_date + ExchangeRate.create!(from_currency: "USD", to_currency: "JPY", date: friday, rate: 150.5) + + saturday = Date.current + + @provider.expects(:fetch_exchange_rate).never + + result = ExchangeRate.find_or_fetch_rate(from: "USD", to: "JPY", date: saturday) + assert_equal 150.5, result.rate + assert_equal friday, result.date + end + + test "does not reuse cached rate outside lookback window" do + old_date = (ExchangeRate::NEAREST_RATE_LOOKBACK_DAYS + 1).days.ago.to_date + ExchangeRate.create!(from_currency: "USD", to_currency: "JPY", date: old_date, rate: 140.0) + + provider_response = provider_success_response( + OpenStruct.new(from: "USD", to: "JPY", date: Date.current, rate: 155.0) + ) + + @provider.expects(:fetch_exchange_rate).returns(provider_response) + + result = ExchangeRate.find_or_fetch_rate(from: "USD", to: "JPY", date: Date.current) + assert_equal 155.0, result.rate + end end