diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Deployment/DebugView.razor b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Deployment/DebugView.razor index 5b0b00ae..b5a041ca 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Deployment/DebugView.razor +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Deployment/DebugView.razor @@ -85,7 +85,7 @@ + Disabled="@(_connected || _instancesLoading)" DataTestPrefix="debug-instance" />
@if (!_connected) @@ -263,6 +263,12 @@ private bool _loading = true; private bool _connected; private bool _connecting; + + // True for the whole of LoadInstancesForSite. Blazor renders as soon as that load hits its + // first await, and the page is not _connected then — so without this the picker is enabled + // while it is still showing the PREVIOUS site's tree, and anything typed into it is wiped + // when the new lists land (the picker reads a new list object as "the site changed"). + private bool _instancesLoading; private bool _connectedFromStorage; // Which tab pane is visible — "attributes" (default) or "alarms". @@ -428,30 +434,45 @@ } } + /// + /// Loads the selected site's instances and areas into the picker's two lists, holding + /// _instancesLoading for the whole round trip so the field cannot be typed into + /// while it is still showing the previous site's tree. The flag is cleared in a + /// finally: a scope refusal or a repository fault must not leave the picker + /// disabled forever. + /// private async Task LoadInstancesForSite() { - _siteInstances = new(); - _siteAreas = new(); - _selectedInstanceId = 0; - if (_selectedSiteId == 0) return; - // Site scoping (CentralUI-002): re-check the claim server-side — a query - // string or stale localStorage value could name a site outside the grant. - if (!await SiteScope.IsSiteAllowedAsync(_selectedSiteId)) - { - _selectedSiteId = 0; - _toast.ShowError("You are not permitted to debug instances on that site."); - return; - } + _instancesLoading = true; try { - _siteInstances = (await TemplateEngineRepository.GetInstancesBySiteIdAsync(_selectedSiteId)) - .Where(i => i.State == InstanceState.Enabled) - .ToList(); - _siteAreas = (await TemplateEngineRepository.GetAreasBySiteIdAsync(_selectedSiteId)).ToList(); + _siteInstances = new(); + _siteAreas = new(); + _selectedInstanceId = 0; + if (_selectedSiteId == 0) return; + // Site scoping (CentralUI-002): re-check the claim server-side — a query + // string or stale localStorage value could name a site outside the grant. + if (!await SiteScope.IsSiteAllowedAsync(_selectedSiteId)) + { + _selectedSiteId = 0; + _toast.ShowError("You are not permitted to debug instances on that site."); + return; + } + try + { + _siteInstances = (await TemplateEngineRepository.GetInstancesBySiteIdAsync(_selectedSiteId)) + .Where(i => i.State == InstanceState.Enabled) + .ToList(); + _siteAreas = (await TemplateEngineRepository.GetAreasBySiteIdAsync(_selectedSiteId)).ToList(); + } + catch (Exception ex) + { + _toast.ShowError($"Failed to load instances: {ex.Message}"); + } } - catch (Exception ex) + finally { - _toast.ShowError($"Failed to load instances: {ex.Message}"); + _instancesLoading = false; } } diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Deployment/FlowDebugger.razor b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Deployment/FlowDebugger.razor index d5ab097f..1dfb1a66 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Deployment/FlowDebugger.razor +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Deployment/FlowDebugger.razor @@ -69,7 +69,7 @@ loaded once per site — the picker treats a new list as a site change. *@
diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Deployment/FlowDebugger.razor.cs b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Deployment/FlowDebugger.razor.cs index a2296c36..9e98afb1 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Deployment/FlowDebugger.razor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Deployment/FlowDebugger.razor.cs @@ -122,6 +122,16 @@ public partial class FlowDebugger : ComponentBase, IDisposable /// The selected site's areas; the picker nests them into its tree. See . private List _areas = []; + + /// + /// True while is fetching a site's instances and areas, and + /// the sole reason the picker is disabled for that window. Blazor renders the moment the load + /// hits its first await, and by then is already the NEW site + /// while the two lists are still the OLD ones — so without this the field is enabled over + /// stale data, and anything typed into it is wiped when the lists land (the picker reads a new + /// list object as "the site changed" and clears its text). + /// + private bool _instancesLoading; private List _scripts = []; private int _selectedMethodId; @@ -419,19 +429,33 @@ public partial class FlowDebugger : ComponentBase, IDisposable await PredictAsync(); } + /// + /// Loads the selected site's instances and areas into the picker's two lists, holding + /// for the whole round trip so the field cannot be typed + /// into while it is still showing the previous site's tree. The flag is cleared in a + /// finally: a repository fault must not leave the picker disabled forever. + /// private async Task LoadInstancesAsync() { - if (_selectedSiteId <= 0) + _instancesLoading = true; + try { - _instances = []; - _areas = []; - return; - } + if (_selectedSiteId <= 0) + { + _instances = []; + _areas = []; + return; + } - _instances = (await TemplateEngineRepository.GetInstancesBySiteIdAsync(_selectedSiteId)) - .OrderBy(i => i.UniqueName, StringComparer.OrdinalIgnoreCase).ToList(); - // The picker nests these itself (InstanceTreeBuilder), so order does not matter here. - _areas = (await TemplateEngineRepository.GetAreasBySiteIdAsync(_selectedSiteId)).ToList(); + _instances = (await TemplateEngineRepository.GetInstancesBySiteIdAsync(_selectedSiteId)) + .OrderBy(i => i.UniqueName, StringComparer.OrdinalIgnoreCase).ToList(); + // The picker nests these itself (InstanceTreeBuilder), so order does not matter here. + _areas = (await TemplateEngineRepository.GetAreasBySiteIdAsync(_selectedSiteId)).ToList(); + } + finally + { + _instancesLoading = false; + } } /// diff --git a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Deployment/DebugViewInstancePickerTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Deployment/DebugViewInstancePickerTests.cs index 99486101..31b7e37c 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Deployment/DebugViewInstancePickerTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Deployment/DebugViewInstancePickerTests.cs @@ -29,23 +29,24 @@ public class DebugViewInstancePickerTests : BunitContext { private const int SiteId = 1; + private readonly ITemplateEngineRepository _repo = Substitute.For(); + private void RegisterServices() { // DebugView touches localStorage on render; let bUnit answer loosely. JSInterop.Mode = JSRuntimeMode.Loose; - var repo = Substitute.For(); - repo.GetInstancesBySiteIdAsync(SiteId).Returns(new List + _repo.GetInstancesBySiteIdAsync(SiteId).Returns(new List { new("Sim01") { Id = 7, SiteId = SiteId, AreaId = 2, State = InstanceState.Enabled }, new("Off01") { Id = 8, SiteId = SiteId, State = InstanceState.Disabled }, }); - repo.GetAreasBySiteIdAsync(SiteId).Returns(new List + _repo.GetAreasBySiteIdAsync(SiteId).Returns(new List { new("Line1") { Id = 1, SiteId = SiteId }, new("Cell A") { Id = 2, SiteId = SiteId, ParentAreaId = 1 }, }); - Services.AddSingleton(repo); + Services.AddSingleton(_repo); var siteRepo = Substitute.For(); siteRepo.GetAllSitesAsync().Returns(new List @@ -156,6 +157,35 @@ public class DebugViewInstancePickerTests : BunitContext Assert.False(connect.HasAttribute("disabled")); } + /// + /// The picker must stay disabled for the WHOLE site load, not just until the site id lands. + /// Blazor renders as soon as LoadInstancesForSite hits its first await, and at that point + /// the page is not _connected — so the field was already enabled while the site's lists + /// were still loading. Typing into it then (an operator, or Playwright's FillAsync, which + /// waits only for "enabled") lost the text: the picker reads the freshly-assigned lists as + /// "the site changed" and clears the field and closes the panel. + /// + [Fact] + public void PickerStaysDisabled_UntilTheSitesInstancesHaveLoaded() + { + RegisterServices(); + // Hold the instance load open so the render that follows the first await is observable. + var gate = new TaskCompletionSource>(); + _repo.GetInstancesBySiteIdAsync(SiteId).Returns(gate.Task); + + var cut = Render(); + cut.Find("[data-test='debug-site-select']").Change(SiteId.ToString()); + + Assert.True(cut.Find("[data-test='debug-instance-input']").HasAttribute("disabled")); + Assert.True(cut.Find("[data-test='debug-instance-toggle']").HasAttribute("disabled")); + + cut.InvokeAsync(() => gate.SetResult( + [new Instance("Sim01") { Id = 7, SiteId = SiteId, AreaId = 2, State = InstanceState.Enabled }])); + + cut.WaitForAssertion(() => + Assert.False(cut.Find("[data-test='debug-instance-input']").HasAttribute("disabled"))); + } + [Fact] public void WhileConnected_ThePickerIsDisabled() { diff --git a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Deployment/FlowDebuggerPageTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Deployment/FlowDebuggerPageTests.cs index ed738665..1b0e19d0 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Deployment/FlowDebuggerPageTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Deployment/FlowDebuggerPageTests.cs @@ -816,6 +816,36 @@ public class FlowDebuggerPageTests : BunitContext Assert.Equal("Sim01", cut.Find("[data-test='flow-instance-input']").GetAttribute("value"))); } + /// + /// The picker must stay disabled for the WHOLE site load, not just until the site id lands. + /// Blazor renders as soon as OnSiteChangedAsync hits its first await, and at that point + /// _selectedSiteId is already non-zero while _instances/_areas are still the + /// previous site's lists — so an operator (or Playwright's FillAsync, which waits only for + /// "enabled") could type into the field and have the load wipe the text and close the panel a + /// moment later, because the picker reads a new list object as "the site changed". + /// + [Fact] + public void ScriptEntry_PickerStaysDisabled_UntilTheSitesInstancesHaveLoaded() + { + RegisterServices(); + WireInstance(); + // Hold the instance load open so the render that follows the first await is observable. + var gate = new TaskCompletionSource>(); + _templates.GetInstancesBySiteIdAsync(1, Arg.Any()).Returns(gate.Task); + + var cut = Render(); + cut.Find("button[data-test='flow-entry-script']").Click(); + cut.Find("select[data-test='flow-site-select']").Change("1"); + + Assert.True(cut.Find("[data-test='flow-instance-input']").HasAttribute("disabled")); + Assert.True(cut.Find("[data-test='flow-instance-toggle']").HasAttribute("disabled")); + + cut.InvokeAsync(() => gate.SetResult([new Instance("Sim01") { Id = 7, SiteId = 1, TemplateId = 10 }])); + + cut.WaitForAssertion(() => + Assert.False(cut.Find("[data-test='flow-instance-input']").HasAttribute("disabled"))); + } + [Fact] public void ScriptEntry_SiteChange_ClearsThePickedInstance() {