fix(cr-2026-04-22): uncomplete_task reopens auto-completed parents
MAJOR from the 2026-04-22 review (database.rs:389-449): complete_subtask_and_check_parent auto-completes a parent task when the last child completes, but uncomplete_task only flipped the requested row — reopening a child left the parent wrongly marked done, breaking the "parent done iff every child done" invariant. Wraps uncomplete_task in a transaction and, after flipping the subtask, looks up its parent_task_id. If present, resets the parent to done=0 as well. Scoped to "done=1" on the parent update so an already-open parent is untouched. Two regression tests: - uncomplete_subtask_reopens_auto_completed_parent: the direct mirror of the existing subtask_crud_roundtrip completion flow. - uncomplete_top_level_task_does_not_touch_siblings: ensures the parent-reopen branch is a no-op for tasks with no parent, and siblings without a parent relationship are unaffected.
This commit is contained in:
@@ -441,11 +441,42 @@ pub async fn complete_task(pool: &SqlitePool, id: &str) -> Result<()> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
pub async fn uncomplete_task(pool: &SqlitePool, id: &str) -> Result<()> {
|
pub async fn uncomplete_task(pool: &SqlitePool, id: &str) -> Result<()> {
|
||||||
|
let mut tx = pool
|
||||||
|
.begin()
|
||||||
|
.await
|
||||||
|
.map_err(|e| KonError::StorageError(format!("Begin transaction failed: {e}")))?;
|
||||||
|
|
||||||
sqlx::query("UPDATE tasks SET done = 0, done_at = NULL WHERE id = ?")
|
sqlx::query("UPDATE tasks SET done = 0, done_at = NULL WHERE id = ?")
|
||||||
.bind(id)
|
.bind(id)
|
||||||
.execute(pool)
|
.execute(&mut *tx)
|
||||||
.await
|
.await
|
||||||
.map_err(|e| KonError::StorageError(format!("Uncomplete task failed: {e}")))?;
|
.map_err(|e| KonError::StorageError(format!("Uncomplete task failed: {e}")))?;
|
||||||
|
|
||||||
|
// Mirror the auto-complete invariant from
|
||||||
|
// `complete_subtask_and_check_parent`: a parent task is done iff
|
||||||
|
// every child is done. If the child we just reopened had a done
|
||||||
|
// parent, reopen the parent too so state stays consistent
|
||||||
|
// (2026-04-22 review MAJOR). No-op for top-level tasks.
|
||||||
|
let parent_id: Option<String> =
|
||||||
|
sqlx::query_scalar("SELECT parent_task_id FROM tasks WHERE id = ?")
|
||||||
|
.bind(id)
|
||||||
|
.fetch_optional(&mut *tx)
|
||||||
|
.await
|
||||||
|
.map_err(|e| KonError::StorageError(format!("Get parent_task_id failed: {e}")))?
|
||||||
|
.flatten();
|
||||||
|
|
||||||
|
if let Some(pid) = parent_id {
|
||||||
|
sqlx::query("UPDATE tasks SET done = 0, done_at = NULL WHERE id = ? AND done = 1")
|
||||||
|
.bind(&pid)
|
||||||
|
.execute(&mut *tx)
|
||||||
|
.await
|
||||||
|
.map_err(|e| KonError::StorageError(format!("Reopen parent failed: {e}")))?;
|
||||||
|
}
|
||||||
|
|
||||||
|
tx.commit()
|
||||||
|
.await
|
||||||
|
.map_err(|e| KonError::StorageError(format!("Commit transaction failed: {e}")))?;
|
||||||
|
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1005,6 +1036,57 @@ mod tests {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn uncomplete_subtask_reopens_auto_completed_parent() {
|
||||||
|
// Regression for the 2026-04-22 review MAJOR on
|
||||||
|
// asymmetric complete / uncomplete semantics: once all
|
||||||
|
// subtasks completed auto-completes the parent, reopening a
|
||||||
|
// child must also reopen the parent so "parent is done iff
|
||||||
|
// every child is done" holds in both directions.
|
||||||
|
let pool = test_pool().await;
|
||||||
|
insert_task(&pool, "p1", "Ship release", "inbox", None, None, None)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
insert_subtask(&pool, "s1", "Final test", "p1").await.unwrap();
|
||||||
|
insert_subtask(&pool, "s2", "Tag release", "p1").await.unwrap();
|
||||||
|
|
||||||
|
complete_subtask_and_check_parent(&pool, "s1").await.unwrap();
|
||||||
|
complete_subtask_and_check_parent(&pool, "s2").await.unwrap();
|
||||||
|
let parent = get_task_by_id(&pool, "p1").await.unwrap().unwrap();
|
||||||
|
assert!(parent.done);
|
||||||
|
|
||||||
|
uncomplete_task(&pool, "s2").await.unwrap();
|
||||||
|
let parent = get_task_by_id(&pool, "p1").await.unwrap().unwrap();
|
||||||
|
assert!(
|
||||||
|
!parent.done,
|
||||||
|
"reopening any child must reopen the auto-completed parent"
|
||||||
|
);
|
||||||
|
let s2 = get_task_by_id(&pool, "s2").await.unwrap().unwrap();
|
||||||
|
assert!(!s2.done, "subtask itself must also be reopened");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn uncomplete_top_level_task_does_not_touch_siblings() {
|
||||||
|
// Sanity: tasks without a parent must not trigger the parent-
|
||||||
|
// reopen branch (it should no-op cleanly).
|
||||||
|
let pool = test_pool().await;
|
||||||
|
insert_task(&pool, "a", "A", "inbox", None, None, None)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
insert_task(&pool, "b", "B", "inbox", None, None, None)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
complete_task(&pool, "a").await.unwrap();
|
||||||
|
complete_task(&pool, "b").await.unwrap();
|
||||||
|
|
||||||
|
uncomplete_task(&pool, "a").await.unwrap();
|
||||||
|
|
||||||
|
let a = get_task_by_id(&pool, "a").await.unwrap().unwrap();
|
||||||
|
let b = get_task_by_id(&pool, "b").await.unwrap().unwrap();
|
||||||
|
assert!(!a.done);
|
||||||
|
assert!(b.done, "sibling must be untouched");
|
||||||
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn update_transcript_meta_happy_path() {
|
async fn update_transcript_meta_happy_path() {
|
||||||
// Task 2.5 — insert a transcript, update starred=true, read it back.
|
// Task 2.5 — insert a transcript, update starred=true, read it back.
|
||||||
|
|||||||
Reference in New Issue
Block a user