Disallow multiple incoming sequence flows to target events of event-based gateway #1006

gustavjf · 2019-04-29T15:00:29Z

Which issue does this PR address?

This updates an existing BPMN rule and adds new modeling behavior for event-based gateways.

When disallowing multiple incoming sequence flows to target events of an event-based gateway, there are three scenarios that are covered here:

Scenario 1: Disallowing a connection to an event-based target when the source is not an event-based gateway.

Scenario 2: Removing existing sequence flows of event-based targets when connecting from an event-based gateway.

Scenario 3: When a user replaces a gateway, that is already connected to event-based targets, with an event-based gateway, then the existing incoming sequence flows of the targets, which do not belong to the newly replaced event-based gateway, are removed.

A fourth scenario which is not covered here would be preventing the user from connecting an event-based gateway to a target which already has an incoming connection from that same event-based gateway. This will be looked at as a separate case.

Instead, the existing connection would be implicitly removed:

gustavjf · 2019-05-02T10:30:14Z

lib/features/modeling/behavior/EventBasedGatewayBehavior.js

+
+    if (
+      is(source, 'bpmn:EventBasedGateway') &&
+      target.incoming.length > 0


~~This handles multiple connections, despite the test checking only for single connections.~~

This is because after ddc5c02, multiple connections are disallowed. However, it may be that a diagram is imported which was created with a tool that does not disallow multiple connections, so better to cover the case with multiple connections.

Disregard, multiple-connections test case added:

barmac

We need to handle properly a case when the event based gateway target has already two incoming connections and then we connect the event based gateway.

barmac · 2019-05-02T13:15:42Z

It seems like somehow we don't remove the connection properly when we replace the gateways with event based gateways.

gustavjf · 2019-05-03T10:50:15Z

@barmac Thanks, I've added tests and new behavior for dealing with the replacement scenario you mentioned. PR description has been updated to demonstrate this.

Could you have another look? Hopefully we've got it this time 👍

barmac · 2019-05-03T13:33:24Z

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

+
+    beforeEach(bootstrapModeler(
+      diagramXML,
+      { modules: [ coreModule, modelingModule, replaceModule ] }


Suggested change

{ modules: [ coreModule, modelingModule, replaceModule ] }

diagramXML, {

modules: [

coreModule,

modelingModule,

replaceModule

]

}));

Same in other places for consistence's sake.

barmac · 2019-05-03T13:37:10Z

lib/features/rules/BpmnRules.js

+    }
+  }
+
+  return result;


923-932 could be replaced simply with connections.some(isOutgoingEventBasedGatewayConnection). It's implemented in IE11.

barmac · 2019-05-03T13:39:01Z

lib/features/modeling/behavior/EventBasedGatewayBehavior.js

+      return;
+    }
+
+    newShapeOutgoingConnections


This part should be refactored as it's really difficult to understand. For sure we don't need map here. Consider assigning step results to variables.

barmac · 2019-05-03T13:41:01Z

lib/features/modeling/behavior/EventBasedGatewayBehavior.js

