Files
sure/app/models/balance_sheet/account_totals.rb
GermanDZ 7e1de420ca perf(accounts): kill sidebar/sparkline N+1s and cache the sidebar (#1683)
* perf(accounts): kill sidebar/sparkline N+1s and cache the sidebar

The dashboard was issuing hundreds of per-account `SELECT 1` and
polymorphic `accountable` lookups on every page load. Sidebar render
alone hit the DB ~50–100× and ran twice per request (mobile + desktop).

Changes:

- AccountableSparklinesController: short-circuit
  `requires_normalized_aggregation?` to Investment/Crypto only and
  collapse the per-account `linked?` loop into a single `EXISTS`. Kills
  the N+1 `AccountProvider Exists?` queries on every sparkline endpoint.

- BalanceSheet::AccountTotals#visible_accounts: preload `:accountable`,
  `:plaid_account`, `:simplefin_account`, and
  `account_providers: :provider` so the sidebar's
  `account.subtype` / `account.linked?` / `account.provider` calls don't
  trigger per-row polymorphic loads.

- AccountsController#index: same preloads on `@manual_accounts`.

- accounts/index/_account_groups.erb: extend the existing `Preloader`
  call to batch-load accountable + provider associations so the
  per-provider-item partials (Plaid, SimpleFIN, Coinbase, etc.) stop
  re-issuing N+1s when rendering account rows on /accounts.

- accounts/_account_sidebar_tabs.html.erb: wrap the partial in a
  `cache` block keyed on the family's data-version, the current user,
  shares fingerprint, locale, mobile flag, active tab, and a
  path-derived "current account" component (`sidebar_active_account_id`
  helper). The sidebar is rendered on every page in the layout
  (twice — mobile + desktop drawers), so most navigations now serve
  the cached fragment instead of re-walking accounts/balances.

Local impact (DZG family, 23 accounts, 6.1k transactions):
- Dashboard `/`: ~6.5s → ~1.95s
- /accounts: ~2.7s → ~0.85s on warm cache
- /accountable_sparklines/*: per-request N+1s eliminated; remaining
  cost is request boilerplate which can be addressed by bumping
  `RAILS_MAX_THREADS` (the dashboard fans out 5 sparkline turbo frames
  in parallel and Puma's default 3 threads serialize them).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(perf): address PR review on sidebar/sparkline perf changes

- AccountableSparklinesController#requires_normalized_aggregation?
  also matches legacy plaid_account_id / simplefin_account_id links,
  not just new-style account_providers, so investment/crypto accounts
  in the legacy linking state still get LinkedInvestmentSeriesNormalizer
  applied (Codex P1 / CodeRabbit major).

- Sidebar share fingerprint includes both `count` and `max(updated_at)`
  so deleting a non-most-recent AccountShare invalidates the cached
  fragment for users who lost access (Codex P1).

- Move the sidebar cache-key construction (incl. the AccountShare
  query) from the ERB into a new `account_sidebar_tabs_cache_key`
  helper, per the project's "no heavy logic in ERB" rule (CodeRabbit).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(perf): address human review on perf PR

- Account.linked: new SQL-level scope mirroring `Account#linked?` so
  the controller and per-instance method share one definition. Removes
  the duplicated raw SQL string in
  `AccountableSparklinesController#requires_normalized_aggregation?`,
  which now reads `accounts.linked.exists?` (jjmata, sure-design).

- AccountsHelper: move `sidebar_active_account_id` and
  `account_sidebar_tabs_cache_key` out of `ApplicationHelper`. The
  cache-key helper also collapses the AccountShare `count` + `max(updated_at)`
  fingerprint into a single `pick` query so we don't pay two round-trips
  on every render (jjmata, sure-design).

- test/models/account/linkable_test.rb: pin the `Account.linked` scope
  against all three link types (account_providers, legacy plaid_account,
  legacy simplefin_account) so any future schema change that diverges
  the SQL definition from `linked?` breaks a test instead of silently
  serving wrong sparkline aggregations (sure-design).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(perf): correct shares cache fingerprint on raw-SQL pick

`pick(Arel.sql("count(*), max(updated_at)"))` passes a single comma-
separated fragment, which Rails returns as a String (per the documented
behavior of `pluck` with SQL fragments). The previous `max_at&.to_i`
silently truncated `"2025-05-06 12:34:56.789 UTC"` to `2025`, so the
sidebar cache key would not change for share `updated_at` movements
within the same calendar year — including share deletions — leaving
revoked users with a stale sidebar until the 12h expiry.

Pass the aggregates as two separate `Arel.sql` args and just concatenate
the raw String values into the cache key. The values only need to be
stable for a given DB state, not numerically meaningful.

Caught by CodeRabbit on PR #1683.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-07 17:31:16 +02:00

106 lines
3.5 KiB
Ruby

class BalanceSheet::AccountTotals
def initialize(family, user: nil, sync_status_monitor:)
@family = family
@user = user
@sync_status_monitor = sync_status_monitor
end
def asset_accounts
@asset_accounts ||= account_rows.filter { |t| t.classification == "asset" }
end
def liability_accounts
@liability_accounts ||= account_rows.filter { |t| t.classification == "liability" }
end
private
attr_reader :family, :user, :sync_status_monitor
AccountRow = Data.define(:account, :converted_balance, :is_syncing, :included_in_finances) do
def syncing? = is_syncing
def included_in_finances? = included_in_finances
# Allows Rails path helpers to generate URLs from the wrapper
def to_param = account.to_param
delegate_missing_to :account
end
def visible_accounts
@visible_accounts ||= begin
scope = family.accounts.visible.with_attached_logo
.includes(
:account_shares,
:accountable,
:plaid_account,
:simplefin_account,
account_providers: :provider
)
scope = scope.accessible_by(user) if user
scope
end
end
def finance_account_ids
@finance_account_ids ||= if user
family.accounts.included_in_finances_for(user).pluck(:id).to_set
else
nil
end
end
# Wraps each account in an AccountRow with its converted balance and sync status.
def account_rows
@account_rows ||= accounts.map do |account|
AccountRow.new(
account: account,
converted_balance: converted_balance_for(account),
is_syncing: sync_status_monitor.account_syncing?(account),
included_in_finances: finance_account_ids.nil? || finance_account_ids.include?(account.id)
)
end
end
# Returns the cache key for storing visible account IDs, invalidated on data updates.
def cache_key
shares_version = user ? AccountShare.where(user: user).maximum(:updated_at)&.to_i : nil
family.build_cache_key(
[ "balance_sheet_account_ids", user&.id, shares_version ].compact.join("_"),
invalidate_on_data_updates: true
)
end
# 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