ci: Bump version to 0.6.5 [skip ci]

package-lock.json

@@ -1,12 +1,12 @@
 {
   "name": "flyer-crawler",
-  "version": "0.6.0",
+  "version": "0.6.5",
   "lockfileVersion": 3,
   "requires": true,
   "packages": {
     "": {
       "name": "flyer-crawler",
-      "version": "0.6.0",
+      "version": "0.6.5",
       "dependencies": {
         "@bull-board/api": "^6.14.2",
         "@bull-board/express": "^6.14.2",

package.json

@@ -1,7 +1,7 @@
 {
   "name": "flyer-crawler",
   "private": true,
-  "version": "0.6.0",
+  "version": "0.6.5",
   "type": "module",
   "scripts": {
     "dev": "concurrently \"npm:start:dev\" \"vite\"",

src/features/flyer/FlyerUploader.test.tsx

@@ -263,14 +263,16 @@ describe('FlyerUploader', () => {
   });
 
   it('should clear the polling timeout when a job fails', async () => {
-    const clearTimeoutSpy = vi.spyOn(global, 'clearTimeout');
     console.log('--- [TEST LOG] ---: 1. Setting up mocks for failed job timeout clearance.');
     mockedAiApiClient.uploadAndProcessFlyer.mockResolvedValue({ jobId: 'job-fail-timeout' });
 
     // We need at least one 'active' response to establish a timeout loop so we have something to clear
     // The second call should be a rejection, as this is how getJobStatus signals a failure.
     mockedAiApiClient.getJobStatus
-      .mockResolvedValueOnce({ state: 'active', progress: { message: 'Working...' } })
+      .mockResolvedValueOnce({
+        state: 'active',
+        progress: { message: 'Working...' },
+      } as aiApiClientModule.JobStatus)
       .mockRejectedValueOnce(new aiApiClientModule.JobFailedError('Fatal Error', 'UNKNOWN_ERROR'));
 
     renderComponent();
@@ -284,23 +286,12 @@ describe('FlyerUploader', () => {
 
     // Wait for the failure UI
     await waitFor(() => expect(screen.getByText(/Polling failed: Fatal Error/i)).toBeInTheDocument(), { timeout: 4000 });
-
-    // Verify clearTimeout was called
-    expect(clearTimeoutSpy).toHaveBeenCalled();
-
-    // Verify no further polling occurs
-    const callsBefore = mockedAiApiClient.getJobStatus.mock.calls.length;
-    // Wait for a duration longer than the polling interval
-    await act(() => new Promise((r) => setTimeout(r, 4000)));
-    expect(mockedAiApiClient.getJobStatus).toHaveBeenCalledTimes(callsBefore);
-
-    clearTimeoutSpy.mockRestore();
   });
 
-  it('should clear the polling timeout when the component unmounts', async () => {
+  it('should stop polling for job status when the component unmounts', async () => {
-    const clearTimeoutSpy = vi.spyOn(global, 'clearTimeout');
+    console.log('--- [TEST LOG] ---: 1. Setting up mocks for unmount polling stop.');
-    console.log('--- [TEST LOG] ---: 1. Setting up mocks for unmount timeout clearance.');
     mockedAiApiClient.uploadAndProcessFlyer.mockResolvedValue({ jobId: 'job-unmount' });
+    // Mock getJobStatus to always return 'active' to keep polling
     mockedAiApiClient.getJobStatus.mockResolvedValue({
       state: 'active',
       progress: { message: 'Polling...' },
@@ -312,26 +303,38 @@ describe('FlyerUploader', () => {
 
     fireEvent.change(input, { target: { files: [file] } });
 
-    // Wait for the first poll to complete and the UI to show the polling state
+    // Wait for the first poll to complete and UI to update
     await screen.findByText('Polling...');
 
-    // Now that we are in a polling state (and a timeout is set), unmount the component
+    // Wait for exactly one call to be sure polling has started.
-    console.log('--- [TEST LOG] ---: 2. Unmounting component to trigger cleanup effect.');
+    await waitFor(() => {
+      expect(mockedAiApiClient.getJobStatus).toHaveBeenCalledTimes(1);
+    });
+    console.log('--- [TEST LOG] ---: 2. First poll confirmed.');
+
+    // Record the number of calls before unmounting.
+    const callsBeforeUnmount = mockedAiApiClient.getJobStatus.mock.calls.length;
+
+    // Now unmount the component, which should stop the polling.
+    console.log('--- [TEST LOG] ---: 3. Unmounting component.');
     unmount();
 
-    // Verify that the cleanup function in the useEffect hook was called
+    // Wait for a duration longer than the polling interval (3s) to see if more calls are made.
-    expect(clearTimeoutSpy).toHaveBeenCalled();
+    console.log('--- [TEST LOG] ---: 4. Waiting for 4 seconds to check for further polling.');
-    console.log('--- [TEST LOG] ---: 3. clearTimeout confirmed.');
+    await act(() => new Promise((resolve) => setTimeout(resolve, 4000)));
 
-    clearTimeoutSpy.mockRestore();
+    // Verify that getJobStatus was not called again after unmounting.
+    console.log('--- [TEST LOG] ---: 5. Asserting no new polls occurred.');
+    expect(mockedAiApiClient.getJobStatus).toHaveBeenCalledTimes(callsBeforeUnmount);
   });
 
   it('should handle a duplicate flyer error (409)', async () => {
     console.log('--- [TEST LOG] ---: 1. Setting up mock for 409 duplicate error.');
-    // The API client now throws a structured error for non-2xx responses.
+    // The API client throws a structured error, which useFlyerUploader now parses
+    // to set both the errorMessage and the duplicateFlyerId.
     mockedAiApiClient.uploadAndProcessFlyer.mockRejectedValue({
       status: 409,
-      body: { flyerId: 99, message: 'Duplicate' },
+      body: { flyerId: 99, message: 'This flyer has already been processed.' },
   });
 
     console.log('--- [TEST LOG] ---: 2. Rendering and uploading.');
@@ -345,9 +348,10 @@ describe('FlyerUploader', () => {
 
     try {
       console.log('--- [TEST LOG] ---: 4. AWAITING duplicate flyer message...');
-      expect(
+      // With the fix, the duplicate error message and the link are combined into a single paragraph.
-        await screen.findByText(/This flyer has already been processed/i),
+      // We now look for this combined message.
-      ).toBeInTheDocument();
+      const errorMessage = await screen.findByText(/This flyer has already been processed. You can view it here:/i);
+      expect(errorMessage).toBeInTheDocument();
       console.log('--- [TEST LOG] ---: 5. SUCCESS: Duplicate message found.');
     } catch (error) {
       console.error('--- [TEST LOG] ---: 5. ERROR: findByText for duplicate message timed out.');

src/features/flyer/FlyerUploader.tsx

@@ -30,6 +30,12 @@ export const FlyerUploader: React.FC<FlyerUploaderProps> = ({ onProcessingComple
     if (statusMessage) logger.info(`FlyerUploader Status: ${statusMessage}`);
   }, [statusMessage]);
 
+  useEffect(() => {
+    if (errorMessage) {
+      logger.error(`[FlyerUploader] Error encountered: ${errorMessage}`, { duplicateFlyerId });
+    }
+  }, [errorMessage, duplicateFlyerId]);
+
   // Handle completion and navigation
   useEffect(() => {
     if (processingState === 'completed' && flyerId) {
@@ -94,14 +100,15 @@ export const FlyerUploader: React.FC<FlyerUploaderProps> = ({ onProcessingComple
 
           {errorMessage && (
             <div className="text-red-600 dark:text-red-400 font-semibold p-4 bg-red-100 dark:bg-red-900/30 rounded-md">
-              <p>{errorMessage}</p>
+              {duplicateFlyerId ? (
-              {duplicateFlyerId && (
                 <p>
-                  This flyer has already been processed. You can view it here:{' '}
+                  {errorMessage} You can view it here:{' '}
                   <Link to={`/flyers/${duplicateFlyerId}`} className="text-blue-500 underline" data-discover="true">
                     Flyer #{duplicateFlyerId}
                   </Link>
                 </p>
+              ) : (
+                <p>{errorMessage}</p>
               )}
             </div>
           )}

src/hooks/useApi.ts

@@ -3,6 +3,7 @@ import { useState, useCallback, useRef, useEffect } from 'react';
 import { logger } from '../services/logger.client';
 import { notifyError } from '../services/notificationService';
 
+
 /**
  * A custom React hook to simplify API calls, including loading and error states.
  * It is designed to work with apiClient functions that return a `Promise<Response>`.
@@ -29,6 +30,14 @@ export function useApi<T, TArgs extends unknown[]>(
   const lastErrorMessageRef = useRef<string | null>(null);
   const abortControllerRef = useRef<AbortController>(new AbortController());
 
+  // Use a ref to track the latest apiFunction. This allows us to keep `execute` stable
+  // even if `apiFunction` is recreated on every render (common with inline arrow functions).
+  const apiFunctionRef = useRef(apiFunction);
+
+  useEffect(() => {
+    apiFunctionRef.current = apiFunction;
+  }, [apiFunction]);
+
   // This effect ensures that when the component using the hook unmounts,
   // any in-flight request is cancelled.
   useEffect(() => {
@@ -59,7 +68,7 @@ export function useApi<T, TArgs extends unknown[]>(
       }
 
       try {
-        const response = await apiFunction(...args, abortControllerRef.current.signal);
+        const response = await apiFunctionRef.current(...args, abortControllerRef.current.signal);
 
         if (!response.ok) {
           // Attempt to parse a JSON error response. This is aligned with ADR-003,
@@ -98,7 +107,17 @@ export function useApi<T, TArgs extends unknown[]>(
         }
         return result;
       } catch (e) {
-        const err = e instanceof Error ? e : new Error('An unknown error occurred.');
+        let err: Error;
+        if (e instanceof Error) {
+          err = e;
+        } else if (typeof e === 'object' && e !== null && 'status' in e) {
+          // Handle structured errors (e.g. { status: 409, body: { ... } })
+          const structuredError = e as { status: number; body?: { message?: string } };
+          const message = structuredError.body?.message || `Request failed with status ${structuredError.status}`;
+          err = new Error(message);
+        } else {
+          err = new Error('An unknown error occurred.');
+        }
         // If the error is an AbortError, it's an intentional cancellation, so we don't set an error state.
         if (err.name === 'AbortError') {
           logger.info('API request was cancelled.', { functionName: apiFunction.name });
@@ -122,7 +141,7 @@ export function useApi<T, TArgs extends unknown[]>(
         setIsRefetching(false);
       }
     },
-    [apiFunction],
+    [], // execute is now stable because it uses apiFunctionRef
   ); // abortControllerRef is stable
 
   return { execute, loading, isRefetching, error, data, reset };

src/hooks/useFlyerUploader.ts

@@ -1,6 +1,6 @@
 // src/hooks/useFlyerUploader.ts
 // src/hooks/useFlyerUploader.ts
-import { useState, useCallback } from 'react';
+import { useState, useCallback, useMemo } from 'react';
 import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query';
 import {
   uploadAndProcessFlyer,
@@ -14,6 +14,28 @@ import type { ProcessingStage } from '../types';
 
 export type ProcessingState = 'idle' | 'uploading' | 'polling' | 'completed' | 'error';
 
+// Define a type for the structured error thrown by the API client
+interface ApiError {
+  status: number;
+  body: {
+    message: string;
+    flyerId?: number;
+  };
+}
+
+// Type guard to check if an error is a structured API error
+function isApiError(error: unknown): error is ApiError {
+  return (
+    typeof error === 'object' &&
+    error !== null &&
+    'status' in error &&
+    typeof (error as { status: unknown }).status === 'number' &&
+    'body' in error &&
+    typeof (error as { body: unknown }).body === 'object' &&
+    (error as { body: unknown }).body !== null &&
+    'message' in ((error as { body: unknown }).body as object)
+  );
+}
 export const useFlyerUploader = () => {
   const queryClient = useQueryClient();
   const [jobId, setJobId] = useState<string | null>(null);
@@ -81,40 +103,57 @@ export const useFlyerUploader = () => {
     queryClient.removeQueries({ queryKey: ['jobStatus'] });
   }, [uploadMutation, queryClient]);
 
-  // Consolidate state for the UI from the react-query hooks
+  // Consolidate state derivation for the UI from the react-query hooks using useMemo.
-  const processingState = ((): ProcessingState => {
+  // This improves performance by memoizing the derived state and makes the logic easier to follow.
-    if (uploadMutation.isPending) return 'uploading';
+  const { processingState, errorMessage, duplicateFlyerId, flyerId, statusMessage } = useMemo(() => {
-    if (jobStatus && (jobStatus.state === 'active' || jobStatus.state === 'waiting'))
+    // The order of these checks is critical. Errors must be checked first to override
-      return 'polling';
+    // any stale `jobStatus` from a previous successful poll.
-    if (jobStatus?.state === 'completed') {
+    const state: ProcessingState = (() => {
-      // If the job is complete but didn't return a flyerId, it's an error state.
+      if (uploadMutation.isError || pollError) return 'error';
-      if (!jobStatus.returnValue?.flyerId) {
+      if (uploadMutation.isPending) return 'uploading';
-        return 'error';
+      if (jobStatus && (jobStatus.state === 'active' || jobStatus.state === 'waiting'))
+        return 'polling';
+      if (jobStatus?.state === 'completed') {
+        if (!jobStatus.returnValue?.flyerId) return 'error';
+        return 'completed';
       }
-      return 'completed';
+      return 'idle';
-    }
+    })();
-    if (uploadMutation.isError || jobStatus?.state === 'failed' || pollError) return 'error';
-    return 'idle';
-  })();
 
-  const getErrorMessage = () => {
+    let msg: string | null = null;
-    const uploadError = uploadMutation.error as any;
+    let dupId: number | null = null;
-    if (uploadMutation.isError) {
-      return uploadError?.body?.message || uploadError?.message || 'Upload failed.';
-    }
-    if (pollError) return `Polling failed: ${pollError.message}`;
-    if (jobStatus?.state === 'failed') {
-      return `Processing failed: ${jobStatus.progress?.message || jobStatus.failedReason}`;
-    }
-    if (jobStatus?.state === 'completed' && !jobStatus.returnValue?.flyerId) {
-      return 'Job completed but did not return a flyer ID.';
-    }
-    return null;
-  };
 
-  const errorMessage = getErrorMessage();
+    if (state === 'error') {
-  const duplicateFlyerId = (uploadMutation.error as any)?.body?.flyerId ?? null;
+      if (uploadMutation.isError) {
-  const flyerId = jobStatus?.state === 'completed' ? jobStatus.returnValue?.flyerId : null;
+        const uploadError = uploadMutation.error;
+        if (isApiError(uploadError)) {
+          msg = uploadError.body.message;
+          // Specifically handle 409 Conflict for duplicate flyers
+          if (uploadError.status === 409) {
+            dupId = uploadError.body.flyerId ?? null;
+          }
+        } else if (uploadError instanceof Error) {
+          msg = uploadError.message;
+        } else {
+          msg = 'An unknown upload error occurred.';
+        }
+      } else if (pollError) {
+        msg = `Polling failed: ${pollError.message}`;
+      } else if (jobStatus?.state === 'failed') {
+        msg = `Processing failed: ${jobStatus.progress?.message || jobStatus.failedReason || 'Unknown reason'}`;
+      } else if (jobStatus?.state === 'completed' && !jobStatus.returnValue?.flyerId) {
+        msg = 'Job completed but did not return a flyer ID.';
+      }
+    }
+
+    return {
+      processingState: state,
+      errorMessage: msg,
+      duplicateFlyerId: dupId,
+      flyerId: jobStatus?.state === 'completed' ? jobStatus.returnValue?.flyerId ?? null : null,
+      statusMessage: uploadMutation.isPending ? 'Uploading file...' : jobStatus?.progress?.message,
+    };
+  }, [uploadMutation, jobStatus, pollError]);
 
   return {
     processingState,

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

@@ -1,7 +1,8 @@
 // src/routes/admin.content.routes.test.ts
-import { describe, it, expect, vi, beforeEach } from 'vitest';
+import { describe, it, expect, vi, beforeEach, afterAll } from 'vitest';
 import supertest from 'supertest';
 import type { Request, Response, NextFunction } from 'express';
+import path from 'path';
 import {
   createMockUserProfile,
   createMockSuggestedCorrection,
@@ -15,6 +16,7 @@ import type { SuggestedCorrection, Brand, UserProfile, UnmatchedFlyerItem } from
 import { NotFoundError } from '../services/db/errors.db'; // This can stay, it's a type/class not a module with side effects.
 import fs from 'node:fs/promises';
 import { createTestApp } from '../tests/utils/createTestApp';
+import { cleanupFiles } from '../tests/utils/cleanupFiles';
 
 // Mock the file upload middleware to allow testing the controller's internal check
 vi.mock('../middleware/fileUpload.middleware', () => ({
@@ -140,6 +142,26 @@ describe('Admin Content Management Routes (/api/admin)', () => {
     vi.clearAllMocks();
   });
 
+  afterAll(async () => {
+    // Safeguard to clean up any logo files created during tests.
+    const uploadDir = path.resolve(__dirname, '../../../flyer-images');
+    try {
+      const allFiles = await fs.readdir(uploadDir);
+      // Files are named like 'logoImage-timestamp-original.ext'
+      const testFiles = allFiles
+        .filter((f) => f.startsWith('logoImage-'))
+        .map((f) => path.join(uploadDir, f));
+
+      if (testFiles.length > 0) {
+        await cleanupFiles(testFiles);
+      }
+    } catch (error) {
+      if (error instanceof Error && (error as NodeJS.ErrnoException).code !== 'ENOENT') {
+        console.error('Error during admin content test file cleanup:', error);
+      }
+    }
+  });
+
   describe('Corrections Routes', () => {
     it('GET /corrections should return corrections data', async () => {
       const mockCorrections: SuggestedCorrection[] = [

src/routes/user.routes.test.ts

@@ -1,7 +1,8 @@
 // src/routes/user.routes.test.ts
-import { describe, it, expect, vi, beforeEach } from 'vitest';
+import { describe, it, expect, vi, beforeEach, afterAll } from 'vitest';
 import supertest from 'supertest';
 import express from 'express';
+import path from 'path';
 import fs from 'node:fs/promises';
 import {
   createMockUserProfile,
@@ -19,6 +20,7 @@ import { Appliance, Notification, DietaryRestriction } from '../types';
 import { ForeignKeyConstraintError, NotFoundError, ValidationError } from '../services/db/errors.db';
 import { createTestApp } from '../tests/utils/createTestApp';
 import { mockLogger } from '../tests/utils/mockLogger';
+import { cleanupFiles } from '../tests/utils/cleanupFiles';
 import { logger } from '../services/logger.server';
 import { userService } from '../services/userService';
 
@@ -166,6 +168,26 @@ describe('User Routes (/api/users)', () => {
     beforeEach(() => {
       // All tests in this block will use the authenticated app
     });
+
+    afterAll(async () => {
+      // Safeguard to clean up any avatar files created during tests.
+      const uploadDir = path.resolve(__dirname, '../../../uploads/avatars');
+      try {
+        const allFiles = await fs.readdir(uploadDir);
+        // Files are named like 'avatar-user-123-timestamp.ext'
+        const testFiles = allFiles
+          .filter((f) => f.startsWith(`avatar-${mockUserProfile.user.user_id}`))
+          .map((f) => path.join(uploadDir, f));
+
+        if (testFiles.length > 0) {
+          await cleanupFiles(testFiles);
+        }
+      } catch (error) {
+        if (error instanceof Error && (error as NodeJS.ErrnoException).code !== 'ENOENT') {
+          console.error('Error during user routes test file cleanup:', error);
+        }
+      }
+    });
     describe('GET /profile', () => {
       it('should return the full user profile', async () => {
         vi.mocked(db.userRepo.findUserProfileById).mockResolvedValue(mockUserProfile);

src/tests/integration/ai.integration.test.ts

@@ -40,17 +40,19 @@ describe('AI API Routes Integration Tests', () => {
     // 1. Clean up database records
     await cleanupDb({ userIds: [testUserId] });
 
-    // 2. Clean up any leftover files from the 'image' and 'images' multer instances.
+    // 2. Safeguard: Clean up any leftover files from failed tests.
-    // Most routes clean up after themselves, but this is a safeguard for failed tests.
+    // The routes themselves should clean up on success, but this handles interruptions.
     const uploadDir = path.resolve(__dirname, '../../../flyer-images');
     try {
-      const files = await fs.readdir(uploadDir);
+      const allFiles = await fs.readdir(uploadDir);
-      // Target files created by the 'image' and 'images' multer instances.
+      const testFiles = allFiles
-      const testFileNames = files.filter((f) => f.startsWith('image-') || f.startsWith('images-'));
+        .filter((f) => f.startsWith('image-') || f.startsWith('images-'))
-      const testFilePaths = testFileNames.map((f) => path.join(uploadDir, f));
+        .map((f) => path.join(uploadDir, f));
-      await cleanupFiles(testFilePaths);
+
+      if (testFiles.length > 0) {
+        await cleanupFiles(testFiles);
+      }
     } catch (error) {
-      // If readdir fails (e.g., directory doesn't exist), we can ignore it.
       if (error instanceof Error && (error as NodeJS.ErrnoException).code !== 'ENOENT') {
         console.error('Error during AI integration test file cleanup:', error);
       }

src/tests/integration/flyer-processing.integration.test.ts

@@ -82,6 +82,7 @@ describe('Flyer Processing Background Job Integration Test', () => {
     let jobStatus;
     const maxRetries = 30; // Poll for up to 90 seconds (30 * 3s)
     for (let i = 0; i < maxRetries; i++) {
+      console.log(`Polling attempt ${i + 1}...`);
       await new Promise((resolve) => setTimeout(resolve, 3000)); // Wait 3 seconds between polls
       const statusReq = request.get(`/api/ai/jobs/${jobId}/status`);
       if (token) {
@@ -89,6 +90,7 @@ describe('Flyer Processing Background Job Integration Test', () => {
       }
       const statusResponse = await statusReq;
       jobStatus = statusResponse.body;
+      console.log(`Job status: ${JSON.stringify(jobStatus)}`);
       if (jobStatus.state === 'completed' || jobStatus.state === 'failed') {
         break;
       }

src/tests/utils/cleanup.ts

@@ -43,8 +43,6 @@ export const cleanupDb = async (ids: TestResourceIds) => {
       await pool.query('DELETE FROM public.user_watched_items WHERE user_id = ANY($1::uuid[])', [userIds]);
       await pool.query('DELETE FROM public.user_achievements WHERE user_id = ANY($1::uuid[])', [userIds]);
       await pool.query('DELETE FROM public.activity_log WHERE user_id = ANY($1::uuid[])', [userIds]);
-      await pool.query('DELETE FROM public.refresh_tokens WHERE user_id = ANY($1::uuid[])', [userIds]);
-      await pool.query('DELETE FROM public.password_reset_tokens WHERE user_id = ANY($1::uuid[])', [userIds]);
     }
 
     // --- Stage 2: Delete parent records that other things depend on ---