Skip to content

Commit 7f0252e

Browse files
os-zhuangclaude
andauthored
fix(list,data-objectstack,types): exporting a searched list no longer downloads the unsearched superset (#3078)
The server-streamed export mirrored the view's `filter` and `sort`, and the code comment claimed that made the file match the screen: Mirrors the active view's filter + sort so the exported file matches what the user sees. It mirrored one half. There was no way to carry the term a user had typed into the search box — `ExportDownloadRequest` had no field for one — so exporting during a search produced MORE ROWS than the list showed, in a file that looks authoritative, with nothing indicating the difference. The client-side fallback was always correct (it serializes the already-searched `data`); only the server path was wrong, and it is the one that handles xlsx. Same family as a dropped filter (objectstack#3948, objectstack#4181): a plausible answer that is quietly broader than the one asked for. - `ExportDownloadRequest` gains `search` / `searchFields`. - `ObjectStackAdapter.exportDownload` sends them as `search=` / `searchFields=`, trimming the term and omitting both when it is blank (`searchFields` alone means nothing). - `ListView` passes the active `searchTerm` and the view's `searchableFields`, and both are now in the export callback's dependency array — a stale closure would export the wrong row set. Requires a server with objectstack#4230. Older servers ignore unknown query params on this route, so they keep today's behaviour rather than erroring. Also: the filter merge is no longer written twice. The three filter sources (view filter, filter-panel group, per-field user filters) were merged by verbatim copies in the data fetch and in the export — two copies that must agree, deciding respectively what the user SEES and what they DOWNLOAD. Both now call `buildEffectiveFilter`. That half is a PURE EXTRACTION, and the tests say so: the four parity tests added for it pass against the old duplicated code too. They exist to keep it that way — the adapter's duplicated filter-shape check had already drifted apart unnoticed (#3072). The parity tests compare the arguments of the two real calls rather than asserting an expected AST twice, which would go on passing if both copies drifted the same wrong way. Verification: 9 new tests (4 ListView export/search, 4 fetch-vs-export parity, 5 adapter query params). Reverting ListView.tsx fails the 2 search tests and passes the 4 parity ones; reverting the adapter fails 4. Full suite 758 files / 8819 tests green; tsc clean across types, data-objectstack, plugin-list; eslint 0 errors. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 97c72a0 commit 7f0252e

6 files changed

Lines changed: 317 additions & 44 deletions

File tree

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
---
2+
"@object-ui/types": patch
3+
"@object-ui/data-objectstack": patch
4+
"@object-ui/plugin-list": patch
5+
---
6+
7+
fix(list,data-objectstack,types): exporting a searched list no longer downloads the unsearched superset
8+
9+
The server-streamed export mirrored the view's `filter` and `sort`, and the
10+
code comment claimed that made the file match the screen:
11+
12+
> Mirrors the active view's filter + sort so the exported file matches what the
13+
> user sees.
14+
15+
It mirrored one half. There was no way to carry the term a user had typed into
16+
the search box — `ExportDownloadRequest` had no field for one — so exporting
17+
during a search produced **more rows than the list showed**, in a file that
18+
looks authoritative, with nothing indicating the difference. The client-side
19+
fallback was always correct (it serializes the already-searched `data`); only
20+
the server path was wrong, and it is the one that handles xlsx.
21+
22+
Same family as a dropped filter (objectstack#3948, objectstack#4181): a
23+
plausible answer that is quietly broader than the one asked for.
24+
25+
- `ExportDownloadRequest` gains `search` / `searchFields`.
26+
- `ObjectStackAdapter.exportDownload` sends them as `search=` / `searchFields=`,
27+
trimming the term and omitting both when it is blank (`searchFields` alone
28+
means nothing).
29+
- `ListView` passes the active `searchTerm` and the view's `searchableFields`,
30+
and both are now in the export callback's dependency array — a stale closure
31+
would export the wrong row set.
32+
33+
Requires a server with objectstack#4230. Older servers ignore unknown query
34+
params on this route, so they keep today's behaviour rather than erroring.
35+
36+
**Also: the filter merge is no longer written twice.** The three filter sources
37+
(view filter, filter-panel group, per-field user filters) were merged by
38+
verbatim copies in the data fetch and in the export — two copies that must
39+
agree, deciding respectively what the user *sees* and what they *download*.
40+
Both now call `buildEffectiveFilter`. This is a pure extraction: the copies did
41+
agree, and the four parity tests added for it pass against the old code too.
42+
They exist to keep it that way — the adapter's duplicated filter-shape check
43+
had already drifted apart unnoticed (#3072).

packages/data-objectstack/src/exportDownload.test.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,3 +97,45 @@ describe('ObjectStackAdapter.exportDownload', () => {
9797
});
9898
});
9999
});
100+
101+
/**
102+
* `search` — the half of a list this request could not carry.
103+
*
104+
* The route mirrored `filter` and `orderby` only, so an export taken while a
105+
* search was active downloaded the UNSEARCHED superset: more rows than the
106+
* screen showed, in a file that looks authoritative. Server half:
107+
* objectstack#4230.
108+
*/
109+
describe('ObjectStackAdapter.exportDownload — search', () => {
110+
const paramsFor = async (request: Record<string, unknown>) => {
111+
const fetchImpl = vi.fn(async (_url: string, _init: RequestInit) => csvResponse());
112+
await makeDS(fetchImpl).exportDownload('task', request);
113+
return new URL(fetchImpl.mock.calls[0][0]).searchParams;
114+
};
115+
116+
it('sends the term as `search`', async () => {
117+
expect((await paramsFor({ search: 'acme' })).get('search')).toBe('acme');
118+
});
119+
120+
it('sends `searchFields` as a comma list alongside the term', async () => {
121+
const p = await paramsFor({ search: 'acme', searchFields: ['name', 'stage'] });
122+
expect(p.get('searchFields')).toBe('name,stage');
123+
});
124+
125+
it('omits both when no term is given — `searchFields` alone means nothing', async () => {
126+
const p = await paramsFor({ searchFields: ['name'] });
127+
expect(p.get('search')).toBeNull();
128+
expect(p.get('searchFields')).toBeNull();
129+
});
130+
131+
it('treats a whitespace-only term as absent, and trims a real one', async () => {
132+
expect((await paramsFor({ search: ' ' })).get('search')).toBeNull();
133+
expect((await paramsFor({ search: ' acme ' })).get('search')).toBe('acme');
134+
});
135+
136+
it('carries search and filter together — neither replaces the other', async () => {
137+
const p = await paramsFor({ search: 'acme', filter: [['status', '=', 'open']] });
138+
expect(p.get('search')).toBe('acme');
139+
expect(JSON.parse(p.get('filter')!)).toEqual([['status', '=', 'open']]);
140+
});
141+
});

