feat(transactions): match overshoot guards + supplier voucher linking (#602)

* feat(transactions): match overshoot guards + supplier voucher linking Three changes that together close the "I can't link a bank transaction to an already-booked verifikat on the supplier side" gap and fix a latent data-corruption bug on the per-tx match endpoints. 1. fix: clamp paid_amount on match endpoints when tx > remaining /api/transactions/[id]/match-{invoice,supplier-invoice} previously used transaction.amount wholesale as the paid amount, pushing invoice.paid_amount past invoice.total whenever the bank tx was larger than what was owed. Both endpoints now reject with MATCH_AMOUNT_EXCEEDS_REMAINING / MATCH_SI_AMOUNT_EXCEEDS_REMAINING and a structured { transaction_amount, remaining_amount, excess } payload that points the user at the future split-payment flow. FX branch already clamps to invoice.remaining_amount and is unchanged. 2. feat: supplier-side "link existing verifikat" (mirror of #591) lib/invoices/supplier-voucher-matching.ts mirrors the customer voucher-matching module: finds posted JEs that debit 2440 (Leverantörsskulder), validates currency + remaining-amount, and atomically links them as supplier_invoice_payments rows. New /api/supplier-invoices/[id]/{voucher-candidates,link-to-voucher} routes wrap it. LinkVoucherPicker gains a mode='supplier_invoice' prop so the same component renders both flows. The supplier-invoice mark-paid dialog now uses Tabs ("Ny betalning" / "Befintlig verifikation") to match the customer-side UX. 3. infra: transaction_voucher_links junction + denorm guard Foundation migration for upcoming multi-tx ↔ multi-voucher flows. Adds the junction table (with RLS, updated_at, indexes), a block_contradictory_invoice_denorm trigger on transactions that refuses to set invoice_id/supplier_invoice_id to a value that contradicts an existing payment row, and is_transaction_booked(uuid) as a single source of truth for "is this tx anchored?" once multi-allocation leaves denorm columns NULL. No application code uses these yet — they unlock the batch allocation and bulk-book flows in follow-up PRs. Tests: 98 unit tests pass across the touched paths (match-invoice, match-supplier-invoice, supplier-voucher-matching, link-to-voucher). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(supplier-invoices): PR review — atomic link RPC, computeRemaining edge case, pg-real tests Addresses the three real issues raised by Greptile on PR #602. 1. (P1) Atomic supplier voucher linking — new link_supplier_invoice_to_voucher PL/pgSQL RPC. The TS-side linkSupplierInvoiceToVoucher() previously did UPDATE-then-INSERT with a manual unconditional rollback. Under concurrent linking against the same invoice, request A's rollback could overwrite a sibling B's successful write while leaving B's payment row in place. Moving both writes into a single PG transaction (one RPC call) lets PG's own rollback handle the failure path correctly. TS wrapper now just translates the structured RPC return into the lib's Result type. 2. (P1) pg-real tests — tests/pg/transaction_voucher_links.pg.test.ts. CLAUDE.md mandates *.pg.test.ts for any PR adding a trigger, RPC, or RLS. The Phase 1A foundation migration added all three but had no pg-real coverage. Tests now cover: - trg_block_contradictory_invoice_denorm refusing contradictory UPDATEs on invoice_id and supplier_invoice_id - the same trigger PERMITTING a matching UPDATE (no false positives) - is_transaction_booked() returning true via journal_entry_id, via invoice_payments, and via transaction_voucher_links rows. 3. (P2) computeRemaining edge case — trust remaining_amount whenever the column is non-null (including the legitimate 0 for fully-paid invoices). The old "> 0" guard fell through to total - paid_amount, which under rounding drift could compute a tiny positive residue and slip a fully-paid invoice past LINK_SI_VOUCHER_INVOICE_FULLY_PAID. The fourth Greptile comment (overdue invoices silently get no candidates) was a misread: 'overdue' IS in the open-state list at route.ts:35. No code change needed there. Tests: 100 unit tests pass (16 in the directly-touched paths). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(supplier-invoices): PR review round 2 — broaden AP range, log event failures Addresses the actionable findings from the compliance-swarm and Swedish-accounting-compliance bot reviews on PR #602. 1. (swedish-accounting-compliance, high) AP account hardcoded to 2440 rejected legitimate samlingsverifikationer that debit 2441 (Leverantörsskulder i utländsk valuta), 2443 (Skuldfakturor), etc. BAS 2026 reserves the full 2440–2449 range for Leverantörsskulder. The TS-side AP_ACCOUNT constant becomes AP_ACCOUNT_PREFIX ('244') used with .like() and .startsWith(). The PL/pgSQL RPC's account_number filter becomes LIKE '244%'. The LINK_SI_VOUCHER_NO_AP_DEBIT error message updates to reference the 244x range with examples. 2. (ISO 27001:2022 A.8.15 / OWASP V16) Empty catch on the supplier_invoice.paid event emission now logs with log.warn so a failure in the downstream reminder/audit subscriber leaves an auditable trail without blocking the response. 3. (GDPR Art.5(1)(c)) Documented design rationale for retaining select('*') on the post-link invoice re-fetch: the supplier_invoice.paid event payload is typed as `supplierInvoice: SupplierInvoice` in lib/events/types.ts, narrowing would break the subscriber contract. The event stays in-process and consumers legitimately need the full context. Skipped findings: - V8.2.1 ownership concerns: route + RPC already filter by company_id from withRouteContext; the RPC's WHERE clause covers it. - DELETE policy scoping: matches the gnubok pattern across all company-scoped tables — any member with write access manages records. - transaction_id = NULL on the voucher-link path: by design — the flow has no bank tx (the voucher's 1930 line represents it). - Reverse-charge VAT (2614/2647) validation on linked vouchers: real concern but invasive change; tracked for follow-up. - Storno-chain integrity (linking the original of a storno pair): edge case; tracked for follow-up. Tests: 26 unit tests pass in the directly-touched paths. RPC patch applied to remote via Supabase MCP. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-29 12:31:19 +02:00
parent a586cc8a58
commit 7bcd46d503
19 changed files with 2510 additions and 44 deletions
@@ -0,0 +1,106 @@
+import { describe, it, expect, beforeEach, vi } from 'vitest'
+import {
+  createMockRequest,
+  createMockRouteParams,
+  parseJsonResponse,
+  createQueuedMockSupabase,
+} from '@/tests/helpers'
+
+const { supabase: mockSupabase, reset } = createQueuedMockSupabase()
+vi.mock('@/lib/supabase/server', () => ({
+  createClient: () => Promise.resolve(mockSupabase),
+}))
+
+const mockLink = vi.fn()
+vi.mock('@/lib/invoices/supplier-voucher-matching', () => ({
+  linkSupplierInvoiceToVoucher: (...args: unknown[]) => mockLink(...args),
+}))
+
+vi.mock('@/lib/init', () => ({
+  ensureInitialized: vi.fn(),
+}))
+
+vi.mock('@/lib/company/context', () => ({
+  requireCompanyId: vi.fn().mockResolvedValue('company-1'),
+  getActiveCompanyId: vi.fn().mockResolvedValue('company-1'),
+}))
+
+vi.mock('@/lib/auth/require-write', () => ({
+  requireWritePermission: vi.fn().mockResolvedValue({ ok: true }),
+}))
+
+import { POST } from '../route'
+
+const VALID_UUID = '550e8400-e29b-41d4-a716-446655440000'
+const VALID_JE_UUID = '550e8400-e29b-41d4-a716-446655440001'
+const mockUser = { id: 'user-1', email: 'test@test.se' }
+
+describe('POST /api/supplier-invoices/[id]/link-to-voucher', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+    reset()
+    mockSupabase.auth.getUser.mockResolvedValue({ data: { user: mockUser } })
+  })
+
+  it('returns 400 when journal_entry_id is missing', async () => {
+    const request = createMockRequest(`/api/supplier-invoices/${VALID_UUID}/link-to-voucher`, {
+      method: 'POST',
+      body: {},
+    })
+    const response = await POST(request, createMockRouteParams({ id: VALID_UUID }))
+    expect(response.status).toBe(400)
+  })
+
+  it('returns 200 with the linked payment payload on success', async () => {
+    mockLink.mockResolvedValue({
+      ok: true,
+      result: {
+        paymentId: 'sip-1',
+        invoiceStatus: 'paid',
+        paidAmount: 1000,
+        remainingAmount: 0,
+        paymentAmount: 1000,
+        journalEntryId: VALID_JE_UUID,
+      },
+    })
+
+    const request = createMockRequest(`/api/supplier-invoices/${VALID_UUID}/link-to-voucher`, {
+      method: 'POST',
+      body: { journal_entry_id: VALID_JE_UUID },
+    })
+    const response = await POST(request, createMockRouteParams({ id: VALID_UUID }))
+    const { status, body } = await parseJsonResponse<{
+      data: {
+        invoice_status: string
+        paid_amount: number
+        remaining_amount: number
+        payment_amount: number
+        payment_id: string
+        journal_entry_id: string
+      }
+    }>(response)
+    expect(status).toBe(200)
+    expect(body.data.invoice_status).toBe('paid')
+    expect(body.data.paid_amount).toBe(1000)
+    expect(body.data.remaining_amount).toBe(0)
+    expect(body.data.payment_id).toBe('sip-1')
+    expect(body.data.journal_entry_id).toBe(VALID_JE_UUID)
+  })
+
+  it('maps a structured failure code to the correct HTTP status', async () => {
+    mockLink.mockResolvedValue({
+      ok: false,
+      code: 'LINK_SI_VOUCHER_NO_AP_DEBIT',
+      details: { source_type: 'opening_balance' },
+    })
+
+    const request = createMockRequest(`/api/supplier-invoices/${VALID_UUID}/link-to-voucher`, {
+      method: 'POST',
+      body: { journal_entry_id: VALID_JE_UUID },
+    })
+    const response = await POST(request, createMockRouteParams({ id: VALID_UUID }))
+    const { status, body } = await parseJsonResponse<{ error: { code: string } }>(response)
+    expect(status).toBe(400)
+    expect(body.error.code).toBe('LINK_SI_VOUCHER_NO_AP_DEBIT')
+  })
+})
@@ -0,0 +1,60 @@
+import { NextResponse } from 'next/server'
+import { withRouteContext } from '@/lib/api/with-route-context'
+import { validateBody } from '@/lib/api/validate'
+import { LinkSupplierInvoiceToVoucherSchema } from '@/lib/api/schemas'
+import { errorResponseFromCode } from '@/lib/errors/get-structured-error'
+import { linkSupplierInvoiceToVoucher } from '@/lib/invoices/supplier-voucher-matching'
+import { ensureInitialized } from '@/lib/init'
+
+ensureInitialized()
+
+/**
+ * POST /api/supplier-invoices/[id]/link-to-voucher
+ *
+ * Marks a supplier invoice as paid (or partially paid) by linking an existing
+ * posted verifikat whose lines already debit AP (2440). Creates no new journal
+ * entry — only a supplier_invoice_payments row + invoice status advance.
+ *
+ * Rejects with LINK_SI_VOUCHER_NO_AP_DEBIT for vouchers that book the expense
+ * directly without going through 2440 — those require gnubok_correct_entry first.
+ */
+export const POST = withRouteContext(
+  'supplier_invoice.link_to_voucher',
+  async (request, ctx, { params }: { params: Promise<{ id: string }> }) => {
+    const { id } = await params
+    const { user, supabase, companyId, log, requestId } = ctx
+    const opLog = log.child({ supplierInvoiceId: id })
+
+    const validation = await validateBody(request, LinkSupplierInvoiceToVoucherSchema, {
+      log: opLog,
+      operation: 'supplier_invoice.link_to_voucher',
+    })
+    if (!validation.success) return validation.response
+    const { journal_entry_id, notes } = validation.data
+
+    const outcome = await linkSupplierInvoiceToVoucher(supabase, user.id, companyId, {
+      supplierInvoiceId: id,
+      journalEntryId: journal_entry_id,
+      notes,
+    })
+
+    if (!outcome.ok) {
+      return errorResponseFromCode(outcome.code, opLog, {
+        requestId,
+        details: outcome.details,
+      })
+    }
+
+    return NextResponse.json({
+      data: {
+        invoice_status: outcome.result.invoiceStatus,
+        paid_amount: outcome.result.paidAmount,
+        remaining_amount: outcome.result.remainingAmount,
+        payment_amount: outcome.result.paymentAmount,
+        payment_id: outcome.result.paymentId,
+        journal_entry_id: outcome.result.journalEntryId,
+      },
+    })
+  },
+  { requireWrite: true },
+)
@@ -0,0 +1,50 @@
+import { NextResponse } from 'next/server'
+import { withRouteContext } from '@/lib/api/with-route-context'
+import { errorResponseFromCode } from '@/lib/errors/get-structured-error'
+import { findMatchingVouchersForSupplierInvoice } from '@/lib/invoices/supplier-voucher-matching'
+import type { Supplier, SupplierInvoice } from '@/types'
+
+/**
+ * GET /api/supplier-invoices/[id]/voucher-candidates
+ *
+ * Returns posted verifikat candidates that could be linked as payment for
+ * this supplier invoice. Used by the "Befintlig verifikation" tab in the
+ * supplier-invoice mark-paid dialog to auto-suggest matches.
+ */
+export const GET = withRouteContext(
+  'supplier_invoice.voucher_candidates',
+  async (_request, ctx, { params }: { params: Promise<{ id: string }> }) => {
+    const { id } = await params
+    const { supabase, companyId, log, requestId } = ctx
+
+    // Project only the fields the matcher actually reads. Avoids leaking the
+    // full supplier row (bank details, contact info, etc.) into the response.
+    const { data: invoice, error } = await supabase
+      .from('supplier_invoices')
+      .select(
+        'id, supplier_invoice_number, arrival_number, status, currency, total, paid_amount, remaining_amount, due_date, paid_at, exchange_rate, supplier_id, supplier:suppliers(id, name)',
+      )
+      .eq('id', id)
+      .eq('company_id', companyId)
+      .single()
+
+    if (error || !invoice) {
+      return errorResponseFromCode('LINK_SI_VOUCHER_INVOICE_NOT_FOUND', log, { requestId })
+    }
+
+    if (!['registered', 'approved', 'overdue', 'partially_paid'].includes(invoice.status)) {
+      return NextResponse.json({ data: { candidates: [], invoice_status: invoice.status } })
+    }
+
+    const candidates = await findMatchingVouchersForSupplierInvoice(
+      supabase,
+      companyId,
+      // The narrow projection above means TS infers `supplier` as `{ id, name }[]`
+      // from the join shorthand. The matcher only reads `supplier?.name`, so
+      // cast through unknown to the runtime shape it expects.
+      invoice as unknown as SupplierInvoice & { supplier?: Supplier },
+    )
+
+    return NextResponse.json({ data: { candidates } })
+  },
+)
@@ -393,6 +393,40 @@ describe('POST /api/transactions/[id]/match-invoice', () => {
    expect(body.remaining_amount).toBe(7500)
  })

