diff --git a/AGENTS.md b/AGENTS.md index 2824cf7..246c2b5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -490,3 +490,46 @@ number differs from `wfdash-whats-new-seen` in the operator's localStorage. - To publish a release: merge, take `git log --oneline -1`, add an entry at the top with the next number (the next one after this change is 6) and that SHA. The number is the "seen" marker — bumping it re-notifies everyone. + +# POS price confirmation (Collection Confirm) — the till price is a decision + +A POS item's price (`stock_items.selling_price`) is what the till charges. It used to +move on its own: a bulk import wrote a spreadsheet's selling price straight over the +catalogue, and the /stock pricing grid silently recomputed the selling price whenever its +cost cell was edited. Nothing asked. + +Now the operator decides, per item, when they confirm a PO card. Files: +`src/lib/pos-price.ts` (pure price maths), `src/lib/server/collection-pos-price-service.ts` +(the write), `src/components/collection-confirm/pos-price-dialog.tsx` (the dialog), route +`POST /api/v1/collection-confirm/pos-price`. + +- **The dialog opens BEFORE the invoice/stock confirm**, once per PO card, and only when + confirming would move a till price. A PO card with nothing to decide goes straight to + the confirm dialog. +- **Every row shows the old price AND the new price.** The default is "Keep price" — + moving a till price is always deliberate. +- **The price is derived, never accepted from the client.** The route reads the item's own + markup and computes the price; the request body carries the COST only. +- **Update writes cost AND price together**, plus a zero-quantity `cost_history` entry + and a `stock_ledger` row with reason `adjustment`. Writing the price alone would leave + the cost stale and the next markup run would compute from the wrong base. The `markup` + field is NEVER written — the operator's rate is theirs. +- **Markup rule**: the item's `markup` field if set, else the margin implied by its + current price and cost, else the 80% default. `src/lib/pos-price.ts` is the only place + that rule lives, so the number the operator sees is the number that lands. +- **A failed price write does NOT block the confirm.** Goods stuck unconfirmed is a bigger + problem than a till price that did not move; the failure is reported by name. +- `src/lib/pos-price.ts` cannot import the server markup constant (it pulls in the PB admin + client), so `DEFAULT_MARKUP_PERCENT` is duplicated there. `pos-price.test.ts` fails if + the two ever disagree. + +## The stock_item expand bug — `fields` silently empties `expand` + +`listCollectedItems` expanded `purchase.stock_item` for the stock leg. `stock_purchases` has +no `stock_item` relation (only `supplier` and `booked_by`) — the relation lives on +`stock_purchase_items` — so `expand.stock_item` came back undefined on every row and the POS +pricing was never available. Fixed on 2026-11-09 by expanding `stock_item` directly. + +**Do not add a `fields` option to that query.** PocketBase applies `fields` to expanded +relations too, so naming only the parent columns empties `expand` again and the POS pricing +disappears — the same bug, one option away. diff --git a/src/app/(app)/collection-confirm/page-ai.test.tsx b/src/app/(app)/collection-confirm/page-ai.test.tsx index c1fd9e7..5f951a4 100644 --- a/src/app/(app)/collection-confirm/page-ai.test.tsx +++ b/src/app/(app)/collection-confirm/page-ai.test.tsx @@ -47,7 +47,17 @@ function aiRow(id: string, over: Partial = {}): ConfirmRow { const rows: ConfirmRow[] = [ aiRow("row-a", { in_pos: false }), - aiRow("row-b", { in_pos: true, pos_stock_item_id: "stock-1" }), + // The POS item is priced consistently with the line cost (45 at + // 80% = 81), so confirming it moves no till price and the price + // confirmation stays out of the way of these AI-flow tests. The + // price confirmation has its own tests. + aiRow("row-b", { + in_pos: true, + pos_stock_item_id: "stock-1", + pos_cost_price: 45, + pos_selling_price: 81, + pos_markup: 80, + }), aiRow("row-c", { in_pos: null }), ]; diff --git a/src/app/(app)/collection-confirm/page-pos-price.test.tsx b/src/app/(app)/collection-confirm/page-pos-price.test.tsx new file mode 100644 index 0000000..8fbf40b --- /dev/null +++ b/src/app/(app)/collection-confirm/page-pos-price.test.tsx @@ -0,0 +1,262 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { render, screen, waitFor, within } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; + +import Page from "./page"; +import type { ConfirmRow } from "@/lib/server/collection-confirm-service"; + +const toastMock = vi.hoisted(() => ({ + success: vi.fn(), + warning: vi.fn(), + error: vi.fn(), + info: vi.fn(), +})); +vi.mock("sonner", () => ({ toast: toastMock })); + +vi.mock("@/lib/pocketbase", () => ({ + getPocketBase: () => ({ authStore: { token: "tok-123" } }), +})); + +function jsonResponse(body: unknown, init: Partial = {}): Response { + return { ok: true, status: 200, json: async () => body, ...init } as Response; +} + +function fetchUrl(input: RequestInfo | URL): string { + if (typeof input === "string") return input; + if (input instanceof URL) return input.toString(); + return input.url; +} + +/** + * One customer row matched to a POS item by SKU. The POS item is + * priced at 180 for a cost of 100 (80% markup); the line cost of + * 120 would price it at 216, so confirming moves the till price. + */ +function posRow(over: Partial = {}): ConfirmRow { + return { + id: "row-pos", + sku: "PAINT-5L", + description: "PAINT 5L", + item_status: "collected", + source: "customer", + order_qty: 1, + qty_collected: 1, + cost_price: 120, + in_pos: true, + pos_stock_item_id: "stock-1", + pos_cost_price: 100, + pos_selling_price: 180, + pos_markup: 80, + expand: { + purchase_order: { + id: "po-1", + po_number: "PO-1", + expand: { supplier: { id: "sup-1", suppliername: "Alpha" } }, + }, + }, + ...over, + }; +} + +/** + * A stock line: the purchase line IS the catalogue item, so its cost + * is the item cost and its POS pricing is the same catalogue row. + */ +function posStockRow(over: Partial = {}): ConfirmRow { + return { + id: "row-stock", + source: "stock", + description: "WELD ROD", + sku: "ROD-1", + line_sku: "ROD-1", + item_status: "collected", + order_qty: 10, + qty_collected: 10, + quantity: 10, + cost_price: 60, + in_pos: true, + pos_stock_item_id: "stock-9", + pos_cost_price: 50, + pos_selling_price: 90, + pos_markup: 80, + expand: { + purchase_order: { + id: "po-1", + po_number: "PO-1", + expand: { supplier: { id: "sup-1", suppliername: "Alpha" } }, + }, + }, + ...over, + }; +} + +let rows: ConfirmRow[] = [posRow()]; + +function setupFetch(opts: { priceOk?: boolean } = {}) { + return vi.spyOn(window, "fetch").mockImplementation(async (input) => { + const url = fetchUrl(input); + if (url === "/api/v1/collection-confirm") return jsonResponse({ items: rows }); + if (url === "/api/v1/collection-confirm/pos-price") { + return opts.priceOk === false + ? jsonResponse({ error: "boom" }, { ok: false, status: 500 }) + : jsonResponse({ results: [{ ok: true, stockItemId: "stock-1" }] }); + } + if (url === "/api/v1/order-items/confirm") { + return jsonResponse({ updatedIds: [] }); + } + return jsonResponse({}); + }); +} + +async function openCardAndConfirm(user: ReturnType) { + render(); + await waitFor(() => + expect(screen.getByTestId("po-card-PO-1")).toBeInTheDocument(), + ); + await user.click(screen.getByTestId("po-card-toggle-PO-1")); + await user.click(screen.getByTestId("po-card-confirm-PO-1")); +} + +/** + * Fill the invoice number and submit the confirm dialog. The confirm + * button stays disabled until the number is there. + */ +async function submitInvoiceConfirm(user: ReturnType) { + const dialog = await screen.findByRole("dialog"); + await user.type(within(dialog).getByLabelText(/Supplier invoice number/i), "INV-1"); + await user.click(within(dialog).getByRole("button", { name: /^Confirm/i })); +} + +describe("CollectionConfirmPage POS price confirmation", () => { + beforeEach(() => { + vi.restoreAllMocks(); + for (const fn of Object.values(toastMock)) fn.mockClear(); + rows = [posRow()]; + }); + + it("asks about the price before opening the invoice confirm", async () => { + setupFetch(); + const user = userEvent.setup(); + await openCardAndConfirm(user); + + const dialog = await screen.findByRole("dialog"); + expect(within(dialog).getByText("POS price changes")).toBeInTheDocument(); + // The old price and the new price are both on screen. + const row = screen.getByTestId("pos-price-row-row-pos"); + expect(row.textContent).toContain("180,00"); + expect(screen.getByTestId("pos-price-new-row-pos")).toHaveTextContent("216,00"); + }); + + it("keeps every price when the operator changes nothing", async () => { + const spy = setupFetch(); + const user = userEvent.setup(); + await openCardAndConfirm(user); + + await user.click(await screen.findByTestId("pos-price-submit")); + await submitInvoiceConfirm(user); + await waitFor(() => + expect(spy.mock.calls.some(([arg]) => fetchUrl(arg) === "/api/v1/order-items/confirm")).toBe( + true, + ), + ); + // No price write at all, and the confirm went ahead. + expect( + spy.mock.calls.some(([arg]) => fetchUrl(arg) === "/api/v1/collection-confirm/pos-price"), + ).toBe(false); + }); + + it("writes the approved price and then confirms", async () => { + const spy = setupFetch(); + const user = userEvent.setup(); + await openCardAndConfirm(user); + + await user.click(await screen.findByTestId("pos-price-toggle-row-pos")); + await user.click(screen.getByTestId("pos-price-submit")); + + await waitFor(() => { + const call = spy.mock.calls.find( + ([arg]) => fetchUrl(arg) === "/api/v1/collection-confirm/pos-price", + ); + expect(call).toBeTruthy(); + const body = JSON.parse((call![1] as RequestInit).body as string); + expect(body.updates).toEqual([ + { stock_item_id: "stock-1", cost: 120, note: "Collection Confirm" }, + ]); + }); + + await submitInvoiceConfirm(user); + await waitFor(() => + expect(spy.mock.calls.some(([arg]) => fetchUrl(arg) === "/api/v1/order-items/confirm")).toBe( + true, + ), + ); + }); + + it("confirms even when the price write fails", async () => { + const spy = setupFetch({ priceOk: false }); + const user = userEvent.setup(); + await openCardAndConfirm(user); + + await user.click(await screen.findByTestId("pos-price-toggle-row-pos")); + await user.click(screen.getByTestId("pos-price-submit")); + await submitInvoiceConfirm(user); + + // The collection still has to be confirmed - a till price that did + // not move is a smaller problem than goods stuck in limbo. + await waitFor(() => + expect(spy.mock.calls.some(([arg]) => fetchUrl(arg) === "/api/v1/order-items/confirm")).toBe( + true, + ), + ); + expect(toastMock.error).toHaveBeenCalled(); + }); + + it("keeps the stock lines through the price confirmation", async () => { + rows = [posStockRow()]; + const spy = setupFetch(); + const user = userEvent.setup(); + render(); + await waitFor(() => + expect(screen.getByTestId("po-card-PO-1")).toBeInTheDocument(), + ); + await user.click(screen.getByTestId("po-card-toggle-PO-1")); + await user.click(screen.getByTestId("po-card-confirm-stock-PO-1")); + + // The price question comes first for a stock line too. + expect(await screen.findByText("POS price changes")).toBeInTheDocument(); + await user.click(screen.getByTestId("pos-price-submit")); + + // Then the stock confirm opens with its line still attached - a + // dropped row id would silently confirm nothing. + const dialog = await screen.findByRole("dialog"); + expect( + within(dialog).getByTestId("confirm-stock-row-row-stock"), + ).toBeInTheDocument(); + await user.click(within(dialog).getByRole("button", { name: /Confirm/i })); + await waitFor(() => + expect( + spy.mock.calls.some( + ([arg]) => fetchUrl(arg) === "/api/v1/collection-confirm/confirm-stock", + ), + ).toBe(true), + ); + }); + + it("does not ask when the price would not move", async () => { + rows = [ + posRow({ + cost_price: 100, + pos_cost_price: 100, + pos_selling_price: 180, + }), + ]; + setupFetch(); + const user = userEvent.setup(); + await openCardAndConfirm(user); + + // Straight to the invoice confirm - no price dialog. + const dialog = await screen.findByRole("dialog"); + expect(within(dialog).getByLabelText(/Supplier invoice number/i)).toBeInTheDocument(); + expect(screen.queryByText("POS price changes")).not.toBeInTheDocument(); + }); +}); diff --git a/src/app/(app)/collection-confirm/page.tsx b/src/app/(app)/collection-confirm/page.tsx index 2b93319..bc3a8c6 100644 --- a/src/app/(app)/collection-confirm/page.tsx +++ b/src/app/(app)/collection-confirm/page.tsx @@ -11,10 +11,15 @@ import { AutoPushLog } from "@/components/collection-confirm/auto-push-log"; import { ConfirmStockDialog } from "@/components/collection-confirm/confirm-stock-dialog"; import { InvoiceViewer } from "@/components/collection-confirm/invoice-viewer"; import { PoCard } from "@/components/collection-confirm/po-card"; +import { + PosPriceDialog, + type PosPriceDialogRow, +} from "@/components/collection-confirm/pos-price-dialog"; import type { Attachment } from "@/components/collection-confirm/photo-strip"; import { aiInvoiceDefaults, priceListResultToast } from "@/lib/collection-ai-card"; import { getClientIdentity } from "@/lib/dev-identity"; import { getPocketBase } from "@/lib/pocketbase"; +import { computePosPriceChange } from "@/lib/pos-price"; import type { ConfirmRow } from "@/lib/server/collection-confirm-service"; import { @@ -44,6 +49,65 @@ function rowPoNumber(row: ConfirmRow): string { return row.expand?.purchase_order?.po_number ?? row.poNumber ?? ""; } +/** + * The lines in a PO group whose POS price would move if the cost being + * confirmed were written to their till item. + * + * A line qualifies when all three hold: + * + * 1. It is linked to a POS item - either a stock line (the line IS the + * catalogue item) or a customer line whose SKU matches one. + * 2. There is a cost to price at. For a customer line that is the + * cost override the operator typed, else the cost already on the + * line. A line with no cost has nothing to reprice. + * 3. The price at that cost differs from what the till charges today. + * + * The cost OVERRIDE wins over the line cost: the operator typed that + * number deliberately on the cost pencil, and it is the number that is + * about to be confirmed. + * + * `rowIds` narrows the result to the lines a given confirm actually + * covers. A PO card can hold both customer and stock lines, and each + * confirm covers a different set - the price question must only be + * asked about the lines that confirm is about to commit. + */ +function posPriceRowsForGroup( + group: { poNumber: string; rows: ConfirmRow[] }, + costOverrides: Record, + rowIds?: string[], +): PosPriceDialogRow[] { + const out: PosPriceDialogRow[] = []; + const scope = rowIds ? new Set(rowIds) : null; + for (const row of group.rows) { + if (scope && !scope.has(row.id)) continue; + if (!row.pos_stock_item_id) continue; + const override = costOverrides[row.id]; + const lineCost = typeof override === "number" ? override : row.cost_price; + if (typeof lineCost !== "number" || !Number.isFinite(lineCost) || lineCost <= 0) { + continue; + } + const change = computePosPriceChange({ + currentCost: row.pos_cost_price, + currentPrice: row.pos_selling_price, + markup: row.pos_markup, + newCost: lineCost, + }); + if (!change.changed) continue; + out.push({ + rowId: row.id, + stockItemId: row.pos_stock_item_id, + sku: row.line_sku || row.sku || null, + description: row.description || row.po_description || null, + currentCost: row.pos_cost_price ?? null, + currentPrice: row.pos_selling_price ?? null, + markup: row.pos_markup ?? null, + lineCost, + catalogueCost: row.pos_cost_price ?? null, + }); + } + return out; +} + function workspaceDocumentKind(filename: string): string { const normalized = filename.toLowerCase(); if (normalized.startsWith("[invoice]") || normalized.includes("invoice")) return "Invoice"; @@ -87,6 +151,21 @@ export default function CollectionConfirmPage() { poNumber: string; rowIds: string[]; } | null>(null); + // Price-confirmation state. When the operator confirms a PO whose + // lines are on the POS, and the cost being confirmed would move a + // till price, this dialog asks what to do about each one BEFORE + // the invoice confirm. It remembers what the operator was trying + // to confirm so the flow can carry on once the prices are settled. + const [posPriceState, setPosPriceState] = useState<{ + poNumber: string; + rowIds: string[]; + /** Where to go once the price decision is made. */ + next: "invoice" | "stock"; + /** Customer rows to add to the price list once the confirm succeeds. */ + addToPosRowIds?: string[]; + /** The stock lines the confirm covers, for the stock confirm. */ + stockRowIds?: string[]; + } | null>(null); const [busy, setBusy] = useState(false); /** * Floating-viewer state for "Pop out" (draggable, defaults) and @@ -782,6 +861,173 @@ export default function CollectionConfirmPage() { ); }, [confirmState, costOverrides]); + /** + * Open the invoice or stock confirm dialog for a PO card. + */ + const openConfirmDialog = useCallback( + ( + group: { poNumber: string; rows: ConfirmRow[] }, + next: "invoice" | "stock", + addToPosRowIds?: string[], + stockRowIds?: string[], + ) => { + if (next === "stock") { + setConfirmStockState({ + poNumber: group.poNumber, + rowIds: stockRowIds ?? [], + }); + return; + } + setConfirmState({ + poNumber: group.poNumber, + rowIds: group.rows.map((r) => r.id), + ...(addToPosRowIds ? { addToPosRowIds } : {}), + defaultValues: aiInvoiceDefaults(group.rows), + }); + }, + [], + ); + + /** + * Start a confirm. If confirming would move a till price, the + * price confirmation comes FIRST and the invoice/stock confirm + * follows it. Otherwise the confirm dialog opens straight away. + * + * The price question comes first because it is the one with a + * consequence outside this screen: the till keeps charging the old + * price until somebody says otherwise. Letting the operator read + * the invoice details first, and only then discover the price + * moved, is how the surprise happened in the first place. + */ + const beginConfirm = useCallback( + ( + group: { poNumber: string; rows: ConfirmRow[] }, + next: "invoice" | "stock", + addToPosRowIds?: string[], + stockRowIds?: string[], + ) => { + const rowIds = + next === "stock" ? (stockRowIds ?? []) : group.rows.map((r) => r.id); + const scoped = Object.fromEntries( + Object.entries(costOverrides).filter(([id]) => rowIds.includes(id)), + ); + const priceRows = posPriceRowsForGroup(group, scoped, rowIds); + if (priceRows.length > 0) { + setPosPriceState({ + poNumber: group.poNumber, + rowIds, + next, + ...(addToPosRowIds ? { addToPosRowIds } : {}), + ...(stockRowIds ? { stockRowIds } : {}), + }); + return; + } + openConfirmDialog(group, next, addToPosRowIds, stockRowIds); + }, + [costOverrides, openConfirmDialog], + ); + + /** + * Apply the price changes the operator approved, then carry on with + * the confirm they were trying to do. + * + * A failed price update does NOT block the confirm - the collection + * still has to be confirmed for invoicing, and a till price that did + * not move is a smaller problem than goods stuck in limbo. The + * failure is reported by name so the operator can fix it. + */ + const handlePosPriceSubmit = useCallback( + async (updates: { row: PosPriceDialogRow; cost: number }[]) => { + const state = posPriceState; + if (!state) return; + if (updates.length === 0) { + // "Keep all prices" - nothing to write, carry straight on. + const group = groupedRows.find((g) => g.poNumber === state.poNumber); + setPosPriceState(null); + if (group) { + openConfirmDialog( + group, + state.next, + state.addToPosRowIds, + state.stockRowIds, + ); + } + return; + } + setBusy(true); + try { + const response = await fetch("/api/v1/collection-confirm/pos-price", { + method: "POST", + headers: authedJsonHeaders(), + body: JSON.stringify({ + updates: updates.map((u) => ({ + stock_item_id: u.row.stockItemId, + cost: u.cost, + note: "Collection Confirm", + })), + }), + }); + const data = (await response.json().catch(() => ({}))) as { + results?: Array<{ ok: boolean; message?: string }>; + error?: string; + }; + if (!response.ok) { + throw new Error(data.error || `Failed to update prices (HTTP ${response.status})`); + } + const failed = (data.results ?? []).filter((r) => !r.ok); + if (failed.length > 0) { + toast.warning( + failed.length + + " POS price" + + (failed.length === 1 ? "" : "s") + + " did not update - the till keeps the old price.", + ); + } else { + toast.success( + "Updated " + + updates.length + + " POS price" + + (updates.length === 1 ? "" : "s") + + ".", + ); + } + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + toast.error(message); + } finally { + setBusy(false); + } + // Carry on with the confirm either way - the price decision does + // not block the collection. + const group = groupedRows.find((g) => g.poNumber === state.poNumber); + setPosPriceState(null); + if (group) { + openConfirmDialog( + group, + state.next, + state.addToPosRowIds, + state.stockRowIds, + ); + } + }, + [posPriceState, groupedRows, openConfirmDialog, authedJsonHeaders], + ); + + /** + * The price-confirmation rows for the PO card being confirmed. + * Empty when nothing on the card would move a till price. + */ + const scopedPosPriceRows = useMemo(() => { + if (!posPriceState) return []; + const rowIdSet = new Set(posPriceState.rowIds); + const scoped = Object.fromEntries( + Object.entries(costOverrides).filter(([id]) => rowIdSet.has(id)), + ); + const group = groupedRows.find((g) => g.poNumber === posPriceState.poNumber); + if (!group) return []; + return posPriceRowsForGroup(group, scoped, posPriceState.rowIds); + }, [posPriceState, costOverrides, groupedRows]); + return (
@@ -886,29 +1132,14 @@ export default function CollectionConfirmPage() { onMovePhoto={handleMovePhoto} onDeletePhoto={handleDeletePhoto} onConfirmAndAddToPos={() => - setConfirmState({ - poNumber: group.poNumber, - rowIds: group.rows.map((r) => r.id), - addToPosRowIds: group.rows - .filter((r) => r.source !== "stock" && !r.in_pos) - .map((r) => r.id), - defaultValues: aiInvoiceDefaults(group.rows), - }) - } - onConfirm={() => - setConfirmState({ - poNumber: group.poNumber, - rowIds: group.rows.map((r) => r.id), - defaultValues: aiInvoiceDefaults(group.rows), - }) + beginConfirm(group, "invoice", group.rows + .filter((r) => r.source !== "stock" && !r.in_pos) + .map((r) => r.id)) } + onConfirm={() => beginConfirm(group, "invoice")} onConfirmStock={ stockRowIds.length > 0 - ? () => - setConfirmStockState({ - poNumber: group.poNumber, - rowIds: stockRowIds, - }) + ? () => beginConfirm(group, "stock", undefined, stockRowIds) : undefined } onInvoiceUpload={ @@ -934,6 +1165,23 @@ export default function CollectionConfirmPage() {
)} + {/* + Price confirmation. Sits between the operator clicking Confirm + and the invoice confirm dialog: it asks what should happen to + the till prices of the items on this PO card whose price the + confirmed cost would move. + */} + { + if (!open) setPosPriceState(null); + }} + onSubmit={(updates) => void handlePosPriceSubmit(updates)} + /> +

