diff --git a/.claude/agent-memory/qa-integration-tester/MEMORY.md b/.claude/agent-memory/qa-integration-tester/MEMORY.md index ae5b48877..45832ab54 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) +- [Story #1923 — report table cleanup](story-1923-report-table-cleanup.md) (2026-08-02) — unnumbered shared †/‡ markers, isDeposit/isClaim/areaText fixture ripple across 8 report test files, overviewPdf allocated-cell-is-always-an-array-of-runs gotcha, worktree `node_modules/@cornerstone/shared` symlink pointing at a differently-branched base repo (false-positive `tsc` errors; trust jest). - [Bugs #1895/#1896/#1918 — claim/deposit scope fixes](bugs-1895-1896-1918-claim-deposit-scope.md) (2026-08-01) — `markInvoicesClaimed` gained `sourceId`+required `depositIds` params (cross-source claim guard, decoupled sweep, quotation+sweepable-deposit no longer 409s); `getSourceReport` drops zero-portion `budgetLines[]` on `claim` reports only; text-content query collision gotcha (banner text contains data also shown elsewhere on page). ## Recent bug/story notes (2026-07) diff --git a/.claude/agent-memory/qa-integration-tester/story-1923-report-table-cleanup.md b/.claude/agent-memory/qa-integration-tester/story-1923-report-table-cleanup.md new file mode 100644 index 000000000..85d3e1269 --- /dev/null +++ b/.claude/agent-memory/qa-integration-tester/story-1923-report-table-cleanup.md @@ -0,0 +1,106 @@ +--- +name: story-1923-report-table-cleanup +description: QA test coverage for issue #1923 (shared footnotes, inline deposit labels, claim metadata, total-only summary, area in Usage) — unnumbered marker semantics, isDeposit/isClaim/areaText fixture ripple across 8 report test files, and a worktree symlink false-positive gotcha. +metadata: + type: project +--- + +Story/issue #1923 (branch `feat/1923-report-table-cleanup`, worktree `develop-claim-semantics`). +Production code (buildReportContent.ts, ReportContentEditor.tsx, overviewPdf.ts, +sourceReportService.ts, shared/src/types/sourceReport.ts) was already implemented when QA started — +only test files needed writing/updating. All 8 touched files pass (302 tests total), 100% +statement/line coverage on buildReportContent.ts and sourceReportService.ts, 100% on +ReportContentEditor.tsx, ~100%/98% branch on overviewPdf.ts. + +**New/changed behavior to remember for future report-content work:** +- Split (`†`) and reduced-deposit (`‡`) markers are now UNNUMBERED and SHARED — at most one + footnote entry each (`id: 'split'`/`'deposit-reduced'`, no vendor/invoice prefix). A constituted + (tagged-to-this-source) deposit produces `isDeposit: true` on the row instead of a `‡` marker or + footnote — no footnote entry at all for that case. +- `summaryRows` is always exactly 1 entry (`key: 'total'`) — no more per-status subtotal rows. +- `ReportContent` gained `isClaim: boolean` (gates the source-info block/stack in both the editor + and the PDF). `ReportContentRow` gained `isDeposit: boolean` and `areaText: string | null`. +- **overviewPdf.ts's allocated-amount cell `.text` is now ALWAYS an array of runs** (`allocatedRuns`), + even with zero deposit/refund — not conditionally a plain string. Any test using a `rowTexts()`- + style helper that does `cell.text` directly must handle both string and array-of-`{text}` shapes + (join array runs) or every existing "plain amount" assertion silently breaks (`toBe('€400.00')` + vs received `[{text:'€400.00'}]`). Fixed by making the shared `rowTexts()` helper flatten arrays. +- `SourceReportLinkedItem` gained `areaId`/`areaName` (nullable), resolved server-side via + `LEFT JOIN areas` on `work_items.area_id`/`household_items.area_id` — leaf-only area name, no + parent-path expansion (verified with an explicit child-area-with-parent test). +- `linkedItem` stays `null` when the join partially resolves but the item's own title/name is falsy + (empty string) — a real, defensible edge case reachable via `row.work_item_id && row.work_item_title` + guard in the service; used this to cover "linkedItem null unaffected" rather than trying to force + an FK-violating null-linkedItem state (not practically reachable given the schema's constraints). + +**Worktree gotcha reconfirmed and clarified**: raw `npx tsc --noEmit -p client/tsconfig.json` (or +server/tsconfig.json) in this worktree resolves `@cornerstone/shared` via `node_modules/@cornerstone/shared`, +which is a symlink to the **base repo checkout** (`/Users/.../cornerstone/shared`), not the worktree's +own `shared/`. If the base repo happens to be checked out on a *different, unrelated branch* (observed: +base was on `fix/1895-1918-claim-deposit-scope` while this worktree was on `feat/1923-...`), that stale +symlinked `dist/index.d.ts` produces convincing but FALSE-POSITIVE type errors (e.g. "Property 'areaId' +does not exist on type 'SourceReportLinkedItem'") for types the current branch's `shared/src` genuinely +already has. **Jest itself did not reproduce these errors and all tests passed with correct runtime +values** for the files I'd already fixed — client project's jest config has `moduleNameMapper: +'^@cornerstone/shared$' -> '/shared/src/index.ts'` (worktree source, always fresh); the +**server** project has no such mapper, yet still passed cleanly too (mechanism unclear). + +**Definitive fix applied this round** (repoint the worktree's own node_modules symlink instead of +relying on jest's leniency, so raw `tsc -p` becomes trustworthy again for this session): +```bash +rm node_modules/@cornerstone/shared +ln -s /absolute/path/to/THIS/worktree/shared node_modules/@cornerstone/shared +cd shared && npx tsc && cd .. # rebuild worktree-local shared/dist +``` +After this, `npx tsc --noEmit -p client/tsconfig.json` and `-p server/tsconfig.json` both went from +~15-30 false-positive errors to 0, and stayed 0 after all real fixes. **Use this fix proactively at the +start of any session that needs a real `tsc -p` sanity sweep** (e.g. when a coordinator reports a CI +typecheck failure) rather than trusting jest's silence alone — jest's leniency masks real errors too +easily to be the sole signal when hunting for "any other fixture drift somewhere in the tree" (this is +exactly how the ReportInvoiceList.test.tsx / realRender.test.ts CI failures escaped my first local pass). +Do NOT touch the base checkout itself — this only repoints the worktree's own node_modules entry. + +**Real bug this fix uncovered** (CI Quality Gates failure on PR #1924, reported by coordinator): +`ReportInvoiceList.test.tsx` (~L727/733) and `realRender.test.ts` (~L447/453) built `SourceReportLinkedItem` +literals missing `areaId`/`areaName` — straightforward fixture-drift fixes (add the two null fields). +But `realRender.test.ts` also had a **second, deeper bug**: a real-i18n end-to-end test +(`renders both real deposit-footnote wordings ("constituted" vs "reduced")...`) still asserted the +OLD numbered/vendor-prefixed constituted-deposit footnote text (`'‡1: Constituted Vendor (U-5) — This +is a deposit.'`), which the AC2.1/AC2.2 change removed entirely (constituted deposits now render as an +inline `isDeposit` badge/run, not a footnote). Rewrote the test to assert real translated text for both: +the inline deposit-label run (array-of-runs allocated cell, second run text ` (Deposit)`/` +(Abschlagszahlung)`) AND the still-existing shared/unnumbered reduced-deposit footnote — plus explicit +negative assertions that the old "This is a deposit."/"Dies ist eine Abschlagszahlung." footnote text +no longer appears anywhere. Lesson: a `grep`-based sweep for the type-shape drift (missing fields) is +necessary but not sufficient — real-render/integration tests asserting exact translated STRINGS for a +feature whose wording changed need their own pass, since `tsc` won't catch stale string assertions. + +**Round 3** (coordinator follow-up on PR #1924): the Deposit badge label moved further — out of +per-consumer `t()` calls entirely and into the shared content model as `ReportContentLabels.deposit` +(built once in `buildReportContent.ts` via `reportT`; `ReportContentEditor.tsx` and `overviewPdf.ts` +both just read `content.labels.deposit` / `reportContent.labels.deposit` now — `overviewPdf.ts` also +switched to named color/fontSize constants `DEPOSIT_NOTE_TEXT_COLOR`/`DEPOSIT_NOTE_FONT_SIZE` from +`reportPdf/shared.ts` instead of inline magic values). By the time this request landed, an external +process (not me) had already patched most existing fixtures' `labels`/`makeLabels()` objects to include +`deposit: 'REPORT_DEPOSIT_LABEL'`-style values — but NOT the 4 files I'd fixed in round 2 +(`applyAiContent.test.ts`, `applyOverrides.test.ts`, `coverLetterPdf.test.ts`, `merge.test.ts`), which +still lacked the `deposit` key on their `ReportContentLabels` fixtures and failed `tsc` (`TS2741: +Property 'deposit' is missing`) once I re-swept. **Lesson: when a shared type gains a new required +field mid-story, re-run the `tsc -p client/tsconfig.json` sweep after EVERY round, even on files you +"already fixed" in a prior round for a different reason** — the type can grow again between rounds +without any signal other than a fresh typecheck. + +For the mixed-language regression itself: added to `realRender.test.ts`'s existing +`describe('production i18n singleton — getFixedT resolves a language independent of the ambient one')` +block (established pattern for "UI locale stays X while report language resolves Y" — uses the REAL +app i18n singleton via `(await import('../../i18n/index.js')).default` + `i18n.getFixedT(lang, 'budget')`, +not the file's separate isolated `i18next.createInstance()` used everywhere else in that file). Gotcha: +helper functions declared with `function` inside a nested `describe(...)` callback (e.g. +`makeUsageFeatureReport()` inside the `'Usage column...'` block) are scoped to that closure only — +NOT visible from a sibling top-level `describe` block later in the same file. Had to inline a minimal +one-invoice fixture using the file's top-level `makeInvoice()` helper instead of reaching into the +nested one. Final test: builds `content` via `reportT = i18n.getFixedT('de', 'budget')` while asserting +`i18n.language` stays `'en'` throughout (both before and after — proves `getFixedT` never calls +`changeLanguage()`), asserts `content.labels.deposit === 'Abschlagszahlung'` (exact real string, not +`.toContain`), contrasts with `getFixedT('en', ...)` → `'Deposit'`, and pins the same value through to +the rendered PDF's inline deposit run. diff --git a/.claude/agent-memory/ux-designer/feature-spec-history.md b/.claude/agent-memory/ux-designer/feature-spec-history.md index cf4c0423c..8fb2e3d1e 100644 --- a/.claude/agent-memory/ux-designer/feature-spec-history.md +++ b/.claude/agent-memory/ux-designer/feature-spec-history.md @@ -28,6 +28,16 @@ Adds an opt-in "Enable AI assistance" toggle (Step 4) + "Generate with AI" batch - **Table/mobile-card breakpoint**: reused `ReportInvoiceList`'s existing `max-width: 767px` split verbatim rather than the page's own ad hoc `860px` breakpoint (`.step4Layout` collapse) — the two breakpoints coexist in this file for different purposes (860px = two-column layout collapse, 767px = table→cards), don't conflate them. - Full field inventory for the cover letter (from `coverLetterPdf.ts`): sender (household name+address), recipient (`source.contactAddress`), reference (`source.reference`, optional), subject (per-use-case string), body (per-use-case template with `{{total}}`). A signature block also exists in the generated PDF (echoes household name a second time) but isn't in the issue's "settled decisions" list of 5 editable fields — spec'd it as derived-display-only (mirrors Sender), flagged as an open question rather than deciding unilaterally. +## Issue #1923 — Report table cleanup: shared footnotes, inline deposit labels, claim metadata, total-only summary, area in Usage + +Client-only content-model cleanup on `ReportContentEditor.tsx`/`overviewPdf.ts` (mostly de-numbering footnotes + moving 2 facts from footnote-only to inline). Spec posted covering all 5 ACs. + +- **"Deposit" inline label reuses `Badge`'s existing `.attachmentDeposit` variant** (`--color-attachment-deposit-bg`/`-text`, teal) rather than inventing a new badge color — the exact same wording (`sourceReports.table.attachmentType.deposit`) already exists as a Document-Type badge elsewhere in the same reports feature (`ReportInvoiceList.tsx`). When a new inline "this row is an X" label is needed, check `attachmentType`-style badges first — this app already has a full palette of document/entry-type pills before reaching for a new variant. +- **PDF "no Badge" fallback pattern**: pdfmake has no pill primitive, so a Badge-equivalent becomes a bracketed plain-text suffix `(Deposit)` in the *same* translation key, rendered as a separate lower-weight text run (`{ text: '...', color: '#6b7280', fontSize: 8 }`) via pdfmake's array-of-runs `text` field — not a new stacked line, not a filled/colored cell (per-run `fillColor` doesn't compose reliably with a whole-cell `text` array in pdfmake tables). `#6b7280` is the PDF-baked literal equivalent of `--color-text-muted`/`--color-gray-500` (confirmed via `merge.ts`'s hardcoded `styles.small.color`); PDF exports have no dark mode by design (fixed light-background documents), so hardcoded hex is expected/correct there, unlike in `client/src/**/*.css` where it would be a stylelint violation. +- **Conditional block removal — no placeholder, just omit from the render tree**: for "this metadata block doesn't apply to this report type" (claim reports skip `sourceInfoBlock`), the correct spec is `{condition &&
...}` (full omission), relying on the parent's existing `display:flex; gap: var(--spacing-N)` to naturally close the space — never a `display:none`-but-present placeholder or a manually tightened margin override. Same principle applies to the pdfmake side: skip the whole `content.push(...)` call rather than pushing an empty/near-empty stack. +- **Secondary/muted metadata line under an editable field**: reused the `.dateLineLabel`/`.footnotes` muted-xs-text convention (`--font-size-xs` + `--color-text-muted`) for the new "area name" sub-line under Usage — this is the established in-file precedent for "annotation, not content" text, not a new pattern. Placed *below* the `EditableField`, never inline/parenthetical beside it, specifically because AC required it be visually distinguishable as non-editable — inline-beside-an-input reads as part of the same string. +- **PDF stack-building refactor flagged, not just a style note**: extending `overviewPdf.ts`'s Usage cell from a 2-way ternary (`attachmentsNote ? stack : text`) to a 3-optional-line array build (usage + area + attachmentsNote) is a real code-shape change for `frontend-developer`, called out explicitly in the spec as an implementation note so it isn't missed as "just add one more line." + ## Issue #1876 — Deposit Refunds with Negative Claim Adjustments `InvoiceDepositsSection` gains an entry-type choice (Deposit/Refund); refunds render as negative rows reusing the exact same status Badge/labels (Pending/Paid/Claimed) — no relabeling, per explicit user decision. diff --git a/.claude/agent-memory/ux-designer/pr-review-findings.md b/.claude/agent-memory/ux-designer/pr-review-findings.md index e20b2d872..c081c4ee9 100644 --- a/.claude/agent-memory/ux-designer/pr-review-findings.md +++ b/.claude/agent-memory/ux-designer/pr-review-findings.md @@ -11,6 +11,10 @@ metadata: - Legend dot `8px` = `var(--spacing-2)` — always swap raw px dot sizes to nearest spacing token - `--color-border-strong` as text `color` for a separator — use `--color-text-muted` instead +## PR #1924 — Report table cleanup (#1923) (APPROVED via `gh pr comment`) + +Clean match to spec: `.depositBadge` composed `attachmentDeposit` verbatim, mobile Badge correctly omitted `.depositLabel` (relies on `.mobileCardAllocated` flex-gap instead), `.usageAreaText` rendered as separate div/span never concatenated into the editable value, summary border-removal + `.summaryAmount`-only size bump both matched exactly, PDF deposit-suffix run and usage-stack area line matched fontSize/color/ordering. No findings. Verdict was posted as a `gh pr comment` with an explicit "Verdict: APPROVED" line. + ## PR #1490 — Measurement & Freehand Tools (APPROVED/comment) See `pr-1490-measurement-freehand.md`. Medium: `labelAttrs { display:'none' }` dead code in render.ts — refinement item. diff --git a/client/src/components/reports/ReportContentEditor.module.css b/client/src/components/reports/ReportContentEditor.module.css index 6a588103e..0cf9daf84 100644 --- a/client/src/components/reports/ReportContentEditor.module.css +++ b/client/src/components/reports/ReportContentEditor.module.css @@ -154,6 +154,28 @@ color: var(--color-status-quotation-text); } +/* Deposit badge and area text */ +.depositBadge { + composes: attachmentDeposit from '../Badge/Badge.module.css'; +} + +.depositLabel { + margin-left: var(--spacing-2); +} + +.mobileCardAllocated { + display: flex; + align-items: center; + gap: var(--spacing-2); + flex-wrap: wrap; +} + +.usageAreaText { + font-size: var(--font-size-xs); + color: var(--color-text-muted); + margin-top: var(--spacing-1); +} + /* Summary Table */ .summaryTable { width: 100%; @@ -167,6 +189,10 @@ border-bottom: 1px solid var(--color-border); } +.summaryTable tbody tr:last-child td { + border-bottom: none; +} + .summaryLabel { font-weight: var(--font-weight-semibold); color: var(--color-text-primary); @@ -175,6 +201,7 @@ .summaryAmount { font-weight: var(--font-weight-semibold); color: var(--color-text-primary); + font-size: var(--font-size-base); } /* Footnotes */ diff --git a/client/src/components/reports/ReportContentEditor.test.tsx b/client/src/components/reports/ReportContentEditor.test.tsx index a226df23e..5e1f70615 100644 --- a/client/src/components/reports/ReportContentEditor.test.tsx +++ b/client/src/components/reports/ReportContentEditor.test.tsx @@ -78,6 +78,7 @@ const LABELS: ReportContentLabels = { allocatedAmount: 'REPORT_ALLOCATED_AMOUNT_LABEL', usage: 'REPORT_USAGE_LABEL', attachmentsNote: 'REPORT_ATTACHMENTS_NOTE_LABEL', + deposit: 'REPORT_DEPOSIT_LABEL', source: 'REPORT_SOURCE_LABEL', sourceType: 'REPORT_SOURCE_TYPE_LABEL', reference: 'REPORT_REFERENCE_LABEL', @@ -95,10 +96,12 @@ function makeRow(overrides: Partial = {}): ReportContentRow { invoiceAmountText: '€100.00', allocatedAmountValueText: '€100.00', allocatedMarkers: '', + isDeposit: false, isRefund: false, refundNoteText: '', usageText: 'Baseline usage', attachmentsNote: null, + areaText: null, ...overrides, }; } @@ -106,6 +109,7 @@ function makeRow(overrides: Partial = {}): ReportContentRow { function makeContent(overrides: Partial = {}): ReportContent { return { isOverview: false, + isClaim: false, tableTitle: 'Title', labels: LABELS, sourceInfo: { @@ -648,20 +652,17 @@ describe('ReportContentEditor — table rows', () => { }); describe('ReportContentEditor — summary rows and footnotes', () => { - it('renders each summaryRow as a plain (non-editable) row with label and amount', () => { + it('AC4: renders a single Total-only summaryRow as a plain (non-editable) row with label and amount', () => { const content = makeContent({ - // Distinct amount strings from the default row's own €100.00 to avoid ambiguous matches + // Distinct amount string from the default row's own €100.00 to avoid ambiguous matches // against the table body (which also renders the fixture row's amount cells). - summaryRows: [ - { key: 'subtotal-paid', label: 'Paid Subtotal', amountText: '€150.00' }, - { key: 'total', label: 'Total', amountText: '€550.00' }, - ], + summaryRows: [{ key: 'total', label: 'Total', amountText: '€550.00' }], }); renderEditor({ content }); - expect(screen.getByText('Paid Subtotal')).toBeInTheDocument(); - expect(screen.getByText('€150.00')).toBeInTheDocument(); expect(screen.getByText('Total')).toBeInTheDocument(); expect(screen.getByText('€550.00')).toBeInTheDocument(); + // No subtotal-labeled row survives alongside it. + expect(screen.queryByText(/Subtotal/)).not.toBeInTheDocument(); }); it('renders no summary table when summaryRows is empty', () => { @@ -670,17 +671,23 @@ describe('ReportContentEditor — summary rows and footnotes', () => { expect(container.querySelector('table + table')).not.toBeInTheDocument(); }); - it('renders each footnote with its marker and text, read-only', () => { + it('renders each footnote with its unnumbered/shared marker and text, read-only', () => { const content = makeContent({ footnotes: [ - { id: 'split-1', marker: '†1', text: 'ACME (A-1) — split footnote' }, - { id: 'deposit-1', marker: '‡1', text: 'Beta (B-2) — deposit footnote' }, + { id: 'split', marker: '†', text: 'Amount shown reflects only the portion allocated.' }, + { + id: 'deposit-reduced', + marker: '‡', + text: 'This position reflects deposits claimed separately.', + }, ], }); renderEditor({ content }); - expect(screen.getByText('†1:')).toBeInTheDocument(); - expect(screen.getByText(/ACME \(A-1\) — split footnote/)).toBeInTheDocument(); - expect(screen.getByText('‡1:')).toBeInTheDocument(); + expect(screen.getByText('†:')).toBeInTheDocument(); + expect( + screen.getByText(/Amount shown reflects only the portion allocated\./), + ).toBeInTheDocument(); + expect(screen.getByText('‡:')).toBeInTheDocument(); }); it('renders no footnotes block when footnotes is empty', () => { @@ -690,6 +697,86 @@ describe('ReportContentEditor — summary rows and footnotes', () => { }); }); +describe('ReportContentEditor — isClaim (AC3: claim reports omit the source info metadata block)', () => { + it('AC3.1: omits the sourceInfoBlock entirely when content.isClaim is true', () => { + const content = makeContent({ isClaim: true }); + renderEditor({ content }); + expect(screen.queryByText(`${LABELS.source}: Home Loan`)).not.toBeInTheDocument(); + expect(screen.queryByText(new RegExp(`^${LABELS.sourceType}:`))).not.toBeInTheDocument(); + expect(screen.queryByText(new RegExp(`^${LABELS.generatedAt}:`))).not.toBeInTheDocument(); + }); + + it('renders the sourceInfoBlock when content.isClaim is false (budget-overview/proof-of-funds)', () => { + const content = makeContent({ isClaim: false }); + renderEditor({ content }); + expect(screen.getByText(`${LABELS.source}: Home Loan`)).toBeInTheDocument(); + }); +}); + +describe('ReportContentEditor — isDeposit (AC2.1: inline Deposit badge, no marker)', () => { + it('renders a Deposit badge in the desktop allocated cell and no ‡ marker for an isDeposit row', () => { + const rows = [ + makeRow({ + invoiceId: 'inv-1', + isDeposit: true, + allocatedMarkers: '', + allocatedAmountValueText: '€300.00', + }), + ]; + const { container } = renderEditor({ content: makeContent({ rows }) }); + const table = getDesktopTable(container); + expect(within(table).getByText('REPORT_DEPOSIT_LABEL')).toBeInTheDocument(); + expect(within(table).queryByText(/‡/)).not.toBeInTheDocument(); + }); + + it('renders the same Deposit badge in the mobile card allocated row', () => { + const rows = [makeRow({ invoiceId: 'inv-1', isDeposit: true })]; + const { container } = renderEditor({ content: makeContent({ rows }) }); + const card = within(getMobileList(container)); + expect(card.getByText('REPORT_DEPOSIT_LABEL')).toBeInTheDocument(); + }); + + it('renders no Deposit badge for a non-deposit row', () => { + const rows = [makeRow({ invoiceId: 'inv-1', isDeposit: false })]; + const { container } = renderEditor({ content: makeContent({ rows }) }); + expect(screen.queryByText('REPORT_DEPOSIT_LABEL')).not.toBeInTheDocument(); + void container; + }); +}); + +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', () => { + const rows = [ + makeRow({ invoiceId: 'inv-1', usageText: 'Kitchen work', areaText: 'Ground Floor' }), + ]; + const { container } = renderEditor({ content: makeContent({ rows }) }); + 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'); + const areaEl = within(table).getByText('Ground Floor'); + expect(areaEl.className).toContain(styles.usageAreaText); + expect(areaEl.tagName).toBe('DIV'); + }); + + it('renders areaText as a 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.tagName).toBe('SPAN'); + }); + + it('renders no area element (desktop or mobile) when areaText is null', () => { + const rows = [makeRow({ invoiceId: 'inv-1', areaText: null })]; + const { container } = renderEditor({ content: makeContent({ rows }) }); + expect(container.querySelectorAll(`.${styles.usageAreaText}`)).toHaveLength(0); + }); +}); + describe( 'ReportContentEditor — mobile card list (CSS-only responsive, ' + 'always rendered alongside the desktop table — see header comment)', diff --git a/client/src/components/reports/ReportContentEditor.tsx b/client/src/components/reports/ReportContentEditor.tsx index 5356d7275..855b2e802 100644 --- a/client/src/components/reports/ReportContentEditor.tsx +++ b/client/src/components/reports/ReportContentEditor.tsx @@ -133,22 +133,24 @@ export function ReportContentEditor({ )} {/* Source Info Block */} -
-

- {content.labels.source}: {content.sourceInfo.sourceName} -

-

- {content.labels.sourceType}: {content.sourceInfo.sourceTypeText} -

- {content.sourceInfo.referenceText && ( + {!content.isClaim && ( +

- {content.labels.reference}: {content.sourceInfo.referenceText} + {content.labels.source}: {content.sourceInfo.sourceName}

- )} -

- {content.labels.generatedAt}: {content.sourceInfo.generatedAtText} -

-
+

+ {content.labels.sourceType}: {content.sourceInfo.sourceTypeText} +

+ {content.sourceInfo.referenceText && ( +

+ {content.labels.reference}: {content.sourceInfo.referenceText} +

+ )} +

+ {content.labels.generatedAt}: {content.sourceInfo.generatedAtText} +

+
+ )} {/* Report Table */}

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

@@ -194,6 +196,18 @@ export function ReportContentEditor({ {row.allocatedAmountValueText} {row.allocatedMarkers} {row.isRefund && ` ${row.refundNoteText}`} + {row.isDeposit && ( + + )} onFieldReset(overrideKey.row(row.invoiceId).usageText)} /> + {row.areaText &&
{row.areaText}
} {row.attachmentsNote !== null && ( @@ -281,12 +296,25 @@ export function ReportContentEditor({
{content.labels.allocatedAmount} - - {row.allocatedAmountValueText} - {row.allocatedMarkers} - {row.isRefund && ` ${row.refundNoteText}`} + + + {row.allocatedAmountValueText} + {row.allocatedMarkers} + {row.isRefund && ` ${row.refundNoteText}`} + + {row.isDeposit && ( + + )}
@@ -306,6 +334,7 @@ export function ReportContentEditor({ isEdited={isFieldEdited(overrideKey.row(row.invoiceId).usageText)} onReset={() => onFieldReset(overrideKey.row(row.invoiceId).usageText)} /> + {row.areaText && {row.areaText}}
{row.attachmentsNote !== null && (
diff --git a/client/src/components/reports/ReportInvoiceList.test.tsx b/client/src/components/reports/ReportInvoiceList.test.tsx index d2c39ef22..956e7f39d 100644 --- a/client/src/components/reports/ReportInvoiceList.test.tsx +++ b/client/src/components/reports/ReportInvoiceList.test.tsx @@ -724,13 +724,25 @@ describe('ReportInvoiceList', () => { id: 'line-2', description: 'Roofing', allocatedPortion: 200, - linkedItem: { type: 'work_item' as const, id: 'wi-1', name: 'Roof Replacement' }, + linkedItem: { + type: 'work_item' as const, + id: 'wi-1', + name: 'Roof Replacement', + areaId: null, + areaName: null, + }, }; const householdItemLine = { id: 'line-3', description: 'Cabinet', allocatedPortion: 100, - linkedItem: { type: 'household_item' as const, id: 'hi-1', name: 'Kitchen Cabinet' }, + linkedItem: { + type: 'household_item' as const, + id: 'hi-1', + name: 'Kitchen Cabinet', + areaId: null, + areaName: null, + }, }; const deposit = { id: 'dep-1', diff --git a/client/src/i18n/de/budget.json b/client/src/i18n/de/budget.json index 600f112fd..0ffff9107 100644 --- a/client/src/i18n/de/budget.json +++ b/client/src/i18n/de/budget.json @@ -1237,7 +1237,6 @@ "status": "Status", "invoiceAmount": "Rechnungsbetrag", "allocatedAmount": "Zugeordneter Betrag", - "subtotal": "Zwischensumme", "total": "Gesamt", "refundNote": "(Rückerstattung)", "footnoteFetchFailed": "Dokument konnte nicht abgerufen werden", @@ -1253,8 +1252,7 @@ "attachmentsNote_other": "{{count}} Anhänge: {{types}}", "attachmentsNoteNoType_one": "{{count}} Anhang", "attachmentsNoteNoType_other": "{{count}} Anhänge", - "depositReducedFootnote": "Diese Position berücksichtigt separat eingereichte Abschlagszahlungen.", - "depositConstitutedFootnote": "Dies ist eine Abschlagszahlung." + "depositReducedFootnote": "Diese Position berücksichtigt separat eingereichte Abschlagszahlungen." }, "sourceType": { "bank_loan": "Bankdarlehen", diff --git a/client/src/i18n/en/budget.json b/client/src/i18n/en/budget.json index 2d2e7f05a..11d3dbec2 100644 --- a/client/src/i18n/en/budget.json +++ b/client/src/i18n/en/budget.json @@ -1247,14 +1247,12 @@ "attachmentsNote_other": "{{count}} attachments: {{types}}", "attachmentsNoteNoType_one": "{{count}} attachment", "attachmentsNoteNoType_other": "{{count}} attachments", - "subtotal": "Subtotal", "total": "Total", "refundNote": "(refund)", "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.", - "depositConstitutedFootnote": "This is a deposit." + "depositReducedFootnote": "This position reflects deposits claimed separately." }, "sourceType": { "bank_loan": "Bank Loan", diff --git a/client/src/lib/reportContent/applyAiContent.test.ts b/client/src/lib/reportContent/applyAiContent.test.ts index 3ca2b2ac8..8f344fd52 100644 --- a/client/src/lib/reportContent/applyAiContent.test.ts +++ b/client/src/lib/reportContent/applyAiContent.test.ts @@ -23,10 +23,12 @@ function makeRow(overrides: Partial = {}): ReportContentRow { invoiceAmountText: '€100.00', allocatedAmountValueText: '€100.00', allocatedMarkers: '', + isDeposit: false, isRefund: false, refundNoteText: '', usageText: 'Baseline usage', attachmentsNote: null, + areaText: null, ...overrides, }; } @@ -41,6 +43,7 @@ function makeLabels(): ReportContent['labels'] { allocatedAmount: 'Allocated Amount', usage: 'Usage', attachmentsNote: 'Attachments Note', + deposit: 'Deposit', source: 'Source', sourceType: 'Source Type', reference: 'Reference', @@ -51,6 +54,7 @@ function makeLabels(): ReportContent['labels'] { function makeContent(overrides: Partial = {}): ReportContent { return { isOverview: false, + isClaim: false, tableTitle: 'Title', labels: makeLabels(), sourceInfo: { diff --git a/client/src/lib/reportContent/applyOverrides.test.ts b/client/src/lib/reportContent/applyOverrides.test.ts index d800f78d2..8dfc81835 100644 --- a/client/src/lib/reportContent/applyOverrides.test.ts +++ b/client/src/lib/reportContent/applyOverrides.test.ts @@ -22,10 +22,12 @@ function makeRow(overrides: Partial = {}): ReportContentRow { invoiceAmountText: '€100.00', allocatedAmountValueText: '€100.00', allocatedMarkers: '', + isDeposit: false, isRefund: false, refundNoteText: '', usageText: 'Baseline usage', attachmentsNote: null, + areaText: null, ...overrides, }; } @@ -40,6 +42,7 @@ function makeLabels(): ReportContent['labels'] { allocatedAmount: 'Allocated Amount', usage: 'Usage', attachmentsNote: 'Attachments Note', + deposit: 'Deposit', source: 'Source', sourceType: 'Source Type', reference: 'Reference', @@ -50,6 +53,7 @@ function makeLabels(): ReportContent['labels'] { function makeContent(overrides: Partial = {}): ReportContent { return { isOverview: false, + isClaim: false, tableTitle: 'Title', labels: makeLabels(), sourceInfo: { diff --git a/client/src/lib/reportContent/buildReportContent.test.ts b/client/src/lib/reportContent/buildReportContent.test.ts index 294b6caa3..2b353c24c 100644 --- a/client/src/lib/reportContent/buildReportContent.test.ts +++ b/client/src/lib/reportContent/buildReportContent.test.ts @@ -62,6 +62,19 @@ function makeBudgetLine(overrides: Partial = {}): Source }; } +function makeLinkedItem( + overrides: Partial = {}, +): NonNullable { + return { + type: 'work_item', + id: 'wi-1', + name: 'Kitchen', + areaId: null, + areaName: null, + ...overrides, + }; +} + function makeDeposit(overrides: Partial = {}): SourceReportDeposit { return { id: 'dep-1', @@ -236,9 +249,9 @@ describe('buildReportContent — rows', () => { it('dedupes and comma-joins distinct linked-item names in first-occurrence order', () => { const invoice = makeInvoice({ budgetLines: [ - makeBudgetLine({ linkedItem: { type: 'work_item', id: 'wi-1', name: 'Kitchen' } }), - makeBudgetLine({ linkedItem: { type: 'work_item', id: 'wi-2', name: 'Bathroom' } }), - makeBudgetLine({ linkedItem: { type: 'work_item', id: 'wi-1', name: 'Kitchen' } }), + makeBudgetLine({ linkedItem: makeLinkedItem({ id: 'wi-1', name: 'Kitchen' }) }), + makeBudgetLine({ linkedItem: makeLinkedItem({ id: 'wi-2', name: 'Bathroom' }) }), + makeBudgetLine({ linkedItem: makeLinkedItem({ id: 'wi-1', name: 'Kitchen' }) }), ], }); const report = makeReport([invoice]); @@ -323,8 +336,8 @@ describe('buildReportContent — rows', () => { }); }); - describe('allocatedMarkers (split † / deposit ‡)', () => { - it('adds † only when isSplit and budgetLines.length > 0, no deposits', () => { + describe('allocatedMarkers (split † / deposit ‡, unnumbered/shared per story #1923) + isDeposit', () => { + it('adds unnumbered † only when isSplit and budgetLines.length > 0, no deposits', () => { const invoice = makeInvoice({ isSplit: true, budgetLines: [makeBudgetLine()], @@ -332,32 +345,58 @@ describe('buildReportContent — rows', () => { }); const report = makeReport([invoice]); const content = buildReportContent(report, new Set(['inv-1']), 'claim', t, formatters); - expect(content.rows[0]!.allocatedMarkers).toBe('†1'); + expect(content.rows[0]!.allocatedMarkers).toBe('†'); + expect(content.rows[0]!.isDeposit).toBe(false); + }); + + it('adds unnumbered ‡ only when isSplit and the deposit is untagged (reduced), no budget lines', () => { + const invoice = makeInvoice({ + isSplit: true, + budgetLines: [], + deposits: [makeDeposit({ budgetSourceId: null })], + }); + 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]!.isDeposit).toBe(false); }); - it('adds ‡ only when isSplit and deposits.length > 0, no budget lines', () => { + it('AC2.1: adds NO marker and sets isDeposit=true when the deposit is tagged to this report source (constituted)', () => { const invoice = makeInvoice({ isSplit: true, budgetLines: [], deposits: [makeDeposit({ budgetSourceId: 'src-1' })], }); - const report = makeReport([invoice]); + 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]!.isDeposit).toBe(true); + }); + + it('AC2.4: adds both † and ‡ (order †‡) when split budget lines and a reduced (untagged) deposit are both present', () => { + const invoice = makeInvoice({ + isSplit: true, + budgetLines: [makeBudgetLine()], + deposits: [makeDeposit({ budgetSourceId: null })], + }); + const report = makeReport([invoice], { id: 'src-1' }); const content = buildReportContent(report, new Set(['inv-1']), 'claim', t, formatters); - expect(content.rows[0]!.allocatedMarkers).toBe('‡1'); + expect(content.rows[0]!.allocatedMarkers).toBe('†‡'); }); - it('adds both † and ‡ when both budget lines and deposits are present', () => { + it('adds only † (no ‡, isDeposit=true) when split budget lines are combined with a constituted (tagged) deposit', () => { const invoice = makeInvoice({ isSplit: true, budgetLines: [makeBudgetLine()], deposits: [makeDeposit({ budgetSourceId: 'src-1' })], }); - const report = makeReport([invoice]); + const report = makeReport([invoice], { id: 'src-1' }); const content = buildReportContent(report, new Set(['inv-1']), 'claim', t, formatters); - expect(content.rows[0]!.allocatedMarkers).toBe('†1‡1'); + expect(content.rows[0]!.allocatedMarkers).toBe('†'); + expect(content.rows[0]!.isDeposit).toBe(true); }); - it('adds neither marker when isSplit is false, regardless of budgetLines/deposits content', () => { + it('adds neither marker nor isDeposit when isSplit is false, regardless of budgetLines/deposits content', () => { const invoice = makeInvoice({ isSplit: false, budgetLines: [makeBudgetLine()], @@ -366,6 +405,7 @@ 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]!.isDeposit).toBe(false); }); it('adds neither marker when isSplit is true but budgetLines and deposits are both empty', () => { @@ -373,6 +413,7 @@ 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]!.isDeposit).toBe(false); }); it('never assigns markers to an excluded invoice, even when isSplit with lines/deposits', () => { @@ -384,34 +425,63 @@ 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 numbered for the excluded one. + // 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', () => { - it('produces a split footnote with vendor/invoice-number attribution using the dedicated splitFootnote key', () => { - const invoice = makeInvoice({ - invoiceId: 'inv-split', +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', () => { + const inv1 = makeInvoice({ + invoiceId: 'inv-1', vendorName: 'Gamma Corp', invoiceNumber: 'G-9', isSplit: true, budgetLines: [makeBudgetLine()], }); - const report = makeReport([invoice]); - const content = buildReportContent(report, new Set(['inv-split']), 'claim', t, formatters); + 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-1', - marker: '†1', - text: 'Gamma Corp (G-9) — sourceReports.table.splitFootnote', + 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('produces a "constituted" deposit footnote when the deposit is tagged to this source', () => { + it('AC1.3: produces no † marker and no split footnote anywhere when no included invoice is split', () => { + 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(''); + }); + + it('AC2.2: produces NO footnote entry for a constituted (tagged) deposit — the row gets isDeposit instead', () => { const invoice = makeInvoice({ isSplit: true, budgetLines: [], @@ -419,10 +489,11 @@ describe('buildReportContent — footnotes', () => { }); const report = makeReport([invoice], { id: 'src-1' }); const content = buildReportContent(report, new Set(['inv-1']), 'claim', t, formatters); - expect(content.footnotes[0]!.text).toContain('sourceReports.table.depositConstitutedFootnote'); + expect(content.footnotes).toEqual([]); + expect(content.rows[0]!.isDeposit).toBe(true); }); - it('produces a "reduced" deposit footnote when the deposit is untagged (or tagged elsewhere)', () => { + it('AC2.3: produces exactly one shared, unnumbered ‡ footnote when one or more invoices have a reduced (untagged) deposit', () => { const invoice = makeInvoice({ isSplit: true, budgetLines: [], @@ -430,78 +501,188 @@ describe('buildReportContent — footnotes', () => { }); const report = makeReport([invoice], { id: 'src-1' }); const content = buildReportContent(report, new Set(['inv-1']), 'claim', t, formatters); - expect(content.footnotes[0]!.text).toContain('sourceReports.table.depositReducedFootnote'); + expect(content.footnotes).toEqual([ + { + id: 'deposit-reduced', + marker: '‡', + text: 'sourceReports.table.depositReducedFootnote', + }, + ]); }); - it('orders footnotes split-block-first, then deposit-block, independent numbering per block', () => { - const splitOnly = makeInvoice({ - invoiceId: 'inv-split', - vendorName: 'Split Vendor', - invoiceNumber: 'S-1', + it('AC2.3: multiple invoices with reduced deposits still produce exactly one shared ‡ entry', () => { + const inv1 = makeInvoice({ + invoiceId: 'inv-1', isSplit: true, - budgetLines: [makeBudgetLine()], + budgetLines: [], + deposits: [makeDeposit({ id: 'dep-1', budgetSourceId: null })], }); - const depositOnly = makeInvoice({ - invoiceId: 'inv-deposit', - vendorName: 'Deposit Vendor', - invoiceNumber: 'D-1', + const inv2 = makeInvoice({ + invoiceId: 'inv-2', isSplit: true, budgetLines: [], - deposits: [makeDeposit({ budgetSourceId: 'src-1' })], + deposits: [makeDeposit({ id: 'dep-2', budgetSourceId: null })], }); - const report = makeReport([splitOnly, depositOnly], { id: 'src-1' }); - const content = buildReportContent( - report, - new Set(['inv-split', 'inv-deposit']), - 'claim', - t, - formatters, - ); - expect(content.footnotes.map((f) => f.id)).toEqual(['split-1', 'deposit-1']); + 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); + }); + + it('AC2.4: both split and reduced-deposit invoices present → markers "†‡" on the combined row, footnotes ordered [split, deposit-reduced]', () => { + const combined = makeInvoice({ + invoiceId: 'inv-1', + isSplit: true, + budgetLines: [makeBudgetLine()], + deposits: [makeDeposit({ budgetSourceId: null })], + }); + 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); }); - it('produces no footnotes when no invoice is split', () => { + it('produces no footnotes when no invoice is split or has a reduced deposit', () => { const report = makeReport([makeInvoice()]); const content = buildReportContent(report, new Set(['inv-1']), 'claim', t, formatters); expect(content.footnotes).toEqual([]); }); }); -describe('buildReportContent — summaryRows', () => { - it('adds one subtotal row per distinct status among included invoices, in pending/paid/claimed/quotation order', () => { +describe('buildReportContent — summaryRows (AC4: total-only summary)', () => { + it('AC4.1/4.2: produces exactly one summary row (key "total"), even when included invoices span 2+ distinct statuses', () => { const pending = makeInvoice({ invoiceId: 'inv-1', status: 'pending', allocatedAmount: 100 }); const paid = makeInvoice({ invoiceId: 'inv-2', status: 'paid', allocatedAmount: 200 }); - const report = makeReport([pending, paid]); - const content = buildReportContent(report, new Set(['inv-1', 'inv-2']), 'claim', t, formatters); - const subtotalKeys = content.summaryRows - .filter((r) => r.key.startsWith('subtotal-')) - .map((r) => r.key); - expect(subtotalKeys).toEqual(['subtotal-pending', 'subtotal-paid']); - expect(content.summaryRows[0]!.amountText).toBe('€100.00'); - expect(content.summaryRows[1]!.amountText).toBe('€200.00'); + const quotation = makeInvoice({ + invoiceId: 'inv-3', + status: 'quotation', + allocatedAmount: 50, + }); + const report = makeReport([pending, paid, quotation]); + const content = buildReportContent( + report, + new Set(['inv-1', 'inv-2', 'inv-3']), + 'budget-overview', + t, + formatters, + ); + expect(content.summaryRows).toHaveLength(1); + expect(content.summaryRows[0]!.key).toBe('total'); + // No subtotal-* rows of any kind survive. + expect(content.summaryRows.some((r) => r.key.startsWith('subtotal'))).toBe(false); }); - it('does not add a subtotal row for a status with zero included invoices', () => { + it('AC4.1: produces exactly one summary row even when only a single status is present', () => { const pending = makeInvoice({ invoiceId: 'inv-1', status: 'pending', allocatedAmount: 100 }); const report = makeReport([pending]); const content = buildReportContent(report, new Set(['inv-1']), 'claim', t, formatters); - const subtotalKeys = content.summaryRows.filter((r) => r.key.startsWith('subtotal-')); - expect(subtotalKeys).toHaveLength(1); + expect(content.summaryRows).toHaveLength(1); }); - it('always appends a final "total" row with the sum of included invoices\' allocatedAmount', () => { + it("AC4.3: the total row's amount is the sum of allocatedAmount over included invoices — unchanged math", () => { const invoice1 = makeInvoice({ invoiceId: 'inv-1', allocatedAmount: 100 }); const invoice2 = makeInvoice({ invoiceId: 'inv-2', allocatedAmount: 250 }); const excluded = makeInvoice({ invoiceId: 'inv-3', allocatedAmount: 9999 }); const report = makeReport([invoice1, invoice2, excluded]); const content = buildReportContent(report, new Set(['inv-1', 'inv-2']), 'claim', t, formatters); - const total = content.summaryRows.at(-1)!; + const total = content.summaryRows[0]!; expect(total.key).toBe('total'); expect(total.label).toBe('sourceReports.table.total'); expect(total.amountText).toBe('€350.00'); }); }); +describe('buildReportContent — isClaim (AC3)', () => { + it('is true only for the claim useCase, false for budget-overview and proof-of-funds', () => { + const report = makeReport([]); + expect(buildReportContent(report, new Set(), 'claim', t).isClaim).toBe(true); + expect(buildReportContent(report, new Set(), 'budget-overview', t).isClaim).toBe(false); + expect(buildReportContent(report, new Set(), 'proof-of-funds', t).isClaim).toBe(false); + }); +}); + +describe('buildReportContent — areaText (AC5.2–5.5)', () => { + it('AC5.2: renders a single leaf area name when one budget line resolves a linked item with an area', () => { + const invoice = makeInvoice({ + budgetLines: [ + makeBudgetLine({ + linkedItem: makeLinkedItem({ areaId: 'area-1', areaName: 'Kitchen' }), + }), + ], + }); + const report = makeReport([invoice]); + const content = buildReportContent(report, new Set(['inv-1']), 'claim', t, formatters); + expect(content.rows[0]!.areaText).toBe('Kitchen'); + }); + + it('AC5.2: dedupes and comma-joins distinct area names in first-occurrence order across multiple budget lines', () => { + const invoice = makeInvoice({ + budgetLines: [ + makeBudgetLine({ + id: 'bl-1', + linkedItem: makeLinkedItem({ id: 'wi-1', areaId: 'area-1', areaName: 'Kitchen' }), + }), + makeBudgetLine({ + id: 'bl-2', + linkedItem: makeLinkedItem({ id: 'wi-2', areaId: 'area-2', areaName: 'Bathroom' }), + }), + makeBudgetLine({ + id: 'bl-3', + linkedItem: makeLinkedItem({ id: 'wi-1', areaId: 'area-1', areaName: 'Kitchen' }), + }), + ], + }); + const report = makeReport([invoice]); + const content = buildReportContent(report, new Set(['inv-1']), 'claim', t, formatters); + expect(content.rows[0]!.areaText).toBe('Kitchen, Bathroom'); + }); + + it('AC5.4: is null when budgetLines is empty (no linkedItem at all)', () => { + const invoice = makeInvoice({ budgetLines: [] }); + const report = makeReport([invoice]); + const content = buildReportContent(report, new Set(['inv-1']), 'claim', t, formatters); + expect(content.rows[0]!.areaText).toBeNull(); + }); + + it('AC5.4: is null when the linked item has no area assigned (areaName null)', () => { + const invoice = makeInvoice({ + budgetLines: [ + makeBudgetLine({ linkedItem: makeLinkedItem({ areaId: null, areaName: null }) }), + ], + }); + const report = makeReport([invoice]); + const content = buildReportContent(report, new Set(['inv-1']), 'claim', t, formatters); + expect(content.rows[0]!.areaText).toBeNull(); + }); + + it('AC5.4: is null when budget lines have no linkedItem (description-only fallback)', () => { + const invoice = makeInvoice({ + budgetLines: [makeBudgetLine({ linkedItem: null, description: 'Materials' })], + }); + const report = makeReport([invoice]); + const content = buildReportContent(report, new Set(['inv-1']), 'claim', t, formatters); + expect(content.rows[0]!.areaText).toBeNull(); + }); + + it('AC5.5: renders only the leaf (own) area name, never a parent-path expansion — the row consumes areaName verbatim', () => { + // The child's bare name only — buildReportContent does not expand the hierarchy; it trusts + // sourceReportService to have already resolved the leaf-only areaName (see + // sourceReportService.test.ts "child area with parent" coverage for the server-side guarantee). + const invoice = makeInvoice({ + budgetLines: [ + makeBudgetLine({ + linkedItem: makeLinkedItem({ areaId: 'area-child', areaName: 'Ensuite' }), + }), + ], + }); + const report = makeReport([invoice]); + const content = buildReportContent(report, new Set(['inv-1']), 'claim', t, formatters); + expect(content.rows[0]!.areaText).toBe('Ensuite'); + expect(content.rows[0]!.areaText).not.toContain('/'); + }); +}); + describe('buildReportContent — cover letter', () => { it('is null when includeCoverLetter is false (default when options omitted)', () => { const report = makeReport([]); diff --git a/client/src/lib/reportContent/buildReportContent.ts b/client/src/lib/reportContent/buildReportContent.ts index 5dbb505c0..ca51bb52b 100644 --- a/client/src/lib/reportContent/buildReportContent.ts +++ b/client/src/lib/reportContent/buildReportContent.ts @@ -26,6 +26,24 @@ function uniqueInOrder(items: T[]): T[] { return Array.from(new Set(items)); } +/** + * Helper: get area text from invoice budget lines. + * Returns distinct linked item areaName values, first-seen order, comma-joined, or null when empty. + */ +function getAreaText(invoice: { + budgetLines: Array<{ linkedItem: { areaName: string | null } | null }>; +}): string | null { + const areaNames = invoice.budgetLines + .map((line) => line.linkedItem?.areaName) + .filter((name) => name !== null && name !== undefined) as string[]; + + if (areaNames.length === 0) { + return null; + } + + return uniqueInOrder(areaNames).join(', '); +} + /** * Helper: get usage text from invoice budget lines. * Returns distinct linked item names if any line has linkedItem; else distinct descriptions; else '—'. @@ -120,39 +138,32 @@ export function buildReportContent( generatedAtText, }; - // Track footnotes for split/deposit (skip footnotes handled at generation time) - const splitFootnotesByInvoiceId = new Map(); - const depositFootnotesByInvoiceId = new Map< - string, - { num: number; wording: 'reduced' | 'constituted' } - >(); - let splitFootnoteNum = 1; - let depositFootnoteNum = 1; + // Track invoices for split/deposit markers + const splitInvoiceIds = new Set(); + const depositReducedInvoiceIds = new Set(); + const depositConstitutedInvoiceIds = new Set(); for (const invoice of report.invoices) { - if (!includedInvoiceIds.has(invoice.invoiceId) || !invoice.isSplit) { + if (!includedInvoiceIds.has(invoice.invoiceId)) { continue; } - if (invoice.budgetLines.length > 0) { - splitFootnotesByInvoiceId.set(invoice.invoiceId, splitFootnoteNum++); + + if (invoice.isSplit && invoice.budgetLines.length > 0) { + splitInvoiceIds.add(invoice.invoiceId); } - if (invoice.deposits.length > 0) { + + if (invoice.isSplit && invoice.deposits.length > 0) { const taggedDeposit = invoice.deposits.some((d) => d.budgetSourceId === report.source.id); - depositFootnotesByInvoiceId.set(invoice.invoiceId, { - num: depositFootnoteNum++, - wording: taggedDeposit ? 'constituted' : 'reduced', - }); + if (taggedDeposit) { + depositConstitutedInvoiceIds.add(invoice.invoiceId); + } else { + depositReducedInvoiceIds.add(invoice.invoiceId); + } } } // Build table rows const rows: ReportContentRow[] = []; - const statusCounts: Record = { - pending: 0, - paid: 0, - claimed: 0, - quotation: 0, - }; for (const invoice of report.invoices) { if (!includedInvoiceIds.has(invoice.invoiceId)) { @@ -160,7 +171,6 @@ export function buildReportContent( } const status = invoice.status as InvoiceStatus; - statusCounts[status]++; const invoiceAmountText = reportFormatters ? reportFormatters.formatCurrency(invoice.invoiceAmount) @@ -172,20 +182,20 @@ export function buildReportContent( const statusText = isOverview ? reportT(`sources.lines.invoiceStatus.${status}`) : null; - // Compute allocated markers (split + deposit only; skip markers added at generation time) - const splitMarker = splitFootnotesByInvoiceId.get(invoice.invoiceId); - const depositMarker = depositFootnotesByInvoiceId.get(invoice.invoiceId); + // Compute allocated markers († for split, ‡ for reduced; no markers for constituted deposits) let allocatedMarkers = ''; - if (splitMarker) { - allocatedMarkers += `†${splitMarker}`; + if (splitInvoiceIds.has(invoice.invoiceId)) { + allocatedMarkers += '†'; } - if (depositMarker) { - allocatedMarkers += `‡${depositMarker.num}`; + if (depositReducedInvoiceIds.has(invoice.invoiceId)) { + allocatedMarkers += '‡'; } + const isDeposit = depositConstitutedInvoiceIds.has(invoice.invoiceId); const refundNoteText = reportT('sourceReports.table.refundNote'); const usageText = getUsageText(invoice); const attachmentsNote = getAttachmentNote(invoice, reportT); + const areaText = getAreaText(invoice); rows.push({ invoiceId: invoice.invoiceId, @@ -197,39 +207,18 @@ export function buildReportContent( invoiceAmountText, allocatedAmountValueText, allocatedMarkers, + isDeposit, isRefund: invoice.lineKind === 'refund-adjustment', refundNoteText, usageText, attachmentsNote, + areaText, }); } - // Build summary rows (subtotal per status + total) + // Build summary rows (single total row only) const summaryRows: ReportContentSummaryRow[] = []; - const statusLabels: Record = { - pending: 'sources.lines.invoiceStatus.pending', - paid: 'sources.lines.invoiceStatus.paid', - claimed: 'sources.lines.invoiceStatus.claimed', - quotation: 'sources.lines.invoiceStatus.quotation', - }; - - for (const [status] of Object.entries(statusCounts)) { - const count = statusCounts[status as InvoiceStatus]; - if (count > 0) { - const invoicesWithStatus = report.invoices.filter( - (inv) => inv.status === status && includedInvoiceIds.has(inv.invoiceId), - ); - const subtotal = invoicesWithStatus.reduce((sum, inv) => sum + inv.allocatedAmount, 0); - - const amountText = reportFormatters ? reportFormatters.formatCurrency(subtotal) : '—'; - const label = `${reportT(statusLabels[status as InvoiceStatus])} ${reportT('sourceReports.table.subtotal')}`; - const key = `subtotal-${status}`; - - summaryRows.push({ key, label, amountText }); - } - } - // Add total row const includedTotal = report.invoices .filter((inv) => includedInvoiceIds.has(inv.invoiceId)) .reduce((sum, inv) => sum + inv.allocatedAmount, 0); @@ -241,36 +230,22 @@ export function buildReportContent( amountText: totalAmountText, }); - // Build footnotes (split + deposit blocks; skip footnotes added at generation time) + // Build footnotes (at most 2 shared entries: split + deposit-reduced) const footnotes: ReportContentFootnote[] = []; - // Split block - for (const [invoiceId, splitNum] of splitFootnotesByInvoiceId) { - const invoice = report.invoices.find((inv) => inv.invoiceId === invoiceId); - const vendorName = invoice?.vendorName ?? '—'; - const invoiceNumber = invoice?.invoiceNumber ?? '—'; - + if (splitInvoiceIds.size > 0) { footnotes.push({ - id: `split-${splitNum}`, - marker: `†${splitNum}`, - text: `${vendorName} (${invoiceNumber}) — ${reportT('sourceReports.table.splitFootnote')}`, + id: 'split', + marker: '†', + text: reportT('sourceReports.table.splitFootnote'), }); } - // Deposit block - for (const [invoiceId, depositMarker] of depositFootnotesByInvoiceId) { - const invoice = report.invoices.find((inv) => inv.invoiceId === invoiceId); - const vendorName = invoice?.vendorName ?? '—'; - const invoiceNumber = invoice?.invoiceNumber ?? '—'; - const wordingKey = - depositMarker.wording === 'constituted' - ? 'depositConstitutedFootnote' - : 'depositReducedFootnote'; - + if (depositReducedInvoiceIds.size > 0) { footnotes.push({ - id: `deposit-${depositMarker.num}`, - marker: `‡${depositMarker.num}`, - text: `${vendorName} (${invoiceNumber}) — ${reportT(`sourceReports.table.${wordingKey}`)}`, + id: 'deposit-reduced', + marker: '‡', + text: reportT('sourceReports.table.depositReducedFootnote'), }); } @@ -300,8 +275,11 @@ export function buildReportContent( }; } + const isClaim = useCase === 'claim'; + return { isOverview, + isClaim, tableTitle, labels: { vendor: reportT('sourceReports.table.vendor'), @@ -312,6 +290,7 @@ export function buildReportContent( allocatedAmount: reportT('sourceReports.table.allocatedAmount'), usage: reportT('sourceReports.table.usage'), attachmentsNote: reportT('sourceReports.editable.attachmentsNoteLabel'), + deposit: reportT('sourceReports.table.attachmentType.deposit'), 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 dd655d677..6e7473f53 100644 --- a/client/src/lib/reportContent/types.ts +++ b/client/src/lib/reportContent/types.ts @@ -13,11 +13,13 @@ export interface ReportContentRow { statusText: string | null; // null when useCase !== 'budget-overview' invoiceAmountText: string; allocatedAmountValueText: string; // formatted currency only — no markers/refund note - allocatedMarkers: string; // '', '†1', '‡2', '†1‡2' + allocatedMarkers: string; // '', '†', '‡', '†‡' — shared/unnumbered per report + isDeposit: boolean; // constituted-deposit row → inline Deposit badge, no marker isRefund: boolean; refundNoteText: string; // shown only when isRefund usageText: string; // EDITABLE — key `row..usageText` attachmentsNote: string | null; // EDITABLE when non-null — key `row..attachmentsNote`; null = no docs, omitted entirely + areaText: string | null; // read-only leaf area names, distinct comma-joined } export interface ReportContentSummaryRow { @@ -51,6 +53,7 @@ export interface ReportContentLabels { allocatedAmount: string; usage: string; attachmentsNote: string; + deposit: string; // translated in report language source: string; sourceType: string; reference: string; @@ -59,6 +62,7 @@ export interface ReportContentLabels { export interface ReportContent { isOverview: boolean; + isClaim: boolean; tableTitle: string; labels: ReportContentLabels; sourceInfo: { diff --git a/client/src/lib/reportPdf/coverLetterPdf.test.ts b/client/src/lib/reportPdf/coverLetterPdf.test.ts index f05eb83be..5e612ca03 100644 --- a/client/src/lib/reportPdf/coverLetterPdf.test.ts +++ b/client/src/lib/reportPdf/coverLetterPdf.test.ts @@ -38,6 +38,7 @@ function makeCoverLetter( function makeContent(overrides: Partial = {}): ReportContent { return { isOverview: false, + isClaim: false, tableTitle: 'Title', labels: { vendor: 'Vendor', @@ -48,6 +49,7 @@ function makeContent(overrides: Partial = {}): ReportContent { allocatedAmount: 'Allocated Amount', usage: 'Usage', attachmentsNote: 'Attachments Note', + deposit: 'Deposit', source: 'Source', sourceType: 'Source Type', reference: 'Reference', diff --git a/client/src/lib/reportPdf/merge.test.ts b/client/src/lib/reportPdf/merge.test.ts index 4e2d88290..0fb376fba 100644 --- a/client/src/lib/reportPdf/merge.test.ts +++ b/client/src/lib/reportPdf/merge.test.ts @@ -177,6 +177,7 @@ function makeReport(invoices: SourceReportInvoice[]): SourceReportResponse { function makeContent(overrides: Partial = {}): ReportContent { return { isOverview: false, + isClaim: false, tableTitle: 'Claim Report', labels: { vendor: 'Vendor', @@ -187,6 +188,7 @@ function makeContent(overrides: Partial = {}): ReportContent { allocatedAmount: 'Allocated Amount', usage: 'Usage', attachmentsNote: 'Attachments Note', + deposit: 'Deposit', source: 'Source', sourceType: 'Source Type', reference: 'Reference', diff --git a/client/src/lib/reportPdf/overviewPdf.test.ts b/client/src/lib/reportPdf/overviewPdf.test.ts index 8b16c9a6f..98d26bb53 100644 --- a/client/src/lib/reportPdf/overviewPdf.test.ts +++ b/client/src/lib/reportPdf/overviewPdf.test.ts @@ -34,10 +34,12 @@ function makeRow(overrides: Partial = {}): ReportContentRow { invoiceAmountText: '€1000.00', allocatedAmountValueText: '€1000.00', allocatedMarkers: '', + isDeposit: false, isRefund: false, refundNoteText: 'sourceReports.table.refundNote', usageText: '—', attachmentsNote: null, + areaText: null, ...overrides, }; } @@ -57,6 +59,7 @@ function makeLabels(): ReportContent['labels'] { allocatedAmount: 'sourceReports.table.allocatedAmount', usage: 'sourceReports.table.usage', attachmentsNote: 'sourceReports.editable.attachmentsNoteLabel', + deposit: 'sourceReports.table.attachmentType.deposit', source: 'sourceReports.table.source', sourceType: 'sourceReports.table.sourceType', reference: 'sourceReports.table.reference', @@ -67,6 +70,7 @@ function makeLabels(): ReportContent['labels'] { function makeContent(overrides: Partial = {}): ReportContent { return { isOverview: false, + isClaim: false, tableTitle: 'sourceReports.table.title.claim', labels: makeLabels(), sourceInfo: { @@ -84,9 +88,18 @@ function makeContent(overrides: Partial = {}): ReportContent { } // Flattens a pdfmake `table.body` row into plain text strings for easy assertions. Cells that are -// `stack`s (the Usage column when an attachment note is present) yield `undefined`. +// `stack`s (the Usage column when an attachment note or area text is present) yield `undefined`. +// The allocated-amount cell's `text` is always an array of runs (story #1923: the isDeposit inline +// label is a distinct, separately-styled run) — concatenate those runs' own `.text` values so +// existing plain-string assertions keep working; dedicated tests inspect the raw run array instead +// where the per-run styling itself is under test. function rowTexts(row: unknown): (string | undefined)[] { - return (row as { text?: string }[]).map((cell) => cell.text); + return (row as { text?: string | { text: string }[] }[]).map((cell) => { + if (Array.isArray(cell.text)) { + return cell.text.map((run) => run.text).join(''); + } + return cell.text; + }); } function getTable(content: unknown[]): { headerRows: number; widths: string[]; body: unknown[][] } { @@ -133,6 +146,22 @@ describe('buildOverviewContent — title and source info', () => { const infoStack = result[1] as { stack: { text: string }[] }; expect(infoStack.stack.find((s) => s.text.includes('REF-99'))).toBeDefined(); }); + + it('AC3.2: omits the sourceInfoStack entirely when isClaim is true — the table follows the title directly', () => { + const content = makeContent({ isClaim: true, rows: [] }); + const result = buildOverviewContent(content, new Map(), t); + expect(result).toHaveLength(2); // title + table only, no stack in between + const second = result[1] as unknown as Record; + expect(second.table).toBeDefined(); + expect(second.stack).toBeUndefined(); + }); + + it('renders the sourceInfoStack when isClaim is false (budget-overview/proof-of-funds), unchanged', () => { + const content = makeContent({ isClaim: false, rows: [] }); + const result = buildOverviewContent(content, new Map(), t); + const second = result[1] as unknown as Record; + expect(second.stack).toBeDefined(); + }); }); describe('buildOverviewContent — column layout', () => { @@ -276,6 +305,49 @@ describe('buildOverviewContent — row rendering (consumes already-derived Repor expect(cell.stack[0]!.text).toBe('Kitchen work'); expect(cell.stack[1]!.text).toBe('1 attachment: Invoice'); }); + + it('AC5.2: renders a stack with the area line (style "small") between usageText and attachmentsNote when areaText is present', () => { + const row = makeRow({ + usageText: 'Kitchen work', + areaText: 'Ground Floor', + attachmentsNote: '1 attachment: Invoice', + }); + const content = makeContent({ rows: [row] }); + const result = buildOverviewContent(content, new Map(), t); + const table = getTable(result); + const cell = (table.body[1] as unknown[])[5] as { + stack: { text: string; style?: string }[]; + }; + expect(cell.stack.map((s) => s.text)).toEqual([ + 'Kitchen work', + 'Ground Floor', + '1 attachment: Invoice', + ]); + expect(cell.stack[1]!.style).toBe('small'); + }); + + it('renders a stack with only [usageText, areaText] when areaText is present but attachmentsNote is null', () => { + const row = makeRow({ + usageText: 'Kitchen work', + areaText: 'Ground Floor', + attachmentsNote: null, + }); + const content = makeContent({ rows: [row] }); + const result = buildOverviewContent(content, new Map(), t); + const table = getTable(result); + const cell = (table.body[1] as unknown[])[5] as { stack: { text: string }[] }; + expect(cell.stack.map((s) => s.text)).toEqual(['Kitchen work', 'Ground Floor']); + }); + + 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); + const table = getTable(result); + const cell = (table.body[1] as unknown[])[5] as { text?: string; stack?: unknown }; + expect(cell.stack).toBeUndefined(); + expect(cell.text).toBe('Kitchen work'); + }); }); describe('allocated cell composition (skip markers + allocatedMarkers + refund note)', () => { @@ -287,25 +359,25 @@ describe('buildOverviewContent — row rendering (consumes already-derived Repor expect(rowTexts(table.body[1])[4]).toBe('€400.00'); }); - it('appends the pre-computed split/deposit markers verbatim (already formatted by buildReportContent)', () => { - const row = makeRow({ allocatedAmountValueText: '€400.00', allocatedMarkers: '†1‡1' }); + it('appends the pre-computed, unnumbered/shared split+deposit markers verbatim (already formatted by buildReportContent)', () => { + const row = makeRow({ allocatedAmountValueText: '€400.00', allocatedMarkers: '†‡' }); 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†1‡1'); + expect(rowTexts(table.body[1])[4]).toBe('€400.00†‡'); }); it('prepends skip-footnote markers (*N) BEFORE the allocatedMarkers, numbered from skippedDocuments', () => { const row = makeRow({ invoiceId: 'inv-1', allocatedAmountValueText: '€400.00', - allocatedMarkers: '†1', + allocatedMarkers: '†', }); 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†1'); + expect(rowTexts(table.body[1])[4]).toBe('€400.00*1†'); }); it('numbers multiple skip reasons on the same invoice sequentially', () => { @@ -319,6 +391,42 @@ describe('buildOverviewContent — row rendering (consumes already-derived Repor expect(rowTexts(table.body[1])[4]).toBe('€400.00*1*2'); }); }); + + describe('allocated cell: isDeposit inline label (AC2.1)', () => { + it('renders the allocated cell text as an array of runs when isDeposit is true', () => { + const row = makeRow({ allocatedAmountValueText: '€300.00', isDeposit: true }); + const content = makeContent({ rows: [row] }); + const result = buildOverviewContent(content, new Map(), t); + const table = getTable(result); + const cell = (table.body[1] as unknown[])[4] as { text: unknown }; + expect(Array.isArray(cell.text)).toBe(true); + }); + + it('the second run carries the deposit label, gray color and small fontSize', () => { + const row = makeRow({ allocatedAmountValueText: '€300.00', isDeposit: true }); + const content = makeContent({ rows: [row] }); + const result = buildOverviewContent(content, new Map(), t); + const table = getTable(result); + const cell = (table.body[1] as unknown[])[4] as { + text: { text: string; color?: string; fontSize?: number }[]; + }; + expect(cell.text[0]!.text).toBe('€300.00'); + const depositRun = cell.text[1]!; + expect(depositRun.color).toBe('#6b7280'); + expect(depositRun.fontSize).toBe(8); + expect(depositRun.text).toContain('sourceReports.table.attachmentType.deposit'); + }); + + it('renders exactly one run (no deposit run appended) when isDeposit is false', () => { + const row = makeRow({ allocatedAmountValueText: '€300.00', isDeposit: false }); + const content = makeContent({ rows: [row] }); + const result = buildOverviewContent(content, new Map(), t); + const table = getTable(result); + const cell = (table.body[1] as unknown[])[4] as { text: { text: string }[] }; + expect(cell.text).toHaveLength(1); + expect(cell.text[0]!.text).toBe('€300.00'); + }); + }); }); describe('buildOverviewContent — footnotes (skip block first, then reportContent.footnotes verbatim)', () => { diff --git a/client/src/lib/reportPdf/overviewPdf.ts b/client/src/lib/reportPdf/overviewPdf.ts index 63635b405..b2d43bfee 100644 --- a/client/src/lib/reportPdf/overviewPdf.ts +++ b/client/src/lib/reportPdf/overviewPdf.ts @@ -5,7 +5,12 @@ import type { TFunction } from 'i18next'; import type { Content } from 'pdfmake/build/pdfmake'; import type { ReportContent } from '../reportContent/index.js'; -import { TABLE_LAYOUT, REFUND_TEXT_COLOR } from './shared.js'; +import { + TABLE_LAYOUT, + REFUND_TEXT_COLOR, + DEPOSIT_NOTE_TEXT_COLOR, + DEPOSIT_NOTE_FONT_SIZE, +} from './shared.js'; export function buildOverviewContent( reportContent: ReportContent, @@ -21,32 +26,34 @@ export function buildOverviewContent( margin: [0, 0, 0, 20], }); - // Source info - const sourceInfoStack: Array = [ - { - text: `${reportContent.labels.source}: ${reportContent.sourceInfo.sourceName}`, - style: 'small', - }, - { - text: `${reportContent.labels.sourceType}: ${reportContent.sourceInfo.sourceTypeText}`, - style: 'small', - }, - reportContent.sourceInfo.referenceText - ? { - text: `${reportContent.labels.reference}: ${reportContent.sourceInfo.referenceText}`, - style: 'small', - } - : null, - { - text: `${reportContent.labels.generatedAt}: ${reportContent.sourceInfo.generatedAtText}`, - style: 'small', - }, - ]; + // Source info (skip for claim reports) + if (!reportContent.isClaim) { + const sourceInfoStack: Array = [ + { + text: `${reportContent.labels.source}: ${reportContent.sourceInfo.sourceName}`, + style: 'small', + }, + { + text: `${reportContent.labels.sourceType}: ${reportContent.sourceInfo.sourceTypeText}`, + style: 'small', + }, + reportContent.sourceInfo.referenceText + ? { + text: `${reportContent.labels.reference}: ${reportContent.sourceInfo.referenceText}`, + style: 'small', + } + : null, + { + text: `${reportContent.labels.generatedAt}: ${reportContent.sourceInfo.generatedAtText}`, + style: 'small', + }, + ]; - content.push({ - stack: sourceInfoStack.filter(Boolean) as Content[], - margin: [0, 0, 0, 20], - }); + content.push({ + stack: sourceInfoStack.filter(Boolean) as Content[], + margin: [0, 0, 0, 20], + }); + } // Build table columns const columns: Content[] = [ @@ -152,32 +159,41 @@ export function buildOverviewContent( } markerText += contentRow.allocatedMarkers; - const allocatedCell = `${contentRow.allocatedAmountValueText}${markerText}${contentRow.isRefund ? ' ' + contentRow.refundNoteText : ''}`; - - if (contentRow.isRefund) { - row.push({ - text: allocatedCell, - style: 'tableCell', - alignment: 'right', - color: REFUND_TEXT_COLOR, - }); - } else { - row.push({ - text: allocatedCell, - style: 'tableCell', - alignment: 'right', + // Build allocated runs: value+markers, then optional deposit badge, then optional refund note + const allocatedRuns: Content[] = [ + { text: `${contentRow.allocatedAmountValueText}${markerText}` }, + ]; + if (contentRow.isDeposit) { + allocatedRuns.push({ + text: ` (${reportContent.labels.deposit})`, + color: DEPOSIT_NOTE_TEXT_COLOR, + fontSize: DEPOSIT_NOTE_FONT_SIZE, }); } + if (contentRow.isRefund) { + allocatedRuns.push({ text: ` ${contentRow.refundNoteText}` }); + } + + row.push({ + text: allocatedRuns, + style: 'tableCell', + alignment: 'right', + color: contentRow.isRefund ? REFUND_TEXT_COLOR : undefined, + }); + + // Usage cell with optional area text and attachment note + const usageStack: Content[] = [{ text: contentRow.usageText, style: 'tableCell' }]; + if (contentRow.areaText) { + usageStack.push({ text: contentRow.areaText, style: 'small', margin: [0, 2, 0, 0] }); + } + if (contentRow.attachmentsNote) { + usageStack.push({ text: contentRow.attachmentsNote, style: 'small', margin: [0, 2, 0, 0] }); + } - // Usage cell with optional attachment note - const usageCell: Content = contentRow.attachmentsNote - ? { - stack: [ - { text: contentRow.usageText, style: 'tableCell' }, - { text: contentRow.attachmentsNote, style: 'small', margin: [0, 2, 0, 0] }, - ], - } - : { text: contentRow.usageText, style: 'tableCell' }; + const usageCell: Content = + usageStack.length > 1 + ? { stack: usageStack } + : { text: contentRow.usageText, style: 'tableCell' }; row.push(usageCell); rows.push(row); diff --git a/client/src/lib/reportPdf/realRender.test.ts b/client/src/lib/reportPdf/realRender.test.ts index aba7554bb..1f07d0c2b 100644 --- a/client/src/lib/reportPdf/realRender.test.ts +++ b/client/src/lib/reportPdf/realRender.test.ts @@ -444,13 +444,25 @@ describe('report PDF pipeline — real, unmocked end-to-end render', () => { id: 'bl-linked-1', description: null, allocatedPortion: 100, - linkedItem: { type: 'work_item', id: 'wi-1', name: 'Roof Replacement' }, + linkedItem: { + type: 'work_item', + id: 'wi-1', + name: 'Roof Replacement', + areaId: null, + areaName: null, + }, }, { id: 'bl-linked-2', description: null, allocatedPortion: 50, - linkedItem: { type: 'work_item', id: 'wi-1', name: 'Roof Replacement' }, + linkedItem: { + type: 'work_item', + id: 'wi-1', + name: 'Roof Replacement', + areaId: null, + areaName: null, + }, }, ], }); @@ -638,7 +650,11 @@ describe('report PDF pipeline — real, unmocked end-to-end render', () => { } }); - it('renders both real deposit-footnote wordings ("constituted" vs "reduced") in both locales', async () => { + // Story #1923: the "constituted" deposit case no longer produces a footnote at all — it + // renders as an inline, unnumbered Deposit label in the allocated cell (a second run, real + // i18next `sourceReports.table.attachmentType.deposit` text). Only the "reduced" case still + // produces a footnote, now shared/unnumbered (marker `‡`, no vendor/invoice-number prefix). + it('renders the real, unnumbered "constituted" Deposit inline label and the real, shared, unnumbered "reduced" footnote in both locales', async () => { const { buildOverviewContent } = await import('./overviewPdf.js'); const report = makeUsageFeatureReport(); const includedIds = new Set(report.invoices.map((inv) => inv.invoiceId)); @@ -648,18 +664,17 @@ describe('report PDF pipeline — real, unmocked end-to-end render', () => { tEn, formattersFor('en-US'), { - constituted: '‡1: Constituted Vendor (U-5) — This is a deposit.', - reduced: - '‡2: Reduced Vendor (U-6) — This position reflects deposits claimed separately.', + depositLabel: ' (Deposit)', + reducedFootnote: '‡: This position reflects deposits claimed separately.', }, ], [ tDe, formattersFor('de-DE'), { - constituted: '‡1: Constituted Vendor (U-5) — Dies ist eine Abschlagszahlung.', - reduced: - '‡2: Reduced Vendor (U-6) — Diese Position berücksichtigt separat eingereichte Abschlagszahlungen.', + depositLabel: ' (Abschlagszahlung)', + reducedFootnote: + '‡: Diese Position berücksichtigt separat eingereichte Abschlagszahlungen.', }, ], ] as const) { @@ -667,11 +682,34 @@ describe('report PDF pipeline — real, unmocked end-to-end render', () => { includeCoverLetter: false, household: null, }); + + // Sanity: the constituted-deposit row carries isDeposit=true and no ‡ marker; the + // reduced-deposit row carries the ‡ marker and isDeposit=false. + const constitutedRow = content.rows.find((r) => r.invoiceId === 'inv-deposit-constituted')!; + expect(constitutedRow.isDeposit).toBe(true); + expect(constitutedRow.allocatedMarkers).not.toContain('‡'); + const reducedRow = content.rows.find((r) => r.invoiceId === 'inv-deposit-reduced')!; + expect(reducedRow.isDeposit).toBe(false); + expect(reducedRow.allocatedMarkers).toContain('‡'); + const pdfContent = buildOverviewContent(content, new Map(), t); + const tableItem = pdfContent.find( + (c) => typeof c === 'object' && c !== null && 'table' in c, + ) as { table: { body: unknown[][] } }; + const constitutedRowCells = tableItem.table.body.find( + (row) => (row[0] as { text?: string })?.text === 'Constituted Vendor', + ) as { text: string | { text: string }[] }[]; + const allocatedCell = constitutedRowCells[4] as { text: { text: string }[] }; + expect(Array.isArray(allocatedCell.text)).toBe(true); + expect(allocatedCell.text[1]!.text).toBe(expected.depositLabel); + const notesStack = pdfContent[pdfContent.length - 1] as { stack: { text: string }[] }; const texts = notesStack.stack.map((n) => n.text); - expect(texts).toContain(expected.constituted); - expect(texts).toContain(expected.reduced); + expect(texts).toContain(expected.reducedFootnote); + // No "constituted" wording (footnote form) survives anywhere — it moved to the inline + // label above, and the deposit-constituted footnote key/string no longer exists at all. + expect(texts.some((text) => text.includes('This is a deposit'))).toBe(false); + expect(texts.some((text) => text.includes('Dies ist eine Abschlagszahlung'))).toBe(false); } }); }); @@ -854,4 +892,103 @@ describe('production i18n singleton — getFixedT resolves a language independen const fixedEn = i18n.getFixedT('en', 'budget'); expect(fixedEn('sourceReports.table.vendor')).toBe('Vendor'); }); + + // Story #1923 follow-up: the Deposit badge label moved into the shared content model + // (`ReportContentLabels.deposit`, built in buildReportContent.ts via the report-language + // `reportT`) so that ReportContentEditor.tsx and overviewPdf.ts both consume `labels.deposit` + // instead of independently calling a t() of their own. This pins that "report content never + // uses UI t" rule for this specific field: with the UI/ambient locale left at its default + // ('en', per the assertion above) and the REPORT language explicitly chosen as German — exactly + // ReportWizardPage's real `i18n.getFixedT(reportLanguage, 'budget')` construction, not the + // isolated test-only i18next instance used elsewhere in this file — `content.labels.deposit` + // must resolve the real German copy, and that same value must be what the PDF pipeline renders. + it('content.labels.deposit resolves via the report-language reportT (real "Abschlagszahlung"), independent of the UI locale staying English', async () => { + const i18n = (await import('../../i18n/index.js')).default; + expect(i18n.language).toBe('en'); // UI/ambient locale — untouched throughout this test + + const reportTDe = i18n.getFixedT('de', 'budget'); + // Minimal self-contained report: one invoice whose entire allocation is a deposit tagged to + // this source (the "constituted" case, AC2.1) — the only shape that triggers isDeposit=true. + const constitutedDepositInvoice = makeInvoice({ + invoiceId: 'inv-deposit-constituted', + vendorName: 'Constituted Vendor', + invoiceNumber: 'U-5', + isSplit: true, + invoiceAmount: 250, + allocatedAmount: 250, + budgetLines: [], + deposits: [ + { + id: 'dep-constituted', + amount: 250, + status: 'paid', + entryType: 'deposit', + dueDate: '2026-01-01', + paidDate: '2026-01-05', + claimedDate: null, + budgetSourceId: 'src-1', // tagged to THIS source -> "constituted" wording + }, + ], + }); + const report: SourceReportResponse = { + type: 'claim', + source: { + id: 'src-1', + name: 'Home Loan', + sourceType: 'bank_loan', + reference: null, + contactAddress: null, + }, + invoices: [constitutedDepositInvoice], + totalAmount: 250, + unallocatedInvoices: [], + generatedAt: '2026-02-15T00:00:00.000Z', + }; + const includedIds = new Set(report.invoices.map((inv) => inv.invoiceId)); + + const contentDe = buildReportContent( + report, + includedIds, + 'claim', + reportTDe, + formattersFor('de-DE'), + { + includeCoverLetter: false, + household: null, + }, + ); + expect(contentDe.labels.deposit).toBe('Abschlagszahlung'); + expect(i18n.language).toBe('en'); // still untouched — getFixedT never called changeLanguage() + + // Contrast: choosing English as the report language (independent of any UI concept) resolves + // the English copy — proving the field tracks whichever reportT was passed in, not a fixed + // value and not the UI locale. + const reportTEn = i18n.getFixedT('en', 'budget'); + const contentEn = buildReportContent( + report, + includedIds, + 'claim', + reportTEn, + formattersFor('en-US'), + { + includeCoverLetter: false, + household: null, + }, + ); + expect(contentEn.labels.deposit).toBe('Deposit'); + + // Pipeline pin: the rendered PDF's inline deposit run for the constituted-deposit row carries + // the same real German label sourced from content.labels.deposit — overviewPdf.ts never + // re-derives it from its own `t` parameter. + const { buildOverviewContent } = await import('./overviewPdf.js'); + const pdfContent = buildOverviewContent(contentDe, new Map(), reportTDe); + const tableItem = pdfContent.find( + (c) => typeof c === 'object' && c !== null && 'table' in c, + ) as { table: { body: unknown[][] } }; + const constitutedRowCells = tableItem.table.body.find( + (row) => (row[0] as { text?: string })?.text === 'Constituted Vendor', + ) as { text: string | { text: string }[] }[]; + const allocatedCell = constitutedRowCells[4] as { text: { text: string }[] }; + expect(allocatedCell.text[1]!.text).toBe(' (Abschlagszahlung)'); + }); }); diff --git a/client/src/lib/reportPdf/shared.ts b/client/src/lib/reportPdf/shared.ts index f64509214..58e1454e2 100644 --- a/client/src/lib/reportPdf/shared.ts +++ b/client/src/lib/reportPdf/shared.ts @@ -8,6 +8,16 @@ import type { Content } from 'pdfmake/build/pdfmake'; */ export const REFUND_TEXT_COLOR = '#991b1b'; +/** + * Deposit note text color for PDF tables (gray). + */ +export const DEPOSIT_NOTE_TEXT_COLOR = '#6b7280'; + +/** + * Deposit note font size for PDF tables (points). + */ +export const DEPOSIT_NOTE_FONT_SIZE = 8; + /** * Builds a page header for the PDF (title, source name, generated timestamp). */ diff --git a/e2e/pages/ReportWizardPage.ts b/e2e/pages/ReportWizardPage.ts index af80945ec..a940199bc 100644 --- a/e2e/pages/ReportWizardPage.ts +++ b/e2e/pages/ReportWizardPage.ts @@ -187,6 +187,29 @@ * generated content does not survive a discarded/confirmed upstream change any more than a * manual edit does, and the derived (#1898/#1900) baseline reasserts itself. * + * Story #1923: report table cleanup — shared footnotes, inline deposit label, claim metadata + * suppression, total-only summary, area in Usage. + * - `sourceInfoBlock` (declared above) is now conditionally rendered — `{!content.isClaim && ( + * ...)}` — and ABSENT from the DOM entirely for `claim` reports (AC3.1), not merely hidden. + * `budget-overview`/`proof-of-funds` reports are unaffected (still render it). + * - `allocatedMarkers` (the `†`/`‡` text appended after the Allocated Amount, still plain text — + * no dedicated locator, read via the row/card's own text) is now SHARED/unnumbered per report + * — at most one `†` (any split row) and one `‡` (any deposit-reduced row) — never `†1`/`†2`/ + * per-invoice numbering. `footnotesBlock`/`footnoteItems` (declared above) mirror this: at + * most 2 `
  • ` entries total, each `{marker}: {text}` with NO `Vendor (Invoice No.) — ` + * prefix (dropped — the note is no longer invoice-specific). + * - A constituted-deposit row (the allocation is made up entirely by a deposit tagged to the + * reported source) carries NO marker at all — instead an inline `Badge` (`depositBadge`/ + * `mobileDepositBadge` below) reading "Deposit"/"Abschlagszahlung". There is correspondingly + * no "This is a deposit." footnote anymore (`depositConstitutedFootnote` key removed). + * - `summaryTable`/`summaryTableRows` (declared above) now contains exactly ONE row — `Total`/ + * `Gesamt` — regardless of how many distinct invoice statuses are included; the old + * per-status `Outstanding`/`Paid`/`Quotation`/`Claimed Subtotal` rows are gone + * (`sourceReports.table.subtotal` key removed). + * - `usageAreaText`/`mobileUsageAreaText` below: a read-only leaf-area sub-line rendered under + * a row's Usage field, only when the linked item(s) resolve to an area — see the method's own + * docstring for why it can never be silently dropped by AI-generated usage text. + * * Back/Next button locators (`step2BackButton`/`step2NextButton`/`step4BackButton`/ * `step4NextButton`/`step5BackButton`, etc.): every step body is rendered from a single * `{currentStep === N && ...}` block, so exactly ONE `[class*="buttonRow"]` div is ever present @@ -349,6 +372,16 @@ export class ReportWizardPage { // follow-up bug). readonly mobileCardList: Locator; + // Story #1923: report table cleanup — total-only summary table (`.summaryTable`, a sibling + // `` OUTSIDE `.tableWrapper` — see `contentTable`'s own docstring for why that scoping + // avoids a substring collision) and the shared footnotes block (`.footnotes`, now at most 2 + // `
  • ` entries — one per marker (`†`/`‡`) — never one per invoice; see `depositBadge`'s + // docstring below for the constituted-deposit case, which carries NO marker at all). + readonly summaryTable: Locator; + readonly summaryTableRows: Locator; + readonly footnotesBlock: Locator; + readonly footnoteItems: Locator; + // Claim confirm modal readonly claimConfirmModal: Locator; readonly claimConfirmModalBody: Locator; @@ -475,6 +508,12 @@ export class ReportWizardPage { this.contentTable = page.locator('[class*="tableWrapper"] table'); this.mobileCardList = page.locator('[class*="mobileCardList"]'); + // Story #1923. + this.summaryTable = page.locator('[class*="summaryTable"]'); + this.summaryTableRows = this.summaryTable.locator('tbody tr'); + this.footnotesBlock = page.locator('[class*="footnotes"]'); + this.footnoteItems = this.footnotesBlock.locator('li'); + this.claimConfirmModal = page.getByRole('dialog', { name: 'Mark Invoices as Claimed?' }); this.claimConfirmModalBody = this.claimConfirmModal.locator('p'); this.claimConfirmConfirmButton = this.claimConfirmModal.locator('[class*="btnPrimary"]'); @@ -987,6 +1026,43 @@ export class ReportWizardPage { return this.mobileCard(vendorName, invoiceNumber).getByLabel('Usage', { exact: true }); } + // ─── Story #1923: report table cleanup ─────────────────────────────────────────────────── + + /** + * The inline "Deposit" `Badge` (`[class*="depositBadge"]`, composed from the shared + * `.attachmentDeposit` variant) rendered in a desktop content-table row's Allocated Amount + * cell when `row.isDeposit` — a constituted-deposit row (AC2.1), i.e. the row's allocation is + * made up entirely by a deposit tagged to the CURRENTLY reported source. Carries NO `†`/`‡` + * marker of its own. Scoped to the row so it never collides with `mobileDepositBadge`'s copy + * (both trees share the `depositBadge` class and are always in the DOM simultaneously, per + * the class docstring's dual-DOM-tree convention). + */ + depositBadge(vendorName: string, invoiceNumber: string): Locator { + return this.contentTableRow(vendorName, invoiceNumber).locator('[class*="depositBadge"]'); + } + + /** The same inline "Deposit" badge within a mobile card's Allocated Amount value (AC6.2). */ + mobileDepositBadge(vendorName: string, invoiceNumber: string): Locator { + return this.mobileCard(vendorName, invoiceNumber).locator('[class*="depositBadge"]'); + } + + /** + * The read-only, non-editable leaf-area sub-line (`[class*="usageAreaText"]`) rendered below + * a desktop row's `usageText` field when `row.areaText` is non-null (AC5.2) — distinct + * comma-joined leaf area names, never concatenated into the editable `usageText` string + * itself (AC5.3), so it survives both manual edits and AI-generated usage text overwriting + * `usageText` (`applyAiContent.ts` only ever assigns `row.usageText`, never `row.areaText`). + * Absent entirely (not an empty element) when the row has no area (AC5.4). + */ + usageAreaText(vendorName: string, invoiceNumber: string): Locator { + return this.contentTableRow(vendorName, invoiceNumber).locator('[class*="usageAreaText"]'); + } + + /** The same area sub-line within a mobile card's Usage field (AC6.2). */ + mobileUsageAreaText(vendorName: string, invoiceNumber: string): Locator { + return this.mobileCard(vendorName, invoiceNumber).locator('[class*="usageAreaText"]'); + } + /** Fills an `EditableField` (input or textarea) with `value`, firing its `onChange`. */ async editField(field: Locator, value: string): Promise { await field.fill(value); diff --git a/e2e/tests/budget/reportWizardAiGeneration.spec.ts b/e2e/tests/budget/reportWizardAiGeneration.spec.ts index bd3504d24..4f87eb5fa 100644 --- a/e2e/tests/budget/reportWizardAiGeneration.spec.ts +++ b/e2e/tests/budget/reportWizardAiGeneration.spec.ts @@ -38,6 +38,10 @@ * - Scenario 7: A confirmed Step 1-4 change (via the existing discard-confirm modal) clears * previously-generated AI content, same as it clears manual overrides — the fields revert to * the plain derived (#1898/#1900) baseline and the provenance note disappears. + * - Scenario 8 (Story #1923 AC5.3): the read-only area sub-line under a row's Usage field + * survives AI generation — `applyAiContent.ts` only ever assigns `row.usageText`, never + * `row.areaText`, so a row whose linked item has an assigned area keeps showing that area + * after "Generate with AI" overwrites the usage text itself. */ import { test, expect } from '../../fixtures/auth.js'; @@ -51,6 +55,8 @@ import { deleteBudgetSourceViaApi, createWorkItemViaApi, deleteWorkItemViaApi, + createAreaViaApi, + deleteAreaViaApi, } from '../../fixtures/apiHelpers.js'; // ───────────────────────────────────────────────────────────────────────────── @@ -720,3 +726,78 @@ test.describe('Report wizard AI generation — discard clears AI content (Scenar } }); }); + +// ───────────────────────────────────────────────────────────────────────────── +// Scenario 8: The area sub-line survives AI generation (Story #1923 AC5.3) +// ───────────────────────────────────────────────────────────────────────────── + +test.describe('Report wizard AI generation — area sub-line survives generation (Scenario 8)', () => { + test('A row\'s read-only area sub-line is still present after "Generate with AI" overwrites the usage text, because applyAiContent only ever assigns usageText, never areaText', async ({ + page, + testPrefix, + }) => { + test.slow(); + await mockLlmEnabled(page); + const wizard = new ReportWizardPage(page); + + let vendorId = ''; + let sourceId = ''; + let areaId = ''; + let workItemId = ''; + try { + vendorId = await createVendorViaApi(page, { name: `${testPrefix} AiArea Vendor` }); + sourceId = await createBudgetSourceViaApi(page, { + name: `${testPrefix} AiArea Source`, + totalAmount: 10000, + contactAddress: '1 AiArea St, Testville', + reference: 'Ref-AIAREA', + }); + areaId = await createAreaViaApi(page, { name: `${testPrefix} Bathroom` }); + workItemId = await createWorkItemViaApi(page, { title: `${testPrefix} WI AiArea`, areaId }); + const invoice = await seedAllocatedInvoice(page, workItemId, vendorId, sourceId, { + invoiceNumber: `${testPrefix}-AIAREA-001`, + amount: 210, + date: '2026-07-09', + status: 'pending', + }); + + const counter = createCallCounter(); + await mockGenerateContentImmediate( + page, + { + letterSubject: 'AI Area Subject', + letterBody: 'AI area cover letter body.', + descriptions: { [invoice.id]: 'AI-generated usage description overwriting the row' }, + }, + counter, + ); + + const vendorName = `${testPrefix} AiArea Vendor`; + await reachStep5WithAiEnabled(wizard, sourceId); + const usage = wizard.usageField(vendorName, invoice.invoiceNumber!); + const areaLine = wizard.usageAreaText(vendorName, invoice.invoiceNumber!); + const areaName = `${testPrefix} Bathroom`; + + // Before generation: the derived baseline usage text (linked item name) alongside the + // area sub-line. + await expect(areaLine).toBeVisible(); + await expect(areaLine).toHaveText(areaName); + + await wizard.clickGenerateWithAi(); + await expect(usage).toHaveValue('AI-generated usage description overwriting the row'); + await expect(wizard.aiGeneratedNote).toBeVisible(); + + // After generation: usageText was overwritten by the AI description, but the area + // sub-line — a separate, non-editable field never touched by `applyAiContent` — is + // unchanged (AC5.3). + await expect(areaLine).toBeVisible(); + await expect(areaLine).toHaveText(areaName); + expect(counter.count).toBe(1); + } finally { + if (workItemId) await deleteWorkItemViaApi(page, workItemId); + if (areaId) await deleteAreaViaApi(page, areaId); + if (sourceId) await deleteBudgetSourceViaApi(page, sourceId); + if (vendorId) await deleteVendorViaApi(page, vendorId); + } + }); +}); diff --git a/e2e/tests/budget/reportWizardEditableContent.spec.ts b/e2e/tests/budget/reportWizardEditableContent.spec.ts index ff715dcc5..c2a134137 100644 --- a/e2e/tests/budget/reportWizardEditableContent.spec.ts +++ b/e2e/tests/budget/reportWizardEditableContent.spec.ts @@ -13,7 +13,8 @@ * hardening (Story #1891). THIS file is scoped to the NEW editable-content behavior only: * * - Scenario 1: The step-5 surface is live editable inputs, not an iframe — no PDF generation - * happens just by arriving on step 5. + * happens just by arriving on step 5. Story #1923: also doubles as the AC3.3 non-claim + * regression guard for the source-info metadata block (uses `budget-overview`, not `claim`). * - Scenario 1b: Desktop — regression guard for #1908 (fixed): the mobile-card fallback (added * for #1904) must stay hidden at desktop width. It previously lacked a base `display: none` * and duplicated the table; the fix landed in `ReportContentEditor.module.css`. @@ -63,6 +64,25 @@ * attach-documents/cover-letter checkboxes on the same step (previously it bypassed the * guard and applied silently). * + * Story #1923 (report table cleanup): shared footnotes, inline deposit label, claim metadata + * suppression, total-only summary, area in the Usage cell. See `ReportWizardPage.ts`'s class + * docstring for the full locator reference (`sourceInfoBlock`, `depositBadge`/ + * `mobileDepositBadge`, `usageAreaText`/`mobileUsageAreaText`, `summaryTable`/ + * `summaryTableRows`, `footnotesBlock`/`footnoteItems`). + * - Scenario 16: A `claim` report omits the source-info metadata block entirely (AC3.1) — the + * counterpart to Scenario 1's `budget-overview` regression guard (AC3.3). + * - Scenario 17: A constituted-deposit row (the row's allocation is made up entirely by a + * deposit tagged to the currently reported source) shows the inline "Deposit" badge on + * desktop, tablet, AND mobile, carries no `‡` marker, and the footnotes list has no entry at + * all for it (AC2.1, AC2.2). + * - Scenario 18: Two or more split invoices share exactly ONE unnumbered `†` marker and exactly + * one footnote entry, with no vendor/invoice-number prefix (AC1.1-AC1.2). + * - Scenario 19: Invoices spanning two or more statuses still produce exactly one summary row + * (`Total`) — no per-status subtotal rows (AC4.1-AC4.2). + * - Scenario 20: A budget line linked to an item with an assigned area shows the item's leaf + * area name as a distinct, read-only line below the Usage field on desktop, tablet, AND + * mobile; an item with no area renders no area line and no empty gap (AC5.2, AC5.4, AC5.5). + * * PDF generation (pdfmake + pdf-lib via dynamic `import()`) can be slow, especially on a cold * chunk load — every scenario that opens the preview modal, downloads, or uploads uses * `test.slow()`. @@ -71,7 +91,7 @@ import { test, expect } from '../../fixtures/auth.js'; import { statSync } from 'node:fs'; import type { Page } from '@playwright/test'; -import { ReportWizardPage } from '../../pages/ReportWizardPage.js'; +import { ReportWizardPage, type SourceReportUseCase } from '../../pages/ReportWizardPage.js'; import { API } from '../../fixtures/testData.js'; import { createVendorViaApi, @@ -80,6 +100,8 @@ import { deleteBudgetSourceViaApi, createWorkItemViaApi, deleteWorkItemViaApi, + createAreaViaApi, + deleteAreaViaApi, } from '../../fixtures/apiHelpers.js'; // ───────────────────────────────────────────────────────────────────────────── @@ -229,10 +251,86 @@ async function linkDocumentToInvoiceViaApi( expect(response.ok(), `POST document-link failed: ${response.status()}`).toBeTruthy(); } -/** Walks a fresh wizard through steps 1-4 (claim, single source) to land on step 5. */ -async function reachStep5(wizard: ReportWizardPage, sourceId: string): Promise { +/** + * Creates a deposit on `invoiceId`, optionally tagged to a budget source + * (`data.budgetSourceId`) — mirrors the established pattern in `reportWizardExpansion.spec.ts` + * (Story #1891/#1895/#1896). Used by Scenario 17 to construct a constituted-deposit row (Story + * #1923 AC2.1). + */ +async function createDepositViaApi( + page: Page, + invoiceId: string, + data: { + amount: number; + dueDate: string; + status?: 'pending' | 'paid' | 'claimed'; + entryType?: 'deposit' | 'refund'; + budgetSourceId?: string | null; + }, +): Promise<{ id: string }> { + const response = await page.request.post(`/api/invoices/${invoiceId}/deposits`, { + data: { status: 'pending', ...data }, + }); + expect(response.ok(), `POST deposit failed: ${response.status()}`).toBeTruthy(); + const body = (await response.json()) as { deposit: { id: string } }; + return body.deposit; +} + +/** + * Creates an invoice whose funding is SPLIT across two distinct budget sources via two separate + * budget lines (one work item per source) — the server's `isSplit` flag + * (`sourceReportService.ts`) is true whenever an invoice's funding spans 2+ distinct budget + * sources across budget lines and tagged deposits, so this genuinely produces a `†`-marked row + * (not just a multi-line invoice within a single source, which `seedInvoiceWithTwoLines` above + * produces and does NOT mark). Used by Scenario 18 (Story #1923 AC1). + */ +async function seedSplitInvoice( + page: Page, + vendorId: string, + reportedSourceId: string, + otherSourceId: string, + reportedWorkItemId: string, + otherWorkItemId: string, + data: { invoiceNumber: string; date: string; status: 'pending' | 'paid' | 'claimed' }, + reportedAmount: number, + otherAmount: number, +): Promise { + const invoice = await createInvoiceViaApi(page, vendorId, { + invoiceNumber: data.invoiceNumber, + date: data.date, + status: data.status, + amount: reportedAmount + otherAmount, + }); + const reportedBudgetId = await createWorkItemBudgetViaApi(page, reportedWorkItemId, { + plannedAmount: reportedAmount, + budgetSourceId: reportedSourceId, + }); + await linkInvoiceToBudgetLineViaApi(page, invoice.id, { + workItemBudgetId: reportedBudgetId, + itemizedAmount: reportedAmount, + }); + const otherBudgetId = await createWorkItemBudgetViaApi(page, otherWorkItemId, { + plannedAmount: otherAmount, + budgetSourceId: otherSourceId, + }); + await linkInvoiceToBudgetLineViaApi(page, invoice.id, { + workItemBudgetId: otherBudgetId, + itemizedAmount: otherAmount, + }); + return invoice; +} + +/** + * Walks a fresh wizard through steps 1-4 (single source, `useCase` defaults to `claim`) to land + * on step 5. + */ +async function reachStep5( + wizard: ReportWizardPage, + sourceId: string, + useCase: SourceReportUseCase = 'claim', +): Promise { await wizard.goto(); - await wizard.selectUseCase('claim'); + await wizard.selectUseCase(useCase); await wizard.goNextFromStep1(); await wizard.selectSource(sourceId); await wizard.goNextFromStep2(); @@ -270,7 +368,11 @@ test.describe('Report wizard editable content — live surface, no auto-generati status: 'pending', }); - await reachStep5(wizard, sourceId); + // Story #1923 AC3.3 regression guard: `budget-overview` (a non-claim use case) is + // deliberately used here (rather than the file's usual `claim` default) so this scenario + // doubles as proof the metadata block still renders for non-claim reports — see Scenario + // 16 below for the claim counterpart (AC3.1: the block is entirely ABSENT for `claim`). + await reachStep5(wizard, sourceId, 'budget-overview'); // Live editable content is visible immediately — no generation, no loading state. await expect(wizard.letterField('subject')).toBeVisible(); @@ -1328,3 +1430,378 @@ test.describe('Report wizard editable content — report-language guard (Scenari } }); }); + +// ───────────────────────────────────────────────────────────────────────────── +// Scenario 16: Claim reports omit the metadata block (Story #1923 AC3.1) +// ───────────────────────────────────────────────────────────────────────────── + +test.describe('Report wizard editable content — claim reports omit the metadata block (Scenario 16)', () => { + test('A claim report hides the source-info metadata block entirely, while the title, table, summary, and (when present) footnotes still render (Story #1923 AC3.1, AC3.4)', async ({ + page, + testPrefix, + }) => { + const wizard = new ReportWizardPage(page); + + let vendorId = ''; + let sourceId = ''; + let workItemId = ''; + try { + vendorId = await createVendorViaApi(page, { name: `${testPrefix} ClaimMeta Vendor` }); + sourceId = await createBudgetSourceViaApi(page, { + name: `${testPrefix} ClaimMeta Source`, + totalAmount: 10000, + reference: 'Ref-CLAIMMETA', + }); + workItemId = await createWorkItemViaApi(page, { title: `${testPrefix} WI ClaimMeta` }); + const invoice = await seedAllocatedInvoice(page, workItemId, vendorId, sourceId, { + invoiceNumber: `${testPrefix}-CLAIMMETA-001`, + amount: 300, + date: '2026-06-05', + status: 'pending', + }); + + // `reachStep5` defaults to `useCase: 'claim'`. + await reachStep5(wizard, sourceId); + + // The block is entirely absent from the DOM (not merely hidden) — `toHaveCount(0)`, + // not `not.toBeVisible()`, proves the `{!content.isClaim && (...)}` conditional actually + // omits the render, matching the ux-designer spec's "no placeholder, no ghost block" note. + await expect(wizard.sourceInfoBlock).toHaveCount(0); + + // The rest of step 5 is unaffected: title, table, and summary still render normally. + await expect(wizard.contentTable).toBeVisible(); + const vendorName = `${testPrefix} ClaimMeta Vendor`; + await expect(wizard.contentTableRow(vendorName, invoice.invoiceNumber!)).toBeVisible(); + await expect(wizard.summaryTable).toBeVisible(); + await expect(wizard.summaryTableRows).toHaveCount(1); + } finally { + if (workItemId) await deleteWorkItemViaApi(page, workItemId); + if (sourceId) await deleteBudgetSourceViaApi(page, sourceId); + if (vendorId) await deleteVendorViaApi(page, vendorId); + } + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// Scenario 17: Constituted-deposit row shows the inline Deposit badge, no ‡ marker, no footnote +// (Story #1923 AC2.1, AC2.2) +// ───────────────────────────────────────────────────────────────────────────── + +test.describe( + 'Report wizard editable content — inline deposit badge (Scenario 17)', + { tag: '@responsive' }, + () => { + test('A row whose allocation is made up entirely by a deposit tagged to the reported source shows an inline "Deposit" badge instead of a ‡ marker, with no matching footnote entry, on desktop, tablet, and mobile', async ({ + page, + testPrefix, + }) => { + const wizard = new ReportWizardPage(page); + + let vendorId = ''; + // Source A holds the invoice's own budget line; source B only ever gets contribution via + // the tagged deposit — mirrors `reportWizardExpansion.spec.ts` Scenario 5's "zero-line + // source, surfaced via the tagged deposit" shape, viewed from source B's own report. + let sourceAId = ''; + let sourceBId = ''; + let workItemId = ''; + try { + vendorId = await createVendorViaApi(page, { name: `${testPrefix} Deposit Vendor` }); + sourceAId = await createBudgetSourceViaApi(page, { + name: `${testPrefix} Deposit Source A`, + totalAmount: 10000, + }); + sourceBId = await createBudgetSourceViaApi(page, { + name: `${testPrefix} Deposit Source B`, + totalAmount: 10000, + }); + workItemId = await createWorkItemViaApi(page, { title: `${testPrefix} WI Deposit` }); + + const invoice = await seedAllocatedInvoice(page, workItemId, vendorId, sourceAId, { + invoiceNumber: `${testPrefix}-DEPOSIT-001`, + amount: 1000, + date: '2026-06-08', + status: 'paid', + }); + await createDepositViaApi(page, invoice.id, { + amount: 150, + dueDate: '2026-06-12', + status: 'paid', + entryType: 'deposit', + budgetSourceId: sourceBId, + }); + + // View source B's own claim report — this invoice has zero budget lines for B, so its + // entire row here is the tagged deposit: isSplit (spans A + B) with an empty + // `budgetLines` slice for B → constituted-deposit, no † (no budget-line split for B) + // and no ‡ (the deposit IS tagged to B, not "reduced"). + await reachStep5(wizard, sourceBId); + + const vendorName = `${testPrefix} Deposit Vendor`; + const isMobile = test.info().project.name === 'mobile'; + + if (isMobile) { + await expect(wizard.mobileCardList).toBeVisible(); + const card = wizard.mobileCard(vendorName, invoice.invoiceNumber!); + await expect(card).toBeVisible(); + const badge = wizard.mobileDepositBadge(vendorName, invoice.invoiceNumber!); + await expect(badge).toBeVisible(); + await expect(badge).toHaveText('Deposit'); + const cardText = (await card.textContent()) ?? ''; + expect(cardText).not.toContain('‡'); + } else { + await expect(wizard.contentTable).toBeVisible(); + const row = wizard.contentTableRow(vendorName, invoice.invoiceNumber!); + await expect(row).toBeVisible(); + const badge = wizard.depositBadge(vendorName, invoice.invoiceNumber!); + await expect(badge).toBeVisible(); + await expect(badge).toHaveText('Deposit'); + const rowText = (await row.textContent()) ?? ''; + expect(rowText).not.toContain('‡'); + } + + // No footnote entry at all — no split (B has zero budget lines here), no + // deposit-reduced (the deposit IS tagged to B, not "reduced"), and the removed + // `depositConstitutedFootnote` key ("This is a deposit.") never had a footnote to begin + // with even before this story. + await expect(wizard.footnotesBlock).not.toBeVisible(); + } finally { + if (workItemId) await deleteWorkItemViaApi(page, workItemId); + if (sourceBId) await deleteBudgetSourceViaApi(page, sourceBId); + if (sourceAId) await deleteBudgetSourceViaApi(page, sourceAId); + if (vendorId) await deleteVendorViaApi(page, vendorId); + } + }); + }, +); + +// ───────────────────────────────────────────────────────────────────────────── +// Scenario 18: Two or more split invoices share ONE unnumbered † footnote (Story #1923 AC1) +// ───────────────────────────────────────────────────────────────────────────── + +test.describe('Report wizard editable content — shared split footnote (Scenario 18)', () => { + test('Two split invoices both carry an unnumbered † marker, and the footnote list has exactly one † entry with no vendor/invoice-number prefix', async ({ + page, + testPrefix, + }) => { + const wizard = new ReportWizardPage(page); + + let vendorId = ''; + let reportedSourceId = ''; + let otherSourceId = ''; + let reportedWorkItemId = ''; + let otherWorkItemId = ''; + try { + vendorId = await createVendorViaApi(page, { name: `${testPrefix} Split Vendor` }); + reportedSourceId = await createBudgetSourceViaApi(page, { + name: `${testPrefix} Split Source A`, + totalAmount: 10000, + }); + otherSourceId = await createBudgetSourceViaApi(page, { + name: `${testPrefix} Split Source B`, + totalAmount: 10000, + }); + reportedWorkItemId = await createWorkItemViaApi(page, { title: `${testPrefix} WI Split A` }); + otherWorkItemId = await createWorkItemViaApi(page, { title: `${testPrefix} WI Split B` }); + + const invoice1 = await seedSplitInvoice( + page, + vendorId, + reportedSourceId, + otherSourceId, + reportedWorkItemId, + otherWorkItemId, + { invoiceNumber: `${testPrefix}-SPLIT-001`, date: '2026-06-10', status: 'pending' }, + 100, + 200, + ); + const invoice2 = await seedSplitInvoice( + page, + vendorId, + reportedSourceId, + otherSourceId, + reportedWorkItemId, + otherWorkItemId, + { invoiceNumber: `${testPrefix}-SPLIT-002`, date: '2026-06-11', status: 'paid' }, + 150, + 250, + ); + + await reachStep5(wizard, reportedSourceId); + + const vendorName = `${testPrefix} Split Vendor`; + const row1 = wizard.contentTableRow(vendorName, invoice1.invoiceNumber!); + const row2 = wizard.contentTableRow(vendorName, invoice2.invoiceNumber!); + await expect(row1).toContainText('†'); + await expect(row2).toContainText('†'); + + // Unnumbered — no digit ever directly follows the marker glyph. + const row1Text = (await row1.textContent()) ?? ''; + const row2Text = (await row2.textContent()) ?? ''; + expect(row1Text).not.toMatch(/†\d/); + expect(row2Text).not.toMatch(/†\d/); + + // Exactly one shared footnote entry, no per-invoice prefix. + await expect(wizard.footnoteItems).toHaveCount(1); + const footnoteText = (await wizard.footnoteItems.first().textContent()) ?? ''; + expect(footnoteText).toMatch(/^†:/); + expect(footnoteText).not.toContain(vendorName); + expect(footnoteText).not.toContain(invoice1.invoiceNumber!); + expect(footnoteText).not.toContain(invoice2.invoiceNumber!); + expect(footnoteText).toContain( + 'Amount shown reflects only the portion allocated to this source.', + ); + } finally { + if (reportedWorkItemId) await deleteWorkItemViaApi(page, reportedWorkItemId); + if (otherWorkItemId) await deleteWorkItemViaApi(page, otherWorkItemId); + if (reportedSourceId) await deleteBudgetSourceViaApi(page, reportedSourceId); + if (otherSourceId) await deleteBudgetSourceViaApi(page, otherSourceId); + if (vendorId) await deleteVendorViaApi(page, vendorId); + } + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// Scenario 19: Summary shows only the Total row, even across multiple statuses +// (Story #1923 AC4) +// ───────────────────────────────────────────────────────────────────────────── + +test.describe('Report wizard editable content — total-only summary (Scenario 19)', () => { + test('Invoices spanning two statuses (pending + paid) still produce exactly one summary row — Total — with no per-status Subtotal rows', async ({ + page, + testPrefix, + }) => { + const wizard = new ReportWizardPage(page); + + let vendorId = ''; + let sourceId = ''; + let workItemId = ''; + try { + vendorId = await createVendorViaApi(page, { name: `${testPrefix} Summary Vendor` }); + sourceId = await createBudgetSourceViaApi(page, { + name: `${testPrefix} Summary Source`, + totalAmount: 10000, + }); + workItemId = await createWorkItemViaApi(page, { title: `${testPrefix} WI Summary` }); + await seedAllocatedInvoice(page, workItemId, vendorId, sourceId, { + invoiceNumber: `${testPrefix}-SUMMARY-001`, + amount: 300, + date: '2026-06-13', + status: 'pending', + }); + await seedAllocatedInvoice(page, workItemId, vendorId, sourceId, { + invoiceNumber: `${testPrefix}-SUMMARY-002`, + amount: 450, + date: '2026-06-14', + status: 'paid', + }); + + // `reachStep5` defaults to `useCase: 'claim'` — pending + paid is already 2+ statuses. + await reachStep5(wizard, sourceId); + + await expect(wizard.summaryTable).toBeVisible(); + await expect(wizard.summaryTableRows).toHaveCount(1); + await expect(wizard.summaryTableRows.first()).toContainText('Total'); + // Sum of both invoices' allocated amounts (300 + 450 = 750). + await expect(wizard.summaryTableRows.first()).toContainText('750'); + + const summaryText = (await wizard.summaryTable.textContent()) ?? ''; + expect(summaryText).not.toContain('Subtotal'); + expect(summaryText).not.toContain('Outstanding'); + expect(summaryText).not.toContain('Quotation'); + expect(summaryText).not.toContain('Claimed'); + } finally { + if (workItemId) await deleteWorkItemViaApi(page, workItemId); + if (sourceId) await deleteBudgetSourceViaApi(page, sourceId); + if (vendorId) await deleteVendorViaApi(page, vendorId); + } + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// Scenario 20: Area sub-line in the Usage cell (Story #1923 AC5) +// ───────────────────────────────────────────────────────────────────────────── + +test.describe( + 'Report wizard editable content — area sub-line in Usage cell (Scenario 20)', + { tag: '@responsive' }, + () => { + test('A budget line linked to an item with an assigned area renders the leaf area name as a distinct, read-only line below Usage text (desktop, tablet, mobile); an item with no area renders no area line at all', async ({ + page, + testPrefix, + }) => { + const wizard = new ReportWizardPage(page); + + let vendorId = ''; + let sourceId = ''; + let areaId = ''; + let workItemWithAreaId = ''; + let workItemNoAreaId = ''; + try { + vendorId = await createVendorViaApi(page, { name: `${testPrefix} Area Vendor` }); + sourceId = await createBudgetSourceViaApi(page, { + name: `${testPrefix} Area Source`, + totalAmount: 10000, + }); + areaId = await createAreaViaApi(page, { name: `${testPrefix} Kitchen` }); + workItemWithAreaId = await createWorkItemViaApi(page, { + title: `${testPrefix} WI HasArea`, + areaId, + }); + workItemNoAreaId = await createWorkItemViaApi(page, { title: `${testPrefix} WI NoArea` }); + + const invoiceWithArea = await seedAllocatedInvoice( + page, + workItemWithAreaId, + vendorId, + sourceId, + { + invoiceNumber: `${testPrefix}-AREA-001`, + amount: 120, + date: '2026-06-20', + status: 'pending', + }, + ); + const invoiceNoArea = await seedAllocatedInvoice( + page, + workItemNoAreaId, + vendorId, + sourceId, + { + invoiceNumber: `${testPrefix}-AREA-002`, + amount: 130, + date: '2026-06-21', + status: 'pending', + }, + ); + + await reachStep5(wizard, sourceId); + + const vendorName = `${testPrefix} Area Vendor`; + const areaName = `${testPrefix} Kitchen`; + const isMobile = test.info().project.name === 'mobile'; + + if (isMobile) { + const areaLine = wizard.mobileUsageAreaText(vendorName, invoiceWithArea.invoiceNumber!); + await expect(areaLine).toBeVisible(); + await expect(areaLine).toHaveText(areaName); + await expect( + wizard.mobileUsageAreaText(vendorName, invoiceNoArea.invoiceNumber!), + ).toHaveCount(0); + } else { + const areaLine = wizard.usageAreaText(vendorName, invoiceWithArea.invoiceNumber!); + await expect(areaLine).toBeVisible(); + await expect(areaLine).toHaveText(areaName); + await expect(wizard.usageAreaText(vendorName, invoiceNoArea.invoiceNumber!)).toHaveCount( + 0, + ); + } + } finally { + if (workItemWithAreaId) await deleteWorkItemViaApi(page, workItemWithAreaId); + if (workItemNoAreaId) await deleteWorkItemViaApi(page, workItemNoAreaId); + if (areaId) await deleteAreaViaApi(page, areaId); + if (sourceId) await deleteBudgetSourceViaApi(page, sourceId); + if (vendorId) await deleteVendorViaApi(page, vendorId); + } + }); + }, +); diff --git a/server/src/services/sourceReportService.test.ts b/server/src/services/sourceReportService.test.ts index 0c045a7cd..8a8d4ae99 100644 --- a/server/src/services/sourceReportService.test.ts +++ b/server/src/services/sourceReportService.test.ts @@ -219,6 +219,24 @@ describe('sourceReportService', () => { return db.select().from(schema.diaryEntries).all().length; } + /** Insert an area (Story #1923 AC5), returns its id. Migration seeds no default areas. */ + function insertArea(name: string, parentId: string | null = null): string { + const id = `area-${++counter}`; + const now = ts(); + db.insert(schema.areas) + .values({ + id, + name, + parentId, + color: null, + sortOrder: counter, + createdAt: now, + updatedAt: now, + }) + .run(); + return id; + } + // ═══════════════════════════════════════════════════════════════════════ // getSourceReport // ═══════════════════════════════════════════════════════════════════════ @@ -817,7 +835,13 @@ describe('sourceReportService', () => { const line = result.invoices[0]!.budgetLines[0]!; expect(line.id).toBe(iblId); expect(line.allocatedPortion).toBeCloseTo(500); - expect(line.linkedItem).toEqual({ type: 'work_item', id: wiId, name: 'Foundation Work' }); + expect(line.linkedItem).toEqual({ + type: 'work_item', + id: wiId, + name: 'Foundation Work', + areaId: null, + areaName: null, + }); }); it('budgetLines[] linkedItem resolves a household item budget line', async () => { @@ -864,6 +888,8 @@ describe('sourceReportService', () => { type: 'household_item', id: hiId, name: 'Kitchen Cabinet', + areaId: null, + areaName: null, }); }); @@ -1061,6 +1087,204 @@ describe('sourceReportService', () => { }); }); + // ═══════════════════════════════════════════════════════════════════════ + // Story #1923 AC5: budgetLines[].linkedItem gains areaId/areaName, populated via a + // LEFT JOIN areas on work_items.area_id / household_items.area_id. + // ═══════════════════════════════════════════════════════════════════════ + + describe('getSourceReport — Story #1923 AC5 (linkedItem areaId/areaName)', () => { + it('AC5.1: a work-item-linked budget line whose work item has an area → linkedItem.areaId/areaName populated', async () => { + const sourceId = insertSource(); + const vendorId = insertVendor(); + const areaId = insertArea('Kitchen'); + const wiId = `wi-area-${++counter}`; + const now = ts(); + db.insert(schema.workItems) + .values({ + id: wiId, + title: 'Cabinetry', + status: 'not_started', + areaId, + createdAt: now, + updatedAt: now, + }) + .run(); + const budgetId = `wib-area-${counter}`; + db.insert(schema.workItemBudgets) + .values({ + id: budgetId, + workItemId: wiId, + budgetSourceId: sourceId, + plannedAmount: 0, + confidence: 'own_estimate', + createdAt: now, + updatedAt: now, + }) + .run(); + const invId = insertInvoice(vendorId, { status: 'paid', amount: 500 }); + insertInvoiceBudgetLine(invId, budgetId, 500); + + const result = await getSourceReport(db, 'claim', sourceId, PAPERLESS_DISABLED); + + expect(result.invoices[0]!.budgetLines[0]!.linkedItem).toEqual({ + type: 'work_item', + id: wiId, + name: 'Cabinetry', + areaId, + areaName: 'Kitchen', + }); + }); + + it('AC5.1: a household-item-linked budget line whose household item has an area → linkedItem.areaId/areaName populated', async () => { + const sourceId = insertSource(); + const vendorId = insertVendor(); + const areaId = insertArea('Living Room'); + const hiId = `hi-area-${++counter}`; + const now = ts(); + db.insert(schema.householdItems) + .values({ + id: hiId, + name: 'Sofa', + categoryId: 'hic-furniture', + areaId, + createdAt: now, + updatedAt: now, + }) + .run(); + const budgetId = `hib-area-${counter}`; + db.insert(schema.householdItemBudgets) + .values({ + id: budgetId, + householdItemId: hiId, + budgetSourceId: sourceId, + plannedAmount: 0, + confidence: 'own_estimate', + createdAt: now, + updatedAt: now, + }) + .run(); + const invId = insertInvoice(vendorId, { status: 'paid', amount: 250 }); + db.insert(schema.invoiceBudgetLines) + .values({ + id: randomUUID(), + invoiceId: invId, + householdItemBudgetId: budgetId, + itemizedAmount: 250, + createdAt: ts(), + updatedAt: ts(), + }) + .run(); + + const result = await getSourceReport(db, 'claim', sourceId, PAPERLESS_DISABLED); + + expect(result.invoices[0]!.budgetLines[0]!.linkedItem).toEqual({ + type: 'household_item', + id: hiId, + name: 'Sofa', + areaId, + areaName: 'Living Room', + }); + }); + + it('AC5.4: linkedItem.areaId/areaName are null (not undefined/"") when the linked work item has no area assigned', async () => { + const sourceId = insertSource(); + const vendorId = insertVendor(); + const budgetId = insertWorkItemBudget(sourceId); // areaId defaults to null (insertWorkItemBudget doesn't set it) + const invId = insertInvoice(vendorId, { status: 'paid', amount: 500 }); + insertInvoiceBudgetLine(invId, budgetId, 500); + + const result = await getSourceReport(db, 'claim', sourceId, PAPERLESS_DISABLED); + + const linkedItem = result.invoices[0]!.budgetLines[0]!.linkedItem!; + expect(linkedItem.areaId).toBeNull(); + expect(linkedItem.areaName).toBeNull(); + expect(linkedItem.areaId).not.toBeUndefined(); + expect(linkedItem.areaName).not.toBeUndefined(); + }); + + it('AC5.5: only the leaf (own) area name is returned for a child area with a parent — no parent-path expansion', async () => { + const sourceId = insertSource(); + const vendorId = insertVendor(); + const parentAreaId = insertArea('Ground Floor'); + const childAreaId = insertArea('Ensuite', parentAreaId); + const wiId = `wi-child-area-${++counter}`; + const now = ts(); + db.insert(schema.workItems) + .values({ + id: wiId, + title: 'Tiling', + status: 'not_started', + areaId: childAreaId, + createdAt: now, + updatedAt: now, + }) + .run(); + const budgetId = `wib-child-area-${counter}`; + db.insert(schema.workItemBudgets) + .values({ + id: budgetId, + workItemId: wiId, + budgetSourceId: sourceId, + plannedAmount: 0, + confidence: 'own_estimate', + createdAt: now, + updatedAt: now, + }) + .run(); + const invId = insertInvoice(vendorId, { status: 'paid', amount: 500 }); + insertInvoiceBudgetLine(invId, budgetId, 500); + + const result = await getSourceReport(db, 'claim', sourceId, PAPERLESS_DISABLED); + + const linkedItem = result.invoices[0]!.budgetLines[0]!.linkedItem!; + expect(linkedItem.areaId).toBe(childAreaId); + expect(linkedItem.areaName).toBe('Ensuite'); + expect(linkedItem.areaName).not.toContain('Ground Floor'); + expect(linkedItem.areaName).not.toContain('/'); + }); + + it('linkedItem stays null (unaffected by the areaId/areaName join) when the work item id resolves but its title is empty (defensive coalesce)', async () => { + const sourceId = insertSource(); + const vendorId = insertVendor(); + const areaId = insertArea('Attic'); + const wiId = `wi-empty-title-${++counter}`; + const now = ts(); + // A work item with an empty-string title and an assigned area: the service's + // `row.work_item_id && row.work_item_title` guard treats the falsy empty title as + // "unresolved", so linkedItem must stay null — the area join must not leak through when + // the item itself doesn't resolve. + db.insert(schema.workItems) + .values({ + id: wiId, + title: '', + status: 'not_started', + areaId, + createdAt: now, + updatedAt: now, + }) + .run(); + const budgetId = `wib-empty-title-${counter}`; + db.insert(schema.workItemBudgets) + .values({ + id: budgetId, + workItemId: wiId, + budgetSourceId: sourceId, + plannedAmount: 0, + confidence: 'own_estimate', + createdAt: now, + updatedAt: now, + }) + .run(); + const invId = insertInvoice(vendorId, { status: 'paid', amount: 500 }); + insertInvoiceBudgetLine(invId, budgetId, 500); + + const result = await getSourceReport(db, 'claim', sourceId, PAPERLESS_DISABLED); + + expect(result.invoices[0]!.budgetLines).toHaveLength(1); + expect(result.invoices[0]!.budgetLines[0]!.linkedItem).toBeNull(); + }); + }); + // ═══════════════════════════════════════════════════════════════════════ // Bug #1918: claim reports drop zero-contribution budget lines. A quotation // invoice whose only funding is a deposit TAGGED to the reported source has its diff --git a/server/src/services/sourceReportService.ts b/server/src/services/sourceReportService.ts index 00e313444..13eb2c232 100644 --- a/server/src/services/sourceReportService.ts +++ b/server/src/services/sourceReportService.ts @@ -90,6 +90,8 @@ export async function getSourceReport( work_item_title: string | null; household_item_id: string | null; household_item_name: string | null; + area_id: string | null; + area_name: string | null; }; const railARows = db.all( @@ -112,7 +114,9 @@ export async function getSourceReport( wib.work_item_id AS work_item_id, wi.title AS work_item_title, hib.household_item_id AS household_item_id, - hi.name AS household_item_name + hi.name AS household_item_name, + COALESCE(awi.id, ahi.id) AS area_id, + COALESCE(awi.name, ahi.name) AS area_name FROM invoice_budget_lines ibl INNER JOIN invoices i ON i.id = ibl.invoice_id INNER JOIN vendors v ON v.id = i.vendor_id @@ -121,6 +125,8 @@ export async function getSourceReport( LEFT JOIN work_items wi ON wi.id = wib.work_item_id LEFT JOIN household_item_budgets hib ON hib.id = ibl.household_item_budget_id LEFT JOIN household_items hi ON hi.id = hib.household_item_id + LEFT JOIN areas awi ON awi.id = wi.area_id + LEFT JOIN areas ahi ON ahi.id = hi.area_id WHERE ( (ibl.work_item_budget_id IS NOT NULL AND EXISTS ( SELECT 1 FROM work_item_budgets wib2 @@ -150,12 +156,20 @@ export async function getSourceReport( if (!iblDetails.has(row.ibl_id)) { let linkedItem: SourceReportLinkedItem | null = null; if (row.work_item_id && row.work_item_title) { - linkedItem = { type: 'work_item', id: row.work_item_id, name: row.work_item_title }; + linkedItem = { + type: 'work_item', + id: row.work_item_id, + name: row.work_item_title, + areaId: row.area_id, + areaName: row.area_name, + }; } else if (row.household_item_id && row.household_item_name) { linkedItem = { type: 'household_item', id: row.household_item_id, name: row.household_item_name, + areaId: row.area_id, + areaName: row.area_name, }; } iblDetails.set(row.ibl_id, { diff --git a/shared/src/types/sourceReport.ts b/shared/src/types/sourceReport.ts index 6061ff2f9..d7805aae2 100644 --- a/shared/src/types/sourceReport.ts +++ b/shared/src/types/sourceReport.ts @@ -9,6 +9,8 @@ export interface SourceReportLinkedItem { type: 'work_item' | 'household_item'; id: string; name: string; + areaId: string | null; + areaName: string | null; } /** Budget line subtraction row: allocatedPortion is subtraction-only, never independently summed. */ diff --git a/wiki b/wiki index d6e13dc42..f3101b524 160000 --- a/wiki +++ b/wiki @@ -1 +1 @@ -Subproject commit d6e13dc422ccd4a23ef5239bb7441c7184f048f6 +Subproject commit f3101b524dda76c264b744f2e8f083b9a62ceeb3