fix(central-ui): Debug View reconnect guards the stored instance; drop select-era sort; deep-link-in-area test

Three final-review findings on the two picker pages: 1. DebugView's localStorage auto-reconnect restored the stored instance id with no membership check, where the ?instanceId= path has one — a stored instance that is no longer Enabled (or was deleted, or moved sites) is filtered out of the list the picker is handed, so the field rendered blank while the page held a non-zero id and auto-connected to it anyway. The guard now mirrors the deep-link path: forget the stored instance id, tell the operator, and leave the site selected and loaded so they can pick again. 2. FlowDebugger's _instances kept an OrderBy(UniqueName) from the flat <select> era; InstanceTreeBuilder sorts areas and instances itself. 3. The deep-link test is joined by one where the instance sits two areas deep, so the field text is pinned to resolve from the flat list with the panel never opened. Claude-Session: https://claude.ai/code/session_01NcMG6xXdsJbmD2wYAgkK29
2026-09-08 10:01:22 -04:00
parent 82f6fb949e
commit 52726cf30d
4 changed files with 77 additions and 5 deletions
@@ -421,15 +421,31 @@
        {
            _selectedSiteId = siteId;
            await LoadInstancesForSite();
+
+            // Membership guard, mirroring the ?instanceId= path above. A stored instance that
+            // has since been disabled (or deleted, or moved to another site) is not in the list
+            // the picker was handed, so it renders a blank field while the page holds a non-zero
+            // id and auto-connects to it anyway. Forget the stored instance instead — the site
+            // stays selected and loaded, so the operator just picks again.
+            var stored = _siteInstances.FirstOrDefault(i => i.Id == instanceId);
+            if (stored == null)
+            {
+                await JS.InvokeVoidAsync("localStorage.removeItem", "debugView.instanceId");
+                _toast.ShowInfo(
+                    "The instance from your previous session is no longer available for debug streaming.",
+                    autoDismissMs: 8000);
+                StateHasChanged();
+                return;
+            }
+
            _selectedInstanceId = instanceId;
            _connectedFromStorage = true;
            StateHasChanged();
            await Connect();

            // Auto-reconnect notice — the user didn't initiate this connection.
-            var inst = _siteInstances.FirstOrDefault(i => i.Id == instanceId);
            _toast.ShowInfo(
-                $"Auto-reconnected to {inst?.UniqueName ?? "instance"} from previous session.",
+                $"Auto-reconnected to {stored.UniqueName} from previous session.",
                autoDismissMs: 8000);
        }
    }
@@ -447,9 +447,9 @@ public partial class FlowDebugger : ComponentBase, IDisposable
                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.
+            // Neither list is sorted here: the picker nests and orders both itself
+            // (InstanceTreeBuilder). The OrderBy this replaces dated from the flat <select>.
+            _instances = (await TemplateEngineRepository.GetInstancesBySiteIdAsync(_selectedSiteId)).ToList();
            _areas = (await TemplateEngineRepository.GetAreasBySiteIdAsync(_selectedSiteId)).ToList();
        }
        finally
@@ -186,6 +186,31 @@ public class DebugViewInstancePickerTests : BunitContext
            Assert.False(cut.Find("[data-test='debug-instance-input']").HasAttribute("disabled")));
    }

+    /// <summary>
+    /// The localStorage auto-reconnect must apply the same membership guard the
+    /// <c>?instanceId=</c> deep link does. Off01 is Disabled, so the page filters it out of the
+    /// list the picker is handed — restoring it anyway left the field blank while the page held
+    /// a non-zero id and auto-connected to it. The stored id is dropped instead, and the site
+    /// stays selected and loaded so the operator can pick again.
+    /// </summary>
+    [Fact]
+    public void AutoReconnect_FromStorage_SkipsAnInstanceThatIsNoLongerAvailable()
+    {
+        RegisterServices();
+        JSInterop.Setup<string>("localStorage.getItem", "debugView.siteId").SetResult(SiteId.ToString());
+        JSInterop.Setup<string>("localStorage.getItem", "debugView.instanceId").SetResult("8");
+
+        var cut = Render<DebugViewPage>();
+
+        cut.WaitForAssertion(() =>
+            Assert.Equal(string.Empty, cut.Find("[data-test='debug-instance-input']").GetAttribute("value")));
+
+        // Nothing is selected, so Connect stays disabled — and the stale id is forgotten.
+        var connect = cut.FindAll("button.btn.btn-primary.btn-sm").First(b => b.TextContent.Contains("Connect"));
+        Assert.True(connect.HasAttribute("disabled"));
+        JSInterop.VerifyInvoke("localStorage.removeItem");
+    }
+
    [Fact]
    public void WhileConnected_ThePickerIsDisabled()
    {
@@ -816,6 +816,37 @@ public class FlowDebuggerPageTests : BunitContext
            Assert.Equal("Sim01", cut.Find("[data-test='flow-instance-input']").GetAttribute("value")));
    }

+    /// <summary>
+    /// The same deep link, but the instance sits two areas deep. The picker resolves the field
+    /// text from <c>SelectedInstanceId</c> against the flat instance list, not by walking the
+    /// tree — so a nested instance must show its name with the panel never opened (and therefore
+    /// with Line1 / Cell A never expanded).
+    /// </summary>
+    [Fact]
+    public void ScriptEntry_DeepLink_InstanceIdInsideAnArea_ShowsItsNameWithoutOpeningThePanel()
+    {
+        RegisterServices();
+        WireInstance();
+        _templates.GetAreasBySiteIdAsync(1, Arg.Any<CancellationToken>())
+            .Returns(Task.FromResult<IReadOnlyList<Area>>(
+                [new Area("Line1") { Id = 1, SiteId = 1 }, new Area("Cell A") { Id = 2, SiteId = 1, ParentAreaId = 1 }]));
+        var nested = new Instance("Sim01") { Id = 7, SiteId = 1, TemplateId = 10, AreaId = 2 };
+        _templates.GetInstancesBySiteIdAsync(1, Arg.Any<CancellationToken>())
+            .Returns(Task.FromResult<IReadOnlyList<Instance>>([nested]));
+        _templates.GetInstanceByIdAsync(7, Arg.Any<CancellationToken>()).Returns(nested);
+
+        var nav = Services.GetRequiredService<NavigationManager>();
+        nav.NavigateTo("/deployment/flow-debugger?instanceId=7");
+
+        var cut = Render<FlowDebuggerPage>();
+
+        cut.WaitForAssertion(() =>
+            Assert.Equal("Sim01", cut.Find("[data-test='flow-instance-input']").GetAttribute("value")));
+        // The panel was never opened, so nothing of the tree is rendered.
+        Assert.Empty(cut.FindAll("[data-test='flow-instance-panel']"));
+        Assert.Empty(cut.FindAll("[data-test='flow-instance-row']"));
+    }
+
    /// <summary>
    /// The picker must stay disabled for the WHOLE site load, not just until the site id lands.
    /// Blazor renders as soon as <c>OnSiteChangedAsync</c> hits its first await, and at that point