fix(delivery-notes): the detail page's Review pricing bypassed the review
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.
This commit is contained in:
parent
55be94b08f
commit
7ce23e04f3
2 changed files with 42 additions and 41 deletions
|
|
@ -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<HTMLInputElement | null>(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. */}
|
||||
<QuoteReviewDialog
|
||||
key={`dn-quote-review-${deliveryNote.id}`}
|
||||
open={quoteReviewOpen}
|
||||
onOpenChange={setQuoteReviewOpen}
|
||||
deliveryNoteId={deliveryNote.id}
|
||||
deliveryNoteNumber={deliveryNote.delivery_note_number}
|
||||
customerName={deliveryNote.expand?.customer?.customer ?? ""}
|
||||
onCreated={() => {
|
||||
setQuoteReviewOpen(false);
|
||||
void loadAll();
|
||||
}}
|
||||
/>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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),
|
||||
|
|
|
|||
Loading…
Reference in a new issue