diff --git a/blocks/procedures.js b/blocks/procedures.js index e2d4147af..31e652a3f 100644 --- a/blocks/procedures.js +++ b/blocks/procedures.js @@ -27,6 +27,7 @@ const {Block} = goog.requireType('Blockly.Block'); // TODO (6248): Properly import the BlockDefinition type. /* eslint-disable-next-line no-unused-vars */ const BlockDefinition = Object; +const {isProcedureBlock} = goog.require('Blockly.procedures.IProcedureModel'); const {ObservableProcedureModel} = goog.require('Blockly.procedures.ObservableProcedureModel'); const {ObservableParameterModel} = goog.require('Blockly.procedures.ObservableParameterModel'); const {config} = goog.require('Blockly.config'); @@ -576,7 +577,9 @@ const procedureDefMutator = { const map = this.workspace.getProcedureMap(); const procedureId = state['procedureId']; if (procedureId && procedureId != this.model_.getId() && - map.has(procedureId)) { + map.has(procedureId) && + (this.isInsertionMarker() || + this.noBlockHasClaimedModel_(procedureId))) { if (map.has(this.model_.getId())) { map.delete(this.model_.getId()); } @@ -598,6 +601,21 @@ const procedureDefMutator = { Procedures.mutateCallers(this); }, + /** + * Returns true if there is no definition block currently associated with the + * given procedure ID. False otherwise. + * @param {string} procedureId The ID of the procedure to check for a claiming + * block. + * @return {boolean} True if there is no definition block currently associated + * with the given procedure ID. False otherwise. + */ + noBlockHasClaimedModel_(procedureId) { + const model = this.workspace.getProcedureMap().get(procedureId); + return this.workspace.getAllBlocks(false).every( + (b) => !isProcedureBlock(b) || !b.isProcedureDef() || + b.getProcedureModel() !== model); + }, + /** * Populate the mutator's dialog with this block's components. * @param {!Workspace} workspace Mutator's workspace. diff --git a/core/interfaces/i_procedure_block.ts b/core/interfaces/i_procedure_block.ts index b2ab39629..59ca26b23 100644 --- a/core/interfaces/i_procedure_block.ts +++ b/core/interfaces/i_procedure_block.ts @@ -6,6 +6,8 @@ import type {Block} from '../block.js'; import {IProcedureModel} from './i_procedure_model.js'; +import * as goog from '../../closure/goog/goog.js'; +goog.declareModuleId('Blockly.procedures.IProcedureModel'); /** The interface for a block which models a procedure. */ diff --git a/tests/mocha/blocks/procedures_test.js b/tests/mocha/blocks/procedures_test.js index ed293a5b1..24a908a45 100644 --- a/tests/mocha/blocks/procedures_test.js +++ b/tests/mocha/blocks/procedures_test.js @@ -1985,7 +1985,7 @@ suite('Procedures', function() { }); }); - suite('extra serialization test cases', function() { + suite('full workspace serialization test cases', function() { test('definitions with parameters are properly rendered', function() { Blockly.serialization.workspaces.load({ "blocks": { @@ -2032,6 +2032,58 @@ suite('Procedures', function() { assertDefBlockStructure( this.workspace.getTopBlocks(false)[0], false, ['x'], ['varId']); }); + test( + 'multiple definitions pointing to the same model end up with ' + + 'different models', + function() { + Blockly.serialization.workspaces.load({ + "blocks": { + "languageVersion": 0, + "blocks": [ + { + "type": "procedures_defnoreturn", + "extraState": { + "procedureId": "procId", + }, + "fields": { + "NAME": "do something", + }, + }, + { + "type": "procedures_defnoreturn", + "y": 10, + "extraState": { + "procedureId": "procId", + }, + "fields": { + "NAME": "do something", + }, + }, + ], + }, + "procedures": [ + { + "id": "procId", + "name": "do something", + "returnTypes": null, + }, + ], + }, this.workspace); + const def1 = this.workspace.getTopBlocks(true)[0]; + const def2 = this.workspace.getTopBlocks(true)[1]; + chai.assert.equal( + def1.getProcedureModel().getName(), + 'do something', + 'Expected the first procedure definition to have the name in XML'); + chai.assert.equal( + def2.getProcedureModel().getName(), + 'do something2', + 'Expected the second procedure definition to be renamed'); + chai.assert.notEqual( + def1.getProcedureModel(), + def2.getProcedureModel(), + 'Expected the procedures to have different models'); + }); }); const testSuites = [