Repository: ProcessMaker/modeler
Pull request #1202 (2020-04-17 17:00:13)

Validation of signal reference
Requires: https://github.com/ProcessMaker/bpmnlint-plugin/pull/6

This PR includes:

- Add required validation for signal selector

![image](https://user-images.githubusercontent.com/8028650/79594548-2fec4e00-80ab-11ea-8ad9-95ef8a5d69c0.png)




Patch:
diff --git a/src/ModelerApp.vue b/src/ModelerApp.vue
index d9807d87..0b2fc543 100644
--- a/src/ModelerApp.vue
+++ b/src/ModelerApp.vue
@@ -51,6 +51,7 @@
 import Modeler from './components/modeler/Modeler.vue';
 import FileUpload from 'vue-upload-component';
 import ValidationStatus from '@/components/validationStatus/ValidationStatus';
+import undoRedoStore from '@/undoRedoStore';
 
 const reader = new FileReader();
 
@@ -84,6 +85,7 @@ export default {
   methods: {
     loadXmlIntoModeler() {
       this.$refs.modeler.loadXML(this.uploadedXml);
+      undoRedoStore.dispatch('pushState', this.uploadedXml);
     },
     clearUpload() {
       this.uploadedXml = null;
diff --git a/src/components/crown/crownButtons/crownColorDropdown.vue b/src/components/crown/crownButtons/crownColorDropdown.vue
index 2e950fdf..f943dc65 100644
--- a/src/components/crown/crownButtons/crownColorDropdown.vue
+++ b/src/components/crown/crownButtons/crownColorDropdown.vue
@@ -83,7 +83,6 @@ export default {
       this.selectedColor = hex8;
     },
     unsetNodeColor() {
-      Vue.delete(this.node.definition, 'color');
       store.commit('updateNodeProp', { node: this.node, key: 'color', value: undefined });
       this.$emit('save-state');
     },
diff --git a/src/components/crown/crownConfig/crownConfig.vue b/src/components/crown/crownConfig/crownConfig.vue
index 5d0470bc..0771b9ab 100644
--- a/src/components/crown/crownConfig/crownConfig.vue
+++ b/src/components/crown/crownConfig/crownConfig.vue
@@ -79,7 +79,6 @@ import CopyButton from '@/components/crown/crownButtons/copyButton.vue';
 import CrownDropdowns from '@/components/crown/crownButtons/crownDropdowns';
 import DefaultFlow from '@/components/crown/crownButtons/defaultFlowButton.vue';
 import poolLaneCrownConfig from '@/mixins/poolLaneCrownConfig';
-import { removeFlows } from '@/components/crown/utils.js';
 import pull from 'lodash/pull';
 import store from '@/store';
 import isEqual from 'lodash/isEqual';
@@ -182,7 +181,6 @@ export default {
     paperNotRendered() {
       return !this.isRendering;
     },
-    removeFlows,
     replaceNodeTypePrompt(node) {
       if (this.taskDropdownInitiallyOpen) {
         this.$emit('replace-node', node);
@@ -192,7 +190,6 @@ export default {
       this.nodeToReplace = node;
     },
     confirmedReplaceNodeType() {
-      this.removeFlows(this.graph, this.shape, true);
       this.$emit('replace-node', this.nodeToReplace);
     },
     setNodePosition() {
diff --git a/src/components/inspectors/SignalSelect.vue b/src/components/inspectors/SignalSelect.vue
index fba8a440..a4eb4e9e 100644
--- a/src/components/inspectors/SignalSelect.vue
+++ b/src/components/inspectors/SignalSelect.vue
@@ -1,7 +1,7 @@
 <template>
   <div>
     <label class="typo__label">{{ label }}</label>
-    <div class="d-flex">
+    <div class="d-flex" :class="{invalid}">
       <multiselect
         :value="selectedOption"
         @input="change"
@@ -30,6 +30,9 @@
         </button>
       </div>
     </div>
+    <div v-if="invalid" class="invalid-feedback d-block"><div>
+      {{ $t('Signal reference is required') }}
+    </div></div>
     <small v-if="helper" class="form-text text-muted">{{ $t(helper) }}</small>
     <div v-if="showNewSignal" class="card">
       <div class="card-body p-2">
@@ -133,6 +136,9 @@ export default {
     },
   },
   computed: {
+    invalid() {
+      return !this.value;
+    },
     localSignals() {
       return store.getters.rootElements
         .filter(element => element.$type === 'bpmn:Signal');
@@ -347,3 +353,9 @@ export default {
     padding: 0px
   }
 </style>
+
+<style>
+  .invalid .multiselect__tags {
+    border-color:#dc3545!important
+  }
+</style>
diff --git a/src/components/modeler/Modeler.vue b/src/components/modeler/Modeler.vue
index b973d1fb..8b4e2f44 100644
--- a/src/components/modeler/Modeler.vue
+++ b/src/components/modeler/Modeler.vue
@@ -71,7 +71,7 @@
         :node="node"
         :id="node.id"
         :highlighted="highlightedNodes.includes(node)"
-        :has-error="invalidNodes.includes(node.id)"
+        :has-error="invalidNodes.includes(node)"
         :border-outline="borderOutline(node.id)"
         :collaboration="collaboration"
         :process-node="processNode"
@@ -139,7 +139,7 @@ import TimerEventNode from '@/components/nodes/timerEventNode';
 import focusNameInputAndHighlightLabel from '@/components/modeler/focusNameInputAndHighlightLabel';
 import XMLManager from '@/components/modeler/XMLManager';
 import { removeOutgoingAndIncomingRefsToFlow } from '@/components/crown/utils';
-import { keepOriginalName } from '@/components/modeler/modelerUtils';
+import { getAssociationFlowsForNode, getInvalidNodes, keepOriginalName } from '@/components/modeler/modelerUtils';
 
 export default {
   components: {
@@ -247,8 +247,7 @@ export default {
     highlightedNode: () => store.getters.highlightedNodes[0],
     highlightedNodes: () => store.getters.highlightedNodes,
     invalidNodes() {
-      return Object.entries(this.validationErrors)
-        .flatMap(([, errors]) => errors.map(error => error.id));
+      return getInvalidNodes(this.validationErrors, this.nodes);
     },
   },
   methods: {
@@ -725,6 +724,29 @@ export default {
         forceNodeToRemount(ref);
       });
 
+      const associationFlows = getAssociationFlowsForNode(nodeThatWillBeReplaced, this.processes);
+      associationFlows.forEach(flow => {
+        flow.set('targetRef', newNode.definition);
+        forceNodeToRemount(flow);
+      });
+
+      if (this.collaboration) {
+        const messageFlows = this.collaboration.get('messageFlows');
+        messageFlows
+          .filter(flow => flow.sourceRef === nodeThatWillBeReplaced.definition)
+          .forEach(flow => {
+            flow.set('sourceRef', newNode.definition);
+            forceNodeToRemount(flow);
+          });
+
+        messageFlows
+          .filter(flow => flow.targetRef === nodeThatWillBeReplaced.definition)
+          .forEach(flow => {
+            flow.set('targetRef', newNode.definition);
+            forceNodeToRemount(flow);
+          });
+      }
+
       return newNode;
     },
     setShapeCenterUnderCursor(diagram) {
diff --git a/src/components/modeler/modelerUtils.js b/src/components/modeler/modelerUtils.js
index b4cfccb2..d5b151b5 100644
--- a/src/components/modeler/modelerUtils.js
+++ b/src/components/modeler/modelerUtils.js
@@ -1,9 +1,9 @@
 import {
-  defaultStartNames,
   defaultEndNames,
-  defaultTaskNames,
   defaultGatewayNames,
   defaultIntermediateNames,
+  defaultStartNames,
+  defaultTaskNames,
 } from '@/components/nodes/defaultNames';
 
 function getDefaultNames(node) {
@@ -32,3 +32,21 @@ export function keepOriginalName(node) {
   const defaultNames = getDefaultNames(node);
   return defaultNames ? !Object.values(defaultNames).includes(node.definition.name) : true;
 }
+
+export function getAssociationFlowsForNode(node, processes) {
+  return processes
+    .reduce((artifacts, process) => artifacts.concat(process.get('artifacts')), [])
+    .filter(artifact => artifact.$type === 'bpmn:Association')
+    .filter(association => association.targetRef === node.definition);
+}
+
+export function getInvalidNodes(validationErrors, nodes) {
+  const invalidNodes = Object.values(validationErrors)
+    .flatMap(errors => {
+      return errors.map(error => nodes.find(node => node.id === error.id));
+    });
+
+  const nodesWithoutIds = nodes.filter(node => !node.id);
+
+  return [...invalidNodes, ...nodesWithoutIds];
+}
diff --git a/src/components/nodes/boundaryErrorEvent/index.js b/src/components/nodes/boundaryErrorEvent/index.js
index bdb24861..654746b0 100644
--- a/src/components/nodes/boundaryErrorEvent/index.js
+++ b/src/components/nodes/boundaryErrorEvent/index.js
@@ -2,6 +2,7 @@ import component from './boundaryErrorEvent.vue';
 import boundaryEventConfig from '../boundaryEvent';
 import merge from 'lodash/merge';
 import cloneDeep from 'lodash/cloneDeep';
+import advancedAccordionConfig from '@/components/inspectors/advancedAccordionConfig';
 
 export const id = 'processmaker-modeler-boundary-error-event';
 
@@ -19,4 +20,16 @@ export default merge(cloneDeep(boundaryEventConfig), {
       ],
     });
   },
+  inspectorConfig: [
+    {
+      items: [
+        {
+          items: [
+            {},
+          ],
+        },
+        advancedAccordionConfig,
+      ],
+    },
+  ],
 });
diff --git a/src/components/nodes/boundaryEscalationEvent/index.js b/src/components/nodes/boundaryEscalationEvent/index.js
index cf504386..4da8ee4f 100644
--- a/src/components/nodes/boundaryEscalationEvent/index.js
+++ b/src/components/nodes/boundaryEscalationEvent/index.js
@@ -3,6 +3,7 @@ import boundaryEventConfig from '../boundaryEvent';
 import merge from 'lodash/merge';
 import cloneDeep from 'lodash/cloneDeep';
 import interruptingToggleConfig from '../boundaryEvent/interruptingToggleInspector';
+import advancedAccordionConfig from '@/components/inspectors/advancedAccordionConfig';
 
 export const id = 'processmaker-modeler-boundary-escalation-event';
 
@@ -30,6 +31,7 @@ export default merge(cloneDeep(boundaryEventConfig), {
             interruptingToggleConfig,
           ],
         },
+        advancedAccordionConfig,
       ],
     },
   ],
diff --git a/src/components/nodes/boundaryEvent/boundaryEventDropdownData.js b/src/components/nodes/boundaryEvent/boundaryEventDropdownData.js
new file mode 100644
index 00000000..d17c8b16
--- /dev/null
+++ b/src/components/nodes/boundaryEvent/boundaryEventDropdownData.js
@@ -0,0 +1,30 @@
+const boundaryEventDropdownData = [
+  {
+    label: 'Boundary Timer Event',
+    nodeType: 'processmaker-modeler-boundary-timer-event',
+    dataTest: 'add-boundary-timer-event',
+  },
+  {
+    label: 'Boundary Error Event',
+    nodeType: 'processmaker-modeler-boundary-error-event',
+    dataTest: 'add-boundary-error-event',
+  },
+  {
+    label: 'Boundary Signal Event',
+    nodeType: 'processmaker-modeler-boundary-signal-event',
+    dataTest: 'add-boundary-signal-event',
+  },
+  /*{
+    label: 'Boundary Conditional Event',
+    nodeType: 'processmaker-modeler-boundary-conditional-event',
+    dataTest: 'add-boundary-conditional-event',
+  },*/
+  {
+    label: 'Boundary Message Event',
+    nodeType: 'processmaker-modeler-boundary-message-event',
+    dataTest: 'add-boundary-message-event',
+    disabledLabel: 'Allowed on Sub Process',
+  },
+];
+
+export default boundaryEventDropdownData;
diff --git a/src/components/nodes/boundaryEvent/index.js b/src/components/nodes/boundaryEvent/index.js
index f4e8c2f7..7378edf8 100644
--- a/src/components/nodes/boundaryEvent/index.js
+++ b/src/components/nodes/boundaryEvent/index.js
@@ -1,6 +1,5 @@
 import component from './boundaryEvent.vue';
 import nameConfigSettings from '@/components/inspectors/nameConfigSettings';
-import advancedAccordionConfig from '@/components/inspectors/advancedAccordionConfig';
 
 export default {
   id: 'processmaker-modeler-boundary-event',
@@ -46,7 +45,6 @@ export default {
             },
           ],
         },
-        advancedAccordionConfig,
       ],
     },
   ],
diff --git a/src/components/nodes/boundaryMessageEvent/index.js b/src/components/nodes/boundaryMessageEvent/index.js
index 9007c8f2..584c5deb 100644
--- a/src/components/nodes/boundaryMessageEvent/index.js
+++ b/src/components/nodes/boundaryMessageEvent/index.js
@@ -3,6 +3,7 @@ import boundaryEventConfig from '../boundaryEvent';
 import merge from 'lodash/merge';
 import cloneDeep from 'lodash/cloneDeep';
 import interruptingToggleConfig from '../boundaryEvent/interruptingToggleInspector';
+import advancedAccordionConfig from '@/components/inspectors/advancedAccordionConfig';
 
 export const id = 'processmaker-modeler-boundary-message-event';
 export default merge(cloneDeep(boundaryEventConfig), {
@@ -29,6 +30,7 @@ export default merge(cloneDeep(boundaryEventConfig), {
             interruptingToggleConfig,
           ],
         },
+        advancedAccordionConfig,
       ],
     },
   ],
