From 75e4f7ffc74c217a2b68195cb4953a539f046cb1 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 17:05:48 +0000 Subject: [PATCH 1/2] fix(list,grid): honour userActions.delete.visibleWhen per selected record on the selection bar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q --- .../core/src/evaluator/listConditional.ts | 65 +++++++++++++ packages/plugin-grid/src/bulkEligibility.ts | 52 +++------- .../src/components/BulkActionDialog.tsx | 18 +++- .../plugin-grid/src/rowCrudAffordances.ts | 15 +++ packages/plugin-list/src/ListView.tsx | 95 ++++++++++++++++++- 5 files changed, 204 insertions(+), 41 deletions(-) diff --git a/packages/core/src/evaluator/listConditional.ts b/packages/core/src/evaluator/listConditional.ts index 23c4b2168d..e081912c21 100644 --- a/packages/core/src/evaluator/listConditional.ts +++ b/packages/core/src/evaluator/listConditional.ts @@ -334,6 +334,71 @@ export function evalRowPredicate( return verdict.value; } +/** The two halves of a {@link partitionRowsByPredicate} verdict over a selection. */ +export interface RowPartition { + /** Records whose predicate passed — the ones an operation may act on. */ + eligible: TRow[]; + /** + * How many records were filtered out. A caller that shrinks a user's + * selection owes them this number: a run over fewer records than they picked + * must say so rather than quietly shrinking. + */ + skipped: number; +} + +/** + * Split a set of records by one per-record predicate — the SET-shaped + * counterpart of {@link evalRowPredicate}, and the reason a bulk gate never + * needs a hook. + * + * A bulk affordance spans N records, so its gate is a **loop**, and React + * forbids calling `useRowPredicate` (or any hook) per iteration. Every bulk + * surface therefore reaches for `evalRowPredicate` directly; this wrapper is + * that loop written once, with the three cases each caller was otherwise + * re-deriving: + * + * - **absent** (`null` / `undefined` / blank string) — nothing is gated, so + * `rows` is returned BY REFERENCE and `skipped` is 0. The common case + * allocates nothing and a caller's downstream memo still holds. + * - **boolean** — a verdict, not an expression, short-circuited exactly as + * `useCondition` / `useRowPredicate` do. Handing `true` to the engine + * produces `{ dialect: 'cel', source: undefined }`, which faults and — on + * this fail-closed path — would disqualify every record, so the most + * explicit way to say "always" would exclude everyone (objectui#3492). + * - **expression** — evaluated once per record with that record in scope, + * **fail-closed**: a predicate that faults must not hand the caller records + * it was written to exclude. `warnOnError` (default `true`) makes the + * resulting exclusion diagnosable once per predicate instead of silent. + * + * The fail-closed default is what separates this from a lenient record-free + * evaluation, and the difference is not mere strictness: evaluated with NO + * record bound, `record.status != 'paid'` returns `true` for every row — + * including the ones it was written to exclude — so an authored gate is not + * weakened, it is inverted for half its inputs (objectui#3067). + */ +export function partitionRowsByPredicate>( + pred: FieldRulePredicate | boolean | undefined | null, + rows: readonly TRow[], + opts: Omit = {}, +): RowPartition { + if (pred == null || pred === '') { + return { eligible: rows as TRow[], skipped: 0 }; + } + if (typeof pred === 'boolean') { + return pred + ? { eligible: rows as TRow[], skipped: 0 } + : { eligible: [], skipped: rows.length }; + } + const eligible = (rows as TRow[]).filter(row => + evalRowPredicate(pred, row, { + ...opts, + warnOnError: opts.warnOnError ?? true, + fallback: false, + }), + ); + return { eligible, skipped: rows.length - eligible.length }; +} + // --------------------------------------------------------------------------- // Conditional formatting // --------------------------------------------------------------------------- diff --git a/packages/plugin-grid/src/bulkEligibility.ts b/packages/plugin-grid/src/bulkEligibility.ts index 31db95a0a7..9afb51afda 100644 --- a/packages/plugin-grid/src/bulkEligibility.ts +++ b/packages/plugin-grid/src/bulkEligibility.ts @@ -39,7 +39,7 @@ * made row-scoped predicates land in a record-free evaluation. */ -import { evalRowPredicate, type FieldContainerLike } from '@object-ui/core'; +import { partitionRowsByPredicate, type FieldContainerLike } from '@object-ui/core'; import type { BulkActionDef } from '@object-ui/types'; export interface BulkEligibility { @@ -79,8 +79,14 @@ export function hasVisibilityGate(def: BulkEligibilityDef | null | undefined): b /** * Split selected records into the ones this def may act on and a count of the - * ones it may not. A def without `visible` is a no-op: `rows` is returned by - * REFERENCE so the common case allocates nothing and downstream memos hold. + * ones it may not. + * + * The loop, the boolean short-circuit and the fail-closed posture all live in + * `@object-ui/core`'s {@link partitionRowsByPredicate} — the ONE per-record + * fold every bulk surface shares (the built-in selection-bar Delete reads it + * through the same primitive, objectui#4420). This function is the `def`-shaped + * door onto it: it knows only that a bulk def spells its predicate `visible` + * and labels warnings with the def's name. */ export function partitionBulkRows>( def: BulkEligibilityDef | null | undefined, @@ -96,38 +102,10 @@ export function partitionBulkRows>( fields?: FieldContainerLike; } = {}, ): BulkEligibility { - const visible = def?.visible; - if (visible == null || visible === '') { - return { eligible: rows as TRow[], skipped: 0 }; - } - // A BOOLEAN `visible` is a verdict, not an expression — short-circuit it - // exactly as `useCondition` / `useRowPredicate` do (objectui#3492). Handing - // it to the engine instead produced `{ dialect: 'cel', source: undefined }`, - // which faults ("AST-only evaluation not yet supported") and fails CLOSED — - // so `visible: true`, the most explicit way to say "always offer this", - // silently disqualified every selected record and hid the button from - // everyone. `BulkActionDefSchema.visible` is `ExpressionInputSchema` (no - // boolean), so `objectstack build` cannot emit this shape — hand-written view - // JSON and in-process callers constructing defs can, and did. - if (typeof visible === 'boolean') { - return visible - ? { eligible: rows as TRow[], skipped: 0 } - : { eligible: [], skipped: rows.length }; - } - - const eligible = (rows as TRow[]).filter(row => - evalRowPredicate(visible as never, row, { - // Fail CLOSED, exactly as the row kebab does: a predicate that faults - // must not hand the user a button that acts on records it was written to - // exclude. `warnOnError` makes the resulting hide diagnosable (once per - // predicate) instead of silent. - fallback: false, - scope: opts.scope, - fields: opts.fields, - warnOnError: true, - label: def?.name, - }), - ); - - return { eligible, skipped: rows.length - eligible.length }; + return partitionRowsByPredicate(def?.visible as never, rows, { + scope: opts.scope, + fields: opts.fields, + warnOnError: true, + label: def?.name, + }); } diff --git a/packages/plugin-grid/src/components/BulkActionDialog.tsx b/packages/plugin-grid/src/components/BulkActionDialog.tsx index 7e210d38f0..a29cebd321 100644 --- a/packages/plugin-grid/src/components/BulkActionDialog.tsx +++ b/packages/plugin-grid/src/components/BulkActionDialog.tsx @@ -266,6 +266,22 @@ export const BulkActionDialog: React.FC = ({ const maxRecords = def?.maxRecords ?? Infinity; const overLimit = rows.length > maxRecords; + /** + * [objectui#4420] Nothing survived the eligibility fold. + * + * The dialog still OPENS on an all-excluded selection — that is the ruled + * shape for the built-in Delete (maintainer, 2026-08-17): "a legible + * refusal, not a hidden button whose absence is unexplained". What it must + * not do is offer to run: `Affected records (0)` beside an enabled Run + * button reads as a live operation, and pressing it reports + * `Succeeded 0 / 0` — a success panel for a run that never had a subject. + * The skipped notice above already carries the WHY, so the refusal needs no + * new copy, only a control that declines. + * + * Deliberately `rows.length`, not "skipped > 0": a run over zero records is + * meaningless for every def, however the selection got here. + */ + const noEligibleRows = rows.length === 0; const handleRun = useCallback(async () => { if (!def) return; @@ -531,7 +547,7 @@ export const BulkActionDialog: React.FC = ({ diff --git a/packages/plugin-grid/src/rowCrudAffordances.ts b/packages/plugin-grid/src/rowCrudAffordances.ts index 82a4ef3bb5..504f84d668 100644 --- a/packages/plugin-grid/src/rowCrudAffordances.ts +++ b/packages/plugin-grid/src/rowCrudAffordances.ts @@ -151,6 +151,20 @@ export function resolveRowCrudAffordances(opts: { objectCanDelete: boolean; editPredicates?: RowCrudPredicates; deletePredicates?: RowCrudPredicates; + /** + * [objectui#4420] The per-record delete predicates tied to + * {@link objectCanDelete} rather than to `canDelete` — the BULK bar's half. + * + * `deletePredicates` above rides `canDelete`, which folds in the ROW wiring + * (`operations.delete`/`rowActions` ∧ `onDelete`). That is right for the row + * kebab and wrong for the selection bar for the same reason `objectCanDelete` + * exists: bulk delete rides `onBulkDelete`, so a consumer that wires only the + * bulk handler would otherwise have its author-declared `visibleWhen` + * silently dropped — judged by whether the *row* handler happens to be + * present. Same predicates, gated on the same verdict the bulk bar itself is + * gated on. + */ + objectDeletePredicates?: RowCrudPredicates; } { // The object-level verdict comes from the shared policy — bucket default, // `userActions` override, then the server's effective operation set. The row @@ -174,6 +188,7 @@ export function resolveRowCrudAffordances(opts: { objectCanDelete, editPredicates: canEdit ? aff.editPredicates : undefined, deletePredicates: canDelete ? aff.deletePredicates : undefined, + objectDeletePredicates: objectCanDelete ? aff.deletePredicates : undefined, }; } diff --git a/packages/plugin-list/src/ListView.tsx b/packages/plugin-list/src/ListView.tsx index 8106a7c6e1..322126febe 100644 --- a/packages/plugin-list/src/ListView.tsx +++ b/packages/plugin-list/src/ListView.tsx @@ -15,13 +15,13 @@ import { VALUELESS_FILTER_BUILDER_OPERATORS, isFilterValueComplete } from '@obje import { ViewSwitcherDropdown, ViewType } from './ViewSwitcher'; import { ViewSettingsPopover } from './components/ViewSettingsPopover'; import { UserFilters } from './UserFilters'; -import { SchemaRenderer, useNavigationOverlay, classifyLoadError } from '@object-ui/react'; +import { SchemaRenderer, useNavigationOverlay, classifyLoadError, usePredicateScope } from '@object-ui/react'; import type { LoadErrorKind } from '@object-ui/react'; import { useDensityMode } from '@object-ui/react'; import type { ListViewSchema, ObjectMapConfig } from '@object-ui/types'; import { detectStatusField } from '@object-ui/types'; import { usePullToRefresh } from '@object-ui/mobile'; -import { resolveConditionalFormatting, buildExpandFields, buildExportFileName, resolveEffectiveCrudAffordances, isObjectInlineEditable, normalizeListViewSchema, rowHeightToDensityMode, mergeFilterNodes, columnIdentity, collectPredicateFieldRefs, listViewPredicates, PLATFORM_RECORD_COLUMNS, EXPANDABLE_FIELD_TYPES, UNMATERIALIZED_FIELD_TYPES } from '@object-ui/core'; +import { resolveConditionalFormatting, buildExpandFields, buildExportFileName, resolveEffectiveCrudAffordances, isObjectInlineEditable, partitionRowsByPredicate, normalizeListViewSchema, rowHeightToDensityMode, mergeFilterNodes, columnIdentity, collectPredicateFieldRefs, listViewPredicates, PLATFORM_RECORD_COLUMNS, EXPANDABLE_FIELD_TYPES, UNMATERIALIZED_FIELD_TYPES } from '@object-ui/core'; import { useObjectTranslation, useObjectLabel, useSafeFieldLabel, createSafeTranslation, useDisplayLocale } from '@object-ui/i18n'; // Two resolvers, two vocabularies — the repo spells the distinction into the // NAMES (objectui#4167). `resolveInlineI18nLabel` is the spec's own @@ -1179,6 +1179,61 @@ export const ListView = React.forwardRef(({ return declared.filter((a: unknown) => String(a).toLowerCase() !== 'delete'); }, [schema.bulkActions, schema.objectName, objectDef, effectiveApiOps, canDo]); + /** + * [objectui#4420] The PER-RECORD half of the same key, for the same bar. + * + * `permittedBulkActions` above reads `userActions.delete` as a BOOLEAN — the + * object-level verdict — and that is all it ever read. Since objectui#2614 + * the key also accepts `{ enabled?, visibleWhen?, disabledWhen? }`, whose + * `visibleWhen` gates the affordance **per record**; the row kebab has + * honoured it since, and this bar did not. Tick only a record the predicate + * excludes and the bar still offered the red Delete — the same declared key + * meaning two different things on two surfaces. + * + * ## What was ruled (maintainer, 2026-08-17 — behaviour 1 of three) + * + * Filter the operation and report the skipped. The bar evaluates + * `visibleWhen` once per selected record, Delete runs over the allowed + * SUBSET, and the excluded records are reported rather than silently + * dropped. Two rejected alternatives, restated because each is a way to + * misread this code: the button is **never hidden or disabled** by the + * predicate (behaviour 2 — one stray tick would disable the whole bar), and + * the predicate is **not** declared out of scope for set operations + * (behaviour 3 — the key must not mean different things on two surfaces). + * A selection where EVERY row is excluded therefore still renders the + * button; what the user gets is a legible refusal, not an absence. + * + * ## Why `evalRowPredicate`, never `useRowPredicate` + * + * A bulk gate evaluates N records in a LOOP, and React forbids a hook per + * iteration. `partitionRowsByPredicate` is that loop — the shared, + * fail-closed fold in `@object-ui/core` that the grid's own bulk bar reads + * through `partitionBulkRows`, so the two bars cannot drift on what + * "eligible" means. + * + * ## Why `objectDef`, and only the built-in `delete` + * + * The predicates come off the OBJECT's `userActions` block (the CRUD + * predicate vocabulary), never the VIEW's `schema.userActions` (toolbar + * policy) — the same name collision `toolbarFlags` above is pinned against. + * Only the built-in `delete` entry is filtered: custom action ids route + * through the action runner carrying their own gates, exactly as the + * object-level gate above leaves them alone. + */ + const deleteVisibleWhen = React.useMemo( + () => resolveEffectiveCrudAffordances(objectDef as any, effectiveApiOps).deletePredicates?.visibleWhen, + [objectDef, effectiveApiOps], + ); + const predicateScope = usePredicateScope(); + const bulkDeleteEligibility = React.useMemo( + () => partitionRowsByPredicate(deleteVisibleWhen as never, selectedRows as Array>, { + scope: predicateScope, + fields: objectDef?.fields, + label: 'delete', + }), + [deleteVisibleWhen, selectedRows, predicateScope, objectDef], + ); + /** * [#4647] Is the grid toolbar's inline-edit toggle offered at all? * @@ -3532,13 +3587,23 @@ export const ListView = React.forwardRef(({ const actionStr = String(action).toLowerCase(); const isDestructive = actionStr.includes('delete') || actionStr.includes('remove') || actionStr.includes('destroy'); const Icon = isDestructive ? Trash2 : null; + // [objectui#4420] The built-in `delete` runs over the ALLOWED + // SUBSET — the records `userActions.delete.visibleWhen` admits — + // never the raw tick list. The button itself is untouched by the + // predicate (ruled: never hidden, never disabled); what shrinks + // is what it acts on, and the notice below owns up to it. Only + // the canonical `delete` is filtered: every other id routes + // through the action runner with its own gates. + const rowsForAction = actionStr === 'delete' + ? (bulkDeleteEligibility.eligible as any[]) + : selectedRows; return (