feat(deploy): surface connection-level changes in the deployment diff (#10)

ComputeConnectionsDiff existed with tests but was never called and ConfigurationDiff had no slot for it, so standalone connection endpoint/protocol/failover drift never appeared in the deployment diff (only per-attribute binding drift did). Add a ConnectionChanges slot, wire ComputeConnectionsDiff into ComputeDiff, and render the connection section in the deployment diff UI.
2026-06-15 13:36:40 -04:00
parent 41d828e38e
commit e9a84ba220
5 changed files with 336 additions and 8 deletions
@@ -864,12 +864,137 @@
                    ? "The deployed revision hash differs from the current template-derived hash. Redeploy to apply changes."
                    : "No differences between deployed and current configuration.");
                builder.CloseElement();
+
+                // DeploymentManager-018: render the structured diff sections so
+                // the operator sees WHAT changed, not just that the hash moved.
+                // Each section uses the same compact change-table idiom; the
+                // connection section surfaces standalone endpoint/protocol/
+                // failover drift that no per-attribute row would show (#10).
+                var d = diffResult.Diff;
+                if (d != null)
+                {
+                    RenderChangeSection(builder, 100_000, "Attributes", d.AttributeChanges,
+                        a => a.Value ?? "—");
+
+                    RenderChangeSection(builder, 200_000, "Alarms", d.AlarmChanges,
+                        a => $"P{a.PriorityLevel} · {a.TriggerType}");
+
+                    RenderChangeSection(builder, 300_000, "Scripts", d.ScriptChanges,
+                        s => s.TriggerType ?? "—");
+
+                    RenderChangeSection(builder, 400_000, "Connections", d.ConnectionChanges,
+                        c => FormatConnection(c));
+                }
            }
        };

        await _diffDialog.ShowAsync($"Deployment Diff — {inst.UniqueName}", body);
    }

