-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[TS] LPS-148820 | Can't select a page from the staging environment in link to page selector DDMForms #1534
Conversation
Hi, you don’t need to run the CI commands anymore, I’ll do it for you and let you know if any issue happens. |
ci:test:sf |
To conserve resources, the PR Tester does not automatically run for every pull. If your code changes were already tested in another pull, reference that pull in this pull so the test results can be analyzed. If your pull was never tested, comment "ci:test" to run the PR Tester for this pull. |
✔️ ci:test:sf - 1 out of 1 jobs passed in 3 minutesClick here for more details.Base Branch:Branch Name: master Sender Branch:Branch Name: LPS-148820 1 Successful Jobs:For more details click here. |
Jenkins Build:test-portal-source-format#2075 Jenkins Report:jenkins-report.html Jenkins Suite:sf Pull Request:liferay-forms#1534 Testray Routine:EE Pull Request Testray Importer:publish-testray-report#7733 |
ci:test:relevant |
Jenkins Build:test-portal-acceptance-pullrequest(master)#687 Jenkins Report:jenkins-report.html Jenkins Suite:relevant Pull Request:liferay-forms#1534 Testray Routine:EE Pull Request Testray Build:[master] ci:test:relevant - bakayattila > liferay-forms - PR#1534 - 2022-03-16[10:06:51] Testray Importer:publish-testray-report#1694 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, @bakayattila.
Thanks for your PR. Could you please explain this logic?
Looking at the bug, it seems to me that just assigning the scopeGroupId
to groupId
would fix the problem. Would it work for you and the other cases?
Regards.
Hi @rodrigopaulino, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for clarifying, @bakayattila. I understand your logic now, although I believe the fix for LPS-88009 wasn't working all along:
It attempted to solve the bug at publication by inferring another groupId at selection. If the problem was that links to pages weren't pointing to the correct site after publication, the record values should have been updated to the correct site during publication.
For LPS-135851, I don't understand why they used the getScopeGroupIdOrLiveGroupId
. I assume they were trying to fix their problem of not having the scope groupId while not breaking the expected behavior on LPS-88009 which involved the links pointing to the live site at the final step.
Nevertheless, the correct behavior is to have the scope groupId to list its respective pages in this type of field. If you intend to fix LPS-88009 in this PR, I encourage you to seek for solution during publication in order to swap the staging site for the live site at the stored records.
Does this make sense?
Let me know anything. 😃
Hi @rodrigopaulino, |
Hey, @bakayattila. Once again, thanks for reminding me about the Web Content stage condition in LPS-88009. To check if a portlet is staged or not, I believe you could introduce to frontend's
And for the logic inside ddm_form, you could create a function and reuse it on all different places where it's needed to avoid duplicating code. |
I also thought that the problem with this idea is that you can edit the dynamic data list from the content > dynamic data list and from the dynamic data list display widget and the latter one will always show it's staged even if you don't stage the dynamic data lists. |
@bakayattila, could you please explain why would it "always show it's staged even if you don't stage the dynamic data lists"? I'm sorry I didn't quite get the rationale. |
@rodrigopaulino, You can edit the dynamic data list from two locations. the dynamic data list portlet and from the dynamic data list display portlet. If I remember right even if we don't stage the dynamic data lists the dynamic data list display portlet will show that it is staged probably because it is on a layout that is staged. |
Hey, @bakayattila. The UnicodeProperties (TypeSettingsProperties) of a group contains the site's staging configuration. If you create a staged environment and disable staging for a certain content, then the TypeSettingsProperties will contain that information independently from the page you're visiting. |
✔️ ci:test:sf - 1 out of 1 jobs passed in 6 minutesClick here for more details.Base Branch:Branch Name: master Sender Branch:Branch Name: LPS-148820 1 Successful Jobs:For more details click here. |
ci:test:relevant |
Jenkins Build:test-portal-source-format#6257 Jenkins Report:jenkins-report.html Jenkins Suite:sf Pull Request:liferay-forms#1534 Testray Routine:EE Pull Request Testray Importer:publish-testray-report#2405 |
❌ ci:test:stable - 30 out of 33 jobs passed❌ ci:test:relevant - 30 out of 34 jobs passed in 1 hour 50 minutesClick here for more details.Base Branch:Branch Name: master Upstream Comparison:Branch GIT ID: b2fde097c2a23b9dc3e508874f2b32a04bab25c7 ci:test:stable - 30 out of 33 jobs PASSED3 Failed Jobs:30 Successful Jobs:
ci:test:relevant - 30 out of 34 jobs PASSED4 Failed Jobs:
30 Successful Jobs:
For more details click here.Failures unique to this pull:
Failures in common with acceptance upstream results at b2fde09:
|
Hey @bakayattila, can you please check if these failures are related to your changes? Thanks |
Jenkins Build:test-portal-acceptance-pullrequest(master)#857 Jenkins Report:jenkins-report.html Jenkins Suite:relevant Pull Request:liferay-forms#1534 Testray Routine:EE Pull Request Testray Build:[master] ci:test:relevant - bakayattila > liferay-forms - PR#1534 - 2022-04-01[03:00:56] Testray Importer:publish-testray-report#2013 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🎉
ci:forward |
CI is automatically triggering the following test suites:
The pull request will automatically be forwarded to the user
|
Skipping previously passed test suites: |
All required test suite(s) passed. |
Pull request has been successfully forwarded to brianchandotcom#115591 |
Related Issue: https://issues.liferay.com/browse/LPS-148820
Hi Team,
Could you please review this pull request?
https://issues.liferay.com/browse/LPS-148820
Thank you!
I tried to fix it without breaking LPS-135851 or LPS-88009.