Merge pull request #1165 from marisaleung/develop_checkForDeleteVariable

In DropdownCreate check for Msg.DELETE_VARIABLE.
2026-01-09 18:10:08 +01:00 · 2017-06-09 16:26:03 -07:00
parent bb3ebfa93e d852a2b116
commit d8eb2fb4f4
6 changed files with 50 additions and 103 deletions
--- a/core/field_variable.js
+++ b/core/field_variable.js
@@ -169,8 +169,10 @@ Blockly.FieldVariable.dropdownCreate = function() {
    options[i] = [variableModelList[i].name, variableModelList[i].getId()];
  }
  options.push([Blockly.Msg.RENAME_VARIABLE, Blockly.RENAME_VARIABLE_ID]);
-  options.push([Blockly.Msg.DELETE_VARIABLE.replace('%1', name),
+  if (Blockly.Msg.DELETE_VARIABLE) {
+    options.push([Blockly.Msg.DELETE_VARIABLE.replace('%1', name),
      Blockly.DELETE_VARIABLE_ID]);
+  }
  return options;
 };

--- a/tests/jsunit/event_test.js
+++ b/tests/jsunit/event_test.js
@@ -28,7 +28,6 @@ goog.require('goog.testing');
 goog.require('goog.testing.MockControl');

 var mockControl_;
-var saved_msg = Blockly.Msg.DELETE_VARIABLE;
 var workspace;

 function eventTest_setUp() {
@@ -49,11 +48,6 @@ function eventTest_setUpWithMockBlocks() {
      }
    ],
  }]);