+
+      existingIncomingConnections
+        .filter(function(connection) {
+          return is(connection, 'bpmn:SequenceFlow');


We check this condition multiple times in the file so why don't we have a helper for this? It will increase the readibility of the code :)

Suggested change

return is(connection, 'bpmn:SequenceFlow');

filter(isSequenceFlow)

barmac · 2019-05-03T13:53:56Z

lib/features/modeling/behavior/EventBasedGatewayBehavior.js

+
+    if (
+      is(source, 'bpmn:EventBasedGateway') &&
+      target.incoming.length > 0


Suggested change

target.incoming.length > 0

target.incoming.length

This prevents multiple incoming sequence flows to target events of an event-based gateway when the connection source is not an event-based gateway.

Handles two new scenarios: 1. A user wants to connect an event-based gateway to an event-based gateway target with existing incoming sequence flows. The existing sequence flows are removed before connecting the new one. 2. A user wants to replace a gateway, that is already connected to event-based gateway targets, with an event-based gateway. The existing incoming sequence flows of the targets, which do not belong to the newly replaced event-based gateway, are removed before the replacement operation finishes. This is because target elements in an event gateway configuration must not have any additional incoming sequence flows other than that from the event gateway.

barmac

⛵️

gustavjf added the in progress Currently worked on label Apr 29, 2019

bpmn-io-tasks bot added needs review Review pending and removed in progress Currently worked on labels Apr 29, 2019

ghost assigned gustavjf Apr 29, 2019

gustavjf added in progress Currently worked on and removed needs review Review pending labels Apr 29, 2019

gustavjf force-pushed the disallow-multiple-incoming-sequence-flows-to-target-events-of-event-based-gateway branch from 836a36a to 645f8e0 Compare April 30, 2019 09:20

ghost added needs review Review pending and removed in progress Currently worked on labels Apr 30, 2019

gustavjf force-pushed the disallow-multiple-incoming-sequence-flows-to-target-events-of-event-based-gateway branch 3 times, most recently from 2cd88c1 to eb4aa6f Compare May 2, 2019 09:48

gustavjf added in progress Currently worked on and removed needs review Review pending labels May 2, 2019

gustavjf changed the title ~~WIP Disallow multiple incoming sequence flows to target events of event based gateway~~ WIP Disallow multiple incoming sequence flows to target events of event-based gateway May 2, 2019

gustavjf commented May 2, 2019

View reviewed changes

gustavjf requested a review from barmac May 2, 2019 10:30

gustavjf changed the title ~~WIP Disallow multiple incoming sequence flows to target events of event-based gateway~~ Disallow multiple incoming sequence flows to target events of event-based gateway May 2, 2019

gustavjf added needs review Review pending and removed in progress Currently worked on labels May 2, 2019

barmac requested changes May 2, 2019

View reviewed changes

gustavjf added in progress Currently worked on and removed needs review Review pending labels May 2, 2019

gustavjf force-pushed the disallow-multiple-incoming-sequence-flows-to-target-events-of-event-based-gateway branch from eb4aa6f to 5ed36d8 Compare May 2, 2019 12:44

ghost added needs review Review pending and removed in progress Currently worked on labels May 2, 2019

gustavjf requested a review from barmac May 2, 2019 12:51

gustavjf added in progress Currently worked on and removed needs review Review pending labels May 2, 2019

gustavjf changed the title ~~Disallow multiple incoming sequence flows to target events of event-based gateway~~ WIP Disallow multiple incoming sequence flows to target events of event-based gateway May 2, 2019

ghost added needs review Review pending and removed in progress Currently worked on labels May 2, 2019

gustavjf force-pushed the disallow-multiple-incoming-sequence-flows-to-target-events-of-event-based-gateway branch 4 times, most recently from 8ee7439 to b466fbd Compare May 3, 2019 10:43

gustavjf changed the title ~~WIP Disallow multiple incoming sequence flows to target events of event-based gateway~~ Disallow multiple incoming sequence flows to target events of event-based gateway May 3, 2019

barmac reviewed May 3, 2019

View reviewed changes

fix(BpmnRules): update canConnectSequenceFlow()

2cf202a

This prevents multiple incoming sequence flows to target events of an event-based gateway when the connection source is not an event-based gateway.

gustavjf force-pushed the disallow-multiple-incoming-sequence-flows-to-target-events-of-event-based-gateway branch 2 times, most recently from acff1f6 to 169d101 Compare May 6, 2019 09:03

gustavjf requested a review from barmac May 6, 2019 09:06

gustavjf and others added 2 commits May 6, 2019 11:38

style(modeling): simplify event-based-gateway behavior

4db3cad

barmac force-pushed the disallow-multiple-incoming-sequence-flows-to-target-events-of-event-based-gateway branch from 169d101 to 4db3cad Compare May 6, 2019 09:42

ghost assigned barmac May 6, 2019

barmac approved these changes May 6, 2019

View reviewed changes

gustavjf merged commit e4fe8c2 into master May 6, 2019

gustavjf deleted the disallow-multiple-incoming-sequence-flows-to-target-events-of-event-based-gateway branch May 6, 2019 10:41

ghost removed the needs review Review pending label May 6, 2019

philippfromme mentioned this pull request Oct 13, 2022

Ensure BPMN 2.0 Compliance of Event-Based Gateways through Behavior Instead of Rule #1750

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Disallow multiple incoming sequence flows to target events of event-based gateway #1006

Disallow multiple incoming sequence flows to target events of event-based gateway #1006

gustavjf commented Apr 29, 2019 •

edited

Loading

gustavjf May 2, 2019 •

edited

Loading

barmac left a comment

barmac commented May 2, 2019

gustavjf commented May 3, 2019

barmac May 3, 2019

barmac May 3, 2019

barmac May 3, 2019

barmac May 3, 2019

barmac May 3, 2019

barmac May 3, 2019

barmac May 3, 2019 •

edited

Loading

barmac left a comment

	return is(connection, 'bpmn:SequenceFlow');
	filter(isSequenceFlow)

Disallow multiple incoming sequence flows to target events of event-based gateway #1006

Disallow multiple incoming sequence flows to target events of event-based gateway #1006

Conversation

gustavjf commented Apr 29, 2019 • edited Loading

gustavjf May 2, 2019 • edited Loading

Choose a reason for hiding this comment

barmac left a comment

Choose a reason for hiding this comment

barmac commented May 2, 2019

gustavjf commented May 3, 2019

barmac May 3, 2019

Choose a reason for hiding this comment

barmac May 3, 2019

Choose a reason for hiding this comment

barmac May 3, 2019

Choose a reason for hiding this comment

barmac May 3, 2019

Choose a reason for hiding this comment

barmac May 3, 2019

Choose a reason for hiding this comment

barmac May 3, 2019

Choose a reason for hiding this comment

barmac May 3, 2019 • edited Loading

Choose a reason for hiding this comment

barmac left a comment

Choose a reason for hiding this comment

gustavjf commented Apr 29, 2019 •

edited

Loading

gustavjf May 2, 2019 •

edited

Loading

barmac May 3, 2019 •

edited

Loading