diff --git a/src/features/flyer/FlyerDisplay.test.tsx b/src/features/flyer/FlyerDisplay.test.tsx index f1483f12..33f5b050 100644 --- a/src/features/flyer/FlyerDisplay.test.tsx +++ b/src/features/flyer/FlyerDisplay.test.tsx @@ -129,13 +129,15 @@ describe('FlyerDisplay', () => { expect(screen.queryByText(/invalid date/i)).not.toBeInTheDocument(); // Ensure no "Invalid Date" text }); - it('should handle a mix of valid and invalid date strings gracefully', () => { + it('should handle a mix of valid and invalid date strings gracefully (start date valid)', () => { render(); expect(screen.getByText('Deals start October 26, 2023')).toBeInTheDocument(); expect(screen.queryByText(/deals valid from/i)).not.toBeInTheDocument(); expect(screen.queryByText(/deals end/i)).not.toBeInTheDocument(); expect(screen.queryByText(/invalid date/i)).not.toBeInTheDocument(); + }); + it('should handle a mix of valid and invalid date strings gracefully (end date valid)', () => { render(); expect(screen.getByText('Deals end November 1, 2023')).toBeInTheDocument(); expect(screen.queryByText(/deals valid from/i)).not.toBeInTheDocument(); diff --git a/src/features/flyer/FlyerList.test.tsx b/src/features/flyer/FlyerList.test.tsx index 63a8cf15..d4b3584f 100644 --- a/src/features/flyer/FlyerList.test.tsx +++ b/src/features/flyer/FlyerList.test.tsx @@ -163,7 +163,8 @@ describe('FlyerList', () => { expect(tooltipText).toContain('Store: Metro'); expect(tooltipText).toContain('Items: 50'); expect(tooltipText).toContain('Valid: October 5, 2023 to October 11, 2023'); - expect(tooltipText).toContain('Processed: October 1, 2023 at 10:00:00 AM'); + // Use a regex for the processed time to avoid timezone-related flakiness in tests. + expect(tooltipText).toMatch(/Processed: October 1, 2023 at \d{1,2}:\d{2}:\d{2} (AM|PM)/); }); it('should handle invalid dates gracefully in display and tooltip', () => { diff --git a/src/features/flyer/FlyerList.tsx b/src/features/flyer/FlyerList.tsx index c5d8b717..8e02ded1 100644 --- a/src/features/flyer/FlyerList.tsx +++ b/src/features/flyer/FlyerList.tsx @@ -56,9 +56,9 @@ export const FlyerList: React.FC = ({ flyers, onFlyerSelect, sel const dateRange = from && to ? (from === to ? from : `${from} - ${to}`) : from || to; // Build a more detailed tooltip string - const processedDate = isValid(parseISO(flyer.created_at)) ? format(parseISO(flyer.created_at), 'PPPP p') : 'N/A'; - const validFromFull = flyer.valid_from && isValid(parseISO(flyer.valid_from)) ? format(parseISO(flyer.valid_from), 'PPPP') : 'N/A'; - const validToFull = flyer.valid_to && isValid(parseISO(flyer.valid_to)) ? format(parseISO(flyer.valid_to), 'PPPP') : 'N/A'; + const processedDate = isValid(parseISO(flyer.created_at)) ? format(parseISO(flyer.created_at), "MMMM d, yyyy 'at' h:mm:ss a") : 'N/A'; + const validFromFull = flyer.valid_from && isValid(parseISO(flyer.valid_from)) ? format(parseISO(flyer.valid_from), 'MMMM d, yyyy') : 'N/A'; + const validToFull = flyer.valid_to && isValid(parseISO(flyer.valid_to)) ? format(parseISO(flyer.valid_to), 'MMMM d, yyyy') : 'N/A'; const tooltipLines = [ `File: ${flyer.file_name}`, diff --git a/src/features/shopping/ShoppingList.tsx b/src/features/shopping/ShoppingList.tsx index 100626d8..7b5fc2ec 100644 --- a/src/features/shopping/ShoppingList.tsx +++ b/src/features/shopping/ShoppingList.tsx @@ -72,7 +72,7 @@ export const ShoppingListComponent: React.FC = ({ us setIsReadingAloud(true); try { - const listText = "Here is your shopping list: " + neededItems.map(item => item.custom_item_name || item.master_item?.name).join(', '); + const listText = "Here is your shopping list: " + neededItems.map(item => item.custom_item_name || item.master_item?.name).filter(Boolean).join(', '); const response = await generateSpeechFromText(listText); const base64Audio: string = await response.json(); diff --git a/src/features/shopping/WatchedItemsList.test.tsx b/src/features/shopping/WatchedItemsList.test.tsx index 161d821b..2d7bfee1 100644 --- a/src/features/shopping/WatchedItemsList.test.tsx +++ b/src/features/shopping/WatchedItemsList.test.tsx @@ -225,10 +225,19 @@ describe('WatchedItemsList (in shopping feature)', () => { describe('UI Edge Cases', () => { it('should display a specific message when a filter results in no items', () => { - render(); + const { rerender } = render(); const categoryFilter = screen.getByRole('combobox', { name: /filter by category/i }); - fireEvent.change(categoryFilter, { target: { value: 'Beverages' } }); - expect(screen.getByText('No watched items in the "Beverages" category.')).toBeInTheDocument(); + + // Select 'Produce' - 'Apples' should be visible + fireEvent.change(categoryFilter, { target: { value: 'Produce' } }); + expect(screen.getByText('Apples')).toBeInTheDocument(); + + // Rerender with the 'Produce' item removed, but keep the filter active + const itemsWithoutProduce = mockItems.filter(item => item.category_name !== 'Produce'); + rerender(); + + // Now the message should appear for the 'Produce' category + expect(screen.getByText('No watched items in the "Produce" category.')).toBeInTheDocument(); }); it('should hide the sort button if there is only one item', () => { diff --git a/src/features/voice-assistant/VoiceAssistant.test.tsx b/src/features/voice-assistant/VoiceAssistant.test.tsx index 3477d474..4f2a5e74 100644 --- a/src/features/voice-assistant/VoiceAssistant.test.tsx +++ b/src/features/voice-assistant/VoiceAssistant.test.tsx @@ -26,16 +26,18 @@ vi.mock('../../components/icons/XMarkIcon', () => ({ // Mock browser APIs that are not available in JSDOM Object.defineProperty(window, 'AudioContext', { writable: true, - value: vi.fn().mockImplementation(() => ({ - createMediaStreamSource: vi.fn(() => ({ - connect: vi.fn(), - })), - createScriptProcessor: vi.fn(() => ({ - connect: vi.fn(), - disconnect: vi.fn(), - })), - close: vi.fn(), - })), + value: vi.fn().mockImplementation(function () { + return { + createMediaStreamSource: vi.fn(() => ({ + connect: vi.fn(), + })), + createScriptProcessor: vi.fn(() => ({ + connect: vi.fn(), + disconnect: vi.fn(), + })), + close: vi.fn(), + }; + }), }); Object.defineProperty(navigator, 'mediaDevices', { diff --git a/src/pages/ResetPasswordPage.test.tsx b/src/pages/ResetPasswordPage.test.tsx index cb961728..aa621653 100644 --- a/src/pages/ResetPasswordPage.test.tsx +++ b/src/pages/ResetPasswordPage.test.tsx @@ -32,7 +32,6 @@ const renderWithRouter = (token: string) => { describe('ResetPasswordPage', () => { beforeEach(() => { vi.clearAllMocks(); - vi.useFakeTimers(); }); afterEach(() => { @@ -48,6 +47,7 @@ describe('ResetPasswordPage', () => { }); it('should call resetPassword and show success message on valid submission', async () => { + vi.useFakeTimers(); mockedApiClient.resetPassword.mockResolvedValue(new Response(JSON.stringify({ message: 'Password reset was successful!' }))); const token = 'valid-token'; renderWithRouter(token); @@ -71,6 +71,8 @@ describe('ResetPasswordPage', () => { vi.advanceTimersByTime(4000); }); expect(screen.getByText('Home Page')).toBeInTheDocument(); + + vi.useRealTimers(); }); it('should show an error message if passwords do not match', async () => { @@ -113,7 +115,8 @@ describe('ResetPasswordPage', () => { fireEvent.click(screen.getByRole('button', { name: /reset password/i })); // Expect button to be disabled and text to be gone (replaced by spinner) - expect(screen.getByRole('button')).toBeDisabled(); + // We use the accessible name which persists via aria-label even when text content is replaced + expect(screen.getByRole('button', { name: /reset password/i })).toBeDisabled(); expect(screen.queryByText('Reset Password')).not.toBeInTheDocument(); await act(async () => { diff --git a/src/pages/ResetPasswordPage.tsx b/src/pages/ResetPasswordPage.tsx index e6484ce1..b6c8fae2 100644 --- a/src/pages/ResetPasswordPage.tsx +++ b/src/pages/ResetPasswordPage.tsx @@ -94,7 +94,12 @@ export const ResetPasswordPage: React.FC = () => { {error &&

{error}

}
-
diff --git a/src/pages/UserProfilePage.test.tsx b/src/pages/UserProfilePage.test.tsx index e60b070d..e35768ce 100644 --- a/src/pages/UserProfilePage.test.tsx +++ b/src/pages/UserProfilePage.test.tsx @@ -135,7 +135,7 @@ describe('UserProfilePage', () => { }); it('should use email for avatar seed if full_name is missing', async () => { - const profileNoName = { ...mockProfile, full_name: null }; + const profileNoName = { ...mockProfile, full_name: null, avatar_url: null }; mockedApiClient.getAuthenticatedUserProfile.mockResolvedValue(new Response(JSON.stringify(profileNoName))); mockedApiClient.getUserAchievements.mockResolvedValue(new Response(JSON.stringify(mockAchievements))); diff --git a/src/pages/admin/components/ProfileManager.test.tsx b/src/pages/admin/components/ProfileManager.test.tsx index fe4d50ec..e8b44381 100644 --- a/src/pages/admin/components/ProfileManager.test.tsx +++ b/src/pages/admin/components/ProfileManager.test.tsx @@ -288,17 +288,19 @@ describe('ProfileManager', () => { }); it('should automatically geocode address after user stops typing', async () => { - vi.useFakeTimers(); + // Only mock setTimeout/clearTimeout to prevent Date freezing which can hang waitFor + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }); const addressWithoutCoords = { ...mockAddress, latitude: undefined, longitude: undefined }; mockedApiClient.getUserAddress.mockResolvedValue(new Response(JSON.stringify(addressWithoutCoords))); console.log('[TEST LOG] Rendering for automatic geocode test'); render(); + console.log('[TEST LOG] Waiting for initial address load...'); await waitFor(() => expect(screen.getByLabelText(/city/i)).toHaveValue('Anytown')); + console.log('[TEST LOG] Initial address loaded. Changing city...'); // Change address, geocode should not be called immediately - console.log('[TEST LOG] Changing city to NewCity'); fireEvent.change(screen.getByLabelText(/city/i), { target: { value: 'NewCity' } }); expect(mockedApiClient.geocodeAddress).not.toHaveBeenCalled(); @@ -316,9 +318,11 @@ describe('ProfileManager', () => { }); it('should not geocode if address already has coordinates', async () => { - vi.useFakeTimers(); + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }); render(); + console.log('[TEST LOG] Waiting for initial address load (no geocode test)...'); await waitFor(() => expect(screen.getByLabelText(/city/i)).toHaveValue('Anytown')); + console.log('[TEST LOG] Initial address loaded.'); console.log('[TEST LOG] Advancing timers for "no geocode" test...'); // Advance timers @@ -510,7 +514,7 @@ describe('ProfileManager', () => { }); it('should handle account deletion flow', async () => { - vi.useFakeTimers(); + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }); const { unmount } = render(); console.log('[TEST LOG] Deletion flow: clicking data privacy tab'); fireEvent.click(screen.getByRole('button', { name: /data & privacy/i })); @@ -539,7 +543,8 @@ describe('ProfileManager', () => { await act(async () => { await vi.advanceTimersByTimeAsync(3500); }); - + console.log('[TEST LOG] Timers advanced. Checking for sign out...'); + expect(mockOnClose).toHaveBeenCalled(); expect(mockOnSignOut).toHaveBeenCalled(); diff --git a/src/routes/admin.content.routes.test.ts b/src/routes/admin.content.routes.test.ts index 7b02224c..d4462ced 100644 --- a/src/routes/admin.content.routes.test.ts +++ b/src/routes/admin.content.routes.test.ts @@ -218,6 +218,20 @@ describe('Admin Content Management Routes (/api/admin)', () => { }); describe('Recipe and Comment Routes', () => { + it('DELETE /recipes/:recipeId should delete a recipe', async () => { + const recipeId = 300; + vi.mocked(mockedDb.recipeRepo.deleteRecipe).mockResolvedValue(undefined); + + const response = await supertest(app).delete(`/api/admin/recipes/${recipeId}`); + expect(response.status).toBe(204); + expect(vi.mocked(mockedDb.recipeRepo.deleteRecipe)).toHaveBeenCalledWith(recipeId, expect.anything(), true, expect.anything()); + }); + + it('DELETE /recipes/:recipeId should return 400 for invalid ID', async () => { + const response = await supertest(app).delete('/api/admin/recipes/abc'); + expect(response.status).toBe(400); + }); + it('PUT /recipes/:id/status should update a recipe status', async () => { const recipeId = 201; const requestBody = { status: 'public' as const }; diff --git a/src/routes/admin.jobs.routes.test.ts b/src/routes/admin.jobs.routes.test.ts index bf7f56fe..b61c27f3 100644 --- a/src/routes/admin.jobs.routes.test.ts +++ b/src/routes/admin.jobs.routes.test.ts @@ -65,7 +65,7 @@ vi.mock('@bull-board/express', () => ({ // Import the mocked modules to control them import { backgroundJobService } from '../services/backgroundJobService'; // This is now a mock -import { flyerQueue, analyticsQueue, cleanupQueue } from '../services/queueService.server'; +import { flyerQueue, analyticsQueue, cleanupQueue, weeklyAnalyticsQueue } from '../services/queueService.server'; // Mock the logger vi.mock('../services/logger.server', () => ({ @@ -137,6 +137,44 @@ describe('Admin Job Trigger Routes (/api/admin/trigger)', () => { }); }); + describe('POST /trigger/analytics-report', () => { + it('should trigger the analytics report job and return 202 Accepted', async () => { + const mockJob = { id: 'manual-report-job-123' } as Job; + vi.mocked(analyticsQueue.add).mockResolvedValue(mockJob); + + const response = await supertest(app).post('/api/admin/trigger/analytics-report'); + + expect(response.status).toBe(202); + expect(response.body.message).toContain('Analytics report generation job has been enqueued'); + expect(analyticsQueue.add).toHaveBeenCalledWith('generate-daily-report', expect.objectContaining({ reportDate: expect.any(String) }), expect.any(Object)); + }); + + it('should return 500 if enqueuing the analytics job fails', async () => { + vi.mocked(analyticsQueue.add).mockRejectedValue(new Error('Queue error')); + const response = await supertest(app).post('/api/admin/trigger/analytics-report'); + expect(response.status).toBe(500); + }); + }); + + describe('POST /trigger/weekly-analytics', () => { + it('should trigger the weekly analytics job and return 202 Accepted', async () => { + const mockJob = { id: 'manual-weekly-report-job-123' } as Job; + vi.mocked(weeklyAnalyticsQueue.add).mockResolvedValue(mockJob); + + const response = await supertest(app).post('/api/admin/trigger/weekly-analytics'); + + expect(response.status).toBe(202); + expect(response.body.message).toContain('Successfully enqueued weekly analytics job'); + expect(weeklyAnalyticsQueue.add).toHaveBeenCalledWith('generate-weekly-report', expect.objectContaining({ reportYear: expect.any(Number), reportWeek: expect.any(Number) }), expect.any(Object)); + }); + + it('should return 500 if enqueuing the weekly analytics job fails', async () => { + vi.mocked(weeklyAnalyticsQueue.add).mockRejectedValue(new Error('Queue error')); + const response = await supertest(app).post('/api/admin/trigger/weekly-analytics'); + expect(response.status).toBe(500); + }); + }); + describe('POST /flyers/:flyerId/cleanup', () => { it('should enqueue a cleanup job for a valid flyer ID', async () => { const flyerId = 789; diff --git a/src/routes/ai.routes.test.ts b/src/routes/ai.routes.test.ts index 6f4fb782..ae1d0834 100644 --- a/src/routes/ai.routes.test.ts +++ b/src/routes/ai.routes.test.ts @@ -523,8 +523,27 @@ describe('AI Routes (/api/ai)', () => { expect(response.body.message).toBe('Speech generation is not yet implemented.'); }); - it('POST /plan-trip should return 500 if the AI service fails', async () => { - vi.mocked(aiService.aiService.planTripWithMaps).mockRejectedValue(new Error('Maps API key invalid')); + it('POST /search-web should return the stubbed response', async () => { + const response = await supertest(app) + .post('/api/ai/search-web') + .send({ query: 'test query' }); + + expect(response.status).toBe(200); + expect(response.body.text).toContain('The web says this is good'); + }); + + it('POST /compare-prices should return the stubbed response', async () => { + const response = await supertest(app) + .post('/api/ai/compare-prices') + .send({ items: [{ name: 'Milk' }] }); + + expect(response.status).toBe(200); + expect(response.body.text).toContain('server-generated price comparison'); + }); + + it('POST /plan-trip should return result on success', async () => { + const mockResult = { text: 'Trip plan', sources: [] }; + vi.mocked(aiService.aiService.planTripWithMaps).mockResolvedValue(mockResult); const response = await supertest(app) .post('/api/ai/plan-trip') @@ -534,8 +553,8 @@ describe('AI Routes (/api/ai)', () => { userLocation: { latitude: 0, longitude: 0 }, }); - expect(response.status).toBe(500); - expect(response.body.message).toBe('Maps API key invalid'); + expect(response.status).toBe(200); + expect(response.body).toEqual(mockResult); }); it('POST /plan-trip should return 500 if the AI service fails', async () => { @@ -552,5 +571,25 @@ describe('AI Routes (/api/ai)', () => { expect(response.status).toBe(500); expect(response.body.message).toBe('Maps API key invalid'); }); + + it('POST /quick-insights should return 400 if items are missing', async () => { + const response = await supertest(app).post('/api/ai/quick-insights').send({}); + expect(response.status).toBe(400); + }); + + it('POST /search-web should return 400 if query is missing', async () => { + const response = await supertest(app).post('/api/ai/search-web').send({}); + expect(response.status).toBe(400); + }); + + it('POST /compare-prices should return 400 if items are missing', async () => { + const response = await supertest(app).post('/api/ai/compare-prices').send({}); + expect(response.status).toBe(400); + }); + + it('POST /plan-trip should return 400 if required fields are missing', async () => { + const response = await supertest(app).post('/api/ai/plan-trip').send({ items: [] }); + expect(response.status).toBe(400); + }); }); }); \ No newline at end of file diff --git a/src/routes/auth.routes.test.ts b/src/routes/auth.routes.test.ts index e6f2fad2..238cda7f 100644 --- a/src/routes/auth.routes.test.ts +++ b/src/routes/auth.routes.test.ts @@ -185,6 +185,24 @@ describe('Auth Routes (/api/auth)', () => { expect(db.userRepo.createUser).toHaveBeenCalled(); }); + it('should set a refresh token cookie on successful registration', async () => { + const mockNewUser = createMockUserProfile({ user_id: 'new-user-id', user: { user_id: 'new-user-id', email: 'cookie@test.com' } }); + vi.mocked(db.userRepo.createUser).mockResolvedValue(mockNewUser); + vi.mocked(db.userRepo.saveRefreshToken).mockResolvedValue(undefined); + vi.mocked(db.adminRepo.logActivity).mockResolvedValue(undefined); + + const response = await supertest(app) + .post('/api/auth/register') + .send({ + email: 'cookie@test.com', + password: 'StrongPassword123!', + }); + + expect(response.status).toBe(201); + expect(response.headers['set-cookie']).toBeDefined(); + expect(response.headers['set-cookie'][0]).toContain('refreshToken='); + }); + it('should reject registration with a weak password', async () => { const weakPassword = 'password'; @@ -444,6 +462,19 @@ describe('Auth Routes (/api/auth)', () => { expect(response.body.message).toBe('Invalid or expired password reset token.'); }); + it('should reject if token does not match any valid tokens in DB', async () => { + const tokenRecord = { user_id: 'user-123', token_hash: 'hashed-token', expires_at: new Date(Date.now() + 3600000) }; + vi.mocked(db.userRepo.getValidResetTokens).mockResolvedValue([tokenRecord]); + vi.mocked(bcrypt.compare).mockResolvedValue(false as never); // Token does not match + + const response = await supertest(app) + .post('/api/auth/reset-password') + .send({ token: 'wrong-token', newPassword: 'a-Very-Strong-Password-123!' }); + + expect(response.status).toBe(400); + expect(response.body.message).toBe('Invalid or expired password reset token.'); + }); + it('should return 400 for a weak new password', async () => { const tokenRecord = { user_id: 'user-123', token_hash: 'hashed-token', expires_at: new Date(Date.now() + 3600000) }; vi.mocked(db.userRepo.getValidResetTokens).mockResolvedValue([tokenRecord]); diff --git a/src/routes/budget.routes.test.ts b/src/routes/budget.routes.test.ts index 80bf4f93..fd788699 100644 --- a/src/routes/budget.routes.test.ts +++ b/src/routes/budget.routes.test.ts @@ -167,6 +167,12 @@ describe('Budget Routes (/api/budgets)', () => { expect(response.status).toBe(400); expect(response.body.errors[0].message).toBe('At least one field to update must be provided.'); }); + + it('should return 400 for an invalid budget ID', async () => { + const response = await supertest(app).put('/api/budgets/abc').send({ amount_cents: 5000 }); + expect(response.status).toBe(400); + expect(response.body.errors[0].message).toMatch(/Invalid ID|number/i); + }); }); describe('DELETE /:id', () => { @@ -193,6 +199,12 @@ describe('Budget Routes (/api/budgets)', () => { expect(response.status).toBe(500); // The custom handler will now be used expect(response.body.message).toBe('DB Error'); }); + + it('should return 400 for an invalid budget ID', async () => { + const response = await supertest(app).delete('/api/budgets/abc'); + expect(response.status).toBe(400); + expect(response.body.errors[0].message).toMatch(/Invalid ID|number/i); + }); }); describe('GET /spending-analysis', () => { @@ -222,5 +234,12 @@ describe('Budget Routes (/api/budgets)', () => { expect(response.status).toBe(400); expect(response.body.errors).toHaveLength(2); }); + + it('should return 400 if required query parameters are missing', async () => { + const response = await supertest(app).get('/api/budgets/spending-analysis'); + expect(response.status).toBe(400); + // Expect errors for both startDate and endDate + expect(response.body.errors).toHaveLength(2); + }); }); }); \ No newline at end of file diff --git a/src/routes/deals.routes.test.ts b/src/routes/deals.routes.test.ts index 8708a36f..d386d3d5 100644 --- a/src/routes/deals.routes.test.ts +++ b/src/routes/deals.routes.test.ts @@ -73,14 +73,17 @@ describe('Deals Routes (/api/users/deals)', () => { expect(response.status).toBe(200); expect(response.body).toEqual(mockDeals); expect(dealsRepo.findBestPricesForWatchedItems).toHaveBeenCalledWith(mockUser.user_id, expectLogger); + expect(mockLogger.info).toHaveBeenCalledWith({ dealCount: 1 }, 'Successfully fetched best watched item deals.'); }); it('should return 500 if the database call fails', async () => { - vi.mocked(dealsRepo.findBestPricesForWatchedItems).mockRejectedValue(new Error('DB Error')); + const dbError = new Error('DB Error'); + vi.mocked(dealsRepo.findBestPricesForWatchedItems).mockRejectedValue(dbError); const response = await supertest(authenticatedApp).get('/api/users/deals/best-watched-prices'); expect(response.status).toBe(500); expect(response.body.message).toBe('DB Error'); + expect(mockLogger.error).toHaveBeenCalledWith({ error: dbError }, 'Error fetching best watched item deals.'); }); }); }); \ No newline at end of file diff --git a/src/routes/flyer.routes.test.ts b/src/routes/flyer.routes.test.ts index 0b014e3d..0fb5a955 100644 --- a/src/routes/flyer.routes.test.ts +++ b/src/routes/flyer.routes.test.ts @@ -65,10 +65,12 @@ describe('Flyer Routes (/api/flyers)', () => { }); it('should return 500 if the database call fails', async () => { - vi.mocked(db.flyerRepo.getFlyers).mockRejectedValue(new Error('DB Error')); + const dbError = new Error('DB Error'); + vi.mocked(db.flyerRepo.getFlyers).mockRejectedValue(dbError); const response = await supertest(app).get('/api/flyers'); expect(response.status).toBe(500); expect(response.body.message).toBe('DB Error'); + expect(mockLogger.error).toHaveBeenCalledWith({ error: dbError }, 'Error fetching flyers in /api/flyers:'); }); it('should return 400 for invalid query parameters', async () => { @@ -110,10 +112,12 @@ describe('Flyer Routes (/api/flyers)', () => { }); it('should return 500 if the database call fails', async () => { - vi.mocked(db.flyerRepo.getFlyerById).mockRejectedValue(new Error('DB Error')); + const dbError = new Error('DB Error'); + vi.mocked(db.flyerRepo.getFlyerById).mockRejectedValue(dbError); const response = await supertest(app).get('/api/flyers/123'); expect(response.status).toBe(500); expect(response.body.message).toBe('DB Error'); + expect(mockLogger.error).toHaveBeenCalledWith({ error: dbError, flyerId: 123 }, 'Error fetching flyer by ID:'); }); }); @@ -135,10 +139,12 @@ describe('Flyer Routes (/api/flyers)', () => { }); it('should return 500 if the database call fails', async () => { - vi.mocked(db.flyerRepo.getFlyerItems).mockRejectedValue(new Error('DB Error')); + const dbError = new Error('DB Error'); + vi.mocked(db.flyerRepo.getFlyerItems).mockRejectedValue(dbError); const response = await supertest(app).get('/api/flyers/123/items'); expect(response.status).toBe(500); expect(response.body.message).toBe('DB Error'); + expect(mockLogger.error).toHaveBeenCalledWith({ error: dbError }, 'Error fetching flyer items in /api/flyers/:id/items:'); }); }); diff --git a/src/routes/gamification.routes.test.ts b/src/routes/gamification.routes.test.ts index eebc9999..57f892d5 100644 --- a/src/routes/gamification.routes.test.ts +++ b/src/routes/gamification.routes.test.ts @@ -229,6 +229,17 @@ describe('Gamification Routes (/api/achievements)', () => { expect(db.gamificationRepo.getLeaderboard).toHaveBeenCalledWith(5, expect.anything()); }); + it('should use the default limit of 10 when no limit is provided', async () => { + const mockLeaderboard = [createMockLeaderboardUser({ user_id: 'user-1', full_name: 'Leader', points: 1000, rank: '1' })]; + vi.mocked(db.gamificationRepo.getLeaderboard).mockResolvedValue(mockLeaderboard); + + const response = await supertest(unauthenticatedApp).get('/api/achievements/leaderboard'); + + expect(response.status).toBe(200); + expect(response.body).toEqual(mockLeaderboard); + expect(db.gamificationRepo.getLeaderboard).toHaveBeenCalledWith(10, expect.anything()); + }); + it('should return 500 if the database call fails', async () => { vi.mocked(db.gamificationRepo.getLeaderboard).mockRejectedValue(new Error('DB Error')); const response = await supertest(unauthenticatedApp).get('/api/achievements/leaderboard'); diff --git a/src/routes/health.routes.test.ts b/src/routes/health.routes.test.ts index 524cfdb2..bfa31443 100644 --- a/src/routes/health.routes.test.ts +++ b/src/routes/health.routes.test.ts @@ -35,6 +35,7 @@ vi.mock('../services/logger.server', () => ({ debug: vi.fn(), error: vi.fn(), warn: vi.fn(), + child: vi.fn().mockReturnThis(), // Add child mock for req.log }, })); @@ -43,9 +44,17 @@ const mockedRedisConnection = redisConnection as Mocked; const mockedDbConnection = dbConnection as Mocked; const mockedFs = fs as Mocked; +const { logger } = await import('../services/logger.server'); + // 2. Create a minimal Express app to host the router for testing. const app = createTestApp({ router: healthRouter, basePath: '/api/health' }); +// Add a basic error handler to capture errors passed to next(err) and return JSON. +// This prevents unhandled error crashes in tests and ensures we get the 500 response we expect. +app.use((err: any, req: any, res: any, next: any) => { + res.status(err.status || 500).json({ message: err.message, errors: err.errors }); +}); + describe('Health Routes (/api/health)', () => { beforeEach(() => { // Clear mock history before each test to ensure isolation. @@ -149,6 +158,7 @@ describe('Health Routes (/api/health)', () => { expect(response.status).toBe(500); expect(response.body.message).toContain('Missing tables: missing_table_1, missing_table_2'); + // The error is passed to next(), so the global error handler would log it, not the route handler itself. }); it('should return 500 if the database check fails', async () => { @@ -160,6 +170,7 @@ describe('Health Routes (/api/health)', () => { expect(response.status).toBe(500); expect(response.body.message).toBe('DB connection failed'); + expect(logger.error).toHaveBeenCalledWith({ error: 'DB connection failed' }, 'Error during DB schema check:'); }); }); @@ -188,6 +199,7 @@ describe('Health Routes (/api/health)', () => { // Assert expect(response.status).toBe(500); expect(response.body.message).toContain('Storage check failed.'); + expect(logger.error).toHaveBeenCalledWith({ error: 'EACCES: permission denied' }, expect.stringContaining('Storage check failed for path:')); }); }); @@ -225,6 +237,7 @@ describe('Health Routes (/api/health)', () => { expect(response.body.success).toBe(false); expect(response.body.message).toContain('Pool may be under stress.'); expect(response.body.message).toContain('Pool Status: 20 total, 5 idle, 15 waiting.'); + expect(logger.warn).toHaveBeenCalledWith('Database pool health check shows high waiting count: 15'); }); }); @@ -237,5 +250,6 @@ describe('Health Routes (/api/health)', () => { expect(response.status).toBe(500); expect(response.body.message).toBe('Pool is not initialized'); + expect(logger.error).toHaveBeenCalledWith({ error: 'Pool is not initialized' }, 'Error during DB pool health check:'); }); }); \ No newline at end of file diff --git a/src/routes/passport.routes.test.ts b/src/routes/passport.routes.test.ts index d819588e..a173d97e 100644 --- a/src/routes/passport.routes.test.ts +++ b/src/routes/passport.routes.test.ts @@ -88,7 +88,7 @@ vi.mock('passport', () => { }); // Now, import the passport configuration which will use our mocks -import passport, { isAdmin, optionalAuth } from './passport.routes'; +import passport, { isAdmin, optionalAuth, mockAuth } from './passport.routes'; import { logger } from '../services/logger.server'; describe('Passport Configuration', () => { @@ -426,6 +426,24 @@ describe('Passport Configuration', () => { expect(mockNext).toHaveBeenCalledTimes(1); }); + it('should log info and call next() if authentication provides an info Error object', () => { + // Arrange + const mockReq = {} as Request; + const mockInfoError = new Error('Token is malformed'); + // Mock passport.authenticate to call its callback with an info object + vi.mocked(passport.authenticate).mockImplementation( + (_strategy, _options, callback) => () => callback?.(null, false, mockInfoError) + ); + + // Act + optionalAuth(mockReq, mockRes as Response, mockNext); + + // Assert + // info.message is 'Token is malformed' + expect(logger.info).toHaveBeenCalledWith({ info: 'Token is malformed' }, 'Optional auth info:'); + expect(mockNext).toHaveBeenCalledTimes(1); + }); + it('should call next() and not populate user if passport returns an error', () => { // Arrange const mockReq = {} as Request; @@ -444,62 +462,44 @@ describe('Passport Configuration', () => { }); }); - // ... (Keep other describe blocks: LocalStrategy, isAdmin Middleware, optionalAuth Middleware) - // I'm omitting them here for brevity as they didn't have specific failures related to the hoisting issue, - // but they should be preserved in the final file. - describe('isAdmin Middleware', () => { + describe('mockAuth Middleware', () => { const mockNext: NextFunction = vi.fn(); let mockRes: Partial; + let originalNodeEnv: string | undefined; beforeEach(() => { - mockRes = { - status: vi.fn().mockReturnThis(), - json: vi.fn(), - }; + mockRes = { status: vi.fn().mockReturnThis(), json: vi.fn() }; + originalNodeEnv = process.env.NODE_ENV; }); - it('should call next() if user has "admin" role', () => { - const mockReq: Partial = { - user: { - user_id: 'admin-id', - role: 'admin', - points: 100, - user: { user_id: 'admin-id', email: 'admin@test.com' } - } - }; - isAdmin(mockReq as Request, mockRes as Response, mockNext); + afterEach(() => { + process.env.NODE_ENV = originalNodeEnv; + }); + + it('should attach a mock admin user to req when NODE_ENV is "test"', () => { + // Arrange + process.env.NODE_ENV = 'test'; + const mockReq = {} as Request; + + // Act + mockAuth(mockReq, mockRes as Response, mockNext); + + // Assert + expect(mockReq.user).toBeDefined(); + expect((mockReq.user as UserProfile).role).toBe('admin'); expect(mockNext).toHaveBeenCalledTimes(1); }); - it('should return 403 Forbidden if user does not have "admin" role', () => { - const mockReq: Partial = { - user: { - user_id: 'user-id', - role: 'user', - points: 50, - user: { user_id: 'user-id', email: 'user@test.com' } - } - }; - isAdmin(mockReq as Request, mockRes as Response, mockNext); - expect(mockRes.status).toHaveBeenCalledWith(403); - }); - }); - - describe('optionalAuth Middleware', () => { - const mockNext: NextFunction = vi.fn(); - let mockRes: Partial; - beforeEach(() => { - mockRes = { status: vi.fn().mockReturnThis(), json: vi.fn() }; - }); - - it('should populate req.user and call next() if authentication succeeds', () => { + it('should do nothing and call next() when NODE_ENV is not "test"', () => { + // Arrange + process.env.NODE_ENV = 'production'; const mockReq = {} as Request; - const mockUser = createMockUserProfile({ user_id: 'user-123' }); - vi.mocked(passport.authenticate).mockImplementation( - (_strategy, _options, callback) => () => callback?.(null, mockUser, undefined) - ); - optionalAuth(mockReq, mockRes as Response, mockNext); - expect(mockReq.user).toEqual(mockUser); + + // Act + mockAuth(mockReq, mockRes as Response, mockNext); + + // Assert + expect(mockReq.user).toBeUndefined(); expect(mockNext).toHaveBeenCalledTimes(1); }); }); diff --git a/src/routes/personalization.routes.test.ts b/src/routes/personalization.routes.test.ts index efacb955..3fd22ef2 100644 --- a/src/routes/personalization.routes.test.ts +++ b/src/routes/personalization.routes.test.ts @@ -48,17 +48,12 @@ describe('Personalization Routes (/api/personalization)', () => { }); it('should return 500 if the database call fails', async () => { - vi.mocked(db.personalizationRepo.getAllMasterItems).mockRejectedValue(new Error('DB Error')); + const dbError = new Error('DB Error'); + vi.mocked(db.personalizationRepo.getAllMasterItems).mockRejectedValue(dbError); const response = await supertest(app).get('/api/personalization/master-items'); expect(response.status).toBe(500); expect(response.body.message).toBe('DB Error'); - }); - - it('should return 500 if the database call fails for dietary restrictions', async () => { - vi.mocked(db.personalizationRepo.getDietaryRestrictions).mockRejectedValue(new Error('DB Error')); - const response = await supertest(app).get('/api/personalization/dietary-restrictions'); - expect(response.status).toBe(500); - expect(response.body.message).toBe('DB Error'); + expect(mockLogger.error).toHaveBeenCalledWith({ error: dbError }, 'Error fetching master items in /api/personalization/master-items:'); }); }); @@ -74,10 +69,12 @@ describe('Personalization Routes (/api/personalization)', () => { }); it('should return 500 if the database call fails', async () => { - vi.mocked(db.personalizationRepo.getDietaryRestrictions).mockRejectedValue(new Error('DB Error')); + const dbError = new Error('DB Error'); + vi.mocked(db.personalizationRepo.getDietaryRestrictions).mockRejectedValue(dbError); const response = await supertest(app).get('/api/personalization/dietary-restrictions'); expect(response.status).toBe(500); expect(response.body.message).toBe('DB Error'); + expect(mockLogger.error).toHaveBeenCalledWith({ error: dbError }, 'Error fetching dietary restrictions in /api/personalization/dietary-restrictions:'); }); }); @@ -93,10 +90,12 @@ describe('Personalization Routes (/api/personalization)', () => { }); it('should return 500 if the database call fails', async () => { - vi.mocked(db.personalizationRepo.getAppliances).mockRejectedValue(new Error('DB Error')); + const dbError = new Error('DB Error'); + vi.mocked(db.personalizationRepo.getAppliances).mockRejectedValue(dbError); const response = await supertest(app).get('/api/personalization/appliances'); expect(response.status).toBe(500); expect(response.body.message).toBe('DB Error'); + expect(mockLogger.error).toHaveBeenCalledWith({ error: dbError }, 'Error fetching appliances in /api/personalization/appliances:'); }); }); }); \ No newline at end of file diff --git a/src/routes/price.routes.test.ts b/src/routes/price.routes.test.ts index 2d81e27c..8f06a766 100644 --- a/src/routes/price.routes.test.ts +++ b/src/routes/price.routes.test.ts @@ -9,9 +9,16 @@ vi.mock('../services/logger.server', () => ({ logger: { info: vi.fn(), error: vi.fn(), + // The test app setup injects a child logger into `req.log`. + // We need to mock `child()` to return the mock logger itself + // so that `req.log.info()` calls `logger.info()`. + child: vi.fn().mockReturnThis(), }, })); +// Import the mocked logger to make assertions on it +import { logger } from '../services/logger.server'; + describe('Price Routes (/api/price-history)', () => { const app = createTestApp({ router: priceRouter, basePath: '/api/price-history' }); beforeEach(() => { @@ -20,12 +27,17 @@ describe('Price Routes (/api/price-history)', () => { describe('POST /', () => { it('should return 200 OK with an empty array for a valid request', async () => { + const masterItemIds = [1, 2, 3]; const response = await supertest(app) .post('/api/price-history') - .send({ masterItemIds: [1, 2, 3] }); + .send({ masterItemIds }); expect(response.status).toBe(200); expect(response.body).toEqual([]); + expect(logger.info).toHaveBeenCalledWith( + { itemCount: masterItemIds.length }, + '[API /price-history] Received request for historical price data.' + ); }); it('should return 400 if masterItemIds is not an array', async () => { diff --git a/src/routes/recipe.routes.test.ts b/src/routes/recipe.routes.test.ts index 9207851b..db4e2784 100644 --- a/src/routes/recipe.routes.test.ts +++ b/src/routes/recipe.routes.test.ts @@ -63,10 +63,12 @@ describe('Recipe Routes (/api/recipes)', () => { }); it('should return 500 if the database call fails', async () => { - vi.mocked(db.recipeRepo.getRecipesBySalePercentage).mockRejectedValue(new Error('DB Error')); + const dbError = new Error('DB Error'); + vi.mocked(db.recipeRepo.getRecipesBySalePercentage).mockRejectedValue(dbError); const response = await supertest(app).get('/api/recipes/by-sale-percentage'); expect(response.status).toBe(500); expect(response.body.message).toBe('DB Error'); + expect(mockLogger.error).toHaveBeenCalledWith({ error: dbError }, 'Error fetching recipes in /api/recipes/by-sale-percentage:'); }); it('should return 400 for an invalid minPercentage', async () => { @@ -91,10 +93,12 @@ describe('Recipe Routes (/api/recipes)', () => { }); it('should return 500 if the database call fails', async () => { - vi.mocked(db.recipeRepo.getRecipesByMinSaleIngredients).mockRejectedValue(new Error('DB Error')); + const dbError = new Error('DB Error'); + vi.mocked(db.recipeRepo.getRecipesByMinSaleIngredients).mockRejectedValue(dbError); const response = await supertest(app).get('/api/recipes/by-sale-ingredients'); expect(response.status).toBe(500); expect(response.body.message).toBe('DB Error'); + expect(mockLogger.error).toHaveBeenCalledWith({ error: dbError }, 'Error fetching recipes in /api/recipes/by-sale-ingredients:'); }); it('should return 400 for an invalid minIngredients', async () => { @@ -116,11 +120,13 @@ describe('Recipe Routes (/api/recipes)', () => { }); it('should return 500 if the database call fails', async () => { - vi.mocked(db.recipeRepo.findRecipesByIngredientAndTag).mockRejectedValue(new Error('DB Error')); + const dbError = new Error('DB Error'); + vi.mocked(db.recipeRepo.findRecipesByIngredientAndTag).mockRejectedValue(dbError); const response = await supertest(app) .get('/api/recipes/by-ingredient-and-tag?ingredient=chicken&tag=quick'); expect(response.status).toBe(500); expect(response.body.message).toBe('DB Error'); + expect(mockLogger.error).toHaveBeenCalledWith({ error: dbError }, 'Error fetching recipes in /api/recipes/by-ingredient-and-tag:'); }); it('should return 400 if required query parameters are missing', async () => { @@ -149,10 +155,12 @@ describe('Recipe Routes (/api/recipes)', () => { }); it('should return 500 if the database call fails', async () => { - vi.mocked(db.recipeRepo.getRecipeComments).mockRejectedValue(new Error('DB Error')); + const dbError = new Error('DB Error'); + vi.mocked(db.recipeRepo.getRecipeComments).mockRejectedValue(dbError); const response = await supertest(app).get('/api/recipes/1/comments'); expect(response.status).toBe(500); expect(response.body.message).toBe('DB Error'); + expect(mockLogger.error).toHaveBeenCalledWith({ error: dbError }, `Error fetching comments for recipe ID 1:`); }); it('should return 400 for an invalid recipeId', async () => { @@ -175,17 +183,21 @@ describe('Recipe Routes (/api/recipes)', () => { }); it('should return 404 if the recipe is not found', async () => { - vi.mocked(db.recipeRepo.getRecipeById).mockRejectedValue(new NotFoundError('Recipe not found')); + const notFoundError = new NotFoundError('Recipe not found'); + vi.mocked(db.recipeRepo.getRecipeById).mockRejectedValue(notFoundError); const response = await supertest(app).get('/api/recipes/999'); expect(response.status).toBe(404); expect(response.body.message).toContain('not found'); + expect(mockLogger.error).toHaveBeenCalledWith({ error: notFoundError }, `Error fetching recipe ID 999:`); }); it('should return 500 if the database call fails', async () => { - vi.mocked(db.recipeRepo.getRecipeById).mockRejectedValue(new Error('DB Error')); + const dbError = new Error('DB Error'); + vi.mocked(db.recipeRepo.getRecipeById).mockRejectedValue(dbError); const response = await supertest(app).get('/api/recipes/456'); expect(response.status).toBe(500); expect(response.body.message).toBe('DB Error'); + expect(mockLogger.error).toHaveBeenCalledWith({ error: dbError }, `Error fetching recipe ID 456:`); }); it('should return 400 for an invalid recipeId', async () => { diff --git a/src/routes/recipe.routes.ts b/src/routes/recipe.routes.ts index 9cce1eb2..6360b262 100644 --- a/src/routes/recipe.routes.ts +++ b/src/routes/recipe.routes.ts @@ -43,8 +43,7 @@ type BySalePercentageRequest = z.infer; router.get('/by-sale-percentage', validateRequest(bySalePercentageSchema), async (req, res, next) => { try { const { query } = req as unknown as BySalePercentageRequest; - const minPercentage = query.minPercentage !== undefined ? Number(query.minPercentage) : 50; - const recipes = await db.recipeRepo.getRecipesBySalePercentage(minPercentage, req.log); + const recipes = await db.recipeRepo.getRecipesBySalePercentage(query.minPercentage, req.log); res.json(recipes); } catch (error) { req.log.error({ error }, 'Error fetching recipes in /api/recipes/by-sale-percentage:'); @@ -59,8 +58,7 @@ type BySaleIngredientsRequest = z.infer; router.get('/by-sale-ingredients', validateRequest(bySaleIngredientsSchema), async (req, res, next) => { try { const { query } = req as unknown as BySaleIngredientsRequest; - const minIngredients = query.minIngredients !== undefined ? Number(query.minIngredients) : 3; - const recipes = await db.recipeRepo.getRecipesByMinSaleIngredients(minIngredients, req.log); + const recipes = await db.recipeRepo.getRecipesByMinSaleIngredients(query.minIngredients, req.log); res.json(recipes); } catch (error) { req.log.error({ error }, 'Error fetching recipes in /api/recipes/by-sale-ingredients:'); @@ -90,8 +88,7 @@ type RecipeIdRequest = z.infer; router.get('/:recipeId/comments', validateRequest(recipeIdParamsSchema), async (req, res, next) => { try { const { params } = req as unknown as RecipeIdRequest; - const recipeId = Number(params.recipeId); - const comments = await db.recipeRepo.getRecipeComments(recipeId, req.log); + const comments = await db.recipeRepo.getRecipeComments(params.recipeId, req.log); res.json(comments); } catch (error) { req.log.error({ error }, `Error fetching comments for recipe ID ${req.params.recipeId}:`); @@ -105,8 +102,7 @@ router.get('/:recipeId/comments', validateRequest(recipeIdParamsSchema), async ( router.get('/:recipeId', validateRequest(recipeIdParamsSchema), async (req, res, next) => { try { const { params } = req as unknown as RecipeIdRequest; - const recipeId = Number(params.recipeId); - const recipe = await db.recipeRepo.getRecipeById(recipeId, req.log); + const recipe = await db.recipeRepo.getRecipeById(params.recipeId, req.log); res.json(recipe); } catch (error) { req.log.error({ error }, `Error fetching recipe ID ${req.params.recipeId}:`); diff --git a/src/routes/stats.routes.test.ts b/src/routes/stats.routes.test.ts index 9d9ad9c6..fa3f08d5 100644 --- a/src/routes/stats.routes.test.ts +++ b/src/routes/stats.routes.test.ts @@ -53,10 +53,12 @@ describe('Stats Routes (/api/stats)', () => { }); it('should return 500 if the database call fails', async () => { - vi.mocked(db.adminRepo.getMostFrequentSaleItems).mockRejectedValue(new Error('DB Error')); + const dbError = new Error('DB Error'); + vi.mocked(db.adminRepo.getMostFrequentSaleItems).mockRejectedValue(dbError); const response = await supertest(app).get('/api/stats/most-frequent-sales'); expect(response.status).toBe(500); expect(response.body.message).toBe('DB Error'); + expect(mockLogger.error).toHaveBeenCalledWith({ error: dbError }, 'Error fetching most frequent sale items in /api/stats/most-frequent-sales:'); }); it('should return 400 for invalid query parameters', async () => { diff --git a/src/routes/stats.routes.ts b/src/routes/stats.routes.ts index 9765a903..724d0296 100644 --- a/src/routes/stats.routes.ts +++ b/src/routes/stats.routes.ts @@ -25,9 +25,7 @@ type MostFrequentSalesRequest = z.infer; router.get('/most-frequent-sales', validateRequest(mostFrequentSalesSchema), async (req: Request, res: Response, next: NextFunction) => { try { const { query } = req as unknown as MostFrequentSalesRequest; - const days = query.days !== undefined ? Number(query.days) : 30; - const limit = query.limit !== undefined ? Number(query.limit) : 10; - const items = await db.adminRepo.getMostFrequentSaleItems(days, limit, req.log); + const items = await db.adminRepo.getMostFrequentSaleItems(query.days, query.limit, req.log); res.json(items); } catch (error) { req.log.error({ error }, 'Error fetching most frequent sale items in /api/stats/most-frequent-sales:'); diff --git a/src/routes/system.routes.test.ts b/src/routes/system.routes.test.ts index 6ec0403d..0fec2b88 100644 --- a/src/routes/system.routes.test.ts +++ b/src/routes/system.routes.test.ts @@ -41,6 +41,12 @@ vi.mock('../services/logger.server', () => ({ describe('System Routes (/api/system)', () => { const app = createTestApp({ router: systemRouter, basePath: '/api/system' }); + + // Add a basic error handler to capture errors passed to next(err) and return JSON. + app.use((err: any, req: any, res: any, next: any) => { + res.status(err.status || 500).json({ message: err.message, errors: err.errors }); + }); + beforeEach(() => { // We cast here to get type-safe access to mock functions like .mockImplementation vi.clearAllMocks(); @@ -103,6 +109,53 @@ describe('System Routes (/api/system)', () => { expect(response.body.success).toBe(false); }); + it('should return success: false when pm2 process does not exist', async () => { + // Arrange: Simulate `pm2 describe` failing because the process isn't found. + const processNotFoundOutput = "[PM2][ERROR] Process or Namespace flyer-crawler-api doesn't exist"; + const processNotFoundError = new Error('Command failed: pm2 describe flyer-crawler-api') as ExecException; + processNotFoundError.code = 1; + + vi.mocked(exec).mockImplementation(( + command: string, + options?: ExecOptions | ((error: ExecException | null, stdout: string, stderr: string) => void) | null, + callback?: ((error: ExecException | null, stdout: string, stderr: string) => void) | null + ) => { + const actualCallback = (typeof options === 'function' ? options : callback) as ((error: ExecException | null, stdout: string, stderr: string) => void); + if (actualCallback) { + actualCallback(processNotFoundError, processNotFoundOutput, ''); + } + return { unref: () => {} } as ReturnType; + }); + + // Act + const response = await supertest(app).get('/api/system/pm2-status'); + + // Assert + expect(response.status).toBe(200); + expect(response.body).toEqual({ success: false, message: 'Application process is not running under PM2.' }); + }); + + it('should return 500 if pm2 command produces stderr output', async () => { + // Arrange: Simulate a successful exit code but with content in stderr. + const stderrOutput = 'A non-fatal warning occurred.'; + + vi.mocked(exec).mockImplementation(( + command: string, + options?: ExecOptions | ((error: ExecException | null, stdout: string, stderr: string) => void) | null, + callback?: ((error: ExecException | null, stdout: string, stderr: string) => void) | null + ) => { + const actualCallback = (typeof options === 'function' ? options : callback) as ((error: ExecException | null, stdout: string, stderr: string) => void); + if (actualCallback) { + actualCallback(null, 'Some stdout', stderrOutput); + } + return { unref: () => {} } as ReturnType; + }); + + const response = await supertest(app).get('/api/system/pm2-status'); + expect(response.status).toBe(500); + expect(response.body.message).toBe(`PM2 command produced an error: ${stderrOutput}`); + }); + it('should return 500 on a generic exec error', async () => { vi.mocked(exec).mockImplementation(( command: string, diff --git a/src/routes/user.routes.test.ts b/src/routes/user.routes.test.ts index 6fb6d477..ca9b70d9 100644 --- a/src/routes/user.routes.test.ts +++ b/src/routes/user.routes.test.ts @@ -10,6 +10,7 @@ import { Appliance, Notification, DietaryRestriction } from '../types'; import { ForeignKeyConstraintError, NotFoundError } from '../services/db/errors.db'; import { createTestApp } from '../tests/utils/createTestApp'; +import { logger } from '../services/logger.server'; // 1. Mock the Service Layer directly. // The user.routes.ts file imports from '.../db/index.db'. We need to mock that module. vi.mock('../services/db/index.db', () => ({ @@ -81,6 +82,7 @@ vi.mock('../services/logger.server', () => ({ debug: vi.fn(), error: vi.fn(), warn: vi.fn(), + child: vi.fn().mockReturnThis(), }, })); @@ -160,6 +162,7 @@ describe('User Routes (/api/users)', () => { vi.mocked(db.userRepo.findUserProfileById).mockRejectedValue(dbError); const response = await supertest(app).get('/api/users/profile'); expect(response.status).toBe(500); + expect(logger.error).toHaveBeenCalledWith({ error: dbError }, `[ROUTE] GET /api/users/profile - ERROR`); }); }); @@ -177,6 +180,7 @@ describe('User Routes (/api/users)', () => { vi.mocked(db.personalizationRepo.getWatchedItems).mockRejectedValue(dbError); const response = await supertest(app).get('/api/users/watched-items'); expect(response.status).toBe(500); + expect(logger.error).toHaveBeenCalledWith({ error: dbError }, `[ROUTE] GET /api/users/watched-items - ERROR`); }); }); @@ -199,6 +203,7 @@ describe('User Routes (/api/users)', () => { .post('/api/users/watched-items') .send({ itemName: 'Test', category: 'Produce' }); expect(response.status).toBe(500); + expect(logger.error).toHaveBeenCalled(); }); }); @@ -243,6 +248,7 @@ describe('User Routes (/api/users)', () => { vi.mocked(db.personalizationRepo.removeWatchedItem).mockRejectedValue(dbError); const response = await supertest(app).delete(`/api/users/watched-items/99`); expect(response.status).toBe(500); + expect(logger.error).toHaveBeenCalledWith({ error: dbError }, `[ROUTE] DELETE /api/users/watched-items/:masterItemId - ERROR`); }); }); @@ -260,6 +266,7 @@ describe('User Routes (/api/users)', () => { vi.mocked(db.shoppingRepo.getShoppingLists).mockRejectedValue(dbError); const response = await supertest(app).get('/api/users/shopping-lists'); expect(response.status).toBe(500); + expect(logger.error).toHaveBeenCalledWith({ error: dbError }, `[ROUTE] GET /api/users/shopping-lists - ERROR`); }); it('POST /shopping-lists should create a new list', async () => { @@ -293,6 +300,7 @@ describe('User Routes (/api/users)', () => { const response = await supertest(app).post('/api/users/shopping-lists').send({ name: 'Failing List' }); expect(response.status).toBe(500); expect(response.body.message).toBe('DB Connection Failed'); + expect(logger.error).toHaveBeenCalled(); }); it('should return 400 for an invalid listId on DELETE', async () => { @@ -321,6 +329,7 @@ describe('User Routes (/api/users)', () => { vi.mocked(db.shoppingRepo.deleteShoppingList).mockRejectedValue(dbError); const response = await supertest(app).delete('/api/users/shopping-lists/1'); expect(response.status).toBe(500); + expect(logger.error).toHaveBeenCalled(); }); it('should return 400 for an invalid listId', async () => { @@ -330,7 +339,41 @@ describe('User Routes (/api/users)', () => { }); }); }); - describe('Shopping List Item Routes', () => { it('POST /shopping-lists/:listId/items should add an item to a list', async () => { + describe('Shopping List Item Routes', () => { + describe('POST /shopping-lists/:listId/items (Validation)', () => { + it('should return 400 if neither masterItemId nor customItemName are provided', async () => { + const response = await supertest(app) + .post('/api/users/shopping-lists/1/items') + .send({}); + expect(response.status).toBe(400); + expect(response.body.errors[0].message).toBe('Either masterItemId or customItemName must be provided.'); + }); + + it('should succeed if only masterItemId is provided', async () => { + vi.mocked(db.shoppingRepo.addShoppingListItem).mockResolvedValue(createMockShoppingListItem({})); + const response = await supertest(app) + .post('/api/users/shopping-lists/1/items') + .send({ masterItemId: 123 }); + expect(response.status).toBe(201); + }); + + it('should succeed if only customItemName is provided', async () => { + vi.mocked(db.shoppingRepo.addShoppingListItem).mockResolvedValue(createMockShoppingListItem({})); + const response = await supertest(app) + .post('/api/users/shopping-lists/1/items') + .send({ customItemName: 'Custom Item' }); + expect(response.status).toBe(201); + }); + + it('should succeed if both masterItemId and customItemName are provided', async () => { + vi.mocked(db.shoppingRepo.addShoppingListItem).mockResolvedValue(createMockShoppingListItem({})); + const response = await supertest(app) + .post('/api/users/shopping-lists/1/items') + .send({ masterItemId: 123, customItemName: 'Custom Item' }); + expect(response.status).toBe(201); + }); + }); + it('POST /shopping-lists/:listId/items should add an item to a list', async () => { const listId = 1; const itemData = { customItemName: 'Paper Towels' }; const mockAddedItem = createMockShoppingListItem({ shopping_list_item_id: 101, shopping_list_id: listId, ...itemData }); @@ -356,6 +399,7 @@ describe('User Routes (/api/users)', () => { .post('/api/users/shopping-lists/1/items') .send({ customItemName: 'Test' }); expect(response.status).toBe(500); + expect(logger.error).toHaveBeenCalled(); }); it('PUT /shopping-lists/items/:itemId should update an item', async () => { @@ -384,6 +428,15 @@ describe('User Routes (/api/users)', () => { .put('/api/users/shopping-lists/items/101') .send({ is_purchased: true }); expect(response.status).toBe(500); + expect(logger.error).toHaveBeenCalled(); + }); + + it('should return 400 if no update fields are provided for an item', async () => { + const response = await supertest(app) + .put(`/api/users/shopping-lists/items/101`) + .send({}); + expect(response.status).toBe(400); + expect(response.body.errors[0].message).toContain('At least one field (quantity, is_purchased) must be provided.'); }); describe('DELETE /shopping-lists/items/:itemId', () => { @@ -404,6 +457,7 @@ describe('User Routes (/api/users)', () => { vi.mocked(db.shoppingRepo.removeShoppingListItem).mockRejectedValue(dbError); const response = await supertest(app).delete('/api/users/shopping-lists/items/101'); expect(response.status).toBe(500); + expect(logger.error).toHaveBeenCalled(); }); }); }); @@ -428,6 +482,7 @@ describe('User Routes (/api/users)', () => { .put('/api/users/profile') .send({ full_name: 'New Name' }); expect(response.status).toBe(500); + expect(logger.error).toHaveBeenCalledWith({ error: dbError }, `[ROUTE] PUT /api/users/profile - ERROR`); }); it('should return 400 if the body is empty', async () => { @@ -459,6 +514,7 @@ describe('User Routes (/api/users)', () => { .put('/api/users/profile/password') .send({ newPassword: 'a-Very-Strong-Password-456!' }); expect(response.status).toBe(500); + expect(logger.error).toHaveBeenCalledWith({ error: dbError }, `[ROUTE] PUT /api/users/profile/password - ERROR`); }); it('should return 400 for a weak password', async () => { @@ -506,6 +562,20 @@ describe('User Routes (/api/users)', () => { expect(response.body.message).toBe('User not found or password not set.'); }); + it('should return 404 if user is an OAuth user without a password', async () => { + // Simulate an OAuth user who has no password_hash set. + const userWithoutHash = createMockUserWithPasswordHash({ ...mockUserProfile.user, password_hash: null }); + vi.mocked(db.userRepo.findUserWithPasswordHashById).mockResolvedValue(userWithoutHash); + + const response = await supertest(app) + .delete('/api/users/account') + .send({ password: 'any-password' }); + + expect(response.status).toBe(404); + expect(response.body.message).toBe('User not found or password not set.'); + }); + + it('should return 500 on a generic database error', async () => { const userWithHash = createMockUserWithPasswordHash({ ...mockUserProfile.user, password_hash: 'hashed-password' }); vi.mocked(db.userRepo.findUserWithPasswordHashById).mockResolvedValue(userWithHash); @@ -515,6 +585,7 @@ describe('User Routes (/api/users)', () => { .delete('/api/users/account') .send({ password: 'correct-password' }); expect(response.status).toBe(500); + expect(logger.error).toHaveBeenCalledWith({ error: new Error('DB Connection Failed') }, `[ROUTE] DELETE /api/users/account - ERROR`); }); }); @@ -541,6 +612,7 @@ describe('User Routes (/api/users)', () => { .put('/api/users/profile/preferences') .send({ darkMode: true }); expect(response.status).toBe(500); + expect(logger.error).toHaveBeenCalledWith({ error: dbError }, `[ROUTE] PUT /api/users/profile/preferences - ERROR`); }); it('should return 400 if the request body is not a valid object', async () => { @@ -568,6 +640,7 @@ describe('User Routes (/api/users)', () => { vi.mocked(db.personalizationRepo.getUserDietaryRestrictions).mockRejectedValue(dbError); const response = await supertest(app).get('/api/users/me/dietary-restrictions'); expect(response.status).toBe(500); + expect(logger.error).toHaveBeenCalledWith({ error: dbError }, `[ROUTE] GET /api/users/me/dietary-restrictions - ERROR`); }); it('should return 400 for an invalid masterItemId', async () => { @@ -601,6 +674,14 @@ describe('User Routes (/api/users)', () => { .put('/api/users/me/dietary-restrictions') .send({ restrictionIds: [1] }); expect(response.status).toBe(500); + expect(logger.error).toHaveBeenCalled(); + }); + + it('PUT should return 400 if restrictionIds is not an array', async () => { + const response = await supertest(app) + .put('/api/users/me/dietary-restrictions') + .send({ restrictionIds: 'not-an-array' }); + expect(response.status).toBe(400); }); }); @@ -618,6 +699,7 @@ describe('User Routes (/api/users)', () => { vi.mocked(db.personalizationRepo.getUserAppliances).mockRejectedValue(dbError); const response = await supertest(app).get('/api/users/me/appliances'); expect(response.status).toBe(500); + expect(logger.error).toHaveBeenCalledWith({ error: dbError }, `[ROUTE] GET /api/users/me/appliances - ERROR`); }); it('PUT should successfully set the appliances', async () => { @@ -643,6 +725,14 @@ describe('User Routes (/api/users)', () => { .put('/api/users/me/appliances') .send({ applianceIds: [1] }); expect(response.status).toBe(500); + expect(logger.error).toHaveBeenCalled(); + }); + + it('PUT should return 400 if applianceIds is not an array', async () => { + const response = await supertest(app) + .put('/api/users/me/appliances') + .send({ applianceIds: 'not-an-array' }); + expect(response.status).toBe(400); }); }); }); @@ -702,6 +792,23 @@ describe('User Routes (/api/users)', () => { }); }); + describe('Address Routes', () => { + it('GET /addresses/:addressId should return the address if it belongs to the user', async () => { + const appWithUser = createTestApp({ router: userRouter, basePath, authenticatedUser: { ...mockUserProfile, address_id: 1 } }); + const mockAddress = { address_id: 1, address_line_1: '123 Main St' }; + vi.mocked(db.addressRepo.getAddressById).mockResolvedValue(mockAddress as any); + const response = await supertest(appWithUser).get('/api/users/addresses/1'); + expect(response.status).toBe(200); + expect(response.body).toEqual(mockAddress); + }); + + it('GET /addresses/:addressId should return 500 on a generic database error', async () => { + const appWithUser = createTestApp({ router: userRouter, basePath, authenticatedUser: { ...mockUserProfile, address_id: 1 } }); + vi.mocked(db.addressRepo.getAddressById).mockRejectedValue(new Error('DB Error')); + const response = await supertest(appWithUser).get('/api/users/addresses/1'); + expect(response.status).toBe(500); + }); + describe('GET /addresses/:addressId', () => { it('should return 400 for a non-numeric address ID', async () => { const response = await supertest(app).get('/api/users/addresses/abc'); // This was a duplicate, fixed. @@ -709,7 +816,6 @@ describe('User Routes (/api/users)', () => { }); }); - describe('Address Routes', () => { it('GET /addresses/:addressId should return 403 if address does not belong to user', async () => { const appWithDifferentUser = createTestApp({ router: userRouter, basePath, authenticatedUser: { ...mockUserProfile, address_id: 999 } }); const response = await supertest(appWithDifferentUser).get('/api/users/addresses/1'); @@ -747,6 +853,13 @@ describe('User Routes (/api/users)', () => { expect(response.status).toBe(500); }); + it('should return 400 if the address body is empty', async () => { + const response = await supertest(app) + .put('/api/users/profile/address') + .send({}); + expect(response.status).toBe(400); + expect(response.body.errors[0].message).toContain('At least one address field must be provided'); + }); }); describe('POST /profile/avatar', () => { @@ -815,6 +928,7 @@ describe('User Routes (/api/users)', () => { vi.mocked(db.recipeRepo.deleteRecipe).mockRejectedValue(dbError); const response = await supertest(app).delete('/api/users/recipes/1'); expect(response.status).toBe(500); + expect(logger.error).toHaveBeenCalled(); }); it('PUT /recipes/:recipeId should update a user\'s own recipe', async () => { @@ -842,6 +956,15 @@ describe('User Routes (/api/users)', () => { vi.mocked(db.recipeRepo.updateRecipe).mockRejectedValue(dbError); const response = await supertest(app).put('/api/users/recipes/1').send({ name: 'New Name' }); expect(response.status).toBe(500); + expect(logger.error).toHaveBeenCalled(); + }); + + it('PUT /recipes/:recipeId should return 400 if no update fields are provided', async () => { + const response = await supertest(app) + .put('/api/users/recipes/1') + .send({}); + expect(response.status).toBe(400); + expect(response.body.errors[0].message).toBe('No fields provided to update.'); }); it('GET /shopping-lists/:listId should return 404 if list is not found', async () => { @@ -851,11 +974,21 @@ describe('User Routes (/api/users)', () => { expect(response.body.message).toBe('Shopping list not found'); }); + it('GET /shopping-lists/:listId should return a single shopping list', async () => { + const mockList = createMockShoppingList({ shopping_list_id: 1, user_id: mockUserProfile.user_id }); + vi.mocked(db.shoppingRepo.getShoppingListById).mockResolvedValue(mockList); + const response = await supertest(app).get('/api/users/shopping-lists/1'); + expect(response.status).toBe(200); + expect(response.body).toEqual(mockList); + expect(db.shoppingRepo.getShoppingListById).toHaveBeenCalledWith(1, mockUserProfile.user_id, expectLogger); + }); + it('GET /shopping-lists/:listId should return 500 on a generic database error', async () => { const dbError = new Error('DB Connection Failed'); vi.mocked(db.shoppingRepo.getShoppingListById).mockRejectedValue(dbError); const response = await supertest(app).get('/api/users/shopping-lists/1'); expect(response.status).toBe(500); + expect(logger.error).toHaveBeenCalled(); }); }); // End of Recipe Routes }); diff --git a/src/routes/user.routes.ts b/src/routes/user.routes.ts index 1d514239..873647ab 100644 --- a/src/routes/user.routes.ts +++ b/src/routes/user.routes.ts @@ -146,9 +146,7 @@ router.get( // Apply ADR-003 pattern for type safety try { const { query } = req as unknown as GetNotificationsRequest; - const limit = query.limit ? Number(query.limit) : 20; - const offset = query.offset ? Number(query.offset) : 0; - const notifications = await db.notificationRepo.getNotificationsForUser(user.user_id, limit, offset, req.log); + const notifications = await db.notificationRepo.getNotificationsForUser(user.user_id, query.limit, query.offset, req.log); res.json(notifications); } catch (error) { next(error); @@ -420,7 +418,7 @@ router.delete('/shopping-lists/:listId', validateRequest(shoppingListIdSchema), const addShoppingListItemSchema = shoppingListIdSchema.extend({ body: z.object({ masterItemId: z.number().int().positive().optional(), - customItemName: requiredString('customItemName required?'), + customItemName: z.string().min(1, 'customItemName cannot be empty if provided').optional(), }).refine(data => data.masterItemId || data.customItemName, { message: 'Either masterItemId or customItemName must be provided.' }), }); type AddShoppingListItemRequest = z.infer; diff --git a/src/services/aiApiClient.ts b/src/services/aiApiClient.ts index 2abd59d8..694b98bf 100644 --- a/src/services/aiApiClient.ts +++ b/src/services/aiApiClient.ts @@ -157,7 +157,7 @@ export const startVoiceSession = (callbacks: { onmessage: (message: import('@google/genai').LiveServerMessage) => void; onerror?: (error: ErrorEvent) => void; onclose?: () => void; -}) : void => { +}) : Promise => { logger.debug('Stub: startVoiceSession called.', { callbacks }); // In a real implementation, this would connect to a WebSocket endpoint on your server, // which would then proxy the connection to the Google AI Live API.