The task-list done toggle (both islands) and RemoveFromQueue wrote TaskEntity.Status directly via EF, bypassing TaskStateService: no TaskUpdated broadcast, no guard against a concurrent picker claim (lost update), and no status-based filter. Added guarded MarkDoneAsync/UnmarkDoneAsync/DequeueToIdleAsync transitions plus matching hub methods (SetTaskDone/UnsetTaskDone/DequeueTask) and IWorkerClient wrappers; the three UI call sites now route through the hub with optimistic-then-revert row updates and ErrorReported on failure. RemoveFromQueueAsync dequeues each queued child individually through the same guarded path instead of cascading via a raw EF update. Also closes two hub guard gaps: UpdateListConfig's delete branch now preserves a list's SerializeOnFileOverlap flag instead of dropping it, and SubmitTaskForReview's Idle/Failed status gate now runs before either mutation branch so a Done/Cancelled task can't get committed or stamped and then rejected.
282 lines
11 KiB
C#
282 lines
11 KiB
C#
using ClaudeDo.Data;
|
|
using ClaudeDo.Data.Models;
|
|
using ClaudeDo.Localization;
|
|
using ClaudeDo.Ui.Localization;
|
|
using ClaudeDo.Ui.Services;
|
|
using ClaudeDo.Ui.ViewModels.Islands;
|
|
using Microsoft.EntityFrameworkCore;
|
|
using TaskStatus = ClaudeDo.Data.Models.TaskStatus;
|
|
|
|
namespace ClaudeDo.Ui.Tests.ViewModels;
|
|
|
|
// UX-Audit #1: SetStatusOnRow/CancelRunningTask/RejectReviewToQueue/RejectReviewToIdle used
|
|
// to swallow worker failures with a bare `catch { }` — nothing surfaced in the footer strip.
|
|
// These now raise ErrorReported.
|
|
public class TasksIslandErrorFeedbackTests : IDisposable
|
|
{
|
|
private readonly string _dbPath;
|
|
|
|
public TasksIslandErrorFeedbackTests()
|
|
{
|
|
_dbPath = Path.Combine(Path.GetTempPath(), $"claudedo_tasksisland_error_feedback_test_{Guid.NewGuid():N}.db");
|
|
using var ctx = NewContext();
|
|
ctx.Database.EnsureCreated();
|
|
|
|
var dir = AppContext.BaseDirectory;
|
|
while (dir is not null && !Directory.Exists(Path.Combine(dir, "src", "ClaudeDo.Localization", "locales")))
|
|
dir = Path.GetDirectoryName(dir);
|
|
Loc.Current = new Localizer(
|
|
LocaleStore.Load(Path.Combine(dir!, "src", "ClaudeDo.Localization", "locales")), "en");
|
|
}
|
|
|
|
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 sealed class ThrowingWorkerClient : StubWorkerClient
|
|
{
|
|
public Exception? ThrowOnSetTaskStatus;
|
|
public Exception? ThrowOnCancelTask;
|
|
public Exception? ThrowOnRejectToQueue;
|
|
public Exception? ThrowOnRejectToIdle;
|
|
public Exception? ThrowOnSetTaskDone;
|
|
public Exception? ThrowOnUnsetTaskDone;
|
|
public Exception? ThrowOnDequeueTask;
|
|
public readonly List<string> DequeuedTaskIds = new();
|
|
|
|
public override Task<BaseDirtyWarningDto?> SetTaskStatusAsync(string taskId, TaskStatus status)
|
|
{
|
|
if (ThrowOnSetTaskStatus is not null) throw ThrowOnSetTaskStatus;
|
|
return Task.FromResult<BaseDirtyWarningDto?>(null);
|
|
}
|
|
|
|
public override Task CancelTaskAsync(string taskId)
|
|
{
|
|
if (ThrowOnCancelTask is not null) throw ThrowOnCancelTask;
|
|
return Task.CompletedTask;
|
|
}
|
|
|
|
public override Task RejectReviewToQueueAsync(string taskId, string feedback)
|
|
{
|
|
if (ThrowOnRejectToQueue is not null) throw ThrowOnRejectToQueue;
|
|
return Task.CompletedTask;
|
|
}
|
|
|
|
public override Task RejectReviewToIdleAsync(string taskId)
|
|
{
|
|
if (ThrowOnRejectToIdle is not null) throw ThrowOnRejectToIdle;
|
|
return Task.CompletedTask;
|
|
}
|
|
|
|
public override Task SetTaskDoneAsync(string taskId)
|
|
{
|
|
if (ThrowOnSetTaskDone is not null) throw ThrowOnSetTaskDone;
|
|
return Task.CompletedTask;
|
|
}
|
|
|
|
public override Task UnsetTaskDoneAsync(string taskId)
|
|
{
|
|
if (ThrowOnUnsetTaskDone is not null) throw ThrowOnUnsetTaskDone;
|
|
return Task.CompletedTask;
|
|
}
|
|
|
|
public override Task DequeueTaskAsync(string taskId)
|
|
{
|
|
DequeuedTaskIds.Add(taskId);
|
|
if (ThrowOnDequeueTask is not null) throw ThrowOnDequeueTask;
|
|
return Task.CompletedTask;
|
|
}
|
|
}
|
|
|
|
[Fact]
|
|
public async Task SetStatusOnRow_WhenWorkerThrows_RaisesErrorReported()
|
|
{
|
|
var worker = new ThrowingWorkerClient { ThrowOnSetTaskStatus = new Exception("status update offline") };
|
|
var vm = new TasksIslandViewModel(new TestDbFactory(NewContext), worker);
|
|
|
|
string? reportedError = null;
|
|
vm.ErrorReported += msg => reportedError = msg;
|
|
|
|
var row = new TaskRowViewModel { Id = "task-status-1", Status = TaskStatus.Idle };
|
|
await vm.SetStatusOnRowAsync(row, TaskStatus.Queued);
|
|
|
|
Assert.NotNull(reportedError);
|
|
Assert.Contains("status update offline", reportedError);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task CancelRunningTask_WhenWorkerThrows_RaisesErrorReported()
|
|
{
|
|
var worker = new ThrowingWorkerClient { ThrowOnCancelTask = new Exception("cancel offline") };
|
|
var vm = new TasksIslandViewModel(new TestDbFactory(NewContext), worker);
|
|
|
|
string? reportedError = null;
|
|
vm.ErrorReported += msg => reportedError = msg;
|
|
|
|
var row = new TaskRowViewModel { Id = "task-cancel-1", Status = TaskStatus.Running };
|
|
await vm.CancelRunningTaskCommand.ExecuteAsync(row);
|
|
|
|
Assert.NotNull(reportedError);
|
|
Assert.Contains("cancel offline", reportedError);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task RejectReviewToQueue_WhenWorkerThrows_RaisesErrorReported()
|
|
{
|
|
var worker = new ThrowingWorkerClient { ThrowOnRejectToQueue = new Exception("reject-to-queue offline") };
|
|
var vm = new TasksIslandViewModel(new TestDbFactory(NewContext), worker);
|
|
|
|
string? reportedError = null;
|
|
vm.ErrorReported += msg => reportedError = msg;
|
|
|
|
var row = new TaskRowViewModel { Id = "task-reject-queue-1", Status = TaskStatus.WaitingForReview };
|
|
await vm.RejectReviewToQueueAsync(row, "needs another pass");
|
|
|
|
Assert.NotNull(reportedError);
|
|
Assert.Contains("reject-to-queue offline", reportedError);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task RejectReviewToIdle_WhenWorkerThrows_RaisesErrorReported()
|
|
{
|
|
var worker = new ThrowingWorkerClient { ThrowOnRejectToIdle = new Exception("reject-to-idle offline") };
|
|
var vm = new TasksIslandViewModel(new TestDbFactory(NewContext), worker);
|
|
|
|
string? reportedError = null;
|
|
vm.ErrorReported += msg => reportedError = msg;
|
|
|
|
var row = new TaskRowViewModel { Id = "task-reject-idle-1", Status = TaskStatus.WaitingForReview };
|
|
await vm.RejectReviewToIdleCommand.ExecuteAsync(row);
|
|
|
|
Assert.NotNull(reportedError);
|
|
Assert.Contains("reject-to-idle offline", reportedError);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task ToggleDone_MarkDone_WhenWorkerThrows_RaisesErrorReported_AndRevertsRow()
|
|
{
|
|
var worker = new ThrowingWorkerClient { ThrowOnSetTaskDone = new Exception("mark done offline") };
|
|
var vm = new TasksIslandViewModel(new TestDbFactory(NewContext), worker);
|
|
|
|
string? reportedError = null;
|
|
vm.ErrorReported += msg => reportedError = msg;
|
|
|
|
var row = new TaskRowViewModel { Id = "task-toggle-done-1", Status = TaskStatus.Idle, Done = false };
|
|
await vm.ToggleDoneCommand.ExecuteAsync(row);
|
|
|
|
Assert.NotNull(reportedError);
|
|
Assert.Contains("mark done offline", reportedError);
|
|
Assert.False(row.Done);
|
|
Assert.Equal(TaskStatus.Idle, row.Status);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task ToggleDone_Untoggle_WhenWorkerThrows_RaisesErrorReported_AndRevertsRow()
|
|
{
|
|
var worker = new ThrowingWorkerClient { ThrowOnUnsetTaskDone = new Exception("unmark done offline") };
|
|
var vm = new TasksIslandViewModel(new TestDbFactory(NewContext), worker);
|
|
|
|
string? reportedError = null;
|
|
vm.ErrorReported += msg => reportedError = msg;
|
|
|
|
var row = new TaskRowViewModel { Id = "task-toggle-done-2", Status = TaskStatus.Done, Done = true };
|
|
await vm.ToggleDoneCommand.ExecuteAsync(row);
|
|
|
|
Assert.NotNull(reportedError);
|
|
Assert.Contains("unmark done offline", reportedError);
|
|
Assert.True(row.Done);
|
|
Assert.Equal(TaskStatus.Done, row.Status);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task ToggleDone_MarkDone_WhenWorkerSucceeds_UpdatesRow_NoError()
|
|
{
|
|
var worker = new ThrowingWorkerClient();
|
|
var vm = new TasksIslandViewModel(new TestDbFactory(NewContext), worker);
|
|
|
|
string? reportedError = null;
|
|
vm.ErrorReported += msg => reportedError = msg;
|
|
|
|
var row = new TaskRowViewModel { Id = "task-toggle-done-3", Status = TaskStatus.Idle, Done = false };
|
|
await vm.ToggleDoneCommand.ExecuteAsync(row);
|
|
|
|
Assert.Null(reportedError);
|
|
Assert.True(row.Done);
|
|
Assert.Equal(TaskStatus.Done, row.Status);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task RemoveFromQueue_WhenWorkerThrows_RaisesErrorReported_AndRevertsRow()
|
|
{
|
|
var worker = new ThrowingWorkerClient { ThrowOnDequeueTask = new Exception("dequeue offline") };
|
|
var vm = new TasksIslandViewModel(new TestDbFactory(NewContext), worker);
|
|
|
|
string? reportedError = null;
|
|
vm.ErrorReported += msg => reportedError = msg;
|
|
|
|
var row = new TaskRowViewModel { Id = "task-dequeue-1", Status = TaskStatus.Queued };
|
|
await vm.RemoveFromQueueCommand.ExecuteAsync(row);
|
|
|
|
Assert.NotNull(reportedError);
|
|
Assert.Contains("dequeue offline", reportedError);
|
|
Assert.Equal(TaskStatus.Queued, row.Status);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task RemoveFromQueue_CascadesToQueuedChildren_ViaGuardedHubCall()
|
|
{
|
|
var listId = Guid.NewGuid().ToString();
|
|
var parentId = Guid.NewGuid().ToString();
|
|
var childId = Guid.NewGuid().ToString();
|
|
await using (var db = NewContext())
|
|
{
|
|
db.Lists.Add(new ListEntity { Id = listId, Name = "L", CreatedAt = DateTime.UtcNow });
|
|
db.Tasks.Add(new TaskEntity
|
|
{
|
|
Id = parentId, ListId = listId, Title = "parent", Status = TaskStatus.Queued,
|
|
Number = 1, CreatedAt = DateTime.UtcNow,
|
|
});
|
|
db.Tasks.Add(new TaskEntity
|
|
{
|
|
Id = childId, ListId = listId, Title = "child", Status = TaskStatus.Queued,
|
|
Number = 2, ParentTaskId = parentId, BlockedByTaskId = parentId, CreatedAt = DateTime.UtcNow,
|
|
});
|
|
await db.SaveChangesAsync();
|
|
}
|
|
|
|
var worker = new ThrowingWorkerClient();
|
|
var vm = new TasksIslandViewModel(new TestDbFactory(NewContext), worker);
|
|
var parentRow = new TaskRowViewModel { Id = parentId, Status = TaskStatus.Queued, HasQueuedSubtasks = true };
|
|
var childRow = new TaskRowViewModel { Id = childId, ParentTaskId = parentId, Status = TaskStatus.Queued, BlockedByTaskId = parentId };
|
|
vm.Items.Add(parentRow);
|
|
vm.Items.Add(childRow);
|
|
|
|
await vm.RemoveFromQueueCommand.ExecuteAsync(parentRow);
|
|
|
|
Assert.Contains(childId, worker.DequeuedTaskIds);
|
|
Assert.Contains(parentId, worker.DequeuedTaskIds);
|
|
Assert.Equal(TaskStatus.Idle, childRow.Status);
|
|
Assert.Null(childRow.BlockedByTaskId);
|
|
Assert.Equal(TaskStatus.Idle, parentRow.Status);
|
|
Assert.False(parentRow.HasQueuedSubtasks);
|
|
}
|
|
}
|