From a84b78468d63e7d8e3f17a6572fd56908ab89dd0 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 --- .../components/data_source_view_example.tsx | 37 +++-- .../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 | 156 ++++++++++++++++-- .../data_source_view.test.tsx | 102 +++++++++--- .../data_source_view/data_source_view.tsx | 83 +++++++--- .../public/components/utils.test.ts | 31 +++- .../public/components/utils.ts | 15 ++ .../data_source_management/public/mocks.ts | 11 ++ 10 files changed, 388 insertions(+), 66 deletions(-) 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/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..5dbb202b1a47 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 @@ -44,9 +44,11 @@ export const DataSourceComponentType = { export type DataSourceComponentType = typeof DataSourceComponentType[keyof typeof DataSourceComponentType]; export interface DataSourceViewConfig extends DataSourceBaseConfig { + onSelectedDataSources: (dataSources: DataSourceOption[]) => void; activeOption: DataSourceOption[]; savedObjects?: SavedObjectsClientContract; notifications?: NotificationsStart; + dataSourceFilter?: (dataSource: SavedObject) => boolean; } 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..9c6f554b2b81 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 succeed when provided datasource not as been filtered out 1`] = ` + > + test1 + `; -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 succeed when provided datasource not as 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..2eed218575f6 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,27 @@ 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 } from '../utils'; import { MenuPanelItem } from '../../types'; +export const LocalCluster: DataSourceOption = { + label: i18n.translate('dataSource.localCluster', { + defaultMessage: 'Local cluster', + }), + id: '', +}; + interface DataSourceViewProps { fullWidth: boolean; selectedOption: DataSourceOption[]; savedObjectsClient?: SavedObjectsClientContract; notifications?: ToastsStart; + uiSettings?: IUiSettingsClient; + hideLocalCluster: boolean; + dataSourceFilter?: (dataSource: any) => boolean; + onSelectedDataSources: (dataSources: DataSourceOption[]) => void; } interface DataSourceViewState { @@ -40,33 +52,56 @@ export class DataSourceView extends React.Component { }); }); + describe('Handle Fetch Data Source Error', () => { + const { toasts } = notificationServiceMock.createStartContract(); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + test('should add warning with error message when error is provided', () => { + const error = 'Failed to fetch data'; + handleDataSourceFetchError(toasts, error); + expect(toasts.addWarning).toHaveBeenCalledWith(`Failed to fetch data source due to ${error}`); + }); + + test('should add warning with default message when error is not provided', () => { + handleDataSourceFetchError(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..be71bd2cf02d 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,16 @@ export async function setFirstDataSourceAsDefault( } } +export function handleDataSourceFetchError(notifications: ToastsStart, error?: any) { + notifications.addWarning( + i18n.translate('dataSource.fetchDataSourceError', { + defaultMessage: error + ? `Failed to fetch data source due to ${error}` + : `Data source is not available`, + }) + ); +} + export function getFilteredDataSources( dataSources: Array>, filter = (ds: SavedObject) => true @@ -130,6 +142,9 @@ export async function getDataSourceById( savedObjectsClient: SavedObjectsClientContract ) { return savedObjectsClient.get('data-source', id).then((response) => { + if (!response || response.error) { + throw new Error('Unable to find data source'); + } const attributes: any = response?.attributes || {}; return { id: response.id, 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',