refactor: clean up imports and improve error messages across multiple components

src/components/FlyerCorrectionTool.test.tsx

@@ -1,6 +1,6 @@
 // src/components/FlyerCorrectionTool.test.tsx
 import React from 'react';
-import { render, screen, fireEvent, waitFor, act } from '@testing-library/react';
+import { render, screen, fireEvent, waitFor } from '@testing-library/react';
 import { describe, it, expect, vi, beforeEach, type Mocked } from 'vitest';
 import { FlyerCorrectionTool } from './FlyerCorrectionTool';
 import * as aiApiClient from '../services/aiApiClient';
@@ -16,7 +16,6 @@ vi.mock('../services/logger', () => ({
 }));
 
 const mockedAiApiClient = aiApiClient as Mocked<typeof aiApiClient>;
-const mockedNotifyError = notifyError as Mocked<typeof notifyError>;
 const mockedNotifySuccess = notifySuccess as Mocked<typeof notifySuccess>;
 
 const defaultProps = {

src/components/icons/CheckCircleIcon.tsx

@@ -1,7 +1,16 @@
 import React from 'react';
 
-export const CheckCircleIcon: React.FC<React.SVGProps<SVGSVGElement>> = (props) => (
-  <svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" strokeWidth={1.5} stroke="currentColor" {...props}>
-    <path strokeLinecap="round" strokeLinejoin="round" d="M9 12.75 11.25 15 15 9.75M21 12a9 9 0 1 1-18 0 9 9 0 0 1 18 0Z" />
-  </svg>
+interface CheckCircleIconProps extends React.SVGProps<SVGSVGElement> {
+  title?: string;
+}
+
+export const CheckCircleIcon: React.FC<CheckCircleIconProps> = ({ title, ...props }) => (
+    <svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" strokeWidth={1.5} stroke="currentColor" {...props}>
+        {title && <title>{title}</title>}
+        <path
+            strokeLinecap="round"
+            strokeLinejoin="round"
+            d="M9 12.75L11.25 15 15 9.75M21 12a9 9 0 11-18 0 9 9 0 0118 0z"
+        />
+    </svg>
 );

src/features/flyer/AnalysisPanel.test.tsx

@@ -1,7 +1,7 @@
 // src/features/flyer/AnalysisPanel.test.tsx
 import React from 'react';
 import { render, screen, fireEvent, waitFor } from '@testing-library/react';
-import { describe, it, expect, vi, beforeEach, type Mock, type Mocked } from 'vitest';
+import { describe, it, expect, vi, beforeEach, type Mocked } from 'vitest';
 import { AnalysisPanel } from './AnalysisPanel';
 import * as aiApiClient from '../../services/aiApiClient';
 import type { FlyerItem, Store } from '../../types';

src/pages/VoiceLabPage.test.tsx

@@ -1,7 +1,7 @@
 // src/pages/VoiceLabPage.test.tsx
 import React from 'react';
 import { render, screen, fireEvent, waitFor } from '@testing-library/react';
-import { describe, it, expect, vi, beforeEach, type Mocked } from 'vitest';
+import { describe, it, expect, vi, beforeEach } from 'vitest';
 import { VoiceLabPage } from './VoiceLabPage';
 import * as aiApiClient from '../services/aiApiClient';
 import { notifyError } from '../services/notificationService';

src/pages/admin/components/ProfileManager.tsx

@@ -160,7 +160,7 @@ export const ProfileManager: React.FC<ProfileManagerProps> = ({ isOpen, onClose,
       setAddress(prev => ({ ...prev, latitude: lat, longitude: lng }));
       toast.success('Address re-geocoded successfully!');
     } catch (error) {
-      toast.error('Failed to re-geocode address.');
+      toast.error('Failed to re-geocode address: ${error.message}.');
     } finally {
       setIsGeocoding(false);
     }
@@ -192,7 +192,7 @@ export const ProfileManager: React.FC<ProfileManagerProps> = ({ isOpen, onClose,
         setAddress(prev => ({ ...prev, latitude: lat, longitude: lng }));
         toast.success('Address geocoded successfully!');
       } catch (error) {
-        toast.error('Failed to geocode address.');
+        toast.error('Failed to geocode address: ${error.message}.');
       } finally {
         setIsGeocoding(false);
       }

src/pages/admin/components/SystemCheck.tsx

@@ -35,14 +35,18 @@ const initialChecks: Check[] = [
     { id: CheckID.BACKEND, name: 'Backend Server Connection', description: 'Checks if the local Express.js server is running and reachable.', status: 'idle', message: '' },
     { id: CheckID.PM2_STATUS, name: 'PM2 Process Status', description: 'Checks if the application is running under PM2.', status: 'idle', message: '' }, // Restoring PM2 Status check
     { id: CheckID.DB_POOL, name: 'Database Connection Pool', description: 'Checks the health of the database connection pool.', status: 'idle', message: '' },
-    { id: CheckID.REDIS, name: 'Redis Connection', description: 'Checks if the backend can connect to the Redis server for background jobs.', status: 'idle', message: '' },
+    { id: CheckID.REDIS, name: 'Redis Connection', description: 'Checks if the backend can connect to the Redis server, used for background jobs and caching.', status: 'idle', message: '' },
     { id: CheckID.SCHEMA, name: 'Database Schema', description: 'Verifies required tables exist in the database.', status: 'idle', message: '' },
     { id: CheckID.SEED, name: 'Default Admin User', description: 'Verifies the default admin user can be logged into.', status: 'idle', message: '' },
     { id: CheckID.STORAGE, name: 'Assets Storage Directory', description: 'Checks if the local assets folder exists and is writable.', status: 'idle', message: '' },
     { id: CheckID.GEMINI, name: 'Gemini API Key', description: 'Verifies the GEMINI_API_KEY is set for AI features.', status: 'idle', message: '' },
 ];
 
-const GeocodeCacheManager: React.FC = () => {
+interface GeocodeCacheManagerProps {
+  redisOk: boolean;
+}
+
+const GeocodeCacheManager: React.FC<GeocodeCacheManagerProps> = ({ redisOk }) => {
   const [isLoading, setIsLoading] = useState(false);
 
   const handleClearCache = async () => {
@@ -70,22 +74,27 @@ const GeocodeCacheManager: React.FC = () => {
 
   return (
     <div className="mt-4">
-      <h4 className="font-medium text-gray-800 dark:text-gray-200">Geocoding Service</h4>
+      <div className="flex items-center space-x-2">
+        <h4 className="font-medium text-gray-800 dark:text-gray-200">Geocoding Service</h4>
+        {redisOk && <CheckCircleIcon className="w-5 h-5 text-green-500" title="Redis cache is connected" />}
+      </div>
       <p className="text-sm text-gray-500 dark:text-gray-400 mt-1">
         The application uses a Redis cache to store geocoding results and reduce API calls. You can manually clear this cache if you suspect the data is stale.
       </p>
-      <button
-        onClick={handleClearCache}
-        disabled={isLoading}
-        className="mt-3 inline-flex items-center justify-center px-4 py-2 border border-transparent text-sm font-medium rounded-md shadow-sm text-white bg-red-600 hover:bg-red-700 focus:outline-none focus:ring-2 focus:ring-offset-2 focus:ring-red-500 disabled:bg-red-400"
-      >
-        {isLoading ? (
-          <>
-            <div className="w-5 h-5 mr-2"><LoadingSpinner /></div>
-            <span>Clearing...</span>
-          </>
-        ) : ( 'Clear Geocode Cache' )}
-      </button>
+      {redisOk && (
+        <button
+          onClick={handleClearCache}
+          disabled={isLoading}
+          className="mt-3 inline-flex items-center justify-center px-4 py-2 border border-transparent text-sm font-medium rounded-md shadow-sm text-white bg-red-600 hover:bg-red-700 focus:outline-none focus:ring-2 focus:ring-offset-2 focus:ring-red-500 disabled:bg-red-400"
+        >
+          {isLoading ? (
+            <>
+              <div className="w-5 h-5 mr-2"><LoadingSpinner /></div>
+              <span>Clearing...</span>
+            </>
+          ) : ( 'Clear Geocode Cache' )}
+        </button>
+      )}
     </div>
   );
 };
@@ -96,6 +105,7 @@ export const SystemCheck: React.FC = () => {
     const [hasRunAutoTest, setHasRunAutoTest] = useState(false);
     const [elapsedTime, setElapsedTime] = useState<number | null>(null);
     const [isTriggeringJob, setIsTriggeringJob] = useState(false);
+    const [redisOk, setRedisOk] = useState(false);
     
     const updateCheckStatus = useCallback((id: CheckID, status: TestStatus, message: string) => {
         setChecks(prev => prev.map(c => c.id === id ? { ...c, status, message } : c));
@@ -178,6 +188,7 @@ export const SystemCheck: React.FC = () => {
             if (!response.ok) throw new Error((await response.json()).message || 'Failed to check Redis health');
             const { success, message } = await response.json();
             updateCheckStatus(CheckID.REDIS, success ? 'pass' : 'fail', message);
+            setRedisOk(success);
             return success;
         } catch (e) {
             updateCheckStatus(CheckID.REDIS, 'fail', getErrorMessage(e));
@@ -245,6 +256,11 @@ export const SystemCheck: React.FC = () => {
                 updateCheckStatus(CheckID.SEED, 'fail', 'Skipped: Database connection pool is unhealthy.');
             }
             // No checks currently depend on Redis, so no skipping logic is needed here.
+            
+            // If Redis is not OK, we can update the UI accordingly.
+            if (!redisOk) {
+                // You could add logic here to disable Redis-dependent features in the UI if needed.
+            }
 
             // --- Step 3: Run remaining, less-dependent checks in parallel. ---
             await Promise.all([
@@ -368,7 +384,7 @@ export const SystemCheck: React.FC = () => {
                         ) : 'Trigger Failing Job'}
                     </button>
                 </div>
-                <GeocodeCacheManager />
+                <GeocodeCacheManager redisOk={redisOk}/>
             </div>
         </div>
     );

src/routes/admin.test.ts

@@ -1,5 +1,5 @@
 // src/routes/admin.test.ts
-import { describe, it, expect, vi, beforeEach, type Mocked, type Mock } from 'vitest';
+import { describe, it, expect, vi, beforeEach, afterAll, type Mocked, type Mock } from 'vitest';
 import supertest from 'supertest';
 import express, { Request, Response, NextFunction } from 'express';
 import path from 'node:path';
@@ -31,14 +31,14 @@ vi.mock('./passport', () => ({
   // Mock the default export (the passport instance)
   default: {
     // The 'authenticate' method returns a middleware function. We mock that.
-    authenticate: vi.fn((strategy, options) => (req: Request, res: Response, next: NextFunction) => {
+    authenticate: vi.fn(() => (req: Request, res: Response, next: NextFunction) => {
       // This mock will be controlled by the isAdmin mock, but we ensure it calls next()
       // to allow the isAdmin middleware to run.
       next();
     }),
   },
   // Mock the named export 'isAdmin'
-  isAdmin: vi.fn((_req: Request, res: Response, _next: NextFunction) => {
+  isAdmin: vi.fn((_req: Request, res: Response) => {
     // The default behavior of this mock is to deny access.
     // We will override this implementation in specific tests.
     res.status(401).json({ message: 'Unauthorized' });
@@ -58,8 +58,8 @@ describe('Admin Routes (/api/admin)', () => {
   beforeEach(() => {
     vi.clearAllMocks();
     // Reset the isAdmin mock to its default implementation before each test.
-    // This prevents mock configurations from one test leaking into another.
-    mockedIsAdmin.mockImplementation((_req: Request, res: Response, _next: NextFunction) => {
+    // This prevents mock configurations from one test leaking into another.    
+    mockedIsAdmin.mockImplementation((_req: Request, res: Response) => {
       // The default behavior is to deny access, which is correct for unauthenticated tests.
       res.status(401).json({ message: 'Unauthorized' });
     });
@@ -68,8 +68,8 @@ describe('Admin Routes (/api/admin)', () => {
   it('should deny access if user is not an admin', async () => {
     // Arrange: Configure the isAdmin mock to simulate a non-admin user.
     // It will call next(), but since req.user.role is not 'admin', the real logic
-    // inside the original isAdmin would fail. Here, we just simulate the end result of a 403 Forbidden.
-    mockedIsAdmin.mockImplementation((_req: Request, res: Response, _next: NextFunction) => {
+    // inside the original isAdmin would fail. Here, we just simulate the end result of a 403 Forbidden.    
+    mockedIsAdmin.mockImplementation((_req: Request, res: Response) => {
       res.status(403).json({ message: 'Forbidden: Administrator access required.' });
     });
 

src/routes/admin.ts

@@ -1,5 +1,5 @@
 // src/routes/admin.ts
-import { Router, Request, Response, NextFunction } from 'express';
+import { Router } from 'express';
 import passport from './passport';
 import { isAdmin } from './passport'; // Correctly imported
 import multer from 'multer';
@@ -7,7 +7,6 @@ import multer from 'multer';
 import * as db from '../services/db';
 import { logger } from '../services/logger.server';
 import { UserProfile } from '../types';
-import { AsyncRequestHandler } from '../types/express';
 import { clearGeocodeCache } from '../services/geocodingService.server';
 
 // --- Bull Board (Job Queue UI) Imports ---
@@ -56,27 +55,27 @@ router.use(passport.authenticate('jwt', { session: false }), isAdmin);
 
 // --- Admin Routes ---
 
-router.get('/corrections', async (req, res, next) => {
+router.get('/corrections', async (req, res) => {
     const corrections = await db.getSuggestedCorrections();
     res.json(corrections);
 });
 
-router.get('/brands', async (req, res, next) => {
+router.get('/brands', async (req, res) => {
     const brands = await db.getAllBrands();
     res.json(brands);
 });
 
-router.get('/stats', async (req, res, next) => {
+router.get('/stats', async (req, res) => {
     const stats = await db.getApplicationStats();
     res.json(stats);
 });
 
-router.get('/stats/daily', async (req, res, next) => {
+router.get('/stats/daily', async (req, res) => {
     const dailyStats = await db.getDailyStatsForLast30Days();
     res.json(dailyStats);
 });
 
-router.post('/corrections/:id/approve', async (req, res, next) => {
+router.post('/corrections/:id/approve', async (req, res) => {
     const correctionId = parseInt(req.params.id, 10);
     // Add validation to ensure the ID is a valid number.
     if (isNaN(correctionId)) {
@@ -87,7 +86,7 @@ router.post('/corrections/:id/approve', async (req, res, next) => {
     res.status(200).json({ message: 'Correction approved successfully.' });
 });
 
-router.post('/corrections/:id/reject', async (req, res, next) => {
+router.post('/corrections/:id/reject', async (req, res) => {
     const correctionId = parseInt(req.params.id, 10);
     await db.rejectCorrection(correctionId);
     res.status(200).json({ message: 'Correction rejected successfully.' });
@@ -111,7 +110,7 @@ router.put('/corrections/:id', async (req, res, next) => {
     }
 });
 
-router.put('/recipes/:id/status', async (req, res, next) => {
+router.put('/recipes/:id/status', async (req, res) => {
     const recipeId = parseInt(req.params.id, 10);
     const { status } = req.body;
 
@@ -122,7 +121,7 @@ router.put('/recipes/:id/status', async (req, res, next) => {
     res.status(200).json(updatedRecipe);
 });
 
-router.post('/brands/:id/logo', upload.single('logoImage'), async (req, res, next) => {
+router.post('/brands/:id/logo', upload.single('logoImage'), async (req, res) => {
     const brandId = parseInt(req.params.id, 10);
     if (!req.file) {
         return res.status(400).json({ message: 'Logo image file is required.' });
@@ -135,12 +134,12 @@ router.post('/brands/:id/logo', upload.single('logoImage'), async (req, res, nex
     res.status(200).json({ message: 'Brand logo updated successfully.', logoUrl });
 });
 
-router.get('/unmatched-items', async (req, res, next) => {
+router.get('/unmatched-items', async (req, res) => {
     const items = await db.getUnmatchedFlyerItems();
     res.json(items);
 });
 
-router.put('/comments/:id/status', async (req, res, next) => {
+router.put('/comments/:id/status', async (req, res) => {
     const commentId = parseInt(req.params.id, 10);
     const { status } = req.body;
 
@@ -151,19 +150,19 @@ router.put('/comments/:id/status', async (req, res, next) => {
     res.status(200).json(updatedComment);
 });
 
-router.get('/users', async (req, res, next) => {
+router.get('/users', async (req, res) => {
     const users = await db.getAllUsers();
     res.json(users);
 });
 
-router.get('/activity-log', async (req, res, next) => {
+router.get('/activity-log', async (req, res) => {
     const limit = parseInt(req.query.limit as string, 10) || 50;
     const offset = parseInt(req.query.offset as string, 10) || 0;
     const logs = await db.getActivityLog(limit, offset);
     res.json(logs);
 });
 
-router.get('/users/:id', async (req, res, next) => {
+router.get('/users/:id', async (req, res) => {
     const user = await db.findUserProfileById(req.params.id);
     if (!user) {
         return res.status(404).json({ message: 'User not found.' });
@@ -190,7 +189,7 @@ router.put('/users/:id', async (req, res, next) => {
     }
 });
 
-router.delete('/users/:id', async (req, res, next) => {
+router.delete('/users/:id', async (req, res) => {
     const adminUser = req.user as UserProfile;
     if (adminUser.user.user_id === req.params.id) {
         return res.status(400).json({ message: 'Admins cannot delete their own account.' });

src/routes/auth.ts

@@ -10,7 +10,6 @@ import passport from './passport';
 import * as db from '../services/db';
 import { getPool } from '../services/db/connection';
 import { logger } from '../services/logger.server';
-import { AsyncRequestHandler } from '../types/express';
 import { sendPasswordResetEmail } from '../services/emailService.server';
 
 const router = Router();
@@ -42,7 +41,7 @@ const resetPasswordLimiter = rateLimit({
 // --- Authentication Routes ---
 
 // Registration Route
-router.post('/register', async (req, res, next) => {
+router.post('/register', async (req, res) => {
   const { email, password, full_name, avatar_url } = req.body;
 
   if (!email || !password) {

src/routes/budget.test.ts

@@ -36,7 +36,7 @@ app.use(express.json());
 app.use('/api/budgets', budgetRouter);
 
 // Add a basic error handler to return JSON errors instead of Express default HTML
-app.use((err: any, req: Request, res: Response, next: NextFunction) => {
+app.use((err: Error, req: Request, res: Response) => {
   res.status(500).json({ message: 'Internal Server Error' });
 });
 
@@ -54,7 +54,7 @@ describe('Budget Routes (/api/budgets)', () => {
   beforeEach(() => {
     vi.clearAllMocks();
     // Default to unauthorized
-    mockAuthMiddleware = (req: Request, res: Response, _next: NextFunction) => {
+    mockAuthMiddleware = (req: Request, res: Response) => {
       res.status(401).json({ message: 'Unauthorized' });
     };
   });

src/routes/budget.ts

@@ -1,5 +1,5 @@
 // src/routes/budget.ts
-import express, { Request, Response, NextFunction } from 'express';
+import express from 'express';
 import passport from './passport';
 import {
   getBudgetsForUser,
@@ -10,7 +10,6 @@ import {
 } from '../services/db';
 import { logger } from '../services/logger.server';
 import { UserProfile } from '../types';
-import { AsyncRequestHandler } from '../types/express';
 
 const router = express.Router();