fix: json serialize lists_getIndex with json extraState (#6136) (#6170)

* fix: json serialize lists_getIndex with json extraState (#6136)

* address review comments

* fix: move block tests to mocha/block folder

blocks/lists.js

@@ -13,6 +13,7 @@
 goog.module('Blockly.libraryBlocks.lists');
 
 const xmlUtils = goog.require('Blockly.utils.xml');
+const Xml = goog.require('Blockly.Xml');
 const {Align} = goog.require('Blockly.Input');
 /* eslint-disable-next-line no-unused-vars */
 const {Block} = goog.requireType('Blockly.Block');
@@ -452,10 +453,34 @@ blocks['lists_getIndex'] = {
     this.updateAt_(isAt);
   },
 
-  // This block does not need JSO serialization hooks (saveExtraState and
+  /**
-  // loadExtraState) because the state of this object is already encoded in the
+   * Returns the state of this block as a JSON serializable object.
-  // dropdown values.
+   * Returns null for efficiency if no state is needed (not a statement)
-  // XML hooks are kept for backwards compatibility.
+   * @return {?{isStatement: boolean}} The state of this block, ie whether it's
+   *     a statement.
+   */
+  saveExtraState: function() {
+    if (!this.outputConnection) {
+      return {
+        'isStatement': true,
+      };
+    }
+    return null;
+  },
+
+  /**
+   * Applies the given state to this block.
+   * @param {*} state The state to apply to this block, ie whether it's a
+   *     statement.
+   */
+  loadExtraState: function(state) {
+    if (state['isStatement']) {
+      this.updateStatement_(true);
+    } else if (typeof state === 'string') {
+      // backward compatible for json serialised mutations
+      this.domToMutation(Xml.textToDom(state));
+    }
+  },
 
   /**
    * Switch between a value block and a statement block.

core/serialization/blocks.js

@@ -441,6 +441,9 @@ const loadAttributes = function(block, state) {
 /**
  * Applies any extra state information available on the state object to the
  * block.
+ * For json serialised blocks with a loadExtraState method, if the extraState
+ * is an xml mutation (not an object), domToMutation will be called instead for
+ * backward compatibility.
  * @param {!Block} block The block to set the extra state of.
  * @param {!State} state The state object to reference.
  */

tests/deps.js

@@ -1,6 +1,6 @@
 goog.addDependency('../../blocks/blocks.js', ['Blockly.libraryBlocks'], ['Blockly.libraryBlocks.colour', 'Blockly.libraryBlocks.lists', 'Blockly.libraryBlocks.logic', 'Blockly.libraryBlocks.loops', 'Blockly.libraryBlocks.math', 'Blockly.libraryBlocks.procedures', 'Blockly.libraryBlocks.texts', 'Blockly.libraryBlocks.variables', 'Blockly.libraryBlocks.variablesDynamic'], {'lang': 'es6', 'module': 'goog'});
 goog.addDependency('../../blocks/colour.js', ['Blockly.libraryBlocks.colour'], ['Blockly.FieldColour', 'Blockly.common'], {'lang': 'es6', 'module': 'goog'});
-goog.addDependency('../../blocks/lists.js', ['Blockly.libraryBlocks.lists'], ['Blockly.ConnectionType', 'Blockly.FieldDropdown', 'Blockly.FieldDropdown', 'Blockly.Input', 'Blockly.Msg', 'Blockly.Mutator', 'Blockly.common', 'Blockly.utils.xml'], {'lang': 'es6', 'module': 'goog'});
+goog.addDependency('../../blocks/lists.js', ['Blockly.libraryBlocks.lists'], ['Blockly.ConnectionType', 'Blockly.FieldDropdown', 'Blockly.FieldDropdown', 'Blockly.Input', 'Blockly.Msg', 'Blockly.Mutator', 'Blockly.Xml', 'Blockly.common', 'Blockly.utils.xml'], {'lang': 'es6', 'module': 'goog'});
 goog.addDependency('../../blocks/logic.js', ['Blockly.libraryBlocks.logic'], ['Blockly.Events', 'Blockly.Extensions', 'Blockly.FieldDropdown', 'Blockly.FieldLabel', 'Blockly.Msg', 'Blockly.Mutator', 'Blockly.common', 'Blockly.utils.xml'], {'lang': 'es6', 'module': 'goog'});
 goog.addDependency('../../blocks/loops.js', ['Blockly.libraryBlocks.loops'], ['Blockly.ContextMenu', 'Blockly.Events', 'Blockly.Extensions', 'Blockly.FieldDropdown', 'Blockly.FieldLabel', 'Blockly.FieldNumber', 'Blockly.FieldVariable', 'Blockly.Msg', 'Blockly.Variables', 'Blockly.Warning', 'Blockly.common', 'Blockly.utils.xml'], {'lang': 'es6', 'module': 'goog'});
 goog.addDependency('../../blocks/math.js', ['Blockly.libraryBlocks.math'], ['Blockly.Extensions', 'Blockly.FieldDropdown', 'Blockly.FieldLabel', 'Blockly.FieldNumber', 'Blockly.FieldVariable', 'Blockly.common', 'Blockly.utils.xml'], {'lang': 'es6', 'module': 'goog'});

tests/deps.mocha.js

@@ -4,6 +4,10 @@ goog.addDependency('../../tests/mocha/block_change_event_test.js', ['Blockly.tes
 goog.addDependency('../../tests/mocha/block_create_event_test.js', ['Blockly.test.blockCreateEvent'], ['Blockly.Events.utils', 'Blockly.test.helpers.events', 'Blockly.test.helpers.setupTeardown'], {'lang': 'es6', 'module': 'goog'});
 goog.addDependency('../../tests/mocha/block_json_test.js', ['Blockly.test.blockJson'], ['Blockly.Input'], {'lang': 'es6', 'module': 'goog'});
 goog.addDependency('../../tests/mocha/block_test.js', ['Blockly.test.blocks'], ['Blockly.ConnectionType', 'Blockly.Events.utils', 'Blockly.blocks', 'Blockly.test.helpers.blockDefinitions', 'Blockly.test.helpers.setupTeardown', 'Blockly.test.helpers.warnings'], {'lang': 'es6', 'module': 'goog'});
+goog.addDependency('../../tests/mocha/blocks/lists_test.js', ['Blockly.test.lists'], ['Blockly.ConnectionType', 'Blockly.test.helpers.blockDefinitions', 'Blockly.test.helpers.serialization', 'Blockly.test.helpers.setupTeardown'], {'lang': 'es6', 'module': 'goog'});
+goog.addDependency('../../tests/mocha/blocks/logic_ternary_test.js', ['Blockly.test.logicTernary'], ['Blockly.Events.utils', 'Blockly.test.helpers.serialization', 'Blockly.test.helpers.setupTeardown'], {'lang': 'es6', 'module': 'goog'});
+goog.addDependency('../../tests/mocha/blocks/procedures_test.js', ['Blockly.test.procedures'], ['Blockly', 'Blockly.Msg', 'Blockly.test.helpers.procedures', 'Blockly.test.helpers.serialization', 'Blockly.test.helpers.setupTeardown'], {'lang': 'es6', 'module': 'goog'});
+goog.addDependency('../../tests/mocha/blocks/variables_test.js', ['Blockly.test.variables'], ['Blockly.test.helpers.setupTeardown'], {'lang': 'es6', 'module': 'goog'});
 goog.addDependency('../../tests/mocha/comment_deserialization_test.js', ['Blockly.test.commentDeserialization'], ['Blockly.test.helpers.setupTeardown', 'Blockly.test.helpers.userInput'], {'lang': 'es6', 'module': 'goog'});
 goog.addDependency('../../tests/mocha/comment_test.js', ['Blockly.test.comments'], ['Blockly.Events.utils', 'Blockly.test.helpers.events', 'Blockly.test.helpers.setupTeardown'], {'lang': 'es6', 'module': 'goog'});
 goog.addDependency('../../tests/mocha/connection_checker_test.js', ['Blockly.test.connectionChecker'], ['Blockly.ConnectionType', 'Blockly.test.helpers.setupTeardown'], {'lang': 'es6', 'module': 'goog'});
@@ -36,11 +40,9 @@ goog.addDependency('../../tests/mocha/jso_deserialization_test.js', ['Blockly.te
 goog.addDependency('../../tests/mocha/jso_serialization_test.js', ['Blockly.test.jsoSerialization'], ['Blockly.test.helpers.blockDefinitions', 'Blockly.test.helpers.setupTeardown'], {'lang': 'es8', 'module': 'goog'});
 goog.addDependency('../../tests/mocha/json_test.js', ['Blockly.test.json'], ['Blockly.test.helpers.setupTeardown', 'Blockly.test.helpers.warnings'], {'lang': 'es6', 'module': 'goog'});
 goog.addDependency('../../tests/mocha/keydown_test.js', ['Blockly.test.keydown'], ['Blockly.test.helpers.blockDefinitions', 'Blockly.test.helpers.setupTeardown', 'Blockly.test.helpers.userInput'], {'lang': 'es6', 'module': 'goog'});
-goog.addDependency('../../tests/mocha/logic_ternary_test.js', ['Blockly.test.logicTernary'], ['Blockly.Events.utils', 'Blockly.test.helpers.serialization', 'Blockly.test.helpers.setupTeardown'], {'lang': 'es6', 'module': 'goog'});
 goog.addDependency('../../tests/mocha/metrics_test.js', ['Blockly.test.metrics'], ['Blockly.test.helpers.setupTeardown'], {'lang': 'es6', 'module': 'goog'});
 goog.addDependency('../../tests/mocha/mutator_test.js', ['Blockly.test.mutator'], ['Blockly.test.helpers.blockDefinitions', 'Blockly.test.helpers.setupTeardown'], {'lang': 'es6', 'module': 'goog'});
 goog.addDependency('../../tests/mocha/names_test.js', ['Blockly.test.names'], ['Blockly.test.helpers.setupTeardown'], {'lang': 'es6', 'module': 'goog'});
-goog.addDependency('../../tests/mocha/procedures_test.js', ['Blockly.test.procedures'], ['Blockly', 'Blockly.Msg', 'Blockly.test.helpers.procedures', 'Blockly.test.helpers.serialization', 'Blockly.test.helpers.setupTeardown'], {'lang': 'es6', 'module': 'goog'});
 goog.addDependency('../../tests/mocha/registry_test.js', ['Blockly.test.registry'], ['Blockly.test.helpers.setupTeardown', 'Blockly.test.helpers.warnings'], {'lang': 'es6', 'module': 'goog'});
 goog.addDependency('../../tests/mocha/run_mocha_tests_in_browser.js', [], [], {'lang': 'es8'});
 goog.addDependency('../../tests/mocha/serializer_test.js', ['Blockly.test.serialization'], ['Blockly.test.helpers.common', 'Blockly.test.helpers.setupTeardown'], {'lang': 'es6', 'module': 'goog'});
@@ -65,7 +67,6 @@ goog.addDependency('../../tests/mocha/trashcan_test.js', ['Blockly.test.trashcan
 goog.addDependency('../../tests/mocha/utils_test.js', ['Blockly.test.utils'], ['Blockly.test.helpers.setupTeardown'], {'lang': 'es6', 'module': 'goog'});
 goog.addDependency('../../tests/mocha/variable_map_test.js', ['Blockly.test.variableMap'], ['Blockly.test.helpers.setupTeardown', 'Blockly.test.helpers.variables'], {'lang': 'es6', 'module': 'goog'});
 goog.addDependency('../../tests/mocha/variable_model_test.js', ['Blockly.test.variableModel'], ['Blockly.test.helpers.setupTeardown'], {'lang': 'es6', 'module': 'goog'});
-goog.addDependency('../../tests/mocha/variables_test.js', ['Blockly.test.variables'], ['Blockly.test.helpers.setupTeardown'], {'lang': 'es6', 'module': 'goog'});
 goog.addDependency('../../tests/mocha/widget_div_test.js', ['Blockly.test.widgetDiv'], ['Blockly.test.helpers.setupTeardown'], {'lang': 'es6', 'module': 'goog'});
 goog.addDependency('../../tests/mocha/workspace_comment_test.js', ['Blockly.test.workspaceComment'], ['Blockly.WorkspaceComment', 'Blockly.test.helpers.setupTeardown'], {'lang': 'es6', 'module': 'goog'});
 goog.addDependency('../../tests/mocha/workspace_svg_test.js', ['Blockly.test.workspaceSvg'], ['Blockly.Events.utils', 'Blockly.test.helpers.blockDefinitions', 'Blockly.test.helpers.events', 'Blockly.test.helpers.setupTeardown', 'Blockly.test.helpers.variables', 'Blockly.test.helpers.workspace'], {'lang': 'es6', 'module': 'goog'});

tests/mocha/blocks/lists_test.js

@@ -0,0 +1,109 @@
+/**
+ * @license
+ * Copyright 2022 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+goog.module('Blockly.test.lists');
+
+const {runSerializationTestSuite} = goog.require('Blockly.test.helpers.serialization');
+const {sharedTestSetup, sharedTestTeardown} = goog.require('Blockly.test.helpers.setupTeardown');
+const {ConnectionType} = goog.require('Blockly.ConnectionType');
+const {defineStatementBlock} = goog.require('Blockly.test.helpers.blockDefinitions');
+
+
+suite('Lists', function() {
+  setup(function() {
+    defineStatementBlock();
+    sharedTestSetup.call(this);
+    this.workspace = new Blockly.Workspace();
+  });
+
+  teardown(function() {
+    sharedTestTeardown.call(this);
+  });
+
+  suite('ListsGetIndex', function() {
+    /**
+     * Test cases for serialization tests.
+     * @type {Array<SerializationTestCase>}
+     */
+    const testCases = [
+      {
+        title: 'JSON not requiring mutations',
+        json: {
+          type: 'lists_getIndex',
+          id: '1',
+          fields: {MODE: 'GET', WHERE: 'FIRST'},
+        },
+        assertBlockStructure: (block) => {
+          chai.assert.equal(block.type, 'lists_getIndex');
+          chai.assert.exists(block.outputConnection);
+        },
+      },
+      {
+        title: 'JSON requiring mutations',
+        json: {
+          type: 'lists_getIndex',
+          id: '1',
+          extraState: {isStatement: true},
+          fields: {MODE: 'REMOVE', WHERE: 'FROM_START'},
+        },
+        assertBlockStructure: (block) => {
+          chai.assert.equal(block.type, 'lists_getIndex');
+          chai.assert.isNotTrue(block.outputConnection);
+          chai.assert.isTrue(
+            block.getInput('AT').type === ConnectionType.INPUT_VALUE
+          );
+        },
+      },
+      {
+        title:
+          'JSON requiring mutations and extra state for previous connection',
+        json: {
+          type: 'statement_block',
+          id: '1',
+          next: {
+            block: {
+              type: 'lists_getIndex',
+              id: '2',
+              extraState: {isStatement: true},
+              fields: {MODE: 'REMOVE', WHERE: 'FROM_START'},
+            },
+          },
+        },
+        assertBlockStructure: (block) => {},
+      },
+      {
+        title:
+          'JSON requiring mutations with XML extra state',
+        json: {
+          type: 'statement_block',
+          id: '1',
+          next: {
+            block: {
+              type: 'lists_getIndex',
+              id: '2',
+              extraState: '<mutation statement="true" at="true"></mutation>',
+              fields: {MODE: 'REMOVE', WHERE: 'FROM_START'},
+            },
+          },
+        },
+        expectedJson: {
+          type: 'statement_block',
+          id: '1',
+          next: {
+            block: {
+              type: 'lists_getIndex',
+              id: '2',
+              extraState: {isStatement: true},
+              fields: {MODE: 'REMOVE', WHERE: 'FROM_START'},
+            },
+          },
+        },
+        assertBlockStructure: (block) => {},
+      },
+    ];
+    runSerializationTestSuite(testCases);
+  });
+});

tests/mocha/index.html

@@ -91,6 +91,7 @@
       goog.require('Blockly.test.jsoSerialization');
       goog.require('Blockly.test.json');
       goog.require('Blockly.test.keydown');
+      goog.require('Blockly.test.lists');
       goog.require('Blockly.test.logicTernary');
       goog.require('Blockly.test.metrics');
       goog.require('Blockly.test.mutator');