feat(delivery-notes): one linear flow control, and fix the quote lookup

The operator asked for one action at the end of the row that walks
her through the work, instead of a strip of unlabelled icons.

  [ Create quote ]  [ Send ]  [ Create invoice ]

Once a quote exists the FIRST slot becomes the quote number itself,
clickable, so she never hunts for where the quote went. Clicking it
opens the quote PDF in a popup with Print and Download — she stays on
the delivery-notes list, which is the whole point.

Deliberately not a fixed step 1 -> 2 -> 3: the counter also has
customer-to-customer work that never needs a quote, so "Create
invoice" is always offered and never depends on the quote having
happened. A fixed sequence would strand that path.

Send is mounted in place rather than by navigating to the quote page,
using the existing purpose-built modal (loud To field, alt-email
toggle, rep auto-CC, PDF attached, retry banner on a bounce). The
quote fetch now expands `customer,company` because that modal reads
`quote.expand.customer.rep` to pre-fill CC — without it, sends went
out silently with no CC and the office never saw a copy.

Everything else (signed copy, capture PO, mark invoiced, cancel,
regenerate PDF) stays on the detail page. That is a deliberate
hierarchy, not the drift this component was extracted to prevent: the
list is for scanning plus the three things you do to every note.

--- the bug this uncovered ---

The flow control could never show a quote, because the quote map was
always empty. `getQuotesForDeliveryNotes` built ONE PocketBase filter
of `delivery_note = "x" || ... ` across every id. The list fetches 500
notes, so that filter carried ~250 clauses, and PocketBase rejects it
with a bare `400 Something went wrong`. Measured on this instance: 75
clauses succeed, 100 fail.

The caller treats that fetch as non-fatal and swallows the error, so
it was completely silent. For the life of the feature:
  - the Q icon never switched to "open quote" mode,
  - `hasQuote` was always false,
  - the blue "quoted" row tint never appeared, and
  - the Send action could never be enabled.
A whole session of 3676 tests passed without catching it — they all
use a handful of ids, far under the limit. Only driving the real app
surfaced it.

Now chunked at 50 ids per request. Five sequential requests for a
250-note list is nothing next to the full delivery_notes page already
fetched alongside it.

Verified in the browser as David Els: DN26100599 now shows
Q202610102 in slot 1, Send is enabled, and clicking the quote opens
the generated PDF in a popup showing the priced line (1/2" X 2 1/4"
UNF BOLT, qty 10, R150.00, total R1 500.00) with Print and Download.

Full suite 3676 passing, tsc clean.
This commit is contained in:
Dave 2026-10-05 18:21:19 +02:00
parent 7ce23e04f3
commit afae9cdd99
3 changed files with 288 additions and 108 deletions

View file

