fix(view,list,core): a view's filter no longer disappears, or arrives as a predicate on columns that don't exist

.changeset/filter-sources-merge-without-losing-any.md

@@ -0,0 +1,57 @@
+---
+"@object-ui/core": patch
+"@object-ui/data-objectstack": patch
+"@object-ui/plugin-list": patch
+"@object-ui/plugin-view": patch
+---
+
+fix(view,list,core): a view's filter no longer disappears, or arrives as a predicate on columns that don't exist
+
+Sweeping the other `$filter` producers after #3078 turned up two live defects in
+`ObjectView`, which fetches its own data for calendar / kanban / gallery /
+timeline (grid delegates to `ObjectGrid`).
+
+**1. An object filter was dropped, and only for non-grid views.**
+`table.defaultFilters` is declared `Record<string, any>`, and the merge tested
+`baseFilter.length > 0` — `undefined > 0` for an object. So the filter vanished
+and the view returned **every record**. `ObjectGrid` assigns the same value
+straight to `params.$filter`, so one view definition filtered correctly as a
+grid and returned everything as a calendar.
+
+**2. Rule objects were spread into the `and`, not wrapped.**
+`['and', ...baseFilter, ...userFilter]` is only correct when the source is an
+array of AST nodes. `activeView.filter` is a spec `ViewFilterRule[]`, so
+spreading put bare rule objects where the AST expects nodes:
+
+```js
+isFilterAST(['and', {field:'stage',operator:'eq',value:'won'}, ['owner','=','me']])
+// false → 400 since objectstack#4121
+parseFilterAST(same)
+// {$and:[{field:'stage',operator:'eq',value:'won'}, {owner:'me'}]}
+```
+
+That second line is a predicate over three columns named `field`, `operator`
+and `value` — which don't exist. Reachable whenever a view with a filter meets a
+user filter value.
+
+New in `@object-ui/core`: `toFilterNode` normalizes one source (rule array / AST
+/ MongoDB object) and `mergeFilterNodes` combines sources as siblings under one
+`and`. `ObjectView` and `ListView.buildEffectiveFilter` both use them, so the
+three filter shapes are reconciled in one place instead of by hand at each
+renderer.
+
+`ObjectStackAdapter` also now translates a bare rule object sitting directly
+under a logical node — the chokepoint defence for any producer still emitting
+the spread shape. Only rule-*shaped* objects are touched; a child with no
+`field` is a genuine MongoDB condition and passes through untouched.
+
+**Correcting a comment shipped in #3078.** `buildEffectiveFilter` documented the
+dropped-object case as unreachable, "nothing in this repo produces one for a
+list view". That was wrong: `ObjectView` passes `mergedFilters` straight into
+that schema's `filter`, and its last fallback is `table.defaultFilters`. The
+case is now handled rather than explained away.
+
+Verified with 19 tests across the four packages; reverting each source file
+fails the ones that cover it. Emitted filters are asserted against the spec's
+own `isFilterAST` / `parseFilterAST`, including an executable pin on what the
+old spread shape produced.

packages/core/src/utils/__tests__/filter-source-merge.test.ts

