fix(quotes): persist operator's letterhead choice to prevent silent letterhead-flip
This commit addresses the silent letterhead-flip bug reported on 2026-09-09, where the operator's selected letterhead was not being saved, leading to incorrect letterhead being used in subsequent PDF generations. The operator's choice is now persisted in the quote row during PDF generation, ensuring that the correct letterhead is used for future auto-regens and email sends. Changes include: - Updated the `generateQuotePdf` function to include the operator's letterhead choice in the database write. - Adjusted the API response to surface the persisted letterhead for UI synchronization. - Added tests to verify that the letterhead choice is correctly saved and not overwritten by empty or invalid inputs. This fix enhances the reliability of the quote generation process and improves user experience by maintaining the operator's selections.
This commit is contained in:
parent
a19ad698cd
commit
1e3121d838
5 changed files with 176 additions and 2 deletions
File diff suppressed because one or more lines are too long
|
|
@ -471,6 +471,14 @@ export default function QuoteProcessPage({ params }: { params: Promise<{ id: str
|
|||
// means "no claim"). Treat null and "" identically: the
|
||||
// pre-existing `claimed_by` is left alone in that case so a
|
||||
// stale local copy doesn't get clobbered.
|
||||
// Same pattern for `company_letterhead`: the service
|
||||
// persists the operator's explicit choice (CNL / ISherwood)
|
||||
// in the same PB write, and surfaces the resolved value as
|
||||
// `data.companyLetterhead` so the chip / dropdown updates
|
||||
// without a full refetch — and so the next auto-regen
|
||||
// (from `sendQuoteEmail`'s staleness check) ships the same
|
||||
// letterhead, not the email-intake default. Fix for the
|
||||
// silent letterhead-flip bug reported 2026-09-09.
|
||||
setQuote((prev) =>
|
||||
prev
|
||||
? {
|
||||
|
|
@ -481,6 +489,11 @@ export default function QuoteProcessPage({ params }: { params: Promise<{ id: str
|
|||
typeof data.claimedBy === "string" && data.claimedBy !== ""
|
||||
? data.claimedBy
|
||||
: prev.claimed_by,
|
||||
company_letterhead:
|
||||
typeof data.companyLetterhead === "string" &&
|
||||
data.companyLetterhead !== ""
|
||||
? data.companyLetterhead
|
||||
: prev.company_letterhead,
|
||||
}
|
||||
: prev
|
||||
);
|
||||
|
|
|
|||
|
|
@ -136,6 +136,15 @@ function renderOutcomeToResponse(outcome: GenerateQuotePdfOutcome) {
|
|||
// already claimed by someone, or actor email didn't
|
||||
// resolve); the UI treats null and "" the same.
|
||||
claimedBy: outcome.claimedBy,
|
||||
// Surface the post-update `company_letterhead` so the UI
|
||||
// can sync the letterhead chip / dropdown without a full
|
||||
// refetch. Same value `sendQuoteEmail`'s staleness regen
|
||||
// will fall back to on the next send — fixes the silent
|
||||
// letterhead-flip bug reported 2026-09-09 (the operator's
|
||||
// CNL / ISherwood choice used to render correctly but
|
||||
// wasn't persisted, so a later auto-regen shipped a PDF
|
||||
// with the wrong letterhead to the customer).
|
||||
companyLetterhead: outcome.companyLetterhead,
|
||||
});
|
||||
}
|
||||
if (outcome.kind === "readiness_failure") {
|
||||
|
|
|
|||
|
|
@ -684,6 +684,124 @@ describe("generateQuotePdf", () => {
|
|||
expect(outcome.readyItemCount).toBe(1);
|
||||
}
|
||||
});
|
||||
|
||||
it("persists the operator's letterhead choice to the quote row", async () => {
|
||||
// Regression for the silent letterhead-flip bug reported
|
||||
// 2026-09-09 (quote Q202609068, Masego Makoloi @ Norplats):
|
||||
// the operator clicked "ISherwood" on the process page, the
|
||||
// PDF rendered on ISHERWOOD letterhead, but the stored
|
||||
// `company_letterhead` stayed at "CNL" (the email-intake
|
||||
// default). A subsequent auto-regen — triggered by either
|
||||
// a price edit bumping `item.updated` or the rep's
|
||||
// `review-approve` markup tweak — used the stale stored
|
||||
// value and shipped a CNL PDF to the customer. Fix: persist
|
||||
// the operator's explicit choice on every successful render.
|
||||
const { quote } = await seedQuoteWithPdf();
|
||||
await pbMock.collection("quote_items").create({
|
||||
quote: quote.id,
|
||||
description: "Drywall & Panel Hoist",
|
||||
quantity: 1,
|
||||
cost_price: 9128,
|
||||
selling_price: 10862.32,
|
||||
markup: 19,
|
||||
supplier: "sup-acme",
|
||||
is_processed: true,
|
||||
moved_to_quote: null,
|
||||
});
|
||||
|
||||
const outcome = await generateQuotePdf(quote.id, {
|
||||
letterhead: "ISHERWOOD",
|
||||
});
|
||||
expect(outcome.kind).toBe("ok");
|
||||
if (outcome.kind !== "ok") throw new Error("expected ok outcome");
|
||||
|
||||
// Outcome echoes the resolved letterhead — same value the
|
||||
// next regen / send will fall back to.
|
||||
expect(outcome.companyLetterhead).toBe("ISHERWOOD");
|
||||
|
||||
// And the quote row reflects it. The seed writes "CNL" so a
|
||||
// successful persistence flips it.
|
||||
const after = await pbMock
|
||||
.collection("quotes")
|
||||
.getOne<{ company_letterhead: string }>(quote.id);
|
||||
expect(after.company_letterhead).toBe("ISHERWOOD");
|
||||
});
|
||||
|
||||
it("does NOT change the stored letterhead when the caller passes none", async () => {
|
||||
// `sendQuoteEmail`'s staleness check calls `generateQuotePdf`
|
||||
// without `options.letterhead`. We must NOT silently rewrite
|
||||
// the stored value to CNL just because the regen didn't pass
|
||||
// an override — otherwise every auto-regen would silently
|
||||
// flip the quote back to CNL (the email-intake default).
|
||||
const { quote } = await seedQuoteWithPdf();
|
||||
await pbMock.collection("quotes").update(quote.id, {
|
||||
company_letterhead: "ISHERWOOD",
|
||||
});
|
||||
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 outcome = await generateQuotePdf(quote.id); // no letterhead override
|
||||
expect(outcome.kind).toBe("ok");
|
||||
if (outcome.kind !== "ok") throw new Error("expected ok outcome");
|
||||
|
||||
// Stored letterhead still ISHERWOOD (the regen just used it).
|
||||
const after = await pbMock
|
||||
.collection("quotes")
|
||||
.getOne<{ company_letterhead: string }>(quote.id);
|
||||
expect(after.company_letterhead).toBe("ISHERWOOD");
|
||||
});
|
||||
|
||||
it("does NOT persist an unknown / typo'd letterhead code", async () => {
|
||||
// Defence against a silent flip-to-CNL: if the caller passes
|
||||
// a typo (e.g. "ISHERWOODS") the service treats it as no
|
||||
// override (the LETTERHEAD_CODES check rejects it), falls
|
||||
// back to the quote's stored `company_letterhead`, renders
|
||||
// with that, and must NOT rewrite the stored value. The
|
||||
// operator's known choice stays intact until they explicitly
|
||||
// click a known letterhead button.
|
||||
const { quote } = await seedQuoteWithPdf();
|
||||
await pbMock.collection("quotes").update(quote.id, {
|
||||
company_letterhead: "ISHERWOOD",
|
||||
});
|
||||
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 outcome = await generateQuotePdf(quote.id, {
|
||||
// not in LETTERHEAD_CODES — service ignores the override
|
||||
// and uses the quote's stored letterhead ("ISHERWOOD")
|
||||
letterhead: "ISHERWOODS",
|
||||
});
|
||||
expect(outcome.kind).toBe("ok");
|
||||
if (outcome.kind !== "ok") throw new Error("expected ok outcome");
|
||||
// Render used the stored value (ISHERWOOD) because the
|
||||
// typo'd input was rejected. Outcome echoes what was
|
||||
// actually rendered.
|
||||
expect(outcome.companyLetterhead).toBe("ISHERWOOD");
|
||||
// And the stored value stayed exactly as the operator set
|
||||
// it — no silent flip.
|
||||
const after = await pbMock
|
||||
.collection("quotes")
|
||||
.getOne<{ company_letterhead: string }>(quote.id);
|
||||
expect(after.company_letterhead).toBe("ISHERWOOD");
|
||||
});
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
|
|
|||
|
|
@ -147,6 +147,14 @@ export type GenerateQuotePdfOutcome =
|
|||
* status, existing claim, or actor email did not resolve).
|
||||
*/
|
||||
claimedBy: string | null;
|
||||
/**
|
||||
* The `company_letterhead` value persisted to the quote row in
|
||||
* this render's PB write. Equals `letterheadCode` (the resolved
|
||||
* letterhead that was just rendered) — the value the next
|
||||
* regen / send will fall back to. Surfaced so the route can
|
||||
* echo it back to the UI for optimistic local-state sync.
|
||||
*/
|
||||
companyLetterhead: string;
|
||||
}
|
||||
| {
|
||||
kind: "readiness_failure";
|
||||
|
|
@ -233,7 +241,12 @@ export async function generateQuotePdf(
|
|||
// Default to the quote's stored letterhead; let an explicit
|
||||
// caller override win (the route does this from the request
|
||||
// body; the send-email pipeline uses the stored value).
|
||||
const requestedCode = options?.letterhead || quote.company_letterhead;
|
||||
const operatorLetterheadOverride =
|
||||
typeof options?.letterhead === "string" &&
|
||||
(LETTERHEAD_CODES as readonly string[]).includes(options.letterhead)
|
||||
? (options.letterhead as LetterheadCode)
|
||||
: null;
|
||||
const requestedCode = operatorLetterheadOverride ?? quote.company_letterhead;
|
||||
const letterheadCode = (LETTERHEAD_CODES as readonly string[]).includes(
|
||||
requestedCode || "",
|
||||
)
|
||||
|
|
@ -426,6 +439,24 @@ export async function generateQuotePdf(
|
|||
// flip / timestamp bump — one PB write, no race window where
|
||||
// another client could observe a "claimed but no claimer"
|
||||
// state. Both update branches below honour `autoClaimUserId`.
|
||||
//
|
||||
// We ALSO persist the operator's explicit letterhead choice
|
||||
// (when one was passed) in the same PB write. This is the
|
||||
// fix for the silent letterhead-flip bug reported 2026-09-09:
|
||||
// the process page's CNL / ISherwood buttons used to render
|
||||
// the right letterhead but never persist the choice, so the
|
||||
// next auto-regen (from `sendQuoteEmail`'s staleness check
|
||||
// or from the rep's `review-approve`) used the stale stored
|
||||
// value — typically the email-intake default of CNL — and
|
||||
// shipped a PDF with the wrong letterhead to the customer.
|
||||
// Persisting here makes the stored value track what was just
|
||||
// rendered, so any future regen is faithful. We only persist
|
||||
// when the caller passed a *known* letterhead code (not a
|
||||
// typo'd unknown code) so a silent fallback to CNL doesn't
|
||||
// silently rewrite the stored value either.
|
||||
const letterheadPersistence = operatorLetterheadOverride
|
||||
? { company_letterhead: operatorLetterheadOverride }
|
||||
: {};
|
||||
let newStatus = quote.status;
|
||||
let pdfGeneratedAt: string | null = quote.pdf_generated_at || null;
|
||||
if (
|
||||
|
|
@ -442,6 +473,7 @@ export async function generateQuotePdf(
|
|||
await pb.collection("quotes").update(quoteId, {
|
||||
pdf_generated_at: pdfGeneratedAt,
|
||||
...(autoClaimUserId ? { claimed_by: autoClaimUserId } : {}),
|
||||
...letterheadPersistence,
|
||||
});
|
||||
}
|
||||
} else {
|
||||
|
|
@ -454,6 +486,7 @@ export async function generateQuotePdf(
|
|||
status: targetStatus,
|
||||
pdf_generated_at: pdfGeneratedAt,
|
||||
...(autoClaimUserId ? { claimed_by: autoClaimUserId } : {}),
|
||||
...letterheadPersistence,
|
||||
});
|
||||
newStatus = targetStatus;
|
||||
}
|
||||
|
|
@ -476,6 +509,7 @@ export async function generateQuotePdf(
|
|||
totalItemCount: allItems.length,
|
||||
isPartial: !!partialNote,
|
||||
claimedBy,
|
||||
companyLetterhead: letterheadCode,
|
||||
};
|
||||
} catch (err) {
|
||||
return {
|
||||
|
|
|
|||
Loading…
Reference in a new issue