From 6f8b50211610f66ea4fe352250ffff6e503a6818 Mon Sep 17 00:00:00 2001 From: Codex Review Integration Date: Wed, 16 Sep 2026 22:40:48 -0300 Subject: [PATCH] fix(uplifts): align approval queue contracts (SH-207) --- e2e/vendors/vendor-uplift-workflow.spec.ts | 2 +- .../_components/uplift-approvals-table.tsx | 46 +++++++++++++++---- src/app/(protected)/uplifts/index.tsx | 19 +++++++- src/domain/uplifts/api/uplifts-api.ts | 2 +- src/domain/uplifts/mappers/uplift-mapper.ts | 28 ++++++++++- .../uplifts/use-cases/use-uplift-actions.ts | 39 ++++++++++++---- .../use-uplift-approval-controller.ts | 16 +++++-- .../uplift-queue-decision-flow.test.tsx | 2 +- .../domain/uplifts/api/uplifts-api.test.ts | 2 +- .../uplifts/mappers/uplift-mapper.test.ts | 12 +++-- 10 files changed, 135 insertions(+), 33 deletions(-) diff --git a/e2e/vendors/vendor-uplift-workflow.spec.ts b/e2e/vendors/vendor-uplift-workflow.spec.ts index 7900f0a0..e0ca3300 100644 --- a/e2e/vendors/vendor-uplift-workflow.spec.ts +++ b/e2e/vendors/vendor-uplift-workflow.spec.ts @@ -367,7 +367,7 @@ test("internal approver can approve, reject, and revoke with audited notes", asy }, { path: "/api/uplifts/41/revoke", - body: { reason: "Approval was made in error." }, + body: { note: "Approval was made in error." }, }, ]); }); diff --git a/src/app/(protected)/uplifts/_components/uplift-approvals-table.tsx b/src/app/(protected)/uplifts/_components/uplift-approvals-table.tsx index 7eed68c7..8aa2ec8c 100644 --- a/src/app/(protected)/uplifts/_components/uplift-approvals-table.tsx +++ b/src/app/(protected)/uplifts/_components/uplift-approvals-table.tsx @@ -42,7 +42,13 @@ function WaitingCell({ row }: { row: UpliftQueueItem }) { ); } -function AttachmentsCell({ row }: { row: UpliftQueueItem }) { +function AttachmentsCell({ + row, + onOpenAttachment, +}: { + row: UpliftQueueItem; + onOpenAttachment: (row: UpliftQueueItem) => void; +}) { if (row.evidenceDocumentId == null && !row.evidenceFileName) { return ( @@ -56,7 +62,12 @@ function AttachmentsCell({ row }: { row: UpliftQueueItem }) { : 0; return ( - + onOpenAttachment(row) : undefined} + clickable={row.evidenceDocumentId != null} + /> {extraCount > 0 && } ); @@ -66,10 +77,12 @@ function PendingRowActions({ row, onApprove, onReject, + isDecisionPending, }: { row: UpliftQueueItem; onApprove: (row: UpliftQueueItem) => void; onReject: (row: UpliftQueueItem) => void; + isDecisionPending: boolean; }) { const tooltip = row.canDecide ? "" : `Requires Tier ${row.requiredTier} role`; return ( @@ -80,7 +93,7 @@ function PendingRowActions({ size="small" variant="contained" color="success" - disabled={!row.canDecide} + disabled={!row.canDecide || isDecisionPending} onClick={() => onApprove(row)} > Approve @@ -93,7 +106,7 @@ function PendingRowActions({ size="small" variant="contained" color="error" - disabled={!row.canDecide} + disabled={!row.canDecide || isDecisionPending} onClick={() => onReject(row)} > Reject @@ -107,9 +120,11 @@ function PendingRowActions({ function ApprovedRowActions({ row, onRevoke, + isDecisionPending, }: { row: UpliftQueueItem; onRevoke: (row: UpliftQueueItem) => void; + isDecisionPending: boolean; }) { const closed = row.workOrderClosed === true; return ( @@ -120,7 +135,7 @@ function ApprovedRowActions({ size="small" variant="outlined" color="error" - disabled={closed || row.workOrderId == null} + disabled={isDecisionPending || closed || row.workOrderId == null} onClick={() => onRevoke(row)} > Revoke @@ -138,6 +153,8 @@ function UpliftApprovalRow({ onApprove, onReject, onRevoke, + onOpenAttachment, + isDecisionPending, }: { row: UpliftQueueItem; tab: UpliftApprovalTab; @@ -145,6 +162,8 @@ function UpliftApprovalRow({ onApprove: (row: UpliftQueueItem) => void; onReject: (row: UpliftQueueItem) => void; onRevoke: (row: UpliftQueueItem) => void; + onOpenAttachment: (row: UpliftQueueItem) => void; + isDecisionPending: boolean; }) { return ( - + {row.requestedByVendorName || "—"} @@ -209,9 +228,14 @@ function UpliftApprovalRow({ {tab === "pending" ? ( - + ) : ( - + )} @@ -238,6 +262,8 @@ export function UpliftApprovalsTable({ onApprove, onReject, onRevoke, + onOpenAttachment, + isDecisionPending, }: { tab: UpliftApprovalTab; isLoading: boolean; @@ -246,6 +272,8 @@ export function UpliftApprovalsTable({ onApprove: (row: UpliftQueueItem) => void; onReject: (row: UpliftQueueItem) => void; onRevoke: (row: UpliftQueueItem) => void; + onOpenAttachment: (row: UpliftQueueItem) => void; + isDecisionPending: boolean; }) { return ( @@ -293,6 +321,8 @@ export function UpliftApprovalsTable({ onApprove={onApprove} onReject={onReject} onRevoke={onRevoke} + onOpenAttachment={onOpenAttachment} + isDecisionPending={isDecisionPending} /> )) )} diff --git a/src/app/(protected)/uplifts/index.tsx b/src/app/(protected)/uplifts/index.tsx index 8f2f418b..d2ad8a23 100644 --- a/src/app/(protected)/uplifts/index.tsx +++ b/src/app/(protected)/uplifts/index.tsx @@ -1,5 +1,6 @@ import { useMemo, useState } from "react"; import { Alert, Box, Chip, Stack, Tab, Tabs, TablePagination } from "@mui/material"; +import { toast } from "react-toastify"; import { RejectDialog } from "@/app/(protected)/uplifts/_components/reject-dialog"; import { RevokeDialog } from "@/app/(protected)/uplifts/_components/revoke-dialog"; import { @@ -11,6 +12,7 @@ import { formatUpliftMoney } from "@/app/(protected)/uplifts/_components/format- import type { UpliftQueueItem } from "@/domain/uplifts/types/uplift"; import { useUpliftApprovalController } from "@/domain/uplifts/use-cases/use-uplift-approval-controller"; import { useUpliftsQueue } from "@/domain/uplifts/use-cases/use-uplifts-queue"; +import { upliftsApi } from "@/domain/uplifts/api/uplifts-api"; import { Text } from "@/components/ui/text"; const PAGE_SIZE = 25; @@ -38,7 +40,10 @@ function UpliftApprovalsHeader({ pendingExposureTotal }: { pendingExposureTotal: allowance. - Pending exposure: {formatUpliftMoney(pendingExposureTotal ?? 0)} + Pending exposure:{" "} + {pendingExposureTotal && pendingExposureTotal > 0 + ? formatUpliftMoney(pendingExposureTotal) + : "—"} ); @@ -156,6 +161,16 @@ export default function UpliftQueuePage() { } }; + const handleOpenAttachment = (row: UpliftQueueItem) => { + void upliftsApi + .downloadEvidence(row.id, row.evidenceFileName || "uplift-evidence") + .catch((error: unknown) => { + toast.error( + error instanceof Error ? error.message : "Unable to download evidence right now.", + ); + }); + }; + return ( @@ -169,6 +184,8 @@ export default function UpliftQueuePage() { onApprove={handleApprove} onReject={handleRejectRequest} onRevoke={handleRevokeRequest} + onOpenAttachment={handleOpenAttachment} + isDecisionPending={approvePending || rejectPending || revokePending} /> => { - await apiPost(`${API_PATHS.rest.uplifts}/${id}/revoke`, { reason }); + await apiPost(`${API_PATHS.rest.uplifts}/${id}/revoke`, { note: reason }); }, downloadEvidence: async ( diff --git a/src/domain/uplifts/mappers/uplift-mapper.ts b/src/domain/uplifts/mappers/uplift-mapper.ts index 5eaca683..300a6877 100644 --- a/src/domain/uplifts/mappers/uplift-mapper.ts +++ b/src/domain/uplifts/mappers/uplift-mapper.ts @@ -57,7 +57,13 @@ export function mapUpliftRequest(raw: unknown): UpliftRequest { delta: readNumber(item, "delta", "Delta") ?? 0, vendorReason: readString(item, "vendorReason", "VendorReason"), requestedAt: readString(item, "requestedAt", "RequestedAt"), - requestedByVendorName: readString(item, "requestedByVendorName", "RequestedByVendorName"), + requestedByVendorName: readString( + item, + "requestedByName", + "RequestedByName", + "requestedByVendorName", + "RequestedByVendorName", + ), decidedAt: readString(item, "decidedAt", "DecidedAt"), decidedByName: readString(item, "decidedByName", "DecidedByName"), decisionNote: readString(item, "decisionNote", "DecisionNote"), @@ -88,9 +94,21 @@ export function mapUpliftQueueItem(raw: unknown): UpliftQueueItem { workOrderId: readOptionalId(item, "workOrderId", "WorkOrderId"), dispatchId: readOptionalId(item, "dispatchId", "DispatchId"), woNumber: readString(item, "woNumber", "WoNumber", "workOrderNumber", "WorkOrderNumber"), - site: readString(item, "site", "Site", "siteCode", "SiteCode", "locationName", "LocationName"), + site: readString( + item, + "workOrderSite", + "WorkOrderSite", + "site", + "Site", + "siteCode", + "SiteCode", + "locationName", + "LocationName", + ), serviceName: readString( item, + "workOrderService", + "WorkOrderService", "serviceName", "ServiceName", "service", @@ -107,6 +125,8 @@ export function mapUpliftQueueItem(raw: unknown): UpliftQueueItem { ), approvedOnWoAuto: readNumber( item, + "workOrderAutoApprovedTotal", + "WorkOrderAutoApprovedTotal", "approvedOnWoAuto", "ApprovedOnWoAuto", "autoApprovedTotal", @@ -114,6 +134,8 @@ export function mapUpliftQueueItem(raw: unknown): UpliftQueueItem { ), approvedOnWoAdmin: readNumber( item, + "workOrderAdminApprovedTotal", + "WorkOrderAdminApprovedTotal", "approvedOnWoAdmin", "ApprovedOnWoAdmin", "adminApprovedTotal", @@ -121,6 +143,8 @@ export function mapUpliftQueueItem(raw: unknown): UpliftQueueItem { ), approvedOnWoTotal: readNumber( item, + "workOrderApprovedExposureTotal", + "WorkOrderApprovedExposureTotal", "approvedOnWoTotal", "ApprovedOnWoTotal", "approvedOnWorkOrderTotal", diff --git a/src/domain/uplifts/use-cases/use-uplift-actions.ts b/src/domain/uplifts/use-cases/use-uplift-actions.ts index 9dda66a7..1de4d83b 100644 --- a/src/domain/uplifts/use-cases/use-uplift-actions.ts +++ b/src/domain/uplifts/use-cases/use-uplift-actions.ts @@ -44,11 +44,24 @@ function invalidateWorkOrderUpliftQueries(queryClient: ReturnType - upliftsApi.approve(id, note), - onSuccess: () => { + mutationFn: ({ id, note }: ApproveUpliftInput) => upliftsApi.approve(id, note), + onSuccess: (_, variables) => { invalidateUpliftQueries(queryClient, dispatchId); - toast.success("Uplift approved — NTE updated"); + toast.success( + `Uplift approved — ${decisionAmount(variables.amount)} on ${decisionWorkOrder(variables.woNumber)}`, + ); }, onError: (error: Error) => { toast.error(error.message || "Failed to approve uplift"); @@ -75,11 +89,12 @@ export function useRejectUplift( const queryClient = useQueryClient(); return useMutation({ - mutationFn: ({ id, note }: { id: string | number; note: string }) => - upliftsApi.reject(id, note), - onSuccess: () => { + mutationFn: ({ id, note }: DecisionUpliftInput) => upliftsApi.reject(id, note), + onSuccess: (_, variables) => { invalidateUpliftQueries(queryClient, dispatchId); - toast.success("Uplift rejected"); + toast.success( + `Uplift rejected — ${decisionAmount(variables.amount)} on ${decisionWorkOrder(variables.woNumber)}`, + ); }, onError: (error: Error) => { toast.error(error.message || "Failed to reject uplift"); @@ -108,6 +123,8 @@ export function useRequestChangesUplift( export interface RevokeUpliftInput { upliftId: string | number; reason: string; + amount?: number; + woNumber?: string; } export function useRevokeUplift(): UseMutationResult { @@ -115,9 +132,11 @@ export function useRevokeUplift(): UseMutationResult upliftsApi.revoke(upliftId, reason), - onSuccess: () => { + onSuccess: (_, variables) => { invalidateWorkOrderUpliftQueries(queryClient); - toast.success("Uplift revoked"); + toast.success( + `Uplift revoked — ${decisionAmount(variables.amount)} on ${decisionWorkOrder(variables.woNumber)}`, + ); }, onError: (error: Error) => { toast.error(error.message || "Failed to revoke uplift"); diff --git a/src/domain/uplifts/use-cases/use-uplift-approval-controller.ts b/src/domain/uplifts/use-cases/use-uplift-approval-controller.ts index 190ed24f..f02e7664 100644 --- a/src/domain/uplifts/use-cases/use-uplift-approval-controller.ts +++ b/src/domain/uplifts/use-cases/use-uplift-approval-controller.ts @@ -15,7 +15,7 @@ export function useUpliftApprovalController() { const revokeUplift = useRevokeUplift(); const handleApprove = (row: UpliftQueueItem) => { - approveUplift.mutate({ id: row.id }); + approveUplift.mutate({ id: row.id, amount: row.delta, woNumber: row.woNumber }); setDetailItem(null); }; @@ -32,7 +32,12 @@ export function useUpliftApprovalController() { const handleRejectConfirm = (reason: string) => { if (!rejectTarget) return; rejectUplift.mutate( - { id: rejectTarget.id, note: reason }, + { + id: rejectTarget.id, + note: reason, + amount: rejectTarget.delta, + woNumber: rejectTarget.woNumber, + }, { onSuccess: () => setRejectTarget(null) }, ); }; @@ -40,7 +45,12 @@ export function useUpliftApprovalController() { const handleRevokeConfirm = (reason: string) => { if (!revokeTarget) return; revokeUplift.mutate( - { upliftId: revokeTarget.id, reason }, + { + upliftId: revokeTarget.id, + reason, + amount: revokeTarget.delta, + woNumber: revokeTarget.woNumber, + }, { onSuccess: () => setRevokeTarget(null) }, ); }; diff --git a/src/test/app/(protected)/uplifts/uplift-queue-decision-flow.test.tsx b/src/test/app/(protected)/uplifts/uplift-queue-decision-flow.test.tsx index d90aeed9..65d89594 100644 --- a/src/test/app/(protected)/uplifts/uplift-queue-decision-flow.test.tsx +++ b/src/test/app/(protected)/uplifts/uplift-queue-decision-flow.test.tsx @@ -93,7 +93,7 @@ describe("Uplift Approvals decision flow", () => { renderWithProviders(); fireEvent.click(screen.getByRole("button", { name: "Approve" })); - expect(approveMutate).toHaveBeenCalledWith({ id: 41 }); + expect(approveMutate).toHaveBeenCalledWith({ id: 41, amount: 150, woNumber: "WO-99" }); expect(screen.queryByRole("button", { name: "Confirm" })).not.toBeInTheDocument(); expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); }); diff --git a/src/test/domain/uplifts/api/uplifts-api.test.ts b/src/test/domain/uplifts/api/uplifts-api.test.ts index a1424bc8..4c4d2494 100644 --- a/src/test/domain/uplifts/api/uplifts-api.test.ts +++ b/src/test/domain/uplifts/api/uplifts-api.test.ts @@ -25,7 +25,7 @@ describe("upliftsApi", () => { await upliftsApi.revoke(42, "Scope was already covered"); expect(apiPost).toHaveBeenCalledWith("uplifts/42/revoke", { - reason: "Scope was already covered", + note: "Scope was already covered", }); }); }); diff --git a/src/test/domain/uplifts/mappers/uplift-mapper.test.ts b/src/test/domain/uplifts/mappers/uplift-mapper.test.ts index ece11ec2..702b7cae 100644 --- a/src/test/domain/uplifts/mappers/uplift-mapper.test.ts +++ b/src/test/domain/uplifts/mappers/uplift-mapper.test.ts @@ -64,18 +64,20 @@ describe("mapUpliftQueueItem", () => { const result = mapUpliftQueueItem({ id: 1, WoNumber: "WO-9", - Site: "Site A", - ServiceName: "Plumbing", + WorkOrderSite: "Site A", + WorkOrderService: "Plumbing", + RequestedByName: "Pat Approver", AttachmentCount: 3, - ApprovedOnWoAuto: 100, - ApprovedOnWoAdmin: 250, - ApprovedOnWoTotal: 350, + WorkOrderAutoApprovedTotal: 100, + WorkOrderAdminApprovedTotal: 250, + WorkOrderApprovedExposureTotal: 350, WorkOrderClosed: true, }); expect(result.woNumber).toBe("WO-9"); expect(result.site).toBe("Site A"); expect(result.serviceName).toBe("Plumbing"); + expect(result.requestedByVendorName).toBe("Pat Approver"); expect(result.attachmentCount).toBe(3); expect(result.approvedOnWoAuto).toBe(100); expect(result.approvedOnWoAdmin).toBe(250);