Repository: ProcessMaker/modeler (js)

Issue:
Duplicated ID issue
# Bug report issue
## Description
When updating the provided bpmn in a certain way, it duplicates node_17_di.
https://github.com/ProcessMaker/modeler/files/3617104/bugProcess.bpmn.txt
## Reproduction steps
You must upload the provided bpmn and then edit it from this:
![Screen Shot 2019-10-30 at 10 20 14 AM](https://user-images.githubusercontent.com/6653340/67866184-ed37b900-fafe-11e9-93c8-50e25052c493.png)
to this:
![Screen Shot 2019-10-30 at 10 33 28 AM](https://user-images.githubusercontent.com/6653340/67867408-bf537400-fb00-11e9-8ff3-eb88ae66a3d7.png)

The Sequence Flow is changed from **Report Expense** to **Before Dynaform**

Then it does duplicate **node_17_di** in the xml.
## Expected behaviour
Change the Start Sequence Flow to **Before Dynaform** and it should not duplicate **node_17_di**
## Actual behaviour
**node_17_di** is duplicated when the sequence flow is changed from **Report Expense** to **Before Dynaform**
## Software used
Latest Chrome. Modeler v 0.17.5.



Pull request description:
Fix duplicate diagram IDs
Fixes #774.

Code is already in place for ensuring nodes do not receive duplicate IDs; this PR prevents added nodes from receiving a duplicate _**diagram**_ ID. 

Patch:
diff --git a/src/MessageEventIdGenerator.js b/src/MessageEventIdGenerator.js
deleted file mode 100644
index c1191c4c..00000000
--- a/src/MessageEventIdGenerator.js
+++ /dev/null
@@ -1,25 +0,0 @@
-import  NodeIdGenerator from './NodeIdGenerator';
-import store from '@/store';
-import get from 'lodash/get';
-
-export default class MessageEventIdGenerator extends NodeIdGenerator {
-  generateNewMessageEventId() {
-    let id = super.generateNewNodeId('message_event_');
-
-    while (!this.isIdUnique(id)) {
-      id = this.generateNewMessageEventId();
-    }
-
-    return id;
-  }
-
-  isIdUnique(id) {
-    const nodes = store.getters.nodes;
-
-    const eventDefinitionIdList = nodes
-      .map(node => get(node, 'definition.eventDefinitions[0].id'))
-      .filter(id => id !== undefined);
-
-    return !eventDefinitionIdList.includes(id);
-  }
-}
diff --git a/src/NodeIdGenerator.js b/src/NodeIdGenerator.js
index 9c14cafd..518b0f3c 100644
--- a/src/NodeIdGenerator.js
+++ b/src/NodeIdGenerator.js
@@ -1,33 +1,55 @@
-const nodeIdPrefix = 'node_';
-
 export default class NodeIdGenerator {
+  static prefix = 'node_';
+
+  #counter = 1;
+  #diagramCounter = 1;
+
   constructor(definitions) {
-    this.counter = 1;
     this.definitions = definitions;
   }
 
-  generateUniqueNodeId() {
-    let id = this.generateNewNodeId();
+  generate() {
+    let definitionId = this.#generateDefinitionId();
+    let diagramId = this.#generateDiagramId();
 
-    while (!this.isIdUnique(id)) {
-      id = this.generateNewNodeId();
+    while (!this.#isDefinitionIdUnique(definitionId)) {
+      definitionId = this.#generateDefinitionId();
     }
 
-    return id;
+    while (!this.#isDiagramIdUnique(diagramId)) {
+      diagramId = this.#generateDiagramId();
+    }
+
+    return [definitionId, diagramId];
   }
 
-  generateNewNodeId(prefix = nodeIdPrefix) {
-    const id = prefix + this.counter;
-    this.counter++;
+  #generateDefinitionId = () => {
+    const id = NodeIdGenerator.prefix + this.#counter;
+    this.#counter++;
 
     return id;
-  }
+  };
 
-  isIdUnique(id) {
+  #generateDiagramId = () => {
+    const id = NodeIdGenerator.prefix + this.#diagramCounter + '_di';
+    this.#diagramCounter++;
+
+    return id;
+  };
+
+  #isDefinitionIdUnique = id => {
     const planeElementIds = this.definitions.diagrams[0].plane
       .get('planeElement')
       .map(planeElement => planeElement.get('bpmnElement').id);
 
     return !planeElementIds.includes(id);
-  }
+  };
+
+  #isDiagramIdUnique = id => {
+    const diagramElementIds = this.definitions.diagrams[0].plane
+      .get('planeElement')
+      .map(planeElement => planeElement.id);
+
+    return !diagramElementIds.includes(id);
+  };
 }
