unit test fixes + error refactor

src/routes/user.routes.test.ts

@@ -8,6 +8,7 @@ import userRouter from './user.routes';
 import { createMockUserProfile, createMockMasterGroceryItem, createMockShoppingList, createMockShoppingListItem, createMockRecipe } from '../tests/utils/mockFactories';
 import { Appliance, Notification } from '../types';
 import { ForeignKeyConstraintError } from '../services/db/errors.db';
+import { errorHandler } from '../middleware/errorHandler';
 
 // 1. Mock the Service Layer directly.
 // The user.routes.ts file imports from '.../db/index.db'. We need to mock that module.
@@ -108,12 +109,7 @@ const createApp = (authenticatedUser?: any) => {
   }
   
   app.use('/api/users', userRouter);
-
-  // Add a generic error handler to catch errors passed via next()
-  app.use((err: Error, req: Request, res: Response, next: NextFunction) => {
-    res.status(500).json({ message: err.message || 'Internal Server Error' });
-  });
-
+  app.use(errorHandler);
   return app;
 };
 
@@ -123,9 +119,9 @@ describe('User Routes (/api/users)', () => {
   });
 
   describe('when user is not authenticated', () => {
-    it('GET /profile should return 401 because the mockAuth middleware is not active for this test block', async () => {
+    it('GET /profile should return 401', async () => {
       const app = createApp(); // No user injected
-      const response = await supertest(app).get('/api/users/watched-items');
+      const response = await supertest(app).get('/api/users/profile');
       expect(response.status).toBe(401);
     });
   });
@@ -157,6 +153,7 @@ describe('User Routes (/api/users)', () => {
         vi.mocked(db.userRepo.findUserProfileById).mockRejectedValue(new Error('DB Error'));
         const response = await supertest(app).get('/api/users/profile');
         expect(response.status).toBe(500);
+        expect(response.body.message).toBe('DB Error');
       });
     });
 
@@ -173,6 +170,7 @@ describe('User Routes (/api/users)', () => {
         vi.mocked(db.personalizationRepo.getWatchedItems).mockRejectedValue(new Error('DB Error'));
         const response = await supertest(app).get('/api/users/watched-items');
         expect(response.status).toBe(500);
+        expect(response.body.message).toBe('DB Error');
       });
     });
 
@@ -194,6 +192,7 @@ describe('User Routes (/api/users)', () => {
           .post('/api/users/watched-items')
           .send({ itemName: 'Failing Item', category: 'Errors' });
         expect(response.status).toBe(500);
+        expect(response.body.message).toBe('DB Error');
       });
     });
 
@@ -235,6 +234,7 @@ describe('User Routes (/api/users)', () => {
         vi.mocked(db.personalizationRepo.removeWatchedItem).mockRejectedValue(new Error('DB Error'));
         const response = await supertest(app).delete('/api/users/watched-items/99');
         expect(response.status).toBe(500);
+        expect(response.body.message).toBe('DB Error');
       });
     });
 
