unit test fixes

src/hooks/useShoppingLists.test.tsx

@@ -1,6 +1,6 @@
 // src/hooks/useShoppingLists.test.tsx
 import { renderHook, act, waitFor } from '@testing-library/react';
-import { describe, it, expect, vi, beforeEach, Mock } from 'vitest';
+import { describe, it, expect, vi, beforeEach, type Mock, test } from 'vitest';
 import { useShoppingLists } from './useShoppingLists';
 import { useApi } from './useApi';
 import { useAuth } from '../hooks/useAuth';
@@ -8,6 +8,16 @@ import { useUserData } from '../hooks/useUserData';
 import type { ShoppingList, ShoppingListItem, User } from '../types';
 import React from 'react'; // Required for Dispatch/SetStateAction types
 
+// Define a type for the mock return value of useApi to ensure type safety in tests
+type MockApiResult = {
+  execute: Mock;
+  error: Error | null;
+  loading: boolean;
+  isRefetching: boolean;
+  data: any;
+  reset: Mock;
+};
+
 // Mock the hooks that useShoppingLists depends on
 vi.mock('./useApi');
 vi.mock('../hooks/useAuth');
@@ -31,56 +41,30 @@ describe('useShoppingLists Hook', () => {
   const mockUpdateItemApi = vi.fn();
   const mockRemoveItemApi = vi.fn();
 
+  const defaultApiMocks: MockApiResult[] = [
+    { execute: mockCreateListApi, error: null, loading: false, isRefetching: false, data: null, reset: vi.fn() },
+    { execute: mockDeleteListApi, error: null, loading: false, isRefetching: false, data: null, reset: vi.fn() },
+    { execute: mockAddItemApi, error: null, loading: false, isRefetching: false, data: null, reset: vi.fn() },
+    { execute: mockUpdateItemApi, error: null, loading: false, isRefetching: false, data: null, reset: vi.fn() },
+    { execute: mockRemoveItemApi, error: null, loading: false, isRefetching: false, data: null, reset: vi.fn() },
+  ];
+
+  // Helper function to set up the useApi mock for a specific test run
+  const setupApiMocks = (mocks: MockApiResult[] = defaultApiMocks) => {
+    let callCount = 0;
+    mockedUseApi.mockImplementation(() => {
+      const mock = mocks[callCount % mocks.length];
+      callCount++;
+      return mock;
+    });
+  };
+
   beforeEach(() => {
     // Reset all mocks before each test to ensure isolation
     vi.clearAllMocks();
 
-    // Define the sequence of mocks corresponding to the hook's useApi calls
-    const apiMocks = [
-      { execute: mockCreateListApi, error: null, loading: false, isRefetching: false, data: null, reset: vi.fn() },
-      { execute: mockDeleteListApi, error: null, loading: false, isRefetching: false, data: null, reset: vi.fn() },
-      { execute: mockAddItemApi, error: null, loading: false, isRefetching: false, data: null, reset: vi.fn() },
-      { execute: mockUpdateItemApi, error: null, loading: false, isRefetching: false, data: null, reset: vi.fn() },
-      { execute: mockRemoveItemApi, error: null, loading: false, isRefetching: false, data: null, reset: vi.fn() },
-    ];
-
-    let callCount = 0;
-    mockedUseApi.mockImplementation(() => {
-      const mockIndex = callCount % apiMocks.length;
-      callCount++;
-      const mockConfig = apiMocks[mockIndex];
-
-      // eslint-disable-next-line react-hooks/rules-of-hooks
-      const [state, setState] = React.useState<{
-        data: any;
-        error: Error | null;
-        loading: boolean;
-      }>({
-        data: mockConfig.data,
-        error: mockConfig.error,
-        loading: mockConfig.loading
-      });
-
-      // eslint-disable-next-line react-hooks/rules-of-hooks
-      const execute = React.useCallback(async (...args: any[]) => {
-        setState(prev => ({ ...prev, loading: true, error: null }));
-        try {
-          const result = await mockConfig.execute(...args);
-          setState({ data: result, loading: false, error: null });
-          return result;
-        } catch (err) {
-          setState({ data: null, loading: false, error: err as Error });
-          return null;
-        }
-      }, [mockConfig]);
-
-      return {
-        ...state,
-        execute,
-        isRefetching: false,
-        reset: vi.fn()
-      };
-    });
+    // Mock useApi to return a sequence of successful API configurations by default
+    setupApiMocks();
 
     mockedUseAuth.mockReturnValue({
       user: mockUser,
@@ -156,6 +140,56 @@ describe('useShoppingLists Hook', () => {
     expect(result.current.activeListId).toBeNull();
   });
 
+  it('should set activeListId to null when lists become empty', async () => {
+    const mockLists: ShoppingList[] = [{ shopping_list_id: 1, name: 'Groceries', user_id: 'user-123', created_at: '', items: [] }];
+
+    // Initial render with a list
+    mockedUseUserData.mockReturnValue({
+      shoppingLists: mockLists,
+      setShoppingLists: mockSetShoppingLists,
+      watchedItems: [],
+      setWatchedItems: vi.fn(),
+      isLoading: false,
+      error: null,
+    });
+
+    const { result, rerender } = renderHook(() => useShoppingLists());
+
+    await waitFor(() => expect(result.current.activeListId).toBe(1));
+
+    // Rerender with empty lists
+    mockedUseUserData.mockReturnValue({
+      shoppingLists: [],
+      setShoppingLists: mockSetShoppingLists,
+      watchedItems: [],
+      setWatchedItems: vi.fn(),
+      isLoading: false,
+      error: null,
+    });
+    rerender();
+
+    // The effect should update the activeListId to null
+    await waitFor(() => expect(result.current.activeListId).toBeNull());
+  });
+
+  it('should expose loading states for API operations', () => {
+    // Mock useApi to return loading: true for each specific operation in sequence
+    mockedUseApi
+      .mockReturnValueOnce({ ...defaultApiMocks[0], loading: true }) // create
+      .mockReturnValueOnce({ ...defaultApiMocks[1], loading: true }) // delete
+      .mockReturnValueOnce({ ...defaultApiMocks[2], loading: true }) // add item
+      .mockReturnValueOnce({ ...defaultApiMocks[3], loading: true }) // update item
+      .mockReturnValueOnce({ ...defaultApiMocks[4], loading: true }); // remove item
+
+    const { result } = renderHook(() => useShoppingLists());
+
+    expect(result.current.isCreatingList).toBe(true);
+    expect(result.current.isDeletingList).toBe(true);
+    expect(result.current.isAddingItem).toBe(true);
+    expect(result.current.isUpdatingItem).toBe(true);
+    expect(result.current.isRemovingItem).toBe(true);
+  });
+
   describe('createList', () => {
     it('should call the API and update state on successful creation', async () => {
       const newList: ShoppingList = { shopping_list_id: 99, name: 'New List', user_id: 'user-123', created_at: '', items: [] };
@@ -190,33 +224,18 @@ describe('useShoppingLists Hook', () => {
       expect(currentLists).toEqual([newList]);
     });
 
-    it('should set an error message if API call fails', async () => {
-      mockCreateListApi.mockRejectedValue(new Error('API Failed'));
-
-      const { result } = renderHook(() => useShoppingLists());
-
-      await act(async () => {
-        await result.current.createList('New List');
-      });
-
-      await waitFor(() => expect(result.current.error).toBe('API Failed'));
-    });
   });
 
   describe('deleteList', () => {
-    it('should call the API and update state on successful deletion', async () => {
-      const mockLists: ShoppingList[] = [
+    const mockLists: ShoppingList[] = [
         { shopping_list_id: 1, name: 'Groceries', user_id: 'user-123', created_at: '', items: [] },
         { shopping_list_id: 2, name: 'Hardware Store', user_id: 'user-123', created_at: '', items: [] },
-      ];
-      mockedUseUserData.mockReturnValue({
-        shoppingLists: mockLists,
-        setShoppingLists: mockSetShoppingLists,
-        watchedItems: [],
-        setWatchedItems: vi.fn(),
-        isLoading: false,
-        error: null,
-      });
+    ];
+    beforeEach(() => {
+        mockedUseUserData.mockReturnValue({ shoppingLists: mockLists, setShoppingLists: mockSetShoppingLists, watchedItems: [], setWatchedItems: vi.fn(), isLoading: false, error: null });
+    });
+
+    it('should call the API and update state on successful deletion', async () => {
       mockDeleteListApi.mockResolvedValue(null); // Successful delete returns null
 
       const { result } = renderHook(() => useShoppingLists());
@@ -257,27 +276,50 @@ describe('useShoppingLists Hook', () => {
       await waitFor(() => expect(result.current.activeListId).toBe(2));
     });
 
-    it('should set an error message if API call fails', async () => {
-      mockDeleteListApi.mockRejectedValue(new Error('Deletion failed'));
+    it('should not change activeListId if a non-active list is deleted', async () => {
+      mockDeleteListApi.mockResolvedValue(null);
+      const { result } = renderHook(() => useShoppingLists());
+      await waitFor(() => expect(result.current.activeListId).toBe(1)); // Initial active is 1
+
+      await act(async () => {
+        await result.current.deleteList(2); // Delete list 2
+      });
+
+      await waitFor(() => expect(mockDeleteListApi).toHaveBeenCalledWith(2));
+      expect(mockSetShoppingLists).toHaveBeenCalledWith([mockLists[0]]); // Only list 1 remains
+      expect(result.current.activeListId).toBe(1); // Active list ID should not change
+    });
+
+    it('should set activeListId to null when the last list is deleted', async () => {
+      const singleList: ShoppingList[] = [{ shopping_list_id: 1, name: 'Groceries', user_id: 'user-123', created_at: '', items: [] }];
+      mockedUseUserData.mockReturnValue({ shoppingLists: singleList, setShoppingLists: mockSetShoppingLists, watchedItems: [], setWatchedItems: vi.fn(), isLoading: false, error: null });
+      mockDeleteListApi.mockResolvedValue(null);
 
       const { result } = renderHook(() => useShoppingLists());
-      await act(async () => { await result.current.deleteList(1); });
-      await waitFor(() => expect(result.current.error).toBe('Deletion failed'));
+      await waitFor(() => expect(result.current.activeListId).toBe(1));
+
+      await act(async () => {
+        await result.current.deleteList(1);
+      });
+
+      // The hook's internal logic will set the active list to null
+      // We also need to check that the global state setter was called to empty the list
+      await waitFor(() => expect(mockSetShoppingLists).toHaveBeenCalledWith([]));
+
+      // After the list is empty, the effect will run and set activeListId to null
+      await waitFor(() => expect(result.current.activeListId).toBeNull());
     });
+
   });
 
   describe('addItemToList', () => {
+    const mockLists: ShoppingList[] = [{ shopping_list_id: 1, name: 'Groceries', user_id: 'user-123', created_at: '', items: [] }];
+    beforeEach(() => {
+        mockedUseUserData.mockReturnValue({ shoppingLists: mockLists, setShoppingLists: mockSetShoppingLists, watchedItems: [], setWatchedItems: vi.fn(), isLoading: false, error: null });
+    });
+
     it('should call API and add item to the correct list', async () => {
-        const mockLists: ShoppingList[] = [{ shopping_list_id: 1, name: 'Groceries', user_id: 'user-123', created_at: '', items: [] }];
         const newItem: ShoppingListItem = { shopping_list_item_id: 101, shopping_list_id: 1, custom_item_name: 'Milk', is_purchased: false, quantity: 1, added_at: new Date().toISOString() };
-        mockedUseUserData.mockReturnValue({
-          shoppingLists: mockLists,
-          setShoppingLists: mockSetShoppingLists,
-          watchedItems: [],
-          setWatchedItems: vi.fn(),
-          isLoading: false,
-          error: null,
-        });
         mockAddItemApi.mockResolvedValue(newItem);
 
         const { result } = renderHook(() => useShoppingLists());
@@ -293,27 +335,41 @@ describe('useShoppingLists Hook', () => {
         expect(newState[0].items[0]).toEqual(newItem);
     });
 
-    it('should set an error message if API call fails', async () => {
-      mockAddItemApi.mockRejectedValue(new Error('Failed to add item'));
+    it('should not add a duplicate item (by master_item_id) to a list', async () => {
+      const existingItem: ShoppingListItem = { shopping_list_item_id: 100, shopping_list_id: 1, master_item_id: 5, custom_item_name: 'Milk', is_purchased: false, quantity: 1, added_at: '' };
+      const listWithItem: ShoppingList[] = [{ shopping_list_id: 1, name: 'Groceries', user_id: 'user-123', created_at: '', items: [existingItem] }];
+      // This is what the API would return for adding master_item_id 5 again. It has a new shopping_list_item_id.
+      const newItemFromApi: ShoppingListItem = { shopping_list_item_id: 101, shopping_list_id: 1, master_item_id: 5, custom_item_name: 'Milk', is_purchased: false, quantity: 1, added_at: '' };
+
+      mockedUseUserData.mockReturnValue({ shoppingLists: listWithItem, setShoppingLists: mockSetShoppingLists, watchedItems: [], setWatchedItems: vi.fn(), isLoading: false, error: null });
+      mockAddItemApi.mockResolvedValue(newItemFromApi);
+
       const { result } = renderHook(() => useShoppingLists());
-      await act(async () => { await result.current.addItemToList(1, { customItemName: 'Milk' }); });
-      await waitFor(() => expect(result.current.error).toBe('Failed to add item'));
+
+      await act(async () => {
+        await result.current.addItemToList(1, { masterItemId: 5 });
+      });
+
+      expect(mockAddItemApi).toHaveBeenCalledWith(1, { masterItemId: 5 });
+      // setShoppingLists should have been called, but the updater function should not have added the new item.
+      expect(mockSetShoppingLists).toHaveBeenCalled();
+      const updater = (mockSetShoppingLists as Mock).mock.calls[0][0];
+      const newState = updater(listWithItem);
+      expect(newState[0].items).toHaveLength(1); // Length should remain 1
+      expect(newState[0].items[0].shopping_list_item_id).toBe(100); // It should be the original item
     });
+
   });
 
   describe('updateItemInList', () => {
+    const initialItem: ShoppingListItem = { shopping_list_item_id: 101, shopping_list_id: 1, custom_item_name: 'Milk', is_purchased: false, quantity: 1, added_at: new Date().toISOString() };
+    const mockLists: ShoppingList[] = [{ shopping_list_id: 1, name: 'Groceries', user_id: 'user-123', created_at: '', items: [initialItem] }];
+    beforeEach(() => {
+        mockedUseUserData.mockReturnValue({ shoppingLists: mockLists, setShoppingLists: mockSetShoppingLists, watchedItems: [], setWatchedItems: vi.fn(), isLoading: false, error: null });
+    });
+
     it('should call API and update the correct item', async () => {
-        const initialItem: ShoppingListItem = { shopping_list_item_id: 101, shopping_list_id: 1, custom_item_name: 'Milk', is_purchased: false, quantity: 1, added_at: new Date().toISOString() };
-        const mockLists: ShoppingList[] = [{ shopping_list_id: 1, name: 'Groceries', user_id: 'user-123', created_at: '', items: [initialItem] }];
         const updatedItem: ShoppingListItem = { ...initialItem, is_purchased: true };
-        mockedUseUserData.mockReturnValue({
-          shoppingLists: mockLists,
-          setShoppingLists: mockSetShoppingLists,
-          watchedItems: [],
-          setWatchedItems: vi.fn(),
-          isLoading: false,
-          error: null,
-        });
         mockUpdateItemApi.mockResolvedValue(updatedItem);
 
         const { result } = renderHook(() => useShoppingLists());
@@ -329,13 +385,17 @@ describe('useShoppingLists Hook', () => {
         expect(newState[0].items[0].is_purchased).toBe(true);
     });
 
-    it('should set an error message if API call fails', async () => {
-      mockUpdateItemApi.mockRejectedValue(new Error('Update failed'));
+    it('should not call update API if no list is active', async () => {
       const { result } = renderHook(() => useShoppingLists());
-      act(() => { result.current.setActiveListId(1); });
-      await act(async () => { await result.current.updateItemInList(101, { is_purchased: true }); });
-      await waitFor(() => expect(result.current.error).toBe('Update failed'));
+      act(() => { result.current.setActiveListId(null); }); // Ensure no active list
+
+      await act(async () => {
+        await result.current.updateItemInList(101, { is_purchased: true });
+      });
+
+      expect(mockUpdateItemApi).not.toHaveBeenCalled();
     });
+
   });
 
   describe('removeItemFromList', () => {
@@ -368,13 +428,71 @@ describe('useShoppingLists Hook', () => {
       expect(newState[0].items).toHaveLength(0);
     });
 
-    it('should set an error message if API call fails', async () => {
-      mockRemoveItemApi.mockRejectedValue(new Error('Removal failed'));
+    it('should not call remove API if no list is active', async () => {
       const { result } = renderHook(() => useShoppingLists());
-      act(() => { result.current.setActiveListId(1); });
+      act(() => { result.current.setActiveListId(null); }); // Ensure no active list
 
-      await act(async () => { await result.current.removeItemFromList(101); });
-      await waitFor(() => expect(result.current.error).toBe('Removal failed'));
+      await act(async () => {
+        await result.current.removeItemFromList(101);
+      });
+
+      expect(mockRemoveItemApi).not.toHaveBeenCalled();
+    });
+
+  });
+
+  describe('API Error Handling', () => {
+    test.each([
+      {
+        name: 'createList',
+        action: (hook: any) => hook.createList('New List'),
+        apiMock: mockCreateListApi,
+        mockIndex: 0,
+        errorMessage: 'API Failed',
+      },
+      {
+        name: 'deleteList',
+        action: (hook: any) => hook.deleteList(1),
+        apiMock: mockDeleteListApi,
+        mockIndex: 1,
+        errorMessage: 'Deletion failed',
+      },
+      {
+        name: 'addItemToList',
+        action: (hook: any) => hook.addItemToList(1, { customItemName: 'Milk' }),
+        apiMock: mockAddItemApi,
+        mockIndex: 2,
+        errorMessage: 'Failed to add item',
+      },
+      {
+        name: 'updateItemInList',
+        action: (hook: any) => {
+          act(() => { hook.setActiveListId(1); });
+          return hook.updateItemInList(101, { is_purchased: true });
+        },
+        apiMock: mockUpdateItemApi,
+        mockIndex: 3,
+        errorMessage: 'Update failed',
+      },
+      {
+        name: 'removeItemFromList',
+        action: (hook: any) => {
+          act(() => { hook.setActiveListId(1); });
+          return hook.removeItemFromList(101);
+        },
+        apiMock: mockRemoveItemApi,
+        mockIndex: 4,
+        errorMessage: 'Removal failed',
+      },
+    ])('should set an error for $name if the API call fails', async ({ action, apiMock, mockIndex, errorMessage }) => {
+      const apiMocksWithError = [...defaultApiMocks];
+      apiMocksWithError[mockIndex] = { ...apiMocksWithError[mockIndex], error: new Error(errorMessage) };
+      setupApiMocks(apiMocksWithError);
+      apiMock.mockRejectedValue(new Error(errorMessage));
+
+      const { result } = renderHook(() => useShoppingLists());
+      await act(async () => { await action(result.current); });
+      await waitFor(() => expect(result.current.error).toBe(errorMessage));
     });
   });