From 7bc21150f7287ceb5aa8cb5ba70bc2cc2348cf6b Mon Sep 17 00:00:00 2001 From: Frank Steiler Date: Mon, 11 May 2026 19:40:57 +0200 Subject: [PATCH] fix(invoice): make status breakdown summary deposit-aware MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit InvoiceStatusBreakdown.summary (consumed by the InvoicesPage header and the Dashboard InvoicePipelineCard) was missed when #1405 migrated budget rollups to the deposit-aware split. A quotation invoice of €1000 with a pending deposit of €200 showed €1000 under Quotation and €0 under Pending — should be €800 (residual) and €200 (deposit). Adds aggregateInvoiceStatusBreakdown() to depositAggregateUtils.ts and rewrites listAllInvoices() summary to use it with a LEFT JOIN onto invoice_deposits. Per-invoice split: summary[parent].totalAmount accrues max(0, amount - Σ deposits), each summary[deposit.status].totalAmount accrues deposit.amount; count stays per-invoice (not per row). Summary remains GLOBAL (filter-independent) — the existing UX where the header cards stay stable while the user filters the list is preserved. The pre-existing "summary reflects global counts" test is unmodified. Wiki updated: API-Contract.md documents the deposit-aware semantic and adds quotation to the example summary block. Fixes #1411 Co-Authored-By: Claude Opus 4.7 (1M context) Co-Authored-By: Claude backend-developer (Haiku 4.5) Co-Authored-By: Claude qa-integration-tester (Sonnet 4.6) --- server/src/services/invoiceService.test.ts | 147 ++++++++++++++++ server/src/services/invoiceService.ts | 42 ++--- .../shared/depositAggregateUtils.test.ts | 160 ++++++++++++++++++ .../services/shared/depositAggregateUtils.ts | 83 +++++++++ wiki | 2 +- 5 files changed, 412 insertions(+), 22 deletions(-) diff --git a/server/src/services/invoiceService.test.ts b/server/src/services/invoiceService.test.ts index 9a34d59b3..4b651c5c8 100644 --- a/server/src/services/invoiceService.test.ts +++ b/server/src/services/invoiceService.test.ts @@ -980,6 +980,153 @@ describe('Invoice Service', () => { }); }); + // ─── listAllInvoices() deposit-aware summary (#1411) ──────────────────────── + + describe('listAllInvoices() deposit-aware summary (#1411)', () => { + /** + * Insert a deposit directly into the DB (bypassing service validation). + * Scoped to this describe block; mirrors the helper in the #1403 block. + */ + function insertRawDeposit( + invoiceId: string, + amount: number, + status: 'pending' | 'paid' | 'claimed' = 'pending', + ): string { + const id = `dep-summary-${Date.now()}-${Math.random().toString(36).substring(7)}`; + const ts = new Date(Date.now() + timestampOffset++).toISOString(); + db.insert(schema.invoiceDeposits) + .values({ + id, + invoiceId, + amount, + dueDate: '2026-03-01', + paidDate: status === 'paid' || status === 'claimed' ? '2026-02-15' : null, + claimedDate: status === 'claimed' ? '2026-02-20' : null, + description: null, + status, + createdBy: null, + createdAt: ts, + updatedAt: ts, + }) + .run(); + return id; + } + + // ─── AC-1: quotation invoice with pending deposit ────────────────────── + + it('AC-1: quotation invoice (1000) with pending deposit (200) — residual 800 under quotation, 200 under pending', () => { + const vendorId = createTestVendor('AC1 Vendor'); + const invoiceId = insertRawInvoice(vendorId, { + // insertRawInvoice only types 'pending'|'paid'|'claimed' but the DB accepts 'quotation' + status: 'pending' as 'pending', + amount: 1000, + }); + // Override status to 'quotation' via underlying better-sqlite3 (Drizzle type doesn't include it) + sqlite.prepare(`UPDATE invoices SET status = 'quotation' WHERE id = ?`).run(invoiceId); + insertRawDeposit(invoiceId, 200, 'pending'); + + const result = invoiceService.listAllInvoices(db, {}); + + expect(result.summary.quotation.totalAmount).toBe(800); + expect(result.summary.pending.totalAmount).toBe(200); + expect(result.summary.quotation.count).toBe(1); + expect(result.summary.pending.count).toBe(0); + }); + + // ─── AC-2: invoice with no deposits unchanged ────────────────────────── + + it('AC-2 regression: invoice with no deposits has its full amount in the summary', () => { + const vendorId = createTestVendor('AC2 Vendor'); + insertRawInvoice(vendorId, { status: 'pending', amount: 1000 }); + // no deposits — behavior must be unchanged from pre-deposit era + + const result = invoiceService.listAllInvoices(db, {}); + + expect(result.summary.pending.totalAmount).toBe(1000); + expect(result.summary.pending.count).toBe(1); + }); + + // ─── AC-3: count reflects invoices not deposits ──────────────────────── + + it('AC-3: one invoice with two deposits — summary count = 1, not 3', () => { + const vendorId = createTestVendor('AC3 Vendor'); + const invoiceId = insertRawInvoice(vendorId, { status: 'pending', amount: 1000 }); + insertRawDeposit(invoiceId, 300, 'pending'); + insertRawDeposit(invoiceId, 400, 'paid'); + + const result = invoiceService.listAllInvoices(db, {}); + + // The invoice itself is pending, so count must be 1 (not 2 deposits + 1 invoice = 3) + expect(result.summary.pending.count).toBe(1); + }); + + // ─── Sum invariant integration ───────────────────────────────────────── + + it('sum invariant: all status totals sum to the invoice amount', () => { + const invoiceAmount = 1000; + const vendorId = createTestVendor('Sum Invariant Vendor'); + const invoiceId = insertRawInvoice(vendorId, { status: 'pending', amount: invoiceAmount }); + insertRawDeposit(invoiceId, 300, 'pending'); + insertRawDeposit(invoiceId, 400, 'paid'); + // residual = 1000 - 300 - 400 = 300 under pending + + const result = invoiceService.listAllInvoices(db, {}); + + const totalAcrossStatuses = + result.summary.pending.totalAmount + + result.summary.paid.totalAmount + + result.summary.claimed.totalAmount + + result.summary.quotation.totalAmount; + expect(totalAcrossStatuses).toBe(invoiceAmount); + }); + + // ─── Multi-status deposits (residual + per-status accrual) ──────────── + + it('multi-status deposits: residual under parent status + per-deposit status accrual', () => { + const vendorId = createTestVendor('Multi Deposit Status Vendor'); + const invoiceId = insertRawInvoice(vendorId, { status: 'pending', amount: 1200 }); + insertRawDeposit(invoiceId, 300, 'pending'); + insertRawDeposit(invoiceId, 400, 'paid'); + // residual = 1200 - 300 - 400 = 500 under pending (invoice status) + + const result = invoiceService.listAllInvoices(db, {}); + + // pending.totalAmount = residual (500) + pending deposit (300) = 800 + expect(result.summary.pending.totalAmount).toBe(800); + expect(result.summary.paid.totalAmount).toBe(400); + expect(result.summary.pending.count).toBe(1); // only the invoice, not deposits + expect(result.summary.paid.count).toBe(0); + }); + + // ─── Global behavior preserved ───────────────────────────────────────── + + it('global behavior preserved: summary is not filtered even when status + pageSize filter applied', () => { + const vendorId = createTestVendor('Global Behavior Vendor'); + const pending1Id = insertRawInvoice(vendorId, { status: 'pending', amount: 400 }); + insertRawInvoice(vendorId, { status: 'pending', amount: 300 }); + insertRawInvoice(vendorId, { status: 'paid', amount: 500 }); + // Add a pending deposit to the first pending invoice to exercise deposit-aware path globally + insertRawDeposit(pending1Id, 50, 'pending'); + // pending1 residual = 400 - 50 = 350 under pending; deposit 50 under pending + // pending1 total contribution to pending = 350 + 50 = 400 (same as before, just split) + // pending2 = 300 under pending; paid = 500 under paid + + // Filter to only paid invoices, page 1 of 1 + const result = invoiceService.listAllInvoices(db, { status: 'paid', pageSize: 1 }); + + // Filtered list: only the paid invoice + expect(result.invoices).toHaveLength(1); + expect(result.invoices[0]!.amount).toBe(500); + + // Summary must be GLOBAL (all 3 invoices, deposit-aware) + expect(result.summary.pending.count).toBe(2); + // pending total = 350 (residual of pending1) + 50 (deposit from pending1, status pending) + 300 (pending2) = 700 + expect(result.summary.pending.totalAmount).toBe(700); + expect(result.summary.paid.count).toBe(1); + expect(result.summary.paid.totalAmount).toBe(500); + }); + }); + // ─── getInvoiceById() ─────────────────────────────────────────────────────── describe('getInvoiceById()', () => { diff --git a/server/src/services/invoiceService.ts b/server/src/services/invoiceService.ts index 09966c158..7a4402c8e 100644 --- a/server/src/services/invoiceService.ts +++ b/server/src/services/invoiceService.ts @@ -18,6 +18,10 @@ import { NotFoundError, ValidationError } from '../errors/AppError.js'; import { deleteLinksForEntity } from './documentLinkService.js'; import { getInvoiceBudgetLinesForInvoice } from './invoiceBudgetLineService.js'; import { onInvoiceStatusChanged } from './diaryAutoEventService.js'; +import { + aggregateInvoiceStatusBreakdown, + type InvoiceDepositRow, +} from './shared/depositAggregateUtils.js'; type DbType = BetterSQLite3Database; @@ -269,35 +273,31 @@ export function listAllInvoices( .offset(offset) .all(); - // Compute global summary (unfiltered — across all invoices) - const summaryRows = db + // Compute deposit-aware GLOBAL summary across ALL invoices (filter-independent). + // The page filters only narrow the listed rows; the header summary always shows totals + // across the entire dataset so users can see what the other filter values would yield. + const summaryRawRows: InvoiceDepositRow[] = db .select({ - status: invoices.status, - count: sql`COUNT(*)`, - totalAmount: sql`COALESCE(SUM(${invoices.amount}), 0)`, + invoice_id: invoices.id, + invoice_amount: invoices.amount, + invoice_status: invoices.status, + deposit_id: invoiceDeposits.id, + deposit_amount: invoiceDeposits.amount, + deposit_status: invoiceDeposits.status, }) .from(invoices) - .groupBy(invoices.status) + .leftJoin(invoiceDeposits, eq(invoiceDeposits.invoiceId, invoices.id)) .all(); + const aggregated = aggregateInvoiceStatusBreakdown(summaryRawRows); + const defaultSummary: InvoiceStatusSummary = { count: 0, totalAmount: 0 }; const summary: InvoiceStatusBreakdown = { - pending: { ...defaultSummary }, - paid: { ...defaultSummary }, - claimed: { ...defaultSummary }, - quotation: { ...defaultSummary }, + pending: aggregated['pending'] ?? { ...defaultSummary }, + paid: aggregated['paid'] ?? { ...defaultSummary }, + claimed: aggregated['claimed'] ?? { ...defaultSummary }, + quotation: aggregated['quotation'] ?? { ...defaultSummary }, }; - for (const row of summaryRows) { - const status = row.status as InvoiceStatus; - if ( - status === 'pending' || - status === 'paid' || - status === 'claimed' || - status === 'quotation' - ) { - summary[status] = { count: row.count, totalAmount: row.totalAmount }; - } - } // Map rows directly (not using toInvoice()) to avoid fetching full deposits in list. // NOTE: List endpoints set deposits: [] and finalPaymentAmount: row.amount to keep payload small. diff --git a/server/src/services/shared/depositAggregateUtils.test.ts b/server/src/services/shared/depositAggregateUtils.test.ts index 4878a1fce..b87f6e351 100644 --- a/server/src/services/shared/depositAggregateUtils.test.ts +++ b/server/src/services/shared/depositAggregateUtils.test.ts @@ -2,7 +2,9 @@ import { describe, it, expect } from '@jest/globals'; import { computeDepositAwareAggregates, computeStatusContribution, + aggregateInvoiceStatusBreakdown, type DepositAwareRow, + type InvoiceDepositRow, } from './depositAggregateUtils.js'; // ─── Helpers ────────────────────────────────────────────────────────────────── @@ -399,3 +401,161 @@ describe('computeStatusContribution', () => { expect(() => computeStatusContribution(rows, 'claimed')).not.toThrow(); }); }); + +// ─── aggregateInvoiceStatusBreakdown ────────────────────────────────────────── + +/** + * Build an InvoiceDepositRow with no deposit (deposit columns null). + */ +function makeInvoiceRow( + invoiceId: string, + invoiceAmount: number, + invoiceStatus: string, + deposit?: { id: string; amount: number; status: string }, +): InvoiceDepositRow { + return { + invoice_id: invoiceId, + invoice_amount: invoiceAmount, + invoice_status: invoiceStatus, + deposit_id: deposit?.id ?? null, + deposit_amount: deposit?.amount ?? null, + deposit_status: deposit?.status ?? null, + }; +} + +describe('aggregateInvoiceStatusBreakdown', () => { + // ─── Scenario 1: Empty rows ──────────────────────────────────────────────── + + it('returns {} for empty rows array', () => { + const result = aggregateInvoiceStatusBreakdown([]); + expect(result).toEqual({}); + }); + + // ─── Scenario 2: Single invoice, no deposits ────────────────────────────── + + it('single invoice with no deposits: full amount under invoice status', () => { + const rows: InvoiceDepositRow[] = [ + makeInvoiceRow('i1', 1000, 'quotation'), + ]; + const result = aggregateInvoiceStatusBreakdown(rows); + expect(result['quotation']).toEqual({ count: 1, totalAmount: 1000 }); + // No deposit key should exist + expect(result['pending']).toBeUndefined(); + }); + + // ─── Scenario 3: Single invoice with one pending deposit ────────────────── + + it('single quotation invoice (1000) with one pending deposit (200): residual 800 under quotation, 200 under pending', () => { + const rows: InvoiceDepositRow[] = [ + makeInvoiceRow('i1', 1000, 'quotation', { id: 'd1', amount: 200, status: 'pending' }), + ]; + const result = aggregateInvoiceStatusBreakdown(rows); + expect(result['quotation']!.count).toBe(1); + expect(result['quotation']!.totalAmount).toBe(800); + expect(result['pending']!.count).toBe(0); + expect(result['pending']!.totalAmount).toBe(200); + }); + + // ─── Scenario 4: Sum invariant ──────────────────────────────────────────── + + it('sum invariant: quotation.totalAmount + pending.totalAmount === invoiceAmount', () => { + const invoiceAmount = 1000; + const depositAmount = 200; + const rows: InvoiceDepositRow[] = [ + makeInvoiceRow('i1', invoiceAmount, 'quotation', { + id: 'd1', + amount: depositAmount, + status: 'pending', + }), + ]; + const result = aggregateInvoiceStatusBreakdown(rows); + const quotationTotal = result['quotation']!.totalAmount; + const pendingTotal = result['pending']!.totalAmount; + expect(quotationTotal + pendingTotal).toBe(invoiceAmount); + }); + + // ─── Scenario 5: Multiple deposits on one invoice ───────────────────────── + + it('multiple deposits on one invoice: residual + per-status accrual', () => { + // invoice 1200 quotation, deposits: 300 pending + 400 paid + // residual = 1200 - 300 - 400 = 500 under quotation + const rows: InvoiceDepositRow[] = [ + makeInvoiceRow('i1', 1200, 'quotation', { id: 'd1', amount: 300, status: 'pending' }), + makeInvoiceRow('i1', 1200, 'quotation', { id: 'd2', amount: 400, status: 'paid' }), + ]; + const result = aggregateInvoiceStatusBreakdown(rows); + expect(result['quotation']!.count).toBe(1); + expect(result['quotation']!.totalAmount).toBe(500); + expect(result['pending']!.totalAmount).toBe(300); + expect(result['paid']!.totalAmount).toBe(400); + // count is only incremented for the parent invoice status bucket + expect(result['pending']!.count).toBe(0); + expect(result['paid']!.count).toBe(0); + }); + + // ─── Scenario 6: Deposit sum exceeds invoice amount (clamped to 0) ───────── + + it('deposit sum exceeds invoice amount: residual clamped to 0 via Math.max(0,...)', () => { + // invoice 100 quotation, deposit 150 paid → residual = Math.max(0, 100 - 150) = 0 + const rows: InvoiceDepositRow[] = [ + makeInvoiceRow('i1', 100, 'quotation', { id: 'd1', amount: 150, status: 'paid' }), + ]; + const result = aggregateInvoiceStatusBreakdown(rows); + // residual = 0: quotation.totalAmount must be 0 (not negative) + expect(result['quotation']!.totalAmount).toBe(0); + // deposit amount still fully accrued + expect(result['paid']!.totalAmount).toBe(150); + }); + + // ─── Scenario 7: Multiple invoices, count once each ─────────────────────── + + it('multiple invoices with same status: count equals invoice count (not row count)', () => { + // Two pending invoices, no deposits + const rows: InvoiceDepositRow[] = [ + makeInvoiceRow('i1', 300, 'pending'), + makeInvoiceRow('i2', 500, 'pending'), + ]; + const result = aggregateInvoiceStatusBreakdown(rows); + expect(result['pending']!.count).toBe(2); + expect(result['pending']!.totalAmount).toBe(800); + }); + + // ─── Scenario 8: Duplicate deposit rows deduped ─────────────────────────── + + it('same deposit_id appearing multiple times in rows is counted only once', () => { + // SQLite LEFT JOIN can produce one row per invoice-deposit pair; this simulates + // that scenario where the same deposit appears twice (e.g. from a different source row). + const rows: InvoiceDepositRow[] = [ + makeInvoiceRow('i1', 1000, 'pending', { id: 'd1', amount: 200, status: 'paid' }), + // Same deposit_id 'd1' appearing again (duplicate) — must be deduplicated + makeInvoiceRow('i1', 1000, 'pending', { id: 'd1', amount: 200, status: 'paid' }), + ]; + const result = aggregateInvoiceStatusBreakdown(rows); + // residual = 1000 - 200 = 800, not 1000 - 400 = 600 + expect(result['pending']!.totalAmount).toBe(800); + expect(result['paid']!.totalAmount).toBe(200); + // sum invariant: 800 + 200 = 1000 + expect(result['pending']!.totalAmount + result['paid']!.totalAmount).toBe(1000); + }); + + // ─── Scenario 9: Mixed scenario ─────────────────────────────────────────── + + it('mixed scenario: invoice A (quotation, 500, pending deposit 100) + invoice B (paid, 200, no deposits)', () => { + // Invoice A: quotation 500, deposit 100 pending → quotation residual 400, pending 100 + // Invoice B: paid 200, no deposits → paid 200 + const rows: InvoiceDepositRow[] = [ + makeInvoiceRow('invA', 500, 'quotation', { id: 'dA1', amount: 100, status: 'pending' }), + makeInvoiceRow('invB', 200, 'paid'), + ]; + const result = aggregateInvoiceStatusBreakdown(rows); + + expect(result['quotation']!.count).toBe(1); + expect(result['quotation']!.totalAmount).toBe(400); + + expect(result['paid']!.count).toBe(1); + expect(result['paid']!.totalAmount).toBe(200); + + expect(result['pending']!.count).toBe(0); + expect(result['pending']!.totalAmount).toBe(100); + }); +}); diff --git a/server/src/services/shared/depositAggregateUtils.ts b/server/src/services/shared/depositAggregateUtils.ts index 5ce03edea..d951b73a6 100644 --- a/server/src/services/shared/depositAggregateUtils.ts +++ b/server/src/services/shared/depositAggregateUtils.ts @@ -239,3 +239,86 @@ export function computeStatusContribution( return total; } + +/** + * Raw row for (invoices LEFT JOIN invoice_deposits) jointures, used by the + * InvoiceStatusBreakdown summary computation in invoiceService.listAllInvoices. + */ +export interface InvoiceDepositRow { + invoice_id: string; + invoice_amount: number; + invoice_status: string; + deposit_id: string | null; + deposit_amount: number | null; + deposit_status: string | null; +} + +/** + * Computes the InvoiceStatusBreakdown summary from (invoices LEFT JOIN invoice_deposits) rows. + * + * Per-invoice split (same as the budget-line rollup formula but at the invoice level): + * summary[I.status].totalAmount += max(0, I.amount − Σ deposits.amount) + * summary[deposit.status].totalAmount += deposit.amount (for each deposit) + * summary[I.status].count += 1 (once per invoice, regardless of deposit rows) + * + * Invariant: Σ summary[s].totalAmount === Σ I.amount across all invoices in the input. + * + * Returns a sparse map — callers must merge with defaults (e.g. { count: 0, totalAmount: 0 }). + */ +export function aggregateInvoiceStatusBreakdown( + rows: InvoiceDepositRow[], +): Record { + if (rows.length === 0) return {}; + + const invoiceMap = new Map(); + const depositsByInvoice = new Map< + string, + Array<{ depositId: string; depositAmount: number; depositStatus: string }> + >(); + + for (const row of rows) { + if (!invoiceMap.has(row.invoice_id)) { + invoiceMap.set(row.invoice_id, { + invoiceAmount: row.invoice_amount, + invoiceStatus: row.invoice_status, + }); + } + if (row.deposit_id !== null && row.deposit_amount !== null && row.deposit_status !== null) { + const deps = depositsByInvoice.get(row.invoice_id) ?? []; + if (!deps.some((d) => d.depositId === row.deposit_id)) { + deps.push({ + depositId: row.deposit_id, + depositAmount: row.deposit_amount, + depositStatus: row.deposit_status, + }); + depositsByInvoice.set(row.invoice_id, deps); + } + } + } + + const result: Record = {}; + const ensure = (status: string) => { + if (!result[status]) result[status] = { count: 0, totalAmount: 0 }; + }; + + for (const [invoiceId, inv] of invoiceMap) { + const S = inv.invoiceStatus; + ensure(S); + result[S]!.count += 1; + + const deposits = depositsByInvoice.get(invoiceId) ?? []; + if (deposits.length === 0) { + result[S]!.totalAmount += inv.invoiceAmount; + } else { + const depositSum = deposits.reduce((s, d) => s + d.depositAmount, 0); + const residual = Math.max(0, inv.invoiceAmount - depositSum); + result[S]!.totalAmount += residual; + for (const deposit of deposits) { + ensure(deposit.depositStatus); + result[deposit.depositStatus]!.totalAmount += deposit.depositAmount; + } + } + } + + return result; +} diff --git a/wiki b/wiki index eca04e8f3..3cf498df6 160000 --- a/wiki +++ b/wiki @@ -1 +1 @@ -Subproject commit eca04e8f30c8990502f49362be882b63849237ff +Subproject commit 3cf498df6c434d7b02898fcba6707fd9f14a692f