From 28e30f8edbd9b276ba0a8300c70644f68de98216 Mon Sep 17 00:00:00 2001 From: Alexandre Brandizzi Date: Fri, 25 Sep 2026 01:49:24 -0300 Subject: [PATCH 1/3] fix(uplifts): show the requested increase as the one uplift amount The queue Amount column, the detail modal and the reject/revoke dialogs showed requestedNTE (the resulting NTE total) while the decision toasts showed delta (the increase), so a work order with an existing NTE named two different figures for one request. Route every surface through one upliftAmount helper that returns the increase, matching the prototype's single uplift amount, and show it on both Approve buttons. --- .../_components/uplift-approvals-table.tsx | 5 +- .../_components/uplift-decision-dialogs.tsx | 5 +- .../_components/uplift-detail-modal.tsx | 5 +- .../use-uplift-approval-controller.ts | 7 +- src/domain/uplifts/utils/uplift-amount.ts | 11 ++ .../uplift-approvals-approved-tab.test.tsx | 2 +- .../uplift-approvals-one-amount.test.tsx | 187 ++++++++++++++++++ .../uplifts/uplift-detail-modal.test.tsx | 12 +- .../uplift-queue-decision-flow.test.tsx | 8 +- 9 files changed, 223 insertions(+), 19 deletions(-) create mode 100644 src/domain/uplifts/utils/uplift-amount.ts create mode 100644 src/test/app/(protected)/uplifts/uplift-approvals-one-amount.test.tsx diff --git a/src/app/(protected)/uplifts/_components/uplift-approvals-table.tsx b/src/app/(protected)/uplifts/_components/uplift-approvals-table.tsx index 5cc446ce..3f52d36b 100644 --- a/src/app/(protected)/uplifts/_components/uplift-approvals-table.tsx +++ b/src/app/(protected)/uplifts/_components/uplift-approvals-table.tsx @@ -18,6 +18,7 @@ import { } from "@/app/(protected)/uplifts/_components/format-uplift-money"; import { Text } from "@/components/ui/text"; import { useUpliftsCanApprove } from "@/domain/uplifts/use-cases/use-uplifts-can-approve"; +import { upliftAmount } from "@/domain/uplifts/utils/uplift-amount"; import type { UpliftQueueItem } from "@/domain/uplifts/types/uplift"; import { formatDateTime, getWaitTimeTextClass, timeSince, waitTimeColor } from "@/lib/time-utils"; @@ -103,7 +104,7 @@ function PendingRowActions({ disabled={!canDecide || isDecisionPending} onClick={() => onApprove(row)} > - Approve + Approve {formatUpliftMoney(upliftAmount(row))} @@ -214,7 +215,7 @@ function UpliftApprovalRow({ - {formatUpliftMoney(row.requestedNTE)} + {formatUpliftMoney(upliftAmount(row))} diff --git a/src/app/(protected)/uplifts/_components/uplift-decision-dialogs.tsx b/src/app/(protected)/uplifts/_components/uplift-decision-dialogs.tsx index 7cc8537b..f6653a7b 100644 --- a/src/app/(protected)/uplifts/_components/uplift-decision-dialogs.tsx +++ b/src/app/(protected)/uplifts/_components/uplift-decision-dialogs.tsx @@ -1,5 +1,6 @@ import { RejectDialog } from "@/app/(protected)/uplifts/_components/reject-dialog"; import { RevokeDialog } from "@/app/(protected)/uplifts/_components/revoke-dialog"; +import { upliftAmount } from "@/domain/uplifts/utils/uplift-amount"; import type { useUpliftApprovalController } from "@/domain/uplifts/use-cases/use-uplift-approval-controller"; export type UpliftApprovalController = ReturnType; @@ -23,7 +24,7 @@ export function UpliftDecisionDialogs({ controller }: { controller: UpliftApprov <> onApprove(item)} > - Approve + Approve {formatUpliftMoney(upliftAmount(item))} @@ -394,7 +395,7 @@ function UpliftDetailContent({ Amount - {formatUpliftMoney(item.requestedNTE)} + {formatUpliftMoney(upliftAmount(item))} 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 f02e7664..bbf7bbce 100644 --- a/src/domain/uplifts/use-cases/use-uplift-approval-controller.ts +++ b/src/domain/uplifts/use-cases/use-uplift-approval-controller.ts @@ -5,6 +5,7 @@ import { useRejectUplift, useRevokeUplift, } from "@/domain/uplifts/use-cases/use-uplift-actions"; +import { upliftAmount } from "@/domain/uplifts/utils/uplift-amount"; export function useUpliftApprovalController() { const [detailItem, setDetailItem] = useState(null); @@ -15,7 +16,7 @@ export function useUpliftApprovalController() { const revokeUplift = useRevokeUplift(); const handleApprove = (row: UpliftQueueItem) => { - approveUplift.mutate({ id: row.id, amount: row.delta, woNumber: row.woNumber }); + approveUplift.mutate({ id: row.id, amount: upliftAmount(row), woNumber: row.woNumber }); setDetailItem(null); }; @@ -35,7 +36,7 @@ export function useUpliftApprovalController() { { id: rejectTarget.id, note: reason, - amount: rejectTarget.delta, + amount: upliftAmount(rejectTarget), woNumber: rejectTarget.woNumber, }, { onSuccess: () => setRejectTarget(null) }, @@ -48,7 +49,7 @@ export function useUpliftApprovalController() { { upliftId: revokeTarget.id, reason, - amount: revokeTarget.delta, + amount: upliftAmount(revokeTarget), woNumber: revokeTarget.woNumber, }, { onSuccess: () => setRevokeTarget(null) }, diff --git a/src/domain/uplifts/utils/uplift-amount.ts b/src/domain/uplifts/utils/uplift-amount.ts new file mode 100644 index 00000000..bd0110db --- /dev/null +++ b/src/domain/uplifts/utils/uplift-amount.ts @@ -0,0 +1,11 @@ +import type { UpliftRequest } from "@/domain/uplifts/types/uplift"; + +/** + * The amount of an uplift: the increase being requested on top of the work + * order's current NTE. Every surface that names "the uplift" (queue Amount, + * Approve label, detail modal, reject/revoke dialogs and decision toasts) + * shows this figure; `requestedNTE` is the resulting NTE total, not the uplift. + */ +export function upliftAmount(request: Pick): number { + return request.delta; +} diff --git a/src/test/app/(protected)/uplifts/uplift-approvals-approved-tab.test.tsx b/src/test/app/(protected)/uplifts/uplift-approvals-approved-tab.test.tsx index 05034d48..3bbf9dc1 100644 --- a/src/test/app/(protected)/uplifts/uplift-approvals-approved-tab.test.tsx +++ b/src/test/app/(protected)/uplifts/uplift-approvals-approved-tab.test.tsx @@ -132,7 +132,7 @@ describe("Uplift Approvals approved tab", () => { expect(screen.getByRole("heading", { name: "Revoke this approval?" })).toBeInTheDocument(); expect( screen.getByText( - "The approved uplift of $900.00 on WO WO-56 will be withdrawn. This does not recover money already spent — it records that the authorization was a mistake.", + "The approved uplift of $400.00 on WO WO-56 will be withdrawn. This does not recover money already spent — it records that the authorization was a mistake.", ), ).toBeInTheDocument(); }); diff --git a/src/test/app/(protected)/uplifts/uplift-approvals-one-amount.test.tsx b/src/test/app/(protected)/uplifts/uplift-approvals-one-amount.test.tsx new file mode 100644 index 00000000..f8db7ceb --- /dev/null +++ b/src/test/app/(protected)/uplifts/uplift-approvals-one-amount.test.tsx @@ -0,0 +1,187 @@ +import { fireEvent, screen, waitFor, within } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { UpliftQueueItem, UpliftsQueueResult } from "@/domain/uplifts/types/uplift"; +import { renderWithProviders } from "@/test/test-utils"; + +// A work order that already carries an NTE: the uplift (the increase) and the +// requested NTE total differ, so any surface reading the wrong field shows up. +const UPLIFT = "$4,999,997,000.00"; +const REQUESTED_NTE_TOTAL = "$5,000,000,000.00"; + +const api = vi.hoisted(() => ({ + approve: vi.fn(), + reject: vi.fn(), + revoke: vi.fn(), +})); +const toastSuccess = vi.hoisted(() => vi.fn()); +const queueState = vi.hoisted(() => ({ + data: null as UpliftsQueueResult | null, +})); + +vi.mock("@/domain/uplifts/api/uplifts-api", () => ({ + upliftsApi: { + approve: (...args: unknown[]) => api.approve(...args), + reject: (...args: unknown[]) => api.reject(...args), + revoke: (...args: unknown[]) => api.revoke(...args), + }, +})); + +vi.mock("react-toastify", async (importOriginal) => ({ + ...(await importOriginal()), + toast: { error: vi.fn(), success: toastSuccess, warning: vi.fn() }, +})); + +vi.mock("@/domain/uplifts/use-cases/use-uplifts-queue", () => ({ + useUpliftsQueue: () => ({ + data: queueState.data, + isLoading: false, + isFetching: false, + error: null, + refetch: vi.fn(), + }), +})); + +vi.mock("@/domain/work-orders/use-cases/use-work-order-uplifts", () => ({ + useWorkOrderUplifts: () => ({ data: [], isLoading: false, isError: false }), +})); + +vi.mock("@/domain/uplifts/use-cases/use-uplifts-can-approve", () => ({ + useUpliftsCanApprove: () => ({ data: true }), +})); + +vi.mock("@/providers/auth-context", async (importOriginal) => ({ + ...(await importOriginal()), + useAuthContext: () => ({ user: { userRoles: "Admin" } }), +})); + +const pendingItem: UpliftQueueItem = { + id: 41, + status: "Pending", + currentNTE: 3000, + requestedNTE: 5_000_000_000, + delta: 4_999_997_000, + vendorReason: "Full replacement", + requestedAt: "2026-01-15T10:00:00Z", + requestedByVendorName: "Gateway", + decidedAt: "", + decidedByName: "", + decisionNote: "", + requiredTier: 1, + canDecide: true, + expiresAt: "", + notificationStatus: "", + notificationError: "", + evidenceDocumentId: null, + evidenceFileName: "", + evidenceContentType: "", + evidenceSizeBytes: null, + dispatchNumber: "DSP-41", + poNumber: "PO-41", + vendorCompanyName: "Gateway Plumbing", + workOrderId: 99, + dispatchId: 7, + woNumber: "WO-99", + site: "Site A", + serviceName: "Plumbing repair", + technicianName: "", + workOrderDispatcherName: "", + workOrderScheduledDate: "", + attachmentCount: null, + approvedOnWoAuto: null, + approvedOnWoAdmin: null, + approvedOnWoTotal: null, + workOrderClosed: false, +}; + +const approvedItem: UpliftQueueItem = { + ...pendingItem, + id: 42, + status: "Approved", + decidedAt: "2026-01-16T10:00:00Z", + decidedByName: "Admin User", + canDecide: false, +}; + +function queueOf(item: UpliftQueueItem): UpliftsQueueResult { + return { items: [item], totalCount: 1, page: 1, pageSize: 25, pendingExposureTotal: null }; +} + +async function renderQueue() { + const { default: UpliftQueuePage } = await import("@/app/(protected)/uplifts/index"); + renderWithProviders(); +} + +function amountCell(): HTMLElement { + const row = screen.getByLabelText(/open uplift details for/i); + const header = screen.getByRole("columnheader", { name: "Amount" }); + const index = Array.from(header.parentElement?.children ?? []).indexOf(header); + return row.querySelectorAll("td")[index] as HTMLElement; +} + +describe("Uplift Approvals shows one amount per request", () => { + beforeEach(() => { + api.approve.mockReset().mockResolvedValue(undefined); + api.reject.mockReset().mockResolvedValue(undefined); + api.revoke.mockReset().mockResolvedValue(undefined); + toastSuccess.mockReset(); + }); + + it("uses the uplift amount in the pending row, the detail modal and the approve toast", async () => { + queueState.data = queueOf(pendingItem); + await renderQueue(); + + expect(amountCell()).toHaveTextContent(UPLIFT); + expect(screen.getByRole("button", { name: `Approve ${UPLIFT}` })).toBeInTheDocument(); + + fireEvent.click(screen.getByLabelText(/open uplift details for/i)); + const modal = screen.getByRole("dialog"); + expect(within(modal).getByText(UPLIFT)).toBeInTheDocument(); + fireEvent.click(within(modal).getByRole("button", { name: `Approve ${UPLIFT}` })); + + await waitFor(() => + expect(toastSuccess).toHaveBeenCalledWith(`Uplift approved — ${UPLIFT} on WO-99`), + ); + expect(screen.queryByText(REQUESTED_NTE_TOTAL)).not.toBeInTheDocument(); + }); + + it("uses the uplift amount in the reject dialog title and the reject toast", async () => { + queueState.data = queueOf(pendingItem); + await renderQueue(); + + fireEvent.click(screen.getByRole("button", { name: "Reject" })); + expect(screen.getByRole("heading", { name: `Reject uplift of ${UPLIFT}?` })).toBeInTheDocument(); + fireEvent.change(screen.getByLabelText(/reason for rejection/i), { + target: { value: "Out of scope" }, + }); + fireEvent.click(screen.getByRole("button", { name: "Reject uplift" })); + + await waitFor(() => + expect(toastSuccess).toHaveBeenCalledWith(`Uplift rejected — ${UPLIFT} on WO-99`), + ); + expect(screen.queryByText(REQUESTED_NTE_TOTAL)).not.toBeInTheDocument(); + }); + + it("uses the uplift amount in the approved row, the revoke dialog and the revoke toast", async () => { + queueState.data = queueOf(approvedItem); + await renderQueue(); + + fireEvent.click(screen.getByRole("tab", { name: "Approved" })); + expect(amountCell()).toHaveTextContent(UPLIFT); + + fireEvent.click(screen.getByRole("button", { name: "Revoke" })); + expect( + screen.getByText( + `The approved uplift of ${UPLIFT} on WO WO-99 will be withdrawn. This does not recover money already spent — it records that the authorization was a mistake.`, + ), + ).toBeInTheDocument(); + fireEvent.change(screen.getByLabelText(/reason for revoking/i), { + target: { value: "Approved in error" }, + }); + fireEvent.click(screen.getByRole("button", { name: `Revoke ${UPLIFT}` })); + + await waitFor(() => + expect(toastSuccess).toHaveBeenCalledWith(`Uplift revoked — ${UPLIFT} on WO-99`), + ); + expect(screen.queryByText(REQUESTED_NTE_TOTAL)).not.toBeInTheDocument(); + }); +}); diff --git a/src/test/app/(protected)/uplifts/uplift-detail-modal.test.tsx b/src/test/app/(protected)/uplifts/uplift-detail-modal.test.tsx index 4c999d2b..3c16ca6b 100644 --- a/src/test/app/(protected)/uplifts/uplift-detail-modal.test.tsx +++ b/src/test/app/(protected)/uplifts/uplift-detail-modal.test.tsx @@ -195,10 +195,12 @@ describe("UpliftDetailModal", () => { const footerButtons = screen .getAllByRole("button") .map((button) => button.textContent) - .filter((label) => label === "Reject" || label === "Approve" || label === "Revoke"); - expect(footerButtons).toEqual(["Reject", "Approve"]); + .filter( + (label) => label === "Reject" || label === "Approve $150.00" || label === "Revoke", + ); + expect(footerButtons).toEqual(["Reject", "Approve $150.00"]); - fireEvent.click(screen.getByRole("button", { name: "Approve" })); + fireEvent.click(screen.getByRole("button", { name: "Approve $150.00" })); expect(onApprove).toHaveBeenCalled(); fireEvent.click(screen.getByRole("button", { name: "Reject" })); expect(onReject).toHaveBeenCalled(); @@ -208,7 +210,7 @@ describe("UpliftDetailModal", () => { canApproveState.data = false; const { onApprove, onReject } = renderModal(); - const approve = screen.getByRole("button", { name: "Approve" }); + const approve = screen.getByRole("button", { name: "Approve $150.00" }); const reject = screen.getByRole("button", { name: "Reject" }); expect(approve).toBeDisabled(); expect(reject).toBeDisabled(); @@ -223,7 +225,7 @@ describe("UpliftDetailModal", () => { it("offers Revoke for an approved uplift and disables it with a tooltip on a closed work order", async () => { const { onRevoke } = renderModal({ status: "Approved", workOrderClosed: true }); - expect(screen.queryByRole("button", { name: "Approve" })).not.toBeInTheDocument(); + expect(screen.queryByRole("button", { name: /^Approve/ })).not.toBeInTheDocument(); expect(screen.queryByRole("button", { name: "Reject" })).not.toBeInTheDocument(); const revoke = screen.getByRole("button", { name: "Revoke" }); expect(revoke).toBeDisabled(); 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 66d0855d..0fedd04c 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 @@ -106,7 +106,7 @@ describe("Uplift Approvals decision flow", () => { const { default: UpliftQueuePage } = await import("@/app/(protected)/uplifts/index"); renderWithProviders(); - fireEvent.click(screen.getByRole("button", { name: "Approve" })); + fireEvent.click(screen.getByRole("button", { name: "Approve $150.00" })); expect(approveMutate).toHaveBeenCalledWith({ id: 41, amount: 150, woNumber: "WO-99" }); expect(screen.queryByRole("button", { name: "Confirm" })).not.toBeInTheDocument(); expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); @@ -117,7 +117,7 @@ describe("Uplift Approvals decision flow", () => { renderWithProviders(); fireEvent.click(screen.getByRole("button", { name: "Reject" })); - expect(screen.getByRole("heading", { name: /reject uplift of \$250/i })).toBeInTheDocument(); + expect(screen.getByRole("heading", { name: "Reject uplift of $150.00?" })).toBeInTheDocument(); expect(screen.getByText("The dispatcher sees this reason on WO #WO-99.")).toBeInTheDocument(); fireEvent.click(screen.getByRole("button", { name: "Cancel" })); expect(rejectMutate).not.toHaveBeenCalled(); @@ -142,7 +142,7 @@ describe("Uplift Approvals decision flow", () => { const { default: UpliftQueuePage } = await import("@/app/(protected)/uplifts/index"); renderWithProviders(); - const approve = screen.getByRole("button", { name: "Approve" }); + const approve = screen.getByRole("button", { name: "Approve $150.00" }); const reject = screen.getByRole("button", { name: "Reject" }); expect(approve).toBeDisabled(); expect(reject).toBeDisabled(); @@ -160,7 +160,7 @@ describe("Uplift Approvals decision flow", () => { const { default: UpliftQueuePage } = await import("@/app/(protected)/uplifts/index"); renderWithProviders(); - expect(screen.getByRole("button", { name: "Approve" })).toBeEnabled(); + expect(screen.getByRole("button", { name: "Approve $150.00" })).toBeEnabled(); expect(screen.getByRole("button", { name: "Reject" })).toBeEnabled(); }); }); From 3d10ba8a0c9290c8a28848ba7e408d1c16167a33 Mon Sep 17 00:00:00 2001 From: Alexandre Brandizzi Date: Fri, 25 Sep 2026 01:53:05 -0300 Subject: [PATCH 2/3] style(uplifts): apply prettier to uplift approval tests --- .../(protected)/uplifts/uplift-approvals-one-amount.test.tsx | 4 +++- src/test/app/(protected)/uplifts/uplift-detail-modal.test.tsx | 4 +--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/app/(protected)/uplifts/uplift-approvals-one-amount.test.tsx b/src/test/app/(protected)/uplifts/uplift-approvals-one-amount.test.tsx index f8db7ceb..30a13a51 100644 --- a/src/test/app/(protected)/uplifts/uplift-approvals-one-amount.test.tsx +++ b/src/test/app/(protected)/uplifts/uplift-approvals-one-amount.test.tsx @@ -149,7 +149,9 @@ describe("Uplift Approvals shows one amount per request", () => { await renderQueue(); fireEvent.click(screen.getByRole("button", { name: "Reject" })); - expect(screen.getByRole("heading", { name: `Reject uplift of ${UPLIFT}?` })).toBeInTheDocument(); + expect( + screen.getByRole("heading", { name: `Reject uplift of ${UPLIFT}?` }), + ).toBeInTheDocument(); fireEvent.change(screen.getByLabelText(/reason for rejection/i), { target: { value: "Out of scope" }, }); diff --git a/src/test/app/(protected)/uplifts/uplift-detail-modal.test.tsx b/src/test/app/(protected)/uplifts/uplift-detail-modal.test.tsx index 3c16ca6b..aed0bfb6 100644 --- a/src/test/app/(protected)/uplifts/uplift-detail-modal.test.tsx +++ b/src/test/app/(protected)/uplifts/uplift-detail-modal.test.tsx @@ -195,9 +195,7 @@ describe("UpliftDetailModal", () => { const footerButtons = screen .getAllByRole("button") .map((button) => button.textContent) - .filter( - (label) => label === "Reject" || label === "Approve $150.00" || label === "Revoke", - ); + .filter((label) => label === "Reject" || label === "Approve $150.00" || label === "Revoke"); expect(footerButtons).toEqual(["Reject", "Approve $150.00"]); fireEvent.click(screen.getByRole("button", { name: "Approve $150.00" })); From 56ca5b0c392c75ca7c4d2d6de6a56bd683552afe Mon Sep 17 00:00:00 2001 From: Alexandre Brandizzi Date: Fri, 25 Sep 2026 01:58:38 -0300 Subject: [PATCH 3/3] test(uplifts): assert the uplift amount on the approver e2e buttons --- e2e/vendors/vendor-uplift-workflow.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/e2e/vendors/vendor-uplift-workflow.spec.ts b/e2e/vendors/vendor-uplift-workflow.spec.ts index 69205a2e..f23f3ee1 100644 --- a/e2e/vendors/vendor-uplift-workflow.spec.ts +++ b/e2e/vendors/vendor-uplift-workflow.spec.ts @@ -349,7 +349,7 @@ test("internal approver can approve, reject, and revoke with audited notes", asy await expect(page.getByRole("heading", { name: "Uplift Approvals" })).toBeVisible(); const approvalRow = page.getByRole("row").filter({ hasText: "DSP-41" }); - await approvalRow.getByRole("button", { name: "Approve" }).click(); + await approvalRow.getByRole("button", { name: "Approve $25.00" }).click(); const changesRow = page.getByRole("row").filter({ hasText: "DSP-42" }); await changesRow.getByRole("button", { name: "Reject" }).click(); @@ -360,7 +360,7 @@ test("internal approver can approve, reject, and revoke with audited notes", asy const approvedRow = page.getByRole("row").filter({ hasText: "DSP-41" }); await approvedRow.getByRole("button", { name: "Revoke" }).click(); await page.getByLabel("Reason for revoking").fill("Approval was made in error."); - await page.getByRole("button", { name: "Revoke $125" }).click(); + await page.getByRole("button", { name: "Revoke $25.00" }).click(); await expect .poll(() => decisions)