Requirements PATCH: explicit null clears parent_id (issue #30) - #82
Merged
Conversation
87cf6ad made five nullable fields clearable but left parent_id on the None-skip path, so `{"parent_id": null}` returned 200 and silently kept the parent — the exact symptom #30 was filed for, in the one nullable field the fix missed. A child requirement could never be promoted back to a level-0 root; the API reported success while ignoring the edit. parent_id joins the clearable set. level is deliberately not adjusted: the two are already independent PATCH fields, and re-levelling a subtree on a parent clear would be a surprise the caller didn't ask for. The existing validation guard still only runs for a non-null parent_id, which is what clearing wants — there is no parent to look up. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014jPbhRuDUZLie159gQxTDZ
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
1 participant
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.
Closes the last gap in #30.
87cf6ad(merged in #56) maderationale,category,verification_method,tbr_owner_idandtbr_dueclearable viamodel_fields_set, but leftparent_idon theNone-skip path — even though it is nullable by design ("level-0 roots have no parent"). So the original symptom survived in that one field:Reproduced against a running instance before fixing: a child requirement can never be promoted back to a level-0 root, and the API reports success while ignoring the edit.
The change
parent_idjoins theclearableset. Two deliberate non-changes:levelis not adjusted.levelandparent_idare already independent PATCH fields (you can change either alone today), and re-levelling on a parent clear would be a surprise the caller didn't ask for. The caller setslevelin the same PATCH if it should change.if data.parent_id is not None. Clearing has no parent to look up, so skipping the existence/self-parent checks is correct.Verification
New test asserts the clear works, survives reload, and that omitting the field leaves it alone. Confirmed non-vacuous — it fails with
assert 1 is Nonewhen the one-line change is reverted. Full suite 1054 passed / 1 skipped;ruff check src/andruff format --check src/clean.Note the two
F841s and one format diffruffreports undertests/are pre-existing and untouched here; CI only coverssrc/.🤖 Generated with Claude Code
https://claude.ai/code/session_014jPbhRuDUZLie159gQxTDZ