JBPM-7217 Stunner - Boundary event morphing cause issues

[tiagodolphine]

Now it is possible to morph a docked Event on an activity inside a sub-process and change its locations, save and load, and undo/redo without any issues.
The problem was on the MorphCanvasNodeCommand.java that has not handling the dock/undock in case morphing a docked node.

@romartin
@LuboTerifaj
@hasys

[tiagodolphine]

Jenkins execute full downstream build

[romartin]

Hey @tiagodolphine ,

The fix looks good for me! 👍

Anyway just a comment, I was looking at the morphing stuff and it looks that we can probably move that logic about checking the parent and dock relationships, for the candidate node, into the core/shared side, isn't it?

I mean this code you updated on this commit, is only client stuff, but parent and dock relationships are in the "shared" graph structure, so this code could be also present on the "shared" morph command, right? IMO This is kinda refactors that we never have time to do but finally will blow up at some point as a bug hehe... WDYT? Makes sense? Is it possible/feasible now?

Thanks!!

[tiagodolphine]

Hey @romartin well I think the issue and all the parent and docking stuff is only a problem on the client side, because if we think on the graph nothing changes, because we are only changing the content of a node, I mean we just change the references, however the relationships are preserved. The thing is, on canvas we need in fact to unegister the old shape and register the new shape, according to the node that was morphed, and as you know we have some peculiarities, on canvas the parent of a docked node should be the node where it is docked, anyway I think the parent/docking handling makes sense only on canvas command, does it makes sense or am I missing some point?
Thanks !!!

[LuboTerifaj]

Issues are still present. If you create boundary event on task in sub-process and change type of event using morphing, the event changes its position.

[romartin]

yeah @tiagodolphine good point, it's just pure client side stuff... it does not make sense what I said, sorry!! thanks! 👍

[tiagodolphine]

hey @LuboTerifaj strange, anyway I'll check that, maybe I let something missing.

[tiagodolphine]

Jenkins execute full downstream build

[tiagodolphine]

@LuboTerifaj fixed, that was a a wrong calling to the dock method that should be done after the connection handling. I changed this before the first commit, sorry for that.

[tiagodolphine]

@LuboTerifaj fulldownstream is ready.

[LuboTerifaj]

Hi @tiagodolphine
It seems that it works well on tasks in sub-processes, but if you try morph event, which is on boundary of sub-process, "copy" of this event is displayed on different place in diagram. After that, if you move with any of these events (original or "copy of original"), both of them change location in the diagram.

[tiagodolphine]

hey @LuboTerifaj @romartin I'll take a look on this.
Thanks.

[tiagodolphine]

Jenkins execute full downstream build

[tiagodolphine]

@LuboTerifaj I pushed the fix, let's wait the fulldownstream.

Thanks.

[tiagodolphine]

Jenkins retest this

[tiagodolphine]

Jenkins execute full downstream build

[tiagodolphine]

Jenkins retest this

[LuboTerifaj]

Hey @tiagodolphine
It works better now, but there is still a small issue. When morphing boundary event, a quick menu is displayed for a very short time on another place on canvas. Can you fix this?
Thank you!

[LuboTerifaj]

Hi @tiagodolphine
I have found one more issue related to morphing. When you create boundary event and change its type using morphing, then resize an activity, position of this event is changed to another edge of activity.

[tiagodolphine]

Hi @LuboTerifaj ok, thanks for the review, I'll check the issues you found.

[tiagodolphine]

Jenkins execute full downstream build

[tiagodolphine]

@LuboTerifaj is it ready to review. Tks.

[tiagodolphine]

hey @LuboTerifaj if you have a chance to review this before the release it would be great.
Thanks

[tiagodolphine]

Hey @romartin I think @LuboTerifaj is on PTO, so if this is important to the release we should review and merge ASAP, anyone else to help on this? After merge it is necessary the cherry-pick to 7.7.x.

[romartin]

Hey @tiagodolphine
Thanks for the good job man, it works great in general - tested all comments from lubo and they work fine, but still found a couple of issues:

1. Consider the following process

When deleting the intermediate timer event docked to the task, and undoing, the event is no longer docked to the task, and so it appears on the wrong location, see:

2.- Minor thing - when deleting the intermediate event mentioned above, the contextual menu appears for a second in some canvas location.. same thing as @LuboTerifaj reported, but when deleting the event.

So I don't know if these issues found are related to this change, as you're currently working on more commits about docking, what do you prefer? we can open a ticket and fix those later, or in any of your further commits, or waiting to fix and merge.

Thx!!

[romartin]

@tiagodolphine just realized the bugs above are about docking, not morphing, so if they're not easy to fix we can open a ticket and address it later. Up to you! Thanks

[tiagodolphine]

@romartin ok I'll take a look to see if it is be feasible to fix.

Thanks.

[tiagodolphine]

hey @romartin I investigated and the issue is related to the DeleteCanvasNodeCommand that should consider whether the deleted node is docked in case of undo.
Anyway this is not related to this PR that mainly consider morphing, IMO we can open a new JIRA for this issue.
If there aren't other issues we can proceed to merge and cherry pick.

[tiagodolphine]

Jenkins execute full downstream build

[tiagodolphine]

Jenkins execute full downstream build

[tiagodolphine]

Jenkins retest this

[romartin]

hey @tiagodolphine @LuboTerifaj what's the status for this? Thx!

[LuboTerifaj]

Jenkins execute full downstream build

[LuboTerifaj]

Hi @romartin
As you can see, there is a problem with full downstream build.

[tiagodolphine]

@LuboTerifaj yeah it is faling to an unrelated issue
Could not resolve dependencies for project org.kie:kie-tomcat-integration:jar:7.9.0-SNAPSHOT
... well let's try to trigger the full downstream again

[tiagodolphine]

Jenkins execute full downstream build

[LuboTerifaj]

Jenkins execute full downstream build

[LuboTerifaj]

Hi @tiagodolphine
except last bug, that @romartin mentioned, it looks that it works great! :) I think it can be merged. Just one question, is that issue already reported or shall I report it?
Thank you, great job!

[tiagodolphine]

@LuboTerifaj I'm fixing this on the other PR #1952

[LuboTerifaj]

@tiagodolphine
Oh yes, great, thank you.