@ -20,6 +20,7 @@ import {
import { CreateAdHocDeliveryNoteDialog } from "@/components/delivery-notes/create-adhoc-dialog"; import { CreateAdHocDeliveryNoteDialog } from "@/components/delivery-notes/create-adhoc-dialog";
import { DeliveryNoteItemsExpanded } from "@/components/delivery-notes/items-expanded"; import { DeliveryNoteItemsExpanded } from "@/components/delivery-notes/items-expanded";
import { DeliveryNoteActions } from "@/components/delivery-notes/action-buttons"; import { DeliveryNoteActions } from "@/components/delivery-notes/action-buttons";
import { SendQuoteModal } from "@/components/quotes/send-quote-modal";
import { QuoteReviewDialog } from "@/components/delivery-notes/quote-review-dialog"; import { QuoteReviewDialog } from "@/components/delivery-notes/quote-review-dialog";
import { import {
PdfPreviewDialog, PdfPreviewDialog,
@ -219,6 +220,34 @@ export default function DeliveryNotesPage() {
customer: string; customer: string;
}>({ open: false, noteId: "", number: "", customer: "" }); }>({ open: false, noteId: "", number: "", customer: "" });
// Send DN + quote, in a modal on THIS screen. The operator's whole
// point is not to leave the list. The quote must carry its customer
// expanded — `SendQuoteModal` reads `quote.expand.customer.rep` to
// pre-fill the rep on CC.
const [sendModal, setSendModal] = useState<{
open: boolean;
noteId: string;
quoteId: string;
quoteNumber: string;
letterhead: string;
}>({ open: false, noteId: "", quoteId: "", quoteNumber: "", letterhead: "" });
const [sendSubmitting, setSendSubmitting] = useState(false);
const openSendModal = useCallback(
(note: DeliveryNote, quote: Quote) => {
setSendModal({
open: true,
noteId: note.id,
quoteId: quote.id,
quoteNumber: quote.quote_number,
letterhead: quote.company_letterhead || "CNL",
});
},
[],
);
// PDF preview dialog state. The D icon on every row opens // PDF preview dialog state. The D icon on every row opens
// this instead of opening the PDF in a new tab — operator // this instead of opening the PDF in a new tab — operator
// stays in the list, can compare two DNs side by side, doesn't // stays in the list, can compare two DNs side by side, doesn't
@ -732,20 +761,6 @@ export default function DeliveryNotesPage() {
[fetchNotes, markBusy, markIdle, quoteByDnId], [fetchNotes, markBusy, markIdle, quoteByDnId],
); );
// ── Action: Capture Customer PO ─────────────────────────────────
// Opens the modal pre-filled with any existing PO (so a re-open
// to fix a typo is one-keystroke-friendly). Submission calls the
// API route, which stamps the PO and flips status to
// "po_received" (or just stamps if already past that status).
const openPoModal = useCallback((note: DeliveryNote) => {
setPoModal({
open: true,
noteId: note.id,
value: note.customer_po ?? "",
saving: false,
});
}, []);
const submitPoModal = useCallback(async () => { const submitPoModal = useCallback(async () => {
if (!poModal.noteId) return; if (!poModal.noteId) return;
const trimmed = poModal.value.trim(); const trimmed = poModal.value.trim();
@ -790,48 +805,6 @@ export default function DeliveryNotesPage() {
} }
}, [fetchNotes, markBusy, markIdle, poModal]); }, [fetchNotes, markBusy, markIdle, poModal]);
// ── Action: Mark Invoiced ───────────────────────────────────────
// Flips the order_items to "ready_invoice" so they show up in
// the Invoicing page, and flips the delivery note's status to
// "invoiced". The actual invoice creation still happens through
// the Invoicing page.
const handleMarkInvoiced = useCallback(
async (note: DeliveryNote) => {
markBusy(note.id);
try {
const res = await fetch(`/api/v1/delivery-notes/${note.id}/mark-invoiced`, {
method: "POST",
});
if (!res.ok) {
let message = `Failed to mark as invoiced (${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;
}
toast.success(
`Delivery note ${note.delivery_note_number} marked as invoiced. The items now appear in the Invoicing page.`,
);
await fetchNotes();
} catch (err: unknown) {
toast.error(err instanceof Error ? err.message : "Failed to mark as invoiced");
} finally {
markIdle(note.id);
}
},
[fetchNotes, markBusy, markIdle],
);
// ── Status-driven action button ───────────────────────────────── // ── Status-driven action button ─────────────────────────────────
// One button per row, swapped by the delivery note's status. The // One button per row, swapped by the delivery note's status. The
@ -889,42 +862,33 @@ export default function DeliveryNotesPage() {
<DeliveryNoteActions <DeliveryNoteActions
note={note} note={note}
hasQuote={!!quote} hasQuote={!!quote}
variant="icons" quoteNumber={quote?.quote_number}
variant="flow"
busy={busyIds.has(note.id)} busy={busyIds.has(note.id)}
primary={
note.status === "sent"
? "capture-po"
: note.status === "po_received"
? "mark-invoiced"
: undefined
}
onAction={{ onAction={{
"preview-dn": () => setPreviewDnNumber(note.delivery_note_number), // Slot 1 — "Create quote", or the quote itself once it
"preview-quote": () => quote && setPreviewQuote(quote), // exists. The popup means she never leaves this list to
// read a quote.
"create-quote": () => openQuoteReview(note), "create-quote": () => openQuoteReview(note),
send: () => { "preview-quote": () => quote && setPreviewQuote(quote),
if (quote) router.push(`/quotes/${quote.id}?sendNow=true`); // Slot 2 — send, in a modal on this screen.
}, send: () => quote && openSendModal(note, quote),
"capture-po": () => openPoModal(note), // Slot 3 — the customer-to-customer path, always
"mark-invoiced": () => void handleMarkInvoiced(note), // available; no quote required. Opens the QuickBooks
// modal on the note's own page via ?invoice=1, which is
// one click and lands ready to use.
"invoice-qb": () => "invoice-qb": () =>
router.push(`/delivery-notes/${note.id}?invoice=1`), router.push(`/delivery-notes/${note.id}?invoice=1`),
"mark-complete": () =>
router.push(`/delivery-notes/${note.id}`),
cancel: () => router.push(`/delivery-notes/${note.id}`),
}} }}
/> />
</div> </div>
); );
}, },
[ // `openPoModal` / `handleMarkInvoiced` are no longer referenced
busyIds, // here: the flow control offers quote / send / invoice only. Those
openPoModal, // two actions live on the detail page, which still renders the
openQuoteReview, // full set from the same component.
handleMarkInvoiced, [busyIds, openQuoteReview, openSendModal, quoteByDnId, router],
quoteByDnId,
router,
],
); );
const columns = useMemo( const columns = useMemo(
@ -1721,6 +1685,59 @@ export default function DeliveryNotesPage() {
on any row that doesn't have a quote yet. The dialog POSTs on any row that doesn't have a quote yet. The dialog POSTs
the create itself with the operator's confirmed pricing; the create itself with the operator's confirmed pricing;
`handleQuoteCreated` only refreshes the row behind it. */} `handleQuoteCreated` only refreshes the row behind it. */}
{/* Send DN + quote, in place. This is the existing
purpose-built modal (loud To field, alt-email toggle, rep
auto-CC, PDF attached, retry banner on a bounce) mounted
here so the operator never leaves the list to use it. */}
{sendModal.quoteId && (
<SendQuoteModal
key={`dn-send-${sendModal.quoteId}`}
open={sendModal.open}
onOpenChange={(v) => setSendModal((prev) => ({ ...prev, open: v }))}
quote={quoteByDnId.get(sendModal.noteId) as Quote}
outboundAttachments={[]}
claimerName={null}
submitting={sendSubmitting}
onSend={async (payload) => {
setSendSubmitting(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/quotes/${sendModal.quoteId}/send-email`,
{
method: "POST",
headers: {
"Content-Type": "application/json",
Authorization: token,
},
body: JSON.stringify(payload),
},
);
const body = (await res.json().catch(() => ({}))) as {
error?: string;
};
if (!res.ok) {
throw new Error(
body.error || `Failed to send (${res.status})`,
);
}
toast.success(`Sending ${sendModal.quoteNumber} to the customer…`);
setSendModal((prev) => ({ ...prev, open: false }));
} catch (err: unknown) {
toast.error(
err instanceof Error ? err.message : "Failed to send the quote",
);
} finally {
setSendSubmitting(false);
}
}}
/>
)}
{quoteReview.noteId && ( {quoteReview.noteId && (
<QuoteReviewDialog <QuoteReviewDialog
key={quoteReview.noteId} key={quoteReview.noteId}

View file

@ -208,10 +208,127 @@ export const DN_ACTIONS: DnActionSpec[] = [
}, },
]; ];
/**
* The LINEAR FLOW control — the one the counter operator uses on
* every note.
*
* Three labelled buttons, left to right, in the order the work
* happens:
*
* [ Create quote ] [ Send ] [ Create invoice ]
*
* Once a quote exists the first button IS the quote — it becomes
* the quote number, clickable, so the operator never has to hunt for
* "where did that quote go". The same slot, the same place, every
* time; it just says the next useful thing.
*
* Deliberately NOT a fixed sequence: the counter also has
* customer-to-customer work that never needs a quote, so "Invoice"
* is always offered and never depends on the quote step having
* happened. A fixed "step 1 → 2 → 3" would strand that path.
*
* Everything else (signed copy, capture PO, mark invoiced, cancel,
* regenerate PDF) stays on the detail page. That is a deliberate
* hierarchy, not the drift this component was extracted to prevent:
* the list is for scanning and the three things you do to every
* note; the detail page is where you do everything to one note.
* The full set is still reachable in one click, and is still the same
* component deciding what applies.
*/
function FlowControl({
ctx,
note,
hasQuote,
quoteNumber,
onAction,
busy,
disabled,
}: {
ctx: DnActionContext;
note: Pick<DeliveryNote, "id" | "status" | "signed_copy">;
hasQuote: boolean;
quoteNumber?: string;
onAction: Partial<Record<DnActionId, () => void>>;
busy: boolean;
disabled: boolean;
}) {
const isLive = ctx.status !== "completed" && ctx.status !== "cancelled";
const off = busy || disabled;
return (
<div className="flex items-center justify-end gap-1.5 whitespace-nowrap">
{/* Slot 1 — the thing to do next. Quote link once it exists. */}
{hasQuote ? (
<button
type="button"
onClick={onAction["preview-quote"]}
disabled={off}
data-testid={`dn-flow-quote-${note.id}`}
title={`Open quote ${quoteNumber ?? ""} — click to preview the PDF`}
className="inline-flex items-center gap-1 rounded-md border border-info/40 bg-info/10 px-2.5 py-1 font-mono text-xs font-medium text-info transition-colors hover:bg-info/20 disabled:opacity-50"
>
{quoteNumber}
</button>
) : (
<Button
size="sm"
onClick={onAction["create-quote"]}
disabled={off || !isLive}
data-testid={`dn-flow-create-quote-${note.id}`}
title="Review the pricing, then create the quote"
className="h-7 px-2.5 text-xs"
>
Create quote
</Button>
)}
{/* Slot 2 — send it. Needs a quote; there is nothing to attach
without one, so it is disabled rather than hidden, which
keeps the three slots in fixed positions. */}
<Button
size="sm"
variant="outline"
onClick={onAction.send}
disabled={off || !hasQuote || !isLive}
data-testid={`dn-flow-send-${note.id}`}
title={
hasQuote
? "Send the delivery note and quote to the customer"
: "Create a quote first — there is nothing to send yet"
}
className="h-7 px-2.5 text-xs"
>
Send
</Button>
{/* Slot 3 — the customer-to-customer path, always available
while the note is live. */}
<Button
size="sm"
variant="outline"
onClick={onAction["invoice-qb"]}
disabled={off || !isLive}
data-testid={`dn-flow-invoice-${note.id}`}
title="Create the invoice in QuickBooks"
className="h-7 px-2.5 text-xs"
>
Create invoice
</Button>
</div>
);
}
export interface DeliveryNoteActionsProps { export interface DeliveryNoteActionsProps {
note: Pick<DeliveryNote, "id" | "status" | "signed_copy">; note: Pick<DeliveryNote, "id" | "status" | "signed_copy">;
hasQuote: boolean; hasQuote: boolean;
variant: "icons" | "buttons"; /** Shown on the flow variant's quote link. Falls back to nothing. */
quoteNumber?: string;
/**
* - "flow" — the three-button linear control (list rows)
* - "icons" — compact icon row
* - "buttons" — labelled full set (detail page)
*/
variant: "flow" | "icons" | "buttons";
/** Per-action handlers. An action with no handler is not rendered. */ /** Per-action handlers. An action with no handler is not rendered. */
onAction: Partial<Record<DnActionId, () => void>>; onAction: Partial<Record<DnActionId, () => void>>;
/** Actions currently in flight, for the spinner / disabled state. */ /** Actions currently in flight, for the spinner / disabled state. */
@ -230,6 +347,7 @@ export interface DeliveryNoteActionsProps {
export function DeliveryNoteActions({ export function DeliveryNoteActions({
note, note,
hasQuote, hasQuote,
quoteNumber,
variant, variant,
onAction, onAction,
busy = false, busy = false,
@ -239,6 +357,21 @@ export function DeliveryNoteActions({
testIdPrefix = "dn-action", testIdPrefix = "dn-action",
}: DeliveryNoteActionsProps) { }: DeliveryNoteActionsProps) {
const ctx = dnActionContext(note, hasQuote); const ctx = dnActionContext(note, hasQuote);
if (variant === "flow") {
return (
<FlowControl
ctx={ctx}
note={note}
hasQuote={hasQuote}
quoteNumber={quoteNumber}
onAction={onAction}
busy={busy}
disabled={disabled}
/>
);
}
const shown = DN_ACTIONS.filter( const shown = DN_ACTIONS.filter(
(a) => a.available(ctx) && typeof onAction[a.id] === "function", (a) => a.available(ctx) && typeof onAction[a.id] === "function",
); );

View file

@ -1129,37 +1129,67 @@ export async function getQuotesForDeliveryNotes(
const out = new Map<string, Quote>(); const out = new Map<string, Quote>();
if (deliveryNoteIds.length === 0) return out; if (deliveryNoteIds.length === 0) return out;
const pb = getPocketBase(); const pb = getPocketBase();
const clauses = deliveryNoteIds
.map((id) => id.replace(/"/g, '\\"')) // CHUNKED, deliberately.
.map((id) => `delivery_note = "${id}"`) //
.join(" || "); // This used to build ONE filter of `delivery_note = "x" || ... `
// PB pages the result — for the realistic list size (≤50 // across every id. The list page fetches 500 delivery notes, so
// rows on a 20-per-page table) one page is enough. If we // that filter carried ~250 clauses — and PocketBase rejects it
// ever hit a "no rows" response we still want the caller // with a bare `400 {"message":"Something went wrong..."}`. Measured
// to get an empty map, not throw. // against this instance: 75 clauses succeed, 100 fail.
let page = 1; //
// Generous cap: list pages won't show > 200 DNs; the cap // The caller treats this fetch as non-fatal and swallows the error,
// exists so a malformed input doesn't loop forever. // so the symptom was completely silent: `quoteByDnId` came back
while (page < 20) { // EMPTY, and therefore
const result = await pb.collection("quotes").getList<Quote>( // - the Q icon never switched to "open quote" mode,
page, // - `hasQuote` was always false,
200, // - the blue "quoted" row tint never appeared, and
{ // - the Send action could never be enabled.
filter: clauses, // Found in the browser, after a whole session of tests passing:
sort: "-created", // the unit tests all use a handful of ids, well under the limit.
}, //
); // 50 is comfortably under the measured ceiling. Five sequential
for (const q of result.items) { // requests for a 250-note list is nothing next to the full
if (q.delivery_note && !out.has(q.delivery_note)) { // delivery_notes page already being fetched alongside it.
out.set(q.delivery_note, q); const CHUNK = 50;
const ids = deliveryNoteIds.map((id) => id.replace(/"/g, '\\"'));
for (let start = 0; start < ids.length; start += CHUNK) {
const slice = ids.slice(start, start + CHUNK);
const clauses = slice.map((id) => `delivery_note = "${id}"`).join(" || ");
// PB pages the result — the caller only ever needs the most
// recent quote per note, and one page covers a 50-note chunk.
// Generous cap so a malformed input can't loop forever.
let page = 1;
while (page < 20) {
const result = await pb.collection("quotes").getList<Quote>(
page,
200,
{
filter: clauses,
sort: "-created",
// The delivery-notes list mounts `SendQuoteModal` directly,
// and that modal reads `quote.expand.customer.rep` to
// pre-fill the rep on CC. Without this expand it silently
// sends with no CC, so the office never sees a copy of what
// went out.
expand: "customer,company",
},
);
for (const q of result.items) {
if (q.delivery_note && !out.has(q.delivery_note)) {
out.set(q.delivery_note, q);
}
} }
if (result.items.length < 200) break;
page++;
} }
if (result.items.length < 200) break;
page++;
} }
return out; return out;
} }
export async function generateDeliveryNoteNumber(): Promise<string> { export async function generateDeliveryNoteNumber(): Promise<string> {
const pb = getPocketBase(); const pb = getPocketBase();
const now = new Date(); const now = new Date();