Skip to content

Commit

Permalink
[bug] address different issues with dataset selector (#8665)
Browse files Browse the repository at this point in the history
* [bug] address some issues with dataset selector

Moved the dataset selector back to the search bar and access it by ref in the sidebar.

Avoid out of sync issue.

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>

* update the logic for ensuring index pattern

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>

* dont add ability to open button

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>

* empty state but missing data set selector button

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>

* fix tests

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>

* add back styling

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>

---------

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
(cherry picked from commit e23f332)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
github-actions[bot] committed Oct 19, 2024
1 parent 74a69d6 commit 75e5f5a
Show file tree
Hide file tree
Showing 29 changed files with 286 additions and 553 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,7 @@ import { includes } from 'lodash';
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 @@ export const createEnsureDefaultIndexPattern = (
* 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 @@ export const createEnsureDefaultIndexPattern = (
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
38 changes: 7 additions & 31 deletions src/plugins/data/public/ui/dataset_selector/advanced_selector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,27 +13,19 @@ import {
} 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 @@ export const AdvancedSelector = ({
}),
},
]);
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
5 changes: 2 additions & 3 deletions src/plugins/data/public/ui/dataset_selector/configurator.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,20 @@ import { i18n } from '@osd/i18n';
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
18 changes: 5 additions & 13 deletions src/plugins/data/public/ui/dataset_selector/dataset_selector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,7 @@ type EuiSmallButtonEmptyProps = React.ComponentProps<typeof EuiSmallButtonEmpty>

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 @@ const RootComponent: React.FC<
export const DatasetSelector = ({
selectedDataset,
setSelectedDataset,
setIndexPattern,
handleDatasetChange,
services,
appearance,
buttonProps,
Expand Down Expand Up @@ -106,7 +102,7 @@ export const DatasetSelector = ({

// 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 @@ export const DatasetSelector = ({
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 @@ export const DatasetSelector = ({
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

0 comments on commit 75e5f5a

Please sign in to comment.