@@ -0,0 +1,104 @@
+/**
+ * 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.
+ */
+
+/**
+ * Merging the filter sources a view can carry.
+ *
+ * Three shapes are in circulation and all are legitimate — a spec
+ * `ViewFilterRule[]`, an AST node, and a MongoDB-style object. Renderers merged
+ * them by hand and got two things wrong, both silent:
+ *
+ *   1. They tested `source.length > 0` before using it. That is `undefined > 0`
+ *      for an object, so a `table.defaultFilters` (declared `Record<string,
+ *      any>`) was DROPPED and the view returned every record.
+ *   2. They SPREAD a source into the `and` (`['and', ...rules]`). That is only
+ *      correct when the source is an array of nodes; for a `ViewFilterRule[]`
+ *      it puts bare rule objects where the AST expects nodes. `isFilterAST`
+ *      rejects that (a 400 since objectstack#4121) and `parseFilterAST` reads
+ *      the rule as a Mongo condition — filtering on columns literally named
+ *      `field`, `operator` and `value`.
+ *
+ * The emitted shape is asserted against the server's own `isFilterAST` rather
+ * than a restated literal wherever the result is an AST.
+ */
+
+import { describe, it, expect } from 'vitest';
+import { isFilterAST, parseFilterAST } from '@objectstack/spec/data';
+import { toFilterNode, mergeFilterNodes } from '../filter-converter';
+
+const RULES = [{ field: 'stage', operator: 'eq', value: 'won' }];
+const TUPLE = ['owner', '=', 'me'];
+
+describe('toFilterNode', () => {
+  it('passes a non-empty array source through unchanged', () => {
+    expect(toFilterNode(RULES)).toEqual(RULES);
+    expect(toFilterNode([TUPLE])).toEqual([TUPLE]);
+  });
+
+  it('converts a MongoDB-style object into an AST node', () => {
+    expect(toFilterNode({ status: 'active' })).toEqual(['status', '=', 'active']);
+  });
+
+  it('treats absent and empty sources as nothing', () => {
+    for (const empty of [undefined, null, [], {}, '', 0]) {
+      expect(toFilterNode(empty)).toBeUndefined();
+    }
+  });
+});
+
+describe('mergeFilterNodes', () => {
+  it('returns undefined when every source is empty', () => {
+    expect(mergeFilterNodes(undefined, [], {})).toBeUndefined();
+  });
+
+  it('returns a lone source as-is rather than wrapping it in a pointless and', () => {
+    expect(mergeFilterNodes([TUPLE], undefined)).toEqual([TUPLE]);
+  });
+
+  it('wraps each source as its own child — never spreads it', () => {
+    // The regression. Spreading would give ['and', {field…}, 'owner', '=', 'me'].
+    expect(mergeFilterNodes(RULES, TUPLE)).toEqual(['and', RULES, TUPLE]);
+  });
+
+  it('keeps an object source instead of dropping it', () => {
+    // `table.defaultFilters` is declared `Record<string, any>`; the old
+    // `.length > 0` guard read false and the whole filter disappeared.
+    expect(mergeFilterNodes({ status: 'active' }, TUPLE))
+      .toEqual(['and', ['status', '=', 'active'], TUPLE]);
+  });
+});
+
+describe('what reaches the server', () => {
+  /**
+   * `ViewFilterRule[]` is not itself AST — the adapter translates it on the way
+   * out — so `isFilterAST` is only the right oracle for the all-AST cases.
+   */
+  it('produces a node the server accepts when every source is AST', () => {
+    expect(isFilterAST(mergeFilterNodes([TUPLE], ['amount', '>', 1]))).toBe(true);
+    expect(isFilterAST(mergeFilterNodes({ status: 'active' }, ['amount', '>', 1]))).toBe(true);
+    expect(isFilterAST(mergeFilterNodes({ status: 'active' }))).toBe(true);
+  });
+
+  it('an object source survives the round trip to a real predicate', () => {
+    const merged = mergeFilterNodes({ status: 'active' }, ['amount', '>', 1]);
+    expect(parseFilterAST(merged)).toEqual({
+      $and: [{ status: 'active' }, { amount: { $gt: 1 } }],
+    });
+  });
+
+  it('the spread shape it replaces was NOT acceptable — pinning why', () => {
+    // What `['and', ...rules, ...tuples]` produced. Kept as executable evidence
+    // that the wrapping above is not a stylistic preference.
+    const spread = ['and', ...RULES, TUPLE];
+    expect(isFilterAST(spread)).toBe(false);
+    // Worse than rejected: read as a predicate over three columns that do not exist.
+    expect(parseFilterAST(spread)).toEqual({
+      $and: [{ field: 'stage', operator: 'eq', value: 'won' }, { owner: 'me' }],
+    });
+  });
+});

packages/core/src/utils/filter-converter.ts

@@ -152,3 +152,54 @@ export function convertFiltersToAST(filter: Record<string, any>): FilterNode | R
   // Multiple conditions: combine with 'and'
   return ['and', ...conditions];
 }
+
+/**
+ * Normalize ONE filter source into a single filter node.
+ *
+ * A "source" is whatever a view hands a renderer, and there are three shapes in
+ * circulation, all legitimate:
+ *
+ *   - `[{ field, operator, value }, ...]`  a spec `ViewFilterRule[]`
+ *   - `[['stage', '=', 'won'], ...]`       an AST node / legacy flat array
+ *   - `{ status: 'active' }`               a MongoDB-style object
+ *
+ * The third is the one that kept getting lost. Renderers tested `source.length
+ * > 0` before using it, which is `undefined > 0` for an object — so a
+ * `table.defaultFilters` (declared `Record<string, any>`) was DROPPED and the
+ * view returned every record. Silently: no error, just a wider answer.
+ *
+ * Returns `undefined` for an absent or empty source, so callers can skip
+ * `$filter` rather than sending an empty array.
+ */
+export function toFilterNode(source: unknown): FilterNode | Record<string, any> | undefined {
+  if (source === null || source === undefined) return undefined;
+  if (Array.isArray(source)) return source.length > 0 ? (source as FilterNode) : undefined;
+  if (typeof source !== 'object') return undefined;
+  const obj = source as Record<string, any>;
+  if (Object.keys(obj).length === 0) return undefined;
+  // MongoDB-style → AST, so it can sit beside the other shapes under one `and`.
+  return convertFiltersToAST(obj);
+}
+
+/**
+ * Combine filter sources under a single `and`, each as its OWN child.
+ *
+ * Wrapping rather than spreading, on purpose. `['and', ...rules]` looks
+ * equivalent and is not: spreading a `ViewFilterRule[]` puts bare rule OBJECTS
+ * where the AST expects nodes, and the server neither understands nor rejects
+ * that cleanly — `isFilterAST` says no (a 400 since objectstack#4121), while
+ * `parseFilterAST` reads the rule as a Mongo condition and filters on columns
+ * literally named `field` / `operator` / `value`. Spreading is only correct
+ * when the source happens to be an array of nodes, which is why it survived.
+ *
+ * Sources that normalize to nothing are skipped; one surviving source is
+ * returned as-is rather than wrapped in a pointless `and`.
+ */
+export function mergeFilterNodes(
+  ...sources: unknown[]
+): FilterNode | Record<string, any> | undefined {
+  const nodes = sources.map(toFilterNode).filter((n) => n !== undefined);
+  if (nodes.length === 0) return undefined;
+  if (nodes.length === 1) return nodes[0];
+  return ['and', ...nodes] as FilterNode;
+}

