diff --git a/scripts/pb-add-dn-note-dismissed.py b/scripts/pb-add-dn-note-dismissed.py new file mode 100644 index 0000000..75c3402 --- /dev/null +++ b/scripts/pb-add-dn-note-dismissed.py @@ -0,0 +1,54 @@ +#!/usr/bin/env python3 +""" +Adds "dismissed" to DN_BATCH_NOTES.status. + +Duplication happens per note, not per batch. The same stack of Vula +invoices was scanned three times in overlapping groupings (13, 16 and 14 +pages, three different file hashes), so every one of those batches holds +a mix of promoted notes, duplicates of another batch, and invoices unique +to it. Dismissing a whole batch therefore either discards unique +invoices or zeroes a counter for notes already on the ledger. + +"dismissed" lets a single duplicate note stand down, which is the only +safe granularity. + +Idempotent. +""" +import sys + +from pb_env import PB, http, superuser_token + +NEW_VALUE = "dismissed" + + +def main(): + token = superuser_token() + print(f"\n→ dn_batch_notes on {PB}") + + code, coll = http("GET", "/api/collections/dn_batch_notes", token=token) + if code != 200: + sys.exit(f"GET failed: {code} {coll}") + + # Read the live field list and modify in place. PocketBase replaces + # the whole array on PATCH, so never build it from anything but a + # fresh read. + fields = list(coll.get("fields", [])) + status = next((f for f in fields if f.get("name") == "status"), None) + if not status: + sys.exit("no status field on dn_batch_notes") + + values = list(status.get("values") or []) + if NEW_VALUE in values: + print(f" • status already allows '{NEW_VALUE}'") + return + + status["values"] = values + [NEW_VALUE] + code, resp = http("PATCH", f"/api/collections/{coll['id']}", {"fields": fields}, token) + if code != 200: + sys.exit(f"PATCH failed: {code} {resp}") + print(f" ✓ status values now {status['values']} (kept {len(fields)} fields)") + + +if __name__ == "__main__": + main() + print("\nDone.") diff --git a/src/app/api/dn-batches/[id]/dismiss/route.test.ts b/src/app/api/dn-batches/[id]/dismiss/route.test.ts index 3e11c5d..0b347ab 100644 --- a/src/app/api/dn-batches/[id]/dismiss/route.test.ts +++ b/src/app/api/dn-batches/[id]/dismiss/route.test.ts @@ -3,7 +3,8 @@ * * The "already ingested" button. The route's contract: * 1. Path validation: id is a 15-char alnum PocketBase id. - * 2. Park the batch at `processed` with notes_promoted=0, an empty + * 2. Park the batch at `processed` with notes_promoted reflecting + * what actually reached the ledger, an empty * claimed_at, an empty last_error, and a single passing check * whose reason explains why nothing was promoted. The check * keeps the board consistent — Processed always means "ran the @@ -21,8 +22,9 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; /* Hoisted mocks */ /* ------------------------------------------------------------------ */ -const { dnBatchesUpdate, viPbAdmin } = vi.hoisted(() => ({ +const { dnBatchesUpdate, dnBatchNotesGetFullList, viPbAdmin } = vi.hoisted(() => ({ dnBatchesUpdate: vi.fn(), + dnBatchNotesGetFullList: vi.fn(), viPbAdmin: vi.fn(), })); @@ -30,10 +32,11 @@ function makePb() { return { authStore: { token: "test-admin-token" }, collection: (name: string) => { - if (name !== "dn_batches") { - throw new Error(`unexpected collection in test: ${name}`); - } - return { update: dnBatchesUpdate }; + if (name === "dn_batches") return { update: dnBatchesUpdate }; + // The route counts what genuinely promoted rather than assuming + // none, so it reads the batch's notes. + if (name === "dn_batch_notes") return { getFullList: dnBatchNotesGetFullList }; + throw new Error(`unexpected collection in test: ${name}`); }, }; } @@ -67,6 +70,8 @@ beforeEach(() => { vi.stubEnv("POCKETBASE_ADMIN_PASSWORD", "test-password"); dnBatchesUpdate.mockReset(); + dnBatchNotesGetFullList.mockReset(); + dnBatchNotesGetFullList.mockResolvedValue([]); viPbAdmin.mockReset(); viPbAdmin.mockResolvedValue(makePb() as never); @@ -177,3 +182,33 @@ describe("POST /api/dn-batches/[id]/dismiss — admin auth", () => { expect(res.status).toBe(200); }); }); + +describe("POST /api/dn-batches/[id]/dismiss — promoted notes are not erased", () => { + it("records how many notes really reached the ledger", async () => { + // The bug this replaces: notes_promoted was hardcoded to 0. A batch + // that had already promoted 8 of 13 notes then reported nothing + // while 8 delivery notes sat in the figures people pay from. + dnBatchNotesGetFullList.mockResolvedValue([ + { id: "n1", status: "promoted", delivery_note: "dn1" }, + { id: "n2", status: "promoted", delivery_note: "dn2" }, + { id: "n3", status: "blocked", delivery_note: "" }, + ]); + + const res = await POST(post(), { params: Promise.resolve({ id: ID }) }); + expect(res.status).toBe(200); + + const u = allUpdates().at(-1)!; + expect(u.data.notes_promoted).toBe(2); + expect(u.data.status).toBe("processed"); + const checks = u.data.checks as { reason: string }[]; + expect(checks[0].reason).toContain("2 note(s) had promoted"); + }); + + it("still reports zero when nothing promoted", async () => { + dnBatchNotesGetFullList.mockResolvedValue([ + { id: "n1", status: "blocked", delivery_note: "" }, + ]); + await POST(post(), { params: Promise.resolve({ id: ID }) }); + expect(allUpdates().at(-1)!.data.notes_promoted).toBe(0); + }); +}); diff --git a/src/app/api/dn-batches/[id]/dismiss/route.ts b/src/app/api/dn-batches/[id]/dismiss/route.ts index d560e80..0df7716 100644 --- a/src/app/api/dn-batches/[id]/dismiss/route.ts +++ b/src/app/api/dn-batches/[id]/dismiss/route.ts @@ -7,12 +7,18 @@ export const runtime = "nodejs"; /** * POST /api/dn-batches/[id]/dismiss * - * Marks a batch processed without promoting anything. + * Marks a whole batch processed without promoting anything further. * - * Needed because the four notes of scan_20260915_c5p7fkmzgv.pdf were - * captured by hand during phase 1, so when this pipeline reads that - * file the not_duplicate check will correctly block every note. Without - * this action that card would sit in Needs attention forever. + * Needed for a document whose notes were captured by hand before the + * pipeline existed: not_duplicate correctly blocks every one, and the + * card would otherwise sit in Needs attention forever. + * + * NOT for duplicate batches. Duplication happens per note — the same + * stack of Vula invoices was scanned three times in overlapping + * groupings, so each of those batches mixes promoted notes, duplicates + * of another batch, and invoices unique to it. Dismissing one wholesale + * discards the unique ones. Use + * POST /api/dn-batches/[id]/notes/[noteId]/dismiss instead. */ export async function POST( _request: Request, @@ -24,17 +30,37 @@ export async function POST( } const pb = await pbAdmin(); + + // Count what is genuinely on the ledger rather than assuming none. + // This used to write notes_promoted: 0 unconditionally, which for a + // batch that had already promoted 8 of its 13 notes left the board + // reporting nothing while 8 delivery notes sat in the figures people + // pay from. + const promoted = ( + await pb + .collection("dn_batch_notes") + .getFullList({ filter: `batch='${id}'`, fields: "id,status,delivery_note" }) + .catch(() => []) + ).filter( + (n) => + (n as { status?: string }).status === "promoted" || + Boolean((n as { delivery_note?: string }).delivery_note), + ).length; + try { await pb.collection("dn_batches").update(id, { status: "processed", - notes_promoted: 0, + notes_promoted: promoted, claimed_at: "", last_error: "", checks: [ { id: "dismissed", passed: true, - reason: "marked as already ingested by an operator", + reason: + promoted > 0 + ? `marked as already ingested by an operator; ${promoted} note(s) had promoted and remain on the ledger` + : "marked as already ingested by an operator", }, ], }); diff --git a/src/app/api/dn-batches/[id]/notes/[noteId]/dismiss/route.ts b/src/app/api/dn-batches/[id]/notes/[noteId]/dismiss/route.ts new file mode 100644 index 0000000..da603b5 --- /dev/null +++ b/src/app/api/dn-batches/[id]/notes/[noteId]/dismiss/route.ts @@ -0,0 +1,119 @@ +import { NextResponse } from "next/server"; +import { pbAdmin } from "@/lib/pb"; + +export const dynamic = "force-dynamic"; +export const runtime = "nodejs"; + +/** + * POST /api/dn-batches/[id]/notes/[noteId]/dismiss + * Body: { reason?: string } + * + * Stands one candidate note down without promoting it. + * + * This exists because duplication happens per NOTE, not per batch. The + * same stack of Vula invoices was scanned three times in overlapping + * groupings — 13, 16 and 14 pages, three different file hashes, so + * upload-time hash dedup could not see it. Each of those batches holds a + * mix of notes that promoted, notes duplicating another batch, and notes + * unique to it. `vdn074468` alone carries four invoices that exist + * nowhere else. + * + * So there is no correct batch-level action: dismissing the batch either + * discards unique invoices or zeroes a counter for notes already on the + * ledger. Dismissing the individual duplicate note is the only safe + * granularity. + * + * Refuses to touch a promoted note. Its delivery note is already on the + * ledger, and standing the candidate down would leave the two disagreeing + * — delete the delivery note first if that is really the intent. + */ +export async function POST( + request: Request, + { params }: { params: Promise<{ id: string; noteId: string }> }, +) { + const { id, noteId } = await params; + if (!/^[a-z0-9]{15}$/i.test(id) || !/^[a-z0-9]{15}$/i.test(noteId)) { + return NextResponse.json({ error: "invalid_id" }, { status: 400 }); + } + + let reason = "duplicate of a note already ingested"; + try { + const body = await request.json(); + if (typeof body?.reason === "string" && body.reason.trim()) reason = body.reason.trim(); + } catch { + // Body is optional. + } + + const pb = await pbAdmin(); + + const note = await pb + .collection("dn_batch_notes") + .getOne(noteId, { fields: "id,batch,status,delivery_note,delivery_note_number,checks" }) + .catch(() => null); + if (!note) return NextResponse.json({ error: "note_not_found" }, { status: 404 }); + if (note.batch !== id) { + return NextResponse.json({ error: "note_not_in_batch" }, { status: 400 }); + } + if (note.status === "promoted" || note.delivery_note) { + return NextResponse.json( + { + error: "already_promoted", + detail: "this note is on the ledger — delete its delivery note first", + }, + { status: 409 }, + ); + } + + const checks = Array.isArray(note.checks) ? note.checks : []; + + try { + await pb.collection("dn_batch_notes").update(noteId, { + status: "dismissed", + checks: [ + ...checks, + { + id: "dismissed", + passed: true, + reason: `${reason} (dismissed ${new Date().toISOString()})`, + }, + ], + }); + } catch (err) { + return NextResponse.json( + { error: err instanceof Error ? err.message : "dismiss_failed" }, + { status: 500 }, + ); + } + + // A batch whose every note is now either promoted or dismissed has + // nothing left to answer for. + const siblings = await pb + .collection("dn_batch_notes") + .getFullList({ filter: `batch='${id}'`, fields: "id,status" }) + .catch(() => []); + const outstanding = siblings.filter( + (s) => (s as { status?: string }).status === "blocked" || (s as { status?: string }).status === "candidate", + ).length; + + if (outstanding === 0) { + const batch = await pb + .collection("dn_batches") + .getOne(id, { fields: "id,open_pages" }) + .catch(() => null); + const openPages = Array.isArray(batch?.open_pages) + ? (batch!.open_pages as { dismissed?: boolean }[]).filter((p) => !p.dismissed).length + : 0; + if (openPages === 0) { + await pb + .collection("dn_batches") + .update(id, { status: "processed" }) + .catch(() => undefined); + } + } + + return NextResponse.json({ + ok: true, + note: note.delivery_note_number ?? "", + outstanding_notes: outstanding, + }); +}