- + {/* Full description, wrapped. `line-clamp-2` used to + cut it off after two lines, hiding the part of the + description that distinguishes one line from + another (size, spec, colour). `break-words` keeps a + long unbroken token from widening the card. */} + {row.description || "(no description)"} {/* Stock-source badge. Same primary-color treatment as diff --git a/src/app/api/v1/collection-confirm/pos-price/route.test.ts b/src/app/api/v1/collection-confirm/pos-price/route.test.ts new file mode 100644 index 0000000..09e8426 --- /dev/null +++ b/src/app/api/v1/collection-confirm/pos-price/route.test.ts @@ -0,0 +1,120 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { NextRequest } from "next/server"; + +vi.mock("@/lib/server/browser-session", () => ({ getBrowserSessionUser: vi.fn() })); +vi.mock("@/lib/server/collection-pos-price-service", () => ({ applyPosPriceUpdates: vi.fn() })); +import { getBrowserSessionUser } from "@/lib/server/browser-session"; +import { applyPosPriceUpdates } from "@/lib/server/collection-pos-price-service"; +import { POST } from "./route"; + +const req = (body: unknown) => + new NextRequest("http://localhost/api/v1/collection-confirm/pos-price", { + method: "POST", + body: JSON.stringify(body), + headers: { + Authorization: "Bearer token", + "content-type": "application/json", + }, + }); + +beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(getBrowserSessionUser).mockResolvedValue({ + id: "buyer", + name: "Buyer", + user_type: [], + }); + vi.mocked(applyPosPriceUpdates).mockResolvedValue([ + { ok: true, stockItemId: "s1", previousPrice: 180, newPrice: 216, cost: 120 }, + ]); +}); + +describe("POST collection-confirm/pos-price", () => { + it("requires a real browser session", async () => { + vi.mocked(getBrowserSessionUser).mockRejectedValue( + Object.assign(new Error("Not authenticated."), { status: 401 }), + ); + expect((await POST(req({ updates: [{ stock_item_id: "s1", cost: 120 }] }))).status).toBe(401); + expect(applyPosPriceUpdates).not.toHaveBeenCalled(); + }); + + it.each([null, {}, { updates: [] }, { updates: "nope" }])( + "rejects a malformed body %#", + async (body) => { + expect((await POST(req(body))).status).toBe(400); + expect(applyPosPriceUpdates).not.toHaveBeenCalled(); + }, + ); + + it("rejects more than 100 updates", async () => { + const updates = Array.from({ length: 101 }, (_, i) => ({ + stock_item_id: "s" + i, + cost: 1, + })); + expect((await POST(req({ updates }))).status).toBe(400); + expect(applyPosPriceUpdates).not.toHaveBeenCalled(); + }); + + it("uses the authenticated actor, never an actor supplied in the body", async () => { + const res = await POST( + req({ + updates: [{ stock_item_id: "s1", cost: 120 }], + actor: { id: "forged" }, + }), + ); + expect(res.status).toBe(200); + expect(applyPosPriceUpdates).toHaveBeenCalledWith( + [ + { + stockItemId: "s1", + cost: 120, + note: "Collection Confirm", + }, + ], + { id: "buyer", name: "Buyer" }, + ); + }); + + it("never accepts a price from the client - only the cost", async () => { + await POST( + req({ + updates: [ + { stock_item_id: "s1", cost: 120, selling_price: 99999, price: 99999 }, + ], + }), + ); + const [updates] = vi.mocked(applyPosPriceUpdates).mock.calls[0]; + expect(updates[0]).toEqual({ + stockItemId: "s1", + cost: 120, + note: "Collection Confirm", + }); + expect(updates[0]).not.toHaveProperty("selling_price"); + }); + + it("returns one result per update", async () => { + vi.mocked(applyPosPriceUpdates).mockResolvedValue([ + { ok: true, stockItemId: "s1", previousPrice: 180, newPrice: 216, cost: 120 }, + { ok: false, stockItemId: "s2", reason: "not_found", message: "gone" }, + ]); + const res = await POST( + req({ + updates: [ + { stock_item_id: "s1", cost: 120 }, + { stock_item_id: "s2", cost: 50 }, + ], + }), + ); + expect(res.status).toBe(200); + const body = await res.json(); + expect(body.results).toHaveLength(2); + expect(body.results[1].ok).toBe(false); + }); + + it("does not leak backend errors", async () => { + vi.mocked(applyPosPriceUpdates).mockRejectedValue(new Error("internal secret")); + const res = await POST(req({ updates: [{ stock_item_id: "s1", cost: 1 }] })); + expect(res.status).toBe(500); + expect(JSON.stringify(await res.json())).not.toContain("internal secret"); + }); +}); diff --git a/src/app/api/v1/collection-confirm/pos-price/route.ts b/src/app/api/v1/collection-confirm/pos-price/route.ts new file mode 100644 index 0000000..b88fcb7 --- /dev/null +++ b/src/app/api/v1/collection-confirm/pos-price/route.ts @@ -0,0 +1,97 @@ +import { NextRequest, NextResponse } from "next/server"; +import { getBrowserSessionUser } from "@/lib/server/browser-session"; +import { applyPosPriceUpdates } from "@/lib/server/collection-pos-price-service"; + +/** + * POST /api/v1/collection-confirm/pos-price + * + * Apply the POS price changes the operator approved in the price + * confirmation dialog. + * + * The dialog shows the price the till charges today next to the price + * it WOULD charge at the new cost, and the operator picks per item. + * This route applies the ones they picked. + * + * ## Request + * + * ```json + * { + * "updates": [ + * { "stock_item_id": "abc123", "cost": 120.50, "note": "Collection Confirm" } + * ] + * } + * ``` + * + * `updates` is 1-100 entries. Each carries the POS item id and the + * cost to price it at. The PRICE IS NOT ACCEPTED FROM THE CLIENT: the + * route reads the item's own markup and computes the price, so the + * number the operator saw is the number that lands, and a tampered + * request cannot set an arbitrary till price. + * + * ## Responses + * + * - `200 { results: [...] }` - one result per update, in the same + * order. A failed row does not stop the others (per-row isolation). + * - `400 { error }` when the body is malformed. + * - `401 { error }` without a browser session. + * + * Auth: browser session (PB token in `Authorization`). This route + * changes what the till charges, so it is gated like the repairs + * family rather than left open. + */ +export const runtime = "nodejs"; +export const dynamic = "force-dynamic"; + +const MAX_UPDATES = 100; + +export async function POST(req: NextRequest) { + try { + const user = await getBrowserSessionUser(req); + // A body of `null` parses fine but is not an object - coerce it + // to `{}` so it lands on the 400 below instead of a 500. + const parsed = (await req.json().catch(() => ({}))) as unknown; + const body = (parsed && typeof parsed === "object" ? parsed : {}) as { + updates?: unknown; + }; + + if (!Array.isArray(body.updates) || body.updates.length === 0) { + return NextResponse.json( + { error: "updates must be a non-empty array" }, + { status: 400 }, + ); + } + if (body.updates.length > MAX_UPDATES) { + return NextResponse.json( + { error: `At most ${MAX_UPDATES} updates per call` }, + { status: 400 }, + ); + } + + const updates = body.updates.map((raw) => { + const u = (raw ?? {}) as { + stock_item_id?: unknown; + cost?: unknown; + note?: unknown; + }; + return { + stockItemId: typeof u.stock_item_id === "string" ? u.stock_item_id : "", + cost: typeof u.cost === "number" ? u.cost : Number.NaN, + note: typeof u.note === "string" ? u.note : "Collection Confirm", + }; + }); + + const results = await applyPosPriceUpdates(updates, { + id: user.id, + name: user.name, + }); + return NextResponse.json({ results }); + } catch (err) { + const message = err instanceof Error ? err.message : "Failed to update POS prices"; + const status = (err as Error & { status?: number }).status || 500; + if (status === 401) { + return NextResponse.json({ error: message }, { status: 401 }); + } + console.error("[POST /api/v1/collection-confirm/pos-price]", message); + return NextResponse.json({ error: "internal error" }, { status: 500 }); + } +} diff --git a/src/components/collection-confirm/items-table.tsx b/src/components/collection-confirm/items-table.tsx index eab5cde..4ef56f1 100644 --- a/src/components/collection-confirm/items-table.tsx +++ b/src/components/collection-confirm/items-table.tsx @@ -415,7 +415,14 @@ export function ItemsTable({ className="px-3 py-2 align-top" title={row.description ?? row.po_description ?? undefined} > - + {/* Full description, wrapped. `line-clamp-2` used to + cut long descriptions off after two lines, which hid + exactly the detail the operator needs (size, spec, + colour). `break-words` keeps a long unbroken token + (a part number with no spaces) from widening the + table; `align-top` on the cell keeps the row's + other columns pinned to the top of a tall row. */} + {row.description ?? row.po_description ?? "—"} {onReturnToPool ? ( diff --git a/src/components/collection-confirm/pos-price-dialog.test.tsx b/src/components/collection-confirm/pos-price-dialog.test.tsx new file mode 100644 index 0000000..22399c0 --- /dev/null +++ b/src/components/collection-confirm/pos-price-dialog.test.tsx @@ -0,0 +1,158 @@ +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { describe, expect, it, vi } from "vitest"; + +import { formatCurrency } from "@/lib/currency"; +import { PosPriceDialog, type PosPriceDialogRow } from "./pos-price-dialog"; + +const paint: PosPriceDialogRow = { + rowId: "row-1", + stockItemId: "stock-1", + sku: "PAINT-5L", + description: "PAINT 5L", + currentCost: 100, + currentPrice: 180, + markup: 80, + lineCost: 120, + catalogueCost: 100, +}; + +const desk: PosPriceDialogRow = { + rowId: "row-2", + stockItemId: "stock-2", + sku: "DESK", + description: "DESK 1200", + currentCost: 500, + currentPrice: 900, + markup: 80, + lineCost: 500, + catalogueCost: 500, +}; + +describe("PosPriceDialog", () => { + it("shows the old price and the new price for every row", () => { + render( + , + ); + // R180.00 is what the till charges now; R216.00 is what it would + // charge at the new cost of R120. + // The old and new price sit in one cell, so assert on the text + // content rather than an exact-text match. + const cell = screen.getByTestId("pos-price-new-row-1"); + expect(cell.textContent).toContain(formatCurrency(216)); + expect(screen.getByTestId("pos-price-row-row-1").textContent).toContain( + formatCurrency(180), + ); + }); + + it("defaults to keeping the price", () => { + render( + , + ); + expect(screen.getByTestId("pos-price-toggle-row-1")).toHaveTextContent("Keep price"); + expect(screen.getByTestId("pos-price-summary")).toHaveTextContent("Keeping every price"); + }); + + it("submits only the rows the operator chose to update", async () => { + const onSubmit = vi.fn(); + render( + , + ); + + await userEvent.click(screen.getByTestId("pos-price-toggle-row-1")); + expect(screen.getByTestId("pos-price-summary")).toHaveTextContent("Updating 1 of 2"); + + await userEvent.click(screen.getByTestId("pos-price-submit")); + expect(onSubmit).toHaveBeenCalledTimes(1); + const updates = onSubmit.mock.calls[0][0]; + expect(updates).toHaveLength(1); + expect(updates[0].row.rowId).toBe("row-1"); + // The line cost is the default basis. + expect(updates[0].cost).toBe(120); + }); + + it("submits nothing when every price is kept", async () => { + const onSubmit = vi.fn(); + render( + , + ); + + await userEvent.click(screen.getByTestId("pos-price-submit")); + expect(onSubmit).toHaveBeenCalledWith([]); + }); + + it("offers both cost bases when the catalogue cost differs from the line cost", async () => { + const onSubmit = vi.fn(); + render( + , + ); + + expect( + screen.getByTestId("pos-price-basis-line-row-1").textContent, + ).toContain(formatCurrency(120)); + expect( + screen.getByTestId("pos-price-basis-catalogue-row-1").textContent, + ).toContain(formatCurrency(100)); + + // Switching to the catalogue basis reprices from R100: 100 * 1.8. + await userEvent.click(screen.getByTestId("pos-price-basis-catalogue-row-1")); + expect(screen.getByTestId("pos-price-new-row-1").textContent).toContain( + formatCurrency(180), + ); + + await userEvent.click(screen.getByTestId("pos-price-toggle-row-1")); + await userEvent.click(screen.getByTestId("pos-price-submit")); + const updates = onSubmit.mock.calls[0][0]; + expect(updates[0].cost).toBe(100); + }); + + it("does not offer a basis choice when the two costs are the same", () => { + render( + , + ); + expect(screen.queryByTestId("pos-price-basis-line-row-2")).not.toBeInTheDocument(); + }); + + it("shows the markup rate the new price was derived at", () => { + render( + , + ); + expect(screen.getByText(/at 80% markup/)).toBeInTheDocument(); + }); +}); diff --git a/src/components/collection-confirm/pos-price-dialog.tsx b/src/components/collection-confirm/pos-price-dialog.tsx new file mode 100644 index 0000000..78af85c --- /dev/null +++ b/src/components/collection-confirm/pos-price-dialog.tsx @@ -0,0 +1,310 @@ +"use client"; + +import { useMemo, useState } from "react"; +import { ArrowRight, Loader2, TrendingDown, TrendingUp } from "lucide-react"; + +import { Button } from "@/components/ui/button"; +import { ModalShell } from "@/components/ui/modal-shell"; +import { computePosPriceChange } from "@/lib/pos-price"; +import { formatCurrency } from "@/lib/currency"; + +/** + * Which cost the operator wants the till priced at. + * + * "line" is the cost on the collection line being confirmed - what we + * paid for this delivery. "catalogue" is the POS item's own cost - our + * standing cost for the item. For a stock line these are the same + * thing (the line IS the catalogue item); for a customer line matched + * by SKU they can differ, and the operator decides which one the till + * should be priced from. + */ +export type PosPriceBasis = "line" | "catalogue"; + +/** + * One line of the price confirmation: a POS item whose price would + * move if the cost being confirmed were written to it. + */ +export type PosPriceDialogRow = { + /** The line id on the collection card (used for keys + the parent map). */ + rowId: string; + /** The POS item id - what gets repriced. */ + stockItemId: string; + /** SKU as the till knows it. */ + sku?: string | null; + /** The line description, for the operator to recognise it. */ + description?: string | null; + /** The POS item cost today. */ + currentCost?: number | null; + /** What the till charges today. */ + currentPrice?: number | null; + /** The POS item markup field, when it has one. */ + markup?: number | null; + /** The cost being confirmed on the collection line. */ + lineCost: number; + /** + * The POS item's own cost. Null when it has none (never priced), + * in which case the line cost is the only basis on offer. + */ + catalogueCost?: number | null; +}; + +type Props = { + open: boolean; + onOpenChange: (open: boolean) => void; + /** The PO card the operator is confirming (shown as the eyebrow). */ + poNumber?: string; + /** The lines whose POS price would move. */ + rows: PosPriceDialogRow[]; + /** Disables the buttons and shows a spinner while the parent persists. */ + busy?: boolean; + /** + * Called with the rows the operator chose to UPDATE, and the cost + * each one should be priced at. Only the "Update price" rows are in + * this list; the ones left on "Keep price" are not, and their POS + * item keeps its current price AND its current cost. + */ + onSubmit: (updates: { row: PosPriceDialogRow; cost: number }[]) => void | Promise; +}; + +/** + * The price confirmation. + * + * A POS item price can move without anyone deciding: new stock arrives + * at a new cost, a spreadsheet or the /stock grid recomputes the + * selling price, and the till is charging a different number than it + * was yesterday. This dialog is the deliberate version of that + * decision - it shows what the till charges now next to what it would + * charge at the new cost, per item, and the operator says which way + * each one goes. + * + * Three rules the dialog holds to: + * + * 1. EVERY row shows its old price AND its new price. A row with no + * visible from-to is not a decision, it is a surprise. + * 2. The default is "Keep price". Moving a till price is always + * deliberate - the safe outcome is the one you get for free. + * 3. When the line cost and the catalogue cost differ, BOTH are + * shown and the operator picks which one prices the till. + * + * The per-item choice is the point: a paint tin whose cost moved and a + * desk whose price is fine are different decisions, so one global + * toggle would be the wrong tool. + */ +export function PosPriceDialog(props: Props) { + const { open, onOpenChange, poNumber, rows, busy = false, onSubmit } = props; + + // Which rows the operator chose to update. Absent = keep the price. + // Seeded as "keep" (nothing selected) so the default is the safe one. + const [updating, setUpdating] = useState>({}); + // Which cost each row should be priced at. Defaults to the line + // cost - the number the operator just confirmed. + const [basis, setBasis] = useState>({}); + + const computed = useMemo( + () => + rows.map((row) => { + const chosen = basis[row.rowId] ?? "line"; + const cost = + chosen === "catalogue" && row.catalogueCost != null + ? row.catalogueCost + : row.lineCost; + const change = computePosPriceChange({ + currentCost: row.currentCost, + currentPrice: row.currentPrice, + markup: row.markup, + newCost: cost, + }); + // The two bases only differ when the catalogue cost is known + // and is not the same number as the line cost. + const twoBases = + row.catalogueCost != null && row.catalogueCost !== row.lineCost; + return { row, change, chosen, cost, twoBases }; + }), + [rows, basis], + ); + + const toggle = (rowId: string) => { + setUpdating((prev) => ({ ...prev, [rowId]: !prev[rowId] })); + }; + + const selected = computed.filter((c) => updating[c.row.rowId]); + const totalMoving = selected.length; + + const submit = () => { + void onSubmit(selected.map((c) => ({ row: c.row, cost: c.cost }))); + }; + + const summary = + totalMoving === 0 + ? "Keeping every price as it is." + : "Updating " + + totalMoving + + " of " + + rows.length + + " price" + + (rows.length === 1 ? "" : "s") + + "."; + + return ( + +

+ {summary} +

+
+ + +
+
+ } + > +
    + {computed.map(({ row, change, chosen: chosenBasis, cost, twoBases }) => { + const isUpdating = !!updating[row.rowId]; + const up = change.delta > 0; + return ( +
  • +
    +
    +

    + {row.description || row.sku || row.rowId} +

    + {row.sku ? ( +

    + {row.sku} +

    + ) : null} +
    +
    +

    + {formatCurrency(change.previousPrice)} + + + {formatCurrency(change.newPrice)} + +

    +

    + at {change.markupPercent.toFixed(0)}% markup +

    +
    +
    + + {twoBases ? ( +
    + Price it from: + + +
    + ) : null} + +
    + + + {isUpdating ? ( + <> + Cost {formatCurrency(cost)} - the till will charge{" "} + {formatCurrency(change.newPrice)}. + + ) : ( + <>The till keeps charging {formatCurrency(change.previousPrice)}. + )} + + {up ? ( + + ) : ( + + )} +
    +
  • + ); + })} +
+ + ); +} diff --git a/src/lib/pos-price.test.ts b/src/lib/pos-price.test.ts new file mode 100644 index 0000000..7ad80cb --- /dev/null +++ b/src/lib/pos-price.test.ts @@ -0,0 +1,139 @@ +import { describe, expect, it } from "vitest"; + +import { + DEFAULT_MARKUP_PERCENT, + computePosPriceChange, + effectiveMarkupPercent, + priceFromCost, + roundCents, +} from "./pos-price"; +import { DEFAULT_MARKUP_PERCENT as SERVER_DEFAULT_MARKUP_PERCENT } from "./server/stock-default-markup-service"; + +describe("roundCents", () => { + it("rounds to cents the way a person would", () => { + expect(roundCents(11.325)).toBe(11.33); + expect(roundCents(11.324)).toBe(11.32); + }); + + it("is 0 for a non-finite number", () => { + expect(roundCents(Number.NaN)).toBe(0); + expect(roundCents(Number.POSITIVE_INFINITY)).toBe(0); + }); +}); + +describe("effectiveMarkupPercent", () => { + it("uses the item markup when it is set", () => { + expect(effectiveMarkupPercent({ markup: 30, cost: 100, selling: 130 })).toBe(30); + }); + + it("falls back to the margin implied by the current price", () => { + // No markup field, but cost 100 sells at 120 -> a 20% margin. + expect(effectiveMarkupPercent({ markup: 0, cost: 100, selling: 120 })).toBe(20); + }); + + it("falls back to the default when there is no price to learn from", () => { + expect(effectiveMarkupPercent({ markup: 0, cost: 0, selling: 0 })).toBe(80); + expect(effectiveMarkupPercent({})).toBe(80); + }); + + it("ignores a zero or negative markup field", () => { + expect(effectiveMarkupPercent({ markup: 0, cost: 100, selling: 120 })).toBe(20); + expect(effectiveMarkupPercent({ markup: -5, cost: 100, selling: 120 })).toBe(20); + }); +}); + +describe("priceFromCost", () => { + it("applies the markup", () => { + expect(priceFromCost(100, 80)).toBe(180); + expect(priceFromCost(167, 30)).toBe(217.1); + }); + + it("is 0 for a zero or negative cost", () => { + expect(priceFromCost(0, 80)).toBe(0); + expect(priceFromCost(-5, 80)).toBe(0); + }); +}); + +describe("computePosPriceChange", () => { + it("reports the price move when the cost changes", () => { + const change = computePosPriceChange({ + currentCost: 100, + currentPrice: 180, + markup: 80, + newCost: 120, + }); + expect(change.previousPrice).toBe(180); + expect(change.newPrice).toBe(216); + expect(change.delta).toBe(36); + expect(change.changed).toBe(true); + }); + + it("reports no change when the cost is the same", () => { + const change = computePosPriceChange({ + currentCost: 100, + currentPrice: 180, + markup: 80, + newCost: 100, + }); + expect(change.changed).toBe(false); + expect(change.newPrice).toBe(change.previousPrice); + }); + + it("reports a price decrease when the cost comes down", () => { + const change = computePosPriceChange({ + currentCost: 100, + currentPrice: 180, + markup: 80, + newCost: 80, + }); + expect(change.newPrice).toBe(144); + expect(change.delta).toBe(-36); + expect(change.changed).toBe(true); + }); + + it("treats a sub-cent difference as no change", () => { + // 100 at 80% is 180. A cost of 100.0001 would give 180.00018 - + // float noise, not a price move. + const change = computePosPriceChange({ + currentCost: 100, + currentPrice: 180, + markup: 80, + newCost: 100.0001, + }); + expect(change.changed).toBe(false); + }); + + it("prices a never-priced item at the default markup", () => { + const change = computePosPriceChange({ + currentCost: 0, + currentPrice: 0, + markup: 0, + newCost: 50, + }); + expect(change.markupPercent).toBe(DEFAULT_MARKUP_PERCENT); + expect(change.newPrice).toBe(90); + expect(change.changed).toBe(true); + }); + + it("prices an item with no markup field at its implied margin", () => { + // Hand-priced at 100 for a cost of 50 -> a 100% margin. A new cost + // of 60 should give 120, keeping the margin the shop set by hand. + const change = computePosPriceChange({ + currentCost: 50, + currentPrice: 100, + markup: 0, + newCost: 60, + }); + expect(change.markupPercent).toBe(100); + expect(change.newPrice).toBe(120); + }); +}); + +describe("the two markup constants agree", () => { + it("the client-safe default matches the server default", () => { + // The client module cannot import the server one (it pulls in the + // PB admin client), so the constant is duplicated. If the server + // rate ever changes, this fails and both are updated together. + expect(DEFAULT_MARKUP_PERCENT).toBe(SERVER_DEFAULT_MARKUP_PERCENT); + }); +}); diff --git a/src/lib/pos-price.ts b/src/lib/pos-price.ts new file mode 100644 index 0000000..1945159 --- /dev/null +++ b/src/lib/pos-price.ts @@ -0,0 +1,134 @@ +/** + * The price a POS item SHOULD carry for a given cost. + * + * This is the single source of truth for "what would the till charge if + * the cost were R X?". Both the collection-confirm popup (which shows + * the operator the old and new price) and the server-side write path + * (which persists the new price) import from here, so the number the + * operator approves is exactly the number that lands on the till. + * + * Why this lives in its own module and not in the service: + * + * - The popup is a CLIENT component. It has to compute the new price + * to render "R 217.10 -> R 251.30" without a round-trip, and the + * server has to compute the SAME number when it writes. A shared + * pure module is the only way those two can be guaranteed equal. + * - It is pure: no PB, no dates, no I/O. Trivially unit-testable. + * + * The markup rule, in order: + * + * 1. The POS item's own `markup` field, when it is set. That is the + * rate the shop already decided for that item, so honouring it + * keeps the shelf price consistent with the rest of the catalogue. + * 2. Otherwise the markup IMPLIED by the item's current cost and + * selling price ((selling - cost) / cost). An item priced by hand + * (no markup field) still has an effective margin; using it means + * a cost correction moves the price the same way it would have + * moved if the item had a markup field. + * 3. Otherwise the shop default (80%). Covers the items that have + * neither a markup nor a price yet - they need SOME rate to be + * priced at all. + * + * `DEFAULT_MARKUP_PERCENT` is duplicated from + * `stock-default-markup-service.ts` rather than imported, because that + * module is server-only (it imports the PB admin client) and this one + * has to be importable from a client component. The two constants are + * asserted to agree in `pos-price.test.ts`. + */ + +export const DEFAULT_MARKUP_PERCENT = 80; + +/** Round to cents the way a person would: 11.325 -> 11.33. */ +export function roundCents(n: number): number { + if (!Number.isFinite(n)) return 0; + return Math.round((n + Number.EPSILON) * 100) / 100; +} + +/** + * The markup rate to price a POS item at. + * + * @param markup The item's `markup` field, if it has one. + * @param cost The item's current cost price. + * @param selling The item's current selling price. + */ +export function effectiveMarkupPercent(args: { + markup?: number | null; + cost?: number | null; + selling?: number | null; +}): number { + const markup = Number(args.markup ?? 0); + if (Number.isFinite(markup) && markup > 0) return markup; + + const cost = Number(args.cost ?? 0); + const selling = Number(args.selling ?? 0); + if (cost > 0 && selling > 0) { + const implied = ((selling - cost) / cost) * 100; + if (Number.isFinite(implied)) return implied; + } + + return DEFAULT_MARKUP_PERCENT; +} + +/** + * The selling price for a cost, at a given markup rate. + * Returns 0 for a zero/absent cost - an unpriced line has no price. + */ +export function priceFromCost(cost: number, markupPercent: number): number { + if (!Number.isFinite(cost) || cost <= 0) return 0; + return roundCents(cost * (1 + markupPercent / 100)); +} + +/** + * One line of the price-confirmation popup: what the till charges now, + * what it would charge after the cost change, and how far apart they are. + */ +export type PosPriceChange = { + /** The POS item's current selling price. */ + previousPrice: number; + /** What the selling price becomes at the new cost. */ + newPrice: number; + /** The markup rate used to derive `newPrice`. */ + markupPercent: number; + /** True when `newPrice` and `previousPrice` differ by more than a cent. */ + changed: boolean; + /** newPrice - previousPrice. Negative when the cost came down. */ + delta: number; +}; + +/** + * Compare the price a POS item charges today against the price it would + * charge at a new cost. + * + * `newCost` is the cost being proposed. For a stock line that is the + * PO line's cost; for a customer line matched to a POS item by SKU it + * is that line's cost (the operator can choose which basis to use in + * the dialog, so the caller passes the cost it settled on). + */ +export function computePosPriceChange(args: { + /** The POS item's current cost. */ + currentCost?: number | null; + /** The POS item's current selling price. */ + currentPrice?: number | null; + /** The POS item's markup field, if any. */ + markup?: number | null; + /** The cost being proposed. */ + newCost: number; +}): PosPriceChange { + const previousPrice = roundCents(Number(args.currentPrice ?? 0)); + const markupPercent = effectiveMarkupPercent({ + markup: args.markup, + cost: args.currentCost, + selling: args.currentPrice, + }); + const newPrice = priceFromCost(args.newCost, markupPercent); + const delta = roundCents(newPrice - previousPrice); + return { + previousPrice, + newPrice, + markupPercent, + // A cent is the smallest unit anyone prices at; anything smaller is + // float noise, not a real price move. + changed: Math.abs(delta) >= 0.01, + delta, + }; +} diff --git a/src/lib/server/collection-confirm-service.test.ts b/src/lib/server/collection-confirm-service.test.ts index 5a541b7..27304dc 100644 --- a/src/lib/server/collection-confirm-service.test.ts +++ b/src/lib/server/collection-confirm-service.test.ts @@ -204,8 +204,13 @@ describe("listCollectedItems", () => { expect(stockOpts?.filter).toBe( 'status = "collected" && purchase.status != "cancelled" && (quantity_received < quantity || purchase.status != "received")', ); + // The stock item is expanded from THIS collection. + // `stock_purchases` has no `stock_item` relation, so the old + // `purchase.stock_item` never resolved and every stock row came + // back with no POS pricing. Verified against live PB on + // 2026-11-09. expect(stockOpts?.expand).toBe( - "purchase.supplier,purchase.stock_item,purchase.additional_attachments", + "purchase.supplier,purchase.additional_attachments,stock_item", ); // Order-items leg must NOT be called when we only returned stock rows. // (In the real code it IS called — but it returned empty.) diff --git a/src/lib/server/collection-confirm-service.ts b/src/lib/server/collection-confirm-service.ts index 7b62cd7..101afa9 100644 --- a/src/lib/server/collection-confirm-service.ts +++ b/src/lib/server/collection-confirm-service.ts @@ -75,6 +75,18 @@ export class CollectionConfirmValidationError extends Error { */ export type ConfirmRowSource = "customer" | "stock"; +/** + * The subset of a `stock_items` row the collection-confirm page needs + * to price a line against the till. + */ +export type PosItem = { + id: string; + sku?: string; + cost_price?: number | null; + selling_price?: number | null; + markup?: number | null; +}; + export type ConfirmRow = { id: string; /** @@ -166,6 +178,16 @@ export type ConfirmRow = { /** True when the line's supplier code matches a stock item (POS). */ in_pos?: boolean | null; pos_stock_item_id?: string | null; + /** + * The POS item's own cost price. For a stock row this is the same + * number as the line cost; for a customer row matched by SKU it is + * the catalogue cost, which can differ from the line's cost. + */ + pos_cost_price?: number | null; + /** What the till charges for the POS item today. */ + pos_selling_price?: number | null; + /** The POS item's markup field, when it has one. */ + pos_markup?: number | null; /** * Lightweight summary of the most recent `price_diff_report` entry * the `po-agent` wrote to this PO's `notes` field. Populated by @@ -345,6 +367,14 @@ export async function listCollectedItems(): Promise { sku?: string; name?: string; description?: string; + /** + * The POS item's own pricing. Carried here so the price + * confirmation popup can show what the till charges today + * without a second round-trip. See the expand note below. + */ + cost_price?: number | null; + selling_price?: number | null; + markup?: number | null; }; }; } @@ -364,7 +394,19 @@ export async function listCollectedItems(): Promise { // `confirmStockRows` having run first). filter: STOCK_ON_PAGE_FILTER, sort: "-created", - expand: "purchase.supplier,purchase.stock_item,purchase.additional_attachments", + // NOTE: the stock item is expanded from THIS collection, not + // from purchase. `stock_purchases` has no `stock_item` relation + // (only `supplier` and `booked_by`), so the old + // `purchase.stock_item` never resolved and `expand.stock_item` + // came back undefined on every row - which is why the POS + // pricing below was always missing. Verified against live PB + // on 2026-11-09. + // + // DO NOT add a `fields` option to this query. PocketBase + // applies `fields` to expanded relations too, so naming only + // the parent columns silently empties `expand` and the POS + // pricing disappears again - the same bug, one option away. + expand: "purchase.supplier,purchase.additional_attachments,stock_item", }) // Fail-soft: matches the page's `load()` at page.tsx:294 — a // stock-side outage must not wipe the customer-deliveries view. @@ -397,6 +439,10 @@ export async function listCollectedItems(): Promise { sku?: string; name?: string; description?: string; + /** The POS item's pricing, for the price-confirmation popup. */ + cost_price?: number | null; + selling_price?: number | null; + markup?: number | null; }; }; }> @@ -419,6 +465,13 @@ export async function listCollectedItems(): Promise { // Stock lines are already tied to a catalogue item. in_pos: true, pos_stock_item_id: s.stock_item || null, + // The POS item's own pricing, for the price-confirmation popup. + // A stock line IS the catalogue item, so the line cost and the + // POS item cost are the same number - there is no ambiguity here + // about which cost basis applies. + pos_cost_price: s.expand?.stock_item?.cost_price ?? null, + pos_selling_price: s.expand?.stock_item?.selling_price ?? null, + pos_markup: s.expand?.stock_item?.markup ?? null, assigned_at: s.assigned_at ?? null, // Stock rows have no `drv_collected_at`, so the card header uses // this as the "when was this last touched" fallback. @@ -680,18 +733,22 @@ async function attachAiFields(pb: AdminPb, rows: ConfirmRow[]): Promise { // 2. In POS: a stock_items row whose normalised SKU equals the line's. const customer = rows.filter((r) => r.source !== "stock"); const skus = [...new Set(customer.map((r) => (r.sku ?? "").trim()).filter((s) => s !== ""))]; - const posByCode = new Map(); + const posByCode = new Map(); let posFailed = false; if (skus.length > 0) { try { for (const group of chunked(skus)) { - const found = await pb.collection("stock_items").getFullList<{ id: string; sku?: string }>({ + // The POS item's pricing comes back with the row: the price + // confirmation popup needs "what the till charges today" to + // show the old-vs-new comparison, and one round-trip is + // cheaper than a second lookup per row. + const found = await pb.collection("stock_items").getFullList({ filter: group.map((s) => `sku ~ "${pbQuote(s)}"`).join(" || "), - fields: "id,sku", + fields: "id,sku,cost_price,selling_price,markup", }); for (const item of found) { const code = normaliseCode(item.sku); - if (code && !posByCode.has(code)) posByCode.set(code, item.id); + if (code && !posByCode.has(code)) posByCode.set(code, item); } } } catch { @@ -704,9 +761,16 @@ async function attachAiFields(pb: AdminPb, rows: ConfirmRow[]): Promise { row.pos_stock_item_id = null; continue; } - const id = posByCode.get(normaliseCode(row.sku)) ?? null; - row.in_pos = id !== null && normaliseCode(row.sku) !== ""; - row.pos_stock_item_id = row.in_pos ? id : null; + const posItem = posByCode.get(normaliseCode(row.sku)) ?? null; + row.in_pos = posItem !== null && normaliseCode(row.sku) !== ""; + row.pos_stock_item_id = row.in_pos ? posItem?.id ?? null : null; + // Carry the POS item's own pricing for the price-confirmation + // popup. A customer line's cost is that ONE order's cost, which + // may differ from the catalogue cost of the same SKU - the dialog + // shows both and lets the operator pick the basis. + row.pos_cost_price = posItem?.cost_price ?? null; + row.pos_selling_price = posItem?.selling_price ?? null; + row.pos_markup = posItem?.markup ?? null; } } diff --git a/src/lib/server/collection-pos-price-service.test.ts b/src/lib/server/collection-pos-price-service.test.ts new file mode 100644 index 0000000..b0bd7b4 --- /dev/null +++ b/src/lib/server/collection-pos-price-service.test.ts @@ -0,0 +1,213 @@ +import { describe, expect, it, vi } from "vitest"; + +import { + applyPosPriceUpdate, + applyPosPriceUpdates, + type PosPricePb, +} from "./collection-pos-price-service"; + +const actor = { id: "useropid1234567", name: "Gary" }; + +type Stub = { + pb: PosPricePb; + updates: Array<{ id: string; patch: Record }>; + ledgers: Array>; +}; + +function stubPb(opts: { + before?: Record; + getOneError?: Error; + updateError?: Error; + ledgerError?: Error; +} = {}): Stub { + const updates: Array<{ id: string; patch: Record }> = []; + const ledgers: Array> = []; + const before = opts.before ?? { + id: "abcdefghijklmno", + cost_price: 100, + selling_price: 180, + markup: 80, + cost_history: [], + }; + const pb: PosPricePb = { + collection: (name: string) => { + if (name === "stock_items") { + return { + getOne: vi.fn(async () => { + if (opts.getOneError) throw opts.getOneError; + return before; + }), + update: vi.fn(async (id: string, patch: Record) => { + if (opts.updateError) throw opts.updateError; + updates.push({ id, patch }); + return { ...before, ...patch }; + }), + create: vi.fn(), + }; + } + if (name === "stock_ledger") { + return { + getOne: vi.fn(), + update: vi.fn(), + create: vi.fn(async (row: Record) => { + if (opts.ledgerError) throw opts.ledgerError; + ledgers.push(row); + return { id: "ledger1", ...row }; + }), + }; + } + throw new Error("unexpected collection " + name); + }, + }; + return { pb, updates, ledgers }; +} + +describe("applyPosPriceUpdate", () => { + it("writes the new cost AND the new price, derived at the item markup", async () => { + const stub = stubPb(); + const result = await applyPosPriceUpdate( + { stockItemId: "abcdefghijklmno", cost: 120 }, + actor, + stub.pb, + ); + + expect(result.ok).toBe(true); + expect(result.previousPrice).toBe(180); + expect(result.newPrice).toBe(216); + expect(result.cost).toBe(120); + expect(stub.updates).toHaveLength(1); + // Cost and price move together - a price with a stale cost behind + // it would mis-price the next markup run. + expect(stub.updates[0].patch).toMatchObject({ + cost_price: 120, + selling_price: 216, + }); + }); + + it("never writes a markup - the operator rate is theirs", async () => { + const stub = stubPb(); + await applyPosPriceUpdate( + { stockItemId: "abcdefghijklmno", cost: 120 }, + actor, + stub.pb, + ); + expect(stub.updates[0].patch).not.toHaveProperty("markup"); + }); + + it("appends a zero-quantity cost_history entry and preserves the old history", async () => { + const stub = stubPb({ + before: { + id: "abcdefghijklmno", + cost_price: 100, + selling_price: 180, + markup: 80, + cost_history: [ + { + cost_price: 90, + quantity: 5, + purchased_at: "2026-01-01T00:00:00.000Z", + purchase_id: "po1", + }, + ], + }, + }); + await applyPosPriceUpdate( + { stockItemId: "abcdefghijklmno", cost: 120 }, + actor, + stub.pb, + ); + const history = stub.updates[0].patch.cost_history as Array< + Record + >; + expect(history).toHaveLength(2); + expect(history[0].cost_price).toBe(90); + // A reprice is not a receipt: zero quantity, no purchase id. + expect(history[1]).toMatchObject({ cost_price: 120, quantity: 0, purchase_id: "" }); + }); + + it("writes a zero-quantity adjustment ledger row", async () => { + const stub = stubPb(); + await applyPosPriceUpdate( + { stockItemId: "abcdefghijklmno", cost: 120 }, + actor, + stub.pb, + ); + expect(stub.ledgers).toHaveLength(1); + expect(stub.ledgers[0]).toMatchObject({ + stock_item: "abcdefghijklmno", + change_qty: 0, + unit_cost: 120, + reason: "adjustment", + }); + }); + + it("prices an item with no markup field at its implied margin", async () => { + const stub = stubPb({ + before: { + id: "abcdefghijklmno", + cost_price: 50, + selling_price: 100, + markup: 0, + }, + }); + const result = await applyPosPriceUpdate( + { stockItemId: "abcdefghijklmno", cost: 60 }, + actor, + stub.pb, + ); + // 100 for a cost of 50 is a 100% margin, so 60 -> 120. + expect(result.newPrice).toBe(120); + }); + + it("rejects a negative cost without writing anything", async () => { + const stub = stubPb(); + const result = await applyPosPriceUpdate( + { stockItemId: "abcdefghijklmno", cost: -5 }, + actor, + stub.pb, + ); + expect(result.ok).toBe(false); + expect(result.reason).toBe("validation_error"); + expect(stub.updates).toHaveLength(0); + }); + + it("reports a missing POS item without writing anything", async () => { + const stub = stubPb({ getOneError: new Error("404 not found") }); + const result = await applyPosPriceUpdate( + { stockItemId: "abcdefghijklmno", cost: 120 }, + actor, + stub.pb, + ); + expect(result.ok).toBe(false); + expect(result.reason).toBe("not_found"); + expect(stub.updates).toHaveLength(0); + }); + + it("reports a failed ledger write rather than pretending it succeeded", async () => { + const stub = stubPb({ ledgerError: new Error("ledger down") }); + const result = await applyPosPriceUpdate( + { stockItemId: "abcdefghijklmno", cost: 120 }, + actor, + stub.pb, + ); + expect(result.ok).toBe(false); + expect(result.message).toContain("ledger"); + }); +}); + +describe("applyPosPriceUpdates", () => { + it("keeps going after one row fails", async () => { + const stub = stubPb(); + const results = await applyPosPriceUpdates( + [ + { stockItemId: "abcdefghijklmno", cost: 120 }, + { stockItemId: "anotherone1234", cost: 50 }, + ], + actor, + stub.pb, + ); + expect(results).toHaveLength(2); + expect(results.every((r) => r.ok)).toBe(true); + expect(stub.updates).toHaveLength(2); + }); +}); diff --git a/src/lib/server/collection-pos-price-service.ts b/src/lib/server/collection-pos-price-service.ts new file mode 100644 index 0000000..499c358 --- /dev/null +++ b/src/lib/server/collection-pos-price-service.ts @@ -0,0 +1,282 @@ +import { getAdminPocketBase } from "./pocketbase-admin"; +import { computePosPriceChange, roundCents } from "@/lib/pos-price"; + +/** + * Apply an operator-approved POS price change. + * + * The problem this solves + * ----------------------- + * A POS item (a row in `stock_items`) carries a `selling_price` - what + * the till charges. New stock arrives at a new cost, and the sensible + * new price is that cost plus the item's markup. But nothing in the app + * decided THAT, so the question kept getting answered by accident: + * a bulk import wrote a spreadsheet's selling price straight over the + * catalogue, and the /stock pricing grid silently recomputed the + * selling price whenever its cost cell was edited. + * + * So the operator confirms the price instead. This service is the + * write side of that decision: it takes the cost the operator chose + * and the markup already on the item, works out the new price, and + * writes cost + price + a cost_history entry + a stock_ledger row. + * + * Why cost AND price together + * ---------------------------- + * Writing only the selling price would leave the item's cost stale, + * so the NEXT markup run would compute from a base that no longer + * matched reality. Cost and price move as a pair or not at all. + * + * What it does NOT touch + * ---------------------- + * - `current_qty` - no stock moved. + * - `markup` - the operator's rate for the item is their business. + * We price at THEIR markup; we do not replace it. + * - The order/stock-purchase line's own cost - that is the caller's + * job (the cost pencil already wrote it). This service only + * re-prices the TILL side. + * + * The audit trail + * --------------- + * Two rows are written, mirroring what `stock-pricing-service.ts` does + * for a manual price edit: + * + * - `stock_items.cost_history` gains an entry, so the price change + * is visible next to the purchase history. + * - `stock_ledger` gains a ZERO-quantity row with reason + * "adjustment" - a price move is not a stock movement, but it must + * still leave a trace. The existing ledger reasons are + * purchase_surplus | stock_order | customer_fulfillment | + * adjustment | return, and "adjustment" is the one that fits. + */ + +export type PosPriceUpdate = { + /** The `stock_items` row id. */ + stockItemId: string; + /** + * The cost the operator approved. This is the basis the new price + * is computed from. + */ + cost: number; + /** + * A human-readable note for the ledger, e.g. "Collection Confirm". + * Falls back to a generic label. + */ + note?: string; +}; + +export type PosPriceUpdateActor = { + id: string; + name: string; +}; + +/** + * The PB surface this service needs. Narrow on purpose so the service + * can be driven by a stub in tests and by the admin client in + * production. + */ +export type PosPricePb = { + collection: (name: string) => { + getOne: (id: string) => Promise; + update: (id: string, patch: Record) => Promise; + create: (row: Record) => Promise; + }; +}; + +export type PosPriceUpdateResult = { + ok: boolean; + /** The POS item id. */ + stockItemId: string; + /** The price the till charged before. */ + previousPrice?: number; + /** The price the till charges now. */ + newPrice?: number; + /** The cost the item now carries. */ + cost?: number; + /** Why it failed, when ok is false. */ + reason?: "not_found" | "validation_error" | "pb_error"; + message?: string; +}; + +/** + * Apply one approved price update. + * + * Per-row isolation: a failure on one item never rolls back or blocks + * another. The caller reports each outcome to the operator. + */ +export async function applyPosPriceUpdate( + update: PosPriceUpdate, + actor: PosPriceUpdateActor, + pbOverride?: PosPricePb, +): Promise { + if (!update.stockItemId || typeof update.stockItemId !== "string") { + return { + ok: false, + stockItemId: String(update.stockItemId ?? ""), + reason: "validation_error", + message: "stockItemId is required", + }; + } + if ( + typeof update.cost !== "number" || + !Number.isFinite(update.cost) || + update.cost < 0 + ) { + return { + ok: false, + stockItemId: update.stockItemId, + reason: "validation_error", + message: "cost must be a finite number >= 0", + }; + } + + const pb = pbOverride ?? (await getAdminPocketBase()); + + // 1. Read the current row. The markup and the CURRENT selling price + // both come from here - never from the client - so a stale or + // tampered request cannot set an arbitrary price. + let item: { + id: string; + cost_price?: number | null; + selling_price?: number | null; + markup?: number | null; + cost_history?: unknown; + }; + try { + item = (await pb + .collection("stock_items") + .getOne(update.stockItemId)) as { + id: string; + cost_price?: number | null; + selling_price?: number | null; + markup?: number | null; + cost_history?: unknown; + }; + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + if (/not found|404/i.test(message)) { + return { + ok: false, + stockItemId: update.stockItemId, + reason: "not_found", + message: "POS item not found - it may have been deleted", + }; + } + return { + ok: false, + stockItemId: update.stockItemId, + reason: "pb_error", + message: message, + }; + } + + // 2. Compute the new price from the operator's cost and the item's + // own markup, using the shared pure helper so the number shown in + // the dialog is exactly the number written here. + const change = computePosPriceChange({ + currentCost: item.cost_price, + currentPrice: item.selling_price, + markup: item.markup, + newCost: update.cost, + }); + + const newCost = roundCents(update.cost); + const newPrice = change.newPrice; + + // 3. Append to cost_history, preserving whatever is already there. + const history = Array.isArray(item.cost_history) + ? item.cost_history.filter( + (entry): entry is { cost_price: number; quantity: number; purchased_at: string; purchase_id: string } => + Boolean(entry) && + typeof entry === "object" && + typeof (entry as { cost_price?: unknown }).cost_price === "number", + ) + : []; + + const entry = { + cost_price: newCost, + // Zero quantity: this is a REPRICE, not a receipt. No stock moved. + quantity: 0, + purchased_at: new Date().toISOString(), + // Empty purchase id: this price change did not come from a purchase + // order. Same convention as the default-markup seed entry. + purchase_id: "", + }; + + // 4. Write the item: new cost + new price + the appended history. + try { + await pb.collection("stock_items").update(update.stockItemId, { + cost_price: newCost, + selling_price: newPrice, + cost_history: [...history, entry], + }); + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + return { + ok: false, + stockItemId: update.stockItemId, + reason: "pb_error", + message: message, + }; + } + + // 5. Ledger row - a zero-quantity "adjustment". A price move is not a + // stock movement, but it must still leave a trace. + try { + await pb.collection("stock_ledger").create({ + stock_item: update.stockItemId, + change_qty: 0, + unit_cost: newCost, + reason: "adjustment", + reference_type: update.note || "price_confirmation", + notes: + (update.note || "Price updated") + + ": price " + + change.previousPrice.toFixed(2) + + " -> " + + newPrice.toFixed(2) + + " at cost " + + newCost.toFixed(2) + + (actor?.name ? " by " + actor.name : ""), + created_by: actor?.id || undefined, + }); + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + return { + ok: false, + stockItemId: update.stockItemId, + previousPrice: change.previousPrice, + newPrice, + cost: newCost, + reason: "pb_error", + message: + "Updated the POS price but failed to write the stock_ledger audit row: " + + message, + }; + } + + return { + ok: true, + stockItemId: update.stockItemId, + previousPrice: change.previousPrice, + newPrice, + cost: newCost, + }; +} + +/** + * Apply a batch of approved price updates. + * + * Per-row isolation: every row is attempted, and the results array has + * one entry per input in the same order. A row that fails does not + * stop the others. + */ +export async function applyPosPriceUpdates( + updates: PosPriceUpdate[], + actor: PosPriceUpdateActor, + pbOverride?: PosPricePb, +): Promise { + const results: PosPriceUpdateResult[] = []; + for (const update of updates) { + results.push(await applyPosPriceUpdate(update, actor, pbOverride)); + } + return results; +}