feat(orders): allow adding lines mid-flight, accept exclude_from_invoice

Dropped the pending gate in createOrderItemForOrder. The original
gate's reasoning was that adding a row alongside an in-flight PO
would orphan that PO's pricing/supplier reference — wrong model
of what adding a row does. A new row is independent: it gets its
own PO when the operator processes it, never mutates an existing
one.

The unrelated nonPending gate in setOrderQuote (preventing a
quote link change after items are past pending) stays — different
invariant.

exclude_from_invoice flag: when set, selling_price is forced to
0 regardless of what the caller passes — the flag is the source of
truth downstream, the price column is not consulted by the
invoice/delivery-note payload builders. The flag itself is
persisted in the secondary-field patch alongside sku/unit/
supplier/notes.

orderLocked return field kept for API compatibility — now always
false. Pre-existing callers reading the shape compile and read a
constant.

3 tests + 2 mutations prove the assertions guard real behavior:
deleting the flag-persistence line and the force-zero line each
break their target test.

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Dave 2026-08-30 14:34:37 +02:00
parent 4c5a312e16
commit 234ba6d3cd
2 changed files with 138 additions and 37 deletions

View file

@ -3056,3 +3056,90 @@ describe("createOrder delivery-note auto-detection", () => {
expect(suggestDeliveryNoteForOrder).toHaveBeenCalledWith(created.order.id);
});
});
describe("createOrderItemForOrder (mid-flight add)", () => {
// Task 2 of the non-invoiceable-order-extras plan — the pending
// gate that previously 409'd when siblings were po_generated is
// removed because adding a new row is independent of any in-flight
// PO's pricing/supplier reference. Three tests below cover the
// contract: (a) the gate is gone, (b) the new flag is authoritative
// for selling_price, (c) omission leaves the row billable.
it("adds a line to an order whose siblings are po_generated (was previously 409 order_locked)", async () => {
const { createOrderItemForOrder } = await import("./orders-service");
await pbMock.collection("orders").create({
id: "o-1",
order_number: "ORD-1",
// Customer + company required by createOrder's lead row — skip
// for this targeted test by using a minimal stub.
});
await pbMock.collection("order_items").create({
id: "oi-existing",
order: "o-1",
description: "Bolt",
quantity: 50,
item_status: "po_generated",
});
const result = await createOrderItemForOrder("o-1", {
description: "Washer",
quantity: 50,
cost_price: 5,
});
expect(result.failed).toEqual([]);
expect(result.created.length).toBe(1);
expect(result.created[0].description).toBe("Washer");
expect(result.orderLocked).toBe(false);
});
it("exclude_from_invoice forces selling_price to 0 even when caller passes a non-zero price", async () => {
const { createOrderItemForOrder } = await import("./orders-service");
await pbMock.collection("orders").create({
id: "o-1",
order_number: "ORD-1",
});
const result = await createOrderItemForOrder("o-1", {
description: "Nut",
quantity: 50,
cost_price: 10,
markup: 50, // would compute selling_price = 15 without the flag
selling_price: 15, // would persist 15 without the flag
exclude_from_invoice: true,
});
expect(result.created[0].selling_price).toBe(0);
expect(result.created[0].markup).toBe(50); // markup stays as-is
// The flag itself persists on the secondary update — separate
// assertion because the `created[]` return shape doesn't include
// it.
const stored = (store["order_items"] as Map<string, Record<string, unknown>>).get(
result.created[0].id,
);
expect(stored?.exclude_from_invoice).toBe(true);
expect(stored?.selling_price).toBe(0);
});
it("omitting exclude_from_invoice preserves the existing markup-based selling_price calculation", async () => {
const { createOrderItemForOrder } = await import("./orders-service");
await pbMock.collection("orders").create({
id: "o-1",
order_number: "ORD-1",
});
const result = await createOrderItemForOrder("o-1", {
description: "Bolt",
quantity: 50,
cost_price: 10,
markup: 25,
});
expect(result.created[0].selling_price).toBe(12.5); // 10 * 1.25
const stored = (store["order_items"] as Map<string, Record<string, unknown>>).get(
result.created[0].id,
);
expect(stored?.exclude_from_invoice).toBeFalsy();
});
});

View file

