diff --git a/src/app/(app)/orders/[id]/process/page.tsx b/src/app/(app)/orders/[id]/process/page.tsx index ad51c53..b87a49f 100644 --- a/src/app/(app)/orders/[id]/process/page.tsx +++ b/src/app/(app)/orders/[id]/process/page.tsx @@ -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"; diff --git a/src/lib/server/order-item-margin-approval-service.test.ts b/src/lib/server/order-item-margin-approval-service.test.ts index 63bdd05..0942778 100644 --- a/src/lib/server/order-item-margin-approval-service.test.ts +++ b/src/lib/server/order-item-margin-approval-service.test.ts @@ -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(); + }); }); diff --git a/src/lib/server/order-item-margin-approval-service.ts b/src/lib/server/order-item-margin-approval-service.ts index 94567d0..30ef89d 100644 --- a/src/lib/server/order-item-margin-approval-service.ts +++ b/src/lib/server/order-item-margin-approval-service.ts @@ -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,