Skip to content

Commit f0d6594

Browse files
os-zhuangclaude
andauthored
fix(rest): GET /data/:object/export honours a search term (#4230)
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. 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 (#3948, #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=<term>` — folded into findData as `$search`, so it COMPOSES with `filter` (`{ $and: [filter, search] }`) rather than replacing it. Empty or whitespace-only terms are ignored, not 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 sending `search` to an older server gets today's behaviour rather than an error. Tested against the real engine + protocol in export-integration.test.ts. The composition case is built so each half ALONE returns a different non-empty result and only "both applied" returns none — a case where the two agree would pass just as well with `search` dropped. Reverting rest-server.ts fails 4 of the tests. The file's in-memory driver also learned `$or` / `$contains`: `$search` folds to `{ $or: [{ field: { $contains: term } }] }`, and a driver that skips `$`-prefixed keys returns every row — so an "it filtered" assertion would have passed for the wrong reason. Note: `packages/rest/src/package-routes.ts` has two pre-existing TS2345 errors on main (verified against a clean tree); untouched here. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 42eeb7d commit f0d6594

3 files changed

Lines changed: 146 additions & 0 deletions

File tree

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
---
2+
"@objectstack/rest": patch
3+
---
4+
5+
fix(rest): `GET /data/:object/export` honours a `search` term
6+
7+
The streaming export route accepted `filter` and `orderby` but had no way to
8+
carry the term a user had typed into the list's search box. So exporting after
9+
a search downloaded the **unsearched superset** — more rows than the screen
10+
showed, in a file that looks authoritative, with nothing indicating the
11+
difference. The route's own comment claimed the opposite: that it "mirrors the
12+
active view's filter + sort so the exported file matches what the user sees".
13+
14+
Same family as a dropped filter (objectstack#3948, objectstack#4181): a
15+
plausible answer that is quietly broader than the one asked for.
16+
17+
Two new query params, both matching the list endpoint's semantics:
18+
19+
- `search=<term>` — folded into `findData` as `$search`, so it **composes**
20+
with `filter` (`{ $and: [filter, search] }`) rather than replacing it. Empty
21+
or whitespace-only terms are ignored rather than applied as a blank predicate.
22+
- `searchFields=a,b` — the ADR-0061 override for which fields the term scans.
23+
Only meaningful alongside `search`, and intersected with the object's allowed
24+
searchable set by the engine, exactly as on the list endpoint.
25+
26+
Unknown query params on this route were already ignored, so a client that sends
27+
`search` to an older server gets today's behaviour rather than an error.
28+
29+
Covered by `export-integration.test.ts` against the real engine + protocol: the
30+
composition case is built so each half alone returns a different non-empty
31+
result and only "both applied" returns none. Reverting the route change fails 4
32+
of the tests. The file's in-memory driver also learned `$or` / `$contains`
33+
without them a search predicate is a silent no-op and an "it filtered"
34+
assertion would pass for the wrong reason.

packages/rest/src/export-integration.test.ts

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,20 @@ function makeMemoryDriver() {
4545
if ('$in' in c) return Array.isArray(c.$in) && c.$in.some((x) => (cell ?? null) === (x ?? null));
4646
if ('$eq' in c) return (cell ?? null) === ((c.$eq as unknown) ?? null);
4747
if ('$ne' in c) return (cell ?? null) !== ((c.$ne as unknown) ?? null);
48+
// `$search` folds to `{ $or: [{ field: { $contains: term } }] }`, so the
49+
// driver must understand `$contains` or a search predicate is a no-op and
50+
// an "it filtered" assertion passes for the wrong reason.
51+
if ('$contains' in c) return String(cell ?? '').includes(String(c.$contains ?? ''));
4852
}
4953
return (cell ?? null) === ((cond as unknown) ?? null);
5054
};
5155
const matches = (row: Record<string, unknown>, where: any): boolean => {
5256
if (!where || typeof where !== 'object') return true;
5357
for (const [k, v] of Object.entries(where)) {
58+
// Logical nodes — the shape `$search` and a composed `filter` produce.
59+
// Skipping them (as this driver used to) silently returns every row.
60+
if (k === '$or') { if (!(Array.isArray(v) && v.some((sub) => matches(row, sub)))) return false; continue; }
61+
if (k === '$and') { if (!(Array.isArray(v) && v.every((sub) => matches(row, sub)))) return false; continue; }
5462
if (k.startsWith('$')) continue;
5563
if (!matchOne(row[k], v)) return false;
5664
}
@@ -571,3 +579,83 @@ describe('export route — FLS column projection via getReadableFields (#3547)',
571579
expect(ws.rowCount).toBe(1); // header only
572580
});
573581
});
582+
583+
/**
584+
* `search` — the half of a list this route could not mirror.
585+
*
586+
* The route accepted `filter` and `orderby` but had no way to carry the term a
587+
* user had typed into the list's search box, and `ExportDownloadRequest` had no
588+
* field for one. So "export" after a search downloaded the UNSEARCHED superset:
589+
* more rows than the screen showed, in a file that looks authoritative, with
590+
* nothing anywhere saying so. The route comment claimed the opposite — that the
591+
* export "matches what the user sees".
592+
*
593+
* Same family as a dropped filter (objectstack#3948, #4181): a plausible answer
594+
* that is quietly broader than the one asked for.
595+
*/
596+
describe('export route — search', () => {
597+
let route: any;
598+
599+
beforeEach(async () => {
600+
({ route } = await boot());
601+
});
602+
603+
const csvRows = async (query: Record<string, unknown>) => {
604+
const { res, chunks } = makeRes();
605+
await route.handler({ params: { object: 'task' }, query } as any, res);
606+
return parseCsv(chunks.join('')).slice(1); // drop header
607+
};
608+
609+
it('narrows the exported rows to the search term', async () => {
610+
const rows = await csvRows({ format: 'csv', search: '代码' });
611+
expect(rows.map((r) => r[1])).toEqual(['写代码']);
612+
});
613+
614+
it('exports everything when no term is given (unchanged behaviour)', async () => {
615+
expect((await csvRows({ format: 'csv' })).length).toBe(2);
616+
});
617+
618+
it('composes with `filter` — both halves apply, neither replaces the other', async () => {
619+
// Chosen so each half ALONE gives a different non-empty answer, and only
620+
// "both applied" gives none. A test where the two agree would pass just as
621+
// well with `search` dropped entirely.
622+
const onlyFilter = await csvRows({ format: 'csv', filter: JSON.stringify(['done', '=', true]) });
623+
expect(onlyFilter.map((r) => r[1])).toEqual(['写代码']);
624+
const onlySearch = await csvRows({ format: 'csv', search: '文档' });
625+
expect(onlySearch.map((r) => r[1])).toEqual(['写文档']);
626+
627+
// Disjoint, so the intersection is empty — which it can only be if BOTH
628+
// reached the engine. Dropping either one yields a row.
629+
const both = await csvRows({
630+
format: 'csv',
631+
filter: JSON.stringify(['done', '=', true]),
632+
search: '文档',
633+
});
634+
expect(both.length).toBe(0);
635+
});
636+
637+
it('an empty or whitespace term is ignored, not applied as a blank predicate', async () => {
638+
expect((await csvRows({ format: 'csv', search: '' })).length).toBe(2);
639+
expect((await csvRows({ format: 'csv', search: ' ' })).length).toBe(2);
640+
});
641+
642+
it('honours a `searchFields` override that excludes the matching column', async () => {
643+
// TASK's auto-default searchable set is { title, priority } (text + select).
644+
// `高` is the label of priority=high, so by default it finds 写代码 …
645+
expect((await csvRows({ format: 'csv', search: '高' })).map((r) => r[1])).toEqual(['写代码']);
646+
// … and restricting the scan to `title` finds nothing, which is only true if
647+
// the override actually reached the engine (ADR-0061).
648+
expect((await csvRows({ format: 'csv', search: '高', searchFields: 'title' })).length).toBe(0);
649+
});
650+
651+
it('applies to xlsx too, not just the csv path', async () => {
652+
const { res, getBuffer } = makeBinRes();
653+
await route.handler(
654+
{ params: { object: 'task' }, query: { format: 'xlsx', search: '代码' } } as any,
655+
res,
656+
);
657+
const wb = new ExcelJS.Workbook();
658+
await wb.xlsx.load(getBuffer() as any);
659+
expect(wb.worksheets[0].rowCount).toBe(2); // header + one match
660+
});
661+
});

