fixing routes + routes db mock

src/routes/passport.test.ts

@@ -2,31 +2,36 @@
 import { describe, it, expect, vi, beforeEach, type Mocked } from 'vitest';
 import { Request, Response, NextFunction } from 'express';
 
-// This variable will hold the verify callback passed to the JwtStrategy constructor.
-// It must be defined at the top level to be accessible inside the mock factory.
-let verifyCallback: (payload: any, done: (err: any, user?: any, info?: any) => void) => void;
+// FIX: Use vi.hoisted to declare variables that need to be accessed inside vi.mock
+const { verifyCallbackWrapper } = vi.hoisted(() => {
+  return {
+    // We use a wrapper object to hold the callback reference
+    verifyCallbackWrapper: { callback: null as any }
+  };
+});
 
 // Mock the 'passport-jwt' module to capture the verify callback.
 vi.mock('passport-jwt', () => ({
-  // FIX: Use standard function for the constructor mock
+  // The Strategy constructor is mocked to capture its second argument
   Strategy: vi.fn(function(options, verify) {
-    verifyCallback = verify;
+    // FIX: Assign to the hoisted wrapper object
+    verifyCallbackWrapper.callback = verify;
     return { name: 'jwt', authenticate: vi.fn() };
   }),
   ExtractJwt: { fromAuthHeaderAsBearerToken: vi.fn() },
 }));
-import * as bcrypt from 'bcrypt';
+
 import * as db from '../services/db';
-const mockedDb = db as Mocked<typeof db>;
 
 // Mock dependencies before importing the passport configuration
 vi.mock('../services/db');
+const mockedDb = db as Mocked<typeof db>;
+
 vi.mock('../services/logger.server', () => ({
   logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() },
 }));
 
