fix(payments): refuse to book a bank row that unlinked vouchers already explain (#2300)
* fix(payments): refuse to book a bank row that unlinked vouchers already explain A bank feed can deliver several affarshandelser as one row (a Bankgirot daily aggregate: two customers' invoices, one "BGGIRERING" row with no payer). When each invoice was already marked paid by hand, nothing on the account equals the row, the 1:1 duplicate check passes, and "Dela betalning" books the money a second time against whatever open invoices the user picks (the next period's identical ones, in the reported case). - lib/reconciliation/covering-set.ts: exact ore subset sum over a capped candidate list, smallest set first, closest in date second. - detectExplainingVoucherSet(+ForTransaction): the vouchers whose bank legs on the row's settlement account, in the row's direction, within 7 days, add up exactly to the row; linked through any of the three anchors drops a voucher, a payment row without a bank transaction keeps it. - POST match-batch refuses with BATCH_TX_POSSIBLE_DUPLICATE and returns the set; force=true must echo expected_journal_entry_ids (same binding as the single door). Fails open on a detection error. - GET duplicate-payment-check returns candidate_set next to candidate. - MatchAllocationDialog: pre-flight panel with the vouchers, one click links the row to them through the existing 1:1 or 1:N bank link (no new voucher), "Bokfor anda" acknowledges the set; confirm is disabled until then. Invoices dated after the bank row get a hint badge. - Mark-paid guard: aggregate sweep (row = this invoice + an exact subset of other open invoices, 7 days, kronor) when the name sweeps found nothing; PaymentBookingDialog shows the covered invoice numbers and points to the split under Transaktioner. Follow-ups: #2293 (1:N proposals in the auto-matcher), #2294 (MCP staging guard), #2299 (supplier-side text guard). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NyjeEi1U8vnuPT4QXgayXu * test(invoices): account for the aggregate sweep in the mark-paid route queue The sweep issues one more transactions query whenever the name probes come back empty, so every queued-mock sequence that reaches it gains a slot. The sweep itself now fails open on odd client shapes (a single object for a list query) and on errors: an advisory guard must never block "Markera som betald". Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NyjeEi1U8vnuPT4QXgayXu * fix(payments): fail open on resolved query errors; aggregate sweep without a payer name Review follow-ups on #2300. A PostgREST failure resolves with { data: null, error } instead of throwing, so the set detector read a failed link lookup as "no links" and a failed cash-account lookup as "scan every 19xx account"; both now return null (the booking RPC keeps the last word). The aggregate sweep never needed a customer name (a Bankgirot row names nobody), so a nameless invoice goes straight to it instead of skipping the guard. The already-booked panel is announced as a live region, and the "also covers" string is plural-aware. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NyjeEi1U8vnuPT4QXgayXu --------- Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Jakob Wennberg
co-authored by
Claude Fable 5.1
Jakob Wennberg
Claude Fable 5.1
Jakob Wennberg
parent
9418de585f
commit
287828a850
@@ -30,7 +30,7 @@ import type { EntityType } from '@/types'
|
||||
import type { InvoiceWithRelations } from '@/components/invoices/types'
|
||||
import { loadBasCatalog, type CatalogAccount } from '@/lib/bookkeeping/bas-catalog-client'
|
||||
|
||||
type DuplicateMatchReason = 'ocr_exact' | 'name_amount_fuzzy' | 'amount_only'
|
||||
type DuplicateMatchReason = 'ocr_exact' | 'name_amount_fuzzy' | 'amount_only' | 'aggregate_exact'
|
||||
|
||||
interface DuplicateCandidate {
|
||||
id: string
|
||||
@@ -41,6 +41,8 @@ interface DuplicateCandidate {
|
||||
reference: string | null
|
||||
match_reason: DuplicateMatchReason
|
||||
match_confidence: number
|
||||
/** aggregate_exact: the other open invoices the bank row also covers. */
|
||||
aggregate_invoice_numbers?: string[]
|
||||
}
|
||||
|
||||
interface PaymentBookingDialogProps {
|
||||
@@ -67,6 +69,7 @@ export default function PaymentBookingDialog({
|
||||
ocr_exact: t('match_reason_ocr_exact'),
|
||||
name_amount_fuzzy: t('match_reason_name_amount_fuzzy'),
|
||||
amount_only: t('match_reason_amount_only'),
|
||||
aggregate_exact: t('match_reason_aggregate_exact'),
|
||||
}
|
||||
|
||||
// Session-cached reference data (lib/reference-data), seeded by the
|
||||
@@ -363,11 +366,13 @@ export default function PaymentBookingDialog({
|
||||
<ul className="space-y-2">
|
||||
{duplicateCandidates.map((c) => {
|
||||
const reasonVariant: 'success' | 'secondary' | 'outline' =
|
||||
c.match_reason === 'ocr_exact'
|
||||
c.match_reason === 'ocr_exact' || c.match_reason === 'aggregate_exact'
|
||||
? 'success'
|
||||
: c.match_reason === 'name_amount_fuzzy'
|
||||
? 'secondary'
|
||||
: 'outline'
|
||||
const isAggregate =
|
||||
c.match_reason === 'aggregate_exact' && (c.aggregate_invoice_numbers?.length ?? 0) > 0
|
||||
return (
|
||||
<li
|
||||
key={c.id}
|
||||
@@ -386,6 +391,18 @@ export default function PaymentBookingDialog({
|
||||
<p className="truncate text-xs text-muted-foreground">
|
||||
{c.merchant_name || c.description || '-'}
|
||||
</p>
|
||||
{/* A Bankgirot aggregate: the row also settles other
|
||||
invoices, so the remedy is the split under
|
||||
Transaktioner (one samlingsverifikation, row linked),
|
||||
never marking the invoices paid one by one. */}
|
||||
{isAggregate && (
|
||||
<p className="text-xs text-muted-foreground">
|
||||
{t('aggregate_covers', {
|
||||
count: c.aggregate_invoice_numbers!.length,
|
||||
numbers: c.aggregate_invoice_numbers!.join(', '),
|
||||
})}
|
||||
</p>
|
||||
)}
|
||||
</div>
|
||||
<Button
|
||||
type="button"
|
||||
@@ -394,7 +411,7 @@ export default function PaymentBookingDialog({
|
||||
onClick={() => handleLinkExisting(c.id)}
|
||||
className="shrink-0"
|
||||
>
|
||||
{t('link_transaction')}
|
||||
{isAggregate ? t('allocate_transaction') : t('link_transaction')}
|
||||
</Button>
|
||||
</li>
|
||||
)
|
||||
|
||||
@@ -19,7 +19,7 @@ import { Badge } from '@/components/ui/badge'
|
||||
import { useToast } from '@/components/ui/use-toast'
|
||||
import { getErrorMessage } from '@/lib/errors/get-error-message'
|
||||
import { formatCurrency, formatDate, cn, isValidExchangeRate } from '@/lib/utils'
|
||||
import { Loader2, Search, X, Plus, Check, AlertTriangle } from 'lucide-react'
|
||||
import { Loader2, Search, X, Plus, Check, AlertTriangle, Link2 } from 'lucide-react'
|
||||
import type { Invoice, Customer, SupplierInvoice, Supplier } from '@/types'
|
||||
import type { TransactionWithInvoice } from './transaction-types'
|
||||
|
||||
@@ -51,6 +51,43 @@ interface AllocationCandidate {
|
||||
currency: string
|
||||
exchangeRate: number | null
|
||||
dueDate: string
|
||||
/** Invoice date: an invoice issued AFTER the bank row cannot normally be what it paid. */
|
||||
invoiceDate: string | null
|
||||
}
|
||||
|
||||
/**
|
||||
* Mirror of ExplainingVoucherSet (lib/invoices/duplicate-payment-detection.ts):
|
||||
* the posted, unlinked vouchers whose bank legs add up exactly to this row.
|
||||
* Served by GET /api/transactions/[id]/duplicate-payment-check and by the
|
||||
* BATCH_TX_POSSIBLE_DUPLICATE refusal of POST match-batch.
|
||||
*/
|
||||
interface ExplainingVoucher {
|
||||
journal_entry_id: string
|
||||
voucher_label: string
|
||||
entry_date: string
|
||||
description: string | null
|
||||
source_type: string | null
|
||||
amount: number
|
||||
bank_account_number: string
|
||||
}
|
||||
|
||||
interface ExplainingSet {
|
||||
vouchers: ExplainingVoucher[]
|
||||
total: number
|
||||
bank_account_number: string
|
||||
same_date: boolean
|
||||
}
|
||||
|
||||
function readExplainingSet(value: unknown): ExplainingSet | null {
|
||||
if (!value || typeof value !== 'object') return null
|
||||
const v = value as Partial<ExplainingSet>
|
||||
if (!Array.isArray(v.vouchers) || v.vouchers.length === 0) return null
|
||||
return {
|
||||
vouchers: v.vouchers,
|
||||
total: Number(v.total ?? 0),
|
||||
bank_account_number: v.bank_account_number ?? v.vouchers[0].bank_account_number,
|
||||
same_date: v.same_date === true,
|
||||
}
|
||||
}
|
||||
|
||||
type AllocationDraft = {
|
||||
@@ -95,6 +132,13 @@ export default function MatchAllocationDialog({
|
||||
const [search, setSearch] = useState('')
|
||||
const [drafts, setDrafts] = useState<Record<string, AllocationDraft>>({})
|
||||
const [submitting, setSubmitting] = useState(false)
|
||||
// Already-explained guard: the vouchers that already book this row, if any.
|
||||
// Set from the pre-flight on open, or from the route's 409 on submit. The
|
||||
// user either links the row to them (no new voucher) or acknowledges the
|
||||
// set, which is what lets the confirm through with force=true.
|
||||
const [explaining, setExplaining] = useState<ExplainingSet | null>(null)
|
||||
const [explainingAcknowledged, setExplainingAcknowledged] = useState(false)
|
||||
const [linking, setLinking] = useState(false)
|
||||
|
||||
useEffect(() => {
|
||||
if (!open || !transaction || !company) return
|
||||
@@ -129,6 +173,7 @@ export default function MatchAllocationDialog({
|
||||
currency: r.currency,
|
||||
exchangeRate: r.exchange_rate != null ? Number(r.exchange_rate) : null,
|
||||
dueDate: r.due_date,
|
||||
invoiceDate: r.invoice_date ?? null,
|
||||
})),
|
||||
)
|
||||
} else {
|
||||
@@ -152,6 +197,7 @@ export default function MatchAllocationDialog({
|
||||
currency: r.currency,
|
||||
exchangeRate: r.exchange_rate != null ? Number(r.exchange_rate) : null,
|
||||
dueDate: r.due_date,
|
||||
invoiceDate: r.invoice_date ?? null,
|
||||
})),
|
||||
)
|
||||
}
|
||||
@@ -165,16 +211,46 @@ export default function MatchAllocationDialog({
|
||||
}
|
||||
}, [open, transaction, company, kind, supabase, t])
|
||||
|
||||
// Pre-flight: does the ledger already explain this row? Same detector the
|
||||
// route refuses with, so the panel shows before a doomed submit. Fail-open:
|
||||
// a failed pre-flight only means the route's own check does the refusing.
|
||||
useEffect(() => {
|
||||
if (!open || !transaction) return
|
||||
let cancelled = false
|
||||
async function check() {
|
||||
try {
|
||||
const res = await fetch(`/api/transactions/${transaction!.id}/duplicate-payment-check`)
|
||||
if (!res.ok) return
|
||||
const json = (await res.json()) as { candidate_set?: unknown }
|
||||
if (!cancelled) setExplaining(readExplainingSet(json.candidate_set))
|
||||
} catch {
|
||||
// Pre-flight is advisory; the POST guard still runs.
|
||||
}
|
||||
}
|
||||
void check()
|
||||
return () => {
|
||||
cancelled = true
|
||||
}
|
||||
}, [open, transaction])
|
||||
|
||||
// Reset state every time the dialog re-opens for a new tx.
|
||||
useEffect(() => {
|
||||
if (!open) {
|
||||
setDrafts({})
|
||||
setSearch('')
|
||||
setExplaining(null)
|
||||
setExplainingAcknowledged(false)
|
||||
}
|
||||
}, [open])
|
||||
|
||||
const txAmountAbs = transaction ? Math.abs(transaction.amount) : 0
|
||||
const txCurrency = transaction?.currency ?? 'SEK'
|
||||
// The explaining set is stated in SEK and the 1:N link slices are stated in
|
||||
// the row's currency, so the one-click link is only offered for kronor rows;
|
||||
// a foreign row is pointed to the reconciliation view instead.
|
||||
const explainingLinkable = !!explaining && txCurrency === 'SEK'
|
||||
const explainingBlocks = !!explaining && !explainingAcknowledged
|
||||
const explainingLabels = explaining ? explaining.vouchers.map((v) => v.voucher_label).join(' + ') : ''
|
||||
|
||||
// Each draft's `amount` is the allocation in TRANSACTION currency (SEK
|
||||
// for a Swedish bank import). For cross-currency invoices the FX
|
||||
@@ -298,11 +374,34 @@ export default function MatchAllocationDialog({
|
||||
const response = await fetch(`/api/transactions/${transaction.id}/match-batch`, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ allocations }),
|
||||
body: JSON.stringify({
|
||||
allocations,
|
||||
// Acknowledged set: echo its ids so the route can verify that what
|
||||
// the user overrode is what it still detects.
|
||||
...(explaining && explainingAcknowledged
|
||||
? {
|
||||
force: true,
|
||||
expected_journal_entry_ids: explaining.vouchers.map((v) => v.journal_entry_id),
|
||||
}
|
||||
: {}),
|
||||
}),
|
||||
})
|
||||
|
||||
if (!response.ok) {
|
||||
const body = await response.json().catch(() => null)
|
||||
const code = (body as { error?: { code?: string; details?: unknown } } | null)?.error?.code
|
||||
if (code === 'BATCH_TX_POSSIBLE_DUPLICATE') {
|
||||
// The pre-flight missed it (or the set changed since): show the
|
||||
// vouchers instead of an error toast and let the user decide.
|
||||
const set = readExplainingSet(
|
||||
(body as { error?: { details?: unknown } }).error?.details,
|
||||
)
|
||||
if (set) {
|
||||
setExplaining(set)
|
||||
setExplainingAcknowledged(false)
|
||||
return
|
||||
}
|
||||
}
|
||||
toast({
|
||||
title: t('error_submit_title'),
|
||||
description: getErrorMessage(body, {
|
||||
@@ -334,6 +433,65 @@ export default function MatchAllocationDialog({
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Link the row to the vouchers that already book it. One voucher goes
|
||||
* through the 1:1 link, several through the 1:N split
|
||||
* (linkTransactionToVouchers): slices carry the row's sign and each
|
||||
* voucher's SEK bank leg, which the engine checks against the voucher's
|
||||
* line and against the row total. No new verifikat is created.
|
||||
*/
|
||||
async function handleLinkToExplaining() {
|
||||
if (!transaction || !explaining || !explainingLinkable) return
|
||||
setLinking(true)
|
||||
try {
|
||||
const sign = transaction.amount > 0 ? 1 : -1
|
||||
const body =
|
||||
explaining.vouchers.length === 1
|
||||
? {
|
||||
transaction_id: transaction.id,
|
||||
journal_entry_id: explaining.vouchers[0].journal_entry_id,
|
||||
account_number: explaining.bank_account_number,
|
||||
}
|
||||
: {
|
||||
transaction_id: transaction.id,
|
||||
account_number: explaining.bank_account_number,
|
||||
allocations: explaining.vouchers.map((v) => ({
|
||||
journal_entry_id: v.journal_entry_id,
|
||||
amount: round2(sign * v.amount),
|
||||
})),
|
||||
}
|
||||
const res = await fetch('/api/reconciliation/bank/link', {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify(body),
|
||||
})
|
||||
const json = await res.json().catch(() => null)
|
||||
if (!res.ok || (json as { error?: unknown } | null)?.error) {
|
||||
toast({
|
||||
title: t('already_booked_link_failed'),
|
||||
description: getErrorMessage(json, { context: 'transaction', statusCode: res.status }),
|
||||
variant: 'destructive',
|
||||
})
|
||||
return
|
||||
}
|
||||
toast({
|
||||
title: t('already_booked_link_success_title'),
|
||||
description: t('already_booked_link_success_description', { labels: explainingLabels }),
|
||||
variant: 'success',
|
||||
})
|
||||
onSuccess()
|
||||
onOpenChange(false)
|
||||
} catch (err) {
|
||||
toast({
|
||||
title: t('already_booked_link_failed'),
|
||||
description: getErrorMessage(err, { context: 'transaction' }),
|
||||
variant: 'destructive',
|
||||
})
|
||||
} finally {
|
||||
setLinking(false)
|
||||
}
|
||||
}
|
||||
|
||||
if (!transaction) return null
|
||||
|
||||
return (
|
||||
@@ -369,6 +527,108 @@ export default function MatchAllocationDialog({
|
||||
</div>
|
||||
</div>
|
||||
|
||||
{/* Already-explained guard: the ledger already books this row.
|
||||
Shown first, before any invoice can be picked: the mistake this
|
||||
prevents is picking the next period's identical invoices for a
|
||||
row whose payment was already booked by hand. */}
|
||||
{explaining && (
|
||||
<div
|
||||
className={cn(
|
||||
'rounded-lg border p-4 space-y-3',
|
||||
explainingAcknowledged ? 'border-border bg-muted/20' : 'border-attn/40 bg-muted/30',
|
||||
)}
|
||||
data-testid="already-booked-panel"
|
||||
role="status"
|
||||
aria-live="polite"
|
||||
>
|
||||
<div className="flex items-start gap-2">
|
||||
<AlertTriangle className="h-4 w-4 flex-shrink-0 mt-0.5 text-attn" />
|
||||
<div className="min-w-0 flex-1 space-y-1 text-sm">
|
||||
<p className="font-medium text-attn">
|
||||
{explainingAcknowledged
|
||||
? t('already_booked_acknowledged_title')
|
||||
: t('already_booked_title')}
|
||||
</p>
|
||||
{!explainingAcknowledged && (
|
||||
<p className="text-muted-foreground">
|
||||
{t(
|
||||
transaction.amount > 0 ? 'already_booked_body_in' : 'already_booked_body_out',
|
||||
{
|
||||
amount: formatCurrency(explaining.total, 'SEK'),
|
||||
count: explaining.vouchers.length,
|
||||
},
|
||||
)}
|
||||
</p>
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
{!explainingAcknowledged && (
|
||||
<ul className="space-y-1.5">
|
||||
{explaining.vouchers.map((v) => (
|
||||
<li
|
||||
key={v.journal_entry_id}
|
||||
className="flex items-center justify-between gap-3 rounded-sm border bg-card px-3 py-2 text-sm"
|
||||
>
|
||||
<div className="min-w-0 space-y-0.5">
|
||||
<div className="flex items-center gap-2">
|
||||
<span className="font-medium tabular-nums">{v.voucher_label}</span>
|
||||
<span className="text-xs tabular-nums text-muted-foreground">
|
||||
{formatDate(v.entry_date)}
|
||||
</span>
|
||||
</div>
|
||||
{v.description && (
|
||||
<p className="truncate text-xs text-muted-foreground">{v.description}</p>
|
||||
)}
|
||||
</div>
|
||||
<span className="shrink-0 font-medium tabular-nums">
|
||||
{formatCurrency(v.amount, 'SEK')}
|
||||
</span>
|
||||
</li>
|
||||
))}
|
||||
</ul>
|
||||
)}
|
||||
{!explainingAcknowledged && (
|
||||
<div className="flex flex-col gap-2 sm:flex-row">
|
||||
{explainingLinkable ? (
|
||||
<Button
|
||||
type="button"
|
||||
size="sm"
|
||||
onClick={handleLinkToExplaining}
|
||||
disabled={linking || submitting}
|
||||
className="sm:flex-1"
|
||||
>
|
||||
{linking ? (
|
||||
<Loader2 className="mr-2 h-4 w-4 animate-spin" />
|
||||
) : (
|
||||
<Link2 className="mr-2 h-4 w-4" />
|
||||
)}
|
||||
{t('already_booked_link', { labels: explainingLabels })}
|
||||
</Button>
|
||||
) : (
|
||||
<p className="text-xs text-muted-foreground sm:flex-1">
|
||||
{t('already_booked_foreign_hint', { currency: txCurrency })}
|
||||
</p>
|
||||
)}
|
||||
<Button
|
||||
type="button"
|
||||
size="sm"
|
||||
variant="ghost"
|
||||
onClick={() => setExplainingAcknowledged(true)}
|
||||
disabled={linking || submitting}
|
||||
className="text-muted-foreground"
|
||||
>
|
||||
{t('already_booked_book_anyway')}
|
||||
</Button>
|
||||
</div>
|
||||
)}
|
||||
{explainingAcknowledged && (
|
||||
<p className="text-xs text-muted-foreground">
|
||||
{t('already_booked_acknowledged_note', { labels: explainingLabels })}
|
||||
</p>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
|
||||
{/* Search */}
|
||||
<div className="relative">
|
||||
<Search className="absolute left-3 top-1/2 h-4 w-4 -translate-y-1/2 text-muted-foreground" />
|
||||
@@ -424,6 +684,16 @@ export default function MatchAllocationDialog({
|
||||
amount: formatCurrency(c.remaining, c.currency),
|
||||
})}
|
||||
</p>
|
||||
{/* Money that arrived before the invoice existed rarely
|
||||
paid it: the next period's identical invoice is the
|
||||
classic wrong pick when the real one is already
|
||||
settled. A hint, not a block: prepayments exist. */}
|
||||
{c.invoiceDate && c.invoiceDate > transaction.date && (
|
||||
<Badge variant="warning" className="gap-1">
|
||||
<AlertTriangle className="h-3 w-3" />
|
||||
{t('invoiced_after_payment_badge', { date: formatDate(c.invoiceDate) })}
|
||||
</Badge>
|
||||
)}
|
||||
</div>
|
||||
{isSelected ? (
|
||||
<div className="flex flex-col items-end gap-1">
|
||||
@@ -537,7 +807,7 @@ export default function MatchAllocationDialog({
|
||||
// Confirm requires sum == tx_abs exactly (within rounding).
|
||||
// Anything else lets the JE diverge from the bank line and
|
||||
// breaks reconciliation. PR #607 round-1 review fix.
|
||||
disabled={submitting || !balanced || overshoot}
|
||||
disabled={submitting || linking || !balanced || overshoot || explainingBlocks}
|
||||
>
|
||||
{submitting && <Loader2 className="mr-2 h-4 w-4 animate-spin" />}
|
||||
{t('confirm')}
|
||||
|
||||
Reference in New Issue
Block a user