Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

784 no boundary event after event based gateway #873

Merged
merged 2 commits into from
Oct 16, 2018

Conversation

pinussilvestrus
Copy link
Contributor

Implemented a Bpmn rule that it shouldn't be possible to attach a BoundaryEvent on a ReceiveTask following a EventBasedGateway in two different ways:

  • It isn't possible to attach a BoundaryEvent on a ReceiveTask which is already connected after a EventBasedGateway (BpmnRule)
  • Deletion of attached BoundaryEvent after connecting a EventBasedGateway to a ReceiveTask (Behavior)

Closes camunda/camunda-modeler#784

@ghost ghost assigned pinussilvestrus Oct 16, 2018
@ghost ghost added the needs review Review pending label Oct 16, 2018
@pinussilvestrus
Copy link
Contributor Author

pinussilvestrus commented Oct 16, 2018

after review session with @barmac another case should be considered:

  • change type of gateway to eventbasedgateway which is still connected to receive task with boundary --> boundary should be removed. As discussed in our weekly, I'll open another issue for this case

@pinussilvestrus pinussilvestrus requested review from barmac and removed request for nikku October 16, 2018 08:45
* Behavior for deleting boundaries from receive task after connecting them with event based gateway
*/
export default function ConnectEventBasedGatewayBehavior(
eventBus, modeling, bpmnRules) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bpmnRules is actually never used, so I think we should remove it before merge.


if (is(source, 'bpmn:EventBasedGateway') &&
isReceiveTaskWithBoundary(target)) {
modeling.removeElements(extractBoundaryEvents(target));
Copy link
Member

@barmac barmac Oct 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we invoke extractBoundaryEvents twice: first to check if the element is a ReceiveTask with BoundaryEvents, and then to filter out all the BoundaryEvents and remove them. Why don't we filter the events once and then remove them if it's necessary? :)

Copy link
Contributor Author

@pinussilvestrus pinussilvestrus Oct 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adjusted and rebased it :)


var diagramXML = require('./ConnectEventBasedGatewayBehavior.bpmn');

beforeEach(bootstrapModeler(diagramXML, { modules: testModules.concat(modelingModule) }));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we add modelingModule second time here? The tests seem to work without it.

Copy link
Member

@barmac barmac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a few comments and after polishing it should be ready for merge.

@pinussilvestrus pinussilvestrus force-pushed the 784-no-boundary-event-after-event-based-gateway branch from a91b54a to e25e816 Compare October 16, 2018 10:38
…ive tasks after connecting to event based gateway
@pinussilvestrus pinussilvestrus force-pushed the 784-no-boundary-event-after-event-based-gateway branch from e25e816 to b8b84e3 Compare October 16, 2018 10:41
Copy link
Member

@barmac barmac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@merge-me merge-me bot merged commit b3e4b1d into master Oct 16, 2018
@ghost ghost removed the needs review Review pending label Oct 16, 2018
@delete-merged-branch delete-merged-branch bot deleted the 784-no-boundary-event-after-event-based-gateway branch October 16, 2018 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants