From c62d00bcb39bbd3bdc596ab874800e3050af8597 Mon Sep 17 00:00:00 2001 From: Jakob Wennberg <149234542+jakobwennberg@users.noreply.github.com> Date: Mon, 27 Jul 2026 14:59:33 +0200 Subject: [PATCH] feat(invoices): preview invoices and underlag in the browser instead of downloading (#1228) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewing an invoice or a verifikat bilaga meant saving a file and opening it from the Downloads folder (user request, christian@odinaero.se 2026-07-25). - GET /api/invoices/[id]/pdf accepts ?disposition=inline and serves the PDF for in-browser review; anything else keeps the download behaviour every existing caller relies on. The filename still travels in the header, so the browser viewer's own save action produces the same name as the download button, and nosniff pins the content type. - The invoice detail page gets a "Förhandsgranska" action next to "Ladda ner PDF". It resolves the document through the same resolveInvoicePdfSource path as the download, so preview cannot become the shortcut that presents a re-render as the invoice the customer received: the archived delivery wins, a re-render is shown with its caveat, and an unreadable delivery history still asks instead of guessing. The archive dialog now remembers whether the user asked to view or to save, and its fallback does that. - DocumentViewButton (supplier-invoice underlag, staged agent previews) points at the existing /api/documents/:id/inline proxy, so bilagor render in the browser. Navigation now happens straight from the click, so the signed-URL fetch and its popup-blocker workaround are gone. - The three re-render caveat strings and the two archive-dialog descriptions lose their "you downloaded" wording so they stay true for both actions; five new keys in sv + en. Tests: route cases for the default, inline and unknown disposition values; invoiceRerenderUrl cases for both modes and id encoding. npm test 11364 passed, lint 0 errors. Button row screenshotted against the design system (pill outline, Eye icon) via a temporary sandbox route. Closes #1190 Co-authored-by: Claude Opus 5 (1M context) --- DECISIONS.md | 1 + app/(dashboard)/invoices/[id]/page.tsx | 124 +++++++++++++++--- .../invoices/[id]/pdf/__tests__/route.test.ts | 46 +++++++ app/api/invoices/[id]/pdf/route.ts | 17 ++- components/bookkeeping/DocumentViewButton.tsx | 69 ++++------ .../__tests__/invoice-pdf-source.test.ts | 22 ++++ lib/invoices/invoice-pdf-source.ts | 10 +- messages/en.json | 15 ++- messages/sv.json | 15 ++- 9 files changed, 238 insertions(+), 81 deletions(-) diff --git a/DECISIONS.md b/DECISIONS.md index 6694f821..8bc15a47 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -589,3 +589,4 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-07-27] mcp-oauth consent form action is HTML-escaped even though the CodeQL js/reflected-xss finding is not exploitable (WHATWG URL parsing already percent-encodes " < > in the query component): & is not in that encode set so the attribute was emitting invalid raw ampersands, and resting the page on an unstated parser-normalisation invariant is one refactor away from being wrong. [2026-07-27] Compliance-review artifact unpacks to runner.temp instead of the workspace root: extracting fork-influenced content over the trusted checkout, with AWS secrets in scope, was safe only because stage 1 happens to write fixed filenames; moving it makes overwrite unreachable by construction. [2026-07-27] Supplier-invoice 'overdue' stays a stored status, made symmetric instead of derived (#1206): added approved_at as the durable attest marker and an un-flip branch in update_overdue_supplier_invoices(), rather than computing overdue at read time. Computing it would have touched every list/filter/report query that reads status plus the v1 API contract; the symmetric-cron fix is the same user-visible outcome at a fraction of the blast radius. +[2026-07-27] In-browser preview (#1190) opens a new tab against an inline-disposition URL instead of an in-app viewer surface for invoice PDFs: the browser's native PDF viewer already does the job, and reusing resolveInvoicePdfSource keeps the archived-vs-rerender distinction intact, which a separate preview path would have had to duplicate. diff --git a/app/(dashboard)/invoices/[id]/page.tsx b/app/(dashboard)/invoices/[id]/page.tsx index dfa99d90..d04d84c3 100644 --- a/app/(dashboard)/invoices/[id]/page.tsx +++ b/app/(dashboard)/invoices/[id]/page.tsx @@ -33,6 +33,7 @@ import { CheckCircle, FileText, Download, + Eye, XCircle, Mail, ReceiptText, @@ -126,6 +127,10 @@ export default function InvoiceDetailPage({ params }: { params: Promise<{ id: st // Set when the archived copy could not be produced, so the user is asked // instead of being handed a substitute that looks like the original. const [pdfArchiveIssue, setPdfArchiveIssue] = useState<'history' | 'document' | null>(null) + // Which action raised that question: the dialog's fallback must do what the + // user originally asked for (open in the browser vs save the file), not + // silently switch mechanism (#1190). + const [pdfIntent, setPdfIntent] = useState<'download' | 'preview'>('download') // Payment history backing the new Betalningsstatus card. Fetched alongside // the invoice itself so the card stays in sync with paid_amount / // remaining_amount on the invoice row. @@ -483,6 +488,7 @@ export default function InvoiceDetailPage({ params }: { params: Promise<{ id: st if (!invoice) return if (source.kind === 'unavailable') { + setPdfIntent('download') setPdfArchiveIssue('history') return } @@ -496,6 +502,7 @@ export default function InvoiceDetailPage({ params }: { params: Promise<{ id: st // A missing archive is not a generation failure and must not offer a // silent substitute: hand the choice back to the user. if (source.kind === 'archived') { + setPdfIntent('download') setPdfArchiveIssue('document') return } @@ -551,6 +558,60 @@ export default function InvoiceDetailPage({ params }: { params: Promise<{ id: st ) } + /** + * Show one specific document in the browser instead of saving it (#1190): + * granskning should not require leaving the app for the Downloads folder. + * + * Which document may be shown is the same question as for the download, and + * gets the same answer: the archived delivery when it exists, a re-render only + * with the caveat spelled out, and a question rather than a guess when the + * delivery history could not be read. Only the mechanism differs, so a tab is + * opened synchronously (before any await) to keep the click's user activation + * and stay clear of the popup blocker. + */ + function runInvoicePreview(source: InvoicePdfSource) { + if (!invoice) return + + if (source.kind === 'unavailable') { + setPdfIntent('preview') + setPdfArchiveIssue('history') + return + } + + const url = + source.kind === 'archived' ? source.url : invoiceRerenderUrl(invoice.id, { inline: true }) + + if (!window.open(url, '_blank', 'noopener,noreferrer')) { + toast({ + title: t('pdf_preview_blocked_title'), + description: t('pdf_preview_blocked_description'), + variant: 'destructive', + }) + return + } + + const caveat = invoiceDocumentCaveat(source) + if (caveat) { + toast({ + title: t('pdf_rerender_preview_title'), + description: t(RERENDER_CAVEAT_KEYS[caveat]), + }) + } + } + + function previewPDF() { + if (!invoice) return + setPdfArchiveIssue(null) + runInvoicePreview( + resolveInvoicePdfSource({ + invoiceId: invoice.id, + invoiceStatus: invoice.status, + deliveriesLoaded: !deliveriesUnreadable, + deliveries, + }), + ) + } + // "Försök igen" from the archive dialog. Re-reads the delivery history first // so a transient list failure resolves back to the archived copy instead of // getting stuck on the stale empty state. @@ -560,14 +621,21 @@ export default function InvoiceDetailPage({ params }: { params: Promise<{ id: st const result = await retryLoadDeliveries() setIsDownloading(false) setPdfArchiveIssue(null) - await runInvoiceDownload( - resolveInvoicePdfSource({ - invoiceId: invoice.id, - invoiceStatus: invoice.status, - deliveriesLoaded: result.ok, - deliveries: result.deliveries, - }), - ) + const source = resolveInvoicePdfSource({ + invoiceId: invoice.id, + invoiceStatus: invoice.status, + deliveriesLoaded: result.ok, + deliveries: result.deliveries, + }) + // The retry is a second attempt at what the user asked for, not a switch to + // the other mechanism. A preview retry re-resolves the source first, so the + // tab it opens is no longer inside the original click's activation window; + // a blocked popup is reported rather than swallowed. + if (pdfIntent === 'preview') { + runInvoicePreview(source) + return + } + await runInvoiceDownload(source) } // The user explicitly accepted a re-render after being told it is not the @@ -575,11 +643,16 @@ export default function InvoiceDetailPage({ params }: { params: Promise<{ id: st async function downloadRerenderAnyway() { if (!invoice) return setPdfArchiveIssue(null) - await runInvoiceDownload({ - kind: 'rerender', + const source = { + kind: 'rerender' as const, url: invoiceRerenderUrl(invoice.id), - reason: 'archive_unreachable', - }) + reason: 'archive_unreachable' as const, + } + if (pdfIntent === 'preview') { + runInvoicePreview(source) + return + } + await runInvoiceDownload(source) } // Open the finalize dialog and peek the next F-number so the user can see @@ -896,14 +969,21 @@ export default function InvoiceDetailPage({ params }: { params: Promise<{ id: st )} {/* No own PDF for a received self-billing invoice: the verifikationsunderlag is the document the customer sent us. */} {!isSelfBilled && ( - + <> + {/* Review in the browser (#1190); the download stays for keeping a copy. */} + + + )} @@ -1702,7 +1782,9 @@ export default function InvoiceDetailPage({ params }: { params: Promise<{ id: st onClick={downloadRerenderAnyway} disabled={isDownloading} > - {t('pdf_archive_issue_rerender')} + {pdfIntent === 'preview' + ? t('pdf_archive_issue_rerender_preview') + : t('pdf_archive_issue_rerender')} ) diff --git a/lib/invoices/__tests__/invoice-pdf-source.test.ts b/lib/invoices/__tests__/invoice-pdf-source.test.ts index 42c49832..2d4b9cb3 100644 --- a/lib/invoices/__tests__/invoice-pdf-source.test.ts +++ b/lib/invoices/__tests__/invoice-pdf-source.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from 'vitest' import { invoiceDocumentCaveat, + invoiceRerenderUrl, resolveInvoicePdfSource, type InvoicePdfDelivery, } from '@/lib/invoices/invoice-pdf-source' @@ -209,3 +210,24 @@ describe('invoiceDocumentCaveat', () => { ).toBeNull() }) }) + +describe('invoiceRerenderUrl', () => { + it('defaults to the download endpoint', () => { + expect(invoiceRerenderUrl(INVOICE_ID)).toBe(`/api/invoices/${INVOICE_ID}/pdf`) + }) + + // #1190: previewing must not silently become a download, and the archived + // delivery URL above is already an inline proxy, so both source kinds can be + // opened in the browser the same way. + it('asks for an inline response when previewing', () => { + expect(invoiceRerenderUrl(INVOICE_ID, { inline: true })).toBe( + `/api/invoices/${INVOICE_ID}/pdf?disposition=inline`, + ) + }) + + it('encodes the invoice id', () => { + expect(invoiceRerenderUrl('a/b', { inline: true })).toBe( + '/api/invoices/a%2Fb/pdf?disposition=inline', + ) + }) +}) diff --git a/lib/invoices/invoice-pdf-source.ts b/lib/invoices/invoice-pdf-source.ts index cbe6468c..d6e78f9e 100644 --- a/lib/invoices/invoice-pdf-source.ts +++ b/lib/invoices/invoice-pdf-source.ts @@ -86,8 +86,14 @@ export type InvoicePdfSource = reason: 'delivery_history_unreadable' } -export function invoiceRerenderUrl(invoiceId: string): string { - return `/api/invoices/${encodeURIComponent(invoiceId)}/pdf` +/** + * The re-render endpoint. `inline` asks it to serve the PDF for in-browser + * review instead of a download (#1190); the archived-delivery URL below is + * already an inline proxy, so both source kinds can be previewed the same way. + */ +export function invoiceRerenderUrl(invoiceId: string, options?: { inline?: boolean }): string { + const base = `/api/invoices/${encodeURIComponent(invoiceId)}/pdf` + return options?.inline ? `${base}?disposition=inline` : base } /** diff --git a/messages/en.json b/messages/en.json index 8acb7496..55795eab 100644 --- a/messages/en.json +++ b/messages/en.json @@ -3127,6 +3127,7 @@ "mark_as_paid": "Mark as paid", "copy_invoice": "Copy invoice", "download_pdf": "Download PDF", + "preview_pdf": "Preview", "viewer_disabled_tooltip": "You only have read-only access to this company", "customer_card_title": "Customer", "org_number_label": "Org. no: {value}", @@ -3245,15 +3246,19 @@ "pdf_downloaded_draft": "The draft has been downloaded", "pdf_download_failed_title": "Could not download PDF", "pdf_rerender_downloaded_title": "Freshly generated PDF downloaded", - "pdf_rerender_reason_sent_outside": "The latest send happened outside Accounted, so there is no archived copy of that particular send. The file you downloaded was generated just now, from today's data, template and logo.", - "pdf_rerender_reason_no_archive": "No archived copy exists for this invoice; it was sent before delivery history was recorded. The file you downloaded was generated just now, from today's data, template and logo.", - "pdf_rerender_reason_archive_unreachable": "The archived copy could not be retrieved. The file you downloaded was generated just now, from today's data, template and logo, and is not necessarily identical to the one the customer received.", + "pdf_rerender_preview_title": "Showing a freshly generated PDF", + "pdf_preview_blocked_title": "Could not open the preview", + "pdf_preview_blocked_description": "Allow pop-up windows for Accounted in your browser and try again.", + "pdf_rerender_reason_sent_outside": "The latest send happened outside Accounted, so there is no archived copy of that particular send. The file was generated just now, from today's data, template and logo.", + "pdf_rerender_reason_no_archive": "No archived copy exists for this invoice; it was sent before delivery history was recorded. The file was generated just now, from today's data, template and logo.", + "pdf_rerender_reason_archive_unreachable": "The archived copy could not be retrieved. The file was generated just now, from today's data, template and logo, and is not necessarily identical to the one the customer received.", "pdf_archive_issue_title": "The archived copy could not be retrieved", - "pdf_archive_issue_history_desc": "The delivery history could not be loaded, so Accounted does not know which PDF the customer actually received. Nothing was downloaded. A freshly generated PDF is not the same document: it is built from today's data, template and logo.", - "pdf_archive_issue_document_desc": "The archived PDF that was sent to the customer could not be retrieved right now. Nothing was downloaded. A freshly generated PDF is not the same document: it is built from today's data, template and logo.", + "pdf_archive_issue_history_desc": "The delivery history could not be loaded, so Accounted does not know which PDF the customer actually received. Nothing was fetched. A freshly generated PDF is not the same document: it is built from today's data, template and logo.", + "pdf_archive_issue_document_desc": "The archived PDF that was sent to the customer could not be retrieved right now. Nothing was fetched. A freshly generated PDF is not the same document: it is built from today's data, template and logo.", "pdf_archive_issue_cancel": "Cancel", "pdf_archive_issue_retry": "Try again", "pdf_archive_issue_rerender": "Download freshly generated PDF anyway", + "pdf_archive_issue_rerender_preview": "Show the freshly generated PDF anyway", "cancel_failed_fallback": "Could not cancel the invoice", "cancelled_toast_title": "Invoice cancelled", "cancelled_with_number": "Invoice {number} has been cancelled. The number is kept in the series.", diff --git a/messages/sv.json b/messages/sv.json index 3f2b6373..e198e36d 100644 --- a/messages/sv.json +++ b/messages/sv.json @@ -3127,6 +3127,7 @@ "mark_as_paid": "Markera som betald", "copy_invoice": "Kopiera faktura", "download_pdf": "Ladda ner PDF", + "preview_pdf": "Förhandsgranska", "viewer_disabled_tooltip": "Du har endast läsbehörighet i detta företag", "customer_card_title": "Kund", "org_number_label": "Org.nr: {value}", @@ -3245,15 +3246,19 @@ "pdf_downloaded_draft": "Utkastet har laddats ner", "pdf_download_failed_title": "Kunde inte ladda ner PDF", "pdf_rerender_downloaded_title": "Nyskapad PDF nedladdad", - "pdf_rerender_reason_sent_outside": "Det senaste utskicket gjordes utanför Accounted, så det finns ingen arkiverad kopia av just det utskicket. Filen du laddade ner skapades nyss, utifrån dagens uppgifter, mall och logotyp.", - "pdf_rerender_reason_no_archive": "Ingen arkiverad kopia finns för den här fakturan; den skickades innan utskickshistoriken började sparas. Filen du laddade ner skapades nyss, utifrån dagens uppgifter, mall och logotyp.", - "pdf_rerender_reason_archive_unreachable": "Den arkiverade kopian kunde inte hämtas. Filen du laddade ner skapades nyss, utifrån dagens uppgifter, mall och logotyp, och är inte nödvändigtvis identisk med den kunden fick.", + "pdf_rerender_preview_title": "Nyskapad PDF visas", + "pdf_preview_blocked_title": "Kunde inte öppna förhandsgranskningen", + "pdf_preview_blocked_description": "Tillåt popupfönster för Accounted i webbläsaren och försök igen.", + "pdf_rerender_reason_sent_outside": "Det senaste utskicket gjordes utanför Accounted, så det finns ingen arkiverad kopia av just det utskicket. Filen skapades nyss, utifrån dagens uppgifter, mall och logotyp.", + "pdf_rerender_reason_no_archive": "Ingen arkiverad kopia finns för den här fakturan; den skickades innan utskickshistoriken började sparas. Filen skapades nyss, utifrån dagens uppgifter, mall och logotyp.", + "pdf_rerender_reason_archive_unreachable": "Den arkiverade kopian kunde inte hämtas. Filen skapades nyss, utifrån dagens uppgifter, mall och logotyp, och är inte nödvändigtvis identisk med den kunden fick.", "pdf_archive_issue_title": "Den arkiverade kopian kunde inte hämtas", - "pdf_archive_issue_history_desc": "Utskickshistoriken kunde inte läsas in, så Accounted vet inte vilken PDF kunden faktiskt fick. Ingenting laddades ner. En nyskapad PDF är inte samma dokument: den bygger på dagens uppgifter, mall och logotyp.", - "pdf_archive_issue_document_desc": "Den arkiverade PDF:en som skickades till kunden kunde inte hämtas just nu. Ingenting laddades ner. En nyskapad PDF är inte samma dokument: den bygger på dagens uppgifter, mall och logotyp.", + "pdf_archive_issue_history_desc": "Utskickshistoriken kunde inte läsas in, så Accounted vet inte vilken PDF kunden faktiskt fick. Ingenting har hämtats. En nyskapad PDF är inte samma dokument: den bygger på dagens uppgifter, mall och logotyp.", + "pdf_archive_issue_document_desc": "Den arkiverade PDF:en som skickades till kunden kunde inte hämtas just nu. Ingenting har hämtats. En nyskapad PDF är inte samma dokument: den bygger på dagens uppgifter, mall och logotyp.", "pdf_archive_issue_cancel": "Avbryt", "pdf_archive_issue_retry": "Försök igen", "pdf_archive_issue_rerender": "Ladda ner nyskapad PDF ändå", + "pdf_archive_issue_rerender_preview": "Visa nyskapad PDF ändå", "cancel_failed_fallback": "Kunde inte makulera fakturan", "cancelled_toast_title": "Faktura makulerad", "cancelled_with_number": "Faktura {number} har makulerats. Numret behålls i serien.",