From 27ef5901f19e2f445eee5aa511da66cb14072429 Mon Sep 17 00:00:00 2001 From: Torben Sorensen Date: Thu, 11 Dec 2025 19:20:38 -0800 Subject: [PATCH] unit test fixes + error refactor --- src/routes/admin.content.routes.test.ts | 7 +++--- src/routes/admin.users.routes.test.ts | 5 ++-- src/routes/ai.routes.ts | 31 ++++++++++++------------- src/services/db/admin.db.ts | 14 ++++++++++- src/services/db/flyer.db.ts | 10 ++++++-- src/services/db/notification.db.ts | 30 ++++++++++++------------ src/services/db/recipe.db.ts | 9 +++++++ src/services/db/shopping.db.ts | 3 +++ src/services/db/user.db.ts | 12 ++++++++-- 9 files changed, 80 insertions(+), 41 deletions(-) diff --git a/src/routes/admin.content.routes.test.ts b/src/routes/admin.content.routes.test.ts index 770361b3..ad43715c 100644 --- a/src/routes/admin.content.routes.test.ts +++ b/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.'); diff --git a/src/routes/admin.users.routes.test.ts b/src/routes/admin.users.routes.test.ts index c4cb23ea..8753b0b5 100644 --- a/src/routes/admin.users.routes.test.ts +++ b/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.'); diff --git a/src/routes/ai.routes.ts b/src/routes/ai.routes.ts index 16e8a324..72d9ee59 100644 --- a/src/routes/ai.routes.ts +++ b/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 }); }); diff --git a/src/services/db/admin.db.ts b/src/services/db/admin.db.ts index 20307839..58367986 100644 --- a/src/services/db/admin.db.ts +++ b/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 } } diff --git a/src/services/db/flyer.db.ts b/src/services/db/flyer.db.ts index a7c3efae..aed5c422 100644 --- a/src/services/db/flyer.db.ts +++ b/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.'); diff --git a/src/services/db/notification.db.ts b/src/services/db/notification.db.ts index 1f8f1544..0e415944 100644 --- a/src/services/db/notification.db.ts +++ b/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 { - try { - const res = await this.db.query( - `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( + `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. diff --git a/src/services/db/recipe.db.ts b/src/services/db/recipe.db.ts index 184cddf8..7d414d1c 100644 --- a/src/services/db/recipe.db.ts +++ b/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.'); } diff --git a/src/services/db/shopping.db.ts b/src/services/db/shopping.db.ts index 639069c9..2b9f13cd 100644 --- a/src/services/db/shopping.db.ts +++ b/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.'); } diff --git a/src/services/db/user.db.ts b/src/services/db/user.db.ts index 2fecca9c..3ad9c5fc 100644 --- a/src/services/db/user.db.ts +++ b/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.'); }