Skip to content

fix(tasks): journal toggle and rename keep task details; completion is Status-only - #310

Merged
nevridge merged 6 commits into
mainfrom
bugfix/journal-put-clears-fields
Oct 22, 2026
Merged

nevridge merged 6 commits into
mainfrom
bugfix/journal-put-clears-fields

Conversation

@nevridge

@nevridge nevridge commented Oct 22, 2026 •

Copy link
Copy Markdown
Owner

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. IsComplete was a leftover column from before Status existed, and every reader had to OR the two. It is now removed from requests, responses, the frontend and the database.

Closes #308

What changed

  • PATCH endpoint (new):
    • Absent fields stay unchanged, and an explicit null clears description, due date or parent link.
    • Presence is tracked with setters on PatchTaskItemDto, so no custom converter is needed and the OpenAPI schema is plain nullable properties.
    • PUT and PATCH share one update path, so PATCH gets the same parent, parent-complete and past-day reopen guards, validation, history events and auto-complete-parent. PUT behaves as before, except that it no longer reads a completion flag.
  • Status is the only completion state:
    • isComplete is removed from the create, update and PATCH requests and from all responses.
    • A request that still sends it is ignored with no 400, and a PUT or PATCH that omits status keeps the stored status.
    • All backend read sites (task update, history, project counts, journal carry-forward) check Status only.
  • Migration RemoveTaskItemIsComplete:
    1. Inserts one Completed history event for each row where IsComplete is true and Status is not Completed. The event summary says a data migration recorded it, and it is timestamped at the migration run time.
    2. Sets those rows to Status Completed, matching what the OR logic showed before.
    3. Drops the column.
    • Down re-adds the column and sets it from Status. It leaves the synthetic events in place.
  • Frontend:
    • The journal toggle sends {status, autoCompleteParentWhenChildrenDone, timezoneOffsetMinutes}, and the rename sends {title}.
    • commitEdit no longer sends parentTaskItemId, which previously cleared the parent link of a subtask that wasn't in the loaded list.
    • Every isComplete read and send is removed.
  • Generated client: regenerated by the backend-builder, with patchApiV1TaskItemsById and PatchTaskItemDto added and isComplete removed.
  • Cleanup: the stale body in TaskFlow.Api.http and the smoke-test POST in ci.yml.

Release notes

  • Back up the database before deploying. The migration changes rows, adds history events and drops a column. It is not reversible as such, because the original two values are merged. Migrations run on startup in Docker.
  • Update the taskflow-assistant MCP (nevridge/taskflow-assistant#12) before or alongside this deploy. Requests no longer accept isComplete and responses no longer contain it. Until it is updated, update_task with complete=true and no status may silently stop completing tasks, and its list and weekly-review output may mis-report completion.
  • A client that sends only isComplete gets a success response and no change.
  • Tasks that were completed on the Tasks page and then unticked in the journal (flag set, status Todo) become Completed. Their history shows a Completed entry dated at the migration run time, not when they were actually completed.
  • Descriptions and due dates already cleared by this bug can't be recovered.
  • The snapshot's ProductVersion goes from 10.0.8 to 10.0.11 because of the regenerated EF tooling. It has no runtime effect.

Testing

  • Backend: 482 passed, 94.5% line coverage (threshold 75%).
  • Frontend: 683 passed, 91.93% line coverage (threshold 80%).
  • Build, format, type-check and lint: clean. Lint has one existing warning at TasksPage.tsx:287.
  • New backend tests:
    • PATCH: absent versus null, guards, 400/404/422, history events, auto-complete-parent and carry-forward.
    • A stale isComplete in 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.
    • A migration test on real SQLite: it migrates to AddProjects, seeds rows with raw SQL, migrates forward and checks the statuses, the synthetic events, the foreign keys, indexes and dependent rows. It then checks Down and a second Up.
  • New frontend tests:
    • The exact PATCH body keys for the journal toggle and the rename.
    • A subtask rename and a completed-task rename.
    • The create and update bodies without isComplete.
    • The history panel rendering the migration summary.
  • A manual testing checklist is on bug: journal todo toggle and title edit may clear task description and due date #308: bug: journal todo toggle and title edit may clear task description and due date #308 (comment)

Validation

The implementation validator reports Critical 0 and Important 0, with all 26 acceptance criteria met. Four Minor findings were raised:

  • Fixed: two backend tests were named as if they proved the stale-isComplete behavior but only check that Status flows through the controller. They are renamed to Update_ShouldReturnRequestedStatus and Create_ShouldPersistRequestedStatus, and the misleading comment now points to TaskItemsControllerLegacyIsCompleteTests, which has the real end-to-end proof.
  • Accepted: the legacy-isComplete tests 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 assert UnmappedMemberHandling == Skip, so tightening the options later fails a test.
  • Accepted: the migration test checks Notes and JournalLogEntries only after Up, not after Down. Down only adds a column and runs one UPDATE, and the table rebuild where the risk was happens in Up, which is tested.
  • Accepted: the snapshot ProductVersion bump (see the release notes).

Follow-ups

@nevridge nevridge left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@nevridge
nevridge merged commit 5237984 into main Oct 22, 2026
15 checks passed
@nevridge
nevridge deleted the bugfix/journal-put-clears-fields branch October 22, 2026 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: journal todo toggle and title edit may clear task description and due date

1 participant