From 33df3b781ea941c2bd8818bb164d34456163ae68 Mon Sep 17 00:00:00 2001 From: charsel <1127835+Charsel@users.noreply.github.com> Date: Tue, 27 Jan 2026 12:28:33 +0100 Subject: [PATCH] fix: Handle uncategorized transactions filter correctly (#802) * fix: Handle uncategorized transactions filter correctly When filtering for 'Uncategorized' transactions, the filter was not working because 'Uncategorized' is a virtual category (Category.uncategorized returns a non-persisted Category object) and does not exist in the database. The filter was attempting to match 'categories.name IN (Uncategorized)' which returned zero results. This fix removes 'Uncategorized' from the category names array before querying the database, allowing the existing 'category_id IS NULL' condition to work correctly. Fixes filtering for uncategorized transactions while maintaining backward compatibility with all other category filters. * test: Add comprehensive tests for Uncategorized filter - Test filtering for only uncategorized transactions - Test combining uncategorized with real categories - Test excluding uncategorized when not in filter - Ensures fix prevents regression * refactor: Use Category.uncategorized.name for i18n support - Replace hard-coded 'Uncategorized' string with Category.uncategorized.name - Conditionally build SQL query based on include_uncategorized flag - Avoid adding category_id IS NULL clause when not needed - Update tests to use Category.uncategorized.name for consistency - Cleaner logic: only include uncategorized condition when requested Addresses code review feedback on i18n support and query optimization. * test: Fix travel category fixture error Create travel category dynamically instead of using non-existent fixture * style: Fix rubocop spacing in array brackets --------- Co-authored-by: Charsel --- app/models/transaction/search.rb | 44 ++++++++----- test/models/transaction/search_test.rb | 86 +++++++++++++++++++++++++- 2 files changed, 112 insertions(+), 18 deletions(-) diff --git a/app/models/transaction/search.rb b/app/models/transaction/search.rb index 242527c16..287dae467 100644 --- a/app/models/transaction/search.rb +++ b/app/models/transaction/search.rb @@ -102,28 +102,38 @@ class Transaction::Search def apply_category_filter(query, categories) return query unless categories.present? + # Remove "Uncategorized" from category names to query the database + uncategorized_name = Category.uncategorized.name + include_uncategorized = categories.include?(uncategorized_name) + real_categories = categories - [ uncategorized_name ] + # Get parent category IDs for the given category names - parent_category_ids = family.categories.where(name: categories).pluck(:id) + parent_category_ids = family.categories.where(name: real_categories).pluck(:id) + + uncategorized_condition = "(categories.id IS NULL AND transactions.kind NOT IN ('funds_movement', 'cc_payment'))" # Build condition based on whether parent_category_ids is empty if parent_category_ids.empty? - query = query.left_joins(:category).where( - "categories.name IN (?) OR ( - categories.id IS NULL AND (transactions.kind NOT IN ('funds_movement', 'cc_payment')) - )", - categories - ) + if include_uncategorized + query = query.left_joins(:category).where( + "categories.name IN (?) OR #{uncategorized_condition}", + real_categories.presence || [] + ) + else + query = query.left_joins(:category).where(categories: { name: real_categories }) + end else - query = query.left_joins(:category).where( - "categories.name IN (?) OR categories.parent_id IN (?) OR ( - categories.id IS NULL AND (transactions.kind NOT IN ('funds_movement', 'cc_payment')) - )", - categories, parent_category_ids - ) - end - - if categories.exclude?("Uncategorized") - query = query.where.not(category_id: nil) + if include_uncategorized + query = query.left_joins(:category).where( + "categories.name IN (?) OR categories.parent_id IN (?) OR #{uncategorized_condition}", + real_categories, parent_category_ids + ) + else + query = query.left_joins(:category).where( + "categories.name IN (?) OR categories.parent_id IN (?)", + real_categories, parent_category_ids + ) + end end query diff --git a/test/models/transaction/search_test.rb b/test/models/transaction/search_test.rb index bdfb264ac..164398d5a 100644 --- a/test/models/transaction/search_test.rb +++ b/test/models/transaction/search_test.rb @@ -114,7 +114,7 @@ class Transaction::SearchTest < ActiveSupport::TestCase ) # Search for uncategorized transactions - uncategorized_results = Transaction::Search.new(@family, filters: { categories: [ "Uncategorized" ] }).transactions_scope + uncategorized_results = Transaction::Search.new(@family, filters: { categories: [ Category.uncategorized.name ] }).transactions_scope uncategorized_ids = uncategorized_results.pluck(:id) # Should include standard uncategorized transactions @@ -126,6 +126,90 @@ class Transaction::SearchTest < ActiveSupport::TestCase assert_not_includes uncategorized_ids, uncategorized_transfer.entryable.id end + test "filtering for only Uncategorized returns only uncategorized transactions" do + # Create a mix of categorized and uncategorized transactions + categorized = create_transaction( + account: @checking_account, + amount: 100, + category: categories(:food_and_drink) + ) + + uncategorized = create_transaction( + account: @checking_account, + amount: 200 + ) + + # Filter for only uncategorized + results = Transaction::Search.new(@family, filters: { categories: [ Category.uncategorized.name ] }).transactions_scope + result_ids = results.pluck(:id) + + # Should only include uncategorized transaction + assert_includes result_ids, uncategorized.entryable.id + assert_not_includes result_ids, categorized.entryable.id + assert_equal 1, result_ids.size + end + + test "filtering for Uncategorized plus a real category returns both" do + # Create a travel category for testing + travel_category = @family.categories.create!( + name: "Travel", + color: "#3b82f6", + classification: "expense" + ) + + # Create transactions with different categories + food_transaction = create_transaction( + account: @checking_account, + amount: 100, + category: categories(:food_and_drink) + ) + + travel_transaction = create_transaction( + account: @checking_account, + amount: 150, + category: travel_category + ) + + uncategorized = create_transaction( + account: @checking_account, + amount: 200 + ) + + # Filter for food category + uncategorized + results = Transaction::Search.new(@family, filters: { categories: [ "Food & Drink", Category.uncategorized.name ] }).transactions_scope + result_ids = results.pluck(:id) + + # Should include both food and uncategorized + assert_includes result_ids, food_transaction.entryable.id + assert_includes result_ids, uncategorized.entryable.id + # Should NOT include travel + assert_not_includes result_ids, travel_transaction.entryable.id + assert_equal 2, result_ids.size + end + + test "filtering excludes uncategorized when not in filter" do + # Create a mix of transactions + categorized = create_transaction( + account: @checking_account, + amount: 100, + category: categories(:food_and_drink) + ) + + uncategorized = create_transaction( + account: @checking_account, + amount: 200 + ) + + # Filter for only food category (without Uncategorized) + results = Transaction::Search.new(@family, filters: { categories: [ "Food & Drink" ] }).transactions_scope + result_ids = results.pluck(:id) + + # Should only include categorized transaction + assert_includes result_ids, categorized.entryable.id + assert_not_includes result_ids, uncategorized.entryable.id + assert_equal 1, result_ids.size + end + test "new family-based API works correctly" do # Create transactions for testing transaction1 = create_transaction(