feat(orders): bypass margin gate for non-invoiceable extras

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>
This commit is contained in:
Dave 2026-08-30 17:21:28 +02:00
parent 50e75e6955
commit 8b23cc1b6a
3 changed files with 92 additions and 1 deletions

View file

@ -641,7 +641,19 @@ export default function ProcessOrderPage({ params }: { params: Promise<{ id: str
currentItem?.item_status === "approval_rejected" ||
(currentItem?.item_status === "processed" && !pricingUnchanged);
// Task 4 of the non-invoiceable-order-extras plan. An excluded
// line has selling_price forced to 0 (Task 2), so computedMarkup
// would always be -100 and the gate below would wedge every
// extra into approval_needed. Bypass the gate entirely when
// the flag is set — the row's whole point is "we don't bill the
// customer for this", so margin is meaningless. The server-side
// `requestOrderItemMarginApproval` independently refuses to
// gate an excluded item, so this client bypass can't be
// weaponised to push an excluded row through the approval queue.
const isExcluded = !!currentItem?.exclude_from_invoice;
if (
!isExcluded &&
approvalEligibleState &&
committed.cost_price > 0 &&
Number(committed.selling_price) > 0 &&
@ -2068,8 +2080,12 @@ function ItemCard({
);
// Margin color — green ≥ 30%, amber ≥ LOW_MARGIN_THRESHOLD (20%),
// red below.
// red below. Suppressed for excluded rows: their selling is forced
// to 0 so the markup is always -100, which would render red on
// every extra. The "Not invoiced" badge replaces the price chip
// visually.
const marginColor = (() => {
if (item.exclude_from_invoice) return "text-muted-foreground";
if (markupPct === null) return "text-muted-foreground";
if (markupPct < LOW_MARGIN_THRESHOLD * 100) return "text-destructive";
if (markupPct < 30) return "text-warn-foreground dark:text-warn";

View file

@ -233,4 +233,59 @@ describe("order-item-margin-approval-service", () => {
}),
).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();
});
});

View file

@ -161,6 +161,26 @@ export async function requestOrderItemMarginApproval(input: {
);
}
// Task 4 of the non-invoiceable-order-extras plan. Excluded lines
// have selling forced to 0 by the server (createOrderItemForOrder)
// and bypass the client-side margin gate (the process page short-
// circuits the approval prompt when `exclude_from_invoice` is set).
// This server-side guard is the 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 `excluded_from_invoice`
// error rather than the misleading `missing_prices` one — both
// are 422s but only the first tells the operator what to do.
if (current.exclude_from_invoice === true) {
throw new MarginApprovalRequestError(
"Excluded-from-invoice items do not require margin approval — save them straight through.",
422,
"excluded_from_invoice",
);
}
const costPrice = Number(input.draft.cost_price ?? current.cost_price ?? 0);
const sellingPrice = Number(
input.draft.selling_price ?? current.selling_price ?? 0,