diff --git a/src/app/(app)/delivery-notes/page.tsx b/src/app/(app)/delivery-notes/page.tsx index 3e16332..5d341c2 100644 --- a/src/app/(app)/delivery-notes/page.tsx +++ b/src/app/(app)/delivery-notes/page.tsx @@ -1817,10 +1817,15 @@ export default function DeliveryNotesPage() { {/* The invoice that was just created, shown in place so she can print or download it without leaving the list — the same - affordance the quote PDF popup gives. The file is already in - PocketBase: n8n downloads the QuickBooks PDF and attaches it - to the note's order_items as `qb_invoice_pdf`, so this is - just surfacing a file that exists, not a new integration. + affordance the quote PDF popup gives. The PDF is stored on one + of the note's order_items as `qb_invoice_pdf` by the invoice + route (which decodes the base64 n8n returns), so this is just + surfacing a file that exists, not a new integration. + + `null` rather than `""` when there is no file: an empty `src` + makes Chrome reload the current page into the frame, which the + operator sees as a blank white screen. The dialog explains the + absence instead. */} {createdInvoice && ( )} @@ -1986,11 +1995,12 @@ export default function DeliveryNotesPage() { ? `/api/invoicing/delivery-note-pdf?deliveryNoteNumber=${encodeURIComponent( previewDnNumber, )}` - : "" + : null } title={previewDnNumber ?? ""} subtitle="Delivery note" downloadFilename={`${previewDnNumber ?? "delivery-note"}.pdf`} + missingMessage="This delivery note's PDF could not be generated. Try again, or ask an administrator to check the server logs." /> {/* Q-icon PDF preview. The Q icon on every row opens diff --git a/src/app/(app)/invoicing/view-model.test.ts b/src/app/(app)/invoicing/view-model.test.ts index 83d002e..7930c64 100644 --- a/src/app/(app)/invoicing/view-model.test.ts +++ b/src/app/(app)/invoicing/view-model.test.ts @@ -889,11 +889,11 @@ describe("readQbWebhookInvoice", () => { it("reads the n8n success response", () => { expect( readQbWebhookInvoice({ status: "success", invoice_number: "INV10983", invoice_id: "29790", qb_invoice_date: "2026-09-25" }), - ).toEqual({ number: "INV10983", date: "2026-09-25" }); + ).toEqual({ number: "INV10983", date: "2026-09-25", pdfBase64: null }); }); it("falls back to today for the date", () => { - expect(readQbWebhookInvoice({ invoiceNumber: "INV1" }, "2026-09-26")).toEqual({ number: "INV1", date: "2026-09-26" }); + expect(readQbWebhookInvoice({ invoiceNumber: "INV1" }, "2026-09-26")).toEqual({ number: "INV1", date: "2026-09-26", pdfBase64: null }); }); it("never treats the QuickBooks internal id as the invoice number", () => { @@ -901,4 +901,43 @@ describe("readQbWebhookInvoice", () => { expect(readQbWebhookInvoice(null)).toBeNull(); expect(readQbWebhookInvoice({ invoice_number: " " })).toBeNull(); }); + + // The PDF is the whole reason this function grew a third field: a + // freshly-created invoice used to open a blank preview because n8n + // returns the PDF and the app discarded it. + describe("pdf_base64", () => { + const pdf = Buffer.from("%PDF-1.7\nfake\n%%EOF").toString("base64"); + + it("keeps a bare base64 payload", () => { + expect(readQbWebhookInvoice({ invoice_number: "INV1", pdf_base64: pdf })?.pdfBase64).toBe(pdf); + }); + + it("accepts the qb_invoice_pdf alias n8n's own write-back reads", () => { + expect(readQbWebhookInvoice({ invoice_number: "INV1", qb_invoice_pdf: pdf })?.pdfBase64).toBe(pdf); + }); + + it("strips a data: URI prefix so the caller can hand over raw bytes", () => { + const got = readQbWebhookInvoice({ + invoice_number: "INV1", + pdf_base64: `data:application/pdf;base64,${pdf}`, + }); + expect(got?.pdfBase64).toBe(pdf); + }); + + it("treats a missing PDF as normal — n8n sends null when it has none", () => { + expect(readQbWebhookInvoice({ invoice_number: "INV1", pdf_base64: null })?.pdfBase64).toBeNull(); + expect(readQbWebhookInvoice({ invoice_number: "INV1", pdf_base64: "" })?.pdfBase64).toBeNull(); + }); + + it("rejects a non-PDF payload rather than storing a broken file", () => { + // An error page or a truncated body would otherwise be written + // to PB and then fail to render — turning a clean "no PDF" state + // into a confusing broken one. + expect( + readQbWebhookInvoice({ invoice_number: "INV1", pdf_base64: "PGh0bWw+PGJvZHk+PC9ib2R5PjwvaHRtbD4=" }) + ?.pdfBase64, + ).toBeNull(); + expect(readQbWebhookInvoice({ invoice_number: "INV1", pdf_base64: "not-base64-at-all" })?.pdfBase64).toBeNull(); + }); + }); }); diff --git a/src/app/(app)/invoicing/view-model.ts b/src/app/(app)/invoicing/view-model.ts index b36a237..66352e3 100644 --- a/src/app/(app)/invoicing/view-model.ts +++ b/src/app/(app)/invoicing/view-model.ts @@ -602,18 +602,41 @@ export function buildReconcileRows( }); } +export interface QbWebhookInvoice { + number: string; + date: string; + /** + * The rendered invoice PDF as raw base64, when n8n supplied one. + * + * `Build Created Response` in the QuickBooks workflow threads + * `pdf_base64` straight through to us. It is optional — the workflow + * logs `has_pdf: false` and sends `null` when the "Extract from File" + * node produced no binary — so treat absence as normal, never as an + * error. + * + * We persist this ourselves rather than relying on n8n's own PB + * write-back: that node builds a `data:` URI string and assigns it to + * the `qb_invoice_pdf` FILE field, which PocketBase silently discards + * (a file field only accepts multipart uploads). That is why a + * freshly-created invoice had no PDF to preview even though the + * number, date and `qty_invoiced` all landed correctly. + */ + pdfBase64: string | null; +} + /** - * The customer invoice number + date from the n8n QuickBooks webhook - * response (`Finalize PocketBase Invoice Update` / `Build Existing - * Response` in "1 - WFDASH QUICKBOOKS TEST_NEXTJS"). Returns null when - * the response carries no invoice number — callers must then skip the - * reconcile rather than invent one. `invoice_id` / `qb_invoice_id` are - * QuickBooks' internal ids, NOT the invoice number, so they're ignored. + * The customer invoice number + date (and, when present, the rendered + * PDF) from the n8n QuickBooks webhook response (`Finalize PocketBase + * Invoice Update` / `Build Existing Response` in "1 - WFDASH + * QUICKBOOKS TEST_NEXTJS"). Returns null when the response carries no + * invoice number — callers must then skip the reconcile rather than + * invent one. `invoice_id` / `qb_invoice_id` are QuickBooks' internal + * ids, NOT the invoice number, so they're ignored. */ export function readQbWebhookInvoice( body: unknown, today: string = new Date().toISOString().slice(0, 10), -): { number: string; date: string } | null { +): QbWebhookInvoice | null { if (!body || typeof body !== "object") return null; const b = body as Record; const pick = (...keys: string[]) => { @@ -626,7 +649,40 @@ export function readQbWebhookInvoice( const number = pick("invoice_number", "invoiceNumber"); if (!number) return null; const date = pick("qb_invoice_date", "invoice_date", "invoiceDate").slice(0, 10) || today; - return { number, date }; + + // `qb_invoice_pdf` is an accepted alias: n8n's own write-back node + // reads `ctx.pdf_base64 ?? ctx.qb_invoice_pdf`, so some workflow + // versions name it that way on the way out too. Accept a bare base64 + // payload or a full `data:` URI — we normalise to the former and let + // the caller hand the bytes straight to a multipart upload. + let pdfBase64 = pick("pdf_base64", "qb_invoice_pdf"); + if (pdfBase64.startsWith("data:")) { + const comma = pdfBase64.indexOf(","); + pdfBase64 = comma === -1 ? "" : pdfBase64.slice(comma + 1); + } + // A real PDF always starts with "%PDF" — but that is the DECODED + // prefix. The base64 text itself begins "JVBERi…", so testing the + // encoded string against "%PDF" rejects every genuine PDF. Decode a + // short head and check the bytes. `atob` exists in both Node 16+ and + // the browser, so this stays safe in the client bundle. + // + // Why bother: a truncated body or an HTML error page would otherwise + // be written to PB and then fail to render — turning a working + // "no PDF" state into a confusing broken one. + if (pdfBase64 && !looksLikePdf(pdfBase64)) { + pdfBase64 = ""; + } + + return { number, date, pdfBase64: pdfBase64 || null }; +} + +function looksLikePdf(base64: string): boolean { + try { + // 16 is a multiple of 4 so `atob` won't trip on the padding. + return atob(base64.slice(0, 16)).startsWith("%PDF"); + } catch { + return false; + } } /** diff --git a/src/app/api/v1/delivery-notes/[id]/invoice/route.test.ts b/src/app/api/v1/delivery-notes/[id]/invoice/route.test.ts index 7849aa9..a755815 100644 --- a/src/app/api/v1/delivery-notes/[id]/invoice/route.test.ts +++ b/src/app/api/v1/delivery-notes/[id]/invoice/route.test.ts @@ -1,16 +1,17 @@ import { NextRequest } from "next/server"; import { beforeEach, describe, expect, it, vi } from "vitest"; -const { reconcile, markCompleted, getOne, getFullList } = vi.hoisted(() => ({ +const { reconcile, markCompleted, getOne, getFullList, update } = vi.hoisted(() => ({ reconcile: vi.fn(), markCompleted: vi.fn(), getOne: vi.fn(), getFullList: vi.fn(), + update: vi.fn(), })); vi.mock("@/lib/server/pocketbase-admin", () => ({ getAdminPocketBase: vi.fn().mockResolvedValue({ - collection: () => ({ getOne, getFullList }), + collection: () => ({ getOne, getFullList, update }), }), })); vi.mock("@/lib/server/orders-service", () => ({ reconcileOrderItemInvoicedStatus: reconcile })); @@ -57,6 +58,7 @@ beforeEach(() => { ]); reconcile.mockResolvedValue({}); markCompleted.mockResolvedValue({ id: "dn-1", status: "completed" }); + update.mockResolvedValue({ qb_invoice_pdf: "abc123_Invoice-INV10990.pdf" }); }); describe("POST /api/v1/delivery-notes/[id]/invoice", () => { @@ -91,4 +93,71 @@ describe("POST /api/v1/delivery-notes/[id]/invoice", () => { expect(res.status).toBe(200); expect(reconcile).not.toHaveBeenCalled(); }); + + // The blank-preview bug. n8n returns `pdf_base64` but its own PB + // write-back can't store it (a file field rejects a plain string), so + // the app has to do the multipart upload itself — otherwise the + // operator gets an invoice that "worked" and an empty white frame. + describe("invoice PDF", () => { + const pdfB64 = Buffer.from("%PDF-1.7\nfake\n%%EOF").toString("base64"); + + it("uploads the returned PDF to an order_item and hands back the reference", async () => { + mockWebhook({ status: "success", invoice_number: "INV10990", qb_invoice_date: "2026-09-25", pdf_base64: pdfB64 }); + + const res = await POST(request(), params); + expect(res.status).toBe(200); + + // `qb_invoice_pdf` is a FILE field — only multipart works. + const [itemId, form] = update.mock.calls[0]; + expect(itemId).toBe("a"); + expect(form).toBeInstanceOf(FormData); + const uploaded = form.get("qb_invoice_pdf") as File; + expect(uploaded.name).toBe("Invoice-INV10990.pdf"); + expect(uploaded.type).toBe("application/pdf"); + expect(Buffer.from(await uploaded.arrayBuffer()).toString()).toBe("%PDF-1.7\nfake\n%%EOF"); + + // PocketBase prefixes the stored name with a random token — the + // caller needs that exact string to build the file URL. + const body = (await res.json()) as { invoice: { pdfFileName: string; pdfRecordId: string } }; + expect(body.invoice.pdfFileName).toBe("abc123_Invoice-INV10990.pdf"); + expect(body.invoice.pdfRecordId).toBe("a"); + }); + + it("does not re-upload when n8n already attached a PDF", async () => { + getFullList.mockResolvedValue([ + { id: "a", description: "M16 bolt", quantity: 10, qty_collected: 10, selling_price: 3, qb_invoice_pdf: "already_there.pdf" }, + ]); + mockWebhook({ status: "success", invoice_number: "INV10990", pdf_base64: pdfB64 }); + + const res = await POST(request(), params); + expect(update).not.toHaveBeenCalled(); + const body = (await res.json()) as { invoice: { pdfFileName: string } }; + expect(body.invoice.pdfFileName).toBe("already_there.pdf"); + }); + + it("still succeeds when QuickBooks sends no PDF at all", async () => { + mockWebhook({ status: "success", invoice_number: "INV10990", pdf_base64: null }); + + const res = await POST(request(), params); + expect(res.status).toBe(200); + expect(update).not.toHaveBeenCalled(); + const body = (await res.json()) as { invoice: { number: string; pdfFileName: null } }; + // null, never "" — an empty string becomes an empty iframe src. + expect(body.invoice.number).toBe("INV10990"); + expect(body.invoice.pdfFileName).toBeNull(); + }); + + it("does not fail the invoice when the upload itself fails", async () => { + update.mockRejectedValueOnce(new Error("PB 500")); + mockWebhook({ status: "success", invoice_number: "INV10990", pdf_base64: pdfB64 }); + + const res = await POST(request(), params); + // The QuickBooks invoice exists; a failed PDF upload must not + // undo it or raise a 500 the operator would read as "no invoice". + expect(res.status).toBe(200); + const body = (await res.json()) as { invoice: { number: string; pdfFileName: null } }; + expect(body.invoice.number).toBe("INV10990"); + expect(body.invoice.pdfFileName).toBeNull(); + }); + }); }); diff --git a/src/app/api/v1/delivery-notes/[id]/invoice/route.ts b/src/app/api/v1/delivery-notes/[id]/invoice/route.ts index 5fcce9c..5b5b300 100644 --- a/src/app/api/v1/delivery-notes/[id]/invoice/route.ts +++ b/src/app/api/v1/delivery-notes/[id]/invoice/route.ts @@ -40,6 +40,60 @@ interface QbWebhookResponse { qb_invoice_id?: string; id?: string; invoice_number?: string; + /** Raw base64 of the rendered invoice PDF. Optional. */ + pdf_base64?: string | null; +} + +/** + * Persist the invoice PDF that n8n returned as base64 onto one of the + * delivery note's order_items, then return the reference the browser + * needs to stream it (`qb_invoice_pdf` is a PB FILE field, so the URL + * is `/api/files/order_items//`). + * + * ## Why the app uploads it rather than trusting n8n + * + * n8n's own "Build PocketBase Invoice Update" node *tries* to write the + * PDF back: it builds `data:application/pdf;base64,…` and assigns it to + * `qb_invoice_pdf`. PocketBase silently drops that. A file field only + * accepts a multipart upload — a plain string assignment is a no-op, + * not an error. The visible symptom was an invoice that looked + * completely successful (number, date and `qty_invoiced` all landed) + * and then opened a blank preview, because the PDF had never been + * stored anywhere. + * + * Non-fatal by design: the QuickBooks invoice exists regardless. If + * this fails we log it, return `null` references, and the caller shows + * an explanation instead of a blank frame. + */ +async function storeInvoicePdf(input: { + pb: Awaited>; + orderItemId: string; + base64: string; + invoiceNumber: string; +}): Promise { + try { + const bytes = Buffer.from(input.base64, "base64"); + if (bytes.byteLength === 0) return null; + const safeNumber = input.invoiceNumber.replace(/[^\w.-]/g, "_"); + const filename = `Invoice-${safeNumber}.pdf`; + const form = new FormData(); + form.append( + "qb_invoice_pdf", + new Blob([new Uint8Array(bytes)], { type: "application/pdf" }), + filename, + ); + const updated = await input.pb + .collection("order_items") + .update<{ qb_invoice_pdf?: string | string[] }>(input.orderItemId, form); + const stored = updated.qb_invoice_pdf; + return Array.isArray(stored) ? (stored[0] ?? null) : (stored ?? null); + } catch (err) { + console.error( + `[POST /api/v1/delivery-notes/[id]/invoice] failed to store invoice PDF on order item ${input.orderItemId}:`, + err, + ); + return null; + } } /** @@ -303,17 +357,30 @@ export async function POST( // delivery-notes LIST show the invoice in a popup after creating it, // instead of the operator having to go find it in QuickBooks. // - // The PDF is not fetched here. n8n already downloads it and - // attaches it to the note's order_items as `qb_invoice_pdf` (1,200+ - // rows carry one), so we hand back that reference and the browser - // prints it from PocketBase like any other file. - // `rowsBilled` is a shape-only projection for reconcile; it has no - // PDF field. Read the reference off the DN's own items. - // The PDF is a file on the ORDER_ITEM, not on the delivery note, - // so the browser needs that record's id to build the PB file URL - // (`/api/files///`). - const pdfItem = items.find((it) => it.qb_invoice_pdf); - const pdfRef = pdfItem?.qb_invoice_pdf ?? null; + // The PDF hangs off an ORDER_ITEM, not the delivery note, so the + // browser needs that record's id to build the PB file URL + // (`/api/files///`). Prefer one n8n + // already attached; otherwise store the base64 it just sent us. + const pdfItemId = items.find((it) => it.qb_invoice_pdf)?.id ?? items[0]?.id ?? null; + let pdfFileName = (() => { + const ref = items.find((it) => it.qb_invoice_pdf)?.qb_invoice_pdf; + return Array.isArray(ref) ? (ref[0] ?? null) : (ref ?? null); + })(); + + if (!pdfFileName && qbInvoice?.pdfBase64 && pdfItemId) { + pdfFileName = await storeInvoicePdf({ + pb, + orderItemId: pdfItemId, + base64: qbInvoice.pdfBase64, + invoiceNumber: qbInvoice.number, + }); + if (!pdfFileName) { + console.error( + `[POST /api/v1/delivery-notes/[id]/invoice] QuickBooks sent a PDF for ${qbInvoice.number} but it could not be stored; the preview will have no file.`, + ); + } + } + return NextResponse.json({ ...updated, uncollectedLineCount: uncollected.length, @@ -324,9 +391,8 @@ export async function POST( invoice: { number: invoiceNumber, date: qbInvoice?.date ?? new Date().toISOString().slice(0, 10), - // A single-file field is a bare string; several is an array. - pdfFileName: Array.isArray(pdfRef) ? (pdfRef[0] ?? null) : (pdfRef ?? null), - pdfRecordId: pdfItem?.id ?? null, + pdfFileName, + pdfRecordId: pdfItemId, }, }); } catch (err: unknown) { diff --git a/src/components/delivery-notes/preview-dialog.tsx b/src/components/delivery-notes/preview-dialog.tsx index 67f4353..78559cb 100644 --- a/src/components/delivery-notes/preview-dialog.tsx +++ b/src/components/delivery-notes/preview-dialog.tsx @@ -34,7 +34,7 @@ * detail page. */ import { useEffect, useRef, useState } from "react"; -import { Download, Loader2, Printer, RefreshCw, X } from "lucide-react"; +import { Download, FileWarning, Loader2, Printer, RefreshCw, X } from "lucide-react"; import { Button } from "@/components/ui/button"; import { Dialog, @@ -76,14 +76,29 @@ const PDF_DIALOG_SIZE = export interface PdfPreviewDialogProps { open: boolean; onOpenChange: (open: boolean) => void; - /** The full URL of the PDF to render in the iframe. */ - pdfUrl: string; + /** + * The full URL of the PDF to render in the iframe, or `null` when no + * PDF is available. + * + * `null` is NOT the same as `""`. Passing `""` renders an iframe + * whose `src` is empty, which Chrome treats as "load the current + * page" — the operator gets a blank white frame plus a console + * warning. It is a bug, not a missing file, so the dialog refuses + * the empty string and explains the absence instead. + */ + pdfUrl: string | null; /** Title shown in the dialog header (e.g. "DN26071401" or "Q202607001"). */ title: string; /** Short subtitle shown under the title (e.g. "Delivery note" or "Quote"). */ subtitle?: string; /** Filename used for the Download button's `download` attribute. */ downloadFilename: string; + /** + * Shown in place of the PDF when `pdfUrl` is null. Should tell the + * operator what IS true and what they can still do — a blank frame + * with no explanation is the failure this whole path exists to avoid. + */ + missingMessage?: string; } export function PdfPreviewDialog({ @@ -93,12 +108,19 @@ export function PdfPreviewDialog({ title, subtitle, downloadFilename, + missingMessage, }: PdfPreviewDialogProps) { // Ref on the iframe so we can tell whether the PDF has finished // loading before offering a print action. const iframeRef = useRef(null); const [blocked, setBlocked] = useState(false); + // An empty string is the dangerous case — Chrome reloads the page + // into the frame. Treat it as "no PDF" so the explanation shows + // instead. Every caller that can produce a missing file goes through + // here, so this is the single place the rule needs to hold. + const resolvedUrl = pdfUrl && pdfUrl.trim() ? pdfUrl : null; + // Clear the pop-up-blocked notice on close rather than in an effect: // syncing state from an effect triggers a second render pass, and // the flag is only ever set by a click inside this component. @@ -108,6 +130,7 @@ export function PdfPreviewDialog({ }; const handlePrint = () => { + if (!resolvedUrl) return; // Deliberately opens the PDF in a new tab rather than calling // `iframe.contentWindow.print()`. // @@ -127,7 +150,7 @@ export function PdfPreviewDialog({ // // A new tab hands the operator Chrome's real PDF viewer, whose own // print button works. Same document, no blank pages. - const url = pdfUrl; + const url = resolvedUrl; const win = window.open(url, "_blank", "noopener,noreferrer"); if (!win) { // Popup blocked. Say so rather than appearing to do nothing. @@ -151,8 +174,8 @@ export function PdfPreviewDialog({ - - @@ -177,8 +200,10 @@ export function PdfPreviewDialog({
{title} - {subtitle ? `${subtitle} · ` : ""}Use Print to send to a - printer, or Download to save a copy. + {subtitle ? `${subtitle} · ` : ""} + {resolvedUrl + ? "Use Print to send to a printer, or Download to save a copy." + : "There is no file to print or download."}