Add test suite, bug fixes, and PATCH /tasks/:id/assign endpoint - #85
Open
Satyamdixit2802 wants to merge 1 commit into
Open
Add test suite, bug fixes, and PATCH /tasks/:id/assign endpoint#85Satyamdixit2802 wants to merge 1 commit into
Satyamdixit2802 wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds test coverage for the existing Task Manager API, documents bugs found during testing,
fixes one of them, and implements the new
PATCH /tasks/:id/assignendpoint.What's included
1. Tests (
tests/)taskService.jsfunctionsPOST /tasks, PUT /tasks/:id, DELETE /tasks/:id, PATCH /tasks/:id/complete, GET /tasks/stats,
PATCH /tasks/:id/assign
2. Bug report (
BUGS.md)Found [N] bugs while writing tests. Full details with location, expected vs actual behavior,
and root cause are in
BUGS.md. Summary:getByStatususes substring matching (.includes()) instead of exact equality, so filteringby status can match unintended tasks
getPaginatedhas an off-by-one:offset = page * limitskips the first page's worth ofresults instead of starting at 0
completeTaskunconditionally resetsprioritytomedium, silently discarding the task'soriginal priority
3. Fix
Fixed the pagination off-by-one in
getPaginated(offset = (page - 1) * limit). Chose thisone because it's the most clear-cut and highest-impact —
page=1was previously returning thesecond page of results, which would be a visible, confusing bug for any real client. Updated
tests to lock in the corrected behavior. Other bugs are documented but left unfixed per the
assignment scope.
4. New feature:
PATCH /tasks/:id/assign{ "assignee": "string" }, stores it on the task, returns the updated taskassigneeis missing, non-string, or empty/whitespace-onlyupdate()and
completeTask()already behave in this codebase, with no existing precedent for blockingoverwrites
DESIGN_NOTES.mdTest results
Coverage
Notes on the process
in-memory store, malformed JSON bodies, very large pagination limits]
swapped for persistent storage before this goes live? should
assigneevalidate against areal user list instead of accepting any string?]