From abdc3a4d9fe515f273367fa3682698cdd52b55fb Mon Sep 17 00:00:00 2001 From: SJCoetz Date: Thu, 6 Aug 2026 18:06:22 +0200 Subject: [PATCH] fix(send-po-modal): preserve previous message body on resend (don't overwrite with default template) Co-authored-by: Orca --- .../buyer-tasks/buyer-tasks-page.test.tsx | 10 -- .../purchase-orders/send-po-modal.tsx | 117 ++++++++++-------- 2 files changed, 62 insertions(+), 65 deletions(-) diff --git a/src/components/buyer-tasks/buyer-tasks-page.test.tsx b/src/components/buyer-tasks/buyer-tasks-page.test.tsx index 365ed4d..03b6132 100644 --- a/src/components/buyer-tasks/buyer-tasks-page.test.tsx +++ b/src/components/buyer-tasks/buyer-tasks-page.test.tsx @@ -2,16 +2,6 @@ import { describe, it, expect, beforeEach, vi } from "vitest"; import { render, screen, fireEvent, waitFor } from "@testing-library/react"; import { BuyerTasksPage } from "./buyer-tasks-page"; -vi.mock("@/lib/pocketbase", () => ({ - default: () => ({ - collection: () => ({ - // Keep the line-count lookup pending so this test observes the - // synchronous resend prefill rather than the async default-body preview. - getFullList: () => new Promise(() => {}), - }), - }), -})); - const sampleGroup = { groupId: "order-1::sup-1", orderId: "order-1", diff --git a/src/components/purchase-orders/send-po-modal.tsx b/src/components/purchase-orders/send-po-modal.tsx index 023004c..14d817e 100644 --- a/src/components/purchase-orders/send-po-modal.tsx +++ b/src/components/purchase-orders/send-po-modal.tsx @@ -92,61 +92,68 @@ export function SendPoModal({ open, purchaseOrder, onClose, onSent }: Props) { setSubject(`Purchase Order ${po.po_number}`); setMessage(""); } - setMessageLoading(true); - (async () => { - try { - const poItems = await getPocketBase() - .collection("order_items") - .getFullList({ - filter: `purchase_order = "${po.id}"`, - fields: "id", - }); - if (cancelled) return; - // Seed the message textarea with the supplier-personalised default. - // We deliberately pass ONLY the supplier-facing context (PO - // number, supplier name, line count). End-customer context is - // not surfaced here even if we have it — see the confidentiality - // note in `po-email-template.ts`. Operators wanting to add - // specifics can still type over the textarea before sending. - // - // The textarea is plain-text (operator can edit freely), so we - // seed it with the plain-text form of the default body. The - // route converts it to HTML at send time via the operator's - // current text — operators who want to preserve the rich - // formatting should not edit it; operators who want to type - // their own message get a clean single-paragraph wrap. - setMessage( - buildDefaultPoEmailBody({ - poNumber: po.po_number || po.id, - supplierName: po.expand?.supplier?.suppliername || "", - lineCount: poItems.length, - }).text - ); - } catch { - if (cancelled) return; - // Fall back to a generic body if the count lookup fails. The - // route still uses its own (correct) count at send time, so - // the worst case is the operator sees a slightly misleading - // preview — not a wrong email. - setMessage( - buildDefaultPoEmailBody({ - poNumber: po.po_number || po.id, - supplierName: po.expand?.supplier?.suppliername || "", - lineCount: 0, - }).text - ); - toast.error("Couldn't load line count for preview; check the PO before sending."); - } finally { - if (cancelled) return; - setMessageLoading(false); - // Seed the attachments list from the PO record that just opened - // the modal. PB returns a multi-file field as either an array - // or (for legacy single-file rows) a bare string — normalise - // to string[] so the rest of the modal can treat it uniformly. - const rawAtt = po.additional_attachments ?? []; - setAttachments(Array.isArray(rawAtt) ? rawAtt : rawAtt ? [rawAtt] : []); - } - })(); + // Phase 2: async default-body seed. SKIPPED on the resend flow — the + // operator's previous bodyText is already prefilled in phase 1 above + // and we mustn't clobber it with the generic template once the fetch + // resolves. Phase 2 also gates messageLoading so the textarea doesn't + // grey out mid-edit on resend. + if (!isResendFlow) { + setMessageLoading(true); + (async () => { + try { + const poItems = await getPocketBase() + .collection("order_items") + .getFullList({ + filter: `purchase_order = "${po.id}"`, + fields: "id", + }); + if (cancelled) return; + // Seed the message textarea with the supplier-personalised default. + // We deliberately pass ONLY the supplier-facing context (PO + // number, supplier name, line count). End-customer context is + // not surfaced here even if we have it — see the confidentiality + // note in `po-email-template.ts`. Operators wanting to add + // specifics can still type over the textarea before sending. + // + // The textarea is plain-text (operator can edit freely), so we + // seed it with the plain-text form of the default body. The + // route converts it to HTML at send time via the operator's + // current text — operators who want to preserve the rich + // formatting should not edit it; operators who want to type + // their own message get a clean single-paragraph wrap. + setMessage( + buildDefaultPoEmailBody({ + poNumber: po.po_number || po.id, + supplierName: po.expand?.supplier?.suppliername || "", + lineCount: poItems.length, + }).text + ); + } catch { + if (cancelled) return; + // Fall back to a generic body if the count lookup fails. The + // route still uses its own (correct) count at send time, so + // the worst case is the operator sees a slightly misleading + // preview — not a wrong email. + setMessage( + buildDefaultPoEmailBody({ + poNumber: po.po_number || po.id, + supplierName: po.expand?.supplier?.suppliername || "", + lineCount: 0, + }).text + ); + toast.error("Couldn't load line count for preview; check the PO before sending."); + } finally { + if (cancelled) return; + setMessageLoading(false); + } + })(); + } + + // Additional-attachments seed — read synchronously from the PO record. + // Independent of phase 2, so we always run it (resend or draft). + const rawAtt = po.additional_attachments ?? []; + setAttachments(Array.isArray(rawAtt) ? rawAtt : rawAtt ? [rawAtt] : []); + return () => { cancelled = true; };