fix(invoices): say what is missing when an invoice preview cannot be rendered (#2303)

* fix(invoices): say what is missing when an invoice preview cannot be rendered The PDF route already refuses with a structured envelope that names exactly what the invoice lacks (no bankgiro, plusgiro, Swish or bank account for a SEK invoice; no IBAN account for a foreign currency) and where to add it. Two clients threw that away: - The settings preview dialog (Inställningar -> Fakturering -> Förhandsvisa faktura) wrapped the envelope's inner object in new Error(), which stringified it to "[object Object]" and left only the generic "Kunde inte hantera fakturan. Försök igen." fallback. The parsed body now goes to the error mapper whole, with the invoice context and status. - The invoice page's Förhandsgranska navigated a new tab straight to the re-render URL, so a 400 showed the raw JSON in that tab. The tab is now opened blank inside the click's activation window, the PDF is fetched first, and the tab gets the PDF as a blob URL or is closed again with the refusal in a toast. The archived delivery copy keeps the direct open. Ladda ner on the same page had a fixed "Kunde inte generera PDF" for re-render refusals and now maps the body the same way. Regression test on the mapper covers the exact call shape the two surfaces use and pins the old mangled shape as the fallback it produced. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GUdZPW46a16GWUdgt2qSZA * fix(invoices): probe the PDF route before opening the preview tab Resolves the review findings on the first push in one pass. Skeptic (correctness): the archived-copy branch still called window.open with 'noopener', which returns null by spec even on success, so every successful archived preview also fired the "popup blocked" toast (#1613 had the same defect). Both branches now go through openDeferredTab, which opens with a real handle and severs the opener itself. Skeptic (regression): serving the re-render as a blob URL lost the Content-Disposition filename and gave the tab an address that dies on reload. The route gains ?probe=1, which runs every refusal check and answers 204 without rendering; the page probes first, shows a refusal as a toast, and otherwise points the tab at the real inline URL. Filename, reload and the single render are all kept. The blob URL is gone, which also settles the compliance swarm's noopener and unrevoked-blob notes and CodeRabbit's revoke request. CodeRabbit: the probe fetch is bounded by AbortSignal.timeout so a stalled route cannot leave a blank tab open, and the network-error mapper now receives the active locale and invoice context. Tests: route probe (204 without render, same 400 envelope as the render, unknown value ignored) and the URL helper's probe flag. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GUdZPW46a16GWUdgt2qSZA --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-05 11:55:45 +02:00
parent 34bf5a7387
commit 9418de585f
9 changed files with 207 additions and 13 deletions
@@ -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<InvoiceStatus>(['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<string> {
+    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)
@@ -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', () => {
@@ -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)

@@ -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()
@@ -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.')
+  })
 })


@@ -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,
@@ -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
 }

 /**
@@ -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.",
@@ -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.",