ADR-142: Introduce modal message service with FIFO queuing#99
Conversation
Co-authored-by: jodavis <6740581+jodavis@users.noreply.github.com>
Co-authored-by: jodavis <6740581+jodavis@users.noreply.github.com>
| } | ||
|
|
||
| @if (SpeakingMessage is not null) | ||
| @if (CurrentMessage is not null) |
There was a problem hiding this comment.
This should be moved to a different view component, since it's no longer specifically part of the Conversation UI.
There was a problem hiding this comment.
Done in edebe56. Created ModalMessageUI.razor with the modal rendering, added it to Remote.razor, and removed it from ConversationUI.razor.
| .AddScoped<ModalMessageService>() | ||
| .AddScoped<IModalMessageService>(sp => sp.GetRequiredService<ModalMessageService>()) | ||
| .AddScoped<Models.ModalMessageView>(sp => sp.GetRequiredService<ModalMessageService>().View); |
There was a problem hiding this comment.
| .AddScoped<ModalMessageService>() | |
| .AddScoped<IModalMessageService>(sp => sp.GetRequiredService<ModalMessageService>()) | |
| .AddScoped<Models.ModalMessageView>(sp => sp.GetRequiredService<ModalMessageService>().View); | |
| .AddSingleton<IModalMessageService, ModalMessageService>() | |
| .AddSingleton<Models.ModalMessageView>(); |
If ModalMessageService accepts the view as a constructor parameter, we can simplify the configuration of these services.
Also, these services should be Singletons, since (a) they don't have any configuration that can change, and (b) they are tied to the UI, so the UI could be holding onto the view.
There was a problem hiding this comment.
Done in edebe56. ModalMessageService now accepts ModalMessageView via its constructor, and both are registered as Singletons:
.AddSingleton<Models.ModalMessageView>()
.AddSingleton<IModalMessageService, ModalMessageService>()
test/AdaptiveRemote.App.Tests/Services/ModalMessages/ModalMessageServiceTests.cs
Show resolved
Hide resolved
| } | ||
|
|
||
| [TestMethod] | ||
| public async Task ShowMessageAsync_KeepAlive_ViewModelRaisesPropertyChangedAsync() |
There was a problem hiding this comment.
This isn't testing keepAlive, but the method name suggests it is.
There was a problem hiding this comment.
Fixed in edebe56 — renamed to ShowMessageAsync_ViewModelRaisesPropertyChanged.
| [TestMethod] | ||
| public async Task ShowMessageAsync_SingleMessage_SetsAndClearsCurrentMessageAsync() | ||
| { | ||
| // Arrange | ||
| using ModalMessageService sut = CreateSut(); | ||
| string? messageWhileRunning = null; | ||
|
|
||
| // Act | ||
| await sut.ShowMessageAsync("Hello", async _ => | ||
| { | ||
| messageWhileRunning = sut.View.CurrentMessage; | ||
| await Task.CompletedTask; | ||
| }); | ||
|
|
||
| // Assert | ||
| messageWhileRunning.Should().Be("Hello", because: "message should be visible while body runs"); | ||
| sut.View.CurrentMessage.Should().BeNull(because: "message should be cleared after body completes"); | ||
| } |
There was a problem hiding this comment.
Add a case that verifies the message stays set and the ShowMessageAsync task doesn't complete, until the body method completes.
There was a problem hiding this comment.
Added in edebe56 as ShowMessageAsync_WhileBodyRunning_TaskIsNotComplete. It uses a TaskCompletionSource to block the body, then asserts showTask.Should().NotBeComplete() and View.CurrentMessage.Should().Be("Hello") before unblocking.
| secondMessageSeen.Should().Be("Second", because: "the queue should continue processing after a faulted body"); | ||
| sut.View.CurrentMessage.Should().BeNull(); | ||
| } | ||
|
|
There was a problem hiding this comment.
Add a cases for cancellation -- verify the token is passed to the body message, the message value is cleared, the ShowMessageAsync method returns cancelled, and the next method call is unblocked. (These don't all have to be separate tests, but all the behaviors should be covered.)
There was a problem hiding this comment.
Added in edebe56 as three tests:
ShowMessageAsync_Cancellation_TokenIsPassedToBody— captures the token in the body and verifiesIsCancellationRequestedchanges whencts.Cancel()is calledShowMessageAsync_Cancellation_ClearsMessageAndReturnsCancelled— verifiesView.CurrentMessageis null and the task isBeCanceledShowMessageAsync_Cancellation_SubsequentMessageStillShows— verifies the queue continues processing after a cancelled entry
Also updated ModalMessageService.ProcessQueueAsync to call TrySetCanceled (not TrySetException) when the body throws OperationCanceledException due to the request's cancellation token.
…tests, cancellation tests Co-authored-by: jodavis <6740581+jodavis@users.noreply.github.com>
All six comments addressed in edebe56. Summary of changes:
|
* Implement ADR-142: Modal message service with FIFO queuing * Fix: propagate cancellationToken to body in ModalMessageService * Address PR review: extract ModalMessageUI, Singleton DI, synchronous tests, cancellation tests --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jodavis <6740581+jodavis@users.noreply.github.com>
* Implement ADR-142: Modal message service with FIFO queuing * Fix: propagate cancellationToken to body in ModalMessageService * Address PR review: extract ModalMessageUI, Singleton DI, synchronous tests, cancellation tests --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jodavis <6740581+jodavis@users.noreply.github.com>
Implements a dedicated modal message service to decouple message display from
ConversationController, enabling both conversation and future programming messages to share a single, properly queued overlay UI.New:
IModalMessageService+ModalMessageServicekeepAliveflag keeps the message visible after the body completes (used for confirmation prompts that need to remain while awaiting spoken response)cancellationTokenis stored per-request and forwarded to the body;OperationCanceledExceptionproduces a Canceled (not Faulted) taskNew:
ModalMessageView(MvvmObject)Holds
CurrentMessage : string?, consistent with the existingConversationViewMVVM pattern. Registered as a Singleton alongsideIModalMessageService, withModalMessageViewinjected intoModalMessageServicevia constructor.New:
ModalMessageUI.razorExtracted into its own component (rendered from
Remote.razor) since the modal is no longer specific to conversation.ConversationUI.razoris simplified to only handle conversation status and the listening border.Removed:
ConversationView.SpeakingMessageThe
conversation-speaking-messageCSS class is preserved; it is now rendered byModalMessageUI.razorbound toModalMessageView.CurrentMessage.Tests
ModalMessageServiceTests(all[TestMethod] void) covering:PropertyChangednotificationConversationControllerTestsupdated:Expect_Synthesis_Sayalso wires the modal message mock so synthesis expectations implicitly verify the service is called with the correct phrase🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.