Skip to content

Commit

Permalink
feat: optimize recent items and filter out items whose workspace is d…
Browse files Browse the repository at this point in the history 
…eleted

Signed-off-by: tygao <tygao@amazon.com>
  • Loading branch information
@raintygao
raintygao committed Nov 20, 2024
1 parent a107509 commit cdc3160
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 43 deletions.
33 changes: 23 additions & 10 deletions src/core/public/chrome/ui/header/recent_items.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ jest.mock('./nav_link', () => ({
}),
}));

const mockRecentlyAccessed = new BehaviorSubject([
const mockRecentlyAccessed$ = new BehaviorSubject([
{
id: '6ef856c0-5f86-11ef-b7df-1bb1cf26ce5b',
label: 'visualizeMock',
Expand All @@ -28,7 +28,7 @@ const mockRecentlyAccessed = new BehaviorSubject([
},
]);

const mockWorkspaceList = new BehaviorSubject([
const mockWorkspaceList$ = new BehaviorSubject([
{
id: 'workspace_1',
name: 'WorkspaceMock_1',
Expand All @@ -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: <></>,
Expand Down Expand Up @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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();
Expand All @@ -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 () => {
Expand All @@ -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(<RecentItems {...defaultMockProps} recentlyAccessed$={mockRecentlyAccessed$} />);
expect(screen.queryByText('visualizeMock')).not.toBeInTheDocument();
});
});
81 changes: 48 additions & 33 deletions src/core/public/chrome/ui/header/recent_items.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -122,6 +122,7 @@ export const RecentItems = ({
[]
);
const navLinks = useObservable(navLinks$, []);
const prevNavLinksRef = useRef<ChromeNavLink[]>();
const loadingCount = useObservable(loadingCount$, 0);

const handleItemClick = (link: string) => {
Expand All @@ -143,7 +144,9 @@ export const RecentItems = ({
setIsPreferencesPopoverOpen((IsPreferencesPopoverOpe) => !IsPreferencesPopoverOpe);
}}
>
Preferences
{i18n.translate('core.header.recent.preferences', {
defaultMessage: 'Preferences',
})}
</EuiButtonEmpty>
}
isOpen={isPreferencesPopoverOpen}
Expand All @@ -152,7 +155,11 @@ export const RecentItems = ({
setIsPreferencesPopoverOpen(false);
}}
>
<EuiPopoverTitle>Preferences</EuiPopoverTitle>
<EuiPopoverTitle>
{i18n.translate('core.header.recent.preferences.title', {
defaultMessage: 'Preferences',
})}
</EuiPopoverTitle>
<EuiRadioGroup
options={recentsRadios}
idSelected={recentsRadioIdSelected}
Expand All @@ -162,7 +169,13 @@ export const RecentItems = ({
}}
name="radio group"
legend={{
children: <span>Recents</span>,
children: (
<span>
{i18n.translate('core.header.recent.preferences.legend', {
defaultMessage: 'Recents',
})}
</span>
),
}}
/>
</EuiPopover>
Expand Down Expand Up @@ -213,45 +226,45 @@ 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;

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]);

Expand Down Expand Up @@ -283,9 +296,9 @@ export const RecentItems = ({
</h4>
</EuiTitle>
<EuiSpacer size="s" />
{selectedRecentsItems.length > 0 ? (
{selectedRecentItems.length > 0 ? (
<EuiListGroup flush={true} gutterSize="s">
{selectedRecentsItems.map((item) => (
{selectedRecentItems.map((item) => (
<EuiListGroupItem
onClick={() => handleItemClick(item.link)}
key={item.link}
Expand All @@ -309,7 +322,9 @@ export const RecentItems = ({
</EuiListGroup>
) : (
<EuiText color="subdued" size="s">
No recently viewed items
{i18n.translate('core.header.recent.no.recents', {
defaultMessage: 'No recently viewed items',
})}
</EuiText>
)}
<EuiSpacer size="s" />
Expand Down

0 comments on commit cdc3160

Please sign in to comment.