fix(transport-ui): reset connection choices on site change + clear state on back (M8 E2 review)

Fix 1: OnSiteChoiceChangedAsync now resets _connectionChoices for every RequiredConnectionMapping under the changed source site after loading the new target's connections. Choices are re-seeded to the same-named connection on the new target if present, or CreateNewValue otherwise — preventing BuildNameMap from emitting MapToExisting for a connection absent from the newly-chosen target. Fix 2: BackToUpload now calls ResetSessionState() before resetting _step so _session, _preview, _resolutions, _siteChoices, _connectionChoices, _targetSites, and _targetConnections are all cleared when the operator backs out to re-upload, making it safe to start a new import flow from a clean slate. Tests 12 + 13 added to TransportImportPageTests.
2026-06-18 07:39:19 -04:00
parent d45a7a5760
commit f881521cc9
2 changed files with 168 additions and 4 deletions
@@ -405,6 +405,11 @@ public partial class TransportImport : ComponentBase, IDisposable

    private void BackToUpload()
    {
+        // Reset all wizard + Map state so a subsequent re-upload starts from a
+        // clean slate and cannot observe stale session / preview / name-map data
+        // from the previous flow. ResetSessionState also clears _preview,
+        // _resolutions, _siteChoices, _connectionChoices, etc.
+        ResetSessionState();
        _step = ImportWizardStep.Upload;
        _errorMessage = null;
    }
@@ -509,10 +514,14 @@ public partial class TransportImport : ComponentBase, IDisposable
    }

    /// <summary>
-    /// Handles a site-mapping dropdown change. Records the choice and, when a
-    /// concrete target site is chosen, eagerly loads + caches that site's
-    /// connections so the per-connection dropdowns underneath it can render
-    /// their option sets synchronously.
+    /// Handles a site-mapping dropdown change. Records the choice, eagerly loads +
+    /// caches the new target's connections, then resets the per-connection choices
+    /// for every <see cref="RequiredConnectionMapping"/> under this source site so
+    /// that stale selections from the previous target cannot survive into
+    /// <see cref="BuildNameMap"/>. Each connection is re-seeded: if the new target
+    /// has a same-named connection the choice defaults to that name (matching the
+    /// auto-match logic in <see cref="InitNameMappingStateAsync"/>); otherwise it
+    /// falls back to the <see cref="CreateNewValue"/> sentinel ("Create new").
    /// </summary>
    private async Task OnSiteChoiceChangedAsync(string sourceSiteIdentifier, string? value)
    {
@@ -527,6 +536,26 @@ public partial class TransportImport : ComponentBase, IDisposable
                    await SiteRepo.GetDataConnectionsBySiteIdAsync(target.Id, CancellationToken.None);
            }
        }
+
+        // Reset connection choices for every RequiredConnectionMapping belonging
+        // to this source site. The previously-chosen connection name may not exist
+        // on the newly-chosen target site, and leaving it would cause BuildNameMap
+        // to emit a MapToExisting for a connection that does not exist there.
+        if (_preview is not null)
+        {
+            foreach (var rcm in _preview.RequiredConnectionMappings
+                .Where(m => m.SourceSiteIdentifier == sourceSiteIdentifier))
+            {
+                // Default to the new target's same-named connection when present;
+                // otherwise fall back to Create-new.
+                _connectionChoices[(sourceSiteIdentifier, rcm.SourceConnectionName)] =
+                    (!string.IsNullOrEmpty(chosen)
+                     && _targetConnections.TryGetValue(chosen, out var conns)
+                     && conns.Any(c => c.Name == rcm.SourceConnectionName))
+                        ? rcm.SourceConnectionName
+                        : CreateNewValue;
+            }
+        }
    }

    private void OnConnectionChoiceChanged(string sourceSiteIdentifier, string sourceConnectionName, string? value)
@@ -567,6 +567,127 @@ public class TransportImportPageTests : BunitContext
        Assert.Contains("-8", marker.TextContent);
    }

