Skip to content

Commit 4545380

Browse files
os-zhuangclaude
andauthored
fix(view): the spec→FilterBuilder map follows the four operators #2942 added (#3022)
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/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 was false. is_null/is_not_null map to isNull/isNotNull, NOT isEmpty/isNotEmpty: the builder draws both pairs now, and folding the NULL predicate onto the empty-string one would rewrite the author's operator on the next save. The old guard could not catch this — it compared the unmapped set against a hand-kept gap list, and neither side moves when the BUILDER gains an operator. The new assertion is derived: 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. Confirmed by reverting the four mappings, which reproduces the drift as four named failures. The unmapped set is now empty — all 19 canonical members translate. Refs #2945, #2942, #2989 Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent b41f401 commit 4545380

3 files changed

Lines changed: 91 additions & 21 deletions

File tree

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
---
2+
"@object-ui/plugin-view": patch
3+
---
4+
5+
fix(view): the spec→FilterBuilder map follows the four operators #2942 added
6+
7+
`CANONICAL_TO_BUILDER` mapped `starts_with`, `ends_with`, `is_null` and
8+
`is_not_null` to `null`, with a comment asserting the FilterBuilder had no such
9+
operator. #2942 gave it `startsWith`, `endsWith`, `isNull` and `isNotNull`
10+
and this table did not follow, so a stored view carrying any of the four still
11+
reached the builder as a raw spelling it could by then have rendered, and the
12+
comment claiming otherwise was simply false.
13+
14+
All four now map. `is_null`/`is_not_null` go to `isNull`/`isNotNull` and **not**
15+
to `isEmpty`/`isNotEmpty`: the builder draws both pairs, and folding the NULL
16+
predicate onto the empty-string one would silently rewrite the author's operator
17+
the next time the view was saved.
18+
19+
**The guard could not have caught this, and now can.** The parity test asserted
20+
the unmapped set equalled a hand-kept list of gaps — which stays true when the
21+
*builder* gains an operator, because neither side of that comparison moves. The
22+
new assertion is derived instead: `starts_with` and `startsWith` fold to the same
23+
key, so an unmapped canonical operator whose folded name matches a folded builder
24+
id is an omission by definition. Verified by reverting the four mappings, which
25+
reproduces the drift as four named failures.
26+
27+
The unmapped set is now empty — all 19 canonical `VIEW_FILTER_OPERATORS` members
28+
translate.
29+
30+
Refs #2945, #2942, #2989

packages/plugin-view/src/config/__tests__/view-operator-builder-parity.test.ts

Lines changed: 45 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -31,15 +31,18 @@ import { specToBuilderOperator, __CANONICAL_TO_BUILDER } from '../view-config-ut
3131
/**
3232
* Canonical view operators the FilterBuilder cannot express.
3333
*
34-
* `starts_with`/`ends_with`: no such operator in the builder's vocabulary.
35-
* `is_null`/`is_not_null`: distinct from `is_empty`/`is_not_empty`, which the
36-
* builder does have — folding them together would rewrite the author's operator
37-
* on the next save.
34+
* Empty — every one of the 19 now maps. It was `starts_with`, `ends_with`,
35+
* `is_null`, `is_not_null` until #2942 gave the builder `startsWith`/`endsWith`/
36+
* `isNull`/`isNotNull`.
3837
*
39-
* Shrink this list by adding the operator to the FilterBuilder, never by
40-
* mapping onto a near-equivalent.
38+
* Shrink it by adding the operator to the FilterBuilder, never by mapping onto a
39+
* near-equivalent: `is_null` → `isEmpty` would rewrite a NULL predicate into an
40+
* empty-string one on the next save.
4141
*/
42-
const NO_BUILDER_EQUIVALENT = ['starts_with', 'ends_with', 'is_null', 'is_not_null'];
42+
const NO_BUILDER_EQUIVALENT: string[] = [];
43+
44+
/** Case- and separator-insensitive key, matching the module's own `fold`. */
45+
const fold = (op: string) => op.toLowerCase().replace(/[\s_-]+/g, '');
4346

