From 1e4040cb2dbfbb63c16d1d7665ef18123879d6f9 Mon Sep 17 00:00:00 2001
From: Martin Misol Monzo <mmisol@users.noreply.github.com>
Date: Tue, 2 Mar 2021 10:50:34 -0500
Subject: [PATCH] Do not set execution hint on undo move (#11519) (#11524)

There were a couple of issues that were causing a node to be set to be
executed on undoing an action that consisted of just a move:

1. The 'UsingDefaultValue' property was triggering a property changed
even if the deserialized value was the same as the current. This was
fixed by checking if the value is the same and not doing anything in
that case.

2. Even if the 'IsFrozen' property for the current node had not changed
an undo would mark the current and all downstream nodes as if they were
modified. This was fixed by relying on the 'IsFrozen' property
setter, which correctly handles the cases when the previous should be
done.
---
 src/DynamoCore/Graph/Nodes/NodeModel.cs      |  9 +-------
 src/DynamoCore/Graph/Nodes/PortModel.cs      |  9 +++++---
 src/Libraries/PythonNodeModels/PythonNode.cs |  9 +++++++-
 test/DynamoCoreTests/CoreTests.cs            | 23 ++++++++++++++++++++
 4 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/src/DynamoCore/Graph/Nodes/NodeModel.cs b/src/DynamoCore/Graph/Nodes/NodeModel.cs
index 2e9cf0073b1..32e8905644d 100644
--- a/src/DynamoCore/Graph/Nodes/NodeModel.cs
+++ b/src/DynamoCore/Graph/Nodes/NodeModel.cs
@@ -2234,7 +2234,7 @@ protected override void DeserializeCore(XmlElement nodeElement, SaveContext cont
             argumentLacing = helper.ReadEnum("lacing", LacingStrategy.Disabled);
             IsSetAsInput = helper.ReadBoolean("isSelectedInput", false);
             IsSetAsOutput = helper.ReadBoolean("isSelectedOutput", false);
-            isFrozenExplicitly = helper.ReadBoolean("IsFrozen", false);
+            IsFrozen = helper.ReadBoolean("IsFrozen", false);
             PreviewPinned = helper.ReadBoolean("isPinned", false);
 
             var portInfoProcessed = new HashSet<int>();
@@ -2306,13 +2306,6 @@ var port in
                 RaisePropertyChanged(nameof(ArgumentLacing));
                 RaisePropertyChanged(nameof(IsVisible));
                 RaisePropertyChanged(nameof(DisplayLabels));
-                RaisePropertyChanged(nameof(IsSetAsInput));
-                RaisePropertyChanged(nameof(IsSetAsOutput));
-                //we need to modify the downstream nodes manually in case the
-                //undo is for toggling freeze. This is ONLY modifying the execution hint.
-                // this does not run the graph.
-                RaisePropertyChanged(nameof(IsFrozen));
-                MarkDownStreamNodesAsModified(this);
 
                 // Notify listeners that the position of the node has changed,
                 // then all connected connectors will also redraw themselves.
diff --git a/src/DynamoCore/Graph/Nodes/PortModel.cs b/src/DynamoCore/Graph/Nodes/PortModel.cs
index 9ebef7c86dd..ed38bf5b699 100644
--- a/src/DynamoCore/Graph/Nodes/PortModel.cs
+++ b/src/DynamoCore/Graph/Nodes/PortModel.cs
@@ -189,9 +189,12 @@ public bool UsingDefaultValue
             get { return usingDefaultValue; }
             set
             {
-                usingDefaultValue = value;
-                RaisePropertyChanged("UsingDefaultValue");
-                RaisePropertyChanged("ToolTip");
+                if (usingDefaultValue != value)
+                {
+                    usingDefaultValue = value;
+                    RaisePropertyChanged("UsingDefaultValue");
+                    RaisePropertyChanged("ToolTip");
+                }
             }
         }
 
diff --git a/src/Libraries/PythonNodeModels/PythonNode.cs b/src/Libraries/PythonNodeModels/PythonNode.cs
index f00fb45b126..4a7ce1c0dac 100644
--- a/src/Libraries/PythonNodeModels/PythonNode.cs
+++ b/src/Libraries/PythonNodeModels/PythonNode.cs
@@ -347,7 +347,14 @@ protected override void DeserializeCore(XmlElement nodeElement, SaveContext cont
 
             if (engineNode != null)
             {
-                this.Engine = (PythonEngineVersion)Enum.Parse(typeof(PythonEngineVersion),engineNode.InnerText);
+                var oldEngine = Engine;
+                Engine = (PythonEngineVersion)Enum.Parse(typeof(PythonEngineVersion), engineNode.InnerText);
+                if (context == SaveContext.Undo && oldEngine != this.Engine)
+                {
+                    // For Python nodes, changing the Engine property does not set the node Modified flag.
+                    // This is done externally in all event handlers, so for Undo we do it here.
+                    OnNodeModified();
+                }
             }
         }
 
diff --git a/test/DynamoCoreTests/CoreTests.cs b/test/DynamoCoreTests/CoreTests.cs
index 3a1397e6d16..f869780ded8 100644
--- a/test/DynamoCoreTests/CoreTests.cs
+++ b/test/DynamoCoreTests/CoreTests.cs
@@ -676,6 +676,29 @@ public void PasteInputAndOutputNodeInHomeWorkspace()
             Assert.IsInstanceOf<CodeBlockNodeModel>(homeNodes.ElementAt(1));
         }
 
+        [Test]
+        public void UndoMoveDoesNotForceExecution()
+        {
+            string openPath = Path.Combine(TestDirectory, @"core\LacingTest.dyn");
+            OpenModel(openPath);
+            RunCurrentModel();
+
+            var sumNodeGuid = new Guid("088365b5-4543-4e73-9430-463475147e19");
+            var sumNode = CurrentDynamoModel.CurrentWorkspace.Nodes.First(node => node.GUID == sumNodeGuid);
+            Assert.IsFalse(sumNode.IsModified);
+
+            var oldX = sumNode.X;
+            var newX = sumNode.X + 100;
+            var newPosition = $"{newX};{sumNode.Y}";
+            CurrentDynamoModel.ExecuteCommand(new DynCmd.UpdateModelValueCommand(Guid.Empty, sumNodeGuid, nameof(NodeModel.Position), newPosition));
+            Assert.AreEqual(newX, sumNode.X);
+
+            CurrentDynamoModel.ExecuteCommand(new DynCmd.UndoRedoCommand(DynCmd.UndoRedoCommand.Operation.Undo));
+            Assert.AreEqual(oldX, sumNode.X);
+
+            Assert.IsFalse(sumNode.IsModified);
+        }
+
         [Test]
         public void TestFileDirtyOnLacingChange()
         {