From af87ac75600060939b7c27e99548bace462a0d73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Falc=C3=B3n?= Date: Fri, 5 Jun 2026 17:11:02 +0200 Subject: [PATCH] fix(transactions): combine category and label filters with OR (#495) ## Problem On the transactions page, the **Category** and **Labels** filters were combined with **AND**. Selecting both a category and a label only showed transactions that matched the category *and* carried that label. ## Fix In `Transaction::scopeApplyFilters`, both filters are now wrapped in a single `where` group with the label condition using `orWhereHas`, so they combine as **OR**: a transaction matches if it's in a selected category **or** has a selected label. - Multi-category OR, category-tree expansion, and the *uncategorized* option are preserved. - Other filters (date, amount, account, search) remain AND. ## Tests Added `category and label filters combine with OR` to `TransactionFilterTest`. All 34 filter + bulk-update tests pass; pint clean. --- app/Models/Transaction.php | 48 ++++++++++++++++--------- tests/Feature/TransactionFilterTest.php | 35 ++++++++++++++++++ 2 files changed, 66 insertions(+), 17 deletions(-) diff --git a/app/Models/Transaction.php b/app/Models/Transaction.php index dd8d249d..7363c09a 100644 --- a/app/Models/Transaction.php +++ b/app/Models/Transaction.php @@ -132,25 +132,43 @@ class Transaction extends Model $query->where('amount', '<=', $filters['amount_max'] * 100); } - if (! empty($filters['category_ids'])) { - $ids = collect($filters['category_ids']); - $hasUncategorized = $ids->contains('uncategorized'); - $realIds = $ids->reject(fn ($id) => $id === 'uncategorized')->values()->all(); + $hasCategoryFilter = ! empty($filters['category_ids']); + $hasLabelFilter = ! empty($filters['label_ids']); - if ($realIds !== []) { - $userId = $filters['user_id'] ?? Category::query()->whereIn('id', $realIds)->value('user_id'); + if ($hasCategoryFilter || $hasLabelFilter) { + $realIds = []; + $hasUncategorized = false; - if ($userId !== null) { - $realIds = app(CategoryTree::class)->expand($userId, $realIds); + if ($hasCategoryFilter) { + $ids = collect($filters['category_ids']); + $hasUncategorized = $ids->contains('uncategorized'); + $realIds = $ids->reject(fn ($id) => $id === 'uncategorized')->values()->all(); + + if ($realIds !== []) { + $userId = $filters['user_id'] ?? Category::query()->whereIn('id', $realIds)->value('user_id'); + + if ($userId !== null) { + $realIds = app(CategoryTree::class)->expand($userId, $realIds); + } } } - $query->where(function (Builder $q) use ($realIds, $hasUncategorized) { - if (! empty($realIds)) { - $q->whereIn('category_id', $realIds); + $labelIds = $filters['label_ids'] ?? []; + + $query->where(function (Builder $outer) use ($hasCategoryFilter, $realIds, $hasUncategorized, $hasLabelFilter, $labelIds) { + if ($hasCategoryFilter) { + $outer->where(function (Builder $q) use ($realIds, $hasUncategorized) { + if (! empty($realIds)) { + $q->whereIn('category_id', $realIds); + } + if ($hasUncategorized) { + $q->orWhereNull('category_id'); + } + }); } - if ($hasUncategorized) { - $q->orWhereNull('category_id'); + + if ($hasLabelFilter) { + $outer->orWhereHas('labels', fn (Builder $q) => $q->whereIn('labels.id', $labelIds)); } }); } @@ -159,10 +177,6 @@ class Transaction extends Model $query->whereIn('account_id', $filters['account_ids']); } - if (! empty($filters['label_ids'])) { - $query->whereHas('labels', fn (Builder $q) => $q->whereIn('labels.id', $filters['label_ids'])); - } - if (! empty($filters['creditor_name'])) { $term = '%'.$filters['creditor_name'].'%'; $query->where('creditor_name', 'LIKE', $term); diff --git a/tests/Feature/TransactionFilterTest.php b/tests/Feature/TransactionFilterTest.php index 3b2bcf6f..0b2a2e92 100644 --- a/tests/Feature/TransactionFilterTest.php +++ b/tests/Feature/TransactionFilterTest.php @@ -215,6 +215,41 @@ test('filter by label', function () { ); }); +test('category and label filters combine with OR', function () { + $category = Category::factory()->create(['user_id' => $this->user->id]); + $label = Label::factory()->create(['user_id' => $this->user->id]); + + $txWithCategory = Transaction::factory()->plaintext()->create([ + 'user_id' => $this->user->id, + 'account_id' => $this->account->id, + 'category_id' => $category->id, + ]); + + $txWithLabel = Transaction::factory()->plaintext()->create([ + 'user_id' => $this->user->id, + 'account_id' => $this->account->id, + 'category_id' => null, + ]); + $txWithLabel->labels()->attach($label->id); + + Transaction::factory()->plaintext()->create([ + 'user_id' => $this->user->id, + 'account_id' => $this->account->id, + 'category_id' => null, + ]); + + $response = actingAs($this->user)->get(route('transactions.index', [ + 'category_ids' => $category->id, + 'label_ids' => $label->id, + ])); + + $ids = collect($response->viewData('page')['props']['transactions']['data'])->pluck('id'); + + expect($ids)->toHaveCount(2) + ->and($ids)->toContain($txWithCategory->id) + ->and($ids)->toContain($txWithLabel->id); +}); + test('search matches description', function () { Transaction::factory()->plaintext()->create([ 'user_id' => $this->user->id,