packages/data-objectstack/src/index.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2130,6 +2130,16 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
21302130
queryParams.set('filter', JSON.stringify(translated));
21312131
}
21322132
}
2133+
// Search — the other half of what a list is showing. Without it an export
2134+
// taken while a search is active returns the unsearched superset
2135+
// (objectstack#4230). Servers predating that ignore the param.
2136+
const searchTerm = typeof request.search === 'string' ? request.search.trim() : '';
2137+
if (searchTerm) {
2138+
queryParams.set('search', searchTerm);
2139+
if (request.searchFields && request.searchFields.length > 0) {
2140+
queryParams.set('searchFields', request.searchFields.join(','));
2141+
}
2142+
}
21332143

21342144
const baseUrl = this.baseUrl.replace(/\/$/, '');
21352145
// Avoid doubling /api/v1 if baseUrl already includes the version suffix.

packages/plugin-list/src/ListView.tsx

Lines changed: 66 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,42 @@ export function normalizeFilters(filters: any[]): any[] {
218218
.filter(f => Array.isArray(f) && f.length > 0);
219219
}
220220

221+
/**
222+
* Merge the three filter sources a list can have — the view's own `filter`, the
223+
* filter-panel group, and the per-field user filters — into one AST node.
224+
*
225+
* ONE definition on purpose. The data fetch and the export used to carry
226+
* verbatim copies of this, and those two decide, respectively, **what the user
227+
* sees** and **what they download**. Two copies that must agree, where a
228+
* divergence is a file that silently disagrees with the screen — the same shape
229+
* of bug that had already bitten the adapter, whose duplicated filter-shape
230+
* check drifted apart unnoticed (#3072).
231+
*
232+
* Returns `undefined` when nothing is active, so callers can skip `$filter`
233+
* entirely rather than sending an empty array.
234+
*
235+
* Known gap, preserved deliberately: a MongoDB-style object `schema.filter` has
236+
* no `.length` and is dropped here, as it always has been. Nothing in this repo
237+
* produces one for a list view (they come from dashboard widgets, which query
238+
* directly), so this stays a pure extraction rather than a behaviour change.
239+
*/
240+
export function buildEffectiveFilter(
241+
baseFilter: unknown,
242+
currentFilters: FilterGroup,
243+
userFilterConditions: any[],
244+
): any[] | undefined {
245+
const base = Array.isArray(baseFilter) ? baseFilter : [];
246+
const userFilter = convertFilterGroupToAST(currentFilters);
247+
const allFilters = [
248+
...(base.length > 0 ? [base] : []),
249+
...(userFilter.length > 0 ? [userFilter] : []),
250+
// Normalize the per-field conditions (expands `in` into an `or` of `=`).
251+
...normalizeFilters(userFilterConditions),
252+
].filter((f: any) => Array.isArray(f) && f.length > 0);
253+
if (allFilters.length === 0) return undefined;
254+
return allFilters.length === 1 ? allFilters[0] : ['and', ...allFilters];
255+
}
256+
221257
export function convertFilterGroupToAST(group: FilterGroup): any[] {
222258
if (!group || !group.conditions || group.conditions.length === 0) return [];
223259

@@ -1044,28 +1080,10 @@ export const ListView = React.forwardRef<ListViewHandle, ListViewProps>(({
10441080
setLoading(true);
10451081
setLoadError(null);
10461082
try {
1047-
// Construct filter
1048-
let finalFilter: any = [];
1049-
const baseFilter = schema.filter || [];
1050-
const userFilter = convertFilterGroupToAST(currentFilters);
1051-
1052-
1053-
// Normalize userFilter conditions (convert `in` to `or` of `=`)
1054-
const normalizedUserFilterConditions = normalizeFilters(userFilterConditions);
1055-
1056-
// Merge all filter sources with consistent structure
1057-
const allFilters = [
1058-
...(baseFilter.length > 0 ? [baseFilter] : []),
1059-
...(userFilter.length > 0 ? [userFilter] : []),
1060-
...normalizedUserFilterConditions,
1061-
].filter(f => Array.isArray(f) && f.length > 0);
1062-
1063-
if (allFilters.length > 1) {
1064-
finalFilter = ['and', ...allFilters];
1065-
} else if (allFilters.length === 1) {
1066-
finalFilter = allFilters[0];
1067-
}
1068-
1083+
// Construct filter — shared with the export path so the file a user
1084+
// downloads is built from the same three sources as the rows on screen.
1085+
const finalFilter = buildEffectiveFilter(schema.filter, currentFilters, userFilterConditions);
1086+
10691087
// Convert sort to query format
10701088
// Use array format to ensure order is preserved (Object keys are not guaranteed ordered)
10711089
const sort: any = currentSort.length > 0
@@ -1183,12 +1201,11 @@ export const ListView = React.forwardRef<ListViewHandle, ListViewProps>(({
11831201
return Array.from(required);
11841202
})();
11851203

1186-
// Only send $filter when it is a non-empty AST. Sending an empty
1187-
// array results in `?filter=%5B%5D` which is wasted bandwidth and
1188-
// can defeat server-side query parsing/caching.
1189-
const hasFilter = Array.isArray(finalFilter)
1190-
? finalFilter.length > 0
1191-
: !!finalFilter && Object.keys(finalFilter).length > 0;
1204+
// Only send $filter when there is one. Sending an empty array results in
1205+
// `?filter=%5B%5D` which is wasted bandwidth and can defeat server-side
1206+
// query parsing/caching. `buildEffectiveFilter` returns a non-empty AST
1207+
// or `undefined`, so this is the whole test.
1208+
const hasFilter = finalFilter !== undefined;
11921209

11931210
// Window the request only for the flat grid view. Grouped grids and the
11941211
// visual views (kanban/calendar/gantt/gallery) consume the whole batch,
@@ -1750,7 +1767,12 @@ export const ListView = React.forwardRef<ListViewHandle, ListViewProps>(({
17501767
// Server-streamed path: csv / xlsx / json via dataSource.exportDownload.
17511768
// XLSX is server-only; type-aware value formatting, field resolution and
17521769
// permission enforcement all happen server-side. Mirrors the active view's
1753-
// filter + sort so the exported file matches what the user sees.
1770+
// filter + SEARCH + sort so the exported file matches what the user sees.
1771+
//
1772+
// The search half was missing, and this comment asserted the match anyway:
1773+
// exporting during a search downloaded the unsearched superset. The client
1774+
// fallback below was always right (it serializes `data`, already searched);
1775+
// only this path was wrong, and it is the one that handles xlsx.
17541776
const serverEligible = (format === 'csv' || format === 'xlsx' || format === 'json')
17551777
&& typeof dataSource?.exportDownload === 'function'
17561778
&& !!schema.objectName
@@ -1760,18 +1782,8 @@ export const ListView = React.forwardRef<ListViewHandle, ListViewProps>(({
17601782
.map((f: any) => typeof f === 'string' ? f : (f.name || f.fieldName || f.field))
17611783
.filter(Boolean);
17621784

1763-
// Merge the same filter sources as the data fetch (base + user + conditions).
1764-
const baseFilter = schema.filter || [];
1765-
const userFilter = convertFilterGroupToAST(currentFilters);
1766-
const normalizedUserFilterConditions = normalizeFilters(userFilterConditions);
1767-
const allFilters = [
1768-
...(baseFilter.length > 0 ? [baseFilter] : []),
1769-
...(userFilter.length > 0 ? [userFilter] : []),
1770-
...normalizedUserFilterConditions,
1771-
].filter((f: any) => Array.isArray(f) && f.length > 0);
1772-
const finalFilter = allFilters.length > 1
1773-
? ['and', ...allFilters]
1774-
: allFilters.length === 1 ? allFilters[0] : undefined;
1785+
// The same three filter sources as the data fetch, from the same function.
1786+
const finalFilter = buildEffectiveFilter(schema.filter, currentFilters, userFilterConditions);
17751787

17761788
const sort = currentSort.length > 0
17771789
? currentSort
@@ -1787,6 +1799,16 @@ export const ListView = React.forwardRef<ListViewHandle, ListViewProps>(({
17871799
format: format as 'csv' | 'xlsx' | 'json',
17881800
fields: fields.length ? fields : undefined,
17891801
filter: finalFilter,
1802+
// The other half of what the list is showing. Without it an export
1803+
// taken during a search returned the UNSEARCHED superset — more rows
1804+
// than the screen, in a file that looks authoritative. Needs a server
1805+
// with objectstack#4230; older ones ignore it and behave as before.
1806+
...(searchTerm ? {
1807+
search: searchTerm,
1808+
...(schema.searchableFields && schema.searchableFields.length > 0
1809+
? { searchFields: schema.searchableFields }
1810+
: {}),
1811+
} : {}),
17901812
sort,
17911813
includeHeaders,
17921814
limit: maxRecords > 0 ? maxRecords : undefined,
@@ -1861,7 +1883,9 @@ export const ListView = React.forwardRef<ListViewHandle, ListViewProps>(({
18611883
URL.revokeObjectURL(url);
18621884
}
18631885
setShowExport(false);
1864-
}, [data, effectiveFields, resolvedExportOptions, schema.objectName, schema.filter, exportPermitted, dataSource, currentFilters, userFilterConditions, currentSort, objectDef, resolveObjectLabel]);
1886+
// `searchTerm` / `searchableFields` belong here: the export now narrows by
1887+
// the active search, so a stale closure would export the wrong row set.
1888+
}, [data, effectiveFields, resolvedExportOptions, schema.objectName, schema.filter, schema.searchableFields, exportPermitted, dataSource, currentFilters, userFilterConditions, currentSort, searchTerm, objectDef, resolveObjectLabel]);
18651889

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

0 commit comments

Comments
 (0)