diff --git a/DECISIONS.md b/DECISIONS.md index c38bcbca..1a54d244 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1055,6 +1055,7 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-08-18] Shopify webshop_orders port: vat_breakdown is reconstructed from the ORDER-LEVEL taxLines (net = tax / rate, remainder as a 0%-bucket, refuse on missing rates or overshoot) instead of summing line items like the WooCommerce sync: Shopify's discountedTotalSet excludes cart-level discount allocations and lineItems is a paginated connection, so part-summing can silently produce a wrong per-rate net, while tax-per-rate and the charged total are authoritative order-level facts. Refund VAT is always prorated from the parent's mix (Shopify's Refund object exposes no per-rate tax without paging refundLineItems per refund). [2026-08-18] Shopify order feed keeps its paid-only qualification (PAID/PARTIALLY_REFUNDED/REFUNDED) after the webshop_orders port, unlike WooCommerce which also imports unpaid orders for the invoice flow: widening qualification is a product decision, out of scope for the port; unpaid orders re-surface via updatedAt when payment captures. The line-item snapshot is stored only when the parts reconstruct the charged total to the ore (else [] and the invoice conversion falls back to one aggregate line), and the bookkeeping-lock row filter was dropped: an Orders-page row behind the lock is an overview row, not permanent inbox noise, and booking is still blocked by the lock triggers (parity with WooCommerce). [2026-08-18] Skattekontoutdrag sum mismatch (opening + events != closing) demoted from a hard 400 to a preview confirm gate showing ingående/händelser/utgående/differens, mirroring the orgnr-mismatch gate: Sebastian's real export was refused on it (2026-08-18) with no way forward and no figures to diagnose; nothing is booked at import and dedup makes a later complete re-import safe, so refusing the file only blocked the rows that WERE readable. Parser also takes the earliest opening / latest closing across several marker pairs, reads a marker saldo from a trailing running-saldo column, and accepts U+2212 / plus-sign amounts; the route logs the figures (amounts and counts, never row text) so the next report is diagnosable from Vercel logs. Kept the hard reject only for zero readable rows. +[2026-08-18] Categorizing an ignored transaction atomically clears is_ignored in the batch route, single-transaction routes, and shared categorization core: categorization is explicit intent to book the row, and one update preserves transactions_is_ignored_no_journal_entry without forcing a separate unignore-and-retry action; the constraint name maps to TX_CATEGORIZE_IGNORED_CONFLICT as defense in depth. [2026-08-18] Issue #1668 keeps VAT confirmation in a dedicated sticky end column and makes truncated source names reveal on hover, focus, and activation: column truncation alone would still strand the hard-blocking action at responsive widths, while activation gives touch users the same full-text affordance. [2026-08-18] Issue #1659 exposes one canonical per-period VAT deadline resolver from deadline-config and makes both the MCP close check and VAT period default consume it: monthly, quarterly, annual, over-40M, and January/August rules must not drift across parallel formulas again; the MCP adapter alone applies the same banking-day adjustment as generated tax deadlines. [2026-08-18] PR #1679 reports deadline_unavailable instead of guessing when VAT settings are missing, and annual AB deadlines require the configured fiscal year to match the resolved report period: a missing or stale filing profile must not produce a plausible but legally wrong date. @@ -1067,3 +1068,4 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-08-18] Skatteverket read data is visible to every company member, no new role gate (#1673): token rows are per (user, company) but the fetched skattekonto/declaration data belongs to the company, and viewers already read `skattekonto_transactions` and the local snapshot with no role check; membership (dispatcher-resolved ctx.companyId + company-scoped SELECT policy on `skatteverket_tokens`) is the gate. Reads resolve the caller's own token first, then the most recently issued active token of any member (all rows ordered, never `.maybeSingle()`, which errored once two members had connected). Writes (moms utkast/las/submit, AGI submit/spara/las, connect/disconnect, /status) stay on the caller's own token: BankID signing is personal. [2026-08-18] AGI receipt fallback (#1597): GET /agi/status serves the signed record from agi_declarations (kvittensnummer, response_data.signeradAv/signeradTid, submitted_at) only when the agi_submission_{period} cache is absent, and the declaration-sourced record deliberately carries NO salaryRunId: the period row is UNIQUE per company+period and regenerating a correction repoints its salary_run_id at the correction run while the stored kvittens still belongs to the original, so trusting the column would render the correction as filed with a superseded receipt. Ownership rests on signeradTid/submittedAt vs the run's agi_submitted_at stamp (same value) plus updatedAt = submitted_at, which predates any later correction's XML. Cache present still wins because it is the only place the in-flight states live. Rejected: a second client fetch in AGIPanel (two sources of truth for one card) and merging both records in the route (mixes another declaration's fields into an in-flight state). [2026-08-18] Invoice ROT/RUT personnummer surfaces (detail page, invoice PDF, preview PDF, editor kept-hint) switch to the payroll mask convention YYYYMMDD-XXXX (birth date visible, last four hidden), computed on read from deduction_personnummer_encrypted via lib/invoices/deduction-personnummer.ts: no schema change, nothing stored, never throws (bad ciphertext logs and renders no personnummer). The browser gets the mask from GET /api/invoices/[id]/rot-rut and never both the mask and the last four (that is the full number); v1 REST and MCP keep deduction_personnummer_last4 for compatibility (an additive deduction_personnummer_masked is a possible follow-up). InvoicePDF derives the mask itself when the caller passes the stored row, so none of the 11 render call sites can silently drop the personnummer; the preview route passes an already-masked value since it only has plaintext. The separate Skattereduktion card on the invoice detail page is folded into Detaljer as plain rows (Personnummer, Fastighet, Skattereduktion status with the begäran lifecycle) per founder decision 2026-08-18: it duplicated the totals block. +[2026-08-19] Keep reversal allocation metadata limited to failures before any reversal header exists: later cleanup preserves a cancelled header with the allocated voucher number, so documenting it as an unused voucher gap would be false. diff --git a/app/api/pending-operations/[id]/commit/__tests__/route.test.ts b/app/api/pending-operations/[id]/commit/__tests__/route.test.ts index 1ad975bc..b4b46529 100644 --- a/app/api/pending-operations/[id]/commit/__tests__/route.test.ts +++ b/app/api/pending-operations/[id]/commit/__tests__/route.test.ts @@ -36,6 +36,10 @@ vi.mock('@/lib/bookkeeping/transaction-entries', () => ({ createTransactionJournalEntry: (...args: unknown[]) => mockCreateJournalEntry(...args), })) +vi.mock('@/lib/transactions/booking-duplicate-detection', () => ({ + detectBookingDuplicate: vi.fn().mockResolvedValue(null), +})) + // Mock VAT validation vi.mock('@/lib/vat/vies-client', () => ({ validateVatNumber: vi.fn().mockResolvedValue({ valid: true }), @@ -130,7 +134,7 @@ describe('POST /api/pending-operations/:id/commit', () => { { data: tx }, // fetch transaction { data: settings }, // fetch company settings { data: [{ id: 'fp-1' }] }, // fiscal period check - { data: null, error: null }, // update transaction + { data: [{ id: 'tx-1' }], error: null }, // transaction CAS matched { data: null, error: null }, // upsert counterparty template { data: null, error: null }, // update pending op status ]) diff --git a/app/api/transactions/[id]/book/__tests__/route.test.ts b/app/api/transactions/[id]/book/__tests__/route.test.ts index 8de3896a..f9d81857 100644 --- a/app/api/transactions/[id]/book/__tests__/route.test.ts +++ b/app/api/transactions/[id]/book/__tests__/route.test.ts @@ -36,6 +36,11 @@ vi.mock('@/lib/bookkeeping/engine', () => ({ createJournalEntry: (...args: unknown[]) => mockCreateJournalEntry(...args), })) +const mockReverseOrphanedJournalEntry = vi.fn() +vi.mock('@/lib/bookkeeping/cancel-orphaned-entry', () => ({ + reverseOrphanedJournalEntry: (...args: unknown[]) => mockReverseOrphanedJournalEntry(...args), +})) + // Booking-time duplicate guard: mocked so route tests exercise the WIRING // (warn / force / mismatch); the detection query itself is unit-tested in // lib/transactions/__tests__/booking-duplicate-detection.test.ts. @@ -79,6 +84,7 @@ describe('POST /api/transactions/[id]/book', () => { // No booking-duplicate by default; guard tests override per-case. mockDetectDup.mockResolvedValue(null) mockAppendProcessingHistory.mockResolvedValue('evt-1') + mockReverseOrphanedJournalEntry.mockResolvedValue(undefined) }) it('returns 401 when not authenticated', async () => { @@ -197,7 +203,7 @@ describe('POST /api/transactions/[id]/book', () => { mockCreateJournalEntry.mockResolvedValue(je) // Update transaction - enqueue({ data: null, error: null }) + enqueue({ data: [{ id: 'tx-1' }], error: null }) const emitSpy = vi.spyOn(eventBus, 'emit') @@ -231,6 +237,32 @@ describe('POST /api/transactions/[id]/book', () => { ) }) + it('atomically unignores an ignored transaction when booking it', async () => { + const tx = makeTransaction({ + id: 'tx-1', + amount: -500, + journal_entry_id: null, + is_ignored: true, + }) + enqueue({ data: tx, error: null }) + mockCreateJournalEntry.mockResolvedValue(makeJournalEntry({ id: 'je-new' })) + enqueue({ data: [{ id: 'tx-1' }], error: null }) + + const response = await POST( + createMockRequest('/api/transactions/tx-1/book', { method: 'POST', body: validBody }), + createMockRouteParams({ id: 'tx-1' }), + ) + + expect(response.status).toBe(200) + expect(findCalls('transactions', 'update')).toContainEqual([ + expect.objectContaining({ + journal_entry_id: 'je-new', + is_business: true, + is_ignored: false, + }), + ]) + }) + it('returns 500 when transaction update fails', async () => { const tx = makeTransaction({ id: 'tx-1', journal_entry_id: null }) const je = makeJournalEntry({ id: 'je-new' }) @@ -245,10 +277,72 @@ describe('POST /api/transactions/[id]/book', () => { body: validBody, }) const response = await POST(request, createMockRouteParams({ id: 'tx-1' })) - const { status, body } = await parseJsonResponse<{ error: string }>(response) + const { status, body } = await parseJsonResponse<{ + error: { code: string; message: string } + }>(response) expect(status).toBe(500) - expect(body.error).toBe('Failed to update transaction') + expect(body.error).toMatchObject({ + code: 'INTERNAL_ERROR', + message: 'Ett oväntat serverfel uppstod. Försök igen senare.', + }) + expect(mockReverseOrphanedJournalEntry).toHaveBeenCalledWith( + expect.anything(), + 'company-1', + 'user-1', + 'je-new', + expect.any(String), + ) + }) + + it('stornos the posted orphan when another booking wins the transaction-link race', async () => { + const tx = makeTransaction({ id: 'tx-1', journal_entry_id: null }) + const je = makeJournalEntry({ id: 'je-new' }) + + enqueue({ data: tx, error: null }) + mockCreateJournalEntry.mockResolvedValue(je) + enqueue({ data: [], error: null }) + + const response = await POST( + createMockRequest('/api/transactions/tx-1/book', { method: 'POST', body: validBody }), + createMockRouteParams({ id: 'tx-1' }), + ) + const { status, body } = await parseJsonResponse<{ error: { code: string } }>(response) + + expect(status).toBe(409) + expect(body.error.code).toBe('TX_CATEGORIZE_RACE') + expect(mockReverseOrphanedJournalEntry).toHaveBeenCalledWith( + expect.anything(), + 'company-1', + 'user-1', + 'je-new', + expect.any(String), + ) + }) + + it('maps the ignored-row constraint to a typed conflict and stornos the posted orphan', async () => { + const tx = makeTransaction({ id: 'tx-1', journal_entry_id: null, is_ignored: true }) + enqueue({ data: tx, error: null }) + mockCreateJournalEntry.mockResolvedValue(makeJournalEntry({ id: 'je-new' })) + enqueue({ + data: null, + error: { + code: '23514', + message: + 'new row for relation "transactions" violates check constraint "transactions_is_ignored_no_journal_entry"', + }, + }) + + const response = await POST( + createMockRequest('/api/transactions/tx-1/book', { method: 'POST', body: validBody }), + createMockRouteParams({ id: 'tx-1' }), + ) + const { status, body } = await parseJsonResponse<{ error: { code: string; message: string } }>(response) + + expect(status).toBe(409) + expect(body.error.code).toBe('TX_CATEGORIZE_IGNORED_CONFLICT') + expect(body.error.message).not.toContain('check constraint') + expect(mockReverseOrphanedJournalEntry).toHaveBeenCalledTimes(1) }) // ── Underlag propagation (pinned document + matched inbox items) ────── @@ -262,7 +356,7 @@ describe('POST /api/transactions/[id]/book', () => { const je = makeJournalEntry({ id: 'je-new' }) enqueue({ data: tx, error: null }) // fetch transaction mockCreateJournalEntry.mockResolvedValue(je) - enqueue({ data: null, error: null }) // update transaction + enqueue({ data: [{ id: 'tx-1' }], error: null }) // update transaction enqueue({ data: { document_id: 'doc-1' } }) // propagate: tx pin lookup enqueue({ data: { journal_entry_id: null } }) // pinned doc unanchored enqueue({ data: { id: 'je-new' } }) // linkToJournalEntry: entry ownership check @@ -287,7 +381,7 @@ describe('POST /api/transactions/[id]/book', () => { const je = makeJournalEntry({ id: 'je-new' }) enqueue({ data: tx, error: null }) // fetch transaction mockCreateJournalEntry.mockResolvedValue(je) - enqueue({ data: null, error: null }) // update transaction + enqueue({ data: [{ id: 'tx-1' }], error: null }) // update transaction enqueue({ data: { document_id: null } }) // propagate: nothing pinned enqueue({ data: [{ id: 'i1', document_id: null }] }) // matched inbox item enqueue({ data: null }) // stamp update @@ -310,7 +404,7 @@ describe('POST /api/transactions/[id]/book', () => { const je = makeJournalEntry({ id: 'je-new' }) enqueue({ data: tx, error: null }) // fetch transaction mockCreateJournalEntry.mockResolvedValue(je) - enqueue({ data: null, error: null }) // update transaction + enqueue({ data: [{ id: 'tx-1' }], error: null }) // update transaction enqueue({ data: { document_id: 'doc-1' } }) // propagate: tx pin lookup enqueue({ data: { journal_entry_id: null } }) // pinned doc unanchored enqueue({ data: null }) // linkToJournalEntry: entry lookup fails -> throws @@ -364,7 +458,7 @@ describe('POST /api/transactions/[id]/book', () => { const tx = makeTransaction({ id: 'tx-1', amount: -500, journal_entry_id: null }) const je = makeJournalEntry({ id: 'je-new' }) enqueue({ data: tx, error: null }) // fetch - enqueue({ data: null, error: null }) // update + enqueue({ data: [{ id: 'tx-1' }], error: null }) // update mockDetectDup.mockResolvedValue({ transaction_id: SIBLING_UUID, journal_entry_id: 'je-existing', @@ -431,7 +525,7 @@ describe('POST /api/transactions/[id]/book', () => { const tx = makeTransaction({ id: 'tx-1', amount: 98565, journal_entry_id: null }) const je = makeJournalEntry({ id: 'je-new' }) enqueue({ data: tx, error: null }) // fetch - enqueue({ data: null, error: null }) // update + enqueue({ data: [{ id: 'tx-1' }], error: null }) // update mockDetectDup.mockResolvedValue({ transaction_id: null, journal_entry_id: VOUCHER_JE_UUID, diff --git a/app/api/transactions/[id]/book/route.ts b/app/api/transactions/[id]/book/route.ts index c4d55652..7879c1a5 100644 --- a/app/api/transactions/[id]/book/route.ts +++ b/app/api/transactions/[id]/book/route.ts @@ -3,12 +3,13 @@ import { eventBus } from '@/lib/events' import { ensureInitialized } from '@/lib/init' import { withRouteContext } from '@/lib/api/with-route-context' import { createJournalEntry } from '@/lib/bookkeeping/engine' +import { reverseOrphanedJournalEntry } from '@/lib/bookkeeping/cancel-orphaned-entry' import { bookkeepingErrorResponse } from '@/lib/bookkeeping/errors' import { validateBody } from '@/lib/api/validate' import { BookTransactionSchema } from '@/lib/api/schemas' import { detectBookingDuplicate } from '@/lib/transactions/booking-duplicate-detection' import { propagateUnderlagForBookedTransaction } from '@/lib/transactions/inbox-underlag' -import { errorResponseFromCode } from '@/lib/errors/get-structured-error' +import { errorResponse, errorResponseFromCode } from '@/lib/errors/get-structured-error' import { getErrorMessage } from '@/lib/errors/get-error-message' import { appendProcessingHistory } from '@/lib/processing-history/append' import type { Transaction } from '@/types' @@ -17,7 +18,7 @@ ensureInitialized() export const POST = withRouteContext<{ params: Promise<{ id: string }> }>( 'transaction.book', - async (request, { supabase, user, companyId, log }, { params }) => { + async (request, { supabase, user, companyId, log, requestId }, { params }) => { const { id } = await params const validation = await validateBody(request, BookTransactionSchema) @@ -166,22 +167,46 @@ export const POST = withRouteContext<{ params: Promise<{ id: string }> }>( } // Link transaction to the journal entry - const { error: updateError } = await supabase + const { data: updateResult, error: updateError } = await supabase .from('transactions') .update({ journal_entry_id: journalEntry.id, is_business: true, + is_ignored: false, category: 'uncategorized', }) .eq('id', id) + .eq('company_id', companyId) + .is('journal_entry_id', null) + .select('*') if (updateError) { - return NextResponse.json( - { error: 'Failed to update transaction' }, - { status: 500 } + await reverseOrphanedJournalEntry( + supabase, + companyId, + user.id, + journalEntry.id, + 'Bokföringsverifikation utan transaktionskoppling; automatisk storno misslyckades. Manuell avstämning krävs.', ) + return errorResponse(updateError, log, { requestId }) } + if (!updateResult || updateResult.length === 0) { + // CAS guard: another request linked this transaction after our read. The + // posted orphan is immutable, so compensate through the engine with a + // storno entry instead of overwriting the winning journal entry link. + await reverseOrphanedJournalEntry( + supabase, + companyId, + user.id, + journalEntry.id, + 'Bokföringsverifikation utan transaktionskoppling; automatisk storno misslyckades. Manuell avstämning krävs.', + ) + return errorResponseFromCode('TX_CATEGORIZE_RACE', log, { requestId }) + } + + const updatedTransaction = updateResult[0] as Transaction + // A hunt- or hand-matched inbox item is consumed by this booking even // though the dialog never saw it: link its underlag to the verifikat and // stamp it so it leaves the active inbox (best-effort, logged inside). @@ -192,7 +217,7 @@ export const POST = withRouteContext<{ params: Promise<{ id: string }> }>( await eventBus.emit({ type: 'transaction.categorized', payload: { - transaction: transaction as Transaction, + transaction: updatedTransaction, account: lines[0]?.account_number || '', taxCode: '', userId: user.id, diff --git a/app/api/transactions/[id]/categorize/__tests__/route.test.ts b/app/api/transactions/[id]/categorize/__tests__/route.test.ts index c92f62d2..47742465 100644 --- a/app/api/transactions/[id]/categorize/__tests__/route.test.ts +++ b/app/api/transactions/[id]/categorize/__tests__/route.test.ts @@ -9,7 +9,7 @@ import { import { eventBus } from '@/lib/events' import { JournalEntryNotBalancedError } from '@/lib/bookkeeping/errors' -const { supabase: mockSupabase, enqueue, reset } = createQueuedMockSupabase() +const { supabase: mockSupabase, enqueue, reset, findCalls } = createQueuedMockSupabase() vi.mock('@/lib/supabase/server', () => ({ createClient: () => Promise.resolve(mockSupabase), })) @@ -80,13 +80,10 @@ vi.mock('@/lib/bookkeeping/counterparty-templates', () => ({ upsertCounterpartyTemplate: vi.fn().mockResolvedValue(undefined), })) -// CAS-race compensation is centralized in lib/bookkeeping/cancel-orphaned-entry. -// The route must delegate to it rather than hand-rolling the cancel + the -// voucher_gap_explanations insert (BFNAR 2013:2). The exact insert payload is -// asserted in that helper's own test. -const mockCancelOrphanedPaymentEntry = vi.fn() +// Posted-orphan compensation is centralized and routes through engine storno. +const mockReverseOrphanedJournalEntry = vi.fn() vi.mock('@/lib/bookkeeping/cancel-orphaned-entry', () => ({ - cancelOrphanedPaymentEntry: (...args: unknown[]) => mockCancelOrphanedPaymentEntry(...args), + reverseOrphanedJournalEntry: (...args: unknown[]) => mockReverseOrphanedJournalEntry(...args), })) const mockFindMissingActiveAccounts = vi.fn() @@ -128,10 +125,10 @@ describe('POST /api/transactions/[id]/categorize', () => { // Default: no booking-time duplicate. The dedicated guard test overrides this. mockDetectDup.mockResolvedValue(null) mockAppendProcessingHistory.mockResolvedValue('evt-1') - mockCancelOrphanedPaymentEntry.mockResolvedValue(undefined) + mockReverseOrphanedJournalEntry.mockResolvedValue(undefined) }) - it('delegates the CAS-race orphan to cancelOrphanedPaymentEntry (documented voucher gap)', async () => { + it('delegates the CAS-race orphan to engine-backed storno compensation', async () => { const tx = makeTransaction({ id: 'tx-1', amount: -500, @@ -160,13 +157,13 @@ describe('POST /api/transactions/[id]/categorize', () => { expect((body.error as { code: string }).code).toBe('TX_CATEGORIZE_RACE') // No hand-rolled insert: the helper owns the real column set. - expect(mockCancelOrphanedPaymentEntry).toHaveBeenCalledTimes(1) - expect(mockCancelOrphanedPaymentEntry).toHaveBeenCalledWith( + expect(mockReverseOrphanedJournalEntry).toHaveBeenCalledTimes(1) + expect(mockReverseOrphanedJournalEntry).toHaveBeenCalledWith( expect.anything(), 'company-1', 'user-1', 'je-1', - 'Automatiskt makulerad: dubblettbokning förhindrad av samtidighetsskydd', + 'Kategoriseringsverifikation utan transaktionskoppling; automatisk storno misslyckades. Manuell avstämning krävs.', ) }) @@ -207,7 +204,7 @@ describe('POST /api/transactions/[id]/categorize', () => { // Fetch transaction enqueue({ data: tx, error: null }) // Update transaction - enqueue({ data: null, error: null }) + enqueue({ data: [{ ...tx, is_business: true, category: 'expense_software' }], error: null }) const request = createMockRequest('/api/transactions/tx-1/categorize', { method: 'POST', @@ -225,6 +222,37 @@ describe('POST /api/transactions/[id]/categorize', () => { expect(body.already_had_journal_entry).toBe(true) expect(body.journal_entry_id).toBe('je-existing') expect(mockCreateTransactionJournalEntry).not.toHaveBeenCalled() + expect( + findCalls('transactions', 'eq').filter(([column]) => column === 'company_id'), + ).toHaveLength(2) + }) + + it('returns a race conflict when the guarded update matches no row without creating an entry', async () => { + const tx = makeTransaction({ + id: 'tx-1', + amount: -500, + merchant_name: null, + journal_entry_id: null, + }) + + enqueue({ data: tx, error: null }) + enqueue({ data: { entity_type: 'enskild_firma', fiscal_year_start_month: 1 }, error: null }) + enqueue({ data: [{ id: 'period-1' }], error: null }) + mockCreateTransactionJournalEntry.mockResolvedValueOnce(null) + enqueue({ data: [], error: null }) + + const response = await POST( + createMockRequest('/api/transactions/tx-1/categorize', { + method: 'POST', + body: { is_business: false }, + }), + createMockRouteParams({ id: 'tx-1' }), + ) + const { status, body } = await parseJsonResponse<{ error: { code: string } }>(response) + + expect(status).toBe(409) + expect(body.error.code).toBe('TX_CATEGORIZE_RACE') + expect(mockReverseOrphanedJournalEntry).not.toHaveBeenCalled() }) it('creates journal entry for business expense', async () => { @@ -282,6 +310,46 @@ describe('POST /api/transactions/[id]/categorize', () => { ) }) + it('atomically unignores an ignored transaction when categorizing it', async () => { + const tx = makeTransaction({ + id: 'tx-1', + amount: -500, + merchant_name: null, + journal_entry_id: null, + is_ignored: true, + }) + + enqueue({ data: tx, error: null }) + enqueue({ data: { entity_type: 'enskild_firma', fiscal_year_start_month: 1 }, error: null }) + enqueue({ data: [{ id: 'period-1' }], error: null }) + mockCreateTransactionJournalEntry.mockResolvedValue({ id: 'je-1' }) + enqueue({ data: [{ ...tx, is_business: false, category: 'private', is_ignored: false, journal_entry_id: 'je-1' }], error: null }) + + const categorizedHandler = vi.fn() + eventBus.on('transaction.categorized', categorizedHandler) + + const request = createMockRequest('/api/transactions/tx-1/categorize', { + method: 'POST', + body: { is_business: false }, + }) + const response = await POST(request, createMockRouteParams({ id: 'tx-1' })) + + expect(response.status).toBe(200) + expect(findCalls('transactions', 'update')).toContainEqual([ + expect.objectContaining({ + is_business: false, + category: 'private', + is_ignored: false, + journal_entry_id: 'je-1', + }), + ]) + expect(categorizedHandler).toHaveBeenCalledWith( + expect.objectContaining({ + transaction: expect.objectContaining({ is_ignored: false }), + }), + ) + }) + it('passes body.dimensions onto the mapping result the engine books', async () => { const tx = makeTransaction({ id: 'tx-1', @@ -418,7 +486,7 @@ describe('POST /api/transactions/[id]/categorize', () => { mockCreateTransactionJournalEntry.mockRejectedValue(new Error('Period locked')) // Update transaction - enqueue({ data: null, error: null }) + enqueue({ data: [{ ...tx, is_business: true, category: 'expense_software' }], error: null }) const request = createMockRequest('/api/transactions/tx-1/categorize', { method: 'POST', @@ -454,7 +522,7 @@ describe('POST /api/transactions/[id]/categorize', () => { mockCreateTransactionJournalEntry.mockRejectedValue(new JournalEntryNotBalancedError(100, 80)) // Update transaction - enqueue({ data: null, error: null }) + enqueue({ data: [{ ...tx, is_business: true, category: 'expense_software' }], error: null }) const request = createMockRequest('/api/transactions/tx-1/categorize', { method: 'POST', @@ -502,6 +570,54 @@ describe('POST /api/transactions/[id]/categorize', () => { expect(status).toBe(500) expect((body.error as unknown as { code: string }).code).toBe('INTERNAL_ERROR') + expect(mockReverseOrphanedJournalEntry).toHaveBeenCalledWith( + expect.anything(), + 'company-1', + 'user-1', + 'je-1', + expect.any(String), + ) + }) + + it('maps an ignored-row constraint to a typed conflict and stornos the posted orphan', async () => { + const tx = makeTransaction({ + id: 'tx-1', + journal_entry_id: null, + merchant_name: null, + is_ignored: true, + }) + enqueue({ data: tx, error: null }) + enqueue({ data: { entity_type: 'enskild_firma', fiscal_year_start_month: 1 }, error: null }) + enqueue({ data: [{ id: 'period-1' }], error: null }) + mockCreateTransactionJournalEntry.mockResolvedValue({ id: 'je-1' }) + enqueue({ + data: null, + error: { + code: '23514', + message: + 'new row for relation "transactions" violates check constraint "transactions_is_ignored_no_journal_entry"', + }, + }) + + const response = await POST( + createMockRequest('/api/transactions/tx-1/categorize', { + method: 'POST', + body: { is_business: false }, + }), + createMockRouteParams({ id: 'tx-1' }), + ) + const { status, body } = await parseJsonResponse<{ error: { code: string; message: string } }>(response) + + expect(status).toBe(409) + expect(body.error.code).toBe('TX_CATEGORIZE_IGNORED_CONFLICT') + expect(body.error.message).not.toContain('check constraint') + expect(mockReverseOrphanedJournalEntry).toHaveBeenCalledWith( + expect.anything(), + 'company-1', + 'user-1', + 'je-1', + expect.any(String), + ) }) it('returns 400 when mapping result has empty debit_account', async () => { diff --git a/app/api/transactions/[id]/categorize/route.ts b/app/api/transactions/[id]/categorize/route.ts index 3cfea377..03585d2e 100644 --- a/app/api/transactions/[id]/categorize/route.ts +++ b/app/api/transactions/[id]/categorize/route.ts @@ -5,7 +5,7 @@ import { ensureInitialized } from '@/lib/init' import { buildMappingResultFromCategory } from '@/lib/bookkeeping/category-mapping' import { getTemplateById, buildMappingResultFromTemplate, validateTemplateForEntity } from '@/lib/bookkeeping/booking-templates' import { createTransactionJournalEntry } from '@/lib/bookkeeping/transaction-entries' -import { cancelOrphanedPaymentEntry } from '@/lib/bookkeeping/cancel-orphaned-entry' +import { reverseOrphanedJournalEntry } from '@/lib/bookkeeping/cancel-orphaned-entry' import { detectBookingDuplicate } from '@/lib/transactions/booking-duplicate-detection' import { appendProcessingHistory } from '@/lib/processing-history/append' import { saveUserMappingRule, applySettlementAccount } from '@/lib/bookkeeping/mapping-engine' @@ -138,8 +138,9 @@ export const POST = withRouteContext( const { error: updateErr } = await supabase .from('transactions') - .update({ is_business, category: finalCat }) + .update({ is_business, category: finalCat, is_ignored: false }) .eq('id', id) + .eq('company_id', companyId) if (updateErr) { txLog.error('failed to update already-categorized transaction', updateErr) @@ -921,33 +922,47 @@ export const POST = withRouteContext( .update({ is_business, category: finalCategory, + is_ignored: false, journal_entry_id: journalEntryId, }) .eq('id', id) + .eq('company_id', companyId) .is('journal_entry_id', null) - .select('id') + .select('*') if (updateError) { txLog.error('failed to update transaction', updateError) + if (journalEntryId) { + await reverseOrphanedJournalEntry( + supabase, + companyId, + user.id, + journalEntryId, + 'Kategoriseringsverifikation utan transaktionskoppling; automatisk storno misslyckades. Manuell avstämning krävs.', + ) + } return errorResponse(updateError, txLog, { requestId }) } - if ((!updateResult || updateResult.length === 0) && journalEntryId) { + if (!updateResult || updateResult.length === 0) { // CAS guard: another request set journal_entry_id between our read and - // write. Cancel the orphaned entry and document the voucher gap through - // the shared helper (BFNAR 2013:2), which owns the correct - // voucher_gap_explanations column set and logs failures loudly. - await cancelOrphanedPaymentEntry( - supabase, - companyId, - user.id, - journalEntryId, - 'Automatiskt makulerad: dubblettbokning förhindrad av samtidighetsskydd', - ) + // write. If this request posted an orphan, compensate through the + // bookkeeping engine with a storno entry. + if (journalEntryId) { + await reverseOrphanedJournalEntry( + supabase, + companyId, + user.id, + journalEntryId, + 'Kategoriseringsverifikation utan transaktionskoppling; automatisk storno misslyckades. Manuell avstämning krävs.', + ) + } return errorResponseFromCode('TX_CATEGORIZE_RACE', txLog, { requestId }) } + const updatedTransaction = updateResult[0] as Transaction + // Flag any inbox underlag already matched to this transaction as booked. // The block above only fires when the caller passes an explicit // inbox_item_id (booking straight from the inbox flow). Booking the same @@ -991,7 +1006,7 @@ export const POST = withRouteContext( await eventBus.emit({ type: 'transaction.categorized', payload: { - transaction: transaction as Transaction, + transaction: updatedTransaction, account: mappingResult.debit_account, taxCode: mappingResult.vat_lines[0]?.account_number || '', userId: user.id, diff --git a/app/api/v1/companies/[companyId]/transactions/[id]/categorize/__tests__/route.test.ts b/app/api/v1/companies/[companyId]/transactions/[id]/categorize/__tests__/route.test.ts index 003eee25..18b09539 100644 --- a/app/api/v1/companies/[companyId]/transactions/[id]/categorize/__tests__/route.test.ts +++ b/app/api/v1/companies/[companyId]/transactions/[id]/categorize/__tests__/route.test.ts @@ -69,6 +69,7 @@ vi.mock('@/lib/transactions/inbox-underlag', () => ({ })) import { validateApiKey, createServiceClientNoCookies } from '@/lib/auth/api-keys' +import { withUnusedVoucherAllocation } from '@/lib/bookkeeping/errors' import { POST } from '../route' const mockValidate = validateApiKey as ReturnType @@ -83,6 +84,7 @@ function makeFlexibleSupabase(byTable: Record // Insert payloads are recorded verbatim: the proxy would happily accept a // phantom column, so the assertion has to inspect the object itself. const inserts: Record = {} + const updates: Record = {} const buildChain = (table: string): unknown => { const handler: ProxyHandler = { get(_target, prop) { @@ -95,13 +97,14 @@ function makeFlexibleSupabase(byTable: Record } return (...args: unknown[]) => { if (prop === 'insert') (inserts[table] ??= []).push(args[0]) + if (prop === 'update') (updates[table] ??= []).push(args[0]) return buildChain(table) } }, } return new Proxy({}, handler) } - return { supabase: { from: vi.fn((table: string) => buildChain(table)) }, inserts } + return { supabase: { from: vi.fn((table: string) => buildChain(table)) }, inserts, updates } } const COMPANY_ID = 'aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa' @@ -170,7 +173,7 @@ beforeEach(() => { }) }) -function happyPathSupabase() { +function happyPathSupabase(transactionOverrides: Record = {}) { return makeFlexibleSupabase({ company_members: { data: { company_id: COMPANY_ID, role: 'owner' }, error: null }, transactions: [ @@ -184,6 +187,7 @@ function happyPathSupabase() { merchant_name: 'ICA', cash_account_id: null, journal_entry_id: null, + ...transactionOverrides, }, error: null, }, @@ -244,13 +248,98 @@ describe('POST /api/v1/.../transactions/{id}/categorize underlag propagation', ( expect(body.error.code).toBe('TX_CATEGORIZE_RACE') expect(propagateUnderlagMock).not.toHaveBeenCalled() }) + + it('returns a race conflict when the guarded update matches no row without creating an entry', async () => { + const { supabase } = casRaceSupabase() + mockServiceClient.mockReturnValue(supabase) + createTxJE.mockResolvedValueOnce(null) + + const res = await POST( + makeRequest({ is_business: true, category: 'expense_office' }), + routeParams(), + ) + + const body = await res.json() + expect(res.status).toBe(409) + expect(body.error.code).toBe('TX_CATEGORIZE_RACE') + expect(reverseEntryMock).not.toHaveBeenCalled() + }) + + it('atomically unignores an ignored transaction when categorizing it', async () => { + const { supabase, updates } = happyPathSupabase({ is_ignored: true }) + mockServiceClient.mockReturnValue(supabase) + + const res = await POST(makeRequest({ is_business: false }), routeParams()) + + expect(res.status).toBe(200) + expect(updates.transactions).toContainEqual( + expect.objectContaining({ + is_business: false, + category: 'private', + is_ignored: false, + journal_entry_id: 'je-fresh', + }), + ) + }) + + it('maps an ignored-row constraint to a typed conflict and stornos the posted orphan', async () => { + const { supabase } = makeFlexibleSupabase({ + company_members: { data: { company_id: COMPANY_ID, role: 'owner' }, error: null }, + transactions: [ + { + data: { + id: TX_ID, + company_id: COMPANY_ID, + date: '2026-05-12', + amount: -349.5, + currency: 'SEK', + merchant_name: 'ICA', + cash_account_id: null, + journal_entry_id: null, + is_ignored: true, + }, + error: null, + }, + { + data: null, + error: { + code: '23514', + message: + 'new row for relation "transactions" violates check constraint "transactions_is_ignored_no_journal_entry"', + }, + }, + ], + company_settings: { data: { entity_type: 'enskild_firma' }, error: null }, + fiscal_periods: { data: { id: 'period-1', is_closed: false, locked_at: null }, error: null }, + }) + mockServiceClient.mockReturnValue(supabase) + + const res = await POST(makeRequest({ is_business: false }), routeParams()) + const body = await res.json() + + expect(res.status).toBe(409) + expect(body.error.code).toBe('TX_CATEGORIZE_IGNORED_CONFLICT') + expect(body.error.message).not.toContain('check constraint') + expect(reverseEntryMock).toHaveBeenCalledWith( + expect.anything(), + COMPANY_ID, + 'user-1', + 'je-fresh', + ) + }) }) describe('POST /api/v1/.../transactions/{id}/categorize CAS race', () => { it('documents the stranded voucher with the real voucher_gap_explanations columns when the storno fails', async () => { const { supabase, inserts } = casRaceSupabase() mockServiceClient.mockReturnValue(supabase) - reverseEntryMock.mockRejectedValueOnce(new Error('period locked')) + reverseEntryMock.mockRejectedValueOnce( + withUnusedVoucherAllocation(new Error('account lookup failed'), { + fiscalPeriodId: 'period-1', + voucherSeries: 'B', + voucherNumber: 43, + }), + ) const res = await POST( makeRequest({ is_business: true, category: 'expense_office' }), @@ -268,9 +357,10 @@ describe('POST /api/v1/.../transactions/{id}/categorize CAS race', () => { user_id: 'user-1', fiscal_period_id: 'period-1', voucher_series: 'B', - gap_start: 42, - gap_end: 42, - explanation: 'CAS-race orphan; automatisk storno misslyckades. Manuell reconciliation krävs.', + gap_start: 43, + gap_end: 43, + explanation: + 'Kategoriseringsverifikation utan transaktionskoppling; automatisk storno misslyckades. Manuell avstämning krävs.', }) }) diff --git a/app/api/v1/companies/[companyId]/transactions/[id]/categorize/route.ts b/app/api/v1/companies/[companyId]/transactions/[id]/categorize/route.ts index 4093e4f8..549d3ddc 100644 --- a/app/api/v1/companies/[companyId]/transactions/[id]/categorize/route.ts +++ b/app/api/v1/companies/[companyId]/transactions/[id]/categorize/route.ts @@ -32,8 +32,7 @@ import { buildMappingResultFromCounterpartyTemplate, } from '@/lib/bookkeeping/counterparty-templates' import { createTransactionJournalEntry } from '@/lib/bookkeeping/transaction-entries' -import { recordVoucherGapExplanation } from '@/lib/bookkeeping/cancel-orphaned-entry' -import { reverseEntry } from '@/lib/bookkeeping/engine' +import { reverseOrphanedJournalEntry } from '@/lib/bookkeeping/cancel-orphaned-entry' import { saveUserMappingRule, applySettlementAccount } from '@/lib/bookkeeping/mapping-engine' import { resolveSettlementAccount } from '@/lib/bookkeeping/settlement-account' import { AccountsNotInChartError, isBookkeepingError } from '@/lib/bookkeeping/errors' @@ -155,7 +154,7 @@ export const POST = withApiV1<{ params: Promise<{ companyId: string; id: string : 'private' const { error: updateErr } = await ctx.supabase .from('transactions') - .update({ is_business, category: finalCat }) + .update({ is_business, category: finalCat, is_ignored: false }) .eq('id', txId) .eq('company_id', ctx.companyId!) if (updateErr) return v1ErrorResponse(updateErr, txLog, { requestId: ctx.requestId }) @@ -443,72 +442,44 @@ export const POST = withApiV1<{ params: Promise<{ companyId: string; id: string .update({ is_business, category: finalCategory, + is_ignored: false, journal_entry_id: journalEntryId, }) .eq('id', txId) .eq('company_id', ctx.companyId!) .is('journal_entry_id', null) - .select('id') + .select('*') - if (updateErr) return v1ErrorResponse(updateErr, txLog, { requestId: ctx.requestId }) + if (updateErr) { + if (journalEntryId) { + await reverseOrphanedJournalEntry( + ctx.supabase, + ctx.companyId!, + ctx.userId, + journalEntryId, + 'Kategoriseringsverifikation utan transaktionskoppling; automatisk storno misslyckades. Manuell avstämning krävs.', + ) + } + return v1ErrorResponse(updateErr, txLog, { requestId: ctx.requestId }) + } - if ((!updateResult || updateResult.length === 0) && journalEntryId) { - // Lost the race. The orphan JE was created with status='posted' by the - // engine, so the immutability trigger blocks a direct status flip to - // 'cancelled'. BFL 5 kap 5 § requires corrections via a reversing - // entry (storno): issue one. The pair (orphan + storno) keeps the - // verifikationsnummer series unbroken; no voucher_gap_explanations row - // is needed because there's no gap. - try { - await reverseEntry(ctx.supabase, ctx.companyId!, ctx.userId, journalEntryId) - } catch (revErr) { - // Storno failure on the orphan is rare but creates an unreconcilable - // ledger state (posted JE with no reversal). BFL 5 kap 5 § requires - // every correction be traceable. Document the gap explicitly so a - // human can reconcile manually rather than losing the trail to logs. - txLog.error('TX_CATEGORIZE_RACE: failed to storno orphaned JE', revErr as Error, { - orphanJournalEntryId: journalEntryId, - }) - try { - const { data: orphan } = await ctx.supabase - .from('journal_entries') - .select('fiscal_period_id, voucher_series, voucher_number') - .eq('id', journalEntryId) - .eq('company_id', ctx.companyId!) - .single() - if (orphan && orphan.voucher_series) { - // Skip the gap row when the engine didn't tag a series on the - // orphan. Filing under a fallback series (previously 'A') would - // index the gap explanation under the wrong key, hiding it from - // series-specific audit queries (BFL 5 kap 6 §). A missing series - // is logged above already; a human will reconcile via that trail. - // - // The insert itself lives in the shared helper: it owns the real - // voucher_gap_explanations column set (gap_start/gap_end/user_id) - // and logs a failed insert loudly instead of swallowing it. - await recordVoucherGapExplanation(ctx.supabase, { - companyId: ctx.companyId!, - userId: ctx.userId, - fiscalPeriodId: orphan.fiscal_period_id, - voucherSeries: orphan.voucher_series, - voucherNumber: orphan.voucher_number, - explanation: - 'CAS-race orphan; automatisk storno misslyckades. Manuell reconciliation krävs.', - }) - } - } catch (gapErr) { - txLog.error( - 'TX_CATEGORIZE_RACE: failed to look up the orphan for its gap explanation', - gapErr as Error, - { orphanJournalEntryId: journalEntryId }, - ) - } + if (!updateResult || updateResult.length === 0) { + if (journalEntryId) { + await reverseOrphanedJournalEntry( + ctx.supabase, + ctx.companyId!, + ctx.userId, + journalEntryId, + 'Kategoriseringsverifikation utan transaktionskoppling; automatisk storno misslyckades. Manuell avstämning krävs.', + ) } return v1ErrorResponseFromCode('TX_CATEGORIZE_RACE', txLog, { requestId: ctx.requestId, }) } + const updatedTransaction = updateResult[0] as Transaction + // Propagate the underlag onto the new verifikat: anchor the transaction's // pinned document and stamp matched inbox items so they leave the active // inbox. Same shared step the dashboard categorize, /book and bulk-book @@ -528,7 +499,7 @@ export const POST = withApiV1<{ params: Promise<{ companyId: string; id: string await eventBus.emit({ type: 'transaction.categorized', payload: { - transaction: transaction as Transaction, + transaction: updatedTransaction, account: mappingResult.debit_account, taxCode: mappingResult.vat_lines[0]?.account_number || '', userId: ctx.userId, diff --git a/app/api/v1/companies/[companyId]/transactions/batch-categorize/__tests__/route.test.ts b/app/api/v1/companies/[companyId]/transactions/batch-categorize/__tests__/route.test.ts index 4f66d322..bbfe7e79 100644 --- a/app/api/v1/companies/[companyId]/transactions/batch-categorize/__tests__/route.test.ts +++ b/app/api/v1/companies/[companyId]/transactions/batch-categorize/__tests__/route.test.ts @@ -60,6 +60,7 @@ vi.mock('@/lib/transactions/inbox-underlag', () => ({ })) import { validateApiKey, createServiceClientNoCookies } from '@/lib/auth/api-keys' +import { withUnusedVoucherAllocation } from '@/lib/bookkeeping/errors' import { POST } from '../route' const mockValidate = validateApiKey as ReturnType @@ -74,6 +75,7 @@ function makeFlexibleSupabase(byTable: Record // Insert payloads are recorded verbatim: the proxy would happily accept a // phantom column, so assertions have to inspect the object itself. const inserts: Record = {} + const updates: Record = {} const buildChain = (table: string): unknown => { const handler: ProxyHandler = { get(_target, prop) { @@ -86,13 +88,14 @@ function makeFlexibleSupabase(byTable: Record } return (...args: unknown[]) => { if (prop === 'insert') (inserts[table] ??= []).push(args[0]) + if (prop === 'update') (updates[table] ??= []).push(args[0]) return buildChain(table) } }, } return new Proxy({}, handler) } - return { supabase: { from: vi.fn((table: string) => buildChain(table)) }, inserts } + return { supabase: { from: vi.fn((table: string) => buildChain(table)) }, inserts, updates } } const COMPANY_ID = 'aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa' @@ -129,6 +132,193 @@ beforeEach(() => { }) describe('POST batch-categorize', () => { + it('atomically unignores ignored rows when categorizing them as private', async () => { + const { supabase, updates } = makeFlexibleSupabase({ + company_members: { data: { company_id: COMPANY_ID, role: 'owner' }, error: null }, + transactions: [ + { + data: { + id: TX_A, + company_id: COMPANY_ID, + date: '2026-05-12', + amount: -349.5, + currency: 'SEK', + merchant_name: 'ICA', + cash_account_id: null, + journal_entry_id: null, + is_ignored: true, + }, + error: null, + }, + { data: [{ id: TX_A }], error: null }, + ], + company_settings: { data: { entity_type: 'enskild_firma' }, error: null }, + fiscal_periods: { data: { id: 'period-1', is_closed: false, locked_at: null }, error: null }, + }) + mockServiceClient.mockReturnValue(supabase) + + const res = await POST( + makeRequest( + `https://x.test/api/v1/companies/${COMPANY_ID}/transactions/batch-categorize`, + { + items: [{ transaction_id: TX_A, categorization: { is_business: false } }], + }, + ), + batchParams(), + ) + + expect(res.status).toBe(200) + const body = await res.json() + expect(body.data.summary).toEqual({ total: 1, succeeded: 1, failed: 0 }) + expect(updates.transactions).toContainEqual( + expect.objectContaining({ + is_business: false, + category: 'private', + is_ignored: false, + journal_entry_id: 'je-fresh', + }), + ) + }) + + it('maps the ignored-row constraint to a typed per-item error', async () => { + const { supabase } = makeFlexibleSupabase({ + company_members: { data: { company_id: COMPANY_ID, role: 'owner' }, error: null }, + transactions: [ + { + data: { + id: TX_A, + company_id: COMPANY_ID, + date: '2026-05-12', + amount: -349.5, + currency: 'SEK', + merchant_name: 'ICA', + cash_account_id: null, + journal_entry_id: null, + is_ignored: true, + }, + error: null, + }, + { + data: null, + error: { + code: '23514', + message: + 'new row for relation "transactions" violates check constraint "transactions_is_ignored_no_journal_entry"', + }, + }, + ], + company_settings: { data: { entity_type: 'enskild_firma' }, error: null }, + fiscal_periods: { data: { id: 'period-1', is_closed: false, locked_at: null }, error: null }, + }) + mockServiceClient.mockReturnValue(supabase) + + const res = await POST( + makeRequest( + `https://x.test/api/v1/companies/${COMPANY_ID}/transactions/batch-categorize`, + { + items: [{ transaction_id: TX_A, categorization: { is_business: false } }], + }, + ), + batchParams(), + ) + + const body = await res.json() + expect(body.data.results[0].error).toMatchObject({ + code: 'TX_CATEGORIZE_IGNORED_CONFLICT', + message: + 'Transaktionen är fortfarande markerad som ignorerad och kan därför inte kopplas till en verifikation.', + }) + expect(body.data.summary).toEqual({ total: 1, succeeded: 0, failed: 1 }) + expect(reverseEntryMock).toHaveBeenCalledWith( + expect.anything(), + COMPANY_ID, + 'user-1', + 'je-fresh', + ) + }) + + it('returns a per-item race conflict when the guarded update matches no row without creating an entry', async () => { + const { supabase } = makeFlexibleSupabase({ + company_members: { data: { company_id: COMPANY_ID, role: 'owner' }, error: null }, + transactions: [ + { + data: { + id: TX_A, + company_id: COMPANY_ID, + date: '2026-05-12', + amount: -349.5, + currency: 'SEK', + merchant_name: 'ICA', + journal_entry_id: null, + }, + error: null, + }, + { data: [], error: null }, + ], + company_settings: { data: { entity_type: 'enskild_firma' }, error: null }, + fiscal_periods: { data: { id: 'period-1', is_closed: false, locked_at: null }, error: null }, + }) + mockServiceClient.mockReturnValue(supabase) + createTxJE.mockResolvedValueOnce(null) + + const res = await POST( + makeRequest( + `https://x.test/api/v1/companies/${COMPANY_ID}/transactions/batch-categorize`, + { + items: [{ transaction_id: TX_A, categorization: { is_business: true, category: 'expense_office' } }], + }, + ), + batchParams(), + ) + + const body = await res.json() + expect(body.data.results[0].error.code).toBe('TX_CATEGORIZE_RACE') + expect(body.data.summary).toEqual({ total: 1, succeeded: 0, failed: 1 }) + expect(reverseEntryMock).not.toHaveBeenCalled() + }) + + it('keeps unrelated transaction update errors mapped to INTERNAL_ERROR', async () => { + const { supabase } = makeFlexibleSupabase({ + company_members: { data: { company_id: COMPANY_ID, role: 'owner' }, error: null }, + transactions: [ + { + data: { + id: TX_A, + company_id: COMPANY_ID, + date: '2026-05-12', + amount: -349.5, + currency: 'SEK', + merchant_name: 'ICA', + cash_account_id: null, + journal_entry_id: null, + }, + error: null, + }, + { + data: null, + error: { code: 'P0001', message: 'Invoice not found' }, + }, + ], + company_settings: { data: { entity_type: 'enskild_firma' }, error: null }, + fiscal_periods: { data: { id: 'period-1', is_closed: false, locked_at: null }, error: null }, + }) + mockServiceClient.mockReturnValue(supabase) + + const res = await POST( + makeRequest( + `https://x.test/api/v1/companies/${COMPANY_ID}/transactions/batch-categorize`, + { + items: [{ transaction_id: TX_A, categorization: { is_business: false } }], + }, + ), + batchParams(), + ) + + const body = await res.json() + expect(body.data.results[0].error.code).toBe('INTERNAL_ERROR') + expect(reverseEntryMock).toHaveBeenCalledTimes(1) + }) + it('uses the linked cash account in validation and the posted mapping', async () => { mockServiceClient.mockReturnValue( makeFlexibleSupabase({ @@ -430,9 +620,15 @@ describe('POST batch-categorize', () => { voucher_gap_explanations: { data: null, error: null }, }) mockServiceClient.mockReturnValue(supabase) - // Storno fails: the orphan keeps its number, so the break in the - // verifikationsnummerserie must be documented (BFNAR 2013:2). - reverseEntryMock.mockRejectedValueOnce(new Error('period locked')) + // The reversal sequence allocation fails before a reversal row is stored, + // so the engine exposes the exact unused number for documentation. + reverseEntryMock.mockRejectedValueOnce( + withUnusedVoucherAllocation(new Error('account lookup failed'), { + fiscalPeriodId: 'period-1', + voucherSeries: 'B', + voucherNumber: 43, + }), + ) const res = await POST( makeRequest( @@ -458,9 +654,10 @@ describe('POST batch-categorize', () => { user_id: 'user-1', fiscal_period_id: 'period-1', voucher_series: 'B', - gap_start: 42, - gap_end: 42, - explanation: 'CAS-race orphan; automatisk storno misslyckades. Manuell reconciliation krävs.', + gap_start: 43, + gap_end: 43, + explanation: + 'Kategoriseringsverifikation utan transaktionskoppling; automatisk storno misslyckades. Manuell avstämning krävs.', }) }) }) diff --git a/app/api/v1/companies/[companyId]/transactions/batch-categorize/route.ts b/app/api/v1/companies/[companyId]/transactions/batch-categorize/route.ts index 2e34f44f..9413d1f2 100644 --- a/app/api/v1/companies/[companyId]/transactions/batch-categorize/route.ts +++ b/app/api/v1/companies/[companyId]/transactions/batch-categorize/route.ts @@ -27,12 +27,13 @@ import { validateTemplateForEntity, } from '@/lib/bookkeeping/booking-templates' import { createTransactionJournalEntry } from '@/lib/bookkeeping/transaction-entries' -import { recordVoucherGapExplanation } from '@/lib/bookkeeping/cancel-orphaned-entry' -import { reverseEntry } from '@/lib/bookkeeping/engine' +import { reverseOrphanedJournalEntry } from '@/lib/bookkeeping/cancel-orphaned-entry' import { AccountsNotInChartError, isBookkeepingError } from '@/lib/bookkeeping/errors' import { collectMappingResultAccounts, findUnresolvableAccounts } from '@/lib/bookkeeping/account-validation' import { propagateUnderlagForBookedTransaction } from '@/lib/transactions/inbox-underlag' import { getErrorMessage } from '@/lib/errors/get-error-message' +import { getStructuredError } from '@/lib/errors/get-structured-error' +import { getErrorEntry } from '@/lib/errors/structured-errors' import { eventBus } from '@/lib/events' import type { Logger } from '@/lib/logger' import type { EntityType, Transaction, TransactionCategory } from '@/types' @@ -117,6 +118,21 @@ interface Item { error?: { code: string; message: string; details?: unknown } } +function categorizeUpdateError(error: unknown): NonNullable { + const structured = getStructuredError(error) + if (structured.code === 'TX_CATEGORIZE_IGNORED_CONFLICT') { + const mapped = getErrorEntry(structured.code) + return { + code: structured.code, + message: mapped?.message_sv ?? 'Transaktionens tillstånd ändrades samtidigt.', + } + } + return { + code: 'INTERNAL_ERROR', + message: getErrorMessage(error), + } +} + async function categorizeOne( supabase: SupabaseClient, companyId: string, @@ -277,7 +293,7 @@ async function categorizeOne( if (transaction.journal_entry_id) { const { error: updateErr } = await supabase .from('transactions') - .update({ is_business, category: finalCategory }) + .update({ is_business, category: finalCategory, is_ignored: false }) .eq('id', transactionId) .eq('company_id', companyId) if (updateErr) { @@ -285,7 +301,7 @@ async function categorizeOne( ok: false, request_index: index, transaction_id: transactionId, - error: { code: 'INTERNAL_ERROR', message: 'Failed to update flags.' }, + error: categorizeUpdateError(updateErr), } } return { @@ -367,62 +383,39 @@ async function categorizeOne( .update({ is_business, category: finalCategory, + is_ignored: false, journal_entry_id: journalEntryId, }) .eq('id', transactionId) .eq('company_id', companyId) .is('journal_entry_id', null) - .select('id') + .select('*') if (updateErr) { + if (journalEntryId) { + await reverseOrphanedJournalEntry( + supabase, + companyId, + userId, + journalEntryId, + 'Kategoriseringsverifikation utan transaktionskoppling; automatisk storno misslyckades. Manuell avstämning krävs.', + ) + } return { ok: false, request_index: index, transaction_id: transactionId, - error: { code: 'INTERNAL_ERROR', message: getErrorMessage(updateErr) }, + error: categorizeUpdateError(updateErr), } } - if ((!updated || updated.length === 0) && journalEntryId) { - // CAS race: storno the orphan (BFL 5 kap 5 §). Direct statusflip - // would be blocked by enforce_journal_entry_immutability since the - // engine writes the JE as posted. Same fix as the single :categorize - // route. Storno keeps the verifikationsnummer series unbroken. - try { - await reverseEntry(supabase, companyId, userId, journalEntryId) - } catch (revErr) { - log.error('batch-categorize TX_CATEGORIZE_RACE: failed to storno orphaned JE', revErr as Error, { - request_index: index, - orphanJournalEntryId: journalEntryId, - }) - // Document the gap so the orphan is traceable per BFL 5 kap 5 §. - try { - const { data: orphan } = await supabase - .from('journal_entries') - .select('fiscal_period_id, voucher_series, voucher_number') - .eq('id', journalEntryId) - .eq('company_id', companyId) - .single() - if (orphan && orphan.voucher_series) { - // Same rationale as the single :categorize route: skip the gap row - // when no series exists rather than filing under a fallback series - // that an audit query won't find. The insert itself lives in the - // shared helper, which owns the real voucher_gap_explanations - // column set (gap_start/gap_end/user_id) and logs failures loudly. - await recordVoucherGapExplanation(supabase, { - companyId, - userId, - fiscalPeriodId: orphan.fiscal_period_id, - voucherSeries: orphan.voucher_series, - voucherNumber: orphan.voucher_number, - explanation: - 'CAS-race orphan; automatisk storno misslyckades. Manuell reconciliation krävs.', - }) - } - } catch (gapErr) { - log.error('batch-categorize: failed to look up the orphan for its gap explanation', gapErr as Error, { - request_index: index, - orphanJournalEntryId: journalEntryId, - }) - } + if (!updated || updated.length === 0) { + if (journalEntryId) { + await reverseOrphanedJournalEntry( + supabase, + companyId, + userId, + journalEntryId, + 'Kategoriseringsverifikation utan transaktionskoppling; automatisk storno misslyckades. Manuell avstämning krävs.', + ) } return { ok: false, @@ -432,6 +425,8 @@ async function categorizeOne( } } + const updatedTransaction = updated[0] as Transaction + // Propagate the underlag onto the new verifikat: anchor the transaction's // pinned document and stamp matched inbox items. Same shared step as the // single :categorize route and every dashboard booking path; best-effort @@ -445,7 +440,7 @@ async function categorizeOne( await eventBus.emit({ type: 'transaction.categorized', payload: { - transaction: transaction as Transaction, + transaction: updatedTransaction, account: mappingResult.debit_account, taxCode: mappingResult.vat_lines[0]?.account_number || '', userId, diff --git a/lib/bookkeeping/__tests__/cancel-orphaned-entry.test.ts b/lib/bookkeeping/__tests__/cancel-orphaned-entry.test.ts index 24b3fa87..3912ebe3 100644 --- a/lib/bookkeeping/__tests__/cancel-orphaned-entry.test.ts +++ b/lib/bookkeeping/__tests__/cancel-orphaned-entry.test.ts @@ -1,8 +1,19 @@ -import { describe, it, expect, vi } from 'vitest' +import { beforeEach, describe, it, expect, vi } from 'vitest' + +const { reverseEntryMock } = vi.hoisted(() => ({ + reverseEntryMock: vi.fn(), +})) + +vi.mock('@/lib/bookkeeping/engine', () => ({ + reverseEntry: reverseEntryMock, +})) + import { cancelOrphanedPaymentEntry, recordVoucherGapExplanation, + reverseOrphanedJournalEntry, } from '../cancel-orphaned-entry' +import { withUnusedVoucherAllocation } from '../errors' // The real voucher_gap_explanations column set (supabase/migrations/ // 20260402100100_voucher_gap_explanations.sql). company_id, user_id, @@ -55,6 +66,11 @@ function createMockSupabase(opts: { return { supabase, updates, inserts } } +beforeEach(() => { + reverseEntryMock.mockReset() + reverseEntryMock.mockResolvedValue(undefined) +}) + describe('recordVoucherGapExplanation', () => { it('writes exactly the real NOT NULL columns, with the single voucher as a closed gap range', async () => { const { supabase, inserts } = createMockSupabase({}) @@ -143,6 +159,77 @@ describe('recordVoucherGapExplanation', () => { }) }) +describe('reverseOrphanedJournalEntry', () => { + it('routes posted-orphan compensation through engine storno', async () => { + const { supabase, inserts, updates } = createMockSupabase({}) + + await reverseOrphanedJournalEntry( + supabase as never, + 'company-1', + 'user-1', + 'je-1', + 'Manuell avstämning krävs.', + ) + + expect(reverseEntryMock).toHaveBeenCalledWith( + supabase, + 'company-1', + 'user-1', + 'je-1', + ) + expect(updates).toEqual([]) + expect(inserts['voucher_gap_explanations']).toBeUndefined() + }) + + it('documents only the exact unused reversal voucher exposed by the engine', async () => { + const { supabase, inserts } = createMockSupabase({}) + reverseEntryMock.mockRejectedValueOnce( + withUnusedVoucherAllocation(new Error('account lookup failed'), { + fiscalPeriodId: 'fp-1', + voucherSeries: 'B', + voucherNumber: 67, + }), + ) + + await reverseOrphanedJournalEntry( + supabase as never, + 'company-1', + 'user-1', + 'je-1', + 'Manuell avstämning krävs.', + ) + + expect(inserts['voucher_gap_explanations']).toEqual([ + { + company_id: 'company-1', + user_id: 'user-1', + fiscal_period_id: 'fp-1', + voucher_series: 'B', + gap_start: 67, + gap_end: 67, + explanation: 'Manuell avstämning krävs.', + }, + ]) + }) + + it('does not mislabel the original posted voucher when storno failure has no unused allocation', async () => { + const { supabase, inserts } = createMockSupabase({ + orphan: { fiscal_period_id: 'fp-1', voucher_series: 'B', voucher_number: 66 }, + }) + reverseEntryMock.mockRejectedValueOnce(new Error('period locked')) + + await reverseOrphanedJournalEntry( + supabase as never, + 'company-1', + 'user-1', + 'je-1', + 'Manuell avstämning krävs.', + ) + + expect(inserts['voucher_gap_explanations']).toBeUndefined() + }) +}) + describe('cancelOrphanedPaymentEntry', () => { it('cancels the voucher and records a gap explanation', async () => { const { supabase, updates, inserts } = createMockSupabase({ diff --git a/lib/bookkeeping/__tests__/engine.test.ts b/lib/bookkeeping/__tests__/engine.test.ts index 2215976e..f2b39466 100644 --- a/lib/bookkeeping/__tests__/engine.test.ts +++ b/lib/bookkeeping/__tests__/engine.test.ts @@ -1,6 +1,11 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' import { validateBalance, getSwedishLocalDate, createDraftEntry, reverseEntry } from '../engine' -import { BookkeepingDatabaseError, AccountsNotInChartError, CannotReverseStornoError } from '../errors' +import { + AccountsNotInChartError, + BookkeepingDatabaseError, + CannotReverseStornoError, + getUnusedVoucherAllocation, +} from '../errors' import type { CreateJournalEntryLineInput, JournalEntryStatus } from '@/types' // Mock Supabase client for createDraftEntry/reverseEntry tests @@ -612,6 +617,141 @@ describe('reverseEntry: entry_date defaults to original entry date', () => { }) }) +describe('reverseEntry: unused voucher allocation', () => { + it('exposes the exact allocated number when account resolution fails before the reversal insert', async () => { + const original = { + id: 'entry-1', + company_id: 'company-1', + status: 'posted', + fiscal_period_id: 'period-1', + voucher_series: 'B', + voucher_number: 41, + entry_date: '2024-11-15', + description: 'Hyra november', + source_type: 'manual', + source_id: null, + lines: [ + { account_number: '5010', debit_amount: 10000, credit_amount: 0 }, + { account_number: '1930', debit_amount: 0, credit_amount: 10000 }, + ], + } + const supabase = { + rpc: vi.fn().mockResolvedValue({ data: 42, error: null }), + from: vi.fn().mockImplementation((table: string) => { + if (table === 'journal_entries') { + const chain = createMockChain({ singleData: original }) + return chain + } + if (table === 'chart_of_accounts') { + const chain: Record = {} + for (const method of ['select', 'eq', 'in']) { + chain[method] = vi.fn().mockReturnValue(chain) + } + chain.then = (resolve: (value: unknown) => void) => + resolve({ data: null, error: { message: 'account lookup failed' } }) + return chain + } + return createMockChain() + }), + } + + let caught: unknown + try { + await reverseEntry(supabase as never, 'company-1', 'user-1', 'entry-1') + } catch (error) { + caught = error + } + + expect(caught).toBeInstanceOf(BookkeepingDatabaseError) + expect(getUnusedVoucherAllocation(caught)).toEqual({ + fiscalPeriodId: 'period-1', + voucherSeries: 'B', + voucherNumber: 42, + }) + }) + + it('does not label a preserved cancelled reversal header as an unused voucher', async () => { + const original = { + id: 'entry-1', + company_id: 'company-1', + status: 'posted', + fiscal_period_id: 'period-1', + voucher_series: 'B', + voucher_number: 41, + entry_date: '2024-11-15', + description: 'Hyra november', + source_type: 'manual', + source_id: null, + lines: [ + { account_number: '5010', debit_amount: 10000, credit_amount: 0 }, + { account_number: '1930', debit_amount: 0, credit_amount: 10000 }, + ], + } + const reversal = { id: 'reversal-1', reverses_id: 'entry-1' } + const cancelUpdate = vi.fn() + const deleteLines = vi.fn() + let journalEntryCall = 0 + let journalLineCall = 0 + + const supabase = { + rpc: vi.fn().mockResolvedValue({ data: 42, error: null }), + from: vi.fn().mockImplementation((table: string) => { + if (table === 'journal_entries') { + journalEntryCall += 1 + if (journalEntryCall === 1) return createMockChain({ singleData: original }) + if (journalEntryCall === 2) return createMockChain({ singleData: reversal }) + return { + update: cancelUpdate.mockReturnValue({ + eq: vi.fn().mockResolvedValue({ data: null, error: null }), + }), + } + } + if (table === 'chart_of_accounts') { + const chain: Record = {} + for (const method of ['select', 'eq', 'in']) { + chain[method] = vi.fn().mockReturnValue(chain) + } + chain.then = (resolve: (value: unknown) => void) => + resolve({ + data: [ + { id: 'acc-5010', account_number: '5010' }, + { id: 'acc-1930', account_number: '1930' }, + ], + error: null, + }) + return chain + } + if (table === 'journal_entry_lines') { + journalLineCall += 1 + if (journalLineCall === 1) { + return { + insert: vi.fn().mockResolvedValue({ error: { message: 'line insert failed' } }), + } + } + return { + delete: deleteLines.mockReturnValue({ + eq: vi.fn().mockResolvedValue({ data: null, error: null }), + }), + } + } + return createMockChain() + }), + } + + let caught: unknown + try { + await reverseEntry(supabase as never, 'company-1', 'user-1', 'entry-1') + } catch (error) { + caught = error + } + + expect(caught).toBeInstanceOf(BookkeepingDatabaseError) + expect(getUnusedVoucherAllocation(caught)).toBeNull() + expect(cancelUpdate).toHaveBeenCalledWith({ status: 'cancelled' }) + expect(deleteLines).toHaveBeenCalled() + }) +}) + describe('reverseEntry: storno guard', () => { // BFL 5 kap 5§: a storno-of-a-storno makes the original verifikat's // cancellation chain ambiguous, so stornos are never reversible. A diff --git a/lib/bookkeeping/cancel-orphaned-entry.ts b/lib/bookkeeping/cancel-orphaned-entry.ts index 8e54956e..b8c15b7f 100644 --- a/lib/bookkeeping/cancel-orphaned-entry.ts +++ b/lib/bookkeeping/cancel-orphaned-entry.ts @@ -1,5 +1,7 @@ import type { SupabaseClient } from '@supabase/supabase-js' import { createLogger } from '@/lib/logger' +import { reverseEntry } from '@/lib/bookkeeping/engine' +import { getUnusedVoucherAllocation } from '@/lib/bookkeeping/errors' const log = createLogger('cancel-orphaned-entry') @@ -71,6 +73,49 @@ export async function recordVoucherGapExplanation( } } +/** + * Storno a posted journal entry that could not be linked to its transaction. + * + * The bookkeeping engine posts entries before the transaction CAS runs. When + * that CAS definitively fails, the entry is immutable and must be reversed, + * never edited or cancelled in place. Compensation is best-effort so the + * caller can preserve the original conflict response. + */ +export async function reverseOrphanedJournalEntry( + supabase: SupabaseClient, + companyId: string, + userId: string, + journalEntryId: string, + gapExplanation: string, +): Promise { + let unusedVoucher: ReturnType = null + try { + await reverseEntry(supabase, companyId, userId, journalEntryId) + return + } catch (reverseError) { + unusedVoucher = getUnusedVoucherAllocation(reverseError) + log.error('failed to storno orphaned journal entry', reverseError as Error, { + companyId, + journalEntryId, + unusedVoucher, + }) + } + + // The original posted voucher is live accounting evidence, never a gap. + // Only the engine can identify an exact reversal number that its durable + // sequence allocated before a reversal row existed. + if (!unusedVoucher) return + + await recordVoucherGapExplanation(supabase, { + companyId, + userId, + fiscalPeriodId: unusedVoucher.fiscalPeriodId, + voucherSeries: unusedVoucher.voucherSeries, + voucherNumber: unusedVoucher.voucherNumber, + explanation: gapExplanation, + }) +} + /** * Compensation for the payment-flow CAS guard: a payment voucher was posted, * but the invoice row was settled by a concurrent request between our read diff --git a/lib/bookkeeping/engine.ts b/lib/bookkeeping/engine.ts index 725f0b14..d04a088c 100644 --- a/lib/bookkeeping/engine.ts +++ b/lib/bookkeeping/engine.ts @@ -11,6 +11,7 @@ import { EntryAlreadyReversedError, EntryDateOutsideFiscalPeriodError, FiscalPeriodNotFoundError, + withUnusedVoucherAllocation, JournalEntryNotBalancedError, JournalEntryNotFoundError, } from '@/lib/bookkeeping/errors' @@ -1122,18 +1123,33 @@ export async function reverseEntry( original.fiscal_period_id, original.voucher_series || 'A' ) + const unusedVoucherAllocation = { + fiscalPeriodId: original.fiscal_period_id, + voucherSeries: original.voucher_series || 'A', + voucherNumber, + } // Resolve account IDs: include inactive rows. The accounts on the // original committed entry were active at commit time; if the user has // since toggled one off, the storno must still be allowed to go through // (BFL 5 kap 5§). Only a truly missing chart row (rare: would require // the row to have been deleted) still throws AccountsNotInChartError. - const accountIdMap = await resolveAccountIds(supabase, companyId, reversedLines, { includeInactive: true }) + let accountIdMap: Map + try { + accountIdMap = await resolveAccountIds(supabase, companyId, reversedLines, { includeInactive: true }) + } catch (resolveError) { + // The sequence RPC committed, but no reversal row exists yet. Carry the + // exact unused number so the caller can document this real gap. + throw withUnusedVoucherAllocation(resolveError, unusedVoucherAllocation) + } const reversalAccountNumbers = [...new Set(reversedLines.map(l => l.account_number))] const missingReversalAccounts = reversalAccountNumbers.filter(num => !accountIdMap.has(num)) if (missingReversalAccounts.length > 0) { - throw new AccountsNotInChartError(missingReversalAccounts) + throw withUnusedVoucherAllocation( + new AccountsNotInChartError(missingReversalAccounts), + unusedVoucherAllocation, + ) } // Create reversal entry with reverses_id link @@ -1155,8 +1171,18 @@ export async function reverseEntry( .select() .single() - if (reversalError || !reversalEntry) { - throw new BookkeepingDatabaseError('create_reversal_entry', reversalError?.message) + if (reversalError) { + // PostgreSQL rejected the insert, so the allocated number is confirmed + // unused and can be explained without guessing from the original entry. + throw withUnusedVoucherAllocation( + new BookkeepingDatabaseError('create_reversal_entry', reversalError.message), + unusedVoucherAllocation, + ) + } + if (!reversalEntry) { + // No database error means the outcome is ambiguous. Do not label the + // number unused unless the engine has a confirmed failure state. + throw new BookkeepingDatabaseError('create_reversal_entry', undefined) } // Insert reversal lines with dimensions @@ -1167,6 +1193,9 @@ export async function reverseEntry( .insert(lineInserts) if (linesError) { + // Keep the reversal header as cancelled bookkeeping evidence. The gap + // detector counts every non-draft header, including cancelled rows, so + // this allocation is still used and must not be labelled as a gap. await supabase.from('journal_entries').update({ status: 'cancelled' }).eq('id', reversalEntry.id) await supabase.from('journal_entry_lines').delete().eq('journal_entry_id', reversalEntry.id) throw new BookkeepingDatabaseError('create_reversal_lines', linesError.message) @@ -1179,6 +1208,8 @@ export async function reverseEntry( .eq('id', reversalEntry.id) if (postError) { + // As above, cleanup preserves the allocated voucher on the cancelled + // header. A failed or ambiguous cleanup also cannot prove it unused. await supabase.from('journal_entries').update({ status: 'cancelled' }).eq('id', reversalEntry.id) await supabase.from('journal_entry_lines').delete().eq('journal_entry_id', reversalEntry.id) throw new BookkeepingDatabaseError('post_reversal_entry', postError.message) @@ -1198,6 +1229,7 @@ export async function reverseEntry( if (casError || !updatedOriginal || updatedOriginal.length === 0) { // Another concurrent reversal already changed the status: mark the orphaned // reversal as cancelled so it's excluded from reports but remains traceable. + // Its header still occupies the voucher number, so no gap metadata applies. await supabase.from('journal_entries').update({ status: 'cancelled' }).eq('id', reversalEntry.id) await supabase.from('journal_entry_lines').delete().eq('journal_entry_id', reversalEntry.id) throw new EntryAlreadyReversedError() diff --git a/lib/bookkeeping/errors.ts b/lib/bookkeeping/errors.ts index 28026b47..52a6ac95 100644 --- a/lib/bookkeeping/errors.ts +++ b/lib/bookkeeping/errors.ts @@ -341,6 +341,39 @@ export class BookkeepingDatabaseError extends Error { } } +export interface UnusedVoucherAllocation { + fiscalPeriodId: string + voucherSeries: string + voucherNumber: number +} + +const UNUSED_VOUCHER_ALLOCATION = Symbol('unused-voucher-allocation') + +/** + * Preserve the exact durable sequence allocation when an engine operation + * fails before any journal-entry row uses the number. The original error type + * is retained so existing API mappings remain unchanged. + */ +export function withUnusedVoucherAllocation( + error: T, + allocation: UnusedVoucherAllocation, +): T { + if (error instanceof Error) { + Object.defineProperty(error, UNUSED_VOUCHER_ALLOCATION, { + value: allocation, + enumerable: false, + }) + } + return error +} + +export function getUnusedVoucherAllocation(error: unknown): UnusedVoucherAllocation | null { + if (!(error instanceof Error)) return null + return ( + error as Error & { [UNUSED_VOUCHER_ALLOCATION]?: UnusedVoucherAllocation } + )[UNUSED_VOUCHER_ALLOCATION] ?? null +} + // ============================================================================ // Type guard // ============================================================================ diff --git a/lib/errors/__tests__/structured-errors.test.ts b/lib/errors/__tests__/structured-errors.test.ts index d40ae54e..ba6a7843 100644 --- a/lib/errors/__tests__/structured-errors.test.ts +++ b/lib/errors/__tests__/structured-errors.test.ts @@ -102,6 +102,31 @@ describe('errorResponse', () => { expect(body.error.details).toMatchObject({ pgCode: '23505' }) }) + it('maps the ignored-transaction journal constraint to a typed conflict', async () => { + const pgErr = Object.assign( + new Error( + 'new row for relation "transactions" violates check constraint "transactions_is_ignored_no_journal_entry"', + ), + { code: '23514' }, + ) + const res = errorResponse(pgErr, noopLogger, { requestId: 'req_ignored_tx' }) + + expect(res.status).toBe(409) + const body = await readEnvelope(res) + expect(body.error.code).toBe('TX_CATEGORIZE_IGNORED_CONFLICT') + expect(body.error.message).not.toContain('check constraint') + expect(body.error.details).toMatchObject({ pgCode: '23514' }) + }) + + it('does not apply unrelated message heuristics to Postgres errors', async () => { + const pgErr = Object.assign(new Error('Invoice not found'), { code: 'P0001' }) + const res = errorResponse(pgErr, noopLogger, { requestId: 'req_pg_unrelated' }) + + expect(res.status).toBe(500) + const body = await readEnvelope(res) + expect(body.error.code).toBe('INTERNAL_ERROR') + }) + it('maps Postgres no-data-found to NOT_FOUND with pgCode', async () => { const pgErr = Object.assign(new Error('invoice not found'), { code: 'P0002' }) const res = errorResponse(pgErr, noopLogger, { requestId: 'req_pg_not_found' }) diff --git a/lib/errors/get-structured-error.ts b/lib/errors/get-structured-error.ts index 3b5b3b49..479166d7 100644 --- a/lib/errors/get-structured-error.ts +++ b/lib/errors/get-structured-error.ts @@ -189,7 +189,9 @@ export function getStructuredError( const message_sv = getErrorMessage(error) const transient = isTransientFailure(error, message_en) - let code = extractCode(error) ?? inferCode(message_en) ?? 'UNKNOWN_ERROR' + let code = isIgnoredTransactionJournalConstraint(error) + ? 'TX_CATEGORIZE_IGNORED_CONFLICT' + : extractCode(error) ?? inferCode(message_en) ?? 'UNKNOWN_ERROR' // Nothing more specific matched but the failure is transient: surface the // stable TRANSIENT_ERROR code so agents can dispatch on it. if (code === 'UNKNOWN_ERROR' && transient) code = 'TRANSIENT_ERROR' @@ -313,6 +315,13 @@ function isPostgresError(err: unknown): err is { code: string; message: string } ) } +export function isIgnoredTransactionJournalConstraint(error: unknown): boolean { + return ( + isPostgresError(error) && + /transactions_is_ignored_no_journal_entry/i.test(error.message) + ) +} + /** * Build the canonical REST error envelope for any thrown value. * @@ -358,7 +367,9 @@ export function errorResponse( // 3. Postgres errors if (isPostgresError(err)) { - const mapped = postgresCodeToStructured(err.code) + const mapped = isIgnoredTransactionJournalConstraint(err) + ? 'TX_CATEGORIZE_IGNORED_CONFLICT' + : postgresCodeToStructured(err.code) if (mapped) { const entry = entryFor(mapped) logAtLevel(log, entry.httpStatus, 'database error', err as unknown as Error, { diff --git a/lib/errors/structured-errors.ts b/lib/errors/structured-errors.ts index 174ff90f..e9964b97 100644 --- a/lib/errors/structured-errors.ts +++ b/lib/errors/structured-errors.ts @@ -447,6 +447,16 @@ const TRANSACTIONS: Record = { message_sv: 'Transaktionen kategoriserades av en annan förfrågan. Ladda om och försök igen.', message_en: 'Transaction was already categorized by another request.', }, + TX_CATEGORIZE_IGNORED_CONFLICT: { + httpStatus: 409, + message_sv: + 'Transaktionen är fortfarande markerad som ignorerad och kan därför inte kopplas till en verifikation.', + message_en: + 'The transaction is still marked as ignored and cannot be linked to a journal entry.', + remediation: { + description: 'Reload and retry categorization. Report the conflict if it persists.', + }, + }, TX_CATEGORIZE_SUGGEST_SI_MATCH: { httpStatus: 409, message_sv: diff --git a/lib/pending-operations/__tests__/commit-duplicate-guard.test.ts b/lib/pending-operations/__tests__/commit-duplicate-guard.test.ts index c01ed8f8..d6f7cef3 100644 --- a/lib/pending-operations/__tests__/commit-duplicate-guard.test.ts +++ b/lib/pending-operations/__tests__/commit-duplicate-guard.test.ts @@ -120,7 +120,19 @@ describe('commit duplicate guard: categorize_transaction (reverse / book the ban { data: { id: 'op-1' } }, { data: { id: 'tx-1', date: '2026-03-26', amount: 98565, cash_account_id: null, journal_entry_id: null } }, { data: { entity_type: 'aktiebolag', fiscal_year_start_month: 1 } }, - { data: [] }, + { data: [] }, // no fiscal period yet + { data: null }, // fiscal-period upsert + { data: null }, // journal-entry period lookup: partial categorization path + { + data: [{ + id: 'tx-1', + date: '2026-03-26', + amount: 98565, + cash_account_id: null, + journal_entry_id: null, + is_ignored: false, + }], + }, // guarded transaction update matched ]) const op = makePendingOp({ @@ -156,7 +168,19 @@ describe('commit duplicate guard: categorize_transaction (reverse / book the ban { data: { id: 'op-1' } }, { data: { id: 'tx-1', date: '2026-03-26', amount: 98565, cash_account_id: null, journal_entry_id: null } }, { data: { entity_type: 'aktiebolag', fiscal_year_start_month: 1 } }, - { data: [] }, + { data: [] }, // no fiscal period yet + { data: null }, // fiscal-period upsert + { data: null }, // journal-entry period lookup: partial categorization path + { + data: [{ + id: 'tx-1', + date: '2026-03-26', + amount: 98565, + cash_account_id: null, + journal_entry_id: null, + is_ignored: false, + }], + }, // guarded transaction update matched ]) const op = makePendingOp({ diff --git a/lib/transactions/__tests__/categorize-core.bulk.test.ts b/lib/transactions/__tests__/categorize-core.bulk.test.ts index 97b0ea74..f2b0ca3f 100644 --- a/lib/transactions/__tests__/categorize-core.bulk.test.ts +++ b/lib/transactions/__tests__/categorize-core.bulk.test.ts @@ -234,7 +234,7 @@ describe('bulkBookMatchedInboxItems: booking', () => { // 4. ensureFiscalPeriod → existing period { data: [{ id: 'fp-1' }] }, // 5. transactions update (mark booked) - { error: null }, + { data: [{ id: 'tx-1' }], error: null }, // 6. propagation select (no matched inbox rows to stamp in this mock) { data: [] }, ]) @@ -275,7 +275,7 @@ describe('bulkBookMatchedInboxItems: booking', () => { { data: { entity_type: 'aktiebolag', fiscal_year_start_month: 1 } }, { data: { ledger_account: '1931' } }, { data: [{ id: 'fp-1' }] }, - { error: null }, + { data: [{ id: 'tx-1' }], error: null }, { data: [] }, ]) @@ -315,7 +315,7 @@ describe('bulkBookMatchedInboxItems: booking', () => { { data: { id: 'tx-1', date: '2026-06-01', amount: -700, currency: 'SEK', cash_account_id: null, journal_entry_id: null } }, { data: { entity_type: 'aktiebolag', fiscal_year_start_month: 1 } }, { data: [{ id: 'fp-1' }] }, - { error: null }, + { data: [{ id: 'tx-1' }], error: null }, { data: [] }, ]) @@ -346,7 +346,7 @@ describe('bulkBookMatchedInboxItems: booking', () => { { data: { id: 'tx-2', date: '2026-06-02', amount: -25, currency: 'SEK', cash_account_id: null, journal_entry_id: null } }, { data: { entity_type: 'aktiebolag', fiscal_year_start_month: 1 } }, { data: [{ id: 'fp-1' }] }, - { error: null }, + { data: [{ id: 'tx-2' }], error: null }, { data: [] }, ]) @@ -382,7 +382,7 @@ describe('bulkBookMatchedInboxItems: WhatsApp channel-context notes threading', { data: { id: 'tx-1', date: '2026-06-01', amount: -700, currency: 'SEK', cash_account_id: null, journal_entry_id: null } }, { data: { entity_type: 'aktiebolag', fiscal_year_start_month: 1 } }, { data: [{ id: 'fp-1' }] }, - { error: null }, + { data: [{ id: 'tx-1' }], error: null }, { data: [] }, ] @@ -537,7 +537,7 @@ describe('bulkBookMatchedInboxItems: intra-batch duplicate handling', () => { { data: { id: txId, date: '2026-06-01', amount, currency: 'SEK', cash_account_id: null, journal_entry_id: null } }, { data: { entity_type: 'aktiebolag', fiscal_year_start_month: 1 } }, { data: [{ id: 'fp-1' }] }, - { error: null }, + { data: [{ id: txId }], error: null }, { data: { document_id: null } }, // propagation: tx pin lookup { data: [] }, // propagation: matched inbox items ] diff --git a/lib/transactions/__tests__/categorize-core.override.test.ts b/lib/transactions/__tests__/categorize-core.override.test.ts index 6064d447..bda14285 100644 --- a/lib/transactions/__tests__/categorize-core.override.test.ts +++ b/lib/transactions/__tests__/categorize-core.override.test.ts @@ -12,9 +12,13 @@ import { createQueuedMockSupabase } from '@/tests/helpers' import { eventBus } from '@/lib/events' const mockCreateJE = vi.fn() +const mockReverseOrphanedJE = vi.fn() vi.mock('@/lib/bookkeeping/transaction-entries', () => ({ createTransactionJournalEntry: (...args: unknown[]) => mockCreateJE(...args), })) +vi.mock('@/lib/bookkeeping/cancel-orphaned-entry', () => ({ + reverseOrphanedJournalEntry: (...args: unknown[]) => mockReverseOrphanedJE(...args), +})) vi.mock('@/lib/transactions/booking-duplicate-detection', () => ({ detectBookingDuplicate: vi.fn().mockResolvedValue(null), })) @@ -57,16 +61,77 @@ beforeEach(() => { vi.clearAllMocks() eventBus.clear() mockCreateJE.mockResolvedValue({ id: 'je-override-1' }) + mockReverseOrphanedJE.mockResolvedValue(undefined) }) describe('categorizeMatchedTransaction: accountOverride', () => { + it('atomically unignores the transaction when categorizing it', async () => { + const { supabase, enqueue, findCalls } = createQueuedMockSupabase() + const categorizedHandler = vi.fn() + eventBus.on('transaction.categorized', categorizedHandler) + enqueue({ data: txRow({ is_ignored: true }) }) + enqueue({ data: settingsRow }) + enqueue({ data: [{ id: 'fp-1' }] }) + enqueue({ + data: [txRow({ + is_business: false, + category: 'private', + is_ignored: false, + journal_entry_id: 'je-override-1', + })], + }) + + const result = await categorizeMatchedTransaction( + supabase as never, + 'user-1', + 'company-1', + TX_ID, + { category: 'private' }, + ) + + expect(result.error).toBeUndefined() + expect(findCalls('transactions', 'update')).toContainEqual([ + expect.objectContaining({ + is_business: false, + category: 'private', + is_ignored: false, + journal_entry_id: 'je-override-1', + }), + ]) + expect(categorizedHandler).toHaveBeenCalledWith( + expect.objectContaining({ + transaction: expect.objectContaining({ is_ignored: false }), + }), + ) + }) + + it('returns a race conflict when the guarded update matches no row without creating an entry', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: txRow() }) + enqueue({ data: settingsRow }) + enqueue({ data: [{ id: 'fp-1' }] }) + enqueue({ data: [] }) + mockCreateJE.mockResolvedValueOnce(null) + + const result = await categorizeMatchedTransaction( + supabase as never, + 'user-1', + 'company-1', + TX_ID, + { category: 'private' }, + ) + + expect(result.status).toBe(409) + expect(mockReverseOrphanedJE).not.toHaveBeenCalled() + }) + it('posts the entry with the override on the business side', async () => { const { supabase, enqueue } = createQueuedMockSupabase() enqueue({ data: txRow() }) // transactions select enqueue({ data: settingsRow }) // company_settings enqueue({ data: { account_number: '4020', account_class: 4, is_active: true } }) // override chart hit enqueue({ data: [{ id: 'fp-1' }] }) // ensureFiscalPeriod: open period exists - enqueue({ data: null }) // transactions update + enqueue({ data: [{ id: TX_ID }] }) // transactions update const result = await categorizeMatchedTransaction( supabase as never, 'user-1', 'company-1', TX_ID, @@ -90,7 +155,7 @@ describe('categorizeMatchedTransaction: accountOverride', () => { enqueue({ data: settingsRow }) enqueue({ data: { account_number: '4020', account_class: 4, is_active: true } }) enqueue({ data: [{ id: 'fp-1' }] }) // ensureFiscalPeriod - enqueue({ data: null }) // transactions update + enqueue({ data: [{ id: TX_ID }] }) // transactions update const result = await categorizeMatchedTransaction( supabase as never, 'user-1', 'company-1', TX_ID, @@ -138,4 +203,62 @@ describe('categorizeMatchedTransaction: accountOverride', () => { expect(result.error).toMatch(/private/) expect(mockCreateJE).not.toHaveBeenCalled() }) + + it('stornos a posted entry when the ignored-row constraint rejects the link', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: txRow({ is_ignored: true }) }) + enqueue({ data: settingsRow }) + enqueue({ data: [{ id: 'fp-1' }] }) + enqueue({ + data: null, + error: { + code: '23514', + message: + 'new row for relation "transactions" violates check constraint "transactions_is_ignored_no_journal_entry"', + }, + }) + + const result = await categorizeMatchedTransaction( + supabase as never, + 'user-1', + 'company-1', + TX_ID, + { category: 'private' }, + ) + + expect(result.status).toBe(409) + expect(mockReverseOrphanedJE).toHaveBeenCalledWith( + supabase, + 'company-1', + 'user-1', + 'je-override-1', + expect.any(String), + ) + }) + + it('uses a company-scoped CAS and stornos a concurrent loser', async () => { + const { supabase, enqueue, calls } = createQueuedMockSupabase() + enqueue({ data: txRow() }) + enqueue({ data: settingsRow }) + enqueue({ data: [{ id: 'fp-1' }] }) + enqueue({ data: [] }) + + const result = await categorizeMatchedTransaction( + supabase as never, + 'user-1', + 'company-1', + TX_ID, + { category: 'private' }, + ) + + expect(result.status).toBe(409) + expect(mockReverseOrphanedJE).toHaveBeenCalledTimes(1) + expect(calls).toEqual( + expect.arrayContaining([ + { table: 'transactions', method: 'eq', args: ['id', TX_ID] }, + { table: 'transactions', method: 'eq', args: ['company_id', 'company-1'] }, + { table: 'transactions', method: 'is', args: ['journal_entry_id', null] }, + ]), + ) + }) }) diff --git a/lib/transactions/categorize-core.ts b/lib/transactions/categorize-core.ts index 58430790..5606ce98 100644 --- a/lib/transactions/categorize-core.ts +++ b/lib/transactions/categorize-core.ts @@ -28,6 +28,7 @@ import { applyAccountOverride } from '@/lib/bookkeeping/account-override' import { applySettlementAccount } from '@/lib/bookkeeping/mapping-engine' import { resolveSettlementAccount } from '@/lib/bookkeeping/settlement-account' import { createTransactionJournalEntry } from '@/lib/bookkeeping/transaction-entries' +import { reverseOrphanedJournalEntry } from '@/lib/bookkeeping/cancel-orphaned-entry' import { upsertCounterpartyTemplate } from '@/lib/bookkeeping/counterparty-templates' import { isBookkeepingError } from '@/lib/bookkeeping/errors' import { renderChannelContextNotes } from '@/lib/documents/channel-context-notes' @@ -40,6 +41,7 @@ import { hasLiveJournalEntryLink } from '@/lib/transactions/link-journal-entry' import { propagateUnderlagForBookedTransaction } from '@/lib/transactions/inbox-underlag' import { appendProcessingHistory } from '@/lib/processing-history/append' import { createLogger } from '@/lib/logger' +import { getStructuredError } from '@/lib/errors/get-structured-error' import type { InboxChannelContext, Transaction, TransactionCategory, EntityType, VatTreatment } from '@/types' const log = createLogger('transactions/categorize-core') @@ -232,9 +234,9 @@ export async function categorizeMatchedTransaction( // must not block re-categorization: the row reads as "utan koppling" in the // UI, so a fresh booking has to be allowed (issue #988). Only a live posted // link means it was genuinely categorized in the meantime. The UPDATE below - // is unconditional (no null-lock), so it overwrites the stale pointer; the - // duplicate guard still catches an existing live correction and steers the - // user to link instead. + // uses the observed stale pointer as its CAS value, so it only replaces the + // pointer if no concurrent request changed it. The duplicate guard still + // catches an existing live correction and steers the user to link instead. if ( transaction.journal_entry_id && (await hasLiveJournalEntryLink(supabase, companyId, transaction.journal_entry_id)) @@ -392,16 +394,55 @@ export async function categorizeMatchedTransaction( return { error: err instanceof Error ? err.message : 'Failed to create journal entry', status: 500 } } - const { error: updateError } = await supabase + const updateQuery = supabase .from('transactions') - .update({ is_business: isBusiness, category, journal_entry_id: journalEntryId }) + .update({ + is_business: isBusiness, + category, + is_ignored: false, + journal_entry_id: journalEntryId, + }) .eq('id', txId) + .eq('company_id', companyId) + + const guardedUpdate = transaction.journal_entry_id + ? updateQuery.eq('journal_entry_id', transaction.journal_entry_id) + : updateQuery.is('journal_entry_id', null) + + const { data: updateResult, error: updateError } = await guardedUpdate.select('*') if (updateError) { log.error('Failed to update transaction:', updateError) - return { error: 'Failed to update transaction', status: 500 } + if (journalEntryId) { + await reverseOrphanedJournalEntry( + supabase, + companyId, + userId, + journalEntryId, + 'Kategoriseringsverifikation utan transaktionskoppling; automatisk storno misslyckades. Manuell avstämning krävs.', + ) + } + const structured = getStructuredError(updateError) + return structured.code === 'TX_CATEGORIZE_IGNORED_CONFLICT' + ? { error: structured.message_sv, status: 409 } + : { error: 'Failed to update transaction', status: 500 } } + if (!updateResult || updateResult.length === 0) { + if (journalEntryId) { + await reverseOrphanedJournalEntry( + supabase, + companyId, + userId, + journalEntryId, + 'Kategoriseringsverifikation utan transaktionskoppling; automatisk storno misslyckades. Manuell avstämning krävs.', + ) + } + return { error: 'Transaction was categorized by another request.', status: 409 } + } + + const updatedTransaction = updateResult[0] as Transaction + // Propagate the underlag from matched invoice-inbox items onto the new // verifikation and stamp them consumed (BFL 7 kap): shared with the other // booking paths, see lib/transactions/inbox-underlag.ts. Best-effort: the @@ -419,7 +460,7 @@ export async function categorizeMatchedTransaction( await eventBus.emit({ type: 'transaction.categorized', payload: { - transaction: transaction as Transaction, + transaction: updatedTransaction, account: mappingResult.debit_account, taxCode: mappingResult.vat_lines[0]?.account_number || '', userId,