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.
This commit is contained in:
arthur.bassi 2026-08-18 09:28:49 -03:00
parent 0941a1aa3f
commit b60d0474bc
6 changed files with 46 additions and 32 deletions

View file

@ -11,7 +11,6 @@ import {
type WorkOrderUpliftListItemProps = { type WorkOrderUpliftListItemProps = {
uplift: WorkOrderUplift; uplift: WorkOrderUplift;
readOnly: boolean; readOnly: boolean;
isAdmin: boolean;
currentUserId: string | number | null | undefined; currentUserId: string | number | null | undefined;
pendingAction?: boolean; pendingAction?: boolean;
onCancel?: () => void; onCancel?: () => void;
@ -21,7 +20,6 @@ type WorkOrderUpliftListItemProps = {
export function WorkOrderUpliftListItem({ export function WorkOrderUpliftListItem({
uplift, uplift,
readOnly, readOnly,
isAdmin,
currentUserId, currentUserId,
pendingAction = false, pendingAction = false,
onCancel, onCancel,
@ -30,7 +28,7 @@ export function WorkOrderUpliftListItem({
const pillStyle = getUpliftStatusPillStyle(uplift.status); const pillStyle = getUpliftStatusPillStyle(uplift.status);
const showCancel = !readOnly && uplift.status === "pending" && Boolean(onCancel); const showCancel = !readOnly && uplift.status === "pending" && Boolean(onCancel);
const showRevoke = const showRevoke =
!readOnly && canRevokeWorkOrderUplift(uplift, currentUserId, isAdmin) && Boolean(onRevoke); !readOnly && canRevokeWorkOrderUplift(uplift, currentUserId) && Boolean(onRevoke);
return ( return (
<div className="rounded-lg border border-[var(--color-border)] p-3"> <div className="rounded-lg border border-[var(--color-border)] p-3">

View file

@ -11,7 +11,6 @@ import { hasOpenWorkOrderUplift } from "@/domain/work-orders/utils/uplift-displa
type WorkOrderUpliftsDialogContentProps = { type WorkOrderUpliftsDialogContentProps = {
readOnly: boolean; readOnly: boolean;
isAdmin: boolean;
currentUserId: string | number | null | undefined; currentUserId: string | number | null | undefined;
readOnlyStatusLabel?: string; readOnlyStatusLabel?: string;
isLoading: boolean; isLoading: boolean;
@ -28,7 +27,6 @@ type WorkOrderUpliftsDialogContentProps = {
export function WorkOrderUpliftsDialogContent({ export function WorkOrderUpliftsDialogContent({
readOnly, readOnly,
isAdmin,
currentUserId, currentUserId,
readOnlyStatusLabel, readOnlyStatusLabel,
isLoading, isLoading,
@ -78,7 +76,6 @@ export function WorkOrderUpliftsDialogContent({
key={String(uplift.id)} key={String(uplift.id)}
uplift={uplift} uplift={uplift}
readOnly={readOnly} readOnly={readOnly}
isAdmin={isAdmin}
currentUserId={currentUserId} currentUserId={currentUserId}
pendingAction={actionPending} pendingAction={actionPending}
onCancel={readOnly ? undefined : () => onCancelUplift(uplift.id)} onCancel={readOnly ? undefined : () => onCancelUplift(uplift.id)}

View file

@ -15,7 +15,6 @@ import {
isWorkOrderUpliftsReadOnly, isWorkOrderUpliftsReadOnly,
upliftRevokeRequiresReason, upliftRevokeRequiresReason,
} from "@/domain/work-orders/utils/uplift-display-utils"; } from "@/domain/work-orders/utils/uplift-display-utils";
import { isAdminUser } from "@/lib/auth/user-utils";
import { useAuthContext } from "@/providers/auth-context"; import { useAuthContext } from "@/providers/auth-context";
type RevokeTarget = { type RevokeTarget = {
@ -31,7 +30,6 @@ type WorkOrderUpliftsDialogProps = {
export function WorkOrderUpliftsDialog({ row, open, onClose }: WorkOrderUpliftsDialogProps) { export function WorkOrderUpliftsDialog({ row, open, onClose }: WorkOrderUpliftsDialogProps) {
const { user } = useAuthContext(); const { user } = useAuthContext();
const isAdmin = isAdminUser(user?.userRoles);
const workOrderId = row?.id ?? null; const workOrderId = row?.id ?? null;
const readOnly = row ? isWorkOrderUpliftsReadOnly(row.status) : true; const readOnly = row ? isWorkOrderUpliftsReadOnly(row.status) : true;
const { const {
@ -79,7 +77,6 @@ export function WorkOrderUpliftsDialog({ row, open, onClose }: WorkOrderUpliftsD
<DialogContent className="flex flex-col gap-3"> <DialogContent className="flex flex-col gap-3">
<WorkOrderUpliftsDialogContent <WorkOrderUpliftsDialogContent
readOnly={readOnly} readOnly={readOnly}
isAdmin={isAdmin}
currentUserId={user?.id} currentUserId={user?.id}
readOnlyStatusLabel={row?.status} readOnlyStatusLabel={row?.status}
isLoading={isLoading} isLoading={isLoading}

View file

@ -87,18 +87,17 @@ export function canOpenUpliftsDialog(
export function canRevokeWorkOrderUplift( export function canRevokeWorkOrderUplift(
uplift: Pick<WorkOrderUplift, "status" | "requestedById">, uplift: Pick<WorkOrderUplift, "status" | "requestedById">,
currentUserId: string | number | null | undefined, currentUserId: string | number | null | undefined,
isAdmin: boolean,
): boolean { ): boolean {
if (uplift.status === "auto_approved") { if (uplift.status !== "auto_approved") {
// SH-196: auto-approved revoke is dispatcher-own only (no admin bypass). // SH-214/SH-212: admin-approved revoke lives on Uplift Approvals, not the WO dialog.
return ( return false;
currentUserId != null &&
currentUserId !== "" &&
uplift.requestedById != null &&
String(uplift.requestedById) === String(currentUserId)
);
} }
return isAdmin && uplift.status === "approved"; return (
currentUserId != null &&
currentUserId !== "" &&
uplift.requestedById != null &&
String(uplift.requestedById) === String(currentUserId)
);
} }
export function upliftRevokeRequiresReason(status: WorkOrderUpliftStatus): boolean { export function upliftRevokeRequiresReason(status: WorkOrderUpliftStatus): boolean {

View file

@ -1,6 +1,7 @@
import { fireEvent, screen } from "@testing-library/react"; import { fireEvent, screen } from "@testing-library/react";
import { describe, expect, it, vi } from "vitest"; import { describe, expect, it, vi } from "vitest";
import { UpliftCell } from "@/app/(protected)/workorders/_components/list/table/cells/uplift-cell"; 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 { 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 { WorkOrderTableRow } from "@/domain/work-orders/types/work-order-table-row";
import type { WorkOrderUplift } from "@/domain/work-orders/types/work-order-uplift"; 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(); 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(
<WorkOrderUpliftListItem
uplift={{ ...pendingUplift, status: "approved" }}
readOnly={false}
currentUserId="admin-1"
onRevoke={vi.fn()}
/>,
{ withAuth: false },
);
expect(screen.queryByRole("button", { name: /revoke/i })).not.toBeInTheDocument();
});
it("shows Revoke for the requesting dispatcher on auto-approved uplifts", () => {
renderWithProviders(
<WorkOrderUpliftListItem
uplift={{ ...pendingUplift, status: "auto_approved", requestedById: "dispatcher-1" }}
readOnly={false}
currentUserId="dispatcher-1"
onRevoke={vi.fn()}
/>,
{ withAuth: false },
);
expect(screen.getByRole("button", { name: /revoke/i })).toBeInTheDocument();
});
});

View file

@ -12,7 +12,6 @@ describe("canRevokeWorkOrderUplift", () => {
canRevokeWorkOrderUplift( canRevokeWorkOrderUplift(
{ status: "auto_approved", requestedById: "dispatcher-1" }, { status: "auto_approved", requestedById: "dispatcher-1" },
"dispatcher-1", "dispatcher-1",
false,
), ),
).toBe(true); ).toBe(true);
}); });
@ -22,7 +21,6 @@ describe("canRevokeWorkOrderUplift", () => {
canRevokeWorkOrderUplift( canRevokeWorkOrderUplift(
{ status: "auto_approved", requestedById: "other-dispatcher" }, { status: "auto_approved", requestedById: "other-dispatcher" },
"dispatcher-1", "dispatcher-1",
false,
), ),
).toBe(false); ).toBe(false);
}); });
@ -32,7 +30,6 @@ describe("canRevokeWorkOrderUplift", () => {
canRevokeWorkOrderUplift( canRevokeWorkOrderUplift(
{ status: "auto_approved", requestedById: "dispatcher-1" }, { status: "auto_approved", requestedById: "dispatcher-1" },
"admin-user", "admin-user",
true,
), ),
).toBe(false); ).toBe(false);
}); });
@ -42,24 +39,19 @@ describe("canRevokeWorkOrderUplift", () => {
canRevokeWorkOrderUplift( canRevokeWorkOrderUplift(
{ status: "auto_approved", requestedById: "admin-user" }, { status: "auto_approved", requestedById: "admin-user" },
"admin-user", "admin-user",
true,
), ),
).toBe(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" }; const uplift = { status: "approved" as const, requestedById: "dispatcher-1" };
expect(canRevokeWorkOrderUplift(uplift, "dispatcher-1", false)).toBe(false); expect(canRevokeWorkOrderUplift(uplift, "dispatcher-1")).toBe(false);
expect(canRevokeWorkOrderUplift(uplift, "dispatcher-1", true)).toBe(true); expect(canRevokeWorkOrderUplift(uplift, "admin-user")).toBe(false);
}); });
it("rejects revoke for other statuses", () => { it("rejects revoke for other statuses", () => {
expect(canRevokeWorkOrderUplift({ status: "pending", requestedById: "u1" }, "u1", true)).toBe( expect(canRevokeWorkOrderUplift({ status: "pending", requestedById: "u1" }, "u1")).toBe(false);
false, expect(canRevokeWorkOrderUplift({ status: "rejected", requestedById: "u1" }, "u1")).toBe(false);
);
expect(canRevokeWorkOrderUplift({ status: "rejected", requestedById: "u1" }, "u1", true)).toBe(
false,
);
}); });
}); });