From b60d0474bc505c5bac521ef06565fa88d4661c42 Mon Sep 17 00:00:00 2001 From: "arthur.bassi" Date: Tue, 18 Aug 2026 09:28:49 -0300 Subject: [PATCH] fix(work-orders): keep admin-approved revoke off the WO dialog (SH-214) SH-214/SH-212 own that action on Uplift Approvals; the WO surface only revokes the requester's auto-approved uplift. --- .../uplifts/work-order-uplift-list-item.tsx | 4 +-- .../work-order-uplifts-dialog-content.tsx | 3 -- .../uplifts/work-order-uplifts-dialog.tsx | 3 -- .../work-orders/utils/uplift-display-utils.ts | 19 ++++++------ .../work-order-uplifts-affordances.test.tsx | 31 +++++++++++++++++++ .../utils/uplift-display-utils.test.ts | 18 +++-------- 6 files changed, 46 insertions(+), 32 deletions(-) diff --git a/src/app/(protected)/workorders/_components/uplifts/work-order-uplift-list-item.tsx b/src/app/(protected)/workorders/_components/uplifts/work-order-uplift-list-item.tsx index 795209b0..0578302c 100644 --- a/src/app/(protected)/workorders/_components/uplifts/work-order-uplift-list-item.tsx +++ b/src/app/(protected)/workorders/_components/uplifts/work-order-uplift-list-item.tsx @@ -11,7 +11,6 @@ import { type WorkOrderUpliftListItemProps = { uplift: WorkOrderUplift; readOnly: boolean; - isAdmin: boolean; currentUserId: string | number | null | undefined; pendingAction?: boolean; onCancel?: () => void; @@ -21,7 +20,6 @@ type WorkOrderUpliftListItemProps = { export function WorkOrderUpliftListItem({ uplift, readOnly, - isAdmin, currentUserId, pendingAction = false, onCancel, @@ -30,7 +28,7 @@ export function WorkOrderUpliftListItem({ const pillStyle = getUpliftStatusPillStyle(uplift.status); const showCancel = !readOnly && uplift.status === "pending" && Boolean(onCancel); const showRevoke = - !readOnly && canRevokeWorkOrderUplift(uplift, currentUserId, isAdmin) && Boolean(onRevoke); + !readOnly && canRevokeWorkOrderUplift(uplift, currentUserId) && Boolean(onRevoke); return (
diff --git a/src/app/(protected)/workorders/_components/uplifts/work-order-uplifts-dialog-content.tsx b/src/app/(protected)/workorders/_components/uplifts/work-order-uplifts-dialog-content.tsx index d8548d82..b0c0c6a0 100644 --- a/src/app/(protected)/workorders/_components/uplifts/work-order-uplifts-dialog-content.tsx +++ b/src/app/(protected)/workorders/_components/uplifts/work-order-uplifts-dialog-content.tsx @@ -11,7 +11,6 @@ import { hasOpenWorkOrderUplift } from "@/domain/work-orders/utils/uplift-displa type WorkOrderUpliftsDialogContentProps = { readOnly: boolean; - isAdmin: boolean; currentUserId: string | number | null | undefined; readOnlyStatusLabel?: string; isLoading: boolean; @@ -28,7 +27,6 @@ type WorkOrderUpliftsDialogContentProps = { export function WorkOrderUpliftsDialogContent({ readOnly, - isAdmin, currentUserId, readOnlyStatusLabel, isLoading, @@ -78,7 +76,6 @@ export function WorkOrderUpliftsDialogContent({ key={String(uplift.id)} uplift={uplift} readOnly={readOnly} - isAdmin={isAdmin} currentUserId={currentUserId} pendingAction={actionPending} onCancel={readOnly ? undefined : () => onCancelUplift(uplift.id)} diff --git a/src/app/(protected)/workorders/_components/uplifts/work-order-uplifts-dialog.tsx b/src/app/(protected)/workorders/_components/uplifts/work-order-uplifts-dialog.tsx index 57de111a..7e6cbc27 100644 --- a/src/app/(protected)/workorders/_components/uplifts/work-order-uplifts-dialog.tsx +++ b/src/app/(protected)/workorders/_components/uplifts/work-order-uplifts-dialog.tsx @@ -15,7 +15,6 @@ import { isWorkOrderUpliftsReadOnly, upliftRevokeRequiresReason, } from "@/domain/work-orders/utils/uplift-display-utils"; -import { isAdminUser } from "@/lib/auth/user-utils"; import { useAuthContext } from "@/providers/auth-context"; type RevokeTarget = { @@ -31,7 +30,6 @@ type WorkOrderUpliftsDialogProps = { export function WorkOrderUpliftsDialog({ row, open, onClose }: WorkOrderUpliftsDialogProps) { const { user } = useAuthContext(); - const isAdmin = isAdminUser(user?.userRoles); const workOrderId = row?.id ?? null; const readOnly = row ? isWorkOrderUpliftsReadOnly(row.status) : true; const { @@ -79,7 +77,6 @@ export function WorkOrderUpliftsDialog({ row, open, onClose }: WorkOrderUpliftsD , currentUserId: string | number | null | undefined, - isAdmin: boolean, ): boolean { - if (uplift.status === "auto_approved") { - // SH-196: auto-approved revoke is dispatcher-own only (no admin bypass). - return ( - currentUserId != null && - currentUserId !== "" && - uplift.requestedById != null && - String(uplift.requestedById) === String(currentUserId) - ); + if (uplift.status !== "auto_approved") { + // SH-214/SH-212: admin-approved revoke lives on Uplift Approvals, not the WO dialog. + return false; } - return isAdmin && uplift.status === "approved"; + return ( + currentUserId != null && + currentUserId !== "" && + uplift.requestedById != null && + String(uplift.requestedById) === String(currentUserId) + ); } export function upliftRevokeRequiresReason(status: WorkOrderUpliftStatus): boolean { diff --git a/src/test/app/(protected)/workorders/work-order-uplifts-affordances.test.tsx b/src/test/app/(protected)/workorders/work-order-uplifts-affordances.test.tsx index f6bf5dc3..101e0dd3 100644 --- a/src/test/app/(protected)/workorders/work-order-uplifts-affordances.test.tsx +++ b/src/test/app/(protected)/workorders/work-order-uplifts-affordances.test.tsx @@ -1,6 +1,7 @@ import { fireEvent, screen } from "@testing-library/react"; import { describe, expect, it, vi } from "vitest"; import { UpliftCell } from "@/app/(protected)/workorders/_components/list/table/cells/uplift-cell"; +import { WorkOrderUpliftListItem } from "@/app/(protected)/workorders/_components/uplifts/work-order-uplift-list-item"; import { WorkOrderUpliftsDialog } from "@/app/(protected)/workorders/_components/uplifts/work-order-uplifts-dialog"; import type { WorkOrderTableRow } from "@/domain/work-orders/types/work-order-table-row"; import type { WorkOrderUplift } from "@/domain/work-orders/types/work-order-uplift"; @@ -143,3 +144,33 @@ describe("WorkOrderUpliftsDialog affordances", () => { expect(screen.queryByRole("button", { name: /cancel pending/i })).not.toBeInTheDocument(); }); }); + +describe("WorkOrderUpliftListItem revoke affordances", () => { + it("hides Revoke for admin-approved uplifts on the Work Order surface", () => { + renderWithProviders( + , + { withAuth: false }, + ); + + expect(screen.queryByRole("button", { name: /revoke/i })).not.toBeInTheDocument(); + }); + + it("shows Revoke for the requesting dispatcher on auto-approved uplifts", () => { + renderWithProviders( + , + { withAuth: false }, + ); + + expect(screen.getByRole("button", { name: /revoke/i })).toBeInTheDocument(); + }); +}); diff --git a/src/test/domain/work-orders/utils/uplift-display-utils.test.ts b/src/test/domain/work-orders/utils/uplift-display-utils.test.ts index a05580f6..af91feeb 100644 --- a/src/test/domain/work-orders/utils/uplift-display-utils.test.ts +++ b/src/test/domain/work-orders/utils/uplift-display-utils.test.ts @@ -12,7 +12,6 @@ describe("canRevokeWorkOrderUplift", () => { canRevokeWorkOrderUplift( { status: "auto_approved", requestedById: "dispatcher-1" }, "dispatcher-1", - false, ), ).toBe(true); }); @@ -22,7 +21,6 @@ describe("canRevokeWorkOrderUplift", () => { canRevokeWorkOrderUplift( { status: "auto_approved", requestedById: "other-dispatcher" }, "dispatcher-1", - false, ), ).toBe(false); }); @@ -32,7 +30,6 @@ describe("canRevokeWorkOrderUplift", () => { canRevokeWorkOrderUplift( { status: "auto_approved", requestedById: "dispatcher-1" }, "admin-user", - true, ), ).toBe(false); }); @@ -42,24 +39,19 @@ describe("canRevokeWorkOrderUplift", () => { canRevokeWorkOrderUplift( { status: "auto_approved", requestedById: "admin-user" }, "admin-user", - true, ), ).toBe(true); }); - it("allows approved revoke only for admin", () => { + it("rejects approved revoke on the Work Order surface (SH-214)", () => { const uplift = { status: "approved" as const, requestedById: "dispatcher-1" }; - expect(canRevokeWorkOrderUplift(uplift, "dispatcher-1", false)).toBe(false); - expect(canRevokeWorkOrderUplift(uplift, "dispatcher-1", true)).toBe(true); + expect(canRevokeWorkOrderUplift(uplift, "dispatcher-1")).toBe(false); + expect(canRevokeWorkOrderUplift(uplift, "admin-user")).toBe(false); }); it("rejects revoke for other statuses", () => { - expect(canRevokeWorkOrderUplift({ status: "pending", requestedById: "u1" }, "u1", true)).toBe( - false, - ); - expect(canRevokeWorkOrderUplift({ status: "rejected", requestedById: "u1" }, "u1", true)).toBe( - false, - ); + expect(canRevokeWorkOrderUplift({ status: "pending", requestedById: "u1" }, "u1")).toBe(false); + expect(canRevokeWorkOrderUplift({ status: "rejected", requestedById: "u1" }, "u1")).toBe(false); }); });