fix(quotes): persist letterhead choice on terminal-status quotes too

The first letterhead-persistence fix (1e3121d) gated the WHOLE PB
write for generateQuotePdf's operator-choice persistence behind
"quote.status is not terminal" -- because terminal statuses
(approved/emailed/complete/cancelled/sending) deliberately skip the
pdf_generated_at re-stamp.

But regenerating a PDF on an already-terminal quote (e.g. clicking
"ISherwood" on an already-`emailed` quote from the process page) is
a normal, supported operator action -- confirmed live: it renders
the right letterhead and toasts "quote kept at emailed". That regen
hit the terminal branch and skipped the PB write entirely, so
company_letterhead never updated. A later auto-regen (sendQuoteEmail's
staleness check, e.g. triggered by a markup edit) then fell back to
the stale stored value and shipped the wrong letterhead again --
reproducing the original bug through a path the first fix didn't
cover.

Verified against the live quote via a direct PocketBase read
(bypassing app/MCP caching): company_letterhead was unchanged by the
first fix's terminal-status regen, and is now correctly persisted
while pdf_generated_at/status stay untouched as intended.

Split the letterhead write out from the timestamp/status write so it
always persists on an explicit operator choice regardless of quote
status. Added a regression test for the terminal-status + explicit
letterhead case.

Also syncs the n8n "Quote Outbound Send" workflow snapshot with two
more live fixes made during testing:
- Build attachments list: this.helpers.getBinaryDataBuffer() takes
  an item INDEX (number), not an item object -- per n8n's own docs.
  Was passing the item object, which threw an opaque non-Error value
  on every real send.
- Send Email (SMTP): the html field injected the plain-text
  email_message (with literal \n breaks) straight into HTML, so
  clients collapsed every paragraph into one block. Now converts \n
  to <br> first.
Also captures that the four WhatsApp-notify nodes and the workflow
itself are currently disabled/inactive in the live instance (manual
n8n edits during testing, not part of this fix).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Dave 2026-09-09 18:09:27 +02:00
parent 1e3121d838
commit 3d009b144f
3 changed files with 143 additions and 145 deletions

File diff suppressed because one or more lines are too long

View file

@ -802,6 +802,68 @@ describe("generateQuotePdf", () => {
.getOne<{ company_letterhead: string }>(quote.id); .getOne<{ company_letterhead: string }>(quote.id);
expect(after.company_letterhead).toBe("ISHERWOOD"); expect(after.company_letterhead).toBe("ISHERWOOD");
}); });
it("persists the operator's letterhead choice even when the quote is already terminal (e.g. emailed)", async () => {
// Regression: the first fix for the silent letterhead-flip bug
// gated the WHOLE persistence write (letterhead included)
// behind "quote.status is not terminal" — because terminal
// statuses (approved/emailed/complete/cancelled/sending)
// deliberately skip the pdf_generated_at re-stamp. But the
// process page's CNL/ISherwood buttons are usable on an
// already-`emailed` quote (regenerating a PDF after the first
// send is a normal operator action, confirmed live: clicking
// ISherwood on an `emailed` quote renders an ISHERWOOD PDF and
// toasts "quote kept at emailed"). That regen hit the terminal
// branch, rendered correctly, but skipped the PB write
// entirely — so `company_letterhead` never updated, and a
// later auto-regen (sendQuoteEmail's staleness check, e.g.
// triggered by a markup edit) fell back to the stale CNL value
// and shipped the wrong letterhead again. Confirmed against the
// real quote via a direct PocketBase read: company_letterhead
// and pdf_generated_at were both untouched by the "first fix".
const { quote } = await seedQuoteWithPdf();
await pbMock.collection("quotes").update(quote.id, {
status: "emailed",
});
await pbMock.collection("quote_items").create({
quote: quote.id,
description: "M16 hex bolt",
quantity: 1,
cost_price: 2.39,
selling_price: 3.11,
markup: 30,
supplier: "sup-acme",
is_processed: true,
moved_to_quote: null,
});
const before = await pbMock
.collection("quotes")
.getOne<{ pdf_generated_at: string }>(quote.id);
const outcome = await generateQuotePdf(quote.id, {
letterhead: "ISHERWOOD",
});
expect(outcome.kind).toBe("ok");
if (outcome.kind !== "ok") throw new Error("expected ok outcome");
expect(outcome.companyLetterhead).toBe("ISHERWOOD");
// Status stays exactly as it was — terminal statuses are never
// silently flipped by a PDF regen.
expect(outcome.status).toBe("emailed");
const after = await pbMock
.collection("quotes")
.getOne<{ company_letterhead: string; pdf_generated_at: string; status: string }>(
quote.id,
);
// The letterhead choice IS persisted now, even on a terminal
// quote — this is the fix.
expect(after.company_letterhead).toBe("ISHERWOOD");
// But pdf_generated_at and status are untouched, exactly as
// before — terminal-status semantics are preserved.
expect(after.pdf_generated_at).toBe(before.pdf_generated_at);
expect(after.status).toBe("emailed");
});
}); });
// ───────────────────────────────────────────────────────────────────────────── // ─────────────────────────────────────────────────────────────────────────────

View file

@ -475,6 +475,21 @@ export async function generateQuotePdf(
...(autoClaimUserId ? { claimed_by: autoClaimUserId } : {}), ...(autoClaimUserId ? { claimed_by: autoClaimUserId } : {}),
...letterheadPersistence, ...letterheadPersistence,
}); });
} else if (Object.keys(letterheadPersistence).length > 0) {
// Terminal statuses (approved/emailed/complete/cancelled/
// sending) skip the pdf_generated_at re-stamp above on
// purpose, but an explicit operator letterhead choice must
// still be persisted regardless of status. This is the gap
// that let the silent letterhead-flip bug survive the first
// fix: regenerating on an already-`emailed` quote (exactly
// what the process page's CNL/ISherwood buttons allow) hit
// this terminal branch, rendered the right letterhead, but
// never wrote it back — so a later auto-regen (e.g. from
// sendQuoteEmail's staleness check after a markup edit)
// fell back to the stale stored value and shipped the wrong
// letterhead again. Deliberately does NOT touch
// pdf_generated_at or status here.
await pb.collection("quotes").update(quoteId, letterheadPersistence);
} }
} else { } else {
pdfGeneratedAt = new Date().toISOString(); pdfGeneratedAt = new Date().toISOString();