feat(order-review): edit item description, show email body, fix duplicate key

Three review-modal polish items rolled up:

1. Persisted line items now expose a description input in edit mode
   (mirroring the existing qty / selling inputs). The save route +
   the orders service's `updateOrderItemsPrices` accept the new
   `description` field and write `description` + `po_description`
   together (rejecting a blank description per row, with a
   per-row failure entry so the operator can retry just those).

2. The source-of-truth panel now ALWAYS renders the email body, not
   only when there are no attachments. The body is collapsed by
   default and capped at `max-h-40` so the email stays a secondary
   audit block instead of pushing the operator's real work
   (line items + attachments) down the page. The toggle button is
   unchanged.

3. The chip strip and file list inside the source-of-truth panel
   were using `key={a.id}`. `orderAttachementsToQuotes` synthesizes
   every attachment with `id: order.id` (PB file URLs need
   `/files/{collectionId}/{recordId}/{filename}`), so a multi-file
   order was throwing 'Encountered two children with the same key'.
   Switched to `key={a.filename}` — unique per file in the
   multi-file field, leaves the file URL untouched.

The Order type grew `email_subject`, `email_sender`,
`email_body`, and `email_message_id`. These columns exist in the
live PB (the local `pb_schema-2.json` snapshot is reference-only
per AGENTS.md, so the drift is expected). `getOrderReviewData`
prefers the order's own `email_body` and falls back to the
related `quote.email_body` for legacy orders that pre-date the
snapshot columns — quoting the operator's outbound is better than
showing '(no body)'.

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
SJCoetz 2026-08-04 08:51:22 +02:00
parent 69175e493d
commit f819478bc0
4 changed files with 347 additions and 134 deletions

View file

@ -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;

View file

