[DE-Migration] LPS-125543 Some fields have maps as values #118

Hi @rodrigopaulino!

Can you please sort these? :

cb3b534#diff-b41d54bd1fe8c9e86e51af8165fcce698d63c3b3d7b40b7f267a9bc866e2aa99R172-R178

cb3b534#diff-4b2de1d0236005170fc53069bb5bf9650f04d657b44acd73ca4d7033feb508fcR52-R54

cb3b534#diff-21e838073ea4f97d5e50aba388a68f45014bef5338f48736aca387111d565254R73-R77

Also i think the if you introduced falls in the same case that i mentioned here to @victorg1991 .

Looks like you're trying to fix similar problems?

Hey, @jeyvison. I can't sort those. It was the sourceFormatter who did it.
In regards to your comment to Victor's PR, we can't treat the Map objects in the else clause because their string values are not proper serialized JSON. It will cause problems with parsing. Also, what Victor is doing is basically the same thing as I am with a different approach, but I'm not sure which one of those will perform better. ;)

jeyvison · 2021-01-07T19:20:33Z

Hey, @jeyvison. I can't sort those. It was the sourceFormatter who did it.

Got it.

In regards to your comment to Victor's PR, we can't treat the Map objects in the else clause because their string values are not proper serialized JSON. It will cause problems with parsing. Also, what Victor is doing is basically the same thing as I am with a different approach, but I'm not sure which one of those will perform better. ;)

Humm. understood. But we don't need the else anymore right? Or we dont need this if. Since we are considering that everything that falls under else is a Map we can just move this logic to there. Am i right?

rodrigopaulino · 2021-01-07T19:23:51Z

Humm. understood. But we don't need the else anymore right? Or we dont need this if. Since we are considering that everything that falls under else is a Map we can just move this logic to there. Am i right?

I see your question. I don't know that answer. Is every possible value an Array of Objects or a Map? If yes, we can sure replace the else block with this new code.

Edit: Oh! I went there again and saw the casting for Map<String, ?>. Sure, sure. We can avoid the "else if". Should I send this again or will you keep Victor's PR? Let me know either way. ;)

Edit 2: @jeyvison, actually I can't guarantee by reading that interface that every value.getValue() will be a Map if not an array of Objects. You see, the else block casts the value itself, not the "value of the value". Please validate my answer here. I'm starting to get confused 😅

liferay-continuous-integration · 2021-01-07T20:07:57Z

✔️ ci:test:stable - 9 out of 9 jobs passed

❌ ci:test:relevant - 23 out of 26 jobs passed in 3 hours 2 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: d04f890cc2894e1402c65e958e6d4ec70c8dfece

Copied in Private Modules Branch:

Branch Name: master-private
Branch GIT ID: 15218ef33680a1fe233ea812e8d097cfcb0feb42

ci:test:stable - 9 out of 9 jobs PASSED

9 Successful Jobs:

ci:test:relevant - 23 out of 26 jobs PASSED

3 Failed Jobs:

23 Successful Jobs:

For more details click here.

Failures unique to this pull:

test-portal-acceptance-pullrequest-batch(master)/functional-tomcat90-mysql57-jdk8/1
Job Results:
61 Tests Passed.
1 Test Failed.
1. AXIS_VARIABLE=16,label_exp=!master #450767
  1. LocalFile.FormRules#ViewPreviousFormRuleIsCopiedToNewDuplicatedForm - Poshi Report - Poshi Summary - Console Output
test-portal-acceptance-pullrequest-batch(master)/functional-tomcat90-mysql57-jdk8/2
Job Results:
58 Tests Passed.
1 Test Failed.
1. AXIS_VARIABLE=8,label_exp=!master #460745
  1. LocalFile.FormRules#ViewShowRuleActivationAfterInputtingUnrelatedField - Poshi Report - Poshi Summary - Console Output

Failures in common with acceptance upstream results at d04f890:

test-portal-acceptance-pullrequest-batch(master)/functional-tomcat90-mysql57-jdk8/1
Job Results:
61 Tests Passed.
2 Tests Failed.
1. AXIS_VARIABLE=17,label_exp=!master #450767
  1. LocalFile.CPDynamicdatalists#AddFormTemplate - Poshi Report - Poshi Summary - Console Output
  2. LocalFile.CPDynamicdatalists#AddListRecordWithLinkToPageField - Poshi Report - Poshi Summary - Console Output
test-portal-acceptance-pullrequest-batch(master)/functional-tomcat90-mysql57-jdk8/2
Job Results:
58 Tests Passed.
3 Tests Failed.

jeyvison · 2021-01-07T20:27:41Z

@rodrigopaulino You're completely right!

@victorg1991 Can you check if this PR fix your LPS? If it does i think i'll foward this one.

Thanks a lot to you both!

liferay-continuous-integration · 2021-01-07T20:49:32Z

Jenkins Build:test-portal-acceptance-pullrequest(master)#7235
Jenkins Report:jenkins-report.html
Jenkins Suite: relevant
Pull Request:liferay-data-engine#118
Spira Release:/Liferay DXP 7.4/Pull Request/ci:test:relevant
Spira Release Build:master - rodrigopaulino > liferay-data-engine - PR#118 - 2021-01-07[09:27:33]
Spira Jenkins Build:publish-spira-report#12592

