JBPM-7916 - Stunner BPMN Process Documentation Feature #2363

tiagodolphine · 2018-12-21T16:45:24Z

Feature to generate BPMN documentation on Stunner.

new tab on the designer for documentation (only enabled when documentation is implemented for a domain, for instance on DMN, should not be enabled)
all icon for activities, service tasks, and categories were implemented
template engine on client side implemented on appformer (JBPM-7916 - Stunner - Process Documentation (Template Engine) appformer#603)
svg form of the diagram inserted and redimensioned to fit the documentation
the documentation is processed on the respective tab onclick
properties of all elements are inserted generically but some of them were filtered (backgroud, dimesion, among others) need to check if all properties being showed for elements are ok
printable version of documentation content

OBS missing the unit tests, I'm working on them now.

Related PR kiegroup/appformer#603

@romartin
@LuboTerifaj

tiagodolphine · 2018-12-21T16:46:22Z

Jenkins execute full downstream build

tiagodolphine · 2018-12-22T15:36:02Z

Jenkins execute full downstream build

tiagodolphine · 2018-12-22T15:36:17Z

Jenkins retest this

tiagodolphine · 2019-01-02T12:26:52Z

Jenkins retest this

tiagodolphine · 2019-01-02T12:27:01Z

Jenkins execute full downstream build

tiagodolphine · 2019-01-02T13:42:08Z

Jenkins execute full downstream build

tiagodolphine · 2019-01-02T20:21:27Z

Jenkins retest this

tiagodolphine · 2019-01-02T20:21:35Z

Jenkins execute full downstream build

romartin

Hey @tiagodolphine

Great job! Although it's still in progress, code and UI are looking good! 👍

For now added some review comments on the code and also found a bug - cannot scroll on the documentation generated for the process inside the mortgages_process project, I think it's just missing the overflow style attribute somewhere... see:

Thanks!

...org/kie/workbench/common/stunner/core/documentation/service/DiagramDocumentationService.java

tiagodolphine · 2019-01-08T15:56:36Z

Jenkins execute full downstream build

tiagodolphine · 2019-01-08T20:01:59Z

Just pushed some more changes, service tasks, and scroll for the documentation panel.

tiagodolphine · 2019-01-09T15:04:25Z

Jenkins execute full downstream build

romartin · 2019-01-09T23:13:21Z

thanks for the fixes @tiagodolphine , soo... what's still missing in this PR for considering it candidate for merging?

Adding fixes Adding fixes

… documentation, fix canvas clear selection on the SVG

LuboTerifaj · 2019-01-14T12:22:22Z

Hello @tiagodolphine

Is this PR ready for review? I am asking because, you have mentioned here, that it is WIP. There is also an issue with full downstream build.

Thanks.

tiagodolphine · 2019-01-14T12:28:01Z

Hi @LuboTerifaj it is a WIP, I plan to push some more changes today, I think it is better for you to wait for a deeper review, I mean the idea of this early PR is to allow you to give tome try and some suggestions.
Anyway, I'll let you know when I push these new changes.
Thanks.

romartin · 2019-01-14T20:50:34Z

Yep agree with @LuboTerifaj , please let us know once you should start the review again @tiagodolphine , thanks guys!!

…ate, move mustache to appformer

tiagodolphine · 2019-01-16T19:25:24Z

Jenkins execute full downstream build

tiagodolphine · 2019-01-16T19:36:06Z

Hi @romartin @LuboTerifaj I updated the PR with the last changes, including the template engine was moved to appformer. This is ok to be reviewed.

OBS missing the unit tests, I'm working on them.

romartin

Just added a few more review comments @tiagodolphine , anyway looking good!

romartin · 2019-01-17T00:36:38Z

.../workbench/common/stunner/core/client/session/command/impl/AbstractExportSessionCommand.java

@@ -39,11 +38,6 @@ public boolean accepts(final ClientSession session) {

    @Override
    public <T> void execute(final Callback<T> callback) {
-        //prevents to render selection on canvas
-        if (getSession() instanceof EditorSession) {


There is no need clear selection anymore?

Yes, every time we are generating SVG or any other images, before that we are clearing the selection on canvas, to avoid showing up the Toolbox, but now I centralized it on the CanvasExport, it is better to keep only on one place IMO, see https://github.com/kiegroup/kie-wb-common/pull/2363/files#diff-0121e8ab393e44ea36b71825a5f6b861

romartin · 2019-01-17T00:36:42Z

...kie/workbench/common/stunner/core/client/session/command/impl/SaveDiagramSessionCommand.java

@@ -82,11 +82,6 @@ protected void onSaveDiagram(@Observes SaveDiagramSessionCommandExecutedEvent ev

        final Metadata diagramMetadata = getCanvasHandler().getDiagram().getMetadata();
        if (Objects.equals(diagramMetadata.getCanvasRootUUID(), event.getDiagramUUID())) {
-
-            //prevents to render selection on canvas
-            getSession().getSelectionControl().clearSelection();


There is no need clear selection anymore?

Yes, every time we are generating SVG or any other images, before that we are clearing the selection on canvas, to avoid showing up the Toolbox, but now I centralized it on the CanvasExport, it is better to keep only on one place IMO, see https://github.com/kiegroup/kie-wb-common/pull/2363/files#diff-0121e8ab393e44ea36b71825a5f6b861

romartin · 2019-01-17T00:38:44Z

...kbench/common/stunner/bpmn/client/documentation/template/process-documentation-template.html

+                        {{&icon}}
+                    </th>
+                    <th width="45%">
+                        <b>Name:</b> {{name}}


don't we support i18n on the templates?

good question, should we handle internationalization within the documentation? Is this working on the old designer? I mean this is possible to be made but in this way, we should test a lot of scenarios to guarantee it is all internationalized, IMO we can handle on a separate JIRA, this will allow this PR to be merged faster, WDYT?

well not sure, but I understand that we need i18n. Anyway yeah, we can handle it in a separate JIRA np 👍

romartin · 2019-01-17T00:38:56Z

...kbench/common/stunner/bpmn/client/documentation/template/process-documentation-template.html

+                        <b>Name:</b> {{name}}
+                    </th>
+                    <th width="50%">
+                        <b>Type:</b> {{title}}


romartin · 2019-01-17T00:39:14Z

...kbench/common/stunner/bpmn/client/documentation/template/process-documentation-template.html

+            <thead>
+            <tr>
+                <th>#</th>
+                <th>Name</th>


romartin · 2019-01-17T00:39:46Z

...kbench/common/stunner/bpmn/client/documentation/template/process-documentation-template.html

+                <td>{{pkg}}</td>
+            </tr>
+            <tr>
+                <td><b>Name</b></td>


romartin · 2019-01-17T00:39:49Z

...kbench/common/stunner/bpmn/client/documentation/template/process-documentation-template.html

+                <td>{{name}}</td>
+            </tr>
+            <tr>
+                <td><b>Is executable</b></td>


romartin · 2019-01-17T00:39:53Z

...kbench/common/stunner/bpmn/client/documentation/template/process-documentation-template.html

+                <td>{{isExecutable}}</td>
+            </tr>
+            <tr>
+                <td><b>Is AdHoc</b></td>


romartin · 2019-01-17T00:40:01Z

...kbench/common/stunner/bpmn/client/documentation/template/process-documentation-template.html

+        <p id="processdatatotals">
+        <ul class="list-group">
+            <li class="list-group-item">
+                Variables


romartin · 2019-01-17T00:40:12Z

...kbench/common/stunner/bpmn/client/documentation/template/process-documentation-template.html

+                <thead>
+                <tr>
+                    <th colspan="2">Property Name</th>
+                    <th>Property Value</th>


romartin

Hey @tiagodolphine now tested the generated WAR, just found a couple of issues:

When changing between tabs sometimes the is some "refresh" on the page that is appreciable. It always happens I think once selecting a node on the canvas, and then clicking on the Documentation tab. Adding video that shows this.

documentation-refresh-issue.zip

I think it does not make sense, once in the Documentation tab, showing the dock screens (forms, preview, etc), but not sure if it's easy to fix at this point. Anyway, considering we're not going to remove that docks now, two more issues:

As the forms properties panel can be opened at same time, it's possible to update some task name, for example, but the documentation is not being updated until switching tabs again
It's then missing to show the horizontal overflow, in case both west and east panels are open. See screenshot:

Thanks!

…e, avoid page flickering when selecting doc tab

tiagodolphine · 2019-01-18T13:34:27Z

hi @romartin about your comments, I applied the fixed below:

fixed, this was caused buy the css loading
- now when select documentation tab, the docks will be minimized
- on the changes on the forms this will reprocess and update the documentation
- horizontal overflow working with a min-width set

Thanks.

tiagodolphine · 2019-01-18T13:34:48Z

Jenkins execute full downstream build

romartin

Great job @tiagodolphine , it works good and bugs are fixed 👍 Thanks!

romartin · 2019-01-19T01:37:16Z

Also reported the bug about missing i18n: JBPM-8145

romartin · 2019-01-19T01:41:51Z

Hey @LuboTerifaj @hasys

It's missing here some QE reivews yet, but @tiagodolphine will be on holidays during next couple of weeks. As far as I know for completing this PR it's missing some unit tests, but it's full feature completed, and due to this feature is not related to any other existing one, probably makes sense, once you give a try to it and ensure it works (I also did), to merge it and let @tiagodolphine complete the unit tests or whatever issue we find when he's back.

Anyway, please give this a try and let's decide on Monday after your feedback. Thanks!!

Inserting a _template engine_ to run on Client that is based on [Mustache](https://mustache.github.io/) using the implementation for `Javascript` [MustacheJS](https://github.com/janl/mustache.js). The idea is to process text templates on client side now, although it will allow us to keep the same API having an implementation on backend if necessary since there is a Java implementation (https://github.com/spullara/mustache.java). The template engine is going to be used on Stunner to generate the **Process Documentation**. Related PR kiegroup/kie-wb-common#2363

romartin · 2019-01-21T00:04:12Z

Well @ederign just merged... hehe no problem, it was approved by me also.
Sorry @hasys @LuboTerifaj , please feel free to give this feature a try on master when possible, and if you find any bugs or whatever missing, just create new tickets as usually.
Also please @tiagodolphine remember to complete the missing unit tests related to this PR once you're back 👍
Thanks guys!

LuboTerifaj

Hello @tiagodolphine

I have found several issues. Please let me know, which make sense so I will report them:

There is missing Process Instance Description in process Overview.
All properties in Documentation don't contain new line characters when the real properties do.
Properties panel is closing with every switch between panels. As it can be seen in comments above, this was implented because of issue:
"As the forms properties panel can be opened at same time, it's possible to update some task name, for example, but the documentation is not being updated until switching tabs again".
From my point of view, it would be better to not hide the panel but change view from any selected node to process properties.
All nodes in a process Documentation have two "Name" properties.
There are missing data assignments in all nodes (the nodes, which are supposed to have them).
There is missing ConditionExpression property name (property value is there) in Conditional events.
Start events have different properties then intermediate and end events -> start events has additional properties like:
Min, Max, Mean, Standard Deviation, Time Unit, Distribution Type
All tasks have additional properties:
Min, Max, Mean, Standard Deviation, Time Unit, Distribution Type
Sub-processes contains additional properties like:
Min, Max, Mean, Standard Deviation, Time Unit, Distribution Type, Staff availability, Working Hours, Cost per time unit, Currency, Time Unit, Distribution Type

Thank you!

romartin · 2019-01-23T01:18:47Z

Hey @LuboTerifaj

Can you please create a new ticket with all the bugs above and assign to @tiagodolphine , no matter if you're not sure about some point.. let's report them all for now 👍

Thanks!

LuboTerifaj · 2019-01-23T14:48:30Z

Reported issues:
https://issues.jboss.org/browse/JBPM-8169
https://issues.jboss.org/browse/JBPM-8170

hasys · 2019-01-23T15:03:08Z

Great job @LuboTerifaj, thank you!

I putted my 2c to JBPM-8169 issue.

tiagodolphine force-pushed the JBPM-7916-master branch from 31e2ec3 to 5f855f2 Compare December 21, 2018 17:38

romartin requested review from romartin and LuboTerifaj January 7, 2019 14:47

romartin requested changes Jan 7, 2019

View reviewed changes

...org/kie/workbench/common/stunner/core/documentation/service/DiagramDocumentationService.java Outdated Show resolved Hide resolved

...org/kie/workbench/common/stunner/core/documentation/service/DiagramDocumentationService.java Outdated Show resolved Hide resolved

tiagodolphine force-pushed the JBPM-7916-master branch from 0f1756b to 74797ee Compare January 8, 2019 13:57

tiagodolphine added 5 commits January 14, 2019 09:36

JBPM-7916 - Stunner Process Documentation Feature

990cab1

Adding fixes Adding fixes

JBPM-7916 fix checkstyle and add element title on documentation

0e35861

JBPM-7916 add support to service tasks documentation, fix overflow on…

7f65ef7

… documentation, fix canvas clear selection on the SVG

JBPM-7916 fix broken tests

a87469a

JBPM-7916 i18n + refactor

50f0b40

tiagodolphine mentioned this pull request Jan 15, 2019

JBPM-7916 - Stunner - Process Documentation (Template Engine) kiegroup/appformer#603

Merged

JBPM-7916 add support to print documentation, i18n, improve doc templ…

be13e6b

…ate, move mustache to appformer

tiagodolphine force-pushed the JBPM-7916-master branch from d3c557e to be13e6b Compare January 16, 2019 19:25

tiagodolphine changed the title ~~[DO NOT MERGE] JBPM-7916 - Stunner BPMN Process Documentation Feature~~ JBPM-7916 - Stunner BPMN Process Documentation Feature Jan 16, 2019

romartin reviewed Jan 17, 2019

View reviewed changes

romartin requested changes Jan 17, 2019

View reviewed changes

JBPM-7916 horizontal overflow on doc page, refresh doc on forms chang…

e6f1a28

…e, avoid page flickering when selecting doc tab

romartin approved these changes Jan 18, 2019

View reviewed changes

ederign approved these changes Jan 19, 2019

View reviewed changes

ederign merged commit 540191c into kiegroup:master Jan 19, 2019

This was referenced Jan 21, 2019

JBPM-7916 - Stunner BPMN Process Documentation Feature. Fix to allow multiple Stunner based editors side-by-side #2412

Closed

DROOLS-3537: [DMN Designer] Unexpected error appears when diagram is created #2413

Merged

LuboTerifaj reviewed Jan 22, 2019

View reviewed changes

JBPM-7916 - Stunner BPMN Process Documentation Feature #2363

JBPM-7916 - Stunner BPMN Process Documentation Feature #2363

Conversation

tiagodolphine commented Dec 21, 2018 • edited

tiagodolphine commented Dec 21, 2018

tiagodolphine commented Dec 22, 2018

tiagodolphine commented Dec 22, 2018

tiagodolphine commented Jan 2, 2019

tiagodolphine commented Jan 2, 2019

tiagodolphine commented Jan 2, 2019

tiagodolphine commented Jan 2, 2019

tiagodolphine commented Jan 2, 2019

romartin left a comment • edited

Choose a reason for hiding this comment

tiagodolphine commented Jan 8, 2019

tiagodolphine commented Jan 8, 2019

tiagodolphine commented Jan 9, 2019

romartin commented Jan 9, 2019

LuboTerifaj commented Jan 14, 2019

tiagodolphine commented Jan 14, 2019

romartin commented Jan 14, 2019

tiagodolphine commented Jan 16, 2019

tiagodolphine commented Jan 16, 2019

romartin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

romartin left a comment • edited

Choose a reason for hiding this comment

tiagodolphine commented Jan 18, 2019

tiagodolphine commented Jan 18, 2019

romartin left a comment

Choose a reason for hiding this comment

romartin commented Jan 19, 2019

romartin commented Jan 19, 2019

romartin commented Jan 21, 2019

LuboTerifaj left a comment

Choose a reason for hiding this comment

romartin commented Jan 23, 2019

LuboTerifaj commented Jan 23, 2019

hasys commented Jan 23, 2019

tiagodolphine commented Dec 21, 2018 •

edited

romartin left a comment •

edited

romartin left a comment •

edited