fix(ordering): correctly attach boundary element in front of task

The way we create labels during import (right after the respective
element) interfered with the ordering during boundary attach.

This commit fixes the behavior.

Closes #437

lib/features/ordering/BpmnOrderingProvider.js

@@ -103,6 +103,14 @@ function BpmnOrderingProvider(eventBus) {
     var currentIndex = newParent.children.indexOf(element);
 
     var insertIndex = findIndex(newParent.children, function(child) {
+
+      // do not compare with labels, they are created
+      // in the wrong order (right after elements) during import and
+      // mess up the positioning.
+      if (!element.labelTarget && child.labelTarget) {
+        return false;
+      }
+
       return elementOrder.level < getOrder(child).level;
     });
 

test/spec/features/ordering/BpmnOrderingProviderSpec.js

@@ -5,6 +5,7 @@ var Helper = require('./Helper');
 /* global bootstrapModeler, inject */
 
 var move = Helper.move,
+    attach = Helper.attach,
     expectZOrder = Helper.expectZOrder;
 
 var modelingModule = require('../../../../lib/features/modeling'),
@@ -15,39 +16,66 @@ describe('features/modeling - ordering', function() {
 
   var testModules = [ coreModule, modelingModule ];
 
+
   describe('boundary events', function() {
 
-    var diagramXML = require('./ordering.bpmn');
+    describe('move', function() {
 
-    beforeEach(bootstrapModeler(diagramXML, { modules: testModules }));
+      var diagramXML = require('./ordering.bpmn');
 
-    it('should stay in front of Task', inject(function() {
-
-      // when
-      move('Task_With_Boundary');
-
-      // then
-      expectZOrder('Task_With_Boundary', 'BoundaryEvent');
-    }));
+      beforeEach(bootstrapModeler(diagramXML, { modules: testModules }));
 
 
-    it('should stay in front of Task, moving both', inject(function() {
+      it('should stay in front of Task', inject(function() {
 
-      // when
-      move([ 'BoundaryEvent', 'Task_With_Boundary' ], 'Participant_StartEvent');
+        // when
+        move('Task_With_Boundary');
 
-      // then
-      expectZOrder('Task_With_Boundary', 'BoundaryEvent');
-    }));
+        // then
+        expectZOrder('Task_With_Boundary', 'BoundaryEvent');
+      }));
+
+
+      it('should stay in front of Task, moving both', inject(function() {
+
+        // when
+        move([ 'BoundaryEvent', 'Task_With_Boundary' ], 'Participant_StartEvent');
+
+        // then
+        expectZOrder('Task_With_Boundary', 'BoundaryEvent');
+      }));
+
+    });
+
+
+    describe('add', function() {
+
+      var diagramXML = require('./ordering-start-event.bpmn');
+
+      beforeEach(bootstrapModeler(diagramXML, { modules: testModules }));
+
+
+      it('should add in front of Task', inject(function() {
+
+        // when
+        var boundaryShape = attach({ type: 'bpmn:BoundaryEvent' }, { x: 300, y: 80 }, 'Task');
+
+        // then
+        expectZOrder('Task', boundaryShape.id);
+      }));
+
+    });
 
   });
 
+
   describe('participants', function() {
 
     var diagramXML = require('./ordering.bpmn');
 
     beforeEach(bootstrapModeler(diagramXML, { modules: testModules }));
 
+
     it('should stay behind MessageFlow', inject(function() {
 
       // when
@@ -66,6 +94,7 @@ describe('features/modeling - ordering', function() {
 
     beforeEach(bootstrapModeler(diagramXML, { modules: testModules }));
 
+
     it('should stay behind boundary events', inject(function() {
 
       // when

test/spec/features/ordering/Helper.js

@@ -55,10 +55,43 @@ function move(elementIds, delta, targetId, isAttach) {
   });
 }
 
-
 module.exports.move = move;
 
 
+function add(attrs, position, target, isAttach) {
+
+  return TestHelper.getBpmnJS().invoke(function(canvas, elementRegistry, modeling) {
+
+    function getElement(id) {
+
+      var element = elementRegistry.get(id);
+
+      expect(element).to.exist;
+
+      return element;
+    }
+
+    if (!target) {
+      target = canvas.getRootElement();
+    } else
+    if (typeof target === 'string') {
+      target = getElement(target);
+    }
+
+    return modeling.createShape(attrs, position, target, isAttach);
+  });
+}
+
+module.exports.add = add;
+
+
+function attach(attrs, position, target) {
+  return add(attrs, position, target, true);
+}
+
+module.exports.attach = attach;
+
+
 function getAncestors(element) {
   var ancestors = [];
 

test/spec/features/ordering/ordering-start-event.bpmn

@@ -0,0 +1,17 @@
+<?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" id="Definitions_1" targetNamespace="http://bpmn.io/schema/bpmn">
+  <bpmn:process id="Process_1" isExecutable="false">
+    <bpmn:startEvent id="StartEvent_1" />
+    <bpmn:task id="Task" />
+  </bpmn:process>
+  <bpmndi:BPMNDiagram id="BPMNDiagram_1">
+    <bpmndi:BPMNPlane id="BPMNPlane_1" bpmnElement="Process_1">
+      <bpmndi:BPMNShape id="_BPMNShape_StartEvent_2" bpmnElement="StartEvent_1">
+        <dc:Bounds x="173" y="102" width="36" height="36" />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="Task_di" bpmnElement="Task">
+        <dc:Bounds x="271" y="80" width="100" height="80" />
+      </bpmndi:BPMNShape>
+    </bpmndi:BPMNPlane>
+  </bpmndi:BPMNDiagram>
+</bpmn:definitions>