Fix transaction filters when transfers are present (#1986)

* Proper filtering of transfers in search

* Fix transaction search

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

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

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

app/views/transactions/index.html.erb

@@ -1,4 +1,4 @@
-<div class="space-y-4 h-fit max-h-full flex flex-col" data-controller="focus-record" data-focus-record-id-value="<%= @focused_record ? dom_id(@focused_record) : nil %>">
+<div class="space-y-4 pb-20 flex flex-col" data-controller="focus-record" data-focus-record-id-value="<%= @focused_record ? dom_id(@focused_record) : nil %>">
   <%= 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" %>
 
     <div id="entry-selection-bar" data-bulk-select-target="selectionBar" class="flex justify-center hidden">
@@ -29,8 +29,7 @@
         </div>
         <div class="space-y-6">
           <%= 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 %>
         </div>
       </div>