From bf74c581ea60798ff46b23c33370aedbcaa80593 Mon Sep 17 00:00:00 2001 From: Frank Steiler Date: Mon, 3 Aug 2026 13:06:03 +0200 Subject: [PATCH 1/6] =?UTF-8?q?fix(reports):=20improve=20report=20PDF=20UX?= =?UTF-8?q?=20=E2=80=94=20paragraph=20breaks,=20inline=20meta,=20inline=20?= =?UTF-8?q?notes,=20column=20toggles?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - AI cover letter body now splits on double newlines into separate pdfmake content blocks, preserving paragraph breaks from the LLM response - Area and attachments count moved inline into the usage cell (grey, same font size) instead of a separate stack/sub-row - Split (†) and deposit-reduced (‡) footnotes replaced with inline grey labels in the allocated amount cell, matching the existing deposit pattern; allocatedMarkers field removed from ReportContentRow in favour of boolean isSplit / isDepositReduced flags - ReportContentEditor gains per-column visibility toggles (local state, no persistence); attachments column removed — content is now inline under usage Co-Authored-By: Claude frontend-developer --- .../reports/ReportContentEditor.module.css | 36 ++- .../reports/ReportContentEditor.test.tsx | 16 +- .../reports/ReportContentEditor.tsx | 300 ++++++++++-------- client/src/i18n/de/budget.json | 5 +- client/src/i18n/en/budget.json | 5 +- .../lib/reportContent/applyAiContent.test.ts | 5 +- .../lib/reportContent/applyOverrides.test.ts | 5 +- .../reportContent/buildReportContent.test.ts | 124 +++----- .../lib/reportContent/buildReportContent.ts | 33 +- client/src/lib/reportContent/types.ts | 7 +- client/src/lib/reportPdf/coverLetterPdf.ts | 23 +- client/src/lib/reportPdf/overviewPdf.test.ts | 97 +++--- client/src/lib/reportPdf/overviewPdf.ts | 92 ++---- client/src/lib/reportPdf/realRender.test.ts | 8 +- 14 files changed, 384 insertions(+), 372 deletions(-) diff --git a/client/src/components/reports/ReportContentEditor.module.css b/client/src/components/reports/ReportContentEditor.module.css index a88bde78e..ba129259f 100644 --- a/client/src/components/reports/ReportContentEditor.module.css +++ b/client/src/components/reports/ReportContentEditor.module.css @@ -67,12 +67,37 @@ gap: var(--spacing-1); } -/* Table Heading */ +/* Table Heading + Column Toggles */ +.tableHeadingRow { + display: flex; + align-items: flex-start; + gap: var(--spacing-4); + flex-wrap: wrap; +} + .tableHeading { margin: 0 0 var(--spacing-3) 0; font-size: var(--font-size-lg); font-weight: var(--font-weight-semibold); color: var(--color-text-primary); + flex-shrink: 0; +} + +.columnToggles { + display: flex; + flex-wrap: wrap; + gap: var(--spacing-2); + margin-bottom: var(--spacing-3); +} + +.columnToggle { + display: flex; + align-items: center; + gap: var(--spacing-1); + font-size: var(--font-size-xs); + color: var(--color-text-muted); + cursor: pointer; + white-space: nowrap; } /* Table Wrapper (Desktop) */ @@ -170,12 +195,17 @@ flex-wrap: wrap; } -.usageAreaText { - font-size: var(--font-size-xs); +.usageMetaText { color: var(--color-text-muted); margin-top: var(--spacing-1); } +.inlineNote { + font-size: var(--font-size-xs); + color: var(--color-text-muted); + margin-left: var(--spacing-1); +} + /* Summary Table */ .summaryTable { width: 100%; diff --git a/client/src/components/reports/ReportContentEditor.test.tsx b/client/src/components/reports/ReportContentEditor.test.tsx index 1e2f6b585..042af6fa2 100644 --- a/client/src/components/reports/ReportContentEditor.test.tsx +++ b/client/src/components/reports/ReportContentEditor.test.tsx @@ -79,6 +79,8 @@ const LABELS: ReportContentLabels = { usage: 'REPORT_USAGE_LABEL', attachmentsNote: 'REPORT_ATTACHMENTS_NOTE_LABEL', deposit: 'REPORT_DEPOSIT_LABEL', + splitNote: 'REPORT_SPLIT_NOTE_LABEL', + depositReducedNote: 'REPORT_DEPOSIT_REDUCED_NOTE_LABEL', source: 'REPORT_SOURCE_LABEL', sourceType: 'REPORT_SOURCE_TYPE_LABEL', reference: 'REPORT_REFERENCE_LABEL', @@ -95,7 +97,8 @@ function makeRow(overrides: Partial = {}): ReportContentRow { statusText: null, invoiceAmountText: '€100.00', allocatedAmountValueText: '€100.00', - allocatedMarkers: '', + isSplit: false, + isDepositReduced: false, isDeposit: false, isRefund: false, refundNoteText: '', @@ -704,18 +707,17 @@ describe('ReportContentEditor — table rows', () => { expect(within(table).queryByDisplayValue('€555.00')).not.toBeInTheDocument(); }); - it('composes the allocated cell as valueText + markers + refund note when isRefund', () => { + it('composes the allocated cell as valueText + refund note when isRefund', () => { const rows = [ makeRow({ allocatedAmountValueText: '€-200.00', - allocatedMarkers: '†1', isRefund: true, refundNoteText: '(refund)', }), ]; const { container } = renderEditor({ content: makeContent({ rows }) }); const table = getDesktopTable(container); - expect(within(table).getByText('€-200.00†1 (refund)')).toBeInTheDocument(); + expect(within(table).getByText('€-200.00 (refund)')).toBeInTheDocument(); }); it('applies the refundAmount CSS class (not an inline style) to both amount cells when isRefund', () => { @@ -913,7 +915,6 @@ describe('ReportContentEditor — isDeposit (AC2.1: inline Deposit badge, no mar makeRow({ invoiceId: 'inv-1', isDeposit: true, - allocatedMarkers: '', allocatedAmountValueText: '€300.00', }), ]; @@ -1054,18 +1055,17 @@ describe( expect(card.queryByLabelText(LABELS.attachmentsNote)).not.toBeInTheDocument(); }); - it('composes the mobile card allocated amount as valueText + markers + refund note when isRefund, matching the desktop cell', () => { + it('composes the mobile card allocated amount as valueText + refund note when isRefund, matching the desktop cell', () => { const rows = [ makeRow({ allocatedAmountValueText: '€-200.00', - allocatedMarkers: '†1', isRefund: true, refundNoteText: '(refund)', }), ]; const { container } = renderEditor({ content: makeContent({ rows }) }); const card = within(getMobileList(container)); - expect(card.getByText('€-200.00†1 (refund)')).toBeInTheDocument(); + expect(card.getByText('€-200.00 (refund)')).toBeInTheDocument(); }); it('wires the mobile card usage/attachmentsNote EditableFields to the same onFieldChange keys as the desktop table', () => { diff --git a/client/src/components/reports/ReportContentEditor.tsx b/client/src/components/reports/ReportContentEditor.tsx index c81a2b98e..d019bc87f 100644 --- a/client/src/components/reports/ReportContentEditor.tsx +++ b/client/src/components/reports/ReportContentEditor.tsx @@ -3,6 +3,7 @@ * Handles field changes and resets via callbacks; no state management. */ +import { useState } from 'react'; import type { TFunction } from 'i18next'; import type { InvoiceStatus } from '@cornerstone/shared'; import type { ReportContent, ReportContentOverrides } from '../../lib/reportContent/index.js'; @@ -27,6 +28,9 @@ const STATUS_BADGE_CLASSNAME: Record = { quotation: styles.statusQuotation!, }; +type ColumnKey = + 'vendor' | 'invoiceNumber' | 'date' | 'status' | 'invoiceAmount' | 'allocatedAmount' | 'usage'; + export function ReportContentEditor({ content, overrides, @@ -37,6 +41,18 @@ export function ReportContentEditor({ // Helper: check if a field has been overridden const isFieldEdited = (key: string): boolean => key in overrides; + // Column visibility state + const [hiddenColumns, setHiddenColumns] = useState>(new Set()); + const toggleColumn = (col: ColumnKey) => { + setHiddenColumns((prev) => { + const next = new Set(prev); + if (next.has(col)) next.delete(col); + else next.add(col); + return next; + }); + }; + const show = (col: ColumnKey) => !hiddenColumns.has(col); + return (
{/* Cover Letter */} @@ -174,30 +190,57 @@ export function ReportContentEditor({ )} {/* Report Table */} -

{t('sourceReports.editable.tableHeading')}

+
+

{t('sourceReports.editable.tableHeading')}

+
+ {( + [ + ['vendor', content.labels.vendor], + ['invoiceNumber', content.labels.invoiceNumber], + ['date', content.labels.date], + ...(content.isOverview + ? [['status', content.labels.status] as [ColumnKey, string]] + : []), + ['invoiceAmount', content.labels.invoiceAmount], + ['allocatedAmount', content.labels.allocatedAmount], + ['usage', content.labels.usage], + ] as [ColumnKey, string][] + ).map(([col, label]) => ( + + ))} +
+
- - - - {content.isOverview && } - - - - {content.rows.some((r) => r.attachmentsNote !== null) && ( - + {show('vendor') && } + {show('invoiceNumber') && } + {show('date') && } + {content.isOverview && show('status') && } + {show('invoiceAmount') && ( + + )} + {show('allocatedAmount') && ( + )} + {show('usage') && } {content.rows.map((row) => ( - - - - {content.isOverview && row.status && row.statusText != null && ( + {show('vendor') && } + {show('invoiceNumber') && } + {show('date') && } + {content.isOverview && show('status') && row.status && row.statusText != null && ( )} - - - - {row.attachmentsNote !== null && ( + {content.isOverview && + show('status') && + (!row.status || row.statusText == null) && + )} + {show('allocatedAmount') && ( + + )} + {show('usage') && ( )} @@ -281,19 +324,25 @@ export function ReportContentEditor({
{content.rows.map((row) => (
-
- {content.labels.vendor} - {row.vendor} -
-
- {content.labels.invoiceNumber} - {row.invoiceNumber} -
-
- {content.labels.date} - {row.dateText} -
- {content.isOverview && row.status && row.statusText != null && ( + {show('vendor') && ( +
+ {content.labels.vendor} + {row.vendor} +
+ )} + {show('invoiceNumber') && ( +
+ {content.labels.invoiceNumber} + {row.invoiceNumber} +
+ )} + {show('date') && ( +
+ {content.labels.date} + {row.dateText} +
+ )} + {content.isOverview && show('status') && row.status && row.statusText != null && (
{content.labels.status}
)} -
- {content.labels.invoiceAmount} - - {row.invoiceAmountText} - -
-
- {content.labels.allocatedAmount} - + {show('invoiceAmount') && ( +
+ {content.labels.invoiceAmount} - {row.allocatedAmountValueText} - {row.allocatedMarkers} - {row.isRefund && ` ${row.refundNoteText}`} + {row.invoiceAmountText} - {row.isDeposit && ( - - )} - -
-
- onFieldChange(overrideKey.row(row.invoiceId).usageText, value)} - isEdited={isFieldEdited(overrideKey.row(row.invoiceId).usageText)} - onReset={() => onFieldReset(overrideKey.row(row.invoiceId).usageText)} - /> - {row.areaText && {row.areaText}} -
- {row.attachmentsNote !== null && ( +
+ )} + {show('allocatedAmount') && ( +
+ {content.labels.allocatedAmount} + + + {row.allocatedAmountValueText} + {row.isRefund && ` ${row.refundNoteText}`} + + {row.isDeposit && ( + + )} + {row.isSplit && ( + ({content.labels.splitNote}) + )} + {row.isDepositReduced && ( + ({content.labels.depositReducedNote}) + )} + +
+ )} + {show('usage') && (
- onFieldChange(overrideKey.row(row.invoiceId).attachmentsNote, value) + onFieldChange(overrideKey.row(row.invoiceId).usageText, value) } - isEdited={isFieldEdited(overrideKey.row(row.invoiceId).attachmentsNote)} - onReset={() => onFieldReset(overrideKey.row(row.invoiceId).attachmentsNote)} + isEdited={isFieldEdited(overrideKey.row(row.invoiceId).usageText)} + onReset={() => onFieldReset(overrideKey.row(row.invoiceId).usageText)} /> + {(row.areaText || row.attachmentsNote) && ( + + {[row.areaText, row.attachmentsNote].filter(Boolean).join(' · ')} + + )}
)}
diff --git a/client/src/i18n/de/budget.json b/client/src/i18n/de/budget.json index 95b65fa31..572cf2640 100644 --- a/client/src/i18n/de/budget.json +++ b/client/src/i18n/de/budget.json @@ -1215,6 +1215,7 @@ "keepEditing": "Weiter bearbeiten", "coverLetterHeading": "Anschreiben", "tableHeading": "Berichtstabelle", + "columnVisibilityLabel": "Spalten ein-/ausblenden", "enhanceWithAi": "Mit KI verbessern", "enhanceWithAiDescription": "Ersetzt die unten stehenden Verwendungstexte und das Anschreiben durch KI-generierte Inhalte. Vorgenommene Bearbeitungen gehen dabei verloren.", "generating": "Generiere… ({{seconds}}s)", @@ -1256,7 +1257,9 @@ "attachmentsNote_other": "{{count}} Anhänge: {{types}}", "attachmentsNoteNoType_one": "{{count}} Anhang", "attachmentsNoteNoType_other": "{{count}} Anhänge", - "depositReducedFootnote": "Diese Position berücksichtigt separat eingereichte Abschlagszahlungen." + "depositReducedFootnote": "Diese Position berücksichtigt separat eingereichte Abschlagszahlungen.", + "splitInlineLabel": "Teilbetrag", + "depositReducedInlineLabel": "abzgl. Abschlag" }, "sourceType": { "bank_loan": "Bankdarlehen", diff --git a/client/src/i18n/en/budget.json b/client/src/i18n/en/budget.json index 882a2f6f5..1921c8803 100644 --- a/client/src/i18n/en/budget.json +++ b/client/src/i18n/en/budget.json @@ -1215,6 +1215,7 @@ "keepEditing": "Keep Editing", "coverLetterHeading": "Cover Letter", "tableHeading": "Report Table", + "columnVisibilityLabel": "Show/hide columns", "enhanceWithAi": "Enhance with AI", "enhanceWithAiDescription": "Replaces the usage descriptions and cover letter below with AI-generated content. Any edits you've made will be discarded.", "generating": "Generating… ({{seconds}}s)", @@ -1256,7 +1257,9 @@ "footnoteFetchFailed": "Document could not be retrieved", "footnoteInvalidPdf": "Document is not a valid PDF", "splitFootnote": "Amount shown reflects only the portion allocated to this source.", - "depositReducedFootnote": "This position reflects deposits claimed separately." + "depositReducedFootnote": "This position reflects deposits claimed separately.", + "splitInlineLabel": "partial", + "depositReducedInlineLabel": "less deposit" }, "sourceType": { "bank_loan": "Bank Loan", diff --git a/client/src/lib/reportContent/applyAiContent.test.ts b/client/src/lib/reportContent/applyAiContent.test.ts index a8b44d55f..715382c22 100644 --- a/client/src/lib/reportContent/applyAiContent.test.ts +++ b/client/src/lib/reportContent/applyAiContent.test.ts @@ -22,7 +22,8 @@ function makeRow(overrides: Partial = {}): ReportContentRow { statusText: null, invoiceAmountText: '€100.00', allocatedAmountValueText: '€100.00', - allocatedMarkers: '', + isSplit: false, + isDepositReduced: false, isDeposit: false, isRefund: false, refundNoteText: '', @@ -44,6 +45,8 @@ function makeLabels(): ReportContent['labels'] { usage: 'Usage', attachmentsNote: 'Attachments Note', deposit: 'Deposit', + splitNote: 'partial', + depositReducedNote: 'less deposit', source: 'Source', sourceType: 'Source Type', reference: 'Reference', diff --git a/client/src/lib/reportContent/applyOverrides.test.ts b/client/src/lib/reportContent/applyOverrides.test.ts index c92dbf528..1ced2be53 100644 --- a/client/src/lib/reportContent/applyOverrides.test.ts +++ b/client/src/lib/reportContent/applyOverrides.test.ts @@ -21,7 +21,8 @@ function makeRow(overrides: Partial = {}): ReportContentRow { statusText: null, invoiceAmountText: '€100.00', allocatedAmountValueText: '€100.00', - allocatedMarkers: '', + isSplit: false, + isDepositReduced: false, isDeposit: false, isRefund: false, refundNoteText: '', @@ -43,6 +44,8 @@ function makeLabels(): ReportContent['labels'] { usage: 'Usage', attachmentsNote: 'Attachments Note', deposit: 'Deposit', + splitNote: 'partial', + depositReducedNote: 'less deposit', source: 'Source', sourceType: 'Source Type', reference: 'Reference', diff --git a/client/src/lib/reportContent/buildReportContent.test.ts b/client/src/lib/reportContent/buildReportContent.test.ts index 121de3181..75e7a4eb9 100644 --- a/client/src/lib/reportContent/buildReportContent.test.ts +++ b/client/src/lib/reportContent/buildReportContent.test.ts @@ -342,8 +342,8 @@ describe('buildReportContent — rows', () => { }); }); - describe('allocatedMarkers (split † / deposit ‡, unnumbered/shared per story #1923) + isDeposit', () => { - it('adds unnumbered † only when isSplit and budgetLines.length > 0, no deposits', () => { + describe('isSplit / isDepositReduced / isDeposit flags', () => { + it('sets isSplit=true when isSplit and budgetLines.length > 0, no deposits', () => { const invoice = makeInvoice({ isSplit: true, budgetLines: [makeBudgetLine()], @@ -351,11 +351,12 @@ describe('buildReportContent — rows', () => { }); const report = makeReport([invoice]); const content = buildReportContent(report, new Set(['inv-1']), 'claim', t, formatters); - expect(content.rows[0]!.allocatedMarkers).toBe('†'); + expect(content.rows[0]!.isSplit).toBe(true); + expect(content.rows[0]!.isDepositReduced).toBe(false); expect(content.rows[0]!.isDeposit).toBe(false); }); - it('adds unnumbered ‡ only when isSplit and the deposit is untagged (reduced), no budget lines', () => { + it('sets isDepositReduced=true when isSplit and the deposit is untagged (reduced), no budget lines', () => { const invoice = makeInvoice({ isSplit: true, budgetLines: [], @@ -363,11 +364,12 @@ describe('buildReportContent — rows', () => { }); const report = makeReport([invoice], { id: 'src-1' }); const content = buildReportContent(report, new Set(['inv-1']), 'claim', t, formatters); - expect(content.rows[0]!.allocatedMarkers).toBe('‡'); + expect(content.rows[0]!.isSplit).toBe(false); + expect(content.rows[0]!.isDepositReduced).toBe(true); expect(content.rows[0]!.isDeposit).toBe(false); }); - it('AC2.1: adds NO marker and sets isDeposit=true when the deposit is tagged to this report source (constituted)', () => { + it('AC2.1: sets isDeposit=true (not isSplit/isDepositReduced) when the deposit is tagged to this report source (constituted)', () => { const invoice = makeInvoice({ isSplit: true, budgetLines: [], @@ -375,11 +377,12 @@ describe('buildReportContent — rows', () => { }); const report = makeReport([invoice], { id: 'src-1' }); const content = buildReportContent(report, new Set(['inv-1']), 'claim', t, formatters); - expect(content.rows[0]!.allocatedMarkers).toBe(''); + expect(content.rows[0]!.isSplit).toBe(false); + expect(content.rows[0]!.isDepositReduced).toBe(false); expect(content.rows[0]!.isDeposit).toBe(true); }); - it('AC2.4: adds both † and ‡ (order †‡) when split budget lines and a reduced (untagged) deposit are both present', () => { + it('AC2.4: sets both isSplit and isDepositReduced when split budget lines and a reduced deposit are both present', () => { const invoice = makeInvoice({ isSplit: true, budgetLines: [makeBudgetLine()], @@ -387,10 +390,11 @@ describe('buildReportContent — rows', () => { }); const report = makeReport([invoice], { id: 'src-1' }); const content = buildReportContent(report, new Set(['inv-1']), 'claim', t, formatters); - expect(content.rows[0]!.allocatedMarkers).toBe('†‡'); + expect(content.rows[0]!.isSplit).toBe(true); + expect(content.rows[0]!.isDepositReduced).toBe(true); }); - it('adds only † (no ‡, isDeposit=true) when split budget lines are combined with a constituted (tagged) deposit', () => { + it('sets isSplit=true and isDeposit=true when split budget lines combined with a constituted (tagged) deposit', () => { const invoice = makeInvoice({ isSplit: true, budgetLines: [makeBudgetLine()], @@ -398,11 +402,12 @@ describe('buildReportContent — rows', () => { }); const report = makeReport([invoice], { id: 'src-1' }); const content = buildReportContent(report, new Set(['inv-1']), 'claim', t, formatters); - expect(content.rows[0]!.allocatedMarkers).toBe('†'); + expect(content.rows[0]!.isSplit).toBe(true); + expect(content.rows[0]!.isDepositReduced).toBe(false); expect(content.rows[0]!.isDeposit).toBe(true); }); - it('adds neither marker nor isDeposit when isSplit is false, regardless of budgetLines/deposits content', () => { + it('all flags are false when isSplit is false, regardless of budgetLines/deposits content', () => { const invoice = makeInvoice({ isSplit: false, budgetLines: [makeBudgetLine()], @@ -410,19 +415,21 @@ describe('buildReportContent — rows', () => { }); const report = makeReport([invoice]); const content = buildReportContent(report, new Set(['inv-1']), 'claim', t, formatters); - expect(content.rows[0]!.allocatedMarkers).toBe(''); + expect(content.rows[0]!.isSplit).toBe(false); + expect(content.rows[0]!.isDepositReduced).toBe(false); expect(content.rows[0]!.isDeposit).toBe(false); }); - it('adds neither marker when isSplit is true but budgetLines and deposits are both empty', () => { + it('all flags are false when isSplit is true but budgetLines and deposits are both empty', () => { const invoice = makeInvoice({ isSplit: true, budgetLines: [], deposits: [] }); const report = makeReport([invoice]); const content = buildReportContent(report, new Set(['inv-1']), 'claim', t, formatters); - expect(content.rows[0]!.allocatedMarkers).toBe(''); + expect(content.rows[0]!.isSplit).toBe(false); + expect(content.rows[0]!.isDepositReduced).toBe(false); expect(content.rows[0]!.isDeposit).toBe(false); }); - it('never assigns markers to an excluded invoice, even when isSplit with lines/deposits', () => { + it('never sets flags on an excluded invoice, even when isSplit with lines/deposits', () => { const invoice = makeInvoice({ invoiceId: 'inv-excluded', isSplit: true, @@ -431,63 +438,38 @@ describe('buildReportContent — rows', () => { const included = makeInvoice({ invoiceId: 'inv-1' }); const report = makeReport([invoice, included]); const content = buildReportContent(report, new Set(['inv-1']), 'claim', t, formatters); - // Only the included row is present; footnotes must not have been generated for the excluded one. expect(content.rows).toHaveLength(1); expect(content.footnotes).toEqual([]); }); }); }); -describe('buildReportContent — footnotes (AC1/AC2: shared, unnumbered, at most 2 entries, no vendor prefix)', () => { - it('AC1.2: produces exactly one shared split footnote (no vendor/invoice-number prefix) when three included invoices are split', () => { +describe('buildReportContent — footnotes (always empty; split/deposit annotations are inline)', () => { + it('produces no footnotes when invoices are split with budget lines', () => { const inv1 = makeInvoice({ invoiceId: 'inv-1', - vendorName: 'Gamma Corp', - invoiceNumber: 'G-9', isSplit: true, budgetLines: [makeBudgetLine()], }); const inv2 = makeInvoice({ invoiceId: 'inv-2', - vendorName: 'Delta Corp', - invoiceNumber: 'D-1', isSplit: true, budgetLines: [makeBudgetLine()], }); - const inv3 = makeInvoice({ - invoiceId: 'inv-3', - vendorName: 'Epsilon Corp', - invoiceNumber: 'E-2', - isSplit: true, - budgetLines: [makeBudgetLine()], - }); - const report = makeReport([inv1, inv2, inv3]); - const content = buildReportContent( - report, - new Set(['inv-1', 'inv-2', 'inv-3']), - 'claim', - t, - formatters, - ); - expect(content.footnotes).toEqual([ - { - id: 'split', - marker: '†', - text: 'sourceReports.table.splitFootnote', - }, - ]); - // Every split row carries the marker — no per-invoice numbering distinguishes them. - expect(content.rows.every((r) => r.allocatedMarkers === '†')).toBe(true); - }); - - it('AC1.3: produces no † marker and no split footnote anywhere when no included invoice is split', () => { + const report = makeReport([inv1, inv2]); + const content = buildReportContent(report, new Set(['inv-1', 'inv-2']), 'claim', t, formatters); + expect(content.footnotes).toEqual([]); + expect(content.rows.every((r) => r.isSplit)).toBe(true); + }); + + it('produces no footnotes when all invoices are unsplit', () => { const report = makeReport([makeInvoice({ isSplit: false })]); const content = buildReportContent(report, new Set(['inv-1']), 'claim', t, formatters); expect(content.footnotes).toEqual([]); - expect(content.rows[0]!.allocatedMarkers).toBe(''); + expect(content.rows[0]!.isSplit).toBe(false); }); - it('AC2.2: produces NO footnote entry for a constituted (tagged) deposit — the row gets isDeposit instead', () => { + it('produces no footnotes for constituted (tagged) deposit — the row gets isDeposit instead', () => { const invoice = makeInvoice({ isSplit: true, budgetLines: [], @@ -499,7 +481,7 @@ describe('buildReportContent — footnotes (AC1/AC2: shared, unnumbered, at most expect(content.rows[0]!.isDeposit).toBe(true); }); - it('AC2.3: produces exactly one shared, unnumbered ‡ footnote when one or more invoices have a reduced (untagged) deposit', () => { + it('produces no footnotes when invoices have reduced (untagged) deposits — isDepositReduced is set instead', () => { const invoice = makeInvoice({ isSplit: true, budgetLines: [], @@ -507,35 +489,11 @@ describe('buildReportContent — footnotes (AC1/AC2: shared, unnumbered, at most }); const report = makeReport([invoice], { id: 'src-1' }); const content = buildReportContent(report, new Set(['inv-1']), 'claim', t, formatters); - expect(content.footnotes).toEqual([ - { - id: 'deposit-reduced', - marker: '‡', - text: 'sourceReports.table.depositReducedFootnote', - }, - ]); - }); - - it('AC2.3: multiple invoices with reduced deposits still produce exactly one shared ‡ entry', () => { - const inv1 = makeInvoice({ - invoiceId: 'inv-1', - isSplit: true, - budgetLines: [], - deposits: [makeDeposit({ id: 'dep-1', budgetSourceId: null })], - }); - const inv2 = makeInvoice({ - invoiceId: 'inv-2', - isSplit: true, - budgetLines: [], - deposits: [makeDeposit({ id: 'dep-2', budgetSourceId: null })], - }); - const report = makeReport([inv1, inv2], { id: 'src-1' }); - const content = buildReportContent(report, new Set(['inv-1', 'inv-2']), 'claim', t, formatters); - const reducedFootnotes = content.footnotes.filter((f) => f.id === 'deposit-reduced'); - expect(reducedFootnotes).toHaveLength(1); + expect(content.footnotes).toEqual([]); + expect(content.rows[0]!.isDepositReduced).toBe(true); }); - it('AC2.4: both split and reduced-deposit invoices present → markers "†‡" on the combined row, footnotes ordered [split, deposit-reduced]', () => { + it('produces no footnotes when an invoice has both split lines and a reduced deposit', () => { const combined = makeInvoice({ invoiceId: 'inv-1', isSplit: true, @@ -544,9 +502,9 @@ describe('buildReportContent — footnotes (AC1/AC2: shared, unnumbered, at most }); const report = makeReport([combined], { id: 'src-1' }); const content = buildReportContent(report, new Set(['inv-1']), 'claim', t, formatters); - expect(content.rows[0]!.allocatedMarkers).toBe('†‡'); - expect(content.footnotes.map((f) => f.id)).toEqual(['split', 'deposit-reduced']); - expect(content.footnotes).toHaveLength(2); + expect(content.footnotes).toEqual([]); + expect(content.rows[0]!.isSplit).toBe(true); + expect(content.rows[0]!.isDepositReduced).toBe(true); }); it('produces no footnotes when no invoice is split or has a reduced deposit', () => { diff --git a/client/src/lib/reportContent/buildReportContent.ts b/client/src/lib/reportContent/buildReportContent.ts index a685718d1..811da696a 100644 --- a/client/src/lib/reportContent/buildReportContent.ts +++ b/client/src/lib/reportContent/buildReportContent.ts @@ -187,15 +187,8 @@ export function buildReportContent( const statusText = isOverview ? reportT(`sources.lines.invoiceStatus.${status}`) : null; - // Compute allocated markers († for split, ‡ for reduced; no markers for constituted deposits) - let allocatedMarkers = ''; - if (splitInvoiceIds.has(invoice.invoiceId)) { - allocatedMarkers += '†'; - } - if (depositReducedInvoiceIds.has(invoice.invoiceId)) { - allocatedMarkers += '‡'; - } - + const isSplit = splitInvoiceIds.has(invoice.invoiceId); + const isDepositReduced = depositReducedInvoiceIds.has(invoice.invoiceId); const isDeposit = depositConstitutedInvoiceIds.has(invoice.invoiceId); const refundNoteText = reportT('sourceReports.table.refundNote'); const usageText = getUsageText(invoice); @@ -211,7 +204,8 @@ export function buildReportContent( statusText, invoiceAmountText, allocatedAmountValueText, - allocatedMarkers, + isSplit, + isDepositReduced, isDeposit, isRefund: invoice.lineKind === 'refund-adjustment', refundNoteText, @@ -235,25 +229,8 @@ export function buildReportContent( amountText: totalAmountText, }); - // Build footnotes (at most 2 shared entries: split + deposit-reduced) const footnotes: ReportContentFootnote[] = []; - if (splitInvoiceIds.size > 0) { - footnotes.push({ - id: 'split', - marker: '†', - text: reportT('sourceReports.table.splitFootnote'), - }); - } - - if (depositReducedInvoiceIds.size > 0) { - footnotes.push({ - id: 'deposit-reduced', - marker: '‡', - text: reportT('sourceReports.table.depositReducedFootnote'), - }); - } - // Build cover letter (if enabled) let coverLetter: ReportContentCoverLetter | null = null; if (includeCoverLetter) { @@ -298,6 +275,8 @@ export function buildReportContent( usage: reportT('sourceReports.table.usage'), attachmentsNote: reportT('sourceReports.editable.attachmentsNoteLabel'), deposit: reportT('sourceReports.table.attachmentType.deposit'), + splitNote: reportT('sourceReports.table.splitInlineLabel'), + depositReducedNote: reportT('sourceReports.table.depositReducedInlineLabel'), source: reportT('sourceReports.table.source'), sourceType: reportT('sourceReports.table.sourceType'), reference: reportT('sourceReports.table.reference'), diff --git a/client/src/lib/reportContent/types.ts b/client/src/lib/reportContent/types.ts index a7582a290..cc82f2388 100644 --- a/client/src/lib/reportContent/types.ts +++ b/client/src/lib/reportContent/types.ts @@ -13,8 +13,9 @@ export interface ReportContentRow { statusText: string | null; // null when useCase !== 'budget-overview' invoiceAmountText: string; allocatedAmountValueText: string; // formatted currency only — no markers/refund note - allocatedMarkers: string; // '', '†', '‡', '†‡' — shared/unnumbered per report - isDeposit: boolean; // constituted-deposit row → inline Deposit badge, no marker + isSplit: boolean; // split invoice with budget lines → inline "partial" label + isDepositReduced: boolean; // split invoice reduced by untagged deposits → inline label + isDeposit: boolean; // constituted-deposit row → inline Deposit badge isRefund: boolean; refundNoteText: string; // shown only when isRefund usageText: string; // EDITABLE — key `row..usageText` @@ -55,6 +56,8 @@ export interface ReportContentLabels { usage: string; attachmentsNote: string; deposit: string; // translated in report language + splitNote: string; // short inline label for split rows + depositReducedNote: string; // short inline label for deposit-reduced rows source: string; sourceType: string; reference: string; diff --git a/client/src/lib/reportPdf/coverLetterPdf.ts b/client/src/lib/reportPdf/coverLetterPdf.ts index e61c2705b..5bdc9608c 100644 --- a/client/src/lib/reportPdf/coverLetterPdf.ts +++ b/client/src/lib/reportPdf/coverLetterPdf.ts @@ -56,12 +56,23 @@ export function buildCoverLetterContent(reportContent: ReportContent, t: TFuncti margin: [0, 0, 0, 16], }); - // Body text — literal blank-line rendering, no paragraph-spacing model (see spec §B). - content.push({ - text: coverLetter.body, - style: 'normal', - margin: [0, 0, 0, 32], - }); + // Body text — split on double newlines so AI-generated paragraphs render with spacing + const paragraphs = coverLetter.body.split(/\n\n+/).filter(Boolean); + if (paragraphs.length <= 1) { + content.push({ + text: coverLetter.body, + style: 'normal', + margin: [0, 0, 0, 32], + }); + } else { + for (let i = 0; i < paragraphs.length; i++) { + content.push({ + text: paragraphs[i]!, + style: 'normal', + margin: [0, 0, 0, i === paragraphs.length - 1 ? 32 : 8], + }); + } + } // Signature block — closing + reserved blank space + name are ALWAYS emitted together (AC 2.4), // never gated behind `if (coverLetter.signature)`: pdfmake reserves the same line height for an diff --git a/client/src/lib/reportPdf/overviewPdf.test.ts b/client/src/lib/reportPdf/overviewPdf.test.ts index 213ddca5b..68e8d0744 100644 --- a/client/src/lib/reportPdf/overviewPdf.test.ts +++ b/client/src/lib/reportPdf/overviewPdf.test.ts @@ -73,7 +73,8 @@ function makeRow(overrides: Partial = {}): ReportContentRow { statusText: null, invoiceAmountText: '€1000.00', allocatedAmountValueText: '€1000.00', - allocatedMarkers: '', + isSplit: false, + isDepositReduced: false, isDeposit: false, isRefund: false, refundNoteText: 'sourceReports.table.refundNote', @@ -100,6 +101,8 @@ function makeLabels(): ReportContent['labels'] { usage: 'sourceReports.table.usage', attachmentsNote: 'sourceReports.editable.attachmentsNoteLabel', deposit: 'sourceReports.table.attachmentType.deposit', + splitNote: 'sourceReports.table.splitInlineLabel', + depositReducedNote: 'sourceReports.table.depositReducedInlineLabel', source: 'sourceReports.table.source', sourceType: 'sourceReports.table.sourceType', reference: 'sourceReports.table.reference', @@ -767,8 +770,8 @@ describe('buildOverviewContent — row rendering (consumes already-derived Repor }); }); - describe('Usage cell: always a plain { text } cell — areaText/attachmentsNote render as SEPARATE continuation rows (#1929 round 4 architect review HIGH: the round-3 stack: [usageChunk, areaText, attachmentsNote] construction left their COMBINED height in one cell unbounded and silently dropped rows needing 3+/9+ pages; each field now gets its own independently-chunked row(s), never sharing a cell with usageText or with each other)', () => { - it('renders a plain { text } cell with no extra rows when both areaText and attachmentsNote are null', () => { + describe('Usage cell: plain text vs inline grey meta text', () => { + it('renders a plain { text } cell (not a stack) when both areaText and attachmentsNote are null', () => { const row = makeRow({ usageText: 'Kitchen work', areaText: null, attachmentsNote: null }); const content = makeContent({ rows: [row] }); const result = buildOverviewContent(content, new Map(), t); @@ -780,27 +783,23 @@ describe('buildOverviewContent — row rendering (consumes already-derived Repor expect(table.body).toHaveLength(3); }); - it('renders the usage row PLUS one continuation row (style "small") for attachmentsNote — never stacked into the usage cell', () => { + it('renders a text array with a grey newline run when attachmentsNote is present', () => { const row = makeRow({ usageText: 'Kitchen work', attachmentsNote: '1 attachment: Invoice' }); const content = makeContent({ rows: [row] }); const result = buildOverviewContent(content, new Map(), t); const table = getTable(result); - - const usageCell = (table.body[1] as unknown[])[5] as { text?: unknown; stack?: unknown }; - expect(usageCell.stack).toBeUndefined(); - expect(usageRunsText(usageCell.text)).toBe('Kitchen work'); - - // header (1) + usage row (1) + attachmentsNote continuation row (1) + summary row (1) = 4. - expect(table.body).toHaveLength(4); - const noteRow = table.body[2] as { text?: unknown; style?: string }[]; - // Leading/amount cells on the continuation row are all blank. - expect(rowTexts(noteRow).slice(0, 5)).toEqual(['', '', '', '', '']); - const noteCell = noteRow[5] as { text: unknown; style?: string }; - expect(usageRunsText(noteCell.text)).toBe('1 attachment: Invoice'); - expect(noteCell.style).toBe('small'); + const cell = (table.body[1] as unknown[])[5] as { + text: { text: string; color?: string }[]; + stack?: unknown; + }; + expect(cell.stack).toBeUndefined(); + expect(Array.isArray(cell.text)).toBe(true); + expect(cell.text[0]!.text).toBe('Kitchen work'); + expect(cell.text[1]!.text).toContain('1 attachment: Invoice'); + expect(cell.text[1]!.color).toBe('#6b7280'); }); - it('AC5.2: renders the usage row, then an areaText continuation row, then an attachmentsNote continuation row — in that order, each its own row', () => { + it('AC5.2: renders areaText and attachmentsNote joined in the grey meta run when both are present', () => { const row = makeRow({ usageText: 'Kitchen work', areaText: 'Ground Floor', @@ -809,20 +808,16 @@ describe('buildOverviewContent — row rendering (consumes already-derived Repor const content = makeContent({ rows: [row] }); const result = buildOverviewContent(content, new Map(), t); const table = getTable(result); - - // header (1) + usage row + areaText row + attachmentsNote row + summary row = 5. - expect(table.body).toHaveLength(5); - const usageCell = (table.body[1] as unknown[])[5] as { text: unknown }; - const areaCell = (table.body[2] as unknown[])[5] as { text: unknown; style?: string }; - const noteCell = (table.body[3] as unknown[])[5] as { text: unknown; style?: string }; - expect(usageRunsText(usageCell.text)).toBe('Kitchen work'); - expect(usageRunsText(areaCell.text)).toBe('Ground Floor'); - expect(areaCell.style).toBe('small'); - expect(usageRunsText(noteCell.text)).toBe('1 attachment: Invoice'); - expect(noteCell.style).toBe('small'); + const cell = (table.body[1] as unknown[])[5] as { + text: { text: string; color?: string }[]; + }; + expect(cell.text[0]!.text).toBe('Kitchen work'); + expect(cell.text[1]!.text).toContain('Ground Floor'); + expect(cell.text[1]!.text).toContain('1 attachment: Invoice'); + expect(cell.text[1]!.color).toBe('#6b7280'); }); - it('renders the usage row plus only an areaText continuation row when areaText is present but attachmentsNote is null', () => { + it('renders areaText alone in the grey meta run when attachmentsNote is null', () => { const row = makeRow({ usageText: 'Kitchen work', areaText: 'Ground Floor', @@ -831,10 +826,14 @@ describe('buildOverviewContent — row rendering (consumes already-derived Repor const content = makeContent({ rows: [row] }); const result = buildOverviewContent(content, new Map(), t); const table = getTable(result); - // header (1) + usage row + areaText row + summary row = 4. - expect(table.body).toHaveLength(4); - const areaCell = (table.body[2] as unknown[])[5] as { text: unknown }; - expect(usageRunsText(areaCell.text)).toBe('Ground Floor'); + const cell = (table.body[1] as unknown[])[5] as { + text: { text: string; color?: string }[]; + stack?: unknown; + }; + expect(cell.stack).toBeUndefined(); + expect(cell.text[0]!.text).toBe('Kitchen work'); + expect(cell.text[1]!.text).toContain('Ground Floor'); + expect(cell.text[1]!.color).toBe('#6b7280'); }); it('[#1929 round 2] the plain-cell Usage text is a run array of the individual whitespace-preserving tokens (buildUsageTextRuns wiring, not a plain string)', () => { @@ -848,34 +847,40 @@ describe('buildOverviewContent — row rendering (consumes already-derived Repor }); }); - describe('allocated cell composition (skip markers + allocatedMarkers + refund note)', () => { + describe('allocated cell composition (skip markers + inline labels + refund note)', () => { it('renders allocatedAmountValueText plain when there are no markers and not a refund', () => { - const row = makeRow({ allocatedAmountValueText: '€400.00', allocatedMarkers: '' }); + const row = makeRow({ allocatedAmountValueText: '€400.00' }); const content = makeContent({ rows: [row] }); const result = buildOverviewContent(content, new Map(), t); const table = getTable(result); expect(rowTexts(table.body[1])[4]).toBe('€400.00'); }); - it('appends the pre-computed, unnumbered/shared split+deposit markers verbatim (already formatted by buildReportContent)', () => { - const row = makeRow({ allocatedAmountValueText: '€400.00', allocatedMarkers: '†‡' }); + it('appends inline isSplit label when isSplit=true', () => { + const row = makeRow({ allocatedAmountValueText: '€400.00', isSplit: true }); const content = makeContent({ rows: [row] }); const result = buildOverviewContent(content, new Map(), t); const table = getTable(result); - expect(rowTexts(table.body[1])[4]).toBe('€400.00†‡'); + expect(rowTexts(table.body[1])[4]).toContain('€400.00'); + expect(rowTexts(table.body[1])[4]).toContain('sourceReports.table.splitInlineLabel'); }); - it('prepends skip-footnote markers (*N) BEFORE the allocatedMarkers, numbered from skippedDocuments', () => { - const row = makeRow({ - invoiceId: 'inv-1', - allocatedAmountValueText: '€400.00', - allocatedMarkers: '†', - }); + it('appends inline isDepositReduced label when isDepositReduced=true', () => { + const row = makeRow({ allocatedAmountValueText: '€400.00', isDepositReduced: true }); + const content = makeContent({ rows: [row] }); + const result = buildOverviewContent(content, new Map(), t); + const table = getTable(result); + expect(rowTexts(table.body[1])[4]).toContain('€400.00'); + expect(rowTexts(table.body[1])[4]).toContain('sourceReports.table.depositReducedInlineLabel'); + }); + + it('prepends skip-footnote markers (*N) before the allocated value, numbered from skippedDocuments', () => { + const row = makeRow({ invoiceId: 'inv-1', allocatedAmountValueText: '€400.00' }); const content = makeContent({ rows: [row] }); const skipped = new Map([['inv-1', ['footnoteFetchFailed']]]); const result = buildOverviewContent(content, skipped, t); const table = getTable(result); - expect(rowTexts(table.body[1])[4]).toBe('€400.00*1†'); + expect(rowTexts(table.body[1])[4]).toBe('€400.00*1'); }); it('numbers multiple skip reasons on the same invoice sequentially', () => { diff --git a/client/src/lib/reportPdf/overviewPdf.ts b/client/src/lib/reportPdf/overviewPdf.ts index b51a8cfc1..02aa870d3 100644 --- a/client/src/lib/reportPdf/overviewPdf.ts +++ b/client/src/lib/reportPdf/overviewPdf.ts @@ -581,45 +581,18 @@ export function buildOverviewContent( const usageSafeTokenChars = reportContent.isOverview ? USAGE_SAFE_TOKEN_CHARS_7COL : USAGE_SAFE_TOKEN_CHARS_6COL; - const smallSafeTokenChars = reportContent.isOverview - ? SMALL_SAFE_TOKEN_CHARS_7COL - : SMALL_SAFE_TOKEN_CHARS_6COL; - - /** - * Pushes one continuation row (empty leading/amount cells) per chunk of `text`, at `style`. - * Used for usageText overflow, and — separately, never combined with usageText or with each - * other — for areaText/attachmentsNote (#1929 round-4 architect review HIGH: each field gets - * its own row(s) so no row's Usage cell ever holds more than one bounded chunk of one field; - * see MAX_SAFE_USAGE_CHUNK_CHARS's comment for what "bounded" means and how it was measured). - */ - function pushChunkedRows( - text: string, - maxChunkChars: number, - safeTokenCharsForStyle: number, - style: 'tableCell' | 'small', - ): void { - for (const chunk of splitIntoPageSafeChunks(text, maxChunkChars)) { - const cell: Content = { text: buildUsageTextRuns(chunk, safeTokenCharsForStyle), style }; - rows.push([ - ...buildEmptyLeadingCells(reportContent.isOverview), - ...buildEmptyAmountCells(), - cell, - ]); - } - } for (const contentRow of reportContent.rows) { - // Allocated amount with footnote markers (skip + allocated) + // Allocated amount with skip markers and inline labels const skipMarkers = skipFootnotesByInvoiceId.get(contentRow.invoiceId) ?? []; - let markerText = ''; + let skipMarkerText = ''; for (const noteNum of skipMarkers) { - markerText += `*${noteNum}`; + skipMarkerText += `*${noteNum}`; } - markerText += contentRow.allocatedMarkers; - // Build allocated runs: value+markers, then optional deposit badge, then optional refund note + // Build allocated runs: value+skip markers, then optional inline labels, then optional refund note const allocatedRuns: Content[] = [ - { text: `${contentRow.allocatedAmountValueText}${markerText}` }, + { text: `${contentRow.allocatedAmountValueText}${skipMarkerText}` }, ]; if (contentRow.isDeposit) { allocatedRuns.push({ @@ -628,6 +601,20 @@ export function buildOverviewContent( fontSize: DEPOSIT_NOTE_FONT_SIZE, }); } + if (contentRow.isSplit) { + allocatedRuns.push({ + text: ` (${reportContent.labels.splitNote})`, + color: DEPOSIT_NOTE_TEXT_COLOR, + fontSize: DEPOSIT_NOTE_FONT_SIZE, + }); + } + if (contentRow.isDepositReduced) { + allocatedRuns.push({ + text: ` (${reportContent.labels.depositReducedNote})`, + color: DEPOSIT_NOTE_TEXT_COLOR, + fontSize: DEPOSIT_NOTE_FONT_SIZE, + }); + } if (contentRow.isRefund) { allocatedRuns.push({ text: ` ${contentRow.refundNoteText}` }); } @@ -637,13 +624,18 @@ export function buildOverviewContent( // table rows instead of one unbreakable (and potentially content-dropping) row. The FIRST // chunk shares this invoice's leading/amount-cell row; any further chunks (rare-by- // construction: AC12 requires 600 chars with zero degradation, well under - // MAX_SAFE_USAGE_CHUNK_CHARS) get their own continuation row via pushChunkedRows — no - // "continued" marker, per the product-owner's explicit ruling. + // MAX_SAFE_USAGE_CHUNK_CHARS) get their own continuation row — no "continued" marker, + // per the product-owner's explicit ruling. const usageChunks = splitIntoPageSafeChunks(contentRow.usageText, MAX_SAFE_USAGE_CHUNK_CHARS); - const firstUsageCell: Content = { - text: buildUsageTextRuns(usageChunks[0]!, usageSafeTokenChars), - style: 'tableCell', - }; + // Area and attachmentsNote appear inline in grey after the first usage chunk + const metaPieces: string[] = []; + if (contentRow.areaText) metaPieces.push(contentRow.areaText); + if (contentRow.attachmentsNote) metaPieces.push(contentRow.attachmentsNote); + const firstUsageRuns = buildUsageTextRuns(usageChunks[0]!, usageSafeTokenChars); + if (metaPieces.length > 0) { + firstUsageRuns.push({ text: '\n' + metaPieces.join(' · '), color: DEPOSIT_NOTE_TEXT_COLOR }); + } + const firstUsageCell: Content = { text: firstUsageRuns, style: 'tableCell' }; rows.push([ ...buildLeadingCells(contentRow, reportContent.isOverview, contentRow.statusText ?? ''), ...buildAmountCells(contentRow, allocatedRuns), @@ -660,28 +652,6 @@ export function buildOverviewContent( cell, ]); } - - // areaText and attachmentsNote each get their OWN continuation row(s) — never stacked into - // the usage row's cell (#1929 round-4 architect review HIGH: attachmentsNote has no - // maxLength anywhere, and areaText is aggregate-unbounded across N leaf areas, so their - // combined height with usageText in one cell was unbounded and silently dropped rows that - // needed more than one page). - if (contentRow.areaText) { - pushChunkedRows( - contentRow.areaText, - MAX_SAFE_SMALL_CHUNK_CHARS, - smallSafeTokenChars, - 'small', - ); - } - if (contentRow.attachmentsNote) { - pushChunkedRows( - contentRow.attachmentsNote, - MAX_SAFE_SMALL_CHUNK_CHARS, - smallSafeTokenChars, - 'small', - ); - } } // Add summary rows from reportContent.summaryRows @@ -727,7 +697,7 @@ export function buildOverviewContent( margin: [0, 0, 0, 20], }); - // Add footnotes (skip block + split/deposit from reportContent.footnotes) + // Add footnotes (skip block only; split/deposit annotations are now rendered inline) const footnotes: Content[] = []; // Skip block (generation-time data) diff --git a/client/src/lib/reportPdf/realRender.test.ts b/client/src/lib/reportPdf/realRender.test.ts index e6bb190dd..9b7968262 100644 --- a/client/src/lib/reportPdf/realRender.test.ts +++ b/client/src/lib/reportPdf/realRender.test.ts @@ -914,14 +914,14 @@ describe('report PDF pipeline — real, unmocked end-to-end render', () => { household: null, }); - // Sanity: the constituted-deposit row carries isDeposit=true and no ‡ marker; the - // reduced-deposit row carries the ‡ marker and isDeposit=false. + // Sanity: the constituted-deposit row carries isDeposit=true; the + // reduced-deposit row carries isDepositReduced=true and isDeposit=false. const constitutedRow = content.rows.find((r) => r.invoiceId === 'inv-deposit-constituted')!; expect(constitutedRow.isDeposit).toBe(true); - expect(constitutedRow.allocatedMarkers).not.toContain('‡'); + expect(constitutedRow.isDepositReduced).toBe(false); const reducedRow = content.rows.find((r) => r.invoiceId === 'inv-deposit-reduced')!; expect(reducedRow.isDeposit).toBe(false); - expect(reducedRow.allocatedMarkers).toContain('‡'); + expect(reducedRow.isDepositReduced).toBe(true); const pdfContent = buildOverviewContent(content, new Map(), t); const tableItem = pdfContent.find( From 9eaa92955eccd9fb6cd66e77475e2e5d6d1f0ef5 Mon Sep 17 00:00:00 2001 From: Frank Steiler Date: Mon, 3 Aug 2026 18:10:21 +0200 Subject: [PATCH 2/6] fix(reports): bound the inline usage meta so the PDF stops dropping content MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The inline-meta change appended areaText/attachmentsNote as one unchunked run on the usage cell. With table.dontBreakRows, pdfmake does not paginate an over-tall row — it silently discards the overflow. This reintroduced the #1929 round-4 content-loss regression: page count saturated at 2 and went non-monotonic (3 -> 2 as content grew) while rendered line count kept rising linearly, so ~7 pages of measured content were thrown away. Reachable from ordinary data: attachmentsNote has no maxLength and areaText is unbounded across leaf areas. packUsageCellRows() now bounds the whole rendered cell stream rather than usageText alone, splitting via the existing splitIntoPageSafeChunks(). Page count is monotonic again and identical whichever channel carries the text. Where the cell fits one page-safe row — the dominant case — output is byte-identical. The grey suffix now lands on the last chunk row rather than the first: pinning it to row 0 rendered meta mid-prose with more usage below it, and forced the usage chunk boundary to shrink. Also updates the tests that encoded the unbounded model. Two of them passed while the PDF lost content, because they inspected the pdfmake content tree rather than rendered output; the guard is now framed as rendered-output-only and asserts that the same text costs the same number of pages whichever channel carries it. Adds coverage for packUsageCellRows (none existed), the inline split/deposit labels, and the column toggles. Removes the small-chunk constants, now dead. E2E: reportWizard scenarios updated for the renamed usage-meta class, the removed attachments field, and the footnote-to-inline-label change. German strings verified against the glossary; abzgl. Abschlag kept over the approved Abschlagszahlung because the compliant term measures 96pt against a 75pt column. Co-Authored-By: Claude frontend-developer Co-Authored-By: Claude qa-integration-tester Co-Authored-By: Claude e2e-test-engineer Co-Authored-By: Claude translator --- .../agent-memory/e2e-test-engineer/MEMORY.md | 1 + .../issue-1959-inline-meta-and-labels.md | 55 ++ .../agent-memory/frontend-developer/MEMORY.md | 1 + .../frontend-developer/qa-tripwire-tests.md | 30 + .../qa-integration-tester/MEMORY.md | 1 + .../pr-1959-inline-meta-content-loss.md | 114 ++++ .../reports/ReportContentEditor.test.tsx | 482 +++++++++++++-- client/src/lib/reportContent/types.ts | 6 +- .../src/lib/reportPdf/coverLetterPdf.test.ts | 2 + client/src/lib/reportPdf/merge.test.ts | 2 + client/src/lib/reportPdf/overviewPdf.test.ts | 442 +++++++++++--- client/src/lib/reportPdf/overviewPdf.ts | 274 ++++++--- client/src/lib/reportPdf/pageGeometry.test.ts | 9 +- client/src/lib/reportPdf/pageGeometry.ts | 12 +- client/src/lib/reportPdf/realRender.test.ts | 572 ++++++++++++++---- e2e/pages/ReportWizardPage.ts | 134 ++-- .../budget/reportWizardAiGeneration.spec.ts | 32 +- .../reportWizardEditableContent.spec.ts | 235 +++++-- 18 files changed, 1909 insertions(+), 495 deletions(-) create mode 100644 .claude/agent-memory/e2e-test-engineer/issue-1959-inline-meta-and-labels.md create mode 100644 .claude/agent-memory/frontend-developer/qa-tripwire-tests.md create mode 100644 .claude/agent-memory/qa-integration-tester/pr-1959-inline-meta-content-loss.md diff --git a/.claude/agent-memory/e2e-test-engineer/MEMORY.md b/.claude/agent-memory/e2e-test-engineer/MEMORY.md index dcd665f33..c2037cf37 100644 --- a/.claude/agent-memory/e2e-test-engineer/MEMORY.md +++ b/.claude/agent-memory/e2e-test-engineer/MEMORY.md @@ -29,6 +29,7 @@ - [story-1901-ai-report-generation.md](story-1901-ai-report-generation.md) — AI-generated usage descriptions/cover letter: new `reportWizardAiGeneration.spec.ts` + POM AI locators; llmEnabled-mock pattern; overwrite-modal-guard-checks-overrides-not-aiContent gotcha; cover-letter-fields-need-contactAddress/reference seed trap; reused auto-itemize LLM error strings. - [story-1900-editable-report-preview.md](story-1900-editable-report-preview.md) — step 5 reworked from always-present auto-regenerating iframe → live editable surface + on-demand PDF Modal; POM rename `waitForPreviewReady/Regenerated` → `openPdfPreviewModal`/`closePdfPreviewModal` (must close before another modal-opening action); 4 filed bugs (#1904-#1907, all now FIXED+CLOSED, see file's re-verification section); deliberate deviation editing a file explicitly marked "do not touch" because leaving it broke `tsc` workspace-wide — see file for the general lesson; `footnoteFetchFailed` skip note naturally reachable with no Paperless container, no mock needed; re-verification added `mobileCard()`/`mobileUsageField()` POM locators + Scenario 15 (#1907 regression guard) + found/filed NEW bug #1908 (mobile-card fallback visible on desktop, no default `display:none`) with its own expected-to-fail Scenario 1b. +- [issue-1959-inline-meta-and-labels.md](issue-1959-inline-meta-and-labels.md) — #1959 reversed #1923's †/‡ footnotes → inline `(partial)`/`(less deposit)` and merged area+attachments into one `.usageMetaText` line; POM renames, rewritten scenarios 2/17/18/20 + AI 8, attachment-tier facts, untested column toggles. - [claim-deposit-scope-1922.md](claim-deposit-scope-1922.md) — PR #1922 invoice/deposit claim-scope split: `handleMarkClaimed`'s two-array submit, server-truth success-banner counts, the three "deposit surfaces the invoice" shapes, `claimNothingClaimable` guard. ## Open follow-ups to flag to orchestrator diff --git a/.claude/agent-memory/e2e-test-engineer/issue-1959-inline-meta-and-labels.md b/.claude/agent-memory/e2e-test-engineer/issue-1959-inline-meta-and-labels.md new file mode 100644 index 000000000..5375285b2 --- /dev/null +++ b/.claude/agent-memory/e2e-test-engineer/issue-1959-inline-meta-and-labels.md @@ -0,0 +1,55 @@ +--- +name: issue-1959-inline-meta-and-labels +description: PR #1959 reversed two earlier report-table designs (†/‡ shared footnotes from #1923, distinct area sub-line) into inline labels + one combined meta line; which E2E locators/scenarios had to be rewritten and how each new assertion was made non-vacuous. +metadata: + type: project +--- + +PR #1959 ("improve report PDF UX") deliberately **superseded** two designs earlier rounds had +asked for, in `ReportContentEditor.tsx` / `buildReportContent.ts`: + +1. `†`/`‡` markers + the shared footnote list (Story #1923 AC1) → grey inline `` in the **Allocated Amount cell**: `(partial)` / `(less deposit)` + (de `(Teilbetrag)` / `(abzgl. Abschlag)`). `ReportContentRow.allocatedMarkers` → `isSplit` / + `isDepositReduced` booleans. `buildReportContent` now pushes **zero** footnotes, so + `.footnotes` has no producer at all — `footnotesBlock`/`footnoteItems` survive in the POM as + **negative-only** guards. +2. `.usageAreaText` sub-line + the separate editable `Attachments Note` column → ONE read-only + `.usageMetaText` line inside the Usage cell: `[areaText, attachmentsNote].join(' · ')` + (U+00B7 middle dot, spaces on both sides). The `attachmentsNote` `EditableField` is gone + entirely, so a content-table row / mobile card now has **exactly one textbox** (Usage) — the + crispest available guard against that column coming back. + +**Why:** the user owns #1959 and asked for it in the promotion; source-of-truth hierarchy makes +the PR body the spec, so the tests were rewritten, not the code. + +**How to apply:** POM renames are `usageAreaText`→`usageMetaText`, +`mobileUsageAreaText`→`mobileUsageMetaText`, plus new `inlineNote()`/`mobileInlineNote()`; +`attachmentsNoteField()` deleted. Rewritten scenarios: editableContent 2, 17, 18, 20 and +aiGeneration 8. Every "old design is gone" negative is paired with a positive so it cannot pass +against a mis-seeded page (e.g. Scenario 18 asserts `(partial)` present *and* `†`/`‡` absent +*and* the long-form footnote sentence absent from `main`; Scenario 20 asserts the attachments +note text IS rendered *and* the row has one textbox). + +Facts worth reusing: + +- `toHaveText`/`toContainText` normalize whitespace, so the desktop `inlineNote` span's leading + space (` (partial)`) is absorbed — `toHaveText('(partial)')` is correct. +- Attachment note text for a `claim` report with `attachmentType: 'invoice'` is + `1 attachment: Invoice`. The tier gate is + `server/src/services/shared/attachmentTierUtils.ts`: floors are quotation(1) for + budget-overview, deposit(2) for claim, invoice(3) for proof-of-funds; `null` counts as tier 3. + So a 'invoice'-tagged link shows up in claim reports — don't guess, that file is the only + definition. +- `document_links` is unique on `(entity_type, entity_id, paperless_document_id)`, so a + hardcoded `paperlessDocumentId` in a spec cannot collide across parallel workers/projects. +- `AppShell.tsx` renders a real `
` element, so `page.locator('main')` is a safe + page-scope text container. +- **Column-visibility checkboxes (`role="group"`, "Show/hide columns") shipped with #1959 with + NO E2E coverage** — deliberately not added on the critical path, because a new test case + reshuffles shard membership. Pick this up when the promotion isn't blocking. +- Sub-agents share the worktree here: a `prettier --check` on a `client/` file can transiently + fail because another agent is mid-write. Re-check before reporting it as broken. + +See [[known-flakes-and-regressions]], [[story-1900-editable-report-preview]], +[[story-1879-report-wizard]]. diff --git a/.claude/agent-memory/frontend-developer/MEMORY.md b/.claude/agent-memory/frontend-developer/MEMORY.md index f8666cdbf..aed9b8519 100644 --- a/.claude/agent-memory/frontend-developer/MEMORY.md +++ b/.claude/agent-memory/frontend-developer/MEMORY.md @@ -98,6 +98,7 @@ removing it will fail typecheck. Keep the export and note it as QA-deferred. - `budget-invoices.md` — open for budget/invoice/subsidy pages: BudgetSubNav, budget line APIs, invoice CRUD patterns, pipeline dashboard cards - `gantt-calendar.md` — open for Timeline/Gantt/Calendar/Milestone work: SVG architecture, drag hook (React 19 refs rule), arrow hover, lane allocation, constraints section, related E2E POM layout - `api-routing.md` — open when calling APIs or navigating: UserResponse/PaginatedResponse shapes, auth routes, dependency direction-swap semantics, navigation origin state +- `qa-tripwire-tests.md` — open when a task says a fix will flip an `it.failing` guard: why the flip signal can be swallowed by a shared helper, and what to do instead - `testing-mocks.md` — open when your change will break existing tests: ESM mock patterns, mock-block update checklist, Toast/context mocks, DefinePlugin globals in Jest - `git-tooling.md` — open on webpack config changes or git/CI trouble: worktree remotes, object corruption/permission recovery, npm audit policy, Prettier/CI format gap - `photo-metadata-sidepanel.md` — photo metadata side panel notes (open for photo UI work) diff --git a/.claude/agent-memory/frontend-developer/qa-tripwire-tests.md b/.claude/agent-memory/frontend-developer/qa-tripwire-tests.md new file mode 100644 index 000000000..14fff6f1d --- /dev/null +++ b/.claude/agent-memory/frontend-developer/qa-tripwire-tests.md @@ -0,0 +1,30 @@ +--- +name: qa-tripwire-tests +description: QA encodes known production defects as `it.failing` tripwires; a shared helper that hard-codes the broken model can swallow the flip signal, so verify the fix independently +metadata: + type: project +--- + +QA (`qa-integration-tester`) encodes a known-but-unfixed production defect as `it.failing` with a +passing control test alongside. The suite stays green while the defect is open; when production is +fixed the test starts passing and Jest errors with "Failing test passed even though it was supposed +to fail" — that error is the intended confirmation signal, and the fixing agent is expected to flip +`it.failing` → `it` (the one sanctioned developer edit to a test file). + +**Why:** it keeps the defect encoded in executable code rather than only in a report, so a fix can't +land without someone noticing the guard. + +**How to apply:** the flip signal is NOT reliable on its own. If the tripwire calls a shared render +helper that asserts the _broken_ model (e.g. `expect(body).toHaveLength(1 + usageChunkCount + 1)`, +which bakes in "meta never adds rows"), a correct fix makes the helper throw first — the tripwire +then still "passes" as a failing test and no signal appears. Seen on #1959 (`realRender.test.ts` +cell-scope block): the fix was verified, the tripwire never flipped. + +So: (1) always verify the fix independently with your own scratch render/measurement harness before +trusting or distrusting the tripwire; (2) if the flip leaves the test failing inside a shared helper +rather than at its own assertions, revert to `it.failing`, leave QA's file byte-identical, and report +the exact helper line QA must update — do not "fix" the helper to make the flip work; +(3) expect sibling tests in the same block to fail too, for the same reason (they assert the broken +model at content-tree level, which is why they passed while the rendered PDF lost content). + +Related: [[../MEMORY.md]] "Refinement Workflow — QA Test Coordination". diff --git a/.claude/agent-memory/qa-integration-tester/MEMORY.md b/.claude/agent-memory/qa-integration-tester/MEMORY.md index a42ecf663..0a27ee35a 100644 --- a/.claude/agent-memory/qa-integration-tester/MEMORY.md +++ b/.claude/agent-memory/qa-integration-tester/MEMORY.md @@ -15,6 +15,7 @@ ## Recent bug/story notes (2026-08) +- [PR #1959 — inline meta content loss + `it.failing` tripwires](pr-1959-inline-meta-content-loss.md) (2026-08-03, RESOLVED) — prod defect found+fixed (unchunked meta in a `dontBreakRows` cell silently drops pages); **a tripwire is worthless if a shared helper bakes in the buggy assumption** — mine nearly stayed green through the fix; channel-equivalence is the threshold-free assertion; tree-level assertions cannot see this bug class; `grep` silently returns nothing on these test files (use `awk`). - [Bug #1955 — echo-race harness + mutation probes](bug-1955-echo-race-harness.md) (2026-08-03) — echo must fire on the write's *resolve* (not the call) or the queue fix masks the guard and the test passes pre-fix; `rerender()` stands in for the optimistic `setPreferences`; 4 perl mutation probes prove each test guards a distinct part of the fix; never run repo-wide `npm run format` (38 unrelated files drift). - [Story #1930 — attachment tier rules](story-1930-attachment-tier.md) (2026-08-02) — new `attachmentTierUtils.test.ts` (100% cov, table+explicit-null pattern); sourceReportService.test.ts 16a/b/c/e → AC1/AC2/AC3/AC5; cross-report-type test needs a fresh invoice per report type; round 2 added a genuinely discriminating proof-of-funds fixture (deposit-only invoice, tagged+claimed deposit) after PO flagged the original AC3/16 pof blocks weren't change-detecting — inline-swap isolation technique to prove it fails on old logic. diff --git a/.claude/agent-memory/qa-integration-tester/pr-1959-inline-meta-content-loss.md b/.claude/agent-memory/qa-integration-tester/pr-1959-inline-meta-content-loss.md new file mode 100644 index 000000000..4f76d8f57 --- /dev/null +++ b/.claude/agent-memory/qa-integration-tester/pr-1959-inline-meta-content-loss.md @@ -0,0 +1,114 @@ +--- +name: pr-1959-inline-meta-content-loss +description: PR #1959 (report PDF UX) reintroduced and then fixed the #1929 round-4 PDF content-loss regression; records the packUsageCellRows bound, the measured page-count saturation table, the it.failing tripwire technique AND the trap that nearly disarmed it (a helper that bakes in the buggy assumption), and the run-index gotcha for grey meta runs +metadata: + type: project +--- + +# PR #1959 — inline meta content loss: the defect, the fix, and the tripwire trap + +## Status: RESOLVED (2026-08-03, branch `fix/1959-pdf-ux-green`) + +Production is fixed; the guard tests are normal `it` again. Everything below is the reusable lesson. + +## The defect + +`overviewPdf.ts` `buildOverviewContent` appended `areaText`/`attachmentsNote` as ONE **unchunked** +grey run on a usage-chunk row's cell. With `table.dontBreakRows: true` pdfmake does not paginate an +over-tall row — it measures the text and then **silently drops** whatever doesn't fit. #1929 round 4 +had bounded exactly this by giving each field its own chunked continuation row; #1959's inline move +removed the bound without replacing it. + +**Why it happened:** the PR body asked for inline rendering and the implementer took it literally +without re-bounding the *combined* cell height. `attachmentsNote` has no maxLength anywhere (editor +or server) and `areaText` is aggregate-unbounded across N leaf areas, so ordinary user data reached it. + +**Measured evidence** (real unmocked renders, production 6-column shape). Rendered *line* count kept +growing linearly (~0.023 lines/char) while page count saturated — proof pdfmake measured everything +then discarded it: + +| chars | usageText (chunked) | attachmentsNote — before fix | after fix | +| --- | --- | --- | --- | +| 2000 | 6 rows / 2 pages | 3 rows / 3 pages | 6 rows / 2 pages | +| 4000 | 9 rows / 3 pages | 3 rows / **2** pages | 9 rows / 3 pages | +| 8000 | 15 rows / 5 pages | 3 rows / **2** pages | 15 rows / 5 pages | +| 16000 | 27 rows / 9 pages | 3 rows / **2** pages | 27 rows / 9 pages | + +Page count was even non-monotonic (3 → 2 as content grew). Reproduces the architect's original tell +verbatim: "rows requiring 3 and 9 pages both rendered as 2". + +## The fix, and what it changed for tests + +`packUsageCellRows(segments, maxChars)` packs the cell's **whole** content stream (prose, then the +grey meta segment) into per-row groups of ≤ `MAX_SAFE_USAGE_CHUNK_CHARS`. Lossless; byte-identical +output when the whole cell fits one row (the dominant case). + +Two consequences tests must encode, not work around: + +1. **The suffix now lands on the LAST row, not the first.** Pinning it to row 0 rendered grey meta + mid-prose with more usage below it, and shrank the prose's chunk boundary (adding rows). +2. **The suffix can span many rows** — one grey run per row, always last within its own cell. So + reconstructing it means concatenating the grey run of *every* row in the group. + +`MAX_SAFE_SMALL_CHUNK_CHARS` and `SMALL_SAFE_TOKEN_CHARS_7COL/_6COL` were 9pt ceilings for the +continuation rows that no longer exist; the meta renders at 8pt `tableCell`, so the usage budget is +the right one. All three (plus the orphaned `SMALL_WORST_CASE_CHAR_WIDTH_PT`) were deleted along with +their imports and value-only tests. `TABLE_SMALL_FONT_SIZE` stays — `PDF_STYLES.small` still renders +footnotes, cover-letter date/reference lines, and the running header/footer. + +## Technique: `it.failing` as a tripwire for a known-open production defect + +Jest 30 here supports `it.failing` / `test.failing`. It is the right primitive when QA must leave a +suite green but must not bless broken behaviour: the suite reports **passed** while production is +broken, and the moment production is fixed Jest errors with `Failing test passed even though it was +supposed to fail`, forcing a flip back to `it`. Verify the guard is live by temporarily relaxing its +threshold and confirming that error appears. + +### The trap that nearly disarmed it — check this every time + +**A tripwire is worthless if a shared helper bakes in the very assumption the bug lives in.** Here +`renderCellScopeRow` derived its expected row count from `usageText` alone — i.e. it asserted "the +meta never adds rows", which is precisely what the bug did. Any correct fix *must* add rows, so the +helper threw on its own row-count assertion before the test reached its page-count assertion: the +test failed for the wrong reason and `it.failing` stayed green through the fix. + +Rules that follow: +- Derive expected shape from the **same function production uses** (here `packUsageCellRows`), never + from a re-derivation of one input channel. +- Assert losslessness and budget bounds against the **INPUT and the declared constant**, never + against the packer's own output — otherwise a packing regression satisfies them by moving in step. +- After flipping a tripwire, confirm it passes **on its own assertion**, not merely that the suite is + green. + +## Strongest formulation found: channel equivalence + +The assertion that needed no calibrated threshold and is immune to future layout tuning: +**the same text costs the same number of pages whichever channel carries it** (`usageText` vs +`attachmentsNote`). Pre-fix this failed spectacularly (9 pages vs 2 at 16,000 chars). Pair it with a +per-channel losslessness check first, so "same page count" can't be satisfied by both channels +dropping equally. + +Related: tree-level assertions **cannot see this bug class at all** — the dropped text is present in +the pdfmake content tree while the reader receives a truncated PDF. Where the claim is about what the +reader receives, assert against rendered output (page count). + +## Gotcha: the grey meta run is never at a fixed run index + +`buildUsageTextRuns()` tokenizes prose into **one run per whitespace-delimited token**, so a cell's +`.text` for `'Kitchen work'` is `[{text:'Kitchen'},{text:' '},{text:'work'}]` and the meta run is at +`text[3]`, not `text[1]`. Locate it by its colour (`#6b7280` = `DEPOSIT_NOTE_TEXT_COLOR`), assert it +is **last within its cell**, and reconstruct prose from the runs before it — see `splitUsageCell()` +in both `overviewPdf.test.ts` and `realRender.test.ts`. Note the production code strips the leading +`'\n'` when the suffix *starts* a cell (its own continuation row), so keep both a raw and a stripped +accessor for faithful cross-row concatenation. + +## Environment gotcha: `grep` silently returns nothing on these test files + +After `npx prettier --write`, `grep` (even `grep -c ""`) on `overviewPdf.test.ts` and +`ReportContentEditor.test.tsx` returns **no output and no error** — binary-content detection on some +byte sequence. This produced a false "constant is unused" conclusion mid-task. Use +`awk '/pattern/{print NR": "$0}' file` instead whenever a grep result on a report-PDF test file looks +suspiciously empty. `npx eslint` is the reliable authority on unused imports. + +Related: [[test-infra-reference]], [[story-1929-round2-real-render-technique]], +[[story-1923-report-table-cleanup]], [[story-1898-report-table-refinements]] diff --git a/client/src/components/reports/ReportContentEditor.test.tsx b/client/src/components/reports/ReportContentEditor.test.tsx index 042af6fa2..59b6f1b15 100644 --- a/client/src/components/reports/ReportContentEditor.test.tsx +++ b/client/src/components/reports/ReportContentEditor.test.tsx @@ -40,6 +40,27 @@ * `label={content.labels.attachmentsNote}` directly to `EditableField`, which renders a real * `
{content.labels.vendor}{content.labels.invoiceNumber}{content.labels.date}{content.labels.status}{content.labels.invoiceAmount}{content.labels.allocatedAmount}{content.labels.usage}{content.labels.attachmentsNote}{content.labels.vendor}{content.labels.invoiceNumber}{content.labels.date}{content.labels.status}{content.labels.invoiceAmount}{content.labels.allocatedAmount}{content.labels.usage}
{row.vendor}{row.invoiceNumber}{row.dateText}{row.vendor}{row.invoiceNumber}{row.dateText} - {row.invoiceAmountText} - - {row.allocatedAmountValueText} - {row.allocatedMarkers} - {row.isRefund && ` ${row.refundNoteText}`} - {row.isDeposit && ( - - )} - - - onFieldChange(overrideKey.row(row.invoiceId).usageText, value) - } - isEdited={isFieldEdited(overrideKey.row(row.invoiceId).usageText)} - onReset={() => onFieldReset(overrideKey.row(row.invoiceId).usageText)} - /> - {row.areaText &&
{row.areaText}
} -
} + {show('invoiceAmount') && ( + + {row.invoiceAmountText} + + {row.allocatedAmountValueText} + {row.isRefund && ` ${row.refundNoteText}`} + {row.isDeposit && ( + + )} + {row.isSplit && ( + ({content.labels.splitNote}) + )} + {row.isDepositReduced && ( + + {' '} + ({content.labels.depositReducedNote}) + + )} + - onFieldChange(overrideKey.row(row.invoiceId).attachmentsNote, value) + onFieldChange(overrideKey.row(row.invoiceId).usageText, value) } - isEdited={isFieldEdited(overrideKey.row(row.invoiceId).attachmentsNote)} - onReset={() => onFieldReset(overrideKey.row(row.invoiceId).attachmentsNote)} + isEdited={isFieldEdited(overrideKey.row(row.invoiceId).usageText)} + onReset={() => onFieldReset(overrideKey.row(row.invoiceId).usageText)} /> + {(row.areaText || row.attachmentsNote) && ( +
+ {[row.areaText, row.attachmentsNote].filter(Boolean).join(' · ')} +
+ )}
`/`` from the + * desktop table AND its mobile-card row. Local state only — no persistence, no callback. * The fixture's `labels` values below are deliberately prefixed `REPORT_*_LABEL` — a differently- * shaped string from anything the chrome `t` mock would ever echo — so that any test asserting * header/caption/source-info/mobile-card-editable-label text is a genuine regression guard: if the @@ -581,12 +602,19 @@ describe('ReportContentEditor — full field-wiring matrix (onChange + onReset p expect(onFieldReset).toHaveBeenCalledWith(key); }); - it('calls onFieldChange for the row-level attachmentsNote field', () => { + it('#1959: attachmentsNote is READ-ONLY inline text — it renders, but no form control carries it and no onFieldChange fires for it', () => { const { onFieldChange, container } = renderEditor({ content: fullContent() }); const table = getDesktopTable(container); - const field = within(table).getByDisplayValue('Note baseline'); - fireEvent.change(field, { target: { value: 'edited note' } }); - expect(onFieldChange).toHaveBeenCalledWith('row.inv-1.attachmentsNote', 'edited note'); + // Positive: the note text IS on screen (so the negatives below are looking at the right tree). + expect(within(table).getByText('Note baseline')).toBeInTheDocument(); + // Negative: it is not the value of any input/textarea — there is nothing to type into. + expect(within(table).queryByDisplayValue('Note baseline')).not.toBeInTheDocument(); + // Editing every remaining editable field in the row never produces an attachmentsNote key. + for (const input of within(table).getAllByRole('textbox')) { + fireEvent.change(input, { target: { value: 'anything' } }); + } + expect(onFieldChange).toHaveBeenCalled(); // the usage field did fire — the loop was not empty + expect(onFieldChange).not.toHaveBeenCalledWith('row.inv-1.attachmentsNote', expect.anything()); }); it('calls onFieldReset for the row-level usageText field', () => { @@ -606,12 +634,14 @@ describe('ReportContentEditor — full field-wiring matrix (onChange + onReset p }); describe('ReportContentEditor — reset button accessible names (no raw field-identifier leakage)', () => { - it('composes the row-level usage/attachmentsNote reset button names from a translated field name, never the raw "usage"/"attachmentsNote" identifier', () => { + it('composes the row-level usage reset button name from a translated field name, never the raw "usage" identifier — and #1959 leaves no attachmentsNote reset button at all', () => { const content = fullContent(); const { container } = renderEditor({ content, overrides: { 'row.inv-1.usageText': content.rows[0]!.usageText, + // Still supplied: even with an attachmentsNote override present in the map, the component + // must not grow a reset affordance for a field it no longer lets you edit. 'row.inv-1.attachmentsNote': content.rows[0]!.attachmentsNote as string, }, }); @@ -622,11 +652,14 @@ describe('ReportContentEditor — reset button accessible names (no raw field-id name: 'sourceReports.editable.resetFieldAriaLabel::{"field":"sourceReports.table.usage"}', }), ).toBeInTheDocument(); + + // #1959: usage is now the ONLY editable row field, so exactly one reset button per row. + expect(within(table).getAllByRole('button', { name: /resetFieldAriaLabel/ })).toHaveLength(1); expect( - within(table).getByRole('button', { + within(table).queryByRole('button', { name: 'sourceReports.editable.resetFieldAriaLabel::{"field":"sourceReports.editable.attachmentsNoteLabel"}', }), - ).toBeInTheDocument(); + ).not.toBeInTheDocument(); // Neither raw, untranslated identifier ever stands alone as a button's accessible name. expect(within(table).queryByRole('button', { name: 'usage' })).not.toBeInTheDocument(); @@ -794,20 +827,50 @@ describe('ReportContentEditor — table rows', () => { expect(badge.className).toContain(styles.statusPaid); }); - it('renders an Attachments Note column (labeled from content.labels.attachmentsNote) and EditableField only for rows with a non-null note', () => { + it('#1959: renders attachmentsNote as grey inline .usageMetaText INSIDE the usage cell — no Attachments Note column header, and only for rows that have a note', () => { const rows = [ - makeRow({ invoiceId: 'inv-1', attachmentsNote: '1 attachment: Invoice' }), - makeRow({ invoiceId: 'inv-2', attachmentsNote: null }), + // Distinct usageText per row so the cell lookup below is unambiguous. + makeRow({ + invoiceId: 'inv-1', + usageText: 'Noted usage', + attachmentsNote: '1 attachment: Invoice', + }), + makeRow({ invoiceId: 'inv-2', usageText: 'Plain usage', attachmentsNote: null }), ]; const { container } = renderEditor({ content: makeContent({ rows }) }); const table = getDesktopTable(container); - expect(within(table).getByText(LABELS.attachmentsNote)).toBeInTheDocument(); - expect(within(table).getByDisplayValue('1 attachment: Invoice')).toBeInTheDocument(); + + // Positive: the note text renders, as a .usageMetaText element, in the SAME as inv-1's + // usage input — proving it moved inline rather than merely disappearing. + const noteEl = within(table).getByText('1 attachment: Invoice'); + expect(noteEl.className).toContain(styles.usageMetaText); + const usageCell = within(table).getByDisplayValue('Noted usage').closest('td')!; + expect(usageCell).toContainElement(noteEl); + // inv-2's own usage cell does NOT get the note. + expect(within(table).getByDisplayValue('Plain usage').closest('td')!).not.toContainElement( + noteEl, + ); + + // Negative (now safe — we just proved we're looking at the populated tree): the dedicated + // column's header label is gone, and the note is not an editable value. + expect(within(table).queryByText(LABELS.attachmentsNote)).not.toBeInTheDocument(); + expect(within(table).queryByDisplayValue('1 attachment: Invoice')).not.toBeInTheDocument(); + + // inv-2 (no note) contributes no .usageMetaText element — exactly one exists in the table. + expect(table.querySelectorAll(`.${styles.usageMetaText}`)).toHaveLength(1); }); - it('omits the Attachments Note column entirely when no row has a non-null note', () => { - const rows = [makeRow({ attachmentsNote: null })]; - renderEditor({ content: makeContent({ rows }) }); + it('#1959: renders no .usageMetaText element at all when a row has neither attachmentsNote nor areaText', () => { + const withMeta = [makeRow({ attachmentsNote: '1 attachment: Invoice' })]; + const { container: populated } = renderEditor({ content: makeContent({ rows: withMeta }) }); + // Control: the selector DOES match when meta is present, so the 0-length assertion below is + // testing absence of content rather than a typo'd class name. + expect(populated.querySelectorAll(`.${styles.usageMetaText}`).length).toBeGreaterThan(0); + + const { container } = renderEditor({ + content: makeContent({ rows: [makeRow({ attachmentsNote: null, areaText: null })] }), + }); + expect(container.querySelectorAll(`.${styles.usageMetaText}`)).toHaveLength(0); expect(screen.queryByText(LABELS.attachmentsNote)).not.toBeInTheDocument(); }); @@ -820,31 +883,36 @@ describe('ReportContentEditor — table rows', () => { expect(onFieldChange).toHaveBeenCalledWith('row.inv-42.usageText', 'Changed'); }); - it('calls onFieldReset with the correct row..attachmentsNote key on reset (desktop table)', () => { - const rows = [makeRow({ invoiceId: 'inv-42', attachmentsNote: 'Edited note' })]; - const { onFieldReset, container } = renderEditor({ - content: makeContent({ rows }), - overrides: { 'row.inv-42.attachmentsNote': 'Edited note' }, - }); - const table = getDesktopTable(container); - const resetButtons = within(table).getAllByRole('button', { name: /resetFieldAriaLabel/ }); - fireEvent.click(resetButtons[resetButtons.length - 1]!); - expect(onFieldReset).toHaveBeenCalledWith('row.inv-42.attachmentsNote'); - }); - - it('calls onFieldReset with the correct row..attachmentsNote key on reset (mobile card)', () => { - const rows = [makeRow({ invoiceId: 'inv-42', attachmentsNote: 'Edited note' })]; - const { onFieldReset, container } = renderEditor({ - content: makeContent({ rows }), - overrides: { 'row.inv-42.attachmentsNote': 'Edited note' }, - }); - const mobileList = getMobileList(container); - const resetButtons = within(mobileList).getAllByRole('button', { - name: /resetFieldAriaLabel/, - }); - fireEvent.click(resetButtons[resetButtons.length - 1]!); - expect(onFieldReset).toHaveBeenCalledWith('row.inv-42.attachmentsNote'); - }); + it.each([ + ['desktop table', getDesktopTable], + ['mobile card', getMobileList], + ])( + '#1959: clicking every reset button in a row resets ONLY row..usageText — attachmentsNote has no reset control (%s)', + (_label, getTree) => { + const rows = [ + makeRow({ invoiceId: 'inv-42', usageText: 'Edited usage', attachmentsNote: 'Some note' }), + ]; + const { onFieldReset, container } = renderEditor({ + content: makeContent({ rows }), + // Both keys overridden, so a lingering attachmentsNote affordance would render its reset + // button and be caught below. + overrides: { + 'row.inv-42.usageText': 'Edited usage', + 'row.inv-42.attachmentsNote': 'Some note', + }, + }); + const tree = within(getTree(container)); + // Positive: the note text is present in this tree (read-only), so the negative below is not + // passing merely because we picked an empty subtree. + expect(tree.getByText('Some note')).toBeInTheDocument(); + + const resetButtons = tree.getAllByRole('button', { name: /resetFieldAriaLabel/ }); + expect(resetButtons).toHaveLength(1); + for (const btn of resetButtons) fireEvent.click(btn); + expect(onFieldReset).toHaveBeenCalledWith('row.inv-42.usageText'); + expect(onFieldReset).not.toHaveBeenCalledWith('row.inv-42.attachmentsNote'); + }, + ); }); describe('ReportContentEditor — summary rows and footnotes', () => { @@ -939,8 +1007,238 @@ describe('ReportContentEditor — isDeposit (AC2.1: inline Deposit badge, no mar }); }); -describe('ReportContentEditor — areaText (AC5.2/5.3: distinct element below the usage field)', () => { - it('renders areaText as a distinct element below the desktop Usage EditableField, not inside its value', () => { +describe('ReportContentEditor — #1959 isSplit / isDepositReduced inline labels (replacing the † / ‡ footnote markers)', () => { + // The allocated cell is a composite of value text + optional badge + optional inline notes, so + // assert on the CELL's whole textContent — that is what a user reads — rather than on a single + // text node, which would miss ordering/spacing regressions between the runs. + function allocatedCellText(container: HTMLElement): string { + const table = getDesktopTable(container); + // The allocated cell is the only right-aligned holding the €400.00 value; match on the + // itself so composite children (badge + inline note spans) are included in textContent. + return within(table).getByText(/^€400\.00/, { selector: 'td' }).textContent!; + } + + it('appends an inline (splitNote) label to the desktop allocated cell when isSplit, and no † marker', () => { + const rows = [ + makeRow({ invoiceId: 'inv-1', isSplit: true, allocatedAmountValueText: '€400.00' }), + ]; + const { container } = renderEditor({ content: makeContent({ rows }) }); + expect(allocatedCellText(container)).toBe('€400.00 (REPORT_SPLIT_NOTE_LABEL)'); + // The label is styled as a grey inline note, not a plain text run. + const table = getDesktopTable(container); + const noteEl = within(table).getByText('(REPORT_SPLIT_NOTE_LABEL)'); + expect(noteEl.className).toContain(styles.inlineNote); + expect(within(table).queryByText(/†/)).not.toBeInTheDocument(); + }); + + it('appends an inline (depositReducedNote) label to the desktop allocated cell when isDepositReduced, and no ‡ marker', () => { + const rows = [ + makeRow({ + invoiceId: 'inv-1', + isDepositReduced: true, + allocatedAmountValueText: '€400.00', + }), + ]; + const { container } = renderEditor({ content: makeContent({ rows }) }); + expect(allocatedCellText(container)).toBe('€400.00 (REPORT_DEPOSIT_REDUCED_NOTE_LABEL)'); + const table = getDesktopTable(container); + expect(within(table).getByText('(REPORT_DEPOSIT_REDUCED_NOTE_LABEL)').className).toContain( + styles.inlineNote, + ); + expect(within(table).queryByText(/‡/)).not.toBeInTheDocument(); + }); + + it('renders BOTH inline labels, split before deposit-reduced, when both flags are set', () => { + const rows = [ + makeRow({ + invoiceId: 'inv-1', + isSplit: true, + isDepositReduced: true, + allocatedAmountValueText: '€400.00', + }), + ]; + const { container } = renderEditor({ content: makeContent({ rows }) }); + expect(allocatedCellText(container)).toBe( + '€400.00 (REPORT_SPLIT_NOTE_LABEL) (REPORT_DEPOSIT_REDUCED_NOTE_LABEL)', + ); + }); + + it('renders the same inline labels in the mobile card allocated row', () => { + const rows = [ + makeRow({ + invoiceId: 'inv-1', + isSplit: true, + isDepositReduced: true, + allocatedAmountValueText: '€400.00', + }), + ]; + const { container } = renderEditor({ content: makeContent({ rows }) }); + const card = within(getMobileList(container)); + expect(card.getByText('(REPORT_SPLIT_NOTE_LABEL)').className).toContain(styles.inlineNote); + expect(card.getByText('(REPORT_DEPOSIT_REDUCED_NOTE_LABEL)').className).toContain( + styles.inlineNote, + ); + }); + + it('renders neither inline label when both flags are false', () => { + const rows = [ + makeRow({ + invoiceId: 'inv-1', + isSplit: false, + isDepositReduced: false, + allocatedAmountValueText: '€400.00', + }), + ]; + const { container } = renderEditor({ content: makeContent({ rows }) }); + // Positive anchor: the allocated cell rendered its value, so the absences are meaningful. + expect(allocatedCellText(container)).toBe('€400.00'); + expect(screen.queryByText('(REPORT_SPLIT_NOTE_LABEL)')).not.toBeInTheDocument(); + expect(screen.queryByText('(REPORT_DEPOSIT_REDUCED_NOTE_LABEL)')).not.toBeInTheDocument(); + expect(container.querySelectorAll(`.${styles.inlineNote}`)).toHaveLength(0); + }); +}); + +describe('ReportContentEditor — #1959 column visibility toggles (local state, no persistence)', () => { + function getToggleGroup(container: HTMLElement): HTMLElement { + return within(container).getByRole('group', { + name: 'sourceReports.editable.columnVisibilityLabel', + }); + } + + it('renders one checked checkbox per column, labeled from content.labels.*, with a Status toggle only for overview reports', () => { + const { container } = renderEditor({ content: makeContent({ isOverview: false }) }); + const group = within(getToggleGroup(container)); + const boxes = group.getAllByRole('checkbox'); + expect(boxes).toHaveLength(6); // vendor, invoiceNumber, date, invoiceAmount, allocatedAmount, usage + for (const box of boxes) expect(box).toBeChecked(); + // Labels come from content.labels.* (report language), never a chrome t() echo. + for (const label of [ + LABELS.vendor, + LABELS.invoiceNumber, + LABELS.date, + LABELS.invoiceAmount, + LABELS.allocatedAmount, + LABELS.usage, + ]) { + expect(group.getByLabelText(label)).toBeChecked(); + } + expect(group.queryByLabelText(LABELS.status)).not.toBeInTheDocument(); + }); + + it('adds a Status toggle when isOverview is true', () => { + const { container } = renderEditor({ content: makeContent({ isOverview: true }) }); + const group = within(getToggleGroup(container)); + expect(group.getAllByRole('checkbox')).toHaveLength(7); + expect(group.getByLabelText(LABELS.status)).toBeChecked(); + }); + + // Every column gets a DISTINCT value so a disappearing cell can be attributed to the toggled + // column and not shadowed by an identical string elsewhere in the table (the fixture's default + // invoice/allocated/summary amounts are all €100.00). + function distinctValueContent(): ReportContent { + return makeContent({ + rows: [ + makeRow({ + invoiceId: 'inv-1', + vendor: 'ACME', + invoiceNumber: 'INV-001', + dateText: '01/10/2026', + invoiceAmountText: '€111.00', + allocatedAmountValueText: '€222.00', + usageText: 'Kitchen work', + }), + ], + summaryRows: [{ key: 'total', label: 'Total', amountText: '€333.00' }], + }); + } + + it.each([ + [LABELS.vendor, 'ACME'], + [LABELS.invoiceNumber, 'INV-001'], + [LABELS.date, '01/10/2026'], + [LABELS.invoiceAmount, '€111.00'], + [LABELS.allocatedAmount, '€222.00'], + ])( + 'unchecking the "%s" toggle removes that column header AND its cell value from the desktop table and the mobile card', + (label, cellValue) => { + const { container } = renderEditor({ content: distinctValueContent() }); + const table = getDesktopTable(container); + const mobileList = getMobileList(container); + const headerCountBefore = table.querySelectorAll('thead th').length; + + // Positive: header label and cell value are both present before the toggle. + expect(within(table).getByText(label, { selector: 'th' })).toBeInTheDocument(); + expect(within(table).getByText(cellValue)).toBeInTheDocument(); + expect(within(mobileList).getByText(cellValue)).toBeInTheDocument(); + + fireEvent.click(within(getToggleGroup(container)).getByLabelText(label)); + + // ...and both are gone afterwards, in BOTH responsive trees. + expect(within(table).queryByText(label, { selector: 'th' })).not.toBeInTheDocument(); + expect(within(table).queryByText(cellValue)).not.toBeInTheDocument(); + expect(within(mobileList).queryByText(cellValue)).not.toBeInTheDocument(); + expect(table.querySelectorAll('thead th')).toHaveLength(headerCountBefore - 1); + + // The toggle itself stays visible (so the column can be restored) and reflects hidden state. + const box = within(getToggleGroup(container)).getByLabelText(label); + expect(box).not.toBeChecked(); + // Re-checking restores the column — the state is a real toggle, not a one-way hide. + fireEvent.click(box); + expect(within(table).getByText(label, { selector: 'th' })).toBeInTheDocument(); + expect(within(table).getByText(cellValue)).toBeInTheDocument(); + }, + ); + + it('unchecking the Usage toggle removes the usage EditableField (and its inline meta text) entirely', () => { + const rows = [ + makeRow({ invoiceId: 'inv-1', usageText: 'Kitchen work', areaText: 'Ground Floor' }), + ]; + const { container } = renderEditor({ content: makeContent({ rows }) }); + expect(screen.getAllByDisplayValue('Kitchen work').length).toBeGreaterThan(0); + expect(container.querySelectorAll(`.${styles.usageMetaText}`).length).toBeGreaterThan(0); + + fireEvent.click(within(getToggleGroup(container)).getByLabelText(LABELS.usage)); + + expect(screen.queryAllByDisplayValue('Kitchen work')).toHaveLength(0); + expect(container.querySelectorAll(`.${styles.usageMetaText}`)).toHaveLength(0); + }); + + it('unchecking the Status toggle removes the status Badge from an overview report', () => { + const rows = [makeRow({ invoiceId: 'inv-1', status: 'paid', statusText: 'REPORT_PAID_TEXT' })]; + const { container } = renderEditor({ content: makeContent({ isOverview: true, rows }) }); + expect(screen.getAllByText('REPORT_PAID_TEXT').length).toBeGreaterThan(0); + + fireEvent.click(within(getToggleGroup(container)).getByLabelText(LABELS.status)); + + expect(screen.queryAllByText('REPORT_PAID_TEXT')).toHaveLength(0); + expect( + within(getDesktopTable(container)).queryByText(LABELS.status, { selector: 'th' }), + ).not.toBeInTheDocument(); + }); + + it('hides only the toggled column, leaving the others rendered (toggles are independent)', () => { + const { container } = renderEditor(); + fireEvent.click(within(getToggleGroup(container)).getByLabelText(LABELS.vendor)); + const table = getDesktopTable(container); + expect(within(table).queryByText('ACME')).not.toBeInTheDocument(); + // Every other column's value survives. + expect(within(table).getByText('INV-001')).toBeInTheDocument(); + expect(within(table).getByText('01/10/2026')).toBeInTheDocument(); + expect(within(table).getByDisplayValue('Baseline usage')).toBeInTheDocument(); + }); + + it('does not invoke onFieldChange/onFieldReset when a column is toggled (visibility is local state, never an override)', () => { + const { container, onFieldChange, onFieldReset } = renderEditor(); + for (const box of within(getToggleGroup(container)).getAllByRole('checkbox')) { + fireEvent.click(box); + } + expect(onFieldChange).not.toHaveBeenCalled(); + expect(onFieldReset).not.toHaveBeenCalled(); + }); +}); + +describe('ReportContentEditor — areaText / attachmentsNote inline meta (#1959: one grey element inside the usage cell)', () => { + it('renders areaText as a distinct .usageMetaText
inside the desktop usage cell, not inside the editable value', () => { const rows = [ makeRow({ invoiceId: 'inv-1', usageText: 'Kitchen work', areaText: 'Ground Floor' }), ]; @@ -948,27 +1246,77 @@ describe('ReportContentEditor — areaText (AC5.2/5.3: distinct element below th const table = getDesktopTable(container); const usageInput = within(table).getByDisplayValue('Kitchen work'); // The area text is not baked into the editable input's value. - expect(usageInput).not.toHaveValue('Kitchen work / Ground Floor'); + expect(usageInput).toHaveValue('Kitchen work'); const areaEl = within(table).getByText('Ground Floor'); - expect(areaEl.className).toContain(styles.usageAreaText); + expect(areaEl.className).toContain(styles.usageMetaText); expect(areaEl.tagName).toBe('DIV'); + // ...and it lives in the same cell as the usage field (inline, not a separate column). + expect(usageInput.closest('td')).toContainElement(areaEl); }); - it('renders areaText as a in the mobile card usage row', () => { + it('renders areaText as a .usageMetaText in the mobile card usage row', () => { const rows = [ makeRow({ invoiceId: 'inv-1', usageText: 'Kitchen work', areaText: 'Ground Floor' }), ]; const { container } = renderEditor({ content: makeContent({ rows }) }); const card = within(getMobileList(container)); const areaEl = card.getByText('Ground Floor'); - expect(areaEl.className).toContain(styles.usageAreaText); + expect(areaEl.className).toContain(styles.usageMetaText); expect(areaEl.tagName).toBe('SPAN'); }); - it('renders no area element (desktop or mobile) when areaText is null', () => { - const rows = [makeRow({ invoiceId: 'inv-1', areaText: null })]; + it.each([ + ['desktop table', getDesktopTable], + ['mobile card', getMobileList], + ])( + '#1959: joins areaText and attachmentsNote with " · " into ONE meta element, area first (%s)', + (_label, getTree) => { + const rows = [ + makeRow({ + invoiceId: 'inv-1', + usageText: 'Kitchen work', + areaText: 'Ground Floor', + attachmentsNote: '1 attachment: Invoice', + }), + ]; + const { container } = renderEditor({ content: makeContent({ rows }) }); + const tree = getTree(container); + // Exact single-node text match pins the join order AND the separator: two sibling elements, + // a different separator, or a swapped order would all fail this. + const metaEl = within(tree).getByText('Ground Floor · 1 attachment: Invoice'); + expect(metaEl.className).toContain(styles.usageMetaText); + expect(tree.querySelectorAll(`.${styles.usageMetaText}`)).toHaveLength(1); + }, + ); + + it('renders areaText alone with no dangling separator when attachmentsNote is null', () => { + const rows = [ + makeRow({ + invoiceId: 'inv-1', + usageText: 'Kitchen work', + areaText: 'Ground Floor', + attachmentsNote: null, + }), + ]; const { container } = renderEditor({ content: makeContent({ rows }) }); - expect(container.querySelectorAll(`.${styles.usageAreaText}`)).toHaveLength(0); + const metaEls = container.querySelectorAll(`.${styles.usageMetaText}`); + expect(metaEls).toHaveLength(2); // one desktop
, one mobile + for (const el of metaEls) { + expect(el.textContent).toBe('Ground Floor'); + } + }); + + it('renders no meta element (desktop or mobile) when areaText and attachmentsNote are both null', () => { + // Control: prove the selector matches when meta IS present, so the 0-length assertion below + // cannot pass on a stale/renamed class name. + const { container: populated } = renderEditor({ + content: makeContent({ rows: [makeRow({ areaText: 'Ground Floor' })] }), + }); + expect(populated.querySelectorAll(`.${styles.usageMetaText}`).length).toBeGreaterThan(0); + + const rows = [makeRow({ invoiceId: 'inv-1', areaText: null, attachmentsNote: null })]; + const { container } = renderEditor({ content: makeContent({ rows }) }); + expect(container.querySelectorAll(`.${styles.usageMetaText}`)).toHaveLength(0); }); }); @@ -1018,7 +1366,7 @@ describe( expect(card.queryByText('sourceReports.table.usage')).not.toBeInTheDocument(); }); - it('renders the mobile card Status label (from content.labels.status) and Badge (labeled from row.statusText) and Attachments Note field (labeled from content.labels.attachmentsNote via a real htmlFor association) consistently with the desktop table', () => { + it('renders the mobile card Status label (from content.labels.status) and Badge (labeled from row.statusText), and #1959 renders attachmentsNote as read-only inline meta text with no labeled field of its own', () => { const rows = [ makeRow({ invoiceId: 'inv-1', @@ -1031,9 +1379,14 @@ describe( const card = within(getMobileList(container)); expect(card.getByText(LABELS.status)).toBeInTheDocument(); expect(card.getByText('REPORT_PAID_TEXT')).toBeInTheDocument(); - const noteField = card.getByLabelText(LABELS.attachmentsNote); - expect(noteField).toHaveValue('1 attachment: Invoice'); - expect(noteField.tagName).toBe('INPUT'); + + // Positive: the note text renders inline, as grey meta text. + const noteEl = card.getByText('1 attachment: Invoice'); + expect(noteEl.className).toContain(styles.usageMetaText); + expect(noteEl.tagName).toBe('SPAN'); + // Negative: no