Repository navigation
fix(tasks): journal toggle and rename keep task details; completion is Status-only - #310
Merged
Merged
Conversation
- drop isComplete from task request/response examples, field lists and validation rules
- add a PATCH /api/v1/TaskItems/{id} section with merge-patch semantics
nevridge
commented
Oct 22, 2026
nevridge
left a comment
Owner
Author
There was a problem hiding this comment.
Review: 💬 Comments
PR moves the journal toggle and rename onto a merge-patch endpoint and makes Status the only completion input, with a verified migration. One stale-docs finding was fixed in this review; nothing else is open.
| # | Severity | Location | Finding | Status |
|---|---|---|---|---|
| 1 | 🔵 Docs | docs/API.md:172 |
Still documented isComplete as a required field and omitted PATCH |
✅ Fixed in 299a99e |
Fixes pushed
Commit 299a99e addresses: [1]
Reviewed at 299a99e · findings from pr-review + code-review, verified and triaged by @nevridge with Claude Code
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
Checking off or renaming a todo in the journal no longer clears the task's description, due date or parent link. The journal toggle and rename now use a new
PATCH /api/v1/TaskItems/{id}, which leaves absent fields unchanged.While fixing this, completion became Status-only.
IsCompletewas a leftover column from beforeStatusexisted, and every reader had to OR the two. It is now removed from requests, responses, the frontend and the database.Closes #308
What changed
nullclears description, due date or parent link.PatchTaskItemDto, so no custom converter is needed and the OpenAPI schema is plain nullable properties.isCompleteis removed from the create, update and PATCH requests and from all responses.statuskeeps the stored status.RemoveTaskItemIsComplete:Completedhistory event for each row whereIsCompleteis true and Status is not Completed. The event summary says a data migration recorded it, and it is timestamped at the migration run time.Downre-adds the column and sets it from Status. It leaves the synthetic events in place.{status, autoCompleteParentWhenChildrenDone, timezoneOffsetMinutes}, and the rename sends{title}.commitEditno longer sendsparentTaskItemId, which previously cleared the parent link of a subtask that wasn't in the loaded list.isCompleteread and send is removed.patchApiV1TaskItemsByIdandPatchTaskItemDtoadded andisCompleteremoved.TaskFlow.Api.httpand the smoke-test POST inci.yml.Release notes
isCompleteand responses no longer contain it. Until it is updated,update_taskwithcomplete=trueand nostatusmay silently stop completing tasks, and its list and weekly-review output may mis-report completion.isCompletegets a success response and no change.Completedentry dated at the migration run time, not when they were actually completed.ProductVersiongoes from 10.0.8 to 10.0.11 because of the regenerated EF tooling. It has no runtime effect.Testing
TasksPage.tsx:287.isCompletein a POST, PUT or PATCH body goes through the app's own MVC JSON options and the real controller and does not return a 400.AddProjects, seeds rows with raw SQL, migrates forward and checks the statuses, the synthetic events, the foreign keys, indexes and dependent rows. It then checksDownand a secondUp.isComplete.Validation
The implementation validator reports Critical 0 and Important 0, with all 26 acceptance criteria met. Four Minor findings were raised:
isCompletebehavior but only check that Status flows through the controller. They are renamed toUpdate_ShouldReturnRequestedStatusandCreate_ShouldPersistRequestedStatus, and the misleading comment now points toTaskItemsControllerLegacyIsCompleteTests, which has the real end-to-end proof.isCompletetests deserialize with the app's own MVC options but don't go through the MVC input formatter. That would need an integration-test harness, which is out of scope. They also assertUnmappedMemberHandling == Skip, so tightening the options later fails a test.Up, not afterDown.Downonly adds a column and runs one UPDATE, and the table rebuild where the risk was happens inUp, which is tested.ProductVersionbump (see the release notes).Follow-ups
IsCompleteconcept from the MCP is tracked in nevridge/taskflow-assistant#12.