4447
describe('CANONICAL_TO_BUILDER', () => {
4548
it('covers every canonical view operator, and nothing else', () => {
@@ -54,12 +57,35 @@ describe('CANONICAL_TO_BUILDER', () => {
5457
expect(notRenderable).toEqual([]);
5558
});
5659

60+
/**
61+
* The guard that catches drift in the direction the hand-kept gap list could
62+
* not: the builder GAINING an operator this table still calls unmappable.
63+
* `starts_with` and `startsWith` fold to the same key, so an unmapped operator
64+
* whose folded name matches a folded builder id is an omission by definition —
65+
* which is exactly how #2942's four new operators went unnoticed here.
66+
*/
67+
it('leaves nothing unmapped that the builder can already draw', () => {
68+
const byFolded = new Map(FILTER_BUILDER_OPERATORS.map(id => [fold(id), id]));
69+
const missed = Object.entries(__CANONICAL_TO_BUILDER)
70+
.filter(([, id]) => id === null)
71+
.filter(([op]) => byFolded.has(fold(op)))
72+
.map(([op]) => `${op} -> ${byFolded.get(fold(op))} exists but is unmapped`);
73+
expect(missed).toEqual([]);
74+
});
75+
5776
it('records exactly the documented gaps', () => {
5877
const unmapped = Object.entries(__CANONICAL_TO_BUILDER)
5978
.filter(([, id]) => id === null)
6079
.map(([op]) => op);
6180
expect(unmapped.sort()).toEqual([...NO_BUILDER_EQUIVALENT].sort());
6281
});
82+
83+
it('keeps the NULL and empty-string predicates distinct', () => {
84+
expect(__CANONICAL_TO_BUILDER.is_null).toBe('isNull');
85+
expect(__CANONICAL_TO_BUILDER.is_not_null).toBe('isNotNull');
86+
expect(__CANONICAL_TO_BUILDER.is_empty).toBe('isEmpty');
87+
expect(__CANONICAL_TO_BUILDER.is_not_empty).toBe('isNotEmpty');
88+
});
6389
});
6490

6591
describe('specToBuilderOperator', () => {
@@ -102,12 +128,21 @@ describe('specToBuilderOperator', () => {
102128
}
103129
});
104130

105-
it('returns an unmapped operator unchanged rather than coercing it', () => {
131+
it('returns an unrecognised operator unchanged rather than coercing it', () => {
106132
// Visible in the UI as an incomplete condition row beats a silent rewrite
107133
// to `equals`, which would drop the author's predicate on the next save.
108-
expect(specToBuilderOperator('is_null')).toBe('is_null');
109-
expect(specToBuilderOperator('starts_with')).toBe('starts_with');
110134
expect(specToBuilderOperator('totally_made_up')).toBe('totally_made_up');
135+
expect(specToBuilderOperator('$regex')).toBe('$regex');
136+
});
137+
138+
it('resolves the four operators #2942 made expressible', () => {
139+
expect(specToBuilderOperator('starts_with')).toBe('startsWith');
140+
expect(specToBuilderOperator('ends_with')).toBe('endsWith');
141+
expect(specToBuilderOperator('is_null')).toBe('isNull');
142+
expect(specToBuilderOperator('is_not_null')).toBe('isNotNull');
143+
// …without collapsing them onto the empty-string pair.
144+
expect(specToBuilderOperator('is_empty')).toBe('isEmpty');
145+
expect(specToBuilderOperator('is_not_empty')).toBe('isNotEmpty');
111146
});
112147

113148
it('defaults an absent operator to equals', () => {

packages/plugin-view/src/config/view-config-utils.ts

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,21 +28,26 @@ import type { ViewFilterOperator } from '@objectstack/spec/ui';
2828
* operator added to the spec's view vocabulary fails to compile here instead
2929
* of reaching the builder as a raw spelling its dropdown cannot select.
3030
*
31-
* The four `null`s are honest gaps, not oversights. `starts_with`/`ends_with`
32-
* have no FilterBuilder operator at all, and `is_null`/`is_not_null` carry a
33-
* NULL/empty distinction that `isEmpty`/`isNotEmpty` erase — folding them onto
34-
* the near-equivalent would silently rewrite the author's operator the next
35-
* time the view was saved. An unmapped operator instead reaches the builder
36-
* unchanged and shows up as a condition row the author must complete, which is
37-
* the failure we want: visible, and lossless until they touch it.
31+
* **Every canonical operator now maps.** The four that did not —
32+
* `starts_with`/`ends_with`/`is_null`/`is_not_null` — were unmapped because the
33+
* FilterBuilder had no such operator; #2942 added `startsWith`/`endsWith`/
34+
* `isNull`/`isNotNull` to it, and this table did not follow, so a stored view
35+
* carrying them still reached the builder as a raw spelling it could by then
36+
* have rendered. The parity guard catches that class now: a canonical operator
37+
* whose name folds onto a builder id it is not mapped to fails the test.
38+
*
39+
* `is_null`/`is_not_null` map to `isNull`/`isNotNull` and NOT to
40+
* `isEmpty`/`isNotEmpty` — the builder now draws both pairs, and folding the
41+
* NULL predicate onto the empty-string one would silently rewrite the author's
42+
* operator the next time the view was saved.
3843
*/
3944
const CANONICAL_TO_BUILDER: Record<ViewFilterOperator, string | null> = {
4045
'equals': 'equals',
4146
'not_equals': 'notEquals',
4247
'contains': 'contains',
4348
'not_contains': 'notContains',
44-
'starts_with': null,
45-
'ends_with': null,
49+
'starts_with': 'startsWith',
50+
'ends_with': 'endsWith',
4651
'greater_than': 'greaterThan',
4752
'less_than': 'lessThan',
4853
'greater_than_or_equal': 'greaterOrEqual',
@@ -51,8 +56,8 @@ const CANONICAL_TO_BUILDER: Record<ViewFilterOperator, string | null> = {
5156
'not_in': 'notIn',
5257
'is_empty': 'isEmpty',
5358
'is_not_empty': 'isNotEmpty',
54-
'is_null': null,
55-
'is_not_null': null,
59+
'is_null': 'isNull',
60+
'is_not_null': 'isNotNull',
5661
'before': 'before',
5762
'after': 'after',
5863
'between': 'between',

0 commit comments

Comments
 (0)