feat(dn-batches): dismiss a single duplicate note, and stop erasing promoted counts
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 with three different file hashes, so upload-time hash dedup could not see it. Each of those batches mixes notes that promoted, notes duplicating another batch, and invoices unique to it: vdn074468 alone carries four that exist nowhere else. So no batch-level action is correct. Dismissing one wholesale either discards unique invoices or zeroes a counter for notes already on the ledger. Adds POST /api/dn-batches/[id]/notes/[noteId]/dismiss, which stands one note down, refuses to touch a promoted one, and parks the batch only once nothing is outstanding. Also fixes the batch dismiss route, which wrote notes_promoted: 0 unconditionally. A batch that had promoted 8 of 13 notes would report nothing while 8 delivery notes sat in the figures people pay from — the same board-versus-ledger disagreement already fixed in run.ts. It now counts what actually reached the ledger and says so in the reason. 929 tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
293606448e
commit
2c5cde6bce
4 changed files with 247 additions and 13 deletions
54
scripts/pb-add-dn-note-dismissed.py
Normal file
54
scripts/pb-add-dn-note-dismissed.py
Normal file
|
|
@ -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.")
|
||||
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
},
|
||||
],
|
||||
});
|
||||
|
|
|
|||
119
src/app/api/dn-batches/[id]/notes/[noteId]/dismiss/route.ts
Normal file
119
src/app/api/dn-batches/[id]/notes/[noteId]/dismiss/route.ts
Normal file
|
|
@ -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,
|
||||
});
|
||||
}
|
||||
Loading…
Reference in a new issue