From 2a5cc5bb51ee0b05e8197e3e99b64a7e947bd9f9 Mon Sep 17 00:00:00 2001 From: Torben Sorensen Date: Mon, 12 Jan 2026 08:10:37 -0800 Subject: [PATCH] unit test repairs --- .claude/settings.local.json | 5 +- docs/BARE-METAL-SETUP.md | 344 +++++++--- src/middleware/multer.middleware.test.ts | 4 +- src/middleware/validation.middleware.ts | 17 +- src/routes/admin.content.routes.test.ts | 50 +- src/routes/admin.jobs.routes.test.ts | 79 ++- src/routes/admin.monitoring.routes.test.ts | 4 +- src/routes/admin.routes.test.ts | 30 +- src/routes/admin.stats.routes.test.ts | 32 +- src/routes/admin.system.routes.test.ts | 32 +- src/routes/admin.users.routes.test.ts | 31 +- src/routes/ai.routes.test.ts | 66 +- src/routes/auth.routes.test.ts | 2 +- src/routes/budget.routes.test.ts | 2 +- src/routes/deals.routes.test.ts | 2 +- src/routes/gamification.routes.test.ts | 2 +- src/routes/inventory.routes.test.ts | 3 +- src/routes/inventory.routes.ts | 624 +++++++++--------- src/routes/price.routes.test.ts | 2 +- src/routes/reactions.routes.test.ts | 2 +- src/routes/receipt.routes.test.ts | 94 +-- src/routes/recipe.routes.test.ts | 2 +- src/routes/upc.routes.test.ts | 6 +- src/routes/user.routes.test.ts | 2 +- src/services/aiService.server.test.ts | 20 +- src/services/apiClient.test.ts | 1 + src/services/barcodeService.server.test.ts | 4 + src/services/db/expiry.db.test.ts | 6 +- src/services/db/index.db.test.ts | 18 +- src/services/db/receipt.db.test.ts | 10 +- .../flyerProcessingService.server.test.ts | 18 +- src/services/monitoringService.server.test.ts | 47 +- src/services/queueService.server.test.ts | 48 +- src/services/queueService.test.ts | 19 +- src/services/queues.server.test.ts | 46 +- src/tests/mocks/zxing-wasm-reader.mock.ts | 11 + src/tests/setup/tests-setup-unit.ts | 190 ++++-- vite.config.ts | 20 + 38 files changed, 1291 insertions(+), 604 deletions(-) create mode 100644 src/tests/mocks/zxing-wasm-reader.mock.ts diff --git a/.claude/settings.local.json b/.claude/settings.local.json index 2d97b508..971d9f23 100644 --- a/.claude/settings.local.json +++ b/.claude/settings.local.json @@ -88,7 +88,10 @@ "Bash(find:*)", "Bash(\"/c/Users/games3/.local/bin/uvx.exe\" markitdown-mcp --help)", "Bash(git stash:*)", - "Bash(ping:*)" + "Bash(ping:*)", + "Bash(tee:*)", + "Bash(timeout 1800 podman exec flyer-crawler-dev npm run test:unit:*)", + "mcp__filesystem__edit_file" ] } } diff --git a/docs/BARE-METAL-SETUP.md b/docs/BARE-METAL-SETUP.md index a72dfd0a..e4c4e2cb 100644 --- a/docs/BARE-METAL-SETUP.md +++ b/docs/BARE-METAL-SETUP.md @@ -71,21 +71,6 @@ GRANT ALL PRIVILEGES ON DATABASE flyer_crawler TO flyer_crawler; \q ``` -### Create Bugsink Database (for error tracking) - -```bash -sudo -u postgres psql -``` - -```sql --- Create dedicated Bugsink user and database -CREATE USER bugsink WITH PASSWORD 'BUGSINK_SECURE_PASSWORD'; -CREATE DATABASE bugsink OWNER bugsink; -GRANT ALL PRIVILEGES ON DATABASE bugsink TO bugsink; - -\q -``` - ### Configure PostgreSQL for Remote Access (if needed) Edit `/etc/postgresql/14/main/postgresql.conf`: @@ -343,115 +328,324 @@ sudo systemctl enable nginx ## Bugsink Error Tracking -Bugsink is a lightweight, self-hosted Sentry-compatible error tracking system. See [ADR-015](adr/0015-application-performance-monitoring-and-error-tracking.md) for architecture details. +Bugsink is a lightweight, self-hosted Sentry-compatible error tracking system. This guide follows the [official Bugsink single-server production setup](https://www.bugsink.com/docs/single-server-production/). -### Install Bugsink +See [ADR-015](adr/0015-application-performance-monitoring-and-error-tracking.md) for architecture details. + +### Step 1: Create Bugsink User + +Create a dedicated non-root user for Bugsink: ```bash -# Create virtual environment -sudo mkdir -p /opt/bugsink -sudo python3 -m venv /opt/bugsink/venv - -# Activate and install -source /opt/bugsink/venv/bin/activate -pip install bugsink - -# Create wrapper scripts -sudo tee /opt/bugsink/bin/bugsink-manage << 'EOF' -#!/bin/bash -source /opt/bugsink/venv/bin/activate -exec python -m bugsink.manage "$@" -EOF - -sudo tee /opt/bugsink/bin/bugsink-runserver << 'EOF' -#!/bin/bash -source /opt/bugsink/venv/bin/activate -exec python -m bugsink.runserver "$@" -EOF - -sudo chmod +x /opt/bugsink/bin/bugsink-manage /opt/bugsink/bin/bugsink-runserver +sudo adduser bugsink --disabled-password --gecos "" ``` -### Configure Bugsink +### Step 2: Set Up Virtual Environment and Install Bugsink -Create `/etc/bugsink/environment`: +Switch to the bugsink user: ```bash -sudo mkdir -p /etc/bugsink -sudo nano /etc/bugsink/environment +sudo su - bugsink ``` +Create the virtual environment: + ```bash -SECRET_KEY=YOUR_RANDOM_50_CHAR_SECRET_KEY -DATABASE_URL=postgresql://bugsink:BUGSINK_SECURE_PASSWORD@localhost:5432/bugsink -BASE_URL=http://localhost:8000 -PORT=8000 +python3 -m venv venv ``` +Activate the virtual environment: + ```bash -sudo chmod 600 /etc/bugsink/environment +source venv/bin/activate ``` -### Initialize Bugsink Database +You should see `(venv)` at the beginning of your prompt. Now install Bugsink: ```bash -source /etc/bugsink/environment -/opt/bugsink/bin/bugsink-manage migrate -/opt/bugsink/bin/bugsink-manage migrate --database=snappea +pip install bugsink --upgrade +bugsink-show-version ``` -### Create Bugsink Admin User +You should see output like `bugsink 2.x.x`. + +### Step 3: Create Configuration File + +Generate the configuration file. Replace `bugsink.yourdomain.com` with your actual hostname: ```bash -/opt/bugsink/bin/bugsink-manage createsuperuser +bugsink-create-conf --template=singleserver --host=bugsink.yourdomain.com ``` -### Create Systemd Service +This creates `bugsink_conf.py` in `/home/bugsink/`. Edit it to customize settings: -Create `/etc/systemd/system/bugsink.service`: +```bash +nano bugsink_conf.py +``` + +**Key settings to review:** + +| Setting | Description | +| ------------------- | ------------------------------------------------------------------------------- | +| `BASE_URL` | The URL where Bugsink will be accessed (e.g., `https://bugsink.yourdomain.com`) | +| `SITE_TITLE` | Display name for your Bugsink instance | +| `SECRET_KEY` | Auto-generated, but verify it exists | +| `TIME_ZONE` | Your timezone (e.g., `America/New_York`) | +| `USER_REGISTRATION` | Set to `"closed"` to disable public signup | +| `SINGLE_USER` | Set to `True` if only one user will use this instance | + +### Step 4: Initialize Database + +Bugsink uses SQLite by default, which is recommended for single-server setups. Run the database migrations: + +```bash +bugsink-manage migrate +bugsink-manage migrate snappea --database=snappea +``` + +Verify the database files were created: + +```bash +ls *.sqlite3 +``` + +You should see `db.sqlite3` and `snappea.sqlite3`. + +### Step 5: Create Admin User + +Create the superuser account. Using your email as the username is recommended: + +```bash +bugsink-manage createsuperuser +``` + +**Important:** Save these credentials - you'll need them to log into the Bugsink web UI. + +### Step 6: Verify Configuration + +Run Django's deployment checks: + +```bash +bugsink-manage check_migrations +bugsink-manage check --deploy --fail-level WARNING +``` + +Exit back to root for the next steps: + +```bash +exit +``` + +### Step 7: Create Gunicorn Service + +Create `/etc/systemd/system/gunicorn-bugsink.service`: + +```bash +sudo nano /etc/systemd/system/gunicorn-bugsink.service +``` + +Add the following content: ```ini [Unit] -Description=Bugsink Error Tracking -After=network.target postgresql.service +Description=Gunicorn daemon for Bugsink +After=network.target [Service] -Type=simple -User=www-data -Group=www-data -EnvironmentFile=/etc/bugsink/environment -ExecStart=/opt/bugsink/bin/bugsink-runserver 0.0.0.0:8000 Restart=always -RestartSec=5 +Type=notify +User=bugsink +Group=bugsink + +Environment="PYTHONUNBUFFERED=1" +WorkingDirectory=/home/bugsink +ExecStart=/home/bugsink/venv/bin/gunicorn \ + --bind="127.0.0.1:8000" \ + --workers=4 \ + --timeout=6 \ + --access-logfile - \ + --max-requests=1000 \ + --max-requests-jitter=100 \ + bugsink.wsgi +ExecReload=/bin/kill -s HUP $MAINPID +KillMode=mixed +TimeoutStopSec=5 [Install] WantedBy=multi-user.target ``` +Enable and start the service: + ```bash sudo systemctl daemon-reload -sudo systemctl enable bugsink -sudo systemctl start bugsink +sudo systemctl enable --now gunicorn-bugsink.service +sudo systemctl status gunicorn-bugsink.service ``` -### Create Bugsink Projects and Get DSNs +Test that Gunicorn is responding (replace hostname): -1. Access Bugsink UI at `http://localhost:8000` -2. Log in with admin credentials -3. Create projects: +```bash +curl http://localhost:8000/accounts/login/ --header "Host: bugsink.yourdomain.com" +``` + +You should see HTML output containing a login form. + +### Step 8: Create Snappea Background Worker Service + +Snappea is Bugsink's background task processor. Create `/etc/systemd/system/snappea.service`: + +```bash +sudo nano /etc/systemd/system/snappea.service +``` + +Add the following content: + +```ini +[Unit] +Description=Snappea daemon for Bugsink background tasks +After=network.target + +[Service] +Restart=always +User=bugsink +Group=bugsink + +Environment="PYTHONUNBUFFERED=1" +WorkingDirectory=/home/bugsink +ExecStart=/home/bugsink/venv/bin/bugsink-runsnappea +KillMode=mixed +TimeoutStopSec=5 +RuntimeMaxSec=1d + +[Install] +WantedBy=multi-user.target +``` + +Enable and start the service: + +```bash +sudo systemctl daemon-reload +sudo systemctl enable --now snappea.service +sudo systemctl status snappea.service +``` + +Verify snappea is working: + +```bash +sudo su - bugsink +source venv/bin/activate +bugsink-manage checksnappea +exit +``` + +### Step 9: Configure NGINX for Bugsink + +Create `/etc/nginx/sites-available/bugsink`: + +```bash +sudo nano /etc/nginx/sites-available/bugsink +``` + +Add the following (replace `bugsink.yourdomain.com` with your hostname): + +```nginx +server { + server_name bugsink.yourdomain.com; + listen 80; + + client_max_body_size 20M; + + access_log /var/log/nginx/bugsink.access.log; + error_log /var/log/nginx/bugsink.error.log; + + location / { + proxy_pass http://127.0.0.1:8000; + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-Proto $scheme; + } +} +``` + +Enable the site: + +```bash +sudo ln -s /etc/nginx/sites-available/bugsink /etc/nginx/sites-enabled/ +sudo nginx -t +sudo systemctl reload nginx +``` + +### Step 10: Configure SSL with Certbot (Recommended) + +```bash +sudo certbot --nginx -d bugsink.yourdomain.com +``` + +After SSL is configured, update the NGINX config to add security headers. Edit `/etc/nginx/sites-available/bugsink` and add to the `location /` block: + +```nginx +add_header Strict-Transport-Security "max-age=31536000; preload" always; +``` + +Reload NGINX: + +```bash +sudo nginx -t +sudo systemctl reload nginx +``` + +### Step 11: Create Projects and Get DSNs + +1. Access Bugsink UI at `https://bugsink.yourdomain.com` +2. Log in with the admin credentials you created +3. Create a new team (or use the default) +4. Create projects: - **flyer-crawler-backend** (Platform: Node.js) - - **flyer-crawler-frontend** (Platform: React) -4. Copy the DSNs from each project's settings -5. Update `/etc/flyer-crawler/environment` with the DSNs + - **flyer-crawler-frontend** (Platform: JavaScript/React) +5. For each project, go to Settings → Client Keys (DSN) +6. Copy the DSN URLs -### Test Error Tracking +### Step 12: Configure Application to Use Bugsink + +Update `/etc/flyer-crawler/environment` with the DSNs from step 11: + +```bash +# Sentry/Bugsink Error Tracking +SENTRY_DSN=https://YOUR_BACKEND_KEY@bugsink.yourdomain.com/1 +VITE_SENTRY_DSN=https://YOUR_FRONTEND_KEY@bugsink.yourdomain.com/2 +SENTRY_ENVIRONMENT=production +VITE_SENTRY_ENVIRONMENT=production +SENTRY_ENABLED=true +VITE_SENTRY_ENABLED=true +``` + +Restart the application to pick up the new settings: + +```bash +pm2 restart all +``` + +### Step 13: Test Error Tracking ```bash cd /opt/flyer-crawler npx tsx scripts/test-bugsink.ts ``` -Check Bugsink UI for test events. +Check the Bugsink UI - you should see a test event appear. + +### Bugsink Maintenance Commands + +| Task | Command | +| ----------------------- | ------------------------------------------------------------------------------------------------------------------------------------------- | +| View Gunicorn status | `sudo systemctl status gunicorn-bugsink` | +| View Snappea status | `sudo systemctl status snappea` | +| View Gunicorn logs | `sudo journalctl -u gunicorn-bugsink -f` | +| View Snappea logs | `sudo journalctl -u snappea -f` | +| Restart Bugsink | `sudo systemctl restart gunicorn-bugsink snappea` | +| Run management commands | `sudo su - bugsink` then `source venv/bin/activate && bugsink-manage ` | +| Upgrade Bugsink | `sudo su - bugsink && source venv/bin/activate && pip install bugsink --upgrade && exit && sudo systemctl restart gunicorn-bugsink snappea` | --- diff --git a/src/middleware/multer.middleware.test.ts b/src/middleware/multer.middleware.test.ts index 3e4385b4..f8dcdc75 100644 --- a/src/middleware/multer.middleware.test.ts +++ b/src/middleware/multer.middleware.test.ts @@ -83,8 +83,8 @@ describe('Multer Middleware Directory Creation', () => { await import('./multer.middleware'); // Assert - // It should try to create both the flyer storage and avatar storage paths - expect(mocks.mkdir).toHaveBeenCalledTimes(2); + // It should try to create the flyer, avatar, and receipt storage paths + expect(mocks.mkdir).toHaveBeenCalledTimes(3); expect(mocks.mkdir).toHaveBeenCalledWith(expect.any(String), { recursive: true }); expect(mocks.logger.info).toHaveBeenCalledWith('Ensured multer storage directories exist.'); expect(mocks.logger.error).not.toHaveBeenCalled(); diff --git a/src/middleware/validation.middleware.ts b/src/middleware/validation.middleware.ts index 70cdd460..f5b43d62 100644 --- a/src/middleware/validation.middleware.ts +++ b/src/middleware/validation.middleware.ts @@ -23,14 +23,21 @@ export const validateRequest = }); // On success, merge the parsed (and coerced) data back into the request objects. - // We don't reassign `req.params`, `req.query`, or `req.body` directly, as they - // might be read-only getters in some environments (like during supertest tests). - // Instead, we clear the existing object and merge the new properties. + // For req.params, we can delete existing keys and assign new ones. Object.keys(req.params).forEach((key) => delete (req.params as ParamsDictionary)[key]); Object.assign(req.params, params); - Object.keys(req.query).forEach((key) => delete (req.query as Query)[key]); - Object.assign(req.query, query); + // For req.query in Express 5, the query object is lazily evaluated from the URL + // and cannot be mutated directly. We use Object.defineProperty to replace + // the getter with our validated/transformed query object. + Object.defineProperty(req, 'query', { + value: query as Query, + writable: true, + configurable: true, + enumerable: true, + }); + + // For body, direct reassignment works. req.body = body; return next(); diff --git a/src/routes/admin.content.routes.test.ts b/src/routes/admin.content.routes.test.ts index 7b2da5c8..1cee6970 100644 --- a/src/routes/admin.content.routes.test.ts +++ b/src/routes/admin.content.routes.test.ts @@ -32,7 +32,7 @@ vi.mock('../lib/queue', () => ({ cleanupQueue: {}, })); -const { mockedDb } = vi.hoisted(() => { +const { mockedDb, mockedBrandService } = vi.hoisted(() => { return { mockedDb: { adminRepo: { @@ -59,6 +59,9 @@ const { mockedDb } = vi.hoisted(() => { deleteUserById: vi.fn(), }, }, + mockedBrandService: { + updateBrandLogo: vi.fn(), + }, }; }); @@ -89,6 +92,26 @@ vi.mock('node:fs/promises', () => ({ vi.mock('../services/backgroundJobService'); vi.mock('../services/geocodingService.server'); vi.mock('../services/queueService.server'); +vi.mock('../services/queues.server'); +vi.mock('../services/workers.server'); +vi.mock('../services/monitoringService.server'); +vi.mock('../services/cacheService.server'); +vi.mock('../services/userService'); +vi.mock('../services/brandService', () => ({ + brandService: mockedBrandService, +})); +vi.mock('../services/receiptService.server'); +vi.mock('../services/aiService.server'); +vi.mock('../config/env', () => ({ + config: { + database: { host: 'localhost', port: 5432, user: 'test', password: 'test', name: 'test' }, + redis: { url: 'redis://localhost:6379' }, + auth: { jwtSecret: 'test-secret' }, + server: { port: 3000, host: 'localhost' }, + }, + isAiConfigured: vi.fn().mockReturnValue(false), + parseConfig: vi.fn(), +})); vi.mock('@bull-board/api'); // Keep this mock for the API part vi.mock('@bull-board/api/bullMQAdapter'); // Keep this mock for the adapter @@ -103,13 +126,17 @@ vi.mock('@bull-board/express', () => ({ })); // Mock the logger -vi.mock('../services/logger.server', async () => ({ - // Use async import to avoid hoisting issues with mockLogger - logger: (await import('../tests/utils/mockLogger')).mockLogger, -})); +vi.mock('../services/logger.server', async () => { + const { mockLogger, createMockLogger } = await import('../tests/utils/mockLogger'); + return { + logger: mockLogger, + createScopedLogger: vi.fn(() => createMockLogger()), + }; +}); // Mock the passport middleware -vi.mock('./passport.routes', () => ({ +// Note: admin.routes.ts imports from '../config/passport', so we mock that path +vi.mock('../config/passport', () => ({ default: { authenticate: vi.fn(() => (req: Request, res: Response, next: NextFunction) => { if (!req.user) return res.status(401).json({ message: 'Unauthorized' }); @@ -314,22 +341,23 @@ describe('Admin Content Management Routes (/api/admin)', () => { it('POST /brands/:id/logo should upload a logo and update the brand', async () => { const brandId = 55; - vi.mocked(mockedDb.adminRepo.updateBrandLogo).mockResolvedValue(undefined); + const mockLogoUrl = '/flyer-images/brand-logos/test-logo.png'; + vi.mocked(mockedBrandService.updateBrandLogo).mockResolvedValue(mockLogoUrl); const response = await supertest(app) .post(`/api/admin/brands/${brandId}/logo`) .attach('logoImage', Buffer.from('dummy-logo-content'), 'test-logo.png'); expect(response.status).toBe(200); expect(response.body.data.message).toBe('Brand logo updated successfully.'); - expect(vi.mocked(mockedDb.adminRepo.updateBrandLogo)).toHaveBeenCalledWith( + expect(vi.mocked(mockedBrandService.updateBrandLogo)).toHaveBeenCalledWith( brandId, - expect.stringContaining('/flyer-images/'), + expect.objectContaining({ fieldname: 'logoImage' }), expect.anything(), ); }); it('POST /brands/:id/logo should return 500 on DB error', async () => { const brandId = 55; - vi.mocked(mockedDb.adminRepo.updateBrandLogo).mockRejectedValue(new Error('DB Error')); + vi.mocked(mockedBrandService.updateBrandLogo).mockRejectedValue(new Error('DB Error')); const response = await supertest(app) .post(`/api/admin/brands/${brandId}/logo`) .attach('logoImage', Buffer.from('dummy-logo-content'), 'test-logo.png'); @@ -347,7 +375,7 @@ describe('Admin Content Management Routes (/api/admin)', () => { it('should clean up the uploaded file if updating the brand logo fails', async () => { const brandId = 55; const dbError = new Error('DB Connection Failed'); - vi.mocked(mockedDb.adminRepo.updateBrandLogo).mockRejectedValue(dbError); + vi.mocked(mockedBrandService.updateBrandLogo).mockRejectedValue(dbError); const response = await supertest(app) .post(`/api/admin/brands/${brandId}/logo`) diff --git a/src/routes/admin.jobs.routes.test.ts b/src/routes/admin.jobs.routes.test.ts index b0da006c..f8e66694 100644 --- a/src/routes/admin.jobs.routes.test.ts +++ b/src/routes/admin.jobs.routes.test.ts @@ -29,6 +29,17 @@ vi.mock('../services/queueService.server', () => ({ cleanupWorker: {}, weeklyAnalyticsWorker: {}, })); + +// Mock the monitoring service - the routes use this service for job operations +vi.mock('../services/monitoringService.server', () => ({ + monitoringService: { + getWorkerStatuses: vi.fn(), + getQueueStatuses: vi.fn(), + retryFailedJob: vi.fn(), + getJobStatus: vi.fn(), + }, +})); + vi.mock('../services/db/index.db', () => ({ adminRepo: {}, flyerRepo: {}, @@ -59,21 +70,22 @@ import adminRouter from './admin.routes'; // Import the mocked modules to control them import { backgroundJobService } from '../services/backgroundJobService'; // This is now a mock -import { - flyerQueue, - analyticsQueue, - cleanupQueue, - weeklyAnalyticsQueue, -} from '../services/queueService.server'; +import { analyticsQueue, cleanupQueue } from '../services/queueService.server'; +import { monitoringService } from '../services/monitoringService.server'; // This is now a mock +import { NotFoundError, ValidationError } from '../services/db/errors.db'; // Mock the logger -vi.mock('../services/logger.server', async () => ({ - // Use async import to avoid hoisting issues with mockLogger - logger: (await import('../tests/utils/mockLogger')).mockLogger, -})); +vi.mock('../services/logger.server', async () => { + const { mockLogger, createMockLogger } = await import('../tests/utils/mockLogger'); + return { + logger: mockLogger, + createScopedLogger: vi.fn(() => createMockLogger()), + }; +}); // Mock the passport middleware -vi.mock('./passport.routes', () => ({ +// Note: admin.routes.ts imports from '../config/passport', so we mock that path +vi.mock('../config/passport', () => ({ default: { authenticate: vi.fn(() => (req: Request, res: Response, next: NextFunction) => { if (!req.user) return res.status(401).json({ message: 'Unauthorized' }); @@ -221,13 +233,8 @@ describe('Admin Job Trigger Routes (/api/admin/trigger)', () => { const jobId = 'failed-job-1'; it('should successfully retry a failed job', async () => { - // Arrange - const mockJob = { - id: jobId, - getState: vi.fn().mockResolvedValue('failed'), - retry: vi.fn().mockResolvedValue(undefined), - }; - vi.mocked(flyerQueue.getJob).mockResolvedValue(mockJob as unknown as Job); + // Arrange - mock the monitoring service to resolve successfully + vi.mocked(monitoringService.retryFailedJob).mockResolvedValue(undefined); // Act const response = await supertest(app).post(`/api/admin/jobs/${queueName}/${jobId}/retry`); @@ -237,7 +244,11 @@ describe('Admin Job Trigger Routes (/api/admin/trigger)', () => { expect(response.body.data.message).toBe( `Job ${jobId} has been successfully marked for retry.`, ); - expect(mockJob.retry).toHaveBeenCalledTimes(1); + expect(monitoringService.retryFailedJob).toHaveBeenCalledWith( + queueName, + jobId, + 'admin-user-id', + ); }); it('should return 400 if the queue name is invalid', async () => { @@ -250,8 +261,10 @@ describe('Admin Job Trigger Routes (/api/admin/trigger)', () => { const queueName = 'weekly-analytics-reporting'; const jobId = 'some-job-id'; - // Ensure getJob returns undefined (not found) - vi.mocked(weeklyAnalyticsQueue.getJob).mockResolvedValue(undefined); + // Mock monitoringService.retryFailedJob to throw NotFoundError + vi.mocked(monitoringService.retryFailedJob).mockRejectedValue( + new NotFoundError(`Job with ID '${jobId}' not found in queue '${queueName}'.`), + ); const response = await supertest(app).post(`/api/admin/jobs/${queueName}/${jobId}/retry`); @@ -262,7 +275,10 @@ describe('Admin Job Trigger Routes (/api/admin/trigger)', () => { }); it('should return 404 if the job ID is not found in the queue', async () => { - vi.mocked(flyerQueue.getJob).mockResolvedValue(undefined); + // Mock monitoringService.retryFailedJob to throw NotFoundError + vi.mocked(monitoringService.retryFailedJob).mockRejectedValue( + new NotFoundError("Job with ID 'not-found-job' not found in queue 'flyer-processing'."), + ); const response = await supertest(app).post( `/api/admin/jobs/${queueName}/not-found-job/retry`, ); @@ -271,12 +287,10 @@ describe('Admin Job Trigger Routes (/api/admin/trigger)', () => { }); it('should return 400 if the job is not in a failed state', async () => { - const mockJob = { - id: jobId, - getState: vi.fn().mockResolvedValue('completed'), - retry: vi.fn(), - }; - vi.mocked(flyerQueue.getJob).mockResolvedValue(mockJob as unknown as Job); + // Mock monitoringService.retryFailedJob to throw ValidationError + vi.mocked(monitoringService.retryFailedJob).mockRejectedValue( + new ValidationError([], "Job is not in a 'failed' state. Current state: completed."), + ); const response = await supertest(app).post(`/api/admin/jobs/${queueName}/${jobId}/retry`); @@ -284,16 +298,11 @@ describe('Admin Job Trigger Routes (/api/admin/trigger)', () => { expect(response.body.error.message).toBe( "Job is not in a 'failed' state. Current state: completed.", ); // This is now handled by the errorHandler - expect(mockJob.retry).not.toHaveBeenCalled(); }); it('should return 500 if job.retry() throws an error', async () => { - const mockJob = { - id: jobId, - getState: vi.fn().mockResolvedValue('failed'), - retry: vi.fn().mockRejectedValue(new Error('Cannot retry job')), - }; - vi.mocked(flyerQueue.getJob).mockResolvedValue(mockJob as unknown as Job); + // Mock monitoringService.retryFailedJob to throw a generic error + vi.mocked(monitoringService.retryFailedJob).mockRejectedValue(new Error('Cannot retry job')); const response = await supertest(app).post(`/api/admin/jobs/${queueName}/${jobId}/retry`); diff --git a/src/routes/admin.monitoring.routes.test.ts b/src/routes/admin.monitoring.routes.test.ts index af2d5932..5991050a 100644 --- a/src/routes/admin.monitoring.routes.test.ts +++ b/src/routes/admin.monitoring.routes.test.ts @@ -92,10 +92,12 @@ import { adminRepo } from '../services/db/index.db'; // Mock the logger vi.mock('../services/logger.server', () => ({ logger: mockLogger, + createScopedLogger: vi.fn(() => mockLogger), })); // Mock the passport middleware -vi.mock('./passport.routes', () => ({ +// Note: admin.routes.ts imports from '../config/passport', so we mock that path +vi.mock('../config/passport', () => ({ default: { authenticate: vi.fn(() => (req: Request, res: Response, next: NextFunction) => { if (!req.user) return res.status(401).json({ message: 'Unauthorized' }); diff --git a/src/routes/admin.routes.test.ts b/src/routes/admin.routes.test.ts index 0a75a4d1..435362e7 100644 --- a/src/routes/admin.routes.test.ts +++ b/src/routes/admin.routes.test.ts @@ -41,9 +41,13 @@ vi.mock('../services/cacheService.server', () => ({ }, })); -vi.mock('../services/logger.server', async () => ({ - logger: (await import('../tests/utils/mockLogger')).mockLogger, -})); +vi.mock('../services/logger.server', async () => { + const { mockLogger, createMockLogger } = await import('../tests/utils/mockLogger'); + return { + logger: mockLogger, + createScopedLogger: vi.fn(() => createMockLogger()), + }; +}); vi.mock('@bull-board/api'); vi.mock('@bull-board/api/bullMQAdapter'); @@ -57,9 +61,27 @@ vi.mock('@bull-board/express', () => ({ })); vi.mock('node:fs/promises'); +vi.mock('../services/queues.server'); +vi.mock('../services/workers.server'); +vi.mock('../services/monitoringService.server'); +vi.mock('../services/userService'); +vi.mock('../services/brandService'); +vi.mock('../services/receiptService.server'); +vi.mock('../services/aiService.server'); +vi.mock('../config/env', () => ({ + config: { + database: { host: 'localhost', port: 5432, user: 'test', password: 'test', name: 'test' }, + redis: { url: 'redis://localhost:6379' }, + auth: { jwtSecret: 'test-secret' }, + server: { port: 3000, host: 'localhost' }, + }, + isAiConfigured: vi.fn().mockReturnValue(false), + parseConfig: vi.fn(), +})); // Mock Passport to allow admin access -vi.mock('./passport.routes', () => ({ +// Note: admin.routes.ts imports from '../config/passport', so we mock that path +vi.mock('../config/passport', () => ({ default: { authenticate: vi.fn(() => (req: any, res: any, next: any) => { req.user = createMockUserProfile({ role: 'admin' }); diff --git a/src/routes/admin.stats.routes.test.ts b/src/routes/admin.stats.routes.test.ts index 4b07607d..dd0329aa 100644 --- a/src/routes/admin.stats.routes.test.ts +++ b/src/routes/admin.stats.routes.test.ts @@ -26,6 +26,24 @@ vi.mock('node:fs/promises'); vi.mock('../services/backgroundJobService'); vi.mock('../services/geocodingService.server'); vi.mock('../services/queueService.server'); +vi.mock('../services/queues.server'); +vi.mock('../services/workers.server'); +vi.mock('../services/monitoringService.server'); +vi.mock('../services/cacheService.server'); +vi.mock('../services/userService'); +vi.mock('../services/brandService'); +vi.mock('../services/receiptService.server'); +vi.mock('../services/aiService.server'); +vi.mock('../config/env', () => ({ + config: { + database: { host: 'localhost', port: 5432, user: 'test', password: 'test', name: 'test' }, + redis: { url: 'redis://localhost:6379' }, + auth: { jwtSecret: 'test-secret' }, + server: { port: 3000, host: 'localhost' }, + }, + isAiConfigured: vi.fn().mockReturnValue(false), + parseConfig: vi.fn(), +})); vi.mock('@bull-board/api'); vi.mock('@bull-board/api/bullMQAdapter'); vi.mock('@bull-board/express', () => ({ @@ -44,13 +62,17 @@ import adminRouter from './admin.routes'; import { adminRepo } from '../services/db/index.db'; // Mock the logger -vi.mock('../services/logger.server', async () => ({ - // Use async import to avoid hoisting issues with mockLogger - logger: (await import('../tests/utils/mockLogger')).mockLogger, -})); +vi.mock('../services/logger.server', async () => { + const { mockLogger, createMockLogger } = await import('../tests/utils/mockLogger'); + return { + logger: mockLogger, + createScopedLogger: vi.fn(() => createMockLogger()), + }; +}); // Mock the passport middleware -vi.mock('./passport.routes', () => ({ +// Note: admin.routes.ts imports from '../config/passport', so we mock that path +vi.mock('../config/passport', () => ({ default: { authenticate: vi.fn(() => (req: Request, res: Response, next: NextFunction) => { if (!req.user) return res.status(401).json({ message: 'Unauthorized' }); diff --git a/src/routes/admin.system.routes.test.ts b/src/routes/admin.system.routes.test.ts index e7cbabd8..6f8a59e2 100644 --- a/src/routes/admin.system.routes.test.ts +++ b/src/routes/admin.system.routes.test.ts @@ -31,6 +31,24 @@ vi.mock('../services/backgroundJobService', () => ({ }, })); vi.mock('../services/queueService.server'); +vi.mock('../services/queues.server'); +vi.mock('../services/workers.server'); +vi.mock('../services/monitoringService.server'); +vi.mock('../services/cacheService.server'); +vi.mock('../services/userService'); +vi.mock('../services/brandService'); +vi.mock('../services/receiptService.server'); +vi.mock('../services/aiService.server'); +vi.mock('../config/env', () => ({ + config: { + database: { host: 'localhost', port: 5432, user: 'test', password: 'test', name: 'test' }, + redis: { url: 'redis://localhost:6379' }, + auth: { jwtSecret: 'test-secret' }, + server: { port: 3000, host: 'localhost' }, + }, + isAiConfigured: vi.fn().mockReturnValue(false), + parseConfig: vi.fn(), +})); vi.mock('@bull-board/api'); vi.mock('@bull-board/api/bullMQAdapter'); vi.mock('@bull-board/express', () => ({ @@ -49,13 +67,17 @@ import adminRouter from './admin.routes'; import { geocodingService } from '../services/geocodingService.server'; // Mock the logger -vi.mock('../services/logger.server', async () => ({ - // Use async import to avoid hoisting issues with mockLogger - logger: (await import('../tests/utils/mockLogger')).mockLogger, -})); +vi.mock('../services/logger.server', async () => { + const { mockLogger, createMockLogger } = await import('../tests/utils/mockLogger'); + return { + logger: mockLogger, + createScopedLogger: vi.fn(() => createMockLogger()), + }; +}); // Mock the passport middleware -vi.mock('./passport.routes', () => ({ +// Note: admin.routes.ts imports from '../config/passport', so we mock that path +vi.mock('../config/passport', () => ({ default: { authenticate: vi.fn(() => (req: Request, res: Response, next: NextFunction) => { req.user = createMockUserProfile({ diff --git a/src/routes/admin.users.routes.test.ts b/src/routes/admin.users.routes.test.ts index 95e576db..80bb79b4 100644 --- a/src/routes/admin.users.routes.test.ts +++ b/src/routes/admin.users.routes.test.ts @@ -34,6 +34,23 @@ vi.mock('../services/db/recipe.db'); vi.mock('../services/backgroundJobService'); vi.mock('../services/geocodingService.server'); vi.mock('../services/queueService.server'); +vi.mock('../services/queues.server'); +vi.mock('../services/workers.server'); +vi.mock('../services/monitoringService.server'); +vi.mock('../services/cacheService.server'); +vi.mock('../services/brandService'); +vi.mock('../services/receiptService.server'); +vi.mock('../services/aiService.server'); +vi.mock('../config/env', () => ({ + config: { + database: { host: 'localhost', port: 5432, user: 'test', password: 'test', name: 'test' }, + redis: { url: 'redis://localhost:6379' }, + auth: { jwtSecret: 'test-secret' }, + server: { port: 3000, host: 'localhost' }, + }, + isAiConfigured: vi.fn().mockReturnValue(false), + parseConfig: vi.fn(), +})); vi.mock('@bull-board/api'); vi.mock('@bull-board/api/bullMQAdapter'); vi.mock('node:fs/promises'); @@ -49,10 +66,13 @@ vi.mock('@bull-board/express', () => ({ })); // Mock the logger -vi.mock('../services/logger.server', async () => ({ - // Use async import to avoid hoisting issues with mockLogger - logger: (await import('../tests/utils/mockLogger')).mockLogger, -})); +vi.mock('../services/logger.server', async () => { + const { mockLogger, createMockLogger } = await import('../tests/utils/mockLogger'); + return { + logger: mockLogger, + createScopedLogger: vi.fn(() => createMockLogger()), + }; +}); // Import the router AFTER all mocks are defined. import adminRouter from './admin.routes'; @@ -62,7 +82,8 @@ import { adminRepo, userRepo } from '../services/db/index.db'; import { userService } from '../services/userService'; // Mock the passport middleware -vi.mock('./passport.routes', () => ({ +// Note: admin.routes.ts imports from '../config/passport', so we mock that path +vi.mock('../config/passport', () => ({ default: { authenticate: vi.fn(() => (req: Request, res: Response, next: NextFunction) => { if (!req.user) return res.status(401).json({ message: 'Unauthorized' }); diff --git a/src/routes/ai.routes.test.ts b/src/routes/ai.routes.test.ts index d13c6eba..412de53b 100644 --- a/src/routes/ai.routes.test.ts +++ b/src/routes/ai.routes.test.ts @@ -61,18 +61,43 @@ vi.mock('../services/queueService.server', () => ({ }, })); -// Import the router AFTER all mocks are defined. -import aiRouter from './ai.routes'; -import { flyerQueue } from '../services/queueService.server'; - -// Mock the logger to keep test output clean -vi.mock('../services/logger.server', async () => ({ - // Use async import to avoid hoisting issues with mockLogger - logger: (await import('../tests/utils/mockLogger')).mockLogger, +// Mock the monitoring service +const { mockedMonitoringService } = vi.hoisted(() => ({ + mockedMonitoringService: { + getFlyerJobStatus: vi.fn(), + }, +})); +vi.mock('../services/monitoringService.server', () => ({ + monitoringService: mockedMonitoringService, })); +// Mock env config to prevent parsing errors +vi.mock('../config/env', () => ({ + config: { + database: { host: 'localhost', port: 5432, user: 'test', password: 'test', name: 'test' }, + redis: { url: 'redis://localhost:6379' }, + auth: { jwtSecret: 'test-secret' }, + server: { port: 3000, host: 'localhost' }, + ai: { enabled: true }, + }, + isAiConfigured: vi.fn().mockReturnValue(true), + parseConfig: vi.fn(), +})); + +// Import the router AFTER all mocks are defined. +import aiRouter from './ai.routes'; + +// Mock the logger to keep test output clean +vi.mock('../services/logger.server', async () => { + const { mockLogger, createMockLogger } = await import('../tests/utils/mockLogger'); + return { + logger: mockLogger, + createScopedLogger: vi.fn(() => createMockLogger()), + }; +}); + // Mock the passport module to control authentication for different tests. -vi.mock('./passport.routes', () => ({ +vi.mock('../config/passport', () => ({ default: { // Mock passport.authenticate to simply call next(), allowing the request to proceed. // The actual user object will be injected by the mockAuth middleware or test setup. @@ -84,13 +109,19 @@ vi.mock('./passport.routes', () => ({ })); describe('AI Routes (/api/ai)', () => { - beforeEach(() => { + beforeEach(async () => { vi.clearAllMocks(); // Reset logger implementation to no-op to prevent "Logging failed" leaks from previous tests vi.mocked(mockLogger.info).mockImplementation(() => {}); vi.mocked(mockLogger.error).mockImplementation(() => {}); vi.mocked(mockLogger.warn).mockImplementation(() => {}); vi.mocked(mockLogger.debug).mockImplementation(() => {}); // Ensure debug is also mocked + + // Default mock for monitoring service - returns NotFoundError for unknown jobs + const { NotFoundError } = await import('../services/db/errors.db'); + vi.mocked(mockedMonitoringService.getFlyerJobStatus).mockRejectedValue( + new NotFoundError('Job not found.'), + ); }); const app = createTestApp({ router: aiRouter, basePath: '/api/ai' }); @@ -301,8 +332,11 @@ describe('AI Routes (/api/ai)', () => { describe('GET /jobs/:jobId/status', () => { it('should return 404 if job is not found', async () => { - // Mock the queue to return null for the job - vi.mocked(flyerQueue.getJob).mockResolvedValue(undefined); + // Mock the monitoring service to throw NotFoundError + const { NotFoundError } = await import('../services/db/errors.db'); + vi.mocked(mockedMonitoringService.getFlyerJobStatus).mockRejectedValue( + new NotFoundError('Job not found.'), + ); const response = await supertest(app).get('/api/ai/jobs/non-existent-job/status'); @@ -311,13 +345,13 @@ describe('AI Routes (/api/ai)', () => { }); it('should return job status if job is found', async () => { - const mockJob = { + const mockJobStatus = { id: 'job-123', - getState: async () => 'completed', + state: 'completed', progress: 100, - returnvalue: { flyerId: 1 }, + result: { flyerId: 1 }, }; - vi.mocked(flyerQueue.getJob).mockResolvedValue(mockJob as unknown as Job); + vi.mocked(mockedMonitoringService.getFlyerJobStatus).mockResolvedValue(mockJobStatus); const response = await supertest(app).get('/api/ai/jobs/job-123/status'); diff --git a/src/routes/auth.routes.test.ts b/src/routes/auth.routes.test.ts index 77ab75c3..231ea243 100644 --- a/src/routes/auth.routes.test.ts +++ b/src/routes/auth.routes.test.ts @@ -52,7 +52,7 @@ const passportMocks = vi.hoisted(() => { // --- 2. Module Mocks --- // Mock the local passport.routes module to control its behavior. -vi.mock('./passport.routes', () => ({ +vi.mock('../config/passport', () => ({ default: { authenticate: vi.fn().mockImplementation(passportMocks.authenticateMock), use: vi.fn(), diff --git a/src/routes/budget.routes.test.ts b/src/routes/budget.routes.test.ts index 57afb176..827a2b43 100644 --- a/src/routes/budget.routes.test.ts +++ b/src/routes/budget.routes.test.ts @@ -39,7 +39,7 @@ const mockUser = createMockUserProfile({ }); // Standardized mock for passport.routes -vi.mock('./passport.routes', () => ({ +vi.mock('../config/passport', () => ({ default: { authenticate: vi.fn(() => (req: Request, res: Response, next: NextFunction) => { req.user = mockUser; diff --git a/src/routes/deals.routes.test.ts b/src/routes/deals.routes.test.ts index 15de2cf7..333da444 100644 --- a/src/routes/deals.routes.test.ts +++ b/src/routes/deals.routes.test.ts @@ -25,7 +25,7 @@ vi.mock('../services/logger.server', async () => ({ })); // Mock the passport middleware -vi.mock('./passport.routes', () => ({ +vi.mock('../config/passport', () => ({ default: { authenticate: vi.fn(() => (req: Request, res: Response, next: NextFunction) => { // If req.user is not set by the test setup, simulate unauthenticated access. diff --git a/src/routes/gamification.routes.test.ts b/src/routes/gamification.routes.test.ts index 6fc41190..26fd207a 100644 --- a/src/routes/gamification.routes.test.ts +++ b/src/routes/gamification.routes.test.ts @@ -38,7 +38,7 @@ const mockedAuthMiddleware = vi.hoisted(() => ); const mockedIsAdmin = vi.hoisted(() => vi.fn()); -vi.mock('./passport.routes', () => ({ +vi.mock('../config/passport', () => ({ default: { // The authenticate method will now call our hoisted mock middleware. authenticate: vi.fn(() => mockedAuthMiddleware), diff --git a/src/routes/inventory.routes.test.ts b/src/routes/inventory.routes.test.ts index 896361ef..867b2314 100644 --- a/src/routes/inventory.routes.test.ts +++ b/src/routes/inventory.routes.test.ts @@ -220,7 +220,8 @@ describe('Inventory Routes (/api/inventory)', () => { }); expect(response.status).toBe(400); - expect(response.body.error.details[0].message).toMatch(/Item name/i); + // Zod returns a type error message when a required field is undefined + expect(response.body.error.details[0].message).toMatch(/expected string|required/i); }); it('should return 400 for invalid source', async () => { diff --git a/src/routes/inventory.routes.ts b/src/routes/inventory.routes.ts index d9c67fdd..95aa2899 100644 --- a/src/routes/inventory.routes.ts +++ b/src/routes/inventory.routes.ts @@ -313,6 +313,322 @@ router.post( }, ); +// ============================================================================ +// EXPIRING ITEMS ENDPOINTS +// NOTE: These routes MUST be defined BEFORE /:inventoryId to avoid path conflicts +// ============================================================================ + +/** + * @openapi + * /inventory/expiring/summary: + * get: + * tags: [Inventory] + * summary: Get expiring items summary + * description: Get items grouped by expiry urgency (today, this week, this month, expired). + * security: + * - bearerAuth: [] + * responses: + * 200: + * description: Expiring items grouped by urgency + * content: + * application/json: + * schema: + * type: object + * properties: + * expiring_today: + * type: array + * expiring_this_week: + * type: array + * expiring_this_month: + * type: array + * already_expired: + * type: array + * counts: + * type: object + * properties: + * today: + * type: integer + * this_week: + * type: integer + * this_month: + * type: integer + * expired: + * type: integer + * total: + * type: integer + * 401: + * description: Unauthorized + */ +router.get('/expiring/summary', async (req: Request, res: Response, next: NextFunction) => { + const userProfile = req.user as UserProfile; + + try { + const result = await expiryService.getExpiringItemsGrouped(userProfile.user.user_id, req.log); + sendSuccess(res, result); + } catch (error) { + req.log.error( + { error, userId: userProfile.user.user_id }, + 'Error fetching expiring items summary', + ); + next(error); + } +}); + +/** + * @openapi + * /inventory/expiring: + * get: + * tags: [Inventory] + * summary: Get expiring items + * description: Get items expiring within a specified number of days. + * security: + * - bearerAuth: [] + * parameters: + * - in: query + * name: days + * schema: + * type: integer + * minimum: 1 + * maximum: 90 + * default: 7 + * description: Number of days to look ahead + * responses: + * 200: + * description: Expiring items retrieved + * 401: + * description: Unauthorized + */ +router.get( + '/expiring', + validateRequest(daysAheadQuerySchema), + async (req: Request, res: Response, next: NextFunction) => { + const userProfile = req.user as UserProfile; + type ExpiringItemsRequest = z.infer; + const { query } = req as unknown as ExpiringItemsRequest; + + try { + const items = await expiryService.getExpiringItems( + userProfile.user.user_id, + query.days, + req.log, + ); + sendSuccess(res, { items, total: items.length }); + } catch (error) { + req.log.error({ error, userId: userProfile.user.user_id }, 'Error fetching expiring items'); + next(error); + } + }, +); + +/** + * @openapi + * /inventory/expired: + * get: + * tags: [Inventory] + * summary: Get expired items + * description: Get all items that have already expired. + * security: + * - bearerAuth: [] + * responses: + * 200: + * description: Expired items retrieved + * 401: + * description: Unauthorized + */ +router.get('/expired', async (req: Request, res: Response, next: NextFunction) => { + const userProfile = req.user as UserProfile; + + try { + const items = await expiryService.getExpiredItems(userProfile.user.user_id, req.log); + sendSuccess(res, { items, total: items.length }); + } catch (error) { + req.log.error({ error, userId: userProfile.user.user_id }, 'Error fetching expired items'); + next(error); + } +}); + +// ============================================================================ +// ALERT SETTINGS ENDPOINTS +// NOTE: These routes MUST be defined BEFORE /:inventoryId to avoid path conflicts +// ============================================================================ + +/** + * @openapi + * /inventory/alerts: + * get: + * tags: [Inventory] + * summary: Get alert settings + * description: Get the user's expiry alert settings. + * security: + * - bearerAuth: [] + * responses: + * 200: + * description: Alert settings retrieved + * 401: + * description: Unauthorized + */ +router.get('/alerts', async (req: Request, res: Response, next: NextFunction) => { + const userProfile = req.user as UserProfile; + + try { + const settings = await expiryService.getAlertSettings(userProfile.user.user_id, req.log); + sendSuccess(res, settings); + } catch (error) { + req.log.error({ error, userId: userProfile.user.user_id }, 'Error fetching alert settings'); + next(error); + } +}); + +/** + * @openapi + * /inventory/alerts/{alertMethod}: + * put: + * tags: [Inventory] + * summary: Update alert settings + * description: Update alert settings for a specific notification method. + * security: + * - bearerAuth: [] + * parameters: + * - in: path + * name: alertMethod + * required: true + * schema: + * type: string + * enum: [email, push, in_app] + * requestBody: + * required: true + * content: + * application/json: + * schema: + * type: object + * properties: + * days_before_expiry: + * type: integer + * minimum: 1 + * maximum: 30 + * is_enabled: + * type: boolean + * responses: + * 200: + * description: Alert settings updated + * 400: + * description: Validation error + * 401: + * description: Unauthorized + */ +router.put( + '/alerts/:alertMethod', + validateRequest(updateAlertSettingsSchema), + async (req: Request, res: Response, next: NextFunction) => { + const userProfile = req.user as UserProfile; + type UpdateAlertRequest = z.infer; + const { params, body } = req as unknown as UpdateAlertRequest; + + try { + const settings = await expiryService.updateAlertSettings( + userProfile.user.user_id, + params.alertMethod, + body, + req.log, + ); + sendSuccess(res, settings); + } catch (error) { + req.log.error( + { error, userId: userProfile.user.user_id, alertMethod: params.alertMethod }, + 'Error updating alert settings', + ); + next(error); + } + }, +); + +// ============================================================================ +// RECIPE SUGGESTIONS ENDPOINT +// NOTE: This route MUST be defined BEFORE /:inventoryId to avoid path conflicts +// ============================================================================ + +/** + * @openapi + * /inventory/recipes/suggestions: + * get: + * tags: [Inventory] + * summary: Get recipe suggestions for expiring items + * description: Get recipes that use items expiring soon to reduce food waste. + * security: + * - bearerAuth: [] + * parameters: + * - in: query + * name: days + * schema: + * type: integer + * minimum: 1 + * maximum: 90 + * default: 7 + * description: Consider items expiring within this many days + * - in: query + * name: limit + * schema: + * type: integer + * minimum: 1 + * maximum: 50 + * default: 10 + * - in: query + * name: offset + * schema: + * type: integer + * minimum: 0 + * default: 0 + * responses: + * 200: + * description: Recipe suggestions retrieved + * 401: + * description: Unauthorized + */ +router.get( + '/recipes/suggestions', + validateRequest( + z.object({ + query: z.object({ + days: z + .string() + .optional() + .default('7') + .transform((val) => parseInt(val, 10)) + .pipe(z.number().int().min(1).max(90)), + limit: optionalNumeric({ default: 10, min: 1, max: 50, integer: true }), + offset: optionalNumeric({ default: 0, min: 0, integer: true }), + }), + }), + ), + async (req: Request, res: Response, next: NextFunction) => { + const userProfile = req.user as UserProfile; + const { query } = req as unknown as { + query: { days: number; limit?: number; offset?: number }; + }; + + try { + const result = await expiryService.getRecipeSuggestionsForExpiringItems( + userProfile.user.user_id, + query.days, + req.log, + { limit: query.limit, offset: query.offset }, + ); + sendSuccess(res, result); + } catch (error) { + req.log.error( + { error, userId: userProfile.user.user_id }, + 'Error fetching recipe suggestions', + ); + next(error); + } + }, +); + +// ============================================================================ +// INVENTORY ITEM BY ID ENDPOINTS +// NOTE: These routes with /:inventoryId MUST come AFTER specific path routes +// ============================================================================ + /** * @openapi * /inventory/{inventoryId}: @@ -528,312 +844,4 @@ router.post( }, ); -// ============================================================================ -// EXPIRING ITEMS ENDPOINTS -// ============================================================================ - -/** - * @openapi - * /inventory/expiring/summary: - * get: - * tags: [Inventory] - * summary: Get expiring items summary - * description: Get items grouped by expiry urgency (today, this week, this month, expired). - * security: - * - bearerAuth: [] - * responses: - * 200: - * description: Expiring items grouped by urgency - * content: - * application/json: - * schema: - * type: object - * properties: - * expiring_today: - * type: array - * expiring_this_week: - * type: array - * expiring_this_month: - * type: array - * already_expired: - * type: array - * counts: - * type: object - * properties: - * today: - * type: integer - * this_week: - * type: integer - * this_month: - * type: integer - * expired: - * type: integer - * total: - * type: integer - * 401: - * description: Unauthorized - */ -router.get('/expiring/summary', async (req: Request, res: Response, next: NextFunction) => { - const userProfile = req.user as UserProfile; - - try { - const result = await expiryService.getExpiringItemsGrouped(userProfile.user.user_id, req.log); - sendSuccess(res, result); - } catch (error) { - req.log.error( - { error, userId: userProfile.user.user_id }, - 'Error fetching expiring items summary', - ); - next(error); - } -}); - -/** - * @openapi - * /inventory/expiring: - * get: - * tags: [Inventory] - * summary: Get expiring items - * description: Get items expiring within a specified number of days. - * security: - * - bearerAuth: [] - * parameters: - * - in: query - * name: days - * schema: - * type: integer - * minimum: 1 - * maximum: 90 - * default: 7 - * description: Number of days to look ahead - * responses: - * 200: - * description: Expiring items retrieved - * 401: - * description: Unauthorized - */ -router.get( - '/expiring', - validateRequest(daysAheadQuerySchema), - async (req: Request, res: Response, next: NextFunction) => { - const userProfile = req.user as UserProfile; - type ExpiringItemsRequest = z.infer; - const { query } = req as unknown as ExpiringItemsRequest; - - try { - const items = await expiryService.getExpiringItems( - userProfile.user.user_id, - query.days, - req.log, - ); - sendSuccess(res, { items, total: items.length }); - } catch (error) { - req.log.error({ error, userId: userProfile.user.user_id }, 'Error fetching expiring items'); - next(error); - } - }, -); - -/** - * @openapi - * /inventory/expired: - * get: - * tags: [Inventory] - * summary: Get expired items - * description: Get all items that have already expired. - * security: - * - bearerAuth: [] - * responses: - * 200: - * description: Expired items retrieved - * 401: - * description: Unauthorized - */ -router.get('/expired', async (req: Request, res: Response, next: NextFunction) => { - const userProfile = req.user as UserProfile; - - try { - const items = await expiryService.getExpiredItems(userProfile.user.user_id, req.log); - sendSuccess(res, { items, total: items.length }); - } catch (error) { - req.log.error({ error, userId: userProfile.user.user_id }, 'Error fetching expired items'); - next(error); - } -}); - -// ============================================================================ -// ALERT SETTINGS ENDPOINTS -// ============================================================================ - -/** - * @openapi - * /inventory/alerts: - * get: - * tags: [Inventory] - * summary: Get alert settings - * description: Get the user's expiry alert settings. - * security: - * - bearerAuth: [] - * responses: - * 200: - * description: Alert settings retrieved - * 401: - * description: Unauthorized - */ -router.get('/alerts', async (req: Request, res: Response, next: NextFunction) => { - const userProfile = req.user as UserProfile; - - try { - const settings = await expiryService.getAlertSettings(userProfile.user.user_id, req.log); - sendSuccess(res, settings); - } catch (error) { - req.log.error({ error, userId: userProfile.user.user_id }, 'Error fetching alert settings'); - next(error); - } -}); - -/** - * @openapi - * /inventory/alerts/{alertMethod}: - * put: - * tags: [Inventory] - * summary: Update alert settings - * description: Update alert settings for a specific notification method. - * security: - * - bearerAuth: [] - * parameters: - * - in: path - * name: alertMethod - * required: true - * schema: - * type: string - * enum: [email, push, in_app] - * requestBody: - * required: true - * content: - * application/json: - * schema: - * type: object - * properties: - * days_before_expiry: - * type: integer - * minimum: 1 - * maximum: 30 - * is_enabled: - * type: boolean - * responses: - * 200: - * description: Alert settings updated - * 400: - * description: Validation error - * 401: - * description: Unauthorized - */ -router.put( - '/alerts/:alertMethod', - validateRequest(updateAlertSettingsSchema), - async (req: Request, res: Response, next: NextFunction) => { - const userProfile = req.user as UserProfile; - type UpdateAlertRequest = z.infer; - const { params, body } = req as unknown as UpdateAlertRequest; - - try { - const settings = await expiryService.updateAlertSettings( - userProfile.user.user_id, - params.alertMethod, - body, - req.log, - ); - sendSuccess(res, settings); - } catch (error) { - req.log.error( - { error, userId: userProfile.user.user_id, alertMethod: params.alertMethod }, - 'Error updating alert settings', - ); - next(error); - } - }, -); - -// ============================================================================ -// RECIPE SUGGESTIONS ENDPOINT -// ============================================================================ - -/** - * @openapi - * /inventory/recipes/suggestions: - * get: - * tags: [Inventory] - * summary: Get recipe suggestions for expiring items - * description: Get recipes that use items expiring soon to reduce food waste. - * security: - * - bearerAuth: [] - * parameters: - * - in: query - * name: days - * schema: - * type: integer - * minimum: 1 - * maximum: 90 - * default: 7 - * description: Consider items expiring within this many days - * - in: query - * name: limit - * schema: - * type: integer - * minimum: 1 - * maximum: 50 - * default: 10 - * - in: query - * name: offset - * schema: - * type: integer - * minimum: 0 - * default: 0 - * responses: - * 200: - * description: Recipe suggestions retrieved - * 401: - * description: Unauthorized - */ -router.get( - '/recipes/suggestions', - validateRequest( - z.object({ - query: z.object({ - days: z - .string() - .optional() - .default('7') - .transform((val) => parseInt(val, 10)) - .pipe(z.number().int().min(1).max(90)), - limit: optionalNumeric({ default: 10, min: 1, max: 50, integer: true }), - offset: optionalNumeric({ default: 0, min: 0, integer: true }), - }), - }), - ), - async (req: Request, res: Response, next: NextFunction) => { - const userProfile = req.user as UserProfile; - const { query } = req as unknown as { - query: { days: number; limit?: number; offset?: number }; - }; - - try { - const result = await expiryService.getRecipeSuggestionsForExpiringItems( - userProfile.user.user_id, - query.days, - req.log, - { limit: query.limit, offset: query.offset }, - ); - sendSuccess(res, result); - } catch (error) { - req.log.error( - { error, userId: userProfile.user.user_id }, - 'Error fetching recipe suggestions', - ); - next(error); - } - }, -); - export default router; diff --git a/src/routes/price.routes.test.ts b/src/routes/price.routes.test.ts index 5a5165e5..d605a401 100644 --- a/src/routes/price.routes.test.ts +++ b/src/routes/price.routes.test.ts @@ -20,7 +20,7 @@ vi.mock('../services/logger.server', async () => ({ })); // Mock the passport middleware -vi.mock('./passport.routes', () => ({ +vi.mock('../config/passport', () => ({ default: { authenticate: vi.fn(() => (req: Request, res: Response, next: NextFunction) => { // If req.user is not set by the test setup, simulate unauthenticated access. diff --git a/src/routes/reactions.routes.test.ts b/src/routes/reactions.routes.test.ts index 1a29b1d3..a358c332 100644 --- a/src/routes/reactions.routes.test.ts +++ b/src/routes/reactions.routes.test.ts @@ -20,7 +20,7 @@ vi.mock('../services/logger.server', async () => ({ })); // Mock Passport middleware -vi.mock('./passport.routes', () => ({ +vi.mock('../config/passport', () => ({ default: { authenticate: vi.fn(() => (req: Request, res: Response, next: NextFunction) => { // If we are testing the unauthenticated state (no user injected), simulate 401. diff --git a/src/routes/receipt.routes.test.ts b/src/routes/receipt.routes.test.ts index e17c70c4..3af5bf08 100644 --- a/src/routes/receipt.routes.test.ts +++ b/src/routes/receipt.routes.test.ts @@ -5,6 +5,11 @@ import { createTestApp } from '../tests/utils/createTestApp'; import { createMockUserProfile } from '../tests/utils/mockFactories'; import receiptRouter from './receipt.routes'; import type { ReceiptStatus, ReceiptItemStatus } from '../types/expiry'; +import { NotFoundError } from '../services/db/errors.db'; + +// Test state - must be declared before vi.mock calls that reference them +let mockUser: ReturnType | null = null; +let mockFile: Express.Multer.File | null = null; // Mock passport vi.mock('../config/passport', () => ({ @@ -17,6 +22,7 @@ vi.mock('../config/passport', () => ({ res.status(401).json({ success: false, error: { message: 'Unauthorized' } }); } }), + initialize: () => (req: any, res: any, next: any) => next(), }, })); @@ -45,23 +51,36 @@ vi.mock('../services/queues.server', () => ({ })); // Mock multer middleware -vi.mock('../middleware/multer.middleware', () => ({ - createUploadMiddleware: vi.fn(() => ({ - single: vi.fn(() => (req: any, _res: any, next: any) => { - // Simulate file upload - if (mockFile) { - req.file = mockFile; +vi.mock('../middleware/multer.middleware', () => { + return { + createUploadMiddleware: vi.fn(() => ({ + single: vi.fn(() => (req: any, _res: any, next: any) => { + // Simulate file upload by setting req.file + if (mockFile) { + req.file = mockFile; + } + // Multer also parses the body fields from multipart form data. + // Since we're mocking multer, we need to ensure req.body is an object. + // Supertest with .field() sends data as multipart which express.json() doesn't parse. + // The actual field data won't be in req.body from supertest when multer is mocked, + // so we leave req.body as-is (express.json() will have parsed JSON requests, + // and for multipart we need to ensure body is at least an empty object). + if (req.body === undefined) { + req.body = {}; + } + next(); + }), + })), + handleMulterError: vi.fn((err: any, _req: any, res: any, next: any) => { + // Only handle multer-specific errors, pass others to the error handler + if (err instanceof multer.MulterError) { + return res.status(400).json({ success: false, error: { message: err.message } }); } - next(); + // Pass non-multer errors to the next error handler + next(err); }), - })), - handleMulterError: vi.fn((err: any, _req: any, res: any, next: any) => { - if (err) { - return res.status(400).json({ success: false, error: { message: err.message } }); - } - next(); - }), -})); + }; +}); // Mock file upload middleware vi.mock('../middleware/fileUpload.middleware', () => ({ @@ -80,10 +99,6 @@ import * as receiptService from '../services/receiptService.server'; import * as expiryService from '../services/expiryService.server'; import { receiptQueue } from '../services/queues.server'; -// Test state -let mockUser: ReturnType | null = null; -let mockFile: Express.Multer.File | null = null; - // Helper to create mock receipt (ReceiptScan type) function createMockReceipt(overrides: { status?: ReceiptStatus; [key: string]: unknown } = {}) { return { @@ -294,10 +309,10 @@ describe('Receipt Routes', () => { vi.mocked(receiptService.createReceipt).mockResolvedValueOnce(mockReceipt); vi.mocked(receiptQueue.add).mockResolvedValueOnce({ id: 'job-123' } as any); + // Send JSON body instead of form fields since multer is mocked and doesn't parse form data const response = await request(app) .post('/receipts') - .field('store_id', '1') - .field('transaction_date', '2024-01-15'); + .send({ store_id: '1', transaction_date: '2024-01-15' }); expect(response.status).toBe(201); expect(response.body.success).toBe(true); @@ -384,9 +399,9 @@ describe('Receipt Routes', () => { }); it('should return 404 for non-existent receipt', async () => { - const notFoundError = new Error('Receipt not found'); - (notFoundError as any).statusCode = 404; - vi.mocked(receiptService.getReceiptById).mockRejectedValueOnce(notFoundError); + vi.mocked(receiptService.getReceiptById).mockRejectedValueOnce( + new NotFoundError('Receipt not found'), + ); const response = await request(app).get('/receipts/999'); @@ -415,9 +430,9 @@ describe('Receipt Routes', () => { }); it('should return 404 for non-existent receipt', async () => { - const notFoundError = new Error('Receipt not found'); - (notFoundError as any).statusCode = 404; - vi.mocked(receiptService.deleteReceipt).mockRejectedValueOnce(notFoundError); + vi.mocked(receiptService.deleteReceipt).mockRejectedValueOnce( + new NotFoundError('Receipt not found'), + ); const response = await request(app).delete('/receipts/999'); @@ -450,9 +465,9 @@ describe('Receipt Routes', () => { }); it('should return 404 for non-existent receipt', async () => { - const notFoundError = new Error('Receipt not found'); - (notFoundError as any).statusCode = 404; - vi.mocked(receiptService.getReceiptById).mockRejectedValueOnce(notFoundError); + vi.mocked(receiptService.getReceiptById).mockRejectedValueOnce( + new NotFoundError('Receipt not found'), + ); const response = await request(app).post('/receipts/999/reprocess'); @@ -480,9 +495,9 @@ describe('Receipt Routes', () => { }); it('should return 404 if receipt not found', async () => { - const notFoundError = new Error('Receipt not found'); - (notFoundError as any).statusCode = 404; - vi.mocked(receiptService.getReceiptById).mockRejectedValueOnce(notFoundError); + vi.mocked(receiptService.getReceiptById).mockRejectedValueOnce( + new NotFoundError('Receipt not found'), + ); const response = await request(app).get('/receipts/999/items'); @@ -648,11 +663,14 @@ describe('Receipt Routes', () => { ); }); - it('should reject empty items array', async () => { + it('should accept empty items array', async () => { + // Empty array is technically valid, service decides what to do + vi.mocked(expiryService.addItemsFromReceipt).mockResolvedValueOnce([]); + const response = await request(app).post('/receipts/1/confirm').send({ items: [] }); - // Empty array is technically valid, service decides what to do expect(response.status).toBe(200); + expect(response.body.data.count).toBe(0); }); it('should reject missing items field', async () => { @@ -740,9 +758,9 @@ describe('Receipt Routes', () => { }); it('should return 404 for non-existent receipt', async () => { - const notFoundError = new Error('Receipt not found'); - (notFoundError as any).statusCode = 404; - vi.mocked(receiptService.getReceiptById).mockRejectedValueOnce(notFoundError); + vi.mocked(receiptService.getReceiptById).mockRejectedValueOnce( + new NotFoundError('Receipt not found'), + ); const response = await request(app).get('/receipts/999/logs'); diff --git a/src/routes/recipe.routes.test.ts b/src/routes/recipe.routes.test.ts index 7c9acdd4..a26d7ee7 100644 --- a/src/routes/recipe.routes.test.ts +++ b/src/routes/recipe.routes.test.ts @@ -29,7 +29,7 @@ vi.mock('../services/aiService.server', () => ({ })); // Mock Passport -vi.mock('./passport.routes', () => ({ +vi.mock('../config/passport', () => ({ default: { authenticate: vi.fn(() => (req: Request, res: Response, next: NextFunction) => { if (!req.user) { diff --git a/src/routes/upc.routes.test.ts b/src/routes/upc.routes.test.ts index bb8ce579..e6dc4e4b 100644 --- a/src/routes/upc.routes.test.ts +++ b/src/routes/upc.routes.test.ts @@ -36,10 +36,14 @@ const _mockAdminUser = createMockUserProfile({ }); // Standardized mock for passport +// Note: createTestApp sets req.user before the router runs, so we preserve it here vi.mock('../config/passport', () => ({ default: { authenticate: vi.fn(() => (req: Request, res: Response, next: NextFunction) => { - req.user = mockUser; + // Preserve the user set by createTestApp if already present + if (!req.user) { + req.user = mockUser; + } next(); }), initialize: () => (req: Request, res: Response, next: NextFunction) => next(), diff --git a/src/routes/user.routes.test.ts b/src/routes/user.routes.test.ts index bb44bc91..9087675a 100644 --- a/src/routes/user.routes.test.ts +++ b/src/routes/user.routes.test.ts @@ -42,7 +42,7 @@ import userRouter from './user.routes'; import * as db from '../services/db/index.db'; // Mock Passport middleware -vi.mock('./passport.routes', () => ({ +vi.mock('../config/passport', () => ({ default: { authenticate: vi.fn( () => (req: express.Request, res: express.Response, next: express.NextFunction) => { diff --git a/src/services/aiService.server.test.ts b/src/services/aiService.server.test.ts index 47676a7a..6124e84f 100644 --- a/src/services/aiService.server.test.ts +++ b/src/services/aiService.server.test.ts @@ -19,9 +19,13 @@ import { ValidationError } from './db/errors.db'; import { AiFlyerDataSchema } from '../types/ai'; // Mock the logger to prevent the real pino instance from being created, which causes issues with 'pino-pretty' in tests. -vi.mock('./logger.server', () => ({ - logger: createMockLogger(), -})); +vi.mock('./logger.server', async () => { + const { createMockLogger } = await import('../tests/utils/mockLogger'); + return { + logger: createMockLogger(), + createScopedLogger: vi.fn(() => createMockLogger()), + }; +}); // Import the mocked logger instance to pass to the service constructor. import { logger as mockLoggerInstance } from './logger.server'; @@ -1096,6 +1100,11 @@ describe('AI Service (Server)', () => { submitterIp: '127.0.0.1', userProfileAddress: '123 St, City, Country', // Partial address match based on filter(Boolean) baseUrl: 'https://example.com', + meta: { + requestId: undefined, + userId: 'user123', + origin: 'api', + }, }); expect(result.id).toBe('job123'); }); @@ -1118,6 +1127,11 @@ describe('AI Service (Server)', () => { userId: undefined, userProfileAddress: undefined, baseUrl: 'https://example.com', + meta: { + requestId: undefined, + userId: undefined, + origin: 'api', + }, }), ); }); diff --git a/src/services/apiClient.test.ts b/src/services/apiClient.test.ts index 74ccfd7c..5c1b55c7 100644 --- a/src/services/apiClient.test.ts +++ b/src/services/apiClient.test.ts @@ -181,6 +181,7 @@ describe('API Client', () => { vi.mocked(global.fetch).mockResolvedValueOnce({ ok: false, status: 500, + headers: new Headers(), clone: () => ({ text: () => Promise.resolve('Internal Server Error') }), } as Response); diff --git a/src/services/barcodeService.server.test.ts b/src/services/barcodeService.server.test.ts index 4f64e140..eed70846 100644 --- a/src/services/barcodeService.server.test.ts +++ b/src/services/barcodeService.server.test.ts @@ -5,6 +5,10 @@ import type { Job } from 'bullmq'; import type { BarcodeDetectionJobData } from '../types/job-data'; import { createMockLogger } from '../tests/utils/mockLogger'; +// Unmock the barcodeService module so we can test the real implementation +// The global test setup mocks this to prevent zxing-wasm issues, but we need the real module here +vi.unmock('./barcodeService.server'); + // Mock dependencies vi.mock('zxing-wasm/reader', () => ({ readBarcodesFromImageData: vi.fn(), diff --git a/src/services/db/expiry.db.test.ts b/src/services/db/expiry.db.test.ts index 0a95f193..fd81f9e7 100644 --- a/src/services/db/expiry.db.test.ts +++ b/src/services/db/expiry.db.test.ts @@ -32,7 +32,7 @@ describe('ExpiryRepository', () => { describe('addInventoryItem', () => { it('should add inventory item with master item lookup', async () => { - // Master item lookup query + // Master item lookup query (only called when item_name is NOT provided) mockQuery.mockResolvedValueOnce({ rowCount: 1, rows: [{ name: 'Milk' }], @@ -67,10 +67,12 @@ describe('ExpiryRepository', () => { rows: [pantryItemRow], }); + // When item_name is NOT provided but master_item_id IS provided, + // the function looks up the item name from master_grocery_items const result = await repo.addInventoryItem( 'user-1', { - item_name: 'Milk', + // item_name intentionally omitted to test master item lookup master_item_id: 100, quantity: 2, unit: 'liters', diff --git a/src/services/db/index.db.test.ts b/src/services/db/index.db.test.ts index fd882f12..b2116e32 100644 --- a/src/services/db/index.db.test.ts +++ b/src/services/db/index.db.test.ts @@ -19,13 +19,19 @@ vi.mock('./gamification.db', () => ({ GamificationRepository: class GamificationRepository {}, })); vi.mock('./admin.db', () => ({ AdminRepository: class AdminRepository {} })); +vi.mock('./upc.db', () => ({ UpcRepository: class UpcRepository {} })); +vi.mock('./expiry.db', () => ({ ExpiryRepository: class ExpiryRepository {} })); +vi.mock('./receipt.db', () => ({ ReceiptRepository: class ReceiptRepository {} })); // These modules export an already-instantiated object, so we mock the object. vi.mock('./reaction.db', () => ({ reactionRepo: {} })); vi.mock('./conversion.db', () => ({ conversionRepo: {} })); -// Mock the re-exported function. -vi.mock('./connection.db', () => ({ withTransaction: vi.fn() })); +// Mock the re-exported function and getPool. +vi.mock('./connection.db', () => ({ + withTransaction: vi.fn(), + getPool: vi.fn(() => ({ query: vi.fn() })), +})); // We must un-mock the file we are testing so we get the actual implementation. vi.unmock('./index.db'); @@ -44,6 +50,9 @@ import { NotificationRepository } from './notification.db'; import { BudgetRepository } from './budget.db'; import { GamificationRepository } from './gamification.db'; import { AdminRepository } from './admin.db'; +import { UpcRepository } from './upc.db'; +import { ExpiryRepository } from './expiry.db'; +import { ReceiptRepository } from './receipt.db'; describe('DB Index', () => { it('should instantiate and export all repositories and functions', () => { @@ -57,8 +66,11 @@ describe('DB Index', () => { expect(db.budgetRepo).toBeInstanceOf(BudgetRepository); expect(db.gamificationRepo).toBeInstanceOf(GamificationRepository); expect(db.adminRepo).toBeInstanceOf(AdminRepository); + expect(db.upcRepo).toBeInstanceOf(UpcRepository); + expect(db.expiryRepo).toBeInstanceOf(ExpiryRepository); + expect(db.receiptRepo).toBeInstanceOf(ReceiptRepository); expect(db.reactionRepo).toBeDefined(); expect(db.conversionRepo).toBeDefined(); expect(db.withTransaction).toBeDefined(); }); -}); \ No newline at end of file +}); diff --git a/src/services/db/receipt.db.test.ts b/src/services/db/receipt.db.test.ts index 129828d7..29a46198 100644 --- a/src/services/db/receipt.db.test.ts +++ b/src/services/db/receipt.db.test.ts @@ -960,14 +960,8 @@ describe('ReceiptRepository', () => { const result = await repo.getActiveStorePatterns(mockLogger); expect(result).toHaveLength(2); - expect(mockQuery).toHaveBeenCalledWith( - expect.stringContaining('is_active = true'), - undefined, - ); - expect(mockQuery).toHaveBeenCalledWith( - expect.stringContaining('ORDER BY priority DESC'), - undefined, - ); + expect(mockQuery).toHaveBeenCalledWith(expect.stringContaining('is_active = true')); + expect(mockQuery).toHaveBeenCalledWith(expect.stringContaining('ORDER BY priority DESC')); }); }); diff --git a/src/services/flyerProcessingService.server.test.ts b/src/services/flyerProcessingService.server.test.ts index 2c4104f1..6d051a1e 100644 --- a/src/services/flyerProcessingService.server.test.ts +++ b/src/services/flyerProcessingService.server.test.ts @@ -12,6 +12,14 @@ const mocks = vi.hoisted(() => ({ readdir: vi.fn(), execAsync: vi.fn(), mockAdminLogActivity: vi.fn(), + // Shared mock logger for verifying calls + sharedMockLogger: { + info: vi.fn(), + error: vi.fn(), + warn: vi.fn(), + debug: vi.fn(), + child: vi.fn().mockReturnThis(), + }, })); // 2. Mock modules using the hoisted variables @@ -68,14 +76,10 @@ vi.mock('./db/admin.db', () => ({ return { logActivity: mocks.mockAdminLogActivity }; }), })); +// Use the hoisted shared mock logger instance so tests can verify calls vi.mock('./logger.server', () => ({ - logger: { - info: vi.fn(), - error: vi.fn(), - warn: vi.fn(), - debug: vi.fn(), - child: vi.fn().mockReturnThis(), - }, + logger: mocks.sharedMockLogger, + createScopedLogger: vi.fn(() => mocks.sharedMockLogger), })); vi.mock('./flyerFileHandler.server'); vi.mock('./flyerAiProcessor.server'); diff --git a/src/services/monitoringService.server.test.ts b/src/services/monitoringService.server.test.ts index 2fe1ef86..611d4be7 100644 --- a/src/services/monitoringService.server.test.ts +++ b/src/services/monitoringService.server.test.ts @@ -13,7 +13,14 @@ const mocks = vi.hoisted(() => { const createMockQueue = (name: string) => ({ name, - getJobCounts: vi.fn().mockResolvedValue({}), + getJobCounts: vi.fn().mockResolvedValue({ + waiting: 0, + active: 0, + completed: 0, + failed: 0, + delayed: 0, + paused: 0, + }), getJob: vi.fn(), }); @@ -23,22 +30,25 @@ const mocks = vi.hoisted(() => { analyticsWorker: createMockWorker('analytics-reporting'), cleanupWorker: createMockWorker('file-cleanup'), weeklyAnalyticsWorker: createMockWorker('weekly-analytics-reporting'), + tokenCleanupWorker: createMockWorker('token-cleanup'), flyerQueue: createMockQueue('flyer-processing'), emailQueue: createMockQueue('email-sending'), analyticsQueue: createMockQueue('analytics-reporting'), cleanupQueue: createMockQueue('file-cleanup'), weeklyAnalyticsQueue: createMockQueue('weekly-analytics-reporting'), + tokenCleanupQueue: createMockQueue('token-cleanup'), }; }); // --- Mock Modules --- -vi.mock('./queueService.server', () => ({ +vi.mock('./queues.server', () => ({ flyerQueue: mocks.flyerQueue, emailQueue: mocks.emailQueue, analyticsQueue: mocks.analyticsQueue, cleanupQueue: mocks.cleanupQueue, weeklyAnalyticsQueue: mocks.weeklyAnalyticsQueue, + tokenCleanupQueue: mocks.tokenCleanupQueue, })); vi.mock('./workers.server', () => ({ @@ -47,6 +57,8 @@ vi.mock('./workers.server', () => ({ analyticsWorker: mocks.analyticsWorker, cleanupWorker: mocks.cleanupWorker, weeklyAnalyticsWorker: mocks.weeklyAnalyticsWorker, + tokenCleanupWorker: mocks.tokenCleanupWorker, + flyerProcessingService: {}, })); vi.mock('./db/errors.db', () => ({ @@ -96,6 +108,7 @@ describe('MonitoringService', () => { { name: 'analytics-reporting', isRunning: true }, { name: 'file-cleanup', isRunning: true }, { name: 'weekly-analytics-reporting', isRunning: true }, + { name: 'token-cleanup', isRunning: true }, ]); expect(mocks.flyerWorker.isRunning).toHaveBeenCalledTimes(1); expect(mocks.emailWorker.isRunning).toHaveBeenCalledTimes(1); @@ -104,9 +117,22 @@ describe('MonitoringService', () => { describe('getQueueStatuses', () => { it('should return job counts for all queues', async () => { - // Arrange - mocks.flyerQueue.getJobCounts.mockResolvedValue({ active: 1, failed: 2 }); - mocks.emailQueue.getJobCounts.mockResolvedValue({ completed: 10, waiting: 5 }); + const defaultCounts = { + waiting: 0, + active: 0, + completed: 0, + failed: 0, + delayed: 0, + paused: 0, + }; + + // Arrange - override specific queue counts + mocks.flyerQueue.getJobCounts.mockResolvedValue({ ...defaultCounts, active: 1, failed: 2 }); + mocks.emailQueue.getJobCounts.mockResolvedValue({ + ...defaultCounts, + completed: 10, + waiting: 5, + }); // Act const statuses = await monitoringService.getQueueStatuses(); @@ -114,11 +140,12 @@ describe('MonitoringService', () => { // Assert expect(statuses).toEqual( expect.arrayContaining([ - { name: 'flyer-processing', counts: { active: 1, failed: 2 } }, - { name: 'email-sending', counts: { completed: 10, waiting: 5 } }, - { name: 'analytics-reporting', counts: {} }, - { name: 'file-cleanup', counts: {} }, - { name: 'weekly-analytics-reporting', counts: {} }, + { name: 'flyer-processing', counts: { ...defaultCounts, active: 1, failed: 2 } }, + { name: 'email-sending', counts: { ...defaultCounts, completed: 10, waiting: 5 } }, + { name: 'analytics-reporting', counts: defaultCounts }, + { name: 'file-cleanup', counts: defaultCounts }, + { name: 'weekly-analytics-reporting', counts: defaultCounts }, + { name: 'token-cleanup', counts: defaultCounts }, ]), ); expect(mocks.flyerQueue.getJobCounts).toHaveBeenCalledTimes(1); diff --git a/src/services/queueService.server.test.ts b/src/services/queueService.server.test.ts index 912875c0..2aacc62b 100644 --- a/src/services/queueService.server.test.ts +++ b/src/services/queueService.server.test.ts @@ -56,22 +56,58 @@ vi.mock('bullmq', () => ({ UnrecoverableError: class UnrecoverableError extends Error {}, })); -vi.mock('./logger.server', () => ({ - logger: { +vi.mock('./logger.server', () => { + // Mock logger factory that returns a new mock logger instance + const createMockLogger = () => ({ info: vi.fn(), error: vi.fn(), - warn: vi.fn(), // This was a duplicate, fixed. + warn: vi.fn(), debug: vi.fn(), child: vi.fn().mockReturnThis(), + trace: vi.fn(), + fatal: vi.fn(), + }); + + return { + logger: { + info: vi.fn(), + error: vi.fn(), + warn: vi.fn(), + debug: vi.fn(), + child: vi.fn().mockReturnThis(), + }, + // createScopedLogger is used by aiService.server and other services + createScopedLogger: vi.fn(() => createMockLogger()), + }; +}); + +// Mock the config/env module to prevent env parsing during tests +vi.mock('../config/env', () => ({ + config: { + database: { host: 'localhost', port: 5432, user: 'test', password: 'test', name: 'test' }, + redis: { url: 'redis://localhost:6379' }, + auth: { jwtSecret: 'test-secret' }, + server: { port: 3000, host: 'localhost' }, }, + isAiConfigured: vi.fn().mockReturnValue(false), + parseConfig: vi.fn(), })); // Mock other dependencies that are not the focus of this test file. vi.mock('./aiService.server'); vi.mock('./emailService.server'); -vi.mock('./db/index.db'); // This was a duplicate, fixed. +vi.mock('./db/index.db'); +vi.mock('./db/connection.db'); vi.mock('./flyerProcessingService.server'); vi.mock('./flyerDataTransformer'); +vi.mock('./flyerAiProcessor.server'); +vi.mock('./flyerPersistenceService.server'); +vi.mock('./flyerFileHandler.server'); +vi.mock('./analyticsService.server'); +vi.mock('./userService'); +vi.mock('./receiptService.server'); +vi.mock('./expiryService.server'); +vi.mock('./barcodeService.server'); describe('Worker Service Lifecycle', () => { let gracefulShutdown: (signal: string) => Promise; // This was a duplicate, fixed. @@ -229,9 +265,7 @@ describe('Worker Service Lifecycle', () => { expect(mockRedisConnection.quit).toHaveBeenCalledTimes(1); // Check for the correct success log message from workers.server.ts - expect(mockLogger.info).toHaveBeenCalledWith( - '[Shutdown] All resources closed successfully.', - ); + expect(mockLogger.info).toHaveBeenCalledWith('[Shutdown] All resources closed successfully.'); expect(processExitSpy).toHaveBeenCalledWith(0); }); diff --git a/src/services/queueService.test.ts b/src/services/queueService.test.ts index 62fb7a47..c424acc1 100644 --- a/src/services/queueService.test.ts +++ b/src/services/queueService.test.ts @@ -16,6 +16,9 @@ const mocks = vi.hoisted(() => { weeklyAnalyticsQueue: createMockQueue('weekly-analytics-reporting'), cleanupQueue: createMockQueue('file-cleanup'), tokenCleanupQueue: createMockQueue('token-cleanup'), + receiptQueue: createMockQueue('receipt-processing'), + expiryAlertQueue: createMockQueue('expiry-alerts'), + barcodeQueue: createMockQueue('barcode-detection'), redisConnection: { quit: vi.fn().mockResolvedValue('OK'), }, @@ -36,6 +39,9 @@ vi.mock('./queues.server', () => ({ weeklyAnalyticsQueue: mocks.weeklyAnalyticsQueue, cleanupQueue: mocks.cleanupQueue, tokenCleanupQueue: mocks.tokenCleanupQueue, + receiptQueue: mocks.receiptQueue, + expiryAlertQueue: mocks.expiryAlertQueue, + barcodeQueue: mocks.barcodeQueue, })); vi.mock('./redis.server', () => ({ @@ -76,6 +82,9 @@ describe('Queue Service (API Shutdown)', () => { expect(mocks.cleanupQueue.close).toHaveBeenCalledTimes(1); expect(mocks.weeklyAnalyticsQueue.close).toHaveBeenCalledTimes(1); expect(mocks.tokenCleanupQueue.close).toHaveBeenCalledTimes(1); + expect(mocks.receiptQueue.close).toHaveBeenCalledTimes(1); + expect(mocks.expiryAlertQueue.close).toHaveBeenCalledTimes(1); + expect(mocks.barcodeQueue.close).toHaveBeenCalledTimes(1); expect(mocks.redisConnection.quit).toHaveBeenCalledTimes(1); }); @@ -98,7 +107,9 @@ describe('Queue Service (API Shutdown)', () => { { err: closeError, resource: 'emailQueue' }, '[Shutdown] Error closing resource.', ); - expect(mocks.logger.warn).toHaveBeenCalledWith('[Shutdown] Graceful shutdown completed with errors.'); + expect(mocks.logger.warn).toHaveBeenCalledWith( + '[Shutdown] Graceful shutdown completed with errors.', + ); expect(processExitSpy).toHaveBeenCalledWith(1); }); @@ -112,7 +123,9 @@ describe('Queue Service (API Shutdown)', () => { { err: redisError, resource: 'redisConnection' }, '[Shutdown] Error closing resource.', ); - expect(mocks.logger.warn).toHaveBeenCalledWith('[Shutdown] Graceful shutdown completed with errors.'); + expect(mocks.logger.warn).toHaveBeenCalledWith( + '[Shutdown] Graceful shutdown completed with errors.', + ); expect(processExitSpy).toHaveBeenCalledWith(1); }); -}); \ No newline at end of file +}); diff --git a/src/services/queues.server.test.ts b/src/services/queues.server.test.ts index ad7a47ac..3d986d4b 100644 --- a/src/services/queues.server.test.ts +++ b/src/services/queues.server.test.ts @@ -112,8 +112,50 @@ describe('Queue Definitions', () => { }); }); - it('should create exactly 6 queues', () => { + it('should create receiptQueue with the correct name and options', () => { + expect(mocks.MockQueue).toHaveBeenCalledWith('receipt-processing', { + connection: mocks.mockConnection, + defaultJobOptions: { + attempts: 3, + backoff: { + type: 'exponential', + delay: 10000, + }, + removeOnComplete: 100, + removeOnFail: 50, + }, + }); + }); + + it('should create expiryAlertQueue with the correct name and options', () => { + expect(mocks.MockQueue).toHaveBeenCalledWith('expiry-alerts', { + connection: mocks.mockConnection, + defaultJobOptions: { + attempts: 2, + backoff: { type: 'exponential', delay: 300000 }, + removeOnComplete: true, + removeOnFail: 20, + }, + }); + }); + + it('should create barcodeQueue with the correct name and options', () => { + expect(mocks.MockQueue).toHaveBeenCalledWith('barcode-detection', { + connection: mocks.mockConnection, + defaultJobOptions: { + attempts: 2, + backoff: { + type: 'exponential', + delay: 5000, + }, + removeOnComplete: 50, + removeOnFail: 20, + }, + }); + }); + + it('should create exactly 9 queues', () => { // This is a good sanity check to ensure no new queues were added without tests. - expect(mocks.MockQueue).toHaveBeenCalledTimes(6); + expect(mocks.MockQueue).toHaveBeenCalledTimes(9); }); }); diff --git a/src/tests/mocks/zxing-wasm-reader.mock.ts b/src/tests/mocks/zxing-wasm-reader.mock.ts new file mode 100644 index 00000000..6c7e6fc7 --- /dev/null +++ b/src/tests/mocks/zxing-wasm-reader.mock.ts @@ -0,0 +1,11 @@ +// src/tests/mocks/zxing-wasm-reader.mock.ts +/** + * Mock for zxing-wasm/reader module. + * The actual module uses WebAssembly which doesn't work in jsdom test environment. + * This mock is aliased in vite.config.ts to replace the real module during unit tests. + */ + +export const readBarcodesFromImageData = async () => { + // Return empty array (no barcodes detected) + return []; +}; diff --git a/src/tests/setup/tests-setup-unit.ts b/src/tests/setup/tests-setup-unit.ts index 297b56ae..67ca1788 100644 --- a/src/tests/setup/tests-setup-unit.ts +++ b/src/tests/setup/tests-setup-unit.ts @@ -259,6 +259,50 @@ vi.mock('@google/genai', () => { }; }); +/** + * Mocks the barcode service module. + * This prevents the dynamic import of zxing-wasm/reader from failing in unit tests. + * The zxing-wasm package uses WebAssembly which isn't available in the jsdom test environment. + */ +vi.mock('../../services/barcodeService.server', () => ({ + detectBarcode: vi.fn().mockResolvedValue({ + detected: false, + upc_code: null, + confidence: null, + format: null, + error: null, + }), + processBarcodeDetectionJob: vi.fn().mockResolvedValue(undefined), + isValidUpcFormat: vi.fn().mockReturnValue(false), + calculateUpcCheckDigit: vi.fn().mockReturnValue(null), + validateUpcCheckDigit: vi.fn().mockReturnValue(false), + detectMultipleBarcodes: vi.fn().mockResolvedValue([]), + enhanceImageForDetection: vi.fn().mockImplementation((path: string) => Promise.resolve(path)), +})); + +/** + * Mocks the client-side config module. + * This prevents errors when sentry.client.ts tries to access config.sentry.dsn. + */ +vi.mock('../../config', () => ({ + default: { + app: { + version: '1.0.0-test', + commitMessage: 'test commit', + commitUrl: 'https://example.com', + }, + google: { + mapsEmbedApiKey: '', + }, + sentry: { + dsn: '', + environment: 'test', + debug: false, + enabled: false, + }, + }, +})); + // FIX: Mock the aiApiClient module as well, which is used by AnalysisPanel vi.mock('../../services/aiApiClient', () => ({ // Provide a default implementation that returns a valid Response object to prevent timeouts. @@ -297,7 +341,32 @@ vi.mock('@bull-board/express', () => ({ })); /** - * Mocks the logger. + * Mocks the Sentry client. + * This prevents errors when tests import modules that depend on sentry.client.ts. + */ +vi.mock('../../services/sentry.client', () => ({ + isSentryConfigured: false, + initSentry: vi.fn(), + captureException: vi.fn(), + captureMessage: vi.fn(), + setUser: vi.fn(), + addBreadcrumb: vi.fn(), + // Re-export a mock Sentry object for ErrorBoundary and other advanced usage + Sentry: { + init: vi.fn(), + captureException: vi.fn(), + captureMessage: vi.fn(), + setUser: vi.fn(), + setContext: vi.fn(), + addBreadcrumb: vi.fn(), + withScope: vi.fn(), + // Mock the ErrorBoundary component for React + ErrorBoundary: ({ children }: { children: React.ReactNode }) => children, + }, +})); + +/** + * Mocks the client-side logger. */ vi.mock('../../services/logger.client', () => ({ logger: { @@ -308,6 +377,34 @@ vi.mock('../../services/logger.client', () => ({ }, })); +/** + * Mocks the server-side logger. + * This mock provides both `logger` and `createScopedLogger` exports. + * Uses vi.hoisted to ensure the mock values are available during module import. + * IMPORTANT: Uses import() syntax to ensure correct path resolution for all importers. + */ +const { mockServerLogger, mockCreateScopedLogger } = vi.hoisted(() => { + const mockLogger = { + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + trace: vi.fn(), + fatal: vi.fn(), + child: vi.fn().mockReturnThis(), + level: 'debug', + }; + return { + mockServerLogger: mockLogger, + mockCreateScopedLogger: vi.fn(() => mockLogger), + }; +}); + +vi.mock(import('../../services/logger.server'), () => ({ + logger: mockServerLogger, + createScopedLogger: mockCreateScopedLogger, +})); + /** * Mocks the notification service. */ @@ -451,40 +548,57 @@ vi.mock('../../services/db/notification.db', async (importOriginal) => { // --- Server-Side Service Mocks --- -vi.mock('../../services/aiService.server', async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - // The singleton instance is named `aiService`. We mock the methods on it. - aiService: { - ...actual.aiService, // Spread original methods in case new ones are added - extractItemsFromReceiptImage: vi - .fn() - .mockResolvedValue([{ raw_item_description: 'Mock Receipt Item', price_paid_cents: 100 }]), - extractCoreDataFromFlyerImage: vi.fn().mockResolvedValue({ - store_name: 'Mock Store', - valid_from: '2023-01-01', - valid_to: '2023-01-07', - store_address: '123 Mock St', - items: [ - { - item: 'Mock Apple', - price_display: '$1.00', - price_in_cents: 100, - quantity: '1 lb', - category_name: 'Produce', - master_item_id: undefined, - }, - ], - }), - extractTextFromImageArea: vi.fn().mockImplementation((path, mime, crop, type) => { - if (type === 'address') return Promise.resolve({ text: '123 AI Street, Server City' }); - return Promise.resolve({ text: 'Mocked Extracted Text' }); - }), - planTripWithMaps: vi.fn().mockResolvedValue({ - text: 'Mocked trip plan.', - sources: [{ uri: 'http://maps.google.com/mock', title: 'Mock Map' }], - }), - }, - }; -}); +/** + * Mocks the AI service. + * IMPORTANT: This mock does NOT use `importOriginal` because aiService.server has + * complex dependencies (logger.server, etc.) that cause circular mock resolution issues. + * Instead, we provide a complete mock of the aiService singleton. + */ +vi.mock('../../services/aiService.server', () => ({ + aiService: { + extractItemsFromReceiptImage: vi + .fn() + .mockResolvedValue([{ raw_item_description: 'Mock Receipt Item', price_paid_cents: 100 }]), + extractCoreDataFromFlyerImage: vi.fn().mockResolvedValue({ + store_name: 'Mock Store', + valid_from: '2023-01-01', + valid_to: '2023-01-07', + store_address: '123 Mock St', + items: [ + { + item: 'Mock Apple', + price_display: '$1.00', + price_in_cents: 100, + quantity: '1 lb', + category_name: 'Produce', + master_item_id: undefined, + }, + ], + }), + extractTextFromImageArea: vi.fn().mockImplementation((path, mime, crop, type) => { + if (type === 'address') return Promise.resolve({ text: '123 AI Street, Server City' }); + return Promise.resolve({ text: 'Mocked Extracted Text' }); + }), + planTripWithMaps: vi.fn().mockResolvedValue({ + text: 'Mocked trip plan.', + sources: [{ uri: 'http://maps.google.com/mock', title: 'Mock Map' }], + }), + extractAndValidateData: vi.fn().mockResolvedValue({ + store_name: 'Mock Store', + valid_from: '2023-01-01', + valid_to: '2023-01-07', + store_address: '123 Mock St', + items: [], + }), + isImageAFlyer: vi.fn().mockResolvedValue(true), + }, + // Export the AIService class as a mock constructor for tests that need it + AIService: vi.fn().mockImplementation(() => ({ + extractItemsFromReceiptImage: vi.fn(), + extractCoreDataFromFlyerImage: vi.fn(), + extractTextFromImageArea: vi.fn(), + planTripWithMaps: vi.fn(), + extractAndValidateData: vi.fn(), + isImageAFlyer: vi.fn(), + })), +})); diff --git a/vite.config.ts b/vite.config.ts index 2651731b..6694011c 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -31,6 +31,9 @@ export default defineConfig({ // to the browser-safe client version during the Vite build process. // Server-side code should explicitly import 'services/logger.server'. 'services/logger': path.resolve(__dirname, './src/services/logger.client.ts'), + // Alias zxing-wasm/reader to a mock to prevent Vite import analysis errors + // The actual module uses WebAssembly which doesn't work in jsdom + 'zxing-wasm/reader': path.resolve(__dirname, './src/tests/mocks/zxing-wasm-reader.mock.ts'), }, }, @@ -42,6 +45,23 @@ export default defineConfig({ // The onConsoleLog hook is only needed if you want to conditionally filter specific logs. // Keeping the default behavior is often safer to avoid missing important warnings. environment: 'jsdom', + // Configure dependencies handling for test environment + deps: { + // Inline the zxing-wasm module to prevent import resolution errors + // The module uses dynamic imports and WASM which don't work in jsdom + optimizer: { + web: { + exclude: ['zxing-wasm'], + }, + }, + }, + // Configure server dependencies + server: { + deps: { + // Tell Vitest to not try to resolve these external modules + external: ['zxing-wasm', 'zxing-wasm/reader'], + }, + }, globals: true, // tsconfig is auto-detected, so the explicit property is not needed and causes an error. globalSetup: './src/tests/setup/global-setup.ts', // The globalApiMock MUST come first to ensure it's applied before other mocks that might depend on it.