diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a1089042e7c..966d0f18e2b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -85,6 +85,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Multiple Datasource] Add default icon in multi-selectable picker ([#6357](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6357)) - [Workspace] Add APIs to support plugin state in request ([#6303](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6303)) - [Workspace] Filter left nav menu items according to the current workspace ([#6234](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6234)) +- [Multiple Datasource] Refactor data source selector component to include placeholder and add tests ([#6372](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6372)) ### 🐛 Bug Fixes diff --git a/src/plugins/data_source_management/public/components/data_source_selector/__snapshots__/data_source_selector.test.tsx.snap b/src/plugins/data_source_management/public/components/data_source_selector/__snapshots__/data_source_selector.test.tsx.snap index 5cb8738e5c17..4f65f5d38085 100644 --- a/src/plugins/data_source_management/public/components/data_source_selector/__snapshots__/data_source_selector.test.tsx.snap +++ b/src/plugins/data_source_management/public/components/data_source_selector/__snapshots__/data_source_selector.test.tsx.snap @@ -80,15 +80,15 @@ exports[`DataSourceSelector: check dataSource options should always place local }, Object { "id": "test1", - "label": "", + "label": "test1", }, Object { "id": "test2", - "label": "", + "label": "test2", }, Object { "id": "test3", - "label": "", + "label": "test3", }, ] } @@ -130,11 +130,11 @@ exports[`DataSourceSelector: check dataSource options should filter options if c }, Object { "id": "test2", - "label": "", + "label": "test2", }, Object { "id": "test3", - "label": "", + "label": "test3", }, ] } @@ -174,6 +174,18 @@ exports[`DataSourceSelector: check dataSource options should get default datasou "id": "", "label": "Local cluster", }, + Object { + "id": "test1", + "label": "test1", + }, + Object { + "id": "test2", + "label": "test2", + }, + Object { + "id": "test3", + "label": "test3", + }, ] } placeholder="Select a data source" @@ -182,8 +194,8 @@ exports[`DataSourceSelector: check dataSource options should get default datasou selectedOptions={ Array [ Object { - "id": "", - "label": "Local cluster", + "id": "test1", + "label": "test1", }, ] } @@ -214,15 +226,15 @@ exports[`DataSourceSelector: check dataSource options should hide prepend if rem }, Object { "id": "test1", - "label": "", + "label": "test1", }, Object { "id": "test2", - "label": "", + "label": "test2", }, Object { "id": "test3", - "label": "", + "label": "test3", }, ] } @@ -325,15 +337,15 @@ exports[`DataSourceSelector: check dataSource options should show custom placeho }, Object { "id": "test1", - "label": "", + "label": "test1", }, Object { "id": "test2", - "label": "", + "label": "test2", }, Object { "id": "test3", - "label": "", + "label": "test3", }, ] } diff --git a/src/plugins/data_source_management/public/components/data_source_selector/data_source_selector.test.tsx b/src/plugins/data_source_management/public/components/data_source_selector/data_source_selector.test.tsx index d1203584d4b5..af5086f35a50 100644 --- a/src/plugins/data_source_management/public/components/data_source_selector/data_source_selector.test.tsx +++ b/src/plugins/data_source_management/public/components/data_source_selector/data_source_selector.test.tsx @@ -4,7 +4,7 @@ */ import { ShallowWrapper, shallow } from 'enzyme'; -import { DataSourceSelector } from './data_source_selector'; +import { DataSourceSelector, LocalCluster } from './data_source_selector'; import { SavedObjectsClientContract } from '../../../../../core/public'; import { notificationServiceMock } from '../../../../../core/public/mocks'; import React from 'react'; @@ -15,6 +15,7 @@ import { } from '../../mocks'; import { AuthType } from 'src/plugins/data_source/common/data_sources'; import * as utils from '../utils'; +import { EuiComboBox } from '@elastic/eui'; describe('DataSourceSelector', () => { let component: ShallowWrapper, React.Component<{}, {}, any>>; @@ -23,6 +24,7 @@ describe('DataSourceSelector', () => { const { toasts } = notificationServiceMock.createStartContract(); beforeEach(() => { + jest.clearAllMocks(); client = { find: jest.fn().mockResolvedValue([]), } as any; @@ -181,8 +183,6 @@ describe('DataSourceSelector: check dataSource options', () => { it('should get default datasource if uiSettings exists', async () => { spyOn(uiSettings, 'get').and.returnValue('test1'); - spyOn(utils, 'getFilteredDataSources').and.returnValue([]); - spyOn(utils, 'getDefaultDataSource').and.returnValue([]); component = shallow( { await nextTick(); expect(component).toMatchSnapshot(); expect(uiSettings.get).toBeCalledWith('defaultDataSource', null); - expect(utils.getFilteredDataSources).toHaveBeenCalled(); - expect(utils.getDefaultDataSource).toHaveBeenCalled(); - expect(toasts.addWarning).toHaveBeenCalled(); }); it('should not render options with default badge when id does not matches defaultDataSource', () => { @@ -220,3 +217,302 @@ describe('DataSourceSelector: check dataSource options', () => { expect(component.find('EuiComboBox').exists()).toBe(true); }); }); + +describe('DataSourceSelector: check defaultOption behavior', () => { + /** + * Test Cases + * - []: 2 cases + * - Some value: 4 * 3 = 12 cases + */ + let component: ShallowWrapper, React.Component<{}, {}, any>>; + let client: SavedObjectsClientContract; + const { toasts } = notificationServiceMock.createStartContract(); + const nextTick = () => new Promise((res) => process.nextTick(res)); + const mockedContext = mockManagementPlugin.createDataSourceManagementContext(); + const uiSettings = mockedContext.uiSettings; + const getMockedDataSourceOptions = () => { + return getDataSourcesWithFieldsResponse.savedObjects.map((response) => { + return { id: response.id, label: response.attributes.title }; + }); + }; + + beforeEach(async () => { + jest.clearAllMocks(); + client = { + find: jest.fn().mockResolvedValue([]), + } as any; + mockResponseForSavedObjectsCalls(client, 'find', getDataSourcesWithFieldsResponse); + }); + + // When defaultOption is undefined + it('should render defaultDataSource as the selected option', async () => { + spyOn(uiSettings, 'get').and.returnValue('test1'); + component = shallow( + + ); + component.instance().componentDidMount!(); + await nextTick(); + const euiComboBox = component.find(EuiComboBox); + expect(euiComboBox.prop('selectedOptions')).toEqual( + expect.arrayContaining([ + { + id: 'test1', + label: 'test1', + }, + ]) + ); + }); + + it('should render Local Cluster as the selected option when hideLocalCluster is false', async () => { + spyOn(uiSettings, 'get').and.returnValue(null); + component = shallow( + + ); + component.instance().componentDidMount!(); + await nextTick(); + const euiComboBox = component.find(EuiComboBox); + expect(euiComboBox.prop('selectedOptions')).toEqual(expect.arrayContaining([LocalCluster])); + }); + + it('should render random datasource as the selected option if defaultDataSource and Local Cluster are not present', async () => { + spyOn(uiSettings, 'get').and.returnValue(null); + component = shallow( + { + return dataSource.id !== 'test1'; + }} + /> + ); + component.instance().componentDidMount!(); + await nextTick(); + const euiComboBox = component.find(EuiComboBox); + expect(euiComboBox.prop('selectedOptions')).toEqual( + expect.arrayContaining([ + { + id: 'test2', + label: 'test2', + }, + ]) + ); + }); + + it('should return toast', async () => { + spyOn(uiSettings, 'get').and.returnValue(null); + component = shallow( + { + return false; + }} + /> + ); + component.instance().componentDidMount!(); + await nextTick(); + const euiComboBox = component.find(EuiComboBox); + expect(euiComboBox.prop('selectedOptions')).toEqual(expect.arrayContaining([])); + expect(toasts.addWarning).toBeCalled(); + }); + + // When defaultOption is [] + it('should render placeholder and all options when Local Cluster is not hidden', async () => { + spyOn(uiSettings, 'get').and.returnValue('test1'); + component = shallow( + + ); + component.instance().componentDidMount!(); + await nextTick(); + const euiComboBox = component.find(EuiComboBox); + expect(euiComboBox.prop('selectedOptions')).toEqual(expect.arrayContaining([])); + expect(euiComboBox.prop('options')).toEqual( + expect.arrayContaining(getMockedDataSourceOptions().concat([LocalCluster])) + ); + }); + + it('should render placeholder and all options when Local Cluster is hidden', async () => { + spyOn(uiSettings, 'get').and.returnValue('test1'); + component = shallow( + + ); + component.instance().componentDidMount!(); + await nextTick(); + const euiComboBox = component.find(EuiComboBox); + expect(euiComboBox.prop('selectedOptions')).toEqual(expect.arrayContaining([])); + expect(euiComboBox.prop('options')).toEqual( + expect.arrayContaining(getMockedDataSourceOptions()) + ); + }); + + // When defaultOption is [{id}] + it.each([ + { + id: undefined, + }, + { + id: '', + }, + { + id: 'test2', + }, + { + id: 'non-existent-id', + }, + ])('should all throw a toast warning when the available dataSources is empty', async ({ id }) => { + spyOn(uiSettings, 'get').and.returnValue('test1'); + component = shallow( + { + return false; + }} + // @ts-expect-error + defaultOption={[{ id }]} + /> + ); + component.instance().componentDidMount!(); + await nextTick(); + const euiComboBox = component.find(EuiComboBox); + expect(euiComboBox.prop('selectedOptions')).toEqual(expect.arrayContaining([])); + expect(toasts.addWarning).toBeCalled(); + }); + + it.each([ + { + id: undefined, + }, + { + id: '', + }, + { + id: 'test2', + }, + { + id: 'non-existent-id', + }, + ])('should all throw a toast warning when the id is filtered out', async ({ id }) => { + spyOn(uiSettings, 'get').and.returnValue('test1'); + component = shallow( + { + return dataSource.attributes.title !== id; + }} + // @ts-expect-error + defaultOption={[{ id }]} + /> + ); + component.instance().componentDidMount!(); + await nextTick(); + const euiComboBox = component.find(EuiComboBox); + expect(euiComboBox.prop('selectedOptions')).toEqual(expect.arrayContaining([])); + expect(toasts.addWarning).toBeCalled(); + }); + + it.each([ + { + id: undefined, + error: true, + selectedOption: [], + }, + { + id: '', + error: false, + selectedOption: [LocalCluster], + }, + { + id: 'test2', + error: false, + selectedOption: [{ id: 'test2', label: 'test2' }], + }, + { + id: 'non-existent-id', + error: true, + selectedOption: [], + }, + ])( + 'should handle selectedOption correctly when defaultOption = [{id}]', + async ({ id, error, selectedOption }) => { + spyOn(uiSettings, 'get').and.returnValue('test1'); + component = shallow( + + ); + component.instance().componentDidMount!(); + await nextTick(); + const euiComboBox = component.find(EuiComboBox); + expect(euiComboBox.prop('selectedOptions')).toEqual(expect.arrayContaining(selectedOption)); + if (error) { + expect(toasts.addWarning).toBeCalled(); + } else { + expect(toasts.addWarning).toBeCalledTimes(0); + } + } + ); +}); diff --git a/src/plugins/data_source_management/public/components/data_source_selector/data_source_selector.tsx b/src/plugins/data_source_management/public/components/data_source_selector/data_source_selector.tsx index 9674d4e0c9c8..fa0235a6ec3d 100644 --- a/src/plugins/data_source_management/public/components/data_source_selector/data_source_selector.tsx +++ b/src/plugins/data_source_management/public/components/data_source_selector/data_source_selector.tsx @@ -37,6 +37,7 @@ interface DataSourceSelectorState { selectedOption: DataSourceOption[]; allDataSources: Array>; defaultDataSource: string | null; + dataSourceOptions?: DataSourceOption[]; } export interface DataSourceOption { @@ -57,11 +58,7 @@ export class DataSourceSelector extends React.Component< this.state = { allDataSources: [], defaultDataSource: '', - selectedOption: this.props.defaultOption - ? this.props.defaultOption - : this.props.hideLocalCluster - ? [] - : [LocalCluster], + selectedOption: this.props.hideLocalCluster ? [] : [LocalCluster], }; } @@ -69,54 +66,122 @@ export class DataSourceSelector extends React.Component< this._isMounted = false; } - async componentDidMount() { - this._isMounted = true; + handleSelectedOption( + dataSourceOptions: DataSourceOption[], + allDataSources: Array>, + defaultDataSource: string | null + ) { + const [{ id }] = this.props.defaultOption!; + const dataSource = dataSourceOptions.find((ds) => ds.id === id); + const selectedOption = dataSource ? [{ id, label: dataSource.label }] : []; + + // Invalid/filtered out datasource + if (!dataSource) { + this.props.notifications.addWarning( + i18n.translate('dataSource.fetchDataSourceError', { + defaultMessage: 'Data source with id is not available', + }) + ); + } - const currentDefaultDataSource = this.props.uiSettings?.get('defaultDataSource', null) ?? null; this.setState({ ...this.state, - defaultDataSource: currentDefaultDataSource, + dataSourceOptions, + selectedOption, + defaultDataSource, + allDataSources, }); + this.props.onSelectedDataSource(selectedOption); + } - getDataSourcesWithFields(this.props.savedObjectsClient, ['id', 'title', 'auth.type']) - .then((fetchedDataSources) => { - if (fetchedDataSources?.length) { - if (!this._isMounted) return; - this.setState({ - ...this.state, - allDataSources: fetchedDataSources, - }); - } - const dataSources = getFilteredDataSources( - this.state.allDataSources, - this.props.dataSourceFilter - ); - const selectedDataSource = getDefaultDataSource( - dataSources, - LocalCluster, - this.props.uiSettings, - this.props.hideLocalCluster, - this.props.defaultOption - ); - if (selectedDataSource.length === 0) { - this.props.notifications.addWarning('No connected data source available.'); - } else { - this.props.onSelectedDataSource(selectedDataSource); - this.setState({ - selectedOption: selectedDataSource, - }); - } - }) - .catch(() => { - this.props.notifications.addWarning( - i18n.translate('dataSource.fetchDataSourceError', { - defaultMessage: 'Unable to fetch existing data sources', - }) - ); - }); + handleDefaultDataSource( + dataSourceOptions: DataSourceOption[], + allDataSources: Array>, + defaultDataSource: string | null + ) { + const selectedDataSource = getDefaultDataSource( + dataSourceOptions, + LocalCluster, + defaultDataSource, + this.props.hideLocalCluster + ); + + // No active option, did not find valid option + if (selectedDataSource.length === 0) { + this.props.notifications.addWarning('No connected data source available.'); + this.props.onSelectedDataSource([]); + return; + } + + this.setState({ + ...this.state, + dataSourceOptions, + selectedOption: selectedDataSource, + defaultDataSource, + allDataSources, + }); + this.props.onSelectedDataSource(selectedDataSource); } - onChange(e) { + async componentDidMount() { + this._isMounted = true; + try { + // 1. Fetch + const fetchedDataSources = await getDataSourcesWithFields(this.props.savedObjectsClient, [ + 'id', + 'title', + 'auth.type', + ]); + + // 2. Process + const dataSourceOptions = getFilteredDataSources( + fetchedDataSources, + this.props.dataSourceFilter + ); + + // 3. Add local cluster as option + if (!this.props.hideLocalCluster) { + dataSourceOptions.unshift(LocalCluster); + } + + // 4. Error state if filter filters out everything + if (!dataSourceOptions.length) { + this.props.notifications.addWarning('No connected data source available.'); + this.props.onSelectedDataSource([]); + return; + } + + const defaultDataSource = this.props.uiSettings?.get('defaultDataSource', null) ?? null; + // 5.1 Empty default option, [], just want to show placeholder + if (this.props.defaultOption?.length === 0) { + this.setState({ + ...this.state, + dataSourceOptions, + selectedOption: [], + defaultDataSource, + allDataSources: fetchedDataSources, + }); + return; + } + + // 5.2 Handle active option, [{}] + if (this.props.defaultOption?.length) { + this.handleSelectedOption(dataSourceOptions, fetchedDataSources, defaultDataSource); + return; + } + + // 5.3 Handle default data source + this.handleDefaultDataSource(dataSourceOptions, fetchedDataSources, defaultDataSource); + } catch (err) { + this.props.notifications.addWarning( + i18n.translate('dataSource.fetchDataSourceError', { + defaultMessage: 'Unable to fetch existing data sources', + }) + ); + } + } + + onChange(e: DataSourceOption[]) { if (!this._isMounted) return; this.setState({ selectedOption: e, @@ -131,12 +196,8 @@ export class DataSourceSelector extends React.Component< : this.props.placeholderText; // The filter condition can be changed, thus we filter again here to make sure each time we will get the filtered data sources before rendering - const dataSources = getFilteredDataSources( - this.state.allDataSources, - this.props.dataSourceFilter - ); + const options = getFilteredDataSources(this.state.allDataSources, this.props.dataSourceFilter); - const options = dataSources.map((ds) => ({ id: ds.id, label: ds.attributes?.title || '' })); if (!this.props.hideLocalCluster) { options.unshift(LocalCluster); } diff --git a/src/plugins/data_source_management/public/components/utils.ts b/src/plugins/data_source_management/public/components/utils.ts index 81c0cd5c707a..b8c76bcf858c 100644 --- a/src/plugins/data_source_management/public/components/utils.ts +++ b/src/plugins/data_source_management/public/components/utils.ts @@ -16,7 +16,7 @@ import { noAuthCredentialAuthMethod, } from '../types'; import { AuthenticationMethodRegistry } from '../auth_registry'; -import { DataSourceOption } from './data_source_menu/types'; +import { DataSourceOption } from './data_source_selector/data_source_selector'; export async function getDataSources(savedObjectsClient: SavedObjectsClientContract) { return savedObjectsClient