From 93541d718608fbb2077a367ded2c38d1de5d7d2f Mon Sep 17 00:00:00 2001 From: Jakob Wennberg Date: Mon, 17 Aug 2026 14:30:25 +0200 Subject: [PATCH] fix(ux): smoothness follow-ups - detail pages, batches, toasts, and the last edges (#1633) * fix(ux): smoothness follow-ups - detail pages, batches, toasts, and the last edges Follow-up batch to #1629: the six documented deferred items from dev_docs/loading_states_analysis.md, in the same vocabulary (first-load-only takeovers, background reconcile behind mounted content, row/button-level pending, sequence guards). - Invoice detail pages: kundfaktura and leverantorsfaktura detail no longer blank the whole page for one-field changes. fetchInvoice shows the blocking spinner/skeleton only before the first paint (or when the pager steps to a different invoice); Bokfor / status / finalize / payment / send / Attestera / Markera betald / kreditera refetch behind the mounted page, the acting button shows a spinner-in-button, and the handlers await the refetch so pending covers until the content reflects the new state. The supplier detail's single isProcessing boolean became processingAction so the spinner lands on the clicked button only. (The leverantorsfakturor LIST try/catch/res.ok item was already fixed by #1629.) - useDestructiveConfirm: confirm(opts, action?) can now carry the destructive operation, so the dialog's existing isLoading spinner actually shows while it runs, dismissal is blocked meanwhile, and confirm resolves false if the action throws. Adopted at the /transactions row delete and the supplier- invoice detail delete (which previously permitted duplicate DELETEs with zero feedback). - Batch parallelization: new lib/concurrency.ts mapWithConcurrency (bounded worker pool, order-preserving, tested). /transactions batch categorize / ignore / delete run per-row requests 5 at a time instead of strictly sequentially; the bulkbar counter ticks per completed row. - Toast-spam reduction: batch categorize rows run silent (exit animation, count decrement and state patch stay; no per-row Bokford or generic failure toast) and ONE aggregate toast reports "N bokforda[, M misslyckades]" with a single Angra alla action that pools the same /uncategorize endpoint over every booked row (per-row undo is feasible today, so the aggregate is too). Interactive escalations (SI/CI match suggestions, duplicate warning, activate-account) deliberately keep their dialogs. - Underlag row-click flash: InvoiceInboxWorkspace handleSelect seeds the detail pane synchronously from the clicked list row and starts the document load in parallel with the detail GET (which hydrates on arrival), so a row click never flashes the onboarding/empty state, and a stale-response guard keeps a slow fetch from overwriting a newer selection. - #1629 round-2 edges: /pending holds the loading state when a fetch for a not-yet-loaded tab FAILS (never renders the previous tab's rows under the new tab's header, and never fakes an empty state); /transactions clears transactions/skvRows (+ count/paging) and bumps both fetch sequences on company switch, and loadSkvRows got the same sequence-guard pattern as fetchTransactions. Gates: full vitest suite green (14772 passed), tsc byte-identical to the origin/main baseline (stash-diffed), eslint 0 errors on touched files (warnings identical to baseline), check:guards green, package-lock untouched. Co-Authored-By: Claude Fable 5 * fix(ui): harden action feedback against stale responses and failures Address the seven CodeRabbit findings on #1633: - invoices/[id] + supplier-invoices/[id]: latest-request guard in fetchInvoice (sequence token) so a mutation refresh overlapping pager navigation can never commit invoice A's state under invoice B's URL; the deferred related-document writes are guarded too - supplier-invoices/[id]: try/catch/finally in approve/book/mark-paid/ credit/uncredit so a rejected fetch()/json() clears processingAction instead of leaving every invoice action disabled until reload - transactions: extend the skattekonto sequence guard to the connection-status write so a status response started under the previous company cannot flip the reconnect banner for the new one - transactions: runCategorize resolves { ok, journalEntryId } so the batch aggregate counts a 200-with-null-journal-entry booking (flag flip) as success instead of narrating it as misslyckades; Angra alla only targets rows with an actual verifikat, since the storno endpoint rejects rows without one - transactions: shared undoneIdsRef lets "Angra alla" cancel a pending finishBooking state patch; a fresh booking clears its row's entry so re-booked rows still get their delayed patch - InvoiceInboxWorkspace: monotonic request tokens for the detail and document reads so a same-item reload cannot resolve out of order and paint a stale snapshot or document URL - messages: ICU plural for the success part of both partial batch descriptions in sv and en (1 bokford, not 1 bokforda) Co-Authored-By: Claude Fable 5 --------- Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com> Co-authored-by: Claude Fable 5 --- DECISIONS.md | 2 + app/(dashboard)/invoices/[id]/page.tsx | 50 ++- app/(dashboard)/pending/page.tsx | 6 + .../supplier-invoices/[id]/page.tsx | 292 ++++++++---- app/(dashboard)/transactions/page.tsx | 425 +++++++++++++----- .../general/InvoiceInboxWorkspace.tsx | 64 ++- .../__tests__/booking-feedback-parity.test.ts | 11 +- components/ui/destructive-confirm-dialog.tsx | 68 ++- lib/__tests__/concurrency.test.ts | 64 +++ lib/concurrency.ts | 33 ++ messages/en.json | 9 + messages/sv.json | 9 + 12 files changed, 780 insertions(+), 253 deletions(-) create mode 100644 lib/__tests__/concurrency.test.ts create mode 100644 lib/concurrency.ts diff --git a/DECISIONS.md b/DECISIONS.md index d6538132..70b9dfb1 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1028,6 +1028,8 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-08-16] /transactions FyPicker double-fetch fixed by gating the initial fetch on FyPicker's existing onReady (fires after its restore onChange) instead of the analysis doc's literal "read the persisted period synchronously in initial state": localStorage only holds the period ID, not the FiscalPeriod bounds, so a synchronous read would suppress FyPicker's restore (value !== null) and leave the fetch permanently unscoped while the chip claimed a year. Same outcome (one scoped fetch per mount, background refetch on period change) without a stale-bounds cache or new FyPicker API. [2026-08-16] Row exit animation for dry-table rows collapses via td padding/line-height/font-size transitions plus a numeric max-height (.row-collapsible) on the fixed-height cell spans, not grid-template-rows 0fr (the AttGoraSection pattern): table cells cannot host the grid wrapper without restructuring every td, and max-height needs a numeric rest value because auto/none does not interpolate. prefers-reduced-motion hides the exiting row instantly (display: none) while the 350ms timer does the state cleanup. [2026-08-16] Restyled QuickReviewDialog's inbox-picker trigger to the same full-width dropzone-footer row as TransactionBookingDialog even though it did not share the orphan-button layout: both surfaces come from #1620 and should present the same underlag affordance; the alternative (leaving a small outline button in one dialog and a footer row in the other) would split the visual language of one control. Presentation only, disabled-while-booking kept (PR #1628). +[2026-08-17] Batch "Bokför valda" ships ONE aggregate toast with an "Ångra alla" action instead of dropping undo from batch toasts: per-row undo is feasible today (each booked row storno-reverses via POST /uncategorize, the same endpoint as the per-row Ångra), so the aggregate action just pools it over every booked row. Silent mode suppresses the per-row success toast and the generic failure toast only; interactive escalations (SI/CI match suggestions, duplicate warning, activate-account) keep their dialogs because they are the only way forward for those rows. +[2026-08-17] /pending: a FAILED fetch for a tab whose rows are not on screen HOLDS the loading state (spinner + error toast) rather than clearing rows to the empty state: "Inget att granska" after a failed load would be indistinguishable from a genuinely empty list, and BFL-relevant pending work must not look done when it is unknown. [2026-08-17] Full-archive direct download resurfaced as an ImportRow + small centered dialog on /import's Exportera tab (row "Komplett arkiv", hash #full-archive), not by re-mounting the orphaned components/settings/BackupDownloadForm.tsx: the form was pre-frame card styling with a duplicate cloud-backup section, while the export tab's existing SIE dialog sets the house pattern (ImportRow -> sm:max-w-md dialog). Its logic (estimate, 413 handling, last-download stamp) ported into components/import/FullArchiveDialog.tsx; the orphan and its dead settings_backup_download i18n namespace deleted. The dialog reuses FiscalYearSelector despite design.md's "legacy, no new uses" line: FyPicker is a toolbar context chip, and the SIE dialog in the same file already uses FiscalYearSelector for the identical dialog-form slot, so matching it beats introducing a third pattern. Row gated to owner/admin because GET /api/reports/full-archive enforces that role server-side; showing members a download that can only 403 helps nobody. [2026-08-17] Article picker now overwrites the line's ROT/RUT (deduction_type + work_type) from the article's housework_type, INCLUDING clearing it when the article has none: article-defines-the-row is the established applyArticle semantic (description/price/unit already overwrite), and keeping a RUT flag when switching a row to a material article would silently claim a deduction on material (HUSFL labor-only rule). Kundkort personnummer prefill is a server-side fallback in buildInvoiceWriteData (typed > stored draft > kundkort), never a client prefill: customers.personal_number reaches the browser only as ciphertext/mask by design, so the editor just relaxes the required-mark and says where the number will come from. [2026-08-17] Arcim's "610 bilagor i Hela historiken men 100 i räkenskapsåret" in the full-archive dialog is NOT a pagination bug: verified against prod, exactly 100 documents are linked to posted vouchers in the single (extended) fiscal year and 510 are unlinked inbox/receipt docs, which scope=all includes by design (same split as cloud backup's year-ZIPs vs Grunddata.zip). Kept the semantics, fixed two things instead: estimateArchiveSize's period branch ran one unpaginated read with one flat IN() over every entry id (undercounts past the PostgREST row cap, URL blowup past ~a few hundred ids) -> now CHILD_FK_CHUNK-chunked and fetchAllRows-paginated like writeDocuments already was; and the dialog now states per scope which document set is counted, so the gap reads as intent, not as a bug. diff --git a/app/(dashboard)/invoices/[id]/page.tsx b/app/(dashboard)/invoices/[id]/page.tsx index f56b8526..eb3258e3 100644 --- a/app/(dashboard)/invoices/[id]/page.tsx +++ b/app/(dashboard)/invoices/[id]/page.tsx @@ -1,6 +1,6 @@ 'use client' -import { useState, useEffect, use } from 'react' +import { useState, useEffect, useRef, use } from 'react' import { useRouter } from 'next/navigation' import Link from 'next/link' import { useLocale, useTranslations } from 'next-intl' @@ -181,6 +181,12 @@ export default function InvoiceDetailPage({ params }: { params: Promise<{ id: st const statusLabel = (status: InvoiceStatus): string => t(`status_${status}`) const reminderLevelLabel = (level: 1 | 2 | 3): string => t(`reminder_level_${level}`) + // Latest-request guard for fetchInvoice. A mutation refresh can overlap the + // pager stepping to a sibling invoice (the component stays mounted, only + // `id` changes), and without it the older response would commit invoice A's + // state under invoice B's URL. Only the newest request may write state. + const fetchSeqRef = useRef(0) + useEffect(() => { fetchInvoice() }, [id]) @@ -214,7 +220,13 @@ export default function InvoiceDetailPage({ params }: { params: Promise<{ id: st } async function fetchInvoice() { - setIsLoading(true) + const seq = ++fetchSeqRef.current + // The blocking spinner is reserved for the first load (or stepping to a + // different invoice via the pager). Refetches after Bokför / status + // change / finalize / payment / send reconcile BEHIND the mounted page: + // a one-field state change must not collapse the whole detail view to a + // spinner, reset scroll, and remount every card. + if (!invoice || invoice.id !== id) setIsLoading(true) // Settings depend only on the active company, so start them with the main // invoice batch instead of waiting for the invoice row first. @@ -260,6 +272,10 @@ export default function InvoiceDetailPage({ params }: { params: Promise<{ id: st deliveriesPromise, ]) + // A newer fetch owns the page now (pager step or later refresh): commit + // nothing from this one, not even the not-found redirect. + if (seq !== fetchSeqRef.current) return + if (error || !data) { toast({ title: t('load_failed_title'), @@ -306,6 +322,7 @@ export default function InvoiceDetailPage({ params }: { params: Promise<{ id: st } const settingsRes = await settingsPromise + if (seq !== fetchSeqRef.current) return if (settingsRes) { const settings = settingsRes.data setOreRounding(settings?.ore_rounding ?? true) @@ -352,6 +369,10 @@ export default function InvoiceDetailPage({ params }: { params: Promise<{ id: st .single() : Promise.resolve(null), ]).then(([creditNoteRes, originalRes, convertedRes]) => { + // Deferred writes need the same guard: they land after first paint + // and would otherwise attach the previous invoice's related documents + // to the one the pager has since navigated to. + if (seq !== fetchSeqRef.current) return setCreditNote(creditNoteRes?.data ? (creditNoteRes.data as Invoice) : null) if (originalRes?.data) { setOriginalInvoice(originalRes.data as Invoice) @@ -400,7 +421,9 @@ export default function InvoiceDetailPage({ params }: { params: Promise<{ id: st } else { toast({ title: t('booked_title'), description: t('booked_description') }) } - fetchInvoice() + // Awaited so the Bokför button's pending state covers the in-place + // refresh: the spinner stops when the page shows the booked state. + await fetchInvoice() } catch (error) { toast({ title: t('book_failed_title'), @@ -453,7 +476,9 @@ export default function InvoiceDetailPage({ params }: { params: Promise<{ id: st title: t('status_update_toast_title'), description: t('status_update_toast_description', { status: statusLabel(status).toLowerCase() }), }) - fetchInvoice() + // Awaited: the acting button keeps its pending state until the page + // reflects the new status (the refetch runs behind the mounted content). + await fetchInvoice() } catch (error) { toast({ title: t('status_update_failed_title'), @@ -810,7 +835,7 @@ export default function InvoiceDetailPage({ params }: { params: Promise<{ id: st }) setShowFinalizeDialog(false) - fetchInvoice() + await fetchInvoice() } catch (error) { toast({ title: t('finalize_failed_title'), @@ -1073,7 +1098,13 @@ export default function InvoiceDetailPage({ params }: { params: Promise<{ id: st disabled={isUpdating || !canWrite} title={!canWrite ? t('viewer_disabled_tooltip') : undefined} > - {canWrite ? : } + {isUpdating ? ( + + ) : canWrite ? ( + + ) : ( + + )} {t('mark_as_sent')} )} @@ -1451,6 +1482,7 @@ export default function InvoiceDetailPage({ params }: { params: Promise<{ id: st {t('not_booked_yet')} {canWrite && ( )} @@ -1774,7 +1806,11 @@ export default function InvoiceDetailPage({ params }: { params: Promise<{ id: st onClick={() => updateStatus('cancelled')} disabled={isUpdating} > - + {isUpdating ? ( + + ) : ( + + )} {t('cancel_action')} )} diff --git a/app/(dashboard)/pending/page.tsx b/app/(dashboard)/pending/page.tsx index 3ca80375..141b6c88 100644 --- a/app/(dashboard)/pending/page.tsx +++ b/app/(dashboard)/pending/page.tsx @@ -387,6 +387,12 @@ export default function PendingOperationsPage() { } catch { if (!isCurrent()) return toast({ title: 'Kunde inte ladda operationer', variant: 'destructive' }) + // The rows on screen belong to another tab (or no load has succeeded + // yet): dropping the loading state here would render those foreign rows + // under this tab's header as if they were its content. Hold the loading + // state instead; the toast says why, and the next tab switch or + // realtime echo retries. + if (loadedTabRef.current !== activeTab) return } if (!isCurrent()) return setIsLoading(false) diff --git a/app/(dashboard)/supplier-invoices/[id]/page.tsx b/app/(dashboard)/supplier-invoices/[id]/page.tsx index af350814..fa8896c2 100644 --- a/app/(dashboard)/supplier-invoices/[id]/page.tsx +++ b/app/(dashboard)/supplier-invoices/[id]/page.tsx @@ -1,6 +1,6 @@ 'use client' -import { useState, useEffect, useMemo } from 'react' +import { useState, useEffect, useMemo, useRef } from 'react' import { useParams, useRouter } from 'next/navigation' import { useTranslations } from 'next-intl' import { Button } from '@/components/ui/button' @@ -13,7 +13,7 @@ import { Dialog, DialogContent, DialogHeader, DialogTitle } from '@/components/u import { Tabs, TabsList, TabsTrigger, TabsContent } from '@/components/ui/tabs' import { useToast } from '@/components/ui/use-toast' import { getErrorMessage } from '@/lib/errors/get-error-message' -import { ArrowLeft, CheckCircle, CreditCard, FileText, Trash2, Lock, Undo2, Info, Pencil, Plus, CalendarClock, Paperclip } from 'lucide-react' +import { ArrowLeft, CheckCircle, CreditCard, FileText, Trash2, Lock, Undo2, Info, Loader2, Pencil, Plus, CalendarClock, Paperclip } from 'lucide-react' import LinkVoucherPicker from '@/components/invoices/LinkVoucherPicker' import { useCanWrite } from '@/lib/hooks/use-can-write' import { formatDate, cn } from '@/lib/utils' @@ -97,7 +97,13 @@ export default function SupplierInvoiceDetailPage() { const [paymentAccount, setPaymentAccount] = useState('1930') const [accounts, setAccounts] = useState([]) const [areAccountsLoading, setAreAccountsLoading] = useState(false) - const [isProcessing, setIsProcessing] = useState(false) + // Which action is in flight, not just whether one is: the acting button + // shows the spinner while the others only disable. A single boolean put + // identical pending feedback (none) on every button at once. + const [processingAction, setProcessingAction] = useState< + 'approve' | 'book' | 'mark_paid' | 'credit' | 'uncredit' | 'delete' | null + >(null) + const isProcessing = processingAction !== null const [duplicateCandidates, setDuplicateCandidates] = useState< Array<{ id: string @@ -113,6 +119,13 @@ export default function SupplierInvoiceDetailPage() { const [editLines, setEditLines] = useState([]) const { dialogProps: confirmDialogProps, confirm: confirmAction } = useDestructiveConfirm() + // Latest-request guard for fetchInvoice. A mutation refetch can overlap the + // pager stepping to a sibling invoice (the component stays mounted, only + // params.id changes), and without it the older response would commit + // invoice A's row and payment-form defaults under invoice B's URL. Only the + // newest request may write state. + const fetchSeqRef = useRef(0) + const statusLabels = useMemo>(() => ({ registered: t('status_registered'), approved: t('status_approved'), @@ -125,13 +138,20 @@ export default function SupplierInvoiceDetailPage() { }), [t]) async function fetchInvoice() { - setIsLoading(true) + const seq = ++fetchSeqRef.current + // Blocking skeleton only before the first paint (or when the pager steps + // to a different invoice). Attest/Bokför/Markera betald/kreditera each + // refetch after their mutation: those reconcile behind the mounted page + // instead of swapping the whole detail for a skeleton and back. + if (!invoice || invoice.id !== params.id) setIsLoading(true) // try/finally: a dropped connection or a non-JSON error page makes // res.json() throw, and this runs from an effect, so the rejection is // unhandled and isLoading would stay true: a spinner that never resolves. try { const res = await fetch(`/api/supplier-invoices/${params.id}`) const body = await res.json().catch(() => null) + // A newer fetch owns the page now: commit nothing from this one. + if (seq !== fetchSeqRef.current) return // See the identical fix in suppliers/[id]: `body.error` is the canonical // envelope object, and rendering an object as a toast description throws // out of the root layout into global-error. @@ -147,13 +167,16 @@ export default function SupplierInvoiceDetailPage() { setPaymentDate(new Date().toISOString().split('T')[0]) } } catch (err) { + if (seq !== fetchSeqRef.current) return toast({ title: t('load_failed_title'), description: getErrorMessage(err, { context: 'supplier_invoice' }), variant: 'destructive', }) } finally { - setIsLoading(false) + // A stale request must not stop the newest one's skeleton early: only + // the request that still owns the page resolves the loading state. + if (seq === fetchSeqRef.current) setIsLoading(false) } } @@ -293,38 +316,53 @@ export default function SupplierInvoiceDetailPage() { }, [accounts.length, isPayDialogOpen]) async function handleApprove() { - setIsProcessing(true) - const res = await fetch(`/api/supplier-invoices/${params.id}/approve`, { method: 'POST' }) - const result = await res.json() - if (!res.ok) { - toast({ title: t('approve_failed_title'), description: getErrorMessage(result, { context: 'supplier_invoice' }), variant: 'destructive' }) - } else { - toast({ title: t('approved_title'), description: t('approved_description') }) - fetchInvoice() + setProcessingAction('approve') + // try/catch/finally like handleDelete: a rejected fetch()/res.json() + // must not skip the reset below, or isProcessing keeps every invoice + // action disabled until a full page reload. + try { + const res = await fetch(`/api/supplier-invoices/${params.id}/approve`, { method: 'POST' }) + const result = await res.json() + if (!res.ok) { + toast({ title: t('approve_failed_title'), description: getErrorMessage(result, { context: 'supplier_invoice' }), variant: 'destructive' }) + } else { + toast({ title: t('approved_title'), description: t('approved_description') }) + // Awaited: the Attestera button keeps its spinner until the page shows + // the approved state (the refetch runs behind the mounted content). + await fetchInvoice() + } + } catch (err) { + toast({ title: t('approve_failed_title'), description: getErrorMessage(err, { context: 'supplier_invoice' }), variant: 'destructive' }) + } finally { + setProcessingAction(null) } - setIsProcessing(false) } // #967: deferred booking: create the registration verifikat afterwards. async function handleBook() { - setIsProcessing(true) - const res = await fetch(`/api/supplier-invoices/${params.id}/book`, { method: 'POST' }) - const result = await res.json() - if (!res.ok) { - toast({ title: t('book_failed_title'), description: getErrorMessage(result, { context: 'supplier_invoice' }), variant: 'destructive' }) - } else if (Array.isArray(result.warnings) && result.warnings.length > 0) { - // Booked, but a follow-up is needed (e.g. periodiseringar failed). - toast({ title: t('booked_title'), description: t('booked_with_warnings_description'), variant: 'destructive' }) - fetchInvoice() - } else { - toast({ title: t('booked_title'), description: t('booked_description') }) - fetchInvoice() + setProcessingAction('book') + try { + const res = await fetch(`/api/supplier-invoices/${params.id}/book`, { method: 'POST' }) + const result = await res.json() + if (!res.ok) { + toast({ title: t('book_failed_title'), description: getErrorMessage(result, { context: 'supplier_invoice' }), variant: 'destructive' }) + } else if (Array.isArray(result.warnings) && result.warnings.length > 0) { + // Booked, but a follow-up is needed (e.g. periodiseringar failed). + toast({ title: t('booked_title'), description: t('booked_with_warnings_description'), variant: 'destructive' }) + await fetchInvoice() + } else { + toast({ title: t('booked_title'), description: t('booked_description') }) + await fetchInvoice() + } + } catch (err) { + toast({ title: t('book_failed_title'), description: getErrorMessage(err, { context: 'supplier_invoice' }), variant: 'destructive' }) + } finally { + setProcessingAction(null) } - setIsProcessing(false) } async function handleMarkPaid(force: boolean = false) { - setIsProcessing(true) + setProcessingAction('mark_paid') // When the user has edited the booking rows in this session, forward // them so the server validates balance and posts via createJournalEntry // directly. Otherwise the server picks the default routing (clearing @@ -342,39 +380,44 @@ export default function SupplierInvoiceDetailPage() { }) : undefined - const res = await fetch(`/api/supplier-invoices/${params.id}/mark-paid`, { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ - amount: parseFloat(payAmount), - payment_date: paymentDate, - payment_account: paymentAccount, - ...(force ? { force: true } : {}), - ...(linesPayload ? { lines: linesPayload } : {}), - }), - }) - const result = await res.json() - if (!res.ok) { - if (result?.error?.code === 'SI_PAID_LIKELY_DUPLICATE' && Array.isArray(result.error.details?.candidates)) { - setDuplicateCandidates(result.error.details.candidates) - setIsPayDialogOpen(false) - } else { - toast({ title: t('payment_failed_title'), description: getErrorMessage(result, { context: 'supplier_invoice' }), variant: 'destructive' }) - } - } else { - toast({ - title: result.status === 'paid' ? t('paid_title') : t('partial_payment_title'), - // The paid amount is in the invoice's currency (the dialog's helper - // text says so): the toast must not relabel it as kr. - description: t('amount_registered_description', { - amount: formatCurrency(parseFloat(payAmount), invoice?.currency || 'SEK'), + try { + const res = await fetch(`/api/supplier-invoices/${params.id}/mark-paid`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + amount: parseFloat(payAmount), + payment_date: paymentDate, + payment_account: paymentAccount, + ...(force ? { force: true } : {}), + ...(linesPayload ? { lines: linesPayload } : {}), }), }) - setIsPayDialogOpen(false) - setDuplicateCandidates(null) - fetchInvoice() + const result = await res.json() + if (!res.ok) { + if (result?.error?.code === 'SI_PAID_LIKELY_DUPLICATE' && Array.isArray(result.error.details?.candidates)) { + setDuplicateCandidates(result.error.details.candidates) + setIsPayDialogOpen(false) + } else { + toast({ title: t('payment_failed_title'), description: getErrorMessage(result, { context: 'supplier_invoice' }), variant: 'destructive' }) + } + } else { + toast({ + title: result.status === 'paid' ? t('paid_title') : t('partial_payment_title'), + // The paid amount is in the invoice's currency (the dialog's helper + // text says so): the toast must not relabel it as kr. + description: t('amount_registered_description', { + amount: formatCurrency(parseFloat(payAmount), invoice?.currency || 'SEK'), + }), + }) + setIsPayDialogOpen(false) + setDuplicateCandidates(null) + await fetchInvoice() + } + } catch (err) { + toast({ title: t('payment_failed_title'), description: getErrorMessage(err, { context: 'supplier_invoice' }), variant: 'destructive' }) + } finally { + setProcessingAction(null) } - setIsProcessing(false) } async function handleCredit() { @@ -385,34 +428,56 @@ export default function SupplierInvoiceDetailPage() { variant: 'warning', }) if (!ok) return - setIsProcessing(true) - const res = await fetch(`/api/supplier-invoices/${params.id}/credit`, { method: 'POST' }) - const result = await res.json() - if (!res.ok) { - toast({ title: t('credit_failed_title'), description: getErrorMessage(result, { context: 'supplier_invoice' }), variant: 'destructive' }) - } else { - toast({ title: t('credit_success_title') }) - fetchInvoice() + setProcessingAction('credit') + try { + const res = await fetch(`/api/supplier-invoices/${params.id}/credit`, { method: 'POST' }) + const result = await res.json() + if (!res.ok) { + toast({ title: t('credit_failed_title'), description: getErrorMessage(result, { context: 'supplier_invoice' }), variant: 'destructive' }) + } else { + toast({ title: t('credit_success_title') }) + await fetchInvoice() + } + } catch (err) { + toast({ title: t('credit_failed_title'), description: getErrorMessage(err, { context: 'supplier_invoice' }), variant: 'destructive' }) + } finally { + setProcessingAction(null) } - setIsProcessing(false) } async function handleDelete() { - const ok = await confirmAction({ + // The DELETE runs as the confirm's action: the dialog holds open with its + // pending spinner until the server answers (it used to close on click and + // leave the destructive icon button active with no feedback until the + // route swap, permitting duplicate DELETEs). + await confirmAction({ title: t('delete_confirm_title'), description: t('delete_confirm_description'), confirmLabel: t('delete_confirm_label'), variant: 'destructive', + }, async () => { + setProcessingAction('delete') + try { + const res = await fetch(`/api/supplier-invoices/${params.id}`, { method: 'DELETE' }) + const result = await res.json() + if (!res.ok) { + toast({ title: t('delete_failed_title'), description: getErrorMessage(result, { context: 'supplier_invoice' }), variant: 'destructive' }) + } else { + toast({ title: t('deleted_title') }) + router.push('/supplier-invoices') + return + } + } catch (err) { + toast({ + title: t('delete_failed_title'), + description: getErrorMessage(err, { context: 'supplier_invoice' }), + variant: 'destructive', + }) + } + // Only cleared on failure: on success the pending state rides through + // the route swap instead of re-enabling the button mid-navigation. + setProcessingAction(null) }) - if (!ok) return - const res = await fetch(`/api/supplier-invoices/${params.id}`, { method: 'DELETE' }) - const result = await res.json() - if (!res.ok) { - toast({ title: t('delete_failed_title'), description: getErrorMessage(result, { context: 'supplier_invoice' }), variant: 'destructive' }) - } else { - toast({ title: t('deleted_title') }) - router.push('/supplier-invoices') - } } async function handleUncredit() { @@ -423,23 +488,32 @@ export default function SupplierInvoiceDetailPage() { variant: 'warning', }) if (!ok) return - setIsProcessing(true) - const res = await fetch(`/api/supplier-invoices/${params.id}/uncredit`, { method: 'POST' }) - const result = await res.json() - if (!res.ok) { + setProcessingAction('uncredit') + try { + const res = await fetch(`/api/supplier-invoices/${params.id}/uncredit`, { method: 'POST' }) + const result = await res.json() + if (!res.ok) { + toast({ + title: t('uncredit_failed_title'), + description: getErrorMessage(result, { context: 'supplier_invoice' }), + variant: 'destructive', + }) + } else { + toast({ + title: t('uncredit_success_title'), + description: t('uncredit_success_description'), + }) + await fetchInvoice() + } + } catch (err) { toast({ title: t('uncredit_failed_title'), - description: getErrorMessage(result, { context: 'supplier_invoice' }), + description: getErrorMessage(err, { context: 'supplier_invoice' }), variant: 'destructive', }) - } else { - toast({ - title: t('uncredit_success_title'), - description: t('uncredit_success_description'), - }) - fetchInvoice() + } finally { + setProcessingAction(null) } - setIsProcessing(false) } if (isLoading) { @@ -534,7 +608,13 @@ export default function SupplierInvoiceDetailPage() { disabled={isProcessing || !canWrite} title={!canWrite ? t('viewer_disabled_tooltip') : undefined} > - {canWrite ? : } + {processingAction === 'approve' ? ( + + ) : canWrite ? ( + + ) : ( + + )} {t('approve')} )} @@ -554,7 +634,13 @@ export default function SupplierInvoiceDetailPage() { title={!canWrite ? t('viewer_disabled_tooltip') : undefined} aria-label={t('delete_confirm_label')} > - {canWrite ? : } + {processingAction === 'delete' ? ( + + ) : canWrite ? ( + + ) : ( + + )} )} {['approved', 'overdue', 'partially_paid'].includes(invoice.status) && ( @@ -574,7 +660,13 @@ export default function SupplierInvoiceDetailPage() { disabled={isProcessing || !canWrite} title={!canWrite ? t('viewer_disabled_tooltip') : undefined} > - {canWrite ? : } + {processingAction === 'credit' ? ( + + ) : canWrite ? ( + + ) : ( + + )} {t('credit_note_button')} )} @@ -587,7 +679,13 @@ export default function SupplierInvoiceDetailPage() { disabled={isProcessing || !canWrite} title={!canWrite ? t('viewer_disabled_tooltip') : undefined} > - {canWrite ? : } + {processingAction === 'uncredit' ? ( + + ) : canWrite ? ( + + ) : ( + + )} {t('uncredit_button')} )} @@ -908,7 +1006,13 @@ export default function SupplierInvoiceDetailPage() { disabled={isProcessing || !canWrite} title={!canWrite ? t('viewer_disabled_tooltip') : undefined} > - {canWrite ? : } + {processingAction === 'book' ? ( + + ) : canWrite ? ( + + ) : ( + + )} {t('book_action')} diff --git a/app/(dashboard)/transactions/page.tsx b/app/(dashboard)/transactions/page.tsx index 389865e5..a912819e 100644 --- a/app/(dashboard)/transactions/page.tsx +++ b/app/(dashboard)/transactions/page.tsx @@ -32,6 +32,7 @@ import TransactionHistoryList from '@/components/transactions/TransactionHistory import InboxZeroState from '@/components/transactions/InboxZeroState' import SkattekontoInboxCard from '@/components/transactions/SkattekontoInboxCard' import type { BookedDuplicateCandidate } from '@/lib/transactions/booking-duplicate-detection' +import { mapWithConcurrency } from '@/lib/concurrency' import { DialogLoadingSkeleton } from '@/components/ui/dialog-loading-skeleton' import { getTemplateById, type BookingTemplate } from '@/lib/bookkeeping/booking-templates' @@ -370,7 +371,9 @@ export default function TransactionsPage() { // categorizing direct to 2440. Triggered by a 409 TX_CATEGORIZE_SUGGEST_SI_MATCH. const [siMatchSuggestion, setSiMatchSuggestion] = useState<{ transactionId: string - retry: () => Promise + // Resolved value unused: callers only await completion. runCategorize + // resolves its outcome object, the counterparty path a journal-entry id. + retry: () => Promise candidates: Array<{ supplier_invoice_id: string invoice_number: string @@ -387,7 +390,9 @@ export default function TransactionsPage() { // Triggered by a 409 TX_CATEGORIZE_SUGGEST_CI_MATCH. const [ciMatchSuggestion, setCiMatchSuggestion] = useState<{ transactionId: string - retry: () => Promise + // Resolved value unused: callers only await completion. runCategorize + // resolves its outcome object, the counterparty path a journal-entry id. + retry: () => Promise candidates: Array<{ invoice_id: string invoice_number: string | null @@ -411,7 +416,9 @@ export default function TransactionsPage() { // candidate kinds), which the server re-detects so a stale id can't wave it. const [duplicateWarning, setDuplicateWarning] = useState<{ transactionId: string - retry: () => Promise + // Resolved value unused: callers only await completion. runCategorize + // resolves its outcome object, the counterparty path a journal-entry id. + retry: () => Promise candidate: BookedDuplicateCandidate } | null>(null) const [duplicateProcessing, setDuplicateProcessing] = useState(false) @@ -881,12 +888,23 @@ export default function TransactionsPage() { } }, [companyId]) + // Same sequence-guard pattern as fetchGenerationRef: only the newest + // skattekonto fetch may write rows. A company switch bumps the sequence so a + // response started under the previous company can never land its rows in + // the new company's inbox, and overlapping refreshes resolve last-write- + // correct instead of last-resolved-wins. + const skvFetchSeqRef = useRef(0) const loadSkvRows = useCallback(async () => { + const seq = ++skvFetchSeqRef.current // Connection health, fetched alongside the rows: any failure (extension // disabled, capability gate, not connected) just hides the banner. void (async () => { try { const res = await fetch('/api/extensions/ext/skatteverket/status') + // Same guard as the rows below: a status response started under the + // previous company must not set or clear the reconnect banner for + // the new one. + if (skvFetchSeqRef.current !== seq) return if (!res.ok) { setSkvNeedsReconnect(false) return @@ -898,6 +916,7 @@ export default function TransactionsPage() { expired?: boolean canRefresh?: boolean } + if (skvFetchSeqRef.current !== seq) return setSkvNeedsReconnect( Boolean( s.connected && @@ -906,22 +925,26 @@ export default function TransactionsPage() { ), ) } catch { + if (skvFetchSeqRef.current !== seq) return setSkvNeedsReconnect(false) } })() try { const res = await fetch('/api/extensions/ext/skatteverket/skattekonto/transaktioner') + if (skvFetchSeqRef.current !== seq) return if (!res.ok) { setSkvRows([]) return } const json = await res.json() + if (skvFetchSeqRef.current !== seq) return const booked = (json.data?.booked ?? []) as SkattekontoTransactionWithSuggestion[] // Keep all booked SKV rows in state: inbox view filters to obokförda // (journal_entry_id null), history view shows all of them (matched // and unmatched) interleaved with bank tx by date. setSkvRows(booked) } catch { + if (skvFetchSeqRef.current !== seq) return setSkvRows([]) } }, []) @@ -1304,6 +1327,20 @@ export default function TransactionsPage() { // finally-guard and drop the cue. scopeRefreshSeqRef.current++ setIsScopeRefreshing(false) + // Data boundary: the previous company's rows must neither render for the + // new one (the takeover fetch is async; until it resolves the old rows + // would sit under the new company's header) nor land later from a fetch + // still in flight. Clearing rows + bumping both fetch sequences closes + // both holes; the counts and paging state describe the cleared rows, so + // they reset with them. + setTransactions([]) + setSkvRows([]) + setTotalUncategorizedCount(null) + setPagedThroughDate(null) + setHasMore(false) + pagedCountRef.current = 0 + fetchGenerationRef.current++ + skvFetchSeqRef.current++ } // Fetch the page whenever the scope changes (mount, company switch, period @@ -1398,7 +1435,8 @@ export default function TransactionsPage() { }, [transactions.length]) const handleCategorize: CategorizeHandler = async (id, isBusiness, category, vatTreatment, accountOverride, templateId, inboxItemId, dimensions) => { - return runCategorize({ id, isBusiness, category, vatTreatment, accountOverride, templateId, inboxItemId, dimensions, confirmNoMatch: false }) + const outcome = await runCategorize({ id, isBusiness, category, vatTreatment, accountOverride, templateId, inboxItemId, dimensions, confirmNoMatch: false }) + return outcome.journalEntryId } /** @@ -1412,6 +1450,21 @@ export default function TransactionsPage() { * from exitingIds again: an undo would have restored the row's data while * leaving it filtered out of the inbox. */ + // The one place that calls the storno endpoint (pinned by + // booking-feedback-parity.test.ts): the per-row Ångra action and the batch + // "Ångra alla" both route through it. + const undoCategorize = (id: string) => + fetch(`/api/transactions/${id}/uncategorize`, { method: 'POST' }) + + // Rows whose booking was undone while finishBooking's 350ms patch was still + // pending. The per-row Ångra covers this with a closure-local flag, but + // "Ångra alla" (undoBatchCategorize) cannot reach those closures: without a + // shared record its undo patch could land first and the timer then re-apply + // the booked shape, pointing journal_entry_id at a storno-reversed entry. + // Ids are removed again when a new booking for the row starts, so a + // re-booked row still gets its delayed patch. + const undoneIdsRef = useRef>(new Set()) + function finishBooking(args: { id: string isBusiness: boolean @@ -1419,26 +1472,36 @@ export default function TransactionsPage() { journalEntryId?: string | null journalEntryCreated?: boolean journalEntryError?: string | null + // Batch rows: keep the exit animation, count decrement and state patch, + // but leave the narration to the caller's single aggregate toast instead + // of stacking one toast per row. + silent?: boolean }) { - const { id, isBusiness, category, journalEntryId, journalEntryCreated, journalEntryError } = args + const { id, isBusiness, category, journalEntryId, journalEntryCreated, journalEntryError, silent } = 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 + // A fresh booking supersedes any earlier undo of the same row: without + // this, a row booked again after an Ångra would skip its delayed patch. + undoneIdsRef.current.delete(id) setExitingIds((prev) => new Set(prev).add(id)) setTotalUncategorizedCount((prev) => Math.max(0, (prev ?? 1) - 1)) - if (journalEntryCreated) { + if (silent) { + // No per-row toast: fall through to the delayed state patch below. + } else if (journalEntryCreated) { toast({ title: 'Bokförd', action: ( { try { - const undoRes = await fetch(`/api/transactions/${id}/uncategorize`, { method: 'POST' }) + const undoRes = await undoCategorize(id) if (undoRes.ok) { undone = true + undoneIdsRef.current.add(id) setTransactions((prev) => prev.map((t) => t.id === id @@ -1474,7 +1537,10 @@ export default function TransactionsPage() { // 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) { + // Both undo records gate the patch: the closure-local flag (per-row + // Ångra) and the shared ref ("Ångra alla", which runs outside this + // closure). The exitingIds/processingId cleanup below always runs. + if (!undone && !undoneIdsRef.current.has(id)) { setTransactions((prev) => prev.map((tx) => tx.id === id @@ -1515,8 +1581,19 @@ export default function TransactionsPage() { // ledger-only voucher candidate. force?: boolean expectedDuplicateJournalEntryId?: string - }): Promise { - const { id, isBusiness, category, vatTreatment, accountOverride, templateId, inboxItemId, dimensions, confirmNoMatch, force, expectedDuplicateJournalEntryId } = args + // Batch rows: the caller narrates the outcome once for the whole batch, + // so the per-row success toast (finishBooking) and the generic per-row + // failure toast stay quiet. Interactive escalations (match suggestions, + // duplicate warning, activate-account) keep their dialogs/actions: they + // are the only way forward for those rows. + silent?: boolean + // ok distinguishes "the server accepted the categorization" from "nothing + // happened". A 2xx with a null journal_entry_id is a real success (e.g. an + // already-categorized flag flip), so the id alone cannot carry that signal: + // the batch aggregate would count the row as failed after finishBooking + // already animated it out of the inbox. + }): Promise<{ ok: boolean; journalEntryId: string | null }> { + const { id, isBusiness, category, vatTreatment, accountOverride, templateId, inboxItemId, dimensions, confirmNoMatch, force, expectedDuplicateJournalEntryId, silent } = args try { setProcessingId(id) const response = await fetch(`/api/transactions/${id}/categorize`, { @@ -1552,7 +1629,7 @@ export default function TransactionsPage() { candidates: result.error.details.candidates, }) setProcessingId(null) - return null + return { ok: false, journalEntryId: null } } if ( result?.error?.code === 'TX_CATEGORIZE_SUGGEST_CI_MATCH' && @@ -1567,7 +1644,7 @@ export default function TransactionsPage() { candidates: result.error.details.candidates, }) setProcessingId(null) - return null + return { ok: false, journalEntryId: null } } if (result?.error?.code === 'TX_CATEGORIZE_INVALID_ACCOUNT') { // The user picked a library template (or typed an account @@ -1639,7 +1716,7 @@ export default function TransactionsPage() { ) : undefined, }) setProcessingId(null) - return null + return { ok: false, journalEntryId: null } } if (result?.error?.code === 'ACCOUNTS_NOT_IN_CHART') { // The mapped template/category references one or more accounts @@ -1699,7 +1776,7 @@ export default function TransactionsPage() { ) : undefined, }) setProcessingId(null) - return null + return { ok: false, journalEntryId: null } } if ( result?.error?.code === 'TRANSACTION_BOOK_POSSIBLE_DUPLICATE' && @@ -1721,15 +1798,17 @@ export default function TransactionsPage() { candidate, }) setProcessingId(null) - return null + return { ok: false, journalEntryId: null } + } + if (!silent) { + toast({ + title: 'Kategorisering misslyckades', + description: getErrorMessage(result, { context: 'transaction', statusCode: response.status }), + variant: 'destructive', + }) } - toast({ - title: 'Kategorisering misslyckades', - description: getErrorMessage(result, { context: 'transaction', statusCode: response.status }), - variant: 'destructive', - }) setProcessingId(null) - return null + return { ok: false, journalEntryId: null } } finishBooking({ @@ -1739,13 +1818,16 @@ export default function TransactionsPage() { journalEntryId: result.journal_entry_id, journalEntryCreated: result.journal_entry_created, journalEntryError: result.journal_entry_error, + silent, }) - return result.journal_entry_id || null + return { ok: true, journalEntryId: result.journal_entry_id || null } } catch { - toast({ title: t('booking_failed_title'), description: t('booking_failed_description'), variant: 'destructive' }) + if (!silent) { + toast({ title: t('booking_failed_title'), description: t('booking_failed_description'), variant: 'destructive' }) + } setProcessingId(null) - return null + return { ok: false, journalEntryId: null } } } @@ -2425,65 +2507,69 @@ export default function TransactionsPage() { const transaction = transactions.find((t) => t.id === id) if (!transaction) return - const ok = await confirm({ + // The DELETE runs as the confirm's action: the dialog stays open with its + // pending spinner until the operation settles (confirm-then-fetch used to + // close the dialog synchronously and leave the fetch with no visible + // state), then the row plays the same exit path as booking. + await confirm({ title: 'Ta bort transaktion', description: `Är du säker på att du vill ta bort "${transaction.description}"? Åtgärden kan inte ångras.`, confirmLabel: 'Ta bort', variant: 'destructive', - }) - if (!ok) return - - try { - // Row-level pending state while the DELETE runs (the card renders a - // spinner for processingId): confirm-to-completion used to be dead air. - setProcessingId(id) - const response = await fetch(`/api/transactions/${id}`, { method: 'DELETE' }) - if (!response.ok) { - const result = await response.json() - setProcessingId((prev) => (prev === id ? null : prev)) - toast({ - title: 'Kunde inte ta bort', - description: getErrorMessage(result, { context: 'transaction' }), - variant: 'destructive', - }) - return - } - // Same exit path as booking/ignore: the row animates out over the - // 350ms window instead of vanishing with a hard jump, then the delayed - // filter actually removes it. - setExitingIds((prev) => new Set(prev).add(id)) - // Drop the row from the batch selection immediately: leaving it there - // keeps the bulk bar counting (and acting on) a row that no longer - // exists once the timer removes it. - setSelectedIds((prev) => { - if (!prev.has(id)) return prev - const next = new Set(prev) - next.delete(id) - return next - }) - // The realtime echo is not guaranteed for DELETE on a filtered - // subscription, so the pending badge must decrement locally. - if (transaction.is_business === null && !transaction.is_ignored) { - setTotalUncategorizedCount((prev) => Math.max(0, (prev ?? 1) - 1)) - } - toast({ title: t('deleted_title'), description: t('deleted_description') }) - setTimeout(() => { - setTransactions((prev) => prev.filter((t) => t.id !== id)) - setExitingIds((prev) => { + }, async () => { + try { + // Row-level pending state while the DELETE runs (the card renders a + // spinner for processingId): covers the exit window after the dialog + // closes. + setProcessingId(id) + const response = await fetch(`/api/transactions/${id}`, { method: 'DELETE' }) + if (!response.ok) { + const result = await response.json() + setProcessingId((prev) => (prev === id ? null : prev)) + toast({ + title: 'Kunde inte ta bort', + description: getErrorMessage(result, { context: 'transaction' }), + variant: 'destructive', + }) + return + } + // Same exit path as booking/ignore: the row animates out over the + // 350ms window instead of vanishing with a hard jump, then the delayed + // filter actually removes it. + setExitingIds((prev) => new Set(prev).add(id)) + // Drop the row from the batch selection immediately: leaving it there + // keeps the bulk bar counting (and acting on) a row that no longer + // exists once the timer removes it. + setSelectedIds((prev) => { + if (!prev.has(id)) return prev const next = new Set(prev) next.delete(id) return next }) + // The realtime echo is not guaranteed for DELETE on a filtered + // subscription, so the pending badge must decrement locally. + if (transaction.is_business === null && !transaction.is_ignored) { + setTotalUncategorizedCount((prev) => Math.max(0, (prev ?? 1) - 1)) + } + toast({ title: t('deleted_title'), description: t('deleted_description') }) + setTimeout(() => { + setTransactions((prev) => prev.filter((t) => t.id !== id)) + setExitingIds((prev) => { + const next = new Set(prev) + next.delete(id) + return next + }) + setProcessingId((prev) => (prev === id ? null : prev)) + }, 350) + } catch { setProcessingId((prev) => (prev === id ? null : prev)) - }, 350) - } catch { - setProcessingId((prev) => (prev === id ? null : prev)) - toast({ - title: 'Kunde inte ta bort', - description: t('delete_failed_description'), - variant: 'destructive', - }) - } + toast({ + title: 'Kunde inte ta bort', + description: t('delete_failed_description'), + variant: 'destructive', + }) + } + }) } function openEditTitleDialog(transaction: TransactionWithInvoice) { @@ -2828,6 +2914,13 @@ export default function TransactionsPage() { } // Batch mode handlers + + // Bounded pool for the per-row batch requests: parallel enough that a + // 20-row batch finishes in a few round trips instead of 20 serialized ones, + // bounded so 100 selected rows never fan out as 100 concurrent requests. + // The bulkbar counter ticks per completed row. + const BATCH_CONCURRENCY = 5 + function toggleBatchSelect(id: string) { setSelectedIds((prev) => { const next = new Set(prev) @@ -2878,23 +2971,30 @@ export default function TransactionsPage() { const deletedIds = new Set() setBatchProgress({ done: 0, total: ids.length }) + let completed = 0 + const results = await mapWithConcurrency(ids, BATCH_CONCURRENCY, async (id) => { + let deleted = false + try { + const response = await fetch(`/api/transactions/${id}`, { method: 'DELETE' }) + deleted = response.ok + } catch { + deleted = false + } + completed++ + setBatchProgress({ done: completed, total: ids.length }) + return deleted + }) let successes = 0 const failures: string[] = [] - for (let i = 0; i < ids.length; i++) { - try { - const response = await fetch(`/api/transactions/${ids[i]}`, { method: 'DELETE' }) - if (response.ok) { - successes++ - deletedIds.add(ids[i]) - } else { - const tx = transactions.find((t) => t.id === ids[i]) - failures.push(tx?.description || ids[i]) - } - } catch { - failures.push(ids[i]) + ids.forEach((id, i) => { + if (results[i]) { + successes++ + deletedIds.add(id) + } else { + const tx = transactions.find((t) => t.id === id) + failures.push(tx?.description || id) } - setBatchProgress({ done: i + 1, total: ids.length }) - } + }) if (deletedIds.size > 0) { setTransactions((prev) => prev.filter((t) => !deletedIds.has(t.id))) } @@ -2927,23 +3027,30 @@ export default function TransactionsPage() { const ignoredIds = new Set() setBatchProgress({ done: 0, total: ids.length }) + let completed = 0 + const results = await mapWithConcurrency(ids, BATCH_CONCURRENCY, async (id) => { + let ignored = false + try { + const res = await fetch(`/api/transactions/${id}/ignore`, { method: 'POST' }) + ignored = res.ok + } catch { + ignored = false + } + completed++ + setBatchProgress({ done: completed, total: ids.length }) + return ignored + }) let successes = 0 const failures: string[] = [] - for (let i = 0; i < ids.length; i++) { - try { - const res = await fetch(`/api/transactions/${ids[i]}/ignore`, { method: 'POST' }) - if (res.ok) { - successes++ - ignoredIds.add(ids[i]) - } else { - const tx = transactions.find((t) => t.id === ids[i]) - failures.push(tx?.description || ids[i]) - } - } catch { - failures.push(ids[i]) + ids.forEach((id, i) => { + if (results[i]) { + successes++ + ignoredIds.add(id) + } else { + const tx = transactions.find((t) => t.id === id) + failures.push(tx?.description || id) } - setBatchProgress({ done: i + 1, total: ids.length }) - } + }) if (ignoredIds.size > 0) { setExitingIds((prev) => { const next = new Set(prev) @@ -2986,32 +3093,114 @@ export default function TransactionsPage() { async function handleBatchCategorize(category: TransactionCategory, vatTreatment?: VatTreatment) { const ids = Array.from(selectedIds) setBatchProgress({ done: 0, total: ids.length }) - let successes = 0 - const failures: string[] = [] - for (let i = 0; i < ids.length; i++) { - const result = await handleCategorize(ids[i], true, category, vatTreatment) - if (result) { - successes++ - } else { - const tx = transactions.find((t) => t.id === ids[i]) - failures.push(tx?.description || ids[i]) - } - setBatchProgress({ done: i + 1, total: ids.length }) - } + let completed = 0 + const results = await mapWithConcurrency(ids, BATCH_CONCURRENCY, async (id) => { + // silent: rows still animate out and decrement the count through + // finishBooking, but the narration is the single aggregate toast below + // instead of one "Bokförd" toast per row stacking over the list. + const outcome = await runCategorize({ + id, + isBusiness: true, + category, + vatTreatment, + confirmNoMatch: false, + silent: true, + }) + completed++ + setBatchProgress({ done: completed, total: ids.length }) + return outcome + }) setBatchProgress(null) setShowBatchSelector(false) - if (failures.length === 0) { - toast({ title: 'Klart', description: `${successes} transaktioner bokförda` }) + // Success is the server's 2xx (outcome.ok), not a non-null journal entry + // id: a flag-flip booking returns 200 with a null id and must not be + // narrated as "misslyckades" after finishBooking already animated it out. + const successCount = results.filter((r) => r.ok).length + const failedCount = ids.length - successCount + // One Ångra-alla for the whole batch: it runs the same storno endpoint as + // the per-row toast's Ångra, which requires a posted journal entry, so + // only rows that actually got one are undoable. A successful flag-flip + // row (ok, null id) has no verifikat to reverse. + const undoableIds = ids.filter((_, i) => results[i].ok && results[i].journalEntryId) + const undoAllAction = + undoableIds.length > 0 ? ( + void undoBatchCategorize(undoableIds)} + > + {t('batch_undo_all')} + + ) : undefined + if (failedCount === 0) { + toast({ + title: t('batch_done_title'), + description: t('batch_categorize_done_description', { count: successCount }), + action: undoAllAction, + }) } else { toast({ - title: 'Delvis klart', - description: `${successes} lyckades, ${failures.length} misslyckades: ${failures.slice(0, 3).join(', ')}${failures.length > 3 ? '...' : ''}`, + title: t('batch_partial_title'), + description: t('batch_categorize_partial_description', { + success: successCount, + failed: failedCount, + }), variant: 'destructive', + action: undoAllAction, }) } exitBatchMode() } + /** + * Ångra alla for a batch booking: storno-reverses every booked row through + * the same single undo endpoint as the per-row Ångra (undoCategorize), with + * the same bounded pool, then restores the rows into the inbox in one state + * patch and reports the outcome once. + */ + async function undoBatchCategorize(ids: string[]) { + const results = await mapWithConcurrency(ids, BATCH_CONCURRENCY, async (id) => { + try { + const res = await undoCategorize(id) + return res.ok + } catch { + return false + } + }) + const undoneIds = new Set(ids.filter((_, i) => results[i])) + // Record the undos BEFORE patching state: any finishBooking timer still + // pending for these rows must see them as undone, or it would re-apply + // the booked shape over the restore below. + for (const undoneId of undoneIds) undoneIdsRef.current.add(undoneId) + if (undoneIds.size > 0) { + setTransactions((prev) => + prev.map((tx) => + undoneIds.has(tx.id) + ? { + ...tx, + is_business: null, + category: null as unknown as TransactionCategory, + journal_entry_id: null, + } + : tx, + ), + ) + setTotalUncategorizedCount((prev) => (prev ?? 0) + undoneIds.size) + } + const failed = ids.length - undoneIds.size + if (failed === 0) { + toast({ + title: t('undone_title'), + description: t('batch_undo_done_description', { count: undoneIds.size }), + }) + } else { + toast({ + title: t('batch_undo_partial_title'), + description: t('batch_undo_partial_description', { success: undoneIds.size, failed }), + variant: 'destructive', + }) + } + } + function openMatchDialog(transaction: TransactionWithInvoice) { setSelectedTransaction(transaction) setMatchDialogOpen(true) diff --git a/components/extensions/general/InvoiceInboxWorkspace.tsx b/components/extensions/general/InvoiceInboxWorkspace.tsx index 45918ad2..7a6a692f 100644 --- a/components/extensions/general/InvoiceInboxWorkspace.tsx +++ b/components/extensions/general/InvoiceInboxWorkspace.tsx @@ -360,10 +360,13 @@ export default function InvoiceInboxWorkspace(_props: WorkspaceComponentProps) { const [docUrl, setDocUrl] = useState(null) const [docMime, setDocMime] = useState(null) const [docState, setDocState] = useState('none') - // Which selection the in-flight document read belongs to. The user can click - // another row while it is running, and a late resolution must not paint its - // outcome (a URL, or an error) onto the row that is now selected. - const docRequestRef = useRef(null) + // Monotonic tokens for the in-flight detail and document reads. The user + // can click another row while one is running, but also re-request the SAME + // item (action refreshes, the processing->received re-select effect), so an + // id comparison is not enough: only the newest request of each kind may + // paint its outcome (a detail snapshot, a URL, or an error) onto the pane. + const detailRequestRef = useRef(0) + const docRequestRef = useRef(0) const [inboxAddress, setInboxAddress] = useState(null) // We asked for the inbox address and did not get an answer we can trust // (5xx, network, unparseable). Distinct from a 404, which honestly means no @@ -727,8 +730,8 @@ export default function InvoiceInboxWorkspace(_props: WorkspaceComponentProps) { // document, still loading, ready, or "we could not load it". The pane must // never fall back to "Inget underlag bifogat" for a row that has a // document_id, which is what the old silent catch produced. - const loadDocument = useCallback(async (itemId: string, documentId: string | null) => { - docRequestRef.current = itemId + const loadDocument = useCallback(async (documentId: string | null) => { + const request = ++docRequestRef.current setDocUrl(null) setDocMime(null) if (!documentId) { @@ -742,13 +745,13 @@ export default function InvoiceInboxWorkspace(_props: WorkspaceComponentProps) { { method: 'GET' }, { timeoutMs: DOCUMENT_FETCH_TIMEOUT_MS, description: `document ${documentId}` }, ) - if (docRequestRef.current !== itemId) return + if (docRequestRef.current !== request) return if (!res.ok) { setDocState('error') return } const { data } = await res.json() - if (docRequestRef.current !== itemId) return + if (docRequestRef.current !== request) return const url: string | null = data?.download_url ?? null // HTML mail underlag renders via the same-origin inline proxy: it // serves text/html with a CSP sandbox header and guaranteed inline @@ -767,39 +770,66 @@ export default function InvoiceInboxWorkspace(_props: WorkspaceComponentProps) { } catch { // Timeout, offline, or an unparseable body. The document itself is // untouched, so the retry in the preview pane is the whole recovery. - if (docRequestRef.current !== itemId) return + if (docRequestRef.current !== request) return setDocState('error') } }, []) const handleSelect = useCallback(async (id: string) => { + const request = ++detailRequestRef.current setSelectedId(id) setSelectedPurchaseId(null) - setSelected(null) - setDocUrl(null) - setDocMime(null) - setDocState('none') - docRequestRef.current = id // Intentionally no auto-scroll: in the vertical-stack layout (below xl) // scrolling the preview into view pushes the list off-screen, and the // user has no obvious way back to pick another item. The row-highlight // + the preview content update are enough feedback that the tap took. + // Seed the detail pane synchronously from the list row already in hand + // (fetchItems returns full rows: status, amounts, extracted fields), and + // start the document load in parallel with the detail GET. Clearing + // `selected` first made every row click flash the no-selection branch + // (onboarding card / "Välj en post") for a full round trip, then run a + // second serialized round trip before the PDF even started loading. + const listRow = items.find((it) => it.id === id) ?? null + if (listRow) { + setSelected(listRow) + void loadDocument(listRow.document_id) + } else { + // Invalidate any in-flight document read: the pane is being cleared, + // and a late resolution must not paint a URL or error onto it. + docRequestRef.current++ + setSelected(null) + setDocUrl(null) + setDocMime(null) + setDocState('none') + } + try { const res = await fetch(`/api/extensions/ext/invoice-inbox/items/${id}`) if (!res.ok) throw await resolveFailure(res) const json = await res.json() const item = json.data as InboxItem + // A newer selection owns the pane now: dropping this response keeps a + // slower earlier fetch (same item or another) from overwriting the + // newest request's detail snapshot. + if (detailRequestRef.current !== request) return setSelected(item) - await loadDocument(id, item.document_id) + if (!listRow) { + await loadDocument(item.document_id) + } else if (item.document_id !== listRow.document_id) { + // The detail row knows a different underlag than the list row we + // seeded from (e.g. processing finished between paint and click). + void loadDocument(item.document_id) + } } catch (err) { + if (detailRequestRef.current !== request) return toast({ title: 'Kunde inte ladda dokumentet', description: failureText(err), variant: 'destructive', }) } - }, [toast, loadDocument]) + }, [items, toast, loadDocument]) // The detail pane renders from its own fetched snapshot (`selected`), so // the realtime refetch updates the list row but would leave a selected @@ -1758,7 +1788,7 @@ export default function InvoiceInboxWorkspace(_props: WorkspaceComponentProps) { docMime={docMime} isProcessing={!!selected.isPlaceholder} loadState={docState} - onRetry={() => { void loadDocument(selected.id, selected.document_id) }} + onRetry={() => { void loadDocument(selected.document_id) }} /> ) : showOnboarding ? (
diff --git a/components/transactions/__tests__/booking-feedback-parity.test.ts b/components/transactions/__tests__/booking-feedback-parity.test.ts index 637ca4f9..ca4c175a 100644 --- a/components/transactions/__tests__/booking-feedback-parity.test.ts +++ b/components/transactions/__tests__/booking-feedback-parity.test.ts @@ -61,10 +61,17 @@ describe('transactions page booking feedback', () => { 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. + // Ångra has already storno-reversed the verifikat server-side. The + // closure-local flag covers the per-row Ångra; the shared undoneIdsRef + // covers "Ångra alla", which runs outside finishBooking's closure. expect(PAGE_SRC).toMatch(/let undone = false/) expect(PAGE_SRC).toMatch(/undone = true/) - expect(PAGE_SRC).toMatch(/if \(!undone\) \{/) + expect(PAGE_SRC).toMatch(/if \(!undone && !undoneIdsRef\.current\.has\(id\)\) \{/) + // Both undo paths record into the shared ref, and a fresh booking clears + // its row's entry again so a re-booked row still gets its delayed patch. + expect(PAGE_SRC).toMatch(/undoneIdsRef\.current\.add\(id\)/) + expect(PAGE_SRC).toMatch(/undoneIdsRef\.current\.add\(undoneId\)/) + expect(PAGE_SRC).toMatch(/undoneIdsRef\.current\.delete\(id\)/) }) it('clears only the finished row\'s spinner', () => { diff --git a/components/ui/destructive-confirm-dialog.tsx b/components/ui/destructive-confirm-dialog.tsx index 71d926ab..797ee941 100644 --- a/components/ui/destructive-confirm-dialog.tsx +++ b/components/ui/destructive-confirm-dialog.tsx @@ -119,21 +119,30 @@ interface ConfirmOptions { interface UseDestructiveConfirmReturn { dialogProps: DestructiveConfirmDialogProps - confirm: (options: ConfirmOptions) => Promise + confirm: (options: ConfirmOptions, action?: () => void | Promise) => Promise } /** * Hook that returns a `confirm()` function as a drop-in replacement for `window.confirm()`. * Returns `Promise`: true if user confirms, false if they cancel. * + * Pass the destructive operation itself as the second argument to run it + * INSIDE the confirm: the dialog stays open with its pending spinner (and + * blocks dismissal) until the action settles, instead of closing on click and + * leaving the fetch to run with no visible state anywhere. The action owns its + * own error feedback (toast); if it throws, `confirm` resolves `false` so a + * caller's success tail is skipped. + * * Usage: * ``` * const { dialogProps, confirm } = useDestructiveConfirm() * * async function handleDelete() { - * const ok = await confirm({ title: '...', description: '...' }) + * const ok = await confirm({ title: '...', description: '...' }, async () => { + * // the DELETE runs while the dialog shows its spinner + * }) * if (!ok) return - * // proceed with deletion + * // confirmed and the action completed * } * * return <> @@ -146,28 +155,57 @@ export function useDestructiveConfirm(): UseDestructiveConfirmReturn { description: '', }) const resolveRef = useRef<((value: boolean) => void) | null>(null) + const actionRef = useRef<(() => void | Promise) | null>(null) + const runningRef = useRef(false) - const confirm = useCallback((opts: ConfirmOptions): Promise => { - setOptions(opts) - setOpen(true) - return new Promise((resolve) => { - resolveRef.current = resolve - }) - }, []) + const confirm = useCallback( + (opts: ConfirmOptions, action?: () => void | Promise): Promise => { + setOptions(opts) + actionRef.current = action ?? null + setOpen(true) + return new Promise((resolve) => { + resolveRef.current = resolve + }) + }, + [], + ) const handleOpenChange = useCallback((v: boolean) => { setOpen(v) - if (!v && resolveRef.current) { - resolveRef.current(false) - resolveRef.current = null + if (!v) { + actionRef.current = null + if (resolveRef.current) { + resolveRef.current(false) + resolveRef.current = null + } } }, []) - const handleConfirm = useCallback(() => { + const handleConfirm = useCallback(async () => { + // Re-entry guard: a second confirm firing while the action is still in + // flight must not resolve the promise early (the dialog disables its + // button on isLoading, this covers the same-tick edge). + if (runningRef.current) return + runningRef.current = true + const action = actionRef.current + actionRef.current = null + let completed = true + if (action) { + try { + // Awaited by the dialog's own handleConfirm, so its isLoading spinner + // shows for the duration and the dialog closes only when this settles. + await action() + } catch { + // The action surfaces its own error (toast); resolving false here + // keeps the caller's post-confirm tail from running on a failure. + completed = false + } + } if (resolveRef.current) { - resolveRef.current(true) + resolveRef.current(completed) resolveRef.current = null } + runningRef.current = false }, []) return { diff --git a/lib/__tests__/concurrency.test.ts b/lib/__tests__/concurrency.test.ts new file mode 100644 index 00000000..63d0d528 --- /dev/null +++ b/lib/__tests__/concurrency.test.ts @@ -0,0 +1,64 @@ +import { describe, it, expect } from 'vitest' +import { mapWithConcurrency } from '@/lib/concurrency' + +describe('mapWithConcurrency', () => { + it('preserves input order in the result', async () => { + const items = [50, 10, 30, 0, 20] + const result = await mapWithConcurrency(items, 3, async (ms) => { + await new Promise((r) => setTimeout(r, ms)) + return ms * 2 + }) + expect(result).toEqual([100, 20, 60, 0, 40]) + }) + + it('never runs more than `limit` workers at once', async () => { + let inFlight = 0 + let peak = 0 + await mapWithConcurrency(Array.from({ length: 20 }, (_, i) => i), 4, async () => { + inFlight++ + peak = Math.max(peak, inFlight) + await new Promise((r) => setTimeout(r, 5)) + inFlight-- + }) + expect(peak).toBeLessThanOrEqual(4) + expect(peak).toBeGreaterThan(1) + }) + + it('processes every item exactly once', async () => { + const seen: number[] = [] + await mapWithConcurrency(Array.from({ length: 13 }, (_, i) => i), 5, async (i) => { + seen.push(i) + }) + expect(seen.slice().sort((a, b) => a - b)).toEqual(Array.from({ length: 13 }, (_, i) => i)) + }) + + it('handles an empty input', async () => { + expect(await mapWithConcurrency([], 4, async () => 1)).toEqual([]) + }) + + it('caps the pool at the item count', async () => { + // 2 items with limit 10 must not spin up idle workers that read past the + // end; the result stays correct. + expect(await mapWithConcurrency([1, 2], 10, async (n) => n + 1)).toEqual([2, 3]) + }) + + it('passes the item index to the worker', async () => { + const idx = await mapWithConcurrency(['a', 'b', 'c'], 2, async (_item, i) => i) + expect(idx).toEqual([0, 1, 2]) + }) + + it('rejects the whole map when a worker rejects (Promise.all semantics)', async () => { + await expect( + mapWithConcurrency([1, 2, 3], 2, async (n) => { + if (n === 2) throw new Error('boom') + return n + }), + ).rejects.toThrow('boom') + }) + + it('rejects a non-positive limit instead of hanging', async () => { + await expect(mapWithConcurrency([1], 0, async (n) => n)).rejects.toThrow( + 'limit must be >= 1', + ) + }) +}) diff --git a/lib/concurrency.ts b/lib/concurrency.ts new file mode 100644 index 00000000..6c244955 --- /dev/null +++ b/lib/concurrency.ts @@ -0,0 +1,33 @@ +/** + * Map over `items` with a bounded worker pool, preserving input order in the + * result array. + * + * Built for client batch actions (bulk categorize/ignore/delete) that used to + * run strictly sequentially: N round trips one after another made a 20-row + * batch take 10-20s. A small pool keeps the server load bounded (never an + * unbounded Promise.all over 100 rows) while finishing in a few round trips. + * + * `fn` is expected to handle its own errors and resolve with a result value + * (the batch handlers resolve per-row success/failure objects); a rejection + * from `fn` rejects the whole map, exactly like Promise.all. + */ +export async function mapWithConcurrency( + items: readonly T[], + limit: number, + fn: (item: T, index: number) => Promise, +): Promise { + if (!Number.isFinite(limit) || limit < 1) { + throw new Error(`mapWithConcurrency: limit must be >= 1, got ${limit}`) + } + const out = new Array(items.length) + let next = 0 + const workers = Array.from({ length: Math.min(limit, items.length) }, async () => { + for (;;) { + const i = next++ + if (i >= items.length) return + out[i] = await fn(items[i], i) + } + }) + await Promise.all(workers) + return out +} diff --git a/messages/en.json b/messages/en.json index c96f74a4..5ad2187c 100644 --- a/messages/en.json +++ b/messages/en.json @@ -5492,6 +5492,15 @@ "batch_clear": "Clear selection", "batch_progress": "Booking {done} of {total}…", "batch_skv_book": "Post selected ({count})", + "batch_done_title": "Done", + "batch_partial_title": "Partially done", + "batch_categorize_done_description": "{count, plural, one {1 transaction posted} other {# transactions posted}}", + "batch_categorize_partial_description": "{success, plural, one {1 transaction posted} other {# transactions posted}}, {failed} failed", + "batch_undo_all": "Undo all", + "batch_undo_all_alt": "Undo all posted transactions", + "batch_undo_done_description": "{count, plural, one {1 posting undone} other {# postings undone}}", + "batch_undo_partial_title": "Partially undone", + "batch_undo_partial_description": "{success, plural, one {1 posting undone} other {# postings undone}}, {failed} could not be undone", "skv_booked_title": "Posted", "skv_booked_description": "Voucher {voucher} was created.", "skv_booked_show": "Show voucher", diff --git a/messages/sv.json b/messages/sv.json index 28af91c0..22a9bca8 100644 --- a/messages/sv.json +++ b/messages/sv.json @@ -5492,6 +5492,15 @@ "batch_clear": "Avmarkera", "batch_progress": "Bokför {done} av {total}…", "batch_skv_book": "Bokför valda ({count})", + "batch_done_title": "Klart", + "batch_partial_title": "Delvis klart", + "batch_categorize_done_description": "{count, plural, one {1 transaktion bokförd} other {# transaktioner bokförda}}", + "batch_categorize_partial_description": "{success, plural, one {1 transaktion bokförd} other {# transaktioner bokförda}}, {failed} misslyckades", + "batch_undo_all": "Ångra alla", + "batch_undo_all_alt": "Ångra alla bokförda transaktioner", + "batch_undo_done_description": "{count, plural, one {1 bokföring ångrad} other {# bokföringar ångrade}}", + "batch_undo_partial_title": "Delvis ångrat", + "batch_undo_partial_description": "{success, plural, one {1 bokföring ångrad} other {# bokföringar ångrade}}, {failed} kunde inte ångras", "skv_booked_title": "Bokförd", "skv_booked_description": "Verifikat {voucher} skapades.", "skv_booked_show": "Visa verifikat",