From 9219bb3f4ea7016948e8e353fce70d33b55bf187 Mon Sep 17 00:00:00 2001 From: Rajmishra26 <120782135+Rajmishra26@users.noreply.github.com> Date: Sat, 18 Apr 2026 22:24:20 +0530 Subject: [PATCH] Complete task API assignment --- BUG_REPORT.md | 33 ++++ SUBMISSION_NOTES.md | 46 +++++ task-api/src/routes/tasks.js | 16 +- task-api/src/services/taskService.js | 29 ++- task-api/src/utils/validators.js | 10 +- task-api/tests/taskService.test.js | 159 +++++++++++++++++ task-api/tests/tasks.routes.test.js | 252 +++++++++++++++++++++++++++ 7 files changed, 539 insertions(+), 6 deletions(-) create mode 100644 BUG_REPORT.md create mode 100644 SUBMISSION_NOTES.md create mode 100644 task-api/tests/taskService.test.js create mode 100644 task-api/tests/tasks.routes.test.js diff --git a/BUG_REPORT.md b/BUG_REPORT.md new file mode 100644 index 00000000..6a65bc9a --- /dev/null +++ b/BUG_REPORT.md @@ -0,0 +1,33 @@ +# Bug Report + +The issues below were identified while writing the service and API tests in `task-api/tests/`. + +## 1. Pagination skipped the first page of results + +- Location: `task-api/src/services/taskService.js:11-13` +- Expected behavior: `GET /tasks?page=1&limit=2` should return the first two tasks. +- Actual behavior: page 1 returned tasks starting at index 2, so the first page skipped the earliest results. +- How I discovered it: the regression tests in `tests/taskService.test.js` and `tests/tasks.routes.test.js` both failed when asserting that page 1 should contain the first created tasks. +- Why it happened: the pagination offset used `page * limit` instead of `(page - 1) * limit`. +- Fix: updated the offset calculation to `(page - 1) * limit`. +- Status: fixed. + +## 2. Status filtering matched partial strings + +- Location: `task-api/src/services/taskService.js:9` +- Expected behavior: filtering with `status=todo` should only return tasks whose status is exactly `todo`. +- Actual behavior: filtering used `String.prototype.includes`, so partial strings like `progress` matched `in_progress`. +- How I discovered it: a service-level characterization test showed `getByStatus('progress')` incorrectly returning `in_progress` tasks. +- Why it happened: the filter compared by substring rather than strict equality. +- Fix: changed the comparison to `t.status === status`. +- Status: fixed. + +## 3. Completing a task reset its priority + +- Location: `task-api/src/services/taskService.js:75-79` +- Expected behavior: marking a task complete should update its status and completion timestamp without changing unrelated fields like priority. +- Actual behavior: `completeTask` forced `priority` to `'medium'`, overwriting the task's existing value. +- How I discovered it: both the service and route tests for `PATCH /tasks/:id/complete` failed when a high-priority task came back as medium priority after completion. +- Why it happened: the completion logic rebuilt the task and hard-coded a new priority value. +- Fix: removed the priority overwrite and preserved the task's current priority. +- Status: fixed. diff --git a/SUBMISSION_NOTES.md b/SUBMISSION_NOTES.md new file mode 100644 index 00000000..da480ed6 --- /dev/null +++ b/SUBMISSION_NOTES.md @@ -0,0 +1,46 @@ +# Submission Notes + +## Coverage + +`npm run coverage -- --runInBand` + +| Metric | Result | +| --- | --- | +| Statements | 94.15% | +| Branches | 86.2% | +| Functions | 93.33% | +| Lines | 93.57% | + +## What I changed + +- Added unit tests for `taskService.js` and integration tests for all task routes with Jest and Supertest. +- Fixed three bugs uncovered by the test suite: + - pagination skipped the first page + - status filtering matched partial strings + - completing a task reset priority to `medium` +- Implemented `PATCH /tasks/:id/assign` with validation for a non-empty assignee string. + +## Design decisions for `PATCH /tasks/:id/assign` + +- I chose to trim the incoming assignee value before storing it so `" Priya "` is saved as `"Priya"`. +- Empty or whitespace-only assignees return `400` because an assignment with no usable name does not add value and usually indicates a client-side validation miss. +- Reassignment is allowed. If a task is already assigned, sending a new valid assignee updates that value and returns the latest task state. +- New tasks now include `assignee: null` so the task shape stays consistent before and after assignment. + +## What I would test next + +- Validation behavior for unusual pagination inputs such as negative numbers, `page=0`, and very large limits. +- Full task lifecycle consistency, especially how `PUT /tasks/:id` should behave if a completed task is moved back to another status. +- API behavior under concurrent updates, since the current store is in-memory and mutation-based. + +## Anything that surprised me + +- The README and the implementation describe different status vocabularies in a few places, so the tests had to follow the actual code paths instead of the prose alone. +- The bugs were small but meaningful, which is exactly why the service-level tests paid off quickly. + +## Questions I would ask before shipping to production + +- Should the documented status values be `todo / in_progress / done`, or should the implementation change to match the README wording? +- Do we want `PUT` to behave as a full replacement or the current partial-merge behavior? +- Should assignment support unassigning a task, and if so, should that be `null`, an empty payload, or a dedicated endpoint? +- What limits should we enforce on `title`, `description`, and `assignee` length? diff --git a/task-api/src/routes/tasks.js b/task-api/src/routes/tasks.js index e8c370fe..3896114c 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 { validateAssignTask, validateCreateTask, validateUpdateTask } = 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..0db9344e 100644 --- a/task-api/src/services/taskService.js +++ b/task-api/src/services/taskService.js @@ -6,10 +6,10 @@ 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 offset = (page - 1) * limit; return tasks.slice(offset, offset + limit); }; @@ -28,7 +28,14 @@ const getStats = () => { return { ...counts, overdue }; }; -const create = ({ title, description = '', status = 'todo', priority = 'medium', dueDate = null }) => { +const create = ({ + title, + description = '', + status = 'todo', + priority = 'medium', + dueDate = null, + assignee = null, +}) => { const task = { id: uuidv4(), title, @@ -36,6 +43,7 @@ const create = ({ title, description = '', status = 'todo', priority = 'medium', status, priority, dueDate, + assignee, completedAt: null, createdAt: new Date().toISOString(), }; @@ -66,7 +74,6 @@ const completeTask = (id) => { const updated = { ...task, - priority: 'medium', status: 'done', completedAt: new Date().toISOString(), }; @@ -76,6 +83,19 @@ const completeTask = (id) => { return updated; }; +const assignTask = (id, assignee) => { + const index = tasks.findIndex((t) => t.id === id); + if (index === -1) return null; + + const updated = { + ...tasks[index], + assignee: assignee.trim(), + }; + + tasks[index] = updated; + return updated; +}; + const _reset = () => { tasks = []; }; @@ -90,5 +110,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..0821f5ed 100644 --- a/task-api/src/utils/validators.js +++ b/task-api/src/utils/validators.js @@ -33,4 +33,12 @@ const validateUpdateTask = (body) => { return null; }; -module.exports = { validateCreateTask, validateUpdateTask }; +const validateAssignTask = (body) => { + if (!body || typeof body.assignee !== 'string' || 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..a0fb16d7 --- /dev/null +++ b/task-api/tests/taskService.test.js @@ -0,0 +1,159 @@ +const taskService = require('../src/services/taskService'); + +describe('taskService', () => { + beforeEach(() => { + taskService._reset(); + }); + + describe('create', () => { + test('creates a task with defaults', () => { + const task = taskService.create({ + title: 'Write tests', + }); + + expect(task).toEqual( + expect.objectContaining({ + title: 'Write tests', + description: '', + status: 'todo', + priority: 'medium', + dueDate: null, + completedAt: null, + assignee: null, + }) + ); + expect(task.id).toEqual(expect.any(String)); + expect(task.createdAt).toEqual(expect.any(String)); + }); + }); + + describe('getByStatus', () => { + test('returns only exact status matches', () => { + const todoTask = taskService.create({ title: 'Todo item', status: 'todo' }); + const inProgressTask = taskService.create({ title: 'In progress item', status: 'in_progress' }); + + const results = taskService.getByStatus('progress'); + + expect(results).toEqual([]); + expect(results).not.toContain(inProgressTask); + expect(results).not.toContain(todoTask); + }); + }); + + describe('getPaginated', () => { + test('returns the first page when page is 1', () => { + const first = taskService.create({ title: 'First task' }); + const second = taskService.create({ title: 'Second task' }); + taskService.create({ title: 'Third task' }); + + const results = taskService.getPaginated(1, 2); + + expect(results).toHaveLength(2); + expect(results.map((task) => task.id)).toEqual([first.id, second.id]); + }); + + test('returns an empty array when the requested page is beyond the data set', () => { + taskService.create({ title: 'Only task' }); + + const results = taskService.getPaginated(3, 10); + + expect(results).toEqual([]); + }); + }); + + describe('getStats', () => { + test('counts tasks by status and overdue tasks', () => { + const overdueDate = new Date(Date.now() - 60_000).toISOString(); + const futureDate = new Date(Date.now() + 60_000).toISOString(); + + taskService.create({ title: 'Todo overdue', status: 'todo', dueDate: overdueDate }); + taskService.create({ title: 'In progress', status: 'in_progress', dueDate: futureDate }); + taskService.create({ title: 'Done overdue but finished', status: 'done', dueDate: overdueDate }); + + expect(taskService.getStats()).toEqual({ + todo: 1, + in_progress: 1, + done: 1, + overdue: 1, + }); + }); + }); + + describe('update', () => { + test('updates an existing task', () => { + const task = taskService.create({ title: 'Original title', priority: 'low' }); + + const updated = taskService.update(task.id, { + title: 'Updated title', + priority: 'high', + }); + + expect(updated).toEqual( + expect.objectContaining({ + id: task.id, + title: 'Updated title', + priority: 'high', + }) + ); + }); + + test('returns null when the task does not exist', () => { + expect(taskService.update('missing-id', { title: 'Nope' })).toBeNull(); + }); + }); + + describe('remove', () => { + test('removes an existing task', () => { + const task = taskService.create({ title: 'Delete me' }); + + const removed = taskService.remove(task.id); + + expect(removed).toBe(true); + expect(taskService.findById(task.id)).toBeUndefined(); + }); + + test('returns false when the task does not exist', () => { + expect(taskService.remove('missing-id')).toBe(false); + }); + }); + + describe('completeTask', () => { + test('marks a task as done without changing its priority', () => { + const task = taskService.create({ title: 'Ship feature', priority: 'high' }); + + const completed = taskService.completeTask(task.id); + + expect(completed).toEqual( + expect.objectContaining({ + id: task.id, + status: 'done', + priority: 'high', + }) + ); + expect(completed.completedAt).toEqual(expect.any(String)); + }); + + test('returns null when the task does not exist', () => { + expect(taskService.completeTask('missing-id')).toBeNull(); + }); + }); + + describe('assignTask', () => { + test('stores the trimmed assignee on the task', () => { + const task = taskService.create({ title: 'Assign me' }); + + const updated = taskService.assignTask(task.id, ' Priya '); + + expect(updated).toEqual( + expect.objectContaining({ + id: task.id, + assignee: 'Priya', + }) + ); + }); + + test('returns null when the task does not exist', () => { + expect(taskService.assignTask('missing-id', 'Priya')).toBeNull(); + }); + }); +}); diff --git a/task-api/tests/tasks.routes.test.js b/task-api/tests/tasks.routes.test.js new file mode 100644 index 00000000..0d0144c7 --- /dev/null +++ b/task-api/tests/tasks.routes.test.js @@ -0,0 +1,252 @@ +const request = require('supertest'); +const app = require('../src/app'); +const taskService = require('../src/services/taskService'); + +describe('tasks API', () => { + beforeEach(() => { + taskService._reset(); + }); + + describe('GET /tasks', () => { + test('returns all tasks', async () => { + const first = taskService.create({ title: 'First task' }); + const second = taskService.create({ title: 'Second task', status: 'in_progress' }); + + const response = await request(app).get('/tasks'); + + expect(response.status).toBe(200); + expect(response.body).toHaveLength(2); + expect(response.body.map((task) => task.id)).toEqual([first.id, second.id]); + }); + + test('filters by exact status', async () => { + const todoTask = taskService.create({ title: 'Todo task', status: 'todo' }); + taskService.create({ title: 'In progress task', status: 'in_progress' }); + + const response = await request(app).get('/tasks?status=todo'); + + expect(response.status).toBe(200); + expect(response.body).toHaveLength(1); + expect(response.body[0].id).toBe(todoTask.id); + }); + + test('returns the first page when page=1 and limit is set', async () => { + const first = taskService.create({ title: 'Page task 1' }); + const second = taskService.create({ title: 'Page task 2' }); + taskService.create({ title: 'Page task 3' }); + + const response = await request(app).get('/tasks?page=1&limit=2'); + + expect(response.status).toBe(200); + expect(response.body).toHaveLength(2); + expect(response.body.map((task) => task.id)).toEqual([first.id, second.id]); + }); + }); + + describe('GET /tasks/stats', () => { + test('returns counts by status and overdue tasks', async () => { + const overdueDate = new Date(Date.now() - 60_000).toISOString(); + const futureDate = new Date(Date.now() + 60_000).toISOString(); + + taskService.create({ title: 'Todo overdue', status: 'todo', dueDate: overdueDate }); + taskService.create({ title: 'In progress future', status: 'in_progress', dueDate: futureDate }); + taskService.create({ title: 'Done overdue', status: 'done', dueDate: overdueDate }); + + const response = await request(app).get('/tasks/stats'); + + expect(response.status).toBe(200); + expect(response.body).toEqual({ + todo: 1, + in_progress: 1, + done: 1, + overdue: 1, + }); + }); + }); + + describe('POST /tasks', () => { + test('creates a task', async () => { + const response = await request(app).post('/tasks').send({ + title: 'Write integration tests', + description: 'Cover the routes with Supertest', + priority: 'high', + }); + + expect(response.status).toBe(201); + expect(response.body).toEqual( + expect.objectContaining({ + title: 'Write integration tests', + description: 'Cover the routes with Supertest', + status: 'todo', + priority: 'high', + assignee: null, + }) + ); + }); + + test('returns 400 when title is missing', async () => { + const response = await request(app).post('/tasks').send({ + priority: 'high', + }); + + expect(response.status).toBe(400); + expect(response.body).toEqual({ + error: 'title is required and must be a non-empty string', + }); + }); + + test('returns 400 when dueDate is invalid', async () => { + const response = await request(app).post('/tasks').send({ + title: 'Bad due date', + dueDate: 'not-a-date', + }); + + expect(response.status).toBe(400); + expect(response.body).toEqual({ + error: 'dueDate must be a valid ISO date string', + }); + }); + }); + + describe('PUT /tasks/:id', () => { + test('updates an existing task', async () => { + const task = taskService.create({ title: 'Before update', priority: 'low' }); + + const response = await request(app).put(`/tasks/${task.id}`).send({ + title: 'After update', + priority: 'high', + }); + + expect(response.status).toBe(200); + expect(response.body).toEqual( + expect.objectContaining({ + id: task.id, + title: 'After update', + priority: 'high', + }) + ); + }); + + test('returns 404 when the task does not exist', async () => { + const response = await request(app).put('/tasks/missing-id').send({ + title: 'Missing task', + }); + + expect(response.status).toBe(404); + expect(response.body).toEqual({ error: 'Task not found' }); + }); + + test('returns 400 when the payload is invalid', async () => { + const task = taskService.create({ title: 'Keep me' }); + + const response = await request(app).put(`/tasks/${task.id}`).send({ + title: '', + }); + + expect(response.status).toBe(400); + expect(response.body).toEqual({ + error: 'title must be a non-empty string', + }); + }); + }); + + describe('DELETE /tasks/:id', () => { + test('deletes an existing task', async () => { + const task = taskService.create({ title: 'Delete me' }); + + const response = await request(app).delete(`/tasks/${task.id}`); + + expect(response.status).toBe(204); + expect(taskService.findById(task.id)).toBeUndefined(); + }); + + test('returns 404 when the task does not exist', async () => { + const response = await request(app).delete('/tasks/missing-id'); + + expect(response.status).toBe(404); + expect(response.body).toEqual({ error: 'Task not found' }); + }); + }); + + describe('PATCH /tasks/:id/complete', () => { + test('marks a task as complete', async () => { + const task = taskService.create({ title: 'Finish me', priority: 'high' }); + + const response = await request(app).patch(`/tasks/${task.id}/complete`); + + expect(response.status).toBe(200); + expect(response.body).toEqual( + expect.objectContaining({ + id: task.id, + status: 'done', + priority: 'high', + }) + ); + expect(response.body.completedAt).toEqual(expect.any(String)); + }); + + test('returns 404 when the task does not exist', async () => { + const response = await request(app).patch('/tasks/missing-id/complete'); + + expect(response.status).toBe(404); + expect(response.body).toEqual({ error: 'Task not found' }); + }); + }); + + describe('PATCH /tasks/:id/assign', () => { + test('assigns a task to a user', async () => { + const task = taskService.create({ title: 'Assign me' }); + + const response = await request(app).patch(`/tasks/${task.id}/assign`).send({ + assignee: 'Asha', + }); + + expect(response.status).toBe(200); + expect(response.body).toEqual( + expect.objectContaining({ + id: task.id, + assignee: 'Asha', + }) + ); + }); + + test('reassigns a task when it is already assigned', async () => { + const task = taskService.create({ title: 'Already assigned' }); + taskService.assignTask(task.id, 'Rahul'); + + const response = await request(app).patch(`/tasks/${task.id}/assign`).send({ + assignee: 'Neha', + }); + + expect(response.status).toBe(200); + expect(response.body).toEqual( + expect.objectContaining({ + id: task.id, + assignee: 'Neha', + }) + ); + }); + + test('returns 400 when assignee is empty', async () => { + const task = taskService.create({ title: 'Needs a person' }); + + const response = await request(app).patch(`/tasks/${task.id}/assign`).send({ + assignee: ' ', + }); + + expect(response.status).toBe(400); + expect(response.body).toEqual({ + error: 'assignee must be a non-empty string', + }); + }); + + test('returns 404 when the task does not exist', async () => { + const response = await request(app).patch('/tasks/missing-id/assign').send({ + assignee: 'Asha', + }); + + expect(response.status).toBe(404); + expect(response.body).toEqual({ error: 'Task not found' }); + }); + }); +});