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

Boundary Events not detached when moved out of host in a batch move #1202

Closed
barmac opened this issue Sep 25, 2019 · 4 comments
Closed

Boundary Events not detached when moved out of host in a batch move #1202

barmac opened this issue Sep 25, 2019 · 4 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@barmac
Copy link
Member

barmac commented Sep 25, 2019

Screen Recording 2019-09-25 at 11 22 08

Describe the Bug

The Boundary Events are not converted to Intermediate Events when moved to canvas in a batch operation.

Steps to Reproduce

  1. Select multiple Boundary Events
  2. Move them to canvas.

Expected Behavior

Such operation should either be disallowed or the events should be converted to Intermediate Events.

Environment

  • Browser: Chrome
  • OS: MacOS
  • Library version: 5.0.4
@barmac barmac added the bug Something isn't working label Sep 25, 2019
@oguzeroglu oguzeroglu self-assigned this Sep 26, 2019
@oguzeroglu
Copy link
Contributor

This line causes the problem, since the array length is more than 1 in this case DetachEventBehavior just does an early return instead of replacing elements.

@volkergersabeck volkergersabeck added the ready Ready to be worked on label Sep 27, 2019 — with bpmn-io-tasks
@oguzeroglu oguzeroglu added in progress Currently worked on and removed ready Ready to be worked on labels Sep 27, 2019 — with bpmn-io-tasks
@oguzeroglu
Copy link
Contributor

oguzeroglu commented Sep 27, 2019

Replacing elements.move preExecute in DetachEventBehaviour fixes the problem:

  this.preExecute('elements.move', function(context) {
    var shapes = context.shapes;

    shapes.filter(function(shape) {
      var hasHost = (shape.host && (context.shapes.indexOf(shape.host) > -1)) ? true : !!context.newHost;
      return shouldReplace(shape, hasHost);
    }).map(function(shape) {
      return shapes.indexOf(shape);
    }).forEach(function(index) {
      shapes[ index ] = replaceShape(shapes[ index ], bpmnReplace);
    });
  }, true);

This is the tricky part:

var hasHost = (shape.host && (context.shapes.indexOf(shape.host) > -1)) ? true : !!context.newHost;

For edge cases where a boundary event is attached to a task and we move them all together, context.newHost is null however the Detach behaviour in that case should not apply to the Boundary Event (since it is being carried with its host), therefore we check if a shape has a host and this host is inside the moved shapes within the context to decide if it has a host or not, for other cases checking if context.newHost exists is enough. Since this is preExecute shape.host refers to the host before the replace.

@oguzeroglu
Copy link
Contributor

Closed via #1206

@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label Oct 1, 2019
@philippfromme
Copy link
Contributor

Will be properly fixed through #1203

@philippfromme philippfromme reopened this Oct 13, 2019
philippfromme added a commit that referenced this issue Oct 13, 2019
* allow copying boundary events without host
* remove CreateBoundaryEventBehavior in favor of AttachEventBehavior

Closes #1154
Closes #1202
Closes #1204
Closes #1205
philippfromme added a commit that referenced this issue Oct 13, 2019
* allow copying boundary events without host
* remove CreateBoundaryEventBehavior in favor of AttachEventBehavior

Closes #1154
Closes #1202
Closes #1204
Closes #1205
@nikku nikku added this to the M31 milestone Oct 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

5 participants