From 46499894b9843465c6b6da46f42cd9c93e13a3f6 Mon Sep 17 00:00:00 2001 From: pabitra Date: Sat, 4 Apr 2026 12:28:26 +0530 Subject: [PATCH] bugfix: fix three bugs in the codebase and write testcases for them --- task-api/BUG_REPORT.md | 122 ++++++++++ task-api/SUBMISSION_NOTES.md | 43 ++++ task-api/src/routes/tasks.js | 16 +- task-api/src/services/taskService.js | 16 +- task-api/src/utils/validators.js | 9 +- task-api/tests/routes.test.js | 338 +++++++++++++++++++++++++++ task-api/tests/taskService.test.js | 310 ++++++++++++++++++++++++ task-api/tests/validators.test.js | 106 +++++++++ 8 files changed, 955 insertions(+), 5 deletions(-) create mode 100644 task-api/BUG_REPORT.md create mode 100644 task-api/SUBMISSION_NOTES.md create mode 100644 task-api/tests/routes.test.js create mode 100644 task-api/tests/taskService.test.js create mode 100644 task-api/tests/validators.test.js diff --git a/task-api/BUG_REPORT.md b/task-api/BUG_REPORT.md new file mode 100644 index 00000000..be5e9f1d --- /dev/null +++ b/task-api/BUG_REPORT.md @@ -0,0 +1,122 @@ +# Bug Report — Task Manager API + +--- + +## Bug 1: `getByStatus` uses partial string matching instead of exact match + +**File:** `src/services/taskService.js` — line 9 +**Severity:** High + +### Expected behavior +Filtering by `status=todo` should return only tasks with status `"todo"`. + +### What actually happens +`getByStatus` uses `t.status.includes(status)` which performs a **substring match**. This means: +- `getByStatus('do')` matches both `"todo"` and `"done"` +- `getByStatus('in')` matches `"in_progress"` + +Any partial status string returns incorrect results. + +### How I discovered it +Wrote a unit test that creates tasks with different statuses and filtered with a partial string `"do"` — the test revealed it matched `"todo"` and `"done"`. + +### Fix +Replace `t.status.includes(status)` with `t.status === status` for strict equality. + +```diff +- const getByStatus = (status) => tasks.filter((t) => t.status.includes(status)); ++ const getByStatus = (status) => tasks.filter((t) => t.status === status); +``` + +**Status:** ✅ Fixed + +--- + +## Bug 2: `getPaginated` has off-by-one error — page 1 skips results + +**File:** `src/services/taskService.js` — line 12 +**Severity:** High + +### Expected behavior +Page 1 should return the first `limit` items (offset 0). + +### What actually happens +The offset is calculated as `page * limit` instead of `(page - 1) * limit`. So page 1 with limit 2 starts at index 2 (skipping the first 2 items), and page 0 would be needed to get the first page — which is not the expected API convention. + +### How I discovered it +Integration test for `GET /tasks?page=1&limit=2` returned `Task 3` instead of `Task 1` as the first result. + +### Fix +```diff +- const offset = page * limit; ++ const offset = (page - 1) * limit; +``` + +**Status:** ✅ Fixed + +--- + +## Bug 3: `completeTask` resets priority to `"medium"` + +**File:** `src/services/taskService.js` — line 69 +**Severity:** Medium + +### Expected behavior +Marking a task as complete should only change `status` to `"done"` and set `completedAt`. All other fields (including `priority`) should remain unchanged. + +### What actually happens +The `completeTask` function includes `priority: 'medium'` in the updated object, overwriting whatever priority the task originally had. + +### How I discovered it +Created a task with `priority: 'high'`, then completed it via `PATCH /tasks/:id/complete`. The response showed `priority: 'medium'` instead of `'high'`. + +### Fix +Remove the `priority: 'medium'` line from the `completeTask` function. + +```diff + const updated = { + ...task, +- priority: 'medium', + status: 'done', + completedAt: new Date().toISOString(), + }; +``` + +**Status:** ✅ Fixed + +--- + +## Bug 4: `update` allows overwriting internal fields (`id`, `createdAt`) + +**File:** `src/services/taskService.js` — line 50 +**Severity:** Low + +### Expected behavior +Internal/system-managed fields like `id` and `createdAt` should not be overwritable by a client update request. + +### What actually happens +The update function uses a plain spread: `{ ...tasks[index], ...fields }`. If a client sends `{ id: 'hacked' }` in the PUT body, the task's ID gets overwritten, making it unreachable by the original ID. + +### How I discovered it +Wrote a unit test that sends `{ id: 'hacked' }` as an update — it succeeded. + +### Suggested fix +Strip protected fields before applying the spread: + +```js +const { id, createdAt, ...safeFields } = fields; +const updated = { ...tasks[index], ...safeFields }; +``` + +**Status:** ⚠️ Not fixed (documented only — low severity, not required by assignment) + +--- + +## Summary + +| # | Bug | Severity | Fixed? | +|---|-----|----------|--------| +| 1 | `getByStatus` partial matching | High | ✅ | +| 2 | `getPaginated` off-by-one | High | ✅ | +| 3 | `completeTask` resets priority | Medium | ✅ | +| 4 | `update` allows internal field overwrite | Low | ⚠️ Documented | diff --git a/task-api/SUBMISSION_NOTES.md b/task-api/SUBMISSION_NOTES.md new file mode 100644 index 00000000..ae8a17ae --- /dev/null +++ b/task-api/SUBMISSION_NOTES.md @@ -0,0 +1,43 @@ +# Submission Notes + +## Coverage Summary + +``` +-|---------|----------|---------|---------|------------------- + | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s +-|---------|----------|---------|---------|------------------- + | 97.41 | 97.67 | 93.33 | 97.16 | +-|---------|----------|---------|---------|------------------- + +Test Suites: 3 passed, 3 total +Tests: 85 passed, 85 total +``` + +## What I'd test next if I had more time + +- **Concurrency / race conditions:** The in-memory store uses direct array mutation. Concurrent requests could cause data corruption (e.g., two updates to the same task at once). +- **Request body edge cases:** Sending deeply nested objects, extremely long strings, or unexpected content types (XML, form-data). +- **Error handler middleware:** The global error handler in `app.js` is currently uncovered. I'd test it by forcing an unhandled error in a route. +- **Performance/load testing:** How does the in-memory store handle thousands of tasks? Pagination would become critical. + +## What surprised me in the codebase + +- The `completeTask` function silently resetting `priority` to `"medium"` — this is the kind of bug that would be extremely hard to catch visually in production, since tasks still get marked as "done" correctly. +- The `getByStatus` using `.includes()` for string matching — it works for exact status names by coincidence (e.g., "todo" doesn't contain the substring "done"), but breaks for partial matches like "do". +- The `update` function allows overwriting `id` and `createdAt` via the spread, which could cause data integrity issues. + +## Questions I'd ask before shipping to production + +1. **Authentication/Authorization:** Who can create/assign/complete tasks? Should there be user accounts? +2. **Persistence:** The in-memory store means all data is lost on restart. What database should back this? +3. **Rate limiting:** Should we add rate limiting to prevent abuse? +4. **Input sanitization:** Should we sanitize title/description for XSS if they're ever rendered in a frontend? +5. **Assignee validation:** Should `assignee` be validated against a list of known users, or is it freeform text? +6. **Idempotency:** What should happen if you `PATCH /complete` on a task that's already done? Currently it overwrites `completedAt`. + +## Design decisions for `PATCH /tasks/:id/assign` + +- **Validation:** Requires `assignee` to be a non-empty string (whitespace-only is rejected). Non-string values (like numbers) are also rejected. +- **Re-assignment:** Allowed. If a task is already assigned, sending a new assignee simply overwrites the previous one. This keeps the API simple and follows the principle of least surprise. +- **Trimming:** The assignee name is trimmed before storage to normalize whitespace. +- **No unassign:** To unassign, we'd need a separate mechanism (or allow `null`). I chose not to allow empty strings as an unassign action since the assignment explicitly asks about empty string validation. diff --git a/task-api/src/routes/tasks.js b/task-api/src/routes/tasks.js index e8c370fe..0587ecc0 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.trim()); + 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..25bc42ac 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); }; @@ -66,7 +66,6 @@ const completeTask = (id) => { const updated = { ...task, - priority: 'medium', status: 'done', completedAt: new Date().toISOString(), }; @@ -76,6 +75,16 @@ 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 +99,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/routes.test.js b/task-api/tests/routes.test.js new file mode 100644 index 00000000..72dc6b7f --- /dev/null +++ b/task-api/tests/routes.test.js @@ -0,0 +1,338 @@ +const request = require('supertest'); +const app = require('../src/app'); +const taskService = require('../src/services/taskService'); + +describe('Task API Routes', () => { + beforeEach(() => { + taskService._reset(); + }); + + // ─── POST /tasks ───────────────────────────────────────────────── + + describe('POST /tasks', () => { + it('should create a task and return 201', async () => { + const res = await request(app) + .post('/tasks') + .send({ title: 'New task' }); + + expect(res.status).toBe(201); + expect(res.body).toMatchObject({ + title: 'New task', + status: 'todo', + priority: 'medium', + }); + expect(res.body.id).toBeDefined(); + }); + + it('should create a task with all fields', async () => { + const res = await request(app) + .post('/tasks') + .send({ + title: 'Full', + description: 'A full task', + status: 'in_progress', + priority: 'high', + dueDate: '2026-12-31T00:00:00.000Z', + }); + + expect(res.status).toBe(201); + expect(res.body.description).toBe('A full task'); + expect(res.body.status).toBe('in_progress'); + }); + + 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).toMatch(/title/i); + }); + + it('should return 400 for invalid status', async () => { + const res = await request(app) + .post('/tasks') + .send({ title: 'T', status: 'bad' }); + expect(res.status).toBe(400); + }); + + it('should return 400 for empty string title', async () => { + const res = await request(app) + .post('/tasks') + .send({ title: '' }); + expect(res.status).toBe(400); + }); + }); + + // ─── GET /tasks ────────────────────────────────────────────────── + + describe('GET /tasks', () => { + it('should return 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 () => { + taskService.create({ title: 'A' }); + taskService.create({ 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=', () => { + it('should filter tasks by status', async () => { + taskService.create({ title: 'A', status: 'todo' }); + taskService.create({ title: 'B', status: 'done' }); + taskService.create({ title: 'C', status: 'in_progress' }); + + const res = await request(app).get('/tasks?status=todo'); + expect(res.status).toBe(200); + expect(res.body).toHaveLength(1); + expect(res.body[0].title).toBe('A'); + }); + + it('should return empty array for unmatched status', async () => { + taskService.create({ title: 'A', status: 'todo' }); + + const res = await request(app).get('/tasks?status=done'); + expect(res.status).toBe(200); + expect(res.body).toEqual([]); + }); + }); + + // ─── GET /tasks?page=&limit= ───────────────────────────────────── + + describe('GET /tasks?page=&limit=', () => { + beforeEach(() => { + for (let i = 1; i <= 5; i++) { + taskService.create({ title: `Task ${i}` }); + } + }); + + it('should return first page', 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'); + }); + + it('should return second page', 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'); + }); + + it('should default to page=1 limit=10 if not supplied', async () => { + const res = await request(app).get('/tasks?page='); + expect(res.status).toBe(200); + expect(res.body).toHaveLength(5); + }); + }); + + // ─── PUT /tasks/:id ────────────────────────────────────────────── + + describe('PUT /tasks/:id', () => { + it('should update a task', async () => { + const task = taskService.create({ title: 'Old' }); + + const res = await request(app) + .put(`/tasks/${task.id}`) + .send({ title: 'New' }); + + expect(res.status).toBe(200); + expect(res.body.title).toBe('New'); + }); + + it('should return 404 for non-existent task', async () => { + const res = await request(app) + .put('/tasks/fake-id') + .send({ title: 'X' }); + expect(res.status).toBe(404); + }); + + it('should return 400 for invalid update data', async () => { + const task = taskService.create({ title: 'T' }); + const res = await request(app) + .put(`/tasks/${task.id}`) + .send({ title: '' }); + expect(res.status).toBe(400); + }); + + it('should preserve fields not included in the update', async () => { + const task = taskService.create({ title: 'T', priority: 'high' }); + const res = await request(app) + .put(`/tasks/${task.id}`) + .send({ title: 'Updated' }); + + expect(res.status).toBe(200); + expect(res.body.priority).toBe('high'); + }); + }); + + // ─── DELETE /tasks/:id ─────────────────────────────────────────── + + describe('DELETE /tasks/:id', () => { + it('should delete a task and return 204', async () => { + const task = taskService.create({ title: 'Bye' }); + + const res = await request(app).delete(`/tasks/${task.id}`); + expect(res.status).toBe(204); + }); + + it('should return 404 for non-existent task', async () => { + const res = await request(app).delete('/tasks/fake-id'); + expect(res.status).toBe(404); + }); + + it('should actually remove the task from the store', async () => { + const task = taskService.create({ title: 'Gone' }); + await request(app).delete(`/tasks/${task.id}`); + + const res = await request(app).get('/tasks'); + expect(res.body).toHaveLength(0); + }); + }); + + // ─── PATCH /tasks/:id/complete ─────────────────────────────────── + + describe('PATCH /tasks/:id/complete', () => { + it('should mark task as done', async () => { + const task = taskService.create({ title: 'Do it' }); + + 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 non-existent task', async () => { + const res = await request(app).patch('/tasks/fake-id/complete'); + expect(res.status).toBe(404); + }); + + it('should preserve original priority (BUG test)', async () => { + const task = taskService.create({ title: 'Urgent', priority: 'high' }); + + const res = await request(app).patch(`/tasks/${task.id}/complete`); + expect(res.body.priority).toBe('high'); + }); + }); + + // ─── GET /tasks/stats ──────────────────────────────────────────── + + describe('GET /tasks/stats', () => { + it('should return zeroes when empty', 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', async () => { + taskService.create({ title: 'A', status: 'todo' }); + taskService.create({ title: 'B', status: 'done' }); + taskService.create({ + title: 'C', + status: 'todo', + dueDate: '2020-01-01T00:00:00.000Z', + }); + + const res = await request(app).get('/tasks/stats'); + expect(res.body.todo).toBe(2); + expect(res.body.done).toBe(1); + expect(res.body.overdue).toBe(1); + }); + }); + + // ─── PATCH /tasks/:id/assign ───────────────────────────────────── + + describe('PATCH /tasks/:id/assign', () => { + it('should assign a user to a task', async () => { + const task = taskService.create({ 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'); + }); + + it('should return 404 for non-existent task', async () => { + const res = await request(app) + .patch('/tasks/fake-id/assign') + .send({ assignee: 'Bob' }); + expect(res.status).toBe(404); + }); + + it('should return 400 when assignee is missing', async () => { + const task = taskService.create({ title: 'T' }); + + const res = await request(app) + .patch(`/tasks/${task.id}/assign`) + .send({}); + expect(res.status).toBe(400); + }); + + it('should return 400 when assignee is empty string', async () => { + const task = taskService.create({ title: 'T' }); + + const res = await request(app) + .patch(`/tasks/${task.id}/assign`) + .send({ assignee: '' }); + expect(res.status).toBe(400); + }); + + it('should return 400 when assignee is whitespace only', async () => { + const task = taskService.create({ title: 'T' }); + + const res = await request(app) + .patch(`/tasks/${task.id}/assign`) + .send({ assignee: ' ' }); + expect(res.status).toBe(400); + }); + + it('should return 400 when assignee is not a string', async () => { + const task = taskService.create({ title: 'T' }); + + const res = await request(app) + .patch(`/tasks/${task.id}/assign`) + .send({ assignee: 123 }); + expect(res.status).toBe(400); + }); + + it('should allow re-assigning to a different person', async () => { + const task = taskService.create({ title: 'T' }); + + 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 persist the assignee in the store', async () => { + const task = taskService.create({ title: 'T' }); + + await request(app) + .patch(`/tasks/${task.id}/assign`) + .send({ assignee: 'Charlie' }); + + const res = await request(app).get('/tasks'); + expect(res.body[0].assignee).toBe('Charlie'); + }); + }); +}); diff --git a/task-api/tests/taskService.test.js b/task-api/tests/taskService.test.js new file mode 100644 index 00000000..50405b18 --- /dev/null +++ b/task-api/tests/taskService.test.js @@ -0,0 +1,310 @@ +const taskService = require('../src/services/taskService'); + +describe('taskService', () => { + beforeEach(() => { + taskService._reset(); + }); + + // ─── create ────────────────────────────────────────────────────── + + describe('create', () => { + it('should create a task with default values', () => { + const task = taskService.create({ title: 'Test task' }); + + expect(task).toMatchObject({ + title: 'Test task', + description: '', + status: 'todo', + priority: 'medium', + dueDate: null, + completedAt: null, + }); + expect(task.id).toBeDefined(); + expect(task.createdAt).toBeDefined(); + }); + + it('should create a task with all fields provided', () => { + const dueDate = '2026-12-31T00:00:00.000Z'; + const task = taskService.create({ + title: 'Full task', + description: 'desc', + status: 'in_progress', + priority: 'high', + dueDate, + }); + + expect(task).toMatchObject({ + title: 'Full task', + description: 'desc', + status: 'in_progress', + priority: 'high', + dueDate, + }); + }); + + it('should store the task so it appears in getAll', () => { + taskService.create({ title: 'A' }); + taskService.create({ title: 'B' }); + + expect(taskService.getAll()).toHaveLength(2); + }); + }); + + // ─── getAll ────────────────────────────────────────────────────── + + describe('getAll', () => { + it('should return an empty array when no tasks exist', () => { + expect(taskService.getAll()).toEqual([]); + }); + + it('should return a copy (mutations do not affect store)', () => { + taskService.create({ title: 'A' }); + const list = taskService.getAll(); + list.push({ fake: true }); + expect(taskService.getAll()).toHaveLength(1); + }); + }); + + // ─── findById ──────────────────────────────────────────────────── + + describe('findById', () => { + it('should return the correct task', () => { + const t = taskService.create({ title: 'Find me' }); + expect(taskService.findById(t.id)).toEqual(t); + }); + + it('should return undefined for non-existent id', () => { + expect(taskService.findById('nope')).toBeUndefined(); + }); + }); + + // ─── getByStatus ───────────────────────────────────────────────── + // BUG: uses .includes() instead of strict equality – "do" matches "done" and "todo" + + describe('getByStatus', () => { + it('should return tasks matching the exact status', () => { + taskService.create({ title: 'A', status: 'todo' }); + taskService.create({ title: 'B', status: 'done' }); + taskService.create({ title: 'C', status: 'in_progress' }); + + const todos = taskService.getByStatus('todo'); + expect(todos).toHaveLength(1); + expect(todos[0].title).toBe('A'); + }); + + it('should return empty array when no tasks match', () => { + taskService.create({ title: 'A', status: 'todo' }); + expect(taskService.getByStatus('done')).toEqual([]); + }); + + it('should not partially match status strings (BUG test)', () => { + taskService.create({ title: 'A', status: 'todo' }); + taskService.create({ title: 'B', status: 'done' }); + + // "do" should NOT match anything – but the current bug causes partial matches + const results = taskService.getByStatus('do'); + expect(results).toHaveLength(0); + }); + }); + + // ─── getPaginated ──────────────────────────────────────────────── + // BUG: offset should be (page - 1) * limit, not page * limit + + describe('getPaginated', () => { + beforeEach(() => { + for (let i = 1; i <= 5; i++) { + taskService.create({ title: `Task ${i}` }); + } + }); + + it('should return the first page correctly', () => { + 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 second page correctly', () => { + 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 remaining items on the last page', () => { + const result = taskService.getPaginated(3, 2); + expect(result).toHaveLength(1); + expect(result[0].title).toBe('Task 5'); + }); + + it('should return empty array when page is past the end', () => { + const result = taskService.getPaginated(10, 2); + expect(result).toHaveLength(0); + }); + }); + + // ─── getStats ──────────────────────────────────────────────────── + + describe('getStats', () => { + it('should return all zeroes when there are no tasks', () => { + expect(taskService.getStats()).toEqual({ + todo: 0, + in_progress: 0, + done: 0, + overdue: 0, + }); + }); + + it('should count tasks by status', () => { + taskService.create({ title: 'A', status: 'todo' }); + taskService.create({ title: 'B', status: 'todo' }); + taskService.create({ title: 'C', status: 'in_progress' }); + taskService.create({ title: 'D', 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 dueDate, not done)', () => { + taskService.create({ + title: 'Overdue', + status: 'todo', + dueDate: '2020-01-01T00:00:00.000Z', + }); + taskService.create({ + title: 'Overdue but done', + status: 'done', + dueDate: '2020-01-01T00:00:00.000Z', + }); + taskService.create({ + title: 'Not yet due', + status: 'todo', + dueDate: '2099-12-31T00:00:00.000Z', + }); + + const stats = taskService.getStats(); + expect(stats.overdue).toBe(1); + }); + }); + + // ─── update ────────────────────────────────────────────────────── + + describe('update', () => { + it('should update specified fields', () => { + const t = taskService.create({ title: 'Old' }); + const updated = taskService.update(t.id, { title: 'New', priority: 'high' }); + + expect(updated.title).toBe('New'); + expect(updated.priority).toBe('high'); + expect(updated.id).toBe(t.id); + }); + + it('should return null for non-existent id', () => { + expect(taskService.update('fake', { title: 'X' })).toBeNull(); + }); + + it('should persist updates in the store', () => { + const t = taskService.create({ title: 'Before' }); + taskService.update(t.id, { title: 'After' }); + expect(taskService.findById(t.id).title).toBe('After'); + }); + + it('should allow overwriting id and createdAt via spread (potential bug)', () => { + const t = taskService.create({ title: 'A' }); + const updated = taskService.update(t.id, { id: 'hacked', createdAt: 'tampered' }); + // Documenting this behavior — the spread allows internal field overwrites + expect(updated.id).toBe('hacked'); + }); + }); + + // ─── remove ────────────────────────────────────────────────────── + + describe('remove', () => { + it('should remove an existing task and return true', () => { + const t = taskService.create({ title: 'Bye' }); + expect(taskService.remove(t.id)).toBe(true); + expect(taskService.getAll()).toHaveLength(0); + }); + + it('should return false for non-existent id', () => { + expect(taskService.remove('nope')).toBe(false); + }); + }); + + // ─── completeTask ──────────────────────────────────────────────── + // BUG: resets priority to 'medium' + + describe('completeTask', () => { + it('should mark a task as done with a completedAt timestamp', () => { + const t = taskService.create({ title: 'Finish me' }); + const completed = taskService.completeTask(t.id); + + expect(completed.status).toBe('done'); + expect(completed.completedAt).toBeDefined(); + expect(new Date(completed.completedAt).getTime()).not.toBeNaN(); + }); + + it('should return null for non-existent id', () => { + expect(taskService.completeTask('nope')).toBeNull(); + }); + + it('should preserve the original priority (BUG test)', () => { + const t = taskService.create({ title: 'High priority', priority: 'high' }); + const completed = taskService.completeTask(t.id); + + expect(completed.priority).toBe('high'); + }); + + it('should update the task in the store', () => { + const t = taskService.create({ title: 'Store check' }); + taskService.completeTask(t.id); + + const stored = taskService.findById(t.id); + expect(stored.status).toBe('done'); + }); + }); + + // ─── assignTask ────────────────────────────────────────────────── + + describe('assignTask', () => { + it('should assign a person to a task', () => { + const t = taskService.create({ title: 'Assign me' }); + const result = taskService.assignTask(t.id, 'Alice'); + + expect(result.assignee).toBe('Alice'); + expect(result.id).toBe(t.id); + }); + + it('should return null for non-existent id', () => { + expect(taskService.assignTask('nope', 'Bob')).toBeNull(); + }); + + it('should overwrite a previous assignee', () => { + const t = taskService.create({ title: 'Reassign me' }); + taskService.assignTask(t.id, 'Alice'); + const result = taskService.assignTask(t.id, 'Bob'); + + expect(result.assignee).toBe('Bob'); + }); + + it('should persist the assignee in the store', () => { + const t = taskService.create({ title: 'Persist me' }); + taskService.assignTask(t.id, 'Charlie'); + + const stored = taskService.findById(t.id); + expect(stored.assignee).toBe('Charlie'); + }); + }); + + // ─── _reset ────────────────────────────────────────────────────── + + describe('_reset', () => { + it('should clear all tasks', () => { + taskService.create({ title: 'X' }); + taskService._reset(); + expect(taskService.getAll()).toEqual([]); + }); + }); +}); diff --git a/task-api/tests/validators.test.js b/task-api/tests/validators.test.js new file mode 100644 index 00000000..b1a0ce3f --- /dev/null +++ b/task-api/tests/validators.test.js @@ -0,0 +1,106 @@ +const { validateCreateTask, validateUpdateTask, validateAssignTask } = require('../src/utils/validators'); + +describe('validators', () => { + // ─── validateCreateTask ────────────────────────────────────────── + + describe('validateCreateTask', () => { + it('should return null for a valid task', () => { + expect(validateCreateTask({ title: 'Test' })).toBeNull(); + }); + + it('should return null when all optional fields are valid', () => { + expect( + validateCreateTask({ + title: 'Test', + status: 'in_progress', + priority: 'high', + dueDate: '2026-12-31T00:00:00.000Z', + }) + ).toBeNull(); + }); + + it('should reject missing title', () => { + expect(validateCreateTask({})).toMatch(/title/i); + }); + + it('should reject empty string title', () => { + expect(validateCreateTask({ title: '' })).toMatch(/title/i); + }); + + it('should reject whitespace-only title', () => { + expect(validateCreateTask({ title: ' ' })).toMatch(/title/i); + }); + + it('should reject non-string title', () => { + expect(validateCreateTask({ title: 123 })).toMatch(/title/i); + }); + + it('should reject invalid status', () => { + expect(validateCreateTask({ title: 'T', status: 'invalid' })).toMatch(/status/i); + }); + + it('should reject invalid priority', () => { + expect(validateCreateTask({ title: 'T', priority: 'urgent' })).toMatch(/priority/i); + }); + + it('should reject invalid dueDate', () => { + expect(validateCreateTask({ title: 'T', dueDate: 'not-a-date' })).toMatch(/dueDate/i); + }); + }); + + // ─── validateUpdateTask ────────────────────────────────────────── + + describe('validateUpdateTask', () => { + it('should return null when body is empty (no-op update)', () => { + expect(validateUpdateTask({})).toBeNull(); + }); + + it('should return null for valid fields', () => { + expect(validateUpdateTask({ title: 'Updated', priority: 'low' })).toBeNull(); + }); + + it('should reject empty string title', () => { + expect(validateUpdateTask({ title: '' })).toMatch(/title/i); + }); + + it('should reject whitespace-only title', () => { + expect(validateUpdateTask({ title: ' ' })).toMatch(/title/i); + }); + + it('should reject invalid status', () => { + expect(validateUpdateTask({ status: 'nope' })).toMatch(/status/i); + }); + + it('should reject invalid priority', () => { + expect(validateUpdateTask({ priority: 'critical' })).toMatch(/priority/i); + }); + + it('should reject invalid dueDate', () => { + expect(validateUpdateTask({ dueDate: 'xyz' })).toMatch(/dueDate/i); + }); + }); + + // ─── validateAssignTask ────────────────────────────────────────── + + describe('validateAssignTask', () => { + it('should return null for a valid assignee', () => { + expect(validateAssignTask({ assignee: 'Alice' })).toBeNull(); + }); + + it('should reject missing assignee', () => { + expect(validateAssignTask({})).toMatch(/assignee/i); + }); + + it('should reject empty string assignee', () => { + expect(validateAssignTask({ assignee: '' })).toMatch(/assignee/i); + }); + + it('should reject whitespace-only assignee', () => { + expect(validateAssignTask({ assignee: ' ' })).toMatch(/assignee/i); + }); + + it('should reject non-string assignee', () => { + expect(validateAssignTask({ assignee: 42 })).toMatch(/assignee/i); + }); + }); +});