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

fix(quotes): delivery-note quotes skip approvals; company follows customer

Browse source 
Two quote-flow defects found while auditing Q202610315.

1. Delivery-note quotes were being dumped into the rep approval queue.

   A DN quote is never sent on its own — it goes out with the note, via
   the note's "Send the delivery note and quote to the customer" button.
   But `generateQuotePdf` auto-flipped every non-terminal quote to
   `approval_pending`, which overwrote the DN status before the
   skip-approval branch in `sendQuoteEmail` could ever run. Ten DN
   quotes were sitting in Pending Approval as a result.

   - `createQuoteFromDeliveryNote` now creates the quote `approved`
     rather than `delivery_note`.
   - The PDF render no longer flips DN quotes at all, as a backstop.
   - The quote page gets a non-sending action row pointing back at the
     note. No "send" button — a second way to email a customer the same
     invoice is a liability, not a feature.

   Also fixes a latent duplicate-invoice bug: the duplicate guard
   pinned `status = "delivery_note"`, so once the quote had been sent
   it fell through and a second quote could be raised for the same
   note. Verified by test — the old guard silently minted a duplicate.

2. `quotes.company` never followed a customer change.

   `quotes.company` is a denormalised copy of `customer.company` and
   PB does not re-cascade relations, so swapping the customer left the
   old company behind forever (Q202610315 read "Mre" while its
   customer sat at Harmony — two contradictory companies on one row).

   The customer swap ran as a raw client-side PB PATCH that wrote only
   `customer`, which also bypassed the server's placeholder guard. It
   now goes through `PATCH /api/v1/quotes/[id]/pdf`, which cascades
   both relations atomically.

   The customer-facing PDF already read `customer.expand.company`,
   which is why this never reached a customer — but every consumer of
   the quote's own `company` got the wrong answer. A read-only audit
   (`scripts/audit-quote-company-drift.mjs`) puts it at 2,591 of 9,950
   quotes. That data is NOT backfilled here.

Also drops the duplicate standalone "COMPANY" column on the quote
detail page, and the stale-company fallback in approvals, so this
class of bug can't hide in the UI again.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Dave 2026-10-16 00:44:45 +02:00
parent 248c390184
commit 3b926570c7
11 changed files with 549 additions and 29 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

89
scripts/audit-quote-company-drift.mjs Normal file
View file

@ -0,0 +1,89 @@
/**
* READ-ONLY audit: quotes whose `company` relation disagrees with
* their customer's `company`.
*
* `quotes.company` is a denormalised copy of `customer.company` that
* PocketBase does not re-cascade, so any quote whose customer was
* swapped after creation is now inconsistent. This script only READS
* and prints — it makes no writes.
*
* Run: npx tsx scripts/audit-quote-company-drift.mjs
*/
import PocketBase from "pocketbase";
const PB_URL = process.env.PB_URL ?? "https://pocketbase-gnqc88suy8fqk6wyryjjdpvl.169.239.180.130.sslip.io";
const PB_ADMIN_EMAIL = process.env.PB_ADMIN_EMAIL ?? "david.els1@outlook.com";
const PB_ADMIN_PASSWORD = process.env.PB_ADMIN_PASSWORD ?? "1234567890";
const pb = new PocketBase(PB_URL);
await pb.collection("_superusers").authWithPassword(PB_ADMIN_EMAIL, PB_ADMIN_PASSWORD);
const quotes = await pb.collection("quotes").getFullList(200, {
expand: "customer,customer.company,company",
sort: "-created",
});
// Company records store their display name in `company`, not `name`.
const nameOf = (expand, key) => expand?.[key]?.company ?? null;
// When an expand doesn't resolve, show the raw relation id rather than a
// bare "(none)" — otherwise "unresolved" and "genuinely empty" look identical.
const label = (name, id) => (name ? name : id ? `<id ${id}>` : "(none)");
let checked = 0;
let consistent = 0;
let noCustomerCompany = 0;
const drifted = [];
for (const q of quotes) {
if (!q.customer) continue;
checked++;
const cust = q.expand?.customer;
const custCompanyId = cust?.company ?? "";
const custCompanyName = nameOf(cust?.expand, "company");
if (!custCompanyId) {
noCustomerCompany++;
if (q.company) {
drifted.push({
n: q.quote_number,
id: q.id,
status: q.status,
quoteCompany: label(nameOf(q.expand, "company"), q.company),
custCompany: "(customer has none)",
created: (q.created ?? "").slice(0, 10),
});
}
continue;
}
if (q.company === custCompanyId) {
consistent++;
} else {
drifted.push({
n: q.quote_number,
id: q.id,
status: q.status,
quoteCompany: label(nameOf(q.expand, "company"), q.company),
custCompany: custCompanyName ? label(custCompanyName, custCompanyId) : label(null, custCompanyId),
created: (q.created ?? "").slice(0, 10),
});
}
}
console.log(`Scanned : ${quotes.length} quotes`);
console.log(`With customer: ${checked}`);
console.log(`Consistent : ${consistent}`);
console.log(`No cust co : ${noCustomerCompany} (customer has no company)`);
console.log(`DRIFTED : ${drifted.length}`);
console.log("");
if (drifted.length) {
console.log("quote created status quote.company -> customer.company");
console.log("-".repeat(84));
for (const d of drifted.slice(0, 60)) {
console.log(
`${String(d.n).padEnd(10)} ${d.created} ${String(d.status).padEnd(18)} ${d.quoteCompany} -> ${d.custCompany}`,
);
}
if (drifted.length > 60) console.log(`… and ${drifted.length - 60} more`);
}

