From dac5bb397b478c557e20c6440f111527faf8bb7e Mon Sep 17 00:00:00 2001 From: Nico Rehwaldt Date: Thu, 30 Jul 2015 10:48:17 +0200 Subject: [PATCH] fix(modeling/rules): correct boundary rules * clean up boundary related rules * split boundary tests into separate section * add boundary message flow tests * fix boundary message flow behavior Closes #319 --- lib/features/modeling/rules/BpmnRules.js | 46 +-- .../rules/BpmnRules.boundaryEvent.bpmn | 93 ++++++ .../rules/BpmnRules.collaboration.bpmn | 7 - .../modeling/rules/BpmnRules.process.bpmn | 14 - .../features/modeling/rules/BpmnRulesSpec.js | 290 +++++++++--------- test/spec/features/modeling/rules/Helper.js | 78 +++++ 6 files changed, 321 insertions(+), 207 deletions(-) create mode 100644 test/spec/features/modeling/rules/BpmnRules.boundaryEvent.bpmn create mode 100644 test/spec/features/modeling/rules/Helper.js diff --git a/lib/features/modeling/rules/BpmnRules.js b/lib/features/modeling/rules/BpmnRules.js index e4a4cd98..297e41e2 100644 --- a/lib/features/modeling/rules/BpmnRules.js +++ b/lib/features/modeling/rules/BpmnRules.js @@ -94,8 +94,6 @@ BpmnRules.prototype.canConnectSequenceFlow = canConnectSequenceFlow; BpmnRules.prototype.canConnectAssociation = canConnectAssociation; -BpmnRules.prototype.canConnectBoundaryEvent = canConnectBoundaryEvent; - BpmnRules.prototype.canMove = canMove; BpmnRules.prototype.canAttach = canAttach; @@ -145,7 +143,7 @@ function isSameOrganization(a, b) { } function isMessageFlowSource(element) { - return is(element, 'bpmn:InteractionNode') && ( is(element, 'bpmn:BoundaryEvent') || + return is(element, 'bpmn:InteractionNode') && ( !is(element, 'bpmn:Event') || ( is(element, 'bpmn:ThrowEvent') && hasEventDefinitionOrNone(element, 'bpmn:MessageEventDefinition') @@ -154,7 +152,7 @@ function isMessageFlowSource(element) { } function isMessageFlowTarget(element) { - return is(element, 'bpmn:InteractionNode') && ( is(element, 'bpmn:BoundaryEvent') || + return is(element, 'bpmn:InteractionNode') && ( !is(element, 'bpmn:Event') || ( is(element, 'bpmn:CatchEvent') && hasEventDefinitionOrNone(element, 'bpmn:MessageEventDefinition') @@ -212,7 +210,8 @@ function isSequenceFlowSource(element) { } function isSequenceFlowTarget(element) { - return is(element, 'bpmn:FlowNode') && !is(element, 'bpmn:StartEvent') && + return is(element, 'bpmn:FlowNode') && + !is(element, 'bpmn:StartEvent') && !is(element, 'bpmn:BoundaryEvent') && !(is(element, 'bpmn:IntermediateCatchEvent') && hasEventDefinition(element, 'bpmn:LinkEventDefinition')); @@ -257,10 +256,6 @@ function canConnect(source, target, connection) { return false; } - if (is(source, 'bpmn:BoundaryEvent')) { - return canConnectBoundaryEvent(source, target); - } - if (canConnectMessageFlow(source, target) || canConnectSequenceFlow(source, target)) { return true; @@ -417,7 +412,7 @@ function canCreate(shape, target, source, position) { return false; } - return canDrop(shape, target) || canInsert(shape, target); + return canDrop(shape, target, position) || canInsert(shape, target, position); } function canResize(shape, newBounds) { @@ -465,7 +460,7 @@ function canConnectSequenceFlow(source, target) { !(is(source, 'bpmn:EventBasedGateway') && !isEventBasedTarget(target)); } -function canInsert(shape, flow) { +function canInsert(shape, flow, position) { // return true if we can drop on the // underlying flow parent @@ -476,35 +471,10 @@ function canInsert(shape, flow) { is(flow, 'bpmn:SequenceFlow') || is(flow, 'bpmn:MessageFlow') ) && is(shape, 'bpmn:FlowNode') && !is(shape, 'bpmn:BoundaryEvent') && - canDrop(shape, flow.parent); + + canDrop(shape, flow.parent, position); } -function canConnectBoundaryEvent(source, target) { - if (source.businessObject.$parent.id === target.id) { - return false; - } - - if (source.businessObject.attachedToRef === target.id) { - return false; - } - - if (canConnectMessageFlow(source, target)) { - return true; - } - - if (is(target, 'bpmn:Activity')) { - return true; - } - - return false; -} - -/** - * Returns true if all elements have the same parent - * - * @param {Array} elements - * @return {Boolean} - */ function haveSameParent(elements) { return size(groupBy(elements, function(e) { return e.parent && e.parent.id; })) === 1; } diff --git a/test/spec/features/modeling/rules/BpmnRules.boundaryEvent.bpmn b/test/spec/features/modeling/rules/BpmnRules.boundaryEvent.bpmn new file mode 100644 index 00000000..f7361235 --- /dev/null +++ b/test/spec/features/modeling/rules/BpmnRules.boundaryEvent.bpmn @@ -0,0 +1,93 @@ + + + + + Boundary Events are awesome! + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/spec/features/modeling/rules/BpmnRules.collaboration.bpmn b/test/spec/features/modeling/rules/BpmnRules.collaboration.bpmn index 0589c235..89906be0 100644 --- a/test/spec/features/modeling/rules/BpmnRules.collaboration.bpmn +++ b/test/spec/features/modeling/rules/BpmnRules.collaboration.bpmn @@ -34,7 +34,6 @@ - @@ -129,12 +128,6 @@ - - - - - - diff --git a/test/spec/features/modeling/rules/BpmnRules.process.bpmn b/test/spec/features/modeling/rules/BpmnRules.process.bpmn index 67f7f8fd..933648dc 100644 --- a/test/spec/features/modeling/rules/BpmnRules.process.bpmn +++ b/test/spec/features/modeling/rules/BpmnRules.process.bpmn @@ -18,8 +18,6 @@ - - @@ -69,18 +67,6 @@ - - - - - - - - - - - - diff --git a/test/spec/features/modeling/rules/BpmnRulesSpec.js b/test/spec/features/modeling/rules/BpmnRulesSpec.js index 8faf2aec..46ff8096 100644 --- a/test/spec/features/modeling/rules/BpmnRulesSpec.js +++ b/test/spec/features/modeling/rules/BpmnRulesSpec.js @@ -1,6 +1,10 @@ 'use strict'; -var TestHelper = require('../../../../TestHelper'); +var Helper = require('./Helper'); + +var expectCanConnect = Helper.expectCanConnect, + expectCanDrop = Helper.expectCanDrop, + expectCanMove = Helper.expectCanMove; /* global bootstrapModeler, inject */ @@ -8,75 +12,6 @@ var modelingModule = require('../../../../../lib/features/modeling'), coreModule = require('../../../../../lib/core'); -function expectCanConnect(source, target, rules) { - - var results = {}; - - TestHelper.getBpmnJS().invoke(function(elementRegistry, bpmnRules) { - - source = elementRegistry.get(source); - target = elementRegistry.get(target); - - expect(source).to.exist; - expect(target).to.exist; - - if ('sequenceFlow' in rules) { - results.sequenceFlow = bpmnRules.canConnectSequenceFlow(source, target); - } - - if ('messageFlow' in rules) { - results.messageFlow = bpmnRules.canConnectMessageFlow(source, target); - } - - if ('association' in rules) { - results.association = bpmnRules.canConnectAssociation(source, target); - } - }); - - expect(results).to.eql(rules); -} - - -function expectCanDrop(element, target, expectedResult) { - - var result; - - TestHelper.getBpmnJS().invoke(function(elementRegistry, bpmnRules) { - - element = elementRegistry.get(element); - target = elementRegistry.get(target); - - expect(element).to.exist; - expect(target).to.exist; - - result = bpmnRules.canDrop(element, target); - }); - - expect(result).to.eql(expectedResult); -} - - -function expectCanExecute(elements, target, rules) { - - var results = {}; - - TestHelper.getBpmnJS().invoke(function(elementRegistry, bpmnRules) { - - target = elementRegistry.get(target); - - if ('canAttach' in rules) { - results.canAttach = bpmnRules.canAttach(elements, target); - } - - if ('canMove' in rules) { - results.canMove = bpmnRules.canMove(elements, target); - } - }); - - expect(results).to.eql(rules); -} - - describe('features/modeling/rules - BpmnRules', function() { var testModules = [ coreModule, modelingModule ]; @@ -149,56 +84,6 @@ describe('features/modeling/rules - BpmnRules', function() { })); - it('connect BoundaryEvent -> Task', inject(function() { - - expectCanConnect('BoundaryEvent', 'Task', { - sequenceFlow: true, - messageFlow: false, - association: true - }); - })); - - - it('connect BoundaryEvent_1 -> SubProcess', inject(function() { - - expectCanConnect('BoundaryEvent', 'SubProcess', { - sequenceFlow: true, - messageFlow: false, - association: true - }); - })); - - - it('connect BoundaryEvent -> BoundaryEvent_1', inject(function() { - - expectCanConnect('BoundaryEvent', 'BoundaryEvent_1', { - sequenceFlow: false, - messageFlow: false, - association: true - }); - })); - - - it('connect BoundaryEvent -> StartEvent_None', inject(function() { - - expectCanConnect('BoundaryEvent', 'BoundaryEvent_1', { - sequenceFlow: false, - messageFlow: false, - association: true - }); - })); - - - it('connect StartEvent_None -> BoundaryEvent', inject(function() { - - expectCanConnect('StartEvent_None', 'BoundaryEvent', { - sequenceFlow: false, - messageFlow: false, - association: true - }); - })); - - it('drop TextAnnotation -> Process', inject(function() { expectCanDrop('TextAnnotation', 'Process', true); @@ -207,6 +92,125 @@ describe('features/modeling/rules - BpmnRules', function() { }); + describe('boundary events', function() { + + var testXML = require('./BpmnRules.boundaryEvent.bpmn'); + + beforeEach(bootstrapModeler(testXML, { modules: testModules })); + + + it('connect BoundaryEvent_on_SubProcess -> Task', inject(function() { + + expectCanConnect('BoundaryEvent_on_SubProcess', 'Task', { + sequenceFlow: true, + messageFlow: false, + association: true + }); + })); + + + it('connect BoundaryEvent_on_SubProcess -> ExclusiveGateway', inject(function() { + + expectCanConnect('BoundaryEvent_on_SubProcess', 'ExclusiveGateway', { + sequenceFlow: true, + messageFlow: false, + association: true + }); + })); + + + it('connect BoundaryEvent_on_SubProcess -> SubProcess', inject(function() { + + expectCanConnect('BoundaryEvent_on_SubProcess', 'SubProcess', { + sequenceFlow: true, + messageFlow: false, + association: true + }); + })); + + + it('connect BoundaryEvent_on_SubProcess -> BoundaryEvent_on_Task', inject(function() { + + expectCanConnect('BoundaryEvent_on_SubProcess', 'BoundaryEvent_on_Task', { + sequenceFlow: false, + messageFlow: false, + association: true + }); + })); + + + it('connect BoundaryEvent_on_SubProcess -> StartEvent_None', inject(function() { + + expectCanConnect('BoundaryEvent_on_SubProcess', 'StartEvent_None', { + sequenceFlow: false, + messageFlow: false, + association: true + }); + })); + + + it('connect StartEvent_None -> BoundaryEvent_on_SubProcess', inject(function() { + + expectCanConnect('StartEvent_None', 'BoundaryEvent_on_SubProcess', { + sequenceFlow: false, + messageFlow: false, + association: true + }); + })); + + + it('connect BoundaryEvent_nested -> Task', inject(function() { + + expectCanConnect('BoundaryEvent_nested', 'Task', { + sequenceFlow: false, + messageFlow: false, + association: true + }); + })); + + + it('connect BoundaryEvent_nested -> EndEvent_nested', inject(function() { + + expectCanConnect('BoundaryEvent_nested', 'EndEvent_nested', { + sequenceFlow: true, + messageFlow: false, + association: true + }); + })); + + + it('connect BoundaryEvent_on_SubProcess -> BoundaryEvent_in_OtherProcess', inject(function() { + + expectCanConnect('BoundaryEvent_on_SubProcess', 'BoundaryEvent_in_OtherProcess', { + sequenceFlow: false, + messageFlow: false, + association: true + }); + })); + + + it('connect BoundaryEvent_on_SubProcess -> Task_in_OtherProcess', inject(function() { + + expectCanConnect('BoundaryEvent_on_SubProcess', 'Task_in_OtherProcess', { + sequenceFlow: false, + messageFlow: false, + association: true + }); + })); + + + it('connect Task_in_OtherProcess -> BoundaryEvent_on_SubProcess', inject(function() { + + expectCanConnect('Task_in_OtherProcess', 'BoundaryEvent_on_SubProcess', { + sequenceFlow: false, + messageFlow: true, + association: true + }); + })); + + }); + + describe('event based gateway', function() { var testXML = require('./BpmnRules.eventBasedGateway.bpmn'); @@ -516,16 +520,6 @@ describe('features/modeling/rules - BpmnRules', function() { })); - it('connect BoundaryEvent -> Task_in_OtherParticipant', inject(function() { - - expectCanConnect('BoundaryEvent', 'Task_in_OtherParticipant', { - sequenceFlow: false, - messageFlow: true, - association: true - }); - })); - - it('drop TextAnnotation_Global -> Participant', inject(function() { expectCanDrop('TextAnnotation_Global', 'Participant', true); @@ -564,9 +558,9 @@ describe('features/modeling/rules - BpmnRules', function() { var elements = [ boundaryEvent ]; // then - expectCanExecute(elements, 'Process_1', { - canAttach: false, - canMove: false + expectCanMove(elements, 'Process_1', { + attach: false, + move: false }); })); @@ -580,9 +574,9 @@ describe('features/modeling/rules - BpmnRules', function() { var elements = [ boundaryEvent ]; // then - expectCanExecute(elements, 'Task_2', { - canAttach: 'attach', - canMove: false + expectCanMove(elements, 'Task_2', { + attach: 'attach', + move: false }); })); @@ -597,9 +591,9 @@ describe('features/modeling/rules - BpmnRules', function() { var elements = [ label ]; // then - expectCanExecute(elements, 'SubProcess_1', { - canAttach: false, - canMove: true + expectCanMove(elements, 'SubProcess_1', { + attach: false, + move: true }); })); @@ -615,9 +609,9 @@ describe('features/modeling/rules - BpmnRules', function() { var elements = [ boundaryEvent, boundaryEvent2 ]; // then - expectCanExecute(elements, 'SubProcess_1', { - canAttach: false, - canMove: false + expectCanMove(elements, 'SubProcess_1', { + attach: false, + move: false }); })); @@ -633,9 +627,9 @@ describe('features/modeling/rules - BpmnRules', function() { var elements = [ subProcess, boundaryEvent, label ]; // then - expectCanExecute(elements, 'Process_1', { - canAttach: false, - canMove: false + expectCanMove(elements, 'Process_1', { + attach: false, + move: false }); })); @@ -658,9 +652,9 @@ describe('features/modeling/rules - BpmnRules', function() { }); // then - expectCanExecute([ eventShape ], 'Task_1', { - canAttach: 'attach', - canMove: false + expectCanMove([ eventShape ], 'Task_1', { + attach: 'attach', + move: false }); })); diff --git a/test/spec/features/modeling/rules/Helper.js b/test/spec/features/modeling/rules/Helper.js new file mode 100644 index 00000000..8ed0790b --- /dev/null +++ b/test/spec/features/modeling/rules/Helper.js @@ -0,0 +1,78 @@ +'use strict'; + +var TestHelper = require('../../../../TestHelper'); + + +function expectCanConnect(source, target, rules) { + + var results = {}; + + TestHelper.getBpmnJS().invoke(function(elementRegistry, bpmnRules) { + + source = elementRegistry.get(source); + target = elementRegistry.get(target); + + expect(source).to.exist; + expect(target).to.exist; + + if ('sequenceFlow' in rules) { + results.sequenceFlow = bpmnRules.canConnectSequenceFlow(source, target); + } + + if ('messageFlow' in rules) { + results.messageFlow = bpmnRules.canConnectMessageFlow(source, target); + } + + if ('association' in rules) { + results.association = bpmnRules.canConnectAssociation(source, target); + } + }); + + expect(results).to.eql(rules); +} + +module.exports.expectCanConnect = expectCanConnect; + + +function expectCanDrop(element, target, expectedResult) { + + var result; + + TestHelper.getBpmnJS().invoke(function(elementRegistry, bpmnRules) { + + element = elementRegistry.get(element); + target = elementRegistry.get(target); + + expect(element).to.exist; + expect(target).to.exist; + + result = bpmnRules.canDrop(element, target); + }); + + expect(result).to.eql(expectedResult); +} + +module.exports.expectCanDrop = expectCanDrop; + + +function expectCanMove(elements, target, rules) { + + var results = {}; + + TestHelper.getBpmnJS().invoke(function(elementRegistry, bpmnRules) { + + target = elementRegistry.get(target); + + if ('attach' in rules) { + results.attach = bpmnRules.canAttach(elements, target); + } + + if ('move' in rules) { + results.move = bpmnRules.canMove(elements, target); + } + }); + + expect(results).to.eql(rules); +} + +module.exports.expectCanMove = expectCanMove; \ No newline at end of file