fix(supplier-invoices): guard against duplicate payment when bank tx already booked (#461)

* fix(supplier-invoices): guard against duplicate payment when bank tx already booked Two-pronged fix for a UX trap where a supplier invoice could be marked paid even though the bank payment was already booked on 2440, creating a duplicate verifikation. Prong A — mark-paid duplicate guard: before booking, scan for an unlinked outgoing bank transaction matching this supplier (merchant_name ILIKE) within ±2% / ±60 days. If found, return 409 SI_PAID_LIKELY_DUPLICATE with candidates so the UI can offer "link existing" instead. Override via { force: true }. Prong B — categorize match suggestion: when the user assigns 2440 directly on a negative business transaction and an open supplier invoice from the same supplier covers the same amount, return 409 TX_CATEGORIZE_SUGGEST_SI_MATCH with candidates and route the user to match-supplier-invoice. Override via { confirm_no_match: true }. Frontend dialogs added on the supplier-invoice detail page and the transactions inbox. Partial payments skip the mark-paid guard (deliberate action). Tests cover the 409 path, the override path, and the no-candidates happy path on both routes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(supplier-invoices): apply PR review fixes to duplicate-payment guards - Add `is_business = true` filter to mark-paid candidate query so private bank withdrawals don't surface as false-positive duplicates - Escape LIKE wildcards (`%`, `_`, `\`) in both ILIKE patterns to avoid silent over-matching when a supplier/merchant name contains those chars - Round paymentAmount and remaining_amount to 2 decimals before the partial-payment guard comparison to avoid float-equality fragility - Require credit account to be in the 1xxx (bank/cash) series for the Prong B 2440 intercept so 2440 against clearing/equity accounts isn't misinterpreted as a supplier payment - Extract DUPLICATE_AMOUNT_TOLERANCE_PCT (0.02) and DUPLICATE_DATE_WINDOW_DAYS (60) into a shared helper module with the LIKE-escape utility Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(supplier-invoices): broaden 244x match, audit log overrides, drop JE id from response Second-round PR review fixes: - Widen Prong B regex from /^2440$/ to /^244\d$/ so payments mapped to BAS sub-accounts (e.g. 2441 leverantörsskulder i utländsk valuta) also trigger the suggestion (swedish-invoice-compliance bot) - Log a structured warning when force=true or confirm_no_match=true is honored, with the relevant context (amount, date, accounts) so the override is traceable per BFNAR 2013:2 kap 8 (behandlingshistorik) - Drop journal_entry_id from the SI_PAID_LIKELY_DUPLICATE candidate response payload (data minimization, GDPR Art.5(1)(c)); the UI never rendered it Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(supplier-invoices): third-round PR review — date window on Prong B, length cap, VAT message - Add the missing date window to the Prong B categorize candidate query (swedish-compliance bot): without it, an open invoice from years back can surface as a "match" for an unrelated bank transaction. Uses the shared DUPLICATE_DATE_WINDOW_DAYS against invoice_date. - Cap supplier/merchant names to 200 chars before they enter escapeLikePattern (OWASP V1.2.5 / ISO A.8.28). Bounds DB work on pathological inputs. - Log a structured warning when the mark-paid guard is skipped because the invoice has no resolved supplier name (BFL 5 kap 7 § — motpart should be identifiable; the absence is itself worth surfacing). - Update the SI-match suggestion error and the matching UI copy to call out the actual compliance risk: a duplicate 244x posting double-deducts ingående moms (ML 8 kap 3 §), not just bookkeeping symmetry. - Reword "Bokför på 2440 ändå" to "Bokför på leverantörsskulder ändå" now that the regex covers BAS sub-accounts 244x. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(supplier-invoices): correct Prong B framing — duplicate verifikation, not VAT double-deduction Latest swedish-compliance review correctly walked back the earlier finding that asked for ML 8 kap 3 § VAT framing. Plain 244x categorization via account_override does not include VAT lines (account class 2), so the risk is a duplicate verifikation (BFL 5 kap 5 §), not a double VAT deduction. Update both the structured error message and the dialog body to reflect the actual mechanism. 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-13 11:38:19 +02:00
parent 01e99d3220
commit 07a7964e8d
9 changed files with 673 additions and 4 deletions
@@ -111,6 +111,8 @@ describe('POST /api/supplier-invoices/[id]/mark-paid', () => {

    // Fetch invoice
    enqueue({ data: invoice, error: null })
+    // Duplicate-payment guard: no candidate transactions
+    enqueue({ data: [], error: null })
    // Fetch company settings
    enqueue({ data: { accounting_method: 'accrual' }, error: null })

@@ -212,6 +214,8 @@ describe('POST /api/supplier-invoices/[id]/mark-paid', () => {
    })

    enqueue({ data: invoice, error: null })
+    // Duplicate-payment guard: no candidate transactions
+    enqueue({ data: [], error: null })
    enqueue({ data: { accounting_method: 'cash' }, error: null })

    mockCreateSupplierInvoiceCashEntry.mockResolvedValue({ id: 'je-3' })
@@ -249,6 +253,8 @@ describe('POST /api/supplier-invoices/[id]/mark-paid', () => {
    })

    enqueue({ data: invoice, error: null })
+    // Duplicate-payment guard: no candidate transactions
+    enqueue({ data: [], error: null })
    enqueue({ data: { accounting_method: 'accrual' }, error: null })

    mockCreateSupplierInvoicePaymentEntry.mockRejectedValue(new Error('Period locked'))
@@ -264,6 +270,109 @@ describe('POST /api/supplier-invoices/[id]/mark-paid', () => {
    expect((body.error as unknown as { code: string }).code).toBe('SI_PAID_FAILED')
  })

+  it('returns 409 SI_PAID_LIKELY_DUPLICATE when an unlinked transaction matches', async () => {
+    const supplier = makeSupplier()
+    const invoice = makeSupplierInvoice({
+      id: 'si-1',
+      status: 'approved',
+      total: 10000,
+      remaining_amount: 10000,
+      paid_amount: 0,
+      supplier,
+      items: [],
+    })
+
+    enqueue({ data: invoice, error: null })
+    // Duplicate-payment guard: one likely-matching unlinked transaction
+    enqueue({
+      data: [
+        {
+          id: 'tx-99',
+          date: '2026-05-10',
+          amount: -10000,
+          description: 'Faktura Leverantör AB',
+          merchant_name: 'Leverantör AB',
+          journal_entry_id: 'je-99',
+        },
+      ],
+      error: null,
+    })
+
+    const request = createMockRequest('/api/supplier-invoices/si-1/mark-paid', {
+      method: 'POST',
+      body: {},
+    })
+    const response = await POST(request, createMockRouteParams({ id: 'si-1' }))
+    const { status, body } = await parseJsonResponse<{ error: { code: string; details: { candidates: unknown[] } } }>(response)
+
+    expect(status).toBe(409)
+    expect(body.error.code).toBe('SI_PAID_LIKELY_DUPLICATE')
+    expect(body.error.details.candidates).toHaveLength(1)
+    expect(mockCreateSupplierInvoicePaymentEntry).not.toHaveBeenCalled()
+  })
+
+  it('proceeds when force=true even with candidates present', async () => {
+    const supplier = makeSupplier()
+    const invoice = makeSupplierInvoice({
+      id: 'si-1',
+      status: 'approved',
+      total: 10000,
+      remaining_amount: 10000,
+      paid_amount: 0,
+      supplier,
+      items: [],
+    })
+
+    enqueue({ data: invoice, error: null })
+    // No candidates query happens because force=true skips it
+    enqueue({ data: { accounting_method: 'accrual' }, error: null })
+    mockCreateSupplierInvoicePaymentEntry.mockResolvedValue({ id: 'je-1' })
+    enqueue({ data: [{ id: 'si-1' }], error: null })
+    enqueue({ data: null, error: null })
+
+    const request = createMockRequest('/api/supplier-invoices/si-1/mark-paid', {
+      method: 'POST',
+      body: { force: true },
+    })
+    const response = await POST(request, createMockRouteParams({ id: 'si-1' }))
+    const { status, body } = await parseJsonResponse<{ success: boolean; status: string }>(response)
+
+    expect(status).toBe(200)
+    expect(body.success).toBe(true)
+    expect(body.status).toBe('paid')
+    expect(mockCreateSupplierInvoicePaymentEntry).toHaveBeenCalled()
+  })
+
+  it('skips duplicate guard on partial payment (amount < remaining)', async () => {
+    const supplier = makeSupplier()
+    const invoice = makeSupplierInvoice({
+      id: 'si-1',
+      status: 'approved',
+      total: 10000,
+      remaining_amount: 10000,
+      paid_amount: 0,
+      supplier,
+      items: [],
+    })
+
+    // Note: no candidates enqueue — guard is skipped for partial payments
+    enqueue({ data: invoice, error: null })
+    enqueue({ data: { accounting_method: 'accrual' }, error: null })
+    mockCreateSupplierInvoicePaymentEntry.mockResolvedValue({ id: 'je-1' })
+    enqueue({ data: [{ id: 'si-1' }], error: null })
+    enqueue({ data: null, error: null })
+
+    const request = createMockRequest('/api/supplier-invoices/si-1/mark-paid', {
+      method: 'POST',
+      body: { amount: 3000 },
+    })
+    const response = await POST(request, createMockRouteParams({ id: 'si-1' }))
+    const { status, body } = await parseJsonResponse<{ status: string }>(response)
+
+    expect(status).toBe(200)
+    expect(body.status).toBe('partially_paid')
+  })
+
  it('emits supplier_invoice.paid event', async () => {
    const supplier = makeSupplier()
    const invoice = makeSupplierInvoice({
@@ -277,6 +386,8 @@ describe('POST /api/supplier-invoices/[id]/mark-paid', () => {
    })

    enqueue({ data: invoice, error: null })
+    // Duplicate-payment guard: no candidate transactions
+    enqueue({ data: [], error: null })
    enqueue({ data: { accounting_method: 'accrual' }, error: null })
    mockCreateSupplierInvoicePaymentEntry.mockResolvedValue({ id: 'je-1' })
    // Update invoice (CAS guard: returns matched row)
@@ -10,6 +10,11 @@ import { validateBody } from '@/lib/api/validate'
 import { MarkSupplierInvoicePaidSchema } from '@/lib/api/schemas'
 import { withRouteContext } from '@/lib/api/with-route-context'
 import { errorResponse, errorResponseFromCode } from '@/lib/errors/get-structured-error'
+import {
+  DUPLICATE_AMOUNT_TOLERANCE_PCT,
+  DUPLICATE_DATE_WINDOW_DAYS,
+  escapeLikePattern,
+} from '@/lib/invoices/duplicate-payment-guard'
 import type { SupplierInvoice, SupplierInvoiceItem } from '@/types'

 ensureInitialized()
@@ -50,6 +55,73 @@ export const POST = withRouteContext(
    const paymentAmount = body.amount || invoice.remaining_amount
    const now = new Date().toISOString()

+    if (body.force) {
+      opLog.warn('duplicate-payment guard bypassed', {
+        reason: 'force=true',
+        paymentAmount,
+        paymentDate,
+      })
+    }
+
+    // Duplicate-payment guard: if a likely-matching unlinked bank transaction
+    // exists for this supplier, surface it before booking a new payment entry.
+    // Caller can override with `force: true`. Skipped on partial payments —
+    // those are an explicit, deliberate action.
+    const paidRounded = Math.round(paymentAmount * 100) / 100
+    const remainingRounded = Math.round(invoice.remaining_amount * 100) / 100
+    if (!body.force && paidRounded >= remainingRounded) {
+      const supplierName = (invoice as SupplierInvoice & { supplier?: { name?: string } })
+        .supplier?.name
+      if (!supplierName) {
+        // An invoice without a resolved supplier name is arguably *higher* risk
+        // for duplicate booking, not lower (BFL 5 kap 7 § — motpart should be
+        // identifiable). Log the skip so the gap is visible in audit.
+        opLog.warn('duplicate-payment guard skipped', {
+          reason: 'missing_supplier_name',
+          supplierInvoiceId: id,
+        })
+      }
+      if (supplierName) {
+        const windowLow = Math.round(paymentAmount * (1 - DUPLICATE_AMOUNT_TOLERANCE_PCT) * 100) / 100
+        const windowHigh = Math.round(paymentAmount * (1 + DUPLICATE_AMOUNT_TOLERANCE_PCT) * 100) / 100
+        const dateMs = new Date(paymentDate).getTime()
+        const dateLow = new Date(dateMs - DUPLICATE_DATE_WINDOW_DAYS * 24 * 3600 * 1000).toISOString().split('T')[0]
+        const dateHigh = new Date(dateMs + DUPLICATE_DATE_WINDOW_DAYS * 24 * 3600 * 1000).toISOString().split('T')[0]
+        const escapedSupplierName = escapeLikePattern(supplierName)
+
+        const { data: candidates } = await supabase
+          .from('transactions')
+          .select('id, date, amount, description, merchant_name')
+          .eq('company_id', companyId!)
+          .eq('is_business', true)
+          .is('supplier_invoice_id', null)
+          .is('invoice_id', null)
+          .lt('amount', 0)
+          .gte('amount', -windowHigh)
+          .lte('amount', -windowLow)
+          .gte('date', dateLow)
+          .lte('date', dateHigh)
+          .ilike('merchant_name', `%${escapedSupplierName}%`)
+          .order('date', { ascending: false })
+          .limit(5)
+
+        if (candidates && candidates.length > 0) {
+          return errorResponseFromCode('SI_PAID_LIKELY_DUPLICATE', opLog, {
+            requestId,
+            details: {
+              candidates: candidates.map((c) => ({
+                id: c.id,
+                date: c.date,
+                amount: c.amount,
+                description: c.description,
+                merchant_name: c.merchant_name,
+              })),
+            },
+          })
+        }
+      }
+    }
+
    const { data: settings } = await supabase
      .from('company_settings')
      .select('accounting_method')
@@ -271,6 +271,130 @@ describe('POST /api/transactions/[id]/categorize', () => {
    expect(mockCreateTransactionJournalEntry).not.toHaveBeenCalled()
  })

+  it('returns 409 TX_CATEGORIZE_SUGGEST_SI_MATCH when 2440 mapping matches an open supplier invoice', async () => {
+    const tx = makeTransaction({
+      id: 'tx-1',
+      amount: -10000,
+      merchant_name: 'Leverantör AB',
+      journal_entry_id: null,
+    })
+
+    enqueue({ data: tx, error: null })
+    enqueue({ data: { entity_type: 'enskild_firma', fiscal_year_start_month: 1 }, error: null })
+
+    mockBuildMappingResultFromCategory.mockReturnValue({
+      ...defaultMappingResult,
+      debit_account: '2440',
+    })
+
+    // Prong B: supplier lookup
+    enqueue({ data: [{ id: 'sup-1' }], error: null })
+    // Open supplier invoices candidate query
+    enqueue({
+      data: [
+        {
+          id: 'si-1',
+          supplier_invoice_number: 'INV-2026-0042',
+          invoice_date: '2026-05-01',
+          remaining_amount: 10000,
+          currency: 'SEK',
+          supplier: { name: 'Leverantör AB' },
+        },
+      ],
+      error: null,
+    })
+
+    const request = createMockRequest('/api/transactions/tx-1/categorize', {
+      method: 'POST',
+      body: { is_business: true, category: 'expense_software' },
+    })
+    const response = await POST(request, createMockRouteParams({ id: 'tx-1' }))
+    const { status, body } = await parseJsonResponse<{ error: { code: string; details: { candidates: unknown[] } } }>(response)
+
+    expect(status).toBe(409)
+    expect(body.error.code).toBe('TX_CATEGORIZE_SUGGEST_SI_MATCH')
+    expect(body.error.details.candidates).toHaveLength(1)
+    expect(mockCreateTransactionJournalEntry).not.toHaveBeenCalled()
+  })
+
+  it('proceeds with 2440 categorization when confirm_no_match=true', async () => {
+    const tx = makeTransaction({
+      id: 'tx-1',
+      amount: -10000,
+      merchant_name: 'Leverantör AB',
+      journal_entry_id: null,
+    })
+
+    enqueue({ data: tx, error: null })
+    enqueue({ data: { entity_type: 'enskild_firma', fiscal_year_start_month: 1 }, error: null })
+
+    mockBuildMappingResultFromCategory.mockReturnValue({
+      ...defaultMappingResult,
+      debit_account: '2440',
+    })
+
+    // No supplier/invoice lookups happen because confirm_no_match=true skips the block
+    // ensureFiscalPeriod
+    enqueue({ data: [{ id: 'period-1' }], error: null })
+    mockCreateTransactionJournalEntry.mockResolvedValue({ id: 'je-1' })
+    // Transaction update
+    enqueue({ data: [{ id: 'tx-1' }], error: null })
+
+    const request = createMockRequest('/api/transactions/tx-1/categorize', {
+      method: 'POST',
+      body: { is_business: true, category: 'expense_software', confirm_no_match: true },
+    })
+    const response = await POST(request, createMockRouteParams({ id: 'tx-1' }))
+    const { status, body } = await parseJsonResponse<{
+      success: boolean
+      journal_entry_created: boolean
+      journal_entry_id: string
+    }>(response)
+
+    expect(status).toBe(200)
+    expect(body.success).toBe(true)
+    expect(body.journal_entry_created).toBe(true)
+    expect(body.journal_entry_id).toBe('je-1')
+  })
+
+  it('does not trigger SI suggestion when 2440 has no matching open supplier invoice', async () => {
+    const tx = makeTransaction({
+      id: 'tx-1',
+      amount: -10000,
+      merchant_name: 'Leverantör AB',
+      journal_entry_id: null,
+    })
+
+    enqueue({ data: tx, error: null })
+    enqueue({ data: { entity_type: 'enskild_firma', fiscal_year_start_month: 1 }, error: null })
+
+    mockBuildMappingResultFromCategory.mockReturnValue({
+      ...defaultMappingResult,
+      debit_account: '2440',
+    })
+
+    // Supplier lookup returns a supplier
+    enqueue({ data: [{ id: 'sup-1' }], error: null })
+    // No open invoices in the amount window
+    enqueue({ data: [], error: null })
+    // ensureFiscalPeriod
+    enqueue({ data: [{ id: 'period-1' }], error: null })
+    mockCreateTransactionJournalEntry.mockResolvedValue({ id: 'je-1' })
+    // Transaction update
+    enqueue({ data: [{ id: 'tx-1' }], error: null })
+
+    const request = createMockRequest('/api/transactions/tx-1/categorize', {
+      method: 'POST',
+      body: { is_business: true, category: 'expense_software' },
+    })
+    const response = await POST(request, createMockRouteParams({ id: 'tx-1' }))
+    const { status, body } = await parseJsonResponse<{ success: boolean; journal_entry_created: boolean }>(response)
+
+    expect(status).toBe(200)
+    expect(body.success).toBe(true)
+    expect(body.journal_entry_created).toBe(true)
+  })
+
  it('categorizes as private when is_business is false', async () => {
    const tx = makeTransaction({
      id: 'tx-1',
@@ -9,6 +9,11 @@ import { saveUserMappingRule } from '@/lib/bookkeeping/mapping-engine'
 import { upsertCounterpartyTemplate, buildMappingResultFromCounterpartyTemplate } from '@/lib/bookkeeping/counterparty-templates'
 import { withRouteContext } from '@/lib/api/with-route-context'
 import { errorResponse, errorResponseFromCode } from '@/lib/errors/get-structured-error'
+import {
+  DUPLICATE_AMOUNT_TOLERANCE_PCT,
+  DUPLICATE_DATE_WINDOW_DAYS,
+  escapeLikePattern,
+} from '@/lib/invoices/duplicate-payment-guard'
 import { isBookkeepingError } from '@/lib/bookkeeping/errors'
 import { getErrorMessage } from '@/lib/errors/get-error-message'
 import type { Logger } from '@/lib/logger'
@@ -262,6 +267,86 @@ export const POST = withRouteContext(
      })
    }

+    if (body.confirm_no_match && /^244\d$/.test(mappingResult.debit_account)) {
+      txLog.warn('supplier-invoice match suggestion bypassed', {
+        reason: 'confirm_no_match=true',
+        debitAccount: mappingResult.debit_account,
+        creditAccount: mappingResult.credit_account,
+      })
+    }
+
+    // Prong B: intercept plain 244x categorization of supplier payments when
+    // an open supplier invoice already covers this amount. Categorizing direct
+    // to 244x leaves the invoice with status='approved' and lures the user
+    // into a duplicate "Markera som betald" later. Credit must be a bank/cash
+    // account (1xxx) — 244x against a clearing account, equity, etc. isn't a
+    // supplier payment and the suggestion would misdirect the user.
+    if (
+      !body.confirm_no_match &&
+      is_business &&
+      transaction.amount < 0 &&
+      /^244\d$/.test(mappingResult.debit_account) &&
+      /^1\d{3}$/.test(mappingResult.credit_account)
+    ) {
+      const txAmountAbs = Math.abs(transaction.amount)
+      const windowLow = Math.round(txAmountAbs * (1 - DUPLICATE_AMOUNT_TOLERANCE_PCT) * 100) / 100
+      const windowHigh = Math.round(txAmountAbs * (1 + DUPLICATE_AMOUNT_TOLERANCE_PCT) * 100) / 100
+
+      let supplierIds: string[] = []
+      if (transaction.merchant_name) {
+        const escapedMerchant = escapeLikePattern(transaction.merchant_name)
+        const { data: matchedSuppliers } = await supabase
+          .from('suppliers')
+          .select('id')
+          .eq('company_id', companyId)
+          .ilike('name', `%${escapedMerchant}%`)
+          .limit(10)
+        supplierIds = (matchedSuppliers || []).map((s) => s.id)
+      }
+
+      if (supplierIds.length > 0) {
+        // Restrict candidates to invoices within the date window relative to
+        // the bank tx date. Without this, an open invoice from years back can
+        // surface as a match and misdirect the user (swedish-compliance bot).
+        const txDateMs = new Date(transaction.date).getTime()
+        const invoiceDateLow = new Date(txDateMs - DUPLICATE_DATE_WINDOW_DAYS * 24 * 3600 * 1000)
+          .toISOString()
+          .split('T')[0]
+        const invoiceDateHigh = new Date(txDateMs + DUPLICATE_DATE_WINDOW_DAYS * 24 * 3600 * 1000)
+          .toISOString()
+          .split('T')[0]
+
+        const { data: openInvoices } = await supabase
+          .from('supplier_invoices')
+          .select('id, supplier_invoice_number, invoice_date, remaining_amount, currency, supplier:suppliers(name)')
+          .eq('company_id', companyId)
+          .in('supplier_id', supplierIds)
+          .in('status', ['registered', 'approved', 'partially_paid', 'overdue'])
+          .gte('remaining_amount', windowLow)
+          .lte('remaining_amount', windowHigh)
+          .gte('invoice_date', invoiceDateLow)
+          .lte('invoice_date', invoiceDateHigh)
+          .order('invoice_date', { ascending: false })
+          .limit(5)
+
+        if (openInvoices && openInvoices.length > 0) {
+          return errorResponseFromCode('TX_CATEGORIZE_SUGGEST_SI_MATCH', txLog, {
+            requestId,
+            details: {
+              candidates: openInvoices.map((inv) => ({
+                supplier_invoice_id: inv.id,
+                invoice_number: inv.supplier_invoice_number,
+                invoice_date: inv.invoice_date,
+                remaining_amount: inv.remaining_amount,
+                currency: inv.currency,
+                supplier_name: (inv.supplier as { name?: string } | null)?.name ?? null,
+              })),
+            },
+          })
+        }
+      }
+    }
+
    await ensureFiscalPeriod(supabase, user.id, companyId, transaction.date, fiscalYearStartMonth, txLog)

    let journalEntryCreated = false