diff --git a/.changeset/filter-ast-combinators-6948.md b/.changeset/filter-ast-combinators-6948.md new file mode 100644 index 0000000000..6a55240eae --- /dev/null +++ b/.changeset/filter-ast-combinators-6948.md @@ -0,0 +1,30 @@ +--- +'@object-ui/core': minor +'@object-ui/data-objectstack': patch +--- + +`convertFiltersToAST` lowers the `$and` / `$or` combinators to real ObjectQL AST +group nodes (objectui#6948). + +`FilterCondition` declares `$and` / `$or` / `$not`, and this repo's one lowering +had no branch for any of them. `$and` / `$or` fell through to the +simple-equality branch and became a leaf naming a field literally called `$and` +/ `$or`. That leaf reached the server intact — `parseFilterAST` reads +`['$or', '=', [...]]` back as a real `$or`, so the wire condition was correct +and is unchanged by this release — but it is a well-formed *comparison* node, so +every AST evaluator in this repo read `$or` as a field name, found no such key +on any record, and returned an EMPTY list with no error. Producers that reach +this today include `mergeFilters` (dashboard scope broadcast, dataset report +blocks), `FilterConditionField`, and `Field.relatedListFilter`. + +`minor` rather than `patch`: shipped results move. A list filtered by a +combinator through any in-process data source went from zero rows to the rows +the author asked for, and an unknown or refused operator *inside* a combinator +branch — which used to travel to the wire unchecked inside the leaf's value slot +— is now refused at the same door as every other operator. + +`$not` is refused with an accurate message instead of translated: the AST has no +negation keyword (`FILTER_ARRAY_LOGIC_KEYWORDS` is `['and', 'or']`) and several +operators it carries have no negated counterpart, so a rewrite would be silently +partial. It threw before this change too, naming the author's own nested field +as a bogus operator; the verdict is unchanged, only the diagnostic. diff --git a/packages/core/src/utils/__tests__/filter-combinators-6948.test.ts b/packages/core/src/utils/__tests__/filter-combinators-6948.test.ts new file mode 100644 index 0000000000..e93640df51 --- /dev/null +++ b/packages/core/src/utils/__tests__/filter-combinators-6948.test.ts @@ -0,0 +1,345 @@ +/** + * 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. + */ + +/** + * `$and` / `$or` / `$not` through the repo's ONE lowering — objectui#6948. + * + * ## What this file asserts, and why it is row sets and not source shape + * + * The card asked for a branch. A branch existing is not the property that + * matters, so nothing here greps the source: every assertion is either the node + * that REACHES THE WIRE, put through the spec's own doors (`isFilterAST` / + * `parseFilterAST`, `@objectstack/spec/data`), or the ROW SET a real + * `ValueDataSource` returns for it. + * + * Both halves are pinned in both directions, because each half alone passes on + * an implementation strictly worse than the bug: + * + * - an assertion that a combinator is PRESENT passes on a builder that emits + * everything; + * - an assertion that a row set is EMPTY passes on a converter that emits + * nothing at all — `$filter: undefined` returns EVERY row, and a node no + * evaluator reads returns NONE. + * + * So every row-set case names the exact ids, and {@link ALL_IDS} is asserted to + * be neither of them. + * + * ## What was actually wrong — NOT what the card said + * + * The card stated that the pre-fix emission for `$and` / `$or` — a leaf naming a + * field literally called `$and` / `$or` — was refused by the server with `400 + * INVALID_FILTER`. Re-measured against `@objectstack/spec` 17.3.0 that is FALSE, + * and this repo already knew: `data-objectstack/src/filter-entry-translation.test.ts` + * pins the same round trip with the note "verified". `parseFilterAST` lowers + * `['$or', '=', [...]]` to `{ $or: [...] }` — the FilterCondition the author + * wrote — because a `[field, '=', value]` node becomes `{ [field]: value }` and + * `$or` is a legal FilterCondition key. So the combinator DID reach the wire and + * WAS interpreted correctly there. {@link WIRE_UNMOVED} pins that it still is. + * + * The real defect is one door further in, and it is silent: `['$or', '=', [...]]` + * is a well-formed COMPARISON node, so every AST evaluator in this repo reads + * `$or` as a field name. `ValueDataSource`'s matcher looks up `record['$or']`, + * finds nothing, and excludes every row — no error, no console line, an empty + * list where the author asked for a union. That is the failure this file fixes + * in place, and {@link PRE_FIX_OR_NODE} is the control that it really did. + * + * ## `$not` is refused, and that is an open contract question + * + * The ObjectQL AST has no negation: `FILTER_ARRAY_LOGIC_KEYWORDS` is + * `['and', 'or']`. Rewriting the negation inward is not available either — + * `startswith`, `endswith`, `between` and `icontains` have no negated + * counterpart in `VALID_AST_OPERATORS`, so a De Morgan lowering would be + * silently partial, and `$not` is NULL-safe by ruling (objectstack#5146), which + * a partial rewrite would quietly drop. So `$not` is refused with an accurate + * message instead of translated. It threw before this card too — with a message + * naming a nonsense operator — so this changes the diagnostic, not the verdict; + * whether the AST should GAIN a negation is a spec decision, not this file's. + */ + +import { describe, it, expect } from 'vitest'; +import { isFilterAST, parseFilterAST } from '@objectstack/spec/data'; +import { convertFiltersToAST, mergeFilterNodes } from '../filter-converter'; +import { ValueDataSource } from '../../adapters/ValueDataSource'; + +// --------------------------------------------------------------------------- +// Fixture — one row per branch of the combinators under test +// --------------------------------------------------------------------------- + +const ROWS = [ + { id: 'open-active', status: 'open', is_active: true, age: 30 }, + { id: 'blocked-idle', status: 'blocked', is_active: false, age: 20 }, + { id: 'done-active', status: 'done', is_active: true, age: 40 }, + { id: 'null-status', status: null, is_active: null, age: null }, + { id: 'no-status-key' }, +]; + +/** What "no filter reached the evaluator" looks like. Never an expected answer. */ +const ALL_IDS = ['open-active', 'blocked-idle', 'done-active', 'null-status', 'no-status-key']; + +async function selectedIds(filter: unknown): Promise { + const ds = new ValueDataSource({ items: ROWS }); + const result = await ds.find('tasks', { $filter: filter as any }); + return result.data.map((r: any) => r.id as string); +} + +/** The authored filter of the card: a two-branch union. */ +const AUTHORED_OR = { $or: [{ status: 'open' }, { status: 'blocked' }] }; + +/** Its emission BEFORE this card — a comparison node on a field named `$or`. */ +const PRE_FIX_OR_NODE = ['$or', '=', [{ status: 'open' }, { status: 'blocked' }]]; + +// --------------------------------------------------------------------------- +// 1. The lowering — the node that reaches the wire +// --------------------------------------------------------------------------- + +describe('objectui#6948 — the lowering', () => { + it('lowers $or to an AST group node, children lowered recursively', () => { + expect(convertFiltersToAST(AUTHORED_OR)).toEqual([ + 'or', + ['status', '=', 'open'], + ['status', '=', 'blocked'], + ]); + }); + + it('lowers $and the same way, and lowers operator children too', () => { + expect(convertFiltersToAST({ $and: [{ status: 'open' }, { age: { $gte: 18 } }] })).toEqual([ + 'and', + ['status', '=', 'open'], + ['age', '>=', 18], + ]); + }); + + it('nests, so a combinator inside a combinator survives as a group', () => { + expect( + convertFiltersToAST({ + $and: [{ $or: [{ status: 'open' }, { status: 'done' }] }, { is_active: true }], + }), + ).toEqual([ + 'and', + ['or', ['status', '=', 'open'], ['status', '=', 'done']], + ['is_active', '=', true], + ]); + }); + + it('AND-combines a combinator with the sibling fields of its own object', () => { + expect(convertFiltersToAST({ is_active: true, ...AUTHORED_OR })).toEqual([ + 'and', + ['is_active', '=', true], + ['or', ['status', '=', 'open'], ['status', '=', 'blocked']], + ]); + }); + + it('survives mergeFilterNodes, so a parent scope still NARROWS it', () => { + // The related-list / line-items shape: a parent scope AND the list's own + // filter. The union must stay one child of the `and`, never spread into it. + expect(mergeFilterNodes({ task_version: 'tv-1' }, AUTHORED_OR)).toEqual([ + 'and', + ['task_version', '=', 'tv-1'], + ['or', ['status', '=', 'open'], ['status', '=', 'blocked']], + ]); + }); +}); + +// --------------------------------------------------------------------------- +// 2. The wire door — the spec's own predicate and lowering +// --------------------------------------------------------------------------- + +/** + * The FilterCondition the server sees. Identical before and after this card — + * the pre-fix leaf round-tripped correctly, which is why the defect was invisible + * from the wire and why the changeset argues its level from the EVALUATOR side. + */ +const WIRE_UNMOVED = { $or: [{ status: 'open' }, { status: 'blocked' }] }; + +describe('objectui#6948 — what the wire receives', () => { + it('emits a node the spec accepts AS A GROUP, which the old leaf was not', () => { + const node = convertFiltersToAST(AUTHORED_OR); + expect(isFilterAST(node)).toBe(true); + // Control that fires: the pre-fix node also passed `isFilterAST` — as a + // COMPARISON. Passing that gate was never the property in question, which + // is why this file does not stop here. + expect(isFilterAST(PRE_FIX_OR_NODE)).toBe(true); + expect(node).not.toEqual(PRE_FIX_OR_NODE); + }); + + it('lowers to the FilterCondition the author wrote', () => { + expect(parseFilterAST(convertFiltersToAST(AUTHORED_OR))).toEqual(WIRE_UNMOVED); + }); + + it('does not move the wire: the old leaf lowered to the SAME condition', () => { + expect(parseFilterAST(PRE_FIX_OR_NODE)).toEqual(WIRE_UNMOVED); + expect(parseFilterAST(convertFiltersToAST(AUTHORED_OR))).toEqual(parseFilterAST(PRE_FIX_OR_NODE)); + }); + + it('keeps the parent scope AROUND the union once merged', () => { + expect(parseFilterAST(mergeFilterNodes({ task_version: 'tv-1' }, AUTHORED_OR) as any)).toEqual({ + $and: [{ task_version: 'tv-1' }, WIRE_UNMOVED], + }); + }); +}); + +// --------------------------------------------------------------------------- +// 3. Row sets — both directions, one fixture +// --------------------------------------------------------------------------- + +describe('objectui#6948 — row sets through ValueDataSource', () => { + it('the pre-fix node selected NOTHING — the control this repair is measured against', async () => { + // Silently: `matchesComparisonNode` reads `$or` as a field, `record['$or']` + // is undefined, no row equals the array. No throw, no console line. + expect(await selectedIds(PRE_FIX_OR_NODE)).toEqual([]); + }); + + it('INCLUDES both branches of the union — not empty', async () => { + expect(await selectedIds(convertFiltersToAST(AUTHORED_OR))).toEqual([ + 'open-active', + 'blocked-idle', + ]); + }); + + it('EXCLUDES the rows outside the union — not everything', async () => { + const kept = await selectedIds(convertFiltersToAST(AUTHORED_OR)); + for (const excluded of ['done-active', 'null-status', 'no-status-key']) { + expect(kept).not.toContain(excluded); + } + }); + + it('so the answer is neither of the two failure shapes', async () => { + const kept = await selectedIds(convertFiltersToAST(AUTHORED_OR)); + // A converter that emits nothing usable leaves `$filter` unread: every row. + expect(kept).not.toEqual(ALL_IDS); + // A converter that emits a node no evaluator reads: no row. That is the bug. + expect(kept).not.toEqual([]); + // And the fixture is not trivially the answer either way. + expect(ALL_IDS.length).toBeGreaterThan(kept.length); + expect(await selectedIds(undefined)).toEqual(ALL_IDS); + }); + + it('$and intersects, both directions', async () => { + const node = convertFiltersToAST({ $and: [{ status: 'open' }, { is_active: true }] }); + expect(await selectedIds(node)).toEqual(['open-active']); + expect(await selectedIds(node)).not.toEqual(ALL_IDS); + }); + + it('a parent scope still narrows a union it wraps', async () => { + const scoped = mergeFilterNodes({ is_active: true }, AUTHORED_OR); + // `blocked-idle` is in the union but fails the scope; `done-active` passes + // the scope but is outside the union. Both directions in one assertion. + expect(await selectedIds(scoped)).toEqual(['open-active']); + }); + + it('nested combinators evaluate as written', async () => { + const node = convertFiltersToAST({ + $and: [{ $or: [{ status: 'open' }, { status: 'done' }] }, { is_active: true }], + }); + expect(await selectedIds(node)).toEqual(['open-active', 'done-active']); + }); +}); + +// --------------------------------------------------------------------------- +// 4. The boolean identities (#5322) — the boundary of the new branch +// --------------------------------------------------------------------------- + +describe('objectui#6948 — empty and vacuous combinators', () => { + it('drops `$and: []` — the TRUE identity constrains nothing', async () => { + // NOT `['and']`: measured, `isFilterAST(['and'])` is false and + // `parseFilterAST(['and'])` is `undefined`, i.e. NO filter — every row. A + // TRUE conjunct must disappear, never become an empty group. + expect(isFilterAST(['and'])).toBe(false); + expect(parseFilterAST(['and'] as any)).toBeUndefined(); + expect(convertFiltersToAST({ $and: [], status: 'open' })).toEqual(['status', '=', 'open']); + expect(await selectedIds(convertFiltersToAST({ $and: [], status: 'open' }))).toEqual([ + 'open-active', + ]); + }); + + it('keeps the pre-existing leaf for `$or: []` — FALSE is not expressible', async () => { + // The OR identity is FALSE (#5322) and the AST has no contradiction literal. + // The leaf answers FALSE at BOTH consumers, which no group node does. + const node = convertFiltersToAST({ $or: [] }); + expect(node).toEqual(['$or', '=', []]); + expect(parseFilterAST(node)).toEqual({ $or: [] }); + expect(await selectedIds(node)).toEqual([]); + }); + + it('lets a `{}` disjunct absorb its `$or`, and drops it from an `$and`', () => { + // #5322: `{}` is TRUE, so it absorbs an OR and vanishes from an AND. The + // absorbed OR must not leave an object in AST child position — that makes + // `isFilterAST` false for the whole filter. + expect(convertFiltersToAST({ $and: [{}, { status: 'open' }] })).toEqual([ + 'and', + ['status', '=', 'open'], + ]); + const absorbed = convertFiltersToAST({ $or: [{}, { status: 'open' }], is_active: true }); + expect(absorbed).toEqual(['is_active', '=', true]); + }); +}); + +// --------------------------------------------------------------------------- +// 5. Refusals — envelope, not a bare throw +// --------------------------------------------------------------------------- + +/** Assert the data-API refusal envelope (ADR-0112 / objectui#3066), not just that it threw. */ +function expectRefusal(run: () => unknown, messageMatch: RegExp): void { + expect(run).toThrow(); + try { + run(); + } catch (e: any) { + expect(e.code).toBe('INVALID_FILTER'); + expect(e.httpStatus).toBe(400); + expect(String(e.message)).toMatch(messageMatch); + } +} + +describe('objectui#6948 — refusals', () => { + it('refuses $not, naming the missing AST negation rather than a nonsense operator', () => { + // Before this card the message read "Unknown filter operator 'status' for + // field '$not'" — the author's own nested field name reported as an + // operator. It threw then and it throws now; only the diagnostic moved. + expectRefusal( + () => convertFiltersToAST({ $not: { status: 'done' } }), + /'\$not' filter combinator cannot be lowered/, + ); + expect(() => convertFiltersToAST({ $not: { status: 'done' } })).not.toThrow( + /Unknown filter operator 'status'/, + ); + }); + + it('refuses $not nested inside a combinator too', () => { + expectRefusal( + () => convertFiltersToAST({ $or: [{ status: 'open' }, { $not: { status: 'done' } }] }), + /'\$not' filter combinator cannot be lowered/, + ); + }); + + it('refuses a combinator whose value is not an array', () => { + expectRefusal( + () => convertFiltersToAST({ $or: { status: 'open' } as any }), + /'\$or' filter combinator takes an ARRAY/, + ); + }); + + it('refuses a non-object member', () => { + expectRefusal( + () => convertFiltersToAST({ $or: ['open' as any] }), + /Every member of '\$or' must be a filter condition OBJECT/, + ); + }); + + it('runs the unknown-operator guard on combinator children, which used to escape it', () => { + // Pre-fix the whole array travelled to the wire verbatim inside the leaf's + // value slot, so a typo inside a `$or` branch was never checked here. + expectRefusal( + () => convertFiltersToAST({ $or: [{ status: { $bogus: 1 } as any }] }), + /Unknown filter operator '\$bogus'/, + ); + expectRefusal( + () => convertFiltersToAST({ $or: [{ name: { $regex: 'a.c' } as any }] }), + /\$regex/, + ); + }); +}); diff --git a/packages/core/src/utils/filter-converter.ts b/packages/core/src/utils/filter-converter.ts index 2f69f27c13..5f99be9529 100644 --- a/packages/core/src/utils/filter-converter.ts +++ b/packages/core/src/utils/filter-converter.ts @@ -80,6 +80,115 @@ export function convertOperatorToAST(operator: string): string | null { return operatorMap[operator] || null; } +/** + * The `FilterCondition` combinators this lowering can carry, and the AST + * keyword each becomes. + * + * The vocabulary is the spec's, not a second list: `FILTER_ARRAY_LOGIC_KEYWORDS` + * (`@objectstack/spec/data`) is `['and', 'or']` — measured — and those two are + * exactly the heads `isFilterAST` opens a group on. `$not` is absent from it, + * which is why it has no row here and is refused below rather than translated. + */ +const AST_LOGIC_KEYWORD: Record = { + $and: 'and', + $or: 'or', +}; + +/** + * Lower ONE `$and` / `$or` group to an AST group node. + * + * Returns `undefined` when the group is the TRUE identity and therefore + * constrains nothing, so the caller drops it instead of emitting a childless + * `['and']` — which is NOT the same thing. Measured against the spec's own + * doors: `isFilterAST(['and'])` is `false` and `parseFilterAST(['and'])` is + * `undefined`, i.e. NO FILTER — every row. A combinator that reduces to "no + * constraint" must therefore disappear at THIS level; emitting an empty group + * would widen the result set, which is the one failure direction this file + * exists to avoid. + * + * `#5322` (maintainer ruling 2026-08-04, recorded on `FilterConditionSchema`) + * fixes the identities: `{ $and: [] }` is TRUE, `{ $or: [] }` is FALSE, and a + * `{}` disjunct is TRUE and ABSORBS its `$or`. TRUE is expressible here — it is + * the absence of a constraint. FALSE is not: the AST has no contradiction + * literal, so `{ $or: [] }` keeps the emission it already had (see + * {@link falseIdentityLeaf}). + */ +function lowerLogicalGroup( + field: string, + keyword: 'and' | 'or', + value: unknown, +): FilterNode | undefined { + if (!Array.isArray(value)) { + throw new FilterOperatorError( + `[ObjectUI] The '${field}' filter combinator takes an ARRAY of conditions. ` + + `Received ${typeof value === 'object' ? 'an object' : typeof value}: ` + + `${JSON.stringify(value)}. Spec: FilterCondition declares ` + + `'${field}?: FilterCondition[]' (data/filter.zod.ts).` + ); + } + + if (value.length === 0) { + return keyword === 'and' ? undefined : falseIdentityLeaf(field, value); + } + + const children: FilterNode[] = []; + for (const child of value) { + if (child === null || typeof child !== 'object' || Array.isArray(child)) { + throw new FilterOperatorError( + `[ObjectUI] Every member of '${field}' must be a filter condition OBJECT. ` + + `Received ${JSON.stringify(child)}. Spec: FilterCondition declares ` + + `'${field}?: FilterCondition[]' (data/filter.zod.ts).` + ); + } + const lowered = convertFiltersToAST(child as Record); + if (!Array.isArray(lowered)) { + // `convertFiltersToAST` hands back the ORIGINAL OBJECT when the child + // produced no conditions — a `{}` disjunct, or one holding only + // null/undefined values. That child is the TRUE identity (#5322), so it + // absorbs an `$or` outright and drops out of an `$and`. It must not be + // pushed as a child either way: an object in AST child position makes + // `isFilterAST` false (measured), and the wire face answers `400 + // INVALID_FILTER` for the whole filter. + if (keyword === 'or') return undefined; + continue; + } + children.push(lowered as FilterNode); + } + + // Every conjunct reduced to TRUE, so the `$and` constrains nothing. + if (children.length === 0) return undefined; + + // A one-child group is emitted as a group, not unwrapped. `['or', node]` is + // accepted by `isFilterAST` (length >= 2 = keyword + one condition) and + // `parseFilterAST` reduces it to the child, so the extra hop costs nothing + // and keeps this function's output shape a function of the INPUT shape. + return [keyword, ...children] as FilterNode; +} + +/** + * `{ $or: [] }` — FALSE, the OR identity (#5322) — as the leaf this file has + * always emitted for it. + * + * Deliberately unchanged, and deliberately not a group node. Three measurements + * against `@objectstack/spec` 17.3.0 and this repo's own evaluator decide it: + * + * - `parseFilterAST(['$or', '=', []])` is `{ $or: [] }` — the FilterCondition + * the author wrote, which every backend reduces to zero rows. Correct. + * - `ValueDataSource`'s matcher reads it as a comparison on a field named + * `$or`, which no record has, so it excludes every row. Also correct, and + * the same answer. + * - `['or']` — the "obvious" empty group — is `isFilterAST` FALSE and + * `parseFilterAST` `undefined`: no filter at all, i.e. EVERY row. That is + * the widening direction, on a filter whose whole purpose is to hide rows + * (#5134), so it is the one shape that must not be emitted. + * + * A leaf naming `$or` as a field is not a shape to be proud of; it is the shape + * that answers FALSE at both consumers, which the alternatives do not. + */ +function falseIdentityLeaf(field: string, value: unknown[]): FilterNode { + return [field, '=', value] as FilterNode; +} + /** * Convert object-based filters to ObjectStack FilterNode AST format. * Converts MongoDB-like operators to ObjectStack filter expressions. @@ -101,15 +210,49 @@ export function convertOperatorToAST(operator: string): string | null { * // Multiple conditions * convertFiltersToAST({ age: { $gte: 18, $lte: 65 }, status: 'active' }) * // => ['and', ['age', '>=', 18], ['age', '<=', 65], ['status', '=', 'active']] - * - * @throws {Error} If an unknown operator is encountered + * + * @example + * // Logical combinators (objectui#6948) — children lower recursively + * convertFiltersToAST({ $or: [{ status: 'open' }, { status: 'blocked' }] }) + * // => ['or', ['status', '=', 'open'], ['status', '=', 'blocked']] + * + * @throws {FilterOperatorError} If an unknown operator is encountered, or if + * `$not` is used — see the `$not` arm for why the AST cannot carry it. */ export function convertFiltersToAST(filter: Record): FilterNode | Record { const conditions: FilterNode[] = []; for (const [field, value] of Object.entries(filter)) { if (value === null || value === undefined) continue; - + + // Logical combinators are read BEFORE the field/operator machinery below, + // because they are not fields and their value is not an operator map. + // Without this arm `$and` / `$or` reached the simple-equality branch (their + // value is an array, so the operator loop was skipped) and became a leaf + // naming a field literally called `$and` / `$or`, while `$not` entered the + // operator loop with its OWN nested object's keys read as operator names. + const logicKeyword = AST_LOGIC_KEYWORD[field]; + if (logicKeyword) { + const group = lowerLogicalGroup(field, logicKeyword, value); + if (group !== undefined) conditions.push(group); + continue; + } + + if (field === '$not') { + throw new FilterOperatorError( + `[ObjectUI] The '$not' filter combinator cannot be lowered to the ObjectQL ` + + `filter AST. '@objectstack/spec' declares it on FilterCondition, but the AST ` + + `this layer emits has no negation keyword (FILTER_ARRAY_LOGIC_KEYWORDS is ` + + `['and', 'or']), and rewriting the negation inward is not available either — ` + + `'startswith', 'endswith', 'between' and 'icontains' have no negated ` + + `counterpart in VALID_AST_OPERATORS, so the rewrite would be silently ` + + `partial. Express the negation with a negated operator instead ($ne, $nin, ` + + `$notContains); note those follow each operator's own answer for a missing ` + + `value rather than $not's NULL-safe rule (objectstack#5146). ` + + `Value: ${JSON.stringify(value)}.` + ); + } + // Check if value is a complex operator object if (typeof value === 'object' && !Array.isArray(value)) { // Handle operator-based filters diff --git a/packages/data-objectstack/src/filter-entry-translation.test.ts b/packages/data-objectstack/src/filter-entry-translation.test.ts index cd43be4a80..22ba85a616 100644 --- a/packages/data-objectstack/src/filter-entry-translation.test.ts +++ b/packages/data-objectstack/src/filter-entry-translation.test.ts @@ -22,7 +22,7 @@ */ import { describe, it, expect, beforeEach, vi } from 'vitest'; -import { isFilterAST } from '@objectstack/spec/data'; +import { isFilterAST, parseFilterAST } from '@objectstack/spec/data'; import { ObjectStackAdapter, clearSharedDiscoveryCache, isMalformedFilterError } from './index'; function makeAdapter() { @@ -309,12 +309,37 @@ describe('an OBJECT filter reaches the same predicate on both routes', () => { ); bothRoutes( - 'leaves a top-level Mongo logical node alone', - // `mergeFilters` (dashboard scope filters) produces this. It survives as a - // `['$and', '=', [...]]` comparison that `parseFilterAST` reads back as a - // real `$and` — verified, and the reason this is NOT rewritten here. + 'lowers a top-level Mongo logical node to an AST group', + // `mergeFilters` (dashboard scope filters) produces this, and it used to go + // out as a `['$and', '=', [...]]` comparison — a leaf naming a field + // literally called `$and`. The note this case carried was accurate about the + // WIRE and that half still holds: `parseFilterAST` reads that leaf back as a + // real `$and`, so nothing was ever refused there, and the assertion below + // pins that the lowered form reaches the same FilterCondition. + // + // What the note did not cover is the consumer one door in. The leaf is a + // well-formed COMPARISON node, so every AST evaluator in this repo — the + // matcher in `@object-ui/core`'s `ValueDataSource` above all — reads `$and` + // as a FIELD NAME, finds no such key on any record, and returns an EMPTY + // list with no error. objectui#6948 taught `convertFiltersToAST` the group + // spelling the spec's own `FILTER_ARRAY_LOGIC_KEYWORDS` declares, so the + // node is now executable as well as parseable. { $and: [{ a: 1 }, { b: 2 }] }, - (wire) => expect(wire).toEqual(['$and', '=', [{ a: 1 }, { b: 2 }]]), + (wire) => expect(wire).toEqual(['and', ['a', '=', 1], ['b', '=', 2]]), + ); + + bothRoutes( + 'and the wire CONDITION is unchanged by that lowering', + // The half of the old note that still holds, kept as an assertion rather + // than a sentence: both spellings lower to the same `FilterCondition`, so + // the server's answer to this filter did not move. + { $and: [{ a: 1 }, { b: 2 }] }, + (wire) => { + expect(parseFilterAST(wire as any)).toEqual({ $and: [{ a: 1 }, { b: 2 }] }); + expect(parseFilterAST(wire as any)).toEqual( + parseFilterAST(['$and', '=', [{ a: 1 }, { b: 2 }]] as any), + ); + }, ); for (const route of ['plain', 'expand'] as const) {