e.stopPropagation()}
+ role="dialog"
+ aria-modal="true"
>
Voice Assistant
-
@@ -195,6 +197,7 @@ export const VoiceAssistant: React.FC
= ({ isOpen, onClose
diff --git a/src/hooks/useAiAnalysis.ts b/src/hooks/useAiAnalysis.ts
index b1c684cd..96e005cb 100644
--- a/src/hooks/useAiAnalysis.ts
+++ b/src/hooks/useAiAnalysis.ts
@@ -26,6 +26,7 @@ export const useAiAnalysis = ({ flyerItems, selectedFlyer, watchedItems }: UseAi
// --- State for results ---
const [results, setResults] = useState<{ [key in AnalysisType]?: string }>({});
const [sources, setSources] = useState<{ [key in AnalysisType]?: Source[] }>({});
+ const [internalError, setInternalError] = useState(null);
// --- API Hooks for each analysis type ---
const { execute: getQuickInsights, data: quickInsightsData, loading: loadingQuickInsights, error: errorQuickInsights } = useApi(aiApiClient.getQuickInsights);
@@ -45,9 +46,10 @@ export const useAiAnalysis = ({ flyerItems, selectedFlyer, watchedItems }: UseAi
}), [loadingQuickInsights, loadingDeepDive, loadingWebSearch, loadingTripPlan, loadingComparePrices]);
const error = useMemo(() => {
+ if (internalError) return internalError;
const firstError = errorQuickInsights || errorDeepDive || errorWebSearch || errorTripPlan || errorComparePrices || errorGenerateImage;
return firstError ? firstError.message : null;
- }, [errorQuickInsights, errorDeepDive, errorWebSearch, errorTripPlan, errorComparePrices, errorGenerateImage]);
+ }, [internalError, errorQuickInsights, errorDeepDive, errorWebSearch, errorTripPlan, errorComparePrices, errorGenerateImage]);
// --- Effects to update state when API data changes ---
useEffect(() => {
@@ -76,6 +78,7 @@ export const useAiAnalysis = ({ flyerItems, selectedFlyer, watchedItems }: UseAi
const generatedImageUrl = useMemo(() => generatedImageData ? `data:image/png;base64,${generatedImageData}` : null, [generatedImageData]);
const runAnalysis = useCallback(async (type: AnalysisType) => {
+ setInternalError(null);
try {
if (type === AnalysisType.QUICK_INSIGHTS) {
await getQuickInsights(flyerItems);
@@ -95,13 +98,23 @@ export const useAiAnalysis = ({ flyerItems, selectedFlyer, watchedItems }: UseAi
// The useApi hook now handles setting the error state.
// We can add specific logging here if needed.
logger.error(`runAnalysis caught an error for type ${type}`, { error: e });
- if (e instanceof GeolocationPositionError && e.code === GeolocationPositionError.PERMISSION_DENIED) {
- // The useApi hook won't catch this, so we can manually set an error.
- // However, the current useApi implementation doesn't expose setError.
- // For now, we rely on the error thrown by useApi's execute function.
- // A future improvement could be to have useApi return its setError function.
+ let message = 'An unexpected error occurred';
+
+ // Check for Geolocation error specifically or by code (1 = PERMISSION_DENIED)
+ if (
+ (typeof e === 'object' && e !== null && 'code' in e && (e as any).code === 1) ||
+ (typeof GeolocationPositionError !== 'undefined' && e instanceof GeolocationPositionError && e.code === GeolocationPositionError.PERMISSION_DENIED)
+ ) {
+ message = "Geolocation permission denied.";
+ } else if (e instanceof Error) {
+ message = e.message;
+ } else if (typeof e === 'object' && e !== null && 'message' in e) {
+ message = (e as any).message;
+ } else if (typeof e === 'string') {
+ message = e;
}
- }
+
+ setInternalError(message); }
}, [
flyerItems,
selectedFlyer?.store,
@@ -116,10 +129,20 @@ export const useAiAnalysis = ({ flyerItems, selectedFlyer, watchedItems }: UseAi
const generateImage = useCallback(async () => {
const mealPlanText = results[AnalysisType.DEEP_DIVE];
if (!mealPlanText) return;
+ setInternalError(null);
try {
await generateImageApi(mealPlanText);
} catch (e) {
logger.error('generateImage failed', { error: e });
+ let message = 'An unexpected error occurred';
+ if (e instanceof Error) {
+ message = e.message;
+ } else if (typeof e === 'object' && e !== null && 'message' in e) {
+ message = (e as any).message;
+ } else if (typeof e === 'string') {
+ message = e;
+ }
+ setInternalError(message);
}
}, [results, generateImageApi]);
diff --git a/src/hooks/useAuth.test.tsx b/src/hooks/useAuth.test.tsx
index 00edca6d..ab178574 100644
--- a/src/hooks/useAuth.test.tsx
+++ b/src/hooks/useAuth.test.tsx
@@ -8,7 +8,11 @@ import * as apiClient from '../services/apiClient';
import type { User, UserProfile } from '../types';
// Mock the dependencies
-vi.mock('../services/apiClient');
+vi.mock('../services/apiClient', () => ({
+ // Mock other functions if needed
+ getAuthenticatedUserProfile: vi.fn(),
+}));
+
vi.mock('../services/logger.client', () => ({
logger: {
info: vi.fn(),
@@ -90,8 +94,8 @@ describe('useAuth Hook and AuthProvider', () => {
localStorageMock.setItem('authToken', 'valid-token');
mockedApiClient.getAuthenticatedUserProfile.mockResolvedValue({
ok: true,
- json: () => Promise.resolve(mockProfile),
- } as Response);
+ json: async () => mockProfile,
+ } as unknown as Response);
const { result } = renderHook(() => useAuth(), { wrapper });
@@ -127,11 +131,10 @@ describe('useAuth Hook and AuthProvider', () => {
describe('login function', () => {
it('sets token, fetches profile, and updates state on successful login', async () => {
- // Mock the API response for the login call
mockedApiClient.getAuthenticatedUserProfile.mockResolvedValue({
ok: true,
- json: () => Promise.resolve(mockProfile),
- } as Response);
+ json: async () => mockProfile,
+ } as unknown as Response);
const { result } = renderHook(() => useAuth(), { wrapper });
@@ -181,8 +184,8 @@ describe('useAuth Hook and AuthProvider', () => {
localStorageMock.setItem('authToken', 'valid-token');
mockedApiClient.getAuthenticatedUserProfile.mockResolvedValue({
ok: true,
- json: () => Promise.resolve(mockProfile),
- } as Response);
+ json: async () => mockProfile,
+ } as unknown as Response);
const { result } = renderHook(() => useAuth(), { wrapper });
await waitFor(() => expect(result.current.authStatus).toBe('AUTHENTICATED'));
@@ -206,8 +209,8 @@ describe('useAuth Hook and AuthProvider', () => {
localStorageMock.setItem('authToken', 'valid-token');
mockedApiClient.getAuthenticatedUserProfile.mockResolvedValue({
ok: true,
- json: () => Promise.resolve(mockProfile),
- } as Response);
+ json: async () => mockProfile,
+ } as unknown as Response);
const { result } = renderHook(() => useAuth(), { wrapper });
await waitFor(() => expect(result.current.authStatus).toBe('AUTHENTICATED'));
diff --git a/src/pages/admin/components/AdminBrandManager.test.tsx b/src/pages/admin/components/AdminBrandManager.test.tsx
index 6b6616ff..f4722667 100644
--- a/src/pages/admin/components/AdminBrandManager.test.tsx
+++ b/src/pages/admin/components/AdminBrandManager.test.tsx
@@ -53,7 +53,7 @@ describe('AdminBrandManager', () => {
});
it('should render the list of brands when data is fetched successfully', async () => {
- mockedApiClient.fetchAllBrands.mockResolvedValue(new Response(JSON.stringify(mockBrands)));
+ mockedApiClient.fetchAllBrands.mockImplementation(async () => new Response(JSON.stringify(mockBrands)));
render();
await waitFor(() => {
expect(screen.getByRole('heading', { name: /brand management/i })).toBeInTheDocument();
@@ -65,8 +65,8 @@ describe('AdminBrandManager', () => {
});
it('should handle successful logo upload', async () => {
- mockedApiClient.fetchAllBrands.mockResolvedValue(new Response(JSON.stringify(mockBrands)));
- mockedApiClient.uploadBrandLogo.mockResolvedValue(new Response(JSON.stringify({ logoUrl: 'http://example.com/new-logo.png' })));
+ mockedApiClient.fetchAllBrands.mockImplementation(async () => new Response(JSON.stringify(mockBrands)));
+ mockedApiClient.uploadBrandLogo.mockImplementation(async () => new Response(JSON.stringify({ logoUrl: 'http://example.com/new-logo.png' })));
mockedToast.loading.mockReturnValue('toast-1');
render();
@@ -88,7 +88,7 @@ describe('AdminBrandManager', () => {
});
it('should handle failed logo upload', async () => {
- mockedApiClient.fetchAllBrands.mockResolvedValue(new Response(JSON.stringify(mockBrands)));
+ mockedApiClient.fetchAllBrands.mockImplementation(async () => new Response(JSON.stringify(mockBrands)));
mockedApiClient.uploadBrandLogo.mockRejectedValue(new Error('Upload failed'));
mockedToast.loading.mockReturnValue('toast-2');
@@ -106,7 +106,7 @@ describe('AdminBrandManager', () => {
});
it('should show an error toast for invalid file type', async () => {
- mockedApiClient.fetchAllBrands.mockResolvedValue(new Response(JSON.stringify(mockBrands)));
+ mockedApiClient.fetchAllBrands.mockImplementation(async () => new Response(JSON.stringify(mockBrands)));
render();
await waitFor(() => expect(screen.getByText('No Frills')).toBeInTheDocument());
@@ -122,7 +122,7 @@ describe('AdminBrandManager', () => {
});
it('should show an error toast for oversized file', async () => {
- mockedApiClient.fetchAllBrands.mockResolvedValue(new Response(JSON.stringify(mockBrands)));
+ mockedApiClient.fetchAllBrands.mockImplementation(async () => new Response(JSON.stringify(mockBrands)));
render();
await waitFor(() => expect(screen.getByText('No Frills')).toBeInTheDocument());
diff --git a/src/pages/admin/components/ProfileManager.Authenticated.test.tsx b/src/pages/admin/components/ProfileManager.Authenticated.test.tsx
index 68ed0c56..dab04790 100644
--- a/src/pages/admin/components/ProfileManager.Authenticated.test.tsx
+++ b/src/pages/admin/components/ProfileManager.Authenticated.test.tsx
@@ -135,8 +135,8 @@ describe('ProfileManager Authenticated User Features', () => {
// Wait for the updates to complete and assertions
await waitFor(() => {
- expect(mockedApiClient.updateUserProfile).toHaveBeenCalledWith({ full_name: 'Updated Name', avatar_url: authenticatedProfile.avatar_url }, { signal: expect.any(AbortSignal) });
- expect(mockedApiClient.updateUserAddress).toHaveBeenCalledWith(expect.objectContaining({ ...mockAddress, city: 'NewCity' }), { signal: expect.any(AbortSignal) });
+ expect(mockedApiClient.updateUserProfile).toHaveBeenCalledWith({ full_name: 'Updated Name', avatar_url: authenticatedProfile.avatar_url }, expect.objectContaining({ signal: expect.anything() }));
+ expect(mockedApiClient.updateUserAddress).toHaveBeenCalledWith(expect.objectContaining({ ...mockAddress, city: 'NewCity' }), expect.objectContaining({ signal: expect.anything() }));
expect(mockOnProfileUpdate).toHaveBeenCalledWith(expect.objectContaining({ full_name: 'Updated Name' }));
expect(notifySuccess).toHaveBeenCalledWith(expect.stringMatching(/Profile.*updated/));
});
@@ -184,10 +184,17 @@ describe('ProfileManager Authenticated User Features', () => {
fireEvent.change(screen.getByLabelText(/city/i), { target: { value: 'NewCity' } });
fireEvent.click(screen.getByRole('button', { name: /save profile/i }));
+ // The component logic is: if profile succeeds but address fails, it shows a specific success message about partial updates
await waitFor(() => {
- expect(notifyError).toHaveBeenCalledWith('Address update failed');
+ expect(notifySuccess).toHaveBeenCalledWith('Profile details updated, but address failed to save.');
});
+ // onProfileUpdate should NOT be called if there's a partial failure that involves core profile data not being the main focus,
+ // or if the component logic decides to withhold the update.
+ // However, looking at the component code:
+ // "if (profileDataChanged && index === 0 && result.value) { updatedProfileData = result.value as Profile; onProfileUpdate(updatedProfileData); }"
+ // Since we DID NOT change profile data in this test (inputs for name/avatar weren't changed), `profileDataChanged` is false.
+ // So onProfileUpdate is NOT called. Correct.
expect(mockOnProfileUpdate).not.toHaveBeenCalled();
});
@@ -201,7 +208,7 @@ describe('ProfileManager Authenticated User Features', () => {
fireEvent.submit(screen.getByTestId('update-password-form'));
await waitFor(() => {
- expect(mockedApiClient.updateUserPassword).toHaveBeenCalledWith('newpassword123', expect.any(AbortSignal));
+ expect(mockedApiClient.updateUserPassword).toHaveBeenCalledWith('newpassword123', expect.objectContaining({ signal: expect.anything() }));
expect(notifySuccess).toHaveBeenCalledWith('Password updated successfully!');
});
});
@@ -250,7 +257,7 @@ describe('ProfileManager Authenticated User Features', () => {
fireEvent.click(confirmButton);
await waitFor(() => {
- expect(mockedApiClient.deleteUserAccount).toHaveBeenCalledWith('correctpassword', expect.any(AbortSignal));
+ expect(mockedApiClient.deleteUserAccount).toHaveBeenCalledWith('correctpassword', expect.objectContaining({ signal: expect.anything() }));
expect(notifySuccess).toHaveBeenCalledWith("Account deleted successfully. You will be logged out shortly.");
});
@@ -293,7 +300,7 @@ describe('ProfileManager Authenticated User Features', () => {
fireEvent.click(darkModeToggle);
await waitFor(() => {
- expect(mockedApiClient.updateUserPreferences).toHaveBeenCalledWith({ darkMode: true }, expect.any(AbortSignal));
+ expect(mockedApiClient.updateUserPreferences).toHaveBeenCalledWith({ darkMode: true }, expect.objectContaining({ signal: expect.anything() }));
expect(mockOnProfileUpdate).toHaveBeenCalledWith(
expect.objectContaining({ preferences: expect.objectContaining({ darkMode: true }) })
);
@@ -313,7 +320,7 @@ describe('ProfileManager Authenticated User Features', () => {
fireEvent.click(metricRadio);
await waitFor(() => {
- expect(mockedApiClient.updateUserPreferences).toHaveBeenCalledWith({ unitSystem: 'metric' }, expect.any(AbortSignal));
+ expect(mockedApiClient.updateUserPreferences).toHaveBeenCalledWith({ unitSystem: 'metric' }, expect.objectContaining({ signal: expect.anything() }));
expect(mockOnProfileUpdate).toHaveBeenCalledWith(expect.objectContaining({ preferences: expect.objectContaining({ unitSystem: 'metric' }) }));
});
});
diff --git a/src/pages/admin/components/SystemCheck.test.tsx b/src/pages/admin/components/SystemCheck.test.tsx
index 932b218c..459b8840 100644
--- a/src/pages/admin/components/SystemCheck.test.tsx
+++ b/src/pages/admin/components/SystemCheck.test.tsx
@@ -26,8 +26,8 @@ describe('SystemCheck', () => {
beforeEach(() => {
vi.clearAllMocks();
- // CRITICAL FIX: Use `mockImplementation` to create a new Response object for every call.
- // This prevents the "Body has already been read" error and the resulting memory leak.
+ // Use `mockImplementation` to create a new Response object for every call.
+ // This prevents "Body has already been read" errors and memory leaks.
mockedApiClient.pingBackend.mockImplementation(() => Promise.resolve(new Response('pong')));
mockedApiClient.checkStorage.mockImplementation(() => Promise.resolve(new Response(JSON.stringify({ success: true, message: 'Storage OK' }))));
mockedApiClient.checkDbPoolHealth.mockImplementation(() => Promise.resolve(new Response(JSON.stringify({ success: true, message: 'DB Pool OK' }))));
@@ -37,7 +37,7 @@ describe('SystemCheck', () => {
mockedApiClient.loginUser.mockImplementation(() => Promise.resolve(new Response(JSON.stringify({ user: {}, token: '' }), { status: 200 })));
// Reset GEMINI_API_KEY for each test to its original value.
- import.meta.env.GEMINI_API_KEY = originalGeminiApiKey;
+ setGeminiApiKey(originalGeminiApiKey);
});
// Restore all mocks after each test to ensure test isolation.
@@ -48,7 +48,11 @@ describe('SystemCheck', () => {
// Helper to set GEMINI_API_KEY for specific tests.
const setGeminiApiKey = (value: string | undefined) => {
- import.meta.env.GEMINI_API_KEY = value;
+ if (value === undefined) {
+ delete (import.meta.env as any).GEMINI_API_KEY;
+ } else {
+ (import.meta.env as any).GEMINI_API_KEY = value;
+ }
};
it('should render initial idle state and then run checks automatically on mount', async () => {
diff --git a/src/routes/ai.routes.ts b/src/routes/ai.routes.ts
index d31c5ac9..daf9fac3 100644
--- a/src/routes/ai.routes.ts
+++ b/src/routes/ai.routes.ts
@@ -30,7 +30,9 @@ interface FlyerProcessPayload extends Partial {
const uploadAndProcessSchema = z.object({
body: z.object({
- checksum: z.string().min(1, 'File checksum is required.'),
+ checksum: z.string().refine(val => val && val.length > 0, {
+ message: 'File checksum is required.',
+ }),
}),
});
@@ -40,9 +42,18 @@ const jobIdParamSchema = z.object({
}),
});
+// Helper to safely extract an error message from unknown `catch` values.
+const errMsg = (e: unknown) => {
+ if (e instanceof Error) return e.message;
+ if (typeof e === 'object' && e !== null && 'message' in e) return String((e as { message: unknown }).message);
+ return String(e || 'An unknown error occurred.');
+};
+
const rescanAreaSchema = z.object({
body: z.object({
- cropArea: z.string().transform((val, ctx) => {
+ cropArea: z.string().refine(val => val && val.length > 0, {
+ message: 'cropArea must be a valid JSON string.',
+ }).transform((val, ctx) => {
try { return JSON.parse(val); }
catch (err) {
// Log the actual parsing error for better debugging if invalid JSON is sent.
@@ -50,7 +61,9 @@ const rescanAreaSchema = z.object({
ctx.addIssue({ code: z.ZodIssueCode.custom, message: 'cropArea must be a valid JSON string.' }); return z.NEVER;
}
}),
- extractionType: z.string().min(1, 'extractionType is required.'),
+ extractionType: z.string().refine(val => val && val.length > 0, {
+ message: 'extractionType is required.',
+ }),
}),
});
@@ -89,13 +102,6 @@ const searchWebSchema = z.object({
body: z.object({ query: z.string().min(1, 'A search query is required.') }),
});
-// Helper to safely extract an error message from unknown `catch` values.
-const errMsg = (e: unknown) => {
- if (e instanceof Error) return e.message;
- if (typeof e === 'object' && e !== null && 'message' in e) return String((e as { message: unknown }).message);
- return String(e || 'An unknown error occurred.');
-};
-
// --- Multer Configuration for File Uploads ---
const storagePath = process.env.STORAGE_PATH || '/var/www/flyer-crawler.projectium.com/flyer-images';
@@ -104,20 +110,20 @@ try {
fs.mkdirSync(storagePath, { recursive: true });
logger.debug(`AI upload storage path ready: ${storagePath}`);
} catch (err) {
- logger.error({ error: err }, `Failed to create storage path (${storagePath}). File uploads may fail.`);
+ logger.error({ error: errMsg(err) }, `Failed to create storage path (${storagePath}). File uploads may fail.`);
}
-const diskStorage = multer.diskStorage({
+const diskStorage = multer.diskStorage({
destination: function (req, file, cb) {
cb(null, storagePath);
},
filename: function (req, file, cb) {
// If in a test environment, use a predictable filename for easy cleanup.
if (process.env.NODE_ENV === 'test') {
- cb(null, `${file.fieldname}-test-flyer-image.jpg`);
+ return cb(null, `${file.fieldname}-test-flyer-image.jpg`);
} else {
const uniqueSuffix = Date.now() + '-' + Math.round(Math.random() * 1E9);
// Sanitize the original filename to remove spaces and special characters
- cb(null, file.fieldname + '-' + uniqueSuffix + '-' + sanitizeFilename(file.originalname));
+ return cb(null, file.fieldname + '-' + uniqueSuffix + '-' + sanitizeFilename(file.originalname));
}
}
});
@@ -131,7 +137,7 @@ router.use((req: Request, res: Response, next: NextFunction) => {
const contentLength = req.headers['content-length'] || 'unknown';
const authPresent = !!req.headers['authorization'];
logger.debug({ method: req.method, url: req.originalUrl, contentType, contentLength, authPresent }, '[API /ai] Incoming request');
- } catch (e) {
+ } catch (e: unknown) {
logger.error({ error: e }, 'Failed to log incoming AI request headers');
}
next();
@@ -202,7 +208,6 @@ router.get('/jobs/:jobId/status', validateRequest(jobIdParamSchema), async (req,
const job = await flyerQueue.getJob(jobId);
if (!job) {
// Adhere to ADR-001 by throwing a specific error to be handled centrally.
- throw new NotFoundError('Job not found.');
return res.status(404).json({ message: 'Job not found.' });
}
const state = await job.getState();
@@ -246,7 +251,7 @@ router.post('/flyers/process', optionalAuth, uploadToDisk.single('flyerImage'),
} catch (err) {
logger.warn({ error: errMsg(err) }, '[API /ai/flyers/process] Failed to JSON.parse raw extractedData; falling back to direct assign');
parsed = (typeof raw === 'string' ? JSON.parse(String(raw).slice(0, 2000)) : raw) as FlyerProcessPayload;
- }
+ }
// If parsed itself contains an `extractedData` field, use that, otherwise assume parsed is the extractedData
extractedData = parsed.extractedData ?? (parsed as Partial);
} else {
@@ -255,7 +260,7 @@ router.post('/flyers/process', optionalAuth, uploadToDisk.single('flyerImage'),
parsed = typeof req.body === 'string' ? JSON.parse(req.body) : req.body;
} catch (err) {
logger.warn({ error: errMsg(err) }, '[API /ai/flyers/process] Failed to JSON.parse req.body; using empty object');
- parsed = req.body || {};
+ parsed = req.body as FlyerProcessPayload || {};
}
// extractedData might be nested under `data` or `extractedData`, or the body itself may be the extracted data.
if (parsed.data) {
@@ -264,7 +269,7 @@ router.post('/flyers/process', optionalAuth, uploadToDisk.single('flyerImage'),
extractedData = inner.extractedData ?? inner;
} catch (err) {
logger.warn({ error: errMsg(err) }, '[API /ai/flyers/process] Failed to parse parsed.data; falling back');
- extractedData = parsed.data as Partial;
+ extractedData = parsed.data as unknown as Partial;
}
} else if (parsed.extractedData) {
extractedData = parsed.extractedData;
@@ -473,7 +478,9 @@ router.post(
if (!req.file) {
return res.status(400).json({ message: 'Image file is required.' });
}
- const cropArea = JSON.parse(req.body.cropArea);
+ // validateRequest transforms the cropArea JSON string into an object in req.body.
+ // So we use it directly instead of JSON.parse().
+ const cropArea = req.body.cropArea;
const { extractionType } = req.body;
const { path, mimetype } = req.file;
diff --git a/src/routes/auth.routes.test.ts b/src/routes/auth.routes.test.ts
index 48ccf8f5..53eb4f27 100644
--- a/src/routes/auth.routes.test.ts
+++ b/src/routes/auth.routes.test.ts
@@ -1,7 +1,7 @@
// src/routes/auth.routes.test.ts
import { describe, it, expect, vi, beforeEach } from 'vitest';
import supertest from 'supertest';
-import { Request, Response, NextFunction } from 'express';
+import { Request, Response, NextFunction, RequestHandler } from 'express';
import cookieParser from 'cookie-parser';
import * as bcrypt from 'bcrypt';
import { createMockUserProfile, createMockUserWithPasswordHash } from '../tests/utils/mockFactories';
@@ -137,6 +137,13 @@ import { errorHandler } from '../middleware/errorHandler'; // Assuming this exis
const app = express();
app.use(express.json());
app.use(cookieParser()); // Mount BEFORE router
+
+// Middleware to inject the mock logger into req
+app.use((req, res, next) => {
+ req.log = mockLogger;
+ next();
+});
+
app.use('/api/auth', authRouter);
app.use(errorHandler); // Mount AFTER router
@@ -517,7 +524,10 @@ describe('Auth Routes (/api/auth)', () => {
// Assert
expect(response.status).toBe(200);
- expect(logger.error).toHaveBeenCalledWith('Failed to delete refresh token from DB during logout.', { error: dbError });
+ expect(logger.error).toHaveBeenCalledWith(
+ expect.objectContaining({ error: dbError }),
+ 'Failed to delete refresh token from DB during logout.'
+ );
});
});
});
\ No newline at end of file
diff --git a/src/routes/flyer.routes.test.ts b/src/routes/flyer.routes.test.ts
index 591b36b5..567015ed 100644
--- a/src/routes/flyer.routes.test.ts
+++ b/src/routes/flyer.routes.test.ts
@@ -174,7 +174,7 @@ describe('Flyer Routes (/api/flyers)', () => {
describe('POST /items/batch-count', () => {
it('should return the count of items for multiple flyers', async () => {
- vi.mocked(db.flyerRepo.countFlyerItemsForFlyers).mockResolvedValue(42);
+ vi.mocked(db.flyerRepo.countFlyerItemsForFlyers).mockResolvedValueOnce(42);
const response = await supertest(app)
.post('/api/flyers/items/batch-count')
@@ -192,6 +192,8 @@ describe('Flyer Routes (/api/flyers)', () => {
});
it('should return a count of 0 if flyerIds is empty', async () => {
+ vi.mocked(db.flyerRepo.countFlyerItemsForFlyers).mockResolvedValueOnce(0);
+
const response = await supertest(app)
.post('/api/flyers/items/batch-count')
.send({ flyerIds: [] });
diff --git a/src/routes/flyer.routes.ts b/src/routes/flyer.routes.ts
index 91e68619..56468327 100644
--- a/src/routes/flyer.routes.ts
+++ b/src/routes/flyer.routes.ts
@@ -17,7 +17,7 @@ const getFlyersSchema = z.object({
const flyerIdParamSchema = z.object({
params: z.object({
- id: z.coerce.number().int().positive('Invalid flyer ID provided.'),
+ id: z.coerce.number().int('Invalid flyer ID provided.').positive('Invalid flyer ID provided.'),
}),
});
diff --git a/src/services/db/admin.db.ts b/src/services/db/admin.db.ts
index e0175e88..a88b988b 100644
--- a/src/services/db/admin.db.ts
+++ b/src/services/db/admin.db.ts
@@ -347,6 +347,9 @@ export class AdminRepository {
logger.info(`Successfully resolved unmatched item ${unmatchedFlyerItemId} to master item ${masterItemId}.`);
});
} catch (error) {
+ if (error instanceof NotFoundError) {
+ throw error;
+ }
logger.error({ err: error, unmatchedFlyerItemId, masterItemId }, 'Database transaction error in resolveUnmatchedFlyerItem');
throw new Error('Failed to resolve unmatched flyer item.');
}
diff --git a/src/services/db/budget.db.test.ts b/src/services/db/budget.db.test.ts
index 5695bc31..3a98a859 100644
--- a/src/services/db/budget.db.test.ts
+++ b/src/services/db/budget.db.test.ts
@@ -27,17 +27,19 @@ vi.mock('./connection.db', async (importOriginal) => {
return { ...actual, withTransaction: vi.fn() };
});
-// Mock the gamification repository, as createBudget calls it.
-vi.mock('./gamification.db', () => ({
- GamificationRepository: class { awardAchievement = vi.fn(); },
+const { mockedAwardAchievement } = vi.hoisted(() => ({
+ mockedAwardAchievement: vi.fn(),
}));
-import { withTransaction } from './connection.db';
// Mock the gamification repository, as createBudget calls it.
vi.mock('./gamification.db', () => ({
- GamificationRepository: class { awardAchievement = vi.fn(); },
+ GamificationRepository: class {
+ awardAchievement = mockedAwardAchievement;
+ },
}));
+import { withTransaction } from './connection.db';
+
describe('Budget DB Service', () => {
let budgetRepo: BudgetRepository;
@@ -92,9 +94,8 @@ describe('Budget DB Service', () => {
const result = await budgetRepo.createBudget('user-123', budgetData, mockLogger);
// Now we can assert directly on the mockClient we created.
- const { GamificationRepository } = await import('./gamification.db');
expect(mockClient.query).toHaveBeenCalledWith(expect.stringContaining('INSERT INTO public.budgets'), expect.any(Array));
- expect(GamificationRepository.prototype.awardAchievement).toHaveBeenCalledWith('user-123', 'First Budget Created', mockLogger);
+ expect(mockedAwardAchievement).toHaveBeenCalledWith('user-123', 'First Budget Created', mockLogger);
expect(result).toEqual(mockCreatedBudget);
expect(withTransaction).toHaveBeenCalledTimes(1);
});
@@ -119,16 +120,19 @@ describe('Budget DB Service', () => {
const budgetData = { name: 'Groceries', amount_cents: 50000, period: 'monthly' as const, start_date: '2024-01-01' };
const mockCreatedBudget: Budget = { budget_id: 1, user_id: 'user-123', ...budgetData };
const achievementError = new Error('Achievement award failed');
+
+ mockedAwardAchievement.mockRejectedValueOnce(achievementError);
+
vi.mocked(withTransaction).mockImplementation(async (callback) => {
const mockClient = { query: vi.fn() };
(mockClient.query as Mock)
- .mockResolvedValueOnce({ rows: [mockCreatedBudget] }) // INSERT...RETURNING
- .mockRejectedValueOnce(achievementError); // award_achievement fails
+ .mockResolvedValueOnce({ rows: [mockCreatedBudget] }); // INSERT...RETURNING
+
await expect(callback(mockClient as unknown as PoolClient)).rejects.toThrow(achievementError);
throw achievementError; // Re-throw for the outer expect
});
- await expect(budgetRepo.createBudget('user-123', budgetData, mockLogger)).rejects.toThrow('Failed to create budget.'); // This was a duplicate, fixed.
+ await expect(budgetRepo.createBudget('user-123', budgetData, mockLogger)).rejects.toThrow('Failed to create budget.');
expect(mockLogger.error).toHaveBeenCalledWith({ err: achievementError, budgetData, userId: 'user-123' }, 'Database error in createBudget');
});
diff --git a/src/services/db/budget.db.ts b/src/services/db/budget.db.ts
index e5d7851b..415be1e1 100644
--- a/src/services/db/budget.db.ts
+++ b/src/services/db/budget.db.ts
@@ -86,6 +86,7 @@ export class BudgetRepository {
if (res.rowCount === 0) throw new NotFoundError('Budget not found or user does not have permission to update.');
return res.rows[0];
} catch (error) {
+ if (error instanceof NotFoundError) throw error;
logger.error({ err: error, budgetId, userId }, 'Database error in updateBudget');
throw new Error('Failed to update budget.');
}
@@ -103,6 +104,7 @@ export class BudgetRepository {
throw new NotFoundError('Budget not found or user does not have permission to delete.');
}
} catch (error) {
+ if (error instanceof NotFoundError) throw error;
logger.error({ err: error, budgetId, userId }, 'Database error in deleteBudget');
throw new Error('Failed to delete budget.');
}
diff --git a/src/services/db/flyer.db.test.ts b/src/services/db/flyer.db.test.ts
index f844bb75..1ed9d152 100644
--- a/src/services/db/flyer.db.test.ts
+++ b/src/services/db/flyer.db.test.ts
@@ -256,8 +256,10 @@ describe('Flyer DB Service', () => {
});
// The transactional function re-throws the original error from the failed step.
- await expect(createFlyerAndItems(flyerData, itemsData, mockLogger)).rejects.toThrow(dbError);
- expect(mockLogger.error).toHaveBeenCalledWith({ err: dbError }, 'Database transaction error in createFlyerAndItems');
+ // Since insertFlyer wraps errors, we expect the wrapped error message.
+ await expect(createFlyerAndItems(flyerData, itemsData, mockLogger)).rejects.toThrow('Failed to insert flyer into database.');
+ // The error object passed to the logger will be the wrapped Error object, not the original dbError
+ expect(mockLogger.error).toHaveBeenCalledWith({ err: expect.any(Error) }, 'Database transaction error in createFlyerAndItems');
expect(withTransaction).toHaveBeenCalledTimes(1);
});
});
@@ -467,7 +469,7 @@ describe('Flyer DB Service', () => {
vi.mocked(withTransaction).mockImplementation(cb => cb(mockClient as unknown as PoolClient));
await expect(flyerRepo.deleteFlyer(999, mockLogger)).rejects.toThrow('Failed to delete flyer.');
- expect(mockLogger.error).toHaveBeenCalledWith({ err: expect.any(NotFoundError) }, 'Database transaction error in deleteFlyer');
+ expect(mockLogger.error).toHaveBeenCalledWith({ err: expect.any(NotFoundError), flyerId: 999 }, 'Database transaction error in deleteFlyer');
});
it('should rollback transaction on generic error', async () => {
@@ -477,7 +479,7 @@ describe('Flyer DB Service', () => {
});
await expect(flyerRepo.deleteFlyer(42, mockLogger)).rejects.toThrow('Failed to delete flyer.'); // This was a duplicate, fixed.
- expect(mockLogger.error).toHaveBeenCalledWith({ err: dbError }, 'Database transaction error in deleteFlyer');
+ expect(mockLogger.error).toHaveBeenCalledWith({ err: dbError, flyerId: 42 }, 'Database transaction error in deleteFlyer');
});
});
});
\ No newline at end of file
diff --git a/src/services/db/flyer.db.ts b/src/services/db/flyer.db.ts
index efe6461c..ce74ccb7 100644
--- a/src/services/db/flyer.db.ts
+++ b/src/services/db/flyer.db.ts
@@ -36,9 +36,14 @@ export class FlyerRepository {
// Check for a unique constraint violation on name, which could happen in a race condition
// if two processes try to create the same store at the same time.
if (error instanceof Error && 'code' in error && error.code === '23505') {
- logger.warn({ storeName }, `Race condition avoided: Store was created by another process. Refetching.`);
- const result = await this.db.query<{ store_id: number }>('SELECT store_id FROM public.stores WHERE name = $1', [storeName]);
- if (result.rows.length > 0) return result.rows[0].store_id;
+ try {
+ logger.warn({ storeName }, `Race condition avoided: Store was created by another process. Refetching.`);
+ const result = await this.db.query<{ store_id: number }>('SELECT store_id FROM public.stores WHERE name = $1', [storeName]);
+ if (result.rows.length > 0) return result.rows[0].store_id;
+ } catch (recoveryError) {
+ // If recovery fails, log a warning and fall through to the generic error handler
+ logger.warn({ err: recoveryError, storeName }, 'Race condition recovery failed');
+ }
}
logger.error({ err: error, storeName }, 'Database error in findOrCreateStore');
throw new Error('Failed to find or create store in database.');
diff --git a/src/services/db/notification.db.test.ts b/src/services/db/notification.db.test.ts
index ecfa020c..e7819830 100644
--- a/src/services/db/notification.db.test.ts
+++ b/src/services/db/notification.db.test.ts
@@ -152,8 +152,8 @@ describe('Notification DB Service', () => {
});
it('should re-throw the specific "not found" error if it occurs', async () => {
- // This tests the `if (error instanceof Error && error.message.startsWith('Notification not found'))` line
- const notFoundError = new Error('Notification not found or user does not have permission.');
+ // This tests the `if (error instanceof NotFoundError)` line
+ const notFoundError = new NotFoundError('Notification not found or user does not have permission.');
mockPoolInstance.query.mockImplementation(() => {
throw notFoundError;
});
diff --git a/src/services/db/recipe.db.test.ts b/src/services/db/recipe.db.test.ts
index f53775b2..f79ca2d6 100644
--- a/src/services/db/recipe.db.test.ts
+++ b/src/services/db/recipe.db.test.ts
@@ -240,7 +240,7 @@ describe('Recipe DB Service', () => {
});
it('should throw NotFoundError if recipe is not found', async () => {
- mockQuery.mockResolvedValue({ rows: [] });
+ mockQuery.mockResolvedValue({ rows: [], rowCount: 0 });
await expect(recipeRepo.getRecipeById(999, mockLogger)).rejects.toThrow('Recipe with ID 999 not found');
});
diff --git a/src/services/db/recipe.db.ts b/src/services/db/recipe.db.ts
index a3d488db..42690d97 100644
--- a/src/services/db/recipe.db.ts
+++ b/src/services/db/recipe.db.ts
@@ -92,6 +92,9 @@ export class RecipeRepository {
}
return res.rows[0];
} catch (error) {
+ if (error instanceof UniqueConstraintError) {
+ throw error;
+ }
logger.error({ err: error, userId, recipeId }, 'Database error in addFavoriteRecipe');
if (error instanceof Error && 'code' in error && error.code === '23503') {
throw new ForeignKeyConstraintError('The specified user or recipe does not exist.');
diff --git a/src/services/db/user.db.ts b/src/services/db/user.db.ts
index 2b94b29d..2ebcd33e 100644
--- a/src/services/db/user.db.ts
+++ b/src/services/db/user.db.ts
@@ -178,11 +178,12 @@ export class UserRepository {
'SELECT user_id, email, password_hash FROM public.users WHERE user_id = $1',
[userId]
);
- if (res.rowCount === 0) {
+ if ((res.rowCount ?? 0) === 0) {
throw new NotFoundError(`User with ID ${userId} not found.`);
}
return res.rows[0];
} catch (error) {
+ if (error instanceof NotFoundError) throw error;
logger.error({ err: error, userId }, 'Database error in findUserWithPasswordHashById');
throw new Error('Failed to retrieve user with sensitive data by ID from database.');
}
@@ -366,7 +367,7 @@ export class UserRepository {
'SELECT user_id, email FROM public.users WHERE refresh_token = $1',
[refreshToken]
);
- if (res.rowCount === 0) {
+ if ((res.rowCount ?? 0) === 0) {
throw new NotFoundError('User not found for the given refresh token.');
}
return res.rows[0];
diff --git a/src/services/flyerProcessingService.server.test.ts b/src/services/flyerProcessingService.server.test.ts
index b625ce2b..c4a72ee5 100644
--- a/src/services/flyerProcessingService.server.test.ts
+++ b/src/services/flyerProcessingService.server.test.ts
@@ -293,9 +293,6 @@ describe('FlyerProcessingService', () => {
userId: 'user-abc',
};
- // The transformer is already spied on in beforeEach, we can just check its call.
- const transformerSpy = vi.spyOn(FlyerDataTransformer.prototype, 'transform');
-
// The DB create function is also mocked in beforeEach.
// Create a complete mock that satisfies the Flyer type.
const mockNewFlyer: Flyer = {
@@ -315,11 +312,15 @@ describe('FlyerProcessingService', () => {
// Assert
// 1. Transformer was called correctly
- expect(transformerSpy).toHaveBeenCalledWith(mockExtractedData, mockImagePaths, mockJobData.originalFileName, mockJobData.checksum, mockJobData.userId, logger);
+ expect(FlyerDataTransformer.prototype.transform).toHaveBeenCalledWith(mockExtractedData, mockImagePaths, mockJobData.originalFileName, mockJobData.checksum, mockJobData.userId, logger);
// 2. DB function was called with the transformed data
- const transformedData = transformerSpy.mock.results[0].value;
- expect(createFlyerAndItems).toHaveBeenCalledWith(transformedData.flyerData, transformedData.itemsForDb);
+ // The data comes from the mock defined in `beforeEach`.
+ expect(createFlyerAndItems).toHaveBeenCalledWith(
+ expect.objectContaining({ store_name: 'Mock Store', checksum: 'checksum-123' }),
+ [], // itemsForDb from the mock
+ logger
+ );
// 3. Activity was logged with all expected fields
expect(mockedDb.adminRepo.logActivity).toHaveBeenCalledWith({
@@ -327,7 +328,7 @@ describe('FlyerProcessingService', () => {
action: 'flyer_processed' as const,
displayText: 'Processed a new flyer for Mock Store.', // This was a duplicate, fixed.
details: { flyerId: 1, storeName: 'Mock Store' },
- });
+ }, logger);
// 4. The method returned the new flyer
expect(result).toEqual(mockNewFlyer);
diff --git a/src/services/logger.server.ts b/src/services/logger.server.ts
index 01138f3b..34a35f13 100644
--- a/src/services/logger.server.ts
+++ b/src/services/logger.server.ts
@@ -7,11 +7,13 @@
import pino from 'pino';
const isProduction = process.env.NODE_ENV === 'production';
+const isTest = process.env.NODE_ENV === 'test';
export const logger = pino({
level: isProduction ? 'info' : 'debug',
// Use pino-pretty for human-readable logs in development, and JSON in production.
- transport: isProduction ? undefined : {
+ // Disable transport in tests to prevent worker thread issues.
+ transport: (isProduction || isTest) ? undefined : {
target: 'pino-pretty',
options: {
colorize: true,
diff --git a/src/services/queueService.workers.test.ts b/src/services/queueService.workers.test.ts
index ad52df1e..7c7c4cf1 100644
--- a/src/services/queueService.workers.test.ts
+++ b/src/services/queueService.workers.test.ts
@@ -25,9 +25,15 @@ const mocks = vi.hoisted(() => {
});
// --- Mock Modules ---
-vi.mock('./emailService.server', () => ({
- sendEmail: mocks.sendEmail,
-}));
+vi.mock('./emailService.server', async (importOriginal) => {
+ const actual = await importOriginal();
+ return {
+ ...actual,
+ // We only need to mock the specific function being called by the worker.
+ // The rest of the module can retain its original implementation if needed elsewhere.
+ sendEmail: mocks.sendEmail,
+ };
+});
// The workers use an `fsAdapter`. We can mock the underlying `fsPromises`
// that the adapter is built from in queueService.server.ts.
@@ -40,7 +46,13 @@ vi.mock('node:fs/promises', () => ({
}));
vi.mock('./logger.server', () => ({
- logger: { info: vi.fn(), error: vi.fn(), warn: vi.fn(), debug: vi.fn() },
+ logger: {
+ info: vi.fn(),
+ error: vi.fn(),
+ warn: vi.fn(),
+ debug: vi.fn(),
+ child: vi.fn().mockReturnThis(),
+ },
}));
// Mock bullmq to capture the processor functions passed to the Worker constructor
@@ -136,7 +148,8 @@ describe('Queue Workers', () => {
await emailProcessor(job);
expect(mocks.sendEmail).toHaveBeenCalledTimes(1);
- expect(mocks.sendEmail).toHaveBeenCalledWith(jobData);
+ // The implementation passes the logger as the second argument
+ expect(mocks.sendEmail).toHaveBeenCalledWith(jobData, expect.anything());
});
it('should re-throw an error if sendEmail fails', async () => {
diff --git a/src/services/userService.test.ts b/src/services/userService.test.ts
index 71e950a3..04732096 100644
--- a/src/services/userService.test.ts
+++ b/src/services/userService.test.ts
@@ -16,10 +16,10 @@ const mocks = vi.hoisted(() => {
return callback({});
}),
// Mock the repository classes.
- MockAddressRepository: vi.fn(() => ({
+ MockAddressRepository: vi.fn().mockImplementation(() => ({
upsertAddress: mockUpsertAddress,
})),
- MockUserRepository: vi.fn(() => ({
+ MockUserRepository: vi.fn().mockImplementation(() => ({
updateUserProfile: mockUpdateUserProfile,
})),
// Expose the method mocks for assertions.
diff --git a/src/utils/checksum.test.ts b/src/utils/checksum.test.ts
index 79e1539e..a4194cc5 100644
--- a/src/utils/checksum.test.ts
+++ b/src/utils/checksum.test.ts
@@ -62,10 +62,10 @@ describe('generateFileChecksum', () => {
it('should use FileReader fallback if file.arrayBuffer is not a function', async () => {
const fileContent = 'fallback test';
- // FIX: Wrap the content in a Blob. JSDOM's FileReader has issues reading
- // a raw array of strings (`[fileContent]`) and produces an incorrect buffer.
- // Using a Blob ensures the content is read correctly by the fallback mechanism.
- const file = new File([new Blob([fileContent])], 'test.txt', { type: 'text/plain' });
+ // FIX: Use TextEncoder to create a Uint8Array. Passing a Blob to the File constructor
+ // in some JSDOM versions can result in the string "[object Blob]" being read instead of content.
+ // Uint8Array is handled reliably by the File constructor and FileReader.
+ const file = new File([new TextEncoder().encode(fileContent)], 'test.txt', { type: 'text/plain' });
// Simulate an environment where file.arrayBuffer does not exist to force the fallback.
Object.defineProperty(file, 'arrayBuffer', { value: undefined });
@@ -77,9 +77,8 @@ describe('generateFileChecksum', () => {
it('should use FileReader fallback if file.arrayBuffer throws an error', async () => {
const fileContent = 'error fallback';
- // FIX: Wrap the content in a Blob for the same reason as the test above.
- // This ensures the FileReader fallback produces the correct checksum.
- const file = new File([new Blob([fileContent])], 'test.txt', { type: 'text/plain' });
+ // Use TextEncoder to create a Uint8Array for reliable file content creation in JSDOM.
+ const file = new File([new TextEncoder().encode(fileContent)], 'test.txt', { type: 'text/plain' });
// Mock the function to throw an error
vi.spyOn(file, 'arrayBuffer').mockRejectedValue(new Error('Simulated error'));
diff --git a/src/utils/pdfConverter.test.ts b/src/utils/pdfConverter.test.ts
index 72ad71f9..17139010 100644
--- a/src/utils/pdfConverter.test.ts
+++ b/src/utils/pdfConverter.test.ts
@@ -69,6 +69,25 @@ describe('pdfConverter', () => {
beforeEach(() => {
// Clear all mock history before each test
vi.clearAllMocks();
+
+ // Reset shared state objects to their default happy-path values
+ mockPdfDocument.numPages = 3;
+
+ // Reset mock implementations to defaults to clear any leftover 'Once' mocks
+ // that might have leaked from failed tests.
+ mockGetContext.mockReset();
+ mockGetContext.mockImplementation(() => ({} as CanvasRenderingContext2D));
+
+ mockToBlob.mockReset();
+ mockToBlob.mockImplementation((callback) => {
+ const blob = new Blob(['mock-jpeg-content'], { type: 'image/jpeg' });
+ callback(blob);
+ });
+
+ // Ensure getPage always returns the mock page by default
+ // (clears any mockRejectedValueOnce from previous tests)
+ mockPdfDocument.getPage.mockReset();
+ mockPdfDocument.getPage.mockResolvedValue(mockPdfPage);
});
afterEach(() => {