fix(tables): reset stale action records before checking row ActionGroup visibility - #20319
Conversation
…up visibility Table::getAction() sets the mounted record directly on whichever specific action was clicked (not just on the parent ActionGroup). Because InteractsWithRecord::getRecord() checks an action's own record before falling back to its group, that action keeps using the clicked row's record for the rest of the request - including when Livewire re-renders the whole table to show the resulting confirmation modal. Every other row rendered afterwards then evaluates that action's visible()/hidden() closures, and even its rendered click handler, against the wrong record. Reset each child action's record to the group's current (correct, per-row) record before checking visibility, in both the dropdown-trigger check (tables/resources/views/components/actions.blade.php) and the dropdown-content check (actions/resources/views/components/group.blade.php). Fixes filamentphp#16282.
danharrin
left a comment
There was a problem hiding this comment.
Thanks. Your diagnosis of the root cause is correct. However, this isn't a backport of the 4.x fix, it's a new workaround at the view layer, and I don't want to merge it in this form.
4.x had this exact bug too. It was fixed in 486a4b3, which stops binding the mounted record to the child action entirely, it binds only the root ActionGroup (falling back to the action itself when ungrouped):
$resolvedAction->getRootGroup()?->record($record) ?? $resolvedAction->record($record);Since InteractsWithRecord::getRecord() already falls back to the group, the child never needs its own record, so there's nothing stale to leak into other rows. Resetting records inside the Blade views on every render mutates shared action objects during rendering, the same fragile pattern that caused the bug, duplicates logic across two views, and only covers code paths that happen to go through those templates.
I think the equivalent change for 3.x is in Filament\Tables\Table\Concerns\HasActions::getAction():
if (($actionGroup = $action->getRootGroup()) instanceof HasRecord) {
$actionGroup->record($mountedRecord);
} else {
$action->record($mountedRecord);
}
return $this->getMountableModalActionFromAction(
$action,
modalActionNames: $modalActionNames ?? [],
mountedRecord: $mountedRecord,
);I have not tested this code myself, so it is definitely worth testing manually when you make this change for a variety of scenarios and please confirm to me that each of these continue to function:
- Original bug: mount a grouped action with a modal, confirm it no longer leaks into other rows' visibility
- Grouped action with
->visible()closure shows/hides correctly per row - Grouped action modal shows the correct record's data (heading, description, form fill)
- Submitting the grouped action modal affects the correct record
- Modal stays correct after form validation errors and live field updates
- Nested
ActionGroupinside a group: visibility and execution still work - Nested modal actions (
extraModalFooterActionswithmountableModalAction) receive the correct record - Modal footer actions render and execute with the correct record
- Ungrouped row actions with modals still work unchanged
recordUrl/recordActionon a resource List page (view/edit inside a group) still resolve- Run existing table/action test suites plus the new test from the PR
Please rework the PR to make that change instead and drop the view changes, then confirm your test still fails before / passes after.
Summary
Fixes #16282 — table row
ActionGroupvisibility (and even its rendered click handler) can leak from whichever row's action was last mounted into every other row rendered in the same response, for v3.Root cause
Filament\Tables\Table\Concerns\HasActions::getAction()sets the mounted record directly on the specific action that was clicked ($action->record($mountedRecord)), not only on the parentActionGroup. BecauseInteractsWithRecord::getRecord()checks an action's own$recordbefore falling back to its group, that action keeps using the clicked row's record for the rest of the request — including when Livewire re-renders the whole table to show the resulting confirmation modal.So for every other row rendered afterwards, that one action evaluates its
->visible()/->hidden()closures (and its click-handler record, viagenerateJavaScriptClickHandler()) against the wrong record. This can make an action wrongly appear on a row it shouldn't (or make the whole group's trigger button appear on a row where every action should be hidden), and the wrongly-shown button ends up pointing at the other row's record.Fix
Reset each child action's own bound record to the group's current (correct, per-row) record right before checking visibility, in both places that matter:
packages/tables/resources/views/components/actions.blade.php— recursively, before the row's top-level$action->isVisible()check (this is what decides whether the trigger button itself renders).packages/actions/resources/views/components/group.blade.php— before deciding what to render inside the opened dropdown.Both are guarded to only run for table
ActionGroups (instanceof HasRecord), since this same view also renders page/form/infolist action groups that don't carry a record at all.Test
Added
tests/src/Tables/Actions/ActionGroupTest.php, plus one new conditionally-visible action (groupedConditional) on the sharedPostsTablefixture. The test mounts a confirmation-requiring grouped action for one record, then asserts the other record's row doesn't end up with a spurious extra copy of a different record's action.I confirmed the new test fails on
3.xbefore this fix (asserts2 === 1, i.e. the action wrongly renders twice) and passes after it. Ran the fulltests/src/Tablesandtests/src/Actionssuites afterward — 115 passed, no regressions. Also ranpint --config pint-strict-imports.json.@danharrin — per your comment on #16282 that you'd merge a v3 backport, here it is 🙂