diff --git a/src/app/api/v1/orders/[id]/review-save/route.ts b/src/app/api/v1/orders/[id]/review-save/route.ts index 0687d2b..35428b3 100644 --- a/src/app/api/v1/orders/[id]/review-save/route.ts +++ b/src/app/api/v1/orders/[id]/review-save/route.ts @@ -99,6 +99,7 @@ export async function POST( rep?: string; items?: Array<{ id: string; + description?: string | null; quantity?: number | null; selling_price?: number | null; cost_price?: number | null; diff --git a/src/components/orders-review-modal.test.tsx b/src/components/orders-review-modal.test.tsx index b6d2e35..878d531 100644 --- a/src/components/orders-review-modal.test.tsx +++ b/src/components/orders-review-modal.test.tsx @@ -450,8 +450,11 @@ describe("OrdersReviewModal", () => { fireEvent.click(screen.getByTestId("review-edit-items-btn")); fireEvent.click(screen.getByTestId("review-edit-confirm-enable")); - // Dialog should be closed and items enabled. - const itemRow = screen.getByText("HSS drill 6mm").closest("li")!; + // Dialog should be closed and items enabled. In edit mode the + // description is now an (data-testid + // `review-item-description`), so locate the row by testid + // rather than by the rendered text. + const itemRow = screen.getByTestId("review-item-row-item-1"); for (const input of Array.from(itemRow.querySelectorAll("input"))) { expect(input).not.toBeDisabled(); } @@ -465,6 +468,52 @@ describe("OrdersReviewModal", () => { ).toBeInTheDocument(); }); + it("editing the persisted item's description dirties the modal and saves via the items endpoint", async () => { + // The review step needs to let the operator correct an + // AI-misread item title. The persisted description is rendered + // as a read-only

until editing is enabled; once enabled + // it becomes an input and the change goes through the same + // dirty-check / save path as a price edit. + const fetchMock = vi + .spyOn(global, "fetch") + .mockResolvedValueOnce( + new Response( + JSON.stringify({ + order: { ...baseOrder, description: "HSS drill 8mm" }, + items: { updated: 1, failed: [] }, + itemsAdded: { created: [], failed: [] }, + itemsRemoved: { softFlagged: [], failed: [] }, + partialFailures: [], + }), + { status: 200 }, + ), + ); + + render(); + fireEvent.click(screen.getByTestId("review-edit-items-btn")); + fireEvent.click(screen.getByTestId("review-edit-confirm-enable")); + + const descInput = screen.getByTestId( + "review-item-description", + ) as HTMLInputElement; + expect(descInput).toBeInTheDocument(); + expect(descInput.value).toBe("HSS drill 6mm"); + fireEvent.change(descInput, { + target: { value: "HSS drill 8mm" }, + }); + + fireEvent.click(screen.getByTestId("review-save-button")); + + await waitFor(() => { + expect(fetchMock).toHaveBeenCalled(); + }); + + const callBody = JSON.parse(fetchMock.mock.calls[0][1]?.body as string); + expect(callBody.items).toEqual([ + { id: "item-1", description: "HSS drill 8mm" }, + ]); + }); + it("canceling the dialog keeps items read-only", () => { render(); fireEvent.click(screen.getByTestId("review-edit-items-btn")); @@ -700,6 +749,82 @@ describe("OrdersReviewModal", () => { }); }); + describe("source of truth email body", () => { + it("collapses the email body by default — the operator opens it on demand", () => { + // The email body is the secondary audit trail, not the primary + // edit surface. Default state: collapsed, so the order items + + // attachments stay at the top of the modal without a giant + // block of email text pushing them down. + render( + , + ); + // Body starts hidden behind the toggle. + expect( + screen.queryByTestId("review-email-body"), + ).not.toBeInTheDocument(); + + // Open it. + fireEvent.click(screen.getByTestId("review-email-toggle")); + const body = screen.getByTestId("review-email-body"); + expect(body).toBeInTheDocument(); + expect(body.textContent).toMatch(/Re: RFQ/); + expect(body.textContent).toMatch(/Hi team\.\.\./); + }); + + it("collapses the email body by default when there are no attachments", () => { + // The empty-attachment path also keeps the email collapsed by + // default — the operator opens it via the same toggle. + render(); + expect( + screen.queryByTestId("review-email-body"), + ).not.toBeInTheDocument(); + + fireEvent.click(screen.getByTestId("review-email-toggle")); + const body = screen.getByTestId("review-email-body"); + expect(body).toBeInTheDocument(); + expect(body.textContent).toMatch(/Hi team\.\.\./); + }); + + it("email body collapse toggle is reversible", () => { + render(); + const toggle = screen.getByTestId("review-email-toggle"); + // Body starts collapsed. + expect( + screen.queryByTestId("review-email-body"), + ).not.toBeInTheDocument(); + + // Expand. + fireEvent.click(toggle); + expect( + screen.getByTestId("review-email-body"), + ).toBeInTheDocument(); + + // Collapse again. + fireEvent.click(toggle); + expect( + screen.queryByTestId("review-email-body"), + ).not.toBeInTheDocument(); + }); + }); + describe("customer cascade (company + rep)", () => { const expandedCustomerA: Customer = { ...baseCustomer, diff --git a/src/components/orders-review-modal.tsx b/src/components/orders-review-modal.tsx index 0b55c48..49563f3 100644 --- a/src/components/orders-review-modal.tsx +++ b/src/components/orders-review-modal.tsx @@ -4,6 +4,8 @@ import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { AlertTriangle, Check, + ChevronDown, + ChevronRight, ExternalLink, Loader2, Mail, @@ -242,15 +244,16 @@ export function OrdersReviewModal({ // none of the price fields changed, the removal has to be // persisted. if (d.pendingRemoval) return true; + if (d.description !== (it.description || "")) return true; if (d.quantity !== it.quantity) return true; if (d.selling_price !== it.selling_price) return true; if (d.cost_price !== it.cost_price) return true; // NOTE: markup is deliberately not compared here. The save body - // only carries quantity/selling_price/cost_price; the service - // layer recomputes markup server-side from the selling/cost pair. - // At the review step the operator doesn't edit markup directly, - // but onSellingChange recomputes cost from selling+markup, so a - // cost change here IS captured above. + // only carries quantity/selling_price/cost_price/description; the + // service layer recomputes markup server-side from the + // selling/cost pair. At the review step the operator doesn't edit + // markup directly, but onSellingChange recomputes cost from + // selling+markup, so a cost change here IS captured above. } // Any new draft row (one with no matching persistedItem) is dirty // by default — it must be POSTed to the create endpoint. @@ -388,6 +391,7 @@ export function OrdersReviewModal({ rep?: string; items?: Array<{ id: string; + description?: string | null; quantity?: number | null; selling_price?: number | null; cost_price?: number | null; @@ -435,11 +439,16 @@ export function OrdersReviewModal({ if (d.pendingRemoval) return null; const changed: { id: string; + description?: string | null; quantity?: number | null; selling_price?: number | null; cost_price?: number | null; } = { id: it.id }; let isDirty = false; + if (d.description !== (it.description || "")) { + changed.description = d.description; + isDirty = true; + } if (d.quantity !== it.quantity) { changed.quantity = d.quantity; isDirty = true; @@ -857,14 +866,30 @@ export function OrdersReviewModal({ >

-

- {it.description || "(no description)"} -

+ {editingItems ? ( + + onDescriptionChange(it.id, e.target.value) + } + placeholder="Item description" + disabled={saving || d.pendingRemoval} + data-testid="review-item-description" + className={cn( + "h-8 text-xs", + d.pendingRemoval && "line-through", + )} + /> + ) : ( +

+ {d.description || it.description || "(no description)"} +

+ )}
{editingItems && ( d.pendingRemoval ? ( @@ -1181,9 +1206,11 @@ function FailureSection({ } // --- Source-of-truth panel ----------------------------------------------- -// Inline preview of the source attachment so the operator can scan -// the actual PDF/image against the order items in one screen. -// Falls back to the email body when no attachments exist. +// Always renders BOTH the source email body AND the source attachments +// (if any). The email body is the audit trail the customer sent the +// PO from; it can be long, so it's collapsible. Attachments preview +// inline. When there are no attachments the email body sits at the +// top so the operator can scan it before checking the items. function SourceOfTruth({ quoteEmail, attachments, @@ -1191,9 +1218,13 @@ function SourceOfTruth({ quoteEmail: { subject: string; body: string }; attachments: QuoteAttachment[]; }) { - if (attachments.length === 0) { - return ; - } + // Email body is always rendered; the operator can expand it when + // they want to cross-check the customer's wording. Default: collapsed + // — the email is the secondary audit trail, not the primary edit + // surface. Even when expanded, the body scrolls inside a short + // container so it doesn't push the order items / attachments off + // screen. + const [emailOpen, setEmailOpen] = useState(false); // Pick the first PDF if any, else first image, else the first // entry of any other kind (which gets the "no preview" treatment). @@ -1209,124 +1240,149 @@ function SourceOfTruth({ ); return ( -
-
- - Attachments ({attachments.length}) +
+ {/* Email body — ALWAYS visible (collapsible). Audit trail of + the customer's request: the operator reads it when an AI + field looks wrong. Long quotes get a collapse toggle so the + preview pane stays usable. */} +
+ + {emailOpen && ( +
+

+ {quoteEmail.subject || "(no subject)"} +

+ {/* Short scroll container so even long emails don't push + the operator's real work (order items + attachments) + down the page. The toggle above lets them skip the + email entirely when they don't need it. */} + 
+              {quoteEmail.body || "(no body)"}
+
+
+ )}
- {/* Tab strip — DECORATIVE in v1 (no click handler, no activeId state). - Only the first previewable attachment (PDF > image) is rendered below; - the chips are visual context. Future v2 could wire them with - `useState` inside `SourceOfTruth` to swap the preview. */} - {attachments.length > 1 && ( -
- {attachments.map((a) => { - const isActive = previewTarget && a.id === previewTarget.id; - const previewable = a.kind === "pdf" || a.kind === "image"; - return ( - 0 && ( +
+
+ + Attachments ({attachments.length}) +
+ + {/* Tab strip — DECORATIVE in v1 (no click handler, no activeId state). + Only the first previewable attachment (PDF > image) is rendered below; + the chips are visual context. Future v2 could wire them with + `useState` inside `SourceOfTruth` to swap the preview. */} + {attachments.length > 1 && ( +
+ {attachments.map((a) => { + const isActive = previewTarget && a.id === previewTarget.id; + const previewable = a.kind === "pdf" || a.kind === "image"; + return ( + + {previewable ? "●" : "○"} {a.filename} + + ); + })} +
+ )} + + {previewTarget && previewTarget.kind === "pdf" && ( +