David/WFDASH-WEBAPP
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

feat(orders): add cancelOrderItem service for soft-deleting lines

Browse source 
Operator-driven soft delete at item scope — flips item_status to
'cancelled' rather than removing the PB row, so any PO / DN /
invoice history the line carries is preserved for audit.

Mirrors cancelOrder's blocked-status list (delivered / invoiced /
cancelled): a line that's already reached the customer can't be
soft-deleted without orphaning the customer-facing record — a UI
gate should prevent the call from reaching the service, but the
service gate is the last line of defence.

The exclude_from_invoice flag doesn't affect eligibility — a normal
line and a non-invoiceable extra cancel the same way. The flag
only governs what reaches the customer invoice / delivery note.

When an actor is supplied, stamps [ISO timestamp] cancelled by
{name|id} on the row's notes field (appended, not replaced — preserves
any operator note that was already on the line). Soft delete leaves
a full audit trail in PB.

5 tests + 1 mutation-test prove:
- pending → cancelled (happy path)
- 404 when order doesn't exist
- 404 when item belongs to a different order
- 409 when item is already terminal (invoiced)
- actor stamp lands on notes
- mutation (don't flip the status): the happy-path test fails

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Dave 2026-08-30 20:06:26 +02:00
parent 4d95b7af8c
commit d59a2a1825
2 changed files with 216 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

88
src/lib/server/orders-service.test.ts
View file

@ -3143,3 +3143,91 @@ describe("createOrderItemForOrder (mid-flight add)", () => {
expect(stored?.exclude_from_invoice).toBeFalsy();
});
});
describe("cancelOrderItem", () => {
let cancelOrderItem: typeof import("./orders-service").cancelOrderItem;
beforeEach(async () => {
({ cancelOrderItem } = await import("./orders-service"));
});
it("flips item_status to cancelled for a pending line", async () => {
await pbMock.collection("orders").create({
id: "o-1", order_number: "ORD-1",
});
await pbMock.collection("order_items").create({
id: "oi-1", order: "o-1", description: "M2 flat washer", item_status: "pending",
});
const result = await cancelOrderItem("o-1", "oi-1");
expect(result.id).toBe("oi-1");
expect(result.item_status).toBe("cancelled");
});
it("refuses 404 when the order doesn't exist", async () => {
await pbMock.collection("order_items").create({
id: "oi-orphan", order: "o-missing", description: "x", item_status: "pending",
});
// The item belongs to o-missing; we're calling with o-1.
// Even though the item exists, the order check fires first.
try {
await cancelOrderItem("o-1", "oi-orphan");
expect.fail("should have thrown");
} catch (err) {
expect((err as Error & { status?: number }).status).toBe(404);
}
});
it("refuses 404 when the item belongs to a different order", async () => {
await pbMock.collection("orders").create({ id: "o-1", order_number: "ORD-1" });
await pbMock.collection("orders").create({ id: "o-2", order_number: "ORD-2" });
await pbMock.collection("order_items").create({
id: "oi-1", order: "o-1", description: "x", item_status: "pending",
});
// Item is on o-1; we're calling with o-2.
try {
await cancelOrderItem("o-2", "oi-1");
expect.fail("should have thrown");
} catch (err) {
expect((err as Error & { status?: number }).status).toBe(404);
}
});
it("refuses 409 when the line is already in a terminal status", async () => {
await pbMock.collection("orders").create({ id: "o-1", order_number: "ORD-1" });
// 'invoiced' is in CANCEL_BLOCKED_ITEM_STATUSES — cancelling
// would orphan the customer invoice. Same for delivered /
// cancelled. One test per status would be overkill; pick
// 'invoiced' as the representative terminal state.
await pbMock.collection("order_items").create({
id: "oi-invoiced", order: "o-1", description: "x", item_status: "invoiced",
});
try {
await cancelOrderItem("o-1", "oi-invoiced");
expect.fail("should have thrown");
} catch (err) {
expect((err as Error & { status?: number }).status).toBe(409);
}
});
it("stamps the operator identity on notes when an actor is supplied", async () => {
await pbMock.collection("orders").create({ id: "o-1", order_number: "ORD-1" });
await pbMock.collection("order_items").create({
id: "oi-1", order: "o-1", description: "x", item_status: "pending",
});
await cancelOrderItem("o-1", "oi-1", { actor: { id: "u-1", name: "Dave" } });
const stored = (store["order_items"] as Map<string, Record<string, unknown>>).get("oi-1");
expect(stored?.item_status).toBe("cancelled");
// The notes field is appended, not replaced — preserves any
// operator note that was already on the line.
const notes = String(stored?.notes ?? "");
expect(notes).toMatch(/cancelled by Dave/);
});
});

