fix(invoice): make status breakdown summary deposit-aware (#1411)

[steilerDev]

Summary

• InvoiceStatusBreakdown.summary (used by the InvoicesPage header cards and Dashboard InvoicePipelineCard) was not deposit-aware: a €1000 quotation invoice with a €200 pending deposit incorrectly showed €1000 under Quotation and €0 under Pending
• Adds aggregateInvoiceStatusBreakdown() to depositAggregateUtils.ts and rewrites listAllInvoices() summary to split amounts across parent + deposit statuses via LEFT JOIN on invoice_deposits; counts remain per-invoice (deposit rows never increment a count)
• Summary stays globally unfiltered (filter-independent) — consistent with existing UX; wiki API-Contract.md updated with the deposit-aware invariant and quotation example entry

Fixes #1411

Test plan

• Unit tests for aggregateInvoiceStatusBreakdown() cover: no deposits, single deposit, multiple deposits with mixed statuses, deposit sum exceeding invoice amount (clamped to 0 residual)
• invoiceService.test.ts verifies deposit-aware summary in listAllInvoices() for global unfiltered totals
• Quality Gates CI passes (typecheck + unit tests + build)

[steilerDev]

[product-architect] Review: APPROVED with medium findings for refinement.

Verdict

APPROVED — the fix is correct, complete, and resolves the gap that #1405 left behind. CI is green. Wiki is updated in the same PR (deposit-aware invariant documented on summary.<status>.totalAmount, quotation example added) — Wiki Update Discipline observed.

Verified

• Completes Invoice deposits: budget rollup integration (residual under parent status, deposits under their own state) #1405's missed surface. Invoice status summary breakdown is not deposit-aware (InvoicesPage header + Dashboard pipeline card) #1411 is precisely the rollup path that Invoice deposits: budget rollup integration (residual under parent status, deposits under their own state) #1405's spec listed but the implementation skipped (InvoiceStatusBreakdown.summary consumed by InvoicesPage header cards + Dashboard InvoicePipelineCard). No other gap remains: the budget-line/source/category/work-item/household-item rollups were already migrated in feat(invoice,budget): invoice deposits UI + deposit-aware budget rollups (#1404, #1405) #1407, and the per-invoice detail view (getInvoiceById) shows deposits[] directly rather than re-aggregating.
• Math. Invoice-level proportional split: summary[I.status].totalAmount += max(0, I.amount − Σdeposits), summary[deposit.status].totalAmount += deposit.amount, count once per invoice. Defense-in-depth on negative residual via Math.max(0,...). Sum invariant holds: Σ summary[s].totalAmount === Σ I.amount (Scenario 4 + integration test "sum invariant" both assert this).
• Deduplication. deposits.some(d => d.depositId === row.deposit_id) correctly handles the LEFT JOIN's row expansion (Scenario 8 explicitly tests this).
• Global summary preserved (correct architectural call). Pre-existing test at invoiceService.test.ts:934 already pinned this: summary reflects global invoice counts and totals across ALL invoices (ignores page filter). The wiki API-Contract.md line 3820 also documents it: summary field is always the global unfiltered summary. The PR preserves both. This deviates from Invoice status summary breakdown is not deposit-aware (InvoicesPage header + Dashboard pipeline card) #1411 AC-5 ('breakdown computed for the filtered set'), but the deviation is correct — the established UX is that header cards remain stable while filtering the list ('see what the other filter values would yield'), and the deposit-aware fix is orthogonal to filter scoping. AC-5 was inconsistent with the pre-existing system; the PR correctly chose the system's source of truth (existing test + wiki) over the issue's text.
• Performance. One LEFT JOIN over the full invoices table for the global summary. No N+1 (deduplication is in-memory by Map). At <5 users this is comfortably acceptable; even with thousands of invoices and deposits per project, this is one O(N+M) scan.
• Test coverage. 9 unit tests on aggregateInvoiceStatusBreakdown exercise empty input, no-deposits, single deposit, sum invariant, multi-deposit residual+accrual, clamp-to-zero, multi-invoice count, dedupe, and mixed scenarios. 6 integration tests cover AC-1..AC-8 against the live SQL. Coverage is 100% on the new helper.

Medium findings (refinement-eligible, not blocking)

1. Fourth instance of the row-grouping + deposit-dedup pattern. depositAggregateUtils.ts now has three functions (computeDepositAwareAggregates, computeStatusContribution, aggregateInvoiceStatusBreakdown) that each re-implement the same Map<invoiceId, ...> + depositsByInvoice + dedupe-by-depositId preamble (lines 51-87, 170-205, 279-297) plus a budgetSourceService.computeDiscretionaryInvoiceAmount inline implementation. My review on feat(invoice,budget): invoice deposits UI + deposit-aware budget rollups (#1404, #1405) #1407 already flagged this and recommended extracting a splitByDeposits() helper. This PR adds the fourth instance.

   Recommendation: Refactor in a follow-up — not in this PR. Reasoning: (a) the bug fix is small and focused, mixing it with a 3-call-site refactor would balloon the diff and risk breaking the working rollups; (b) the duplication is now well-isolated to one file with all four call sites visible together, which is the right shape for a clean extraction; (c) no tracking issue exists yet — please open one (refactor(invoice): extract splitByDeposits helper from depositAggregateUtils) and tag it for refinement during /epic-close or the next cleanup pass. The right abstraction is a single function that takes the raw rows + a per-invoice callback (invoiceAmount, parentStatus, deposits[]) => void, leaving the per-call-site accumulation logic to the caller.

2. Two minor nits in the new helper (not blocking, can fold into the refactor above):

   • ensure(deposit.depositStatus) is called even for deposits whose status equals the parent invoice's status — no functional issue (idempotent), just a tiny waste.
   • The interface name InvoiceDepositRow is fine but consider InvoiceWithDepositRow to disambiguate from invoice_deposits table rows.

3. Test comment cleanliness. invoiceService.test.ts AC-1 uses a raw SQL UPDATE to set status='quotation' because insertRawInvoice doesn't accept 'quotation' in its typed union. Consider widening that helper's status param to the full InvoiceStatus union in a follow-up — it would clean up this and any future quotation test cases. Not a blocker.

Not findings

• The global-stays-global decision is correct and consistent with the existing system. The wiki, the pre-existing test, and this PR all align.
• Performance concerns at scale are theoretical at this project's <5-user target; not worth optimizing.

Architecturally sound. Approving — please open the refactor tracking issue before merge so the duplication doesn't get forgotten.

[github-actions]

🎉 This PR is included in version 2.6.0-beta.4 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