From 43a3b07ab255325cbc931be54c170a20999daa6c Mon Sep 17 00:00:00 2001 From: Philipp Fromme Date: Mon, 10 Apr 2017 14:21:14 +0200 Subject: [PATCH] fix(bpmn-rules): only connect flow nodes and artifacts through association Closes camunda/camunda-modeler#554 --- lib/features/modeling/Modeling.js | 6 +- lib/features/rules/BpmnRules.js | 35 ++++-- test/integration/CustomElementsSpec.js | 7 +- test/spec/features/rules/BpmnRulesSpec.js | 130 +++++++++++----------- 4 files changed, 98 insertions(+), 80 deletions(-) diff --git a/lib/features/modeling/Modeling.js b/lib/features/modeling/Modeling.js index 69d031b1..56cca88a 100644 --- a/lib/features/modeling/Modeling.js +++ b/lib/features/modeling/Modeling.js @@ -64,7 +64,11 @@ Modeling.prototype.connect = function(source, target, attrs, hints) { var bpmnRules = this._bpmnRules; if (!attrs) { - attrs = bpmnRules.canConnect(source, target) || { type: 'bpmn:Association' }; + attrs = bpmnRules.canConnect(source, target); + } + + if (!attrs) { + return; } return this.createConnection(source, target, attrs, source.parent, hints); diff --git a/lib/features/rules/BpmnRules.js b/lib/features/rules/BpmnRules.js index aeb3b494..20bd012f 100644 --- a/lib/features/rules/BpmnRules.js +++ b/lib/features/rules/BpmnRules.js @@ -190,6 +190,22 @@ function isTextAnnotation(element) { return is(element, 'bpmn:TextAnnotation'); } +function isArtifact(element) { + return isAny(element, [ + 'bpmn:DataObjectReference', + 'bpmn:DataStoreReference', + 'bpmn:Group', + 'bpmn:TextAnnotation' + ]); +} + +function isDataObject(element) { + return isAny(element, [ + 'bpmn:DataObjectReference', + 'bpmn:DataStoreReference' + ]); +} + function isCompensationBoundary(element) { return is(element, 'bpmn:BoundaryEvent') && hasEventDefinition(element, 'bpmn:CompensateEventDefinition'); @@ -369,14 +385,7 @@ function canConnect(source, target, connection) { }; } - if (is(connection, 'bpmn:Association') && canConnectAssociation(source, target)) { - - return { - type: 'bpmn:Association' - }; - } - - if (isTextAnnotation(source) || isTextAnnotation(target)) { + if (canConnectAssociation(source, target)) { return { type: 'bpmn:Association' @@ -712,8 +721,16 @@ function canConnectAssociation(source, target) { return false; } + // compensation boundary events are exception + if (isCompensationBoundary(source) && isForCompensation(target)) { + return true; + } + + // only connect artifacts with other BPMN (graphical) elements // connect if different parent - return !isParent(target, source) && + return ((isArtifact(source) && !isDataObject(source) && !isArtifact(target)) || + (!isArtifact(source) && isArtifact(target) && !isDataObject(target))) && + !isParent(target, source) && !isParent(source, target); } diff --git a/test/integration/CustomElementsSpec.js b/test/integration/CustomElementsSpec.js index fcbf3921..1a93d707 100644 --- a/test/integration/CustomElementsSpec.js +++ b/test/integration/CustomElementsSpec.js @@ -154,7 +154,7 @@ describe('custom elements', function() { ); - it('should connect a bpmn element to a custom one', + it('should not connect a bpmn element to a custom one', inject(function(elementFactory, dragging, elementRegistry, connect) { // given @@ -170,11 +170,8 @@ describe('custom elements', function() { dragging.end(); - var connection = triangle.incoming[0]; - // then - expect(connection.type).to.equal('bpmn:Association'); - expect(connection.source).to.equal(subProcess); + expect(triangle.incoming).to.have.lengthOf(0); }) ); diff --git a/test/spec/features/rules/BpmnRulesSpec.js b/test/spec/features/rules/BpmnRulesSpec.js index d166404a..4efdb1c7 100644 --- a/test/spec/features/rules/BpmnRulesSpec.js +++ b/test/spec/features/rules/BpmnRulesSpec.js @@ -29,7 +29,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('StartEvent_None', 'Task', { sequenceFlow: true, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -51,7 +51,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('Task', 'IntermediateThrowEvent_Link', { sequenceFlow: true, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -62,7 +62,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('IntermediateThrowEvent_Link', 'EndEvent_None', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -73,7 +73,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('StartEvent_None', 'IntermediateCatchEvent_Link', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -84,7 +84,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('IntermediateCatchEvent_Link', 'Task', { sequenceFlow: true, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -113,7 +113,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('DataObjectReference', 'StartEvent_None', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -124,7 +124,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('StartEvent_None', 'DataObjectReference', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: { type: 'bpmn:DataOutputAssociation' } }); })); @@ -135,7 +135,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('DataObjectReference', 'EndEvent_None', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: { type: 'bpmn:DataInputAssociation' } }); })); @@ -146,7 +146,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('EndEvent_None', 'DataObjectReference', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -157,7 +157,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('Task', 'DataObjectReference', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: { type: 'bpmn:DataOutputAssociation' } }); })); @@ -168,7 +168,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('DataObjectReference', 'Task', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: { type: 'bpmn:DataInputAssociation' } }); })); @@ -179,7 +179,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('SubProcess', 'DataObjectReference', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: { type: 'bpmn:DataOutputAssociation' } }); })); @@ -190,7 +190,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('DataObjectReference', 'SubProcess', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: { type: 'bpmn:DataInputAssociation' } }); })); @@ -201,7 +201,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('DataStoreReference', 'StartEvent_None', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -212,7 +212,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('StartEvent_None', 'DataStoreReference', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: { type: 'bpmn:DataOutputAssociation' } }); })); @@ -223,7 +223,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('DataStoreReference', 'EndEvent_None', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: { type: 'bpmn:DataInputAssociation' } }); })); @@ -234,7 +234,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('EndEvent_None', 'DataStoreReference', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -245,7 +245,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('Task', 'DataStoreReference', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: { type: 'bpmn:DataOutputAssociation' } }); })); @@ -256,7 +256,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('DataStoreReference', 'Task', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: { type: 'bpmn:DataInputAssociation' } }); })); @@ -267,7 +267,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('SubProcess', 'DataStoreReference', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: { type: 'bpmn:DataOutputAssociation' } }); })); @@ -278,7 +278,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('DataStoreReference', 'SubProcess', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: { type: 'bpmn:DataInputAssociation' } }); })); @@ -298,7 +298,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('BoundaryEvent_on_SubProcess', 'Task', { sequenceFlow: true, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -309,7 +309,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('BoundaryEvent_on_SubProcess', 'ExclusiveGateway', { sequenceFlow: true, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -320,7 +320,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('BoundaryEvent_on_SubProcess', 'SubProcess', { sequenceFlow: true, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -331,7 +331,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('BoundaryEvent_on_SubProcess', 'BoundaryEvent_on_Task', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -342,7 +342,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('BoundaryEvent_on_SubProcess', 'StartEvent_None', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -353,7 +353,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('StartEvent_None', 'BoundaryEvent_on_SubProcess', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -364,7 +364,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('BoundaryEvent_nested', 'Task', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -375,7 +375,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('BoundaryEvent_nested', 'EndEvent_nested', { sequenceFlow: true, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -386,7 +386,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('BoundaryEvent_on_SubProcess', 'BoundaryEvent_in_OtherProcess', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -397,7 +397,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('BoundaryEvent_on_SubProcess', 'Task_in_OtherProcess', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -408,7 +408,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('Task_in_OtherProcess', 'BoundaryEvent_on_SubProcess', { sequenceFlow: false, messageFlow: true, - association: true, + association: false, dataAssociation: false }); })); @@ -428,7 +428,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('EventBasedGateway', 'IntermediateCatchEvent_Message', { sequenceFlow: true, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -439,7 +439,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('EventBasedGateway', 'IntermediateCatchEvent_Message', { sequenceFlow: true, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -450,7 +450,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('EventBasedGateway', 'IntermediateCatchEvent_Signal', { sequenceFlow: true, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -461,7 +461,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('EventBasedGateway', 'IntermediateCatchEvent_Condition', { sequenceFlow: true, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -472,7 +472,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('EventBasedGateway', 'IntermediateCatchEvent_Timer', { sequenceFlow: true, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -483,7 +483,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('EventBasedGateway', 'IntermediateCatchEvent', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -494,7 +494,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('EventBasedGateway', 'IntermediateThrowEvent_Message', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -505,7 +505,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('EventBasedGateway', 'ReceiveTask', { sequenceFlow: true, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -516,7 +516,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('EventBasedGateway', 'Task_None', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -527,7 +527,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('EventBasedGateway', 'ParallelGateway', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -538,7 +538,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('EventBasedGateway', 'ParallelGateway', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -558,7 +558,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('CompensationBoundary', 'Task', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -581,7 +581,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('CompensationBoundary', 'Gateway', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: false }); @@ -593,7 +593,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('CompensationBoundary', 'IntermediateEvent', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: false }); @@ -605,7 +605,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('Task', 'TaskForCompensation', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: false }); @@ -617,7 +617,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('TaskForCompensation', 'Task', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: false }); @@ -638,7 +638,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('StartEvent_None', 'IntermediateThrowEvent_Message', { sequenceFlow: true, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -649,7 +649,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('StartEvent_None', 'OtherParticipant', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -660,7 +660,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('OtherParticipant', 'StartEvent_None', { sequenceFlow: false, messageFlow: true, - association: true, + association: false, dataAssociation: false }); @@ -672,7 +672,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('OtherParticipant', 'StartEvent_Timer', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: false }); @@ -684,7 +684,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('OtherParticipant', 'StartEvent_Message', { sequenceFlow: false, messageFlow: true, - association: true, + association: false, dataAssociation: false }); @@ -696,7 +696,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('EndEvent_None', 'OtherParticipant', { sequenceFlow: false, messageFlow: true, - association: true, + association: false, dataAssociation: false }); })); @@ -707,7 +707,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('EndEvent_Cancel', 'OtherParticipant', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -718,7 +718,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('EndEvent_Message', 'OtherParticipant', { sequenceFlow: false, messageFlow: true, - association: true, + association: false, dataAssociation: false }); })); @@ -729,7 +729,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('OtherParticipant', 'EndEvent_None', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -740,7 +740,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('IntermediateThrowEvent_Message', 'OtherParticipant', { sequenceFlow: false, messageFlow: true, - association: true, + association: false, dataAssociation: false }); })); @@ -751,7 +751,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('IntermediateThrowEvent_None', 'OtherParticipant', { sequenceFlow: false, messageFlow: true, - association: true, + association: false, dataAssociation: false }); })); @@ -762,7 +762,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('IntermediateThrowEvent_Signal', 'OtherParticipant', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -773,7 +773,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('OtherParticipant', 'IntermediateThrowEvent_Message', { sequenceFlow: false, messageFlow: false, - association: true, + association: false, dataAssociation: false }); })); @@ -784,7 +784,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('Task_in_SubProcess', 'OtherParticipant', { sequenceFlow: false, messageFlow: true, - association: true, + association: false, dataAssociation: false }); })); @@ -795,7 +795,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('EndEvent_None_in_SubProcess', 'OtherParticipant', { sequenceFlow: false, messageFlow: true, - association: true, + association: false, dataAssociation: false }); })); @@ -806,7 +806,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('OtherParticipant', 'Task_in_SubProcess', { sequenceFlow: false, messageFlow: true, - association: true, + association: false, dataAssociation: false }); })); @@ -817,7 +817,7 @@ describe('features/modeling/rules - BpmnRules', function() { expectCanConnect('Participant', 'OtherParticipant', { sequenceFlow: false, messageFlow: true, - association: true, + association: false, dataAssociation: false }); }));