filamentphp / filament

A collection of beautiful full-stack components for Laravel. The perfect starting point for your next app. Using Livewire, Alpine.js and Tailwind CSS.
https://filamentphp.com
MIT License
19.09k stars 2.94k forks source link

HasRecords::resolveTableRecord doesn't work with orWhere in a filter #14179

Closed dcaswel closed 1 month ago

dcaswel commented 2 months ago

Package

filament/filament

Package Version

v3.2.110

Laravel Version

v11.21.0

Livewire Version

v3.5.6

PHP Version

PHP 8.3.10

Problem description

I have a CopyResource with a table that has multiple actions on each row and the ListCopies page has a few tabs that filter what is being shown in the table. If I activate one of the tabs that uses orWhere then click on an action button in one of the rows at the bottom, it will perform the action on the wrong copy.

Expected behavior

With the setup described in the problem description, clicking on the action button should perform the action on the record that had the button.

Steps to reproduce

  1. Create a model with a status column (Copy)
  2. Create a Resource (CopyResource)
  3. In the table add an action button similar to:
    Action::make('post)->action(fn($record) => $record->post())->button();
  4. In the List page (ListCopies) implement the getTabs() method and return a tab that uses orWhere similar to:
    Tab::make('Purchased')
    ->modifyQueryUsing(fn($query) => $query->where('status', 'purchased')->orWhere('status', 'available'))
  5. Create a bunch of records that have one of those 2 statuses
  6. Go to the resource page and click on the tab from step 4
  7. Click on the action button at the bottom of the table
  8. This will run the action on a different record

The attached repo has a README with steps for reproducing this in that project as well as some information I found on a partial cause for the issue.

Reproduction repository (issue will be closed if this is not valid)

https://github.com/dcaswel/filament-issue

Relevant log output

No response

Donate 💰 to fund this issue

Fund with Polar

hamidroohani commented 2 months ago

I realized that when you use orWhere in the tabs, a CancelException is triggered, which causes an unmountTableAction. However, I couldn't find where this exception is being triggered.

https://github.com/filamentphp/filament/blob/3.x/packages/tables/src/Concerns/HasActions.php#L219-L223

dcaswel commented 2 months ago

As far as I can tell, it's not throwing any exceptions. It just passes the wrong record into the action. So the wrong record gets updated.

jackwh commented 1 month ago

@dcaswel I've been tripped up by this before... Just thinking aloud as I haven't tested, but does the problem go away if you change the query like this?

Tab::make('Purchased')
    ->modifyQueryUsing(fn($query) => $query->where(
        fn($query) => $query->where('status', 'purchased')->orWhere('status', 'available')
    ))

The Laravel docs warn orWhere clauses should always be placed in logical groups. Without doing so, the orWhere you've added might be cancelling out an internal where clause added by Filament.

zepfietje commented 1 month ago

Was about to comment with the approach of @JackWH. I think that should solve the issue, @dcaswel.

Is there anything we can do in Filament to make the ungrouped where calls work, @danharrin?

dcaswel commented 1 month ago

I actually fixed my issue by just changing it to a whereIn('status', ['purchased', 'available']) but I wanted to open this issue because that won't always work. The solution above does work as well. I didn't think about needing to contain it in a where but it would be good if there was a way for Filament to prevent this if possible or maybe put a warning on the documentation for the modify query stuff.

zepfietje commented 1 month ago

Glad you managed to fix it for your case, @dcaswel! I'm going to close this issue as it's not really a bug, but if Dan knows of a way to handle this in Filament (which I'm not sure of) we'll improve this in core.

danharrin commented 1 month ago

If we automatically group wheres, it will break global scope removal functionality, which cannot be used inside a nested where function.