fix(reports): stop the 'first N rows' option failing on large exports

The export path used one number for two different things: the format's hard
row cap, and the caller's explicit 'give me the first N rows'. Because
resolveExportCap() returned min(requested, formatCap) and runAll() then threw
when the result reached it, picking 'Records: First 100' in the export dialog
400'd on any report with more than 100 rows — the user asked to be truncated
and got an error instead.

Splits them: formatRowCap() is the hard, non-caller-controllable ceiling that
still throws when exceeded (a silently short file hides missing rows), while
resolveRowLimit() is the deliberate truncation and is honoured by slicing.
Verified against a 223-row dataset: limit=5 now returns 5 rows, and no limit
returns all 223.
This commit is contained in:
Nathnael
2026-08-20 05:28:51 +00:00
parent fc2b5ee0e3
commit ce90be5c88
4 changed files with 78 additions and 33 deletions

View File

@@ -1,4 +1,10 @@
import { EXPORT_MIME, pickByKey, resolveExportCap, resolveExportFormat } from './export-request.util'; import {
EXPORT_MIME,
formatRowCap,
pickByKey,
resolveExportFormat,
resolveRowLimit,
} from './export-request.util';
import { CSV_ROW_CAP, PDF_ROW_CAP, XLSX_ROW_CAP } from './tabular-export.service'; import { CSV_ROW_CAP, PDF_ROW_CAP, XLSX_ROW_CAP } from './tabular-export.service';
describe('resolveExportFormat', () => { describe('resolveExportFormat', () => {
@@ -15,25 +21,32 @@ describe('resolveExportFormat', () => {
}); });
}); });
describe('resolveExportCap', () => { describe('formatRowCap', () => {
it('missing limit uses the full format cap', () => { it('is the format\'s hard ceiling and is not caller-controllable', () => {
expect(resolveExportCap('xlsx', undefined)).toBe(XLSX_ROW_CAP); expect(formatRowCap('xlsx')).toBe(XLSX_ROW_CAP);
expect(resolveExportCap('csv', undefined)).toBe(CSV_ROW_CAP); expect(formatRowCap('csv')).toBe(CSV_ROW_CAP);
expect(resolveExportCap('pdf', undefined)).toBe(PDF_ROW_CAP); expect(formatRowCap('pdf')).toBe(PDF_ROW_CAP);
});
});
describe('resolveRowLimit', () => {
it('no limit means "everything, up to the cap"', () => {
expect(resolveRowLimit('xlsx', undefined)).toBeUndefined();
}); });
it('a limit under the cap is used as-is', () => { it('an explicit limit is the caller asking to be truncated — kept as-is', () => {
expect(resolveExportCap('pdf', '100')).toBe(100); // Distinct from the cap: 100 here must yield 100 rows, not a 400, even
// when the unfiltered result is far larger.
expect(resolveRowLimit('pdf', '100')).toBe(100);
}); });
it('a limit over the cap is clamped down', () => { it('an explicit limit over the format cap is clamped down', () => {
expect(resolveExportCap('pdf', String(PDF_ROW_CAP + 1000))).toBe(PDF_ROW_CAP); expect(resolveRowLimit('pdf', String(PDF_ROW_CAP + 1000))).toBe(PDF_ROW_CAP);
expect(resolveExportCap('xlsx', String(XLSX_ROW_CAP + 1))).toBe(XLSX_ROW_CAP); expect(resolveRowLimit('csv', String(CSV_ROW_CAP + 1))).toBe(CSV_ROW_CAP);
expect(resolveExportCap('csv', String(CSV_ROW_CAP + 1))).toBe(CSV_ROW_CAP);
}); });
it.each(['0', '-5', 'not-a-number', ''])('non-positive/invalid limit %p falls back to the cap', (raw) => { it.each(['0', '-5', 'not-a-number', ''])('non-positive/invalid limit %p means no limit', (raw) => {
expect(resolveExportCap('xlsx', raw)).toBe(XLSX_ROW_CAP); expect(resolveRowLimit('xlsx', raw)).toBeUndefined();
}); });
}); });

View File

@@ -9,13 +9,30 @@ export function resolveExportFormat(raw: string | undefined): ExportFormat {
return 'xlsx'; return 'xlsx';
} }
/** Caller's requested row limit, clamped to the format's hard cap. A /**
* missing/non-positive/non-numeric limit means "as many as the format allows". */ * The format's hard ceiling. Not caller-controllable: exceeding it is an error,
export function resolveExportCap(format: ExportFormat, rawLimit: string | undefined): number { * because a silently short file is worse than a clear failure.
const formatCap = */
format === 'pdf' ? PDF_ROW_CAP : format === 'csv' ? CSV_ROW_CAP : XLSX_ROW_CAP; export function formatRowCap(format: ExportFormat): number {
return format === 'pdf' ? PDF_ROW_CAP : format === 'csv' ? CSV_ROW_CAP : XLSX_ROW_CAP;
}
/**
* The caller's deliberate "just the first N rows", clamped to the format cap.
* `undefined` means "everything, up to the cap".
*
* This is a DIFFERENT thing from the cap and must not share a number with it.
* Conflating them (as this code did originally) makes the dialog's
* "Records: First 100" option fail outright on any export with more than 100
* rows — the user explicitly asked to be truncated, so truncating is the
* correct answer, not a 400.
*/
export function resolveRowLimit(
format: ExportFormat,
rawLimit: string | undefined,
): number | undefined {
const requested = Number(rawLimit); const requested = Number(rawLimit);
return requested > 0 ? Math.min(requested, formatCap) : formatCap; return requested > 0 ? Math.min(requested, formatRowCap(format)) : undefined;
} }
/** Content type + file extension per format, for the download response headers. */ /** Content type + file extension per format, for the download response headers. */