80
src/app/(app)/quotes/[id]/page.tsx
View file

@ -82,6 +82,7 @@ import {
Loader2,
Settings,
Activity,
Truck,
type LucideIcon,
} from "lucide-react";
import { toast } from "sonner";
@ -2028,6 +2029,58 @@ export default function QuoteDetailPage({ params }: { params: Promise<{ id: stri
const renderActions = () => {
const btns: React.ReactNode[] = [];
/**
* Delivery-note quotes get a small, deliberately non-sending action
* row (2026-10-16).
*
* A DN quote shares its statuses with the regular pipeline, so it
* can't own a clean `switch` arm without duplicating three existing
* ones. Branch on the `delivery_note` relation before the switch.
*
* NOTE: there is deliberately NO "send" button here. A
* delivery-note quote is never sent on its own — it goes out with
* the note, via the delivery note's own "Send the delivery note and
* quote to the customer" button. The quote is created pre-approved
* (`createQuoteFromDeliveryNote`), so the rep approval queue never
* sees it. This row only points back at the note where Send lives,
* plus preview/re-price. An earlier draft of this fix added a
* "Send to customer" button that POSTed the quote directly — that
* would have created a second, competing way to email a customer
* the same invoice.
*/
if (
quote.delivery_note &&
(quote.status === "delivery_note" ||
quote.status === "claimed" ||
quote.status === "processed" ||
quote.status === "approved")
) {
return (
<div className="flex flex-wrap items-center gap-2">
<Button variant="outline" size="sm" onClick={() => setViewQuoteOpen(true)} disabled={actionLoading !== ""}>
<FileText className="mr-2 h-4 w-4" />
Preview PDF
</Button>
<Button
variant="outline"
size="sm"
onClick={() => router.push(`/delivery-notes/${quote.delivery_note}`)}
disabled={actionLoading !== ""}
>
<Truck className="mr-2 h-4 w-4" />
Go to delivery note
</Button>
<Button variant="outline" size="sm" onClick={() => router.push(`/quotes/${id}/process`)} disabled={actionLoading !== ""}>
<RefreshCw className="mr-2 h-4 w-4" />
{quote.status === "processed" ? "Re-price" : "Price items"}
</Button>
<span className="text-xs text-muted-foreground">
Sent together with the delivery note — not on its own.
</span>
</div>
);
}
switch (quote.status) {
case "new":
btns.push(
@ -2072,6 +2125,8 @@ export default function QuoteDetailPage({ params }: { params: Promise<{ id: stri
// audit trail.
// - Submit for Approval: PDF generation IS the submit.
// The PDF route auto-flips to approval_pending on render.
// (Regular quotes only — delivery-note quotes are exempt
// from that flip and return early above.)
btns.push(
<Button key="process2" variant="outline" size="sm" onClick={() => router.push(`/quotes/${id}/process`)}>
<RefreshCw className="mr-2 h-4 w-4" />
@ -2375,11 +2430,22 @@ export default function QuoteDetailPage({ params }: { params: Promise<{ id: stri
{/*
Compact customer strip — replaces the old "Customer" sidebar
card. Lifts the customer / company / dates / letterhead /
claimed-by info onto a single horizontal band directly below
the header so the operator sees "who is this quote for"
before they decide which sidebar panel to open. Mobile wraps
the columns; desktop shows them inline.
card. Lifts the customer / dates / letterhead / claimed-by info
onto a single horizontal band directly below the header so the
operator sees "who is this quote for" before they decide which
sidebar panel to open. Mobile wraps the columns; desktop shows
them inline.
The company is rendered ONCE, nested under the customer name
(derived via `customer.company`). There used to be a second
standalone "Company" column reading `quote.company` — the
quote's own denormalised company relation. It was removed
2026-10-16 because it was redundant AND wrong: nothing
re-cascades `quotes.company` when the customer is changed, so
the two columns drifted apart. Quote Q202610315 (Pierre Visser
/ Harmony - Moab Khotsong) showed the real company under the
customer and a stale "Mre" in the standalone column. The
customer-derived value is the authoritative one.
*/}
<Card>
<CardContent className="pt-4 pb-4">
@ -2393,10 +2459,6 @@ export default function QuoteDetailPage({ params }: { params: Promise<{ id: stri
</p>
)}
</div>
<div>
<p className="text-[10px] uppercase tracking-wider text-muted-foreground">Company</p>
<p className="font-medium">{quote.expand?.company?.company || "—"}</p>
</div>
<div>
<p className="text-[10px] uppercase tracking-wider text-muted-foreground">Created</p>
<p className="font-medium">{formatDate(quote.created)}</p>

37
src/app/(app)/quotes/[id]/process/page.tsx
View file

@ -843,20 +843,43 @@ export default function QuoteProcessPage({ params }: { params: Promise<{ id: str
/**
* Swap the customer on this quote. Called by the `<CustomerSearch>`
* dropdown in the header. PATCHes the quote on the server, refreshes
* the quote record from PB, and toasts. Refuses placeholder picks
* both client-side (the search hides them) and server-side (the API
* rejects them with 422).
* dropdown in the header. Refuses placeholder picks both client-side
* (the search hides them) and server-side (the API rejects them
* with 422).
*
* Goes through `PATCH /api/v1/quotes/[id]/pdf` rather than writing
* to PocketBase from the browser. It used to call
* `pb.collection("quotes").update(...)` directly, which had two
* problems:
*
* 1. It wrote ONLY `customer`, leaving `quotes.company` pointing
* at the old customer's company forever — quote Q202610315
* ended up with `company = Mre` while its customer sat at
* Harmony. The route now cascades `company` from the new
* customer.
* 2. It bypassed the server-side placeholder/email guard that
* this route enforces, by writing with the browser's own PB
* credentials.
*/
const changeCustomer = async (newCustomerId: string) => {
if (!quote || !newCustomerId || newCustomerId === quote.customer) return;
try {
const pb = getPocketBase();
await pb.collection("quotes").update(id, { customer: newCustomerId });
const res = await fetch(`/api/v1/quotes/${id}/pdf`, {
method: "PATCH",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({ customer: newCustomerId }),
});
if (!res.ok) {
const body = (await res.json().catch(() => null)) as { error?: string } | null;
throw new Error(body?.error || `Failed to change customer (${res.status})`);
}
toast.success("Customer updated");
// Pull the freshly-expanded quote so the header reflects the change.
// expand=company too — the header shows the customer's company, and
// the cascade just rewrote it.
const pb = getPocketBase();
const fresh = await pb.collection("quotes").getOne<Quote>(id, {
expand: "customer",
expand: "customer,customer.company",
});
setQuote(fresh);
} catch (err) {

11
src/app/(app)/quotes/approvals/[id]/page.tsx
View file

@ -626,9 +626,14 @@ export default function ApprovalReviewPage() {
}
const customerName = quote.expand?.customer?.customer ?? "—";
const companyName = quote.expand?.customer?.expand?.company?.company
?? quote.expand?.company?.company
?? "";
// Company comes from the CUSTOMER, never from the quote's own
// `company` relation. That relation is a denormalised copy which
// used to go stale whenever the quote's customer was changed
// (Q202610315 showed "Mre" while its customer sat at Harmony).
// The customer cascade now keeps them in step, but falling back to
// the stale copy would just re-hide the same class of bug — and a
// missing company is a harmless blank, a wrong one is not.
const companyName = quote.expand?.customer?.expand?.company?.company ?? "";
const repName = quote.expand?.customer?.expand?.rep?.name ?? "";
// Frozen at fetch time so the value is stable across re-renders
// (React's purity rule: no Date.now() during render). The rep sees

138
src/app/api/v1/quotes/[id]/pdf/route.test.ts Normal file
View file

@ -0,0 +1,138 @@
import { describe, it, expect, beforeEach, vi } from "vitest";
import { NextRequest } from "next/server";
/**
* Tests for `PATCH /api/v1/quotes/[id]/pdf` — the customer-swap branch.
*
* The route exists so the process page can replace a placeholder
* customer with the real one. It also owns the `quotes.company`
* cascade: `quotes.company` is a denormalised copy of the customer's
* company and PB does not re-cascade relations, so writing only
* `customer` leaves the quote pointing at the previous customer's
* company forever (see quote Q202610315, which held `company = Mre`
* while its customer sat at Harmony).
*/
const { getAdminPocketBase } = vi.hoisted(() => ({
getAdminPocketBase: vi.fn(),
}));
vi.mock("@/lib/pocketbase", () => ({
default: vi.fn(),
}));
vi.mock("@/lib/server/pocketbase-admin", () => ({
getAdminPocketBase: (...args: unknown[]) => getAdminPocketBase(...args),
}));
vi.mock("@/lib/server/quote-pdf-service", () => ({
generateQuotePdf: vi.fn(),
isQuotePdfStale: vi.fn(),
QuotePdfError: class QuotePdfError extends Error {},
}));
import { PATCH } from "./route";
type Rec = Record<string, unknown>;
/** Minimal in-memory PB stand-in: customers + quotes collections. */
function makePb(customers: Rec[], quotes: Rec[]) {
const update = vi.fn(async (id: string, data: Rec) => {
const row = quotes.find((q) => q.id === id);
if (!row) throw new Error("not found");
Object.assign(row, data);
return { ...row };
});
const getOne = vi.fn(async (id: string) => {
const c = customers.find((x) => x.id === id);
if (!c) throw new Error("not found");
return { ...c };
});
return {
collection: vi.fn((name: string) =>
name === "customers" ? { getOne } : { update }
),
__update: update,
};
}
function makeReq(id: string, body: unknown) {
return new NextRequest(`http://localhost/api/v1/quotes/${id}/pdf`, {
method: "PATCH",
headers: { "content-type": "application/json" },
body: JSON.stringify(body),
});
}
const CUSTOMER_WITH_COMPANY = {
id: "cust-harmony",
customer: "Pierre Visser",
email: "pierre.visser@harmony.co.za",
company: "comp-harmony",
};
const CUSTOMER_NO_COMPANY = {
id: "cust-solo",
customer: "Walk-in Buyer",
email: "walkin@buyer.co.za",
company: "",
};
describe("PATCH /api/v1/quotes/[id]/pdf — customer swap", () => {
beforeEach(() => {
vi.clearAllMocks();
});
it("cascades quotes.company to the new customer's company", async () => {
const quote = { id: "q1", customer: "cust-old", company: "comp-mre" };
const pb = makePb([CUSTOMER_WITH_COMPANY], [quote]);
getAdminPocketBase.mockResolvedValue(pb);
const res = await PATCH(makeReq("q1", { customer: "cust-harmony" }), {
params: Promise.resolve({ id: "q1" }),
});
expect(res.status).toBe(200);
// The whole point — both relations move together.
expect(pb.__update).toHaveBeenCalledWith("q1", {
customer: "cust-harmony",
company: "comp-harmony",
});
expect(quote.company).toBe("comp-harmony");
});
it("clears the company when the new customer has none", async () => {
// Must not leave the PREVIOUS customer's company dangling.
const quote = { id: "q1", customer: "cust-old", company: "comp-mre" };
const pb = makePb([CUSTOMER_NO_COMPANY], [quote]);
getAdminPocketBase.mockResolvedValue(pb);
await PATCH(makeReq("q1", { customer: "cust-solo" }), {
params: Promise.resolve({ id: "q1" }),
});
expect(pb.__update).toHaveBeenCalledWith("q1", {
customer: "cust-solo",
company: "",
});
expect(quote.company).toBe("");
});
it("422s when no customer id is supplied", async () => {
const res = await PATCH(makeReq("q1", {}), {
params: Promise.resolve({ id: "q1" }),
});
expect(res.status).toBe(422);
});
it("422s and writes nothing when the customer has no email", async () => {
const noEmail = { ...CUSTOMER_WITH_COMPANY, email: "" };
const pb = makePb([noEmail], [{ id: "q1" }]);
getAdminPocketBase.mockResolvedValue(pb);
const res = await PATCH(makeReq("q1", { customer: "cust-harmony" }), {
params: Promise.resolve({ id: "q1" }),
});
expect(res.status).toBe(422);
expect(pb.__update).not.toHaveBeenCalled();
});
});

17
src/app/api/v1/quotes/[id]/pdf/route.ts
View file

@ -197,7 +197,7 @@ export async function PATCH(
const pb = await getAdminPocketBase();
const customer = await pb
.collection("customers")
.getOne<{ id: string; customer: string; email?: string }>(newCustomerId);
.getOne<{ id: string; customer: string; email?: string; company?: string }>(newCustomerId);
// Same placeholder block as the PDF route. We also reject if
// the new customer has no email — the operator would hit
@ -218,8 +218,23 @@ export async function PATCH(
);
}
// Reassign BOTH relations together.
//
// `quotes.company` is a denormalised copy of the customer's
// company, and nothing in PB re-cascades it. Writing only
// `customer` left the quote pointing at the OLD customer's
// company forever: quote Q202610315 (changed to Pierre Visser at
// Harmony) kept `company = Mre`, so one quote held two
// contradictory companies. The customer-facing PDF reads
// `customer.expand.company` so the customer never saw it, which
// is exactly why it went unnoticed — but any consumer reading the
// quote's own `company` got the wrong answer.
//
// Empty company (customer has none) clears the relation rather
// than leaving the previous one dangling.
const updated = await pb.collection("quotes").update(quoteId, {
customer: customer.id,
company: customer.company ?? "",
});
return NextResponse.json({ ok: true, quote: updated, customer });
} catch (err) {

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

@ -244,7 +244,9 @@ describe("createQuoteFromDeliveryNote", () => {
const quotes = Array.from(getOrCreate("quotes").values());
expect(quotes).toHaveLength(1);
const q = quotes[0] as Record<string, unknown>;
expect(q.status).toBe("delivery_note");
// Born APPROVED — a DN quote is never sent on its own, it goes out
// with the note. It must never enter the rep approval queue.
expect(q.status).toBe("approved");
expect(q.delivery_note).toBe(dnId);
expect(q.customer).toBe("customer-1");
expect(q.company).toBe("company-1");
@ -412,6 +414,26 @@ describe("createQuoteFromDeliveryNote", () => {
);
});
it("still blocks a duplicate once the existing quote has been sent", async () => {
// Regression pin. The guard used to also require
// `status = "delivery_note"`, so a quote that had already gone out
// (`emailed`) fell straight through it — a second "Create quote"
// click would then raise a duplicate invoice for the same note.
// The guard is now relation-only and strictly broader than the UI.
const dnId = seedDeliveryNote();
seedOrderItem("DN26071401");
getOrCreate("quotes").set("already-sent", {
id: "already-sent",
quote_number: "Q202607002",
status: "emailed",
delivery_note: dnId,
});
await expect(createQuoteFromDeliveryNote(dnId)).rejects.toThrow(
/already exists/,
);
});
it("throws when the delivery note has no order_items", async () => {
const dnId = seedDeliveryNote();
// No seedOrderItem call → the items lookup returns [].

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

@ -258,11 +258,21 @@ export async function createQuoteFromDeliveryNote(
// status filter is a belt-and-braces check — the operator could
// in theory have transitioned the previous quote to "sent" or
// beyond, in which case the relationship is no longer "active".
// We look for the *primary* quote (status="delivery_note" with
// delivery_note=dn.id) — a quote that has been sent and is now
// in flight doesn't count.
// Duplicate guard: has THIS note already got a quote?
//
// Match on the relation alone. The old filter also pinned
// `status = "delivery_note"`, which quietly scoped the guard to
// quotes that had never been sent — so once the quote went
// `emailed`, this returned nothing and a second quote could be
// raised for the same note (duplicate invoice to the customer).
// It also broke the moment DN quotes stopped being created in
// `delivery_note` status.
//
// The UI already hides "Create quote" when `hasQuote`, so this
// server-side check is the real backstop — it must be strictly
// broader than the UI gate, not narrower.
const existing = await pb.collection(QUOTES).getList<Quote>(1, 1, {
filter: `delivery_note = "${deliveryNoteId}" && status = "delivery_note"`,
filter: `delivery_note = "${deliveryNoteId}"`,
});
if (existing.items.length > 0) {
const q = existing.items[0];
@ -289,7 +299,27 @@ export async function createQuoteFromDeliveryNote(
// Create the quote header.
const quoteCreate: Record<string, unknown> = {
quote_number: quoteNumber,
status: "delivery_note",
// Created APPROVED — the quote skips the approval queue.
//
// Why: a delivery-note quote is never sent on its own. The office
// creates it off the note ("Create quote"), prices it in the same
// dialog, and then sends DN + quote together with the note's own
// "Send" button. There is no separate quote-send step and no rep
// in the loop — the goods have already physically gone out with
// the driver, so an approval round-trip is pure friction on an
// invoice the customer is already liable for.
//
// This used to create as `delivery_note`, which was then silently
// overwritten to `approval_pending` by the PDF render
// (`generateQuotePdf` flips every non-terminal quote). That's how
// DN quotes ended up clogging the rep's Pending Approvals queue.
// The quote is now born approved, and the PDF render leaves
// `approved` alone — so the quote never enters the approval queue.
//
// `approved` is in `RESENDABLE_STATUSES`, so the note's Send button
// (which POSTs `/api/v1/quotes/[id]/send-email`) can send it
// straight through to `sending` → `emailed`.
status: "approved",
delivery_note: dn.id,
// customer/company are required on the Quote collection schema.
// The delivery note has both, so we just copy through.

7
src/lib/server/order-delivery-note-link-service.ts
View file

@ -205,7 +205,7 @@ function mostRecent(candidates: DnCandidate[]): DnCandidate {
}
/** Among tied exact-item-match candidates, prefer the one whose priced
* quote (status="delivery_note") also matches the order's line
* quote (the quote linked to that note) also matches the order's line
* prices. Returns null if no candidate's quote prices match — caller
* falls back to most-recent. */
async function breakTieByQuotePrice(
@ -222,10 +222,13 @@ async function breakTieByQuotePrice(
}
for (const dn of candidates) {
// Status-agnostic on purpose: we want the note's quote whatever
// stage it's at. It used to pin `status = "delivery_note"`, which
// stopped matching as soon as the quote was approved or sent.
const quotes = await pb
.collection(QUOTES_COLLECTION)
.getFullList<{ id: string; delivery_note: string }>({
filter: `delivery_note = "${esc(dn.id)}" && status = "delivery_note"`,
filter: `delivery_note = "${esc(dn.id)}"`,
fields: "id,delivery_note",
batch: 5,
});

111
src/lib/server/quote-pdf-service.test.ts
View file

@ -893,6 +893,117 @@ describe("generateQuotePdf", () => {
});
});
// ─────────────────────────────────────────────────────────────────────────────
// generateQuotePdf — delivery-note quotes skip the approval queue
//
// Regression pin for the 2026-10-16 bug. The render used to auto-flip EVERY
// non-terminal quote to `approval_pending`, which forced every DN quote into
// the rep's approval queue before it could be sent — defeating the
// skip-approval path in `sendQuoteEmail` (which gates on the quote still
// being `delivery_note` / `claimed` / `processed`).
// ─────────────────────────────────────────────────────────────────────────────
describe("generateQuotePdf — delivery-note exemption", () => {
/**
* A DN quote mid-flow: real customer, a fully priced line, and the
* `delivery_note` relation set. `status` is what the assertion pins.
*/
async function seedDnQuote(status: "delivery_note" | "claimed" | "processed") {
const customer = await pbMock.collection("customers").create({
customer: "Pierre Visser",
email: "pierre.visser@harmony.co.za",
company: "Harmony",
dateadded: "",
lastvisiteddate: "",
rep: "",
coordinates: "",
description: "",
alternative_email: "",
buyer_email: "",
number: "",
});
const quote = await pbMock.collection("quotes").create({
quote_number: "Q202610315",
customer: customer.id,
status,
delivery_note: "dn-abc123",
notes: "",
email_sender: "",
email_subject: "",
email_body: "",
has_attachments: false,
company_letterhead: "ISHERWOOD",
});
await pbMock.collection("quote_items").create({
quote: quote.id,
description: "M16 hex bolt",
quantity: 1,
cost_price: 2.39,
selling_price: 3.11,
markup: 30,
supplier: "sup-acme",
is_processed: true,
moved_to_quote: null,
});
return quote;
}
it("does NOT flip a delivery_note quote into approval_pending", async () => {
const quote = await seedDnQuote("delivery_note");
const outcome = await generateQuotePdf(quote.id, { letterhead: "ISHERWOOD" });
expect(outcome.kind).toBe("ok");
if (outcome.kind !== "ok") throw new Error("expected ok outcome");
// The whole point: the status survives the render so the operator's
// send button hits the DN branch in `sendQuoteEmail`.
expect(outcome.status).toBe("delivery_note");
const after = await pbMock
.collection("quotes")
.getOne<{ status: string; pdf_generated_at: string }>(quote.id);
expect(after.status).toBe("delivery_note");
// The PDF timestamp still gets stamped — `sendQuoteEmail`'s staleness
// check depends on it being fresh.
expect(after.pdf_generated_at).toBeTruthy();
});
it("keeps a claimed delivery-note quote claimed (no approval flip)", async () => {
const quote = await seedDnQuote("claimed");
const outcome = await generateQuotePdf(quote.id, { letterhead: "CNL" });
expect(outcome.kind).toBe("ok");
if (outcome.kind !== "ok") throw new Error("expected ok outcome");
expect(outcome.status).toBe("claimed");
const after = await pbMock.collection("quotes").getOne<{ status: string }>(quote.id);
expect(after.status).toBe("claimed");
});
it("still flips a REGULAR quote to approval_pending", async () => {
// Guard against over-correcting: the exemption must be scoped to DN
// quotes. A normal quote with no `delivery_note` keeps the old
// auto-submit behaviour.
const { quote } = await seedQuoteWithPdf();
await pbMock.collection("quotes").update(quote.id, { status: "processed" });
await pbMock.collection("quote_items").create({
quote: quote.id,
description: "M16 hex bolt",
quantity: 1,
cost_price: 2.39,
selling_price: 3.11,
markup: 30,
supplier: "sup-acme",
is_processed: true,
moved_to_quote: null,
});
const outcome = await generateQuotePdf(quote.id, { letterhead: "CNL" });
expect(outcome.kind).toBe("ok");
if (outcome.kind !== "ok") throw new Error("expected ok outcome");
expect(outcome.status).toBe("approval_pending");
});
});
// ─────────────────────────────────────────────────────────────────────────────
// generateQuotePdf — auto-claim (Option 1)
//

24
src/lib/server/quote-pdf-service.ts
View file

@ -46,6 +46,9 @@ import type {
* implementation — PDF generation is the moment the quote
* becomes "ready for rep review" and replaces the old
* "Submit for Approval" button.
* - EXCEPTION (2026-10-16): delivery-note quotes
* (`quote.delivery_note` set) are NOT flipped. They skip the
* approval queue entirely — see the flip block below.
* - When the quote is already in `approval_pending` /
* `partial_approval_pending`, the route does an idempotent
* regen — `pdf_generated_at` gets bumped but status stays
@ -477,9 +480,28 @@ export async function generateQuotePdf(
: {};
let newStatus = quote.status;
let pdfGeneratedAt: string | null = quote.pdf_generated_at || null;
/**
* Delivery-note quotes skip the approval queue by design (see
* `sendQuoteEmail`'s `isDeliveryNoteQuote` branch — it accepts
* `delivery_note` / `claimed` / `processed` and flips straight to
* `sending`). That design was silently defeated here: rendering the
* PDF is the operator's only pricing step for a DN quote, and this
* function auto-flipped ANY non-terminal quote to
* `approval_pending`, so every DN quote was pushed into the rep's
* queue before it could ever be sent. The skip-approval path was
* unreachable — the status it gates on had already been changed.
*
* Fix 2026-10-16: exempt DN quotes from the flip. They keep their
* own status (`delivery_note` → `claimed` → `processed`) while the
* PDF is stamped, so the operator's send button on the quote page
* hits the DN branch and goes straight out. The approval queue now
* contains only quotes that genuinely need a rep's eyes.
*/
const isDeliveryNoteQuote = !!quote.delivery_note;
if (
TERMINAL_STATUSES.has(quote.status) ||
APPROVAL_STATES.has(quote.status)
APPROVAL_STATES.has(quote.status) ||
isDeliveryNoteQuote
) {
// Idempotent / no-flip path: still update the timestamp so
// we know the PDF was regenerated, but don't change the