chore(replace): retain definitions where appropriate

* simplify implementation * retain event definitions when switching from interrupting to non-interrupting and vice versa Closes #799
2018-06-27 15:11:34 +02:00 · 2018-06-27 15:11:34 +02:00 · 75c0880341
parent 628e2d1019
commit 75c0880341
4 changed files with 192 additions and 68 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -6,6 +6,8 @@ All notable changes to [bpmn-js](https://github.com/bpmn-io/bpmn-js) are documen

 ___Note:__ Yet to be released changes appear here._

+* `FIX`: keep event definitions when switching from interrupting to non-interrupting boundary event ([#799](https://github.com/bpmn-io/bpmn-js/issues/799))
+
 ## 2.3.0

 * `CHORE`: update to `diagram-js@2.4.0`
--- a/lib/features/replace/BpmnReplace.js
+++ b/lib/features/replace/BpmnReplace.js
@ -1,14 +1,13 @@
 import {
  pick,
  assign,
-  uniqueBy,
-  findIndex,
  filter,
  has
 } from 'min-dash';

 import {
  is,
+  getBusinessObject
 } from '../../util/ModelUtil';

 import {
@ -110,29 +109,35 @@ export default function BpmnReplace(

    var elementProps = getProperties(oldBusinessObject.$descriptor),
        newElementProps = getProperties(newBusinessObject.$descriptor, true),
-        properties = uniqueBy(function(e) { return e; }, filter(elementProps, function(value) {
-          return findIndex(newElementProps, value) !== -1;
-        }));
+        copyProps = intersection(elementProps, newElementProps);

    // initialize special properties defined in target definition
    assign(newBusinessObject, pick(target, CUSTOM_PROPERTIES));

-    properties = filter(properties, function(property) {
+    var properties = filter(copyProps, function(property) {
      var propName = property.replace(/bpmn:/, '');

-      // so the applied properties from 'target' don't get lost
-      if (newBusinessObject[property] !== undefined) {
-        return false;
+      // copying event definitions, unless we replace
+      if (propName === 'eventDefinitions') {
+        return hasEventDefinition(element, target.eventDefinitionType);
      }

-      // retain loop characteristics if the target element is not an event sub process
+      // retain loop characteristics if the target element
+      // is not an event sub process
      if (propName === 'loopCharacteristics') {
        return !isEventSubProcess(newBusinessObject);
      }

-      if ((propName === 'processRef' && target.isExpanded === false) ||
-           propName === 'triggeredByEvent' ||
-           propName === 'eventDefinitions') {
+      // so the applied properties from 'target' don't get lost
+      if (property in newBusinessObject) {
+        return false;
+      }
+
+      if (propName === 'processRef' && target.isExpanded === false) {
+        return false;
+      }
+
+      if (propName === 'triggeredByEvent') {
        return false;
      }

@ -143,7 +148,13 @@ export default function BpmnReplace(

    // initialize custom BPMN extensions
    if (target.eventDefinitionType) {
-      newElement.eventDefinitionType = target.eventDefinitionType;
+
+      // only initialize with new eventDefinition
+      // if we did not set an event definition yet,
+      // i.e. because we cloned it
+      if (!hasEventDefinition(newBusinessObject, target.eventDefinitionType)) {
+        newElement.eventDefinitionType = target.eventDefinitionType;
+      }
    }

    if (is(oldBusinessObject, 'bpmn:Activity')) {
@ -232,4 +243,22 @@ BpmnReplace.$inject = [

 function isSubProcess(bo) {
  return is(bo, 'bpmn:SubProcess');
+}
+
+function hasEventDefinition(element, type) {
+
+  var bo = getBusinessObject(element);
+
+  return type && bo.get('eventDefinitions').some(function(definition) {
+    return is(definition, type);
+  });
+}
+
+/**
+ * Compute intersection between two arrays.
+ */
+function intersection(a1, a2) {
+  return a1.filter(function(el) {
+    return a2.indexOf(el) !== -1;
+  });
 }
--- a/test/fixtures/bpmn/features/replace/01_replace.bpmn
+++ b/test/fixtures/bpmn/features/replace/01_replace.bpmn
@ -1,5 +1,5 @@
 <?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:di="http://www.omg.org/spec/DD/20100524/DI" xmlns:dc="http://www.omg.org/spec/DD/20100524/DC" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" id="Definitions_1" targetNamespace="http://bpmn.io/schema/bpmn" exporter="Camunda Modeler" exporterVersion="1.14.0">
+<bpmn:definitions xmlns:bpmn="http://www.omg.org/spec/BPMN/20100524/MODEL" xmlns:bpmndi="http://www.omg.org/spec/BPMN/20100524/DI" xmlns:di="http://www.omg.org/spec/DD/20100524/DI" xmlns:dc="http://www.omg.org/spec/DD/20100524/DC" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" id="Definitions_1" targetNamespace="http://bpmn.io/schema/bpmn" exporter="Camunda Modeler" exporterVersion="1.16.1">
  <bpmn:process id="Process_1" isExecutable="false">
    <bpmn:startEvent id="StartEvent_1" name="KEEP ME">
      <bpmn:outgoing>SequenceFlow_1</bpmn:outgoing>
@ -43,11 +43,15 @@
    <bpmn:adHocSubProcess id="AdHocSubProcessCollapsed" />
    <bpmn:adHocSubProcess id="AdHocSubProcessExpanded" />
    <bpmn:boundaryEvent id="BoundaryEvent_1" cancelActivity="false" attachedToRef="Task_1">
-      <bpmn:timerEventDefinition />
+      <bpmn:timerEventDefinition>
+        <bpmn:timeDuration>P1D</bpmn:timeDuration>
+      </bpmn:timerEventDefinition>
    </bpmn:boundaryEvent>
    <bpmn:boundaryEvent id="BoundaryEvent_2" attachedToRef="Task_1">
      <bpmn:outgoing>SequenceFlow_7</bpmn:outgoing>
-      <bpmn:conditionalEventDefinition />
+      <bpmn:conditionalEventDefinition>
+        <bpmn:condition xsi:type="bpmn:tFormalExpression">${a &lt; b}</bpmn:condition>
+      </bpmn:conditionalEventDefinition>
    </bpmn:boundaryEvent>
    <bpmn:sequenceFlow id="SequenceFlow_6" sourceRef="SubProcess_1" targetRef="Transaction_1" />
    <bpmn:sequenceFlow id="SequenceFlow_7" sourceRef="BoundaryEvent_2" targetRef="SubProcess_1" />
--- a/test/spec/features/replace/BpmnReplaceSpec.js
+++ b/test/spec/features/replace/BpmnReplaceSpec.js
@ -133,70 +133,147 @@ describe('features/replace - bpmn replace', function() {
    }));


-    it('non interrupting boundary event by interrupting boundary event',
-      inject(function(elementRegistry, modeling, bpmnReplace, canvas) {
+    describe('boundary event', function() {

-        // given
-        var boundaryEvent = elementRegistry.get('BoundaryEvent_1'),
-            newElementData = {
-              type: 'bpmn:BoundaryEvent',
-              eventDefinitionType: 'bpmn:EscalationEventDefinition'
-            };
+      it('<non-interrupting> with <interrupting>',
+        inject(function(elementRegistry, modeling, bpmnReplace, canvas) {

-        // when
-        var newElement = bpmnReplace.replaceElement(boundaryEvent, newElementData);
+          // given
+          var boundaryEvent = elementRegistry.get('BoundaryEvent_1'),
+              boundaryBo = boundaryEvent.businessObject,
+              newElementData = {
+                type: 'bpmn:BoundaryEvent',
+                eventDefinitionType: 'bpmn:TimerEventDefinition'
+              };

-        // then
-        expect(newElement).to.exist;
-        expect(is(newElement.businessObject, 'bpmn:BoundaryEvent')).to.be.true;
-        expect(newElement.businessObject.eventDefinitions[0].$type).to.equal('bpmn:EscalationEventDefinition');
-        expect(newElement.businessObject.cancelActivity).to.be.true;
-      })
-    );
+          var eventDefinitions = boundaryBo.eventDefinitions.slice();
+
+          // when
+          var newElement = bpmnReplace.replaceElement(boundaryEvent, newElementData);
+          var newBo = newElement.businessObject;
+
+          // then
+          expect(newElement).to.exist;
+
+          expect(is(newBo, 'bpmn:BoundaryEvent')).to.be.true;
+
+          expect(newBo.eventDefinitions).to.jsonEqual(eventDefinitions, skipId);
+
+          expect(newBo.cancelActivity).to.be.true;
+        })
+      );


-    it('interrupting boundary event by non interrupting boundary event',
-      inject(function(elementRegistry, modeling, bpmnReplace, canvas) {
+      it('<interrupting> with <non-interrupting>',
+        inject(function(elementRegistry, modeling, bpmnReplace, canvas) {

-        // given
-        var boundaryEvent = elementRegistry.get('BoundaryEvent_2'),
-            newElementData = {
-              type: 'bpmn:BoundaryEvent',
-              eventDefinitionType: 'bpmn:SignalEventDefinition',
-              cancelActivity: false
-            };
+          // given
+          var boundaryEvent = elementRegistry.get('BoundaryEvent_2'),
+              boundaryBo = boundaryEvent.businessObject,
+              newElementData = {
+                type: 'bpmn:BoundaryEvent',
+                eventDefinitionType: 'bpmn:ConditionalEventDefinition',
+                cancelActivity: false
+              };

-        // when
-        var newElement = bpmnReplace.replaceElement(boundaryEvent, newElementData);
+          var eventDefinitions = boundaryBo.eventDefinitions.slice();

-        // then
-        expect(newElement).to.exist;
-        expect(is(newElement, 'bpmn:BoundaryEvent')).to.be.true;
-        expect(newElement.businessObject.eventDefinitions[0].$type).to.equal('bpmn:SignalEventDefinition');
-        expect(newElement.businessObject.cancelActivity).to.be.false;
-      })
-    );
+          // when
+          var newElement = bpmnReplace.replaceElement(boundaryEvent, newElementData);
+          var newBo = newElement.businessObject;
+
+          // then
+          expect(newElement).to.exist;
+
+          expect(is(newBo, 'bpmn:BoundaryEvent')).to.be.true;
+
+          expect(newBo.eventDefinitions).to.jsonEqual(eventDefinitions, skipId);
+
+          expect(newBo.cancelActivity).to.be.false;
+        })
+      );


-    it('boundary event and update host',
-      inject(function(elementRegistry, modeling, bpmnReplace, canvas) {
+      it('<timer> with <signal>',
+        inject(function(elementRegistry, modeling, bpmnReplace, canvas) {

-        // given
-        var boundaryEvent = elementRegistry.get('BoundaryEvent_1'),
-            host = elementRegistry.get('Task_1'),
-            newElementData = {
-              type: 'bpmn:BoundaryEvent',
-              eventDefinitionType: 'bpmn:ErrorEventDefinition'
-            };
+          // given
+          var boundaryEvent = elementRegistry.get('BoundaryEvent_1'),
+              newElementData = {
+                type: 'bpmn:BoundaryEvent',
+                eventDefinitionType: 'bpmn:SignalEventDefinition',
+                cancelActivity: false
+              };

-        // when
-        var newElement = bpmnReplace.replaceElement(boundaryEvent, newElementData);
+          // when
+          var newElement = bpmnReplace.replaceElement(boundaryEvent, newElementData);

-        // then
-        expect(newElement.host).to.exist;
-        expect(newElement.host).to.eql(host);
-      })
-    );
+          var newBo = newElement.businessObject;
+          var newEventDefinitions = newBo.eventDefinitions;
+          var newEventDefinition = newEventDefinitions[0];
+
+          // then
+          expect(newElement).to.exist;
+          expect(newEventDefinitions).to.have.length(1);
+
+          expect(is(newBo, 'bpmn:BoundaryEvent')).to.be.true;
+          expect(is(newEventDefinition, 'bpmn:SignalEventDefinition')).to.be.true;
+
+          expect(newBo.cancelActivity).to.be.false;
+        })
+      );
+
+
+      it('<conditional> with <timer>',
+        inject(function(elementRegistry, modeling, bpmnReplace, canvas) {
+
+          // given
+          var boundaryEvent = elementRegistry.get('BoundaryEvent_2'),
+              newElementData = {
+                type: 'bpmn:BoundaryEvent',
+                eventDefinitionType: 'bpmn:TimerEventDefinition'
+              };
+
+          // when
+          var newElement = bpmnReplace.replaceElement(boundaryEvent, newElementData);
+
+          var newBo = newElement.businessObject;
+          var newEventDefinitions = newBo.eventDefinitions;
+          var newEventDefinition = newEventDefinitions[0];
+
+          // then
+          expect(newElement).to.exist;
+          expect(newEventDefinitions).to.have.length(1);
+
+          expect(is(newBo, 'bpmn:BoundaryEvent')).to.be.true;
+          expect(is(newEventDefinition, 'bpmn:TimerEventDefinition')).to.be.true;
+
+          expect(newBo.cancelActivity).to.be.true;
+        })
+      );
+
+
+      it('updating host',
+        inject(function(elementRegistry, modeling, bpmnReplace, canvas) {
+
+          // given
+          var boundaryEvent = elementRegistry.get('BoundaryEvent_1'),
+              host = elementRegistry.get('Task_1'),
+              newElementData = {
+                type: 'bpmn:BoundaryEvent',
+                eventDefinitionType: 'bpmn:ErrorEventDefinition'
+              };
+
+          // when
+          var newElement = bpmnReplace.replaceElement(boundaryEvent, newElementData);
+
+          // then
+          expect(newElement.host).to.exist;
+          expect(newElement.host).to.eql(host);
+        })
+      );
+
+    });

  });

@ -1385,3 +1462,15 @@ describe('features/replace - bpmn replace', function() {
  });

 });
+
+
+// helpers ////////////////////////
+
+function skipId(key, value) {
+
+  if (key === 'id') {
+    return;
+  }
+
+  return value;
+}