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

[Datasource selector] Sort datasource option list alphabetically #5719

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 @@ -30,6 +30,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- [Discover] Added customizable pagination options based on Discover UI settings [#5610](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5610)
- [Chrome] Introduce registerCollapsibleNavHeader to allow plugins to customize the rendering of nav menu header ([#5244](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5244))
- [Custom Branding] Relative URL should be allowed for logos ([#5572](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5572))
- [Discover] Enhanced the data source selector with added sorting functionality ([#5609](https://github.com/opensearch-project/OpenSearch-Dashboards/issues/5609))

### 🐛 Bug Fixes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/

import React from 'react';
import { render, act } from '@testing-library/react';
import { render, act, screen, fireEvent } from '@testing-library/react';
import { DataSourceSelectable } from './datasource_selectable';
import { DataSourceType, GenericDataSource } from '../datasource_services';
import { DataSourceGroup, DataSourceOption } from './types';
Expand Down Expand Up @@ -81,4 +81,104 @@ describe('DataSourceSelectable', () => {

expect(onFetchDataSetErrorMock).toHaveBeenCalledWith(new Error('Fetch error'));
});

it('should sort index patterns list alphabetically', async () => {
const mockDataSourceOptionList = [
{
label: 'Index patterns',
options: [
{ label: 'logstash-*' },
{ label: '000-*' },
{ label: 'p000-1' },
{ label: 'pattern_archive' },
{ label: 'index_main' },
{ label: 'index-2024' },
],
},
] as any;

render(
<DataSourceSelectable
dataSources={[
({
getDataSet: jest.fn().mockResolvedValue([]),
getType: jest.fn().mockReturnValue('DEFAULT_INDEX_PATTERNS'),
getName: jest.fn().mockReturnValue('Index patterns'),
} as unknown) as DataSourceType,
]}
dataSourceOptionList={mockDataSourceOptionList}
selectedSources={selectedSourcesMock}
onDataSourceSelect={setSelectedSourcesMock}
setDataSourceOptionList={setDataSourceOptionListMock}
onGetDataSetError={onFetchDataSetErrorMock}
/>
);

const button = screen.getByLabelText('Open list of options');
fireEvent.click(button);
expect(
screen.getByTestId('comboBoxOptionsList dataExplorerDSSelect-optionsList')
).toBeInTheDocument();
const defaultDSOptions = document.querySelectorAll('.euiComboBoxOption__content');
const optionTexts = Array.from(defaultDSOptions).map((option) => option.innerHTML);
const expectedIndexPatternSortedOrder = [
'000-*',
'index_main',
'index-2024',
'logstash-*',
'p000-1',
'pattern_archive',
];
expect(optionTexts).toEqual(expectedIndexPatternSortedOrder);
});

it('should sort non index patterns list alphabetically', async () => {
const mockDataSourceOptionList = [
{
label: 'Amazon S3',
options: [
{ label: 'mys3' },
{ label: '*starred' },
{ label: 'alpha-test-s3' },
{ label: '@special' },
{ label: 's3-2024' },
{ label: 'S3_Archive' },
],
},
] as any;

render(
<DataSourceSelectable
dataSources={[
({
getDataSet: jest.fn().mockResolvedValue([]),
getType: jest.fn().mockReturnValue('s3glue'),
getName: jest.fn().mockReturnValue('Amazon S3'),
} as unknown) as DataSourceType,
]}
dataSourceOptionList={mockDataSourceOptionList}
selectedSources={selectedSourcesMock}
onDataSourceSelect={setSelectedSourcesMock}
setDataSourceOptionList={setDataSourceOptionListMock}
onGetDataSetError={onFetchDataSetErrorMock}
/>
);

const button = screen.getByLabelText('Open list of options');
fireEvent.click(button);
expect(
screen.getByTestId('comboBoxOptionsList dataExplorerDSSelect-optionsList')
).toBeInTheDocument();
const defaultDSOptions = document.querySelectorAll('.euiComboBoxOption__content');
const optionTexts = Array.from(defaultDSOptions).map((option) => option.innerHTML);
const expectedIndexPatternSortedOrder = [
'@special',
'*starred',
'alpha-test-s3',
'mys3',
'S3_Archive',
's3-2024',
];
expect(optionTexts).toEqual(expectedIndexPatternSortedOrder);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import React, { useEffect, useCallback } from 'react';
import React, { useEffect, useCallback, useMemo } from 'react';
import { EuiComboBox } from '@elastic/eui';
import { i18n } from '@osd/i18n';
import { ISourceDataSet, IndexPatternOption } from '../datasource';
Expand Down Expand Up @@ -118,13 +118,24 @@ export const DataSourceSelectable = ({
[onDataSourceSelect]
);

const memorizedDataSourceOptionList = useMemo(() => {
return dataSourceOptionList.map((dsGroup: DataSourceGroup) => {
return {
...dsGroup,
options: [...dsGroup.options].sort((ds1, ds2) => {
return ds1.label.localeCompare(ds2.label, undefined, { sensitivity: 'base' });
Copy link
Member

Choose a reason for hiding this comment

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

base was the original behavior in 2.9?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It uses lodash's sortBy without any addition settings which I believe is not using 'base' as I tested that for example 'a' vs 'A' will be sorted in different groups. For index patterns it's fine as there will be no Uppercase allowed plus a list of restrictions, but since we also introduced non index pattern datasource with some different, flexible naming restrictions, I figured this way should have the better support.

}),
};
});
}, [dataSourceOptionList]);

return (
<EuiComboBox
data-test-subj="dataExplorerDSSelect"
placeholder={i18n.translate('data.datasource.selectADatasource', {
defaultMessage: 'Select a datasource',
})}
options={dataSourceOptionList as any}
options={memorizedDataSourceOptionList as any}
selectedOptions={selectedSources as any}
onChange={handleSourceChange}
singleSelection={singleSelection}
Expand Down
Loading