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 <charsel@charsel.com>
This commit is contained in:
charsel
2026-01-27 12:28:33 +01:00
committed by GitHub
parent 51c7f7a3f0
commit 33df3b781e
2 changed files with 112 additions and 18 deletions

View File

@@ -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

View File

@@ -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(