From 85df6629fc62374b898edefec5984ead0e811dc1 Mon Sep 17 00:00:00 2001 From: Yu Jin <112784385+yujin-emma@users.noreply.github.com> Date: Tue, 9 Apr 2024 10:30:21 -0700 Subject: [PATCH 1/8] [Multiple DataSource] DataSourceSelectable support to render label by getting it from dataSourceOptions (#6358) * get label from dataSourceOptions Signed-off-by: yujin-emma * update dataSourceOptions Signed-off-by: yujin-emma * update changelog Signed-off-by: yujin-emma * fix failed test Signed-off-by: yujin-emma * address comments and fix test Signed-off-by: yujin-emma * update selected option checked status and udpate snapshot Signed-off-by: yujin-emma * update selectable test Signed-off-by: yujin-emma * revert example code Signed-off-by: yujin-emma * revern config file Signed-off-by: yujin-emma * push the utils Signed-off-by: yujin-emma * udpate snapshot Signed-off-by: yujin-emma * remove console log Signed-off-by: yujin-emma * udate default data source Signed-off-by: yujin-emma * remove unnessary check for empty input Signed-off-by: yujin-emma * fix failed test Signed-off-by: yujin-emma * fix failed test Signed-off-by: yujin-emma --------- Signed-off-by: yujin-emma --- CHANGELOG.md | 1 + .../create_data_source_menu.test.tsx.snap | 8 +- .../create_data_source_menu.test.tsx | 3 +- .../data_source_selectable.test.tsx.snap | 8 +- .../data_source_selectable.test.tsx | 224 +++++++++++++++++- .../data_source_selectable.tsx | 134 +++++++---- .../data_source_selector.test.tsx.snap | 22 +- .../public/components/utils.test.ts | 39 ++- .../public/components/utils.ts | 46 ++-- .../data_source_management/public/mocks.ts | 15 ++ 10 files changed, 376 insertions(+), 124 deletions(-) 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: [ From 87deeda94c2825230e3cc911d08e1434f8635a7a Mon Sep 17 00:00:00 2001 From: "Yuanqi(Ella) Zhu" <53279298+zhyuanqi@users.noreply.github.com> Date: Tue, 9 Apr 2024 12:57:51 -0700 Subject: [PATCH 2/8] Add default icon in multi-selectable picker (#6357) Signed-off-by: Yuanqi(Ella) Zhu --- CHANGELOG.md | 1 + .../data_source_menu/data_source_menu.tsx | 1 + .../data_source_filter_group.test.tsx.snap | 50 +++++++++++- ...data_source_multi_selectable.test.tsx.snap | 2 + .../data_source_filter_group.test.tsx | 2 + .../data_source_filter_group.tsx | 5 +- .../data_source_multi_selectable.test.tsx | 46 ++++++++++- .../data_source_multi_selectable.tsx | 80 ++++++++++--------- .../components/data_source_option.test.tsx | 46 +++++++++++ .../public/components/data_source_option.tsx | 29 +++++++ 10 files changed, 221 insertions(+), 41 deletions(-) create mode 100644 src/plugins/data_source_management/public/components/data_source_option.test.tsx create mode 100644 src/plugins/data_source_management/public/components/data_source_option.tsx diff --git a/CHANGELOG.md b/CHANGELOG.md index aa3de4587fb8..290b0531f838 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -82,6 +82,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [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)) - [Multiple Datasource] Add installedPlugins list to data source saved object ([#6348](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6348)) +- [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)) ### 🐛 Bug Fixes diff --git a/src/plugins/data_source_management/public/components/data_source_menu/data_source_menu.tsx b/src/plugins/data_source_management/public/components/data_source_menu/data_source_menu.tsx index a9030ec6c9d9..fdc5d2c7508b 100644 --- a/src/plugins/data_source_management/public/components/data_source_menu/data_source_menu.tsx +++ b/src/plugins/data_source_management/public/components/data_source_menu/data_source_menu.tsx @@ -44,6 +44,7 @@ export function DataSourceMenu(props: DataSourceMenuProps): ReactElement | savedObjectsClient={savedObjects!} notifications={notifications!.toasts} onSelectedDataSources={onSelectedDataSources!} + uiSettings={uiSettings} /> ); } diff --git a/src/plugins/data_source_management/public/components/data_source_multi_selectable/__snapshots__/data_source_filter_group.test.tsx.snap b/src/plugins/data_source_management/public/components/data_source_multi_selectable/__snapshots__/data_source_filter_group.test.tsx.snap index 665f9cfa27dc..fe89b1409510 100644 --- a/src/plugins/data_source_management/public/components/data_source_multi_selectable/__snapshots__/data_source_filter_group.test.tsx.snap +++ b/src/plugins/data_source_management/public/components/data_source_multi_selectable/__snapshots__/data_source_filter_group.test.tsx.snap @@ -62,7 +62,17 @@ exports[`DataSourceFilterGroup should render normally 1`] = ` } } > - name1 + @@ -204,7 +214,33 @@ Object { - name1 +
+
+ name1 +
+
+ + + + Default + + + +
+
@@ -497,7 +533,15 @@ Object { - name1 +
+
+ name1 +
+
diff --git a/src/plugins/data_source_management/public/components/data_source_multi_selectable/__snapshots__/data_source_multi_selectable.test.tsx.snap b/src/plugins/data_source_management/public/components/data_source_multi_selectable/__snapshots__/data_source_multi_selectable.test.tsx.snap index 627a1169c15b..9f457a898d74 100644 --- a/src/plugins/data_source_management/public/components/data_source_multi_selectable/__snapshots__/data_source_multi_selectable.test.tsx.snap +++ b/src/plugins/data_source_management/public/components/data_source_multi_selectable/__snapshots__/data_source_multi_selectable.test.tsx.snap @@ -2,6 +2,7 @@ exports[`DataSourceMultiSelectable should render normally with local cluster hidden 1`] = ` @@ -9,6 +10,7 @@ exports[`DataSourceMultiSelectable should render normally with local cluster hid exports[`DataSourceMultiSelectable should render normally with local cluster not hidden 1`] = ` diff --git a/src/plugins/data_source_management/public/components/data_source_multi_selectable/data_source_filter_group.test.tsx b/src/plugins/data_source_management/public/components/data_source_multi_selectable/data_source_filter_group.test.tsx index 62bf10ec8310..b75dbdf8a55c 100644 --- a/src/plugins/data_source_management/public/components/data_source_multi_selectable/data_source_filter_group.test.tsx +++ b/src/plugins/data_source_management/public/components/data_source_multi_selectable/data_source_filter_group.test.tsx @@ -17,6 +17,7 @@ describe('DataSourceFilterGroup', () => { mockCallBack(items)} + defaultDataSource="1" /> ); expect(component).toMatchSnapshot(); @@ -28,6 +29,7 @@ describe('DataSourceFilterGroup', () => { mockCallBack(items)} + defaultDataSource="1" /> ); const button = await container.findByTestId('dataSourceFilterGroupButton'); diff --git a/src/plugins/data_source_management/public/components/data_source_multi_selectable/data_source_filter_group.tsx b/src/plugins/data_source_management/public/components/data_source_multi_selectable/data_source_filter_group.tsx index 8d0e22beadaf..948a788268d6 100644 --- a/src/plugins/data_source_management/public/components/data_source_multi_selectable/data_source_filter_group.tsx +++ b/src/plugins/data_source_management/public/components/data_source_multi_selectable/data_source_filter_group.tsx @@ -16,6 +16,7 @@ import { EuiButtonEmpty, } from '@elastic/eui'; import { DataSourceOption } from '../data_source_selector/data_source_selector'; +import { DataSourceOptionItem } from '../data_source_option'; export interface SelectedDataSourceOption extends DataSourceOption { label: string; @@ -27,6 +28,7 @@ export interface SelectedDataSourceOption extends DataSourceOption { export interface DataSourceFilterGroupProps { selectedOptions: SelectedDataSourceOption[]; setSelectedOptions: (options: SelectedDataSourceOption[]) => void; + defaultDataSource: string | null; } type SelectionToggleOptionIds = 'select_all' | 'deselect_all'; @@ -45,6 +47,7 @@ const selectionToggleButtons = [ export const DataSourceFilterGroup: React.FC = ({ selectedOptions, setSelectedOptions, + defaultDataSource, }) => { const [isPopoverOpen, setIsPopoverOpen] = useState(false); const [selectionToggleSelectedId, setSelectionToggleSelectedId] = useState< @@ -148,7 +151,7 @@ export const DataSourceFilterGroup: React.FC = ({ showIcons={true} style={itemStyle} > - {item.label} + ); })} diff --git a/src/plugins/data_source_management/public/components/data_source_multi_selectable/data_source_multi_selectable.test.tsx b/src/plugins/data_source_management/public/components/data_source_multi_selectable/data_source_multi_selectable.test.tsx index afe65f554626..448eb404d995 100644 --- a/src/plugins/data_source_management/public/components/data_source_multi_selectable/data_source_multi_selectable.test.tsx +++ b/src/plugins/data_source_management/public/components/data_source_multi_selectable/data_source_multi_selectable.test.tsx @@ -5,7 +5,11 @@ import { SavedObjectsClientContract } from 'opensearch-dashboards/public'; import { notificationServiceMock } from '../../../../../core/public/mocks'; -import { getDataSourcesWithFieldsResponse, mockResponseForSavedObjectsCalls } from '../../mocks'; +import { + getDataSourcesWithFieldsResponse, + mockResponseForSavedObjectsCalls, + mockManagementPlugin, +} from '../../mocks'; import { ShallowWrapper, shallow } from 'enzyme'; import { DataSourceMultiSelectable } from './data_source_multi_selectable'; import React from 'react'; @@ -17,8 +21,12 @@ describe('DataSourceMultiSelectable', () => { let client: SavedObjectsClientContract; const { toasts } = notificationServiceMock.createStartContract(); const nextTick = () => new Promise((res) => process.nextTick(res)); + const mockedContext = mockManagementPlugin.createDataSourceManagementContext(); + const uiSettings = mockedContext.uiSettings; beforeEach(() => { + jest.clearAllMocks(); + client = { find: jest.fn().mockResolvedValue([]), } as any; @@ -102,4 +110,40 @@ describe('DataSourceMultiSelectable', () => { expect(callbackMock).toBeCalledWith([]); }); + + it('should retrun correct state when ui Settings provided', async () => { + spyOn(uiSettings, 'get').and.returnValue('test1'); + component = shallow( + + ); + await component.instance().componentDidMount!(); + expect(uiSettings.get).toBeCalledWith('defaultDataSource', null); + expect(component.state('defaultDataSource')).toEqual('test1'); + expect(component.state('selectedOptions')).toHaveLength(3); + }); + + it('should retrun correct state when ui Settings provided and hide cluster is false', async () => { + spyOn(uiSettings, 'get').and.returnValue('test1'); + component = shallow( + + ); + await component.instance().componentDidMount!(); + expect(uiSettings.get).toBeCalledWith('defaultDataSource', null); + expect(component.state('defaultDataSource')).toEqual('test1'); + expect(component.state('selectedOptions')).toHaveLength(4); + }); }); diff --git a/src/plugins/data_source_management/public/components/data_source_multi_selectable/data_source_multi_selectable.tsx b/src/plugins/data_source_management/public/components/data_source_multi_selectable/data_source_multi_selectable.tsx index 2bcf57899189..1a1d958b618c 100644 --- a/src/plugins/data_source_management/public/components/data_source_multi_selectable/data_source_multi_selectable.tsx +++ b/src/plugins/data_source_management/public/components/data_source_multi_selectable/data_source_multi_selectable.tsx @@ -6,6 +6,7 @@ import React from 'react'; import { SavedObjectsClientContract, ToastsStart } from 'opensearch-dashboards/public'; import { i18n } from '@osd/i18n'; +import { IUiSettingsClient } from 'src/core/public'; import { DataSourceFilterGroup, SelectedDataSourceOption } from './data_source_filter_group'; import { getDataSourcesWithFields } from '../utils'; @@ -15,11 +16,13 @@ export interface DataSourceMultiSeletableProps { onSelectedDataSources: (dataSources: SelectedDataSourceOption[]) => void; hideLocalCluster: boolean; fullWidth: boolean; + uiSettings?: IUiSettingsClient; } interface DataSourceMultiSeletableState { dataSourceOptions: SelectedDataSourceOption[]; selectedOptions: SelectedDataSourceOption[]; + defaultDataSource: string | null; } export class DataSourceMultiSelectable extends React.Component< @@ -34,6 +37,7 @@ export class DataSourceMultiSelectable extends React.Component< this.state = { dataSourceOptions: [], selectedOptions: [], + defaultDataSource: null, }; } @@ -43,46 +47,49 @@ export class DataSourceMultiSelectable extends React.Component< async componentDidMount() { this._isMounted = true; - getDataSourcesWithFields(this.props.savedObjectsClient, ['id', 'title', 'auth.type']) - .then((fetchedDataSources) => { - if (fetchedDataSources?.length) { - // all data sources are selected by default on initial page load - const selectedOptions: SelectedDataSourceOption[] = fetchedDataSources.map( - (dataSource) => ({ - id: dataSource.id, - label: dataSource.attributes?.title || '', - checked: 'on', - visible: true, - }) - ); + try { + const defaultDataSource = this.props.uiSettings?.get('defaultDataSource', null) ?? null; + let selectedOptions: SelectedDataSourceOption[] = []; + const fetchedDataSources = await getDataSourcesWithFields(this.props.savedObjectsClient, [ + 'id', + 'title', + 'auth.type', + ]); - if (!this.props.hideLocalCluster) { - selectedOptions.unshift({ - id: '', - label: 'Local cluster', - checked: 'on', - visible: true, - }); - } + if (fetchedDataSources?.length) { + selectedOptions = fetchedDataSources.map((dataSource) => ({ + id: dataSource.id, + label: dataSource.attributes?.title || '', + checked: 'on', + visible: true, + })); + } - if (!this._isMounted) return; - this.setState({ - ...this.state, - selectedOptions, - }); + if (!this.props.hideLocalCluster) { + selectedOptions.unshift({ + id: '', + label: 'Local cluster', + checked: 'on', + visible: true, + }); + } - this.props.onSelectedDataSources( - selectedOptions.filter((option) => option.checked === 'on') - ); - } - }) - .catch(() => { - this.props.notifications.addWarning( - i18n.translate('dataSource.fetchDataSourceError', { - defaultMessage: 'Unable to fetch existing data sources', - }) - ); + if (!this._isMounted) return; + + this.setState({ + ...this.state, + selectedOptions, + defaultDataSource, }); + + this.props.onSelectedDataSources(selectedOptions); + } catch (error) { + this.props.notifications.addWarning( + i18n.translate('dataSource.fetchDataSourceError', { + defaultMessage: 'Unable to fetch existing data sources', + }) + ); + } } onChange(selectedOptions: SelectedDataSourceOption[]) { @@ -98,6 +105,7 @@ export class DataSourceMultiSelectable extends React.Component< ); } diff --git a/src/plugins/data_source_management/public/components/data_source_option.test.tsx b/src/plugins/data_source_management/public/components/data_source_option.test.tsx new file mode 100644 index 000000000000..7d6375f47d29 --- /dev/null +++ b/src/plugins/data_source_management/public/components/data_source_option.test.tsx @@ -0,0 +1,46 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { shallow } from 'enzyme'; +import React from 'react'; +import { EuiBadge, EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; +import { DataSourceOptionItem } from './data_source_option'; +import { SelectedDataSourceOption } from './data_source_multi_selectable/data_source_filter_group'; + +describe('Test on ShowDataSourceOption', () => { + it('should render the component with label', () => { + const item: SelectedDataSourceOption = { + id: '1', + label: 'DataSource 1', + visible: true, + }; + const defaultDataSource = null; + + const component = shallow( + + ); + + expect(component.find(EuiFlexGroup)).toHaveLength(1); + expect(component.find(EuiFlexItem)).toHaveLength(1); + expect(component.find(EuiBadge)).toHaveLength(0); + }); + + it('should render the component with label and default badge', () => { + const item = { + id: '1', + label: 'DataSource 1', + visible: true, + }; + const defaultDataSource = '1'; + + const component = shallow( + + ); + + expect(component.find(EuiFlexGroup)).toHaveLength(1); + expect(component.find(EuiFlexItem)).toHaveLength(2); + expect(component.find(EuiBadge)).toHaveLength(1); + }); +}); diff --git a/src/plugins/data_source_management/public/components/data_source_option.tsx b/src/plugins/data_source_management/public/components/data_source_option.tsx new file mode 100644 index 000000000000..cb5dac27d5ae --- /dev/null +++ b/src/plugins/data_source_management/public/components/data_source_option.tsx @@ -0,0 +1,29 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import React from 'react'; +import { EuiFlexGroup, EuiFlexItem, EuiBadge } from '@elastic/eui'; +import { SelectedDataSourceOption } from './data_source_multi_selectable/data_source_filter_group'; + +export interface DataSourceOptionItemProps { + item: SelectedDataSourceOption; + defaultDataSource: string | null; +} + +export const DataSourceOptionItem: React.FC = ({ + item, + defaultDataSource, +}) => { + return ( + + {item.label} + {item.id === defaultDataSource && ( + + Default + + )} + + ); +}; From b70c3273151759bf9131e1dc4b167ef4f822c880 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 9 Apr 2024 16:12:38 -0400 Subject: [PATCH 3/8] Fix for checkForFunctionProperty so that order does not matter (#6248) * Fix for checkForFunctionProperty so that order does not matter Signed-off-by: Craig Perkins * Remove import for inspect Signed-off-by: Craig Perkins * Add to CHANGELOG Signed-off-by: Craig Perkins * replace with jest.fn() Signed-off-by: Craig Perkins * Update CHANGELOG Signed-off-by: Craig Perkins * Add scenarios where object does not contain fn Signed-off-by: Craig Perkins --------- Signed-off-by: Craig Perkins Signed-off-by: SuZhou-Joe Co-authored-by: SuZhou-Joe Co-authored-by: ZilongX <99905560+ZilongX@users.noreply.github.com> --- CHANGELOG.md | 1 + .../public/data_model/utils.test.js | 51 ++++++++++++++++--- .../vis_type_vega/public/data_model/utils.ts | 16 +++--- 3 files changed, 54 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 290b0531f838..800b987d2f6b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -107,6 +107,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] Fix for checkForFunctionProperty so that order does not matter ([#6248](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6248)) ### 🚞 Infrastructure diff --git a/src/plugins/vis_type_vega/public/data_model/utils.test.js b/src/plugins/vis_type_vega/public/data_model/utils.test.js index 9fe648c71139..9d6d67d793bb 100644 --- a/src/plugins/vis_type_vega/public/data_model/utils.test.js +++ b/src/plugins/vis_type_vega/public/data_model/utils.test.js @@ -59,9 +59,7 @@ describe('Utils.handleInvalidQuery', () => { test('should return null if input object has function as property', async () => { const input = { key1: 'value1', - key2: () => { - alert('Hello!'); - }, + key2: () => jest.fn(), }; expect(Utils.handleInvalidQuery(input)).toBe(null); @@ -71,9 +69,7 @@ describe('Utils.handleInvalidQuery', () => { const input = { key1: 'value1', key2: { - func: () => { - alert('Hello!'); - }, + func: () => jest.fn(), }, }; expect(Utils.handleInvalidQuery(input)).toBe(null); @@ -91,4 +87,47 @@ describe('Utils.handleInvalidQuery', () => { }).toThrowError(); }); }); + + test('should identify object contains function properties', async () => { + const input1 = { + key1: 'value1', + key2: () => jest.fn(), + }; + + expect(Utils.checkForFunctionProperty(input1)).toBe(true); + + const input2 = { + key1: () => jest.fn(), + key2: 'value1', + }; + + expect(Utils.checkForFunctionProperty(input2)).toBe(true); + + const nestedInput = { + key1: { + nestedKey1: 'nestedValue1', + nestedKey2: () => jest.fn(), + }, + key2: 'value1', + }; + + expect(Utils.checkForFunctionProperty(nestedInput)).toBe(true); + + const inputWithoutFn = { + key1: 'value1', + key2: 'value2', + }; + + expect(Utils.checkForFunctionProperty(inputWithoutFn)).toBe(false); + + const nestedInputWithoutFn = { + key1: { + nestedKey1: 'nestedValue1', + nestedKey2: 'nestedValue2', + }, + key2: 'value2', + }; + + expect(Utils.checkForFunctionProperty(nestedInputWithoutFn)).toBe(false); + }); }); diff --git a/src/plugins/vis_type_vega/public/data_model/utils.ts b/src/plugins/vis_type_vega/public/data_model/utils.ts index 4e7a85062354..2aba9f9fa577 100644 --- a/src/plugins/vis_type_vega/public/data_model/utils.ts +++ b/src/plugins/vis_type_vega/public/data_model/utils.ts @@ -79,14 +79,14 @@ export class Utils { } static checkForFunctionProperty(object: object): boolean { - let result = false; - Object.values(object).forEach((value) => { - result = - typeof value === 'function' - ? true - : Utils.isObject(value) && Utils.checkForFunctionProperty(value); - }); - return result; + for (const value of Object.values(object)) { + if (typeof value === 'function') { + return true; + } else if (Utils.isObject(value) && Utils.checkForFunctionProperty(value)) { + return true; + } + } + return false; } static handleInvalidDate(date: unknown): number | string | Date | null { From cb9b26cedc65b874b9acce8afb0b47dbdaa037d7 Mon Sep 17 00:00:00 2001 From: Yulong Ruan Date: Wed, 10 Apr 2024 11:11:13 +0800 Subject: [PATCH 4/8] [Workspace] Filter left nav menu items according to the current workspace (#6234) * Filter left nav menu items according to the current workspace An "Application Not Found" page will be displayed if accessing app which is not configured by the workspace --------- Signed-off-by: Yulong Ruan Signed-off-by: SuZhou-Joe Co-authored-by: SuZhou-Joe --- CHANGELOG.md | 1 + src/plugins/workspace/public/plugin.ts | 39 +++++++++++++- src/plugins/workspace/public/utils.test.ts | 60 +++++++++++++++++++++- src/plugins/workspace/public/utils.ts | 46 ++++++++++++++++- 4 files changed, 142 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 800b987d2f6b..7a1089042e7c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -84,6 +84,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Multiple Datasource] Add installedPlugins list to data source saved object ([#6348](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6348)) - [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)) ### 🐛 Bug Fixes diff --git a/src/plugins/workspace/public/plugin.ts b/src/plugins/workspace/public/plugin.ts index 5357aa08cbf7..b27c4b3bdd4a 100644 --- a/src/plugins/workspace/public/plugin.ts +++ b/src/plugins/workspace/public/plugin.ts @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import type { Subscription } from 'rxjs'; +import { BehaviorSubject, Subscription } from 'rxjs'; import React from 'react'; import { i18n } from '@osd/i18n'; import { @@ -12,6 +12,8 @@ import { CoreSetup, AppMountParameters, AppNavLinkStatus, + AppUpdater, + AppStatus, } from '../../../core/public'; import { WORKSPACE_FATAL_ERROR_APP_ID, @@ -26,6 +28,7 @@ import { WorkspaceClient } from './workspace_client'; import { SavedObjectsManagementPluginSetup } from '../../../plugins/saved_objects_management/public'; import { WorkspaceMenu } from './components/workspace_menu/workspace_menu'; import { getWorkspaceColumn } from './components/workspace_column'; +import { isAppAccessibleInWorkspace } from './utils'; type WorkspaceAppType = (params: AppMountParameters, services: Services) => () => void; @@ -36,6 +39,8 @@ interface WorkspacePluginSetupDeps { export class WorkspacePlugin implements Plugin<{}, {}, WorkspacePluginSetupDeps> { private coreStart?: CoreStart; private currentWorkspaceSubscription?: Subscription; + private currentWorkspaceIdSubscription?: Subscription; + private appUpdater$ = new BehaviorSubject(() => undefined); private _changeSavedObjectCurrentWorkspace() { if (this.coreStart) { return this.coreStart.workspaces.currentWorkspaceId$.subscribe((currentWorkspaceId) => { @@ -46,9 +51,34 @@ export class WorkspacePlugin implements Plugin<{}, {}, WorkspacePluginSetupDeps> } } + /** + * Filter nav links by the current workspace, once the current workspace change, the nav links(left nav bar) + * should also be updated according to the configured features of the current workspace + */ + private filterNavLinks(core: CoreStart) { + const currentWorkspace$ = core.workspaces.currentWorkspace$; + this.currentWorkspaceSubscription?.unsubscribe(); + + this.currentWorkspaceSubscription = currentWorkspace$.subscribe((currentWorkspace) => { + if (currentWorkspace) { + this.appUpdater$.next((app) => { + if (isAppAccessibleInWorkspace(app, currentWorkspace)) { + return; + } + /** + * Change the app to `inaccessible` if it is not configured in the workspace + * If trying to access such app, an "Application Not Found" page will be displayed + */ + return { status: AppStatus.inaccessible }; + }); + } + }); + } + public async setup(core: CoreSetup, { savedObjectsManagement }: WorkspacePluginSetupDeps) { const workspaceClient = new WorkspaceClient(core.http, core.workspaces); await workspaceClient.init(); + core.application.registerAppUpdater(this.appUpdater$); /** * Retrieve workspace id from url @@ -171,11 +201,16 @@ export class WorkspacePlugin implements Plugin<{}, {}, WorkspacePluginSetupDeps> public start(core: CoreStart) { this.coreStart = core; - this.currentWorkspaceSubscription = this._changeSavedObjectCurrentWorkspace(); + this.currentWorkspaceIdSubscription = this._changeSavedObjectCurrentWorkspace(); + + // When starts, filter the nav links based on the current workspace + this.filterNavLinks(core); + return {}; } public stop() { this.currentWorkspaceSubscription?.unsubscribe(); + this.currentWorkspaceIdSubscription?.unsubscribe(); } } diff --git a/src/plugins/workspace/public/utils.test.ts b/src/plugins/workspace/public/utils.test.ts index 510a775cd745..f81e248c4469 100644 --- a/src/plugins/workspace/public/utils.test.ts +++ b/src/plugins/workspace/public/utils.test.ts @@ -3,7 +3,8 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { featureMatchesConfig } from './utils'; +import { AppNavLinkStatus } from '../../../core/public'; +import { featureMatchesConfig, isAppAccessibleInWorkspace } from './utils'; describe('workspace utils: featureMatchesConfig', () => { it('feature configured with `*` should match any features', () => { @@ -91,3 +92,60 @@ describe('workspace utils: featureMatchesConfig', () => { ); }); }); + +describe('workspace utils: isAppAccessibleInWorkspace', () => { + it('any app is accessible when workspace has no features configured', () => { + expect( + isAppAccessibleInWorkspace( + { id: 'any_app', title: 'Any app', mount: jest.fn() }, + { id: 'workspace_id', name: 'workspace name' } + ) + ).toBe(true); + }); + + it('An app is accessible when the workspace has the app configured', () => { + expect( + isAppAccessibleInWorkspace( + { id: 'dev_tools', title: 'Any app', mount: jest.fn() }, + { id: 'workspace_id', name: 'workspace name', features: ['dev_tools'] } + ) + ).toBe(true); + }); + + it('An app is not accessible when the workspace does not have the app configured', () => { + expect( + isAppAccessibleInWorkspace( + { id: 'dev_tools', title: 'Any app', mount: jest.fn() }, + { id: 'workspace_id', name: 'workspace name', features: [] } + ) + ).toBe(false); + }); + + it('An app is accessible if the nav link is hidden', () => { + expect( + isAppAccessibleInWorkspace( + { + id: 'dev_tools', + title: 'Any app', + mount: jest.fn(), + navLinkStatus: AppNavLinkStatus.hidden, + }, + { id: 'workspace_id', name: 'workspace name', features: [] } + ) + ).toBe(true); + }); + + it('An app is accessible if it is chromeless', () => { + expect( + isAppAccessibleInWorkspace( + { + id: 'dev_tools', + title: 'Any app', + mount: jest.fn(), + chromeless: true, + }, + { id: 'workspace_id', name: 'workspace name', features: [] } + ) + ).toBe(true); + }); +}); diff --git a/src/plugins/workspace/public/utils.ts b/src/plugins/workspace/public/utils.ts index 444b3aadadf3..e70a26028525 100644 --- a/src/plugins/workspace/public/utils.ts +++ b/src/plugins/workspace/public/utils.ts @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { AppCategory } from '../../../core/public'; +import { App, AppCategory, AppNavLinkStatus, WorkspaceObject } from '../../../core/public'; /** * Checks if a given feature matches the provided feature configuration. @@ -25,6 +25,11 @@ export const featureMatchesConfig = (featureConfigs: string[]) => ({ }) => { let matched = false; + /** + * Iterate through each feature configuration to determine if the given feature matches any of them. + * Note: The loop will not break prematurely because the order of featureConfigs array matters. + * Later configurations may override previous ones, so each configuration must be evaluated in sequence. + */ for (const featureConfig of featureConfigs) { // '*' matches any feature if (featureConfig === '*') { @@ -55,3 +60,42 @@ export const featureMatchesConfig = (featureConfigs: string[]) => ({ return matched; }; + +/** + * Check if an app is accessible in a workspace based on the workspace configured features + */ +export function isAppAccessibleInWorkspace(app: App, workspace: WorkspaceObject) { + /** + * When workspace has no features configured, all apps are considered to be accessible + */ + if (!workspace.features) { + return true; + } + + /** + * The app is configured into a workspace, it is accessible after entering the workspace + */ + const featureMatcher = featureMatchesConfig(workspace.features); + if (featureMatcher({ id: app.id, category: app.category })) { + return true; + } + + /* + * An app with hidden nav link is not configurable by workspace, which means user won't be + * able to select/unselect it when configuring workspace features. Such apps are by default + * accessible when in a workspace. + */ + if (app.navLinkStatus === AppNavLinkStatus.hidden) { + return true; + } + + /** + * A chromeless app is not configurable by workspace, which means user won't be + * able to select/unselect it when configuring workspace features. Such apps are by default + * accessible when in a workspace. + */ + if (app.chromeless) { + return true; + } + return false; +} From 8151ef51725081b4fa100dd713ad13bb4d780548 Mon Sep 17 00:00:00 2001 From: Huy Nguyen <73027756+huyaboo@users.noreply.github.com> Date: Tue, 9 Apr 2024 22:45:49 -0700 Subject: [PATCH 5/8] [MDS] Update Data source selector behavior for defaultOption (#6372) * Refactor selector component changes Signed-off-by: Huy Nguyen <73027756+huyaboo@users.noreply.github.com> * Refactor selector Signed-off-by: Huy Nguyen <73027756+huyaboo@users.noreply.github.com> * Add unit tests for selector component Signed-off-by: Huy Nguyen <73027756+huyaboo@users.noreply.github.com> * Set initial state as [] to prevent page breaking Signed-off-by: Huy Nguyen <73027756+huyaboo@users.noreply.github.com> * Remove change Signed-off-by: Huy Nguyen <73027756+huyaboo@users.noreply.github.com> * Add changelog Signed-off-by: Huy Nguyen <73027756+huyaboo@users.noreply.github.com> * Update snapshots Signed-off-by: Huy Nguyen <73027756+huyaboo@users.noreply.github.com> * Update snapshots and fix tests Signed-off-by: Huy Nguyen <73027756+huyaboo@users.noreply.github.com> * Fix merge conflict Signed-off-by: Huy Nguyen <73027756+huyaboo@users.noreply.github.com> --------- Signed-off-by: Huy Nguyen <73027756+huyaboo@users.noreply.github.com> Co-authored-by: Lu Yu --- CHANGELOG.md | 1 + .../data_source_selector.test.tsx.snap | 38 ++- .../data_source_selector.test.tsx | 308 +++++++++++++++++- .../data_source_selector.tsx | 163 ++++++--- .../public/components/utils.ts | 2 +- 5 files changed, 441 insertions(+), 71 deletions(-) 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 From 7e1d94001049057ccf4daa7c83ad0638f3fb3ae0 Mon Sep 17 00:00:00 2001 From: Zhongnan Su Date: Wed, 10 Apr 2024 17:13:53 -0700 Subject: [PATCH 6/8] [MD] Add OpenSearch cluster group label to top of options in DataSourceSelectable dropdown (#6400) Signed-off-by: Zhongnan Su --- CHANGELOG.md | 1 + .../components/data_source_menu/types.ts | 5 +++ .../data_source_selectable.test.tsx.snap | 5 +++ .../data_source_selectable.test.tsx | 44 ++++++++++++++++++- .../data_source_selectable.tsx | 20 ++++++++- 5 files changed, 71 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 966d0f18e2b4..12391cc13614 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -86,6 +86,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [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)) +- [MD] Add OpenSearch cluster group label to top of single selectable dropdown ([#6400](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6400)) ### 🐛 Bug Fixes 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 32edbddf09a7..17aa35d8b8d0 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 @@ -16,6 +16,11 @@ export interface DataSourceOption { label?: string; } +export interface DataSourceGroupLabelOption extends DataSourceOption { + label: string; + isGroupLabel: true; +} + export interface DataSourceBaseConfig { fullWidth: boolean; disabled?: boolean; 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 adc2929a7bc8..851d37b3d71b 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 @@ -51,6 +51,11 @@ exports[`DataSourceSelectable should filter options if configured 1`] = ` onChange={[Function]} options={ Array [ + Object { + "id": "opensearchClusterGroupLabel", + "isGroupLabel": true, + "label": "OpenSearch cluster", + }, Object { "checked": "on", "id": "", 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 efa1215ed11a..78b63a57144c 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 @@ -7,11 +7,12 @@ import { ShallowWrapper, shallow, mount } from 'enzyme'; import { SavedObjectsClientContract } from '../../../../../core/public'; import { notificationServiceMock } from '../../../../../core/public/mocks'; import React from 'react'; -import { DataSourceSelectable } from './data_source_selectable'; +import { DataSourceSelectable, opensearchClusterGroupLabel } from './data_source_selectable'; import { AuthType } from '../../types'; import { getDataSourcesWithFieldsResponse, mockResponseForSavedObjectsCalls } from '../../mocks'; -import { getByRole, render } from '@testing-library/react'; +import { render } from '@testing-library/react'; import * as utils from '../utils'; +import { EuiSelectable } from '@elastic/eui'; describe('DataSourceSelectable', () => { let component: ShallowWrapper, React.Component<{}, {}, any>>; @@ -391,4 +392,43 @@ describe('DataSourceSelectable', () => { expect(onSelectedDataSource).toBeCalledWith([{ id: 'test2', label: 'test2' }]); expect(onSelectedDataSource).toHaveBeenCalled(); }); + + it('should render opensearch cluster group label at the top of options, when there are options availiable', async () => { + const onSelectedDataSource = jest.fn(); + component = shallow( + + ); + + component.instance().componentDidMount!(); + await nextTick(); + const optionsProp = component.find(EuiSelectable).prop('options'); + expect(optionsProp[0]).toEqual(opensearchClusterGroupLabel); + }); + + it('should not render opensearch cluster group label, when there is no option availiable', async () => { + const onSelectedDataSource = jest.fn(); + spyOn(utils, 'getDefaultDataSource').and.returnValue([]); + component = shallow( + + ); + + component.instance().componentDidMount!(); + await nextTick(); + const optionsProp = component.find(EuiSelectable).prop('options'); + expect(optionsProp).toEqual([]); + }); }); 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 88cee94cf6af..40a7edce7558 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 @@ -25,7 +25,7 @@ import { getDataSourcesWithFields, getDefaultDataSource, getFilteredDataSources import { LocalCluster } from '../data_source_selector/data_source_selector'; import { SavedObject } from '../../../../../core/public'; import { DataSourceAttributes } from '../../types'; -import { DataSourceOption } from '../data_source_menu/types'; +import { DataSourceGroupLabelOption, DataSourceOption } from '../data_source_menu/types'; interface DataSourceSelectableProps { savedObjectsClient: SavedObjectsClientContract; @@ -50,6 +50,12 @@ interface SelectedDataSourceOption extends DataSourceOption { checked?: string; } +export const opensearchClusterGroupLabel: DataSourceGroupLabelOption = { + id: 'opensearchClusterGroupLabel', + label: 'OpenSearch cluster', + isGroupLabel: true, +}; + export class DataSourceSelectable extends React.Component< DataSourceSelectableProps, DataSourceSelectableState @@ -207,6 +213,16 @@ export class DataSourceSelectable extends React.Component< } } + getOptionsWithGroupLabel = (dataSourceOptions: DataSourceOption[]): DataSourceOption[] => { + let optionsWithGroupLabel: DataSourceOption[] = []; + if (dataSourceOptions.length === 0) { + optionsWithGroupLabel = []; + } else { + optionsWithGroupLabel = [opensearchClusterGroupLabel, ...dataSourceOptions]; + } + return optionsWithGroupLabel; + }; + render() { const button = ( <> @@ -248,7 +264,7 @@ export class DataSourceSelectable extends React.Component< searchProps={{ placeholder: 'Search', }} - options={this.state.dataSourceOptions} + options={this.getOptionsWithGroupLabel(this.state.dataSourceOptions)} onChange={(newOptions) => this.onChange(newOptions)} singleSelection={true} data-test-subj={'dataSourceSelectable'} From cd857cb42e507e210f5b2ae2672bf251285e196f Mon Sep 17 00:00:00 2001 From: Tianle Huang <60111637+tianleh@users.noreply.github.com> Date: Wed, 10 Apr 2024 21:18:15 -0700 Subject: [PATCH 7/8] Improve dynamic configurations by adding cache and simplifying client fetch (#6364) * Improve dynamic config Signed-off-by: Tianle Huang * reset yml Signed-off-by: Tianle Huang * bring back previous changes Signed-off-by: Tianle Huang --------- Signed-off-by: Tianle Huang --- CHANGELOG.md | 1 + .../server/opensearch_config_client.test.ts | 106 ++++++++++++++--- .../server/opensearch_config_client.ts | 25 +++- .../application_config/server/plugin.test.ts | 108 +++++++++++++++++- .../application_config/server/plugin.ts | 22 +++- .../application_config/server/types.ts | 4 +- .../csp_handler/server/csp_handlers.test.ts | 7 ++ .../csp_handler/server/csp_handlers.ts | 6 +- 8 files changed, 249 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 12391cc13614..423a7554f07c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -86,6 +86,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [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)) +- [Dynamic Configurations] Improve dynamic configurations by adding cache and simplifying client fetch ([#6364](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6364)) - [MD] Add OpenSearch cluster group label to top of single selectable dropdown ([#6400](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6400)) ### 🐛 Bug Fixes diff --git a/src/plugins/application_config/server/opensearch_config_client.test.ts b/src/plugins/application_config/server/opensearch_config_client.test.ts index 827d309303cb..17b22dad7295 100644 --- a/src/plugins/application_config/server/opensearch_config_client.test.ts +++ b/src/plugins/application_config/server/opensearch_config_client.test.ts @@ -48,7 +48,9 @@ describe('OpenSearch Configuration Client', () => { }, }; - const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger); + const cache = {}; + + const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger, cache); const value = await client.getConfig(); @@ -77,7 +79,10 @@ describe('OpenSearch Configuration Client', () => { }), }, }; - const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger); + + const cache = {}; + + const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger, cache); await expect(client.getConfig()).rejects.toThrowError(ERROR_MESSAGE); }); @@ -99,11 +104,45 @@ describe('OpenSearch Configuration Client', () => { }, }; - const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger); + const cache = { + has: jest.fn().mockReturnValue(false), + set: jest.fn(), + }; + + const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger, cache); const value = await client.getEntityConfig('config1'); expect(value).toBe('value1'); + expect(cache.set).toBeCalledWith('config1', 'value1'); + }); + + it('return configuration value from cache', async () => { + const opensearchClient = { + asInternalUser: { + get: jest.fn().mockImplementation(() => { + return { + body: { + _source: { + value: 'value1', + }, + }, + }; + }), + }, + }; + + const cache = { + has: jest.fn().mockReturnValue(true), + get: jest.fn().mockReturnValue('cachedValue'), + }; + + const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger, cache); + + const value = await client.getEntityConfig('config1'); + + expect(value).toBe('cachedValue'); + expect(cache.get).toBeCalledWith('config1'); }); it('throws error when input is empty', async () => { @@ -121,7 +160,9 @@ describe('OpenSearch Configuration Client', () => { }, }; - const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger); + const cache = {}; + + const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger, cache); await expect(client.getEntityConfig(EMPTY_INPUT)).rejects.toThrowError( ERROR_MESSSAGE_FOR_EMPTY_INPUT @@ -151,9 +192,16 @@ describe('OpenSearch Configuration Client', () => { }, }; - const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger); + const cache = { + has: jest.fn().mockReturnValue(false), + set: jest.fn(), + }; + + const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger, cache); await expect(client.getEntityConfig('config1')).rejects.toThrowError(ERROR_MESSAGE); + + expect(cache.set).toBeCalledWith('config1', undefined); }); }); @@ -167,11 +215,16 @@ describe('OpenSearch Configuration Client', () => { }, }; - const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger); + const cache = { + del: jest.fn(), + }; + + const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger, cache); const value = await client.deleteEntityConfig('config1'); expect(value).toBe('config1'); + expect(cache.del).toBeCalledWith('config1'); }); it('throws error when input entity is empty', async () => { @@ -183,7 +236,9 @@ describe('OpenSearch Configuration Client', () => { }, }; - const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger); + const cache = {}; + + const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger, cache); await expect(client.deleteEntityConfig(EMPTY_INPUT)).rejects.toThrowError( ERROR_MESSSAGE_FOR_EMPTY_INPUT @@ -213,11 +268,16 @@ describe('OpenSearch Configuration Client', () => { }, }; - const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger); + const cache = { + del: jest.fn(), + }; + + const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger, cache); const value = await client.deleteEntityConfig('config1'); expect(value).toBe('config1'); + expect(cache.del).toBeCalledWith('config1'); }); it('return deleted document entity when deletion fails due to document not found', async () => { @@ -241,11 +301,16 @@ describe('OpenSearch Configuration Client', () => { }, }; - const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger); + const cache = { + del: jest.fn(), + }; + + const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger, cache); const value = await client.deleteEntityConfig('config1'); expect(value).toBe('config1'); + expect(cache.del).toBeCalledWith('config1'); }); it('throws error when opensearch throws error', async () => { @@ -271,7 +336,9 @@ describe('OpenSearch Configuration Client', () => { }, }; - const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger); + const cache = {}; + + const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger, cache); await expect(client.deleteEntityConfig('config1')).rejects.toThrowError(ERROR_MESSAGE); }); @@ -287,11 +354,16 @@ describe('OpenSearch Configuration Client', () => { }, }; - const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger); + const cache = { + set: jest.fn(), + }; + + const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger, cache); const value = await client.updateEntityConfig('config1', 'newValue1'); expect(value).toBe('newValue1'); + expect(cache.set).toBeCalledWith('config1', 'newValue1'); }); it('throws error when entity is empty ', async () => { @@ -303,7 +375,9 @@ describe('OpenSearch Configuration Client', () => { }, }; - const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger); + const cache = {}; + + const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger, cache); await expect(client.updateEntityConfig(EMPTY_INPUT, 'newValue1')).rejects.toThrowError( ERROR_MESSSAGE_FOR_EMPTY_INPUT @@ -319,7 +393,9 @@ describe('OpenSearch Configuration Client', () => { }, }; - const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger); + const cache = {}; + + const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger, cache); await expect(client.updateEntityConfig('config1', EMPTY_INPUT)).rejects.toThrowError( ERROR_MESSSAGE_FOR_EMPTY_INPUT @@ -349,7 +425,9 @@ describe('OpenSearch Configuration Client', () => { }, }; - const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger); + const cache = {}; + + const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger, cache); await expect(client.updateEntityConfig('config1', 'newValue1')).rejects.toThrowError( ERROR_MESSAGE diff --git a/src/plugins/application_config/server/opensearch_config_client.ts b/src/plugins/application_config/server/opensearch_config_client.ts index 9103919c396f..3a2c90147ade 100644 --- a/src/plugins/application_config/server/opensearch_config_client.ts +++ b/src/plugins/application_config/server/opensearch_config_client.ts @@ -3,8 +3,8 @@ * SPDX-License-Identifier: Apache-2.0 */ +import LRUCache from 'lru-cache'; import { IScopedClusterClient, Logger } from '../../../../src/core/server'; - import { ConfigurationClient } from './types'; import { validate } from './string_utils'; @@ -12,32 +12,45 @@ export class OpenSearchConfigurationClient implements ConfigurationClient { private client: IScopedClusterClient; private configurationIndexName: string; private readonly logger: Logger; + private cache: LRUCache; constructor( scopedClusterClient: IScopedClusterClient, configurationIndexName: string, - logger: Logger + logger: Logger, + cache: LRUCache ) { this.client = scopedClusterClient; this.configurationIndexName = configurationIndexName; this.logger = logger; + this.cache = cache; } async getEntityConfig(entity: string) { const entityValidated = validate(entity, this.logger); + if (this.cache.has(entityValidated)) { + return this.cache.get(entityValidated); + } + + this.logger.info(`Key ${entityValidated} is not found from cache.`); + try { const data = await this.client.asInternalUser.get({ index: this.configurationIndexName, id: entityValidated, }); + const value = data?.body?._source?.value; - return data?.body?._source?.value || ''; + this.cache.set(entityValidated, value); + + return value; } catch (e) { const errorMessage = `Failed to get entity ${entityValidated} due to error ${e}`; this.logger.error(errorMessage); + this.cache.set(entityValidated, undefined); throw e; } } @@ -55,6 +68,8 @@ export class OpenSearchConfigurationClient implements ConfigurationClient { }, }); + this.cache.set(entityValidated, newValueValidated); + return newValueValidated; } catch (e) { const errorMessage = `Failed to update entity ${entityValidated} with newValue ${newValueValidated} due to error ${e}`; @@ -74,15 +89,19 @@ export class OpenSearchConfigurationClient implements ConfigurationClient { id: entityValidated, }); + this.cache.del(entityValidated); + return entityValidated; } catch (e) { if (e?.body?.error?.type === 'index_not_found_exception') { this.logger.info('Attemp to delete a not found index.'); + this.cache.del(entityValidated); return entityValidated; } if (e?.body?.result === 'not_found') { this.logger.info('Attemp to delete a not found document.'); + this.cache.del(entityValidated); return entityValidated; } diff --git a/src/plugins/application_config/server/plugin.test.ts b/src/plugins/application_config/server/plugin.test.ts index e1ac45444c14..5390223f4d87 100644 --- a/src/plugins/application_config/server/plugin.test.ts +++ b/src/plugins/application_config/server/plugin.test.ts @@ -6,6 +6,11 @@ import { of } from 'rxjs'; import { ApplicationConfigPlugin } from './plugin'; import { ConfigurationClient } from './types'; +import LRUCache from 'lru-cache'; +import { OpenSearchConfigurationClient } from './opensearch_config_client'; + +jest.mock('lru-cache'); +jest.mock('./opensearch_config_client'); describe('application config plugin', () => { it('throws error when trying to register twice', async () => { @@ -54,8 +59,8 @@ describe('application config plugin', () => { setup.registerConfigurationClient(client1); - const scopedClient = {}; - expect(setup.getConfigurationClient(scopedClient)).toBe(client1); + const request = {}; + expect(setup.getConfigurationClient(request)).toBe(client1); const client2: ConfigurationClient = { getConfig: jest.fn(), @@ -71,6 +76,103 @@ describe('application config plugin', () => { 'Configuration client is already registered! Cannot register again!' ); - expect(setup.getConfigurationClient(scopedClient)).toBe(client1); + expect(setup.getConfigurationClient(request)).toBe(client1); + }); + + it('getConfigurationClient returns opensearch client when no external registration', async () => { + let capturedLRUCacheConstructorArgs = []; + + const cache = { + get: jest.fn(), + }; + + LRUCache.mockImplementation(function (...args) { + capturedLRUCacheConstructorArgs = args; + return cache; + }); + + let capturedConfigurationClientConstructorArgs = []; + + const client: ConfigurationClient = { + getConfig: jest.fn(), + getEntityConfig: jest.fn(), + updateEntityConfig: jest.fn(), + deleteEntityConfig: jest.fn(), + }; + + OpenSearchConfigurationClient.mockImplementation(function (...args) { + capturedConfigurationClientConstructorArgs = args; + return client; + }); + + const logger = { + info: jest.fn(), + error: jest.fn(), + }; + + const initializerContext = { + logger: { + get: jest.fn().mockReturnValue(logger), + }, + config: { + legacy: { + globalConfig$: of({ + opensearchDashboards: { + configIndex: '.osd_test', + }, + }), + }, + }, + }; + + const plugin = new ApplicationConfigPlugin(initializerContext); + + const coreSetup = { + http: { + createRouter: jest.fn().mockImplementation(() => { + return { + get: jest.fn(), + post: jest.fn(), + delete: jest.fn(), + }; + }), + }, + }; + + const setup = await plugin.setup(coreSetup); + + const scopedClient = { + asCurrentUser: jest.fn(), + }; + + const coreStart = { + opensearch: { + client: { + asScoped: jest.fn().mockReturnValue(scopedClient), + }, + }, + }; + + await plugin.start(coreStart); + + const request = {}; + + expect(setup.getConfigurationClient(request)).toBe(client); + + expect(capturedLRUCacheConstructorArgs).toEqual([ + { + max: 100, + maxAge: 600000, + }, + ]); + + expect(capturedConfigurationClientConstructorArgs).toEqual([ + scopedClient, + '.osd_test', + logger, + cache, + ]); + + expect(coreStart.opensearch.client.asScoped).toBeCalledTimes(1); }); }); diff --git a/src/plugins/application_config/server/plugin.ts b/src/plugins/application_config/server/plugin.ts index d0bd2ab42270..8536f7e134e3 100644 --- a/src/plugins/application_config/server/plugin.ts +++ b/src/plugins/application_config/server/plugin.ts @@ -6,14 +6,16 @@ import { Observable } from 'rxjs'; import { first } from 'rxjs/operators'; +import LRUCache from 'lru-cache'; import { PluginInitializerContext, CoreSetup, CoreStart, Plugin, Logger, - IScopedClusterClient, SharedGlobalConfig, + OpenSearchDashboardsRequest, + IClusterClient, } from '../../../core/server'; import { @@ -31,11 +33,20 @@ export class ApplicationConfigPlugin private configurationClient: ConfigurationClient; private configurationIndexName: string; + private clusterClient: IClusterClient; + + private cache: LRUCache; constructor(initializerContext: PluginInitializerContext) { this.logger = initializerContext.logger.get(); this.config$ = initializerContext.config.legacy.globalConfig$; this.configurationIndexName = ''; + this.clusterClient = null; + + this.cache = new LRUCache({ + max: 100, // at most 100 entries + maxAge: 10 * 60 * 1000, // 10 mins + }); } private registerConfigurationClient(configurationClient: ConfigurationClient) { @@ -50,15 +61,16 @@ export class ApplicationConfigPlugin this.configurationClient = configurationClient; } - private getConfigurationClient(scopedClusterClient: IScopedClusterClient): ConfigurationClient { + private getConfigurationClient(request?: OpenSearchDashboardsRequest): ConfigurationClient { if (this.configurationClient) { return this.configurationClient; } const openSearchConfigurationClient = new OpenSearchConfigurationClient( - scopedClusterClient, + this.clusterClient.asScoped(request), this.configurationIndexName, - this.logger + this.logger, + this.cache ); return openSearchConfigurationClient; @@ -81,6 +93,8 @@ export class ApplicationConfigPlugin } public start(core: CoreStart) { + this.clusterClient = core.opensearch.client; + return {}; } diff --git a/src/plugins/application_config/server/types.ts b/src/plugins/application_config/server/types.ts index 416d0258169e..c8039cf6cff3 100644 --- a/src/plugins/application_config/server/types.ts +++ b/src/plugins/application_config/server/types.ts @@ -3,10 +3,10 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { IScopedClusterClient, Headers } from 'src/core/server'; +import { Headers, OpenSearchDashboardsRequest } from 'src/core/server'; export interface ApplicationConfigPluginSetup { - getConfigurationClient: (inputOpenSearchClient: IScopedClusterClient) => ConfigurationClient; + getConfigurationClient: (request?: OpenSearchDashboardsRequest) => ConfigurationClient; registerConfigurationClient: (inputConfigurationClient: ConfigurationClient) => void; } // eslint-disable-next-line @typescript-eslint/no-empty-interface diff --git a/src/plugins/csp_handler/server/csp_handlers.test.ts b/src/plugins/csp_handler/server/csp_handlers.test.ts index d6c2f8a16d49..b185410f6174 100644 --- a/src/plugins/csp_handler/server/csp_handlers.test.ts +++ b/src/plugins/csp_handler/server/csp_handlers.test.ts @@ -55,6 +55,7 @@ describe('CSP handlers', () => { }); expect(configurationClient.getEntityConfig).toBeCalledTimes(1); + expect(getConfigurationClient).toBeCalledWith(request); }); it('do not add CSP headers when the client returns empty and CSP from YML already has frame-ancestors', async () => { @@ -89,6 +90,7 @@ describe('CSP handlers', () => { expect(toolkit.next).toHaveBeenCalledWith({}); expect(configurationClient.getEntityConfig).toBeCalledTimes(1); + expect(getConfigurationClient).toBeCalledWith(request); }); it('add frame-ancestors CSP headers when the client returns empty and CSP from YML has no frame-ancestors', async () => { @@ -128,6 +130,7 @@ describe('CSP handlers', () => { }); expect(configurationClient.getEntityConfig).toBeCalledTimes(1); + expect(getConfigurationClient).toBeCalledWith(request); }); it('do not add CSP headers when the configuration does not exist and CSP from YML already has frame-ancestors', async () => { @@ -164,6 +167,7 @@ describe('CSP handlers', () => { expect(toolkit.next).toBeCalledWith({}); expect(configurationClient.getEntityConfig).toBeCalledTimes(1); + expect(getConfigurationClient).toBeCalledWith(request); }); it('add frame-ancestors CSP headers when the configuration does not exist and CSP from YML has no frame-ancestors', async () => { @@ -200,6 +204,7 @@ describe('CSP handlers', () => { }); expect(configurationClient.getEntityConfig).toBeCalledTimes(1); + expect(getConfigurationClient).toBeCalledWith(request); }); it('do not add CSP headers when request dest exists and shall skip', async () => { @@ -235,6 +240,7 @@ describe('CSP handlers', () => { expect(toolkit.next).toBeCalledWith({}); expect(configurationClient.getEntityConfig).toBeCalledTimes(0); + expect(getConfigurationClient).toBeCalledTimes(0); }); it('do not add CSP headers when request dest does not exist', async () => { @@ -269,5 +275,6 @@ describe('CSP handlers', () => { expect(toolkit.next).toBeCalledWith({}); expect(configurationClient.getEntityConfig).toBeCalledTimes(0); + expect(getConfigurationClient).toBeCalledTimes(0); }); }); diff --git a/src/plugins/csp_handler/server/csp_handlers.ts b/src/plugins/csp_handler/server/csp_handlers.ts index 3bfa90115518..1a76ed942460 100644 --- a/src/plugins/csp_handler/server/csp_handlers.ts +++ b/src/plugins/csp_handler/server/csp_handlers.ts @@ -30,7 +30,7 @@ const CSP_RULES_CONFIG_KEY = 'csp.rules'; export function createCspRulesPreResponseHandler( core: CoreSetup, cspHeader: string, - getConfigurationClient: (scopedClusterClient: IScopedClusterClient) => ConfigurationClient, + getConfigurationClient: (request?: OpenSearchDashboardsRequest) => ConfigurationClient, logger: Logger ): OnPreResponseHandler { return async ( @@ -47,9 +47,7 @@ export function createCspRulesPreResponseHandler( return toolkit.next({}); } - const [coreStart] = await core.getStartServices(); - - const client = getConfigurationClient(coreStart.opensearch.client.asScoped(request)); + const client = getConfigurationClient(request); const cspRules = await client.getEntityConfig(CSP_RULES_CONFIG_KEY, { headers: request.headers, From 46b17e4bb96e8b1293b23ac12a3ba739c2c83701 Mon Sep 17 00:00:00 2001 From: Anan Zhuang Date: Wed, 10 Apr 2024 23:27:54 -0700 Subject: [PATCH 8/8] [OSCI] Removed KUI usage in visualizations -- Completion of Original PR (#6360) This PR completes https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5462 Please refer to the above PR for more details Signed-off-by: Anan Zhuang --- CHANGELOG.md | 1 + .../public/doc_links/doc_links_service.ts | 5 +++ .../embeddable/disabled_lab_visualization.tsx | 44 ++++++++++++------- src/plugins/visualizations/public/plugin.ts | 4 ++ src/plugins/visualizations/public/services.ts | 3 ++ 5 files changed, 40 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 423a7554f07c..6e713d7caaf1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -144,6 +144,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Remove unused Sass in `tile_map` plugin ([#4110](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4110)) - [Multiple Datasource] Move data source selectable to its own folder, fix test and a few type errors for data source selectable component ([#6287](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6287)) +- Remove KUI usage in `disabled_lab_visualization` ([#5462](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5462)) ### 🔩 Tests diff --git a/src/core/public/doc_links/doc_links_service.ts b/src/core/public/doc_links/doc_links_service.ts index d73a663a64b3..41f52b7dc46a 100644 --- a/src/core/public/doc_links/doc_links_service.ts +++ b/src/core/public/doc_links/doc_links_service.ts @@ -424,6 +424,10 @@ export class DocLinksService { // https://opensearch.org/docs/latest/dashboards/visualize/viz-index/ guide: `${OPENSEARCH_WEBSITE_DOCS}visualize/viz-index/`, }, + management: { + // https://opensearch.org/docs/latest/dashboards/management/advanced-settings/ + advancedSettings: `${OPENSEARCH_DASHBOARDS_VERSIONED_DOCS}management/advanced-settings/`, + }, }, noDocumentation: { auditbeat: `${OPENSEARCH_WEBSITE_DOCS}tools/index/#downloads`, @@ -819,6 +823,7 @@ export interface DocLinksStart { readonly guide: string; }; readonly visualize: Record; + readonly management: Record; }; readonly noDocumentation: { readonly auditbeat: string; diff --git a/src/plugins/visualizations/public/embeddable/disabled_lab_visualization.tsx b/src/plugins/visualizations/public/embeddable/disabled_lab_visualization.tsx index 79d6369fa88d..7088fdd3b7ef 100644 --- a/src/plugins/visualizations/public/embeddable/disabled_lab_visualization.tsx +++ b/src/plugins/visualizations/public/embeddable/disabled_lab_visualization.tsx @@ -28,29 +28,39 @@ * under the License. */ +import React, { Fragment } from 'react'; import { FormattedMessage } from '@osd/i18n/react'; -import React from 'react'; +import { EuiEmptyPrompt, EuiLink, EuiText } from '@elastic/eui'; +import { getDocLinks } from '../services'; +import './_visualize_lab_disabled.scss'; export function DisabledLabVisualization({ title }: { title: string }) { + const docLinks = getDocLinks(); + const advancedSettingsLink = docLinks.links.opensearchDashboards.management.advancedSettings; return (
- ); } diff --git a/src/plugins/visualizations/public/plugin.ts b/src/plugins/visualizations/public/plugin.ts index 3542e0cc26ff..bd255b82c6af 100644 --- a/src/plugins/visualizations/public/plugin.ts +++ b/src/plugins/visualizations/public/plugin.ts @@ -63,6 +63,7 @@ import { setSavedSearchLoader, setEmbeddable, setNotifications, + setDocLinks, } from './services'; import { VISUALIZE_EMBEDDABLE_TYPE, @@ -96,6 +97,7 @@ import { import { createSavedSearchesLoader } from '../../discover/public'; import { DashboardStart } from '../../dashboard/public'; import { createSavedAugmentVisLoader } from '../../vis_augmenter/public'; +import { DocLinksStart } from '../../../core/public'; /** * Interface for this plugin's returned setup/start contracts. @@ -133,6 +135,7 @@ export interface VisualizationsStartDeps { getAttributeService: DashboardStart['getAttributeService']; savedObjectsClient: SavedObjectsClientContract; notifications: NotificationsStart; + docLinks: DocLinksStart; } /** @@ -224,6 +227,7 @@ export class VisualizationsPlugin }); setSavedSearchLoader(savedSearchLoader); setNotifications(core.notifications); + setDocLinks(core.docLinks); return { ...types, showNewVisModal, diff --git a/src/plugins/visualizations/public/services.ts b/src/plugins/visualizations/public/services.ts index a99a7010af28..62266ff2ba53 100644 --- a/src/plugins/visualizations/public/services.ts +++ b/src/plugins/visualizations/public/services.ts @@ -54,6 +54,7 @@ import { SavedVisualizationsLoader } from './saved_visualizations'; import { SavedObjectLoader } from '../../saved_objects/public'; import { EmbeddableStart } from '../../embeddable/public'; import { SavedObjectLoaderAugmentVis } from '../../vis_augmenter/public'; +import { DocLinksStart } from '../../../core/public'; export const [getUISettings, setUISettings] = createGetterSetter('UISettings'); @@ -116,3 +117,5 @@ export const [getNotifications, setNotifications] = createGetterSetter('savedAugmentVisLoader'); + +export const [getDocLinks, setDocLinks] = createGetterSetter('docLinks');