feat(delivery-notes): review pricing before a quote is created
The Create quote button fired blind: the quote appeared with whatever
pricing happened to be on the order items, and a wrong price was only
discoverable by finding the quote afterwards. A new review modal puts
the check in front of the create.
Modal (components/delivery-notes/quote-review-dialog.tsx)
- Letterhead picker. DN-backed quotes never set company_letterhead, so
the renderer defaulted every one of them to CNL.
- Cost and markup per line; selling price is DERIVED, never typed, so
the two cannot disagree. See src/lib/quote-pricing.ts.
- A no-cost line shows "No cost on file" and stays at R0.00 rather
than deriving a fake price off a zero cost, which reads as a
deliberate free-of-charge item.
- Bulk "set all markups" field — typing 25% into eight lines one at a
time is the tedium the modal exists to remove.
- Totals, with margin as a share of REVENUE not cost: 25% markup is a
20% margin, and conflating them quietly under-prices a job.
- Nothing persists until Generate. Cancel discards the whole grid.
Backend
- create-quote now accepts { letterhead, items: [{ orderItemId,
costPrice, markupPercent }] } and derives selling server-side.
- Overrides are keyed by order_item id and validated against THIS
note's lines. An unknown id aborts the create rather than moving
another order's goods into this customer's quote.
- Quote items now go out as one PocketBase batch instead of a loop of
individual creates. The batch is transactional: the old loop could
fail on line 5 of 8 and leave a real quote header attached to half
its lines. The header still has to be created first (the batch API
can't reference a record created in the same batch), so a failed
batch deletes the orphaned header rather than leaving it visible.
- Malformed bodies are 400s the operator can read, not 500s. "A quote
already exists" and "not in draft" are now 409s.
Tests
- 33 new: pricing maths, and the route's validation guards. Verified
the guards bite by removing them (2 fail) and restored.
- The existing service test's PB mock gained createBatch, modelled as
genuinely all-or-nothing — the compensation path depends on that
guarantee being real.
Full suite: 3615 passing. tsc clean.
This commit is contained in:
parent
980f40aa79
commit
97c3cd4d7e
8 changed files with 1317 additions and 81 deletions
|
|
@ -19,6 +19,7 @@ import {
|
|||
} from "@/components/ui/dialog";
|
||||
import { CreateAdHocDeliveryNoteDialog } from "@/components/delivery-notes/create-adhoc-dialog";
|
||||
import { DeliveryNoteItemsExpanded } from "@/components/delivery-notes/items-expanded";
|
||||
import { QuoteReviewDialog } from "@/components/delivery-notes/quote-review-dialog";
|
||||
import {
|
||||
PdfPreviewDialog,
|
||||
QuotePdfPreviewDialog,
|
||||
|
|
@ -215,6 +216,16 @@ export default function DeliveryNotesPage() {
|
|||
// navigates to the new delivery note's detail view.
|
||||
const [createOpen, setCreateOpen] = useState(false);
|
||||
|
||||
// "Create quote" review modal. Holds enough of the note to label
|
||||
// the modal and to refresh the right row afterwards; the pricing
|
||||
// grid itself is owned by the dialog.
|
||||
const [quoteReview, setQuoteReview] = useState<{
|
||||
open: boolean;
|
||||
noteId: string;
|
||||
number: string;
|
||||
customer: string;
|
||||
}>({ open: false, noteId: "", number: "", customer: "" });
|
||||
|
||||
// PDF preview dialog state. The D icon on every row opens
|
||||
// this instead of opening the PDF in a new tab — operator
|
||||
// stays in the list, can compare two DNs side by side, doesn't
|
||||
|
|
@ -663,74 +674,42 @@ export default function DeliveryNotesPage() {
|
|||
}, []);
|
||||
|
||||
// ── Action: Create Quote from Delivery Note ─────────────────────
|
||||
// Calls the API route, which creates a Quote with status
|
||||
// "delivery_note" + a quote_item per order_item, then flips the
|
||||
// delivery note's status to "sent". On success we navigate the
|
||||
// operator to the new quote so they can fill in prices. The
|
||||
// server returns the new quote's id + quote_number.
|
||||
const handleCreateQuote = useCallback(
|
||||
async (note: DeliveryNote) => {
|
||||
// The Q icon no longer fires the create blind. It opens the review
|
||||
// modal, where the operator confirms letterhead, cost and markup
|
||||
// per line, and the quote is created from what they see. The modal
|
||||
// POSTs itself; this handler only owns the state around it.
|
||||
const openQuoteReview = useCallback((note: DeliveryNote) => {
|
||||
setQuoteReview({
|
||||
open: true,
|
||||
noteId: note.id,
|
||||
number: note.delivery_note_number,
|
||||
customer: note.expand?.customer?.customer ?? "",
|
||||
});
|
||||
}, []);
|
||||
|
||||
// Fired after the modal has created the quote. Refreshes the list
|
||||
// so the row's Q icon flips to "open" mode and the status badge
|
||||
// changes, then auto-opens the new quote's PDF so the operator can
|
||||
// print or download it straight away.
|
||||
const handleQuoteCreated = useCallback(
|
||||
async (note: DeliveryNote, result: { id: string; quote_number: string }) => {
|
||||
markBusy(note.id);
|
||||
try {
|
||||
const res = await fetch(`/api/v1/delivery-notes/${note.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;
|
||||
};
|
||||
// Stay on the list page — the user explicitly asked
|
||||
// for "create the quote here or there, without
|
||||
// opening the quotes card". We refresh so the row's
|
||||
// Q icon switches from "create" to "open" mode and
|
||||
// the status badge flips to "Sent", then auto-open
|
||||
// the new quote's PDF in the preview dialog so the
|
||||
// operator can immediately print/download the
|
||||
// freshly-generated PDF (or move on — the dialog
|
||||
// has a Close button).
|
||||
const quoteId = body.id;
|
||||
const quoteNumber = body.quote_number;
|
||||
await fetchNotes();
|
||||
// Read the new quote back from the refreshed
|
||||
// `quoteByDnId` map so the preview dialog has the
|
||||
// full record (letterhead, customer, etc.) and
|
||||
// doesn't need a second round-trip. The map is
|
||||
// updated by `fetchNotes` before this line runs
|
||||
// because `fetchNotes` awaits the refresh.
|
||||
const newQuote = quoteByDnId.get(note.id) ?? {
|
||||
id: quoteId,
|
||||
quote_number: quoteNumber,
|
||||
id: result.id,
|
||||
quote_number: result.quote_number,
|
||||
company_letterhead: "",
|
||||
} as Quote;
|
||||
setPreviewQuote(newQuote);
|
||||
toast.success(
|
||||
`Created quote ${quoteNumber} for ${note.delivery_note_number}`,
|
||||
`Created quote ${result.quote_number} for ${note.delivery_note_number}`,
|
||||
);
|
||||
} catch (err: unknown) {
|
||||
toast.error(err instanceof Error ? err.message : "Failed to create quote");
|
||||
} finally {
|
||||
markIdle(note.id);
|
||||
}
|
||||
},
|
||||
[fetchNotes, markBusy, markIdle, quoteByDnId, router],
|
||||
[fetchNotes, markBusy, markIdle, quoteByDnId],
|
||||
);
|
||||
|
||||
// ── Action: Capture Customer PO ─────────────────────────────────
|
||||
|
|
@ -935,10 +914,10 @@ export default function DeliveryNotesPage() {
|
|||
<button
|
||||
type="button"
|
||||
disabled={busy}
|
||||
onClick={() => void handleCreateQuote(note)}
|
||||
onClick={() => openQuoteReview(note)}
|
||||
data-testid={`dn-action-create-quote-${note.id}`}
|
||||
aria-label="Create quote from delivery note"
|
||||
title="Create quote"
|
||||
aria-label="Review pricing and create quote from delivery note"
|
||||
title="Review pricing and create quote"
|
||||
className="inline-flex items-center justify-center rounded-md border border-info/40 bg-info/15 hover:bg-info/25 transition-colors disabled:opacity-50"
|
||||
style={{ width: "1.75rem", height: "1.75rem" }}
|
||||
>
|
||||
|
|
@ -1062,7 +1041,7 @@ export default function DeliveryNotesPage() {
|
|||
},
|
||||
[
|
||||
busyIds,
|
||||
handleCreateQuote,
|
||||
openQuoteReview,
|
||||
handleMarkInvoiced,
|
||||
openPoModal,
|
||||
quoteByDnId,
|
||||
|
|
@ -1790,6 +1769,27 @@ export default function DeliveryNotesPage() {
|
|||
onOpenChange={setCreateOpen}
|
||||
/>
|
||||
|
||||
{/* Pricing review before a quote exists. Opened by the Q icon
|
||||
on any row that doesn't have a quote yet. The dialog POSTs
|
||||
the create itself with the operator's confirmed pricing;
|
||||
`handleQuoteCreated` only refreshes the row behind it. */}
|
||||
{quoteReview.noteId && (
|
||||
<QuoteReviewDialog
|
||||
key={quoteReview.noteId}
|
||||
open={quoteReview.open}
|
||||
onOpenChange={(next) =>
|
||||
setQuoteReview((prev) => ({ ...prev, open: next }))
|
||||
}
|
||||
deliveryNoteId={quoteReview.noteId}
|
||||
deliveryNoteNumber={quoteReview.number}
|
||||
customerName={quoteReview.customer}
|
||||
onCreated={(result) => {
|
||||
const note = notes.find((n) => n.id === quoteReview.noteId);
|
||||
if (note) void handleQuoteCreated(note, result);
|
||||
}}
|
||||
/>
|
||||
)}
|
||||
|
||||
{/* D-icon PDF preview. The D icon on every row opens
|
||||
this. Operator stays in the list — the PDF renders
|
||||
inside an iframe, no new tab, no navigation. The
|
||||
|
|
|
|||
135
src/app/api/v1/delivery-notes/[id]/create-quote/route.test.ts
Normal file
135
src/app/api/v1/delivery-notes/[id]/create-quote/route.test.ts
Normal file
|
|
@ -0,0 +1,135 @@
|
|||
import { describe, it, expect, beforeEach, vi } from "vitest";
|
||||
import { POST } from "@/app/api/v1/delivery-notes/[id]/create-quote/route";
|
||||
import { createQuoteFromDeliveryNote } from "@/lib/server/delivery-notes-service";
|
||||
|
||||
/**
|
||||
* The route's job is to refuse bad pricing BEFORE the service runs,
|
||||
* because by the time the service is reached a quote header may
|
||||
* already exist and be attached to this customer.
|
||||
*
|
||||
* Two failure modes are worth more than a status code:
|
||||
* - a malformed body must be a 400 the operator can read, not a 500
|
||||
* that reads as "the server broke";
|
||||
* - a negative cost or markup is not a typo, it's a sign the modal
|
||||
* sent something we never intended to accept, and must not reach
|
||||
* the money path.
|
||||
*/
|
||||
|
||||
vi.mock("@/lib/server/delivery-notes-service", () => ({
|
||||
createQuoteFromDeliveryNote: vi.fn(async () => ({
|
||||
id: "quote1",
|
||||
quote_number: "Q001",
|
||||
})),
|
||||
}));
|
||||
|
||||
const makeReq = (body: unknown) =>
|
||||
({
|
||||
text: async () => (typeof body === "string" ? body : JSON.stringify(body)),
|
||||
}) as unknown as Parameters<typeof POST>[0];
|
||||
|
||||
const call = (body: unknown) =>
|
||||
POST(makeReq(body), { params: Promise.resolve({ id: "dn1" }) });
|
||||
|
||||
describe("POST /create-quote body validation", () => {
|
||||
beforeEach(() => {
|
||||
vi.mocked(createQuoteFromDeliveryNote).mockClear();
|
||||
});
|
||||
|
||||
it("accepts a valid letterhead and passes it to the service", async () => {
|
||||
const res = await call({ letterhead: "ISHERWOOD" });
|
||||
expect(res.status).toBe(200);
|
||||
expect(vi.mocked(createQuoteFromDeliveryNote)).toHaveBeenCalledWith("dn1", {
|
||||
letterhead: "ISHERWOOD",
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects an unknown letterhead with 400 and never calls the service", async () => {
|
||||
const res = await call({ letterhead: "ACME" });
|
||||
expect(res.status).toBe(400);
|
||||
// The service is what creates the quote. Reaching it here would
|
||||
// mean an unvalidated letterhead could reach the renderer.
|
||||
expect(vi.mocked(createQuoteFromDeliveryNote)).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects a negative cost with 400", async () => {
|
||||
const res = await call({
|
||||
items: [{ orderItemId: "oi1", costPrice: -5, markupPercent: 20 }],
|
||||
});
|
||||
expect(res.status).toBe(400);
|
||||
expect(vi.mocked(createQuoteFromDeliveryNote)).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects a negative markup with 400", async () => {
|
||||
const res = await call({
|
||||
items: [{ orderItemId: "oi1", costPrice: 10, markupPercent: -20 }],
|
||||
});
|
||||
expect(res.status).toBe(400);
|
||||
expect(vi.mocked(createQuoteFromDeliveryNote)).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects a missing orderItemId — that is what keys the line", async () => {
|
||||
const res = await call({ items: [{ costPrice: 10, markupPercent: 20 }] });
|
||||
expect(res.status).toBe(400);
|
||||
expect(vi.mocked(createQuoteFromDeliveryNote)).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects a non-array items field", async () => {
|
||||
const res = await call({ items: { orderItemId: "oi1" } });
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
|
||||
it("rejects malformed JSON with 400, not 500", async () => {
|
||||
const res = await call("{not json");
|
||||
expect(res.status).toBe(400);
|
||||
expect(vi.mocked(createQuoteFromDeliveryNote)).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("treats an empty body as the pre-modal behaviour", async () => {
|
||||
// Back-compat: the detail page and any older caller POST with no
|
||||
// body at all and must keep working.
|
||||
const res = await call("");
|
||||
expect(res.status).toBe(200);
|
||||
expect(vi.mocked(createQuoteFromDeliveryNote)).toHaveBeenCalledWith("dn1", {});
|
||||
});
|
||||
|
||||
it("normalises a full item list and forwards it", async () => {
|
||||
await call({
|
||||
letterhead: "CNL",
|
||||
items: [
|
||||
{ orderItemId: "oi1", costPrice: 100, markupPercent: 25 },
|
||||
{ orderItemId: "oi2", costPrice: 0, markupPercent: 0 },
|
||||
],
|
||||
});
|
||||
expect(vi.mocked(createQuoteFromDeliveryNote)).toHaveBeenCalledWith("dn1", {
|
||||
letterhead: "CNL",
|
||||
items: [
|
||||
{ orderItemId: "oi1", costPrice: 100, markupPercent: 25 },
|
||||
{ orderItemId: "oi2", costPrice: 0, markupPercent: 0 },
|
||||
],
|
||||
});
|
||||
});
|
||||
|
||||
it("maps 'a quote already exists' to 409, not a generic 500", async () => {
|
||||
vi.mocked(createQuoteFromDeliveryNote).mockRejectedValueOnce(
|
||||
new Error("A delivery-note quote already exists for this delivery note (Q001)."),
|
||||
);
|
||||
const res = await call({});
|
||||
expect(res.status).toBe(409);
|
||||
});
|
||||
|
||||
it("maps a non-draft note to 409", async () => {
|
||||
vi.mocked(createQuoteFromDeliveryNote).mockRejectedValueOnce(
|
||||
new Error('Delivery note must be in "draft" status (current: "sent").'),
|
||||
);
|
||||
const res = await call({});
|
||||
expect(res.status).toBe(409);
|
||||
});
|
||||
|
||||
it("still returns 500 for a genuinely unknown failure", async () => {
|
||||
vi.mocked(createQuoteFromDeliveryNote).mockRejectedValueOnce(
|
||||
new Error("PocketBase exploded"),
|
||||
);
|
||||
const res = await call({});
|
||||
expect(res.status).toBe(500);
|
||||
});
|
||||
});
|
||||
|
|
@ -1,5 +1,9 @@
|
|||
import { NextRequest, NextResponse } from "next/server";
|
||||
import { createQuoteFromDeliveryNote } from "@/lib/server/delivery-notes-service";
|
||||
import {
|
||||
createQuoteFromDeliveryNote,
|
||||
type CreateQuoteFromDeliveryNoteOptions,
|
||||
type QuotePriceOverride,
|
||||
} from "@/lib/server/delivery-notes-service";
|
||||
|
||||
export const runtime = "nodejs";
|
||||
export const dynamic = "force-dynamic";
|
||||
|
|
@ -18,31 +22,114 @@ export const dynamic = "force-dynamic";
|
|||
* - order_items are UNCHANGED — they stay at item_status="delivery_note"
|
||||
* until the operator clicks "Mark Invoiced" later in the flow.
|
||||
*
|
||||
* Body: none required.
|
||||
* Body (optional — an empty body keeps the pre-modal behaviour):
|
||||
* {
|
||||
* letterhead?: "CNL" | "ISHERWOOD",
|
||||
* items?: [{ orderItemId, costPrice, markupPercent }]
|
||||
* }
|
||||
*
|
||||
* `items` is the pricing the operator confirmed in the review modal.
|
||||
* The service derives `selling_price` from cost + markup, so a client
|
||||
* cannot submit two numbers that disagree with each other. Each
|
||||
* `orderItemId` is validated against this delivery note's own lines;
|
||||
* an unknown id aborts the whole create rather than pulling another
|
||||
* order's goods into this customer's quote.
|
||||
*
|
||||
* Returns: `{ id, quote_number }` of the newly-created quote.
|
||||
*
|
||||
* Errors:
|
||||
* - 400 if the body is malformed
|
||||
* - 404 if the delivery note id doesn't exist (PB throws on getOne)
|
||||
* - 409 if a delivery-note quote already exists for this note
|
||||
* - 400 if the delivery note is not in "draft" status
|
||||
* - 409 if a delivery-note quote already exists for this note, or
|
||||
* the note isn't in "draft" any more
|
||||
* - 500 on any other failure
|
||||
*/
|
||||
export async function POST(
|
||||
_req: NextRequest,
|
||||
req: NextRequest,
|
||||
{ params }: { params: Promise<{ id: string }> }
|
||||
) {
|
||||
const { id } = await params;
|
||||
|
||||
// Parse the body defensively. A malformed override must fail as a
|
||||
// 400 the operator can act on, not a 500 that reads as "the server
|
||||
// broke".
|
||||
const options: CreateQuoteFromDeliveryNoteOptions = {};
|
||||
try {
|
||||
const { id } = await params;
|
||||
const result = await createQuoteFromDeliveryNote(id);
|
||||
const raw = (await req.text())?.trim();
|
||||
if (raw) {
|
||||
const body = JSON.parse(raw) as {
|
||||
letterhead?: unknown;
|
||||
items?: unknown;
|
||||
};
|
||||
if (
|
||||
body.letterhead !== undefined &&
|
||||
body.letterhead !== "CNL" &&
|
||||
body.letterhead !== "ISHERWOOD"
|
||||
) {
|
||||
return NextResponse.json(
|
||||
{ error: 'letterhead must be "CNL" or "ISHERWOOD"' },
|
||||
{ status: 400 },
|
||||
);
|
||||
}
|
||||
if (body.letterhead) {
|
||||
options.letterhead = body.letterhead as "CNL" | "ISHERWOOD";
|
||||
}
|
||||
if (body.items !== undefined) {
|
||||
if (!Array.isArray(body.items)) {
|
||||
return NextResponse.json(
|
||||
{ error: "items must be an array" },
|
||||
{ status: 400 },
|
||||
);
|
||||
}
|
||||
const parsed: QuotePriceOverride[] = [];
|
||||
for (const [i, entry] of body.items.entries()) {
|
||||
const e = (entry ?? {}) as Record<string, unknown>;
|
||||
const orderItemId =
|
||||
typeof e.orderItemId === "string" ? e.orderItemId : "";
|
||||
const costPrice = Number(e.costPrice);
|
||||
const markupPercent = Number(e.markupPercent);
|
||||
if (!orderItemId) {
|
||||
return NextResponse.json(
|
||||
{ error: `items[${i}].orderItemId is required` },
|
||||
{ status: 400 },
|
||||
);
|
||||
}
|
||||
if (!Number.isFinite(costPrice) || costPrice < 0) {
|
||||
return NextResponse.json(
|
||||
{ error: `items[${i}].costPrice must be a number >= 0` },
|
||||
{ status: 400 },
|
||||
);
|
||||
}
|
||||
if (!Number.isFinite(markupPercent) || markupPercent < 0) {
|
||||
return NextResponse.json(
|
||||
{ error: `items[${i}].markupPercent must be a number >= 0` },
|
||||
{ status: 400 },
|
||||
);
|
||||
}
|
||||
parsed.push({ orderItemId, costPrice, markupPercent });
|
||||
}
|
||||
options.items = parsed;
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
return NextResponse.json(
|
||||
{ error: "Request body is not valid JSON" },
|
||||
{ status: 400 },
|
||||
);
|
||||
}
|
||||
|
||||
try {
|
||||
const result = await createQuoteFromDeliveryNote(id, options);
|
||||
return NextResponse.json(result);
|
||||
} catch (err: unknown) {
|
||||
const message = err instanceof Error ? err.message : "Failed to create quote from delivery note";
|
||||
// PB throws ClientResponseError; we don't get a clean status
|
||||
// here so the route returns 500 for any failure. The Invoicing
|
||||
// toast surfaces the message verbatim, which is enough for
|
||||
// the operator to understand "this delivery note is not in
|
||||
// draft" or "a quote already exists".
|
||||
return NextResponse.json({ error: message }, { status: 500 });
|
||||
// Two rejections the operator can actually fix get a 409 rather
|
||||
// than a generic 500: a quote already exists for this note, or
|
||||
// the note isn't in "draft" any more. Everything else is a 500.
|
||||
const status =
|
||||
/already exists/i.test(message) || /must be in "draft"/i.test(message)
|
||||
? 409
|
||||
: 500;
|
||||
return NextResponse.json({ error: message }, { status });
|
||||
}
|
||||
}
|
||||
|
|
|
|||
447
src/components/delivery-notes/quote-review-dialog.tsx
Normal file
447
src/components/delivery-notes/quote-review-dialog.tsx
Normal file
|
|
@ -0,0 +1,447 @@
|
|||
"use client";
|
||||
|
||||
/**
|
||||
* "Create quote from delivery note" review modal.
|
||||
*
|
||||
* The operator's first question on a delivery note is "what am I
|
||||
* charging for this, and am I happy with it?" Previously the Create
|
||||
* quote button fired the request blind — the quote appeared with
|
||||
* whatever pricing happened to be on the order items, and if that
|
||||
* was wrong the only fix was to find the quote afterwards.
|
||||
*
|
||||
* This modal puts the review *in front of* the create, so the quote
|
||||
* is right the first time it exists.
|
||||
*
|
||||
* What it deliberately does NOT do:
|
||||
* - It doesn't persist anything until Generate is pressed. Cancel
|
||||
* discards the whole grid. There is no "half-saved" draft, which
|
||||
* is the behaviour the operator described wanting.
|
||||
* - It doesn't let you type a selling price. Selling is derived
|
||||
* from cost + markup so the two can't disagree. See
|
||||
* `src/lib/quote-pricing.ts` for why that's worth enforcing.
|
||||
* - It doesn't invent a cost. A line with no cost on file shows a
|
||||
* blank cost and a "no cost on file" marker rather than a
|
||||
* plausible-looking zero that reads as free-of-charge.
|
||||
*/
|
||||
import { useEffect, useMemo, useState } from "react";
|
||||
import { useRouter } from "next/navigation";
|
||||
import { toast } from "sonner";
|
||||
import { AlertTriangle, Loader2 } from "lucide-react";
|
||||
import { Button } from "@/components/ui/button";
|
||||
import { Input } from "@/components/ui/input";
|
||||
import { Label } from "@/components/ui/label";
|
||||
import {
|
||||
Dialog,
|
||||
DialogContent,
|
||||
DialogDescription,
|
||||
DialogFooter,
|
||||
DialogHeader,
|
||||
DialogTitle,
|
||||
} from "@/components/ui/dialog";
|
||||
import {
|
||||
buildLinePricing,
|
||||
hasCostPrice,
|
||||
repriceLine,
|
||||
totalsForLines,
|
||||
type LinePricing,
|
||||
} from "@/lib/quote-pricing";
|
||||
import { formatCurrency } from "@/lib/pb-helpers";
|
||||
import getPocketBase from "@/lib/pocketbase";
|
||||
import type { OrderItem } from "@/lib/types";
|
||||
|
||||
type Letterhead = "CNL" | "ISHERWOOD";
|
||||
|
||||
const LETTERHEADS: Array<{ value: Letterhead; label: string }> = [
|
||||
{ value: "CNL", label: "CNL" },
|
||||
{ value: "ISHERWOOD", label: "Isherwood" },
|
||||
];
|
||||
|
||||
export interface QuoteReviewDialogProps {
|
||||
open: boolean;
|
||||
onOpenChange: (open: boolean) => void;
|
||||
/** The delivery note being quoted. */
|
||||
deliveryNoteId: string;
|
||||
/** Shown in the header so the operator can confirm the right note. */
|
||||
deliveryNoteNumber: string;
|
||||
/** Customer name, shown under the title. */
|
||||
customerName?: string;
|
||||
/** Fired after a successful create, with the new quote. */
|
||||
onCreated?: (result: { id: string; quote_number: string }) => void;
|
||||
}
|
||||
|
||||
export function QuoteReviewDialog({
|
||||
open,
|
||||
onOpenChange,
|
||||
deliveryNoteId,
|
||||
deliveryNoteNumber,
|
||||
customerName,
|
||||
onCreated,
|
||||
}: QuoteReviewDialogProps) {
|
||||
const router = useRouter();
|
||||
|
||||
const [loading, setLoading] = useState(false);
|
||||
const [loadError, setLoadError] = useState<string | null>(null);
|
||||
const [submitting, setSubmitting] = useState(false);
|
||||
const [letterhead, setLetterhead] = useState<Letterhead>("CNL");
|
||||
const [lines, setLines] = useState<LinePricing[]>([]);
|
||||
|
||||
// Load the note's lines on open. Cancelled on close so a slow
|
||||
// response can't repopulate the grid after the operator moved on.
|
||||
useEffect(() => {
|
||||
if (!open) return;
|
||||
let cancelled = false;
|
||||
const controller = new AbortController();
|
||||
|
||||
const load = async () => {
|
||||
setLoading(true);
|
||||
setLoadError(null);
|
||||
try {
|
||||
const token = getPocketBase().authStore.token;
|
||||
if (!token) throw new Error("Your session has expired — please sign in again.");
|
||||
const res = await fetch(
|
||||
`/api/v1/delivery-notes/${encodeURIComponent(deliveryNoteId)}/detail`,
|
||||
{ headers: { Authorization: token }, signal: controller.signal },
|
||||
);
|
||||
if (!res.ok) {
|
||||
throw new Error(
|
||||
res.status === 401
|
||||
? "Your session has expired — please sign in again."
|
||||
: `Could not load the delivery note (${res.status}).`,
|
||||
);
|
||||
}
|
||||
const detail = (await res.json()) as { items?: OrderItem[] };
|
||||
if (cancelled) return;
|
||||
setLines(buildLinePricing(detail.items ?? []));
|
||||
} catch (err: unknown) {
|
||||
if (err instanceof DOMException && err.name === "AbortError") return;
|
||||
if (!cancelled) {
|
||||
setLoadError(
|
||||
err instanceof Error ? err.message : "Failed to load the delivery note",
|
||||
);
|
||||
}
|
||||
} finally {
|
||||
if (!cancelled) setLoading(false);
|
||||
}
|
||||
};
|
||||
|
||||
void load();
|
||||
return () => {
|
||||
cancelled = true;
|
||||
controller.abort();
|
||||
};
|
||||
}, [open, deliveryNoteId]);
|
||||
|
||||
const totals = useMemo(() => totalsForLines(lines), [lines]);
|
||||
// A line with no cost can't be priced, and we won't guess one. The
|
||||
// per-row "No cost on file" marker says it inline; this count backs
|
||||
// the footer warning so the operator hears it before committing,
|
||||
// not after the quote has gone to the customer.
|
||||
const unpriced = useMemo(
|
||||
() => lines.filter((l) => !hasCostPrice(l.costPrice) && l.sellingPrice === 0),
|
||||
[lines],
|
||||
);
|
||||
|
||||
const updateLine = (
|
||||
index: number,
|
||||
patch: { costPrice?: number; markupPercent?: number },
|
||||
) => {
|
||||
setLines((prev) =>
|
||||
prev.map((line, i) => (i === index ? repriceLine(line, patch) : line)),
|
||||
);
|
||||
};
|
||||
|
||||
const applyMarkupToAll = (markupPercent: number) => {
|
||||
setLines((prev) =>
|
||||
prev.map((line) =>
|
||||
hasCostPrice(line.costPrice)
|
||||
? repriceLine(line, { markupPercent })
|
||||
: line,
|
||||
),
|
||||
);
|
||||
};
|
||||
|
||||
const handleSubmit = async () => {
|
||||
if (unpriced.length > 0) {
|
||||
const proceed = window.confirm(
|
||||
`${unpriced.length} line${unpriced.length === 1 ? " has" : "s have"} no cost on file, so the selling price is R0.00.\n\n` +
|
||||
"The quote will be created with those lines unpriced. Continue?",
|
||||
);
|
||||
if (!proceed) return;
|
||||
}
|
||||
setSubmitting(true);
|
||||
try {
|
||||
const token = getPocketBase().authStore.token;
|
||||
if (!token) throw new Error("Your session has expired — please sign in again.");
|
||||
const res = await fetch(
|
||||
`/api/v1/delivery-notes/${encodeURIComponent(deliveryNoteId)}/create-quote`,
|
||||
{
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json", Authorization: token },
|
||||
body: JSON.stringify({
|
||||
letterhead,
|
||||
items: lines.map((l) => ({
|
||||
orderItemId: l.orderItemId,
|
||||
costPrice: l.costPrice,
|
||||
markupPercent: l.markupPercent,
|
||||
})),
|
||||
}),
|
||||
},
|
||||
);
|
||||
const body = (await res.json().catch(() => ({}))) as {
|
||||
id?: string;
|
||||
quote_number?: string;
|
||||
error?: string;
|
||||
};
|
||||
if (!res.ok || !body.id) {
|
||||
throw new Error(body.error || `Failed to create the quote (${res.status}).`);
|
||||
}
|
||||
toast.success(`Created quote ${body.quote_number}`);
|
||||
onOpenChange(false);
|
||||
onCreated?.({ id: body.id, quote_number: body.quote_number ?? "" });
|
||||
router.push(`/quotes/${body.id}`);
|
||||
} catch (err: unknown) {
|
||||
toast.error(err instanceof Error ? err.message : "Failed to create the quote");
|
||||
} finally {
|
||||
setSubmitting(false);
|
||||
}
|
||||
};
|
||||
|
||||
return (
|
||||
<Dialog open={open} onOpenChange={onOpenChange}>
|
||||
<DialogContent className="sm:max-w-4xl max-w-4xl max-h-[90vh] overflow-y-auto">
|
||||
<DialogHeader>
|
||||
<DialogTitle className="font-mono">
|
||||
Create quote from {deliveryNoteNumber}
|
||||
</DialogTitle>
|
||||
<DialogDescription>
|
||||
{customerName ? `${customerName} · ` : ""}
|
||||
Check the cost, margin and selling price before the quote is created.
|
||||
Nothing is saved until you press Generate.
|
||||
</DialogDescription>
|
||||
</DialogHeader>
|
||||
|
||||
{loading ? (
|
||||
<div className="flex items-center justify-center gap-2 py-12 text-sm text-muted-foreground">
|
||||
<Loader2 className="h-4 w-4 animate-spin" />
|
||||
Loading items…
|
||||
</div>
|
||||
) : loadError ? (
|
||||
<div className="rounded-md border border-destructive/40 bg-destructive/5 p-4 text-sm text-destructive">
|
||||
{loadError}
|
||||
</div>
|
||||
) : lines.length === 0 ? (
|
||||
<p className="py-12 text-center text-sm text-muted-foreground">
|
||||
This delivery note has no items, so there's nothing to quote.
|
||||
</p>
|
||||
) : (
|
||||
<div className="space-y-4">
|
||||
{/* Letterhead. DN-backed quotes never used to set this, so
|
||||
the renderer defaulted to CNL for every one of them. */}
|
||||
<div className="space-y-2">
|
||||
<Label>Letterhead</Label>
|
||||
<div className="flex flex-wrap gap-2">
|
||||
{LETTERHEADS.map((l) => (
|
||||
<button
|
||||
key={l.value}
|
||||
type="button"
|
||||
onClick={() => setLetterhead(l.value)}
|
||||
data-testid={`dn-quote-letterhead-${l.value}`}
|
||||
className={
|
||||
letterhead === l.value
|
||||
? "rounded-md border border-primary bg-primary/10 px-4 py-1.5 text-sm font-medium text-primary"
|
||||
: "rounded-md border px-4 py-1.5 text-sm text-muted-foreground hover:bg-accent"
|
||||
}
|
||||
>
|
||||
{l.label}
|
||||
</button>
|
||||
))}
|
||||
</div>
|
||||
</div>
|
||||
|
||||
{/* Bulk markup. Typing a percentage on eight lines one at
|
||||
a time is the tedium this whole modal exists to remove. */}
|
||||
<div className="flex items-end gap-2">
|
||||
<div className="w-40 space-y-2">
|
||||
<Label htmlFor="dn-quote-bulk-markup" className="text-xs">
|
||||
Set all markups
|
||||
</Label>
|
||||
<Input
|
||||
id="dn-quote-bulk-markup"
|
||||
type="number"
|
||||
min="0"
|
||||
step="0.5"
|
||||
placeholder="25"
|
||||
onKeyDown={(e) => {
|
||||
if (e.key !== "Enter") return;
|
||||
e.preventDefault();
|
||||
const v = Number((e.target as HTMLInputElement).value);
|
||||
if (Number.isFinite(v)) applyMarkupToAll(v);
|
||||
}}
|
||||
onBlur={(e) => {
|
||||
const v = Number(e.target.value);
|
||||
if (Number.isFinite(v) && e.target.value !== "") {
|
||||
applyMarkupToAll(v);
|
||||
}
|
||||
}}
|
||||
/>
|
||||
</div>
|
||||
<p className="pb-2 text-xs text-muted-foreground">
|
||||
Press Enter or click away to apply to every costed line.
|
||||
</p>
|
||||
</div>
|
||||
|
||||
{/* Price grid. Cost and markup are the inputs; selling is
|
||||
derived and shown, not typed. */}
|
||||
<div className="rounded-md border overflow-x-auto">
|
||||
<table className="w-full text-sm">
|
||||
<thead>
|
||||
<tr className="border-b bg-muted/40 text-left">
|
||||
<th className="px-3 py-2 font-medium">Item</th>
|
||||
<th className="px-3 py-2 text-right font-medium w-20">Qty</th>
|
||||
<th className="px-3 py-2 text-right font-medium w-32">Cost</th>
|
||||
<th className="px-3 py-2 text-right font-medium w-28">Markup %</th>
|
||||
<th className="px-3 py-2 text-right font-medium w-32">Selling</th>
|
||||
<th className="px-3 py-2 text-right font-medium w-32">Line total</th>
|
||||
</tr>
|
||||
</thead>
|
||||
<tbody>
|
||||
{lines.map((line, index) => {
|
||||
const costed = hasCostPrice(line.costPrice);
|
||||
return (
|
||||
<tr
|
||||
key={line.orderItemId}
|
||||
className="border-b last:border-b-0"
|
||||
data-testid={`dn-quote-line-${index}`}
|
||||
>
|
||||
<td className="px-3 py-2">
|
||||
<div className="font-medium break-words">
|
||||
{line.description || "(no description)"}
|
||||
</div>
|
||||
{!costed && (
|
||||
<div className="mt-1 inline-flex items-center gap-1 text-[11px] text-warn">
|
||||
<AlertTriangle className="h-3 w-3" />
|
||||
No cost on file
|
||||
</div>
|
||||
)}
|
||||
</td>
|
||||
<td className="px-3 py-2 text-right tabular-nums text-muted-foreground">
|
||||
{line.quantity}
|
||||
</td>
|
||||
<td className="px-3 py-2">
|
||||
<Input
|
||||
type="number"
|
||||
min="0"
|
||||
step="0.01"
|
||||
value={line.costPrice === 0 ? "" : line.costPrice}
|
||||
placeholder="0.00"
|
||||
aria-label={`Cost for line ${index + 1}`}
|
||||
onChange={(e) =>
|
||||
updateLine(index, {
|
||||
costPrice: Number(e.target.value) || 0,
|
||||
})
|
||||
}
|
||||
className="h-8 text-right tabular-nums"
|
||||
/>
|
||||
</td>
|
||||
<td className="px-3 py-2">
|
||||
<Input
|
||||
type="number"
|
||||
min="0"
|
||||
step="0.5"
|
||||
value={line.markupPercent === 0 ? "" : line.markupPercent}
|
||||
placeholder="0"
|
||||
aria-label={`Markup for line ${index + 1}`}
|
||||
onChange={(e) =>
|
||||
updateLine(index, {
|
||||
markupPercent: Number(e.target.value) || 0,
|
||||
})
|
||||
}
|
||||
className="h-8 text-right tabular-nums"
|
||||
/>
|
||||
</td>
|
||||
<td className="px-3 py-2 text-right tabular-nums font-medium">
|
||||
{costed ? formatCurrency(line.sellingPrice) : "—"}
|
||||
</td>
|
||||
<td className="px-3 py-2 text-right tabular-nums">
|
||||
{costed
|
||||
? formatCurrency(line.sellingPrice * line.quantity)
|
||||
: "—"}
|
||||
</td>
|
||||
</tr>
|
||||
);
|
||||
})}
|
||||
</tbody>
|
||||
</table>
|
||||
</div>
|
||||
|
||||
{/* Totals. Margin is shown as a share of REVENUE, not of
|
||||
cost — 25% markup is a 20% margin, and conflating them
|
||||
is the classic way to quietly under-price a job. */}
|
||||
<div className="flex flex-wrap items-center justify-end gap-x-6 gap-y-1 rounded-md border bg-muted/30 px-4 py-3 text-sm">
|
||||
<span className="text-muted-foreground">
|
||||
Total cost{" "}
|
||||
<span className="font-semibold tabular-nums text-foreground">
|
||||
{formatCurrency(totals.costTotal)}
|
||||
</span>
|
||||
</span>
|
||||
<span className="text-muted-foreground">
|
||||
Total selling{" "}
|
||||
<span className="font-semibold tabular-nums text-foreground">
|
||||
{formatCurrency(totals.sellingTotal)}
|
||||
</span>
|
||||
</span>
|
||||
<span className="text-muted-foreground">
|
||||
Margin{" "}
|
||||
<span
|
||||
className={
|
||||
totals.marginValue > 0
|
||||
? "font-semibold tabular-nums text-success"
|
||||
: "font-semibold tabular-nums text-muted-foreground"
|
||||
}
|
||||
>
|
||||
{formatCurrency(totals.marginValue)} (
|
||||
{totals.marginPercent.toFixed(1)}%)
|
||||
</span>
|
||||
</span>
|
||||
</div>
|
||||
|
||||
{unpriced.length > 0 && (
|
||||
<p className="flex items-start gap-2 text-xs text-warn">
|
||||
<AlertTriangle className="mt-0.5 h-3.5 w-3.5 shrink-0" />
|
||||
{unpriced.length} line{unpriced.length === 1 ? " has" : "s have"} no
|
||||
cost on file. Enter a cost price before sending, or the quote will
|
||||
go out with {unpriced.length === 1 ? "that line" : "those lines"} at
|
||||
R0.00.
|
||||
</p>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
|
||||
<DialogFooter className="gap-2">
|
||||
<Button
|
||||
variant="ghost"
|
||||
onClick={() => onOpenChange(false)}
|
||||
disabled={submitting}
|
||||
data-testid="dn-quote-cancel"
|
||||
>
|
||||
Cancel
|
||||
</Button>
|
||||
<Button
|
||||
onClick={() => void handleSubmit()}
|
||||
disabled={loading || submitting || lines.length === 0}
|
||||
data-testid="dn-quote-generate"
|
||||
>
|
||||
{submitting ? (
|
||||
<>
|
||||
<Loader2 className="mr-1 h-4 w-4 animate-spin" />
|
||||
Creating…
|
||||
</>
|
||||
) : (
|
||||
"Generate quote"
|
||||
)}
|
||||
</Button>
|
||||
</DialogFooter>
|
||||
</DialogContent>
|
||||
</Dialog>
|
||||
);
|
||||
}
|
||||
211
src/lib/quote-pricing.test.ts
Normal file
211
src/lib/quote-pricing.test.ts
Normal file
|
|
@ -0,0 +1,211 @@
|
|||
import { describe, it, expect } from "vitest";
|
||||
import {
|
||||
buildLinePricing,
|
||||
DEFAULT_MARKUP_PERCENT,
|
||||
hasCostPrice,
|
||||
markupFromCostAndSelling,
|
||||
repriceLine,
|
||||
round2,
|
||||
sellingFromCostAndMarkup,
|
||||
totalsForLines,
|
||||
} from "./quote-pricing";
|
||||
|
||||
/**
|
||||
* A wrong selling price here is invisible: it renders as a plausible
|
||||
* number, the total still sums, and no screen says "error". These
|
||||
* tests pin the arithmetic that the review modal's totals depend on.
|
||||
*/
|
||||
|
||||
describe("round2", () => {
|
||||
it("kills float drift rather than surfacing it", () => {
|
||||
// 100 * 1.15 is 114.99999999999999 in IEEE-754. That has to reach
|
||||
// the quote as 115, not as a 14-decimal number.
|
||||
expect(sellingFromCostAndMarkup(100, 15)).toBe(115);
|
||||
expect(round2(0.1 + 0.2)).toBe(0.3);
|
||||
});
|
||||
|
||||
it("rounds half away from zero, not to even", () => {
|
||||
expect(round2(1.005)).toBe(1.01);
|
||||
expect(round2(2.675)).toBe(2.68);
|
||||
});
|
||||
|
||||
it("coerces non-finite input to 0 rather than NaN", () => {
|
||||
expect(round2(Number.NaN)).toBe(0);
|
||||
expect(round2(Number.POSITIVE_INFINITY)).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe("markup convention", () => {
|
||||
it("treats markup as a PERCENTAGE, not a multiplier", () => {
|
||||
// 25 means 25%, so cost 100 -> 125. Getting this backwards turns
|
||||
// a 25% margin into a 25x price and nobody would notice until
|
||||
// someone tried to pay it.
|
||||
expect(sellingFromCostAndMarkup(100, 25)).toBe(125);
|
||||
expect(sellingFromCostAndMarkup(100, 0)).toBe(100);
|
||||
expect(sellingFromCostAndMarkup(100, 100)).toBe(200);
|
||||
});
|
||||
|
||||
it("round-trips cost -> selling -> markup", () => {
|
||||
for (const [cost, markup] of [
|
||||
[100, 25],
|
||||
[57.5, 33.3],
|
||||
[1234.56, 18.75],
|
||||
[1, 99],
|
||||
] as const) {
|
||||
const selling = sellingFromCostAndMarkup(cost, markup);
|
||||
const back = markupFromCostAndSelling(cost, selling);
|
||||
expect(Math.abs(back - markup)).toBeLessThan(0.02);
|
||||
}
|
||||
});
|
||||
|
||||
it("reports zero markup for a zero cost instead of dividing by zero", () => {
|
||||
expect(markupFromCostAndSelling(0, 500)).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe("hasCostPrice", () => {
|
||||
it("requires a positive finite number", () => {
|
||||
expect(hasCostPrice(0)).toBe(false);
|
||||
expect(hasCostPrice(null)).toBe(false);
|
||||
expect(hasCostPrice(undefined)).toBe(false);
|
||||
expect(hasCostPrice(Number.NaN)).toBe(false);
|
||||
expect(hasCostPrice(0.01)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("buildLinePricing", () => {
|
||||
it("applies the default markup to a cost-only line", () => {
|
||||
const [line] = buildLinePricing([
|
||||
{ id: "i1", description: "Bolt", quantity: 2, cost_price: 100 },
|
||||
]);
|
||||
expect(line).toMatchObject({
|
||||
orderItemId: "i1",
|
||||
quantity: 2,
|
||||
costPrice: 100,
|
||||
markupPercent: DEFAULT_MARKUP_PERCENT,
|
||||
sellingPrice: 125,
|
||||
});
|
||||
});
|
||||
|
||||
it("respects an EXISTING selling price instead of re-marking it", () => {
|
||||
// A human priced this before the delivery note existed. Silently
|
||||
// replacing it with the default markup would overwrite a decision.
|
||||
const [line] = buildLinePricing([
|
||||
{ id: "i1", cost_price: 100, selling_price: 160 },
|
||||
]);
|
||||
expect(line.sellingPrice).toBe(160);
|
||||
expect(line.markupPercent).toBe(60);
|
||||
});
|
||||
|
||||
it("leaves a no-cost line at zero rather than quoting a fake R0.00", () => {
|
||||
// cost 0 x (1 + 25%) is still 0 — and a selling price of 0 looks
|
||||
// like a deliberate free-of-charge item. It has to read as
|
||||
// "no cost on file" in the UI, which means a real 0 here.
|
||||
const [line] = buildLinePricing([{ id: "i1", cost_price: 0 }], 25);
|
||||
expect(line.costPrice).toBe(0);
|
||||
expect(line.markupPercent).toBe(0);
|
||||
expect(line.sellingPrice).toBe(0);
|
||||
expect(hasCostPrice(line.costPrice)).toBe(false);
|
||||
});
|
||||
|
||||
it("defaults a missing quantity to 1", () => {
|
||||
const [line] = buildLinePricing([{ id: "i1", cost_price: 50 }]);
|
||||
expect(line.quantity).toBe(1);
|
||||
});
|
||||
|
||||
it("maps 1:1 in the order it was given", () => {
|
||||
const lines = buildLinePricing([
|
||||
{ id: "a", cost_price: 10 },
|
||||
{ id: "b", cost_price: 20 },
|
||||
{ id: "c", cost_price: 0 },
|
||||
]);
|
||||
expect(lines.map((l) => l.orderItemId)).toEqual(["a", "b", "c"]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("repriceLine", () => {
|
||||
const base = {
|
||||
orderItemId: "i1",
|
||||
description: "Bolt",
|
||||
quantity: 1,
|
||||
costPrice: 100,
|
||||
markupPercent: 25,
|
||||
sellingPrice: 125,
|
||||
};
|
||||
|
||||
it("re-derives selling when the markup changes", () => {
|
||||
expect(repriceLine(base, { markupPercent: 40 }).sellingPrice).toBe(140);
|
||||
});
|
||||
|
||||
it("re-derives selling when the cost changes", () => {
|
||||
const r = repriceLine(base, { costPrice: 200 });
|
||||
expect(r.sellingPrice).toBe(250);
|
||||
expect(r.markupPercent).toBe(25);
|
||||
});
|
||||
|
||||
it("collapses to zero when the cost is zeroed out", () => {
|
||||
// Editing cost to 0 must NOT leave the old selling price hanging
|
||||
// on the line — that is a R125 charge on a free item.
|
||||
const r = repriceLine(base, { costPrice: 0 });
|
||||
expect(r.sellingPrice).toBe(0);
|
||||
});
|
||||
|
||||
it("does not mutate the input line", () => {
|
||||
repriceLine(base, { markupPercent: 99 });
|
||||
expect(base.sellingPrice).toBe(125);
|
||||
expect(base.markupPercent).toBe(25);
|
||||
});
|
||||
|
||||
it("leaves untouched fields alone", () => {
|
||||
const r = repriceLine(base, { markupPercent: 50 });
|
||||
expect(r.orderItemId).toBe("i1");
|
||||
expect(r.description).toBe("Bolt");
|
||||
expect(r.quantity).toBe(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe("totalsForLines", () => {
|
||||
it("multiplies by quantity and sums both sides", () => {
|
||||
// 100 @ 25% x3 = 375, and 50 @ 25% x2 = 125.
|
||||
const totals = totalsForLines([
|
||||
{ ...buildLinePricing([{ id: "a", cost_price: 100, quantity: 3 }])[0] },
|
||||
{ ...buildLinePricing([{ id: "b", cost_price: 50, quantity: 2 }])[0] },
|
||||
]);
|
||||
expect(totals.costTotal).toBe(400);
|
||||
expect(totals.sellingTotal).toBe(500);
|
||||
expect(totals.marginValue).toBe(100);
|
||||
});
|
||||
|
||||
it("reports margin as a share of REVENUE, not of cost", () => {
|
||||
// cost 100 -> selling 125 is a 25% markup but only a 20% margin.
|
||||
// Conflating the two is the classic pricing mistake.
|
||||
const totals = totalsForLines([
|
||||
buildLinePricing([{ id: "a", cost_price: 100, selling_price: 125 }])[0],
|
||||
]);
|
||||
expect(totals.marginPercent).toBe(20);
|
||||
});
|
||||
|
||||
it("is zero-safe on an empty set and on no-revenue lines", () => {
|
||||
expect(totalsForLines([])).toEqual({
|
||||
costTotal: 0,
|
||||
sellingTotal: 0,
|
||||
marginValue: 0,
|
||||
marginPercent: 0,
|
||||
});
|
||||
const noCost = totalsForLines([
|
||||
buildLinePricing([{ id: "a", cost_price: 0 }])[0],
|
||||
]);
|
||||
expect(noCost.marginPercent).toBe(0);
|
||||
});
|
||||
|
||||
it("keeps cost and selling totals in step after an edit", () => {
|
||||
// The modal recomputes these live on every keystroke, so an
|
||||
// inconsistency here shows up as footer numbers that don't match
|
||||
// the visible rows.
|
||||
let lines = buildLinePricing([{ id: "a", cost_price: 100, quantity: 4 }]);
|
||||
lines = [repriceLine(lines[0], { markupPercent: 30 })];
|
||||
const totals = totalsForLines(lines);
|
||||
expect(totals.costTotal).toBe(400);
|
||||
expect(totals.sellingTotal).toBe(520);
|
||||
});
|
||||
});
|
||||
189
src/lib/quote-pricing.ts
Normal file
189
src/lib/quote-pricing.ts
Normal file
|
|
@ -0,0 +1,189 @@
|
|||
/**
|
||||
* Quote pricing maths for the "create quote from a delivery note"
|
||||
* review modal.
|
||||
*
|
||||
* Pulled into its own module because this is the one place in the
|
||||
* create-quote flow where a wrong answer is invisible: a selling
|
||||
* price that's out by a few cents still renders as a plausible
|
||||
* number, the total still sums, and nothing anywhere says "error".
|
||||
* The operator only finds out when the customer is invoiced.
|
||||
*
|
||||
* The house rule, used consistently across `quotes.create`,
|
||||
* `bulk_update_quote_items` and `quote_items.create`:
|
||||
*
|
||||
* selling = cost x (1 + markup / 100)
|
||||
*
|
||||
* Markup is the INPUT; selling price is DERIVED. Letting both be
|
||||
* typed independently is how a 25% markup and a 30% selling price
|
||||
* end up disagreeing on the same line with nothing to flag it.
|
||||
*/
|
||||
|
||||
/** Round to 2dp, avoiding the float drift that makes 100*1.15 come
|
||||
* out as 114.99999999999999. */
|
||||
export function round2(value: number): number {
|
||||
if (!Number.isFinite(value)) return 0;
|
||||
return Math.round((value + Number.EPSILON) * 100) / 100;
|
||||
}
|
||||
|
||||
/**
|
||||
* Selling price for a cost + markup pair. Markup is a percentage, so
|
||||
* 25 means 25% (i.e. cost * 1.25), NOT 25x — the same convention the
|
||||
* rest of the codebase uses.
|
||||
*/
|
||||
export function sellingFromCostAndMarkup(
|
||||
cost: number,
|
||||
markupPercent: number,
|
||||
): number {
|
||||
const costNum = Number(cost) || 0;
|
||||
const markupNum = Number(markupPercent) || 0;
|
||||
return round2(costNum * (1 + markupNum / 100));
|
||||
}
|
||||
|
||||
/**
|
||||
* The markup percentage implied by a cost + selling pair. The inverse
|
||||
* of the above. Returns 0 for a zero cost rather than dividing by
|
||||
* zero — a line with no cost price genuinely has no margin to quote.
|
||||
*/
|
||||
export function markupFromCostAndSelling(
|
||||
cost: number,
|
||||
selling: number,
|
||||
): number {
|
||||
const costNum = Number(cost) || 0;
|
||||
if (costNum <= 0) return 0;
|
||||
const sellingNum = Number(selling) || 0;
|
||||
return round2(((sellingNum - costNum) / costNum) * 100);
|
||||
}
|
||||
|
||||
/** True when a line has a usable, fixed cost price to work from. */
|
||||
export function hasCostPrice(cost: number | null | undefined): boolean {
|
||||
const n = Number(cost);
|
||||
return Number.isFinite(n) && n > 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* Default markup to seed a line with when the operator hasn't chosen
|
||||
* one. Deliberately a constant rather than a per-customer or
|
||||
* per-category lookup: there is no markup table in the schema today,
|
||||
* and inventing a "smart" default would be guessing at money.
|
||||
*/
|
||||
export const DEFAULT_MARKUP_PERCENT = 25;
|
||||
|
||||
export interface LinePricing {
|
||||
orderItemId: string;
|
||||
description: string;
|
||||
quantity: number;
|
||||
/** Cost carried from the order item. 0 means "nothing on file". */
|
||||
costPrice: number;
|
||||
/** Operator-editable. */
|
||||
markupPercent: number;
|
||||
/** Derived from cost + markup; never typed independently. */
|
||||
sellingPrice: number;
|
||||
}
|
||||
|
||||
export interface LineTotals {
|
||||
costTotal: number;
|
||||
sellingTotal: number;
|
||||
/** sellingTotal - costTotal */
|
||||
marginValue: number;
|
||||
/** margin as a percentage of selling total. 0 when there's no revenue. */
|
||||
marginPercent: number;
|
||||
}
|
||||
|
||||
/** Roll a set of lines up into the footer figures. */
|
||||
export function totalsForLines(lines: LinePricing[]): LineTotals {
|
||||
let costTotal = 0;
|
||||
let sellingTotal = 0;
|
||||
for (const line of lines) {
|
||||
const qty = Number(line.quantity) || 0;
|
||||
costTotal += (Number(line.costPrice) || 0) * qty;
|
||||
sellingTotal += (Number(line.sellingPrice) || 0) * qty;
|
||||
}
|
||||
const marginValue = round2(sellingTotal - costTotal);
|
||||
return {
|
||||
costTotal: round2(costTotal),
|
||||
sellingTotal: round2(sellingTotal),
|
||||
marginValue,
|
||||
marginPercent:
|
||||
sellingTotal > 0 ? round2((marginValue / sellingTotal) * 100) : 0,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the opening price grid for a delivery note's lines.
|
||||
*
|
||||
* `fallbackMarkup` is applied only where a real cost exists — a line
|
||||
* with no cost gets markup 0 and selling 0, because deriving a selling
|
||||
* price from a zero cost produces a confidently wrong R0.00 that looks
|
||||
* like a deliberate free-of-charge item. Those lines are surfaced as
|
||||
* "no cost on file" in the UI instead.
|
||||
*/
|
||||
export function buildLinePricing(
|
||||
items: Array<{
|
||||
id: string;
|
||||
description?: string | null;
|
||||
quantity?: number | null;
|
||||
cost_price?: number | null;
|
||||
selling_price?: number | null;
|
||||
}>,
|
||||
fallbackMarkup: number = DEFAULT_MARKUP_PERCENT,
|
||||
): LinePricing[] {
|
||||
return items.map((it) => {
|
||||
const costPrice = round2(Number(it.cost_price) || 0);
|
||||
const quantity = Number(it.quantity) || 1;
|
||||
|
||||
// A line that already carries a selling price keeps it — the
|
||||
// office may have priced it deliberately before the DN existed,
|
||||
// and silently re-marking it to a default would override a human
|
||||
// decision. Only cost-only lines get the default markup.
|
||||
const existingSelling = round2(Number(it.selling_price) || 0);
|
||||
const hasCost = hasCostPrice(costPrice);
|
||||
|
||||
if (existingSelling > 0) {
|
||||
return {
|
||||
orderItemId: it.id,
|
||||
description: it.description ?? "",
|
||||
quantity,
|
||||
costPrice,
|
||||
markupPercent: markupFromCostAndSelling(costPrice, existingSelling),
|
||||
sellingPrice: existingSelling,
|
||||
};
|
||||
}
|
||||
|
||||
const markupPercent = hasCost ? fallbackMarkup : 0;
|
||||
return {
|
||||
orderItemId: it.id,
|
||||
description: it.description ?? "",
|
||||
quantity,
|
||||
costPrice,
|
||||
markupPercent,
|
||||
sellingPrice: hasCost
|
||||
? sellingFromCostAndMarkup(costPrice, markupPercent)
|
||||
: 0,
|
||||
};
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Re-derive one line's selling price after its cost or markup changed.
|
||||
* Kept as a pure function so the modal's edit handler is a single
|
||||
* call and the derivation rule lives in exactly one place.
|
||||
*/
|
||||
export function repriceLine(
|
||||
line: LinePricing,
|
||||
patch: { costPrice?: number; markupPercent?: number },
|
||||
): LinePricing {
|
||||
const costPrice =
|
||||
patch.costPrice !== undefined ? round2(Number(patch.costPrice) || 0) : line.costPrice;
|
||||
const markupPercent =
|
||||
patch.markupPercent !== undefined
|
||||
? Number(patch.markupPercent) || 0
|
||||
: line.markupPercent;
|
||||
return {
|
||||
...line,
|
||||
costPrice,
|
||||
markupPercent,
|
||||
sellingPrice: hasCostPrice(costPrice)
|
||||
? sellingFromCostAndMarkup(costPrice, markupPercent)
|
||||
: 0,
|
||||
};
|
||||
}
|
||||
|
|
@ -100,7 +100,64 @@ const { store, getOrCreate, where, pbMock, clearStore } = vi.hoisted(() => {
|
|||
Object.assign(item, data);
|
||||
return item;
|
||||
},
|
||||
async delete(id: string) {
|
||||
getOrCreate(name).delete(id);
|
||||
return true;
|
||||
},
|
||||
}),
|
||||
/**
|
||||
* Transactional batch, matching PocketBase's real API. The
|
||||
* create-quote path builds every quote_item in one batch rather
|
||||
* than a loop of individual creates, so the mock has to honour
|
||||
* it — without this the queued creates go nowhere and the tests
|
||||
* that assert on quote_items fail with an empty result.
|
||||
*
|
||||
* All-or-nothing is modelled too: a create that throws poisons
|
||||
* the batch so `send()` applies nothing. That is the real
|
||||
* guarantee, and the compensation path in the service depends on
|
||||
* it being real rather than best-effort.
|
||||
*/
|
||||
createBatch() {
|
||||
const queued: Array<{
|
||||
name: string;
|
||||
run: () => Promise<Record<string, unknown>>;
|
||||
}> = [];
|
||||
return {
|
||||
collection(name: string) {
|
||||
return {
|
||||
create(data: Record<string, unknown>) {
|
||||
queued.push({
|
||||
name,
|
||||
run: async () => {
|
||||
const id = `mock-${Math.random().toString(36).slice(2, 10)}`;
|
||||
const item = { id, ...data };
|
||||
getOrCreate(name).set(id, item);
|
||||
return item;
|
||||
},
|
||||
});
|
||||
return this;
|
||||
},
|
||||
};
|
||||
},
|
||||
async send() {
|
||||
// Two phases: run everything against a scratch copy first
|
||||
// so a mid-batch failure can't leave half the rows behind.
|
||||
const snapshot = new Map(
|
||||
Object.entries(store).map(([k, v]) => [k, new Map(v)]),
|
||||
);
|
||||
const results: Array<Record<string, unknown>> = [];
|
||||
try {
|
||||
for (const op of queued) {
|
||||
results.push(await op.run());
|
||||
}
|
||||
} catch (err) {
|
||||
for (const [k, v] of snapshot) store[k] = v;
|
||||
throw err;
|
||||
}
|
||||
return results;
|
||||
},
|
||||
};
|
||||
},
|
||||
};
|
||||
return { store, getOrCreate, where, pbMock, clearStore };
|
||||
});
|
||||
|
|
|
|||
|
|
@ -38,6 +38,7 @@ import { renderDeliveryNotePdf } from "@/lib/pdf/render";
|
|||
import { compressBuffer } from "@/lib/server/attachment-compressor";
|
||||
import { buildDeliveryNoteLine } from "@/lib/pdf/delivery-note-document";
|
||||
import { firstFileName } from "@/lib/file-fields";
|
||||
import { sellingFromCostAndMarkup } from "@/lib/quote-pricing";
|
||||
import {
|
||||
DELIVERY_NOTE_LETTERHEAD_CODE,
|
||||
deliveryNoteLetterheadCode,
|
||||
|
|
@ -54,7 +55,6 @@ import type {
|
|||
DeliveryNoteStatus,
|
||||
OrderItem,
|
||||
Quote,
|
||||
QuoteItem,
|
||||
} from "@/lib/types";
|
||||
|
||||
const DELIVERY_NOTES = "delivery_notes";
|
||||
|
|
@ -161,8 +161,66 @@ export interface CreateQuoteFromDeliveryNoteResult {
|
|||
* exists, the route surfaces a 409 — the operator can re-open the
|
||||
* existing quote instead of creating a duplicate.
|
||||
*/
|
||||
/**
|
||||
* Best-effort removal of a half-built quote.
|
||||
*
|
||||
* The item batch is transactional but the HEADER is created before it
|
||||
* (the lines need its id, and the PocketBase batch API can't reference
|
||||
* a record created in the same batch). So the one failure mode we
|
||||
* have to close by hand is "header written, lines not".
|
||||
*
|
||||
* A failure here is logged loudly rather than swallowed: it means a
|
||||
* stray empty quote is now sitting in the operator's list and someone
|
||||
* needs to know to remove it.
|
||||
*/
|
||||
async function deleteOrphanQuote(
|
||||
pb: Awaited<ReturnType<typeof getAdminPocketBase>>,
|
||||
quoteId: string,
|
||||
): Promise<void> {
|
||||
try {
|
||||
await pb.collection(QUOTES).delete(quoteId);
|
||||
} catch (err) {
|
||||
console.error(
|
||||
`[delivery-notes-service] FAILED to remove half-built quote ${quoteId}. An empty quote may now be visible in the list and should be deleted manually.`,
|
||||
err,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Per-line pricing the operator reviewed and confirmed in the
|
||||
* "create quote" modal.
|
||||
*
|
||||
* Keyed by `order_item` id, never by array position — the modal
|
||||
* renders whatever lines the detail endpoint returned, and a client
|
||||
* that reordered them (or sent three overrides for a four-line note)
|
||||
* must not silently re-price the wrong goods. `orderItemId` is
|
||||
* validated against this delivery note's own lines server-side.
|
||||
*/
|
||||
export interface QuotePriceOverride {
|
||||
orderItemId: string;
|
||||
costPrice: number;
|
||||
/** Percentage margin. Selling price is derived from it. */
|
||||
markupPercent: number;
|
||||
}
|
||||
|
||||
export interface CreateQuoteFromDeliveryNoteOptions {
|
||||
/**
|
||||
* Which letterhead the quote is rendered on. `CNL` matches the
|
||||
* historical default; DN-backed quotes never used to set this, so
|
||||
* the operator picks it in the modal instead.
|
||||
*/
|
||||
letterhead?: "CNL" | "ISHERWOOD";
|
||||
/**
|
||||
* Operator-confirmed pricing. Omit entirely (the old behaviour) to
|
||||
* fall back to whatever the order_items already carry.
|
||||
*/
|
||||
items?: QuotePriceOverride[];
|
||||
}
|
||||
|
||||
export async function createQuoteFromDeliveryNote(
|
||||
deliveryNoteId: string,
|
||||
options: CreateQuoteFromDeliveryNoteOptions = {},
|
||||
): Promise<CreateQuoteFromDeliveryNoteResult> {
|
||||
const pb = await getAdminPocketBase();
|
||||
|
||||
|
|
@ -226,6 +284,9 @@ export async function createQuoteFromDeliveryNote(
|
|||
// and know instantly "this came from a delivery note, not
|
||||
// from a customer email".
|
||||
rfq_number: dn.delivery_note_number,
|
||||
// The operator picks the letterhead in the review modal. Left
|
||||
// unset by older callers, the renderer falls back to CNL.
|
||||
company_letterhead: options.letterhead ?? "CNL",
|
||||
description:
|
||||
// `??` only falls back on null/undefined, so an empty
|
||||
// `dn.notes` would pass through as an empty description.
|
||||
|
|
@ -235,14 +296,48 @@ export async function createQuoteFromDeliveryNote(
|
|||
};
|
||||
const quote = await pb.collection(QUOTES).create<Quote>(quoteCreate);
|
||||
|
||||
// ── Pricing overrides ────────────────────────────────────────
|
||||
// Validate BEFORE writing anything, so a bad id fails while the
|
||||
// note is still untouched rather than after a quote exists.
|
||||
const overrideById = new Map<string, QuotePriceOverride>();
|
||||
for (const o of options.items ?? []) {
|
||||
const target = items.find((it) => it.id === o.orderItemId);
|
||||
if (!target) {
|
||||
// The client sent a line that isn't on THIS delivery note.
|
||||
// Pricing it here would quietly move another order's goods
|
||||
// into this customer's quote.
|
||||
await deleteOrphanQuote(pb, quote.id);
|
||||
throw new Error(
|
||||
`Pricing override references order item ${o.orderItemId}, which is not on delivery note ${dn.delivery_note_number}.`,
|
||||
);
|
||||
}
|
||||
overrideById.set(o.orderItemId, o);
|
||||
}
|
||||
|
||||
// Create one quote_item per order_item. We deliberately DON'T
|
||||
// touch order_item status here — the items stay at
|
||||
// item_status="delivery_note" until the operator clicks
|
||||
// "Mark Invoiced". The quote and the order_items are two views
|
||||
// of the same physical goods; flipping order_items here would
|
||||
// break the Invoiced-tab count before any invoice exists.
|
||||
//
|
||||
// These go out as ONE PocketBase batch rather than a loop of
|
||||
// individual creates. The batch is transactional: a failure on
|
||||
// line 5 of 8 aborts the whole thing instead of leaving a real
|
||||
// quote header attached to 4 of its 8 lines — which is exactly
|
||||
// the state that made the old loop unrecoverable without manual
|
||||
// cleanup in PB.
|
||||
const batch = pb.createBatch();
|
||||
for (const it of items) {
|
||||
const itemCreate: Record<string, unknown> = {
|
||||
const override = overrideById.get(it.id);
|
||||
const costPrice = override
|
||||
? Number(override.costPrice) || 0
|
||||
: Number(it.cost_price ?? 0);
|
||||
const sellingPrice = override
|
||||
? sellingFromCostAndMarkup(costPrice, override.markupPercent)
|
||||
: Number(it.selling_price ?? 0);
|
||||
|
||||
batch.collection(QUOTE_ITEMS).create({
|
||||
quote: quote.id,
|
||||
quote_number: quoteNumber,
|
||||
description: it.description ?? "",
|
||||
|
|
@ -253,15 +348,30 @@ export async function createQuoteFromDeliveryNote(
|
|||
supplier: it.supplier ?? null,
|
||||
// Costs always carry over — the operator needs to see what
|
||||
// the office paid to set a margin.
|
||||
cost_price: Number(it.cost_price ?? 0),
|
||||
// Selling price is best-effort: copy if set, otherwise 0
|
||||
// and pricing_source="manual" so the UI flags it.
|
||||
selling_price: Number(it.selling_price ?? 0),
|
||||
cost_price: costPrice,
|
||||
// Selling price is best-effort when unoverridden: copy if set,
|
||||
// otherwise 0 and pricing_source="manual" so the UI flags it.
|
||||
selling_price: sellingPrice,
|
||||
notes: it.notes ?? "",
|
||||
sku: it.sku ?? "",
|
||||
pricing_source: it.selling_price ? it.pricing_source ?? "" : "manual",
|
||||
};
|
||||
await pb.collection(QUOTE_ITEMS).create<QuoteItem>(itemCreate);
|
||||
// An override is a deliberate human decision, so it's recorded
|
||||
// as one — same marker the operator's own quote-page edits use.
|
||||
pricing_source: override
|
||||
? "manual"
|
||||
: it.selling_price
|
||||
? it.pricing_source ?? ""
|
||||
: "manual",
|
||||
});
|
||||
}
|
||||
|
||||
try {
|
||||
await batch.send();
|
||||
} catch (batchErr) {
|
||||
// Compensate: the header exists but its lines don't. Remove it so
|
||||
// the operator isn't left with a quote that looks real and has
|
||||
// some of its lines missing.
|
||||
await deleteOrphanQuote(pb, quote.id);
|
||||
throw batchErr;
|
||||
}
|
||||
|
||||
// v3-C: also attach the delivery note to the new quote so
|
||||
|
|
|
|||
Loading…
Reference in a new issue