DYN-8656: connector pins reconnect undo fix (#16331)

dnenov · web-flow · commit ebc72172e872 · 2025-07-08T22:19:18.000-04:00
diff --git a/src/DynamoCore/Graph/Workspaces/UndoRedo.cs b/src/DynamoCore/Graph/Workspaces/UndoRedo.cs
@@ -3,6 +3,7 @@
 using System.Diagnostics;
 using System.Linq;
 using System.Xml;
+using Autodesk.DesignScript.Geometry;
 using Dynamo.Core;
 using Dynamo.Graph.Annotations;
 using Dynamo.Graph.Connectors;
@@ -288,19 +289,20 @@ internal void RecordAndDeleteModels(List<ModelBase> models)
 
                         RemoveAndDisposeNode(node);
                     }
-                    else if (model is ConnectorModel connector)
+                    else if (model is ConnectorModel conn)
                     {
-                        undoRecorder.RecordDeletionForUndo(connector);
-                        if (connector.ConnectorPinModels.Count > 0)
+                        if (conn.ConnectorPinModels.Count > 0)
                         {
-                            foreach (var connectorPin in connector.ConnectorPinModels.ToList())
+                            foreach (var connectorPin in conn.ConnectorPinModels.ToList())
                             {
                                 undoRecorder.RecordDeletionForUndo(connectorPin);
                                 var matchingConnector = Connectors.FirstOrDefault(c => c.GUID == connectorPin.ConnectorId);
                                 if (matchingConnector is null) return;
                                 matchingConnector.ConnectorPinModels.Remove(connectorPin);
                             }
                         }
+                        undoRecorder.RecordDeletionForUndo(conn);
+                        conn.Delete();
                     }
                     else if (model is ConnectorPinModel connectorPinModel)
                     {
@@ -323,9 +325,17 @@ internal void SaveAndDeleteModels(List<ModelBase> models)
         {
             if (null != models)
             {
-                // If an existing connector is grabbed (to be reconnected), save the 
+                // Add connector pins if any
+                var allPins = models
+                    .OfType<ConnectorModel>()
+                    .SelectMany(connector => connector.ConnectorPinModels)
+                    .Cast<ModelBase>()
+                    .ToList();
+                var fullSet = allPins.Concat(models).ToList();
+
+                // If an existing connector/pin set is grabbed (to be reconnected), save the 
                 // models for deletion later in one action group.
-                savedModels = models;
+                savedModels = fullSet;
 
                 // After saving the models, delete them from the workspace
                 // in one action group.
@@ -420,7 +430,7 @@ public void DeleteModel(XmlElement modelData)
 
                 RemoveAndDisposeNode(model as NodeModel);
             }
-            else if(model == null)
+            else if (model == null)
             {
                 return;
             }
@@ -451,7 +461,7 @@ public void RemoveGroup(ModelBase model)
         public void ReloadModel(XmlElement modelData)
         {
             ModelBase model = GetModelForElement(modelData);
-            if(model != null)
+            if (model != null)
             {
                 model.Deserialize(modelData, SaveContext.Undo);
             }
@@ -589,7 +599,7 @@ public ModelBase GetModelForElement(XmlElement modelData)
             ModelBase foundModel = GetModelInternal(modelGuid);
             if (null != foundModel)
                 return foundModel;
-            
+
             //if we could not find a matching model
             this.Log(string.Format("Please Report: Unhandled model type: {0}, could not find a matching model with given id", helper.ReadString("type", modelData.Name)), Logging.WarningLevel.Error);
             return null;
@@ -608,11 +618,11 @@ public ModelBase GetModelInternal(Guid modelGuid)
                 ?? Annotations.FirstOrDefault(annotation => annotation.GUID == modelGuid) as ModelBase
                 ?? Presets.FirstOrDefault(preset => preset.GUID == modelGuid) as ModelBase);
 
-            if(foundModel is null)
+            if (foundModel is null)
             {
-                foreach(var connector in Connectors)
+                foreach (var connector in Connectors)
                 {
-                    foreach(var pin in connector.ConnectorPinModels)
+                    foreach (var pin in connector.ConnectorPinModels)
                     {
                         if (pin.GUID == modelGuid)
                             return pin as ModelBase;
diff --git a/src/DynamoCore/Models/DynamoModelCommands.cs b/src/DynamoCore/Models/DynamoModelCommands.cs
@@ -458,10 +458,6 @@ private void BeginShiftReconnections(Guid nodeId, int portIndex, PortType portTy
                 activeStartPorts[i] = connector.End;
             }
             CurrentWorkspace.SaveAndDeleteModels(selectedConnectors.ToList<ModelBase>());
-            for (int i = 0; i < numOfConnectors; i++) //delete the connectors
-            {
-                selectedConnectors[i].Delete();
-            }
             return;
         }
 
diff --git a/src/DynamoCoreWpf/Commands/DynamoCommands.cs b/src/DynamoCoreWpf/Commands/DynamoCommands.cs
@@ -158,6 +158,8 @@ void OnModelCommandCompleted(DynamoModel.RecordableCommand command)
                 case "ForceRunCancelCommand":
                 case "SelectModelCommand":
                 case "MakeConnectionCommand":
+                    RaiseCanExecuteUndoRedo();
+                    break;
                 case "CreateCustomNodeCommand":
                 case "AddPresetCommand":
                 case "ApplyPresetCommand":
diff --git a/test/DynamoCoreWpf2Tests/ViewExtensions/NotificationsExtensionTests.cs b/test/DynamoCoreWpf2Tests/ViewExtensions/NotificationsExtensionTests.cs
@@ -39,7 +39,7 @@ public void PressNotificationButtonAndShowPopup()
                     return notificationUI != null;
                 }
                 return false;
-            }, 180);
+            }, 300);
             
             Assert.NotNull(notificationUI, "Notification popup not part of the dynamo visual tree");
             var webView = notificationUI.FindName("webView");
