From 61bb6af092ac57bcc63f9e65a5dbca8b5070cd7a Mon Sep 17 00:00:00 2001 From: nikit Date: Mon, 6 Jul 2026 20:36:35 -0400 Subject: [PATCH] fix(work-tracker): check res.ok on delete ops before toasting success Three deletes (session, expense, route) fired the DELETE request then unconditionally toasted success and refreshed, with no res.ok check and no .catch. A failed delete still showed "deleted", and a rejected fetch was an unhandled promise rejection. Added a regression test suite covering all three delete handlers across fail/success/reject paths (confirmed it fails without the fix, passes with it). Fixes #21 --- .../__tests__/work-tracker-delete.test.tsx | 77 +++++++++++++++++++ src/components/work/work-tracker.tsx | 42 +++++++--- 2 files changed, 110 insertions(+), 9 deletions(-) create mode 100644 src/components/work/__tests__/work-tracker-delete.test.tsx diff --git a/src/components/work/__tests__/work-tracker-delete.test.tsx b/src/components/work/__tests__/work-tracker-delete.test.tsx new file mode 100644 index 0000000..88d5507 --- /dev/null +++ b/src/components/work/__tests__/work-tracker-delete.test.tsx @@ -0,0 +1,77 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { render, screen, cleanup, fireEvent, waitFor } from "@testing-library/react"; +import { WorkTracker } from "../work-tracker"; + +vi.mock("next/navigation", () => ({ useRouter: () => ({ refresh: mockRefresh }) })); +vi.mock("sonner", () => ({ toast: { success: vi.fn(), error: vi.fn() } })); + +import { toast } from "sonner"; + +const mockRefresh = vi.fn(); + +const baseProps = { + jobName: "Cafe", + hourlyRate: 15, + weeklyTarget: 20, + enableExpenseTracking: true, + todayHours: 0, + weekHours: 0, + monthHours: 0, + sessions: [{ _id: "s1", date: "2026-01-01", hours: 4, note: "", jobName: "Cafe" }], + expenses: [{ _id: "e1", date: "2026-01-01", amount: 10, currency: "USD", description: "Gas", category: "other", reimbursed: false }], + routes: [{ _id: "r1", date: "2026-01-01", origin: "A", destination: "B", distanceKm: 5, note: "" }], + gasPrice: 1, + carConsumption: 5, + currency: "USD", +}; + +describe("WorkTracker delete handlers", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + afterEach(() => { + cleanup(); + }); + + const cases = [ + { label: "Delete session", tab: null }, + { label: "Delete expense", tab: "Expenses" }, + { label: "Delete route", tab: "Routes & Gas" }, + ]; + + for (const { label, tab } of cases) { + const openTab = () => { + if (tab) fireEvent.click(screen.getByRole("button", { name: tab })); + }; + + it(`shows an error toast and does not refresh when ${label.toLowerCase()} fails`, async () => { + global.fetch = vi.fn().mockResolvedValue({ ok: false }) as unknown as typeof fetch; + render(); + openTab(); + fireEvent.click(screen.getByLabelText(label)); + await waitFor(() => expect(toast.error).toHaveBeenCalled()); + expect(toast.success).not.toHaveBeenCalled(); + expect(mockRefresh).not.toHaveBeenCalled(); + }); + + it(`shows a success toast and refreshes when ${label.toLowerCase()} succeeds`, async () => { + global.fetch = vi.fn().mockResolvedValue({ ok: true }) as unknown as typeof fetch; + render(); + openTab(); + fireEvent.click(screen.getByLabelText(label)); + await waitFor(() => expect(toast.success).toHaveBeenCalled()); + expect(toast.error).not.toHaveBeenCalled(); + expect(mockRefresh).toHaveBeenCalled(); + }); + + it(`shows an error toast when the ${label.toLowerCase()} fetch rejects`, async () => { + global.fetch = vi.fn().mockRejectedValue(new Error("network error")) as unknown as typeof fetch; + render(); + openTab(); + fireEvent.click(screen.getByLabelText(label)); + await waitFor(() => expect(toast.error).toHaveBeenCalled()); + expect(mockRefresh).not.toHaveBeenCalled(); + }); + } +}); diff --git a/src/components/work/work-tracker.tsx b/src/components/work/work-tracker.tsx index 6d99521..d8f3bda 100644 --- a/src/components/work/work-tracker.tsx +++ b/src/components/work/work-tracker.tsx @@ -242,9 +242,17 @@ export function WorkTracker({ size="icon" aria-label="Delete session" onClick={async () => { - await fetch(`/api/work/sessions/${s._id}`, { method: "DELETE" }); - toast.success("Session deleted"); - router.refresh(); + try { + const res = await fetch(`/api/work/sessions/${s._id}`, { method: "DELETE" }); + if (res.ok) { + toast.success("Session deleted"); + router.refresh(); + } else { + toast.error("Failed to delete session"); + } + } catch { + toast.error("Failed to delete session"); + } }} className="h-7 w-7 text-[var(--text-muted)] hover:text-destructive" > @@ -329,9 +337,17 @@ export function WorkTracker({ size="icon" aria-label="Delete expense" onClick={async () => { - await fetch(`/api/expenses/${e._id}`, { method: "DELETE" }); - toast.success("Expense deleted"); - router.refresh(); + try { + const res = await fetch(`/api/expenses/${e._id}`, { method: "DELETE" }); + if (res.ok) { + toast.success("Expense deleted"); + router.refresh(); + } else { + toast.error("Failed to delete expense"); + } + } catch { + toast.error("Failed to delete expense"); + } }} className="h-7 w-7 text-[var(--text-muted)] hover:text-destructive" > @@ -426,9 +442,17 @@ export function WorkTracker({ size="icon" aria-label="Delete route" onClick={async () => { - await fetch(`/api/routes/${r._id}`, { method: "DELETE" }); - toast.success("Route deleted"); - router.refresh(); + try { + const res = await fetch(`/api/routes/${r._id}`, { method: "DELETE" }); + if (res.ok) { + toast.success("Route deleted"); + router.refresh(); + } else { + toast.error("Failed to delete route"); + } + } catch { + toast.error("Failed to delete route"); + } }} className="h-7 w-7 text-[var(--text-muted)] hover:text-destructive" >