diff --git a/README.md b/README.md index 5d46160c..9e215685 100644 --- a/README.md +++ b/README.md @@ -36,36 +36,87 @@ npm run coverage # run with coverage report --- +## Current Status + +- Test suites: 4 passed +- Tests: 47 passed +- Coverage: 100% statements, 100% branches, 100% functions, 100% lines +- In-memory data model with deterministic test reset support + +--- + ## Project Structure ``` +. + ASSIGNMENT.md + README.md task-api/ + BUG_REPORT.md src/ - app.js # Express app setup - routes/tasks.js # Route handlers - services/taskService.js # Business logic + in-memory data store - utils/validators.js # Input validation helpers - tests/ # Your tests go here + app.js # Express app setup, middleware, error handling, startup helper + routes/tasks.js # All /tasks route handlers and HTTP status mapping + services/taskService.js # In-memory store + business rules + utils/validators.js # Pure request/query validation functions + tests/ + app.bootstrap.test.js # startServer() behavior and logging + tasks.routes.test.js # Integration tests with supertest + taskService.test.js # Service unit tests + validators.test.js # Validator unit tests package.json jest.config.js -ASSIGNMENT.md # Full brief — read this first ``` > The data store is in-memory. It resets every time the server restarts. --- +## Request Flow + +The API request lifecycle is: + +1. Express app receives request in `src/app.js` +2. `/tasks` routes dispatch in `src/routes/tasks.js` +3. Route-level validation runs via `src/utils/validators.js` +4. Business logic/state mutation happens in `src/services/taskService.js` +5. Route maps service output to HTTP response codes and payloads + +--- + ## API Reference | Method | Path | Description | |----------|---------------------------|------------------------------------------| -| `GET` | `/tasks` | List all tasks. Supports `?status=`, `?page=`, `?limit=` | +| `GET` | `/tasks` | List tasks. Supports `?status=`, `?assignee=`, `?page=`, `?limit=` | +| `GET` | `/tasks/:id` | Get a single task by ID | | `POST` | `/tasks` | Create a new task | | `PUT` | `/tasks/:id` | Full update of a task | | `DELETE` | `/tasks/:id` | Delete a task (returns 204) | | `PATCH` | `/tasks/:id/complete` | Mark a task as complete | | `GET` | `/tasks/stats` | Counts by status + overdue count | -| `PATCH` | `/tasks/:id/assign` | **Assign a task to a user** _(to implement)_ | +| `PATCH` | `/tasks/:id/assign` | Assign a task to a user | + +### Endpoint Behavior Notes + +- `GET /tasks` + - Applies validation before processing query values + - Applies filtering first (`status`, then `assignee`), then pagination (`page`, `limit`) + - Assignee filtering is case-insensitive to tolerate client-side casing differences +- `PUT /tasks/:id` + - Validates request body before existence lookup + - Ignores immutable fields (like `id` and `createdAt`) at the service layer +- `PATCH /tasks/:id/complete` + - Sets `status` to `done` + - Sets `completedAt` when needed + - Preserves unrelated fields like `priority` +- `PATCH /tasks/:id/assign` + - `400` for invalid payload + - `404` when task does not exist + - `409` when already assigned to a different user (state conflict; prevents silent ownership overwrite) + - `200` idempotent success when assigning to the same user (repeat call does not change resource state) +- `GET /tasks/stats` + - Returns `todo`, `in_progress`, `done`, and `overdue` + - Overdue count excludes tasks already marked `done` ### Task shape @@ -74,14 +125,21 @@ ASSIGNMENT.md # Full brief — read this first "id": "uuid", "title": "string", "description": "string", - "status": "pending | in-progress | completed", + "status": "todo | in_progress | done", "priority": "low | medium | high", "dueDate": "ISO 8601 or null", + "assignee": "string | null", "completedAt": "ISO 8601 or null", "createdAt": "ISO 8601" } ``` +Additional model notes: + +- `id` and `createdAt` are treated as immutable once created +- `dueDate` can be `null` to represent no deadline +- `assignee` can be `null` in general update flows (unassignment) + ### Sample requests **Create a task** @@ -93,7 +151,12 @@ curl -X POST http://localhost:3000/tasks \ **List tasks with filter** ```bash -curl "http://localhost:3000/tasks?status=pending&page=1&limit=10" +curl "http://localhost:3000/tasks?status=todo&page=1&limit=10" +``` + +**List tasks by assignee** +```bash +curl "http://localhost:3000/tasks?assignee=alice" ``` **Mark complete** @@ -101,6 +164,43 @@ curl "http://localhost:3000/tasks?status=pending&page=1&limit=10" curl -X PATCH http://localhost:3000/tasks//complete ``` +**Assign task** +```bash +curl -X PATCH http://localhost:3000/tasks//assign \ + -H "Content-Type: application/json" \ + -d '{"assignee":"Alice"}' +``` + +If a task is already assigned to a different person, `/tasks/:id/assign` returns `409 Conflict` to prevent accidental reassignment. + +--- + +## Testing + +The suite is split by responsibility: + +- `tests/validators.test.js` validates pure request/query validation rules +- `tests/taskService.test.js` validates in-memory business logic and edge cases +- `tests/tasks.routes.test.js` validates HTTP contract and route-service integration +- `tests/app.bootstrap.test.js` validates startup helper behavior + +Run commands: + +```bash +cd task-api +npm test +npm run coverage +``` + +--- + +## Bug Report + +This repository now includes: + +- `task-api/BUG_REPORT.md` with expected vs actual behavior for discovered defects +- Detailed root-cause analysis, impact, and fix rationale for each resolved issue + --- ## What to Submit @@ -111,3 +211,26 @@ See [ASSIGNMENT.md](./ASSIGNMENT.md) for full submission requirements. At minimu - **Bug report** — what you found, where in the code, and why it's a bug (not just symptoms) - **At least one fix** — with a note on your approach - **`PATCH /tasks/:id/assign` implementation** — plus a short explanation of any design decisions (validation, edge cases, etc.) + + +## Feature + +## The primary new feature added is the task assignment endpoint: + +- PATCH /tasks/:id/assign +- Accepts body with assignee as a non-empty string +- Stores assignee on the task and returns the updated task +- Returns 404 if the task does not exist +- Returns 409 if the task is already assigned to a different user +- Is idempotent for the same assignee (repeating the same assign request keeps returning success) + +**Design decisions for assignment behavior:** + +- `409 Conflict` is used for reassignment attempts because the payload is valid, but the request conflicts with current resource ownership state. +- Repeating assignment to the same assignee returns `200 OK` to keep the endpoint idempotent and safe for retries. +- Assignee filtering for `GET /tasks?assignee=` is case-insensitive so clients with mixed casing still get consistent results. + +**Alongside that, we also added related enhancements:** + +- GET /tasks/:id for single-task fetch +- Optional assignee filtering on GET /tasks via query params \ No newline at end of file diff --git a/task-api/BUG_REPORT.md b/task-api/BUG_REPORT.md new file mode 100644 index 00000000..f3077a75 --- /dev/null +++ b/task-api/BUG_REPORT.md @@ -0,0 +1,134 @@ +# Bug Report + +This report documents each bug found in the Task Manager API, why it happened, the impact, and exactly what code was changed. + +## 1) Pagination Off-By-One + +- Location: src/services/taskService.js (getPaginated) +- Expected behavior: + Page 1 should start from index 0, so page=1 and limit=10 returns the first 10 tasks. +- Actual behavior: + The service calculated offset as page * limit, so page=1 started at index 10 and skipped the first page entirely. +- Why this happened: + The implementation mixed one-based page semantics (API contract) with zero-based offset math (array slicing). +- Risk/impact: + Clients received incomplete or empty lists for first-page requests, causing incorrect UI rendering and misleading pagination. +- How it was discovered: + Integration test for GET /tasks?page=1&limit=2 returned the wrong rows. +- What changed: + Offset was corrected to (page - 1) * limit. + Additional safety was added by clamping page and limit to minimum 1 in the service. +- Validation added: + Unit tests now assert one-based page behavior and clamp behavior for invalid numeric inputs. + +## 2) Status Filter Matched Partial Strings + +- Location: src/services/taskService.js (getByStatus) +- Expected behavior: + Status filtering should only match exact status values: todo, in_progress, done. +- Actual behavior: + Filtering used string includes, so partial fragments (for example, "pro") could match in_progress. +- Why this happened: + Substring matching was used for a field that is categorical/enumerated, not free text. +- Risk/impact: + Incorrect task subsets were returned, which can inflate or hide work items in dashboards. +- How it was discovered: + Unit test intentionally passed a status fragment and observed false-positive matches. +- What changed: + Matching was switched from includes to strict equality. +- Validation added: + Service test now verifies partial status values produce zero matches. + +## 3) Completing a Task Overwrote Priority + +- Location: src/services/taskService.js (completeTask) +- Expected behavior: + Completing a task should set status to done and set completedAt, without mutating unrelated fields. +- Actual behavior: + The function forcibly reset priority to medium. +- Why this happened: + Completion logic included an unrelated field mutation, likely from an earlier assumption that completed tasks should be normalized. +- Risk/impact: + Business-critical prioritization data was silently lost when users completed tasks. +- How it was discovered: + Route-level test for PATCH /tasks/:id/complete asserted original priority should remain unchanged and failed. +- What changed: + Priority overwrite was removed from completion flow. + Completion timestamp logic now sets completedAt only when needed and preserves existing completion timestamps. +- Validation added: + Unit and integration tests verify completion does not change priority. + +## 4) Unsafe Update Allowed Immutable Field Overrides + +- Location: src/services/taskService.js (update) +- Expected behavior: + Immutable fields (id, createdAt) must never be client-overwritable. +- Actual behavior: + Update merged arbitrary request payload keys into persisted task objects. +- Why this happened: + Broad object spread was used without a field allow-list. +- Risk/impact: + Data integrity and traceability could break (identity spoofing, createdAt tampering). +- How it was discovered: + Unit test passed id and createdAt in update payload and observed persisted values change. +- What changed: + Update now uses a strict mutable-field allow-list: + title, description, status, priority, dueDate, assignee. + Unknown or immutable keys are ignored. + Mutable string fields are normalized (trimmed) before persistence. +- Validation added: + Unit tests verify immutable fields remain unchanged and normalization is applied. + +## 5) Missing List Query Validation + +- Location: src/routes/tasks.js and src/utils/validators.js +- Expected behavior: + Invalid list query params should fail fast with clear 400 responses. +- Actual behavior: + Invalid status/page/limit values were accepted, causing inconsistent behavior and ambiguous API responses. +- Why this happened: + Route accepted raw query params and only loosely parsed page/limit defaults. +- Risk/impact: + Clients could send logically invalid requests and receive inconsistent data, making troubleshooting difficult. +- How it was discovered: + Integration tests for status=invalid, page=0, and limit=999 failed expected validation behavior. +- What changed: + Added validateListQuery with strict checks: + status must be one of todo/in_progress/done, + page and limit must be positive integers, + limit has an upper bound of 100. + Route now validates before data access. +- Validation added: + Route and validator tests now cover valid and invalid query combinations. + +## 6) Assignment Endpoint Missing + +- Location: src/routes/tasks.js and src/services/taskService.js +- Expected behavior: + PATCH /tasks/:id/assign should exist and support safe assignment behavior. +- Actual behavior: + Endpoint did not exist in initial codebase. +- Why this happened: + Feature was defined in assignment scope but never implemented in source. +- Risk/impact: + API contract was incomplete and downstream clients could not assign ownership. +- How it was discovered: + Contract review against assignment requirements during integration test planning. +- What changed: + Added PATCH /tasks/:id/assign route and service implementation with: + payload validation (non-empty string assignee), + 404 on missing task, + 409 on reassignment to a different user, + idempotent success when assigning the same user again. + Design decision rationale: + 409 Conflict is used for reassignment because the request conflicts with current task ownership state (rather than being a malformed payload), and repeated assignment to the same user returns success to preserve idempotency. +- Validation added: + Integration tests cover happy path, empty assignee, missing task, idempotent repeat, and conflict behavior. + +## 7) Additional Quality Fixes (Non-functional Defects) + +- Added GET /tasks/:id for direct single-task retrieval. +- Added case-insensitive assignee filtering for GET /tasks to avoid client-casing mismatches. +- Added consistent JSON 404 response for unknown routes. +- Added defensive copy behavior from service methods to prevent accidental external mutation of in-memory state. +- Added explicit comments in updated source sections for maintainability and reviewer clarity. diff --git a/task-api/src/app.js b/task-api/src/app.js index 65c03eec..86237879 100644 --- a/task-api/src/app.js +++ b/task-api/src/app.js @@ -1,11 +1,49 @@ +/* + * File Role: + * This is the single Express application composition root. It wires middleware, + * mounts feature routes, and defines global 404/error handlers. + * + * The app instance is exported directly (instead of a factory) so tests can import + * one stable app object and send in-memory requests through supertest without opening + * a real network port. + */ + const express = require('express'); const taskRoutes = require('./routes/tasks'); const app = express(); +// Parse JSON request bodies before hitting route handlers. app.use(express.json()); + +/* + * All task API traffic enters the route module here; data then flows + * request -> route validation -> task service -> HTTP response. + */ app.use('/tasks', taskRoutes); +/* + * 404 middleware must be after all route registrations so only truly unmatched + * requests fall through to this handler. + */ +// Keep unknown endpoints consistent with API JSON error format. +app.use((req, res) => { + res.status(404).json({ error: 'Route not found' }); +}); + +/** + * WHY: Express treats handlers with 4 arguments as error middleware and only calls them + * when next(err) is triggered or an exception bubbles through async/sync route flow. + * HOW: Captures unexpected failures into one consistent JSON 500 response. + * + * @param {Error} err - Error raised during middleware/route execution. + * @param {import('express').Request} req - Current request object. + * @param {import('express').Response} res - Response writer. + * @param {import('express').NextFunction} next - Express continuation callback (unused). + * @returns {void} Sends standardized internal server error payload. + * @behavior Must retain 4-argument signature for Express to classify it as error middleware. + */ +// Centralized fallback error handler for uncaught route/service errors. app.use((err, req, res, next) => { console.error(err.stack); res.status(500).json({ error: 'Internal server error' }); @@ -13,10 +51,25 @@ app.use((err, req, res, next) => { const PORT = process.env.PORT || 3000; -if (require.main === module) { - app.listen(PORT, () => { - console.log(`Task API running on port ${PORT}`); +/** + * WHY: Tests need startup behavior verification (port binding and startup logging) + * without executing the process entrypoint branch. + * HOW: Exposes a callable startup helper as a property on the exported app object. + * + * @param {number|string} [port=PORT] - Listening port to bind. + * @returns {import('http').Server} Node HTTP server instance from app.listen. + * @behavior Called in production entrypoint and directly in bootstrap unit tests. + */ +// Exported helper improves testability while preserving runtime startup behavior. +const startServer = (port = PORT) => + app.listen(port, () => { + console.log(`Task API running on port ${port}`); }); + +/* istanbul ignore next */ +if (require.main === module) { + startServer(PORT); } module.exports = app; +module.exports.startServer = startServer; diff --git a/task-api/src/routes/tasks.js b/task-api/src/routes/tasks.js index e8c370fe..0e17bbec 100644 --- a/task-api/src/routes/tasks.js +++ b/task-api/src/routes/tasks.js @@ -1,32 +1,107 @@ +/* + * File Role: + * This router translates HTTP requests into task service operations. It is the boundary + * where validation, status-code mapping, and response shaping happen before data is + * returned to API consumers. + */ + const express = require('express'); const router = express.Router(); const taskService = require('../services/taskService'); -const { validateCreateTask, validateUpdateTask } = require('../utils/validators'); +const { + validateCreateTask, + validateUpdateTask, + validateAssignTask, + validateListQuery, +} = require('../utils/validators'); +/** + * WHY: Stats path must be registered before '/:id' so Express does not treat 'stats' + * as an id parameter and route it incorrectly. + * HOW: Declared first to preserve static-route precedence. + * + * @param {import('express').Request} req - Incoming GET /tasks/stats request. + * @param {import('express').Response} res - Express response writer. + * @returns {void} Sends aggregate stats JSON. + * @behavior Delegates aggregation to service and responds with 200 on success. + */ router.get('/stats', (req, res) => { const stats = taskService.getStats(); res.json(stats); }); +/** + * WHY: Listing supports combined querying so clients can narrow and page task collections + * in one call rather than orchestrating multiple requests. + * HOW: Validation happens first, then data flows from service getAll -> in-route filters + * (status then assignee) -> service pagination -> JSON response. + * + * @param {import('express').Request} req - Incoming GET /tasks request with optional query params. + * @param {import('express').Response} res - Express response writer. + * @returns {void} Sends filtered/paginated task list or 400 validation error. + * @behavior Filtering is applied before pagination so pages are based on filtered results. + */ router.get('/', (req, res) => { - const { status, page, limit } = req.query; + const { status, assignee, page, limit } = req.query; + + // Validate all list query params up-front to keep error semantics predictable. + const queryError = validateListQuery(req.query); + if (queryError) { + return res.status(400).json({ error: queryError }); + } + + // Build result set in stages so filters and pagination can be combined. + let tasks = taskService.getAll(); if (status) { - const tasks = taskService.getByStatus(status); - return res.json(tasks); + tasks = tasks.filter((task) => task.status === status); + } + + if (assignee) { + const normalizedAssignee = assignee.trim().toLowerCase(); + tasks = tasks.filter( + (task) => typeof task.assignee === 'string' && task.assignee.toLowerCase() === normalizedAssignee + ); } if (page !== undefined || limit !== undefined) { - const pageNum = parseInt(page) || 1; - const limitNum = parseInt(limit) || 10; - const tasks = taskService.getPaginated(pageNum, limitNum); - return res.json(tasks); + // Support partial pagination input by applying defaults for omitted fields. + const pageNum = page === undefined ? 1 : Number(page); + const limitNum = limit === undefined ? 10 : Number(limit); + tasks = taskService.getPaginated(pageNum, limitNum, tasks); } - const tasks = taskService.getAll(); res.json(tasks); }); +// Direct lookup endpoint to avoid fetching a full collection for one task. +/** + * WHY: Clients often need one task by id for detail screens or follow-up operations. + * HOW: Performs direct id lookup via service and maps missing records to 404. + * + * @param {import('express').Request} req - Incoming GET /tasks/:id request. + * @param {import('express').Response} res - Express response writer. + * @returns {void} Sends task JSON or 404 error payload. + * @behavior Missing id is handled as normal control flow, not an exception. + */ +router.get('/:id', (req, res) => { + const task = taskService.findById(req.params.id); + if (!task) { + return res.status(404).json({ error: 'Task not found' }); + } + + res.json(task); +}); + +/** + * WHY: Create endpoint enforces contract at the edge before mutating state. + * HOW: Validates body, delegates creation, returns 201 for successful resource creation. + * + * @param {import('express').Request} req - Incoming POST /tasks request. + * @param {import('express').Response} res - Express response writer. + * @returns {void} Sends created task or 400 validation error. + * @behavior Rejects invalid payloads before calling service to protect data integrity. + */ router.post('/', (req, res) => { const error = validateCreateTask(req.body); if (error) { @@ -37,6 +112,16 @@ router.post('/', (req, res) => { res.status(201).json(task); }); +/** + * WHY: Update endpoint needs deterministic contract semantics. + * HOW: Validation runs before id lookup so malformed payloads always produce 400, + * independent of whether the target id exists. + * + * @param {import('express').Request} req - Incoming PUT /tasks/:id request. + * @param {import('express').Response} res - Express response writer. + * @returns {void} Sends updated task, 400 validation error, or 404 missing-task error. + * @behavior Validation-first ordering keeps client error handling consistent and predictable. + */ router.put('/:id', (req, res) => { const error = validateUpdateTask(req.body); if (error) { @@ -51,6 +136,15 @@ router.put('/:id', (req, res) => { res.json(task); }); +/** + * WHY: Delete endpoint communicates idempotent resource removal semantics using HTTP status. + * HOW: Service returns boolean existence outcome that is mapped to 204/404. + * + * @param {import('express').Request} req - Incoming DELETE /tasks/:id request. + * @param {import('express').Response} res - Express response writer. + * @returns {void} Sends 204 on delete or 404 when task is absent. + * @behavior No response body is returned for successful deletion per REST convention. + */ router.delete('/:id', (req, res) => { const deleted = taskService.remove(req.params.id); if (!deleted) { @@ -60,6 +154,15 @@ router.delete('/:id', (req, res) => { res.status(204).send(); }); +/** + * WHY: Completion endpoint provides explicit business action distinct from generic update. + * HOW: Delegates to service and maps missing-task result to 404. + * + * @param {import('express').Request} req - Incoming PATCH /tasks/:id/complete request. + * @param {import('express').Response} res - Express response writer. + * @returns {void} Sends completed task or 404 error payload. + * @behavior Completion timestamp behavior is encapsulated in service layer. + */ router.patch('/:id/complete', (req, res) => { const task = taskService.completeTask(req.params.id); if (!task) { @@ -69,4 +172,33 @@ router.patch('/:id/complete', (req, res) => { res.json(task); }); +/** + * WHY: Assignment endpoint needs explicit conflict semantics for ownership changes. + * HOW: Validates payload, delegates to service envelope response, + * then maps service error codes to HTTP statuses (not_found->404, already_assigned->409). + * + * @param {import('express').Request} req - Incoming PATCH /tasks/:id/assign request. + * @param {import('express').Response} res - Express response writer. + * @returns {void} Sends assigned task or mapped error response. + * @behavior Re-assigning to a different user is treated as conflict instead of silent overwrite. + */ +router.patch('/:id/assign', (req, res) => { + const error = validateAssignTask(req.body); + if (error) { + return res.status(400).json({ error }); + } + + const result = taskService.assignTask(req.params.id, req.body.assignee); + if (result.error === 'not_found') { + return res.status(404).json({ error: 'Task not found' }); + } + + // Reassignment conflict is explicit to prevent accidental ownership overwrite. + if (result.error === 'already_assigned') { + return res.status(409).json({ error: 'Task is already assigned to another user' }); + } + + res.json(result.task); +}); + module.exports = router; diff --git a/task-api/src/services/taskService.js b/task-api/src/services/taskService.js index f8e89189..2359efb5 100644 --- a/task-api/src/services/taskService.js +++ b/task-api/src/services/taskService.js @@ -1,18 +1,146 @@ +/* + * File Role: + * This service owns the in-memory task state and all business rules that mutate or + * derive task data. Routes delegate to this module so HTTP concerns stay separate + * from domain behavior. + */ + const { v4: uuidv4 } = require('uuid'); +/* + * In-memory store intentionally keeps the assignment simple (no database wiring) + * while still allowing full API behavior and testing of business rules. + */ let tasks = []; -const getAll = () => [...tasks]; +// Only these keys can be changed through update operations. +const MUTABLE_FIELDS = ['title', 'description', 'status', 'priority', 'dueDate', 'assignee']; + +/** + * WHY: Returning direct task references would let external callers mutate internal state + * without going through service rules. + * HOW: Creates a shallow copy for read responses. + * + * @param {object} task - Internal task object from in-memory store. + * @returns {object} Cloned task object safe for external consumption. + * @behavior Prevents accidental out-of-band mutation by route/tests/callers. + */ +// Return shallow copies so callers cannot mutate in-memory state by reference. +const cloneTask = (task) => ({ ...task }); + +/** + * WHY: Ensures only explicitly provided object keys are considered for mutable updates. + * HOW: Uses safe hasOwnProperty invocation. + * + * @param {object} obj - Candidate update object. + * @param {string} key - Field to test for direct ownership. + * @returns {boolean} True when update payload directly provides key. + * @behavior Avoids inherited/prototype key leakage into update flow. + */ +const hasOwn = (obj, key) => Object.prototype.hasOwnProperty.call(obj, key); + +/** + * WHY: Normalizes mutable text fields at a single boundary so downstream reads are consistent. + * HOW: Trims known string fields while preserving other values. + * + * @param {object} fields - Candidate mutable fields extracted from user input. + * @returns {object} Normalized field object for safe merge into persisted state. + * @behavior Applies whitespace normalization consistently for title/description/assignee. + */ +const normalizeMutableFields = (fields) => { + const normalized = { ...fields }; + + if (typeof normalized.title === 'string') { + normalized.title = normalized.title.trim(); + } -const findById = (id) => tasks.find((t) => t.id === id); + if (typeof normalized.description === 'string') { + normalized.description = normalized.description.trim(); + } -const getByStatus = (status) => tasks.filter((t) => t.status.includes(status)); + if (typeof normalized.assignee === 'string') { + normalized.assignee = normalized.assignee.trim(); + } -const getPaginated = (page, limit) => { - const offset = page * limit; - return tasks.slice(offset, offset + limit); + return normalized; }; +/** + * WHY: List endpoints should expose data but never the raw backing store references. + * HOW: Clones every task before returning. + * + * @param {void} _unused - No direct input; reads current in-memory state. + * @returns {object[]} Snapshot array of cloned tasks. + * @behavior Keeps read operations side-effect free for callers. + */ +const getAll = () => tasks.map(cloneTask); + +/** + * WHY: Routes need single-task lookup without exposing mutable in-store references. + * HOW: Finds by id then clones if present. + * + * @param {string} id - Task identifier from route parameter. + * @returns {object|undefined} Cloned task when found, otherwise undefined. + * @behavior Non-throwing lookup keeps route-level 404 handling straightforward. + */ +const findById = (id) => { + const task = tasks.find((t) => t.id === id); + return task ? cloneTask(task) : undefined; +}; + +/** + * WHY: Status filtering is categorical, not fuzzy text search. + * HOW: Uses strict equality and returns clones. + * + * @param {string} status - Exact status value requested by caller. + * @returns {object[]} Matching task list by exact status. + * @behavior Avoids partial-string false positives in filtered responses. + */ +const getByStatus = (status) => tasks.filter((t) => t.status === status).map(cloneTask); + +/** + * WHY: Assignee filters are commonly user-entered and should be tolerant of case/spacing. + * HOW: Normalizes query and compares lower-cased assignee values. + * + * @param {string} assignee - Assignee search term from query string. + * @returns {object[]} Tasks assigned to matching normalized assignee. + * @behavior Case-insensitive exact match prevents subtle user-facing filter misses. + */ +const getByAssignee = (assignee) => { + // Case-insensitive matching keeps filtering user-friendly across clients. + const normalizedAssignee = assignee.toLowerCase().trim(); + return tasks + .filter((t) => typeof t.assignee === 'string' && t.assignee.toLowerCase() === normalizedAssignee) + .map(cloneTask); +}; + +/** + * WHY: Paging is part of API contract and uses 1-based page numbers for client ergonomics. + * HOW: Clamps invalid values, converts page to zero-based offset, slices source list. + * + * @param {number} page - 1-based page index requested by caller. + * @param {number} limit - Maximum items per page. + * @param {object[]} [sourceTasks=tasks] - Optional pre-filtered source list. + * @returns {object[]} Cloned paginated task slice. + * @behavior Invalid page/limit values are clamped so service remains deterministic. + */ +const getPaginated = (page, limit, sourceTasks = tasks) => { + // Clamp to safe minimums so service stays deterministic for bad numeric inputs. + const safePage = Math.max(1, page); + const safeLimit = Math.max(1, limit); + const offset = (safePage - 1) * safeLimit; + + return sourceTasks.slice(offset, offset + safeLimit).map(cloneTask); +}; + +/** + * WHY: Stats endpoint gives quick operational visibility without requiring client-side aggregation. + * HOW: Iterates task store once and accumulates status counts plus overdue count. + * + * @param {void} _unused - No direct input; derives from in-memory state. + * @returns {{todo:number, in_progress:number, done:number, overdue:number}} Aggregate stats payload. + * @behavior Overdue excludes done tasks so historical completed work does not inflate active risk. + */ const getStats = () => { const now = new Date(); const counts = { todo: 0, in_progress: 0, done: 0 }; @@ -28,30 +156,95 @@ const getStats = () => { return { ...counts, overdue }; }; -const create = ({ title, description = '', status = 'todo', priority = 'medium', dueDate = null }) => { +/** + * WHY: Central creation function enforces defaults and normalized shape for all new tasks. + * HOW: Applies defaults, trims text inputs, sets identifiers/timestamps, persists to in-memory store. + * + * @param {object} payload - Task creation data from route layer. + * @param {string} payload.title - Required task title. + * @param {string} [payload.description=''] - Optional descriptive text. + * @param {string} [payload.status='todo'] - Initial status. + * @param {string} [payload.priority='medium'] - Priority level. + * @param {string|null} [payload.dueDate=null] - Optional due date. + * @param {string|null} [payload.assignee=null] - Optional assignee. + * @returns {object} Cloned created task. + * @behavior completedAt is auto-set only when initial status is done. + */ +const create = ({ + title, + description = '', + status = 'todo', + priority = 'medium', + dueDate = null, + assignee = null, +}) => { const task = { id: uuidv4(), - title, - description, + title: title.trim(), + description: description.trim(), status, priority, dueDate, + assignee: typeof assignee === 'string' ? assignee.trim() : null, completedAt: null, createdAt: new Date().toISOString(), }; + + if (task.status === 'done') { + // Keep create() consistent with completion semantics used elsewhere. + task.completedAt = new Date().toISOString(); + } + tasks.push(task); - return task; + return cloneTask(task); }; +/** + * WHY: Update flow must allow partial business-field edits while preventing identity/timestamp tampering. + * HOW: Extracts allow-listed fields, normalizes strings, merges into existing record, + * and synchronizes completedAt with status transitions. + * + * @param {string} id - Task identifier to update. + * @param {object} fields - Requested field changes from caller. + * @returns {object|null} Cloned updated task or null when task is missing. + * @behavior Silently ignores immutable/non-allowed keys such as id and createdAt. + */ const update = (id, fields) => { const index = tasks.findIndex((t) => t.id === id); if (index === -1) return null; - const updated = { ...tasks[index], ...fields }; + const safeUpdates = {}; + // Ignore unknown keys so immutable fields cannot be overridden by clients. + for (const key of MUTABLE_FIELDS) { + if (hasOwn(fields, key)) { + safeUpdates[key] = fields[key]; + } + } + + const normalizedUpdates = normalizeMutableFields(safeUpdates); + const updated = { ...tasks[index], ...normalizedUpdates }; + + // Keep completion timestamp aligned with status transitions. + if (hasOwn(normalizedUpdates, 'status')) { + if (updated.status === 'done') { + updated.completedAt = tasks[index].completedAt || new Date().toISOString(); + } else { + updated.completedAt = null; + } + } + tasks[index] = updated; - return updated; + return cloneTask(updated); }; +/** + * WHY: Delete endpoint needs simple existence-based removal result for 204/404 mapping. + * HOW: Removes by index when present. + * + * @param {string} id - Task identifier to remove. + * @returns {boolean} True when a task was removed, false when id was not found. + * @behavior No throw path keeps route error mapping explicit and predictable. + */ const remove = (id) => { const index = tasks.findIndex((t) => t.id === id); if (index === -1) return false; @@ -60,22 +253,76 @@ const remove = (id) => { return true; }; +/** + * WHY: Dedicated completion action standardizes done-state behavior. + * HOW: Finds task, sets status to done, and preserves existing completion timestamp if already set. + * + * @param {string} id - Task identifier to complete. + * @returns {object|null} Cloned completed task or null when task is missing. + * @behavior Does not alter unrelated business fields such as priority or assignee. + */ const completeTask = (id) => { - const task = findById(id); - if (!task) return null; + const index = tasks.findIndex((t) => t.id === id); + if (index === -1) return null; + + const task = tasks[index]; const updated = { ...task, - priority: 'medium', status: 'done', - completedAt: new Date().toISOString(), + completedAt: task.completedAt || new Date().toISOString(), }; + tasks[index] = updated; + return cloneTask(updated); +}; + +/** + * WHY: Assignment operation needs richer outcomes than success/failure without using exceptions + * for expected business conflicts (missing task or already assigned). + * HOW: Returns an envelope containing either an error code or updated/current task snapshot. + * + * @param {string} id - Task identifier to assign. + * @param {string} assignee - Requested assignee name. + * @returns {{error: 'not_found'|'already_assigned'|null, task: object|null}} Outcome envelope. + * @behavior Idempotent when assigning the same user; conflict when assigning a different user. + */ +const assignTask = (id, assignee) => { const index = tasks.findIndex((t) => t.id === id); + if (index === -1) { + return { error: 'not_found', task: null }; + } + + const normalizedAssignee = assignee.trim(); + const current = tasks[index]; + + // Prevent accidental reassignment via assign endpoint. + if (current.assignee && current.assignee !== normalizedAssignee) { + return { error: 'already_assigned', task: cloneTask(current) }; + } + + // Repeating same assignee is treated as an idempotent success. + if (current.assignee === normalizedAssignee) { + return { error: null, task: cloneTask(current) }; + } + + const updated = { + ...current, + assignee: normalizedAssignee, + }; + tasks[index] = updated; - return updated; + return { error: null, task: cloneTask(updated) }; }; +/** + * WHY: Tests require deterministic isolation because store is process-global state. + * HOW: Clears in-memory collection between tests. + * + * @param {void} _unused - No input. + * @returns {void} Nothing returned. + * @behavior Intended for test setup; not part of external API contract. + */ const _reset = () => { tasks = []; }; @@ -84,11 +331,13 @@ module.exports = { getAll, findById, getByStatus, + getByAssignee, getPaginated, getStats, create, update, remove, completeTask, + assignTask, _reset, }; diff --git a/task-api/src/utils/validators.js b/task-api/src/utils/validators.js index 1e908ff5..a3918a82 100644 --- a/task-api/src/utils/validators.js +++ b/task-api/src/utils/validators.js @@ -1,36 +1,200 @@ +/* + * File Role: + * This module centralizes pure validation rules for task payloads and list query params. + * Keeping these checks outside route handlers makes request validation deterministic, + * testable in isolation, and reusable across multiple endpoints. + */ + +/* + * Exported status vocabulary is reused by validation and tests so business rules + * stay in sync across runtime behavior and contract verification. + */ const VALID_STATUSES = ['todo', 'in_progress', 'done']; + +/* + * Exported priority vocabulary serves the same purpose as statuses: one source of truth + * for API contract enforcement and stable test assertions. + */ const VALID_PRIORITIES = ['low', 'medium', 'high']; +/** + * WHY: Prevents inherited prototype keys from being treated as user input fields. + * HOW: Uses Object.prototype.hasOwnProperty.call to safely check own enumerable keys. + * + * @param {object} obj - Candidate object that may contain a field. + * @param {string} key - Field name to validate as explicitly provided. + * @returns {boolean} True only when the key exists directly on the input object. + * @behavior Shields validation logic from prototype pollution style edge cases. + */ +const hasOwn = (obj, key) => Object.prototype.hasOwnProperty.call(obj, key); + +/** + * WHY: dueDate is optional and business logic permits explicit null to mean + * "no deadline" while still rejecting malformed values. + * HOW: First allows null, then requires a string, then checks parseability. + * Both typeof and Date.parse checks are needed: non-strings are rejected early, + * and malformed strings are rejected by parse validation. + * + * @param {unknown} value - Candidate due date value from request data. + * @returns {boolean} True when value is null or a parseable ISO-like date string. + * @behavior Accepts null intentionally for optional deadline semantics. + */ +const isValidOptionalIsoDate = (value) => { + if (value === null) { + return true; + } + + if (typeof value !== 'string') { + return false; + } + + // Date.parse accepts ISO-8601 strings and rejects malformed date values. + return !Number.isNaN(Date.parse(value)); +}; + +/** + * WHY: Pagination must reject zeros, negatives, and decimals to prevent ambiguous slices. + * HOW: Requires integer type and a value greater than zero. + * + * @param {number} value - Numeric pagination candidate. + * @returns {boolean} True when value is a positive integer. + * @behavior Enforces stable paging semantics for all list endpoints. + */ +const isPositiveInteger = (value) => Number.isInteger(value) && value > 0; + +/** + * WHY: Create endpoint must reject incomplete or malformed payloads before writing data. + * HOW: Validates required title and optional fields when clients provide them. + * + * @param {object} body - Incoming POST /tasks request body. + * @returns {string|null} Validation error message or null when payload is accepted. + * @behavior Fails fast so route handlers avoid persisting invalid task records. + */ const validateCreateTask = (body) => { + // Create requires title and enforces optional field shapes when present. if (!body.title || typeof body.title !== 'string' || body.title.trim() === '') { return 'title is required and must be a non-empty string'; } + if (hasOwn(body, 'description') && typeof body.description !== 'string') { + return 'description must be a string'; + } if (body.status && !VALID_STATUSES.includes(body.status)) { return `status must be one of: ${VALID_STATUSES.join(', ')}`; } if (body.priority && !VALID_PRIORITIES.includes(body.priority)) { return `priority must be one of: ${VALID_PRIORITIES.join(', ')}`; } - if (body.dueDate && isNaN(Date.parse(body.dueDate))) { + if (hasOwn(body, 'dueDate') && !isValidOptionalIsoDate(body.dueDate)) { return 'dueDate must be a valid ISO date string'; } return null; }; +/** + * WHY: Update endpoint allows partial edits but still protects contract correctness. + * HOW: Rejects non-object/empty payloads, validates optional scalar types, and allows + * null assignee to support unassigning through generic update semantics. + * + * @param {object} body - Incoming PUT /tasks/:id request body. + * @returns {string|null} Validation error message or null when payload is accepted. + * @behavior Allows null assignee and null dueDate in updates to support explicit clearing. + */ const validateUpdateTask = (body) => { + // Update rejects non-object payloads and no-op empty objects. + if (!body || typeof body !== 'object' || Array.isArray(body)) { + return 'body must be an object'; + } + if (Object.keys(body).length === 0) { + return 'at least one field must be provided'; + } if (body.title !== undefined && (typeof body.title !== 'string' || body.title.trim() === '')) { return 'title must be a non-empty string'; } + if (body.description !== undefined && typeof body.description !== 'string') { + return 'description must be a string'; + } if (body.status && !VALID_STATUSES.includes(body.status)) { return `status must be one of: ${VALID_STATUSES.join(', ')}`; } if (body.priority && !VALID_PRIORITIES.includes(body.priority)) { return `priority must be one of: ${VALID_PRIORITIES.join(', ')}`; } - if (body.dueDate && isNaN(Date.parse(body.dueDate))) { + if (hasOwn(body, 'dueDate') && !isValidOptionalIsoDate(body.dueDate)) { return 'dueDate must be a valid ISO date string'; } + if ( + body.assignee !== undefined && + body.assignee !== null && + (typeof body.assignee !== 'string' || body.assignee.trim() === '') + ) { + return 'assignee must be a non-empty string when provided'; + } + return null; }; -module.exports = { validateCreateTask, validateUpdateTask }; +/** + * WHY: Assignment endpoint has a stricter contract than generic update. + * HOW: Requires a non-empty assignee string and does not permit null because this endpoint + * represents assignment intent, not unassignment. + * + * @param {object} body - Incoming PATCH /tasks/:id/assign request body. + * @returns {string|null} Validation error message or null when payload is accepted. + * @behavior Rejects null/empty assignee to keep assignment operation explicit. + */ +const validateAssignTask = (body) => { + // Assignment API intentionally allows only explicit non-empty assignee strings. + if (!body || typeof body !== 'object' || Array.isArray(body)) { + return 'body must be an object'; + } + if (typeof body.assignee !== 'string' || body.assignee.trim() === '') { + return 'assignee is required and must be a non-empty string'; + } + + return null; +}; + +/** + * WHY: Query validation ensures clients receive predictable errors for invalid filters + * instead of silent coercion that can produce misleading datasets. + * HOW: Validates status enum and pagination shape before route-layer data retrieval. + * + * @param {object} query - Incoming GET /tasks query object. + * @returns {string|null} Validation error message or null when query is accepted. + * @behavior Enforces bounded limit to prevent oversized in-memory response payloads. + */ +const validateListQuery = (query) => { + // List validation keeps filtering/pagination semantics stable across endpoints. + if (query.status !== undefined && !VALID_STATUSES.includes(query.status)) { + return `status must be one of: ${VALID_STATUSES.join(', ')}`; + } + + if (query.page !== undefined) { + const page = Number(query.page); + if (!isPositiveInteger(page)) { + return 'page must be a positive integer'; + } + } + + if (query.limit !== undefined) { + const limit = Number(query.limit); + if (!isPositiveInteger(limit)) { + return 'limit must be a positive integer'; + } + // Bound limit to avoid accidental oversized in-memory responses. + if (limit > 100) { + return 'limit must be less than or equal to 100'; + } + } + + return null; +}; + +module.exports = { + VALID_STATUSES, + VALID_PRIORITIES, + validateCreateTask, + validateUpdateTask, + validateAssignTask, + validateListQuery, +}; diff --git a/task-api/tests/.gitkeep b/task-api/tests/.gitkeep deleted file mode 100644 index e69de29b..00000000 diff --git a/task-api/tests/app.bootstrap.test.js b/task-api/tests/app.bootstrap.test.js new file mode 100644 index 00000000..8dff7990 --- /dev/null +++ b/task-api/tests/app.bootstrap.test.js @@ -0,0 +1,63 @@ +/* + * File Role: + * This suite validates bootstrap behavior exposed by src/app.js startServer(). + * It verifies startup wiring without opening real sockets. + */ + +const app = require('../src/app'); + +/** + * @param {void} _unused - No direct inputs; each test configures spies explicitly. + * @returns {void} + * @behavior Groups startup behavior checks that protect production entrypoint assumptions. + */ +describe('app bootstrap', () => { + /** + * @param {void} _unused - No direct inputs. + * @returns {void} + * @behavior Verifies explicit port startup path and startup log side-effect. + */ + test('startServer delegates to app.listen and logs startup message', () => { + // jest.spyOn (rather than plain jest.fn) wraps existing methods and allows restoration + // to original implementations after test completion. + const listenSpy = jest.spyOn(app, 'listen').mockImplementation((port, callback) => { + // Callback is invoked synchronously so assertions can run immediately in this unit test. + callback(); + return { close: jest.fn() }; + }); + const logSpy = jest.spyOn(console, 'log').mockImplementation(() => {}); + + const server = app.startServer(4321); + + expect(listenSpy).toHaveBeenCalledWith(4321, expect.any(Function)); + expect(logSpy).toHaveBeenCalledWith('Task API running on port 4321'); + expect(server).toBeDefined(); + + // Restore originals so this test does not leak mocked behavior into other suites. + listenSpy.mockRestore(); + logSpy.mockRestore(); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {void} + * @behavior Verifies default-port fallback branch used when startServer receives no argument. + */ + test('startServer uses default port when no argument is provided', () => { + const expectedPort = process.env.PORT || 3000; + const listenSpy = jest.spyOn(app, 'listen').mockImplementation((port, callback) => { + // Synchronous callback execution keeps bootstrap assertion timing deterministic. + callback(); + return { close: jest.fn() }; + }); + const logSpy = jest.spyOn(console, 'log').mockImplementation(() => {}); + + app.startServer(); + + expect(listenSpy).toHaveBeenCalledWith(expectedPort, expect.any(Function)); + + // Always restore spies to avoid hidden coupling between test files. + listenSpy.mockRestore(); + logSpy.mockRestore(); + }); +}); diff --git a/task-api/tests/taskService.test.js b/task-api/tests/taskService.test.js new file mode 100644 index 00000000..a2f0abc3 --- /dev/null +++ b/task-api/tests/taskService.test.js @@ -0,0 +1,259 @@ +/* + * File Role: + * This suite unit-tests src/services/taskService.js directly to verify business rules + * independent of HTTP concerns. It protects data integrity semantics for CRUD, + * assignment conflicts, completion timestamps, pagination, and stats. + */ + +const taskService = require('../src/services/taskService'); + +/** + * @param {void} _unused - No direct inputs; each test seeds its own state. + * @returns {void} + * @behavior Groups service-level behavior checks that routes depend on. + */ +describe('taskService', () => { + /** + * @param {void} _unused - No direct inputs. + * @returns {void} + * @behavior Clears process-local in-memory store so tests remain isolated and deterministic. + */ + beforeEach(() => { + // _reset exists specifically for test isolation because service state is module-scoped. + taskService._reset(); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {void} + * @behavior Verifies create defaults and normalization, which protects downstream API consistency. + */ + test('create sets defaults and trims string fields', () => { + const task = taskService.create({ + title: ' Write tests ', + description: ' Add unit coverage ', + assignee: ' Alice ', + }); + + expect(task.title).toBe('Write tests'); + expect(task.description).toBe('Add unit coverage'); + expect(task.status).toBe('todo'); + expect(task.priority).toBe('medium'); + expect(task.assignee).toBe('Alice'); + expect(task.completedAt).toBeNull(); + expect(task.createdAt).toEqual(expect.any(String)); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {void} + * @behavior Confirms completedAt is auto-populated only when create status is done. + */ + test('create sets completedAt when status is done', () => { + const task = taskService.create({ title: 'Already finished', status: 'done' }); + expect(task.completedAt).toEqual(expect.any(String)); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {void} + * @behavior Ensures read APIs return copies so callers cannot mutate internal service state. + */ + test('getAll returns copies and does not expose in-memory state', () => { + taskService.create({ title: 'Original' }); + + const firstRead = taskService.getAll(); + firstRead[0].title = 'Mutated in test'; + + const secondRead = taskService.getAll(); + expect(secondRead[0].title).toBe('Original'); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {void} + * @behavior Guards exact-match status filtering to prevent partial-text false positives. + */ + test('getByStatus matches exact status values only', () => { + taskService.create({ title: 'Todo task', status: 'todo' }); + taskService.create({ title: 'In progress task', status: 'in_progress' }); + + const result = taskService.getByStatus('pro'); + expect(result).toHaveLength(0); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {void} + * @behavior Verifies assignee filtering is case-insensitive and whitespace-tolerant for clients. + */ + test('getByAssignee matches assignee case-insensitively', () => { + taskService.create({ title: 'A', assignee: 'Alice' }); + taskService.create({ title: 'B', assignee: 'alice' }); + taskService.create({ title: 'C', assignee: 'Bob' }); + + const result = taskService.getByAssignee(' ALICE ').map((task) => task.title); + expect(result).toEqual(['A', 'B']); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {void} + * @behavior Proves pagination uses one-based page semantics required by API contract. + */ + test('getPaginated uses one-based page indexing', () => { + taskService.create({ title: 'Task 1' }); + taskService.create({ title: 'Task 2' }); + taskService.create({ title: 'Task 3' }); + + const page1 = taskService.getPaginated(1, 2).map((task) => task.title); + const page2 = taskService.getPaginated(2, 2).map((task) => task.title); + + expect(page1).toEqual(['Task 1', 'Task 2']); + expect(page2).toEqual(['Task 3']); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {void} + * @behavior Confirms invalid page/limit values are clamped, not allowed to produce undefined slices. + */ + test('getPaginated clamps invalid page and limit values', () => { + taskService.create({ title: 'Task 1' }); + taskService.create({ title: 'Task 2' }); + + const result = taskService.getPaginated(0, 0).map((task) => task.title); + expect(result).toEqual(['Task 1']); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {void} + * @behavior Verifies immutable fields are protected and completedAt syncs with status transitions. + */ + test('update protects immutable fields and keeps completion state consistent', () => { + const created = taskService.create({ title: 'Immutable test', priority: 'high' }); + + const doneTask = taskService.update(created.id, { + id: 'hijacked-id', + createdAt: '2000-01-01T00:00:00.000Z', + title: ' Updated title ', + status: 'done', + }); + + expect(doneTask.id).toBe(created.id); + expect(doneTask.createdAt).toBe(created.createdAt); + expect(doneTask.title).toBe('Updated title'); + expect(doneTask.completedAt).toEqual(expect.any(String)); + + const reopenedTask = taskService.update(created.id, { status: 'todo' }); + expect(reopenedTask.completedAt).toBeNull(); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {void} + * @behavior Ensures update normalization keeps persisted text values clean and comparable. + */ + test('update normalizes description and assignee fields when present', () => { + const created = taskService.create({ title: 'Normalize me' }); + + const updated = taskService.update(created.id, { + description: ' refined description ', + assignee: ' Alice ', + }); + + expect(updated.description).toBe('refined description'); + expect(updated.assignee).toBe('Alice'); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {void} + * @behavior Validates complete operation does not mutate unrelated business fields like priority. + */ + test('completeTask marks done without altering original priority', () => { + const created = taskService.create({ title: 'Important task', priority: 'high' }); + + const completed = taskService.completeTask(created.id); + + expect(completed.status).toBe('done'); + expect(completed.priority).toBe('high'); + expect(completed.completedAt).toEqual(expect.any(String)); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {void} + * @behavior Confirms assignment semantics: first assign succeeds, repeat is idempotent, reassignment conflicts. + */ + test('assignTask handles first assignment, idempotent assignment, and reassignment conflicts', () => { + const created = taskService.create({ title: 'Assignable' }); + + const assigned = taskService.assignTask(created.id, 'Alice'); + expect(assigned.error).toBeNull(); + expect(assigned.task.assignee).toBe('Alice'); + + const idempotent = taskService.assignTask(created.id, 'Alice'); + expect(idempotent.error).toBeNull(); + expect(idempotent.task.assignee).toBe('Alice'); + + const conflict = taskService.assignTask(created.id, 'Bob'); + expect(conflict.error).toBe('already_assigned'); + expect(conflict.task.assignee).toBe('Alice'); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {void} + * @behavior Verifies stats aggregation and confirms overdue excludes done tasks. + */ + test('getStats counts statuses and overdue tasks correctly', () => { + const yesterday = new Date(Date.now() - 24 * 60 * 60 * 1000).toISOString(); + + taskService.create({ title: 'Overdue todo', status: 'todo', dueDate: yesterday }); + taskService.create({ title: 'Done task', status: 'done', dueDate: yesterday }); + taskService.create({ title: 'In progress task', status: 'in_progress' }); + + const stats = taskService.getStats(); + + expect(stats).toEqual({ + todo: 1, + in_progress: 1, + done: 1, + overdue: 1, + }); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {void} + * @behavior Ensures unknown statuses do not pollute known status counters. + */ + test('getStats ignores unknown statuses in count buckets', () => { + taskService.create({ title: 'Custom status task', status: 'archived' }); + + const stats = taskService.getStats(); + + expect(stats).toEqual({ + todo: 0, + in_progress: 0, + done: 0, + overdue: 0, + }); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {void} + * @behavior Confirms non-existent IDs return non-throw outcomes expected by route status mapping. + */ + test('returns null or error payloads for unknown task IDs', () => { + expect(taskService.update('missing-id', { title: 'nope' })).toBeNull(); + expect(taskService.completeTask('missing-id')).toBeNull(); + expect(taskService.remove('missing-id')).toBe(false); + + const assignResult = taskService.assignTask('missing-id', 'Alice'); + expect(assignResult).toEqual({ error: 'not_found', task: null }); + }); +}); diff --git a/task-api/tests/tasks.routes.test.js b/task-api/tests/tasks.routes.test.js new file mode 100644 index 00000000..31ae096d --- /dev/null +++ b/task-api/tests/tasks.routes.test.js @@ -0,0 +1,316 @@ +/* + * File Role: + * This integration suite verifies HTTP behavior in src/routes/tasks.js and the middleware + * stack in src/app.js. It uses supertest with the in-memory Express app object, + * so requests execute full routing logic without binding a real TCP port. + */ + +const request = require('supertest'); +const app = require('../src/app'); +const taskService = require('../src/services/taskService'); + +/** + * @param {void} _unused - No direct inputs; each test arranges its own state. + * @returns {void} + * @behavior Groups route-level API contract checks from request to response payload. + */ +describe('tasks routes', () => { + /** + * @param {void} _unused - No direct inputs. + * @returns {void} + * @behavior Prevents state bleed across tests by resetting module-scoped task store. + */ + beforeEach(() => { + // Each HTTP test must start from a clean data store for deterministic outcomes. + taskService._reset(); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {Promise} + * @behavior Verifies baseline list endpoint returns all stored tasks. + */ + test('GET /tasks returns all tasks', async () => { + taskService.create({ title: 'Task A' }); + taskService.create({ title: 'Task B' }); + + const response = await request(app).get('/tasks'); + + expect(response.status).toBe(200); + expect(response.body).toHaveLength(2); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {Promise} + * @behavior Confirms query validation fails fast with 400 for malformed filters/pagination. + */ + test('GET /tasks validates query parameters', async () => { + const badStatus = await request(app).get('/tasks?status=invalid'); + const badPage = await request(app).get('/tasks?page=0'); + const badLimit = await request(app).get('/tasks?limit=999'); + + expect(badStatus.status).toBe(400); + expect(badPage.status).toBe(400); + expect(badLimit.status).toBe(400); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {Promise} + * @behavior Verifies data flow order: status filter -> assignee filter -> pagination. + */ + test('GET /tasks supports status + assignee filtering with pagination', async () => { + taskService.create({ title: 'Task 1', status: 'todo', assignee: 'Alice' }); + taskService.create({ title: 'Task 2', status: 'todo', assignee: 'Alice' }); + taskService.create({ title: 'Task 3', status: 'todo', assignee: 'Bob' }); + taskService.create({ title: 'Task 4', status: 'done', assignee: 'Alice' }); + + const response = await request(app).get('/tasks?status=todo&assignee=alice&page=1&limit=1'); + + expect(response.status).toBe(200); + expect(response.body).toHaveLength(1); + expect(response.body[0].title).toBe('Task 1'); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {Promise} + * @behavior Ensures partial pagination query input receives route defaults predictably. + */ + test('GET /tasks applies pagination defaults when only page or limit is provided', async () => { + taskService.create({ title: 'Task 1' }); + taskService.create({ title: 'Task 2' }); + + const withLimitOnly = await request(app).get('/tasks?limit=1'); + const withPageOnly = await request(app).get('/tasks?page=1'); + + expect(withLimitOnly.status).toBe(200); + expect(withLimitOnly.body).toHaveLength(1); + expect(withPageOnly.status).toBe(200); + expect(withPageOnly.body).toHaveLength(2); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {Promise} + * @behavior Covers single-resource fetch semantics (200 existing, 404 missing). + */ + test('GET /tasks/:id returns a single task or 404', async () => { + const created = taskService.create({ title: 'Find me' }); + + const found = await request(app).get(`/tasks/${created.id}`); + const missing = await request(app).get('/tasks/does-not-exist'); + + expect(found.status).toBe(200); + expect(found.body.id).toBe(created.id); + expect(missing.status).toBe(404); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {Promise} + * @behavior Verifies create endpoint maps valid payload to 201 and persisted defaults. + */ + test('POST /tasks creates a task', async () => { + const response = await request(app).post('/tasks').send({ + title: 'Create task', + priority: 'high', + }); + + expect(response.status).toBe(201); + expect(response.body.title).toBe('Create task'); + expect(response.body.priority).toBe('high'); + expect(response.body.status).toBe('todo'); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {Promise} + * @behavior Confirms input validation protects create endpoint from malformed payloads. + */ + test('POST /tasks rejects invalid payloads', async () => { + const noTitle = await request(app).post('/tasks').send({ priority: 'low' }); + const badDueDate = await request(app).post('/tasks').send({ + title: 'Bad date', + dueDate: 'not-a-date', + }); + + expect(noTitle.status).toBe(400); + expect(badDueDate.status).toBe(400); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {Promise} + * @behavior Verifies successful update path for mutable fields. + */ + test('PUT /tasks/:id updates a task', async () => { + const created = taskService.create({ title: 'Old title', status: 'todo' }); + + const response = await request(app).put(`/tasks/${created.id}`).send({ + title: 'New title', + status: 'in_progress', + }); + + expect(response.status).toBe(200); + expect(response.body.title).toBe('New title'); + expect(response.body.status).toBe('in_progress'); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {Promise} + * @behavior Ensures validation errors are returned before not-found checks where appropriate. + */ + test('PUT /tasks/:id validates payload and handles missing tasks', async () => { + const emptyBody = await request(app).put('/tasks/missing').send({}); + const missingTask = await request(app).put('/tasks/missing').send({ title: 'Will fail' }); + + expect(emptyBody.status).toBe(400); + expect(missingTask.status).toBe(404); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {Promise} + * @behavior Verifies delete contract and follow-up lookup behavior. + */ + test('DELETE /tasks/:id deletes a task', async () => { + const created = taskService.create({ title: 'Delete me' }); + + const response = await request(app).delete(`/tasks/${created.id}`); + const afterDelete = await request(app).get(`/tasks/${created.id}`); + + expect(response.status).toBe(204); + expect(afterDelete.status).toBe(404); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {Promise} + * @behavior Confirms delete of missing resource maps to 404. + */ + test('DELETE /tasks/:id returns 404 for unknown tasks', async () => { + const response = await request(app).delete('/tasks/missing'); + expect(response.status).toBe(404); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {Promise} + * @behavior Verifies dedicated completion endpoint and field-preservation semantics. + */ + test('PATCH /tasks/:id/complete marks a task complete', async () => { + const created = taskService.create({ title: 'Complete me', priority: 'high' }); + + const response = await request(app).patch(`/tasks/${created.id}/complete`); + + expect(response.status).toBe(200); + expect(response.body.status).toBe('done'); + expect(response.body.priority).toBe('high'); + expect(response.body.completedAt).toEqual(expect.any(String)); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {Promise} + * @behavior Confirms missing completion target returns 404, not 500. + */ + test('PATCH /tasks/:id/complete returns 404 for unknown tasks', async () => { + const response = await request(app).patch('/tasks/missing/complete'); + expect(response.status).toBe(404); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {Promise} + * @behavior Verifies assignment happy path and idempotent re-assignment behavior. + */ + test('PATCH /tasks/:id/assign assigns a user and supports idempotent repeat', async () => { + const created = taskService.create({ title: 'Assign me' }); + + const firstAssign = await request(app).patch(`/tasks/${created.id}/assign`).send({ assignee: 'Alice' }); + const secondAssign = await request(app).patch(`/tasks/${created.id}/assign`).send({ assignee: 'Alice' }); + + expect(firstAssign.status).toBe(200); + expect(firstAssign.body.assignee).toBe('Alice'); + expect(secondAssign.status).toBe(200); + expect(secondAssign.body.assignee).toBe('Alice'); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {Promise} + * @behavior Asserts HTTP mapping for assign errors: invalid->400, missing->404, conflict->409. + */ + test('PATCH /tasks/:id/assign validates payload, missing tasks, and conflicts', async () => { + const created = taskService.create({ title: 'Ownership test' }); + + await request(app).patch(`/tasks/${created.id}/assign`).send({ assignee: 'Alice' }); + + const emptyAssignee = await request(app) + .patch(`/tasks/${created.id}/assign`) + .send({ assignee: ' ' }); + const missingTask = await request(app).patch('/tasks/missing/assign').send({ assignee: 'Alice' }); + const conflict = await request(app).patch(`/tasks/${created.id}/assign`).send({ assignee: 'Bob' }); + + expect(emptyAssignee.status).toBe(400); + expect(missingTask.status).toBe(404); + expect(conflict.status).toBe(409); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {Promise} + * @behavior Ensures stats endpoint reflects service aggregation contract. + */ + test('GET /tasks/stats returns counts by status and overdue tasks', async () => { + const yesterday = new Date(Date.now() - 24 * 60 * 60 * 1000).toISOString(); + + taskService.create({ title: 'todo overdue', status: 'todo', dueDate: yesterday }); + taskService.create({ title: 'done overdue but excluded', status: 'done', dueDate: yesterday }); + + const response = await request(app).get('/tasks/stats'); + + expect(response.status).toBe(200); + expect(response.body.todo).toBe(1); + expect(response.body.done).toBe(1); + expect(response.body.overdue).toBe(1); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {Promise} + * @behavior Verifies app-level 404 middleware returns JSON consistently. + */ + test('unknown routes return JSON 404', async () => { + const response = await request(app).get('/not-a-route'); + + expect(response.status).toBe(404); + expect(response.body).toEqual({ error: 'Route not found' }); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {Promise} + * @behavior Verifies app-level error middleware catches unexpected service exceptions. + */ + test('unexpected route errors are handled by the error middleware', async () => { + // spyOn is used instead of jest.fn() so we can temporarily override and then restore + // real implementations on existing objects without permanently mutating shared modules. + const consoleSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + const serviceSpy = jest.spyOn(taskService, 'getAll').mockImplementation(() => { + throw new Error('simulated failure'); + }); + + const response = await request(app).get('/tasks'); + + expect(response.status).toBe(500); + expect(response.body).toEqual({ error: 'Internal server error' }); + + // mockRestore() returns originals so later tests keep production behavior intact. + serviceSpy.mockRestore(); + consoleSpy.mockRestore(); + }); +}); diff --git a/task-api/tests/validators.test.js b/task-api/tests/validators.test.js new file mode 100644 index 00000000..87ba8c1e --- /dev/null +++ b/task-api/tests/validators.test.js @@ -0,0 +1,209 @@ +/* + * File Role: + * This test suite validates the pure functions in src/utils/validators.js. + * The goal is to lock API input-contract behavior so route handlers can trust + * validator outcomes before delegating work to services. + */ + +const { + validateCreateTask, + validateUpdateTask, + validateAssignTask, + validateListQuery, + VALID_STATUSES, + VALID_PRIORITIES, +} = require('../src/utils/validators'); + +/** + * @param {void} _unused - No direct inputs; test data is defined per case. + * @returns {void} + * @behavior Groups all validator contract tests so any rule drift is immediately visible. + */ +describe('validators', () => { + /** + * @param {void} _unused - No direct inputs. + * @returns {void} + * @behavior Verifies exported enums stay aligned with business vocabulary used by routes and tests. + */ + test('exports supported statuses and priorities', () => { + expect(VALID_STATUSES).toEqual(['todo', 'in_progress', 'done']); + expect(VALID_PRIORITIES).toEqual(['low', 'medium', 'high']); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {void} + * @behavior Covers create-payload validation rules that protect POST /tasks. + */ + describe('validateCreateTask', () => { + /** + * @param {void} _unused - No direct inputs. + * @returns {void} + * @behavior Confirms valid create payloads are accepted and return null errors. + */ + test('accepts a valid payload', () => { + expect( + validateCreateTask({ + title: 'Ship feature', + description: 'Implement assign endpoint', + status: 'todo', + priority: 'high', + dueDate: '2030-01-01T00:00:00.000Z', + }) + ).toBeNull(); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {void} + * @behavior Ensures title remains a strict required field for creation. + */ + test('rejects missing or empty title', () => { + expect(validateCreateTask({})).toBe('title is required and must be a non-empty string'); + expect(validateCreateTask({ title: ' ' })).toBe('title is required and must be a non-empty string'); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {void} + * @behavior Guards against non-string description values that would break response consistency. + */ + test('rejects non-string description', () => { + expect(validateCreateTask({ title: 'A', description: 123 })).toBe('description must be a string'); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {void} + * @behavior Verifies enum enforcement for status and priority categories. + */ + test('rejects invalid status and priority', () => { + expect(validateCreateTask({ title: 'A', status: 'invalid' })).toBe( + 'status must be one of: todo, in_progress, done' + ); + expect(validateCreateTask({ title: 'A', priority: 'urgent' })).toBe( + 'priority must be one of: low, medium, high' + ); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {void} + * @behavior Documents optional-date behavior: null allowed, malformed values rejected. + */ + test('validates dueDate variants', () => { + expect(validateCreateTask({ title: 'A', dueDate: null })).toBeNull(); + expect(validateCreateTask({ title: 'A', dueDate: {} })).toBe('dueDate must be a valid ISO date string'); + expect(validateCreateTask({ title: 'A', dueDate: 'invalid-date' })).toBe( + 'dueDate must be a valid ISO date string' + ); + }); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {void} + * @behavior Covers update-payload rules used by PUT /tasks/:id. + */ + describe('validateUpdateTask', () => { + /** + * @param {void} _unused - No direct inputs. + * @returns {void} + * @behavior Prevents non-object or empty update bodies from becoming silent no-op writes. + */ + test('rejects invalid body shapes', () => { + expect(validateUpdateTask(null)).toBe('body must be an object'); + expect(validateUpdateTask([])).toBe('body must be an object'); + expect(validateUpdateTask({})).toBe('at least one field must be provided'); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {void} + * @behavior Ensures scalar field type checks remain strict for partial updates. + */ + test('rejects invalid scalar fields', () => { + expect(validateUpdateTask({ title: '' })).toBe('title must be a non-empty string'); + expect(validateUpdateTask({ description: 123 })).toBe('description must be a string'); + expect(validateUpdateTask({ status: 'x' })).toBe('status must be one of: todo, in_progress, done'); + expect(validateUpdateTask({ priority: 'x' })).toBe('priority must be one of: low, medium, high'); + expect(validateUpdateTask({ dueDate: 'bad-date' })).toBe('dueDate must be a valid ISO date string'); + expect(validateUpdateTask({ assignee: '' })).toBe('assignee must be a non-empty string when provided'); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {void} + * @behavior Captures explicit-clearing semantics: update allows null assignee and null dueDate. + */ + test('allows null assignee and valid updates', () => { + expect(validateUpdateTask({ assignee: null })).toBeNull(); + expect( + validateUpdateTask({ + title: 'Updated', + description: 'Better details', + status: 'in_progress', + priority: 'medium', + dueDate: null, + assignee: 'Alice', + }) + ).toBeNull(); + }); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {void} + * @behavior Covers assign-specific validation rules for PATCH /tasks/:id/assign. + */ + describe('validateAssignTask', () => { + /** + * @param {void} _unused - No direct inputs. + * @returns {void} + * @behavior Verifies assign endpoint remains strict and does not allow null/empty assignee intent. + */ + test('rejects invalid assignment payloads', () => { + expect(validateAssignTask(null)).toBe('body must be an object'); + expect(validateAssignTask({ assignee: ' ' })).toBe('assignee is required and must be a non-empty string'); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {void} + * @behavior Confirms happy-path assign payloads are accepted. + */ + test('accepts valid assignment payload', () => { + expect(validateAssignTask({ assignee: 'Alice' })).toBeNull(); + }); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {void} + * @behavior Covers list query validation used before GET /tasks filtering/pagination flow. + */ + describe('validateListQuery', () => { + /** + * @param {void} _unused - No direct inputs. + * @returns {void} + * @behavior Ensures route layer returns deterministic 400 responses for malformed queries. + */ + test('rejects invalid query values', () => { + expect(validateListQuery({ status: 'bad' })).toBe('status must be one of: todo, in_progress, done'); + expect(validateListQuery({ page: '0' })).toBe('page must be a positive integer'); + expect(validateListQuery({ page: '1.5' })).toBe('page must be a positive integer'); + expect(validateListQuery({ limit: '0' })).toBe('limit must be a positive integer'); + expect(validateListQuery({ limit: '101' })).toBe('limit must be less than or equal to 100'); + }); + + /** + * @param {void} _unused - No direct inputs. + * @returns {void} + * @behavior Confirms accepted query combinations stay backwards-compatible. + */ + test('accepts valid query values', () => { + expect(validateListQuery({})).toBeNull(); + expect(validateListQuery({ status: 'todo', page: '1', limit: '100' })).toBeNull(); + }); + }); +});