mirror of
https://github.com/we-promise/sure.git
synced 2026-04-19 03:54:08 +00:00
Fix foreign currency accounts using wrong exchange rate in balance sheet totals (#1010)
Balance sheet totals and accountable type summaries used a SQL JOIN on exchange_rates matching only today's date, which returned NULL (defaulting to 1:1) when no rate existed for that exact date. This caused foreign currency accounts to show incorrect totals. Changes: - Refactor BalanceSheet::AccountTotals to batch-fetch exchange rates via ExchangeRate.rates_for, with provider fallback, instead of a SQL join - Refactor Accountable.balance_money to use the same batch approach - Add ExchangeRate.rates_for helper for deduplicated rate lookups - Fix net worth chart query to fall back to the nearest future rate when no historical rate exists for a given date - Add composite index on accounts (family_id, status, accountable_type) - Reuse nearest cached exchange rate within a 5-day lookback window before calling the provider, preventing redundant API calls on weekends and holidays when providers return prior-day rates https://claude.ai/code/session_01GyssBJxQqdWnuYofQRjUu8 Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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?
|
||||
|
||||
Reference in New Issue
Block a user