victorg1991 · 2021-01-08T08:46:53Z

Sure @jeyvison You can send this one, it is almost the same code as mine. In my pr another bug was also solved so I'd create another lps for that and sent it separately

Thanks!

victorg1991 · 2021-01-08T09:38:34Z

I've resent this pr again here: #121 and added a commit as the other commit depended on this

jeyvison · 2021-01-08T12:52:18Z

Awesome! Thanks a lot @victorg1991 !

jeyvison · 2021-01-08T12:57:26Z

ci:forward

liferay-continuous-integration · 2021-01-08T12:57:30Z

CI is automatically triggering the following test suites:

ci:test:relevant
ci:test:sf

The pull request will automatically be forwarded to the user brianchandotcom if the following test suites pass:

ci:test:relevant
ci:test:sf
ci:test:stable

liferay-continuous-integration · 2021-01-08T12:57:33Z

Skipping previously passed test suites:
ci:test:sf

jeyvison

LGTM!

jeyvison · 2021-01-08T13:11:23Z

Hi @rodrigopaulino , i'm forwarding victor's PR instead because he's is fixing his case on top of your PR and the tests are already ok there

liferay-continuous-integration · 2021-01-08T13:25:16Z

Build started.

Jenkins is currently running tests.

Base Branch:

Branch Name: master

Job Summary:

Job Link: test-portal-acceptance-pullrequest(master)

For more details click here.

rodrigopaulino added 3 commits January 7, 2021 10:54

LPS-125543 Possibility of creating JSONObjects by maps

cb3b534

LPS-125543 packageinfo

1d8e6da

LPS-125543 Some fields have maps as values

867df26

liferay-continuous-integration added ci:test:sf - pending ci:test:relevant - pending ci:test:stable - pending ci:test:sf - success and removed ci:test:sf - pending labels Jan 7, 2021

jeyvison suggested changes Jan 7, 2021

View reviewed changes

liferay-continuous-integration added ci:test:relevant - failure ci:test:stable - success and removed ci:test:relevant - pending ci:test:stable - pending labels Jan 7, 2021

victorg1991 mentioned this pull request Jan 8, 2021

LPS-125511 We can receive both string or object as inputValue #110

Closed

jeyvison self-requested a review January 8, 2021 12:57

jeyvison approved these changes Jan 8, 2021

View reviewed changes

jeyvison closed this Jan 8, 2021

liferay-continuous-integration added ci:test:relevant - pending ci:test:stable - pending and removed ci:test:relevant - failure ci:test:stable - success labels Jan 8, 2021

[DE-Migration] LPS-125543 Some fields have maps as values #118

[DE-Migration] LPS-125543 Some fields have maps as values #118

Conversation

rodrigopaulino commented Jan 7, 2021

liferay-continuous-integration commented Jan 7, 2021

rodrigopaulino commented Jan 7, 2021

rodrigopaulino commented Jan 7, 2021

liferay-continuous-integration commented Jan 7, 2021

✔️ ci:test:sf - 1 out of 1 jobs passed in 33 minutes

Base Branch:

Sender Branch:

For more details click here.

liferay-continuous-integration commented Jan 7, 2021

jeyvison left a comment

Choose a reason for hiding this comment

rodrigopaulino commented Jan 7, 2021

jeyvison commented Jan 7, 2021

rodrigopaulino commented Jan 7, 2021 • edited Loading

liferay-continuous-integration commented Jan 7, 2021

✔️ ci:test:stable - 9 out of 9 jobs passed

❌ ci:test:relevant - 23 out of 26 jobs passed in 3 hours 2 minutes

Base Branch:

Copied in Private Modules Branch:

3 Failed Jobs:

For more details click here.

Failures unique to this pull:

test-portal-acceptance-pullrequest-batch(master)/functional-tomcat90-mysql57-jdk8/1

Job Results:

test-portal-acceptance-pullrequest-batch(master)/functional-tomcat90-mysql57-jdk8/2

Job Results:

test-portal-acceptance-pullrequest-batch(master)/functional-tomcat90-mysql57-jdk8/1

Job Results:

test-portal-acceptance-pullrequest-batch(master)/functional-tomcat90-mysql57-jdk8/2

Job Results:

jeyvison commented Jan 7, 2021

liferay-continuous-integration commented Jan 7, 2021

victorg1991 commented Jan 8, 2021

victorg1991 commented Jan 8, 2021 • edited Loading

jeyvison commented Jan 8, 2021

jeyvison commented Jan 8, 2021

liferay-continuous-integration commented Jan 8, 2021

liferay-continuous-integration commented Jan 8, 2021

jeyvison left a comment

Choose a reason for hiding this comment

jeyvison commented Jan 8, 2021

liferay-continuous-integration commented Jan 8, 2021

Build started.

Base Branch:

Job Summary:

rodrigopaulino commented Jan 7, 2021 •

edited

Loading

victorg1991 commented Jan 8, 2021 •

edited

Loading