1
0
Fork 0
mirror of https://github.com/maybe-finance/maybe.git synced 2025-08-10 07:55:21 +02:00

Cleanup comments, logic in search

This commit is contained in:
Zach Gollwitzer 2025-06-18 17:51:36 -04:00
parent d0ca554065
commit 9a3ab9b9a3
6 changed files with 47 additions and 59 deletions

View file

@ -5,7 +5,7 @@ class IncomeStatement::CategoryStats
end
def call
ActiveRecord::Base.connection.select_all(query_sql).map do |row|
ActiveRecord::Base.connection.select_all(sanitized_query_sql).map do |row|
StatRow.new(
category_id: row["category_id"],
classification: row["classification"],
@ -18,16 +18,18 @@ class IncomeStatement::CategoryStats
private
StatRow = Data.define(:category_id, :classification, :median, :avg)
def query_sql
def sanitized_query_sql
ActiveRecord::Base.sanitize_sql_array([
optimized_query_sql,
sql_params
query_sql,
{
target_currency: @family.currency,
interval: @interval,
family_id: @family.id
}
])
end
# OPTIMIZED: Use interval for time bucketing but eliminate unnecessary intermediate CTE
# Still faster than original due to simplified structure and kind filtering
def optimized_query_sql
def query_sql
<<~SQL
WITH period_totals AS (
SELECT
@ -57,12 +59,4 @@ class IncomeStatement::CategoryStats
GROUP BY category_id, classification;
SQL
end
def sql_params
{
target_currency: @family.currency,
interval: @interval,
family_id: @family.id
}
end
end

View file

@ -5,7 +5,7 @@ class IncomeStatement::FamilyStats
end
def call
ActiveRecord::Base.connection.select_all(query_sql).map do |row|
ActiveRecord::Base.connection.select_all(sanitized_query_sql).map do |row|
StatRow.new(
classification: row["classification"],
median: row["median"],
@ -17,16 +17,18 @@ class IncomeStatement::FamilyStats
private
StatRow = Data.define(:classification, :median, :avg)
def query_sql
def sanitized_query_sql
ActiveRecord::Base.sanitize_sql_array([
optimized_query_sql,
sql_params
query_sql,
{
target_currency: @family.currency,
interval: @interval,
family_id: @family.id
}
])
end
# OPTIMIZED: Use interval for time bucketing but eliminate double CTE
# Single CTE instead of base_totals -> aggregated_totals -> final aggregation
def optimized_query_sql
def query_sql
<<~SQL
WITH period_totals AS (
SELECT
@ -53,12 +55,4 @@ class IncomeStatement::FamilyStats
GROUP BY classification;
SQL
end
def sql_params
{
target_currency: @family.currency,
interval: @interval,
family_id: @family.id
}
end
end

View file

@ -17,8 +17,6 @@ class Transaction < ApplicationRecord
one_time: "one_time" # A one-time expense/income, excluded from budget analytics
}
def set_category!(category)
if category.is_a?(String)
category = entry.account.family.categories.find_or_create_by!(

View file

@ -66,9 +66,10 @@ class Transaction::Search
def apply_category_filter(query, categories)
return query unless categories.present?
non_budget_kinds = %w[transfer payment one_time]
query = query.left_joins(:category).where(
"categories.name IN (?) OR (
categories.id IS NULL AND (transactions.kind NOT IN ('transfer', 'payment', 'one_time') OR transactions.kind = 'loan_payment')
categories.id IS NULL AND (transactions.kind NOT IN ('transfer', 'payment'))
)",
categories
)
@ -84,7 +85,7 @@ class Transaction::Search
return query unless types.present?
return query if types.sort == [ "expense", "income", "transfer" ]
transfer_condition = "transactions.kind IN ('transfer', 'payment', 'one_time')"
transfer_condition = "transactions.kind IN ('transfer', 'payment', 'loan_payment')"
expense_condition = "entries.amount >= 0"
income_condition = "entries.amount <= 0"
@ -92,15 +93,15 @@ class Transaction::Search
when [ "transfer" ]
transfer_condition
when [ "expense" ]
Arel.sql("(#{expense_condition} AND transactions.kind NOT IN ('transfer', 'payment', 'one_time')) OR transactions.kind = 'loan_payment'")
Arel.sql("#{expense_condition} AND NOT (#{transfer_condition})")
when [ "income" ]
Arel.sql("#{income_condition} AND transactions.kind NOT IN ('transfer', 'payment', 'one_time', 'loan_payment')")
Arel.sql("#{income_condition} AND NOT (#{transfer_condition})")
when [ "expense", "transfer" ]
Arel.sql("(#{expense_condition} AND transactions.kind NOT IN ('transfer', 'payment', 'one_time')) OR transactions.kind = 'loan_payment' OR #{transfer_condition}")
Arel.sql("#{expense_condition} OR #{transfer_condition}")
when [ "income", "transfer" ]
Arel.sql("(#{income_condition} AND transactions.kind NOT IN ('transfer', 'payment', 'one_time', 'loan_payment')) OR #{transfer_condition}")
Arel.sql("#{income_condition} OR #{transfer_condition}")
when [ "expense", "income" ]
Arel.sql("transactions.kind NOT IN ('transfer', 'payment', 'one_time') OR transactions.kind = 'loan_payment'")
Arel.sql("NOT (#{transfer_condition})")
end
query.where(condition)

View file

@ -10,20 +10,18 @@ class Transaction::Totals
end
def call
result = execute_query.first
ScopeTotals.new(
transactions_count: result["transactions_count"].to_i,
income_money: Money.new(result["income_total"].to_i, result["currency"]),
expense_money: Money.new(result["expense_total"].to_i, result["currency"])
transactions_count: query_result["transactions_count"].to_i,
income_money: Money.new(query_result["income_total"].to_i, query_result["currency"]),
expense_money: Money.new(query_result["expense_total"].to_i, query_result["currency"])
)
end
private
ScopeTotals = Data.define(:transactions_count, :income_money, :expense_money)
def execute_query
ActiveRecord::Base.connection.select_all(sanitized_query)
def query_result
ActiveRecord::Base.connection.select_all(sanitized_query).first
end
def sanitized_query

View file

@ -43,25 +43,27 @@ class Transaction::SearchTest < ActiveSupport::TestCase
kind: "one_time"
)
# Test transfer type filter
# Test transfer type filter (includes loan_payment)
transfer_results = Transaction::Search.new(@family, filters: { types: [ "transfer" ] }).relation
transfer_ids = transfer_results.pluck(:id)
assert_includes transfer_ids, transfer_entry.entryable.id
assert_includes transfer_ids, payment_entry.entryable.id
assert_includes transfer_ids, one_time_entry.entryable.id
assert_includes transfer_ids, loan_payment_entry.entryable.id
assert_not_includes transfer_ids, one_time_entry.entryable.id
assert_not_includes transfer_ids, standard_entry.entryable.id
assert_not_includes transfer_ids, loan_payment_entry.entryable.id
# Test expense type filter (should include loan_payment)
# Test expense type filter (excludes transfer kinds but includes one_time)
expense_results = Transaction::Search.new(@family, filters: { types: [ "expense" ] }).relation
expense_ids = expense_results.pluck(:id)
assert_includes expense_ids, standard_entry.entryable.id
assert_includes expense_ids, loan_payment_entry.entryable.id
assert_includes expense_ids, one_time_entry.entryable.id
assert_not_includes expense_ids, loan_payment_entry.entryable.id
assert_not_includes expense_ids, transfer_entry.entryable.id
assert_not_includes expense_ids, payment_entry.entryable.id
assert_not_includes expense_ids, one_time_entry.entryable.id
# Test income type filter
income_entry = create_transaction(
@ -78,16 +80,16 @@ class Transaction::SearchTest < ActiveSupport::TestCase
assert_not_includes income_ids, loan_payment_entry.entryable.id
assert_not_includes income_ids, transfer_entry.entryable.id
# Test combined expense and income filter (excludes transfers)
# Test combined expense and income filter (excludes transfer kinds but includes one_time)
non_transfer_results = Transaction::Search.new(@family, filters: { types: [ "expense", "income" ] }).relation
non_transfer_ids = non_transfer_results.pluck(:id)
assert_includes non_transfer_ids, standard_entry.entryable.id
assert_includes non_transfer_ids, income_entry.entryable.id
assert_includes non_transfer_ids, loan_payment_entry.entryable.id
assert_includes non_transfer_ids, one_time_entry.entryable.id
assert_not_includes non_transfer_ids, loan_payment_entry.entryable.id
assert_not_includes non_transfer_ids, transfer_entry.entryable.id
assert_not_includes non_transfer_ids, payment_entry.entryable.id
assert_not_includes non_transfer_ids, one_time_entry.entryable.id
end
test "search category filter handles uncategorized transactions correctly with kind filtering" do
@ -114,8 +116,9 @@ class Transaction::SearchTest < ActiveSupport::TestCase
uncategorized_results = Transaction::Search.new(@family, filters: { categories: [ "Uncategorized" ] }).relation
uncategorized_ids = uncategorized_results.pluck(:id)
# Should include standard and loan_payment (budget-relevant) uncategorized transactions
# Should include standard uncategorized transactions
assert_includes uncategorized_ids, uncategorized_standard.entryable.id
# Should include loan_payment since it's treated specially in category logic
assert_includes uncategorized_ids, uncategorized_loan_payment.entryable.id
# Should exclude transfer transactions even if uncategorized
@ -147,9 +150,9 @@ class Transaction::SearchTest < ActiveSupport::TestCase
# Should exclude transfer transactions
assert_not_includes result_ids, transaction2.entryable.id
# Test that the relation builds from family.transactions.active
assert_equal @family.transactions.active.joins(entry: :account).where(
"(entries.amount >= 0 AND transactions.kind NOT IN ('transfer', 'payment', 'one_time')) OR transactions.kind = 'loan_payment'"
# Test that the relation builds from family.transactions correctly
assert_equal @family.transactions.joins(entry: :account).where(
"entries.amount >= 0 AND NOT (transactions.kind IN ('transfer', 'payment', 'loan_payment'))"
).count, results.count
end