From 9847519e3e68d2d471be8dc473cc49a498d57e38 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Aug 2026 22:42:57 +0000 Subject: [PATCH] fix(components,fields,plugin-list): withhold exists/notExists where no dialect can express them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The shared FilterBuilder offered `exists` / `notExists` to every consumer, but only one of the three at-rest dialects it feeds has an existence operator. Measured on @objectstack/spec 17.0.0-rc.6: neither VIEW_FILTER_OPERATORS nor VALID_AST_OPERATORS contains one under any spelling (zero members matching /exist/); only the MongoDB-style FieldOperatorsSchema criteria carries $exists. So on the list toolbar `mapOperator`'s `default:` arm returned the id verbatim and the query went out as ['name','exists','x'], which isFilterAST() rejects — an unfiltered read or a 400. Saving the same panel as a view produced a rule ViewFilterRuleSchema refuses, and the dataset inspector dropped the condition silently. The pair moves behind the existing OPT_IN_OPERATORS gate and FilterConditionField — which writes the criteria dialect — opts in, so $exists stays authorable where it works. Mapping onto isNotNull/isNull was rejected: those already have their own rows, the round trip is lossy, and $exists = has-value is not yet settled across drivers. Closing the class: objectui's three operator-parity guards all sweep spec vocabulary to objectui, never the reverse. A new parity test forces the set the list toolbar offers to EQUAL the set its two dialects can express, in both directions, so neither an unexpressible offering nor a stale withdrawal can survive. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01RnQd8iMMUwXQEV1crFmQiQ --- ...ist-filter-existence-operators-withheld.md | 51 ++++ .../app-shell/src/views/viewFilterFold.ts | 19 +- .../filter-builder-opt-in-operators.test.ts | 69 ++++- .../components/src/custom/filter-builder.tsx | 85 ++++-- .../src/widgets/FilterConditionField.tsx | 24 +- .../FilterConditionField.operators.test.ts | 31 ++ packages/plugin-list/src/ListView.tsx | 24 ++ ...ffered-operator-expressible-parity.test.ts | 278 ++++++++++++++++++ 8 files changed, 540 insertions(+), 41 deletions(-) create mode 100644 .changeset/list-filter-existence-operators-withheld.md create mode 100644 packages/plugin-list/src/__tests__/list-offered-operator-expressible-parity.test.ts diff --git a/.changeset/list-filter-existence-operators-withheld.md b/.changeset/list-filter-existence-operators-withheld.md new file mode 100644 index 0000000000..dec2cf2d61 --- /dev/null +++ b/.changeset/list-filter-existence-operators-withheld.md @@ -0,0 +1,51 @@ +--- +'@object-ui/components': patch +'@object-ui/fields': patch +'@object-ui/plugin-list': patch +'@object-ui/app-shell': patch +--- + +The list filter builder no longer offers `Is set` / `Is not set`, which its query dialects cannot express. + +**User-visible before/after.** The operator dropdown in the list toolbar's +filter popover — and in the Studio view/tab/page filter inspectors, the dataset +inspector and the generic `filter` config field — loses two rows: **"Is set"** +and **"Is not set"**. The sharing-rule criteria builder (`FilterConditionField`) +keeps both, unchanged. `Is null` / `Is not null` and `Is empty` / `Is not empty` +are untouched everywhere and remain the way to filter on a missing value from +the list. + +Nothing that worked stops working. Every save path behind those two rows was +already broken, in three different ways depending on the surface: + +- **Live grid** — `ListView.mapOperator` had no row for either id, so its + `default:` arm returned the id verbatim and the query went out as + `['name', 'exists', 'x']`. `exists` is not a member of the spec's + `VALID_AST_OPERATORS`, so `isFilterAST()` rejects the shape: an unfiltered + read or a 400, never the filter the user asked for. +- **Save as view** — `foldFilterGroupToSpecRules` normalizes through the spec's + `normalizeFilterOperator`, which does not know the pair, and + `ViewFilterRuleSchema`'s enum then refuses the rule. +- **Dataset inspector** — `groupToCondition` has no row either and drops the + condition silently, so the filter simply never applied. + +**Why withheld rather than mapped.** Measured on `@objectstack/spec` +17.0.0-rc.6: neither `VIEW_FILTER_OPERATORS` nor `VALID_AST_OPERATORS` contains +an existence operator, under any spelling — both sets have zero members matching +`/exist/`. Only the MongoDB-style `FieldOperatorsSchema` criteria carries +`$exists`, and that is precisely the dialect `FilterConditionField` writes, so +the pair moves behind the existing `OPT_IN_OPERATORS` gate and that widget opts +in. Collapsing them onto `isNotNull` / `isNull` was rejected: the builder +already draws those as their own rows, the round trip is lossy (a saved +`exists` reads back as `isNotNull`), and the spec's own note records `$exists` = +has-value as still unsettled across drivers — `driver-memory`'s live mingo path +and `driver-mongodb` read key-presence. + +**The class is now closed by an assertion, not by discipline.** objectui's three +existing operator-parity guards all sweep spec vocabulary → objectui; none asked +whether an id the dropdown draws is an id the consumer can persist, which is the +direction that broke. `plugin-list`'s new +`list-offered-operator-expressible-parity.test.ts` forces the set the list +toolbar offers to **equal** the set its two dialects can express, in both +directions — so an unexpressible operator cannot be offered, and an operator +that becomes expressible upstream cannot stay needlessly withheld. diff --git a/packages/app-shell/src/views/viewFilterFold.ts b/packages/app-shell/src/views/viewFilterFold.ts index 3f7eb1a878..28341c9ce3 100644 --- a/packages/app-shell/src/views/viewFilterFold.ts +++ b/packages/app-shell/src/views/viewFilterFold.ts @@ -56,11 +56,20 @@ interface FilterGroupLike { * * The builder renders no value input for these (`needsValueInput` in * `@object-ui/components`'s `filter-builder.tsx`), so "no value" is the row's - * finished state, not an unfinished one. `exists` / `notExists` have no - * canonical spec spelling and pass through verbatim for the server's enum to - * reject loudly (see the operator note above) — they are listed so that - * rejection stays the reason they fail, rather than being silently dropped here - * as incomplete. + * finished state, not an unfinished one. + * + * `exists` / `notExists` are still listed, and no longer for the reason they + * used to be. They have no canonical spec spelling — `VIEW_FILTER_OPERATORS` + * has no existence operator — so a rule carrying one is refused by + * `ViewFilterRuleSchema`. That used to be their whole story here: they reached + * this fold from the shared dropdown, and listing them kept the server's loud + * rejection as the reason they failed rather than a silent drop as incomplete. + * Since objectui#4736 the dropdown no longer OFFERS them to any consumer that + * folds through here (they moved behind `OPT_IN_OPERATORS`, and only + * `FilterConditionField` — which writes MongoDB-style criteria, never a view + * rule — opts in), so the live path stopped producing them. They stay because + * `needsValueInput` still draws them value-less for that widget, and because + * stored metadata may carry one; the parity test below is what forces that. * * `viewFilterFold.emptyValue.test.ts` pins this set against the builder's own * list so the two cannot drift apart unnoticed. diff --git a/packages/components/src/__tests__/filter-builder-opt-in-operators.test.ts b/packages/components/src/__tests__/filter-builder-opt-in-operators.test.ts index 7f11333f52..aae0cf6a18 100644 --- a/packages/components/src/__tests__/filter-builder-opt-in-operators.test.ts +++ b/packages/components/src/__tests__/filter-builder-opt-in-operators.test.ts @@ -7,21 +7,34 @@ */ /** - * The opt-in half of the FilterBuilder's operator vocabulary (objectui#4023). + * The opt-in half of the FilterBuilder's operator vocabulary (objectui#4023, + * objectui#4736). * - * `containsCaseInsensitive` authors the spec's `$icontains`, which the MongoDB- - * style `FieldOperatorsSchema` dialect carries and the OTHER two dialects this - * one dropdown feeds do not: `VIEW_FILTER_OPERATORS` (what a saved view stores) - * and `VALID_AST_OPERATORS` (what the live grid sends) have no case-insensitive - * contains at all. Offering it unconditionally would hand users a filter two of - * three consumers cannot execute — the same hazard that held objectui#4023 - * blocked while no driver implemented the operator, moved from the drivers to - * this repo's own bridges. + * This one dropdown feeds three at-rest dialects and they do not accept the + * same operators. Two families are carried only by the MongoDB-style + * `FieldOperatorsSchema` criteria: + * + * - `containsCaseInsensitive` authors `$icontains`. `VIEW_FILTER_OPERATORS` + * (what a saved view stores) and `VALID_AST_OPERATORS` (what the live grid + * sends) have no case-insensitive contains at all. + * - `exists` / `notExists` author `$exists`. Neither of those two vocabularies + * has an existence operator either, under any spelling. + * + * Offering such an operator unconditionally hands users a filter two of three + * consumers cannot execute — the same hazard that held objectui#4023 blocked + * while no driver implemented the operator, moved from the drivers to this + * repo's own bridges. objectui#4736 is that hazard realised: the existence pair + * shipped to every consumer in objectui#2942 and the list toolbar sent it to + * the wire verbatim. * * So the default answer is "not offered", and these tests pin BOTH directions: * withheld unless asked for, and actually reachable once asked for. A gate that * only checked the second would go green on a component that offers everything * to everybody. + * + * What these tests do NOT decide is which ids belong in `OPT_IN_OPERATORS` — + * this package cannot see the dialects. That equality is forced per consumer: + * `plugin-list`'s `list-offered-operator-expressible-parity.test.ts`. */ import { describe, it, expect } from 'vitest'; import { FILTER_BUILDER_OPERATORS, operatorsForFieldType } from '../custom/filter-builder'; @@ -66,6 +79,44 @@ describe('FilterBuilder opt-in operators', () => { // opt-in operator is drawable, so leaving it out would understate the // vocabulary and let a future spec operator look unreachable when it is not. expect(FILTER_BUILDER_OPERATORS).toContain('containsCaseInsensitive'); + expect(FILTER_BUILDER_OPERATORS).toContain('exists'); + expect(FILTER_BUILDER_OPERATORS).toContain('notExists'); expect(new Set(FILTER_BUILDER_OPERATORS).size).toBe(FILTER_BUILDER_OPERATORS.length); }); + + // objectui#4736. The pair is drawable but no longer unconditional, so the + // list toolbar and the Studio view/tab inspectors — neither of which passes + // `extraOperators` — stop drawing an operator their dialects cannot store. + it('withholds exists / notExists from a consumer that did not ask', () => { + for (const type of [undefined, 'text', 'number', 'date', 'select', 'lookup']) { + const ids = idsFor(type); + expect(ids, String(type)).not.toContain('exists'); + expect(ids, String(type)).not.toContain('notExists'); + } + }); + + it('offers exists / notExists once the consumer opts in', () => { + const extra = ['exists', 'notExists']; + // Reachable on every bucket that had them before the withdrawal — the + // criteria builder filters lookups and dates by presence too, so scoping + // this to text would silently narrow what objectui#2942 made authorable. + for (const type of [undefined, 'text', 'number', 'date', 'select', 'lookup']) { + const ids = idsFor(type, extra); + expect(ids, String(type)).toContain('exists'); + expect(ids, String(type)).toContain('notExists'); + // Beside the null predicates, never instead of them: `$exists` and + // `$null` are distinct spec operators and both keep their own rows. + expect(ids, String(type)).toContain('isNull'); + expect(ids, String(type)).toContain('isNotNull'); + } + }); + + it('grants each opt-in id independently', () => { + // A consumer that can store `$exists` but not `$icontains` (or the reverse) + // must not get the other for free — that would make `extraOperators` a + // single "unlock everything" switch and put the whole opt-in gate back. + expect(idsFor('text', ['exists'])).not.toContain('containsCaseInsensitive'); + expect(idsFor('text', ['containsCaseInsensitive'])).not.toContain('exists'); + expect(idsFor('text', ['exists'])).not.toContain('notExists'); + }); }); diff --git a/packages/components/src/custom/filter-builder.tsx b/packages/components/src/custom/filter-builder.tsx index e626329feb..c1207e6c70 100644 --- a/packages/components/src/custom/filter-builder.tsx +++ b/packages/components/src/custom/filter-builder.tsx @@ -118,40 +118,81 @@ const defaultOperators = [ * `extraOperators`. Every other id in `defaultOperators` is offered to every * consumer. * - * ## Why an operator would ever be opt-in (objectui#4023) + * ## Why an operator would ever be opt-in (objectui#4023, objectui#4736) * * This one dropdown feeds three different at-rest dialects, and they do not * accept the same operators: * * - **MongoDB-style `FieldOperatorsSchema` criteria** — what * `FilterConditionField` writes into a sharing rule's `criteria_json`, - * evaluated by the server's engine. It has `$icontains`, and every driver - * and evaluation face the platform ships executes it - * (objectstack#5702 + objectstack#6520). - * - **`ViewFilterRule[]`** — what `viewFilterFold` persists for a saved view. - * Its vocabulary is the spec's `VIEW_FILTER_OPERATORS`, which has **no** - * case-insensitive contains, so the rule is refused by the schema's enum. + * evaluated by the server's engine. Its vocabulary is the spec's + * `FILTER_OPERATORS`, the widest of the three. + * - **`ViewFilterRule[]`** — what `viewFilterFold` persists for a saved view, + * and what the Studio inspector's `FilterBuilderField` writes. Its + * vocabulary is the spec's `VIEW_FILTER_OPERATORS`; a rule outside that + * enum is refused by `ViewFilterRuleSchema`. * - **The array/triplet filter AST** — what `ListView` sends for the live * grid, via `mapOperator`. Its vocabulary is the spec's - * `VALID_AST_OPERATORS`, which also has no case-insensitive contains: an - * unmapped operator is emitted verbatim and the query comes back broken - * (see `packages/data-objectstack/src/filter-operator-ast-parity.test.ts` - * for what "broken" costs — an unfiltered read or a 400, either way not the + * `VALID_AST_OPERATORS`; an unmapped operator is emitted verbatim and the + * query comes back broken (see + * `packages/data-objectstack/src/filter-operator-ast-parity.test.ts` for + * what "broken" costs — an unfiltered read or a 400, either way not the * filter the user asked for). * - * So offering `containsCaseInsensitive` to every FilterBuilder would put a - * filter in users' hands that two of the three consumers cannot execute — - * which is the exact hazard objectui#4023 was held blocked over, relocated from - * the drivers to this repo's own bridges. Mapping it onto plain `contains` - * there is NOT the alternative: that is a different question silently answered - * (the same reason `view-operator-builder-parity.test.ts` refuses to map - * `is_null` onto `isEmpty`). + * So an operator only one dialect can carry is offered only by the consumer + * that speaks that dialect. Mapping it onto a near-equivalent in the others is + * NOT the alternative: that is a different question silently answered — the + * same reason `view-operator-builder-parity.test.ts` refuses to map `is_null` + * onto `isEmpty`. * - * The lasting fix is upstream — the spec's view and AST vocabularies gaining a - * case-insensitive contains — at which point the entry below is deleted and the - * operator becomes ordinary. Until then the consumer that CAN store it says so. + * Which side of the line an id falls on is not left to this comment. + * `plugin-list`'s `list-offered-operator-expressible-parity.test.ts` forces the + * set the list toolbar offers to EQUAL the set its two dialects can express, in + * both directions: an unexpressible id that stays offered fails, and an id that + * becomes expressible while still withheld fails too. + * + * ### `containsCaseInsensitive` — the spec's `$icontains` (objectui#4023) + * + * Carried by the Mongo criteria dialect, where every driver and evaluation face + * the platform ships executes it (objectstack#5702 + objectstack#6520). The + * other two vocabularies have no case-insensitive contains at all, so offering + * it everywhere is the exact hazard objectui#4023 was held blocked over, + * relocated from the drivers to this repo's own bridges. + * + * ### `exists` / `notExists` — the spec's `$exists` (objectui#4736) + * + * Added by objectui#2942 to make `$exists` reachable from + * `FilterConditionField`, but offered to every consumer, including the two that + * cannot store them. Measured on `@objectstack/spec` 17.0.0-rc.6: neither + * `VIEW_FILTER_OPERATORS` nor `VALID_AST_OPERATORS` contains an existence + * operator — not under this spelling, not under any other; both sets have ZERO + * members matching `/exist/`. So there was nothing to map onto and the id went + * out verbatim. + * + * Collapsing them onto `isNotNull` / `isNull` would be a semantic claim, not a + * bridge, and it is refused on three counts: + * + * 1. The builder already offers `isNull` / `isNotNull` as their own rows, so + * the collapse would draw two labels for one wire predicate. + * 2. The round trip is lossy: a saved `exists` reads back through + * `specToBuilderOperator('is_not_null')` as `isNotNull`, silently + * rewriting the author's choice on reopen. + * 3. The equivalence is not this repo's to declare. `@objectstack/spec`'s own + * `data/index.d.ts` records `$exists` = has-value (`!= null`) as settled + * and shipped on `formula` and `driver-memory`'s reference matcher, while + * `driver-memory`'s live mingo path and `driver-mongodb` still read + * KEY-PRESENCE, both frozen by objectstack#5499 — which is why upstream + * cannot enrol a `$exists` conformance row yet either. + * + * The lasting fix for both families is upstream — the view and AST vocabularies + * gaining the operator — at which point the entry below is deleted, the parity + * test flips it back on by itself, and the operator becomes ordinary. */ -const OPT_IN_OPERATORS = new Set(["containsCaseInsensitive"]) +const OPT_IN_OPERATORS = new Set([ + "containsCaseInsensitive", + "exists", + "notExists", +]) /** * The FilterBuilder's own operator vocabulary — every id its dropdown can diff --git a/packages/fields/src/widgets/FilterConditionField.tsx b/packages/fields/src/widgets/FilterConditionField.tsx index 33275aa8e1..b3c47592f3 100644 --- a/packages/fields/src/widgets/FilterConditionField.tsx +++ b/packages/fields/src/widgets/FilterConditionField.tsx @@ -48,22 +48,36 @@ interface BuilderGroup { const EMPTY_GROUP: BuilderGroup = { id: 'root', logic: 'and', conditions: [] }; /** - * Opt-in FilterBuilder operators this widget offers (objectui#4023). + * Opt-in FilterBuilder operators this widget offers (objectui#4023, + * objectui#4736). * * The shared dropdown withholds these because two of its three consumers * persist into dialects that cannot carry them (see `OPT_IN_OPERATORS` in * `@object-ui/components`'s `filter-builder.tsx`). THIS widget can: its value * is a MongoDB-style `FieldOperatorsSchema` criteria that the server's engine - * evaluates directly — never lowered through the array/triplet AST — and - * `$icontains` is executable on every driver and evaluation face the platform - * ships (objectstack#5702 + objectstack#6520). + * evaluates directly — never lowered through the array/triplet AST and never + * folded into a `ViewFilterRule` — so the spec's `FILTER_OPERATORS` is the only + * vocabulary it has to satisfy. + * + * - `containsCaseInsensitive` authors `$icontains`, executable on every + * driver and evaluation face the platform ships (objectstack#5702 + + * objectstack#6520). + * - `exists` / `notExists` author `$exists`, which `condToMongo` has emitted + * and `kvToCondition` has read back since objectui#2942. Naming them here + * is what KEEPS them reachable now that the shared dropdown no longer + * offers them to the list and view surfaces, whose dialects have no + * existence operator at all (objectui#4736). * * Module scope, not an inline literal: a fresh array each render would reset * `FilterBuilder`'s memo inputs on every keystroke. * * @internal exported for tests */ -export const FILTER_CONDITION_EXTRA_OPERATORS: readonly string[] = ['containsCaseInsensitive']; +export const FILTER_CONDITION_EXTRA_OPERATORS: readonly string[] = [ + 'containsCaseInsensitive', + 'exists', + 'notExists', +]; /** Field types that are not meaningfully filterable in a simple builder. */ const NON_FILTERABLE = new Set([ diff --git a/packages/fields/src/widgets/__tests__/FilterConditionField.operators.test.ts b/packages/fields/src/widgets/__tests__/FilterConditionField.operators.test.ts index dbaffc5dd5..d14b0a3054 100644 --- a/packages/fields/src/widgets/__tests__/FilterConditionField.operators.test.ts +++ b/packages/fields/src/widgets/__tests__/FilterConditionField.operators.test.ts @@ -166,6 +166,37 @@ describe('every spec field operator is reachable from the builder (#2942)', () = expect(offered).toContain('containsCaseInsensitive'); expect(offered).toContain('contains'); }); + + /** + * The same half for `$exists` (objectui#4736). The pair became opt-in when the + * list and view surfaces — whose dialects have no existence operator — were + * found offering it, so THIS widget naming it is now the only thing keeping + * `$exists` authorable at all. + * + * The sweep above cannot notice if that opt-in is dropped: it feeds + * `FILTER_BUILDER_OPERATORS` (every DRAWABLE id) straight to `condToMongo`, + * which answers to any string it is handed, so `$exists` would still count as + * emitted while no admin could click a row that emits it — exactly the state + * objectui#4023 found for `$icontains`. + */ + it('the $exists operator is one this widget still offers, post-withdrawal', () => { + expect(SPEC_OPERATORS.has('$exists'), '$exists is no longer a spec operator').toBe(true); + expect(FILTER_BUILDER_OPERATORS).toContain('exists'); + expect(FILTER_BUILDER_OPERATORS).toContain('notExists'); + expect(FILTER_CONDITION_EXTRA_OPERATORS).toContain('exists'); + expect(FILTER_CONDITION_EXTRA_OPERATORS).toContain('notExists'); + + for (const type of ['text', 'number', 'date', 'select', 'lookup']) { + const offered = operatorsForFieldType(type, FILTER_CONDITION_EXTRA_OPERATORS) + .map((o) => o.value); + expect(offered, `${type} lost the existence pair`).toContain('exists'); + expect(offered, `${type} lost the existence pair`).toContain('notExists'); + // Distinct rows from the null predicates, which author `$null`. The two + // are different spec operators and `condToMongo` keeps them apart. + expect(offered, type).toContain('isNull'); + expect(offered, type).toContain('isNotNull'); + } + }); }); describe('kvToCondition round-trips what condToMongo writes', () => { diff --git a/packages/plugin-list/src/ListView.tsx b/packages/plugin-list/src/ListView.tsx index b57def16ff..9bd54914b9 100644 --- a/packages/plugin-list/src/ListView.tsx +++ b/packages/plugin-list/src/ListView.tsx @@ -157,6 +157,29 @@ export function mapOperator(op: string) { } } +/** + * Opt-in `FilterBuilder` operators the list toolbar offers — deliberately NONE + * (objectui#4736). + * + * `OPT_IN_OPERATORS` in `@object-ui/components` withholds the operator ids that + * only the MongoDB-style criteria dialect can carry. This toolbar persists into + * the other two, and into BOTH of them at once: + * + * - the array/triplet filter AST, via `mapOperator` above, for the live grid; + * - `ViewFilterRule[]`, via app-shell's `foldFilterGroupToSpecRules`, when the + * user saves the panel's group as a view. + * + * Neither vocabulary has a case-insensitive contains or an existence operator, + * so this surface opts into nothing. Named and passed explicitly rather than + * omitted at the call site: this constant is the handle + * `list-offered-operator-expressible-parity.test.ts` reads to compute what the + * toolbar actually offers, and an inline `extraOperators` added later would + * otherwise widen the dropdown without the parity test noticing. + * + * @internal exported for that test + */ +export const LIST_VIEW_EXTRA_OPERATORS: readonly string[] = []; + /** Every not-in spelling this normalizer expands. See the note at the call site. */ const NOT_IN_SPELLINGS = new Set(['nin', 'not_in', 'notIn', 'notin', 'not in']); @@ -2529,6 +2552,7 @@ export const ListView = React.forwardRef(({ { setCurrentFilters(newFilters); if (onFilterChange) onFilterChange(newFilters); 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 new file mode 100644 index 0000000000..d0643958d6 --- /dev/null +++ b/packages/plugin-list/src/__tests__/list-offered-operator-expressible-parity.test.ts @@ -0,0 +1,278 @@ +/** + * 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. + */ + +/** + * Offered ⇄ expressible parity for the list toolbar's filter builder (#4736). + * + * ## The direction nothing guarded + * + * objectui has three operator-parity guards and all three sweep the same way — + * spec vocabulary → objectui, asking whether every operator an author may + * DECLARE is one this repo can render or bridge: + * + * - `filter-operator-ast-parity.test.ts` (this package and `data-objectstack`) + * iterates `VIEW_FILTER_OPERATORS`; + * - `view-operator-builder-parity.test.ts` iterates `__CANONICAL_TO_BUILDER`, + * keyed by that same vocabulary; + * - `FilterConditionField.operators.test.ts` asks whether every spec + * `$`-token is reachable from the dropdown. + * + * None of them asks the reverse: is every id the DROPDOWN can draw an id this + * consumer can persist? That is the direction that broke. `exists` / `notExists` + * were added to the shared `FilterBuilder` by objectui#2942 so `$exists` would + * be reachable from `FilterConditionField`, and they were offered to every + * consumer — including this one, whose two dialects have no existence operator + * at all. `mapOperator`'s `default:` arm returned the id verbatim and the query + * went out as `['name', 'exists', 'x']`, which `isFilterAST()` rejects. + * + * ## What this file asserts, and why it is an equality + * + * The toolbar persists into TWO dialects, and a filter has to survive both: + * + * 1. **the live grid** — `convertFilterGroupToAST` → the array/triplet filter + * AST, gated by `isFilterAST()` over `VALID_AST_OPERATORS`. A rejected + * filter costs an unfiltered read or a 400; see the header of + * `filter-operator-ast-parity.test.ts` for that measurement. + * 2. **save as view** — app-shell's `foldFilterGroupToSpecRules`, whose only + * operator transform is the spec's own `normalizeFilterOperator`, landing + * on `ViewFilterRuleSchema`'s enum. That fold cannot be imported here + * (app-shell depends on this package, not the other way round), so the leg + * below mirrors it through the same two spec exports and finishes on the + * SCHEMA rather than on set membership — the schema is what actually + * refuses the rule. + * + * The spine is `offers exactly the operators both dialects can express`: an + * EQUALITY, not a subset check. A subset check is satisfied by a toolbar that + * offers nothing, and it would let the opposite drift through in silence — an + * operator that becomes expressible upstream and stays needlessly withheld. + * Forcing equality is what makes the `OPT_IN_OPERATORS` entry in + * `@object-ui/components` self-retiring: the day the view and AST vocabularies + * gain an existence operator, this file goes red and says to offer it again. + */ +import { describe, it, expect } from 'vitest'; +import { VALID_AST_OPERATORS, isFilterAST } from '@objectstack/spec/data'; +import { + VIEW_FILTER_OPERATORS, + ViewFilterRuleSchema, + normalizeFilterOperator, +} from '@objectstack/spec/ui'; +import { FILTER_BUILDER_OPERATORS, operatorsForFieldType } from '@object-ui/components'; +import { + LIST_VIEW_EXTRA_OPERATORS, + convertFilterGroupToAST, +} from '../ListView'; + +/** + * One field type per operator bucket in `filter-builder.tsx`, plus `undefined` + * (the default arm) and an unknown type (which falls into the same text + * bucket). The bucket arrays are module-private there, so this list is + * hand-written — and the totality ratchet below is what keeps it honest: if a + * bucket is added or a type moves, the union stops covering + * `FILTER_BUILDER_OPERATORS` and this file goes red rather than silently + * testing a shrinking slice of the dropdown. + */ +const PROBE_FIELD_TYPES: ReadonlyArray = [ + undefined, + 'text', + 'a_type_this_builder_has_never_heard_of', + 'number', + 'currency', + 'percent', + 'rating', + 'boolean', + 'date', + 'datetime', + 'time', + 'select', + 'status', + 'lookup', + 'master_detail', + 'user', + 'owner', +]; + +/** Operator ids offered across every bucket, given the opt-ins granted. */ +function offeredAcrossBuckets(extra: readonly string[]): string[] { + const ids = new Set(); + for (const type of PROBE_FIELD_TYPES) { + for (const op of operatorsForFieldType(type, extra)) ids.add(op.value); + } + return [...ids].sort(); +} + +/** + * What the LIST toolbar offers — read through the constant `ListView` actually + * passes to `FilterBuilder`, never through a literal `[]`. A future + * `extraOperators` added at that call site has to come through here. + */ +const OFFERED_BY_LIST = offeredAcrossBuckets(LIST_VIEW_EXTRA_OPERATORS); + +/** + * Every id the dropdown can draw for some field type with every opt-in granted + * — the pool the toolbar's offering is a selection FROM. Granting the full + * vocabulary as `extraOperators` is how an opt-in id is included; ids that are + * not opt-in are unaffected by it. + */ +const DRAWABLE = offeredAcrossBuckets(FILTER_BUILDER_OPERATORS); + +/** A value that keeps a row from being dropped as incomplete, per operator. */ +function probeValue(operator: string): unknown { + if (operator === 'in' || operator === 'notIn') return ['a', 'b']; + if (operator === 'between') return [1, 5]; + return 'x'; +} + +/** + * 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. + * + * 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 `[]`, + * which is "no filter" and would sail through `isFilterAST()` while the user's + * chosen operator never reached the wire at all. + */ +function liveGridResult(operator: string): { ok: boolean; emitted: unknown } { + const emitted = convertFilterGroupToAST({ + id: 'root', + logic: 'and', + conditions: [{ id: 'c1', field: 'some_field', operator, value: probeValue(operator) }], + } as never); + const nonEmpty = Array.isArray(emitted) && emitted.length > 0; + return { ok: nonEmpty && isFilterAST(emitted), emitted }; +} + +/** + * Leg 2 — save as view. Mirrors `foldFilterGroupToSpecRules`: normalize through + * the spec's own map, then hand the rule to the schema the server validates + * with. `value` is included exactly when the operator takes one, matching the + * fold's `VALUELESS_FILTER_OPERATORS` treatment. + */ +const VALUELESS = new Set(['isEmpty', 'isNotEmpty', 'isNull', 'isNotNull', 'exists', 'notExists']); + +function savedViewResult(operator: string): { ok: boolean; canonical: string } { + const canonical = String(normalizeFilterOperator(operator)); + const rule: Record = { field: 'some_field', operator: canonical }; + if (!VALUELESS.has(operator)) rule.value = probeValue(operator); + return { ok: ViewFilterRuleSchema.safeParse(rule).success, canonical }; +} + +const isExpressible = (operator: string) => + liveGridResult(operator).ok && savedViewResult(operator).ok; + +describe('the list toolbar offers only operators its dialects can express', () => { + it('reads a non-empty vocabulary from each side', () => { + // Guards every assertion below against passing on an empty list. + expect(VALID_AST_OPERATORS.size).toBeGreaterThan(0); + expect(VIEW_FILTER_OPERATORS.length).toBeGreaterThan(0); + expect(FILTER_BUILDER_OPERATORS.length).toBeGreaterThan(0); + expect(OFFERED_BY_LIST.length).toBeGreaterThan(0); + }); + + // The totality ratchet for PROBE_FIELD_TYPES. The bucket arrays are private + // to `filter-builder.tsx`, so a new bucket — or an operator that only appears + // in one — would otherwise leave this file sweeping a subset of the dropdown + // and reporting parity over the part it happens to see. + it('probes every operator the builder can draw', () => { + const unprobed = [...FILTER_BUILDER_OPERATORS].filter((id) => !DRAWABLE.includes(id)); + expect( + unprobed, + `PROBE_FIELD_TYPES reaches no bucket containing: ${unprobed.join(', ')}. ` + + 'A field-type bucket was added or moved in filter-builder.tsx — add a ' + + 'representative type here, or this file stops covering those operators', + ).toEqual([]); + }); + + it.each(OFFERED_BY_LIST)('%s survives to the live grid', (operator) => { + const { ok, emitted } = liveGridResult(operator); + expect( + ok, + `the toolbar offers '${operator}', and convertFilterGroupToAST emitted ` + + `${JSON.stringify(emitted)} — which isFilterAST() rejects (or dropped the row ` + + 'entirely). The operator reaches the wire verbatim and the query comes back ' + + 'broken: an unfiltered read or a 400, never the filter the user asked for. ' + + 'Either give mapOperator a row that is a real member of VALID_AST_OPERATORS, ' + + 'or withdraw the operator from this consumer via OPT_IN_OPERATORS', + ).toBe(true); + }); + + it.each(OFFERED_BY_LIST)('%s is storable as a saved-view rule', (operator) => { + const { ok, canonical } = savedViewResult(operator); + expect( + ok, + `the toolbar offers '${operator}'; foldFilterGroupToSpecRules normalizes it to ` + + `'${canonical}' and ViewFilterRuleSchema refuses that operator. Saving the ` + + 'panel as a view fails on the server enum, so the filter on screen cannot be ' + + 'persisted at all', + ).toBe(true); + }); + + // The spine. An equality, deliberately — see the file header. + it('offers exactly the operators both dialects can express, no more and no less', () => { + const expressible = DRAWABLE.filter(isExpressible); + + const unexpressibleButOffered = OFFERED_BY_LIST.filter((id) => !expressible.includes(id)); + expect( + unexpressibleButOffered, + `the toolbar offers operators neither of its dialects can carry: ` + + `${unexpressibleButOffered.join(', ')}. Add them to OPT_IN_OPERATORS in ` + + '@object-ui/components so only the consumer that can store them offers them', + ).toEqual([]); + + const expressibleButWithheld = expressible.filter((id) => !OFFERED_BY_LIST.includes(id)); + expect( + expressibleButWithheld, + `these operators are expressible on BOTH of this toolbar's dialects and are still ` + + `withheld from it: ${expressibleButWithheld.join(', ')}. Whatever made them ` + + 'opt-in no longer holds — drop the OPT_IN_OPERATORS entry (or name them in ' + + 'LIST_VIEW_EXTRA_OPERATORS) rather than leaving a withdrawal nothing justifies', + ).toEqual([]); + + expect(OFFERED_BY_LIST).toEqual(expressible); + }); + + // The named pin for the pair this guard was written for (#4736). It states + // the measurement the OPT_IN_OPERATORS comment rests on — that BOTH dialects + // refuse them, not just one — so that upstream gaining an existence operator + // on either side lands as a red test with something to do, rather than as a + // comment that has quietly become false. + it('withholds exists / notExists from this consumer, and says why', () => { + for (const id of ['exists', 'notExists']) { + expect(FILTER_BUILDER_OPERATORS, `${id} is no longer an id the builder can draw`) + .toContain(id); + expect( + OFFERED_BY_LIST, + `${id} is back in the list toolbar's dropdown; its save path produces a broken ` + + 'query on the live grid and a refused rule on save-as-view', + ).not.toContain(id); + expect( + liveGridResult(id).ok, + `VALID_AST_OPERATORS now expresses '${id}'. Half the reason it is withheld is ` + + 'gone — recheck the other dialect and the OPT_IN_OPERATORS note', + ).toBe(false); + expect( + savedViewResult(id).ok, + `VIEW_FILTER_OPERATORS now expresses '${id}'. Half the reason it is withheld is ` + + 'gone — recheck the other dialect and the OPT_IN_OPERATORS note', + ).toBe(false); + } + }); + + // The withdrawal is scoped to the existence pair: `isNull` / `isNotNull` are + // real members of both vocabularies and must keep their rows. Collapsing the + // one family onto the other is what this fix deliberately did NOT do — the + // same refusal `view-operator-builder-parity.test.ts` records for + // `is_null` -> `isEmpty`. + it('keeps the null predicates, which both dialects do express', () => { + for (const id of ['isNull', 'isNotNull', 'isEmpty', 'isNotEmpty']) { + expect(OFFERED_BY_LIST, `${id} must still be offered`).toContain(id); + expect(isExpressible(id), `${id} must still be expressible on both dialects`).toBe(true); + } + }); +});