unit test fixes + error refactor

src/App.test.tsx

@@ -25,8 +25,7 @@ vi.mock('./pages/admin/AdminStatsPage', () => ({ AdminStatsPage: () => <div data
 vi.mock('./pages/ResetPasswordPage', () => ({ ResetPasswordPage: () => <div data-testid="reset-password-page-mock">Reset Password Page</div> }));
 vi.mock('./pages/admin/VoiceLabPage', () => ({ VoiceLabPage: () => <div data-testid="voice-lab-page-mock">Voice Lab Page</div> })); // Corrected path
 vi.mock('./components/WhatsNewModal', () => ({ WhatsNewModal: ({ isOpen, onClose }: { isOpen: boolean, onClose: () => void }) => isOpen ? <div data-testid="whats-new-modal-mock"><button onClick={onClose}>Close What's New</button></div> : null }));
-
-vi.mock('./components/FlyerCorrectionTool', () => ({ FlyerCorrectionTool: ({ onClose, onDataExtracted }: { onClose: () => void, onDataExtracted: (type: 'store_name' | 'dates', value: string) => void }) => <div data-testid="flyer-correction-tool-mock"><button onClick={onClose}>Close Correction</button><button onClick={() => onDataExtracted('store_name', 'New Store Name')}>Extract Store</button><button onClick={() => onDataExtracted('dates', '2023-01-01')}>Extract Dates</button></div> }));
+vi.mock('./components/FlyerCorrectionTool', () => ({ FlyerCorrectionTool: ({ isOpen, onClose, onDataExtracted }: { isOpen: boolean, onClose: () => void, onDataExtracted: (type: 'store_name' | 'dates', value: string) => void }) => isOpen ? <div data-testid="flyer-correction-tool-mock"><button onClick={onClose}>Close Correction</button><button onClick={() => onDataExtracted('store_name', 'New Store Name')}>Extract Store</button><button onClick={() => onDataExtracted('dates', '2023-01-01')}>Extract Dates</button></div> : null }));
 // Mock the new layout and page components
 vi.mock('./layouts/MainLayout', () => ({ MainLayout: () => <div data-testid="main-layout-mock"><Outlet /></div> }));
 vi.mock('./pages/HomePage', () => ({ HomePage: (props: any) => <div data-testid="home-page-mock"><button onClick={props.onOpenCorrectionTool}>Open Correction Tool</button></div> }));
@@ -486,6 +485,9 @@ describe('App Component', () => {
         });
 
         it('should open the "What\'s New" modal when the question mark icon is clicked', async () => {
+            // Pre-set the localStorage to prevent the modal from opening automatically
+            localStorageMock.setItem('lastSeenVersion', '2.0.0');
+
             renderApp();
             expect(screen.queryByTestId('whats-new-modal-mock')).not.toBeInTheDocument();
 

src/components/MapView.test.tsx

@@ -1,20 +1,22 @@
 // src/components/MapView.test.tsx
 import React from 'react';
 import { render, screen } from '@testing-library/react';
-import { describe, it, expect, vi, beforeEach } from 'vitest';
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
 import { MapView } from './MapView';
-import config from '../config';
 
-// Mock the new config module
-vi.mock('../config', () => ({
-  default: {
-    app: {
-      version: '1.0.0',
-      commitMessage: 'Initial commit',
-      commitUrl: '#',
-    },
-    google: { mapsEmbedApiKey: undefined } },
-}));
+// Helper to dynamically mock the config
+const setupConfigMock = (apiKey: string | undefined) => {
+  vi.doMock('../config', async () => {
+    const actualConfig = await vi.importActual('../config') as { default: any };
+    return {
+      ...actualConfig,
+      default: {
+        ...actualConfig.default,
+        google: { ...actualConfig.default.google, mapsEmbedApiKey: apiKey },
+      },
+    };
+  });
+};
 
 describe('MapView', () => {
   const defaultProps = {
@@ -22,10 +24,14 @@ describe('MapView', () => {
     longitude: -74.0060,
   };
   
+  afterEach(() => {
+    vi.resetModules(); // Clean up mocks between tests
+  });
+
   describe('when API key is not configured', () => {
     beforeEach(() => {
-      // Reset the mock to its default (undefined key)
-      (vi.mocked(config).google.mapsEmbedApiKey as string | undefined) = undefined;
+      // Set the API key to undefined for this test suite
+      setupConfigMock(undefined);
     });
   
     it('should render a disabled message', () => {
@@ -43,11 +49,11 @@ describe('MapView', () => {
     const mockApiKey = 'test-api-key';
 
     beforeEach(() => {
-      // Set the API key for this test suite
-      (vi.mocked(config).google.mapsEmbedApiKey as string | undefined) = mockApiKey;
+      // Set a specific API key for this test suite
+      setupConfigMock(mockApiKey);
     });
 
-    it('should render the iframe with the correct src URL', () => {
+    it('should render the iframe with the correct src URL', async () => {
       render(<MapView {...defaultProps} />);
       const iframe = screen.getByRole('iframe');
       const expectedSrc = `https://www.google.com/maps/embed/v1/view?key=${mockApiKey}&center=${defaultProps.latitude},${defaultProps.longitude}&zoom=14`;

src/hooks/useAuth.test.tsx

@@ -60,6 +60,12 @@ describe('useAuth Hook and AuthProvider', () => {
   });
 
   it('initializes with a "Determining..." state and isLoading as true', () => {
+    // For this specific test, we want to check the initial state *before* the
+    // auth check promise resolves. We mock it to be a pending promise.
+    mockedApiClient.getAuthenticatedUserProfile.mockReturnValue(
+      new Promise(() => {})
+    );
+
     const { result } = renderHook(() => useAuth(), { wrapper });
     expect(result.current.authStatus).toBe('Determining...');
     expect(result.current.isLoading).toBe(true);

src/middleware/errorHandler.ts

@@ -4,9 +4,44 @@ import { DatabaseError, UniqueConstraintError, ForeignKeyConstraintError, NotFou
 import crypto from 'crypto';
 import { logger } from '../services/logger.server';
 
+// --- Helper Functions for Secure Logging ---
+
+/**
+ * Sanitizes an object by redacting values of sensitive keys.
+ * @param obj The object to sanitize.
+ * @returns A new object with sensitive values redacted.
+ */
+const sanitizeObject = (obj: Record<string, any>): Record<string, any> => {
+  if (!obj || typeof obj !== 'object') return {};
+  const sensitiveKeys = ['password', 'token', 'authorization', 'cookie', 'newPassword', 'currentPassword'];
+  const sanitizedObj: Record<string, any> = {};
+  for (const key in obj) {
+    if (Object.prototype.hasOwnProperty.call(obj, key)) {
+      if (sensitiveKeys.some(sensitiveKey => key.toLowerCase().includes(sensitiveKey))) {
+        sanitizedObj[key] = '[REDACTED]';
+      } else {
+        sanitizedObj[key] = obj[key];
+      }
+    }
+  }
+  return sanitizedObj;
+};
+
+/**
+ * Extracts user information from the request object for logging.
+ * @param req The Express request object.
+ * @returns An object with user details or null if no user is authenticated.
+ */
+const getLoggableUser = (req: Request): { id: string; email?: string } | null => {
+  const user = req.user as { user_id?: string; email?: string };
+  if (user && user.user_id) {
+    return { id: user.user_id, email: user.email };
+  }
+  return null;
+};
+
 interface HttpError extends Error {
   status?: number;
-  // You can add other custom properties like 'code' if needed
 }
 
 export const errorHandler = (err: HttpError, req: Request, res: Response, next: NextFunction) => {
@@ -15,41 +50,55 @@ export const errorHandler = (err: HttpError, req: Request, res: Response, next:
     return next(err);
   }
 
-  let statusCode = err.status || 500;
+  // --- 1. Determine Final Status Code and Message ---
+  let statusCode = err.status ?? 500;
   let message = err.message;
-
-  // --- Handle Specific Custom Error Types ---
-  // All our custom errors inherit from DatabaseError and have a status property.
-  if (err instanceof DatabaseError) {
-    statusCode = err.status || 500;
-  }
-
-  // --- TEST ENVIRONMENT DEBUGGING ---
-  // In a test environment, log the full stack trace to the console for easier debugging.
-  if (process.env.NODE_ENV === 'test') {
-    console.error('--- [TEST] UNHANDLED ERROR ---', err);
-  }
-
   let errorId: string | undefined;
+
+  // Refine the status code for known error types. This block should come first.
+  if (err instanceof DatabaseError) {
+    // This will handle UniqueConstraintError, ForeignKeyConstraintError, NotFoundError, etc.
+    statusCode = err.status;
+  } else if (err instanceof ForeignKeyConstraintError) {
+    statusCode = 400;
+  } else if (err instanceof UniqueConstraintError) {
+    statusCode = 409; // Conflict
+  } else if (err instanceof NotFoundError) {
+    statusCode = 404;
+  } else if (err.name === 'UnauthorizedError') {
+    statusCode = err.status || 401;
+  }
+
+  // --- 2. Log Based on Final Status Code ---
   // Log the full error details for debugging, especially for server errors.
   if (statusCode >= 500) {
     errorId = crypto.randomBytes(4).toString('hex');
     logger.error(`Unhandled API Error (ID: ${errorId}):`, {
+      // Log sanitized data for security
       error: err.stack || err.message,
       path: req.path,
       method: req.method,
-      body: req.body,
+      body: sanitizeObject(req.body),
+      headers: sanitizeObject(req.headers),
+      user: getLoggableUser(req),
     });
   } else {
     // For 4xx errors, log at a lower level (e.g., 'warn') to avoid flooding error trackers.
     logger.warn(`Client Error: ${statusCode} on ${req.method} ${req.path}`, {
       errorMessage: message,
+      user: getLoggableUser(req),
       path: req.path,
       method: req.method,
       ip: req.ip,
     });
   }
 
+  // --- TEST ENVIRONMENT DEBUGGING ---
+  if (process.env.NODE_ENV === 'test') {
+    console.error('--- [TEST] UNHANDLED ERROR ---', err);
+  }
+
+  // --- 3. Send Response ---
   // In production, send a generic message for 5xx errors.
   // In dev/test, send the actual error message for easier debugging.
   const responseMessage = (statusCode >= 500 && process.env.NODE_ENV === 'production')

src/pages/HomePage.test.tsx

@@ -13,7 +13,9 @@ vi.mock('../features/flyer/FlyerDisplay', () => ({
   FlyerDisplay: (props: any) => <div data-testid="flyer-display" data-image-url={props.imageUrl} />,
 }));
 vi.mock('../features/flyer/ExtractedDataTable', () => ({
-  ExtractedDataTable: (props: { items: FlyerItem[] }) => <div data-testid="extracted-data-table">{props.items.length} items</div>,
+  // Wrap the mock component in vi.fn() to allow spying on its calls.
+  // This gives us access to `mock.calls` in our tests.
+  ExtractedDataTable: vi.fn((props: { items: FlyerItem[] }) => <div data-testid="extracted-data-table">{props.items.length} items</div>),
 }));
 vi.mock('../features/flyer/AnalysisPanel', () => ({
   AnalysisPanel: () => <div data-testid="analysis-panel" />,

src/routes/admin.routes.ts

@@ -154,7 +154,7 @@ router.put('/recipes/:id/status', async (req, res, next: NextFunction) => {
         const updatedRecipe = await db.adminRepo.updateRecipeStatus(recipeId, status); // This is still a standalone function in admin.db.ts
         res.status(200).json(updatedRecipe);
     } catch (error) {
-        next(error);
+        next(error); // Pass all errors to the central error handler
     }
 });