fix(modeling/DropOnFlowBehavior): filter redundant connections

This prevents duplicate flows from being created, accidentially.

Closes #774

lib/features/modeling/behavior/DropOnFlowBehavior.js

@@ -2,7 +2,8 @@ import inherits from 'inherits';
 
 import {
   assign,
-  find
+  find,
+  filter
 } from 'min-dash';
 
 import CommandInterceptor from 'diagram-js/lib/command/CommandInterceptor';
@@ -23,7 +24,15 @@ export default function DropOnFlowBehavior(eventBus, bpmnRules, modeling) {
 
   function insertShape(shape, targetFlow, position) {
     var waypoints = targetFlow.waypoints,
-        waypointsBefore, waypointsAfter, dockingPoint, source, target, reconnected;
+        waypointsBefore,
+        waypointsAfter,
+        dockingPoint,
+        source,
+        target,
+        incomingConnection,
+        outgoingConnection,
+        oldOutgoing = shape.outgoing.slice(),
+        oldIncoming = shape.incoming.slice();
 
     var intersection = getApproxIntersection(waypoints, position);
 
@@ -56,18 +65,37 @@ export default function DropOnFlowBehavior(eventBus, bpmnRules, modeling) {
       // reconnect source -> inserted shape
       modeling.reconnectEnd(targetFlow, shape, waypointsBefore || position);
 
-      reconnected = true;
+      incomingConnection = targetFlow;
     }
 
     if (bpmnRules.canConnect(shape, target, targetFlow)) {
 
-      if (!reconnected) {
+      if (!incomingConnection) {
         // reconnect inserted shape -> end
         modeling.reconnectStart(targetFlow, shape, waypointsAfter || position);
+
+        outgoingConnection = targetFlow;
       } else {
-        modeling.connect(shape, target, { type: targetFlow.type, waypoints: waypointsAfter });
+        outgoingConnection = modeling.connect(
+          shape, target, { type: targetFlow.type, waypoints: waypointsAfter }
+        );
       }
     }
+
+    var duplicateConnections = [].concat(
+
+      incomingConnection && filter(oldIncoming, function(connection) {
+        return connection.source === incomingConnection.source;
+      }) || [],
+
+      outgoingConnection && filter(oldOutgoing, function(connection) {
+        return connection.source === outgoingConnection.source;
+      }) || []
+    );
+
+    if (duplicateConnections.length) {
+      modeling.removeElements(duplicateConnections);
+    }
   }
 
   this.preExecute('elements.move', function(context) {

test/spec/features/modeling/behavior/DropOnFlowBehavior.bpmn

@@ -3,9 +3,11 @@
   <bpmn:process id="Process" isExecutable="false">
     <bpmn:startEvent id="StartEvent">
       <bpmn:outgoing>SequenceFlow_1</bpmn:outgoing>
+      <bpmn:outgoing>SequenceFlow_3</bpmn:outgoing>
     </bpmn:startEvent>
     <bpmn:task id="Task_1">
       <bpmn:incoming>SequenceFlow_1</bpmn:incoming>
+      <bpmn:incoming>SequenceFlow_4</bpmn:incoming>
       <bpmn:outgoing>SequenceFlow_2</bpmn:outgoing>
     </bpmn:task>
     <bpmn:sequenceFlow id="SequenceFlow_1" sourceRef="StartEvent" targetRef="Task_1" />
@@ -17,6 +19,12 @@
     </bpmn:task>
     <bpmn:sequenceFlow id="SequenceFlow_2" name="FOO BAR" sourceRef="Task_1" targetRef="Task_2" />
     <bpmn:task id="Task_3" />
+    <bpmn:task id="Task_4">
+      <bpmn:incoming>SequenceFlow_3</bpmn:incoming>
+      <bpmn:outgoing>SequenceFlow_4</bpmn:outgoing>
+    </bpmn:task>
+    <bpmn:sequenceFlow id="SequenceFlow_3" sourceRef="StartEvent" targetRef="Task_4" />
+    <bpmn:sequenceFlow id="SequenceFlow_4" sourceRef="Task_4" targetRef="Task_1" />
   </bpmn:process>
   <bpmndi:BPMNDiagram id="BPMNDiagram_1">
     <bpmndi:BPMNPlane id="BPMNPlane_1" bpmnElement="Process">
@@ -27,10 +35,10 @@
         <dc:Bounds x="502" y="259" width="100" height="80" />
       </bpmndi:BPMNShape>
       <bpmndi:BPMNEdge id="SequenceFlow_di" bpmnElement="SequenceFlow_1">
-        <di:waypoint xsi:type="dc:Point" x="209" y="120" />
+        <di:waypoint x="209" y="120" />
-        <di:waypoint xsi:type="dc:Point" x="340" y="120" />
+        <di:waypoint x="340" y="120" />
-        <di:waypoint xsi:type="dc:Point" x="340" y="299" />
+        <di:waypoint x="340" y="299" />
-        <di:waypoint xsi:type="dc:Point" x="502" y="299" />
+        <di:waypoint x="502" y="299" />
         <bpmndi:BPMNLabel>
           <dc:Bounds x="297.5" y="110" width="90" height="20" />
         </bpmndi:BPMNLabel>
@@ -53,19 +61,31 @@
           <dc:Bounds x="552" y="228" width="0" height="0" />
         </bpmndi:BPMNLabel>
       </bpmndi:BPMNShape>
-      <bpmndi:BPMNShape id="Task_0oupcpb_di" bpmnElement="Task_2">
+      <bpmndi:BPMNShape id="Task_2_di" bpmnElement="Task_2">
         <dc:Bounds x="797" y="259" width="100" height="80" />
       </bpmndi:BPMNShape>
-      <bpmndi:BPMNEdge id="SequenceFlow_19qadsw_di" bpmnElement="SequenceFlow_2">
+      <bpmndi:BPMNEdge id="SequenceFlow_2_di" bpmnElement="SequenceFlow_2">
-        <di:waypoint xsi:type="dc:Point" x="602" y="299" />
+        <di:waypoint x="602" y="299" />
-        <di:waypoint xsi:type="dc:Point" x="797" y="299" />
+        <di:waypoint x="797" y="299" />
         <bpmndi:BPMNLabel>
           <dc:Bounds x="675" y="274" width="50" height="12" />
         </bpmndi:BPMNLabel>
       </bpmndi:BPMNEdge>
-      <bpmndi:BPMNShape id="Task_00wmo3b_di" bpmnElement="Task_3">
+      <bpmndi:BPMNShape id="Task_3_di" bpmnElement="Task_3">
         <dc:Bounds x="141" y="371" width="100" height="80" />
       </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="Task_4_di" bpmnElement="Task_4">
+        <dc:Bounds x="407" y="80" width="100" height="80" />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNEdge id="SequenceFlow_3_di" bpmnElement="SequenceFlow_3">
+        <di:waypoint x="209" y="120" />
+        <di:waypoint x="407" y="120" />
+      </bpmndi:BPMNEdge>
+      <bpmndi:BPMNEdge id="SequenceFlow_4_di" bpmnElement="SequenceFlow_4">
+        <di:waypoint x="457" y="160" />
+        <di:waypoint x="457" y="281" />
+        <di:waypoint x="502" y="281" />
+      </bpmndi:BPMNEdge>
     </bpmndi:BPMNPlane>
   </bpmndi:BPMNDiagram>
 </bpmn:definitions>

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

@@ -69,11 +69,11 @@ describe('modeling/behavior - drop on connection', function() {
 
           // new outgoing connection
           expect(newShape.outgoing.length).to.equal(1);
-          expect(targetConnection).to.be.ok;
+          expect(targetConnection).to.exist;
           expect(targetConnection.type).to.equal('bpmn:SequenceFlow');
 
           expect(startEvent.outgoing[0]).to.equal(newShape.incoming[0]);
-          expect(task.incoming[0]).to.equal(newShape.outgoing[0]);
+          expect(task.incoming[1]).to.equal(newShape.outgoing[0]);
 
           // split target at insertion point
           expect(sequenceFlow).to.have.waypoints(flatten([
@@ -259,11 +259,11 @@ describe('modeling/behavior - drop on connection', function() {
 
           // new outgoing connection
           expect(intermediateThrowEvent.outgoing.length).to.equal(1);
-          expect(targetConnection).to.be.ok;
+          expect(targetConnection).to.exist;
           expect(targetConnection.type).to.equal('bpmn:SequenceFlow');
 
           expect(startEvent.outgoing[0]).to.equal(intermediateThrowEvent.incoming[0]);
-          expect(task.incoming[0]).to.equal(intermediateThrowEvent.outgoing[0]);
+          expect(task.incoming[1]).to.equal(intermediateThrowEvent.outgoing[0]);
 
           // split target at insertion point
           expect(sequenceFlow).to.have.waypoints(flatten([
@@ -319,11 +319,11 @@ describe('modeling/behavior - drop on connection', function() {
 
           // new outgoing connection
           expect(intermediateThrowEvent.outgoing.length).to.equal(1);
-          expect(targetConnection).to.be.ok;
+          expect(targetConnection).to.exist;
           expect(targetConnection.type).to.equal('bpmn:SequenceFlow');
 
           expect(startEvent.outgoing[0]).to.equal(intermediateThrowEvent.incoming[0]);
-          expect(task.incoming[0]).to.equal(intermediateThrowEvent.outgoing[0]);
+          expect(task.incoming[1]).to.equal(intermediateThrowEvent.outgoing[0]);
 
           // split target at insertion point
           expect(sequenceFlow).to.have.waypoints(flatten([
@@ -561,6 +561,40 @@ describe('modeling/behavior - drop on connection', function() {
         }
       ));
 
+
+      it('should remove redundant flows', inject(
+        function(elementRegistry, selection, move, dragging) {
+
+          var existingIncoming = elementRegistry.get('SequenceFlow_3'),
+              existingOutgoing = elementRegistry.get('SequenceFlow_4');
+
+          // given
+          var element = elementRegistry.get('Task_4');
+
+          var targetFlow = elementRegistry.get('SequenceFlow_1'),
+              targetFlowGfx = elementRegistry.getGraphics(targetFlow);
+
+          // when
+          selection.select(element);
+
+          move.start(canvasEvent({ x: 0, y: 0 }), element);
+
+          dragging.hover({
+            element: targetFlow,
+            gfx: targetFlowGfx
+          });
+
+          dragging.move(canvasEvent({ x: -40, y: 179 }));
+
+          dragging.end();
+
+          // then
+          // existing connections are removed, as they are duplicates
+          expect(element.incoming).not.to.contain(existingIncoming);
+          expect(element.outgoing).not.to.contain(existingOutgoing);
+        }
+      ));
+
     });
 
   });