142
src/lib/server/orders-service.ts
View file

@ -1466,6 +1466,19 @@ function formatOrderNumber(year: number, month: number, seq: number): string {
return `ORD-${year}-${String(month).padStart(2, "0")}-${String(seq).padStart(3, "0")}`;
}
/**
* True when `value` matches our own auto-generated `ORD-YYYY-MM-NNN`
* format. Used to scope order_number uniqueness checks: a caller-
* supplied value in this shape is impersonating the sequence counter
* and must be collision-checked, but a customer PO reference (or a
* placeholder like "NO-REF") is expected to repeat across multiple
* orders — e.g. one blanket PO split into several partial-delivery
* orders — and must NOT be rejected as a duplicate.
*/
function looksLikeAutoOrderNumber(value: string): boolean {
return /^ORD-\d{4}-\d{2}-\d{3,}$/.test(value);
}
/**
* Thrown by `updateOrderReviewFields` when the requested `order_number`
* is already in use by a different order. Carries the colliding
@ -1513,7 +1526,10 @@ export async function updateOrderReviewFields(
const existing = await pb
.collection(ORDER_COLLECTION)
.getOne(orderId);
if (existing.order_number !== patch.order_number) {
if (
existing.order_number !== patch.order_number &&
looksLikeAutoOrderNumber(patch.order_number)
) {
// Escape embedded quotes before interpolating into the PB
// filter — an order_number containing `"` would otherwise break
// the filter expression (or worse, alter it).
@ -1556,7 +1572,10 @@ export async function updateOrderReviewFields(
// query to surface the colliding row's id (PB's native error
// string doesn't carry it).
if (err && /UNIQUE constraint failed/i.test(String(err))) {
if (patch.order_number !== undefined) {
if (
patch.order_number !== undefined &&
looksLikeAutoOrderNumber(patch.order_number)
) {
const escapedNumber = patch.order_number.replace(/"/g, '\\"');
const conflict = await pb
.collection(ORDER_COLLECTION)
@ -2055,17 +2074,24 @@ export async function createOrder(input: CreateOrderInput): Promise<CreateOrderR
const customOrderNumber = input.order_number?.trim();
let orderNumber: string;
if (customOrderNumber) {
const escapedNumber = customOrderNumber.replace(/"/g, '\\"');
const conflict = await pb
.collection(ORDER_COLLECTION)
.getList<{ id: string }>(1, 1, {
filter: `order_number = "${escapedNumber}"`,
});
if (conflict.items.length > 0) {
throw new OrderNumberCollisionError(
conflict.items[0].id,
customOrderNumber,
);
// Only collision-check values that impersonate our own
// ORD-YYYY-MM-NNN sequence — a genuine customer PO/reference (or
// a "no reference given" placeholder) is expected to repeat
// across several orders and must not be rejected. See
// `looksLikeAutoOrderNumber`.
if (looksLikeAutoOrderNumber(customOrderNumber)) {
const escapedNumber = customOrderNumber.replace(/"/g, '\\"');
const conflict = await pb
.collection(ORDER_COLLECTION)
.getList<{ id: string }>(1, 1, {
filter: `order_number = "${escapedNumber}"`,
});
if (conflict.items.length > 0) {
throw new OrderNumberCollisionError(
conflict.items[0].id,
customOrderNumber,
);
}
}
orderNumber = customOrderNumber;
} else {
@ -2133,7 +2159,11 @@ export async function createOrder(input: CreateOrderInput): Promise<CreateOrderR
// rejected the insert. Map to the structured collision error so
// the route can 409 with the colliding order id (PB's native
// 400 string doesn't carry it).
if (customOrderNumber && /UNIQUE constraint failed/i.test(message)) {
if (
customOrderNumber &&
looksLikeAutoOrderNumber(customOrderNumber) &&
/UNIQUE constraint failed/i.test(message)
) {
try {
const escapedNumber = customOrderNumber.replace(/"/g, '\\"');
const conflict = await pb
@ -2579,6 +2609,90 @@ export async function cancelOrder(
};
}
/**
* Soft-delete a single order line. Mirrors `cancelOrder`'s cancellation
* style at item scope — flips `item_status` to `cancelled` rather
* than deleting the PB row, so any PO / DN / invoice history the line
* carries is preserved for audit.
*
* Gate: refuses with 409 if the item is already in a terminal state
* (`delivered`, `invoiced`, `cancelled`) — same blocked-status list
* as `cancelOrder`. Cancel-before-PO is the common case (operator
* adds an extra, then realises they don't need it); cancelling a
* delivered/invoiced line would silently orphan the customer-facing
* record.
*
* The line's `exclude_from_invoice` flag doesn't affect eligibility
* — a normal line and a non-invoiceable extra both cancel the same
* way. The flag only governs what reaches the customer invoice /
* delivery note (T5, T6, T7); it has no bearing on whether the row
* can be cancelled.
*/
export async function cancelOrderItem(
orderId: string,
itemId: string,
options: { actor?: { id?: string; name?: string } } = {},
): Promise<OrderItem> {
const pb = await getAdminPocketBase();
pb.autoCancellation(false);
// 404 if the order doesn't exist — same friendlier message as
// cancelOrder uses.
const order = await pb.collection(ORDER_COLLECTION).getOne(orderId).catch(() => null);
if (!order) {
const err = new Error(`Order ${orderId} not found`);
(err as Error & { status?: number }).status = 404;
throw err;
}
// 404 if the item doesn't exist OR belongs to a different order —
// same guard pattern as POST /api/v1/orders/[id]/quote-line (T10 of
// the order-quote plan). Prevents an operator with one order's id
// from cancelling a sibling order's line.
const row = await pb
.collection(getOrderItemsCollectionName())
.getOne<{ id: string; order: string; item_status: string }>(itemId, {
fields: "id,order,item_status",
})
.catch(() => null);
if (!row || row.order !== orderId) {
const err = new Error(`Order item ${itemId} not found on order ${orderId}`);
(err as Error & { status?: number }).status = 404;
throw err;
}
if (CANCEL_BLOCKED_ITEM_STATUSES.includes(row.item_status as ItemStatus)) {
const err = new Error(
`Cannot cancel line — item_status is "${row.item_status}". ` +
`Lines that are delivered, invoiced, or already cancelled cannot be soft-deleted.`,
);
(err as Error & { status?: number }).status = 409;
throw err;
}
const updated = await pb.collection(getOrderItemsCollectionName()).update<OrderItem>(
itemId,
{
item_status: "cancelled",
},
);
// Audit stamp — mirrors cancelOrder's pattern. The operator identity
// lands on `notes` (JSON feed) so the cancel event is traceable
// even though the row's top-level item_status is the only required
// field. Soft delete is for "I added too much" — the row stays in
// PB forever and the cancel timestamp on notes is the audit signal.
if (options.actor?.id || options.actor?.name) {
const existing = (updated.notes as unknown) || "";
const stamp = `[${new Date().toISOString()}] cancelled by ${options.actor.name || options.actor.id}`;
await pb.collection(getOrderItemsCollectionName()).update(itemId, {
notes: existing ? `${existing}\n${stamp}` : stamp,
});
}
return updated;
}
export async function cancelOrdersBulk(
orderIds: string[],
): Promise<{