packages/data-objectstack/src/filter-entry-translation.test.ts

@@ -257,6 +257,32 @@ describe('what this adapter emits passes the server’s own gate', () => {
   }
 });
 
+describe('a bare rule object directly under a logical node', () => {
+  beforeEach(() => clearSharedDiscoveryCache());
+
+  /**
+   * Produced by any caller that SPREADS a `ViewFilterRule[]` into an `and`
+   * rather than wrapping it — `['and', ...rules, ...tuples]`. The rules land as
+   * bare objects where the AST expects nodes, `isFilterAST` rejects the whole
+   * node (a 400 since objectstack#4121), and `parseFilterAST` reads the rule as
+   * a Mongo condition on columns literally named `field`/`operator`/`value`.
+   * objectui's own producer was fixed to wrap; this is the chokepoint defence.
+   */
+  bothRoutes(
+    'is translated in place',
+    ['and', { field: 'stage', operator: 'eq', value: 'won' }, ['owner', '=', 'me']],
+    (wire) => expect(wire).toEqual(['and', ['stage', '=', 'won'], ['owner', '=', 'me']]),
+  );
+
+  bothRoutes(
+    'leaves a genuine MongoDB condition child alone',
+    // No `field` key, so it is a condition on a column named `status` — not a
+    // rule. Translating it would invent a filter the caller never wrote.
+    ['and', { status: 'active' }, ['owner', '=', 'me']],
+    (wire) => expect(wire).toEqual(['and', { status: 'active' }, ['owner', '=', 'me']]),
+  );
+});
+
 describe('the two routes agree on shapes that are neither form', () => {
   beforeEach(() => clearSharedDiscoveryCache());
 

packages/data-objectstack/src/index.ts

@@ -238,9 +238,29 @@ function translateFilterArray(filter: unknown[]): unknown[] {
   return filter;
 }
 
-/** A child of a logical node: another array node, or a value we leave alone. */
+/**
+ * A child of a logical node: another array node, a bare rule object, or a value
+ * we leave alone.
+ *
+ * The bare-rule case comes from producers that SPREAD a `ViewFilterRule[]` into
+ * an `and` (`['and', ...rules, ...tuples]`) instead of wrapping it. That puts
+ * rule objects where the AST expects nodes, and the server has no good answer:
+ * `isFilterAST` rejects it (a 400 since objectstack#4121), while
+ * `parseFilterAST` reads the rule as a MongoDB condition and filters on columns
+ * literally named `field` / `operator` / `value` — three columns that do not
+ * exist, so the honest-looking result is empty.
+ *
+ * Only rule-SHAPED objects are translated: a child with no `field` is a genuine
+ * MongoDB condition (`{ status: 'active' }`) and must pass through untouched.
+ * Same discriminator `isObjectFilterEntryForm` uses at the top level.
+ */
 function translateFilterChild(child: unknown): unknown {
-  return Array.isArray(child) && child.length > 0 ? translateFilterArray(child) : child;
+  if (Array.isArray(child)) return child.length > 0 ? translateFilterArray(child) : child;
+  if (child && typeof child === 'object' && (child as any).field !== undefined) {
+    const tuple = objectFilterEntryToAST(child);
+    if (tuple) return tuple;
+  }
+  return child;
 }
 
 /**

packages/plugin-list/src/ListView.tsx

@@ -19,7 +19,7 @@ import { useDensityMode } from '@object-ui/react';
 import type { ListViewSchema } from '@object-ui/types';
 import { detectStatusField } from '@object-ui/types';
 import { usePullToRefresh } from '@object-ui/mobile';
-import { resolveConditionalFormatting, buildExpandFields, buildExportFileName, resolveCrudAffordances, normalizeListViewSchema, rowHeightToDensityMode } from '@object-ui/core';
+import { resolveConditionalFormatting, buildExpandFields, buildExportFileName, resolveCrudAffordances, normalizeListViewSchema, rowHeightToDensityMode, mergeFilterNodes } from '@object-ui/core';
 import { useObjectTranslation, useObjectLabel, useSafeFieldLabel, createSafeTranslation } from '@object-ui/i18n';
 import { usePermissions } from '@object-ui/permissions';
 
@@ -232,26 +232,27 @@ export function normalizeFilters(filters: any[]): any[] {
  * Returns `undefined` when nothing is active, so callers can skip `$filter`
  * entirely rather than sending an empty array.
  *
- * Known gap, preserved deliberately: a MongoDB-style object `schema.filter` has
- * no `.length` and is dropped here, as it always has been. Nothing in this repo
- * produces one for a list view (they come from dashboard widgets, which query
- * directly), so this stays a pure extraction rather than a behaviour change.
+ * A MongoDB-style object `schema.filter` used to be dropped here (`.length` is
+ * `undefined` on an object, so the guard read false) and the list returned every
+ * record. An earlier version of this comment called that unreachable, on the
+ * grounds that nothing in the repo hands a list view an object filter. That was
+ * wrong: `ObjectView` passes `mergedFilters` straight into this schema's
+ * `filter`, and its last fallback is `table.defaultFilters`, declared
+ * `Record<string, any>`. `toFilterNode` now converts it instead.
  */
 export function buildEffectiveFilter(
   baseFilter: unknown,
   currentFilters: FilterGroup,
   userFilterConditions: any[],
-): any[] | undefined {
-  const base = Array.isArray(baseFilter) ? baseFilter : [];
+): any[] | Record<string, any> | undefined {
   const userFilter = convertFilterGroupToAST(currentFilters);
-  const allFilters = [
-    ...(base.length > 0 ? [base] : []),
-    ...(userFilter.length > 0 ? [userFilter] : []),
-    // Normalize the per-field conditions (expands `in` into an `or` of `=`).
+  return mergeFilterNodes(
+    baseFilter,
+    userFilter.length > 0 ? userFilter : undefined,
+    // Normalize the per-field conditions (expands `in` into an `or` of `=`),
+    // each of which is already its own node.
     ...normalizeFilters(userFilterConditions),
-  ].filter((f: any) => Array.isArray(f) && f.length > 0);
-  if (allFilters.length === 0) return undefined;
-  return allFilters.length === 1 ? allFilters[0] : ['and', ...allFilters];
+  );
 }
 
 export function convertFilterGroupToAST(group: FilterGroup): any[] {

packages/plugin-list/src/__tests__/ListView.exportMatchesView.test.tsx

@@ -100,6 +100,34 @@ describe('ListView export mirrors the active view', () => {
   });
 });
 
+describe('a MongoDB-style view filter is not dropped', () => {
+  beforeEach(() => {
+    vi.spyOn(URL, 'createObjectURL').mockReturnValue('blob:x');
+    vi.spyOn(URL, 'revokeObjectURL').mockImplementation(() => {});
+  });
+  afterEach(() => vi.restoreAllMocks());
+
+  /**
+   * `ObjectView` passes `mergedFilters` straight into this schema's `filter`,
+   * and its last fallback is `table.defaultFilters` — declared
+   * `Record<string, any>`. The old `baseFilter.length > 0` guard read false for
+   * an object, so the list queried unfiltered and showed every record. An
+   * earlier comment here called that unreachable; it was not.
+   */
+  it('reaches the query as an AST instead of vanishing', async () => {
+    const { find } = harness({ ...BASE, filter: { status: 'active' } as any });
+    await vi.waitFor(() => expect(find).toHaveBeenCalled());
+    expect(find.mock.calls[0][1]?.$filter).toEqual(['status', '=', 'active']);
+  });
+
+  it('reaches the export too, by the same route', async () => {
+    const { exportDownload } = harness({ ...BASE, filter: { status: 'active' } as any });
+    await clickExportCsv();
+    await vi.waitFor(() => expect(exportDownload).toHaveBeenCalledTimes(1));
+    expect(exportDownload.mock.calls[0][1]?.filter).toEqual(['status', '=', 'active']);
+  });
+});
+
 describe('the fetch and the export build the SAME filter', () => {
   beforeEach(() => {
     vi.spyOn(URL, 'createObjectURL').mockReturnValue('blob:x');