moar fixes + unit test review of routes

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(<FlyerDisplay {...defaultProps} validFrom="2023-10-26" validTo="invalid-date" />);
       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(<FlyerDisplay {...defaultProps} validFrom="another-bad-date" validTo="2023-11-01" />);
       expect(screen.getByText('Deals end November 1, 2023')).toBeInTheDocument();
       expect(screen.queryByText(/deals valid from/i)).not.toBeInTheDocument();

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', () => {

src/features/flyer/FlyerList.tsx

@@ -56,9 +56,9 @@ export const FlyerList: React.FC<FlyerListProps> = ({ 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}`,

src/features/shopping/ShoppingList.tsx

@@ -72,7 +72,7 @@ export const ShoppingListComponent: React.FC<ShoppingListComponentProps> = ({ 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();
             

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(<WatchedItemsList {...defaultProps} />);
+      const { rerender } = render(<WatchedItemsList {...defaultProps} />);
       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(<WatchedItemsList {...defaultProps} items={itemsWithoutProduce} />);
+
+      // 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', () => {

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', {

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 () => {

src/pages/ResetPasswordPage.tsx

@@ -94,7 +94,12 @@ export const ResetPasswordPage: React.FC = () => {
                         </div>
                         {error && <p className="text-sm text-red-600 dark:text-red-400 text-center">{error}</p>}
                         <div>
-                            <button type="submit" disabled={loading} className="group relative w-full flex justify-center py-2 px-4 border border-transparent text-sm font-medium rounded-md text-white bg-brand-secondary hover:bg-brand-dark focus:outline-none focus:ring-2 focus:ring-offset-2 focus:ring-brand-primary disabled:bg-gray-400">
+                            <button
+                                type="submit"
+                                disabled={loading}
+                                aria-label="Reset Password"
+                                className="group relative w-full flex justify-center py-2 px-4 border border-transparent text-sm font-medium rounded-md text-white bg-brand-secondary hover:bg-brand-dark focus:outline-none focus:ring-2 focus:ring-offset-2 focus:ring-brand-primary disabled:bg-gray-400"
+                            >
                                 {loading ? <div className="w-5 h-5"><LoadingSpinner /></div> : 'Reset Password'}
                             </button>
                         </div>

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)));
     

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(<ProfileManager {...defaultAuthenticatedProps} />);
       
+      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(<ProfileManager {...defaultAuthenticatedProps} />);
+      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(<ProfileManager {...defaultAuthenticatedProps} />);
       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();
   

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 };

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;

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);
+    });
   });
 });

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]);

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);
+      });
     });
 });

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.');
     });
   });
 });

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:');
     });
   });
 

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');

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<typeof redisConnection>;
 const mockedDbConnection = dbConnection as Mocked<typeof dbConnection>;
 const mockedFs = fs as Mocked<typeof fs>;
 
+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:');
   });
 });

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<Response>;
+    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<Request> = {
-        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<Request> = {
-        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<Response>;
-    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);
     });
   });