From 954ce873a8a8f2ebb3a7535c0e8676fa9ae0e1dc Mon Sep 17 00:00:00 2001 From: Jakob Wennberg <149234542+jakobwennberg@users.noreply.github.com> Date: Wed, 29 Jul 2026 19:58:47 +0200 Subject: [PATCH] fix(transactions): confirm and allow undo on a counterparty booking (#1292) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Booking via "Tidigare motparter" ended on a bare setExitingIds().add(id): the row animated out of the inbox and that was the entire feedback. No "Bokförd" toast, no Ångra, no unbooked-count decrement, and the id was never removed from exitingIds again, so an undo would have restored the row's data while leaving it filtered out of the list. Extracted runCategorize's success tail into one finishBooking() rather than copying the toast into the counterparty branch: the counterparty path was already a second, thinner implementation of it, which is why it drifted. Both paths and the counterparty activate-and-retry go through it now, pinned by a parity test. Also fixed on the way: - handleTransactionBooked (manual booking dialog / voucher match) never decremented the unbooked count, so the header read one too high until the next refetch. It deliberately gets no Ångra: its `matched` branch links the transaction to a PRE-EXISTING verifikat, and /uncategorize storno-reverses whatever journal_entry_id the transaction points at. - The 350ms animation timer re-applied the booked shape unconditionally, so an Ångra resolving inside that window left the client claiming a journal_entry_id the server had already storno-reversed. A completed undo now wins. - finishBooking cleared processingId unconditionally; scoped to the finished id so it cannot wipe an unrelated row's spinner. No migrations. --- DECISIONS.md | 2 + app/(dashboard)/transactions/page.tsx | 198 +++++++++++------- .../__tests__/booking-feedback-parity.test.ts | 98 +++++++++ 3 files changed, 227 insertions(+), 71 deletions(-) create mode 100644 components/transactions/__tests__/booking-feedback-parity.test.ts diff --git a/DECISIONS.md b/DECISIONS.md index 1cad3e3e..a87154f5 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -678,3 +678,5 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-07-29] TicWorkspace ignores a cached profile blob with no `statuses` key rather than normalising it and rendering. Prod check: 17 of 17 `extension_data` rows for general/tic + company_profile predate the TIC v2 upgrade (#584) and have no statuses/signatory/board/representatives/payrolls, i.e. the workspace is in the error boundary for every company that ever opened it. Normalising alone would fix the crash but leave those sections permanently blank, because the success render path has no refresh button, and the auto-fetch effect is gated on `!profile`. Dropping the stale blob lets that effect refetch the current shape and re-save it, so the 17 rows self-heal on next open. [2026-07-29] Center Node AB support deletion (Anders Orback) run manually service-side, bypassing the product's AAL2/consent gate on anonymize_user_account: his written support request is the consent (documented in the audit_log row 8501e9e0), and support had already replied "du behöver inte göra något mer", so the fix-the-button-and-let-him-click alternative would have contradicted a sent mail. Manual run mirrored the delete route + RPC body byte-for-byte; GoTrue admin logout endpoint 404s on our GoTrue version, so global signout was done by deleting auth.sessions rows (equivalent effect, ban blocks refresh regardless). + +[2026-07-29] Booking-feedback parity: extracted runCategorize's success tail into one finishBooking() rather than copying the toast into the counterparty branch. The counterparty path was already a second, thinner implementation of the same tail (the reason it silently lacked confirmation, undo and the count decrement), so a third copy was the wrong shape. Also caught while wiring the parity test: handleTransactionBooked (manual booking dialog / voucher match) never decremented totalUncategorizedCount either, so the header count stayed one high until the next refetch; fixed. Deliberately NOT given an Ångra action: its `matched` branch links the transaction to a PRE-EXISTING verifikat, and /uncategorize storno-reverses whatever journal_entry_id the transaction points at, so an undo there would reverse a voucher the user never created in that flow. diff --git a/app/(dashboard)/transactions/page.tsx b/app/(dashboard)/transactions/page.tsx index 0212b00a..7e38c0f5 100644 --- a/app/(dashboard)/transactions/page.tsx +++ b/app/(dashboard)/transactions/page.tsx @@ -918,6 +918,104 @@ export default function TransactionsPage() { return runCategorize({ id, isBusiness, category, vatTreatment, accountOverride, templateId, inboxItemId, dimensions, confirmNoMatch: false }) } + /** + * The shared tail of a successful booking: exit animation, unbooked-count + * decrement, the outcome toast with its Ångra action, and the state patch + * that lands the row in its booked shape. + * + * Both booking paths run this. The counterparty-template path used to do + * only `setExitingIds().add(id)`, so a successful booking gave no + * confirmation, no undo and no count decrement, and the id was never removed + * from exitingIds again: an undo would have restored the row's data while + * leaving it filtered out of the inbox. + */ + function finishBooking(args: { + id: string + isBusiness: boolean + category?: TransactionCategory + journalEntryId?: string | null + journalEntryCreated?: boolean + journalEntryError?: string | null + }) { + const { id, isBusiness, category, journalEntryId, journalEntryCreated, journalEntryError } = args + // A completed Ångra must win over the delayed patch below. The undo has + // already storno-reversed the verifikat server-side, so re-applying the + // booked shape afterwards would show a journal_entry_id that no longer + // represents a live entry. + let undone = false + + setExitingIds((prev) => new Set(prev).add(id)) + setTotalUncategorizedCount((prev) => Math.max(0, (prev ?? 1) - 1)) + + if (journalEntryCreated) { + toast({ + title: 'Bokförd', + action: ( + { + try { + const undoRes = await fetch(`/api/transactions/${id}/uncategorize`, { method: 'POST' }) + if (undoRes.ok) { + undone = true + setTransactions((prev) => + prev.map((t) => + t.id === id + ? { ...t, is_business: null, category: null as unknown as TransactionCategory, journal_entry_id: null } + : t + ) + ) + setTotalUncategorizedCount((prev) => (prev ?? 0) + 1) + toast({ title: t('undone_title'), description: t('undone_description') }) + } else { + const errData = await undoRes.json() + toast({ + title: 'Kunde inte ångra', + description: getErrorMessage(errData, { context: 'transaction', statusCode: undoRes.status }), + variant: 'destructive', + }) + } + } catch { + toast({ title: t('undo_failed_title'), description: t('undo_failed_description'), variant: 'destructive' }) + } + }}> + Ångra + + ), + }) + } else if (journalEntryError) { + toast({ title: 'Delvis bokförd', description: `Verifikation kunde inte skapas: ${journalEntryError}`, variant: 'destructive' }) + } else { + toast({ title: t('partially_booked_title'), description: t('partially_booked_description') }) + } + + // Update transaction in state after a brief delay for animation. Clearing + // the id from exitingIds is what makes an Ångra later put the row back in + // the inbox instead of leaving it invisible. + setTimeout(() => { + if (!undone) { + setTransactions((prev) => + prev.map((tx) => + tx.id === id + ? { + ...tx, + is_business: isBusiness, + ...(category ? { category } : {}), + ...(journalEntryId ? { journal_entry_id: journalEntryId } : {}), + } + : tx + ) + ) + } + setExitingIds((prev) => { + const next = new Set(prev) + next.delete(id) + return next + }) + // Only this row's spinner: the shared helper must not clear another + // transaction's in-flight state. + setProcessingId((prev) => (prev === id ? null : prev)) + }, 350) + } + async function runCategorize(args: { id: string isBusiness: boolean @@ -1151,66 +1249,14 @@ export default function TransactionsPage() { return null } - // Mark as exiting for animation, then update state - setExitingIds((prev) => new Set(prev).add(id)) - setTotalUncategorizedCount((prev) => Math.max(0, (prev ?? 1) - 1)) - - if (result.journal_entry_created) { - toast({ - title: 'Bokförd', - action: ( - { - try { - const undoRes = await fetch(`/api/transactions/${id}/uncategorize`, { method: 'POST' }) - if (undoRes.ok) { - setTransactions((prev) => - prev.map((t) => - t.id === id - ? { ...t, is_business: null, category: null as unknown as TransactionCategory, journal_entry_id: null } - : t - ) - ) - setTotalUncategorizedCount((prev) => (prev ?? 0) + 1) - toast({ title: t('undone_title'), description: t('undone_description') }) - } else { - const errData = await undoRes.json() - toast({ - title: 'Kunde inte ångra', - description: getErrorMessage(errData, { context: 'transaction', statusCode: undoRes.status }), - variant: 'destructive', - }) - } - } catch { - toast({ title: t('undo_failed_title'), description: t('undo_failed_description'), variant: 'destructive' }) - } - }}> - Ångra - - ), - }) - } else if (result.journal_entry_error) { - toast({ title: 'Delvis bokförd', description: `Verifikation kunde inte skapas: ${result.journal_entry_error}`, variant: 'destructive' }) - } else { - toast({ title: t('partially_booked_title'), description: t('partially_booked_description') }) - } - - // Update transaction in state after a brief delay for animation - setExitingIds((prev) => new Set(prev).add(id)) - setTimeout(() => { - setTransactions((prev) => - prev.map((t) => - t.id === id - ? { ...t, is_business: isBusiness, category: result.category, journal_entry_id: result.journal_entry_id } - : t - ) - ) - setExitingIds((prev) => { - const next = new Set(prev) - next.delete(id) - return next - }) - setProcessingId(null) - }, 350) + finishBooking({ + id, + isBusiness, + category: result.category, + journalEntryId: result.journal_entry_id, + journalEntryCreated: result.journal_entry_created, + journalEntryError: result.journal_entry_error, + }) return result.journal_entry_id || null } catch { @@ -1901,6 +1947,10 @@ export default function TransactionsPage() { matched?: boolean, ) { setExitingIds((prev) => new Set(prev).add(transactionId)) + // The row leaves the unbooked inbox either way (booked, or linked to an + // existing voucher), so the header count has to follow: every other path + // that removes a row decrements, this one did not. + setTotalUncategorizedCount((prev) => Math.max(0, (prev ?? 1) - 1)) setTimeout(() => { setTransactions((prev) => prev.map((t) => @@ -2246,7 +2296,7 @@ export default function TransactionsPage() { let journalEntryId: string | null if (!templateId && quickReview?.template?.id && isCounterpartyTemplateId(quickReview.template.id)) { const cpTemplateId = extractCounterpartyId(quickReview.template.id) - const cpCategorize = async (): Promise<{ ok: boolean; journalEntryId: string | null; result: { error?: { code?: string; account_numbers?: string[]; details?: { account_numbers?: string[] } }; journal_entry_id?: string | null }; status: number }> => { + const cpCategorize = async (): Promise<{ ok: boolean; journalEntryId: string | null; result: { error?: { code?: string; account_numbers?: string[]; details?: { account_numbers?: string[] } }; journal_entry_id?: string | null; journal_entry_created?: boolean; journal_entry_error?: string | null; category?: TransactionCategory }; status: number }> => { const r = await fetch(`/api/transactions/${id}/categorize`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, @@ -2303,15 +2353,14 @@ export default function TransactionsPage() { // update conditionally writes the journal_entry_id when it's // actually present. if (retry.ok) { - setExitingIds((prev) => new Set(prev).add(id)) - setTransactions((prev) => - prev.map((t) => - t.id === id - ? { ...t, is_business: true, ...(retry.journalEntryId ? { journal_entry_id: retry.journalEntryId } : {}) } - : t - ) - ) - toast({ title: 'Bokförd' }) + finishBooking({ + id, + isBusiness: true, + category: retry.result?.category, + journalEntryId: retry.journalEntryId, + journalEntryCreated: retry.result?.journal_entry_created, + journalEntryError: retry.result?.journal_entry_error, + }) } else { toast({ title: 'Kategorisering misslyckades', description: getErrorMessage(retry.result, { context: 'transaction', statusCode: retry.status }), variant: 'destructive' }) } @@ -2332,7 +2381,14 @@ export default function TransactionsPage() { setQuickReview(null) return null } - setExitingIds((prev) => new Set(prev).add(id)) + finishBooking({ + id, + isBusiness: true, + category: result?.category, + journalEntryId: cpJeId, + journalEntryCreated: result?.journal_entry_created, + journalEntryError: result?.journal_entry_error, + }) journalEntryId = cpJeId } else { journalEntryId = await handleCategorize(id, true, category, vatTreatment, accountOverride, templateId, undefined, dimensions) diff --git a/components/transactions/__tests__/booking-feedback-parity.test.ts b/components/transactions/__tests__/booking-feedback-parity.test.ts new file mode 100644 index 00000000..8dea3eef --- /dev/null +++ b/components/transactions/__tests__/booking-feedback-parity.test.ts @@ -0,0 +1,98 @@ +import { describe, it, expect } from 'vitest' +import fs from 'node:fs' +import path from 'node:path' + +/** + * Booking-feedback parity between the two booking paths on the transactions + * page. + * + * The counterparty-template path (Bokför → "Tidigare motparter") used to end a + * successful booking with nothing but `setExitingIds().add(id)`: no "Bokförd" + * toast, no Ångra action, no unbooked-count decrement, and the id was never + * removed from exitingIds again, so an undo would have restored the row's data + * while leaving it filtered out of the inbox. Every other booking path ran + * runCategorize's success tail. + * + * Both now go through one `finishBooking`. This repo runs Vitest in the `node` + * environment and never renders components, so, like the sibling + * invoice-match-dialog tests, these are file-level assertions: the two paths + * must not drift apart again. + */ + +const PAGE_SRC = fs.readFileSync( + path.resolve(__dirname, '../../../app/(dashboard)/transactions/page.tsx'), + 'utf8', +) + +const readMessages = (locale: 'sv' | 'en', namespace: string) => + ( + JSON.parse( + fs.readFileSync(path.resolve(__dirname, `../../../messages/${locale}.json`), 'utf8'), + ) as Record> + )[namespace] + +describe('transactions page booking feedback', () => { + it('defines exactly one success tail', () => { + expect(PAGE_SRC).toContain('function finishBooking(') + // One undo implementation, and one place that calls the storno endpoint. + expect(PAGE_SRC.match(/altText="Ångra kategorisering"/g) ?? []).toHaveLength(1) + expect(PAGE_SRC.match(/uncategorize`, \{ method: 'POST' \}/g) ?? []).toHaveLength(1) + }) + + it('routes every successful booking through it', () => { + // runCategorize (category / catalog template / library template), the + // counterparty-template booking, and the counterparty activate-and-retry. + expect(PAGE_SRC.match(/finishBooking\(\{/g) ?? []).toHaveLength(3) + }) + + it('no longer ends the counterparty path on a bare exitingIds add', () => { + // The old tail: setExitingIds(...) immediately followed by `journalEntryId = cpJeId`. + expect(PAGE_SRC).not.toMatch( + /setExitingIds\(\(prev\) => new Set\(prev\)\.add\(id\)\)\s*\n\s*journalEntryId = cpJeId/, + ) + }) + + it('clears the id from exitingIds so an undo puts the row back', () => { + // Without the delete, an undone booking restores is_business: null but the + // row stays filtered out of the inbox (see uncategorizedTransactions). + expect(PAGE_SRC).toMatch(/next\.delete\(id\)/) + }) + + it('lets a completed undo win over the delayed booked-state patch', () => { + // The 350ms animation timer must not re-apply journal_entry_id after an + // Ångra has already storno-reversed the verifikat server-side. + expect(PAGE_SRC).toMatch(/let undone = false/) + expect(PAGE_SRC).toMatch(/undone = true/) + expect(PAGE_SRC).toMatch(/if \(!undone\) \{/) + }) + + it('clears only the finished row\'s spinner', () => { + // The shared tail runs for rows that never set processingId; an + // unconditional clear would wipe an unrelated in-flight row. + expect(PAGE_SRC).toMatch(/setProcessingId\(\(prev\) => \(prev === id \? null : prev\)\)/) + }) + + it('decrements the unbooked count on every path that removes a row', () => { + // finishBooking, handleTransactionBooked (manual booking dialog / voucher + // match), and the three other single-row exits already on the page. + expect( + PAGE_SRC.match(/setTotalUncategorizedCount\(\(prev\) => Math\.max\(0, \(prev \?\? 1\) - 1\)\)/g) ?? [], + ).toHaveLength(5) + }) + + it('ships the undo strings it renders in both locales', () => { + for (const locale of ['sv', 'en'] as const) { + const messages = readMessages(locale, 'transactions') + for (const key of [ + 'undone_title', + 'undone_description', + 'undo_failed_title', + 'undo_failed_description', + 'partially_booked_title', + 'partially_booked_description', + ]) { + expect(messages[key], `${locale}.transactions.${key}`).toBeTruthy() + } + } + }) +})