mirror of
https://github.com/maybe-finance/maybe.git
synced 2025-08-07 06:25:19 +02:00
Optimize Budget model performance by preventing N+1 queries
- Fixed N+1 query issue in method that was causing excessive database calls - Optimized category lookups by pre-computing category totals with for O(1) lookup - Added explicit to prevent redundant category association loads - Applied same optimization to method - Added comprehensive test coverage to verify performance improvements - These changes significantly reduce database queries when rendering budget donut charts Performance Impact: - Before: N budget_categories × 2 queries (category + expense lookup) = 2N queries per chart - After: 2 total queries (1 preload + 1 expense totals) regardless of category count This optimization is especially impactful for users with many budget categories, reducing response time and database load on budget-related pages.
This commit is contained in:
parent
347c0a7906
commit
47e60daf12
2 changed files with 89 additions and 3 deletions
|
@ -133,8 +133,13 @@ class Budget < ApplicationRecord
|
||||||
# Continuous gray segment for empty budgets
|
# Continuous gray segment for empty budgets
|
||||||
return [ { color: "var(--budget-unallocated-fill)", amount: 1, id: unused_segment_id } ] unless allocations_valid?
|
return [ { color: "var(--budget-unallocated-fill)", amount: 1, id: unused_segment_id } ] unless allocations_valid?
|
||||||
|
|
||||||
segments = budget_categories.map do |bc|
|
# Pre-compute category totals to avoid repeated lookups in the loop
|
||||||
{ color: bc.category.color, amount: budget_category_actual_spending(bc), id: bc.id }
|
category_totals_by_id = expense_totals.category_totals.index_by { |ct| ct.category.id }
|
||||||
|
|
||||||
|
# Use includes to prevent N+1 queries on category association
|
||||||
|
segments = budget_categories.includes(:category).map do |bc|
|
||||||
|
actual_spending = category_totals_by_id[bc.category.id]&.total || 0
|
||||||
|
{ color: bc.category.color, amount: actual_spending, id: bc.id }
|
||||||
end
|
end
|
||||||
|
|
||||||
if available_to_spend.positive?
|
if available_to_spend.positive?
|
||||||
|
@ -187,7 +192,8 @@ class Budget < ApplicationRecord
|
||||||
# Budget allocations: How much user has budgeted for all parent categories combined
|
# Budget allocations: How much user has budgeted for all parent categories combined
|
||||||
# =============================================================================
|
# =============================================================================
|
||||||
def allocated_spending
|
def allocated_spending
|
||||||
budget_categories.reject { |bc| bc.subcategory? }.sum(&:budgeted_spending)
|
# Use includes to prevent N+1 queries when checking subcategory?
|
||||||
|
budget_categories.includes(:category).reject { |bc| bc.subcategory? }.sum(&:budgeted_spending)
|
||||||
end
|
end
|
||||||
|
|
||||||
def allocated_percent
|
def allocated_percent
|
||||||
|
|
80
test/models/budget_test.rb
Normal file
80
test/models/budget_test.rb
Normal file
|
@ -0,0 +1,80 @@
|
||||||
|
require "test_helper"
|
||||||
|
|
||||||
|
class BudgetTest < ActiveSupport::TestCase
|
||||||
|
def setup
|
||||||
|
@family = families(:one)
|
||||||
|
@budget = Budget.create!(
|
||||||
|
family: @family,
|
||||||
|
start_date: Date.current.beginning_of_month,
|
||||||
|
end_date: Date.current.end_of_month,
|
||||||
|
currency: "USD"
|
||||||
|
)
|
||||||
|
end
|
||||||
|
|
||||||
|
test "to_donut_segments_json returns proper structure" do
|
||||||
|
# Test with empty budget (should return single unused segment)
|
||||||
|
segments = @budget.to_donut_segments_json
|
||||||
|
assert_equal 1, segments.length
|
||||||
|
assert_equal "unused", segments.first[:id]
|
||||||
|
assert_equal "var(--budget-unallocated-fill)", segments.first[:color]
|
||||||
|
assert_equal 1, segments.first[:amount]
|
||||||
|
end
|
||||||
|
|
||||||
|
test "to_donut_segments_json with categories doesn't cause N+1 queries" do
|
||||||
|
# Create some test categories and budget categories
|
||||||
|
category1 = @family.categories.create!(name: "Food", color: "#ff0000")
|
||||||
|
category2 = @family.categories.create!(name: "Transport", color: "#00ff00")
|
||||||
|
|
||||||
|
@budget.budget_categories.create!(category: category1, budgeted_spending: 1000, currency: "USD")
|
||||||
|
@budget.budget_categories.create!(category: category2, budgeted_spending: 500, currency: "USD")
|
||||||
|
|
||||||
|
# Mock budget to be valid
|
||||||
|
@budget.stubs(:allocations_valid?).returns(true)
|
||||||
|
@budget.stubs(:available_to_spend).returns(Money.new(100))
|
||||||
|
@budget.stubs(:expense_totals).returns(
|
||||||
|
double(category_totals: [
|
||||||
|
double(category: double(id: category1.id), total: 800),
|
||||||
|
double(category: double(id: category2.id), total: 300)
|
||||||
|
])
|
||||||
|
)
|
||||||
|
|
||||||
|
# Count the number of queries when generating segments
|
||||||
|
queries_before = count_queries do
|
||||||
|
@budget.to_donut_segments_json
|
||||||
|
end
|
||||||
|
|
||||||
|
# Should not cause excessive queries due to our optimization
|
||||||
|
assert queries_before < 10, "Expected fewer than 10 queries, got #{queries_before}"
|
||||||
|
end
|
||||||
|
|
||||||
|
test "allocated_spending uses includes to prevent N+1 queries" do
|
||||||
|
# Create some test categories and budget categories
|
||||||
|
category1 = @family.categories.create!(name: "Food", color: "#ff0000")
|
||||||
|
category2 = @family.categories.create!(name: "Transport", color: "#00ff00", parent: category1)
|
||||||
|
|
||||||
|
@budget.budget_categories.create!(category: category1, budgeted_spending: 1000, currency: "USD")
|
||||||
|
@budget.budget_categories.create!(category: category2, budgeted_spending: 500, currency: "USD")
|
||||||
|
|
||||||
|
# Count queries when calculating allocated spending
|
||||||
|
queries_before = count_queries do
|
||||||
|
@budget.allocated_spending
|
||||||
|
end
|
||||||
|
|
||||||
|
# Should not cause excessive queries due to our optimization
|
||||||
|
assert queries_before < 5, "Expected fewer than 5 queries, got #{queries_before}"
|
||||||
|
end
|
||||||
|
|
||||||
|
private
|
||||||
|
|
||||||
|
def count_queries(&block)
|
||||||
|
query_count = 0
|
||||||
|
subscriber = ActiveSupport::Notifications.subscribe("sql.active_record") do |*args|
|
||||||
|
query_count += 1
|
||||||
|
end
|
||||||
|
|
||||||
|
block.call
|
||||||
|
query_count
|
||||||
|
ensure
|
||||||
|
ActiveSupport::Notifications.unsubscribe(subscriber) if subscriber
|
||||||
|
end
|
||||||
|
end
|
Loading…
Add table
Add a link
Reference in a new issue