diff --git a/.changeset/10946-expression-wire-slots-by-reference.md b/.changeset/10946-expression-wire-slots-by-reference.md index e9fa8d5247..fd348fac34 100644 --- a/.changeset/10946-expression-wire-slots-by-reference.md +++ b/.changeset/10946-expression-wire-slots-by-reference.md @@ -15,3 +15,20 @@ The two spec lines differ, and these members follow the installed one. The publi **Breaking for TypeScript readers of these members, released as `minor`** under this repo's version-alignment rule (the fixed group's major follows `@objectstack`). Code that reads `rule.condition` as a `string`, or reads `def.visible.source` as a `string`, stops compiling. Narrow on `typeof` first, as the shared evaluator does. Code that writes these members compiles unchanged. Pins: `spec-expression-wire-slots-10946.test.ts` in `@object-ui/types` covers the relays with no assertion, the zod twin's input equalling the TS member, envelope acceptance with string and empty-string controls, and the reference identity of the spec arm. `specExpressionWire-10946.test.tsx` in `@object-ui/plugin-grid` covers the real grid painting an envelope-matched row, the bulk fold reading `source` beside `ast`, and the `ast`-only fault on both. + +**Note, 2026-10-01 (objectui#11322, shipping in this same release).** +The `ast`-only reading above changed for the bulk def after this entry was +written. The grid's selection bar now asks the action family's one "is a gate +declared?" definition (`hasDeclaredVisibilityGate`) before the bulk fold, as +the row menu and the toolbars already did, and that definition reads an +envelope with no `source` as no gate. +- An `ast`-only bulk `visible` is no gate: the button shows and every selected + record qualifies, with no warning. The formatting rule's `ast`-only + `condition` is unchanged: it does not match, and the fault is warned. +- `specExpressionWire-10946.test.tsx` pins the formatting fault and, for the + bulk def, the no-gate answer. + +So the sentences above that say the bulk def qualifies no record, and that the +test file pins the `ast`-only fault "on both", describe the tree at +objectui#10946, not the code in this release. See objectui#11322's changesets +for `@object-ui/plugin-grid` and `@object-ui/types`. diff --git a/.changeset/11322-bulk-bar-blank-visible-shown.md b/.changeset/11322-bulk-bar-blank-visible-shown.md new file mode 100644 index 0000000000..02104d15df --- /dev/null +++ b/.changeset/11322-bulk-bar-blank-visible-shown.md @@ -0,0 +1,22 @@ +--- +'@object-ui/plugin-grid': patch +--- + +A bulk action whose `visible` is blank now shows on the grid's selection bar and runs over every selected +record, as it already does on the row menu and the toolbars of the same grid (objectui#11322). + +The selection bar decided whether a bulk action "declared a visibility gate" with a test of its own, +`!= null && !== ''`, and handed the value to the shared fold, whose opening test also stops at `''`. A +whitespace-only `visible` (`' '`), or an envelope whose `source` is blank +(`{ dialect: 'cel', source: ' ' }`), therefore counted as a gate: every selected record failed it and +the button disappeared, while the row menu and the toolbars, which ask the action family's one +definition, showed the same action. `hasVisibilityGate` now asks that definition, +`hasDeclaredVisibilityGate`, and `partitionBulkRows` asks it before handing a def's `visible` to the fold. +So blank text in either spelling and the empty envelope are "no gate" here too, and the blank is reported +once as on the other surfaces. An envelope with an `ast` and no `source` is no gate here either, as on +the row menu, where the bar used to evaluate it, fail closed and hide the button. `visible: false` is +unchanged: it is still a declared gate that hides the button. + +The built-in Delete is unchanged. Its `userActions.delete.visibleWhen` is a field-rule key, not an +action's `visible`, so `ObjectGrid` now hands it to the fold directly, as plugin-list's `ListView` does. +A blank one still excludes every selected record. diff --git a/.changeset/11322-bulk-def-visible-docblock.md b/.changeset/11322-bulk-def-visible-docblock.md new file mode 100644 index 0000000000..20d5b45545 --- /dev/null +++ b/.changeset/11322-bulk-def-visible-docblock.md @@ -0,0 +1,11 @@ +--- +'@object-ui/types': patch +--- + +Docblock only: `BulkActionDef.visible` now says what the grid's selection bar does with an `ast`-only +envelope (objectui#11322). + +It said the envelope "is evaluated as a fault, so no selected record qualifies (fail closed, warned)". +The selection bar now asks the action family's one "is a gate declared?" definition, as the row menu and +the toolbars already did, and that definition reads an envelope with no `source` as no gate: every +selected record qualifies. The type itself is unchanged. diff --git a/.changeset/11322-core-docblocks-name-callers.md b/.changeset/11322-core-docblocks-name-callers.md new file mode 100644 index 0000000000..cf31c38079 --- /dev/null +++ b/.changeset/11322-core-docblocks-name-callers.md @@ -0,0 +1,14 @@ +--- +'@object-ui/core': patch +--- + +Docblock only, no behavior change: `partitionRowsByPredicate` now names its callers and says who decides +"is a gate declared?" (objectui#11322). + +The shared partitioner keeps its opening test exactly as it was (`null`, `undefined` or `''` admits +every record). Its docblock used to call that test "blank string", which it never was: whitespace-only +text and an envelope whose `source` is blank are evaluated there and exclude every record. That is the +answer its built-in Delete `userActions.delete.visibleWhen` callers get. The bulk-action fold asks the +action family's `hasDeclaredVisibilityGate` before calling it, so a blank bulk `visible` reaches it as +`undefined`. `hasDeclaredPredicate`'s docblock names that new consumer and the field-rule tests that stay +outside the family. diff --git a/packages/core/src/evaluator/declaredPredicate.ts b/packages/core/src/evaluator/declaredPredicate.ts index 4c97917944..c8879ef5e1 100644 --- a/packages/core/src/evaluator/declaredPredicate.ts +++ b/packages/core/src/evaluator/declaredPredicate.ts @@ -164,8 +164,27 @@ export function isBlankPredicateText(value: unknown): boolean { * an empty predicate made the node VANISH (objectui#3955), and * `ActionEngine.getActionsForLocation`'s `visible` filter, which folded some * empty spellings by hand and coerced the rest with `Boolean(raw)`, so - * `visible: 0` hid an action the renderers showed (objectui#3957). Nothing in the - * repo now asks "is a gate declared?" anywhere but here. + * `visible: 0` hid an action the renderers showed (objectui#3957). + * + * The last one joined with objectui#11322: plugin-grid's `hasVisibilityGate`, + * on the grid's selection bar, asks it through the renderer-side name + * `hasDeclaredVisibilityGate`. It decides whether a bulk button whose `visible` + * admits no record is hidden, and `partitionBulkRows` asks it before handing a + * bulk def's `visible` to `partitionRowsByPredicate`. It had kept the old + * `!= null && !== ''` scope after every other member of the family moved here, + * so a whitespace-only `visible` counted as declared on that bar, failed closed + * for every record, and hid an action the row menu and toolbars showed. + * + * Nothing in the repo now asks "is a gate declared?" anywhere but here for + * the keys this definition serves. What stays outside is a different family, + * not a twin: the field-rule `*When` keys (ADR-0137) decide it by rules of + * their own. Examples are the built-in Edit / Delete row item's + * `visibleWhen != null` (`isBuiltinRowActionVisible`), and the opening test of + * the shared partitioner `partitionRowsByPredicate` (`null` / `undefined` / + * `''`), which is the answer its built-in Delete `visibleWhen` callers get. The + * action family asks here before it calls that partitioner. + * ⚠️ Nothing re-derives this paragraph: it was read off a source search when + * objectui#11322 landed, and no test or gate repeats that search. */ export function hasDeclaredPredicate(value: unknown): boolean { if (isBlankPredicateText(value)) { diff --git a/packages/core/src/evaluator/listConditional.ts b/packages/core/src/evaluator/listConditional.ts index 4336917b84..11e00e88f3 100644 --- a/packages/core/src/evaluator/listConditional.ts +++ b/packages/core/src/evaluator/listConditional.ts @@ -370,9 +370,11 @@ export interface RowPartition { * 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. + * - **absent** (`null` / `undefined` / `''`) — 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. Whitespace-only text + * and an envelope whose `source` is blank are NOT absent here: they are + * evaluated, and fail closed like any other predicate that cannot run. * - **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 @@ -388,6 +390,24 @@ export interface RowPartition { * 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). + * + * ## Who calls it, and who decides "is a gate declared?" (objectui#11322) + * + * This is a shared partitioner, and its opening test is not the action + * family's "is a gate declared?" question. Its callers: + * + * - the bulk ACTION fold, plugin-grid's `partitionBulkRows` (a bulk def's + * `visible`). The action family decides "declared?" BEFORE calling here, + * through `hasDeclaredVisibilityGate`, and hands an undeclared gate over as + * `undefined`. So a whitespace-only `visible` or a blank-`source` envelope + * never reaches the test below from that caller. + * - the built-in Delete's `userActions.delete.visibleWhen`, a field-rule key, + * from plugin-grid's `ObjectGrid` and from plugin-list's `ListView` + * (`bulkDeleteEligibility`). The test below is THEIR answer: a blank one + * that is not `''` is evaluated and excludes every record. + * + * A caller that needs the action family's answer asks it first; it does not + * write a second blank test in front of this one. */ export function partitionRowsByPredicate>( pred: FieldRulePredicate | boolean | undefined | null, diff --git a/packages/plugin-grid/src/ObjectGrid.tsx b/packages/plugin-grid/src/ObjectGrid.tsx index 2e2f3ee62a..05c8cd3dae 100644 --- a/packages/plugin-grid/src/ObjectGrid.tsx +++ b/packages/plugin-grid/src/ObjectGrid.tsx @@ -57,7 +57,7 @@ import { DataEmptyState, resolveIcon, } from '@object-ui/components'; import { usePullToRefresh } from '@object-ui/mobile'; -import { resolveConditionalFormatting, leadWithNameField, buildExpandFields, buildExportFileName, columnIdentity, collectPredicateFieldRefs, collectGroupingFieldRefs, listViewPredicates, isObjectInlineEditable, isProjectableField, isExpandableFieldType, isUnmaterializedFieldType, readObjectSortability, isPlatformSortableField, filterPlatformSortableSort, toFilterNode, toFilterNodeSafely, filterRefusalSubject, FilterOperatorError, convertSortToQueryParams, normalizeSortEntries, type QuerySortEntry, ROW_HEIGHT_TO_DENSITY_MODE, resolveRecordSourceConfig, resolveRecordSourceObjectName, resolveFilterPlaceholders, type FilterTokenScope } from '@object-ui/core'; +import { resolveConditionalFormatting, leadWithNameField, buildExpandFields, buildExportFileName, columnIdentity, collectPredicateFieldRefs, collectGroupingFieldRefs, listViewPredicates, isObjectInlineEditable, isProjectableField, isExpandableFieldType, isUnmaterializedFieldType, readObjectSortability, isPlatformSortableField, filterPlatformSortableSort, toFilterNode, toFilterNodeSafely, filterRefusalSubject, FilterOperatorError, convertSortToQueryParams, normalizeSortEntries, type QuerySortEntry, ROW_HEIGHT_TO_DENSITY_MODE, resolveRecordSourceConfig, resolveRecordSourceObjectName, resolveFilterPlaceholders, partitionRowsByPredicate, type FilterTokenScope } from '@object-ui/core'; import { usePermissions } from '@object-ui/permissions'; import { RECORD_OVERLAY_DEFAULT_WIDTH, @@ -4926,20 +4926,26 @@ export const ObjectGrid: React.FC = ({ // [objectui#4420] `userActions.delete.visibleWhen` gates the built-in // Delete PER RECORD — the same key, the same fail-closed fold and the // same evaluator the row kebab and the rich-def bar already run - // (`partitionBulkRows` → `partitionRowsByPredicate`). Applied to the - // EXPANDED set for the reason #3067 states one line down in - // `dispatchBulkActionDef`: "select all N matching" pulls in records no - // on-screen check ever evaluated. + // (`partitionRowsByPredicate`). Applied to the EXPANDED set for the + // reason #3067 states one line down in `dispatchBulkActionDef`: "select + // all N matching" pulls in records no on-screen check ever evaluated. + // + // The fold directly, NOT `partitionBulkRows` (objectui#11322). That door + // belongs to bulk ACTIONS and asks the action family's "is a gate + // declared?" first, which reads a blank `visible` as no gate. This key + // is a field-rule key: a blank one keeps the fold's own answer and + // excludes every record, as plugin-list's `ListView` does with the + // same key. // // The predicates come from `objectDeletePredicates`, not // `deletePredicates`: the latter rides `canDelete`, which folds in the // ROW wiring (`onDelete`), and bulk delete rides `onBulkDelete`. A // consumer wiring only the bulk handler would otherwise have the // author's predicate silently dropped. - const { eligible, skipped } = partitionBulkRows( - { name: 'delete', visible: objectDeletePredicates?.visibleWhen as never }, + const { eligible, skipped } = partitionRowsByPredicate( + objectDeletePredicates?.visibleWhen as never, expanded, - { scope: predicateScope, fields: objectSchema?.fields }, + { scope: predicateScope, fields: objectSchema?.fields, label: 'delete' }, ); if (skipped === 0) { // Nothing was excluded, so there is nothing to report and no reason diff --git a/packages/plugin-grid/src/__tests__/bulk-action-visible.test.tsx b/packages/plugin-grid/src/__tests__/bulk-action-visible.test.tsx index f49931c123..06bdcdcc4a 100644 --- a/packages/plugin-grid/src/__tests__/bulk-action-visible.test.tsx +++ b/packages/plugin-grid/src/__tests__/bulk-action-visible.test.tsx @@ -11,7 +11,7 @@ * `visible` predicate (a permission / feature gate). It used to render every * `bulkActionDefs` entry unconditionally, ignoring `visible` entirely. */ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { render, screen } from '@testing-library/react'; import '@testing-library/jest-dom'; import React from 'react'; @@ -74,4 +74,44 @@ describe('BulkActionBar — bulk action visible CEL', () => { expect(screen.getByTestId('bulk-actions-bar')).toBeInTheDocument(); }); }); + + // [objectui#11322] A BLANK `visible` is no gate on this bar, as on the row + // menu and the toolbars of the same grid. The bar asked "is a gate declared?" + // with `!= null && !== ''` (`hasVisibilityGate`) and handed the value to core's + // fold, whose own opening test also stops at `''`. So a whitespace-only + // `visible` counted as a gate, every selected record failed it, and the button + // disappeared. `hasVisibilityGate` now asks the action family's one definition, + // and `partitionBulkRows` asks it before handing anything to the fold. + // + // The `''` and no-gate rows are the controls the card measured as shown + // before the fix; `visible: false` keeps "no gate" from passing as "always + // show". A blank is reported once (ADR-0137 D4), so the console is quieted + // and its text is not asserted here. + describe('blank visible (objectui#11322)', () => { + let warn: ReturnType; + beforeEach(() => { + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + }); + afterEach(() => warn.mockRestore()); + + it.each([ + ['a whitespace-only string', ' '], + ['an envelope whose `source` is whitespace', { dialect: 'cel', source: ' ' }], + ['an envelope whose `source` is empty', { dialect: 'cel', source: '' }], + ])('shows an action whose `visible` is %s', (_label, visible) => { + renderBar([{ name: 'blank_gate', label: 'Blank gate', visible }]); + expect(screen.getByTestId('bulk-action-blank_gate')).toBeInTheDocument(); + }); + + it.each([ + ['an empty string (control)', { visible: '' }, true], + ['no `visible` key (control)', {}, true], + ['`false`, a declared gate (control)', { visible: false }, false], + ])('`visible` as %s', (_label, gate, shown) => { + renderBar([{ name: 'control', label: 'Control', ...gate }]); + if (shown) expect(screen.getByTestId('bulk-action-control')).toBeInTheDocument(); + else expect(screen.queryByTestId('bulk-action-control')).toBeNull(); + expect(screen.getByTestId('bulk-actions-bar')).toBeInTheDocument(); + }); + }); }); diff --git a/packages/plugin-grid/src/__tests__/bulkDeleteVisibleWhen.test.tsx b/packages/plugin-grid/src/__tests__/bulkDeleteVisibleWhen.test.tsx index 827316231c..998b808300 100644 --- a/packages/plugin-grid/src/__tests__/bulkDeleteVisibleWhen.test.tsx +++ b/packages/plugin-grid/src/__tests__/bulkDeleteVisibleWhen.test.tsx @@ -337,3 +337,43 @@ describe('selection-bar Delete vs `userActions.delete.visibleWhen` (objectui#442 expect(screen.queryByTestId('bulk-skipped-notice')).not.toBeInTheDocument(); }); }); + +/** + * objectui#11322 — a BLANK `visibleWhen` on the built-in Delete keeps the + * fold's answer: every selected record is excluded, as before. + * + * The same card made a blank bulk ACTION `visible` "no gate" on this bar, + * because the action family decides "is a gate declared?" before it calls the + * fold. This key is not an action's `visible`; it is a field-rule key, and + * `ObjectGrid` hands it to the fold directly. If it went through the action + * door instead, a whitespace `visibleWhen` would admit every record and hand + * them to the host's `onBulkDelete` while the row item still hides its Delete. + * That is the destructive widening triage ruled out. The no-gate case above is + * the control: with nothing declared, the whole selection goes through. + */ +describe('selection-bar Delete vs a blank `visibleWhen` (objectui#11322)', () => { + it.each([ + ['a whitespace-only string', ' '], + ['an envelope whose `source` is whitespace', { dialect: 'cel', source: ' ' }], + ])('%s still excludes every record: the dialog refuses, nothing is deleted', async (_label, visibleWhen) => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const { onBulkDelete, dataSource } = await renderAndSelect({ + rows: [DRAFT, PAID], + userActionsDelete: { visibleWhen }, + // The draft alone: a row the real predicate above would admit, so the + // exclusion is the blank's doing, not the record's. + selectRowNames: ['INV-1010'], + }); + + fireEvent.click(await screen.findByTestId('bulk-action-delete')); + + expect(await screen.findByTestId('bulk-skipped-notice')).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Run' })).toBeDisabled(); + expect(dataSource.delete).not.toHaveBeenCalled(); + expect(onBulkDelete).not.toHaveBeenCalled(); + } finally { + warn.mockRestore(); + } + }); +}); diff --git a/packages/plugin-grid/src/__tests__/bulkEligibility.test.ts b/packages/plugin-grid/src/__tests__/bulkEligibility.test.ts index 53894ba397..3744cd18d1 100644 --- a/packages/plugin-grid/src/__tests__/bulkEligibility.test.ts +++ b/packages/plugin-grid/src/__tests__/bulkEligibility.test.ts @@ -7,6 +7,7 @@ */ import { describe, it, expect, vi, afterEach } from 'vitest'; +import { hasDeclaredVisibilityGate } from '@object-ui/components'; import { partitionBulkRows, hasVisibilityGate } from '../bulkEligibility'; const ROWS = [ @@ -133,4 +134,41 @@ describe('partitionBulkRows', () => { expect(hasVisibilityGate(undefined)).toBe(false); }); }); + + // [objectui#11322] Blank text is no gate, in either spelling. The selection + // bar used to ask "declared?" with `!= null && !== ''`, so the spellings below + // counted as gates, every record failed them, and the bar hid an action the + // row menu and toolbars show. `hasVisibilityGate` now asks the action family's + // definition, and `partitionBulkRows` asks it BEFORE the core fold, whose own + // opening test (`''` only) belongs to its field-rule callers. So the blank + // reaches the fold as `undefined` and every record comes back by reference. + // The `''` row above is the control; the boolean rows keep "not declared" + // from passing as "always". + describe('blank visible (objectui#11322)', () => { + const BLANKS = [ + ['a whitespace-only string', ' '], + ['an envelope whose `source` is whitespace', { dialect: 'cel', source: ' ' }], + ['an envelope whose `source` is empty', { dialect: 'cel', source: '' }], + ] as const; + + it.each(BLANKS)('%s is not a declared gate', (_label, visible) => { + vi.spyOn(console, 'warn').mockImplementation(() => {}); + expect(hasVisibilityGate({ name: 'blank', visible: visible as never })).toBe(false); + }); + + it.each(BLANKS)('%s passes every record through, by reference', (_label, visible) => { + vi.spyOn(console, 'warn').mockImplementation(() => {}); + const { eligible, skipped } = partitionBulkRows({ name: 'blank', visible: visible as never }, ROWS); + expect(eligible).toBe(ROWS); + expect(skipped).toBe(0); + }); + + it('answers exactly what the action family answers, for every shape above', () => { + vi.spyOn(console, 'warn').mockImplementation(() => {}); + const shapes: unknown[] = [undefined, '', false, true, 'record.done', ...BLANKS.map(([, v]) => v)]; + for (const visible of shapes) { + expect(hasVisibilityGate({ name: 'x', visible: visible as never })).toBe(hasDeclaredVisibilityGate(visible)); + } + }); + }); }); diff --git a/packages/plugin-grid/src/__tests__/specExpressionWire-10946.test.tsx b/packages/plugin-grid/src/__tests__/specExpressionWire-10946.test.tsx index 40a414fa7c..e139ab38f8 100644 --- a/packages/plugin-grid/src/__tests__/specExpressionWire-10946.test.tsx +++ b/packages/plugin-grid/src/__tests__/specExpressionWire-10946.test.tsx @@ -20,9 +20,13 @@ * * The `ast`-only envelope is typed loosely on purpose. The installed spec's * slot admits it and spec `main` does not, so a typed fixture would compile on - * one line and fail the other. Its case pins the runtime answer the - * declarations' docblocks describe: evaluated as a fault, so the rule does not - * match and the bulk def qualifies no record, and the fault is warned. + * one line and fail the other. Its cases pin the runtime answers the + * declarations' docblocks describe. As a rule `condition` it is evaluated as a + * fault, so the rule does not match and the fault is warned. As a bulk def's + * `visible` it is not a declared gate at all: the selection bar asks the action + * family's one "declared?" definition, which reads an envelope with no `source` + * as no gate, so every selected record qualifies — as on the row menu and the + * toolbars (objectui#11322). */ import { describe, it, expect, vi, afterEach } from 'vitest'; @@ -121,14 +125,17 @@ describe('objectui#10946 — a bulk def\'s `visible` envelope that carries `ast` expect(partitionBulkRows(def, BULK_ROWS).eligible.map((r) => r.id)).toEqual(['r1']); }); - it('an `ast`-only envelope qualifies no record (fail closed), and the fault is warned', () => { - const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + // [objectui#11322] This case used to pin the opposite: the selection bar kept + // a "declared?" test of its own (`!= null && !== ''`), counted the envelope + // as a gate, evaluated it, and failed closed for every record — while the row + // menu and the toolbars, which ask `hasDeclaredVisibilityGate`, read the same + // value as no gate and showed the action. The bar now asks that definition. + it('an `ast`-only envelope is no declared gate: every record qualifies, as on the row menu', () => { const def = { name: 'mark_done', operation: 'custom', visible: { dialect: 'cel', ast: { op: 'not' } } } as unknown as BulkActionDef; const { eligible, skipped } = partitionBulkRows(def, BULK_ROWS); - expect(eligible).toEqual([]); - expect(skipped).toBe(2); - expect(warn).toHaveBeenCalled(); + expect(eligible).toBe(BULK_ROWS); + expect(skipped).toBe(0); }); }); diff --git a/packages/plugin-grid/src/bulkEligibility.ts b/packages/plugin-grid/src/bulkEligibility.ts index 9afb51afda..5a014a5e53 100644 --- a/packages/plugin-grid/src/bulkEligibility.ts +++ b/packages/plugin-grid/src/bulkEligibility.ts @@ -40,6 +40,7 @@ */ import { partitionRowsByPredicate, type FieldContainerLike } from '@object-ui/core'; +import { hasDeclaredVisibilityGate } from '@object-ui/components'; import type { BulkActionDef } from '@object-ui/types'; export interface BulkEligibility { @@ -63,18 +64,26 @@ export type BulkEligibilityDef = Pick & { }; /** - * Whether the def declares a visibility gate at all — the ONE definition of - * "gated", shared by the fold and by the button that renders its verdict. + * Whether the def declares a visibility gate at all. The `def`-shaped door onto + * the action family's one definition of "declared", `hasDeclaredVisibilityGate` + * (`@object-ui/components`, a re-export of core's `hasDeclaredPredicate`). + * {@link partitionBulkRows} asks it too before it hands anything to the fold, + * so the button and the fold behind it cannot disagree about one def. * * The bar reads it because "no record qualified" only means "hide me" for a def * that gated itself; an ungated def renders regardless. Truthiness cannot * answer this: `visible: false` is a declared gate that excludes everything, * and testing `def.visible &&` classified it as *ungated* — which rendered the * button `false` was written to remove (objectui#3492). + * + * Blank is not declared: `''`, a whitespace-only string and an envelope whose + * `source` is blank are all "no gate", as on the row menu and the toolbars. + * This used to be a test of its own, `!= null && !== ''`, so a whitespace-only + * `visible` counted as a gate here, every record failed it, and the selection + * bar hid an action the rest of the grid showed (objectui#11322). */ export function hasVisibilityGate(def: BulkEligibilityDef | null | undefined): boolean { - const visible = def?.visible; - return visible != null && visible !== ''; + return hasDeclaredVisibilityGate(def?.visible); } /** @@ -83,10 +92,24 @@ export function hasVisibilityGate(def: BulkEligibilityDef | null | undefined): b * * 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. + * fold every bulk surface shares. This function is the bulk ACTION's door onto + * it: it knows that a bulk def spells its predicate `visible`, labels warnings + * with the def's name, and decides "is a gate declared?" first. + * + * That last part is the action family's question, asked here through + * {@link hasVisibilityGate} and not left to the fold (objectui#11322). The + * fold's own opening test (`null` / `undefined` / `''`) is its field-rule + * callers' answer, and it is narrower: a whitespace-only `visible` or an + * envelope whose `source` is blank would reach the evaluator there, fail + * closed for every record, and run the action over nothing, while the row menu + * and the toolbars read the same value as no gate. A def with no declared gate + * therefore reaches the fold as `undefined`, and every record qualifies, by + * reference. + * + * The built-in Delete's `userActions.delete.visibleWhen` does NOT come through + * here. It is a field-rule key, not an action's `visible`, so `ObjectGrid` + * hands it to the fold directly, as plugin-list's `ListView` does, and a blank + * one keeps the fold's answer (objectui#4420). */ export function partitionBulkRows>( def: BulkEligibilityDef | null | undefined, @@ -102,7 +125,7 @@ export function partitionBulkRows>( fields?: FieldContainerLike; } = {}, ): BulkEligibility { - return partitionRowsByPredicate(def?.visible as never, rows, { + return partitionRowsByPredicate(hasVisibilityGate(def) ? (def?.visible as never) : undefined, rows, { scope: opts.scope, fields: opts.fields, warnOnError: true, diff --git a/packages/plugin-grid/src/components/__tests__/RowActionMenu.test.tsx b/packages/plugin-grid/src/components/__tests__/RowActionMenu.test.tsx index cd3b4144d4..9c980126ce 100644 --- a/packages/plugin-grid/src/components/__tests__/RowActionMenu.test.tsx +++ b/packages/plugin-grid/src/components/__tests__/RowActionMenu.test.tsx @@ -269,9 +269,11 @@ describe('declared boolean `visible` on a custom row action (objectui#3758)', () }); // --- the other half of "declared": empty string is NOT a gate ------------ - // `hasVisibilityGate` excludes `''` for the selection bar; the row surfaces - // match it, so an action whose predicate compiled away to an empty string is - // not silently hidden from everyone. + // The row surfaces and the selection bar (`hasVisibilityGate`) ask the same + // `hasDeclaredVisibilityGate`, which reads `''` as no gate — and, since + // objectui#11294 here and objectui#11322 on the bar, blank text in either + // spelling too (see the block below). So an action whose predicate compiled + // away to an empty string is not silently hidden from everyone. it('an empty-string `visible` is not a declared gate — the action still renders', () => { renderMenu({ rowActionDefs: [{ ...OPEN, visible: '' }] }); diff --git a/packages/plugin-list/src/__tests__/ListView.bulkDeleteVisibleWhen.test.tsx b/packages/plugin-list/src/__tests__/ListView.bulkDeleteVisibleWhen.test.tsx index fb1d52c982..d447c51159 100644 --- a/packages/plugin-list/src/__tests__/ListView.bulkDeleteVisibleWhen.test.tsx +++ b/packages/plugin-list/src/__tests__/ListView.bulkDeleteVisibleWhen.test.tsx @@ -208,4 +208,28 @@ describe('ListView non-grid bulk Delete vs `userActions.delete.visibleWhen` (obj fireEvent.click(screen.getByTestId('bulk-action-delete')); expect(dispatchedIds(onBulkAction, 'delete')).toEqual(['inv-1010', 'inv-1011']); }); + + // [objectui#11322] A BLANK `visibleWhen` keeps the fold's answer here: every + // selected record is excluded, as before. That card made a blank bulk ACTION + // `visible` "no gate" by asking the action family's definition before the + // fold; this key is a field-rule key and reaches the fold directly, so its + // blank does not widen into "delete everything". The no-gate case above is + // the control. + it.each([ + ['a whitespace-only string', ' '], + ['an envelope whose `source` is whitespace', { dialect: 'cel', source: ' ' }], + ])('a blank `visibleWhen` (%s) still excludes every record (objectui#11322)', async (_label, visibleWhen) => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + // The draft alone: a row the real predicate above would admit. + const { onBulkAction } = renderGalleryBulk({ selected: [DRAFT], userActionsDelete: { visibleWhen } }); + await waitFor(() => expect(screen.getByTestId('bulk-actions-bar')).toBeInTheDocument()); + + expect(await screen.findByTestId('bulk-skipped-notice')).toHaveTextContent('1'); + fireEvent.click(screen.getByTestId('bulk-action-delete')); + expect(dispatchedIds(onBulkAction, 'delete')).toEqual([]); + } finally { + warn.mockRestore(); + } + }); }); diff --git a/packages/types/src/objectql.ts b/packages/types/src/objectql.ts index eb25a95788..2a44d8e7cb 100644 --- a/packages/types/src/objectql.ts +++ b/packages/types/src/objectql.ts @@ -634,8 +634,11 @@ export interface BulkActionDef { * and may carry `ast` and `meta`, so a named view's defs could not be relayed * into a grid without an assertion. Spec `main` narrows the slot to require a * non-blank `source`, and this arm follows the installed line. An `ast`-only - * envelope is evaluated as a fault, so no selected record qualifies (fail - * closed, warned). + * envelope carries no `source` to evaluate, so it is not a declared gate: the + * selection bar asks the action family's one "declared?" definition, as the + * row menu and the toolbars do, and every selected record qualifies. A blank + * `source` or a whitespace-only string is no gate either, reported once + * (objectui#11322). */ visible?: ExpressionWire | SpecNamedViewBulkActionDef['visible']; /**