From b90129359097485dfb11bfe107189a952cad8c89 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 1 Oct 2026 06:14:36 +0000 Subject: [PATCH 1/4] fix(plugin-grid,core): the selection bar asks the one declared-gate definition `bulkEligibility.hasVisibilityGate` and core's `partitionRowsByPredicate` each answered "is a gate declared?" with a test of their own (`!= null && !== ''`), so a whitespace-only `visible` or an envelope whose `source` is blank counted as a gate on the grid's selection bar, failed closed for every selected record, and hid an action the row menu and the toolbars of the same grid show. `hasVisibilityGate` now asks `hasDeclaredVisibilityGate` (the `@object-ui/components` barrel) and `partitionRowsByPredicate` asks core's `hasDeclaredPredicate`, the same definition under its core name. The `declaredPredicate.ts` docblock names the two new consumers, and the fold's docblock says what it now does, including that the built-in Delete's `userActions.delete.visibleWhen` reaches the same answer on both selection bars. Pins: the real `BulkActionBar` with one selected row shows a whitespace, a blank-source and an empty-source `visible`, with `''`, no gate and `visible: false` as controls; the fold returns every row by reference for the blank shapes. Claude-Session: https://claude.ai/code/session_011p7ikEivgXefNDaE5S5Uec Co-authored-by: Claude --- .../11322-bulk-bar-blank-visible-shown.md | 15 +++ .../11322-partition-rows-declared-gate.md | 16 +++ .../partitionBlankGate-11322.test.ts | 117 ++++++++++++++++++ .../core/src/evaluator/declaredPredicate.ts | 21 +++- .../core/src/evaluator/listConditional.ts | 32 ++++- .../__tests__/bulk-action-visible.test.tsx | 41 +++++- .../src/__tests__/bulkEligibility.test.ts | 34 +++++ packages/plugin-grid/src/bulkEligibility.ts | 20 ++- .../__tests__/RowActionMenu.test.tsx | 8 +- 9 files changed, 289 insertions(+), 15 deletions(-) create mode 100644 .changeset/11322-bulk-bar-blank-visible-shown.md create mode 100644 .changeset/11322-partition-rows-declared-gate.md create mode 100644 packages/core/src/evaluator/__tests__/partitionBlankGate-11322.test.ts 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..61909b870b --- /dev/null +++ b/.changeset/11322-bulk-bar-blank-visible-shown.md @@ -0,0 +1,15 @@ +--- +'@object-ui/plugin-grid': patch +--- + +A bulk action whose `visible` is blank now shows on the grid's selection bar, 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 && !== ''`. 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`, so blank text in either spelling and the empty envelope are "no gate" here +too, and the blank is reported once as it is on the other surfaces. `visible: false` is unchanged: it is +still a declared gate that hides the button. diff --git a/.changeset/11322-partition-rows-declared-gate.md b/.changeset/11322-partition-rows-declared-gate.md new file mode 100644 index 0000000000..965f4195e7 --- /dev/null +++ b/.changeset/11322-partition-rows-declared-gate.md @@ -0,0 +1,16 @@ +--- +'@object-ui/core': patch +--- + +`partitionRowsByPredicate` asks the repo's one "is a gate declared?" definition, `hasDeclaredPredicate`, +instead of its own `pred == null || pred === ''` test (objectui#11322). + +What a user sees: a bulk action whose `visible` is blank now shows on the grid's selection bar and runs +over every selected record, as the row menu and the toolbars already treat it. Before, a whitespace-only +predicate or an envelope whose `source` is blank counted as declared, failed closed for every record and +left nothing eligible. + +The fold is key-neutral, so the same answer reaches the built-in Delete's +`userActions.delete.visibleWhen` on both selection bars: a blank one now admits every selected record +instead of excluding them all. Booleans and real predicates are unchanged, and a blank is reported once +(ADR-0137 D4). diff --git a/packages/core/src/evaluator/__tests__/partitionBlankGate-11322.test.ts b/packages/core/src/evaluator/__tests__/partitionBlankGate-11322.test.ts new file mode 100644 index 0000000000..19bb065b6e --- /dev/null +++ b/packages/core/src/evaluator/__tests__/partitionBlankGate-11322.test.ts @@ -0,0 +1,117 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#11322 — `partitionRowsByPredicate` asks the repo's one "is a gate + * declared?" definition, `hasDeclaredPredicate`, instead of a test of its own. + * + * The fold used to open with `pred == null || pred === ''`. A whitespace-only + * predicate, or an envelope whose `source` is blank, therefore counted as + * DECLARED, was evaluated, failed closed for every record and left nothing + * eligible — which is how the grid's selection bar hid a bulk action that the + * row menu and the toolbars of the same grid show. + * + * ## What each row pins + * + * - The premise: the definition answers "not declared" for both blank + * spellings and "declared" for a boolean. If that stops being true, the + * fold's answer moves with it, which is the point of asking it. + * - The verdict: every row comes back BY REFERENCE with `skipped: 0`, the + * fold's documented "not declared" answer, not merely an equal array. + * - The diagnosis: a blank is still said, once (ADR-0137 D4), read by the + * `[blank]` tag and never by its prose. + * - The controls: no key and `''` were already "not declared"; `false` is a + * declared gate that excludes everything; a real predicate partitions. + * + * ## Fresh module graph per case + * + * The one-time blank report dedupes on MODULE state and the `unit` project runs + * with `isolate: false`, so a spelling another file already reported would read + * as silence here. `vi.resetModules()` + fresh imports give every case its own + * dedupe — the shape `blankGateDiagnosed-11262.test.ts` uses. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; + +let partitionRowsByPredicate: typeof import('../listConditional.js').partitionRowsByPredicate; +let hasDeclaredPredicate: typeof import('../declaredPredicate.js').hasDeclaredPredicate; +let warn: ReturnType; + +beforeEach(async () => { + vi.resetModules(); + ({ partitionRowsByPredicate } = await import('../listConditional.js')); + ({ hasDeclaredPredicate } = await import('../declaredPredicate.js')); + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); +}); +afterEach(() => warn.mockRestore()); + +const blankLines = (): string[] => + warn.mock.calls.map((call: unknown[]) => String(call[0])).filter((w: string) => w.includes('[blank]')); + +const ROWS = [ + { id: 'r1', done: false }, + { id: 'r2', done: true }, + { id: 'r3', done: false }, +]; + +/** The blank shapes the selection bar used to read as a declared gate. */ +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; + +describe('partitionRowsByPredicate asks the one "declared?" definition (objectui#11322)', () => { + it('the premise: blank text in either spelling is not declared, a boolean is', () => { + expect(hasDeclaredPredicate(' ')).toBe(false); + expect(hasDeclaredPredicate({ dialect: 'cel', source: ' ' })).toBe(false); + expect(hasDeclaredPredicate(false)).toBe(true); + expect(hasDeclaredPredicate(true)).toBe(true); + }); + + it.each(BLANKS)('%s: returns every row, by reference', (_label, pred) => { + const { eligible, skipped } = partitionRowsByPredicate(pred as never, ROWS); + expect(eligible).toBe(ROWS); + expect(skipped).toBe(0); + }); + + it.each(BLANKS)('%s: is said once, not answered in silence (ADR-0137 D4)', (_label, pred) => { + partitionRowsByPredicate(pred as never, ROWS); + partitionRowsByPredicate(pred as never, ROWS); + expect(blankLines()).toHaveLength(1); + }); + + describe('controls', () => { + it('no predicate: every row, by reference, and nothing to report', () => { + expect(partitionRowsByPredicate(undefined, ROWS)).toEqual({ eligible: ROWS, skipped: 0 }); + expect(partitionRowsByPredicate(undefined, ROWS).eligible).toBe(ROWS); + expect(partitionRowsByPredicate(null, ROWS).eligible).toBe(ROWS); + expect(blankLines()).toHaveLength(0); + }); + + it("`''`: every row, by reference, as before", () => { + const { eligible, skipped } = partitionRowsByPredicate('', ROWS); + expect(eligible).toBe(ROWS); + expect(skipped).toBe(0); + }); + + it('`false` is a declared gate: no row qualifies', () => { + expect(partitionRowsByPredicate(false, ROWS)).toEqual({ eligible: [], skipped: ROWS.length }); + }); + + it('`true` is a declared gate that admits every row, by reference', () => { + expect(partitionRowsByPredicate(true, ROWS).eligible).toBe(ROWS); + }); + + it('a real predicate still partitions per record', () => { + const { eligible, skipped } = partitionRowsByPredicate('record.done', ROWS); + expect(eligible.map((r) => r.id)).toEqual(['r2']); + expect(skipped).toBe(2); + }); + }); +}); diff --git a/packages/core/src/evaluator/declaredPredicate.ts b/packages/core/src/evaluator/declaredPredicate.ts index 4c97917944..66c610e8ea 100644 --- a/packages/core/src/evaluator/declaredPredicate.ts +++ b/packages/core/src/evaluator/declaredPredicate.ts @@ -164,8 +164,25 @@ 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 two joined with objectui#11322. Both belong to the grid's selection + * bar, and both had kept the old `!= null && !== ''` scope after every other + * member of the family moved here: plugin-grid's `hasVisibilityGate` (which decides whether a + * bulk button whose `visible` admits no record is hidden) asks it through the + * renderer-side name `hasDeclaredVisibilityGate`, and this package's + * `partitionRowsByPredicate` (the per-record bulk fold) asks it directly. A + * whitespace-only `visible` had counted as declared on that bar, failed closed + * for every record, and hidden 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) on row-level items decide + * it by rules of their own, such as the built-in Edit / Delete row item's + * `visibleWhen != null` (`isBuiltinRowActionVisible`). A value of such a key + * that reaches `partitionRowsByPredicate` is answered here like any other. + * ⚠️ 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..577e3e3efb 100644 --- a/packages/core/src/evaluator/listConditional.ts +++ b/packages/core/src/evaluator/listConditional.ts @@ -36,6 +36,7 @@ */ import { evalFieldPredicate, type FieldRulePredicate } from './fieldRules.js'; import { ExpressionEvaluator } from './ExpressionEvaluator.js'; +import { hasDeclaredPredicate } from './declaredPredicate.js'; import { toPredicateRecord, type FieldContainerLike } from '../utils/predicate-record.js'; /** @@ -370,9 +371,14 @@ 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. + * - **not declared** — answered by {@link hasDeclaredPredicate}, the repo's + * one definition of "is a gate declared?": `null` / `undefined`, `''`, + * blank predicate text in either spelling (`' '` and + * `{ dialect: 'cel', source: ' ' }`), the empty envelope, and any value + * that is not a predicate at all. 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. A blank is not silent: that + * definition reports it once (ADR-0137 D4). * - **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,13 +394,31 @@ 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). + * + * ## Why "declared?" is asked of the one definition (objectui#11322) + * + * This fold used to answer it with a test of its own, `pred == null || + * pred === ''`, whose scope was narrower than the family's. A whitespace-only + * `visible` therefore counted as declared here, was evaluated, failed closed + * for every record and left nothing eligible: the grid's selection bar hid a + * bulk action that the same grid's row menu and toolbars show. Asking + * {@link hasDeclaredPredicate} keeps the three surfaces on one answer. + * + * The question is key-neutral, and so is this fold: every value a caller hands + * it gets the same answer. That includes the built-in Delete's + * `userActions.delete.visibleWhen`, which both selection bars pass through here + * (plugin-grid's through `partitionBulkRows`, plugin-list's `ListView` + * directly). A blank one admits every selected record rather than excluding + * them all. The built-in Delete ROW item decides "declared?" for that key by + * `!= null` and still hides on a blank; that row rule belongs to the field-rule + * `*When` keys and is not decided here. */ export function partitionRowsByPredicate>( pred: FieldRulePredicate | boolean | undefined | null, rows: readonly TRow[], opts: Omit = {}, ): RowPartition { - if (pred == null || pred === '') { + if (!hasDeclaredPredicate(pred)) { return { eligible: rows as TRow[], skipped: 0 }; } if (typeof pred === 'boolean') { 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..207a922a0b 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,43 @@ 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 && !== ''` in two places (`hasVisibilityGate` and core's + // `partitionRowsByPredicate`), so a whitespace-only `visible` counted as a + // gate, every selected record failed it, and the button disappeared. Both now + // ask the action family's one definition. + // + // 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__/bulkEligibility.test.ts b/packages/plugin-grid/src/__tests__/bulkEligibility.test.ts index 53894ba397..b40768a6de 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,37 @@ describe('partitionBulkRows', () => { expect(hasVisibilityGate(undefined)).toBe(false); }); }); + + // [objectui#11322] Blank text is no gate, in either spelling. Both halves of + // the selection bar used to ask "declared?" with `!= null && !== ''`, so the + // two spellings below counted as gates, every record failed them, and the bar + // hid an action the row menu and toolbars show. 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/bulkEligibility.ts b/packages/plugin-grid/src/bulkEligibility.ts index 9afb51afda..8fac1865de 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,25 +64,34 @@ 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`) — the + * same definition {@link partitionRowsByPredicate} asks for 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); } /** * Split selected records into the ones this def may act on and a count of the * ones it may not. * - * The loop, the boolean short-circuit and the fail-closed posture all live in + * The loop, the "declared?" question, 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 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: '' }] }); From 88ed8704459264c8737b634d808c7bd5cd630e13 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 1 Oct 2026 06:22:40 +0000 Subject: [PATCH 2/4] test(plugin-grid),docs(types): restate the ast-only bulk `visible` pin the one definition makes false objectui#10946 pinned an `ast`-only envelope on a bulk def's `visible` as evaluated, faulted and failed closed on the selection bar. That was the bar's own "declared?" scope (`!= null && !== ''`); the row menu and the toolbars, which ask `hasDeclaredVisibilityGate`, read the same envelope as no gate. With the bar asking that definition, every selected record qualifies. The pin, the `BulkActionDef.visible` docblock that described the old answer, the fold's docblock and the changesets now say so. Claude-Session: https://claude.ai/code/session_011p7ikEivgXefNDaE5S5Uec Co-authored-by: Claude --- .../11322-bulk-bar-blank-visible-shown.md | 5 ++-- .changeset/11322-bulk-def-visible-docblock.md | 11 +++++++++ .../11322-partition-rows-declared-gate.md | 4 ++++ .../core/src/evaluator/listConditional.ts | 5 +++- .../specExpressionWire-10946.test.tsx | 23 ++++++++++++------- packages/types/src/objectql.ts | 7 ++++-- 6 files changed, 42 insertions(+), 13 deletions(-) create mode 100644 .changeset/11322-bulk-def-visible-docblock.md diff --git a/.changeset/11322-bulk-bar-blank-visible-shown.md b/.changeset/11322-bulk-bar-blank-visible-shown.md index 61909b870b..770a19f901 100644 --- a/.changeset/11322-bulk-bar-blank-visible-shown.md +++ b/.changeset/11322-bulk-bar-blank-visible-shown.md @@ -11,5 +11,6 @@ The selection bar decided whether a bulk action "declared a visibility gate" wit 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`, so blank text in either spelling and the empty envelope are "no gate" here -too, and the blank is reported once as it is on the other surfaces. `visible: false` is unchanged: it is -still a declared gate that hides the button. +too, and the blank is reported once as it is 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. 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-partition-rows-declared-gate.md b/.changeset/11322-partition-rows-declared-gate.md index 965f4195e7..0f0e5f43c5 100644 --- a/.changeset/11322-partition-rows-declared-gate.md +++ b/.changeset/11322-partition-rows-declared-gate.md @@ -10,6 +10,10 @@ over every selected record, as the row menu and the toolbars already treat it. B predicate or an envelope whose `source` is blank counted as declared, failed closed for every record and left nothing eligible. +The definition's whole scope comes with it, not only the blank text: a value that is not a predicate at +all, such as an envelope with an `ast` and no `source`, is now "not declared" here too, so every record +qualifies where the fold used to evaluate it, fault, and qualify none. + The fold is key-neutral, so the same answer reaches the built-in Delete's `userActions.delete.visibleWhen` on both selection bars: a blank one now admits every selected record instead of excluding them all. Booleans and real predicates are unchanged, and a blank is reported once diff --git a/packages/core/src/evaluator/listConditional.ts b/packages/core/src/evaluator/listConditional.ts index 577e3e3efb..2538201ad7 100644 --- a/packages/core/src/evaluator/listConditional.ts +++ b/packages/core/src/evaluator/listConditional.ts @@ -402,7 +402,10 @@ export interface RowPartition { * `visible` therefore counted as declared here, was evaluated, failed closed * for every record and left nothing eligible: the grid's selection bar hid a * bulk action that the same grid's row menu and toolbars show. Asking - * {@link hasDeclaredPredicate} keeps the three surfaces on one answer. + * {@link hasDeclaredPredicate} keeps the three surfaces on one answer, over + * its whole scope: a value that is not a predicate at all, such as an envelope + * with an `ast` and no `source`, is no gate here either, where this fold used + * to evaluate it, fault, and qualify no record. * * The question is key-neutral, and so is this fold: every value a caller hands * it gets the same answer. That includes the built-in Delete's 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/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']; /** From b889377c3f413318b6e29129432862808cc85095 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 1 Oct 2026 07:30:09 +0000 Subject: [PATCH 3/4] fix(plugin-grid,core): the action family asks "declared?" before the shared fold; core's opening test returns to main Triage's ruling on the retriage question (objectui#11322, comment 5926391467): no destructive widening in this PR. A blank built-in Delete `visibleWhen` stays excluded on both bulk bars, as on main. - core: `partitionRowsByPredicate` opens with main's `pred == null || pred === ''` again; its code is byte-identical to the branch point. Its docblock names its callers (the bulk-action fold, and the built-in Delete's `visibleWhen` from `ObjectGrid` and `ListView`), says the action family decides "declared?" before calling it, and no longer calls its test "blank string". `hasDeclaredPredicate`'s docblock names only the consumer that asks it now. - plugin-grid: `partitionBulkRows` asks `hasVisibilityGate` (the action family's `hasDeclaredVisibilityGate`) before handing a def's `visible` to the fold. Measured with core at main's opening: the button showed, but the blank still reached the fold and every record was excluded, so the run would refuse what the bar offered. - plugin-grid: `ObjectGrid`'s built-in Delete hands `userActions.delete.visibleWhen` to `partitionRowsByPredicate` directly, as `ListView` does, with the options `partitionBulkRows` passed (`warnOnError` true, label `delete`). Its answer is main's. - pins: the core-hunk test file is removed. Through the real `ObjectGrid` and `ListView`, a whitespace or blank-envelope built-in Delete `visibleWhen` still excludes every record; the no-gate cases are the controls. The core changeset becomes docblock-only. Claude-Session: https://claude.ai/code/session_011p7ikEivgXefNDaE5S5Uec Co-authored-by: Claude --- .../11322-bulk-bar-blank-visible-shown.md | 20 +-- .../11322-core-docblocks-name-callers.md | 14 +++ .../11322-partition-rows-declared-gate.md | 20 --- .../partitionBlankGate-11322.test.ts | 117 ------------------ .../core/src/evaluator/declaredPredicate.ts | 26 ++-- .../core/src/evaluator/listConditional.ts | 49 ++++---- packages/plugin-grid/src/ObjectGrid.tsx | 22 ++-- .../__tests__/bulk-action-visible.test.tsx | 9 +- .../__tests__/bulkDeleteVisibleWhen.test.tsx | 40 ++++++ .../src/__tests__/bulkEligibility.test.ts | 14 ++- packages/plugin-grid/src/bulkEligibility.ts | 33 +++-- .../ListView.bulkDeleteVisibleWhen.test.tsx | 24 ++++ 12 files changed, 177 insertions(+), 211 deletions(-) create mode 100644 .changeset/11322-core-docblocks-name-callers.md delete mode 100644 .changeset/11322-partition-rows-declared-gate.md delete mode 100644 packages/core/src/evaluator/__tests__/partitionBlankGate-11322.test.ts diff --git a/.changeset/11322-bulk-bar-blank-visible-shown.md b/.changeset/11322-bulk-bar-blank-visible-shown.md index 770a19f901..02104d15df 100644 --- a/.changeset/11322-bulk-bar-blank-visible-shown.md +++ b/.changeset/11322-bulk-bar-blank-visible-shown.md @@ -2,15 +2,21 @@ '@object-ui/plugin-grid': patch --- -A bulk action whose `visible` is blank now shows on the grid's selection bar, as it already does on the -row menu and the toolbars of the same grid (objectui#11322). +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 && !== ''`. A whitespace-only `visible` (`' '`), or an envelope whose `source` is blank +`!= 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`, so blank text in either spelling and the empty envelope are "no gate" here -too, and the blank is reported once as it is 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. +`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-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/.changeset/11322-partition-rows-declared-gate.md b/.changeset/11322-partition-rows-declared-gate.md deleted file mode 100644 index 0f0e5f43c5..0000000000 --- a/.changeset/11322-partition-rows-declared-gate.md +++ /dev/null @@ -1,20 +0,0 @@ ---- -'@object-ui/core': patch ---- - -`partitionRowsByPredicate` asks the repo's one "is a gate declared?" definition, `hasDeclaredPredicate`, -instead of its own `pred == null || pred === ''` test (objectui#11322). - -What a user sees: a bulk action whose `visible` is blank now shows on the grid's selection bar and runs -over every selected record, as the row menu and the toolbars already treat it. Before, a whitespace-only -predicate or an envelope whose `source` is blank counted as declared, failed closed for every record and -left nothing eligible. - -The definition's whole scope comes with it, not only the blank text: a value that is not a predicate at -all, such as an envelope with an `ast` and no `source`, is now "not declared" here too, so every record -qualifies where the fold used to evaluate it, fault, and qualify none. - -The fold is key-neutral, so the same answer reaches the built-in Delete's -`userActions.delete.visibleWhen` on both selection bars: a blank one now admits every selected record -instead of excluding them all. Booleans and real predicates are unchanged, and a blank is reported once -(ADR-0137 D4). diff --git a/packages/core/src/evaluator/__tests__/partitionBlankGate-11322.test.ts b/packages/core/src/evaluator/__tests__/partitionBlankGate-11322.test.ts deleted file mode 100644 index 19bb065b6e..0000000000 --- a/packages/core/src/evaluator/__tests__/partitionBlankGate-11322.test.ts +++ /dev/null @@ -1,117 +0,0 @@ -/** - * ObjectUI - * Copyright (c) 2024-present ObjectStack Inc. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -/** - * objectui#11322 — `partitionRowsByPredicate` asks the repo's one "is a gate - * declared?" definition, `hasDeclaredPredicate`, instead of a test of its own. - * - * The fold used to open with `pred == null || pred === ''`. A whitespace-only - * predicate, or an envelope whose `source` is blank, therefore counted as - * DECLARED, was evaluated, failed closed for every record and left nothing - * eligible — which is how the grid's selection bar hid a bulk action that the - * row menu and the toolbars of the same grid show. - * - * ## What each row pins - * - * - The premise: the definition answers "not declared" for both blank - * spellings and "declared" for a boolean. If that stops being true, the - * fold's answer moves with it, which is the point of asking it. - * - The verdict: every row comes back BY REFERENCE with `skipped: 0`, the - * fold's documented "not declared" answer, not merely an equal array. - * - The diagnosis: a blank is still said, once (ADR-0137 D4), read by the - * `[blank]` tag and never by its prose. - * - The controls: no key and `''` were already "not declared"; `false` is a - * declared gate that excludes everything; a real predicate partitions. - * - * ## Fresh module graph per case - * - * The one-time blank report dedupes on MODULE state and the `unit` project runs - * with `isolate: false`, so a spelling another file already reported would read - * as silence here. `vi.resetModules()` + fresh imports give every case its own - * dedupe — the shape `blankGateDiagnosed-11262.test.ts` uses. - */ - -import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; - -let partitionRowsByPredicate: typeof import('../listConditional.js').partitionRowsByPredicate; -let hasDeclaredPredicate: typeof import('../declaredPredicate.js').hasDeclaredPredicate; -let warn: ReturnType; - -beforeEach(async () => { - vi.resetModules(); - ({ partitionRowsByPredicate } = await import('../listConditional.js')); - ({ hasDeclaredPredicate } = await import('../declaredPredicate.js')); - warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); -}); -afterEach(() => warn.mockRestore()); - -const blankLines = (): string[] => - warn.mock.calls.map((call: unknown[]) => String(call[0])).filter((w: string) => w.includes('[blank]')); - -const ROWS = [ - { id: 'r1', done: false }, - { id: 'r2', done: true }, - { id: 'r3', done: false }, -]; - -/** The blank shapes the selection bar used to read as a declared gate. */ -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; - -describe('partitionRowsByPredicate asks the one "declared?" definition (objectui#11322)', () => { - it('the premise: blank text in either spelling is not declared, a boolean is', () => { - expect(hasDeclaredPredicate(' ')).toBe(false); - expect(hasDeclaredPredicate({ dialect: 'cel', source: ' ' })).toBe(false); - expect(hasDeclaredPredicate(false)).toBe(true); - expect(hasDeclaredPredicate(true)).toBe(true); - }); - - it.each(BLANKS)('%s: returns every row, by reference', (_label, pred) => { - const { eligible, skipped } = partitionRowsByPredicate(pred as never, ROWS); - expect(eligible).toBe(ROWS); - expect(skipped).toBe(0); - }); - - it.each(BLANKS)('%s: is said once, not answered in silence (ADR-0137 D4)', (_label, pred) => { - partitionRowsByPredicate(pred as never, ROWS); - partitionRowsByPredicate(pred as never, ROWS); - expect(blankLines()).toHaveLength(1); - }); - - describe('controls', () => { - it('no predicate: every row, by reference, and nothing to report', () => { - expect(partitionRowsByPredicate(undefined, ROWS)).toEqual({ eligible: ROWS, skipped: 0 }); - expect(partitionRowsByPredicate(undefined, ROWS).eligible).toBe(ROWS); - expect(partitionRowsByPredicate(null, ROWS).eligible).toBe(ROWS); - expect(blankLines()).toHaveLength(0); - }); - - it("`''`: every row, by reference, as before", () => { - const { eligible, skipped } = partitionRowsByPredicate('', ROWS); - expect(eligible).toBe(ROWS); - expect(skipped).toBe(0); - }); - - it('`false` is a declared gate: no row qualifies', () => { - expect(partitionRowsByPredicate(false, ROWS)).toEqual({ eligible: [], skipped: ROWS.length }); - }); - - it('`true` is a declared gate that admits every row, by reference', () => { - expect(partitionRowsByPredicate(true, ROWS).eligible).toBe(ROWS); - }); - - it('a real predicate still partitions per record', () => { - const { eligible, skipped } = partitionRowsByPredicate('record.done', ROWS); - expect(eligible.map((r) => r.id)).toEqual(['r2']); - expect(skipped).toBe(2); - }); - }); -}); diff --git a/packages/core/src/evaluator/declaredPredicate.ts b/packages/core/src/evaluator/declaredPredicate.ts index 66c610e8ea..c8879ef5e1 100644 --- a/packages/core/src/evaluator/declaredPredicate.ts +++ b/packages/core/src/evaluator/declaredPredicate.ts @@ -166,21 +166,23 @@ export function isBlankPredicateText(value: unknown): boolean { * empty spellings by hand and coerced the rest with `Boolean(raw)`, so * `visible: 0` hid an action the renderers showed (objectui#3957). * - * The last two joined with objectui#11322. Both belong to the grid's selection - * bar, and both had kept the old `!= null && !== ''` scope after every other - * member of the family moved here: plugin-grid's `hasVisibilityGate` (which decides whether a - * bulk button whose `visible` admits no record is hidden) asks it through the - * renderer-side name `hasDeclaredVisibilityGate`, and this package's - * `partitionRowsByPredicate` (the per-record bulk fold) asks it directly. A - * whitespace-only `visible` had counted as declared on that bar, failed closed - * for every record, and hidden an action the row menu and toolbars showed. + * 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) on row-level items decide - * it by rules of their own, such as the built-in Edit / Delete row item's - * `visibleWhen != null` (`isBuiltinRowActionVisible`). A value of such a key - * that reaches `partitionRowsByPredicate` is answered here like any other. + * 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. */ diff --git a/packages/core/src/evaluator/listConditional.ts b/packages/core/src/evaluator/listConditional.ts index 2538201ad7..11e00e88f3 100644 --- a/packages/core/src/evaluator/listConditional.ts +++ b/packages/core/src/evaluator/listConditional.ts @@ -36,7 +36,6 @@ */ import { evalFieldPredicate, type FieldRulePredicate } from './fieldRules.js'; import { ExpressionEvaluator } from './ExpressionEvaluator.js'; -import { hasDeclaredPredicate } from './declaredPredicate.js'; import { toPredicateRecord, type FieldContainerLike } from '../utils/predicate-record.js'; /** @@ -371,14 +370,11 @@ export interface RowPartition { * that loop written once, with the three cases each caller was otherwise * re-deriving: * - * - **not declared** — answered by {@link hasDeclaredPredicate}, the repo's - * one definition of "is a gate declared?": `null` / `undefined`, `''`, - * blank predicate text in either spelling (`' '` and - * `{ dialect: 'cel', source: ' ' }`), the empty envelope, and any value - * that is not a predicate at all. 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. A blank is not silent: that - * definition reports it once (ADR-0137 D4). + * - **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 @@ -395,33 +391,30 @@ export interface RowPartition { * including the ones it was written to exclude — so an authored gate is not * weakened, it is inverted for half its inputs (objectui#3067). * - * ## Why "declared?" is asked of the one definition (objectui#11322) + * ## Who calls it, and who decides "is a gate declared?" (objectui#11322) * - * This fold used to answer it with a test of its own, `pred == null || - * pred === ''`, whose scope was narrower than the family's. A whitespace-only - * `visible` therefore counted as declared here, was evaluated, failed closed - * for every record and left nothing eligible: the grid's selection bar hid a - * bulk action that the same grid's row menu and toolbars show. Asking - * {@link hasDeclaredPredicate} keeps the three surfaces on one answer, over - * its whole scope: a value that is not a predicate at all, such as an envelope - * with an `ast` and no `source`, is no gate here either, where this fold used - * to evaluate it, fault, and qualify no record. + * This is a shared partitioner, and its opening test is not the action + * family's "is a gate declared?" question. Its callers: * - * The question is key-neutral, and so is this fold: every value a caller hands - * it gets the same answer. That includes the built-in Delete's - * `userActions.delete.visibleWhen`, which both selection bars pass through here - * (plugin-grid's through `partitionBulkRows`, plugin-list's `ListView` - * directly). A blank one admits every selected record rather than excluding - * them all. The built-in Delete ROW item decides "declared?" for that key by - * `!= null` and still hides on a blank; that row rule belongs to the field-rule - * `*When` keys and is not decided here. + * - 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, rows: readonly TRow[], opts: Omit = {}, ): RowPartition { - if (!hasDeclaredPredicate(pred)) { + if (pred == null || pred === '') { return { eligible: rows as TRow[], skipped: 0 }; } if (typeof pred === 'boolean') { 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 207a922a0b..06bdcdcc4a 100644 --- a/packages/plugin-grid/src/__tests__/bulk-action-visible.test.tsx +++ b/packages/plugin-grid/src/__tests__/bulk-action-visible.test.tsx @@ -77,10 +77,11 @@ describe('BulkActionBar — bulk action visible CEL', () => { // [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 && !== ''` in two places (`hasVisibilityGate` and core's - // `partitionRowsByPredicate`), so a whitespace-only `visible` counted as a - // gate, every selected record failed it, and the button disappeared. Both now - // ask the action family's one definition. + // 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 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 b40768a6de..3744cd18d1 100644 --- a/packages/plugin-grid/src/__tests__/bulkEligibility.test.ts +++ b/packages/plugin-grid/src/__tests__/bulkEligibility.test.ts @@ -135,11 +135,15 @@ describe('partitionBulkRows', () => { }); }); - // [objectui#11322] Blank text is no gate, in either spelling. Both halves of - // the selection bar used to ask "declared?" with `!= null && !== ''`, so the - // two spellings below counted as gates, every record failed them, and the bar - // hid an action the row menu and toolbars show. The `''` row above is the - // control; the boolean rows keep "not declared" from passing as "always". + // [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', ' '], diff --git a/packages/plugin-grid/src/bulkEligibility.ts b/packages/plugin-grid/src/bulkEligibility.ts index 8fac1865de..5a014a5e53 100644 --- a/packages/plugin-grid/src/bulkEligibility.ts +++ b/packages/plugin-grid/src/bulkEligibility.ts @@ -66,9 +66,9 @@ export type BulkEligibilityDef = Pick & { /** * 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`) — the - * same definition {@link partitionRowsByPredicate} asks for the fold, so the - * button and the fold behind it cannot disagree about one def. + * (`@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 @@ -90,13 +90,26 @@ 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. * - * The loop, the "declared?" question, the boolean short-circuit and the - * fail-closed posture all live in + * 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, @@ -112,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-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(); + } + }); }); From 5661606d6f0ef55dfa5c551c09aa35b3372ea1cd Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 1 Oct 2026 07:37:31 +0000 Subject: [PATCH 4/4] docs(changeset): date-note the objectui#10946 entry whose ast-only bulk sentence objectui#11322 makes false The pending `.changeset/10946-expression-wire-slots-by-reference.md` says an `ast`-only bulk `visible` qualifies no record, warned, and that `specExpressionWire-10946.test.tsx` pins that fault "on both". With objectui#11322 (triage 5926391467 point 3) the selection bar asks the action family's definition first, so that envelope is no gate there. An append-only dated note says so; the frontmatter and every existing sentence are unchanged, as in PR objectui#11335's note on the 8655 entry. Claude-Session: https://claude.ai/code/session_011p7ikEivgXefNDaE5S5Uec Co-authored-by: Claude --- .../10946-expression-wire-slots-by-reference.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) 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`.