From fcf50ddbfe5d71e54f78b5889eaa69526d4c5524 Mon Sep 17 00:00:00 2001 From: "Yuanqi(Ella) Zhu" Date: Thu, 11 Apr 2024 19:03:00 +0000 Subject: [PATCH] Refactor read-only component to cover more edge cases Signed-off-by: Yuanqi(Ella) Zhu --- CHANGELOG.md | 1 + .../components/data_source_view_example.tsx | 37 +++-- .../public/components/constants.tsx | 14 ++ .../data_source_menu.test.tsx.snap | 4 + .../data_source_menu/data_source_menu.tsx | 13 +- .../components/data_source_menu/types.ts | 2 + .../data_source_view.test.tsx.snap | 150 +++++++++++++++++- .../data_source_view.test.tsx | 102 +++++++++--- .../data_source_view/data_source_view.tsx | 77 ++++++--- .../public/components/utils.test.ts | 31 +++- .../public/components/utils.ts | 30 +++- .../data_source_management/public/mocks.ts | 11 ++ 12 files changed, 410 insertions(+), 62 deletions(-) create mode 100644 src/plugins/data_source_management/public/components/constants.tsx diff --git a/CHANGELOG.md b/CHANGELOG.md index 17d4ec919e3d..13aab3a37645 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -114,6 +114,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Multiple Datasource] Fix sslConfig for multiple datasource to handle when certificateAuthorities is unset ([#6282](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6282)) - [BUG][Multiple Datasource]Fix bug in data source aggregated view to change it to depend on displayAllCompatibleDataSources property to show the badge value ([#6291](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6291)) - [BUG][Multiple Datasource]Read hideLocalCluster setting from yml and set in data source selector and data source menu ([#6361](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6361)) +- [BUG][Multiple Datasource] Refactor read-only component to cover more edge cases ([#6416](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6416)) - [BUG] Fix for checkForFunctionProperty so that order does not matter ([#6248](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6248)) - [BUG][Multiple Datasource] Validation succeed as long as status code in response is 200 ([#6399](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6399)) diff --git a/examples/multiple_data_source_examples/public/components/data_source_view_example.tsx b/examples/multiple_data_source_examples/public/components/data_source_view_example.tsx index 11ebe9ee343a..b80a65bb1b1a 100644 --- a/examples/multiple_data_source_examples/public/components/data_source_view_example.tsx +++ b/examples/multiple_data_source_examples/public/components/data_source_view_example.tsx @@ -2,7 +2,7 @@ * Copyright OpenSearch Contributors * SPDX-License-Identifier: Apache-2.0 */ -import React from 'react'; +import React, { useState, useMemo } from 'react'; import { EuiBasicTable, EuiPageBody, @@ -34,8 +34,11 @@ export const DataSourceViewExample = ({ dataSourceEnabled, setActionMenu, dataSourceManagement, + notifications, + savedObjects, }: DataSourceViewExampleProps) => { const DataSourceMenu = dataSourceManagement.ui.getDataSourceMenu(); + const [selectedDataSources, setSelectedDataSources] = useState([]); const data: ComponentProp[] = [ { name: 'savedObjects', @@ -68,19 +71,31 @@ export const DataSourceViewExample = ({ }, ]; + const renderDataSourceComponent = useMemo(() => { + return ( + { + return true; + }, + onSelectedDataSources: (ds) => { + setSelectedDataSources(ds); + }, + }} + /> + ); + }, [setActionMenu, notifications, savedObjects]); + return ( - {dataSourceEnabled && ( - - )} + {dataSourceEnabled && renderDataSourceComponent}

Data Source View Example

diff --git a/src/plugins/data_source_management/public/components/constants.tsx b/src/plugins/data_source_management/public/components/constants.tsx new file mode 100644 index 000000000000..0d22aed50179 --- /dev/null +++ b/src/plugins/data_source_management/public/components/constants.tsx @@ -0,0 +1,14 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { i18n } from '@osd/i18n'; +import { DataSourceOption } from './data_source_menu/types'; + +export const LocalCluster: DataSourceOption = { + label: i18n.translate('dataSource.localCluster', { + defaultMessage: 'Local cluster', + }), + id: '', +}; diff --git a/src/plugins/data_source_management/public/components/data_source_menu/__snapshots__/data_source_menu.test.tsx.snap b/src/plugins/data_source_management/public/components/data_source_menu/__snapshots__/data_source_menu.test.tsx.snap index 8f8ea1a0f55d..517f8de733ba 100644 --- a/src/plugins/data_source_management/public/components/data_source_menu/__snapshots__/data_source_menu.test.tsx.snap +++ b/src/plugins/data_source_management/public/components/data_source_menu/__snapshots__/data_source_menu.test.tsx.snap @@ -3,6 +3,7 @@ exports[`DataSourceMenu can render data source view when only pass id in the activeOption 1`] = ` (props: DataSourceMenuProps): ReactElement | const { componentType, componentConfig, uiSettings, hideLocalCluster } = props; function renderDataSourceView(config: DataSourceViewConfig): ReactElement | null { - const { activeOption, fullWidth, savedObjects, notifications } = config; + const { + activeOption, + fullWidth, + savedObjects, + notifications, + dataSourceFilter, + onSelectedDataSources, + } = config; return ( ); } diff --git a/src/plugins/data_source_management/public/components/data_source_menu/types.ts b/src/plugins/data_source_management/public/components/data_source_menu/types.ts index 17aa35d8b8d0..ca589b0ccb4b 100644 --- a/src/plugins/data_source_management/public/components/data_source_menu/types.ts +++ b/src/plugins/data_source_management/public/components/data_source_menu/types.ts @@ -47,6 +47,8 @@ export interface DataSourceViewConfig extends DataSourceBaseConfig { activeOption: DataSourceOption[]; savedObjects?: SavedObjectsClientContract; notifications?: NotificationsStart; + dataSourceFilter?: (dataSource: SavedObject) => boolean; + onSelectedDataSources?: (dataSources: DataSourceOption[]) => void; } export interface DataSourceAggregatedViewConfig extends DataSourceBaseConfig { diff --git a/src/plugins/data_source_management/public/components/data_source_view/__snapshots__/data_source_view.test.tsx.snap b/src/plugins/data_source_management/public/components/data_source_view/__snapshots__/data_source_view.test.tsx.snap index 183ebc563501..8f825f0b4e0f 100644 --- a/src/plugins/data_source_management/public/components/data_source_view/__snapshots__/data_source_view.test.tsx.snap +++ b/src/plugins/data_source_management/public/components/data_source_view/__snapshots__/data_source_view.test.tsx.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`DataSourceView should call getDataSourceById when only pass id no label 1`] = ` +exports[`DataSourceView Should render successfully when provided datasource has not been filtered out 1`] = ` `; -exports[`DataSourceView should call notification warning when there is data source fetch error 1`] = ` +exports[`DataSourceView Should return error when provided datasource has been filtered out 1`] = ` + + + + } + closePopover={[Function]} + display="inlineBlock" + hasArrow={true} + id="dataSourceViewContextMenuPopover" + isOpen={false} + ownFocus={true} + panelPaddingSize="none" + > + + + +`; + +exports[`DataSourceView When selected option is local cluster and hide local Cluster is true, should return error 1`] = ` + + + + } + closePopover={[Function]} + display="inlineBlock" + hasArrow={true} + id="dataSourceViewContextMenuPopover" + isOpen={false} + ownFocus={true} + panelPaddingSize="none" + > + + + +`; + +exports[`DataSourceView should call getDataSourceById when only pass id no label 1`] = ` `; +exports[`DataSourceView should call notification warning when there is data source fetch error 1`] = ` + + + + } + closePopover={[Function]} + display="inlineBlock" + hasArrow={true} + id="dataSourceViewContextMenuPopover" + isOpen={false} + ownFocus={true} + panelPaddingSize="none" + > + + + +`; + exports[`DataSourceView should render normally with local cluster not hidden 1`] = ` - test1 - + />
- test1 - + />
{ let component: ShallowWrapper, React.Component<{}, {}, any>>; @@ -21,6 +18,7 @@ describe('DataSourceView', () => { const { toasts } = notificationServiceMock.createStartContract(); beforeEach(() => { + jest.clearAllMocks(); client = { get: jest.fn().mockResolvedValue([]), } as any; @@ -28,48 +26,114 @@ describe('DataSourceView', () => { }); it('should render normally with local cluster not hidden', () => { + spyOn(utils, 'getDataSourceById').and.returnValue([{ id: 'test1', label: 'test1' }]); component = shallow( - + ); expect(component).toMatchSnapshot(); expect(toasts.addWarning).toBeCalledTimes(0); }); - it('should show popover when click on button', async () => { - const container = render( - + it('When selected option is local cluster and hide local Cluster is true, should return error', () => { + component = shallow( + ); - const button = await container.findByTestId('dataSourceViewContextMenuHeaderLink'); - button.click(); - expect(container).toMatchSnapshot(); + expect(component).toMatchSnapshot(); + expect(toasts.addWarning).toBeCalledTimes(1); + }); + it('Should return error when provided datasource has been filtered out', async () => { + component = shallow( + { + return false; + }} + /> + ); + expect(component).toMatchSnapshot(); + expect(toasts.addWarning).toBeCalledTimes(1); + }); + it('Should render successfully when provided datasource has not been filtered out', async () => { + spyOn(utils, 'getDataSourceById').and.returnValue([{ id: 'test1', label: 'test1' }]); + component = shallow( + { + return true; + }} + /> + ); + expect(component).toMatchSnapshot(); + expect(toasts.addWarning).toBeCalledTimes(0); + expect(utils.getDataSourceById).toBeCalledTimes(1); }); it('should call getDataSourceById when only pass id no label', async () => { + spyOn(utils, 'getDataSourceById').and.returnValue([{ id: 'test1', label: 'test1' }]); component = shallow( ); expect(component).toMatchSnapshot(); - expect(client.get).toBeCalledWith('data-source', 'test1'); + expect(utils.getDataSourceById).toBeCalledTimes(1); expect(toasts.addWarning).toBeCalledTimes(0); }); it('should call notification warning when there is data source fetch error', async () => { - jest.mock('../utils', () => ({ - getDataSourceById: jest.fn(), - })); + spyOn(utils, 'getDataSourceById').and.throwError('Data source is not available'); component = shallow( ); expect(component).toMatchSnapshot(); - mockErrorResponseForSavedObjectsCalls(client, 'get'); expect(toasts.addWarning).toBeCalledTimes(1); - expect(toasts.addWarning).toBeCalledWith(`Data source with id test1 is not available`); + expect(utils.getDataSourceById).toBeCalledTimes(1); + }); + + it('should show popover when click on button', async () => { + const onSelectedDataSource = jest.fn(); + spyOn(utils, 'getDataSourceById').and.returnValue([{ id: 'test1', label: 'test1' }]); + spyOn(utils, 'handleDataSourceFetchError').and.returnValue(''); + const container = render( + + ); + const button = await container.findByTestId('dataSourceViewContextMenuHeaderLink'); + button.click(); + expect(container).toMatchSnapshot(); }); }); diff --git a/src/plugins/data_source_management/public/components/data_source_view/data_source_view.tsx b/src/plugins/data_source_management/public/components/data_source_view/data_source_view.tsx index 005d54e216c6..ab48e925d18f 100644 --- a/src/plugins/data_source_management/public/components/data_source_view/data_source_view.tsx +++ b/src/plugins/data_source_management/public/components/data_source_view/data_source_view.tsx @@ -7,15 +7,25 @@ import React from 'react'; import { i18n } from '@osd/i18n'; import { EuiPopover, EuiButtonEmpty, EuiButtonIcon, EuiContextMenu } from '@elastic/eui'; import { SavedObjectsClientContract, ToastsStart } from 'opensearch-dashboards/public'; +import { IUiSettingsClient } from 'src/core/public'; import { DataSourceOption } from '../data_source_menu/types'; -import { getDataSourceById } from '../utils'; +import { + getDataSourceById, + handleDataSourceFetchError, + handleNoAvailableDataSourceError, +} from '../utils'; import { MenuPanelItem } from '../../types'; +import { LocalCluster } from '../constants'; interface DataSourceViewProps { fullWidth: boolean; selectedOption: DataSourceOption[]; + hideLocalCluster: boolean; savedObjectsClient?: SavedObjectsClientContract; notifications?: ToastsStart; + uiSettings?: IUiSettingsClient; + dataSourceFilter?: (dataSource: any) => boolean; + onSelectedDataSources?: (dataSources: DataSourceOption[]) => void; } interface DataSourceViewState { @@ -40,33 +50,60 @@ export class DataSourceView extends React.Component { }); }); + describe('Handle fetch data source Error', () => { + const { toasts } = notificationServiceMock.createStartContract(); + + test('should send warning when data source fetch failed', () => { + handleDataSourceFetchError(toasts); + expect(toasts.addWarning).toHaveBeenCalledWith(`Failed to fetch data source`); + }); + }); + + describe('Handle no available data source error', () => { + const { toasts } = notificationServiceMock.createStartContract(); + + test('should send warning when data source is not available', () => { + handleNoAvailableDataSourceError(toasts); + expect(toasts.addWarning).toHaveBeenCalledWith(`Data source is not available`); + }); + }); + describe('Get data source by ID', () => { test('Success: getting data source by ID with credential', async () => { mockResponseForSavedObjectsCalls(savedObjects.client, 'get', getDataSourceByIdWithCredential); @@ -99,6 +120,14 @@ describe('DataSourceManagement: Utils.ts', () => { expect(e).toBeTruthy(); } }); + test('failure: gets error when response contains error', async () => { + try { + mockResponseForSavedObjectsCalls(savedObjects.client, 'get', getDataSourceByIdWithError); + await getDataSourceById('alpha-test', savedObjects.client); + } catch (e) { + expect(e).toBeTruthy(); + } + }); }); describe('Create data source', () => { diff --git a/src/plugins/data_source_management/public/components/utils.ts b/src/plugins/data_source_management/public/components/utils.ts index b8c76bcf858c..d71b7bcdef9d 100644 --- a/src/plugins/data_source_management/public/components/utils.ts +++ b/src/plugins/data_source_management/public/components/utils.ts @@ -8,7 +8,9 @@ import { SavedObjectsClientContract, SavedObject, IUiSettingsClient, + ToastsStart, } from 'src/core/public'; +import { i18n } from '@osd/i18n'; import { DataSourceAttributes, DataSourceTableItem, @@ -79,6 +81,22 @@ export async function setFirstDataSourceAsDefault( } } +export function handleDataSourceFetchError(notifications: ToastsStart) { + notifications.addWarning( + i18n.translate('dataSource.fetchDataSourceError', { + defaultMessage: `Failed to fetch data source`, + }) + ); +} + +export function handleNoAvailableDataSourceError(notifications: ToastsStart) { + notifications.addWarning( + i18n.translate('dataSource.noAvailableDataSourceError', { + defaultMessage: `Data source is not available`, + }) + ); +} + export function getFilteredDataSources( dataSources: Array>, filter = (ds: SavedObject) => true @@ -129,7 +147,13 @@ export async function getDataSourceById( id: string, savedObjectsClient: SavedObjectsClientContract ) { - return savedObjectsClient.get('data-source', id).then((response) => { + try { + const response = await savedObjectsClient.get('data-source', id); + + if (!response || response.error) { + throw new Error('Unable to find data source'); + } + const attributes: any = response?.attributes || {}; return { id: response.id, @@ -138,7 +162,9 @@ export async function getDataSourceById( description: attributes.description || '', auth: attributes.auth, }; - }); + } catch (error) { + throw new Error('Failed to fetch data source: ' + error.message); + } } export async function createSingleDataSource( diff --git a/src/plugins/data_source_management/public/mocks.ts b/src/plugins/data_source_management/public/mocks.ts index d6c89dc5c7dd..3d991bd3e10a 100644 --- a/src/plugins/data_source_management/public/mocks.ts +++ b/src/plugins/data_source_management/public/mocks.ts @@ -315,6 +315,17 @@ export const getDataSourceByIdWithoutCredential = { references: [], }; +export const getDataSourceByIdWithError = { + attributes: { + ...getDataSourceByIdWithCredential.attributes, + Error: { + statusCode: 404, + errorMessage: 'Unable to find data source', + }, + }, + references: [], +}; + export const mockResponseForSavedObjectsCalls = ( savedObjectsClient: SavedObjectsClientContract, savedObjectsMethodName: 'get' | 'find' | 'create' | 'delete' | 'update',