From fc2b5ee0e3d8b394704633e84192262d237898a4 Mon Sep 17 00:00:00 2001 From: Nathnael Date: Thu, 20 Aug 2026 05:16:15 +0000 Subject: [PATCH] refactor(reports): export through the shared tabular writer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes the writer extraction whose other half landed in fb21ad154. reports.controller now builds a TabularDoc and calls TabularExportService, so report-export.service.ts and report-export-request.util.ts are dead and removed — HEAD was carrying both copies with the controller still on the old one. Reports gain CSV for free, and the PDF path now passes buildTabularFallbackPdf as its fallback: previously it passed none, so a box without Chromium silently returned PdfRenderService's ~900-character generic text dump instead of a table. Adds a spec covering the CSV writer's quoting of embedded commas and double quotes — the reason this uses ExcelJS's csv writer rather than a hand-rolled join. --- .../exports/tabular-export.service.spec.ts | 65 ++++++++++ .../report-export-request.util.spec.ts | 62 ---------- .../reports/report-export-request.util.ts | 29 ----- .../modules/reports/report-export.service.ts | 117 ------------------ .../src/modules/reports/reports.controller.ts | 40 +++--- .../src/modules/reports/reports.module.ts | 9 +- 6 files changed, 95 insertions(+), 227 deletions(-) create mode 100644 apps/edr-freight-api/src/modules/exports/tabular-export.service.spec.ts delete mode 100644 apps/edr-freight-api/src/modules/reports/report-export-request.util.spec.ts delete mode 100644 apps/edr-freight-api/src/modules/reports/report-export-request.util.ts delete mode 100644 apps/edr-freight-api/src/modules/reports/report-export.service.ts diff --git a/apps/edr-freight-api/src/modules/exports/tabular-export.service.spec.ts b/apps/edr-freight-api/src/modules/exports/tabular-export.service.spec.ts new file mode 100644 index 000000000..7f262b48e --- /dev/null +++ b/apps/edr-freight-api/src/modules/exports/tabular-export.service.spec.ts @@ -0,0 +1,65 @@ +import { PdfRenderService } from '../billing/documents/pdf-render.service'; +import { TabularDoc, TabularExportService } from './tabular-export.service'; + +/** The PDF path is puppeteer-backed; these specs only cover the sheet writers. */ +const service = new TabularExportService(null as unknown as PdfRenderService); + +const doc: TabularDoc = { + title: 'Bookings', + description: 'every booking', + label: 'test', + columns: [ + { key: 'ref', label: 'Reference', type: 'string' }, + { key: 'customer', label: 'Customer', type: 'string' }, + { key: 'amount', label: 'Amount', type: 'money' }, + { key: 'gov', label: 'Government', type: 'boolean' }, + ], + rows: [ + { ref: 'BK-1', customer: 'Acme, Inc.', amount: 1234.5, gov: true }, + { ref: 'BK-2', customer: 'Quote "Q" Ltd', amount: null, gov: false }, + ], + kpis: [{ label: 'Bookings', value: 2 }], +}; + +describe('TabularExportService.toCsv', () => { + it('quotes a value containing the delimiter — the reason we do not hand-roll join(",")', async () => { + const csv = (await service.toCsv(doc)).toString('utf8'); + expect(csv).toContain('"Acme, Inc."'); + }); + + it('escapes embedded double quotes by doubling them', async () => { + const csv = (await service.toCsv(doc)).toString('utf8'); + expect(csv).toContain('"Quote ""Q"" Ltd"'); + }); + + it('starts at the header row — no KPI preamble, so the file parses as a plain table', async () => { + const csv = (await service.toCsv(doc)).toString('utf8'); + expect(csv.split('\n')[0]).toBe('Reference,Customer,Amount,Government'); + expect(csv).not.toContain('Bookings: 2'); + }); + + it('emits one line per row plus the header', async () => { + const csv = (await service.toCsv(doc)).toString('utf8'); + expect(csv.trim().split('\n').filter(Boolean)).toHaveLength(3); + }); + + it('only the selected columns are written, in the order given', async () => { + const csv = ( + await service.toCsv({ ...doc, columns: [doc.columns[2], doc.columns[0]] }) + ).toString('utf8'); + expect(csv.split('\n')[0]).toBe('Amount,Reference'); + }); +}); + +describe('TabularExportService.toXlsx', () => { + it('writes a real xlsx (a zip, so it starts with the PK magic bytes)', async () => { + const buffer = await service.toXlsx(doc); + expect(buffer.subarray(0, 2).toString('utf8')).toBe('PK'); + expect(buffer.length).toBeGreaterThan(1000); + }); + + it('a title longer than Excel\'s 31-char sheet-name limit does not throw', async () => { + const longTitle = 'A'.repeat(60); + await expect(service.toXlsx({ ...doc, title: longTitle })).resolves.toBeInstanceOf(Buffer); + }); +}); diff --git a/apps/edr-freight-api/src/modules/reports/report-export-request.util.spec.ts b/apps/edr-freight-api/src/modules/reports/report-export-request.util.spec.ts deleted file mode 100644 index dc6fa6230..000000000 --- a/apps/edr-freight-api/src/modules/reports/report-export-request.util.spec.ts +++ /dev/null @@ -1,62 +0,0 @@ -import { PDF_ROW_CAP, XLSX_ROW_CAP } from './report-export.service'; -import { resolveExportCap, resolveExportColumns, resolveExportFormat } from './report-export-request.util'; -import { ReportColumn } from './report.types'; - -describe('resolveExportFormat', () => { - it('only \'pdf\' exports as pdf', () => { - expect(resolveExportFormat('pdf')).toBe('pdf'); - }); - - it.each([undefined, 'xlsx', 'csv', ''])('%p falls back to xlsx', (raw) => { - expect(resolveExportFormat(raw)).toBe('xlsx'); - }); -}); - -describe('resolveExportCap', () => { - it('missing limit uses the full format cap', () => { - expect(resolveExportCap('xlsx', undefined)).toBe(XLSX_ROW_CAP); - expect(resolveExportCap('pdf', undefined)).toBe(PDF_ROW_CAP); - }); - - it('a limit under the cap is used as-is', () => { - expect(resolveExportCap('pdf', '100')).toBe(100); - }); - - it('a limit over the cap is clamped down', () => { - expect(resolveExportCap('pdf', String(PDF_ROW_CAP + 1000))).toBe(PDF_ROW_CAP); - expect(resolveExportCap('xlsx', String(XLSX_ROW_CAP + 1))).toBe(XLSX_ROW_CAP); - }); - - it.each(['0', '-5', 'not-a-number', ''])('non-positive/invalid limit %p falls back to the cap', (raw) => { - expect(resolveExportCap('xlsx', raw)).toBe(XLSX_ROW_CAP); - }); -}); - -describe('resolveExportColumns', () => { - const columns: ReportColumn[] = [ - { key: 'a', label: 'A', type: 'string' }, - { key: 'b', label: 'B', type: 'number' }, - { key: 'c', label: 'C', type: 'money' }, - ]; - const def = { columns }; - - it('missing fields returns every column', () => { - expect(resolveExportColumns(def, undefined)).toEqual(columns); - }); - - it('empty fields string returns every column', () => { - expect(resolveExportColumns(def, '')).toEqual(columns); - }); - - it('a known subset filters to just those columns, in the report\'s own order', () => { - expect(resolveExportColumns(def, 'c,a')).toEqual([columns[0], columns[2]]); - }); - - it('unknown keys are dropped, not passed through', () => { - expect(resolveExportColumns(def, 'a,ghost')).toEqual([columns[0]]); - }); - - it('all-unknown keys falls back to every column instead of a blank sheet', () => { - expect(resolveExportColumns(def, 'ghost,also-ghost')).toEqual(columns); - }); -}); diff --git a/apps/edr-freight-api/src/modules/reports/report-export-request.util.ts b/apps/edr-freight-api/src/modules/reports/report-export-request.util.ts deleted file mode 100644 index 18f2fa322..000000000 --- a/apps/edr-freight-api/src/modules/reports/report-export-request.util.ts +++ /dev/null @@ -1,29 +0,0 @@ -import { PDF_ROW_CAP, XLSX_ROW_CAP } from './report-export.service'; -import { ReportColumn, ReportDefinition } from './report.types'; - -export type ExportFormat = 'xlsx' | 'pdf'; - -/** Anything but the literal string 'pdf' exports as xlsx. */ -export function resolveExportFormat(raw: string | undefined): ExportFormat { - return raw === 'pdf' ? 'pdf' : '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". */ -export function resolveExportCap(format: ExportFormat, rawLimit: string | undefined): number { - const formatCap = format === 'pdf' ? PDF_ROW_CAP : XLSX_ROW_CAP; - const requested = Number(rawLimit); - return requested > 0 ? Math.min(requested, formatCap) : formatCap; -} - -/** Caller's requested column subset, whitelisted against the report's own - * columns. Missing, empty, or all-unknown `rawFields` falls back to every - * column rather than shipping a blank sheet. */ -export function resolveExportColumns( - def: Pick, - rawFields: string | undefined, -): ReportColumn[] { - const requested = rawFields?.split(',').filter(Boolean); - const filtered = requested?.length ? def.columns.filter((c) => requested.includes(c.key)) : def.columns; - return filtered.length ? filtered : def.columns; -} diff --git a/apps/edr-freight-api/src/modules/reports/report-export.service.ts b/apps/edr-freight-api/src/modules/reports/report-export.service.ts deleted file mode 100644 index f0919c9c7..000000000 --- a/apps/edr-freight-api/src/modules/reports/report-export.service.ts +++ /dev/null @@ -1,117 +0,0 @@ -import { Injectable } from '@nestjs/common'; -import ExcelJS from 'exceljs'; - -import { PdfRenderService } from '../billing/documents/pdf-render.service'; -import { ReportColumn, ReportDefinition, ReportKpi } from './report.types'; - -// ponytail: in-memory Workbook, cap below. Switch to ExcelJS's streaming -// WorkbookWriter if a report ever needs to outgrow XLSX_ROW_CAP. -export const XLSX_ROW_CAP = 50_000; -// ponytail: HTML→PDF render cost grows with row count; larger exports must -// use XLSX instead. -export const PDF_ROW_CAP = 5_000; - -const NUMBER_FORMAT: Partial> = { - money: '#,##0.00', - tons: '#,##0.0', - percent: '0"%"', - number: '#,##0', -}; - -function formatCell(value: unknown, type: ReportColumn['type']): string { - if (value === null || value === undefined) return ''; - if (type === 'money' || type === 'number') { - return Number(value).toLocaleString('en-US', { maximumFractionDigits: 2 }); - } - if (type === 'tons') return `${Number(value).toLocaleString('en-US')} t`; - if (type === 'percent') return `${value}%`; - return String(value); -} - -@Injectable() -export class ReportExportService { - constructor(private readonly pdfRender: PdfRenderService) {} - - async toXlsx( - def: ReportDefinition, - rows: Record[], - kpis: ReportKpi[], - columns: ReportColumn[] = def.columns, - ): Promise { - const workbook = new ExcelJS.Workbook(); - const sheet = workbook.addWorksheet(def.title.slice(0, 31)); - - if (kpis.length) { - sheet.addRow(kpis.map((k) => `${k.label}: ${k.value.toLocaleString()}${k.unit ? ` ${k.unit}` : ''}`)); - sheet.addRow([]); - } - - const headerRow = sheet.addRow(columns.map((c) => c.label)); - headerRow.font = { bold: true }; - - for (const row of rows) { - sheet.addRow(columns.map((c) => row[c.key] ?? null)); - } - - columns.forEach((col, i) => { - const format = NUMBER_FORMAT[col.type]; - const excelCol = sheet.getColumn(i + 1); - excelCol.width = Math.max(col.label.length + 2, 12); - if (format) excelCol.numFmt = format; - }); - - const buffer = await workbook.xlsx.writeBuffer(); - return Buffer.from(buffer); - } - - async toPdf( - def: ReportDefinition, - rows: Record[], - kpis: ReportKpi[], - columns: ReportColumn[] = def.columns, - ): Promise { - const html = this.buildHtml(def, rows, kpis, columns); - return this.pdfRender.htmlToPdfBuffer(html, { label: `report:${def.key}`, landscape: true }); - } - - private buildHtml( - def: ReportDefinition, - rows: Record[], - kpis: ReportKpi[], - columns: ReportColumn[], - ): string { - const esc = (v: unknown) => - String(v ?? '').replace(/&/g, '&').replace(//g, '>'); - - const kpiHtml = kpis.length - ? `
${kpis - .map( - (k) => - `
${esc(k.label)}
${k.value.toLocaleString()}${k.unit ? ` ${esc(k.unit)}` : ''}
`, - ) - .join('')}
` - : ''; - - const head = columns.map((c) => `${esc(c.label)}`).join(''); - const body = rows - .map( - (row) => - `${columns.map((c) => `${esc(formatCell(row[c.key], c.type))}`).join('')}`, - ) - .join(''); - - return ` -

${esc(def.title)}

-

${esc(def.description)}

- ${kpiHtml} - ${head}${body}
- `; - } -} diff --git a/apps/edr-freight-api/src/modules/reports/reports.controller.ts b/apps/edr-freight-api/src/modules/reports/reports.controller.ts index 107e11097..33a4214c8 100644 --- a/apps/edr-freight-api/src/modules/reports/reports.controller.ts +++ b/apps/edr-freight-api/src/modules/reports/reports.controller.ts @@ -10,8 +10,13 @@ import { BookingStaff } from '../../common/booking-guards'; import { assertFreightPermission, hasFreightPermission } from '../../common/freight-permission.util'; import { FREIGHT_PERMS, reportPermissionKey } from '../../seed/freight-permissions.registry'; import { UserTradeAccessService } from '../user-trade-access/user-trade-access.service'; -import { ReportExportService } from './report-export.service'; -import { resolveExportCap, resolveExportColumns, resolveExportFormat } from './report-export-request.util'; +import { + EXPORT_MIME, + pickByKey, + resolveExportCap, + resolveExportFormat, +} from '../exports/export-request.util'; +import { TabularExportService } from '../exports/tabular-export.service'; import { RawReportQuery, ReportRunnerService } from './report-runner.service'; import { REPORTS, getReport } from './report.registry'; import { ReportCatalogEntry, ReportDefinition, ReportFilterOption } from './report.types'; @@ -59,7 +64,7 @@ async function resolveFilterOptions( export class ReportsController { constructor( private readonly runner: ReportRunnerService, - private readonly exportService: ReportExportService, + private readonly exportService: TabularExportService, private readonly userTradeAccessService: UserTradeAccessService, @InjectDataSource() private readonly dataSource: DataSource, ) {} @@ -86,7 +91,7 @@ export class ReportsController { } @Get(':key/export') - @ApiOperation({ summary: 'Export a report to xlsx or pdf' }) + @ApiOperation({ summary: 'Export a report to xlsx, csv or pdf' }) async export( @Param('key') key: string, @Query() query: RawReportQuery & { format?: string; fields?: string; limit?: string }, @@ -97,22 +102,27 @@ export class ReportsController { const directions = await this.userTradeAccessService.resolveAllowedDirections(user); const format = resolveExportFormat(query.format); const cap = resolveExportCap(format, query.limit); - const exportColumns = resolveExportColumns(def, query.fields); + const exportColumns = pickByKey(def.columns, query.fields); const { items, kpis } = await this.runner.runAll(def, query, directions, cap); + const doc = { + title: def.title, + description: def.description, + label: `report:${def.key}`, + columns: exportColumns, + rows: items, + kpis, + }; const buffer = format === 'pdf' - ? await this.exportService.toPdf(def, items, kpis, exportColumns) - : await this.exportService.toXlsx(def, items, kpis, exportColumns); + ? await this.exportService.toPdf(doc) + : format === 'csv' + ? await this.exportService.toCsv(doc) + : await this.exportService.toXlsx(doc); - const filename = `${def.key}.${format === 'pdf' ? 'pdf' : 'xlsx'}`; - res.setHeader('Content-Disposition', `attachment; filename="${filename}"`); - res.setHeader( - 'Content-Type', - format === 'pdf' - ? 'application/pdf' - : 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', - ); + const mime = EXPORT_MIME[format]; + res.setHeader('Content-Disposition', `attachment; filename="${def.key}.${mime.ext}"`); + res.setHeader('Content-Type', mime.type); res.send(buffer); } diff --git a/apps/edr-freight-api/src/modules/reports/reports.module.ts b/apps/edr-freight-api/src/modules/reports/reports.module.ts index 2f98e9e04..e60f16362 100644 --- a/apps/edr-freight-api/src/modules/reports/reports.module.ts +++ b/apps/edr-freight-api/src/modules/reports/reports.module.ts @@ -1,14 +1,15 @@ import { Module } from '@nestjs/common'; -import { DocumentsModule } from '../billing/documents/documents.module'; +import { ExportsModule } from '../exports/exports.module'; import { UserTradeAccessModule } from '../user-trade-access/user-trade-access.module'; -import { ReportExportService } from './report-export.service'; import { ReportRunnerService } from './report-runner.service'; import { ReportsController } from './reports.controller'; @Module({ - imports: [UserTradeAccessModule, DocumentsModule], + // ExportsModule provides the shared tabular writer (xlsx/csv/pdf) and pulls + // DocumentsModule in for the PDF renderer. + imports: [UserTradeAccessModule, ExportsModule], controllers: [ReportsController], - providers: [ReportRunnerService, ReportExportService], + providers: [ReportRunnerService], }) export class ReportsModule {}