feat(products): filter products by category and unit#592
Conversation
Closes InvoicePlane#390 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds two SelectFilter instances (category_id, unit_id) to the Products table, populated from tenant-scoped ProductCategory and ProductUnit queries, replacing the previously empty filters configuration. Adds a new feature test class validating filtering behavior and tenant scoping of filter options. ChangesProduct table filters
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ListProducts
participant ProductsTable
participant ProductCategory
participant ProductUnit
User->>ListProducts: open products table
ListProducts->>ProductsTable: render filters
ProductsTable->>ProductCategory: query ordered names, pluck id => name
ProductsTable->>ProductUnit: query ordered names, pluck id => name
User->>ListProducts: select category_id/unit_id filter
ListProducts->>ProductsTable: apply filter to query
ProductsTable-->>ListProducts: filtered product rows
Related issues: Suggested labels: enhancement, products, filament Suggested reviewers: none identified 🐰 A carrot filter, snug and neat, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
📋 Issue PlannerLet us write the prompt for your AI agent so you can ship faster (with fewer bugs). View plan for ticket: ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
Modules/Products/Tests/Feature/ProductCategoryFilterTest.php (2)
90-90: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse imported
Companyclass instead of fully-qualified inline reference.
\Modules\Core\Models\Company::factory()could use a top-leveluseimport consistent with the rest of the file's style.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Products/Tests/Feature/ProductCategoryFilterTest.php` at line 90, Replace the fully qualified Company reference in the product category filter test with the imported Company model to match the file’s existing style. Update the setup in ProductCategoryFilterTest to use the top-level Company import where the factory is called, keeping the test code consistent with the other model references in this class.
22-49: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCombine
/* Act */and/* Assert */into separate blocks per test-structure guideline.Both tests merge assertions and actions under a single
/* Act + Assert */comment. As per coding guidelines, "Structure tests with /* Arrange /, / Act /, and / Assert */ blocks; define all variables in the Act section before asserting."✏️ Suggested restructuring (test 1 shown, apply similarly to test 2)
- /* Act + Assert */ - Livewire::actingAs($this->user) + /* Act */ + $component = Livewire::actingAs($this->user) ->test(ListProducts::class, ['tenant' => Str::lower($this->company->search_code)]) - ->assertSuccessful() - ->assertCanSeeTableRecords([$hardwareProduct, $softwareProduct]) - ->filterTable('category_id', $hardware->id) - ->assertCanSeeTableRecords([$hardwareProduct]) - ->assertCanNotSeeTableRecords([$softwareProduct]); + ->filterTable('category_id', $hardware->id); + + /* Assert */ + $component->assertSuccessful() + ->assertCanSeeTableRecords([$hardwareProduct]) + ->assertCanNotSeeTableRecords([$softwareProduct]);Also applies to: 53-80
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Products/Tests/Feature/ProductCategoryFilterTest.php` around lines 22 - 49, The test in ProductCategoryFilterTest::it_filters_products_by_category currently mixes actions and assertions under a single Act + Assert section; split the flow into distinct /* Arrange */, /* Act */, and /* Assert */ blocks. Move the Livewire::actingAs(...)->test(...)->filterTable(...) interaction into the Act block, then place the visibility assertions in the Assert block, and apply the same restructuring to the other test in this file as well.Source: Path instructions
Modules/Products/Filament/Company/Resources/Products/Tables/ProductsTable.php (1)
65-78: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
relationship()for the category/unit filters
ProductCategoryandProductUnitalready carry the company scope, so these filters can be simplified to Filament relationships and the manual option queries/imports can go away.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Products/Filament/Company/Resources/Products/Tables/ProductsTable.php` around lines 65 - 78, The category and unit filters in ProductsTable are manually querying options even though ProductCategory and ProductUnit already have the company scope and can be handled through Filament relationships. Update the SelectFilter definitions for category_id and unit_id to use relationship() instead of options(), and remove the now-unneeded ProductCategory/ProductUnit option query logic and any related imports so the filters rely on the model relationships directly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Modules/Products/Tests/Feature/ProductCategoryFilterTest.php`:
- Around line 87-93: The test setup in ProductCategoryFilterTest creates
`$ownCategory` and `$foreignCategory` but the assertions still use hard-coded
category names, so the variables are unused. Update the relevant assertions in
the test methods that use these fixtures to reference the models’
`category_name` values instead of literals, using the existing `ProductCategory`
variables so the test stays aligned with the seeded data and static analysis no
longer flags them.
---
Nitpick comments:
In
`@Modules/Products/Filament/Company/Resources/Products/Tables/ProductsTable.php`:
- Around line 65-78: The category and unit filters in ProductsTable are manually
querying options even though ProductCategory and ProductUnit already have the
company scope and can be handled through Filament relationships. Update the
SelectFilter definitions for category_id and unit_id to use relationship()
instead of options(), and remove the now-unneeded ProductCategory/ProductUnit
option query logic and any related imports so the filters rely on the model
relationships directly.
In `@Modules/Products/Tests/Feature/ProductCategoryFilterTest.php`:
- Line 90: Replace the fully qualified Company reference in the product category
filter test with the imported Company model to match the file’s existing style.
Update the setup in ProductCategoryFilterTest to use the top-level Company
import where the factory is called, keeping the test code consistent with the
other model references in this class.
- Around line 22-49: The test in
ProductCategoryFilterTest::it_filters_products_by_category currently mixes
actions and assertions under a single Act + Assert section; split the flow into
distinct /* Arrange */, /* Act */, and /* Assert */ blocks. Move the
Livewire::actingAs(...)->test(...)->filterTable(...) interaction into the Act
block, then place the visibility assertions in the Assert block, and apply the
same restructuring to the other test in this file as well.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: da293fd8-9c84-4e24-ab29-810d68f200f4
📒 Files selected for processing (2)
Modules/Products/Filament/Company/Resources/Products/Tables/ProductsTable.phpModules/Products/Tests/Feature/ProductCategoryFilterTest.php
| $ownCategory = ProductCategory::factory()->for($this->company)->create([ | ||
| 'category_name' => 'Own Category', | ||
| ]); | ||
| $companyB = \Modules\Core\Models\Company::factory()->create(); | ||
| $foreignCategory = ProductCategory::factory()->for($companyB)->create([ | ||
| 'category_name' => 'Foreign Category', | ||
| ]); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Unused variables $ownCategory/$foreignCategory flagged by static analysis; use them instead of hard-coded strings.
Static analysis reports both variables as unused since the assertions reference literal strings rather than the created models' attributes, which also weakens the test's coupling to the actual seeded data.
🧹 Suggested fix
- $this->assertContains('Own Category', $options);
- $this->assertNotContains('Foreign Category', $options);
+ $this->assertContains($ownCategory->category_name, $options);
+ $this->assertNotContains($foreignCategory->category_name, $options);Also applies to: 106-107
🧰 Tools
🪛 PHPMD (2.15.0)
[warning] 87-87: Avoid unused local variables such as '$ownCategory'. (undefined)
(UnusedLocalVariable)
[warning] 91-91: Avoid unused local variables such as '$foreignCategory'. (undefined)
(UnusedLocalVariable)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Modules/Products/Tests/Feature/ProductCategoryFilterTest.php` around lines 87
- 93, The test setup in ProductCategoryFilterTest creates `$ownCategory` and
`$foreignCategory` but the assertions still use hard-coded category names, so
the variables are unused. Update the relevant assertions in the test methods
that use these fixtures to reference the models’ `category_name` values instead
of literals, using the existing `ProductCategory` variables so the test stays
aligned with the seeded data and static analysis no longer flags them.
Source: Linters/SAST tools
1 participant
Summary
Adds two dropdown filters to the company-panel products table (
Modules/Products/Filament/Company/Resources/Products/Tables/ProductsTable.php):SelectFilteroncategory_id, options fromProductCategory(ordered by name)SelectFilteronunit_id, options fromProductUnit(ordered by name)Both option lists are tenant-scoped automatically by the
BelongsToCompanyglobal scope, so a company only ever sees its own categories/units. Same pattern as the numbering filter on the invoices table (PR #581).Closes
Closes #390 — products can be filtered by category (and by unit, same pattern, added while in there)
Test plan
Modules/Products/Tests/Feature/ProductCategoryFilterTest.php(3 tests, 15 assertions — all passing locally against MariaDB):Note: the local test run needed the permission-seeding test fixes from
origin/develop(fix(tests): seed roles/permissions in panel test base classes) which are not onupstream/developyet — without them every company-panel Livewire test 403s. The feature itself has no dependency on them.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes