From 8f2bb1fc1309ae8a1107577100f29d8329fe3a28 Mon Sep 17 00:00:00 2001 From: DEls Date: Sat, 19 Sep 2026 18:31:48 +0200 Subject: [PATCH] fix: harden delivery note correction gates --- .../[noteId]/resolve-entities/route.test.ts | 15 +++++ .../notes/[noteId]/resolve-entities/route.ts | 15 +++-- .../notes/[noteId]/resolve-item/route.test.ts | 13 ++++ .../[id]/notes/[noteId]/resolve-item/route.ts | 22 ++++--- .../inbox/[id]/batch-detail-view.test.ts | 13 ++++ .../inbox/[id]/batch-detail-view.tsx | 21 +++--- src/lib/dn-pipeline/checks.ts | 3 +- src/lib/dn-pipeline/readiness.test.ts | 41 ++++++++++++ src/lib/dn-pipeline/readiness.ts | 64 +++++++++++++++++++ 9 files changed, 183 insertions(+), 24 deletions(-) create mode 100644 src/app/dashboard/financial/delivery-notes/inbox/[id]/batch-detail-view.test.ts create mode 100644 src/lib/dn-pipeline/readiness.test.ts create mode 100644 src/lib/dn-pipeline/readiness.ts diff --git a/src/app/api/dn-batches/[id]/notes/[noteId]/resolve-entities/route.test.ts b/src/app/api/dn-batches/[id]/notes/[noteId]/resolve-entities/route.test.ts index fec98fa..8c8cad3 100644 --- a/src/app/api/dn-batches/[id]/notes/[noteId]/resolve-entities/route.test.ts +++ b/src/app/api/dn-batches/[id]/notes/[noteId]/resolve-entities/route.test.ts @@ -90,4 +90,19 @@ describe("resolve-entities route", () => { expect(response.status).toBe(409); } }); + + it("rejects a JSON null body", async () => { + const { POST } = await import("./route"); + const response = await POST(new Request("http://localhost/api", { method: "POST", body: "null" }), { params: Promise.resolve({ id: BATCH, noteId: NOTE }) }); + expect(response.status).toBe(400); + }); + + it("updates an existing alias with manual audit metadata without duplicating it", async () => { + fixtureState.rows.supplier_document_aliases.push({ id: "aliasabcdefghij", supplier: "old-supplierabc", raw_value: "Old", normalised_key: "ACME TRADING", source: "import", approved_by: "importer" }); + const { POST } = await import("./route"); + const response = await POST(request({ supplierId: SUPPLIER, purchaseOrderId: PO, reason: "verified", rememberSupplierAlias: true }), { params: Promise.resolve({ id: BATCH, noteId: NOTE }) }); + expect(response.status).toBe(200); + expect(fixtureState.rows.supplier_document_aliases).toHaveLength(1); + expect(fixtureState.rows.supplier_document_aliases[0]).toMatchObject({ source: "manual_correction", approved_by: "web" }); + }); }); diff --git a/src/app/api/dn-batches/[id]/notes/[noteId]/resolve-entities/route.ts b/src/app/api/dn-batches/[id]/notes/[noteId]/resolve-entities/route.ts index 5179db4..0414641 100644 --- a/src/app/api/dn-batches/[id]/notes/[noteId]/resolve-entities/route.ts +++ b/src/app/api/dn-batches/[id]/notes/[noteId]/resolve-entities/route.ts @@ -3,6 +3,7 @@ import { pbAdmin } from "@/lib/pb"; import { withBatchLease } from "@/lib/dn-pipeline/lock"; import type { BatchMutation } from "@/lib/dn-pipeline/batch-mutations"; import { normaliseSupplierKey } from "@/lib/dn-pipeline/resolve-entities"; +import { evaluateCandidateReadiness } from "@/lib/dn-pipeline/readiness"; export const dynamic = "force-dynamic"; export const runtime = "nodejs"; @@ -29,8 +30,8 @@ export async function POST(request: Request, { params }: { params: Promise<{ id: const leased = await withBatchLease(pb, id, "resolve-entities", async (lease) => { const [batch, note] = await Promise.all([ - pb.collection("dn_batches").getOne(id, { fields: "id,site,status" }).catch(() => null), - pb.collection("dn_batch_notes").getOne(noteId, { fields: "id,batch,status,stage,posted_at,delivery_note,supplier,supplier_name_raw,purchase_order,checks,items" }).catch(() => null), + pb.collection("dn_batches").getOne(id, { fields: "id,site,status,page_count,assembly_revision,revision" }).catch(() => null), + pb.collection("dn_batch_notes").getOne(noteId, { fields: "id,batch,status,stage,posted_at,delivery_note,supplier,supplier_name_raw,purchase_order,checks,items,pages,doc_number,delivery_note_number,assembly_revision,candidate_revision" }).catch(() => null), ]); if (!batch) return NextResponse.json({ error: "batch_not_found" }, { status: 404 }); if (!note) return NextResponse.json({ error: "note_not_found" }, { status: 404 }); @@ -54,11 +55,13 @@ export async function POST(request: Request, { params }: { params: Promise<{ id: if (poSupplier && poSupplier !== supplierId) return NextResponse.json({ error: "purchase_order_supplier_mismatch" }, { status: 400 }); if (!poSupplier && (typeof purchaseOrder.supplier !== "string" || normaliseSupplierKey(purchaseOrder.supplier) !== normaliseSupplierKey(supplier.name))) return NextResponse.json({ error: "purchase_order_supplier_mismatch" }, { status: 400 }); + const poItems = await pb.collection("purchase_order_items").getFullList({ filter: `purchase_order = ${JSON.stringify(purchaseOrderId)}`, fields: "id,purchase_order,description,unit,material_code,qty,rate" }).catch(() => []); const before = { supplier: note.supplier ?? "", purchase_order: note.purchase_order ?? "", stage: note.stage ?? "needs_review", status: note.status ?? "blocked" }; const checks = Array.isArray(note.checks) ? note.checks.map((check: unknown) => isObject(check) && check.id === "po_resolved" ? { ...check, passed: true, reason: "purchase order selected by operator" } : check) : []; const items = Array.isArray(note.items) ? note.items : []; - const ready = checks.every((check) => !isObject(check) || check.passed !== false) && items.every((item) => !isObject(item) || Boolean(item.purchase_order_item || item.purchase_order_item_id)); - const after = { supplier: supplierId, purchase_order: purchaseOrderId, stage: ready ? "ready_to_post" : "needs_review", status: ready ? "candidate" : "blocked" }; + const readiness = evaluateCandidateReadiness({ batch, note: { ...note, supplier: supplierId, purchase_order: purchaseOrderId }, supplierId, purchaseOrderId, purchaseOrderSiteId: poSite, purchaseOrderSupplierId: poSupplier || supplierId, poItems: (Array.isArray(poItems) ? poItems : []) as Record[], checks: checks as Record[] }); + const ready = readiness.ready; + const after = { supplier: supplierId, purchase_order: purchaseOrderId, stage: ready ? "ready_to_post" : "needs_review", status: ready ? "candidate" : "blocked", reasons: readiness.reasons }; const mutations: BatchMutation[] = [ { collection: "dn_batch_notes", method: "update", id: noteId, data: { supplier: supplierId, purchase_order: purchaseOrderId, checks, stage: after.stage, status: after.status } }, { collection: "dn_review_events", method: "create", data: { batch: id, note: noteId, action: "resolve_entities", before, after, reason, actor: "web" } }, @@ -68,11 +71,11 @@ export async function POST(request: Request, { params }: { params: Promise<{ id: const normalisedKey = normaliseSupplierKey(rawValue); const existing = await pb.collection("supplier_document_aliases").getFullList({ filter: `normalised_key = ${JSON.stringify(normalisedKey)}`, fields: "id,supplier,raw_value,normalised_key,source,approved_by" }).catch(() => []); const data = { supplier: supplierId, raw_value: rawValue, normalised_key: normalisedKey, source: "manual_correction", approved_by: "web" }; - if (existing[0]?.id) mutations.push({ collection: "supplier_document_aliases", method: "update", id: existing[0].id, data: { supplier: supplierId, raw_value: rawValue, normalised_key: normalisedKey } }); + if (existing[0]?.id) mutations.push({ collection: "supplier_document_aliases", method: "update", id: existing[0].id, data }); else mutations.push({ collection: "supplier_document_aliases", method: "create", data }); } await lease.batch(mutations); - return NextResponse.json({ ok: true, stage: after.stage, status: after.status, readyToPost: ready }); + return NextResponse.json({ ok: true, stage: after.stage, status: after.status, readyToPost: ready, reasons: readiness.reasons }); }); if (!leased.acquired) return NextResponse.json({ error: "batch_locked" }, { status: 409 }); return leased.value; diff --git a/src/app/api/dn-batches/[id]/notes/[noteId]/resolve-item/route.test.ts b/src/app/api/dn-batches/[id]/notes/[noteId]/resolve-item/route.test.ts index a7d9e47..b2149f5 100644 --- a/src/app/api/dn-batches/[id]/notes/[noteId]/resolve-item/route.test.ts +++ b/src/app/api/dn-batches/[id]/notes/[noteId]/resolve-item/route.test.ts @@ -55,4 +55,17 @@ describe("resolve-item route", () => { await POST(request(body), { params: Promise.resolve({ id: BATCH, noteId: NOTE }) }); expect(rows.dn_item_aliases).toHaveLength(1); }); + + it("rejects malformed null JSON and missing site or supplier ownership", async () => { + const { POST } = await import("./route"); + const malformed = await POST(new Request("http://localhost/api", { method: "POST", body: "null" }), { params: Promise.resolve({ id: BATCH, noteId: NOTE }) }); + expect(malformed.status).toBe(400); + collections.dn_batches.getOne.mockResolvedValue({ id: BATCH, site: "" }); + const missingSite = await POST(request({ lineIndex: 0, purchaseOrderItemId: ITEM, reason: "verified", rememberAlias: false }), { params: Promise.resolve({ id: BATCH, noteId: NOTE }) }); + expect(missingSite.status).toBe(400); + collections.dn_batches.getOne.mockResolvedValue({ id: BATCH, site: "siteabcdefghijk" }); + collections.suppliers.getOne.mockResolvedValue(null); + const missingSupplier = await POST(request({ lineIndex: 0, purchaseOrderItemId: ITEM, reason: "verified", rememberAlias: true }), { params: Promise.resolve({ id: BATCH, noteId: NOTE }) }); + expect(missingSupplier.status).toBe(400); + }); }); diff --git a/src/app/api/dn-batches/[id]/notes/[noteId]/resolve-item/route.ts b/src/app/api/dn-batches/[id]/notes/[noteId]/resolve-item/route.ts index 7327786..6eab1ff 100644 --- a/src/app/api/dn-batches/[id]/notes/[noteId]/resolve-item/route.ts +++ b/src/app/api/dn-batches/[id]/notes/[noteId]/resolve-item/route.ts @@ -4,6 +4,7 @@ import { withBatchLease } from "@/lib/dn-pipeline/lock"; import type { BatchMutation } from "@/lib/dn-pipeline/batch-mutations"; import { aliasKey } from "@/lib/dn-pipeline/aliases"; import { norm } from "@/lib/dn-pipeline/match-po-item"; +import { evaluateCandidateReadiness } from "@/lib/dn-pipeline/readiness"; export const dynamic = "force-dynamic"; export const runtime = "nodejs"; @@ -17,14 +18,15 @@ export async function POST(request: Request, { params }: { params: Promise<{ id: if (!idPattern.test(id) || !idPattern.test(noteId)) return NextResponse.json({ error: "invalid_id" }, { status: 400 }); let body: Record; try { body = await request.json(); } catch { return NextResponse.json({ error: "invalid_json" }, { status: 400 }); } + if (!object(body)) return NextResponse.json({ error: "invalid_item_resolution" }, { status: 400 }); const lineIndex = body.lineIndex; const purchaseOrderItemId = body.purchaseOrderItemId; if (!Number.isInteger(lineIndex) || (lineIndex as number) < 0 || !idPattern.test(String(purchaseOrderItemId ?? "")) || typeof body.reason !== "string" || !String(body.reason).trim() || typeof body.rememberAlias !== "boolean") return NextResponse.json({ error: "invalid_item_resolution" }, { status: 400 }); const pb = await pbAdmin(); const leased = await withBatchLease(pb, id, "resolve-item", async (lease) => { const [batch, note] = await Promise.all([ - pb.collection("dn_batches").getOne(id, { fields: "id,site" }).catch(() => null), - pb.collection("dn_batch_notes").getOne(noteId, { fields: "id,batch,status,stage,posted_at,delivery_note,purchase_order,supplier,items,checks" }).catch(() => null), + pb.collection("dn_batches").getOne(id, { fields: "id,site,page_count,assembly_revision,revision" }).catch(() => null), + pb.collection("dn_batch_notes").getOne(noteId, { fields: "id,batch,status,stage,posted_at,delivery_note,purchase_order,supplier,items,checks,pages,doc_number,delivery_note_number,assembly_revision,candidate_revision" }).catch(() => null), ]); if (!batch) return NextResponse.json({ error: "batch_not_found" }, { status: 404 }); if (!note) return NextResponse.json({ error: "note_not_found" }, { status: 404 }); @@ -34,26 +36,28 @@ export async function POST(request: Request, { params }: { params: Promise<{ id: if ((lineIndex as number) >= items.length || !object(items[lineIndex as number])) return NextResponse.json({ error: "line_index_out_of_range" }, { status: 400 }); const candidatePoId = typeof note.purchase_order === "string" ? note.purchase_order : relationId(note.purchase_order); if (!candidatePoId) return NextResponse.json({ error: "candidate_po_missing" }, { status: 400 }); - const [supplier, purchaseOrder, poItem] = await Promise.all([ + const [supplier, purchaseOrder, poItem, allPoItems] = await Promise.all([ note.supplier ? pb.collection("suppliers").getOne(String(note.supplier), { fields: "id,name" }).catch(() => null) : Promise.resolve(null), pb.collection("purchase_orders").getOne(candidatePoId, { fields: "id,site,supplier,supplier_ref" }).catch(() => null), pb.collection("purchase_order_items").getOne(String(purchaseOrderItemId), { fields: "id,purchase_order,description,material_code,unit,qty,rate" }).catch(() => null), + Promise.resolve(pb.collection("purchase_order_items").getFullList({ filter: `purchase_order = ${JSON.stringify(candidatePoId)}`, fields: "id,purchase_order,description,unit,material_code,qty,rate" })).catch(() => []), ]); if (note.supplier && !supplier) return NextResponse.json({ error: "supplier_not_found" }, { status: 400 }); if (!purchaseOrder) return NextResponse.json({ error: "purchase_order_not_found" }, { status: 404 }); const siteId = relationId(batch.site); const poSite = relationId(purchaseOrder.site); - if (poSite && siteId && poSite !== siteId) return NextResponse.json({ error: "purchase_order_site_mismatch" }, { status: 400 }); + if (!siteId || !poSite || poSite !== siteId) return NextResponse.json({ error: "purchase_order_site_mismatch" }, { status: 400 }); const poSupplier = relationId(purchaseOrder.supplier_ref) || (idPattern.test(relationId(purchaseOrder.supplier)) ? relationId(purchaseOrder.supplier) : ""); - if (note.supplier && poSupplier && poSupplier !== note.supplier) return NextResponse.json({ error: "purchase_order_supplier_mismatch" }, { status: 400 }); + if (!note.supplier || !supplier || !poSupplier || poSupplier !== note.supplier) return NextResponse.json({ error: "purchase_order_supplier_mismatch" }, { status: 400 }); if (!poItem) return NextResponse.json({ error: "po_item_not_found" }, { status: 404 }); if (relationId(poItem.purchase_order) !== candidatePoId) return NextResponse.json({ error: "po_item_not_in_candidate_po" }, { status: 400 }); const before = { lineIndex, purchase_order_item: (items[lineIndex as number] as Record).purchase_order_item ?? null }; const replacement = { ...(items[lineIndex as number] as Record), purchase_order_item: String(purchaseOrderItemId) }; items[lineIndex as number] = replacement; const checks = Array.isArray(note.checks) ? note.checks : []; - const ready = checks.every((check: unknown) => !object(check) || check.passed !== false) && items.every((item: unknown) => !object(item) || Boolean(item.purchase_order_item || item.purchase_order_item_id)); - const after = { lineIndex, purchase_order_item: String(purchaseOrderItemId), stage: ready ? "ready_to_post" : "needs_review", status: ready ? "candidate" : "blocked" }; + const readiness = evaluateCandidateReadiness({ batch, note: { ...note, items }, supplierId: String(note.supplier), purchaseOrderId: candidatePoId, purchaseOrderSiteId: poSite, purchaseOrderSupplierId: poSupplier, poItems: (Array.isArray(allPoItems) && allPoItems.length > 0 ? allPoItems : [poItem]) as Record[], checks: checks as Record[] }); + const ready = readiness.ready; + const after = { lineIndex, purchase_order_item: String(purchaseOrderItemId), stage: ready ? "ready_to_post" : "needs_review", status: ready ? "candidate" : "blocked", reasons: readiness.reasons }; const reason = String(body.reason).trim(); const mutations: BatchMutation[] = [ { collection: "dn_batch_notes", method: "update", id: noteId, data: { items, stage: after.stage, status: after.status } }, @@ -62,6 +66,8 @@ export async function POST(request: Request, { params }: { params: Promise<{ id: if (body.rememberAlias) { const line = items[lineIndex as number] as Record; const description = typeof line.description === "string" ? line.description.trim() : ""; + if (!supplier || !String(note.supplier).trim()) return NextResponse.json({ error: "supplier_unresolved" }, { status: 400 }); + if (!description) return NextResponse.json({ error: "item_description_missing" }, { status: 400 }); const supplierKey = norm(String(supplier?.name ?? note.supplier ?? "")); const key = aliasKey(description); const existing = await pb.collection("dn_item_aliases").getFullList({ filter: `slip_description_key=${JSON.stringify(key)} && supplier_key=${JSON.stringify(supplierKey)}`, fields: "id,times_used" }).catch(() => []); @@ -70,7 +76,7 @@ export async function POST(request: Request, { params }: { params: Promise<{ id: else mutations.push({ collection: "dn_item_aliases", method: "create", data: { ...data, times_used: 1 } }); } await lease.batch(mutations); - return NextResponse.json({ ok: true, stage: after.stage, status: after.status, readyToPost: ready, lineIndex, purchaseOrderItemId: String(purchaseOrderItemId) }); + return NextResponse.json({ ok: true, stage: after.stage, status: after.status, readyToPost: ready, reasons: readiness.reasons, lineIndex, purchaseOrderItemId: String(purchaseOrderItemId) }); }); if (!leased.acquired) return NextResponse.json({ error: "batch_locked" }, { status: 409 }); return leased.value; diff --git a/src/app/dashboard/financial/delivery-notes/inbox/[id]/batch-detail-view.test.ts b/src/app/dashboard/financial/delivery-notes/inbox/[id]/batch-detail-view.test.ts new file mode 100644 index 0000000..94d7ae7 --- /dev/null +++ b/src/app/dashboard/financial/delivery-notes/inbox/[id]/batch-detail-view.test.ts @@ -0,0 +1,13 @@ +import { describe, expect, it } from "vitest"; +import { buildResolveItemRequest } from "./batch-detail-view"; + +describe("ProductDecisions resolve-item payload", () => { + it("uses the stable line index and new route contract", () => { + expect(buildResolveItemRequest(4, "itemabcdefghijk", true)).toEqual({ + lineIndex: 4, + purchaseOrderItemId: "itemabcdefghijk", + reason: "manual product match", + rememberAlias: true, + }); + }); +}); diff --git a/src/app/dashboard/financial/delivery-notes/inbox/[id]/batch-detail-view.tsx b/src/app/dashboard/financial/delivery-notes/inbox/[id]/batch-detail-view.tsx index 6b521e9..2d4c84a 100644 --- a/src/app/dashboard/financial/delivery-notes/inbox/[id]/batch-detail-view.tsx +++ b/src/app/dashboard/financial/delivery-notes/inbox/[id]/batch-detail-view.tsx @@ -362,11 +362,16 @@ function PoLink({ } type ItemDecision = { + lineIndex: number; description: string; kind: "ambiguous" | "none"; candidates: { id: string; description: string; material_code: string }[]; }; +export function buildResolveItemRequest(lineIndex: number, purchaseOrderItemId: string, rememberAlias: boolean) { + return { lineIndex, purchaseOrderItemId, reason: "manual product match", rememberAlias }; +} + function ProductDecisions({ batchId, note, @@ -382,17 +387,17 @@ function ProductDecisions({ const decisions = (Array.isArray(check?.detail) ? check!.detail : []) as ItemDecision[]; if (decisions.length === 0) return null; - async function choose(description: string, poItemId: string) { + async function choose(lineIndex: number, poItemId: string) { if (!poItemId) return; setError(""); - setSaving(description); + setSaving(String(lineIndex)); try { const res = await fetch( `/api/dn-batches/${batchId}/notes/${note.id}/resolve-item`, { method: "POST", headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ description, po_item_id: poItemId, global: applyAll }), + body: JSON.stringify(buildResolveItemRequest(lineIndex, poItemId, applyAll)), }, ); if (!res.ok) { @@ -412,15 +417,13 @@ function ProductDecisions({ These lines need a product decision. Your choice is remembered for next time.

{decisions.map((d, di) => ( - // Keyed by position too: two slip lines on one note can carry - // word-for-word the same description. -
+

{d.description}