From e55c84cda4b14a6852d95350023489d011b16828 Mon Sep 17 00:00:00 2001 From: mika kuns Date: Tue, 11 Aug 2026 18:32:59 +0200 Subject: [PATCH] fix(ui): stop ClearCompletedAsync from mutating chain-display fields ClassifyItems() layers ApplyChainGrouping on top of partitioning, which writes ShowAsChainMember/ChainStep/ChainAfterLabel on every row. ClearCompletedAsync only needed the completed-section count/rows, so it got that mutation as an unwanted side effect of counting. Split the pure overdue/open/completed partition into PartitionItems(); ClassifyItems() now composes it with the chain-grouping mutation for Regroup's actual render path. ClearCompletedAsync uses PartitionItems directly. Regroup's behavior is unchanged. --- .../Islands/TasksIslandViewModel.cs | 22 ++- .../TasksIslandClassifyItemsTests.cs | 156 ++++++++++++++++++ 2 files changed, 173 insertions(+), 5 deletions(-) create mode 100644 tests/ClaudeDo.Ui.Tests/ViewModels/TasksIslandClassifyItemsTests.cs diff --git a/src/ClaudeDo.Ui/ViewModels/Islands/TasksIslandViewModel.cs b/src/ClaudeDo.Ui/ViewModels/Islands/TasksIslandViewModel.cs index bd590100..da2466ba 100644 --- a/src/ClaudeDo.Ui/ViewModels/Islands/TasksIslandViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/Islands/TasksIslandViewModel.cs @@ -529,10 +529,11 @@ public sealed partial class TasksIslandViewModel : ViewModelBase, IDisposable } // Builds the hierarchy-aware flat ordering (top-level rows interleaved with visible children, - // orphans flagged so they render flat) and classifies it into Overdue/Open/Completed, exactly - // as Regroup renders it. Shared with callers that need "every completed row" independent of - // Rows/IsShowingCompleted (e.g. ClearCompletedAsync). - private (List Overdue, List Open, List Completed) ClassifyItems() + // orphans flagged so they render flat) and partitions it into Overdue/Open/Completed. Pure + // with respect to chain state (ShowAsChainMember/ChainStep/ChainAfterLabel) — callers that + // only need section membership/counts (e.g. ClearCompletedAsync) should use this directly + // rather than ClassifyItems, which additionally mutates every row via ApplyChainGrouping. + private (List Overdue, List Open, List Completed) PartitionItems() { // Items is already ordered by SortOrder from the DB query. // Treat rows whose ParentTaskId is not in the current view as orphans -> top-level. @@ -580,6 +581,17 @@ public sealed partial class TasksIslandViewModel : ViewModelBase, IDisposable open.Add(r); } + return (overdue, open, completed); + } + + // ClassifyItems layers the chain-grouping mutation on top of PartitionItems, exactly as + // Regroup renders it — every row in the result has ShowAsChainMember/ChainStep/ChainAfterLabel + // written. Only call this from a path that actually renders the result; a caller that just + // needs section membership (e.g. a count) should call PartitionItems instead. + private (List Overdue, List Open, List Completed) ClassifyItems() + { + var (overdue, open, completed) = PartitionItems(); + // Dependency chains are resolved and pulled together per section (not on the // pre-split `flat` list): a chain head might land in a different section than its // dependent (e.g. a Done head in Completed, an open dependent in Open) — in that case @@ -965,7 +977,7 @@ public sealed partial class TasksIslandViewModel : ViewModelBase, IDisposable [RelayCommand] private async Task ClearCompletedAsync() { - var (_, _, completed) = ClassifyItems(); + var (_, _, completed) = PartitionItems(); if (completed.Count == 0) return; // Delete children before parents so the parent-child FK (Restrict) doesn't diff --git a/tests/ClaudeDo.Ui.Tests/ViewModels/TasksIslandClassifyItemsTests.cs b/tests/ClaudeDo.Ui.Tests/ViewModels/TasksIslandClassifyItemsTests.cs new file mode 100644 index 00000000..3e87a3e3 --- /dev/null +++ b/tests/ClaudeDo.Ui.Tests/ViewModels/TasksIslandClassifyItemsTests.cs @@ -0,0 +1,156 @@ +using ClaudeDo.Data; +using ClaudeDo.Data.Models; +using ClaudeDo.Ui.ViewModels.Islands; +using Microsoft.EntityFrameworkCore; +using TaskStatus = ClaudeDo.Data.Models.TaskStatus; + +namespace ClaudeDo.Ui.Tests.ViewModels; + +// ClassifyItems (used by Regroup) additionally mutates every row's chain-display fields via +// ApplyChainGrouping. ClearCompletedAsync only needs section membership/counts, so it goes +// through the pure PartitionItems instead — this must not write ShowAsChainMember/ChainStep/ +// ChainAfterLabel as a side effect of counting completed rows. +public class TasksIslandClassifyItemsTests : IDisposable +{ + private readonly string _dbPath; + + public TasksIslandClassifyItemsTests() + { + _dbPath = Path.Combine(Path.GetTempPath(), $"claudedo_ui_classify_{Guid.NewGuid():N}.db"); + using var ctx = NewContext(); + ctx.Database.EnsureCreated(); + } + + public void Dispose() + { + try { File.Delete(_dbPath); } catch { } + try { File.Delete(_dbPath + "-wal"); } catch { } + try { File.Delete(_dbPath + "-shm"); } catch { } + } + + private ClaudeDoDbContext NewContext() + { + var opts = new DbContextOptionsBuilder() + .UseSqlite($"Data Source={_dbPath}") + .Options; + return new ClaudeDoDbContext(opts); + } + + private sealed class TestDbFactory : IDbContextFactory + { + private readonly Func _create; + public TestDbFactory(Func create) => _create = create; + public ClaudeDoDbContext CreateDbContext() => _create(); + } + + private TasksIslandViewModel BuildViewModel() + { + var factory = new TestDbFactory(NewContext); + return new TasksIslandViewModel(factory, worker: null); + } + + private static ListNavItemViewModel UserList(string listEntityId, string name) => + new() { Id = $"user:{listEntityId}", Kind = ListKind.User, Name = name }; + + private static async Task LoadAndWaitAsync(TasksIslandViewModel vm, ListNavItemViewModel list, int expectedCount = 1) + { + vm.LoadForList(list); + var deadline = DateTime.UtcNow.AddSeconds(5); + while (DateTime.UtcNow < deadline) + { + await Task.Delay(25); + if (vm.Items.Count >= expectedCount) break; + } + await Task.Delay(50); + } + + private async Task SeedAsync( + params (string Id, TaskStatus Status, string? DependsOnTaskId, int SortOrder)[] tasks) + { + await using var db = NewContext(); + db.Lists.Add(new ListEntity { Id = "list1", Name = "Default", CreatedAt = DateTime.UtcNow }); + foreach (var t in tasks) + { + db.Tasks.Add(new TaskEntity + { + Number = TestTaskNumbers.Next(), + Id = t.Id, + ListId = "list1", + Title = t.Id, + CreatedAt = DateTime.UtcNow, + Status = t.Status, + DependsOnTaskId = t.DependsOnTaskId, + SortOrder = t.SortOrder, + }); + } + await db.SaveChangesAsync(); + } + + [Fact] + public async Task ClearCompleted_counting_path_leaves_chain_fields_untouched() + { + await SeedAsync( + ("head", TaskStatus.Idle, null, 0), + ("dep", TaskStatus.Idle, "head", 1)); + + var vm = BuildViewModel(); + await LoadAndWaitAsync(vm, UserList("list1", "Default"), expectedCount: 2); + + var dep = vm.Items.Single(r => r.Id == "dep"); + // Sanity: the initial Regroup (during load) computed the chain correctly. + Assert.True(dep.ShowAsChainMember); + Assert.Equal(1, dep.ChainStep); + + // Corrupt the row to a sentinel distinguishable from both the correct and default value. + dep.ShowAsChainMember = false; + dep.ChainStep = 99; + dep.ChainAfterLabel = "SENTINEL"; + + // Nothing is completed, so ClearCompletedAsync does nothing but count via PartitionItems. + await vm.ClearCompletedCommand.ExecuteAsync(null); + + Assert.False(dep.ShowAsChainMember); + Assert.Equal(99, dep.ChainStep); + Assert.Equal("SENTINEL", dep.ChainAfterLabel); + } + + [Fact] + public async Task Regroup_still_recomputes_chain_fields() + { + await SeedAsync( + ("head", TaskStatus.Idle, null, 0), + ("dep", TaskStatus.Idle, "head", 1)); + + var vm = BuildViewModel(); + await LoadAndWaitAsync(vm, UserList("list1", "Default"), expectedCount: 2); + + var dep = vm.Items.Single(r => r.Id == "dep"); + dep.ShowAsChainMember = false; + dep.ChainStep = 99; + dep.ChainAfterLabel = "SENTINEL"; + + vm.Regroup(); + + Assert.True(dep.ShowAsChainMember); + Assert.Equal(1, dep.ChainStep); + Assert.Null(dep.ChainAfterLabel); + } + + [Fact] + public async Task ClearCompleted_still_deletes_completed_rows_and_reports_count() + { + await SeedAsync( + ("open1", TaskStatus.Idle, null, 0), + ("done1", TaskStatus.Done, null, 1), + ("done2", TaskStatus.Done, null, 2)); + + var vm = BuildViewModel(); + await LoadAndWaitAsync(vm, UserList("list1", "Default"), expectedCount: 3); + vm.IsShowingCompleted = true; + + await vm.ClearCompletedCommand.ExecuteAsync(null); + + Assert.DoesNotContain(vm.Items, r => r.Id is "done1" or "done2"); + Assert.Contains(vm.Items, r => r.Id == "open1"); + } +}