diff --git a/BUG_REPORT.md b/BUG_REPORT.md new file mode 100644 index 00000000..ec1247d6 --- /dev/null +++ b/BUG_REPORT.md @@ -0,0 +1,42 @@ +# Bug Report + +## Bug 1: Complete Task silently resets priority to 'medium' + +**Title**: `completeTask` changes task priority to 'medium' regardless of its original priority. +**Expected behavior**: When marking a task as done, its `priority` should remain unchanged from what the user previously set. +**Actual behavior**: The `priority` field is overwritten with `'medium'`. +**Steps to reproduce**: +1. Create a task with priority `'high'`. +2. Call `PATCH /tasks/:id/complete` on that task. +3. Observe the returned task data has `priority: 'medium'`. +**Root cause**: In `src/services/taskService.js`, the `completeTask` function explicitly sets `priority: 'medium'` when creating the updated task object. +**Suggested fix**: Remove `priority: 'medium'` from the `updated` object construction in `completeTask`. + +--- + +## Bug 2: Get Tasks by Status uses partial matching + +**Title**: Unintended partial matching when filtering tasks by status. +**Expected behavior**: Querying `/tasks?status=in_progress` should only return tasks with an exact match for the status `in_progress`. +**Actual behavior**: The filter uses `.includes()`, allowing partial string matching. Querying `status=progress` will match `in_progress`, which is undocumented and can cause issues if statuses have overlapping names. +**Steps to reproduce**: +1. Create a task with status `in_progress`. +2. Send a `GET /tasks?status=progress`. +3. The task is returned, although `progress` is not a valid status name and should return empty or error. +**Root cause**: In `src/services/taskService.js`, `getByStatus` filters using `t.status.includes(status)` instead of strict equality `===`. +**Suggested fix**: Change `t.status.includes(status)` to `t.status === status`. + +--- + +## Bug 3: Incorrect pagination math ignores 1-based indexing + +**Title**: Pagination skips the first page data because the offset calculation interprets page 1 as page 2. +**Expected behavior**: `GET /tasks?page=1&limit=10` should return the first 10 tasks (offset 0). +**Actual behavior**: It returns tasks 11-20 (offset 10), effectively skipping the first page. +**Steps to reproduce**: +1. Create 3 tasks (T1, T2, T3). +2. Request `GET /tasks?page=1&limit=2`. +3. Only T3 is returned. T1 and T2 are entirely skipped. +**Root cause**: In `src/services/taskService.js`, `getPaginated` calculates `offset = page * limit`. Since the route sets `page` to default to `1`, `offset` starts at `limit` (e.g., 10), which skips the 0-offset data. +**Suggested fix**: Change the calculation to `const offset = (page - 1) * Math.max(limit, 1);` but since the service should probably support zero or 1 based consistently, we adjust `offset = (page - 1) * limit;` or fix it in the route. +We'll update `getPaginated` to accept `page=1` as the first page and use `offset = (page - 1) * limit`. diff --git a/CHECKLIST.md b/CHECKLIST.md new file mode 100644 index 00000000..a83e5d34 --- /dev/null +++ b/CHECKLIST.md @@ -0,0 +1,14 @@ +# Final Project Checklist + +- [x] **Step 1:** Read and understand the existing code (Project structure, endpoints, logic). +- [x] **Step 2:** Ensure server runs successfully (`npm install`, `npm start` working properly). +- [x] **Step 3:** Create testing structure (`tests/unit`, `tests/integration` created; Jest config verified). +- [x] **Step 4:** Write Unit Tests (`taskService.test.js` covers `create`, `getAll`, `update`, `remove`, `completeTask`, `getPaginated`, `getByStatus`, `getStats` + Assign task). +- [x] **Step 5:** Write Integration Tests (`tasks.test.js` covering standard API endpoints + validations + edge cases). +- [x] **Step 6:** Achieve > 80% Test Coverage (Currently at **92.95%** Lines). +- [x] **Step 7:** Write a professional Bug Report (`BUG_REPORT.md` details 3 separate bugs caught by the tests). +- [x] **Step 8:** Fix a bug (All 3 known bugs were fixed to improve system stability). +- [x] **Step 9:** Add new feature (`PATCH /tasks/:id/assign` implemented with logic, proper validation, and error handling). +- [x] **Step 10:** Write tests for the new feature (Added unit and integration coverage for valid assign, missing assignee, not found errors, reassignments). +- [x] **Step 11:** Final validation check (All tests pass consistently, server operates perfectly). +- [x] **Step 12:** Prepare Submission (Generated `BUG_REPORT.md`, `COVERAGE_SUMMARY.md`, `CHECKLIST.md`, and `REFLECTION.md`). diff --git a/COVERAGE_SUMMARY.md b/COVERAGE_SUMMARY.md new file mode 100644 index 00000000..4b047689 --- /dev/null +++ b/COVERAGE_SUMMARY.md @@ -0,0 +1,14 @@ +# Coverage Summary + +| File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s | +|------|---------|----------|---------|---------|-------------------| +| **All files** | **93.58** | **82.55** | **93.33** | **92.95** | | +| src/app.js | 69.23 | 75 | 0 | 69.23 | 10-11, 17-18 | +| src/routes/tasks.js | 100 | 91.66 | 100 | 100 | 20-21 | +| src/services/taskService.js | 100 | 94.73 | 100 | 100 | 23 | +| src/utils/validators.js | 77.77 | 71.79 | 100 | 77.77 | 9,12,15,22,28,31 | + +**Analysis**: +The overall project coverage stands comfortably at **92.95%**, well above the 80% requirement. +Major logic in `routes` and `services` shows 100% statement and lines coverage. +The uncovered branches mostly relate to error handling block (lines 10-11 in app.js where express sets the default error handler), main execution (lines 17-18 in app.js), edge validation errors that were omitted to not bloat the simple API tests, and fallback default values. diff --git a/REFLECTION.md b/REFLECTION.md new file mode 100644 index 00000000..b5ed4b0b --- /dev/null +++ b/REFLECTION.md @@ -0,0 +1,15 @@ +# Reflection + +### What would you test next? +If I were to expand the testing suite further: +1. **Mocking External Dependencies:** For a real application, I'd integrate a SQLite or PostgreSQL testing database or mock out the data persistence layer using a proper mocking library. +2. **Stress/Load Testing:** Introduce a tool like Artillery or K6 to test API stability under a high concurrent load. +3. **Validation logic specifics:** I would add further test scenarios in validation to guarantee dates follow strict schema parameters for time zones. + +### What surprised you? +It was surprising/intriguing to see a very subtle bug implemented into `taskService.getPaginated`. Pagination bugs involving $1$-based arrays versus $0$-based bounds (`offset = page * limit`) trick developers almost constantly! Also, seeing `.includes()` for an exact enumeration match showed why unit tests shouldn't assume the implementation acts accurately to expectations without checking edge cases specifically (like `progress` vs `in_progress`). + +### Questions before production release? +1. **Data Persistence:** Before production, should we connect this to an actual database (e.g. Postgres or MongoDB)? The `in-memory` array isn't durable over restarts. +2. **Authentication/Authorization:** Who can use the `/assign` or `/complete` routes? Is there an expected security middleware implementation? +3. **Concurrency:** Are there scenarios where a race condition on `tasks.findIndex` + array mutation can leave state inconsistent? diff --git a/task-api/src/routes/tasks.js b/task-api/src/routes/tasks.js index e8c370fe..89a14581 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,18 @@ router.patch('/:id/complete', (req, res) => { res.json(task); }); +router.patch('/:id/assign', (req, res) => { + const error = validateAssignTask(req.body); + if (error) { + return res.status(400).json({ error }); + } + + const task = taskService.assignTask(req.params.id, req.body.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..eeb97a99 100644 --- a/task-api/src/services/taskService.js +++ b/task-api/src/services/taskService.js @@ -6,10 +6,11 @@ const getAll = () => [...tasks]; const findById = (id) => tasks.find((t) => t.id === id); -const getByStatus = (status) => tasks.filter((t) => t.status.includes(status)); +const getByStatus = (status) => tasks.filter((t) => t.status === status); const getPaginated = (page, limit) => { - const offset = page * limit; + const pageIndex = Math.max(0, page - 1); + const offset = pageIndex * limit; return tasks.slice(offset, offset + limit); }; @@ -66,7 +67,6 @@ const completeTask = (id) => { const updated = { ...task, - priority: 'medium', status: 'done', completedAt: new Date().toISOString(), }; @@ -76,6 +76,20 @@ const completeTask = (id) => { return updated; }; +const assignTask = (id, assignee) => { + const task = findById(id); + if (!task) return null; + + const updated = { + ...task, + assignee, + }; + + const index = tasks.findIndex((t) => t.id === id); + tasks[index] = updated; + return updated; +}; + const _reset = () => { tasks = []; }; @@ -90,5 +104,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..a87e14fa 100644 --- a/task-api/src/utils/validators.js +++ b/task-api/src/utils/validators.js @@ -33,4 +33,11 @@ const validateUpdateTask = (body) => { return null; }; -module.exports = { validateCreateTask, validateUpdateTask }; +const validateAssignTask = (body) => { + if (!body.assignee || typeof body.assignee !== 'string' || body.assignee.trim() === '') { + return 'assignee is required and must be a non-empty string'; + } + return null; +}; + +module.exports = { validateCreateTask, validateUpdateTask, validateAssignTask }; diff --git a/task-api/tests/integration/tasks.test.js b/task-api/tests/integration/tasks.test.js new file mode 100644 index 00000000..3211b6cc --- /dev/null +++ b/task-api/tests/integration/tasks.test.js @@ -0,0 +1,195 @@ +const request = require('supertest'); +const app = require('../../src/app'); +const taskService = require('../../src/services/taskService'); + +describe('Tasks API Integration Tests', () => { + beforeEach(() => { + // Reset data before each test to ensure test isolation + taskService._reset(); + }); + + describe('GET /tasks', () => { + it('should return 200 and an empty array initially', async () => { + const res = await request(app).get('/tasks'); + expect(res.status).toBe(200); + expect(res.body).toEqual([]); + }); + + it('should return 200 and an array of tasks when data exists', async () => { + taskService.create({ title: 'Task 1' }); + const res = await request(app).get('/tasks'); + expect(res.status).toBe(200); + expect(res.body.length).toBe(1); + expect(res.body[0].title).toBe('Task 1'); + }); + + it('should filter tasks by status with GET /tasks?status=todo', async () => { + taskService.create({ title: 'T1', status: 'todo' }); + taskService.create({ title: 'T2', status: 'done' }); + + const res = await request(app).get('/tasks?status=todo'); + expect(res.status).toBe(200); + expect(res.body.length).toBe(1); + expect(res.body[0].status).toBe('todo'); + }); + + it('should paginate tasks with GET /tasks?page=1&limit=1', async () => { + taskService.create({ title: 'T1' }); + taskService.create({ title: 'T2' }); + + // Correct expectation: page=1 should return the first item T1 + const res = await request(app).get('/tasks?page=1&limit=1'); + expect(res.status).toBe(200); + expect(res.body.length).toBe(1); + expect(res.body[0].title).toBe('T1'); + }); + }); + + describe('GET /tasks/stats', () => { + it('should return statistics of tasks', async () => { + taskService.create({ title: 'T1', status: 'todo' }); + taskService.create({ title: 'T2', status: 'done' }); + + const res = await request(app).get('/tasks/stats'); + expect(res.status).toBe(200); + expect(res.body).toMatchObject({ + todo: 1, + in_progress: 0, + done: 1, + overdue: 0 + }); + }); + }); + + describe('POST /tasks', () => { + it('should create a new task successfully', async () => { + const res = await request(app) + .post('/tasks') + .send({ title: 'New API Task' }); + + expect(res.status).toBe(201); + expect(res.body.title).toBe('New API Task'); + expect(res.body.id).toBeDefined(); + }); + + it('should fail with 400 when validation fails (missing title)', async () => { + const res = await request(app) + .post('/tasks') + .send({ description: 'No title' }); + + expect(res.status).toBe(400); + expect(res.body.error).toBeDefined(); + }); + }); + + describe('PUT /tasks/:id', () => { + it('should update an existing task successfully', async () => { + const task = taskService.create({ title: 'Initial Title' }); + const res = await request(app) + .put(`/tasks/${task.id}`) + .send({ title: 'Updated Title', status: 'in_progress' }); + + expect(res.status).toBe(200); + expect(res.body.title).toBe('Updated Title'); + expect(res.body.status).toBe('in_progress'); + }); + + it('should fail with 400 on validation error', async () => { + const task = taskService.create({ title: 'Initial Title' }); + const res = await request(app) + .put(`/tasks/${task.id}`) + .send({ status: 'invalid_status' }); + + expect(res.status).toBe(400); + expect(res.body.error).toContain('status must be one of'); + }); + + it('should return 404 when updating non-existent task', async () => { + const res = await request(app) + .put('/tasks/invalid-id') + .send({ title: 'Updated Title' }); + + expect(res.status).toBe(404); + expect(res.body.error).toBe('Task not found'); + }); + }); + + describe('DELETE /tasks/:id', () => { + it('should successfully delete an existing task', async () => { + const task = taskService.create({ title: 'To Delete' }); + const res = await request(app).delete(`/tasks/${task.id}`); + + expect(res.status).toBe(204); + // Verify deletion + expect(taskService.findById(task.id)).toBeUndefined(); + }); + + it('should return 404 when deleting a non-existent task', async () => { + const res = await request(app).delete('/tasks/invalid-id'); + expect(res.status).toBe(404); + expect(res.body.error).toBe('Task not found'); + }); + }); + + describe('PATCH /tasks/:id/complete', () => { + it('should mark a task as complete successfully', async () => { + const task = taskService.create({ title: 'Complete Me' }); + 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).not.toBeNull(); + }); + + it('should return 404 when completing a non-existent task', async () => { + const res = await request(app).patch('/tasks/invalid-id/complete'); + expect(res.status).toBe(404); + expect(res.body.error).toBe('Task not found'); + }); + }); + + describe('PATCH /tasks/:id/assign', () => { + it('should assign a task successfully', async () => { + const task = taskService.create({ title: 'Assign Me' }); + const res = await request(app) + .patch(`/tasks/${task.id}/assign`) + .send({ assignee: 'Jane Smith' }); + + expect(res.status).toBe(200); + expect(res.body.assignee).toBe('Jane Smith'); + }); + + it('should return 404 when assigning a non-existent task', async () => { + const res = await request(app) + .patch('/tasks/invalid-id/assign') + .send({ assignee: 'Jane Smith' }); + expect(res.status).toBe(404); + expect(res.body.error).toBe('Task not found'); + }); + + it('should return 400 when assignee is empty or missing', async () => { + const task = taskService.create({ title: 'Assign Me' }); + const res1 = await request(app) + .patch(`/tasks/${task.id}/assign`) + .send({ assignee: '' }); + expect(res1.status).toBe(400); + + const res2 = await request(app) + .patch(`/tasks/${task.id}/assign`) + .send({}); + expect(res2.status).toBe(400); + }); + + it('should successfully reassign an existing task', async () => { + const task = taskService.create({ title: 'Assign Me' }); + taskService.assignTask(task.id, 'Original Assignee'); + + const res = await request(app) + .patch(`/tasks/${task.id}/assign`) + .send({ assignee: 'New Assignee' }); + + expect(res.status).toBe(200); + expect(res.body.assignee).toBe('New Assignee'); + }); + }); +}); diff --git a/task-api/tests/unit/taskService.test.js b/task-api/tests/unit/taskService.test.js new file mode 100644 index 00000000..6cae99e8 --- /dev/null +++ b/task-api/tests/unit/taskService.test.js @@ -0,0 +1,180 @@ +const taskService = require('../../src/services/taskService'); + +describe('taskService', () => { + beforeEach(() => { + taskService._reset(); + }); + + describe('create', () => { + it('should create a task successfully with valid input', () => { + const task = taskService.create({ title: 'Test Task' }); + expect(task).toBeDefined(); + expect(task.id).toBeDefined(); + expect(task.title).toBe('Test Task'); + expect(task.status).toBe('todo'); + expect(task.priority).toBe('medium'); + }); + + it('should set default values if only title is provided', () => { + const task = taskService.create({ title: 'Simple Task' }); + expect(task.description).toBe(''); + expect(task.status).toBe('todo'); + expect(task.priority).toBe('medium'); + expect(task.dueDate).toBeNull(); + expect(task.completedAt).toBeNull(); + }); + }); + + describe('getAll', () => { + it('should return empty list initially', () => { + expect(taskService.getAll()).toEqual([]); + }); + + it('should return all created tasks', () => { + taskService.create({ title: 'Task 1' }); + taskService.create({ title: 'Task 2' }); + const tasks = taskService.getAll(); + expect(tasks.length).toBe(2); + }); + }); + + describe('findById', () => { + it('should find a task by existing id', () => { + const created = taskService.create({ title: 'Task to find' }); + const found = taskService.findById(created.id); + expect(found).toBeDefined(); + expect(found.id).toBe(created.id); + }); + + it('should return undefined for a non-existent id', () => { + expect(taskService.findById('invalid-id')).toBeUndefined(); + }); + }); + + describe('getByStatus', () => { + it('should return tasks matching a status', () => { + taskService.create({ title: 'Task 1', status: 'todo' }); + taskService.create({ title: 'Task 2', status: 'done' }); + + const todoTasks = taskService.getByStatus('todo'); + expect(todoTasks.length).toBe(1); + expect(todoTasks[0].status).toBe('todo'); + }); + + it('should NOT match partially, only exact status', () => { + taskService.create({ title: 'Task 1', status: 'in_progress' }); + const tasks = taskService.getByStatus('progress'); + expect(tasks.length).toBe(0); // Should be exactly 0, not 1 + }); + }); + + describe('getPaginated', () => { + it('should return subset of tasks based on page and limit', () => { + taskService.create({ title: 'T1' }); + taskService.create({ title: 'T2' }); + taskService.create({ title: 'T3' }); + + // Correct test: page 1 should be the first page + // Therefore, offset = (1 - 1) * 2 = 0 + const page1 = taskService.getPaginated(1, 2); + expect(page1.length).toBe(2); + expect(page1[0].title).toBe('T1'); + expect(page1[1].title).toBe('T2'); + + const page2 = taskService.getPaginated(2, 2); + expect(page2.length).toBe(1); + expect(page2[0].title).toBe('T3'); + }); + }); + + describe('update', () => { + it('should update an existing task successfully', () => { + const task = taskService.create({ title: 'Old Title' }); + const updated = taskService.update(task.id, { title: 'New Title', status: 'in_progress' }); + + expect(updated.title).toBe('New Title'); + expect(updated.status).toBe('in_progress'); + + // Verify in array + const retrieved = taskService.findById(task.id); + expect(retrieved.title).toBe('New Title'); + }); + + it('should return null when updating non-existent task', () => { + const updated = taskService.update('invalid-id', { title: 'New' }); + expect(updated).toBeNull(); + }); + }); + + describe('remove', () => { + it('should correctly remove existing task', () => { + const task = taskService.create({ title: 'To Delete' }); + const result = taskService.remove(task.id); + + expect(result).toBe(true); + expect(taskService.getAll().length).toBe(0); + }); + + it('should return false when trying to remove non-existent task', () => { + const result = taskService.remove('invalid-id'); + expect(result).toBe(false); + }); + }); + + describe('completeTask', () => { + it('should mark an existing task as done and set completedAt', () => { + const task = taskService.create({ title: 'Almost Done', priority: 'high' }); + const completed = taskService.completeTask(task.id); + + expect(completed.status).toBe('done'); + expect(completed.priority).toBe('high'); // Priority should remain unchanged + expect(completed.completedAt).not.toBeNull(); + }); + + it('should return null if task to complete does not exist', () => { + const result = taskService.completeTask('invalid-id'); + expect(result).toBeNull(); + }); + }); + + describe('assignTask', () => { + it('should assign a task to a given assignee successfully', () => { + const task = taskService.create({ title: 'Unassigned Task' }); + const assignedTask = taskService.assignTask(task.id, 'John Doe'); + expect(assignedTask.assignee).toBe('John Doe'); + + const retrieved = taskService.findById(task.id); + expect(retrieved.assignee).toBe('John Doe'); + }); + + it('should reassign an already assigned task', () => { + const task = taskService.create({ title: 'Assigned Task' }); + taskService.assignTask(task.id, 'Alice'); + const reassignedTask = taskService.assignTask(task.id, 'Bob'); + expect(reassignedTask.assignee).toBe('Bob'); + }); + + it('should return null if task does not exist', () => { + const result = taskService.assignTask('invalid-id', 'John Doe'); + expect(result).toBeNull(); + }); + }); + + describe('getStats', () => { + it('should return correct counts of tasks based on status and overdue', () => { + const pastDate = new Date(); + pastDate.setDate(pastDate.getDate() - 1); + + taskService.create({ title: 'Task 1', status: 'todo' }); + taskService.create({ title: 'Task 2', status: 'in_progress' }); + taskService.create({ title: 'Task 3', status: 'done' }); + taskService.create({ title: 'Task 4', status: 'todo', dueDate: pastDate.toISOString() }); + + const stats = taskService.getStats(); + expect(stats.todo).toBe(2); + expect(stats.in_progress).toBe(1); + expect(stats.done).toBe(1); + expect(stats.overdue).toBe(1); // Task 4 is overdue and not done + }); + }); +});