diff --git a/src/components/nodes/boundarySignalEvent/index.js b/src/components/nodes/boundarySignalEvent/index.js
index 2b729a6e..8d8c52ca 100644
--- a/src/components/nodes/boundarySignalEvent/index.js
+++ b/src/components/nodes/boundarySignalEvent/index.js
@@ -4,6 +4,7 @@ import cloneDeep from 'lodash/cloneDeep';
 import boundaryEventConfig from '@/components/nodes/boundaryEvent';
 import interruptingToggleConfig from '@/components/nodes/boundaryEvent/interruptingToggleInspector';
 import {signalSelector, default as signalEventDefinition} from '../signalEventDefinition';
+import advancedAccordionConfig from '@/components/inspectors/advancedAccordionConfig';
 
 export const id = 'processmaker-modeler-boundary-signal-event';
 
@@ -33,6 +34,7 @@ export default merge(cloneDeep(boundaryEventConfig), {
             signalSelector('Signal that will trigger this boundary event'),
           ],
         },
+        advancedAccordionConfig,
       ],
     },
   ],
diff --git a/src/components/nodes/node.js b/src/components/nodes/node.js
index 2e8c0e10..e973b6f3 100644
--- a/src/components/nodes/node.js
+++ b/src/components/nodes/node.js
@@ -6,9 +6,12 @@ import {
   defaultTaskNames,
 } from '@/components/nodes/defaultNames';
 
