fix(documents): anchor underlag at deferred SI booking, sanctioned duplicate detach (#1860)

* fix(documents): anchor underlag at deferred SI booking, sanctioned duplicate detach Support case 2026-08-24: a verifikat booked from a leverantorsfaktura/utlagg stayed under 'Saknar underlag' with the PDF attached, and a twice-uploaded underlag could only be replaced, never removed. - POST /api/supplier-invoices/[id]/book now calls anchorSupplierInvoiceDocument() after the CAS link: the deferred (#967) flow was the last booking surface that never anchored the invoice's retained source document, so every missing-underlag surface kept flagging the registration verifikat until payment. - Repair migration 20260824150000 re-runs the 20260727180000 sweep for rows created since (idempotent, open unlocked periods only). - New detach_underlag_duplicate RPC (migration 20260824151000): the one sanctioned path to detach a redundant duplicate underlag from a posted verifikat. Guarded: writer role, open unlocked period, company lock date, at least one other anchored underlag must remain (BFL 5 kap 7 par), pinned docs (transactions/supplier_invoices.document_id) stay replace-only. Audit-logged first, transaction-local gnubok.allow_delete carve-out. The file is never deleted: it returns to the unlinked pool. - POST /api/documents/[id]/detach + 'Koppla bort dubblett' in the verifikat attachments blocked-dialog when the entry keeps 2+ direct docs (sv+en). - Tests: book-route anchor assertions, detach route unit tests, pg-real suite for the RPC incl. the direct-UPDATE-stays-blocked invariant. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(documents): harden detach_underlag_duplicate per skeptic findings - Require sha256 identity: detach only when a remaining anchored sibling carries the same immutable sha256_hash, so only byte-identical duplicates ever leave a verifikat (two different handlingar both stay behind the WORM guards). UI gates the button on the same condition. - Enforce the documented posted-status guard (reversed/cancelled verifikat refuse detach). - Set company_id on the RPC's audit_log row: the SELECT policy filters on company_id, so the provenance row was invisible to every reader (same defect 20260528120600 fixed for delete_last_voucher). - Swedish 403 message on the tenant guard (CodeRabbit). - pg tests: closed-period case now seeds open and closes via UPDATE (the period-lock trigger blocks seeding into a closed period), duplicate pairs share a hash, added non-duplicate and reversed-entry refusals, audit assertion pins the RPC's own row (description + company_id + actor). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-08-24 17:05:00 +02:00
parent 3edbf0a2e3
commit 174f315d9c
11 changed files with 804 additions and 1 deletions
@@ -0,0 +1,125 @@
+/**
+ * Tests for POST /api/documents/[id]/detach (detach a duplicate underlag from
+ * a posted verifikat via the audited detach_underlag_duplicate RPC).
+ *
+ * Covers: 401, rule-violation 409 passthrough (Swedish RPC messages verbatim),
+ * tenant guard 403, unexpected RPC failure 500, and the happy path.
+ */
+import { describe, it, expect, vi, beforeEach } from 'vitest'
+import { NextResponse } from 'next/server'
+import {
+  createQueuedMockSupabase,
+  createMockRequest,
+  createMockRouteParams,
+  parseJsonResponse,
+} from '@/tests/helpers'
+
+const { supabase, reset } = createQueuedMockSupabase()
+
+const rpcMock = vi.fn()
+;(supabase as { rpc?: unknown }).rpc = rpcMock
+
+const requireAuthMock = vi.fn()
+vi.mock('@/lib/auth/require-auth', () => ({
+  requireAuth: (...args: unknown[]) => requireAuthMock(...args),
+}))
+
+vi.mock('@/lib/company/context', () => ({
+  getActiveCompanyId: vi.fn().mockResolvedValue('company-1'),
+  requireCompanyId: vi.fn().mockResolvedValue('company-1'),
+}))
+
+vi.mock('@/lib/auth/require-write', () => ({
+  requireWritePermission: vi.fn().mockResolvedValue({ ok: true }),
+}))
+
+vi.mock('@/lib/init', () => ({ ensureInitialized: vi.fn() }))
+
+import { POST } from '../route'
+
+const params = () => createMockRouteParams({ id: 'doc-1' })
+
+function makeRequest() {
+  return createMockRequest('/api/documents/doc-1/detach', { method: 'POST' })
+}
+
+describe('POST /api/documents/[id]/detach', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+    reset()
+    requireAuthMock.mockResolvedValue({ user: { id: 'user-1' }, supabase })
+  })
+
+  it('returns 401 when not authenticated', async () => {
+    requireAuthMock.mockResolvedValue({
+      error: NextResponse.json({ error: 'Unauthorized' }, { status: 401 }),
+    })
+
+    const response = await POST(makeRequest(), params())
+    expect(response.status).toBe(401)
+    expect(rpcMock).not.toHaveBeenCalled()
+  })
+
+  it('passes rule violations through as 409 with the Swedish message', async () => {
+    rpcMock.mockResolvedValue({
+      data: null,
+      error: {
+        code: 'P0001',
+        message:
+          'Verifikationen skulle stå utan underlag: det sista underlaget kan inte kopplas bort. Ersätt det med en ny version i stället.',
+      },
+    })
+
+    const response = await POST(makeRequest(), params())
+    const { body } = await parseJsonResponse<{ error: string }>(response)
+
+    expect(response.status).toBe(409)
+    expect(body.error).toContain('sista underlaget')
+  })
+
+  it('maps the tenant guard (42501) to 403', async () => {
+    rpcMock.mockResolvedValue({
+      data: null,
+      error: { code: '42501', message: 'unauthorized: caller is not a member of company company-1' },
+    })
+
+    const response = await POST(makeRequest(), params())
+    expect(response.status).toBe(403)
+  })
+
+  it('returns 500 on unexpected RPC failure', async () => {
+    rpcMock.mockResolvedValue({
+      data: null,
+      error: { code: '57014', message: 'canceling statement due to statement timeout' },
+    })
+
+    const response = await POST(makeRequest(), params())
+    expect(response.status).toBe(500)
+  })
+
+  it('detaches the duplicate and returns the RPC result', async () => {
+    rpcMock.mockResolvedValue({
+      data: {
+        detached: true,
+        document_id: 'doc-1',
+        journal_entry_id: 'je-1',
+        remaining_documents: 1,
+      },
+      error: null,
+    })
+
+    const response = await POST(makeRequest(), params())
+    const { body } = await parseJsonResponse<{
+      data: { detached: boolean; journal_entry_id: string; remaining_documents: number }
+    }>(response)
+
+    expect(response.status).toBe(200)
+    expect(body.data.detached).toBe(true)
+    expect(body.data.remaining_documents).toBe(1)
+    expect(rpcMock).toHaveBeenCalledWith('detach_underlag_duplicate', {
+      p_company_id: 'company-1',
+      p_document_id: 'doc-1',
+      p_user_id: 'user-1',
+    })
+  })
+})
@@ -0,0 +1,48 @@
+import { NextResponse } from 'next/server'
+import { withRouteContext } from '@/lib/api/with-route-context'
+import { getErrorMessage } from '@/lib/errors/get-error-message'
+
+/**
+ * POST /api/documents/[id]/detach
+ *
+ * Detach a redundant duplicate underlag from its posted verifikation. The
+ * detach_underlag_duplicate RPC enforces everything (writer role, open and
+ * unlocked period, company lock date, at least one other anchored underlag
+ * remaining, not pinned to a transaction or supplier invoice) and writes an
+ * append-only audit_log row before the carve-out UPDATE. The document itself
+ * is never deleted: it returns to the unlinked document pool, where the
+ * ordinary deletion rules apply (an unlinked doc may be deleted).
+ */
+export const POST = withRouteContext<{ params: Promise<{ id: string }> }>(
+  'documents.detach',
+  async (_request, { supabase, companyId, user, log }, { params }) => {
+    const { id } = await params
+
+    const { data, error } = await supabase.rpc('detach_underlag_duplicate', {
+      p_company_id: companyId,
+      p_document_id: id,
+      p_user_id: user.id,
+    })
+
+    if (error) {
+      // Rule violations are plain RAISE EXCEPTION (P0001) with user-facing
+      // Swedish messages: surface verbatim as 409. Tenant guard raises 42501.
+      if (error.code === 'P0001') {
+        return NextResponse.json({ error: getErrorMessage(error) }, { status: 409 })
+      }
+      if (error.code === '42501') {
+        // The RPC's tenant-guard message is English (log/diagnostic text);
+        // getErrorMessage would pass it through verbatim, so map it here.
+        return NextResponse.json(
+          { error: 'Du saknar behörighet att ändra underlag i det här företaget.' },
+          { status: 403 },
+        )
+      }
+      log.error('detach_underlag_duplicate failed', new Error(error.message), { documentId: id })
+      return NextResponse.json({ error: 'Underlaget kunde inte kopplas bort' }, { status: 500 })
+    }
+
+    return NextResponse.json({ data })
+  },
+  { requireWrite: true },
+)
@@ -39,6 +39,11 @@ vi.mock('@/lib/bookkeeping/cancel-orphaned-entry', () => ({
  cancelOrphanedPaymentEntry: (...args: unknown[]) => mockCancelOrphan(...args),
 }))

