Skip to content

Commit f1abf0e

Browse files
os-zhuangclaude
os-zhuang
and
claude
authored
fix(views): ListView reads the spec-canonical filter (#2890) (#2935)
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 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 base filter was dropped: a `chart` list view aggregated the whole object, because the chart branch emitted `filters:` and ObjectChart reads `schema.filter`. Conversely a spec-authored view carrying `filter` — what the spec says, and what runtime-metadata-persistence and "Save as view" already persist — rendered unfiltered in ListView, because nothing read that key. Key rename only: both keys carry an ObjectQL FilterNode array everywhere in objectui and every consumer passes the value straight to `$filter`. The spec types `filter` as ViewFilterRule[], so the field is typed from the spec but used as something else — noted in the normalizer, left alone here, since converting formats inside a vocabulary fold would change what reaches the data source. Also collapses a duplicated computation in app-shell's ObjectView, which built the same effective filter twice (once as `filter` for the children, once as `filters` for ListView) with the copies subtly different — only one fell back to listSchema.filter, only the other substituted tokens in the URL filters. One computation now, keeping both behaviors. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6a56aa5 commit f1abf0e

12 files changed

Lines changed: 162 additions & 28 deletions

File tree

‎.changeset/listview-filter-vocabulary.md‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
---
2+
"@object-ui/core": minor
3+
"@object-ui/plugin-list": minor
4+
"@object-ui/plugin-view": minor
5+
"@object-ui/app-shell": minor
6+
"@object-ui/types": patch
7+
---
8+
9+
fix(views): ListView reads the spec-canonical `filter`, so a view's base filter reaches every visualization (#2890 scope A step 4)
10+
11+
Third rename in the ListView vocabulary migration: **`filters` → `filter`**. Unlike
12+
the first two this closes a live bug, because the fork was asymmetric.
13+
14+
`ListView` was the **only** surface in the repo reading `filters`. Every child
15+
view — `ObjectGrid`, `ObjectGallery`, `ObjectKanban`, `ObjectCalendar`,
16+
`ObjectGantt`, `ObjectMap`, `ObjectTree`, `ObjectChart` — reads `filter`, and
17+
`ListView` handed them `filters`. Wherever a child fetches its own rows instead
18+
of receiving `ListView`'s, the view's base filter was silently dropped:
19+
20+
- **a `chart` list view aggregated the whole object.** The chart branch built an
21+
`object-chart` node with `filters:`; `ObjectChart` reads `schema.filter` and
22+
never read `filters`, so a chart view with a base filter charted unfiltered
23+
totals.
24+
- the same applied to any of the other view components rendered standalone from
25+
a list-view-shaped config.
26+
27+
Conversely, a **spec-authored** list view — one carrying `filter`, which is what
28+
the spec says and what `runtime-metadata-persistence` and "Save as view" already
29+
persist — rendered **unfiltered** in `ListView`, because nothing read that key.
30+
31+
The fold is a key rename only. Both keys carry an ObjectQL FilterNode array
32+
everywhere in objectui; every consumer passes the value straight to `$filter`.
33+
(The spec types `filter` as `ViewFilterRule[]` — `{field, operator, value}`
34+
objects — so objectui's field is typed from the spec but used as something else.
35+
That mismatch is real and left alone here: converting formats inside a
36+
vocabulary fold would change what reaches the data source.)
37+
38+
Also collapses a duplicated computation in `app-shell`'s `ObjectView`, which
39+
computed the same effective filter **twice** — once as `filter` for the child
40+
views, once as `filters` for `ListView` — with the two copies subtly different
41+
(only one fell back to `listSchema.filter`; only the other ran token
42+
substitution over the URL filters). There is now one computation, keeping both
43+
behaviors.
44+
45+
`filters` stays declared on `ListViewSchema` and in the drift guard's sanctioned
46+
set — stored views carry it and it is still valid input — but it is input-only.

‎packages/app-shell/src/views/InterfaceListPage.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,7 @@ export function InterfaceListPage({ page, className, onConfigChange, reserveEdit
366366
objectName: objectDef.name,
367367
viewType: (allowed[0] ?? view.type ?? 'grid'),
368368
columns,
369-
...(filters.length ? { filters } : {}),
369+
...(filters.length ? { filter: filters } : {}),
370370
...(sort?.length ? { sort } : {}),
371371
grouping: view.grouping,
372372
rowColor: view.rowColor,

‎packages/app-shell/src/views/ObjectDataPage.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ export function ObjectDataPage({ dataSource, objects }: any) {
218218
objectName: objectDef.name,
219219
viewType: 'grid' as const,
220220
columns,
221-
...(urlFilters.length ? { filters: urlFilters } : {}),
221+
...(urlFilters.length ? { filter: urlFilters } : {}),
222222
kanban,
223223
calendar,
224224
gallery,

‎packages/app-shell/src/views/ObjectView.tsx‎

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1307,12 +1307,18 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: an
13071307
// through the same persistViewPatch helper which debounces and
13081308
// batches concurrent toggles.
13091309
sort: (viewDef as any).sort ?? listSchema.sort,
1310+
// The ONE place this view's effective filter is computed (#2890).
1311+
// It used to be computed twice — once here as `filter` for the child
1312+
// views, once further down as `filters` for ListView — with the two
1313+
// copies subtly different (only this one fell back to
1314+
// `listSchema.filter`; only that one ran token substitution over the
1315+
// URL filters). Now: base ?? listSchema, concatenated with the URL
1316+
// filters, and substituted as a whole.
13101317
filter: (() => {
13111318
const base = (viewDef as any).filter ?? listSchema.filter;
1312-
const substituted = substituteFilterTokens(base, filterScope);
1313-
if (!urlFilters.length) return substituted;
1314-
const baseArr = Array.isArray(substituted) ? substituted : [];
1315-
return [...baseArr, ...urlFilters];
1319+
const baseArr = Array.isArray(base) ? base : base ? [base] : [];
1320+
const combined = urlFilters.length ? [...baseArr, ...urlFilters] : base;
1321+
return substituteFilterTokens(combined, filterScope);
13161322
})(),
13171323
hiddenFields: (viewDef as any).hiddenFields ?? listSchema.hiddenFields,
13181324
columnState: (viewDef as any).columnState ?? (listSchema as any).columnState,
@@ -1444,15 +1450,8 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: an
14441450
?? resolveManagedByEmptyState((objectDef as any)?.managedBy, t, objectDef.name, (objectDef as any)?.userActions),
14451451
),
14461452
aria: viewDef.aria ?? listSchema.aria,
1447-
// Propagate filter/sort as default filters/sort for data flow
1448-
...((() => {
1449-
const combined = [
1450-
...(Array.isArray(viewDef.filter) ? viewDef.filter : []),
1451-
...urlFilters,
1452-
];
1453-
const substituted = substituteFilterTokens(combined, filterScope);
1454-
return Array.isArray(substituted) && substituted.length ? { filters: substituted } : {};
1455-
})()),
1453+
// (the legacy `filters` twin of the `filter` above lived here until
1454+
// #2890 — see the note at its single remaining computation)
14561455
...(viewDef.sort?.length ? { sort: viewDef.sort } : {}),
14571456
options: {
14581457
kanban: {

‎packages/core/src/utils/__tests__/normalize-list-view.test.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,42 @@ describe('normalizeListViewSchema (#2890)', () => {
9898
});
9999
});
100100

101+
describe('filters → filter', () => {
102+
it('folds the legacy `filters` into the spec-canonical `filter`', () => {
103+
const out = normalizeListViewSchema({
104+
viewType: 'grid',
105+
filters: [['stage', '=', 'won']],
106+
}) as Record<string, unknown>;
107+
expect(out.filter).toEqual([['stage', '=', 'won']]);
108+
expect('filters' in out).toBe(false);
109+
});
110+
111+
it('lets the canonical key win when a view carries both', () => {
112+
const out = normalizeListViewSchema({
113+
viewType: 'grid',
114+
filter: [['stage', '=', 'won']],
115+
filters: [['stage', '=', 'lost']],
116+
}) as Record<string, unknown>;
117+
expect(out.filter).toEqual([['stage', '=', 'won']]);
118+
expect('filters' in out).toBe(false);
119+
});
120+
121+
it('preserves the value verbatim — the fold renames the key, it does not convert the format', () => {
122+
// Both keys carry an ObjectQL FilterNode array in objectui, including the
123+
// compound `['and', …]` form. Rewriting it here would change what reaches
124+
// the data source.
125+
const filters = ['and', ['stage', '=', 'won'], ['amount', '>', 100]];
126+
const out = normalizeListViewSchema({ viewType: 'grid', filters }) as Record<string, unknown>;
127+
expect(out.filter).toBe(filters);
128+
});
129+
130+
it('ignores a non-array `filters`', () => {
131+
const out = normalizeListViewSchema({ viewType: 'grid', filters: 'stage == "won"' }) as Record<string, unknown>;
132+
expect(out.filter).toBeUndefined();
133+
expect(out.filters).toBe('stage == "won"');
134+
});
135+
});
136+
101137
describe('viewType defaulting', () => {
102138
it('defaults a missing view kind to the renderable `grid`', () => {
103139
expect(normalizeListViewSchema({ type: 'list-view' })).toEqual({ type: 'list-view', viewType: 'grid' });

‎packages/core/src/utils/normalize-list-view.ts‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,13 @@ export function rowHeightToDensityMode(rowHeight: unknown): DensityMode {
8787
* Currently folded:
8888
* - `fields` → `columns` (#2890 scope A step 1)
8989
* - `densityMode` → `rowHeight` (#2890 scope A step 2)
90+
* - `filters` → `filter` (#2890 scope A step 4). A key rename only: BOTH keys
91+
* carry an ObjectQL FilterNode array (`[['stage','=','won']]`) everywhere in
92+
* objectui — every consumer passes the value straight to `$filter`. The spec
93+
* types `filter` as `ViewFilterRule[]` (`{field, operator, value}` objects),
94+
* so objectui's field is typed from the spec but used as something else.
95+
* That mismatch is real and out of scope here; converting formats inside a
96+
* vocabulary fold would change what reaches the data source.
9097
* - `viewType`: a missing kind, or the view CATEGORY `'list'` that AI-authored
9198
* metadata stores and hosts forward verbatim, becomes the renderable `'grid'`
9299
* — otherwise it reaches the renderer's typeless default branch and shows as
@@ -100,9 +107,11 @@ export function normalizeListViewSchema<T>(schema: T): T {
100107
const foldColumns = Array.isArray(legacyFields);
101108
const legacyDensity = s.densityMode;
102109
const foldRowHeight = typeof legacyDensity === 'string' && legacyDensity in DENSITY_MODE_TO_ROW_HEIGHT;
110+
const legacyFilters = s.filters;
111+
const foldFilter = Array.isArray(legacyFilters);
103112
const viewType = s.viewType;
104113
const defaultViewKind = !viewType || viewType === 'list';
105-
if (!foldColumns && !foldRowHeight && !defaultViewKind) return schema;
114+
if (!foldColumns && !foldRowHeight && !foldFilter && !defaultViewKind) return schema;
106115

107116
const next: Record<string, unknown> = { ...s };
108117
if (foldColumns) {
@@ -115,6 +124,10 @@ export function normalizeListViewSchema<T>(schema: T): T {
115124
}
116125
delete next.densityMode;
117126
}
127+
if (foldFilter) {
128+
if (!Array.isArray(next.filter)) next.filter = legacyFilters;
129+
delete next.filters;
130+
}
118131
if (defaultViewKind) next.viewType = 'grid';
119132
return next as T;
120133
}

‎packages/plugin-list/src/ListView.tsx‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -906,7 +906,7 @@ export const ListView = React.forwardRef<ListViewHandle, ListViewProps>(({
906906
try {
907907
// Construct filter
908908
let finalFilter: any = [];
909-
const baseFilter = schema.filters || [];
909+
const baseFilter = schema.filter || [];
910910
const userFilter = convertFilterGroupToAST(currentFilters);
911911

912912

@@ -1120,7 +1120,7 @@ export const ListView = React.forwardRef<ListViewHandle, ListViewProps>(({
11201120
fetchData();
11211121

11221122
return () => { isMounted = false; };
1123-
}, [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
1123+
}, [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
11241124

11251125
// Any change to the result-defining inputs (object, filters, sort, search,
11261126
// grouping, page size) invalidates the current page number — snap back to
@@ -1130,7 +1130,7 @@ export const ListView = React.forwardRef<ListViewHandle, ListViewProps>(({
11301130
// from under a user who just turned it. serverPage is deliberately NOT part of
11311131
// the signature, so turning the page never triggers a reset.
11321132
const pageResetSignature = JSON.stringify([
1133-
schema.objectName, schema.filters, effectivePageSize, currentSort,
1133+
schema.objectName, schema.filter, effectivePageSize, currentSort,
11341134
currentFilters, userFilterConditions, searchTerm, currentView, groupingConfig,
11351135
]);
11361136
const prevPageResetSignature = React.useRef(pageResetSignature);
@@ -1299,7 +1299,13 @@ export const ListView = React.forwardRef<ListViewHandle, ListViewProps>(({
12991299
const baseProps = {
13001300
objectName: schema.objectName,
13011301
fields: effectiveFields,
1302-
filters: schema.filters,
1302+
// Spec-canonical `filter` (#2890). Every child view — ObjectGrid,
1303+
// ObjectGallery, ObjectKanban, ObjectCalendar, ObjectGantt, ObjectMap,
1304+
// ObjectTree, ObjectChart — reads `schema.filter`; ListView was the only
1305+
// surface speaking `filters`, so a child that fetches its own rows (the
1306+
// chart branch below, and any of these rendered standalone) never saw the
1307+
// view's base filter at all.
1308+
filter: schema.filter,
13031309
sort: currentSort,
13041310
className: "h-full w-full",
13051311
// Disable internal controls that clash with ListView toolbar
@@ -1495,7 +1501,10 @@ export const ListView = React.forwardRef<ListViewHandle, ListViewProps>(({
14951501
type: 'object-chart',
14961502
objectName: schema.objectName,
14971503
chartType: chartCfg.chartType || 'bar',
1498-
filters: schema.filters,
1504+
// `ObjectChart` reads `schema.filter` and never read `filters`, so a
1505+
// chart list view with a base filter used to aggregate the WHOLE
1506+
// object (#2890).
1507+
filter: schema.filter,
14991508
aggregate: {
15001509
field: valueField,
15011510
function: chartCfg.aggregation || 'count',
@@ -1611,7 +1620,7 @@ export const ListView = React.forwardRef<ListViewHandle, ListViewProps>(({
16111620
.filter(Boolean);
16121621

16131622
// Merge the same filter sources as the data fetch (base + user + conditions).
1614-
const baseFilter = schema.filters || [];
1623+
const baseFilter = schema.filter || [];
16151624
const userFilter = convertFilterGroupToAST(currentFilters);
16161625
const normalizedUserFilterConditions = normalizeFilters(userFilterConditions);
16171626
const allFilters = [
@@ -1711,7 +1720,7 @@ export const ListView = React.forwardRef<ListViewHandle, ListViewProps>(({
17111720
URL.revokeObjectURL(url);
17121721
}
17131722
setShowExport(false);
1714-
}, [data, effectiveFields, resolvedExportOptions, schema.objectName, schema.filters, exportPermitted, dataSource, currentFilters, userFilterConditions, currentSort, objectDef, resolveObjectLabel]);
1723+
}, [data, effectiveFields, resolvedExportOptions, schema.objectName, schema.filter, exportPermitted, dataSource, currentFilters, userFilterConditions, currentSort, objectDef, resolveObjectLabel]);
17151724

17161725
// All available fields for hide/show (with i18n)
17171726
const allFields = React.useMemo(() => {

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2740,4 +2740,30 @@ describe('ListView — column vocabulary (#2890)', () => {
27402740
expect(selected).toEqual(expect.arrayContaining(['name']));
27412741
expect(selected).not.toContain('email');
27422742
});
2743+
2744+
// ── filter vocabulary (#2890 step 4) ──────────────────────────────────────
2745+
// ListView was the ONLY surface reading `filters`; every child view
2746+
// (ObjectGrid/Gallery/Kanban/Calendar/Gantt/Map/Tree/Chart) reads `filter`.
2747+
const lastFilter = () => mockDataSource.find.mock.calls.at(-1)?.[1]?.$filter;
2748+
2749+
it('applies the spec-canonical `filter` as the base filter', async () => {
2750+
await renderWithColumns({ columns: ['name'], filter: [['stage', '=', 'won']] });
2751+
expect(JSON.stringify(lastFilter())).toContain('won');
2752+
});
2753+
2754+
it('still honors the legacy `filters` — stored view metadata carries it', async () => {
2755+
await renderWithColumns({ columns: ['name'], filters: [['stage', '=', 'won']] });
2756+
expect(JSON.stringify(lastFilter())).toContain('won');
2757+
});
2758+
2759+
it('lets `filter` win when a view carries both', async () => {
2760+
await renderWithColumns({
2761+
columns: ['name'],
2762+
filter: [['stage', '=', 'won']],
2763+
filters: [['stage', '=', 'lost']],
2764+
});
2765+
const applied = JSON.stringify(lastFilter());
2766+
expect(applied).toContain('won');
2767+
expect(applied).not.toContain('lost');
2768+
});
27432769
});

‎packages/plugin-list/src/index.tsx‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,15 +40,15 @@ ComponentRegistry.register('list-view', ListView, {
4040
{ label: 'Map', value: 'map' },
4141
], defaultValue: 'grid' },
4242
{ name: 'columns', type: 'array', label: 'Columns' },
43-
{ name: 'filters', type: 'array', label: 'Filters' },
43+
{ name: 'filter', type: 'array', label: 'Filter' },
4444
{ name: 'sort', type: 'array', label: 'Sort' },
4545
{ name: 'options', type: 'object', label: 'View Options' },
4646
],
4747
defaultProps: {
4848
objectName: '',
4949
viewType: 'grid',
5050
columns: [],
51-
filters: [],
51+
filter: [],
5252
sort: [],
5353
options: {},
5454
}
@@ -81,7 +81,7 @@ ComponentRegistry.register('list', ListView, {
8181
{ label: 'Map', value: 'map' },
8282
], defaultValue: 'grid' },
8383
{ name: 'columns', type: 'array', label: 'Columns' },
84-
{ name: 'filters', type: 'array', label: 'Filters' },
84+
{ name: 'filter', type: 'array', label: 'Filter' },
8585
{ name: 'sort', type: 'array', label: 'Sort' },
8686
{ name: 'options', type: 'object', label: 'View Options' },
8787
]

‎packages/plugin-view/src/ObjectView.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1007,7 +1007,7 @@ export const ObjectView: React.FC<ObjectViewProps> = ({
10071007
// already `columns`-keyed, so emitting `fields` here was a pure
10081008
// canonical→legacy downgrade.
10091009
columns: currentNamedViewConfig?.columns || activeView?.columns || schema.table?.fields,
1010-
filters: mergedFilters,
1010+
filter: mergedFilters,
10111011
sort: mergedSort,
10121012
// Propagate appearance/view-config properties for live preview
10131013
rowHeight: activeView?.rowHeight,

0 commit comments

Comments
 (0)