+    // ─────────────────────────────────────────────────────────────────────
+    // Test 12 (M8 E2 Fix 1): changing a source site's dropdown to a target
+    // that lacks the previously auto-matched connection name resets that
+    // connection choice to CreateNew (""), and BuildNameMap therefore emits
+    // a CreateNew entry — not a MapToExisting for an absent connection.
+    // ─────────────────────────────────────────────────────────────────────
+    [Fact]
+    public async Task OnSiteChoiceChanged_resets_stale_connection_choices_when_new_target_lacks_connection()
+    {
+        // Target environment: site-b has connection "opc-main"; site-c has NO connections.
+        var siteB = new Site("Site B", "site-b") { Id = 7 };
+        var siteC = new Site("Site C", "site-c") { Id = 8 };
+        _siteRepo.GetAllSitesAsync(Arg.Any<CancellationToken>())
+            .Returns(new List<Site> { siteB, siteC });
+        _siteRepo.GetDataConnectionsBySiteIdAsync(7, Arg.Any<CancellationToken>())
+            .Returns(new List<DataConnection> { new("opc-main", "OpcUa", 7) { Id = 11 } });
+        _siteRepo.GetDataConnectionsBySiteIdAsync(8, Arg.Any<CancellationToken>())
+            .Returns(Array.Empty<DataConnection>());
+
+        // Bundle: source site "site-a" auto-matches site-b; connection "opc-a" auto-matches "opc-main".
+        var session = BuildEncryptedSession(sourceEnv: "prod-cluster");
+        var preview = new ImportPreview(
+            session.SessionId,
+            new List<ImportPreviewItem>(),
+            new List<RequiredSiteMapping> { new("site-a", "Site A", "site-b") },
+            new List<RequiredConnectionMapping> { new("site-a", "opc-a", "opc-main") });
+        _importer.PreviewAsync(session.SessionId, Arg.Any<CancellationToken>()).Returns(preview);
+
+        var cut = Render<TransportImportPage>();
+        await cut.InvokeAsync(() => SetField(cut.Instance, "_session", session));
+        // Drive LoadPreviewAndAdvanceAsync to seed auto-match state (site-a → site-b, opc-a → opc-main).
+        await cut.InvokeAsync(async () =>
+            await InvokeAsyncMethod(cut.Instance, "LoadPreviewAndAdvanceAsync"));
+
+        // Verify the auto-match seed: connection choice is "opc-main".
+        var connChoicesBefore = GetField<Dictionary<(string, string), string>>(
+            cut.Instance, "_connectionChoices");
+        Assert.Equal("opc-main", connChoicesBefore[("site-a", "opc-a")]);
+
+        // Operator changes site-a's target from site-b → site-c (which has no connections).
+        await cut.InvokeAsync(async () =>
+        {
+            var method = cut.Instance.GetType().GetMethod(
+                "OnSiteChoiceChangedAsync",
+                System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic)!;
+            await (Task)method.Invoke(cut.Instance, new object[] { "site-a", "site-c" })!;
+        });
+
+        // Connection choice for (site-a, opc-a) must now be CreateNew ("").
+        var connChoicesAfter = GetField<Dictionary<(string, string), string>>(
+            cut.Instance, "_connectionChoices");
+        Assert.Equal(string.Empty, connChoicesAfter[("site-a", "opc-a")]);
+
+        // BuildNameMap must NOT emit MapToExisting for "opc-a" — it should emit CreateNew.
+        var nameMap = TransportImportPageTests.InvokeBuildNameMap(cut.Instance);
+        var connMapping = nameMap.Connections.Single(c => c.SourceConnectionName == "opc-a");
+        Assert.Equal(MappingAction.CreateNew, connMapping.Action);
+        Assert.Null(connMapping.TargetConnectionName);
+    }
+
+    // ─────────────────────────────────────────────────────────────────────
+    // Test 13 (M8 E2 Fix 2): BackToUpload clears session/preview/Map state
+    // so a subsequent re-upload flow cannot inherit stale data.
+    // ─────────────────────────────────────────────────────────────────────
+    [Fact]
+    public async Task BackToUpload_clears_session_and_preview_state()
+    {
+        // Seed the wizard at the Diff step with non-trivial session + preview + Map state.
+        var session = BuildEncryptedSession(sourceEnv: "prod-cluster");
+        var preview = new ImportPreview(
+            session.SessionId,
+            new List<ImportPreviewItem>
+            {
+                new("Template", "Pump", null, 1, ConflictKind.New, null, null),
+            },
+            new List<RequiredSiteMapping> { new("site-a", "Site A", null) },
+            new List<RequiredConnectionMapping> { new("site-a", "conn-x", null) });
+
+        var cut = Render<TransportImportPage>();
+        await cut.InvokeAsync(() =>
+        {
+            SetField(cut.Instance, "_session", session);
+            SetField(cut.Instance, "_preview", preview);
+            SetField(cut.Instance, "_resolutions",
+                new Dictionary<(string, string), ImportResolution>
+                {
+                    [("Template", "Pump")] = new("Template", "Pump", ResolutionAction.Add, null),
+                });
+            SetField(cut.Instance, "_siteChoices",
+                new Dictionary<string, string> { ["site-a"] = "site-b" });
+            SetField(cut.Instance, "_connectionChoices",
+                new Dictionary<(string, string), string> { [("site-a", "conn-x")] = "conn-y" });
+            SetField(cut.Instance, "_step", TransportImportPage.ImportWizardStep.Diff);
+        });
+
+        // Invoke BackToUpload.
+        await cut.InvokeAsync(() =>
+        {
+            var method = cut.Instance.GetType().GetMethod(
+                "BackToUpload",
+                System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic)!;
+            method.Invoke(cut.Instance, Array.Empty<object?>());
+        });
+
+        // Step must be back to Upload.
+        Assert.Equal(
+            TransportImportPage.ImportWizardStep.Upload,
+            GetField<TransportImportPage.ImportWizardStep>(cut.Instance, "_step"));
+
+        // Session, preview, resolutions, and Map state must all be cleared.
+        Assert.Null(GetField<object?>(cut.Instance, "_session"));
+        Assert.Null(GetField<object?>(cut.Instance, "_preview"));
+        Assert.Null(GetField<object?>(cut.Instance, "_resolutions"));
+
+        var siteChoices = GetField<Dictionary<string, string>>(cut.Instance, "_siteChoices");
+        Assert.Empty(siteChoices);
+
+        var connChoices = GetField<Dictionary<(string, string), string>>(cut.Instance, "_connectionChoices");
+        Assert.Empty(connChoices);
+    }
+
    // ─────────────────────────────────────────────────────────────────────
    // Reflection helpers — the wizard's per-instance state is private (the
    // razor partial pattern). We poke at it via reflection rather than
@@ -607,6 +728,20 @@ public class TransportImportPageTests : BunitContext
        await task;
    }

+    /// <summary>
+    /// Invokes the private <c>BuildNameMap()</c> method on the page instance and
+    /// returns the resulting <see cref="BundleNameMap"/>. Used by Test 12 to assert
+    /// the map shape after a site-choice change without going through ApplyAsync.
+    /// </summary>
+    private static BundleNameMap InvokeBuildNameMap(TransportImportPage instance)
+    {
+        var method = instance.GetType().GetMethod(
+            "BuildNameMap",
+            System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic)
+            ?? throw new InvalidOperationException("Method 'BuildNameMap' not found.");
+        return (BundleNameMap)method.Invoke(instance, Array.Empty<object?>())!;
+    }
+
    /// <summary>
    /// Seeds the wizard at Step 2 (Passphrase) with a staged bundle file — the
    /// shape after an encrypted-bundle upload completed Step 1's peek and