+const mockAnchorDocument = vi.fn()
+vi.mock('@/lib/core/documents/supplier-invoice-underlag', () => ({
+  anchorSupplierInvoiceDocument: (...args: unknown[]) => mockAnchorDocument(...args),
+}))
+
 import { POST } from '../route'

 const mockUser = { id: 'user-1', email: 'test@test.se' }
@@ -152,6 +157,7 @@ describe('POST /api/supplier-invoices/[id]/book', () => {
      'je-1',
      expect.any(String),
    )
+    expect(mockAnchorDocument).not.toHaveBeenCalled()
  })

  it('books the registration entry and links it', async () => {
@@ -172,6 +178,9 @@ describe('POST /api/supplier-invoices/[id]/book', () => {
    expect(mockCreateRegistrationEntry).toHaveBeenCalled()
    // No accrual items on the fixture, so no schedule creation.
    expect(mockCreateSchedules).not.toHaveBeenCalled()
+    // The invoice's retained source document is anchored to the fresh
+    // registration verifikat (no-op inside the helper when there is none).
+    expect(mockAnchorDocument).toHaveBeenCalledWith(mockSupabase, 'company-1', 'si-1')
  })

  it('creates accrual schedules and surfaces failures as warnings', async () => {
@@ -5,6 +5,7 @@ import { isBookkeepingError } from '@/lib/bookkeeping/errors'
 import { createSupplierInvoiceRegistrationEntry } from '@/lib/bookkeeping/supplier-invoice-entries'
 import { createSchedulesForSupplierInvoice } from '@/lib/bookkeeping/accruals/from-invoices'
 import { cancelOrphanedPaymentEntry } from '@/lib/bookkeeping/cancel-orphaned-entry'
+import { anchorSupplierInvoiceDocument } from '@/lib/core/documents/supplier-invoice-underlag'
 import type { SupplierInvoice, SupplierInvoiceItem } from '@/types'

 // Statuses where the registration entry can still be created afterwards.
@@ -122,6 +123,14 @@ export const POST = withRouteContext(
      return errorResponseFromCode('SI_BOOK_CONFLICT', log, { requestId })
    }

+    // The invoice's retained source document (attached at registration) has
+    // been floating until now: the deferred flow books later, and every
+    // missing-underlag surface only accepts an ANCHORED doc
+    // (document_attachments.journal_entry_id set). Anchor it to the fresh
+    // registration verifikat, same as the create route does when it books
+    // immediately. Never throws; a no-op when the invoice has no document.
+    await anchorSupplierInvoiceDocument(supabase, companyId!, id)
+
    // Periodiseringar ride on the registration entry, so they can only be
    // created now. Non-blocking: the entry is committed (immutable); a
    // schedule failure is surfaced as a warning and retried from the