@ -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 <input> (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 <p> 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(<OrdersReviewModal {...baseProps()} />);
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(<OrdersReviewModal {...baseProps()} />);
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(
<OrdersReviewModal
{...baseProps()}
attachments={[
{
id: "att-pdf",
created: "",
updated: "",
collectionId: "coll-1",
collectionName: "quote_attachments",
quote: "quote-1",
filename: "Acme-RFQ.pdf",
file: ["pdf-token"],
content_type: "application/pdf",
size_bytes: 1000,
email_message_id: "",
kind: "pdf",
},
]}
/>,
);
// 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(<OrdersReviewModal {...baseProps()} />);
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(<OrdersReviewModal {...baseProps()} />);
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,

View file

@ -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({
>
<div className="flex items-start gap-2">
<div className="min-w-0 flex-1">
<p
className={cn(
"text-sm font-medium truncate",
d.pendingRemoval && "line-through opacity-60",
)}
>
{it.description || "(no description)"}
</p>
{editingItems ? (
<Input
value={d.description}
onChange={(e) =>
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",
)}
/>
) : (
<p
className={cn(
"text-sm font-medium truncate",
d.pendingRemoval && "line-through opacity-60",
)}
>
{d.description || it.description || "(no description)"}
</p>
)}
</div>
{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 <EmailFallback quoteEmail={quoteEmail} />;
}
// 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 (
<div className="rounded-md border bg-muted/30 text-xs space-y-3">
<div className="px-3 pt-3 flex items-center gap-1.5 font-semibold uppercase tracking-wide text-muted-foreground">
<Paperclip className="h-3 w-3" />
Attachments ({attachments.length})
<div className="rounded-md border bg-muted/30 text-xs space-y-3" data-testid="review-source-of-truth">
{/* 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. */}
<div className="border-b border-border/40">
<button
type="button"
onClick={() => setEmailOpen((open) => !open)}
aria-expanded={emailOpen}
aria-controls="review-email-body"
data-testid="review-email-toggle"
className="w-full px-3 pt-3 pb-2 flex items-center gap-1.5 text-left font-semibold uppercase tracking-wide text-muted-foreground hover:text-foreground"
>
{emailOpen ? (
<ChevronDown className="h-3 w-3" />
) : (
<ChevronRight className="h-3 w-3" />
)}
<Mail className="h-3 w-3" />
Email
</button>
{emailOpen && (
<div
id="review-email-body"
className="px-3 pb-3 space-y-1"
data-testid="review-email-body"
>
<p className="text-foreground font-medium break-words">
{quoteEmail.subject || "(no subject)"}
</p>
{/* 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. */}
<pre className="whitespace-pre-wrap font-sans text-muted-foreground max-h-40 overflow-y-auto text-[11px] leading-snug">
{quoteEmail.body || "(no body)"}
</pre>
</div>
)}
</div>
{/* 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<activeId>` inside `SourceOfTruth` to swap the preview. */}
{attachments.length > 1 && (
<div className="px-3 flex flex-wrap gap-1.5 border-b border-border/40 pb-2">
{attachments.map((a) => {
const isActive = previewTarget && a.id === previewTarget.id;
const previewable = a.kind === "pdf" || a.kind === "image";
return (
<span
key={a.id}
className={cn(
"inline-flex items-center gap-1 rounded-full px-2 py-0.5 leading-5 text-[10px]",
isActive
? "bg-primary/15 text-primary font-medium"
: "text-muted-foreground",
)}
title={`${a.filename} (${a.kind ?? "unknown"})`}
{/* Attachments — only render the section when there is at
least one. Without this, the "Attachments (0)" header is
noisy on attachment-less orders. */}
{attachments.length > 0 && (
<div className="px-3 pb-3 space-y-3">
<div className="flex items-center gap-1.5 font-semibold uppercase tracking-wide text-muted-foreground">
<Paperclip className="h-3 w-3" />
Attachments ({attachments.length})
</div>
{/* 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<activeId>` inside `SourceOfTruth` to swap the preview. */}
{attachments.length > 1 && (
<div className="flex flex-wrap gap-1.5 border-b border-border/40 pb-2">
{attachments.map((a) => {
const isActive = previewTarget && a.id === previewTarget.id;
const previewable = a.kind === "pdf" || a.kind === "image";
return (
<span
key={a.filename}
className={cn(
"inline-flex items-center gap-1 rounded-full px-2 py-0.5 leading-5 text-[10px]",
isActive
? "bg-primary/15 text-primary font-medium"
: "text-muted-foreground",
)}
title={`${a.filename} (${a.kind ?? "unknown"})`}
>
{previewable ? "●" : "○"} {a.filename}
</span>
);
})}
</div>
)}
{previewTarget && previewTarget.kind === "pdf" && (
<iframe
src={attachmentUrl(previewTarget)}
title={previewTarget.filename}
className="w-full h-[420px] bg-background border rounded"
data-testid="review-attachment-iframe"
/>
)}
{previewTarget && previewTarget.kind === "image" && (
<div>
{/* eslint-disable-next-line @next/next/no-img-element */}
<img
src={attachmentUrl(previewTarget)}
alt={previewTarget.filename}
className="max-w-full max-h-[420px] rounded border bg-background"
data-testid="review-attachment-img"
/>
</div>
)}
{/* No previewable attachment at all — show a no-preview message. */}
{!previewTarget && firstOther && (
<div className="space-y-1">
<p
className="text-muted-foreground italic"
data-testid="review-no-preview"
>
{previewable ? "●" : "○"} {a.filename}
</span>
);
})}
{`No preview available for this ${firstOther.kind ?? "file"} type.`}
</p>
</div>
)}
{othersAreUnpreviewable && previewTarget && (
<p className="text-[10px] text-muted-foreground">
+ {attachments.filter(
(a) => a.kind !== "pdf" && a.kind !== "image",
).length}{" "}
unpreviewable attachment(s) — open to view.
</p>
)}
<ul className="space-y-1">
{attachments.map((a) => (
<li
key={a.filename}
className="flex items-center justify-between gap-2 text-[11px]"
>
<span className="truncate text-muted-foreground">
{a.filename}
</span>
<a
href={attachmentUrl(a)}
target="_blank"
rel="noopener noreferrer"
className="text-primary hover:underline inline-flex items-center gap-0.5"
>
open <ExternalLink className="h-2.5 w-2.5" />
</a>
</li>
))}
</ul>
</div>
)}
{previewTarget && previewTarget.kind === "pdf" && (
<iframe
src={attachmentUrl(previewTarget)}
title={previewTarget.filename}
className="w-full h-[420px] bg-background border-t"
data-testid="review-attachment-iframe"
/>
)}
{previewTarget && previewTarget.kind === "image" && (
<div className="px-3 pb-3">
{/* eslint-disable-next-line @next/next/no-img-element */}
<img
src={attachmentUrl(previewTarget)}
alt={previewTarget.filename}
className="max-w-full max-h-[420px] rounded border bg-background"
data-testid="review-attachment-img"
/>
</div>
)}
{/* No previewable attachment at all — show a no-preview message. */}
{!previewTarget && firstOther && (
<div className="px-3 pb-3 space-y-1">
<p
className="text-muted-foreground italic"
data-testid="review-no-preview"
>
{`No preview available for this ${firstOther.kind ?? "file"} type.`}
</p>
</div>
)}
{othersAreUnpreviewable && previewTarget && (
<div className="px-3 pb-3">
<p className="text-[10px] text-muted-foreground">
+ {attachments.filter(
(a) => a.kind !== "pdf" && a.kind !== "image",
).length}{" "}
unpreviewable attachment(s) — open to view.
</p>
</div>
)}
<ul className="px-3 pb-3 space-y-1">
{attachments.map((a) => (
<li
key={a.id}
className="flex items-center justify-between gap-2 text-[11px]"
>
<span className="truncate text-muted-foreground">
{a.filename}
</span>
<a
href={attachmentUrl(a)}
target="_blank"
rel="noopener noreferrer"
className="text-primary hover:underline inline-flex items-center gap-0.5"
>
open <ExternalLink className="h-2.5 w-2.5" />
</a>
</li>
))}
</ul>
</div>
);
}
function EmailFallback({
quoteEmail,
}: {
quoteEmail: { subject: string; body: string };
}) {
return (
<div className="rounded-md border bg-muted/30 p-3 text-xs space-y-2">
<div className="flex items-center gap-1.5 font-semibold uppercase tracking-wide text-muted-foreground">
<Mail className="h-3 w-3" />
Email
</div>
<p className="text-foreground font-medium">
{quoteEmail.subject || "(no subject)"}
</p>
<pre className="whitespace-pre-wrap font-sans text-muted-foreground max-h-[420px] overflow-y-auto">
{quoteEmail.body || "(no body)"}
</pre>
</div>
);
}

View file

@ -673,6 +673,37 @@ export interface Order extends BaseRecord {
* — keep it in sync with the live schema.
*/
order_attachements?: string[];
/**
* Subject line of the customer email that originated this order.
* Populated by the n8n agent on order creation from the
* `orders.email_subject` PB column (added to the live schema
* after the local `pb_schema-2.json` snapshot was taken — the
* snapshot is reference-only per AGENTS.md, so the drift is
* expected).
*/
email_subject?: string;
/**
* Sender (From:) of the customer email that originated this order.
* Lives on the order itself so the review modal can show the
* sender without joining to the related quote.
*/
email_sender?: string;
/**
* Plain-text body of the customer email that originated this order.
* Distinct from `quotes.email_body` (which is the operator's
* outbound email body for the related quote) — this is the
* inbound email the customer sent with their RFQ, snapshotted
* onto the order so the review modal can display the source of
* truth without depending on the quote link.
*/
email_body?: string;
/**
* RFC 5322 Message-ID of the customer email that originated this
* order. Mirrored from the inbound reply tracking on
* `quote_replies` for orders that flowed through the RFQ
* monitoring pipeline; null for orders created without one.
*/
email_message_id?: string;
expand?: {
company?: Company;
customer?: Customer;