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",