fix: uploads to an order line add files instead of replacing them

A PocketBase file PATCH under the bare field name replaces the whole
list. On 2026-10-19 a buyer's "Add supplier invoice" on PO202610008LIF
deleted the driver's delivery-note photo (uploaded 14:38, read by the AI
14:40, replaced 14:41). Generating or regenerating a delivery-note PDF
did the same to every line on the DN.

- New src/lib/server/order-item-files.ts: addAttachmentForm() adds with
  `order_item_attachements+` and removes (`-`) only older stored copies
  of the same document; pickStoredCopy() finds a file by its uploaded
  name (PB stores "DN26101913.pdf" as "dn26101913_<10>.pdf").
- Used by the Collection Confirm invoice upload, DN create, DN regenerate
  (replaces only its own previous PDF) and /api/invoicing/delivery-note.
- /api/invoicing/delivery-note-pdf finds the DN PDF by name instead of
  taking the line's first file (falls back to the old behaviour for
  legacy names).
- AGENTS.md: always add, never replace.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Dave 2026-10-19 14:46:37 +02:00
parent e66ea11845
commit 924f7b764e
10 changed files with 149 additions and 17 deletions

View file

@ -38,6 +38,13 @@ Pattern in code:
stream the bytes back via PB's `/api/files/...` URL
3. Stable filename per document type so the retrieval side can
disambiguate from other attachments on the same item
**Always ADD, never replace:** a PocketBase file PATCH under the bare
field name replaces the whole list — it deleted a driver's photo on
2026-10-19. Build uploads with `addAttachmentForm()` from
`src/lib/server/order-item-files.ts` (`order_item_attachements+`, and
`-` only for the same document's older copies), and find a file by name
with `pickStoredCopy()`, never by position.
<!-- END:pb-attachments-pattern -->
<!-- BEGIN:file-url-paths -->

View file

@ -83,6 +83,20 @@ describe("GET /api/invoicing/delivery-note-pdf", () => {
);
});
it("serves the DN PDF, not a driver photo that sits first on the line", async () => {
getFullListMock.mockResolvedValueOnce([
{ id: "row-1", order_item_attachements: ["driver_photo_ab12cd34ef.jpg", "dn26071401_ta9aoxfc9s.pdf"] },
]);
fetchMock.mockResolvedValueOnce(
new Response(new Uint8Array([0x25, 0x50, 0x44, 0x46]), { status: 200, headers: { "Content-Type": "application/pdf" } }),
);
const response = await GET(
makeRequest("https://test.local/api/invoicing/delivery-note-pdf?deliveryNoteNumber=DN26071401") as any,
);
expect(response.status).toBe(200);
expect(fetchMock).toHaveBeenCalledWith(expect.stringContaining("/api/files/order_items/row-1/dn26071401_ta9aoxfc9s.pdf"));
});
it("returns 502 when the PB file fetch itself fails", async () => {
getFullListMock.mockResolvedValueOnce([
{ id: "row-1", order_item_attachements: "token.pdf" },

View file

@ -1,3 +1,4 @@
import { attachmentList, pickStoredCopy } from "@/lib/server/order-item-files";
import { NextRequest, NextResponse } from "next/server";
import { getAdminPocketBase } from "@/lib/server/pocketbase-admin";
@ -40,7 +41,15 @@ function readStringField(value: unknown, key: string): string {
*/
function findDeliveryNoteAttachmentToken(
items: OrderItemRecord[],
deliveryNoteNumber: string,
): { itemId: string; token: string } | null {
// By name first: the line also carries the driver's photo and any
// supplier invoice, so "the first file" isn't necessarily the DN.
for (const item of items) {
const token = pickStoredCopy(attachmentList(item.order_item_attachements), `${deliveryNoteNumber}.pdf`);
if (token) return { itemId: item.id, token };
}
// Legacy: older DNs were stored under other names, alone on the line.
for (const item of items) {
const raw = item.order_item_attachements;
if (typeof raw === "string" && raw.length > 0) {
@ -71,7 +80,7 @@ async function handleRetrieve(input: { deliveryNoteNumber: string }) {
{ status: 404 },
);
}
const found = findDeliveryNoteAttachmentToken(items);
const found = findDeliveryNoteAttachmentToken(items, deliveryNoteNumber);
if (!found) {
return NextResponse.json(
{ error: "No saved PDF for this delivery note" },

View file

@ -195,7 +195,9 @@ describe("POST /api/invoicing/delivery-note", () => {
expect(formCalls.length).toBe(2);
for (const call of formCalls) {
const form = call[1] as FormData;
const entry = form.get("order_item_attachements");
// "+" adds: the driver's photo and supplier invoice on the line stay.
expect(form.get("order_item_attachements")).toBeNull();
const entry = form.get("order_item_attachements+");
expect(entry).toBeDefined();
// FormData entries can be string | File. We expect a File
// (the PDF blob uploaded under the canonical name).

View file

@ -1,3 +1,4 @@
import { addAttachmentForm, attachmentList } from "@/lib/server/order-item-files";
import { NextRequest, NextResponse } from "next/server";
import { getAdminPocketBase } from "@/lib/server/pocketbase-admin";
@ -473,8 +474,7 @@ async function uploadDeliveryNotePdf(
const blob = new Blob([new Uint8Array(pdfBuffer)], { type: "application/pdf" });
await Promise.all(
items.map(async (item) => {
const form = new FormData();
form.append("order_item_attachements", blob, filename);
const form = addAttachmentForm(blob, filename, attachmentList(item.order_item_attachements));
await pb.collection("order_items").update(item.id, form);
}),
);

View file

@ -1,3 +1,4 @@
import { addAttachmentForm } from "@/lib/server/order-item-files";
import { getAdminPocketBase } from "@/lib/server/pocketbase-admin";
import { getOrderItemsCollectionName } from "@/lib/server/orders-service";
import { compressFile } from "@/lib/server/attachment-compressor";
@ -286,8 +287,8 @@ export async function uploadInvoiceToOrderItems(
// `attachment-compressor.ts` for the URL-churn rationale.
const { file: compressedTaggedFile } = await compressFile(taggedFile);
for (const id of validIds) {
const form = new FormData();
form.append("order_item_attachements", compressedTaggedFile);
// Adds to the line — never replaces the driver's photo already there.
const form = addAttachmentForm(compressedTaggedFile, compressedTaggedFile.name);
await pb.collection(collectionName).update(id, form);
}
return { filename: taggedFile.name, itemIds: validIds };

View file

@ -94,7 +94,9 @@ describe("uploadInvoiceToOrderItems", () => {
expect(update).toHaveBeenCalledTimes(2);
const form = update.mock.calls[0]?.[1] as FormData;
expect(form).toBeInstanceOf(FormData);
const uploaded = form.get("order_item_attachements") as File;
// "+" appends: the driver's photo already on the line must stay.
expect(form.get("order_item_attachements")).toBeNull();
const uploaded = form.get("order_item_attachements+") as File;
expect(uploaded).toBeInstanceOf(File);
expect(uploaded.name).toBe("[invoice] INV-1.pdf");
});

View file

@ -32,6 +32,7 @@
* - Other callers (cron, future bulk tools) can reuse these.
*/
import { addAttachmentForm, attachmentList } from "@/lib/server/order-item-files";
import { getAdminPocketBase } from "./pocketbase-admin";
import { generateQuoteNumberServer } from "./quotes-service";
import { renderDeliveryNotePdf } from "@/lib/pdf/render";
@ -1410,8 +1411,7 @@ export async function createAdHocDeliveryNote(
});
await Promise.all(
createdItems.map(async (it) => {
const form = new FormData();
form.append("order_item_attachements", blob, filename);
const form = addAttachmentForm(blob, filename, attachmentList((it as { order_item_attachements?: unknown }).order_item_attachements));
await pb.collection(ORDER_ITEMS).update(it.id, form);
}),
);
@ -1525,12 +1525,11 @@ export async function regenerateDeliveryNotePdf(
logoDataUri,
});
// Same upload pattern as `/api/invoicing/delivery-note` step 2:
// PB's file field PATCH REPLACES the existing list with whatever
// we send, so a re-regenerate cleanly overwrites the previous
// copy. Same filename → the retrieval endpoint (which looks for
// a file named `{deliveryNoteNumber}.pdf` on the first item)
// just picks up the new bytes.
// Same upload pattern as `/api/invoicing/delivery-note` step 2: add
// the new PDF and remove only this DN's older copies (a bare-field
// PATCH would wipe the driver's photo and supplier invoice too). The
// retrieval endpoint finds the file by its `{deliveryNoteNumber}.pdf`
// name.
const filename = `${deliveryNoteNumber}.pdf`;
// Compress the re-rendered PDF once outside the per-item loop.
// A regenerate is the operator fixing a price/line error and
@ -1548,8 +1547,9 @@ export async function regenerateDeliveryNotePdf(
});
await Promise.all(
items.map(async (it) => {
const form = new FormData();
form.append("order_item_attachements", blob, filename);
// Replaces this DN's previous PDF only; the driver's photo and any
// supplier invoice on the line stay.
const form = addAttachmentForm(blob, filename, attachmentList((it as { order_item_attachements?: unknown }).order_item_attachements));
await pb.collection(ORDER_ITEMS).update(it.id, form);
}),
);

View file

@ -0,0 +1,41 @@
import { describe, expect, it } from "vitest";
import { addAttachmentForm, isStoredCopyOf, pickStoredCopy } from "./order-item-files";
describe("isStoredCopyOf", () => {
it("recognises PocketBase's stored name for an uploaded file", () => {
expect(isStoredCopyOf("dn26101913_ta9aoxfc9s.pdf", "DN26101913.pdf")).toBe(true);
expect(isStoredCopyOf("dn26101913_ta9aoxfc9s.pdf", "DN26101914.pdf")).toBe(false);
expect(isStoredCopyOf("dn261019131_ta9aoxfc9s.pdf", "DN26101913.pdf")).toBe(false);
expect(isStoredCopyOf("photo_ab12cd34ef.jpg", "DN26101913.pdf")).toBe(false);
});
});
describe("pickStoredCopy", () => {
it("finds the delivery-note PDF even when a driver photo comes first", () => {
expect(pickStoredCopy(["photo_ab12cd34ef.jpg", "dn26101913_ta9aoxfc9s.pdf"], "DN26101913.pdf")).toBe(
"dn26101913_ta9aoxfc9s.pdf",
);
expect(pickStoredCopy(["photo_ab12cd34ef.jpg"], "DN26101913.pdf")).toBeNull();
});
});
describe("addAttachmentForm", () => {
it("adds the file (never replaces) and drops older copies of the same document only", () => {
const blob = new Blob(["%PDF"], { type: "application/pdf" });
const form = addAttachmentForm(blob, "DN26101913.pdf", [
"driver_photo_ab12cd34ef.jpg",
"dn26101913_ta9aoxfc9s.pdf",
"invoice_inv_1_x1y2z3w4v5.pdf",
]);
expect(form.get("order_item_attachements")).toBeNull();
const added = form.get("order_item_attachements+") as File;
expect(added.name).toBe("DN26101913.pdf");
expect(form.getAll("order_item_attachements-")).toEqual(["dn26101913_ta9aoxfc9s.pdf"]);
});
it("just adds when nothing older exists", () => {
const form = addAttachmentForm(new Blob(["x"]), "[invoice] INV-1.pdf");
expect((form.get("order_item_attachements+") as File).name).toBe("[invoice] INV-1.pdf");
expect(form.getAll("order_item_attachements-")).toEqual([]);
});
});

View file

@ -0,0 +1,56 @@
/**
* Writing to `order_items.order_item_attachements`.
*
* The field holds everything on a line: the driver's photo, a supplier
* invoice the buyer uploaded, the delivery-note PDF. A PocketBase file
* PATCH under the bare field name REPLACES the list — on 2026-10-19 a
* buyer's "Add supplier invoice" silently deleted the driver's delivery
* note photo on PO202610008LIF, and generating a DN PDF did the same.
* Always add with `field+`; only a document's own older copies are
* removed (`field-`), e.g. when a DN PDF is regenerated.
*/
const FIELD = "order_item_attachements";
/** PocketBase stores "DN26101913.pdf" as "dn26101913_<10 random>.pdf". */
function storedStem(original: string): { stem: string; ext: string } {
const dot = original.lastIndexOf(".");
const base = dot > 0 ? original.slice(0, dot) : original;
const ext = dot > 0 ? original.slice(dot).toLowerCase() : "";
const stem = base
.replace(/([a-z0-9])([A-Z])/g, "$1_$2")
.toLowerCase()
.replace(/[^a-z0-9]+/g, "_")
.replace(/^_+|_+$/g, "");
return { stem, ext };
}
/** True when `stored` is PocketBase's saved name for an upload called `original`. */
export function isStoredCopyOf(stored: string, original: string): boolean {
const { stem, ext } = storedStem(original);
if (!stem) return false;
const s = stored.toLowerCase();
return s.startsWith(`${stem}_`) && (!ext || s.endsWith(ext)) && /^[a-z0-9]{10}$/.test(s.slice(stem.length + 1, s.length - ext.length));
}
/** The stored copy of `original` among a line's files (newest wins), or null. */
export function pickStoredCopy(files: string[], original: string): string | null {
const hits = files.filter((f) => isStoredCopyOf(f, original));
return hits.length > 0 ? hits[hits.length - 1] : null;
}
/**
* FormData that ADDS `file` to a line's attachments and removes only
* older stored copies of the same document from `existing`.
*/
export function addAttachmentForm(file: Blob, filename: string, existing: string[] = []): FormData {
const form = new FormData();
form.append(`${FIELD}+`, file, filename);
for (const old of existing.filter((f) => isStoredCopyOf(f, filename))) form.append(`${FIELD}-`, old);
return form;
}
/** A line's attachment list as an array (PB returns a string for single-file data). */
export function attachmentList(raw: unknown): string[] {
if (Array.isArray(raw)) return raw.filter((f): f is string => typeof f === "string" && f !== "");
return typeof raw === "string" && raw ? [raw] : [];
}