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.
This commit is contained in:
@@ -529,10 +529,11 @@ public sealed partial class TasksIslandViewModel : ViewModelBase, IDisposable
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Builds the hierarchy-aware flat ordering (top-level rows interleaved with visible children,
|
// 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
|
// orphans flagged so they render flat) and partitions it into Overdue/Open/Completed. Pure
|
||||||
// as Regroup renders it. Shared with callers that need "every completed row" independent of
|
// with respect to chain state (ShowAsChainMember/ChainStep/ChainAfterLabel) — callers that
|
||||||
// Rows/IsShowingCompleted (e.g. ClearCompletedAsync).
|
// only need section membership/counts (e.g. ClearCompletedAsync) should use this directly
|
||||||
private (List<TaskRowViewModel> Overdue, List<TaskRowViewModel> Open, List<TaskRowViewModel> Completed) ClassifyItems()
|
// rather than ClassifyItems, which additionally mutates every row via ApplyChainGrouping.
|
||||||
|
private (List<TaskRowViewModel> Overdue, List<TaskRowViewModel> Open, List<TaskRowViewModel> Completed) PartitionItems()
|
||||||
{
|
{
|
||||||
// Items is already ordered by SortOrder from the DB query.
|
// Items is already ordered by SortOrder from the DB query.
|
||||||
// Treat rows whose ParentTaskId is not in the current view as orphans -> top-level.
|
// 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);
|
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<TaskRowViewModel> Overdue, List<TaskRowViewModel> Open, List<TaskRowViewModel> Completed) ClassifyItems()
|
||||||
|
{
|
||||||
|
var (overdue, open, completed) = PartitionItems();
|
||||||
|
|
||||||
// Dependency chains are resolved and pulled together per section (not on the
|
// 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
|
// 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
|
// 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]
|
[RelayCommand]
|
||||||
private async Task ClearCompletedAsync()
|
private async Task ClearCompletedAsync()
|
||||||
{
|
{
|
||||||
var (_, _, completed) = ClassifyItems();
|
var (_, _, completed) = PartitionItems();
|
||||||
if (completed.Count == 0) return;
|
if (completed.Count == 0) return;
|
||||||
|
|
||||||
// Delete children before parents so the parent-child FK (Restrict) doesn't
|
// Delete children before parents so the parent-child FK (Restrict) doesn't
|
||||||
|
|||||||
@@ -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<ClaudeDoDbContext>()
|
||||||
|
.UseSqlite($"Data Source={_dbPath}")
|
||||||
|
.Options;
|
||||||
|
return new ClaudeDoDbContext(opts);
|
||||||
|
}
|
||||||
|
|
||||||
|
private sealed class TestDbFactory : IDbContextFactory<ClaudeDoDbContext>
|
||||||
|
{
|
||||||
|
private readonly Func<ClaudeDoDbContext> _create;
|
||||||
|
public TestDbFactory(Func<ClaudeDoDbContext> 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");
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user