diff --git a/blocks/procedures.js b/blocks/procedures.js index 419631d52..97141471a 100644 --- a/blocks/procedures.js +++ b/blocks/procedures.js @@ -308,6 +308,7 @@ const procedureDefGetDefMixin = function() { * disposed. */ destroy: function() { + if (this.isInsertionMarker()) return; this.workspace.getProcedureMap().delete(this.getProcedureModel().getId()); }, }; @@ -598,10 +599,12 @@ const procedureDefMutator = { * parameters and statements. */ saveExtraState: function() { - const params = this.getProcedureModel().getParameters(); - if (!params.length && this.hasStatements_) return null; - const state = Object.create(null); + state['procedureId'] = this.getProcedureModel().getId(); + + const params = this.getProcedureModel().getParameters(); + if (!params.length && this.hasStatements_) return state; + if (params.length) { state['params'] = params.map((p) => { return { @@ -625,6 +628,16 @@ const procedureDefMutator = { * statements. */ loadExtraState: function(state) { + const map = this.workspace.getProcedureMap(); + const procedureId = state['procedureId']; + if (procedureId && procedureId != this.model_.getId() && + map.has(procedureId)) { + if (map.has(this.model_.getId())) { + map.delete(this.model_.getId()); + } + this.model_ = map.get(procedureId); + } + if (state['params']) { for (let i = 0; i < state['params'].length; i++) { const {name, id, paramId} = state['params'][i]; diff --git a/core/procedures.ts b/core/procedures.ts index ef7e32062..5369b61bd 100644 --- a/core/procedures.ts +++ b/core/procedures.ts @@ -193,7 +193,9 @@ export function rename(this: Field, name: string): string { name = name.trim(); const legalName = findLegalName(name, block); - if (isProcedureBlock(block)) block.getProcedureModel().setName(legalName); + if (isProcedureBlock(block) && !block.isInsertionMarker()) { + block.getProcedureModel().setName(legalName); + } const oldName = this.getValue(); if (oldName !== name && oldName !== legalName) { // Rename any callers. diff --git a/tests/mocha/blocks/procedures_test.js b/tests/mocha/blocks/procedures_test.js index 7e4f9a3fa..b2ccdfa4a 100644 --- a/tests/mocha/blocks/procedures_test.js +++ b/tests/mocha/blocks/procedures_test.js @@ -1338,6 +1338,35 @@ suite('Procedures', function() { chai.assert.isTrue( callBlock2.disposed, 'Expected the second caller to be disposed'); }); + + test('undoing and redoing a procedure delete will still associate ' + + 'procedure and caller with the same model', + function() { + const defBlock = createProcDefBlock(this.workspace); + createProcCallBlock(this.workspace); + // TODO: Apparently we need to call checkAndDelete to handle event + // grouping, this seems like possibly a bug. + const oldModel = defBlock.getProcedureModel(); + defBlock.checkAndDelete(); + this.clock.runAll(); + + this.workspace.undo(); + this.clock.runAll(); + + const newDefBlock = + this.workspace.getBlocksByType('procedures_defnoreturn')[0]; + const newCallBlock = + this.workspace.getBlocksByType('procedures_callnoreturn')[0]; + + chai.assert.equal( + newDefBlock.getProcedureModel(), + newCallBlock.getProcedureModel(), + 'Expected both new blocks to be associated with the same model'); + chai.assert.equal( + oldModel.getId(), + newDefBlock.getProcedureModel().getId(), + 'Expected the new model to have the same ID as the old model'); + }); }); suite('caller blocks creating new def blocks', function() {