From 6d9b6a8274732e7751b4a49f7d67e923db0a6573 Mon Sep 17 00:00:00 2001
From: Dave <david.els17@gmail.com>
Date: Wed, 26 Aug 2026 23:47:27 +0200
Subject: [PATCH] fix(quotes): address UI toggle code review

Addresses the 2 IMPORTANT and 4 NIT findings from the Task 9
code-quality review:

- Initial state bug: useState read 'quote' on first render when it
  was null, sealing isNoQuote as false. A previously-persisted
  is_no_quote: true quote would show the toggle as off. Replace
  with a useEffect that syncs once per loaded quote id.

- Dark mode: add dark: variants to the amber card border/background
  to match the project precedent (collection-pool/components.tsx:175).

- Label size: drop from text-sm to text-xs for consistency with
  every other <Label> in the file.

- Error surface: parse the PATCH 400 body and surface data.error
  rather than a generic toast, matching the lookupItem/generatePdf
  pattern in the same file.

- useCallback deps: stop recreating the callback on every toggle
  by stashing the current value in a ref.

- quote-notes-card id: add a comment so the wrapper div's purpose
  is clear.
---
 src/app/(app)/quotes/[id]/process/page.tsx | 38 +++++++++++++++++-----
 1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/src/app/(app)/quotes/[id]/process/page.tsx b/src/app/(app)/quotes/[id]/process/page.tsx
index 803fb76..b473296 100644
--- a/src/app/(app)/quotes/[id]/process/page.tsx
+++ b/src/app/(app)/quotes/[id]/process/page.tsx
@@ -1,6 +1,6 @@
 "use client";
 
-import { useState, useEffect, useCallback, useMemo, Fragment } from "react";
+import { useState, useEffect, useCallback, useMemo, useRef, Fragment } from "react";
 import { use } from "react";
 import { useRouter } from "next/navigation";
 import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card";
@@ -146,7 +146,8 @@ export default function QuoteProcessPage({ params }: { params: Promise<{ id: str
    * in the same flow (the PDF route bypasses pricing gates when this
    * flag is set — see Task 7).
    */
-  const [isNoQuote, setIsNoQuote] = useState(!!quote?.is_no_quote);
+  const [isNoQuote, setIsNoQuote] = useState(false);
+  const [noQuoteSyncedFor, setNoQuoteSyncedFor] = useState<string | null>(null);
   const [noQuoteSaving, setNoQuoteSaving] = useState(false);
 
   /**
@@ -532,6 +533,22 @@ export default function QuoteProcessPage({ params }: { params: Promise<{ id: str
 
   useEffect(() => { fetchData(); }, [fetchData]);
 
+  /**
+   * Sync the `isNoQuote` toggle once per loaded quote id. The local
+   * state is otherwise sealed at mount time, when `quote` is still
+   * null (useState only reads its initial value on the first render),
+   * so a previously-persisted `is_no_quote: true` quote would render
+   * the toggle as off. Tracking the last synced id protects
+   * optimistic toggles from being clobbered by unrelated `fetchData()`
+   * refreshes once the initial sync is in place.
+   */
+  useEffect(() => {
+    if (quote && quote.id !== noQuoteSyncedFor) {
+      setIsNoQuote(!!quote.is_no_quote);
+      setNoQuoteSyncedFor(quote.id);
+    }
+  }, [quote, noQuoteSyncedFor]);
+
   /**
    * Items to render in the table. Auto-sort: incomplete first, done
    * last. Original row number preserved via `originalIdx` so the
@@ -761,9 +778,12 @@ export default function QuoteProcessPage({ params }: { params: Promise<{ id: str
    * When flipped on (true), scrolls the `quote_notes` editor into view
    * so the operator can type the reason in the same flow.
    */
+  const isNoQuoteRef = useRef(false);
+  isNoQuoteRef.current = isNoQuote;
+
   const onToggleNoQuote = useCallback(async (next: boolean) => {
     if (!quote) return;
-    const previous = isNoQuote;
+    const previous = isNoQuoteRef.current;
     setIsNoQuote(next); // optimistic
     setNoQuoteSaving(true);
     try {
@@ -774,7 +794,8 @@ export default function QuoteProcessPage({ params }: { params: Promise<{ id: str
       });
       if (!res.ok) {
         setIsNoQuote(previous); // rollback
-        toast.error("Couldn't update — try again.");
+        const data = (await res.json().catch(() => null)) as { error?: string } | null;
+        toast.error(data?.error || "Couldn't update — try again.");
       } else if (next) {
         // Scroll the quote_notes editor into view so the operator can
         // type the reason immediately.
@@ -788,7 +809,7 @@ export default function QuoteProcessPage({ params }: { params: Promise<{ id: str
     } finally {
       setNoQuoteSaving(false);
     }
-  }, [isNoQuote, quote]);
+  }, [quote]);
 
   /**
    * Swap the customer on this quote. Called by the `<CustomerSearch>`
@@ -937,12 +958,12 @@ export default function QuoteProcessPage({ params }: { params: Promise<{ id: str
               className={cn(
                 "mt-3 flex items-center justify-between rounded-md border p-3",
                 isNoQuote
-                  ? "border-amber-200 bg-amber-50"
-                  : "border-transparent bg-transparent",
+                  ? "border-amber-200 bg-amber-50 dark:border-amber-500/30 dark:bg-amber-500/10"
+                  : "border-transparent bg-transparent"
               )}
             >
               <div className="space-y-0.5">
-                <Label htmlFor="is_no_quote" className="text-sm font-medium">
+                <Label htmlFor="is_no_quote" className="text-xs font-medium">
                   No-quote letter
                 </Label>
                 <p className="text-xs text-muted-foreground">
@@ -1600,6 +1621,7 @@ export default function QuoteProcessPage({ params }: { params: Promise<{ id: str
           generate in one continuous flow. The wrapping div carries the
           `quote-notes-card` id so the no-quote toggle can scrollIntoView
           here when the operator flips the toggle on. */}
+      {/* scroll target for the no-quote toggle */}
       <div id="quote-notes-card">
         <QuoteNoteCard
           quoteId={quote.id}