fix: json hooks for lists blocks not needing extra state (google#6177)

rachel-fenichel · Jun 3, 2022 · 8b69b61 · 8b69b61
1 parent 87aa4c0
commit 8b69b61
Show file tree

Hide file tree

Showing 2 changed files with 136 additions and 12 deletions.
diff --git a/blocks/lists.js b/blocks/lists.js
@@ -649,10 +649,23 @@ blocks['lists_setIndex'] = {
     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.
+   * This block does not need to serialize any specific state as it is already
+   * encoded in the dropdown values, but must have an implementation to avoid
+   * the backward compatible XML mutations being serialized.
+   * @return {null} The state of this block.
+   */
+  saveExtraState: function() {
+    return null;
+  },
+
+  /**
+   * Applies the given state to this block.
+   * No extra state is needed or expected as it is already encoded in the
+   * dropdown values.
+   */
+  loadExtraState: function() {},
 
   /**
    * Create or delete an input for the numeric index.
@@ -761,10 +774,23 @@ blocks['lists_getSublist'] = {
     this.updateAt_(2, isAt2);
   },
 
-  // 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.
+   * This block does not need to serialize any specific state as it is already
+   * encoded in the dropdown values, but must have an implementation to avoid
+   * the backward compatible XML mutations being serialized.
+   * @return {null} The state of this block.
+   */
+  saveExtraState: function() {
+    return null;
+  },
+
+  /**
+   * Applies the given state to this block.
+   * No extra state is needed or expected as it is already encoded in the
+   * dropdown values.
+   */
+  loadExtraState: function() {},
 
   /**
    * Create or delete an input for a numeric index.
@@ -945,10 +971,23 @@ blocks['lists_split'] = {
     this.updateType_(xmlElement.getAttribute('mode'));
   },
 
-  // 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.
+   * This block does not need to serialize any specific state as it is already
+   * encoded in the dropdown values, but must have an implementation to avoid
+   * the backward compatible XML mutations being serialized.
+   * @return {null} The state of this block.
+   */
+  saveExtraState: function() {
+    return null;
+  },
+
+  /**
+   * Applies the given state to this block.
+   * No extra state is needed or expected as it is already encoded in the
+   * dropdown values.
+   */
+  loadExtraState: function() {},
 };
 
 // Register provided blocks.

diff --git a/tests/mocha/blocks/lists_test.js b/tests/mocha/blocks/lists_test.js
@@ -106,4 +106,89 @@ suite('Lists', function() {
     ];
     runSerializationTestSuite(testCases);
   });
+
+  /**
+   * Test cases for serialization where JSON hooks should have null
+   * implementation to avoid serializing xml mutations in json.
+   * @param {!Object} serializedJson basic serialized json
+   * @param {!string} xmlMutation xml mutation that should be ignored/not reserialized in round trip
+   * @return {Array<SerializationTestCase>} test cases
+   */
+  function makeTestCasesForBlockNotNeedingExtraState_(serializedJson, xmlMutation) {
+    return [
+      {
+        title: 'JSON not requiring mutations',
+        json: serializedJson,
+        assertBlockStructure: (block) => {
+          chai.assert.equal(block.type, serializedJson.type);
+        },
+      },
+      {
+        title:
+          'JSON with XML extra state',
+        json: {
+          ...serializedJson,
+          "extraState": xmlMutation,
+        },
+        expectedJson: serializedJson,
+        assertBlockStructure: (block) => {},
+      },
+    ];
+  }
+
+  suite('ListsSetIndex', function() {
+    /**
+     * Test cases for serialization tests.
+     * @type {Array<SerializationTestCase>}
+     */
+    const testCases = makeTestCasesForBlockNotNeedingExtraState_(
+      {
+        "type": "lists_setIndex",
+        "id": "1",
+        "fields": {
+          "MODE": "SET",
+          "WHERE": "FROM_START",
+        },
+      },
+      "<mutation at=\"true\"></mutation>"
+    );
+    runSerializationTestSuite(testCases);
+  });
+
+  suite('ListsGetSubList', function() {
+    /**
+     * Test cases for serialization tests.
+     * @type {Array<SerializationTestCase>}
+     */
+     const testCases = makeTestCasesForBlockNotNeedingExtraState_(
+      {
+        "type": "lists_getSublist",
+        "id": "1",
+        "fields": {
+          "WHERE1": "FROM_START",
+          "WHERE2": "FROM_START",
+        },
+      },
+      "<mutation at1=\"true\" at2=\"true\"></mutation>"
+    );
+    runSerializationTestSuite(testCases);
+  });
+
+  suite('ListsSplit', function() {
+    /**
+     * Test cases for serialization tests.
+     * @type {Array<SerializationTestCase>}
+     */
+    const testCases = makeTestCasesForBlockNotNeedingExtraState_(
+      {
+        "type": "lists_split",
+        "id": "1",
+        "fields": {
+          "MODE": "SPLIT",
+        },
+      },
+      "<mutation mode=\"SPLIT\"></mutation>"
+    );
+    runSerializationTestSuite(testCases);
+  });
 });