From 736be75d7064d18b4437d7b32b587a428da9b16f Mon Sep 17 00:00:00 2001
From: Beka Westberg <bwestberg@google.com>
Date: Thu, 26 Aug 2021 18:56:02 +0000
Subject: [PATCH] fix: not being able to specifying variable names in toolbox
 (#5408)

* fix: not being able to specifying variable names in toolbox

* fix: id -> ID
---
 core/field_dropdown.js                  |  4 +-
 core/field_variable.js                  | 18 +++++--
 tests/mocha/field_variable_test.js      | 66 ++++++++++++++++++++++++-
 tests/mocha/jso_deserialization_test.js |  8 ++-
 4 files changed, 85 insertions(+), 11 deletions(-)

diff --git a/core/field_dropdown.js b/core/field_dropdown.js
index 0a1a1013a..c9671149e 100644
--- a/core/field_dropdown.js
+++ b/core/field_dropdown.js
@@ -172,12 +172,12 @@ FieldDropdown.prototype.fromXml = function(fieldElement) {
 
 /**
  * Saves this field's value.
- * @return {string} The dropdown value held by this field.
+ * @return {*} The dropdown value held by this field.
  * @override
  * @package
  */
 FieldDropdown.prototype.saveState = function() {
-  return /** @type {string} */ (this.getValue());
+  return this.getValue();
 };
 
 /**
diff --git a/core/field_variable.js b/core/field_variable.js
index eb67b0bf9..9cb95eff5 100644
--- a/core/field_variable.js
+++ b/core/field_variable.js
@@ -199,24 +199,32 @@ FieldVariable.prototype.toXml = function(fieldElement) {
 
 /**
  * Saves this field's value.
- * @return {string} The id of the variable referenced by this field.
+ * @return {{id: string}} The ID of the variable referenced by this field.
  * @override
  * @package
  */
 FieldVariable.prototype.saveState = function() {
   // Make sure the variable is initialized.
   this.initModel();
-  return this.variable_.getId();
+  return {
+    'id': this.variable_.getId()
+  };
 };
 
 /**
  * Sets the field's value based on the given state.
- * @param {*} id The id of the variable to assign to this variable field.
+ * @param {*} state The state of the variable to assign to this variable field.
  * @override
  * @package
  */
-FieldVariable.prototype.loadState = function(id) {
-  this.setValue(id);
+FieldVariable.prototype.loadState = function(state) {
+  // This is necessary so that blocks in the flyout can have custom var names.
+  const variable = Variables.getOrCreateVariablePackage(
+      this.sourceBlock_.workspace,
+      state['id'] || null,
+      state['name'],
+      state['type'] || '');
+  this.setValue(variable.getId());
 };
 
 /**
diff --git a/tests/mocha/field_variable_test.js b/tests/mocha/field_variable_test.js
index d12188f9a..5eb2a9ce0 100644
--- a/tests/mocha/field_variable_test.js
+++ b/tests/mocha/field_variable_test.js
@@ -399,7 +399,7 @@ suite('Variable Fields', function() {
       const field = new Blockly.FieldVariable('x');
       block.getInput('INPUT').appendField(field, 'VAR');
       const jso = Blockly.serialization.blocks.save(block);
-      chai.assert.deepEqual(jso['fields'], {'VAR': 'id2'});
+      chai.assert.deepEqual(jso['fields'], {'VAR': {'id': 'id2'}});
     });
 
     test('Typed', function() {
@@ -408,7 +408,69 @@ suite('Variable Fields', function() {
           new Blockly.FieldVariable('x', undefined, undefined, ['String']);
       block.getInput('INPUT').appendField(field, 'VAR');
       const jso = Blockly.serialization.blocks.save(block);
-      chai.assert.deepEqual(jso['fields'], {'VAR': 'id2'});
+      chai.assert.deepEqual(jso['fields'], {'VAR': {'id': 'id2'}});
+    });
+  });
+
+  suite('Deserialization', function() {
+    setup(function() {
+      this.workspace = new Blockly.Workspace();
+      defineRowBlock();
+      createGenUidStubWithReturns(new Array(10).fill().map((_, i) => 'id' + i));
+    });
+
+    teardown(function() {
+      workspaceTeardown.call(this, this.workspace);
+    });
+
+    test('ID', function() {
+      this.workspace.createVariable('test', '', 'id1');
+      var block = Blockly.serialization.blocks.load({
+        'type': 'variables_get',
+        'fields': {
+          'VAR': {
+            'id': 'id1'
+          }
+        }
+      },
+      this.workspace);
+      var variable = block.getField('VAR').getVariable();
+      chai.assert.equal(variable.name, 'test');
+      chai.assert.equal(variable.type, '');
+      chai.assert.equal(variable.getId(), 'id1');
+    });
+
+    test('Name, untyped', function() {
+      var block = Blockly.serialization.blocks.load({
+        'type': 'variables_get',
+        'fields': {
+          'VAR': {
+            'name': 'test',
+          }
+        }
+      },
+      this.workspace);
+      var variable = block.getField('VAR').getVariable();
+      chai.assert.equal(variable.name, 'test');
+      chai.assert.equal(variable.type, '');
+      chai.assert.equal(variable.getId(), 'id2');
+    });
+
+    test('Name, typed', function() {
+      var block = Blockly.serialization.blocks.load({
+        'type': 'variables_get',
+        'fields': {
+          'VAR': {
+            'name': 'test',
+            'type': 'string',
+          }
+        }
+      },
+      this.workspace);
+      var variable = block.getField('VAR').getVariable();
+      chai.assert.equal(variable.name, 'test');
+      chai.assert.equal(variable.type, 'string');
+      chai.assert.equal(variable.getId(), 'id2');
     });
   });
 });
diff --git a/tests/mocha/jso_deserialization_test.js b/tests/mocha/jso_deserialization_test.js
index 24d74fbf3..003499e5b 100644
--- a/tests/mocha/jso_deserialization_test.js
+++ b/tests/mocha/jso_deserialization_test.js
@@ -80,7 +80,9 @@ suite('JSO Deserialization', function() {
                 'x': 42,
                 'y': 42,
                 'fields': {
-                  'VAR': 'testId'
+                  'VAR': {
+                    'id': 'testId'
+                  }
                 }
               },
             ]
@@ -196,7 +198,9 @@ suite('JSO Deserialization', function() {
                 'x': 42,
                 'y': 42,
                 'fields': {
-                  'VAR': 'testId'
+                  'VAR': {
+                    'id': 'testId'
+                  }
                 }
               },
             ]