diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Audit/ConfigurationAuditLog.razor b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Audit/ConfigurationAuditLog.razor index f6f89c9d..1c90f954 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Audit/ConfigurationAuditLog.razor +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Audit/ConfigurationAuditLog.razor @@ -343,7 +343,12 @@ await FetchPage(); } - private async Task OnPageChanged(int page) { _page = page; await FetchPage(); } + // The state modal is page-scoped here: its content resolves from _entries alone + // (this surface has no keyed detail fetch), so an entry paged out of view can + // never resolve again and the modal would sit on a permanently empty notice. + // Clearing on an explicit navigation action is intent, not a resolve-driven + // unmount, so it does not reopen the handler-disposal race. + private async Task OnPageChanged(int page) { _page = page; _modalEntryId = null; await FetchPage(); } private async Task FetchPage() { diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Monitoring/ParkedMessages.razor b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Monitoring/ParkedMessages.razor index d5c88341..df38904a 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Monitoring/ParkedMessages.razor +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Monitoring/ParkedMessages.razor @@ -388,14 +388,20 @@ // Drawer — holds the row's ID, never the row object. FetchPage() replaces // _messages wholesale after every Retry/Discard and on every page change, so a // captured ParkedMessageEntry would outlive its row and keep rendering values - // (attempt count, last-attempt time, error) that no longer exist server-side; - // PrevPage/NextPage do not clear the drawer, so it could even sit open over a - // page that never contained it. Mirrors ExecutionTreePage's _modalExecutionId. + // (attempt count, last-attempt time, error) that no longer exist server-side. + // Mirrors ExecutionTreePage's _modalExecutionId. + // + // ONLY user intent writes this field: OpenDrawer/CloseDrawer, plus the four + // navigation actions that invalidate page-scoped state (Search, OnSiteChanged, + // PrevPage, NextPage — the same places that clear _selectedIds). FetchPage() + // must never touch it: a data refresh that could clear the id would unmount the + // drawer mid-render and dispose its event handlers, which is the disposal race + // documented on the drawer markup above. private string? _drawerMessageId; - // Re-resolves the open drawer's row from the CURRENT page on every render. - // A row that has been retried, discarded or paged away resolves to null, which - // renders the drawer away — the drawer self-closes rather than going stale. + // Re-resolves the open drawer's row from the CURRENT page on every render, so + // it can never render a stale row. This drives the drawer's CONTENT only — an + // unresolvable row degrades to a notice; it does not unmount the drawer. private ParkedMessageEntry? DrawerMessage => _drawerMessageId is null ? null @@ -439,8 +445,13 @@ await FetchPage(); } - private async Task PrevPage() { _pageNumber--; _selectedIds.Clear(); await FetchPage(); } - private async Task NextPage() { _pageNumber++; _selectedIds.Clear(); await FetchPage(); } + // The drawer is page-scoped state, exactly like _selectedIds: paging away from + // the open message invalidates it, so both are cleared here rather than leaving + // the drawer mounted over a page that never contained its row. Clearing on an + // explicit navigation action is intent, not a resolve-driven unmount, so it does + // not reopen the disposal race. + private async Task PrevPage() { _pageNumber--; _selectedIds.Clear(); _drawerMessageId = null; await FetchPage(); } + private async Task NextPage() { _pageNumber++; _selectedIds.Clear(); _drawerMessageId = null; await FetchPage(); } private async Task FetchPage() { diff --git a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Pages/PageScopedDetailStateTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Pages/PageScopedDetailStateTests.cs new file mode 100644 index 00000000..0a9532e8 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Pages/PageScopedDetailStateTests.cs @@ -0,0 +1,120 @@ +using System.Reflection; +using System.Security.Claims; +using Bunit; +using Microsoft.AspNetCore.Components.Authorization; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; +using NSubstitute; +using ZB.MOM.WW.ScadaBridge.CentralUI.Auth; +using ZB.MOM.WW.ScadaBridge.CentralUI.Components.Shared; +using ZB.MOM.WW.ScadaBridge.Commons.Entities.Sites; +using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories; +using ZB.MOM.WW.ScadaBridge.Communication; +using AuditLogPage = ZB.MOM.WW.ScadaBridge.CentralUI.Components.Pages.Audit.ConfigurationAuditLog; +using ParkedMessagesPage = ZB.MOM.WW.ScadaBridge.CentralUI.Components.Pages.Monitoring.ParkedMessages; + +namespace ZB.MOM.WW.ScadaBridge.CentralUI.Tests.Pages; + +/// +/// The detail modal/drawer on these surfaces is gated on a held id — user intent — +/// rather than on the row still resolving, so that a data refresh can never unmount +/// the subtree mid-render and dispose its event handlers (see +/// NotificationReportDetailModalTests.Modal_StaysOpen_WhenItsRowLeavesThePage). +/// +/// That makes it the surface's own job to clear the id on navigation actions that +/// invalidate page-scoped state, and the criterion is per-surface: +/// +/// • ParkedMessages and ConfigurationAuditLog have NO keyed detail fetch — their +/// modal content resolves from the loaded page alone. An entry paged out of view +/// can never resolve again, so the surface would sit on a permanently empty +/// notice recoverable only by paging back. These must clear on paging. +/// +/// • NotificationReport and SiteCallsReport DO fetch detail by id. Their modal +/// still shows real content after the row leaves the page, so they deliberately +/// do NOT clear on paging, and are not covered here. +/// +/// Clearing on an explicit navigation action is intent, not a resolve-driven +/// unmount, so it does not reopen the handler-disposal race. +/// +public class PageScopedDetailStateTests : BunitContext +{ + private static void SetPrivate(object target, string field, object? value) => + target.GetType() + .GetField(field, BindingFlags.Instance | BindingFlags.NonPublic)! + .SetValue(target, value); + + private static object? GetPrivate(object target, string field) => + target.GetType() + .GetField(field, BindingFlags.Instance | BindingFlags.NonPublic)! + .GetValue(target); + + private static Task InvokePrivate(object target, string method, params object[] args) => + (Task)target.GetType() + .GetMethod(method, BindingFlags.Instance | BindingFlags.NonPublic)! + .Invoke(target, args)!; + + private void RegisterCommonServices() + { + var identity = new ClaimsIdentity( + new[] { new Claim(ClaimTypes.Name, "deployer") }, "TestCookie"); + var stubAuth = new StubAuthStateProvider( + new AuthenticationState(new ClaimsPrincipal(identity))); + Services.AddSingleton(stubAuth); + Services.AddScoped(_ => new SiteScopeService(stubAuth)); + Services.AddScoped(); + JSInterop.Mode = JSRuntimeMode.Loose; + } + + [Theory] + [InlineData("PrevPage")] + [InlineData("NextPage")] + public async Task ParkedMessages_Paging_ClosesTheDrawer(string pagingMethod) + { + RegisterCommonServices(); + + var siteRepo = Substitute.For(); + siteRepo.GetAllSitesAsync().Returns(new List()); + Services.AddSingleton(siteRepo); + Services.AddSingleton(new CommunicationService( + Options.Create(new CommunicationOptions()), + NullLogger.Instance)); + + var cut = Render(); + + // Drawer open on some message. + await cut.InvokeAsync(() => + SetPrivate(cut.Instance, "_drawerMessageId", "msg-being-viewed")); + Assert.Equal("msg-being-viewed", GetPrivate(cut.Instance, "_drawerMessageId")); + + await cut.InvokeAsync(() => InvokePrivate(cut.Instance, pagingMethod)); + + // Paging away invalidates it — this surface has no keyed detail to fall back + // on, so leaving it set would strand the drawer on an empty notice. + Assert.Null(GetPrivate(cut.Instance, "_drawerMessageId")); + } + + [Fact] + public async Task ConfigurationAuditLog_Paging_ClosesTheStateModal() + { + RegisterCommonServices(); + Services.AddSingleton(Substitute.For()); + + var cut = Render(); + + await cut.InvokeAsync(() => SetPrivate(cut.Instance, "_modalEntryId", 4242)); + Assert.Equal(4242, GetPrivate(cut.Instance, "_modalEntryId")); + + await cut.InvokeAsync(() => InvokePrivate(cut.Instance, "OnPageChanged", 2)); + + Assert.Null(GetPrivate(cut.Instance, "_modalEntryId")); + } + + private sealed class StubAuthStateProvider : AuthenticationStateProvider + { + private readonly AuthenticationState _state; + public StubAuthStateProvider(AuthenticationState state) => _state = state; + public override Task GetAuthenticationStateAsync() + => Task.FromResult(_state); + } +}