From 5c82df0685bf46d8c00eb92aa43715b0b67686a4 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 13 Sep 2026 07:06:46 +0000 Subject: [PATCH 1/2] fix(app-shell): bridge `Is null` to the spec's `$null` instead of erasing the filter `groupToCondition` had no row for `isNull` / `isNotNull`, so an `Is null` row fell through to the unmapped-operator drop. A dropped last row makes the function return `undefined`, and the dataset inspector commits on every change, so switching the only condition's operator to an ordinary menu entry committed `{ filter: undefined }` and destroyed the stored `dataset.filter`. Nothing errored and the panel still showed the condition. Both directions now carry the spec's `$null` predicate, kept distinct from the `$exists` pair the two `isEmpty` rows already used. Operators this bridge does not map are still dropped rather than emitted in a spelling that means something else; that behaviour and the list of offered-but-unmappable operators are pinned alongside the fix. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ --- .../9363-dataset-filter-isnull-erases.md | 29 +++ ...FilterCondition.nullOperators-9363.test.ts | 235 ++++++++++++++++++ .../inspectors/datasetFilterCondition.ts | 51 +++- 3 files changed, 312 insertions(+), 3 deletions(-) create mode 100644 .changeset/9363-dataset-filter-isnull-erases.md create mode 100644 packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.nullOperators-9363.test.ts diff --git a/.changeset/9363-dataset-filter-isnull-erases.md b/.changeset/9363-dataset-filter-isnull-erases.md new file mode 100644 index 0000000000..ab21c8303b --- /dev/null +++ b/.changeset/9363-dataset-filter-isnull-erases.md @@ -0,0 +1,29 @@ +--- +'@object-ui/app-shell': patch +--- + +Stop the Studio dataset-filter bridge from ERASING a stored filter when the author +picks `Is null` (objectui#9363). + +`groupToCondition` had no mapping for `isNull` / `isNotNull`, so those rows fell +through to the unmapped-operator drop. A dropped last row makes the function return +`undefined`, and the dataset inspector commits on every change — so an author with a +working `dataset.filter` (or a `measure.filter`) who opened the filter popover and +switched the single condition's operator to **Is null** committed `undefined`, and the +persisted filter was destroyed. Nothing errored and the panel still showed the +condition. `Is null` is an ordinary entry in that menu, not an opt-in one. + +Both directions now bridge the spec's `$null` predicate: `isNull` serializes to +`{ field: { $null: true } }` and `isNotNull` to `{ $null: false }`, and a stored +`$null` reads back as the operator the author picked instead of degrading the whole +filter to "edit it in the Source tab". + +`$null` stays distinct from `$exists`: `isEmpty` / `isNotEmpty` are unchanged, because +the dropdown offers both pairs as their own rows and the spec's filter vocabulary +carries both predicates. + +Operators this bridge still does not map are still dropped rather than emitted in a +spelling that means something else — that behaviour is deliberate and is now pinned +alongside the fix, together with the list of operators the menu offers and this bridge +cannot store (`notContains`, `between`, `startsWith`, `endsWith`), so the next unmapped +addition fails a test instead of erasing a filter. diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.nullOperators-9363.test.ts b/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.nullOperators-9363.test.ts new file mode 100644 index 0000000000..382b4a4f5a --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.nullOperators-9363.test.ts @@ -0,0 +1,235 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `Is null` must not ERASE the stored dataset filter (objectui#9363). + * + * ## The mechanism, and why it is data loss rather than a wrong result set + * + * `groupToCondition` returns `undefined` when no row survives serialization, + * and the Studio dataset inspector commits on EVERY change — the filter popover + * mounts the shared `FilterBuilder` and hands each emitted group straight to + * `onCommit(groupToCondition(g))`, which lands as `onPatch({ filter })` and is + * spread over the draft. So an author with a working `dataset.filter` who opens + * the inspector and switches the single condition's operator to **Is null** + * commits `undefined`: the persisted key is destroyed, nothing errors, and the + * panel still shows the condition. + * + * The inspector passes no `extraOperators`, so `isNull` / `isNotNull` are + * ordinary entries in the default menu — not opt-in ones. The parity block at + * the bottom of this file reads that offering from the builder's own exported + * bucket function rather than restating it. + * + * ## The distinction this file exists to keep + * + * The `continue` these two operators fell into is a DELIBERATE decision for + * operators the bridge does not map: dropping beats emitting a filter that + * means something else. That behaviour is kept, and pinned below, because a + * repair that made the fallback stop dropping everything it cannot map would + * emit wrong filters — a worse defect than this one. + * + * `isNull` / `isNotNull` are a different case and that is the whole repair: the + * dialect CAN express them (the spec's own `$null`, asserted below against + * `FILTER_OPERATORS` rather than assumed), the builder draws them as COMPLETE + * rows with no value input, and this inspector offers them. The drop was an + * unhandled operator falling into the fallback's path, not the fallback doing + * its job. Both readings are pinned so they stay distinguishable. + * + * ## Red-first + * + * Before the repair, the two `$null` expectations fail with `undefined` while + * the `equals` control in the same run passes — a table of all-`undefined` + * answers and a dead function are otherwise indistinguishable. + */ +import { describe, it, expect } from 'vitest'; +import { FILTER_OPERATORS, FieldOperatorsSchema } from '@objectstack/spec/data'; +import { + FILTER_BUILDER_OPERATORS, + VALUELESS_FILTER_BUILDER_OPERATORS, + operatorsForFieldType, +} from '@object-ui/components'; +import { groupToCondition, conditionToGroup } from './datasetFilterCondition'; +import type { BuilderGroup } from './datasetFilterCondition'; + +/** One condition row, as the builder emits it. */ +const row = (operator: string, value: unknown = ''): BuilderGroup => ({ + id: 'g', + logic: 'and', + conditions: [{ id: 'c1', field: 'closed_at', operator, value }], +}); + +describe('groupToCondition — the null predicates this inspector offers (objectui#9363)', () => { + it('CONTROL: a mapped operator still serializes, so an empty answer below is about that operator', () => { + expect(groupToCondition(row('equals', 'acme'))).toEqual({ closed_at: { $eq: 'acme' } }); + }); + + it('isNull serializes to the dialect\'s null predicate instead of vanishing', () => { + expect( + groupToCondition(row('isNull')), + 'an `Is null` row serialized to nothing; committing that ERASES dataset.filter', + ).toEqual({ closed_at: { $null: true } }); + }); + + it('isNotNull serializes to the same predicate negated', () => { + expect(groupToCondition(row('isNotNull'))).toEqual({ closed_at: { $null: false } }); + }); + + it('keeps a null row alongside a complete one instead of dropping either', () => { + expect(groupToCondition({ + id: 'g', + logic: 'and', + conditions: [ + { id: 'c1', field: 'stage', operator: 'equals', value: 'won' }, + { id: 'c2', field: 'closed_at', operator: 'isNull', value: '' }, + ], + })).toEqual({ $and: [{ stage: { $eq: 'won' } }, { closed_at: { $null: true } }] }); + }); + + it('THE DEFECT: switching the only row of a stored filter to Is null no longer commits `undefined`', () => { + // The exact author gesture: a dataset that already has a filter, opened in + // the inspector, one operator change. `undefined` here is not "unchanged" — + // it is what the host spreads over the draft as `{ filter: undefined }`, + // the same patch shape `objectChangePatch` uses to CLEAR the filter. + const stored = { stage: { $eq: 'won' } }; + const { group, representable } = conditionToGroup(stored); + expect(representable).toBe(true); + const edited: BuilderGroup = { + ...group, + conditions: [{ ...group.conditions[0], operator: 'isNull', value: '' }], + }; + expect( + groupToCondition(edited), + 'the commit for this gesture was `undefined`, which erases the stored filter', + ).toEqual({ stage: { $null: true } }); + }); + + it('leaves the $exists pair exactly as it was', () => { + expect(groupToCondition(row('isEmpty'))).toEqual({ closed_at: { $exists: false } }); + expect(groupToCondition(row('isNotEmpty'))).toEqual({ closed_at: { $exists: true } }); + }); + + it('still drops an operator it does not map, rather than emitting a wrong filter', () => { + // Deliberate, and kept: see this file's header. These four are OFFERED by + // the menu and dropped, which erases the same way — tracked as its own + // finding, not widened here on the way past. + expect(groupToCondition(row('notContains', 'a'))).toBeUndefined(); + expect(groupToCondition(row('between', [1, 5]))).toBeUndefined(); + expect(groupToCondition(row('startsWith', 'a'))).toBeUndefined(); + expect(groupToCondition(row('endsWith', 'a'))).toBeUndefined(); + }); + + it('an empty group is still `undefined` — that is the author CLEARING the filter', () => { + expect(groupToCondition({ id: 'g', logic: 'and', conditions: [] })).toBeUndefined(); + }); +}); + +describe('the emitted token is the spec\'s, not a local invention (objectui#9363)', () => { + it('`$null` is a member of the spec\'s filter operator vocabulary', () => { + expect(FILTER_OPERATORS).toContain('$null'); + // Negative control: membership is a real reading, not a list that contains + // everything. A plausible-looking spelling this bridge could have invented + // is NOT in it. + expect(FILTER_OPERATORS).not.toContain('$isNull'); + }); + + it('the spec\'s field-operator door accepts the boolean comparand and refuses a wrong one', () => { + expect(FieldOperatorsSchema.safeParse({ $null: true }).success).toBe(true); + expect(FieldOperatorsSchema.safeParse({ $null: false }).success).toBe(true); + // Negative control: this door judges the VALUE, so a string comparand is + // refused — without this leg the assertion above would pass for a schema + // that accepts anything. + expect(FieldOperatorsSchema.safeParse({ $null: 'yes' }).success).toBe(false); + }); +}); + +describe('conditionToGroup — the read half round-trips the new shape (objectui#9363)', () => { + it('reads a stored $null back as the operator the author picked', () => { + expect(conditionToGroup({ closed_at: { $null: true } })).toEqual({ + group: { id: 'g', logic: 'and', conditions: [{ id: 'c0', field: 'closed_at', operator: 'isNull', value: '' }] }, + representable: true, + }); + expect(conditionToGroup({ closed_at: { $null: false } }).group.conditions[0].operator) + .toBe('isNotNull'); + }); + + it('round-trips condition → group → condition', () => { + for (const c of [{ closed_at: { $null: true } }, { closed_at: { $null: false } }]) { + const { group, representable } = conditionToGroup(c); + expect(representable, `${JSON.stringify(c)} fell back to the source editor`).toBe(true); + expect(groupToCondition(group)).toEqual(c); + } + }); +}); + +/** + * Offered ⇄ expressible parity for THIS inspector. + * + * The direction that broke: every guard in the repo sweeps spec → objectui, + * asking whether an operator an author may DECLARE can be rendered. None asks + * whether an operator this dropdown OFFERS can be stored by the consumer that + * mounted it — and that is the direction where an unmapped operator becomes + * silent data loss rather than a rendering gap. + * + * The offering is read from the builder's own bucket function with NO + * `extraOperators`, which is exactly what `DatasetFilterField` passes, so a + * future opt-in granted at that call site has to come through here. + */ +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', +]; + +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 dataset inspector's filter popover offers. */ +const OFFERED = offeredAcrossBuckets([]); + +/** + * Offered, and deliberately NOT expressible by this bridge today. + * + * Each one drops on commit, and a drop of the last surviving row erases the + * stored filter — the same mechanism objectui#9363 fixed for the null pair. + * They are listed rather than fixed here because mapping them is a separate + * decision per operator (`between` needs a both-bounds-present rule before it + * can be emitted at all), and a blanket "stop dropping" would emit filters that + * mean something else. Mapping one is what makes this list shrink — and this + * assertion go red until it is updated. + */ +const DECLARED_UNEXPRESSIBLE = ['between', 'endsWith', 'notContains', 'startsWith']; + +/** A value that keeps a row from being dropped as INCOMPLETE, per operator. */ +function probeValue(operator: string): unknown { + if (VALUELESS_FILTER_BUILDER_OPERATORS.has(operator)) return ''; + if (operator === 'in' || operator === 'notIn') return ['a', 'b']; + if (operator === 'between') return [1, 5]; + return 'x'; +} + +describe('every operator this inspector OFFERS is either expressible or declared (objectui#9363)', () => { + it('the probe types cover every bucket, so the offering below is the whole dropdown', () => { + // Granting every id as an opt-in yields the full drawable vocabulary only + // if the probe list reaches every bucket. Without this, a bucket added + // later would silently shrink what the partition below is asserted over. + expect(offeredAcrossBuckets(FILTER_BUILDER_OPERATORS)).toEqual([...FILTER_BUILDER_OPERATORS].sort()); + }); + + it('partitions the offering exactly — no operator is silently unhandled', () => { + const expressible: string[] = []; + const dropped: string[] = []; + for (const operator of OFFERED) { + (groupToCondition(row(operator, probeValue(operator))) === undefined ? dropped : expressible) + .push(operator); + } + expect(dropped.sort()).toEqual(DECLARED_UNEXPRESSIBLE); + // The other half of the equality: every remaining offered id serializes. + expect(expressible.sort()).toEqual(OFFERED.filter((o) => !DECLARED_UNEXPRESSIBLE.includes(o))); + // And the null pair is on the expressible side — the card's defect, stated + // as a fact about the offering rather than about two literals. + expect(expressible).toContain('isNull'); + expect(expressible).toContain('isNotNull'); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.ts b/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.ts index 314d05c5b0..1cd8052cd4 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.ts +++ b/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.ts @@ -11,6 +11,11 @@ * (nested groups, `$or`, multi-operator objects, unmapped operators) is reported * as NOT representable so the caller can fall back to the source editor instead * of silently corrupting the author's filter. + * + * The value-less operators are the exception to "field op value": the builder + * draws no input for them, so the row is complete without one. Both pairs the + * spec's vocabulary carries — `$exists` (is empty) and `$null` (is null) — are + * bridged here, in {@link VALUELESS_TO_MONGO}. */ /** FilterBuilder camelCase operator → FilterCondition Mongo operator. */ @@ -26,6 +31,32 @@ const MONGO_TO_OP: Record = { $contains: 'contains', $in: 'in', $nin: 'notIn', }; +/** + * Value-less builder operators, and the predicate each one lowers to. + * + * A row carrying one of these is COMPLETE without a value — the builder draws + * no input for it — so they are matched ahead of the value-completeness check + * in {@link groupToCondition}, not after it. + * + * `isNull` / `isNotNull` are not a spelling of `isEmpty` / `isNotEmpty`. The + * dropdown offers both pairs as their own rows and the spec's filter vocabulary + * carries both `$null` and `$exists`, so they stay distinct in both directions; + * collapsing them would draw two labels for one wire predicate and rewrite the + * author's choice when the filter is read back. + * + * objectui#9363: the null pair was missing here, so an `Is null` row — an + * ordinary entry in this inspector's menu, drawn as a finished row — fell + * through to the unmapped-operator `continue` below and was dropped. Dropping + * the last surviving row makes this function return `undefined`, and the + * inspector commits that as `{ filter: undefined }`, the same patch shape used + * to CLEAR the filter. So picking the entry erased the author's stored filter, + * with no error and the condition still on screen. + */ +const VALUELESS_TO_MONGO: Record> = { + isEmpty: { $exists: false }, isNotEmpty: { $exists: true }, + isNull: { $null: true }, isNotNull: { $null: false }, +}; + export interface BuilderCondition { id?: string; field: string; operator: string; value?: unknown } export interface BuilderGroup { id?: string; logic: 'and' | 'or'; conditions: BuilderCondition[] } @@ -53,10 +84,17 @@ export function groupToCondition(group: BuilderGroup | undefined): FilterConditi const conds = (group?.conditions ?? []).filter((c) => c && c.field); const parts: FilterCondition[] = []; for (const c of conds) { - if (c.operator === 'isEmpty') { parts.push({ [c.field]: { $exists: false } }); continue; } - if (c.operator === 'isNotEmpty') { parts.push({ [c.field]: { $exists: true } }); continue; } + const valueless = VALUELESS_TO_MONGO[c.operator]; + if (valueless) { parts.push({ [c.field]: { ...valueless } }); continue; } const mop = OP_TO_MONGO[c.operator]; - if (!mop) continue; // unmapped (e.g. notContains/between) — drop rather than emit a bad filter + // Still dropped rather than emitted in a spelling that means something + // else. ⚠️ The drop is not free: it is what erases the stored filter when + // no other row survives (see VALUELESS_TO_MONGO), and this menu offers + // `notContains` / `between` / `startsWith` / `endsWith`, none of which this + // table maps. Mapping one is a per-operator decision — `between` needs a + // both-bounds-present rule before it can be emitted at all — so they are + // declared, and pinned, in `datasetFilterCondition.nullOperators-9363`. + if (!mop) continue; // Skip incomplete rows (no value typed yet) — emitting `{field:{$op:''}}` would // be a silently-wrong filter (matches only empty), not "no filter". const v = c.value; @@ -96,6 +134,13 @@ export function conditionToGroup(cond: FilterCondition | undefined | null): { gr const mop = opKeys[0]; if (mop === '$exists') { conditions.push({ id: `c${i}`, field, operator: v.$exists ? 'isNotEmpty' : 'isEmpty', value: '' }); + } else if (mop === '$null') { + // The inverse of the write half: `$null: false` is "is not null", so + // the boolean picks the operator rather than becoming the row's value. + // Without this arm a filter this bridge now WRITES would read back as + // non-representable, sending the author to the Source tab for a row the + // builder can draw. + conditions.push({ id: `c${i}`, field, operator: v.$null ? 'isNull' : 'isNotNull', value: '' }); } else { const op = MONGO_TO_OP[mop]; if (!op) return { group: empty, representable: false }; From 4ae2f6177f0f7259a6c733bd0bbc8272dba384a8 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 13 Sep 2026 08:26:16 +0000 Subject: [PATCH 2/2] fix(app-shell): an unmapped dataset-filter operator is inert, not destructive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `groupToCondition` answers `undefined` both when the author CLEARED the filter and when nothing survived serialization, and the dataset inspector — which commits on every change — treated the two the same. The host applies patches as `{ ...draft, ...patch }`, so that commit SET `dataset.filter` to `undefined`, which is exactly the shape `objectChangePatch` uses deliberately to erase it. Two ordinary gestures reached it: switching the only condition's operator to one this bridge did not map, and — needing no operator at all — blanking the value of the only row. The stored filter was destroyed silently in both. The two meanings are now distinguished by `isClearedGroup`: a group that still holds rows commits nothing and leaves the stored filter alone; a group with no rows is the author's own clear gesture and still commits `undefined`. Deliberately not "emit something anyway" — a filter in a spelling that means something else is worse than a dropped one, so the unmapped arm still drops. `notContains`, `startsWith` and `endsWith` are no longer unmapped: each bridges to the spec's own token, backed by the Filter Protocol's canonical text-operator cases and its declared-type door, which passes them over `text` and refuses them over `number` / `date` — the only bucket this builder offers them on. `between` stays out until it has a both-bounds-present rule, and is now inert. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ --- .changeset/9372-unmapped-operator-inert.md | 47 +++ ...DefaultInspector.filterInert-9372.test.tsx | 116 +++++++ .../inspectors/DatasetDefaultInspector.tsx | 29 +- ...FilterCondition.nullOperators-9363.test.ts | 35 ++- .../inspectors/datasetFilterCondition.test.ts | 7 +- .../inspectors/datasetFilterCondition.ts | 86 +++++- ...FilterCondition.unmappedInert-9372.test.ts | 282 ++++++++++++++++++ 7 files changed, 578 insertions(+), 24 deletions(-) create mode 100644 .changeset/9372-unmapped-operator-inert.md create mode 100644 packages/app-shell/src/views/metadata-admin/inspectors/DatasetDefaultInspector.filterInert-9372.test.tsx create mode 100644 packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.unmappedInert-9372.test.ts diff --git a/.changeset/9372-unmapped-operator-inert.md b/.changeset/9372-unmapped-operator-inert.md new file mode 100644 index 0000000000..c6da2950c9 --- /dev/null +++ b/.changeset/9372-unmapped-operator-inert.md @@ -0,0 +1,47 @@ +--- +'@object-ui/app-shell': patch +--- + +Stop the Studio dataset-filter bridge from ERASING a stored filter when an edit +cannot be serialized (objectui#9372). Behaviour change, not just a fix: three more +operators are now STORED where they used to be dropped. + +**The erase.** `groupToCondition` answers `undefined` both when the author CLEARED the +filter and when nothing survived serialization, and the inspector — which commits on +every change — treated the two the same. The host applies patches as +`{ ...draft, ...patch }`, so that commit SET `dataset.filter` (or a `measure.filter`) +to `undefined`, which is exactly the patch shape `objectChangePatch` uses deliberately +to erase it. Nothing errored. + +Two ordinary gestures reached it. Switching the only condition's operator to one this +bridge did not map — `notContains`, `startsWith`, `endsWith` and `between`, all four +ordinary entries in this inspector's menu, none of them opt-in. And, needing no +operator at all, simply BLANKING the value of the only row: an incomplete row is +dropped by the same path, the last part goes with it, and the answer is `undefined`. + +**The fix, unconditional and ahead of any per-operator question.** The two meanings are +now distinguished: a group that still holds rows commits NOTHING and the stored filter +is left alone; only a group with no rows — Clear all, or the last row removed — still +commits `undefined`, because that is the author's own gesture. An operator this bridge +cannot express is therefore inert, whichever operators it maps. + +⚠️ Deliberately not "emit something anyway". A filter emitted in a spelling that means +something else is worse than one that was dropped, so the unmapped arm still drops. + +**And three of the four are no longer unmapped.** `notContains`, `startsWith` and +`endsWith` now serialize to the spec's own `$notContains` / `$startsWith` / `$endsWith` +and read back as the operator the author picked. The comment calling them operators +"this dialect genuinely cannot express" was stale: `FILTER_OPERATORS` carries all four. +Each is backed by a conformance reading rather than a guess — the Filter Protocol's +canonical `FILTER_TEXT_CASES` covers all three, the spec's declared-type door passes +them over `text` and refuses them over `number` / `date` / `boolean`, and this builder +offers them only on its text bucket. + +`between` stays unmapped, for a reason about this bridge rather than the vocabulary: +the builder pads a half-typed pair with an empty bound and the spec's comparand door +accepts `[1, '']`, so emitting it needs a both-bounds-present rule first. It is now +unmapped and inert instead of unmapped and destructive. + +Forward note for anyone pinning stored filters: a dataset filter written by this +version may carry `$notContains` / `$startsWith` / `$endsWith`, which an older +app-shell reads as non-representable and degrades to "edit it in the Source tab". diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/DatasetDefaultInspector.filterInert-9372.test.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/DatasetDefaultInspector.filterInert-9372.test.tsx new file mode 100644 index 0000000000..b42d5fc5bb --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/inspectors/DatasetDefaultInspector.filterInert-9372.test.tsx @@ -0,0 +1,116 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The inspector does not COMMIT an erase (objectui#9372). + * + * ## Why this file exists next to the pure one + * + * `datasetFilterCondition.unmappedInert-9372.test.ts` pins the decision — + * {@link isClearedGroup} tells the author's CLEAR gesture apart from a + * serialization that produced nothing. A decision nobody consults is a channel + * with no reader, and the pure file cannot tell the difference: it would stay + * green with the guard sitting unused beside the old `onCommit(...)` call. + * + * This file drives the real component, with the real `FilterBuilder` mounted + * inside it, and watches the ONE thing that caused the data loss — the patch. + * + * ## The gesture, and why it is the value and not the operator menu + * + * The card's route is an operator pick, but the same defect is reachable by + * blanking the VALUE of the only row, with no operator involved at all: the + * incomplete-row `continue` drops it, the last part goes, and the commit is + * `undefined`. The host applies patches as `{ ...draft, ...patch }`, so that + * commit SETS `filter` to `undefined` — the patch shape `objectChangePatch` + * uses deliberately to erase it. Blanking a text input is also the one gesture + * that needs no Radix listbox interaction, so this pin holds without driving a + * select open in a headless DOM. + * + * ## The control + * + * "`onPatch` was not called" is also what an unopened popover, a mis-queried + * input and a dead handler all look like. So the same file types a REAL value + * through the same input and asserts the patch that produces — if that control + * stops firing, the absence below stops meaning anything. + */ +import { describe, it, expect, vi, afterEach, type Mock } from 'vitest'; +import { render, screen, fireEvent, cleanup } from '@testing-library/react'; +import '@testing-library/jest-dom'; + +// Stub the catalog hooks so the inspector renders without a MetadataClient / +// network, but with ONE text field so the filter popover has something to draw. +vi.mock('./useDatasetFields', () => ({ + useObjectOptions: () => ({ options: [], loading: false }), + useDatasetFieldCatalog: () => ({ + relationships: [], + fieldOptions: [{ value: 'name', label: 'Name', type: 'text' }], + loading: false, + }), + useDatasetUsage: () => ({ reports: 0, dashboards: 0, loading: false }), + fieldTypeToDimensionType: (t: string) => (t === 'date' ? 'date' : 'string'), +})); + +import { DatasetDefaultInspector } from './DatasetDefaultInspector'; + +afterEach(cleanup); + +const baseProps = { type: 'dataset', name: 'sales', locale: 'en-US' as const }; + +/** A dataset whose filter is ALREADY stored — the thing that got destroyed. */ +const draft = { + name: 'sales', + label: 'Sales', + object: 'opportunity', + include: [], + dimensions: [], + measures: [], + filter: { name: { $eq: 'acme' } }, +}; + +/** The inspector's patch channel, typed as the component declares it. */ +type PatchSpy = Mock<(patch: Record) => void>; +const patchSpy = (): PatchSpy => vi.fn<(patch: Record) => void>(); + +/** Render, open the Scope filter popover, and hand back the row's value input. */ +function openScopeFilter(onPatch: PatchSpy) { + render(); + // The trigger summarises the stored filter; seeing it at all is already a + // reading that `conditionToGroup` found the stored shape representable. + fireEvent.click(screen.getByText('1 condition')); + return screen.getByDisplayValue('acme') as HTMLInputElement; +} + +describe('DatasetDefaultInspector — a filter edit that cannot be stored commits nothing (objectui#9372)', () => { + it('CONTROL: typing a real value still commits it, so the absence below is about the blank', () => { + const onPatch = patchSpy(); + const input = openScopeFilter(onPatch); + fireEvent.change(input, { target: { value: 'contoso' } }); + expect(onPatch).toHaveBeenCalledWith({ filter: { name: { $eq: 'contoso' } } }); + }); + + it('THE DEFECT: blanking the only row\'s value does NOT patch `filter` to undefined', () => { + const onPatch = patchSpy(); + const input = openScopeFilter(onPatch); + fireEvent.change(input, { target: { value: '' } }); + // Before the repair this called `onPatch({ filter: undefined })`, which the + // host spreads over the draft — the stored filter destroyed, nothing shown + // to the author, and `JSON.stringify` then omits the key on save. + for (const [patch] of onPatch.mock.calls) { + expect( + patch, + 'the inspector committed a patch carrying `filter`; if it is undefined, that ERASES the stored filter', + ).not.toHaveProperty('filter'); + } + }); + + it('and the author\'s own CLEAR gesture still reaches the draft', () => { + // The other half: "Clear all" empties the group, which IS the clear + // gesture, and must still commit `undefined`. Without this the repair + // could have been a blanket "never commit undefined", stranding the filter + // an author asked to remove. + const onPatch = patchSpy(); + render(); + fireEvent.click(screen.getByText('1 condition')); + fireEvent.click(screen.getByText('Clear all')); + expect(onPatch).toHaveBeenCalledWith({ filter: undefined }); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/DatasetDefaultInspector.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/DatasetDefaultInspector.tsx index b16666d8d6..15276fb767 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/DatasetDefaultInspector.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/DatasetDefaultInspector.tsx @@ -34,7 +34,7 @@ import { InspectorComboField, type InspectorComboOption } from './InspectorCombo import { toFieldName } from '../previews/object-fields-io.js'; import { formatMeasure } from '@object-ui/core'; import { useDisplayLocale } from '@object-ui/i18n'; -import { conditionToGroup, groupToCondition, type FilterCondition } from './datasetFilterCondition.js'; +import { conditionToGroup, groupToCondition, isClearedGroup, type BuilderGroup, type FilterCondition } from './datasetFilterCondition.js'; import { useObjectOptions, useDatasetFieldCatalog, @@ -244,6 +244,31 @@ function DatasetFilterField({ label, help, value, onCommit, fields, disabled }: }) { const { group, representable } = conditionToGroup(value); const count = group.conditions.length; + /** + * Commit an edit — unless nothing survived serialization while rows are + * still on screen (objectui#9372). + * + * `groupToCondition` answers `undefined` both when the author CLEARED the + * filter and when every row was dropped, and this commit is what turns the + * second one into data loss: `onCommit` lands as `onPatch({ filter })`, the + * host applies it as `{ ...draft, ...patch }`, so `filter` is SET to + * `undefined` — the very patch shape `objectChangePatch` uses to erase it. + * An unmapped operator (`between`) or a blanked value on the only row would + * therefore destroy a working stored filter, silently. + * + * Holding the patch leaves the stored value alone, which is the whole + * requirement. ⛔ It is deliberately not "emit something anyway": a filter in + * a spelling that means something else is worse than one that was dropped. + * ⚠️ Known and accepted: the builder re-seeds its own state from `value` + * whenever the two differ, so an unexpressible row is lost from the panel on + * the next render the inspector happens to do. Losing an edit the bridge + * could never have stored is not in the same class as destroying one it had. + */ + const commitFilterGroup = (g: BuilderGroup) => { + const next = groupToCondition(g); + if (next === undefined && !isClearedGroup(g)) return; + onCommit(next); + }; return (
@@ -263,7 +288,7 @@ function DatasetFilterField({ label, help, value, onCommit, fields, disabled }: {fields.length === 0 ? (

Pick a base object to add filter conditions.

) : ( - onCommit(groupToCondition(g))} /> + )} diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.nullOperators-9363.test.ts b/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.nullOperators-9363.test.ts index 382b4a4f5a..4c8f8735f3 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.nullOperators-9363.test.ts +++ b/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.nullOperators-9363.test.ts @@ -108,13 +108,14 @@ describe('groupToCondition — the null predicates this inspector offers (object }); it('still drops an operator it does not map, rather than emitting a wrong filter', () => { - // Deliberate, and kept: see this file's header. These four are OFFERED by - // the menu and dropped, which erases the same way — tracked as its own - // finding, not widened here on the way past. - expect(groupToCondition(row('notContains', 'a'))).toBeUndefined(); + // Deliberate, and kept: see this file's header. + // + // objectui#9372 took the other three of the four this listed — the + // `notContains` / `startsWith` / `endsWith` rows are asserted as EMITTED + // in `datasetFilterCondition.unmappedInert-9372`, with the conformance + // reading behind each — and made the remaining drop inert. `between` is + // what is left: still offered, still dropped, and no longer destructive. expect(groupToCondition(row('between', [1, 5]))).toBeUndefined(); - expect(groupToCondition(row('startsWith', 'a'))).toBeUndefined(); - expect(groupToCondition(row('endsWith', 'a'))).toBeUndefined(); }); it('an empty group is still `undefined` — that is the author CLEARING the filter', () => { @@ -191,15 +192,21 @@ const OFFERED = offeredAcrossBuckets([]); /** * Offered, and deliberately NOT expressible by this bridge today. * - * Each one drops on commit, and a drop of the last surviving row erases the - * stored filter — the same mechanism objectui#9363 fixed for the null pair. - * They are listed rather than fixed here because mapping them is a separate - * decision per operator (`between` needs a both-bounds-present rule before it - * can be emitted at all), and a blanket "stop dropping" would emit filters that - * mean something else. Mapping one is what makes this list shrink — and this - * assertion go red until it is updated. + * Each one drops on commit. ⚠️ That drop used to ERASE the stored filter when + * no other row survived — the same mechanism objectui#9363 fixed for the null + * pair — and objectui#9372 ended that: the caller now tells "nothing survived" + * apart from "the author cleared", so a drop is inert + * (`datasetFilterCondition.unmappedInert-9372`). Being on this list is now a + * missing capability, not data loss. + * + * objectui#9372 also took three of the four this listed. `between` is what + * remains, and it remains for a reason that is about THIS bridge rather than + * the spec's vocabulary: the builder pads a half-typed pair with `''` and the + * spec's comparand door accepts `[1, '']`, so it needs a both-bounds-present + * rule before it can be emitted at all. Mapping it is what makes this list + * shrink — and this assertion go red until it is updated. */ -const DECLARED_UNEXPRESSIBLE = ['between', 'endsWith', 'notContains', 'startsWith']; +const DECLARED_UNEXPRESSIBLE = ['between']; /** A value that keeps a row from being dropped as INCOMPLETE, per operator. */ function probeValue(operator: string): unknown { diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.test.ts b/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.test.ts index b26d609ab4..65f5536a20 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.test.ts +++ b/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.test.ts @@ -21,7 +21,12 @@ describe('datasetFilterCondition', () => { }); it('drops unmapped operators rather than emitting a bad filter', () => { - expect(groupToCondition({ logic: 'and', conditions: [{ field: 'x', operator: 'notContains', value: 'a' }] })) + // The claim is unchanged; the FIXTURE moved. `notContains` stopped being + // an unmapped operator in objectui#9372 (it is bridged to `$notContains`, + // asserted there), so keeping it here would have pinned a branch it no + // longer reaches — an assertion that passes because nothing is produced. + // `between` is the operator this bridge still declines to emit. + expect(groupToCondition({ logic: 'and', conditions: [{ field: 'x', operator: 'between', value: [1, 5] }] })) .toBeUndefined(); }); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.ts b/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.ts index 1cd8052cd4..a0389a42ed 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.ts +++ b/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.ts @@ -16,6 +16,10 @@ * draws no input for them, so the row is complete without one. Both pairs the * spec's vocabulary carries — `$exists` (is empty) and `$null` (is null) — are * bridged here, in {@link VALUELESS_TO_MONGO}. + * + * An operator that is NOT bridged is dropped, and dropping is where the danger + * used to be: see {@link isClearedGroup} for why an unmapped operator is now + * inert rather than destructive (objectui#9372). */ /** FilterBuilder camelCase operator → FilterCondition Mongo operator. */ @@ -24,11 +28,19 @@ const OP_TO_MONGO: Record = { greaterThan: '$gt', greaterOrEqual: '$gte', lessThan: '$lt', lessOrEqual: '$lte', after: '$gt', before: '$lt', contains: '$contains', in: '$in', notIn: '$nin', + // objectui#9372. The builder offers these three only on its TEXT bucket, + // which is the side the spec's declared-type door passes them on + // (`TEXT_OPERATOR_DOOR_CASES`: `passes` over `text`, `door-refusal` over + // `number` / `date` / `boolean`), and every filter backend answers them + // against the same canonical table (`FILTER_TEXT_CASES`). So mapping them is + // a bridge to a predicate the platform already agrees on, not a new claim. + notContains: '$notContains', startsWith: '$startsWith', endsWith: '$endsWith', }; const MONGO_TO_OP: Record = { $eq: 'equals', $ne: 'notEquals', $gt: 'greaterThan', $gte: 'greaterOrEqual', $lt: 'lessThan', $lte: 'lessOrEqual', $contains: 'contains', $in: 'in', $nin: 'notIn', + $notContains: 'notContains', $startsWith: 'startsWith', $endsWith: 'endsWith', }; /** @@ -79,21 +91,81 @@ export type { FilterCondition } from '@objectstack/spec/data'; import type { FilterCondition } from '@objectstack/spec/data'; +/** + * The rows this bridge will even look at. A row with no field picked is not + * yet a row — the builder seeds one the moment "Add condition" is clicked — + * so it is neither serialized nor counted as something the author typed. + * + * One definition, two readers: {@link groupToCondition} filters by it and + * {@link isClearedGroup} counts it. Two copies of this predicate is exactly + * how "the group is empty" and "the group serialized to nothing" could drift + * apart again. + */ +function liveRows(group: BuilderGroup | undefined): BuilderCondition[] { + return (group?.conditions ?? []).filter((c) => c && c.field); +} + +/** + * Is an `undefined` answer from {@link groupToCondition} the author CLEARING + * the filter (objectui#9372)? + * + * ## The conflation this exists to end + * + * `undefined` out of {@link groupToCondition} meant two different things — + * *"the author cleared the filter"* and *"nothing survived serialization"* — + * and the only caller treated both as clear. Since the inspector commits on + * every change, and the host applies patches as `{ ...draft, ...patch }`, that + * commit SETS `filter` to `undefined`: the same patch shape + * `objectChangePatch` uses deliberately to erase it. So a serialization that + * produced nothing destroyed the author's stored filter. + * + * Reachable two ways, and both are the same defect: + * + * - switching the only row to an operator this bridge does not map + * (objectui#9363 closed `isNull` / `isNotNull`; `between` is still one); + * - blanking the VALUE of the only row, which needs no operator at all — the + * incomplete-row `continue` drops it and the last part goes with it. + * + * ## What the caller does with the answer + * + * `false` means "rows are still on screen": the caller must patch NOTHING and + * leave the stored value alone. `true` — no rows at all, i.e. Clear all, or + * the last row removed — is the author's own gesture and still commits + * `undefined`. + * + * ⛔ Deliberately not "emit something for the unmapped operator". A filter + * emitted in a spelling that means something else is worse than a dropped one, + * which is the whole reason the unmapped arm exists; this makes the drop inert, + * it does not stop it dropping. + */ +export function isClearedGroup(group: BuilderGroup | undefined): boolean { + return liveRows(group).length === 0; +} + /** Serialize the visual group → a spec FilterCondition (flat `$and`). */ export function groupToCondition(group: BuilderGroup | undefined): FilterCondition | undefined { - const conds = (group?.conditions ?? []).filter((c) => c && c.field); + const conds = liveRows(group); const parts: FilterCondition[] = []; for (const c of conds) { const valueless = VALUELESS_TO_MONGO[c.operator]; if (valueless) { parts.push({ [c.field]: { ...valueless } }); continue; } const mop = OP_TO_MONGO[c.operator]; // Still dropped rather than emitted in a spelling that means something - // else. ⚠️ The drop is not free: it is what erases the stored filter when - // no other row survives (see VALUELESS_TO_MONGO), and this menu offers - // `notContains` / `between` / `startsWith` / `endsWith`, none of which this - // table maps. Mapping one is a per-operator decision — `between` needs a - // both-bounds-present rule before it can be emitted at all — so they are - // declared, and pinned, in `datasetFilterCondition.nullOperators-9363`. + // else — that decision is the reason this arm exists and it is unchanged. + // + // What changed (objectui#9372) is the COST of the drop. It used to erase + // the author's stored filter whenever no other row survived; now + // {@link isClearedGroup} lets the caller tell that apart from a real clear, + // so an unmapped operator is inert. ⚠️ Do not read the drop as "this + // dialect cannot express it": the spec's `FILTER_OPERATORS` carries + // `$notContains`, `$startsWith`, `$endsWith` AND `$between`. The three text + // ones are mapped above. `between` is the one still offered here (on the + // date bucket) and still unmapped, for a reason that is about THIS bridge + // rather than the vocabulary: the builder pads a half-typed pair with `''` + // and the spec's comparand door accepts `[1, '']`, so emitting it needs a + // both-bounds-present rule first. The partition is pinned in + // `datasetFilterCondition.nullOperators-9363`, the inertness in + // `datasetFilterCondition.unmappedInert-9372`. if (!mop) continue; // Skip incomplete rows (no value typed yet) — emitting `{field:{$op:''}}` would // be a silently-wrong filter (matches only empty), not "no filter". diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.unmappedInert-9372.test.ts b/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.unmappedInert-9372.test.ts new file mode 100644 index 0000000000..17f74844b3 --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.unmappedInert-9372.test.ts @@ -0,0 +1,282 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * An operator this bridge cannot express must be INERT, never destructive + * (objectui#9372). + * + * ## The defect, and the split that carries it + * + * objectui#9363 repaired `isNull` / `isNotNull` one operator at a time. This + * card is the same destruction four more times — `between`, `endsWith`, + * `notContains`, `startsWith`, every one of them an ordinary entry in this + * inspector's menu — and it separates two questions the card's own option list + * ran together: + * + * (i) WHICH operators map to the dialect — a per-operator conformance + * question, answered below for three of the four. + * (ii) What happens when one does NOT map — not an open question. Erasing + * the author's stored filter is wrong whatever (i) answers. + * + * (ii) is what this file exists for, and it is fixed unconditionally: the + * mechanism is that `undefined` out of {@link groupToCondition} means BOTH + * "the author cleared the filter" AND "nothing survived serialization", and the + * caller treated both as clear. That conflation is what makes an unmapped + * operator destructive rather than inert. + * + * ## The second route, which needs no operator at all + * + * The conflation is reachable by blanking the VALUE of the only row — an + * incomplete row is dropped by the same `continue`, the last part disappears, + * and the function answers `undefined`. Pinned below beside the operator + * route, because it is the same defect and a repair aimed only at operators + * would leave it standing. + * + * ## What "leave it alone" is, and what it is deliberately not + * + * {@link isClearedGroup} answers the one question the caller could not ask + * before: was `undefined` the author's CLEAR gesture? Only then is `undefined` + * committed. Otherwise the caller patches nothing and the stored filter is + * untouched. + * + * ⚠️ NOT "emit something". Emitting a filter in a spelling that means something + * else is worse than dropping — that is the whole reason the unmapped arm + * exists, and it survives this repair intact. `between` is still dropped + * below, and now dropped inertly. + * + * ## Red-first + * + * Predicted before running, on the unmodified tree: the (ii) block fails at + * `isClearedGroup` not being a function, and the three (i) mappings fail with + * `undefined`, while the `equals` control in the SAME run passes — a table of + * all-`undefined` answers and a dead function look identical otherwise. + */ +import { describe, it, expect } from 'vitest'; +import { + FILTER_OPERATORS, + FieldOperatorsSchema, + FILTER_TEXT_CASES, + TEXT_OPERATOR_DOOR_CASES, +} from '@objectstack/spec/data'; +import { filterValueArity, operatorsForFieldType } from '@object-ui/components'; +import { groupToCondition, conditionToGroup, isClearedGroup } from './datasetFilterCondition'; +import type { BuilderGroup } from './datasetFilterCondition'; + +/** One condition row, as the builder emits it. */ +const row = (operator: string, value: unknown = ''): BuilderGroup => ({ + id: 'g', + logic: 'and', + conditions: [{ id: 'c1', field: 'name', operator, value }], +}); + +/** The author's stored filter, before they touch anything. */ +const STORED = { name: { $eq: 'acme' } }; + +/** + * The commit decision, spelled exactly as `DatasetFilterField` spells it. + * + * Returned rather than asserted inline so each gesture below reads as "what + * would this commit", and so the HOLD case is a value rather than the absence + * of a call. + */ +function commitFor(group: BuilderGroup): { hold: true } | { hold: false; filter: unknown } { + const next = groupToCondition(group); + if (next === undefined && !isClearedGroup(group)) return { hold: true }; + return { hold: false, filter: next }; +} + +describe('(ii) an operator this bridge cannot express is inert, not destructive (objectui#9372)', () => { + it('CONTROL: a mapped operator still serializes, so an empty answer below is about that operator', () => { + expect(groupToCondition(row('equals', 'acme'))).toEqual({ name: { $eq: 'acme' } }); + }); + + it('tells the author\'s CLEAR gesture apart from a serialization that produced nothing', () => { + // No rows at all is the clear gesture — "Clear all", or removing the last + // row. `undefined` is the right commit for it and stays that way. + expect(isClearedGroup({ id: 'g', logic: 'and', conditions: [] })).toBe(true); + expect(isClearedGroup(undefined)).toBe(true); + // A row with no field picked is not yet a row — `groupToCondition` filters + // it out before anything else, so the two must agree here. + expect(isClearedGroup(row('equals', 'acme'))).toBe(false); + expect(isClearedGroup({ id: 'g', logic: 'and', conditions: [{ id: 'c1', field: '', operator: 'equals', value: 'x' }] })).toBe(true); + }); + + it('THE GESTURE, operator route: switching the only row to an unmapped operator commits NOTHING', () => { + // The exact author gesture the card describes: a dataset that already has + // a filter, opened in the inspector, one operator change. Before this + // repair the commit was `undefined`, which the host spreads over the draft + // as `{ filter: undefined }` — the same patch shape `objectChangePatch` + // uses deliberately to CLEAR the filter. + const { group, representable } = conditionToGroup(STORED); + expect(representable).toBe(true); + const edited: BuilderGroup = { + ...group, + conditions: [{ ...group.conditions[0], operator: 'between', value: [1, 5] }], + }; + expect(groupToCondition(edited)).toBeUndefined(); + expect( + commitFor(edited), + 'this gesture used to commit `undefined`, which ERASES the stored filter', + ).toEqual({ hold: true }); + }); + + it('THE GESTURE, blank-value route: blanking the only row\'s value commits NOTHING — no operator needed', () => { + // The same defect reached without touching the operator menu at all: an + // incomplete row is dropped by the same `continue`, the last part goes, + // and the answer is `undefined`. + const { group } = conditionToGroup(STORED); + const blanked: BuilderGroup = { + ...group, + conditions: [{ ...group.conditions[0], value: '' }], + }; + expect(groupToCondition(blanked)).toBeUndefined(); + expect( + commitFor(blanked), + 'blanking the only row used to erase the stored filter, with no operator involved', + ).toEqual({ hold: true }); + }); + + it('a partly-edited group still commits the rows that DID survive', () => { + // Holding is only for "nothing survived". One good row and one blank one + // must still commit the good row, exactly as before. + const mixed: BuilderGroup = { + id: 'g', + logic: 'and', + conditions: [ + { id: 'c1', field: 'stage', operator: 'equals', value: 'won' }, + { id: 'c2', field: 'name', operator: 'between', value: [1, 5] }, + ], + }; + expect(commitFor(mixed)).toEqual({ hold: false, filter: { stage: { $eq: 'won' } } }); + }); + + it('the author CLEARING the filter still clears it — the repair does not strand a stale filter', () => { + // The other half of the equality, and the reason this is a disambiguation + // rather than a blanket "never commit undefined": with no rows left there + // is no edit to preserve, and `undefined` is the author's own gesture. + expect(commitFor({ id: 'g', logic: 'and', conditions: [] })).toEqual({ hold: false, filter: undefined }); + }); +}); + +describe('(i) the three text operators this bridge now expresses (objectui#9372)', () => { + const MAPPED: ReadonlyArray = [ + ['notContains', '$notContains'], + ['startsWith', '$startsWith'], + ['endsWith', '$endsWith'], + ]; + + it('serializes each one to the spec\'s own token', () => { + for (const [operator, token] of MAPPED) { + expect(groupToCondition(row(operator, 'ac')), `${operator} serialized to nothing`) + .toEqual({ name: { [token]: 'ac' } }); + } + }); + + it('reads each one back as the operator the author picked', () => { + for (const [operator, token] of MAPPED) { + const { group, representable } = conditionToGroup({ name: { [token]: 'ac' } }); + expect(representable, `${token} fell back to the Source tab`).toBe(true); + expect(group.conditions[0].operator).toBe(operator); + expect(groupToCondition(group)).toEqual({ name: { [token]: 'ac' } }); + } + }); + + it('PREMISE, re-measured: the dialect CAN express all four — the file\'s comment was stale', () => { + // The `unmapped (e.g. notContains/between)` comment read as "operators this + // dialect genuinely cannot express". Measured against the pinned spec, all + // four are members of its filter vocabulary, so the premise is false for + // every one of them: they were not inexpressible, they were unmapped. + for (const token of ['$notContains', '$startsWith', '$endsWith', '$between']) { + expect(FILTER_OPERATORS).toContain(token); + } + // Negative control: membership is a real reading, not a list that contains + // everything. A plausible spelling this bridge could have invented is not + // in it. + expect(FILTER_OPERATORS).not.toContain('$beginsWith'); + }); + + it('CONFORMANCE: each one carries canonical driver cases, and `$between` is not in that table', () => { + // The reading `$null` has and these were said to lack. `FILTER_TEXT_CASES` + // is the Filter Protocol's text-operator standard — the table every filter + // backend is checked against — and it carries rows for all three. + const covered = new Set(); + for (const c of FILTER_TEXT_CASES) { + for (const ops of Object.values(c.filter as Record)) { + if (ops && typeof ops === 'object') for (const k of Object.keys(ops)) covered.add(k); + } + } + for (const [, token] of MAPPED) expect(covered, `${token} has no text-conformance rows`).toContain(token); + // Negative control: this is a reading of one table, not of "every operator + // is covered". `$between` is a range operator and is NOT in it — which is + // why the conformance answer for `between` has to be sought elsewhere, and + // is not what this assertion supplies. + expect(covered).not.toContain('$between'); + }); + + it('CONFORMANCE: the spec\'s declared-type door passes all three over text and refuses them over number', () => { + // The authoring half. This bridge only ever emits these three from the + // builder's TEXT bucket (asserted below), which is the side the door + // passes; the refusals are what make the pass a reading rather than a + // table that says yes to everything. + for (const [, token] of MAPPED) { + const forText = TEXT_OPERATOR_DOOR_CASES.filter((c) => c.operator === token && c.declaredType === 'text'); + expect(forText.length, `${token} has no door case over text`).toBeGreaterThan(0); + for (const c of forText) expect(c.verdict, `${token} over text`).toBe('passes'); + const forNumber = TEXT_OPERATOR_DOOR_CASES.filter((c) => c.operator === token && c.declaredType === 'number'); + expect(forNumber.length, `${token} has no door case over number`).toBeGreaterThan(0); + for (const c of forNumber) expect(c.verdict, `${token} over number`).toBe('door-refusal'); + } + }); + + it('CONFORMANCE: the comparand door accepts the string this builder types and refuses a number', () => { + for (const [, token] of MAPPED) { + expect(FieldOperatorsSchema.safeParse({ [token]: 'ac' }).success, `${token} refused a string`).toBe(true); + // Negative control: this door judges the VALUE, so without this leg the + // assertion above would pass for a schema that accepts anything. + expect(FieldOperatorsSchema.safeParse({ [token]: 5 }).success, `${token} accepted a number`).toBe(false); + } + }); + + it('the builder only OFFERS these three on text-like fields, which is the side the door passes', () => { + // The two halves have to meet: the door refuses these operators over + // `number` / `date` / `boolean`, so mapping them is only safe while the + // dropdown never offers them there. Read from the builder's own bucket + // function rather than restated. + const offered = (type: string | undefined) => operatorsForFieldType(type, []).map((o) => o.value); + for (const [operator] of MAPPED) { + expect(offered('text'), `${operator} is not offered on text`).toContain(operator); + for (const type of ['number', 'currency', 'percent', 'rating', 'date', 'datetime', 'time', 'boolean']) { + expect(offered(type), `${operator} is offered on ${type}, where the spec's door refuses it`) + .not.toContain(operator); + } + } + }); +}); + +describe('`between` stays unmapped — and is now unmapped INERT (objectui#9372)', () => { + it('is still dropped rather than emitted', () => { + expect(groupToCondition(row('between', [1, 5]))).toBeUndefined(); + }); + + it('the reason it stays out, measured: nothing downstream catches a half-filled pair', () => { + // The builder pads a pair with `""` when only one bound is typed + // (`reshapeFilterValue`), and the row is two entries long, so this bridge's + // completeness check — which only rejects `null` / `''` / `[]` — would let + // it through. The spec's comparand door does not catch it either: a bound + // of `''` parses. So emitting `between` today would emit a filter that + // means something the author did not ask for, which is exactly what the + // unmapped arm exists to prevent. A both-bounds-present rule is the + // precondition, and it is a separate decision. + expect(filterValueArity('between')).toBe('pair'); + expect(FieldOperatorsSchema.safeParse({ $between: [1, 5] }).success).toBe(true); + expect( + FieldOperatorsSchema.safeParse({ $between: [1, ''] }).success, + 'if the spec refused a half-filled pair, this bridge could lean on it instead of a local rule', + ).toBe(true); + }); + + it('but picking it no longer erases the stored filter', () => { + // The whole point of the (i)/(ii) split: an operator can stay unmapped + // without staying destructive. + expect(commitFor(row('between', [1, 5]))).toEqual({ hold: true }); + }); +});