diff --git a/test/DynamoCoreWpfTests/ConnectorViewModelTests.cs b/test/DynamoCoreWpfTests/ConnectorViewModelTests.cs
@@ -1,6 +1,8 @@
 using System;
 using System.IO;
 using System.Linq;
+using Dynamo.Graph.Nodes;
+using Dynamo.Models;
 using Dynamo.Selection;
 using Dynamo.Utilities;
 using Dynamo.ViewModels;
@@ -471,6 +473,62 @@ public void CanUndoPlaceWatchNode()
             int finalConnectorCount = this.ViewModel.CurrentSpaceViewModel.Connectors.Count;
             Assert.AreEqual(initialConnectorCount, finalConnectorCount);
         }
+
+        [Test]
+        public void CanUndoShiftReconnectionWithConnectorPin()
+        {
+            // Open a test graph with at least two connected nodes
+            Open(@"UI/ConnectorPinTests.dyn");
+
+            var id = Guid.Parse("90cd0dca-45c7-456b-bb63-726d2193dbb4");
+            var codeblock = this.ViewModel.Model.CurrentWorkspace.GetModelInternal(id);
+
+            var connectorViewModel = this.ViewModel.CurrentSpaceViewModel.Connectors.First();
+            var initialConnectorPinCount = connectorViewModel.ConnectorPinViewCollection.Count;
+
+            // Setup: hover and pin a connector manually
+            connectorViewModel.PanelX = 292.66666;
+            connectorViewModel.PanelY = 278;
+            connectorViewModel.FlipOnConnectorAnchor();
+            connectorViewModel.PinConnectorCommand.Execute(null);
+
+            // Sanity check: we added 1 pin
+            Assert.AreEqual(initialConnectorPinCount + 1, connectorViewModel.ConnectorPinViewCollection.Count);
+            var pinModelGuid = connectorViewModel.ConnectorModel.ConnectorPinModels.First().GUID;
+
+            // Begin reconnection – simulate grabbing the connector and starting a shift drag
+            var connectorGuid = connectorViewModel.ConnectorModel.GUID;
+            var startPort = connectorViewModel.ConnectorModel.Start;
+            var startNode = startPort.Owner;
+            var startPortIndex = startNode.OutPorts.IndexOf(startPort);
+
+            this.ViewModel.ExecuteCommand(
+                new DynamoModel.MakeConnectionCommand(startNode.GUID, startPortIndex, PortType.Output,
+                MakeConnectionCommand.Mode.BeginShiftReconnections));
+
+            // Execute the second part of the workflow - simulate placing the connectors over the new port
+            this.ViewModel.ExecuteCommand(
+                new DynamoModel.MakeConnectionCommand(codeblock.GUID, 0, PortType.Output,
+                MakeConnectionCommand.Mode.EndShiftReconnections));
+
+            // Validate that the pin does not exists anymore
+            var connectorAfterReconnect = this.ViewModel.CurrentSpaceViewModel.Connectors;
+            Assert.AreEqual(1, connectorAfterReconnect.Count());
+            Assert.AreEqual(0, connectorAfterReconnect.First().ConnectorPinViewCollection.Count());
+
+            // --- Undo ---
+            Model.ExecuteCommand(new UndoRedoCommand(UndoRedoCommand.Operation.Undo));
+
+            // Confirm that the pin is also undone (either removed, or restored to original connector)
+            var restoredConnector = this.ViewModel.CurrentSpaceViewModel.Connectors
+                .FirstOrDefault(c => c.ConnectorModel.GUID == connectorGuid);
+
+            Assert.IsNotNull(restoredConnector);
+            Assert.AreEqual(initialConnectorPinCount + 1, restoredConnector.ConnectorPinViewCollection.Count);
+
+            var restoredPin = restoredConnector.ConnectorModel.ConnectorPinModels.FirstOrDefault(p => p.GUID == pinModelGuid);
+            Assert.IsNotNull(restoredPin, "Expected pin model not found after undo.");
+        }
         #endregion
     }
 }
