Task 4 of the non-invoiceable-order-extras plan. The page-level margin gate (process/page.tsx) short-circuits when the item has exclude_from_invoice. An excluded line has selling_price forced to 0 (Task 2), so the computed markup is always -100 and the gate would wedge every extra into approval_needed and block the order. Bypassing the gate keeps the operational flow unblocked. Suppressed low-margin colouring for excluded rows — a red '-100%' chip on every extra is noise. The 'Not invoiced' badge (T3) replaces the price chip visually. Server-side guard in requestOrderItemMarginApproval refuses to gate an excluded item with code 'excluded_from_invoice' (status 422). Belt-and-braces: the MCP path can't weaponise the approval queue to push an excluded row through manager review. Checked BEFORE the missing_prices guard so an excluded item with selling=0 throws the descriptive error rather than the misleading missing_prices one. 2 tests + 1 mutation-test prove both sides of the bypass: - excluded: throws 'excluded_from_invoice', webhook NOT fired - non-excluded: proceeds to approval_needed, webhook DOES fire - mutation (guard disabled): the excluded test fails Co-Authored-By: Claude <noreply@anthropic.com>
291 lines
9.7 KiB
TypeScript
291 lines
9.7 KiB
TypeScript
import { beforeEach, describe, expect, it, vi } from "vitest";
|
|
|
|
const { getAdminPocketBaseMock, getOrderItemsCollectionNameMock } = vi.hoisted(
|
|
() => ({
|
|
getAdminPocketBaseMock: vi.fn(),
|
|
getOrderItemsCollectionNameMock: vi.fn(() => "order_items"),
|
|
}),
|
|
);
|
|
|
|
vi.mock("@/lib/server/pocketbase-admin", () => ({
|
|
getAdminPocketBase: getAdminPocketBaseMock,
|
|
}));
|
|
vi.mock("@/lib/server/orders-service", () => ({
|
|
getOrderItemsCollectionName: getOrderItemsCollectionNameMock,
|
|
}));
|
|
|
|
import {
|
|
calculateOrderItemMarkup,
|
|
requestOrderItemMarginApproval,
|
|
} from "./order-item-margin-approval-service";
|
|
|
|
const currentItem = {
|
|
id: "item-1",
|
|
order: "order-1",
|
|
description: "HSS drill bit",
|
|
po_description: "HSS drill bit",
|
|
quantity: 2,
|
|
order_qty: 2,
|
|
selling_price: 50,
|
|
cost_price: 25,
|
|
supplier: "supplier-1",
|
|
item_status: "pending",
|
|
purchase_order: "",
|
|
expand: {
|
|
order: { id: "order-1", order_number: "ORD-001" },
|
|
supplier: { id: "supplier-1", suppliername: "Steel Co" },
|
|
},
|
|
};
|
|
|
|
describe("order-item-margin-approval-service", () => {
|
|
const itemGetOne = vi.fn();
|
|
const itemUpdate = vi.fn();
|
|
const supplierGetOne = vi.fn();
|
|
|
|
beforeEach(() => {
|
|
vi.clearAllMocks();
|
|
vi.stubEnv(
|
|
"ORDER_MARKUP_APPROVAL_WEBHOOK_URL",
|
|
"https://n8n.example/webhook/order-markup-approval",
|
|
);
|
|
itemGetOne.mockResolvedValue(structuredClone(currentItem));
|
|
itemUpdate.mockImplementation(async (_id: string, patch: Record<string, unknown>) => ({
|
|
...currentItem,
|
|
...patch,
|
|
expand: currentItem.expand,
|
|
}));
|
|
supplierGetOne.mockResolvedValue({ id: "supplier-1", suppliername: "Steel Co" });
|
|
getAdminPocketBaseMock.mockResolvedValue({
|
|
collection(name: string) {
|
|
if (name === "order_items") {
|
|
return { getOne: itemGetOne, update: itemUpdate };
|
|
}
|
|
if (name === "suppliers") return { getOne: supplierGetOne };
|
|
throw new Error(`unexpected collection ${name}`);
|
|
},
|
|
});
|
|
vi.stubGlobal(
|
|
"fetch",
|
|
vi.fn().mockResolvedValue(new Response(null, { status: 200 })),
|
|
);
|
|
});
|
|
|
|
it("calculates markup on cost, not gross margin", () => {
|
|
expect(calculateOrderItemMarkup(100, 118)).toBe(18);
|
|
expect(calculateOrderItemMarkup(100, 122)).toBe(22);
|
|
});
|
|
|
|
it("saves the draft as approval_needed and fires the webhook", async () => {
|
|
const result = await requestOrderItemMarginApproval({
|
|
itemId: "item-1",
|
|
source: "order_sprint",
|
|
requestedBy: "operator@example.com",
|
|
dashboardBaseUrl: "https://wfdash.apphub.co.za/",
|
|
draft: {
|
|
supplier: "supplier-1",
|
|
cost_price: 42,
|
|
selling_price: 50,
|
|
order_qty: 2,
|
|
po_description: "HSS drill bit",
|
|
notes: "Review with manager",
|
|
},
|
|
});
|
|
|
|
expect(itemUpdate).toHaveBeenCalledWith(
|
|
"item-1",
|
|
expect.objectContaining({
|
|
cost_price: 42,
|
|
selling_price: 50,
|
|
markup: 19.05,
|
|
markup_percent: 19.05,
|
|
item_status: "approval_needed",
|
|
}),
|
|
{ expand: "order,supplier" },
|
|
);
|
|
expect(fetch).toHaveBeenCalledWith(
|
|
"https://n8n.example/webhook/order-markup-approval",
|
|
expect.objectContaining({
|
|
method: "POST",
|
|
body: expect.stringContaining('"orderItemId":"item-1"'),
|
|
}),
|
|
);
|
|
expect(result.webhookFired).toBe(true);
|
|
expect(result.markup).toBe(19.05);
|
|
});
|
|
|
|
it("clamps negative stored markup while preserving the real value for approval", async () => {
|
|
const result = await requestOrderItemMarginApproval({
|
|
itemId: "item-1",
|
|
source: "order_process",
|
|
requestedBy: "operator@example.com",
|
|
draft: { cost_price: 60, selling_price: 50 },
|
|
});
|
|
|
|
expect(itemUpdate).toHaveBeenCalledWith(
|
|
"item-1",
|
|
expect.objectContaining({
|
|
markup: 0,
|
|
markup_percent: 0,
|
|
item_status: "approval_needed",
|
|
}),
|
|
{ expand: "order,supplier" },
|
|
);
|
|
expect(result.markup).toBe(-16.67);
|
|
expect(fetch).toHaveBeenCalledWith(
|
|
"https://n8n.example/webhook/order-markup-approval",
|
|
expect.objectContaining({
|
|
body: expect.stringContaining('"markup":-16.67'),
|
|
}),
|
|
);
|
|
});
|
|
it("keeps the item blocked when the webhook fails", async () => {
|
|
// The webhook is now fire-and-forget — we don't propagate its
|
|
// status into the synchronous response (it would force the
|
|
// operator to wait for a notification roundtrip). The contract is:
|
|
// PB state is saved first, the item is blocked, the webhook is
|
|
// dispatched in the background, and server-side logs capture any
|
|
// failure. This test pins the "PB first" half of that contract.
|
|
vi.mocked(fetch).mockResolvedValueOnce(new Response("bad gateway", { status: 502 }));
|
|
|
|
const result = await requestOrderItemMarginApproval({
|
|
itemId: "item-1",
|
|
source: "order_process",
|
|
requestedBy: "operator@example.com",
|
|
draft: { cost_price: 45, selling_price: 50 },
|
|
});
|
|
|
|
expect(itemUpdate).toHaveBeenCalledWith(
|
|
"item-1",
|
|
expect.objectContaining({ item_status: "approval_needed" }),
|
|
{ expand: "order,supplier" },
|
|
);
|
|
expect(result.item).toEqual(
|
|
expect.objectContaining({ item_status: "approval_needed" }),
|
|
);
|
|
// Background dispatch always reports "fired" from the caller's
|
|
// perspective — the response shape is intentionally synchronous
|
|
// success so the UI toast can fire immediately.
|
|
expect(result.webhookFired).toBe(true);
|
|
});
|
|
|
|
it("does not block the response while the webhook is slow", async () => {
|
|
// Operators were paying the full 15s WEBHOOK_TIMEOUT_MS every time
|
|
// the n8n webhook lagged, because the service awaited the fetch
|
|
// before returning. PB state is already saved (item blocked) —
|
|
// the webhook is a best-effort notification, not a precondition
|
|
// for the toast. Dispatch it in the background and return
|
|
// immediately so the UI doesn't hang.
|
|
const SLOW = 5_000;
|
|
vi.mocked(fetch).mockImplementationOnce(
|
|
() => new Promise<Response>((resolve) => setTimeout(() => resolve(new Response(null, { status: 200 })), SLOW)),
|
|
);
|
|
|
|
const start = Date.now();
|
|
const result = await requestOrderItemMarginApproval({
|
|
itemId: "item-1",
|
|
source: "order_process",
|
|
requestedBy: "operator@example.com",
|
|
draft: { cost_price: 45, selling_price: 50 },
|
|
});
|
|
const elapsed = Date.now() - start;
|
|
|
|
// Far less than the 5s the fake webhook would take if we were
|
|
// awaiting it. We give some headroom for test jitter but the
|
|
// point is: this is not the 15s the operator used to see.
|
|
expect(elapsed).toBeLessThan(1_000);
|
|
expect(itemUpdate).toHaveBeenCalledWith(
|
|
"item-1",
|
|
expect.objectContaining({ item_status: "approval_needed" }),
|
|
{ expand: "order,supplier" },
|
|
);
|
|
expect(result.webhookFired).toBe(true);
|
|
});
|
|
|
|
it("does not create an approval request at or above 22%", async () => {
|
|
await expect(
|
|
requestOrderItemMarginApproval({
|
|
itemId: "item-1",
|
|
source: "order_sprint",
|
|
requestedBy: "operator@example.com",
|
|
draft: { cost_price: 100, selling_price: 122 },
|
|
}),
|
|
).rejects.toMatchObject({
|
|
status: 409,
|
|
code: "approval_not_required",
|
|
});
|
|
expect(itemUpdate).not.toHaveBeenCalled();
|
|
expect(fetch).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it("does not allow an item that already has a purchase order", async () => {
|
|
itemGetOne.mockResolvedValueOnce({
|
|
...currentItem,
|
|
item_status: "processed",
|
|
purchase_order: "po-1",
|
|
});
|
|
|
|
await expect(
|
|
requestOrderItemMarginApproval({
|
|
itemId: "item-1",
|
|
source: "order_process",
|
|
requestedBy: "operator@example.com",
|
|
draft: { cost_price: 45, selling_price: 50 },
|
|
}),
|
|
).rejects.toMatchObject({ status: 409, code: "invalid_status" });
|
|
});
|
|
|
|
// Task 4 of the non-invoiceable-order-extras plan. The process
|
|
// page bypasses the margin gate client-side when the row has
|
|
// `exclude_from_invoice` (Task 2 forces selling_price to 0, so the
|
|
// gate would always fire otherwise). These tests prove the server
|
|
// refuses anyway — belt-and-braces against the MCP path
|
|
// weaponising the approval queue to push an excluded row through
|
|
// manager review.
|
|
|
|
it("refuses to gate an item flagged exclude_from_invoice, even with low-margin prices", async () => {
|
|
itemGetOne.mockResolvedValue({
|
|
...currentItem,
|
|
cost_price: 50,
|
|
selling_price: 0,
|
|
exclude_from_invoice: true,
|
|
});
|
|
|
|
await expect(
|
|
requestOrderItemMarginApproval({
|
|
itemId: "item-1",
|
|
source: "order_process",
|
|
requestedBy: "operator@example.com",
|
|
draft: { cost_price: 50, selling_price: 0 },
|
|
}),
|
|
).rejects.toMatchObject({
|
|
status: 422,
|
|
code: "excluded_from_invoice",
|
|
});
|
|
|
|
// The webhook must NOT fire — the row never reached approval.
|
|
expect(vi.mocked(fetch)).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it("does NOT refuse a non-excluded item with the same prices (proves the bypass is keyed on the flag)", async () => {
|
|
itemGetOne.mockResolvedValue({
|
|
...currentItem,
|
|
cost_price: 50,
|
|
selling_price: 5,
|
|
// exclude_from_invoice deliberately absent / false
|
|
});
|
|
|
|
const result = await requestOrderItemMarginApproval({
|
|
itemId: "item-1",
|
|
source: "order_process",
|
|
requestedBy: "operator@example.com",
|
|
draft: { cost_price: 50, selling_price: 5 },
|
|
});
|
|
|
|
// The function proceeds through the existing flow — item flips
|
|
// to approval_needed, webhook fires. (Selling is > 0 here so
|
|
// we don't trip the missing_prices guard; the test point is
|
|
// only that the excluded_from_invoice guard does NOT fire.)
|
|
expect(result.item.item_status).toBe("approval_needed");
|
|
expect(vi.mocked(fetch)).toHaveBeenCalled();
|
|
});
|
|
});
|