fix: copying and pasting procedure definitions (#6747)

* fix: copy-pasting procedure definitions

* chore: add test for loading two procedure defs

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.

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. */

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 = [