+    // Compact summary of a connection's deployment-relevant fields for the diff
+    // table's Before/After cells: protocol, primary endpoint config, and the
+    // failover retry count. Mirrors the fields ConnectionsEqual compares.
+    private static string FormatConnection(
+        ZB.MOM.WW.ScadaBridge.Commons.Types.Flattening.ConnectionConfig c)
+    {
+        var endpoint = string.IsNullOrWhiteSpace(c.ConfigurationJson) ? "—" : c.ConfigurationJson;
+        return $"{c.Protocol} · {endpoint} · failover ×{c.FailoverRetryCount}";
+    }
+
+    // Renders one change section (a heading plus a Bootstrap change-table) for a
+    // set of diff entries, matching the deployment-diff idiom used elsewhere in
+    // the UI: table-sm/table-striped, a colored change badge, and Before/After
+    // text columns. Nothing is rendered when the section has no entries, so the
+    // four sections (attributes, alarms, scripts, connections) all read the same
+    // and only appear when they actually changed. seqBase values are spaced
+    // 100k apart so each section's per-row sequence numbers (13 per row) stay in
+    // a disjoint, ascending range no matter how many entries a section has.
+    private static void RenderChangeSection<T>(
+        Microsoft.AspNetCore.Components.Rendering.RenderTreeBuilder builder,
+        int seqBase,
+        string heading,
+        IReadOnlyList<ZB.MOM.WW.ScadaBridge.Commons.Types.Flattening.DiffEntry<T>> entries,
+        Func<T, string> summarize)
+    {
+        if (entries.Count == 0)
+            return;
+
+        builder.OpenElement(seqBase, "div");
+        builder.AddAttribute(seqBase + 1, "class", "mt-3");
+
+        builder.OpenElement(seqBase + 2, "div");
+        builder.AddAttribute(seqBase + 3, "class", "fw-semibold small mb-1");
+        builder.AddContent(seqBase + 4, $"{heading} ({entries.Count})");
+        builder.CloseElement();
+
+        builder.OpenElement(seqBase + 5, "table");
+        builder.AddAttribute(seqBase + 6, "class", "table table-sm table-striped align-middle mb-0");
+
+        // Header row.
+        builder.OpenElement(seqBase + 7, "thead");
+        builder.OpenElement(seqBase + 8, "tr");
+        AppendHeaderCell(builder, seqBase + 9, "Name");
+        AppendHeaderCell(builder, seqBase + 12, "Change");
+        AppendHeaderCell(builder, seqBase + 15, "Before");
+        AppendHeaderCell(builder, seqBase + 18, "After");
+        builder.CloseElement(); // tr
+        builder.CloseElement(); // thead
+
+        builder.OpenElement(seqBase + 21, "tbody");
+        var rowSeq = seqBase + 22;
+        foreach (var entry in entries)
+        {
+            builder.OpenElement(rowSeq, "tr");
+
+            builder.OpenElement(rowSeq + 1, "td");
+            builder.AddContent(rowSeq + 2, entry.CanonicalName);
+            builder.CloseElement();
+
+            builder.OpenElement(rowSeq + 3, "td");
+            builder.OpenElement(rowSeq + 4, "span");
+            builder.AddAttribute(rowSeq + 5, "class", ChangeBadgeClass(entry.ChangeType));
+            builder.AddContent(rowSeq + 6, entry.ChangeType.ToString());
+            builder.CloseElement();
+            builder.CloseElement();
+
+            builder.OpenElement(rowSeq + 7, "td");
+            builder.AddAttribute(rowSeq + 8, "class", "small text-muted");
+            builder.AddContent(rowSeq + 9,
+                entry.OldValue is null ? "—" : summarize(entry.OldValue));
+            builder.CloseElement();
+
+            builder.OpenElement(rowSeq + 10, "td");
+            builder.AddAttribute(rowSeq + 11, "class", "small");
+            builder.AddContent(rowSeq + 12,
+                entry.NewValue is null ? "—" : summarize(entry.NewValue));
+            builder.CloseElement();
+
+            builder.CloseElement(); // tr
+            rowSeq += 13;
+        }
+        builder.CloseElement(); // tbody
+
+        builder.CloseElement(); // table
+        builder.CloseElement(); // div.mt-3
+    }
+
+    private static void AppendHeaderCell(
+        Microsoft.AspNetCore.Components.Rendering.RenderTreeBuilder builder, int seq, string text)
+    {
+        builder.OpenElement(seq, "th");
+        builder.AddAttribute(seq + 1, "scope", "col");
+        builder.AddContent(seq + 2, text);
+        builder.CloseElement();
+    }
+
+    private static string ChangeBadgeClass(
+        ZB.MOM.WW.ScadaBridge.Commons.Types.Flattening.DiffChangeType changeType) => changeType switch
+        {
+            ZB.MOM.WW.ScadaBridge.Commons.Types.Flattening.DiffChangeType.Added => "badge bg-success",
+            ZB.MOM.WW.ScadaBridge.Commons.Types.Flattening.DiffChangeType.Removed => "badge bg-danger",
+            _ => "badge bg-warning text-dark",
+        };
+
    // ---- Dropdown option helpers ----
    private IEnumerable<(int Id, string Label)> EnumerateSiteOptions()
    {
@@ -12,8 +12,8 @@ public sealed record ConfigurationDiff
    public string? OldRevisionHash { get; init; }
    /// <summary>Revision hash of the new configuration being compared.</summary>
    public string? NewRevisionHash { get; init; }
-    /// <summary>True when any attribute, alarm, or script changes are present.</summary>
-    public bool HasChanges => AttributeChanges.Count > 0 || AlarmChanges.Count > 0 || ScriptChanges.Count > 0;
+    /// <summary>True when any attribute, alarm, script, or connection changes are present.</summary>
+    public bool HasChanges => AttributeChanges.Count > 0 || AlarmChanges.Count > 0 || ScriptChanges.Count > 0 || ConnectionChanges.Count > 0;

    /// <summary>Diff entries for resolved attributes.</summary>
    public IReadOnlyList<DiffEntry<ResolvedAttribute>> AttributeChanges { get; init; } = [];
@@ -21,6 +21,13 @@ public sealed record ConfigurationDiff
    public IReadOnlyList<DiffEntry<ResolvedAlarm>> AlarmChanges { get; init; } = [];
    /// <summary>Diff entries for resolved scripts.</summary>
    public IReadOnlyList<DiffEntry<ResolvedScript>> ScriptChanges { get; init; } = [];
+
+    /// <summary>
+    /// Diff entries for connection configurations, keyed by connection name.
+    /// Surfaces standalone endpoint/protocol/failover drift that does not show
+    /// up as a per-attribute binding change (TemplateEngine-018).
+    /// </summary>
+    public IReadOnlyList<DiffEntry<ConnectionConfig>> ConnectionChanges { get; init; } = [];
 }

 /// <summary>
