fix: reusing process when creating first participant works on redo

* during `#preExecute` of `elements.create` process is passed to `shape.create` to be reused during `#execute` (there is no `#exeute` for `elements.create` as it only executes other commands during `#preExecute`)
* process must be reused during `#execute` of `shape.create` for `#redo` to work
* refactor implementation

Closes #1439

lib/features/modeling/behavior/CreateParticipantBehavior.js

@@ -2,7 +2,7 @@ import inherits from 'inherits';
 
 import CommandInterceptor from 'diagram-js/lib/command/CommandInterceptor';
 
-import { is } from '../../../util/ModelUtil';
+import { getBusinessObject, is } from '../../../util/ModelUtil';
 
 import { isLabel } from '../../../util/LabelUtil';
 
@@ -88,122 +88,97 @@ export default function CreateParticipantBehavior(canvas, eventBus, modeling) {
     }
   });
 
-  function ensureCollaboration(context) {
-    var parent = context.parent,
-        collaboration;
-
+  // turn process into collaboration when creating first participant
+  function getOrCreateCollaboration() {
     var rootElement = canvas.getRootElement();
 
     if (is(rootElement, 'bpmn:Collaboration')) {
-      collaboration = rootElement;
-    } else {
-
-      // update root element by making collaboration
-      collaboration = modeling.makeCollaboration();
-
-      // re-use process when creating first participant
-      context.process = parent;
+      return rootElement;
     }
 
-    context.parent = collaboration;
+    return modeling.makeCollaboration();
   }
 
-  // turn process into collaboration before adding participant
+  // when creating mutliple elements through `elements.create` parent must be set to collaboration
+  // and passed to `shape.create` as hint
+  this.preExecute('elements.create', HIGH_PRIORITY, function(context) {
+    var elements = context.elements,
+        parent = context.parent,
+        participant = findParticipant(elements),
+        hints;
+
+    if (participant && is(parent, 'bpmn:Process')) {
+      context.parent = getOrCreateCollaboration();
+
+      hints = context.hints = context.hints || {};
+
+      hints.participant = participant;
+      hints.process = parent;
+      hints.processRef = getBusinessObject(participant).get('processRef');
+    }
+  }, true);
+
+  // when creating single shape through `shape.create` parent must be set to collaboration
+  // unless it was already set through `elements.create`
   this.preExecute('shape.create', function(context) {
     var parent = context.parent,
         shape = context.shape;
 
     if (is(shape, 'bpmn:Participant') && is(parent, 'bpmn:Process')) {
-      ensureCollaboration(context);
+      context.parent = getOrCreateCollaboration();
+
+      context.process = parent;
+      context.processRef = getBusinessObject(shape).get('processRef');
     }
   }, true);
 
+  // #execute necessary because #preExecute not called on CommandStack#redo
   this.execute('shape.create', function(context) {
-    var process = context.process,
-        shape = context.shape;
+    var hints = context.hints || {},
+        process = context.process || hints.process,
+        shape = context.shape,
+        participant = hints.participant;
 
-    if (process) {
-      context.oldProcessRef = shape.businessObject.processRef;
+    // both shape.create and elements.create must be handled
+    if (process && (!participant || shape === participant)) {
 
-      // re-use process when creating first participant
-      shape.businessObject.processRef = process.businessObject;
+      // monkey-patch process ref
+      getBusinessObject(shape).set('processRef', getBusinessObject(process));
     }
   }, true);
 
   this.revert('shape.create', function(context) {
-    var process = context.process,
-        shape = context.shape;
+    var hints = context.hints || {},
+        process = context.process || hints.process,
+        processRef = context.processRef || hints.processRef,
+        shape = context.shape,
+        participant = hints.participant;
 
-    if (process) {
+    // both shape.create and elements.create must be handled
+    if (process && (!participant || shape === participant)) {
 
-      // re-use process when creating first participant
-      shape.businessObject.processRef = context.oldProcessRef;
+      // monkey-patch process ref
+      getBusinessObject(shape).set('processRef', processRef);
     }
   }, true);
 
   this.postExecute('shape.create', function(context) {
-    var process = context.process,
-        shape = context.shape;
+    var hints = context.hints || {},
+        process = context.process || context.hints.process,
+        shape = context.shape,
+        participant = hints.participant;
 
     if (process) {
+      var children = process.children.slice();
 
-      // move children from process to participant
-      var processChildren = process.children.slice();
-
-      modeling.moveElements(processChildren, { x: 0, y: 0 }, shape);
-    }
-
-  }, true);
-
-  // turn process into collaboration when creating participants
-  this.preExecute('elements.create', HIGH_PRIORITY, function(context) {
-    var elements = context.elements,
-        parent = context.parent,
-        participant;
-
-    var hasParticipants = findParticipant(elements);
-
-    if (hasParticipants && is(parent, 'bpmn:Process')) {
-      ensureCollaboration(context);
-
-      participant = findParticipant(elements);
-
-      context.oldProcessRef = participant.businessObject.processRef;
-
-      // re-use process when creating first participant
-      participant.businessObject.processRef = parent.businessObject;
+      // both shape.create and elements.create must be handled
+      if (!participant) {
+        modeling.moveElements(children, { x: 0, y: 0 }, shape);
+      } else if (shape === participant) {
+        modeling.moveElements(children, { x: 0, y: 0 }, participant);
+      }
     }
   }, true);
-
-  this.revert('elements.create', function(context) {
-    var elements = context.elements,
-        process = context.process,
-        participant;
-
-    if (process) {
-      participant = findParticipant(elements);
-
-      // re-use process when creating first participant
-      participant.businessObject.processRef = context.oldProcessRef;
-    }
-  }, true);
-
-  this.postExecute('elements.create', function(context) {
-    var elements = context.elements,
-        process = context.process,
-        participant;
-
-    if (process) {
-      participant = findParticipant(elements);
-
-      // move children from process to first participant
-      var processChildren = process.children.slice();
-
-      modeling.moveElements(processChildren, { x: 0, y: 0 }, participant);
-    }
-
-  }, true);
-
 }
 
 CreateParticipantBehavior.$inject = [

test/spec/features/modeling/behavior/CreateParticipantBehaviorSpec.js

@@ -29,7 +29,7 @@ describe('features/modeling - create participant', function() {
 
   describe('process', function() {
 
-    describe('should turn process into collaboration', function() {
+    describe('turning process into collaboration', function() {
 
       var processDiagramXML = require('../../../../fixtures/bpmn/collaboration/process-empty.bpmn');
 
@@ -89,6 +89,7 @@ describe('features/modeling - create participant', function() {
           // then
           expect(participantBo.$parent).to.equal(collaborationBo);
           expect(participantBo.processRef).to.equal(processBo);
+          expect(participantBo.processRef.id).to.equal(processBo.id);
 
           expect(collaborationBo.$instanceOf('bpmn:Collaboration')).to.be.true;
           expect(collaborationBo.$parent).to.equal(processBo.$parent);
@@ -114,6 +115,26 @@ describe('features/modeling - create participant', function() {
           expect(processDi.$parent).to.equal(diRoot);
         }));
 
+
+        it('redo', inject(function(commandStack) {
+
+          // when
+          commandStack.undo();
+          commandStack.redo();
+
+          // then
+          expect(participantBo.$parent).to.equal(collaborationBo);
+          expect(participantBo.processRef).to.equal(processBo);
+          expect(participantBo.processRef.id).to.equal(processBo.id);
+
+          expect(collaborationBo.$instanceOf('bpmn:Collaboration')).to.be.true;
+          expect(collaborationBo.$parent).to.equal(processBo.$parent);
+          expect(collaborationBo.participants).to.include(participantBo);
+
+          expect(participantDi.$parent).to.equal(collaborationDi);
+          expect(collaborationDi.$parent).to.equal(diRoot);
+        }));
+
       });
 
 
@@ -168,12 +189,35 @@ describe('features/modeling - create participant', function() {
           expect(processDi.$parent).to.equal(diRoot);
         }));
 
+
+        it('redo', inject(function(commandStack) {
+
+          // when
+          commandStack.undo();
+          commandStack.redo();
+
+          // then
+          expect(participantBo.$parent).to.equal(collaborationBo);
+          expect(participantBo.processRef).to.equal(processBo);
+
+          expect(participant2Bo.$parent).to.equal(collaborationBo);
+          expect(participant2Bo.processRef).not.to.equal(processBo);
+
+          expect(collaborationBo.$instanceOf('bpmn:Collaboration')).to.be.true;
+          expect(collaborationBo.$parent).to.equal(processBo.$parent);
+          expect(collaborationBo.participants).to.include(participantBo);
+
+          expect(participantDi.$parent).to.equal(collaborationDi);
+          expect(participant2Di.$parent).to.equal(collaborationDi);
+          expect(collaborationDi.$parent).to.equal(diRoot);
+        }));
+
       });
 
     });
 
 
-    describe('should move existing elements', function() {
+    describe('moving process children', function() {
 
       var processDiagramXML = require('../../../../fixtures/bpmn/collaboration/process.bpmn');