feat: Enhance logging and type safety across various components and services
Some checks failed
Deploy to Test Environment / deploy-to-test (push) Has been cancelled

This commit is contained in:
2025-12-13 23:12:50 -08:00
parent 77454b04c2
commit 7615d7746e
16 changed files with 96 additions and 61 deletions

View File

@@ -80,6 +80,21 @@ const getDurationInMilliseconds = (start: [number, number]): number => {
return (diff[0] * NS_PER_SEC + diff[1]) / NS_TO_MS; return (diff[0] * NS_PER_SEC + diff[1]) / NS_TO_MS;
}; };
/**
* Defines the structure for the detailed log object created for each request.
* This ensures type safety and consistency in our structured logs.
*/
interface RequestLogDetails {
user_id?: string;
method: string;
originalUrl: string;
statusCode: number;
statusMessage: string;
duration: string;
// The 'req' property is added conditionally for client/server errors.
req?: { headers: express.Request['headers']; body: express.Request['body'] };
}
const requestLogger = (req: Request, res: Response, next: NextFunction) => { const requestLogger = (req: Request, res: Response, next: NextFunction) => {
const requestId = randomUUID(); const requestId = randomUUID();
const user = req.user as UserProfile | undefined; const user = req.user as UserProfile | undefined;
@@ -102,7 +117,7 @@ const requestLogger = (req: Request, res: Response, next: NextFunction) => {
const finalUser = req.user as UserProfile | undefined; const finalUser = req.user as UserProfile | undefined;
// The base log object includes details relevant for all status codes. // The base log object includes details relevant for all status codes.
const logDetails: Record<string, any> = { const logDetails: RequestLogDetails = {
user_id: finalUser?.user_id, user_id: finalUser?.user_id,
method, method,
originalUrl, originalUrl,

View File

@@ -37,8 +37,6 @@ function App() {
const location = useLocation(); const location = useLocation();
const params = useParams<{ flyerId?: string }>(); const params = useParams<{ flyerId?: string }>();
const [error, setError] = useState<string | null>(null);
const [isDarkMode, setIsDarkMode] = useState(false); const [isDarkMode, setIsDarkMode] = useState(false);
// Fetch items for the currently selected flyer // Fetch items for the currently selected flyer
@@ -110,8 +108,9 @@ function App() {
// The useData hook will automatically refetch user data when `user` changes. // The useData hook will automatically refetch user data when `user` changes.
// We can remove the explicit fetch here. // We can remove the explicit fetch here.
} catch (e) { } catch (e) {
const errorMessage = e instanceof Error ? e.message : String(e); // The `login` function within the `useAuth` hook already handles its own errors
setError(errorMessage); // and notifications, so we just need to log any unexpected failures here.
logger.error({ err: e }, 'An error occurred during the login success handling.');
} }
}; };
@@ -150,7 +149,6 @@ function App() {
const handleFlyerSelect = useCallback(async (flyer: Flyer) => { const handleFlyerSelect = useCallback(async (flyer: Flyer) => {
setSelectedFlyer(flyer); setSelectedFlyer(flyer);
setError(null);
}, []); }, []);
useEffect(() => { useEffect(() => {

View File

@@ -1,6 +1,6 @@
// src/features/charts/PriceChart.tsx // src/features/charts/PriceChart.tsx
import React from 'react'; import React from 'react';
import type { DealItem, User } from '../../types'; import type { User } from '../../types';
import { useActiveDeals } from '../../hooks/useActiveDeals'; import { useActiveDeals } from '../../hooks/useActiveDeals';
import { TagIcon } from '../../components/icons/TagIcon'; import { TagIcon } from '../../components/icons/TagIcon';
import { LoadingSpinner } from '../../components/LoadingSpinner'; import { LoadingSpinner } from '../../components/LoadingSpinner';

View File

@@ -3,7 +3,6 @@ import React, { useState, useEffect, useMemo } from 'react';
import { LineChart, Line, XAxis, YAxis, CartesianGrid, Tooltip, Legend, ResponsiveContainer } from 'recharts'; import { LineChart, Line, XAxis, YAxis, CartesianGrid, Tooltip, Legend, ResponsiveContainer } from 'recharts';
import * as apiClient from '../../services/apiClient'; import * as apiClient from '../../services/apiClient';
import { LoadingSpinner } from '../../components/LoadingSpinner'; // This path is correct import { LoadingSpinner } from '../../components/LoadingSpinner'; // This path is correct
import type { MasterGroceryItem } from '../../types';
import { useUserData } from '../../hooks/useUserData'; import { useUserData } from '../../hooks/useUserData';
interface HistoricalPriceDataPoint { interface HistoricalPriceDataPoint {

View File

@@ -1,6 +1,6 @@
// src/features/flyer/AnalysisPanel.tsx // src/features/flyer/AnalysisPanel.tsx
import React, { useState } from 'react'; import React, { useState } from 'react';
import { AnalysisType, Flyer, MasterGroceryItem } from '../../types'; import { AnalysisType, Flyer } from '../../types';
import { LoadingSpinner } from '../../components/LoadingSpinner'; import { LoadingSpinner } from '../../components/LoadingSpinner';
import { LightbulbIcon } from '../../components/icons/LightbulbIcon'; import { LightbulbIcon } from '../../components/icons/LightbulbIcon';
import { BrainIcon } from '../../components/icons/BrainIcon'; import { BrainIcon } from '../../components/icons/BrainIcon';
@@ -16,6 +16,15 @@ interface AnalysisPanelProps {
selectedFlyer: Flyer | null; selectedFlyer: Flyer | null;
} }
/**
* Defines the types of analysis that correspond to a visible tab in the panel.
* This is a subset of the main `AnalysisType` enum and is used to ensure
* that the `activeTab` state can only be one of these values, preventing
* type errors when indexing into results or loading state objects.
*/
type AnalysisTabType = Exclude<AnalysisType, AnalysisType.GENERATE_IMAGE>;
interface Source { interface Source {
uri: string; uri: string;
title: string; title: string;
@@ -47,7 +56,7 @@ const TabButton: React.FC<TabButtonProps> = ({ label, icon, isActive, onClick })
export const AnalysisPanel: React.FC<AnalysisPanelProps> = ({ selectedFlyer }) => { export const AnalysisPanel: React.FC<AnalysisPanelProps> = ({ selectedFlyer }) => {
const { flyerItems, isLoading: isLoadingItems, error: itemsError } = useFlyerItems(selectedFlyer); const { flyerItems, isLoading: isLoadingItems, error: itemsError } = useFlyerItems(selectedFlyer);
const { watchedItems, isLoading: isLoadingWatchedItems } = useUserData(); const { watchedItems, isLoading: isLoadingWatchedItems } = useUserData();
const [activeTab, setActiveTab] = useState<AnalysisType>(AnalysisType.QUICK_INSIGHTS); const [activeTab, setActiveTab] = useState<AnalysisTabType>(AnalysisType.QUICK_INSIGHTS);
const { const {
results, results,

View File

@@ -1,6 +1,6 @@
// src/features/flyer/ExtractedDataTable.tsx // src/features/flyer/ExtractedDataTable.tsx
import React, { useMemo, useState } from 'react'; import React, { useMemo, useState } from 'react';
import type { FlyerItem, ShoppingListItem } from '../../types'; // Import ShoppingListItem import type { FlyerItem, MasterGroceryItem, ShoppingList, ShoppingListItem, User } from '../../types'; // Import ShoppingListItem
import { formatUnitPrice } from '../../utils/unitConverter'; import { formatUnitPrice } from '../../utils/unitConverter';
import { PlusCircleIcon } from '../../components/icons/PlusCircleIcon'; import { PlusCircleIcon } from '../../components/icons/PlusCircleIcon';
import { useAuth } from '../../hooks/useAuth'; import { useAuth } from '../../hooks/useAuth';
@@ -9,10 +9,17 @@ import { useMasterItems } from '../../hooks/useMasterItems';
import { useWatchedItems } from '../../hooks/useWatchedItems'; import { useWatchedItems } from '../../hooks/useWatchedItems';
import { useShoppingLists } from '../../hooks/useShoppingLists'; import { useShoppingLists } from '../../hooks/useShoppingLists';
interface ExtractedDataTableProps { export interface ExtractedDataTableProps {
items: FlyerItem[]; items: FlyerItem[];
totalActiveItems?: number; totalActiveItems?: number;
watchedItems: MasterGroceryItem[];
masterItems: MasterGroceryItem[];
unitSystem: 'metric' | 'imperial'; unitSystem: 'metric' | 'imperial';
user: User | null;
onAddItem: (itemName: string, category: string) => Promise<void>;
shoppingLists: ShoppingList[];
activeListId: number | null;
onAddItemToList: (masterItemId: number) => void;
} }
export const ExtractedDataTable: React.FC<ExtractedDataTableProps> = ({ items, totalActiveItems, unitSystem }) => { export const ExtractedDataTable: React.FC<ExtractedDataTableProps> = ({ items, totalActiveItems, unitSystem }) => {

View File

@@ -16,7 +16,7 @@ const formatDateRange = (from: string | null | undefined, to: string | null | un
return fromDate ? `Deals start ${fromDate}` : (toDate ? `Deals end ${toDate}` : null); return fromDate ? `Deals start ${fromDate}` : (toDate ? `Deals end ${toDate}` : null);
}; };
interface FlyerDisplayProps { export interface FlyerDisplayProps {
imageUrl: string | null; imageUrl: string | null;
store?: Store; store?: Store;
validFrom?: string | null; validFrom?: string | null;

View File

@@ -1,8 +1,8 @@
// src/features/flyer/FlyerUploader.tsx // src/features/flyer/FlyerUploader.tsx
import React, { useState, useEffect, useRef, useCallback, Fragment } from 'react'; import React, { useState, useEffect, useRef, useCallback } from 'react';
import { useNavigate, Link } from 'react-router-dom'; import { useNavigate, Link } from 'react-router-dom';
import { uploadAndProcessFlyer, getJobStatus } from '../../services/aiApiClient'; import { uploadAndProcessFlyer, getJobStatus } from '../../services/aiApiClient';
import { generateFileChecksum } from '../../utils/checksum'; // Assuming you have this utility import { generateFileChecksum } from '../../utils/checksum';
import { logger } from '../../services/logger.client'; import { logger } from '../../services/logger.client';
import { ProcessingStatus } from './ProcessingStatus'; import { ProcessingStatus } from './ProcessingStatus';
import type { ProcessingStage } from '../../types'; import type { ProcessingStage } from '../../types';

View File

@@ -12,7 +12,7 @@ describe('useInfiniteQuery Hook', () => {
}); });
// Helper to create a mock paginated response // Helper to create a mock paginated response
const createMockResponse = <T>(items: T[], nextCursor: any): Response => { const createMockResponse = <T>(items: T[], nextCursor: number | string | null): Response => {
const paginatedResponse: PaginatedResponse<T> = { items, nextCursor }; const paginatedResponse: PaginatedResponse<T> = { items, nextCursor };
return new Response(JSON.stringify(paginatedResponse)); return new Response(JSON.stringify(paginatedResponse));
}; };

View File

@@ -5,7 +5,6 @@ import { useUserData } from './useUserData';
import { useApi } from './useApi'; import { useApi } from './useApi';
import * as apiClient from '../services/apiClient'; import * as apiClient from '../services/apiClient';
import type { ShoppingList, ShoppingListItem } from '../types'; import type { ShoppingList, ShoppingListItem } from '../types';
import { logger } from '../services/logger.client';
/** /**
* A custom hook to manage all state and logic related to shopping lists. * A custom hook to manage all state and logic related to shopping lists.

View File

@@ -5,7 +5,6 @@ import { useApi } from './useApi';
import { useUserData } from './useUserData'; import { useUserData } from './useUserData';
import * as apiClient from '../services/apiClient'; import * as apiClient from '../services/apiClient';
import type { MasterGroceryItem } from '../types'; import type { MasterGroceryItem } from '../types';
import { logger } from '../services/logger.client';
/** /**
* A custom hook to manage all state and logic related to a user's watched items. * A custom hook to manage all state and logic related to a user's watched items.

View File

@@ -5,17 +5,13 @@ import { describe, it, expect, vi, beforeEach } from 'vitest';
import { MemoryRouter, useOutletContext } from 'react-router-dom'; import { MemoryRouter, useOutletContext } from 'react-router-dom';
import { HomePage } from './HomePage'; import { HomePage } from './HomePage';
import { createMockFlyer, createMockFlyerItem } from '../tests/utils/mockFactories'; import { createMockFlyer, createMockFlyerItem } from '../tests/utils/mockFactories';
import type { Flyer, FlyerItem } from '../types'; import type { Flyer, FlyerItem, MasterGroceryItem, ShoppingList } from '../types';
import { ExtractedDataTable } from '../features/flyer/ExtractedDataTable'; import type { FlyerDisplayProps } from '../features/flyer/FlyerDisplay'; // Keep this for FlyerDisplay mock
import type { ExtractedDataTableProps } from '../features/flyer/ExtractedDataTable'; // Import the props for ExtractedDataTable
// Mock child components to isolate the HomePage logic // Mock child components to isolate the HomePage logic
vi.mock('../features/flyer/FlyerDisplay', () => ({ vi.mock('../features/flyer/FlyerDisplay', () => ({
FlyerDisplay: (props: any) => <div data-testid="flyer-display" data-image-url={props.imageUrl} />, FlyerDisplay: (props: FlyerDisplayProps) => <div data-testid="flyer-display" data-image-url={props.imageUrl} />,
}));
vi.mock('../features/flyer/ExtractedDataTable', () => ({
// 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', () => ({ vi.mock('../features/flyer/AnalysisPanel', () => ({
AnalysisPanel: () => <div data-testid="analysis-panel" />, AnalysisPanel: () => <div data-testid="analysis-panel" />,
@@ -30,6 +26,13 @@ vi.mock('react-router-dom', async (importOriginal) => {
}; };
}); });
// Mock ExtractedDataTable separately to use the imported props interface
import { ExtractedDataTable } from '../features/flyer/ExtractedDataTable';
vi.mock('../features/flyer/ExtractedDataTable', () => ({
// Wrap the mock component in vi.fn() to allow spying on its calls.
ExtractedDataTable: vi.fn((props: ExtractedDataTableProps) => <div data-testid="extracted-data-table">{props.items.length} items</div>),
}));
const mockedUseOutletContext = vi.mocked(useOutletContext); const mockedUseOutletContext = vi.mocked(useOutletContext);
describe('HomePage Component', () => { describe('HomePage Component', () => {

View File

@@ -1,5 +1,5 @@
// src/pages/admin/components/AdminBrandManager.tsx // src/pages/admin/components/AdminBrandManager.tsx
import React, { useState, useEffect, useCallback } from 'react'; import React, { useState, useEffect } from 'react';
import toast from 'react-hot-toast'; import toast from 'react-hot-toast';
import { fetchAllBrands, uploadBrandLogo } from '../../../services/apiClient'; import { fetchAllBrands, uploadBrandLogo } from '../../../services/apiClient';
import { Brand } from '../../../types'; import { Brand } from '../../../types';
@@ -8,8 +8,12 @@ import { useApiOnMount } from '../../../hooks/useApiOnMount';
export const AdminBrandManager: React.FC = () => { export const AdminBrandManager: React.FC = () => {
// Use the new hook to fetch brands when the component mounts. // Use the new hook to fetch brands when the component mounts.
// It handles loading and error states for us. // It handles loading and error states for us. We create a wrapper function
const { data: initialBrands, loading, error } = useApiOnMount<Brand[], []>(fetchAllBrands, []); // to ensure the AbortSignal from the hook is correctly passed to the API client.
// The underlying `fetchAllBrands` function takes no arguments, so we create a wrapper
// that accepts the signal from the hook but doesn't pass it down.
const fetchBrandsWrapper = () => fetchAllBrands();
const { data: initialBrands, loading, error } = useApiOnMount<Brand[], []>(fetchBrandsWrapper, []);
// We still need local state for brands so we can update it after a logo upload // We still need local state for brands so we can update it after a logo upload
// without needing to re-fetch the entire list. // without needing to re-fetch the entire list.

View File

@@ -4,21 +4,19 @@ import passport from './passport.routes';
import { isAdmin } from './passport.routes'; // Correctly imported import { isAdmin } from './passport.routes'; // Correctly imported
import multer from 'multer';// --- Zod Schemas for Admin Routes (as per ADR-003) --- import multer from 'multer';// --- Zod Schemas for Admin Routes (as per ADR-003) ---
import { z } from 'zod'; import { z } from 'zod';
import crypto from 'crypto';
import * as db from '../services/db/index.db'; import * as db from '../services/db/index.db';
import { logger } from '../services/logger.server'; import { logger } from '../services/logger.server';
import { UserProfile } from '../types'; import { UserProfile } from '../types';
import { clearGeocodeCache } from '../services/geocodingService.server'; import { geocodingService } from '../services/geocodingService.server';
import { requireFileUpload } from '../middleware/fileUpload.middleware'; // This was a duplicate, fixed. import { requireFileUpload } from '../middleware/fileUpload.middleware'; // This was a duplicate, fixed.
import { ForeignKeyConstraintError, NotFoundError, ValidationError } from '../services/db/errors.db'; import { NotFoundError, ValidationError } from '../services/db/errors.db';
import { validateRequest } from '../middleware/validation.middleware'; import { validateRequest } from '../middleware/validation.middleware';
// --- Bull Board (Job Queue UI) Imports --- // --- Bull Board (Job Queue UI) Imports ---
import { createBullBoard } from '@bull-board/api'; import { createBullBoard } from '@bull-board/api';
import { BullMQAdapter } from '@bull-board/api/bullMQAdapter'; import { BullMQAdapter } from '@bull-board/api/bullMQAdapter';
import { ExpressAdapter } from '@bull-board/express'; import { ExpressAdapter } from '@bull-board/express';
import { render, screen } from '@testing-library/react';
import type { Queue } from 'bullmq'; import type { Queue } from 'bullmq';
import { backgroundJobService } from '../services/backgroundJobService'; import { backgroundJobService } from '../services/backgroundJobService';
@@ -108,7 +106,7 @@ router.use(passport.authenticate('jwt', { session: false }), isAdmin);
router.get('/corrections', async (req, res, next: NextFunction) => { router.get('/corrections', async (req, res, next: NextFunction) => {
try { try {
const corrections = await db.adminRepo.getSuggestedCorrections(); const corrections = await db.adminRepo.getSuggestedCorrections(req.log);
res.json(corrections); res.json(corrections);
} catch (error) { } catch (error) {
next(error); next(error);
@@ -117,7 +115,7 @@ router.get('/corrections', async (req, res, next: NextFunction) => {
router.get('/brands', async (req, res, next: NextFunction) => { router.get('/brands', async (req, res, next: NextFunction) => {
try { try {
const brands = await db.flyerRepo.getAllBrands(); const brands = await db.flyerRepo.getAllBrands(req.log);
res.json(brands); res.json(brands);
} catch (error) { } catch (error) {
next(error); next(error);
@@ -126,7 +124,7 @@ router.get('/brands', async (req, res, next: NextFunction) => {
router.get('/stats', async (req, res, next: NextFunction) => { router.get('/stats', async (req, res, next: NextFunction) => {
try { try {
const stats = await db.adminRepo.getApplicationStats(); const stats = await db.adminRepo.getApplicationStats(req.log);
res.json(stats); res.json(stats);
} catch (error) { } catch (error) {
next(error); next(error);
@@ -135,7 +133,7 @@ router.get('/stats', async (req, res, next: NextFunction) => {
router.get('/stats/daily', async (req, res, next: NextFunction) => { router.get('/stats/daily', async (req, res, next: NextFunction) => {
try { try {
const dailyStats = await db.adminRepo.getDailyStatsForLast30Days(); const dailyStats = await db.adminRepo.getDailyStatsForLast30Days(req.log);
res.json(dailyStats); res.json(dailyStats);
} catch (error) { } catch (error) {
next(error); next(error);
@@ -145,7 +143,7 @@ router.get('/stats/daily', async (req, res, next: NextFunction) => {
router.post('/corrections/:id/approve', validateRequest(numericIdParamSchema('id')), async (req, res, next: NextFunction) => { router.post('/corrections/:id/approve', validateRequest(numericIdParamSchema('id')), async (req, res, next: NextFunction) => {
try { try {
const correctionId = req.params.id as unknown as number; const correctionId = req.params.id as unknown as number;
await db.adminRepo.approveCorrection(correctionId); await db.adminRepo.approveCorrection(correctionId, req.log);
res.status(200).json({ message: 'Correction approved successfully.' }); res.status(200).json({ message: 'Correction approved successfully.' });
} catch (error) { } catch (error) {
next(error); next(error);
@@ -155,7 +153,7 @@ router.post('/corrections/:id/approve', validateRequest(numericIdParamSchema('id
router.post('/corrections/:id/reject', validateRequest(numericIdParamSchema('id')), async (req, res, next: NextFunction) => { router.post('/corrections/:id/reject', validateRequest(numericIdParamSchema('id')), async (req, res, next: NextFunction) => {
try { try {
const correctionId = req.params.id as unknown as number; const correctionId = req.params.id as unknown as number;
await db.adminRepo.rejectCorrection(correctionId); await db.adminRepo.rejectCorrection(correctionId, req.log);
res.status(200).json({ message: 'Correction rejected successfully.' }); res.status(200).json({ message: 'Correction rejected successfully.' });
} catch (error) { } catch (error) {
next(error); next(error);
@@ -166,7 +164,7 @@ router.put('/corrections/:id', validateRequest(updateCorrectionSchema), async (r
const correctionId = req.params.id as unknown as number; const correctionId = req.params.id as unknown as number;
const { suggested_value } = req.body; const { suggested_value } = req.body;
try { try {
const updatedCorrection = await db.adminRepo.updateSuggestedCorrection(correctionId, suggested_value); const updatedCorrection = await db.adminRepo.updateSuggestedCorrection(correctionId, suggested_value, req.log);
res.status(200).json(updatedCorrection); res.status(200).json(updatedCorrection);
} catch (error) { } catch (error) {
next(error); next(error);
@@ -177,7 +175,7 @@ router.put('/recipes/:id/status', validateRequest(updateRecipeStatusSchema), asy
const recipeId = req.params.id as unknown as number; const recipeId = req.params.id as unknown as number;
const { status } = req.body; const { status } = req.body;
try { try {
const updatedRecipe = await db.adminRepo.updateRecipeStatus(recipeId, status); // This is still a standalone function in admin.db.ts const updatedRecipe = await db.adminRepo.updateRecipeStatus(recipeId, status, req.log); // This is still a standalone function in admin.db.ts
res.status(200).json(updatedRecipe); res.status(200).json(updatedRecipe);
} catch (error) { } catch (error) {
next(error); // Pass all errors to the central error handler next(error); // Pass all errors to the central error handler
@@ -193,9 +191,9 @@ router.post('/brands/:id/logo', validateRequest(numericIdParamSchema('id')), upl
throw new ValidationError([], 'Logo image file is missing.'); throw new ValidationError([], 'Logo image file is missing.');
} }
const logoUrl = `/assets/${req.file.filename}`; const logoUrl = `/assets/${req.file.filename}`;
await db.adminRepo.updateBrandLogo(brandId, logoUrl); await db.adminRepo.updateBrandLogo(brandId, logoUrl, req.log);
logger.info(`Brand logo updated for brand ID: ${brandId}`, { brandId, logoUrl }); logger.info({ brandId, logoUrl }, `Brand logo updated for brand ID: ${brandId}`);
res.status(200).json({ message: 'Brand logo updated successfully.', logoUrl }); res.status(200).json({ message: 'Brand logo updated successfully.', logoUrl });
} catch (error) { } catch (error) {
next(error); next(error);
@@ -204,7 +202,7 @@ router.post('/brands/:id/logo', validateRequest(numericIdParamSchema('id')), upl
router.get('/unmatched-items', async (req, res, next: NextFunction) => { router.get('/unmatched-items', async (req, res, next: NextFunction) => {
try { try {
const items = await db.adminRepo.getUnmatchedFlyerItems(); const items = await db.adminRepo.getUnmatchedFlyerItems(req.log);
res.json(items); res.json(items);
} catch (error) { } catch (error) {
next(error); next(error);
@@ -220,7 +218,7 @@ router.delete('/recipes/:recipeId', validateRequest(numericIdParamSchema('recipe
try { try {
// The isAdmin flag bypasses the ownership check in the repository method. // The isAdmin flag bypasses the ownership check in the repository method.
await db.recipeRepo.deleteRecipe(recipeId, adminUser.user_id, true); await db.recipeRepo.deleteRecipe(recipeId, adminUser.user_id, true, req.log);
res.status(204).send(); res.status(204).send();
} catch (error: unknown) { } catch (error: unknown) {
next(error); next(error);
@@ -233,7 +231,7 @@ router.delete('/recipes/:recipeId', validateRequest(numericIdParamSchema('recipe
router.delete('/flyers/:flyerId', validateRequest(numericIdParamSchema('flyerId')), async (req, res, next: NextFunction) => { router.delete('/flyers/:flyerId', validateRequest(numericIdParamSchema('flyerId')), async (req, res, next: NextFunction) => {
const flyerId = req.params.flyerId as unknown as number; const flyerId = req.params.flyerId as unknown as number;
try { try {
await db.flyerRepo.deleteFlyer(flyerId); await db.flyerRepo.deleteFlyer(flyerId, req.log);
res.status(204).send(); res.status(204).send();
} catch (error: unknown) { } catch (error: unknown) {
next(error); next(error);
@@ -244,7 +242,7 @@ router.put('/comments/:id/status', validateRequest(updateCommentStatusSchema), a
const commentId = req.params.id as unknown as number; const commentId = req.params.id as unknown as number;
const { status } = req.body; const { status } = req.body;
try { try {
const updatedComment = await db.adminRepo.updateRecipeCommentStatus(commentId, status); // This is still a standalone function in admin.db.ts const updatedComment = await db.adminRepo.updateRecipeCommentStatus(commentId, status, req.log); // This is still a standalone function in admin.db.ts
res.status(200).json(updatedComment); res.status(200).json(updatedComment);
} catch (error: unknown) { } catch (error: unknown) {
next(error); next(error);
@@ -253,7 +251,7 @@ router.put('/comments/:id/status', validateRequest(updateCommentStatusSchema), a
router.get('/users', async (req, res, next: NextFunction) => { router.get('/users', async (req, res, next: NextFunction) => {
try { try {
const users = await db.adminRepo.getAllUsers(); const users = await db.adminRepo.getAllUsers(req.log);
res.json(users); res.json(users);
} catch (error) { } catch (error) {
next(error); next(error);
@@ -263,7 +261,7 @@ router.get('/users', async (req, res, next: NextFunction) => {
router.get('/activity-log', validateRequest(activityLogSchema), async (req, res, next: NextFunction) => { router.get('/activity-log', validateRequest(activityLogSchema), async (req, res, next: NextFunction) => {
const { limit, offset } = req.query as unknown as { limit: number; offset: number }; const { limit, offset } = req.query as unknown as { limit: number; offset: number };
try { try {
const logs = await db.adminRepo.getActivityLog(limit, offset); const logs = await db.adminRepo.getActivityLog(limit, offset, req.log);
res.json(logs); res.json(logs);
} catch (error) { } catch (error) {
next(error); next(error);
@@ -272,7 +270,7 @@ router.get('/activity-log', validateRequest(activityLogSchema), async (req, res,
router.get('/users/:id', validateRequest(z.object({ params: z.object({ id: z.string().uuid() }) })), async (req, res, next: NextFunction) => { router.get('/users/:id', validateRequest(z.object({ params: z.object({ id: z.string().uuid() }) })), async (req, res, next: NextFunction) => {
try { try {
const user = await db.userRepo.findUserProfileById(req.params.id); const user = await db.userRepo.findUserProfileById(req.params.id, req.log);
res.json(user); res.json(user);
} catch (error) { } catch (error) {
next(error); next(error);
@@ -283,10 +281,10 @@ router.put('/users/:id', validateRequest(updateUserRoleSchema), async (req, res,
const { role } = req.body; const { role } = req.body;
try { try {
const updatedUser = await db.adminRepo.updateUserRole(req.params.id, role); const updatedUser = await db.adminRepo.updateUserRole(req.params.id, role, req.log);
res.json(updatedUser); res.json(updatedUser);
} catch (error) { } catch (error) {
logger.error(`Error updating user ${req.params.id}:`, { error }); logger.error({ error }, `Error updating user ${req.params.id}:`);
next(error); next(error);
} }
}); });
@@ -297,7 +295,7 @@ router.delete('/users/:id', validateRequest(z.object({ params: z.object({ id: z.
if (adminUser.user.user_id === req.params.id) { if (adminUser.user.user_id === req.params.id) {
throw new ValidationError([], 'Admins cannot delete their own account.'); throw new ValidationError([], 'Admins cannot delete their own account.');
} }
await db.userRepo.deleteUserById(req.params.id); await db.userRepo.deleteUserById(req.params.id, req.log);
res.status(204).send(); res.status(204).send();
} catch (error) { } catch (error) {
next(error); next(error);
@@ -318,7 +316,7 @@ router.post('/trigger/daily-deal-check', async (req, res, next: NextFunction) =>
backgroundJobService.runDailyDealCheck(); backgroundJobService.runDailyDealCheck();
res.status(202).json({ message: 'Daily deal check job has been triggered successfully. It will run in the background.' }); res.status(202).json({ message: 'Daily deal check job has been triggered successfully. It will run in the background.' });
} catch (error) { } catch (error) {
logger.error('[Admin] Failed to trigger daily deal check job.', { error }); logger.error({ error }, '[Admin] Failed to trigger daily deal check job.');
next(error); next(error);
} }
}); });
@@ -340,7 +338,7 @@ router.post('/trigger/analytics-report', async (req, res, next: NextFunction) =>
res.status(202).json({ message: `Analytics report generation job has been enqueued successfully. Job ID: ${job.id}` }); res.status(202).json({ message: `Analytics report generation job has been enqueued successfully. Job ID: ${job.id}` });
} catch (error) { } catch (error) {
logger.error('[Admin] Failed to enqueue analytics report job.', { error }); logger.error({ error }, '[Admin] Failed to enqueue analytics report job.');
next(error); next(error);
} }
}); });
@@ -390,10 +388,10 @@ router.post('/system/clear-geocode-cache', async (req, res, next: NextFunction)
logger.info(`[Admin] Manual trigger for geocode cache clear received from user: ${adminUser.user_id}`); logger.info(`[Admin] Manual trigger for geocode cache clear received from user: ${adminUser.user_id}`);
try { try {
const keysDeleted = await clearGeocodeCache(); const keysDeleted = await geocodingService.clearGeocodeCache(req.log);
res.status(200).json({ message: `Successfully cleared the geocode cache. ${keysDeleted} keys were removed.` }); res.status(200).json({ message: `Successfully cleared the geocode cache. ${keysDeleted} keys were removed.` });
} catch (error) { } catch (error) {
logger.error('[Admin] Failed to clear geocode cache.', { error }); logger.error({ error }, '[Admin] Failed to clear geocode cache.');
next(error); next(error);
} }
}); });

View File

@@ -8,7 +8,6 @@
// --- END FIX REGISTRY --- // --- END FIX REGISTRY ---
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import { PoolClient } from 'pg'; import { PoolClient } from 'pg';
import { withTransaction } from './connection.db';
// Mock the logger to prevent stderr noise during tests // Mock the logger to prevent stderr noise during tests
vi.mock('../logger.server', () => ({ vi.mock('../logger.server', () => ({
@@ -24,11 +23,12 @@ import { logger as mockLogger } from '../logger.server';
// Un-mock the module we are testing to ensure we use the real implementation. // Un-mock the module we are testing to ensure we use the real implementation.
vi.unmock('./user.db'); vi.unmock('./user.db');
// Mock the withTransaction helper since we are testing a function that uses it. // Mock the withTransaction helper. This is the key fix.
vi.mock('./connection.db', async (importOriginal) => { vi.mock('./connection.db', async (importOriginal) => {
const actual = await importOriginal<typeof import('./connection.db')>(); const actual = await importOriginal<typeof import('./connection.db')>();
return { ...actual, withTransaction: vi.fn() }; return { ...actual, withTransaction: vi.fn() };
}); });
import { withTransaction } from './connection.db';
import { UserRepository, exportUserData } from './user.db'; import { UserRepository, exportUserData } from './user.db';
import { mockPoolInstance } from '../../tests/setup/tests-setup-unit'; import { mockPoolInstance } from '../../tests/setup/tests-setup-unit';
@@ -61,8 +61,7 @@ describe('User DB Service', () => {
beforeEach(() => { beforeEach(() => {
vi.clearAllMocks(); vi.clearAllMocks();
userRepo = new UserRepository(mockPoolInstance as any); userRepo = new UserRepository(mockPoolInstance as any);
// Reset the withTransaction mock before each test // Provide a default mock implementation for withTransaction for all tests.
const { withTransaction } = require('./connection.db'); // eslint-disable-line @typescript-eslint/no-var-requires
vi.mocked(withTransaction).mockImplementation(async (callback: (client: PoolClient) => Promise<any>) => callback(mockPoolInstance as any)); vi.mocked(withTransaction).mockImplementation(async (callback: (client: PoolClient) => Promise<any>) => callback(mockPoolInstance as any));
}); });

View File

@@ -104,6 +104,11 @@ describe('Queue Service Setup and Lifecycle', () => {
cleanupWorker = queueService.cleanupWorker; cleanupWorker = queueService.cleanupWorker;
}); });
afterEach(() => {
// Clean up all event listeners on the mock connection to prevent open handles.
mocks.mockRedisConnection.removeAllListeners();
});
it('should log a success message when Redis connects', () => { it('should log a success message when Redis connects', () => {
// Act: Simulate the 'connect' event on the mock Redis connection // Act: Simulate the 'connect' event on the mock Redis connection
mocks.mockRedisConnection.emit('connect'); mocks.mockRedisConnection.emit('connect');