diff --git a/.changeset/6939-filter-builder-catalog-operator-spellings.md b/.changeset/6939-filter-builder-catalog-operator-spellings.md new file mode 100644 index 0000000000..529b6f9554 --- /dev/null +++ b/.changeset/6939-filter-builder-catalog-operator-spellings.md @@ -0,0 +1,17 @@ +--- +--- + +Corpus + test-only (objectui#6939): the three `components-complex-filter-builder` +schema-catalog entries now spell `conditions[].operator` with the members +`FilterOperatorSchema` declares (`equals` / `less_than` / `greater_than`) instead +of the spec's alias table (`eq` / `lt` / `gt`), so they pass +`safeValidateSchema`. Contract-first: the protocol declares the snake_case +vocabulary and the corpus was the side that was wrong — no enum member was +added, and no published type, export or runtime path moved. The two pins in +`@object-ui/types` and `@object-ui/components` that recorded the old corpus +state are re-aimed (and, where they were fences for objectui#7561's narrower +scope, inverted rather than deleted) so they still name the key that moved. +Rendering is unchanged, which is this change's acceptance rather than a +side-effect: objectui#7561 had already routed the operator trigger through +`normalizeFilterOperator`, so all three dialects drew the same label before the +rewrite and must still draw it after. diff --git a/examples/schema-catalog/src/schemas/components-complex-filter-builder/product-search.json b/examples/schema-catalog/src/schemas/components-complex-filter-builder/product-search.json index b85b074169..06499875fd 100644 --- a/examples/schema-catalog/src/schemas/components-complex-filter-builder/product-search.json +++ b/examples/schema-catalog/src/schemas/components-complex-filter-builder/product-search.json @@ -36,19 +36,19 @@ { "id": "1", "field": "category", - "operator": "eq", + "operator": "equals", "value": "Electronics" }, { "id": "2", "field": "price", - "operator": "lt", + "operator": "less_than", "value": "1000" }, { "id": "3", "field": "stock", - "operator": "gt", + "operator": "greater_than", "value": "0" } ] diff --git a/examples/schema-catalog/src/schemas/components-complex-filter-builder/search-interface.json b/examples/schema-catalog/src/schemas/components-complex-filter-builder/search-interface.json index e8cec23e50..bcf74b51be 100644 --- a/examples/schema-catalog/src/schemas/components-complex-filter-builder/search-interface.json +++ b/examples/schema-catalog/src/schemas/components-complex-filter-builder/search-interface.json @@ -55,13 +55,13 @@ { "id": "1", "field": "published", - "operator": "eq", + "operator": "equals", "value": "true" }, { "id": "2", "field": "views", - "operator": "gt", + "operator": "greater_than", "value": "1000" } ] diff --git a/examples/schema-catalog/src/schemas/components-complex-filter-builder/with-conditions.json b/examples/schema-catalog/src/schemas/components-complex-filter-builder/with-conditions.json index cecafc8e6e..d2020d6427 100644 --- a/examples/schema-catalog/src/schemas/components-complex-filter-builder/with-conditions.json +++ b/examples/schema-catalog/src/schemas/components-complex-filter-builder/with-conditions.json @@ -36,13 +36,13 @@ { "id": "1", "field": "age", - "operator": "gt", + "operator": "greater_than", "value": "25" }, { "id": "2", "field": "department", - "operator": "eq", + "operator": "equals", "value": "Engineering" } ] diff --git a/examples/schema-catalog/test/filter-builder-mirror-6939.test.tsx b/examples/schema-catalog/test/filter-builder-mirror-6939.test.tsx index 2e5f41419e..11548a5963 100644 --- a/examples/schema-catalog/test/filter-builder-mirror-6939.test.tsx +++ b/examples/schema-catalog/test/filter-builder-mirror-6939.test.tsx @@ -268,10 +268,11 @@ describe('objectui#6939 — the fixtures were the side that was right', () => { // ⚠️ Read on `fieldTriggers`, not on every combobox on the tile. Until // objectui#7561 the two were interchangeable here ONLY because the operator // trigger was blank as well — and it was blank for an unrelated defect, - // these entries authoring `eq` / `lt` / `gt`. Now that the operator cell - // renders its label, an all-triggers reading would be answering about the - // operator repair rather than about the lost field, which is the one thing - // this probe exists to discriminate. + // these entries authoring the spec's alias table (`eq` / `lt` / `gt`; + // rewritten to the canonical members by objectui#6939's remainder). Now + // that the operator cell renders its label, an all-triggers reading would + // be answering about the operator repair rather than about the lost field, + // which is the one thing this probe exists to discriminate. expect(authored.fieldTriggers.some((t) => t !== '')).toBe(true); expect(corrected.fieldTriggers.every((t) => t === '')).toBe(true); // Anti-vacuity for the line above: `every` over an EMPTY list is true, so diff --git a/examples/schema-catalog/test/filter-builder-operator-alias-trigger-7561.test.tsx b/examples/schema-catalog/test/filter-builder-operator-alias-trigger-7561.test.tsx index cf71772609..ce2972f127 100644 --- a/examples/schema-catalog/test/filter-builder-operator-alias-trigger-7561.test.tsx +++ b/examples/schema-catalog/test/filter-builder-operator-alias-trigger-7561.test.tsx @@ -14,20 +14,30 @@ * as authored `… Category Remove condition Price Remove condition …` * operators corrected `… Category Equals Remove condition Price Less than …` * - * Three catalog entries author the spec's alias table (`eq` / `lt` / `gt`) — - * `product-search`, `with-conditions`, and the `filter-builder` nested inside - * `search-interface`. No `SelectItem` in the operator dropdown carries those - * ids, and the trigger used to match its value LITERALLY against the mounted - * items, so all three drew a blank operator cell over rows that filtered - * correctly. + * Three catalog entries carry the rows this measures — `product-search`, + * `with-conditions`, and the `filter-builder` nested inside `search-interface`. + * No `SelectItem` in the operator dropdown carries a spelling from outside its + * own camelCase vocabulary, and the trigger used to match its value LITERALLY + * against the mounted items, so any other spelling of the same operator drew a + * blank operator cell over a row that filtered correctly. + * + * ⚠️ **What these entries author has since changed, and the pin was re-aimed + * rather than deleted.** When objectui#7561 landed they authored the spec's + * ALIAS table (`eq` / `lt` / `gt`), and this file's last assertion was a fence + * saying so — objectui#7561's own scope forbade touching catalog data, and the + * fence existed to stop a lenient "repair" being smuggled in as one. That fence + * was lifted by objectui#6939's remainder, which rewrote those seven spellings + * to the DECLARED vocabulary so the entries validate. ⇒ the assertion is + * INVERTED below, not dropped: it still names the key that moved, and it still + * refuses a migration back to a dialect the mirror does not accept. * * ## Why the swap is the control and not a repair * - * ⛔ No catalog data is changed to make this pass. The swap happens in memory, - * as a CONTROL: after the repair the two columns must be the SAME text, because - * `normalizeFilterOperator` folds `eq` and `equals` onto one operator and the - * trigger now resolves through it. Before the repair they differ — which is - * what makes this measurement able to fail. + * ⛔ No catalog data is changed BY THIS FILE. The swap happens in memory, as a + * CONTROL: after objectui#7561's repair the two columns must be the SAME text, + * because `normalizeFilterOperator` folds every spelling of one operator onto + * one canonical member and the trigger now resolves through it. Before that + * repair they differ — which is what makes this measurement able to fail. * * ⭐ The corrected column deliberately uses the dropdown's OWN camelCase ids * (`lessThan`), not the spec's canonical `less_than`: those ids are the ones @@ -36,13 +46,23 @@ * rendering the same text is the claim; neither arm is a recommendation about * which vocabulary an author SHOULD use — that is objectui#7561's separate * ruling and is not decided here. + * + * ⚠️ The two vocabularies OVERLAP on three members (`equals`, `contains`, + * `in` are spelled identically in both), so "the arms are two dialects" cannot + * be stated as "no member of one appears in the other". The anti-vacuity leg + * below therefore asserts that at least one row's spelling really moved, and + * names the overlap instead of pretending it away. */ import { describe, it, expect } from 'vitest'; import { render } from '@testing-library/react'; import '@object-ui/components'; import { SchemaRenderer, toRenderableSchema } from '@object-ui/react'; +import { FilterOperatorSchema } from '@object-ui/types/zod'; import { getExample } from '../src/index.js'; +/** Taken FROM the mirror, never restated beside it. */ +const DECLARED_OPERATORS: readonly string[] = FilterOperatorSchema.options; + /** The three entries whose rows carry an alias-table operator. */ const AFFECTED = [ 'components-complex-filter-builder/product-search', @@ -62,8 +82,22 @@ const EXPECTED: Record<(typeof AFFECTED)[number], string[]> = { 'components-complex-filter-builder/with-conditions': ['Greater than', 'Equals'], }; -/** The alias spellings these entries author → the dropdown id each folds onto. */ +/** + * The declared spellings these entries author → the dropdown id each folds + * onto. Covers the spec's alias table too, so an entry re-authored in EITHER + * off-dropdown dialect is still carried by the control arm rather than + * silently passed through as an identity. + */ const CORRECTION: Record = { + // what the catalog authors today — `@objectstack/spec`'s canonical members + equals: 'equals', + not_equals: 'notEquals', + greater_than: 'greaterThan', + greater_than_or_equal: 'greaterOrEqual', + less_than: 'lessThan', + less_than_or_equal: 'lessOrEqual', + not_in: 'notIn', + // the alias table these entries authored before objectui#6939's remainder eq: 'equals', ne: 'notEquals', lt: 'lessThan', @@ -73,6 +107,17 @@ const CORRECTION: Record = { nin: 'notIn', }; +/** + * The members whose two spellings COINCIDE — a row on one of these cannot show + * the control arm moving, and saying so is what keeps the anti-vacuity leg + * honest rather than accidentally satisfied. + */ +const SPELT_ALIKE = new Set( + Object.entries(CORRECTION) + .filter(([authored, mounted]) => authored === mounted) + .map(([authored]) => authored), +); + function asAuthored(id: (typeof AFFECTED)[number]): Record { return getExample(id).schema as Record; } @@ -159,22 +204,40 @@ describe('objectui#7561 — the alias spellings the catalog authors render a lab const beforeOps = before.conditions.map((c) => c.operator); const afterOps = after.conditions.map((c) => c.operator); expect(afterOps).not.toEqual(beforeOps); - // Every authored operator really is an alias-table spelling… + // Every authored operator is a spelling the control table knows, so no + // row slipped through as an unmapped identity… expect(beforeOps.every((o) => o in CORRECTION)).toBe(true); - // …and none of the swapped-in ids is, so the arms are genuinely two - // different dialects of the same operators. - expect(afterOps.some((o) => o in CORRECTION)).toBe(false); + // …and every row that is NOT one of the coinciding members really moved, + // which is the arms being two dialects stated in the form the overlap + // permits. (⛔ `afterOps.some(o => o in CORRECTION) === false` is the + // form this once used and it is not available: `equals` is a member of + // both vocabularies.) + const moved = beforeOps.filter((o) => !SPELT_ALIKE.has(o)); + expect(moved.length).toBeGreaterThan(0); + for (const [i, op] of beforeOps.entries()) { + if (SPELT_ALIKE.has(op)) expect(afterOps[i]).toBe(op); + else expect(afterOps[i]).not.toBe(op); + } } }); - it('⛔ the catalog files themselves still author the alias table', () => { - // The fix must NOT be achieved by editing catalog data (objectui#7561's - // fence). If someone ever "repairs" the fixtures instead, this reddens. + it('⛔ the catalog files author the DECLARED vocabulary, not a dialect the mirror refuses', () => { + // INVERTED by objectui#6939's remainder — see this file's header. The + // entries now spell their operators the way `FilterOperatorSchema` declares + // them, which is what makes them pass `safeValidateSchema`; the two-column + // equality above is what says the rewrite cost no pixel. + // + // ⛔ If someone migrates them back to `eq` / `lt` / `gt`, or forward to the + // dropdown's own `greaterThan`, this reddens: both are spellings the mirror + // refuses, and the render is no longer the thing at stake. for (const id of AFFECTED) { const group = builderNode(asAuthored(id)).value as { conditions: { operator: string }[]; }; - for (const c of group.conditions) expect(c.operator in CORRECTION).toBe(true); + expect(group.conditions.length).toBeGreaterThan(0); + for (const c of group.conditions) { + expect(DECLARED_OPERATORS).toContain(c.operator); + } } }); }); diff --git a/examples/schema-catalog/test/filter-builder-operator-vocabulary-6939.test.tsx b/examples/schema-catalog/test/filter-builder-operator-vocabulary-6939.test.tsx new file mode 100644 index 0000000000..a701fb38f1 --- /dev/null +++ b/examples/schema-catalog/test/filter-builder-operator-vocabulary-6939.test.tsx @@ -0,0 +1,287 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * objectui#6939's remainder — the `filter-builder` row's fourth divergence, + * measured in BOTH directions because this repair pulls two ways at once. + * + * ## What changed in the corpus + * + * Three catalog entries authored `conditions[].operator` in the spec's ALIAS + * table (`eq` / `lt` / `gt`) while `FilterOperatorSchema` declares the spec's + * canonical members (`equals` / `less_than` / `greater_than`). Every one of + * those documents therefore failed `safeValidateSchema`. The seven spellings + * were rewritten to the declared vocabulary — contract-first, the protocol + * being the side that is right — and ⛔ the enum was not widened by one member + * to meet them. + * + * ## The two directions, and why neither alone is the acceptance + * + * - **NEW — it validates.** The property this card ADDS. Pinned below per + * entry, and paired with a refusal control so `.success` cannot go green + * by a mirror that accepts everything. + * - **PRESERVED — it renders the same.** The property this card must NOT + * break. ⚠️ objectui#6318's rule is that *a "correction" that renders + * identically proves the edit was wrong*; that rule was written where the + * correction repaired a BROKEN render. Here it inverts: objectui#7561 (PR + * #9305) had already routed the operator trigger's identity comparison + * through `normalizeFilterOperator`, so all three dialects already drew + * their label — the alias spellings were validation-broken, not + * render-broken. ⇒ a render that MOVES is this card's failure signal, and + * that is what the identity legs below exist to catch. + * + * The "before" arm is not read from git history: it is reconstructed in memory + * from the same tree by swapping only the operator spellings back, so the proof + * keeps working in a checkout that never saw the old bytes. + * + * ## ⛔ The open half, recorded rather than repaired + * + * The dropdown EMITS a third vocabulary — its own camelCase ids + * (`notEquals`, `greaterThan`) — which this mirror also refuses. A filter a + * user edits in the UI and stores is therefore refused exactly as these + * fixtures once were. That is the larger half of the same defect and no card + * owns it; it is pinned here as a refusal so it cannot go quiet, and ⛔ it is + * not ruled here. + */ +import { describe, it, expect } from 'vitest'; +import { render } from '@testing-library/react'; +import '@object-ui/components'; +import { SchemaRenderer, toRenderableSchema } from '@object-ui/react'; +import { safeValidateSchema, FilterOperatorSchema } from '@object-ui/types/zod'; +import { getExample } from '../src/index.js'; + +/** The three entries whose rows carry an operator at all. */ +const AFFECTED = [ + 'components-complex-filter-builder/product-search', + 'components-complex-filter-builder/search-interface', + 'components-complex-filter-builder/with-conditions', +] as const; + +type AffectedId = (typeof AFFECTED)[number]; + +/** + * The spelling each entry authored BEFORE this repair, keyed by the declared + * member it was rewritten to. Read in this direction the table is a control, + * not a recipe: it reconstructs the refused document so both halves of the + * claim can be measured against it. + */ +const FORMER_ALIAS: Record = { + equals: 'eq', + greater_than: 'gt', + less_than: 'lt', +}; + +/** + * The dropdown's own camelCase id for the same three operators. ⚠️ `equals` is + * spelled identically in both vocabularies — the overlap is real and the legs + * below name it rather than assume it away. + */ +const DROPDOWN_ID: Record = { + equals: 'equals', + greater_than: 'greaterThan', + less_than: 'lessThan', +}; + +/** Taken FROM the mirror rather than restated beside it. */ +const DECLARED_OPERATORS: readonly string[] = FilterOperatorSchema.options; + +function asAuthored(id: AffectedId): Record { + return getExample(id).schema as Record; +} + +/** The `filter-builder` node, whether it is the root or nested in a `stack`. */ +function builderNode(doc: Record): Record { + if (doc.type === 'filter-builder') return doc; + const child = (doc.children as Record[]).find( + (c) => c.type === 'filter-builder', + ); + if (!child) throw new Error('entry no longer carries a filter-builder'); + return child; +} + +function operatorsOf(doc: Record): string[] { + const group = builderNode(doc).value as { conditions: { operator: string }[] }; + return group.conditions.map((c) => c.operator); +} + +/** The same entry with ONLY the operator spellings rewritten through `table`. */ +function withOperators(id: AffectedId, table: Record): Record { + const doc = asAuthored(id); + const swap = (b: Record) => { + const group = b.value as { conditions: Record[] }; + return { + ...b, + value: { + ...group, + conditions: group.conditions.map((c) => ({ + ...c, + operator: table[c.operator as string] ?? c.operator, + })), + }, + }; + }; + if (doc.type === 'filter-builder') return swap(doc); + return { + ...doc, + children: (doc.children as Record[]).map((c) => + c.type === 'filter-builder' ? swap(c) : c, + ), + }; +} + +/** Report the issues rather than `false`, so a red run says what broke. */ +function reasons(schema: unknown): string[] { + const r = safeValidateSchema(schema); + return r.success ? [] : r.error.issues.map((i) => `${i.path.join('.')}: ${i.message}`); +} + +function measure(schema: unknown) { + const { container, unmount } = render( + , + ); + const out = { + text: container.textContent ?? '', + elements: container.querySelectorAll('*').length, + inputs: Array.from(container.querySelectorAll('input')).map((e) => e.getAttribute('type')), + // The row's cells are `div.col-span-4`, field / operator / value in order, + // so `nth-child(2)` is the operator cell. + operatorTriggers: Array.from( + container.querySelectorAll('div.col-span-4:nth-child(2) [role="combobox"]'), + ).map((e) => e.textContent), + }; + unmount(); + return out; +} + +describe('objectui#6939 — NEW: the catalog entries now pass `safeValidateSchema`', () => { + it.each(AFFECTED)('%s validates as authored', (id) => { + expect(reasons(asAuthored(id))).toEqual([]); + }); + + it('the `stack`-rooted entry validates at its NESTED node too', () => { + // `search-interface.json` roots at `stack`; `objectui check` validates the + // parsed document rather than each child, so the nested `filter-builder` is + // measured here rather than left to the root's verdict. + expect(reasons(builderNode(asAuthored('components-complex-filter-builder/search-interface')))).toEqual([]); + }); + + it.each(AFFECTED)('%s: every authored operator is a DECLARED member', (id) => { + const ops = operatorsOf(asAuthored(id)); + expect(ops.length).toBeGreaterThan(0); + for (const op of ops) expect(DECLARED_OPERATORS).toContain(op); + }); + + it.each(AFFECTED)( + '%s: ⛔ the alias dialect it used to author is still REFUSED', + (id) => { + // The control that carries the contract-first claim. A "repair" that + // widened `FilterOperatorSchema` to accept `eq` / `lt` / `gt` would turn + // THIS green, and it must not. + const aliased = withOperators(id, FORMER_ALIAS); + expect(reasons(aliased)).not.toEqual([]); + // Anti-vacuity: the reconstruction really did move every row, so the + // refusal is about the alias and not about an untouched document. + const before = operatorsOf(asAuthored(id)); + const after = operatorsOf(aliased); + expect(after.length).toBe(before.length); + for (const [i, op] of before.entries()) expect(after[i]).not.toBe(op); + }, + ); + + it.each(AFFECTED)( + '%s: ⛔ the dropdown\'s own camelCase dialect is still REFUSED — the open half', + (id) => { + const dropdown = withOperators(id, DROPDOWN_ID); + // `equals` coincides across the two vocabularies, so only the rows that + // actually moved carry this leg; assert at least one did. + const before = operatorsOf(asAuthored(id)); + const after = operatorsOf(dropdown); + expect(after).not.toEqual(before); + expect(reasons(dropdown)).not.toEqual([]); + }, + ); + + it('`.success` is not vacuous — an invented operator is refused', () => { + // ⚠️ The payload is deliberately spelled for THIS test — it names the card + // and the component, so nobody reaches for it as a repo-wide "is this + // token absent" control. A generic-looking absent token consumed as a test + // literal stops being absent the moment this file lands, and the next + // reader who trusts it gets a non-zero from a token they believed clean. + const INVENTED = 'filter_builder_6939_invented_operator'; + const bogus = withOperators('components-complex-filter-builder/product-search', { + equals: INVENTED, + greater_than: INVENTED, + less_than: INVENTED, + }); + expect(DECLARED_OPERATORS).not.toContain(INVENTED); + expect(operatorsOf(bogus)).not.toEqual(operatorsOf(asAuthored('components-complex-filter-builder/product-search'))); + expect(reasons(bogus)).not.toEqual([]); + }); +}); + +describe('objectui#6939 — PRESERVED: the rewrite cost no pixel', () => { + it.each(AFFECTED)('%s: as authored renders exactly as the alias arm did', (id) => { + const aliased = withOperators(id, FORMER_ALIAS); + // ⛔ Anti-vacuity on the ARM, before anything is rendered — the same guard + // the NEW-direction twin carries on its own use of this table (the leg + // named `⛔ the alias dialect it used to author is still REFUSED`). + // `withOperators` falls through on a key it does not hold + // (`table[c.operator] ?? c.operator`) and `FORMER_ALIAS` is keyed on the + // DECLARED spellings, so against a corpus re-authored in the alias dialect + // this arm is an identity transform: the same document rendered twice and + // asserted equal to itself. Measured — without these two lines the whole + // leg stays GREEN under a revert of the three catalog files to `eq` / `gt` + // / `lt`, which is this card's own failure class reproduced inside the pin + // that carries its headline claim. + const beforeOps = operatorsOf(asAuthored(id)); + const afterOps = operatorsOf(aliased); + expect(afterOps.length).toBe(beforeOps.length); + for (const [i, op] of beforeOps.entries()) expect(afterOps[i]).not.toBe(op); + const authored = measure(asAuthored(id)); + const asItWas = measure(aliased); + // Anti-vacuity first: a tile that failed to mount would make every equality + // below trivially true. + expect(authored.text).not.toContain('failed to render'); + expect(authored.elements).toBeGreaterThan(10); + expect(authored.operatorTriggers.length).toBeGreaterThan(0); + // The three readings that would move if the repair had cost anything. + expect(authored.text).toBe(asItWas.text); + expect(authored.elements).toBe(asItWas.elements); + expect(authored.inputs).toEqual(asItWas.inputs); + expect(authored.operatorTriggers).toEqual(asItWas.operatorTriggers); + }); + + it.each(AFFECTED)('%s: and no operator cell is blank', (id) => { + // The symptom objectui#7561 removed, restated as the floor this card must + // not fall back through. ⛔ Not the same claim as the identity above: two + // blank columns are also identical. + const m = measure(asAuthored(id)); + expect(m.operatorTriggers.length).toBeGreaterThan(0); + expect(m.operatorTriggers.filter((t) => t === '')).toEqual([]); + }); + + it('the identity legs can fail — a DIFFERENT operator moves the render', () => { + // Without this, the equalities above would be satisfied by a `measure` that + // reported the same thing for every tree it was given. + // + // ⭐ The swap table is DERIVED from whatever the entry currently spells, + // never keyed on the spellings this card happens to have landed. Measured: + // a hard-coded `{ equals: …, greater_than: … }` table makes this leg an + // identity transform — and so, silently, a passing assertion about nothing + // — the moment the corpus is re-authored in another dialect. That is the + // failure this whole card is about, reproduced inside its own pin. + const id = 'components-complex-filter-builder/product-search' as const; + const table = Object.fromEntries( + operatorsOf(asAuthored(id)).map((op) => [op, op === 'is_null' ? 'contains' : 'is_null']), + ); + const authored = measure(asAuthored(id)); + const different = measure(withOperators(id, table)); + // The swap really reached the tree it was given. + expect(operatorsOf(withOperators(id, table))).not.toEqual(operatorsOf(asAuthored(id))); + expect(different.operatorTriggers).not.toEqual(authored.operatorTriggers); + expect(different.text).not.toBe(authored.text); + }); +}); diff --git a/packages/components/src/__tests__/filter-builder-operator-alias-trigger-7561.test.tsx b/packages/components/src/__tests__/filter-builder-operator-alias-trigger-7561.test.tsx index 53307059c9..6ec2223611 100644 --- a/packages/components/src/__tests__/filter-builder-operator-alias-trigger-7561.test.tsx +++ b/packages/components/src/__tests__/filter-builder-operator-alias-trigger-7561.test.tsx @@ -88,9 +88,12 @@ function operatorTriggerText() { * - `canonical` — `@objectstack/spec`'s snake_case set, what the mirror * (`FilterOperatorSchema`) accepts and what `foldFilterGroupToSpecRules` * persists; - * - `alias` — the spec's alias table (`eq` / `lt` / `gt` / `ne` / `nin`), - * the THIRD spelling, authored today by `product-search.json`, - * `with-conditions.json` and the builder nested in `search-interface.json`; + * - `alias` — the spec's alias table (`eq` / `lt` / `gt` / `ne` / `nin`), the + * THIRD spelling. ⚠️ It was authored by three schema-catalog entries when + * this pin was written; objectui#6939's remainder rewrote those to the + * canonical members, so the dialect now reaches this builder from STORED + * filters rather than from the corpus — which is the reason the row stays + * here rather than following the corpus; * - `overlap` — the three ids both vocabularies share. NOT controls. */ const SPELLINGS: ReadonlyArray<{ diff --git a/packages/types/src/__tests__/filter-builder-mirror-6939.test.ts b/packages/types/src/__tests__/filter-builder-mirror-6939.test.ts index 4ab1debfd0..bcaa3c0e34 100644 --- a/packages/types/src/__tests__/filter-builder-mirror-6939.test.ts +++ b/packages/types/src/__tests__/filter-builder-mirror-6939.test.ts @@ -102,20 +102,32 @@ * reviewable change. ⛔ Never narrow the doc to match the mirror — under * decision batch #88 a contract does not retract what it published to authors. * - * ## What this change does NOT reach, stated rather than left as an absence + * ## The FOURTH divergence, and how it was settled * - * Two of the four census entries — `product-search` and `with-conditions`, plus - * the `filter-builder` nested inside `search-interface` — still refuse - * afterwards, on a FOURTH divergence the ruling does not address: they author - * `conditions[].operator` as `eq` / `gt` / `lt`, and `FilterOperatorSchema` is - * the spec's canonical `equals` / `greater_than` / `less_than`. - * `assertion the residual refusal is the operator alias and nothing else` - * pins that mechanically — swapping only those three spellings makes both - * entries parse — so the claim "the three ruled divergences are gone from all - * four" is measured rather than asserted. The operator vocabulary is a genuine - * fork (the builder's own dropdown ids are `notEquals` / `greaterThan`, which - * this mirror ALSO refuses, while the canonical spellings it accepts render a - * blank operator trigger) and needs its own ruling. + * This file once recorded a residual: two of the four census entries — plus the + * `filter-builder` nested inside `search-interface` — still refused afterwards, + * because they authored `conditions[].operator` as `eq` / `gt` / `lt` while + * `FilterOperatorSchema` declares the spec's canonical `equals` / + * `greater_than` / `less_than`. That residual was filed as objectui#7561 and is + * now CLOSED, in two landings that must not be confused with one another: + * + * 1. objectui#7561 (PR #9305) routed the operator trigger's identity + * comparison through `normalizeFilterOperator`, so all three spellings of + * one operator DRAW the same label. It deliberately changed no catalog + * data and widened no accepted set. + * 2. objectui#6939's own remainder then rewrote the seven authored spellings + * in those three files to the declared vocabulary — the contract-first + * direction, `@objectstack/spec` being the side that is right. ⛔ The enum + * was NOT widened to meet them; `the alias dialect is still REFUSED` below + * is the control that says so, and it is the assertion that would redden + * if a later change reached for the lenient repair instead. + * + * ⚠️ The fork itself is only half closed, and the open half is named here + * rather than left as an absence: the builder's own dropdown emits camelCase + * ids (`notEquals`, `greaterThan`), which this mirror still refuses, so a + * filter a user edits in the UI and stores is refused exactly as these fixtures + * once were. `the dropdown's own dialect is still REFUSED` pins that, so the + * gap is a recorded decision rather than a silence. */ import { describe, it, expect } from 'vitest'; import { readFileSync } from 'node:fs'; @@ -168,16 +180,33 @@ function reasons(schema: unknown): string[] { return r.success ? [] : r.error.issues.map((i) => `${i.path.join('.')}: ${i.message}`); } -/** The three alias spellings the fixtures use, mapped to what this mirror declares. */ -const CANONICAL: Record = { eq: 'equals', gt: 'greater_than', lt: 'less_than' }; - -function withCanonicalOperators(doc: Record): Record { +/** + * The three spellings these fixtures authored BEFORE objectui#6939's remainder, + * keyed by the declared member each was rewritten to. Read in this direction it + * is a control rather than a repair recipe: putting the old dialect back must + * still be REFUSED, which is what says the enum was not widened to meet it. + */ +const FORMER_ALIAS: Record = { equals: 'eq', greater_than: 'gt', less_than: 'lt' }; + +/** The dropdown's OWN camelCase id for the same three operators — the half of + * the fork that is still open, and still refused. ⛔ Not a recommendation. */ +const DROPDOWN_ID: Record = { + equals: 'equals', + greater_than: 'greaterThan', + less_than: 'lessThan', +}; + +/** Rewrite every `conditions[].operator` through `table`, changing nothing else. */ +function withOperators( + doc: Record, + table: Record, +): Record { const group = doc.value as { conditions: { operator: string }[] }; return { ...doc, value: { ...group, - conditions: group.conditions.map((c) => ({ ...c, operator: CANONICAL[c.operator] ?? c.operator })), + conditions: group.conditions.map((c) => ({ ...c, operator: table[c.operator] ?? c.operator })), }, }; } @@ -698,29 +727,58 @@ describe('objectui#6939 — the catalog entries the mirror refused', () => { ); it.each(['product-search', 'with-conditions'])( - '%s: the residual refusal is the operator alias and NOTHING else', + '%s now validates too — the operator residual is gone', (name) => { - // ⛔ Do NOT "repair" this by widening `FilterOperatorSchema` or by - // rewriting the fixtures. Both are outside the ruling and both need one: - // the builder's dropdown ids (`greaterThan`) and the spec's canonical - // spellings (`greater_than`) are a genuine fork, and this mirror refuses - // the former while the RENDERER draws a blank operator trigger for the - // latter. Reported on objectui#6939. - expect(reasons(entry(name))).not.toEqual([]); - expect(reasons(withCanonicalOperators(entry(name)))).toEqual([]); + // objectui#6939's remainder: the authored spelling is the DECLARED one. + expect(reasons(entry(name))).toEqual([]); }, ); - it('the `stack`-rooted fifth entry: its nested filter-builder behaves the same way', () => { + it('the `stack`-rooted fifth entry: its nested filter-builder validates as well', () => { // `search-interface.json` roots at `stack`, so `objectui check` (which runs - // `safeValidateSchema` on the ROOT only — `packages/cli/src/commands/check.ts:137`) - // counts four entries for this row, not five. The nested node is measured - // here so the fifth file is not silently unexamined. - const node = nestedSearchInterface(); - expect(reasons(node)).not.toEqual([]); - expect(reasons(withCanonicalOperators(node))).toEqual([]); + // `safeValidateSchema` on the ROOT only — the `check` command validates the + // parsed document, not its children) counts four entries for this row, not + // five. The nested node is measured here so the fifth file is not silently + // unexamined. + expect(reasons(nestedSearchInterface())).toEqual([]); }); + it.each(['product-search', 'with-conditions'])( + '%s: ⛔ the alias dialect is still REFUSED — the enum was not widened', + (name) => { + // The control that carries the contract-first claim. If a later change + // repairs a red here by adding `eq` / `gt` / `lt` to + // `FilterOperatorSchema`, THIS is what goes green and must not. + const aliased = withOperators(entry(name), FORMER_ALIAS); + expect(reasons(aliased)).not.toEqual([]); + // Anti-vacuity: the rewrite really did change the tree it was given, so + // the refusal above is about the alias and not about an unchanged doc + // that was refused for some other reason. + const ops = (d: Record) => + ((d.value as { conditions: { operator: string }[] }).conditions).map((c) => c.operator); + expect(ops(aliased)).not.toEqual(ops(entry(name))); + expect(ops(aliased).length).toBeGreaterThan(0); + }, + ); + + it.each(['product-search', 'with-conditions'])( + '%s: ⛔ the dropdown\'s own dialect is still REFUSED — the open half of the fork', + (name) => { + // `FILTER_BUILDER_OPERATORS` is the vocabulary the UI EMITS, so a filter a + // user edits and stores is refused exactly as these fixtures once were. + // Named here, ⛔ not ruled here: widening to meet it is the lenient repair + // this whole card argues against, and narrowing the dropdown is a UI + // ruling nobody has made. Recorded so the gap cannot go quiet. + const dropdown = withOperators(entry(name), DROPDOWN_ID); + const ops = (d: Record) => + ((d.value as { conditions: { operator: string }[] }).conditions).map((c) => c.operator); + // `equals` is spelled the same in both vocabularies — the overlap is real + // and is why this arm needs an anti-vacuity leg of its own. + expect(ops(dropdown)).not.toEqual(ops(entry(name))); + expect(reasons(dropdown)).not.toEqual([]); + }, + ); + it.each(CENSUS)('%s: none of the THREE ruled divergences is left in it', (name) => { // The positive statement behind the split above, key by key. const doc = entry(name) as {