diff --git a/test/UI/ConnectorPinTests.dyn b/test/UI/ConnectorPinTests.dyn
@@ -11,9 +11,8 @@
   "Nodes": [
     {
       "ConcreteType": "Dynamo.Graph.Nodes.CodeBlockNodeModel, DynamoCore",
-      "NodeType": "CodeBlockNode",
-      "Code": "0..13;",
       "Id": "286e9f9ab34d4cbd9b154b58627d5b37",
+      "NodeType": "CodeBlockNode",
       "Inputs": [],
       "Outputs": [
         {
@@ -27,13 +26,13 @@
         }
       ],
       "Replication": "Disabled",
-      "Description": "Allows for DesignScript code to be authored directly"
+      "Description": "Allows for DesignScript code to be authored directly",
+      "Code": "0..13;"
     },
     {
       "ConcreteType": "Dynamo.Graph.Nodes.CodeBlockNodeModel, DynamoCore",
-      "NodeType": "CodeBlockNode",
-      "Code": "asd;",
       "Id": "9d43c824812c4562be7a3877a4e865f1",
+      "NodeType": "CodeBlockNode",
       "Inputs": [
         {
           "Id": "b1f0ee501c7b4c628da090daacf5d73e",
@@ -57,19 +56,41 @@
         }
       ],
       "Replication": "Disabled",
-      "Description": "Allows for DesignScript code to be authored directly"
+      "Description": "Allows for DesignScript code to be authored directly",
+      "Code": "asd;"
+    },
+    {
+      "ConcreteType": "Dynamo.Graph.Nodes.CodeBlockNodeModel, DynamoCore",
+      "Id": "90cd0dca45c7456bbb63726d2193dbb4",
+      "NodeType": "CodeBlockNode",
+      "Inputs": [],
+      "Outputs": [
+        {
+          "Id": "e7682c9fd6734bfdb69ae97b2a8fef8e",
+          "Name": "",
+          "Description": "Value of expression at line 1",
+          "UsingDefaultValue": false,
+          "Level": 2,
+          "UseLevels": false,
+          "KeepListStructure": false
+        }
+      ],
+      "Replication": "Disabled",
+      "Description": "Allows for DesignScript code to be authored directly",
+      "Code": "\"Hello World\";"
     }
   ],
   "Connectors": [
     {
       "Start": "ca4252c5e4174be28206765f5ce7d960",
       "End": "b1f0ee501c7b4c628da090daacf5d73e",
       "Id": "77f6e15a9d984ad4b6ecfe173920fd2a",
-      "IsVisible": "True"
+      "IsHidden": "False"
     }
   ],
   "Dependencies": [],
   "NodeLibraryDependencies": [],
