David/WFDASH-WEBAPP
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(delivery-note): signed copy invisible, and a bigger QR in the corner

Browse source 
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.
This commit is contained in:
Dave 2026-10-04 17:00:30 +02:00
parent 7b844b2e51
commit 4e4e7328b3
9 changed files with 227 additions and 28 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
src/app/(app)/deliveries/page.tsx
View file

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

7
src/app/(app)/delivery-notes/[id]/page.tsx
View file

@ -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"
>

10
src/app/api/v1/delivery-notes/[id]/signed-copy/route.ts
View file

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

86
src/lib/file-fields.test.ts Normal file
View file

@ -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);
});
});

62
src/lib/file-fields.ts Normal file
View file

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

59
src/lib/pdf/delivery-note-document.tsx
View file

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

3
src/lib/server/deliveries-service.ts
View file

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

8
src/lib/server/delivery-notes-service.ts
View file

@ -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;

15
src/lib/types.ts
View file

@ -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;