From 3de1d622b880d3043621fe68614b063149ecdd8c Mon Sep 17 00:00:00 2001 From: Dave Date: Mon, 7 Sep 2026 16:51:55 +0200 Subject: [PATCH] feat(quotes): allow resubmit from any post-submit status, not just rejected The submit action was previously gated on the 'rejected' status only, so reps had no way to push an emailed, follow-up, problem, or completed quote back through the approval queue after a customer came back with changes. Q202609137 sat in 'Awaiting reply' (emailed) with no UI to re-route it. Centralise the resubmit eligibility in quote-status.ts as a single RESUBMITTABLE_STATUSES set (emailed, approval_pending, approved, complete, follow_up, problem, mail_failed, email_failed, partial_approved, rejected) and have quotes-service submitQuote read from it. The page banner switches from a rejection-only red banner to a generic ResubmitBanner with two visual variants: destructive for rejection, info for the other nine states. Cancelled, claimed, sending, and template quotes stay locked. Splits the old submitQuote test into 'still rejects cancelled + claimed' and 'accepts every resubmittable status' so the 10 allowed transitions are each asserted individually. Also fixes a missing pocketbase-admin mock that was breaking the submitQuote suite. --- src/app/(app)/quotes/[id]/page.tsx | 312 +++++++++++++++++++++----- src/lib/quote-status.ts | 98 ++++++++ src/lib/server/quotes-service.test.ts | 96 +++++++- src/lib/server/quotes-service.ts | 60 +++-- 4 files changed, 486 insertions(+), 80 deletions(-) diff --git a/src/app/(app)/quotes/[id]/page.tsx b/src/app/(app)/quotes/[id]/page.tsx index cf3b462..7248609 100644 --- a/src/app/(app)/quotes/[id]/page.tsx +++ b/src/app/(app)/quotes/[id]/page.tsx @@ -87,7 +87,7 @@ import { import { toast } from "sonner"; import getPocketBase from "@/lib/pocketbase"; import { createOrderFromQuote, getOrderByQuote, getQuote, getQuoteItems, getQuoteAttachments, formatDate, formatCurrency } from "@/lib/pb-helpers"; -import { quoteStatusClassName } from "@/lib/quote-status"; +import { quoteStatusClassName, isResubmittableStatus, resubmitStatusMessage } from "@/lib/quote-status"; import QuoteEmailCard from "@/components/quote-email-card"; import RewriteQuoteModal from "@/components/rewrite-quote-modal"; @@ -239,57 +239,151 @@ function formatHistoryTimestamp(iso: string | null | undefined): string { } /** - * RejectedBanner — destructive-tinted banner shown on /quotes/[id] - * when the rep bounced this quote back to the operator. Sits - * directly below the title strip so the operator sees the reason - * before they start re-pricing. + * ResubmitBanner — affordance that surfaces the "Resubmit for + * Approval" action on `/quotes/[id]` whenever the server will + * accept a resubmit (see `RESUBMITTABLE_STATUSES` in + * `@/lib/quote-status`). Sits directly below the title strip so + * the operator sees the prompt before they start re-pricing. * - * The banner is sourced from the most recent rejection line in - * `quotes.notes` (see `parseLatestRejection`). It only renders - * when status is `new` AND a rejection line is found — quotes - * that have never been rejected, or whose notes pre-date the - * structured format, fall through to the normal page. + * Two visual variants: * - * Why we only show it for status `new`: legacy rejected quotes - * (status still `rejected` from before this flow change) surface - * under the Cancelled tab and render as terminal — adding the - * banner there would imply they're actionable. Once everything - * routes through the new flow, status `rejected` becomes - * unreachable and the gate can be relaxed. + * 1. Destructive (red) — the rep bounced this quote back to + * the operator. Triggered when `status === "new"` AND a + * structured rejection line exists in `quotes.notes`. Shows + * the rejection reason (who, when, why) so the operator + * knows what to fix. This is the original "RejectedBanner" + * behaviour, kept verbatim so the existing testid + + * styling survives. + * + * 2. Info (blue) — every other status in + * `RESUBMITTABLE_STATUSES` (emailed / complete / approved / + * follow_up / problem / partial_approved / mail_failed / + * email_failed / rejected-legacy). Shows a status-specific + * one-liner from `resubmitStatusMessage`. No rejection + * context because there isn't one to show — the operator + * is the one driving the resubmit, not the rep. + * + * Why expose the button for every resubmittable status (not just + * rejection): + * + * - `emailed` (waiting reply): the customer came back with + * changes — operator edits quantities/prices and resubmits. + * - `complete` / `approved`: closed/approved quote reopened + * by a follow-up conversation — same audit trail, no need + * to re-quote from scratch. + * - `follow_up` / `problem` / `mail_failed` / `email_failed`: + * operator fixed the underlying issue and wants a fresh + * approval round. + * - `partial_approved`: operator wants a fresh full-approval + * cycle on a previously-partial quote. + * + * The button stays disabled while `resubmitting` is true to + * prevent double-submits; we don't reuse `actionLoading` + * because that flag covers the local-PB status-flip actions and + * the two state machines are independent. Clicking it POSTs + * `/api/v1/quotes/[id]/submit`, which flips `status` to + * `approval_pending` (or returns 422 `unpriced_items` if a line + * is still priced at zero, which the page surfaces as a toast). */ -function RejectedBanner({ quote }: { quote: Quote }) { - if (quote.status !== "new") return null; - const rejection = parseLatestRejection(quote.notes); - if (!rejection) return null; +function ResubmitBanner({ + quote, + onResubmit, + resubmitting, +}: { + quote: Quote; + onResubmit: () => void; + resubmitting: boolean; +}) { + const rejection = quote.status === "new" ? parseLatestRejection(quote.notes) : null; + + if (rejection) { + return ( +
+ +
+
+ Previously rejected — please fix and resubmit +
+
+ {rejection.by}{" "} + rejected this on{" "} + + {formatHistoryTimestamp(rejection.at)} + + . +
+ {rejection.reason && ( +
+ + Reason + +

+ {rejection.reason} +

+
+ )} +
+
+ +
+
+ ); + } + + // Non-rejection case: only show when the status is one the + // server will accept. Cancelled / claimed / sending / template + // fall through to a normal page render. + if (!isResubmittableStatus(quote.status)) return null; + return (
- +
-
- Previously rejected — please fix and resubmit +
Resubmit for approval
+
+ {resubmitStatusMessage(quote.status)}
-
- {rejection.by}{" "} - rejected this on{" "} - - {formatHistoryTimestamp(rejection.at)} - - . -
- {rejection.reason && ( -
- - Reason - -

- {rejection.reason} -

-
- )} +
+
+
); @@ -906,6 +1000,13 @@ export default function QuoteDetailPage({ params }: { params: Promise<{ id: stri }>>([]); const [creatingOrder, setCreatingOrder] = useState(false); const [rewriteOpen, setRewriteOpen] = useState(false); + // Local in-flight flag for the Resubmit-for-Approval button on the + // RejectedBanner. Kept separate from `actionLoading` (which covers + // the local-PB status-flip actions like claim/unclaim/cancel) so + // the two state machines don't interfere — a pending resubmit + // shouldn't disable the kebab-menu actions, and vice versa. Also + // drives the button's `disabled` state and the spinner swap. + const [resubmitting, setResubmitting] = useState(false); // Outbound attachments — files the operator uploaded to send with // the quote email. Kept separate from the inbound `attachments` // list (which the QuoteEmailCard shows) so the two UIs stay @@ -1243,6 +1344,92 @@ export default function QuoteDetailPage({ params }: { params: Promise<{ id: stri } }; + /** + * Resubmit a quote for approval. Two flavours: + * + * 1. Post-rejection resubmit — the rep bounced the quote + * back to the operator's queue (status → `new`), the + * operator fixed the items, and now they're sending it + * back into the rep's approval queue. + * + * 2. Post-send resubmit — the customer came back with + * changes on a quote that's been through the pipeline + * once already (status `emailed`, `complete`, `approved`, + * `follow_up`, etc.). The operator edits quantities or + * prices and resubmits without re-quoting from scratch, + * preserving the original quote_number and email + * history. + * + * Both paths hit `POST /api/v1/quotes/[id]/submit`, which + * validates: + * + * - every priced item has `selling_price > 0` + * (otherwise 422 `unpriced_items` with the offending ids) + * - the quote is in a submittable status (the union of + * `new` and `RESUBMITTABLE_STATUSES` — see + * `@/lib/quote-status` for the canonical list) + * + * The unpriced-items gate matters most for the + * post-rejection path (operator reopens the items table to + * fix the rejection reason, and may have left a line at + * zero), but the server applies it uniformly so a partial + * re-edit on an already-sent quote doesn't ship an + * incomplete line. + * + * Idempotency: re-submitting an already-`approval_pending` + * quote is a server-side no-op with `statusChanged: false`. + * We surface that as an info toast so the operator knows + * their click was registered but didn't change anything. + */ + const handleResubmitForApproval = async () => { + setResubmitting(true); + try { + const res = await fetch(`/api/v1/quotes/${id}/submit`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({}), + }); + const body = (await res.json().catch(() => ({}))) as { + error?: string; + code?: "quote_not_found" | "invalid_status" | "unpriced_items"; + offendingItemIds?: string[]; + statusChanged?: boolean; + targetStatus?: string; + }; + if (!res.ok) { + if (body.code === "unpriced_items") { + const count = body.offendingItemIds?.length ?? 0; + toast.error( + count > 0 + ? `Cannot resubmit: ${count} item${count === 1 ? "" : "s"} still have selling_price = 0. Price every line before resubmitting.` + : body.error || "Cannot resubmit: items are missing selling prices.", + ); + return; + } + if (body.code === "invalid_status" || body.code === "quote_not_found") { + toast.error(body.error || "Cannot resubmit in current status."); + return; + } + throw new Error(body.error || `HTTP ${res.status}`); + } + if (body.statusChanged === false) { + // Already in approval_pending — no-op. The button shouldn't + // normally be visible in this state (the banner gate is + // status === "new"), but defending against a race where the + // page hasn't refetched yet. + toast.info("Already pending approval"); + } else { + toast.success("Resubmitted for approval"); + } + fetchQuote(); + } catch (err: unknown) { + const message = err instanceof Error ? err.message : "Failed to resubmit"; + toast.error(message); + } finally { + setResubmitting(false); + } + }; + const handleDelete = async () => { try { const pb = getPocketBase(); @@ -2084,18 +2271,31 @@ export default function QuoteDetailPage({ params }: { params: Promise<{ id: stri
- {/* ── Previously-rejected banner ────────────────────────── - When the rep bounces this quote back to the operator - (status was flipped to "new"), we surface the rejection - reason right under the header so the operator sees - "what to fix" before they start re-pricing. The banner - is sourced from the same `quotes.notes` audit line both - reject paths write — `/api/v1/quotes/[id]/review-approve` - (the review screen) and the queue-card reject on - `/quotes/approvals` — so either rejection surface shows - up here. Hidden when the quote was never rejected, or - when the notes pre-date the structured format. */} - + {/* ── Resubmit-for-approval banner ──────────────────────── + Surfaces the "Resubmit for Approval" action on every + status the server will accept (see + `RESUBMITTABLE_STATUSES` in `@/lib/quote-status`). + Two visual variants: + - destructive (red) when status === "new" AND notes + contain a structured rejection line — shows the + rejection reason + - info (blue) for every other resubmittable status + (`emailed`, `complete`, `approved`, `follow_up`, + `problem`, `partial_approved`, `mail_failed`, + `email_failed`, legacy `rejected`) — shows a + status-specific one-liner + Hidden when the status is `cancelled`, `claimed`, + `sending`, `template`, or anything else the server + refuses. The rejection line is parsed from the same + `quotes.notes` audit line both reject paths write — + `/api/v1/quotes/[id]/review-approve` (the review + screen) and the queue-card reject on `/quotes/approvals` + — so either rejection surface shows up here. */} + {/* Compact customer strip — replaces the old "Customer" sidebar diff --git a/src/lib/quote-status.ts b/src/lib/quote-status.ts index 0d3e973..55d67c6 100644 --- a/src/lib/quote-status.ts +++ b/src/lib/quote-status.ts @@ -9,6 +9,8 @@ * the status MEANS to the operator, not after a Tailwind palette * name. That way the suite works across every theme in the picker. */ +import type { QuoteStatus } from "@/lib/types"; + const quoteStatusClasses: Record = { // In-flight / awaiting further action — info (cool blue) new: "border-info/30 bg-info/10 text-info", @@ -103,3 +105,99 @@ const STATUS_LABELS: Record = { export function formatQuoteStatus(status: string) { return STATUS_LABELS[status] || status.replace(/_/g, " "); } + +/** + * Quote statuses that can be re-submitted through the approval queue + * via POST /api/v1/quotes/[id]/submit. Lives in the shared status + * helper so the client UI (banner gate) and the server + * (`submitQuote` in `quotes-service.ts`) draw from the same source + * of truth — a new status added here surfaces in both places without + * a second edit. + * + * Background: before this set existed (pre-2026-09-07), `submitQuote` + * only accepted `new`. Once a quote had been emailed and the customer + * came back asking for changes, the operator's only path was to + * re-quote from scratch, which threw away the original quote_number, + * email tracking, and the customer's audit trail. Re-submitting on + * the same quote preserves every one of those. + * + * Includes: + * - `emailed` → customer came back with changes + * - `complete` → customer came back after closure + * - `partial_approved` → operator wants to re-open a partial + * for a fresh full-approval cycle + * - `mail_failed` → operator fixed the underlying issue + * and wants to retry through approval + * - `email_failed` → ditto, for the legacy alias + * - `approved` → approved but not yet sent; operator + * wants a re-review before sending + * - `follow_up` → operator followed up, now wants a + * fresh approval round + * - `problem` → operator flagged a problem, fixed it, + * now wants re-approval + * - `rejected` → legacy terminal state — still reachable + * for quotes that pre-date the new `new` + * bounce-back flow + * + * Does NOT include (left to throw `invalid_status`): + * - `cancelled` → terminal — operator took it out of the + * queue intentionally + * - `claimed` → operator hasn't priced yet; they should + * "Process Items" first + * - `sending` → SMTP in flight; clobbering would orphan + * the outbound send + * - `template` → not a real quote; templates don't go to + * the approval queue + */ +export const RESUBMITTABLE_STATUSES = new Set([ + "emailed", + "complete", + "partial_approved", + "mail_failed", + "email_failed", + "approved", + "follow_up", + "problem", + "rejected", +]); + +export function isResubmittableStatus(status: QuoteStatus | string | undefined | null): boolean { + if (!status) return false; + return RESUBMITTABLE_STATUSES.has(status as QuoteStatus); +} + +/** + * Per-status one-liner for the info variant of the Resubmit banner. + * Keeps the messaging in one place so the page doesn't grow a forest + * of conditionals. Statuses that have no entry fall through to the + * generic `defaultMessage`. The destructive variant (rejection case) + * owns its own copy in the page component — it carries the + * rejection-reason block which these short messages don't. + */ +const RESUBMIT_MESSAGES: Partial> = { + emailed: + "Customer came back with changes — resubmit to send the revised quote through the approval queue.", + complete: + "Quote closed — resubmit to reopen for a fresh approval cycle.", + follow_up: + "Follow-up round complete — resubmit for a fresh approval round.", + problem: + "Problem resolved — resubmit for a fresh approval cycle.", + mail_failed: + "Previous email failed — fix the underlying issue, then resubmit.", + email_failed: + "Previous email failed — fix the underlying issue, then resubmit.", + approved: + "Approved but not yet sent — resubmit to send back through approval before sending.", + partial_approved: + "Partial — resubmit for a fresh full-approval cycle.", + rejected: + "Rejected — fix and resubmit for a fresh approval cycle.", +}; + +const RESUBMIT_DEFAULT_MESSAGE = "Resubmit for a fresh approval cycle."; + +export function resubmitStatusMessage(status: QuoteStatus | string | undefined | null): string { + if (!status) return RESUBMIT_DEFAULT_MESSAGE; + return RESUBMIT_MESSAGES[status as QuoteStatus] ?? RESUBMIT_DEFAULT_MESSAGE; +} diff --git a/src/lib/server/quotes-service.test.ts b/src/lib/server/quotes-service.test.ts index 3961be1..869734b 100644 --- a/src/lib/server/quotes-service.test.ts +++ b/src/lib/server/quotes-service.test.ts @@ -254,6 +254,20 @@ vi.mock("./quote-pdf-service", () => ({ generateQuotePdfMock: vi.fn(), })); +// Route every `getAdminPocketBase()` call inside `quotes-service` +// through the in-memory pbMock above. Without this mock the tests +// fall through to the real PocketBase instance, which requires +// PB_ADMIN_EMAIL / PB_ADMIN_PASSWORD env vars and a live server +// — neither of which the unit suite depends on for correctness. +// All other service test files in this repo follow the same +// pattern (see delivery-notes-service.test.ts, customers-coordinates.test.ts, +// etc.) — this file was missing the mock since the +// "submitQuote" describe block landed, so every submitQuote test +// silently hit real PB and got a "Quote not found" 404. +vi.mock("@/lib/server/pocketbase-admin", () => ({ + getAdminPocketBase: vi.fn().mockResolvedValue(pbMock), +})); + import { generateQuoteNumberServer, findOrCreateCustomer, @@ -2414,15 +2428,16 @@ describe("submitQuote", () => { expect(stored).toEqual(original); }); - it("rejects a quote in any non-new, non-pending status with code: invalid_status", async () => { - for (const status of [ - "cancelled", - "approved", - "emailed", - "rejected", - "partial_approved", - "claimed", - ]) { + it("still rejects cancelled and claimed quotes with code: invalid_status", async () => { + // Terminal / pre-pricing states stay locked out of the + // approval queue: + // - cancelled → operator took it out of the queue intentionally + // - claimed → operator hasn't priced yet; they should + // "Process Items" first + // The `sending` and `template` branches are covered by their + // own tests above; this case is just the two statuses that + // are user-driven terminal-or-prerequisite gates. + for (const status of ["cancelled", "claimed"]) { await pbMock.collection("quotes").create({ id: `q-${status}`, status, @@ -2441,6 +2456,69 @@ describe("submitQuote", () => { } }); + it("accepts every resubmittable status and flips it to approval_pending", async () => { + // The resubmittable set (see RESUBMITTABLE_STATUSES in + // @/lib/quote-status) covers post-send resubmits: customer + // came back with changes on an emailed / completed / approved + // quote, operator fixed a problem or follow-up, mail_failed + // retry, or the legacy `rejected` terminal state. Every one + // of them lands on `approval_pending` — `partial` is + // ignored on re-submissions, only fresh `new` submissions + // can opt into `partial_approval_pending`. + const resubmittable = [ + "emailed", + "complete", + "approved", + "follow_up", + "problem", + "partial_approved", + "mail_failed", + "email_failed", + "rejected", + ]; + for (const status of resubmittable) { + await pbMock.collection("quotes").create({ + id: `q-${status}`, + status, + }); + await pbMock.collection("quote_items").create({ + id: `qi-${status}`, + quote: `q-${status}`, + selling_price: 10, + }); + + // Partial flag is silently swallowed on resubmissions + // (only fresh `new` quotes honour it). Passing it here + // exercises that the resubmission still completes. + const result = await submitQuote(`q-${status}`, undefined, { + partial: true, + }); + expect(result.statusChanged).toBe(true); + expect(result.quote.status).toBe("approval_pending"); + expect(result.targetStatus).toBe("approval_pending"); + } + }); + + it("ignores the partial flag on resubmissions even when caller asks for it", async () => { + // Specifically pin the "partial is swallowed on resubmit" rule + // so a future refactor can't quietly land a partial_approval_pending + // on a quote that's already been through the pipeline once. + await pbMock.collection("quotes").create({ + id: "q-emailed", + status: "emailed", + }); + await pbMock.collection("quote_items").create({ + id: "qi-emailed", + quote: "q-emailed", + selling_price: 10, + }); + + const result = await submitQuote("q-emailed", undefined, { + partial: true, + }); + expect(result.quote.status).toBe("approval_pending"); + }); + it("rejects a quote with unpriced lines and returns the offending item ids", async () => { await pbMock.collection("quotes").create({ id: "q-1", diff --git a/src/lib/server/quotes-service.ts b/src/lib/server/quotes-service.ts index 995ed9e..3e17242 100644 --- a/src/lib/server/quotes-service.ts +++ b/src/lib/server/quotes-service.ts @@ -11,6 +11,7 @@ import { PLACEHOLDER_CUSTOMER_NAMES, } from "@/lib/placeholder-customers"; import type { Quote, QuoteItem, Customer, Company, QuoteStatus } from "@/lib/types"; +import { RESUBMITTABLE_STATUSES } from "@/lib/quote-status"; /** * Server-side quote pipeline used by the MCP layer (n8n) and internal API routes. @@ -2534,21 +2535,49 @@ export async function submitQuote( }; } - // Only `new` quotes can transition via this tool. The full set of - // valid paths to the approval queue is: - // - `new` → here (via `submit`) - // - `partial_approval_pending` → `partial_approved` (via `partial_approve`) - // - `partial_approved` → `approved` (via the operator's review flow) - // Quotes that are already `approved`, `rejected`, `cancelled`, - // `emailed`, etc. have to be re-quoted rather than re-submitted. - if (quote.status !== "new") { + /** + * Re-submittable statuses (added 2026-09-07). Before this set + * existed, `submitQuote` only accepted `new` — once a quote had + * been emailed and the customer came back asking for changes, + * the operator's only path was to re-quote from scratch. That + * throws away the original quote_number, all email tracking, + * and the customer's audit trail. Re-submitting on the same + * quote preserves every one of those — the only thing that + * changes is the PDF (regenerated on next view) and the + * `submitted_at` stamp. + * + * The set itself lives in `@/lib/quote-status` so the client UI + * (banner gate) draws from the same source of truth — a new + * status added there surfaces in both places without a second + * edit. The membership + rationale are documented next to the + * export. + */ + + const isFreshSubmission = quote.status === "new"; + const isResubmission = RESUBMITTABLE_STATUSES.has(quote.status); + + if (!isFreshSubmission && !isResubmission) { throw new SubmitQuoteError( - `Cannot submit a quote in status "${quote.status}". Only "new" quotes can be submitted for approval. Re-quote if the customer needs a fresh approval cycle.`, + `Cannot submit a quote in status "${quote.status}". Only new quotes and post-send re-submissions can go through the approval queue — re-quote for cancelled or claimed quotes.`, 422, "invalid_status", ); } + // Re-submissions always go to full `approval_pending` regardless + // of the `partial` flag — once a quote has been through the + // pipeline once, "submit only some items" no longer makes sense. + // We still honour `partial: true` on a FRESH submission (the + // existing walk-in partial-approval recipe), but ignore it for + // a re-submit. The flag is silently swallowed rather than 422'd + // because the UI button is the same "Resubmit for Approval" + // affordance across both cases — the operator doesn't need to + // think about which mode they're in. + const targetStatus: "approval_pending" | "partial_approval_pending" = + isFreshSubmission && options?.partial + ? "partial_approval_pending" + : "approval_pending"; + // Fetch every line on the quote (small N — typical quotes have // < 30 items) and filter in JS. The PB filter parser mock in // `quotes-service.test.ts` doesn't support `!= null` semantics @@ -2578,6 +2607,13 @@ export async function submitQuote( // via MCP. The `partial: true` flag on the same `submit` tool keeps // the state machine documented in one place — there isn't a parallel // `submit_partial` tool to drift out of sync. + // + // For re-submissions, the gate applies identically — the operator + // might have re-priced an item down to 0 while testing the new + // numbers, and we don't want to send an incomplete quote back to + // the rep. `partial: true` is silently ignored on resubmissions + // (see the targetStatus comment above), so the only way to bypass + // the gate is to actually price the lines. if (unpricedItems.length > 0 && !options?.partial && !quote.is_no_quote) { const offendingItemIds = unpricedItems.map((it) => String(it.id)); throw new SubmitQuoteError( @@ -2588,12 +2624,6 @@ export async function submitQuote( ); } - // Pick the target status. Partial flag wins (operator intent) — - // even if every item happened to be priced, we honour the flag. - // Without the flag and with all items priced: full approval. - const targetStatus: "approval_pending" | "partial_approval_pending" = - options?.partial ? "partial_approval_pending" : "approval_pending"; - // Fluid write: Quote.submitted_at / submitted_by are optional fields // on the Quote type. If/when ops adds them to the PB schema, these // will start persisting. Right now they're silently dropped, which