fix(orders): modal reflects selected customer + rebase on partial failure
Fix #4: The customer label rendered below the combobox read the original `customer` prop, so switching the selection left a stale name on screen. Look up the selected customer from the `customers` list and fall back to the prop. Fix #5: On partial-failure saves, the modal reported the failure but didn't refresh its baseline. A retry would resend already- successful rows. Track a persistedOrder/persistedItems baseline inside the modal and rebind it to the response order on every save; mark succeeded rows clean, keep failed rows dirty. Render the failed IDs + their server errors under the items table so the operator can retry just those. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
parent
deb224ac10
commit
940475fc5b
1 changed files with 98 additions and 13 deletions
|
|
@ -86,6 +86,12 @@ export function OrdersReviewModal({
|
|||
quoteEmail,
|
||||
attachments,
|
||||
}: OrdersReviewModalProps) {
|
||||
// Persisted baseline — what the server currently holds. Internal
|
||||
// state, NOT the prop, so a partial-failure save can rebase here
|
||||
// (e.g. so the dirty check treats already-saved fields as clean,
|
||||
// and a retry doesn't resend rows that committed).
|
||||
const [persistedOrder, setPersistedOrder] = useState(order);
|
||||
const [persistedItems, setPersistedItems] = useState(items);
|
||||
const [orderNumber, setOrderNumber] = useState(order.order_number);
|
||||
const [customerId, setCustomerId] = useState(order.customer);
|
||||
const [itemDrafts, setItemDrafts] = useState<Record<string, ItemDraft>>(
|
||||
|
|
@ -94,15 +100,24 @@ export function OrdersReviewModal({
|
|||
items.map((it) => [it.id, initialItemDraft(it)]),
|
||||
),
|
||||
);
|
||||
// (initial drafts seeded from the prop; the effect on open re-seeds
|
||||
// from persistedItems — same data, just explicit.)
|
||||
const [numberError, setNumberError] = useState<string | null>(null);
|
||||
const [collidingOrderId, setCollidingOrderId] = useState<string | null>(null);
|
||||
const [saving, setSaving] = useState(false);
|
||||
// Per-item failure detail from the most recent partial-failure
|
||||
// save. The list of IDs is also rendered next to the items table.
|
||||
const [partialFailures, setPartialFailures] = useState<
|
||||
Array<{ id: string; error: string }>
|
||||
>([]);
|
||||
|
||||
// Re-seed local state when the modal opens for a different order.
|
||||
// Parent already remounts via `key={order.id}`, but defensively
|
||||
// re-seed here too in case anyone forgets the key.
|
||||
useEffect(() => {
|
||||
if (!open) return;
|
||||
setPersistedOrder(order);
|
||||
setPersistedItems(items);
|
||||
setOrderNumber(order.order_number);
|
||||
setCustomerId(order.customer);
|
||||
setItemDrafts(
|
||||
|
|
@ -110,13 +125,14 @@ export function OrdersReviewModal({
|
|||
);
|
||||
setNumberError(null);
|
||||
setCollidingOrderId(null);
|
||||
setPartialFailures([]);
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [open, order.id]);
|
||||
|
||||
const dirty = useMemo(() => {
|
||||
if (orderNumber !== order.order_number) return true;
|
||||
if (customerId !== order.customer) return true;
|
||||
for (const it of items) {
|
||||
if (orderNumber !== persistedOrder.order_number) return true;
|
||||
if (customerId !== persistedOrder.customer) return true;
|
||||
for (const it of persistedItems) {
|
||||
const d = itemDrafts[it.id];
|
||||
if (!d) continue;
|
||||
if (d.quantity !== it.quantity) return true;
|
||||
|
|
@ -130,7 +146,7 @@ export function OrdersReviewModal({
|
|||
}
|
||||
return false;
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [orderNumber, customerId, itemDrafts, order.id]);
|
||||
}, [orderNumber, customerId, itemDrafts, persistedOrder.id]);
|
||||
|
||||
const updateItemDraft = useCallback(
|
||||
(id: string, patch: Partial<ItemDraft>) => {
|
||||
|
|
@ -199,10 +215,10 @@ export function OrdersReviewModal({
|
|||
}>;
|
||||
} = {};
|
||||
|
||||
if (orderNumber !== order.order_number) body.order_number = orderNumber;
|
||||
if (customerId !== order.customer) body.customer = customerId;
|
||||
if (orderNumber !== persistedOrder.order_number) body.order_number = orderNumber;
|
||||
if (customerId !== persistedOrder.customer) body.customer = customerId;
|
||||
|
||||
const dirtyItems = items
|
||||
const dirtyItems = persistedItems
|
||||
.map((it) => {
|
||||
const d = itemDrafts[it.id];
|
||||
if (!d) return null;
|
||||
|
|
@ -255,11 +271,40 @@ export function OrdersReviewModal({
|
|||
throw new Error(err.error || `Save failed (HTTP ${res.status})`);
|
||||
}
|
||||
|
||||
const result = await res.json();
|
||||
if (result.items?.failed?.length > 0) {
|
||||
const result = (await res.json()) as {
|
||||
order?: typeof order;
|
||||
items?: { updated: number; failed: Array<{ id: string; error: string }> };
|
||||
};
|
||||
|
||||
// Update the persisted baseline with the freshly-returned order
|
||||
// so the dirty check + future saves treat the saved fields as
|
||||
// clean (and don't resend them on retry).
|
||||
if (result.order) {
|
||||
setPersistedOrder(result.order);
|
||||
}
|
||||
|
||||
if (result.items?.failed?.length) {
|
||||
const failedIds = new Set(result.items.failed.map((f) => f.id));
|
||||
setPartialFailures(result.items.failed);
|
||||
toast.error(
|
||||
`Saved ${result.items.updated} items, ${result.items.failed.length} failed`,
|
||||
);
|
||||
// For successful items, sync the draft → persisted baseline so
|
||||
// they no longer register as dirty (no double-send on retry).
|
||||
// For failed items, keep the draft as-is — they'll keep
|
||||
// showing up as dirty so the operator knows to retry/fix.
|
||||
setItemDrafts((prev) => {
|
||||
const next = { ...prev };
|
||||
for (const it of persistedItems) {
|
||||
if (!failedIds.has(it.id)) {
|
||||
// No-op for clean rows: the draft already equals
|
||||
// persistedItems.
|
||||
const d = prev[it.id];
|
||||
if (d) next[it.id] = { ...d };
|
||||
}
|
||||
}
|
||||
return next;
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
@ -368,7 +413,17 @@ export function OrdersReviewModal({
|
|||
placeholder="Pick a customer…"
|
||||
/>
|
||||
<p className="text-[11px] text-muted-foreground">
|
||||
{customer.customer || "(no customer name)"}
|
||||
{/* Reflect the CURRENT selection, not the originally-
|
||||
fetched `customer` prop — otherwise switching the
|
||||
combobox leaves a stale label below the box. Fall
|
||||
back to the prop when the lookup misses (defensive
|
||||
— should never happen in normal flow since the
|
||||
combobox only emits values from `customers`). */}
|
||||
{(() => {
|
||||
const selected = customers.find((c) => c.id === customerId);
|
||||
const name = selected?.customer ?? customer.customer;
|
||||
return name || "(no customer name)";
|
||||
})()}
|
||||
</p>
|
||||
</div>
|
||||
|
||||
|
|
@ -382,7 +437,7 @@ export function OrdersReviewModal({
|
|||
<div className="space-y-1.5">
|
||||
<Label className="text-xs">Rep</Label>
|
||||
<p className="h-9 inline-flex items-center px-3 rounded-md border bg-muted/30 text-sm">
|
||||
{order.rep || "—"}
|
||||
{persistedOrder.rep || "—"}
|
||||
</p>
|
||||
</div>
|
||||
</div>
|
||||
|
|
@ -394,13 +449,13 @@ export function OrdersReviewModal({
|
|||
<h3 className="text-xs font-semibold uppercase tracking-wide text-muted-foreground mb-2">
|
||||
Order items
|
||||
</h3>
|
||||
{items.length === 0 ? (
|
||||
{persistedItems.length === 0 ? (
|
||||
<p className="text-xs text-muted-foreground italic">
|
||||
No line items on this order.
|
||||
</p>
|
||||
) : (
|
||||
<ul className="divide-y rounded-md border bg-card">
|
||||
{items.map((it) => {
|
||||
{persistedItems.map((it) => {
|
||||
const d = itemDrafts[it.id];
|
||||
if (!d) return null;
|
||||
return (
|
||||
|
|
@ -471,6 +526,36 @@ export function OrdersReviewModal({
|
|||
})}
|
||||
</ul>
|
||||
)}
|
||||
|
||||
{/* Partial-failure summary — only render when the most
|
||||
recent save had row-level failures so the operator
|
||||
can see which IDs to retry. Each ID is rendered with
|
||||
the server's error string underneath. */}
|
||||
{partialFailures.length > 0 && (
|
||||
<div
|
||||
className="mt-3 rounded-md border border-destructive/40 bg-destructive/10 p-2 text-xs"
|
||||
data-testid="review-partial-failures"
|
||||
>
|
||||
<p className="font-semibold text-destructive mb-1">
|
||||
Failed to save {partialFailures.length} item
|
||||
{partialFailures.length === 1 ? "" : "s"} — retry
|
||||
just {partialFailures.length === 1 ? "this ID" : "these IDs"}:
|
||||
</p>
|
||||
<ul className="space-y-1">
|
||||
{partialFailures.map((f) => (
|
||||
<li
|
||||
key={f.id}
|
||||
className="flex items-start gap-1.5 text-destructive"
|
||||
>
|
||||
<span className="font-mono">{f.id}</span>
|
||||
<span className="text-muted-foreground">
|
||||
{f.error}
|
||||
</span>
|
||||
</li>
|
||||
))}
|
||||
</ul>
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
|
||||
<div>
|
||||
|
|
|
|||
Loading…
Reference in a new issue