Skip to content

UPSE-386: Update to optionally include 'favorite' flag in portlet sea…#2678

Merged
bjagg merged 1 commit intouPortal-Project:masterfrom
groybal:UPSE-386
Aug 2, 2023
Merged

UPSE-386: Update to optionally include 'favorite' flag in portlet sea…#2678
bjagg merged 1 commit intouPortal-Project:masterfrom
groybal:UPSE-386

Conversation

@groybal
Copy link
Contributor

@groybal groybal commented Jul 17, 2023

…rch results

Checklist
Description of change

The portlets search API ( /uPortal/api/v5-0/portal/search?q={search term}*&type=portlets) has been updated to include "favorite" property in the results.

{"portlets":[
{"description":"Collection of administrative tools displayed in the ESCO Content Grid","favorite":"false","fname":"admin-dashboard","name":"Admin Dashboard","score":"5.0","title":"Admin Dashboard","url":"/uPortal/f/admin/p/admin-dashboard.u13l1n9/max/render.uP"},{"description":"Administrative functions for tenants in the portal","favorite":"false","fname":"tenant-portal-administration","name":"Tenant Portal Administration","score":"4.0","title":"Tenant Portal Administration","url":"/uPortal/p/tenant-portal-administration.ctf4/max/render.uP"},{"description":"Useful administrative links for managing portal entities and impersonating users","favorite":"false","fname":"portal-administration","name":"Portal Administration","score":"4.0","title":"Portal Administration","url":"/uPortal/p/portal-administration.ctf5/max/render.uP"},
{"description":"Portlet for Portlet Administration","favorite":"true","fname":"portlet-admin","name":"Portlet Administration","score":"4.0","title":"Portlet Administration","url":"/uPortal/f/u15l1s4/p/portlet

A new property has been added to enable/disable this:
org.apereo.portal.rest.search.PortletsSearchStrategy.displayFavoriteFlag

There was no existing unit test class for PortletsSearchStrategy. A brief attempt to create one was made, but it was determined that it would take to much time and require some refactoring.

@sonatype-lift
Copy link

sonatype-lift bot commented Jul 17, 2023

Sonatype Lift is retiring

Sonatype Lift will be retiring on Sep 12, 2023, with its analysis stopping on Aug 12, 2023. We understand that this news may come as a disappointment, and Sonatype is committed to helping you transition off it seamlessly. If you’d like to retain your data, please export your issues from the web console.
We are extremely grateful and thank you for your support over the years.

📖 Read about the impacts and timeline

##
## Flag to enable or disable the display of the favorite flag in the results
##
org.apereo.portal.rest.search.PortletsSearchStrategy.favoriteFlagScore:true
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I set this to true but should it instead be 'false' by default?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is a new property I would be okay with it defaulting to true

@groybal groybal force-pushed the UPSE-386 branch 2 times, most recently from 64e0020 to e5c7932 Compare July 17, 2023 22:07
Copy link
Contributor

@cbeach47 cbeach47 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good @groybal ! Leaving the new API config as true works as long as the UX doesn't break with the new field present. Can you confirm the UX works with this new field?

For the tests, can you enhance https://github.com/uPortal-Project/uPortal-start/blob/master/tests/api/search-v5_0.spec.ts with the new field? Even if it's just checking the favorite flag is false, it'd be good. If it's simple/quick, it'd be great to create a new test that favorites a portlet and then confirm the API search call reflect that. Just create a PR in uPortal-start with the change, and once uPortal is released, we can merge in the uPortal-start testing PR.

Note: To run the playwright tests, build uPortal locally, then hook that local build into uPortal-start, and spin up uPortal. Once it's running, then run the uPortal-start gradle task playwrightRun .

@groybal
Copy link
Contributor Author

groybal commented Aug 1, 2023

Changes look good @groybal ! Leaving the new API config as true works as long as the UX doesn't break with the new field present. Can you confirm the UX works with this new field?

For the tests, can you enhance https://github.com/uPortal-Project/uPortal-start/blob/master/tests/api/search-v5_0.spec.ts with the new field? Even if it's just checking the favorite flag is false, it'd be good. If it's simple/quick, it'd be great to create a new test that favorites a portlet and then confirm the API search call reflect that. Just create a PR in uPortal-start with the change, and once uPortal is released, we can merge in the uPortal-start testing PR.

Note: To run the playwright tests, build uPortal locally, then hook that local build into uPortal-start, and spin up uPortal. Once it's running, then run the uPortal-start gradle task playwrightRun .

I have not noticed any issues in the UX with the new field.
It took some work, but I was able update the Playwright test to check that the new 'favorite' flag is included, and that it has value of true for a favorited portlet. See PR: uPortal-Project/uPortal-start#648

Copy link
Contributor

@cbeach47 cbeach47 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, and thanks for adding the uP-start PW tests!

@bjagg bjagg merged commit b02ce60 into uPortal-Project:master Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants