Take-home assignment — Bug reports, fixes, and assign feature

BUG.FIX.md

@@ -0,0 +1,25 @@
+## 🐞 Bug 01: Status Validation Logic
+
+**Status:** ✅ Fixed
+
+### 🔧 Resolution
+Updated the validation logic in `src/utils/validators.js` to handle user input inconsistencies.  
+Instead of validating raw input, status values are now normalized by trimming whitespace and converting to lowercase before validation.
+
+### 💻 Code Change
+```javascript
+// Before
+const isValid = ['todo', 'in_progress', 'done'].includes(status);
+
+// After
+const normalizedStatus = status?.trim().toLowerCase();
+const isValid = ['todo', 'in_progress', 'done'].includes(normalizedStatus);
+
+### Verification (Proof of Fix)
+
+Executed the Jest test suite for this bug. All test cases passed successfully, confirming proper handling of case variations and whitespace.
+Bug 01 — Status accepts case variants and whitespace
+  √ should accept "TODO" (uppercase)
+  √ should accept " todo " (with spaces)
+  √ should accept "In_Progress" (mixed case)
+  √ should still reject completely invalid status

BUGS.md

@@ -0,0 +1,266 @@
+# Bug Report — Task API
+
+---
+
+## Bug 01: Status Validation Logic
+
+- **Location:** `src/utils/validators.js:8`
+
+- **The Issue:** The validator uses strict string matching for task status.
+
+- **Expected:** Should accept "TODO", "todo", or " todo " (normalized input).
+
+- **Actual:** Rejects any input that isn't exactly lowercase and trimmed, returning a 400 Bad Request.
+
+- **Fix:** Apply `.trim().toLowerCase()` to the input before validation.
+
+---
+
+## Bug 02: Title and Description Input Normalisation
+
+- **Location:** `src/utils/validators.js:5`
+
+- **The Issue:** The validator does not normalise `title` and `description` fields before storing them. Raw input is saved as-is, including leading/trailing whitespace.
+
+- **Expected:** `"Write unit tests for auth module    "` should be stored as `"Write unit tests for auth module"` and `"     hel lo       "` should be stored as `"hel lo"`.
+
+- **Actual:** Both fields are stored with the original whitespace intact, causing inconsistent data in the in-memory database.
+
+- **Fix:** Apply `.trim()` to `title` and `description` before validation and storage. For collapsing internal spaces use `.trim().replace(/\s+/g, ' ')`.
+
+---
+
+## Bug 03: Priority Validation Logic
+
+- **Location:** `src/utils/validators.js:11`
+
+- **The Issue:** The validator uses strict string matching for task priority.
+
+- **Expected:** Should accept "LOW", "lOw", or " low " (normalized input).
+
+- **Actual:** Rejects any input that isn't exactly lowercase and trimmed, returning a 400 Bad Request.
+
+- **Fix:** Apply `.trim().toLowerCase()` to the input before validation.
+
+---
+
+## Bug 04: String "null" Accepted as Valid Title
+
+- **Location:** `src/utils/validators.js:5`
+
+- **The Issue:** The validator only checks if `title` is falsy, meaning it catches actual `null` but does not guard against meaningless string values like `"null"`, `"undefined"`, or whitespace-only strings like `"   "`.
+
+- **Expected:** All of the following inputs should be rejected with a 400 Bad Request:
+  - `{ "title": "null" }`
+  - `{ "title": "undefined" }`
+  - `{ "title": "   " }`
+  - `{ "title": "" }`
+
+- **Actual:** `"null"` and `"undefined"` pass validation and get stored in the database as legitimate task titles since they are technically non-empty strings.
+
+- **Fix:** Explicitly check for these invalid string values after normalising the input:
+```javascript
+const invalidTitles = ['null', 'undefined', 'none', ''];
+if (!title || invalidTitles.includes(title.trim().toLowerCase())) {
+  return 'Title must be a valid non-empty string';
+}
+```
+
+---
+
+## Bug 05: No Validation on dueDate Field
+
+- **Location:** `src/utils/validators.js`
+
+- **The Issue:** The `dueDate` field accepts any value without validating format, past dates, or unrealistically far future dates.
+
+- **Expected:** Should reject any `dueDate` that is not in ISO 8601 format, is in the past, or is unrealistically far in the future, returning a 400 Bad Request.
+
+- **Actual:** All of the following were accepted and stored as-is with no error thrown:
+  - `"2026-03-02T12:00:00.000Z"` — past date
+  - `"2026-03-02"` — past date + wrong format
+  - `"10/11/2022"` — past date + completely wrong format
+  - `"2091-9-10"` — unrealistically far future + wrong format
+
+- **Fix:** Apply format checking, past date rejection, and future date capping before storage:
+```javascript
+const iso8601Regex = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}.\d{3}Z$/;
+if (!iso8601Regex.test(dueDate)) {
+  return 'dueDate must be in ISO 8601 format e.g. 2026-04-10T12:00:00.000Z';
+}
+if (new Date(dueDate) < new Date()) {
+  return 'dueDate cannot be in the past';
+}
+if (new Date(dueDate) > new Date('2060-01-01T00:00:00.000Z')) {
+  return 'dueDate is unrealistically far in the future';
+}
+```
+
+---
+
+## Bug 06: Duplicate Task Entries Not Rejected
+
+- **Location:** `src/utils/validators.js:31`
+
+- **The Issue:** The validator does not check for duplicate tasks before creating a new entry. A task with the exact same `title`, `description`, `status`, `priority`, and `dueDate` can be created multiple times without any error.
+
+- **Expected:** If a task with the same `title` and `dueDate` already exists in the database, the API should reject the request with a 409 Conflict error.
+
+- **Actual:** The following identical entries were both accepted and stored with different IDs, created at `2026-04-03T07:44:07.611Z` and `2026-04-03T07:46:49.644Z` with different UUIDs but otherwise identical data:
+```json
+{
+  "title": "hey there",
+  "description": "hello",
+  "status": "todo",
+  "priority": "low",
+  "dueDate": "2091-9-10"
+}
+```
+
+- **Fix:** Before creating a new task, check if a task with the same `title` and `dueDate` already exists:
+```javascript
+const isDuplicate = tasks.some(
+  (t) => t.title.trim().toLowerCase() === title.trim().toLowerCase()
+      && t.dueDate === dueDate
+);
+if (isDuplicate) {
+  return 'A task with the same title and due date already exists';
+}
+```
+
+---
+
+## Bug 07: Incorrect Pagination Offset Calculation
+
+- **Location:** `src/services/taskService.js`
+
+- **The Issue:** The pagination offset is calculated as `page * limit` instead of `(page - 1) * limit`, causing page 1 to skip the first set of results entirely and return wrong data for every page.
+
+- **Expected:** Sending `?page=1&limit=5` should return the first 5 tasks (index 0 to 4).
+
+- **Actual:** Sending `?page=1&limit=5` skips the first 5 tasks and returns index 5 to 9 instead, meaning page 1 behaves like page 2, page 2 behaves like page 3, and so on. The actual first page of results is never accessible.
+
+- **Example:**
+```javascript
+// With 10 tasks in DB, page=1, limit=5
+const offset = 1 * 5;       // ❌ offset = 5, skips first 5 tasks
+const offset = (1 - 1) * 5; // ✅ offset = 0, starts from beginning
+```
+
+- **Fix:** Subtract 1 from the page number before multiplying:
+```javascript
+const getPaginated = (page, limit) => {
+  const offset = (page - 1) * limit;
+  return tasks.slice(offset, offset + limit);
+};
+```
+
+---
+
+## Bug 08: `getByStatus` Uses Partial Match Instead of Exact Match
+
+- **Location:** `src/services/taskService.js`
+
+- **The Issue:** `.includes()` does partial string matching on status instead of exact matching.
+
+- **Expected:** `?status=do` should return no results since `"do"` is not a valid status.
+
+- **Actual:** `?status=do` returns both `todo` and `done` tasks since both strings contain the substring `"do"`.
+
+- **Fix:**
+```javascript
+tasks.filter((t) => t.status === status.trim().toLowerCase());
+```
+
+---
+
+## Bug 09: `completeTask` Forcefully Resets Priority to `medium`
+
+- **Location:** `src/services/taskService.js`
+
+- **The Issue:** Marking a task complete silently overwrites its existing priority with `"medium"` regardless of what it was before.
+
+- **Expected:** A task with `priority: "high"` that is marked as complete should remain `priority: "high"`.
+
+- **Actual:** Priority is hardcoded to `"medium"` on every task completion, losing the original priority value permanently.
+
+- **Fix:** Remove the hardcoded priority override:
+```javascript
+const updated = {
+  ...task,
+  // priority: 'medium'  ← remove this line
+  status: 'done',
+  completedAt: new Date().toISOString(),
+};
+```
+
+---
+
+## Bug 10: `PUT /:id` Allows Overwriting Protected Fields
+
+- **Location:** `src/services/taskService.js`
+
+- **The Issue:** The `update()` function spreads all incoming fields with no restrictions, allowing clients to overwrite fields that should never change.
+
+- **Expected:** `id`, `createdAt`, and `completedAt` should never be overwritable by the client.
+
+- **Actual:** Sending the following body via `PUT` gets accepted and stored without any error:
+```json
+{ "id": "fake-id", "createdAt": "2000-01-01" }
+```
+
+- **Fix:** Strip protected fields before merging:
+```javascript
+const update = (id, fields) => {
+  const { id: _, createdAt, completedAt, ...safeFields } = fields;
+  const updated = { ...tasks[index], ...safeFields };
+  tasks[index] = updated;
+  return updated;
+};
+```
+
+---
+
+## Bug 11: No Validation on `page` and `limit` Query Parameters
+
+- **Location:** `src/routes/tasks.js`
+
+- **The Issue:** Page and limit query parameters are never validated before being used for pagination.
+
+- **Expected:** All of the following should be rejected with a 400 Bad Request:
+  - `?page=-1&limit=5` — negative page number
+  - `?page=0&limit=5` — zero is not a valid page
+  - `?page=1&limit=99999` — absurdly large limit
+  - `?page=abc&limit=5` — non-numeric, silently falls back to 1
+
+- **Actual:** All of the above are accepted silently with no error thrown, causing unexpected results.
+
+- **Fix:**
+```javascript
+if (pageNum < 1 || limitNum < 1 || limitNum > 100) {
+  return res.status(400).json({ error: 'Invalid page or limit value' });
+}
+```
+
+---
+
+## Bug 12: `PUT /:id` Can Set `status: done` Without Setting `completedAt`
+
+- **Location:** `src/routes/tasks.js`
+
+- **The Issue:** The `PUT` endpoint bypasses the `completeTask()` logic entirely, allowing a task to be marked as `done` without a `completedAt` timestamp being set.
+
+- **Expected:** Any task with `status: "done"` should always have a `completedAt` timestamp, consistent with the behaviour of `PATCH /:id/complete`.
+
+- **Actual:** Sending `{ "status": "done" }` via `PUT` leaves `completedAt` as `null`, creating an inconsistency in the data.
+
+- **Fix:** Intercept the status change inside `update()` and set `completedAt` automatically:
+```javascript
+if (fields.status === 'done' && !tasks[index].completedAt) {
+  fields.completedAt = new Date().toISOString();
+}
+```
+
+---
+
+*Total Bugs Found: 12*

task-api/SUBMISSION.md

@@ -0,0 +1,44 @@
+
+## What I'd Test Next
+
+If I had more time I'd focus on:
+- **Load testing** — the in-memory array has no size limit, so I'd test 
+  how the API behaves under a large number of tasks (1000+) and whether 
+  pagination holds up under stress
+- **Concurrent requests** — since JavaScript is single-threaded but async, 
+  I'd test whether two simultaneous POST requests with the same title and 
+  dueDate could both slip past the duplicate check
+- **Edge cases on the assign endpoint** — specifically what happens when 
+  you try to assign a task that has already been marked as done
+
+---
+
+## What Surprised Me in the Codebase
+
+A few things stood out:
+- The `completeTask()` function silently resets priority to `"medium"` 
+  regardless of the original value — this looked intentional at first 
+  but is almost certainly a bug
+- The `getPaginated()` offset used `page * limit` instead of 
+  `(page - 1) * limit`, meaning page 1 always skipped the first set 
+  of results and the true first page was never accessible
+- The `getByStatus()` filter used `.includes()` instead of strict 
+  equality, so querying `?status=do` would return both `todo` and 
+  `done` tasks
+
+---
+
+## Questions I'd Ask Before Shipping to Production
+
+- **Persistence** — the data lives in memory and resets on every server 
+  restart. Is a database like PostgreSQL or MongoDB planned, or is this 
+  intentionally ephemeral?
+- **Authentication** — there's no auth layer. Should endpoints be 
+  protected? Who is allowed to assign, complete, or delete tasks?
+- **Assignee as a string vs a user reference** — right now assignee is 
+  a free-text name. Should it reference an actual user ID from an auth 
+  system instead?
+- **Concurrent write safety** — if two requests hit the server at the 
+  same time, can the duplicate check be bypassed due to a race condition?
+- **Rate limiting** — should the API have rate limiting before going live 
+  to prevent abuse?

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,30 @@ router.patch('/:id/complete', (req, res) => {
   res.json(task);
 });
 
-module.exports = router;
+// New — PATCH /tasks/:id/assign
+router.patch('/:id/assign', (req, res) => {
+  // Validate the assignee field
+  const error = validateAssignTask(req.body);
+  if (error) {
+    return res.status(400).json({ error });
+  }
+
+  const result = taskService.assignTask(req.params.id, req.body.assignee);
+
+  // Task not found
+  if (!result) {
+    return res.status(404).json({ error: 'Task not found' });
+  }
+
+  // Task already assigned to someone else — return 409 Conflict
+  if (result.alreadyAssigned) {
+    return res.status(409).json({
+      error: `Task is already assigned to ${result.task.assignee}`,
+      assignee: result.task.assignee,
+    });
+  }
+
+  res.json(result.task);
+});
+
+module.exports = router;