+import cloneDeep from 'lodash/cloneDeep';
+
 export default class Node {
   static diagramPropertiesToCopy = ['x', 'y', 'width', 'height'];
   static definitionPropertiesToNotCopy = ['$type', 'id'];
+  static eventDefinitionPropertiesToNotCopy = ['errorRef', 'messageRef'];
 
   type;
   definition;
@@ -87,8 +90,14 @@ export default class Node {
     clonedNode.id = null;
     Node.diagramPropertiesToCopy.forEach(prop => clonedNode.diagram.bounds[prop] = this.diagram.bounds[prop]);
     Object.keys(this.definition).filter(key => !Node.definitionPropertiesToNotCopy.includes(key)).forEach(key => {
-      clonedNode.definition.set(key, this.definition.get(key));
+      const definition = this.definition.get(key);
+      clonedNode.definition.set(key, typeof definition === 'object' ? cloneDeep(definition) : definition);
     });
+    Node.eventDefinitionPropertiesToNotCopy.forEach(
+      prop => clonedNode.definition.eventDefinitions &&
+        clonedNode.definition.eventDefinitions[0].hasOwnProperty(prop) &&
+        clonedNode.definition.eventDefinitions[0].set(prop, null)
+    );
 
     return clonedNode;
   }
diff --git a/src/components/nodes/subProcess/subProcess.vue b/src/components/nodes/subProcess/subProcess.vue
index 3ce1fef9..5a85dad8 100644
--- a/src/components/nodes/subProcess/subProcess.vue
+++ b/src/components/nodes/subProcess/subProcess.vue
@@ -28,6 +28,7 @@ import hideLabelOnDrag from '@/mixins/hideLabelOnDrag';
 import CrownConfig from '@/components/crown/crownConfig/crownConfig';
 import highlightConfig from '@/mixins/highlightConfig';
 import defaultNames from '@/components/nodes/task/defaultNames';
+import boundaryEventDropdownData from '@/components/nodes/boundaryEvent/boundaryEventDropdownData';
 
 const labelPadding = 15;
 const topAndBottomMarkersSpace = 2 * markerSize;
@@ -52,23 +53,7 @@ export default {
   mixins: [highlightConfig, portsConfig, hasMarkers, hideLabelOnDrag],
   data() {
     return {
-      boundaryEventDropdownData: [
-        {
-          label: 'Boundary Timer Event',
-          nodeType: 'processmaker-modeler-boundary-timer-event',
-          dataTest: 'add-boundary-timer-event',
-        },
-        {
-          label: 'Boundary Error Event',
-          nodeType: 'processmaker-modeler-boundary-error-event',
-          dataTest: 'add-boundary-error-event',
-        },
-        {
-          label: 'Boundary Message Event',
-          nodeType: 'processmaker-modeler-boundary-message-event',
-          dataTest: 'add-boundary-message-event',
-        },
-      ],
+      boundaryEventDropdownData,
       dropdownData: [
         {
           label: defaultNames['processmaker-modeler-task'],
diff --git a/src/components/nodes/task/index.js b/src/components/nodes/task/index.js
index 8bec8d33..3c3dc5d0 100644
--- a/src/components/nodes/task/index.js
+++ b/src/components/nodes/task/index.js
@@ -14,7 +14,7 @@ export default {
   category: 'BPMN',
   rank: 4,
   icon: require('@/assets/toolpanel/task.svg'),
-  label: defaultNames[id],
+  label: 'Task',
   definition(moddle, $t) {
     return moddle.create('bpmn:Task', {
       name: $t(defaultNames[id]),
diff --git a/src/components/nodes/task/task.vue b/src/components/nodes/task/task.vue
index 8aec5688..d2b98e57 100644
--- a/src/components/nodes/task/task.vue
+++ b/src/components/nodes/task/task.vue
@@ -30,6 +30,7 @@ import { gridSize } from '@/graph';
 import sequentialIcon from '@/assets/sequential.svg';
 import parallelIcon from '@/assets/parallel.svg';
 import defaultNames from '@/components/nodes/task/defaultNames';
+import boundaryEventDropdownData from '@/components/nodes/boundaryEvent/boundaryEventDropdownData';
 
 const labelPadding = 15;
 const topAndBottomMarkersSpace = 2 * markerSize;
@@ -78,29 +79,7 @@ export default {
           dataTest: 'switch-to-sub-process',
         },
       ],
-      boundaryEventDropdownData: [
-        {
-          label: 'Boundary Timer Event',
-          nodeType: 'processmaker-modeler-boundary-timer-event',
-          dataTest: 'add-boundary-timer-event',
-        },
-        {
-          label: 'Boundary Error Event',
-          nodeType: 'processmaker-modeler-boundary-error-event',
-          dataTest: 'add-boundary-error-event',
-        },
-        {
-          label: 'Boundary Signal Event',
-          nodeType: 'processmaker-modeler-boundary-signal-event',
-          dataTest: 'add-boundary-signal-event',
-        },
-        {
-          label: 'Boundary Message Event',
-          nodeType: 'processmaker-modeler-boundary-message-event',
-          dataTest: 'add-boundary-message-event',
-          disabledLabel: 'Allowed on Sub Process',
-        },
-      ],
+      boundaryEventDropdownData,
     };
   },
   computed: {
diff --git a/src/components/nodes/textAnnotation/textAnnotation.vue b/src/components/nodes/textAnnotation/textAnnotation.vue
index b8a6abbf..dbbc598f 100644
--- a/src/components/nodes/textAnnotation/textAnnotation.vue
+++ b/src/components/nodes/textAnnotation/textAnnotation.vue
@@ -86,7 +86,6 @@ export default {
           text: util.breakText(text, {
             width: maxTextAnnotationWidth,
           }),
-          fill: 'black',
           textAnchor: 'left',
         },
       });
diff --git a/src/store.js b/src/store.js
index 2702d860..b7e507e4 100644
--- a/src/store.js
+++ b/src/store.js
@@ -76,6 +76,11 @@ export default new Vuex.Store({
     updateNodeProp(state, { node, key, value }) {
       node.definition.set(key, value);
 
+      if (value === undefined) {
+        Vue.delete(node.definition, key);
+        return;
+      }
+
       makeDefinitionPropertyReactive(node.definition, key, value);
     },
     clearNodes(state) {


Test patch:
diff --git a/tests/e2e/specs/AssociationFlows.spec.js b/tests/e2e/specs/AssociationFlows.spec.js
index ee45e6a6..3f97efb7 100644
--- a/tests/e2e/specs/AssociationFlows.spec.js
+++ b/tests/e2e/specs/AssociationFlows.spec.js
@@ -1,8 +1,10 @@
 import {
+  assertDownloadedXmlContainsExpected,
   connectNodesWithFlow,
   dragFromSourceToDest,
   getElementAtPosition,
   getLinksConnectedToElement,
+  modalConfirm,
 } from '../support/utils';
 
 import { direction } from '../../../src/components/nodes/association/associationConfig';
@@ -33,4 +35,26 @@ describe('Association Flows', () => {
     cy.get(directionSelectSelector).select('Both');
     cy.get(directionSelectSelector).should('have.value', direction.both);
   });
+
+  it('should keep association flow when changing element type', () => {
+    const startEventPosition = { x: 150, y: 150 };
+    const textAnnotationPosition = { x: 400, y: 100 };
+    dragFromSourceToDest(nodeTypes.textAnnotation, textAnnotationPosition);
+
+    connectNodesWithFlow('association-flow-button', textAnnotationPosition, startEventPosition);
+
+    cy.get('[data-test=select-type-dropdown]').click();
+    cy.get('[data-test=switch-to-start-timer-event]').click();
+    modalConfirm();
+
+    getElementAtPosition(startEventPosition)
+      .then(getLinksConnectedToElement)
+      .should($links => {
+        expect($links).to.have.lengthOf(1);
+      });
+
+    assertDownloadedXmlContainsExpected(`
+      <bpmn:association id="node_3" associationDirection="None" sourceRef="node_2" targetRef="node_4" />
+    `);
+  });
 });
diff --git a/tests/e2e/specs/CopyElements.spec.js b/tests/e2e/specs/CopyElements.spec.js
index 5f6621fb..f860678e 100644
--- a/tests/e2e/specs/CopyElements.spec.js
+++ b/tests/e2e/specs/CopyElements.spec.js
@@ -1,7 +1,10 @@
 import {
+  addNodeTypeToPaper,
   assertDownloadedXmlContainsExpected,
   dragFromSourceToDest,
-  getElementAtPosition, typeIntoTextInput,
+  getElementAtPosition,
+  typeIntoTextInput,
+  waitToRenderAllShapes,
 } from '../support/utils';
 import { nodeTypes } from '../support/constants';
 
@@ -63,4 +66,71 @@ describe('Copy element', () => {
 
     assertDownloadedXmlContainsExpected(processWithTwoStartEventCopies);
   });
+
+  it('copies error on Error End Event', () => {
+    const errorEndEventPosition = { x: 250, y: 250 };
+
+    addNodeTypeToPaper(errorEndEventPosition, nodeTypes.endEvent, 'switch-to-error-end-event');
+    waitToRenderAllShapes();
+
+    cy.get('[data-test=copy-button]').click();
+    waitToRenderAllShapes();
+
+    const process = `
+      <bpmn:process id="Process_1" isExecutable="true">
+        <bpmn:startEvent id="node_1" name="Start Event" />
+        <bpmn:endEvent id="node_3" name="Error End Event">
+          <bpmn:errorEventDefinition errorRef="node_3_error" />
+        </bpmn:endEvent>
+        <bpmn:endEvent id="node_4" name="Error End Event">
+          <bpmn:errorEventDefinition errorRef="node_4_error" />
+        </bpmn:endEvent>
+      </bpmn:process>
+      <bpmn:error id="node_3_error" name="node_3_error" />
+      <bpmn:error id="node_4_error" name="node_4_error" />
+    `;
+
+    assertDownloadedXmlContainsExpected(process);
+
+  });
+
+  it('copies message on Message Event', () => {
+    const messageEndEventPosition = { x: 250, y: 250 };
+    addNodeTypeToPaper(messageEndEventPosition, nodeTypes.endEvent, 'switch-to-message-end-event');
+    waitToRenderAllShapes();
+
+    cy.get('[data-test=copy-button]').click();
+    waitToRenderAllShapes();
+
+    const intermediateMessageThrowEventPosition = { x: 250, y: 450 };
+    dragFromSourceToDest(nodeTypes.intermediateCatchEvent, intermediateMessageThrowEventPosition);
+    cy.get('[data-test=switch-to-intermediate-message-throw-event]').click();
+    cy.get('[data-test=copy-button]').click();
+    waitToRenderAllShapes();
+
+    const process = `
+      <bpmn:process id="Process_1" isExecutable="true">
+        <bpmn:startEvent id="node_1" name="Start Event" />
+        <bpmn:endEvent id="node_3" name="Message End Event">
+          <bpmn:messageEventDefinition messageRef="node_3_message" />
+        </bpmn:endEvent>
+        <bpmn:endEvent id="node_4" name="Message End Event">
+          <bpmn:messageEventDefinition messageRef="node_4_message" />
+        </bpmn:endEvent>
+        <bpmn:intermediateThrowEvent id="node_6" name="Intermediate Message Throw Event">
+          <bpmn:messageEventDefinition messageRef="node_6_message" />
+        </bpmn:intermediateThrowEvent>
+        <bpmn:intermediateThrowEvent id="node_7" name="Intermediate Message Throw Event">
+          <bpmn:messageEventDefinition messageRef="node_7_message" />
+        </bpmn:intermediateThrowEvent>
+      </bpmn:process>
+      <bpmn:message id="node_3_message" name="node_3_message" />
+      <bpmn:message id="node_4_message" name="node_4_message" />
+      <bpmn:message id="node_6_message" name="node_6_message" />
+      <bpmn:message id="node_7_message" name="node_7_message" />
+    `;
+
+    assertDownloadedXmlContainsExpected(process);
+
+  });
 });
diff --git a/tests/e2e/specs/MessageFlows.spec.js b/tests/e2e/specs/MessageFlows.spec.js
index a7e74af1..c046fe45 100644
--- a/tests/e2e/specs/MessageFlows.spec.js
+++ b/tests/e2e/specs/MessageFlows.spec.js
@@ -1,7 +1,6 @@
 import {
   addNodeTypeToPaper,
   assertDownloadedXmlContainsExpected,
-  assertDownloadedXmlDoesNotContainExpected,
   connectNodesWithFlow,
   dragFromSourceToDest,
   getCrownButtonForElement,
@@ -197,7 +196,7 @@ describe('Message Flows', () => {
       .then(isElementCovered).should(isCovered => expect(isCovered).to.be.false);
   });
 
-  it('Removes message flows when switching task type', () => {
+  it('Retains message flows when switching task type', () => {
     const startEventPosition = { x: 150, y: 150 };
     const pool1Position = { x: 250, y: 250 };
     const pool2Position = { x: 250, y: 500 };
@@ -234,10 +233,10 @@ describe('Message Flows', () => {
     waitToRenderAllShapes();
 
     getElementAtPosition(taskPosition).then(getLinksConnectedToElement).should($links => {
-      expect($links.length).to.eq(--numberOfMessageFlowsAdded);
+      expect($links.length).to.eq(numberOfMessageFlowsAdded);
     });
 
-    assertDownloadedXmlDoesNotContainExpected(messageFlow);
+    assertDownloadedXmlContainsExpected(messageFlow);
   });
 
   it('should connect message end event to boundary message event in different pools', () => {
diff --git a/tests/e2e/specs/Modeler.spec.js b/tests/e2e/specs/Modeler.spec.js
index 15a132e2..d691ece5 100644
--- a/tests/e2e/specs/Modeler.spec.js
+++ b/tests/e2e/specs/Modeler.spec.js
@@ -14,7 +14,6 @@ import {
   typeIntoTextInput,
   uploadXml,
   waitToRenderAllShapes,
-  waitToRenderNodeUpdates,
 } from '../support/utils';
 
 import { nodeTypes } from '../support/constants';
@@ -136,34 +135,6 @@ describe('Modeler', () => {
     cy.get('[name=id]').should('have.value', 'node_1');
   });
 
-  it('Validates gateway direction', () => {
-    const gatewayPosition = { x: 250, y: 250 };
-    dragFromSourceToDest(nodeTypes.exclusiveGateway, gatewayPosition);
-    cy.get('[data-test=switch-to-inclusive-gateway]').click();
-
-    cy.get('[data-test="validation-toggle"]').click({ force: true });
-    cy.get('[data-test="validation-list-toggle"]').click();
-
-    cy.get('[data-test=validation-list]').should($lsit => {
-      expect($lsit).to.contain('Gateway must have multiple outgoing Sequence Flows');
-    });
-
-    cy.get('[data-test=validation-list-toggle]').click();
-
-    getElementAtPosition(gatewayPosition).click();
-
-    cy.contains('Advanced').click();
-    cy.get('[name=gatewayDirection]').select('Converging');
-
-    waitToRenderNodeUpdates();
-
-    cy.get('[data-test=validation-list-toggle]').click();
-
-    cy.get('[data-test=validation-list]').should($lsit => {
-      expect($lsit).to.contain('Gateway must have multiple incoming Sequence Flows');
-    });
-  });
-
   it('Adding a pool and lanes does not overlap sequence flow', () => {
     const startEventPosition = { x: 150, y: 150 };
     const taskPosition = { x: 250, y: 250 };
@@ -242,54 +213,6 @@ describe('Modeler', () => {
     cy.get('.invalid-feedback').should('not.have.value', 'The Node Identifier format is invalid.');
   });
 
-  it('updates validation after undo/redo', () => {
-    cy.get('[data-test=validation-toggle]').click({ force: true });
-    cy.get('[data-test=validation-list-toggle]').click();
-
-    const initialNumberOfDefinitionListElements = 4;
-    cy.get('[data-test=validation-list]').children().should('have.length', initialNumberOfDefinitionListElements);
-
-    const startEventPosition = { x: 150, y: 150 };
-
-    getElementAtPosition(startEventPosition).then($startEvent => {
-      cy.wrap($startEvent).find('[stroke=red]').should('exist');
-    });
-
-    const taskPosition = { x: 150, y: 300 };
-    dragFromSourceToDest(nodeTypes.task, taskPosition);
-
-    const numberOfNewDefinitionListElements = 2;
-    cy.get('[data-test=validation-list]').children()
-      .should('have.length', initialNumberOfDefinitionListElements + numberOfNewDefinitionListElements)
-      .should('contain', 'node_2');
-
-    cy.get('[data-test=undo]').click();
-    waitToRenderAllShapes();
-
-    getElementAtPosition(startEventPosition).then($startEvent => {
-      cy.wrap($startEvent).find('[stroke=red]').should('exist');
-    });
-
-    cy.get('[data-test=validation-list]').children()
-      .should('have.length', initialNumberOfDefinitionListElements)
-      .should('not.contain', 'node_2');
-
-    cy.get('[data-test=redo]').click();
-    waitToRenderAllShapes();
-
-    cy.get('[data-test=validation-list]').children()
-      .should('have.length', initialNumberOfDefinitionListElements + numberOfNewDefinitionListElements)
-      .should('contain', 'node_2');
-
-    getElementAtPosition(startEventPosition).then($startEvent => {
-      cy.wrap($startEvent).find('[stroke=red]').should('exist');
-    });
-
-    getElementAtPosition(taskPosition).then($task => {
-      cy.wrap($task).find('[stroke=red]').should('exist');
-    });
-  });
-
   it('shows warning for unknown element during parsing', () => {
     uploadXml('unknownElement.xml');
     const warning = 'DataStoreReference is an unsupported element type in parse';
@@ -505,38 +428,6 @@ describe('Modeler', () => {
     });
   });
 
-  it('Does not display a console error on multiple validation errors for one node', () => {
-    cy.window().then((win) => {
-      cy.spy(win.console, 'error');
-    });
-
-    const taskPosition1 = { x: 300, y: 250 };
-    const taskPosition2 = { x: 300, y: 350 };
-    const taskPosition3 = { x: 300, y: 450 };
-    const parallelGatewayPosition = { x: 200, y: 200 };
-
-    addNodeTypeToPaper(parallelGatewayPosition, nodeTypes.exclusiveGateway, 'switch-to-parallel-gateway');
-    dragFromSourceToDest(nodeTypes.task, taskPosition1);
-    dragFromSourceToDest(nodeTypes.task, taskPosition2);
-    dragFromSourceToDest(nodeTypes.task, taskPosition3);
-
-    connectNodesWithFlow('sequence-flow-button', taskPosition1, parallelGatewayPosition);
-    connectNodesWithFlow('sequence-flow-button', taskPosition2, parallelGatewayPosition);
-    connectNodesWithFlow('sequence-flow-button', parallelGatewayPosition, taskPosition3);
-
-    cy.get('[data-test="validation-toggle"]').click({ force: true });
-    cy.get('[data-test="validation-list-toggle"]').click();
-
-    cy.get('[data-test=validation-list]')
-      .should('contain.text', 'Gateway must have multiple outgoing Sequence Flows.Node ID: node_3');
-    cy.get('[data-test=validation-list]')
-      .should('contain.text', 'Gateway must not have multiple incoming Sequence Flows.Node ID: node_3');
-
-    cy.window().then((win) => {
-      expect(win.console.error).to.have.callCount(0);
-    });
-  });
-
   it('should add and remove documentation to element', () => {
     const position = { x: 300, y: 300 };
     const baseElements = [
diff --git a/tests/e2e/specs/TextAnnotation.spec.js b/tests/e2e/specs/TextAnnotation.spec.js
index f1bd59ff..71a3bccc 100644
--- a/tests/e2e/specs/TextAnnotation.spec.js
+++ b/tests/e2e/specs/TextAnnotation.spec.js
@@ -7,6 +7,7 @@ import {
 } from '../support/utils';
 
 import { nodeTypes } from '../support/constants';
+import { baseNodeColors } from '../../../src/components/nodeColors';
 
 describe('Text Annotation', () => {
   it('Update text annotation name', () => {
@@ -40,4 +41,18 @@ describe('Text Annotation', () => {
 
     assertDownloadedXmlContainsExpected(expectedXML);
   });
+
+  it('keeps custom color when updating node text', () => {
+    const colorToSelect = baseNodeColors[0];
+    const textAnnotationPosition = { x: 400, y: 100 };
+    dragFromSourceToDest(nodeTypes.textAnnotation, textAnnotationPosition);
+
+    getElementAtPosition(textAnnotationPosition).click();
+    cy.get('[data-test="picker-dropdown-button"]').click();
+    cy.get(`[data-test="${colorToSelect}"]`).click();
+    typeIntoTextInput('[name=text]', 'new text');
+
+    const annotationTextSelector = '.main-paper [data-type="textAnnotation"] [joint-selector="label"]';
+    cy.get(annotationTextSelector).should('have.attr', 'fill', colorToSelect);
+  });
 });
diff --git a/tests/e2e/specs/Validation.spec.js b/tests/e2e/specs/Validation.spec.js
new file mode 100644
index 00000000..6e6d33c7
--- /dev/null
+++ b/tests/e2e/specs/Validation.spec.js
@@ -0,0 +1,134 @@
+import {
+  addNodeTypeToPaper,
+  connectNodesWithFlow,
+  dragFromSourceToDest,
+  getElementAtPosition,
+  waitToRenderAllShapes,
+  waitToRenderNodeUpdates,
+} from '../support/utils';
+import { nodeTypes } from '../support/constants';
+
+describe('Validation', () => {
+
+  it('Validates gateway direction', () => {
+    const gatewayPosition = { x: 250, y: 250 };
+    dragFromSourceToDest(nodeTypes.exclusiveGateway, gatewayPosition);
+    cy.get('[data-test=switch-to-inclusive-gateway]').click();
+
+    cy.get('[data-test="validation-toggle"]').click({ force: true });
+    cy.get('[data-test="validation-list-toggle"]').click();
+
+    cy.get('[data-test=validation-list]').should($lsit => {
+      expect($lsit).to.contain('Gateway must have multiple outgoing Sequence Flows');
+    });
+
+    cy.get('[data-test=validation-list-toggle]').click();
+
+    getElementAtPosition(gatewayPosition).click();
+
+    cy.contains('Advanced').click();
+    cy.get('[name=gatewayDirection]').select('Converging');
+
+    waitToRenderNodeUpdates();
+
+    cy.get('[data-test=validation-list-toggle]').click();
+
+    cy.get('[data-test=validation-list]').should($lsit => {
+      expect($lsit).to.contain('Gateway must have multiple incoming Sequence Flows');
+    });
+  });
+
+  it('does not have forEach validation errors after emptying documentation', () => {
+    const startEventPosition = { x: 150, y: 150 };
+    getElementAtPosition(startEventPosition).click();
+
+    cy.contains('Advanced').click();
+    cy.get('[name=documentation]').type('Test');
+    cy.get('[name=documentation]').clear();
+
+    cy.get('[data-test=validation-toggle]').click({ force: true });
+    cy.get('[data-test=validation-list-toggle]').click({ force: true });
+
+    cy.get('[data-test=validation-list]').should('not.contain', 'Cannot read property \'forEach\' of undefined.');
+  });
+
+  it('updates validation after undo/redo', () => {
+    cy.get('[data-test=validation-toggle]').click({ force: true });
+    cy.get('[data-test=validation-list-toggle]').click();
+
+    const initialNumberOfDefinitionListElements = 4;
+    cy.get('[data-test=validation-list]').children().should('have.length', initialNumberOfDefinitionListElements);
+
+    const startEventPosition = { x: 150, y: 150 };
+
+    getElementAtPosition(startEventPosition).then($startEvent => {
+      cy.wrap($startEvent).find('[stroke=red]').should('exist');
+    });
+
+    const taskPosition = { x: 150, y: 300 };
+    dragFromSourceToDest(nodeTypes.task, taskPosition);
+
+    const numberOfNewDefinitionListElements = 2;
+    cy.get('[data-test=validation-list]').children()
+      .should('have.length', initialNumberOfDefinitionListElements + numberOfNewDefinitionListElements)
+      .should('contain', 'node_2');
+
+    cy.get('[data-test=undo]').click();
+    waitToRenderAllShapes();
+
+    getElementAtPosition(startEventPosition).then($startEvent => {
+      cy.wrap($startEvent).find('[stroke=red]').should('exist');
+    });
+
+    cy.get('[data-test=validation-list]').children()
+      .should('have.length', initialNumberOfDefinitionListElements)
+      .should('not.contain', 'node_2');
+
+    cy.get('[data-test=redo]').click();
+    waitToRenderAllShapes();
+
+    cy.get('[data-test=validation-list]').children()
+      .should('have.length', initialNumberOfDefinitionListElements + numberOfNewDefinitionListElements)
+      .should('contain', 'node_2');
+
+    getElementAtPosition(startEventPosition).then($startEvent => {
+      cy.wrap($startEvent).find('[stroke=red]').should('exist');
+    });
+
+    getElementAtPosition(taskPosition).then($task => {
+      cy.wrap($task).find('[stroke=red]').should('exist');
+    });
+  });
+
+  it('Does not display a console error on multiple validation errors for one node', () => {
+    cy.window().then((win) => {
+      cy.spy(win.console, 'error');
+    });
+
+    const taskPosition1 = { x: 300, y: 250 };
+    const taskPosition2 = { x: 300, y: 350 };
+    const taskPosition3 = { x: 300, y: 450 };
+    const parallelGatewayPosition = { x: 200, y: 200 };
+
+    addNodeTypeToPaper(parallelGatewayPosition, nodeTypes.exclusiveGateway, 'switch-to-parallel-gateway');
+    dragFromSourceToDest(nodeTypes.task, taskPosition1);
+    dragFromSourceToDest(nodeTypes.task, taskPosition2);
+    dragFromSourceToDest(nodeTypes.task, taskPosition3);
+
+    connectNodesWithFlow('sequence-flow-button', taskPosition1, parallelGatewayPosition);
+    connectNodesWithFlow('sequence-flow-button', taskPosition2, parallelGatewayPosition);
+    connectNodesWithFlow('sequence-flow-button', parallelGatewayPosition, taskPosition3);
+
+    cy.get('[data-test="validation-toggle"]').click({ force: true });
+    cy.get('[data-test="validation-list-toggle"]').click();
+
+    cy.get('[data-test=validation-list]')
+      .should('contain.text', 'Gateway must have multiple outgoing Sequence Flows.Node ID: node_3');
+    cy.get('[data-test=validation-list]')
+      .should('contain.text', 'Gateway must not have multiple incoming Sequence Flows.Node ID: node_3');
+
+    cy.window().then((win) => {
+      expect(win.console.error).to.have.callCount(0);
+    });
+  });
+});
diff --git a/tests/unit/keepOriginalName.spec.js b/tests/unit/modelerUtils.spec.js
similarity index 57%
rename from tests/unit/keepOriginalName.spec.js
rename to tests/unit/modelerUtils.spec.js
index 1a57050f..5a1878e2 100644
--- a/tests/unit/keepOriginalName.spec.js
+++ b/tests/unit/modelerUtils.spec.js
@@ -1,7 +1,7 @@
-import { keepOriginalName} from '@/components/modeler/modelerUtils';
+import { getInvalidNodes, keepOriginalName } from '@/components/modeler/modelerUtils';
 import Node from '@/components/nodes/node';
 
-describe('Switch Types', () => {
+describe('keepOriginalName', () => {
   const nodeTypes = [
     ['Form Task', 'processmaker-modeler-script-task'],
     ['Start Event', 'processmaker-modeler-start-timer-event'],
@@ -18,6 +18,7 @@ describe('Switch Types', () => {
     );
     expect(keepOriginalName(node)).toBe(true);
   });
+
   it.each(nodeTypes)('Should switch default %s name', (name, type) => {
     const node = new Node(
       type,
@@ -27,3 +28,19 @@ describe('Switch Types', () => {
     expect(keepOriginalName(node)).toBe(false);
   });
 });
+
+describe('getInvalidNodes', () => {
+  it('should return node if it does not have an ID', () => {
+    const node = new Node('processmaker-modeler-start-event', { id: '' });
+    const nodes = getInvalidNodes({}, [node]);
+
+    expect(nodes).toContain(node);
+  });
+
+  it('should not return node if it does have an ID', () => {
+    const node = new Node('processmaker-modeler-start-event', { id: 'node_1' });
+    const nodes = getInvalidNodes({}, [node]);
+
+    expect(nodes).not.toContain(node);
+  });
+});