From 7ce23e04f371e061acba1758909bda680f988e6d Mon Sep 17 00:00:00 2001 From: Dave Date: Mon, 5 Oct 2026 16:31:04 +0200 Subject: [PATCH] fix(delivery-notes): the detail page's Review pricing bypassed the review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by running the real flow end-to-end in a browser as David Els. The list page opened the pricing review modal; the detail page did not — its "Review pricing" button still POSTed the create directly. So the same button did two different things on two screens, and on the detail page it created the quote blind, which is exactly what the review modal exists to prevent. It now opens the same `QuoteReviewDialog`, keyed on the note id so its grid state can't leak between notes. Also fixed a message that outlived the check it described. Removing the supplier requirement left the refusal still reading "Every item needs: processed ✓, supplier ✓, cost price, and selling price" — so an operator was told to set a supplier on every line, which changes nothing now, while the three real blockers went unnamed. The message is now derived from the same `issues[].missing` array that drives the check, so the two cannot drift again. E2E evidence (live PocketBase, David Els / CNL Industrial Mining Supplies): DN26100599 -> pricing review -> Q202610102 cost R120 -> markup 25 -> selling R150, qty 10 is_processed = true in the database PDF: 200 ok, 82,184 bytes, %PDF-1.3, 2 pages, valid DN26100508 -> blind create -> Q202610101 PDF: 422 items_not_ready missing: ["not processed", "cost price", "selling price"] That is the whole bug reproduced on both sides: reviewed quotes render, blind ones are refused, and the refusal now names what is actually missing. Also verified in the browser: the stock typeahead returns name, SKU, unit, qty and supplier from the new route; picking a stock item auto-fills the supplier (the field that used to block the PDF); and the create-delivery-note dialog is unchanged. Full suite 3676 passing, tsc clean. --- src/app/(app)/delivery-notes/[id]/page.tsx | 70 ++++++++++------------ src/lib/server/quote-pdf-service.ts | 13 +++- 2 files changed, 42 insertions(+), 41 deletions(-) diff --git a/src/app/(app)/delivery-notes/[id]/page.tsx b/src/app/(app)/delivery-notes/[id]/page.tsx index 093fe4e..8035b8f 100644 --- a/src/app/(app)/delivery-notes/[id]/page.tsx +++ b/src/app/(app)/delivery-notes/[id]/page.tsx @@ -3,6 +3,7 @@ import { useCallback, useEffect, useRef, useState } from "react"; import { useRouter, useSearchParams } from "next/navigation"; import { DeliveryNoteActions } from "@/components/delivery-notes/action-buttons"; +import { QuoteReviewDialog } from "@/components/delivery-notes/quote-review-dialog"; import { use } from "react"; import { toast } from "sonner"; import { @@ -216,6 +217,8 @@ export default function DeliveryNoteDetailPage({ params }: PageProps) { // buttons don't get disabled while the file is uploading. const signedFileInputRef = useRef(null); const [signedUploading, setSignedUploading] = useState(false); + /** Pricing-review modal. The create happens inside it. */ + const [quoteReviewOpen, setQuoteReviewOpen] = useState(false); // Inline PDF preview dialog (replaces the old "open in new // tab" behavior). `true` opens the dialog with the // re-rendered DN PDF in an iframe; operator can close @@ -335,46 +338,16 @@ export default function DeliveryNoteDetailPage({ params }: PageProps) { }, [loadAll]); // ── Action: Create Quote ─────────────────────────────────────── - const handleCreateQuote = useCallback(async () => { + // Opens the pricing review modal, the SAME modal the list page + // uses. This used to POST the create directly, which meant the + // detail page created a quote with whatever pricing happened to be + // on the order items and sent the operator off to the quote page + // to fix it — the exact blind create the review modal exists to + // prevent, and the drift the operator complained about. + const handleCreateQuote = useCallback(() => { if (!deliveryNote) return; - setBusyAction("create_quote"); - try { - const res = await fetch(`/api/v1/delivery-notes/${deliveryNote.id}/create-quote`, { - method: "POST", - }); - if (!res.ok) { - let message = `Failed to create quote (${res.status})`; - try { - const data: unknown = await res.json(); - if ( - data && - typeof data === "object" && - "error" in data && - typeof (data as { error?: unknown }).error === "string" - ) { - message = (data as { error: string }).error; - } - } catch { - // ignore non-json - } - toast.error(message); - return; - } - const body = (await res.json()) as { id: string; quote_number: string }; - toast.success(`Created quote ${body.quote_number}`); - await loadAll(); - // Navigate to the new quote so the operator can fill in - // prices immediately. The list page does the same; the - // detail page does it too because the operator came here - // expecting to keep working, not to be sent back to the - // list. - router.push(`/quotes/${body.id}`); - } catch (err: unknown) { - toast.error(err instanceof Error ? err.message : "Failed to create quote"); - } finally { - setBusyAction(null); - } - }, [deliveryNote, loadAll, router]); + setQuoteReviewOpen(true); + }, [deliveryNote]); // ── Action: Capture Customer PO ───────────────────────────────── const openPoModal = useCallback(() => { @@ -1340,7 +1313,7 @@ export default function DeliveryNoteDetailPage({ params }: PageProps) { : undefined } onAction={{ - "create-quote": () => void handleCreateQuote(), + "create-quote": handleCreateQuote, "preview-quote": () => quote && router.push(`/quotes/${quote.id}`), send: () => quote && router.push(`/quotes/${quote.id}?sendNow=true`), "capture-po": openPoModal, @@ -2310,6 +2283,23 @@ export default function DeliveryNoteDetailPage({ params }: PageProps) { subtitle="Delivery note" downloadFilename={`${deliveryNote.delivery_note_number}.pdf`} /> + + {/* Pricing review before the quote exists. The SAME modal the + list page uses, so the two screens cannot drift: the + operator picks the letterhead, confirms cost and markup per + line, and the create happens inside here. */} + { + setQuoteReviewOpen(false); + void loadAll(); + }} + /> ); } diff --git a/src/lib/server/quote-pdf-service.ts b/src/lib/server/quote-pdf-service.ts index 06ff0e6..23b70ee 100644 --- a/src/lib/server/quote-pdf-service.ts +++ b/src/lib/server/quote-pdf-service.ts @@ -363,13 +363,24 @@ export async function generateQuotePdf( const itemsForPdf = isNoQuote ? allItems : ready; if (!isNoQuote && ready.length === 0) { + // The message must describe what the check ACTUALLY requires. + // It used to advertise a supplier requirement that was removed + // from the check — so an operator was told to go and set a + // supplier on every line, which changed nothing, and the real + // blockers (not processed / no cost / no selling price) went + // unnamed. Keep the two in step: both come from `issues[].missing`. + const needed = [ + ...new Set(issues.flatMap((i) => i.missing)), + ].sort(); return { kind: "readiness_failure", code: allItems.length === 0 ? "no_items" : "items_not_ready", message: allItems.length === 0 ? "This quote has no items yet. Add line items on the quote detail page first." - : "No items are ready to quote. Every item needs: processed ✓, supplier ✓, cost price, and selling price.", + : needed.length > 0 + ? `No items are ready to quote. Every item still needs: ${needed.join(", ")}.` + : "No items are ready to quote. Price every line first.", details: { itemIds: issues.map((i) => i.itemId), itemDescriptions: issues.map((i) => i.description),