fix(invoices): remaining_amount + invoice.paid on agent mark-paid path (#825) (#845)

* fix(invoices): book remaining_amount + emit invoice.paid on agent mark-paid path (#825) The agent/MCP commit path commitMarkInvoicePaid flipped status to 'paid' and set paid_amount = total but never wrote remaining_amount (left at the original total) and never emitted invoice.paid — so partial state and webhooks diverged from the dashboard and v1 mark-paid routes. Route the agent path through the shared planInvoicePayment helper (the source of truth introduced in #841): compute paid/remaining/status with the overpayment guard BEFORE booking the JE (so a rejected payment never burns a voucher number), persist remaining_amount + the partially_paid transition, and best-effort emit invoice.paid for webhook parity. Adds lib/pending-operations/__tests__/mark-invoice-paid.test.ts covering the state + event behaviour of this path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(invoices): address review feedback on agent mark-paid path (#825) - CAS-guard 409 message now reflects the expanded payable states: the UPDATE filter accepts partially_paid (reachable via a concurrent settle race), not just sent/overdue. - Derive the settle amount from total − paid_amount when remaining_amount is null (legacy rows) instead of falling back to the full total, so a prior partial payment is not double-counted into a false overpayment rejection. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(invoices): derive remaining from total − paid_amount on all mark-paid surfaces (#825) The dashboard and v1 mark-paid routes defaulted the settle amount to invoice.total when remaining_amount was null, which over-settles a legacy invoice that has a prior partial payment recorded in paid_amount (false overpayment / AR over-credit). Align both with the agent path (commit.ts): remaining_amount ?? total − paid_amount. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-30 16:01:07 +02:00
parent 46039f14f4
commit 5df6199bd1
6 changed files with 356 additions and 31 deletions
@@ -0,0 +1,110 @@
+/**
+ * State + event coverage for the agent/MCP mark-invoice-paid commit path
+ * (`commitMarkInvoicePaid` in lib/pending-operations/commit.ts).
+ *
+ * Regression guard for issue #825: this path previously flipped status to
+ * 'paid' and set paid_amount = total but never wrote remaining_amount (leaving
+ * it at the original total) and never emitted invoice.paid (so webhooks never
+ * fired). It now routes through the shared planInvoicePayment helper and emits
+ * invoice.paid, matching the dashboard and v1 mark-paid routes.
+ *
+ * Duplicate-payment-guard behaviour is covered separately in
+ * commit-duplicate-guard.test.ts; here the guard is stubbed to "no candidates".
+ */
+import { describe, it, expect, vi, beforeEach } from 'vitest'
+import { eventBus } from '@/lib/events/bus'
+import { createQueuedMockSupabase } from '@/tests/helpers'
+import type { PendingOperation } from '@/types'
+
+const mockCreatePaymentEntry = vi.fn()
+const mockCreateCashEntry = vi.fn()
+vi.mock('@/lib/bookkeeping/invoice-entries', async () => {
+  const actual = await vi.importActual<typeof import('@/lib/bookkeeping/invoice-entries')>(
+    '@/lib/bookkeeping/invoice-entries',
+  )
+  return {
+    ...actual,
+    createInvoicePaymentJournalEntry: (...args: unknown[]) => mockCreatePaymentEntry(...args),
+    createInvoiceCashEntry: (...args: unknown[]) => mockCreateCashEntry(...args),
+  }
+})
+
+const mockFindDupPayments = vi.fn()
+vi.mock('@/lib/invoices/duplicate-payment-candidates', () => ({
+  findDuplicatePaymentCandidatesForInvoice: (...args: unknown[]) => mockFindDupPayments(...args),
+}))
+
+import { commitPendingOperation } from '../commit'
+
+function makePendingOp(overrides: Partial<PendingOperation>): PendingOperation {
+  return {
+    id: 'op-1',
+    user_id: 'user-1',
+    company_id: 'company-1',
+    operation_type: 'mark_invoice_paid',
+    status: 'pending',
+    title: 'test',
+    params: {},
+    preview_data: {},
+    result_data: null,
+    actor_type: 'user',
+    actor_id: null,
+    actor_label: null,
+    risk_level: 'medium',
+    created_at: '2026-05-03T00:00:00Z',
+    resolved_at: null,
+    updated_at: '2026-05-03T00:00:00Z',
+    ...overrides,
+  } as PendingOperation
+}
+
+beforeEach(() => {
+  vi.clearAllMocks()
+  eventBus.clear()
+  mockFindDupPayments.mockResolvedValue([])
+  mockCreatePaymentEntry.mockResolvedValue({ id: 'je-1' })
+  mockCreateCashEntry.mockResolvedValue({ id: 'je-1' })
+})
+
+describe('commitPendingOperation: mark_invoice_paid state + invoice.paid', () => {
+  it('zeroes remaining_amount and emits invoice.paid on full payment (issue #825)', async () => {
+    const { supabase, enqueue } = createQueuedMockSupabase()
+    enqueue({ data: { id: 'op-1' }, error: null }) // CAS claim
+    enqueue({
+      data: {
+        id: 'inv-1',
+        invoice_number: '2026001',
+        status: 'sent',
+        total: 525,
+        remaining_amount: 525,
+        paid_amount: null,
+        document_type: 'invoice',
+        journal_entry_id: null,
+        customer: { name: 'Test AB' },
+      },
+      error: null,
+    }) // invoice fetch
+    enqueue({ data: { accounting_method: 'accrual', entity_type: 'aktiebolag' }, error: null }) // settings
+    enqueue({ data: [{ id: 'inv-1' }], error: null }) // invoice CAS update
+    enqueue({ data: null, error: null }) // dispatcher pending_operations update
+
+    const paidHandler = vi.fn()
+    eventBus.on('invoice.paid', paidHandler)
+
+    const op = makePendingOp({ params: { invoice_id: 'inv-1', payment_date: '2026-03-30' } })
+    const result = await commitPendingOperation(supabase as never, 'user-1', 'company-1', op)
+
+    expect(result.status).toBe('committed')
+    expect(result.data).toMatchObject({ status: 'paid', remaining_amount: 0, journal_entry_id: 'je-1' })
+
+    expect(paidHandler).toHaveBeenCalledTimes(1)
+    expect(paidHandler).toHaveBeenCalledWith(
+      expect.objectContaining({
+        companyId: 'company-1',
+        userId: 'user-1',
+        paymentAmount: 525,
+        invoice: expect.objectContaining({ id: 'inv-1', status: 'paid', remaining_amount: 0, paid_amount: 525 }),
+      }),
+    )
+  })
+})
@@ -1017,6 +1017,29 @@ async function commitMarkInvoicePaid(
  const invoiceAlreadyBooked = !!(invoice as { journal_entry_id?: string | null }).journal_entry_id
  const useCashEntry = !invoiceAlreadyBooked && accountingMethod === 'cash'

+  // Paid/remaining/status math + overpayment guard via the shared
+  // planInvoicePayment helper — the single source of truth across the three
+  // mark-paid surfaces (this agent path, the dashboard route, and the v1 API).
+  // This path settles the full remaining (no custom lines), so it can never
+  // overpay, but routing through the helper keeps the state identical. Runs
+  // BEFORE the JE below so a rejected payment never burns a voucher number.
+  // Settle the full outstanding balance. Prefer remaining_amount; for legacy rows
+  // where it was never written, derive it from total − paid_amount rather than
+  // falling back to the full total (which would double-count a prior partial
+  // payment and trip the overpayment guard).
+  const inv = invoice as { remaining_amount?: number | null; paid_amount?: number | null }
+  const paymentAmount = inv.remaining_amount ?? (invoice.total - (inv.paid_amount ?? 0))
+  const payment = planInvoicePayment(invoice, paymentAmount)
+  if (!payment.ok) {
+    return {
+      error:
+        getErrorEntry('MATCH_AMOUNT_EXCEEDS_REMAINING')?.message_sv ??
+        'Betalningsbeloppet är större än fakturans återstående belopp.',
+      status: 400,
+    }
+  }
+  const { newPaidAmount, newRemaining, newStatus } = payment.plan
+
  if (isRealInvoice) {
    if (useCashEntry) {
      const je = await createInvoiceCashEntry(
@@ -1049,10 +1072,15 @@ async function commitMarkInvoicePaid(
  // invoice no-ops here instead of double-booking the payment.
  const { data: updateResult, error: updateError } = await supabase
    .from('invoices')
-    .update({ status: 'paid', paid_at: now, paid_amount: invoice.total })
+    .update({
+      status: newStatus,
+      paid_amount: newPaidAmount,
+      remaining_amount: newRemaining,
+      ...(newStatus === 'paid' ? { paid_at: now } : {}),
+    })
    .eq('id', invoiceId)
    .eq('company_id', companyId)
-    .in('status', ['sent', 'overdue'])
+    .in('status', ['sent', 'overdue', 'partially_paid'])
    .select('id')

  if (updateError) {
@@ -1078,12 +1106,37 @@ async function commitMarkInvoicePaid(
      )
    }
    return {
-      error: 'Invoice can only be marked as paid when status is "sent" or "overdue"',
+      error: 'Invoice can only be marked as paid from a payable status (sent, overdue or partially paid)',
      status: 409,
    }
  }

-  return { data: { status: 'paid', journal_entry_id: journalEntryId } }
+  // Notify subscribers — invoice.paid fans out to registered webhooks
+  // (lib/webhooks/handler.ts). Best-effort: the payment is already committed,
+  // so an emit failure must not fail the operation. Parity with the v1 and
+  // dashboard mark-paid routes, which previously emitted while this path did not.
+  try {
+    await eventBus.emit({
+      type: 'invoice.paid',
+      payload: {
+        invoice: {
+          ...(invoice as Invoice),
+          status: newStatus,
+          paid_amount: newPaidAmount,
+          remaining_amount: newRemaining,
+          paid_at: newStatus === 'paid' ? now : (invoice as Invoice).paid_at,
+        } as Invoice,
+        companyId,
+        userId,
+        paymentAmount,
+        paymentDate,
+      },
+    })
+  } catch (err) {
+    log.warn('invoice.paid emit failed', err)
+  }
+
+  return { data: { status: newStatus, remaining_amount: newRemaining, journal_entry_id: journalEntryId } }
 }

 async function commitSendInvoice(