torbo/flyer-crawler.projectium.com
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Refactor MainLayout to use new hooks for flyers and master items; consolidate error handling
Some checks failed
Deploy to Test Environment / deploy-to-test (push) Has been cancelled
Details

Browse Source 
Update error handling tests to ensure proper status assignment for errors
This commit is contained in:
Torben Sorensen
2025-12-14 11:05:04 -08:00
parent 571ca59e82
commit 9757f9dd9f
40 changed files with 348 additions and 207 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
src/App.test.tsx
View File

@@ -101,7 +101,7 @@ describe('App Component', () => {
vi.clearAllMocks();
// Default auth state: loading or guest
// Mock the login function to simulate a successful login.
const mockLogin = vi.fn(async (user: User, token: string) => {
const mockLogin = vi.fn(async (_user: User, _token: string) => {
await act(async () => {
// Simulate fetching profile after login
const profileResponse = await mockedApiClient.getAuthenticatedUserProfile();

6
src/components/MapView.test.tsx
View File

@@ -10,7 +10,11 @@ interface MockConfig {
google: {
mapsEmbedApiKey?: string;
};
[key: string]: any; // Allow other properties
app: {
version: string;
commitMessage: string;
commitUrl: string;
}
};
}
const setupConfigMock = (apiKey: string | undefined) => {

4
src/features/flyer/AnalysisPanel.test.tsx
View File

@@ -171,9 +171,9 @@ describe('AnalysisPanel', () => {
it('should display a specific error for geolocation permission denial', async () => {
// Mock getCurrentPosition to reject the promise, which is how the component's logic handles errors.
(navigator.geolocation.getCurrentPosition as Mocked<any>).mockImplementation(
(navigator.geolocation.getCurrentPosition as Mock).mockImplementation(
(
success: (position: GeolocationPosition) => void,
_success: (position: GeolocationPosition) => void,
error: (error: GeolocationPositionError) => void
) => {
// The component wraps this in a Promise, so we call the error callback which causes the promise to reject.

6
src/features/flyer/AnalysisPanel.tsx
View File

@@ -24,12 +24,6 @@ interface AnalysisPanelProps {
*/
type AnalysisTabType = Exclude<AnalysisType, AnalysisType.GENERATE_IMAGE>;
interface Source {
uri: string;
title: string;
}
interface TabButtonProps {
label: string;
icon: React.ReactNode;

2
src/features/flyer/BulkImporter.test.tsx
View File

@@ -1,6 +1,6 @@
// src/features/flyer/BulkImporter.test.tsx
import React from 'react';
import { render, screen, fireEvent, waitFor, act, within } from '@testing-library/react';
import { render, screen, fireEvent, waitFor, act } from '@testing-library/react';
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import { BulkImporter } from './BulkImporter';

2
src/features/flyer/ExtractedDataTable.tsx
View File

@@ -1,6 +1,6 @@
// src/features/flyer/ExtractedDataTable.tsx
import React, { useMemo, useState, useCallback } from 'react';
import type { FlyerItem, MasterGroceryItem, ShoppingList, ShoppingListItem, User } from '../../types'; // Import ShoppingListItem
import type { FlyerItem, ShoppingListItem } from '../../types'; // Import ShoppingListItem
import { formatUnitPrice } from '../../utils/unitConverter';
import { PlusCircleIcon } from '../../components/icons/PlusCircleIcon';
import { useAuth } from '../../hooks/useAuth';

4
src/hooks/useActiveDeals.tsx
View File

@@ -1,9 +1,9 @@
// src/hooks/useActiveDeals.tsx
import { useState, useEffect, useMemo } from 'react';
import { useEffect, useMemo } from 'react';
import { useFlyers } from './useFlyers';
import { useUserData } from './useUserData';
import { useApi } from './useApi';
import type { Flyer, FlyerItem, MasterGroceryItem, DealItem } from '../types';
import type { FlyerItem } from '../types';
import * as apiClient from '../services/apiClient';
import { logger } from '../services/logger.client';

2
src/hooks/useAiAnalysis.test.ts
View File

@@ -118,7 +118,7 @@ describe('useAiAnalysis Hook', () => {
const { result, rerender } = renderHook(() => useAiAnalysis(defaultParams));
mockedUseApi.mockReset()
.mockReturnValue(mockGetQuickInsights as any)
.mockReturnValue(mockGetQuickInsights)
.mockReturnValue(mockGetDeepDive)
.mockReturnValueOnce({ ...mockSearchWeb, data: mockResponse });

14
src/hooks/useApi.test.ts
View File

@@ -1,5 +1,5 @@
// src/hooks/useApi.test.ts
import { renderHook, act, waitFor } from '@testing-library/react';
import { renderHook, act } from '@testing-library/react';
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { useApi } from './useApi';
@@ -32,9 +32,9 @@ describe('useApi Hook', () => {
const mockData = { id: 1, name: 'Test' };
mockApiFunction.mockResolvedValue(new Response(JSON.stringify(mockData)));
const { result } = renderHook(() => useApi(mockApiFunction));
const { result } = renderHook(() => useApi<typeof mockData, [string]>(mockApiFunction));
let promise: any;
let promise: Promise<typeof mockData | null>;
act(() => {
promise = result.current.execute('test-arg');
});
@@ -83,11 +83,11 @@ describe('useApi Hook', () => {
describe('isRefetching state', () => {
it('should set isRefetching to true on second call, but not first', async () => {
mockApiFunction.mockResolvedValue(new Response(JSON.stringify({ data: 'first call' })));
const { result } = renderHook(() => useApi(mockApiFunction));
// Provide the generic type argument to useApi
const { result } = renderHook(() => useApi<{ data: string }, []>(mockApiFunction));
// --- First call ---
let firstCallPromise: any;
let firstCallPromise: Promise<{ data: string; } | null>;
act(() => {
firstCallPromise = result.current.execute();
});
@@ -105,7 +105,7 @@ describe('useApi Hook', () => {
// --- Second call ---
mockApiFunction.mockResolvedValue(new Response(JSON.stringify({ data: 'second call' })));
let secondCallPromise: any;
let secondCallPromise: Promise<{ data: string; } | null>;
act(() => {
secondCallPromise = result.current.execute();
});

2
src/hooks/useFlyerItems.test.ts
View File

@@ -1,5 +1,5 @@
// src/hooks/useFlyerItems.test.ts
import { renderHook, waitFor } from '@testing-library/react';
import { renderHook } from '@testing-library/react';
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { useFlyerItems } from './useFlyerItems';
import { useApiOnMount } from './useApiOnMount';

113
src/hooks/useShoppingLists.test.tsx
View File

@@ -36,22 +36,30 @@ describe('useShoppingLists Hook', () => {
// Provide a default implementation for the mocked hooks
mockedUseApi
.mockReturnValueOnce({ execute: mockCreateListApi, error: null, loading: false } as any)
.mockReturnValueOnce({ execute: mockDeleteListApi, error: null, loading: false } as any)
.mockReturnValueOnce({ execute: mockAddItemApi, error: null, loading: false } as any)
.mockReturnValueOnce({ execute: mockUpdateItemApi, error: null, loading: false } as any)
.mockReturnValueOnce({ execute: mockRemoveItemApi, error: null, loading: false } as any);
.mockReturnValueOnce({ execute: mockCreateListApi, error: null, loading: false, isRefetching: false, data: null })
.mockReturnValueOnce({ execute: mockDeleteListApi, error: null, loading: false, isRefetching: false, data: null })
.mockReturnValueOnce({ execute: mockAddItemApi, error: null, loading: false, isRefetching: false, data: null })
.mockReturnValueOnce({ execute: mockUpdateItemApi, error: null, loading: false, isRefetching: false, data: null })
.mockReturnValueOnce({ execute: mockRemoveItemApi, error: null, loading: false, isRefetching: false, data: null });
mockedUseAuth.mockReturnValue({
user: mockUser,
// other useAuth properties are not needed for this test
} as any);
profile: null,
authStatus: 'AUTHENTICATED',
isLoading: false,
login: vi.fn(),
logout: vi.fn(),
updateProfile: vi.fn(),
});
mockedUseUserData.mockReturnValue({
shoppingLists: [],
setShoppingLists: mockSetShoppingLists,
// other useData properties
} as any);
watchedItems: [],
setWatchedItems: vi.fn(),
isLoading: false,
error: null,
});
});
it('should initialize with no active list when there are no lists', () => {
@@ -70,7 +78,11 @@ describe('useShoppingLists Hook', () => {
mockedUseUserData.mockReturnValue({
shoppingLists: mockLists,
setShoppingLists: mockSetShoppingLists,
} as any);
watchedItems: [],
setWatchedItems: vi.fn(),
isLoading: false,
error: null,
});
const { result } = renderHook(() => useShoppingLists());
@@ -78,11 +90,26 @@ describe('useShoppingLists Hook', () => {
});
it('should not set an active list if the user is not authenticated', () => {
mockedUseAuth.mockReturnValue({ user: null } as any);
mockedUseAuth.mockReturnValue({
user: null,
profile: null,
authStatus: 'SIGNED_OUT',
isLoading: false,
login: vi.fn(),
logout: vi.fn(),
updateProfile: vi.fn(),
});
const mockLists: ShoppingList[] = [
{ shopping_list_id: 1, name: 'Groceries', user_id: 'user-123', created_at: '', items: [] },
];
mockedUseUserData.mockReturnValue({ shoppingLists: mockLists, setShoppingLists: mockSetShoppingLists } as any);
mockedUseUserData.mockReturnValue({
shoppingLists: mockLists,
setShoppingLists: mockSetShoppingLists,
watchedItems: [],
setWatchedItems: vi.fn(),
isLoading: false,
error: null,
});
const { result } = renderHook(() => useShoppingLists());
@@ -104,11 +131,15 @@ describe('useShoppingLists Hook', () => {
mockedUseUserData.mockImplementation(() => ({
shoppingLists: currentLists,
setShoppingLists: mockSetShoppingLists,
} as any));
watchedItems: [],
setWatchedItems: vi.fn(),
isLoading: false,
error: null,
}));
mockCreateListApi.mockResolvedValue(newList);
const { result, rerender } = renderHook(() => useShoppingLists());
const { result } = renderHook(() => useShoppingLists());
// `act` ensures that all state updates from the hook are processed before assertions are made
await act(async () => {
@@ -120,7 +151,13 @@ describe('useShoppingLists Hook', () => {
});
it('should set an error message if API call fails', async () => {
mockedUseApi.mockReturnValueOnce({ execute: vi.fn(), error: new Error('API Failed'), loading: false } as any);
mockedUseApi.mockReturnValueOnce({
execute: vi.fn(),
error: new Error('API Failed'),
loading: false,
isRefetching: false,
data: null
});
const { result } = renderHook(() => useShoppingLists());
@@ -138,7 +175,14 @@ describe('useShoppingLists Hook', () => {
{ 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 } as any);
mockedUseUserData.mockReturnValue({
shoppingLists: mockLists,
setShoppingLists: mockSetShoppingLists,
watchedItems: [],
setWatchedItems: vi.fn(),
isLoading: false,
error: null,
});
mockDeleteListApi.mockResolvedValue(null); // Successful delete returns null
const { result } = renderHook(() => useShoppingLists());
@@ -157,7 +201,14 @@ describe('useShoppingLists Hook', () => {
{ 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 } as any);
mockedUseUserData.mockReturnValue({
shoppingLists: mockLists,
setShoppingLists: mockSetShoppingLists,
watchedItems: [],
setWatchedItems: vi.fn(),
isLoading: false,
error: null,
});
mockDeleteListApi.mockResolvedValue(null);
// Render the hook and wait for the initial effect to set activeListId
@@ -179,7 +230,14 @@ describe('useShoppingLists Hook', () => {
{ 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 } as any);
mockedUseUserData.mockReturnValue({
shoppingLists: mockLists,
setShoppingLists: mockSetShoppingLists,
watchedItems: [],
setWatchedItems: vi.fn(),
isLoading: false,
error: null,
});
mockAddItemApi.mockResolvedValue(newItem);
const { result } = renderHook(() => useShoppingLists());
@@ -203,7 +261,14 @@ describe('useShoppingLists Hook', () => {
{ 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 } as any);
mockedUseUserData.mockReturnValue({
shoppingLists: mockLists,
setShoppingLists: mockSetShoppingLists,
watchedItems: [],
setWatchedItems: vi.fn(),
isLoading: false,
error: null,
});
mockUpdateItemApi.mockResolvedValue(updatedItem);
const { result } = renderHook(() => useShoppingLists());
@@ -221,7 +286,15 @@ describe('useShoppingLists Hook', () => {
});
it('should not perform actions if user is not authenticated', async () => {
mockedUseAuth.mockReturnValue({ user: null } as any);
mockedUseAuth.mockReturnValue({
user: null,
profile: null,
authStatus: 'SIGNED_OUT',
isLoading: false,
login: vi.fn(),
logout: vi.fn(),
updateProfile: vi.fn(),
});
const { result } = renderHook(() => useShoppingLists());

58
src/hooks/useUserData.test.tsx
View File

@@ -56,7 +56,15 @@ describe('useUserData Hook and UserDataProvider', () => {
it('should return initial empty state when user is not authenticated', () => {
// Arrange: Simulate a logged-out user.
mockedUseAuth.mockReturnValue({ user: null } as any);
mockedUseAuth.mockReturnValue({
user: null,
profile: null,
authStatus: 'SIGNED_OUT',
isLoading: false,
login: vi.fn(),
logout: vi.fn(),
updateProfile: vi.fn(),
});
// Arrange: Mock the return value of the inner hooks.
mockedUseApiOnMount.mockReturnValue({ data: null, loading: false, error: null, isRefetching: false });
@@ -74,7 +82,15 @@ describe('useUserData Hook and UserDataProvider', () => {
it('should return loading state when user is authenticated and data is fetching', () => {
// Arrange: Simulate a logged-in user.
mockedUseAuth.mockReturnValue({ user: mockUser } as any);
mockedUseAuth.mockReturnValue({
user: mockUser.user,
profile: mockUser,
authStatus: 'AUTHENTICATED',
isLoading: false,
login: vi.fn(),
logout: vi.fn(),
updateProfile: vi.fn(),
});
// Arrange: Mock one of the inner hooks to be in a loading state.
mockedUseApiOnMount
.mockReturnValueOnce({ data: null, loading: true, error: null, isRefetching: false }) // watched items
@@ -89,7 +105,15 @@ describe('useUserData Hook and UserDataProvider', () => {
it('should return data on successful fetch when user is authenticated', async () => {
// Arrange: Simulate a logged-in user.
mockedUseAuth.mockReturnValue({ user: mockUser } as any);
mockedUseAuth.mockReturnValue({
user: mockUser.user,
profile: mockUser,
authStatus: 'AUTHENTICATED',
isLoading: false,
login: vi.fn(),
logout: vi.fn(),
updateProfile: vi.fn(),
});
// Arrange: Mock successful data fetches for both inner hooks.
mockedUseApiOnMount
.mockReturnValueOnce({ data: mockWatchedItems, loading: false, error: null, isRefetching: false })
@@ -109,7 +133,15 @@ describe('useUserData Hook and UserDataProvider', () => {
it('should return an error state if one of the fetches fails', async () => {
// Arrange: Simulate a logged-in user.
mockedUseAuth.mockReturnValue({ user: mockUser } as any);
mockedUseAuth.mockReturnValue({
user: mockUser.user,
profile: mockUser,
authStatus: 'AUTHENTICATED',
isLoading: false,
login: vi.fn(),
logout: vi.fn(),
updateProfile: vi.fn(),
});
const mockError = new Error('Failed to fetch watched items');
// Arrange: Mock one fetch failing and the other succeeding.
mockedUseApiOnMount
@@ -132,7 +164,15 @@ describe('useUserData Hook and UserDataProvider', () => {
it('should clear data when the user logs out', async () => {
// Arrange: Start with a logged-in user and data.
mockedUseAuth.mockReturnValue({ user: mockUser } as any);
mockedUseAuth.mockReturnValue({
user: mockUser.user,
profile: mockUser,
authStatus: 'AUTHENTICATED',
isLoading: false,
login: vi.fn(),
logout: vi.fn(),
updateProfile: vi.fn(),
});
mockedUseApiOnMount
.mockReturnValueOnce({ data: mockWatchedItems, loading: false, error: null, isRefetching: false })
.mockReturnValueOnce({ data: mockShoppingLists, loading: false, error: null, isRefetching: false });
@@ -140,7 +180,13 @@ describe('useUserData Hook and UserDataProvider', () => {
await waitFor(() => expect(result.current.watchedItems).not.toEqual([]));
// Act: Simulate logout by re-rendering with a null user.
mockedUseAuth.mockReturnValue({ user: null } as any);
mockedUseAuth.mockReturnValue({
user: null,
profile: null,
authStatus: 'SIGNED_OUT',
isLoading: false,
login: vi.fn(), logout: vi.fn(), updateProfile: vi.fn()
});
rerender();
// Assert: The data should now be cleared.

37
src/hooks/useWatchedItems.test.tsx
View File

@@ -35,19 +35,29 @@ describe('useWatchedItems Hook', () => {
// Provide a default implementation for useApi
mockedUseApi
.mockReturnValueOnce({ execute: mockAddWatchedItemApi, error: null } as any)
.mockReturnValueOnce({ execute: mockRemoveWatchedItemApi, error: null } as any);
.mockReturnValueOnce({ execute: mockAddWatchedItemApi, error: null, data: null, loading: false, isRefetching: false })
.mockReturnValueOnce({ execute: mockRemoveWatchedItemApi, error: null, data: null, loading: false, isRefetching: false });
// Provide a default implementation for the mocked hooks
mockedUseAuth.mockReturnValue({
user: mockUser,
} as any);
profile: null,
authStatus: 'AUTHENTICATED',
isLoading: false,
login: vi.fn(),
logout: vi.fn(),
updateProfile: vi.fn(),
});
mockedUseUserData.mockReturnValue({
watchedItems: mockInitialItems,
setWatchedItems: mockSetWatchedItems,
} as any);
shoppingLists: [],
setShoppingLists: vi.fn(),
isLoading: false,
error: null,
});
});
it('should initialize with the watched items from useData', () => {
@@ -85,7 +95,10 @@ describe('useWatchedItems Hook', () => {
mockedUseApi.mockReturnValueOnce({
execute: mockAddWatchedItemApi,
error: new Error('API Error'),
} as any);
data: null,
loading: false,
isRefetching: false,
});
const { result } = renderHook(() => useWatchedItems());
@@ -121,8 +134,8 @@ describe('useWatchedItems Hook', () => {
it('should set an error message if the API call fails', async () => {
// Re-mock useApi for this specific error test case
mockedUseApi.mockReturnValueOnce({ execute: vi.fn(), error: null } as any) // for add
.mockReturnValueOnce({ execute: vi.fn(), error: new Error('Deletion Failed') } as any); // for remove
mockedUseApi.mockReturnValueOnce({ execute: vi.fn(), error: null, data: null, loading: false, isRefetching: false }) // for add
.mockReturnValueOnce({ execute: vi.fn(), error: new Error('Deletion Failed'), data: null, loading: false, isRefetching: false }); // for remove
const { result } = renderHook(() => useWatchedItems());
@@ -136,7 +149,15 @@ describe('useWatchedItems Hook', () => {
});
it('should not perform actions if the user is not authenticated', async () => {
mockedUseAuth.mockReturnValue({ user: null } as any);
mockedUseAuth.mockReturnValue({
user: null,
profile: null,
authStatus: 'SIGNED_OUT',
isLoading: false,
login: vi.fn(),
logout: vi.fn(),
updateProfile: vi.fn(),
});
const { result } = renderHook(() => useWatchedItems());

42
src/layouts/MainLayout.test.tsx
View File

@@ -7,7 +7,8 @@ import { MainLayout } from './MainLayout';
// Mock all custom hooks used by the layout
import { useAuth } from '../hooks/useAuth';
import { useData } from '../hooks/useData';
import { useFlyers } from '../hooks/useFlyers';
import { useMasterItems } from '../hooks/useMasterItems';
import { useShoppingLists } from '../hooks/useShoppingLists';
import { useWatchedItems } from '../hooks/useWatchedItems';
import { useActiveDeals } from '../hooks/useActiveDeals';
@@ -28,13 +29,15 @@ vi.mock('../components/ErrorDisplay', () => ({ ErrorDisplay: ({ message }: { mes
// Mock the hooks
vi.mock('../hooks/useAuth');
vi.mock('../hooks/useData');
vi.mock('../hooks/useFlyers');
vi.mock('../hooks/useMasterItems');
vi.mock('../hooks/useShoppingLists');
vi.mock('../hooks/useWatchedItems');
vi.mock('../hooks/useActiveDeals');
const mockedUseAuth = vi.mocked(useAuth);
const mockedUseData = vi.mocked(useData);
const mockedUseFlyers = vi.mocked(useFlyers);
const mockedUseMasterItems = vi.mocked(useMasterItems);
const mockedUseShoppingLists = vi.mocked(useShoppingLists);
const mockedUseWatchedItems = vi.mocked(useWatchedItems);
const mockedUseActiveDeals = vi.mocked(useActiveDeals);
@@ -70,8 +73,8 @@ describe('MainLayout Component', () => {
login: vi.fn(),
logout: vi.fn(),
updateProfile: vi.fn(),
};
const defaultUseDataReturn = {
} as const;
const defaultUseFlyersReturn = {
flyers: [{
flyer_id: 1,
file_name: 'flyer.jpg',
@@ -79,12 +82,15 @@ describe('MainLayout Component', () => {
image_url: 'http://example.com/flyer.jpg',
item_count: 10,
}],
masterItems: [],
watchedItems: [],
shoppingLists: [],
setWatchedItems: vi.fn(),
setShoppingLists: vi.fn(),
isLoadingFlyers: false,
flyersError: null,
refetchFlyers: vi.fn(),
fetchNextFlyersPage: vi.fn(),
hasNextFlyersPage: false,
isRefetchingFlyers: false,
};
const defaultUseMasterItemsReturn = {
masterItems: [],
isLoading: false,
error: null,
};
@@ -97,6 +103,11 @@ describe('MainLayout Component', () => {
addItemToList: vi.fn(),
updateItemInList: vi.fn(),
removeItemFromList: vi.fn(),
isCreatingList: false,
isDeletingList: false,
isAddingItem: false,
isUpdatingItem: false,
isRemovingItem: false,
error: null,
};
const defaultUseWatchedItemsReturn = {
@@ -116,8 +127,9 @@ describe('MainLayout Component', () => {
vi.clearAllMocks();
// Provide default mock implementations for all hooks
mockedUseAuth.mockReturnValue(defaultUseAuthReturn);
mockedUseData.mockReturnValue(defaultUseDataReturn as any);
mockedUseAuth.mockReturnValue(defaultUseAuthReturn as any);
mockedUseFlyers.mockReturnValue(defaultUseFlyersReturn as any);
mockedUseMasterItems.mockReturnValue(defaultUseMasterItemsReturn);
mockedUseShoppingLists.mockReturnValue(defaultUseShoppingListsReturn as any);
mockedUseWatchedItems.mockReturnValue(defaultUseWatchedItemsReturn as any);
mockedUseActiveDeals.mockReturnValue(defaultUseActiveDealsReturn as any);
@@ -150,7 +162,7 @@ describe('MainLayout Component', () => {
});
it('does not show the AnonymousUserBanner if there are no flyers', () => {
mockedUseData.mockReturnValueOnce({ ...defaultUseDataReturn, flyers: [] });
mockedUseFlyers.mockReturnValueOnce({ ...defaultUseFlyersReturn, flyers: [] });
renderWithRouter(<MainLayout {...defaultProps} />);
expect(screen.queryByTestId('anonymous-banner')).not.toBeInTheDocument();
});
@@ -174,7 +186,7 @@ describe('MainLayout Component', () => {
describe('Error Handling', () => {
it('displays an error message if useData has an error', () => {
mockedUseData.mockReturnValueOnce({ ...defaultUseDataReturn, error: 'Data Fetch Failed' });
mockedUseFlyers.mockReturnValueOnce({ ...defaultUseFlyersReturn, flyersError: new Error('Data Fetch Failed') });
renderWithRouter(<MainLayout {...defaultProps} />);
expect(screen.getByTestId('error-display')).toHaveTextContent('Data Fetch Failed');
});
@@ -201,7 +213,7 @@ describe('MainLayout Component', () => {
describe('Event Handlers', () => {
it('calls setActiveListId when a list is shared via ActivityLog and the list exists', () => {
mockedUseShoppingLists.mockReturnValueOnce({
...defaultUseShoppingListsReturn,
...defaultUseShoppingListsReturn, // Spread the complete default object
shoppingLists: [{ shopping_list_id: 1, name: 'My List', items: [] } as any],
} as any);

14
src/layouts/MainLayout.tsx
View File

@@ -2,8 +2,9 @@
import React, { useCallback } from 'react';
import { Outlet } from 'react-router-dom';
import { useAuth } from '../hooks/useAuth';
import { useData } from '../hooks/useData';
import { useFlyers } from '../hooks/useFlyers';
import { useShoppingLists } from '../hooks/useShoppingLists';
import { useMasterItems } from '../hooks/useMasterItems';
import { useWatchedItems } from '../hooks/useWatchedItems';
import { useActiveDeals } from '../hooks/useActiveDeals';
@@ -26,7 +27,8 @@ interface MainLayoutProps {
export const MainLayout: React.FC<MainLayoutProps> = ({ onFlyerSelect, selectedFlyerId, onOpenProfile }) => {
const { user, authStatus, profile } = useAuth();
const { flyers, masterItems, refetchFlyers, error: dataError } = useData();
const { flyers, refetchFlyers, flyersError } = useFlyers();
const { masterItems, error: masterItemsError } = useMasterItems();
const {
shoppingLists, activeListId, setActiveListId,
createList, deleteList, addItemToList, updateItemInList, removeItemFromList,
@@ -36,7 +38,7 @@ export const MainLayout: React.FC<MainLayoutProps> = ({ onFlyerSelect, selectedF
watchedItems, addWatchedItem, removeWatchedItem,
error: watchedItemsError,
} = useWatchedItems();
const { activeDeals, totalActiveItems, isLoading: activeDealsLoading, error: activeDealsError } = useActiveDeals(flyers, watchedItems);
const { activeDeals, totalActiveItems, isLoading: activeDealsLoading, error: activeDealsError } = useActiveDeals();
const handleActivityLogClick: ActivityLogClickHandler = useCallback((log) => {
if (log.action === 'list_shared') {
@@ -60,7 +62,7 @@ export const MainLayout: React.FC<MainLayoutProps> = ({ onFlyerSelect, selectedF
}, [activeListId, addItemToList]);
// Consolidate error states into a single variable for cleaner display logic.
const combinedError = dataError || shoppingListError || watchedItemsError || activeDealsError;
const combinedError = flyersError?.message || masterItemsError || shoppingListError || watchedItemsError || activeDealsError;
return (
<main className="max-w-screen-2xl mx-auto py-4 px-2.5 sm:py-6 lg:py-8">
@@ -105,12 +107,10 @@ export const MainLayout: React.FC<MainLayoutProps> = ({ onFlyerSelect, selectedF
onAddItemToList={handleAddItemFromWatchedList}
/>
<PriceChart
deals={activeDeals}
isLoading={activeDealsLoading}
unitSystem={'imperial'} // This can be passed down or sourced from a context
user={user}
/>
<PriceHistoryChart watchedItems={watchedItems} />
<PriceHistoryChart />
<Leaderboard />
<ActivityLog user={user} onLogClick={handleActivityLogClick} />
</>

11
src/middleware/errorHandler.test.ts
View File

@@ -41,8 +41,9 @@ app.get('/generic-error', (req, res, next) => {
});
app.get('/http-error-404', (req, res, next) => {
const err = new Error('Resource not found') as any;
err.status = 404;
// Create an error object with a custom status property for testing.
const err = new Error('Resource not found') as Error & { status?: number };
err.status = 404; // This is a common pattern in Express for non-custom errors.
next(err);
});
@@ -59,15 +60,15 @@ app.get('/db-error-500', (req, res, next) => {
});
app.get('/unauthorized-error-no-status', (req, res, next) => {
const err = new Error('Invalid Token');
const err = new Error('Invalid Token') as Error & { status?: number };
err.name = 'UnauthorizedError'; // Simulate an error from a library like express-jwt
next(err);
});
app.get('/unauthorized-error-with-status', (req, res, next) => {
const err = new Error('Invalid Token');
const err = new Error('Invalid Token') as Error & { status?: number };
err.name = 'UnauthorizedError';
(err as any).status = 401; // Explicitly set status
err.status = 401; // Explicitly set status
next(err);
});

9
src/middleware/errorHandler.ts
View File

@@ -19,8 +19,13 @@ export const errorHandler = (err: HttpError, req: Request, res: Response, next:
// --- 1. Determine Final Status Code and Message ---
let statusCode = err.status ?? 500;
let message = err.message;
let errors: any[] | undefined;
const message = err.message;
// Define a more specific type for validation errors from Zod.
type ValidationIssue = {
path: (string | number)[];
message: string;
};
let errors: ValidationIssue[] | undefined;
let errorId: string | undefined;
// Refine the status code for known error types. Check for most specific types first.

4
src/middleware/validation.middleware.test.ts
View File

@@ -82,7 +82,9 @@ describe('validateRequest Middleware', () => {
const mockSchema = {
parseAsync: vi.fn().mockRejectedValue(unexpectedError),
};
const middleware = validateRequest(mockSchema as any);
// For this test, we only need the `parseAsync` method on our mock schema.
// We cast it to `unknown` first, then to the expected ZodObject type to satisfy TypeScript.
const middleware = validateRequest(mockSchema as unknown as z.ZodObject<any>);
// Act
await middleware(mockRequest as Request, mockResponse as Response, mockNext);

6
src/middleware/validation.middleware.ts
View File

@@ -1,7 +1,7 @@
// src/middleware/validation.middleware.ts
import { Request, Response, NextFunction } from 'express';
import { ParamsDictionary } from 'express-serve-static-core';
import { ZodObject, ZodError } from 'zod';
import { ZodObject, ZodError, z } from 'zod';
import { ValidationError } from '../services/db/errors.db';
/**
@@ -11,7 +11,7 @@ import { ValidationError } from '../services/db/errors.db';
* @param schema - The Zod schema to validate against.
* @returns An Express middleware function.
*/
export const validateRequest = (schema: ZodObject<any>) =>
export const validateRequest = (schema: ZodObject<z.ZodRawShape>) =>
async (req: Request, res: Response, next: NextFunction) => {
try {
// Parse and validate the request parts against the schema.
@@ -25,6 +25,8 @@ export const validateRequest = (schema: ZodObject<any>) =>
// On success, replace the request parts with the parsed (and coerced) data.
// This ensures downstream handlers get correctly typed data.
req.params = params as ParamsDictionary;
// The parsed query is of type `unknown` here. We cast it to `any` to assign it back,
// trusting that the Zod schema has correctly shaped the data for downstream use.
req.query = query as any;
req.body = body;

2
src/pages/HomePage.test.tsx
View File

@@ -5,7 +5,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest';
import { MemoryRouter, useOutletContext } from 'react-router-dom';
import { HomePage } from './HomePage';
import { createMockFlyer, createMockFlyerItem } from '../tests/utils/mockFactories';
import type { Flyer, FlyerItem, MasterGroceryItem, ShoppingList } from '../types';
import type { Flyer, FlyerItem } from '../types';
import type { FlyerDisplayProps } from '../features/flyer/FlyerDisplay'; // Keep this for FlyerDisplay mock
import type { ExtractedDataTableProps } from '../features/flyer/ExtractedDataTable'; // Import the props for ExtractedDataTable

2
src/pages/admin/components/AuthView.tsx
View File

@@ -3,7 +3,7 @@ import React, { useState } from 'react';
import type { User } from '../../../types';
import { useApi } from '../../../hooks/useApi';
import * as apiClient from '../../../services/apiClient';
import { notifySuccess, notifyError } from '../../../services/notificationService';
import { notifySuccess } from '../../../services/notificationService';
import { LoadingSpinner } from '../../../components/LoadingSpinner';
import { GoogleIcon } from '../../../components/icons/GoogleIcon';
import { GithubIcon } from '../../../components/icons/GithubIcon';

13
src/routes/admin.content.routes.test.ts
View File

@@ -1,16 +1,5 @@
// --- FIX REGISTRY ---
//
// 1) Updated `vi.mock` for `../services/db/index.db` to use `vi.hoisted` and `importOriginal`.
// This preserves named exports (like repository classes) from the original module,
// fixing 'undefined' errors when other modules tried to import them from the mock.
//
// 2) Added a `default` export to the `node:fs/promises` mock. This resolves import errors
// in modules that use the `import fs from 'node:fs/promises'` syntax.
//
// --- END FIX REGISTRY ---
// src/routes/admin.content.routes.test.ts
import { describe, it, expect, vi, beforeEach, type Mocked } from 'vitest';
import { describe, it, expect, vi, beforeEach } from 'vitest';
import supertest from 'supertest';
import express, { Request, Response, NextFunction } from 'express';
import adminRouter from './admin.routes';

6
src/routes/admin.jobs.routes.test.ts
View File

@@ -194,7 +194,7 @@ describe('Admin Job Trigger Routes (/api/admin/trigger)', () => {
getState: vi.fn().mockResolvedValue('failed'),
retry: vi.fn().mockResolvedValue(undefined),
};
vi.mocked(flyerQueue.getJob).mockResolvedValue(mockJob as any);
vi.mocked(flyerQueue.getJob).mockResolvedValue(mockJob as unknown as Job);
// Act
const response = await supertest(app).post(`/api/admin/jobs/${queueName}/${jobId}/retry`);
@@ -224,7 +224,7 @@ describe('Admin Job Trigger Routes (/api/admin/trigger)', () => {
getState: vi.fn().mockResolvedValue('completed'),
retry: vi.fn(),
};
vi.mocked(flyerQueue.getJob).mockResolvedValue(mockJob as any);
vi.mocked(flyerQueue.getJob).mockResolvedValue(mockJob as unknown as Job);
const response = await supertest(app).post(`/api/admin/jobs/${queueName}/${jobId}/retry`);
@@ -239,7 +239,7 @@ describe('Admin Job Trigger Routes (/api/admin/trigger)', () => {
getState: vi.fn().mockResolvedValue('failed'),
retry: vi.fn().mockRejectedValue(new Error('Cannot retry job')),
};
vi.mocked(flyerQueue.getJob).mockResolvedValue(mockJob as any);
vi.mocked(flyerQueue.getJob).mockResolvedValue(mockJob as unknown as Job);
const response = await supertest(app).post(`/api/admin/jobs/${queueName}/${jobId}/retry`);

2
src/routes/admin.routes.ts
View File

@@ -1,5 +1,5 @@
// src/routes/admin.routes.ts
import { Router, Request, Response, NextFunction } from 'express';
import { Router, NextFunction } from 'express';
import passport from './passport.routes';
import { isAdmin } from './passport.routes'; // Correctly imported
import multer from 'multer';// --- Zod Schemas for Admin Routes (as per ADR-003) ---

9
src/routes/admin.stats.routes.test.ts
View File

@@ -1,12 +1,5 @@
// --- FIX REGISTRY ---
//
// 2024-07-29: Updated `vi.mock` for `../services/db/index.db` to use `vi.hoisted` and `importOriginal`.
// This preserves named exports (like repository classes) from the original module,
// fixing 'undefined' errors when other modules tried to import them from the mock.
// --- END FIX REGISTRY ---
// src/routes/admin.stats.routes.test.ts
import { describe, it, expect, vi, beforeEach, type Mocked } from 'vitest';
import { describe, it, expect, vi, beforeEach } from 'vitest';
import supertest from 'supertest';
import express, { Request, Response, NextFunction } from 'express';
import adminRouter from './admin.routes';

3
src/routes/admin.system.routes.test.ts
View File

@@ -1,10 +1,9 @@
// src/routes/admin.system.routes.test.ts
import { describe, it, expect, vi, beforeEach, type Mocked } from 'vitest';
import { describe, it, expect, vi, beforeEach } from 'vitest';
import supertest from 'supertest';
import express, { Request, Response, NextFunction } from 'express';
import adminRouter from './admin.routes';
import { createMockUserProfile } from '../tests/utils/mockFactories';
import { UserProfile } from '../types';
import { errorHandler } from '../middleware/errorHandler';
// Mock dependencies

9
src/routes/admin.users.routes.test.ts
View File

@@ -1,12 +1,5 @@
// --- FIX REGISTRY ---
//
// 2024-07-29: Updated `vi.mock` for `../services/db/index.db` to use `vi.hoisted` and `importOriginal`.
// This preserves named exports (like repository classes) from the original module,
// fixing 'undefined' errors when other modules tried to import them from the mock.
// --- END FIX REGISTRY ---
// src/routes/admin.users.routes.test.ts
import { describe, it, expect, vi, beforeEach, type Mocked } from 'vitest';
import { describe, it, expect, vi, beforeEach } from 'vitest';
import supertest from 'supertest';
import express, { Request, Response, NextFunction } from 'express';
import adminRouter from './admin.routes';

5
src/routes/ai.routes.test.ts
View File

@@ -3,6 +3,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest';
import supertest from 'supertest';
import express, { type Request, type Response, type NextFunction } from 'express';
import path from 'node:path';
import type { Job } from 'bullmq';
import aiRouter from './ai.routes';
import { createMockUserProfile, createMockFlyer } from '../tests/utils/mockFactories';
import { errorHandler } from '../middleware/errorHandler';
@@ -72,7 +73,7 @@ describe('AI Routes (/api/ai)', () => {
it('should enqueue a job and return 202 on success', async () => {
vi.mocked(db.flyerRepo.findFlyerByChecksum).mockResolvedValue(undefined);
vi.mocked(flyerQueue.add).mockResolvedValue({ id: 'job-123' } as any);
vi.mocked(flyerQueue.add).mockResolvedValue({ id: 'job-123' } as unknown as Job);
const response = await supertest(app)
.post('/api/ai/upload-and-process')
@@ -187,7 +188,7 @@ describe('AI Routes (/api/ai)', () => {
it('should return job status if job is found', async () => {
const mockJob = { id: 'job-123', getState: async () => 'completed', progress: 100, returnvalue: { flyerId: 1 } };
vi.mocked(flyerQueue.getJob).mockResolvedValue(mockJob as any);
vi.mocked(flyerQueue.getJob).mockResolvedValue(mockJob as unknown as Job);
const response = await supertest(app).get('/api/ai/jobs/job-123/status');

1
src/routes/ai.routes.ts
View File

@@ -1,6 +1,5 @@
// src/routes/ai.ts
import { Router, Request, Response, NextFunction } from 'express';
import crypto from 'crypto';
import multer from 'multer';
import path from 'path';
import fs from 'fs';

2
src/routes/auth.routes.test.ts
View File

@@ -459,7 +459,7 @@ describe('Auth Routes (/api/auth)', () => {
});
it('should return 403 if refresh token is invalid', async () => {
vi.mocked(db.userRepo.findUserByRefreshToken).mockResolvedValue(undefined);
vi.mocked(db.userRepo.findUserByRefreshToken).mockRejectedValue(new Error('Invalid or expired refresh token.'));
const response = await supertest(app)
.post('/api/auth/refresh-token')

64
src/routes/auth.routes.ts
View File

@@ -99,7 +99,7 @@ router.post('/register', validateRequest(registerSchema), async (req, res, next)
logger.info(`Hashing password for new user: ${email}`);
// The createUser method in UserRepository now handles its own transaction.
const newUser = await userRepo.createUser(email, hashedPassword, { full_name, avatar_url });
const newUser = await userRepo.createUser(email, hashedPassword, { full_name, avatar_url }, req.log);
const userEmail = newUser.user.email || 'unknown';
const userId = newUser.user_id || 'unknown';
@@ -111,13 +111,13 @@ router.post('/register', validateRequest(registerSchema), async (req, res, next)
action: 'user_registered',
displayText: `${userEmail} has registered.`,
icon: 'user-plus',
});
}, req.log);
const payload = { user_id: newUser.user_id, email: userEmail };
const token = jwt.sign(payload, JWT_SECRET, { expiresIn: '1h' });
const refreshToken = crypto.randomBytes(64).toString('hex');
await userRepo.saveRefreshToken(newUser.user_id, refreshToken);
await userRepo.saveRefreshToken(newUser.user_id, refreshToken, req.log);
res.cookie('refreshToken', refreshToken, {
httpOnly: true,
@@ -131,7 +131,7 @@ router.post('/register', validateRequest(registerSchema), async (req, res, next)
return res.status(409).json({ message: error.message });
}
// The createUser method now handles its own transaction logging, so we just log the route failure.
logger.error(`User registration route failed for email: ${email}.`, { error });
logger.error({ error }, `User registration route failed for email: ${email}.`);
return next(error);
}
});
@@ -140,23 +140,23 @@ router.post('/register', validateRequest(registerSchema), async (req, res, next)
router.post('/login', (req: Request, res: Response, next: NextFunction) => {
passport.authenticate('local', { session: false }, async (err: Error, user: Express.User | false, info: { message: string }) => {
// --- LOGIN ROUTE DEBUG LOGGING ---
logger.debug(`[API /login] Received login request for email: ${req.body.email}`);
if (err) logger.error('[API /login] Passport reported an error.', { err });
if (!user) logger.warn('[API /login] Passport reported NO USER found.', { info });
if (user) logger.debug('[API /login] Passport user object:', { user }); // Log the user object passport returns
if (user) logger.info('[API /login] Passport reported USER FOUND.', { user });
req.log.debug(`[API /login] Received login request for email: ${req.body.email}`);
if (err) req.log.error({ err }, '[API /login] Passport reported an error.');
if (!user) req.log.warn({ info }, '[API /login] Passport reported NO USER found.');
if (user) req.log.debug({ user }, '[API /login] Passport user object:'); // Log the user object passport returns
if (user) req.log.info({ user }, '[API /login] Passport reported USER FOUND.');
try {
const allUsersInDb = await getPool().query('SELECT u.user_id, u.email, p.role FROM public.users u JOIN public.profiles p ON u.user_id = p.user_id');
logger.debug('[API /login] Current users in DB from SERVER perspective:');
req.log.debug('[API /login] Current users in DB from SERVER perspective:');
console.table(allUsersInDb.rows);
} catch (dbError) {
logger.error('[API /login] Could not query users table for debugging.', { dbError });
req.log.error({ dbError }, '[API /login] Could not query users table for debugging.');
}
// --- END DEBUG LOGGING ---
const { rememberMe } = req.body;
if (err) {
logger.error(`Login authentication error in /login route for email: ${req.body.email}`, { error: err });
req.log.error({ error: err }, `Login authentication error in /login route for email: ${req.body.email}`);
return next(err);
}
if (!user) {
@@ -169,8 +169,8 @@ router.post('/login', (req: Request, res: Response, next: NextFunction) => {
try {
const refreshToken = crypto.randomBytes(64).toString('hex');
await userRepo.saveRefreshToken(typedUser.user_id, refreshToken);
logger.info(`JWT and refresh token issued for user: ${typedUser.email}`);
await userRepo.saveRefreshToken(typedUser.user_id, refreshToken, req.log);
req.log.info(`JWT and refresh token issued for user: ${typedUser.email}`);
const cookieOptions = {
httpOnly: true,
@@ -183,7 +183,7 @@ router.post('/login', (req: Request, res: Response, next: NextFunction) => {
return res.json({ user: userResponse, token: accessToken });
} catch (tokenErr) {
logger.error(`Failed to save refresh token during login for user: ${typedUser.email}`, { error: tokenErr });
req.log.error({ error: tokenErr }, `Failed to save refresh token during login for user: ${typedUser.email}`);
return next(tokenErr);
}
})(req, res, next);
@@ -194,10 +194,10 @@ router.post('/forgot-password', forgotPasswordLimiter, validateRequest(forgotPas
const { email } = req.body;
try {
logger.debug(`[API /forgot-password] Received request for email: ${email}`);
const user = await userRepo.findUserByEmail(email);
req.log.debug(`[API /forgot-password] Received request for email: ${email}`);
const user = await userRepo.findUserByEmail(email, req.log);
let token: string | undefined;
logger.debug(`[API /forgot-password] Database search result for ${email}:`, { user: user ? { user_id: user.user_id, email: user.email } : 'NOT FOUND' });
req.log.debug({ user: user ? { user_id: user.user_id, email: user.email } : 'NOT FOUND' }, `[API /forgot-password] Database search result for ${email}:`);
if (user) {
token = crypto.randomBytes(32).toString('hex');
@@ -205,17 +205,17 @@ router.post('/forgot-password', forgotPasswordLimiter, validateRequest(forgotPas
const tokenHash = await bcrypt.hash(token, saltRounds);
const expiresAt = new Date(Date.now() + 3600000); // 1 hour
await userRepo.createPasswordResetToken(user.user_id, tokenHash, expiresAt);
await userRepo.createPasswordResetToken(user.user_id, tokenHash, expiresAt, req.log);
const resetLink = `${process.env.FRONTEND_URL}/reset-password/${token}`;
try {
await sendPasswordResetEmail(email, resetLink);
await sendPasswordResetEmail(email, resetLink, req.log);
} catch (emailError) {
logger.error(`Email send failure during password reset for user: ${emailError}`);
req.log.error({ emailError }, `Email send failure during password reset for user`);
}
} else {
logger.warn(`Password reset requested for non-existent email: ${email}`);
req.log.warn(`Password reset requested for non-existent email: ${email}`);
}
// For testability, return the token in the response only in the test environment.
@@ -225,7 +225,7 @@ router.post('/forgot-password', forgotPasswordLimiter, validateRequest(forgotPas
if (process.env.NODE_ENV === 'test' && user) responsePayload.token = token;
res.status(200).json(responsePayload);
} catch (error) {
logger.error(`An error occurred during /forgot-password for email: ${email}`, { error });
req.log.error({ error }, `An error occurred during /forgot-password for email: ${email}`);
next(error);
}
});
@@ -235,7 +235,7 @@ router.post('/reset-password', resetPasswordLimiter, validateRequest(resetPasswo
const { token, newPassword } = req.body;
try {
const validTokens = await userRepo.getValidResetTokens();
const validTokens = await userRepo.getValidResetTokens(req.log);
let tokenRecord;
for (const record of validTokens) {
const isMatch = await bcrypt.compare(token, record.token_hash);
@@ -252,8 +252,8 @@ router.post('/reset-password', resetPasswordLimiter, validateRequest(resetPasswo
const saltRounds = 10;
const hashedPassword = await bcrypt.hash(newPassword, saltRounds);
await userRepo.updateUserPassword(tokenRecord.user_id, hashedPassword);
await userRepo.deleteResetToken(tokenRecord.token_hash);
await userRepo.updateUserPassword(tokenRecord.user_id, hashedPassword, req.log);
await userRepo.deleteResetToken(tokenRecord.token_hash, req.log);
// Log this security event after a successful password reset.
await adminRepo.logActivity({
@@ -262,11 +262,11 @@ router.post('/reset-password', resetPasswordLimiter, validateRequest(resetPasswo
displayText: `User ID ${tokenRecord.user_id} has reset their password.`,
icon: 'key',
details: { source_ip: req.ip ?? null }
});
}, req.log);
res.status(200).json({ message: 'Password has been reset successfully.' });
} catch (error) {
logger.error(`An error occurred during password reset.`, { error });
req.log.error({ error }, `An error occurred during password reset.`);
next(error);
}
});
@@ -279,7 +279,7 @@ router.post('/refresh-token', async (req: Request, res: Response, next: NextFunc
}
try {
const user = await userRepo.findUserByRefreshToken(refreshToken);
const user = await userRepo.findUserByRefreshToken(refreshToken, req.log);
if (!user) {
return res.status(403).json({ message: 'Invalid or expired refresh token.' });
}
@@ -289,7 +289,7 @@ router.post('/refresh-token', async (req: Request, res: Response, next: NextFunc
res.json({ token: newAccessToken });
} catch (error) {
logger.error('An error occurred during /refresh-token.', { error });
req.log.error({ error }, 'An error occurred during /refresh-token.');
next(error);
}
});
@@ -304,8 +304,8 @@ router.post('/logout', async (req: Request, res: Response) => {
if (refreshToken) {
// Invalidate the token in the database so it cannot be used again.
// We don't need to wait for this to finish to respond to the user.
userRepo.deleteRefreshToken(refreshToken).catch((err: Error) => {
logger.error('Failed to delete refresh token from DB during logout.', { error: err });
userRepo.deleteRefreshToken(refreshToken, req.log).catch((err: Error) => {
req.log.error({ error: err }, 'Failed to delete refresh token from DB during logout.');
});
}
// Instruct the browser to clear the cookie by setting its expiration to the past.

3
src/routes/budget.routes.ts
View File

@@ -1,9 +1,8 @@
// src/routes/budget.ts
import express, { Request, Response, NextFunction } from 'express';
import express, { NextFunction } from 'express';
import { z } from 'zod';
import passport from './passport.routes';
import { budgetRepo } from '../services/db/index.db';
import { logger } from '../services/logger.server';
import type { UserProfile } from '../types';
import { validateRequest } from '../middleware/validation.middleware';

2
src/routes/flyer.routes.test.ts
View File

@@ -1,7 +1,7 @@
// src/routes/flyer.routes.test.ts
import { describe, it, expect, vi, beforeEach } from 'vitest';
import supertest from 'supertest';
import express, { Request, Response, NextFunction } from 'express';
import express from 'express';
import { errorHandler } from '../middleware/errorHandler';
import flyerRouter from './flyer.routes';
import { createMockFlyer, createMockFlyerItem } from '../tests/utils/mockFactories';

17
src/routes/flyer.routes.ts
View File

@@ -1,6 +1,5 @@
// src/routes/flyer.routes.ts
import { Router, Request, Response, NextFunction } from 'express';
import crypto from 'crypto';
import * as db from '../services/db/index.db';
import { z } from 'zod';
import { logger } from '../services/logger.server';
@@ -45,10 +44,10 @@ const trackItemSchema = z.object({
router.get('/', validateRequest(getFlyersSchema), async (req, res, next: NextFunction) => {
try {
const { limit, offset } = req.query as unknown as { limit: number; offset: number };
const flyers = await db.flyerRepo.getFlyers(limit, offset);
const flyers = await db.flyerRepo.getFlyers(req.log, limit, offset);
res.json(flyers);
} catch (error) {
logger.error('Error fetching flyers in /api/flyers:', { error });
req.log.error({ error }, 'Error fetching flyers in /api/flyers:');
next(error);
}
});
@@ -59,7 +58,7 @@ router.get('/', validateRequest(getFlyersSchema), async (req, res, next: NextFun
router.get('/:id', validateRequest(flyerIdParamSchema), async (req, res, next: NextFunction) => {
try {
const flyerId = req.params.id as unknown as number;
const flyer = await db.flyerRepo.getFlyerById(flyerId);
const flyer = await db.flyerRepo.getFlyerById(flyerId, req.log);
res.json(flyer);
} catch (error) {
next(error);
@@ -72,10 +71,10 @@ router.get('/:id', validateRequest(flyerIdParamSchema), async (req, res, next: N
router.get('/:id/items', validateRequest(flyerIdParamSchema), async (req, res, next: NextFunction) => {
try {
const flyerId = req.params.id as unknown as number;
const items = await db.flyerRepo.getFlyerItems(flyerId);
const items = await db.flyerRepo.getFlyerItems(flyerId, req.log);
res.json(items);
} catch (error) {
logger.error('Error fetching flyer items in /api/flyers/:id/items:', { error });
req.log.error({ error }, 'Error fetching flyer items in /api/flyers/:id/items:');
next(error);
}
});
@@ -86,7 +85,7 @@ router.get('/:id/items', validateRequest(flyerIdParamSchema), async (req, res, n
router.post('/items/batch-fetch', validateRequest(batchFetchSchema), async (req, res, next: NextFunction) => {
const { flyerIds } = req.body as { flyerIds: number[] };
try {
const items = await db.flyerRepo.getFlyerItemsForFlyers(flyerIds);
const items = await db.flyerRepo.getFlyerItemsForFlyers(flyerIds, req.log);
res.json(items);
} catch (error) {
next(error);
@@ -100,7 +99,7 @@ router.post('/items/batch-count', validateRequest(batchFetchSchema.partial()), a
const { flyerIds } = req.body as { flyerIds?: number[] };
try {
// The DB function handles an empty array, so we can simplify.
const count = await db.flyerRepo.countFlyerItemsForFlyers(flyerIds ?? []);
const count = await db.flyerRepo.countFlyerItemsForFlyers(flyerIds ?? [], req.log);
res.json({ count });
} catch (error) {
next(error);
@@ -113,7 +112,7 @@ router.post('/items/batch-count', validateRequest(batchFetchSchema.partial()), a
router.post('/items/:itemId/track', validateRequest(trackItemSchema), (req: Request, res: Response) => {
const itemId = req.params.itemId as unknown as number;
const { type } = req.body as { type: 'view' | 'click' };
db.flyerRepo.trackFlyerItemInteraction(itemId, type);
db.flyerRepo.trackFlyerItemInteraction(itemId, type, req.log);
res.status(202).send();
});

2
src/routes/health.routes.test.ts
View File

@@ -1,7 +1,7 @@
// src/routes/health.routes.test.ts
import { describe, it, expect, vi, beforeEach, afterEach, type Mocked } from 'vitest';
import supertest from 'supertest';
import express, { Request, Response, NextFunction } from 'express';
import express from 'express';
import healthRouter from './health.routes';
import * as dbConnection from '../services/db/connection.db';
import { connection as redisConnection } from '../services/queueService.server';

8
src/routes/passport.routes.test.ts
View File

@@ -252,7 +252,10 @@ describe('Passport Configuration', () => {
it('should call done(null, false) when user is not found', async () => {
// Arrange
const jwtPayload = { user_id: 'non-existent-user' };
vi.mocked(mockedDb.userRepo.findUserProfileById).mockResolvedValue(undefined);
// Per ADR-001, the repository method throws an error when the user is not found.
// The JWT strategy's catch block will then handle this and call done(err, false).
const notFoundError = new Error('User not found');
vi.mocked(mockedDb.userRepo.findUserProfileById).mockRejectedValue(notFoundError);
const done = vi.fn();
// Act
@@ -261,7 +264,8 @@ describe('Passport Configuration', () => {
}
// Assert
expect(done).toHaveBeenCalledWith(null, false);
// The strategy's catch block passes the error to done().
expect(done).toHaveBeenCalledWith(notFoundError, false);
});
it('should call done(err) if the database lookup fails', async () => {

29
src/routes/passport.routes.ts
View File

@@ -10,7 +10,6 @@ import { Request, Response, NextFunction } from 'express';
import * as db from '../services/db/index.db';
import { logger } from '../services/logger.server';
import { UserProfile } from '../types';
import { omit } from '../utils/objectUtils';
import { createMockUserProfile } from '../tests/utils/mockFactories';
const JWT_SECRET = process.env.JWT_SECRET!;
@@ -37,7 +36,7 @@ passport.use(new LocalStrategy(
async (req: Request, email, password, done) => {
try {
// 1. Find the user by email, including their profile data for the JWT payload.
const user = await db.userRepo.findUserWithProfileByEmail(email);
const user = await db.userRepo.findUserWithProfileByEmail(email, req.log);
if (!user) {
// User not found
@@ -73,7 +72,7 @@ passport.use(new LocalStrategy(
logger.warn(`Login attempt failed for user ${email} due to incorrect password.`);
// Increment failed attempts
await db.adminRepo.incrementFailedLoginAttempts(user.user_id);
await db.adminRepo.incrementFailedLoginAttempts(user.user_id, req.log);
// Log this security event.
await db.adminRepo.logActivity({
@@ -81,22 +80,22 @@ passport.use(new LocalStrategy(
action: 'login_failed_password',
displayText: `Failed login attempt for user ${user.email}.`,
icon: 'shield-alert',
details: { source_ip: req.ip ?? null }
});
details: { source_ip: req.ip ?? null },
}, req.log);
return done(null, false, { message: 'Incorrect email or password.' });
}
// 3. Success! Return the user object (without password_hash for security).
// Reset failed login attempts upon successful login.
await db.adminRepo.resetFailedLoginAttempts(user.user_id, req.ip ?? 'unknown');
await db.adminRepo.resetFailedLoginAttempts(user.user_id, req.ip ?? 'unknown', req.log);
logger.info(`User successfully authenticated: ${email}`);
// The user object from `findUserWithProfileByEmail` already excludes the password hash.
// This object will be passed to the /login route handler.
const userWithoutHash = user;
return done(null, userWithoutHash);
} catch (err) {
logger.error('Error during local authentication strategy:', { error: err });
} catch (err: unknown) {
req.log.error({ error: err }, 'Error during local authentication strategy:');
return done(err);
}
}
@@ -213,11 +212,11 @@ const jwtOptions = {
};
passport.use(new JwtStrategy(jwtOptions, async (jwt_payload, done) => {
logger.debug('[JWT Strategy] Verifying token payload:', { jwt_payload: jwt_payload ? { user_id: jwt_payload.user_id } : 'null' });
logger.debug({ jwt_payload: jwt_payload ? { user_id: jwt_payload.user_id } : 'null' }, '[JWT Strategy] Verifying token payload:');
try {
// The jwt_payload contains the data you put into the token during login (e.g., { user_id: user.user_id, email: user.email }).
// We re-fetch the user from the database here to ensure they are still active and valid.
const userProfile = await db.userRepo.findUserProfileById(jwt_payload.user_id);
const userProfile = await db.userRepo.findUserProfileById(jwt_payload.user_id, logger);
// --- JWT STRATEGY DEBUG LOGGING ---
logger.debug(`[JWT Strategy] DB lookup for user ID ${jwt_payload.user_id} result: ${userProfile ? 'FOUND' : 'NOT FOUND'}`);
@@ -228,8 +227,8 @@ passport.use(new JwtStrategy(jwtOptions, async (jwt_payload, done) => {
logger.warn(`JWT authentication failed: user with ID ${jwt_payload.user_id} not found.`);
return done(null, false); // User not found or invalid token
}
} catch (err) {
logger.error('Error during JWT authentication strategy:', { error: err });
} catch (err: unknown) {
logger.error({ error: err }, 'Error during JWT authentication strategy:');
return done(err, false);
}
}));
@@ -259,9 +258,9 @@ export const optionalAuth = (req: Request, res: Response, next: NextFunction) =>
// The custom callback for passport.authenticate gives us access to `err`, `user`, and `info`.
passport.authenticate('jwt', { session: false }, (err: Error | null, user: Express.User | false, info: { message: string } | Error) => {
// If there's an authentication error (e.g., malformed token), log it but don't block the request.
if (info) {
logger.info('Optional auth info:', { info: info.message || info.toString() });
}
if (info) { // The patch requested this specific error handling.
logger.info({ info: info.message || info.toString() }, 'Optional auth info:');
} // The patch requested this specific error handling.
if (user) (req as Express.Request).user = user; // Attach user if authentication succeeds
next(); // Always proceed to the next middleware

1
src/routes/personalization.routes.ts
View File

@@ -1,7 +1,6 @@
// src/routes/personalization.routes.ts
import { Router, Request, Response, NextFunction } from 'express';
import * as db from '../services/db/index.db';
import { logger } from '../services/logger.server';
const router = Router();

1
src/routes/recipe.routes.ts
View File

@@ -2,7 +2,6 @@
import { Router, type Request, type Response, type NextFunction } from 'express';
import { z } from 'zod';
import * as db from '../services/db/index.db';
import { logger } from '../services/logger.server';
import { validateRequest } from '../middleware/validation.middleware';
const router = Router();

36
src/routes/system.routes.test.ts
View File

@@ -3,7 +3,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest';
import supertest from 'supertest';
import express from 'express';
import systemRouter from './system.routes';
import { exec } from 'child_process';
import { exec, type ExecException } from 'child_process';
import { geocodingService } from '../services/geocodingService.server';
import { errorHandler } from '../middleware/errorHandler';
@@ -60,11 +60,16 @@ describe('System Routes (/api/system)', () => {
└───────────┴───────────┘
`;
type ExecCallback = (error: ExecException | null, stdout: string, stderr: string) => void;
// Strict implementation that finds the callback (last argument)
vi.mocked(exec).mockImplementation((...args: any[]) => {
const callback = args.find(arg => typeof arg === 'function');
callback(null, pm2OnlineOutput, '');
return {} as any;
vi.mocked(exec).mockImplementation((command: string, ...args: any[]) => {
const callback = args.find(arg => typeof arg === 'function') as ExecCallback | undefined;
if (callback) {
callback(null, pm2OnlineOutput, '');
}
// Return a minimal object that satisfies the ChildProcess type for .unref()
return { unref: () => {} } as ReturnType<typeof exec>;
});
// Act
@@ -78,10 +83,12 @@ describe('System Routes (/api/system)', () => {
it('should return success: false when pm2 process is stopped or errored', async () => {
const pm2StoppedOutput = `│ status │ stopped │`;
vi.mocked(exec).mockImplementation((...args: any[]) => {
const callback = args.find(arg => typeof arg === 'function');
callback(null, pm2StoppedOutput, '');
return {} as any;
vi.mocked(exec).mockImplementation((command: string, ...args: any[]) => {
const callback = args.find(arg => typeof arg === 'function') as ((error: ExecException | null, stdout: string, stderr: string) => void) | undefined;
if (callback) {
callback(null, pm2StoppedOutput, '');
}
return { unref: () => {} } as ReturnType<typeof exec>;
});
const response = await supertest(app).get('/api/system/pm2-status');
@@ -92,11 +99,12 @@ describe('System Routes (/api/system)', () => {
});
it('should return 500 on a generic exec error', async () => {
vi.mocked(exec).mockImplementation((...args: any[]) => {
const callback = args.find(arg => typeof arg === 'function');
// Generic system error (not PM2 specific)
callback(new Error('System error'), '', 'stderr output');
return {} as any;
vi.mocked(exec).mockImplementation((command: string, ...args: any[]) => {
const callback = args.find(arg => typeof arg === 'function') as ((error: ExecException | null, stdout: string, stderr: string) => void) | undefined;
if (callback) {
callback(new Error('System error') as ExecException, '', 'stderr output');
}
return { unref: () => {} } as ReturnType<typeof exec>;
});
// Act