Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Discover - datasource selector] Add extension group title and modal #5815

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- [Discover] Enhanced the data source selector with added sorting functionality ([#5719](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5719))
- [Multiple Datasource] Add datasource picker component and use it in devtools and tutorial page when multiple datasource is enabled ([#5756](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5756))
- [Multiple Datasource] Add datasource picker to import saved object flyout when multiple data source is enabled ([#5781](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5781))
- [Discover] Add extension group title to non-index data source groups to indicate log explorer redirection in discover data source selector. ([#5815](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5815))

### 🐛 Bug Fixes

Expand Down
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,7 @@
"@types/react-router-dom": "^5.3.2",
"@types/react-virtualized": "^9.18.7",
"@types/recompose": "^0.30.6",
"@types/redux-mock-store": "^1.0.6",
"@types/selenium-webdriver": "^4.0.9",
"@types/semver": "^7.5.0",
"@types/sinon": "^7.0.13",
Expand Down Expand Up @@ -451,6 +452,7 @@
"react-test-renderer": "^16.12.0",
"reactcss": "1.2.3",
"redux": "^4.0.5",
"redux-mock-store": "^1.5.4",
"regenerate": "^1.4.0",
"reselect": "^4.0.0",
"resize-observer-polyfill": "^1.5.1",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,17 @@
type DataSourceTypeKey = 'DEFAULT_INDEX_PATTERNS' | 's3glue' | 'spark';

// Mapping between datasource type and its display name.
// Temporary solution, will be removed along with refactoring of data source APIs
const DATASOURCE_TYPE_DISPLAY_NAME_MAP: Record<DataSourceTypeKey, string> = {
DEFAULT_INDEX_PATTERNS: 'Index patterns',
s3glue: 'Amazon S3',
spark: 'Spark',
DEFAULT_INDEX_PATTERNS: i18n.translate('dataExplorer.dataSourceSelector.indexPatternGroupTitle', {
defaultMessage: 'Index patterns',
}),
s3glue: i18n.translate('dataExplorer.dataSourceSelector.amazonS3GroupTitle', {
defaultMessage: 'Amazon S3',
}),
spark: i18n.translate('dataExplorer.dataSourceSelector.sparkGroupTitle', {
defaultMessage: 'Spark',
}),
};

type DataSetType = ISourceDataSet['data_sets'][number];
Expand Down Expand Up @@ -67,7 +74,19 @@
const finalList = [] as DataSourceGroup[];
allDataSets.forEach((curDataSet) => {
const typeKey = curDataSet.ds.getType() as DataSourceTypeKey;
const groupName = DATASOURCE_TYPE_DISPLAY_NAME_MAP[typeKey] || 'Default Group';
let groupName =
DATASOURCE_TYPE_DISPLAY_NAME_MAP[typeKey] ||
i18n.translate('dataExplorer.dataSourceSelector.defaultGroupTitle', {
defaultMessage: 'Default Group',
});

// add '- Opens in Log Explorer' to hint user that selecting these types of data sources
// will lead to redirection to log explorer
if (typeKey !== 'DEFAULT_INDEX_PATTERNS') {
groupName = `${groupName}${i18n.translate('dataExplorer.dataSourceSelector.redirectionHint', {

Check warning on line 86 in src/plugins/data/public/data_sources/datasource_selector/datasource_selectable.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/public/data_sources/datasource_selector/datasource_selectable.tsx#L86

Added line #L86 was not covered by tests
defaultMessage: ' - Opens in Log Explorer',
})}`;
}

const existingGroup = finalList.find((item) => item.label === groupName);
const mappedOptions = curDataSet.data_sets.map((dataSet) =>
Expand Down
110 changes: 110 additions & 0 deletions src/plugins/data_explorer/public/components/sidebar/index.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

import React from 'react';
import { render, fireEvent, waitFor } from '@testing-library/react';
import { Sidebar } from './index'; // Adjust the import path as necessary
import { Provider } from 'react-redux';
import configureMockStore from 'redux-mock-store';
import { MockS3DataSource } from '../../../../discover/public/__mock__/index.test.mock';
import { createMemoryHistory } from 'history';
import { Router } from 'react-router-dom';

const mockStore = configureMockStore();
const initialState = {
metadata: { indexPattern: 'some-index-pattern-id' },
};
const store = mockStore(initialState);

jest.mock('../../../../opensearch_dashboards_react/public', () => {
return {
toMountPoint: jest.fn().mockImplementation((component) => () => component),
useOpenSearchDashboards: jest.fn().mockReturnValue({
services: {
data: {
indexPatterns: {},
dataSources: {
dataSourceService: {
dataSources$: {
subscribe: jest.fn((callback) => {
callback({
's3-prod-mock': new MockS3DataSource({
name: 's3-prod-mock',
type: 's3glue',
metadata: {},
}),
});
return { unsubscribe: jest.fn() };
}),
},
},
},
},
notifications: {
toasts: {
addError: jest.fn(),
},
},
application: {
navigateToUrl: jest.fn(),
},
overlays: {
openConfirm: jest.fn(),
},
},
}),
withOpenSearchDashboards: () => (Component: React.ComponentClass) => (props: any) => (
<Component {...props} />
),
};
});

jest.mock('../../../../data_explorer/public', () => ({
useTypedSelector: jest.fn(),
useTypedDispatch: jest.fn(),
}));

describe('Sidebar Component', () => {
it('renders without crashing', () => {
const { container, getByTestId } = render(
<Provider store={store}>
<Sidebar />
</Provider>
);
expect(container).toBeInTheDocument();
expect(getByTestId('dataExplorerDSSelect')).toBeInTheDocument();
});

it('shows title extensions on the non-index pattern data source', () => {
const { getByText, getByTestId } = render(
<Provider store={store}>
<Sidebar />
</Provider>
);

fireEvent.click(getByTestId('comboBoxToggleListButton'));
waitFor(() => {
expect(getByText('Open in Log Explorer')).toBeInTheDocument();
});
});

it('redirects to log explorer when clicking open-in-log-explorer button', () => {
const history = createMemoryHistory();
const { getByText, getByTestId } = render(
<Provider store={store}>
<Router history={history}>
<Sidebar />
</Router>
</Provider>
);

fireEvent.click(getByTestId('comboBoxToggleListButton'));
waitFor(() => {
expect(getByText('s3-prod-mock')).toBeInTheDocument();
fireEvent.click(getByText('s3-prod-mock'));
expect(history.location.pathname).toContain('observability-logs#/explorer');
});
});
});
20 changes: 14 additions & 6 deletions src/plugins/data_explorer/public/components/sidebar/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,23 +59,31 @@ export const Sidebar: FC = ({ children }) => {
}
}, [indexPatternId, activeDataSources, dataSourceOptionList]);

const redirectToLogExplorer = useCallback(
(dsName: string, dsType: string) => {
return application.navigateToUrl(
`../observability-logs#/explorer?datasourceName=${dsName}&datasourceType=${dsType}`
SuZhou-Joe marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this assume that the plugin handling observability-logs is always installed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The s3 datasource with this redirection is for now only be able to be registered through observability plugin. Therefore it can only see s3 datasources when observability (log explorer as part of it) presented.

Copy link
Collaborator

@AMoo-Miki AMoo-Miki Mar 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if I have an S3 DS configured and then connect to the cluster using an OSD that doesn't have the observability plugin? On the surface, I think it will show the DS and attempt to redirect. This should check if the plugin is available and if not, fail to redirect, and also disable selecting the DS.

PS, I think due to rareness of this coming up, this can be filed as an issue for enhancing this; it is not a blocker.

Copy link
Collaborator Author

@mengweieric mengweieric Mar 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, currently, there is no way to register s3 datasources outside of Observability plugin. Soon in next release probably, discover will be the only data exploration interface for s3 datasources where this temporary redirection will be removed. I feel it should also make sense to users that whatever datasource showed up in discover datasource selector is supported and available through using discover to avoid confusion.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's make sure we have an issue to track this @mengweieric

);
},
[application]
);

const handleSourceSelection = useCallback(
(selectedDataSources: DataSourceOption[]) => {
if (selectedDataSources.length === 0) {
setSelectedSources(selectedDataSources);
return;
}
// Temporary redirection solution for 2.11, where clicking non-index-pattern datasource
// will redirect user to Observability event explorer
// Temporary redirection solution for 2.11, where clicking non-index-pattern data sources
// will prompt users with modal explaining they are being redirected to Observability log explorer
if (selectedDataSources[0]?.ds?.getType() !== 'DEFAULT_INDEX_PATTERNS') {
return application.navigateToUrl(
`../observability-logs#/explorer?datasourceName=${selectedDataSources[0].label}&datasourceType=${selectedDataSources[0].type}`
);
redirectToLogExplorer(selectedDataSources[0].label, selectedDataSources[0].type);
return;
}
setSelectedSources(selectedDataSources);
dispatch(setIndexPattern(selectedDataSources[0].value));
},
[application, dispatch]
[dispatch, redirectToLogExplorer, setSelectedSources]
);

const handleGetDataSetError = useCallback(
Expand Down
28 changes: 28 additions & 0 deletions src/plugins/discover/public/__mock__/index.test.mock.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

export class MockS3DataSource {
protected name: string;
protected type: string;
protected metadata: any;

constructor({ name, type, metadata }: { name: string; type: string; metadata: any }) {
this.name = name;
this.type = type;
this.metadata = metadata;
}

async getDataSet(dataSetParams?: any) {
return [this.name];
}

getName() {
return this.name;
}

getType() {
return this.type;
}
}
14 changes: 14 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3904,6 +3904,13 @@
dependencies:
"@types/react" "*"

"@types/redux-mock-store@^1.0.6":
version "1.0.6"
resolved "https://registry.yarnpkg.com/@types/redux-mock-store/-/redux-mock-store-1.0.6.tgz#0a03b2655028b7cf62670d41ac1de5ca1b1f5958"
integrity sha512-eg5RDfhJTXuoJjOMyXiJbaDb1B8tfTaJixscmu+jOusj6adGC0Krntz09Tf4gJgXeCqCrM5bBMd+B7ez0izcAQ==
dependencies:
redux "^4.0.5"

"@types/refractor@^3.0.0":
version "3.0.2"
resolved "https://registry.yarnpkg.com/@types/refractor/-/refractor-3.0.2.tgz#2d42128d59f78f84d2c799ffc5ab5cadbcba2d82"
Expand Down Expand Up @@ -15472,6 +15479,13 @@ redent@^3.0.0:
indent-string "^4.0.0"
strip-indent "^3.0.0"

redux-mock-store@^1.5.4:
version "1.5.4"
resolved "https://registry.yarnpkg.com/redux-mock-store/-/redux-mock-store-1.5.4.tgz#90d02495fd918ddbaa96b83aef626287c9ab5872"
integrity sha512-xmcA0O/tjCLXhh9Fuiq6pMrJCwFRaouA8436zcikdIpYWWCjU76CRk+i2bHx8EeiSiMGnB85/lZdU3wIJVXHTA==
dependencies:
lodash.isplainobject "^4.0.6"

redux-thunk@^2.3.0, redux-thunk@^2.4.1:
version "2.4.1"
resolved "https://registry.yarnpkg.com/redux-thunk/-/redux-thunk-2.4.1.tgz#0dd8042cf47868f4b29699941de03c9301a75714"
Expand Down
Loading