fix(replace): persist multi-instance props with `isSequential` change

Closes #1581

lib/features/popup-menu/ReplaceMenuProvider.js

@@ -14,7 +14,8 @@ import {
 
 import {
   forEach,
-  filter
+  filter,
+  isUndefined
 } from 'min-dash';
 
 import * as replaceOptions from '../replace/ReplaceOptions';
@@ -426,16 +427,16 @@ ReplaceMenuProvider.prototype._getLoopEntries = function(element) {
   var translate = this._translate;
 
   function toggleLoopEntry(event, entry) {
-    var loopCharacteristics;
+    var loopCharacteristics = getBusinessObject(element).loopCharacteristics;
 
     if (entry.active) {
       loopCharacteristics = undefined;
     } else {
-      loopCharacteristics = self._moddle.create(entry.options.loopCharacteristics);
-
-      if (entry.options.isSequential) {
-        loopCharacteristics.isSequential = entry.options.isSequential;
+      if (isUndefined(entry.options.isSequential) || !loopCharacteristics) {
+        loopCharacteristics = self._moddle.create(entry.options.loopCharacteristics);
       }
+
+      loopCharacteristics.isSequential = entry.options.isSequential;
     }
     self._modeling.updateProperties(element, { loopCharacteristics: loopCharacteristics });
   }

test/fixtures/bpmn/draw/activity-markers-simple.bpmn

@@ -2,10 +2,16 @@
 <bpmn2:definitions xmlns:bpmn2="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="_opd4cBZiEeWgh4rX9Ivzlg" targetNamespace="http://activiti.org/bpmn" exporter="Camunda Modeler" exporterVersion="1.0.0" xsi:schemaLocation="http://www.omg.org/spec/BPMN/20100524/MODEL BPMN20.xsd">
   <bpmn2:process id="Process_1" isExecutable="false">
     <bpmn2:task id="ParallelTask" name="ParallelTask">
-      <bpmn2:multiInstanceLoopCharacteristics />
+      <bpmn2:multiInstanceLoopCharacteristics camunda:collection="foo" camunda:elementVariable="bar">
+        <bpmn2:loopCardinality xsi:type="bpmn2:tFormalExpression">foo</bpmn2:loopCardinality>
+        <bpmn2:completionCondition xsi:type="bpmn2:tFormalExpression">bar</bpmn2:completionCondition>
+      </bpmn2:multiInstanceLoopCharacteristics>
     </bpmn2:task>
     <bpmn2:task id="SequentialTask" name="SequentialTask">
-      <bpmn2:multiInstanceLoopCharacteristics isSequential="true" />
+      <bpmn2:multiInstanceLoopCharacteristics isSequential="true" camunda:collection="doo" camunda:elementVariable="bar">
+        <bpmn2:loopCardinality xsi:type="bpmn2:tFormalExpression">foo</bpmn2:loopCardinality>
+        <bpmn2:completionCondition xsi:type="bpmn2:tFormalExpression">bar</bpmn2:completionCondition>
+      </bpmn2:multiInstanceLoopCharacteristics>
     </bpmn2:task>
     <bpmn2:task id="LoopTask" name="LoopTask">
       <bpmn2:standardLoopCharacteristics />

test/spec/features/popup-menu/ReplaceMenuProviderSpec.js

@@ -14,6 +14,9 @@ import customRulesModule from '../../../util/custom-rules';
 import modelingModule from 'lib/features/modeling';
 import replaceMenuProviderModule from 'lib/features/popup-menu';
 
+import camundaModdleModule from 'camunda-bpmn-moddle/lib';
+import camundaPackage from 'camunda-bpmn-moddle/resources/camunda.json';
+
 import {
   query as domQuery,
   queryAll as domQueryAll,
@@ -23,6 +26,8 @@ import {
 import { is } from 'lib/util/ModelUtil';
 
 import { isExpanded } from 'lib/util/DiUtil';
+import { getBusinessObject } from '../../../../lib/util/ModelUtil';
+import { omit } from 'min-dash';
 
 
 describe('features/popup-menu - replace menu provider', function() {
@@ -281,7 +286,12 @@ describe('features/popup-menu - replace menu provider', function() {
 
   describe('toggle', function() {
 
-    beforeEach(bootstrapModeler(diagramXMLMarkers, { modules: testModules }));
+    beforeEach(bootstrapModeler(diagramXMLMarkers,{
+      modules: Object.assign(testModules, camundaModdleModule),
+      moddleExtensions: {
+        camunda: camundaPackage
+      }
+    }));
 
     var toggleActive;
 
@@ -500,6 +510,29 @@ describe('features/popup-menu - replace menu provider', function() {
         expect(domClasses(loopEntry).has('active')).to.be.false;
       }));
 
+
+      it('should keep sequential properties', inject(function(elementRegistry) {
+
+        // given
+        var task = elementRegistry.get('SequentialTask'),
+            businessObject = getBusinessObject(task),
+            loopCharacteristics = Object.assign({}, businessObject.loopCharacteristics);
+
+        openPopup(task);
+
+        // assume
+        expect(loopCharacteristics.isSequential).to.be.true;
+
+        // when
+        triggerAction('toggle-parallel-mi');
+
+        // then
+        var newLoopCharacteristics = businessObject.loopCharacteristics;
+
+        expect(newLoopCharacteristics.isSequential).to.be.false;
+        expect(omit(newLoopCharacteristics, 'isSequential')).to.eql(omit(loopCharacteristics, 'isSequential'));
+      }));
+
     });
 
 
@@ -583,6 +616,29 @@ describe('features/popup-menu - replace menu provider', function() {
         expect(domClasses(parallelEntry).has('active')).to.be.false;
       }));
 
+
+      it('should keep parallel properties', inject(function(elementRegistry) {
+
+        // given
+        var task = elementRegistry.get('ParallelTask'),
+            businessObject = getBusinessObject(task),
+            loopCharacteristics = Object.assign({}, businessObject.loopCharacteristics);
+
+        openPopup(task);
+
+        // assume
+        expect(loopCharacteristics.isSequential).to.be.undefined;
+
+        // when
+        triggerAction('toggle-sequential-mi');
+
+        // then
+        var newLoopCharacteristics = businessObject.loopCharacteristics;
+
+        expect(newLoopCharacteristics.isSequential).to.be.true;
+        expect(omit(newLoopCharacteristics, 'isSequential')).to.eql(loopCharacteristics);
+      }));
+
     });