unit test fixes + error refactor

src/routes/admin.content.routes.test.ts

@@ -17,6 +17,7 @@ import adminRouter from './admin.routes';
 import { createMockUserProfile, createMockSuggestedCorrection, createMockBrand, createMockRecipe, createMockRecipeComment, createMockFlyerItem } from '../tests/utils/mockFactories';
 import { SuggestedCorrection, Brand, UserProfile, UnmatchedFlyerItem } from '../types';
 import { errorHandler } from '../middleware/errorHandler';
+import { NotFoundError } from '../services/db/errors.db';
 
 vi.mock('../lib/queue', () => ({
   serverAdapter: {
@@ -183,7 +184,7 @@ describe('Admin Content Management Routes (/api/admin)', () => {
     });
 
     it('PUT /corrections/:id should return 404 if correction not found', async () => {
-      vi.mocked(mockedDb.adminRepo.updateSuggestedCorrection).mockRejectedValue(new Error('Correction with ID 999 not found'));
+      vi.mocked(mockedDb.adminRepo.updateSuggestedCorrection).mockRejectedValue(new NotFoundError('Correction with ID 999 not found'));
       const response = await supertest(app).put('/api/admin/corrections/999').send({ suggested_value: 'new value' });
       expect(response.status).toBe(404);
       expect(response.body.message).toBe('Correction with ID 999 not found');
@@ -231,7 +232,7 @@ describe('Admin Content Management Routes (/api/admin)', () => {
     it('PUT /recipes/:id/status should return 400 for an invalid status', async () => {
       // FIX: The route logic checks for a valid recipe ID before it validates the status.
       // If the mock DB returns a "not found" error, the status code will be 404, not 400.
-      vi.mocked(mockedDb.adminRepo.updateRecipeStatus).mockRejectedValue(new Error('Recipe with ID 201 not found.'));
+      vi.mocked(mockedDb.adminRepo.updateRecipeStatus).mockRejectedValue(new NotFoundError('Recipe with ID 201 not found.'));
       const response = await supertest(app).put('/api/admin/recipes/201').send({ status: 'invalid-status' });
       expect(response.status).toBe(404);
       expect(response.body.message).toBe('Recipe with ID 201 not found.');
@@ -294,7 +295,7 @@ describe('Admin Content Management Routes (/api/admin)', () => {
 
     it('DELETE /flyers/:flyerId should return 404 if flyer not found', async () => {
       const flyerId = 999;
-      vi.mocked(mockedDb.flyerRepo.deleteFlyer).mockRejectedValue(new Error('Flyer with ID 999 not found.'));
+      vi.mocked(mockedDb.flyerRepo.deleteFlyer).mockRejectedValue(new NotFoundError('Flyer with ID 999 not found.'));
       const response = await supertest(app).delete(`/api/admin/flyers/${flyerId}`);
       expect(response.status).toBe(404);
       expect(response.body.message).toBe('Flyer with ID 999 not found.');

src/routes/admin.users.routes.test.ts

@@ -13,6 +13,7 @@ import adminRouter from './admin.routes';
 import { createMockUserProfile } from '../tests/utils/mockFactories';
 import { User, UserProfile } from '../types';
 import { errorHandler } from '../middleware/errorHandler';
+import { NotFoundError } from '../services/db/errors.db';
 
 const { mockedDb } = vi.hoisted(() => {
   return {
@@ -127,7 +128,7 @@ describe('Admin User Management Routes (/api/admin/users)', () => {
     });
 
     it('should return 404 for a non-existent user', async () => {
-      vi.mocked(mockedDb.userRepo.findUserProfileById).mockResolvedValue(undefined);
+      vi.mocked(mockedDb.userRepo.findUserProfileById).mockRejectedValue(new NotFoundError('User not found.'));
       const response = await supertest(app).get('/api/admin/users/non-existent-id');
       expect(response.status).toBe(404);
       expect(response.body.message).toBe('User not found.');
@@ -147,7 +148,7 @@ describe('Admin User Management Routes (/api/admin/users)', () => {
     });
 
     it('should return 404 for a non-existent user', async () => {
-      vi.mocked(mockedDb.adminRepo.updateUserRole).mockRejectedValue(new Error('User with ID non-existent not found.'));
+      vi.mocked(mockedDb.adminRepo.updateUserRole).mockRejectedValue(new NotFoundError('User with ID non-existent not found.'));
       const response = await supertest(app).put('/api/admin/users/non-existent').send({ role: 'user' });
       expect(response.status).toBe(404);
       expect(response.body.message).toBe('User with ID non-existent not found.');

src/routes/ai.routes.ts

@@ -134,24 +134,23 @@ router.post('/upload-and-process', optionalAuth, uploadToDisk.single('flyerFile'
 /**
  * NEW ENDPOINT: Checks the status of a background job.
  */
-router.get('/jobs/:jobId/status', async (req, res) => {
+router.get('/jobs/:jobId/status', async (req, res, next: NextFunction) => {
     const { jobId } = req.params;
-    const job = await flyerQueue.getJob(jobId);
-
-    if (!job) {
-        return res.status(404).json({ message: 'Job not found.' });
+    try {
+        const job = await flyerQueue.getJob(jobId);
+        if (!job) {
+            // The queue's getJob method doesn't throw NotFoundError, so we keep this check.
+            return res.status(404).json({ message: 'Job not found.' });
+        }
+        const state = await job.getState();
+        const progress = job.progress;
+        const returnValue = job.returnvalue;
+        const failedReason = job.failedReason;
+        logger.debug(`[API /ai/jobs] Status check for job ${jobId}: ${state}`);
+        res.json({ id: job.id, state, progress, returnValue, failedReason });
+    } catch (error) {
+        next(error);
     }
-
-    const state = await job.getState();
-    const progress = job.progress;
-    const returnValue = job.returnvalue;
-    const failedReason = job.failedReason;
-
-    logger.debug(`[API /ai/jobs] Status check for job ${jobId}: ${state}`);
-
-    // When the job is complete, the return value will contain the new flyerId
-    // which the client can use to navigate to the new flyer's page.
-    res.json({ id: job.id, state, progress, returnValue, failedReason });
 });
 
 

src/services/db/admin.db.ts

@@ -107,6 +107,9 @@ export class AdminRepository {
     }
     return res.rows[0];
     } catch (error) {
+      if (error instanceof NotFoundError) {
+        throw error;
+      }
       logger.error('Database error in updateSuggestedCorrection:', { error, correctionId });
       throw new Error('Failed to update suggested correction.');
     }
@@ -254,6 +257,9 @@ export class AdminRepository {
           }
           return res.rows[0];
       } catch (error) {
+          if (error instanceof NotFoundError) {
+              throw error;
+          }
           logger.error('Database error in updateRecipeCommentStatus:', { error, commentId, status });
           throw new Error('Failed to update recipe comment status.');
       }
@@ -307,6 +313,9 @@ export class AdminRepository {
           }
           return res.rows[0];
       } catch (error) {
+          if (error instanceof NotFoundError) {
+              throw error;
+          }
           logger.error('Database error in updateRecipeStatus:', { error, recipeId, status });
           throw new Error('Failed to update recipe status.');
       }
@@ -479,7 +488,7 @@ export class AdminRepository {
         [status, receiptId]
       );
       if (res.rowCount === 0) {
-        throw new Error(`Receipt with ID ${receiptId} not found.`);
+        throw new NotFoundError(`Receipt with ID ${receiptId} not found.`);
       }
       return res.rows[0];
     } catch (error) {
@@ -518,6 +527,9 @@ export class AdminRepository {
       if (error instanceof Error && 'code' in error && error.code === '23503') {
         throw new ForeignKeyConstraintError('The specified user does not exist.');
       }
+      if (error instanceof NotFoundError) {
+        throw error;
+      }
       throw error; // Re-throw to be handled by the route
     }
   }

src/services/db/flyer.db.ts

@@ -162,8 +162,11 @@ export class FlyerRepository {
       }
       return res.rows[0];
     } catch (error) {
+      if (error instanceof NotFoundError) {
+        throw error;
+      }
       logger.error('Database error in getFlyerById:', { error, flyerId });
-      throw new Error('Failed to retrieve flyer from database.');
+      throw new Error(`Failed to retrieve flyer from database. Original error: ${error instanceof Error ? error.message : String(error)}`);
     }
   }
 
@@ -283,11 +286,14 @@ export class FlyerRepository {
       // The database will handle deleting associated flyer_items, unmatched_flyer_items, etc.
       const res = await client.query('DELETE FROM public.flyers WHERE flyer_id = $1', [flyerId]);
       if (res.rowCount === 0) {
-        throw new Error(`Flyer with ID ${flyerId} not found.`);
+        throw new NotFoundError(`Flyer with ID ${flyerId} not found.`);
       }
       await client.query('COMMIT');
       logger.info(`Successfully deleted flyer with ID: ${flyerId}`);
     } catch (error) {
+      if (error instanceof NotFoundError) {
+        throw error; // Propagate NotFoundError without wrapping it
+      }
       await client.query('ROLLBACK');
       logger.error('Database transaction error in deleteFlyer:', { error, flyerId });
       throw new Error('Failed to delete flyer.');

src/services/db/notification.db.ts

@@ -1,7 +1,7 @@
 // src/services/db/notification.db.ts
 import type { Pool, PoolClient } from 'pg';
 import { getPool } from './connection.db';
-import { ForeignKeyConstraintError } from './errors.db';
+import { ForeignKeyConstraintError, NotFoundError } from './errors.db';
 import { logger } from '../logger.server';
 import { Notification } from '../../types';
 
@@ -119,21 +119,21 @@ export class NotificationRepository {
      * @throws An error if the notification is not found or does not belong to the user.
      */
     async markNotificationAsRead(notificationId: number, userId: string): Promise<Notification> {
-        try {
-            const res = await this.db.query<Notification>(
-                `UPDATE public.notifications SET is_read = true WHERE notification_id = $1 AND user_id = $2 RETURNING *`,
-                [notificationId, userId]
-            );
-            if (res.rowCount === 0) {
-                throw new Error('Notification not found or user does not have permission.');
-            }
-            return res.rows[0];
-        } catch (error) {
-            if (error instanceof Error && error.message.startsWith('Notification not found')) throw error;
-            logger.error('Database error in markNotificationAsRead:', { error, notificationId, userId });
-            throw new Error('Failed to mark notification as read.');
-        }
+    try {
+      const res = await this.db.query<Notification>(
+        `UPDATE public.notifications SET is_read = true WHERE notification_id = $1 AND user_id = $2 RETURNING *`,
+        [notificationId, userId]
+      );
+      if (res.rowCount === 0) {
+        throw new NotFoundError('Notification not found or user does not have permission.');
+      }
+      return res.rows[0];
+    } catch (error) {
+      if (error instanceof NotFoundError) throw error;
+      logger.error('Database error in markNotificationAsRead:', { error, notificationId, userId });
+      throw new Error('Failed to mark notification as read.');
     }
+   }
 
     /**
      * Deletes notifications that are older than a specified number of days.

src/services/db/recipe.db.ts

@@ -87,6 +87,9 @@ export class RecipeRepository {
       );
       return res.rows[0];
     } catch (error) {
+      if (error instanceof Error && 'code' in error && error.code === '23503') {
+        throw new ForeignKeyConstraintError('The specified user or recipe does not exist.');
+      }
       logger.error('Database error in addFavoriteRecipe:', { error, userId, recipeId });
       throw new Error('Failed to add favorite recipe.');
     }
@@ -104,6 +107,9 @@ export class RecipeRepository {
         throw new NotFoundError('Favorite recipe not found for this user.');
       }
     } catch (error) {
+      if (error instanceof NotFoundError) {
+        throw error;
+      }
       logger.error('Database error in removeFavoriteRecipe:', { error, userId, recipeId });
       throw new Error('Failed to remove favorite recipe.');
     }
@@ -209,6 +215,9 @@ export class RecipeRepository {
       }
       return res.rows[0];
     } catch (error) {
+      if (error instanceof NotFoundError) {
+        throw error;
+      }
       logger.error('Database error in getRecipeById:', { error, recipeId });
       throw new Error('Failed to retrieve recipe.');
     }

src/services/db/shopping.db.ts

@@ -117,6 +117,9 @@ export class ShoppingRepository {
       }
       return res.rows[0];
     } catch (error) {
+      if (error instanceof NotFoundError) {
+        throw error;
+      }
       logger.error('Database error in getShoppingListById:', { error, listId, userId });
       throw new Error('Failed to retrieve shopping list.');
     }

src/services/db/user.db.ts

@@ -220,6 +220,9 @@ export class UserRepository {
       }
       return res.rows[0];
     } catch (error) {
+      if (error instanceof NotFoundError) {
+        throw error;
+      }
       logger.error('Database error in findUserProfileById:', { error });
       throw new Error('Failed to retrieve user profile from database.');
     }
@@ -263,6 +266,9 @@ export class UserRepository {
       }
       return res.rows[0];
     } catch (error) {
+      if (error instanceof NotFoundError) {
+        throw error;
+      }
       logger.error('Database error in updateUserProfile:', { error });
       throw new Error('Failed to update user profile in database.');
     }
@@ -285,11 +291,13 @@ export class UserRepository {
         [preferences, userId]
       );
       if (res.rowCount === 0) {
-        throw new Error('User not found or user does not have permission to update.');
+        throw new NotFoundError('User not found or user does not have permission to update.');
       }
       return res.rows[0];
     } catch (error) {
-      if (error instanceof Error && error.message.includes('User not found')) throw error;
+      if (error instanceof NotFoundError) {
+        throw error;
+      }
       logger.error('Database error in updateUserPreferences:', { error });
       throw new Error('Failed to update user preferences in database.');
     }