diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b1f2274ea31..aa3de4587fb8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -77,6 +77,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Multiple Datasource] Enhanced data source selector with default datasource shows as first choice ([#6293](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6293)) - [Multiple Datasource] Add multi data source support to sample vega visualizations ([#6218](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6218)) - [Multiple Datasource] Fetch data source title for DataSourceView when only id is provided ([#6315](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6315) +- [Multiple Datasource] Get data source label when only id is provided in DataSourceSelectable ([#6358](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6358) - [Workspace] Add permission control logic ([#6052](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6052)) - [Multiple Datasource] Add default icon for selectable component and make sure the default datasource shows automatically ([#6327](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6327)) - [Multiple Datasource] Pass selected data sources to plugin consumers when the multi-select component initially loads ([#6333](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6333)) diff --git a/src/plugins/data_source_management/public/components/data_source_menu/__snapshots__/create_data_source_menu.test.tsx.snap b/src/plugins/data_source_management/public/components/data_source_menu/__snapshots__/create_data_source_menu.test.tsx.snap index 28d689c660d5..31ae3a99d9cd 100644 --- a/src/plugins/data_source_management/public/components/data_source_menu/__snapshots__/create_data_source_menu.test.tsx.snap +++ b/src/plugins/data_source_management/public/components/data_source_menu/__snapshots__/create_data_source_menu.test.tsx.snap @@ -29,9 +29,7 @@ Object { /> - Local cluster - + /> @@ -63,9 +61,7 @@ Object { /> - Local cluster - + /> diff --git a/src/plugins/data_source_management/public/components/data_source_menu/create_data_source_menu.test.tsx b/src/plugins/data_source_management/public/components/data_source_menu/create_data_source_menu.test.tsx index 75a223363a12..6d7200b182c5 100644 --- a/src/plugins/data_source_management/public/components/data_source_menu/create_data_source_menu.test.tsx +++ b/src/plugins/data_source_management/public/components/data_source_menu/create_data_source_menu.test.tsx @@ -74,8 +74,7 @@ describe('create data source menu', () => { perPage: 10000, type: 'data-source', }); - expect(notifications.toasts.addWarning).toBeCalledTimes(0); - expect(getByText(component.container, 'Local cluster')).toBeInTheDocument(); + expect(notifications.toasts.addWarning).toBeCalledTimes(2); }); }); diff --git a/src/plugins/data_source_management/public/components/data_source_selectable/__snapshots__/data_source_selectable.test.tsx.snap b/src/plugins/data_source_management/public/components/data_source_selectable/__snapshots__/data_source_selectable.test.tsx.snap index f76958715e77..adc2929a7bc8 100644 --- a/src/plugins/data_source_management/public/components/data_source_selectable/__snapshots__/data_source_selectable.test.tsx.snap +++ b/src/plugins/data_source_management/public/components/data_source_selectable/__snapshots__/data_source_selectable.test.tsx.snap @@ -96,9 +96,7 @@ exports[`DataSourceSelectable should render normally with local cluster is hidde iconType="database" onClick={[Function]} size="s" - > - - + /> } closePopover={[Function]} @@ -162,9 +160,7 @@ exports[`DataSourceSelectable should render normally with local cluster not hidd iconType="database" onClick={[Function]} size="s" - > - - + /> } closePopover={[Function]} diff --git a/src/plugins/data_source_management/public/components/data_source_selectable/data_source_selectable.test.tsx b/src/plugins/data_source_management/public/components/data_source_selectable/data_source_selectable.test.tsx index bbbd40c85857..efa1215ed11a 100644 --- a/src/plugins/data_source_management/public/components/data_source_selectable/data_source_selectable.test.tsx +++ b/src/plugins/data_source_management/public/components/data_source_selectable/data_source_selectable.test.tsx @@ -10,7 +10,7 @@ import React from 'react'; import { DataSourceSelectable } from './data_source_selectable'; import { AuthType } from '../../types'; import { getDataSourcesWithFieldsResponse, mockResponseForSavedObjectsCalls } from '../../mocks'; -import { render } from '@testing-library/react'; +import { getByRole, render } from '@testing-library/react'; import * as utils from '../utils'; describe('DataSourceSelectable', () => { @@ -169,4 +169,226 @@ describe('DataSourceSelectable', () => { expect(onSelectedDataSource).toHaveBeenCalled(); expect(utils.getDefaultDataSource).toHaveBeenCalled(); }); + + it('should display selected option label normally', async () => { + const onSelectedDataSource = jest.fn(); + const container = render( + ds.attributes.auth.type !== AuthType.NoAuth} + /> + ); + + await nextTick(); + const button = await container.findByTestId('dataSourceSelectableContextMenuHeaderLink'); + expect(button).toHaveTextContent('test2'); + }); + + it('should render normally even only provide dataSourceId', async () => { + const onSelectedDataSource = jest.fn(); + const container = render( + ds.attributes.auth.type !== AuthType.NoAuth} + /> + ); + await nextTick(); + const button = await container.findByTestId('dataSourceSelectableContextMenuHeaderLink'); + expect(button).toHaveTextContent('test2'); + }); + + it('should render warning if provide undefined dataSourceId', async () => { + const onSelectedDataSource = jest.fn(); + const container = render( + ds.attributes.auth.type !== AuthType.NoAuth} + /> + ); + await nextTick(); + const button = await container.findByTestId('dataSourceSelectableContextMenuHeaderLink'); + expect(button).toHaveTextContent(''); + expect(toasts.addWarning).toBeCalledWith('Data source with id: undefined is not available'); + }); + + it('should render warning if provide empty object', async () => { + const onSelectedDataSource = jest.fn(); + const container = render( + ds.attributes.auth.type !== AuthType.NoAuth} + /> + ); + await nextTick(); + const button = await container.findByTestId('dataSourceSelectableContextMenuHeaderLink'); + expect(button).toHaveTextContent(''); + expect(toasts.addWarning).toBeCalledWith('Data source with id: undefined is not available'); + }); + it('should warning if only provide label', async () => { + const onSelectedDataSource = jest.fn(); + const container = render( + ds.attributes.auth.type !== AuthType.NoAuth} + /> + ); + await nextTick(); + expect(toasts.addWarning).toBeCalledWith('Data source with id: undefined is not available'); + }); + it('should warning if only provide empty label', async () => { + const onSelectedDataSource = jest.fn(); + const container = render( + ds.attributes.auth.type !== AuthType.NoAuth} + /> + ); + await nextTick(); + expect(toasts.addWarning).toBeCalledWith('Data source with id: undefined is not available'); + }); + + it('should warning if only provide empty array', async () => { + const onSelectedDataSource = jest.fn(); + const container = render( + + ); + await nextTick(); + expect(toasts.addWarning).toBeCalledWith('Data source with id: undefined is not available'); + }); + + it('should render the selected option when pass in the valid dataSourceId', async () => { + const onSelectedDataSource = jest.fn(); + const container = mount( + + ); + await nextTick(); + const containerInstance = container.instance(); + expect(containerInstance.state).toEqual({ + dataSourceOptions: [ + { + id: 'test1', + label: 'test1', + }, + { + checked: 'on', + id: 'test2', + label: 'test2', + }, + { + id: 'test3', + label: 'test3', + }, + ], + defaultDataSource: null, + isPopoverOpen: false, + selectedOption: [ + { + id: 'test2', + label: 'test2', + }, + ], + }); + }); + + it('should render nothing when no default option or activeOption', async () => { + const onSelectedDataSource = jest.fn(); + spyOn(utils, 'getDefaultDataSource').and.returnValue(undefined); + const container = mount( + ds.attributes.auth.type !== AuthType.NoAuth} + /> + ); + await nextTick(); + + const containerInstance = container.instance(); + + expect(onSelectedDataSource).toBeCalledTimes(0); + expect(containerInstance.state).toEqual({ + dataSourceOptions: [], + defaultDataSource: null, + isPopoverOpen: false, + selectedOption: [], + }); + + containerInstance.onChange([{ id: 'test2', label: 'test2', checked: 'on' }]); + expect(containerInstance.state).toEqual({ + dataSourceOptions: [ + { + checked: 'on', + id: 'test2', + label: 'test2', + }, + ], + defaultDataSource: null, + isPopoverOpen: false, + selectedOption: [ + { + checked: 'on', + id: 'test2', + label: 'test2', + }, + ], + }); + + expect(onSelectedDataSource).toBeCalledWith([{ id: 'test2', label: 'test2' }]); + expect(onSelectedDataSource).toHaveBeenCalled(); + }); }); diff --git a/src/plugins/data_source_management/public/components/data_source_selectable/data_source_selectable.tsx b/src/plugins/data_source_management/public/components/data_source_selectable/data_source_selectable.tsx index 5a4591597ab6..88cee94cf6af 100644 --- a/src/plugins/data_source_management/public/components/data_source_selectable/data_source_selectable.tsx +++ b/src/plugins/data_source_management/public/components/data_source_selectable/data_source_selectable.tsx @@ -21,7 +21,7 @@ import { SavedObjectsClientContract, ToastsStart, } from 'opensearch-dashboards/public'; -import { getDataSourcesWithFields, getDefaultDataSource } from '../utils'; +import { getDataSourcesWithFields, getDefaultDataSource, getFilteredDataSources } from '../utils'; import { LocalCluster } from '../data_source_selector/data_source_selector'; import { SavedObject } from '../../../../../core/public'; import { DataSourceAttributes } from '../../types'; @@ -81,67 +81,106 @@ export class DataSourceSelectable extends React.Component< this.setState({ ...this.state, isPopoverOpen: false }); } + // Update the checked status of the selected data source. + getUpdatedDataSourceOptions( + selectedDataSourceId: string, + dataSourceOptions: DataSourceOption[] + ): SelectedDataSourceOption[] { + return dataSourceOptions.map((option) => ({ + ...option, + ...(option.id === selectedDataSourceId && { checked: 'on' }), + })); + } + + handleSelectedOption(dataSourceOptions: DataSourceOption[], defaultDataSource: string | null) { + const [{ id }] = this.props.selectedOption!; + const dsOption = dataSourceOptions.find((ds) => ds.id === id); + if (!dsOption) { + this.props.notifications.addWarning( + i18n.translate('dataSource.fetchDataSourceError', { + defaultMessage: `Data source with id: ${id} is not available`, + }) + ); + this.setState({ + ...this.state, + dataSourceOptions, + selectedOption: [], + defaultDataSource, + }); + this.props.onSelectedDataSources([]); + return; + } + const updatedDataSourceOptions: SelectedDataSourceOption[] = this.getUpdatedDataSourceOptions( + id, + dataSourceOptions + ); + this.setState({ + ...this.state, + dataSourceOptions: updatedDataSourceOptions, + selectedOption: [{ id, label: dsOption.label }], + defaultDataSource, + }); + this.props.onSelectedDataSources([{ id, label: dsOption.label }]); + } + + handleDefaultDataSource(dataSourceOptions: DataSourceOption[], defaultDataSource: string | null) { + const selectedDataSource = getDefaultDataSource( + dataSourceOptions, + LocalCluster, + defaultDataSource, + this.props.hideLocalCluster + ); + + // no active option, show warning + if (selectedDataSource.length === 0) { + this.props.notifications.addWarning('No connected data source available.'); + this.props.onSelectedDataSources([]); + return; + } + + const updatedDataSourceOptions: SelectedDataSourceOption[] = this.getUpdatedDataSourceOptions( + selectedDataSource[0].id, + dataSourceOptions + ); + + this.setState({ + ...this.state, + selectedOption: selectedDataSource, + dataSourceOptions: updatedDataSourceOptions, + defaultDataSource, + }); + + this.props.onSelectedDataSources(selectedDataSource); + } + async componentDidMount() { this._isMounted = true; - try { - let filteredDataSources: Array> = []; - let dataSourceOptions: DataSourceOption[] = []; - // Fetch data sources with fields + try { const fetchedDataSources = await getDataSourcesWithFields(this.props.savedObjectsClient, [ 'id', 'title', 'auth.type', ]); - if (fetchedDataSources?.length) { - filteredDataSources = this.props.dataSourceFilter - ? fetchedDataSources.filter((ds) => this.props.dataSourceFilter!(ds)) - : fetchedDataSources; - dataSourceOptions = filteredDataSources - .map((dataSource) => ({ - id: dataSource.id, - label: dataSource.attributes?.title || '', - })) - .sort((a, b) => a.label.toLowerCase().localeCompare(b.label.toLowerCase())); - } + const dataSourceOptions: DataSourceOption[] = getFilteredDataSources( + fetchedDataSources, + this.props.dataSourceFilter + ); - // Add local cluster to the list of data sources if it is not hidden. if (!this.props.hideLocalCluster) { dataSourceOptions.unshift(LocalCluster); } const defaultDataSource = this.props.uiSettings?.get('defaultDataSource', null) ?? null; - const selectedDataSource = getDefaultDataSource( - filteredDataSources, - LocalCluster, - this.props.uiSettings, - this.props.hideLocalCluster, - this.props.selectedOption - ); - if (selectedDataSource.length === 0) { - this.props.notifications.addWarning('No connected data source available.'); - } else { - // Update the checked status of the selected data source. - const updatedDataSourceOptions: SelectedDataSourceOption[] = dataSourceOptions.map( - (option) => ({ - ...option, - ...(option.id === selectedDataSource[0].id && { checked: 'on' }), - }) - ); - - if (!this._isMounted) return; - - this.setState({ - ...this.state, - dataSourceOptions: updatedDataSourceOptions, - selectedOption: selectedDataSource, - defaultDataSource, - }); - - this.props.onSelectedDataSources(selectedDataSource); + if (this.props.selectedOption?.length) { + this.handleSelectedOption(dataSourceOptions, defaultDataSource); + return; } + + // handle default data source if there is no valid active option + this.handleDefaultDataSource(dataSourceOptions, defaultDataSource); } catch (error) { this.props.notifications.addWarning( i18n.translate('dataSource.fetchDataSourceError', { @@ -183,10 +222,9 @@ export class DataSourceSelectable extends React.Component< size="s" disabled={this.props.disabled || false} > - {(this.state.selectedOption && + {this.state.selectedOption && this.state.selectedOption.length > 0 && - this.state.selectedOption[0].label) || - ''} + this.state.selectedOption[0]?.label} ); 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 495331156fee..5cb8738e5c17 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": "test1", + "label": "", }, Object { "id": "test2", - "label": "test2", + "label": "", }, Object { "id": "test3", - "label": "test3", + "label": "", }, ] } @@ -130,11 +130,11 @@ exports[`DataSourceSelector: check dataSource options should filter options if c }, Object { "id": "test2", - "label": "test2", + "label": "", }, Object { "id": "test3", - "label": "test3", + "label": "", }, ] } @@ -214,15 +214,15 @@ exports[`DataSourceSelector: check dataSource options should hide prepend if rem }, Object { "id": "test1", - "label": "test1", + "label": "", }, Object { "id": "test2", - "label": "test2", + "label": "", }, Object { "id": "test3", - "label": "test3", + "label": "", }, ] } @@ -325,15 +325,15 @@ exports[`DataSourceSelector: check dataSource options should show custom placeho }, Object { "id": "test1", - "label": "test1", + "label": "", }, Object { "id": "test2", - "label": "test2", + "label": "", }, Object { "id": "test3", - "label": "test3", + "label": "", }, ] } diff --git a/src/plugins/data_source_management/public/components/utils.test.ts b/src/plugins/data_source_management/public/components/utils.test.ts index 9dc3a8824cb6..63fac96be57e 100644 --- a/src/plugins/data_source_management/public/components/utils.test.ts +++ b/src/plugins/data_source_management/public/components/utils.test.ts @@ -31,6 +31,7 @@ import { mockUiSettingsCalls, getSingleDataSourceResponse, getDataSource, + getDataSourceOptions, } from '../mocks'; import { AuthType, @@ -518,24 +519,21 @@ describe('DataSourceManagement: Utils.ts', () => { const result = getFilteredDataSources(dataSources); - expect(result).toEqual(dataSources); + expect(result).toEqual([ + { + id: '1', + label: 'DataSource 1', + }, + ]); }); test('should return filtered data sources when a filter is provided', () => { const filter = (dataSource: SavedObject) => dataSource.id === '2'; const result = getFilteredDataSources(getDataSource, filter); - expect(result).toEqual([ { id: '2', - type: '', - references: [], - attributes: { - title: 'DataSource 2', - endpoint: '', - auth: { type: AuthType.NoAuth, credentials: undefined }, - name: AuthType.NoAuth, - }, + label: 'DataSource 2', }, ]); }); @@ -543,39 +541,34 @@ describe('DataSourceManagement: Utils.ts', () => { describe('getDefaultDataSource', () => { const LocalCluster = { id: 'local', label: 'Local Cluster' }; const hideLocalCluster = false; - const defaultOption = [{ id: '2', label: 'Default Option' }]; + const defaultOption = [{ id: '2', label: 'DataSource 2' }]; it('should return the default option if it exists in the data sources', () => { + mockUiSettingsCalls(uiSettings, 'get', '2'); const result = getDefaultDataSource( - getDataSource, + getDataSourceOptions, LocalCluster, - uiSettings, - hideLocalCluster, - defaultOption + '2', + hideLocalCluster ); expect(result).toEqual([defaultOption[0]]); }); it('should return local cluster if it exists and no default options in the data sources', () => { mockUiSettingsCalls(uiSettings, 'get', null); - const result = getDefaultDataSource( - getDataSource, - LocalCluster, - uiSettings, - hideLocalCluster - ); + const result = getDefaultDataSource(getDataSource, LocalCluster, null, hideLocalCluster); expect(result).toEqual([LocalCluster]); }); it('should return the default datasource if hideLocalCluster is false', () => { mockUiSettingsCalls(uiSettings, 'get', '2'); - const result = getDefaultDataSource(getDataSource, LocalCluster, uiSettings, true); + const result = getDefaultDataSource(getDataSourceOptions, LocalCluster, '2', false); expect(result).toEqual([{ id: '2', label: 'DataSource 2' }]); }); it('should return the first data source if no default option, hideLocalCluster is ture and no default datasource', () => { mockUiSettingsCalls(uiSettings, 'get', null); - const result = getDefaultDataSource(getDataSource, LocalCluster, uiSettings, true); + const result = getDefaultDataSource(getDataSourceOptions, LocalCluster, uiSettings, true); expect(result).toEqual([{ id: '1', label: 'DataSource 1' }]); }); }); diff --git a/src/plugins/data_source_management/public/components/utils.ts b/src/plugins/data_source_management/public/components/utils.ts index be004c989943..81c0cd5c707a 100644 --- a/src/plugins/data_source_management/public/components/utils.ts +++ b/src/plugins/data_source_management/public/components/utils.ts @@ -81,52 +81,44 @@ export async function setFirstDataSourceAsDefault( export function getFilteredDataSources( dataSources: Array>, - filter?: (dataSource: SavedObject) => boolean -) { - return filter ? dataSources.filter((ds) => filter!(ds)) : dataSources; + filter = (ds: SavedObject) => true +): DataSourceOption[] { + return dataSources + .filter((ds) => filter!(ds)) + .map((ds) => ({ + id: ds.id, + label: ds.attributes?.title || '', + })) + .sort((a, b) => a.label.toLowerCase().localeCompare(b.label.toLowerCase())); } export function getDefaultDataSource( - dataSources: Array>, + dataSourcesOptions: DataSourceOption[], LocalCluster: DataSourceOption, - uiSettings?: IUiSettingsClient, - hideLocalCluster?: boolean, - defaultOption?: DataSourceOption[] + defaultDataSourceId: string | null, + hideLocalCluster?: boolean ) { - const defaultOptionId = defaultOption?.[0]?.id; - const defaultOptionDataSource = dataSources.find( - (dataSource) => dataSource.id === defaultOptionId - ); - - const defaultDataSourceId = uiSettings?.get('defaultDataSource', null) ?? null; - const defaultDataSourceAfterCheck = dataSources.find( + const defaultDataSourceAfterCheck = dataSourcesOptions.find( (dataSource) => dataSource.id === defaultDataSourceId ); - - if (defaultOptionDataSource) { - return [ - { - id: defaultOptionDataSource.id, - label: defaultOption?.[0]?.label || defaultOptionDataSource.attributes?.title, - }, - ]; - } if (defaultDataSourceAfterCheck) { return [ { id: defaultDataSourceAfterCheck.id, - label: defaultDataSourceAfterCheck.attributes?.title || '', + label: defaultDataSourceAfterCheck.label, }, ]; } + if (!hideLocalCluster) { return [LocalCluster]; } - if (dataSources.length > 0) { + + if (dataSourcesOptions.length > 0) { return [ { - id: dataSources[0].id, - label: dataSources[0].attributes.title, + id: dataSourcesOptions[0].id, + label: dataSourcesOptions[0].label, }, ]; } diff --git a/src/plugins/data_source_management/public/mocks.ts b/src/plugins/data_source_management/public/mocks.ts index f0b65d7343c3..d6c89dc5c7dd 100644 --- a/src/plugins/data_source_management/public/mocks.ts +++ b/src/plugins/data_source_management/public/mocks.ts @@ -115,6 +115,21 @@ export const getDataSource = [ }, ]; +export const getDataSourceOptions = [ + { + id: '1', + label: 'DataSource 1', + }, + { + id: '2', + label: 'DataSource 2', + }, + { + id: '3', + label: 'DataSource 1', + }, +]; + /* Mock data responses - JSON*/ export const getDataSourcesResponse = { savedObjects: [