From f89fd529de5dc2c2e556e755aa23db619fdd5306 Mon Sep 17 00:00:00 2001 From: Nico Rehwaldt Date: Mon, 4 Jan 2016 13:22:04 +0100 Subject: [PATCH] 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 | 8 +++ .../ordering/BpmnOrderingProviderSpec.js | 61 ++++++++++++++----- test/spec/features/ordering/Helper.js | 35 ++++++++++- .../ordering/ordering-start-event.bpmn | 17 ++++++ 4 files changed, 104 insertions(+), 17 deletions(-) create mode 100644 test/spec/features/ordering/ordering-start-event.bpmn diff --git a/lib/features/ordering/BpmnOrderingProvider.js b/lib/features/ordering/BpmnOrderingProvider.js index 0722ad99..79d45489 100644 --- a/lib/features/ordering/BpmnOrderingProvider.js +++ b/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; }); diff --git a/test/spec/features/ordering/BpmnOrderingProviderSpec.js b/test/spec/features/ordering/BpmnOrderingProviderSpec.js index e87cf80c..eb7ed465 100644 --- a/test/spec/features/ordering/BpmnOrderingProviderSpec.js +++ b/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 diff --git a/test/spec/features/ordering/Helper.js b/test/spec/features/ordering/Helper.js index 6a6f3011..34181228 100644 --- a/test/spec/features/ordering/Helper.js +++ b/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 = []; diff --git a/test/spec/features/ordering/ordering-start-event.bpmn b/test/spec/features/ordering/ordering-start-event.bpmn new file mode 100644 index 00000000..19e4341b --- /dev/null +++ b/test/spec/features/ordering/ordering-start-event.bpmn @@ -0,0 +1,17 @@ + + + + + + + + + + + + + + + + +