-
Notifications
You must be signed in to change notification settings - Fork 917
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
[Workspace] feat: optimize recent items and filter out items whose workspace is deleted #8900
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8900 +/- ##
=======================================
Coverage 60.87% 60.87%
=======================================
Files 3802 3802
Lines 91060 91060
Branches 14370 14370
=======================================
+ Hits 55435 55436 +1
+ Misses 32085 32084 -1
Partials 3540 3540
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@@ -213,45 +226,45 @@ export const RecentItems = ({ | |||
type: item.meta?.type || '', | |||
id: item.id, | |||
})); | |||
|
|||
if (savedObjects.length) { | |||
// Deep compare navLinks to avoid unnecessary requests |
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.
How about using useDeepCompareEffect
instead? Current implementation won't be executed again after other dependencies(eg: recentlyAccessedItems
or workspaceList
) changed but navLinks
are same.
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.
After offline discussion, we only need to deep compare navLinks because only its reference would be changed after appUpdater.
.href, | ||
}; | ||
// If the workspace id is existing but the workspace is deleted, filter the item | ||
if (recentAccessItem.workspaceId && !findWorkspace) { |
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.
Do we need to check if workspace feature enabled here?
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.
If workspace feature is disabled, recentAccessItem would not have workspaceId
.
bulkGetDetail(savedObjects, http).then((res) => { | ||
const filteredNavLinks = navLinks.filter((link) => !link.hidden); | ||
const formatDetailedSavedObjects = res.map((obj) => { | ||
const formatDetailedSavedObjects = res.flatMap((obj) => { |
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.
can we do filtering by workspace before bulkGet as ChromeRecentlyAccessedHistoryItem
has workspace already.
…eleted Signed-off-by: tygao <[email protected]>
Signed-off-by: tygao <[email protected]>
Signed-off-by: tygao <[email protected]>
…rkspace is deleted (#8900) * feat: optimize recent items and filter out items whose workspace is deleted Signed-off-by: tygao <[email protected]> * Changeset file for PR #8900 created/updated * seperate link Signed-off-by: tygao <[email protected]> * update filter sequence Signed-off-by: tygao <[email protected]> --------- Signed-off-by: tygao <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> (cherry picked from commit c24d5bc) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…rkspace is deleted (#8900) (#8983) * feat: optimize recent items and filter out items whose workspace is deleted * Changeset file for PR #8900 created/updated * seperate link * update filter sequence --------- (cherry picked from commit c24d5bc) Signed-off-by: tygao <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
…rkspace is deleted (opensearch-project#8900) * feat: optimize recent items and filter out items whose workspace is deleted Signed-off-by: tygao <[email protected]> * Changeset file for PR opensearch-project#8900 created/updated * seperate link Signed-off-by: tygao <[email protected]> * update filter sequence Signed-off-by: tygao <[email protected]> --------- Signed-off-by: tygao <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
…rkspace is deleted (opensearch-project#8900) * feat: optimize recent items and filter out items whose workspace is deleted Signed-off-by: tygao <[email protected]> * Changeset file for PR opensearch-project#8900 created/updated * seperate link Signed-off-by: tygao <[email protected]> * update filter sequence Signed-off-by: tygao <[email protected]> --------- Signed-off-by: tygao <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Signed-off-by: Federico Silva <[email protected]>
…rkspace is deleted (opensearch-project#8900) * feat: optimize recent items and filter out items whose workspace is deleted Signed-off-by: tygao <[email protected]> * Changeset file for PR opensearch-project#8900 created/updated * seperate link Signed-off-by: tygao <[email protected]> * update filter sequence Signed-off-by: tygao <[email protected]> --------- Signed-off-by: tygao <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
Description
Screenshot
No UI change
Testing the changes
Changelog
Check List
yarn test:jest
yarn test:jest_integration