-  // Need to define this because field_variable's dropdownCreate() calls replace
-  // on undefined value, Blockly.Msg.DELETE_VARIABLE. To fix this, define
-  // Blockly.Msg.DELETE_VARIABLE as %1 so the replace function finds the %1 it
-  // expects.
-  Blockly.Msg.DELETE_VARIABLE = '%1';
 }

 function eventTest_tearDown() {
@@ -64,7 +58,6 @@ function eventTest_tearDown() {
 function eventTest_tearDownWithMockBlocks() {
  eventTest_tearDown();
  delete Blockly.Blocks.field_variable_test_block;
-  Blockly.Msg.DELETE_VARIABLE = saved_msg;
 }

 function test_abstract_constructor_block() {
--- a/tests/jsunit/field_variable_test.js
+++ b/tests/jsunit/field_variable_test.js
@@ -19,67 +19,55 @@
 */

 /**
- * @fileoverview Tests for Blockly.Field
+ * @fileoverview Tests for Blockly.FieldVariable
 * @author marisaleung@google.com (Marisa Leung)
 */
 'use strict';

 goog.require('goog.testing');

-var workspace;
-var saved_msg = Blockly.Msg.DELETE_VARIABLE;

-function fieldVariable_setUp() {
-  Blockly.Msg.DELETE_VARIABLE = 'Delete the "%1" variable';
-  workspace = new Blockly.Workspace();
-}
-
-function fieldVariable_tearDown() {
-  workspace.dispose();
-  Blockly.Msg.DELETE_VARIABLE = saved_msg;
-}
-
-function fieldVariable_mockBlock() {
+function fieldVariable_mockBlock(workspace) {
  return {'workspace': workspace, 'isShadow': function(){return false;}};
 }

 function test_fieldVariable_Constructor() {
-  fieldVariable_setUp();
+  var workspace = new Blockly.Workspace();
  var fieldVariable = new Blockly.FieldVariable('name1');
  assertEquals('name1', fieldVariable.getText());
-  fieldVariable_tearDown();
+  workspace.dispose();
 }

 function test_fieldVariable_setValueMatchId() {
  // Expect the fieldVariable value to be set to variable name
-  fieldVariable_setUp();
+  var workspace = new Blockly.Workspace();
  workspace.createVariable('name2', null, 'id1');
  var fieldVariable = new Blockly.FieldVariable('name1');
-  var mockBlock = fieldVariable_mockBlock();
+  var mockBlock = fieldVariable_mockBlock(workspace);
  fieldVariable.setSourceBlock(mockBlock);
  fieldVariable.setValue('id1');
  assertEquals('name2', fieldVariable.getText());
  assertEquals('id1', fieldVariable.value_);
-  fieldVariable_tearDown();
+  workspace.dispose();
 }

 function test_fieldVariable_setValueMatchName() {
  // Expect the fieldVariable value to be set to variable name
-  fieldVariable_setUp();
+  var workspace = new Blockly.Workspace();
  workspace.createVariable('name2', null, 'id2');
  var fieldVariable = new Blockly.FieldVariable('name1');
-  var mockBlock = fieldVariable_mockBlock();
+  var mockBlock = fieldVariable_mockBlock(workspace);
  fieldVariable.setSourceBlock(mockBlock);
  fieldVariable.setValue('name2');
  assertEquals('name2', fieldVariable.getText());
  assertEquals('id2', fieldVariable.value_);
-  fieldVariable_tearDown();
+  workspace.dispose();
 }

 function test_fieldVariable_setValueNoVariable() {
  // Expect the fieldVariable value to be set to the passed in string. No error
  // should be thrown.
-  fieldVariable_setUp();
+  var workspace = new Blockly.Workspace();
  var fieldVariable = new Blockly.FieldVariable('name1');
  var mockBlock = {'workspace': workspace,
    'isShadow': function(){return false;}};
@@ -87,5 +75,5 @@ function test_fieldVariable_setValueNoVariable() {
  fieldVariable.setValue('id1');
  assertEquals('id1', fieldVariable.getText());
  assertEquals('id1', fieldVariable.value_);
-  fieldVariable_tearDown();
+  workspace.dispose();
 }
--- a/tests/jsunit/workspace_test.js
+++ b/tests/jsunit/workspace_test.js
@@ -24,7 +24,6 @@ goog.require('goog.testing.MockControl');

 var workspace;
 var mockControl_;
-var saved_msg = Blockly.Msg.DELETE_VARIABLE;
 Blockly.defineBlocksWithJsonArray([{
  "type": "get_var_block",
  "message0": "%1",
@@ -41,25 +40,11 @@ function workspaceTest_setUp() {
  mockControl_ = new goog.testing.MockControl();
 }

-function workspaceTest_setUpWithMockBlocks() {
-  workspaceTest_setUp();
-  // Need to define this because field_variable's dropdownCreate() calls replace
-  // on undefined value, Blockly.Msg.DELETE_VARIABLE. To fix this, define
-  // Blockly.Msg.DELETE_VARIABLE as %1 so the replace function finds the %1 it
-  // expects.
-  Blockly.Msg.DELETE_VARIABLE = '%1';
-}
-
 function workspaceTest_tearDown() {
  mockControl_.$tearDown();
  workspace.dispose();
 }

-function workspaceTest_tearDownWithMockBlocks() {
-  workspaceTest_tearDown();
-  Blockly.Msg.DELETE_VARIABLE = saved_msg;
-}
-
 /**
 * Create a test get_var_block.
 * @param {?string} variable_name The string to put into the variable field.
@@ -173,7 +158,7 @@ function test_getBlockById() {
 }

 function test_deleteVariable_InternalTrivial() {
-  workspaceTest_setUpWithMockBlocks();
+  workspaceTest_setUp();
  var var_1 = workspace.createVariable('name1', 'type1', 'id1');
  workspace.createVariable('name2', 'type2', 'id2');
  createMockBlock('name1');
@@ -186,7 +171,7 @@ function test_deleteVariable_InternalTrivial() {
  assertNull(variable);
  checkVariableValues(workspace, 'name2', 'type2', 'id2');
  assertEquals('name2', block_var_name);
-  workspaceTest_tearDownWithMockBlocks();
+  workspaceTest_tearDown();
 }

 // TODO(marisaleung): Test the alert for deleting a variable that is a procedure.
@@ -259,7 +244,7 @@ function test_updateVariableStore_ClearAndOneInUse() {
 }

 function test_addTopBlock_TrivialFlyoutIsTrue() {
-  workspaceTest_setUpWithMockBlocks();
+  workspaceTest_setUp();
  workspace.isFlyout = true;
  var block = createMockBlock();
  workspace.removeTopBlock(block);
@@ -272,7 +257,7 @@ function test_addTopBlock_TrivialFlyoutIsTrue() {
    checkVariableValues(workspace, 'name1', '', '1');
  }
  finally {
-    workspaceTest_tearDownWithMockBlocks();
+    workspaceTest_tearDown();
  }
 }

@@ -335,18 +320,18 @@ function test_renameVariable_NoBlocks() {

 function test_renameVariable_SameNameNoBlocks() {
  // Expect 'renameVariable' to create new variable with newName.
-  workspaceTest_setUpWithMockBlocks();
+  workspaceTest_setUp();
  var name = 'name1';
  workspace.createVariable(name, 'type1', 'id1');

  workspace.renameVariable(name, name);
  checkVariableValues(workspace, name, 'type1', 'id1');
-  workspaceTest_tearDownWithMockBlocks();
+  workspaceTest_tearDown();
 }

 function test_renameVariable_OnlyOldNameBlockExists() {
  // Expect 'renameVariable' to change oldName variable name to newName.
-  workspaceTest_setUpWithMockBlocks();
+  workspaceTest_setUp();
  var oldName = 'name1';
  var newName = 'name2';
  workspace.createVariable(oldName, 'type1', 'id1');
@@ -358,13 +343,13 @@ function test_renameVariable_OnlyOldNameBlockExists() {
  var block_var_name = workspace.topBlocks_[0].getVars()[0];
  assertNull(variable);
  assertEquals(newName, block_var_name);
-  workspaceTest_tearDownWithMockBlocks();
+  workspaceTest_tearDown();
 }

 function test_renameVariable_TwoVariablesSameType() {
  // Expect 'renameVariable' to change oldName variable name to newName.
  // Expect oldName block name to change to newName
-  workspaceTest_setUpWithMockBlocks();
+  workspaceTest_setUp();
  var oldName = 'name1';
  var newName = 'name2';
  workspace.createVariable(oldName, 'type1', 'id1');
@@ -380,12 +365,12 @@ function test_renameVariable_TwoVariablesSameType() {
  assertNull(variable);
  assertEquals(newName, block_var_name_1);
  assertEquals(newName, block_var_name_2);
-  workspaceTest_tearDownWithMockBlocks();
+  workspaceTest_tearDown();
 }

 function test_renameVariable_TwoVariablesDifferentType() {
  // Expect triggered error because of different types
-  workspaceTest_setUpWithMockBlocks();
+  workspaceTest_setUp();
  var oldName = 'name1';
  var newName = 'name2';
  workspace.createVariable(oldName, 'type1', 'id1');
@@ -405,12 +390,12 @@ function test_renameVariable_TwoVariablesDifferentType() {
  var block_var_name_2 = workspace.topBlocks_[1].getVars()[0];
  assertEquals(oldName, block_var_name_1);
  assertEquals(newName, block_var_name_2);
-  workspaceTest_tearDownWithMockBlocks();
+  workspaceTest_tearDown();
 }

 function test_renameVariable_OldCase() {
  // Expect triggered error because of different types
-  workspaceTest_setUpWithMockBlocks();
+  workspaceTest_setUp();
  var oldCase = 'Name1';
  var newName = 'name1';
  workspace.createVariable(oldCase, 'type1', 'id1');
@@ -420,12 +405,12 @@ function test_renameVariable_OldCase() {
  checkVariableValues(workspace, newName, 'type1', 'id1');
  var result_oldCase = workspace.getVariable(oldCase).name;
  assertNotEquals(oldCase, result_oldCase);
-  workspaceTest_tearDownWithMockBlocks();
+  workspaceTest_tearDown();
 }

 function test_renameVariable_TwoVariablesAndOldCase() {
  // Expect triggered error because of different types
-  workspaceTest_setUpWithMockBlocks();
+  workspaceTest_setUp();
  var oldName = 'name1';
  var oldCase = 'Name2';
  var newName = 'name2';
@@ -445,7 +430,7 @@ function test_renameVariable_TwoVariablesAndOldCase() {
  assertNotEquals(oldCase, result_oldCase);
  assertEquals(newName, block_var_name_1);
  assertEquals(newName, block_var_name_2);
-  workspaceTest_tearDownWithMockBlocks();
+  workspaceTest_tearDown();
 }

 // Extra testing not required for renameVariableById. It calls renameVariable
@@ -453,7 +438,7 @@ function test_renameVariable_TwoVariablesAndOldCase() {
 function test_renameVariableById_TwoVariablesSameType() {
  // Expect 'renameVariableById' to change oldName variable name to newName.
  // Expect oldName block name to change to newName
-  workspaceTest_setUpWithMockBlocks();
+  workspaceTest_setUp();
  var oldName = 'name1';
  var newName = 'name2';
  workspace.createVariable(oldName, 'type1', 'id1');
@@ -469,11 +454,11 @@ function test_renameVariableById_TwoVariablesSameType() {
  assertNull(variable);
  assertEquals(newName, block_var_name_1);
  assertEquals(newName, block_var_name_2);
-  workspaceTest_tearDownWithMockBlocks();
+  workspaceTest_tearDown();
 }

 function test_deleteVariable_Trivial() {
-  workspaceTest_setUpWithMockBlocks();
+  workspaceTest_setUp();
  workspace.createVariable('name1', 'type1', 'id1');
  workspace.createVariable('name2', 'type1', 'id2');
  createMockBlock('name1');
@@ -485,11 +470,11 @@ function test_deleteVariable_Trivial() {
  var block_var_name = workspace.topBlocks_[0].getVars()[0];
  assertNull(variable);
  assertEquals('name2', block_var_name);
-  workspaceTest_tearDownWithMockBlocks();
+  workspaceTest_tearDown();
 }

 function test_deleteVariableById_Trivial() {
-  workspaceTest_setUpWithMockBlocks();
+  workspaceTest_setUp();
  workspace.createVariable('name1', 'type1', 'id1');
  workspace.createVariable('name2', 'type1', 'id2');
  createMockBlock('name1');
@@ -501,5 +486,5 @@ function test_deleteVariableById_Trivial() {
  var block_var_name = workspace.topBlocks_[0].getVars()[0];
  assertNull(variable);
  assertEquals('name2', block_var_name);
-  workspaceTest_tearDownWithMockBlocks();
+  workspaceTest_tearDown();
 }
--- a/tests/jsunit/workspace_undo_redo_test.js
+++ b/tests/jsunit/workspace_undo_redo_test.js
@@ -29,9 +29,9 @@ goog.require('goog.testing');
 goog.require('goog.testing.events');
 goog.require('goog.testing.MockControl');

+
 var workspace;
 var mockControl_;
-var savedMsg = Blockly.Msg.DELETE_VARIABLE;
 var savedFireFunc = Blockly.Events.fire;
 Blockly.defineBlocksWithJsonArray([{
  "type": "get_var_block",
@@ -58,26 +58,12 @@ function undoRedoTest_setUp() {
  Blockly.Events.fire = temporary_fireEvent;
 }

-function undoRedoTest_setUpWithMockBlocks() {
-  undoRedoTest_setUp();
-  // Need to define this because field_variable's dropdownCreate() calls replace
-  // on undefined value, Blockly.Msg.DELETE_VARIABLE. To fix this, define
-  // Blockly.Msg.DELETE_VARIABLE as %1 so the replace function finds the %1 it
-  // expects.
-  Blockly.Msg.DELETE_VARIABLE = '%1';
-}
-
 function undoRedoTest_tearDown() {
  mockControl_.$tearDown();
  workspace.dispose();
  Blockly.Events.fire = savedFireFunc;
 }

-function undoRedoTest_tearDownWithMockBlocks() {
-  undoRedoTest_tearDown();
-  Blockly.Msg.DELETE_VARIABLE = savedMsg;
-}
-
 /**
 * Create a test get_var_block.
 * @param {string} variableName The string to put into the variable field.
@@ -159,7 +145,7 @@ function test_undoDeleteVariable_NoBlocks() {
 }

 function test_undoDeleteVariable_WithBlocks() {
-  undoRedoTest_setUpWithMockBlocks();
+  undoRedoTest_setUp();
  workspace.createVariable('name1', 'type1', 'id1');
  workspace.createVariable('name2', 'type2', 'id2');
  createMockBlock('name1');
@@ -177,7 +163,7 @@ function test_undoDeleteVariable_WithBlocks() {
  undoRedoTest_checkBlockVariableName(1, 'name1');
  checkVariableValues(workspace, 'name1', 'type1', 'id1');
  checkVariableValues(workspace, 'name2', 'type2', 'id2');
-  undoRedoTest_tearDownWithMockBlocks();
+  undoRedoTest_tearDown();
 }

 function test_redoAndUndoDeleteVariable() {
@@ -203,7 +189,7 @@ function test_redoAndUndoDeleteVariable() {
 }

 function test_redoAndUndoDeleteVariableWithBlocks() {
-  undoRedoTest_setUpWithMockBlocks();
+  undoRedoTest_setUp();
  workspace.createVariable('name1', 'type1', 'id1');
  workspace.createVariable('name2', 'type2', 'id2');
  createMockBlock('name1');
@@ -225,7 +211,7 @@ function test_redoAndUndoDeleteVariableWithBlocks() {
  undoRedoTest_checkBlockVariableName(0, 'name2');
  assertNull(workspace.getVariableById('id1'));
  checkVariableValues(workspace, 'name2', 'type2', 'id2');
-  undoRedoTest_tearDownWithMockBlocks();
+  undoRedoTest_tearDown();
 }

 function test_undoRedoRenameVariable_NeitherVariableExists() {
@@ -260,7 +246,7 @@ function test_undoRedoRenameVariable_OneExists_NoBlocks() {
 }

 function test_undoRedoRenameVariable_OneExists_WithBlocks() {
-  undoRedoTest_setUpWithMockBlocks();
+  undoRedoTest_setUp();
  workspace.createVariable('name1', '', 'id1');
  createMockBlock('name1');
  workspace.renameVariable('name1', 'name2');
@@ -273,7 +259,7 @@ function test_undoRedoRenameVariable_OneExists_WithBlocks() {
  workspace.undo(true);
  checkVariableValues(workspace, 'name2', '', 'id1');
  undoRedoTest_checkBlockVariableName(0, 'name2');
-  undoRedoTest_tearDownWithMockBlocks();
+  undoRedoTest_tearDown();
 }

 function test_undoRedoRenameVariable_BothExist_NoBlocks() {
@@ -292,7 +278,7 @@ function test_undoRedoRenameVariable_BothExist_NoBlocks() {
 }

 function test_undoRedoRenameVariable_BothExist_WithBlocks() {
-  undoRedoTest_setUpWithMockBlocks();
+  undoRedoTest_setUp();
  createTwoVarsEmptyType();
  createMockBlock('name1');
  createMockBlock('name2');
@@ -307,7 +293,7 @@ function test_undoRedoRenameVariable_BothExist_WithBlocks() {
  workspace.undo(true);
  undoRedoTest_checkBlockVariableName(0, 'name2');
  undoRedoTest_checkBlockVariableName(1, 'name2');
-  undoRedoTest_tearDownWithMockBlocks();
+  undoRedoTest_tearDown();
 }

 function test_undoRedoRenameVariable_BothExistCaseChange_NoBlocks() {
@@ -326,7 +312,7 @@ function test_undoRedoRenameVariable_BothExistCaseChange_NoBlocks() {
 }

 function test_undoRedoRenameVariable_BothExistCaseChange_WithBlocks() {
-  undoRedoTest_setUpWithMockBlocks();
+  undoRedoTest_setUp();
  createTwoVarsEmptyType();
  createMockBlock('name1');
  createMockBlock('name2');
@@ -343,7 +329,7 @@ function test_undoRedoRenameVariable_BothExistCaseChange_WithBlocks() {
  assertNull(workspace.getVariable('name1'));
  undoRedoTest_checkBlockVariableName(0, 'Name2');
  undoRedoTest_checkBlockVariableName(1, 'Name2');
-  undoRedoTest_tearDownWithMockBlocks();
+  undoRedoTest_tearDown();
 }

 function test_undoRedoRenameVariable_OnlyCaseChange_NoBlocks() {
@@ -360,7 +346,7 @@ function test_undoRedoRenameVariable_OnlyCaseChange_NoBlocks() {
 }

 function test_undoRedoRenameVariable_OnlyCaseChange_WithBlocks() {
-  undoRedoTest_setUpWithMockBlocks();
+  undoRedoTest_setUp();
  workspace.createVariable('name1', '', 'id1');
  createMockBlock('name1');
  workspace.renameVariable('name1', 'Name1');
@@ -372,5 +358,5 @@ function test_undoRedoRenameVariable_OnlyCaseChange_WithBlocks() {
  workspace.undo(true);
  checkVariableValues(workspace, 'Name1', '', 'id1');
  undoRedoTest_checkBlockVariableName(0, 'Name1');
-  undoRedoTest_tearDownWithMockBlocks();
+  undoRedoTest_tearDown();
 }
--- a/tests/jsunit/xml_test.js
+++ b/tests/jsunit/xml_test.js
@@ -23,7 +23,6 @@ goog.require('goog.testing');
 goog.require('goog.testing.MockControl');

 var mockControl_;
-var saved_msg = Blockly.Msg.DELETE_VARIABLE;
 var workspace;
 var XML_TEXT = ['<xml xmlns="http://www.w3.org/1999/xhtml">',
  '  <block type="controls_repeat_ext" inline="true" x="21" y="23">',
@@ -70,11 +69,6 @@ function xmlTest_setUpWithMockBlocks() {
      }
    ],
  }]);
-  // Need to define this because field_variable's dropdownCreate() calls replace
-  // on undefined value, Blockly.Msg.DELETE_VARIABLE. To fix this, define
-  // Blockly.Msg.DELETE_VARIABLE as %1 so the replace function finds the %1 it
-  // expects.
-  Blockly.Msg.DELETE_VARIABLE = '%1';
 }

 function xmlTest_tearDown() {
@@ -85,7 +79,6 @@ function xmlTest_tearDown() {
 function xmlTest_tearDownWithMockBlocks() {
  xmlTest_tearDown();
  delete Blockly.Blocks.field_variable_test_block;
-  Blockly.Msg.DELETE_VARIABLE = saved_msg;
 }

 /**