From b7c3569c7a632f76ff5c2e419d3197697f02b549 Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Fri, 31 Jul 2026 00:52:54 +0800 Subject: [PATCH] fix(list,data-objectstack,types): exporting a searched list no longer downloads the unsearched superset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: Claude Opus 5 --- .../export-matches-the-searched-view.md | 43 ++++++ .../src/exportDownload.test.ts | 42 ++++++ packages/data-objectstack/src/index.ts | 10 ++ packages/plugin-list/src/ListView.tsx | 108 ++++++++------ .../ListView.exportMatchesView.test.tsx | 141 ++++++++++++++++++ packages/types/src/data.ts | 17 ++- 6 files changed, 317 insertions(+), 44 deletions(-) create mode 100644 .changeset/export-matches-the-searched-view.md create mode 100644 packages/plugin-list/src/__tests__/ListView.exportMatchesView.test.tsx diff --git a/.changeset/export-matches-the-searched-view.md b/.changeset/export-matches-the-searched-view.md new file mode 100644 index 0000000000..e88674e64e --- /dev/null +++ b/.changeset/export-matches-the-searched-view.md @@ -0,0 +1,43 @@ +--- +"@object-ui/types": patch +"@object-ui/data-objectstack": patch +"@object-ui/plugin-list": patch +--- + +fix(list,data-objectstack,types): exporting a searched list no longer downloads the unsearched superset + +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`. This is a pure extraction: the copies did +agree, and the four parity tests added for it pass against the old code too. +They exist to keep it that way — the adapter's duplicated filter-shape check +had already drifted apart unnoticed (#3072). diff --git a/packages/data-objectstack/src/exportDownload.test.ts b/packages/data-objectstack/src/exportDownload.test.ts index eb700003b9..429d9724ad 100644 --- a/packages/data-objectstack/src/exportDownload.test.ts +++ b/packages/data-objectstack/src/exportDownload.test.ts @@ -97,3 +97,45 @@ describe('ObjectStackAdapter.exportDownload', () => { }); }); }); + +/** + * `search` — the half of a list this request could not carry. + * + * The route mirrored `filter` and `orderby` only, so an export taken while a + * search was active downloaded the UNSEARCHED superset: more rows than the + * screen showed, in a file that looks authoritative. Server half: + * objectstack#4230. + */ +describe('ObjectStackAdapter.exportDownload — search', () => { + const paramsFor = async (request: Record) => { + const fetchImpl = vi.fn(async (_url: string, _init: RequestInit) => csvResponse()); + await makeDS(fetchImpl).exportDownload('task', request); + return new URL(fetchImpl.mock.calls[0][0]).searchParams; + }; + + it('sends the term as `search`', async () => { + expect((await paramsFor({ search: 'acme' })).get('search')).toBe('acme'); + }); + + it('sends `searchFields` as a comma list alongside the term', async () => { + const p = await paramsFor({ search: 'acme', searchFields: ['name', 'stage'] }); + expect(p.get('searchFields')).toBe('name,stage'); + }); + + it('omits both when no term is given — `searchFields` alone means nothing', async () => { + const p = await paramsFor({ searchFields: ['name'] }); + expect(p.get('search')).toBeNull(); + expect(p.get('searchFields')).toBeNull(); + }); + + it('treats a whitespace-only term as absent, and trims a real one', async () => { + expect((await paramsFor({ search: ' ' })).get('search')).toBeNull(); + expect((await paramsFor({ search: ' acme ' })).get('search')).toBe('acme'); + }); + + it('carries search and filter together — neither replaces the other', async () => { + const p = await paramsFor({ search: 'acme', filter: [['status', '=', 'open']] }); + expect(p.get('search')).toBe('acme'); + expect(JSON.parse(p.get('filter')!)).toEqual([['status', '=', 'open']]); + }); +}); diff --git a/packages/data-objectstack/src/index.ts b/packages/data-objectstack/src/index.ts index 4a60cce936..67e2426326 100644 --- a/packages/data-objectstack/src/index.ts +++ b/packages/data-objectstack/src/index.ts @@ -2130,6 +2130,16 @@ export class ObjectStackAdapter implements DataSource { queryParams.set('filter', JSON.stringify(translated)); } } + // Search — the other half of what a list is showing. Without it an export + // taken while a search is active returns the unsearched superset + // (objectstack#4230). Servers predating that ignore the param. + const searchTerm = typeof request.search === 'string' ? request.search.trim() : ''; + if (searchTerm) { + queryParams.set('search', searchTerm); + if (request.searchFields && request.searchFields.length > 0) { + queryParams.set('searchFields', request.searchFields.join(',')); + } + } const baseUrl = this.baseUrl.replace(/\/$/, ''); // Avoid doubling /api/v1 if baseUrl already includes the version suffix. diff --git a/packages/plugin-list/src/ListView.tsx b/packages/plugin-list/src/ListView.tsx index c8a34b06dd..d911e6480c 100644 --- a/packages/plugin-list/src/ListView.tsx +++ b/packages/plugin-list/src/ListView.tsx @@ -218,6 +218,42 @@ export function normalizeFilters(filters: any[]): any[] { .filter(f => Array.isArray(f) && f.length > 0); } +/** + * Merge the three filter sources a list can have — the view's own `filter`, the + * filter-panel group, and the per-field user filters — into one AST node. + * + * ONE definition on purpose. The data fetch and the export used to carry + * verbatim copies of this, and those two decide, respectively, **what the user + * sees** and **what they download**. Two copies that must agree, where a + * divergence is a file that silently disagrees with the screen — the same shape + * of bug that had already bitten the adapter, whose duplicated filter-shape + * check drifted apart unnoticed (#3072). + * + * 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. + */ +export function buildEffectiveFilter( + baseFilter: unknown, + currentFilters: FilterGroup, + userFilterConditions: any[], +): any[] | undefined { + const base = Array.isArray(baseFilter) ? baseFilter : []; + 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 `=`). + ...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[] { if (!group || !group.conditions || group.conditions.length === 0) return []; @@ -1044,28 +1080,10 @@ export const ListView = React.forwardRef(({ setLoading(true); setLoadError(null); try { - // Construct filter - let finalFilter: any = []; - const baseFilter = schema.filter || []; - const userFilter = convertFilterGroupToAST(currentFilters); - - - // Normalize userFilter conditions (convert `in` to `or` of `=`) - const normalizedUserFilterConditions = normalizeFilters(userFilterConditions); - - // Merge all filter sources with consistent structure - const allFilters = [ - ...(baseFilter.length > 0 ? [baseFilter] : []), - ...(userFilter.length > 0 ? [userFilter] : []), - ...normalizedUserFilterConditions, - ].filter(f => Array.isArray(f) && f.length > 0); - - if (allFilters.length > 1) { - finalFilter = ['and', ...allFilters]; - } else if (allFilters.length === 1) { - finalFilter = allFilters[0]; - } - + // Construct filter — shared with the export path so the file a user + // downloads is built from the same three sources as the rows on screen. + const finalFilter = buildEffectiveFilter(schema.filter, currentFilters, userFilterConditions); + // Convert sort to query format // Use array format to ensure order is preserved (Object keys are not guaranteed ordered) const sort: any = currentSort.length > 0 @@ -1183,12 +1201,11 @@ export const ListView = React.forwardRef(({ return Array.from(required); })(); - // Only send $filter when it is a non-empty AST. Sending an empty - // array results in `?filter=%5B%5D` which is wasted bandwidth and - // can defeat server-side query parsing/caching. - const hasFilter = Array.isArray(finalFilter) - ? finalFilter.length > 0 - : !!finalFilter && Object.keys(finalFilter).length > 0; + // Only send $filter when there is one. Sending an empty array results in + // `?filter=%5B%5D` which is wasted bandwidth and can defeat server-side + // query parsing/caching. `buildEffectiveFilter` returns a non-empty AST + // or `undefined`, so this is the whole test. + const hasFilter = finalFilter !== undefined; // Window the request only for the flat grid view. Grouped grids and the // visual views (kanban/calendar/gantt/gallery) consume the whole batch, @@ -1750,7 +1767,12 @@ export const ListView = React.forwardRef(({ // Server-streamed path: csv / xlsx / json via dataSource.exportDownload. // XLSX is server-only; type-aware value formatting, field resolution and // permission enforcement all happen server-side. Mirrors the active view's - // filter + sort so the exported file matches what the user sees. + // filter + SEARCH + sort so the exported file matches what the user sees. + // + // The search half was missing, and this comment asserted the match anyway: + // exporting during a search downloaded the unsearched superset. The client + // fallback below was always right (it serializes `data`, already searched); + // only this path was wrong, and it is the one that handles xlsx. const serverEligible = (format === 'csv' || format === 'xlsx' || format === 'json') && typeof dataSource?.exportDownload === 'function' && !!schema.objectName @@ -1760,18 +1782,8 @@ export const ListView = React.forwardRef(({ .map((f: any) => typeof f === 'string' ? f : (f.name || f.fieldName || f.field)) .filter(Boolean); - // Merge the same filter sources as the data fetch (base + user + conditions). - const baseFilter = schema.filter || []; - const userFilter = convertFilterGroupToAST(currentFilters); - const normalizedUserFilterConditions = normalizeFilters(userFilterConditions); - const allFilters = [ - ...(baseFilter.length > 0 ? [baseFilter] : []), - ...(userFilter.length > 0 ? [userFilter] : []), - ...normalizedUserFilterConditions, - ].filter((f: any) => Array.isArray(f) && f.length > 0); - const finalFilter = allFilters.length > 1 - ? ['and', ...allFilters] - : allFilters.length === 1 ? allFilters[0] : undefined; + // The same three filter sources as the data fetch, from the same function. + const finalFilter = buildEffectiveFilter(schema.filter, currentFilters, userFilterConditions); const sort = currentSort.length > 0 ? currentSort @@ -1787,6 +1799,16 @@ export const ListView = React.forwardRef(({ format: format as 'csv' | 'xlsx' | 'json', fields: fields.length ? fields : undefined, filter: finalFilter, + // The other half of what the list is showing. Without it an export + // taken during a search returned the UNSEARCHED superset — more rows + // than the screen, in a file that looks authoritative. Needs a server + // with objectstack#4230; older ones ignore it and behave as before. + ...(searchTerm ? { + search: searchTerm, + ...(schema.searchableFields && schema.searchableFields.length > 0 + ? { searchFields: schema.searchableFields } + : {}), + } : {}), sort, includeHeaders, limit: maxRecords > 0 ? maxRecords : undefined, @@ -1861,7 +1883,9 @@ export const ListView = React.forwardRef(({ URL.revokeObjectURL(url); } setShowExport(false); - }, [data, effectiveFields, resolvedExportOptions, schema.objectName, schema.filter, exportPermitted, dataSource, currentFilters, userFilterConditions, currentSort, objectDef, resolveObjectLabel]); + // `searchTerm` / `searchableFields` belong here: the export now narrows by + // the active search, so a stale closure would export the wrong row set. + }, [data, effectiveFields, resolvedExportOptions, schema.objectName, schema.filter, schema.searchableFields, exportPermitted, dataSource, currentFilters, userFilterConditions, currentSort, searchTerm, objectDef, resolveObjectLabel]); // All available fields for hide/show (with i18n) const allFields = React.useMemo(() => { diff --git a/packages/plugin-list/src/__tests__/ListView.exportMatchesView.test.tsx b/packages/plugin-list/src/__tests__/ListView.exportMatchesView.test.tsx new file mode 100644 index 0000000000..a77d885717 --- /dev/null +++ b/packages/plugin-list/src/__tests__/ListView.exportMatchesView.test.tsx @@ -0,0 +1,141 @@ +/** + * 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. + */ + +/** + * "Export what the user is looking at." + * + * The server-streamed export mirrored the view's FILTER and sort, and the code + * comment claimed that made the file match the screen. It did not: there was no + * way to carry the search term, and `ExportDownloadRequest` had no field for + * one. Exporting during a search downloaded the UNSEARCHED superset — more rows + * than the list showed, in a file that looks authoritative, with nothing saying + * so. (The client-side fallback was always right; it serializes the already + * searched `data`. Only the server path was wrong — and it is the one that + * handles xlsx.) Server half: objectstack#4230. + * + * The filter half was built twice — once for the fetch, once for the export — + * as verbatim copies that had to stay in sync. Those two decide what the user + * SEES and what they DOWNLOAD, so the parity test below compares the real + * arguments of the two real calls rather than asserting a shape twice. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, fireEvent, screen } from '@testing-library/react'; +import { ListView } from '../ListView'; +import { SchemaRendererProvider } from '@object-ui/react'; +import type { ListViewSchema } from '@object-ui/types'; + +const RECORDS = [{ id: '1', name: 'Acme', stage: 'won' }]; + +function harness(schema: ListViewSchema, props: Record = {}) { + const exportDownload = vi.fn().mockResolvedValue(new Blob(['x'], { type: 'text/csv' })); + const find = vi.fn().mockResolvedValue({ data: RECORDS, total: 1 }); + const ds: any = { find, findOne: vi.fn(), create: vi.fn(), update: vi.fn(), delete: vi.fn(), exportDownload }; + render( + + + , + ); + return { exportDownload, find }; +} + +async function clickExportCsv() { + fireEvent.click(screen.getByRole('button', { name: /export/i })); + fireEvent.click(await screen.findByRole('button', { name: /export as csv/i })); +} + +const BASE: ListViewSchema = { + type: 'list-view', + objectName: 'account', + viewType: 'grid', + fields: ['name'], + exportOptions: { formats: ['csv'] }, +}; + +describe('ListView export mirrors the active view', () => { + beforeEach(() => { + vi.spyOn(URL, 'createObjectURL').mockReturnValue('blob:x'); + vi.spyOn(URL, 'revokeObjectURL').mockImplementation(() => {}); + }); + afterEach(() => vi.restoreAllMocks()); + + it('carries the active search term', async () => { + const { exportDownload } = harness(BASE, { initialSearchTerm: 'acme' }); + await clickExportCsv(); + await vi.waitFor(() => expect(exportDownload).toHaveBeenCalledTimes(1)); + expect(exportDownload.mock.calls[0][1]).toMatchObject({ search: 'acme' }); + }); + + it('forwards the view’s searchableFields alongside the term', async () => { + const { exportDownload } = harness( + { ...BASE, searchableFields: ['name', 'stage'] }, + { initialSearchTerm: 'acme' }, + ); + await clickExportCsv(); + await vi.waitFor(() => expect(exportDownload).toHaveBeenCalledTimes(1)); + expect(exportDownload.mock.calls[0][1]).toMatchObject({ + search: 'acme', + searchFields: ['name', 'stage'], + }); + }); + + it('sends no search key when nothing is searched', async () => { + const { exportDownload } = harness(BASE); + await clickExportCsv(); + await vi.waitFor(() => expect(exportDownload).toHaveBeenCalledTimes(1)); + expect(exportDownload.mock.calls[0][1]).not.toHaveProperty('search'); + expect(exportDownload.mock.calls[0][1]).not.toHaveProperty('searchFields'); + }); + + it('omits searchFields when the view declares none', async () => { + const { exportDownload } = harness(BASE, { initialSearchTerm: 'acme' }); + await clickExportCsv(); + await vi.waitFor(() => expect(exportDownload).toHaveBeenCalledTimes(1)); + expect(exportDownload.mock.calls[0][1]).not.toHaveProperty('searchFields'); + }); +}); + +describe('the fetch and the export build the SAME filter', () => { + beforeEach(() => { + vi.spyOn(URL, 'createObjectURL').mockReturnValue('blob:x'); + vi.spyOn(URL, 'revokeObjectURL').mockImplementation(() => {}); + }); + afterEach(() => vi.restoreAllMocks()); + + /** + * Derived, not restated: compare what the two real calls actually carried. + * An assertion that spelled out the expected AST twice would go on passing if + * both copies drifted the same wrong way. + */ + const cases: Array<[string, ListViewSchema]> = [ + ['a view filter alone', { ...BASE, filter: [{ field: 'stage', operator: 'eq', value: 'won' }] as any }], + ['a multi-rule view filter', { + ...BASE, + filter: [ + { field: 'stage', operator: 'eq', value: 'won' }, + { field: 'amount', operator: 'gt', value: 10 }, + ] as any, + }], + ['an AST-shaped view filter', { ...BASE, filter: [['stage', '=', 'won']] as any }], + ['no filter at all', BASE], + ]; + + for (const [name, schema] of cases) { + it(`agree for ${name}`, async () => { + const { exportDownload, find } = harness(schema); + await vi.waitFor(() => expect(find).toHaveBeenCalled()); + const fetched = find.mock.calls[0][1]?.$filter; + + await clickExportCsv(); + await vi.waitFor(() => expect(exportDownload).toHaveBeenCalledTimes(1)); + const exported = exportDownload.mock.calls[0][1]?.filter; + + expect(exported).toEqual(fetched); + }); + } +}); diff --git a/packages/types/src/data.ts b/packages/types/src/data.ts index 9289a263dc..c6cdac3411 100644 --- a/packages/types/src/data.ts +++ b/packages/types/src/data.ts @@ -1007,8 +1007,13 @@ export interface ListImportJobsOptions { /** * Request payload for `DataSource.exportDownload` (synchronous streamed export). * - * Mirrors the active list view: pass the same `filter` / `sort` the list is - * showing so the exported file matches what the user sees. + * Mirrors the active list view: pass the same `filter` / `search` / `sort` the + * list is showing so the exported file matches what the user sees. + * + * `search` was missing until objectstack#4230, and the omission was not + * visible: exporting a searched list quietly produced the UNSEARCHED superset — + * more rows than the screen showed, in a file that looks authoritative. A + * caller that mirrors only `filter` still has that bug. */ export interface ExportDownloadRequest { /** Output file format. Defaults to 'csv'. */ @@ -1017,6 +1022,14 @@ export interface ExportDownloadRequest { fields?: string[]; /** Server-side filter (engine-specific shape, often the active view filter). */ filter?: unknown; + /** + * Full-text term, same semantics as the list read's `$search`. Composes with + * `filter` server-side rather than replacing it. Requires a server with + * objectstack#4230; older servers ignore it (and export the wider set). + */ + search?: string; + /** Optional override for which fields `search` scans (ADR-0061). */ + searchFields?: string[]; /** Sort instructions; multiple keys allowed, order preserved. */ sort?: Array<{ field: string; direction?: 'asc' | 'desc' }>; /** Hard cap on records exported (server enforces its own ceiling too). */