From efecfa328bcee4a0af3f48a0422e202e7de9747a Mon Sep 17 00:00:00 2001
From: Nick O'Leary <nick.oleary@gmail.com>
Date: Wed, 3 Jun 2020 14:23:26 +0100
Subject: [PATCH] Fix up event handling when deleting groups and tabs

---
 .../@node-red/editor-client/src/js/history.js | 29 +++++++++---
 .../@node-red/editor-client/src/js/nodes.js   | 42 +++++++++++------
 .../src/js/ui/tab-info-outliner.js            | 15 +++---
 .../@node-red/editor-client/src/js/ui/view.js | 47 ++++++++++++-------
 4 files changed, 86 insertions(+), 47 deletions(-)

diff --git a/packages/node_modules/@node-red/editor-client/src/js/history.js b/packages/node_modules/@node-red/editor-client/src/js/history.js
index ff4a75a1b..9fcfe30f9 100644
--- a/packages/node_modules/@node-red/editor-client/src/js/history.js
+++ b/packages/node_modules/@node-red/editor-client/src/js/history.js
@@ -66,6 +66,14 @@ RED.history = (function() {
                         }
                         inverseEv.nodes.push(node);
                         RED.nodes.remove(ev.nodes[i]);
+                        if (node.g) {
+                            var group = RED.nodes.group(node.g);
+                            var index = group.nodes.indexOf(node);
+                            if (index !== -1) {
+                                group.nodes.splice(index,1);
+                                RED.group.markDirty(group);
+                            }
+                        }
                     }
                 }
                 if (ev.links) {
@@ -80,7 +88,9 @@ RED.history = (function() {
                     for (i=0;i<ev.groups.length;i++) {
                         group = ev.groups[i];
                         modifiedTabs[group.z] = true;
-                        inverseEv.groups.push(group);
+                        // The order of groups is important
+                        //  - to invert the action, the order is reversed
+                        inverseEv.groups.unshift(group);
                         RED.nodes.removeGroup(group);
                     }
                 }
@@ -205,13 +215,20 @@ RED.history = (function() {
                 }
                 if (ev.groups) {
                     inverseEv.groups = [];
-                    var groupsToAdd = new Set(ev.groups.map(function(g) { return g.id }));
+                    var groupsToAdd = {};
+                    ev.groups.forEach(function(g) { groupsToAdd[g.id] = g; });
                     for (i=0;i<ev.groups.length;i++) {
                         RED.nodes.addGroup(ev.groups[i])
                         modifiedTabs[ev.groups[i].z] = true;
-                        inverseEv.groups.push(ev.groups[i]);
-                        if (ev.groups[i].g && !groupsToAdd.has(ev.groups[i].g)) {
-                            group = RED.nodes.group(ev.groups[i].g);
+                        // The order of groups is important
+                        //  - to invert the action, the order is reversed
+                        inverseEv.groups.unshift(ev.groups[i]);
+                        if (ev.groups[i].g) {
+                            if (!groupsToAdd[ev.groups[i].g]) {
+                                group = RED.nodes.group(ev.groups[i].g);
+                            } else {
+                                group = groupsToAdd[ev.groups[i].g];
+                            }
                             if (group.nodes.indexOf(ev.groups[i]) === -1) {
                                 group.nodes.push(ev.groups[i]);
                             }
@@ -297,7 +314,7 @@ RED.history = (function() {
                     }
                 }
                 if (ev.addToGroup) {
-                    RED.group.removeFromGroup(ev.addToGroup,ev.nodes.map(function(n) { return n.n }),true);
+                    RED.group.removeFromGroup(ev.addToGroup,ev.nodes.map(function(n) { return n.n }),false);
                     inverseEv.removeFromGroup = ev.addToGroup;
                 } else if (ev.removeFromGroup) {
                     RED.group.addToGroup(ev.removeFromGroup,ev.nodes.map(function(n) { return n.n }));
diff --git a/packages/node_modules/@node-red/editor-client/src/js/nodes.js b/packages/node_modules/@node-red/editor-client/src/js/nodes.js
index 15d44a606..887ec062f 100644
--- a/packages/node_modules/@node-red/editor-client/src/js/nodes.js
+++ b/packages/node_modules/@node-red/editor-client/src/js/nodes.js
@@ -364,35 +364,45 @@ RED.nodes = (function() {
             delete workspaces[id];
             delete nodeTabMap[id];
             workspacesOrder.splice(workspacesOrder.indexOf(id),1);
-            var n;
+            var i;
             var node;
-            for (n=0;n<nodes.length;n++) {
-                node = nodes[n];
+            for (i=0;i<nodes.length;i++) {
+                node = nodes[i];
                 if (node.z == id) {
                     removedNodes.push(node);
                 }
             }
-            for(n in configNodes) {
-                if (configNodes.hasOwnProperty(n)) {
-                    node = configNodes[n];
+            for(i in configNodes) {
+                if (configNodes.hasOwnProperty(i)) {
+                    node = configNodes[i];
                     if (node.z == id) {
                         removedNodes.push(node);
                     }
                 }
             }
-            removedGroups = groupsByZ[id] || [];
-            removedGroups.forEach(function(g) {
-                delete groups[g.id]
-            })
-            delete groupsByZ[id];
 
-            for (n=0;n<removedNodes.length;n++) {
-                var result = removeNode(removedNodes[n].id);
+            for (i=0;i<removedNodes.length;i++) {
+                var result = removeNode(removedNodes[i].id);
                 removedLinks = removedLinks.concat(result.links);
             }
+
+            // Must get 'removedGroups' in the right order.
+            //  - start with the top-most groups
+            //  - then recurse into them
+            removedGroups = (groupsByZ[id] || []).filter(function(g) { return !g.g; });
+            for (i=0;i<removedGroups.length;i++) {
+                removedGroups[i].nodes.forEach(function(n) {
+                    if (n.type === "group") {
+                        removedGroups.push(n);
+                    }
+                });
+            }
+            // Now remove them in the reverse order
+            for (i=removedGroups.length-1; i>=0; i--) {
+                removeGroup(removedGroups[i]);
+            }
             RED.events.emit('flows:remove',ws);
         }
-
         return {nodes:removedNodes,links:removedLinks, groups: removedGroups};
     }
 
@@ -1562,7 +1572,9 @@ RED.nodes = (function() {
     function removeGroup(group) {
         var i = groupsByZ[group.z].indexOf(group);
         groupsByZ[group.z].splice(i,1);
-
+        if (groupsByZ[group.z].length === 0) {
+            delete groupsByZ[group.z];
+        }
         if (group.g) {
             if (groups[group.g]) {
                 var index = groups[group.g].nodes.indexOf(group);
diff --git a/packages/node_modules/@node-red/editor-client/src/js/ui/tab-info-outliner.js b/packages/node_modules/@node-red/editor-client/src/js/ui/tab-info-outliner.js
index 7b3ab2c9f..eb0caf810 100644
--- a/packages/node_modules/@node-red/editor-client/src/js/ui/tab-info-outliner.js
+++ b/packages/node_modules/@node-red/editor-client/src/js/ui/tab-info-outliner.js
@@ -387,19 +387,16 @@ RED.sidebar.info.outliner = (function() {
         var existingObject = objects[n.id];
         existingObject.treeList.remove();
         delete objects[n.id]
+
+        // If this is a group being removed, it may have an empty item
+        if (empties[n.id]) {
+            delete empties[n.id];
+        }
+
         var parent = existingObject.parent;
         if (parent.children.length === 0) {
             parent.treeList.addChild(getEmptyItem(parent.id));
         }
-        // if (existingObject.children && (existingObject.children.length > 0)) {
-        //     existingObject.children.forEach(function (nc) {
-        //         if (!nc.empty) {
-        //             var childObject = objects[nc.id];
-        //             nc.parent = parent;
-        //             objects[parent.id].treeList.addChild(childObject);
-        //         }
-        //     });
-        // }
     }
     function getGutter(n) {
         var span = $("<span>",{class:"red-ui-info-outline-gutter"});
diff --git a/packages/node_modules/@node-red/editor-client/src/js/ui/view.js b/packages/node_modules/@node-red/editor-client/src/js/ui/view.js
index 7f541204e..6dee10bd7 100755
--- a/packages/node_modules/@node-red/editor-client/src/js/ui/view.js
+++ b/packages/node_modules/@node-red/editor-client/src/js/ui/view.js
@@ -1929,6 +1929,7 @@ if (DEBUG_EVENTS) { console.warn("clearSelection", mouse_mode); }
             redraw();
         } else if (moving_set.length > 0 || selected_link != null) {
             var result;
+            var node;
             var removedNodes = [];
             var removedLinks = [];
             var removedGroups = [];
@@ -1941,12 +1942,25 @@ if (DEBUG_EVENTS) { console.warn("clearSelection", mouse_mode); }
             var startChanged = false;
             var selectedGroups = [];
             if (moving_set.length > 0) {
+
                 for (var i=0;i<moving_set.length;i++) {
-                    var node = moving_set[i].n;
-                    node.selected = false;
+                    node = moving_set[i].n;
                     if (node.type === "group") {
                         selectedGroups.push(node);
-                    } else if (node.type != "subflow") {
+                    }
+                }
+                // Make sure we have identified all groups about to be deleted
+                for (i=0;i<selectedGroups.length;i++) {
+                    selectedGroups[i].nodes.forEach(function(n) {
+                        if (n.type === "group" && selectedGroups.indexOf(n) === -1) {
+                            selectedGroups.push(n);
+                        }
+                    })
+                }
+                for (var i=0;i<moving_set.length;i++) {
+                    node = moving_set[i].n;
+                    node.selected = false;
+                    if (node.type !== "group" && node.type !== "subflow") {
                         if (node.x < 0) {
                             node.x = 25
                         }
@@ -1956,9 +1970,12 @@ if (DEBUG_EVENTS) { console.warn("clearSelection", mouse_mode); }
                         removedLinks = removedLinks.concat(removedEntities.links);
                         if (node.g) {
                             var group = RED.nodes.group(node.g);
-                            if ((!group.selected || group.active) && (selectedGroups.indexOf(group) < 0)) {
-                                RED.group.removeFromGroup(group,node);
-                                node.g = group.id;
+                            if (selectedGroups.indexOf(group) === -1) {
+                                // Don't use RED.group.removeFromGroup as that emits
+                                // a change event on the node - but we're deleting it
+                                var index = group.nodes.indexOf(node);
+                                group.nodes.splice(index,1);
+                                RED.group.markDirty(group);
                             }
                         }
                     } else {
@@ -1972,17 +1989,13 @@ if (DEBUG_EVENTS) { console.warn("clearSelection", mouse_mode); }
                         node.dirty = true;
                     }
                 }
-                while (selectedGroups.length > 0) {
-                    var g = selectedGroups.shift();
-                    if (removedGroups.indexOf(g) === -1) {
-                        removedGroups.push(g);
-                        g.nodes.forEach(function(n) {
-                            if (n.type === "group") {
-                                selectedGroups.push(n);
-                            }
-                        })
-                        RED.nodes.removeGroup(g);
-                    }
+
+                // Groups must be removed in the right order - from inner-most
+                // to outermost.
+                for (i = selectedGroups.length-1; i>=0; i--) {
+                    var g = selectedGroups[i];
+                    removedGroups.push(g);
+                    RED.nodes.removeGroup(g);
                 }
                 if (removedSubflowOutputs.length > 0) {
                     result = RED.subflow.removeOutput(removedSubflowOutputs);