+  it('returns 400 MATCH_AMOUNT_EXCEEDS_REMAINING when tx amount exceeds invoice remaining', async () => {
+    // Tx is +12 000 SEK, invoice has 5 000 SEK remaining. Legacy code path
+    // would push paid_amount past invoice.total; the new guard rejects so
+    // the user routes the excess through the split-payment flow.
+    const tx = makeTransaction({ id: 'tx-1', amount: 12000, invoice_id: null, date: '2024-06-15' })
+    const invoice = makeInvoice({
+      id: VALID_UUID,
+      status: 'partially_paid',
+      total: 10000,
+      remaining_amount: 5000,
+      paid_amount: 5000,
+    })
+
+    enqueue({ data: tx, error: null })
+    enqueue({ data: invoice, error: null })
+    // Hard-duplicate check is skipped for partially_paid status — no enqueue needed.
+
+    const request = createMockRequest('/api/transactions/tx-1/match-invoice', {
+      method: 'POST',
+      body: { invoice_id: VALID_UUID },
+    })
+    const response = await POST(request, createMockRouteParams({ id: 'tx-1' }))
+    const { status, body } = await parseJsonResponse<{ error: unknown }>(response)
+
+    expect(status).toBe(400)
+    expect((body.error as unknown as { code: string }).code).toBe(
+      'MATCH_AMOUNT_EXCEEDS_REMAINING',
+    )
+    const details = (body.error as unknown as { details: Record<string, number> }).details
+    expect(details.transaction_amount).toBe(12000)
+    expect(details.remaining_amount).toBe(5000)
+    expect(details.excess).toBe(7000)
+  })
+
  it('cash method partial payment uses clearing entry with note', async () => {
    const tx = makeTransaction({ id: 'tx-1', amount: 5000, invoice_id: null, date: '2024-06-15' })
    const invoice = makeInvoice({
@@ -219,8 +219,25 @@ export const POST = withRouteContext(
    const now = new Date().toISOString()
    const paidAmount = transaction.amount

-    const newPaidAmount = Math.round(((invoice.paid_amount || 0) + paidAmount) * 100) / 100
    const currentRemaining = invoice.remaining_amount ?? (invoice.total - (invoice.paid_amount || 0))
+
+    // Overshoot guard: the single-tx match endpoint always books tx.amount in
+    // full against the invoice. If tx > remaining the legacy code path would
+    // push invoice.paid_amount past invoice.total — silently. Reject and
+    // point the user at the split-payment flow which can allocate the excess
+    // across additional invoices.
+    if (paidAmount > currentRemaining + 0.005) {
+      return errorResponseFromCode('MATCH_AMOUNT_EXCEEDS_REMAINING', txLog, {
+        requestId,
+        details: {
+          transaction_amount: paidAmount,
+          remaining_amount: Math.round(currentRemaining * 100) / 100,
+          excess: Math.round((paidAmount - currentRemaining) * 100) / 100,
+        },
+      })
+    }
+
+    const newPaidAmount = Math.round(((invoice.paid_amount || 0) + paidAmount) * 100) / 100
    const newRemaining = Math.max(0, Math.round((currentRemaining - paidAmount) * 100) / 100)
    const isFullyPaid = newRemaining <= 0
    const newStatus = isFullyPaid ? 'paid' : 'partially_paid'
@@ -195,4 +195,56 @@ describe('POST /api/transactions/[id]/match-supplier-invoice — non-FX paths',
    expect(body.paid_amount).toBe(1000)
    expect(body.remaining_amount).toBe(0)
  })
+
+  it('returns 400 MATCH_SI_AMOUNT_EXCEEDS_REMAINING when tx exceeds invoice remaining (same currency)', async () => {
+    // Tx pays out 6 000 SEK, invoice has 5 000 SEK remaining. Legacy code path
+    // would push paid_amount past invoice.total. The new guard rejects so the
+    // user routes the excess through the split-payment flow.
+    enqueue({
+      data: {
+        id: TX_UUID,
+        company_id: 'company-1',
+        amount: -6000,
+        currency: 'SEK',
+        amount_sek: null,
+        supplier_invoice_id: null,
+        date: '2026-05-12',
+      },
+      error: null,
+    })
+    enqueue({
+      data: {
+        id: SI_UUID,
+        currency: 'SEK',
+        exchange_rate: null,
+        status: 'registered',
+        remaining_amount: 5000,
+        paid_amount: 0,
+        supplier: { supplier_type: 'swedish_business' },
+        items: [],
+      },
+      error: null,
+    })
+
+    const res = await POST(makeReq(), createMockRouteParams({ id: TX_UUID }))
+    const { status, body } = await parseJsonResponse<{ error: unknown }>(res)
+    expect(status).toBe(400)
+    expect((body.error as { code: string }).code).toBe('MATCH_SI_AMOUNT_EXCEEDS_REMAINING')
+    const details = (body.error as { details: Record<string, number> }).details
+    expect(details.transaction_amount).toBe(6000)
+    expect(details.remaining_amount).toBe(5000)
+    expect(details.excess).toBe(1000)
+  })
+
+  it('does NOT trigger overshoot guard on currency mismatch (FX path clamps to remaining)', async () => {
+    // SEK transaction paying a EUR invoice. The currency-mismatch branch
+    // collapses paymentAmountInvoiceCurrency to invoice.remaining_amount and
+    // cannot overshoot, so the guard must not fire here.
+    enqueueHappyPath({
+      transaction: { amount: -10000, currency: 'SEK' },
+      invoice: { currency: 'EUR', remaining_amount: 200, exchange_rate: 11.5 },
+    })
+    const res = await POST(makeReq(), createMockRouteParams({ id: TX_UUID }))
+    expect(res.status).toBe(200)
+  })
 })
@@ -81,6 +81,27 @@ export const POST = withRouteContext(

    const txAmountAbs = Math.abs(transaction.amount)

+    // Overshoot guard for the same-currency branch. The legacy code path used
+    // txAmountAbs wholesale and would push supplier_invoices.paid_amount past
+    // invoice.total whenever the bank transaction was larger than what was
+    // owed. Reject and direct the user at the split-payment flow which can
+    // allocate the excess to additional supplier invoices.
+    // FX branch (currency mismatch) is already clamped below to
+    // invoice.remaining_amount, so it cannot overshoot.
+    if (
+      transaction.currency === invoice.currency &&
+      txAmountAbs > invoice.remaining_amount + 0.005
+    ) {
+      return errorResponseFromCode('MATCH_SI_AMOUNT_EXCEEDS_REMAINING', txLog, {
+        requestId,
+        details: {
+          transaction_amount: txAmountAbs,
+          remaining_amount: Math.round(invoice.remaining_amount * 100) / 100,
+          excess: Math.round((txAmountAbs - invoice.remaining_amount) * 100) / 100,
+        },
+      })
+    }
+
    // Amount in the *invoice's* currency — used to update
    // supplier_invoices.paid_amount/remaining_amount and the
    // supplier_invoice_payments row (whose `currency` is the invoice's).