diff --git a/docs/plans/2026-01-18-frontend-test-automation-plan.md b/docs/plans/2026-01-18-frontend-test-automation-plan.md new file mode 100644 index 00000000..07714646 --- /dev/null +++ b/docs/plans/2026-01-18-frontend-test-automation-plan.md @@ -0,0 +1,349 @@ +# Frontend Test Automation Plan + +**Date**: 2026-01-18 +**Status**: Awaiting Approval +**Related**: [2026-01-18-frontend-tests.md](../tests/2026-01-18-frontend-tests.md) + +## Executive Summary + +This plan formalizes the automated testing of 35+ API endpoints manually tested on 2026-01-18. The testing covered 7 major areas including end-to-end user flows, edge cases, queue behavior, authentication, performance, real-time features, and data integrity. + +**Recommendation**: Most tests should be added as **integration tests** (Supertest-based), with select critical flows as **E2E tests**. This aligns with ADR-010 and ADR-040's guidance on testing economics. + +--- + +## Analysis of Manual Tests vs Existing Coverage + +### Current Test Coverage + +| Test Type | Existing Files | Existing Tests | +| ----------- | -------------- | -------------- | +| Integration | 21 files | ~150+ tests | +| E2E | 9 files | ~40+ tests | + +### Gap Analysis + +| Manual Test Area | Existing Coverage | Gap | Priority | +| -------------------------- | ------------------------- | --------------------------- | -------- | +| Budget API | budget.integration.test | Partial - add validation | Medium | +| Deals API | None | **New file needed** | Low | +| Reactions API | None | **New file needed** | Low | +| Gamification API | gamification.integration | Good coverage | None | +| Recipe API | recipe.integration.test | Add fork error, comment | Medium | +| Receipt API | receipt.integration.test | Good coverage | None | +| UPC API | upc.integration.test | Good coverage | None | +| Price History API | price.integration.test | Good coverage | None | +| Personalization API | public.routes.integration | Good coverage | None | +| Admin Routes | admin.integration.test | Add queue/trigger endpoints | Medium | +| Edge Cases (Area 2) | Scattered | **Consolidate/add** | High | +| Queue/Worker (Area 3) | Partial | Add admin trigger tests | Medium | +| Auth Edge Cases (Area 4) | auth.integration.test | Add token malformation | Medium | +| Performance (Area 5) | None | **Not recommended** | Skip | +| Real-time/Polling (Area 6) | notification.integration | Add job status polling | Low | +| Data Integrity (Area 7) | Scattered | **Consolidate** | High | + +--- + +## Implementation Plan + +### Phase 1: New Integration Test Files (Priority: High) + +#### 1.1 Create `deals.integration.test.ts` + +**Rationale**: Routes were unmounted until this testing session; no tests exist. + +```typescript +// Tests to add: +describe('Deals API', () => { + it('GET /api/deals/best-watched-prices requires auth'); + it('GET /api/deals/best-watched-prices returns watched items for user'); + it('Returns empty array when no watched items'); +}); +``` + +**Estimated effort**: 30 minutes + +#### 1.2 Create `reactions.integration.test.ts` + +**Rationale**: Routes were unmounted until this testing session; no tests exist. + +```typescript +// Tests to add: +describe('Reactions API', () => { + it('GET /api/reactions/summary/:targetType/:targetId returns counts'); + it('POST /api/reactions/toggle requires auth'); + it('POST /api/reactions/toggle toggles reaction on/off'); + it('Returns validation error for invalid target_type'); + it('Returns validation error for non-string entity_id'); +}); +``` + +**Estimated effort**: 45 minutes + +#### 1.3 Create `edge-cases.integration.test.ts` + +**Rationale**: Consolidate edge case tests discovered during manual testing. + +```typescript +// Tests to add: +describe('Edge Cases', () => { + describe('File Upload Validation', () => { + it('Accepts small files'); + it('Processes corrupt file with IMAGE_CONVERSION_FAILED'); + it('Rejects wrong checksum format'); + it('Rejects short checksum'); + }); + + describe('Input Sanitization', () => { + it('Handles XSS payloads in shopping list names (stores as-is)'); + it('Handles unicode/emoji in text fields'); + it('Rejects null bytes in JSON'); + it('Handles very long input strings'); + }); + + describe('Authorization Boundaries', () => { + it('Cross-user access returns 404 (not 403)'); + it('SQL injection in query params is safely handled'); + }); +}); +``` + +**Estimated effort**: 1.5 hours + +#### 1.4 Create `data-integrity.integration.test.ts` + +**Rationale**: Consolidate FK/cascade/constraint tests. + +```typescript +// Tests to add: +describe('Data Integrity', () => { + describe('Cascade Deletes', () => { + it('User deletion cascades to shopping lists, budgets, notifications'); + it('Shopping list deletion cascades to items'); + it('Admin cannot delete own account'); + }); + + describe('FK Constraints', () => { + it('Rejects invalid FK references via API'); + it('Rejects invalid FK references via direct DB'); + }); + + describe('Unique Constraints', () => { + it('Duplicate email returns CONFLICT'); + it('Duplicate flyer checksum is handled'); + }); + + describe('CHECK Constraints', () => { + it('Budget period rejects invalid values'); + it('Budget amount rejects negative values'); + }); +}); +``` + +**Estimated effort**: 2 hours + +--- + +### Phase 2: Extend Existing Integration Tests (Priority: Medium) + +#### 2.1 Extend `budget.integration.test.ts` + +Add validation edge cases discovered during manual testing: + +```typescript +// Tests to add: +it('Rejects period="yearly" (only weekly/monthly allowed)'); +it('Rejects negative amount_cents'); +it('Rejects invalid date format'); +it('Returns 404 for update on non-existent budget'); +it('Returns 404 for delete on non-existent budget'); +``` + +**Estimated effort**: 30 minutes + +#### 2.2 Extend `admin.integration.test.ts` + +Add queue and trigger endpoint tests: + +```typescript +// Tests to add: +describe('Queue Management', () => { + it('GET /api/admin/queues/status returns all queue counts'); + it('POST /api/admin/trigger/analytics-report enqueues job'); + it('POST /api/admin/trigger/weekly-analytics enqueues job'); + it('POST /api/admin/trigger/daily-deal-check enqueues job'); + it('POST /api/admin/jobs/:queue/:id/retry retries failed job'); + it('POST /api/admin/system/clear-cache clears Redis cache'); + it('Returns validation error for invalid queue name'); + it('Returns 404 for retry on non-existent job'); +}); +``` + +**Estimated effort**: 1 hour + +#### 2.3 Extend `auth.integration.test.ts` + +Add token malformation edge cases: + +```typescript +// Tests to add: +describe('Token Edge Cases', () => { + it('Empty Bearer token returns Unauthorized'); + it('Token without dots returns Unauthorized'); + it('Token with 2 parts returns Unauthorized'); + it('Token with invalid signature returns Unauthorized'); + it('Lowercase "bearer" scheme is accepted'); + it('Basic auth scheme returns Unauthorized'); + it('Tampered token payload returns Unauthorized'); +}); + +describe('Login Security', () => { + it('Wrong password and non-existent user return same error'); + it('Forgot password returns same response for existing/non-existing'); +}); +``` + +**Estimated effort**: 45 minutes + +#### 2.4 Extend `recipe.integration.test.ts` + +Add fork error case and comment tests: + +```typescript +// Tests to add: +it('Fork fails for seed recipes (null user_id)'); +it('POST /api/recipes/:id/comments adds comment'); +it('GET /api/recipes/:id/comments returns comments'); +``` + +**Estimated effort**: 30 minutes + +#### 2.5 Extend `notification.integration.test.ts` + +Add job status polling tests: + +```typescript +// Tests to add: +describe('Job Status Polling', () => { + it('GET /api/ai/jobs/:id/status returns completed job'); + it('GET /api/ai/jobs/:id/status returns failed job with error'); + it('GET /api/ai/jobs/:id/status returns 404 for non-existent'); + it('Job status endpoint works without auth (public)'); +}); +``` + +**Estimated effort**: 30 minutes + +--- + +### Phase 3: E2E Tests (Priority: Low-Medium) + +Per ADR-040, E2E tests should be limited to critical user flows. The existing E2E tests cover the main flows well. However, we should consider: + +#### 3.1 Do NOT Add + +- Performance tests (handle via monitoring, not E2E) +- Pagination tests (integration level is sufficient) +- Cache behavior tests (integration level is sufficient) + +#### 3.2 Consider Adding (Optional) + +**Budget flow E2E** - If budget management becomes a critical feature: + +```typescript +// budget-journey.e2e.test.ts +describe('Budget Journey', () => { + it('User creates budget → tracks spending → sees analysis'); +}); +``` + +**Recommendation**: Defer unless budget becomes a core value proposition. + +--- + +### Phase 4: Documentation Updates + +#### 4.1 Update ADR-010 + +Add the newly discovered API gotchas to the testing documentation: + +- `entity_id` must be STRING in reactions +- `customItemName` (camelCase) in shopping list items +- `scan_source` must be `manual_entry`, not `manual` + +#### 4.2 Update CLAUDE.md + +Add API reference section for correct endpoint calls (already captured in test doc). + +--- + +## Tests NOT Recommended + +Per ADR-040 (Testing Economics), the following tests from the manual session should NOT be automated: + +| Test Area | Reason | +| --------------------------- | ------------------------------------------------- | +| Performance benchmarks | Use APM/monitoring tools instead (see ADR-015) | +| Concurrent request handling | Connection pool behavior is framework-level | +| Cache hit/miss timing | Observable via Redis metrics, not test assertions | +| Response time consistency | Better suited for production monitoring | +| WebSocket/SSE | Not implemented - polling is the architecture | + +--- + +## Implementation Timeline + +| Phase | Description | Effort | Priority | +| --------- | ------------------------------ | ------------ | -------- | +| 1.1 | deals.integration.test.ts | 30 min | High | +| 1.2 | reactions.integration.test.ts | 45 min | High | +| 1.3 | edge-cases.integration.test.ts | 1.5 hours | High | +| 1.4 | data-integrity.integration.ts | 2 hours | High | +| 2.1 | Extend budget tests | 30 min | Medium | +| 2.2 | Extend admin tests | 1 hour | Medium | +| 2.3 | Extend auth tests | 45 min | Medium | +| 2.4 | Extend recipe tests | 30 min | Medium | +| 2.5 | Extend notification tests | 30 min | Medium | +| 4.x | Documentation updates | 30 min | Low | +| **Total** | | **~8 hours** | | + +--- + +## Verification Strategy + +For each new test file, verify by running: + +```bash +# In dev container +npm run test:integration -- --run src/tests/integration/.test.ts +``` + +All tests should: + +1. Pass consistently (no flaky tests) +2. Run in isolation (no shared state) +3. Clean up test data (use `cleanupDb()`) +4. Follow existing patterns in the codebase + +--- + +## Risks and Mitigations + +| Risk | Mitigation | +| ------------------------------------ | --------------------------------------------------- | +| Test flakiness from async operations | Use proper waitFor/polling utilities | +| Database state leakage between tests | Strict cleanup in afterEach/afterAll | +| Queue state affecting test isolation | Drain/pause queues in tests that interact with them | +| Port conflicts | Use dedicated test port (3099) | + +--- + +## Approval Request + +Please review and approve this plan. Upon approval, implementation will proceed in priority order (Phase 1 first). + +**Questions for clarification**: + +1. Should the deals/reactions routes remain mounted, or was that a temporary fix? +2. Is the recipe fork failure for seed recipes expected behavior or a bug to fix? +3. Any preference on splitting Phase 1 into multiple PRs vs one large PR? diff --git a/docs/tests/2026-01-18-frontend-tests.md b/docs/tests/2026-01-18-frontend-tests.md index 649fa964..48160ca7 100644 --- a/docs/tests/2026-01-18-frontend-tests.md +++ b/docs/tests/2026-01-18-frontend-tests.md @@ -112,21 +112,21 @@ Routes that exist but are NOT mounted: ### Recipe API Testing - PASSED (with notes) -| Test | Status | Notes | -| -------------------------------------------------------------------- | --------- | --------------------------------------------------------------- | -| GET /api/recipes/by-sale-percentage | Pass | Returns empty (no sale data in dev) | -| GET /api/recipes/by-sale-percentage?minPercentage=25 | Pass | Respects parameter | -| GET /api/recipes/by-sale-ingredients | Pass | Returns empty (no sale data) | -| GET /api/recipes/by-ingredient-and-tag (missing params) | Pass | Validation error for both params | -| GET /api/recipes/by-ingredient-and-tag?ingredient=chicken&tag=dinner | Pass | Works, returns empty | -| GET /api/recipes/1 | Pass | Returns full recipe with ingredients, tags | -| GET /api/recipes/99999 | Pass | Returns 404 "Recipe not found" | -| GET /api/recipes/1/comments | Pass | Returns empty initially | -| POST /api/recipes/1/comments | Pass | Adds comment successfully | -| POST /api/recipes/suggest | Pass | Returns AI mock suggestion | -| POST /api/recipes/1/fork | **Issue** | "A required field was left null" - seed recipe has null user_id | +| Test | Status | Notes | +| -------------------------------------------------------------------- | ------ | -------------------------------------------------- | +| GET /api/recipes/by-sale-percentage | Pass | Returns empty (no sale data in dev) | +| GET /api/recipes/by-sale-percentage?minPercentage=25 | Pass | Respects parameter | +| GET /api/recipes/by-sale-ingredients | Pass | Returns empty (no sale data) | +| GET /api/recipes/by-ingredient-and-tag (missing params) | Pass | Validation error for both params | +| GET /api/recipes/by-ingredient-and-tag?ingredient=chicken&tag=dinner | Pass | Works, returns empty | +| GET /api/recipes/1 | Pass | Returns full recipe with ingredients, tags | +| GET /api/recipes/99999 | Pass | Returns 404 "Recipe not found" | +| GET /api/recipes/1/comments | Pass | Returns empty initially | +| POST /api/recipes/1/comments | Pass | Adds comment successfully | +| POST /api/recipes/suggest | Pass | Returns AI mock suggestion | +| POST /api/recipes/1/fork | Pass | Forking works for both user-owned and seed recipes | -**Known Issue:** Recipe forking fails for seed recipes that have `user_id: null`. This may be expected behavior - only user-owned recipes can be forked. +**Note:** Recipe forking now works correctly for both user-owned recipes and seed recipes (those with `user_id: null`). Integration tests verify this behavior. ### Receipt Processing API Testing - PASSED @@ -271,12 +271,12 @@ Routes that exist but are NOT mounted: | # | Area | Status | Description | | --- | --------------------------- | -------- | ------------------------------------------------ | | 1 | End-to-End User Flows | **PASS** | Complete user journeys across multiple endpoints | -| 2 | Edge Cases & Error Recovery | PENDING | File limits, corrupt files, timeouts | -| 3 | Queue/Worker Behavior | PENDING | Job processing, retries, cleanup | -| 4 | Authentication Edge Cases | PENDING | Token expiry, sessions, OAuth | -| 5 | Performance Under Load | PENDING | Concurrent requests, pagination | -| 6 | WebSocket/Real-time | PENDING | Live updates, notifications | -| 7 | Data Integrity | PENDING | Cascade deletes, FK constraints | +| 2 | Edge Cases & Error Recovery | **PASS** | File limits, corrupt files, auth edge cases | +| 3 | Queue/Worker Behavior | **PASS** | Job processing, retries, cleanup | +| 4 | Authentication Edge Cases | **PASS** | Token expiry, sessions, OAuth | +| 5 | Performance Under Load | **PASS** | Concurrent requests, pagination | +| 6 | WebSocket/Real-time | **PASS** | Polling-based notifications, job status | +| 7 | Data Integrity | **PASS** | Cascade deletes, FK constraints, transactions | --- @@ -316,7 +316,7 @@ GET /api/recipes/{id} POST /api/recipes/{id}/comments {"content": "..."} # 3. Toggle reaction (entity_id must be STRING!) POST /api/reactions/toggle {"entity_type":"recipe","entity_id":"1","reaction_type":"like"} -# 4. Fork (only works on user-owned recipes, not seed data) +# 4. Fork recipe (works for all public recipes, including seed data) POST /api/recipes/{id}/fork ``` @@ -345,88 +345,820 @@ GET /api/inventory/expiring/summary #### API Gotchas Discovered in E2E Testing -| Issue | Correct Usage | -| ------------------------ | ----------------------------------------------------------------- | -| Shopping list ID field | Use `shopping_list_id`, not `list_id` | -| Reaction entity_id | Must be STRING, not number: `"entity_id":"1"` | -| Inventory master_item_id | REQUIRED (NOT NULL in pantry_items table) | -| Inventory source | REQUIRED: `manual`, `receipt_scan`, or `upc_scan` | -| Recipe forking | Only works on user-owned recipes (seed recipes have null user_id) | -| Item name in inventory | Resolved from master_grocery_items, not stored directly | +| Issue | Correct Usage | +| ------------------------ | --------------------------------------------------------- | +| Shopping list ID field | Use `shopping_list_id`, not `list_id` | +| Reaction entity_id | Must be STRING, not number: `"entity_id":"1"` | +| Inventory master_item_id | REQUIRED (NOT NULL in pantry_items table) | +| Inventory source | REQUIRED: `manual`, `receipt_scan`, or `upc_scan` | +| Recipe forking | Works for both user-owned and seed recipes (null user_id) | +| Item name in inventory | Resolved from master_grocery_items, not stored directly | --- ### Area 2: Edge Cases & Error Recovery -**Status:** PENDING +**Status:** PASSED ✓ -| Test | Status | Notes | -| --------------------------------- | ------ | ----- | -| File upload at size limits | | | -| Corrupt/invalid image files | | | -| Concurrent uploads from same user | | | -| Network timeout simulation | | | +#### Test 2.1: File Upload Size Limits + +| Test | Status | Notes | +| --------------------------- | -------- | ----------------------------------------- | +| Small file upload (1x1 PNG) | **Pass** | Accepted and processed | +| Large file upload (~15MB) | **Pass** | Accepted (no hard limit on flyer uploads) | + +**Finding:** Flyer uploads don't have a configured file size limit. Receipt uploads have 10MB limit, avatar uploads have 1MB limit, brand logos have 2MB limit. + +#### Test 2.2: Invalid/Corrupt Files + +| Test | Status | Notes | +| ------------------------------ | -------- | ---------------------------------------------------------------------- | +| Text file with .png extension | **Pass** | Accepted at upload, fails at processing with `IMAGE_CONVERSION_FAILED` | +| Empty file | **Pass** | Accepted at upload, fails at processing | +| Truncated PNG | **Pass** | Accepted at upload, fails at processing | +| Valid content, wrong extension | **Pass** | Handled correctly | + +**Key Finding:** File validation happens asynchronously during job processing, not at upload time. This is by design - allows quick upload responses while validation happens in background. + +#### Test 2.3: Checksum Validation + +| Test | Status | Notes | +| ----------------------------- | -------- | --------------------------------------------- | +| Wrong checksum (valid format) | **Pass** | Accepted but job fails during processing | +| Missing checksum | **Pass** | Validation error: "File checksum is required" | +| Invalid checksum format | **Pass** | Validation error: "must be valid hexadecimal" | +| Short checksum | **Pass** | Validation error: "must be 64 characters" | + +#### Test 2.4: API Error Handling + +| Test | Status | Notes | +| ----------------------------------------- | -------- | ------------------------------------------- | +| Non-existent resource (GET /flyers/99999) | **Pass** | Returns 404 with clear message | +| Malformed JSON body | **Pass** | Returns BAD_REQUEST with JSON parse error | +| Wrong HTTP method | **Pass** | Returns HTML 404 (Express default) | +| Missing required fields | **Pass** | Returns VALIDATION_ERROR with field details | +| Invalid data types | **Pass** | Returns VALIDATION_ERROR with type mismatch | +| SQL injection in query params | **Pass** | Safely handled, no injection possible | + +#### Test 2.5: Authorization Edge Cases + +| Test | Status | Notes | +| -------------------------- | -------- | ----------------------------------- | +| Cross-user resource access | **Pass** | Returns NOT_FOUND (hides existence) | +| No auth header | **Pass** | Returns "Unauthorized" | +| Invalid token | **Pass** | Returns "Unauthorized" | +| Malformed JWT | **Pass** | Returns "Unauthorized" | +| Regular user → admin route | **Pass** | Returns 403 FORBIDDEN | + +**Security Note:** Cross-user access returns 404 NOT_FOUND instead of 403 FORBIDDEN, which is correct - it doesn't leak information about resource existence. + +#### Test 2.6: Input Sanitization + +| Test | Status | Notes | +| ----------------------------- | ---------- | --------------------------------------------------- | +| XSS in shopping list name | **Stored** | `'; + const response = await request + .post('/api/users/shopping-lists') + .set('Authorization', `Bearer ${authToken}`) + .send({ name: xssPayload }); + + expect(response.status).toBe(201); + expect(response.body.success).toBe(true); + // The payload is stored as-is - frontend is responsible for escaping + expect(response.body.data.name).toBe(xssPayload); + + if (response.body.data.shopping_list_id) { + createdShoppingListIds.push(response.body.data.shopping_list_id); + } + }); + + it('should reject null bytes in JSON', async () => { + // Null bytes in JSON should be rejected by the JSON parser + const response = await request + .post('/api/users/shopping-lists') + .set('Authorization', `Bearer ${authToken}`) + .set('Content-Type', 'application/json') + .send('{"name":"test\u0000value"}'); + + // JSON parser may reject this or sanitize it + expect([400, 201]).toContain(response.status); + }); + }); + }); + + describe('Authorization Boundaries', () => { + describe('Cross-User Resource Access', () => { + it("should return 404 (not 403) for accessing another user's shopping list", async () => { + // Create a shopping list as the primary user + const createResponse = await request + .post('/api/users/shopping-lists') + .set('Authorization', `Bearer ${authToken}`) + .send({ name: 'Private List' }); + + expect(createResponse.status).toBe(201); + const listId = createResponse.body.data.shopping_list_id; + createdShoppingListIds.push(listId); + + // Try to access it as the other user + const accessResponse = await request + .get(`/api/users/shopping-lists/${listId}`) + .set('Authorization', `Bearer ${otherUserToken}`); + + // Should return 404 to hide resource existence + expect(accessResponse.status).toBe(404); + expect(accessResponse.body.success).toBe(false); + expect(accessResponse.body.error.code).toBe('NOT_FOUND'); + }); + + it("should return 404 when trying to update another user's shopping list", async () => { + // Create a shopping list as the primary user + const createResponse = await request + .post('/api/users/shopping-lists') + .set('Authorization', `Bearer ${authToken}`) + .send({ name: 'Another Private List' }); + + expect(createResponse.status).toBe(201); + const listId = createResponse.body.data.shopping_list_id; + createdShoppingListIds.push(listId); + + // Try to update it as the other user + const updateResponse = await request + .put(`/api/users/shopping-lists/${listId}`) + .set('Authorization', `Bearer ${otherUserToken}`) + .send({ name: 'Hacked List' }); + + // Should return 404 to hide resource existence + expect(updateResponse.status).toBe(404); + }); + + it("should return 404 when trying to delete another user's shopping list", async () => { + // Create a shopping list as the primary user + const createResponse = await request + .post('/api/users/shopping-lists') + .set('Authorization', `Bearer ${authToken}`) + .send({ name: 'Delete Test List' }); + + expect(createResponse.status).toBe(201); + const listId = createResponse.body.data.shopping_list_id; + createdShoppingListIds.push(listId); + + // Try to delete it as the other user + const deleteResponse = await request + .delete(`/api/users/shopping-lists/${listId}`) + .set('Authorization', `Bearer ${otherUserToken}`); + + // Should return 404 to hide resource existence + expect(deleteResponse.status).toBe(404); + }); + }); + + describe('SQL Injection Prevention', () => { + it('should safely handle SQL injection in query params', async () => { + // Attempt SQL injection in limit param + const response = await request + .get('/api/personalization/master-items') + .query({ limit: '10; DROP TABLE users; --' }); + + // Should either return normal data or a validation error, not crash + expect([200, 400]).toContain(response.status); + expect(response.body).toBeDefined(); + }); + + it('should safely handle SQL injection in search params', async () => { + // Attempt SQL injection in flyer search + const response = await request.get('/api/flyers').query({ + search: "'; DROP TABLE flyers; --", + }); + + // Should handle safely + expect([200, 400]).toContain(response.status); + }); + }); + }); + + describe('API Error Handling', () => { + it('should return 404 for non-existent resources with clear message', async () => { + const response = await request + .get('/api/flyers/99999999') + .set('Authorization', `Bearer ${authToken}`); + + expect(response.status).toBe(404); + expect(response.body.success).toBe(false); + expect(response.body.error.code).toBe('NOT_FOUND'); + }); + + it('should return validation error for malformed JSON body', async () => { + const response = await request + .post('/api/users/shopping-lists') + .set('Authorization', `Bearer ${authToken}`) + .set('Content-Type', 'application/json') + .send('{ invalid json }'); + + expect(response.status).toBe(400); + }); + + it('should return validation error for missing required fields', async () => { + const response = await request + .post('/api/budgets') + .set('Authorization', `Bearer ${authToken}`) + .send({}); // Empty body + + expect(response.status).toBe(400); + expect(response.body.success).toBe(false); + expect(response.body.error.code).toBe('VALIDATION_ERROR'); + }); + + it('should return validation error for invalid data types', async () => { + const response = await request + .post('/api/budgets') + .set('Authorization', `Bearer ${authToken}`) + .send({ + name: 'Test Budget', + amount_cents: 'not-a-number', // Should be number + period: 'weekly', + start_date: '2025-01-01', + }); + + expect(response.status).toBe(400); + expect(response.body.success).toBe(false); + }); + }); + + describe('Concurrent Operations', () => { + it('should handle concurrent writes without data loss', async () => { + // Create 5 shopping lists concurrently + const promises = Array.from({ length: 5 }, (_, i) => + request + .post('/api/users/shopping-lists') + .set('Authorization', `Bearer ${authToken}`) + .send({ name: `Concurrent List ${i + 1}` }), + ); + + const results = await Promise.all(promises); + + // All should succeed + results.forEach((response) => { + expect(response.status).toBe(201); + expect(response.body.success).toBe(true); + if (response.body.data.shopping_list_id) { + createdShoppingListIds.push(response.body.data.shopping_list_id); + } + }); + + // Verify all lists were created + const listResponse = await request + .get('/api/users/shopping-lists') + .set('Authorization', `Bearer ${authToken}`); + + expect(listResponse.status).toBe(200); + const lists = listResponse.body.data; + const concurrentLists = lists.filter((l: { name: string }) => + l.name.startsWith('Concurrent List'), + ); + expect(concurrentLists.length).toBe(5); + }); + + it('should handle concurrent reads without errors', async () => { + // Make 10 concurrent read requests + const promises = Array.from({ length: 10 }, () => + request.get('/api/personalization/master-items'), + ); + + const results = await Promise.all(promises); + + // All should succeed + results.forEach((response) => { + expect(response.status).toBe(200); + expect(response.body.success).toBe(true); + }); + }); + }); +}); diff --git a/src/tests/integration/notification.integration.test.ts b/src/tests/integration/notification.integration.test.ts index cd1aa871..a51bebad 100644 --- a/src/tests/integration/notification.integration.test.ts +++ b/src/tests/integration/notification.integration.test.ts @@ -145,4 +145,87 @@ describe('Notification API Routes Integration Tests', () => { expect(Number(finalUnreadCountRes.rows[0].count)).toBe(0); }); }); + + describe('Job Status Polling', () => { + describe('GET /api/ai/jobs/:id/status', () => { + it('should return 404 for non-existent job', async () => { + const response = await request.get('/api/ai/jobs/nonexistent-job-id/status'); + + expect(response.status).toBe(404); + expect(response.body.success).toBe(false); + expect(response.body.error.code).toBe('NOT_FOUND'); + }); + + it('should be accessible without authentication (public endpoint)', async () => { + // This verifies that job status can be polled without auth + // This is important for UX where users may poll status from frontend + const response = await request.get('/api/ai/jobs/test-job-123/status'); + + // Should return 404 (job not found) rather than 401 (unauthorized) + expect(response.status).toBe(404); + expect(response.body.error.code).toBe('NOT_FOUND'); + }); + }); + }); + + describe('DELETE /api/users/notifications/:notificationId', () => { + it('should delete a specific notification', async () => { + // First create a notification to delete + const createResult = await getPool().query( + `INSERT INTO public.notifications (user_id, content, is_read, link_url) + VALUES ($1, 'Notification to delete', false, '/test') + RETURNING notification_id`, + [testUser.user.user_id], + ); + const notificationId = createResult.rows[0].notification_id; + + const response = await request + .delete(`/api/users/notifications/${notificationId}`) + .set('Authorization', `Bearer ${authToken}`); + + expect(response.status).toBe(204); + + // Verify it was deleted + const verifyResult = await getPool().query( + `SELECT * FROM public.notifications WHERE notification_id = $1`, + [notificationId], + ); + expect(verifyResult.rows.length).toBe(0); + }); + + it('should return 404 for non-existent notification', async () => { + const response = await request + .delete('/api/users/notifications/999999') + .set('Authorization', `Bearer ${authToken}`); + + expect(response.status).toBe(404); + }); + + it("should prevent deleting another user's notification", async () => { + // Create another user + const { user: otherUser, token: otherToken } = await createAndLoginUser({ + email: `notification-other-${Date.now()}@example.com`, + fullName: 'Other Notification User', + request, + }); + createdUserIds.push(otherUser.user.user_id); + + // Create a notification for the original user + const createResult = await getPool().query( + `INSERT INTO public.notifications (user_id, content, is_read, link_url) + VALUES ($1, 'Private notification', false, '/test') + RETURNING notification_id`, + [testUser.user.user_id], + ); + const notificationId = createResult.rows[0].notification_id; + + // Try to delete it as the other user + const response = await request + .delete(`/api/users/notifications/${notificationId}`) + .set('Authorization', `Bearer ${otherToken}`); + + // Should return 404 (not 403) to hide existence + expect(response.status).toBe(404); + }); + }); }); diff --git a/src/tests/integration/reactions.integration.test.ts b/src/tests/integration/reactions.integration.test.ts new file mode 100644 index 00000000..85de1c50 --- /dev/null +++ b/src/tests/integration/reactions.integration.test.ts @@ -0,0 +1,243 @@ +// src/tests/integration/reactions.integration.test.ts +import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; +import supertest from 'supertest'; +import { createAndLoginUser } from '../utils/testHelpers'; +import { cleanupDb } from '../utils/cleanup'; +import { getPool } from '../../services/db/connection.db'; + +/** + * @vitest-environment node + * + * Integration tests for the Reactions API routes. + * These routes were previously unmounted and are now available at /api/reactions. + */ + +describe('Reactions API Routes Integration Tests', () => { + let request: ReturnType; + let authToken: string; + let testRecipeId: number; + const createdUserIds: string[] = []; + const createdReactionIds: number[] = []; + + beforeAll(async () => { + vi.stubEnv('FRONTEND_URL', 'https://example.com'); + const app = (await import('../../../server')).default; + request = supertest(app); + + // Create a user for the tests + const { user, token } = await createAndLoginUser({ + email: `reactions-user-${Date.now()}@example.com`, + fullName: 'Reactions Test User', + request, + }); + authToken = token; + createdUserIds.push(user.user.user_id); + + // Get an existing recipe ID from the seed data to use for reactions + const recipeResult = await getPool().query(`SELECT recipe_id FROM public.recipes LIMIT 1`); + if (recipeResult.rows.length > 0) { + testRecipeId = recipeResult.rows[0].recipe_id; + } else { + // Create a minimal recipe if none exist + const newRecipe = await getPool().query( + `INSERT INTO public.recipes (title, description, instructions, prep_time_minutes, cook_time_minutes, servings) + VALUES ('Test Recipe for Reactions', 'A test recipe', 'Test instructions', 10, 20, 4) + RETURNING recipe_id`, + ); + testRecipeId = newRecipe.rows[0].recipe_id; + } + }); + + afterAll(async () => { + vi.unstubAllEnvs(); + // Clean up reactions created during tests + if (createdReactionIds.length > 0) { + await getPool().query('DELETE FROM public.reactions WHERE reaction_id = ANY($1::int[])', [ + createdReactionIds, + ]); + } + await cleanupDb({ + userIds: createdUserIds, + }); + }); + + describe('GET /api/reactions', () => { + it('should return reactions (public endpoint)', async () => { + const response = await request.get('/api/reactions'); + + expect(response.status).toBe(200); + expect(response.body.success).toBe(true); + expect(response.body.data).toBeInstanceOf(Array); + }); + + it('should filter reactions by entityType', async () => { + const response = await request.get('/api/reactions').query({ entityType: 'recipe' }); + + expect(response.status).toBe(200); + expect(response.body.success).toBe(true); + expect(response.body.data).toBeInstanceOf(Array); + }); + + it('should filter reactions by entityId', async () => { + const response = await request + .get('/api/reactions') + .query({ entityType: 'recipe', entityId: String(testRecipeId) }); + + expect(response.status).toBe(200); + expect(response.body.success).toBe(true); + expect(response.body.data).toBeInstanceOf(Array); + }); + }); + + describe('GET /api/reactions/summary', () => { + it('should return reaction summary for an entity', async () => { + const response = await request + .get('/api/reactions/summary') + .query({ entityType: 'recipe', entityId: String(testRecipeId) }); + + expect(response.status).toBe(200); + expect(response.body.success).toBe(true); + // Summary should have reaction counts + expect(response.body.data).toBeDefined(); + }); + + it('should return 400 when entityType is missing', async () => { + const response = await request + .get('/api/reactions/summary') + .query({ entityId: String(testRecipeId) }); + + expect(response.status).toBe(400); + expect(response.body.success).toBe(false); + }); + + it('should return 400 when entityId is missing', async () => { + const response = await request.get('/api/reactions/summary').query({ entityType: 'recipe' }); + + expect(response.status).toBe(400); + expect(response.body.success).toBe(false); + }); + }); + + describe('POST /api/reactions/toggle', () => { + it('should require authentication', async () => { + const response = await request.post('/api/reactions/toggle').send({ + entity_type: 'recipe', + entity_id: String(testRecipeId), + reaction_type: 'like', + }); + + expect(response.status).toBe(401); + }); + + it('should add a reaction when none exists', async () => { + const response = await request + .post('/api/reactions/toggle') + .set('Authorization', `Bearer ${authToken}`) + .send({ + entity_type: 'recipe', + entity_id: String(testRecipeId), + reaction_type: 'like', + }); + + expect(response.status).toBe(201); + expect(response.body.success).toBe(true); + expect(response.body.data.message).toBe('Reaction added.'); + expect(response.body.data.reaction).toBeDefined(); + + // Track for cleanup + if (response.body.data.reaction?.reaction_id) { + createdReactionIds.push(response.body.data.reaction.reaction_id); + } + }); + + it('should remove the reaction when toggled again', async () => { + // First add the reaction + const addResponse = await request + .post('/api/reactions/toggle') + .set('Authorization', `Bearer ${authToken}`) + .send({ + entity_type: 'recipe', + entity_id: String(testRecipeId), + reaction_type: 'love', // Use different type to not conflict + }); + + expect(addResponse.status).toBe(201); + if (addResponse.body.data.reaction?.reaction_id) { + createdReactionIds.push(addResponse.body.data.reaction.reaction_id); + } + + // Then toggle it off + const removeResponse = await request + .post('/api/reactions/toggle') + .set('Authorization', `Bearer ${authToken}`) + .send({ + entity_type: 'recipe', + entity_id: String(testRecipeId), + reaction_type: 'love', + }); + + expect(removeResponse.status).toBe(200); + expect(removeResponse.body.success).toBe(true); + expect(removeResponse.body.data.message).toBe('Reaction removed.'); + }); + + it('should return 400 for missing entity_type', async () => { + const response = await request + .post('/api/reactions/toggle') + .set('Authorization', `Bearer ${authToken}`) + .send({ + entity_id: String(testRecipeId), + reaction_type: 'like', + }); + + expect(response.status).toBe(400); + expect(response.body.success).toBe(false); + }); + + it('should return 400 for missing entity_id', async () => { + const response = await request + .post('/api/reactions/toggle') + .set('Authorization', `Bearer ${authToken}`) + .send({ + entity_type: 'recipe', + reaction_type: 'like', + }); + + expect(response.status).toBe(400); + expect(response.body.success).toBe(false); + }); + + it('should return 400 for missing reaction_type', async () => { + const response = await request + .post('/api/reactions/toggle') + .set('Authorization', `Bearer ${authToken}`) + .send({ + entity_type: 'recipe', + entity_id: String(testRecipeId), + }); + + expect(response.status).toBe(400); + expect(response.body.success).toBe(false); + }); + + it('should accept entity_id as string (required format)', async () => { + // entity_id must be a string per the Zod schema + const response = await request + .post('/api/reactions/toggle') + .set('Authorization', `Bearer ${authToken}`) + .send({ + entity_type: 'recipe', + entity_id: String(testRecipeId), + reaction_type: 'helpful', + }); + + // Should succeed (201 for add, 200 for remove) + expect([200, 201]).toContain(response.status); + expect(response.body.success).toBe(true); + + if (response.body.data.reaction?.reaction_id) { + createdReactionIds.push(response.body.data.reaction.reaction_id); + } + }); + }); +}); diff --git a/src/tests/integration/recipe.integration.test.ts b/src/tests/integration/recipe.integration.test.ts index 034440e6..153acc9a 100644 --- a/src/tests/integration/recipe.integration.test.ts +++ b/src/tests/integration/recipe.integration.test.ts @@ -232,6 +232,88 @@ describe('Recipe API Routes Integration Tests', () => { createdRecipeIds.push(forkedRecipe.recipe_id); }); + it('should allow forking seed recipes (null user_id)', async () => { + // First, find or create a seed recipe (one with null user_id) + let seedRecipeId: number; + const seedRecipeResult = await getPool().query( + `SELECT recipe_id FROM public.recipes WHERE user_id IS NULL LIMIT 1`, + ); + + if (seedRecipeResult.rows.length > 0) { + seedRecipeId = seedRecipeResult.rows[0].recipe_id; + } else { + // Create a seed recipe if none exist + const createSeedResult = await getPool().query( + `INSERT INTO public.recipes (name, instructions, user_id, status, description) + VALUES ('Seed Recipe for Fork Test', 'Seed recipe instructions.', NULL, 'public', 'A seed recipe.') + RETURNING recipe_id`, + ); + seedRecipeId = createSeedResult.rows[0].recipe_id; + createdRecipeIds.push(seedRecipeId); + } + + // Fork the seed recipe - this should succeed + const response = await request + .post(`/api/recipes/${seedRecipeId}/fork`) + .set('Authorization', `Bearer ${authToken}`); + + // Forking should work - seed recipes should be forkable + expect(response.status).toBe(201); + const forkedRecipe: Recipe = response.body.data; + expect(forkedRecipe.original_recipe_id).toBe(seedRecipeId); + expect(forkedRecipe.user_id).toBe(testUser.user.user_id); + + // Track for cleanup + createdRecipeIds.push(forkedRecipe.recipe_id); + }); + + describe('GET /api/recipes/:recipeId/comments', () => { + it('should return comments for a recipe', async () => { + // First add a comment + await request + .post(`/api/recipes/${testRecipe.recipe_id}/comments`) + .set('Authorization', `Bearer ${authToken}`) + .send({ content: 'Test comment for GET request' }); + + // Now fetch comments + const response = await request.get(`/api/recipes/${testRecipe.recipe_id}/comments`); + + expect(response.status).toBe(200); + expect(response.body.success).toBe(true); + expect(response.body.data).toBeInstanceOf(Array); + expect(response.body.data.length).toBeGreaterThan(0); + + // Verify comment structure + const comment = response.body.data[0]; + expect(comment).toHaveProperty('recipe_comment_id'); + expect(comment).toHaveProperty('content'); + expect(comment).toHaveProperty('user_id'); + expect(comment).toHaveProperty('recipe_id'); + }); + + it('should return empty array for recipe with no comments', async () => { + // Create a recipe specifically with no comments + const createRes = await request + .post('/api/users/recipes') + .set('Authorization', `Bearer ${authToken}`) + .send({ + name: 'Recipe With No Comments', + instructions: 'No comments here.', + description: 'Testing empty comments.', + }); + + const noCommentsRecipe: Recipe = createRes.body.data; + createdRecipeIds.push(noCommentsRecipe.recipe_id); + + // Fetch comments for this recipe + const response = await request.get(`/api/recipes/${noCommentsRecipe.recipe_id}/comments`); + + expect(response.status).toBe(200); + expect(response.body.success).toBe(true); + expect(response.body.data).toEqual([]); + }); + }); + describe('POST /api/recipes/suggest', () => { it('should return a recipe suggestion based on ingredients', async () => { const ingredients = ['chicken', 'rice', 'broccoli']; diff --git a/src/utils/zodUtils.ts b/src/utils/zodUtils.ts index 7e311001..37578831 100644 --- a/src/utils/zodUtils.ts +++ b/src/utils/zodUtils.ts @@ -4,15 +4,16 @@ import { z } from 'zod'; /** * A Zod schema for a required, non-empty string. * @param message The error message to display if the string is empty or missing. + * @param maxLength Optional maximum length (defaults to 255). * @returns A Zod string schema. */ -export const requiredString = (message: string) => +export const requiredString = (message: string, maxLength = 255) => z.preprocess( // If the value is null or undefined, preprocess it to an empty string. // This ensures that the subsequent `.min(1)` check will catch missing required fields. (val) => val ?? '', // Now, validate that the (potentially preprocessed) value is a string that, after trimming, has at least 1 character. - z.string().trim().min(1, message), + z.string().trim().min(1, message).max(maxLength, `Must be ${maxLength} characters or less.`), ); /** @@ -76,7 +77,7 @@ export const optionalNumeric = ( // the .optional() and .default() logic for null inputs. We want null to be // treated as "not provided", just like undefined. const schema = z.preprocess((val) => (val === null ? undefined : val), optionalNumberSchema); - + if (options.default !== undefined) return schema.default(options.default); return schema; @@ -89,7 +90,6 @@ export const optionalNumeric = ( */ export const optionalDate = (message?: string) => z.string().date(message).optional(); - /** * Creates a Zod schema for an optional boolean query parameter that is coerced from a string. * Handles 'true', '1' as true and 'false', '0' as false.