From d33e4e20b513b43e0c928e0310992bd5a0c6e0ad Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 6 Aug 2026 17:18:37 -0400 Subject: [PATCH 1/2] feat(adminui): #546 wire DriverStatusPanel into /hosts driver rows (collapsed expander) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DriverStatusPanel was orphaned — zero references repo-wide — so the Restart and Reconnect driver commands had NO reachable UI at all, despite working end to end over the new gRPC mesh transport. Nothing failed to build, test or deploy while that was true, which is why it went unnoticed. The panel is now reachable per driver row on /hosts, behind a per-row "Controls" expander that is collapsed by default: the table stays scannable, and only the opened panels hold snapshot-store subscriptions. Expansion state is a HashSet keyed by DriverInstanceId held on the page, so it survives the 5 s refresh that replaces every row record. Both rows are @key'd by instance id so the panel — which binds its DriverInstanceId once in OnInitializedAsync — can never be diffed onto a different driver. The panel's Enabled parameter is fed from real config rather than defaulted: HostsDriverInstanceInfo/HostsDriverRow now carry DriverInstance.Enabled, which LoadConfigAsync already had in hand. A snapshot with no matching configured instance is treated as enabled — the row exists only because live health arrived, so "Disabled — not deployed" would contradict the same row. The expander is visible to every AuthenticatedRead viewer; the panel self-gates only its Reconnect/Restart buttons on DriverOperator. A source-scan pin (HostsPanelWiringTests) asserts /hosts still renders the panel and still binds it to the row's own driver, cluster and enabled state — removing the tag would silently restore the orphaned state. Claude-Session: https://claude.ai/code/session_014WNM4vjoVksyyBraTXSZE1 --- .../Components/Pages/Hosts.razor | 58 ++++++++++++++++-- .../Hosts/HostsDriverView.cs | 18 ++++-- .../Hosts/HostsDriverViewTests.cs | 37 +++++++++++- .../HostsPanelWiringTests.cs | 59 +++++++++++++++++++ 4 files changed, 162 insertions(+), 10 deletions(-) create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/HostsPanelWiringTests.cs diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Hosts.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Hosts.razor index 80ca1f26..ed5f652b 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Hosts.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Hosts.razor @@ -11,6 +11,7 @@ @using Microsoft.EntityFrameworkCore @using Microsoft.Extensions.Logging @using ZB.MOM.WW.OtOpcUa.Configuration +@using ZB.MOM.WW.OtOpcUa.AdminUI.Components.Shared.Drivers @using ZB.MOM.WW.OtOpcUa.AdminUI.Hosts @using ZB.MOM.WW.OtOpcUa.AdminUI.Hubs @using ZB.MOM.WW.OtOpcUa.Commons.Messages.Drivers @@ -173,20 +174,25 @@ else Last read Errors/5 min Last error + @if (g.Drivers.Count == 0) { - No drivers. + No drivers. } else { @foreach (var d in g.Drivers) { - + @* Keyed by instance id so the detail row's DriverStatusPanel is never + diffed onto a DIFFERENT driver: the panel resolves DriverInstanceId + once in OnInitializedAsync (store subscription + resilience prime), so + a reused instance would keep reporting the driver it first bound. *@ + @(d.Name ?? d.DriverInstanceId) @if (d.Name is not null) @@ -258,7 +264,33 @@ else @(d.LastSuccessfulReadUtc?.ToString("HH:mm:ss 'UTC'") ?? "—") @d.ErrorCount5Min @(d.LastError ?? "—") + + @* Per-row expander for the driver's status panel (Gitea #546). Shown to + every AuthenticatedRead viewer — the panel carries read-only detail + (resilience/breaker state, last error) as well as the operator actions, + and self-gates only the Reconnect/Restart buttons on DriverOperator. *@ + + + @if (IsExpanded(d.DriverInstanceId)) + { + @* Full-width detail row beneath its driver. Rendered only while expanded so + the table stays scannable and only the opened panels hold store + subscriptions — collapsing disposes the panel and unsubscribes. *@ + + + + + + } } } @@ -281,6 +313,11 @@ else private List _nodes = new(); private List _instances = new(); + // Driver instances whose DriverStatusPanel is expanded (Gitea #546). Held on the component, NOT + // on the row records, so it survives the 5 s timer's RebuildDriverGroups — which replaces every + // HostsDriverRow instance and would otherwise collapse a panel an operator just opened. + private readonly HashSet _expandedDrivers = new(StringComparer.OrdinalIgnoreCase); + protected override async Task OnInitializedAsync() { Refresh(); @@ -336,7 +373,7 @@ else .Select(n => new HostsNodeInfo(n.ClusterId, n.NodeId, n.Host, n.OpcUaPort)) .ToListAsync(); _instances = await db.DriverInstances.AsNoTracking() - .Select(d => new HostsDriverInstanceInfo(d.DriverInstanceId, d.ClusterId, d.Name, d.DriverType)) + .Select(d => new HostsDriverInstanceInfo(d.DriverInstanceId, d.ClusterId, d.Name, d.DriverType, d.Enabled)) .ToListAsync(); } catch (Exception ex) @@ -351,6 +388,17 @@ else private void RebuildDriverGroups() => _driverGroups = HostsDriverView.Build(DriverStore.GetAll(), _nodes, _instances); + // Whether this driver's DriverStatusPanel is currently expanded. + private bool IsExpanded(string driverInstanceId) => _expandedDrivers.Contains(driverInstanceId); + + // Open/close one driver's panel. Ids are globally unique, so no cluster qualifier is needed; a + // removed driver simply leaves a stale id in the set, which matches nothing and costs nothing. + private void ToggleExpanded(string driverInstanceId) + { + if (!_expandedDrivers.Remove(driverInstanceId)) + _expandedDrivers.Add(driverInstanceId); + } + // Raised by the snapshot store on the bridge actor's thread for every driver instance. Rebuild // from the cached config (cheap — no DB hit) and marshal onto the render sync context. A // brand-new driver shows by id until the next config reload (timer/manual Refresh) enriches it. @@ -395,7 +443,9 @@ else _ => "chip-idle", }; - // Map DriverState string → chip CSS class (mirrors DriverStatusPanel.ChipClass). + // Map DriverState string → chip CSS class (mirrors DriverStatusPanel.ChipClass). Since #546 that + // panel renders inside this page's expanded rows, so the two mappings are visible side by side — + // they must stay in step or one driver shows two different colours for one state. private static string DriverChipClass(string? state) => state switch { "Healthy" => "chip-ok", diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hosts/HostsDriverView.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hosts/HostsDriverView.cs index 62e752d5..70c0f38b 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hosts/HostsDriverView.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hosts/HostsDriverView.cs @@ -22,7 +22,11 @@ public sealed record HostsNodeInfo(string ClusterId, string NodeId, string Host, /// Cluster the instance is configured under. /// Authored display name. /// Driver type string (Modbus, S7, Galaxy, ...). -public sealed record HostsDriverInstanceInfo(string DriverInstanceId, string ClusterId, string Name, string DriverType); +/// Whether the instance is authored enabled — forwarded to the row's +/// DriverStatusPanel, which renders a "Disabled — not deployed" notice and opens no +/// subscription when false. +public sealed record HostsDriverInstanceInfo( + string DriverInstanceId, string ClusterId, string Name, string DriverType, bool Enabled = true); /// /// One row in the /hosts "Driver Instances" table: a live health snapshot enriched with @@ -48,12 +52,17 @@ public sealed record HostsDriverInstanceInfo(string DriverInstanceId, string Clu /// is no answer — the driver cannot report skips, or it faulted before building a tag table. Lets /// "ignoring 20 of 22 tags" show even while every other column on the row — state, last read, error /// count, last error — reads perfectly healthy (Gitea #541). +/// Whether the matching configured instance is authored enabled; +/// when no instance matched. Fed to the row's expandable DriverStatusPanel (Gitea #546). An +/// unmatched row defaults to enabled on purpose: the row only exists because a live snapshot arrived, +/// so telling the operator it is "not deployed" would contradict the evidence in front of them. public sealed record HostsDriverRow( string DriverInstanceId, string? Name, string? DriverType, string State, DateTime? LastSuccessfulReadUtc, string? LastError, int ErrorCount5Min, DateTime PublishedUtc, DateTime? RediscoveryNeededUtc = null, string? RediscoveryReason = null, IReadOnlyList? HostStatuses = null, - SkippedTagReport? SkippedTags = null) + SkippedTagReport? SkippedTags = null, + bool Enabled = true) { /// Whether this driver is ignoring at least one authored tag — the condition the whole /// column exists to make visible. @@ -114,7 +123,7 @@ public static class HostsDriverView /// seen across and , ordered /// case-insensitively. Each group carries its nodes (ordered by node id) and its driver rows /// (snapshots enriched by joining on driver instance id; - /// unmatched ids yield null name/type), ordered by name-then-instance-id. + /// unmatched ids yield null name/type and an enabled row), ordered by name-then-instance-id. /// /// Live health snapshots; null treated as empty. /// Configured cluster nodes; null treated as empty. @@ -167,7 +176,8 @@ public static class HostsDriverView s.RediscoveryNeededUtc, s.RediscoveryReason, s.HostStatuses, - s.SkippedTags); + s.SkippedTags, + instance?.Enabled ?? true); }) .OrderBy(d => d.Name ?? d.DriverInstanceId, StringComparer.OrdinalIgnoreCase) .ThenBy(d => d.DriverInstanceId, StringComparer.OrdinalIgnoreCase) diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Hosts/HostsDriverViewTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Hosts/HostsDriverViewTests.cs index ea6d5e89..208eb998 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Hosts/HostsDriverViewTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Hosts/HostsDriverViewTests.cs @@ -22,8 +22,9 @@ public sealed class HostsDriverViewTests private static HostsNodeInfo Node(string cluster, string node, string host = "h", int port = 4840) => new(cluster, node, host, port); - private static HostsDriverInstanceInfo Instance(string instance, string cluster, string name, string type) => - new(instance, cluster, name, type); + private static HostsDriverInstanceInfo Instance( + string instance, string cluster, string name, string type, bool enabled = true) => + new(instance, cluster, name, type, enabled); [Fact] public void Build_groups_two_clusters_with_their_nodes_and_snapshots() @@ -214,6 +215,38 @@ public sealed class HostsDriverViewTests row.DegradedHosts.ShouldBeEmpty(); } + // ---- authored enabled-state (Gitea #546) ---- + + /// + /// The authored enabled flag reaches the row, which forwards it to the row's + /// DriverStatusPanel. Defaulting it to true would have the panel open a live + /// subscription for an instance that was deliberately taken out of the deployment. + /// + [Fact] + public void Build_carries_the_authored_enabled_flag_onto_the_row() + { + var groups = HostsDriverView.Build( + [Snap("MAIN", "drv-a")], + nodes: null, + [Instance("drv-a", "MAIN", "Pump House Modbus", "Modbus", enabled: false)]); + + groups.Single().Drivers.Single().Enabled.ShouldBeFalse(); + } + + /// + /// A snapshot with no matching configured instance is treated as enabled. The row only exists + /// because live health arrived for it, so rendering the panel's "Disabled — not deployed" + /// notice would contradict the evidence on the same row. + /// + [Fact] + public void An_unmatched_driver_row_is_treated_as_enabled() + { + var row = HostsDriverView.Build([Snap("MAIN", "orphan")], nodes: null, instances: null) + .Single().Drivers.Single(); + + row.Enabled.ShouldBeTrue(); + } + // ---- skipped tags (Gitea #541) ---- /// diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/HostsPanelWiringTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/HostsPanelWiringTests.cs new file mode 100644 index 00000000..ad533fe8 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/HostsPanelWiringTests.cs @@ -0,0 +1,59 @@ +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests; + +/// +/// Pins DriverStatusPanel to the /hosts page (Gitea #546). The panel is the only +/// component carrying the Restart/Reconnect driver controls, and it had ZERO references +/// repo-wide — the commands worked end to end over the mesh transport while no user could reach +/// them, and nothing failed to build, test or deploy. Deleting the last <DriverStatusPanel +/// tag would silently restore exactly that state, so it is pinned here. +/// +/// +/// This is a source scan rather than a rendering assertion because the repo has no bUnit (same +/// technique as MeshRegistrationRoleScopingTests; panel markup is otherwise live-verified). +/// A scan proves reachability of the component, not that it renders correctly. +/// +[Trait("Category", "Unit")] +public sealed class HostsPanelWiringTests +{ + private static string ReadHostsPageSource() + { + var dir = new DirectoryInfo(AppContext.BaseDirectory); + while (dir is not null && !File.Exists(Path.Combine(dir.FullName, "ZB.MOM.WW.OtOpcUa.slnx"))) + dir = dir.Parent; + dir.ShouldNotBeNull("could not locate the repository root (no ZB.MOM.WW.OtOpcUa.slnx above the test binary)"); + + return File.ReadAllText(Path.Combine( + dir!.FullName, "src", "Server", "ZB.MOM.WW.OtOpcUa.AdminUI", + "Components", "Pages", "Hosts.razor")); + } + + /// THE #546 regression pin: the panel must remain reachable from a routable page. + [Fact] + public void Hosts_page_renders_the_driver_status_panel() + { + ReadHostsPageSource().ShouldContain( + " + /// The panel must be fed the row's real identity and enabled-state, not literals. Hard-coding + /// any of the three would make every expanded row drive the same driver, or claim a disabled + /// instance is live — both of which a source scan for the tag alone would pass. + /// + [Fact] + public void The_panel_is_bound_to_the_rows_own_driver_cluster_and_enabled_state() + { + var source = ReadHostsPageSource(); + + source.ShouldContain("DriverInstanceId=\"@d.DriverInstanceId\""); + source.ShouldContain("ClusterId=\"@g.ClusterId\""); + source.ShouldContain("Enabled=\"@d.Enabled\""); + } +} From e669fb349ac11fdca26b7271b557193c276166a7 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 6 Aug 2026 17:25:41 -0400 Subject: [PATCH 2/2] fix(adminui): #546 recycle DriverStatusPanel on Enabled flip via @key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DriverStatusPanel decides whether to subscribe exactly once, in OnInitializedAsync (`if (!Enabled) return;`), and exposes no OnParametersSet hook. The detail row was keyed only by DriverInstanceId, so a live config edit that flipped a row's Enabled true->false — surfaced on the page by the 5 s LoadConfigAsync refresh — reused the same panel instance: the markup correctly switched to "Disabled - not deployed" while the panel's two store SnapshotChanged subscriptions and its 5 s staleness timer stayed wired underneath, until the operator happened to collapse the row. Including the flag in the detail row's key recycles the component instead. Both directions verified against the panel's own lifecycle: true->false disposes the old instance (unsubscribing both stores, draining both timers) and mounts one that skips subscription setup entirely; false->true disposes an instance that never subscribed (the unsubscribes are no-ops, both timers null) and mounts one that subscribes and primes from the stores. The data row's key is unchanged. No behaviour change when Enabled is stable. Claude-Session: https://claude.ai/code/session_014WNM4vjoVksyyBraTXSZE1 --- .../Components/Pages/Hosts.razor | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Hosts.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Hosts.razor index ed5f652b..be387ea8 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Hosts.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Hosts.razor @@ -189,7 +189,7 @@ else @foreach (var d in g.Drivers) { @* Keyed by instance id so the detail row's DriverStatusPanel is never - diffed onto a DIFFERENT driver: the panel resolves DriverInstanceId + diffed onto a DIFFERENT driver: the panel binds DriverInstanceId once in OnInitializedAsync (store subscription + resilience prime), so a reused instance would keep reporting the driver it first bound. *@ @@ -282,8 +282,17 @@ else { @* Full-width detail row beneath its driver. Rendered only while expanded so the table stays scannable and only the opened panels hold store - subscriptions — collapsing disposes the panel and unsubscribes. *@ - + subscriptions — collapsing disposes the panel and unsubscribes. + + Enabled participates in the key on purpose. The panel decides whether to + subscribe ONCE, in OnInitializedAsync (`if (!Enabled) return;`), and has no + OnParametersSet hook — so a live config edit that flips this row's Enabled + (surfaced by the 5 s LoadConfigAsync refresh) would re-render the panel's + "Disabled — not deployed" markup while its two store subscriptions and its + staleness timer stayed wired underneath. Keying on the flag recycles the + component instead: the old instance is disposed (unsubscribing both stores, + draining both timers) and a fresh one mounts against the new value. *@ +