diff --git a/src/components/crownConfig.vue b/src/components/crownConfig.vue
new file mode 100644
index 00000000..be42dacf
--- /dev/null
+++ b/src/components/crownConfig.vue
@@ -0,0 +1,277 @@
+<template>
+  <div class="crown-config" :style="style" v-if="showCrown">
+    <sequence-flow-button
+      @click="addSequence"
+      class="crown-config__icon"
+      :title="$t('Sequence Flow')"
+    />
+    <message-flow-button
+      @click="addMessageFlow"
+      class="crown-config__icon"
+      :title="$t('Message Flow')"
+    />
+    <delete-button
+      @click="removeShape"
+      class="crown-config__icon"
+      :title="$t('Delete')"
+    />
+  </div>
+</template>
+
+<script>
+import DeleteButton from '@/components/deleteButton';
+import MessageFlowButton from '@/components/messageFlowButton';
+import SequenceFlowButton from '@/components/sequenceFlowButton';
+import pull from 'lodash/pull';
+
+export default {
+  components: {
+    DeleteButton,
+    MessageFlowButton,
+    SequenceFlowButton,
+  },
+  props: [
+    'highlighted',
+    'paper',
+    'graph',
+    'shape',
+    'node',
+    'nodeRegistry',
+    'moddle',
+    'planeElements',
+    'processNode',
+    'collaboration',
+    'isRendering',
+  ],
+  watch: {
+    highlighted() {
+      this.showCrown = this.highlighted;
+    },
+    shape() {
+      if (this.highlighted) {
+        this.showCrown = true;
+      }
+    },
+  },
+  data() {
+    return {
+      showCrown: false,
+      /* allowSetNodePosition is used to prevent setting a node position outside of a pool */
+      allowSetNodePosition: true,
+      savePositionOnPointerupEventSet: false,
+    };
+  },
+  created() {
+    this.$t = this.$t.bind(this);
+  },
+  mounted() {
+    this.$nextTick(() => {
+      /* Use nextTick to ensure this code runs after the component it is mixed into mounts.
+       * This will ensure this.shape is defined. */
+
+      this.configurePoolLane();
+
+      if (this.isRendering) {
+        this.paper.once('render:done', this.setUpCrownConfig);
+      } else {
+        this.setUpCrownConfig();
+      }
+    });
+  },
+  computed: {
+    style() {
+      const { x, y } = this.shape.findView(this.paper).getBBox();
+      const width = (this.node.diagram.bounds.width * this.paper.scale().sx);
+      return 'top:' + (y - 45) + 'px;left:' + (x + width - 20) + 'px;cursor:pointer';
+    },
+  },
+  methods: {
+    addSequence(cellView, evt, x, y) {
+      this.removeCrown();
+      const sequenceFlowConfig = this.nodeRegistry['processmaker-modeler-sequence-flow'];
+      const sequenceLink = sequenceFlowConfig.definition(this.moddle, this.$t);
+      sequenceLink.set('sourceRef', this.node.definition);
+      sequenceLink.set('targetRef', { x, y });
+
+      if (
+        sequenceLink.sourceRef.$type === 'bpmn:ExclusiveGateway' ||
+        sequenceLink.sourceRef.$type === 'bpmn:InclusiveGateway') {
+        sequenceLink.conditionExpression = this.moddle.create('bpmn:FormalExpression', {
+          body: '',
+        });
+      }
+
+      this.$emit('add-node', {
+        type: 'processmaker-modeler-sequence-flow',
+        definition: sequenceLink,
+        diagram: sequenceFlowConfig.diagram(this.moddle),
+      });
+    },
+    addMessageFlow(cellView, evt, x, y) {
+      this.removeCrown();
+
+      const messageFlowDefinition = this.moddle.create('bpmn:MessageFlow', {
+        name: '',
+        sourceRef: this.shape.component.node.definition,
+        targetRef: { x, y },
+      });
+
+      this.$emit('add-node', {
+        type: 'processmaker-modeler-message-flow',
+        definition: messageFlowDefinition,
+        diagram: this.moddle.create('bpmndi:BPMNEdge'),
+      });
+    },
+    removeShape() {
+      this.graph.getConnectedLinks(this.shape).forEach(shape => this.$emit('remove-node', shape.component.node));
+      this.shape.getEmbeddedCells({ deep: true }).forEach(cell => {
+        if (cell.component) {
+          this.graph.getConnectedLinks(cell).forEach(shape => this.$emit('remove-node', shape.component.node));
+          this.shape.unembed(cell);
+          this.$emit('remove-node', cell.component.node);
+        }
+      });
+
+      this.$emit('remove-node', this.node);
+    },
+    removeCrown() {
+      this.showCrown = false;
+    },
+    setNodePosition() {
+      this.shape.stopListening(this.paper, 'element:pointerup', this.setNodePosition);
+      this.savePositionOnPointerupEventSet = false;
+
+      if (!this.allowSetNodePosition) {
+        return;
+      }
+
+      this.$emit('save-state');
+    },
+    setUpCrownConfig() {
+      this.shape.on('change:position', (element, newPosition) => {
+        this.node.diagram.bounds.x = newPosition.x;
+        this.node.diagram.bounds.y = newPosition.y;
+
+        if (!this.savePositionOnPointerupEventSet) {
+          this.shape.listenToOnce(this.paper, 'element:pointerup', this.setNodePosition);
+          this.savePositionOnPointerupEventSet = true;
+        }
+      });
+
+      if (!this.planeElements.includes(this.node.diagram)) {
+        this.planeElements.push(this.node.diagram);
+      }
+
+      const process = this.node.pool
+        ? this.node.pool.component.containingProcess
+        : this.processNode.definition;
+      const nodeTypes = Object.keys(this.node.definition.$descriptor.allTypesByName);
+
+      if (nodeTypes.includes('bpmn:FlowElement') && !process.get('flowElements').includes(this.node.definition)) {
+        process.get('flowElements').push(this.node.definition);
+      }
+
+      if (nodeTypes.includes('bpmn:Artifact') && !process.get('artifacts').includes(this.node.definition)) {
+        process.get('artifacts').push(this.node.definition);
+      }
+
+      if (
+        this.collaboration &&
+        nodeTypes.includes('bpmn:MessageFlow') &&
+        !this.collaboration.get('messageFlows').includes(this.node.definition)
+      ) {
+        this.collaboration.get('messageFlows').push(this.node.definition);
+      }
+    },
+    configurePoolLane() {
+      if ([
+        'processmaker-modeler-pool',
+        'processmaker-modeler-sequence-flow',
+        'processmaker-modeler-association',
+        'processmaker-modeler-message-flow',
+      ].includes(this.node.type)) {
+        return;
+      }
+
+      if (this.node.pool) {
+        if (!this.graph.getCell(this.node.pool)) {
+          this.node.pool = this.graph.getElements().find(element => {
+            return element.component && element.component.node === this.node.pool.component.node;
+          });
+        }
+
+        this.node.pool.component.addToPool(this.shape);
+
+        if (this.isLane) {
+          this.configureLaneInParentPool();
+        }
+
+        return;
+      }
+
+      /* If we are over a pool or lane, add the shape to the pool or lane */
+      const pool = this.graph.findModelsInArea(this.shape.getBBox()).filter(model => {
+        return model.component && model.component.node.type === 'processmaker-modeler-pool';
+      })[0];
+
+      if (pool) {
+        this.node.pool = pool;
+        this.node.pool.component.addToPool(this.shape);
+
+        if (this.isLane) {
+          this.configureLaneInParentPool();
+        }
+      }
+    },
+  },
+  destroyed() {
+    this.shape.stopListening();
+    this.shape.remove();
+
+    const process = this.node.pool
+      ? this.node.pool.component.containingProcess
+      : this.processNode.definition;
+
+    pull(process.get('flowElements'), this.node.definition);
+    pull(this.planeElements, this.node.diagram);
+    pull(process.get('artifacts'), this.node.definition);
+
+    if (this.collaboration) {
+      pull(this.collaboration.get('messageFlows'), this.node.definition);
+    }
+  },
+};
+</script>
+
+<style lang="scss">
+  $primary-color: #5096db;
+
+  .crown-config {
+    background-color: $primary-color;
+    position: absolute;
+    z-index: 5;
+    display: flex;
+    justify-content: center;
+    width: auto;
+    height: 1.85rem;
+    border-radius: 5px;
+
+    &::after {
+      background-color: $primary-color;
+      content: '';
+      width: 1.25rem;
+      height: 1.25rem;
+      position: absolute;
+      top: 0.75rem;
+      left: 0.45rem;
+      z-index: -1;
+      transform: rotate(45deg);
+      border-radius: 1px;
+    }
+
+    &__icon {
+      margin: 5px 10px;
+      font-size: 1rem;
+    }
+  }
+</style>
diff --git a/src/components/deleteButton.vue b/src/components/deleteButton.vue
new file mode 100644
index 00000000..e935cf44
--- /dev/null
+++ b/src/components/deleteButton.vue
@@ -0,0 +1,18 @@
+<template>
+  <svg
+    @click="$emit('click')"
+    id="delete-button"
+    data-test="delete-button"
+    v-b-tooltip.hover.viewport.d50
+    aria-hidden="true"
+    data-prefix="fas"
+    data-icon="trash-alt"
+    class="svg-inline--fa fa-trash-alt fa-w-14"
+    role="img"
+    xmlns="http://www.w3.org/2000/svg" viewBox="0 0 448 512"
+  >
+    <path fill="#fff"
+      d="M0 84V56c0-13.3 10.7-24 24-24h112l9.4-18.7c4-8.2 12.3-13.3 21.4-13.3h114.3c9.1 0 17.4 5.1 21.5 13.3L312 32h112c13.3 0 24 10.7 24 24v28c0 6.6-5.4 12-12 12H12C5.4 96 0 90.6 0 84zm416 56v324c0 26.5-21.5 48-48 48H80c-26.5 0-48-21.5-48-48V140c0-6.6 5.4-12 12-12h360c6.6 0 12 5.4 12 12zm-272 68c0-8.8-7.2-16-16-16s-16 7.2-16 16v224c0 8.8 7.2 16 16 16s16-7.2 16-16V208zm96 0c0-8.8-7.2-16-16-16s-16 7.2-16 16v224c0 8.8 7.2 16 16 16s16-7.2 16-16V208zm96 0c0-8.8-7.2-16-16-16s-16 7.2-16 16v224c0 8.8 7.2 16 16 16s16-7.2 16-16V208z"
+    />
+  </svg>
+</template>
diff --git a/src/components/messageFlowButton.vue b/src/components/messageFlowButton.vue
new file mode 100644
index 00000000..e6b64722
--- /dev/null
+++ b/src/components/messageFlowButton.vue
@@ -0,0 +1,31 @@
+<template>
+  <svg
+    @click="$emit('click')"
+    id="message-flow-button"
+    data-test="message-flow-button"
+    v-b-tooltip.hover.viewport.d50
+    width="19"
+    height="20"
+    viewBox="0 0 19 20"
+    fill="none"
+    xmlns="http://www.w3.org/2000/svg"
+  >
+    <g clip-path="url(#clip0)">
+      <circle cx="2.5" cy="2.5" r="2" stroke="#fff"/>
+      <path d="M15.5 20L15.5 17.5V15L18.5 17.5L15.5 20Z" fill="#fff"/>
+      <line x1="4" y1="2.5" x2="7" y2="2.5" stroke="#fff"/>
+      <line x1="8" y1="2.5" x2="11" y2="2.5" stroke="#fff"/>
+      <line x1="10.5" y1="5" x2="10.5" y2="2" stroke="#fff"/>
+      <line x1="10.5" y1="9" x2="10.5" y2="6" stroke="#fff"/>
+      <line x1="10.5" y1="13" x2="10.5" y2="10" stroke="#fff"/>
+      <line x1="10.5" y1="17" x2="10.5" y2="14" stroke="#fff"/>
+      <line x1="10" y1="17.5" x2="13" y2="17.5" stroke="#fff"/>
+      <line x1="14" y1="17.5" x2="17" y2="17.5" stroke="#fff"/>
+    </g>
+    <defs>
+      <clipPath id="clip0">
+        <rect width="19" height="20" fill="white"/>
+      </clipPath>
+    </defs>
+  </svg>
+</template>
diff --git a/src/components/nodeManager.js b/src/components/nodeManager.js
index ad5e99ea..fc3a871a 100644
--- a/src/components/nodeManager.js
+++ b/src/components/nodeManager.js
@@ -2,12 +2,14 @@ import { id as poolId } from './nodes/pool';
 import { id as laneId } from '@/components/nodes/poolLane';
 
 export function addIdToNodeAndSetUpDiagramReference(node, nodeIdGenerator) {
-  const id = node.definition.id || nodeIdGenerator.generateUniqueNodeId();
+  const [nodeId, diagramId] = nodeIdGenerator.generate();
 
-  node.definition.id = id;
+  if (!node.definition.id) {
+    node.definition.id = nodeId;
+  }
 
   if (node.diagram) {
-    node.diagram.id = `${id}_di`;
+    node.diagram.id = diagramId;
     node.diagram.bpmnElement = node.definition;
   }
 }
