diff --git a/.changeset/canonical-to-builder-follows-2942.md b/.changeset/canonical-to-builder-follows-2942.md new file mode 100644 index 000000000..7dc0c034b --- /dev/null +++ b/.changeset/canonical-to-builder-follows-2942.md @@ -0,0 +1,30 @@ +--- +"@object-ui/plugin-view": patch +--- + +fix(view): the spec→FilterBuilder map follows the four operators #2942 added + +`CANONICAL_TO_BUILDER` mapped `starts_with`, `ends_with`, `is_null` and +`is_not_null` to `null`, with a comment asserting the FilterBuilder had no such +operator. #2942 gave it `startsWith`, `endsWith`, `isNull` and `isNotNull` — +and this table did not follow, so a stored view carrying any of the four still +reached the builder as a raw spelling it could by then have rendered, and the +comment claiming otherwise was simply false. + +All four now map. `is_null`/`is_not_null` go to `isNull`/`isNotNull` and **not** +to `isEmpty`/`isNotEmpty`: the builder draws both pairs, and folding the NULL +predicate onto the empty-string one would silently rewrite the author's operator +the next time the view was saved. + +**The guard could not have caught this, and now can.** The parity test asserted +the unmapped set equalled a hand-kept list of gaps — which stays true when the +*builder* gains an operator, because neither side of that comparison moves. The +new assertion is derived instead: `starts_with` and `startsWith` fold to the same +key, so an unmapped canonical operator whose folded name matches a folded builder +id is an omission by definition. Verified by reverting the four mappings, which +reproduces the drift as four named failures. + +The unmapped set is now empty — all 19 canonical `VIEW_FILTER_OPERATORS` members +translate. + +Refs #2945, #2942, #2989 diff --git a/packages/plugin-view/src/config/__tests__/view-operator-builder-parity.test.ts b/packages/plugin-view/src/config/__tests__/view-operator-builder-parity.test.ts index 813f472f5..1539499f5 100644 --- a/packages/plugin-view/src/config/__tests__/view-operator-builder-parity.test.ts +++ b/packages/plugin-view/src/config/__tests__/view-operator-builder-parity.test.ts @@ -31,15 +31,18 @@ import { specToBuilderOperator, __CANONICAL_TO_BUILDER } from '../view-config-ut /** * Canonical view operators the FilterBuilder cannot express. * - * `starts_with`/`ends_with`: no such operator in the builder's vocabulary. - * `is_null`/`is_not_null`: distinct from `is_empty`/`is_not_empty`, which the - * builder does have — folding them together would rewrite the author's operator - * on the next save. + * Empty — every one of the 19 now maps. It was `starts_with`, `ends_with`, + * `is_null`, `is_not_null` until #2942 gave the builder `startsWith`/`endsWith`/ + * `isNull`/`isNotNull`. * - * Shrink this list by adding the operator to the FilterBuilder, never by - * mapping onto a near-equivalent. + * Shrink it by adding the operator to the FilterBuilder, never by mapping onto a + * near-equivalent: `is_null` → `isEmpty` would rewrite a NULL predicate into an + * empty-string one on the next save. */ -const NO_BUILDER_EQUIVALENT = ['starts_with', 'ends_with', 'is_null', 'is_not_null']; +const NO_BUILDER_EQUIVALENT: string[] = []; + +/** Case- and separator-insensitive key, matching the module's own `fold`. */ +const fold = (op: string) => op.toLowerCase().replace(/[\s_-]+/g, ''); describe('CANONICAL_TO_BUILDER', () => { it('covers every canonical view operator, and nothing else', () => { @@ -54,12 +57,35 @@ describe('CANONICAL_TO_BUILDER', () => { expect(notRenderable).toEqual([]); }); + /** + * The guard that catches drift in the direction the hand-kept gap list could + * not: the builder GAINING an operator this table still calls unmappable. + * `starts_with` and `startsWith` fold to the same key, so an unmapped operator + * whose folded name matches a folded builder id is an omission by definition — + * which is exactly how #2942's four new operators went unnoticed here. + */ + it('leaves nothing unmapped that the builder can already draw', () => { + const byFolded = new Map(FILTER_BUILDER_OPERATORS.map(id => [fold(id), id])); + const missed = Object.entries(__CANONICAL_TO_BUILDER) + .filter(([, id]) => id === null) + .filter(([op]) => byFolded.has(fold(op))) + .map(([op]) => `${op} -> ${byFolded.get(fold(op))} exists but is unmapped`); + expect(missed).toEqual([]); + }); + it('records exactly the documented gaps', () => { const unmapped = Object.entries(__CANONICAL_TO_BUILDER) .filter(([, id]) => id === null) .map(([op]) => op); expect(unmapped.sort()).toEqual([...NO_BUILDER_EQUIVALENT].sort()); }); + + it('keeps the NULL and empty-string predicates distinct', () => { + expect(__CANONICAL_TO_BUILDER.is_null).toBe('isNull'); + expect(__CANONICAL_TO_BUILDER.is_not_null).toBe('isNotNull'); + expect(__CANONICAL_TO_BUILDER.is_empty).toBe('isEmpty'); + expect(__CANONICAL_TO_BUILDER.is_not_empty).toBe('isNotEmpty'); + }); }); describe('specToBuilderOperator', () => { @@ -102,12 +128,21 @@ describe('specToBuilderOperator', () => { } }); - it('returns an unmapped operator unchanged rather than coercing it', () => { + it('returns an unrecognised operator unchanged rather than coercing it', () => { // Visible in the UI as an incomplete condition row beats a silent rewrite // to `equals`, which would drop the author's predicate on the next save. - expect(specToBuilderOperator('is_null')).toBe('is_null'); - expect(specToBuilderOperator('starts_with')).toBe('starts_with'); expect(specToBuilderOperator('totally_made_up')).toBe('totally_made_up'); + expect(specToBuilderOperator('$regex')).toBe('$regex'); + }); + + it('resolves the four operators #2942 made expressible', () => { + expect(specToBuilderOperator('starts_with')).toBe('startsWith'); + expect(specToBuilderOperator('ends_with')).toBe('endsWith'); + expect(specToBuilderOperator('is_null')).toBe('isNull'); + expect(specToBuilderOperator('is_not_null')).toBe('isNotNull'); + // …without collapsing them onto the empty-string pair. + expect(specToBuilderOperator('is_empty')).toBe('isEmpty'); + expect(specToBuilderOperator('is_not_empty')).toBe('isNotEmpty'); }); it('defaults an absent operator to equals', () => { diff --git a/packages/plugin-view/src/config/view-config-utils.ts b/packages/plugin-view/src/config/view-config-utils.ts index 4c5edb0f8..70cc6e7cb 100644 --- a/packages/plugin-view/src/config/view-config-utils.ts +++ b/packages/plugin-view/src/config/view-config-utils.ts @@ -28,21 +28,26 @@ import type { ViewFilterOperator } from '@objectstack/spec/ui'; * operator added to the spec's view vocabulary fails to compile here instead * of reaching the builder as a raw spelling its dropdown cannot select. * - * The four `null`s are honest gaps, not oversights. `starts_with`/`ends_with` - * have no FilterBuilder operator at all, and `is_null`/`is_not_null` carry a - * NULL/empty distinction that `isEmpty`/`isNotEmpty` erase — folding them onto - * the near-equivalent would silently rewrite the author's operator the next - * time the view was saved. An unmapped operator instead reaches the builder - * unchanged and shows up as a condition row the author must complete, which is - * the failure we want: visible, and lossless until they touch it. + * **Every canonical operator now maps.** The four that did not — + * `starts_with`/`ends_with`/`is_null`/`is_not_null` — were unmapped because the + * FilterBuilder had no such operator; #2942 added `startsWith`/`endsWith`/ + * `isNull`/`isNotNull` to it, and this table did not follow, so a stored view + * carrying them still reached the builder as a raw spelling it could by then + * have rendered. The parity guard catches that class now: a canonical operator + * whose name folds onto a builder id it is not mapped to fails the test. + * + * `is_null`/`is_not_null` map to `isNull`/`isNotNull` and NOT to + * `isEmpty`/`isNotEmpty` — the builder now draws both pairs, and folding the + * NULL predicate onto the empty-string one would silently rewrite the author's + * operator the next time the view was saved. */ const CANONICAL_TO_BUILDER: Record = { 'equals': 'equals', 'not_equals': 'notEquals', 'contains': 'contains', 'not_contains': 'notContains', - 'starts_with': null, - 'ends_with': null, + 'starts_with': 'startsWith', + 'ends_with': 'endsWith', 'greater_than': 'greaterThan', 'less_than': 'lessThan', 'greater_than_or_equal': 'greaterOrEqual', @@ -51,8 +56,8 @@ const CANONICAL_TO_BUILDER: Record = { 'not_in': 'notIn', 'is_empty': 'isEmpty', 'is_not_empty': 'isNotEmpty', - 'is_null': null, - 'is_not_null': null, + 'is_null': 'isNull', + 'is_not_null': 'isNotNull', 'before': 'before', 'after': 'after', 'between': 'between',