Multiple and parallel multiple events

[ElCondor1969]

This PR resolve the issue #1090.

[ElCondor1969]

Then I'll add the test cases.
If anyone can direct me on how to write them, they are welcome.

This diagram diagram1.zip can be used.

[pinussilvestrus]

First thanks for your contribution! From a first check of the look and feel the changes seems to follow the intended behavior. Some hints for adjusting your PR:

Test Cases

You can have a look on how we currently test the BpmnRenderer. You could, for example, adjusting the test bpmn file with cases which fits the scenario.

Even better: Test the resulting rendered shape. But it seems like we're not doing it that way in other cases atm.

Commit guidelines

Have a look at our contribution guidelines. There you will also find our commit message guidelines. It would be very nice if you could adjust your commit message, just like this:

feat(bpmn-renderer): correctly render all parallel and multiple events

Closes #1090 

[ElCondor1969]

Hi, Pinus.
Thanks for your suggestions.
In fact, I also thought that the test should check the shape within the event according to the different configurations, but I didn't understand how to do it. But the problem doesn't arise because you confirm that even in other test cases you never check the shape inside the event.
Then I analyzed the file you indicated to me ("events.bpmn") and it seems to me that it is already perfect because, inside, events with more event definitions are defined and I checked that the modeler draws them correctly.
So this file already includes the necessary test for this PR.
Tell me if it's okay for you.
About the commit message, I'll update it with your model by next commits or, anyway, as soon as you tell me that the PR doesn't need any further changes.

[pinussilvestrus]

It would be nice if you can add more events in the events.bpmn so we verify different cases. Just like you described in #1090 (cf. MessageEventDefinition and SignalEventDefinition).

Can you do a double-check on your commits? Seems like there are two (one empty).

[ElCondor1969]

Hi, Pinus.
I updated the file "events.bpmn" adding two lines with the events with more event definitions.
At this point we should be there.
About the previous commit, I think I had messed up a bit to correct the commit message with the one you suggested.
But now should be all ok.

[pinussilvestrus]

Hi @ElCondor1969 ,

So now there are 4 commits listed... Any Chance to have one clear commit?

Additionaly @barmac mentioned we should double-check the new boundary detach functionality with this changes. I'll do it.

[ElCondor1969]

Hi @pinussilvestrus
Yes, in fact I have made four commits and the last one is, of course, what counts.
Maybe you mean I shouldn't have given each commit the same label?
What would you propose to make the situation "clear"?
Maybe a further commit that would only have the purpose to add a definitive label for the PR?
Please tell me what there is to do.

[barmac]

@ElCondor1969 What we would like to achieve is one commit that contains all of the changes that you created. This can be achieved via interactive rebase and commit squashing. You can read about this here: http://gitready.com/advanced/2009/02/10/squashing-commits-with-rebase.html

Note that you will have to git push -f after the history rewriting is done. I'd suggest to create a backup branch before you start the work in case anything goes wrong.

[ElCondor1969]

Hi @barmac.
I'll try to do what you pointed out, hoping not to mess anything up.

[ElCondor1969]

Hi @barmac
I followed your instructions and rebased
There should only be one commit now, if I did things right.
Please tell me if everything is okay now.

[pinussilvestrus]

LGTM right now 🏆