diff --git a/src/components/nodes/manualTask/manualTask.vue b/src/components/nodes/manualTask/manualTask.vue
index ef486d08..4dc6e876 100644
--- a/src/components/nodes/manualTask/manualTask.vue
+++ b/src/components/nodes/manualTask/manualTask.vue
@@ -1,7 +1,3 @@
-<template>
-  <div/>
-</template>
-
 <script>
 import TaskComponent from '@/components/nodes/task/task';
 import scriptIcon from '@/assets/manual-task.svg';
diff --git a/src/components/nodes/scriptTask/scriptTask.vue b/src/components/nodes/scriptTask/scriptTask.vue
index 493e1348..0801bd41 100644
--- a/src/components/nodes/scriptTask/scriptTask.vue
+++ b/src/components/nodes/scriptTask/scriptTask.vue
@@ -1,7 +1,3 @@
-<template>
-  <div/>
-</template>
-
 <script>
 import TaskComponent from '@/components/nodes/task/task';
 import scriptIcon from '@/assets/script.svg';
diff --git a/src/components/nodes/task/task.vue b/src/components/nodes/task/task.vue
index 6f18231a..2b4e52c6 100644
--- a/src/components/nodes/task/task.vue
+++ b/src/components/nodes/task/task.vue
@@ -1,24 +1,57 @@
 <template>
