DYN-10569: Only warn on Python port removal when the port has custom properties (#17313)

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: koeylai-adsk

src/DynamoCoreWpf/Controls/DynamoNodeButton.cs

@@ -68,11 +68,15 @@ private void OnDynamoNodeButtonClick(object sender, RoutedEventArgs e)
             {
                 // Only show the prompt if it is a Python node
                 var nodeVM = (sender as DynamoNodeButton)?.DataContext as NodeViewModel;
-                if (nodeVM?.NodeModel is PythonNodeModels.PythonNode)
-                {                    
+                if (nodeVM?.NodeModel is PythonNodeModels.PythonNode pythonNode)
+                {
                     MessageBoxResult result = MessageBoxResult.None;
 
-                    if (eventName.Equals("RemoveInPort") && ShowWarningForRemovingInPort)
+                    // Removing an input port always removes the last one, so only warn when that
+                    // port carries custom properties that the user would actually lose. This also
+                    // suppresses the prompt when there is no port left to remove.
+                    if (eventName.Equals("RemoveInPort") && ShowWarningForRemovingInPort
+                        && pythonNode.HasCustomInputPortProperties(pythonNode.InPorts.Count - 1))
                     {
                         result = MessageBoxService.Show
                         (

src/Libraries/PythonNodeModels/PythonNode.cs

@@ -128,6 +128,35 @@ protected override string GetInputTooltip(int index)
             return "Input #" + index;
         }
 
+        /// <summary>
+        /// Returns true if the input port at <paramref name="index"/> has a name or tooltip that
+        /// differs from the auto-generated default for that index, i.e. the user renamed the port
+        /// or edited its description through the port context menu. Used to decide whether removing
+        /// the port would actually discard anything the user configured.
+        /// Returns false for an out-of-range index, so callers can pass the index of a port that
+        /// does not exist (for example when there are no input ports left to remove).
+        /// </summary>
+        /// <param name="index">Index of the input port to inspect.</param>
+        /// <returns>True when the port carries user-customized properties.</returns>
+        internal bool HasCustomInputPortProperties(int index)
+        {
+            if (index < 0 || index >= InPorts.Count)
+            {
+                return false;
+            }
+
+            // For PythonNode, port i is created with GetInputName(i)/GetInputTooltip(i), so any
+            // difference from those defaults is a user edit.
+            // This does NOT hold for PythonStringNode: it prepends a fixed "script" port and
+            // overrides GetInputIndex to subtract one, so its port i carries the defaults for
+            // i - 1 and EVERY untouched port would be reported as customized. That is inert only
+            // because the sole caller narrows to PythonNode; widening it to PythonNodeBase
+            // requires correcting the index mapping here first.
+            var port = InPorts[index];
+            return !string.Equals(port.Name, GetInputName(index), StringComparison.Ordinal)
+                || !string.Equals(port.ToolTip, GetInputTooltip(index), StringComparison.Ordinal);
+        }
+
         protected AssociativeNode CreateOutputAST(
             AssociativeNode codeInputNode, List<AssociativeNode> inputAstNodes,
             List<Tuple<string, AssociativeNode>> additionalBindings)

test/DynamoCoreWpfTests/PythonNodeRemovePortWarningTests.cs

@@ -0,0 +1,197 @@
+using System.Linq;
+using System.Windows;
+using System.Windows.Controls.Primitives;
+using Dynamo.Controls;
+using Dynamo.Models;
+using Dynamo.Nodes;
+using Dynamo.Utilities;
+using Dynamo.Wpf.Utilities;
+using DynamoCoreWpfTests.Utility;
+using Moq;
+using NUnit.Framework;
+using PythonNodeModels;
+
+namespace DynamoCoreWpfTests
+{
+    /// <summary>
+    /// Covers the "Remove Port?" warning raised by the '-' button on a Python node.
+    /// The warning must appear only when the port being removed carries custom properties
+    /// that the user would lose, rather than on every removal. See DYN-10569.
+    /// </summary>
+    /// <remarks>
+    /// Not tagged "UnitTests": each case starts a full DynamoView via DynamoTestUIBase and takes
+    /// minutes. The comparison logic itself is covered cheaply by the HasCustomInputPortProperties
+    /// tests in DynamoPythonTests; this fixture exists to verify the button actually consults it.
+    /// </remarks>
+    [Category("RegressionTests")]
+    public class PythonNodeRemovePortWarningTests : DynamoTestUIBase
+    {
+        private Mock<MessageBoxService.IMessageBox> dialogMock;
+
+        [TearDown]
+        public void ResetMessageBoxOverride()
+        {
+            // The override is a static field on MessageBoxService, so it would otherwise
+            // stay installed for every fixture that runs after this one.
+            MessageBoxService.OverrideMessageBoxDuringTests(null);
+            dialogMock = null;
+        }
+
+        /// <summary>
+        /// Installs a recording message box that answers <paramref name="answer"/> to any prompt,
+        /// so a warning does not block the test and can be asserted on afterwards.
+        /// </summary>
+        /// <param name="answer">The result the mocked dialog returns, defaulting to OK.</param>
+        private void InstallDialogMock(MessageBoxResult answer = MessageBoxResult.OK)
+        {
+            dialogMock = new Mock<MessageBoxService.IMessageBox>();
+            dialogMock
+                .Setup(m => m.Show(It.IsAny<Window>(), It.IsAny<string>(), It.IsAny<string>(),
+                    It.IsAny<MessageBoxButton>(), It.IsAny<MessageBoxImage>()))
+                .Returns(answer);
+
+            MessageBoxService.OverrideMessageBoxDuringTests(dialogMock.Object);
+        }
+
+        /// <summary>
+        /// Adds a Python node to the current workspace and returns its realized NodeView.
+        /// </summary>
+        private NodeView CreatePythonNodeView(out PythonNode pythonNode)
+        {
+            pythonNode = new PythonNode();
+            Model.ExecuteCommand(new DynamoModel.CreateNodeCommand(pythonNode, 0, 0, true, false));
+            DispatcherUtil.DoEventsLoop();
+
+            return NodeViewOf<PythonNode>();
+        }
+
+        /// <summary>
+        /// Returns the '-' button that VariableInputNodeViewCustomization adds to the node view.
+        /// </summary>
+        private static DynamoNodeButton RemovePortButton(NodeView nodeView)
+        {
+            var button = nodeView.inputGrid.ChildrenOfType<DynamoNodeButton>()
+                .SingleOrDefault(b => "-".Equals(b.Content));
+
+            Assert.IsNotNull(button, "Expected a single '-' button on the Python node view.");
+            return button;
+        }
+
+        private static void ClickButton(DynamoNodeButton button)
+        {
+            button.RaiseEvent(new RoutedEventArgs(ButtonBase.ClickEvent));
+            DispatcherUtil.DoEventsLoop();
+        }
+
+        private void AssertWarningShown(Times times)
+        {
+            dialogMock.Verify(m => m.Show(It.IsAny<Window>(), It.IsAny<string>(), It.IsAny<string>(),
+                It.IsAny<MessageBoxButton>(), It.IsAny<MessageBoxImage>()), times);
+        }
+
+        [Test]
+        public void WhenRemovingUnmodifiedPythonPortThenNoWarningIsShown()
+        {
+            // Arrange: a Python node with its single default input port, left untouched.
+            InstallDialogMock();
+            var nodeView = CreatePythonNodeView(out var pythonNode);
+            Assert.AreEqual(1, pythonNode.InPorts.Count);
+
+            // Act: click the '-' button.
+            ClickButton(RemovePortButton(nodeView));
+
+            // Assert: nothing was customized, so the user is not prompted and the port just goes.
+            AssertWarningShown(Times.Never());
+            Assert.AreEqual(0, pythonNode.InPorts.Count);
+        }
+
+        [Test]
+        public void WhenRemovingRenamedPythonPortThenWarningIsShown()
+        {
+            // Arrange: a Python node whose last input port has been renamed by the user.
+            InstallDialogMock();
+            var nodeView = CreatePythonNodeView(out var pythonNode);
+            pythonNode.InPorts[0].Name = "myInput";
+
+            // Act: click the '-' button.
+            ClickButton(RemovePortButton(nodeView));
+
+            // Assert: the user is warned before the rename is discarded.
+            AssertWarningShown(Times.Once());
+        }
+
+        [Test]
+        public void WhenRemovePortWarningIsCancelledThenPortIsKept()
+        {
+            // Arrange: a renamed port, so clicking '-' raises the warning. The mocked dialog
+            // answers Cancel, standing in for the user declining.
+            InstallDialogMock(MessageBoxResult.Cancel);
+            var nodeView = CreatePythonNodeView(out var pythonNode);
+            pythonNode.InPorts[0].Name = "myInput";
+
+            // Act: click the '-' button and decline the warning.
+            ClickButton(RemovePortButton(nodeView));
+
+            // Assert: declining aborts the removal outright - the port and its custom name survive.
+            // Without this, nothing verifies that Cancel is honoured rather than ignored.
+            AssertWarningShown(Times.Once());
+            Assert.AreEqual(1, pythonNode.InPorts.Count);
+            Assert.AreEqual("myInput", pythonNode.InPorts[0].Name);
+        }
+
+        [Test]
+        public void WhenRemovingUnmodifiedLastPortWhileEarlierPortIsRenamedThenNoWarningIsShown()
+        {
+            // Arrange: two input ports where only the FIRST is renamed. The '-' button removes the
+            // LAST port, which is untouched, so no customization is actually at risk.
+            InstallDialogMock();
+            var nodeView = CreatePythonNodeView(out var pythonNode);
+            pythonNode.HandleModelEvent("AddInPort", 0, null);
+            DispatcherUtil.DoEventsLoop();
+            Assert.AreEqual(2, pythonNode.InPorts.Count);
+            pythonNode.InPorts[0].Name = "myInput";
+
+            // Act: click the '-' button.
+            ClickButton(RemovePortButton(nodeView));
+
+            // Assert: the gate must inspect the port being removed rather than a fixed index,
+            // so an unrelated rename on an earlier port must not raise the warning.
+            AssertWarningShown(Times.Never());
+            Assert.AreEqual(1, pythonNode.InPorts.Count);
+        }
+
+        [Test]
+        public void WhenRemovingRenamedLastPortWhileEarlierPortIsDefaultThenWarningIsShown()
+        {
+            // Arrange: two input ports where only the LAST one - the one that will be removed -
+            // is renamed. This is the mirror of the test above.
+            InstallDialogMock();
+            var nodeView = CreatePythonNodeView(out var pythonNode);
+            pythonNode.HandleModelEvent("AddInPort", 0, null);
+            DispatcherUtil.DoEventsLoop();
+            pythonNode.InPorts[1].Name = "myInput";
+
+            // Act: click the '-' button.
+            ClickButton(RemovePortButton(nodeView));
+
+            // Assert: the user is warned before the rename on the last port is discarded.
+            AssertWarningShown(Times.Once());
+        }
+
+        [Test]
+        public void WhenRemovingRetooltippedPythonPortThenWarningIsShown()
+        {
+            // Arrange: a customized description alone must also trigger the warning, so that
+            // the gate cannot be narrowed to only check the port name.
+            InstallDialogMock();
+            var nodeView = CreatePythonNodeView(out var pythonNode);
+            pythonNode.InPorts[0].ToolTip = "my description";
+
+            // Act: click the '-' button.
+            ClickButton(RemovePortButton(nodeView));
+
+            // Assert: the user is warned before the description is discarded.
+            AssertWarningShown(Times.Once());
+        }
+    }
+}