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.

Sign up for GitHub

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

[Multiple Datasource] Pass data source menu to top nav via function instead of using component #6594

Merged
merged 2 commits into from
Apr 22, 2024
Merged

[Multiple Datasource] Pass data source menu to top nav via function instead of using component #6594

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
84d682b
change top nav to use interface to render data source menu
BionIT Apr 22, 2024
72e12c5
change value
BionIT Apr 22, 2024
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
28 changes: 15 additions & 13 deletions ...ata_source_management/public/components/data_source_menu/create_data_source_menu.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import React from 'react';
import { act, render } from '@testing-library/react';
import { DataSourceComponentType, DataSourceSelectableConfig } from './types';
import { ReactWrapper } from 'enzyme';
import { mockDataSourcePluginSetupWithShowLocalCluster } from '../../mocks';
import * as utils from '../utils';

describe('create data source menu', () => {
Expand Down Expand Up @@ -44,10 +43,11 @@ describe('create data source menu', () => {
notifications,
},
};
const TestComponent = createDataSourceMenu<DataSourceSelectableConfig>(
uiSettings,
mockDataSourcePluginSetupWithShowLocalCluster
);

spyOn(utils, 'getApplication').and.returnValue({ id: 'test2' });
spyOn(utils, 'getUiSettings').and.returnValue(uiSettings);
spyOn(utils, 'getHideLocalCluster').and.returnValue({ enabled: true });
const TestComponent = createDataSourceMenu<DataSourceSelectableConfig>();

const component = render(<TestComponent {...props} />);
expect(component).toMatchSnapshot();
Expand All @@ -71,10 +71,10 @@ describe('create data source menu', () => {
notifications,
},
};
const TestComponent = createDataSourceMenu<DataSourceSelectableConfig>(
uiSettings,
mockDataSourcePluginSetupWithShowLocalCluster
);
spyOn(utils, 'getApplication').and.returnValue({ id: 'test2' });
spyOn(utils, 'getUiSettings').and.returnValue(uiSettings);
spyOn(utils, 'getHideLocalCluster').and.returnValue({ enabled: true });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we set this to enabled: false based on the test case description?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zhongnansu This test itself needs to be fixed since it doesn't change disregards of local cluster is set since it doesn't check the options from popover. We can cut another meta issue for all the tests that we are missing testing, or do you think this needs to block the change?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BionIT agreed that we should fix the text, this is non blocking issue
I think in this PR, the test was modified to remove the check for local cluster existence
https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6358/files#diff-ff98db435bf1f4c1bfe7ea1869b5d25b7e731148ce6060333bf37b0dee33219fL78

const TestComponent = createDataSourceMenu<DataSourceSelectableConfig>();
await act(async () => {
component = render(<TestComponent {...props} />);
});
Expand Down Expand Up @@ -137,10 +137,12 @@ describe('when setMenuMountPoint is provided', () => {
notifications,
},
};
const TestComponent = createDataSourceMenu<DataSourceSelectableConfig>(
uiSettings,
mockDataSourcePluginSetupWithShowLocalCluster
);

spyOn(utils, 'getApplication').and.returnValue({ id: 'test2' });
spyOn(utils, 'getUiSettings').and.returnValue(uiSettings);
spyOn(utils, 'getHideLocalCluster').and.returnValue({ enabled: true });

const TestComponent = createDataSourceMenu<DataSourceSelectableConfig>();
const component = render(<TestComponent {...props} />);
act(() => {
mountPoint(portalTarget);
Expand Down
12 changes: 4 additions & 8 deletions ...ins/data_source_management/public/components/data_source_menu/create_data_source_menu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,16 @@

import React from 'react';
import { EuiHeaderLinks } from '@elastic/eui';
import { IUiSettingsClient } from 'src/core/public';
import { DataSourcePluginSetup } from 'src/plugins/data_source/public';
import { DataSourceMenu } from './data_source_menu';
import { DataSourceMenuProps } from './types';
import { MountPointPortal } from '../../../../opensearch_dashboards_react/public';
import { getApplication } from '../utils';
import { getApplication, getHideLocalCluster, getUiSettings } from '../utils';

export function createDataSourceMenu<T>(
uiSettings: IUiSettingsClient,
dataSourcePluginSetup: DataSourcePluginSetup
) {
export function createDataSourceMenu<T>() {
const application = getApplication();
const uiSettings = getUiSettings();
const hideLocalCluster = getHideLocalCluster().enabled;
return (props: DataSourceMenuProps<T>) => {
const { hideLocalCluster } = dataSourcePluginSetup;
if (props.setMenuMountPoint) {
return (
<MountPointPortal setMountPoint={props.setMenuMountPoint}>
Expand Down
1 change: 1 addition & 0 deletions src/plugins/data_source_management/public/components/data_source_menu/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/

export { DataSourceMenu } from './data_source_menu';
export { createDataSourceMenu } from './create_data_source_menu';
export {
DataSourceSelectableConfig,
DataSourceAggregatedViewConfig,
Expand Down
12 changes: 12 additions & 0 deletions src/plugins/data_source_management/public/components/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
IUiSettingsClient,
ToastsStart,
ApplicationStart,
CoreStart,
} from 'src/core/public';
import { deepFreeze } from '@osd/std';
import {
Expand Down Expand Up @@ -307,3 +308,14 @@ export const dataSourceOptionGroupLabel = deepFreeze<Readonly<DataSourceOptionGr
});

export const [getApplication, setApplication] = createGetterSetter<ApplicationStart>('Application');
export const [getUiSettings, setUiSettings] = createGetterSetter<CoreStart['uiSettings']>(
'UiSettings'
);

export interface HideLocalCluster {
enabled: boolean;
}

export const [getHideLocalCluster, setHideLocalCluster] = createGetterSetter<HideLocalCluster>(
'HideLocalCluster'
);
1 change: 1 addition & 0 deletions src/plugins/data_source_management/public/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,5 @@ export {
DataSourceViewConfig,
DataSourceMenuProps,
DataSourceMultiSelectableConfig,
createDataSourceMenu,
} from './components/data_source_menu';
7 changes: 5 additions & 2 deletions src/plugins/data_source_management/public/plugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { noAuthCredentialAuthMethod, sigV4AuthMethod, usernamePasswordAuthMethod
import { DataSourceSelectorProps } from './components/data_source_selector/data_source_selector';
import { createDataSourceMenu } from './components/data_source_menu/create_data_source_menu';
import { DataSourceMenuProps } from './components/data_source_menu';
import { setApplication } from './components/utils';
import { setApplication, setHideLocalCluster, setUiSettings } from './components/utils';

export interface DataSourceManagementSetupDependencies {
management: ManagementSetup;
Expand Down Expand Up @@ -101,11 +101,14 @@ export class DataSourceManagementPlugin
registerAuthenticationMethod(sigV4AuthMethod);
}

setHideLocalCluster({ enabled: dataSource.hideLocalCluster });
setUiSettings(uiSettings);

return {
registerAuthenticationMethod,
ui: {
DataSourceSelector: createDataSourceSelector(uiSettings, dataSource),
getDataSourceMenu: <T>() => createDataSourceMenu<T>(uiSettings, dataSource),
getDataSourceMenu: <T>() => createDataSourceMenu<T>(),
},
};
}
Expand Down
77 changes: 77 additions & 0 deletions src/plugins/navigation/public/top_nav_menu/__snapshots__/top_nav_menu.test.tsx.snap
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 10 additions & 2 deletions src/plugins/navigation/public/top_nav_menu/top_nav_menu.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import { MountPoint } from 'opensearch-dashboards/public';
import { TopNavMenu } from './top_nav_menu';
import { TopNavMenuData } from './top_nav_menu_data';
import { shallowWithIntl, mountWithIntl } from 'test_utils/enzyme_helpers';
import * as testUtils from '../../../data_source_management/public/components/utils';

const dataShim = {
ui: {
Expand Down Expand Up @@ -118,6 +119,9 @@ describe('TopNavMenu', () => {
});

it('mounts the data source menu if showDataSourceMenu is true', async () => {
spyOn(testUtils, 'getApplication').and.returnValue({ id: 'test2' });
spyOn(testUtils, 'getUiSettings').and.returnValue({ id: 'test2' });
spyOn(testUtils, 'getHideLocalCluster').and.returnValue(true);
const component = shallowWithIntl(
<TopNavMenu
appName={'test'}
Expand All @@ -133,10 +137,14 @@ describe('TopNavMenu', () => {
/>
);

expect(component.find('DataSourceMenu').length).toBe(1);
expect(component).toMatchSnapshot();
});

it('mounts the data source menu as well as top nav menu', async () => {
spyOn(testUtils, 'getApplication').and.returnValue({ id: 'test2' });
spyOn(testUtils, 'getUiSettings').and.returnValue({ id: 'test2' });
spyOn(testUtils, 'getHideLocalCluster').and.returnValue(true);

const component = shallowWithIntl(
<TopNavMenu
appName={'test'}
Expand All @@ -153,7 +161,7 @@ describe('TopNavMenu', () => {
/>
);

expect(component.find('DataSourceMenu').length).toBe(1);
expect(component).toMatchSnapshot();
expect(component.find(TOP_NAV_ITEM_SELECTOR).length).toBe(menuItems.length);
});

Expand Down
10 changes: 8 additions & 2 deletions src/plugins/navigation/public/top_nav_menu/top_nav_menu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import {
} from '../../../data/public';
import { TopNavMenuData } from './top_nav_menu_data';
import { TopNavMenuItem } from './top_nav_menu_item';
import { DataSourceMenu, DataSourceMenuProps } from '../../../data_source_management/public';
import { DataSourceMenuProps, createDataSourceMenu } from '../../../data_source_management/public';

export type TopNavMenuProps = StatefulSearchBarProps &
Omit<SearchBarProps, 'opensearchDashboards' | 'intl' | 'timeHistory'> & {
Expand Down Expand Up @@ -115,11 +115,17 @@ export function TopNavMenu(props: TopNavMenuProps): ReactElement | null {
return (
<EuiHeaderLinks data-test-subj="top-nav" gutterSize="xs" className={className}>
{renderItems()}
{showDataSourceMenu && <DataSourceMenu {...dataSourceMenuConfig!} />}
{renderDataSourceMenu()}
</EuiHeaderLinks>
);
}

function renderDataSourceMenu(): ReactElement | null {
if (!showDataSourceMenu) return null;
const DataSourceMenu = createDataSourceMenu();
return <DataSourceMenu {...dataSourceMenuConfig!} />;
}

function renderSearchBar(): ReactElement | null {
// Validate presence of all required fields
if (!showSearchBar || !props.data) return null;
Expand Down
Loading