From eec50cdf913fd8cf9f0693b177f17783f1c72790 Mon Sep 17 00:00:00 2001 From: tygao Date: Wed, 20 Nov 2024 11:37:59 +0800 Subject: [PATCH 1/4] feat: optimize recent items and filter out items whose workspace is deleted Signed-off-by: tygao --- .../chrome/ui/header/recent_items.test.tsx | 33 +++++--- .../public/chrome/ui/header/recent_items.tsx | 81 +++++++++++-------- 2 files changed, 71 insertions(+), 43 deletions(-) diff --git a/src/core/public/chrome/ui/header/recent_items.test.tsx b/src/core/public/chrome/ui/header/recent_items.test.tsx index d01912e9c27f..28bae880fcfa 100644 --- a/src/core/public/chrome/ui/header/recent_items.test.tsx +++ b/src/core/public/chrome/ui/header/recent_items.test.tsx @@ -18,7 +18,7 @@ jest.mock('./nav_link', () => ({ }), })); -const mockRecentlyAccessed = new BehaviorSubject([ +const mockRecentlyAccessed$ = new BehaviorSubject([ { id: '6ef856c0-5f86-11ef-b7df-1bb1cf26ce5b', label: 'visualizeMock', @@ -28,7 +28,7 @@ const mockRecentlyAccessed = new BehaviorSubject([ }, ]); -const mockWorkspaceList = new BehaviorSubject([ +const mockWorkspaceList$ = new BehaviorSubject([ { id: 'workspace_1', name: 'WorkspaceMock_1', @@ -49,7 +49,14 @@ const defaultMockProps = { navigateToUrl: applicationServiceMock.createStartContract().navigateToUrl, workspaceList$: new BehaviorSubject([]), recentlyAccessed$: new BehaviorSubject([]), - navLinks$: new BehaviorSubject([]), + navLinks$: new BehaviorSubject([ + { + id: '', + title: '', + baseUrl: '', + href: '', + }, + ]), basePath: httpServiceMock.createStartContract().basePath, http: httpServiceMock.createSetupContract(), renderBreadcrumbs: <>, @@ -85,7 +92,8 @@ describe('Recent items', () => { it('should be able to render recent works', async () => { const mockProps = { ...defaultMockProps, - recentlyAccessed$: mockRecentlyAccessed, + recentlyAccessed$: mockRecentlyAccessed$, + workspaceList$: mockWorkspaceList$, }; await act(async () => { @@ -97,11 +105,11 @@ describe('Recent items', () => { expect(screen.getByText('visualizeMock')).toBeInTheDocument(); }); - it('shoulde be able to display workspace name if the asset is attched to a workspace and render it with brackets wrapper ', async () => { + it('should be able to display workspace name if the asset is attched to a workspace and render it with brackets wrapper ', async () => { const mockProps = { ...defaultMockProps, - recentlyAccessed$: mockRecentlyAccessed, - workspaceList$: mockWorkspaceList, + recentlyAccessed$: mockRecentlyAccessed$, + workspaceList$: mockWorkspaceList$, }; await act(async () => { @@ -116,8 +124,8 @@ describe('Recent items', () => { it('should call navigateToUrl with link generated from createRecentNavLink when clicking a recent item', async () => { const mockProps = { ...defaultMockProps, - recentlyAccessed$: mockRecentlyAccessed, - workspaceList$: mockWorkspaceList, + recentlyAccessed$: mockRecentlyAccessed$, + workspaceList$: mockWorkspaceList$, }; const navigateToUrl = jest.fn(); @@ -137,7 +145,7 @@ describe('Recent items', () => { it('should be able to display the preferences popover setting when clicking Preferences button', async () => { const mockProps = { ...defaultMockProps, - recentlyAccessed$: mockRecentlyAccessed, + recentlyAccessed$: mockRecentlyAccessed$, }; await act(async () => { @@ -158,4 +166,9 @@ describe('Recent items', () => { ); expect(baseElement).toMatchSnapshot(); }); + + it('should show not display item if it is in a workspace which is not available', () => { + render(); + expect(screen.queryByText('visualizeMock')).not.toBeInTheDocument(); + }); }); diff --git a/src/core/public/chrome/ui/header/recent_items.tsx b/src/core/public/chrome/ui/header/recent_items.tsx index 7efd276b8fa9..3194be43be9d 100644 --- a/src/core/public/chrome/ui/header/recent_items.tsx +++ b/src/core/public/chrome/ui/header/recent_items.tsx @@ -2,7 +2,7 @@ * Copyright OpenSearch Contributors * SPDX-License-Identifier: Apache-2.0 */ -import React, { useMemo, useState, useEffect } from 'react'; +import React, { useMemo, useState, useEffect, useRef } from 'react'; import * as Rx from 'rxjs'; import moment from 'moment'; import { @@ -122,6 +122,7 @@ export const RecentItems = ({ [] ); const navLinks = useObservable(navLinks$, []); + const prevNavLinksRef = useRef(); const loadingCount = useObservable(loadingCount$, 0); const handleItemClick = (link: string) => { @@ -143,7 +144,9 @@ export const RecentItems = ({ setIsPreferencesPopoverOpen((IsPreferencesPopoverOpe) => !IsPreferencesPopoverOpe); }} > - Preferences + {i18n.translate('core.header.recent.preferences', { + defaultMessage: 'Preferences', + })} } isOpen={isPreferencesPopoverOpen} @@ -152,7 +155,11 @@ export const RecentItems = ({ setIsPreferencesPopoverOpen(false); }} > - Preferences + + {i18n.translate('core.header.recent.preferences.title', { + defaultMessage: 'Preferences', + })} + Recents, + children: ( + + {i18n.translate('core.header.recent.preferences.legend', { + defaultMessage: 'Recents', + })} + + ), }} /> @@ -213,11 +226,14 @@ export const RecentItems = ({ type: item.meta?.type || '', id: item.id, })); - - if (savedObjects.length) { + // Deep compare navLinks to avoid unnecessary requests + if ( + savedObjects.length && + JSON.stringify(prevNavLinksRef.current) !== JSON.stringify(navLinks) + ) { bulkGetDetail(savedObjects, http).then((res) => { const filteredNavLinks = navLinks.filter((link) => !link.hidden); - const formatDetailedSavedObjects = res.map((obj) => { + const formatDetailedSavedObjects = res.flatMap((obj) => { const recentAccessItem = recentlyAccessedItems.find( (item) => item.id === obj.id ) as ChromeRecentlyAccessedHistoryItem; @@ -225,33 +241,30 @@ export const RecentItems = ({ const findWorkspace = workspaceList.find( (workspace) => workspace.id === recentAccessItem.workspaceId ); - return { - ...recentAccessItem, - ...obj, - ...recentAccessItem.meta, - updatedAt: moment(obj?.updated_at).valueOf(), - workspaceName: findWorkspace?.name, - link: createRecentNavLink(recentAccessItem, filteredNavLinks, basePath, navigateToUrl) - .href, - }; + // If the workspace id is existing but the workspace is deleted, filter the item + if (recentAccessItem.workspaceId && !findWorkspace) { + return []; + } + + return [ + { + ...recentAccessItem, + ...obj, + ...recentAccessItem.meta, + updatedAt: moment(obj?.updated_at).valueOf(), + workspaceName: findWorkspace?.name, + link: createRecentNavLink(recentAccessItem, filteredNavLinks, basePath, navigateToUrl) + .href, + }, + ]; }); - // here I write this argument to avoid Unnecessary re-rendering - if (JSON.stringify(formatDetailedSavedObjects) !== JSON.stringify(detailedSavedObjects)) { - setDetailedSavedObjects(formatDetailedSavedObjects); - } + setDetailedSavedObjects(formatDetailedSavedObjects); }); } - }, [ - navLinks, - basePath, - navigateToUrl, - recentlyAccessedItems, - http, - workspaceList, - detailedSavedObjects, - ]); + prevNavLinksRef.current = navLinks; + }, [navLinks, basePath, navigateToUrl, recentlyAccessedItems, http, workspaceList]); - const selectedRecentsItems = useMemo(() => { + const selectedRecentItems = useMemo(() => { return detailedSavedObjects.slice(0, Number(recentsRadioIdSelected)); }, [detailedSavedObjects, recentsRadioIdSelected]); @@ -283,9 +296,9 @@ export const RecentItems = ({ - {selectedRecentsItems.length > 0 ? ( + {selectedRecentItems.length > 0 ? ( - {selectedRecentsItems.map((item) => ( + {selectedRecentItems.map((item) => ( handleItemClick(item.link)} key={item.link} @@ -309,7 +322,9 @@ export const RecentItems = ({ ) : ( - No recently viewed items + {i18n.translate('core.header.recent.no.recents', { + defaultMessage: 'No recently viewed items', + })} )} From 16f804bdf76790c208321be1078d5ee7eb7bd3c6 Mon Sep 17 00:00:00 2001 From: "opensearch-changeset-bot[bot]" <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Date: Wed, 20 Nov 2024 03:40:41 +0000 Subject: [PATCH 2/4] Changeset file for PR #8900 created/updated --- changelogs/fragments/8900.yml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelogs/fragments/8900.yml diff --git a/changelogs/fragments/8900.yml b/changelogs/fragments/8900.yml new file mode 100644 index 000000000000..78ae369755a7 --- /dev/null +++ b/changelogs/fragments/8900.yml @@ -0,0 +1,2 @@ +feat: +- Optimize recent items and filter out items whose workspace is deleted ([#8900](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8900)) \ No newline at end of file From bc4aa4b1efc4c3450eb6a48af26c45e3e84c7b94 Mon Sep 17 00:00:00 2001 From: tygao Date: Wed, 20 Nov 2024 17:36:53 +0800 Subject: [PATCH 3/4] seperate link Signed-off-by: tygao --- .../public/chrome/ui/header/recent_items.tsx | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/core/public/chrome/ui/header/recent_items.tsx b/src/core/public/chrome/ui/header/recent_items.tsx index 3194be43be9d..61ab5cc1ede1 100644 --- a/src/core/public/chrome/ui/header/recent_items.tsx +++ b/src/core/public/chrome/ui/header/recent_items.tsx @@ -2,7 +2,7 @@ * Copyright OpenSearch Contributors * SPDX-License-Identifier: Apache-2.0 */ -import React, { useMemo, useState, useEffect, useRef } from 'react'; +import React, { useMemo, useState, useEffect } from 'react'; import * as Rx from 'rxjs'; import moment from 'moment'; import { @@ -122,7 +122,6 @@ export const RecentItems = ({ [] ); const navLinks = useObservable(navLinks$, []); - const prevNavLinksRef = useRef(); const loadingCount = useObservable(loadingCount$, 0); const handleItemClick = (link: string) => { @@ -226,13 +225,8 @@ export const RecentItems = ({ type: item.meta?.type || '', id: item.id, })); - // Deep compare navLinks to avoid unnecessary requests - if ( - savedObjects.length && - JSON.stringify(prevNavLinksRef.current) !== JSON.stringify(navLinks) - ) { + if (savedObjects.length) { bulkGetDetail(savedObjects, http).then((res) => { - const filteredNavLinks = navLinks.filter((link) => !link.hidden); const formatDetailedSavedObjects = res.flatMap((obj) => { const recentAccessItem = recentlyAccessedItems.find( (item) => item.id === obj.id @@ -253,16 +247,13 @@ export const RecentItems = ({ ...recentAccessItem.meta, updatedAt: moment(obj?.updated_at).valueOf(), workspaceName: findWorkspace?.name, - link: createRecentNavLink(recentAccessItem, filteredNavLinks, basePath, navigateToUrl) - .href, }, ]; }); setDetailedSavedObjects(formatDetailedSavedObjects); }); } - prevNavLinksRef.current = navLinks; - }, [navLinks, basePath, navigateToUrl, recentlyAccessedItems, http, workspaceList]); + }, [recentlyAccessedItems, http, workspaceList]); const selectedRecentItems = useMemo(() => { return detailedSavedObjects.slice(0, Number(recentsRadioIdSelected)); @@ -300,7 +291,16 @@ export const RecentItems = ({ {selectedRecentItems.map((item) => ( handleItemClick(item.link)} + onClick={() => + handleItemClick( + createRecentNavLink( + item, + navLinks.filter((link) => !link.hidden), + basePath, + navigateToUrl + ).href + ) + } key={item.link} style={{ padding: '1px' }} label={ From 03c6825de38a79c87a27a16cd414399cc28389f3 Mon Sep 17 00:00:00 2001 From: tygao Date: Fri, 22 Nov 2024 13:34:25 +0800 Subject: [PATCH 4/4] update filter sequence Signed-off-by: tygao --- .../public/chrome/ui/header/recent_items.tsx | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/src/core/public/chrome/ui/header/recent_items.tsx b/src/core/public/chrome/ui/header/recent_items.tsx index 61ab5cc1ede1..298bf51d2bc6 100644 --- a/src/core/public/chrome/ui/header/recent_items.tsx +++ b/src/core/public/chrome/ui/header/recent_items.tsx @@ -220,14 +220,21 @@ export const RecentItems = ({ useEffect(() => { const savedObjects = recentlyAccessedItems - .filter((item) => item.meta?.type) + .filter( + (item) => + item.meta?.type && + (!item.workspaceId || + // If the workspace id is existing but the workspace is deleted, filter the item + (item.workspaceId && + !!workspaceList.find((workspace) => workspace.id === item.workspaceId))) + ) .map((item) => ({ type: item.meta?.type || '', id: item.id, })); if (savedObjects.length) { bulkGetDetail(savedObjects, http).then((res) => { - const formatDetailedSavedObjects = res.flatMap((obj) => { + const formatDetailedSavedObjects = res.map((obj) => { const recentAccessItem = recentlyAccessedItems.find( (item) => item.id === obj.id ) as ChromeRecentlyAccessedHistoryItem; @@ -235,20 +242,14 @@ export const RecentItems = ({ const findWorkspace = workspaceList.find( (workspace) => workspace.id === recentAccessItem.workspaceId ); - // If the workspace id is existing but the workspace is deleted, filter the item - if (recentAccessItem.workspaceId && !findWorkspace) { - return []; - } - return [ - { - ...recentAccessItem, - ...obj, - ...recentAccessItem.meta, - updatedAt: moment(obj?.updated_at).valueOf(), - workspaceName: findWorkspace?.name, - }, - ]; + return { + ...recentAccessItem, + ...obj, + ...recentAccessItem.meta, + updatedAt: moment(obj?.updated_at).valueOf(), + workspaceName: findWorkspace?.name, + }; }); setDetailedSavedObjects(formatDetailedSavedObjects); });