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

Author: gregdyke

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
-  // dropdown values.
-  // XML hooks are kept for backwards compatibility.
+  /**
+   * Returns the state of this block as a JSON serializable object.
+   * Returns null for efficiency if no state is needed (not a statement)
+   * @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

@@ -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/deps.mocha.js

@@ -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');