diff --git a/.changeset/export-honors-search-term.md b/.changeset/export-honors-search-term.md new file mode 100644 index 0000000000..572066c0b8 --- /dev/null +++ b/.changeset/export-honors-search-term.md @@ -0,0 +1,34 @@ +--- +"@objectstack/rest": patch +--- + +fix(rest): `GET /data/:object/export` honours a `search` term + +The streaming export route accepted `filter` and `orderby` but had no way to +carry the term a user had typed into the list's search box. So exporting after +a search downloaded the **unsearched superset** — more rows than the screen +showed, in a file that looks authoritative, with nothing indicating the +difference. The route's own comment claimed the opposite: that it "mirrors the +active view's filter + sort so the exported file matches what the user sees". + +Same family as a dropped filter (objectstack#3948, objectstack#4181): a +plausible answer that is quietly broader than the one asked for. + +Two new query params, both matching the list endpoint's semantics: + +- `search=` — folded into `findData` as `$search`, so it **composes** + with `filter` (`{ $and: [filter, search] }`) rather than replacing it. Empty + or whitespace-only terms are ignored rather than applied as a blank predicate. +- `searchFields=a,b` — the ADR-0061 override for which fields the term scans. + Only meaningful alongside `search`, and intersected with the object's allowed + searchable set by the engine, exactly as on the list endpoint. + +Unknown query params on this route were already ignored, so a client that sends +`search` to an older server gets today's behaviour rather than an error. + +Covered by `export-integration.test.ts` against the real engine + protocol: the +composition case is built so each half alone returns a different non-empty +result and only "both applied" returns none. Reverting the route change fails 4 +of the tests. The file's in-memory driver also learned `$or` / `$contains` — +without them a search predicate is a silent no-op and an "it filtered" +assertion would pass for the wrong reason. diff --git a/packages/rest/src/export-integration.test.ts b/packages/rest/src/export-integration.test.ts index d3407300b8..edd38f499f 100644 --- a/packages/rest/src/export-integration.test.ts +++ b/packages/rest/src/export-integration.test.ts @@ -45,12 +45,20 @@ function makeMemoryDriver() { if ('$in' in c) return Array.isArray(c.$in) && c.$in.some((x) => (cell ?? null) === (x ?? null)); if ('$eq' in c) return (cell ?? null) === ((c.$eq as unknown) ?? null); if ('$ne' in c) return (cell ?? null) !== ((c.$ne as unknown) ?? null); + // `$search` folds to `{ $or: [{ field: { $contains: term } }] }`, so the + // driver must understand `$contains` or a search predicate is a no-op and + // an "it filtered" assertion passes for the wrong reason. + if ('$contains' in c) return String(cell ?? '').includes(String(c.$contains ?? '')); } return (cell ?? null) === ((cond as unknown) ?? null); }; const matches = (row: Record, where: any): boolean => { if (!where || typeof where !== 'object') return true; for (const [k, v] of Object.entries(where)) { + // Logical nodes — the shape `$search` and a composed `filter` produce. + // Skipping them (as this driver used to) silently returns every row. + if (k === '$or') { if (!(Array.isArray(v) && v.some((sub) => matches(row, sub)))) return false; continue; } + if (k === '$and') { if (!(Array.isArray(v) && v.every((sub) => matches(row, sub)))) return false; continue; } if (k.startsWith('$')) continue; if (!matchOne(row[k], v)) return false; } @@ -571,3 +579,83 @@ describe('export route — FLS column projection via getReadableFields (#3547)', expect(ws.rowCount).toBe(1); // header only }); }); + +/** + * `search` — the half of a list this route could not mirror. + * + * The route accepted `filter` and `orderby` but had no way to carry the term a + * user had typed into the list's search box, and `ExportDownloadRequest` had no + * field for one. So "export" after a search downloaded the UNSEARCHED superset: + * more rows than the screen showed, in a file that looks authoritative, with + * nothing anywhere saying so. The route comment claimed the opposite — that the + * export "matches what the user sees". + * + * Same family as a dropped filter (objectstack#3948, #4181): a plausible answer + * that is quietly broader than the one asked for. + */ +describe('export route — search', () => { + let route: any; + + beforeEach(async () => { + ({ route } = await boot()); + }); + + const csvRows = async (query: Record) => { + const { res, chunks } = makeRes(); + await route.handler({ params: { object: 'task' }, query } as any, res); + return parseCsv(chunks.join('')).slice(1); // drop header + }; + + it('narrows the exported rows to the search term', async () => { + const rows = await csvRows({ format: 'csv', search: '代码' }); + expect(rows.map((r) => r[1])).toEqual(['写代码']); + }); + + it('exports everything when no term is given (unchanged behaviour)', async () => { + expect((await csvRows({ format: 'csv' })).length).toBe(2); + }); + + it('composes with `filter` — both halves apply, neither replaces the other', async () => { + // Chosen so each half ALONE gives a different non-empty answer, and only + // "both applied" gives none. A test where the two agree would pass just as + // well with `search` dropped entirely. + const onlyFilter = await csvRows({ format: 'csv', filter: JSON.stringify(['done', '=', true]) }); + expect(onlyFilter.map((r) => r[1])).toEqual(['写代码']); + const onlySearch = await csvRows({ format: 'csv', search: '文档' }); + expect(onlySearch.map((r) => r[1])).toEqual(['写文档']); + + // Disjoint, so the intersection is empty — which it can only be if BOTH + // reached the engine. Dropping either one yields a row. + const both = await csvRows({ + format: 'csv', + filter: JSON.stringify(['done', '=', true]), + search: '文档', + }); + expect(both.length).toBe(0); + }); + + it('an empty or whitespace term is ignored, not applied as a blank predicate', async () => { + expect((await csvRows({ format: 'csv', search: '' })).length).toBe(2); + expect((await csvRows({ format: 'csv', search: ' ' })).length).toBe(2); + }); + + it('honours a `searchFields` override that excludes the matching column', async () => { + // TASK's auto-default searchable set is { title, priority } (text + select). + // `高` is the label of priority=high, so by default it finds 写代码 … + expect((await csvRows({ format: 'csv', search: '高' })).map((r) => r[1])).toEqual(['写代码']); + // … and restricting the scan to `title` finds nothing, which is only true if + // the override actually reached the engine (ADR-0061). + expect((await csvRows({ format: 'csv', search: '高', searchFields: 'title' })).length).toBe(0); + }); + + it('applies to xlsx too, not just the csv path', async () => { + const { res, getBuffer } = makeBinRes(); + await route.handler( + { params: { object: 'task' }, query: { format: 'xlsx', search: '代码' } } as any, + res, + ); + const wb = new ExcelJS.Workbook(); + await wb.xlsx.load(getBuffer() as any); + expect(wb.worksheets[0].rowCount).toBe(2); // header + one match + }); +}); diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index a87303fa35..33ff4f3d9d 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -4666,6 +4666,9 @@ export class RestServer { // format=csv|json|xlsx (default: csv. json emits a JSON array, xlsx a workbook.) // fields=a,b,c (default: derive from object schema; falls back to keys of the first row) // filter= ($filter as URL-encoded JSON, same shape as list endpoint) + // search= (full-text term, same semantics as the list endpoint's + // $search; composes with `filter` rather than replacing it) + // searchFields=a,b (optional ADR-0061 override for which fields `search` scans) // orderby=field:desc (optional ordering, mirrors $orderby semantics) // header=false (omit the header row for csv / xlsx; default true) // limit= (default 10000, hard cap 50000) @@ -4740,6 +4743,25 @@ export class RestServer { filter = q.filter; } + // Full-text term, same semantics as the list endpoint's `$search`. + // Without it this route could only ever mirror the FILTER half of a + // list, so a user who searched and then exported downloaded the + // unsearched superset — more rows than the screen showed, with + // nothing to indicate it. `$search` composes with `$filter` inside + // `findData`, so both halves apply. + const search = typeof q.search === 'string' && q.search.trim().length > 0 + ? q.search.trim() + : undefined; + // ADR-0061 override for which fields the term scans. Only meaningful + // alongside `search`; ignored on its own, exactly as in findData. + let searchFields: string[] | undefined; + if (typeof q.searchFields === 'string' && q.searchFields.length > 0) { + searchFields = q.searchFields.split(',').map((s: string) => s.trim()).filter(Boolean); + } else if (Array.isArray(q.searchFields)) { + searchFields = q.searchFields.filter((s: any) => typeof s === 'string' && s.length > 0); + } + if (searchFields && searchFields.length === 0) searchFields = undefined; + let orderby: any = undefined; if (typeof q.orderby === 'string' && q.orderby.length > 0) { // Accept "field:dir,field2:dir" shorthand or a JSON object. @@ -4880,6 +4902,8 @@ export class RestServer { object: objectName, query: { ...(filter ? { $filter: filter } : {}), + ...(search ? { $search: search } : {}), + ...(search && searchFields ? { $searchFields: searchFields } : {}), ...(orderby ? { $orderby: orderby } : {}), ...(expandFields.length > 0 ? { $expand: expandFields.join(',') } : {}), $top: take,