From 6d4509fbe6ab955d0e7ea5a6cba3011073a55f22 Mon Sep 17 00:00:00 2001 From: Josh Pigford Date: Fri, 23 May 2025 11:30:04 -0500 Subject: [PATCH 1/3] Optimize transaction totals caching and improve default date filter behavior - Implement caching for transaction totals to enhance performance, using a unique cache key based on family ID and search parameters. - Adjust default date filter logic to use the user's preferred period when no explicit date filters are provided, reducing the load on the database for large datasets. --- app/controllers/transactions_controller.rb | 64 +++++++++++++++++++--- 1 file changed, 56 insertions(+), 8 deletions(-) diff --git a/app/controllers/transactions_controller.rb b/app/controllers/transactions_controller.rb index e5382e73..e0e85f89 100644 --- a/app/controllers/transactions_controller.rb +++ b/app/controllers/transactions_controller.rb @@ -26,7 +26,24 @@ class TransactionsController < ApplicationController params: ->(params) { params.except(:focused_record_id) } ) - @totals = Current.family.income_statement.totals(transactions_scope: transactions_query) + # ------------------------------------------------------------------- + # Cache totals + # ------------------------------------------------------------------- + # Totals calculation is expensive (heavy SQL with grouping). We cache the + # result keyed by: + # • Family id + # • The family-level cache key that already embeds entries.maximum(:updated_at) + # • A digest of the current search params so each distinct filter set gets + # its own cache entry. + # When any entry is created/updated/deleted, the family cache key changes, + # automatically invalidating all related totals. + + params_digest = Digest::MD5.hexdigest(@q.to_json) + cache_key = Current.family.build_cache_key("transactions_totals_#{params_digest}") + + @totals = Rails.cache.fetch(cache_key) do + Current.family.income_statement.totals(transactions_scope: transactions_query) + end end def clear_filter @@ -140,16 +157,47 @@ class TransactionsController < ApplicationController def search_params cleaned_params = params.fetch(:q, {}) - .permit( - :start_date, :end_date, :search, :amount, - :amount_operator, accounts: [], account_ids: [], - categories: [], merchants: [], types: [], tags: [] - ) - .to_h - .compact_blank + .permit( + :start_date, :end_date, :search, :amount, + :amount_operator, accounts: [], account_ids: [], + categories: [], merchants: [], types: [], tags: [] + ) + .to_h + .compact_blank cleaned_params.delete(:amount_operator) unless cleaned_params[:amount].present? + # ------------------------------------------------------------------- + # Performance optimisation + # ------------------------------------------------------------------- + # When a user lands on the Transactions page without an explicit date + # filter, the previous behaviour queried *all* historical transactions + # for the family. For large datasets this results in very expensive + # SQL (as shown in Skylight) – particularly the aggregation queries + # used for @totals. To keep the UI responsive while still showing a + # sensible period of activity, we fall back to the user's preferred + # default period (stored on User#default_period, defaulting to + # "last_30_days") when **no** date filters have been supplied. + # + # This effectively changes the default view from "all-time" to a + # rolling window, dramatically reducing the rows scanned / grouped in + # Postgres without impacting the UX (the user can always clear the + # filter). + # ------------------------------------------------------------------- + if cleaned_params[:start_date].blank? && cleaned_params[:end_date].blank? + period_key = Current.user&.default_period.presence || "last_30_days" + + begin + period = Period.from_key(period_key) + cleaned_params[:start_date] = period.start_date + cleaned_params[:end_date] = period.end_date + rescue Period::InvalidKeyError + # Fallback – should never happen but keeps things safe. + cleaned_params[:start_date] = 30.days.ago.to_date + cleaned_params[:end_date] = Date.current + end + end + cleaned_params end From fd65b5a7472db1ac843ef61a2adc71d9d15723de Mon Sep 17 00:00:00 2001 From: Josh Pigford Date: Fri, 23 May 2025 11:46:12 -0500 Subject: [PATCH 2/3] Implement caching for classification and account groups in BalanceSheet model and optimize sparkline rendering in views - Add caching for classification groups and account groups in the BalanceSheet model to improve performance. - Update views for accountable sparklines to utilize caching for rendered HTML, enhancing load times and reducing database queries. --- app/models/balance_sheet.rb | 105 +++++++++--------- .../accountable_sparklines/show.html.erb | 20 ++-- app/views/accounts/sparkline.html.erb | 20 ++-- 3 files changed, 77 insertions(+), 68 deletions(-) diff --git a/app/models/balance_sheet.rb b/app/models/balance_sheet.rb index a89a9859..9a3d43ea 100644 --- a/app/models/balance_sheet.rb +++ b/app/models/balance_sheet.rb @@ -22,65 +22,70 @@ class BalanceSheet end def classification_groups - asset_groups = account_groups("asset") - liability_groups = account_groups("liability") + Rails.cache.fetch(family.build_cache_key("bs_classification_groups")) do + asset_groups = account_groups("asset") + liability_groups = account_groups("liability") - [ - ClassificationGroup.new( - key: "asset", - display_name: "Assets", - icon: "plus", - total_money: total_assets_money, - account_groups: asset_groups, - syncing?: asset_groups.any?(&:syncing?) - ), - ClassificationGroup.new( - key: "liability", - display_name: "Debts", - icon: "minus", - total_money: total_liabilities_money, - account_groups: liability_groups, - syncing?: liability_groups.any?(&:syncing?) - ) - ] + [ + ClassificationGroup.new( + key: "asset", + display_name: "Assets", + icon: "plus", + total_money: total_assets_money, + account_groups: asset_groups, + syncing?: asset_groups.any?(&:syncing?) + ), + ClassificationGroup.new( + key: "liability", + display_name: "Debts", + icon: "minus", + total_money: total_liabilities_money, + account_groups: liability_groups, + syncing?: liability_groups.any?(&:syncing?) + ) + ] + end end def account_groups(classification = nil) - classification_accounts = classification ? totals_query.filter { |t| t.classification == classification } : totals_query - classification_total = classification_accounts.sum(&:converted_balance) - account_groups = classification_accounts.group_by(&:accountable_type) - .transform_keys { |k| Accountable.from_type(k) } + Rails.cache.fetch(family.build_cache_key("bs_account_groups_#{classification || 'all'}")) do + classification_accounts = classification ? totals_query.filter { |t| t.classification == classification } : totals_query + classification_total = classification_accounts.sum(&:converted_balance) - groups = account_groups.map do |accountable, accounts| - group_total = accounts.sum(&:converted_balance) + account_groups = classification_accounts.group_by(&:accountable_type) + .transform_keys { |k| Accountable.from_type(k) } - key = accountable.model_name.param_key + groups = account_groups.map do |accountable, accounts| + group_total = accounts.sum(&:converted_balance) - AccountGroup.new( - id: classification ? "#{classification}_#{key}_group" : "#{key}_group", - key: key, - name: accountable.display_name, - classification: accountable.classification, - total: group_total, - total_money: Money.new(group_total, currency), - weight: classification_total.zero? ? 0 : group_total / classification_total.to_d * 100, - missing_rates?: accounts.any? { |a| a.missing_rates? }, - color: accountable.color, - syncing?: accounts.any?(&:is_syncing), - accounts: accounts.map do |account| - account.define_singleton_method(:weight) do - classification_total.zero? ? 0 : account.converted_balance / classification_total.to_d * 100 - end + key = accountable.model_name.param_key - account - end.sort_by(&:weight).reverse - ) - end + AccountGroup.new( + id: classification ? "#{classification}_#{key}_group" : "#{key}_group", + key: key, + name: accountable.display_name, + classification: accountable.classification, + total: group_total, + total_money: Money.new(group_total, currency), + weight: classification_total.zero? ? 0 : group_total / classification_total.to_d * 100, + missing_rates?: accounts.any? { |a| a.missing_rates? }, + color: accountable.color, + syncing?: accounts.any?(&:is_syncing), + accounts: accounts.map do |account| + account.define_singleton_method(:weight) do + classification_total.zero? ? 0 : account.converted_balance / classification_total.to_d * 100 + end - groups.sort_by do |group| - manual_order = Accountable::TYPES - type_name = group.key.camelize - manual_order.index(type_name) || Float::INFINITY + account + end.sort_by(&:weight).reverse + ) + end + + groups.sort_by do |group| + manual_order = Accountable::TYPES + type_name = group.key.camelize + manual_order.index(type_name) || Float::INFINITY + end end end diff --git a/app/views/accountable_sparklines/show.html.erb b/app/views/accountable_sparklines/show.html.erb index ba915d57..236c3616 100644 --- a/app/views/accountable_sparklines/show.html.erb +++ b/app/views/accountable_sparklines/show.html.erb @@ -1,11 +1,13 @@ -<%= turbo_frame_tag "#{@accountable.model_name.param_key}_sparkline" do %> -
-
- <%= render "shared/sparkline", id: dom_id(@accountable, :sparkline_chart), series: @series %> -
+<% cache Current.family.build_cache_key("#{@accountable.name}_sparkline_html") do %> + <%= turbo_frame_tag "#{@accountable.model_name.param_key}_sparkline" do %> +
+
+ <%= render "shared/sparkline", id: dom_id(@accountable, :sparkline_chart), series: @series %> +
- <%= tag.p @series.trend.percent_formatted, - style: "color: #{@series.trend.color}", - class: "font-mono text-right text-xs font-medium text-primary" %> -
+ <%= tag.p @series.trend.percent_formatted, + style: "color: #{@series.trend.color}", + class: "font-mono text-right text-xs font-medium text-primary" %> +
+ <% end %> <% end %> diff --git a/app/views/accounts/sparkline.html.erb b/app/views/accounts/sparkline.html.erb index 3a3316ce..ee7f743b 100644 --- a/app/views/accounts/sparkline.html.erb +++ b/app/views/accounts/sparkline.html.erb @@ -1,11 +1,13 @@ -<%= turbo_frame_tag dom_id(@account, :sparkline) do %> -
-
- <%= render "shared/sparkline", id: dom_id(@account, :sparkline_chart), series: @account.sparkline_series %> -
+<% cache Current.family.build_cache_key("account_#{@account.id}_sparkline_html") do %> + <%= turbo_frame_tag dom_id(@account, :sparkline) do %> +
+
+ <%= render "shared/sparkline", id: dom_id(@account, :sparkline_chart), series: @account.sparkline_series %> +
- <%= tag.p @account.sparkline_series.trend.percent_formatted, - style: "color: #{@account.sparkline_series.trend.color}", - class: "font-mono text-right text-xs font-medium text-primary" %> -
+ <%= tag.p @account.sparkline_series.trend.percent_formatted, + style: "color: #{@account.sparkline_series.trend.color}", + class: "font-mono text-right text-xs font-medium text-primary" %> +
+ <% end %> <% end %> From 5cfb4addbdb7d0ce6637e4fbe82ebaefc029f511 Mon Sep 17 00:00:00 2001 From: Josh Pigford Date: Fri, 23 May 2025 12:25:18 -0500 Subject: [PATCH 3/3] Refactor balance sheet weight calculation and improve group weight rendering - Update BalanceSheet model to directly calculate account weights based on converted balances. - Modify dashboard view to compute account weight as a percentage of classification total, enhancing clarity. - Adjust group weight partial to handle effective weight, ensuring accurate rendering of weight representation. --- app/models/balance_sheet.rb | 10 ++++------ app/views/pages/dashboard/_balance_sheet.html.erb | 7 ++++++- app/views/pages/dashboard/_group_weight.html.erb | 6 ++++-- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/app/models/balance_sheet.rb b/app/models/balance_sheet.rb index 9a3d43ea..8c2af0f3 100644 --- a/app/models/balance_sheet.rb +++ b/app/models/balance_sheet.rb @@ -60,7 +60,7 @@ class BalanceSheet key = accountable.model_name.param_key - AccountGroup.new( + group = AccountGroup.new( id: classification ? "#{classification}_#{key}_group" : "#{key}_group", key: key, name: accountable.display_name, @@ -72,13 +72,11 @@ class BalanceSheet color: accountable.color, syncing?: accounts.any?(&:is_syncing), accounts: accounts.map do |account| - account.define_singleton_method(:weight) do - classification_total.zero? ? 0 : account.converted_balance / classification_total.to_d * 100 - end - account - end.sort_by(&:weight).reverse + end.sort_by(&:converted_balance).reverse ) + + group end groups.sort_by do |group| diff --git a/app/views/pages/dashboard/_balance_sheet.html.erb b/app/views/pages/dashboard/_balance_sheet.html.erb index 7a71afe7..337e5268 100644 --- a/app/views/pages/dashboard/_balance_sheet.html.erb +++ b/app/views/pages/dashboard/_balance_sheet.html.erb @@ -116,7 +116,12 @@ <% else %>
- <%= render "pages/dashboard/group_weight", weight: account.weight, color: account_group.color %> + <% + # Calculate weight as percentage of classification total + classification_total = classification_group.total_money.amount + account_weight = classification_total.zero? ? 0 : account.converted_balance / classification_total * 100 + %> + <%= render "pages/dashboard/group_weight", weight: account_weight, color: account_group.color %>
diff --git a/app/views/pages/dashboard/_group_weight.html.erb b/app/views/pages/dashboard/_group_weight.html.erb index d0a8fe0d..c00655a2 100644 --- a/app/views/pages/dashboard/_group_weight.html.erb +++ b/app/views/pages/dashboard/_group_weight.html.erb @@ -1,10 +1,12 @@ <%# locals: (weight:, color:) %> +<% effective_weight = weight.presence || 0 %> +
<% 10.times do |i| %> -
" style="background-color: <%= color %>;">
+
" style="background-color: <%= color %>;">
<% end %>
-

<%= number_to_percentage(weight, precision: 2) %>

+

<%= number_to_percentage(effective_weight, precision: 2) %>