From 062a1e26c8cbc9cd2669f0036a832539ba503ce1 Mon Sep 17 00:00:00 2001 From: Codex Review Integration Date: Fri, 18 Sep 2026 14:23:43 -0300 Subject: [PATCH] fix: resolve notification review findings --- .../detail/use-slide-over-edit-state.ts | 8 +++++ .../detail/use-work-order-slide-over.ts | 3 ++ .../detail/work-order-slide-over.tsx | 12 +++++++- .../list/work-orders-list-page-panels.tsx | 1 + .../_hooks/use-dashboard-drilldown-filters.ts | 2 +- .../_hooks/use-work-orders-list-page.ts | 3 ++ .../notifications/notification-feed-list.tsx | 2 +- .../notifications/use-notification-center.ts | 1 + .../utils/work-order-drilldown-links.ts | 11 +++++-- .../utils/notification-target-url.ts | 2 +- .../dashboard/dashboard-page.test.tsx | 3 +- ...e-slide-over-edit-state-completed.test.tsx | 29 +++++++++++++++++++ .../use-work-order-deep-link.test.tsx | 26 +++++++++++++++++ .../notification-center.test.tsx | 1 + .../work-order-drilldown-links.test.ts | 7 +++-- 15 files changed, 102 insertions(+), 9 deletions(-) diff --git a/src/app/(protected)/workorders/_components/detail/use-slide-over-edit-state.ts b/src/app/(protected)/workorders/_components/detail/use-slide-over-edit-state.ts index e0c0e434..c7e526d3 100644 --- a/src/app/(protected)/workorders/_components/detail/use-slide-over-edit-state.ts +++ b/src/app/(protected)/workorders/_components/detail/use-slide-over-edit-state.ts @@ -29,6 +29,8 @@ type UseSlideOverEditStateArgs = { setTab: (tab: SlideOverTab) => void; /** Tab to land on when a work order opens outside edit mode. */ initialTab?: SlideOverTab; + /** Changes for each explicit open request, including repeated requests for the same row/tab. */ + openRequestKey?: number; }; export function useSlideOverEditState({ @@ -40,6 +42,7 @@ export function useSlideOverEditState({ closeDisabled, setTab, initialTab = "info", + openRequestKey = 0, }: UseSlideOverEditStateArgs) { const [editing, setEditing] = useState(false); const [draft, setDraft] = useState(null); @@ -69,6 +72,11 @@ export function useSlideOverEditState({ // eslint-disable-next-line react-hooks/exhaustive-deps -- row.id / editMode gate }, [row?.id, editMode]); + useEffect(() => { + if (row?.id == null || editMode) return; + setTab(initialTab); + }, [editMode, initialTab, openRequestKey, row?.id, setTab]); + const lockStatus = infoSource?.status ?? row?.status; const isInfoLocked = isSlideOverInfoLocked(lockStatus); diff --git a/src/app/(protected)/workorders/_components/detail/use-work-order-slide-over.ts b/src/app/(protected)/workorders/_components/detail/use-work-order-slide-over.ts index 7d3e83b1..04d78923 100644 --- a/src/app/(protected)/workorders/_components/detail/use-work-order-slide-over.ts +++ b/src/app/(protected)/workorders/_components/detail/use-work-order-slide-over.ts @@ -36,6 +36,7 @@ type UseWorkOrderSlideOverArgs = { onClose: () => void; saving?: boolean; initialTab?: SlideOverTab; + openRequestKey?: number; }; type CompletionUploadMutate = ( @@ -114,6 +115,7 @@ export function useWorkOrderSlideOver({ onClose, saving, initialTab, + openRequestKey, }: UseWorkOrderSlideOverArgs) { const { user } = useAuthContext(); const workOrderId = row?.id; @@ -163,6 +165,7 @@ export function useWorkOrderSlideOver({ closeDisabled, setTab, initialTab, + openRequestKey, }); const uploadCompletionPdf = (file: File) => { diff --git a/src/app/(protected)/workorders/_components/detail/work-order-slide-over.tsx b/src/app/(protected)/workorders/_components/detail/work-order-slide-over.tsx index 00985a1b..545adc0b 100644 --- a/src/app/(protected)/workorders/_components/detail/work-order-slide-over.tsx +++ b/src/app/(protected)/workorders/_components/detail/work-order-slide-over.tsx @@ -36,6 +36,7 @@ type WorkOrderSlideOverProps = { onClose: () => void; saving?: boolean; initialTab?: SlideOverTab; + openRequestKey?: number; }; export function WorkOrderSlideOver({ @@ -54,8 +55,17 @@ export function WorkOrderSlideOver({ onClose, saving, initialTab, + openRequestKey, }: WorkOrderSlideOverProps) { - const state = useWorkOrderSlideOver({ row, editMode, onSave, onClose, saving, initialTab }); + const state = useWorkOrderSlideOver({ + row, + editMode, + onSave, + onClose, + saving, + initialTab, + openRequestKey, + }); const { infoSource, activeDraft } = state; return ( diff --git a/src/app/(protected)/workorders/_components/list/work-orders-list-page-panels.tsx b/src/app/(protected)/workorders/_components/list/work-orders-list-page-panels.tsx index 7be55394..50972610 100644 --- a/src/app/(protected)/workorders/_components/list/work-orders-list-page-panels.tsx +++ b/src/app/(protected)/workorders/_components/list/work-orders-list-page-panels.tsx @@ -52,6 +52,7 @@ export function WorkOrdersListPagePanels({ page }: WorkOrdersListPagePanelsProps row={page.activeSlideOverRow} editMode={page.slideOverEdit} initialTab={page.slideOverTab} + openRequestKey={page.slideOverOpenRequestKey} users={users} sites={locations} vendors={vendors} diff --git a/src/app/(protected)/workorders/_hooks/use-dashboard-drilldown-filters.ts b/src/app/(protected)/workorders/_hooks/use-dashboard-drilldown-filters.ts index fefe635c..1306fc20 100644 --- a/src/app/(protected)/workorders/_hooks/use-dashboard-drilldown-filters.ts +++ b/src/app/(protected)/workorders/_hooks/use-dashboard-drilldown-filters.ts @@ -30,7 +30,7 @@ export function useDashboardDrilldownFilters( } appliedRef.current = true; - setFromDashboard(true); + setFromDashboard(searchParams.get("fromDashboard") === "1"); onApplyRef.current(drilldown); setSearchParams({}, { replace: true }); }, [searchParams, setSearchParams]); diff --git a/src/app/(protected)/workorders/_hooks/use-work-orders-list-page.ts b/src/app/(protected)/workorders/_hooks/use-work-orders-list-page.ts index e4cb3ada..41f338fa 100644 --- a/src/app/(protected)/workorders/_hooks/use-work-orders-list-page.ts +++ b/src/app/(protected)/workorders/_hooks/use-work-orders-list-page.ts @@ -23,6 +23,7 @@ export function useWorkOrdersListPage() { const [slideOverRow, setSlideOverRow] = useState(null); const [slideOverEdit, setSlideOverEdit] = useState(false); const [slideOverTab, setSlideOverTab] = useState("info"); + const [slideOverOpenRequestKey, setSlideOverOpenRequestKey] = useState(0); const [confirmCancel, setConfirmCancel] = useState(null); const [confirmComplete, setConfirmComplete] = useState(null); const [docRow, setDocRow] = useState(null); @@ -84,6 +85,7 @@ export function useWorkOrdersListPage() { setSlideOverRow(row); setSlideOverEdit(edit); setSlideOverTab(tab); + setSlideOverOpenRequestKey((key) => key + 1); }; const handleCloseSlideOver = () => { @@ -138,6 +140,7 @@ export function useWorkOrdersListPage() { setWizardOpen, slideOverEdit, slideOverTab, + slideOverOpenRequestKey, confirmCancel, setConfirmCancel, confirmComplete, diff --git a/src/components/notifications/notification-feed-list.tsx b/src/components/notifications/notification-feed-list.tsx index 1a363525..48215231 100644 --- a/src/components/notifications/notification-feed-list.tsx +++ b/src/components/notifications/notification-feed-list.tsx @@ -17,7 +17,7 @@ export function NotificationFeedList({ center }: NotificationFeedListProps) { ); } - if (center.error != null) { + if (center.error != null && !center.hasData) { return ( Notifications could not be loaded. They will retry automatically. diff --git a/src/components/notifications/use-notification-center.ts b/src/components/notifications/use-notification-center.ts index 6edad10c..14e8e990 100644 --- a/src/components/notifications/use-notification-center.ts +++ b/src/components/notifications/use-notification-center.ts @@ -35,6 +35,7 @@ export function useNotificationCenter(onNavigate?: () => void) { sections, unreadIds, isLoading: feed.isLoading, + hasData: feed.data != null, error: feed.error, open, dismiss: (item: NotificationItem) => dismiss([item.id]), diff --git a/src/domain/dashboard/utils/work-order-drilldown-links.ts b/src/domain/dashboard/utils/work-order-drilldown-links.ts index c83d0592..a89fc69c 100644 --- a/src/domain/dashboard/utils/work-order-drilldown-links.ts +++ b/src/domain/dashboard/utils/work-order-drilldown-links.ts @@ -14,8 +14,15 @@ export const OPEN_WIZARD_STATUSES: readonly string[] = ALL_WIZARD_STATUSES.filte (status) => status !== "Completed", ); -export function workOrderDrilldownUrl(search: URLSearchParams): string { - const query = search.toString(); +export function workOrderDrilldownUrl( + search: URLSearchParams, + source: "dashboard" | "notification" = "dashboard", +): string { + const params = new URLSearchParams(search); + if (source === "dashboard") { + params.set("fromDashboard", "1"); + } + const query = params.toString(); return query ? `${WORK_ORDERS_ROUTE}?${query}` : WORK_ORDERS_ROUTE; } diff --git a/src/domain/notifications/utils/notification-target-url.ts b/src/domain/notifications/utils/notification-target-url.ts index d97c6fbb..10ac1867 100644 --- a/src/domain/notifications/utils/notification-target-url.ts +++ b/src/domain/notifications/utils/notification-target-url.ts @@ -17,7 +17,7 @@ export function unassignedQueueUrl(): string { search.set("dateTo", UNASSIGNED_QUEUE_DATE_TO); search.set("dispatchers", ASSIGNEE_FILTER_UNASSIGNED); search.set("statuses", OPEN_WIZARD_STATUSES.join(",")); - return workOrderDrilldownUrl(search); + return workOrderDrilldownUrl(search, "notification"); } /** The work order form with vendor assignment open — where the vendor reminders always sent "Choose vendor". */ diff --git a/src/test/app/(protected)/dashboard/dashboard-page.test.tsx b/src/test/app/(protected)/dashboard/dashboard-page.test.tsx index 5ad7396e..37c3632e 100644 --- a/src/test/app/(protected)/dashboard/dashboard-page.test.tsx +++ b/src/test/app/(protected)/dashboard/dashboard-page.test.tsx @@ -80,6 +80,7 @@ import DashboardPage from "@/app/(protected)/dashboard"; import { avetaPendingDrilldownSearch, scheduledTomorrowDrilldownSearch, + workOrderDrilldownUrl, } from "@/domain/dashboard/utils/work-order-drilldown-links"; import { addDaysIso, businessTodayIso } from "@/domain/dashboard/utils/dashboard-range-utils"; @@ -161,7 +162,7 @@ describe("DashboardPage", () => { const today = businessTodayIso(); expect(navigate).toHaveBeenCalledWith( - `/workorders?${scheduledTomorrowDrilldownSearch(today).toString()}`, + workOrderDrilldownUrl(scheduledTomorrowDrilldownSearch(today)), ); fireEvent.click(screen.getByRole("button", { name: /Pending Uplifts/ })); diff --git a/src/test/app/(protected)/workorders/use-slide-over-edit-state-completed.test.tsx b/src/test/app/(protected)/workorders/use-slide-over-edit-state-completed.test.tsx index 1245a22f..b6882d4d 100644 --- a/src/test/app/(protected)/workorders/use-slide-over-edit-state-completed.test.tsx +++ b/src/test/app/(protected)/workorders/use-slide-over-edit-state-completed.test.tsx @@ -2,6 +2,7 @@ import { act, renderHook } from "@testing-library/react"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { toast } from "react-toastify"; import { useSlideOverEditState } from "@/app/(protected)/workorders/_components/detail/use-slide-over-edit-state"; +import type { SlideOverTab } from "@/app/(protected)/workorders/_components/detail/use-work-order-slide-over"; import type { WorkOrderTableRow } from "@/domain/work-orders/types/work-order-table-row"; import { vendorAssignedMessage, @@ -111,6 +112,34 @@ describe("useSlideOverEditState completed lock", () => { expect(onSave).not.toHaveBeenCalled(); }); + it("applies a repeated same-row tab request without resetting the draft", () => { + const open = baseRow(); + const setTab = vi.fn(); + const { result, rerender } = renderHook( + ({ initialTab, openRequestKey }) => + useSlideOverEditState({ + row: open, + editMode: false, + infoSource: open, + onSave: vi.fn(), + onClose: vi.fn(), + closeDisabled: false, + setTab, + initialTab, + openRequestKey, + }), + { initialProps: { initialTab: "info" as SlideOverTab, openRequestKey: 1 } }, + ); + + act(() => { + result.current.handleDraftChange({ woNumber: "edited" }); + }); + rerender({ initialTab: "extras", openRequestKey: 2 }); + + expect(setTab).toHaveBeenLastCalledWith("extras"); + expect(result.current.activeDraft?.woNumber).toBe("edited"); + }); + it("toasts vendor assigned after a successful save that includes vendorId", () => { const onSave = vi.fn(); const open = baseRow(); diff --git a/src/test/app/(protected)/workorders/use-work-order-deep-link.test.tsx b/src/test/app/(protected)/workorders/use-work-order-deep-link.test.tsx index 3d12db23..64da8dd0 100644 --- a/src/test/app/(protected)/workorders/use-work-order-deep-link.test.tsx +++ b/src/test/app/(protected)/workorders/use-work-order-deep-link.test.tsx @@ -108,4 +108,30 @@ describe("board drilldown links", () => { customTo: "2099-12-31", }); }); + + it("keeps dashboard provenance separate from notification drilldowns", async () => { + const onApply = vi.fn(); + const { result } = renderHook( + () => { + const fromDashboard = useDashboardDrilldownFilters(onApply); + return { fromDashboard, navigate: useNavigate(), location: useLocation() }; + }, + { + wrapper: wrapper("/workorders?dateFrom=2026-09-01&dateTo=2026-09-30&fromDashboard=1"), + }, + ); + + await waitFor(() => expect(onApply).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(result.current.location.search).toBe("")); + expect(result.current.fromDashboard).toBe(true); + + act(() => { + void result.current.navigate( + "/workorders?dateFrom=1970-01-01&dateTo=2099-12-31&dispatchers=__unassigned", + ); + }); + + await waitFor(() => expect(onApply).toHaveBeenCalledTimes(2)); + await waitFor(() => expect(result.current.fromDashboard).toBe(false)); + }); }); diff --git a/src/test/components/notifications/notification-center.test.tsx b/src/test/components/notifications/notification-center.test.tsx index 01958dad..388a3661 100644 --- a/src/test/components/notifications/notification-center.test.tsx +++ b/src/test/components/notifications/notification-center.test.tsx @@ -266,6 +266,7 @@ describe("notification center", () => { expect(url.searchParams.get("dateFrom")).toBe("1970-01-01"); expect(url.searchParams.get("dateTo")).toBe("2099-12-31"); expect(url.searchParams.get("statuses")).toBeTruthy(); + expect(url.searchParams.get("fromDashboard")).toBeNull(); expect(screen.getByRole("button", { name: "Notifications, 3 unread" })).toBeInTheDocument(); }); diff --git a/src/test/domain/dashboard/work-order-drilldown-links.test.ts b/src/test/domain/dashboard/work-order-drilldown-links.test.ts index 836da9e6..97be821c 100644 --- a/src/test/domain/dashboard/work-order-drilldown-links.test.ts +++ b/src/test/domain/dashboard/work-order-drilldown-links.test.ts @@ -51,9 +51,12 @@ describe("work-order drilldown links", () => { }); it("renders plain or query-bearing work order URLs", () => { - expect(workOrderDrilldownUrl(new URLSearchParams())).toBe("/workorders"); + expect(workOrderDrilldownUrl(new URLSearchParams())).toBe("/workorders?fromDashboard=1"); expect(workOrderDrilldownUrl(new URLSearchParams({ statuses: "Scheduled" }))).toBe( - "/workorders?statuses=Scheduled", + "/workorders?statuses=Scheduled&fromDashboard=1", ); + expect( + workOrderDrilldownUrl(new URLSearchParams({ statuses: "Scheduled" }), "notification"), + ).toBe("/workorders?statuses=Scheduled"); }); });