diff --git a/.changeset/10790-empty-operators-accepted-shape.md b/.changeset/10790-empty-operators-accepted-shape.md index 3294c08bdb..d00c96ef3b 100644 --- a/.changeset/10790-empty-operators-accepted-shape.md +++ b/.changeset/10790-empty-operators-accepted-shape.md @@ -21,3 +21,10 @@ value OR the empty string, and its complement: Criteria saved in the old shape still open in the builder as the same row, and opening one rewrites nothing; the builder writes the new shape the next time any row of that criteria is edited. + +**Correction, 2026-10-03 (objectui#10813).** The two shapes above are not what this release +writes. Later in the same release the pair moved to the spec's one 「is empty」 operator: +"Is empty" stores `{ FIELD: { $empty: true } }` and "Is not empty" `{ FIELD: { $empty: false } }`, +whose meaning is the field's declared row of the spec's per-type table rather than "no value OR +the empty string" on every type. Criteria saved in either earlier shape still open as the same +row and are written as `$empty` on their next edit (`.changeset/10813-filter-condition-empty-operator.md`). diff --git a/.changeset/10813-dataset-filter-empty-operator.md b/.changeset/10813-dataset-filter-empty-operator.md new file mode 100644 index 0000000000..bdf87d553c --- /dev/null +++ b/.changeset/10813-dataset-filter-empty-operator.md @@ -0,0 +1,13 @@ +--- +'@object-ui/app-shell': minor +--- + +The Studio dataset filter builder writes "Is empty" / "Is not empty" as the spec's `{ FIELD: { $empty: true } }` / `{ FIELD: { $empty: false } }` instead of `$exists` (objectui#10813). + +`dataset.filter` and `measure.filter` stored the pair as `{ FIELD: { $exists: false } }` / `{ FIELD: { $exists: true } }`. `$exists` is the spec's has-a-value test (`!= null`), so a text value of `''` or a multi-value `[]` was never "empty" there, while the same operator in the sharing-rule widget and in a saved view meant something else. `@objectstack/spec` 17.6.0 admits `$empty` (objectstack#20446), whose meaning is the column's DECLARED row of the spec's per-type table (ruling B on objectstack#20311): null or `''` on a text-like column, null or `[]` on a multi-value one, null alone on every other type. The analytics service answers it from the field's declared type and `multiple` on its SQL and ObjectQL strategies. The spec's own face table declares one face that refuses it: the `driver-memory` analytics (cube) face, the lowest-priority fallback strategy (`MemoryAnalyticsService`), refuses a newly authored "Is empty" / "Is not empty" filter with `INVALID_FILTER` / 400, where it answered `$exists`. The bridge keeps no copy of the table. + +The `@objectstack/spec` dependency floor rises from `^17.5.0` to `^17.6.0`, because this package now writes `$empty`, which `@objectstack/spec` 17.5.0 declares staged and refuses. + +The read half reads `$empty` back as the pair, with a boolean flag only. + +**What moves for a stored filter.** A stored `$exists` no longer opens as "Is empty" / "Is not empty": it opens in the Source tab, with its bytes untouched, and keeps matching what it matched before. Opening it as the pair would let an edit to ANOTHER row rewrite it to `$empty` and move `''` / `[]` across the line, which the read-half invariant (objectui#10257) forbids, and this inspector offers no `exists` row. To move such a filter to the new meaning, remove the row and add "Is empty" again. Readable stores (this repository, objectstack, hotcrm and cloud) hold no `dataset.filter` with `$exists`. diff --git a/.changeset/10813-filter-condition-empty-operator.md b/.changeset/10813-filter-condition-empty-operator.md new file mode 100644 index 0000000000..94ab1e3ee2 --- /dev/null +++ b/.changeset/10813-filter-condition-empty-operator.md @@ -0,0 +1,20 @@ +--- +'@object-ui/fields': minor +--- + +`FilterConditionField` writes "Is empty" / "Is not empty" as the spec's one 「is empty」 operator, `{ FIELD: { $empty: true } }` / `{ FIELD: { $empty: false } }` (objectui#10813). + +The widget behind sharing-rule `criteria_json`, `relatedListFilter` and `summaryOperations.filter` used to write its own meaning of 「is empty」: `{ $or: [{ FIELD: { $in: [''] } }, { FIELD: { $null: true } }] }` and its complement `{ FIELD: { $nin: [''], $null: false } }`, i.e. "no value OR `''`" on every field type. The `''` member reached a number or date column as `IN ('')`, and a multi-value column (stored as JSON by the SQL driver) as an `$in` that driver refuses. `@objectstack/spec` 17.6.0 admits `$empty` to `FILTER_OPERATORS` (objectstack#20446), and its meaning is the field's DECLARED row of the spec's per-type table (ruling B on objectstack#20311): null or `''` on a text-like field, null or `[]` on a multi-value field, null alone on every other type. Every evaluator expands it itself (`expandEmptyOperator`), so the widget now writes the same token on every column type and keeps no copy of the table. + +The `@objectstack/spec` dependency floor rises from `^17.5.0` to `^17.6.0`, because this package now writes `$empty`, which `@objectstack/spec` 17.5.0 declares staged and refuses. + +`kvToCondition` reads `$empty` back as the pair, with a boolean flag only: any other flag stays the raw criteria it is, as every evaluator refuses it. + +**What moves for a stored rule.** Reading alone rewrites nothing: a criteria saved in either earlier shape (the objectui#10790 `$or` entry and `$nin` + `$null` pair, or the older `$in: [null, '']` / `$nin: [null, '']`) still opens as the same "Is empty" / "Is not empty" row, and an un-edited rule keeps its stored bytes and keeps matching as before. The next time the criteria is edited, those rows are written as `$empty`, which re-scopes the rule: + +- text-like column: no change (both mean null or `''`); +- every other single-valued column (number, boolean, date, datetime, time, select, a single lookup): a row holding `''` is no longer "empty" and becomes "not empty". Since objectstack#20308 the write door stores a cleared number, boolean, date, datetime or time as null, so on those types only a value written before it can hold `''`; a select or lookup can still hold one, and the spec's ruled table does not count it as empty; +- multi-value column (multiselect, checkboxes, tags, or a `multiple: true` select, lookup or user): a row holding `[]` becomes "empty" and leaves "is not empty". On the SQL driver the earlier shape was refused outright, so a rule there starts running; +- the `$in: [null, '']` shapes were refused by every objectstack filter face, so a rule still in them starts running. + +Measured on the installed 17.6.0 by-value readers (`@objectstack/formula`'s `matchesFilterCondition` and `ValueDataSource`) over null, an absent key, `''`, `[]` and set values: the `$or` entry and `$empty: true` disagree only on `[]`. Readable stores (this repository, objectstack, hotcrm and cloud) hold no criteria in either earlier shape. diff --git a/.changeset/10813-list-view-empty-operator.md b/.changeset/10813-list-view-empty-operator.md new file mode 100644 index 0000000000..a4e560fea6 --- /dev/null +++ b/.changeset/10813-list-view-empty-operator.md @@ -0,0 +1,9 @@ +--- +'@object-ui/plugin-list': minor +--- + +The list view's live query sends "Is empty" / "Is not empty" as the spec's `isempty` / `isnotempty` instead of an equality to `null` (objectui#10813). `@objectstack/spec` 17.6.0 and later lower that pair to `$empty`; an earlier reader lowers it to `$null`, as it does the saved view's `is_empty`. + +`convertFilterGroupToAST` resolved the pair to `[FIELD, '=', null]` / `[FIELD, '!=', null]`, a null-only test, before `mapOperator` was consulted. The same rule saved into the view is persisted as `is_empty`, which `@objectstack/spec` 17.6.0 lowers to `$empty` (objectstack#20446): so one filter panel answered two record sets, depending on whether the view had been saved. The pair now takes the value-less path like `is_null`, and `mapOperator` gains `isempty` / `isnotempty` arms. The node is `[FIELD, 'isempty', null]`; the spec discards the third slot. + +**What moves.** Nothing is stored by this path. Against a 17.6.0 or later server, on a text-like column a row holding `''` is now "empty", and on a multi-value column a row holding `[]` is, matching the server's per-type answer for the saved view. On a `provider: 'value'` list a row with no key at all is now "empty" too: the equality to `null` did not select it there. diff --git a/.changeset/9359-list-ast-valueless-canonical-fold.md b/.changeset/9359-list-ast-valueless-canonical-fold.md index e768243a91..a8c3be8ccf 100644 --- a/.changeset/9359-list-ast-valueless-canonical-fold.md +++ b/.changeset/9359-list-ast-valueless-canonical-fold.md @@ -75,3 +75,9 @@ objectui#9306's census), every pair emits the same node except folds onto `icontains` before a row leaves it. The vocabulary question this entry calls open is answered: the dropdown speaks the protocol's ids, and camelCase is the deprecated alias form. + +**Correction, 2026-10-03 (objectui#10813).** The two `isEmpty` / `isNotEmpty` arms that +"resolve to a null comparison ahead of `mapOperator`" are gone in this release. The empty pair +now takes the same value-less path as `is_null`, and `mapOperator` emits the spec's `isempty` / +`isnotempty`, which the spec lowers to `$empty`; every spelling of the pair still emits one node +(`.changeset/10813-list-view-empty-operator.md`). diff --git a/packages/app-shell/package.json b/packages/app-shell/package.json index c7dcba3277..1850cc8526 100644 --- a/packages/app-shell/package.json +++ b/packages/app-shell/package.json @@ -85,7 +85,7 @@ "@object-ui/types": "workspace:*", "@objectstack/formula": "^17.5.0", "@objectstack/lint": "^17.0.0", - "@objectstack/spec": "^17.5.0", + "@objectstack/spec": "^17.6.0", "@sentry/react": "^10.70.0", "jsonc-parser": "^3.3.1", "lucide-react": "^1.43.0", diff --git a/packages/app-shell/src/views/drillNotNullDialect-9508.test.tsx b/packages/app-shell/src/views/drillNotNullDialect-9508.test.tsx index 13265145ba..203216eeee 100644 --- a/packages/app-shell/src/views/drillNotNullDialect-9508.test.tsx +++ b/packages/app-shell/src/views/drillNotNullDialect-9508.test.tsx @@ -44,7 +44,10 @@ * * ⇒ reading only `$null` would have closed the composed route and left the * uncomposed one degrading exactly as before, for an author who picked "is not - * empty" in the dataset filter inspector (which writes the `$exists` pair). + * empty" in the dataset filter inspector, which wrote the `$exists` pair until + * objectui#10813; a filter stored before then still carries it, and the row + * now writes `{ $empty: false }` instead, which this dialect carries on its + * `[empty]` arm (objectui#11547). * * ## What this card did NOT change * diff --git a/packages/app-shell/src/views/drillUrlFilters.ts b/packages/app-shell/src/views/drillUrlFilters.ts index 9062b6d994..67a9fe4374 100644 --- a/packages/app-shell/src/views/drillUrlFilters.ts +++ b/packages/app-shell/src/views/drillUrlFilters.ts @@ -120,8 +120,11 @@ export const RANGE_OP_PARAM: Record = { $gte: 'gte', $lte: 'lte' * that hands its own resolved filter straight to the escape hatch * (`ObjectMetricWidget`, whose drawer renders `OpenInListButton`) passes * through no canonicaliser at all, so `$exists` reaches this function - * verbatim — and the dataset filter inspector's "is not empty" row writes - * exactly that pair. + * verbatim. The dataset filter inspector's "is not empty" row wrote + * exactly that pair until objectui#10813, so a filter stored before then + * still carries it; that row now writes `{ $empty: false }`, which is not + * one of these two keys: it reaches this function verbatim too, and the + * `[empty]` arm, {@link EMPTY_FILTER}, carries it (objectui#11547). * * ⚠️ A NON-boolean under either key says nothing about emptiness and writes * nothing, which is what it did before this pair existed. diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.nullOperators-9363.test.ts b/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.nullOperators-9363.test.ts index 2ad843413b..f3af76ec99 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.nullOperators-9363.test.ts +++ b/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.nullOperators-9363.test.ts @@ -102,9 +102,13 @@ describe('groupToCondition — the null predicates this inspector offers (object ).toEqual({ stage: { $null: true } }); }); - it('leaves the $exists pair exactly as it was', () => { - expect(groupToCondition(row('is_empty'))).toEqual({ closed_at: { $exists: false } }); - expect(groupToCondition(row('is_not_empty'))).toEqual({ closed_at: { $exists: true } }); + it('keeps the empty pair a predicate of its own — `$empty`, not `$null` (objectui#10813)', () => { + // This file's repair left the empty pair on `$exists`; objectui#10813 moved + // it to the spec's one 「is empty」 operator. Either way it is NOT the null + // pair: `$empty` also counts `''` on a text column and `[]` on a + // multi-value one, so folding the two would rewrite the author's choice. + expect(groupToCondition(row('is_empty'))).toEqual({ closed_at: { $empty: true } }); + expect(groupToCondition(row('is_not_empty'))).toEqual({ closed_at: { $empty: false } }); }); it('still drops an operator it does not map, rather than emitting a wrong filter', () => { diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.readHalfHolds-10257.test.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.readHalfHolds-10257.test.tsx index b79cd9425a..d04156d7ac 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.readHalfHolds-10257.test.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.readHalfHolds-10257.test.tsx @@ -208,7 +208,7 @@ describe('class 1 — an incomplete stored value is not opened as a row the next }); it('CONTROL: the value-less tokens carry no value to be incomplete, and stay opened', () => { - for (const other of [{ name: { $exists: false } }, { name: { $null: true } }]) { + for (const other of [{ name: { $empty: true } }, { name: { $null: true } }]) { const { group, representable } = conditionToGroup(beside(other), FIELDS); expect(representable, JSON.stringify(other)).toBe(true); expect(editStage(group, 'lost')).toEqual({ $and: [{ stage: { $eq: 'lost' } }, other] }); @@ -224,7 +224,7 @@ describe('class 2 — a stored token is not opened as an operator the column\'s { stored: { amount: { $in: [1, 2] } }, type: 'number', readAs: 'in' }, // "Every token" includes the value-less arms: the boolean bucket offers // only `equals` / `notEquals`. - { stored: { flag: { $exists: true } }, type: 'boolean', readAs: 'is_not_empty' }, + { stored: { flag: { $empty: false } }, type: 'boolean', readAs: 'is_not_empty' }, { stored: { flag: { $null: false } }, type: 'boolean', readAs: 'is_not_null' }, ]; @@ -262,7 +262,7 @@ describe('class 2 — a stored token is not opened as an operator the column\'s }); it('CONTROL: the value-less tokens on a column whose bucket offers them still open', () => { - for (const stored of [{ name: { $exists: true } }, { closed_at: { $null: false } }, { region: { $exists: false } }]) { + for (const stored of [{ name: { $empty: false } }, { closed_at: { $null: false } }, { region: { $empty: true } }]) { const { group, representable } = conditionToGroup(stored, FIELDS); expect(representable, JSON.stringify(stored)).toBe(true); expect(groupToCondition(group)).toEqual(stored); @@ -315,7 +315,16 @@ describe('the invariant, swept: every row the read half opens, the panel draws a .flatMap((t) => SCALARS.map((v) => ({ stored: { [t]: v }, implicit: false }))), ...['$in', '$nin'].flatMap((t) => LISTS.map((v) => ({ stored: { [t]: v }, implicit: false }))), ...PAIRS.map((v) => ({ stored: { $between: v }, implicit: false })), - ...[true, false].flatMap((b) => [{ stored: { $exists: b }, implicit: false }, { stored: { $null: b }, implicit: false }]), + // `$empty` is what the empty pair writes since objectui#10813; `$exists`, + // what it wrote before, stays in the domain — it is now REFUSED (no + // operator this inspector offers reads it back), which the invariant + // accepts, and a read that still opened it would be named by the + // byte-identical check below. + ...[true, false].flatMap((b) => [ + { stored: { $empty: b }, implicit: false }, + { stored: { $exists: b }, implicit: false }, + { stored: { $null: b }, implicit: false }, + ]), ...[...SCALARS, ...LISTS].map((v) => ({ stored: v, implicit: true })), ]; diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.test.ts b/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.test.ts index 25e4715a6a..17fa129b3c 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.test.ts +++ b/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.test.ts @@ -15,9 +15,38 @@ describe('datasetFilterCondition', () => { ] })).toEqual({ $and: [{ stage: { $eq: 'won' } }, { amount: { $gt: 1000 } }] }); }); - it('maps isEmpty/isNotEmpty to $exists', () => { + it('maps is_empty / is_not_empty to the spec\'s one 「is empty」 operator, `$empty` (objectui#10813)', () => { + // They wrote `$exists` until objectui#10813 — the has-a-value test, which + // never counted `''` or `[]`. The per-type meaning is the spec's expansion, + // so the SAME token is written whatever the column's type. expect(groupToCondition({ logic: 'and', conditions: [{ field: 'closed_at', operator: 'is_not_empty' }] })) - .toEqual({ closed_at: { $exists: true } }); + .toEqual({ closed_at: { $empty: false } }); + expect(groupToCondition({ logic: 'and', conditions: [{ field: 'closed_at', operator: 'is_empty' }] })) + .toEqual({ closed_at: { $empty: true } }); + }); + + it('a stored `$exists` is no longer read back as the empty pair — it opens in the Source tab, bytes untouched (objectui#10813)', () => { + // Read back as `is_empty`, a sibling edit would rewrite it to `$empty` and + // move `''` / `[]` across the line. This inspector offers no `exists` row, + // so the filter goes to the Source tab rather than opening as a row the + // next commit would change. + for (const stored of [{ closed_at: { $exists: false } }, { closed_at: { $exists: true } }]) { + expect(conditionToGroup(stored).representable, JSON.stringify(stored)).toBe(false); + expect(conditionToGroup({ $and: [{ stage: { $eq: 'won' } }, stored] }).representable).toBe(false); + } + // CONTROL: the `$empty` pair this bridge writes opens as the rows. + expect(conditionToGroup({ closed_at: { $empty: true } }).group.conditions.map((c) => c.operator)) + .toEqual(['is_empty']); + expect(conditionToGroup({ closed_at: { $empty: false } }).group.conditions.map((c) => c.operator)) + .toEqual(['is_not_empty']); + }); + + it('a non-boolean `$empty` flag is not opened as a row (objectui#10813)', () => { + // `$empty` is declared `z.boolean()` and every evaluator refuses another + // flag; opened as a row, the next commit would make it runnable. + for (const flag of ['yes', 1, null]) { + expect(conditionToGroup({ closed_at: { $empty: flag } } as never).representable, JSON.stringify(flag)).toBe(false); + } }); it('drops unmapped operators rather than emitting a bad filter', () => { @@ -49,7 +78,7 @@ describe('datasetFilterCondition', () => { ] })).toEqual({ stage: { $eq: 'won' } }); // value-less operators are still kept expect(groupToCondition({ logic: 'and', conditions: [{ field: 'closed_at', operator: 'is_not_empty', value: '' }] })) - .toEqual({ closed_at: { $exists: true } }); + .toEqual({ closed_at: { $empty: false } }); }); it('round-trips representable conditions (condition → group → condition)', () => { @@ -57,7 +86,8 @@ describe('datasetFilterCondition', () => { { status: { $eq: 'won' } }, { $and: [{ stage: { $eq: 'won' } }, { amount: { $gt: 1000 } }] }, { region: { $in: ['NA', 'EU'] } }, - { closed_at: { $exists: false } }, + { closed_at: { $empty: true } }, + { closed_at: { $empty: false } }, ]) { const { group, representable } = conditionToGroup(c); expect(representable).toBe(true); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.ts b/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.ts index bc50245253..161ab5286f 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.ts +++ b/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.ts @@ -16,8 +16,8 @@ * * The value-less operators are the exception to "field op value": the builder * draws no input for them, so the row is complete without one. Both pairs the - * spec's vocabulary carries — `$exists` (is empty) and `$null` (is null) — are - * bridged here, in {@link VALUELESS_TO_MONGO}. + * builder offers — `$empty` (is empty) and `$null` (is null) — are bridged + * here, in {@link VALUELESS_TO_MONGO}. * * An operator that is NOT bridged is dropped, and dropping is where the danger * used to be: see {@link isClearedGroup} for why an unmapped operator is now @@ -102,10 +102,10 @@ const MONGO_TO_OP: Record = { * is exactly how the two halves drift apart. * * Measured over the whole domain the dropdown can build (objectui#9382): four - * tokens have two operators writing them — `$exists`, `$null`, `$gt`, `$lt`. - * The first two are disambiguated by their PAYLOAD, in the `$exists` / `$null` - * arms of {@link conditionToGroup}, because the stored value is the boolean - * that picks the operator. `$gt` / `$lt` carry the author's comparand instead, + * tokens have two operators writing them — `$empty` (`$exists` until + * objectui#10813), `$null`, `$gt`, `$lt`. The first two are disambiguated by + * their PAYLOAD, in the `$empty` / `$null` arms of {@link conditionToGroup}, + * because the stored value is the boolean that picks the operator. `$gt` / `$lt` carry the author's comparand instead, * so no bit of the stored condition tells `after` from `greater_than` — which * is why the field's declared type has to. */ @@ -174,10 +174,22 @@ function readBackOperator(mop: string, fieldType: string | undefined): string | * * `is_null` / `is_not_null` are not a spelling of `is_empty` / `is_not_empty`. The * dropdown offers both pairs as their own rows and the spec's filter vocabulary - * carries both `$null` and `$exists`, so they stay distinct in both directions; + * carries both `$null` and `$empty`, so they stay distinct in both directions; * collapsing them would draw two labels for one wire predicate and rewrite the * author's choice when the filter is read back. * + * objectui#10813: the empty pair writes the spec's ONE 「is empty」 operator, + * `$empty` (ruling B on objectstack#20311; in `FILTER_OPERATORS` and the + * lowering of the view operators `is_empty` / `is_not_empty` since + * objectstack#20446). What counts as empty is the column's DECLARED row of the + * spec's per-type table — `''` on a text-like column, `[]` on a multi-value + * one, null alone on every other type — expanded by each evaluator + * (`expandEmptyOperator`), so ⛔ no copy of that table lives here. It wrote + * `$exists` before — the spec's has-a-value test (`!= null`), which never + * counts `''` or `[]` — so one operator name, offered in three builders, + * matched three record sets. A stored `$exists` is no longer read back as this pair — + * see the note in {@link conditionToGroup}. + * * objectui#9363: the null pair was missing here, so an `Is null` row — an * ordinary entry in this inspector's menu, drawn as a finished row — fell * through to the unmapped-operator `continue` below and was dropped. Dropping @@ -187,7 +199,7 @@ function readBackOperator(mop: string, fieldType: string | undefined): string | * with no error and the condition still on screen. */ const VALUELESS_TO_MONGO: Record> = { - is_empty: { $exists: false }, is_not_empty: { $exists: true }, + is_empty: { $empty: true }, is_not_empty: { $empty: false }, is_null: { $null: true }, is_not_null: { $null: false }, }; @@ -334,7 +346,7 @@ export function groupToCondition(group: BuilderGroup | undefined): FilterConditi * * 2. DOES THE COLUMN'S BUCKET OFFER ITS OPERATOR? Read back as an operator the * dropdown does not list — `$in` on a date or number column, `$gt` on a - * text one, `$exists` on a boolean one — the panel draws a BLANK operator + * text one, `$empty` on a boolean one — the panel draws a BLANK operator * trigger (objectui#4768 / #7561), and one touch of the row's field picker * reconciles it to `equals` and reshapes the value, committing a different * filter than the one stored (the objectui#9382 defect). The bucket is @@ -394,8 +406,24 @@ export function conditionToGroup( const opKeys = Object.keys(v); if (opKeys.length !== 1) return { group: empty, representable: false }; const mop = opKeys[0]; - if (mop === '$exists') { - row = { id: `c${i}`, field, operator: v.$exists ? 'is_not_empty' : 'is_empty', value: '' }; + if (mop === '$empty') { + // The inverse of the write half, as for `$null` below — but only a + // BOOLEAN flag: the spec declares `$empty: z.boolean()` and every + // evaluator refuses any other, so a non-boolean one goes to the Source + // tab rather than opening as a row the next commit would make runnable. + // + // ⛔ A stored `$exists` no longer reads back as `is_empty` / + // `is_not_empty` (objectui#10813). It is what this pair WROTE before, + // but it is the has-a-value test (`!= null`) and `$empty` is not: read + // back as the pair, a sibling edit would rewrite it to `$empty` and + // move `''` / `[]` across the line — a stored filter changed by an + // edit to a DIFFERENT row, which the invariant pinned for + // {@link builderHolds} (objectui#10257) forbids. + // This inspector offers no `exists` row either, so `$exists` falls to + // the unmapped-token arm below and the filter opens in the Source tab, + // stored bytes untouched. + if (typeof v.$empty !== 'boolean') return { group: empty, representable: false }; + row = { id: `c${i}`, field, operator: v.$empty ? 'is_empty' : 'is_not_empty', value: '' }; } else if (mop === '$null') { // The inverse of the write half: `$null: false` is "is not null", so // the boolean picks the operator rather than becoming the row's value. diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/filter-builder-protocol-ids-census-9306.test.ts b/packages/app-shell/src/views/metadata-admin/inspectors/filter-builder-protocol-ids-census-9306.test.ts index b6816062b7..cb172ee7d6 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/filter-builder-protocol-ids-census-9306.test.ts +++ b/packages/app-shell/src/views/metadata-admin/inspectors/filter-builder-protocol-ids-census-9306.test.ts @@ -28,10 +28,18 @@ * A second, later exception is deliberate and is a CHANGED predicate: the * `mongo` cells for `is_empty` / `is_not_empty` (objectui#10790). They stored * `{ f: { $in: [null, ''] } }` / `{ f: { $nin: [null, ''] } }`, and a `null` - * list member is refused by every objectstack filter face, so those rows now - * store the spelling that refusal prescribes — same meaning, "no value OR the - * empty string" and its complement. "Is empty" is an `$or` entry rather than a - * field entry, and `kvToCondition` reads that entry back as the one row. + * list member is refused by every objectstack filter face, so those rows + * stored the spelling that refusal prescribes — "no value OR the empty string" + * and its complement, as an `$or` entry. + * + * A third, also a CHANGED predicate, and in BOTH columns: the same two rows + * since objectui#10813. Three builders offered 「is empty」 with three meanings + * — this `mongo` column's "no value OR `''`" on every type, the `dataset` + * column's `$exists` (has no value), and the live grid's equality to `null` — + * and all three now store the spec's ONE operator, `{ f: { $empty: true } }` / + * `{ f: { $empty: false } }`, whose meaning is the column's declared row of the + * spec's per-type table (ruling B on objectstack#20311). Both read halves read + * it back as the same row. * * - `mongo` / `readBack` — `@object-ui/fields`' sharing-rule criteria * (`condToMongo`, then `kvToCondition` on what it wrote). @@ -77,9 +85,9 @@ const CENSUS: ReadonlyArray<{ // The named exception in the `dataset` column — see the file header. { legacy: 'containsCaseInsensitive', id: 'icontains', value: 'x', mongo: { f: { $icontains: 'x' } }, readBack: 'icontains', dataset: { f: { $icontains: 'x' } }, datasetReadBack: 'icontains' }, { legacy: 'notContains', id: 'not_contains', value: 'x', mongo: { f: { $notContains: 'x' } }, readBack: 'not_contains', dataset: { f: { $notContains: 'x' } }, datasetReadBack: 'not_contains' }, - // The named exception in the `mongo` column since objectui#10790 — see the file header. - { legacy: 'isEmpty', id: 'is_empty', value: '', mongo: { $or: [{ f: { $in: [''] } }, { f: { $null: true } }] }, readBack: 'is_empty', dataset: { f: { $exists: false } }, datasetReadBack: 'is_empty' }, - { legacy: 'isNotEmpty', id: 'is_not_empty', value: '', mongo: { f: { $nin: [''], $null: false } }, readBack: 'is_not_empty', dataset: { f: { $exists: true } }, datasetReadBack: 'is_not_empty' }, + // The named exceptions in BOTH columns since objectui#10813 (the `mongo` one also objectui#10790) — see the file header. + { legacy: 'isEmpty', id: 'is_empty', value: '', mongo: { f: { $empty: true } }, readBack: 'is_empty', dataset: { f: { $empty: true } }, datasetReadBack: 'is_empty' }, + { legacy: 'isNotEmpty', id: 'is_not_empty', value: '', mongo: { f: { $empty: false } }, readBack: 'is_not_empty', dataset: { f: { $empty: false } }, datasetReadBack: 'is_not_empty' }, { legacy: 'greaterThan', id: 'greater_than', value: 5, mongo: { f: { $gt: 5 } }, readBack: 'greater_than', dataset: { f: { $gt: 5 } }, datasetReadBack: 'greater_than' }, { legacy: 'lessThan', id: 'less_than', value: 5, mongo: { f: { $lt: 5 } }, readBack: 'less_than', dataset: { f: { $lt: 5 } }, datasetReadBack: 'less_than' }, { legacy: 'greaterOrEqual', id: 'greater_than_or_equal', value: 5, mongo: { f: { $gte: 5 } }, readBack: 'greater_than_or_equal', dataset: { f: { $gte: 5 } }, datasetReadBack: 'greater_than_or_equal' }, diff --git a/packages/fields/package.json b/packages/fields/package.json index 8130f899e2..af598172e8 100644 --- a/packages/fields/package.json +++ b/packages/fields/package.json @@ -37,7 +37,7 @@ "@object-ui/providers": "workspace:*", "@object-ui/react": "workspace:*", "@object-ui/types": "workspace:*", - "@objectstack/spec": "^17.5.0", + "@objectstack/spec": "^17.6.0", "lucide-react": "^1.43.0", "react-markdown": "^10.1.0", "rehype-sanitize": "^6.0.0", diff --git a/packages/fields/src/widgets/FilterConditionField.tsx b/packages/fields/src/widgets/FilterConditionField.tsx index d81fa3370e..525e38a25b 100644 --- a/packages/fields/src/widgets/FilterConditionField.tsx +++ b/packages/fields/src/widgets/FilterConditionField.tsx @@ -200,29 +200,6 @@ function toArray(value: any): any[] { return value == null ? [] : [value]; } -/** - * "Is empty" as stored (objectui#10790): the field has NO value, OR its value - * is the empty string. - * - * This used to be `{ [field]: { $in: [null, ''] } }`, and `null` is not a list - * member any objectstack filter face accepts: the shared comparand-shape face - * (`assertListComparandShapes`) refuses it with `INVALID_FILTER` / 400 in every - * position (ruled 2026-08-31), so every related list, roll-up and sharing rule - * authored with this operator failed when it was evaluated. The spelling here - * is the one that refusal prescribes for "one of […] OR has no value" — - * `$or: [{ FIELD: { $in: […] } }, { FIELD: { $null: true } }]` — with the one - * non-null member the old list carried. The meaning is unchanged: `''` still - * counts as empty, which is what separates this operator from `is_null`. - * - * ONE stored entry per builder row, like every other arm: its key is `$or` - * rather than the field, so it merges into an AND group beside field keys and - * {@link kvToCondition} reads it back as one row. Two such rows collide on - * `$or` and fall to the `$and` form, as any two rows on one key already do. - */ -function isEmptyEntry(field: string): Record { - return { $or: [{ [field]: { $in: [''] } }, { [field]: { $null: true } }] }; -} - function isPlainObject(v: unknown): v is Record { return v !== null && typeof v === 'object' && !Array.isArray(v); } @@ -239,10 +216,17 @@ function soleFieldOperator(frag: unknown): [string, string, unknown] | null { } /** - * The FIELD of a stored {@link isEmptyEntry}, or `null` when `v` (the value - * under an `$or` key) is anything else — including an `$or` that means the same - * thing in another order or spelling, which reads back as the ordinary OR group - * it is rather than being folded into this row. + * The FIELD of a stored objectui#10790 "is empty" entry — + * `$or: [{ FIELD: { $in: [''] } }, { FIELD: { $null: true } }]` — or `null` + * when `v` (the value under an `$or` key) is anything else, including an `$or` + * that means the same thing in another order or spelling, which reads back as + * the ordinary OR group it is rather than being folded into this row. + * + * READ-ONLY since objectui#10813: this widget no longer WRITES that entry (see + * the `is_empty` arm of {@link condToMongo}), but a criteria saved before then + * still carries it, so it keeps opening as the same `is_empty` row and is + * written in the spec's `$empty` spelling the next time the criteria is + * edited. Reading alone rewrites nothing. */ function isEmptyEntryField(v: unknown): string | null { if (!Array.isArray(v) || v.length !== 2) return null; @@ -309,14 +293,21 @@ export function condToMongo(c: BuilderCondition, typeOf: (f: string) => string | // builder UI even though FieldOperatorsSchema accepts them (#2942). case 'starts_with': return { [field]: { $startsWith: value } }; case 'ends_with': return { [field]: { $endsWith: value } }; - // objectui#10790 — no `null` list member: see {@link isEmptyEntry}. - // "Is not empty" is its exact complement — has a value (`$null: false`, - // the refusal's own "has a value" half) AND that value is not `''` — on one - // field key, so it reads back through the ordinary two-operator arm. - case 'is_empty': return isEmptyEntry(field); - case 'is_not_empty': return { [field]: { $nin: [''], $null: false } }; + // objectui#10813 — the spec's ONE 「is empty」 operator, `$empty` (ruling B + // on objectstack#20311; admitted to `FILTER_OPERATORS` and made the lowering + // of the view operators `is_empty` / `is_not_empty` by objectstack#20446). + // What counts as empty is the field's DECLARED row of the spec's per-type + // table — `''` on a text-like field, `[]` on a multi-value one, null alone + // on every other type — and every evaluator expands it itself + // (`expandEmptyOperator`). ⛔ No local copy of that table: this arm used to + // spell its own meaning, "no value OR `''`" on EVERY type (objectui#10790's + // `$or` of `$in: ['']` and `$null`), which reached a number or date column + // as `IN ('')` and a JSON-stored multi-value column as an `$in` the SQL + // driver refuses. `false` is the exact complement. + case 'is_empty': return { [field]: { $empty: true } }; + case 'is_not_empty': return { [field]: { $empty: false } }; // Null / existence spec operators. Distinct from is_empty/is_not_empty, - // which also treat '' as empty. + // whose `$empty` also counts `''` (text-like) and `[]` (multi-value). case 'is_null': return { [field]: { $null: true } }; case 'is_not_null': return { [field]: { $null: false } }; case 'exists': return { [field]: { $exists: true } }; @@ -427,8 +418,9 @@ function criteriaKey(mongo: any): string { */ export function kvToCondition(field: string, v: any, idx: number): BuilderCondition | null { // A `$` key is a logical operator, not a field. The one such entry a builder - // row stores is "is empty"'s `$or` (objectui#10790, {@link isEmptyEntry}); - // every other one is a criteria the builder cannot draw as a row. + // row ever stored is objectui#10790's "is empty" `$or` ({@link isEmptyEntryField}, + // read-only since objectui#10813); every other one is a criteria the builder + // cannot draw as a row. if (field.startsWith('$')) { const target = field === '$or' ? isEmptyEntryField(v) : null; return target === null @@ -463,11 +455,23 @@ export function kvToCondition(field: string, v: any, idx: number): BuilderCondit case '$endsWith': return { id, field, operator: 'ends_with', value: val }; case '$null': return { id, field, operator: val === false ? 'is_not_null' : 'is_null', value: '' }; case '$exists': return { id, field, operator: val === false ? 'notExists' : 'exists', value: '' }; + // objectui#10813 — what `condToMongo` writes for the empty pair. Only a + // BOOLEAN flag reads as a row: the spec declares `$empty: z.boolean()` + // and every evaluator refuses any other flag, so a `{ $empty: 'yes' }` + // stays the raw criteria it is rather than opening as a row whose next + // save would quietly turn it into a predicate that runs. + case '$empty': + return val === true + ? { id, field, operator: 'is_empty', value: '' } + : val === false + ? { id, field, operator: 'is_not_empty', value: '' } + : null; // `[null, '']` is the pre-objectui#10790 spelling of "is empty" / // "is not empty", which every objectstack filter face refuses. Rules // saved before the fix still carry it, so it keeps opening as the same - // row — and the builder writes the accepted spelling the next time the - // criteria is edited. Reading alone rewrites nothing. + // row — and the builder writes the accepted spelling (`$empty`, since + // objectui#10813) the next time the criteria is edited. Reading alone + // rewrites nothing. case '$in': return arraysEqual(val, [null, '']) ? { id, field, operator: 'is_empty', value: '' } @@ -482,7 +486,9 @@ export function kvToCondition(field: string, v: any, idx: number): BuilderCondit if (opKeys.length === 2 && '$gte' in v && '$lte' in v) { return { id, field, operator: 'between', value: [v.$gte, v.$lte] }; } - // "Is not empty" as `condToMongo` writes it since objectui#10790. + // "Is not empty" as `condToMongo` wrote it from objectui#10790 until + // objectui#10813 moved the pair onto `$empty`: read-only, so a criteria saved + // in between keeps opening as the same row (see {@link isEmptyEntryField}). if (opKeys.length === 2 && '$nin' in v && '$null' in v && arraysEqual(v.$nin, ['']) && v.$null === false) { return { id, field, operator: 'is_not_empty', value: '' }; } @@ -495,8 +501,9 @@ function mongoToFilterGroup(mongo: any): BuilderGroup | null { if (typeof mongo !== 'object' || Array.isArray(mongo)) return null; const entries = Object.entries(mongo); if (entries.length === 0) return { ...EMPTY_GROUP, conditions: [] }; - // A lone "is empty" row stores a lone `$or` (objectui#10790); it is ONE row, - // not an OR group of its two halves. Read it as the flat entry it is, below. + // A lone "is empty" row stored a lone `$or` from objectui#10790 until + // objectui#10813; it is ONE row, not an OR group of its two halves. Read it + // as the flat entry it is, below. if (entries.length === 1 && (mongo.$or || mongo.$and) && !kvToCondition(entries[0][0], entries[0][1], 0)) { const logic: 'and' | 'or' = mongo.$or ? 'or' : 'and'; const arr = mongo.$or || mongo.$and; @@ -506,8 +513,8 @@ function mongoToFilterGroup(mongo: any): BuilderGroup | null { const frag = arr[i]; if (!frag || typeof frag !== 'object' || Object.keys(frag).length !== 1) return null; const field = Object.keys(frag)[0]; - // A `$` key reads only as "is empty"'s entry; `kvToCondition` answers - // `null` for every other one. + // A `$` key reads only as the legacy "is empty" entry; `kvToCondition` + // answers `null` for every other one. const c = kvToCondition(field, frag[field], i); if (!c) return null; conditions.push(c); @@ -517,9 +524,9 @@ function mongoToFilterGroup(mongo: any): BuilderGroup | null { const conditions: BuilderCondition[] = []; let i = 0; for (const [field, v] of entries) { - // Mixed logical + field → raw, except "is empty"'s `$or` entry, which an - // AND group merges beside field keys (`kvToCondition` answers `null` for - // every other `$` key). + // Mixed logical + field → raw, except the legacy "is empty" `$or` entry, + // which an AND group merged beside field keys (`kvToCondition` answers + // `null` for every other `$` key). const c = kvToCondition(field, v, i++); if (!c) return null; conditions.push(c); diff --git a/packages/fields/src/widgets/__tests__/FilterConditionField.emptyOperators-10790.test.tsx b/packages/fields/src/widgets/__tests__/FilterConditionField.emptyOperators-10790.test.tsx index fc56699fc9..e074bbe883 100644 --- a/packages/fields/src/widgets/__tests__/FilterConditionField.emptyOperators-10790.test.tsx +++ b/packages/fields/src/widgets/__tests__/FilterConditionField.emptyOperators-10790.test.tsx @@ -7,51 +7,63 @@ */ /** - * "Is empty" / "Is not empty" write a criteria every objectstack filter face - * accepts (objectui#10790). + * "Is empty" / "Is not empty" write the spec's ONE 「is empty」 operator, + * `$empty` (objectui#10813), and every shape the widget wrote before keeps + * opening as the same row (objectui#10790). * - * The widget used to store them as `{ FIELD: { $in: [null, ''] } }` and - * `{ FIELD: { $nin: [null, ''] } }`. A `null` list member is refused by the - * shared comparand-shape face (`assertListComparandShapes`, `INVALID_FILTER` / - * 400, ruled 2026-08-31) in every position, so a related list, roll-up or - * sharing rule authored with either operator failed when it was evaluated. The - * stored shapes are now the ones that refusal prescribes: + * The stored shapes, in order: * - * - "Is empty": `{ $or: [{ FIELD: { $in: [''] } }, { FIELD: { $null: true } }] }` - * - "Is not empty": `{ FIELD: { $nin: [''], $null: false } }` — its complement. + * - before objectui#10790: `{ FIELD: { $in: [null, ''] } }` / + * `{ FIELD: { $nin: [null, ''] } }`. A `null` list member is refused by the + * shared comparand-shape face (`assertListComparandShapes`, `INVALID_FILTER` + * / 400), so a rule authored with either failed when evaluated; + * - objectui#10790 to objectui#10813: `{ $or: [{ FIELD: { $in: [''] } }, + * { FIELD: { $null: true } }] }` / `{ FIELD: { $nin: [''], $null: false } }` + * — "no value OR `''`" on EVERY field type, one of the three meanings of + * 「is empty」 objectui#10813 converged; + * - since objectui#10813: `{ FIELD: { $empty: true } }` / `{ FIELD: { $empty: + * false } }`. What counts as empty is the field's DECLARED row of the spec's + * per-type table (ruling B on objectstack#20311), expanded by every + * evaluator (`expandEmptyOperator`); the widget keeps no copy of it, which + * is why the WRITER block pins the SAME token on every column type. * * What is pinned is the DOCUMENT the widget hands `onChange` — the string a * `relatedListFilter`, a roll-up filter or a `criteria_json` ends up holding — * judged by the objectstack faces themselves, imported from the installed - * `@objectstack/spec`: the query face (`assertListComparandShapes`) and the - * save door (`FilterConditionSchema`). The exact bytes are pinned too, so a - * shape that merely passes the faces but means something else is still red. + * `@objectstack/spec`: the query face (`assertListComparandShapes`), the save + * door (`FilterConditionSchema`), and `FILTER_OPERATORS`, the list every + * executor derives its accepted operators from. The exact bytes are pinned + * too, so a shape that merely passes the faces but means something else is + * still red. * * Four blocks: * * 1. WRITER — each operator on each field type that offers it (text, number, - * date, select, lookup: `operatorsForFieldType` in `@object-ui/components`), - * driven through the REAL dropdowns. The `equals` rows are the control: - * the same harness and the same faces, green before and after. - * 2. READER — the old shape AND the new shape each open as the same single - * builder row, and opening one emits nothing (no rewrite on read alone). - * 3. RE-SAVE — an old rule is written in the new shape the next time any row - * of it is edited. - * 4. GROUPS — the new "is empty" entry is a `$or` key, so it is pinned in an - * AND group (merged beside field keys), beside a second "is empty" (the - * `$and` form) and inside an OR group, each read back as the rows written. + * date, select, lookup, and multiselect, whose JSON column the SQL driver + * refused the old `$in` on), driven through the REAL dropdowns. The + * `equals` rows are the control: the same harness and the same faces, + * green before and after. + * 2. READER — the two legacy shapes and the new one each open as the same + * single builder row, and opening one emits nothing (no rewrite on read + * alone). + * 3. RE-SAVE — a legacy rule is written as `$empty` the next time any row of + * it is edited. That is the stored-rule reading objectui#10813's + * changeset names: re-saving moves a rule to the declared-type meaning. + * 4. GROUPS — the new entry is a FIELD key like every other row, so two + * "is empty" rows merge into one AND object instead of the `$and` form the + * legacy `$or` entry needed; a legacy group still reads back as its rows. * - * DIRECTION, predicted before running: on the base tree block 1's two operator - * rows are red (the old bytes, and the face's refusal), block 2's new-shape rows - * are red ("is empty" opens as two OR rows, "is not empty" as raw JSON), block 3 - * and block 4 are red, and block 2's old-shape rows and every `equals` control - * are green. + * DIRECTION, predicted before running on the base tree (the objectui#10790 + * writer): block 1's operator rows are red (the legacy bytes), the `equals` + * controls are green; block 2's legacy rows are green and its `$empty` rows red + * (`kvToCondition` had no `$empty` arm, so the widget fell to raw JSON); block + * 3 is red; block 4's legacy-group rows are green and its new-shape rows red. */ import { describe, it, expect, vi } from 'vitest'; import React from 'react'; import { render, screen, fireEvent, waitFor } from '@testing-library/react'; import '@testing-library/jest-dom'; -import { assertListComparandShapes, FilterConditionSchema } from '@objectstack/spec/data'; +import { assertListComparandShapes, FilterConditionSchema, FILTER_OPERATORS } from '@objectstack/spec/data'; import { FilterConditionField } from '../FilterConditionField'; /** @@ -74,6 +86,15 @@ const OBJECT_SCHEMA = { ], }, { name: 'account', label: 'Account', type: 'lookup', reference: 'account' }, + { + name: 'tags', + label: 'Tags', + type: 'multiselect', + options: [ + { value: 'a', label: 'A' }, + { value: 'b', label: 'B' }, + ], + }, ], }; @@ -142,8 +163,12 @@ function expectAcceptedByTheFaces(stored: string) { expect(parsed.success, JSON.stringify(parsed.success ? null : parsed.error.issues)).toBe(true); } -const isEmpty = (f: string) => ({ $or: [{ [f]: { $in: [''] } }, { [f]: { $null: true } }] }); -const isNotEmpty = (f: string) => ({ [f]: { $nin: [''], $null: false } }); +/** The pair as the widget writes it since objectui#10813. */ +const isEmpty = (f: string) => ({ [f]: { $empty: true } }); +const isNotEmpty = (f: string) => ({ [f]: { $empty: false } }); +/** objectui#10790's shapes, written until objectui#10813 — still READ. */ +const legacyIsEmpty = (f: string) => ({ $or: [{ [f]: { $in: [''] } }, { [f]: { $null: true } }] }); +const legacyIsNotEmpty = (f: string) => ({ [f]: { $nin: [''], $null: false } }); const TYPES: ReadonlyArray<{ type: string; field: string; label: string }> = [ { type: 'text', field: 'name', label: 'Name' }, @@ -151,6 +176,7 @@ const TYPES: ReadonlyArray<{ type: string; field: string; label: string }> = [ { type: 'date', field: 'due_on', label: 'Due on' }, { type: 'select', field: 'stage', label: 'Stage' }, { type: 'lookup', field: 'account', label: 'Account' }, + { type: 'multiselect', field: 'tags', label: 'Tags' }, ]; /** A fresh row on `label`'s column, still on the seed operator (`equals`). */ @@ -161,11 +187,19 @@ async function freshRowOn(label: string) { return utils; } -describe('WRITER — each operator on each offered field type writes the accepted shape (objectui#10790)', () => { +describe('WRITER — each operator on each offered field type writes `$empty` (objectui#10813)', () => { + it('the operator it writes is one every executor accepts', () => { + // `FILTER_OPERATORS` is the list the executors derive acceptance from; the + // operator joined it in `@objectstack/spec` 17.6.0 (objectstack#20446). + expect(FILTER_OPERATORS as readonly string[]).toContain('$empty'); + }); + it.each(TYPES)('"Is empty" on a $type column', async ({ field, label }) => { const { onChange } = await freshRowOn(label); await pickFrom(1, 'Is empty'); const stored = lastEmitted(onChange); + // The SAME token on every column type: the per-type meaning is the spec's + // expansion, not this widget's. expect(stored).toBe(JSON.stringify(isEmpty(field))); expectAcceptedByTheFaces(stored); }); @@ -201,12 +235,14 @@ async function expectOneRow(fieldLabel: string, operatorLabel: string) { expect(screen.queryByPlaceholderText(/"type": "customer"/)).toBeNull(); } -describe('READER — the old and the new shape open as the same builder row (objectui#10790)', () => { +describe('READER — every shape the pair was ever stored in opens as the same builder row (objectui#10790, objectui#10813)', () => { const CASES: ReadonlyArray<{ name: string; stored: unknown; operator: string }> = [ - { name: 'old "is empty" ($in: [null, \'\'])', stored: { name: { $in: [null, ''] } }, operator: 'Is empty' }, - { name: 'old "is not empty" ($nin: [null, \'\'])', stored: { name: { $nin: [null, ''] } }, operator: 'Is not empty' }, - { name: 'new "is empty"', stored: isEmpty('name'), operator: 'Is empty' }, - { name: 'new "is not empty"', stored: isNotEmpty('name'), operator: 'Is not empty' }, + { name: 'pre-objectui#10790 "is empty" ($in: [null, \'\'])', stored: { name: { $in: [null, ''] } }, operator: 'Is empty' }, + { name: 'pre-objectui#10790 "is not empty" ($nin: [null, \'\'])', stored: { name: { $nin: [null, ''] } }, operator: 'Is not empty' }, + { name: 'objectui#10790 "is empty" ($or)', stored: legacyIsEmpty('name'), operator: 'Is empty' }, + { name: 'objectui#10790 "is not empty" ($nin + $null)', stored: legacyIsNotEmpty('name'), operator: 'Is not empty' }, + { name: '"is empty" ($empty: true)', stored: isEmpty('name'), operator: 'Is empty' }, + { name: '"is not empty" ($empty: false)', stored: isNotEmpty('name'), operator: 'Is not empty' }, ]; it.each(CASES)('$name opens as one "$operator" row and is not rewritten', async ({ stored, operator }) => { @@ -218,11 +254,13 @@ describe('READER — the old and the new shape open as the same builder row (obj }); }); -describe('RE-SAVE — an old rule is written in the new shape once it is edited (objectui#10790)', () => { +describe('RE-SAVE — a legacy rule is written as `$empty` once it is edited (objectui#10813)', () => { it.each([ - { name: '"is empty"', old: { name: { $in: [null, ''] } }, fresh: isEmpty('name') }, - { name: '"is not empty"', old: { name: { $nin: [null, ''] } }, fresh: isNotEmpty('name') }, - ])('an old $name row is rewritten when ANOTHER row is edited', async ({ old, fresh }) => { + { name: 'pre-objectui#10790 "is empty"', old: { name: { $in: [null, ''] } }, fresh: isEmpty('name') }, + { name: 'pre-objectui#10790 "is not empty"', old: { name: { $nin: [null, ''] } }, fresh: isNotEmpty('name') }, + { name: 'objectui#10790 "is empty"', old: legacyIsEmpty('name'), fresh: isEmpty('name') }, + { name: 'objectui#10790 "is not empty"', old: legacyIsNotEmpty('name'), fresh: isNotEmpty('name') }, + ])('a $name row is rewritten when ANOTHER row is edited', async ({ old, fresh }) => { const { onChange } = renderWidget(JSON.stringify({ ...old, amount: 5 })); const box = await screen.findByDisplayValue('5'); expect(onChange).not.toHaveBeenCalled(); @@ -233,7 +271,7 @@ describe('RE-SAVE — an old rule is written in the new shape once it is edited }); }); -describe('GROUPS — the "is empty" entry round-trips inside a group (objectui#10790)', () => { +describe('GROUPS — the "is empty" row round-trips inside a group (objectui#10813)', () => { it('beside a field key (AND, merged): written, accepted, and read back as the two rows', async () => { const { onChange } = await freshRowOn('Name'); await pickFrom(1, 'Is empty'); @@ -248,8 +286,22 @@ describe('GROUPS — the "is empty" entry round-trips inside a group (objectui#1 expect(triggers.map((t) => t.textContent)).toEqual(['Name', 'Is empty', 'Amount', 'Equals']); }); - it('beside a second "is empty" (the $and form): read back as two rows', async () => { - const stored = { $and: [isEmpty('name'), isEmpty('amount')] }; + it('two "is empty" rows are two field keys of one AND object — no `$and` form needed', async () => { + const { onChange } = await freshRowOn('Name'); + await pickFrom(1, 'Is empty'); + await addRow(); + await pickFrom(2, 'Amount'); + await pickFrom(3, 'Is empty'); + const stored = lastEmitted(onChange); + expect(stored).toBe(JSON.stringify({ ...isEmpty('name'), ...isEmpty('amount') })); + expectAcceptedByTheFaces(stored); + expect(screen.getAllByRole('combobox').map((t) => t.textContent)).toEqual([ + 'Name', 'Is empty', 'Amount', 'Is empty', + ]); + }); + + it('a legacy pair of "is empty" entries (the `$and` form) still reads back as two rows', async () => { + const stored = { $and: [legacyIsEmpty('name'), legacyIsEmpty('amount')] }; expectAcceptedByTheFaces(JSON.stringify(stored)); const { onChange } = renderWidget(JSON.stringify(stored)); await waitFor(() => { @@ -271,8 +323,17 @@ describe('GROUPS — the "is empty" entry round-trips inside a group (objectui#1 expect(lastEmitted(onChange)).toBe(JSON.stringify({ $or: [{ amount: 6 }, isEmpty('name')] })); }); - it('CONTROL: an OR of the two halves in the OTHER order stays the OR group it is', async () => { - // Only the exact entry the builder writes is folded into one row; the same + it('a legacy entry inside an OR group is re-written as `$empty` once the group is edited', async () => { + const stored = { $or: [{ amount: 5 }, legacyIsEmpty('name')] }; + const { onChange } = renderWidget(JSON.stringify(stored)); + const box = await screen.findByDisplayValue('5'); + expect(onChange).not.toHaveBeenCalled(); + fireEvent.change(box, { target: { value: '6' } }); + expect(lastEmitted(onChange)).toBe(JSON.stringify({ $or: [{ amount: 6 }, isEmpty('name')] })); + }); + + it('CONTROL: an OR of the two legacy halves in the OTHER order stays the OR group it is', async () => { + // Only the exact entry the builder wrote is folded into one row; the same // predicate spelled another way is not guessed at. // A select column, whose bucket offers both `is_null` and `in`. const stored = { $or: [{ stage: { $null: true } }, { stage: { $in: [''] } }] }; @@ -283,4 +344,18 @@ describe('GROUPS — the "is empty" entry round-trips inside a group (objectui#1 ]); }); }); + + it('CONTROL: a non-boolean `$empty` flag is not opened as a row — the raw criteria stays as written', async () => { + // Every evaluator refuses it (`$empty` is declared `z.boolean()`); opening + // it as an "Is empty" row would let the next save turn it into a runnable + // predicate the author never wrote. + const stored = { name: { $empty: 'yes' } }; + const { onChange } = renderWidget(JSON.stringify(stored)); + // The raw-JSON editor, holding the stored bytes — the widget's fallback for + // a criteria it cannot draw. + const raw = await screen.findByPlaceholderText(/"type": "customer"/); + expect(raw).toHaveValue(JSON.stringify(stored)); + expect(screen.queryAllByRole('combobox')).toHaveLength(0); + expect(onChange).not.toHaveBeenCalled(); + }); }); diff --git a/packages/fields/src/widgets/__tests__/FilterConditionField.operators.test.ts b/packages/fields/src/widgets/__tests__/FilterConditionField.operators.test.ts index e7ef01cc84..38af86b30c 100644 --- a/packages/fields/src/widgets/__tests__/FilterConditionField.operators.test.ts +++ b/packages/fields/src/widgets/__tests__/FilterConditionField.operators.test.ts @@ -90,30 +90,27 @@ const noTypes = () => undefined; * not a run. */ /* - * `$empty` arrived with `@objectstack/spec` 17.5.0 (objectui#11073) as a STAGED - * operator: declared, absent from `FILTER_OPERATORS`, refused by every query - * executor, so a dropdown row emitting it would have authored a filter nothing - * could run. The entry was to leave this set the day `FILTER_OPERATORS` admitted - * it, and the expiry row below reddened on `@objectstack/spec` 17.6.0 - * (objectstack#20446), which admitted it and lowers `is_empty` / `is_not_empty` - * to it. + * `$empty` was here from `@objectstack/spec` 17.5.0 (objectui#11073) until + * objectui#10813, and its two reasons expired in turn: * - * ⚠️ It has NOT left, and the reason changed rather than expired. The staging - * reason is gone; what remains is which builder row should author `$empty`. - * This widget's `is_empty` writes "no value OR `''`" on every field type - * (objectui#10790), one of the three meanings of 「is empty」 objectui#10813 - * reconciles. Moving that row onto `$empty` changes what a stored sharing rule - * or roll-up filter selects, so it is that card's decision, not the 17.6.0 - * bump's (objectui#11094 executed the operator and left the builders alone). - * The entry now leaves the day a builder operator emits `$empty`: the second - * row below the ratchet reddens then. + * 1. STAGED — declared, absent from `FILTER_OPERATORS`, refused by every + * query executor, so a row emitting it would have authored a filter + * nothing could run. Expired on `@objectstack/spec` 17.6.0 + * (objectstack#20446), which admitted it and lowers `is_empty` / + * `is_not_empty` to it; the first expiry row below holds that. + * 2. No builder row authored it — this widget's `is_empty` wrote "no value OR + * `''`" on every field type (objectui#10790), one of the three meanings of + * 「is empty」. Expired with objectui#10813, which moved the pair onto + * `$empty`; the second expiry row below holds that, and the sweep above it + * now holds the token to the spec like every other. */ -const KNOWN_UNREACHABLE = new Set(['$eq', '$between', '$like', '$ilike', '$empty']); +const KNOWN_UNREACHABLE = new Set(['$eq', '$between', '$like', '$ilike']); /** * Pull the operator keys out of a `{ field: { $op: v } }` fragment — descending - * into a `$or` / `$and` entry, which is how "is empty" is stored since - * objectui#10790, so its inner operators are judged like every other row's. + * into a `$or` / `$and` entry, so a row stored as a combinator (as "is empty" + * was from objectui#10790 until objectui#10813) has its inner operators judged + * like every other row's. */ function operatorsOf(frag: Record | null): string[] { if (!frag) return []; @@ -197,12 +194,12 @@ describe('every spec field operator is reachable from the builder (#2942)', () = expect(FILTER_OPERATORS as readonly string[]).toContain('$null'); }); - it('the `$empty` exclusion now expires when a builder operator emits it (objectui#10813)', () => { - // The entry's remaining reason, held mechanically: no drawable builder id - // writes `$empty` today. The day objectui#10813 moves a row onto it, this - // reddens and `$empty` leaves KNOWN_UNREACHABLE, so the sweep above starts - // holding that row to the spec like every other. Derived from the same - // drawable vocabulary the sweep feeds `condToMongo`. + it('the `$empty` exclusion expired: a builder operator emits it (objectui#10813)', () => { + // The objectui#11094 expiry row, flipped. It read "no drawable builder id + // writes `$empty`" and reddened when objectui#10813 moved the empty pair + // onto it, which is why `$empty` left KNOWN_UNREACHABLE. Derived from the + // same drawable vocabulary the sweep feeds `condToMongo`, and the pair is + // named so the row says WHICH operators author it. const emitted = new Set(); for (const operator of FILTER_BUILDER_OPERATORS) { const value = operator === 'in' || operator === 'not_in' ? ['a'] : operator === 'between' ? [1, 5] : 'a'; @@ -211,7 +208,13 @@ describe('every spec field operator is reachable from the builder (#2942)', () = } // Lit control: the sweep really reads emitted operators. expect(emitted.has('$null')).toBe(true); - expect(emitted.has('$empty')).toBe(false); + expect(emitted.has('$empty')).toBe(true); + expect(FILTER_BUILDER_OPERATORS).toContain('is_empty'); + expect(FILTER_BUILDER_OPERATORS).toContain('is_not_empty'); + expect(operatorsOf(condToMongo({ id: 'c1', field: 'f', operator: 'is_empty', value: '' }, noTypes))) + .toEqual(['$empty']); + expect(operatorsOf(condToMongo({ id: 'c1', field: 'f', operator: 'is_not_empty', value: '' }, noTypes))) + .toEqual(['$empty']); }); it('every KNOWN_UNREACHABLE token is still a spec operator (the exclusion ratchet)', () => { @@ -302,6 +305,10 @@ describe('kvToCondition round-trips what condToMongo writes', () => { ['is_not_null', ''], ['exists', ''], ['notExists', ''], + // objectui#10813: the pair is a field key again (`$empty`), so it rides the + // same per-field round trip as every other row. + ['is_empty', ''], + ['is_not_empty', ''], ]; it.each(cases)('%s survives the round trip', (operator, value) => { @@ -323,6 +330,16 @@ describe('kvToCondition round-trips what condToMongo writes', () => { it('rejects an operator it cannot represent rather than guessing', () => { expect(kvToCondition('name', { $nope: 'x' }, 0)).toBeNull(); }); + + it('reads `$empty` only with a boolean flag — any other flag is a criteria it cannot represent', () => { + // `$empty` is declared `z.boolean()` and every evaluator refuses another + // flag; opened as a row, the next save would make it runnable. + expect(kvToCondition('name', { $empty: true }, 0)).toMatchObject({ operator: 'is_empty' }); + expect(kvToCondition('name', { $empty: false }, 0)).toMatchObject({ operator: 'is_not_empty' }); + for (const flag of ['yes', 1, null, 'true']) { + expect(kvToCondition('name', { $empty: flag }, 0), JSON.stringify(flag)).toBeNull(); + } + }); }); /** @@ -379,12 +396,12 @@ describe('objectui#8748 — an unfinished text row is dropped, not emitted', () .toEqual({ name: { $exists: true } }); expect(condToMongo({ id: 'c3', field: 'name', operator: 'notExists', value: '' } as any, noTypes)) .toEqual({ name: { $exists: false } }); - // objectui#10790: no `null` list member — the shapes the objectstack - // faces accept (`FilterConditionField.emptyOperators-10790.test.tsx`). + // objectui#10813: the spec's `$empty` (pinned per column type in + // `FilterConditionField.emptyOperators-10790.test.tsx`). expect(condToMongo({ id: 'c4', field: 'name', operator: 'is_empty', value: '' } as any, noTypes)) - .toEqual({ $or: [{ name: { $in: [''] } }, { name: { $null: true } }] }); + .toEqual({ name: { $empty: true } }); expect(condToMongo({ id: 'c5', field: 'name', operator: 'is_not_empty', value: '' } as any, noTypes)) - .toEqual({ name: { $nin: [''], $null: false } }); + .toEqual({ name: { $empty: false } }); }); it('equals with an empty comparand still emits — it is a real predicate', () => { diff --git a/packages/plugin-list/src/ListView.tsx b/packages/plugin-list/src/ListView.tsx index 5cb221ca85..5295667cef 100644 --- a/packages/plugin-list/src/ListView.tsx +++ b/packages/plugin-list/src/ListView.tsx @@ -454,6 +454,15 @@ export function mapOperator(op: string) { case 'between': return 'between'; case 'isnull': return 'isnull'; case 'isnotnull': return 'isnotnull'; + // objectui#10813 — the empty pair, which the spec lowers to its ONE + // 「is empty」 operator, `$empty` (objectstack#20446), expanded by the + // column's declared type on the server. It used to be answered by two arms + // in `convertFilterGroupToAST` as an equality to `null` — a null-only test + // that never counted `''` or `[]`, while the SAME rule saved into the view + // (`foldFilterGroupToSpecRules` persists `is_empty`) ran as `$empty`: one + // panel, two record sets, depending on whether the view had been saved. + case 'isempty': return 'isempty'; + case 'isnotempty': return 'isnotempty'; default: return op; } } @@ -729,14 +738,16 @@ export function convertFilterGroupToAST(group: FilterGroup): any[] { return isFilterValueComplete(c.operator, c.value); }) .map(c => { - // Folded, not compared raw (objectui#9359). These two arms resolve to a - // null comparison BEFORE `mapOperator` is consulted, so leaving them on - // literal camelCase ids would have made the repair below reach `is_null` - // and not `is_empty` — trading one spelling-dependent answer for another, - // which is the defect this card is about rather than a fix for it. - const canonicalOperator = String(normalizeFilterOperator(c.operator)); - if (canonicalOperator === 'is_empty') return [c.field, '=', null]; - if (canonicalOperator === 'is_not_empty') return [c.field, '!=', null]; + // objectui#10813 — `is_empty` / `is_not_empty` no longer have arms of + // their own here. They were answered as `[field, '=' | '!=', null]`, a + // null test; they now take the value-less path below like `is_null`, and + // `mapOperator` emits the spec's `isempty` / `isnotempty`, which the + // spec lowers to `$empty` — the same operator a saved view's `is_empty` + // rule already ran as. The fold objectui#9359 added for those two arms + // lives on in `isValuelessFilterOperator` and in `mapOperator`'s + // case- and underscore-insensitive match, so every spelling of the pair + // still lands on one node. + // // A value-less row's third slot is emitted as `null` rather than as // whatever `c.value` still holds: the operator dropdown PRESERVES the // previous operator's value, so an `Is null` row can carry a leftover @@ -744,7 +755,9 @@ export function convertFilterGroupToAST(group: FilterGroup): any[] { // (`convertComparison`, `@objectstack/spec/data`) ignores the third slot // for `isnull`/`isnotnull` — it emits `{ [field]: { $null: true|false } }` // — so `null` is inert on the wire and keeps the emission a function of - // the operator alone. Same shape the `isEmpty` arms above already use. + // the operator alone. The spec discards the slot for `isempty` / + // `isnotempty` too (`parseFilterAST(['x', 'isempty', null])` is + // `{ x: { $empty: true } }`). // The same fold as the short-circuit above (objectui#9359): a row kept // BECAUSE it is value-less must also be EMITTED as value-less, or the // canonical spelling would carry its stale `value` into the third slot diff --git a/packages/plugin-list/src/__tests__/convertFilterGroupToAST.canonicalSpelling.test.ts b/packages/plugin-list/src/__tests__/convertFilterGroupToAST.canonicalSpelling.test.ts index 4a31548eda..e9a7883326 100644 --- a/packages/plugin-list/src/__tests__/convertFilterGroupToAST.canonicalSpelling.test.ts +++ b/packages/plugin-list/src/__tests__/convertFilterGroupToAST.canonicalSpelling.test.ts @@ -131,14 +131,14 @@ const ROWS: ReadonlyArray<{ { operator: 'is_null', emitted: ['title', 'isnull', null], dialect: 'dropdown' }, { operator: 'isNotNull', emitted: ['title', 'isnotnull', null], dialect: 'deprecated' }, { operator: 'is_not_null', emitted: ['title', 'isnotnull', null], dialect: 'dropdown' }, - // `isEmpty` / `isNotEmpty` are resolved to a null comparison BEFORE - // `mapOperator` is consulted, so their canonical twins must land on the same - // arm — otherwise the repair would trade one spelling-dependent answer for - // another, which is the defect this card is about. - { operator: 'isEmpty', emitted: ['title', '=', null], dialect: 'deprecated' }, - { operator: 'is_empty', emitted: ['title', '=', null], dialect: 'dropdown' }, - { operator: 'isNotEmpty', emitted: ['title', '!=', null], dialect: 'deprecated' }, - { operator: 'is_not_empty', emitted: ['title', '!=', null], dialect: 'dropdown' }, + // The empty pair, on the spec's `isempty` / `isnotempty` since + // objectui#10813 (lowered to `$empty`). It was resolved to a null comparison + // (`'=' | '!=', null`) before `mapOperator` was consulted; both spellings + // must still land on ONE node, now through `mapOperator`'s fold. + { operator: 'isEmpty', emitted: ['title', 'isempty', null], dialect: 'deprecated' }, + { operator: 'is_empty', emitted: ['title', 'isempty', null], dialect: 'dropdown' }, + { operator: 'isNotEmpty', emitted: ['title', 'isnotempty', null], dialect: 'deprecated' }, + { operator: 'is_not_empty', emitted: ['title', 'isnotempty', null], dialect: 'dropdown' }, // No canonical twin exists for these two. { operator: 'exists', emitted: ['title', 'exists', null], dialect: 'dropdown' }, { operator: 'notExists', emitted: ['title', 'notExists', null], dialect: 'dropdown' }, diff --git a/packages/plugin-list/src/__tests__/convertFilterGroupToAST.test.ts b/packages/plugin-list/src/__tests__/convertFilterGroupToAST.test.ts index eb776ca0dc..316a739aaa 100644 --- a/packages/plugin-list/src/__tests__/convertFilterGroupToAST.test.ts +++ b/packages/plugin-list/src/__tests__/convertFilterGroupToAST.test.ts @@ -37,7 +37,9 @@ describe('convertFilterGroupToAST', () => { // `isEmpty` is a deprecated stored spelling, not a builder id: since // objectui#9306 the row type says so, hence the `unknown` hop. } as unknown as FilterGroup; - expect(convertFilterGroupToAST(group)).toEqual(['x', '=', null]); + // The spec's `isempty`, lowered to `$empty` (objectui#10813) — it was an + // equality to `null` before, a null-only test. + expect(convertFilterGroupToAST(group)).toEqual(['x', 'isempty', null]); }); it('keeps a fresh `Is null` row — no value is that row’s finished state', () => { @@ -94,10 +96,11 @@ describe('convertFilterGroupToAST — every value-less operator emits a real nod // Keyed by the builder's own ids, which are the protocol's canonical // spellings since objectui#9306 (camelCase when this table was written). const EMITTED: Record = { - // Resolved to a null comparison before `mapOperator` is consulted — these - // two already worked, and are pinned so the fix cannot regress them. - is_empty: ['f', '=', null], - is_not_empty: ['f', '!=', null], + // The spec's empty pair, lowered to `$empty` (objectui#10813). They were + // resolved to a null comparison (`'=' | '!=', null`) before `mapOperator` + // was consulted, a null-only test the spec's `is_empty` no longer means. + is_empty: ['f', 'isempty', null], + is_not_empty: ['f', 'isnotempty', null], // The defect. `mapOperator` has had these rows all along and both spellings // are members of `VALID_AST_OPERATORS`; the row simply never reached it. is_null: ['f', 'isnull', null], diff --git a/packages/plugin-list/src/__tests__/filter-operator-ast-parity.test.ts b/packages/plugin-list/src/__tests__/filter-operator-ast-parity.test.ts index 53b68dedfe..b5ae299a22 100644 --- a/packages/plugin-list/src/__tests__/filter-operator-ast-parity.test.ts +++ b/packages/plugin-list/src/__tests__/filter-operator-ast-parity.test.ts @@ -62,7 +62,7 @@ * gives this file its teeth. */ import { describe, it, expect } from 'vitest'; -import { VALID_AST_OPERATORS, isFilterAST } from '@objectstack/spec/data'; +import { VALID_AST_OPERATORS, isFilterAST, parseFilterAST } from '@objectstack/spec/data'; import { VIEW_FILTER_OPERATORS, VIEW_FILTER_OPERATOR_ALIASES } from '@objectstack/spec/ui'; import { mapOperator, normalizeFilterCondition } from '../ListView'; @@ -96,42 +96,16 @@ const EXPECTED_AST_TARGET: Record = { before: '<', // case 'before' ─┬ the pair that regressed after: '>', // case 'after' ─┘ between: 'between', // case 'between' - - // The only two rows with no arm of their own: they fall through to - // `default: return op`, so the expected target IS the view spelling. Pinned - // as identity rather than omitted, so that a future branch claiming to - // "handle" either of them has to come and say so here. - // - // Why identity is right for these two and was wrong for `before`/`after`: - // the FilterBuilder path never reaches `mapOperator` with them at all — - // `convertFilterGroupToAST` rewrites its own camelCase spellings - // (`isEmpty` / `isNotEmpty`) to `[field, '=' | '!=', null]` first — and the - // canonical snake_case spellings are themselves accepted by the AST gate, so - // passing them through unchanged is not a silent drop. That second half is a - // fact about the AST vocabulary, i.e. exactly the kind of fact that moved - // under this file before, so it is asserted rather than assumed: see - // 'the identity rows are ones the AST gate accepts unchanged' below. - is_empty: 'is_empty', - is_not_empty: 'is_not_empty', + // objectui#10813. These two were the only rows with no arm of their own — + // pinned as identity, because `convertFilterGroupToAST` resolved the pair + // to `[field, '=' | '!=', null]` before `mapOperator` was consulted. That + // null test is not what the spec's `is_empty` means since `$empty` (ruling B + // on objectstack#20311, lowered from the pair by objectstack#20446), so the + // pair now takes the value-less path like `is_null`, through real arms. + is_empty: 'isempty', // case 'isempty' + is_not_empty: 'isnotempty', // case 'isnotempty' }; -/** - * Operators this bridge deliberately resolves without reaching the AST gate. - * - * Every token here must still be a member of `VIEW_FILTER_OPERATORS` — the - * ratchet below enforces it. Subtracting a name the spec has retired excuses - * nothing and must be deleted rather than left as a dead subtraction (#3628). - * - * This set narrows the two secondary sweeps only. `EXPECTED_AST_TARGET` above - * subtracts nothing: it is total over the vocabulary, so no exclusion set can - * quietly hollow out the file's primary guarantee. - */ -const HANDLED_BEFORE_MAPPING = new Set([ - // convertFilterGroupToAST rewrites these to `[field, '=' | '!=', null]` - // before mapOperator is consulted, so they never need an AST spelling. - 'is_empty', 'is_not_empty', -]); - describe('mapOperator bridges the spec view vocabulary onto the AST vocabulary', () => { it('reads both vocabularies from the spec', () => { // Guards every assertion below against silently passing on an empty list. @@ -139,32 +113,6 @@ describe('mapOperator bridges the spec view vocabulary onto the AST vocabulary', expect(VALID_AST_OPERATORS.size).toBeGreaterThan(0); }); - // The exclusion ratchet (#3628). The two secondary sweeps below subtract a - // hand-written set from a spec-derived vocabulary, and that subtraction only - // excuses something while the spec still lists the subtracted tokens. Once - // upstream retires or renames one, the sweep stays green (it is still total - // over what remains) but the row becomes dead weight, and its comment goes on - // telling the next reader that "the view layer rewrites this first" about an - // operator no author can declare any more. That is the shape that rotted 37 of - // 82 deny-list entries in #3601 with nothing to report it — a hand-written - // list beside a spec-derived vocabulary and no assertion that its members - // still exist in that vocabulary. - // - // Collected rather than asserted per entry on purpose (same call as PR #3623): - // vocabulary retirements land as whole families, and failing on the first entry - // would hide the rest. - it('every HANDLED_BEFORE_MAPPING token is still in the spec view vocabulary', () => { - const vocabulary = new Set(VIEW_FILTER_OPERATORS); - const retired = [...HANDLED_BEFORE_MAPPING].filter((op) => !vocabulary.has(op)); - expect( - retired, - `VIEW_FILTER_OPERATORS no longer lists these HANDLED_BEFORE_MAPPING tokens: ` - + `${retired.join(', ')}. The spec has retired them, so subtracting them from ` - + 'the sweeps below excuses nothing — delete each from the set (with the comment ' - + 'claiming the view layer rewrites it) rather than leaving a dead subtraction', - ).toEqual([]); - }); - // The totality ratchet for the pin table (#3641). Both directions matter and // they fail for different reasons: // @@ -212,25 +160,23 @@ describe('mapOperator bridges the spec view vocabulary onto the AST vocabulary', }, ); - it('the identity rows are ones the AST gate accepts unchanged', () => { - // `is_empty` / `is_not_empty` are pinned to themselves above, which is only - // safe while the AST gate accepts those spellings verbatim. Asserting a - // fixed pair of literals here cannot be cancelled by vocabulary growth — it - // can only go red, which is the point: if upstream ever retires these - // spellings from the AST vocabulary, the identity stops being a pass-through - // and starts being a silent drop, and mapOperator needs real branches. - for (const op of ['is_empty', 'is_not_empty']) { - expect(EXPECTED_AST_TARGET[op], `${op} is expected to be pinned as identity`).toBe(op); - expect( - VALID_AST_OPERATORS.has(op), - `VALID_AST_OPERATORS no longer accepts '${op}', so mapOperator passing it ` - + 'through unchanged is now a silently dropped filter. Give it a real branch ' - + 'in mapOperator and pin the new target in EXPECTED_AST_TARGET', - ).toBe(true); - } + it('the empty pair reaches the wire as the spec\'s `$empty`, not as a null test (objectui#10813)', () => { + // The meaning, asked of the spec's own lowering rather than assumed: the + // node the live grid emits for each row of the pair lowers to `$empty`, + // which every evaluator expands by the column's declared type. The null + // pair is the control — a different operator, still lowered to `$null`. + expect(parseFilterAST(['f', mapOperator('is_empty'), null] as never)).toEqual({ f: { $empty: true } }); + expect(parseFilterAST(['f', mapOperator('is_not_empty'), null] as never)).toEqual({ f: { $empty: false } }); + expect(parseFilterAST(['f', mapOperator('isEmpty'), null] as never)).toEqual({ f: { $empty: true } }); + expect(parseFilterAST(['f', mapOperator('is_null'), null] as never)).toEqual({ f: { $null: true } }); }); - const bridged = VIEW_FILTER_OPERATORS.filter((op) => !HANDLED_BEFORE_MAPPING.has(op)); + // Every canonical view operator. A `HANDLED_BEFORE_MAPPING` set subtracted + // `is_empty` / `is_not_empty` here until objectui#10813, while + // `convertFilterGroupToAST` resolved the pair before mapOperator was asked; + // with no operator left in it, the set and its exclusion ratchet (#3628) were + // deleted rather than kept as a subtraction of nothing. + const bridged = [...VIEW_FILTER_OPERATORS]; // Secondary (#3641): this is why a wrong target matters, not what detects one. // On its own it does not discriminate — the AST vocabulary already spells the @@ -272,7 +218,6 @@ describe('mapOperator bridges the spec view vocabulary onto the AST vocabulary', // membership form an identity mapOperator passed this too, since the AST // vocabulary spells most of these aliases verbatim as well. const mismatched = Object.keys(VIEW_FILTER_OPERATOR_ALIASES) - .filter((alias) => !HANDLED_BEFORE_MAPPING.has(VIEW_FILTER_OPERATOR_ALIASES[alias])) .map((alias) => { const canonical = VIEW_FILTER_OPERATOR_ALIASES[alias]; return { alias, canonical, expected: EXPECTED_AST_TARGET[canonical], actual: mapOperator(alias) }; diff --git a/packages/plugin-list/src/__tests__/list-offered-operator-expressible-parity.test.ts b/packages/plugin-list/src/__tests__/list-offered-operator-expressible-parity.test.ts index 0de6393b1a..09b2a2eda6 100644 --- a/packages/plugin-list/src/__tests__/list-offered-operator-expressible-parity.test.ts +++ b/packages/plugin-list/src/__tests__/list-offered-operator-expressible-parity.test.ts @@ -146,9 +146,9 @@ function probeValue(operator: string): unknown { /** * Leg 1 — the live grid. Drives the REAL production path rather than reasoning - * about `mapOperator` alone, because `convertFilterGroupToAST` resolves some - * ids (`isEmpty` / `isNotEmpty`) to a null comparison before the bridge is ever - * consulted, and those are legitimately expressible without an AST spelling. + * about `mapOperator` alone: what reaches the wire is `convertFilterGroupToAST`'s + * node, and that function has resolved some ids before the bridge was ever + * consulted (the empty pair did, as a null comparison, until objectui#10813). * * The emitted node is required to be NON-EMPTY. Without that, the assertion is * a tautology waiting to happen: a condition dropped as incomplete yields `[]`, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 73b5b94e76..31220e8c78 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -780,7 +780,7 @@ importers: specifier: ^17.0.0 version: 17.6.0(ai@7.0.65(zod@4.6.5)) '@objectstack/spec': - specifier: ^17.5.0 + specifier: ^17.6.0 version: 17.6.0(ai@7.0.65(zod@4.6.5)) '@sentry/react': specifier: ^10.70.0 @@ -1292,7 +1292,7 @@ importers: specifier: workspace:* version: link:../types '@objectstack/spec': - specifier: ^17.5.0 + specifier: ^17.6.0 version: 17.6.0(ai@7.0.65(zod@4.6.5)) lucide-react: specifier: ^1.43.0