diff --git a/src/App.test.tsx b/src/App.test.tsx index fdc7b67b..f2ec3947 100644 --- a/src/App.test.tsx +++ b/src/App.test.tsx @@ -1,6 +1,6 @@ // src/App.test.tsx import React from 'react'; -import { render, screen, waitFor, fireEvent, act, within } from '@testing-library/react'; +import { render, screen, waitFor, fireEvent, act, within, renderHook } from '@testing-library/react'; import { describe, it, expect, vi, beforeEach } from 'vitest'; import { MemoryRouter, Outlet } from 'react-router-dom'; import App from './App'; @@ -15,7 +15,7 @@ vi.mock('./hooks/useAuth', () => ({ // Mock top-level components rendered by App's routes vi.mock('./components/Header', () => ({ Header: (props: any) =>
})); -vi.mock('./pages/admin/components/ProfileManager', () => ({ ProfileManager: ({ onClose }: { onClose: () => void }) =>
})); +vi.mock('./pages/admin/components/ProfileManager', () => ({ ProfileManager: ({ onClose, onProfileUpdate, onLoginSuccess }: { onClose: () => void, onProfileUpdate: (p: any) => void, onLoginSuccess: (u: any, t: string) => void }) =>
})); vi.mock('./features/voice-assistant/VoiceAssistant', () => ({ VoiceAssistant: ({ onClose }: { onClose: () => void }) =>
})); vi.mock('./pages/admin/AdminPage', () => ({ AdminPage: () =>
Admin Page
})); // In react-router v6, wrapper routes must render an for nested routes to appear. @@ -24,9 +24,9 @@ vi.mock('./pages/admin/CorrectionsPage', () => ({ CorrectionsPage: () =>
({ AdminStatsPage: () =>
Admin Stats Page
})); vi.mock('./pages/ResetPasswordPage', () => ({ ResetPasswordPage: () =>
Reset Password Page
})); vi.mock('./pages/admin/VoiceLabPage', () => ({ VoiceLabPage: () =>
Voice Lab Page
})); // Corrected path -vi.mock('./components/WhatsNewModal', () => ({ WhatsNewModal: ({ onClose }: { onClose: () => void }) =>
})); +vi.mock('./components/WhatsNewModal', () => ({ WhatsNewModal: ({ isOpen, onClose }: { isOpen: boolean, onClose: () => void }) => isOpen ?
: null })); -vi.mock('./components/FlyerCorrectionTool', () => ({ FlyerCorrectionTool: ({ onClose, onDataExtracted }: { onClose: () => void, onDataExtracted: (type: 'store_name' | 'dates', value: string) => void }) =>
})); +vi.mock('./components/FlyerCorrectionTool', () => ({ FlyerCorrectionTool: ({ onClose, onDataExtracted }: { onClose: () => void, onDataExtracted: (type: 'store_name' | 'dates', value: string) => void }) =>
})); // Mock the new layout and page components vi.mock('./layouts/MainLayout', () => ({ MainLayout: () =>
})); vi.mock('./pages/HomePage', () => ({ HomePage: (props: any) =>
})); @@ -34,6 +34,7 @@ vi.mock('./pages/HomePage', () => ({ HomePage: (props: any) =>
({ + // pdfjsLib: { GlobalWorkerOptions: { workerSrc: '' } }, GlobalWorkerOptions: { workerSrc: '' }, getDocument: vi.fn(() => ({ promise: Promise.resolve({ getPage: vi.fn() }), @@ -327,6 +328,17 @@ describe('App Component', () => { expect(mockLogin).toHaveBeenCalledWith({ user_id: '', email: '' }, 'test-github-token'); }); }); + + it('should log an error if login with a token fails', async () => { + const mockLogin = vi.fn().mockRejectedValue(new Error('Token login failed')); + mockUseAuth.mockReturnValue({ + user: null, profile: null, authStatus: 'SIGNED_OUT', isLoading: false, + login: mockLogin, logout: vi.fn(), updateProfile: vi.fn(), + }); + + renderApp(['/?googleAuthToken=bad-token']); + await waitFor(() => expect(mockLogin).toHaveBeenCalled()); + }); }); describe('Flyer Selection from URL', () => { @@ -424,14 +436,98 @@ describe('App Component', () => { }); }); - it('should handle data extraction from the correction tool', async () => { - renderApp(); - fireEvent.click(screen.getByText('Open Correction Tool')); - const correctionTool = await screen.findByTestId('flyer-correction-tool-mock'); - // The logic for updating the flyer is inside App.tsx, but we can't see the state. - // We trigger the callback from the mock and ensure it doesn't crash. - // A more integrated test would be needed to see the visual change. - fireEvent.click(within(correctionTool).getByText('Extract Data')); - // The test passes if no errors are thrown here. + describe('Flyer Correction Tool Data Handling', () => { + it('should handle store name extraction from the correction tool', async () => { + renderApp(); + fireEvent.click(screen.getByText('Open Correction Tool')); + const correctionTool = await screen.findByTestId('flyer-correction-tool-mock'); + // We trigger the callback from the mock and ensure it doesn't crash. + fireEvent.click(within(correctionTool).getByText('Extract Store')); + // The test passes if no errors are thrown here. + }); + + it('should handle date extraction from the correction tool', async () => { + renderApp(); + fireEvent.click(screen.getByText('Open Correction Tool')); + const correctionTool = await screen.findByTestId('flyer-correction-tool-mock'); + fireEvent.click(within(correctionTool).getByText('Extract Dates')); + // The test passes if no errors are thrown here, covering the 'dates' branch. + }); + }); + + describe('Version Display and What\'s New', () => { + const mockEnv = { + VITE_APP_VERSION: '2.0.0', + VITE_APP_COMMIT_MESSAGE: 'A new version!', + VITE_APP_COMMIT_URL: 'http://example.com/commit/2.0.0', + }; + + beforeEach(() => { + Object.defineProperty(import.meta, 'env', { + value: mockEnv, + configurable: true, + }); + }); + + it('should display the version number and commit link', () => { + renderApp(); + const versionLink = screen.getByText(`Version: ${mockEnv.VITE_APP_VERSION}`); + expect(versionLink).toBeInTheDocument(); + expect(versionLink).toHaveAttribute('href', mockEnv.VITE_APP_COMMIT_URL); + }); + + it('should open the "What\'s New" modal when the question mark icon is clicked', async () => { + renderApp(); + expect(screen.queryByTestId('whats-new-modal-mock')).not.toBeInTheDocument(); + + const openButton = screen.getByTitle("Show what's new in this version"); + fireEvent.click(openButton); + + expect(await screen.findByTestId('whats-new-modal-mock')).toBeInTheDocument(); + }); + }); + + describe('Dynamic Toaster Styles', () => { + it('should render the correct CSS variables for toast styling in light mode', () => { + renderApp(); + const styleTag = document.querySelector('style'); + expect(styleTag).not.toBeNull(); + expect(styleTag!.innerHTML).toContain('--toast-bg: #FFFFFF'); + expect(styleTag!.innerHTML).toContain('--toast-color: #1F2937'); + }); + + it('should render the correct CSS variables for toast styling in dark mode', () => { + localStorageMock.setItem('darkMode', 'true'); + renderApp(); + const styleTag = document.querySelector('style'); + expect(styleTag).not.toBeNull(); + expect(styleTag!.innerHTML).toContain('--toast-bg: #4B5563'); + }); + }); + + describe('Profile and Login Handlers', () => { + it('should call updateProfile when handleProfileUpdate is triggered', async () => { + const mockUpdateProfile = vi.fn(); + mockUseAuth.mockReturnValue({ + ...mockUseAuth(), + updateProfile: mockUpdateProfile, + }); + + renderApp(); + fireEvent.click(screen.getByText('Open Profile')); + const profileManager = await screen.findByTestId('profile-manager-mock'); + fireEvent.click(within(profileManager).getByText('Update Profile')); + + expect(mockUpdateProfile).toHaveBeenCalledWith({ full_name: 'Updated' }); + }); + + it('should set an error state if login fails inside handleLoginSuccess', async () => { + const mockLogin = vi.fn().mockRejectedValue(new Error('Login failed')); + mockUseAuth.mockReturnValue({ ...mockUseAuth(), login: mockLogin }); + + renderApp(); + fireEvent.click(screen.getByText('Open Profile')); + fireEvent.click(await screen.findByText('Login')); + }); }); }); \ No newline at end of file diff --git a/src/middleware/errorHandler.test.ts b/src/middleware/errorHandler.test.ts index d1fd1fec..c90cca98 100644 --- a/src/middleware/errorHandler.test.ts +++ b/src/middleware/errorHandler.test.ts @@ -4,6 +4,7 @@ import supertest from 'supertest'; import express, { Request, Response, NextFunction } from 'express'; import { errorHandler } from './errorHandler'; import { DatabaseError, ForeignKeyConstraintError, UniqueConstraintError } from '../services/db/errors.db'; +import { logger } from '../services/logger.server'; // Mock the logger to prevent console output during tests and to verify it's called vi.mock('../services/logger.server', () => ({ @@ -58,8 +59,6 @@ app.get('/unauthorized-error-with-status', (req, res, next) => { app.use(errorHandler); describe('errorHandler Middleware', () => { - const { logger } = require('../services/logger.server'); // Get the mocked logger - beforeEach(() => { vi.clearAllMocks(); consoleErrorSpy.mockClear(); // Clear spy for console.error diff --git a/src/middleware/errorHandler.ts b/src/middleware/errorHandler.ts index f9fff7ab..5fdd0d83 100644 --- a/src/middleware/errorHandler.ts +++ b/src/middleware/errorHandler.ts @@ -41,6 +41,10 @@ export const errorHandler = (err: HttpError, req: Request, res: Response, next: } res.status(statusCode).json({ - message: statusCode >= 500 ? 'An unexpected server error occurred.' : message, + // In production, send a generic message for 5xx errors. + // In dev/test, send the actual error message for easier debugging. + message: (statusCode >= 500 && process.env.NODE_ENV === 'production') + ? 'An unexpected server error occurred.' + : message, }); }; \ No newline at end of file diff --git a/src/pages/HomePage.test.tsx b/src/pages/HomePage.test.tsx index c7c61d39..866bbf97 100644 --- a/src/pages/HomePage.test.tsx +++ b/src/pages/HomePage.test.tsx @@ -6,6 +6,7 @@ import { MemoryRouter, useOutletContext } from 'react-router-dom'; import { HomePage } from './HomePage'; import { createMockFlyer, createMockFlyerItem } from '../tests/utils/mockFactories'; import type { Flyer, FlyerItem } from '../types'; +import { ExtractedDataTable } from '../features/flyer/ExtractedDataTable'; // Mock child components to isolate the HomePage logic vi.mock('../features/flyer/FlyerDisplay', () => ({ @@ -109,12 +110,12 @@ describe('HomePage Component', () => { // We can also check the props passed to the mock component by inspecting the mock itself. // This is a more robust way to test prop passing. - const ExtractedDataTableMock = vi.mocked(require('../features/flyer/ExtractedDataTable').ExtractedDataTable); - const props = ExtractedDataTableMock.mock.calls[0][0]; + const ExtractedDataTableMock = vi.mocked(ExtractedDataTable); + const props = ExtractedDataTableMock.mock.calls[ExtractedDataTableMock.mock.calls.length - 1][0]; expect(props.items).toEqual(mockItems); expect(props.masterItems).toEqual(mockContext.masterItems); - expect(props.addWatchedItem).toBe(mockContext.addWatchedItem); + expect(props.onAddItem).toBe(mockContext.addWatchedItem); }); }); }); \ No newline at end of file diff --git a/src/routes/admin.content.routes.test.ts b/src/routes/admin.content.routes.test.ts index 339c3f0e..dc30f75f 100644 --- a/src/routes/admin.content.routes.test.ts +++ b/src/routes/admin.content.routes.test.ts @@ -16,6 +16,7 @@ import express, { Request, Response, NextFunction } from 'express'; import adminRouter from './admin.routes'; import { createMockUserProfile, createMockSuggestedCorrection, createMockBrand, createMockRecipe, createMockRecipeComment, createMockFlyerItem } from '../tests/utils/mockFactories'; import { SuggestedCorrection, Brand, UserProfile, UnmatchedFlyerItem } from '../types'; +import { errorHandler } from '../middleware/errorHandler'; vi.mock('../lib/queue', () => ({ serverAdapter: { @@ -114,9 +115,7 @@ const createApp = (user?: UserProfile) => { }); } app.use('/api/admin', adminRouter); - app.use((err: Error, req: Request, res: Response, next: NextFunction) => { - res.status(500).json({ message: err.message || 'Internal Server Error' }); - }); + app.use(errorHandler); return app; }; diff --git a/src/routes/admin.jobs.routes.test.ts b/src/routes/admin.jobs.routes.test.ts index 75598005..5f9b5b81 100644 --- a/src/routes/admin.jobs.routes.test.ts +++ b/src/routes/admin.jobs.routes.test.ts @@ -15,6 +15,7 @@ import adminRouter from './admin.routes'; import { createMockUserProfile } from '../tests/utils/mockFactories'; import { Job } from 'bullmq'; import { UserProfile } from '../types'; +import { errorHandler } from '../middleware/errorHandler'; vi.mock('../lib/queue', () => ({ serverAdapter: { @@ -102,9 +103,7 @@ const createApp = (user?: UserProfile) => { }); } app.use('/api/admin', adminRouter); - app.use((err: Error, req: Request, res: Response, next: NextFunction) => { - res.status(500).json({ message: err.message || 'Internal Server Error' }); - }); + app.use(errorHandler); return app; }; diff --git a/src/routes/admin.monitoring.routes.test.ts b/src/routes/admin.monitoring.routes.test.ts index b035937d..c947f45c 100644 --- a/src/routes/admin.monitoring.routes.test.ts +++ b/src/routes/admin.monitoring.routes.test.ts @@ -17,6 +17,7 @@ import express, { Request, Response, NextFunction } from 'express'; import adminRouter from './admin.routes'; import { createMockUserProfile, createMockActivityLogItem } from '../tests/utils/mockFactories'; import { UserProfile } from '../types'; +import { errorHandler } from '../middleware/errorHandler'; vi.mock('../lib/queue', () => ({ serverAdapter: { @@ -111,9 +112,7 @@ const createApp = (user?: UserProfile) => { }); } app.use('/api/admin', adminRouter); - app.use((err: Error, req: Request, res: Response, _next: NextFunction) => { - res.status(500).json({ message: err.message || 'Internal Server Error' }); - }); + app.use(errorHandler); return app; }; diff --git a/src/routes/admin.stats.routes.test.ts b/src/routes/admin.stats.routes.test.ts index 6b680da5..14dd5533 100644 --- a/src/routes/admin.stats.routes.test.ts +++ b/src/routes/admin.stats.routes.test.ts @@ -12,6 +12,7 @@ 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'; const { mockedDb } = vi.hoisted(() => { return { @@ -80,9 +81,7 @@ const createApp = (user?: UserProfile) => { }); } app.use('/api/admin', adminRouter); - app.use((err: Error, req: Request, res: Response) => { - res.status(500).json({ message: err.message || 'Internal Server Error' }); - }); + app.use(errorHandler); return app; }; diff --git a/src/routes/admin.system.routes.test.ts b/src/routes/admin.system.routes.test.ts index af18492a..1d0e4f67 100644 --- a/src/routes/admin.system.routes.test.ts +++ b/src/routes/admin.system.routes.test.ts @@ -5,6 +5,7 @@ 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 vi.mock('../services/geocodingService.server', () => ({ @@ -52,9 +53,7 @@ const createApp = () => { const app = express(); app.use(express.json()); app.use('/api/admin', adminRouter); - app.use((err: Error, req: Request, res: Response, next: NextFunction) => { - res.status(500).json({ message: err.message || 'Internal Server Error' }); - }); + app.use(errorHandler); return app; }; diff --git a/src/routes/admin.users.routes.test.ts b/src/routes/admin.users.routes.test.ts index a4655f1b..c46982c5 100644 --- a/src/routes/admin.users.routes.test.ts +++ b/src/routes/admin.users.routes.test.ts @@ -12,6 +12,7 @@ import express, { Request, Response, NextFunction } from 'express'; import adminRouter from './admin.routes'; import { createMockUserProfile } from '../tests/utils/mockFactories'; import { User, UserProfile } from '../types'; +import { errorHandler } from '../middleware/errorHandler'; const { mockedDb } = vi.hoisted(() => { return { @@ -89,9 +90,7 @@ const createApp = (user?: UserProfile) => { }); } app.use('/api/admin', adminRouter); - app.use((err: Error, req: Request, res: Response) => { - res.status(500).json({ message: err.message || 'Internal Server Error' }); - }); + app.use(errorHandler); return app; }; diff --git a/src/routes/ai.routes.test.ts b/src/routes/ai.routes.test.ts index 14fae594..a6705060 100644 --- a/src/routes/ai.routes.test.ts +++ b/src/routes/ai.routes.test.ts @@ -5,6 +5,7 @@ import express, { type Request, type Response, type NextFunction } from 'express import path from 'node:path'; import aiRouter from './ai.routes'; import { createMockUserProfile, createMockFlyer } from '../tests/utils/mockFactories'; +import { errorHandler } from '../middleware/errorHandler'; import * as flyerDb from '../services/db/flyer.db'; import * as db from '../services/db/index.db'; import * as aiService from '../services/aiService.server'; @@ -59,13 +60,7 @@ vi.mock('./passport.routes', () => ({ const app = express(); app.use(express.json({ strict: false })); app.use('/api/ai', aiRouter); - -// FIX: Add a generic error handler with the correct 4-argument signature. -// This ensures that errors passed via `next(error)` in the routes are caught -// and formatted into a JSON response that the tests can assert against. -app.use((err: Error, req: Request, res: Response, next: NextFunction) => { - res.status(500).json({ message: err.message || 'Internal Server Error' }); -}); +app.use(errorHandler); describe('AI Routes (/api/ai)', () => { beforeEach(() => { diff --git a/src/routes/auth.routes.test.ts b/src/routes/auth.routes.test.ts index c7a67f84..f3ca04bb 100644 --- a/src/routes/auth.routes.test.ts +++ b/src/routes/auth.routes.test.ts @@ -12,6 +12,7 @@ import express, { Request, Response, NextFunction } from 'express'; import cookieParser from 'cookie-parser'; import * as bcrypt from 'bcrypt'; import passport from 'passport'; +import { errorHandler } from '../middleware/errorHandler'; import { UserProfile } from '../types'; // --- FIX: Hoist passport mocks to be available for vi.mock --- @@ -151,11 +152,7 @@ app.use((req: Request, res: Response, next: NextFunction) => { }); app.use('/api/auth', authRouter); - -// Add error handler -app.use((err: any, req: Request, res: Response, next: NextFunction) => { - res.status(err.status || 500).json({ message: err.message }); -}); +app.use(errorHandler); // --- 5. Tests --- describe('Auth Routes (/api/auth)', () => { diff --git a/src/routes/budget.routes.test.ts b/src/routes/budget.routes.test.ts index a9d65c53..6c851b25 100644 --- a/src/routes/budget.routes.test.ts +++ b/src/routes/budget.routes.test.ts @@ -4,6 +4,7 @@ import supertest from 'supertest'; import express, { Request, Response, NextFunction } from 'express'; import budgetRouter from './budget.routes'; import * as db from '../services/db/index.db'; +import { errorHandler } from '../middleware/errorHandler'; import { createMockUserProfile, createMockBudget, createMockSpendingByCategory } from '../tests/utils/mockFactories'; import { ForeignKeyConstraintError } from '../services/db/errors.db'; @@ -51,11 +52,7 @@ vi.mock('./passport.routes', () => ({ const app = express(); app.use(express.json()); app.use('/api/budgets', budgetRouter); - -// Add a basic error handler to return JSON errors instead of Express default HTML -app.use((err: Error, req: Request, res: Response, next: NextFunction) => { - res.status(500).json({ message: 'Internal Server Error' }); -}); +app.use(errorHandler); describe('Budget Routes (/api/budgets)', () => { const mockUserProfile = createMockUserProfile({ user_id: 'user-123', points: 100 }); @@ -84,8 +81,8 @@ describe('Budget Routes (/api/budgets)', () => { it('should return 500 if the database call fails', async () => { vi.mocked(db.budgetRepo.getBudgetsForUser).mockRejectedValue(new Error('DB Error')); const response = await supertest(app).get('/api/budgets'); - expect(response.status).toBe(500); - expect(response.body.message).toBe('Internal Server Error'); + expect(response.status).toBe(500); // The custom handler will now be used + expect(response.body.message).toBe('DB Error'); }); }); @@ -155,8 +152,8 @@ describe('Budget Routes (/api/budgets)', () => { const budgetUpdates = { amount_cents: 60000 }; vi.mocked(db.budgetRepo.updateBudget).mockRejectedValue(new Error('DB Error')); const response = await supertest(app).put('/api/budgets/1').send(budgetUpdates); - expect(response.status).toBe(500); - expect(response.body.message).toBe('Internal Server Error'); + expect(response.status).toBe(500); // The custom handler will now be used + expect(response.body.message).toBe('DB Error'); }); }); @@ -186,8 +183,8 @@ describe('Budget Routes (/api/budgets)', () => { it('should return 500 if a generic database error occurs', async () => { vi.mocked(db.budgetRepo.deleteBudget).mockRejectedValue(new Error('DB Error')); const response = await supertest(app).delete('/api/budgets/1'); - expect(response.status).toBe(500); - expect(response.body.message).toBe('Internal Server Error'); + expect(response.status).toBe(500); // The custom handler will now be used + expect(response.body.message).toBe('DB Error'); }); }); diff --git a/src/routes/flyer.routes.test.ts b/src/routes/flyer.routes.test.ts index 3fb95df1..14c4310b 100644 --- a/src/routes/flyer.routes.test.ts +++ b/src/routes/flyer.routes.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import supertest from 'supertest'; import express, { Request, Response, NextFunction } from 'express'; +import { errorHandler } from '../middleware/errorHandler'; import flyerRouter from './flyer.routes'; import { createMockFlyer, createMockFlyerItem } from '../tests/utils/mockFactories'; @@ -32,11 +33,7 @@ const app = express(); app.use(express.json()); // Mount the router under its designated base path app.use('/api/flyers', flyerRouter); - -// Add a generic error handler to catch errors passed via next() -app.use((err: Error, req: Request, res: Response, next: NextFunction) => { - res.status(500).json({ message: err.message || 'Internal Server Error' }); -}); +app.use(errorHandler); describe('Flyer Routes (/api/flyers)', () => { beforeEach(() => { @@ -64,6 +61,7 @@ describe('Flyer Routes (/api/flyers)', () => { vi.mocked(db.flyerRepo.getFlyers).mockRejectedValue(new Error('DB Error')); const response = await supertest(app).get('/api/flyers'); expect(response.status).toBe(500); + expect(response.body.message).toBe('DB Error'); }); }); @@ -96,6 +94,7 @@ describe('Flyer Routes (/api/flyers)', () => { vi.mocked(db.flyerRepo.getFlyerById).mockRejectedValue(new Error('DB Error')); const response = await supertest(app).get('/api/flyers/123'); expect(response.status).toBe(500); + expect(response.body.message).toBe('DB Error'); }); }); @@ -120,6 +119,7 @@ describe('Flyer Routes (/api/flyers)', () => { vi.mocked(db.flyerRepo.getFlyerItems).mockRejectedValue(new Error('DB Error')); const response = await supertest(app).get('/api/flyers/123/items'); expect(response.status).toBe(500); + expect(response.body.message).toBe('DB Error'); }); }); @@ -155,6 +155,7 @@ describe('Flyer Routes (/api/flyers)', () => { vi.mocked(db.flyerRepo.getFlyerItemsForFlyers).mockRejectedValue(new Error('DB Error')); const response = await supertest(app).post('/api/flyers/items/batch-fetch').send({ flyerIds: [1] }); expect(response.status).toBe(500); + expect(response.body.message).toBe('DB Error'); }); }); @@ -189,6 +190,7 @@ describe('Flyer Routes (/api/flyers)', () => { vi.mocked(db.flyerRepo.countFlyerItemsForFlyers).mockRejectedValue(new Error('DB Error')); const response = await supertest(app).post('/api/flyers/items/batch-count').send({ flyerIds: [1] }); expect(response.status).toBe(500); + expect(response.body.message).toBe('DB Error'); }); }); diff --git a/src/routes/gamification.routes.test.ts b/src/routes/gamification.routes.test.ts index fe5d83b9..92e3111a 100644 --- a/src/routes/gamification.routes.test.ts +++ b/src/routes/gamification.routes.test.ts @@ -4,6 +4,7 @@ import supertest from 'supertest'; import express, { Request, Response, NextFunction } from 'express'; import gamificationRouter from './gamification.routes'; import * as db from '../services/db/index.db'; +import { errorHandler } from '../middleware/errorHandler'; import { createMockUserProfile, createMockAchievement, createMockUserAchievement } from '../tests/utils/mockFactories'; import { ForeignKeyConstraintError } from '../services/db/errors.db'; @@ -44,6 +45,7 @@ vi.mock('./passport.routes', () => ({ const app = express(); app.use(express.json({ strict: false })); app.use('/api/achievements', gamificationRouter); +app.use(errorHandler); describe('Gamification Routes (/api/achievements)', () => { const mockUserProfile = createMockUserProfile({ user_id: 'user-123', points: 100 }); @@ -78,6 +80,7 @@ describe('Gamification Routes (/api/achievements)', () => { const response = await supertest(app).get('/api/achievements'); expect(response.status).toBe(500); + expect(response.body.message).toBe('DB Connection Failed'); }); it('should return 400 if awarding an achievement to a non-existent user', async () => { @@ -127,6 +130,7 @@ describe('Gamification Routes (/api/achievements)', () => { vi.mocked(db.gamificationRepo.getUserAchievements).mockRejectedValue(dbError); const response = await supertest(app).get('/api/achievements/me'); expect(response.status).toBe(500); + expect(response.body.message).toBe('DB Error'); }); }); @@ -189,6 +193,7 @@ describe('Gamification Routes (/api/achievements)', () => { const response = await supertest(app).post('/api/achievements/award').send(awardPayload); expect(response.status).toBe(500); + expect(response.body.message).toBe('DB Error'); }); it('should return 400 if awarding an achievement to a non-existent user', async () => { @@ -221,6 +226,7 @@ describe('Gamification Routes (/api/achievements)', () => { vi.mocked(db.gamificationRepo.getLeaderboard).mockRejectedValue(new Error('DB Error')); const response = await supertest(app).get('/api/achievements/leaderboard'); expect(response.status).toBe(500); + expect(response.body.message).toBe('DB Error'); }); }); }); diff --git a/src/routes/health.routes.test.ts b/src/routes/health.routes.test.ts index e39166ab..a0a377d5 100644 --- a/src/routes/health.routes.test.ts +++ b/src/routes/health.routes.test.ts @@ -5,6 +5,7 @@ import express, { Request, Response, NextFunction } from 'express'; import healthRouter from './health.routes'; import * as dbConnection from '../services/db/connection.db'; import { connection as redisConnection } from '../services/queueService.server'; +import { errorHandler } from '../middleware/errorHandler'; import fs from 'node:fs/promises'; // 1. Mock the dependencies of the health router. @@ -46,10 +47,7 @@ const mockedFs = fs as Mocked; // 2. Create a minimal Express app to host the router for testing. const app = express(); app.use('/api/health', healthRouter); -app.use((err: Error, req: Request, res: Response, next: NextFunction) => { - const statusCode = (err as any).status || 500; - res.status(statusCode).json({ message: err.message || 'Internal Server Error' }); -}); +app.use(errorHandler); describe('Health Routes (/api/health)', () => { beforeEach(() => { @@ -115,6 +113,7 @@ describe('Health Routes (/api/health)', () => { // Assert expect(response.status).toBe(500); expect(response.body.success).toBe(false); + expect(response.body.success).toBe(false); expect(response.body.message).toBe('Failed to connect to Redis.'); expect(response.body.error).toContain('Unexpected Redis ping response: OK'); }); @@ -171,6 +170,7 @@ describe('Health Routes (/api/health)', () => { const response = await supertest(app).get('/api/health/db-schema'); expect(response.status).toBe(500); + expect(response.body.message).toBe('An error occurred while checking the database schema.'); }); }); @@ -248,5 +248,6 @@ describe('Health Routes (/api/health)', () => { const response = await supertest(app).get('/api/health/db-pool'); expect(response.status).toBe(500); + expect(response.body.message).toBe('An error occurred while checking the database pool status.'); }); }); \ No newline at end of file diff --git a/src/routes/health.routes.ts b/src/routes/health.routes.ts index 9753cfda..af6b682b 100644 --- a/src/routes/health.routes.ts +++ b/src/routes/health.routes.ts @@ -8,7 +8,7 @@ import { getSimpleWeekAndYear } from '../utils/dateUtils'; const router = Router(); -router.get('/ping', (req: Request, res: Response) => { +router.get('/ping', (_req: Request, res: Response) => { res.status(200).send('pong'); }); @@ -21,19 +21,19 @@ router.get('/db-schema', async (req, res) => { return res.status(500).json({ success: false, message: `Database schema check failed. Missing tables: ${missingTables.join(', ')}.` }); } return res.status(200).json({ success: true, message: 'All required database tables exist.' }); - } catch (error) { - logger.error('Error during DB schema check:', { error }); - return res.status(500).json({ success: false, message: 'An error occurred while checking the database schema.' }); + } catch (error: unknown) { + logger.error('Error during DB schema check:', { error: error instanceof Error ? error.message : error }); + return res.status(500).json({ success: false, message: 'An error occurred while checking the database schema.' }); // No change to next(error) as this is a health check, not a standard API route } }); router.get('/storage', async (req, res) => { const storagePath = process.env.STORAGE_PATH || '/var/www/flyer-crawler.projectium.com/assets'; try { - await fs.access(storagePath, fs.constants.W_OK); + await fs.access(storagePath, fs.constants.W_OK); // Use fs.promises return res.status(200).json({ success: true, message: `Storage directory '${storagePath}' is accessible and writable.` }); - } catch (error) { - logger.error(`Storage check failed for path: ${storagePath}`, { error }); + } catch (error: unknown) { + logger.error(`Storage check failed for path: ${storagePath}`, { error: error instanceof Error ? error.message : error }); return res.status(500).json({ success: false, message: `Storage check failed. Ensure the directory '${storagePath}' exists and is writable by the application.` }); } }); @@ -50,9 +50,9 @@ router.get('/db-pool', (req: Request, res: Response) => { logger.warn(`Database pool health check shows high waiting count: ${status.waitingCount}`); return res.status(500).json({ success: false, message: `Pool may be under stress. ${message}` }); } - } catch (error) { - logger.error('Error during DB pool health check:', { error }); - return res.status(500).json({ success: false, message: 'An error occurred while checking the database pool status.' }); + } catch (error: unknown) { + logger.error('Error during DB pool health check:', { error: error instanceof Error ? error.message : error }); + return res.status(500).json({ success: false, message: 'An error occurred while checking the database pool status.' }); // No change to next(error) as this is a health check } }); @@ -75,9 +75,9 @@ router.get('/redis', async (req: Request, res: Response) => { if (reply === 'PONG') { return res.status(200).json({ success: true, message: 'Redis connection is healthy.' }); } - throw new Error(`Unexpected Redis ping response: ${reply}`); - } catch (error) { - const errorMessage = error instanceof Error ? error.message : 'An unknown error occurred.'; + throw new Error(`Unexpected Redis ping response: ${reply}`); // This will be caught below + } catch (error: unknown) { + const errorMessage = error instanceof Error ? error.message : String(error); return res.status(500).json({ success: false, message: 'Failed to connect to Redis.', error: errorMessage }); } }); diff --git a/src/routes/passport.routes.test.ts b/src/routes/passport.routes.test.ts index 0035c7c0..d6ee2491 100644 --- a/src/routes/passport.routes.test.ts +++ b/src/routes/passport.routes.test.ts @@ -62,6 +62,7 @@ vi.mock('../services/db/index.db', () => ({ const mockedDb = db as Mocked; vi.mock('../services/logger.server', () => ({ + // This mock is used by the module under test and can be imported in the test file. logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, })); @@ -87,6 +88,7 @@ vi.mock('passport', () => { // Now, import the passport configuration which will use our mocks import passport, { isAdmin, optionalAuth } from './passport.routes'; +import { logger } from '../services/logger.server'; describe('Passport Configuration', () => { beforeEach(() => { @@ -390,7 +392,6 @@ describe('Passport Configuration', () => { vi.mocked(passport.authenticate).mockImplementation( (_strategy, _options, callback) => () => callback?.(null, false, mockInfo) ); - const { logger } = require('../services/logger.server'); // Act optionalAuth(mockReq, mockRes as Response, mockNext); diff --git a/src/routes/recipe.routes.test.ts b/src/routes/recipe.routes.test.ts index 9ed148c0..01482bb7 100644 --- a/src/routes/recipe.routes.test.ts +++ b/src/routes/recipe.routes.test.ts @@ -3,6 +3,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import supertest from 'supertest'; import express, { Request, Response, NextFunction } from 'express'; import recipeRouter from './recipe.routes'; +import { errorHandler } from '../middleware/errorHandler'; import { createMockRecipe, createMockRecipeComment } from '../tests/utils/mockFactories'; // 1. Mock the Service Layer directly. @@ -31,11 +32,7 @@ const app = express(); app.use(express.json()); // Mount the router under its designated base path app.use('/api/recipes', recipeRouter); - -// Add a generic error handler to catch errors passed via next() -app.use((err: Error, req: Request, res: Response, next: NextFunction) => { - res.status(500).json({ message: err.message || 'Internal Server Error' }); -}); +app.use(errorHandler); describe('Recipe Routes (/api/recipes)', () => { beforeEach(() => { diff --git a/src/routes/stats.routes.test.ts b/src/routes/stats.routes.test.ts index dee0316d..5f24b3b2 100644 --- a/src/routes/stats.routes.test.ts +++ b/src/routes/stats.routes.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import supertest from 'supertest'; import express, { Request, Response, NextFunction } from 'express'; +import { errorHandler } from '../middleware/errorHandler'; import statsRouter from './stats.routes'; // 1. Mock the Service Layer directly. @@ -26,11 +27,7 @@ const app = express(); app.use(express.json()); // Mount the router under its designated base path app.use('/api/stats', statsRouter); - -// Add a generic error handler to catch errors passed via next() -app.use((err: Error, req: Request, res: Response, next: NextFunction) => { - res.status(500).json({ message: err.message || 'Internal Server Error' }); -}); +app.use(errorHandler); describe('Stats Routes (/api/stats)', () => { beforeEach(() => { @@ -67,6 +64,7 @@ describe('Stats Routes (/api/stats)', () => { vi.mocked(db.adminRepo.getMostFrequentSaleItems).mockRejectedValue(new Error('DB Error')); const response = await supertest(app).get('/api/stats/most-frequent-sales'); expect(response.status).toBe(500); + expect(response.body.message).toBe('DB Error'); }); }); }); \ No newline at end of file