-
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
LPS-195155 Implement saving of FDS item actions in DSM #3639
Conversation
CI is automatically triggering the following test suites:
|
✔️ ci:test:sf - 1 out of 1 jobs passed in 4 minutesClick here for more details.Base Branch:Branch Name: master Sender Branch:Branch Name: LPS-195155 1 Successful Jobs:For more details click here. |
Jenkins Build:test-portal-source-format#5623 Jenkins Report:jenkins-report.html Jenkins Suite:sf Pull Request:liferay-frontend#3639 Testray Routine:EE Pull Request Testray Importer:publish-testray-report#21521 |
Jenkins Build:test-portal-acceptance-pullrequest(master)#5776 Jenkins Report:jenkins-report.html Jenkins Suite:relevant Pull Request:liferay-frontend#3639 Testray Routine:EE Pull Request Testray Build:[master] ci:test:relevant - markocikos > liferay-frontend - PR#3639 - 2023-09-05[08:12:40] Testray Importer:publish-testray-report#10565 |
761de1b
to
bf68a99
Compare
ObjectField objectField = ObjectFieldUtil.createObjectField( | ||
ObjectFieldConstants.BUSINESS_TYPE_TEXT, | ||
ObjectFieldConstants.DB_TYPE_STRING, true, false, null, label, name, | ||
false); |
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.
"required", the last argument, is hardcoded to false
. If it is not, Objects backend API will throw an error.
notOrdered = storedFDSSorts.filter( | ||
(filter) => !fdsSortsOrderArray.includes(String(filter.id)) | ||
); |
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.
This is a bugfix of a case I found by accident. It is possible that the ordered CSV contains a lot of values, and a smaller amount of saved value(s) are not in it. This happens when you delete items externally.
Steps:
- Create few actions in DSM UI.
- Save in any order.
- Delete all action items through published "FDS Action" Objects page.
- Create an item in DSM UI.
What happens:
- New item is not displayed. Length check in this line does not include it.
I applied all ordering fixes to Sorts, Filters and Actions.
} | ||
|
||
setFDSSorts([...ordered, ...notOrdered]); | ||
setFDSSorts([...notOrdered, ...ordered]); |
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.
I'm placing unordered before ordered here, to ensure new items are on top.
@@ -376,14 +376,12 @@ const Sorting = ({ | |||
useEffect(() => { | |||
const getFDSSort = async () => { | |||
const response = await fetch( | |||
`${API_URL.FDS_VIEWS}/${fdsView.id}?nestedFields=${OBJECT_RELATIONSHIP.FDS_VIEW_FDS_SORT}` | |||
`${API_URL.FDS_SORTS}?filter=(${OBJECT_RELATIONSHIP.FDS_VIEW_FDS_SORT_ID} eq '${fdsView.id}')&nestedFields=${OBJECT_RELATIONSHIP.FDS_VIEW_FDS_SORT}&sort=dateCreated:desc` |
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.
I'm adding sort by creation date, to put the newest unordered item first.
Also, I'm using exact resource endpoint, that supports sorting, pagination etc.
<ClayForm.Group> | ||
<label htmlFor="headlessActionKeyInput"> | ||
{Liferay.Language.get( |
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.
I'm removing this whole form group, because the headless action key is not in the line
action type.
const confirmationMessageFormElementId = `${namespace}ConfirmationMessage`; | ||
const confirmationMessageTypeFormElementId = `${namespace}ConfirmationMessageType`; | ||
|
||
const ConfirmationMessageRow = () => ( |
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.
Buying space. No state in this component, so we should be safe.
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.
I spent quite some time debugging this, but abstracting the row into this component bugged the InputLocalized
component inside it, so I will be removing this abstraction in a follow-up. cc @dsanz this is the thing we talked about on friday
bf68a99
to
0d51bd6
Compare
One dependency merged, one to go. |
0d51bd6
to
454ce08
Compare
I added two commits to PR, one applying new toast utils throughout DSM, and another with minor style fixes. I don't plan on adding any additional changes. As soon as the spritemap PR is merged, I'll set this as ready for review. |
Jenkins Build:test-portal-acceptance-pullrequest(master)#7791 Jenkins Report:jenkins-report.html Jenkins Suite:relevant Pull Request:liferay-frontend#3639 Testray Routine:EE Pull Request Testray Build:[master] ci:test:relevant - markocikos > liferay-frontend - PR#3639 - 2023-09-12[02:18:01] Testray Importer:publish-testray-report#18886 |
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:
|
Hey @markocikos these failures are not related to your changes! |
Jenkins Build:test-portal-acceptance-pullrequest(master)#5984 Jenkins Report:jenkins-report.html Jenkins Suite:relevant Pull Request:liferay-frontend#3639 Testray Routine:EE Pull Request Testray Build:[master] ci:test:relevant - markocikos > liferay-frontend - PR#3639 - 2023-09-12[09:16:37] Testray Importer:publish-testray-report#10940 |
ci:forward:force |
CI is automatically triggering the following test suites:
The pull request will automatically be forwarded to the user
|
Skipping previously completed test suites:
|
All required test suite(s) completed. |
Error has occurred while attempting to forward pull request to |
1 similar comment
Error has occurred while attempting to forward pull request to |
Error has occurred while forwarding pull request to |
Manual forward brianchandotcom#140636 |
References
What is the goal of this PR?
We are implementing persistence for FDS Actions, UI to save and reorder FDS Actions, and doing some minor improvements on the way.
See technical notes inline.
TODO in a separate task:
What does it look like?
vokoscreenNG-2023-09-11_11-36-36.mp4