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

[Workspace] feat: optimize recent items and filter out items whose workspace is deleted #8900

Merged
merged 4 commits into from
Nov 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelogs/fragments/8900.yml
Original file line number Diff line number Diff line change
@@ -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))
33 changes: 23 additions & 10 deletions src/core/public/chrome/ui/header/recent_items.test.tsx
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();
});
});
68 changes: 42 additions & 26 deletions src/core/public/chrome/ui/header/recent_items.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,9 @@ export const RecentItems = ({
setIsPreferencesPopoverOpen((IsPreferencesPopoverOpe) => !IsPreferencesPopoverOpe);
}}
>
Preferences
{i18n.translate('core.header.recent.preferences', {
defaultMessage: 'Preferences',
})}
</EuiButtonEmpty>
}
isOpen={isPreferencesPopoverOpen}
Expand All @@ -152,7 +154,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 +168,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 @@ -208,15 +220,20 @@ 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 filteredNavLinks = navLinks.filter((link) => !link.hidden);
const formatDetailedSavedObjects = res.map((obj) => {
const recentAccessItem = recentlyAccessedItems.find(
(item) => item.id === obj.id
Expand All @@ -225,33 +242,21 @@ 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,
};
});
// 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,
]);
}, [recentlyAccessedItems, http, workspaceList]);

const selectedRecentsItems = useMemo(() => {
const selectedRecentItems = useMemo(() => {
return detailedSavedObjects.slice(0, Number(recentsRadioIdSelected));
}, [detailedSavedObjects, recentsRadioIdSelected]);

Expand Down Expand Up @@ -283,11 +288,20 @@ 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)}
onClick={() =>
handleItemClick(
createRecentNavLink(
item,
navLinks.filter((link) => !link.hidden),
basePath,
navigateToUrl
).href
)
}
key={item.link}
style={{ padding: '1px' }}
label={
Expand All @@ -309,7 +323,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
Loading