progress enforcing adr-0005

src/hooks/useWatchedItems.test.tsx

@@ -1,12 +1,11 @@
 // src/hooks/useWatchedItems.test.tsx
-import { renderHook, act, waitFor } from '@testing-library/react';
+import { renderHook, act } from '@testing-library/react';
 import { describe, it, expect, vi, beforeEach } from 'vitest';
 import { useWatchedItems } from './useWatchedItems';
-import { useApi } from './useApi';
 import { useAuth } from '../hooks/useAuth';
 import { useUserData } from '../hooks/useUserData';
-import * as apiClient from '../services/apiClient';
-import type { MasterGroceryItem, User } from '../types';
+import { useAddWatchedItemMutation, useRemoveWatchedItemMutation } from './mutations';
+import type { User } from '../types';
 import {
   createMockMasterGroceryItem,
   createMockUser,
@@ -14,14 +13,17 @@ import {
 } from '../tests/utils/mockFactories';
 
 // Mock the hooks that useWatchedItems depends on
-vi.mock('./useApi');
 vi.mock('../hooks/useAuth');
 vi.mock('../hooks/useUserData');
+vi.mock('./mutations', () => ({
+  useAddWatchedItemMutation: vi.fn(),
+  useRemoveWatchedItemMutation: vi.fn(),
+}));
 
-// The apiClient is globally mocked in our test setup, so we just need to cast it
-const mockedUseApi = vi.mocked(useApi);
 const mockedUseAuth = vi.mocked(useAuth);
 const mockedUseUserData = vi.mocked(useUserData);
+const mockedUseAddWatchedItemMutation = vi.mocked(useAddWatchedItemMutation);
+const mockedUseRemoveWatchedItemMutation = vi.mocked(useRemoveWatchedItemMutation);
 
 const mockUser: User = createMockUser({ user_id: 'user-123', email: 'test@example.com' });
 const mockInitialItems = [
@@ -30,46 +32,34 @@ const mockInitialItems = [
 ];
 
 describe('useWatchedItems Hook', () => {
-  // Create a mock setter function that we can spy on
-  const mockSetWatchedItems = vi.fn();
-  const mockAddWatchedItemApi = vi.fn();
-  const mockRemoveWatchedItemApi = vi.fn();
+  const mockMutateAsync = vi.fn();
+  const mockAddMutation = {
+    mutateAsync: mockMutateAsync,
+    mutate: vi.fn(),
+    isPending: false,
+    error: null,
+    isError: false,
+    isSuccess: false,
+    isIdle: true,
+  };
+  const mockRemoveMutation = {
+    mutateAsync: mockMutateAsync,
+    mutate: vi.fn(),
+    isPending: false,
+    error: null,
+    isError: false,
+    isSuccess: false,
+    isIdle: true,
+  };
 
   beforeEach(() => {
-    // Reset all mocks before each test to ensure isolation
-    // Use resetAllMocks to ensure previous test implementations (like mockResolvedValue) don't leak.
     vi.resetAllMocks();
-    // Default mock for useApi to handle any number of calls/re-renders safely
-    mockedUseApi.mockReturnValue({
-      execute: vi.fn(),
-      error: null,
-      data: null,
-      loading: false,
-      isRefetching: false,
-      reset: vi.fn(),
-    });
 
-    // Specific overrides for the first render sequence:
-    // 1st call = addWatchedItemApi, 2nd call = removeWatchedItemApi
-    mockedUseApi
-      .mockReturnValueOnce({
-        execute: mockAddWatchedItemApi,
-        error: null,
-        data: null,
-        loading: false,
-        isRefetching: false,
-        reset: vi.fn(),
-      })
-      .mockReturnValueOnce({
-        execute: mockRemoveWatchedItemApi,
-        error: null,
-        data: null,
-        loading: false,
-        isRefetching: false,
-        reset: vi.fn(),
-      });
+    // Mock TanStack Query mutation hooks
+    mockedUseAddWatchedItemMutation.mockReturnValue(mockAddMutation as any);
+    mockedUseRemoveWatchedItemMutation.mockReturnValue(mockRemoveMutation as any);
 
-    // Provide a default implementation for the mocked hooks
+    // Provide default implementation for auth
     mockedUseAuth.mockReturnValue({
       userProfile: createMockUserProfile({ user: mockUser }),
       authStatus: 'AUTHENTICATED',
@@ -79,11 +69,10 @@ describe('useWatchedItems Hook', () => {
       updateProfile: vi.fn(),
     });
 
+    // Provide default implementation for user data (no more setters!)
     mockedUseUserData.mockReturnValue({
       watchedItems: mockInitialItems,
-      setWatchedItems: mockSetWatchedItems,
       shoppingLists: [],
-      setShoppingLists: vi.fn(),
       isLoading: false,
       error: null,
     });
@@ -96,26 +85,17 @@ describe('useWatchedItems Hook', () => {
     expect(result.current.error).toBeNull();
   });
 
-  it('should configure useApi with the correct apiClient methods', async () => {
+  it('should use TanStack Query mutation hooks', () => {
     renderHook(() => useWatchedItems());
 
-    // useApi is called twice: once for add, once for remove
-    const addApiCall = mockedUseApi.mock.calls[0][0];
-    const removeApiCall = mockedUseApi.mock.calls[1][0];
-
-    // Test the add callback
-    await addApiCall('New Item', 'Category');
-    expect(apiClient.addWatchedItem).toHaveBeenCalledWith('New Item', 'Category');
-
-    // Test the remove callback
-    await removeApiCall(123);
-    expect(apiClient.removeWatchedItem).toHaveBeenCalledWith(123);
+    // Verify that the mutation hooks were called
+    expect(mockedUseAddWatchedItemMutation).toHaveBeenCalled();
+    expect(mockedUseRemoveWatchedItemMutation).toHaveBeenCalled();
   });
 
   describe('addWatchedItem', () => {
-    it('should call the API and update state on successful addition', async () => {
-      const newItem = createMockMasterGroceryItem({ master_grocery_item_id: 3, name: 'Cheese' });
-      mockAddWatchedItemApi.mockResolvedValue(newItem);
+    it('should call the mutation with correct parameters', async () => {
+      mockMutateAsync.mockResolvedValue({});
 
       const { result } = renderHook(() => useWatchedItems());
 
@@ -123,168 +103,69 @@ describe('useWatchedItems Hook', () => {
         await result.current.addWatchedItem('Cheese', 'Dairy');
       });
 
-      expect(mockAddWatchedItemApi).toHaveBeenCalledWith('Cheese', 'Dairy');
-      // Check that the global state setter was called with an updater function
-      expect(mockSetWatchedItems).toHaveBeenCalledWith(expect.any(Function));
-
-      // To verify the logic inside the updater, we can call it directly
-      const updater = mockSetWatchedItems.mock.calls[0][0];
-      const newState = updater(mockInitialItems);
-
-      expect(newState).toHaveLength(3);
-      expect(newState).toContainEqual(newItem);
+      // Verify mutation was called with correct parameters
+      expect(mockMutateAsync).toHaveBeenCalledWith({
+        itemName: 'Cheese',
+        category: 'Dairy',
+      });
     });
 
-    it('should set an error message if the API call fails', async () => {
-      // Clear existing mocks to set a specific sequence for this test
-      mockedUseApi.mockReset();
+    it('should expose error from mutation', () => {
+      const errorMutation = {
+        ...mockAddMutation,
+        error: new Error('API Error'),
+      };
+      mockedUseAddWatchedItemMutation.mockReturnValue(errorMutation as any);
 
-      // Default fallback
-      mockedUseApi.mockReturnValue({
-        execute: vi.fn(),
-        error: null,
-        data: null,
-        loading: false,
-        isRefetching: false,
-        reset: vi.fn(),
-      });
+      const { result } = renderHook(() => useWatchedItems());
 
-      // Mock the first call (add) to return an error immediately
-      mockedUseApi
-        .mockReturnValueOnce({
-          execute: mockAddWatchedItemApi,
-          error: new Error('API Error'),
-          data: null,
-          loading: false,
-          isRefetching: false,
-          reset: vi.fn(),
-        })
-        .mockReturnValueOnce({
-          execute: mockRemoveWatchedItemApi,
-          error: null,
-          data: null,
-          loading: false,
-          isRefetching: false,
-          reset: vi.fn(),
-        });
+      expect(result.current.error).toBe('API Error');
+    });
+
+    it('should handle mutation errors gracefully', async () => {
+      mockMutateAsync.mockRejectedValue(new Error('Failed to add'));
 
       const { result } = renderHook(() => useWatchedItems());
 
       await act(async () => {
         await result.current.addWatchedItem('Failing Item', 'Error');
       });
-      expect(result.current.error).toBe('API Error');
-      expect(mockSetWatchedItems).not.toHaveBeenCalled();
-    });
 
-    it('should not add duplicate items to the state', async () => {
-      // Item ID 1 ('Milk') already exists in mockInitialItems
-      const existingItem = createMockMasterGroceryItem({ master_grocery_item_id: 1, name: 'Milk' });
-      mockAddWatchedItemApi.mockResolvedValue(existingItem);
-
-      const { result } = renderHook(() => useWatchedItems());
-
-      await act(async () => {
-        await result.current.addWatchedItem('Milk', 'Dairy');
-      });
-
-      expect(mockAddWatchedItemApi).toHaveBeenCalledWith('Milk', 'Dairy');
-
-      // Get the updater function passed to setWatchedItems
-      const updater = mockSetWatchedItems.mock.calls[0][0];
-      const newState = updater(mockInitialItems);
-
-      // Should be unchanged
-      expect(newState).toEqual(mockInitialItems);
-      expect(newState).toHaveLength(2);
-    });
-
-    it('should sort items alphabetically by name when adding a new item', async () => {
-      const unsortedItems = [
-        createMockMasterGroceryItem({ master_grocery_item_id: 2, name: 'Zucchini' }),
-        createMockMasterGroceryItem({ master_grocery_item_id: 1, name: 'Apple' }),
-      ];
-
-      const newItem = createMockMasterGroceryItem({ master_grocery_item_id: 3, name: 'Banana' });
-      mockAddWatchedItemApi.mockResolvedValue(newItem);
-
-      const { result } = renderHook(() => useWatchedItems());
-
-      await act(async () => {
-        await result.current.addWatchedItem('Banana', 'Fruit');
-      });
-
-      const updater = mockSetWatchedItems.mock.calls[0][0];
-      const newState = updater(unsortedItems);
-
-      expect(newState).toHaveLength(3);
-      expect(newState[0].name).toBe('Apple');
-      expect(newState[1].name).toBe('Banana');
-      expect(newState[2].name).toBe('Zucchini');
+      // Should not throw - error is caught and logged
+      expect(mockMutateAsync).toHaveBeenCalled();
     });
   });
 
   describe('removeWatchedItem', () => {
-    it('should call the API and update state on successful removal', async () => {
-      const itemIdToRemove = 1;
-      mockRemoveWatchedItemApi.mockResolvedValue(null); // Successful 204 returns null
+    it('should call the mutation with correct parameters', async () => {
+      mockMutateAsync.mockResolvedValue({});
 
       const { result } = renderHook(() => useWatchedItems());
 
       await act(async () => {
-        await result.current.removeWatchedItem(itemIdToRemove);
+        await result.current.removeWatchedItem(1);
       });
 
-      await waitFor(() => {
-        expect(mockRemoveWatchedItemApi).toHaveBeenCalledWith(itemIdToRemove);
+      // Verify mutation was called with correct parameters
+      expect(mockMutateAsync).toHaveBeenCalledWith({
+        masterItemId: 1,
       });
-      expect(mockSetWatchedItems).toHaveBeenCalledWith(expect.any(Function));
-
-      // Verify the logic inside the updater function
-      const updater = mockSetWatchedItems.mock.calls[0][0];
-      const newState = updater(mockInitialItems);
-
-      expect(newState).toHaveLength(1);
-      expect(
-        newState.some((item: MasterGroceryItem) => item.master_grocery_item_id === itemIdToRemove),
-      ).toBe(false);
     });
 
-    it('should set an error message if the API call fails', async () => {
-      // Clear existing mocks
-      mockedUseApi.mockReset();
+    it('should expose error from remove mutation', () => {
+      const errorMutation = {
+        ...mockRemoveMutation,
+        error: new Error('Deletion Failed'),
+      };
+      mockedUseRemoveWatchedItemMutation.mockReturnValue(errorMutation as any);
 
-      // Ensure the execute function returns null/undefined so the hook doesn't try to set state
-      mockAddWatchedItemApi.mockResolvedValue(null);
+      const { result } = renderHook(() => useWatchedItems());
 
-      // Default fallback
-      mockedUseApi.mockReturnValue({
-        execute: vi.fn(),
-        error: null,
-        data: null,
-        loading: false,
-        isRefetching: false,
-        reset: vi.fn(),
-      });
+      expect(result.current.error).toBe('Deletion Failed');
+    });
 
-      // Mock sequence: 1st (add) success, 2nd (remove) error
-      mockedUseApi
-        .mockReturnValueOnce({
-          execute: vi.fn(),
-          error: null,
-          data: null,
-          loading: false,
-          isRefetching: false,
-          reset: vi.fn(),
-        })
-        .mockReturnValueOnce({
-          execute: vi.fn(),
-          error: new Error('Deletion Failed'),
-          data: null,
-          loading: false,
-          isRefetching: false,
-          reset: vi.fn(),
-        });
+    it('should handle mutation errors gracefully', async () => {
+      mockMutateAsync.mockRejectedValue(new Error('Failed to remove'));
 
       const { result } = renderHook(() => useWatchedItems());
 
@@ -292,8 +173,8 @@ describe('useWatchedItems Hook', () => {
         await result.current.removeWatchedItem(999);
       });
 
-      expect(result.current.error).toBe('Deletion Failed');
-      expect(mockSetWatchedItems).not.toHaveBeenCalled();
+      // Should not throw - error is caught and logged
+      expect(mockMutateAsync).toHaveBeenCalled();
     });
   });
 
@@ -314,7 +195,7 @@ describe('useWatchedItems Hook', () => {
       await result.current.removeWatchedItem(1);
     });
 
-    expect(mockAddWatchedItemApi).not.toHaveBeenCalled();
-    expect(mockRemoveWatchedItemApi).not.toHaveBeenCalled();
+    // Mutations should not be called when user is not authenticated
+    expect(mockMutateAsync).not.toHaveBeenCalled();
   });
 });