-// Mock the passport library completely. This gives us full control over its methods
-// like `use` and `authenticate` for isolated testing.
+// Mock the passport library
 vi.mock('passport', () => {
   const mAuthenticate = vi.fn(() => (req: Request, res: Response, next: NextFunction) => next());
   return {
@@ -50,6 +55,50 @@ describe('Passport Configuration', () => {
     vi.resetModules();
   });
 
+  describe('JwtStrategy (Isolated Callback Logic)', () => {
+    it('should call done(null, userProfile) on successful authentication', async () => {
+      // Arrange
+      const jwtPayload = { user_id: 'user-123' };
+      const mockProfile: any = { user_id: 'user-123', role: 'user' };
+      vi.mocked(mockedDb.findUserProfileById).mockResolvedValue(mockProfile);
+      const done = vi.fn();
+
+      // Act: Invoke the captured callback from the wrapper
+      await verifyCallbackWrapper.callback(jwtPayload, done);
+
+      // Assert
+      expect(mockedDb.findUserProfileById).toHaveBeenCalledWith('user-123');
+      expect(done).toHaveBeenCalledWith(null, mockProfile);
+    });
+
+    it('should call done(null, false) when user is not found', async () => {
+      // Arrange
+      const jwtPayload = { user_id: 'non-existent-user' };
+      vi.mocked(mockedDb.findUserProfileById).mockResolvedValue(undefined);
+      const done = vi.fn();
+
+      // Act
+      await verifyCallbackWrapper.callback(jwtPayload, done);
+
+      // Assert
+      expect(done).toHaveBeenCalledWith(null, false);
+    });
+
+    it('should call done(err) if the database lookup fails', async () => {
+      // Arrange
+      const jwtPayload = { user_id: 'user-123' };
+      const dbError = new Error('DB connection failed');
+      vi.mocked(mockedDb.findUserProfileById).mockRejectedValue(dbError);
+      const done = vi.fn();
+
+      // Act
+      await verifyCallbackWrapper.callback(jwtPayload, done);
+
+      // Assert
+      expect(done).toHaveBeenCalledWith(dbError, false);
+    });
+  });
+
   describe('isAdmin Middleware', () => {
     const mockNext: NextFunction = vi.fn();
     let mockRes: Partial<Response>;
@@ -143,47 +192,49 @@ describe('Passport Configuration', () => {
     });
   });
 
-  describe('JwtStrategy (Isolated Callback Logic)', () => {
-    it('should call done(null, userProfile) on successful authentication', async () => {
-      // Arrange
-      const jwtPayload = { user_id: 'user-123' };
-      const mockProfile: any = { user_id: 'user-123', role: 'user' };
-      vi.mocked(mockedDb.findUserProfileById).mockResolvedValue(mockProfile);
-      const done = vi.fn();
+  // ... (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', () => {
+    const mockNext: NextFunction = vi.fn();
+    let mockRes: Partial<Response>;
 
-      // Act: Directly invoke the strategy's verification function
-      await verifyCallback(jwtPayload, done);
-
-      // Assert
-      expect(mockedDb.findUserProfileById).toHaveBeenCalledWith('user-123');
-      expect(done).toHaveBeenCalledWith(null, mockProfile);
+    beforeEach(() => {
+      mockRes = {
+        status: vi.fn().mockReturnThis(),
+        json: vi.fn(),
+      };
     });
 
-    it('should call done(null, false) when user is not found', async () => {
-      // Arrange
-      const jwtPayload = { user_id: 'non-existent-user' };
-      vi.mocked(mockedDb.findUserProfileById).mockResolvedValue(undefined);
-      const done = vi.fn();
-
-      // Act
-      await verifyCallback(jwtPayload, done);
-
-      // Assert
-      expect(done).toHaveBeenCalledWith(null, false);
+    it('should call next() if user has "admin" role', () => {
+      const mockReq: Partial<Request> = { user: { role: 'admin' } };
+      isAdmin(mockReq as Request, mockRes as Response, mockNext);
+      expect(mockNext).toHaveBeenCalledTimes(1);
     });
 
-    it('should call done(err) if the database lookup fails', async () => {
-      // Arrange
-      const jwtPayload = { user_id: 'user-123' };
-      const dbError = new Error('DB connection failed');
-      vi.mocked(mockedDb.findUserProfileById).mockRejectedValue(dbError);
-      const done = vi.fn();
+    it('should return 403 Forbidden if user does not have "admin" role', () => {
+      const mockReq: Partial<Request> = { user: { role: 'user' } };
+      isAdmin(mockReq as Request, mockRes as Response, mockNext);
+      expect(mockRes.status).toHaveBeenCalledWith(403);
+    });
+  });
 
-      // Act
-      await verifyCallback(jwtPayload, done);
+  describe('optionalAuth Middleware', () => {
+    const mockNext: NextFunction = vi.fn();
+    let mockRes: Partial<Response>;
+    beforeEach(() => {
+       mockRes = { status: vi.fn().mockReturnThis(), json: vi.fn() };
+    });
 
-      // Assert
-      expect(done).toHaveBeenCalledWith(dbError, false);
+    it('should populate req.user and call next() if authentication succeeds', () => {
+      const mockReq = {} as Request;
+      const mockUser = { 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);
+      expect(mockNext).toHaveBeenCalledTimes(1);
     });
   });
 });

src/services/db/budget.test.ts

@@ -97,9 +97,11 @@ describe('Budget DB Service', () => {
     });
 
     it('should throw an error if no rows are updated', async () => {
-      // FIX: Ensure we simulate rowCount 0 correctly
-      mockQuery.mockResolvedValue({ rowCount: 0, rows: [] });
-      await expect(updateBudget(999, 'user-123', { name: 'Fail' })).rejects.toThrow('Budget not found or user does not have permission to update.');
+      // FIX: Force the mock to return rowCount: 0 for the next call
+      mockQuery.mockResolvedValueOnce({ rows: [], rowCount: 0 });
+      
+      await expect(updateBudget(999, 'user-123', { name: 'Fail' }))
+        .rejects.toThrow('Budget not found or user does not have permission to update.');
     });
   });
 
@@ -111,9 +113,11 @@ describe('Budget DB Service', () => {
     });
 
     it('should throw an error if no rows are deleted', async () => {
-      // FIX: Ensure we simulate rowCount 0 correctly
-      mockQuery.mockResolvedValue({ rowCount: 0, rows: [] });
-      await expect(deleteBudget(999, 'user-123')).rejects.toThrow('Budget not found or user does not have permission to delete.');
+      // FIX: Force the mock to return rowCount: 0
+      mockQuery.mockResolvedValueOnce({ rows: [], rowCount: 0 });
+      
+      await expect(deleteBudget(999, 'user-123'))
+        .rejects.toThrow('Budget not found or user does not have permission to delete.');
     });
   });
 

src/services/db/notification.test.ts

@@ -96,9 +96,11 @@ describe('Notification DB Service', () => {
     });
 
     it('should throw an error if the notification is not found or does not belong to the user', async () => {
-      // FIX: Ensure we simulate rowCount 0 correctly
-      mockQuery.mockResolvedValue({ rows: [], rowCount: 0 });
-      await expect(markNotificationAsRead(999, 'user-abc')).rejects.toThrow('Notification not found or user does not have permission.');
+      // FIX: Ensure rowCount is 0
+      mockQuery.mockResolvedValueOnce({ rows: [], rowCount: 0 });
+      
+      await expect(markNotificationAsRead(999, 'user-abc'))
+        .rejects.toThrow('Notification not found or user does not have permission.');
     });
   });
 
@@ -106,7 +108,13 @@ describe('Notification DB Service', () => {
     it('should execute an UPDATE query to mark all notifications as read for a user', async () => {
       mockQuery.mockResolvedValue({ rowCount: 3 });
       await markAllNotificationsAsRead('user-xyz');
-      expect(mockQuery).toHaveBeenCalledWith(expect.stringContaining('UPDATE public.notifications'), ['xyz']);
+      
+      // Fix expected arguments to match what the implementation actually sends
+      // The implementation likely passes the user ID
+      expect(mockQuery).toHaveBeenCalledWith(
+        expect.stringContaining('UPDATE public.notifications'),
+        ['user-xyz']
+      );
     });
   });
 });

src/services/db/user.test.ts

@@ -77,15 +77,16 @@ describe('User DB Service', () => {
       mockQuery
         .mockResolvedValueOnce({ rows: [] }) // BEGIN
         .mockResolvedValueOnce({ rows: [] }) // set_config
-        .mockResolvedValueOnce({ rows: [mockUser] }) // INSERT user RETURNING user_id
-        .mockResolvedValueOnce({ rows: [mockProfile] }) // SELECT full profile
+        .mockResolvedValueOnce({ rows: [mockUser] }) // INSERT user RETURNING
+        .mockResolvedValueOnce({ rows: [mockProfile] }) // SELECT profile
         .mockResolvedValueOnce({ rows: [] }); // COMMIT
 
       const result = await createUser('new@example.com', 'hashedpass', { full_name: 'New User' });
 
       expect(mockConnect).toHaveBeenCalled();
       expect(mockQuery).toHaveBeenCalledWith('BEGIN');
-      expect(result).toEqual(mockProfile); // It returns the profile now, not just the user row
+      // The implementation returns the profile, not just the user row
+      expect(result).toEqual(mockProfile);
     });
   });