+  "EnableLegacyPolyCurveBehavior": true,
   "Thumbnail": "",
   "GraphDocumentationURL": null,
   "ExtensionWorkspaceData": [
@@ -93,12 +114,12 @@
       "ScaleFactor": 1.0,
       "HasRunWithoutCrash": true,
       "IsVisibleInDynamoLibrary": true,
-      "Version": "2.13.0.2482",
+      "Version": "3.6.0.6885",
       "RunType": "Automatic",
       "RunPeriod": "1000"
     },
     "Camera": {
-      "Name": "Background Preview",
+      "Name": "_Background Preview",
       "EyeX": -17.0,
       "EyeY": 24.0,
       "EyeZ": 50.0,
@@ -112,24 +133,34 @@
     "ConnectorPins": [],
     "NodeViews": [
       {
-        "Name": "Code Block",
-        "ShowGeometry": true,
         "Id": "286e9f9ab34d4cbd9b154b58627d5b37",
+        "Name": "Code Block",
         "IsSetAsInput": false,
         "IsSetAsOutput": false,
         "Excluded": false,
+        "ShowGeometry": true,
         "X": 62.0,
         "Y": 184.0
       },
       {
-        "Name": "Code Block",
-        "ShowGeometry": true,
         "Id": "9d43c824812c4562be7a3877a4e865f1",
+        "Name": "Code Block",
         "IsSetAsInput": false,
         "IsSetAsOutput": false,
         "Excluded": false,
+        "ShowGeometry": true,
         "X": 507.3333333333332,
-        "Y": 281.33333333333331
+        "Y": 281.3333333333333
+      },
+      {
+        "Id": "90cd0dca45c7456bbb63726d2193dbb4",
+        "Name": "Code Block",
+        "IsSetAsInput": false,
+        "IsSetAsOutput": false,
+        "Excluded": false,
+        "ShowGeometry": true,
+        "X": 37.0,
+        "Y": 447.0
       }
     ],
     "Annotations": [],

Original file line number	Diff line number	Diff line change
`@@ -3,6 +3,7 @@`
`3`	`3`	`using System.Diagnostics;`
`4`	`4`	`using System.Linq;`
`5`	`5`	`using System.Xml;`
	`6`	`+using Autodesk.DesignScript.Geometry;`
`6`	`7`	`using Dynamo.Core;`
`7`	`8`	`using Dynamo.Graph.Annotations;`
`8`	`9`	`using Dynamo.Graph.Connectors;`
`@@ -288,19 +289,20 @@ internal void RecordAndDeleteModels(List<ModelBase> models)`
`288`	`289`
`289`	`290`	`RemoveAndDisposeNode(node);`
`290`	`291`	`}`
`291`		`- else if (model is ConnectorModel connector)`
	`292`	`+ else if (model is ConnectorModel conn)`
`292`	`293`	`{`
`293`		`- undoRecorder.RecordDeletionForUndo(connector);`
`294`		`- if (connector.ConnectorPinModels.Count > 0)`
	`294`	`+ if (conn.ConnectorPinModels.Count > 0)`
`295`	`295`	`{`
`296`		`- foreach (var connectorPin in connector.ConnectorPinModels.ToList())`
	`296`	`+ foreach (var connectorPin in conn.ConnectorPinModels.ToList())`
`297`	`297`	`{`
`298`	`298`	`undoRecorder.RecordDeletionForUndo(connectorPin);`
`299`	`299`	`var matchingConnector = Connectors.FirstOrDefault(c => c.GUID == connectorPin.ConnectorId);`
`300`	`300`	`if (matchingConnector is null) return;`
`301`	`301`	`matchingConnector.ConnectorPinModels.Remove(connectorPin);`
`302`	`302`	`}`
`303`	`303`	`}`
	`304`	`+ undoRecorder.RecordDeletionForUndo(conn);`
	`305`	`+ conn.Delete();`
`304`	`306`	`}`
`305`	`307`	`else if (model is ConnectorPinModel connectorPinModel)`
`306`	`308`	`{`
`@@ -323,9 +325,17 @@ internal void SaveAndDeleteModels(List<ModelBase> models)`
`323`	`325`	`{`
`324`	`326`	`if (null != models)`
`325`	`327`	`{`
`326`		`- // If an existing connector is grabbed (to be reconnected), save the`
	`328`	`+ // Add connector pins if any`
	`329`	`+ var allPins = models`
	`330`	`+ .OfType<ConnectorModel>()`
	`331`	`+ .SelectMany(connector => connector.ConnectorPinModels)`
	`332`	`+ .Cast<ModelBase>()`
	`333`	`+ .ToList();`
	`334`	`+ var fullSet = allPins.Concat(models).ToList();`
	`335`	`+`
	`336`	`+ // If an existing connector/pin set is grabbed (to be reconnected), save the`
`327`	`337`	`// models for deletion later in one action group.`
`328`		`- savedModels = models;`
	`338`	`+ savedModels = fullSet;`
`329`	`339`
`330`	`340`	`// After saving the models, delete them from the workspace`
`331`	`341`	`// in one action group.`
`@@ -420,7 +430,7 @@ public void DeleteModel(XmlElement modelData)`
`420`	`430`
`421`	`431`	`RemoveAndDisposeNode(model as NodeModel);`
`422`	`432`	`}`
`423`		`- else if(model == null)`
	`433`	`+ else if (model == null)`
`424`	`434`	`{`
`425`	`435`	`return;`
`426`	`436`	`}`
`@@ -451,7 +461,7 @@ public void RemoveGroup(ModelBase model)`
`451`	`461`	`public void ReloadModel(XmlElement modelData)`
`452`	`462`	`{`
`453`	`463`	`ModelBase model = GetModelForElement(modelData);`
`454`		`- if(model != null)`
	`464`	`+ if (model != null)`
`455`	`465`	`{`
`456`	`466`	`model.Deserialize(modelData, SaveContext.Undo);`
`457`	`467`	`}`
`@@ -589,7 +599,7 @@ public ModelBase GetModelForElement(XmlElement modelData)`
`589`	`599`	`ModelBase foundModel = GetModelInternal(modelGuid);`
`590`	`600`	`if (null != foundModel)`
`591`	`601`	`return foundModel;`
`592`		`-`
	`602`	`+`
`593`	`603`	`//if we could not find a matching model`
`594`	`604`	`this.Log(string.Format("Please Report: Unhandled model type: {0}, could not find a matching model with given id", helper.ReadString("type", modelData.Name)), Logging.WarningLevel.Error);`
`595`	`605`	`return null;`
`@@ -608,11 +618,11 @@ public ModelBase GetModelInternal(Guid modelGuid)`
`608`	`618`	`?? Annotations.FirstOrDefault(annotation => annotation.GUID == modelGuid) as ModelBase`
`609`	`619`	`?? Presets.FirstOrDefault(preset => preset.GUID == modelGuid) as ModelBase);`
`610`	`620`
`611`		`- if(foundModel is null)`
	`621`	`+ if (foundModel is null)`
`612`	`622`	`{`
`613`		`- foreach(var connector in Connectors)`
	`623`	`+ foreach (var connector in Connectors)`
`614`	`624`	`{`
`615`		`- foreach(var pin in connector.ConnectorPinModels)`
	`625`	`+ foreach (var pin in connector.ConnectorPinModels)`
`616`	`626`	`{`
`617`	`627`	`if (pin.GUID == modelGuid)`
`618`	`628`	`return pin as ModelBase;`
Original file line number	Diff line number	Diff line change
`@@ -458,10 +458,6 @@ private void BeginShiftReconnections(Guid nodeId, int portIndex, PortType portTy`
`458`	`458`	`activeStartPorts[i] = connector.End;`
`459`	`459`	`}`
`460`	`460`	`CurrentWorkspace.SaveAndDeleteModels(selectedConnectors.ToList<ModelBase>());`
`461`		`- for (int i = 0; i < numOfConnectors; i++) //delete the connectors`
`462`		`- {`
`463`		`- selectedConnectors[i].Delete();`
`464`		`- }`
`465`	`461`	`return;`
`466`	`462`	`}`
`467`	`463`
Original file line number	Diff line number	Diff line change
`@@ -39,7 +39,7 @@ public void PressNotificationButtonAndShowPopup()`
`39`	`39`	`return notificationUI != null;`
`40`	`40`	`}`
`41`	`41`	`return false;`
`42`		`- }, 180);`
	`42`	`+ }, 300);`
`43`	`43`
`44`	`44`	`Assert.NotNull(notificationUI, "Notification popup not part of the dynamo visual tree");`
`45`	`45`	`var webView = notificationUI.FindName("webView");`