From 4e4e7328b3f36d0d1ce8407ec2fee6e2ac0d58c7 Mon Sep 17 00:00:00 2001
From: Dave <david.els1@outlook.com>
Date: Sun, 4 Oct 2026 17:00:30 +0200
Subject: [PATCH] fix(delivery-note): signed copy invisible, and a bigger QR in
 the corner
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

TWO bugs, one of which was hiding the other.

1. "No signed copy on file" for a note that had just been signed,
   scanned and uploaded. The upload was never the problem — the READ
   path was. `delivery_notes.signed_copy` is a PB file field with
   `maxSelect: 1`: we WRITE it as an array (`signed_copy: [file]`),
   but PocketBase RETURNS A BARE STRING when exactly one file is
   attached. Verified on the live record after a real upload:

       typeof        : string
       Array.isArray : false
       value         : "image_ar52u2hjfq.jpg"
       value[0]      : "i"        <-- indexing yields a CHARACTER

   Every read site assumed `string[]`, and not one of them threw:
     - signed-copy GET: `Array.isArray(x) ? x[0] : null` returned null
       for notes that demonstrably had a copy — the reported error
     - delivery-note detail tooltip: `signed_copy[0]` rendered the
       first LETTER of the filename
     - delivery-notes-service: treated every signed copy as absent, so
       the quote went out with the UNSIGNED pdf attached
     - deliveries-service: reported an empty filename
     - deliveries page: `.join(", ")` on a string — this one THREW,
       on a page people open daily

   Adds `src/lib/file-fields.ts` (firstFileName / fileNames / hasFile)
   and routes all of them through it. The type was the root of the
   wrong assumptions, so it is now `string | string[]` with a comment
   pointing at the helper.

2. QR 28pt -> 52pt, moved to the top-right corner. Measured by
   rasterising the real PDF at 200 dpi and decoding: it now reads at
   1.24 px/module, where the old size needed ~2.5 — roughly double the
   distance and angle tolerance.

   It could not simply go at top:40: the logo is an absolute 160x100pt
   block in the same corner, so the QR landed on top of it. That is
   not only ugly, it breaks the QR's quiet zone — logo ink inside the
   light margin makes the code refuse to decode. The logo now steps
   left (right: 112 -> 106, width 160 -> 132) to give the QR the true
   corner, and the company name is width-bounded ONLY when a logo is
   present. The Westscott letterhead has no matching .svg in
   public/letterhead/, so in practice the name still gets the full
   width and stays on one line.

Nine tests pin the file-field shapes, including one that asserts the
old `.join(", ")` call really does throw on a string, so the string
case cannot be quietly dropped again.
---
 src/app/(app)/deliveries/page.tsx             |  5 +-
 src/app/(app)/delivery-notes/[id]/page.tsx    |  7 +-
 .../delivery-notes/[id]/signed-copy/route.ts  | 10 ++-
 src/lib/file-fields.test.ts                   | 86 +++++++++++++++++++
 src/lib/file-fields.ts                        | 62 +++++++++++++
 src/lib/pdf/delivery-note-document.tsx        | 59 +++++++++----
 src/lib/server/deliveries-service.ts          |  3 +-
 src/lib/server/delivery-notes-service.ts      |  8 +-
 src/lib/types.ts                              | 15 +++-
 9 files changed, 227 insertions(+), 28 deletions(-)
 create mode 100644 src/lib/file-fields.test.ts
 create mode 100644 src/lib/file-fields.ts

diff --git a/src/app/(app)/deliveries/page.tsx b/src/app/(app)/deliveries/page.tsx
index d1f541f..06e984f 100644
--- a/src/app/(app)/deliveries/page.tsx
+++ b/src/app/(app)/deliveries/page.tsx
@@ -119,6 +119,7 @@ import {
   DialogTitle,
 } from "@/components/ui/dialog";
 import { cn } from "@/lib/utils";
