fix(transactions): close the booking duplicate guard's blind spots (#1573)
* fix(transactions): close booking duplicate guard blind spots G1-G3 The booking-time duplicate guard missed the most common bank-fee twin shapes: - G1: the sibling scan matched on the EXACT date only, so a duplicate import with a drifted date (CSV bokforingsdag vs PSD2 valutadag) was invisible. The scan now uses a +-3 day window with a deterministic ranking where exact-date candidates always outrank drifted ones (force=true re-detection stays bound to the reviewed candidate). - G2: booked-ness required transactions.journal_entry_id, so bulk-booked (transaction_voucher_links) and multi-allocated (invoice_payments / supplier_invoice_payments) siblings read as unbooked. The scan now batch-fetches the anchor rows and resolves the verifikat via getPrimaryJournalEntryId (is_transaction_booked semantics). - G3: the ledger scan excluded every voucher linked to any transaction, so a voucher booked from a date-drifted duplicate row escaped BOTH halves and the booking proceeded with no warning. A voucher whose linking transaction itself matches the target (same ore in the same currency, compatible cash account, date in the window) is now returned as the twin with transaction_id set. All candidate picks keep explicit total-order tiebreakers so a force re-detect returns the same candidate the user reviewed, and the SEK-or-null amount contract is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(transactions): offer match/ignore for sibling duplicates and route all 409s into the dialog The duplicate dialog hid its match action for sibling-transaction candidates (canMatch required transaction_id === null), so the user who most needed steering saw only 'Bokfor anda'. manualLink explicitly allows N:1 links, so the match action is now offered for both candidate kinds. Sibling candidates get question-form body copy ('vill du matcha mot verifikatet i stallet?') and an additional 'Ignorera transaktionen' action via the existing POST /api/transactions/[id]/ignore, which is the correct resolution when the row itself is a duplicate import (matching would double-count the bank side, booking the ledger side). Two clients dead-ended the TRANSACTION_BOOK_POSSIBLE_DUPLICATE 409 in a destructive toast with no way forward: - the counterparty-template branch of handleQuickReviewConfirm now sets the shared duplicateWarning state exactly like runCategorize, with the force retry bound to the reviewed candidate's voucher - BankReconciliationView's quick-book now opens the same dialog, with match/ignore refreshing the reconciliation lists New sv/en strings: dialog_duplicate_body_sibling, dialog_duplicate_ignore, dialog_duplicate_ignore_failed. File-level parity tests pin the 409 routing and the dialog affordances. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(transactions): duplicate guard on the bulk-book samlingsverifikation path /api/transactions/bulk-book never called detectBookingDuplicate, so a batch containing an already-booked twin minted a second verifikat with no warning. The route now runs the shared per-tx guard before the RPC, with intra-batch exclusions (the other selected txs are distinct events the user picked, and the link-existing target voucher is the batch's own destination), returning 409 TRANSACTION_BOOK_POSSIBLE_DUPLICATE with the candidate and the flagged tx id. BulkBookDialog routes the 409 into DuplicateBookingDialog for review (view voucher / cancel / book anyway) instead of a dead-end toast; 'Bokfor anda' re-runs the batch with force=true. On force the route re-detects and records each dismissed candidate as BankTransactionDuplicateDismissed in behandlingshistorik (BFNAR 2013:2 kap 8), parity with the /categorize bypass. Detection failures stay fail-open. Note: the MCP RPC twin (gnubok_bulk_book_transactions) bypasses this route and remains unguarded; guarding inside the RPC needs a migration and is out of scope here. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(transactions): gate the duplicate-dialog ignore hint on the action being present The sibling body copy mentioned ignoring the row, but two render sites (the manual booking form and the bulk dialog) show sibling candidates without the ignore action. The guidance now lives in a separate dialog_duplicate_ignore_hint string rendered only when the Ignorera button itself renders, so copy never points at a button that is not there. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Jakob Wennberg
co-authored by
Claude Fable 5
Jakob Wennberg
Claude Fable 5
Jakob Wennberg
parent
1b829883ae
commit
0d3ba5268d
@@ -33,16 +33,30 @@ vi.mock('@/lib/bookkeeping/template-library', () => ({
|
||||
applyTemplate: vi.fn(),
|
||||
}))
|
||||
|
||||
// Stubbed so the queued supabase mock stays aligned with the route's own
|
||||
// Stubbed so the queued mock stays aligned with the route's own
|
||||
// queries; the helper's behaviour is covered in
|
||||
// lib/transactions/__tests__/inbox-underlag.test.ts.
|
||||
vi.mock('@/lib/transactions/inbox-underlag', () => ({
|
||||
propagateUnderlagForBookedTransaction: vi.fn().mockResolvedValue(undefined),
|
||||
}))
|
||||
|
||||
// The booking-time duplicate guard runs several queries of its own per tx;
|
||||
// stubbing it keeps the queued supabase mock aligned with the route's queries.
|
||||
// Detection behaviour is covered in
|
||||
// lib/transactions/__tests__/booking-duplicate-detection.test.ts.
|
||||
vi.mock('@/lib/transactions/booking-duplicate-detection', () => ({
|
||||
detectBookingDuplicate: vi.fn().mockResolvedValue(null),
|
||||
}))
|
||||
vi.mock('@/lib/processing-history/append', () => ({
|
||||
appendProcessingHistory: vi.fn().mockResolvedValue(undefined),
|
||||
}))
|
||||
|
||||
import { POST } from '../route'
|
||||
import { applyTemplate } from '@/lib/bookkeeping/template-library'
|
||||
import { propagateUnderlagForBookedTransaction } from '@/lib/transactions/inbox-underlag'
|
||||
import { detectBookingDuplicate } from '@/lib/transactions/booking-duplicate-detection'
|
||||
import { appendProcessingHistory } from '@/lib/processing-history/append'
|
||||
import type { BookedDuplicateCandidate } from '@/lib/transactions/booking-duplicate-detection'
|
||||
|
||||
const TX1 = '11111111-1111-4111-8111-111111111111'
|
||||
const TX2 = '22222222-2222-4222-8222-222222222222'
|
||||
@@ -56,6 +70,17 @@ describe('POST /api/transactions/bulk-book', () => {
|
||||
vi.clearAllMocks()
|
||||
reset()
|
||||
mockSupabase.auth.getUser.mockResolvedValue({ data: { user: mockUser } })
|
||||
vi.mocked(detectBookingDuplicate).mockResolvedValue(null)
|
||||
})
|
||||
|
||||
it('returns 401 when unauthenticated', async () => {
|
||||
mockSupabase.auth.getUser.mockResolvedValue({ data: { user: null }, error: null })
|
||||
const request = createMockRequest('/api/transactions/bulk-book', {
|
||||
method: 'POST',
|
||||
body: { tx_ids: [TX1], existing_journal_entry_id: JE },
|
||||
})
|
||||
const response = await POST(request)
|
||||
expect(response.status).toBe(401)
|
||||
})
|
||||
|
||||
it('returns 400 when neither template_id nor existing_journal_entry_id is set', async () => {
|
||||
@@ -431,3 +456,142 @@ describe('POST /api/transactions/bulk-book: mixed-currency guard', () => {
|
||||
expect(response.status).toBe(200)
|
||||
})
|
||||
})
|
||||
|
||||
/**
|
||||
* Booking-time duplicate guard on the samlingsverifikation path (parity with
|
||||
* /categorize and /book): before this, bulk-book never called
|
||||
* detectBookingDuplicate at all, so a batch containing an already-booked
|
||||
* twin minted a second verifikat with no warning.
|
||||
*/
|
||||
describe('POST /api/transactions/bulk-book: duplicate guard', () => {
|
||||
const mockUser = { id: 'user-1', email: 'test@test.se' }
|
||||
|
||||
const SEK_TXS = [
|
||||
{ id: TX1, amount: 100, currency: 'SEK', description: 'Swish 1', date: '2026-06-05' },
|
||||
{ id: TX2, amount: 200, currency: 'SEK', description: 'Swish 2', date: '2026-06-05' },
|
||||
]
|
||||
|
||||
const CANDIDATE: BookedDuplicateCandidate = {
|
||||
transaction_id: null,
|
||||
journal_entry_id: '55555555-5555-4555-8555-555555555555',
|
||||
voucher_label: 'A17',
|
||||
entry_date: '2026-06-05',
|
||||
description: 'Swish inbetalning',
|
||||
amount: 200,
|
||||
account_number: '1930',
|
||||
currency: null,
|
||||
amount_in_currency: null,
|
||||
amount_verified: true,
|
||||
unverified_reason: null,
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
reset()
|
||||
mockSupabase.auth.getUser.mockResolvedValue({ data: { user: mockUser } })
|
||||
vi.mocked(detectBookingDuplicate).mockResolvedValue(null)
|
||||
})
|
||||
|
||||
it('returns 409 with the candidate and the flagged tx, before the RPC', async () => {
|
||||
enqueue({ data: SEK_TXS, error: null })
|
||||
// TX1 clean, TX2 flagged (checked in tx_ids order).
|
||||
vi.mocked(detectBookingDuplicate)
|
||||
.mockResolvedValueOnce(null)
|
||||
.mockResolvedValueOnce(CANDIDATE)
|
||||
|
||||
const request = createMockRequest('/api/transactions/bulk-book', {
|
||||
method: 'POST',
|
||||
body: { tx_ids: [TX1, TX2], existing_journal_entry_id: JE },
|
||||
})
|
||||
const response = await POST(request)
|
||||
const { status, body } = await parseJsonResponse<{
|
||||
error: { code: string; details?: { transaction_id?: string; candidate?: { journal_entry_id: string } } }
|
||||
}>(response)
|
||||
expect(status).toBe(409)
|
||||
expect(body.error.code).toBe('TRANSACTION_BOOK_POSSIBLE_DUPLICATE')
|
||||
expect(body.error.details?.transaction_id).toBe(TX2)
|
||||
expect(body.error.details?.candidate?.journal_entry_id).toBe(CANDIDATE.journal_entry_id)
|
||||
expect(mockSupabase.rpc).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('passes intra-batch exclusions so siblings and the link target never flag each other', async () => {
|
||||
enqueue({ data: SEK_TXS, error: null })
|
||||
// RPC + event re-fetch for the clean pass-through.
|
||||
enqueue({
|
||||
data: {
|
||||
ok: true, mode: 'link_existing', journal_entry_id: JE,
|
||||
voucher_series: 'A', voucher_number: 12, linked_tx_count: 2, tx_sum: 300,
|
||||
},
|
||||
error: null,
|
||||
})
|
||||
enqueue({ data: [], error: null })
|
||||
|
||||
const request = createMockRequest('/api/transactions/bulk-book', {
|
||||
method: 'POST',
|
||||
body: { tx_ids: [TX1, TX2], existing_journal_entry_id: JE },
|
||||
})
|
||||
const response = await POST(request)
|
||||
expect(response.status).toBe(200)
|
||||
expect(detectBookingDuplicate).toHaveBeenCalledTimes(2)
|
||||
expect(detectBookingDuplicate).toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
'company-1',
|
||||
expect.objectContaining({ id: TX1, date: '2026-06-05', amount: 100 }),
|
||||
{ excludeTransactionIds: [TX1, TX2], excludeJournalEntryIds: [JE] },
|
||||
)
|
||||
})
|
||||
|
||||
it('force=true books through and records each dismissed candidate in behandlingshistorik', async () => {
|
||||
enqueue({ data: SEK_TXS, error: null })
|
||||
enqueue({
|
||||
data: {
|
||||
ok: true, mode: 'link_existing', journal_entry_id: JE,
|
||||
voucher_series: 'A', voucher_number: 12, linked_tx_count: 2, tx_sum: 300,
|
||||
},
|
||||
error: null,
|
||||
})
|
||||
enqueue({ data: [], error: null })
|
||||
// Re-detection under force: TX1 clean, TX2 had the candidate.
|
||||
vi.mocked(detectBookingDuplicate)
|
||||
.mockResolvedValueOnce(null)
|
||||
.mockResolvedValueOnce(CANDIDATE)
|
||||
|
||||
const request = createMockRequest('/api/transactions/bulk-book', {
|
||||
method: 'POST',
|
||||
body: { tx_ids: [TX1, TX2], existing_journal_entry_id: JE, force: true },
|
||||
})
|
||||
const response = await POST(request)
|
||||
expect(response.status).toBe(200)
|
||||
expect(appendProcessingHistory).toHaveBeenCalledTimes(1)
|
||||
expect(appendProcessingHistory).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
aggregateId: TX2,
|
||||
eventType: 'BankTransactionDuplicateDismissed',
|
||||
payload: expect.objectContaining({
|
||||
dismissed_journal_entry_id: CANDIDATE.journal_entry_id,
|
||||
via: 'bulk_book_force',
|
||||
}),
|
||||
}),
|
||||
)
|
||||
})
|
||||
|
||||
it('fails open when detection itself throws (a guard failure never blocks a booking)', async () => {
|
||||
enqueue({ data: SEK_TXS, error: null })
|
||||
enqueue({
|
||||
data: {
|
||||
ok: true, mode: 'link_existing', journal_entry_id: JE,
|
||||
voucher_series: 'A', voucher_number: 12, linked_tx_count: 2, tx_sum: 300,
|
||||
},
|
||||
error: null,
|
||||
})
|
||||
enqueue({ data: [], error: null })
|
||||
vi.mocked(detectBookingDuplicate).mockRejectedValue(new Error('detector down'))
|
||||
|
||||
const request = createMockRequest('/api/transactions/bulk-book', {
|
||||
method: 'POST',
|
||||
body: { tx_ids: [TX1, TX2], existing_journal_entry_id: JE },
|
||||
})
|
||||
const response = await POST(request)
|
||||
expect(response.status).toBe(200)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -12,6 +12,8 @@ import {
|
||||
} from '@/lib/bookkeeping/dimension-rules'
|
||||
import { bookkeepingErrorResponse } from '@/lib/bookkeeping/errors'
|
||||
import { propagateUnderlagForBookedTransaction } from '@/lib/transactions/inbox-underlag'
|
||||
import { detectBookingDuplicate } from '@/lib/transactions/booking-duplicate-detection'
|
||||
import { appendProcessingHistory } from '@/lib/processing-history/append'
|
||||
import { eventBus } from '@/lib/events/bus'
|
||||
import { ensureInitialized } from '@/lib/init'
|
||||
import type { BookingTemplateLibraryLine, Transaction } from '@/types'
|
||||
@@ -88,7 +90,7 @@ export const POST = withRouteContext(
|
||||
// need the currencies for the homogeneity gate below.
|
||||
const { data: txs, error: txError } = await supabase
|
||||
.from('transactions')
|
||||
.select('id, amount, currency, description, date')
|
||||
.select('id, amount, currency, description, date, amount_sek, exchange_rate, cash_account_id')
|
||||
.in('id', body.tx_ids)
|
||||
.eq('company_id', companyId)
|
||||
|
||||
@@ -102,7 +104,10 @@ export const POST = withRouteContext(
|
||||
})
|
||||
}
|
||||
|
||||
const txTyped = txs as Pick<Transaction, 'id' | 'amount' | 'currency' | 'description' | 'date'>[]
|
||||
const txTyped = txs as Pick<
|
||||
Transaction,
|
||||
'id' | 'amount' | 'currency' | 'description' | 'date' | 'amount_sek' | 'exchange_rate' | 'cash_account_id'
|
||||
>[]
|
||||
|
||||
// Currency homogeneity, enforced BEFORE the branch split so it covers
|
||||
// all three paths (template, manual_lines, existing_journal_entry_id).
|
||||
@@ -143,6 +148,100 @@ export const POST = withRouteContext(
|
||||
})
|
||||
}
|
||||
|
||||
// Booking-time duplicate guard, parity with /categorize and /book: each
|
||||
// selected tx is about to be anchored to a verifikat, and a twin already
|
||||
// in the ledger means one affärshändelse gets booked twice (felaktig
|
||||
// bokföring per BFL). Per-tx detection with intra-batch exclusions: the
|
||||
// OTHER selected txs are distinct events the user explicitly picked, and
|
||||
// the link-existing target voucher is the batch's own destination, so
|
||||
// neither may flag. Checked in tx_ids order so the flagged tx is
|
||||
// deterministic. Soft guard: the caller re-runs with force=true after the
|
||||
// user reviews the candidate; detection failures never block a booking.
|
||||
const txById = new Map(txTyped.map((tx) => [tx.id, tx]))
|
||||
const duplicateExclusions = {
|
||||
excludeTransactionIds: body.tx_ids,
|
||||
excludeJournalEntryIds: body.existing_journal_entry_id ? [body.existing_journal_entry_id] : [],
|
||||
}
|
||||
const detectForTx = (tx: (typeof txTyped)[number]) =>
|
||||
detectBookingDuplicate(
|
||||
supabase,
|
||||
companyId!,
|
||||
{
|
||||
id: tx.id,
|
||||
date: tx.date,
|
||||
// `amount` is denominated in `currency`; the guard's FX contract
|
||||
// needs the row's own conversion fields alongside.
|
||||
amount: tx.amount,
|
||||
currency: tx.currency ?? null,
|
||||
amount_sek: tx.amount_sek ?? null,
|
||||
exchange_rate: tx.exchange_rate ?? null,
|
||||
cash_account_id: tx.cash_account_id ?? null,
|
||||
},
|
||||
duplicateExclusions,
|
||||
)
|
||||
if (body.force !== true) {
|
||||
for (const txId of body.tx_ids) {
|
||||
const tx = txById.get(txId)
|
||||
if (!tx) continue
|
||||
let candidate = null
|
||||
try {
|
||||
candidate = await detectForTx(tx)
|
||||
} catch (err) {
|
||||
opLog.warn('bulk-book duplicate detection failed (continuing)', { err, txId })
|
||||
}
|
||||
if (candidate) {
|
||||
return errorResponseFromCode('TRANSACTION_BOOK_POSSIBLE_DUPLICATE', opLog, {
|
||||
requestId,
|
||||
details: { candidate, transaction_id: txId },
|
||||
})
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// force=true bypassed the guard. Booking over a DETECTED possible
|
||||
// double-booking is a bookkeeping decision that needs a durable
|
||||
// behandlingshistorik record (BFNAR 2013:2 kap 8), parity with the
|
||||
// /categorize and agent bypass paths. Best-effort; never blocks.
|
||||
for (const txId of body.tx_ids) {
|
||||
const tx = txById.get(txId)
|
||||
if (!tx) continue
|
||||
try {
|
||||
const dismissed = await detectForTx(tx)
|
||||
if (!dismissed) continue
|
||||
opLog.warn('bulk-book duplicate guard bypassed', {
|
||||
reason: 'force=true',
|
||||
requestId,
|
||||
txId,
|
||||
dismissedJournalEntryId: dismissed.journal_entry_id,
|
||||
})
|
||||
await appendProcessingHistory({
|
||||
companyId: companyId!,
|
||||
correlationId: txId,
|
||||
aggregateType: 'BankTransaction',
|
||||
aggregateId: txId,
|
||||
eventType: 'BankTransactionDuplicateDismissed',
|
||||
payload: {
|
||||
transaction_id: txId,
|
||||
dismissed_transaction_id: dismissed.transaction_id,
|
||||
dismissed_journal_entry_id: dismissed.journal_entry_id,
|
||||
// Null when the candidate's SEK value could not be established;
|
||||
// the foreign figures below then carry the durable record.
|
||||
amount_ore: dismissed.amount != null ? Math.round(dismissed.amount * 100) : null,
|
||||
dismissed_currency: dismissed.currency,
|
||||
dismissed_amount_in_currency: dismissed.amount_in_currency,
|
||||
entry_date: dismissed.entry_date,
|
||||
amount_verified: dismissed.amount_verified,
|
||||
unverified_reason: dismissed.unverified_reason,
|
||||
via: 'bulk_book_force',
|
||||
},
|
||||
actor: { type: 'user', id: user.id },
|
||||
occurredAt: new Date(),
|
||||
})
|
||||
} catch (err) {
|
||||
opLog.error('failed to append duplicate-dismissal behandlingshistorik', err as Error)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Three paths now (PR #608):
|
||||
// 1. existing_journal_entry_id → null new_entry, RPC links txs to JE.
|
||||
// 2. template_id → route expands template per mode, builds lines.
|
||||
|
||||
Reference in New Issue
Block a user