From 3c84d851c09d31d72457dc4934c809c7ad646de7 Mon Sep 17 00:00:00 2001 From: Codex Review Integration Date: Fri, 18 Sep 2026 12:53:55 -0300 Subject: [PATCH 1/2] feat(uplifts): align uplift detail modal with SH-209 Two sections, Work order and Uplift request, divided by a single rule. Work order shows Service, Vendor / Technician, Assigned Dispatcher and Scheduled from the queue item, so it no longer fetches the work order and no longer degrades to Unavailable for account-scoped staff. Uplift request shows Requested By, Requested At, the justification in a bordered block, the approved-on-WO breakdown and a horizontal attachment row whose chips open the evidence in a new tab. Every field renders data or an explicit placeholder. The footer shows Reject and Approve for pending uplifts, Revoke for approved ones and nothing for read-only records; closing is the header X. --- .../_components/format-uplift-dates.ts | 27 + .../_components/open-uplift-evidence.ts | 21 +- .../_components/uplift-detail-modal.tsx | 546 +++++++++--------- src/app/(protected)/uplifts/index.tsx | 7 +- src/domain/uplifts/api/uplifts-api.ts | 77 ++- src/domain/uplifts/mappers/uplift-mapper.ts | 3 + src/domain/uplifts/types/uplift.ts | 3 + .../uplift-approvals-approved-tab.test.tsx | 3 + .../uplift-detail-modal-permissions.test.tsx | 14 +- .../uplifts/uplift-detail-modal.test.tsx | 165 ++++-- .../uplift-queue-decision-flow.test.tsx | 3 + .../domain/uplifts/api/uplifts-api.test.ts | 49 ++ .../uplifts/mappers/uplift-mapper.test.ts | 16 + 13 files changed, 609 insertions(+), 325 deletions(-) create mode 100644 src/app/(protected)/uplifts/_components/format-uplift-dates.ts diff --git a/src/app/(protected)/uplifts/_components/format-uplift-dates.ts b/src/app/(protected)/uplifts/_components/format-uplift-dates.ts new file mode 100644 index 00000000..0e03fcc3 --- /dev/null +++ b/src/app/(protected)/uplifts/_components/format-uplift-dates.ts @@ -0,0 +1,27 @@ +const DATE_FORMAT: Intl.DateTimeFormatOptions = { + month: "short", + day: "numeric", + year: "numeric", +}; + +/** Formats an API UTC instant as local "Jan 15, 2026 · 10:00 AM"; "" when absent or invalid. */ +export function formatUpliftDateTime(value: string): string { + if (!value) return ""; + const hasZone = /(?:Z|[+-]\d{2}:?\d{2})$/i.test(value); + const date = new Date(hasZone ? value : `${value}Z`); + if (Number.isNaN(date.getTime())) return ""; + const time = date.toLocaleTimeString("en-US", { hour: "numeric", minute: "2-digit" }); + return `${date.toLocaleDateString("en-US", DATE_FORMAT)} · ${time}`; +} + +/** + * Formats a work-order schedule as "Apr 10, 2026". The schedule is a calendar day, + * so only its date part is read and it never shifts with the viewer's time zone. + */ +export function formatUpliftCalendarDate(value: string): string { + const match = /^(\d{4})-(\d{2})-(\d{2})/.exec(value); + if (match == null) return ""; + const [, year, month, day] = match; + const date = new Date(Date.UTC(Number(year), Number(month) - 1, Number(day))); + return date.toLocaleDateString("en-US", { ...DATE_FORMAT, timeZone: "UTC" }); +} diff --git a/src/app/(protected)/uplifts/_components/open-uplift-evidence.ts b/src/app/(protected)/uplifts/_components/open-uplift-evidence.ts index 2654fedd..263d40e0 100644 --- a/src/app/(protected)/uplifts/_components/open-uplift-evidence.ts +++ b/src/app/(protected)/uplifts/_components/open-uplift-evidence.ts @@ -2,12 +2,27 @@ import { toast } from "react-toastify"; import type { UpliftQueueItem } from "@/domain/uplifts/types/uplift"; import { upliftsApi } from "@/domain/uplifts/api/uplifts-api"; +function reportEvidenceError(error: unknown): void { + toast.error(error instanceof Error ? error.message : "Unable to download evidence right now."); +} + export function openUpliftEvidence(row: UpliftQueueItem): void { void upliftsApi .downloadEvidence(row.id, row.evidenceFileName || "uplift-evidence") + .catch(reportEvidenceError); +} + +export function openUpliftEvidenceInNewTab(row: UpliftQueueItem): void { + // Open the tab inside the click handler; a tab opened after the fetch resolves + // is treated as an unsolicited popup and blocked. + const tab = window.open("about:blank", "_blank"); + if (tab != null) { + tab.opener = null; + } + void upliftsApi + .openEvidence(row.id, tab, row.evidenceFileName || "uplift-evidence") .catch((error: unknown) => { - toast.error( - error instanceof Error ? error.message : "Unable to download evidence right now.", - ); + tab?.close(); + reportEvidenceError(error); }); } diff --git a/src/app/(protected)/uplifts/_components/uplift-detail-modal.tsx b/src/app/(protected)/uplifts/_components/uplift-detail-modal.tsx index 55422e2e..904fe58e 100644 --- a/src/app/(protected)/uplifts/_components/uplift-detail-modal.tsx +++ b/src/app/(protected)/uplifts/_components/uplift-detail-modal.tsx @@ -1,4 +1,5 @@ import { useMemo, type ReactNode } from "react"; +import CloseIcon from "@mui/icons-material/Close"; import { Box, Button, @@ -7,21 +8,30 @@ import { DialogActions, DialogContent, DialogTitle, + Divider, + IconButton, Tooltip, } from "@mui/material"; +import { + formatUpliftCalendarDate, + formatUpliftDateTime, +} from "@/app/(protected)/uplifts/_components/format-uplift-dates"; import { formatUpliftMoney } from "@/app/(protected)/uplifts/_components/format-uplift-money"; import { Text } from "@/components/ui/text"; -import { useWorkOrderBoardDetail } from "@/domain/work-orders/use-cases/use-work-order-detail"; import { useWorkOrderUplifts } from "@/domain/work-orders/use-cases/use-work-order-uplifts"; import { useUpliftsCanApprove } from "@/domain/uplifts/use-cases/use-uplifts-can-approve"; -import { isWorkOrderUpliftsReadOnly } from "@/domain/work-orders/utils/uplift-display-utils"; -import type { WorkOrderTableRow } from "@/domain/work-orders/types/work-order-table-row"; import type { UpliftQueueItem } from "@/domain/uplifts/types/uplift"; -import { getWaitTimeTextClass, timeSince, waitTimeColor } from "@/lib/time-utils"; const CLOSED_WO_TOOLTIP = "This work order is closed. Uplifts can no longer be revoked."; const ADMIN_ONLY_REVOKE_TOOLTIP = "Only admins can revoke uplifts"; const UNAVAILABLE_LABEL = "Unavailable"; +const BORDERED_BLOCK_SX = { + border: 1, + borderColor: "divider", + borderRadius: 1, + px: 1.75, + py: 1.25, +} as const; type ModalCallbacks = { onClose: () => void; @@ -31,24 +41,122 @@ type ModalCallbacks = { onOpenAttachment: (row: UpliftQueueItem) => void; }; +type ExposureBreakdown = { auto: number | null; admin: number | null; total: number | null }; + function DetailField({ label, children }: { label: string; children: ReactNode }) { return ( {label} - {children} + {children} ); } -function DetailSection({ title, children }: { title: string; children: ReactNode }) { +function SubsectionTitle({ children }: { children: ReactNode }) { return ( - - - {title} + + {children} + + ); +} + +function vendorTechnicianLabel(item: UpliftQueueItem): string { + const parts = [item.vendorCompanyName.trim(), item.technicianName.trim()].filter(Boolean); + const unique = parts.filter((part, index) => parts.indexOf(part) === index); + return unique.length > 0 ? unique.join(" · ") : "Not assigned"; +} + +function WorkOrderSection({ item }: { item: UpliftQueueItem }) { + return ( + + + Work order - {children} + + {item.serviceName || "Not specified"} + {vendorTechnicianLabel(item)} + + {item.workOrderDispatcherName || "Unassigned"} + + + {formatUpliftCalendarDate(item.workOrderScheduledDate) || "Unscheduled"} + + + + ); +} + +function JustificationBlock({ notes }: { notes: string }) { + const text = notes.trim(); + return ( + + {text.length > 0 && {text}} + {text.length === 0 && ( + + No justification provided. + + )} + + ); +} + +function BreakdownRow({ + label, + value, + strong, + unavailable, +}: { + label: string; + value: number | null; + strong: boolean; + unavailable: boolean; +}) { + const fallback = unavailable ? UNAVAILABLE_LABEL : "—"; + return ( + + + {label} + + + {value != null ? formatUpliftMoney(value) : fallback} + + + ); +} + +function ApprovedOnWoBreakdown({ + breakdown, + unavailable, +}: { + breakdown: ExposureBreakdown; + unavailable: boolean; +}) { + return ( + + + + ); } @@ -67,142 +175,61 @@ function UpliftAttachments({ ); } + const canOpen = item.evidenceDocumentId != null; const extraCount = typeof item.attachmentCount === "number" && item.attachmentCount > 1 ? item.attachmentCount - 1 : 0; return ( - + onOpenAttachment(item) : undefined} - clickable={item.evidenceDocumentId != null} + title={canOpen ? "Opens in a new tab" : undefined} + onClick={canOpen ? () => onOpenAttachment(item) : undefined} + clickable={canOpen} + sx={{ flexShrink: 0, maxWidth: 176 }} /> - {extraCount > 0 && } + {extraCount > 0 && ( + + )} ); } -function WorkOrderSection({ - item, - info, - unavailable, -}: { - item: UpliftQueueItem; - info?: WorkOrderTableRow; - unavailable: boolean; -}) { - const statusFallback = unavailable ? UNAVAILABLE_LABEL : "—"; - return ( - - - {item.woNumber || "—"} - - - {info?.site || item.site || "—"} - - - {info?.status || statusFallback} - - - - {info?.scheduledOn || (unavailable ? UNAVAILABLE_LABEL : "Unscheduled")} - - - - {item.vendorCompanyName || "—"} - - - ); -} - -function RequestSection({ +function UpliftRequestSection({ item, + breakdown, + breakdownUnavailable, onOpenAttachment, }: { item: UpliftQueueItem; + breakdown: ExposureBreakdown; + breakdownUnavailable: boolean; onOpenAttachment: (row: UpliftQueueItem) => void; }) { - const waitingClass = item.requestedAt - ? getWaitTimeTextClass(waitTimeColor(item.requestedAt)) - : undefined; return ( - - - - {formatUpliftMoney(item.currentNTE ?? 0)} → {formatUpliftMoney(item.requestedNTE)} - - - - {item.requestedByVendorName || "—"} - - - - {item.requestedAt ? timeSince(item.requestedAt) : "—"} - - - - {item.vendorReason || "No justification provided."} - - - - - - ); -} - -function ApprovedOnWoBreakdown({ - auto, - admin, - total, - unavailable, -}: { - auto: number | null; - admin: number | null; - total: number | null; - unavailable: boolean; -}) { - const rows: Array<{ label: string; value: number | null; strong: boolean }> = [ - { label: "Auto-approved", value: auto, strong: false }, - { label: "Admin-approved", value: admin, strong: false }, - { label: "Total", value: total, strong: true }, - ]; - return ( - - {rows.map((row) => ( - - ))} - - ); -} - -function BreakdownRow({ - label, - value, - strong, - unavailable, -}: { - label: string; - value: number | null; - strong: boolean; - unavailable: boolean; -}) { - return ( - - - {label} - - - {value != null ? formatUpliftMoney(value) : unavailable ? UNAVAILABLE_LABEL : "—"} + + + Uplift request + + + {item.requestedByVendorName || "Unknown requester"} + + + {formatUpliftDateTime(item.requestedAt) || "Not recorded"} + + + Justification + + Approved on WO + + Attachments + ); } @@ -223,6 +250,19 @@ function PendingModalActions({ const tooltip = canDecide ? "" : `Requires Tier ${item.requiredTier} role`; return ( <> + + + + + - - ); } -function UpliftModalActions({ +function RevokeModalAction({ item, - closed, canRevoke, - approvePending, revokePending, - onApprove, - onReject, onRevoke, }: { item: UpliftQueueItem; - closed: boolean; canRevoke: boolean; - approvePending: boolean; revokePending: boolean; - onApprove: (row: UpliftQueueItem) => void; - onReject: (row: UpliftQueueItem) => void; onRevoke: (row: UpliftQueueItem) => void; }) { + const closed = item.workOrderClosed === true; const revokeTooltip = !canRevoke ? ADMIN_ONLY_REVOKE_TOOLTIP : closed ? CLOSED_WO_TOOLTIP : ""; return ( - <> - {item.status === "Pending" && ( - - )} - {item.status === "Approved" && ( - - - - - - )} - + + + + + ); } -export function UpliftDetailModal({ +function useExposureBreakdown(item: UpliftQueueItem) { + // Permission failures (e.g. 403 for account-scoped users) are handled quietly: + // the breakdown falls back to "Unavailable" instead of toasting. + const woUpliftsQuery = useWorkOrderUplifts(item.workOrderId ?? null, { + suppressErrorToast: true, + }); + const breakdown = useMemo(() => { + const woUplifts = woUpliftsQuery.data ?? []; + const autoFallback = sumUpliftAmounts(woUplifts, "auto_approved"); + const adminFallback = sumUpliftAmounts(woUplifts, "approved"); + const hasWoData = woUplifts.length > 0; + return { + auto: item.approvedOnWoAuto ?? (hasWoData ? autoFallback : null), + admin: item.approvedOnWoAdmin ?? (hasWoData ? adminFallback : null), + total: item.approvedOnWoTotal ?? (hasWoData ? autoFallback + adminFallback : null), + }; + }, [item.approvedOnWoAuto, item.approvedOnWoAdmin, item.approvedOnWoTotal, woUpliftsQuery.data]); + return { breakdown, unavailable: woUpliftsQuery.isError }; +} + +function UpliftDetailContent({ item, canRevoke, approvePending, @@ -312,98 +340,102 @@ export function UpliftDetailModal({ onReject, onRevoke, onOpenAttachment, +}: { + item: UpliftQueueItem; + canRevoke: boolean; + approvePending: boolean; + revokePending: boolean; +} & ModalCallbacks) { + const { breakdown, unavailable } = useExposureBreakdown(item); + const isPending = item.status === "Pending"; + const isApproved = item.status === "Approved"; + const subtitle = [item.site, item.status].filter(Boolean).join(" · "); + return ( + <> + + + Uplift — WO #{item.woNumber || item.dispatchNumber || "—"} + + 0}> + {subtitle} + + + + + + + + + Amount + + + {formatUpliftMoney(item.requestedNTE)} + + + + + + + {(isPending || isApproved) && ( + + {isPending && ( + + )} + {isApproved && ( + + )} + + )} + + ); +} + +export function UpliftDetailModal({ + item, + ...rest }: { item: UpliftQueueItem | null; canRevoke: boolean; approvePending: boolean; revokePending: boolean; } & ModalCallbacks) { - const open = item != null; - // Permission failures (e.g. 403 for account-scoped users) are handled quietly - // in this modal: dependent sections fall back to "Unavailable" instead of toasting. - const quietMeta = { suppressErrorToast: true } as const; - const boardDetail = useWorkOrderBoardDetail( - item != null && item.workOrderId != null ? item.workOrderId : undefined, - true, - quietMeta, - ); - const woUpliftsQuery = useWorkOrderUplifts( - item != null && item.workOrderId != null ? item.workOrderId : null, - quietMeta, - ); - - const info = boardDetail.data?.info; - const closed = useMemo(() => { - if (item?.workOrderClosed === true) return true; - return info ? isWorkOrderUpliftsReadOnly(info.status) : false; - }, [item?.workOrderClosed, info]); - - const breakdown = useMemo(() => { - const woUplifts = woUpliftsQuery.data ?? []; - const autoFallback = sumUpliftAmounts(woUplifts, "auto_approved"); - const adminFallback = sumUpliftAmounts(woUplifts, "approved"); - const hasWoData = woUplifts.length > 0; - return { - auto: item?.approvedOnWoAuto ?? (hasWoData ? autoFallback : null), - admin: item?.approvedOnWoAdmin ?? (hasWoData ? adminFallback : null), - total: item?.approvedOnWoTotal ?? (hasWoData ? autoFallback + adminFallback : null), - }; - }, [ - item?.approvedOnWoAuto, - item?.approvedOnWoAdmin, - item?.approvedOnWoTotal, - woUpliftsQuery.data, - ]); - - if (!open) { + if (item == null) { return null; } - return ( - - - - Uplift request - - - {item.woNumber || item.dispatchNumber || "Work order"} · {item.status} - - - - - - - - - - - - - - - + + ); } diff --git a/src/app/(protected)/uplifts/index.tsx b/src/app/(protected)/uplifts/index.tsx index eb891ab7..db538443 100644 --- a/src/app/(protected)/uplifts/index.tsx +++ b/src/app/(protected)/uplifts/index.tsx @@ -6,7 +6,10 @@ import { } from "@/app/(protected)/uplifts/_components/uplift-approvals-table"; import { UpliftDecisionDialogs } from "@/app/(protected)/uplifts/_components/uplift-decision-dialogs"; import { UpliftDetailModal } from "@/app/(protected)/uplifts/_components/uplift-detail-modal"; -import { openUpliftEvidence } from "@/app/(protected)/uplifts/_components/open-uplift-evidence"; +import { + openUpliftEvidence, + openUpliftEvidenceInNewTab, +} from "@/app/(protected)/uplifts/_components/open-uplift-evidence"; import { formatUpliftMoney } from "@/app/(protected)/uplifts/_components/format-uplift-money"; import { isAdminUser } from "@/lib/auth/user-utils"; import { useAuthContext } from "@/providers/auth-context"; @@ -181,7 +184,7 @@ export default function UpliftQueuePage() { onApprove={handleApprove} onReject={handleRejectRequest} onRevoke={handleRevokeRequest} - onOpenAttachment={openUpliftEvidence} + onOpenAttachment={openUpliftEvidenceInNewTab} /> )} diff --git a/src/domain/uplifts/api/uplifts-api.ts b/src/domain/uplifts/api/uplifts-api.ts index 21937764..fa0a5449 100644 --- a/src/domain/uplifts/api/uplifts-api.ts +++ b/src/domain/uplifts/api/uplifts-api.ts @@ -106,28 +106,61 @@ export const upliftsApi = { id: string | number, fallbackFileName = "uplift-evidence", ): Promise => { - let response: Response; - try { - response = await apiRequestRaw( - "get", - evidenceUrl(id), - undefined, - "upliftsApi.downloadEvidence", - ); - } catch (error) { - if (error instanceof HTTPError) { - throw evidenceHttpError(error.response.status); - } - throw error; - } - + const response = await fetchEvidence(id, "upliftsApi.downloadEvidence"); const blob = await response.blob(); - const fileName = readContentDispositionFilename(response, fallbackFileName); - const url = URL.createObjectURL(blob); - const anchor = window.document.createElement("a"); - anchor.href = url; - anchor.download = fileName; - anchor.click(); - URL.revokeObjectURL(url); + saveBlob(blob, readContentDispositionFilename(response, fallbackFileName)); + }, + + /** + * Renders the evidence in `tab`, which the caller opens synchronously on click so + * popup blockers allow it. Only inert types render inline on the app origin; any + * other type (HTML, SVG, ...) is downloaded instead and the tab is closed. + */ + openEvidence: async ( + id: string | number, + tab: Window | null, + fallbackFileName = "uplift-evidence", + ): Promise => { + const response = await fetchEvidence(id, "upliftsApi.openEvidence"); + const blob = await response.blob(); + const type = blob.type.split(";")[0].trim().toLowerCase(); + if (tab == null || !INLINE_EVIDENCE_TYPES.has(type)) { + tab?.close(); + saveBlob(blob, readContentDispositionFilename(response, fallbackFileName)); + return; + } + const url = URL.createObjectURL(new Blob([blob], { type })); + tab.location.href = url; + window.setTimeout(() => URL.revokeObjectURL(url), EVIDENCE_URL_TTL_MS); }, }; + +const INLINE_EVIDENCE_TYPES = new Set([ + "application/pdf", + "image/png", + "image/jpeg", + "image/gif", + "image/webp", +]); + +const EVIDENCE_URL_TTL_MS = 60_000; + +async function fetchEvidence(id: string | number, operation: string): Promise { + try { + return await apiRequestRaw("get", evidenceUrl(id), undefined, operation); + } catch (error) { + if (error instanceof HTTPError) { + throw evidenceHttpError(error.response.status); + } + throw error; + } +} + +function saveBlob(blob: Blob, fileName: string): void { + const url = URL.createObjectURL(blob); + const anchor = window.document.createElement("a"); + anchor.href = url; + anchor.download = fileName; + anchor.click(); + URL.revokeObjectURL(url); +} diff --git a/src/domain/uplifts/mappers/uplift-mapper.ts b/src/domain/uplifts/mappers/uplift-mapper.ts index 300a6877..7fe4fa61 100644 --- a/src/domain/uplifts/mappers/uplift-mapper.ts +++ b/src/domain/uplifts/mappers/uplift-mapper.ts @@ -116,6 +116,9 @@ export function mapUpliftQueueItem(raw: unknown): UpliftQueueItem { "trade", "Trade", ), + technicianName: readString(item, "technicianName", "TechnicianName"), + workOrderDispatcherName: readString(item, "workOrderDispatcherName", "WorkOrderDispatcherName"), + workOrderScheduledDate: readString(item, "workOrderScheduledDate", "WorkOrderScheduledDate"), attachmentCount: readNumber( item, "attachmentCount", diff --git a/src/domain/uplifts/types/uplift.ts b/src/domain/uplifts/types/uplift.ts index 2e46d805..0a696347 100644 --- a/src/domain/uplifts/types/uplift.ts +++ b/src/domain/uplifts/types/uplift.ts @@ -46,6 +46,9 @@ export interface UpliftQueueItem extends UpliftRequest { woNumber: string; site: string; serviceName: string; + technicianName: string; + workOrderDispatcherName: string; + workOrderScheduledDate: string; attachmentCount: number | null; approvedOnWoAuto: number | null; approvedOnWoAdmin: number | null; 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 7b3456a8..05034d48 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 @@ -41,6 +41,9 @@ const approvedItem: UpliftQueueItem = { woNumber: "WO-55", site: "Site B", serviceName: "HVAC service", + technicianName: "", + workOrderDispatcherName: "", + workOrderScheduledDate: "", attachmentCount: 3, approvedOnWoAuto: 100, approvedOnWoAdmin: 400, diff --git a/src/test/app/(protected)/uplifts/uplift-detail-modal-permissions.test.tsx b/src/test/app/(protected)/uplifts/uplift-detail-modal-permissions.test.tsx index 2b65b7ca..533f3151 100644 --- a/src/test/app/(protected)/uplifts/uplift-detail-modal-permissions.test.tsx +++ b/src/test/app/(protected)/uplifts/uplift-detail-modal-permissions.test.tsx @@ -1,4 +1,4 @@ -import { screen, waitFor } from "@testing-library/react"; +import { screen, waitFor, within } from "@testing-library/react"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { UpliftDetailModal } from "@/app/(protected)/uplifts/_components/uplift-detail-modal"; import { ApiError } from "@/api/api-error"; @@ -80,6 +80,9 @@ const dispatcherItem: UpliftQueueItem = { woNumber: "WO-99", site: "Site A", serviceName: "Plumbing repair", + technicianName: "Tom Tech", + workOrderDispatcherName: "Dana Ruiz", + workOrderScheduledDate: "2026-04-10T00:00:00", attachmentCount: null, approvedOnWoAuto: null, approvedOnWoAdmin: null, @@ -113,13 +116,18 @@ describe("UpliftDetailModal permission failures (Dispatcher 403)", () => { woUpliftsMock.calls = 0; }); - it("shows unavailable sections without an error toast and keeps Revoke admin-only", async () => { + it("renders the work order from the queue item without a work-order fetch or error toast", async () => { renderWithGovernedClient(); await waitFor(() => { - expect(boardDetailMock.calls).toBeGreaterThan(0); expect(woUpliftsMock.calls).toBeGreaterThan(0); }); + expect(boardDetailMock.calls).toBe(0); + const workOrder = screen.getByRole("region", { name: "Work order" }); + expect(within(workOrder).getByText("Dana Ruiz")).toBeInTheDocument(); + expect(within(workOrder).getByText("Gateway Plumbing · Tom Tech")).toBeInTheDocument(); + expect(within(workOrder).getByText("Apr 10, 2026")).toBeInTheDocument(); + expect(within(workOrder).queryByText("Unavailable")).not.toBeInTheDocument(); await waitFor(() => { expect(screen.getAllByText("Unavailable").length).toBeGreaterThanOrEqual(3); 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 ea42e5ae..ef1b683b 100644 --- a/src/test/app/(protected)/uplifts/uplift-detail-modal.test.tsx +++ b/src/test/app/(protected)/uplifts/uplift-detail-modal.test.tsx @@ -1,18 +1,13 @@ -import { fireEvent, screen } from "@testing-library/react"; +import { fireEvent, screen, within } from "@testing-library/react"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { UpliftDetailModal } from "@/app/(protected)/uplifts/_components/uplift-detail-modal"; import type { UpliftQueueItem } from "@/domain/uplifts/types/uplift"; import { renderWithProviders } from "@/test/test-utils"; -const boardDetail = vi.hoisted(() => ({ data: null as Record | null })); const woUplifts = vi.hoisted(() => ({ data: null as Array> | null })); -vi.mock("@/domain/work-orders/use-cases/use-work-order-detail", () => ({ - useWorkOrderBoardDetail: () => ({ data: boardDetail.data, isLoading: false, error: null }), -})); - vi.mock("@/domain/work-orders/use-cases/use-work-order-uplifts", () => ({ - useWorkOrderUplifts: () => ({ data: woUplifts.data, isLoading: false, error: null }), + useWorkOrderUplifts: () => ({ data: woUplifts.data, isLoading: false, isError: false }), })); const canApproveState = vi.hoisted(() => ({ data: true as boolean | undefined })); @@ -28,7 +23,7 @@ const baseItem: UpliftQueueItem = { requestedNTE: 250, delta: 150, vendorReason: "", - requestedAt: "2026-01-15T10:00:00Z", + requestedAt: "2026-01-15T15:00:00Z", requestedByVendorName: "Gateway", decidedAt: "", decidedByName: "", @@ -50,6 +45,9 @@ const baseItem: UpliftQueueItem = { woNumber: "WO-99", site: "Site A", serviceName: "Plumbing repair", + technicianName: "", + workOrderDispatcherName: "", + workOrderScheduledDate: "", attachmentCount: null, approvedOnWoAuto: null, approvedOnWoAdmin: null, @@ -58,76 +56,142 @@ const baseItem: UpliftQueueItem = { }; function renderModal(overrides: Partial = {}, canRevoke = true) { - const onApprove = vi.fn(); - const onReject = vi.fn(); - const onRevoke = vi.fn(); - const onOpenAttachment = vi.fn(); + const callbacks = { + onClose: vi.fn(), + onApprove: vi.fn(), + onReject: vi.fn(), + onRevoke: vi.fn(), + onOpenAttachment: vi.fn(), + }; renderWithProviders( , ); - return { onApprove, onReject, onRevoke, onOpenAttachment }; + return callbacks; +} + +function section(name: string): HTMLElement { + return screen.getByRole("region", { name }); +} + +function fieldValue(container: HTMLElement, label: string): string { + const labelNode = within(container).getByText(label); + return labelNode.parentElement?.lastElementChild?.textContent ?? ""; } describe("UpliftDetailModal", () => { beforeEach(() => { canApproveState.data = true; - boardDetail.data = { - info: { woNumber: "WO-99", site: "Site A", status: "Open", dueDate: "" }, - }; woUplifts.data = [ { id: 1, status: "auto_approved", amount: 100 }, { id: 2, status: "approved", amount: 250 }, ]; }); - it("shows work order and request placeholders for a pending uplift", () => { + it("organizes content into Work order and Uplift request divided by a single rule", () => { renderModal(); - expect(screen.getByText("Unscheduled")).toBeInTheDocument(); - expect(screen.getByText("No justification provided.")).toBeInTheDocument(); - expect(screen.getByText("No attachments")).toBeInTheDocument(); + const dialog = screen.getByRole("dialog"); + const regions = within(dialog).getAllByRole("region"); + expect(regions.map((region) => region.getAttribute("aria-label"))).toEqual([ + "Work order", + "Uplift request", + ]); + expect(within(dialog).getAllByRole("separator")).toHaveLength(1); + expect(within(dialog).queryByText("Request")).not.toBeInTheDocument(); }); - it("opens an evidence attachment through the provided callback", () => { + it("renders the work order fields from the queue item", () => { + renderModal({ + technicianName: "Tom Tech", + workOrderDispatcherName: "Dana Ruiz", + workOrderScheduledDate: "2026-04-10T00:00:00", + }); + + const workOrder = section("Work order"); + expect(fieldValue(workOrder, "Service")).toBe("Plumbing repair"); + expect(fieldValue(workOrder, "Vendor / Technician")).toBe("Gateway Plumbing · Tom Tech"); + expect(fieldValue(workOrder, "Assigned Dispatcher")).toBe("Dana Ruiz"); + expect(fieldValue(workOrder, "Scheduled")).toBe("Apr 10, 2026"); + }); + + it("shows explicit placeholders for an empty work order, never a blank field", () => { + renderModal({ serviceName: "", vendorCompanyName: "", technicianName: "" }); + + const workOrder = section("Work order"); + expect(fieldValue(workOrder, "Service")).toBe("Not specified"); + expect(fieldValue(workOrder, "Vendor / Technician")).toBe("Not assigned"); + expect(fieldValue(workOrder, "Assigned Dispatcher")).toBe("Unassigned"); + expect(fieldValue(workOrder, "Scheduled")).toBe("Unscheduled"); + }); + + it("renders requester, request date and time, and justification in a bordered block", () => { + renderModal({ vendorReason: "Scope grew after inspection" }); + + const request = section("Uplift request"); + expect(fieldValue(request, "Requested By")).toBe("Gateway"); + expect(fieldValue(request, "Requested At")).toMatch(/^Jan 15, 2026 · \d{1,2}:\d{2} [AP]M$/u); + expect(screen.getByTestId("uplift-justification")).toHaveTextContent( + "Scope grew after inspection", + ); + }); + + it("shows explicit request placeholders when requester, date, notes and files are missing", () => { + renderModal({ requestedByVendorName: "", requestedAt: "" }); + + const request = section("Uplift request"); + expect(fieldValue(request, "Requested By")).toBe("Unknown requester"); + expect(fieldValue(request, "Requested At")).toBe("Not recorded"); + const placeholder = within(screen.getByTestId("uplift-justification")).getByText( + "No justification provided.", + ); + expect(placeholder).toHaveClass("italic"); + expect(within(request).getByText("No attachments")).toBeInTheDocument(); + }); + + it("renders attachments as a horizontal scrolling chip row that opens the file", () => { const { onOpenAttachment } = renderModal({ evidenceDocumentId: "document-1", evidenceFileName: "quote.pdf", - attachmentCount: 1, + attachmentCount: 3, }); - fireEvent.click(screen.getByText("quote.pdf")); + const row = screen.getByTestId("uplift-attachments"); + expect(row).toHaveStyle({ overflowX: "auto", flexWrap: "nowrap" }); + expect(within(row).getByText("+2")).toBeInTheDocument(); + fireEvent.click(within(row).getByText("quote.pdf")); expect(onOpenAttachment).toHaveBeenCalledWith( expect.objectContaining({ evidenceDocumentId: "document-1" }), ); }); - it("falls back to the work order uplifts for the approved-on-WO breakdown", () => { + it("shows the approved-on-WO exposure breakdown inside the uplift request", () => { renderModal(); - expect(screen.getByText("Auto-approved")).toBeInTheDocument(); - expect(screen.getByText("Admin-approved")).toBeInTheDocument(); - expect(screen.getByText("$100.00")).toBeInTheDocument(); - expect(screen.getByText("$250.00")).toBeInTheDocument(); - expect(screen.getByText("$350.00")).toBeInTheDocument(); + const request = section("Uplift request"); + expect(within(request).getByText("Approved on WO")).toBeInTheDocument(); + expect(within(request).getByText("Auto-approved (within allowance)")).toBeInTheDocument(); + expect(within(request).getByText("Admin-approved")).toBeInTheDocument(); + expect(within(request).getByText("$100.00")).toBeInTheDocument(); + expect(within(request).getByText("$350.00")).toBeInTheDocument(); }); - it("offers status-specific actions for a pending uplift", () => { + it("offers Reject then Approve for a pending uplift", () => { const { onApprove, onReject } = renderModal(); + const footerButtons = screen + .getAllByRole("button") + .map((button) => button.textContent) + .filter((label) => label === "Reject" || label === "Approve" || label === "Revoke"); + expect(footerButtons).toEqual(["Reject", "Approve"]); + fireEvent.click(screen.getByRole("button", { name: "Approve" })); expect(onApprove).toHaveBeenCalled(); - expect(screen.queryByRole("button", { name: "Revoke" })).not.toBeInTheDocument(); - fireEvent.click(screen.getByRole("button", { name: "Reject" })); expect(onReject).toHaveBeenCalled(); }); @@ -148,12 +212,17 @@ describe("UpliftDetailModal", () => { expect(onReject).not.toHaveBeenCalled(); }); - it("offers Revoke for an approved uplift and disables it on a closed work order", () => { + 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: "Reject" })).not.toBeInTheDocument(); const revoke = screen.getByRole("button", { name: "Revoke" }); expect(revoke).toBeDisabled(); + fireEvent.mouseOver(revoke); + expect( + await screen.findByText("This work order is closed. Uplifts can no longer be revoked."), + ).toBeInTheDocument(); fireEvent.click(revoke); expect(onRevoke).not.toHaveBeenCalled(); }); @@ -169,6 +238,26 @@ describe("UpliftDetailModal", () => { expect(onRevoke).not.toHaveBeenCalled(); }); + it.each(["Rejected", "Withdrawn"])("is a read-only record with no actions when %s", (status) => { + renderModal({ status }); + + for (const name of ["Approve", "Reject", "Revoke"]) { + expect(screen.queryByRole("button", { name })).not.toBeInTheDocument(); + } + expect(screen.getByRole("button", { name: "Close" })).toBeInTheDocument(); + }); + + it("closes from the header X without triggering any decision", () => { + const callbacks = renderModal(); + + fireEvent.click(screen.getByRole("button", { name: "Close" })); + + expect(callbacks.onClose).toHaveBeenCalledTimes(1); + expect(callbacks.onApprove).not.toHaveBeenCalled(); + expect(callbacks.onReject).not.toHaveBeenCalled(); + expect(callbacks.onRevoke).not.toHaveBeenCalled(); + }); + it("does not render a heading nested inside the dialog title heading", () => { renderModal(); 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 23a4d3ec..66d0855d 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 @@ -42,6 +42,9 @@ const pendingItem: UpliftQueueItem = { woNumber: "WO-99", site: "Site A", serviceName: "Plumbing repair", + technicianName: "", + workOrderDispatcherName: "", + workOrderScheduledDate: "", attachmentCount: null, approvedOnWoAuto: null, approvedOnWoAdmin: null, diff --git a/src/test/domain/uplifts/api/uplifts-api.test.ts b/src/test/domain/uplifts/api/uplifts-api.test.ts index f43c33ad..656d155e 100644 --- a/src/test/domain/uplifts/api/uplifts-api.test.ts +++ b/src/test/domain/uplifts/api/uplifts-api.test.ts @@ -42,4 +42,53 @@ describe("upliftsApi", () => { await expect(upliftsApi.canApprove(2)).resolves.toBe(false); }); + + describe("openEvidence", () => { + function evidenceResponse(body: string, contentType: string): Response { + return new Response(body, { + headers: { + "Content-Type": contentType, + "Content-Disposition": 'attachment; filename="quote.pdf"', + }, + }); + } + + function fakeTab() { + return { location: { href: "about:blank" }, close: vi.fn() }; + } + + beforeEach(() => { + URL.createObjectURL = vi.fn(() => "blob:evidence"); + URL.revokeObjectURL = vi.fn(); + }); + + it("renders an inert evidence type in the tab opened by the click", async () => { + apiRequestRaw.mockResolvedValueOnce(evidenceResponse("%PDF", "application/pdf")); + const tab = fakeTab(); + + await upliftsApi.openEvidence(7, tab as unknown as Window); + + expect(apiRequestRaw).toHaveBeenCalledWith( + "get", + "uplifts/7/evidence", + undefined, + "upliftsApi.openEvidence", + ); + expect(tab.location.href).toBe("blob:evidence"); + expect(tab.close).not.toHaveBeenCalled(); + }); + + it("downloads a type the browser could execute instead of rendering it on the app origin", async () => { + apiRequestRaw.mockResolvedValueOnce(evidenceResponse("", "image/svg+xml")); + const tab = fakeTab(); + const click = vi.spyOn(HTMLAnchorElement.prototype, "click").mockImplementation(() => {}); + + await upliftsApi.openEvidence(8, tab as unknown as Window); + + expect(tab.location.href).toBe("about:blank"); + expect(tab.close).toHaveBeenCalled(); + expect(click).toHaveBeenCalled(); + click.mockRestore(); + }); + }); }); diff --git a/src/test/domain/uplifts/mappers/uplift-mapper.test.ts b/src/test/domain/uplifts/mappers/uplift-mapper.test.ts index 702b7cae..29dfc338 100644 --- a/src/test/domain/uplifts/mappers/uplift-mapper.test.ts +++ b/src/test/domain/uplifts/mappers/uplift-mapper.test.ts @@ -108,9 +108,25 @@ describe("mapUpliftQueueItem", () => { expect(result.workOrderClosed).toBe(false); }); + it("maps the detail modal work-order context from the queue contract", () => { + const result = mapUpliftQueueItem({ + id: 4, + TechnicianName: "Tom Tech", + WorkOrderDispatcherName: "Dana Ruiz", + WorkOrderScheduledDate: "2026-04-10T09:30:00", + }); + + expect(result.technicianName).toBe("Tom Tech"); + expect(result.workOrderDispatcherName).toBe("Dana Ruiz"); + expect(result.workOrderScheduledDate).toBe("2026-04-10T09:30:00"); + }); + it("tolerates absent optional queue fields", () => { const result = mapUpliftQueueItem({ id: 3 }); + expect(result.technicianName).toBe(""); + expect(result.workOrderDispatcherName).toBe(""); + expect(result.workOrderScheduledDate).toBe(""); expect(result.woNumber).toBe(""); expect(result.site).toBe(""); expect(result.serviceName).toBe(""); From 33d0ab4fd6fe36c107623093bb6c83af34ea117e Mon Sep 17 00:00:00 2001 From: Codex Review Integration Date: Fri, 18 Sep 2026 13:02:11 -0300 Subject: [PATCH 2/2] test(uplifts): scope modal permission and date assertions to what they prove The permission spec asserts the work order renders from the queue item and, separately, that only the approved-on-WO breakdown degrades to Unavailable. The Requested At assertion derives the local calendar day so it holds in every runner time zone. --- .../uplift-detail-modal-permissions.test.tsx | 15 ++++++++++++--- .../uplifts/uplift-detail-modal.test.tsx | 10 +++++++++- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/test/app/(protected)/uplifts/uplift-detail-modal-permissions.test.tsx b/src/test/app/(protected)/uplifts/uplift-detail-modal-permissions.test.tsx index 533f3151..a2ab1b9c 100644 --- a/src/test/app/(protected)/uplifts/uplift-detail-modal-permissions.test.tsx +++ b/src/test/app/(protected)/uplifts/uplift-detail-modal-permissions.test.tsx @@ -128,10 +128,19 @@ describe("UpliftDetailModal permission failures (Dispatcher 403)", () => { expect(within(workOrder).getByText("Gateway Plumbing · Tom Tech")).toBeInTheDocument(); expect(within(workOrder).getByText("Apr 10, 2026")).toBeInTheDocument(); expect(within(workOrder).queryByText("Unavailable")).not.toBeInTheDocument(); + expect(toastMocks.error).not.toHaveBeenCalled(); + }); - await waitFor(() => { - expect(screen.getAllByText("Unavailable").length).toBeGreaterThanOrEqual(3); - }); + it("marks the approved-on-WO breakdown unavailable without a toast and keeps Revoke admin-only", async () => { + renderWithGovernedClient(); + + const request = screen.getByRole("region", { name: "Uplift request" }); + for (const label of ["Auto-approved (within allowance)", "Admin-approved", "Total"]) { + const row = within(request).getByText(label).parentElement as HTMLElement; + await waitFor(() => { + expect(within(row).getByText("Unavailable")).toBeInTheDocument(); + }); + } const revoke = screen.getByRole("button", { name: "Revoke" }); expect(revoke).toBeDisabled(); 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 ef1b683b..4c999d2b 100644 --- a/src/test/app/(protected)/uplifts/uplift-detail-modal.test.tsx +++ b/src/test/app/(protected)/uplifts/uplift-detail-modal.test.tsx @@ -135,7 +135,15 @@ describe("UpliftDetailModal", () => { const request = section("Uplift request"); expect(fieldValue(request, "Requested By")).toBe("Gateway"); - expect(fieldValue(request, "Requested At")).toMatch(/^Jan 15, 2026 · \d{1,2}:\d{2} [AP]M$/u); + // The instant renders in the viewer's zone, so the calendar day is derived, not fixed. + const localDay = new Date(baseItem.requestedAt).toLocaleDateString("en-US", { + month: "short", + day: "numeric", + year: "numeric", + }); + expect(fieldValue(request, "Requested At")).toMatch( + new RegExp(`^${localDay} · \\d{1,2}:\\d{2} [AP]M$`, "u"), + ); expect(screen.getByTestId("uplift-justification")).toHaveTextContent( "Scope grew after inspection", );