fix(list,grid): honour userActions.delete.visibleWhen per selected record on the selection bar - #6377
Merged
Merged
Conversation
…cord on the selection bar The selection bar read `userActions.delete` as a boolean only, so a record the per-record `visibleWhen` excludes was still offered — and deleted — from the bulk bar, while the row kebab correctly hid it. Maintainer ruling 2026-08-17 (behaviour 1): evaluate the predicate per selected record, run over the allowed subset, report the excluded ones. The button is never hidden or disabled by the predicate. - `@object-ui/core`: `partitionRowsByPredicate` — the set-shaped counterpart of `evalRowPredicate`, which is the loop a bulk gate needs and a hook cannot be. - `plugin-grid`: `partitionBulkRows` now delegates to it; `resolveRowCrudAffordances` returns `objectDeletePredicates` (the bulk half, gated on `objectCanDelete`). - `plugin-list`: the non-grid bulk bar filters the built-in `delete` to the eligible subset and states the skipped count. - `BulkActionDialog`: Run declines a zero-record run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q
…k-delete-visiblewhen
All-eligible / mixed / none-eligible on both selection bars, encoding the
card's repro (`showcase_invoice`, `delete: { visibleWhen: "record.status !=
'paid'" }`, INV-1011 the paid one). The mixed case asserts both halves — the
allowed subset was deleted AND the skipped row was reported through
`bulk-skipped-notice`.
Also adds the built-in delete's dialog route in ObjectGrid.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4420
The selection bar's built-in Delete ignored
userActions.delete.visibleWhen. It read that key as a bare boolean — the object-level verdict (ADR-0103 bucket ∧userActions∧ the server'sapiOperations∧ the principal'sallowDelete) — with no per-record layer at all. Tick only a record the author's predicate excludes and the bar still offered the red Delete, and pressing it deleted the record the predicate was written to protect. The row kebab on the very same screen hid its Delete correctly, so one declared key meant two different things on two surfaces.What was ruled
Maintainer, 2026-08-17, behaviour 1 of the card's three — filter the operation and report the skipped. The bar evaluates
visibleWhenonce per selected record, the delete runs over the allowed subset, and the excluded records are reported throughBulkActionDialog's existingbulk-skipped-noticeslot. Behaviour 2 (gate the button) and behaviour 3 (declare the key out of scope for sets) were rejected. Three consequences, each implemented and pinned:Not re-litigated, not re-opened.
packages/specuntouched; nothing outside this fix changes whatvisibleWhenmeans.What changed
@object-ui/corepartitionRowsByPredicate— the set-shaped counterpart ofevalRowPredicate: absent predicate returnsrowsby reference, a boolean short-circuits, an expression is evaluated per record fail-closed.@object-ui/plugin-gridpartitionBulkRowsnow delegates to it (one fold, not two).resolveRowCrudAffordancesreturnsobjectDeletePredicates.dispatchBulkAction's delete branch partitions the expanded selection.BulkActionDialogdeclines to run over zero records.@object-ui/plugin-listdeleteto the eligible subset and states the skipped count inline.No hook was harmed. A bulk gate evaluates N selected records in a loop, and React forbids a hook per iteration — so this reuses
evalRowPredicate(through the shared fold), neveruseRowPredicate. The seam already existed; nothing built a second one.Why
objectDeletePredicatesand notdeletePredicates. The existingdeletePredicatesridescanDelete, which folds in the row wiring (operations.delete/rowActions∧onDelete). Bulk delete ridesonBulkDelete. A consumer wiring only the bulk handler would otherwise have the author's predicate silently dropped — judged by whether the row handler happens to be present. This is the exact reasonobjectCanDeletealready exists besidecanDelete; the predicates now have the same pair.The one design call worth reviewing. In
ObjectGrid, an excluded selection routes throughBulkActionDialog(which owns the confirm and executes overeligible), while a selection with nothing excluded keeps the consumer's ownonBulkDeleteflow untouched. The split is deliberate: routing an excluded selection back throughonBulkDeletewould stack the host's own confirmation dialog behind this one and confirm the same delete twice, and keeping the unexcluded path on the host handler is what makes every object that declares no predicate byte-identical to its previous behaviour (confirm + toast + refresh all still the host's). Declared bulk-delete defs already run through this dialog today, so the excluded path is a shipped route, not a new one.Tests — all three selections, both bars
packages/plugin-grid/src/__tests__/bulkDeleteVisibleWhen.test.tsxandpackages/plugin-list/src/__tests__/ListView.bulkDeleteVisibleWhen.test.tsx, encoding the card's repro verbatim:showcase_invoicedeclaresdelete: { visibleWhen: "record.status != 'paid'" }.The excluded fixture row is
INV-1011—status: 'paid'.INV-1010is the draft the predicate admits.bulk-skipped-noticeis rendered and onlyinv-1010reaches the delete path.INV-1011: the button still renders and is not disabled, the dialog opens, the notice fires, Run is disabled, nothing is deleted.Ghost-assertion guard — both readings
Run in a second worktree checked out at unmodified
origin/main(2e11c8c5b), with the two test files copied in byte-identical (verified bygit hash-object) andgit statusshowing nothing else changed.Failing on
origin/main:The 4 passing there are the two degenerate controls and the two no-predicate controls, exactly as intended.
Those four die on the report half, which would leave the subset half unproven. So a probe was run in the same
origin/maintree with the report assertion removed, to make the failure land on which records reached the delete path:— i.e. on
origin/mainboth bars hand the paid invoice to the delete path. That is the defect, measured. (The probe is amain-side measurement only; it is not one of the delivered assertions and was not committed.)Passing on this branch (all readings below taken at
bef22ef5e, withgit statusclean, so the tree measured is this commit):Other local gates, all at
bef22ef5evitest run packages/plugin-grid/ packages/plugin-list/Test Files 136 passed (136)/Tests 1512 passed (1512)vitest run packages/core/Test Files 100 passed (100)/Tests 2023 passed (2023)pnpm --filter @object-ui/core --filter @object-ui/plugin-grid --filter @object-ui/plugin-list type-checkDonepnpm --filter … lint(the three changed packages)0 errorseach (core 515, plugin-list 399, plugin-grid 689 pre-existing warnings)check:control-bytesOK (scanned 5250 tracked text file(s))check:changeset-presence8 source file(s) of 3 released package(s) changed, and this change declares 1 changeset(s)check:changeset-no-majorNo changeset declares a major bump.check:i18n-keysEvery in-scope call-site key resolves against the en packcheck:i18n-driftNo en value changed in this range.check:vi-mock-specifiersOKcheck:phantom-depsEvery in-scope import is declared by the package that publishes it.check:self-importOKcheck:readme-exportsis NOT MEASURED locally, not red: every one of its 293 findings istype entry ./dist/index.d.ts is not on disk -- run pnpm build first, for packages this worktree never built (app-shell, auth, cli, layout, …). It is a missing prerequisite, not a verdict. The risk it covers is nil here — this diff adds one@object-ui/coreexport, removes none, and touches no README. CI builds everything and measures it properly.No new i18n key was minted: the list bar reuses the dialog's own
grid.bulk.skippedIneligible, already translated in all ten packs. One sentence, one spelling, ten packs that stay in step by construction.Serial clause —
#5853Per the dispatch,
git fetch origin main && git merge origin/mainwas run before the first edit topackages/plugin-grid/src/ObjectGrid.tsx.#5853had NOT landed at that point —origin/mainwas at2e11c8c5bandgit log f53a8d0ae..origin/main -- packages/plugin-grid/src/ObjectGrid.tsxis empty, so the column-type emit seam at:26/:2248is still unpushed. The second lander should re-fetch. My hunks in that file are disjoint from theirs: theresolveRowCrudAffordancesdestructure at:900and the bulk dispatcher at~:2662. Nothing outside my own region was reformatted, tidied or otherwise touched.Scope
UI affordance only — the triage record measured it as "UI-only leak; server enforcement intact." Not widened into a permissions or security change.
#4419(detail-header half) and#4296(row kebab ANDs only the object-level permission) are separate cards and are untouched here. Custom bulk action ids keep passing through unfiltered: they route through the action runner with their own gates.Draft, and staying draft — the PM lands it.
Generated by Claude Code
Generated by Claude Code