packages/rest/src/rest-server.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4666,6 +4666,9 @@ export class RestServer {
46664666
// format=csv|json|xlsx (default: csv. json emits a JSON array, xlsx a workbook.)
46674667
// fields=a,b,c (default: derive from object schema; falls back to keys of the first row)
46684668
// filter=<json> ($filter as URL-encoded JSON, same shape as list endpoint)
4669+
// search=<term> (full-text term, same semantics as the list endpoint's
4670+
// $search; composes with `filter` rather than replacing it)
4671+
// searchFields=a,b (optional ADR-0061 override for which fields `search` scans)
46694672
// orderby=field:desc (optional ordering, mirrors $orderby semantics)
46704673
// header=false (omit the header row for csv / xlsx; default true)
46714674
// limit=<n> (default 10000, hard cap 50000)
@@ -4740,6 +4743,25 @@ export class RestServer {
47404743
filter = q.filter;
47414744
}
47424745

4746+
// Full-text term, same semantics as the list endpoint's `$search`.
4747+
// Without it this route could only ever mirror the FILTER half of a
4748+
// list, so a user who searched and then exported downloaded the
4749+
// unsearched superset — more rows than the screen showed, with
4750+
// nothing to indicate it. `$search` composes with `$filter` inside
4751+
// `findData`, so both halves apply.
4752+
const search = typeof q.search === 'string' && q.search.trim().length > 0
4753+
? q.search.trim()
4754+
: undefined;
4755+
// ADR-0061 override for which fields the term scans. Only meaningful
4756+
// alongside `search`; ignored on its own, exactly as in findData.
4757+
let searchFields: string[] | undefined;
4758+
if (typeof q.searchFields === 'string' && q.searchFields.length > 0) {
4759+
searchFields = q.searchFields.split(',').map((s: string) => s.trim()).filter(Boolean);
4760+
} else if (Array.isArray(q.searchFields)) {
4761+
searchFields = q.searchFields.filter((s: any) => typeof s === 'string' && s.length > 0);
4762+
}
4763+
if (searchFields && searchFields.length === 0) searchFields = undefined;
4764+
47434765
let orderby: any = undefined;
47444766
if (typeof q.orderby === 'string' && q.orderby.length > 0) {
47454767
// Accept "field:dir,field2:dir" shorthand or a JSON object.
@@ -4880,6 +4902,8 @@ export class RestServer {
48804902
object: objectName,
48814903
query: {
48824904
...(filter ? { $filter: filter } : {}),
4905+
...(search ? { $search: search } : {}),
4906+
...(search && searchFields ? { $searchFields: searchFields } : {}),
48834907
...(orderby ? { $orderby: orderby } : {}),
48844908
...(expandFields.length > 0 ? { $expand: expandFields.join(',') } : {}),
48854909
$top: take,

0 commit comments

Comments
 (0)