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

[Backport 2.x] [bug] address different issues with dataset selector #8667

Closed
wants to merge 1 commit into from
Closed
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
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,7 @@
import { IndexPatternsContract } from './index_patterns';
import { UiSettingsCommon } from '../types';

export type EnsureDefaultIndexPattern = (
shouldRedirect?: boolean
) => Promise<unknown | void> | undefined;
export type EnsureDefaultIndexPattern = () => Promise<unknown | void> | undefined;

export const createEnsureDefaultIndexPattern = (
uiSettings: UiSettingsCommon,
Expand All @@ -44,10 +42,7 @@
* Checks whether a default index pattern is set and exists and defines
* one otherwise.
*/
return async function ensureDefaultIndexPattern(
this: IndexPatternsContract,
shouldRedirect: boolean = true
) {
return async function ensureDefaultIndexPattern(this: IndexPatternsContract) {
const patterns = await this.getIds();
let defaultId = await uiSettings.get('defaultIndex');
let defined = !!defaultId;
Expand All @@ -67,6 +62,8 @@
defaultId = patterns[0];
await uiSettings.set('defaultIndex', defaultId);
} else {
const isEnhancementsEnabled = await uiSettings.get('query:enhancements:enabled');
const shouldRedirect = !isEnhancementsEnabled;

Check warning on line 66 in src/plugins/data/common/index_patterns/index_patterns/ensure_default_index_pattern.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/common/index_patterns/index_patterns/ensure_default_index_pattern.ts#L65-L66

Added lines #L65 - L66 were not covered by tests
if (shouldRedirect) return onRedirectNoIndexPattern();
else return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ export const getDQLLanguageConfig = (
visualizable: true,
},
showDocLinks: true,
docLink: {
title: i18n.translate('data.dqlLanguage.docLink', {
defaultMessage: 'DQL documentation',
}),
url: 'https://opensearch.org/docs/latest/query-dsl/full-text/query-string/',
},
editorSupportedAppNames: ['discover'],
supportedAppNames: ['discover', 'dashboards', 'visualize', 'data-explorer', 'vis-builder', '*'],
sampleQueries: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ export const getLuceneLanguageConfig = (
visualizable: true,
},
showDocLinks: true,
docLink: {
title: i18n.translate('data.luceneLanguage.docLink', {
defaultMessage: 'Lucene documentation',
}),
url: 'https://opensearch.org/docs/latest/query-dsl/full-text/query-string/',
},
editorSupportedAppNames: ['discover'],
supportedAppNames: ['discover', 'dashboards', 'visualize', 'data-explorer', 'vis-builder', '*'],
sampleQueries: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ export interface LanguageConfig {
visualizable?: boolean;
};
showDocLinks?: boolean;
docLink?: {
title: string;
url: string;
};
editorSupportedAppNames?: string[];
supportedAppNames?: string[];
hideDatePicker?: boolean;
Expand Down
1 change: 0 additions & 1 deletion src/plugins/data/public/ui/_index.scss
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
@import "./common";
@import "./filter_bar/index";
@import "./typeahead/index";
@import "./saved_query_management/index";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,27 +13,19 @@
} from '../../../common';
import { DatasetExplorer } from './dataset_explorer';
import { Configurator } from './configurator';
import { getQueryService } from '../../services';
import { IDataPluginServices } from '../../types';

export const AdvancedSelector = ({
services,
onSelect,
onCancel,
selectedDataset,
setSelectedDataset,
setIndexPattern,
direct = false,
}: {
services: IDataPluginServices;
onSelect: (dataset: Dataset) => void;
onCancel: () => void;
selectedDataset?: Dataset;
setSelectedDataset: (data: Dataset | undefined) => void;
setIndexPattern: (id: string | undefined) => void;
direct?: boolean;
}) => {
const queryService = services.data.query;
const queryString = queryService.queryString;
const queryString = getQueryService().queryString;

Check warning on line 28 in src/plugins/data/public/ui/dataset_selector/advanced_selector.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/public/ui/dataset_selector/advanced_selector.tsx#L28

Added line #L28 was not covered by tests

const [path, setPath] = useState<DataStructure[]>([
{
Expand All @@ -56,38 +48,22 @@
}),
},
]);
const [selectedDataset, setSelectedDataset] = useState<BaseDataset | undefined>();

Check warning on line 51 in src/plugins/data/public/ui/dataset_selector/advanced_selector.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/public/ui/dataset_selector/advanced_selector.tsx#L51

Added line #L51 was not covered by tests

const [currentSelectedDataset, setCurrentSelectedDataset] = useState<BaseDataset | undefined>(
selectedDataset
);

return currentSelectedDataset ? (
return selectedDataset ? (
<Configurator
baseDataset={currentSelectedDataset}
baseDataset={selectedDataset}
onConfirm={onSelect}
onCancel={onCancel}
onPrevious={() => {
setSelectedDataset(undefined);
setCurrentSelectedDataset(undefined);
}}
queryService={queryService}
onPrevious={() => setSelectedDataset(undefined)}

Check warning on line 58 in src/plugins/data/public/ui/dataset_selector/advanced_selector.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/public/ui/dataset_selector/advanced_selector.tsx#L58

Added line #L58 was not covered by tests
/>
) : (
<DatasetExplorer
services={services}
queryString={queryString}
path={path}
setPath={setPath}
onNext={(dataset) => {
setSelectedDataset(dataset);
setIndexPattern(dataset.id);
setCurrentSelectedDataset(dataset);
if (direct) {
const query = queryString.getInitialQueryByDataset(dataset);
queryString.setQuery(query);
queryString.getDatasetService().addRecentDataset(dataset);
}
}}
onNext={(dataset) => setSelectedDataset(dataset)}

Check warning on line 66 in src/plugins/data/public/ui/dataset_selector/advanced_selector.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/public/ui/dataset_selector/advanced_selector.tsx#L66

Added line #L66 was not covered by tests
onCancel={onCancel}
/>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,20 @@
import { FormattedMessage } from '@osd/i18n/react';
import React, { useEffect, useMemo, useState } from 'react';
import { BaseDataset, DEFAULT_DATA, Dataset, DatasetField } from '../../../common';
import { getIndexPatterns } from '../../services';
import { getIndexPatterns, getQueryService } from '../../services';

export const Configurator = ({
baseDataset,
onConfirm,
onCancel,
onPrevious,
queryService,
}: {
baseDataset: BaseDataset;
onConfirm: (dataset: Dataset) => void;
onCancel: () => void;
onPrevious: () => void;
queryService: any;
}) => {
const queryService = getQueryService();

Check warning on line 36 in src/plugins/data/public/ui/dataset_selector/configurator.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/public/ui/dataset_selector/configurator.tsx#L36

Added line #L36 was not covered by tests
const queryString = queryService.queryString;
const languageService = queryService.queryString.getLanguageService();
const indexPatternsService = getIndexPatterns();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,7 @@

interface DatasetSelectorProps {
selectedDataset?: Dataset;
setSelectedDataset: (data: Dataset | undefined) => void;
setIndexPattern: (id: string | undefined) => void;
handleDatasetChange: (dataset: Dataset) => void;
setSelectedDataset: (dataset: Dataset) => void;
services: IDataPluginServices;
}

Expand Down Expand Up @@ -73,8 +71,6 @@
export const DatasetSelector = ({
selectedDataset,
setSelectedDataset,
setIndexPattern,
handleDatasetChange,
services,
appearance,
buttonProps,
Expand Down Expand Up @@ -106,7 +102,7 @@

// If no dataset is selected, select the first one
if (!selectedDataset && fetchedDatasets.length > 0) {
handleDatasetChange(fetchedDatasets[0]);
setSelectedDataset(fetchedDatasets[0]);

Check warning on line 105 in src/plugins/data/public/ui/dataset_selector/dataset_selector.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/public/ui/dataset_selector/dataset_selector.tsx#L105

Added line #L105 was not covered by tests
}
};

Expand Down Expand Up @@ -183,11 +179,11 @@
indexPatterns.find((dataset) => dataset.id === selectedOption.key);
if (foundDataset) {
closePopover();
handleDatasetChange(foundDataset);
setSelectedDataset(foundDataset);

Check warning on line 182 in src/plugins/data/public/ui/dataset_selector/dataset_selector.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/public/ui/dataset_selector/dataset_selector.tsx#L182

Added line #L182 was not covered by tests
}
}
},
[recentDatasets, indexPatterns, handleDatasetChange, closePopover]
[recentDatasets, indexPatterns, setSelectedDataset, closePopover]
);

const datasetTitle = useMemo(() => {
Expand Down Expand Up @@ -270,14 +266,10 @@
onSelect={(dataset?: Dataset) => {
overlay?.close();
if (dataset) {
handleDatasetChange(dataset);
setSelectedDataset(dataset);

Check warning on line 269 in src/plugins/data/public/ui/dataset_selector/dataset_selector.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/public/ui/dataset_selector/dataset_selector.tsx#L269

Added line #L269 was not covered by tests
}
}}
onCancel={() => overlay?.close()}
selectedDataset={undefined}
setSelectedDataset={setSelectedDataset}
setIndexPattern={setIndexPattern}
direct={true}
/>
),
{
Expand Down
65 changes: 7 additions & 58 deletions src/plugins/data/public/ui/dataset_selector/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

import React from 'react';
import { mount } from 'enzyme';
import { act } from 'react-dom/test-utils';
import { DatasetSelector as ConnectedDatasetSelector } from './index';
import { DatasetSelector } from './dataset_selector';
import { useOpenSearchDashboards } from '../../../../opensearch_dashboards_react/public';
Expand Down Expand Up @@ -38,94 +37,44 @@ describe('ConnectedDatasetSelector', () => {
const mockServices = {
data: {
query: {
// @ts-ignore
queryString: mockQueryString,
},
},
};

beforeEach(() => {
(useOpenSearchDashboards as jest.Mock).mockReturnValue({ services: mockServices });
jest.clearAllMocks();
});

it('should render DatasetSelector with correct props', () => {
const wrapper = mount(
<ConnectedDatasetSelector
onSubmit={mockOnSubmit}
selectedDataset={undefined}
setSelectedDataset={jest.fn()}
setIndexPattern={jest.fn()}
services={mockServices}
/>
);
const wrapper = mount(<ConnectedDatasetSelector onSubmit={mockOnSubmit} />);
expect(wrapper.find(DatasetSelector).props()).toEqual({
selectedDataset: undefined,
setSelectedDataset: expect.any(Function),
setIndexPattern: expect.any(Function),
handleDatasetChange: expect.any(Function),
services: mockServices,
});
});

it('should initialize selectedDataset correctly', () => {
const mockDataset: Dataset = { id: 'initial', title: 'Initial Dataset', type: 'test' };
mockQueryString.getQuery.mockReturnValueOnce({ dataset: mockDataset });

const wrapper = mount(
<ConnectedDatasetSelector
onSubmit={mockOnSubmit}
selectedDataset={mockDataset}
setSelectedDataset={jest.fn()}
setIndexPattern={jest.fn()}
services={mockServices}
/>
);
const wrapper = mount(<ConnectedDatasetSelector onSubmit={mockOnSubmit} />);
expect(wrapper.find(DatasetSelector).prop('selectedDataset')).toEqual(mockDataset);
});

it('should call handleDatasetChange only once when dataset changes', () => {
const setSelectedDataset = jest.fn();
const setIndexPattern = jest.fn();
const wrapper = mount(
<ConnectedDatasetSelector
onSubmit={mockOnSubmit}
selectedDataset={undefined}
setSelectedDataset={setSelectedDataset}
setIndexPattern={setIndexPattern}
services={mockServices}
/>
);
const handleDatasetChange = wrapper.find(DatasetSelector).prop('handleDatasetChange') as (
const wrapper = mount(<ConnectedDatasetSelector onSubmit={mockOnSubmit} />);
const setSelectedDataset = wrapper.find(DatasetSelector).prop('setSelectedDataset') as (
dataset?: Dataset
) => void;

const newDataset: Dataset = { id: 'test', title: 'Test Dataset', type: 'test' };
act(() => {
handleDatasetChange(newDataset);
});
setSelectedDataset(newDataset);

expect(mockQueryString.getInitialQueryByDataset).toHaveBeenCalledTimes(1);
expect(mockQueryString.setQuery).toHaveBeenCalledTimes(1);
expect(mockOnSubmit).toHaveBeenCalledTimes(1);
expect(setSelectedDataset).toHaveBeenCalledWith(newDataset);
expect(setIndexPattern).toHaveBeenCalledWith(newDataset.id);
});

it('should subscribe to queryString.getUpdates$ and unsubscribe on unmount', () => {
const wrapper = mount(
<ConnectedDatasetSelector
onSubmit={mockOnSubmit}
selectedDataset={undefined}
setSelectedDataset={jest.fn()}
setIndexPattern={jest.fn()}
services={mockServices}
/>
);

expect(mockQueryString.getUpdates$).toHaveBeenCalledTimes(1);
expect(mockSubscribe).toHaveBeenCalledTimes(1);

wrapper.unmount();

expect(mockUnsubscribe).toHaveBeenCalledTimes(1);
});
});
Loading
Loading