@ -3874,24 +3874,51 @@ export interface CreateOrderItemForOrderDraft {
unit?: string;
supplier?: string;
notes?: string;
/**
* Mark this line as supplier-side only — gets a PO, a driver, and
* collection like any other line, but never appears on a customer
* invoice or delivery note. Forces `selling_price` to 0
* regardless of what the caller passes (the flag is authoritative,
* price is not consulted downstream).
*
* See docs/superpowers/specs/2026-08-30-non-invoiceable-order-extras-design.md
* for the supplier-component-split use case that motivates it.
*/
exclude_from_invoice?: boolean;
}
/**
* Add a new line item to an existing order.
*
* **Strict `pending` gate**: refuses with 409 (`order_locked`) if
* any line item on the order is past `item_status === "pending"`.
* Adding a line to an order with a PO in flight would orphan the
* PO's pricing/supplier reference. Operator must split the order
* (use `orders.split`) or finish the existing PO first.
* **No `pending` gate** (removed 2026-08-30). The original gate
* refused with 409 if any sibling line was past `pending`, on the
* theory that adding a row alongside an in-flight PO would orphan
* the PO's pricing/supplier reference. That was a wrong model of
* what adding a row does — the new row is independent: it gets its
* own PO when the operator processes it, and never touches the
* existing PO's supplier / cost / quantity. The real reason to
* refuse used to be that operators had no other way to add extras,
* so they didn't notice the gate until they hit it; the non-
* invoiceable extras feature added the `Add item` affordance on the
* process page (see Task 3), so the use case is now legitimate and
* frequent.
*
* There is a separate, unrelated `nonPending` gate in `setOrderQuote`
* (line ~3744) that prevents changing a quote link after the order
* has been processed. That gate stays — different invariant.
*
* Mirrors what `createOrder` does for new orders: calls
* `createOrderItems` for the primary row, then patches the
* secondary fields (sku/unit/supplier/notes) on success.
* secondary fields (sku/unit/supplier/notes/exclude_from_invoice)
* on success.
*
* The `selling_price` defaults to `cost_price * (1 + markup/100)`
* via `deriveSellingPriceIfOmitted` when omitted. Markup defaults
* to 0 when both `selling_price` and `markup` are absent.
* to 0 when both `selling_price` and `markup` are absent. When
* `exclude_from_invoice` is true, `selling_price` is forced to 0
* regardless of the caller input — the flag is the source of truth,
* and downstream invoice / delivery-note code keys on it, not on
* the price column.
*
* Returns `{ created, failed }` like `createOrderItems`. Per-row
* failures never abort the whole call.
@ -3924,37 +3951,19 @@ export async function createOrderItemForOrder(
throw err;
}
// Strict pending gate: refuse if any line item is past `pending`.
// Statuses here mirror `orders.split` — only `pending` items are
// eligible for mutation (adding a line). Anything else would
// orphan an in-flight PO / driver assignment.
const nonPending = await pb
.collection(getOrderItemsCollectionName())
.getFullList<{ id: string; item_status: string }>({
filter: `order = "${orderId}" && item_status != "pending"`,
fields: "id,item_status",
batch: 1000,
});
if (nonPending.length > 0) {
const err = new Error(
`Cannot add items to order — ${nonPending.length} line item(s) are past 'pending' status. ` +
`Use orders.split to create a continuation order, or finish the existing PO first.`,
);
(err as Error & { status?: number }).status = 409;
(err as Error & { data?: unknown }).data = {
code: "order_locked",
offendingItemIds: nonPending.map((i) => i.id),
offendingStatuses: [...new Set(nonPending.map((i) => i.item_status))],
};
throw err;
}
// Compute selling_price if omitted.
// Compute selling_price. When the line is excluded from invoicing
// the flag wins — a non-zero price from the caller is overridden
// to 0 so the row's invoice/collection status stays consistent
// with what the UI said (the flag itself is also persisted in the
// secondary-field patch below).
const costPrice = draft.cost_price ?? 0;
const markup = draft.markup ?? 0;
let sellingPrice = draft.selling_price;
if (sellingPrice == null) {
let sellingPrice: number;
if (draft.exclude_from_invoice) {
sellingPrice = 0;
} else if (draft.selling_price != null) {
sellingPrice = draft.selling_price;
} else {
sellingPrice = Number((costPrice * (1 + markup / 100)).toFixed(2));
}
@ -3969,7 +3978,8 @@ export async function createOrderItemForOrder(
]);
// Patch secondary fields on the newly created row (sku / unit /
// supplier / notes) — same pattern as `createOrder` uses.
// supplier / notes / exclude_from_invoice) — same pattern as
// `createOrder` uses.
if (result.created.length > 0) {
const newId = result.created[0].id;
const secondary: Record<string, unknown> = {};
@ -3977,6 +3987,7 @@ export async function createOrderItemForOrder(
if (draft.unit) secondary.unit = draft.unit;
if (draft.supplier) secondary.supplier = draft.supplier;
if (draft.notes) secondary.notes = draft.notes;
if (draft.exclude_from_invoice) secondary.exclude_from_invoice = true;
if (Object.keys(secondary).length > 0) {
await pb.collection(getOrderItemsCollectionName()).update(newId, secondary);
}
@ -3986,6 +3997,9 @@ export async function createOrderItemForOrder(
orderId,
created: result.created,
failed: result.failed.map((f) => ({ error: f.error })),
// Kept for API compatibility — the field is now always `false`
// because there is no longer a pending gate. Pre-existing callers
// reading this shape continue to compile and read a constant.
orderLocked: false,
};
}