diff --git a/app/api/transactions/[id]/match-invoice/route.ts b/app/api/transactions/[id]/match-invoice/route.ts index d4451a36..9e3c970f 100644 --- a/app/api/transactions/[id]/match-invoice/route.ts +++ b/app/api/transactions/[id]/match-invoice/route.ts @@ -10,6 +10,7 @@ import { errorResponse, errorResponseFromCode } from '@/lib/errors/get-structure import { validateBody } from '@/lib/api/validate' import { MatchInvoiceSchema } from '@/lib/api/schemas' import { logMatchEvent } from '@/lib/invoices/match-log' +import { planInvoicePayment } from '@/lib/invoices/apply-invoice-payment' import { detectDuplicatePaymentVoucher } from '@/lib/invoices/duplicate-payment-detection' import { eventBus } from '@/lib/events/bus' import { ensureInitialized } from '@/lib/init' @@ -296,28 +297,17 @@ export const POST = withRouteContext( ? fx.paidInInvoiceCurrency : transaction.amount - 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 (paidAmountInInvoiceCurrency > currentRemaining + 0.005) { + // Overshoot guard + paid/remaining math — shared with the v1 and agent + // (commit) paths via planInvoicePayment so they cannot drift again. Runs + // before any JE is created, so a doomed match never burns a voucher number. + const payment = planInvoicePayment(invoice, paidAmountInInvoiceCurrency) + if (!payment.ok) { return errorResponseFromCode('MATCH_AMOUNT_EXCEEDS_REMAINING', txLog, { requestId, - details: { - transaction_amount: paidAmountInInvoiceCurrency, - remaining_amount: Math.round(currentRemaining * 100) / 100, - excess: Math.round((paidAmountInInvoiceCurrency - currentRemaining) * 100) / 100, - }, + details: payment.details, }) } - - const newPaidAmount = Math.round(((invoice.paid_amount || 0) + paidAmountInInvoiceCurrency) * 100) / 100 - const newRemaining = Math.max(0, Math.round((currentRemaining - paidAmountInInvoiceCurrency) * 100) / 100) - const isFullyPaid = newRemaining <= 0 - const newStatus = isFullyPaid ? 'paid' : 'partially_paid' + const { newPaidAmount, newRemaining, isFullyPaid, newStatus } = payment.plan const { data: settings } = await supabase .from('company_settings') diff --git a/app/api/v1/companies/[companyId]/transactions/[id]/match-invoice/route.ts b/app/api/v1/companies/[companyId]/transactions/[id]/match-invoice/route.ts index b39ccb33..f3706226 100644 --- a/app/api/v1/companies/[companyId]/transactions/[id]/match-invoice/route.ts +++ b/app/api/v1/companies/[companyId]/transactions/[id]/match-invoice/route.ts @@ -30,6 +30,7 @@ import { reverseEntry, createJournalEntry, findFiscalPeriod } from '@/lib/bookke import { AccountsNotInChartError, isBookkeepingError } from '@/lib/bookkeeping/errors' import { getErrorMessage } from '@/lib/errors/get-error-message' import { logMatchEvent } from '@/lib/invoices/match-log' +import { planInvoicePayment } from '@/lib/invoices/apply-invoice-payment' import { detectDuplicatePaymentVoucher } from '@/lib/invoices/duplicate-payment-detection' import { eventBus } from '@/lib/events/bus' import type { EntityType, Invoice, Transaction } from '@/types' @@ -265,6 +266,22 @@ export const POST = withApiV1<{ params: Promise<{ companyId: string; id: string }) } + const paidAmount = transaction.amount + + // Overshoot guard + paid/remaining math — shared with the dashboard and + // agent (commit) paths via planInvoicePayment. Without this, the public API + // silently overpaid an invoice (recording paid_amount > total, over-crediting + // AR). Runs BEFORE the storno + strict-mode JE creation, so a rejected match + // touches no state. + const payment = planInvoicePayment(invoice, paidAmount) + if (!payment.ok) { + return v1ErrorResponseFromCode('MATCH_AMOUNT_EXCEEDS_REMAINING', txLog, { + requestId: ctx.requestId, + details: payment.details, + }) + } + const { newPaidAmount, newRemaining, isFullyPaid, newStatus } = payment.plan + if (transaction.journal_entry_id) { try { await reverseEntry(ctx.supabase, ctx.companyId!, ctx.userId, transaction.journal_entry_id) @@ -288,17 +305,6 @@ export const POST = withApiV1<{ params: Promise<{ companyId: string; id: string } 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) - const newRemaining = Math.max( - 0, - Math.round((currentRemaining - paidAmount) * 100) / 100, - ) - const isFullyPaid = newRemaining <= 0 - const newStatus = isFullyPaid ? 'paid' : 'partially_paid' const { data: settings } = await ctx.supabase .from('company_settings') diff --git a/lib/invoices/__tests__/apply-invoice-payment.test.ts b/lib/invoices/__tests__/apply-invoice-payment.test.ts new file mode 100644 index 00000000..e6642778 --- /dev/null +++ b/lib/invoices/__tests__/apply-invoice-payment.test.ts @@ -0,0 +1,67 @@ +import { describe, it, expect } from 'vitest' +import { + planInvoicePayment, + PAYMENT_OVERSHOOT_TOLERANCE, +} from '@/lib/invoices/apply-invoice-payment' + +describe('planInvoicePayment', () => { + it('marks fully paid on an exact payment', () => { + const r = planInvoicePayment({ total: 1000, paid_amount: 0, remaining_amount: 1000 }, 1000) + expect(r.ok).toBe(true) + if (r.ok) { + expect(r.plan).toEqual({ + newPaidAmount: 1000, + newRemaining: 0, + isFullyPaid: true, + newStatus: 'paid', + }) + } + }) + + it('marks partially paid on a partial payment', () => { + const r = planInvoicePayment({ total: 1000, paid_amount: 0, remaining_amount: 1000 }, 400) + expect(r.ok).toBe(true) + if (r.ok) { + expect(r.plan.newStatus).toBe('partially_paid') + expect(r.plan.newPaidAmount).toBe(400) + expect(r.plan.newRemaining).toBe(600) + expect(r.plan.isFullyPaid).toBe(false) + } + }) + + it('accumulates onto an existing paid_amount', () => { + const r = planInvoicePayment({ total: 1000, paid_amount: 600, remaining_amount: 400 }, 400) + expect(r.ok).toBe(true) + if (r.ok) { + expect(r.plan.newPaidAmount).toBe(1000) + expect(r.plan.isFullyPaid).toBe(true) + } + }) + + it('REJECTS overpayment (the bug: agent/v1 paths used to swallow it)', () => { + const r = planInvoicePayment({ total: 1000, paid_amount: 0, remaining_amount: 1000 }, 1500) + expect(r.ok).toBe(false) + if (!r.ok) { + expect(r.code).toBe('MATCH_AMOUNT_EXCEEDS_REMAINING') + expect(r.details).toEqual({ + transaction_amount: 1500, + remaining_amount: 1000, + excess: 500, + }) + } + }) + + it('accepts a sub-öre overshoot (float drift) but rejects a real one-öre over', () => { + expect(planInvoicePayment({ total: 1000, remaining_amount: 1000 }, 1000.004).ok).toBe(true) + expect(planInvoicePayment({ total: 1000, remaining_amount: 1000 }, 1000.01).ok).toBe(false) + }) + + it('falls back to total - paid_amount when remaining_amount is absent', () => { + expect(planInvoicePayment({ total: 1000, paid_amount: 300 }, 700).ok).toBe(true) + expect(planInvoicePayment({ total: 1000, paid_amount: 300 }, 701).ok).toBe(false) + }) + + it('overshoot tolerance is half an öre', () => { + expect(PAYMENT_OVERSHOOT_TOLERANCE).toBe(0.005) + }) +}) diff --git a/lib/invoices/apply-invoice-payment.ts b/lib/invoices/apply-invoice-payment.ts new file mode 100644 index 00000000..70262d4f --- /dev/null +++ b/lib/invoices/apply-invoice-payment.ts @@ -0,0 +1,83 @@ +/** + * Single source of truth for applying a payment amount to a customer invoice. + * + * Computes the new paid/remaining/status and — critically — REJECTS overpayment + * before the caller creates any journal entry, so a doomed match never burns a + * voucher number. + * + * Background: this math was copy-pasted across three sites — the dashboard + * match-invoice route (which had the overpayment guard), the v1 public API + * route, and `commitMatchTransactionInvoice` (the agent/MCP path). The latter + * two had drifted WITHOUT the guard, so they silently swallowed overpayment via + * `Math.max(0, …)` — recording e.g. 1500 paid on a 1000 invoice and + * over-crediting accounts receivable. Centralizing the math + guard here closes + * that drift; all three sites delegate to `planInvoicePayment`. + * + * FX: `paymentAmountInInvoiceCurrency` MUST already be in the invoice's + * currency. The caller owns any conversion (cross-currency settlement lives in + * the dashboard route), keeping this helper FX-agnostic. + * + * Extracted from the proven dashboard route with the same half-öre overshoot + * tolerance. Rounding goes through the canonical `roundOre` (@/lib/money) per + * guard rail #9 — identical to the route's previous `Math.round(x*100)/100` + * except on exact-half-öre amounts, where `roundOre` rounds correctly. + */ +import { roundOre, ORE_TOLERANCE } from '@/lib/money' + +/** Half an öre — anything over the remaining by more than this is a real overpayment. */ +export const PAYMENT_OVERSHOOT_TOLERANCE = ORE_TOLERANCE + +export interface InvoicePaymentTotals { + total: number + paid_amount?: number | null + remaining_amount?: number | null +} + +export interface InvoicePaymentPlan { + newPaidAmount: number + newRemaining: number + isFullyPaid: boolean + newStatus: 'paid' | 'partially_paid' +} + +export type PlanInvoicePaymentResult = + | { ok: true; plan: InvoicePaymentPlan } + | { + ok: false + code: 'MATCH_AMOUNT_EXCEEDS_REMAINING' + details: { transaction_amount: number; remaining_amount: number; excess: number } + } + +export function planInvoicePayment( + invoice: InvoicePaymentTotals, + paymentAmountInInvoiceCurrency: number, +): PlanInvoicePaymentResult { + const currentRemaining = + invoice.remaining_amount ?? invoice.total - (invoice.paid_amount || 0) + + if (paymentAmountInInvoiceCurrency > currentRemaining + PAYMENT_OVERSHOOT_TOLERANCE) { + return { + ok: false, + code: 'MATCH_AMOUNT_EXCEEDS_REMAINING', + details: { + transaction_amount: paymentAmountInInvoiceCurrency, + remaining_amount: roundOre(currentRemaining), + excess: roundOre(paymentAmountInInvoiceCurrency - currentRemaining), + }, + } + } + + const newPaidAmount = roundOre((invoice.paid_amount || 0) + paymentAmountInInvoiceCurrency) + const newRemaining = Math.max(0, roundOre(currentRemaining - paymentAmountInInvoiceCurrency)) + const isFullyPaid = newRemaining <= 0 + + return { + ok: true, + plan: { + newPaidAmount, + newRemaining, + isFullyPaid, + newStatus: isFullyPaid ? 'paid' : 'partially_paid', + }, + } +} diff --git a/lib/pending-operations/commit.ts b/lib/pending-operations/commit.ts index 7e2f1ae3..d2f56a3f 100644 --- a/lib/pending-operations/commit.ts +++ b/lib/pending-operations/commit.ts @@ -40,6 +40,7 @@ import { createSupplierInvoiceRegistrationEntry, } from '@/lib/bookkeeping/supplier-invoice-entries' import { linkInvoiceToVoucher } from '@/lib/invoices/voucher-matching' +import { planInvoicePayment } from '@/lib/invoices/apply-invoice-payment' import { linkSupplierInvoiceToVoucher } from '@/lib/invoices/supplier-voucher-matching' import { linkTransactionToJournalEntry } from '@/lib/transactions/link-journal-entry' import { getErrorEntry } from '@/lib/errors/structured-errors' @@ -899,18 +900,29 @@ async function commitMatchTransactionInvoice( return { error: 'Invoice is not in a matchable state', status: 409 } } + // Overshoot guard + paid/remaining math — shared with the dashboard and v1 + // routes via planInvoicePayment. This agent/MCP path previously had NO guard, + // so a 1500 payment on a 1000 invoice was silently accepted (paid_amount > + // total, AR over-credited). Runs BEFORE the storno + JE below, so a rejected + // match leaves the transaction untouched and never burns a voucher number. + const paidAmount = transaction.amount + const payment = planInvoicePayment(invoice, paidAmount) + if (!payment.ok) { + return { + error: + getErrorEntry('MATCH_AMOUNT_EXCEEDS_REMAINING')?.message_sv ?? + 'Transaktionsbeloppet är större än fakturans återstående belopp.', + status: 400, + } + } + const { newPaidAmount, newRemaining, isFullyPaid, newStatus } = payment.plan + if (transaction.journal_entry_id) { await reverseEntry(supabase, companyId, userId, transaction.journal_entry_id) await supabase.from('transactions').update({ journal_entry_id: null }).eq('id', transactionId) } 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)) - const newRemaining = Math.max(0, Math.round((currentRemaining - paidAmount) * 100) / 100) - const isFullyPaid = newRemaining <= 0 - const newStatus = isFullyPaid ? 'paid' : 'partially_paid' const { data: settings } = await supabase .from('company_settings').select('accounting_method, entity_type').eq('company_id', companyId).single() diff --git a/scripts/checks/antipatterns-baseline.json b/scripts/checks/antipatterns-baseline.json index 21a1f88a..1916bdb8 100644 --- a/scripts/checks/antipatterns-baseline.json +++ b/scripts/checks/antipatterns-baseline.json @@ -177,6 +177,6 @@ ] }, "naiveOreRound": { - "count": 668 + "count": 661 } }