From 4ea112fcd3b9c49104685ee27786a1a837f1713c Mon Sep 17 00:00:00 2001
From: Maribeth Bottorff <maribethb@google.com>
Date: Fri, 25 Sep 2020 11:46:29 -0700
Subject: [PATCH] Don't undo the change events from disableOrphans (#4304)

---
 core/events.js            |  29 +++++++----
 tests/mocha/event_test.js | 102 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 120 insertions(+), 11 deletions(-)

diff --git a/core/events.js b/core/events.js
index e803af0ab..a15e99e2b 100644
--- a/core/events.js
+++ b/core/events.js
@@ -369,18 +369,25 @@ Blockly.Events.disableOrphans = function(event) {
     var workspace = Blockly.Workspace.getById(event.workspaceId);
     var block = workspace.getBlockById(event.blockId);
     if (block) {
-      var parent = block.getParent();
-      if (parent && parent.isEnabled()) {
-        var children = block.getDescendants(false);
-        for (var i = 0, child; (child = children[i]); i++) {
-          child.setEnabled(true);
+      // Changing blocks as part of this event shouldn't be undoable.
+      var initialUndoFlag = Blockly.Events.recordUndo;
+      try {
+        Blockly.Events.recordUndo = false;
+        var parent = block.getParent();
+        if (parent && parent.isEnabled()) {
+          var children = block.getDescendants(false);
+          for (var i = 0, child; (child = children[i]); i++) {
+            child.setEnabled(true);
+          }
+        } else if ((block.outputConnection || block.previousConnection) &&
+                  !workspace.isDragging()) {
+          do {
+            block.setEnabled(false);
+            block = block.getNextBlock();
+          } while (block);
         }
-      } else if ((block.outputConnection || block.previousConnection) &&
-                 !workspace.isDragging()) {
-        do {
-          block.setEnabled(false);
-          block = block.getNextBlock();
-        } while (block);
+      } finally {
+        Blockly.Events.recordUndo = initialUndoFlag;
       }
     }
   }
diff --git a/tests/mocha/event_test.js b/tests/mocha/event_test.js
index 77fd0c291..ea90c5bcf 100644
--- a/tests/mocha/event_test.js
+++ b/tests/mocha/event_test.js
@@ -897,4 +897,106 @@ suite('Events', function() {
       chai.assert.isNotNull(this.workspace.getVariableById(TEST_VAR_ID));
     });
   });
+  suite('Disable orphans', function() {
+    setup(function() {
+      // disableOrphans needs a WorkspaceSVG
+      var toolbox = document.getElementById('toolbox-categories');
+      this.workspace = Blockly.inject('blocklyDiv', {toolbox: toolbox});
+    });
+    teardown(function() {
+      workspaceTeardown.call(this, this.workspace);
+    });
+    test('Created orphan block is disabled', function() {
+      this.workspace.addChangeListener(Blockly.Events.disableOrphans);
+      var block = this.workspace.newBlock('controls_for');
+      block.initSvg();
+      block.render();
+
+      // Fire all events
+      this.clock.runAll();
+
+      chai.assert.isFalse(block.isEnabled(),
+          'Expected orphan block to be disabled after creation');
+    });
+    test('Created procedure block is enabled', function() {
+      this.workspace.addChangeListener(Blockly.Events.disableOrphans);
+
+      // Procedure block is never an orphan
+      var functionBlock = this.workspace.newBlock('procedures_defnoreturn');
+      functionBlock.initSvg();
+      functionBlock.render();
+
+      // Fire all events
+      this.clock.runAll();
+
+      chai.assert.isTrue(functionBlock.isEnabled(),
+          'Expected top-level procedure block to be enabled');
+    });
+    test('Moving a block to top-level disables it', function() {
+      this.workspace.addChangeListener(Blockly.Events.disableOrphans);
+      var functionBlock = this.workspace.newBlock('procedures_defnoreturn');
+      functionBlock.initSvg();
+      functionBlock.render();
+
+      var block = this.workspace.newBlock('controls_for');
+      block.initSvg();
+      block.render();
+
+      // Connect the block to the function block input stack
+      functionBlock.inputList[1].connection.connect(block.previousConnection);
+
+      // Disconnect it again
+      block.unplug(false);
+
+      // Fire all events
+      this.clock.runAll();
+
+      chai.assert.isFalse(block.isEnabled(),
+          'Expected disconnected block to be disabled');
+    });
+    test('Giving block a parent enables it', function() {
+      this.workspace.addChangeListener(Blockly.Events.disableOrphans);
+      var functionBlock = this.workspace.newBlock('procedures_defnoreturn');
+      functionBlock.initSvg();
+      functionBlock.render();
+
+      var block = this.workspace.newBlock('controls_for');
+      block.initSvg();
+      block.render();
+
+      // Connect the block to the function block input stack
+      functionBlock.inputList[1].connection.connect(block.previousConnection);
+
+      // Fire all events
+      this.clock.runAll();
+
+      chai.assert.isTrue(block.isEnabled(),
+          'Expected block to be enabled after connecting to parent');
+    });
+    test('disableOrphans events are not undoable', function() {
+      this.workspace.addChangeListener(Blockly.Events.disableOrphans);
+      var functionBlock = this.workspace.newBlock('procedures_defnoreturn');
+      functionBlock.initSvg();
+      functionBlock.render();
+
+      var block = this.workspace.newBlock('controls_for');
+      block.initSvg();
+      block.render();
+
+      // Connect the block to the function block input stack
+      functionBlock.inputList[1].connection.connect(block.previousConnection);
+
+      // Disconnect it again
+      block.unplug(false);
+
+      // Fire all events
+      this.clock.runAll();
+
+      var disabledEvents = this.workspace.getUndoStack().filter(function(e) {
+        return e.element === 'disabled';
+      });
+      chai.assert.isEmpty(disabledEvents,
+          'Undo stack should not contain any disabled events');
+    });
+  });
 });