@@ -42,6 +42,13 @@ public class DiffService
            s => s.CanonicalName,
            ScriptsEqual);

+        // TemplateEngine-018: surface standalone connection endpoint/protocol/
+        // failover drift. Per-attribute binding changes already show up under
+        // AttributeChanges, but a connection's own ConfigurationJson /
+        // BackupConfigurationJson / Protocol / FailoverRetryCount edits do not —
+        // those only appear here.
+        var connectionChanges = ComputeConnectionsDiff(oldConfig, newConfig);
+
        return new ConfigurationDiff
        {
            InstanceUniqueName = newConfig.InstanceUniqueName,
@@ -49,7 +56,8 @@ public class DiffService
            NewRevisionHash = newRevisionHash,
            AttributeChanges = attributeChanges,
            AlarmChanges = alarmChanges,
-            ScriptChanges = scriptChanges
+            ScriptChanges = scriptChanges,
+            ConnectionChanges = connectionChanges
        };
    }

@@ -159,11 +167,10 @@ public class DiffService
    /// TemplateEngine-018: produces a per-connection diff between two flattened
    /// configurations, emitting Added / Removed / Changed entries keyed by the
    /// connection name. Mirrors the existing <see cref="ComputeEntityDiff{T}"/>
-    /// shape used for attributes / alarms / scripts but is exposed as a separate
-    /// method because <see cref="ConfigurationDiff"/> in
-    /// <c>ZB.MOM.WW.ScadaBridge.Commons</c> does not yet carry a <c>ConnectionChanges</c>
-    /// slot — the public diff record will be extended in a paired Commons change
-    /// (this file is the only one in this fix's scope). A null
+    /// shape used for attributes / alarms / scripts. Called by
+    /// <see cref="ComputeDiff"/> to populate
+    /// <see cref="ConfigurationDiff.ConnectionChanges"/>, and exposed publicly so
+    /// callers can compute connection drift in isolation. A null
    /// <c>Connections</c> dictionary on either side is treated as the empty map.
    /// </summary>
    /// <param name="oldConfig">The previously deployed configuration, or null
@@ -1,4 +1,5 @@
 using System.Security.Claims;
+using System.Text.Json;
 using ZB.MOM.WW.ScadaBridge.Security;
 using Bunit;
 using Microsoft.AspNetCore.Components.Authorization;
@@ -12,7 +13,10 @@ using ZB.MOM.WW.ScadaBridge.Commons.Entities.Sites;
 using ZB.MOM.WW.ScadaBridge.Commons.Entities.Templates;
 using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories;
 using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services;
+using ZB.MOM.WW.ScadaBridge.Commons.Types;
 using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums;
+using ZB.MOM.WW.ScadaBridge.Commons.Types.Flattening;
+using ZB.MOM.WW.ScadaBridge.Commons.Entities.Deployment;
 using ZB.MOM.WW.ScadaBridge.Communication;
 using ZB.MOM.WW.ScadaBridge.DeploymentManager;
 using ZB.MOM.WW.ScadaBridge.CentralUI.Components.Shared;
@@ -292,6 +296,90 @@ public class TopologyPageTests : BunitContext
        Assert.Throws<Bunit.MissingEventHandlerException>(() => instanceLabel.DoubleClick());
    }

