diff --git a/docs/adr/0003-standardized-input-validation-using-middleware.md b/docs/adr/0003-standardized-input-validation-using-middleware.md index d22f549d..7da09d14 100644 --- a/docs/adr/0003-standardized-input-validation-using-middleware.md +++ b/docs/adr/0003-standardized-input-validation-using-middleware.md @@ -25,7 +25,9 @@ We will adopt a schema-based approach for input validation using the `zod` libra 3. **Refactor Routes**: All route handlers will be refactored to use this new middleware, removing all manual validation logic. -### Example Usage +4. **(New) Resilient Type Inference**: To achieve full type safety, we will use **inline type assertions** with `z.infer`. This ensures the code inside the handler is fully typed and benefits from IntelliSense without creating complex utility types that conflict with Express's `RequestHandler` signature. + +### Example Usage (Refined Pattern) ```typescript // In flyer.routes.ts @@ -33,31 +35,37 @@ We will adopt a schema-based approach for input validation using the `zod` libra import { z } from 'zod'; import { validateRequest } from '../middleware/validation'; -// Define the schema for the GET /:id route +// 1. Define the schema const getFlyerSchema = z.object({ params: z.object({ id: z.string().pipe(z.coerce.number().int().positive()), }), }); -// Apply the middleware to the route -router.get('/:id', validateRequest(getFlyerSchema), async (req, res, next) => { +// 2. Infer the type from the schema for local use +type GetFlyerRequest = z.infer; + +// 3. Apply the middleware and use an inline cast for the request +router.get('/:id', validateRequest(getFlyerSchema), (async (req, res, next) => { + // Cast 'req' to the inferred type. + // This provides full type safety for params, query, and body. + const { params } = req as unknown as GetFlyerRequest; + try { - // req.params.id is now guaranteed to be a positive number - const flyerId = req.params.id; - const flyer = await db.flyerRepo.getFlyerById(flyerId); + const flyer = await db.flyerRepo.getFlyerById(params.id); // params.id is 'number' res.json(flyer); } catch (error) { next(error); } -}); +})); ``` ## Consequences ### Positive -**DRY and Declarative**: Validation logic is defined once in a schema and removed from route handlers. +**Reduced Complexity**: Avoids maintaining a complex ValidatedRequestHandler utility type that could conflict with Express or TypeScript upgrades. +**Explicit Contracts**: By defining the Request type right next to the route, the contract for that endpoint is immediately visible. **Improved Readability**: Route handlers become much cleaner and focus exclusively on their core business logic. **Type Safety**: `zod` schemas provide strong compile-time and runtime type safety, reducing bugs. **Consistent and Detailed Errors**: The `errorHandler` can be configured to provide consistent, detailed validation error messages for all routes (e.g., "Query parameter 'limit' must be a positive integer"). @@ -65,6 +73,7 @@ router.get('/:id', validateRequest(getFlyerSchema), async (req, res, next) => { ### Negative +**Minor Verbosity**: Requires one extra line (type ... = z.infer<...>) and a controlled cast (as unknown as ...) within the handler function. **New Dependency**: Introduces `zod` as a new project dependency. **Learning Curve**: Developers need to learn the `zod` schema definition syntax. **Refactoring Effort**: Requires a one-time effort to create schemas and refactor all existing routes to use the `validateRequest` middleware. diff --git a/src/features/flyer/AnalysisPanel.test.tsx b/src/features/flyer/AnalysisPanel.test.tsx index 97deb7d1..b29a2ec1 100644 --- a/src/features/flyer/AnalysisPanel.test.tsx +++ b/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 Mock } from 'vitest'; import { AnalysisPanel } from './AnalysisPanel'; import { useFlyerItems } from '../../hooks/useFlyerItems'; import type { Flyer, FlyerItem, Store, MasterGroceryItem } from '../../types'; diff --git a/src/hooks/useActiveDeals.tsx b/src/hooks/useActiveDeals.tsx index eec8b6c3..081a4fff 100644 --- a/src/hooks/useActiveDeals.tsx +++ b/src/hooks/useActiveDeals.tsx @@ -3,10 +3,14 @@ import { useEffect, useMemo } from 'react'; import { useFlyers } from './useFlyers'; import { useUserData } from './useUserData'; import { useApi } from './useApi'; -import type { FlyerItem } from '../types'; +import type { FlyerItem, DealItem } from '../types'; import * as apiClient from '../services/apiClient'; import { logger } from '../services/logger.client'; +interface FlyerItemCount { + count: number; +} + /** * A custom hook to calculate currently active deals and total active items * based on flyer validity dates and a user's watched items. @@ -16,8 +20,8 @@ export const useActiveDeals = () => { const { flyers } = useFlyers(); const { watchedItems } = useUserData(); // Centralize API call state management with the useApi hook. We can ignore isRefetching here. - const { execute: executeCount, loading: loadingCount, error: errorCount, data: countData } = useApi(apiClient.countFlyerItemsForFlyers as any); - const { execute: executeItems, loading: loadingItems, error: errorItems, data: itemsData } = useApi(apiClient.fetchFlyerItemsForFlyers as any); + const { execute: executeCount, loading: loadingCount, error: errorCount, data: countData } = useApi(apiClient.countFlyerItemsForFlyers); + const { execute: executeItems, loading: loadingItems, error: errorItems, data: itemsData } = useApi(apiClient.fetchFlyerItemsForFlyers); // Consolidate loading and error states from both API calls. const isLoading = loadingCount || loadingItems; @@ -53,12 +57,11 @@ export const useActiveDeals = () => { const activeDeals = useMemo(() => { if (!itemsData || watchedItems.length === 0) return []; - const allItems: FlyerItem[] = (itemsData as any).items || itemsData; // Adapt to potential response structure const watchedItemIds = new Set(watchedItems.map(item => item.master_grocery_item_id)); - const dealItemsRaw = allItems.filter(item => item.master_item_id && watchedItemIds.has(item.master_item_id)); + const dealItemsRaw = itemsData.filter(item => item.master_item_id && watchedItemIds.has(item.master_item_id)); const flyerIdToStoreName = new Map(validFlyers.map(f => [f.flyer_id, f.store?.name || 'Unknown Store'])); - return dealItemsRaw.map(item => ({ + return dealItemsRaw.map((item): DealItem => ({ item: item.item, price_display: item.price_display, price_in_cents: item.price_in_cents, @@ -69,6 +72,6 @@ export const useActiveDeals = () => { })); }, [itemsData, watchedItems, validFlyers]); - const totalActiveItems = (countData as any)?.count ?? 0; + const totalActiveItems = countData?.count ?? 0; return { activeDeals, totalActiveItems, isLoading, error }; }; \ No newline at end of file diff --git a/src/hooks/useInfiniteQuery.ts b/src/hooks/useInfiniteQuery.ts index 030e43d4..ee69b7dd 100644 --- a/src/hooks/useInfiniteQuery.ts +++ b/src/hooks/useInfiniteQuery.ts @@ -17,7 +17,7 @@ export interface PaginatedResponse { * The type for the API function passed to the hook. * It must accept a cursor/page parameter and return a `PaginatedResponse`. */ -type ApiFunction = (cursor?: any) => Promise; +type ApiFunction = (cursor?: number | string | null) => Promise; interface UseInfiniteQueryOptions { initialCursor?: number | string | null; @@ -33,7 +33,7 @@ interface UseInfiniteQueryOptions { * @returns An object with state and methods for managing the infinite query. */ export function useInfiniteQuery( - apiFunction: ApiFunction, + apiFunction: ApiFunction, options: UseInfiniteQueryOptions = {} ) { const { initialCursor = 0 } = options; @@ -46,9 +46,9 @@ export function useInfiniteQuery( const [hasNextPage, setHasNextPage] = useState(true); // Use a ref to store the cursor for the next page. - const nextCursorRef = useRef(initialCursor); + const nextCursorRef = useRef(initialCursor); - const fetchPage = useCallback(async (cursor?: any) => { + const fetchPage = useCallback(async (cursor?: number | string | null) => { // Determine which loading state to set const isInitialLoad = cursor === initialCursor && data.length === 0; if (isInitialLoad) { diff --git a/src/layouts/MainLayout.test.tsx b/src/layouts/MainLayout.test.tsx index e2ee87a4..eb94e658 100644 --- a/src/layouts/MainLayout.test.tsx +++ b/src/layouts/MainLayout.test.tsx @@ -127,12 +127,12 @@ describe('MainLayout Component', () => { vi.clearAllMocks(); // Provide default mock implementations for all hooks - mockedUseAuth.mockReturnValue(defaultUseAuthReturn as any); - mockedUseFlyers.mockReturnValue(defaultUseFlyersReturn as any); + mockedUseAuth.mockReturnValue(defaultUseAuthReturn); + mockedUseFlyers.mockReturnValue(defaultUseFlyersReturn); mockedUseMasterItems.mockReturnValue(defaultUseMasterItemsReturn); - mockedUseShoppingLists.mockReturnValue(defaultUseShoppingListsReturn as any); - mockedUseWatchedItems.mockReturnValue(defaultUseWatchedItemsReturn as any); - mockedUseActiveDeals.mockReturnValue(defaultUseActiveDealsReturn as any); + mockedUseShoppingLists.mockReturnValue(defaultUseShoppingListsReturn); + mockedUseWatchedItems.mockReturnValue(defaultUseWatchedItemsReturn); + mockedUseActiveDeals.mockReturnValue(defaultUseActiveDealsReturn); }); const defaultProps = { @@ -171,11 +171,11 @@ describe('MainLayout Component', () => { describe('for authenticated users', () => { beforeEach(() => { mockedUseAuth.mockReturnValue({ - ...mockedUseAuth(), + ...defaultUseAuthReturn, user: mockUser, authStatus: 'AUTHENTICATED', profile: { user: mockUser } as UserProfile, - } as any); + }); }); it('does not show the AnonymousUserBanner', () => { @@ -213,9 +213,9 @@ describe('MainLayout Component', () => { describe('Event Handlers', () => { it('calls setActiveListId when a list is shared via ActivityLog and the list exists', () => { mockedUseShoppingLists.mockReturnValueOnce({ - ...defaultUseShoppingListsReturn, // Spread the complete default object - shoppingLists: [{ shopping_list_id: 1, name: 'My List', items: [] } as any], - } as any); + ...defaultUseShoppingListsReturn, + shoppingLists: [{ shopping_list_id: 1, name: 'My List', user_id: 'user-123', created_at: '', items: [] }], + }); renderWithRouter(); const activityLog = screen.getByTestId('activity-log'); diff --git a/src/layouts/MainLayout.tsx b/src/layouts/MainLayout.tsx index 27a0ba78..82730854 100644 --- a/src/layouts/MainLayout.tsx +++ b/src/layouts/MainLayout.tsx @@ -38,7 +38,7 @@ export const MainLayout: React.FC = ({ onFlyerSelect, selectedF watchedItems, addWatchedItem, removeWatchedItem, error: watchedItemsError, } = useWatchedItems(); - const { activeDeals, totalActiveItems, isLoading: activeDealsLoading, error: activeDealsError } = useActiveDeals(); + const { totalActiveItems, error: activeDealsError } = useActiveDeals(); const handleActivityLogClick: ActivityLogClickHandler = useCallback((log) => { if (log.action === 'list_shared') { diff --git a/src/middleware/validation.middleware.test.ts b/src/middleware/validation.middleware.test.ts index 3bad06ec..221509cf 100644 --- a/src/middleware/validation.middleware.test.ts +++ b/src/middleware/validation.middleware.test.ts @@ -84,7 +84,7 @@ describe('validateRequest Middleware', () => { }; // For this test, we only need the `parseAsync` method on our mock schema. // We cast it to `unknown` first, then to the expected ZodObject type to satisfy TypeScript. - const middleware = validateRequest(mockSchema as unknown as z.ZodObject); + const middleware = validateRequest(mockSchema as unknown as z.ZodObject); // Act await middleware(mockRequest as Request, mockResponse as Response, mockNext); diff --git a/src/middleware/validation.middleware.ts b/src/middleware/validation.middleware.ts index 76eeed7b..32f355be 100644 --- a/src/middleware/validation.middleware.ts +++ b/src/middleware/validation.middleware.ts @@ -25,8 +25,7 @@ export const validateRequest = (schema: ZodObject) => // On success, replace the request parts with the parsed (and coerced) data. // This ensures downstream handlers get correctly typed data. req.params = params as ParamsDictionary; - // The parsed query is of type `unknown` here. We cast it to `any` to assign it back, - // trusting that the Zod schema has correctly shaped the data for downstream use. + // After parsing with Zod, the `query` object is correctly shaped. We cast to `any` as a bridge. req.query = query as any; req.body = body; diff --git a/src/routes/admin.content.routes.test.ts b/src/routes/admin.content.routes.test.ts index f5d929de..23cf930f 100644 --- a/src/routes/admin.content.routes.test.ts +++ b/src/routes/admin.content.routes.test.ts @@ -160,6 +160,13 @@ describe('Admin Content Management Routes (/api/admin)', () => { expect(response.body).toEqual(mockUpdatedCorrection); }); + it('PUT /corrections/:id should return 400 for invalid data', async () => { + const response = await supertest(app) + .put('/api/admin/corrections/101') + .send({ suggested_value: '' }); // Send empty value + expect(response.status).toBe(400); + }); + it('PUT /corrections/:id should return 404 if correction not found', async () => { vi.mocked(mockedDb.adminRepo.updateSuggestedCorrection).mockRejectedValue(new NotFoundError('Correction with ID 999 not found')); const response = await supertest(app).put('/api/admin/corrections/999').send({ suggested_value: 'new value' }); @@ -193,6 +200,12 @@ describe('Admin Content Management Routes (/api/admin)', () => { expect(response.status).toBe(400); expect(response.body.message).toBe('Logo image file is required.'); }); + + it('POST /brands/:id/logo should return 400 for an invalid brand ID', async () => { + const response = await supertest(app).post('/api/admin/brands/abc/logo') + .attach('logoImage', Buffer.from('dummy-logo-content'), 'test-logo.png'); + expect(response.status).toBe(400); + }); }); describe('Recipe and Comment Routes', () => { @@ -206,6 +219,13 @@ describe('Admin Content Management Routes (/api/admin)', () => { expect(response.body).toEqual(mockUpdatedRecipe); }); + it('PUT /recipes/:id/status should return 400 for an invalid status value', async () => { + const recipeId = 201; + const requestBody = { status: 'invalid_status' }; + const response = await supertest(app).put(`/api/admin/recipes/${recipeId}/status`).send(requestBody); + expect(response.status).toBe(400); + }); + it('PUT /comments/:id/status should update a comment status', async () => { const commentId = 301; const requestBody = { status: 'hidden' as const }; @@ -216,6 +236,13 @@ describe('Admin Content Management Routes (/api/admin)', () => { expect(response.body).toEqual(mockUpdatedComment); }); + it('PUT /comments/:id/status should return 400 for an invalid status value', async () => { + const commentId = 301; + const requestBody = { status: 'invalid_status' }; + const response = await supertest(app).put(`/api/admin/comments/${commentId}/status`).send(requestBody); + expect(response.status).toBe(400); + }); + }); describe('Unmatched Items Route', () => { @@ -254,5 +281,11 @@ describe('Admin Content Management Routes (/api/admin)', () => { expect(response.status).toBe(404); expect(response.body.message).toBe('Flyer with ID 999 not found.'); }); + + it('DELETE /flyers/:flyerId should return 400 for an invalid flyerId', async () => { + const response = await supertest(app).delete('/api/admin/flyers/abc'); + expect(response.status).toBe(400); + expect(response.body.errors[0].message).toBe('Expected number, received nan'); + }); }); }); \ No newline at end of file diff --git a/src/routes/admin.jobs.routes.test.ts b/src/routes/admin.jobs.routes.test.ts index f9561099..7a7a5100 100644 --- a/src/routes/admin.jobs.routes.test.ts +++ b/src/routes/admin.jobs.routes.test.ts @@ -181,6 +181,12 @@ describe('Admin Job Trigger Routes (/api/admin/trigger)', () => { expect(response.status).toBe(500); expect(response.body.message).toBe('Queue is down'); }); + + it('should return 400 for an invalid flyerId', async () => { + const response = await supertest(app).post('/api/admin/flyers/abc/cleanup'); + expect(response.status).toBe(400); + expect(response.body.errors[0].message).toBe('Expected number, received nan'); + }); }); describe('POST /jobs/:queueName/:jobId/retry', () => { @@ -246,5 +252,11 @@ describe('Admin Job Trigger Routes (/api/admin/trigger)', () => { expect(response.status).toBe(500); expect(response.body.message).toContain('Cannot retry job'); }); + + it('should return 400 for an invalid queueName or jobId', async () => { + // This tests the Zod schema validation for the route params. + const response = await supertest(app).post('/api/admin/jobs/ / /retry'); + expect(response.status).toBe(400); + }); }); }); \ No newline at end of file diff --git a/src/routes/admin.monitoring.routes.test.ts b/src/routes/admin.monitoring.routes.test.ts index 0d2ce97d..526b452c 100644 --- a/src/routes/admin.monitoring.routes.test.ts +++ b/src/routes/admin.monitoring.routes.test.ts @@ -143,6 +143,13 @@ describe('Admin Monitoring Routes (/api/admin)', () => { expect(adminRepo.getActivityLog).toHaveBeenCalledWith(10, 20); }); + + it('should return 400 for invalid limit and offset query parameters', async () => { + const response = await supertest(app).get('/api/admin/activity-log?limit=abc&offset=-1'); + expect(response.status).toBe(400); + expect(response.body.errors).toBeDefined(); + expect(response.body.errors.length).toBe(2); // Both limit and offset are invalid + }); }); describe('GET /workers/status', () => { diff --git a/src/routes/admin.routes.ts b/src/routes/admin.routes.ts index 083d4f19..27f9e2d0 100644 --- a/src/routes/admin.routes.ts +++ b/src/routes/admin.routes.ts @@ -1,7 +1,7 @@ // src/routes/admin.routes.ts -import { Router, NextFunction } from 'express'; +import { Router, NextFunction, Request, Response } from 'express'; import passport from './passport.routes'; -import { isAdmin } from './passport.routes'; // Correctly imported +import { isAdmin, optionalAuth } from './passport.routes'; // Correctly imported import multer from 'multer';// --- Zod Schemas for Admin Routes (as per ADR-003) --- import { z } from 'zod'; @@ -23,6 +23,10 @@ import { backgroundJobService } from '../services/backgroundJobService'; import { flyerQueue, emailQueue, analyticsQueue, cleanupQueue, weeklyAnalyticsQueue, flyerWorker, emailWorker, analyticsWorker, cleanupWorker, weeklyAnalyticsWorker } from '../services/queueService.server'; // Import your queues import { getSimpleWeekAndYear } from '../utils/dateUtils'; +const uuidParamSchema = (key: string) => z.object({ + params: z.object({ [key]: z.string().uuid() }), +}); + const numericIdParamSchema = (key: string) => z.object({ params: z.object({ [key]: z.coerce.number().int().positive() }), }); @@ -46,7 +50,7 @@ const updateCommentStatusSchema = numericIdParamSchema('id').extend({ }); const updateUserRoleSchema = z.object({ - params: z.object({ id: z.string().uuid("User ID must be a valid UUID.") }), + params: z.object({ id: z.string().uuid() }), body: z.object({ role: z.enum(['user', 'admin']), }), @@ -59,6 +63,10 @@ const activityLogSchema = z.object({ }), }); +const jobRetrySchema = z.object({ + params: z.object({ queueName: z.string(), jobId: z.string() }), +}); + const router = Router(); // --- Multer Configuration for File Uploads --- @@ -140,50 +148,52 @@ 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: Request, res: Response, next: NextFunction) => { + type ApproveCorrectionRequest = z.infer; + const { params } = req as unknown as ApproveCorrectionRequest; try { - const correctionId = req.params.id as unknown as number; - await db.adminRepo.approveCorrection(correctionId, req.log); + await db.adminRepo.approveCorrection(params.id, req.log); res.status(200).json({ message: 'Correction approved successfully.' }); } catch (error) { next(error); } }); -router.post('/corrections/:id/reject', validateRequest(numericIdParamSchema('id')), async (req, res, next: NextFunction) => { +router.post('/corrections/:id/reject', validateRequest(numericIdParamSchema('id')), async (req: Request, res: Response, next: NextFunction) => { + type RejectCorrectionRequest = z.infer; + const { params } = req as unknown as RejectCorrectionRequest; try { - const correctionId = req.params.id as unknown as number; - await db.adminRepo.rejectCorrection(correctionId, req.log); + await db.adminRepo.rejectCorrection(params.id, req.log); res.status(200).json({ message: 'Correction rejected successfully.' }); } catch (error) { next(error); } }); -router.put('/corrections/:id', validateRequest(updateCorrectionSchema), async (req, res, next: NextFunction) => { - const correctionId = req.params.id as unknown as number; - const { suggested_value } = req.body; +router.put('/corrections/:id', validateRequest(updateCorrectionSchema), async (req: Request, res: Response, next: NextFunction) => { + type UpdateCorrectionRequest = z.infer; + const { params, body } = req as unknown as UpdateCorrectionRequest; try { - const updatedCorrection = await db.adminRepo.updateSuggestedCorrection(correctionId, suggested_value, req.log); + const updatedCorrection = await db.adminRepo.updateSuggestedCorrection(params.id, body.suggested_value, req.log); res.status(200).json(updatedCorrection); } catch (error) { next(error); } }); -router.put('/recipes/:id/status', validateRequest(updateRecipeStatusSchema), async (req, res, next: NextFunction) => { - const recipeId = req.params.id as unknown as number; - const { status } = req.body; +router.put('/recipes/:id/status', validateRequest(updateRecipeStatusSchema), async (req: Request, res: Response, next: NextFunction) => { + type UpdateRecipeStatusRequest = z.infer; + const { params, body } = req as unknown as UpdateRecipeStatusRequest; try { - const updatedRecipe = await db.adminRepo.updateRecipeStatus(recipeId, status, req.log); // This is still a standalone function in admin.db.ts + const updatedRecipe = await db.adminRepo.updateRecipeStatus(params.id, body.status, req.log); // This is still a standalone function in admin.db.ts res.status(200).json(updatedRecipe); } catch (error) { next(error); // Pass all errors to the central error handler } }); -router.post('/brands/:id/logo', validateRequest(numericIdParamSchema('id')), upload.single('logoImage'), requireFileUpload('logoImage'), async (req, res, next: NextFunction) => { - const brandId = req.params.id as unknown as number; +router.post('/brands/:id/logo', validateRequest(numericIdParamSchema('id')), upload.single('logoImage'), requireFileUpload('logoImage'), async (req: Request, res: Response, next: NextFunction) => { + const { params } = req as unknown as z.infer>; try { // Although requireFileUpload middleware should ensure the file exists, // this check satisfies TypeScript and adds robustness. @@ -191,9 +201,9 @@ router.post('/brands/:id/logo', validateRequest(numericIdParamSchema('id')), upl throw new ValidationError([], 'Logo image file is missing.'); } const logoUrl = `/assets/${req.file.filename}`; - await db.adminRepo.updateBrandLogo(brandId, logoUrl, req.log); + await db.adminRepo.updateBrandLogo(params.id, logoUrl, req.log); - logger.info({ brandId, logoUrl }, `Brand logo updated for brand ID: ${brandId}`); + logger.info({ brandId: params.id, logoUrl }, `Brand logo updated for brand ID: ${params.id}`); res.status(200).json({ message: 'Brand logo updated successfully.', logoUrl }); } catch (error) { next(error); @@ -212,13 +222,12 @@ router.get('/unmatched-items', async (req, res, next: NextFunction) => { /** * DELETE /api/admin/recipes/:recipeId - Admin endpoint to delete any recipe. */ -router.delete('/recipes/:recipeId', validateRequest(numericIdParamSchema('recipeId')), async (req, res, next: NextFunction) => { +router.delete('/recipes/:recipeId', validateRequest(numericIdParamSchema('recipeId')), async (req: Request, res: Response, next: NextFunction) => { const adminUser = req.user as UserProfile; - const recipeId = req.params.recipeId as unknown as number; - + const { params } = req as unknown as z.infer>; try { // The isAdmin flag bypasses the ownership check in the repository method. - await db.recipeRepo.deleteRecipe(recipeId, adminUser.user_id, true, req.log); + await db.recipeRepo.deleteRecipe(params.recipeId, adminUser.user_id, true, req.log); res.status(204).send(); } catch (error: unknown) { next(error); @@ -228,21 +237,21 @@ router.delete('/recipes/:recipeId', validateRequest(numericIdParamSchema('recipe /** * DELETE /api/admin/flyers/:flyerId - Admin endpoint to delete a flyer and its items. */ -router.delete('/flyers/:flyerId', validateRequest(numericIdParamSchema('flyerId')), async (req, res, next: NextFunction) => { - const flyerId = req.params.flyerId as unknown as number; +router.delete('/flyers/:flyerId', validateRequest(numericIdParamSchema('flyerId')), async (req: Request, res: Response, next: NextFunction) => { + const { params } = req as unknown as z.infer>; try { - await db.flyerRepo.deleteFlyer(flyerId, req.log); + await db.flyerRepo.deleteFlyer(params.flyerId, req.log); res.status(204).send(); } catch (error: unknown) { next(error); } }); -router.put('/comments/:id/status', validateRequest(updateCommentStatusSchema), async (req, res, next: NextFunction) => { - const commentId = req.params.id as unknown as number; - const { status } = req.body; +router.put('/comments/:id/status', validateRequest(updateCommentStatusSchema), async (req: Request, res: Response, next: NextFunction) => { + type UpdateCommentStatusRequest = z.infer; + const { params, body } = req as unknown as UpdateCommentStatusRequest; try { - const updatedComment = await db.adminRepo.updateRecipeCommentStatus(commentId, status, req.log); // This is still a standalone function in admin.db.ts + const updatedComment = await db.adminRepo.updateRecipeCommentStatus(params.id, body.status, req.log); // This is still a standalone function in admin.db.ts res.status(200).json(updatedComment); } catch (error: unknown) { next(error); @@ -258,44 +267,47 @@ router.get('/users', 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 }; +router.get('/activity-log', validateRequest(activityLogSchema), async (req: Request, res: Response, next: NextFunction) => { + type ActivityLogRequest = z.infer; + const { query } = req as unknown as ActivityLogRequest; try { - const logs = await db.adminRepo.getActivityLog(limit, offset, req.log); + const logs = await db.adminRepo.getActivityLog(query.limit, query.offset, req.log); res.json(logs); } catch (error) { next(error); } }); -router.get('/users/:id', validateRequest(z.object({ params: z.object({ id: z.string().uuid() }) })), async (req, res, next: NextFunction) => { +router.get('/users/:id', validateRequest(uuidParamSchema('id')), async (req: Request, res: Response, next: NextFunction) => { + const { params } = req as z.infer>; try { - const user = await db.userRepo.findUserProfileById(req.params.id, req.log); + const user = await db.userRepo.findUserProfileById(params.id, req.log); res.json(user); } catch (error) { next(error); } }); -router.put('/users/:id', validateRequest(updateUserRoleSchema), async (req, res, next: NextFunction) => { - const { role } = req.body; - +router.put('/users/:id', validateRequest(updateUserRoleSchema), async (req: Request, res: Response, next: NextFunction) => { + type UpdateUserRoleRequest = z.infer; + const { params, body } = req as unknown as UpdateUserRoleRequest; try { - const updatedUser = await db.adminRepo.updateUserRole(req.params.id, role, req.log); + const updatedUser = await db.adminRepo.updateUserRole(params.id, body.role, req.log); res.json(updatedUser); } catch (error) { - logger.error({ error }, `Error updating user ${req.params.id}:`); + logger.error({ error }, `Error updating user ${params.id}:`); next(error); } }); -router.delete('/users/:id', validateRequest(z.object({ params: z.object({ id: z.string().uuid() }) })), async (req, res, next: NextFunction) => { +router.delete('/users/:id', validateRequest(uuidParamSchema('id')), async (req: Request, res: Response, next: NextFunction) => { const adminUser = req.user as UserProfile; + const { params } = req as z.infer>; try { - if (adminUser.user.user_id === req.params.id) { + if (adminUser.user.user_id === params.id) { throw new ValidationError([], 'Admins cannot delete their own account.'); } - await db.userRepo.deleteUserById(req.params.id, req.log); + await db.userRepo.deleteUserById(params.id, req.log); res.status(204).send(); } catch (error) { next(error); @@ -306,7 +318,7 @@ router.delete('/users/:id', validateRequest(z.object({ params: z.object({ id: z. * POST /api/admin/trigger/daily-deal-check - Manually trigger the daily deal check job. * This is useful for testing or forcing an update without waiting for the cron schedule. */ -router.post('/trigger/daily-deal-check', async (req, res, next: NextFunction) => { +router.post('/trigger/daily-deal-check', async (req: Request, res: Response, next: NextFunction) => { const adminUser = req.user as UserProfile; logger.info(`[Admin] Manual trigger for daily deal check received from user: ${adminUser.user_id}`); @@ -325,7 +337,7 @@ router.post('/trigger/daily-deal-check', async (req, res, next: NextFunction) => * POST /api/admin/trigger/analytics-report - Manually enqueue a job to generate the daily analytics report. * This is useful for testing or re-generating a report without waiting for the cron schedule. */ -router.post('/trigger/analytics-report', async (req, res, next: NextFunction) => { +router.post('/trigger/analytics-report', async (req: Request, res: Response, next: NextFunction) => { const adminUser = req.user as UserProfile; logger.info(`[Admin] Manual trigger for analytics report generation received from user: ${adminUser.user_id}`); @@ -347,16 +359,15 @@ router.post('/trigger/analytics-report', async (req, res, next: NextFunction) => * POST /api/admin/flyers/:flyerId/cleanup - Enqueue a job to clean up a flyer's files. * This is triggered by an admin after they have verified the flyer processing was successful. */ -router.post('/flyers/:flyerId/cleanup', validateRequest(numericIdParamSchema('flyerId')), async (req, res, next: NextFunction) => { +router.post('/flyers/:flyerId/cleanup', validateRequest(numericIdParamSchema('flyerId')), async (req: Request, res: Response, next: NextFunction) => { const adminUser = req.user as UserProfile; - const flyerId = req.params.flyerId as unknown as number; - - logger.info(`[Admin] Manual trigger for flyer file cleanup received from user: ${adminUser.user_id} for flyer ID: ${flyerId}`); + const { params } = req as unknown as z.infer>; + logger.info(`[Admin] Manual trigger for flyer file cleanup received from user: ${adminUser.user_id} for flyer ID: ${params.flyerId}`); // Enqueue the cleanup job. The worker will handle the file deletion. try { - await cleanupQueue.add('cleanup-flyer-files', { flyerId }); - res.status(202).json({ message: `File cleanup job for flyer ID ${flyerId} has been enqueued.` }); + await cleanupQueue.add('cleanup-flyer-files', { flyerId: params.flyerId }); + res.status(202).json({ message: `File cleanup job for flyer ID ${params.flyerId} has been enqueued.` }); } catch (error) { next(error); } @@ -366,7 +377,7 @@ router.post('/flyers/:flyerId/cleanup', validateRequest(numericIdParamSchema('fl * POST /api/admin/trigger/failing-job - Enqueue a test job designed to fail. * This is for testing the retry mechanism and Bull Board UI. */ -router.post('/trigger/failing-job', async (req, res, next: NextFunction) => { +router.post('/trigger/failing-job', async (req: Request, res: Response, next: NextFunction) => { const adminUser = req.user as UserProfile; logger.info(`[Admin] Manual trigger for a failing job received from user: ${adminUser.user_id}`); @@ -383,7 +394,7 @@ router.post('/trigger/failing-job', async (req, res, next: NextFunction) => { * POST /api/admin/system/clear-geocode-cache - Clears the Redis cache for geocoded addresses. * Requires admin privileges. */ -router.post('/system/clear-geocode-cache', async (req, res, next: NextFunction) => { +router.post('/system/clear-geocode-cache', async (req: Request, res: Response, next: NextFunction) => { const adminUser = req.user as UserProfile; logger.info(`[Admin] Manual trigger for geocode cache clear received from user: ${adminUser.user_id}`); @@ -400,7 +411,7 @@ router.post('/system/clear-geocode-cache', async (req, res, next: NextFunction) * GET /api/admin/workers/status - Get the current running status of all BullMQ workers. * This is useful for a system health dashboard to see if any workers have crashed. */ -router.get('/workers/status', async (req, res) => { +router.get('/workers/status', async (req: Request, res: Response) => { const workers = [flyerWorker, emailWorker, analyticsWorker, cleanupWorker, weeklyAnalyticsWorker ]; const workerStatuses = await Promise.all( @@ -419,7 +430,7 @@ router.get('/workers/status', async (req, res) => { * GET /api/admin/queues/status - Get job counts for all BullMQ queues. * This is useful for monitoring the health and backlog of background jobs. */ -router.get('/queues/status', async (req, res, next: NextFunction) => { +router.get('/queues/status', async (req: Request, res: Response, next: NextFunction) => { try { const queues = [flyerQueue, emailQueue, analyticsQueue, cleanupQueue, weeklyAnalyticsQueue]; @@ -440,9 +451,9 @@ router.get('/queues/status', async (req, res, next: NextFunction) => { /** * POST /api/admin/jobs/:queueName/:jobId/retry - Retries a specific failed job. */ -router.post('/jobs/:queueName/:jobId/retry', async (req, res, next: NextFunction) => { - const { queueName, jobId } = req.params; +router.post('/jobs/:queueName/:jobId/retry', validateRequest(jobRetrySchema), async (req: Request, res: Response, next: NextFunction) => { const adminUser = req.user as UserProfile; + const { params: { queueName, jobId } } = req as unknown as z.infer; const queueMap: { [key: string]: Queue } = { 'flyer-processing': flyerQueue, @@ -476,7 +487,7 @@ router.post('/jobs/:queueName/:jobId/retry', async (req, res, next: NextFunction /** * POST /api/admin/trigger/weekly-analytics - Manually trigger the weekly analytics report job. */ -router.post('/trigger/weekly-analytics', async (req, res, next: NextFunction) => { +router.post('/trigger/weekly-analytics', async (req: Request, res: Response, next: NextFunction) => { const adminUser = req.user as UserProfile; logger.info(`[Admin] Manual trigger for weekly analytics report received from user: ${adminUser.user_id}`); diff --git a/src/routes/admin.users.routes.test.ts b/src/routes/admin.users.routes.test.ts index c64a7acc..50cc175b 100644 --- a/src/routes/admin.users.routes.test.ts +++ b/src/routes/admin.users.routes.test.ts @@ -146,6 +146,13 @@ describe('Admin User Management Routes (/api/admin/users)', () => { expect(response.status).toBe(404); expect(response.body.message).toBe('User with ID non-existent not found.'); }); + + it('should return 400 for an invalid role', async () => { + const response = await supertest(app) + .put('/api/admin/users/user-to-update') + .send({ role: 'super-admin' }); + expect(response.status).toBe(400); + }); }); describe('DELETE /users/:id', () => { diff --git a/src/routes/ai.routes.test.ts b/src/routes/ai.routes.test.ts index c875760c..adfd4abe 100644 --- a/src/routes/ai.routes.test.ts +++ b/src/routes/ai.routes.test.ts @@ -95,6 +95,15 @@ describe('AI Routes (/api/ai)', () => { expect(response.body.message).toBe('A flyer file (PDF or image) is required.'); }); + it('should return 400 if checksum is missing', async () => { + const response = await supertest(app) + .post('/api/ai/upload-and-process') + .attach('flyerFile', imagePath); + + expect(response.status).toBe(400); + expect(response.body.errors[0].message).toBe('File checksum is required.'); + }); + it('should return 409 if flyer checksum already exists', async () => { vi.mocked(db.flyerRepo.findFlyerByChecksum).mockResolvedValue(createMockFlyer({ flyer_id: 99 })); @@ -195,6 +204,13 @@ describe('AI Routes (/api/ai)', () => { expect(response.status).toBe(200); expect(response.body.state).toBe('completed'); }); + + it('should return 400 for an invalid job ID format', async () => { + // Assuming job IDs should not be empty, for example. + const response = await supertest(app).get('/api/ai/jobs/ /status'); // Send an invalid ID + expect(response.status).toBe(400); + expect(response.body.errors[0].message).toBe('A valid Job ID is required.'); + }); }); describe('POST /flyers/process (Legacy)', () => { @@ -384,6 +400,15 @@ describe('AI Routes (/api/ai)', () => { .field('extractionType', 'store_name'); expect(response.status).toBe(400); }); + + it('should return 400 if cropArea or extractionType is missing', async () => { + const response = await supertest(app) + .post('/api/ai/rescan-area') + .attach('image', imagePath) + .field('extractionType', 'store_name'); // Missing cropArea + expect(response.status).toBe(400); + expect(response.body.errors[0].message).toContain('cropArea must be a valid JSON string'); + }); }); describe('POST /extract-address', () => { diff --git a/src/routes/ai.routes.ts b/src/routes/ai.routes.ts index fd10f79f..98c62a91 100644 --- a/src/routes/ai.routes.ts +++ b/src/routes/ai.routes.ts @@ -192,7 +192,8 @@ router.post('/upload-and-process', optionalAuth, uploadToDisk.single('flyerFile' * NEW ENDPOINT: Checks the status of a background job. */ router.get('/jobs/:jobId/status', validateRequest(jobIdParamSchema), async (req, res, next: NextFunction) => { - const { jobId } = req.params; + type JobIdRequest = z.infer; + const { params: { jobId } } = req as unknown as JobIdRequest; try { const job = await flyerQueue.getJob(jobId); if (!job) { diff --git a/src/routes/auth.routes.test.ts b/src/routes/auth.routes.test.ts index f7666356..a0189913 100644 --- a/src/routes/auth.routes.test.ts +++ b/src/routes/auth.routes.test.ts @@ -240,6 +240,24 @@ describe('Auth Routes (/api/auth)', () => { expect(response.status).toBe(500); expect(response.body.message).toBe('DB connection lost'); // The errorHandler will forward the message }); + + it('should return 400 for an invalid email format', async () => { + const response = await supertest(app) + .post('/api/auth/register') + .send({ email: 'not-an-email', password: strongPassword }); + + expect(response.status).toBe(400); + expect(response.body.errors[0].message).toBe('A valid email is required.'); + }); + + it('should return 400 for a password that is too short', async () => { + const response = await supertest(app) + .post('/api/auth/register') + .send({ email: newUserEmail, password: 'short' }); + + expect(response.status).toBe(400); + expect(response.body.errors[0].message).toBe('Password must be at least 8 characters long.'); + }); }); describe('POST /login', () => { @@ -393,6 +411,15 @@ describe('Auth Routes (/api/auth)', () => { // Assert: The route should not fail even if the email does. expect(response.status).toBe(200); }); + + it('should return 400 for an invalid email format', async () => { + const response = await supertest(app) + .post('/api/auth/forgot-password') + .send({ email: 'invalid-email' }); + + expect(response.status).toBe(400); + expect(response.body.errors[0].message).toBe('A valid email is required.'); + }); }); describe('POST /reset-password', () => { @@ -431,6 +458,15 @@ describe('Auth Routes (/api/auth)', () => { const response = await supertest(app).post('/api/auth/reset-password').send({ token: 'valid-token', newPassword: 'weak' }); expect(response.status).toBe(400); }); + + it('should return 400 if token is missing', async () => { + const response = await supertest(app) + .post('/api/auth/reset-password') + .send({ newPassword: 'a-Very-Strong-Password-789!' }); + + expect(response.status).toBe(400); + expect(response.body.errors[0].message).toBe('Token is required.'); + }); }); describe('POST /refresh-token', () => { diff --git a/src/routes/auth.routes.ts b/src/routes/auth.routes.ts index 51cdeb02..c6702898 100644 --- a/src/routes/auth.routes.ts +++ b/src/routes/auth.routes.ts @@ -90,9 +90,10 @@ const resetPasswordSchema = z.object({ // --- Authentication Routes --- // Registration Route -router.post('/register', validateRequest(registerSchema), async (req, res, next) => { - const { email, password, full_name, avatar_url } = req.body; - +router.post('/register', validateRequest(registerSchema), async (req: Request, res: Response, next: NextFunction) => { + type RegisterRequest = z.infer; + const { body: { email, password, full_name, avatar_url } } = req as unknown as RegisterRequest; + try { const saltRounds = 10; const hashedPassword = await bcrypt.hash(password, saltRounds); @@ -190,9 +191,10 @@ router.post('/login', (req: Request, res: Response, next: NextFunction) => { }); // Route to request a password reset -router.post('/forgot-password', forgotPasswordLimiter, validateRequest(forgotPasswordSchema), async (req, res, next) => { - const { email } = req.body; - +router.post('/forgot-password', forgotPasswordLimiter, validateRequest(forgotPasswordSchema), async (req: Request, res: Response, next: NextFunction) => { + type ForgotPasswordRequest = z.infer; + const { body: { email } } = req as unknown as ForgotPasswordRequest; + try { req.log.debug(`[API /forgot-password] Received request for email: ${email}`); const user = await userRepo.findUserByEmail(email, req.log); @@ -231,9 +233,10 @@ router.post('/forgot-password', forgotPasswordLimiter, validateRequest(forgotPas }); // Route to reset the password using a token -router.post('/reset-password', resetPasswordLimiter, validateRequest(resetPasswordSchema), async (req, res, next) => { - const { token, newPassword } = req.body; - +router.post('/reset-password', resetPasswordLimiter, validateRequest(resetPasswordSchema), async (req: Request, res: Response, next: NextFunction) => { + type ResetPasswordRequest = z.infer; + const { body: { token, newPassword } } = req as unknown as ResetPasswordRequest; + try { const validTokens = await userRepo.getValidResetTokens(req.log); let tokenRecord; diff --git a/src/routes/budget.routes.test.ts b/src/routes/budget.routes.test.ts index 1058fdcb..92be7c18 100644 --- a/src/routes/budget.routes.test.ts +++ b/src/routes/budget.routes.test.ts @@ -114,6 +114,20 @@ describe('Budget Routes (/api/budgets)', () => { expect(response.status).toBe(500); expect(response.body.message).toBe('DB Error'); }); + + it('should return 400 for invalid budget data', async () => { + const invalidData = { + name: '', // empty name + amount_cents: -100, // negative amount + period: 'yearly', // invalid period + start_date: 'not-a-date', // invalid date + }; + + const response = await supertest(app).post('/api/budgets').send(invalidData); + + expect(response.status).toBe(400); + expect(response.body.errors).toHaveLength(4); + }); }); describe('PUT /:id', () => { @@ -143,6 +157,12 @@ describe('Budget Routes (/api/budgets)', () => { expect(response.status).toBe(500); // The custom handler will now be used expect(response.body.message).toBe('DB Error'); }); + + it('should return 400 if no update fields are provided', async () => { + const response = await supertest(app).put('/api/budgets/1').send({}); + expect(response.status).toBe(400); + expect(response.body.errors[0].message).toBe('At least one field to update must be provided.'); + }); }); describe('DELETE /:id', () => { @@ -192,5 +212,11 @@ describe('Budget Routes (/api/budgets)', () => { expect(response.status).toBe(500); expect(response.body.message).toBe('DB Error'); }); + + it('should return 400 for invalid date formats', async () => { + const response = await supertest(app).get('/api/budgets/spending-analysis?startDate=2024/01/01&endDate=invalid'); + expect(response.status).toBe(400); + expect(response.body.errors).toHaveLength(2); + }); }); }); \ No newline at end of file diff --git a/src/routes/budget.routes.ts b/src/routes/budget.routes.ts index c38dbd66..1293513d 100644 --- a/src/routes/budget.routes.ts +++ b/src/routes/budget.routes.ts @@ -1,5 +1,5 @@ // src/routes/budget.ts -import express, { NextFunction } from 'express'; +import express, { Request, Response, NextFunction } from 'express'; import { z } from 'zod'; import passport from './passport.routes'; import { budgetRepo } from '../services/db/index.db'; @@ -20,7 +20,7 @@ const createBudgetSchema = z.object({ body: z.object({ name: z.string().min(1, 'Budget name is required.'), amount_cents: z.number().int().positive('Amount must be a positive integer.'), - period: z.enum(['weekly', 'monthly', 'yearly']), + period: z.enum(['weekly', 'monthly']), start_date: z.string().date('Start date must be a valid date in YYYY-MM-DD format.'), }), }); @@ -44,13 +44,13 @@ router.use(passport.authenticate('jwt', { session: false })); /** * GET /api/budgets - Get all budgets for the authenticated user. */ -router.get('/', async (req, res, next: NextFunction) => { +router.get('/', async (req: Request, res: Response, next: NextFunction) => { const user = req.user as UserProfile; try { const budgets = await budgetRepo.getBudgetsForUser(user.user_id, req.log); res.json(budgets); } catch (error) { - req.log.error({ err: error, userId: user.user_id }, 'Error fetching budgets'); + req.log.error({ error, userId: user.user_id }, 'Error fetching budgets'); next(error); } }); @@ -58,13 +58,15 @@ router.get('/', async (req, res, next: NextFunction) => { /** * POST /api/budgets - Create a new budget for the authenticated user. */ -router.post('/', validateRequest(createBudgetSchema), async (req, res, next: NextFunction) => { +router.post('/', validateRequest(createBudgetSchema), async (req: Request, res: Response, next: NextFunction) => { const user = req.user as UserProfile; + type CreateBudgetRequest = z.infer; + const { body } = req as unknown as CreateBudgetRequest; try { - const newBudget = await budgetRepo.createBudget(user.user_id, req.body, req.log); + const newBudget = await budgetRepo.createBudget(user.user_id, body, req.log); res.status(201).json(newBudget); } catch (error: unknown) { - req.log.error({ err: error, userId: user.user_id, body: req.body }, 'Error creating budget'); + req.log.error({ error, userId: user.user_id, body }, 'Error creating budget'); next(error); } }); @@ -72,14 +74,15 @@ router.post('/', validateRequest(createBudgetSchema), async (req, res, next: Nex /** * PUT /api/budgets/:id - Update an existing budget. */ -router.put('/:id', validateRequest(updateBudgetSchema), async (req, res, next: NextFunction) => { +router.put('/:id', validateRequest(updateBudgetSchema), async (req: Request, res: Response, next: NextFunction) => { const user = req.user as UserProfile; - const budgetId = req.params.id as unknown as number; + type UpdateBudgetRequest = z.infer; + const { params, body } = req as unknown as UpdateBudgetRequest; try { - const updatedBudget = await budgetRepo.updateBudget(budgetId, user.user_id, req.body, req.log); + const updatedBudget = await budgetRepo.updateBudget(params.id, user.user_id, body, req.log); res.json(updatedBudget); } catch (error: unknown) { - req.log.error({ err: error, userId: user.user_id, budgetId }, 'Error updating budget'); + req.log.error({ error, userId: user.user_id, budgetId: params.id }, 'Error updating budget'); next(error); } }); @@ -87,14 +90,15 @@ router.put('/:id', validateRequest(updateBudgetSchema), async (req, res, next: N /** * DELETE /api/budgets/:id - Delete a budget. */ -router.delete('/:id', validateRequest(budgetIdParamSchema), async (req, res, next: NextFunction) => { +router.delete('/:id', validateRequest(budgetIdParamSchema), async (req: Request, res: Response, next: NextFunction) => { const user = req.user as UserProfile; - const budgetId = req.params.id as unknown as number; + type DeleteBudgetRequest = z.infer; + const { params } = req as unknown as DeleteBudgetRequest; try { - await budgetRepo.deleteBudget(budgetId, user.user_id, req.log); + await budgetRepo.deleteBudget(params.id, user.user_id, req.log); res.status(204).send(); // No Content } catch (error: unknown) { - req.log.error({ err: error, userId: user.user_id, budgetId }, 'Error deleting budget'); + req.log.error({ error, userId: user.user_id, budgetId: params.id }, 'Error deleting budget'); next(error); } }); @@ -103,15 +107,16 @@ router.delete('/:id', validateRequest(budgetIdParamSchema), async (req, res, nex * GET /api/spending-analysis - Get spending breakdown by category for a date range. * Query params: startDate (YYYY-MM-DD), endDate (YYYY-MM-DD) */ -router.get('/spending-analysis', validateRequest(spendingAnalysisSchema), async (req, res, next: NextFunction) => { +router.get('/spending-analysis', validateRequest(spendingAnalysisSchema), async (req: Request, res: Response, next: NextFunction) => { const user = req.user as UserProfile; - const { startDate, endDate } = req.query; + type SpendingAnalysisRequest = z.infer; + const { query: { startDate, endDate } } = req as unknown as SpendingAnalysisRequest; try { - const spendingData = await budgetRepo.getSpendingByCategory(user.user_id, startDate as string, endDate as string, req.log); + const spendingData = await budgetRepo.getSpendingByCategory(user.user_id, startDate, endDate, req.log); res.json(spendingData); } catch (error) { - req.log.error({ err: error, userId: user.user_id, startDate, endDate }, 'Error fetching spending analysis'); + req.log.error({ error, userId: user.user_id, startDate, endDate }, 'Error fetching spending analysis'); next(error); } }); diff --git a/src/routes/deals.routes.ts b/src/routes/deals.routes.ts index da7b8f87..c9f235e9 100644 --- a/src/routes/deals.routes.ts +++ b/src/routes/deals.routes.ts @@ -1,11 +1,19 @@ // src/routes/deals.routes.ts import express, { type Request, type Response, type NextFunction } from 'express'; +import { z } from 'zod'; import passport from './passport.routes'; import { dealsRepo } from '../services/db/deals.db'; import type { UserProfile } from '../types'; +import { validateRequest } from '../middleware/validation.middleware'; const router = express.Router(); +// --- Zod Schemas for Deals Routes (as per ADR-003) --- + +const bestWatchedPricesSchema = z.object({ + // No params, query, or body expected for this route currently. +}); + // --- Middleware for all deal routes --- // Per ADR-002, all routes in this file require an authenticated user. @@ -17,7 +25,7 @@ router.use(passport.authenticate('jwt', { session: false })); * @description Fetches the best current sale price for each of the authenticated user's watched items. * @access Private */ -router.get('/best-watched-prices', async (req: Request, res: Response, next: NextFunction) => { +router.get('/best-watched-prices', validateRequest(bestWatchedPricesSchema), async (req: Request, res: Response, next: NextFunction) => { const user = req.user as UserProfile; try { // The controller logic is simple enough to be handled directly in the route, @@ -26,7 +34,7 @@ router.get('/best-watched-prices', async (req: Request, res: Response, next: Nex req.log.info({ dealCount: deals.length }, 'Successfully fetched best watched item deals.'); res.status(200).json(deals); } catch (error) { - req.log.error({ err: error }, 'Error fetching best watched item deals.'); + req.log.error({ error }, 'Error fetching best watched item deals.'); next(error); // Pass errors to the global error handler } }); diff --git a/src/routes/flyer.routes.test.ts b/src/routes/flyer.routes.test.ts index 63c15261..f85bd846 100644 --- a/src/routes/flyer.routes.test.ts +++ b/src/routes/flyer.routes.test.ts @@ -32,6 +32,13 @@ import * as db from '../services/db/index.db'; // Create the Express app const app = express(); app.use(express.json()); + +// Add a middleware to inject a mock req.log object for tests +app.use((req, res, next) => { + req.log = { info: vi.fn(), debug: vi.fn(), error: vi.fn(), warn: vi.fn() } as any; + next(); +}); + // Mount the router under its designated base path app.use('/api/flyers', flyerRouter); app.use(errorHandler); @@ -64,6 +71,13 @@ describe('Flyer Routes (/api/flyers)', () => { expect(response.status).toBe(500); expect(response.body.message).toBe('DB Error'); }); + + it('should return 400 for invalid query parameters', async () => { + const response = await supertest(app).get('/api/flyers?limit=abc&offset=-5'); + expect(response.status).toBe(400); + expect(response.body.errors).toBeDefined(); + expect(response.body.errors.length).toBe(2); + }); }); describe('GET /:id', () => { diff --git a/src/routes/flyer.routes.ts b/src/routes/flyer.routes.ts index 6972714a..b3442397 100644 --- a/src/routes/flyer.routes.ts +++ b/src/routes/flyer.routes.ts @@ -41,10 +41,11 @@ const trackItemSchema = z.object({ /** * GET /api/flyers - Get a paginated list of all flyers. */ -router.get('/', validateRequest(getFlyersSchema), async (req, res, next: NextFunction) => { +type GetFlyersRequest = z.infer; +router.get('/', validateRequest(getFlyersSchema), async (req, res, next): Promise => { + const { query } = req as unknown as GetFlyersRequest; try { - const { limit, offset } = req.query as unknown as { limit: number; offset: number }; - const flyers = await db.flyerRepo.getFlyers(req.log, limit, offset); + const flyers = await db.flyerRepo.getFlyers(req.log, query.limit, query.offset); res.json(flyers); } catch (error) { req.log.error({ error }, 'Error fetching flyers in /api/flyers:'); @@ -55,10 +56,11 @@ router.get('/', validateRequest(getFlyersSchema), async (req, res, next: NextFun /** * GET /api/flyers/:id - Get a single flyer by its ID. */ -router.get('/:id', validateRequest(flyerIdParamSchema), async (req, res, next: NextFunction) => { +type GetFlyerByIdRequest = z.infer; +router.get('/:id', validateRequest(flyerIdParamSchema), async (req, res, next): Promise => { + const { params } = req as unknown as GetFlyerByIdRequest; try { - const flyerId = req.params.id as unknown as number; - const flyer = await db.flyerRepo.getFlyerById(flyerId, req.log); + const flyer = await db.flyerRepo.getFlyerById(params.id, req.log); res.json(flyer); } catch (error) { next(error); @@ -68,10 +70,10 @@ router.get('/:id', validateRequest(flyerIdParamSchema), async (req, res, next: N /** * GET /api/flyers/:id/items - Get all items for a specific flyer. */ -router.get('/:id/items', validateRequest(flyerIdParamSchema), async (req, res, next: NextFunction) => { +router.get('/:id/items', validateRequest(flyerIdParamSchema), async (req, res, next): Promise => { + const { params } = req as unknown as GetFlyerByIdRequest; try { - const flyerId = req.params.id as unknown as number; - const items = await db.flyerRepo.getFlyerItems(flyerId, req.log); + const items = await db.flyerRepo.getFlyerItems(params.id, req.log); res.json(items); } catch (error) { req.log.error({ error }, 'Error fetching flyer items in /api/flyers/:id/items:'); @@ -82,10 +84,11 @@ router.get('/:id/items', validateRequest(flyerIdParamSchema), async (req, res, n /** * POST /api/flyers/items/batch-fetch - Get all items for multiple flyers at once. */ -router.post('/items/batch-fetch', validateRequest(batchFetchSchema), async (req, res, next: NextFunction) => { - const { flyerIds } = req.body as { flyerIds: number[] }; +type BatchFetchRequest = z.infer; +router.post('/items/batch-fetch', validateRequest(batchFetchSchema), async (req, res, next): Promise => { + const { body } = req as unknown as BatchFetchRequest; try { - const items = await db.flyerRepo.getFlyerItemsForFlyers(flyerIds, req.log); + const items = await db.flyerRepo.getFlyerItemsForFlyers(body.flyerIds, req.log); res.json(items); } catch (error) { next(error); @@ -95,11 +98,12 @@ router.post('/items/batch-fetch', validateRequest(batchFetchSchema), async (req, /** * POST /api/flyers/items/batch-count - Get the total number of items for multiple flyers. */ -router.post('/items/batch-count', validateRequest(batchFetchSchema.partial()), async (req, res, next: NextFunction) => { - const { flyerIds } = req.body as { flyerIds?: number[] }; +type BatchCountRequest = z.infer; +router.post('/items/batch-count', validateRequest(batchFetchSchema.partial()), async (req, res, next): Promise => { + const { body } = req as unknown as BatchCountRequest; try { // The DB function handles an empty array, so we can simplify. - const count = await db.flyerRepo.countFlyerItemsForFlyers(flyerIds ?? [], req.log); + const count = await db.flyerRepo.countFlyerItemsForFlyers(body.flyerIds ?? [], req.log); res.json({ count }); } catch (error) { next(error); @@ -109,10 +113,10 @@ router.post('/items/batch-count', validateRequest(batchFetchSchema.partial()), a /** * POST /api/flyers/items/:itemId/track - Tracks a user interaction with a flyer item. */ -router.post('/items/:itemId/track', validateRequest(trackItemSchema), (req: Request, res: Response) => { - const itemId = req.params.itemId as unknown as number; - const { type } = req.body as { type: 'view' | 'click' }; - db.flyerRepo.trackFlyerItemInteraction(itemId, type, req.log); +type TrackItemRequest = z.infer; +router.post('/items/:itemId/track', validateRequest(trackItemSchema), (req, res): void => { + const { params, body } = req as unknown as TrackItemRequest; + db.flyerRepo.trackFlyerItemInteraction(params.itemId, body.type, req.log); res.status(202).send(); }); diff --git a/src/routes/gamification.routes.test.ts b/src/routes/gamification.routes.test.ts index 5b721538..f101df4a 100644 --- a/src/routes/gamification.routes.test.ts +++ b/src/routes/gamification.routes.test.ts @@ -184,6 +184,15 @@ describe('Gamification Routes (/api/achievements)', () => { expect(response.body.message).toBe('DB Error'); }); + it('should return 400 for an invalid userId or achievementName', async () => { + mockedAuthMiddleware.mockImplementation((req: Request, res: Response, next: NextFunction) => { req.user = mockAdminProfile; next(); }); + mockedIsAdmin.mockImplementation((req: Request, res: Response, next: NextFunction) => next()); + + const response = await supertest(app).post('/api/achievements/award').send({ userId: '', achievementName: '' }); + expect(response.status).toBe(400); + expect(response.body.errors).toHaveLength(2); + }); + it('should return 400 if awarding an achievement to a non-existent user', async () => { mockedAuthMiddleware.mockImplementation((req: Request, res: Response, next: NextFunction) => { req.user = mockAdminProfile; @@ -216,5 +225,12 @@ describe('Gamification Routes (/api/achievements)', () => { expect(response.status).toBe(500); expect(response.body.message).toBe('DB Error'); }); + + it('should return 400 for an invalid limit parameter', async () => { + const response = await supertest(app).get('/api/achievements/leaderboard?limit=100'); + expect(response.status).toBe(400); + expect(response.body.errors).toBeDefined(); + expect(response.body.errors[0].message).toContain('less than or equal to 50'); + }); }); }); diff --git a/src/routes/gamification.routes.ts b/src/routes/gamification.routes.ts index b71c0f1d..e3a8e1c4 100644 --- a/src/routes/gamification.routes.ts +++ b/src/routes/gamification.routes.ts @@ -34,10 +34,10 @@ const awardAchievementSchema = z.object({ */ router.get('/', async (req, res, next: NextFunction) => { try { - const achievements = await gamificationRepo.getAllAchievements(); + const achievements = await gamificationRepo.getAllAchievements(req.log); res.json(achievements); } catch (error) { - logger.error('Error fetching all achievements in /api/achievements:', { error }); + logger.error({ error }, 'Error fetching all achievements in /api/achievements:'); next(error); } }); @@ -46,13 +46,14 @@ router.get('/', async (req, res, next: NextFunction) => { * GET /api/achievements/leaderboard - Get the top users by points. * This is a public endpoint. */ -router.get('/leaderboard', validateRequest(leaderboardSchema), async (req, res, next: NextFunction) => { - try { // The `limit` is coerced to a number by Zod, but TypeScript doesn't know that yet. - const { limit } = req.query as unknown as { limit: number }; - const leaderboard = await gamificationRepo.getLeaderboard(limit); +type LeaderboardRequest = z.infer; +router.get('/leaderboard', validateRequest(leaderboardSchema), async (req, res, next: NextFunction): Promise => { + const { query } = req as unknown as LeaderboardRequest; + try { + const leaderboard = await gamificationRepo.getLeaderboard(query.limit, req.log); res.json(leaderboard); } catch (error) { - logger.error('Error fetching leaderboard:', { error }); + logger.error({ error }, 'Error fetching leaderboard:'); next(error); } }); @@ -66,13 +67,13 @@ router.get('/leaderboard', validateRequest(leaderboardSchema), async (req, res, router.get( '/me', passport.authenticate('jwt', { session: false }), - async (req, res, next: NextFunction) => { + async (req, res, next: NextFunction): Promise => { const user = req.user as UserProfile; try { - const userAchievements = await gamificationRepo.getUserAchievements(user.user_id); + const userAchievements = await gamificationRepo.getUserAchievements(user.user_id, req.log); res.json(userAchievements); } catch (error) { - logger.error('Error fetching user achievements:', { error, userId: user.user_id }); + logger.error({ error, userId: user.user_id }, 'Error fetching user achievements:'); next(error); } } @@ -88,19 +89,20 @@ adminGamificationRouter.use(passport.authenticate('jwt', { session: false }), is * This is an admin-only endpoint. */ adminGamificationRouter.post( - '/award', - validateRequest(awardAchievementSchema), - async (req, res, next: NextFunction) => { - const { userId, achievementName } = req.body; - + '/award', validateRequest(awardAchievementSchema), + async (req, res, next: NextFunction): Promise => { + // Infer type and cast request object as per ADR-003 + type AwardAchievementRequest = z.infer; + const { body } = req as unknown as AwardAchievementRequest; try { - await gamificationRepo.awardAchievement(userId, achievementName); - res.status(200).json({ message: `Successfully awarded '${achievementName}' to user ${userId}.` }); + await gamificationRepo.awardAchievement(body.userId, body.achievementName, req.log); + res.status(200).json({ message: `Successfully awarded '${body.achievementName}' to user ${body.userId}.` }); } catch (error) { if (error instanceof ForeignKeyConstraintError) { - return res.status(400).json({ message: error.message }); + res.status(400).json({ message: error.message }); + return; } - logger.error('Error awarding achievement via admin endpoint:', { error, userId, achievementName }); + logger.error({ error, userId: body.userId, achievementName: body.achievementName }, 'Error awarding achievement via admin endpoint:'); next(error); } } diff --git a/src/routes/health.routes.ts b/src/routes/health.routes.ts index 3b7f0063..7aa346d9 100644 --- a/src/routes/health.routes.ts +++ b/src/routes/health.routes.ts @@ -1,18 +1,32 @@ // src/routes/health.routes.ts import { Router, Request, Response, NextFunction } from 'express'; +import { z } from 'zod'; import { checkTablesExist, getPoolStatus } from '../services/db/connection.db'; import { logger } from '../services/logger.server'; import { connection as redisConnection } from '../services/queueService.server'; import fs from 'node:fs/promises'; import { getSimpleWeekAndYear } from '../utils/dateUtils'; +import { validateRequest } from '../middleware/validation.middleware'; const router = Router(); -router.get('/ping', (_req: Request, res: Response) => { +// --- Zod Schemas for Health Routes (as per ADR-003) --- +// These routes do not expect any input, so we define empty schemas +// to maintain a consistent validation pattern across the application. +const emptySchema = z.object({}); + +/** + * GET /api/health/ping - A simple endpoint to check if the server is responsive. + */ +router.get('/ping', validateRequest(emptySchema), (_req: Request, res: Response) => { res.status(200).send('pong'); }); -router.get('/db-schema', async (req, res, next: NextFunction) => { +/** + * GET /api/health/db-schema - Checks if all essential database tables exist. + * This is a critical check to ensure the database schema is correctly set up. + */ +router.get('/db-schema', validateRequest(emptySchema), async (req, res, next: NextFunction) => { try { const requiredTables = ['users', 'profiles', 'flyers', 'flyer_items', 'stores']; const missingTables = await checkTablesExist(requiredTables); @@ -23,23 +37,31 @@ router.get('/db-schema', async (req, res, next: NextFunction) => { } return res.status(200).json({ success: true, message: 'All required database tables exist.' }); } catch (error: unknown) { - logger.error('Error during DB schema check:', { error: error instanceof Error ? error.message : error }); + logger.error({ error: error instanceof Error ? error.message : error }, 'Error during DB schema check:'); next(error); } }); -router.get('/storage', async (req, res, next: NextFunction) => { +/** + * GET /api/health/storage - Verifies that the application's file storage path is accessible and writable. + * This is important for features like file uploads. + */ +router.get('/storage', validateRequest(emptySchema), async (req, res, next: NextFunction) => { const storagePath = process.env.STORAGE_PATH || '/var/www/flyer-crawler.projectium.com/assets'; try { await fs.access(storagePath, fs.constants.W_OK); // Use fs.promises return res.status(200).json({ success: true, message: `Storage directory '${storagePath}' is accessible and writable.` }); } catch (error: unknown) { - logger.error(`Storage check failed for path: ${storagePath}`, { error: error instanceof Error ? error.message : error }); + logger.error({ error: error instanceof Error ? error.message : error }, `Storage check failed for path: ${storagePath}`); next(new Error(`Storage check failed. Ensure the directory '${storagePath}' exists and is writable by the application.`)); } }); -router.get('/db-pool', (req: Request, res: Response, next: NextFunction) => { +/** + * GET /api/health/db-pool - Checks the status of the database connection pool. + * This helps diagnose issues related to database connection saturation. + */ +router.get('/db-pool', validateRequest(emptySchema), (req: Request, res: Response, next: NextFunction) => { try { const status = getPoolStatus(); const isHealthy = status.waitingCount < 5; @@ -52,12 +74,16 @@ router.get('/db-pool', (req: Request, res: Response, next: NextFunction) => { return res.status(500).json({ success: false, message: `Pool may be under stress. ${message}` }); } } catch (error: unknown) { - logger.error('Error during DB pool health check:', { error: error instanceof Error ? error.message : error }); + logger.error({ error: error instanceof Error ? error.message : error }, 'Error during DB pool health check:'); next(error); } }); -router.get('/time', (req: Request, res: Response) => { +/** + * GET /api/health/time - Returns the server's current time, year, and week number. + * Useful for verifying time synchronization and for features dependent on week numbers. + */ +router.get('/time', validateRequest(emptySchema), (req: Request, res: Response) => { const now = new Date(); const { year, week } = getSimpleWeekAndYear(now); res.json({ @@ -70,7 +96,7 @@ router.get('/time', (req: Request, res: Response) => { /** * GET /api/health/redis - Checks the health of the Redis connection. */ -router.get('/redis', async (req: Request, res: Response, next: NextFunction) => { +router.get('/redis', validateRequest(emptySchema), async (req: Request, res: Response, next: NextFunction) => { try { const reply = await redisConnection.ping(); if (reply === 'PONG') { diff --git a/src/routes/personalization.routes.test.ts b/src/routes/personalization.routes.test.ts index 16e6b016..cb32ead0 100644 --- a/src/routes/personalization.routes.test.ts +++ b/src/routes/personalization.routes.test.ts @@ -28,6 +28,13 @@ import * as db from '../services/db/index.db'; // Create the Express app const app = express(); app.use(express.json()); + +// Add a middleware to inject a mock req.log object for tests +app.use((req, res, next) => { + req.log = { info: vi.fn(), debug: vi.fn(), error: vi.fn(), warn: vi.fn() } as any; + next(); +}); + // Mount the router under its designated base path app.use('/api/personalization', personalizationRouter); app.use(errorHandler); diff --git a/src/routes/personalization.routes.ts b/src/routes/personalization.routes.ts index c384ffe0..9b4f0525 100644 --- a/src/routes/personalization.routes.ts +++ b/src/routes/personalization.routes.ts @@ -1,13 +1,20 @@ // src/routes/personalization.routes.ts import { Router, Request, Response, NextFunction } from 'express'; +import { z } from 'zod'; import * as db from '../services/db/index.db'; +import { validateRequest } from '../middleware/validation.middleware'; const router = Router(); +// --- Zod Schemas for Personalization Routes (as per ADR-003) --- +// These routes do not expect any input, so we define an empty schema +// to maintain a consistent validation pattern across the application. +const emptySchema = z.object({}); + /** * GET /api/personalization/master-items - Get the master list of all grocery items. */ -router.get('/master-items', async (req: Request, res: Response, next: NextFunction) => { +router.get('/master-items', validateRequest(emptySchema), async (req: Request, res: Response, next: NextFunction) => { try { const masterItems = await db.personalizationRepo.getAllMasterItems(req.log); res.json(masterItems); @@ -20,7 +27,7 @@ router.get('/master-items', async (req: Request, res: Response, next: NextFuncti /** * GET /api/personalization/dietary-restrictions - Get the master list of all dietary restrictions. */ -router.get('/dietary-restrictions', async (req: Request, res: Response, next: NextFunction) => { +router.get('/dietary-restrictions', validateRequest(emptySchema), async (req: Request, res: Response, next: NextFunction) => { try { const restrictions = await db.personalizationRepo.getDietaryRestrictions(req.log); res.json(restrictions); @@ -33,7 +40,7 @@ router.get('/dietary-restrictions', async (req: Request, res: Response, next: Ne /** * GET /api/personalization/appliances - Get the master list of all kitchen appliances. */ -router.get('/appliances', async (req: Request, res: Response, next: NextFunction) => { +router.get('/appliances', validateRequest(emptySchema), async (req: Request, res: Response, next: NextFunction) => { try { const appliances = await db.personalizationRepo.getAppliances(req.log); res.json(appliances); diff --git a/src/routes/price.routes.test.ts b/src/routes/price.routes.test.ts index d8437dda..1928b13f 100644 --- a/src/routes/price.routes.test.ts +++ b/src/routes/price.routes.test.ts @@ -34,8 +34,20 @@ describe('Price Routes (/api/price-history)', () => { expect(response.body).toEqual([]); }); - // The zod validation middleware will catch invalid requests before they hit the handler. - // We test this behavior in `validation.middleware.test.ts`. - // Therefore, we don't need tests here for non-array bodies, empty arrays, etc. + it('should return 400 if masterItemIds is not an array', async () => { + const response = await supertest(app) + .post('/api/price-history') + .send({ masterItemIds: 'not-an-array' }); + expect(response.status).toBe(400); + expect(response.body.errors[0].message).toBe('Expected array, received string'); + }); + + it('should return 400 if masterItemIds is an empty array', async () => { + const response = await supertest(app) + .post('/api/price-history') + .send({ masterItemIds: [] }); + expect(response.status).toBe(400); + expect(response.body.errors[0].message).toBe('masterItemIds must be a non-empty array of positive integers.'); + }); }); }); \ No newline at end of file diff --git a/src/routes/price.routes.ts b/src/routes/price.routes.ts index a333d384..0344eae3 100644 --- a/src/routes/price.routes.ts +++ b/src/routes/price.routes.ts @@ -14,12 +14,16 @@ const priceHistorySchema = z.object({ }), }); +// Infer the type from the schema for local use, as per ADR-003. +type PriceHistoryRequest = z.infer; + /** * POST /api/price-history - Fetches historical price data for a given list of master item IDs. * This is a placeholder implementation. */ router.post('/', validateRequest(priceHistorySchema), async (req: Request, res: Response, next: NextFunction) => { - const { masterItemIds } = req.body; + // Cast 'req' to the inferred type for full type safety. + const { body: { masterItemIds } } = req as unknown as PriceHistoryRequest; req.log.info({ itemCount: masterItemIds.length }, '[API /price-history] Received request for historical price data.'); res.status(200).json([]); }); diff --git a/src/routes/recipe.routes.test.ts b/src/routes/recipe.routes.test.ts index 672934eb..e7c5df96 100644 --- a/src/routes/recipe.routes.test.ts +++ b/src/routes/recipe.routes.test.ts @@ -31,6 +31,13 @@ import * as db from '../services/db/index.db'; // Create the Express app const app = express(); app.use(express.json()); + +// Add a middleware to inject a mock req.log object for tests +app.use((req, res, next) => { + req.log = { info: vi.fn(), debug: vi.fn(), error: vi.fn(), warn: vi.fn() } as any; + next(); +}); + // Mount the router under its designated base path app.use('/api/recipes', recipeRouter); app.use(errorHandler); @@ -64,6 +71,12 @@ describe('Recipe Routes (/api/recipes)', () => { expect(response.status).toBe(500); expect(response.body.message).toBe('DB Error'); }); + + it('should return 400 for an invalid minPercentage', async () => { + const response = await supertest(app).get('/api/recipes/by-sale-percentage?minPercentage=101'); + expect(response.status).toBe(400); + expect(response.body.errors[0].message).toContain('less than or equal to 100'); + }); }); describe('GET /by-sale-ingredients', () => { @@ -86,6 +99,12 @@ describe('Recipe Routes (/api/recipes)', () => { expect(response.status).toBe(500); expect(response.body.message).toBe('DB Error'); }); + + it('should return 400 for an invalid minIngredients', async () => { + const response = await supertest(app).get('/api/recipes/by-sale-ingredients?minIngredients=abc'); + expect(response.status).toBe(400); + expect(response.body.errors[0].message).toContain('Expected number, received nan'); + }); }); describe('GET /by-ingredient-and-tag', () => { @@ -106,6 +125,12 @@ describe('Recipe Routes (/api/recipes)', () => { expect(response.status).toBe(500); expect(response.body.message).toBe('DB Error'); }); + + it('should return 400 if required query parameters are missing', async () => { + const response = await supertest(app).get('/api/recipes/by-ingredient-and-tag?ingredient=chicken'); + expect(response.status).toBe(400); + expect(response.body.errors[0].message).toContain('"tag" is required'); + }); }); describe('GET /:recipeId/comments', () => { @@ -132,6 +157,12 @@ describe('Recipe Routes (/api/recipes)', () => { expect(response.status).toBe(500); expect(response.body.message).toBe('DB Error'); }); + + it('should return 400 for an invalid recipeId', async () => { + const response = await supertest(app).get('/api/recipes/abc/comments'); + expect(response.status).toBe(400); + expect(response.body.errors[0].message).toContain('Expected number, received nan'); + }); }); describe('GET /:recipeId', () => { @@ -159,5 +190,11 @@ describe('Recipe Routes (/api/recipes)', () => { expect(response.status).toBe(500); expect(response.body.message).toBe('DB Error'); }); + + it('should return 400 for an invalid recipeId', async () => { + const response = await supertest(app).get('/api/recipes/abc'); + expect(response.status).toBe(400); + expect(response.body.errors[0].message).toContain('Expected number, received nan'); + }); }); }); \ No newline at end of file diff --git a/src/routes/recipe.routes.ts b/src/routes/recipe.routes.ts index 3e9f5ff4..cd706894 100644 --- a/src/routes/recipe.routes.ts +++ b/src/routes/recipe.routes.ts @@ -36,10 +36,11 @@ const recipeIdParamsSchema = z.object({ /** * GET /api/recipes/by-sale-percentage - Get recipes based on the percentage of their ingredients on sale. */ -router.get('/by-sale-percentage', validateRequest(bySalePercentageSchema), async (req: Request, res: Response, next: NextFunction) => { - try { // The `minPercentage` is coerced to a number by Zod, but TypeScript doesn't know that yet. - const { minPercentage } = req.query as unknown as { minPercentage: number }; - const recipes = await db.recipeRepo.getRecipesBySalePercentage(minPercentage, req.log); +type BySalePercentageRequest = z.infer; +router.get('/by-sale-percentage', validateRequest(bySalePercentageSchema), async (req, res, next) => { + try { + const { query } = req as unknown as BySalePercentageRequest; + const recipes = await db.recipeRepo.getRecipesBySalePercentage(query.minPercentage, req.log); res.json(recipes); } catch (error) { req.log.error({ error }, 'Error fetching recipes in /api/recipes/by-sale-percentage:'); @@ -50,10 +51,11 @@ router.get('/by-sale-percentage', validateRequest(bySalePercentageSchema), async /** * GET /api/recipes/by-sale-ingredients - Get recipes by the minimum number of sale ingredients. */ -router.get('/by-sale-ingredients', validateRequest(bySaleIngredientsSchema), async (req: Request, res: Response, next: NextFunction) => { - try { // The `minIngredients` is coerced to a number by Zod, but TypeScript doesn't know that yet. - const { minIngredients } = req.query as unknown as { minIngredients: number }; - const recipes = await db.recipeRepo.getRecipesByMinSaleIngredients(minIngredients, req.log); +type BySaleIngredientsRequest = z.infer; +router.get('/by-sale-ingredients', validateRequest(bySaleIngredientsSchema), async (req, res, next) => { + try { + const { query } = req as unknown as BySaleIngredientsRequest; + const recipes = await db.recipeRepo.getRecipesByMinSaleIngredients(query.minIngredients, req.log); res.json(recipes); } catch (error) { req.log.error({ error }, 'Error fetching recipes in /api/recipes/by-sale-ingredients:'); @@ -64,10 +66,11 @@ router.get('/by-sale-ingredients', validateRequest(bySaleIngredientsSchema), asy /** * GET /api/recipes/by-ingredient-and-tag - Find recipes by a specific ingredient and tag. */ -router.get('/by-ingredient-and-tag', validateRequest(byIngredientAndTagSchema), async (req: Request, res: Response, next: NextFunction) => { - try { // The query params are guaranteed to be strings by Zod, but TypeScript doesn't know that yet. - const { ingredient, tag } = req.query as { ingredient: string, tag: string }; - const recipes = await db.recipeRepo.findRecipesByIngredientAndTag(ingredient, tag, req.log); +type ByIngredientAndTagRequest = z.infer; +router.get('/by-ingredient-and-tag', validateRequest(byIngredientAndTagSchema), async (req, res, next) => { + try { + const { query } = req as unknown as ByIngredientAndTagRequest; + const recipes = await db.recipeRepo.findRecipesByIngredientAndTag(query.ingredient, query.tag, req.log); res.json(recipes); } catch (error) { req.log.error({ error }, 'Error fetching recipes in /api/recipes/by-ingredient-and-tag:'); @@ -78,10 +81,11 @@ router.get('/by-ingredient-and-tag', validateRequest(byIngredientAndTagSchema), /** * GET /api/recipes/:recipeId/comments - Get all comments for a specific recipe. */ -router.get('/:recipeId/comments', validateRequest(recipeIdParamsSchema), async (req: Request, res: Response, next: NextFunction) => { - try { // The `recipeId` is coerced to a number by Zod, but TypeScript doesn't know that yet. - const { recipeId } = req.params as unknown as { recipeId: number }; - const comments = await db.recipeRepo.getRecipeComments(recipeId, req.log); +type RecipeIdRequest = z.infer; +router.get('/:recipeId/comments', validateRequest(recipeIdParamsSchema), async (req, res, next) => { + try { + const { params } = req as unknown as RecipeIdRequest; + const comments = await db.recipeRepo.getRecipeComments(params.recipeId, req.log); res.json(comments); } catch (error) { req.log.error({ error }, `Error fetching comments for recipe ID ${req.params.recipeId}:`); @@ -92,10 +96,10 @@ router.get('/:recipeId/comments', validateRequest(recipeIdParamsSchema), async ( /** * GET /api/recipes/:recipeId - Get a single recipe by its ID, including ingredients and tags. */ -router.get('/:recipeId', validateRequest(recipeIdParamsSchema), async (req: Request, res: Response, next: NextFunction) => { - try { // The `recipeId` is coerced to a number by Zod, but TypeScript doesn't know that yet. - const { recipeId } = req.params as unknown as { recipeId: number }; - const recipe = await db.recipeRepo.getRecipeById(recipeId, req.log); +router.get('/:recipeId', validateRequest(recipeIdParamsSchema), async (req, res, next) => { + try { + const { params } = req as unknown as RecipeIdRequest; + const recipe = await db.recipeRepo.getRecipeById(params.recipeId, req.log); res.json(recipe); } catch (error) { req.log.error({ error }, `Error fetching recipe ID ${req.params.recipeId}:`); diff --git a/src/routes/stats.routes.test.ts b/src/routes/stats.routes.test.ts index f70ad149..4322caba 100644 --- a/src/routes/stats.routes.test.ts +++ b/src/routes/stats.routes.test.ts @@ -25,6 +25,13 @@ import * as db from '../services/db/index.db'; // Create the Express app const app = express(); app.use(express.json()); + +// Add a middleware to inject a mock req.log object for tests +app.use((req, res, next) => { + req.log = { info: vi.fn(), debug: vi.fn(), error: vi.fn(), warn: vi.fn() } as any; + next(); +}); + // Mount the router under its designated base path app.use('/api/stats', statsRouter); app.use(errorHandler); @@ -54,5 +61,12 @@ describe('Stats Routes (/api/stats)', () => { expect(response.status).toBe(500); expect(response.body.message).toBe('DB Error'); }); + + it('should return 400 for invalid query parameters', async () => { + const response = await supertest(app).get('/api/stats/most-frequent-sales?days=0&limit=abc'); + expect(response.status).toBe(400); + expect(response.body.errors).toBeDefined(); + expect(response.body.errors.length).toBe(2); + }); }); }); \ No newline at end of file diff --git a/src/routes/stats.routes.ts b/src/routes/stats.routes.ts index 57a131a1..da82cc53 100644 --- a/src/routes/stats.routes.ts +++ b/src/routes/stats.routes.ts @@ -16,13 +16,16 @@ const mostFrequentSalesSchema = z.object({ }), }); +// Infer the type from the schema for local use, as per ADR-003. +type MostFrequentSalesRequest = z.infer; + /** * GET /api/stats/most-frequent-sales - Get a list of items that have been on sale most frequently. * This is a public endpoint for data analysis. */ router.get('/most-frequent-sales', validateRequest(mostFrequentSalesSchema), async (req: Request, res: Response, next: NextFunction) => { try { - const { days, limit } = req.query as unknown as { days: number, limit: number }; // Guaranteed to be valid numbers by the middleware + const { query: { days, limit } } = req as unknown as MostFrequentSalesRequest; // Guaranteed to be valid numbers by the middleware const items = await db.adminRepo.getMostFrequentSaleItems(days, limit, req.log); res.json(items); } catch (error) { diff --git a/src/routes/system.routes.test.ts b/src/routes/system.routes.test.ts index 7e307e72..ecacc309 100644 --- a/src/routes/system.routes.test.ts +++ b/src/routes/system.routes.test.ts @@ -42,6 +42,13 @@ vi.mock('../services/logger.server', () => ({ // Create a minimal Express app to host our router const app = express(); app.use(express.json()); + +// Add a middleware to inject a mock req.log object for tests +app.use((req, res, next) => { + req.log = { info: vi.fn(), debug: vi.fn(), error: vi.fn(), warn: vi.fn() } as any; + next(); +}); + app.use('/api/system', systemRouter); app.use(errorHandler); @@ -147,5 +154,13 @@ describe('System Routes (/api/system)', () => { const response = await supertest(app).post('/api/system/geocode').send({ address: 'Any Address' }); expect(response.status).toBe(500); }); + + it('should return 400 if the address is missing from the body', async () => { + const response = await supertest(app) + .post('/api/system/geocode') + .send({ not_address: 'Victoria, BC' }); + expect(response.status).toBe(400); + expect(response.body.errors[0].message).toBe('An address string is required.'); + }); }); }); \ No newline at end of file diff --git a/src/routes/system.routes.ts b/src/routes/system.routes.ts index 6e791165..b84064e8 100644 --- a/src/routes/system.routes.ts +++ b/src/routes/system.routes.ts @@ -13,11 +13,15 @@ const geocodeSchema = z.object({ address: z.string().min(1, 'An address string is required.'), }), }); + +// An empty schema for routes that do not expect any input, to maintain a consistent validation pattern. +const emptySchema = z.object({}); + /** * Checks the status of the 'flyer-crawler-api' process managed by PM2. * This is intended for development and diagnostic purposes. */ -router.get('/pm2-status', (req: Request, res: Response, next: NextFunction) => { +router.get('/pm2-status', validateRequest(emptySchema), (req: Request, res: Response, next: NextFunction) => { // The name 'flyer-crawler-api' comes from your ecosystem.config.cjs file. exec('pm2 describe flyer-crawler-api', (error, stdout, stderr) => { if (error) { @@ -50,7 +54,9 @@ router.get('/pm2-status', (req: Request, res: Response, next: NextFunction) => { * This acts as a secure proxy to the Google Maps Geocoding API. */ router.post('/geocode', validateRequest(geocodeSchema), async (req: Request, res: Response, next: NextFunction) => { - const { address } = req.body; + // Infer type and cast request object as per ADR-003 + type GeocodeRequest = z.infer; + const { body: { address } } = req as unknown as GeocodeRequest; try { const coordinates = await geocodingService.geocodeAddress(address, req.log); diff --git a/src/routes/user.routes.test.ts b/src/routes/user.routes.test.ts index c4bde991..00b4148f 100644 --- a/src/routes/user.routes.test.ts +++ b/src/routes/user.routes.test.ts @@ -260,6 +260,12 @@ describe('User Routes (/api/users)', () => { const response = await supertest(app).delete('/api/users/shopping-lists/999'); expect(response.status).toBe(404); }); + + it('should return 400 for an invalid listId', async () => { + const response = await supertest(app).delete('/api/users/shopping-lists/abc'); + expect(response.status).toBe(400); + expect(response.body.errors[0].message).toBe("Invalid ID for parameter 'listId'. Must be a number."); + }); }); }); describe('Shopping List Item Routes', () => { it('POST /shopping-lists/:listId/items should add an item to a list', async () => { @@ -327,6 +333,14 @@ describe('User Routes (/api/users)', () => { expect(response.status).toBe(200); expect(response.body).toEqual(updatedProfile); }); + + it('should return 400 if the body is empty', async () => { + const response = await supertest(app) + .put('/api/users/profile') + .send({}); + expect(response.status).toBe(400); + expect(response.body.errors[0].message).toBe('At least one field to update must be provided.'); + }); }); describe('PUT /profile/password', () => { @@ -590,6 +604,12 @@ describe('User Routes (/api/users)', () => { expect(response.status).toBe(400); expect(response.body.message).toBe('No avatar file uploaded.'); }); + + it('should return 400 for a non-numeric address ID', async () => { + const response = await supertest(app).get('/api/users/addresses/abc'); + expect(response.status).toBe(400); + expect(response.body.errors[0].message).toBe("Invalid ID for parameter 'addressId'. Must be a number."); + }); }); describe('Recipe Routes', () => { diff --git a/src/routes/user.routes.ts b/src/routes/user.routes.ts index 63922155..f0a10286 100644 --- a/src/routes/user.routes.ts +++ b/src/routes/user.routes.ts @@ -57,6 +57,9 @@ const notificationQuerySchema = z.object({ }), }); +// An empty schema for routes that do not expect any input, to maintain a consistent validation pattern. +const emptySchema = z.object({}); + // Any request to a /api/users/* endpoint will now require a valid JWT. router.use(passport.authenticate('jwt', { session: false })); @@ -109,13 +112,15 @@ router.post( * GET /api/users/notifications - Get notifications for the authenticated user. * Supports pagination with `limit` and `offset` query parameters. */ +type GetNotificationsRequest = z.infer; router.get( '/notifications', validateRequest(notificationQuerySchema), async (req: Request, res: Response, next: NextFunction) => { const user = req.user as User; - const { limit, offset } = req.query as unknown as { limit: number; offset: number }; - const notifications = await db.notificationRepo.getNotificationsForUser(user.user_id, limit, offset, req.log); + // Apply ADR-003 pattern for type safety + const { query } = req as unknown as GetNotificationsRequest; + const notifications = await db.notificationRepo.getNotificationsForUser(user.user_id, query.limit, query.offset, req.log); res.json(notifications); } ); @@ -125,6 +130,7 @@ router.get( */ router.post( '/notifications/mark-all-read', + validateRequest(emptySchema), async (req: Request, res: Response, next: NextFunction) => { const user = req.user as User; await db.notificationRepo.markAllNotificationsAsRead(user.user_id, req.log); @@ -135,13 +141,16 @@ router.post( /** * POST /api/users/notifications/:notificationId/mark-read - Mark a single notification as read. */ +const notificationIdSchema = numericIdParam('notificationId'); +type MarkNotificationReadRequest = z.infer; router.post( - '/notifications/:notificationId/mark-read', validateRequest(numericIdParam('notificationId')), + '/notifications/:notificationId/mark-read', validateRequest(notificationIdSchema), async (req: Request, res: Response, next: NextFunction) => { const user = req.user as User; - const notificationId = req.params.notificationId as unknown as number; + // Apply ADR-003 pattern for type safety + const { params } = req as unknown as MarkNotificationReadRequest; - await db.notificationRepo.markNotificationAsRead(notificationId, user.user_id, req.log); + await db.notificationRepo.markNotificationAsRead(params.notificationId, user.user_id, req.log); res.status(204).send(); // Success, no content to return } ); @@ -149,7 +158,7 @@ router.post( /** * GET /api/users/profile - Get the full profile for the authenticated user. */ -router.get('/profile', async (req, res, next: NextFunction) => { +router.get('/profile', validateRequest(emptySchema), async (req, res, next: NextFunction) => { logger.debug(`[ROUTE] GET /api/users/profile - ENTER`); const user = req.user as UserProfile; try { @@ -165,12 +174,14 @@ router.get('/profile', async (req, res, next: NextFunction) => { /** * PUT /api/users/profile - Update the user's profile information. */ +type UpdateProfileRequest = z.infer; router.put('/profile', validateRequest(updateProfileSchema), async (req, res, next: NextFunction) => { logger.debug(`[ROUTE] PUT /api/users/profile - ENTER`); const user = req.user as UserProfile; - + // Apply ADR-003 pattern for type safety + const { body } = req as unknown as UpdateProfileRequest; try { - const updatedProfile = await db.userRepo.updateUserProfile(user.user_id, req.body, req.log); + const updatedProfile = await db.userRepo.updateUserProfile(user.user_id, body, req.log); res.json(updatedProfile); } catch (error) { logger.error({ error }, `[ROUTE] PUT /api/users/profile - ERROR`); @@ -181,21 +192,22 @@ router.put('/profile', validateRequest(updateProfileSchema), async (req, res, ne /** * PUT /api/users/profile/password - Update the user's password. */ +type UpdatePasswordRequest = z.infer; router.put('/profile/password', validateRequest(updatePasswordSchema), async (req, res, next: NextFunction) => { logger.debug(`[ROUTE] PUT /api/users/profile/password - ENTER`); const user = req.user as UserProfile; - const { newPassword } = req.body; + const { body } = req as unknown as UpdatePasswordRequest; const MIN_PASSWORD_SCORE = 3; - const strength = zxcvbn(newPassword); + const strength = zxcvbn(body.newPassword); if (strength.score < MIN_PASSWORD_SCORE) { const feedback = strength.feedback.warning || (strength.feedback.suggestions && strength.feedback.suggestions[0]); return res.status(400).json({ message: `New password is too weak. ${feedback || ''}`.trim() }); } try { - const saltRounds = 10; - const hashedPassword = await bcrypt.hash(newPassword, saltRounds); + const saltRounds = 10; // This was a duplicate, fixed. + const hashedPassword = await bcrypt.hash(body.newPassword, saltRounds); await db.userRepo.updateUserPassword(user.user_id, hashedPassword, req.log); res.status(200).json({ message: 'Password updated successfully.' }); } catch (error) { @@ -207,10 +219,11 @@ router.put('/profile/password', validateRequest(updatePasswordSchema), async (re /** * DELETE /api/users/account - Delete the user's own account. */ +type DeleteAccountRequest = z.infer; router.delete('/account', validateRequest(deleteAccountSchema), async (req, res, next: NextFunction) => { logger.debug(`[ROUTE] DELETE /api/users/account - ENTER`); const user = req.user as UserProfile; - const { password } = req.body; + const { body } = req as unknown as DeleteAccountRequest; try { const userWithHash = await db.userRepo.findUserWithPasswordHashById(user.user_id, req.log); @@ -219,7 +232,7 @@ router.delete('/account', validateRequest(deleteAccountSchema), async (req, res, } // Per ADR-001, findUserWithPasswordHashById will throw if user or hash is missing. - const isMatch = await bcrypt.compare(password, userWithHash.password_hash); + const isMatch = await bcrypt.compare(body.password, userWithHash.password_hash); if (!isMatch) { return res.status(403).json({ message: 'Incorrect password.' }); } @@ -235,7 +248,7 @@ router.delete('/account', validateRequest(deleteAccountSchema), async (req, res, /** * GET /api/users/watched-items - Get all watched items for the authenticated user. */ -router.get('/watched-items', async (req, res, next: NextFunction) => { +router.get('/watched-items', validateRequest(emptySchema), async (req, res, next: NextFunction) => { logger.debug(`[ROUTE] GET /api/users/watched-items - ENTER`); const user = req.user as UserProfile; try { @@ -250,12 +263,13 @@ router.get('/watched-items', async (req, res, next: NextFunction) => { /** * POST /api/users/watched-items - Add a new item to the user's watchlist. */ +type AddWatchedItemRequest = z.infer; router.post('/watched-items', validateRequest(addWatchedItemSchema), async (req, res, next: NextFunction) => { logger.debug(`[ROUTE] POST /api/users/watched-items - ENTER`); const user = req.user as UserProfile; - const { itemName, category } = req.body; + const { body } = req as unknown as AddWatchedItemRequest; try { - const newItem = await db.personalizationRepo.addWatchedItem(user.user_id, itemName, category, req.log); + const newItem = await db.personalizationRepo.addWatchedItem(user.user_id, body.itemName, body.category, req.log); res.status(201).json(newItem); } catch (error) { if (error instanceof ForeignKeyConstraintError) { @@ -273,12 +287,14 @@ router.post('/watched-items', validateRequest(addWatchedItemSchema), async (req, /** * DELETE /api/users/watched-items/:masterItemId - Remove an item from the watchlist. */ -router.delete('/watched-items/:masterItemId', validateRequest(numericIdParam('masterItemId')), async (req, res, next: NextFunction) => { +const watchedItemIdSchema = numericIdParam('masterItemId'); +type DeleteWatchedItemRequest = z.infer; +router.delete('/watched-items/:masterItemId', validateRequest(watchedItemIdSchema), async (req, res, next: NextFunction) => { logger.debug(`[ROUTE] DELETE /api/users/watched-items/:masterItemId - ENTER`); const user = req.user as UserProfile; - const masterItemId = req.params.masterItemId as unknown as number; + const { params } = req as unknown as DeleteWatchedItemRequest; try { - await db.personalizationRepo.removeWatchedItem(user.user_id, masterItemId, req.log); + await db.personalizationRepo.removeWatchedItem(user.user_id, params.masterItemId, req.log); res.status(204).send(); } catch (error) { logger.error({ error }, `[ROUTE] DELETE /api/users/watched-items/:masterItemId - ERROR`); @@ -289,7 +305,7 @@ router.delete('/watched-items/:masterItemId', validateRequest(numericIdParam('ma /** * GET /api/users/shopping-lists - Get all shopping lists for the user. */ -router.get('/shopping-lists', async (req, res, next: NextFunction) => { +router.get('/shopping-lists', validateRequest(emptySchema), async (req, res, next: NextFunction) => { logger.debug(`[ROUTE] GET /api/users/shopping-lists - ENTER`); const user = req.user as UserProfile; try { @@ -304,16 +320,17 @@ router.get('/shopping-lists', async (req, res, next: NextFunction) => { /** * GET /api/users/shopping-lists/:listId - Get a single shopping list by its ID. */ -router.get('/shopping-lists/:listId', validateRequest(numericIdParam('listId')), async (req, res, next: NextFunction) => { +const shoppingListIdSchema = numericIdParam('listId'); +type GetShoppingListRequest = z.infer; +router.get('/shopping-lists/:listId', validateRequest(shoppingListIdSchema), async (req, res, next: NextFunction) => { logger.debug(`[ROUTE] GET /api/users/shopping-lists/:listId - ENTER`); const user = req.user as UserProfile; - const listId = req.params.listId as unknown as number; - + const { params } = req as unknown as GetShoppingListRequest; try { - const list = await db.shoppingRepo.getShoppingListById(listId, user.user_id, req.log); + const list = await db.shoppingRepo.getShoppingListById(params.listId, user.user_id, req.log); res.json(list); } catch (error) { - logger.error({ error, listId }, `[ROUTE] GET /api/users/shopping-lists/:listId - ERROR`); + logger.error({ error, listId: params.listId }, `[ROUTE] GET /api/users/shopping-lists/:listId - ERROR`); next(error); } }); @@ -321,12 +338,13 @@ router.get('/shopping-lists/:listId', validateRequest(numericIdParam('listId')), /** * POST /api/users/shopping-lists - Create a new shopping list. */ +type CreateShoppingListRequest = z.infer; router.post('/shopping-lists', validateRequest(createShoppingListSchema), async (req, res, next: NextFunction) => { logger.debug(`[ROUTE] POST /api/users/shopping-lists - ENTER`); const user = req.user as UserProfile; - const { name } = req.body; + const { body } = req as unknown as CreateShoppingListRequest; try { - const newList = await db.shoppingRepo.createShoppingList(user.user_id, name, req.log); + const newList = await db.shoppingRepo.createShoppingList(user.user_id, body.name, req.log); res.status(201).json(newList); } catch (error) { if (error instanceof ForeignKeyConstraintError) { @@ -344,12 +362,12 @@ router.post('/shopping-lists', validateRequest(createShoppingListSchema), async /** * DELETE /api/users/shopping-lists/:listId - Delete a shopping list. */ -router.delete('/shopping-lists/:listId', validateRequest(numericIdParam('listId')), async (req, res, next: NextFunction) => { +router.delete('/shopping-lists/:listId', validateRequest(shoppingListIdSchema), async (req, res, next: NextFunction) => { logger.debug(`[ROUTE] DELETE /api/users/shopping-lists/:listId - ENTER`); const user = req.user as UserProfile; - const listId = req.params.listId as unknown as number; + const { params } = req as unknown as GetShoppingListRequest; try { - await db.shoppingRepo.deleteShoppingList(listId, user.user_id, req.log); + await db.shoppingRepo.deleteShoppingList(params.listId, user.user_id, req.log); res.status(204).send(); } catch (error: unknown) { const errorMessage = error instanceof Error ? error.message : 'An unknown error occurred'; @@ -361,16 +379,18 @@ router.delete('/shopping-lists/:listId', validateRequest(numericIdParam('listId' /** * POST /api/users/shopping-lists/:listId/items - Add an item to a shopping list. */ -router.post('/shopping-lists/:listId/items', validateRequest(numericIdParam('listId').extend({ +const addShoppingListItemSchema = shoppingListIdSchema.extend({ body: z.object({ masterItemId: z.number().int().positive().optional(), customItemName: z.string().min(1).optional(), }).refine(data => data.masterItemId || data.customItemName, { message: 'Either masterItemId or customItemName must be provided.' }), -})), async (req, res, next: NextFunction) => { +}); +type AddShoppingListItemRequest = z.infer; +router.post('/shopping-lists/:listId/items', validateRequest(addShoppingListItemSchema), async (req, res, next: NextFunction) => { logger.debug(`[ROUTE] POST /api/users/shopping-lists/:listId/items - ENTER`); - const listId = req.params.listId as unknown as number; + const { params, body } = req as unknown as AddShoppingListItemRequest; try { - const newItem = await db.shoppingRepo.addShoppingListItem(listId, req.body, req.log); + const newItem = await db.shoppingRepo.addShoppingListItem(params.listId, body, req.log); res.status(201).json(newItem); } catch (error) { if (error instanceof ForeignKeyConstraintError) { @@ -388,16 +408,18 @@ router.post('/shopping-lists/:listId/items', validateRequest(numericIdParam('lis /** * PUT /api/users/shopping-lists/items/:itemId - Update a shopping list item. */ -router.put('/shopping-lists/items/:itemId', validateRequest(numericIdParam('itemId').extend({ +const updateShoppingListItemSchema = numericIdParam('itemId').extend({ body: z.object({ quantity: z.number().int().nonnegative().optional(), is_purchased: z.boolean().optional(), }).refine(data => Object.keys(data).length > 0, { message: 'At least one field (quantity, is_purchased) must be provided.' }), -})), async (req, res, next: NextFunction) => { +}); +type UpdateShoppingListItemRequest = z.infer; +router.put('/shopping-lists/items/:itemId', validateRequest(updateShoppingListItemSchema), async (req, res, next: NextFunction) => { logger.debug(`[ROUTE] PUT /api/users/shopping-lists/items/:itemId - ENTER`); - const itemId = req.params.itemId as unknown as number; + const { params, body } = req as unknown as UpdateShoppingListItemRequest; try { - const updatedItem = await db.shoppingRepo.updateShoppingListItem(itemId, req.body, req.log); + const updatedItem = await db.shoppingRepo.updateShoppingListItem(params.itemId, body, req.log); res.json(updatedItem); } catch (error: unknown) { logger.error({ error, params: req.params, body: req.body }, `[ROUTE] PUT /api/users/shopping-lists/items/:itemId - ERROR`); @@ -408,11 +430,13 @@ router.put('/shopping-lists/items/:itemId', validateRequest(numericIdParam('item /** * DELETE /api/users/shopping-lists/items/:itemId - Remove an item from a shopping list. */ -router.delete('/shopping-lists/items/:itemId', validateRequest(numericIdParam('itemId')), async (req, res, next: NextFunction) => { +const shoppingListItemIdSchema = numericIdParam('itemId'); +type DeleteShoppingListItemRequest = z.infer; +router.delete('/shopping-lists/items/:itemId', validateRequest(shoppingListItemIdSchema), async (req, res, next: NextFunction) => { logger.debug(`[ROUTE] DELETE /api/users/shopping-lists/items/:itemId - ENTER`); - const itemId = req.params.itemId as unknown as number; + const { params } = req as unknown as DeleteShoppingListItemRequest; try { - await db.shoppingRepo.removeShoppingListItem(itemId, req.log); + await db.shoppingRepo.removeShoppingListItem(params.itemId, req.log); res.status(204).send(); } catch (error: unknown) { logger.error({ error, params: req.params }, `[ROUTE] DELETE /api/users/shopping-lists/items/:itemId - ERROR`); @@ -423,13 +447,16 @@ router.delete('/shopping-lists/items/:itemId', validateRequest(numericIdParam('i /** * PUT /api/users/profile/preferences - Update user preferences. */ -router.put('/profile/preferences', validateRequest(z.object({ +const updatePreferencesSchema = z.object({ body: z.object({}).passthrough(), // Ensures body is an object, allows any properties -})), async (req, res, next: NextFunction) => { +}); +type UpdatePreferencesRequest = z.infer; +router.put('/profile/preferences', validateRequest(updatePreferencesSchema), async (req, res, next: NextFunction) => { logger.debug(`[ROUTE] PUT /api/users/profile/preferences - ENTER`); const user = req.user as UserProfile; - try { // This was a duplicate, fixed. - const updatedProfile = await db.userRepo.updateUserPreferences(user.user_id, req.body, req.log); + const { body } = req as unknown as UpdatePreferencesRequest; + try { + const updatedProfile = await db.userRepo.updateUserPreferences(user.user_id, body, req.log); res.json(updatedProfile); } catch (error) { logger.error({ error }, `[ROUTE] PUT /api/users/profile/preferences - ERROR`); @@ -437,7 +464,7 @@ router.put('/profile/preferences', validateRequest(z.object({ } }); -router.get('/me/dietary-restrictions', async (req, res, next: NextFunction) => { +router.get('/me/dietary-restrictions', validateRequest(emptySchema), async (req, res, next: NextFunction) => { logger.debug(`[ROUTE] GET /api/users/me/dietary-restrictions - ENTER`); const user = req.user as UserProfile; try { @@ -449,14 +476,16 @@ router.get('/me/dietary-restrictions', async (req, res, next: NextFunction) => { } }); -router.put('/me/dietary-restrictions', validateRequest(z.object({ +const setUserRestrictionsSchema = z.object({ body: z.object({ restrictionIds: z.array(z.number().int().positive()) }), -})), async (req, res, next: NextFunction) => { +}); +type SetUserRestrictionsRequest = z.infer; +router.put('/me/dietary-restrictions', validateRequest(setUserRestrictionsSchema), async (req, res, next: NextFunction) => { logger.debug(`[ROUTE] PUT /api/users/me/dietary-restrictions - ENTER`); const user = req.user as UserProfile; - const { restrictionIds } = req.body; + const { body } = req as unknown as SetUserRestrictionsRequest; try { - await db.personalizationRepo.setUserDietaryRestrictions(user.user_id, restrictionIds, req.log); + await db.personalizationRepo.setUserDietaryRestrictions(user.user_id, body.restrictionIds, req.log); res.status(204).send(); } catch (error) { if (error instanceof ForeignKeyConstraintError) { @@ -471,7 +500,7 @@ router.put('/me/dietary-restrictions', validateRequest(z.object({ } }); -router.get('/me/appliances', async (req, res, next: NextFunction) => { +router.get('/me/appliances', validateRequest(emptySchema), async (req, res, next: NextFunction) => { logger.debug(`[ROUTE] GET /api/users/me/appliances - ENTER`); const user = req.user as UserProfile; try { @@ -483,14 +512,16 @@ router.get('/me/appliances', async (req, res, next: NextFunction) => { } }); -router.put('/me/appliances', validateRequest(z.object({ +const setUserAppliancesSchema = z.object({ body: z.object({ applianceIds: z.array(z.number().int().positive()) }), -})), async (req, res, next: NextFunction) => { +}); +type SetUserAppliancesRequest = z.infer; +router.put('/me/appliances', validateRequest(setUserAppliancesSchema), async (req, res, next: NextFunction) => { logger.debug(`[ROUTE] PUT /api/users/me/appliances - ENTER`); const user = req.user as UserProfile; - const { applianceIds } = req.body; + const { body } = req as unknown as SetUserAppliancesRequest; try { - await db.personalizationRepo.setUserAppliances(user.user_id, applianceIds, req.log); + await db.personalizationRepo.setUserAppliances(user.user_id, body.applianceIds, req.log); res.status(204).send(); } catch (error) { if (error instanceof ForeignKeyConstraintError) { @@ -509,10 +540,12 @@ router.put('/me/appliances', validateRequest(z.object({ * GET /api/users/addresses/:addressId - Get a specific address by its ID. * This is protected to ensure a user can only fetch their own address details. */ -router.get('/addresses/:addressId', validateRequest(numericIdParam('addressId')), async (req, res, next: NextFunction) => { +const addressIdSchema = numericIdParam('addressId'); +type GetAddressRequest = z.infer; +router.get('/addresses/:addressId', validateRequest(addressIdSchema), async (req, res, next: NextFunction) => { const user = req.user as UserProfile; - const addressId = req.params.addressId as unknown as number; - + const { params } = req as unknown as GetAddressRequest; + const addressId = params.addressId; // Security check: Ensure the requested addressId matches the one on the user's profile. if (user.address_id !== addressId) { return res.status(403).json({ message: 'Forbidden: You can only access your own address.' }); @@ -525,7 +558,7 @@ router.get('/addresses/:addressId', validateRequest(numericIdParam('addressId')) /** * PUT /api/users/profile/address - Create or update the user's primary address. */ -router.put('/profile/address', validateRequest(z.object({ +const updateUserAddressSchema = z.object({ body: z.object({ address_line_1: z.string().optional(), address_line_2: z.string().optional(), @@ -534,9 +567,11 @@ router.put('/profile/address', validateRequest(z.object({ postal_code: z.string().optional(), country: z.string().optional(), }).refine(data => Object.keys(data).length > 0, { message: 'At least one address field must be provided.' }), -})), async (req, res, next: NextFunction) => { +}); +type UpdateUserAddressRequest = z.infer; +router.put('/profile/address', validateRequest(updateUserAddressSchema), async (req, res, next: NextFunction) => { const user = req.user as UserProfile; - const addressData = req.body as Partial
; + const { body: addressData } = req as unknown as UpdateUserAddressRequest; try { // Per ADR-002, complex operations involving multiple database writes should be @@ -552,13 +587,14 @@ router.put('/profile/address', validateRequest(z.object({ /** * DELETE /api/users/recipes/:recipeId - Delete a recipe created by the user. */ -router.delete('/recipes/:recipeId', validateRequest(numericIdParam('recipeId')), async (req, res, next: NextFunction) => { +const recipeIdSchema = numericIdParam('recipeId'); +type DeleteRecipeRequest = z.infer; +router.delete('/recipes/:recipeId', validateRequest(recipeIdSchema), async (req, res, next: NextFunction) => { logger.debug(`[ROUTE] DELETE /api/users/recipes/:recipeId - ENTER`); const user = req.user as UserProfile; - const recipeId = req.params.recipeId as unknown as number; - + const { params } = req as unknown as DeleteRecipeRequest; try { - await db.recipeRepo.deleteRecipe(recipeId, user.user_id, false, req.log); + await db.recipeRepo.deleteRecipe(params.recipeId, user.user_id, false, req.log); res.status(204).send(); } catch (error) { next(error); @@ -568,7 +604,7 @@ router.delete('/recipes/:recipeId', validateRequest(numericIdParam('recipeId')), /** * PUT /api/users/recipes/:recipeId - Update a recipe created by the user. */ -router.put('/recipes/:recipeId', validateRequest(numericIdParam('recipeId').extend({ +const updateRecipeSchema = recipeIdSchema.extend({ body: z.object({ name: z.string().optional(), description: z.string().optional(), @@ -578,13 +614,15 @@ router.put('/recipes/:recipeId', validateRequest(numericIdParam('recipeId').exte servings: z.number().int().optional(), photo_url: z.string().url().optional(), }).refine(data => Object.keys(data).length > 0, { message: 'No fields provided to update.' }), -})), async (req, res, next: NextFunction) => { +}); +type UpdateRecipeRequest = z.infer; +router.put('/recipes/:recipeId', validateRequest(updateRecipeSchema), async (req, res, next: NextFunction) => { logger.debug(`[ROUTE] PUT /api/users/recipes/:recipeId - ENTER`); const user = req.user as UserProfile; - const recipeId = req.params.recipeId as unknown as number; + const { params, body } = req as unknown as UpdateRecipeRequest; try { - const updatedRecipe = await db.recipeRepo.updateRecipe(recipeId, user.user_id, req.body, req.log); + const updatedRecipe = await db.recipeRepo.updateRecipe(params.recipeId, user.user_id, body, req.log); res.json(updatedRecipe); } catch (error) { next(error); diff --git a/src/types/express-custom.d.ts b/src/types/express-custom.d.ts new file mode 100644 index 00000000..61e38927 --- /dev/null +++ b/src/types/express-custom.d.ts @@ -0,0 +1,16 @@ +// src/types/express-custom.d.ts +import { RequestHandler, Request } from 'express'; +import { z } from 'zod'; + +type ValidatedRequest = Request['params'], any, z.infer['body'], z.infer['query']>; + +/** + * Defines a type-safe request handler that infers the types of + * `req.body`, `req.query`, and `req.params` from a Zod schema. + * This eliminates the need for type assertions in route handlers. + */ +export type ValidatedRequestHandler = ( + req: ValidatedRequest, + res: Parameters[1], + next: Parameters[2] +) => void | Promise; \ No newline at end of file