JBPM-10058 Task form with LocalDateTime datatype displays time format… #2768

bxf12315 · 2022-04-07T06:53:11Z

… even when the option is unflagged

Thank you for submitting this pull request

JIRA: (please edit the JIRA link if it exists)

[link](JBPM-10058)

referenced Pull Requests:

JBPM-10058 Task form with LocalDateTime datatype displays time format… jbpm#2145 was rejected as not right change

How to replicate CI configuration locally?

Build Chain tool does "simple" maven build(s), the builds are just Maven commands, but because the repositories relates and depends on each other and any change in API or class method could affect several of those repositories there is a need to use build-chain tool to handle cross repository builds and be sure that we always use latest version of the code for each repository.

build-chain tool is a build tool which can be used on command line locally or in Github Actions workflow(s), in case you need to change multiple repositories and send multiple dependent pull requests related with a change you can easily reproduce the same build by executing it on Github hosted environment or locally in your development environment. See local execution details to get more information about it.

How to retest this PR or trigger a specific build:

a pull request please add comment: Jenkins retest this
a full downstream build please add comment: Jenkins run fdb
a compile downstream build please add comment: Jenkins run cdb
a full production downstream build please add comment: Jenkins execute product fdb
an upstream build please add comment: Jenkins run upstream

jstastny-cz

Just naming convention remarks.

...vices-jbpm-ui/src/main/java/org/kie/server/services/jbpm/ui/form/render/model/FormField.java

...ices-jbpm-ui/src/main/java/org/kie/server/services/jbpm/ui/form/render/model/LayoutItem.java

bxf12315 · 2022-04-07T12:39:45Z

Jenkins retest this

bxf12315 · 2022-04-07T15:12:15Z

Jenkins retest this

bxf12315 · 2022-04-08T05:50:23Z

Jenkins run fdb

yesamer · 2022-04-11T16:03:31Z

jenkins retest this please

nmirasch · 2022-04-12T08:25:39Z

@bxf12315 It works fine and changes looks good. I just miss some testing

ChughShilpa · 2022-04-12T13:57:16Z