+    [Fact]
+    public void Diff_ConnectionEndpointChange_RendersConnectionSection()
+    {
+        // TemplateEngine-018 / DeploymentManager-018: a standalone connection
+        // endpoint edit (no per-attribute binding change) must surface in the
+        // deployment-diff modal. Before ConnectionChanges was wired through
+        // ComputeDiff + the UI, this redeploy showed only the stale-hash badge
+        // with no indication that the connection endpoint had moved.
+        // The DiffDialog body-scroll lock + focus call out to JS interop on
+        // open; loose mode no-ops the handlers we don't explicitly set up.
+        JSInterop.Mode = JSRuntimeMode.Loose;
+
+        var areasBySite = new Dictionary<int, IReadOnlyList<Area>>
+        {
+            [1] = new List<Area> { new("Line-1") { Id = 10, SiteId = 1 } }
+        };
+        SeedRepos(
+            sites: new[] { new Site("Plant-A", "plant-a") { Id = 1 } },
+            instances: new[]
+            {
+                new Instance("Pump-001") { Id = 100, SiteId = 1, AreaId = 10, State = InstanceState.Enabled }
+            },
+            areasBySite: areasBySite);
+
+        // Deployed snapshot: connection "plc1" points at host-a.
+        var deployedConfig = new FlattenedConfiguration
+        {
+            InstanceUniqueName = "Pump-001",
+            Connections = new Dictionary<string, ConnectionConfig>
+            {
+                ["plc1"] = new ConnectionConfig
+                {
+                    Protocol = "OpcUa",
+                    ConfigurationJson = "{\"endpoint\":\"opc.tcp://host-a:4840\"}",
+                    FailoverRetryCount = 3,
+                }
+            }
+        };
+        _deployRepo.GetDeployedSnapshotByInstanceIdAsync(100, Arg.Any<CancellationToken>())
+            .Returns(Task.FromResult<DeployedConfigSnapshot?>(
+                new DeployedConfigSnapshot("dep-1", "hash-old",
+                    JsonSerializer.Serialize(deployedConfig))));
+
+        // Current template-derived config: same connection now points at host-b.
+        var currentConfig = new FlattenedConfiguration
+        {
+            InstanceUniqueName = "Pump-001",
+            Connections = new Dictionary<string, ConnectionConfig>
+            {
+                ["plc1"] = new ConnectionConfig
+                {
+                    Protocol = "OpcUa",
+                    ConfigurationJson = "{\"endpoint\":\"opc.tcp://host-b:4840\"}",
+                    FailoverRetryCount = 3,
+                }
+            }
+        };
+        _pipeline.FlattenAndValidateAsync(100, Arg.Any<CancellationToken>())
+            .Returns(Task.FromResult(Result<FlatteningPipelineResult>.Success(
+                new FlatteningPipelineResult(currentConfig, "hash-new", ValidationResult.Success()))));
+
+        var cut = Render<TopologyPage>();
+        FindToggleForLabel(cut, "Plant-A")!.Click();
+        FindToggleForLabel(cut, "Line-1")!.Click();
+
+        // The per-node action menu only renders after a context-menu (right
+        // click) on the instance row, so open it first, then click "Diff".
+        var instanceRow = cut.FindAll(".tv-row")
+            .First(row => row.QuerySelector(".tv-label")?.TextContent == "Pump-001");
+        instanceRow.ContextMenu();
+
+        var diffButton = cut.FindAll("button.dropdown-item")
+            .First(b => b.TextContent.Trim() == "Diff");
+        diffButton.Click();
+
+        var markup = cut.Markup;
+        Assert.Contains("Connections", markup);
+        Assert.Contains("plc1", markup);
+        Assert.Contains("host-a", markup);
+        Assert.Contains("host-b", markup);
+        // The change is a modification, so the row carries the "Changed" badge.
+        Assert.Contains("Changed", markup);
+    }
+
    [Fact]
    public void LegacyInstancesRoute_IsDeclaredOnTopologyPage()
    {
@@ -369,4 +369,105 @@ public class DiffServiceTests

        Assert.Empty(diff);
    }
