From 4674309ea1df6d3c444f61ff349a5a354bccdf86 Mon Sep 17 00:00:00 2001 From: Torben Sorensen Date: Tue, 9 Dec 2025 18:18:56 -0800 Subject: [PATCH] fix tests ugh --- src/routes/passport.routes.test.ts | 57 ++++++++++---- src/services/apiClient.test.ts | 11 +-- src/services/db/personalization.db.ts | 2 +- src/services/db/shopping.db.test.ts | 2 +- .../flyerProcessingService.server.test.ts | 75 ++++++------------- src/services/flyerProcessingService.types.ts | 9 --- 6 files changed, 67 insertions(+), 89 deletions(-) delete mode 100644 src/services/flyerProcessingService.types.ts diff --git a/src/routes/passport.routes.test.ts b/src/routes/passport.routes.test.ts index 5fcca67b..16f988b8 100644 --- a/src/routes/passport.routes.test.ts +++ b/src/routes/passport.routes.test.ts @@ -49,6 +49,7 @@ import { UserProfile } from '../types'; vi.mock('../services/db/index.db', () => ({ userRepo: { findUserByEmail: vi.fn(), + findUserWithProfileByEmail: vi.fn(), // ADD THIS findUserProfileById: vi.fn(), }, adminRepo: { @@ -99,8 +100,16 @@ describe('Passport Configuration', () => { it('should call done(null, user) on successful authentication', async () => { // Arrange - const mockUser = { user_id: 'user-123', email: 'test@test.com', password_hash: 'hashed_password', failed_login_attempts: 0, last_failed_login: null }; - vi.mocked(mockedDb.userRepo.findUserByEmail).mockResolvedValue(mockUser); + const mockUser = { + user_id: 'user-123', + email: 'test@test.com', + password_hash: 'hashed_password', + failed_login_attempts: 0, + last_failed_login: null, + points: 0, + role: 'user' as const + }; + vi.mocked(mockedDb.userRepo.findUserWithProfileByEmail).mockResolvedValue(mockUser); vi.mocked(bcrypt.compare).mockResolvedValue(true as never); // Act @@ -109,14 +118,14 @@ describe('Passport Configuration', () => { } // Assert - expect(mockedDb.userRepo.findUserByEmail).toHaveBeenCalledWith('test@test.com'); + expect(mockedDb.userRepo.findUserWithProfileByEmail).toHaveBeenCalledWith('test@test.com'); expect(bcrypt.compare).toHaveBeenCalledWith('password', 'hashed_password'); expect(mockedDb.adminRepo.resetFailedLoginAttempts).toHaveBeenCalledWith('user-123', '127.0.0.1'); - expect(done).toHaveBeenCalledWith(null, { user_id: 'user-123', email: 'test@test.com', failed_login_attempts: 0, last_failed_login: null }); + expect(done).toHaveBeenCalledWith(null, mockUser); }); it('should call done(null, false) if user is not found', async () => { - vi.mocked(mockedDb.userRepo.findUserByEmail).mockResolvedValue(undefined); + vi.mocked(mockedDb.userRepo.findUserWithProfileByEmail).mockResolvedValue(undefined); if (localStrategyCallbackWrapper.callback) { await localStrategyCallbackWrapper.callback(mockReq, 'notfound@test.com', 'password', done); @@ -126,8 +135,16 @@ describe('Passport Configuration', () => { }); it('should call done(null, false) and increment failed attempts on password mismatch', async () => { - const mockUser = { user_id: 'user-123', email: 'test@test.com', password_hash: 'hashed_password', failed_login_attempts: 1, last_failed_login: null }; - vi.mocked(mockedDb.userRepo.findUserByEmail).mockResolvedValue(mockUser); + const mockUser = { + user_id: 'user-123', + email: 'test@test.com', + password_hash: 'hashed_password', + failed_login_attempts: 1, + last_failed_login: null, + points: 0, + role: 'user' as const + }; + vi.mocked(mockedDb.userRepo.findUserWithProfileByEmail).mockResolvedValue(mockUser); vi.mocked(bcrypt.compare).mockResolvedValue(false as never); if (localStrategyCallbackWrapper.callback) { @@ -140,8 +157,16 @@ describe('Passport Configuration', () => { }); it('should call done(null, false) for an OAuth user (no password hash)', async () => { - const mockUser = { user_id: 'oauth-user', email: 'oauth@test.com', password_hash: null, failed_login_attempts: 0, last_failed_login: null }; - vi.mocked(mockedDb.userRepo.findUserByEmail).mockResolvedValue(mockUser); + const mockUser = { + user_id: 'oauth-user', + email: 'oauth@test.com', + password_hash: null, + failed_login_attempts: 0, + last_failed_login: null, + points: 0, + role: 'user' as const + }; + vi.mocked(mockedDb.userRepo.findUserWithProfileByEmail).mockResolvedValue(mockUser); if (localStrategyCallbackWrapper.callback) { await localStrategyCallbackWrapper.callback(mockReq, 'oauth@test.com', 'any_password', done); @@ -156,9 +181,11 @@ describe('Passport Configuration', () => { email: 'locked@test.com', password_hash: 'hashed_password', failed_login_attempts: 5, - last_failed_login: new Date().toISOString(), // Recently locked + last_failed_login: new Date().toISOString(), + points: 0, + role: 'user' as const }; - vi.mocked(mockedDb.userRepo.findUserByEmail).mockResolvedValue(mockUser); + vi.mocked(mockedDb.userRepo.findUserWithProfileByEmail).mockResolvedValue(mockUser); if (localStrategyCallbackWrapper.callback) { await localStrategyCallbackWrapper.callback(mockReq, 'locked@test.com', 'any_password', done); @@ -173,9 +200,11 @@ describe('Passport Configuration', () => { email: 'expired@test.com', password_hash: 'hashed_password', failed_login_attempts: 5, - last_failed_login: new Date(Date.now() - 20 * 60 * 1000).toISOString(), // Locked 20 mins ago + last_failed_login: new Date(Date.now() - 20 * 60 * 1000).toISOString(), + points: 0, + role: 'user' as const }; - vi.mocked(mockedDb.userRepo.findUserByEmail).mockResolvedValue(mockUser); + vi.mocked(mockedDb.userRepo.findUserWithProfileByEmail).mockResolvedValue(mockUser); vi.mocked(bcrypt.compare).mockResolvedValue(true as never); // Correct password if (localStrategyCallbackWrapper.callback) { @@ -189,7 +218,7 @@ describe('Passport Configuration', () => { it('should call done(err) if the database lookup fails', async () => { const dbError = new Error('DB connection failed'); - vi.mocked(mockedDb.userRepo.findUserByEmail).mockRejectedValue(dbError); + vi.mocked(mockedDb.userRepo.findUserWithProfileByEmail).mockRejectedValue(dbError); if (localStrategyCallbackWrapper.callback) { await localStrategyCallbackWrapper.callback(mockReq, 'any@test.com', 'any_password', done); diff --git a/src/services/apiClient.test.ts b/src/services/apiClient.test.ts index 3315969f..5cec77df 100644 --- a/src/services/apiClient.test.ts +++ b/src/services/apiClient.test.ts @@ -402,17 +402,8 @@ describe('API Client', () => { it('updateUserPreferences should send a PUT request with preferences data', async () => { const preferences = { darkMode: true }; - let capturedBody: typeof preferences | null = null; - // Restore the original fetch so MSW can intercept this request. - // The global fetch spy from beforeEach would otherwise capture this call. - vi.mocked(global.fetch).mockRestore(); - server.use( - http.put('http://localhost/api/users/profile/preferences', async ({ request }) => { - capturedBody = await request.json() as typeof preferences; - return HttpResponse.json({ success: true }); - }) - ); await apiClient.updateUserPreferences(preferences); + expect(capturedUrl?.pathname).toBe('/api/users/profile/preferences'); expect(capturedBody).toEqual(preferences); }); diff --git a/src/services/db/personalization.db.ts b/src/services/db/personalization.db.ts index 664f85c5..50bb91c5 100644 --- a/src/services/db/personalization.db.ts +++ b/src/services/db/personalization.db.ts @@ -269,11 +269,11 @@ export class PersonalizationRepository { await client.query('COMMIT'); return newAppliances; } catch (error) { + await client.query('ROLLBACK'); // The patch requested this specific error handling. if ((error as any).code === '23503') { throw new ForeignKeyConstraintError('Invalid appliance ID'); } - await client.query('ROLLBACK'); logger.error('Database error in setUserAppliances:', { error, userId }); throw new Error('Failed to set user appliances.'); } finally { diff --git a/src/services/db/shopping.db.test.ts b/src/services/db/shopping.db.test.ts index 64a381de..83c794cd 100644 --- a/src/services/db/shopping.db.test.ts +++ b/src/services/db/shopping.db.test.ts @@ -205,7 +205,7 @@ describe('Shopping DB Service', () => { it('should throw an error if the item to update is not found', async () => { mockPoolInstance.query.mockResolvedValue({ rowCount: 0, rows: [], command: 'UPDATE' }); - await expect(shoppingRepo.updateShoppingListItem(999, { quantity: 5 })).rejects.toThrow('Failed to update shopping list item.'); + await expect(shoppingRepo.updateShoppingListItem(999, { quantity: 5 })).rejects.toThrow('Shopping list item not found.'); }); it('should throw an error if no valid fields are provided to update', async () => { diff --git a/src/services/flyerProcessingService.server.test.ts b/src/services/flyerProcessingService.server.test.ts index 99700e55..17badb9f 100644 --- a/src/services/flyerProcessingService.server.test.ts +++ b/src/services/flyerProcessingService.server.test.ts @@ -1,15 +1,25 @@ +// src/services/flyerProcessingService.server.test.ts // --- FIX REGISTRY --- // // 2024-07-30: Fixed `FlyerDataTransformer` mock to be a constructible class. The previous mock was not a constructor, // causing a `TypeError` when `FlyerProcessingService` tried to instantiate it with `new`. // 2024-12-09: Fixed duplicate imports of FlyerProcessingService and FlyerJobData. Consolidated imports to use // FlyerJobData from types file and FlyerProcessingService from server file. +// 2024-12-09: Removed duplicate _saveProcessedFlyerData test suite. Fixed assertion to match actual logActivity call +// signature which includes displayText and userId fields. // --- END FIX REGISTRY --- -// src/services/flyerProcessingService.server.test.ts import { describe, it, expect, vi, beforeEach, type Mocked } from 'vitest'; import { Job } from 'bullmq'; import type { Dirent } from 'node:fs'; -import type { FlyerJobData } from './flyerProcessingService.types'; + +export interface FlyerJobData { + filePath: string; + originalFileName: string; + checksum: string; + userId?: string; + submitterIp?: string; + userProfileAddress?: string; +} // 1. Create hoisted mocks FIRST const mocks = vi.hoisted(() => ({ @@ -227,6 +237,7 @@ describe('FlyerProcessingService', () => { }; const mockImagePaths = [{ path: '/tmp/flyer.jpg', mimetype: 'image/jpeg' }]; const mockJobData = { + filePath: '/tmp/flyer.jpg', originalFileName: 'flyer.jpg', checksum: 'checksum-123', userId: 'user-abc', @@ -236,7 +247,7 @@ describe('FlyerProcessingService', () => { const transformerSpy = vi.spyOn(FlyerDataTransformer.prototype, 'transform'); // The DB create function is also mocked in beforeEach. - const mockNewFlyer = { flyer_id: 1, file_name: 'flyer.jpg', store_name: 'Test Store' }; + const mockNewFlyer = { flyer_id: 1, file_name: 'flyer.jpg', store_name: 'Mock Store' }; vi.mocked(createFlyerAndItems).mockResolvedValue({ flyer: mockNewFlyer, items: [] } as any); // Act: Access and call the private method for testing @@ -250,11 +261,13 @@ describe('FlyerProcessingService', () => { const transformedData = await transformerSpy.mock.results[0].value; expect(createFlyerAndItems).toHaveBeenCalledWith(transformedData.flyerData, transformedData.itemsForDb); - // 3. Activity was logged - expect(mockedDb.adminRepo.logActivity).toHaveBeenCalledWith(expect.objectContaining({ + // 3. Activity was logged with all expected fields + expect(mockedDb.adminRepo.logActivity).toHaveBeenCalledWith({ + userId: 'user-abc', action: 'flyer_processed', - details: { flyerId: mockNewFlyer.flyer_id, storeName: mockNewFlyer.store_name } - })); + displayText: 'Processed a new flyer for Mock Store.', + details: { flyerId: 1, storeName: 'Mock Store' } + }); // 4. The method returned the new flyer expect(result).toEqual(mockNewFlyer); @@ -305,50 +318,4 @@ describe('FlyerProcessingService', () => { .rejects.toThrow(commandError); }); }); - - describe('_saveProcessedFlyerData (private method)', () => { - it('should transform data, create flyer in DB, and log activity', async () => { - // Arrange - const mockExtractedData = { - store_name: 'Test Store', - valid_from: '2024-01-01', - valid_to: '2024-01-07', - store_address: '123 Mock St', - items: [{ item: 'Test Item', price_display: '$1.99', price_in_cents: 199, quantity: 'each', category_name: 'Test Category', master_item_id: 1 }], - }; - const mockImagePaths = [{ path: '/tmp/flyer.jpg', mimetype: 'image/jpeg' }]; - const mockJobData = { - originalFileName: 'flyer.jpg', - checksum: 'checksum-123', - userId: 'user-abc', - }; - - // The transformer is already spied on in beforeEach, we can just check its call. - const transformerSpy = vi.spyOn(FlyerDataTransformer.prototype, 'transform'); - - // The DB create function is also mocked in beforeEach. - const mockNewFlyer = { flyer_id: 1, file_name: 'flyer.jpg', store_name: 'Test Store' }; - vi.mocked(createFlyerAndItems).mockResolvedValue({ flyer: mockNewFlyer, items: [] } as any); - - // Act: Access and call the private method for testing - const result = await (service as any)._saveProcessedFlyerData(mockExtractedData, mockImagePaths, mockJobData); - - // Assert - // 1. Transformer was called correctly - expect(transformerSpy).toHaveBeenCalledWith(mockExtractedData, mockImagePaths, mockJobData.originalFileName, mockJobData.checksum, mockJobData.userId); - - // 2. DB function was called with the transformed data - const transformedData = await transformerSpy.mock.results[0].value; - expect(createFlyerAndItems).toHaveBeenCalledWith(transformedData.flyerData, transformedData.itemsForDb); - - // 3. Activity was logged - expect(mockedDb.adminRepo.logActivity).toHaveBeenCalledWith(expect.objectContaining({ - action: 'flyer_processed', - details: { flyerId: mockNewFlyer.flyer_id, storeName: mockNewFlyer.store_name } - })); - - // 4. The method returned the new flyer - expect(result).toEqual(mockNewFlyer); - }); - }); -}); \ No newline at end of file +}); diff --git a/src/services/flyerProcessingService.types.ts b/src/services/flyerProcessingService.types.ts deleted file mode 100644 index b29c902a..00000000 --- a/src/services/flyerProcessingService.types.ts +++ /dev/null @@ -1,9 +0,0 @@ -// src/services/flyerProcessingService.types.ts -export interface FlyerJobData { - filePath: string; - originalFileName: string; - checksum: string; - userId?: string; - submitterIp?: string; - userProfileAddress?: string; -} \ No newline at end of file