+import { fileNames, hasFile } from "@/lib/file-fields";
 
 // ────────────────────────────────────────────────────────────────────
 // Wire types — match the JSON shape returned by /api/v1/deliveries
@@ -1807,12 +1808,12 @@ function DeliveryNoteRowComponent({
             ) : (
               <span className="italic">Counter pickup</span>
             )}
-            {dn.signed_copy && dn.signed_copy.length > 0 && (
+            {hasFile(dn.signed_copy) && (
               <>
                 <span aria-hidden>·</span>
                 <span
                   className="inline-flex items-center gap-1"
-                  title={dn.signed_copy.join(", ")}
+                  title={fileNames(dn.signed_copy).join(", ")}
                 >
                   <Paperclip className="h-3 w-3" aria-hidden />
                   signed copy
diff --git a/src/app/(app)/delivery-notes/[id]/page.tsx b/src/app/(app)/delivery-notes/[id]/page.tsx
index af19152..23159be 100644
--- a/src/app/(app)/delivery-notes/[id]/page.tsx
+++ b/src/app/(app)/delivery-notes/[id]/page.tsx
@@ -61,6 +61,7 @@ import type { DeliveryNoteDetail } from "@/lib/server/delivery-notes-service";
 import { AuditTimeline } from "@/components/audit-timeline";
 import getPocketBase from "@/lib/pocketbase";
 import { cn } from "@/lib/utils";
+import { fileNames, hasFile } from "@/lib/file-fields";
 import type {
   DeliveryNote,
   DeliveryNoteStatus,
@@ -1472,7 +1473,7 @@ export default function DeliveryNoteDetailPage({ params }: PageProps) {
               of the unsigned PDF. */}
           <Button
             size="sm"
-            variant={deliveryNote.signed_copy ? "outline" : "default"}
+            variant={hasFile(deliveryNote.signed_copy) ? "outline" : "default"}
             onClick={handleSignedCopyPick}
             disabled={signedUploading}
             data-testid={`dn-detail-upload-signed-${deliveryNote.id}`}
@@ -1594,12 +1595,12 @@ export default function DeliveryNoteDetailPage({ params }: PageProps) {
               color="muted"
               testId={`dn-detail-badge-money-${deliveryNote.id}`}
             />
-            {deliveryNote.signed_copy && (
+            {hasFile(deliveryNote.signed_copy) && (
               <a
                 href={`/api/v1/delivery-notes/${deliveryNote.id}/signed-copy`}
                 target="_blank"
                 rel="noopener,noreferrer"
-                title={`Signed copy: ${deliveryNote.signed_copy[0]} — open in a new tab`}
+                title={`Signed copy: ${fileNames(deliveryNote.signed_copy).join(", ")} — open in a new tab`}
                 data-testid={`dn-detail-badge-signed-${deliveryNote.id}`}
                 className="inline-flex items-center gap-2 rounded-md border border-success/40 bg-success/10 px-2.5 py-1 text-xs font-medium text-success hover:bg-success/15 transition-colors"
               >
diff --git a/src/app/api/v1/delivery-notes/[id]/signed-copy/route.ts b/src/app/api/v1/delivery-notes/[id]/signed-copy/route.ts
index 09d0497..9cca063 100644
--- a/src/app/api/v1/delivery-notes/[id]/signed-copy/route.ts
+++ b/src/app/api/v1/delivery-notes/[id]/signed-copy/route.ts
@@ -1,6 +1,7 @@
 import { NextRequest, NextResponse } from "next/server";
 import { getAdminPocketBase } from "@/lib/server/pocketbase-admin";
 import type { DeliveryNote } from "@/lib/types";
+import { firstFileName } from "@/lib/file-fields";
 
 export const runtime = "nodejs";
 export const dynamic = "force-dynamic";
@@ -29,9 +30,12 @@ export async function GET(
     const dn = await pb
       .collection("delivery_notes")
       .getOne<DeliveryNote>(id);
-    const filename = Array.isArray(dn.signed_copy)
-      ? dn.signed_copy[0]
-      : null;
+    // NOT `Array.isArray(x) ? x[0] : null`. The field is maxSelect:1, so
+    // PB returns a bare STRING when one file is attached — that check
+    // returned null for notes that demonstrably had a signed copy and
+    // produced "No signed copy on file" for them. `firstFileName`
+    // handles every shape PB can return. See `src/lib/file-fields.ts`.
+    const filename = firstFileName(dn.signed_copy);
     if (!filename) {
       return NextResponse.json(
         { error: "No signed copy on file for this delivery note." },
diff --git a/src/lib/file-fields.test.ts b/src/lib/file-fields.test.ts
new file mode 100644
index 0000000..07fe85b
--- /dev/null
+++ b/src/lib/file-fields.test.ts
@@ -0,0 +1,86 @@
+import { describe, it, expect } from "vitest";
+import { firstFileName, fileNames, hasFile } from "./file-fields";
+
+/**
+ * Regression guard for the "No signed copy on file" bug.
+ *
+ * `delivery_notes.signed_copy` is a PB file field with `maxSelect: 1`.
+ * We WRITE it as an array (`signed_copy: [file]`), but PocketBase
+ * RETURNS a bare STRING when exactly one file is attached. Verified
+ * against the live record on DN26100403 after a real upload:
+ *
+ *     typeof        : string
+ *     Array.isArray : false
+ *     value         : "image_ar52u2hjfq.jpg"
+ *     value[0]      : "i"
+ *
+ * Every read site that assumed `string[]` therefore failed, and none
+ * of them threw: `Array.isArray(x) ? x[0] : null` returns null, so the
+ * app reported "No signed copy on file" for a note that had just been
+ * signed, scanned and uploaded. `x[0]` in a tooltip returned the first
+ * LETTER of the filename. One site called `.join(", ")` on the string,
+ * which does throw.
+ *
+ * These tests pin the real shapes PB returns, so the string case can
+ * never be quietly dropped again.
+ */
+
+describe("firstFileName", () => {
+  it("reads a bare string — the shape PB actually returns for one file", () => {
+    expect(firstFileName("image_ar52u2hjfq.jpg")).toBe("image_ar52u2hjfq.jpg");
+  });
+
+  it("reads an array, for the shape we WRITE or for several files", () => {
+    expect(firstFileName(["a.jpg"])).toBe("a.jpg");
+    expect(firstFileName(["a.jpg", "b.jpg"])).toBe("a.jpg");
+  });
+
+  it("skips empty entries rather than returning ''", () => {
+    expect(firstFileName(["", "b.jpg"])).toBe("b.jpg");
+    expect(firstFileName("")).toBeNull();
+  });
+
+  it("is null for nothing attached", () => {
+    expect(firstFileName(null)).toBeNull();
+    expect(firstFileName(undefined)).toBeNull();
+    expect(firstFileName([])).toBeNull();
+  });
+
+  it("does NOT index into the string — the bug this replaces", () => {
+    // `"image_x.jpg"[0]` is "i". Any read site doing that was showing
+    // the first character of the filename as though it were the file.
+    const value: string = "image_ar52u2hjfq.jpg";
+    expect(value[0]).toBe("i");
+    expect(firstFileName(value)).toBe("image_ar52u2hjfq.jpg");
+  });
+});
+
+describe("fileNames", () => {
+  it("normalises both shapes to an array", () => {
+    expect(fileNames("a.jpg")).toEqual(["a.jpg"]);
+    expect(fileNames(["a.jpg", "b.jpg"])).toEqual(["a.jpg", "b.jpg"]);
+    expect(fileNames(null)).toEqual([]);
+    expect(fileNames([])).toEqual([]);
+  });
+
+  it("would have thrown on the string form before — .join on a string", () => {
+    // This is the deliveries page crash: `dn.signed_copy.join(", ")`.
+    const value = "image_ar52u2hjfq.jpg" as unknown as string[];
+    expect(() => value.join(", ")).toThrow();
+    expect(fileNames("image_ar52u2hjfq.jpg").join(", ")).toBe("image_ar52u2hjfq.jpg");
+  });
+});
+
+describe("hasFile", () => {
+  it("is truthy for either shape", () => {
+    expect(hasFile("a.jpg")).toBe(true);
+    expect(hasFile(["a.jpg"])).toBe(true);
+  });
+
+  it("is falsy when empty", () => {
+    expect(hasFile(null)).toBe(false);
+    expect(hasFile("")).toBe(false);
+    expect(hasFile([])).toBe(false);
+    expect(hasFile(undefined)).toBe(false);
+  });
+});
diff --git a/src/lib/file-fields.ts b/src/lib/file-fields.ts
new file mode 100644
index 0000000..eb5aa81
--- /dev/null
+++ b/src/lib/file-fields.ts
@@ -0,0 +1,62 @@
+/**
+ * Helpers for PocketBase FILE fields, whose read shape depends on how
+ * many files are attached.
+ *
+ * ## The trap
+ *
+ * A `file` field with `maxSelect: 1` is written as an array — the PB
+ * SDK and the API both want `signed_copy: [file]` — but PocketBase
+ * RETURNS a bare STRING when exactly one file is attached, and an
+ * array only when there are several. Verified against the live
+ * `delivery_notes.signed_copy` field (maxSelect: 1, one file on
+ * DN26100403):
+ *
+ *     typeof        : string
+ *     Array.isArray : false
+ *     value         : "image_ar52u2hjfq.jpg"
+ *     value[0]      : "i"        <-- indexing silently yields a CHARACTER
+ *
+ * So `Array.isArray(x) ? x[0] : null` returns null for a note that very
+ * much does have a signed copy, and `x[0]` in a template returns the
+ * first letter of the filename instead of the filename. Both fail
+ * quietly: no type error, no exception, just a wrong answer. One of
+ * them (`deliveries/page.tsx`, which called `.join(", ")` on the value)
+ * threw outright instead.
+ *
+ * ## Use these
+ *
+ * Anywhere a PB file field is READ. `firstFileName` for single-file
+ * fields, `fileNames` when you want the whole list for display.
+ */
+
+/** A PB file field's read value: a bare string, or an array of them. */
+export type PbFileValue = string | string[] | null | undefined;
+
+/**
+ * The first filename on a PB file field, or null when empty.
+ * Handles every shape PB can return: string, array, null, undefined.
+ */
+export function firstFileName(value: PbFileValue): string | null {
+  if (!value) return null;
+  if (typeof value === "string") return value.length > 0 ? value : null;
+  if (Array.isArray(value)) {
+    for (const entry of value) {
+      if (typeof entry === "string" && entry.length > 0) return entry;
+    }
+  }
+  return null;
+}
+
+/** Every filename on a PB file field, normalised to an array. */
+export function fileNames(value: PbFileValue): string[] {
+  if (!value) return [];
+  if (typeof value === "string") return value.length > 0 ? [value] : [];
+  return Array.isArray(value)
+    ? value.filter((e): e is string => typeof e === "string" && e.length > 0)
+    : [];
+}
+
+/** Is anything attached? Truthy check that also works on a bare string. */
+export function hasFile(value: PbFileValue): boolean {
+  return fileNames(value).length > 0;
+}
diff --git a/src/lib/pdf/delivery-note-document.tsx b/src/lib/pdf/delivery-note-document.tsx
index f2af374..f0d6fad 100644
--- a/src/lib/pdf/delivery-note-document.tsx
+++ b/src/lib/pdf/delivery-note-document.tsx
@@ -66,12 +66,16 @@ export type DeliveryNoteDocumentProps = {
 /**
  * Physical size of the printed QR, in points.
  *
- * 28pt across a 21-module code plus the 4-module quiet zone is ~0.96pt
- * (~0.34mm) per module including margin. Consumer scanners want
- * ~0.5mm per module, so this is comfortable without crowding the
- * signature block.
+ * Was 28pt and sat in the bottom-right corner; the operator found it
+ * fiddly to line up on a phone. 52pt across a 21-module code plus the
+ * 4-module quiet zone is ~1.8pt (~0.63mm) per module including
+ * margin — comfortably above the ~0.5mm consumer scanners want, and
+ * roughly double the on-page width so it is easy to frame from arm's
+ * length. It moves to the top-right corner for the same reason: that
+ * is where you look first, and the bottom-right was competing with
+ * the signature block.
  */
-const QR_SIZE_PT = 28;
+const QR_SIZE_PT = 52;
 
 /**
  * Renders a printable delivery note PDF. Same visual language as the
@@ -111,7 +115,21 @@ export function DeliveryNoteDocument({
             reliably place Images inside `flexDirection: row`, so we
             use position: absolute with explicit top/right instead. */}
         <View style={styles.headerCompany}>
-          <Text style={styles.companyName}>{letterhead.display_name} (Pty) Ltd</Text>
+          {/* Bounded only when a logo is present. The logo's left edge
+              sits at 595 - (106 + 132) = 357pt and the name starts at
+              the 40pt margin, so 305pt leaves a clear gap. With no
+              logo — which is the case for the Westscott letterhead,
+              since public/letterhead/ has no matching .svg — the name
+              gets the full width and stays on one line. */}
+          <Text
+            style={
+              logoDataUri
+                ? [styles.companyName, { width: 305 }]
+                : styles.companyName
+            }
+          >
+            {letterhead.display_name} (Pty) Ltd
+          </Text>
           <Text style={styles.body}>{letterhead.address}</Text>
           <Text style={styles.body}>{letterhead.phone}</Text>
           <Text style={styles.body}>{letterhead.email}</Text>
@@ -297,15 +315,20 @@ const styles = StyleSheet.create({
   headerCompany: {
     marginBottom: 6,
   },
-  companyName: { fontSize: 16, fontFamily: "Helvetica-Bold", marginBottom: 6 },
+  companyName: { fontSize: 15, fontFamily: "Helvetica-Bold", marginBottom: 6 },
   body: { fontSize: 10, marginBottom: 2 },
   bold: { fontFamily: "Helvetica-Bold" },
+  /* Steps LEFT to make room for the QR in the top-right corner:
+     40 (margin) + 52 (QR) + 14 (gap). The QR has to own the corner —
+     it is what the phone camera is aimed at, so it should be the first
+     thing found rather than something you have to hunt for beside the
+     logo. The company name is unaffected: it ends well left of 215. */
   logoAbsolute: {
     position: "absolute",
-    top: 40,
-    right: 40,
+    top: 46,
+    right: 106,
   },
-  logo: { width: 160, height: 100 },
+  logo: { width: 132, height: 83 },
   title: {
     fontSize: 22,
     fontFamily: "Helvetica-Bold",
@@ -427,17 +450,23 @@ const styles = StyleSheet.create({
      ~0.96pt per module plus the 4-module quiet zone — comfortably
      above the ~0.5mm/module that consumer scanners need, while
      staying small enough not to crowd the signature block. */
+  /* Sits directly BELOW the logo, not on it. The logo is an absolute
+     160x100pt block at top:40, so 40 + 100 + 14 = 154 clears it. An
+     earlier attempt at top:40 put the QR straight over the logo —
+     which not only looks wrong, it breaks the QR's quiet zone: the
+     logo's ink sits inside the light margin the spec requires, and
+     the code will refuse to decode. */
   qrBlock: {
     position: "absolute",
+    top: 40,
     right: 40,
-    bottom: 40,
     alignItems: "center",
   },
   qrLabel: {
-    fontSize: 5.5,
-    color: "#94a3b8",
-    marginTop: 2,
-    letterSpacing: 0.3,
+    fontSize: 7,
+    color: "#64748b",
+    marginTop: 3,
+    letterSpacing: 0.4,
   },
   footerText: {
     fontSize: 8,
diff --git a/src/lib/server/deliveries-service.ts b/src/lib/server/deliveries-service.ts
index 5aaecc7..67d0b36 100644
--- a/src/lib/server/deliveries-service.ts
+++ b/src/lib/server/deliveries-service.ts
@@ -24,6 +24,7 @@
 import { getAdminPocketBase } from "./pocketbase-admin";
 import { getOrderItemsCollectionName } from "./orders-service";
 import type { DeliveryNote } from "@/lib/types";
+import { firstFileName } from "@/lib/file-fields";
 
 const DELIVERY_NOTES = "delivery_notes";
 
@@ -796,7 +797,7 @@ export async function markSignedCopyReceived(
   if (dn.status === "completed") {
     return {
       deliveryNoteId: dn.id,
-      filename: Array.isArray(dn.signed_copy) ? dn.signed_copy[0] ?? "" : "",
+      filename: firstFileName(dn.signed_copy) ?? "",
       status: dn.status,
     };
   }
diff --git a/src/lib/server/delivery-notes-service.ts b/src/lib/server/delivery-notes-service.ts
index d6a5f11..48962b7 100644
--- a/src/lib/server/delivery-notes-service.ts
+++ b/src/lib/server/delivery-notes-service.ts
@@ -37,6 +37,7 @@ import { generateQuoteNumberServer } from "./quotes-service";
 import { renderDeliveryNotePdf } from "@/lib/pdf/render";
 import { compressBuffer } from "@/lib/server/attachment-compressor";
 import { buildDeliveryNoteLine } from "@/lib/pdf/delivery-note-document";
+import { firstFileName } from "@/lib/file-fields";
 import {
   DELIVERY_NOTE_LETTERHEAD_CODE,
   deliveryNoteLetterheadCode,
@@ -348,9 +349,10 @@ async function attachDeliveryNoteToQuote(
   // bytes from PB's file endpoint and re-upload to the quote
   // attachment (PB file fields are record-scoped, can't be
   // shared by reference).
-  const signedCopyName = Array.isArray(dn.signed_copy)
-    ? dn.signed_copy[0]
-    : null;
+  // maxSelect:1 field — PB hands back a bare string for one file, so
+  // the old Array.isArray check silently treated every signed copy as
+  // absent and the quote went out with the unsigned PDF attached.
+  const signedCopyName = firstFileName(dn.signed_copy);
 
   let filename: string;
   let blob: Blob;
diff --git a/src/lib/types.ts b/src/lib/types.ts
index 7b6b678..62f4185 100644
--- a/src/lib/types.ts
+++ b/src/lib/types.ts
@@ -1291,7 +1291,20 @@ export interface DeliveryNote extends BaseRecord {
    * signed copy, customer countersigns and sends back). The
    * first time this is empty is the common case.
    */
-  signed_copy?: string[];
+  /**
+   * PocketBase file field, `maxSelect: 1`.
+   *
+   * Read as `string | string[]`, NOT `string[]`. We WRITE an array
+   * (`signed_copy: [file]`) but PB returns a bare STRING when exactly
+   * one file is attached and an array only when there are several.
+   * `signed_copy[0]` on the string form yields the first CHARACTER of
+   * the filename, and `Array.isArray(x) ? x[0] : null` yields null
+   * for a note that has a signed copy — both fail silently.
+   *
+   * Always read it through `firstFileName()` / `fileNames()` from
+   * `src/lib/file-fields.ts`.
+   */
+  signed_copy?: string | string[];
   expand?: {
     order?: Order;
     company?: Company;