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.

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

feat: Enhance news load resources using load-group - EXO-63805 #910

Merged
merged 1 commit into from
Aug 17, 2023

Conversation

hakermi
Copy link
Member

@hakermi hakermi commented Aug 11, 2023

Prior to this change, all news extensions were loaded inside all-bottom-container including the non needed once in all pages. This PR add the use of load-group mechanism to enhance the resources load and load only what is needed in the current page.

Copy link
Member

@rdenarie rdenarie left a comment

Choose a reason for hiding this comment

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

Can you check the sonar feedback marked as BUG ?

Prior to this change, all news extensions were loaded inside all-bottom-container including the non needed once in all pages.
This PR add the use of load-group mechanism to enhance the resources load and load only what is needed in the current page.
@exo-swf
Copy link
Contributor

exo-swf commented Aug 17, 2023

Your PR triggers too many exo-ci builds! Please finish your work and then, set your PR ready! Thank you

@hakermi hakermi marked this pull request as ready for review August 17, 2023 09:55
@sonarcloud
Copy link

sonarcloud bot commented Aug 17, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

85.7% 85.7% Coverage
0.0% 0.0% Duplication

@hakermi hakermi requested a review from rdenarie August 17, 2023 09:56
@hakermi
Copy link
Member Author

hakermi commented Aug 17, 2023

Can you check the sonar feedback marked as BUG ?

Done

Comment on lines +583 to +589
Identity userIdentity = identityManager.getOrCreateUserIdentity(currentIdentity.getUserId());
if (userIdentity != null) {
news.setFavorite(favoriteService.isFavorite(new Favorite("news",
news.getId(),
"",
Long.parseLong(userIdentity.getId()))));
}
Copy link
Member

Choose a reason for hiding this comment

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

This should be removed ?

Copy link
Member Author

Choose a reason for hiding this comment

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

why it should be removed? i have faced an existing old issue and i fixed it.

Copy link
Member Author

Choose a reason for hiding this comment

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

the issue is that when access news details and that you have set the news as favorite, you will still have the option to set it again and the icon of favorite is empty icon which indicates that news is not marked as favorite while it's the case

@@ -152,6 +152,7 @@
activityId: '${activityId}',
hiddenSpace: ${news.hiddenSpace},
isSpaceMember: ${news.isSpaceMember},
favorite: ${news.isFavorite}
Copy link
Member

Choose a reason for hiding this comment

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

Why do you need this variable ?

Copy link
Member Author

Choose a reason for hiding this comment

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

@hakermi hakermi requested a review from rdenarie August 17, 2023 12:14
Copy link
Member

@rdenarie rdenarie left a comment

Choose a reason for hiding this comment

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

ok for me

@hakermi hakermi merged commit a3f9738 into feature/devx Aug 17, 2023
10 checks passed
@hakermi hakermi deleted the TASK-63805 branch August 17, 2023 12:29
hakermi added a commit that referenced this pull request Aug 18, 2023
Prior to this change, all news extensions were loaded inside all-bottom-container including the non needed once in all pages.
This PR add the use of load-group mechanism to enhance the resources load and load only what is needed in the current page.
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.

3 participants