diff --git a/app/(dashboard)/invoices/[id]/page.tsx b/app/(dashboard)/invoices/[id]/page.tsx index 9417360a..7f41f14b 100644 --- a/app/(dashboard)/invoices/[id]/page.tsx +++ b/app/(dashboard)/invoices/[id]/page.tsx @@ -86,6 +86,7 @@ import { import type { Invoice, InvoiceItem, InvoiceStatus, InvoiceReminder, InvoiceDocumentType } from '@/types' import type { InvoiceWithRelations } from '@/components/invoices/types' import { getErrorMessage as getUserErrorMessage, type ErrorLocale } from '@/lib/errors/get-error-message' +import { openDeferredTab } from '@/lib/browser/deferred-tab' import { useBranding } from '@/lib/branding/brand-context' import { getCountryName } from '@/lib/vat/country-codes' import { DetailPageSkeleton } from '@/components/common/DetailPageSkeleton' @@ -107,6 +108,11 @@ const PEPPOL_STATUS_KEYS = new Set([ ]) const PEPPOL_SENDABLE_STATUSES = new Set(['draft', 'sent', 'overdue']) +// How long the preview waits for the PDF route to say whether it will render +// before giving up and closing the placeholder tab. Generous: a cold +// serverless start plus the invoice and settings reads, not the render itself. +const PDF_PROBE_TIMEOUT_MS = 20_000 + // Why the downloaded file is not the invoice the customer received. One key // per reason: "no archived copy exists" and "the archive could not be reached" // are different facts and must not be told as the same story. @@ -824,7 +830,12 @@ export default function InvoiceDetailPage({ params }: { params: Promise<{ id: st setPdfArchiveIssue('document') return } - throw new Error(t('pdf_generate_failed')) + toast({ + title: t('pdf_download_failed_title'), + description: await describePdfRouteFailure(response), + variant: 'destructive', + }) + return } const blob = await response.blob() @@ -863,6 +874,22 @@ export default function InvoiceDetailPage({ params }: { params: Promise<{ id: st } } + /** + * Turn a refusal from the PDF route into the sentence the user needs. The + * route answers with the structured envelope ({ error: { code, message, + * details } }), and the mapper reads it whole: for a missing payment + * account it names what is missing for the invoice's currency and where to + * add it, instead of the generic "Kunde inte generera PDF". + */ + async function describePdfRouteFailure(response: Response): Promise { + const body: unknown = await response.json().catch(() => null) + return getUserErrorMessage(body ?? new Error(t('pdf_generate_failed')), { + locale: locale as ErrorLocale, + context: 'invoice', + statusCode: response.status, + }) + } + async function downloadPDF() { if (!invoice) return setPdfArchiveIssue(null) @@ -1105,8 +1132,20 @@ export default function InvoiceDetailPage({ params }: { params: Promise<{ id: st * 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. + * + * The tab comes from openDeferredTab: window.open() with 'noopener' returns + * null by spec even on success, so the direct call this replaced fired the + * "popup blocked" toast on every open (#1613 had the same defect). + * + * A re-render is probed before the tab is pointed at it: the PDF route + * refuses with a JSON envelope when the invoice cannot be rendered (no + * payment account for its currency, for one), and navigating the tab + * straight to the route showed that JSON raw. The probe runs the same + * checks without rendering; on refusal the tab is closed again and the + * message goes in a toast, otherwise the tab gets the real inline URL, so + * the viewer keeps the invoice filename and the address survives a reload. */ - function runInvoicePreview(source: InvoicePdfSource) { + async function runInvoicePreview(source: InvoicePdfSource) { if (!invoice) return if (source.kind === 'unavailable') { @@ -1115,10 +1154,8 @@ export default function InvoiceDetailPage({ params }: { params: Promise<{ id: st return } - const url = - source.kind === 'archived' ? source.url : invoiceRerenderUrl(invoice.id, { inline: true }) - - if (!window.open(url, '_blank', 'noopener,noreferrer')) { + const tab = openDeferredTab(t('pdf_preview_opening')) + if (tab.blocked) { toast({ title: t('pdf_preview_blocked_title'), description: t('pdf_preview_blocked_description', { appName }), @@ -1127,6 +1164,42 @@ export default function InvoiceDetailPage({ params }: { params: Promise<{ id: st return } + if (source.kind === 'archived') { + tab.navigate(source.url) + return + } + + try { + // Bounded: a stalled probe must not leave the placeholder tab open with + // no word from the app. The abort lands in the catch below. + const probe = await fetch(invoiceRerenderUrl(invoice.id, { inline: true, probe: true }), { + signal: AbortSignal.timeout(PDF_PROBE_TIMEOUT_MS), + }) + if (!probe.ok) { + tab.close() + toast({ + title: t('pdf_preview_failed_title'), + description: await describePdfRouteFailure(probe), + variant: 'destructive', + }) + return + } + } catch (error) { + tab.close() + toast({ + title: t('pdf_preview_failed_title'), + description: error instanceof Error + ? getUserErrorMessage(error, { locale: locale as ErrorLocale, context: 'invoice' }) + : t('fallback_try_again'), + variant: 'destructive', + }) + return + } + + // The user may have closed the placeholder tab while the probe ran; then + // nothing is shown and the caveat about what would have been shown is moot. + if (!tab.navigate(invoiceRerenderUrl(invoice.id, { inline: true }))) return + const caveat = invoiceDocumentCaveat(source) if (caveat) { toast({ @@ -1139,7 +1212,7 @@ export default function InvoiceDetailPage({ params }: { params: Promise<{ id: st function previewPDF() { if (!invoice) return setPdfArchiveIssue(null) - runInvoicePreview( + void runInvoicePreview( resolveInvoicePdfSource({ invoiceId: invoice.id, invoiceStatus: invoice.status, @@ -1169,7 +1242,7 @@ export default function InvoiceDetailPage({ params }: { params: Promise<{ id: st // 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) + await runInvoicePreview(source) return } await runInvoiceDownload(source) @@ -1186,7 +1259,7 @@ export default function InvoiceDetailPage({ params }: { params: Promise<{ id: st reason: 'archive_unreachable' as const, } if (pdfIntent === 'preview') { - runInvoicePreview(source) + await runInvoicePreview(source) return } await runInvoiceDownload(source) diff --git a/app/api/invoices/[id]/pdf/__tests__/route.test.ts b/app/api/invoices/[id]/pdf/__tests__/route.test.ts index c3da6519..e8c4ca3c 100644 --- a/app/api/invoices/[id]/pdf/__tests__/route.test.ts +++ b/app/api/invoices/[id]/pdf/__tests__/route.test.ts @@ -158,6 +158,59 @@ describe('GET /api/invoices/[id]/pdf', () => { expect(renderToBufferMock).not.toHaveBeenCalled() }) + // The in-app preview probes before pointing a tab at the inline URL, so a + // refusal is shown as a message in the app instead of as raw JSON in the tab. + describe('?probe=1', () => { + it('answers 204 without rendering when the PDF would be served', async () => { + enqueue({ data: invoice, error: null }) + enqueue({ data: company, error: null }) + + const response = await GET( + createMockRequest('/api/invoices/invoice-1/pdf', { + searchParams: { disposition: 'inline', probe: '1' }, + }), + createMockRouteParams({ id: 'invoice-1' }), + ) + + expect(response.status).toBe(204) + expect(response.headers.get('Cache-Control')).toBe('private, no-store') + expect(renderToBufferMock).not.toHaveBeenCalled() + }) + + it('returns the same refusal envelope the render would', async () => { + enqueue({ data: { ...invoice, currency: 'EUR' }, error: null }) + enqueue({ data: { ...company, invoice_payment_accounts: {} }, error: null }) + + const response = await GET( + createMockRequest('/api/invoices/invoice-1/pdf', { + searchParams: { disposition: 'inline', probe: '1' }, + }), + createMockRouteParams({ id: 'invoice-1' }), + ) + const body = await response.json() + + expect(response.status).toBe(400) + expect(body.error.code).toBe('INVOICE_SEND_PAYMENT_ACCOUNT_MISSING') + expect(body.error.details.currency).toBe('EUR') + expect(renderToBufferMock).not.toHaveBeenCalled() + }) + + it('ignores any other probe value and renders', async () => { + enqueue({ data: invoice, error: null }) + enqueue({ data: company, error: null }) + + const response = await GET( + createMockRequest('/api/invoices/invoice-1/pdf', { + searchParams: { probe: 'yes' }, + }), + createMockRouteParams({ id: 'invoice-1' }), + ) + + expect(response.status).toBe(200) + expect(renderToBufferMock).toHaveBeenCalledTimes(1) + }) + }) + // #1693: the betalningsbekräftelse variant. Same render, refused unless the // faktura is fully paid, named as a payment confirmation, archive untouched. describe('?variant=paid', () => { diff --git a/app/api/invoices/[id]/pdf/route.ts b/app/api/invoices/[id]/pdf/route.ts index 8dc19008..08ced9c0 100644 --- a/app/api/invoices/[id]/pdf/route.ts +++ b/app/api/invoices/[id]/pdf/route.ts @@ -37,6 +37,11 @@ function resolveVariant(request: Request): 'invoice' | 'paid' { return requested === 'paid' ? 'paid' : 'invoice' } +/** `?probe=1` asks only whether the render would be refused; see the handler. */ +function isProbe(request: Request): boolean { + return new URL(request.url).searchParams.get('probe') === '1' +} + export const GET = withRouteContext<{ params: Promise<{ id: string }> }>( 'invoice.pdf', async (request, { supabase, companyId, log, requestId }, { params }) => { @@ -93,6 +98,15 @@ export const GET = withRouteContext<{ params: Promise<{ id: string }> }>( })) } + // `?probe=1`: every refusal above has been checked, so answer without + // rendering. The in-app preview asks this first, from a fetch whose JSON + // refusal it can show as a message, and only then points the new tab at the + // real inline URL. Navigating the tab straight here showed the refusal + // envelope as raw JSON in that tab. + if (isProbe(request)) { + return new NextResponse(null, { status: 204, headers: PRIVATE_NO_STORE_HEADERS }) + } + // Sort items by sort_order const items = (invoice.items as InvoiceItem[]).sort((a, b) => a.sort_order - b.sort_order) diff --git a/components/settings/InvoicePreviewCard.tsx b/components/settings/InvoicePreviewCard.tsx index 5c0da9a3..a9a2c020 100644 --- a/components/settings/InvoicePreviewCard.tsx +++ b/components/settings/InvoicePreviewCard.tsx @@ -90,8 +90,22 @@ export function InvoicePreviewCard({ settings }: InvoicePreviewCardProps) { }) if (!response.ok) { - const body = await response.json().catch(() => null) - throw new Error(body?.error || `HTTP ${response.status}`) + // The route answers with the structured envelope ({ error: { code, + // message, details } }); the mapper reads it whole and says exactly + // what is missing (e.g. no bankgiro for a SEK invoice). Wrapping + // `body.error` in `new Error()` stringified the object and left only + // the generic "Kunde inte hantera fakturan" fallback. + const body: unknown = await response.json().catch(() => null) + if (cancelled) return + setError( + getErrorMessage(body ?? new Error(`HTTP ${response.status}`), { + locale, + context: 'invoice', + statusCode: response.status, + }), + ) + setIsLoading(false) + return } const blob = await response.blob() diff --git a/lib/errors/__tests__/get-error-message.test.ts b/lib/errors/__tests__/get-error-message.test.ts index 01ab9aa9..3303984e 100644 --- a/lib/errors/__tests__/get-error-message.test.ts +++ b/lib/errors/__tests__/get-error-message.test.ts @@ -605,6 +605,19 @@ describe('getErrorMessage: INVOICE_SEND_PAYMENT_ACCOUNT_MISSING (#2126)', () => const unknown = getErrorMessage(envelope('JPY'), { statusCode: 400 }) expect(unknown).toBe(getErrorEntry('INVOICE_SEND_PAYMENT_ACCOUNT_MISSING')!.message_sv) }) + + // The preview surfaces (settings dialog, invoice page) hand the whole + // parsed body to the mapper with the invoice context. The context fallback + // must not shadow the specific text, and the stringified-object shape the + // old `new Error(body.error)` produced must be recognisably the bug. + it('preview surfaces: whole body + invoice context still yields the specific text', () => { + const msg = getErrorMessage(envelope('SEK'), { statusCode: 400, context: 'invoice', locale: 'sv' }) + expect(msg).toContain('bankgiro') + expect(msg).not.toBe('Kunde inte hantera fakturan. Försök igen.') + + const mangled = getErrorMessage(new Error(String(envelope('SEK').error)), { context: 'invoice' }) + expect(mangled).toBe('Kunde inte hantera fakturan. Försök igen.') + }) }) diff --git a/lib/invoices/__tests__/invoice-pdf-source.test.ts b/lib/invoices/__tests__/invoice-pdf-source.test.ts index 87696c39..922eb8fb 100644 --- a/lib/invoices/__tests__/invoice-pdf-source.test.ts +++ b/lib/invoices/__tests__/invoice-pdf-source.test.ts @@ -232,6 +232,18 @@ describe('invoiceRerenderUrl', () => { '/api/invoices/a%2Fb/pdf?disposition=inline', ) }) + + // The preview probes the route before pointing the tab at it, so a refusal + // (no payment account for the currency) is shown in the app instead of as + // raw JSON in the new tab. + it('adds the probe flag next to the inline disposition', () => { + expect(invoiceRerenderUrl(INVOICE_ID, { inline: true, probe: true })).toBe( + `/api/invoices/${INVOICE_ID}/pdf?disposition=inline&probe=1`, + ) + expect(invoiceRerenderUrl(INVOICE_ID, { probe: true })).toBe( + `/api/invoices/${INVOICE_ID}/pdf?probe=1`, + ) + }) }) // #1693: the betalningsbekräftelse is always a re-render and always says so, diff --git a/lib/invoices/invoice-pdf-source.ts b/lib/invoices/invoice-pdf-source.ts index 4770db8a..4beaa315 100644 --- a/lib/invoices/invoice-pdf-source.ts +++ b/lib/invoices/invoice-pdf-source.ts @@ -97,9 +97,20 @@ export type InvoicePdfSource = * 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 { +export function invoiceRerenderUrl( + invoiceId: string, + options?: { inline?: boolean; probe?: boolean }, +): string { const base = `/api/invoices/${encodeURIComponent(invoiceId)}/pdf` - return options?.inline ? `${base}?disposition=inline` : base + const params = new URLSearchParams() + if (options?.inline) params.set('disposition', 'inline') + // `probe=1` runs every refusal check the render would run and answers 204 + // without rendering. The preview asks this first so a refusal can be shown + // as a message in the app, and the tab is then pointed at the real inline + // URL, keeping the filename and a reloadable address. + if (options?.probe) params.set('probe', '1') + const query = params.toString() + return query ? `${base}?${query}` : base } /** diff --git a/messages/en.json b/messages/en.json index 5d9b0477..15ba341b 100644 --- a/messages/en.json +++ b/messages/en.json @@ -4208,6 +4208,8 @@ "peppol_send_limit_reached": "The company has used its Peppol sends. Contact support for more.", "pdf_rerender_downloaded_title": "Freshly generated PDF downloaded", "pdf_rerender_preview_title": "Showing a freshly generated PDF", + "pdf_preview_failed_title": "Could not preview the invoice", + "pdf_preview_opening": "Opening the invoice...", "pdf_preview_blocked_title": "Could not open the preview", "pdf_preview_blocked_description": "Allow pop-up windows for {appName} in your browser and try again.", "pdf_rerender_reason_sent_outside": "The latest send happened outside {appName}, so there is no archived copy of that particular send. The file was generated just now, from today's data, template and logo.", diff --git a/messages/sv.json b/messages/sv.json index 16d5e229..fc06cd42 100644 --- a/messages/sv.json +++ b/messages/sv.json @@ -4208,6 +4208,8 @@ "peppol_send_limit_reached": "Bolagets Peppol-sändningar är slut. Hör av dig till support för fler.", "pdf_rerender_downloaded_title": "Nyskapad PDF nedladdad", "pdf_rerender_preview_title": "Nyskapad PDF visas", + "pdf_preview_failed_title": "Kunde inte förhandsgranska fakturan", + "pdf_preview_opening": "Öppnar fakturan...", "pdf_preview_blocked_title": "Kunde inte öppna förhandsgranskningen", "pdf_preview_blocked_description": "Tillåt popupfönster för {appName} i webbläsaren och försök igen.", "pdf_rerender_reason_sent_outside": "Det senaste utskicket gjordes utanför {appName}, så det finns ingen arkiverad kopia av just det utskicket. Filen skapades nyss, utifrån dagens uppgifter, mall och logotyp.",