feat: concurrency guards, account validation, and reversal side-effects (#247)

* feat: add concurrency guards, account validation, and reversal side-effects to bookkeeping engine

Prevent double-booking via CAS guards on mark-paid and categorize routes (409 on conflict),
make payment GL entries blocking (AP/AR must match GL), validate account resolution in engine,
and auto-sync invoice status on payment reversal. Adds journal_entry.reversed event type.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: address Greptile review — company_id filter, voucher gaps, status restore

- Add missing company_id filter on supplier-invoice CAS update (defense in depth)
- Add voucher_gap_explanations insert on CAS-cancelled entries in both mark-paid
  routes (BFNAR 2013:2 compliance, matching categorize route pattern)
- Fix reversal status restore: check due_date to determine overdue vs sent/approved
  instead of always reverting to sent/approved
- Rename shadowed reversedLines variable to originalLines (P2 clarity)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: derive reversal payment amount from payments table, not GL lines

The reversal GL entry is already a line-by-line mirror per BFL 5 kap 5§.
For the business-level invoice sync, use the payment record amount from
supplier_invoice_payments / invoice_payments instead of inspecting GL
account numbers — works identically for kontantmetod and faktureringsmetod
without needing to know which accounts were used.

Also adds company_id filter on all reversal sync queries (defense in depth).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: allow reversal of partially_paid customer invoices

Widen the status filter from .eq('status', 'paid') to
.in('status', ['paid', 'partially_paid']) so that reversing a partial
payment GL entry correctly updates the invoice state.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Jakob Wennberg
2026-04-15 15:23:08 +02:00
committed by GitHub
co-authored by Claude Opus 4.6
parent d484c341a4
commit e46654ab25
12 changed files with 285 additions and 36 deletions
+20 -3
View File
@@ -64,9 +64,9 @@ export async function POST(
}
```
## Non-Blocking Journal Entry Creation
## Supplementary Journal Entry Creation (Non-Blocking)
Journal entry failures must never block the business operation:
When the journal entry is a side effect of the primary operation (e.g., categorizing a transaction), failures must not block:
```typescript
try {
@@ -82,6 +82,23 @@ try {
}
```
## Payment Journal Entry Creation (Blocking)
When the journal entry IS the accounting record (mark-paid, mark-sent for cash method), GL failure must block the operation. Without the GL entry, AP/AR diverges from GL:
```typescript
try {
const journalEntry = await createPaymentJournalEntry(...)
if (journalEntry) journalEntryId = journalEntry.id
} catch (err) {
console.error('Failed to create payment journal entry:', err)
return NextResponse.json(
{ error: 'Kunde inte bokföra betalningen' },
{ status: 500 }
)
}
```
## Response Conventions
- Success: `NextResponse.json({ data: result })`
@@ -112,6 +129,6 @@ if (!data) {
1. Forgetting `ensureInitialized()` on routes that emit events — events silently won't fire
2. Using `params.id` instead of `(await params).id` — Next.js 16 breaking change
3. Missing `user_id` filter on queries — relies solely on RLS
4. Blocking on journal entry failure — must wrap in try/catch
4. Blocking on supplementary journal entry failure — must wrap in try/catch (but payment entries MUST block — see above)
5. Returning `{ message }` instead of `{ error }` on failure — inconsistent with codebase
6. Forgetting `await` on `createClient()` — it's async in server context
@@ -125,8 +125,8 @@ describe('POST /api/invoices/[id]/mark-paid', () => {
enqueue({ data: invoice, error: null })
// Fetch company settings (now before update due to journal-first ordering)
enqueue({ data: { accounting_method: 'accrual', entity_type: 'enskild_firma' }, error: null })
// Update invoice status
enqueue({ data: null, error: null })
// Update invoice status (CAS guard: returns matched row)
enqueue({ data: [{ id: 'inv-1' }], error: null })
mockCreateInvoicePaymentJournalEntry.mockResolvedValue({ id: 'je-1' })
@@ -166,7 +166,8 @@ describe('POST /api/invoices/[id]/mark-paid', () => {
enqueue({ data: invoice, error: null })
enqueue({ data: { accounting_method: 'cash', entity_type: 'enskild_firma' }, error: null })
enqueue({ data: null, error: null })
// Update invoice status (CAS guard: returns matched row)
enqueue({ data: [{ id: 'inv-1' }], error: null })
mockCreateInvoiceCashEntry.mockResolvedValue({ id: 'je-2' })
@@ -213,8 +214,8 @@ describe('POST /api/invoices/[id]/mark-paid', () => {
enqueue({ data: invoice, error: null })
// Fetch company settings (before update — journal-first ordering)
enqueue({ data: { accounting_method: 'accrual', entity_type: 'enskild_firma' }, error: null })
// Update invoice status
enqueue({ data: null, error: null })
// Update invoice status (CAS guard: returns matched row)
enqueue({ data: [{ id: 'inv-1' }], error: null })
mockFindFiscalPeriod.mockResolvedValue('fp-1')
mockCreateJournalEntry.mockResolvedValue({ id: 'je-custom' })
@@ -314,7 +315,8 @@ describe('POST /api/invoices/[id]/mark-paid', () => {
enqueue({ data: invoice, error: null })
enqueue({ data: { accounting_method: 'accrual', entity_type: 'enskild_firma' }, error: null })
enqueue({ data: null, error: null })
// Update invoice status (CAS guard: returns matched row)
enqueue({ data: [{ id: 'inv-1' }], error: null })
mockCreateInvoicePaymentJournalEntry.mockResolvedValue({ id: 'je-auto' })
+35 -2
View File
@@ -168,8 +168,8 @@ export async function POST(
}
}
// Update status to paid — only after journal entry succeeds
const { error: updateError } = await supabase
// Update status to paid (CAS guard: only if still in payable status)
const { data: updateResult, error: updateError } = await supabase
.from('invoices')
.update({
status: 'paid',
@@ -178,11 +178,44 @@ export async function POST(
})
.eq('id', id)
.eq('company_id', companyId)
.in('status', ['sent', 'overdue'])
.select('id')
if (updateError) {
return NextResponse.json({ error: 'Kunde inte uppdatera status' }, { status: 500 })
}
// CAS guard: status changed between our read and write
if (!updateResult || updateResult.length === 0) {
if (journalEntryId) {
const { data: orphan } = await supabase
.from('journal_entries')
.select('fiscal_period_id, voucher_series, voucher_number')
.eq('id', journalEntryId)
.single()
await supabase
.from('journal_entries')
.update({ status: 'cancelled' })
.eq('id', journalEntryId)
if (orphan) {
await supabase.from('voucher_gap_explanations').insert({
company_id: companyId,
fiscal_period_id: orphan.fiscal_period_id,
voucher_series: orphan.voucher_series || 'A',
gap_number: orphan.voucher_number,
explanation: 'Automatiskt makulerad: dubblettbokning förhindrad av samtidighetsskydd',
created_by: user.id,
})
}
}
return NextResponse.json(
{ error: 'Fakturan har redan betalats av en annan förfrågan' },
{ status: 409 }
)
}
return NextResponse.json({
success: true,
status: 'paid',
@@ -116,8 +116,8 @@ describe('POST /api/supplier-invoices/[id]/mark-paid', () => {
mockCreateSupplierInvoicePaymentEntry.mockResolvedValue({ id: 'je-1' })
// Update invoice
enqueue({ data: null, error: null })
// Update invoice (CAS guard: returns matched row)
enqueue({ data: [{ id: 'si-1' }], error: null })
// Record payment
enqueue({ data: null, error: null })
@@ -160,7 +160,8 @@ describe('POST /api/supplier-invoices/[id]/mark-paid', () => {
mockCreateSupplierInvoicePaymentEntry.mockResolvedValue({ id: 'je-2' })
enqueue({ data: null, error: null })
// Update invoice (CAS guard: returns matched row)
enqueue({ data: [{ id: 'si-1' }], error: null })
enqueue({ data: null, error: null })
const request = createMockRequest('/api/supplier-invoices/si-1/mark-paid', {
@@ -215,7 +216,8 @@ describe('POST /api/supplier-invoices/[id]/mark-paid', () => {
mockCreateSupplierInvoiceCashEntry.mockResolvedValue({ id: 'je-3' })
enqueue({ data: null, error: null })
// Update invoice (CAS guard: returns matched row)
enqueue({ data: [{ id: 'si-1' }], error: null })
enqueue({ data: null, error: null })
const request = createMockRequest('/api/supplier-invoices/si-1/mark-paid', {
@@ -234,7 +236,7 @@ describe('POST /api/supplier-invoices/[id]/mark-paid', () => {
expect(mockCreateSupplierInvoicePaymentEntry).not.toHaveBeenCalled()
})
it('returns success when journal entry creation fails (non-blocking)', async () => {
it('returns 500 when journal entry creation fails (blocking — GL must succeed for payment)', async () => {
const supplier = makeSupplier()
const invoice = makeSupplierInvoice({
id: 'si-1',
@@ -251,22 +253,15 @@ describe('POST /api/supplier-invoices/[id]/mark-paid', () => {
mockCreateSupplierInvoicePaymentEntry.mockRejectedValue(new Error('Period locked'))
enqueue({ data: null, error: null })
enqueue({ data: null, error: null })
const request = createMockRequest('/api/supplier-invoices/si-1/mark-paid', {
method: 'POST',
body: {},
})
const response = await POST(request, createMockRouteParams({ id: 'si-1' }))
const { status, body } = await parseJsonResponse<{
success: boolean
journal_entry_id: null
}>(response)
const { status, body } = await parseJsonResponse<{ error: string }>(response)
expect(status).toBe(200)
expect(body.success).toBe(true)
expect(body.journal_entry_id).toBeNull()
expect(status).toBe(500)
expect(body.error).toBe('Kunde inte bokföra betalningen')
})
it('emits supplier_invoice.paid event', async () => {
@@ -284,7 +279,8 @@ describe('POST /api/supplier-invoices/[id]/mark-paid', () => {
enqueue({ data: invoice, error: null })
enqueue({ data: { accounting_method: 'accrual' }, error: null })
mockCreateSupplierInvoicePaymentEntry.mockResolvedValue({ id: 'je-1' })
enqueue({ data: null, error: null })
// Update invoice (CAS guard: returns matched row)
enqueue({ data: [{ id: 'si-1' }], error: null })
enqueue({ data: null, error: null })
const emitSpy = vi.spyOn(eventBus, 'emit')
@@ -99,6 +99,10 @@ export async function POST(
}
} catch (err) {
console.error('Failed to create payment journal entry:', err)
return NextResponse.json(
{ error: 'Kunde inte bokföra betalningen' },
{ status: 500 }
)
}
// Calculate new remaining amount
@@ -107,8 +111,8 @@ export async function POST(
const isFullyPaid = newRemaining <= 0
const newStatus = isFullyPaid ? 'paid' : 'partially_paid'
// Update invoice
const { error: updateError } = await supabase
// Update invoice (CAS guard: only if status hasn't changed since we read it)
const { data: updateResult, error: updateError } = await supabase
.from('supplier_invoices')
.update({
status: newStatus,
@@ -118,11 +122,45 @@ export async function POST(
payment_journal_entry_id: journalEntryId,
})
.eq('id', id)
.eq('company_id', companyId)
.in('status', ['registered', 'approved', 'partially_paid', 'overdue'])
.select('id')
if (updateError) {
return NextResponse.json({ error: updateError.message }, { status: 500 })
}
// CAS guard: status changed between our read and write
if (!updateResult || updateResult.length === 0) {
if (journalEntryId) {
const { data: orphan } = await supabase
.from('journal_entries')
.select('fiscal_period_id, voucher_series, voucher_number')
.eq('id', journalEntryId)
.single()
await supabase
.from('journal_entries')
.update({ status: 'cancelled' })
.eq('id', journalEntryId)
if (orphan) {
await supabase.from('voucher_gap_explanations').insert({
company_id: companyId,
fiscal_period_id: orphan.fiscal_period_id,
voucher_series: orphan.voucher_series || 'A',
gap_number: orphan.voucher_number,
explanation: 'Automatiskt makulerad: dubblettbokning förhindrad av samtidighetsskydd',
created_by: user.id,
})
}
}
return NextResponse.json(
{ error: 'Fakturan har redan betalats av en annan förfrågan' },
{ status: 409 }
)
}
// Record payment
const { error: paymentError } = await supabase
.from('supplier_invoice_payments')
@@ -146,8 +146,8 @@ describe('POST /api/transactions/[id]/categorize', () => {
mockCreateTransactionJournalEntry.mockResolvedValue({ id: 'je-1' })
mockSaveUserMappingRule.mockResolvedValue(undefined)
// Update transaction
enqueue({ data: null, error: null })
// Update transaction (CAS guard: returns matched row)
enqueue({ data: [{ id: 'tx-1' }], error: null })
const emitSpy = vi.spyOn(eventBus, 'emit')
@@ -284,8 +284,8 @@ describe('POST /api/transactions/[id]/categorize', () => {
mockCreateTransactionJournalEntry.mockResolvedValue({ id: 'je-1' })
// Update transaction
enqueue({ data: null, error: null })
// Update transaction (CAS guard: returns matched row)
enqueue({ data: [{ id: 'tx-1' }], error: null })
const request = createMockRequest('/api/transactions/tx-1/categorize', {
method: 'POST',
+35 -2
View File
@@ -380,8 +380,8 @@ export async function POST(
}
}
// Update the transaction
const { error: updateError } = await supabase
// Update the transaction (CAS guard: only set journal_entry_id if still null)
const { data: updateResult, error: updateError } = await supabase
.from('transactions')
.update({
is_business,
@@ -389,6 +389,8 @@ export async function POST(
journal_entry_id: journalEntryId,
})
.eq('id', id)
.is('journal_entry_id', null)
.select('id')
if (updateError) {
console.error('Failed to update transaction:', updateError)
@@ -398,6 +400,37 @@ export async function POST(
)
}
// CAS guard: another request already set journal_entry_id
if ((!updateResult || updateResult.length === 0) && journalEntryId) {
// Cancel the orphaned journal entry and document the voucher gap
const { data: orphan } = await supabase
.from('journal_entries')
.select('fiscal_period_id, voucher_series, voucher_number')
.eq('id', journalEntryId)
.single()
await supabase
.from('journal_entries')
.update({ status: 'cancelled' })
.eq('id', journalEntryId)
if (orphan) {
await supabase.from('voucher_gap_explanations').insert({
company_id: companyId,
fiscal_period_id: orphan.fiscal_period_id,
voucher_series: orphan.voucher_series || 'A',
gap_number: orphan.voucher_number,
explanation: 'Automatiskt makulerad: dubblettbokning förhindrad av samtidighetsskydd',
created_by: user.id,
})
}
return NextResponse.json(
{ error: 'Transaction was already categorized by another request' },
{ status: 409 }
)
}
await eventBus.emit({
type: 'transaction.categorized',
payload: {
+111
View File
@@ -174,6 +174,15 @@ export async function createDraftEntry(
// Resolve account IDs
const accountIdMap = await resolveAccountIds(supabase, companyId, input.lines)
// Validate all account numbers resolved to IDs
const allAccountNumbers = [...new Set(input.lines.map(l => l.account_number))]
const missingAccounts = allAccountNumbers.filter(num => !accountIdMap.has(num))
if (missingAccounts.length > 0) {
throw new Error(
`Account(s) not found in chart of accounts: ${missingAccounts.join(', ')}`
)
}
// Insert journal entry header as draft (voucher_number = 0, will be assigned on commit)
const { data: entry, error: entryError } = await supabase
.from('journal_entries')
@@ -350,6 +359,15 @@ export async function reverseEntry(
// Resolve account IDs
const accountIdMap = await resolveAccountIds(supabase, companyId, reversedLines)
// Validate all account numbers resolved to IDs
const reversalAccountNumbers = [...new Set(reversedLines.map(l => l.account_number))]
const missingReversalAccounts = reversalAccountNumbers.filter(num => !accountIdMap.has(num))
if (missingReversalAccounts.length > 0) {
throw new Error(
`Account(s) not found in chart of accounts: ${missingReversalAccounts.join(', ')}`
)
}
// Create reversal entry with reverses_id link
const { data: reversalEntry, error: reversalError } = await supabase
.from('journal_entries')
@@ -417,6 +435,94 @@ export async function reverseEntry(
throw new Error('Entry was already reversed by a concurrent operation')
}
// If this was a payment entry, sync the linked invoice/supplier-invoice status
const paymentSourceTypes = [
'invoice_paid', 'invoice_cash_payment',
'supplier_invoice_paid', 'supplier_invoice_cash_payment',
]
if (paymentSourceTypes.includes(original.source_type) && original.source_id) {
// The GL reversal is already handled above (line-by-line mirror of the original
// verifikation per BFL 5 kap 5§). Here we sync the business-level invoice state.
// Payment amounts come from the payments table, not from GL line inspection —
// this works identically for kontantmetod and faktureringsmetod.
const entryId = original.id
if (original.source_type.startsWith('supplier_invoice')) {
const { data: payment } = await supabase
.from('supplier_invoice_payments')
.select('amount')
.eq('journal_entry_id', entryId)
.single()
const { data: supplierInvoice } = await supabase
.from('supplier_invoices')
.select('paid_amount, total_amount, due_date')
.eq('id', original.source_id)
.eq('company_id', companyId)
.single()
if (supplierInvoice && payment) {
const newPaidAmount = Math.round((supplierInvoice.paid_amount - payment.amount) * 100) / 100
const newRemaining = Math.round((supplierInvoice.total_amount - Math.max(0, newPaidAmount)) * 100) / 100
let newStatus: string
if (newPaidAmount > 0) {
newStatus = 'partially_paid'
} else if (supplierInvoice.due_date && new Date(supplierInvoice.due_date) < new Date()) {
newStatus = 'overdue'
} else {
newStatus = 'approved'
}
await supabase
.from('supplier_invoices')
.update({
status: newStatus,
paid_amount: Math.max(0, newPaidAmount),
remaining_amount: newRemaining,
paid_at: null,
payment_journal_entry_id: null,
})
.eq('id', original.source_id)
.eq('company_id', companyId)
}
} else {
const { data: payment } = await supabase
.from('invoice_payments')
.select('amount')
.eq('journal_entry_id', entryId)
.single()
const { data: customerInvoice } = await supabase
.from('invoices')
.select('paid_amount, due_date')
.eq('id', original.source_id)
.eq('company_id', companyId)
.single()
if (customerInvoice) {
const paymentAmount = payment?.amount ?? customerInvoice.paid_amount
const newPaidAmount = Math.round((customerInvoice.paid_amount - paymentAmount) * 100) / 100
const revertStatus = newPaidAmount > 0
? 'partially_paid'
: customerInvoice.due_date && new Date(customerInvoice.due_date) < new Date()
? 'overdue'
: 'sent'
await supabase
.from('invoices')
.update({
status: revertStatus,
paid_at: null,
paid_amount: Math.max(0, newPaidAmount),
})
.eq('id', original.source_id)
.eq('company_id', companyId)
.in('status', ['paid', 'partially_paid'])
}
}
}
// Fetch complete reversal entry with lines
const { data: completeEntry } = await supabase
.from('journal_entries')
@@ -431,5 +537,10 @@ export async function reverseEntry(
payload: { entry: result, userId, companyId },
})
await eventBus.emit({
type: 'journal_entry.reversed',
payload: { originalEntry: original as JournalEntry, reversalEntry: result, userId, companyId },
})
return result
}
@@ -163,7 +163,7 @@ describe('correctEntry', () => {
{ data: reversalEntry, error: null }, // 1: insert reversal
{ data: null, error: null }, // 2: insert reversal lines
{ data: null, error: null }, // 3: post reversal
{ data: [], error: null }, // 4: accounts
{ data: [{ id: 'acc-5420', account_number: '5420' }, { id: 'acc-1930', account_number: '1930' }], error: null }, // 4: accounts
{ data: null, error: { message: 'DB error' } }, // 5: insert corrected FAILS
{ data: null, error: null }, // 6: cancelEntry reversal update
{ data: null, error: null }, // 7: cancelEntry reversal lines delete
+8
View File
@@ -174,6 +174,14 @@ export async function correctEntry(
accountIdMap.set(account.account_number, account.id)
}
// Validate all account numbers resolved to IDs
const missingAccounts = accountNumbers.filter(num => !accountIdMap.has(num))
if (missingAccounts.length > 0) {
throw new Error(
`Account(s) not found in chart of accounts: ${missingAccounts.join(', ')}`
)
}
const { data: newEntry, error: correctedError } = await supabase
.from('journal_entries')
.insert({
+10
View File
@@ -11,6 +11,7 @@ const log = createLogger('event-log')
*/
const PERSISTED_EVENT_TYPES: CoreEventType[] = [
'journal_entry.committed',
'journal_entry.reversed',
'journal_entry.corrected',
'document.uploaded',
'document.accessed',
@@ -67,6 +68,15 @@ function extractEntityId(payload: Record<string, unknown>): string | null {
}
}
// For journal_entry.reversed: use the reversal entry's ID
if ('reversalEntry' in payload) {
const reversalEntry = payload.reversalEntry
if (reversalEntry && typeof reversalEntry === 'object' && 'id' in reversalEntry) {
const id = (reversalEntry as Record<string, unknown>).id
if (typeof id === 'string') return id
}
}
return null
}
+1
View File
@@ -21,6 +21,7 @@ export type CoreEvent =
| { type: 'journal_entry.drafted'; payload: { entry: JournalEntry; userId: string; companyId: string } }
| { type: 'journal_entry.committed'; payload: { entry: JournalEntry; userId: string; companyId: string } }
| { type: 'journal_entry.corrected'; payload: { original: JournalEntry; storno: JournalEntry; corrected: JournalEntry; userId: string; companyId: string } }
| { type: 'journal_entry.reversed'; payload: { originalEntry: JournalEntry; reversalEntry: JournalEntry; userId: string; companyId: string } }
| { type: 'journal_entry.deleted'; payload: { entryId: string; voucherSeries: string; voucherNumber: number; userId: string; companyId: string } }
// Documents
| { type: 'document.uploaded'; payload: { document: DocumentAttachment; userId: string; companyId: string } }