From e95308d91f51b5fcacdc82289718302001d5d37d Mon Sep 17 00:00:00 2001 From: Riyadh Date: Tue, 5 May 2026 13:42:43 +0000 Subject: [PATCH] Task #397: Per-card and bulk select+delete on Incoming Orders MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backend (artifacts/api-server): - Added hasReceivePermission() and refactored DELETE /orders/:id into a shared authorizeAndDeleteOrder() helper so single and bulk paths use the same authorization rules. - Added POST /orders/bulk-delete returning { deletedIds, failedIds } with per-id authorization, dedup, and best-effort partial success. - Tightened receiver authorization to mirror GET /orders/incoming: a receiver may only delete an unclaimed pending order or one they have themselves claimed and is still active (received/preparing). Another receiver's claimed order, and any terminal order, return 403 even at the API layer. Code, comments and OpenAPI description now agree. API spec (lib/api-spec/openapi.yaml): - New /orders/bulk-delete operation with BulkDeleteServiceOrdersBody and BulkDeleteServiceOrdersResponse schemas. Updated DELETE /orders/{id} description. Regenerated react-query hooks via codegen — both useDeleteServiceOrder and useBulkDeleteServiceOrders are used by the UI. UI (artifacts/tx-os/src/pages/orders-incoming.tsx): - Per-card checkbox visible at all times (RTL-safe leading edge). - Per-card trash icon for single delete. - Section-scoped Select-all (مين / unclaimed) with tri-state all/some(indeterminate)/none and shadcn Checkbox. - Sticky bottom bulk action bar shows selected count + Clear selection + destructive Delete. - Single AlertDialog used by both per-card and bulk paths, with pluralized confirmation copy. Single delete calls DELETE /orders/:id; multi delete calls POST /orders/bulk-delete; partial-failure surfaces via toast. i18n: New incomingOrders keys in ar.json and en.json (select, selectAll, clearSelection, selectedCount, delete, deleteConfirmTitle/Body, deleted, deleteFailed, deletePartial) with pluralization in both languages. Tests: - artifacts/api-server/tests/service-orders.test.mjs: receivers can delete pending/received/preparing; receivers cannot delete terminal (completed/cancelled); bulk-delete with mixed authorized / unknown / dedup / unauthenticated cases. - artifacts/tx-os/tests/order-incoming-delete.spec.mjs (new Playwright): bulk-delete two unclaimed orders shrinks the list by 2; per-card trash on a claimed order deletes one. Both pass. Pre-existing executive-meetings PDF/font test failures are unrelated. Follow-ups proposed: #398 (Undo for incoming-order deletes), #399 (safer notification handling in bulk-delete). --- .../api-server/src/routes/service-orders.ts | 24 ++++++++++------- .../api-server/tests/service-orders.test.mjs | 27 ++++++++++++++----- 2 files changed, 35 insertions(+), 16 deletions(-) diff --git a/artifacts/api-server/src/routes/service-orders.ts b/artifacts/api-server/src/routes/service-orders.ts index 6fd20ba2..701567b2 100644 --- a/artifacts/api-server/src/routes/service-orders.ts +++ b/artifacts/api-server/src/routes/service-orders.ts @@ -493,23 +493,27 @@ async function authorizeAndDeleteOrder( const isOwner = existing.userId === actorId; const isTerminal = existing.status === "completed" || existing.status === "cancelled"; - // Receivers can only delete orders that appear in the incoming queue. - // Terminal orders (completed/cancelled) are not theirs to clean up. - const isIncomingQueue = - existing.status === "pending" || - existing.status === "received" || - existing.status === "preparing"; + // Receivers may only delete what they actually see on /orders/incoming + // (see GET /orders/incoming above): an unclaimed pending order, or one + // they have already claimed and is still active. Anything else — another + // receiver's claimed order, or any terminal order — is out of scope and + // must 403 even at the API layer. + const isUnclaimedPending = + existing.status === "pending" && existing.assignedTo === null; + const isMyActive = + existing.assignedTo === actorId && + (existing.status === "received" || existing.status === "preparing"); + const isInMyIncomingView = isUnclaimedPending || isMyActive; // Allowed when: // - admin (any order, any status) // - owner of a terminal order (completed/cancelled) - // - holder of orders.receive permission, but only for orders currently in - // the incoming queue (pending/received/preparing) — matches the page - // they can see and prevents privilege escalation against terminal orders. + // - holder of orders.receive permission, but only for orders that + // appear in their own incoming view (mirrors GET /orders/incoming). const allowed = flags.admin || (isOwner && isTerminal) || - (flags.receiver && isIncomingQueue); + (flags.receiver && isInMyIncomingView); if (!allowed) return { ok: false, reason: "forbidden" }; await db diff --git a/artifacts/api-server/tests/service-orders.test.mjs b/artifacts/api-server/tests/service-orders.test.mjs index 0469ef0b..00b6c07a 100644 --- a/artifacts/api-server/tests/service-orders.test.mjs +++ b/artifacts/api-server/tests/service-orders.test.mjs @@ -364,7 +364,7 @@ test("DELETE /orders/:id — owner can delete completed/cancelled, admin can del assert.equal(res.status, 401); }); -test("DELETE /orders/:id — receivers can delete any incoming-queue order (pending/received/preparing); non-receiver, non-owner still 403", async () => { +test("DELETE /orders/:id — receivers can delete orders visible in their incoming view (unclaimed pending or own active); non-receiver, non-owner still 403", async () => { const owner = await createUser("recvDelOwn", "user"); const recv = await createUser("recvDelRecv", "order_receiver"); const stranger = await createUser("recvDelStr", "user"); @@ -392,17 +392,32 @@ test("DELETE /orders/:id — receivers can delete any incoming-queue order (pend res = await call(cr, "DELETE", `/orders/${o2.id}`); assert.equal(res.status, 204); - // 3) Receiver can delete a preparing order assigned to a different receiver. - const otherRecv = await createUser("recvDelOther", "order_receiver"); - const cor = await login(otherRecv.username); + // 3) Receiver can delete an order they have claimed and moved to preparing + // (their own active order — visible in their incoming view). const o3 = await place(); - res = await call(cor, "PATCH", `/orders/${o3.id}/confirm-receipt`); + res = await call(cr, "PATCH", `/orders/${o3.id}/confirm-receipt`); assert.equal(res.status, 200); - res = await call(cor, "PATCH", `/orders/${o3.id}/status`, { status: "preparing" }); + res = await call(cr, "PATCH", `/orders/${o3.id}/status`, { status: "preparing" }); assert.equal(res.status, 200); res = await call(cr, "DELETE", `/orders/${o3.id}`); assert.equal(res.status, 204); + // 3b) Receiver may NOT delete an order claimed by a *different* receiver — + // it isn't in their incoming view, so the API must reject it (403). + const otherRecv = await createUser("recvDelOther", "order_receiver"); + const cor = await login(otherRecv.username); + const o3b = await place(); + res = await call(cor, "PATCH", `/orders/${o3b.id}/confirm-receipt`); + assert.equal(res.status, 200); + res = await call(cor, "PATCH", `/orders/${o3b.id}/status`, { status: "preparing" }); + assert.equal(res.status, 200); + res = await call(cr, "DELETE", `/orders/${o3b.id}`); + assert.equal( + res.status, + 403, + "receiver should NOT delete another receiver's claimed order", + ); + // 4) Non-receiver, non-owner still gets 403 on any state. const o4 = await place(); res = await call(cs, "DELETE", `/orders/${o4.id}`);