@bxf12315
With Show Time Unselected:
When tried to submit the process form
(http://localhost:8080/kie-server/services/rest/server/containers/03145275/forms/processes/_03145275.TestDate/content)
got some server error (please see attached screenshot and logs)

logs.txt

jstastny-cz · 2022-04-13T06:09:08Z

@bxf12315 With Show Time Unselected: When tried to submit the process form (http://localhost:8080/kie-server/services/rest/server/containers/03145275/forms/processes/_03145275.TestDate/content) got some server error (please see attached screenshot and logs)

logs.txt

jstastny-cz · 2022-04-13T06:10:18Z

I am sorry, wrong button :-( .

I just wanted to confirm Shilpa's finding. I am able to reproduce the same using FDB build.

jstastny-cz · 2022-04-13T09:15:17Z

It seems we should extend testing as proposed by @nmirasch and test that the server is able to handle the localdatetime without time part.

bxf12315 · 2022-04-14T10:41:27Z

Updated, later I will add some tests for it. Thanks
@ChughShilpa @jstastny-cz

bxf12315 · 2022-04-14T14:06:26Z

@nmirasch @jstastny-cz UT added.

bxf12315 · 2022-04-14T14:29:29Z

Jenkins retest this

bxf12315 · 2022-04-14T14:29:41Z

Jenkins run fdb

jstastny-cz · 2022-04-14T14:30:18Z

...-jbpm-ui/src/main/java/org/kie/server/services/jbpm/ui/form/render/AbstractFormRenderer.java

@@ -503,6 +503,7 @@ private String getFieldType(FormField field) {
             default: 
                 switch (field.getType()) {
                     case "java.time.LocalDateTime":
+                         return "datetime-local";


This does not change the result of the switch, is that a leftover?

it's a short name and will be used in the HTML template.
https://github.com/kiegroup/droolsjbpm-integration/pull/2768/files#diff-f490d20f356970cad0c84cd0ffe2f5396127ad65bcf5a6d08c0ee3439efbb4f9R65

There's no operational difference in previous:

case "java.time.LocalDateTime": case "java.util.Date": case "java.sql.Date": case "java.sql.Timestamp": return "datetime-local";

and after this change:

case "java.time.LocalDateTime": return "datetime-local"; case "java.util.Date": case "java.sql.Date": case "java.sql.Timestamp": return "datetime-local";

So IMO this should be fixed or reverted.

previous "java.time.LocalDateTime" return default name, now is "datetime-local"

In original switch the case statement "case "java.time.LocalDateTime": was activated when input matching the literal so the first return value was actually returned. That's the way of execution in case statement without break statement is to continue in the following lines (and also cross cases) until it reaches interruption point (usually a break statement in this case it's return). In any case those two switches are identical when it comes to how it's executed.
The default branch is activated only when none of the cases matched the input value.

https://stackoverflow.com/questions/18192255/regarding-java-switch-statements-using-return-and-omitting-breaks-in-each-case

I'm with Jan here, if the case statement doesn't have a end (break/return) the execution will continue until it reaches the next return/break

@pefernan wdyt, should we rollback this change? IMO it's unnecessary isolated change of coding style - there is another approach applied for the rest of cases, so diverging might puzzle people (it did puzzle myself).

jstastny-cz · 2022-04-14T14:31:30Z

...-ui/src/test/java/org/kie/server/services/jbpm/ui/form/render/BootstrapFormRendererTest.java

+        processAssetDesc.setId("test-id");
+
+        BootstrapFormRenderer bootstrapFormRenderer = new BootstrapFormRenderer();
+        String outString = bootstrapFormRenderer.renderProcess("test-contianerId", processAssetDesc, form);


Suggested change

String outString = bootstrapFormRenderer.renderProcess("test-contianerId", processAssetDesc, form);

String outString = bootstrapFormRenderer.renderProcess("test-containerId", processAssetDesc, form);

bxf12315 · 2022-04-15T01:21:51Z

Jenkins run fdb

bxf12315 · 2022-04-15T13:18:54Z

Jenkins run fdb

bxf12315 · 2022-04-15T13:58:43Z

Jenkins run fdb

bxf12315 · 2022-04-15T13:59:11Z

jenkins retest this please

bxf12315 · 2022-05-07T14:58:52Z

Jenkins run fdb

bxf12315 · 2022-05-08T13:45:20Z

Jenkins run fdb

bxf12315 · 2022-05-08T17:02:01Z

Jenkins run fdb

bxf12315 · 2022-05-09T00:15:36Z

@ChughShilpa @jstastny-cz @nmirasch @pefernan updated, please check agian,

ChughShilpa · 2022-05-09T08:24:50Z

@bxf12315
Tested with latest fdb and https://issues.redhat.com/secure/attachment/12692075/12692075_reproducer_03145275.zip (which includes only "ProcessLocalDateTime" )

With ShowTime Unchecked:

Start the process from kie-server

date is visible in both kie-server and BC
can modify and save through kie-server and change is correctly visible in kie-server and BC
can modify and save through BC but no change is visible either in kie-server or in BC

Start the process from BC

no value is visible in kie-server. Only 'mm/dd/yy' format is visible
no value is visible in BC. Only 'datebirth' label is visible
can modify and save through kie-server and change is correctly visible in kie-server and BC
can modify and save through BC but no change is visible either in kie-server or in BC

bxf12315 · 2022-05-09T08:54:23Z

@bxf12315 Tested with latest fdb and https://issues.redhat.com/secure/attachment/12692075/12692075_reproducer_03145275.zip (which includes only "ProcessLocalDateTime" )

With ShowTime Unchecked:

Start the process from kie-server

date is visible in both kie-server and BC

can modify and save through kie-server and change is correctly visible in kie-server and BC

can modify and save through BC but no change is visible either in kie-server or in BC

Start the process from BC

BC. can not save local-datetime, so the. issue related to kie-server, I think we needs create new issue for it.

no value is visible in kie-server. Only 'mm/dd/yy' format is visible

no value is visible in BC. Only 'datebirth' label is visible

can modify and save through kie-server and change is correctly visible in kie-server and BC

can modify and save through BC but no change is visible either in kie-server or in BC

ChughShilpa · 2022-05-09T12:02:40Z

Agree with you @bxf12315 , these issues are not particularly related to this change.

pefernan · 2022-05-06T15:55:33Z

...-jbpm-ui/src/main/java/org/kie/server/services/jbpm/ui/form/render/AbstractFormRenderer.java

@@ -503,6 +503,7 @@ private String getFieldType(FormField field) {
             default: 
                 switch (field.getType()) {
                     case "java.time.LocalDateTime":
+                         return "datetime-local";


I'm with Jan here, if the case statement doesn't have a end (break/return) the execution will continue until it reaches the next return/break

pefernan · 2022-05-10T08:08:05Z

...-jbpm-ui/src/main/java/org/kie/server/services/jbpm/ui/form/render/AbstractFormRenderer.java

@@ -409,7 +409,10 @@ protected void processFormLayout(FormInstance topLevelForm,
                            if (outputs.get(field.getBinding()) != null) {
                                value = outputs.get(field.getBinding());
                            }
-
+
+                            if (value != null && value.toString().length() >= 10 && "datetime-local".equals(fieldType) && !field.isShowTime()) {


I'd do this inside of the next switch

pefernan · 2022-05-10T08:22:49Z

...-jbpm-ui/src/main/java/org/kie/server/services/jbpm/ui/form/render/AbstractFormRenderer.java

@@ -503,6 +503,7 @@ private String getFieldType(FormField field) {
             default: 
                 switch (field.getType()) {
                     case "java.time.LocalDateTime":
+                         return "datetime-local";


pefernan · 2022-05-10T08:31:43Z

...-jbpm-ui/src/main/java/org/kie/server/services/jbpm/ui/form/render/AbstractFormRenderer.java

@@ -762,6 +775,19 @@ protected String appendExtractionExpression(String type, String name, String id,
            jsonTemplate.append("getMultipleInputData('")
                        .append(id)
                        .append("')");
+        } else if (type.equals("datetime-local")) {
+            if (isShowTime) {


wouldn't it make sense here to use a mechansim like the ones in the date type where the it converts the input value into the proper JS Date object correctly formatted with the getDateFormatted function instead of adding the value as string?

getDateFormatted will return an object, we need string in here.

droolsjbpm-integration/kie-server-parent/kie-server-services/kie-server-services-jbpm/src/main/java/org/kie/server/services/jbpm/ProcessServiceBase.java

Line 112 in 88f4182

Map<String, Object> parameters = marshallerHelper.unmarshal(containerId, payload, marshallingType, Map.class);

mareknovotny · 2022-05-11T08:53:46Z

jenkins retest this please

nmirasch · 2022-05-11T10:11:26Z

@bxf12315 the fix worked fine with the reproducer attached to the jira. The only thing that failed was to store date without the show time check.

… even when the option is unflagged

jstastny-cz

LGTM. Tested fine.

nmirasch

this code solves the customer issue.

ChughShilpa

Tested and it looks fine

mareknovotny · 2022-05-12T09:22:31Z

jenkins retest this please

the build was green, the failure happened only on ant running jacoco coverage instrumentation

[2022-05-11T15:50:38.034Z] [ERROR] Failed to execute goal org.apache.maven.plugins:maven-antrun-plugin:1.8:run (generate-aggregated-jacoco-report) on project droolsjbpm-integration: An Ant BuildException has occured: Unable to read execution data file /home/jenkins/workspace/KIE/main/pullrequest/droolsjbpm-integration-main.pr/bc/kiegroup_droolsjbpm_integration/target/jacoco.exec
[2022-05-11T15:50:38.034Z] [ERROR] around Ant part ...<report>... @ 7:11 in /home/jenkins/workspace/KIE/main/pullrequest/droolsjbpm-integration-main.pr/bc/kiegroup_droolsjbpm_integration/target/antrun/build-main.xml: EOFException
[2022-05-11T15:50:38.034Z] [ERROR] -> [Help 1]
[2022-05-11T15:50:38.034Z] [ERROR] 
[2022-05-11T15:50:38.034Z] [ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[2022-05-11T15:50:38.034Z] [ERROR] Re-run Maven using the -X switch to enable full debug logging.
[2022-05-11T15:50:38.034Z] [ERROR] 
[2022-05-11T15:50:38.034Z] [ERROR] For more information about the errors and possible solutions, please read the following articles:
[2022-05-11T15:50:38.034Z] [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
[2022-05-11T15:50:38.034Z] + test 1 -eq 0
[Pipeline] }

pefernan

works! thanks @bxf12315

sonarcloud · 2022-05-12T11:58:44Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
4 Code Smells

91.1% Coverage
0.9% Duplication

… even when the option is unflagged (kiegroup#2768)

… even when the option is unflagged (#2768)

bxf12315 requested review from nmirasch and jstastny-cz April 7, 2022 06:53

jstastny-cz reviewed Apr 7, 2022

View reviewed changes

...vices-jbpm-ui/src/main/java/org/kie/server/services/jbpm/ui/form/render/model/FormField.java Outdated Show resolved Hide resolved

...ices-jbpm-ui/src/main/java/org/kie/server/services/jbpm/ui/form/render/model/LayoutItem.java Outdated Show resolved Hide resolved

bxf12315 force-pushed the JBPM-10058 branch from 3fae370 to 67b1baf Compare April 8, 2022 05:49

jstastny-cz requested a review from ChughShilpa April 12, 2022 05:14

jstastny-cz closed this Apr 13, 2022

jstastny-cz reopened this Apr 13, 2022

bxf12315 force-pushed the JBPM-10058 branch from 67b1baf to f41de71 Compare April 14, 2022 10:40

bxf12315 force-pushed the JBPM-10058 branch from f41de71 to b5fa205 Compare April 14, 2022 14:04

jstastny-cz reviewed Apr 14, 2022

View reviewed changes

bxf12315 force-pushed the JBPM-10058 branch from b5fa205 to 997b8ec Compare April 15, 2022 01:21

bxf12315 force-pushed the JBPM-10058 branch from 997b8ec to 0d7fd4c Compare April 15, 2022 06:01

bxf12315 force-pushed the JBPM-10058 branch from 0d7fd4c to 09be90a Compare April 15, 2022 13:25

bxf12315 force-pushed the JBPM-10058 branch from 258e6f4 to e1ec59e Compare May 8, 2022 13:44

bxf12315 force-pushed the JBPM-10058 branch from e1ec59e to 791f2f9 Compare May 8, 2022 17:01

mareknovotny requested a review from jstastny-cz May 10, 2022 06:43

pefernan requested changes May 10, 2022

View reviewed changes

elguardian mentioned this pull request May 11, 2022

JBPM-10058 Task form with LocalDateTime datatype displays time format… kiegroup/jbpm#2145

Closed

bxf12315 force-pushed the JBPM-10058 branch 2 times, most recently from c504c4a to baf0116 Compare May 11, 2022 13:07

JBPM-10058 Task form with LocalDateTime datatype displays time format…

5a219f9

… even when the option is unflagged

bxf12315 force-pushed the JBPM-10058 branch from baf0116 to 5a219f9 Compare May 11, 2022 13:09

jstastny-cz approved these changes May 11, 2022

View reviewed changes

nmirasch approved these changes May 12, 2022

View reviewed changes

ChughShilpa approved these changes May 12, 2022

View reviewed changes

mareknovotny requested a review from pefernan May 12, 2022 09:20

pefernan approved these changes May 12, 2022

View reviewed changes

mareknovotny merged commit a5de604 into kiegroup:main May 12, 2022

bxf12315 added a commit to Cory-jbpm/droolsjbpm-integration that referenced this pull request Jun 14, 2022

JBPM-10058 Task form with LocalDateTime datatype displays time format…

50a3140

… even when the option is unflagged (kiegroup#2768)

bxf12315 added a commit to Cory-jbpm/droolsjbpm-integration that referenced this pull request Jul 22, 2022

JBPM-10058 Task form with LocalDateTime datatype displays time format…

6eccc73

… even when the option is unflagged (kiegroup#2768)

nmirasch pushed a commit that referenced this pull request Jul 27, 2022

JBPM-10058 Task form with LocalDateTime datatype displays time format…

5a7d208

… even when the option is unflagged (#2768)

nmirasch pushed a commit that referenced this pull request Jul 27, 2022

JBPM-10058 Task form with LocalDateTime datatype displays time format…

fd11109

… even when the option is unflagged (#2768)

	String outString = bootstrapFormRenderer.renderProcess("test-contianerId", processAssetDesc, form);
	String outString = bootstrapFormRenderer.renderProcess("test-containerId", processAssetDesc, form);

JBPM-10058 Task form with LocalDateTime datatype displays time format… #2768

JBPM-10058 Task form with LocalDateTime datatype displays time format… #2768

Conversation

bxf12315 commented Apr 7, 2022 • edited by mareknovotny Loading

jstastny-cz left a comment

Choose a reason for hiding this comment

bxf12315 commented Apr 7, 2022

bxf12315 commented Apr 7, 2022

bxf12315 commented Apr 8, 2022

yesamer commented Apr 11, 2022

nmirasch commented Apr 12, 2022

ChughShilpa commented Apr 12, 2022

jstastny-cz commented Apr 13, 2022

jstastny-cz commented Apr 13, 2022

jstastny-cz commented Apr 13, 2022

bxf12315 commented Apr 14, 2022

bxf12315 commented Apr 14, 2022

bxf12315 commented Apr 14, 2022

bxf12315 commented Apr 14, 2022

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

bxf12315 commented Apr 15, 2022

bxf12315 commented Apr 15, 2022

bxf12315 commented Apr 15, 2022

bxf12315 commented Apr 15, 2022

bxf12315 commented May 7, 2022

bxf12315 commented May 8, 2022

bxf12315 commented May 8, 2022

bxf12315 commented May 9, 2022

ChughShilpa commented May 9, 2022 • edited Loading

bxf12315 commented May 9, 2022

ChughShilpa commented May 9, 2022

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

bxf12315 May 10, 2022 • edited Loading

Choose a reason for hiding this comment

mareknovotny commented May 11, 2022

nmirasch commented May 11, 2022

jstastny-cz left a comment

Choose a reason for hiding this comment

nmirasch left a comment

Choose a reason for hiding this comment

ChughShilpa left a comment

Choose a reason for hiding this comment

mareknovotny commented May 12, 2022

pefernan left a comment

Choose a reason for hiding this comment

sonarcloud bot commented May 12, 2022

bxf12315 commented Apr 7, 2022 •

edited by mareknovotny

Loading

ChughShilpa commented May 9, 2022 •

edited

Loading

bxf12315 May 10, 2022 •

edited

Loading