diff --git a/apps/web/src/components/kanban/KanbanBoard.test.tsx b/apps/web/src/components/kanban/KanbanBoard.test.tsx index d7ea43d..2ddfd3c 100644 --- a/apps/web/src/components/kanban/KanbanBoard.test.tsx +++ b/apps/web/src/components/kanban/KanbanBoard.test.tsx @@ -1,22 +1,53 @@ /* eslint-disable @typescript-eslint/no-non-null-assertion */ /* eslint-disable @typescript-eslint/no-empty-function */ import { describe, it, expect, vi, beforeEach } from "vitest"; -import { render, screen, within } from "@testing-library/react"; +import { render, screen, within, waitFor, act } from "@testing-library/react"; import { KanbanBoard } from "./KanbanBoard"; import type { Task } from "@mosaic/shared"; import { TaskStatus, TaskPriority } from "@mosaic/shared"; +import type { ToastContextValue } from "@mosaic/ui"; // Mock fetch globally global.fetch = vi.fn(); +// Mock useToast hook from @mosaic/ui +const mockShowToast = vi.fn(); +vi.mock("@mosaic/ui", () => ({ + useToast: (): ToastContextValue => ({ + showToast: mockShowToast, + removeToast: vi.fn(), + }), +})); + +// Mock the api client's apiPatch function +const mockApiPatch = vi.fn<(endpoint: string, data: unknown) => Promise>(); +vi.mock("@/lib/api/client", () => ({ + apiPatch: (endpoint: string, data: unknown): Promise => mockApiPatch(endpoint, data), +})); + +// Store drag event handlers for testing +type DragEventHandler = (event: { + active: { id: string }; + over: { id: string } | null; +}) => Promise | void; +let capturedOnDragEnd: DragEventHandler | null = null; + // Mock @dnd-kit modules vi.mock("@dnd-kit/core", async () => { const actual = await vi.importActual("@dnd-kit/core"); return { ...actual, - DndContext: ({ children }: { children: React.ReactNode }): React.JSX.Element => ( -
{children}
- ), + DndContext: ({ + children, + onDragEnd, + }: { + children: React.ReactNode; + onDragEnd?: DragEventHandler; + }): React.JSX.Element => { + // Capture the event handler for testing + capturedOnDragEnd = onDragEnd ?? null; + return
{children}
; + }, }; }); @@ -114,9 +145,14 @@ describe("KanbanBoard", (): void => { beforeEach((): void => { vi.clearAllMocks(); + mockShowToast.mockClear(); + mockApiPatch.mockClear(); + // Default: apiPatch succeeds + mockApiPatch.mockResolvedValue({}); + // Also set up fetch mock for other tests that may use it (global.fetch as ReturnType).mockResolvedValue({ ok: true, - json: () => ({}), + json: (): Promise => Promise.resolve({}), } as Response); }); @@ -273,6 +309,191 @@ describe("KanbanBoard", (): void => { }); }); + describe("Optimistic Updates and Rollback", (): void => { + it("should apply optimistic update immediately on drag", async (): Promise => { + // apiPatch is already mocked to succeed in beforeEach + render(); + + // Verify initial state - task-1 is in NOT_STARTED column + const todoColumn = screen.getByTestId("column-NOT_STARTED"); + expect(within(todoColumn).getByText("Design homepage")).toBeInTheDocument(); + + // Trigger drag end event to move task-1 to IN_PROGRESS and wait for completion + await act(async () => { + if (capturedOnDragEnd) { + const result = capturedOnDragEnd({ + active: { id: "task-1" }, + over: { id: TaskStatus.IN_PROGRESS }, + }); + if (result instanceof Promise) { + await result; + } + } + }); + + // After the drag completes, task should be in the new column (optimistic update persisted) + const inProgressColumn = screen.getByTestId("column-IN_PROGRESS"); + expect(within(inProgressColumn).getByText("Design homepage")).toBeInTheDocument(); + + // Verify the task is NOT in the original column anymore + const todoColumnAfter = screen.getByTestId("column-NOT_STARTED"); + expect(within(todoColumnAfter).queryByText("Design homepage")).not.toBeInTheDocument(); + }); + + it("should persist update when API call succeeds", async (): Promise => { + // apiPatch is already mocked to succeed in beforeEach + render(); + + // Trigger drag end event + await act(async () => { + if (capturedOnDragEnd) { + const result = capturedOnDragEnd({ + active: { id: "task-1" }, + over: { id: TaskStatus.IN_PROGRESS }, + }); + if (result instanceof Promise) { + await result; + } + } + }); + + // Wait for API call to complete + await waitFor(() => { + expect(mockApiPatch).toHaveBeenCalledWith("/api/tasks/task-1", { + status: TaskStatus.IN_PROGRESS, + }); + }); + + // Verify task is in the new column after API success + const inProgressColumn = screen.getByTestId("column-IN_PROGRESS"); + expect(within(inProgressColumn).getByText("Design homepage")).toBeInTheDocument(); + + // Verify callback was called + expect(mockOnStatusChange).toHaveBeenCalledWith("task-1", TaskStatus.IN_PROGRESS); + }); + + it("should rollback to original position when API call fails", async (): Promise => { + const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + + // Mock API failure + mockApiPatch.mockRejectedValueOnce(new Error("Network error")); + + render(); + + // Verify initial state - task-1 is in NOT_STARTED column + const todoColumnBefore = screen.getByTestId("column-NOT_STARTED"); + expect(within(todoColumnBefore).getByText("Design homepage")).toBeInTheDocument(); + + // Trigger drag end event + await act(async () => { + if (capturedOnDragEnd) { + const result = capturedOnDragEnd({ + active: { id: "task-1" }, + over: { id: TaskStatus.IN_PROGRESS }, + }); + if (result instanceof Promise) { + await result; + } + } + }); + + // Wait for rollback to occur + await waitFor(() => { + // After rollback, task should be back in original column + const todoColumnAfter = screen.getByTestId("column-NOT_STARTED"); + expect(within(todoColumnAfter).getByText("Design homepage")).toBeInTheDocument(); + }); + + // Verify callback was NOT called due to error + expect(mockOnStatusChange).not.toHaveBeenCalled(); + + consoleErrorSpy.mockRestore(); + }); + + it("should show error toast notification when API call fails", async (): Promise => { + const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + + // Mock API failure + mockApiPatch.mockRejectedValueOnce(new Error("Server error")); + + render(); + + // Trigger drag end event + await act(async () => { + if (capturedOnDragEnd) { + const result = capturedOnDragEnd({ + active: { id: "task-1" }, + over: { id: TaskStatus.IN_PROGRESS }, + }); + if (result instanceof Promise) { + await result; + } + } + }); + + // Wait for error handling + await waitFor(() => { + // Verify showToast was called with error message + expect(mockShowToast).toHaveBeenCalledWith( + "Unable to update task status. Please try again.", + "error" + ); + }); + + consoleErrorSpy.mockRestore(); + }); + + it("should not make API call when dropping on same column", async (): Promise => { + const fetchMock = global.fetch as ReturnType; + + render(); + + // Trigger drag end event with same status + await act(async () => { + if (capturedOnDragEnd) { + const result = capturedOnDragEnd({ + active: { id: "task-1" }, + over: { id: TaskStatus.NOT_STARTED }, // Same as task's current status + }); + if (result instanceof Promise) { + await result; + } + } + }); + + // No API call should be made + expect(fetchMock).not.toHaveBeenCalled(); + // No callback should be called + expect(mockOnStatusChange).not.toHaveBeenCalled(); + }); + + it("should handle drag cancel (no drop target)", async (): Promise => { + const fetchMock = global.fetch as ReturnType; + + render(); + + // Trigger drag end event with no drop target + await act(async () => { + if (capturedOnDragEnd) { + const result = capturedOnDragEnd({ + active: { id: "task-1" }, + over: null, + }); + if (result instanceof Promise) { + await result; + } + } + }); + + // Task should remain in original column + const todoColumn = screen.getByTestId("column-NOT_STARTED"); + expect(within(todoColumn).getByText("Design homepage")).toBeInTheDocument(); + + // No API call should be made + expect(fetchMock).not.toHaveBeenCalled(); + }); + }); + describe("Accessibility", (): void => { it("should have proper heading hierarchy", (): void => { render(); diff --git a/apps/web/src/components/kanban/KanbanBoard.tsx b/apps/web/src/components/kanban/KanbanBoard.tsx index bf721d9..1bcd4e1 100644 --- a/apps/web/src/components/kanban/KanbanBoard.tsx +++ b/apps/web/src/components/kanban/KanbanBoard.tsx @@ -1,7 +1,7 @@ /* eslint-disable @typescript-eslint/no-unnecessary-condition */ "use client"; -import React, { useState, useMemo } from "react"; +import React, { useState, useMemo, useEffect, useCallback } from "react"; import type { Task } from "@mosaic/shared"; import { TaskStatus } from "@mosaic/shared"; import type { DragEndEvent, DragStartEvent } from "@dnd-kit/core"; @@ -9,6 +9,7 @@ import { DndContext, DragOverlay, PointerSensor, useSensor, useSensors } from "@ import { KanbanColumn } from "./KanbanColumn"; import { TaskCard } from "./TaskCard"; import { apiPatch } from "@/lib/api/client"; +import { useToast } from "@mosaic/ui"; interface KanbanBoardProps { tasks: Task[]; @@ -34,9 +35,18 @@ const columns = [ * - Drag-and-drop using @dnd-kit/core * - Task cards with title, priority badge, assignee avatar * - PATCH /api/tasks/:id on status change + * - Optimistic updates with rollback on error */ export function KanbanBoard({ tasks, onStatusChange }: KanbanBoardProps): React.ReactElement { const [activeTaskId, setActiveTaskId] = useState(null); + // Local task state for optimistic updates + const [localTasks, setLocalTasks] = useState(tasks || []); + const { showToast } = useToast(); + + // Sync local state with props when tasks prop changes + useEffect(() => { + setLocalTasks(tasks || []); + }, [tasks]); const sensors = useSensors( useSensor(PointerSensor, { @@ -46,7 +56,7 @@ export function KanbanBoard({ tasks, onStatusChange }: KanbanBoardProps): React. }) ); - // Group tasks by status + // Group tasks by status (using local state for optimistic updates) const tasksByStatus = useMemo(() => { const grouped: Record = { [TaskStatus.NOT_STARTED]: [], @@ -56,7 +66,7 @@ export function KanbanBoard({ tasks, onStatusChange }: KanbanBoardProps): React. [TaskStatus.ARCHIVED]: [], }; - (tasks || []).forEach((task) => { + localTasks.forEach((task) => { if (grouped[task.status]) { grouped[task.status].push(task); } @@ -68,17 +78,29 @@ export function KanbanBoard({ tasks, onStatusChange }: KanbanBoardProps): React. }); return grouped; - }, [tasks]); + }, [localTasks]); const activeTask = useMemo( - () => (tasks || []).find((task) => task.id === activeTaskId), - [tasks, activeTaskId] + () => localTasks.find((task) => task.id === activeTaskId), + [localTasks, activeTaskId] ); function handleDragStart(event: DragStartEvent): void { setActiveTaskId(event.active.id as string); } + // Apply optimistic update to local state + const applyOptimisticUpdate = useCallback((taskId: string, newStatus: TaskStatus): void => { + setLocalTasks((prevTasks) => + prevTasks.map((task) => (task.id === taskId ? { ...task, status: newStatus } : task)) + ); + }, []); + + // Rollback to previous state + const rollbackUpdate = useCallback((previousTasks: Task[]): void => { + setLocalTasks(previousTasks); + }, []); + async function handleDragEnd(event: DragEndEvent): Promise { const { active, over } = event; @@ -91,9 +113,15 @@ export function KanbanBoard({ tasks, onStatusChange }: KanbanBoardProps): React. const newStatus = over.id as TaskStatus; // Find the task and check if status actually changed - const task = (tasks || []).find((t) => t.id === taskId); + const task = localTasks.find((t) => t.id === taskId); if (task && task.status !== newStatus) { + // Store previous state for potential rollback + const previousTasks = [...localTasks]; + + // Apply optimistic update immediately + applyOptimisticUpdate(taskId, newStatus); + // Call PATCH /api/tasks/:id to update status (using API client for CSRF protection) try { await apiPatch(`/api/tasks/${taskId}`, { status: newStatus }); @@ -103,8 +131,12 @@ export function KanbanBoard({ tasks, onStatusChange }: KanbanBoardProps): React. onStatusChange(taskId, newStatus); } } catch (error) { + // Rollback to previous state on error + rollbackUpdate(previousTasks); + + // Show error notification + showToast("Unable to update task status. Please try again.", "error"); console.error("Error updating task status:", error); - // TODO: Show error toast/notification } }