Note PinUnpin Bugfix (#12925)

* Note PinUnpin Bugfix

* Accidental Selection Pin Fixed

- fixed an issue where a previously selected Node will get the target for re-pinning after Undo
- small clean-up from existing code

* Test Fails Fix

- attempt to fix multiple unit tests failing

src/DynamoCore/Graph/Notes/NoteModel.cs

@@ -11,6 +11,11 @@ namespace Dynamo.Graph.Notes
     /// </summary>
     public class NoteModel : ModelBase
     {
+        public enum UndoAction
+        {
+            Pin, Unpin
+        }
+
         /// <summary>
         /// This action is triggered when undo command is pressed and a node is pinned
         /// </summary>
@@ -45,11 +50,33 @@ public NodeModel PinnedNode
             get { return pinnedNode; }
             set
             {
-                pinnedNode = value;               
+                pinnedNode = value;
                 RaisePropertyChanged(nameof(PinnedNode));
             }
         }
 
+        private Guid pinnedNodeGuid;
+
+        public Guid PinnedNodeGuid
+        {
+            get { return pinnedNodeGuid; }
+            set
+            {
+                pinnedNodeGuid = value;
+                RaisePropertyChanged(nameof(PinnedNodeGuid));
+            }
+        }
+
+        private UndoAction undoAction;
+        public UndoAction UndoRedoAction
+        {
+            get { return undoAction; }
+            set
+            {
+                undoAction = value;
+                RaisePropertyChanged(nameof(UndoRedoAction));
+            }
+        }
 
         /// <summary>
         /// Creates NoteModel.
@@ -91,13 +118,13 @@ protected override bool UpdateValueCore(UpdateValueParams updateValueParams)
             string name = updateValueParams.PropertyName;
             string value = updateValueParams.PropertyValue;
 
-            if (name != "Text") 
+            if (name != "Text")
                 return base.UpdateValueCore(updateValueParams);
-            
+
             Text = value;
 
             return true;
-        }        
+        }
         #endregion
 
         #region Serialization/Deserialization Methods
@@ -120,7 +147,13 @@ protected override void DeserializeCore(XmlElement nodeElement, SaveContext cont
             X = helper.ReadDouble("x", 0.0);
             Y = helper.ReadDouble("y", 0.0);
 
-            if(pinnedNode != null)
+            try
+            {
+                PinnedNodeGuid = helper.ReadGuid("pinnedNode");
+            }
+            catch (Exception) { }
+
+            if (pinnedNode != null && helper.ReadGuid("pinnedNode") != Guid.Empty)
                 pinnedNode.GUID = helper.ReadGuid("pinnedNode");
 
             // Notify listeners that the position of the note has changed, 
@@ -134,8 +167,15 @@ protected override void DeserializeCore(XmlElement nodeElement, SaveContext cont
         /// </summary>
         internal void TryToSubscribeUndoNote()
         {
-            if (pinnedNode!=null && pinnedNode.GUID == Guid.Empty && UndoRequest != null)
+            if (pinnedNode != null && PinnedNodeGuid == Guid.Empty && UndoRequest != null)
+            {
+                UndoRedoAction = UndoAction.Unpin;
+                UndoRequest(this);
+                return;
+            }
+            else if (pinnedNode == null && PinnedNodeGuid != Guid.Empty && UndoRequest != null)
             {
+                UndoRedoAction = UndoAction.Pin;
                 UndoRequest(this);
             }
         }

src/DynamoCoreWpf/ViewModels/Core/NoteViewModel.cs

@@ -16,7 +16,7 @@
 
 namespace Dynamo.ViewModels
 {
-    public partial class NoteViewModel: ViewModelBase
+    public partial class NoteViewModel : ViewModelBase
     {
         private int DISTANCE_TO_PINNED_NODE = 16;
         private int DISTANCE_TO_PINNED_NODE_WITH_WARNING = 64;
@@ -131,7 +131,7 @@ public NodeViewModel PinnedNode
         {
             get
             {
-                if (Model.PinnedNode==null)
+                if (Model.PinnedNode == null)
                 {
                     return null;
                 }
@@ -149,6 +149,7 @@ public NoteViewModel(WorkspaceViewModel workspaceViewModel, NoteModel model)
             this.WorkspaceViewModel = workspaceViewModel;
             _model = model;
             model.PropertyChanged += note_PropertyChanged;
+            model.UndoRequest += note_PinUnpinToNode;
             DynamoSelection.Instance.Selection.CollectionChanged += SelectionOnCollectionChanged;
             ZIndex = ++StaticZIndex; // places the note on top of all nodes/notes
 
@@ -166,9 +167,36 @@ public override void Dispose()
                 UnsuscribeFromPinnedNode();
             }
             _model.PropertyChanged -= note_PropertyChanged;
+            _model.UndoRequest -= note_PinUnpinToNode;
             DynamoSelection.Instance.Selection.CollectionChanged -= SelectionOnCollectionChanged;
         }
 
+        private void note_PinUnpinToNode(ModelBase obj)
+        {
+            if (Model.UndoRedoAction.Equals(NoteModel.UndoAction.Unpin))
+            {
+                UnpinFromNode(obj);
+                return;
+            }
+            if (Model.UndoRedoAction.Equals(NoteModel.UndoAction.Pin))
+            {
+                NodeModel node = WorkspaceViewModel.Model.Nodes
+                    .Where(x => x.GUID.Equals(Model.PinnedNodeGuid))
+                    .FirstOrDefault();
+
+                if (node == null) return;
+
+                // In case the user has selected a different Node before Undo
+                // We run the risk of pinning to the wrong Node
+                // Therefore clear selection before running
+                DynamoSelection.Instance.ClearSelection();
+                DynamoSelection.Instance.Selection.Add(node);
+                PinToNode(obj);
+                return;
+            }
+        }
+
+
         private void SelectionOnCollectionChanged(object sender, System.Collections.Specialized.NotifyCollectionChangedEventArgs e)
         {
             CreateGroupCommand.RaiseCanExecuteChanged();
@@ -184,7 +212,7 @@ private void Select(object parameter)
 
         public void UpdateSizeFromView(double w, double h)
         {
-            this._model.SetSize(w,h);
+            this._model.SetSize(w, h);
             MoveNoteAbovePinnedNode();
         }
 
@@ -307,7 +335,6 @@ private void PinToNode(object parameters)
             }
 
             Model.PinnedNode = nodeToPin;
-            Model.UndoRequest += UnpinFromNode;
 
             MoveNoteAbovePinnedNode();
             SubscribeToPinnedNode();
@@ -327,7 +354,7 @@ private bool CanPinToNode(object parameters)
             var noteSelection = DynamoSelection.Instance.Selection
                     .OfType<NoteModel>();
 
-            if (nodeSelection == null || noteSelection == null || 
+            if (nodeSelection == null || noteSelection == null ||
                 nodeSelection.Count() != 1 || noteSelection.Count() != 1)
                 return false;
 
@@ -361,7 +388,6 @@ private bool CanUnpinFromNode(object parameters)
         private void UnpinFromNode(object parameters)
         {
             UnsuscribeFromPinnedNode();
-            Model.UndoRequest -= UnpinFromNode;
 
             Model.PinnedNode = null;
             WorkspaceViewModel.HasUnsavedChanges = true;