diff --git a/.changeset/listview-filter-vocabulary.md b/.changeset/listview-filter-vocabulary.md new file mode 100644 index 0000000000..d5abcdfd32 --- /dev/null +++ b/.changeset/listview-filter-vocabulary.md @@ -0,0 +1,46 @@ +--- +"@object-ui/core": minor +"@object-ui/plugin-list": minor +"@object-ui/plugin-view": minor +"@object-ui/app-shell": minor +"@object-ui/types": patch +--- + +fix(views): ListView reads the spec-canonical `filter`, so a view's base filter reaches every visualization (#2890 scope A step 4) + +Third rename in the ListView vocabulary migration: **`filters` → `filter`**. Unlike +the first two this closes a live bug, because the fork was asymmetric. + +`ListView` was the **only** surface in the repo reading `filters`. Every child +view — `ObjectGrid`, `ObjectGallery`, `ObjectKanban`, `ObjectCalendar`, +`ObjectGantt`, `ObjectMap`, `ObjectTree`, `ObjectChart` — reads `filter`, and +`ListView` handed them `filters`. Wherever a child fetches its own rows instead +of receiving `ListView`'s, the view's base filter was silently dropped: + +- **a `chart` list view aggregated the whole object.** The chart branch built an + `object-chart` node with `filters:`; `ObjectChart` reads `schema.filter` and + never read `filters`, so a chart view with a base filter charted unfiltered + totals. +- the same applied to any of the other view components rendered standalone from + a list-view-shaped config. + +Conversely, a **spec-authored** list view — one carrying `filter`, which is what +the spec says and what `runtime-metadata-persistence` and "Save as view" already +persist — rendered **unfiltered** in `ListView`, because nothing read that key. + +The fold is a key rename only. Both keys carry an ObjectQL FilterNode array +everywhere in objectui; every consumer passes the value straight to `$filter`. +(The spec types `filter` as `ViewFilterRule[]` — `{field, operator, value}` +objects — so objectui's field is typed from the spec but used as something else. +That mismatch is real and left alone here: converting formats inside a +vocabulary fold would change what reaches the data source.) + +Also collapses a duplicated computation in `app-shell`'s `ObjectView`, which +computed the same effective filter **twice** — once as `filter` for the child +views, once as `filters` for `ListView` — with the two copies subtly different +(only one fell back to `listSchema.filter`; only the other ran token +substitution over the URL filters). There is now one computation, keeping both +behaviors. + +`filters` stays declared on `ListViewSchema` and in the drift guard's sanctioned +set — stored views carry it and it is still valid input — but it is input-only. diff --git a/packages/app-shell/src/views/InterfaceListPage.tsx b/packages/app-shell/src/views/InterfaceListPage.tsx index 6f11ebcedf..36b181745f 100644 --- a/packages/app-shell/src/views/InterfaceListPage.tsx +++ b/packages/app-shell/src/views/InterfaceListPage.tsx @@ -366,7 +366,7 @@ export function InterfaceListPage({ page, className, onConfigChange, reserveEdit objectName: objectDef.name, viewType: (allowed[0] ?? view.type ?? 'grid'), columns, - ...(filters.length ? { filters } : {}), + ...(filters.length ? { filter: filters } : {}), ...(sort?.length ? { sort } : {}), grouping: view.grouping, rowColor: view.rowColor, diff --git a/packages/app-shell/src/views/ObjectDataPage.tsx b/packages/app-shell/src/views/ObjectDataPage.tsx index 257ab2b62c..ff7926f096 100644 --- a/packages/app-shell/src/views/ObjectDataPage.tsx +++ b/packages/app-shell/src/views/ObjectDataPage.tsx @@ -218,7 +218,7 @@ export function ObjectDataPage({ dataSource, objects }: any) { objectName: objectDef.name, viewType: 'grid' as const, columns, - ...(urlFilters.length ? { filters: urlFilters } : {}), + ...(urlFilters.length ? { filter: urlFilters } : {}), kanban, calendar, gallery, diff --git a/packages/app-shell/src/views/ObjectView.tsx b/packages/app-shell/src/views/ObjectView.tsx index f2835b3170..45df2c3ef5 100644 --- a/packages/app-shell/src/views/ObjectView.tsx +++ b/packages/app-shell/src/views/ObjectView.tsx @@ -1307,12 +1307,18 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: an // through the same persistViewPatch helper which debounces and // batches concurrent toggles. sort: (viewDef as any).sort ?? listSchema.sort, + // The ONE place this view's effective filter is computed (#2890). + // It used to be computed twice — once here as `filter` for the child + // views, once further down as `filters` for ListView — with the two + // copies subtly different (only this one fell back to + // `listSchema.filter`; only that one ran token substitution over the + // URL filters). Now: base ?? listSchema, concatenated with the URL + // filters, and substituted as a whole. filter: (() => { const base = (viewDef as any).filter ?? listSchema.filter; - const substituted = substituteFilterTokens(base, filterScope); - if (!urlFilters.length) return substituted; - const baseArr = Array.isArray(substituted) ? substituted : []; - return [...baseArr, ...urlFilters]; + const baseArr = Array.isArray(base) ? base : base ? [base] : []; + const combined = urlFilters.length ? [...baseArr, ...urlFilters] : base; + return substituteFilterTokens(combined, filterScope); })(), hiddenFields: (viewDef as any).hiddenFields ?? listSchema.hiddenFields, columnState: (viewDef as any).columnState ?? (listSchema as any).columnState, @@ -1444,15 +1450,8 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: an ?? resolveManagedByEmptyState((objectDef as any)?.managedBy, t, objectDef.name, (objectDef as any)?.userActions), ), aria: viewDef.aria ?? listSchema.aria, - // Propagate filter/sort as default filters/sort for data flow - ...((() => { - const combined = [ - ...(Array.isArray(viewDef.filter) ? viewDef.filter : []), - ...urlFilters, - ]; - const substituted = substituteFilterTokens(combined, filterScope); - return Array.isArray(substituted) && substituted.length ? { filters: substituted } : {}; - })()), + // (the legacy `filters` twin of the `filter` above lived here until + // #2890 — see the note at its single remaining computation) ...(viewDef.sort?.length ? { sort: viewDef.sort } : {}), options: { kanban: { diff --git a/packages/core/src/utils/__tests__/normalize-list-view.test.ts b/packages/core/src/utils/__tests__/normalize-list-view.test.ts index a170f7d7b7..0cbbc4ffcb 100644 --- a/packages/core/src/utils/__tests__/normalize-list-view.test.ts +++ b/packages/core/src/utils/__tests__/normalize-list-view.test.ts @@ -98,6 +98,42 @@ describe('normalizeListViewSchema (#2890)', () => { }); }); + describe('filters → filter', () => { + it('folds the legacy `filters` into the spec-canonical `filter`', () => { + const out = normalizeListViewSchema({ + viewType: 'grid', + filters: [['stage', '=', 'won']], + }) as Record; + expect(out.filter).toEqual([['stage', '=', 'won']]); + expect('filters' in out).toBe(false); + }); + + it('lets the canonical key win when a view carries both', () => { + const out = normalizeListViewSchema({ + viewType: 'grid', + filter: [['stage', '=', 'won']], + filters: [['stage', '=', 'lost']], + }) as Record; + expect(out.filter).toEqual([['stage', '=', 'won']]); + expect('filters' in out).toBe(false); + }); + + it('preserves the value verbatim — the fold renames the key, it does not convert the format', () => { + // Both keys carry an ObjectQL FilterNode array in objectui, including the + // compound `['and', …]` form. Rewriting it here would change what reaches + // the data source. + const filters = ['and', ['stage', '=', 'won'], ['amount', '>', 100]]; + const out = normalizeListViewSchema({ viewType: 'grid', filters }) as Record; + expect(out.filter).toBe(filters); + }); + + it('ignores a non-array `filters`', () => { + const out = normalizeListViewSchema({ viewType: 'grid', filters: 'stage == "won"' }) as Record; + expect(out.filter).toBeUndefined(); + expect(out.filters).toBe('stage == "won"'); + }); + }); + describe('viewType defaulting', () => { it('defaults a missing view kind to the renderable `grid`', () => { expect(normalizeListViewSchema({ type: 'list-view' })).toEqual({ type: 'list-view', viewType: 'grid' }); diff --git a/packages/core/src/utils/normalize-list-view.ts b/packages/core/src/utils/normalize-list-view.ts index f33272a0a2..e45ffa3582 100644 --- a/packages/core/src/utils/normalize-list-view.ts +++ b/packages/core/src/utils/normalize-list-view.ts @@ -87,6 +87,13 @@ export function rowHeightToDensityMode(rowHeight: unknown): DensityMode { * Currently folded: * - `fields` → `columns` (#2890 scope A step 1) * - `densityMode` → `rowHeight` (#2890 scope A step 2) + * - `filters` → `filter` (#2890 scope A step 4). A key rename only: BOTH keys + * carry an ObjectQL FilterNode array (`[['stage','=','won']]`) everywhere in + * objectui — every consumer passes the value straight to `$filter`. The spec + * types `filter` as `ViewFilterRule[]` (`{field, operator, value}` objects), + * so objectui's field is typed from the spec but used as something else. + * That mismatch is real and out of scope here; converting formats inside a + * vocabulary fold would change what reaches the data source. * - `viewType`: a missing kind, or the view CATEGORY `'list'` that AI-authored * metadata stores and hosts forward verbatim, becomes the renderable `'grid'` * — otherwise it reaches the renderer's typeless default branch and shows as @@ -100,9 +107,11 @@ export function normalizeListViewSchema(schema: T): T { const foldColumns = Array.isArray(legacyFields); const legacyDensity = s.densityMode; const foldRowHeight = typeof legacyDensity === 'string' && legacyDensity in DENSITY_MODE_TO_ROW_HEIGHT; + const legacyFilters = s.filters; + const foldFilter = Array.isArray(legacyFilters); const viewType = s.viewType; const defaultViewKind = !viewType || viewType === 'list'; - if (!foldColumns && !foldRowHeight && !defaultViewKind) return schema; + if (!foldColumns && !foldRowHeight && !foldFilter && !defaultViewKind) return schema; const next: Record = { ...s }; if (foldColumns) { @@ -115,6 +124,10 @@ export function normalizeListViewSchema(schema: T): T { } delete next.densityMode; } + if (foldFilter) { + if (!Array.isArray(next.filter)) next.filter = legacyFilters; + delete next.filters; + } if (defaultViewKind) next.viewType = 'grid'; return next as T; } diff --git a/packages/plugin-list/src/ListView.tsx b/packages/plugin-list/src/ListView.tsx index 27a873c791..bdac2681d7 100644 --- a/packages/plugin-list/src/ListView.tsx +++ b/packages/plugin-list/src/ListView.tsx @@ -906,7 +906,7 @@ export const ListView = React.forwardRef(({ try { // Construct filter let finalFilter: any = []; - const baseFilter = schema.filters || []; + const baseFilter = schema.filter || []; const userFilter = convertFilterGroupToAST(currentFilters); @@ -1120,7 +1120,7 @@ export const ListView = React.forwardRef(({ fetchData(); return () => { isMounted = false; }; - }, [schema.objectName, schema.data, dataSource, schema.filters, effectivePageSize, currentSort, currentFilters, userFilterConditions, refreshKey, searchTerm, schema.searchableFields, expandFields, objectDefLoaded, schema.refreshTrigger, perms, serverPage, currentView, groupingConfig, ganttOwnsData]); // Re-fetch on filter/sort/search/refreshTrigger/perms/page change + }, [schema.objectName, schema.data, dataSource, schema.filter, effectivePageSize, currentSort, currentFilters, userFilterConditions, refreshKey, searchTerm, schema.searchableFields, expandFields, objectDefLoaded, schema.refreshTrigger, perms, serverPage, currentView, groupingConfig, ganttOwnsData]); // Re-fetch on filter/sort/search/refreshTrigger/perms/page change // Any change to the result-defining inputs (object, filters, sort, search, // grouping, page size) invalidates the current page number — snap back to @@ -1130,7 +1130,7 @@ export const ListView = React.forwardRef(({ // from under a user who just turned it. serverPage is deliberately NOT part of // the signature, so turning the page never triggers a reset. const pageResetSignature = JSON.stringify([ - schema.objectName, schema.filters, effectivePageSize, currentSort, + schema.objectName, schema.filter, effectivePageSize, currentSort, currentFilters, userFilterConditions, searchTerm, currentView, groupingConfig, ]); const prevPageResetSignature = React.useRef(pageResetSignature); @@ -1299,7 +1299,13 @@ export const ListView = React.forwardRef(({ const baseProps = { objectName: schema.objectName, fields: effectiveFields, - filters: schema.filters, + // Spec-canonical `filter` (#2890). Every child view — ObjectGrid, + // ObjectGallery, ObjectKanban, ObjectCalendar, ObjectGantt, ObjectMap, + // ObjectTree, ObjectChart — reads `schema.filter`; ListView was the only + // surface speaking `filters`, so a child that fetches its own rows (the + // chart branch below, and any of these rendered standalone) never saw the + // view's base filter at all. + filter: schema.filter, sort: currentSort, className: "h-full w-full", // Disable internal controls that clash with ListView toolbar @@ -1495,7 +1501,10 @@ export const ListView = React.forwardRef(({ type: 'object-chart', objectName: schema.objectName, chartType: chartCfg.chartType || 'bar', - filters: schema.filters, + // `ObjectChart` reads `schema.filter` and never read `filters`, so a + // chart list view with a base filter used to aggregate the WHOLE + // object (#2890). + filter: schema.filter, aggregate: { field: valueField, function: chartCfg.aggregation || 'count', @@ -1611,7 +1620,7 @@ export const ListView = React.forwardRef(({ .filter(Boolean); // Merge the same filter sources as the data fetch (base + user + conditions). - const baseFilter = schema.filters || []; + const baseFilter = schema.filter || []; const userFilter = convertFilterGroupToAST(currentFilters); const normalizedUserFilterConditions = normalizeFilters(userFilterConditions); const allFilters = [ @@ -1711,7 +1720,7 @@ export const ListView = React.forwardRef(({ URL.revokeObjectURL(url); } setShowExport(false); - }, [data, effectiveFields, resolvedExportOptions, schema.objectName, schema.filters, exportPermitted, dataSource, currentFilters, userFilterConditions, currentSort, objectDef, resolveObjectLabel]); + }, [data, effectiveFields, resolvedExportOptions, schema.objectName, schema.filter, exportPermitted, dataSource, currentFilters, userFilterConditions, currentSort, objectDef, resolveObjectLabel]); // All available fields for hide/show (with i18n) const allFields = React.useMemo(() => { diff --git a/packages/plugin-list/src/__tests__/ListView.test.tsx b/packages/plugin-list/src/__tests__/ListView.test.tsx index e2753fca8e..4ecf1ea44a 100644 --- a/packages/plugin-list/src/__tests__/ListView.test.tsx +++ b/packages/plugin-list/src/__tests__/ListView.test.tsx @@ -2740,4 +2740,30 @@ describe('ListView — column vocabulary (#2890)', () => { expect(selected).toEqual(expect.arrayContaining(['name'])); expect(selected).not.toContain('email'); }); + + // ── filter vocabulary (#2890 step 4) ────────────────────────────────────── + // ListView was the ONLY surface reading `filters`; every child view + // (ObjectGrid/Gallery/Kanban/Calendar/Gantt/Map/Tree/Chart) reads `filter`. + const lastFilter = () => mockDataSource.find.mock.calls.at(-1)?.[1]?.$filter; + + it('applies the spec-canonical `filter` as the base filter', async () => { + await renderWithColumns({ columns: ['name'], filter: [['stage', '=', 'won']] }); + expect(JSON.stringify(lastFilter())).toContain('won'); + }); + + it('still honors the legacy `filters` — stored view metadata carries it', async () => { + await renderWithColumns({ columns: ['name'], filters: [['stage', '=', 'won']] }); + expect(JSON.stringify(lastFilter())).toContain('won'); + }); + + it('lets `filter` win when a view carries both', async () => { + await renderWithColumns({ + columns: ['name'], + filter: [['stage', '=', 'won']], + filters: [['stage', '=', 'lost']], + }); + const applied = JSON.stringify(lastFilter()); + expect(applied).toContain('won'); + expect(applied).not.toContain('lost'); + }); }); diff --git a/packages/plugin-list/src/index.tsx b/packages/plugin-list/src/index.tsx index b5f5798b8f..b0f70ac3c3 100644 --- a/packages/plugin-list/src/index.tsx +++ b/packages/plugin-list/src/index.tsx @@ -40,7 +40,7 @@ ComponentRegistry.register('list-view', ListView, { { label: 'Map', value: 'map' }, ], defaultValue: 'grid' }, { name: 'columns', type: 'array', label: 'Columns' }, - { name: 'filters', type: 'array', label: 'Filters' }, + { name: 'filter', type: 'array', label: 'Filter' }, { name: 'sort', type: 'array', label: 'Sort' }, { name: 'options', type: 'object', label: 'View Options' }, ], @@ -48,7 +48,7 @@ ComponentRegistry.register('list-view', ListView, { objectName: '', viewType: 'grid', columns: [], - filters: [], + filter: [], sort: [], options: {}, } @@ -81,7 +81,7 @@ ComponentRegistry.register('list', ListView, { { label: 'Map', value: 'map' }, ], defaultValue: 'grid' }, { name: 'columns', type: 'array', label: 'Columns' }, - { name: 'filters', type: 'array', label: 'Filters' }, + { name: 'filter', type: 'array', label: 'Filter' }, { name: 'sort', type: 'array', label: 'Sort' }, { name: 'options', type: 'object', label: 'View Options' }, ] diff --git a/packages/plugin-view/src/ObjectView.tsx b/packages/plugin-view/src/ObjectView.tsx index 8cbdb0c79f..cf9ef26121 100644 --- a/packages/plugin-view/src/ObjectView.tsx +++ b/packages/plugin-view/src/ObjectView.tsx @@ -1007,7 +1007,7 @@ export const ObjectView: React.FC = ({ // already `columns`-keyed, so emitting `fields` here was a pure // canonical→legacy downgrade. columns: currentNamedViewConfig?.columns || activeView?.columns || schema.table?.fields, - filters: mergedFilters, + filter: mergedFilters, sort: mergedSort, // Propagate appearance/view-config properties for live preview rowHeight: activeView?.rowHeight, diff --git a/packages/types/src/__tests__/list-view-spec-parity.test.ts b/packages/types/src/__tests__/list-view-spec-parity.test.ts index 46012dd39b..6f48fcbeef 100644 --- a/packages/types/src/__tests__/list-view-spec-parity.test.ts +++ b/packages/types/src/__tests__/list-view-spec-parity.test.ts @@ -78,6 +78,7 @@ const SANCTIONED_LOCAL = new Set([ // so stored metadata keeps validating and so the spec's react-blocks // `` prop keeps a schema anchor. 'fields', + // `filters` is likewise INPUT-ONLY since #2890 — folded into `filter`. 'filters', // legacy toolbar visibility flags (spec-canonical: `userActions`) 'showSearch', diff --git a/packages/types/src/zod/objectql.zod.ts b/packages/types/src/zod/objectql.zod.ts index f6a2615e2a..492252034d 100644 --- a/packages/types/src/zod/objectql.zod.ts +++ b/packages/types/src/zod/objectql.zod.ts @@ -386,7 +386,11 @@ export const ListViewSchema = BaseSchema // (`@object-ui/core`) folds it into `columns` at the ListView boundary // (#2890). Producers must emit `columns`. fields: z.array(z.string()).optional().describe('Legacy alias for string[] columns'), - // Legacy tuple/CEL filter format (spec-canonical `filter` is imported above). + // Legacy alias for the spec's `filter`, still accepted because stored view + // metadata carries it. NO renderer reads it: `normalizeListViewSchema` + // (`@object-ui/core`) folds it into `filter` at the ListView boundary + // (#2890). Note both keys carry an ObjectQL FilterNode array at runtime, + // even though `filter` is typed from the spec as `ViewFilterRule[]`. filters: z.array(z.union([z.array(z.any()), z.string()])).optional().describe('Filter conditions (legacy)'), // Legacy toolbar visibility flags (spec-canonical is `userActions`; runtime dual-reads). showSearch: z.boolean().optional().describe('Show search in toolbar'),