+
+    // ── TemplateEngine-018: ComputeDiff wires ComputeConnectionsDiff into the
+    // public ConfigurationDiff.ConnectionChanges slot so standalone connection
+    // protocol/endpoint/failover drift surfaces in the deployment diff (#10). ──
+
+    [Fact]
+    public void ComputeDiff_ConnectionProtocolAndEndpointAndFailoverChange_PopulatesConnectionChanges()
+    {
+        // Protocol, endpoint config JSON, and failover retry count all differ
+        // on the same connection. Before this wiring, ComputeDiff dropped the
+        // entire connection dimension so this redeploy showed "no changes".
+        var oldConfig = new FlattenedConfiguration
+        {
+            InstanceUniqueName = "Instance1",
+            Connections = new Dictionary<string, ConnectionConfig>
+            {
+                ["plc1"] = new ConnectionConfig
+                {
+                    Protocol = "OpcUa",
+                    ConfigurationJson = "{\"endpoint\":\"opc.tcp://host-a:4840\"}",
+                    FailoverRetryCount = 3,
+                }
+            }
+        };
+        var newConfig = new FlattenedConfiguration
+        {
+            InstanceUniqueName = "Instance1",
+            Connections = new Dictionary<string, ConnectionConfig>
+            {
+                ["plc1"] = new ConnectionConfig
+                {
+                    Protocol = "Modbus",
+                    ConfigurationJson = "{\"endpoint\":\"opc.tcp://host-b:4840\"}",
+                    FailoverRetryCount = 5,
+                }
+            }
+        };
+
+        var diff = _sut.ComputeDiff(oldConfig, newConfig);
+
+        Assert.True(diff.HasChanges);
+        Assert.Single(diff.ConnectionChanges);
+        var entry = diff.ConnectionChanges[0];
+        Assert.Equal("plc1", entry.CanonicalName);
+        Assert.Equal(DiffChangeType.Changed, entry.ChangeType);
+        Assert.Equal("OpcUa", entry.OldValue!.Protocol);
+        Assert.Equal("Modbus", entry.NewValue!.Protocol);
+        Assert.Contains("host-a", entry.OldValue!.ConfigurationJson);
+        Assert.Contains("host-b", entry.NewValue!.ConfigurationJson);
+        Assert.Equal(3, entry.OldValue!.FailoverRetryCount);
+        Assert.Equal(5, entry.NewValue!.FailoverRetryCount);
+    }
+
+    [Fact]
+    public void ComputeDiff_OnlyConnectionDiffers_HasChangesIsTrue()
+    {
+        // Attributes, alarms, and scripts are identical; only a connection's
+        // endpoint changed. HasChanges must be true so the diff view does not
+        // claim "no differences" while a connection endpoint silently moved.
+        var attributes = new List<ResolvedAttribute>
+        {
+            new ResolvedAttribute { CanonicalName = "Temp", Value = "25", DataType = "Double" }
+        };
+        var oldConfig = new FlattenedConfiguration
+        {
+            InstanceUniqueName = "Instance1",
+            Attributes = attributes,
+            Connections = new Dictionary<string, ConnectionConfig>
+            {
+                ["plc1"] = new ConnectionConfig
+                {
+                    Protocol = "OpcUa",
+                    ConfigurationJson = "{\"endpoint\":\"opc.tcp://host-a:4840\"}",
+                    FailoverRetryCount = 3,
+                }
+            }
+        };
+        var newConfig = new FlattenedConfiguration
+        {
+            InstanceUniqueName = "Instance1",
+            Attributes = attributes,
+            Connections = new Dictionary<string, ConnectionConfig>
+            {
+                ["plc1"] = new ConnectionConfig
+                {
+                    Protocol = "OpcUa",
+                    ConfigurationJson = "{\"endpoint\":\"opc.tcp://host-b:4840\"}",
+                    FailoverRetryCount = 3,
+                }
+            }
+        };
+
+        var diff = _sut.ComputeDiff(oldConfig, newConfig);
+
+        Assert.True(diff.HasChanges);
+        Assert.Empty(diff.AttributeChanges);
+        Assert.Empty(diff.AlarmChanges);
+        Assert.Empty(diff.ScriptChanges);
+        Assert.Single(diff.ConnectionChanges);
+        Assert.Equal(DiffChangeType.Changed, diff.ConnectionChanges[0].ChangeType);
+    }
 }