fix(delivery-notes): return to the list after quoting; kill the phantom supplier

Two things the operator hit straight away.

1. Creating a quote parked her on the quote detail page.
   She is working through notes one after another, so that pulled her
   out of the list for a document she had just reviewed in the modal.
   `afterCreate` now defaults to "list": the modal closes, the toast
   confirms, and she is back where she was with the row's first slot
   now showing the quote number.

2. "Couldn't load this quote's PDF" — blaming a supplier.
   The message was right about the failure and wrong about the cause.
   The readiness rule existed in TWO places: `quotes-service.ts` for
   the send path, `quote-pdf-service.ts` for the PDF route. When the
   supplier requirement was dropped I changed one of them and left the
   other enforcing it. So a counter note whose lines never had a
   supplier could be quoted and sent but never rendered.

   Both copies now call one function, `quoteItemReadinessGaps`, so a
   third copy cannot appear. Verified in the browser: Q202609631 now
   fails with "Every item still needs: cost price, not processed,
   selling price" — no supplier — and that quote's line genuinely has
   cost 0, selling 0 and is_processed false, so refusing it is correct.
   The phantom requirement is gone; the real blockers are named.

   `is_processed` is deliberately KEPT. It is the flag that says a
   person reviewed the line, and nothing else can set it on a
   delivery-note quote — the quote_items update whitelists exclude it.
   Dropping it would let a quote go out with lines nobody looked at.

The quotes-service test mock now spreads the real module, so these
tests run against the real rule rather than a stub that could agree
with the implementation while production disagreed.

Full suite 3676 passing, tsc clean, no new lint errors.

src/components/delivery-notes/quote-review-dialog.tsx

