From cfc974c08062dd73c0336bdedbece7e72c13fa4c Mon Sep 17 00:00:00 2001 From: SJCoetz Date: Wed, 29 Jul 2026 14:32:41 +0200 Subject: [PATCH] fix(orders): review-marker review fixes (collection helper, empty patch, markup_percent) Co-Authored-By: Claude Opus 4.8 Co-authored-by: Orca --- src/lib/server/orders-service-review.test.ts | 100 +++++++++++++++++++ src/lib/server/orders-service.ts | 29 ++++-- 2 files changed, 120 insertions(+), 9 deletions(-) diff --git a/src/lib/server/orders-service-review.test.ts b/src/lib/server/orders-service-review.test.ts index 8d3f250..9a3d81f 100644 --- a/src/lib/server/orders-service-review.test.ts +++ b/src/lib/server/orders-service-review.test.ts @@ -294,4 +294,104 @@ describe("updateOrderItemsPrices", () => { expect(result.updated).toBe(0); expect(result.failed).toEqual([]); }); + + it("skips pb.update when the resolved patch is empty (no field changes)", async () => { + const order = await pbMock.collection("orders").create({ + order_number: "ORD-2026-07-001", + }); + await pbMock.collection("order_items").create({ + id: "item-noop", + order: order.id, + description: "Drill", + quantity: 2, + selling_price: 50, + cost_price: 25, + markup: 100, + }); + await pbMock.collection("order_items").create({ + id: "item-does", + order: order.id, + description: "Bit", + quantity: 1, + selling_price: 10, + cost_price: 5, + markup: 100, + }); + + // Spy on update to confirm the no-op row doesn't trigger a write. + // `pbMock.collection(name)` returns a fresh object each call, so we + // wrap the `collection` factory itself and intercept at every call. + const originalCollection = pbMock.collection; + const updateCalls: Array<{ id: string; data: Record }> = []; + pbMock.collection = (name: string) => { + const col = originalCollection(name); + if (name === "order_items") { + const originalUpdate = col.update; + col.update = vi.fn(async (id: string, data: Record) => { + updateCalls.push({ id, data }); + return originalUpdate(id, data); + }); + } + return col; + }; + + try { + const result = await updateOrderItemsPrices(order.id, [ + // No field changes — caller's patch has no quantity/price/markup. + { id: "item-noop" }, + // Real change — should issue an update. + { id: "item-does", quantity: 7 }, + ]); + + // Only the row with an actual change counts as updated. + expect(result.updated).toBe(1); + expect(result.failed).toEqual([]); + + // The no-op row did NOT receive a pb.update call. + const noopCalls = updateCalls.filter((c) => c.id === "item-noop"); + expect(noopCalls).toEqual([]); + + // The real row did. + const doesCalls = updateCalls.filter((c) => c.id === "item-does"); + expect(doesCalls).toHaveLength(1); + expect(doesCalls[0].data).toEqual({ quantity: 7 }); + + // The row's quantity was actually written. + const item = await pbMock.collection("order_items").getOne("item-does"); + expect(item.quantity).toBe(7); + } finally { + pbMock.collection = originalCollection; + } + }); + + it("writes markup_percent alongside markup when selling_price is supplied", async () => { + const order = await pbMock.collection("orders").create({ + order_number: "ORD-2026-07-001", + }); + await pbMock.collection("order_items").create({ + id: "item-mp", + order: order.id, + description: "Drill", + quantity: 2, + selling_price: 50, + cost_price: 25, + // Start with stale markup_percent to confirm it gets overwritten. + markup: 100, + markup_percent: 0, + }); + + const result = await updateOrderItemsPrices(order.id, [ + // selling_price 60, cost_price unchanged at 25 → markup = (60-25)/25*100 = 140 + { id: "item-mp", selling_price: 60 }, + ]); + + expect(result.updated).toBe(1); + expect(result.failed).toEqual([]); + + const item = await pbMock.collection("order_items").getOne("item-mp"); + expect(item.selling_price).toBe(60); + expect(item.markup).toBe(140); + // markup_percent must mirror markup — the two fields stay in lock-step. + expect(item.markup_percent).toBe(140); + }); }); diff --git a/src/lib/server/orders-service.ts b/src/lib/server/orders-service.ts index 57fac7f..23013a3 100644 --- a/src/lib/server/orders-service.ts +++ b/src/lib/server/orders-service.ts @@ -1,7 +1,6 @@ import { getAdminPocketBase } from "./pocketbase-admin"; const ORDER_COLLECTION = "orders"; -const ORDER_ITEMS_COLLECTION = "order_items"; const QUOTE_COLLECTION = "quotes"; const QUOTE_ITEMS_COLLECTION = "quote_items"; const ORDER_COUNTERS_COLLECTION = "order_counters"; @@ -363,6 +362,7 @@ export async function updateOrderItemsPrices( failed: Array<{ id: string; error: string }>; }> { const pb = await getAdminPocketBase(); + const orderItemsCollection = getOrderItemsCollectionName(); if (items.length === 0) return { updated: 0, failed: [] }; @@ -373,7 +373,7 @@ export async function updateOrderItemsPrices( try { // Read the current row so we can compute markup when only // one of selling_price/cost_price is supplied. - const current = await pb.collection(ORDER_ITEMS_COLLECTION).getOne(item.id); + const current = await pb.collection(orderItemsCollection).getOne(item.id); const patch: Record = {}; if (item.quantity !== undefined) patch.quantity = item.quantity; @@ -386,23 +386,34 @@ export async function updateOrderItemsPrices( // - cost_price supplied, selling_price not → recompute from current-selling/cost // - both supplied → recompute from the new pair // - neither supplied, no explicit markup → leave current markup alone + // + // `markup_percent` mirrors `markup` (the two fields are kept in + // lock-step by `createOrderFromQuote`); write both together so + // the row never goes stale after a price edit. if (item.markup !== undefined) { patch.markup = item.markup; + patch.markup_percent = item.markup; } else if ( item.selling_price !== undefined || item.cost_price !== undefined ) { const newSelling = item.selling_price ?? current.selling_price; const newCost = item.cost_price ?? current.cost_price; - if (Number(newCost) > 0) { - patch.markup = - Math.round(((Number(newSelling) - Number(newCost)) / Number(newCost)) * 10000) / 100; - } else { - patch.markup = 0; - } + const newMarkup = + Number(newCost) > 0 + ? Math.round(((Number(newSelling) - Number(newCost)) / Number(newCost)) * 10000) / 100 + : 0; + patch.markup = newMarkup; + patch.markup_percent = newMarkup; } - await pb.collection(ORDER_ITEMS_COLLECTION).update(item.id, patch); + // Empty patch: nothing to write. Skip the PB round-trip so we + // don't stamp `updated` and don't bump the row's `updated` ts. + if (Object.keys(patch).length === 0) { + continue; + } + + await pb.collection(orderItemsCollection).update(item.id, patch); updated.push(item.id); } catch (err) { failed.push({