diff --git a/core/variable_map.ts b/core/variable_map.ts index f2dd95edb..ecba4b87f 100644 --- a/core/variable_map.ts +++ b/core/variable_map.ts @@ -59,6 +59,7 @@ export class VariableMap { * @internal */ renameVariable(variable: VariableModel, newName: string) { + if (variable.name === newName) return; const type = variable.type; const conflictVar = this.getVariable(newName, type); const blocks = this.workspace.getAllBlocks(false); @@ -184,6 +185,8 @@ export class VariableMap { this.variableMap.delete(type); this.variableMap.set(type, variables); + eventUtils.fire(new (eventUtils.get(eventUtils.VAR_CREATE))(variable)); + return variable; } /* Begin functions for variable deletion. */ diff --git a/core/variable_model.ts b/core/variable_model.ts index cd404d316..cfd832a90 100644 --- a/core/variable_model.ts +++ b/core/variable_model.ts @@ -15,7 +15,6 @@ goog.declareModuleId('Blockly.VariableModel'); // Unused import preserved for side-effects. Remove if unneeded. import './events/events_var_create.js'; -import * as eventUtils from './events/utils.js'; import * as idGenerator from './utils/idgenerator.js'; import type {Workspace} from './workspace.js'; @@ -58,8 +57,6 @@ export class VariableModel { * UUID. */ this.id_ = opt_id || idGenerator.genUid(); - - eventUtils.fire(new (eventUtils.get(eventUtils.VAR_CREATE))(this)); } /** @returns The ID for the variable. */ diff --git a/tests/mocha/variable_map_test.js b/tests/mocha/variable_map_test.js index a8bec4a1f..2efb3afbd 100644 --- a/tests/mocha/variable_map_test.js +++ b/tests/mocha/variable_map_test.js @@ -8,6 +8,7 @@ goog.declareModuleId('Blockly.test.variableMap'); import {assertVariableValues} from './test_helpers/variables.js'; import {createGenUidStubWithReturns, sharedTestSetup, sharedTestTeardown} from './test_helpers/setup_teardown.js'; +import {assertEventFired, assertEventNotFired, createChangeListenerSpy} from './test_helpers/events.js'; suite('Variable Map', function() { @@ -253,4 +254,203 @@ suite('Variable Map', function() { chai.assert.deepEqual(resultArray, []); }); }); + + suite('event firing', function() { + setup(function() { + this.eventSpy = createChangeListenerSpy(this.workspace); + }); + + teardown(function() { + this.workspace.removeChangeListener(this.eventSpy); + }); + + suite('variable create events', function() { + test('create events are fired when a variable is created', function() { + this.variableMap.createVariable('test name', 'test type', 'test id'); + + assertEventFired( + this.eventSpy, + Blockly.Events.VarCreate, + { + varType: 'test type', + varName: 'test name', + varId: 'test id', + }, + this.workspace.id); + }); + + test( + 'create events are not fired if a variable is already exists', + function() { + this.variableMap.createVariable('test name', 'test type', 'test id'); + + this.eventSpy.resetHistory(); + this.variableMap.createVariable('test name', 'test type', 'test id'); + + assertEventNotFired( + this.eventSpy, + Blockly.Events.VarCreate, + {}, + this.workspace.id); + }); + }); + + suite('variable delete events', function() { + suite('deleting with a variable', function() { + test('delete events are fired when a variable is deleted', function() { + const variable = + this.variableMap.createVariable('test name', 'test type', 'test id'); + this.variableMap.deleteVariable(variable); + + assertEventFired( + this.eventSpy, + Blockly.Events.VarDelete, + { + varType: 'test type', + varName: 'test name', + varId: 'test id', + }, + this.workspace.id); + }); + + test( + 'delete events are not fired when a variable does not exist', + function() { + const variable = + new Blockly.VariableModel( + this.workspace, 'test name', 'test type', 'test id'); + this.variableMap.deleteVariable(variable); + + assertEventNotFired( + this.eventSpy, + Blockly.Events.VarDelete, + {}, + this.workspace.id); + }); + }); + + suite('deleting by ID', function() { + test( + 'delete events are fired when a variable is deleted', + function() { + this.variableMap.createVariable('test name', 'test type', 'test id'); + this.variableMap.deleteVariableById('test id'); + + assertEventFired( + this.eventSpy, + Blockly.Events.VarDelete, + { + varType: 'test type', + varName: 'test name', + varId: 'test id', + }, + this.workspace.id); + }); + + test( + 'delete events are not fired when a variable does not exist', + function() { + this.variableMap.deleteVariableById('test id'); + + assertEventNotFired( + this.eventSpy, + Blockly.Events.VarDelete, + {}, + this.workspace.id); + }); + }); + }); + + suite('variable rename events', function() { + suite('renaming with variable', function() { + test('rename events are fired when a variable is renamed', function() { + const variable = + this.variableMap.createVariable( + 'test name', 'test type', 'test id'); + this.variableMap.renameVariable(variable, 'new test name'); + + assertEventFired( + this.eventSpy, + Blockly.Events.VarRename, + { + oldName: 'test name', + newName: 'new test name', + varId: 'test id', + }, + this.workspace.id); + }); + + test( + 'rename events are not fired if the variable name already matches', + function() { + const variable = + this.variableMap.createVariable( + 'test name', 'test type', 'test id'); + this.variableMap.renameVariable(variable, 'test name'); + + assertEventNotFired( + this.eventSpy, + Blockly.Events.VarRename, + {}, + this.workspace.id); + }); + + test( + 'rename events are not fired if the variable does not exist', + function() { + const variable = + new Blockly.VariableModel( + 'test name', 'test type', 'test id'); + this.variableMap.renameVariable(variable, 'test name'); + + assertEventNotFired( + this.eventSpy, + Blockly.Events.VarRename, + {}, + this.workspace.id); + }); + }); + + suite('renaming by ID', function() { + test('rename events are fired when a variable is renamed', function() { + this.variableMap.createVariable('test name', 'test type', 'test id'); + this.variableMap.renameVariableById('test id', 'new test name'); + + assertEventFired( + this.eventSpy, + Blockly.Events.VarRename, + { + oldName: 'test name', + newName: 'new test name', + varId: 'test id', + }, + this.workspace.id); + }); + + test( + 'rename events are not fired if the variable name already matches', + function() { + this.variableMap.createVariable( + 'test name', 'test type', 'test id'); + this.variableMap.renameVariableById('test id', 'test name'); + + assertEventNotFired( + this.eventSpy, + Blockly.Events.VarRename, + {}, + this.workspace.id); + }); + + test( + 'renaming throws if the variable does not exist', + function() { + // Not sure why this throws when the other one doesn't but might + // as well test it. + chai.assert.throws(() => { + this.variableMap.renameVariableById('test id', 'test name'); + }, `Tried to rename a variable that didn't exist`); + }); + }); + }); + }); });