From 41ec771a0cef9db76f9415c2cb0118225b4b3ee2 Mon Sep 17 00:00:00 2001 From: npal Date: Wed, 30 Sep 2026 12:13:35 -0500 Subject: [PATCH] fix: stop duplicate global toasts on vendor-portal dispatch actions All six vendor-portal dispatch mutation hooks lacked meta.suppressErrorToast, so a failed checklist toggle, signoff, comment, or uplift request/withdraw/ revise raised both the global MutationCache toast (leaking the raw server message) and the section's own inline error, contradicting this ticket's "no behavior change" goal versus main's try/catch-only handling. Also fixes a related gap in checklist-section.tsx: updateItem.isError only reflects the latest call on the shared mutation observer, so a failed toggle whose item was not the most recent click showed no error at all. Tracks failure locally instead, mirroring main's per-call error state. Added regression coverage with the app's real query client (createAppQueryClient) for comments, signoff, and uplift withdraw, asserting the inline message shows and the global toast does not fire; plus a mixed pass/fail concurrent-toggle case for the checklist. Co-Authored-By: Claude Sonnet 5 --- .../_components/checklist-section.tsx | 9 +- .../use-vendor-portal-dispatch-actions.ts | 7 ++ src/test/app/v/checklist-section.test.tsx | 36 +++++++ src/test/app/v/comments-section.test.tsx | 31 ++++++ src/test/app/v/signoff-section.test.tsx | 48 ++++++--- .../app/v/uplift-requests-section.test.tsx | 102 ++++++++++++++++++ 6 files changed, 219 insertions(+), 14 deletions(-) create mode 100644 src/test/app/v/uplift-requests-section.test.tsx diff --git a/src/app/v/[token]/dispatch/_components/checklist-section.tsx b/src/app/v/[token]/dispatch/_components/checklist-section.tsx index 4c1acb2e..f966dd93 100644 --- a/src/app/v/[token]/dispatch/_components/checklist-section.tsx +++ b/src/app/v/[token]/dispatch/_components/checklist-section.tsx @@ -1,3 +1,4 @@ +import { useState } from "react"; import { Text } from "@/components/ui/text"; import { formatVendorPortalDateTime } from "@/domain/vendor-portal/lib/status-helpers"; import type { VendorPortalChecklistItem } from "@/domain/vendor-portal/types/vendor-portal"; @@ -19,15 +20,19 @@ export function ChecklistSection({ onItemUpdated, }: ChecklistSectionProps) { const updateItem = useUpdateVendorPortalChecklistItem(token, dispatchId); + // updateItem.isError only reflects the latest call on the shared mutation observer, so it + // can't tell an earlier failed toggle from a later one that succeeded. Track it locally. + const [failed, setFailed] = useState(false); const toggle = (item: VendorPortalChecklistItem) => { if (locked) return; + setFailed(false); // mutateAsync resolves per call, unlike mutate's onSuccess which only fires for the // latest call on this shared observer — needed so two quick toggles both report back. void updateItem .mutateAsync({ itemId: item.id, isCompleted: !item.isCompleted }) .then((updated) => onItemUpdated(updated)) - .catch(() => undefined); + .catch(() => setFailed(true)); }; if (!items.length) { @@ -36,7 +41,7 @@ export function ChecklistSection({ return (
- + Unable to update the checklist. Please try again. {items.map((item) => ( diff --git a/src/domain/vendor-portal/use-cases/use-vendor-portal-dispatch-actions.ts b/src/domain/vendor-portal/use-cases/use-vendor-portal-dispatch-actions.ts index 1d21a8a9..8771df52 100644 --- a/src/domain/vendor-portal/use-cases/use-vendor-portal-dispatch-actions.ts +++ b/src/domain/vendor-portal/use-cases/use-vendor-portal-dispatch-actions.ts @@ -25,6 +25,8 @@ export function useUpdateVendorPortalChecklistItem( return useMutation({ mutationFn: ({ itemId, isCompleted }: UpdateChecklistItemInput) => vendorPortalApi.updateChecklistItem(token, dispatchId, itemId, isCompleted), + // Each caller already renders its own inline failure copy. + meta: { suppressErrorToast: true }, }); } @@ -43,6 +45,7 @@ export function useAddVendorPortalSignoff( signatureMethod: string; signoffType: string; }) => vendorPortalApi.addSignoff(token, dispatchId, payload), + meta: { suppressErrorToast: true }, }); } @@ -52,6 +55,7 @@ export function useAddVendorPortalComment( ): UseMutationResult { return useMutation({ mutationFn: (commentText: string) => vendorPortalApi.addComment(token, dispatchId, commentText), + meta: { suppressErrorToast: true }, }); } @@ -62,6 +66,7 @@ export function useRequestVendorUplift( return useMutation({ mutationFn: (payload: VendorUpliftRequestBody) => vendorPortalApi.requestUplift(token, dispatchId, payload), + meta: { suppressErrorToast: true }, }); } @@ -72,6 +77,7 @@ export function useWithdrawVendorUplift( return useMutation({ mutationFn: (requestId: string | number) => vendorPortalApi.withdrawUplift(token, dispatchId, requestId), + meta: { suppressErrorToast: true }, }); } @@ -87,5 +93,6 @@ export function useReviseVendorUplift( return useMutation({ mutationFn: ({ requestId, payload }: ReviseUpliftInput) => vendorPortalApi.reviseUplift(token, dispatchId, requestId, payload), + meta: { suppressErrorToast: true }, }); } diff --git a/src/test/app/v/checklist-section.test.tsx b/src/test/app/v/checklist-section.test.tsx index e0e1013e..533b1dd9 100644 --- a/src/test/app/v/checklist-section.test.tsx +++ b/src/test/app/v/checklist-section.test.tsx @@ -92,4 +92,40 @@ describe("ChecklistSection", () => { expect(onItemUpdated.mock.calls.map(([updated]) => updated.id)).toEqual([1, 2]), ); }); + + it("keeps the inline error visible when one of two in-flight toggles fails and the other succeeds", async () => { + const a: VendorPortalChecklistItem = { id: 1, itemText: "Check filters", isCompleted: false }; + const b: VendorPortalChecklistItem = { id: 2, itemText: "Check belts", isCompleted: false }; + let rejectA: (error: Error) => void = () => {}; + let resolveB: () => void = () => {}; + let aRequested = false; + let bRequested = false; + vi.spyOn(vendorPortalApi, "updateChecklistItem").mockImplementation( + (_token, _dispatchId, itemId) => + itemId === 1 + ? new Promise((_resolve, reject) => { + rejectA = reject; + aRequested = true; + }) + : new Promise((resolve) => { + resolveB = () => resolve({ ...b, isCompleted: true }); + bRequested = true; + }), + ); + const onItemUpdated = vi.fn(); + renderSection({ items: [a, b] }, onItemUpdated); + + const [boxA, boxB] = screen.getAllByRole("checkbox"); + fireEvent.click(boxA); + fireEvent.click(boxB); + await waitFor(() => expect(aRequested && bRequested).toBe(true)); + + rejectA(new Error("boom")); + resolveB(); + + await waitFor(() => expect(onItemUpdated).toHaveBeenCalledWith({ ...b, isCompleted: true })); + expect( + await screen.findByText("Unable to update the checklist. Please try again."), + ).toBeInTheDocument(); + }); }); diff --git a/src/test/app/v/comments-section.test.tsx b/src/test/app/v/comments-section.test.tsx index 5ef0c60f..de350524 100644 --- a/src/test/app/v/comments-section.test.tsx +++ b/src/test/app/v/comments-section.test.tsx @@ -3,10 +3,16 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { CommentsSection } from "@/app/v/[token]/dispatch/_components/comments-section"; import { vendorPortalApi } from "@/domain/vendor-portal/api/vendor-portal-api"; import type { VendorPortalComment } from "@/domain/vendor-portal/types/vendor-portal"; +import { createAppQueryClient } from "@/lib/query/query-client"; import { renderWithProviders } from "@/test/test-utils"; +const toastMocks = vi.hoisted(() => ({ success: vi.fn(), error: vi.fn() })); +vi.mock("react-toastify", () => ({ toast: toastMocks })); + afterEach(() => { vi.restoreAllMocks(); + toastMocks.success.mockReset(); + toastMocks.error.mockReset(); }); function renderSection(comments: VendorPortalComment[] = [], onAdded = vi.fn()) { @@ -19,6 +25,17 @@ function renderSection(comments: VendorPortalComment[] = [], onAdded = vi.fn()) }; } +/** The app's real query client, so the global mutation error toast is live. */ +function renderSectionWithAppQueryClient(onAdded = vi.fn()) { + return { + onAdded, + ...renderWithProviders( + , + { withAuth: false, queryClient: createAppQueryClient() }, + ), + }; +} + describe("CommentsSection", () => { it("shows a placeholder when there are no comments yet", () => { renderSection(); @@ -70,4 +87,18 @@ describe("CommentsSection", () => { screen.queryByText("Unable to post your comment. Please try again."), ).not.toBeInTheDocument(); }); + + it("shows only the inline error on a failed post, not a duplicate global toast", async () => { + vi.spyOn(vendorPortalApi, "addComment").mockRejectedValue(new Error("network down")); + renderSectionWithAppQueryClient(); + + const textarea = screen.getByPlaceholderText("Add a comment for the dispatcher…"); + fireEvent.change(textarea, { target: { value: "Looks good" } }); + fireEvent.click(screen.getByRole("button", { name: "Post Comment" })); + + expect( + await screen.findByText("Unable to post your comment. Please try again."), + ).toBeInTheDocument(); + expect(toastMocks.error).not.toHaveBeenCalled(); + }); }); diff --git a/src/test/app/v/signoff-section.test.tsx b/src/test/app/v/signoff-section.test.tsx index 3c2b8b25..e14e87ea 100644 --- a/src/test/app/v/signoff-section.test.tsx +++ b/src/test/app/v/signoff-section.test.tsx @@ -2,13 +2,22 @@ import { fireEvent, screen, waitFor } from "@testing-library/react"; import { afterEach, describe, expect, it, vi } from "vitest"; import { SignoffSection } from "@/app/v/[token]/dispatch/_components/signoff-section"; import { vendorPortalApi } from "@/domain/vendor-portal/api/vendor-portal-api"; +import { createAppQueryClient } from "@/lib/query/query-client"; import { renderWithProviders } from "@/test/test-utils"; +const toastMocks = vi.hoisted(() => ({ success: vi.fn(), error: vi.fn() })); +vi.mock("react-toastify", () => ({ toast: toastMocks })); + afterEach(() => { vi.restoreAllMocks(); + toastMocks.success.mockReset(); + toastMocks.error.mockReset(); }); -function renderSection(overrides: Partial> = {}) { +function renderSection( + overrides: Partial> = {}, + options: { useAppQueryClient?: boolean } = {}, +) { return renderWithProviders( , - { withAuth: false }, + { + withAuth: false, + ...(options.useAppQueryClient ? { queryClient: createAppQueryClient() } : {}), + }, ); } +function submitTypedSignoff(name = "Jane Doe") { + fireEvent.click(screen.getByRole("radio", { name: "Typed" })); + const [nameInput, signatureInput] = screen.getAllByRole("textbox"); + fireEvent.change(nameInput, { target: { value: name } }); + fireEvent.change(signatureInput, { target: { value: name } }); + fireEvent.click(screen.getByRole("button", { name: "Submit Signoff" })); +} + describe("SignoffSection", () => { it("shows status-neutral locked copy when submission is locked", () => { renderSection({ submissionLocked: true }); @@ -70,11 +90,7 @@ describe("SignoffSection", () => { const onAdded = vi.fn(); renderSection({ onAdded }); - fireEvent.click(screen.getByRole("radio", { name: "Typed" })); - const [nameInput, signatureInput] = screen.getAllByRole("textbox"); - fireEvent.change(nameInput, { target: { value: "Jane Doe" } }); - fireEvent.change(signatureInput, { target: { value: "Jane Doe" } }); - fireEvent.click(screen.getByRole("button", { name: "Submit Signoff" })); + submitTypedSignoff(); await waitFor(() => expect(onAdded).toHaveBeenCalled()); expect(vendorPortalApi.addSignoff).toHaveBeenCalledWith("portal-token", 9, { @@ -89,14 +105,22 @@ describe("SignoffSection", () => { vi.spyOn(vendorPortalApi, "addSignoff").mockRejectedValue(new Error("boom")); renderSection(); - fireEvent.click(screen.getByRole("radio", { name: "Typed" })); - const [nameInput, signatureInput] = screen.getAllByRole("textbox"); - fireEvent.change(nameInput, { target: { value: "Jane Doe" } }); - fireEvent.change(signatureInput, { target: { value: "Jane Doe" } }); - fireEvent.click(screen.getByRole("button", { name: "Submit Signoff" })); + submitTypedSignoff(); expect( await screen.findByText("Unable to submit the signoff. Please try again."), ).toBeInTheDocument(); }); + + it("shows only the inline error on a failed submit, not a duplicate global toast", async () => { + vi.spyOn(vendorPortalApi, "addSignoff").mockRejectedValue(new Error("boom")); + renderSection({}, { useAppQueryClient: true }); + + submitTypedSignoff(); + + expect( + await screen.findByText("Unable to submit the signoff. Please try again."), + ).toBeInTheDocument(); + expect(toastMocks.error).not.toHaveBeenCalled(); + }); }); diff --git a/src/test/app/v/uplift-requests-section.test.tsx b/src/test/app/v/uplift-requests-section.test.tsx new file mode 100644 index 00000000..a1c3c360 --- /dev/null +++ b/src/test/app/v/uplift-requests-section.test.tsx @@ -0,0 +1,102 @@ +import { fireEvent, screen, waitFor } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { UpliftRequestsSection } from "@/app/v/[token]/dispatch/_components/uplift-requests-section"; +import { vendorPortalApi } from "@/domain/vendor-portal/api/vendor-portal-api"; +import type { VendorPortalUpliftRequest } from "@/domain/vendor-portal/types/vendor-portal"; +import { createAppQueryClient } from "@/lib/query/query-client"; +import { renderWithProviders } from "@/test/test-utils"; + +const toastMocks = vi.hoisted(() => ({ success: vi.fn(), error: vi.fn() })); +vi.mock("react-toastify", () => ({ toast: toastMocks })); + +const pendingRequest: VendorPortalUpliftRequest = { + id: 501, + status: "Pending", + requestedNTE: 750, + currentNTE: 500, + raisedByVendor: true, +}; + +beforeEach(() => { + vi.spyOn(window, "confirm").mockReturnValue(true); +}); + +afterEach(() => { + vi.restoreAllMocks(); + toastMocks.success.mockReset(); + toastMocks.error.mockReset(); +}); + +function renderSection( + overrides: Partial> = {}, + options: { useAppQueryClient?: boolean } = {}, +) { + return renderWithProviders( + , + { + withAuth: false, + ...(options.useAppQueryClient ? { queryClient: createAppQueryClient() } : {}), + }, + ); +} + +describe("UpliftRequestsSection withdraw", () => { + it("withdraws a pending request and refreshes the dispatch", async () => { + vi.spyOn(vendorPortalApi, "withdrawUplift").mockResolvedValue(undefined); + const onChanged = vi.fn().mockResolvedValue(undefined); + renderSection({ onChanged }); + + fireEvent.click(screen.getByRole("button", { name: "Withdraw request" })); + + await waitFor(() => + expect(vendorPortalApi.withdrawUplift).toHaveBeenCalledWith( + "portal-token", + 9, + pendingRequest.id, + ), + ); + await waitFor(() => expect(onChanged).toHaveBeenCalled()); + }); + + it("does nothing when the confirm dialog is dismissed", () => { + const withdrawUplift = vi.spyOn(vendorPortalApi, "withdrawUplift"); + vi.spyOn(window, "confirm").mockReturnValue(false); + renderSection(); + + fireEvent.click(screen.getByRole("button", { name: "Withdraw request" })); + + expect(withdrawUplift).not.toHaveBeenCalled(); + }); + + it("shows an inline error when withdrawing fails", async () => { + vi.spyOn(vendorPortalApi, "withdrawUplift").mockRejectedValue(new Error("boom")); + renderSection(); + + fireEvent.click(screen.getByRole("button", { name: "Withdraw request" })); + + expect( + await screen.findByText("Unable to withdraw the uplift request. Please try again."), + ).toBeInTheDocument(); + }); + + it("shows only the inline error on a failed withdraw, not a duplicate global toast", async () => { + vi.spyOn(vendorPortalApi, "withdrawUplift").mockRejectedValue(new Error("boom")); + renderSection({}, { useAppQueryClient: true }); + + fireEvent.click(screen.getByRole("button", { name: "Withdraw request" })); + + expect( + await screen.findByText("Unable to withdraw the uplift request. Please try again."), + ).toBeInTheDocument(); + expect(toastMocks.error).not.toHaveBeenCalled(); + }); +});