-  <div/>
+  <div>
+    <crown-config
+      :highlighted="highlighted"
+      :paper="paper"
+      :graph="graph"
+      :shape="shape"
+      :node="node"
+      :nodeRegistry="nodeRegistry"
+      :moddle="moddle"
+      :collaboration="collaboration"
+      :process-node="processNode"
+      :plane-elements="planeElements"
+      :is-rendering="isRendering"
+      @remove-node="$emit('remove-node', $event)"
+      @add-node="$emit('add-node', $event)"
+      @save-state="$emit('save-state', $event)"
+    />
+  </div>
 </template>
 
 <script>
 import { util } from 'jointjs';
 import connectIcon from '@/assets/connect-elements.svg';
-import crownConfig from '@/mixins/crownConfig';
+import highlightConfig from '@/mixins/highlightConfig';
 import portsConfig from '@/mixins/portsConfig';
 import hasMarkers from '@/mixins/hasMarkers';
 import {markerSize} from '@/mixins/hasMarkers';
 import TaskShape from '@/components/nodes/task/shape';
 import { taskHeight } from './taskConfig';
 import hideLabelOnDrag from '@/mixins/hideLabelOnDrag';
+import CrownConfig from '@/components/crownConfig';
 
 const labelPadding = 15;
 const topAndBottomMarkersSpace = 2 * markerSize;
 
 export default {
-  props: ['graph', 'node', 'id'],
-  mixins: [crownConfig, portsConfig, hasMarkers, hideLabelOnDrag],
+  components: {
+    CrownConfig,
+  },
+  props: [
+    'graph',
+    'node',
+    'id',
+    'highlighted',
+    'nodeRegistry',
+    'moddle',
+    'paper',
+    'collaboration',
+    'processNode',
+    'planeElements',
+    'isRendering',
+  ],
+  mixins: [highlightConfig, portsConfig, hasMarkers, hideLabelOnDrag],
   data() {
     return {
       shape: null,
@@ -65,6 +98,7 @@ export default {
   },
   mounted() {
     this.shape = new TaskShape();
+    this.shape.set('type', 'newCrown');
     let bounds = this.node.diagram.bounds;
     this.shape.position(bounds.x, bounds.y);
     this.shape.resize(bounds.width, bounds.height);
@@ -81,7 +115,6 @@ export default {
 
     this.shape.addTo(this.graph);
     this.shape.component = this;
-
   },
 };
 </script>
diff --git a/src/components/sequenceFlowButton.vue b/src/components/sequenceFlowButton.vue
new file mode 100644
index 00000000..1a247ebb
--- /dev/null
+++ b/src/components/sequenceFlowButton.vue
@@ -0,0 +1,22 @@
+<template>
+  <svg
+    @click="$emit('click')"
+    id="sequence-flow-button"
+    data-test="sequence-flow-button"
+    v-b-tooltip.hover.viewport.d50
+    width="19px"
+    height="20px"
+    viewBox="0 0 19 20"
+    version="1.1"
+    xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"
+  >
+    <g id="Symbols" stroke="none" stroke-width="1" fill="none" fill-rule="evenodd">
+      <g id="custom/connect-elements" transform="translate(-3.000000, -2.000000)" fill="#000000">
+        <path fill="#fff"
+          d="M18.5,20 L13,20 L13,5 L8,5 L8,7 L3,7 L3,2 L8,2 L8,4 L14,4 L14,19 L18.5,19 L18.5,17 L21.5,19.5 L18.5,22 L18.5,20 Z M4,3 L4,6 L7,6 L7,3 L4,3 Z"
+          id="connect-elements"
+        />
+      </g>
+    </g>
+  </svg>
+</template>
diff --git a/src/mixins/highlightConfig.js b/src/mixins/highlightConfig.js
new file mode 100644
index 00000000..cc69a81f
--- /dev/null
+++ b/src/mixins/highlightConfig.js
@@ -0,0 +1,109 @@
+const errorHighlighter = {
+  name: 'stroke',
+  options: {
+    padding: 12,
+    attrs: {
+      stroke: 'red',
+      'stroke-width': 3,
+      opacity: 0.6,
+    },
+  },
+};
+
+const defaultHighlighter = {
+  name: 'stroke',
+  options: {
+    attrs: {
+      stroke: '#feb663',
+      'stroke-width': 3,
+    },
+  },
+};
+
+export default {
+  props: [
+    'highlighted',
+    'paper',
+    'hasError',
+    'isRendering',
+  ],
+  data() {
+    return {
+      shape: null,
+    };
+  },
+  watch: {
+    highlighted() {
+      this.setHighlight();
+    },
+    hasError() {
+      if (this.isRendering) {
+        return;
+      }
+
+      this.setErrorHighlight();
+    },
+  },
+  computed: {
+    shapeView() {
+      return this.shape.findView(this.paper);
+    },
+    shapeBody() {
+      return this.shapeView.$el.find('[joint-selector=body]');
+    },
+  },
+  methods: {
+    setHighlight() {
+      if (this.highlighted) {
+        this.shapeView.highlight(this.shapeBody, {highlighter: defaultHighlighter});
+        return;
+      }
+
+      this.shapeView.unhighlight(this.shapeBody, {highlighter: defaultHighlighter});
+    },
+    setErrorHighlight() {
+      if (!this.shapeView) {
+        return;
+      }
+
+      if (this.hasError) {
+        this.shapeView.highlight(null, { highlighter: errorHighlighter });
+      } else {
+        this.shapeView.unhighlight(null, { highlighter: errorHighlighter });
+      }
+    },
+    configureCrown() {
+      if (!this.crownConfig) {
+        this.crownConfig = [];
+      }
+      this.paper.once('render:done', () => this.setHighlight());
+
+      const shapeView = this.shape.findView(this.paper);
+
+      this.shape.on('change:size', () => {
+        if (this.highlighted) {
+          /* Ensure the highlight box expands to fit element */
+          shapeView.unhighlight(this.shapeBody, { highlighter: defaultHighlighter });
+          shapeView.highlight(this.shapeBody, { highlighter: defaultHighlighter });
+        }
+      });
+    },
+    setUpCrownConfig() {
+      this.configureCrown();
+
+      this.setErrorHighlight();
+    },
+  },
+  mounted() {
+    this.$nextTick(() => {
+      /* Use nextTick to ensure this code runs after the component it is mixed into mounts.
+       * This will ensure this.shape is defined. */
+
+      if (this.isRendering) {
+        this.paper.once('render:done', this.setUpCrownConfig);
+      } else {
+        this.setUpCrownConfig();
+      }
+    });
+  },
+};


Test patch:
diff --git a/tests/e2e/fixtures/setUpForDuplicateDiagramId.xml b/tests/e2e/fixtures/setUpForDuplicateDiagramId.xml
new file mode 100644
index 00000000..152f3ace
--- /dev/null
+++ b/tests/e2e/fixtures/setUpForDuplicateDiagramId.xml
@@ -0,0 +1,13 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<bpmn:definitions xmlns:bpmn="http://www.omg.org/spec/BPMN/20100524/MODEL" xmlns:bpmndi="http://www.omg.org/spec/BPMN/20100524/DI" xmlns:dc="http://www.omg.org/spec/DD/20100524/DC" id="Definitions_03dabax" targetNamespace="http://bpmn.io/schema/bpmn" exporter="ProcessMaker Modeler" exporterVersion="1.0">
+  <bpmn:process id="Process_1" isExecutable="true">
+    <bpmn:task id="node_2" name="Task" />
+  </bpmn:process>
+  <bpmndi:BPMNDiagram id="BPMNDiagram_1">
+    <bpmndi:BPMNPlane id="BPMNPlane_1" bpmnElement="Process_1">
+      <bpmndi:BPMNShape id="node_1_di" bpmnElement="node_2">
+        <dc:Bounds x="310" y="170" width="116" height="76" />
+      </bpmndi:BPMNShape>
+    </bpmndi:BPMNPlane>
+  </bpmndi:BPMNDiagram>
+</bpmn:definitions>
diff --git a/tests/e2e/specs/BoundaryEventCommonBehaviour.spec.js b/tests/e2e/specs/BoundaryEventCommonBehaviour.spec.js
index 34d1307a..7933807d 100644
--- a/tests/e2e/specs/BoundaryEventCommonBehaviour.spec.js
+++ b/tests/e2e/specs/BoundaryEventCommonBehaviour.spec.js
@@ -21,24 +21,32 @@ const boundaryEventData = [{
   eventXMLSnippet: '<bpmn:boundaryEvent id="node_3" name="New Boundary Timer Event" attachedToRef="node_2"><bpmn:timerEventDefinition><bpmn:timeDuration>PT1H</bpmn:timeDuration></bpmn:timerEventDefinition></bpmn:boundaryEvent>',
   taskType: nodeTypes.task,
   invalidTargets: [{ type: nodeTypes.startEvent }],
+  taskSelector: '.main-paper ' +
+    '[data-type="newCrown"]',
 }, {
   type: 'Boundary Error Event',
   nodeType: nodeTypes.boundaryErrorEvent,
   eventXMLSnippet: '<bpmn:boundaryEvent id="node_3" name="New Boundary Error Event" attachedToRef="node_2"><bpmn:errorEventDefinition /></bpmn:boundaryEvent>',
   taskType: nodeTypes.task,
   invalidTargets: [{ type: nodeTypes.startEvent }],
+  taskSelector: '.main-paper ' +
+    '[data-type="newCrown"]',
 }, {
   type: 'Boundary Escalation Event',
   nodeType: nodeTypes.boundaryEscalationEvent,
   eventXMLSnippet: '<bpmn:boundaryEvent id="node_3" name="New Boundary Escalation Event" attachedToRef="node_2"><bpmn:escalationEventDefinition /></bpmn:boundaryEvent>',
   taskType: nodeTypes.callActivity,
   invalidTargets: [{ type: nodeTypes.startEvent }, { type: nodeTypes.task, color: defaultNodeColor }],
+  taskSelector: '.main-paper ' +
+    '[data-type="processmaker.components.nodes.task.Shape"]',
 }, {
   type: 'Boundary Message Event',
   nodeType: nodeTypes.boundaryMessageEvent,
   eventXMLSnippet: '<bpmn:boundaryEvent id="node_3" name="New Boundary Message Event" attachedToRef="node_2"><bpmn:messageEventDefinition /></bpmn:boundaryEvent>',
   taskType: nodeTypes.callActivity,
   invalidTargets: [{ type: nodeTypes.startEvent }],
+  taskSelector: '.main-paper ' +
+    '[data-type="processmaker.components.nodes.task.Shape"]',
 }];
 
 function testThatBoundaryEventIsCloseToTask(boundaryEvent, task) {
@@ -61,7 +69,7 @@ function configurePool(poolPosition, nodeType, taskType) {
   dragFromSourceToDest(nodeTypes.pool, poolPosition);
 }
 
-boundaryEventData.forEach(({ type, nodeType, eventXMLSnippet, taskType, invalidTargets }) => {
+boundaryEventData.forEach(({ type, nodeType, eventXMLSnippet, taskType, invalidTargets, taskSelector }) => {
   describe(`Common behaviour test for boundary event type ${type}`, () => {
     it('can render a boundary event of this type', function() {
       dragFromSourceToDest(taskType, taskPosition);
@@ -102,9 +110,6 @@ boundaryEventData.forEach(({ type, nodeType, eventXMLSnippet, taskType, invalidT
     it('can stay anchored to task when moving pool', function() {
       configurePool({ x: 300, y: 300 }, nodeType, taskType);
 
-      const taskSelector = '.main-paper ' +
-        '[data-type="processmaker.components.nodes.task.Shape"]';
-
       cy.get(boundaryEventSelector).then($boundaryEvent => {
         cy.get(taskSelector).then($task => {
           testThatBoundaryEventIsCloseToTask($boundaryEvent, $task);
diff --git a/tests/e2e/specs/BoundaryTimerEvent.spec.js b/tests/e2e/specs/BoundaryTimerEvent.spec.js
index 07d43a3f..51ff9b86 100644
--- a/tests/e2e/specs/BoundaryTimerEvent.spec.js
+++ b/tests/e2e/specs/BoundaryTimerEvent.spec.js
@@ -156,7 +156,7 @@ describe('Boundary Timer Event', () => {
     dragFromSourceToDest(nodeTypes.task, task2Position);
 
     const boundaryTimerEventSelector = '.main-paper ' +
-      '[data-type="processmaker.components.nodes.task.Shape"] ~ ' +
+      '[data-type="newCrown"] ~ ' +
       '[data-type="processmaker.components.nodes.boundaryEvent.Shape"]';
 
     getPositionInPaperCoords(task2Position).then(newPosition => {
@@ -205,7 +205,7 @@ describe('Boundary Timer Event', () => {
     dragFromSourceToDest(nodeTypes.task, task2Position);
 
     const boundaryTimerEventSelector = '.main-paper ' +
-      '[data-type="processmaker.components.nodes.task.Shape"] ~ ' +
+      '[data-type="newCrown"] ~ ' +
       '[data-type="processmaker.components.nodes.boundaryEvent.Shape"]';
 
     getPositionInPaperCoords(task2Position).then(newPosition => {
diff --git a/tests/e2e/specs/Modeler.spec.js b/tests/e2e/specs/Modeler.spec.js
index dba8ad19..0338b8bb 100644
--- a/tests/e2e/specs/Modeler.spec.js
+++ b/tests/e2e/specs/Modeler.spec.js
@@ -5,6 +5,7 @@ import {
   getElementAtPosition,
   getGraphElements,
   getLinksConnectedToElement,
+  getXml,
   isElementCovered,
   removeIndentationAndLinebreaks,
   typeIntoTextInput,
@@ -444,4 +445,13 @@ describe('Modeler', () => {
       getType().
       should('equal', nodeTypes.task);
   });
+
+  it('should not generate duplicate diagram IDs', function() {
+    uploadXml('setUpForDuplicateDiagramId.xml');
+    dragFromSourceToDest(nodeTypes.task, { x: 300, y: 300 });
+    getXml().then(xml => {
+      expect(xml.match(/node_1_di/g)).to.have.length(1);
+      expect(xml.match(/node_2_di/g)).to.have.length(1);
+    });
+  });
 });
diff --git a/tests/e2e/support/utils.js b/tests/e2e/support/utils.js
index 8236bcce..f0201262 100644
--- a/tests/e2e/support/utils.js
+++ b/tests/e2e/support/utils.js
@@ -92,6 +92,9 @@ export function getPositionInPaperCoords(position) {
 }
 
 export function getCrownButtonForElement($element, crownButton) {
+  if ($element.data('type') === 'newCrown') {
+    return cy.get(`#${ crownButton }`);
+  }
   return cy
     .get(`#${$element.attr('id')} ~ [data-test=${crownButton}]`)
     .then(crownButtons => crownButtons.filter((index, button) => Cypress.$(button).is(':visible')))
@@ -245,22 +248,21 @@ export function getNumberOfLinks() {
     .then(({ graph }) => graph.getLinks().length);
 }
 
-export function assertDownloadedXmlContainsExpected(xmlString) {
+export function getXml() {
   cy.get('[data-test=downloadXMLBtn]').click();
-  cy.window()
+  return cy.window()
     .its('xml')
-    .then(removeIndentationAndLinebreaks)
-    .then(xml => {
-      expect(xml).to.contain(removeIndentationAndLinebreaks(xmlString));
-    });
+    .then(removeIndentationAndLinebreaks);
+}
+
+export function assertDownloadedXmlContainsExpected(xmlString) {
+  getXml().then(xml => {
+    expect(xml).to.contain(removeIndentationAndLinebreaks(xmlString));
+  });
 }
 
 export function assertDownloadedXmlDoesNotContainExpected(xmlString) {
-  cy.get('[data-test=downloadXMLBtn]').click();
-  cy.window()
-    .its('xml')
-    .then(removeIndentationAndLinebreaks)
-    .then(xml => {
-      expect(xml).to.not.contain(removeIndentationAndLinebreaks(xmlString));
-    });
+  getXml().then(xml => {
+    expect(xml).to.not.contain(removeIndentationAndLinebreaks(xmlString));
+  });
 }
diff --git a/tests/unit/nodeIdGenerator.spec.js b/tests/unit/nodeIdGenerator.spec.js
new file mode 100644
index 00000000..966263e4
--- /dev/null
+++ b/tests/unit/nodeIdGenerator.spec.js
@@ -0,0 +1,81 @@
+import NodeIdGenerator from '@/NodeIdGenerator';
+import zip from 'lodash/zip';
+
+function definitionsFactory(definitionIds = [], diagramIds = []) {
+  const planeElement = zip(definitionIds, diagramIds).map(([definitionId, diagramId]) => ({
+    id: diagramId,
+    bpmnElement: { id: definitionId },
+    get(key) {
+      return this[key];
+    },
+  }));
+
+  return {
+    diagrams: [{
+      plane: {
+        planeElement,
+        get(key) {
+          return this[key];
+        },
+      },
+    }],
+  };
+}
+
+describe('NodeIdGenerator', () => {
+  it.each([['definition', 0], ['diagram', 1]])('should generate unique %s IDs', (name, idIndex) => {
+    const generator = new NodeIdGenerator(definitionsFactory());
+    const ids = [];
+    const numberOfIdsToGenerate = 10;
+
+    for (let i = 0; i < numberOfIdsToGenerate; i++) {
+      ids.push(generator.generate()[idIndex]);
+    }
+
+    while (ids.length > 0) {
+      expect(ids).not.toContain(ids.pop());
+    }
+  });
+
+  it('should not generate definition IDs that already exist in definitions', () => {
+    const duplicateGenerator = new NodeIdGenerator(definitionsFactory());
+    const definitionIds = [
+      duplicateGenerator.generate()[0],
+      duplicateGenerator.generate()[0],
+      duplicateGenerator.generate()[0],
+    ];
+    const generator = new NodeIdGenerator(definitionsFactory(definitionIds));
+
+    const newDefinitionIds = [
+      generator.generate()[0],
+      generator.generate()[0],
+      generator.generate()[0],
+      generator.generate()[0],
+    ];
+
+    while (newDefinitionIds.length > 0) {
+      expect(definitionIds).not.toContain(newDefinitionIds.pop());
+    }
+  });
+
+  it('should not generate diagram IDs that already exist in definitions', () => {
+    const duplicateGenerator = new NodeIdGenerator(definitionsFactory());
+    const diagramIds = [
+      duplicateGenerator.generate()[1],
+      duplicateGenerator.generate()[1],
+      duplicateGenerator.generate()[1],
+    ];
+    const generator = new NodeIdGenerator(definitionsFactory([], diagramIds));
+
+    const newDefinitionIds = [
+      generator.generate()[1],
+      generator.generate()[1],
+      generator.generate()[1],
+      generator.generate()[1],
+    ];
+
+    while (newDefinitionIds.length > 0) {
+      expect(diagramIds).not.toContain(newDefinitionIds.pop());
+    }
+  });
+});
diff --git a/tests/unit/nodeManager.spec.js b/tests/unit/nodeManager.spec.js
index 66f1ba84..ab9d275e 100644
--- a/tests/unit/nodeManager.spec.js
+++ b/tests/unit/nodeManager.spec.js
@@ -4,17 +4,22 @@ import { id as laneId } from '@/components/nodes/poolLane';
 
 describe('nodeManager', () => {
   describe('addIdToNodeAndSetUpDiagramReference', () => {
-    it('should not change diagram ID if it already has an ID set', () => {
+    it('should not change node ID if it already has an ID set', () => {
       const nodeId = 'foobar';
+      const newNodeId = 'baz';
+      const diagramId = 'diagram_id';
       const node = {
         definition: { id: nodeId },
         diagram: {},
       };
+      const nodeIdGenerator = {
+        generate: () => ([newNodeId, diagramId]),
+      };
 
-      addIdToNodeAndSetUpDiagramReference(node);
+      addIdToNodeAndSetUpDiagramReference(node, nodeIdGenerator);
 
       expect(node.definition.id).toBe(nodeId);
-      expect(node.diagram.id).toBe(`${nodeId}_di`);
+      expect(node.diagram.id).toBe(diagramId);
       expect(node.diagram.bpmnElement).toBe(node.definition);
     });
 
@@ -22,13 +27,17 @@ describe('nodeManager', () => {
       const node = {
         definition: { id: 123 },
       };
+      const nodeIdGenerator = {
+        generate: () => ([]),
+      };
 
-      expect(() => addIdToNodeAndSetUpDiagramReference(node)).not.toThrow();
+      expect(() => addIdToNodeAndSetUpDiagramReference(node, nodeIdGenerator)).not.toThrow();
     });
 
     it('should use ID generator if definition does not have ID', () => {
       const nodeId = 'foobar';
-      const nodeIdGenerator = { generateUniqueNodeId: () => nodeId };
+      const diagramId = 'baz';
+      const nodeIdGenerator = { generate: () => [nodeId, diagramId] };
       const node = {
         definition: {},
         diagram: {},
@@ -37,7 +46,7 @@ describe('nodeManager', () => {
       addIdToNodeAndSetUpDiagramReference(node, nodeIdGenerator);
 
       expect(node.definition.id).toBe(nodeId);
-      expect(node.diagram.id).toBe(`${nodeId}_di`);
+      expect(node.diagram.id).toBe(diagramId);
     });
   });