@@ -277,6 +277,7 @@ describe('User Routes (/api/users)', () => {
           .post('/api/users/shopping-lists')
           .send({ name: 'New List' });
         expect(response.status).toBe(500);
+        expect(response.body.message).toBe('DB Error');
       });
 
       it('should return 400 for an invalid listId on DELETE', async () => {
@@ -324,6 +325,7 @@ describe('User Routes (/api/users)', () => {
         vi.mocked(db.shoppingRepo.addShoppingListItem).mockRejectedValue(new Error('DB Error'));
         const response = await supertest(app).post('/api/users/shopping-lists/1/items').send({ customItemName: 'Test' });
         expect(response.status).toBe(500);
+        expect(response.body.message).toBe('DB Error');
       });
 
       it('should return 400 for an invalid listId on POST', async () => {
@@ -363,6 +365,7 @@ describe('User Routes (/api/users)', () => {
           .put('/api/users/shopping-lists/items/101')
           .send({ is_purchased: true });
         expect(response.status).toBe(500);
+        expect(response.body.message).toBe('DB Error');
       });
 
       it('should return 500 if the database call fails', async () => {
@@ -371,6 +374,7 @@ describe('User Routes (/api/users)', () => {
           .put('/api/users/shopping-lists/items/101')
           .send({ is_purchased: true });
         expect(response.status).toBe(500);
+        expect(response.body.message).toBe('DB Error');
       });
 
       it('should return 400 for an invalid itemId on DELETE', async () => {
@@ -421,6 +425,7 @@ describe('User Routes (/api/users)', () => {
           .put('/api/users/profile')
           .send({ full_name: 'Failing Name' });
         expect(response.status).toBe(500);
+        expect(response.body.message).toBe('DB Error');
       });
     });
 
@@ -452,6 +457,7 @@ describe('User Routes (/api/users)', () => {
           .put('/api/users/profile/password')
           .send({ newPassword: 'a-Very-Strong-Password-456!' });
         expect(response.status).toBe(500);
+        expect(response.body.message).toBe('DB Error');
       });
     });
 
@@ -505,6 +511,7 @@ describe('User Routes (/api/users)', () => {
           .delete('/api/users/account')
           .send({ password: 'any-password' });
         expect(response.status).toBe(500);
+        expect(response.body.message).toBe('DB Error');
       });
 
       it('should return 400 if password is not provided', async () => {
@@ -555,6 +562,7 @@ describe('User Routes (/api/users)', () => {
           vi.mocked(db.personalizationRepo.getUserDietaryRestrictions).mockRejectedValue(new Error('DB Error'));
           const response = await supertest(app).get('/api/users/me/dietary-restrictions');
           expect(response.status).toBe(500);
+          expect(response.body.message).toBe('DB Error');
         });
 
       it('should return 400 for an invalid masterItemId', async () => {
@@ -567,6 +575,7 @@ describe('User Routes (/api/users)', () => {
           vi.mocked(db.personalizationRepo.getUserDietaryRestrictions).mockRejectedValue(new Error('DB Error'));
           const response = await supertest(app).get('/api/users/me/dietary-restrictions');
           expect(response.status).toBe(500);
+          expect(response.body.message).toBe('DB Error');
         });
 
         it('PUT should successfully set the restrictions', async () => {
@@ -584,6 +593,7 @@ describe('User Routes (/api/users)', () => {
             .put('/api/users/me/dietary-restrictions')
             .send({ restrictionIds: [1] });
           expect(response.status).toBe(500);
+          expect(response.body.message).toBe('DB Error');
         });
 
         it('PUT should return 500 if the database call fails', async () => {
@@ -592,6 +602,7 @@ describe('User Routes (/api/users)', () => {
             .put('/api/users/me/dietary-restrictions')
             .send({ restrictionIds: [1] });
           expect(response.status).toBe(500);
+          expect(response.body.message).toBe('DB Error');
         });
 
         it('PUT should return 400 on foreign key constraint error', async () => {
@@ -616,6 +627,7 @@ describe('User Routes (/api/users)', () => {
           vi.mocked(db.personalizationRepo.getUserAppliances).mockRejectedValue(new Error('DB Error'));
           const response = await supertest(app).get('/api/users/me/appliances');
           expect(response.status).toBe(500);
+          expect(response.body.message).toBe('DB Error');
         });
 
         it('PUT should successfully set the appliances', async () => {
@@ -631,6 +643,7 @@ describe('User Routes (/api/users)', () => {
             .put('/api/users/me/appliances')
             .send({ applianceIds: [1] });
           expect(response.status).toBe(500);
+          expect(response.body.message).toBe('DB Error');
         });
 
         it('PUT should return 400 if applianceIds is not an array', async () => {
@@ -668,12 +681,14 @@ describe('User Routes (/api/users)', () => {
         vi.mocked(db.notificationRepo.getNotificationsForUser).mockRejectedValue(new Error('DB Error'));
         const response = await supertest(app).get('/api/users/notifications');
         expect(response.status).toBe(500);
+        expect(response.body.message).toBe('DB Error');
       });
 
       it('POST /notifications/mark-all-read should return 500 if the database call fails', async () => {
         vi.mocked(db.notificationRepo.markAllNotificationsAsRead).mockRejectedValue(new Error('DB Error'));
         const response = await supertest(app).post('/api/users/notifications/mark-all-read');
         expect(response.status).toBe(500);
+        expect(response.body.message).toBe('DB Error');
       });
 
       it('POST /notifications/mark-all-read should return 204', async () => {
@@ -706,27 +721,28 @@ 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 = createApp({ ...mockUserProfile, address_id: 999 });
+        const appWithDifferentUser = createApp({ ...mockUserProfile, address_id: 999 } as any);
         const response = await supertest(appWithDifferentUser).get('/api/users/addresses/1');
         expect(response.status).toBe(403);
       });
 
       it('should return 500 if database call fails', async () => {
-        const appWithUser = createApp({ ...mockUserProfile, address_id: 1 });
+        const appWithUser = createApp({ ...mockUserProfile, address_id: 1 } as any);
         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);
+        expect(response.body.message).toBe('DB Error');
       });
 
       it('GET /addresses/:addressId should return 404 if address not found', async () => {
-        const appWithUser = createApp({ ...mockUserProfile, address_id: 1 });
+        const appWithUser = createApp({ ...mockUserProfile, address_id: 1 } as any);
         vi.mocked(db.addressRepo.getAddressById).mockResolvedValue(undefined);
         const response = await supertest(appWithUser).get('/api/users/addresses/1');
         expect(response.status).toBe(404);
       });
 
       it('PUT /profile/address should call upsertAddress and updateUserProfile if needed', async () => {
-        const appWithUser = createApp({ ...mockUserProfile, address_id: null }); // User has no address yet
+        const appWithUser = createApp({ ...mockUserProfile, address_id: null } as any); // User has no address yet
         const addressData = { address_line_1: '123 New St' };
         vi.mocked(db.addressRepo.upsertAddress).mockResolvedValue(5); // New address ID is 5
         vi.mocked(db.userRepo.updateUserProfile).mockResolvedValue({} as any);
@@ -742,15 +758,16 @@ describe('User Routes (/api/users)', () => {
       });
 
       it('should return 500 if upsertAddress fails', async () => {
-        const appWithUser = createApp({ ...mockUserProfile, address_id: null });
+        const appWithUser = createApp({ ...mockUserProfile, address_id: null } as any);
         const addressData = { address_line_1: '123 New St' };
         vi.mocked(db.addressRepo.upsertAddress).mockRejectedValue(new Error('DB Error'));
         const response = await supertest(appWithUser).put('/api/users/profile/address').send(addressData);
         expect(response.status).toBe(500);
+        expect(response.body.message).toBe('DB Error');
       });
 
       it('should return 500 if upsertAddress succeeds but updateUserProfile fails', async () => {
-        const appWithUser = createApp({ ...mockUserProfile, address_id: null });
+        const appWithUser = createApp({ ...mockUserProfile, address_id: null } as any);
         const addressData = { address_line_1: '123 Failing St' };
         vi.mocked(db.addressRepo.upsertAddress).mockResolvedValue(6); // upsert succeeds
         vi.mocked(db.userRepo.updateUserProfile).mockRejectedValue(new Error('DB link error')); // update fails
@@ -758,6 +775,7 @@ describe('User Routes (/api/users)', () => {
         const response = await supertest(appWithUser).put('/api/users/profile/address').send(addressData);
 
         expect(response.status).toBe(500);
+        expect(response.body.message).toBe('DB link error');
       });
     });
 
@@ -804,6 +822,7 @@ describe('User Routes (/api/users)', () => {
           .put('/api/users/profile/password')
           .send({ newPassword: 'a-Very-Strong-Password-456!' });
         expect(response.status).toBe(500);
+        expect(response.body.message).toBe('DB Error');
       });
     });
 
@@ -819,6 +838,7 @@ describe('User Routes (/api/users)', () => {
         vi.mocked(db.recipeRepo.deleteRecipe).mockRejectedValue(new Error('DB Error'));
         const response = await supertest(app).delete('/api/users/recipes/1');
         expect(response.status).toBe(500);
+        expect(response.body.message).toBe('DB Error');
       });
 
       it('DELETE /recipes/:recipeId should return 404 if recipe to delete is not found', async () => {
@@ -857,12 +877,14 @@ describe('User Routes (/api/users)', () => {
         vi.mocked(db.recipeRepo.updateRecipe).mockRejectedValue(new Error('DB Error'));
         const response = await supertest(app).put('/api/users/recipes/1').send({ name: 'New Name' });
         expect(response.status).toBe(500);
+        expect(response.body.message).toBe('DB Error');
       });
 
       it('GET /shopping-lists/:listId should return 500 if the database call fails', async () => {
         vi.mocked(db.shoppingRepo.getShoppingListById).mockRejectedValue(new Error('DB Error'));
         const response = await supertest(app).get('/api/users/shopping-lists/1');
         expect(response.status).toBe(500);
+        expect(response.body.message).toBe('DB Error');
       });
 
       it('GET /shopping-lists/:listId should return 404 if list is not found', async () => {