Skip to content

Commit

Permalink
Merge branch 'main' into autosuggest-dql-bugs
Browse files Browse the repository at this point in the history
  • Loading branch information
paulstn authored Nov 27, 2024
2 parents db5a604 + 8f58bce commit de97f8f
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 39 deletions.
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))
2 changes: 2 additions & 0 deletions changelogs/fragments/8951.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
fix:
- SQL syntax highlighting double quotes ([#8951](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8951))
10 changes: 7 additions & 3 deletions packages/osd-monaco/src/xjson/lexer_rules/opensearchsql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,18 +134,22 @@ export const lexerRules = {
[new RegExp(operators.join('|')), 'operator'],
[/[0-9]+(\.[0-9]+)?/, 'number'],
[/'([^'\\]|\\.)*$/, 'string.invalid'], // non-terminated string
[/'/, 'string', '@string'],
[/"/, 'string', '@string'],
[/'/, 'string', '@stringSingle'],
[/"/, 'string', '@stringDouble'],
],
whitespace: [
[/[ \t\r\n]+/, 'white'],
[/\/\*/, 'comment', '@comment'],
[/--.*$/, 'comment'],
],
string: [
stringSingle: [
[/[^'\\]+/, 'string'],
[/\\./, 'string.escape'],
[/'/, 'string', '@pop'],
],
stringDouble: [
[/[^"\\]+/, 'string'],
[/\\./, 'string.escape'],
[/"/, 'string', '@pop'],
],
comment: [
Expand Down
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

0 comments on commit de97f8f

Please sign in to comment.