unit test fixes

src/services/aiApiClient.test.ts

@@ -19,6 +19,7 @@ vi.mock('./logger.client', () => ({
     debug: vi.fn(),
     info: vi.fn(),
     error: vi.fn(),
+    warn: vi.fn(),
   },
 }));
 

src/services/aiApiClient.ts

@@ -12,7 +12,7 @@ import type {
   GroundedResponse,
 } from '../types';
 import { logger } from './logger.client';
-import { apiFetch } from './apiClient';
+import { apiFetch, authedGet, authedPost, authedPostForm } from './apiClient';
 
 /**
  * Uploads a flyer file to the backend to be processed asynchronously.
@@ -33,14 +33,7 @@ export const uploadAndProcessFlyer = async (
 
   logger.info(`[aiApiClient] Starting background processing for file: ${file.name}`);
 
-  const response = await apiFetch(
-    '/ai/upload-and-process',
-    {
-      method: 'POST',
-      body: formData,
-    },
-    { tokenOverride },
-  );
+  const response = await authedPostForm('/ai/upload-and-process', formData, { tokenOverride });
 
   if (!response.ok) {
     let errorBody;
@@ -101,7 +94,7 @@ export const getJobStatus = async (
   jobId: string,
   tokenOverride?: string,
 ): Promise<JobStatus> => {
-  const response = await apiFetch(`/ai/jobs/${jobId}/status`, {}, { tokenOverride });
+  const response = await authedGet(`/ai/jobs/${jobId}/status`, { tokenOverride });
 
   // Handle non-OK responses first, as they might not have a JSON body.
   if (!response.ok) {
@@ -141,6 +134,10 @@ export const getJobStatus = async (
 
     return statusData;
   } catch (error) {
+    // If it's the specific error we threw, just re-throw it.
+    if (error instanceof JobFailedError) {
+      throw error;
+    }
     // This now primarily catches JSON parsing errors on an OK response, which is unexpected.
     logger.error('getJobStatus failed to parse a successful API response.', { error });
     throw new Error('Failed to parse job status from a successful API response.');
@@ -156,14 +153,7 @@ export const isImageAFlyer = (
 
   // Use apiFetchWithAuth for FormData to let the browser set the correct Content-Type.
   // The URL must be relative, as the helper constructs the full path.
-  return apiFetch(
-    '/ai/check-flyer',
-    {
-      method: 'POST',
-      body: formData,
-    },
-    { tokenOverride },
-  );
+  return authedPostForm('/ai/check-flyer', formData, { tokenOverride });
 };
 
 export const extractAddressFromImage = (
@@ -173,14 +163,7 @@ export const extractAddressFromImage = (
   const formData = new FormData();
   formData.append('image', imageFile);
 
-  return apiFetch(
-    '/ai/extract-address',
-    {
-      method: 'POST',
-      body: formData,
-    },
-    { tokenOverride },
-  );
+  return authedPostForm('/ai/extract-address', formData, { tokenOverride });
 };
 
 export const extractLogoFromImage = (
@@ -192,14 +175,7 @@ export const extractLogoFromImage = (
     formData.append('images', file);
   });
 
-  return apiFetch(
-    '/ai/extract-logo',
-    {
-      method: 'POST',
-      body: formData,
-    },
-    { tokenOverride },
-  );
+  return authedPostForm('/ai/extract-logo', formData, { tokenOverride });
 };
 
 export const getQuickInsights = (
@@ -207,16 +183,7 @@ export const getQuickInsights = (
   signal?: AbortSignal,
   tokenOverride?: string,
 ): Promise<Response> => {
-  return apiFetch(
-    '/ai/quick-insights',
-    {
-      method: 'POST',
-      headers: { 'Content-Type': 'application/json' },
-      body: JSON.stringify({ items }),
-      signal,
-    },
-    { tokenOverride, signal },
-  );
+  return authedPost('/ai/quick-insights', { items }, { tokenOverride, signal });
 };
 
 export const getDeepDiveAnalysis = (
@@ -224,16 +191,7 @@ export const getDeepDiveAnalysis = (
   signal?: AbortSignal,
   tokenOverride?: string,
 ): Promise<Response> => {
-  return apiFetch(
-    '/ai/deep-dive',
-    {
-      method: 'POST',
-      headers: { 'Content-Type': 'application/json' },
-      body: JSON.stringify({ items }),
-      signal,
-    },
-    { tokenOverride, signal },
-  );
+  return authedPost('/ai/deep-dive', { items }, { tokenOverride, signal });
 };
 
 export const searchWeb = (
@@ -241,16 +199,7 @@ export const searchWeb = (
   signal?: AbortSignal,
   tokenOverride?: string,
 ): Promise<Response> => {
-  return apiFetch(
-    '/ai/search-web',
-    {
-      method: 'POST',
-      headers: { 'Content-Type': 'application/json' },
-      body: JSON.stringify({ query }),
-      signal,
-    },
-    { tokenOverride, signal },
-  );
+  return authedPost('/ai/search-web', { query }, { tokenOverride, signal });
 };
 
 // ============================================================================
@@ -265,15 +214,7 @@ export const planTripWithMaps = async (
   tokenOverride?: string,
 ): Promise<Response> => {
   logger.debug('Stub: planTripWithMaps called with location:', { userLocation });
-  return apiFetch(
-    '/ai/plan-trip',
-    {
-      method: 'POST',
-      headers: { 'Content-Type': 'application/json' },
-      body: JSON.stringify({ items, store, userLocation }),
-    },
-    { signal, tokenOverride },
-  );
+  return authedPost('/ai/plan-trip', { items, store, userLocation }, { signal, tokenOverride });
 };
 
 /**
@@ -287,16 +228,7 @@ export const generateImageFromText = (
   tokenOverride?: string,
 ): Promise<Response> => {
   logger.debug('Stub: generateImageFromText called with prompt:', { prompt });
-  return apiFetch(
-    '/ai/generate-image',
-    {
-      method: 'POST',
-      headers: { 'Content-Type': 'application/json' },
-      body: JSON.stringify({ prompt }),
-      signal,
-    },
-    { tokenOverride, signal },
-  );
+  return authedPost('/ai/generate-image', { prompt }, { tokenOverride, signal });
 };
 
 /**
@@ -310,16 +242,7 @@ export const generateSpeechFromText = (
   tokenOverride?: string,
 ): Promise<Response> => {
   logger.debug('Stub: generateSpeechFromText called with text:', { text });
-  return apiFetch(
-    '/ai/generate-speech',
-    {
-      method: 'POST',
-      headers: { 'Content-Type': 'application/json' },
-      body: JSON.stringify({ text }),
-      signal,
-    },
-    { tokenOverride, signal },
-  );
+  return authedPost('/ai/generate-speech', { text }, { tokenOverride, signal });
 };
 
 /**
@@ -372,11 +295,7 @@ export const rescanImageArea = (
   formData.append('cropArea', JSON.stringify(cropArea));
   formData.append('extractionType', extractionType);
 
-  return apiFetch(
-    '/ai/rescan-area',
-    { method: 'POST', body: formData },
-    { tokenOverride },
-  );
+  return authedPostForm('/ai/rescan-area', formData, { tokenOverride });
 };
 
 /**
@@ -390,12 +309,5 @@ export const compareWatchedItemPrices = (
 ): Promise<Response> => {
   // Use the apiFetch wrapper for consistency with other API calls in this file.
   // This centralizes token handling and base URL logic.
-  return apiFetch(
-    '/ai/compare-prices',
-    {
-      method: 'POST',
-      headers: { 'Content-Type': 'application/json' },
-      body: JSON.stringify({ items: watchedItems }),
-    },
-    { signal },
-  )};
+  return authedPost('/ai/compare-prices', { items: watchedItems }, { signal });
+};

src/services/apiClient.test.ts

@@ -7,6 +7,17 @@ import { http, HttpResponse } from 'msw';
 vi.unmock('./apiClient');
 
 import * as apiClient from './apiClient';
+import {
+  createMockAddressPayload,
+  createMockBudget,
+  createMockLoginPayload,
+  createMockProfileUpdatePayload,
+  createMockRecipeCommentPayload,
+  createMockRegisterUserPayload,
+  createMockSearchQueryPayload,
+  createMockShoppingListItemPayload,
+  createMockWatchedItemPayload,
+} from '../tests/utils/mockFactories';
 
 // Mock the logger to keep test output clean and verifiable.
 vi.mock('./logger', () => ({
@@ -229,33 +240,6 @@ describe('API Client', () => {
     });
   });
 
-  describe('Analytics API Functions', () => {
-    it('trackFlyerItemInteraction should log a warning on failure', async () => {
-      const { logger } = await import('./logger.client');
-      const apiError = new Error('Network failed');
-      vi.mocked(global.fetch).mockRejectedValue(apiError);
-
-      // We can now await this properly because we added 'return' in apiClient.ts
-      await apiClient.trackFlyerItemInteraction(123, 'click');
-      expect(logger.warn).toHaveBeenCalledWith('Failed to track flyer item interaction', {
-        error: apiError,
-      });
-    });
-
-    it('logSearchQuery should log a warning on failure', async () => {
-      const { logger } = await import('./logger.client');
-      const apiError = new Error('Network failed');
-      vi.mocked(global.fetch).mockRejectedValue(apiError);
-
-      await apiClient.logSearchQuery({
-        query_text: 'test',
-        result_count: 0,
-        was_successful: false,
-      });
-      expect(logger.warn).toHaveBeenCalledWith('Failed to log search query', { error: apiError });
-    });
-  });
-
   describe('apiFetch (with FormData)', () => {
     it('should handle FormData correctly by not setting Content-Type', async () => {
       localStorage.setItem('authToken', 'form-data-token');
@@ -317,10 +301,11 @@ describe('API Client', () => {
     });
 
     it('addWatchedItem should send a POST request with the correct body', async () => {
-      await apiClient.addWatchedItem('Apples', 'Produce');
+      const watchedItemData = createMockWatchedItemPayload({ itemName: 'Apples', category: 'Produce' });
+      await apiClient.addWatchedItem(watchedItemData.itemName, watchedItemData.category);
 
       expect(capturedUrl?.pathname).toBe('/api/users/watched-items');
-      expect(capturedBody).toEqual({ itemName: 'Apples', category: 'Produce' });
+      expect(capturedBody).toEqual(watchedItemData);
     });
 
     it('removeWatchedItem should send a DELETE request to the correct URL', async () => {
@@ -337,12 +322,12 @@ describe('API Client', () => {
     });
 
     it('createBudget should send a POST request with budget data', async () => {
-      const budgetData = {
+      const budgetData = createMockBudget({
         name: 'Groceries',
         amount_cents: 50000,
-        period: 'monthly' as const,
+        period: 'monthly',
         start_date: '2024-01-01',
-      };
+      });
       await apiClient.createBudget(budgetData);
 
       expect(capturedUrl?.pathname).toBe('/api/budgets');
@@ -461,7 +446,7 @@ describe('API Client', () => {
 
     it('addShoppingListItem should send a POST request with item data', async () => {
       const listId = 42;
-      const itemData = { customItemName: 'Paper Towels' };
+      const itemData = createMockShoppingListItemPayload({ customItemName: 'Paper Towels' });
       await apiClient.addShoppingListItem(listId, itemData);
 
       expect(capturedUrl?.pathname).toBe(`/api/users/shopping-lists/${listId}/items`);
@@ -547,7 +532,7 @@ describe('API Client', () => {
 
     it('addRecipeComment should send a POST request with content and optional parentId', async () => {
       const recipeId = 456;
-      const commentData = { content: 'This is a reply', parentCommentId: 789 };
+      const commentData = createMockRecipeCommentPayload({ content: 'This is a reply', parentCommentId: 789 });
       await apiClient.addRecipeComment(recipeId, commentData.content, commentData.parentCommentId);
       expect(capturedUrl?.pathname).toBe(`/api/recipes/${recipeId}/comments`);
       expect(capturedBody).toEqual(commentData);
@@ -563,7 +548,7 @@ describe('API Client', () => {
   describe('User Profile and Settings API Functions', () => {
     it('updateUserProfile should send a PUT request with profile data', async () => {
       localStorage.setItem('authToken', 'user-settings-token');
-      const profileData = { full_name: 'John Doe' };
+      const profileData = createMockProfileUpdatePayload({ full_name: 'John Doe' });
       await apiClient.updateUserProfile(profileData, { tokenOverride: 'override-token' });
       expect(capturedUrl?.pathname).toBe('/api/users/profile');
       expect(capturedBody).toEqual(profileData);
@@ -619,14 +604,14 @@ describe('API Client', () => {
     });
 
     it('registerUser should send a POST request with user data', async () => {
-      await apiClient.registerUser('test@example.com', 'password123', 'Test User');
-      expect(capturedUrl?.pathname).toBe('/api/auth/register');
-      expect(capturedBody).toEqual({
+      const userData = createMockRegisterUserPayload({
         email: 'test@example.com',
         password: 'password123',
         full_name: 'Test User',
-        avatar_url: undefined,
       });
+      await apiClient.registerUser(userData.email, userData.password, userData.full_name);
+      expect(capturedUrl?.pathname).toBe('/api/auth/register');
+      expect(capturedBody).toEqual(userData);
     });
 
     it('deleteUserAccount should send a DELETE request with the confirmation password', async () => {
@@ -654,7 +639,7 @@ describe('API Client', () => {
     });
 
     it('updateUserAddress should send a PUT request with address data', async () => {
-      const addressData = { address_line_1: '123 Main St', city: 'Anytown' };
+      const addressData = createMockAddressPayload({ address_line_1: '123 Main St', city: 'Anytown' });
       await apiClient.updateUserAddress(addressData);
       expect(capturedUrl?.pathname).toBe('/api/users/profile/address');
       expect(capturedBody).toEqual(addressData);
@@ -942,53 +927,49 @@ describe('API Client', () => {
     });
 
     it('logSearchQuery should send a POST request with query data', async () => {
-      const queryData = { query_text: 'apples', result_count: 10, was_successful: true };
+      const queryData = createMockSearchQueryPayload({ query_text: 'apples', result_count: 10, was_successful: true });
       await apiClient.logSearchQuery(queryData);
       expect(capturedUrl?.pathname).toBe('/api/search/log');
       expect(capturedBody).toEqual(queryData);
     });
 
     it('trackFlyerItemInteraction should log a warning on failure', async () => {
-      const { logger } = await import('./logger.client');
       const apiError = new Error('Network failed');
       vi.mocked(global.fetch).mockRejectedValue(apiError);
+      const { logger } = await import('./logger.client');
 
       // We can now await this properly because we added 'return' in apiClient.ts
       await apiClient.trackFlyerItemInteraction(123, 'click');
       expect(logger.warn).toHaveBeenCalledWith('Failed to track flyer item interaction', {
         error: apiError,
       });
-
-      expect(logger.warn).toHaveBeenCalledWith('Failed to track flyer item interaction', {
-        error: apiError,
-      });
     });
 
     it('logSearchQuery should log a warning on failure', async () => {
-      const { logger } = await import('./logger.client');
       const apiError = new Error('Network failed');
       vi.mocked(global.fetch).mockRejectedValue(apiError);
+      const { logger } = await import('./logger.client');
 
-      await apiClient.logSearchQuery({
+      const queryData = createMockSearchQueryPayload({
         query_text: 'test',
         result_count: 0,
         was_successful: false,
       });
-      expect(logger.warn).toHaveBeenCalledWith('Failed to log search query', { error: apiError });
-
+      await apiClient.logSearchQuery(queryData);
       expect(logger.warn).toHaveBeenCalledWith('Failed to log search query', { error: apiError });
     });
   });
 
   describe('Authentication API Functions', () => {
     it('loginUser should send a POST request with credentials', async () => {
-      await apiClient.loginUser('test@example.com', 'password123', true);
-      expect(capturedUrl?.pathname).toBe('/api/auth/login');
-      expect(capturedBody).toEqual({
+      const loginData = createMockLoginPayload({
         email: 'test@example.com',
         password: 'password123',
         rememberMe: true,
       });
+      await apiClient.loginUser(loginData.email, loginData.password, loginData.rememberMe);
+      expect(capturedUrl?.pathname).toBe('/api/auth/login');
+      expect(capturedBody).toEqual(loginData);
     });
   });
 

src/services/flyerDataTransformer.test.ts

@@ -109,9 +109,6 @@ describe('FlyerDataTransformer', () => {
         view_count: 0,
         click_count: 0,
       }),
-    ); // Use a more specific type assertion to check for the added property.
-    expect((itemsForDb[0] as FlyerItemInsert & { updated_at: string }).updated_at).toBeTypeOf(
-      'string',
     );
 
     // 3. Check that generateFlyerIcon was called correctly

src/services/flyerProcessingService.server.ts

@@ -8,6 +8,7 @@ import { createFlyerAndItems } from './db/flyer.db';
 import {
   AiDataValidationError,
   UnsupportedFileTypeError,
+  FlyerProcessingError,
   PdfConversionError,
 } from './processingErrors';
 import { FlyerDataTransformer } from './flyerDataTransformer';
@@ -163,25 +164,25 @@ export class FlyerProcessingService {
     let errorPayload: { errorCode: string; message: string; [key: string]: any };
 
     // Handle our custom, structured processing errors.
-    if (error instanceof UnsupportedFileTypeError) {
+    if (wrappedError instanceof FlyerProcessingError) {
       // Use the properties from the custom error itself.
-      errorPayload = error.toErrorPayload();
+      errorPayload = wrappedError.toErrorPayload();
       // Log with specific details based on the error type
-      if (error instanceof AiDataValidationError) {
+      if (wrappedError instanceof AiDataValidationError) {
         logger.error(
-          { err: error, validationErrors: error.validationErrors, rawData: error.rawData },
+          { err: wrappedError, validationErrors: wrappedError.validationErrors, rawData: wrappedError.rawData },
           `AI Data Validation failed.`,
         );
-      } else if (error instanceof PdfConversionError) {
-        logger.error({ err: error, stderr: error.stderr }, `PDF Conversion failed.`);
+      } else if (wrappedError instanceof PdfConversionError) {
+        logger.error({ err: wrappedError, stderr: wrappedError.stderr }, `PDF Conversion failed.`);
       } else {
         // Generic log for other FlyerProcessingErrors like UnsupportedFileTypeError
-        logger.error({ err: error }, `${error.name} occurred during processing.`);
+        logger.error({ err: wrappedError }, `${wrappedError.name} occurred during processing.`);
       }
     } else {
       // Handle generic/unknown errors.
       logger.error(
-        { err: error, attemptsMade: job.attemptsMade, totalAttempts: job.opts.attempts },
+        { err: wrappedError, attemptsMade: job.attemptsMade, totalAttempts: job.opts.attempts },
         `A generic error occurred in job.`,
       );
       errorPayload = {

src/services/queueService.workers.test.ts

@@ -73,10 +73,12 @@ vi.mock('bullmq', () => ({
 vi.mock('./flyerProcessingService.server', () => {
   // Mock the constructor to return an object with the mocked methods
   return {
-    FlyerProcessingService: vi.fn().mockImplementation(() => ({
-      processJob: mocks.processFlyerJob,
-      processCleanupJob: mocks.processCleanupJob,
-    })),
+    FlyerProcessingService: vi.fn().mockImplementation(function () {
+      return {
+        processJob: mocks.processFlyerJob,
+        processCleanupJob: mocks.processCleanupJob,
+      };
+    }),
   };
 });
 

src/services/workers.server.test.ts

@@ -80,10 +80,12 @@ vi.mock('bullmq', () => ({
 vi.mock('./flyerProcessingService.server', () => {
   // Mock the constructor to return an object with the mocked methods
   return {
-    FlyerProcessingService: vi.fn().mockImplementation(() => ({
-      processJob: mocks.processFlyerJob,
-      processCleanupJob: mocks.processCleanupJob,
-    })),
+    FlyerProcessingService: vi.fn().mockImplementation(function () {
+      return {
+        processJob: mocks.processFlyerJob,
+        processCleanupJob: mocks.processCleanupJob,
+      };
+    }),
   };
 });
 

src/tests/utils/mockFactories.ts

@@ -39,6 +39,7 @@ import {
   ShoppingTripItem,
   Receipt,
   ReceiptItem,
+  SearchQuery,
   ProcessingStage,
   UserAlert,
   UserSubmittedPrice,
@@ -1451,3 +1452,66 @@ export const createMockAppliance = (overrides: Partial<Appliance> = {}): Applian
     ...overrides,
   };
 };
+
+// src/tests/utils/mockFactories.ts
+
+// ... existing factories
+
+export const createMockShoppingListItemPayload = (overrides: Partial<{ masterItemId: number; customItemName: string }> = {}): { masterItemId?: number; customItemName?: string } => ({
+  customItemName: 'Mock Item',
+  ...overrides,
+});
+
+export const createMockRecipeCommentPayload = (overrides: Partial<{ content: string; parentCommentId: number }> = {}): { content: string; parentCommentId?: number } => ({
+  content: 'This is a mock comment.',
+  ...overrides,
+});
+
+export const createMockProfileUpdatePayload = (overrides: Partial<Profile> = {}): Partial<Profile> => ({
+  full_name: 'Mock User',
+  ...overrides,
+});
+
+export const createMockAddressPayload = (overrides: Partial<Address> = {}): Partial<Address> => ({
+  address_line_1: '123 Mock St',
+  city: 'Mockville',
+  province_state: 'MS',
+  postal_code: '12345',
+  country: 'Mockland',
+  ...overrides,
+});
+
+export const createMockSearchQueryPayload = (overrides: Partial<Omit<SearchQuery, 'search_query_id' | 'id' | 'created_at' | 'user_id'>> = {}): Omit<SearchQuery, 'search_query_id' | 'id' | 'created_at' | 'user_id'> => ({
+  query_text: 'mock search',
+  result_count: 5,
+  was_successful: true,
+  ...overrides,
+});
+
+export const createMockWatchedItemPayload = (overrides: Partial<{ itemName: string; category: string }> = {}): { itemName: string; category: string } => ({
+  itemName: 'Mock Watched Item',
+  category: 'Pantry',
+  ...overrides,
+});
+
+export const createMockRegisterUserPayload = (
+  overrides: Partial<{
+    email: string;
+    password: string;
+    full_name: string;
+    avatar_url: string | undefined;
+  }> = {},
+) => ({
+  email: 'mock@example.com',
+  password: 'password123',
+  full_name: 'Mock User',
+  avatar_url: undefined,
+  ...overrides,
+});
+
+export const createMockLoginPayload = (overrides: Partial<{ email: string; password: string; rememberMe: boolean }> = {}) => ({
+  email: 'mock@example.com',
+  password: 'password123',
+  rememberMe: false,
+  ...overrides,
+});