From 76e036fb42e52edcdc93ccf9795524a5ef774d8f Mon Sep 17 00:00:00 2001 From: Allamagoosa <36551512+dmatlock171@users.noreply.github.com> Date: Mon, 24 Aug 2026 10:02:46 -0700 Subject: [PATCH] Report the abort on the book the user answered for With several books in flight, the book whose dialog was answered showed "Cancelled" while some unrelated book showed "Error, Abort". Tearing the queue down is claimed by whichever book finishes first, and the books genuinely are interchangeable for that. The status left on a row is not: it is read afterwards by someone who remembers which book they were asked about. The two jobs are now separate. ShowRetryDialogAsync records the answering book as BadBookSessionContext.AbortOriginator, written before Override so that a book reading Override and racing ahead to the queue loop cannot find the originator still unset and conclude there was none. The loop gives the abort to that book and Cancelled to every book that inherited the answer. ClaimAbort is unchanged and still decides the teardown. With no dialog in play - Bad Book set to Abort in settings - nobody answered anything, there is no originator, and the book that claimed the teardown keeps the abort as before. --- .../ProcessQueue/BadBookSessionContext.cs | 19 +++++++++- .../ProcessQueue/ProcessBookViewModel.cs | 8 ++++ .../ProcessQueue/ProcessQueueViewModel.cs | 24 ++++++++++-- .../ProcessQueueDispatchTests.cs | 37 ++++++++++++++++++- 4 files changed, 82 insertions(+), 6 deletions(-) diff --git a/Source/LibationUiBase/ProcessQueue/BadBookSessionContext.cs b/Source/LibationUiBase/ProcessQueue/BadBookSessionContext.cs index 6a2fd885..e2cb1a88 100644 --- a/Source/LibationUiBase/ProcessQueue/BadBookSessionContext.cs +++ b/Source/LibationUiBase/ProcessQueue/BadBookSessionContext.cs @@ -14,5 +14,22 @@ public class BadBookSessionContext /// internal SemaphoreSlim DialogGate { get; } = new(1, 1); - public void Reset() => Override = null; + /// + /// The book the user actually answered "Abort" for, when one was asked. Null when the abort came + /// from the Bad Book setting rather than a dialog, and null when nothing has aborted. + /// + /// + /// Recorded because tearing the queue down and reporting the abort are different jobs. The + /// teardown is claimed by whichever book finishes first and the books are interchangeable for + /// it; the status left on a row is read afterwards by a person who remembers which book they + /// were asked about, and "Cancelled" on that row with "Error, Abort" on some other book's is + /// the wrong way round. + /// + public ProcessBookViewModel? AbortOriginator { get; set; } + + public void Reset() + { + Override = null; + AbortOriginator = null; + } } diff --git a/Source/LibationUiBase/ProcessQueue/ProcessBookViewModel.cs b/Source/LibationUiBase/ProcessQueue/ProcessBookViewModel.cs index bcf3da20..5f6d4ad8 100644 --- a/Source/LibationUiBase/ProcessQueue/ProcessBookViewModel.cs +++ b/Source/LibationUiBase/ProcessQueue/ProcessBookViewModel.cs @@ -533,6 +533,14 @@ public class ProcessBookViewModel : ReactiveObject { var result = await BadBookActionDialogBase.Show(skipDialogText, "Skip this book?"); + // This is the book the user was looking at when they said stop, so this is the book whose + // row should say so afterwards. Every other book in flight inherits the answer through + // Override below and was cancelled by it, which is a different thing and should read as + // one. Written before Override, not after: a book reading Override and racing ahead to + // the queue loop must not find the originator still unset and conclude there wasn't one. + if (result.Action is DialogResult.Abort && _badBookSession is not null) + _badBookSession.AbortOriginator = this; + // Abort is a statement about the run, not about this one book, so it becomes the session // answer whether or not "apply to all" was checked. Without this a user who aborts with // several books in flight is asked the same question again by every one of them, and the diff --git a/Source/LibationUiBase/ProcessQueue/ProcessQueueViewModel.cs b/Source/LibationUiBase/ProcessQueue/ProcessQueueViewModel.cs index 85c10efc..0862bca5 100644 --- a/Source/LibationUiBase/ProcessQueue/ProcessQueueViewModel.cs +++ b/Source/LibationUiBase/ProcessQueue/ProcessQueueViewModel.cs @@ -29,6 +29,13 @@ public class ProcessQueueViewModel : ReactiveObject public ObservableCollection LogEntries { get; } = new(); public TrackedQueue Queue { get; } = new(); private readonly BadBookSessionContext _badBookSession = new(); + + /// + /// Internal rather than private so a test can stand in for a user having answered the bad book + /// dialog, which the dispatch tests otherwise never reach - their books finish on command instead + /// of failing their way into it. + /// + internal BadBookSessionContext BadBookSession => _badBookSession; public Task? QueueRunner { get; private set; } public bool Running => !QueueRunner?.IsCompleted ?? false; @@ -756,10 +763,21 @@ public class ProcessQueueViewModel : ReactiveObject { if (tearsDownTheQueue) await CancelAllAsync(book); - else + + // Tearing the queue down and reporting the abort are separate jobs, and only the + // first one is indifferent to which book does it. The status left on a row is read + // afterwards by someone who remembers which book they were asked about, so the + // abort belongs to the book they answered for - not to whichever book happened to + // reach the teardown first, which put "Cancelled" on the row they aborted and + // "Error, Abort" on an unrelated one. With no dialog in play (Bad Book set to + // Abort in settings) nobody answered anything, and the book that claimed the + // teardown keeps the abort as before. + var originator = _badBookSession.AbortOriginator; + var reportsTheAbort = originator is null ? tearsDownTheQueue : ReferenceEquals(originator, book); + + if (!reportsTheAbort) { - // Inherited the abort rather than triggering the teardown. It was cancelled by - // the book that did, and that is what it should report. + // Cancelled by the abort rather than the cause of it. book.Result = ProcessBookResult.Cancelled; book.Status = ProcessBookStatus.Cancelled; } diff --git a/Source/_Tests/LibationUiBase.Tests/ProcessQueueDispatchTests.cs b/Source/_Tests/LibationUiBase.Tests/ProcessQueueDispatchTests.cs index b8d023dc..672b160b 100644 --- a/Source/_Tests/LibationUiBase.Tests/ProcessQueueDispatchTests.cs +++ b/Source/_Tests/LibationUiBase.Tests/ProcessQueueDispatchTests.cs @@ -264,8 +264,11 @@ public class ProcessQueueDispatchTests queue.AddToQueue([a, b, c]); await books.WaitForStarted(3); - // All three inherit the abort, which is what a session-wide Abort answer produces. Only the - // first through gets to tear the queue down; the others were cancelled by it. + // No dialog was answered here - this is Bad Book set to Abort in settings, where every book + // that fails aborts on its own account. Nobody is the one the user was asked about, so the + // first book through tears the queue down and keeps the abort; the others were cancelled by it. + Assert.IsNull(queue.BadBookSession.AbortOriginator); + books.Finish("A", ProcessBookResult.FailedAbort); books.Finish("B", ProcessBookResult.FailedAbort); books.Finish("C", ProcessBookResult.FailedAbort); @@ -278,6 +281,36 @@ public class ProcessQueueDispatchTests Assert.AreEqual(2, cancelled, "Books that inherited the abort should report as cancelled."); } + [TestMethod] + public async Task the_book_the_user_aborted_reports_the_abort_whichever_book_tears_the_queue_down() + { + var (queue, books) = NewQueue(atOnce: 3); + + var a = Book("A"); + var b = Book("B"); + var c = Book("C"); + queue.AddToQueue([a, b, c]); + await books.WaitForStarted(3); + + // C is the book the user was looking at when they answered Abort. A and B inherit that answer + // through the session override, which is what puts all three here reporting FailedAbort. + queue.BadBookSession.AbortOriginator = c; + + // A finishes first and so claims the teardown. Nothing below depends on it winning - that race + // is what made the old status arbitrary - but this is the ordering that used to leave the row + // the user actually aborted saying "Cancelled" while A's said "Error, Abort". + books.Finish("A", ProcessBookResult.FailedAbort); + await Task.Delay(50); + books.Finish("B", ProcessBookResult.FailedAbort); + books.Finish("C", ProcessBookResult.FailedAbort); + + await RunToCompletion(queue); + + Assert.AreEqual(ProcessBookResult.FailedAbort, c.Result, "The book the user aborted did not report the abort."); + Assert.AreEqual(ProcessBookResult.Cancelled, a.Result, "A book that inherited the abort reported it as its own."); + Assert.AreEqual(ProcessBookResult.Cancelled, b.Result, "A book that inherited the abort reported it as its own."); + } + [TestMethod] public async Task a_book_that_throws_is_logged_and_the_rest_of_the_queue_still_finishes() {