@@ -68,6 +68,19 @@ export interface QuoteReviewDialogProps {
   customerName?: string;
   /** Fired after a successful create, with the new quote. */
   onCreated?: (result: { id: string; quote_number: string }) => void;
+  /**
+   * Where to go once the quote exists.
+   *
+   * `"list"` returns to the delivery-notes list, which is what the
+   * operator asked for: create the quote, then carry on with the next
+   * note. Landing on the quote detail page pulled her out of the list
+   * she was working through, for a document she had just already
+   * reviewed in this very modal.
+   *
+   * `"quote"` opens the quote itself — kept for any caller that wants
+   * to land on the finished document.
+   */
+  afterCreate?: "list" | "quote";
 }
 
 export function QuoteReviewDialog({
@@ -77,6 +90,7 @@ export function QuoteReviewDialog({
   deliveryNoteNumber,
   customerName,
   onCreated,
+  afterCreate = "list",
 }: QuoteReviewDialogProps) {
   const router = useRouter();
 
@@ -201,7 +215,15 @@ export function QuoteReviewDialog({
       toast.success(`Created quote ${body.quote_number}`);
       onOpenChange(false);
       onCreated?.({ id: body.id, quote_number: body.quote_number ?? "" });
-      router.push(`/quotes/${body.id}`);
+      if (afterCreate === "quote") {
+        router.push(`/quotes/${body.id}`);
+      } else {
+        // Back to the list. The operator is working through notes one
+        // after another, so we hand her back where she was rather than
+        // parking her on a document she has already seen the contents
+        // of in this modal.
+        router.push("/delivery-notes");
+      }
     } catch (err: unknown) {
       toast.error(err instanceof Error ? err.message : "Failed to create the quote");
     } finally {

src/lib/server/quote-pdf-service.test.ts

@@ -607,7 +607,12 @@ describe("generateQuotePdf", () => {
       expect(outcome.code).toBe("items_not_ready");
       expect(outcome.details.issues?.length).toBe(1);
       expect(outcome.details.issues?.[0].description).toBe("Unpriced item");
-      expect(outcome.details.issues?.[0].missing).toContain("no supplier");
+      // A missing supplier is NOT a blocker — it was removed, and this
+      // is the copy of the rule that kept enforcing it after the other
+      // copy had dropped it. Assert the real blockers instead, and
+      // assert the supplier is absent so a future re-add is deliberate.
+      expect(outcome.details.issues?.[0].missing).toContain("cost price");
+      expect(outcome.details.issues?.[0].missing).not.toContain("no supplier");
     }
   });
 

src/lib/server/quote-pdf-service.ts

@@ -336,14 +336,19 @@ export async function generateQuotePdf(
     //   - selling_price > 0     (we computed a sell price)
     // Items missing any of these get flagged with `missing` so the UI
     // can highlight them.
-    const ready: QuoteItem[] = [];
+        const ready: QuoteItem[] = [];
     const issues: QuotePdfReadinessIssue[] = [];
     for (const it of allItems) {
-      const missing: string[] = [];
-      if (!it.is_processed) missing.push("not processed");
-      if (!it.supplier) missing.push("no supplier");
-      if (!(it.cost_price > 0)) missing.push("cost price");
-      if (!(it.selling_price > 0)) missing.push("selling price");
+      // The rule lives in `quoteItemReadinessGaps` — ONE place.
+      //
+      // It used to be written out twice (here and in
+      // `quotes-service.ts`, for the send-email path) and the two
+      // copies drifted: the supplier requirement was removed from one
+      // and left in the other. Symptom: a quote the send path accepted
+      // still refused to render, with "no supplier" in the message, on a
+      // counter note whose lines never carried one. Two copies of a
+      // business rule is one more than this codebase should have.
+      const missing = quoteItemReadinessGaps(it);
       if (missing.length === 0) {
         ready.push(it);
       } else {
@@ -627,3 +632,58 @@ export async function isQuotePdfStale(quoteId: string): Promise<boolean> {
   // only the quote-level timestamp decides in that case.
   return (itemAt !== undefined && itemAt > pdfAt) || quoteAt > pdfAt;
 }
+
+/* ============================================================================
+ * Shared readiness rule
+ * ========================================================================= */
+
+/**
+ * What, if anything, is missing before a quote item can be priced and
+ * drawn on a customer-facing PDF.
+ *
+ * ## Why this is one function
+ *
+ * It was previously written out twice — once here for the PDF route,
+ * once in `quotes-service.ts` for the send-email route. The copies
+ * drifted: the `no supplier` requirement was removed from one and left
+ * in the other. A quote the send path happily accepted would then
+ * refuse to render, blaming a supplier on a counter note whose lines
+ * never carried one. Verified in the browser: quote Q202609631 failed
+ * with "no supplier" long after the requirement was dropped.
+ *
+ * ## Why no supplier
+ *
+ * A delivery note raised at the counter has no parent order, so its
+ * lines routinely have no supplier recorded. Requiring one refused
+ * the PDF for a field the counter form never asks for. The supplier
+ * is still PRINTED on the document when one is present.
+ *
+ * ## Why `is_processed`
+ *
+ * Kept deliberately. It is the flag that says a person looked at the
+ * line, and nothing else can set it on a delivery-note quote — the
+ * `quote_items` update whitelists exclude it. Dropping it would let a
+ * quote go out with lines nobody ever reviewed.
+ *
+ * ## `isNoQuote`
+ *
+ * A decline letter is explicitly unpriced, so the pricing gaps are
+ * skipped. The caller (the send-email path) still runs this so it can
+ * report on the decline.
+ */
+export function quoteItemReadinessGaps(
+  it: {
+    is_processed?: boolean;
+    cost_price?: number | null;
+    selling_price?: number | null;
+  },
+  opts: { isNoQuote?: boolean } = {},
+): string[] {
+  const missing: string[] = [];
+  if (!opts.isNoQuote) {
+    if (!it.is_processed) missing.push("not processed");
+    if (!(Number(it.cost_price) > 0)) missing.push("cost price");
+    if (!(Number(it.selling_price) > 0)) missing.push("selling price");
+  }
+  return missing;
+}

src/lib/server/quotes-service.test.ts

@@ -247,12 +247,17 @@ const { pbMock, clearStore } = vi.hoisted(() => {
   return { store, pbMock, clearStore };
 });
 
-vi.mock("./quote-pdf-service", () => {
+vi.mock("./quote-pdf-service", async (importOriginal) => {
   // The `*Mock` aliases point at the SAME fns the service imports, so
   // tests can configure them via `await import("./quote-pdf-service")`.
   const isQuotePdfStale = vi.fn();
   const generateQuotePdf = vi.fn();
   return {
+    // Spread the real module first. `quoteItemReadinessGaps` is the
+    // ONE readiness rule both the PDF route and the send path use —
+    // it must be the real implementation, not a stub, or these tests
+    // would pass against a fake rule while production used another.
+    ...(await importOriginal<typeof import("./quote-pdf-service")>()),
     isQuotePdfStale,
     generateQuotePdf,
     isQuotePdfStaleMock: isQuotePdfStale,

src/lib/server/quotes-service.ts

@@ -1,4 +1,5 @@
 import { getAdminPocketBase } from "./pocketbase-admin";
+import { quoteItemReadinessGaps } from "./quote-pdf-service";
 import { compressFile } from "@/lib/server/attachment-compressor";
 import { QUOTE_VALID_DAYS } from "@/lib/pdf/format";
 import { createQuoteItem } from "./quote-items-service";
@@ -2012,24 +2013,14 @@ async function diagnosePdfReadiness(
     missing: string[];
   }> = [];
   for (const it of allItems) {
-    const missing: string[] = [];
-    // Pricing / processed checks are skipped when the operator has
-    // marked this quote as a decline (`is_no_quote`) — they explicitly
-    // don't want a priced PDF.
-    if (!quote.is_no_quote) {
-      if (!it.is_processed) missing.push("not processed");
-      if (!(Number(it.cost_price) > 0)) missing.push("cost price");
-      if (!(Number(it.selling_price) > 0)) missing.push("selling price");
-    }
-    // A missing supplier is NO LONGER a blocker. It used to be, and
-    // that was wrong for this workflow: a delivery note raised at the
-    // counter (no parent order, so no procurement trail) often has
-    // goods with no supplier recorded against the line at all. The
-    // block meant the operator could quote the note, see the quote
-    // appear, and then be refused a PDF for a field that the counter
-    // workflow never asks for. The supplier is still printed on the
-    // document when one IS present — we just no longer refuse to
-    // render without it.
+    // ONE readiness rule, shared with the PDF route via
+    // `quoteItemReadinessGaps`. It used to be written out here AND in
+    // quote-pdf-service.ts, and the two copies drifted — the supplier
+    // requirement was dropped from this one and left in that one, so a
+    // quote could be sent but never rendered.
+    const missing = quoteItemReadinessGaps(it, {
+      isNoQuote: Boolean(quote.is_no_quote),
+    });
     if (missing.length > 0) {
       issues.push({
         itemId: String(it.id),