Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/report-export-column-inference-all-rows.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
---
"@objectstack/plugin-reports": patch
---

Report CSV and HTML exports now infer their columns from **every** row of the result set, not from the first 50. Column inference sampled `rows.slice(0, 50)` while the projection it produced was applied to all rows, so when a report declared no explicit `query.fields`, any key whose first occurrence fell at row 51 or later was absent from the header *and* dropped from every row that carried it. The export gave no signal: the CSV was well-formed, every row had the same arity, and nothing marked a column as inferred rather than declared, so a recipient of a scheduled report attachment could not tell. Sparse columns are the normal shape of report output — an optional field, a formula only some records satisfy, a lookup that resolves for a subset — and the sampled prefix is the query's first page in its own `orderBy`, so for a report sorted by status or created date the sample correlated with exactly the column it dropped. Both affected renderers are fixed (`csv`, which is also the `default:` format branch, and `html_table`); `json` was never affected. Already-inferred columns keep their position and late-appearing ones are appended, so an export that was correct before is byte-identical now.
81 changes: 79 additions & 2 deletions packages/plugins/plugin-reports/src/report-service.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -98,9 +98,65 @@ describe('renderReport', () => {
expect(JSON.parse(out)).toEqual([{ a: 1 }]);
});

it('auto-detects fields from first 50 rows when none specified', () => {
it('auto-detects fields from the rows when none specified', () => {
const out = renderReport([{ a: 1 }, { b: 2 }], 'csv');
expect(out.split('\r\n')[0]).toMatch(/a|b/);
expect(out.split('\r\n')[0].split(',')).toEqual(['a', 'b']);
});

// [#11774] Column inference sampled only `rows.slice(0, 50)` while the
// projection was applied to ALL rows, so a key whose first occurrence was at
// row 51+ never reached the header and its values were dropped from every
// row that carried it - a well-formed CSV of uniform arity with nothing
// marking the loss. Sparse columns are the normal shape of report output,
// and the first 50 rows are the query's first page IN ITS SORT ORDER, so
// the sample is precisely the correlated case.
describe('a column whose first row is past the old 50-row sample boundary', () => {
const LATE_ROW = 55;
const LATE_COL = 'escalation_note';
const LATE_VALUE = 'breached SLA';

/** 60 rows sharing `id` + `status`; only row 55 carries LATE_COL. */
function sparseRows(): Array<Record<string, unknown>> {
return Array.from({ length: 60 }, (_v, i) => ({
id: `l${String(i).padStart(2, '0')}`,
status: 'open',
...(i === LATE_ROW ? { [LATE_COL]: LATE_VALUE } : {}),
}));
}

it('csv: reaches the header, and its value reaches the row that carries it', () => {
const out = renderReport(sparseRows(), 'csv');
const lines = out.split('\r\n');
const header = lines[0].split(',');

expect(header).toContain(LATE_COL);
// Identity and order, not arity: already-inferred columns keep their
// position and the late one is appended.
expect(header).toEqual(['id', 'status', LATE_COL]);

// The value arrives, under its own column - `toContain(LATE_COL)` alone
// would pass on an all-empty column.
const col = header.indexOf(LATE_COL);
expect(lines[1 + LATE_ROW].split(',')[col]).toBe(LATE_VALUE);
// ...and only in that row.
expect(lines[1].split(',')[col]).toBe('');
});

it('html_table: reaches the header, and its value reaches the row that carries it', () => {
const out = renderReport(sparseRows(), 'html_table');
// `<th[^>]*>` would also match `<thead>` and swallow the first cell —
// require a space or the closing angle right after the tag name.
const header = [...out.matchAll(/<th(?:\s[^>]*)?>(.*?)<\/th>/g)].map(m => m[1]);
const body = [...out.matchAll(/<tr>((?:<td[^>]*>.*?<\/td>)+)<\/tr>/g)]
.map(m => [...m[1].matchAll(/<td[^>]*>(.*?)<\/td>/g)].map(c => c[1]));

expect(header).toContain(LATE_COL);
expect(header).toEqual(['id', 'status', LATE_COL]);

const col = header.indexOf(LATE_COL);
expect(body[LATE_ROW][col]).toBe(LATE_VALUE);
expect(body[0][col]).toBe('');
});
});
});

Expand DownExpand Up@@ -210,6 +266,27 @@ describe('ReportService', () => {
expect(stored.last_run_at).toBe(now.toISOString());
});

it('run: a field first appearing past row 50 still reaches the export body', async () => {
// [#11774] The scheduled-attachment path end to end: the saved report
// declares no `query.fields`, so the header is inferred from the result
// set. Only row 55 carries `escalation_note`.
engine._tables['lead'] = Array.from({ length: 60 }, (_v, i) => ({
id: `l${String(i).padStart(2, '0')}`,
status: 'open',
...(i === 55 ? { escalation_note: 'breached SLA' } : {}),
}));
const r = await svc.saveReport({
name: 'Open', object: 'lead', query: { filter: { status: 'open' } }, format: 'csv',
}, CTX);
const result = await svc.run(r.id, CTX);

expect(result.rowCount).toBe(60);
const lines = result.body.split('\r\n');
const header = lines[0].split(',');
expect(header).toContain('escalation_note');
expect(lines[1 + 55].split(',')[header.indexOf('escalation_note')]).toBe('breached SLA');
});

it('run: throws REPORT_NOT_FOUND for unknown id', async () => {
await expect(svc.run('nope', CTX)).rejects.toThrow(/REPORT_NOT_FOUND/);
});
Expand Down
20 changes: 19 additions & 1 deletion packages/plugins/plugin-reports/src/report-service.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -118,10 +118,28 @@ function escapeCsvCell(v: unknown): string {
return s;
}

/**
* Column set for an export, in first-seen order.
*
* [#11774] EVERY row, never a sample. The projection these columns drive is
* applied to ALL rows, so inferring them from a prefix made any key whose
* first occurrence fell past the boundary vanish from the header *and* from
* every row that carried it — silently, in a well-formed export of uniform
* arity with nothing marking a column as inferred rather than declared. The
* prefix was also not a random sample: it is the query's first page in its
* `orderBy`, so for a report sorted by status or date it correlates with
* exactly the sparse column it drops.
*
* Cost is one `Object.keys` pass per row, and is bounded twice over: the row
* array is already capped by `Math.min(query.limit, maxRows)` before it gets
* here, and both renderers already walk the same array once per row per
* column. Inference is therefore a fraction of the render it feeds, not a new
* order of magnitude on the export path.
*/
function pickFields(rows: any[], explicit?: string[]): string[] {
if (explicit && explicit.length > 0) return explicit;
const seen = new Set<string>();
for (const r of rows.slice(0, 50)) {
for (const r of rows) {
if (r && typeof r === 'object') for (const k of Object.keys(r)) seen.add(k);
}
return Array.from(seen);
Expand Down
Loading