Repository: ProcessMaker/modeler (js)

Issue:
Highlight node red when missing ID
Related to https://github.com/ProcessMaker/modeler/issues/1026.

Elements that fail BPMNLint validation receive a red border, however, elements that are missing an ID do not receive this red border highlight. Ensure that an element with a missing ID is highlighted red when validation is enabled.

Pull request:
Highlight nodes without an ID red
Fixes #1174.

<img width="547" alt="Screen Shot 2020-04-23 at 3 57 08 PM" src="https://user-images.githubusercontent.com/7561061/80240055-42c4cc80-862f-11ea-906d-e995cbb90113.png">

**Note:** Until a new version of the `bpmnlint-plugin-processmaker` package is published and pulled into modeler (to have the node ID rule, https://github.com/ProcessMaker/bpmnlint-plugin/pull/5, enabled), the validation status won't show the missing ID error.

Patch:
diff --git a/src/components/modeler/Modeler.vue b/src/components/modeler/Modeler.vue
index bc62c547..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 { getAssociationFlowsForNode, 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: {
diff --git a/src/components/modeler/modelerUtils.js b/src/components/modeler/modelerUtils.js
index 82eed9ae..d5b151b5 100644
--- a/src/components/modeler/modelerUtils.js
+++ b/src/components/modeler/modelerUtils.js
@@ -39,3 +39,14 @@ export function getAssociationFlowsForNode(node, processes) {
     .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];
+}


Test patch:
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);
+  });
+});