moar fixes + unit test review of routes

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

@@ -7,16 +7,7 @@ import { createMockUserProfile, createMockSuggestedCorrection, createMockBrand,
 import { SuggestedCorrection, Brand, UserProfile, UnmatchedFlyerItem } from '../types';
 import { NotFoundError } from '../services/db/errors.db';
 import { createTestApp } from '../tests/utils/createTestApp';
-
-const { mockLogger } = vi.hoisted(() => ({
-  mockLogger: {
-    info: vi.fn(),
-    debug: vi.fn(),
-    warn: vi.fn(),
-    error: vi.fn(),
-    child: vi.fn().mockReturnThis(),
-  },
-}));
+import { mockLogger } from '../tests/utils/mockLogger';
 
 vi.mock('../lib/queue', () => ({
   serverAdapter: {

src/routes/admin.jobs.routes.test.ts

@@ -7,18 +7,7 @@ import { createMockUserProfile } from '../tests/utils/mockFactories';
 import type { Job } from 'bullmq';
 import type { UserProfile } from '../types';
 import { createTestApp } from '../tests/utils/createTestApp';
-
-const { mockLogger } = vi.hoisted(() => ({
-  mockLogger: {
-    info: vi.fn(),
-    debug: vi.fn(),
-    warn: vi.fn(),
-    error: vi.fn(),
-    child: vi.fn().mockReturnThis(),
-  },
-}));
-
-// --- Mocks ---
+import { mockLogger } from '../tests/utils/mockLogger';
 
 // Mock the background job service to control its methods.
 vi.mock('../services/backgroundJobService', () => ({

src/routes/admin.monitoring.routes.test.ts

@@ -9,16 +9,8 @@ import {
 } from '../tests/utils/mockFactories';
 import { UserProfile } from '../types';
 import { createTestApp } from '../tests/utils/createTestApp';
+import { mockLogger } from '../tests/utils/mockLogger';
 
-const { mockLogger } = vi.hoisted(() => ({
-  mockLogger: {
-    info: vi.fn(),
-    debug: vi.fn(),
-    warn: vi.fn(),
-    error: vi.fn(),
-    child: vi.fn().mockReturnThis(),
-  },
-}));
 vi.mock('../lib/queue', () => ({
   serverAdapter: {
     getRouter: () => (req: Request, res: Response, next: NextFunction) => next(), // Return a dummy express handler

src/routes/admin.stats.routes.test.ts

@@ -6,16 +6,7 @@ import adminRouter from './admin.routes';
 import { createMockUserProfile } from '../tests/utils/mockFactories';
 import { UserProfile } from '../types';
 import { createTestApp } from '../tests/utils/createTestApp';
-
-const { mockLogger } = vi.hoisted(() => ({
-  mockLogger: {
-    info: vi.fn(),
-    debug: vi.fn(),
-    warn: vi.fn(),
-    error: vi.fn(),
-    child: vi.fn().mockReturnThis(),
-  },
-}));
+import { mockLogger } from '../tests/utils/mockLogger';
 
 vi.mock('../services/db/index.db', () => ({
   adminRepo: {

src/routes/admin.system.routes.test.ts

@@ -5,6 +5,7 @@ import { Request, Response, NextFunction } from 'express';
 import adminRouter from './admin.routes';
 import { createMockUserProfile } from '../tests/utils/mockFactories';
 import { createTestApp } from '../tests/utils/createTestApp';
+import { mockLogger } from '../tests/utils/mockLogger';
 
 // Mock dependencies
 vi.mock('../services/geocodingService.server', () => ({
@@ -46,7 +47,7 @@ import { geocodingService } from '../services/geocodingService.server';
 
 // Mock the logger
 vi.mock('../services/logger.server', () => ({
-  logger: { info: vi.fn(), debug: vi.fn(), error: vi.fn(), warn: vi.fn() },
+  logger: mockLogger,
 }));
 
 // Mock the passport middleware

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

@@ -7,16 +7,7 @@ import { createMockUserProfile, createMockAdminUserView } from '../tests/utils/m
 import { UserProfile, Profile } from '../types';
 import { NotFoundError } from '../services/db/errors.db';
 import { createTestApp } from '../tests/utils/createTestApp';
-
-const { mockLogger } = vi.hoisted(() => ({
-  mockLogger: {
-    info: vi.fn(),
-    debug: vi.fn(),
-    warn: vi.fn(),
-    error: vi.fn(),
-    child: vi.fn().mockReturnThis(),
-  },
-}));
+import { mockLogger } from '../tests/utils/mockLogger';
 
 vi.mock('../services/db/index.db', () => ({
   adminRepo: {

src/routes/ai.routes.test.ts

@@ -5,19 +5,10 @@ import { type Request, type Response, type NextFunction } from 'express';
 import path from 'node:path';
 import type { Job } from 'bullmq';
 import aiRouter from './ai.routes';
-import { createMockUserProfile, createMockFlyer } from '../tests/utils/mockFactories';
+import { createMockUserProfile, createMockFlyer, createMockAddress } from '../tests/utils/mockFactories';
 import * as aiService from '../services/aiService.server';
 import { createTestApp } from '../tests/utils/createTestApp';
-
-const { mockLogger } = vi.hoisted(() => ({
-  mockLogger: {
-    info: vi.fn(),
-    debug: vi.fn(),
-    warn: vi.fn(),
-    error: vi.fn(),
-    child: vi.fn().mockReturnThis(),
-  },
-}));
+import { mockLogger } from '../tests/utils/mockLogger';
 
 // Mock the AI service methods to avoid making real AI calls
 vi.mock('../services/aiService.server', () => ({
@@ -166,16 +157,17 @@ describe('AI Routes (/api/ai)', () => {
 
     it('should pass user profile address to the job when authenticated user has an address', async () => {
       // Arrange: Create a mock user with a complete address object
+      const mockAddress = createMockAddress({
+        address_id: 1,
+        address_line_1: '123 Pacific St',
+        city: 'Anytown',
+        province_state: 'BC',
+        postal_code: 'V8T 1A1',
+        country: 'CA',
+      });
       const mockUserWithAddress = createMockUserProfile({
         user_id: 'auth-user-2',
-        address: {
-          address_id: 1,
-          address_line_1: '123 Main St',
-          city: 'Anytown',
-          province_state: 'CA',
-          postal_code: '12345',
-          country: 'USA',
-        },
+        address: mockAddress,
       });
       const authenticatedApp = createTestApp({ router: aiRouter, basePath: '/api/ai', authenticatedUser: mockUserWithAddress });
 

src/routes/gamification.routes.test.ts

@@ -5,6 +5,7 @@ import { Request, Response, NextFunction } from 'express';
 import gamificationRouter from './gamification.routes';
 import * as db from '../services/db/index.db';
 import { createMockUserProfile, createMockAchievement, createMockUserAchievement, createMockLeaderboardUser } from '../tests/utils/mockFactories';
+import { mockLogger } from '../tests/utils/mockLogger';
 import { ForeignKeyConstraintError } from '../services/db/errors.db';
 import { createTestApp } from '../tests/utils/createTestApp';
 
@@ -20,12 +21,7 @@ vi.mock('../services/db/index.db', () => ({
 
 // Mock the logger to keep test output clean
 vi.mock('../services/logger.server', () => ({
-  logger: {
-    info: vi.fn(),
-    debug: vi.fn(),
-    error: vi.fn(),
-    warn: vi.fn(),
-  },
+  logger: mockLogger,
 }));
 
 // Use vi.hoisted to create mutable mock function references.

src/routes/health.routes.test.ts

@@ -6,6 +6,7 @@ import * as dbConnection from '../services/db/connection.db';
 import { connection as redisConnection } from '../services/queueService.server';
 import fs from 'node:fs/promises';
 import { createTestApp } from '../tests/utils/createTestApp';
+import { mockLogger } from '../tests/utils/mockLogger';
 
 // 1. Mock the dependencies of the health router.
 vi.mock('../services/db/connection.db', () => ({
@@ -30,13 +31,7 @@ vi.mock('../services/queueService.server', () => ({
 
 // Mock the logger to keep test output clean.
 vi.mock('../services/logger.server', () => ({
-  logger: {
-    info: vi.fn(),
-    debug: vi.fn(),
-    error: vi.fn(),
-    warn: vi.fn(),
-    child: vi.fn().mockReturnThis(), // Add child mock for req.log
-  },
+  logger: mockLogger,
 }));
 
 // Cast the mocked import to a Mocked type for type-safe access to mock functions.

src/routes/passport.routes.test.ts

@@ -45,6 +45,7 @@ vi.mock('passport-local', () => ({
 import * as db from '../services/db/index.db';
 import { UserProfile } from '../types';
 import { createMockUserProfile } from '../tests/utils/mockFactories';
+import { mockLogger } from '../tests/utils/mockLogger';
 
 // Mock dependencies before importing the passport configuration
 vi.mock('../services/db/index.db', () => ({
@@ -64,7 +65,7 @@ const mockedDb = db as Mocked<typeof db>;
 
 vi.mock('../services/logger.server', () => ({
   // This mock is used by the module under test and can be imported in the test file.
-  logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() },
+  logger: mockLogger,
 }));
 
 // Mock bcrypt for password comparisons

src/routes/price.routes.test.ts

@@ -3,21 +3,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest';
 import supertest from 'supertest';
 import priceRouter from './price.routes';
 import { createTestApp } from '../tests/utils/createTestApp';
-
-// Mock the logger to keep test output clean
-vi.mock('../services/logger.server', () => ({
-  logger: {
-    info: vi.fn(),
-    error: vi.fn(),
-    // The test app setup injects a child logger into `req.log`.
-    // We need to mock `child()` to return the mock logger itself
-    // so that `req.log.info()` calls `logger.info()`.
-    child: vi.fn().mockReturnThis(),
-  },
-}));
-
-// Import the mocked logger to make assertions on it
-import { logger } from '../services/logger.server';
+import { mockLogger } from '../tests/utils/mockLogger';
 
 describe('Price Routes (/api/price-history)', () => {
   const app = createTestApp({ router: priceRouter, basePath: '/api/price-history' });
@@ -34,7 +20,7 @@ describe('Price Routes (/api/price-history)', () => {
 
       expect(response.status).toBe(200);
       expect(response.body).toEqual([]);
-      expect(logger.info).toHaveBeenCalledWith(
+      expect(mockLogger.info).toHaveBeenCalledWith(
         { itemCount: masterItemIds.length },
         '[API /price-history] Received request for historical price data.'
       );

src/routes/system.routes.test.ts

@@ -5,6 +5,7 @@ import systemRouter from './system.routes'; // This was a duplicate, fixed.
 import { exec, type ExecException, type ExecOptions } from 'child_process';
 import { geocodingService } from '../services/geocodingService.server';
 import { createTestApp } from '../tests/utils/createTestApp';
+import { mockLogger } from '../tests/utils/mockLogger';
 
 // FIX: Use the simple factory pattern for child_process to avoid default export issues
 vi.mock('child_process', () => {
@@ -30,13 +31,7 @@ vi.mock('../services/geocodingService.server', () => ({
 
 // 3. Mock Logger
 vi.mock('../services/logger.server', () => ({
-  logger: {
-    info: vi.fn(),
-    debug: vi.fn(),
-    error: vi.fn(),
-    warn: vi.fn(),
-    child: vi.fn().mockReturnThis(),
-  },
+  logger: mockLogger,
 }));
 
 describe('System Routes (/api/system)', () => {

src/routes/user.routes.test.ts

@@ -5,10 +5,11 @@ import express from 'express';
 // Use * as bcrypt to match the implementation's import style and ensure mocks align.
 import * as bcrypt from 'bcrypt';
 import userRouter from './user.routes';
-import { createMockUserProfile, createMockMasterGroceryItem, createMockShoppingList, createMockShoppingListItem, createMockRecipe, createMockNotification, createMockDietaryRestriction, createMockAppliance, createMockUserWithPasswordHash } from '../tests/utils/mockFactories';
-import { Appliance, Notification, DietaryRestriction } from '../types';
+import { createMockUserProfile, createMockMasterGroceryItem, createMockShoppingList, createMockShoppingListItem, createMockRecipe, createMockNotification, createMockDietaryRestriction, createMockAppliance, createMockUserWithPasswordHash, createMockAddress } from '../tests/utils/mockFactories';
+import { Appliance, Notification, DietaryRestriction, Address } from '../types';
 import { ForeignKeyConstraintError, NotFoundError } from '../services/db/errors.db';
 import { createTestApp } from '../tests/utils/createTestApp';
+import { mockLogger } from '../tests/utils/mockLogger';
 
 import { logger } from '../services/logger.server';
 // 1. Mock the Service Layer directly.
@@ -77,13 +78,7 @@ vi.mock('bcrypt', () => {
 
 // Mock the logger
 vi.mock('../services/logger.server', () => ({
-  logger: {
-    info: vi.fn(),
-    debug: vi.fn(),
-    error: vi.fn(),
-    warn: vi.fn(),
-    child: vi.fn().mockReturnThis(),
-  },
+  logger: mockLogger,
 }));
 
 import { userService } from '../services/userService'; // Import for checking calls
@@ -795,8 +790,8 @@ describe('User Routes (/api/users)', () => {
     describe('Address Routes', () => {
       it('GET /addresses/:addressId should return the address if it belongs to the user', async () => {
         const appWithUser = createTestApp({ router: userRouter, basePath, authenticatedUser: { ...mockUserProfile, address_id: 1 } });
-        const mockAddress = { address_id: 1, address_line_1: '123 Main St' };
-        vi.mocked(db.addressRepo.getAddressById).mockResolvedValue(mockAddress as any);
+        const mockAddress = createMockAddress({ address_id: 1, address_line_1: '123 Main St' });
+        vi.mocked(db.addressRepo.getAddressById).mockResolvedValue(mockAddress);
         const response = await supertest(appWithUser).get('/api/users/addresses/1');
         expect(response.status).toBe(200);
         expect(response.body).toEqual(mockAddress);