fix(orders): guard the pairing call site properly, correct comments
The previous commit's two "happy path" pairing tests never reset pairOrderLines' mock call history — vitest.config.ts doesn't set clearMocks, and the file's global beforeEach only clears the PB store. A prior passing setOrderQuote test left toHaveBeenCalledWith already satisfied on entry, so those two tests passed even with the production call site deleted. Add mockClear() after the dynamic import in both so they actually exercise the call they claim to. Also: reworded the idempotent-path comment to note the strict-pending gate runs first, so pairing only re-runs there while every line is still pending (not a general refresh mechanism); fixed the "don't write, don't stamp" line, which the pairing call now contradicts (order_items.quote_item is a write); and replaced a comment pointer to a nonexistent plan section with a standalone statement of the deferred-signal gap.
This commit is contained in:
parent
a0e7e152f7
commit
1453f14419
2 changed files with 22 additions and 11 deletions
|
|
@ -2674,6 +2674,7 @@ describe("setOrderQuote", () => {
|
|||
describe("setOrderQuote line pairing", () => {
|
||||
it("pairs order lines to the quote's lines after writing the hard link", async () => {
|
||||
const { pairOrderLines } = await import("./order-quote-link-service");
|
||||
vi.mocked(pairOrderLines).mockClear();
|
||||
await pbMock.collection("quotes").create({ id: "q-1" });
|
||||
await pbMock.collection("orders").create({
|
||||
id: "o-1", order_number: "ORD-1", quote: "", created: "2026-08-21T00:00:00Z",
|
||||
|
|
@ -2690,6 +2691,7 @@ describe("setOrderQuote line pairing", () => {
|
|||
|
||||
it("does not fail the link when pairing throws", async () => {
|
||||
const { pairOrderLines } = await import("./order-quote-link-service");
|
||||
vi.mocked(pairOrderLines).mockClear();
|
||||
vi.mocked(pairOrderLines).mockRejectedValueOnce(new Error("PB down"));
|
||||
|
||||
await pbMock.collection("quotes").create({ id: "q-1" });
|
||||
|
|
|
|||
|
|
@ -3721,12 +3721,19 @@ export async function setOrderQuote(
|
|||
const previous: { quote: string } = { quote: existing.quote || "" };
|
||||
|
||||
// Idempotent no-op when the order already points at this quote.
|
||||
// Don't write, don't stamp — just return the order as-is. Pairing
|
||||
// still runs on this path: it's safe to repeat (confident pairs
|
||||
// are only ever added, never overwritten) and it's the only way
|
||||
// to pick up lines that were added or edited after the original
|
||||
// link was made — e.g. a re-run of `orders.set_quote` used purely
|
||||
// to refresh pairing.
|
||||
// Don't write `orders.quote`, don't stamp — just return the order
|
||||
// as-is. Pairing still runs on this path: it's safe to repeat
|
||||
// (confident pairs are only ever added, never overwritten by
|
||||
// `pairOrderLines`), so there's no reason to skip it here.
|
||||
//
|
||||
// Note this doesn't buy as much as it might look like: the
|
||||
// strict-pending gate above already refuses the call the moment any
|
||||
// line has moved past `pending`, so this path only re-runs pairing
|
||||
// while every line is still pending — the exact window in which
|
||||
// lines are least likely to have drifted since the original link.
|
||||
// It's a real but narrow benefit (e.g. a line added right after
|
||||
// linking, before anything progressed), not a general "refresh
|
||||
// pairing any time" mechanism.
|
||||
if (previous.quote === quoteId) {
|
||||
const fresh = await pb.collection(ORDER_COLLECTION).getOne<Order>(orderId);
|
||||
await pairOrderLinesSafely(orderId, quoteId);
|
||||
|
|
@ -3766,11 +3773,13 @@ ${stamp}` : stamp;
|
|||
*
|
||||
* `console.error` is the only trace of a pairing failure right now —
|
||||
* there's no user-facing surface for "the link worked but pairing
|
||||
* didn't." That's an acceptable gap for this task (see the plan's
|
||||
* notes on watch-for #4) but worth revisiting if silent pairing
|
||||
* failures turn out to be common in practice — e.g. surfacing them
|
||||
* via the `potential_quote` suggestion machinery or an operator-visible
|
||||
* warning instead of only a server log line.
|
||||
* didn't." That's a deliberate gap for this task: adding one means
|
||||
* either a new field on `SetOrderQuoteResult` or a new field on the
|
||||
* order record, and plumbing it through to the UI/API/MCP callers —
|
||||
* real scope beyond this two-file change. Worth revisiting if silent
|
||||
* pairing failures turn out to be common in practice — e.g. surfacing
|
||||
* them via the `potential_quote` suggestion machinery or an
|
||||
* operator-visible warning instead of only a server log line.
|
||||
*/
|
||||
async function pairOrderLinesSafely(orderId: string, quoteId: string): Promise<void> {
|
||||
try {
|
||||
|
|
|
|||
Loading…
Reference in a new issue