From ab976a6b07a81fe15f9973ae99496e9d8b57529c Mon Sep 17 00:00:00 2001 From: varun-s20 Date: Sat, 18 Apr 2026 11:58:23 +0530 Subject: [PATCH] bug fixes --- task-api/BUG_REPORT.md | 134 ++++++ task-api/SUBMISSION_NOTES.md | 23 ++ task-api/src/routes/tasks.js | 24 +- task-api/src/services/taskService.js | 37 +- task-api/src/utils/validators.js | 26 +- task-api/tests/taskService.test.js | 436 ++++++++++++++++++++ task-api/tests/tasks.integration.test.js | 500 +++++++++++++++++++++++ 7 files changed, 1177 insertions(+), 3 deletions(-) create mode 100644 task-api/BUG_REPORT.md create mode 100644 task-api/SUBMISSION_NOTES.md create mode 100644 task-api/tests/taskService.test.js create mode 100644 task-api/tests/tasks.integration.test.js diff --git a/task-api/BUG_REPORT.md b/task-api/BUG_REPORT.md new file mode 100644 index 00000000..12f701c0 --- /dev/null +++ b/task-api/BUG_REPORT.md @@ -0,0 +1,134 @@ +# Bug Report — Task Manager API + +This document describes bugs discovered through systematic testing of the Task Manager API codebase. Each bug includes the expected behaviour, actual behaviour, how it was discovered, and a proposed fix. + +--- + +## Bug 1: `getByStatus` uses substring match instead of strict equality + +**Severity:** Medium +**File:** `src/services/taskService.js`, line 9 +**Code:** `tasks.filter((t) => t.status.includes(status))` + +### Expected Behaviour +`GET /tasks?status=done` should return **only** tasks with status exactly equal to `"done"`. + +### Actual Behaviour +Because `String.prototype.includes()` performs a **substring** search, filtering by `"do"` returns both `"done"` and `"todo"` tasks. Similarly, `"in"` would match `"in_progress"`. Any partial string that appears inside a valid status will produce false positives. + +### How I Discovered It +While writing unit tests for `getByStatus`, I added an edge-case test that filters using the substring `"do"`. The test confirmed the function returns tasks with statuses `"todo"` and `"done"`, which should not happen with strict matching. + +### Proposed Fix +Replace `.includes()` with strict equality (`===`): + +```diff +- const getByStatus = (status) => tasks.filter((t) => t.status.includes(status)); ++ const getByStatus = (status) => tasks.filter((t) => t.status === status); +``` + +--- + +## Bug 2: `getPaginated` has an off-by-one error (**FIXED**) + +**Severity:** High +**File:** `src/services/taskService.js`, line 12 (original) +**Code:** `const offset = page * limit;` + +### Expected Behaviour +`GET /tasks?page=1&limit=10` should return the **first** 10 tasks (indices 0–9). + +### Actual Behaviour +Because the offset is calculated as `page * limit` instead of `(page - 1) * limit`, page 1 starts at index 10, completely skipping the first page of results. Page 2 starts at index 20, etc. + +### How I Discovered It +My integration test for `GET /tasks?page=1&limit=2` with 5 tasks expected `Task 1` and `Task 2`, but received `Task 3` and `Task 4` — confirming the offset was shifted by exactly one page. + +### Fix Applied +```diff +- const offset = page * limit; ++ const offset = (page - 1) * limit; +``` + +Pages are 1-indexed at the API level, so subtracting 1 before multiplying gives the correct zero-based offset for `Array.slice()`. + +--- + +## Bug 3: `completeTask` silently resets priority to `'medium'` + +**Severity:** Medium +**File:** `src/services/taskService.js`, line 69 (original) +**Code:** `priority: 'medium'` inside the spread + +### Expected Behaviour +Marking a task as complete (`PATCH /tasks/:id/complete`) should set `status` to `"done"` and set `completedAt` to the current timestamp **without modifying any other fields**. + +### Actual Behaviour +The `completeTask` function hardcodes `priority: 'medium'` in the updated object: + +```js +const updated = { + ...task, + priority: 'medium', // ← This overwrites the task's actual priority + status: 'done', + completedAt: new Date().toISOString(), +}; +``` + +A `high`-priority task silently becomes `medium` when completed. This is a data-integrity bug — the task's priority history is lost. + +### How I Discovered It +My unit test created a task with `priority: 'high'`, completed it, and asserted `priority` was still `'high'`. It wasn't — it was `'medium'`. + +### Proposed Fix +Remove the `priority: 'medium'` line: + +```diff + const updated = { + ...task, +- priority: 'medium', + status: 'done', + completedAt: new Date().toISOString(), + }; +``` + +--- + +## Bug 4: `update` allows overwriting system-managed fields (`id`, `createdAt`) + +**Severity:** Low +**File:** `src/services/taskService.js`, line 50 (original) +**Code:** `const updated = { ...tasks[index], ...fields };` + +### Expected Behaviour +`PUT /tasks/:id` should only allow updating user-editable fields (`title`, `description`, `status`, `priority`, `dueDate`). System-managed fields like `id` and `createdAt` should be immutable. + +### Actual Behaviour +Because the update function blindly spreads all incoming fields, a request like this: + +```json +PUT /tasks/abc-123 +{ "id": "hacked-id", "createdAt": "1999-01-01" } +``` + +…will overwrite the task's `id` and `createdAt`. This could break referential integrity (the old UUID no longer resolves) and forge audit timestamps. + +### How I Discovered It +My unit test sent `{ id: 'hacked-id' }` through `taskService.update()` and confirmed that `updated.id` was indeed `'hacked-id'`, proving the service does not protect system fields. + +### Proposed Fix +Destructure allowed fields explicitly, or strip protected keys before spreading: + +```diff + const update = (id, fields) => { + const index = tasks.findIndex((t) => t.id === id); + if (index === -1) return null; + ++ // Strip system-managed fields to prevent overwrites ++ const { id: _id, createdAt: _ca, completedAt: _co, ...allowed } = fields; +- const updated = { ...tasks[index], ...fields }; ++ const updated = { ...tasks[index], ...allowed }; + tasks[index] = updated; + return updated; + }; +``` diff --git a/task-api/SUBMISSION_NOTES.md b/task-api/SUBMISSION_NOTES.md new file mode 100644 index 00000000..76544266 --- /dev/null +++ b/task-api/SUBMISSION_NOTES.md @@ -0,0 +1,23 @@ +# Submission Notes + +## What I'd Test Next + +- **Concurrency / race conditions**: The in-memory store has no locking. If this were deployed with `cluster` mode or moved to a real database, concurrent writes to the same task could produce stale reads. I'd add stress tests with parallel requests. +- **Input sanitisation**: The API currently trusts string inputs (title, description, assignee) without length limits or content filtering. I'd test with very long strings (10k+ characters), unicode edge cases, and potential XSS payloads to verify the API either truncates or rejects them. +- **Error handler coverage**: The global error handler in `app.js` (lines 9–12) is never triggered by the current test suite because no route throws an unhandled error. I'd add a test that forces an internal error to verify the 500 response. +- **Pagination boundary conditions**: Negative page/limit values, `page=0`, `limit=0`, extremely large values. + +## What Surprised Me + +- **The `completeTask` priority reset** was the most surprising bug. It's the kind of defect that silently corrupts data without producing any visible error — users would only notice if they checked a completed task's priority later. It was clearly an accidental leftover from a copy-paste or refactoring. +- **The `getByStatus` substring bug** is subtle. For the three valid statuses (`todo`, `in_progress`, `done`), strict full-status queries like `?status=todo` happen to work because `"todo"` is not a substring of `"in_progress"` or `"done"`. The bug only surfaces with partial inputs — which makes it hard to catch without deliberate edge-case testing. +- **No request-body size limit**: There's no `express.json({ limit: ... })` option, so the server accepts arbitrarily large payloads. In production, this would be a denial-of-service vector. + +## Questions Before Shipping to Production + +1. **Persistence**: The in-memory store means all data is lost on restart. Is that intentional for a demo, or do we need to add a database (PostgreSQL, MongoDB, etc.) before go-live? +2. **Authentication & authorisation**: There's no auth layer at all. Who should be able to create, update, delete, and assign tasks? Are there roles (admin vs. member)? +3. **Rate limiting**: Should we add rate limiting to prevent abuse? +4. **Status enum consistency**: README says `"pending | in-progress | completed"`, but the code uses `"todo | in_progress | done"`. Which is the source of truth? This should be aligned before any client is built against the API. +5. **Validation of `id` and `createdAt` on update**: The current `PUT` endpoint allows overwriting system-managed fields. Is that a feature or a bug? (I've documented it as a bug in my report.) +6. **Idempotency**: Should `PATCH /tasks/:id/complete` be idempotent (no-op if already done) or should it return a 409 Conflict? diff --git a/task-api/src/routes/tasks.js b/task-api/src/routes/tasks.js index e8c370fe..2e443160 100644 --- a/task-api/src/routes/tasks.js +++ b/task-api/src/routes/tasks.js @@ -1,7 +1,7 @@ const express = require('express'); const router = express.Router(); const taskService = require('../services/taskService'); -const { validateCreateTask, validateUpdateTask } = require('../utils/validators'); +const { validateCreateTask, validateUpdateTask, validateAssignTask } = require('../utils/validators'); router.get('/stats', (req, res) => { const stats = taskService.getStats(); @@ -69,4 +69,26 @@ router.patch('/:id/complete', (req, res) => { res.json(task); }); +/** + * PATCH /tasks/:id/assign + * + * Assigns a user to an existing task. The assignee name is trimmed before + * storage to normalise whitespace, but we reject empty/whitespace-only values + * at the validation layer to ensure meaningful data. + */ +router.patch('/:id/assign', (req, res) => { + const error = validateAssignTask(req.body); + if (error) { + return res.status(400).json({ error }); + } + + const assignee = req.body.assignee.trim(); + const task = taskService.assignTask(req.params.id, assignee); + if (!task) { + return res.status(404).json({ error: 'Task not found' }); + } + + res.json(task); +}); + module.exports = router; diff --git a/task-api/src/services/taskService.js b/task-api/src/services/taskService.js index f8e89189..100e8fd4 100644 --- a/task-api/src/services/taskService.js +++ b/task-api/src/services/taskService.js @@ -8,8 +8,18 @@ const findById = (id) => tasks.find((t) => t.id === id); const getByStatus = (status) => tasks.filter((t) => t.status.includes(status)); +/** + * Returns a paginated slice of the tasks array. + * + * FIX: Changed `page * limit` to `(page - 1) * limit`. + * Pages are 1-indexed (page=1 is the first page), so we need to subtract 1 + * before multiplying to calculate the correct zero-based offset. + * + * Before the fix, page 1 with limit 10 would start at index 10, + * completely skipping the first 10 tasks. + */ const getPaginated = (page, limit) => { - const offset = page * limit; + const offset = (page - 1) * limit; return tasks.slice(offset, offset + limit); }; @@ -76,6 +86,30 @@ const completeTask = (id) => { return updated; }; +/** + * Assigns a user to an existing task. + * + * Design decisions: + * - Reassignment is allowed: if the task already has an assignee, it is + * overwritten. This mirrors real workflows where tasks get reassigned. + * - Completed tasks can be assigned: no restriction on task status, because + * assignment is metadata — useful for auditing or retrospectives. + * - The assignee string is stored as-is (trimming is handled at the route + * layer to keep the service layer focused on data operations). + * + * @param {string} id - UUID of the task to assign + * @param {string} assignee - Name of the person to assign the task to + * @returns {object|null} The updated task, or null if not found + */ +const assignTask = (id, assignee) => { + const index = tasks.findIndex((t) => t.id === id); + if (index === -1) return null; + + const updated = { ...tasks[index], assignee }; + tasks[index] = updated; + return updated; +}; + const _reset = () => { tasks = []; }; @@ -90,5 +124,6 @@ module.exports = { update, remove, completeTask, + assignTask, _reset, }; diff --git a/task-api/src/utils/validators.js b/task-api/src/utils/validators.js index 1e908ff5..cb8f3fd5 100644 --- a/task-api/src/utils/validators.js +++ b/task-api/src/utils/validators.js @@ -33,4 +33,28 @@ const validateUpdateTask = (body) => { return null; }; -module.exports = { validateCreateTask, validateUpdateTask }; +/** + * Validates the request body for the PATCH /tasks/:id/assign endpoint. + * + * Rules: + * - `assignee` must be present in the body + * - `assignee` must be a string (not a number, boolean, etc.) + * - `assignee` must not be empty or whitespace-only after trimming + * + * @param {object} body - The request body + * @returns {string|null} Error message if invalid, null if valid + */ +const validateAssignTask = (body) => { + if (body.assignee === undefined || body.assignee === null) { + return 'assignee is required'; + } + if (typeof body.assignee !== 'string') { + return 'assignee must be a string'; + } + if (body.assignee.trim() === '') { + return 'assignee must be a non-empty string'; + } + return null; +}; + +module.exports = { validateCreateTask, validateUpdateTask, validateAssignTask }; diff --git a/task-api/tests/taskService.test.js b/task-api/tests/taskService.test.js new file mode 100644 index 00000000..114e51e7 --- /dev/null +++ b/task-api/tests/taskService.test.js @@ -0,0 +1,436 @@ +/** + * Unit Tests for taskService.js + * + * Tests every exported function from the task service layer directly, + * without going through HTTP. This isolates business logic from routing + * and validates the in-memory data store operations. + */ + +const taskService = require('../src/services/taskService'); + +/* ------------------------------------------------------------------ */ +/* Test lifecycle — reset the in-memory store between every test */ +/* ------------------------------------------------------------------ */ +beforeEach(() => { + taskService._reset(); +}); + +/* ================================================================== */ +/* Helper: create a task with sensible defaults */ +/* ================================================================== */ +const createTask = (overrides = {}) => + taskService.create({ title: 'Test Task', ...overrides }); + +/* ================================================================== */ +/* _reset */ +/* ================================================================== */ +describe('_reset', () => { + it('should clear all tasks from the store', () => { + createTask(); + createTask({ title: 'Another' }); + + taskService._reset(); + + expect(taskService.getAll()).toHaveLength(0); + }); +}); + +/* ================================================================== */ +/* create */ +/* ================================================================== */ +describe('create', () => { + it('should create a task with required title and apply defaults', () => { + const task = taskService.create({ title: 'Buy milk' }); + + expect(task).toMatchObject({ + title: 'Buy milk', + description: '', + status: 'todo', + priority: 'medium', + dueDate: null, + completedAt: null, + }); + // System-generated fields + expect(task.id).toBeDefined(); + expect(task.createdAt).toBeDefined(); + expect(new Date(task.createdAt).toISOString()).toBe(task.createdAt); + }); + + it('should create a task with all fields provided', () => { + const task = taskService.create({ + title: 'Deploy', + description: 'Push to prod', + status: 'in_progress', + priority: 'high', + dueDate: '2026-12-31T00:00:00.000Z', + }); + + expect(task).toMatchObject({ + title: 'Deploy', + description: 'Push to prod', + status: 'in_progress', + priority: 'high', + dueDate: '2026-12-31T00:00:00.000Z', + completedAt: null, + }); + }); + + it('should assign unique IDs to each task', () => { + const t1 = createTask({ title: 'First' }); + const t2 = createTask({ title: 'Second' }); + + expect(t1.id).not.toBe(t2.id); + }); + + it('should add the task to the store', () => { + createTask(); + + expect(taskService.getAll()).toHaveLength(1); + }); +}); + +/* ================================================================== */ +/* getAll */ +/* ================================================================== */ +describe('getAll', () => { + it('should return an empty array when no tasks exist', () => { + expect(taskService.getAll()).toEqual([]); + }); + + it('should return all created tasks', () => { + createTask({ title: 'A' }); + createTask({ title: 'B' }); + createTask({ title: 'C' }); + + const all = taskService.getAll(); + expect(all).toHaveLength(3); + expect(all.map((t) => t.title)).toEqual(['A', 'B', 'C']); + }); + + it('should return a shallow copy so mutations do not affect the store', () => { + createTask(); + + const all = taskService.getAll(); + all.pop(); // mutate the returned array + + expect(taskService.getAll()).toHaveLength(1); // store is unaffected + }); +}); + +/* ================================================================== */ +/* findById */ +/* ================================================================== */ +describe('findById', () => { + it('should return the task matching the given ID', () => { + const created = createTask({ title: 'Find me' }); + + const found = taskService.findById(created.id); + + expect(found).toBeDefined(); + expect(found.title).toBe('Find me'); + }); + + it('should return undefined when no task matches', () => { + expect(taskService.findById('non-existent-id')).toBeUndefined(); + }); +}); + +/* ================================================================== */ +/* getByStatus */ +/* ================================================================== */ +describe('getByStatus', () => { + beforeEach(() => { + createTask({ title: 'Todo 1', status: 'todo' }); + createTask({ title: 'Todo 2', status: 'todo' }); + createTask({ title: 'In Progress', status: 'in_progress' }); + createTask({ title: 'Done', status: 'done' }); + }); + + it('should return only tasks with the exact matching status', () => { + const todos = taskService.getByStatus('todo'); + + expect(todos).toHaveLength(2); + todos.forEach((t) => expect(t.status).toBe('todo')); + }); + + it('should return an empty array when no tasks match the status', () => { + expect(taskService.getByStatus('nonexistent')).toEqual([]); + }); + + /** + * BUG: getByStatus uses String.prototype.includes() instead of ===. + * This means a partial match like "do" matches both "todo" and "done". + * + * This test documents the bug — it will FAIL if the bug is fixed + * (which is the correct outcome). Flip the assertion after fixing. + */ + it('BUG: substring match — "do" incorrectly matches "todo" and "done"', () => { + const results = taskService.getByStatus('do'); + + // Current (buggy) behaviour: includes() matches "todo" and "done" + // Expected (correct) behaviour: strict === would return [] + expect(results.length).toBeGreaterThan(0); // proves the bug exists + }); +}); + +/* ================================================================== */ +/* getPaginated */ +/* ================================================================== */ +describe('getPaginated', () => { + beforeEach(() => { + // Create 5 tasks with deterministic titles + for (let i = 1; i <= 5; i++) { + createTask({ title: `Task ${i}` }); + } + }); + + it('should return the correct slice for page 1', () => { + const result = taskService.getPaginated(1, 2); + + expect(result).toHaveLength(2); + expect(result[0].title).toBe('Task 1'); + expect(result[1].title).toBe('Task 2'); + }); + + it('should return the correct slice for page 2', () => { + const result = taskService.getPaginated(2, 2); + + expect(result).toHaveLength(2); + expect(result[0].title).toBe('Task 3'); + expect(result[1].title).toBe('Task 4'); + }); + + it('should return a partial page when at the end of the list', () => { + const result = taskService.getPaginated(3, 2); + + expect(result).toHaveLength(1); + expect(result[0].title).toBe('Task 5'); + }); + + it('should return an empty array for a page beyond the total', () => { + const result = taskService.getPaginated(10, 2); + + expect(result).toEqual([]); + }); +}); + +/* ================================================================== */ +/* getStats */ +/* ================================================================== */ +describe('getStats', () => { + it('should return zero counts when the store is empty', () => { + const stats = taskService.getStats(); + + expect(stats).toEqual({ todo: 0, in_progress: 0, done: 0, overdue: 0 }); + }); + + it('should correctly count tasks by status', () => { + createTask({ status: 'todo' }); + createTask({ status: 'todo' }); + createTask({ status: 'in_progress' }); + createTask({ status: 'done' }); + + const stats = taskService.getStats(); + + expect(stats.todo).toBe(2); + expect(stats.in_progress).toBe(1); + expect(stats.done).toBe(1); + }); + + it('should count overdue tasks (past due, not done)', () => { + // Overdue: past due date + not done + createTask({ + status: 'todo', + dueDate: '2020-01-01T00:00:00.000Z', + }); + // Not overdue: past due date but done + createTask({ + status: 'done', + dueDate: '2020-01-01T00:00:00.000Z', + }); + // Not overdue: future due date + createTask({ + status: 'todo', + dueDate: '2099-12-31T00:00:00.000Z', + }); + // Not overdue: no due date + createTask({ status: 'todo' }); + + const stats = taskService.getStats(); + + expect(stats.overdue).toBe(1); + }); +}); + +/* ================================================================== */ +/* update */ +/* ================================================================== */ +describe('update', () => { + it('should update the specified fields on an existing task', () => { + const task = createTask({ title: 'Original' }); + + const updated = taskService.update(task.id, { + title: 'Updated', + priority: 'high', + }); + + expect(updated.title).toBe('Updated'); + expect(updated.priority).toBe('high'); + // Unchanged fields remain + expect(updated.status).toBe('todo'); + }); + + it('should return null when the task does not exist', () => { + const result = taskService.update('non-existent-id', { title: 'Nope' }); + + expect(result).toBeNull(); + }); + + it('should persist the update in the store', () => { + const task = createTask({ title: 'Old' }); + + taskService.update(task.id, { title: 'New' }); + + const found = taskService.findById(task.id); + expect(found.title).toBe('New'); + }); + + /** + * BUG: update allows overwriting system-managed fields (id, createdAt). + * The spread `{ ...tasks[index], ...fields }` does not filter out + * protected keys. + */ + it('BUG: allows overwriting the task id via update', () => { + const task = createTask(); + + const updated = taskService.update(task.id, { id: 'hacked-id' }); + + // Current (buggy) behaviour: id is overwritten + expect(updated.id).toBe('hacked-id'); + // After fix: expect(updated.id).toBe(task.id); + }); +}); + +/* ================================================================== */ +/* remove */ +/* ================================================================== */ +describe('remove', () => { + it('should delete the task and return true', () => { + const task = createTask(); + + const result = taskService.remove(task.id); + + expect(result).toBe(true); + expect(taskService.getAll()).toHaveLength(0); + }); + + it('should return false when the task does not exist', () => { + const result = taskService.remove('non-existent-id'); + + expect(result).toBe(false); + }); + + it('should not affect other tasks when one is removed', () => { + const t1 = createTask({ title: 'Keep' }); + const t2 = createTask({ title: 'Remove' }); + + taskService.remove(t2.id); + + expect(taskService.getAll()).toHaveLength(1); + expect(taskService.getAll()[0].id).toBe(t1.id); + }); +}); + +/* ================================================================== */ +/* completeTask */ +/* ================================================================== */ +describe('completeTask', () => { + it('should mark the task as done with a completedAt timestamp', () => { + const task = createTask({ status: 'todo' }); + + const completed = taskService.completeTask(task.id); + + expect(completed.status).toBe('done'); + expect(completed.completedAt).toBeDefined(); + expect(new Date(completed.completedAt).toISOString()).toBe( + completed.completedAt, + ); + }); + + it('should return null when the task does not exist', () => { + const result = taskService.completeTask('non-existent-id'); + + expect(result).toBeNull(); + }); + + it('should persist the completion in the store', () => { + const task = createTask(); + + taskService.completeTask(task.id); + + const found = taskService.findById(task.id); + expect(found.status).toBe('done'); + }); + + /** + * BUG: completeTask resets the priority to 'medium', regardless of + * the task's original priority. A high-priority task silently becomes + * medium-priority when completed. + */ + it('BUG: completing a high-priority task resets priority to medium', () => { + const task = createTask({ priority: 'high' }); + + const completed = taskService.completeTask(task.id); + + // Current (buggy) behaviour: priority is reset to 'medium' + expect(completed.priority).toBe('medium'); + // After fix: expect(completed.priority).toBe('high'); + }); +}); + +/* ================================================================== */ +/* assignTask */ +/* ================================================================== */ +describe('assignTask', () => { + it('should assign a user to an existing task', () => { + const task = createTask(); + + const assigned = taskService.assignTask(task.id, 'Alice'); + + expect(assigned.assignee).toBe('Alice'); + }); + + it('should return null when the task does not exist', () => { + const result = taskService.assignTask('non-existent-id', 'Alice'); + + expect(result).toBeNull(); + }); + + it('should allow reassignment to a different user', () => { + const task = createTask(); + + taskService.assignTask(task.id, 'Alice'); + const reassigned = taskService.assignTask(task.id, 'Bob'); + + expect(reassigned.assignee).toBe('Bob'); + }); + + it('should persist the assignment in the store', () => { + const task = createTask(); + + taskService.assignTask(task.id, 'Charlie'); + + const found = taskService.findById(task.id); + expect(found.assignee).toBe('Charlie'); + }); + + it('should not modify other task fields when assigning', () => { + const task = createTask({ title: 'Important', priority: 'high' }); + + const assigned = taskService.assignTask(task.id, 'Diana'); + + expect(assigned.title).toBe('Important'); + expect(assigned.priority).toBe('high'); + expect(assigned.status).toBe('todo'); + }); +}); diff --git a/task-api/tests/tasks.integration.test.js b/task-api/tests/tasks.integration.test.js new file mode 100644 index 00000000..5a931572 --- /dev/null +++ b/task-api/tests/tasks.integration.test.js @@ -0,0 +1,500 @@ +/** + * Integration Tests for Task API Routes + * + * These tests exercise the full HTTP stack (Express routing → service → response) + * using Supertest. They verify status codes, response bodies, and side-effects + * at the API boundary, treating the server as a black box. + */ + +const request = require('supertest'); +const app = require('../src/app'); +const taskService = require('../src/services/taskService'); + +/* ------------------------------------------------------------------ */ +/* Reset the in-memory store between tests to guarantee isolation */ +/* ------------------------------------------------------------------ */ +beforeEach(() => { + taskService._reset(); +}); + +/* ================================================================== */ +/* Helper: create a task via the API and return the response body */ +/* ================================================================== */ +const createTaskViaAPI = async (body = { title: 'Test Task' }) => { + const res = await request(app).post('/tasks').send(body); + return res.body; +}; + +/* ================================================================== */ +/* POST /tasks */ +/* ================================================================== */ +describe('POST /tasks', () => { + it('should create a task and return 201 with the task body', async () => { + const res = await request(app) + .post('/tasks') + .send({ title: 'Write tests' }); + + expect(res.status).toBe(201); + expect(res.body).toMatchObject({ + title: 'Write tests', + status: 'todo', + priority: 'medium', + description: '', + dueDate: null, + completedAt: null, + }); + expect(res.body.id).toBeDefined(); + expect(res.body.createdAt).toBeDefined(); + }); + + it('should create a task with all optional fields', async () => { + const res = await request(app).post('/tasks').send({ + title: 'Deploy', + description: 'Push to prod', + status: 'in_progress', + priority: 'high', + dueDate: '2026-12-31T00:00:00.000Z', + }); + + expect(res.status).toBe(201); + expect(res.body.description).toBe('Push to prod'); + expect(res.body.status).toBe('in_progress'); + expect(res.body.priority).toBe('high'); + expect(res.body.dueDate).toBe('2026-12-31T00:00:00.000Z'); + }); + + it('should return 400 when title is missing', async () => { + const res = await request(app).post('/tasks').send({}); + + expect(res.status).toBe(400); + expect(res.body.error).toBeDefined(); + }); + + it('should return 400 when title is an empty string', async () => { + const res = await request(app).post('/tasks').send({ title: ' ' }); + + expect(res.status).toBe(400); + expect(res.body.error).toBeDefined(); + }); + + it('should return 400 for an invalid status', async () => { + const res = await request(app) + .post('/tasks') + .send({ title: 'Bad', status: 'invalid' }); + + expect(res.status).toBe(400); + expect(res.body.error).toContain('status'); + }); + + it('should return 400 for an invalid priority', async () => { + const res = await request(app) + .post('/tasks') + .send({ title: 'Bad', priority: 'critical' }); + + expect(res.status).toBe(400); + expect(res.body.error).toContain('priority'); + }); + + it('should return 400 for an invalid dueDate', async () => { + const res = await request(app) + .post('/tasks') + .send({ title: 'Bad', dueDate: 'not-a-date' }); + + expect(res.status).toBe(400); + expect(res.body.error).toContain('dueDate'); + }); +}); + +/* ================================================================== */ +/* GET /tasks */ +/* ================================================================== */ +describe('GET /tasks', () => { + it('should return an empty array when no tasks exist', async () => { + const res = await request(app).get('/tasks'); + + expect(res.status).toBe(200); + expect(res.body).toEqual([]); + }); + + it('should return all tasks', async () => { + await createTaskViaAPI({ title: 'A' }); + await createTaskViaAPI({ title: 'B' }); + + const res = await request(app).get('/tasks'); + + expect(res.status).toBe(200); + expect(res.body).toHaveLength(2); + }); +}); + +/* ================================================================== */ +/* GET /tasks?status= */ +/* ================================================================== */ +describe('GET /tasks?status=', () => { + beforeEach(async () => { + await createTaskViaAPI({ title: 'Todo', status: 'todo' }); + await createTaskViaAPI({ title: 'In Progress', status: 'in_progress' }); + await createTaskViaAPI({ title: 'Done', status: 'done' }); + }); + + it('should filter tasks by exact status', async () => { + const res = await request(app).get('/tasks?status=todo'); + + expect(res.status).toBe(200); + // Note: due to the substring-match bug, this may return unexpected results + // for certain status values. For "todo" specifically, it works because + // no other status contains the substring "todo". + res.body.forEach((task) => expect(task.status).toBe('todo')); + }); + + it('should return an empty array for a status with no tasks', async () => { + const res = await request(app).get('/tasks?status=nonexistent'); + + expect(res.status).toBe(200); + expect(res.body).toEqual([]); + }); +}); + +/* ================================================================== */ +/* GET /tasks?page=&limit= */ +/* ================================================================== */ +describe('GET /tasks?page=&limit=', () => { + beforeEach(async () => { + for (let i = 1; i <= 5; i++) { + await createTaskViaAPI({ title: `Task ${i}` }); + } + }); + + it('should return the first page of results', async () => { + const res = await request(app).get('/tasks?page=1&limit=2'); + + expect(res.status).toBe(200); + expect(res.body).toHaveLength(2); + expect(res.body[0].title).toBe('Task 1'); + expect(res.body[1].title).toBe('Task 2'); + }); + + it('should return subsequent pages correctly', async () => { + const res = await request(app).get('/tasks?page=2&limit=2'); + + expect(res.status).toBe(200); + expect(res.body).toHaveLength(2); + expect(res.body[0].title).toBe('Task 3'); + expect(res.body[1].title).toBe('Task 4'); + }); + + it('should return a partial page at the end', async () => { + const res = await request(app).get('/tasks?page=3&limit=2'); + + expect(res.status).toBe(200); + expect(res.body).toHaveLength(1); + expect(res.body[0].title).toBe('Task 5'); + }); + + it('should return an empty array for a page beyond the total', async () => { + const res = await request(app).get('/tasks?page=100&limit=2'); + + expect(res.status).toBe(200); + expect(res.body).toEqual([]); + }); + + it('should default to page=1 and limit=10 when given invalid values', async () => { + const res = await request(app).get('/tasks?page=abc&limit=xyz'); + + expect(res.status).toBe(200); + // With defaults page=1, limit=10, we should get all 5 tasks + expect(res.body).toHaveLength(5); + }); +}); + +/* ================================================================== */ +/* PUT /tasks/:id */ +/* ================================================================== */ +describe('PUT /tasks/:id', () => { + it('should update the task and return the updated version', async () => { + const task = await createTaskViaAPI({ title: 'Old' }); + + const res = await request(app) + .put(`/tasks/${task.id}`) + .send({ title: 'New', priority: 'high' }); + + expect(res.status).toBe(200); + expect(res.body.title).toBe('New'); + expect(res.body.priority).toBe('high'); + }); + + it('should return 404 for a non-existent task', async () => { + const res = await request(app) + .put('/tasks/non-existent-id') + .send({ title: 'Nope' }); + + expect(res.status).toBe(404); + expect(res.body.error).toBeDefined(); + }); + + it('should return 400 when title is set to an empty string', async () => { + const task = await createTaskViaAPI(); + + const res = await request(app) + .put(`/tasks/${task.id}`) + .send({ title: '' }); + + expect(res.status).toBe(400); + expect(res.body.error).toBeDefined(); + }); + + it('should return 400 for an invalid status on update', async () => { + const task = await createTaskViaAPI(); + + const res = await request(app) + .put(`/tasks/${task.id}`) + .send({ status: 'invalid' }); + + expect(res.status).toBe(400); + }); + + it('should return 400 for an invalid priority on update', async () => { + const task = await createTaskViaAPI(); + + const res = await request(app) + .put(`/tasks/${task.id}`) + .send({ priority: 'urgent' }); + + expect(res.status).toBe(400); + }); + + it('should return 400 for an invalid dueDate on update', async () => { + const task = await createTaskViaAPI(); + + const res = await request(app) + .put(`/tasks/${task.id}`) + .send({ dueDate: 'not-a-date' }); + + expect(res.status).toBe(400); + }); +}); + +/* ================================================================== */ +/* DELETE /tasks/:id */ +/* ================================================================== */ +describe('DELETE /tasks/:id', () => { + it('should delete the task and return 204', async () => { + const task = await createTaskViaAPI(); + + const res = await request(app).delete(`/tasks/${task.id}`); + + expect(res.status).toBe(204); + + // Verify the task is gone + const getRes = await request(app).get('/tasks'); + expect(getRes.body).toHaveLength(0); + }); + + it('should return 404 for a non-existent task', async () => { + const res = await request(app).delete('/tasks/non-existent-id'); + + expect(res.status).toBe(404); + expect(res.body.error).toBeDefined(); + }); +}); + +/* ================================================================== */ +/* PATCH /tasks/:id/complete */ +/* ================================================================== */ +describe('PATCH /tasks/:id/complete', () => { + it('should mark the task as done and set completedAt', async () => { + const task = await createTaskViaAPI({ title: 'Finish this' }); + + const res = await request(app).patch(`/tasks/${task.id}/complete`); + + expect(res.status).toBe(200); + expect(res.body.status).toBe('done'); + expect(res.body.completedAt).toBeDefined(); + }); + + it('should return 404 for a non-existent task', async () => { + const res = await request(app).patch('/tasks/non-existent-id/complete'); + + expect(res.status).toBe(404); + expect(res.body.error).toBeDefined(); + }); + + it('should be idempotent — completing an already-done task still succeeds', async () => { + const task = await createTaskViaAPI(); + + await request(app).patch(`/tasks/${task.id}/complete`); + const res = await request(app).patch(`/tasks/${task.id}/complete`); + + expect(res.status).toBe(200); + expect(res.body.status).toBe('done'); + }); +}); + +/* ================================================================== */ +/* GET /tasks/stats */ +/* ================================================================== */ +describe('GET /tasks/stats', () => { + it('should return zero counts when no tasks exist', async () => { + const res = await request(app).get('/tasks/stats'); + + expect(res.status).toBe(200); + expect(res.body).toEqual({ + todo: 0, + in_progress: 0, + done: 0, + overdue: 0, + }); + }); + + it('should return correct counts by status', async () => { + await createTaskViaAPI({ title: 'A', status: 'todo' }); + await createTaskViaAPI({ title: 'B', status: 'in_progress' }); + await createTaskViaAPI({ title: 'C', status: 'done' }); + await createTaskViaAPI({ title: 'D', status: 'todo' }); + + const res = await request(app).get('/tasks/stats'); + + expect(res.status).toBe(200); + expect(res.body.todo).toBe(2); + expect(res.body.in_progress).toBe(1); + expect(res.body.done).toBe(1); + }); + + it('should count overdue tasks correctly', async () => { + // Overdue: past due + not done + await createTaskViaAPI({ + title: 'Overdue', + status: 'todo', + dueDate: '2020-01-01T00:00:00.000Z', + }); + // Not overdue: past due but done + await createTaskViaAPI({ + title: 'Done old', + status: 'done', + dueDate: '2020-01-01T00:00:00.000Z', + }); + // Not overdue: future due + await createTaskViaAPI({ + title: 'Future', + status: 'todo', + dueDate: '2099-12-31T00:00:00.000Z', + }); + + const res = await request(app).get('/tasks/stats'); + + expect(res.body.overdue).toBe(1); + }); +}); + +/* ================================================================== */ +/* PATCH /tasks/:id/assign */ +/* ================================================================== */ +describe('PATCH /tasks/:id/assign', () => { + it('should assign a user and return the updated task', async () => { + const task = await createTaskViaAPI({ title: 'Assign me' }); + + const res = await request(app) + .patch(`/tasks/${task.id}/assign`) + .send({ assignee: 'Alice' }); + + expect(res.status).toBe(200); + expect(res.body.assignee).toBe('Alice'); + expect(res.body.id).toBe(task.id); + expect(res.body.title).toBe('Assign me'); + }); + + it('should return 404 for a non-existent task', async () => { + const res = await request(app) + .patch('/tasks/non-existent-id/assign') + .send({ assignee: 'Alice' }); + + expect(res.status).toBe(404); + expect(res.body.error).toBeDefined(); + }); + + it('should return 400 when assignee is missing', async () => { + const task = await createTaskViaAPI(); + + const res = await request(app) + .patch(`/tasks/${task.id}/assign`) + .send({}); + + expect(res.status).toBe(400); + expect(res.body.error).toBeDefined(); + }); + + it('should return 400 when assignee is an empty string', async () => { + const task = await createTaskViaAPI(); + + const res = await request(app) + .patch(`/tasks/${task.id}/assign`) + .send({ assignee: '' }); + + expect(res.status).toBe(400); + expect(res.body.error).toBeDefined(); + }); + + it('should return 400 when assignee is only whitespace', async () => { + const task = await createTaskViaAPI(); + + const res = await request(app) + .patch(`/tasks/${task.id}/assign`) + .send({ assignee: ' ' }); + + expect(res.status).toBe(400); + expect(res.body.error).toBeDefined(); + }); + + it('should return 400 when assignee is not a string', async () => { + const task = await createTaskViaAPI(); + + const res = await request(app) + .patch(`/tasks/${task.id}/assign`) + .send({ assignee: 123 }); + + expect(res.status).toBe(400); + expect(res.body.error).toBeDefined(); + }); + + it('should allow reassignment to a different user', async () => { + const task = await createTaskViaAPI(); + + await request(app) + .patch(`/tasks/${task.id}/assign`) + .send({ assignee: 'Alice' }); + + const res = await request(app) + .patch(`/tasks/${task.id}/assign`) + .send({ assignee: 'Bob' }); + + expect(res.status).toBe(200); + expect(res.body.assignee).toBe('Bob'); + }); + + it('should allow assigning a completed task', async () => { + const task = await createTaskViaAPI(); + + // Complete the task first + await request(app).patch(`/tasks/${task.id}/complete`); + + const res = await request(app) + .patch(`/tasks/${task.id}/assign`) + .send({ assignee: 'Alice' }); + + expect(res.status).toBe(200); + expect(res.body.assignee).toBe('Alice'); + expect(res.body.status).toBe('done'); + }); + + it('should trim the assignee name but preserve it', async () => { + const task = await createTaskViaAPI(); + + const res = await request(app) + .patch(`/tasks/${task.id}/assign`) + .send({ assignee: ' Alice ' }); + + expect(res.status).toBe(200); + // The trimmed value should be stored + expect(res.body.assignee).toBe('Alice'); + }); +});