fix(journal-entry): add error logging for delete operation in journal… (#379)

* fix(journal-entry): add error logging for delete operation in journal entries

* fix(journal-entry): enhance error logging and add tests for delete_last_voucher functionality

* fix(journal-entry): restore enforce_journal_entry_immutability function to handle DELETE and un-reversal updates

* fix(tests): refactor delete_last_voucher tests to use insertPostedEntryWithLines for consistency

app/api/bookkeeping/journal-entries/[id]/route.ts

@@ -5,6 +5,9 @@ import { requireWritePermission } from '@/lib/auth/require-write'
 import { ensureInitialized } from '@/lib/init'
 import { eventBus } from '@/lib/events/bus'
 import { getErrorMessage } from '@/lib/errors/get-error-message'
+import { createLogger } from '@/lib/logger'
+
+const logger = createLogger('journal-entries')
 
 ensureInitialized()
 
@@ -59,6 +62,7 @@ export async function DELETE(
   })
 
   if (error) {
+    logger.error('delete_last_voucher failed', { entryId: id, error })
     return NextResponse.json(
       { error: getErrorMessage(error, { context: 'journal_entry', statusCode: 400 }) },
       { status: 400 }

lib/bookkeeping/__tests__/delete-last-voucher.pg.test.ts

@@ -0,0 +1,149 @@
+import { randomUUID } from 'node:crypto'
+import { describe, expect, it } from 'vitest'
+import { getPool, withUserContext } from '@/tests/pg/setup'
+import {
+  insertBalancedLines,
+  insertDraftJournalEntry,
+  seedCompany,
+} from '@/tests/pg/fixtures'
+
+// Set up a posted journal entry with balanced lines, going through draft so
+// the line-immutability trigger is happy. Returns the entry id.
+async function insertPostedEntryWithLines(params: {
+  userId: string
+  companyId: string
+  fiscalPeriodId: string
+  voucherNumber: number
+  reversesId?: string
+  sourceType?: string
+}): Promise<string> {
+  const id = randomUUID()
+  await getPool().query(
+    `INSERT INTO public.journal_entries
+       (id, user_id, company_id, fiscal_period_id, voucher_number, voucher_series,
+        entry_date, description, source_type, status, reverses_id)
+     VALUES ($1, $2, $3, $4, $5, 'A', '2026-06-01', 'Test entry', $6, 'draft', $7)`,
+    [
+      id,
+      params.userId,
+      params.companyId,
+      params.fiscalPeriodId,
+      params.voucherNumber,
+      params.sourceType ?? 'manual',
+      params.reversesId ?? null,
+    ],
+  )
+  await insertBalancedLines(id)
+  await getPool().query(
+    `UPDATE public.journal_entries SET status = 'posted' WHERE id = $1`,
+    [id],
+  )
+  return id
+}
+
+describe('delete_last_voucher.pg — RPC + immutability trigger interaction', () => {
+  it('deletes the last posted voucher in a series', async () => {
+    const { userId, companyId, fiscalPeriodId } = await seedCompany()
+    const entryId = await insertPostedEntryWithLines({
+      userId, companyId, fiscalPeriodId, voucherNumber: 1,
+    })
+
+    await withUserContext(userId, async (client) => {
+      await client.query(
+        `SELECT public.delete_last_voucher($1::uuid, $2::uuid)`,
+        [companyId, entryId],
+      )
+      // Verify inside the txn — withUserContext rolls back on exit, so an
+      // outer pool query would see the row again.
+      const after = await client.query(
+        `SELECT 1 FROM public.journal_entries WHERE id = $1`,
+        [entryId],
+      )
+      expect(after.rowCount).toBe(0)
+    })
+  })
+
+  it('flips original from reversed back to posted when its storno is deleted', async () => {
+    const { userId, companyId, fiscalPeriodId } = await seedCompany()
+
+    const originalId = await insertPostedEntryWithLines({
+      userId, companyId, fiscalPeriodId, voucherNumber: 1,
+    })
+
+    // Storno: insert with reverses_id already set so the immutability trigger
+    // never sees an UPDATE that adds it after the fact.
+    const stornoId = await insertPostedEntryWithLines({
+      userId, companyId, fiscalPeriodId, voucherNumber: 2,
+      sourceType: 'storno', reversesId: originalId,
+    })
+
+    // Mark original as reversed — posted → reversed is allowed by the state
+    // machine as long as no other fields change.
+    await getPool().query(
+      `UPDATE public.journal_entries SET status = 'reversed', reversed_by_id = $1 WHERE id = $2`,
+      [stornoId, originalId],
+    )
+
+    await withUserContext(userId, async (client) => {
+      await client.query(
+        `SELECT public.delete_last_voucher($1::uuid, $2::uuid)`,
+        [companyId, stornoId],
+      )
+      const restored = await client.query<{ status: string; reversed_by_id: string | null }>(
+        `SELECT status, reversed_by_id FROM public.journal_entries WHERE id = $1`,
+        [originalId],
+      )
+      expect(restored.rows[0]!.status).toBe('posted')
+      expect(restored.rows[0]!.reversed_by_id).toBeNull()
+    })
+  })
+
+  it('blocks direct DELETE on a posted entry without the bypass flag', async () => {
+    const { userId, companyId, fiscalPeriodId } = await seedCompany()
+    const entryId = await insertPostedEntryWithLines({
+      userId, companyId, fiscalPeriodId, voucherNumber: 1,
+    })
+
+    await expect(
+      getPool().query(`DELETE FROM public.journal_entries WHERE id = $1`, [entryId]),
+    ).rejects.toThrow(/Cannot delete journal entries/i)
+  })
+
+  it('blocks UPDATE of arbitrary fields on a posted entry even when bypass flag is set', async () => {
+    const { userId, companyId, fiscalPeriodId } = await seedCompany()
+    const entryId = await insertPostedEntryWithLines({
+      userId, companyId, fiscalPeriodId, voucherNumber: 1,
+    })
+
+    const client = await getPool().connect()
+    try {
+      await client.query(`SELECT set_config('gnubok.allow_delete', 'true', true)`)
+      await expect(
+        client.query(
+          `UPDATE public.journal_entries SET description = 'tampered' WHERE id = $1`,
+          [entryId],
+        ),
+      ).rejects.toThrow(/Cannot modify a posted journal entry/i)
+    } finally {
+      client.release()
+    }
+  })
+
+  it('blocks reversed → posted UPDATE without the bypass flag', async () => {
+    const { userId, companyId, fiscalPeriodId } = await seedCompany()
+    const entryId = await insertPostedEntryWithLines({
+      userId, companyId, fiscalPeriodId, voucherNumber: 1,
+    })
+    await getPool().query(
+      `UPDATE public.journal_entries SET status = 'reversed' WHERE id = $1`,
+      [entryId],
+    )
+
+    await expect(
+      getPool().query(
+        `UPDATE public.journal_entries SET status = 'posted' WHERE id = $1`,
+        [entryId],
+      ),
+    ).rejects.toThrow(/Cannot modify a reversed journal entry/i)
+  })
+})

supabase/migrations/20260428160000_fix_journal_entry_immutability_delete_bypass.sql

@@ -0,0 +1,68 @@
+-- Fix: enforce_journal_entry_immutability blocks DELETE and the
+-- reversed → posted "un-reversal" UPDATE that delete_last_voucher RPC needs
+-- when removing a storno entry. The RPC sets gnubok.allow_delete, but the
+-- trigger blocks both operations unconditionally. Result: the RPC fails,
+-- even though BFNAR 2013:2 explicitly permits deletion of the last voucher
+-- in a series.
+--
+-- enforce_journal_entry_line_immutability already respects this flag;
+-- this migration brings the entries trigger in line — but narrowly.
+--
+-- Bypass scope:
+--   * DELETE: allowed when gnubok.allow_delete='true'. delete_last_voucher
+--     enforces all legal constraints (last-in-series, no references, period
+--     not locked, owner/admin) before setting the flag.
+--   * UPDATE: allowed only for the specific reversed → posted transition
+--     used to "un-reverse" the original entry when its storno is being
+--     deleted. All other UPDATE paths still go through the normal state
+--     machine — defense-in-depth on posted-entry mutation is preserved.
+
+CREATE OR REPLACE FUNCTION public.enforce_journal_entry_immutability()
+RETURNS trigger
+LANGUAGE plpgsql
+AS $function$
+BEGIN
+  IF TG_OP = 'DELETE' THEN
+    IF current_setting('gnubok.allow_delete', true) = 'true' THEN
+      RETURN OLD;
+    END IF;
+    RAISE EXCEPTION 'Cannot delete journal entries (id: %, status: %). Use cancelled status instead.',
+      OLD.id, OLD.status;
+  END IF;
+
+  IF OLD.status = 'draft' AND NEW.status IN ('draft', 'posted', 'cancelled') THEN
+    RETURN NEW;
+  END IF;
+
+  IF OLD.status = 'posted' AND NEW.status IN ('reversed', 'cancelled') THEN
+    IF NEW.status = 'reversed' THEN
+      IF NEW.description != OLD.description OR NEW.entry_date != OLD.entry_date
+         OR NEW.fiscal_period_id != OLD.fiscal_period_id
+         OR NEW.voucher_number != OLD.voucher_number
+         OR NEW.commit_method IS DISTINCT FROM OLD.commit_method
+         OR NEW.rubric_version IS DISTINCT FROM OLD.rubric_version
+         OR NEW.source_voucher_series IS DISTINCT FROM OLD.source_voucher_series
+         OR NEW.source_voucher_number IS DISTINCT FROM OLD.source_voucher_number THEN
+        RAISE EXCEPTION 'Cannot modify fields of a posted entry during reversal (id: %)', OLD.id;
+      END IF;
+    END IF;
+    RETURN NEW;
+  END IF;
+
+  -- Narrow un-reversal path: when delete_last_voucher removes a storno entry,
+  -- it flips the original from 'reversed' back to 'posted'. No other fields
+  -- may change, and the bypass flag must be set.
+  IF OLD.status = 'reversed' AND NEW.status = 'posted'
+     AND current_setting('gnubok.allow_delete', true) = 'true' THEN
+    IF NEW.description != OLD.description OR NEW.entry_date != OLD.entry_date
+       OR NEW.fiscal_period_id != OLD.fiscal_period_id
+       OR NEW.voucher_number != OLD.voucher_number THEN
+      RAISE EXCEPTION 'Cannot modify fields during un-reversal (id: %)', OLD.id;
+    END IF;
+    RETURN NEW;
+  END IF;
+
+  RAISE EXCEPTION 'Cannot modify a % journal entry (id: %). Committed entries are immutable per Bokforingslagen.',
+    OLD.status, OLD.id;
+END;
+$function$;