Skip to content
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.

Sign up for GitHub

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

Jump to bottom

LPS-173907 Display list of available headless resources in FDS Views Manager #129996

Closed
wants to merge 4 commits into from
Closed

LPS-173907 Display list of available headless resources in FDS Views Manager #129996

wants to merge 4 commits into from

Conversation

liferay-continuous-integration
Copy link
Collaborator

Forwarded from: liferay-frontend#3031 (Took 1 ci:forward attempt in 9 minutes)
Console

@markocikos
@liferay-frontend

Original pull request comment:

References

What is the goal of this PR?

We are collecting a list of headless resources in DXP, and displaying them in UI for selection.

This is a bit chonky PR. I originally sent part of it as unfinished placeholder, but PR got stuck and we changed parts of UI. In this PR, both frontend and backend are complete.

Pinging @LuismiBarcos @ccorreagg @matijapetanjek for review of the backend part.

Backend notes:

  • We are defining a DXP headless resource as an OSGi component with two properties:
    • osgi.jaxrs.resource=true. This means that headless endpoint exists. Is this correct?
    • entity.class.name is not null. This is the unique identifier of resource. Is this correct?
  • For this, I used a similar pattern as batch planner, that is leveraging vulcan registry. We are doing similar fetching in an internal util.
  • It looks like above does not cover Object entities. Supporting them may be a separate task.
  • FDSHeadlessResource class is just for convenience, containing relevant properties of headless resources. It makes for a bit more readable code, at least in my opinion.
  • We are not using entityClassName in this PR, but this is unique identifier that will be key for the next task: fetching of resource fields based on selection in UI.

Pinging @bryceosterhaus @carloslancha @edalgrin for review of the frontend part

Frontend notes:

  • We are making a deliberate choice to implement FDS directly, as a React component. This is in contrast to standard JSP tag implementation in DXP. Intent is to iron out quirks and improve developer experience with this type of development.
  • I ran into few complications with ClayAutocomplete. It it difficult or impossible to implement it with custom HTML. For initial version, in this PR, we opted for a simpler solution, with just text in items. This is the only significant difference from mockups.
  • In this PR, we are preloading the whole list of resources. There are 261 headless resources in DXP. Size of hundreds is a bit awkward for UI, as it starts to impact autocomplete performance and page load time. It still seems acceptable to me, but we may want to implement async loading instead. We can do this as a separate task. On the other hand, we may want to restrict pool of available resources for FDS, which could make async solution unnecessary. This is TBD.
  • Custom style in AddFDSView.scss is a bit awkward, as the style may bleed to rest of the page. The problem is, we cannot place a custom wrapper CSS class on a dropdown in ClayAutocomplete. Fortunately, this is the only dropdown on the page.

cc @dsanz @ugeortiz @matuzalemsteles

What does it look like?

screen.mp4

Steps to reproduce

  1. Add feature.flag.LPS-164563=true to portal-ext.properties
  2. Open Global Menu > Control Panel > Object > Datasets

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

✔️ ci:test:relevant - 70 out of 70 jobs passed in 3 hours 31 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: 6005c85e2ecc68aa2d02852902b1c6794ad83fad

Upstream Comparison:

Branch GIT ID: 6005c85e2ecc68aa2d02852902b1c6794ad83fad
Jenkins Build URL: EE Development Acceptance (master) - 4265 - 2023-02-07[10:08:23]

ci:test:stable - 27 out of 27 jobs PASSED
27 Successful Jobs:
ci:test:relevant - 70 out of 70 jobs PASSED
70 Successful Jobs:
For more details click here.
Test bundle downloads:

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

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: 6005c85e2ecc68aa2d02852902b1c6794ad83fad

Sender Branch:

Branch Name: LPS-173907
Branch GIT ID: f996beb22e6902b4c2ebf7ae38d063e21b375957

1 out of 1jobs PASSED
1 Successful Jobs:
For more details click here.

@liferay-continuous-integration liferay-continuous-integration mentioned this pull request Feb 9, 2023
Closed
@liferay-continuous-integration
Copy link
Collaborator Author

To conserve resources, the PR Tester does not automatically run for forwarded pull requests.

@brianchandotcom
Copy link
Owner

Merged. Thank you.
View total diff: 9bbad2b...cbf67ef

@liferay-continuous-integration liferay-continuous-integration deleted the ci-forward-LPS-173907-pr-3031-sender-markocikos-ts-1675943825698 branch February 17, 2023 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
ci:test:relevant - success ci:test:sf - success ci:test:stable - success
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@liferay-continuous-integration @brianchandotcom @markocikos