* feat(transactions): "Ta bort underlag" detach action on a transaction (#2132) Wrong receipt pinned, no way back: the DELETE /api/transactions/[id]/attach-document route and its tests already existed, but nothing in the UI called it. This wires it up, frontend only. - Inbox card and history list: "Ta bort underlag" in the row menu, shown only for writers on unbooked rows that carry a pin (canDetachDocument helper). - Attach dialog: a small "Ta bort underlag" link beside the already-attached hint, the one place the app previously admitted a doc was pinned. - Page: handleDetachDocument confirms (useDestructiveConfirm, warning), then DELETEs; 200 clears document_id in local state (list, dialog snapshot, and the inbox card's optimistic override via a -unlinked window event) and toasts; 409 renders the route's Swedish BFL message verbatim; other errors map through get-error-message. - Strings under tx_detach in sv.json and en.json. - Tests: gate hidden when booked / read-only / no pin / no handler; 409 rendered unchanged; wiring and locale assertions. Out of scope, follow-up: MCP detach tool (new pending-op type + CHECK migration), detaching from the inbox for non-email docs, and clearing invoice_inbox_items.matched_transaction_id on detach so the doc is offered again by inbox-available. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YRXN5CqHrfuuDw5LLcSgTg * fix(transactions): clear the inbox back-link when detaching underlag (#2132) Skeptic finding on PR #2144: DELETE attach-document nulled only transactions.document_id and left invoice_inbox_items.matched_transaction_id pointing at the transaction. propagateUnderlagForBookedTransaction selects on exactly that column at categorize / book / bulk-book time, so the detached receipt would have been re-anchored onto the new verifikation as immutable underlag (BFL 5 kap 7 §), and the doc never reappeared in inbox-available for re-matching. The route now clears the back-link for the detached document, scoped to items not yet consumed by a verifikat (created_journal_entry_id null), mirroring the invoice-inbox extension's unmatch. Best-effort like the POST side: the pin removal is the primary effect. Three DELETE tests cover the filters, the no-pin case, and a failing unlink. DECISIONS.md and the PR body record the accepted bulk-booked-row limitation in the history list. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YRXN5CqHrfuuDw5LLcSgTg * fix(transactions): detach reports a failed inbox unlink instead of success (#2132) Swedish compliance review on PR #2144: the inbox back-link cleanup was fire-and-forget, so a failed UPDATE returned 200 while leaving exactly the stale matched_transaction_id that re-anchors a detached document onto the next verifikation (BFL 5 kap 6-7 §). The unlink is now scoped by transaction only (the unique index on matched_transaction_id means at most one item points here, and a stale item from the replace path would re-anchor just the same), runs even when nothing was pinned so a retry is idempotent, and a failure answers 500 with an honest Swedish partial-failure message, mirroring the POST side's propagation failure. Tests updated accordingly. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YRXN5CqHrfuuDw5LLcSgTg * fix(transactions): release inbox back-link before a compare-and-set pin clear (#2132) Review findings on PR #2144, one pass: - CodeRabbit (major): DELETE cleared the pin and then released the inbox back-link scoped by transaction, so a POST landing in between could end up as "new doc pinned, its inbox item unlinked". The release now runs FIRST, and the pin clear is a compare-and-set on the document that was read (.eq document_id, or .is null when nothing was pinned). Zero rows answers 409 "ändrades samtidigt" and keeps the newer pin. A failed release returns 500 before anything changed, so a retry is trivially idempotent. - Compliance swarm (A.8.15): the unlink failure log carried the raw driver error; it now logs errorCauseTag() only. - CodeRabbit docstring check: JSDoc on handleDetachDocument. Tests: order of the two writes, CAS filters for both pinned and empty states, 409 on concurrent re-attach, coded-cause logging. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YRXN5CqHrfuuDw5LLcSgTg --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Mattsson
co-authored by
Claude Fable 5.1
Claude Fable 5.1
parent
4f33184a9a
commit
4c76fb10d7
@@ -6,7 +6,7 @@ import {
|
||||
createQueuedMockSupabase,
|
||||
} from '@/tests/helpers'
|
||||
|
||||
const { supabase: mockSupabase, enqueue, reset } = createQueuedMockSupabase()
|
||||
const { supabase: mockSupabase, enqueue, reset, findCalls } = createQueuedMockSupabase()
|
||||
|
||||
const requireAuthMock = vi.fn()
|
||||
vi.mock('@/lib/auth/require-auth', () => ({
|
||||
@@ -324,12 +324,16 @@ describe('DELETE /api/transactions/[id]/attach-document', () => {
|
||||
const { status, body } = await parseJsonResponse<{ error: string }>(res)
|
||||
expect(status).toBe(409)
|
||||
expect(body.error).toContain('verifikation')
|
||||
// Nothing is written on the immutability path.
|
||||
expect(findCalls('invoice_inbox_items', 'update')).toEqual([])
|
||||
expect(findCalls('transactions', 'update')).toEqual([])
|
||||
})
|
||||
|
||||
it('clears document_id when no journal entry link', async () => {
|
||||
enqueue({ data: { id: 'tx-1', document_id: 'doc-1' }, error: null }) // tx fetch
|
||||
enqueue({ data: { journal_entry_id: null }, error: null }) // doc fetch
|
||||
enqueue({ data: null, error: null }) // update
|
||||
enqueue({ data: null, error: null }) // inbox unlink
|
||||
enqueue({ data: { id: 'tx-1' }, error: null }) // pin CAS (RETURNING id)
|
||||
const res = await DELETE(makeReq(null, 'DELETE'), createMockRouteParams({ id: 'tx-1' }))
|
||||
const { status, body } = await parseJsonResponse<{ data: { document_id: string | null } }>(res)
|
||||
expect(status).toBe(200)
|
||||
@@ -338,9 +342,78 @@ describe('DELETE /api/transactions/[id]/attach-document', () => {
|
||||
|
||||
it('clears document_id when no doc was attached', async () => {
|
||||
enqueue({ data: { id: 'tx-1', document_id: null }, error: null }) // tx fetch
|
||||
enqueue({ data: null, error: null }) // update
|
||||
enqueue({ data: null, error: null }) // inbox unlink
|
||||
enqueue({ data: { id: 'tx-1' }, error: null }) // pin CAS
|
||||
const res = await DELETE(makeReq(null, 'DELETE'), createMockRouteParams({ id: 'tx-1' }))
|
||||
const { status } = await parseJsonResponse(res)
|
||||
expect(status).toBe(200)
|
||||
// The back-link is released even with nothing pinned (a stale item from
|
||||
// the replace path would re-anchor just the same), and the CAS then
|
||||
// requires the pin to still be empty.
|
||||
expect(findCalls('invoice_inbox_items', 'update')).toEqual([[{ matched_transaction_id: null }]])
|
||||
expect(findCalls('transactions', 'is')).toContainEqual(['document_id', null])
|
||||
})
|
||||
|
||||
it('releases the inbox back-link BEFORE the pin, scoped by transaction (else booking re-anchors it)', async () => {
|
||||
// propagateUnderlagForBookedTransaction selects inbox items by
|
||||
// matched_transaction_id at categorize time; a stale back-link would pin
|
||||
// the rejected receipt onto the new verifikation as immutable underlag.
|
||||
enqueue({ data: { id: 'tx-1', document_id: 'doc-1' }, error: null }) // tx fetch
|
||||
enqueue({ data: { journal_entry_id: null }, error: null }) // doc fetch
|
||||
enqueue({ data: null, error: null }) // inbox unlink
|
||||
enqueue({ data: { id: 'tx-1' }, error: null }) // pin CAS
|
||||
const res = await DELETE(makeReq(null, 'DELETE'), createMockRouteParams({ id: 'tx-1' }))
|
||||
const { status } = await parseJsonResponse(res)
|
||||
expect(status).toBe(200)
|
||||
expect(findCalls('invoice_inbox_items', 'update')).toEqual([[{ matched_transaction_id: null }]])
|
||||
const eqArgs = findCalls('invoice_inbox_items', 'eq')
|
||||
// Scoped by transaction (unique index: at most one item points here), not
|
||||
// by the pinned doc, so a stale item from the replace path is cleared too.
|
||||
expect(eqArgs).toContainEqual(['matched_transaction_id', 'tx-1'])
|
||||
expect(eqArgs).toContainEqual(['company_id', 'company-1'])
|
||||
expect(eqArgs).not.toContainEqual(['document_id', 'doc-1'])
|
||||
// Items already consumed by a verifikat are left alone.
|
||||
expect(findCalls('invoice_inbox_items', 'is')).toContainEqual(['created_journal_entry_id', null])
|
||||
// Order: the unlink table is touched before the pin update.
|
||||
const tables = mockSupabase.from.mock.calls.map((c) => c[0])
|
||||
expect(tables.indexOf('invoice_inbox_items')).toBeLessThan(tables.lastIndexOf('transactions'))
|
||||
// Compare-and-set: the pin is cleared only if it is still doc-1.
|
||||
expect(findCalls('transactions', 'eq')).toContainEqual(['document_id', 'doc-1'])
|
||||
expect(findCalls('transactions', 'update')).toEqual([[{ document_id: null }]])
|
||||
})
|
||||
|
||||
it('refuses with 409 when the pin changed under us (concurrent re-attach)', async () => {
|
||||
// Interleaving: we read doc-1, a POST pins doc-2 (and links its inbox
|
||||
// item) before our CAS runs. Zero rows come back: the new pin is kept and
|
||||
// the caller is told nothing happened, instead of a success for a state
|
||||
// that is now "doc-2 pinned, doc-2 inbox item unlinked".
|
||||
enqueue({ data: { id: 'tx-1', document_id: 'doc-1' }, error: null }) // tx fetch
|
||||
enqueue({ data: { journal_entry_id: null }, error: null }) // doc fetch
|
||||
enqueue({ data: null, error: null }) // inbox unlink
|
||||
enqueue({ data: null, error: null }) // pin CAS: 0 rows
|
||||
const res = await DELETE(makeReq(null, 'DELETE'), createMockRouteParams({ id: 'tx-1' }))
|
||||
const { status, body } = await parseJsonResponse<{ error: string }>(res)
|
||||
expect(status).toBe(409)
|
||||
expect(body.error).toContain('samtidigt')
|
||||
})
|
||||
|
||||
it('reports a failing inbox unlink as 500 with nothing changed, logging a coded cause only', async () => {
|
||||
// A stale back-link is the exact defect the detach exists to prevent, so
|
||||
// a 200 here would hide a compliance hazard. Because the unlink runs
|
||||
// first, the pin is untouched and a retry is trivially idempotent.
|
||||
enqueue({ data: { id: 'tx-1', document_id: 'doc-1' }, error: null }) // tx fetch
|
||||
enqueue({ data: { journal_entry_id: null }, error: null }) // doc fetch
|
||||
enqueue({ data: null, error: { code: '42501', message: 'rls denied: row values here' } }) // unlink fails
|
||||
const spy = vi.spyOn(console, 'error').mockImplementation(() => {})
|
||||
const res = await DELETE(makeReq(null, 'DELETE'), createMockRouteParams({ id: 'tx-1' }))
|
||||
const { status, body } = await parseJsonResponse<{ error: string }>(res)
|
||||
expect(status).toBe(500)
|
||||
expect(body.error).toContain('fortfarande kopplat')
|
||||
expect(findCalls('transactions', 'update')).toEqual([])
|
||||
// Raw driver messages can quote row values: only the coded cause is logged.
|
||||
expect(spy).toHaveBeenCalledWith('[attach-document] Failed to unlink inbox item:', {
|
||||
cause: '42501',
|
||||
})
|
||||
spy.mockRestore()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -4,6 +4,7 @@ import { withRouteContext } from '@/lib/api/with-route-context'
|
||||
import { validateBody } from '@/lib/api/validate'
|
||||
import { AttachDocumentSchema } from '@/lib/api/schemas'
|
||||
import { appendProcessingHistory } from '@/lib/processing-history/append'
|
||||
import { errorCauseTag } from '@/lib/errors/db-error'
|
||||
import {
|
||||
completeInboxItemsForBookedTransaction,
|
||||
resolveVoucherLinkedEntryIds,
|
||||
@@ -220,6 +221,9 @@ export const POST = withRouteContext<{ params: Promise<{ id: string }> }>(
|
||||
*
|
||||
* Detach a document from a transaction.
|
||||
*
|
||||
* Also undoes the invoice_inbox_items back-link written by POST, so the next
|
||||
* booking does not re-anchor the detached doc (see propagateUnderlag...).
|
||||
*
|
||||
* Blocked once the document has propagated into a journal entry (BFL 5 kap 6 §
|
||||
* räkenskapsinformation immutability): at that point the doc is the
|
||||
* verifikation's underlag and can only be undone by reversing the entry.
|
||||
@@ -258,11 +262,57 @@ export const DELETE = withRouteContext<{ params: Promise<{ id: string }> }>(
|
||||
}
|
||||
}
|
||||
|
||||
const { error: updateError } = await supabase
|
||||
// Step 1: release the inbox back-link the POST path wrote, BEFORE the pin.
|
||||
// Without this the item still says matched_transaction_id = this tx, and
|
||||
// the next categorize / book / bulk-book runs
|
||||
// propagateUnderlagForBookedTransaction, which anchors the DETACHED
|
||||
// document onto the new verifikation as immutable underlag (BFL 5 kap 7 §:
|
||||
// the verifikat would cite a receipt the user rejected). Scoped by
|
||||
// transaction, not by the pinned doc: the unique index on
|
||||
// matched_transaction_id means at most one item points here, and a stale
|
||||
// item whose doc differs from the pin (replace path) would re-anchor just
|
||||
// the same. Items already consumed by a verifikat are left alone.
|
||||
//
|
||||
// Ordering: unlink first, then compare-and-set the pin (step 2). A POST
|
||||
// that lands in between re-pins a new doc and its back-link, and the CAS
|
||||
// below then refuses to clobber it (409), so the two writes cannot end up
|
||||
// as "new doc pinned, its inbox item unlinked". A failed unlink returns
|
||||
// before anything changed, so a retry is trivially idempotent.
|
||||
const { error: inboxUnlinkErr } = await supabase
|
||||
.from('invoice_inbox_items')
|
||||
.update({ matched_transaction_id: null })
|
||||
.eq('company_id', companyId)
|
||||
.eq('matched_transaction_id', transactionId)
|
||||
.is('created_journal_entry_id', null)
|
||||
if (inboxUnlinkErr) {
|
||||
// Not best-effort: a stale back-link is the exact defect this detach
|
||||
// exists to prevent, so a "succeeded" response would hide a compliance
|
||||
// hazard. Coded cause only: raw driver messages can quote row values.
|
||||
console.error('[attach-document] Failed to unlink inbox item:', {
|
||||
cause: errorCauseTag(inboxUnlinkErr),
|
||||
})
|
||||
return NextResponse.json(
|
||||
{
|
||||
error:
|
||||
'Inkorgsposten kunde inte släppas, så underlaget är fortfarande kopplat. Försök igen.',
|
||||
},
|
||||
{ status: 500 },
|
||||
)
|
||||
}
|
||||
|
||||
// Step 2: clear the pin, but only if it is still the doc we read above
|
||||
// (or still empty). A concurrent POST that re-pinned in the meantime wins:
|
||||
// we must not detach a document the user just attached, nor report
|
||||
// success for a detach that did not happen.
|
||||
let clearPin = supabase
|
||||
.from('transactions')
|
||||
.update({ document_id: null })
|
||||
.eq('id', transactionId)
|
||||
.eq('company_id', companyId)
|
||||
clearPin = tx.document_id
|
||||
? clearPin.eq('document_id', tx.document_id)
|
||||
: clearPin.is('document_id', null)
|
||||
const { data: cleared, error: updateError } = await clearPin.select('id').maybeSingle()
|
||||
|
||||
if (updateError) {
|
||||
// The enforce_transactions_document_immutability trigger raises a
|
||||
@@ -284,6 +334,16 @@ export const DELETE = withRouteContext<{ params: Promise<{ id: string }> }>(
|
||||
return NextResponse.json({ error: 'Failed to detach document' }, { status: 500 })
|
||||
}
|
||||
|
||||
if (!cleared) {
|
||||
// Zero rows: the pin changed under us (concurrent attach) or the row
|
||||
// vanished. Either way this detach did not happen; say so instead of
|
||||
// reporting success for someone else's state.
|
||||
return NextResponse.json(
|
||||
{ error: 'Transaktionen ändrades samtidigt. Ladda om sidan och försök igen.' },
|
||||
{ status: 409 },
|
||||
)
|
||||
}
|
||||
|
||||
return NextResponse.json({ data: { transaction_id: transactionId, document_id: null } })
|
||||
},
|
||||
{ requireWrite: true },
|
||||
|
||||
Reference in New Issue
Block a user