fix(list,data): bridge every spec view operator onto the filter AST (#2901)

packages/data-objectstack/package.json

@@ -35,6 +35,7 @@
     "@objectstack/client": "^17.0.0-rc.0"
   },
   "devDependencies": {
+    "@objectstack/spec": "^17.0.0-rc.0",
     "tsup": "^8.5.1",
     "typescript": "^6.0.3",
     "vitest": "^4.1.10"

packages/data-objectstack/src/filter-operator-ast-parity.test.ts

@@ -0,0 +1,71 @@
+/**
+ * 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.
+ */
+
+/**
+ * Adapter operator table → filter-AST parity (#2901, objectstack#3948).
+ *
+ * `FILTER_OPERATOR_ALIASES` is the last translation a filter passes through
+ * before it goes on the wire, and `normalizeFilterOperator` ends in `?? op` —
+ * an unmapped operator is emitted verbatim. The server then rejects the shape
+ * at `isFilterAST()`, passes the array through unconverted, and driver-sql
+ * skips it entirely: **no WHERE clause, no error, every row returned.**
+ *
+ * So a missing row in this table is not a validation failure, it is an
+ * unfiltered query. `before`/`after` — canonical members of the spec's
+ * `VIEW_FILTER_OPERATORS` — were missing, which is exactly how a stored
+ * "close_date before X" view came back unfiltered.
+ *
+ * These tests pin the table against the spec vocabularies in both directions.
+ */
+import { describe, it, expect } from 'vitest';
+import { VALID_AST_OPERATORS } from '@objectstack/spec/data';
+import { VIEW_FILTER_OPERATORS } from '@objectstack/spec/ui';
+import { FILTER_OPERATOR_ALIASES } from './index';
+
+/**
+ * View operators this adapter is not the bridge for — the value-shape ones the
+ * view layer resolves to a null comparison before an operator is ever emitted.
+ */
+const NOT_THIS_ADAPTERS_JOB = new Set(['is_empty', 'is_not_empty']);
+
+describe('FILTER_OPERATOR_ALIASES lands inside the spec AST vocabulary', () => {
+  it('reads both vocabularies from the spec', () => {
+    expect(VIEW_FILTER_OPERATORS.length).toBeGreaterThan(0);
+    expect(VALID_AST_OPERATORS.size).toBeGreaterThan(0);
+  });
+
+  it('every alias target is an operator the AST gate accepts', () => {
+    const bad = Object.entries(FILTER_OPERATOR_ALIASES)
+      .filter(([, target]) => !VALID_AST_OPERATORS.has(String(target).toLowerCase()))
+      .map(([alias, target]) => `${alias} → ${target}`);
+    expect(
+      bad,
+      'these aliases translate to operators VALID_AST_OPERATORS rejects, so the '
+        + 'server drops the filter silently instead of erroring',
+    ).toEqual([]);
+  });
+
+  it('covers every canonical view operator the spec defines', () => {
+    const uncovered = VIEW_FILTER_OPERATORS
+      .filter((op) => !NOT_THIS_ADAPTERS_JOB.has(op))
+      .filter((op) => {
+        const target = FILTER_OPERATOR_ALIASES[op] ?? op;
+        return !VALID_AST_OPERATORS.has(String(target).toLowerCase());
+      });
+    expect(
+      uncovered,
+      'an author can declare these on a ViewFilterRule and the spec validates them, '
+        + 'but they reach the wire unmapped and the filter is silently dropped',
+    ).toEqual([]);
+  });
+
+  it('maps the date comparisons that regressed', () => {
+    expect(FILTER_OPERATOR_ALIASES.before).toBe('<');
+    expect(FILTER_OPERATOR_ALIASES.after).toBe('>');
+  });
+});

packages/data-objectstack/src/index.ts

@@ -47,8 +47,15 @@ import {
  * (e.g. `lead.view.ts`) to the canonical operator symbols expected by the
  * ObjectStack server's filter AST. Unknown operators fall through unchanged
  * so existing AST-style entries keep working.
+ *
+ * Every VALUE here must be a member of the spec's `VALID_AST_OPERATORS`
+ * (`@objectstack/spec/data`) — that set gates `isFilterAST()`, and a filter it
+ * rejects is not converted, not validated, and then silently DROPPED by
+ * driver-sql (objectstack#3948). Pinned by `filter-operator-ast-parity.test.ts`.
+ *
+ * Exported for that test. @internal
  */
-const FILTER_OPERATOR_ALIASES: Record<string, string> = {
+export const FILTER_OPERATOR_ALIASES: Record<string, string> = {
   equals: '=',
   eq: '=',
   '==': '=',
@@ -85,6 +92,15 @@ const FILTER_OPERATOR_ALIASES: Record<string, string> = {
   isnull: 'isnull',
   is_not_null: 'isnotnull',
   isnotnull: 'isnotnull',
+  // Date comparisons. `before`/`after` are CANONICAL members of the spec's
+  // `VIEW_FILTER_OPERATORS` (ui/view.zod.ts), so a stored view legitimately
+  // carries them — but they are absent from `VALID_AST_OPERATORS`
+  // (data/filter.zod.ts), which gates `isFilterAST()`. Without these two
+  // entries they reached the wire unchanged, the server's AST gate rejected
+  // the shape, and driver-sql skipped the filter ENTIRELY — an unfiltered
+  // result set with no error anywhere. objectstack#3948.
+  before: '<',
+  after: '>',
 };
 
 function normalizeFilterOperator(op: unknown): string | null {

packages/plugin-list/package.json

@@ -53,6 +53,7 @@
     "@object-ui/mobile": "workspace:*",
     "@object-ui/react": "workspace:*",
     "@object-ui/types": "workspace:*",
+    "@objectstack/spec": "^17.0.0-rc.0",
     "@types/react": "19.2.17",
     "@types/react-dom": "19.2.3",
     "@vitejs/plugin-react": "^6.0.4",

packages/plugin-list/src/ListView.tsx

@@ -59,27 +59,57 @@ export interface ListViewProps {
 // Helper to convert FilterBuilder group to ObjectStack AST.
 // Accepts both the FilterBuilder vocabulary (camelCase) and the
 // @objectstack/spec ViewFilterRule vocabulary (snake_case).
-function mapOperator(op: string) {
-  switch (op) {
+/**
+ * Filter-builder / view operator → filter-AST operator.
+ *
+ * Every value returned must be a member of the spec's `VALID_AST_OPERATORS`
+ * (`@objectstack/spec/data`). That set gates `isFilterAST()`, and a filter it
+ * rejects is passed through unconverted and then silently DROPPED by driver-sql
+ * — an unfiltered result set with no error (objectstack#3948). Pinned by
+ * `filter-operator-ast-parity.test.ts`.
+ *
+ * Exported for that test. @internal
+ */
+export function mapOperator(op: string) {
+  // The spec's alias table carries the same operator in up to four spellings
+  // (`not_equals`, `notEquals`, `notequals`, `ne`), and stored view metadata
+  // holds all of them — `saveMeta` persists the authored body verbatim, so the
+  // spec's own normalization never reaches the row. Matching them case- and
+  // underscore-insensitively collapses that whole class instead of enumerating
+  // it: a switch listing spellings by hand had already missed eight.
+  switch (op.toLowerCase().replace(/[_\s]/g, '')) {
     case 'equals': case 'eq': return '=';
-    case 'notEquals': case 'not_equals': case 'ne': case 'neq': return '!=';
+    case 'notequals': case 'ne': case 'neq': return '!=';
     case 'contains': return 'contains';
-    case 'notContains': case 'not_contains': case 'notcontains': return 'notcontains';
-    case 'startsWith': case 'starts_with': return 'startswith';
-    case 'greaterThan': case 'greater_than': case 'gt': return '>';
-    case 'greaterOrEqual': case 'greater_than_or_equal': case 'gte': return '>=';
-    case 'lessThan': case 'less_than': case 'lt': return '<';
-    case 'lessOrEqual': case 'less_than_or_equal': case 'lte': return '<=';
+    case 'notcontains': return 'notcontains';
+    case 'startswith': return 'startswith';
+    case 'endswith': return 'endswith';
+    case 'greaterthan': case 'gt': return '>';
+    case 'greaterorequal': case 'greaterthanorequal': case 'gte': return '>=';
+    case 'lessthan': case 'lt': return '<';
+    case 'lessorequal': case 'lessthanorequal': case 'lte': return '<=';
     case 'in': return 'in';
-    case 'notIn': case 'not_in': case 'nin': return 'not in';
+    // `nin`, not `'not in'`: the spaced spelling is in no spec vocabulary, so
+    // `isFilterAST()` rejected it and driver-sql skipped the filter entirely.
+    // The array case never reached the wire (normalizeFilterCondition expands
+    // it below), but a non-array value escaped as an unfiltered query.
+    case 'notin': case 'nin': return 'nin';
+    // Canonical `VIEW_FILTER_OPERATORS` members with no AST counterpart; the
+    // gap here is what returned unfiltered rows for a stored date filter.
     case 'before': return '<';
     case 'after': return '>';
+    case 'between': return 'between';
+    case 'isnull': return 'isnull';
+    case 'isnotnull': return 'isnotnull';
     default: return op;
   }
 }
 
+/** Every not-in spelling this normalizer expands. See the note at the call site. */
+const NOT_IN_SPELLINGS = new Set(['nin', 'not_in', 'notIn', 'notin', 'not in']);
+
 /**
- * Normalize a single filter condition: convert `in`/`not in` operators
+ * Normalize a single filter condition: convert `in`/not-in operators
  * into backend-compatible `or`/`and` of equality conditions.
  * E.g., ['status', 'in', ['a','b']] → ['or', ['status','=','a'], ['status','=','b']]
  */
@@ -101,7 +131,10 @@ export function normalizeFilterCondition(condition: any[]): any[] {
     return ['or', ...value.map((v: any) => [field, '=', v])];
   }
 
-  if (op === 'not in' && Array.isArray(value)) {
+  // `nin` is what mapOperator now emits; the rest are spellings an external
+  // caller may still pass, since this function is part of plugin-list's public
+  // surface. Accepting all of them keeps the expansion working either way.
+  if (NOT_IN_SPELLINGS.has(op) && Array.isArray(value)) {
     if (value.length === 0) return [];
     if (value.length === 1) return [field, '!=', value[0]];
     return ['and', ...value.map((v: any) => [field, '!=', v])];

packages/plugin-list/src/__tests__/filter-operator-ast-parity.test.ts

@@ -0,0 +1,98 @@
+/**
+ * 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.
+ */
+
+/**
+ * View operator → filter-AST operator parity (#2901, objectstack#3948).
+ *
+ * The spec ships two operator vocabularies that must agree at this boundary:
+ *
+ *   - `VIEW_FILTER_OPERATORS` (`ui/view.zod.ts`) — what an author may declare on
+ *     a `ViewFilterRule`, and what `ViewFilterRuleSchema` validates against.
+ *   - `VALID_AST_OPERATORS` (`data/filter.zod.ts`) — what gates `isFilterAST()`
+ *     on the server, deciding whether a filter is parsed into a query at all.
+ *
+ * They do NOT overlap: 8 of the 19 canonical view operators are absent from the
+ * AST set. `mapOperator` is what bridges them, and a gap in it is invisible —
+ * `isFilterAST()` returns false, the protocol passes the array through
+ * unconverted, and driver-sql then skips it entirely. **No WHERE clause, no
+ * error, every row returned.** That is how `before`/`after` shipped broken: they
+ * are canonical view operators with no entry in the bridge.
+ *
+ * These tests assert the bridge is total, so the next operator the spec adds to
+ * the view vocabulary fails here instead of silently returning unfiltered rows.
+ */
+import { describe, it, expect } from 'vitest';
+import { VALID_AST_OPERATORS, isFilterAST } from '@objectstack/spec/data';
+import { VIEW_FILTER_OPERATORS, VIEW_FILTER_OPERATOR_ALIASES } from '@objectstack/spec/ui';
+import { mapOperator, normalizeFilterCondition } from '../ListView';
+
+/** Operators this bridge deliberately resolves without reaching the AST gate. */
+const HANDLED_BEFORE_MAPPING = new Set([
+  // convertFilterGroupToAST rewrites these to `[field, '=' | '!=', null]`
+  // before mapOperator is consulted, so they never need an AST spelling.
+  'is_empty', 'is_not_empty',
+]);
+
+describe('mapOperator bridges the spec view vocabulary onto the AST vocabulary', () => {
+  it('reads both vocabularies from the spec', () => {
+    // Guards every assertion below against silently passing on an empty list.
+    expect(VIEW_FILTER_OPERATORS.length).toBeGreaterThan(0);
+    expect(VALID_AST_OPERATORS.size).toBeGreaterThan(0);
+  });
+
+  const bridged = VIEW_FILTER_OPERATORS.filter((op) => !HANDLED_BEFORE_MAPPING.has(op));
+
+  it.each(bridged)('%s maps to an AST-valid operator', (viewOp) => {
+    const mapped = mapOperator(viewOp);
+    expect(
+      VALID_AST_OPERATORS.has(String(mapped).toLowerCase()),
+      `mapOperator('${viewOp}') → '${mapped}', which VALID_AST_OPERATORS rejects. `
+        + 'isFilterAST() will return false and the filter will be silently dropped '
+        + 'server-side — an unfiltered result set, not an error.',
+    ).toBe(true);
+  });
+
+  it.each(bridged)('a single %s condition survives the isFilterAST gate', (viewOp) => {
+    // The reachable shape: one condition, AND logic, emitted as a bare triple.
+    // This is exactly what silently full-scanned before the fix.
+    const value = viewOp === 'in' || viewOp === 'not_in' ? ['a', 'b'] : 'x';
+    const triple = normalizeFilterCondition(['some_field', mapOperator(viewOp), value]);
+    expect(
+      isFilterAST(triple),
+      `a '${viewOp}' filter produced ${JSON.stringify(triple)}, which isFilterAST() rejects`,
+    ).toBe(true);
+  });
+
+  it('also bridges every legacy alias the spec still folds', () => {
+    // Stored view metadata carries these: saveMeta persists the authored body
+    // verbatim, so the spec's z.preprocess normalization never reaches the row.
+    const unbridged = Object.keys(VIEW_FILTER_OPERATOR_ALIASES)
+      .filter((alias) => !HANDLED_BEFORE_MAPPING.has(VIEW_FILTER_OPERATOR_ALIASES[alias]))
+      .filter((alias) => !VALID_AST_OPERATORS.has(String(mapOperator(alias)).toLowerCase()));
+    expect(
+      unbridged,
+      'these legacy spellings exist in stored view metadata and map to no AST operator',
+    ).toEqual([]);
+  });
+
+  it('emits `nin`, never the spaced `not in`, which no spec vocabulary defines', () => {
+    for (const spelling of ['notIn', 'not_in', 'nin']) {
+      expect(mapOperator(spelling)).toBe('nin');
+    }
+  });
+
+  it('still expands a not-in array into an AND of inequalities', () => {
+    // Regression: the expansion keyed on the old spaced spelling.
+    expect(normalizeFilterCondition(['stage', 'nin', ['won', 'lost']]))
+      .toEqual(['and', ['stage', '!=', 'won'], ['stage', '!=', 'lost']]);
+    // …and keeps accepting the spellings an external caller may pass, since
+    // normalizeFilterCondition is part of plugin-list's public surface.
+    expect(normalizeFilterCondition(['stage', 'not in', ['won', 'lost']]))
+      .toEqual(['and', ['stage', '!=', 'won'], ['stage', '!=', 'lost']]);
+  });
+});