Skip to content

Commit

Permalink
Improve speed of transactions page (maybe-finance#1752)
Browse files Browse the repository at this point in the history
* Make demo data more realistic

* Fix N+1 transactions query

* Lint fixes

* Totals query

* Consolidate stats calcs

* Fix preload

* Fix filter clearing

* Fix N+1 queries for family sync detection

* Reduce queries for rendering transfers

* Fix tests

* Remove flaky test
  • Loading branch information
zachgoll authored Feb 1, 2025
1 parent 53f4b32 commit 2c2b600
Show file tree
Hide file tree
Showing 22 changed files with 199 additions and 185 deletions.
2 changes: 2 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ gem "good_job"

# Error logging
gem "stackprof"
gem "rack-mini-profiler"
gem "sentry-ruby"
gem "sentry-rails"

Expand Down Expand Up @@ -67,6 +68,7 @@ group :development do
gem "ruby-lsp-rails"
gem "web-console"
gem "faker"
gem "benchmark-ips"
end

group :test do
Expand Down
5 changes: 5 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ GEM
base64 (0.2.0)
bcrypt (3.1.20)
benchmark (0.4.0)
benchmark-ips (2.14.0)
better_html (2.1.1)
actionview (>= 6.0)
activesupport (>= 6.0)
Expand Down Expand Up @@ -317,6 +318,8 @@ GEM
raabro (1.4.0)
racc (1.8.1)
rack (3.1.8)
rack-mini-profiler (3.3.1)
rack (>= 1.2.0)
rack-session (2.1.0)
base64 (>= 0.1.0)
rack (>= 3.0.0)
Expand Down Expand Up @@ -501,6 +504,7 @@ PLATFORMS
DEPENDENCIES
aws-sdk-s3 (~> 1.177.0)
bcrypt (~> 3.1)
benchmark-ips
bootsnap
brakeman
capybara
Expand Down Expand Up @@ -531,6 +535,7 @@ DEPENDENCIES
plaid
propshaft
puma (>= 5.0)
rack-mini-profiler
rails (~> 7.2.2)
rails-settings-cached
redcarpet
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/pages_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def dashboard

@accounts = Current.family.accounts.active
@account_groups = @accounts.by_group(period: @period, currency: Current.family.currency)
@transaction_entries = Current.family.entries.account_transactions.limit(6).reverse_chronological
@transaction_entries = Current.family.entries.incomes_and_expenses.limit(6).reverse_chronological

# TODO: Placeholders for trendlines
placeholder_series_data = 10.times.map do |i|
Expand Down
32 changes: 19 additions & 13 deletions app/controllers/transactions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,30 +7,36 @@ class TransactionsController < ApplicationController

def index
@q = search_params
search_query = Current.family.transactions.search(@q).reverse_chronological
search_query = Current.family.transactions.search(@q)

set_focused_record(search_query, params[:focused_record_id], default_per_page: 50)

@pagy, @transaction_entries = pagy(
search_query,
search_query.reverse_chronological.preload(
:account,
entryable: [
:category, :merchant, :tags,
:transfer_as_inflow,
transfer_as_outflow: {
inflow_transaction: { entry: :account },
outflow_transaction: { entry: :account }
}
]
),
limit: params[:per_page].presence || default_params[:per_page],
params: ->(params) { params.except(:focused_record_id) }
)

totals_query = search_query.incomes_and_expenses
family_currency = Current.family.currency
count_with_transfers = search_query.count
count_without_transfers = totals_query.count

@totals = {
count: ((count_with_transfers - count_without_transfers) / 2) + count_without_transfers,
income: totals_query.income_total(family_currency).abs,
expense: totals_query.expense_total(family_currency)
}
@transfers = @transaction_entries.map { |entry| entry.entryable.transfer_as_outflow }.compact
@totals = search_query.stats(Current.family.currency)
end

def clear_filter
updated_params = stored_params.deep_dup
updated_params = {
"q" => search_params,
"page" => params[:page],
"per_page" => params[:per_page]
}

q_params = updated_params["q"] || {}

Expand Down
6 changes: 3 additions & 3 deletions app/helpers/account/entries_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ def transfer_entries(entries)
transfers.map(&:transfer).uniq
end

def entries_by_date(entries, selectable: true, totals: false)
entries.reverse_chronological.group_by(&:date).map do |date, grouped_entries|
def entries_by_date(entries, transfers: [], selectable: true, totals: false)
entries.group_by(&:date).map do |date, grouped_entries|
content = capture do
yield grouped_entries
yield [ grouped_entries, transfers.select { |t| t.outflow_transaction.entry.date == date } ]
end

next if content.blank?
Expand Down
44 changes: 25 additions & 19 deletions app/models/account/entry.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
class Account::Entry < ApplicationRecord
include Monetizable

Stats = Struct.new(:currency, :count, :income_total, :expense_total, keyword_init: true)

monetize :amount

belongs_to :account
Expand Down Expand Up @@ -33,11 +35,11 @@ class Account::Entry < ApplicationRecord
# All non-transfer entries, rejected transfers, and the outflow of a loan payment transfer are incomes/expenses
scope :incomes_and_expenses, -> {
joins("INNER JOIN account_transactions ON account_transactions.id = account_entries.entryable_id AND account_entries.entryable_type = 'Account::Transaction'")
.joins("LEFT JOIN transfers ON transfers.inflow_transaction_id = account_transactions.id OR transfers.outflow_transaction_id = account_transactions.id")
.joins("LEFT JOIN account_transactions inflow_txns ON inflow_txns.id = transfers.inflow_transaction_id")
.joins("LEFT JOIN account_entries inflow_entries ON inflow_entries.entryable_id = inflow_txns.id AND inflow_entries.entryable_type = 'Account::Transaction'")
.joins("LEFT JOIN accounts inflow_accounts ON inflow_accounts.id = inflow_entries.account_id")
.where("transfers.id IS NULL OR transfers.status = 'rejected' OR (account_entries.amount > 0 AND inflow_accounts.accountable_type = 'Loan')")
.joins("LEFT JOIN transfers ON transfers.inflow_transaction_id = account_transactions.id OR transfers.outflow_transaction_id = account_transactions.id")
.joins("LEFT JOIN account_transactions inflow_txns ON inflow_txns.id = transfers.inflow_transaction_id")
.joins("LEFT JOIN account_entries inflow_entries ON inflow_entries.entryable_id = inflow_txns.id AND inflow_entries.entryable_type = 'Account::Transaction'")
.joins("LEFT JOIN accounts inflow_accounts ON inflow_accounts.id = inflow_entries.account_id")
.where("transfers.id IS NULL OR transfers.status = 'rejected' OR (account_entries.amount > 0 AND inflow_accounts.accountable_type = 'Loan')")
}

scope :incomes, -> {
Expand Down Expand Up @@ -154,20 +156,24 @@ def bulk_update!(bulk_update_params)
all.size
end

def income_total(currency = "USD", start_date: nil, end_date: nil)
total = incomes.where(date: start_date..end_date)
.map { |e| e.amount_money.exchange_to(currency, date: e.date, fallback_rate: 0) }
.sum

Money.new(total, currency)
end

def expense_total(currency = "USD", start_date: nil, end_date: nil)
total = expenses.where(date: start_date..end_date)
.map { |e| e.amount_money.exchange_to(currency, date: e.date, fallback_rate: 0) }
.sum

Money.new(total, currency)
def stats(currency = "USD")
result = all
.incomes_and_expenses
.joins(sanitize_sql_array([ "LEFT JOIN exchange_rates er ON account_entries.date = er.date AND account_entries.currency = er.from_currency AND er.to_currency = ?", currency ]))
.select(
"COUNT(*) AS count",
"SUM(CASE WHEN account_entries.amount < 0 THEN (account_entries.amount * COALESCE(er.rate, 1)) ELSE 0 END) AS income_total",
"SUM(CASE WHEN account_entries.amount > 0 THEN (account_entries.amount * COALESCE(er.rate, 1)) ELSE 0 END) AS expense_total"
)
.to_a
.first

Stats.new(
currency: currency,
count: result.count,
income_total: result.income_total ? result.income_total * -1 : 0,
expense_total: result.expense_total || 0
)
end
end
end
Loading

0 comments on commit 2c2b600

Please sign in to comment.