From 174f315d9c97a402a60ff6453833f0d5ba0850b8 Mon Sep 17 00:00:00 2001 From: Mattsson <111893710+mattssonn@users.noreply.github.com> Date: Mon, 24 Aug 2026 17:05:00 +0200 Subject: [PATCH] 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 * 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 --------- Co-authored-by: Claude Fable 5 --- DECISIONS.md | 1 + .../[id]/detach/__tests__/route.test.ts | 125 +++++++++ app/api/documents/[id]/detach/route.ts | 48 ++++ .../[id]/book/__tests__/route.test.ts | 9 + app/api/supplier-invoices/[id]/book/route.ts | 9 + .../bookkeeping/JournalEntryAttachments.tsx | 59 +++- messages/en.json | 4 + messages/sv.json | 4 + ..._floating_supplier_invoice_documents_2.sql | 76 +++++ ...24151000_detach_underlag_duplicate_rpc.sql | 210 ++++++++++++++ tests/pg/detach-underlag-duplicate.pg.test.ts | 260 ++++++++++++++++++ 11 files changed, 804 insertions(+), 1 deletion(-) create mode 100644 app/api/documents/[id]/detach/__tests__/route.test.ts create mode 100644 app/api/documents/[id]/detach/route.ts create mode 100644 supabase/migrations/20260824150000_reanchor_floating_supplier_invoice_documents_2.sql create mode 100644 supabase/migrations/20260824151000_detach_underlag_duplicate_rpc.sql create mode 100644 tests/pg/detach-underlag-duplicate.pg.test.ts diff --git a/DECISIONS.md b/DECISIONS.md index f3ec41aa..b8026751 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1188,4 +1188,5 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-08-24] Declared currency/voucher_series nullable in three MCP listing schemas on column-nullability alone (no traced null producer): loosening an output schema can only stop false validation failures, never cause one, and legacy rows predate the columns' defaults. Declined (for now) a full Ajv execute-vs-schema round-trip harness in output-schema.test.ts: right long-term answer to this bug class, but a session-sized project of its own; the audit's seven confirmed sites are pinned by a targeted declaration test instead. [2026-08-24] Manual matching (PR 6b) ships N:1 only (many outside rows -> one verifikat): bank links are independent per transaction (the engine allows it by design), skattekonto groups are all-or-nothing with the sum settling the verifikat (one guarded UPDATE, partial hit rolled back). 1:M (one row over several verifikat) and residual booking wait for a link table in 6c: the single journal_entry_id pointer on both row kinds cannot express them, and faking it (pointing the row at the residual verifikat) would break the bridge. The worksheet therefore enables Koppla only when the selection nets to zero and says so otherwise. [2026-08-24] Skattekonto payment file gets pain.001 through the supplier-payment generator (generateSupplierPain001), not the salary pain001 generator: the payment is a plain BG+OCR giro transfer (no SALA CtgyPurp), and the supplier dialect is the Validex-validated shape for exactly that; the LB path stays the default so nothing changes for banks still on LB. +[2026-08-24] Detach-duplicate underlag ships as a SECURITY DEFINER RPC (detach_underlag_duplicate) instead of loosening the document triggers: the WORM guards stay intact for every other path, the carve-out is transaction-local (gnubok.allow_delete) and audit-logged first, and detach is refused unless another anchored underlag remains on the verifikat (BFL 5 kap 7 par) AND a remaining sibling has an identical sha256_hash (only byte-identical duplicates detach; skeptic-hardened 2026-08-24, along with an enforced posted-status guard and company_id on the audit row). Pinned docs (transactions.document_id / supplier_invoices.document_id) stay replace-only. [2026-08-24] Single-call chat console (general.help, AskConsole → /api/agent/ask) now carries the thread's earlier turns into every model call, via a new optional `history` on the provider-agnostic GenerateTextRequest (real message turns before the prompt in BOTH adapters: Anthropic-family messages array, OpenAI-compatible via AI SDK `messages`; an absent/empty history leaves the request byte-identical to the single-turn call, so hosted extraction and every other caller are untouched). The 08-20 RIP-3 cutover made each turn stateless (conversationId was only the tool actor id), so a follow-up in a resumed thread was answered blind (user report: "frågar vad jag refererar till"). History is loaded server-side from agent_messages (loadChatHistory: text only, hidden + tool rows dropped, alternation repaired, newest 16 rows / 10k chars) rather than sent by the client, so the client cannot forge earlier turns and old streaming threads replay cleanly. Rejected: inlining a transcript into the prompt (works everywhere but weaker turn semantics and blurs data vs instructions) and loading history in AskConsole (client-trusted history). Separately: the docked assistant panel now remembers its open thread per tab in sessionStorage (lib/agent-panel/session-restore) and reopens it after a full reload (the deploy prompt's "Ladda om" wiped it); sessionStorage, not user_preferences, because this is this-tab-this-session state that must not follow the user to other devices or tabs. And DeployReloadPrompt's full-width wrapper gets pointer-events-none: at z-[60] after the panel in DOM order it swallowed clicks on the panel's composer ("går ej att skriva"). diff --git a/app/api/documents/[id]/detach/__tests__/route.test.ts b/app/api/documents/[id]/detach/__tests__/route.test.ts new file mode 100644 index 00000000..48a854ff --- /dev/null +++ b/app/api/documents/[id]/detach/__tests__/route.test.ts @@ -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', + }) + }) +}) diff --git a/app/api/documents/[id]/detach/route.ts b/app/api/documents/[id]/detach/route.ts new file mode 100644 index 00000000..acafc390 --- /dev/null +++ b/app/api/documents/[id]/detach/route.ts @@ -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 }, +) diff --git a/app/api/supplier-invoices/[id]/book/__tests__/route.test.ts b/app/api/supplier-invoices/[id]/book/__tests__/route.test.ts index 113831c3..25d4cb35 100644 --- a/app/api/supplier-invoices/[id]/book/__tests__/route.test.ts +++ b/app/api/supplier-invoices/[id]/book/__tests__/route.test.ts @@ -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 () => { diff --git a/app/api/supplier-invoices/[id]/book/route.ts b/app/api/supplier-invoices/[id]/book/route.ts index c33f4b08..0447c73b 100644 --- a/app/api/supplier-invoices/[id]/book/route.ts +++ b/app/api/supplier-invoices/[id]/book/route.ts @@ -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 diff --git a/components/bookkeeping/JournalEntryAttachments.tsx b/components/bookkeeping/JournalEntryAttachments.tsx index 284cdefe..133a9a33 100644 --- a/components/bookkeeping/JournalEntryAttachments.tsx +++ b/components/bookkeeping/JournalEntryAttachments.tsx @@ -40,6 +40,7 @@ interface DocumentRecord { mime_type: string | null storage_path: string created_at: string + sha256_hash?: string | null download_url?: string referenced?: boolean } @@ -97,6 +98,7 @@ export default function JournalEntryAttachments({ // so the original stays in the version chain. const [blockedDoc, setBlockedDoc] = useState(null) const [replacingDocId, setReplacingDocId] = useState(null) + const [detachingDocId, setDetachingDocId] = useState(null) const replaceFileInputRef = useRef(null) const replaceTargetIdRef = useRef(null) @@ -214,6 +216,39 @@ export default function JournalEntryAttachments({ setBlockedDoc(doc) } + // A duplicate may be detached (not deleted) only when another directly + // anchored doc with the SAME content hash remains on the verifikat: the + // detach_underlag_duplicate RPC enforces sha256 equality, so the button is + // gated on the same condition. Referenced docs (via a supplier invoice) + // don't count: they are not anchored to this entry. + const hasDuplicateSibling = (doc: DocumentRecord) => + Boolean(doc.sha256_hash) && + documents.some( + (d) => !d.referenced && d.id !== doc.id && d.sha256_hash === doc.sha256_hash, + ) + + const handleDetach = async (doc: DocumentRecord) => { + setDetachingDocId(doc.id) + try { + const res = await fetch(`/api/documents/${doc.id}/detach`, { method: 'POST' }) + if (!res.ok) { + const { error } = await res.json().catch(() => ({ error: undefined })) + toast({ + title: t('detach_failed'), + description: typeof error === 'string' ? error : undefined, + variant: 'destructive', + }) + } else { + await fetchDocuments() + setBlockedDoc(null) + } + } catch { + toast({ title: t('detach_failed'), variant: 'destructive' }) + } finally { + setDetachingDocId(null) + } + } + const handleOpenReplacePicker = (docId: string) => { replaceTargetIdRef.current = docId replaceFileInputRef.current?.click() @@ -460,7 +495,11 @@ export default function JournalEntryAttachments({
-

{t('remove_blocked_hint')}

+

+ {blockedDoc !== null && hasDuplicateSibling(blockedDoc) + ? t('detach_hint') + : t('remove_blocked_hint')} +

@@ -468,6 +507,24 @@ export default function JournalEntryAttachments({ + {blockedDoc !== null && hasDuplicateSibling(blockedDoc) && ( + + )}