View File

@@ -122,12 +122,19 @@ export class ReportRunnerService {
}; };
} }
/** Same query, no paging — used by the export path. */ /**
* Same query, no paging — used by the export path.
*
* `limit` is the caller's deliberate "first N" (the dialog's "Records: First
* 100"), honoured by truncating. `cap` is the format's hard ceiling, which
* throws instead. These used to be one number, which made "First 100" fail
* outright on any report with more than 100 rows.
*/
async runAll( async runAll(
def: ReportDefinition, def: ReportDefinition,
raw: RawReportQuery, raw: RawReportQuery,
directions: string[] | null, directions: string[] | null,
limit: number, { cap, limit }: { cap: number; limit?: number },
): Promise<{ columns: typeof def.columns; items: Record<string, unknown>[]; kpis: ReportRunResult['kpis'] }> { ): Promise<{ columns: typeof def.columns; items: Record<string, unknown>[]; kpis: ReportRunResult['kpis'] }> {
const params = coerceParams(def, raw); const params = coerceParams(def, raw);
const ctx = { ds: this.ds, params, directions }; const ctx = { ds: this.ds, params, directions };
@@ -136,14 +143,19 @@ export class ReportRunnerService {
// export is supposed to match what the user is looking at. // export is supposed to match what the user is looking at.
const sort = resolveSort(def, raw.sortBy, raw.sortOrder); const sort = resolveSort(def, raw.sortBy, raw.sortOrder);
if (sort) qb.orderBy(sort.expr, sort.dir); if (sort) qb.orderBy(sort.expr, sort.dir);
// limit + 1: fetching exactly `limit` cannot distinguish "there are exactly
// limit rows" from "there are more" — which is why the old `>= limit` check const ceiling = limit ?? cap;
// rejected a legitimate export of exactly the cap. // ceiling + 1: fetching exactly `ceiling` cannot distinguish "there are
const items = await qb.limit(limit + 1).getRawMany(); // exactly that many rows" from "there are more" — which is why the old
if (items.length > limit) { // `>= limit` check rejected a legitimate export of exactly the cap.
throw new BadRequestException( const items = await qb.limit(ceiling + 1).getRawMany();
`Export exceeds the ${limit}-row cap for this format. Narrow the filters.`, if (items.length > ceiling) {
); if (limit === undefined) {
throw new BadRequestException(
`Export exceeds the ${cap}-row cap for this format. Narrow the filters.`,
);
}
items.length = limit;
} }
const kpis = def.summary ? await def.summary(ctx) : []; const kpis = def.summary ? await def.summary(ctx) : [];
return { columns: def.columns, items, kpis }; return { columns: def.columns, items, kpis };

View File

@@ -12,9 +12,10 @@ import { FREIGHT_PERMS, reportPermissionKey } from '../../seed/freight-permissio
import { UserTradeAccessService } from '../user-trade-access/user-trade-access.service'; import { UserTradeAccessService } from '../user-trade-access/user-trade-access.service';
import { import {
EXPORT_MIME, EXPORT_MIME,
formatRowCap,
pickByKey, pickByKey,
resolveExportCap,
resolveExportFormat, resolveExportFormat,
resolveRowLimit,
} from '../exports/export-request.util'; } from '../exports/export-request.util';
import { TabularExportService } from '../exports/tabular-export.service'; import { TabularExportService } from '../exports/tabular-export.service';
import { RawReportQuery, ReportRunnerService } from './report-runner.service'; import { RawReportQuery, ReportRunnerService } from './report-runner.service';
@@ -101,10 +102,12 @@ export class ReportsController {
const def = this.resolve(key, user); const def = this.resolve(key, user);
const directions = await this.userTradeAccessService.resolveAllowedDirections(user); const directions = await this.userTradeAccessService.resolveAllowedDirections(user);
const format = resolveExportFormat(query.format); const format = resolveExportFormat(query.format);
const cap = resolveExportCap(format, query.limit);
const exportColumns = pickByKey(def.columns, query.fields); const exportColumns = pickByKey(def.columns, query.fields);
const { items, kpis } = await this.runner.runAll(def, query, directions, cap); const { items, kpis } = await this.runner.runAll(def, query, directions, {
cap: formatRowCap(format),
limit: resolveRowLimit(format, query.limit),
});
const doc = { const doc = {
title: def.title, title: def.title,
description: def.description, description: def.description,