From dd75cadebcafe395eb241a43b9a4a0b06232924d Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Tue, 11 Mar 2025 15:38:45 -0400 Subject: [PATCH] Fix transaction filters when transfers are present (#1986) * Proper filtering of transfers in search * Fix transaction search --- app/helpers/account/entries_helper.rb | 20 ++++- app/models/account/entry_search.rb | 15 ---- app/models/account/transaction_search.rb | 99 ++++++++++++++++++------ app/views/transactions/index.html.erb | 7 +- 4 files changed, 96 insertions(+), 45 deletions(-) diff --git a/app/helpers/account/entries_helper.rb b/app/helpers/account/entries_helper.rb index 3350a328..af2f16f7 100644 --- a/app/helpers/account/entries_helper.rb +++ b/app/helpers/account/entries_helper.rb @@ -1,6 +1,24 @@ module Account::EntriesHelper def entries_by_date(entries, totals: false) - entries.group_by(&:date).map do |date, grouped_entries| + transfer_groups = entries.group_by do |entry| + # Only check for transfer if it's a transaction + next nil unless entry.entryable_type == "Account::Transaction" + entry.entryable.transfer&.id + end + + # For a more intuitive UX, we do not want to show the same transfer twice in the list + deduped_entries = transfer_groups.flat_map do |transfer_id, grouped_entries| + if transfer_id.nil? || grouped_entries.size == 1 + grouped_entries + else + grouped_entries.reject do |e| + e.entryable_type == "Account::Transaction" && + e.entryable.transfer_as_inflow.present? + end + end + end + + deduped_entries.group_by(&:date).map do |date, grouped_entries| content = capture do yield grouped_entries end diff --git a/app/models/account/entry_search.rb b/app/models/account/entry_search.rb index b6617037..b08c338f 100644 --- a/app/models/account/entry_search.rb +++ b/app/models/account/entry_search.rb @@ -31,20 +31,6 @@ class Account::EntrySearch query end - def apply_type_filter(scope, types) - return scope if types.blank? - - query = scope - - if types.include?("income") && !types.include?("expense") - query = query.where("account_entries.amount < 0") - elsif types.include?("expense") && !types.include?("income") - query = query.where("account_entries.amount >= 0") - end - - query - end - def apply_amount_filter(scope, amount, amount_operator) return scope if amount.blank? || amount_operator.blank? @@ -76,7 +62,6 @@ class Account::EntrySearch query = scope.joins(:account) query = self.class.apply_search_filter(query, search) query = self.class.apply_date_filters(query, start_date, end_date) - query = self.class.apply_type_filter(query, types) query = self.class.apply_amount_filter(query, amount, amount_operator) query = self.class.apply_accounts_filter(query, accounts, account_ids) query diff --git a/app/models/account/transaction_search.rb b/app/models/account/transaction_search.rb index 3cf927e8..215c6a98 100644 --- a/app/models/account/transaction_search.rb +++ b/app/models/account/transaction_search.rb @@ -14,39 +14,88 @@ class Account::TransactionSearch attribute :merchants, array: true attribute :tags, array: true - # Returns array of Account::Entry objects to stay consistent with partials, which only deal with Account::Entry def build_query(scope) query = scope.joins(entry: :account) + .joins(transfer_join) - if types.present? && types.exclude?("transfer") - query = query.joins("LEFT JOIN transfers ON transfers.inflow_transaction_id = account_entries.id OR transfers.outflow_transaction_id = account_entries.id") - .where("transfers.id IS NULL") - end - - if categories.present? - if categories.exclude?("Uncategorized") - query = query - .joins(:category) - .where(categories: { name: categories }) - else - query = query - .left_joins(:category) - .where(categories: { name: categories }) - .or(query.where(category_id: nil)) - end - end - - query = query.joins(:merchant).where(merchants: { name: merchants }) if merchants.present? - - query = query.joins(:tags).where(tags: { name: tags }) if tags.present? - - # Apply common entry search filters + query = apply_category_filter(query, categories) + query = apply_type_filter(query, types) + query = apply_merchant_filter(query, merchants) + query = apply_tag_filter(query, tags) query = Account::EntrySearch.apply_search_filter(query, search) query = Account::EntrySearch.apply_date_filters(query, start_date, end_date) - query = Account::EntrySearch.apply_type_filter(query, types) query = Account::EntrySearch.apply_amount_filter(query, amount, amount_operator) query = Account::EntrySearch.apply_accounts_filter(query, accounts, account_ids) query end + + private + def transfer_join + <<~SQL + LEFT JOIN ( + SELECT t.*, t.id as transfer_id, a.accountable_type + FROM transfers t + JOIN account_entries ae ON ae.entryable_id = t.inflow_transaction_id + AND ae.entryable_type = 'Account::Transaction' + JOIN accounts a ON a.id = ae.account_id + ) transfer_info ON ( + transfer_info.inflow_transaction_id = account_transactions.id OR + transfer_info.outflow_transaction_id = account_transactions.id + ) + SQL + end + + def apply_category_filter(query, categories) + return query unless categories.present? + + query = query.left_joins(:category).where( + "categories.name IN (?) OR ( + categories.id IS NULL AND (transfer_info.transfer_id IS NULL OR transfer_info.accountable_type = 'Loan') + )", + categories + ) + + if categories.exclude?("Uncategorized") + query = query.where.not(category_id: nil) + end + + query + end + + def apply_type_filter(query, types) + return query unless types.present? + return query if types.sort == [ "expense", "income", "transfer" ] + + transfer_condition = "transfer_info.transfer_id IS NOT NULL" + expense_condition = "account_entries.amount >= 0" + income_condition = "account_entries.amount <= 0" + + condition = case types.sort + when [ "transfer" ] + transfer_condition + when [ "expense" ] + Arel.sql("#{expense_condition} AND NOT (#{transfer_condition})") + when [ "income" ] + Arel.sql("#{income_condition} AND NOT (#{transfer_condition})") + when [ "expense", "transfer" ] + Arel.sql("#{expense_condition} OR #{transfer_condition}") + when [ "income", "transfer" ] + Arel.sql("#{income_condition} OR #{transfer_condition}") + when [ "expense", "income" ] + Arel.sql("NOT (#{transfer_condition})") + end + + query.where(condition) + end + + def apply_merchant_filter(query, merchants) + return query unless merchants.present? + query.joins(:merchant).where(merchants: { name: merchants }) + end + + def apply_tag_filter(query, tags) + return query unless tags.present? + query.joins(:tags).where(tags: { name: tags }) + end end diff --git a/app/views/transactions/index.html.erb b/app/views/transactions/index.html.erb index 35c99e4a..e6772607 100644 --- a/app/views/transactions/index.html.erb +++ b/app/views/transactions/index.html.erb @@ -1,4 +1,4 @@ -
+
<%= render "header" %> <%= render "summary", totals: @totals %> @@ -7,7 +7,7 @@ data-controller="bulk-select" data-bulk-select-singular-label-value="<%= t(".transaction") %>" data-bulk-select-plural-label-value="<%= t(".transactions") %>" - class="overflow-y-auto flex flex-col bg-white rounded-xl shadow-border-xs p-4"> + class="flex flex-col bg-white rounded-xl shadow-border-xs p-4"> <%= render "transactions/searches/search" %>
<%= entries_by_date(@transactions.map(&:entry), totals: true) do |entries| %> - <%# Only render the outflow side of transfers to avoid duplicate entries %> - <%= render partial: "account/entries/entry", collection: entries.reject { |e| e.entryable.transfer_as_inflow.present? } %> + <%= render entries %> <% end %>