From 84ebbeb67785e67097545c36fd0d26b4fd412577 Mon Sep 17 00:00:00 2001 From: Torben Sorensen Date: Tue, 9 Dec 2025 20:46:25 -0800 Subject: [PATCH] routes cleanup --- server.ts | 8 +- src/routes/flyer.routes.test.ts | 146 +++++++ src/routes/flyer.routes.ts | 20 + src/routes/personalization.routes.test.ts | 84 ++++ src/routes/public.routes.test.ts | 371 ------------------ src/routes/recipe.routes.test.ts | 134 +++++++ src/routes/recipe.routes.ts | 18 +- src/routes/stats.routes.test.ts | 72 ++++ .../{public.routes.ts => stats.routes.ts} | 10 +- src/services/apiClient.test.ts | 16 +- src/services/apiClient.ts | 22 +- 11 files changed, 513 insertions(+), 388 deletions(-) create mode 100644 src/routes/flyer.routes.test.ts create mode 100644 src/routes/personalization.routes.test.ts delete mode 100644 src/routes/public.routes.test.ts create mode 100644 src/routes/recipe.routes.test.ts create mode 100644 src/routes/stats.routes.test.ts rename src/routes/{public.routes.ts => stats.routes.ts} (75%) diff --git a/server.ts b/server.ts index 3f93844b..593c41ed 100644 --- a/server.ts +++ b/server.ts @@ -10,7 +10,6 @@ import { logger } from './src/services/logger.server'; // Import routers import authRouter from './src/routes/auth.routes'; -import publicRouter from './src/routes/public.routes'; // This seems to be missing from the original file list, but is required. import userRouter from './src/routes/user.routes'; import adminRouter from './src/routes/admin.routes'; import aiRouter from './src/routes/ai.routes'; @@ -18,6 +17,7 @@ import budgetRouter from './src/routes/budget.routes'; import flyerRouter from './src/routes/flyer.routes'; import recipeRouter from './src/routes/recipe.routes'; import personalizationRouter from './src/routes/personalization.routes'; +import statsRouter from './src/routes/stats.routes'; import gamificationRouter from './src/routes/gamification.routes'; import systemRouter from './src/routes/system.routes'; import healthRouter from './src/routes/health.routes'; @@ -136,10 +136,10 @@ app.use('/api/achievements', gamificationRouter); app.use('/api/flyers', flyerRouter); // 8. Public recipe routes. app.use('/api/recipes', recipeRouter); -// 9. Public personalization data routes. +// 9. Public personalization data routes (master items, etc.). app.use('/api/personalization', personalizationRouter); -// 8. Public routes that require no authentication. This should be last among the API routes. -app.use('/api', publicRouter); +// 10. Public statistics routes. +app.use('/api/stats', statsRouter); // --- Error Handling and Server Startup --- diff --git a/src/routes/flyer.routes.test.ts b/src/routes/flyer.routes.test.ts new file mode 100644 index 00000000..86b9dff2 --- /dev/null +++ b/src/routes/flyer.routes.test.ts @@ -0,0 +1,146 @@ +// src/routes/flyer.routes.test.ts +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import supertest from 'supertest'; +import express, { Request, Response, NextFunction } from 'express'; +import flyerRouter from './flyer.routes'; +import { createMockFlyer, createMockFlyerItem } from '../tests/utils/mockFactories'; + +// 1. Mock the Service Layer directly. +vi.mock('../services/db/index.db', () => ({ + flyerRepo: { + getFlyerById: vi.fn(), + getFlyers: vi.fn(), + getFlyerItems: vi.fn(), + getFlyerItemsForFlyers: vi.fn(), + countFlyerItemsForFlyers: vi.fn(), + trackFlyerItemInteraction: vi.fn(), + }, +})); + +// Mock the logger to keep test output clean +vi.mock('../services/logger.server', () => ({ + logger: { + error: vi.fn(), + }, +})); + +// Import the mocked db module to control its functions in tests +import * as db from '../services/db/index.db'; + +// Create the Express app +const app = express(); +app.use(express.json()); +// Mount the router under its designated base path +app.use('/api/flyers', flyerRouter); + +// Add a generic error handler to catch errors passed via next() +app.use((err: Error, req: Request, res: Response, next: NextFunction) => { + res.status(500).json({ message: err.message || 'Internal Server Error' }); +}); + +describe('Flyer Routes (/api/flyers)', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe('GET /', () => { + it('should return a list of flyers on success', async () => { + const mockFlyers = [createMockFlyer({ flyer_id: 1 }), createMockFlyer({ flyer_id: 2 })]; + vi.mocked(db.flyerRepo.getFlyers).mockResolvedValue(mockFlyers); + + const response = await supertest(app).get('/api/flyers'); + + expect(response.status).toBe(200); + expect(response.body).toEqual(mockFlyers); + }); + + it('should pass limit and offset query parameters to the db function', async () => { + vi.mocked(db.flyerRepo.getFlyers).mockResolvedValue([]); + await supertest(app).get('/api/flyers?limit=15&offset=30'); + expect(db.flyerRepo.getFlyers).toHaveBeenCalledWith(15, 30); + }); + }); + + describe('GET /:id', () => { + it('should return a single flyer on success', async () => { + const mockFlyer = createMockFlyer({ flyer_id: 123 }); + vi.mocked(db.flyerRepo.getFlyerById).mockResolvedValue(mockFlyer); + + const response = await supertest(app).get('/api/flyers/123'); + + expect(response.status).toBe(200); + expect(response.body).toEqual(mockFlyer); + expect(db.flyerRepo.getFlyerById).toHaveBeenCalledWith(123); + }); + + it('should return 404 if the flyer is not found', async () => { + vi.mocked(db.flyerRepo.getFlyerById).mockResolvedValue(undefined); + const response = await supertest(app).get('/api/flyers/999'); + expect(response.status).toBe(404); + expect(response.body.message).toContain('not found'); + }); + + it('should return 400 for an invalid flyer ID', async () => { + const response = await supertest(app).get('/api/flyers/abc'); + expect(response.status).toBe(400); + expect(response.body.message).toBe('Invalid flyer ID provided.'); + }); + }); + + describe('GET /:id/items', () => { + it('should return items for a specific flyer', async () => { + const mockFlyerItems = [createMockFlyerItem({ flyer_item_id: 1, flyer_id: 123 })]; + vi.mocked(db.flyerRepo.getFlyerItems).mockResolvedValue(mockFlyerItems); + + const response = await supertest(app).get('/api/flyers/123/items'); + + expect(response.status).toBe(200); + expect(response.body).toEqual(mockFlyerItems); + }); + }); + + describe('POST /items/batch-fetch', () => { + it('should return items for multiple flyers', async () => { + const mockFlyerItems = [createMockFlyerItem({ flyer_item_id: 1, flyer_id: 1 })]; + vi.mocked(db.flyerRepo.getFlyerItemsForFlyers).mockResolvedValue(mockFlyerItems); + + const response = await supertest(app) + .post('/api/flyers/items/batch-fetch') + .send({ flyerIds: [1, 2] }); + + expect(response.status).toBe(200); + expect(response.body).toEqual(mockFlyerItems); + }); + + it('should return 400 if flyerIds is not an array', async () => { + const response = await supertest(app) + .post('/api/flyers/items/batch-fetch') + .send({ flyerIds: 'not-an-array' }); + expect(response.status).toBe(400); + }); + }); + + describe('POST /items/batch-count', () => { + it('should return the count of items for multiple flyers', async () => { + vi.mocked(db.flyerRepo.countFlyerItemsForFlyers).mockResolvedValue(42); + + const response = await supertest(app) + .post('/api/flyers/items/batch-count') + .send({ flyerIds: [1, 2, 3] }); + + expect(response.status).toBe(200); + expect(response.body).toEqual({ count: 42 }); + }); + }); + + describe('POST /items/:itemId/track', () => { + it('should return 202 Accepted and call the tracking function', async () => { + const response = await supertest(app) + .post('/api/flyers/items/99/track') + .send({ type: 'click' }); + + expect(response.status).toBe(202); + expect(db.flyerRepo.trackFlyerItemInteraction).toHaveBeenCalledWith(99, 'click'); + }); + }); +}); \ No newline at end of file diff --git a/src/routes/flyer.routes.ts b/src/routes/flyer.routes.ts index 3ccad508..a64da316 100644 --- a/src/routes/flyer.routes.ts +++ b/src/routes/flyer.routes.ts @@ -20,6 +20,26 @@ router.get('/', async (req, res, next: NextFunction) => { } }); +/** + * GET /api/flyers/:id - Get a single flyer by its ID. + */ +router.get('/:id', async (req, res, next: NextFunction) => { + try { + const flyerId = parseInt(req.params.id, 10); + if (isNaN(flyerId)) { + return res.status(400).json({ message: 'Invalid flyer ID provided.' }); + } + const flyer = await db.flyerRepo.getFlyerById(flyerId); + if (!flyer) { + return res.status(404).json({ message: `Flyer with ID ${flyerId} not found.` }); + } + res.json(flyer); + } catch (error) { + logger.error(`Error fetching flyer ID ${req.params.id}:`, { error }); + next(error); + } +}); + /** * GET /api/flyers/:id/items - Get all items for a specific flyer. */ diff --git a/src/routes/personalization.routes.test.ts b/src/routes/personalization.routes.test.ts new file mode 100644 index 00000000..968885ea --- /dev/null +++ b/src/routes/personalization.routes.test.ts @@ -0,0 +1,84 @@ +// src/routes/personalization.routes.test.ts +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import supertest from 'supertest'; +import express, { Request, Response, NextFunction } from 'express'; +import personalizationRouter from './personalization.routes'; +import { createMockMasterGroceryItem, createMockDietaryRestriction, createMockAppliance } from '../tests/utils/mockFactories'; + +// 1. Mock the Service Layer directly. +vi.mock('../services/db/index.db', () => ({ + personalizationRepo: { + getAllMasterItems: vi.fn(), + getDietaryRestrictions: vi.fn(), + getAppliances: vi.fn(), + }, +})); + +// Mock the logger to keep test output clean +vi.mock('../services/logger.server', () => ({ + logger: { + error: vi.fn(), + }, +})); + +// Import the mocked db module to control its functions in tests +import * as db from '../services/db/index.db'; + +// Create the Express app +const app = express(); +app.use(express.json()); +// Mount the router under its designated base path +app.use('/api/personalization', personalizationRouter); + +// Add a generic error handler to catch errors passed via next() +app.use((err: Error, req: Request, res: Response, next: NextFunction) => { + res.status(500).json({ message: err.message || 'Internal Server Error' }); +}); + +describe('Personalization Routes (/api/personalization)', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe('GET /master-items', () => { + it('should return a list of master items', async () => { + const mockItems = [createMockMasterGroceryItem({ master_grocery_item_id: 1, name: 'Milk' })]; + vi.mocked(db.personalizationRepo.getAllMasterItems).mockResolvedValue(mockItems); + + const response = await supertest(app).get('/api/personalization/master-items'); + + expect(response.status).toBe(200); + expect(response.body).toEqual(mockItems); + }); + + it('should return 500 if the database call fails', async () => { + vi.mocked(db.personalizationRepo.getAllMasterItems).mockRejectedValue(new Error('DB Error')); + const response = await supertest(app).get('/api/personalization/master-items'); + expect(response.status).toBe(500); + }); + }); + + describe('GET /dietary-restrictions', () => { + it('should return a list of all dietary restrictions', async () => { + const mockRestrictions = [createMockDietaryRestriction({ name: 'Gluten-Free' })]; + vi.mocked(db.personalizationRepo.getDietaryRestrictions).mockResolvedValue(mockRestrictions); + + const response = await supertest(app).get('/api/personalization/dietary-restrictions'); + + expect(response.status).toBe(200); + expect(response.body).toEqual(mockRestrictions); + }); + }); + + describe('GET /appliances', () => { + it('should return a list of all appliances', async () => { + const mockAppliances = [createMockAppliance({ name: 'Air Fryer' })]; + vi.mocked(db.personalizationRepo.getAppliances).mockResolvedValue(mockAppliances); + + const response = await supertest(app).get('/api/personalization/appliances'); + + expect(response.status).toBe(200); + expect(response.body).toEqual(mockAppliances); + }); + }); +}); \ No newline at end of file diff --git a/src/routes/public.routes.test.ts b/src/routes/public.routes.test.ts deleted file mode 100644 index dc0976f9..00000000 --- a/src/routes/public.routes.test.ts +++ /dev/null @@ -1,371 +0,0 @@ -// src/routes/public.test.ts -import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import supertest from 'supertest'; -import express from 'express'; -import publicRouter from './public.routes'; // Import the router we want to test -import { createMockFlyer, createMockFlyerItem, createMockMasterGroceryItem, createMockRecipe, createMockRecipeComment, createMockDietaryRestriction, createMockAppliance } from '../tests/utils/mockFactories'; - -// 1. Mock the Service Layer directly. -// This decouples the route tests from the SQL implementation details. -vi.mock('../services/db/index.db', () => ({ - adminRepo: { - getMostFrequentSaleItems: vi.fn(), - }, - flyerRepo: { - getFlyers: vi.fn(), - getFlyerItems: vi.fn(), - getFlyerItemsForFlyers: vi.fn(), - countFlyerItemsForFlyers: vi.fn(), - }, - recipeRepo: { - getRecipesBySalePercentage: vi.fn(), - getRecipesByMinSaleIngredients: vi.fn(), - findRecipesByIngredientAndTag: vi.fn(), - getRecipeComments: vi.fn(), - }, - personalizationRepo: { - getAllMasterItems: vi.fn(), - getDietaryRestrictions: vi.fn(), - getAppliances: vi.fn(), - }, -})); - -// Mock the connection.db module separately as it's a direct dependency of the router. -vi.mock('../services/db/connection.db', () => ({ - checkTablesExist: vi.fn(), - getPoolStatus: vi.fn(), -})); - -// Mock the logger to keep test output clean -vi.mock('../services/logger.server', () => ({ - logger: { - info: vi.fn(), - debug: vi.fn(), - error: vi.fn(), - warn: vi.fn(), - }, -})); - -// 2. Mock node:fs/promises. -// We provide both named exports and a default export to support different import styles. -vi.mock('node:fs/promises', () => { - const mockAccess = vi.fn(); - const mockConstants = { W_OK: 1 }; - return { - access: mockAccess, - constants: mockConstants, - default: { - access: mockAccess, - constants: mockConstants, - }, - }; -}); -import * as fs from 'node:fs/promises'; - -// Import the mocked db module to control its functions in tests -import * as db from '../services/db/index.db'; -import * as dbConnection from '../services/db/connection.db'; - - -// Create the Express app -const app = express(); -app.use(express.json({ strict: false })); -// Mount the router under a base path, similar to how it's done in the main server -app.use('/api', publicRouter); // This was correct, no change needed. Re-evaluating. - -describe('Public Routes (/api)', () => { - beforeEach(() => { - vi.clearAllMocks(); - }); - - describe('GET /health/ping', () => { - it('should respond with 200 and "pong"', async () => { - const response = await supertest(app).get('/api/health/ping'); - - expect(response.status).toBe(200); - expect(response.text).toBe('pong'); - }); - }); - - describe('GET /health/db-schema', () => { - it('should return 200 OK if all tables exist', async () => { - // Mock the service function to return an empty array (no missing tables) - vi.mocked(dbConnection.checkTablesExist).mockResolvedValue([]); - - const response = await supertest(app).get('/api/health/db-schema'); - - expect(response.status).toBe(200); - expect(response.body.success).toBe(true); - }); - - it('should return 500 if tables are missing', async () => { - // Mock the service function to return missing table names - vi.mocked(dbConnection.checkTablesExist).mockResolvedValue(['missing_table']); - - const response = await supertest(app).get('/api/health/db-schema'); - - expect(response.status).toBe(500); - expect(response.body.success).toBe(false); - expect(response.body.message).toContain('Missing tables: missing_table'); - }); - }); - - describe('GET /health/storage', () => { - it('should return 200 OK if storage is writable', async () => { - // Mock fs.access to resolve successfully (simulating success) - vi.mocked(fs.access).mockResolvedValue(undefined); - - const response = await supertest(app).get('/api/health/storage'); - - expect(response.status).toBe(200); - expect(response.body.success).toBe(true); - }); - - it('should return 500 if storage is not accessible', async () => { - // Mock fs.access to reject (simulating permission error) - vi.mocked(fs.access).mockRejectedValue(new Error('Permission denied')); - - const response = await supertest(app).get('/api/health/storage'); - - expect(response.status).toBe(500); - expect(response.body.success).toBe(false); - }); - }); - - describe('GET /health/db-pool', () => { - it('should return 200 OK for a healthy pool', async () => { - // Mock the simple synchronous status function - vi.mocked(dbConnection.getPoolStatus).mockReturnValue({ totalCount: 10, idleCount: 5, waitingCount: 0 }); - - const response = await supertest(app).get('/api/health/db-pool'); - - expect(response.status).toBe(200); - expect(response.body.success).toBe(true); - }); - }); - - describe('GET /time', () => { - beforeEach(() => { - vi.useFakeTimers(); - }); - - afterEach(() => { - vi.useRealTimers(); - }); - - it('should return the current server time, year, and week number', async () => { - // Arrange: Set a specific date to test against - const fakeDate = new Date('2024-03-15T10:30:00.000Z'); // This is a Friday, in the 11th week of 2024 - vi.setSystemTime(fakeDate); - - // Act - const response = await supertest(app).get('/api/time'); - - // Assert - expect(response.status).toBe(200); - expect(response.body.currentTime).toBe('2024-03-15T10:30:00.000Z'); - expect(response.body.year).toBe(2024); - expect(response.body.week).toBe(11); - }); - }); - - describe('GET /flyers', () => { - it('should return a list of flyers on success', async () => { - const mockFlyers = [createMockFlyer({ flyer_id: 1 }), createMockFlyer({ flyer_id: 2 })]; - // Mock the service function - vi.mocked(db.flyerRepo.getFlyers).mockResolvedValue(mockFlyers); - - const response = await supertest(app).get('/api/flyers'); - - expect(response.status).toBe(200); - expect(response.body).toEqual(mockFlyers); - }); - - it('should pass limit and offset query parameters to the db function', async () => { - const mockFlyers = [createMockFlyer({ flyer_id: 1 })]; - vi.mocked(db.flyerRepo.getFlyers).mockResolvedValue(mockFlyers); - - await supertest(app).get('/api/flyers?limit=15&offset=30'); - - expect(db.flyerRepo.getFlyers).toHaveBeenCalledTimes(1); - expect(db.flyerRepo.getFlyers).toHaveBeenCalledWith(15, 30); - }); - - it('should handle database errors gracefully', async () => { - vi.mocked(db.flyerRepo.getFlyers).mockRejectedValue(new Error('DB Error')); - - const response = await supertest(app).get('/api/flyers'); - - // Express default error handler usually returns 500 for unhandled errors - expect(response.status).toBe(500); - }); - }); - - describe('GET /master-items', () => { - it('should return a list of master items', async () => { - const mockItems = [createMockMasterGroceryItem({ master_grocery_item_id: 1, name: 'Milk' })]; - vi.mocked(db.personalizationRepo.getAllMasterItems).mockResolvedValue(mockItems); - - const response = await supertest(app).get('/api/master-items'); - - expect(response.status).toBe(200); - expect(response.body).toEqual(mockItems); - }); - }); - - describe('GET /flyers/:id/items', () => { - it('should return items for a specific flyer', async () => { - const mockFlyerItems = [createMockFlyerItem({ flyer_item_id: 1, flyer_id: 123 })]; - vi.mocked(db.flyerRepo.getFlyerItems).mockResolvedValue(mockFlyerItems); - - const response = await supertest(app).get('/api/flyers/123/items'); - - expect(response.status).toBe(200); - expect(response.body).toEqual(mockFlyerItems); - }); - }); - - describe('POST /flyer-items/batch-fetch', () => { - it('should return items for multiple flyers', async () => { - const mockFlyerItems = [createMockFlyerItem({ flyer_item_id: 1, flyer_id: 1 })]; - vi.mocked(db.flyerRepo.getFlyerItemsForFlyers).mockResolvedValue(mockFlyerItems); - - const response = await supertest(app) - .post('/api/flyer-items/batch-fetch') - .send({ flyerIds: [1, 2] }); - - expect(response.status).toBe(200); - expect(response.body).toEqual(mockFlyerItems); - }); - - it('should return 400 if flyerIds is not an array', async () => { - const response = await supertest(app) - .post('/api/flyer-items/batch-fetch') - .send({ flyerIds: 'not-an-array' }); - expect(response.status).toBe(400); - }); - }); - - describe('GET /recipes/by-sale-percentage', () => { - it('should return recipes based on sale percentage', async () => { - const mockRecipes = [createMockRecipe({ recipe_id: 1, name: 'Pasta' })]; - vi.mocked(db.recipeRepo.getRecipesBySalePercentage).mockResolvedValue(mockRecipes); - - const response = await supertest(app).get('/api/recipes/by-sale-percentage?minPercentage=75'); - - expect(response.status).toBe(200); - expect(response.body).toEqual(mockRecipes); - }); - }); - - describe('POST /flyer-items/batch-count', () => { - it('should return the count of items for multiple flyers', async () => { - vi.mocked(db.flyerRepo.countFlyerItemsForFlyers).mockResolvedValue(42); - - const response = await supertest(app) - .post('/api/flyer-items/batch-count') - .send({ flyerIds: [1, 2, 3] }); - - expect(response.status).toBe(200); - expect(response.body).toEqual({ count: 42 }); - }); - - it('should return 400 if flyerIds is not an array', async () => { - const response = await supertest(app) - .post('/api/flyer-items/batch-count') - .send({ flyerIds: 'invalid-data' }); - - expect(response.status).toBe(400); - expect(response.body.message).toBe('flyerIds must be an array.'); - }); - }); - - describe('GET /recipes/by-sale-ingredients', () => { - it('should return recipes with default minIngredients', async () => { - vi.mocked(db.recipeRepo.getRecipesByMinSaleIngredients).mockResolvedValue([]); - const response = await supertest(app).get('/api/recipes/by-sale-ingredients'); - expect(response.status).toBe(200); - }); - - it('should return 400 for an invalid minIngredients parameter', async () => { - const response = await supertest(app).get('/api/recipes/by-sale-ingredients?minIngredients=abc'); - expect(response.status).toBe(400); - expect(response.body.message).toBe('Query parameter "minIngredients" must be a positive integer.'); - }); - }); - - describe('GET /recipes/by-ingredient-and-tag', () => { - it('should return recipes for a given ingredient and tag', async () => { - const mockRecipes = [createMockRecipe({ recipe_id: 2, name: 'Chicken Tacos' })]; - vi.mocked(db.recipeRepo.findRecipesByIngredientAndTag).mockResolvedValue(mockRecipes); - - const response = await supertest(app).get('/api/recipes/by-ingredient-and-tag?ingredient=chicken&tag=quick'); - - expect(response.status).toBe(200); - expect(response.body).toEqual(mockRecipes); - }); - - it('should return 400 if a query parameter is missing', async () => { - const response = await supertest(app).get('/api/recipes/by-ingredient-and-tag?ingredient=chicken'); - expect(response.status).toBe(400); - expect(response.body.message).toBe('Both "ingredient" and "tag" query parameters are required.'); - }); - }); - - describe('GET /stats/most-frequent-sales', () => { - it('should return most frequent sale items with default parameters', async () => { - vi.mocked(db.adminRepo.getMostFrequentSaleItems).mockResolvedValue([]); - const response = await supertest(app).get('/api/stats/most-frequent-sales'); - expect(response.status).toBe(200); - }); - - it('should use provided query parameters', async () => { - vi.mocked(db.adminRepo.getMostFrequentSaleItems).mockResolvedValue([]); - await supertest(app).get('/api/stats/most-frequent-sales?days=90&limit=5'); - expect(db.adminRepo.getMostFrequentSaleItems).toHaveBeenCalledWith(90, 5); - }); - - it('should return 400 for an invalid "days" parameter', async () => { - const response = await supertest(app).get('/api/stats/most-frequent-sales?days=400'); - expect(response.status).toBe(400); - expect(response.body.message).toBe('Query parameter "days" must be an integer between 1 and 365.'); - }); - }); - - describe('GET /recipes/:recipeId/comments', () => { - it('should return comments for a specific recipe', async () => { - const mockComments = [createMockRecipeComment({ recipe_id: 1, content: 'Great recipe!' })]; // This was a duplicate, fixed. - vi.mocked(db.recipeRepo.getRecipeComments).mockResolvedValue(mockComments); - - const response = await supertest(app).get('/api/recipes/1/comments'); - - expect(response.status).toBe(200); - expect(response.body).toEqual(mockComments); - expect(db.recipeRepo.getRecipeComments).toHaveBeenCalledWith(1); - }); - }); // This was a duplicate, fixed. - - describe('GET /dietary-restrictions', () => { - it('should return a list of all dietary restrictions', async () => { - const mockRestrictions = [createMockDietaryRestriction({ name: 'Gluten-Free' })]; - vi.mocked(db.personalizationRepo.getDietaryRestrictions).mockResolvedValue(mockRestrictions); - - const response = await supertest(app).get('/api/dietary-restrictions'); - - expect(response.status).toBe(200); - expect(response.body).toEqual(mockRestrictions); - }); - }); - - describe('GET /appliances', () => { - it('should return a list of all appliances', async () => { - const mockAppliances = [createMockAppliance({ name: 'Air Fryer' })]; - vi.mocked(db.personalizationRepo.getAppliances).mockResolvedValue(mockAppliances); - - const response = await supertest(app).get('/api/appliances'); - - expect(response.status).toBe(200); - expect(response.body).toEqual(mockAppliances); - }); - }); -}); diff --git a/src/routes/recipe.routes.test.ts b/src/routes/recipe.routes.test.ts new file mode 100644 index 00000000..b119d9ae --- /dev/null +++ b/src/routes/recipe.routes.test.ts @@ -0,0 +1,134 @@ +// src/routes/recipe.routes.test.ts +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import supertest from 'supertest'; +import express, { Request, Response, NextFunction } from 'express'; +import recipeRouter from './recipe.routes'; +import { createMockRecipe, createMockRecipeComment } from '../tests/utils/mockFactories'; + +// 1. Mock the Service Layer directly. +vi.mock('../services/db/index.db', () => ({ + recipeRepo: { + getRecipesBySalePercentage: vi.fn(), + getRecipesByMinSaleIngredients: vi.fn(), + getRecipeById: vi.fn(), + findRecipesByIngredientAndTag: vi.fn(), + getRecipeComments: vi.fn(), + }, +})); + +// Mock the logger to keep test output clean +vi.mock('../services/logger.server', () => ({ + logger: { + error: vi.fn(), + }, +})); + +// Import the mocked db module to control its functions in tests +import * as db from '../services/db/index.db'; + +// Create the Express app +const app = express(); +app.use(express.json()); +// Mount the router under its designated base path +app.use('/api/recipes', recipeRouter); + +// Add a generic error handler to catch errors passed via next() +app.use((err: Error, req: Request, res: Response, next: NextFunction) => { + res.status(500).json({ message: err.message || 'Internal Server Error' }); +}); + +describe('Recipe Routes (/api/recipes)', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe('GET /by-sale-percentage', () => { + it('should return recipes based on sale percentage', async () => { + const mockRecipes = [createMockRecipe({ recipe_id: 1, name: 'Pasta' })]; + vi.mocked(db.recipeRepo.getRecipesBySalePercentage).mockResolvedValue(mockRecipes); + + const response = await supertest(app).get('/api/recipes/by-sale-percentage?minPercentage=75'); + + expect(response.status).toBe(200); + expect(response.body).toEqual(mockRecipes); + expect(db.recipeRepo.getRecipesBySalePercentage).toHaveBeenCalledWith(75); + }); + + it('should return 400 for an invalid minPercentage parameter', async () => { + const response = await supertest(app).get('/api/recipes/by-sale-percentage?minPercentage=abc'); + expect(response.status).toBe(400); + }); + }); + + describe('GET /by-sale-ingredients', () => { + it('should return recipes with default minIngredients', async () => { + vi.mocked(db.recipeRepo.getRecipesByMinSaleIngredients).mockResolvedValue([]); + const response = await supertest(app).get('/api/recipes/by-sale-ingredients'); + expect(response.status).toBe(200); + expect(db.recipeRepo.getRecipesByMinSaleIngredients).toHaveBeenCalledWith(3); + }); + + it('should return 400 for an invalid minIngredients parameter', async () => { + const response = await supertest(app).get('/api/recipes/by-sale-ingredients?minIngredients=abc'); + expect(response.status).toBe(400); + expect(response.body.message).toBe('Query parameter "minIngredients" must be a positive integer.'); + }); + }); + + describe('GET /by-ingredient-and-tag', () => { + it('should return recipes for a given ingredient and tag', async () => { + const mockRecipes = [createMockRecipe({ recipe_id: 2, name: 'Chicken Tacos' })]; + vi.mocked(db.recipeRepo.findRecipesByIngredientAndTag).mockResolvedValue(mockRecipes); + + const response = await supertest(app).get('/api/recipes/by-ingredient-and-tag?ingredient=chicken&tag=quick'); + + expect(response.status).toBe(200); + expect(response.body).toEqual(mockRecipes); + }); + + it('should return 400 if a query parameter is missing', async () => { + const response = await supertest(app).get('/api/recipes/by-ingredient-and-tag?ingredient=chicken'); + expect(response.status).toBe(400); + expect(response.body.message).toBe('Both "ingredient" and "tag" query parameters are required.'); + }); + }); + + describe('GET /:recipeId/comments', () => { + it('should return comments for a specific recipe', async () => { + const mockComments = [createMockRecipeComment({ recipe_id: 1, content: 'Great recipe!' })]; + vi.mocked(db.recipeRepo.getRecipeComments).mockResolvedValue(mockComments); + + const response = await supertest(app).get('/api/recipes/1/comments'); + + expect(response.status).toBe(200); + expect(response.body).toEqual(mockComments); + expect(db.recipeRepo.getRecipeComments).toHaveBeenCalledWith(1); + }); + }); + + describe('GET /:recipeId', () => { + it('should return a single recipe on success', async () => { + const mockRecipe = createMockRecipe({ recipe_id: 456, name: 'Specific Recipe' }); + vi.mocked(db.recipeRepo.getRecipeById).mockResolvedValue(mockRecipe); + + const response = await supertest(app).get('/api/recipes/456'); + + expect(response.status).toBe(200); + expect(response.body).toEqual(mockRecipe); + expect(db.recipeRepo.getRecipeById).toHaveBeenCalledWith(456); + }); + + it('should return 404 if the recipe is not found', async () => { + vi.mocked(db.recipeRepo.getRecipeById).mockResolvedValue(undefined); + const response = await supertest(app).get('/api/recipes/999'); + expect(response.status).toBe(404); + expect(response.body.message).toContain('not found'); + }); + + it('should return 400 for an invalid recipe ID', async () => { + const response = await supertest(app).get('/api/recipes/xyz'); + expect(response.status).toBe(400); + expect(response.body.message).toBe('Invalid recipe ID provided.'); + }); + }); +}); \ No newline at end of file diff --git a/src/routes/recipe.routes.ts b/src/routes/recipe.routes.ts index acdef52f..6c956610 100644 --- a/src/routes/recipe.routes.ts +++ b/src/routes/recipe.routes.ts @@ -57,16 +57,24 @@ router.get('/by-ingredient-and-tag', async (req: Request, res: Response, next: N } }); - -router.get('/:recipeId/comments', async (req, res, next: NextFunction) => { +/** + * GET /api/recipes/:recipeId - Get a single recipe by its ID, including ingredients and tags. + */ +router.get('/:recipeId', async (req: Request, res: Response, next: NextFunction) => { try { const recipeId = parseInt(req.params.recipeId, 10); - const comments = await db.recipeRepo.getRecipeComments(recipeId); - res.json(comments); + if (isNaN(recipeId)) { + return res.status(400).json({ message: 'Invalid recipe ID provided.' }); + } + const recipe = await db.recipeRepo.getRecipeById(recipeId); + if (!recipe) { + return res.status(404).json({ message: `Recipe with ID ${recipeId} not found.` }); + } + res.json(recipe); } catch (error) { + logger.error(`Error fetching recipe ID ${req.params.recipeId}:`, { error }); next(error); } }); - export default router; \ No newline at end of file diff --git a/src/routes/stats.routes.test.ts b/src/routes/stats.routes.test.ts new file mode 100644 index 00000000..dee0316d --- /dev/null +++ b/src/routes/stats.routes.test.ts @@ -0,0 +1,72 @@ +// src/routes/stats.routes.test.ts +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import supertest from 'supertest'; +import express, { Request, Response, NextFunction } from 'express'; +import statsRouter from './stats.routes'; + +// 1. Mock the Service Layer directly. +vi.mock('../services/db/index.db', () => ({ + adminRepo: { + getMostFrequentSaleItems: vi.fn(), + }, +})); + +// Mock the logger to keep test output clean +vi.mock('../services/logger.server', () => ({ + logger: { + error: vi.fn(), + }, +})); + +// Import the mocked db module to control its functions in tests +import * as db from '../services/db/index.db'; + +// Create the Express app +const app = express(); +app.use(express.json()); +// Mount the router under its designated base path +app.use('/api/stats', statsRouter); + +// Add a generic error handler to catch errors passed via next() +app.use((err: Error, req: Request, res: Response, next: NextFunction) => { + res.status(500).json({ message: err.message || 'Internal Server Error' }); +}); + +describe('Stats Routes (/api/stats)', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe('GET /most-frequent-sales', () => { + it('should return most frequent sale items with default parameters', async () => { + vi.mocked(db.adminRepo.getMostFrequentSaleItems).mockResolvedValue([]); + const response = await supertest(app).get('/api/stats/most-frequent-sales'); + expect(response.status).toBe(200); + expect(db.adminRepo.getMostFrequentSaleItems).toHaveBeenCalledWith(30, 10); + }); + + it('should use provided query parameters', async () => { + vi.mocked(db.adminRepo.getMostFrequentSaleItems).mockResolvedValue([]); + await supertest(app).get('/api/stats/most-frequent-sales?days=90&limit=5'); + expect(db.adminRepo.getMostFrequentSaleItems).toHaveBeenCalledWith(90, 5); + }); + + it('should return 400 for an invalid "days" parameter', async () => { + const response = await supertest(app).get('/api/stats/most-frequent-sales?days=400'); + expect(response.status).toBe(400); + expect(response.body.message).toBe('Query parameter "days" must be an integer between 1 and 365.'); + }); + + it('should return 400 for an invalid "limit" parameter', async () => { + const response = await supertest(app).get('/api/stats/most-frequent-sales?limit=100'); + expect(response.status).toBe(400); + expect(response.body.message).toBe('Query parameter "limit" must be an integer between 1 and 50.'); + }); + + it('should return 500 if the database call fails', async () => { + vi.mocked(db.adminRepo.getMostFrequentSaleItems).mockRejectedValue(new Error('DB Error')); + const response = await supertest(app).get('/api/stats/most-frequent-sales'); + expect(response.status).toBe(500); + }); + }); +}); \ No newline at end of file diff --git a/src/routes/public.routes.ts b/src/routes/stats.routes.ts similarity index 75% rename from src/routes/public.routes.ts rename to src/routes/stats.routes.ts index 24f95918..e0ec70ff 100644 --- a/src/routes/public.routes.ts +++ b/src/routes/stats.routes.ts @@ -1,13 +1,15 @@ -// src/routes/public.routes.ts +// src/routes/stats.routes.ts import { Router, Request, Response, NextFunction } from 'express'; import * as db from '../services/db/index.db'; import { logger } from '../services/logger.server'; const router = Router(); -// --- Public Data Routes --- - -router.get('/stats/most-frequent-sales', async (req, res, next: NextFunction) => { +/** + * 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', async (req: Request, res: Response, next: NextFunction) => { try { const daysStr = req.query.days as string || '30'; const limitStr = req.query.limit as string || '10'; diff --git a/src/services/apiClient.test.ts b/src/services/apiClient.test.ts index 5cec77df..2f79d289 100644 --- a/src/services/apiClient.test.ts +++ b/src/services/apiClient.test.ts @@ -383,6 +383,12 @@ describe('API Client', () => { expect(capturedUrl?.pathname).toBe(`/api/recipes/${recipeId}/comments`); }); + it('getRecipeById should call the correct public endpoint', async () => { + const recipeId = 789; + await apiClient.getRecipeById(recipeId); + expect(capturedUrl?.pathname).toBe(`/api/recipes/${recipeId}`); + }); + it('addRecipeComment should send a POST request with content and optional parentId', async () => { const recipeId = 456; const commentData = { content: 'This is a reply', parentCommentId: 789 }; @@ -540,17 +546,23 @@ describe('API Client', () => { expect(capturedUrl?.pathname).toBe(`/api/flyers/${flyerId}/items`); }); + it('fetchFlyerById should call the correct public endpoint for a specific flyer', async () => { + const flyerId = 456; + await apiClient.fetchFlyerById(flyerId); + expect(capturedUrl?.pathname).toBe(`/api/flyers/${flyerId}`); + }); + it('fetchFlyerItemsForFlyers should send a POST request with flyer IDs', async () => { const flyerIds = [1, 2, 3]; await apiClient.fetchFlyerItemsForFlyers(flyerIds); - expect(capturedUrl?.pathname).toBe('/api/flyer-items/batch-fetch'); + expect(capturedUrl?.pathname).toBe('/api/flyers/items/batch-fetch'); expect(capturedBody).toEqual({ flyerIds }); }); it('countFlyerItemsForFlyers should send a POST request with flyer IDs', async () => { const flyerIds = [1, 2, 3]; await apiClient.countFlyerItemsForFlyers(flyerIds); - expect(capturedUrl?.pathname).toBe('/api/flyer-items/batch-count'); + expect(capturedUrl?.pathname).toBe('/api/flyers/items/batch-count'); expect(capturedBody).toEqual({ flyerIds }); }); diff --git a/src/services/apiClient.ts b/src/services/apiClient.ts index d5b62a97..a6c99d89 100644 --- a/src/services/apiClient.ts +++ b/src/services/apiClient.ts @@ -198,6 +198,15 @@ export const fetchFlyers = async (): Promise => { return fetch(`${API_BASE_URL}/flyers`); }; +/** + * Fetches a single flyer by its ID. + * @param flyerId The ID of the flyer to fetch. + * @returns A promise that resolves to the API response. + */ +export const fetchFlyerById = async (flyerId: number): Promise => { + return fetch(`${API_BASE_URL}/flyers/${flyerId}`); +}; + /** * Fetches all master grocery items from the backend. * @returns A promise that resolves to an array of MasterGroceryItem objects. @@ -227,7 +236,7 @@ export const fetchFlyerItemsForFlyers = async (flyerIds: number[]): Promise => return fetch(`${API_BASE_URL}/recipes/${recipeId}/comments`); // This was a duplicate, fixed. }; +/** + * Fetches a single recipe by its ID. + * @param recipeId The ID of the recipe to fetch. + * @returns A promise that resolves to the API response. + */ +export const getRecipeById = async (recipeId: number): Promise => { + return fetch(`${API_BASE_URL}/recipes/${recipeId}`); +}; + export const addRecipeComment = async (recipeId: number, content: string, parentCommentId?: number, tokenOverride?: string): Promise => { return apiFetch(`/recipes/${recipeId}/comments`, { method: 'POST',