Merge pull request #2169 from glopesdev/issue-1523

Ensure topological sort before deletion

Author: glopesdev

Bonsai.Core.Tests/TestWorkflow.cs

@@ -87,6 +87,11 @@ public TestWorkflow AppendOutput()
             return Append(new WorkflowOutputBuilder());
         }
 
+        public TestWorkflow AppendNamed(string name)
+        {
+            return Append(new GroupWorkflowBuilder { Name = name });
+        }
+
         public TestWorkflow AppendPropertyMapping(params string[] propertyNames)
         {
             var mappingBuilder = new PropertyMappingBuilder();

Bonsai.Editor.Tests/EditorHelper.cs

@@ -1,4 +1,5 @@
-using System.Collections.Generic;
+using System;
+using System.Collections.Generic;
 using System.Linq;
 using Bonsai.Editor.GraphModel;
 using Bonsai.Expressions;
@@ -45,6 +46,11 @@ internal static GraphNode FindNode(this WorkflowEditor editor, ExpressionBuilder
             return editor.FindGraphNode(node.Value);
         }
 
+        internal static IEnumerable<GraphNode> FindNodes(this WorkflowEditor editor, params string[] names)
+        {
+            return Array.ConvertAll(names, name => editor.FindNode(name));
+        }
+
         internal static void ConnectNodes(this WorkflowEditor editor, string source, string target)
         {
             var sourceNodes = new[] { editor.FindNode(source) };

Bonsai.Editor.Tests/WorkflowEditorTests.cs

@@ -230,6 +230,30 @@ public void DisconnectGraphNodes_TargetIsNotRoot_InsertAfterClosestRoot()
             assertIsReversible();
         }
 
+        [TestMethod]
+        public void DeleteGraphNodes_SinglePathWithMultipleMergePoints_KeepConnectedPath()
+        {
+            var workflow = new TestWorkflow()
+                .AppendNamed("A")
+                .AppendNamed("B")
+                .AppendBranch(source => source.AddArguments(source.AppendInput(index: 0)))
+                .AppendNamed("C")
+                .AppendBranch(source => source.AddArguments(source.AppendInput(index: 1)))
+                .AppendNamed("D")
+                .TopologicalSort()
+                .ToInspectableGraph();
+
+            var (editor, assertIsReversible) = CreateMockEditor(workflow);
+            var nodesToDelete = editor.FindNodes("B", "Source1", "C", "Source2");
+            editor.DeleteGraphNodes(nodesToDelete);
+
+            var sourceNode = editor.FindNode("A");
+            var targetNode = editor.FindNode("D");
+            Assert.AreEqual(expected: 1, sourceNode.Successors.Count());
+            Assert.IsTrue(sourceNode.Successors.Any(edge => edge.Node == targetNode));
+            assertIsReversible();
+        }
+
         [TestMethod]
         public void CreateAnnotation_EmptySelection_InsertAfterClosestRoot()
         {

Bonsai.Editor/GraphModel/WorkflowEditor.cs

@@ -1502,7 +1502,7 @@ public void DeleteGraphNodes(IEnumerable<GraphNode> nodes)
             var updateGraphLayout = CreateUpdateGraphLayoutDelegate();
             commandExecutor.BeginCompositeCommand();
             commandExecutor.Execute(EmptyAction, reorder.Undo + updateGraphLayout);
-            foreach (var node in nodes)
+            foreach (var node in nodes.SortSelection(Workflow).ToList())
             {
                 DeleteGraphNode(node);
             }