fix: do not set colors when invalid

Closes #1734
bpmn-io · Sep 2, 2022 · 6e3819c · 6e3819c
1 parent be07ee8
commit 6e3819c
Show file tree

Hide file tree

Showing 3 changed files with 85 additions and 29 deletions.
diff --git a/lib/features/modeling/cmd/SetColorHandler.js b/lib/features/modeling/cmd/SetColorHandler.js
@@ -6,7 +6,8 @@ import {
 } from 'min-dash';
 
 import {
-  getDi
+  getDi,
+  isAny
 } from '../../../util/ModelUtil';
 
 import {
@@ -65,7 +66,8 @@ SetColorHandler.prototype.postExecute = function(context) {
   }
 
   forEach(elements, function(element) {
-    var assignedDi = isConnection(element) ? pick(di, [ 'border-color' ]) : di;
+    var assignedDi = isConnection(element) ? pick(di, [ 'border-color' ]) : di,
+        elementDi = getDi(element);
 
     // TODO @barmac: remove once we drop bpmn.io properties
     ensureLegacySupport(assignedDi);
@@ -75,13 +77,18 @@ SetColorHandler.prototype.postExecute = function(context) {
       // set label colors as bpmndi:BPMNLabel#color
       self._commandStack.execute('element.updateModdleProperties', {
         element: element,
-        moddleElement: getDi(element).label,
+        moddleElement: elementDi.label,
         properties: {
           color: di['border-color']
         }
       });
     } else {
 
+      // ignore non-compliant di
+      if (!isAny(elementDi, [ 'bpmndi:BPMNEdge', 'bpmndi:BPMNShape' ])) {
+        return;
+      }
+
       // set colors bpmndi:BPMNEdge or bpmndi:BPMNShape
       self._commandStack.execute('element.updateProperties', {
         element: element,

diff --git a/test/spec/features/modeling/SetColor.bpmn b/test/spec/features/modeling/SetColor.bpmn
@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<bpmn2:definitions xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:bpmn2="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" xmlns:di="http://www.omg.org/spec/DD/20100524/DI" xsi:schemaLocation="http://www.omg.org/spec/BPMN/20100524/MODEL BPMN20.xsd" id="simple" exporter="camunda modeler" exporterVersion="2.6.0" targetNamespace="http://bpmn.io/schema/bpmn">
+<bpmn2:definitions xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:bpmn2="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" xmlns:di="http://www.omg.org/spec/DD/20100524/DI" id="simple" targetNamespace="http://bpmn.io/schema/bpmn" exporter="Camunda Modeler" exporterVersion="5.2.0" xsi:schemaLocation="http://www.omg.org/spec/BPMN/20100524/MODEL BPMN20.xsd">
   <bpmn2:process id="Process_1" isExecutable="false">
     <bpmn2:subProcess id="SubProcess_1" name="Sub Process 1">
       <bpmn2:incoming>SequenceFlow_3</bpmn2:incoming>
@@ -10,52 +10,72 @@
       <bpmn2:task id="Task_1" name="Task">
         <bpmn2:incoming>SequenceFlow_1</bpmn2:incoming>
       </bpmn2:task>
-      <bpmn2:sequenceFlow id="SequenceFlow_1" name="" sourceRef="StartEvent_1" targetRef="Task_1"/>
+      <bpmn2:sequenceFlow id="SequenceFlow_1" name="" sourceRef="StartEvent_1" targetRef="Task_1" />
     </bpmn2:subProcess>
     <bpmn2:endEvent id="EndEvent_1" name="End Event">
       <bpmn2:incoming>SequenceFlow_2</bpmn2:incoming>
     </bpmn2:endEvent>
-    <bpmn2:sequenceFlow id="SequenceFlow_2" name="" sourceRef="SubProcess_1" targetRef="EndEvent_1"/>
+    <bpmn2:sequenceFlow id="SequenceFlow_2" name="" sourceRef="SubProcess_1" targetRef="EndEvent_1" />
     <bpmn2:startEvent id="StartEvent_2" name="Start">
       <bpmn2:outgoing>SequenceFlow_3</bpmn2:outgoing>
     </bpmn2:startEvent>
-    <bpmn2:sequenceFlow id="SequenceFlow_3" name="Flow" sourceRef="StartEvent_2" targetRef="SubProcess_1"/>
+    <bpmn2:sequenceFlow id="SequenceFlow_3" name="Flow" sourceRef="StartEvent_2" targetRef="SubProcess_1" />
+    <bpmn2:subProcess id="Collapsed" name="Collapsed">
+      <bpmn2:startEvent id="Event_0ql2pb3" />
+    </bpmn2:subProcess>
   </bpmn2:process>
   <bpmndi:BPMNDiagram id="BPMNDiagram_1">
     <bpmndi:BPMNPlane id="BPMNPlane_1" bpmnElement="Process_1">
+      <bpmndi:BPMNEdge id="BPMNEdge_SequenceFlow_3" bpmnElement="SequenceFlow_3" sourceElement="_BPMNShape_StartEvent_11" targetElement="_BPMNShape_SubProcess_2">
+        <di:waypoint x="194" y="230" />
+        <di:waypoint x="350" y="230" />
+        <bpmndi:BPMNLabel>
+          <dc:Bounds x="247" y="204" width="24" height="14" />
+        </bpmndi:BPMNLabel>
+      </bpmndi:BPMNEdge>
+      <bpmndi:BPMNEdge id="BPMNEdge_SequenceFlow_2" bpmnElement="SequenceFlow_2" sourceElement="_BPMNShape_SubProcess_2" targetElement="_BPMNShape_EndEvent_2">
+        <di:waypoint x="650" y="230" />
+        <di:waypoint x="700" y="230" />
+      </bpmndi:BPMNEdge>
       <bpmndi:BPMNShape id="_BPMNShape_SubProcess_2" bpmnElement="SubProcess_1" isExpanded="true">
-        <dc:Bounds height="300.0" width="300.0" x="300.0" y="80.0"/>
+        <dc:Bounds x="350" y="80" width="300" height="300" />
       </bpmndi:BPMNShape>
+      <bpmndi:BPMNEdge id="BPMNEdge_SequenceFlow_1" bpmnElement="SequenceFlow_1" sourceElement="_BPMNShape_StartEvent_2" targetElement="_BPMNShape_Task_2">
+        <di:waypoint x="438" y="260" />
+        <di:waypoint x="470" y="260" />
+      </bpmndi:BPMNEdge>
       <bpmndi:BPMNShape id="_BPMNShape_StartEvent_2" bpmnElement="StartEvent_1">
-        <dc:Bounds height="36.0" width="36.0" x="352.0" y="242.0"/>
+        <dc:Bounds x="402" y="242" width="36" height="36" />
+        <bpmndi:BPMNLabel>
+          <dc:Bounds x="388" y="278" width="64" height="14" />
+        </bpmndi:BPMNLabel>
       </bpmndi:BPMNShape>
       <bpmndi:BPMNShape id="_BPMNShape_Task_2" bpmnElement="Task_1">
-        <dc:Bounds height="80.0" width="100.0" x="420.0" y="220.0"/>
+        <dc:Bounds x="470" y="220" width="100" height="80" />
       </bpmndi:BPMNShape>
-      <bpmndi:BPMNEdge id="BPMNEdge_SequenceFlow_1" bpmnElement="SequenceFlow_1" sourceElement="_BPMNShape_StartEvent_2" targetElement="_BPMNShape_Task_2">
-        <di:waypoint xsi:type="dc:Point" x="388.0" y="260.0"/>
-        <di:waypoint xsi:type="dc:Point" x="420.0" y="260.0"/>
-      </bpmndi:BPMNEdge>
       <bpmndi:BPMNShape id="_BPMNShape_EndEvent_2" bpmnElement="EndEvent_1">
-        <dc:Bounds height="36.0" width="36.0" x="650.0" y="212.0"/>
-      </bpmndi:BPMNShape>
-      <bpmndi:BPMNEdge id="BPMNEdge_SequenceFlow_2" bpmnElement="SequenceFlow_2" sourceElement="_BPMNShape_SubProcess_2" targetElement="_BPMNShape_EndEvent_2">
-        <di:waypoint xsi:type="dc:Point" x="600.0" y="230.0"/>
-        <di:waypoint xsi:type="dc:Point" x="650.0" y="230.0"/>
-      </bpmndi:BPMNEdge>
-      <bpmndi:BPMNShape id="_BPMNShape_StartEvent_11" bpmnElement="StartEvent_2">
-        <dc:Bounds height="36.0" width="36.0" x="108.0" y="212.0"/>
+        <dc:Bounds x="700" y="212" width="36" height="36" />
         <bpmndi:BPMNLabel>
-          <dc:Bounds height="0.0" width="0.0" x="126.0" y="253.0"/>
+          <dc:Bounds x="693" y="248" width="51" height="14" />
         </bpmndi:BPMNLabel>
       </bpmndi:BPMNShape>
-      <bpmndi:BPMNEdge id="BPMNEdge_SequenceFlow_3" bpmnElement="SequenceFlow_3" sourceElement="_BPMNShape_StartEvent_11" targetElement="_BPMNShape_SubProcess_2">
-        <di:waypoint xsi:type="dc:Point" x="144.0" y="230.0"/>
-        <di:waypoint xsi:type="dc:Point" x="300.0" y="230.0"/>
+      <bpmndi:BPMNShape id="_BPMNShape_StartEvent_11" bpmnElement="StartEvent_2">
+        <dc:Bounds x="158" y="212" width="36" height="36" />
         <bpmndi:BPMNLabel>
-          <dc:Bounds height="21.0" width="33.0" x="192.0" y="204.0"/>
+          <dc:Bounds x="164" y="253" width="24" height="14" />
         </bpmndi:BPMNLabel>
-      </bpmndi:BPMNEdge>
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="Activity_0uievla_di" bpmnElement="Collapsed" isExpanded="false">
+        <dc:Bounds x="475" y="540" width="100" height="80" />
+        <bpmndi:BPMNLabel />
+      </bpmndi:BPMNShape>
+    </bpmndi:BPMNPlane>
+  </bpmndi:BPMNDiagram>
+  <bpmndi:BPMNDiagram id="BPMNDiagram_0dfg6ho">
+    <bpmndi:BPMNPlane id="BPMNPlane_SubProcess" bpmnElement="Collapsed">
+      <bpmndi:BPMNShape id="Event_0ql2pb3_di" bpmnElement="Event_0ql2pb3">
+        <dc:Bounds x="180" y="160" width="36" height="36" />
+      </bpmndi:BPMNShape>
     </bpmndi:BPMNPlane>
   </bpmndi:BPMNDiagram>
-</bpmn2:definitions>
+</bpmn2:definitions>
diff --git a/test/spec/features/modeling/SetColorSpec.js b/test/spec/features/modeling/SetColorSpec.js
@@ -315,6 +315,35 @@ describe('features/modeling - set color', function() {
       // then
       expect(setColor).to.throw(/^invalid color value/);
     }));
+
+
+    it('should ignore BPMNPlane (Process)', inject(function(elementRegistry, modeling) {
+
+      // given
+      var processElement = elementRegistry.get('Process_1'),
+          processDi = getDi(processElement);
+
+      // when
+      modeling.setColor(processElement, { fill: '#abcdef' });
+
+      // then
+      expect(processDi.get('background-color')).not.to.exist;
+    }));
+
+
+    it('should ignore BPMNPlane (SubProcess)', inject(function(canvas, elementRegistry, modeling) {
+
+      // given
+      canvas.setRootElement(canvas.findRoot('Collapsed_plane'));
+      var subprocess = canvas.getRootElement(),
+          subprocessDi = getDi(subprocess);
+
+      // when
+      modeling.setColor(subprocess, { fill: '#abcdef' });
+
+      // then
+      expect(subprocessDi.get('background-color')).not.to.exist;
+    }));
   });