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

fix(discover): switch language based on dataset selection #7924

Merged
merged 3 commits into from
Sep 3, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const createSetupDatasetServiceMock = (): jest.Mocked<DatasetServiceContract> =>
toDataset: jest.fn(),
fetch: jest.fn(),
fetchFields: jest.fn(),
supportedLanguages: jest.fn().mockReturnValue(['DQL', 'Lucene', 'PPL', 'SQL']),
supportedLanguages: jest.fn().mockReturnValue(['kuery', 'lucene', 'PPL', 'SQL']),
};

const defaultDataset: Dataset = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,9 @@

supportedLanguages: (dataset): string[] => {
if (dataset.dataSource?.type === 'OpenSearch Serverless') {
return ['DQL', 'Lucene'];
return ['kuery', 'lucene'];

Check warning on line 68 in src/plugins/data/public/query/query_string/dataset_service/lib/index_pattern_type.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/public/query/query_string/dataset_service/lib/index_pattern_type.ts#L68

Added line #L68 was not covered by tests
}
return ['DQL', 'Lucene', 'PPL', 'SQL'];
return ['kuery', 'lucene', 'PPL', 'SQL'];

Check warning on line 70 in src/plugins/data/public/query/query_string/dataset_service/lib/index_pattern_type.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/public/query/query_string/dataset_service/lib/index_pattern_type.ts#L70

Added line #L70 was not covered by tests
},
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export interface DatasetTypeConfig {
fetchFields: (dataset: Dataset) => Promise<DatasetField[]>;
/**
* Retrieves the supported query languages for this dataset type.
* @returns {Promise<string[]>} A promise that resolves to an array of supported language names.
* @returns {Promise<string[]>} A promise that resolves to an array of supported language ids.
*/
supportedLanguages: (dataset: Dataset) => string[];
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@
this.languages.set(config.id, config);
}

public getLanguage(language: string): LanguageConfig | undefined {
return this.languages.get(language);
public getLanguage(languageId: string): LanguageConfig | undefined {
Copy link
Collaborator

@AMoo-Miki AMoo-Miki Sep 3, 2024

Choose a reason for hiding this comment

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

Is this a released public API we are changing? If unreleased ignore plz.

Copy link
Member Author

Choose a reason for hiding this comment

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

changing the parameter variable name shouldn't affect users of this API, and regarding documentation i don't think it's released, didn't find documentation for it

return this.languages.get(languageId);

Check warning on line 61 in src/plugins/data/public/query/query_string/language_service/language_service.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/public/query/query_string/language_service/language_service.ts#L61

Added line #L61 was not covered by tests
}

public getLanguages(): LanguageConfig[] {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,14 +157,14 @@

public getInitialQueryByDataset = (newDataset: Dataset) => {
const curQuery = this.query$.getValue();
const languageId = curQuery.language;
const languageId = newDataset.language || curQuery.language;
const language = this.languageService.getLanguage(languageId);
const newQuery = { ...curQuery, dataset: newDataset };
const input = language?.getQueryString(newQuery) || '';
const newQuery = { ...curQuery, language: languageId, dataset: newDataset };

Check warning on line 162 in src/plugins/data/public/query/query_string/query_string_manager.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/public/query/query_string/query_string_manager.ts#L162

Added line #L162 was not covered by tests
const newQueryString = language?.getQueryString(newQuery) || '';

return {
...newQuery,
query: input,
query: newQueryString,
};
};

Expand Down
27 changes: 16 additions & 11 deletions src/plugins/data/public/ui/dataset_selector/configurator.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
* SPDX-License-Identifier: Apache-2.0
*/

import React, { useEffect, useState } from 'react';
import {
EuiButton,
EuiButtonEmpty,
Expand All @@ -19,8 +18,9 @@ import {
} from '@elastic/eui';
import { i18n } from '@osd/i18n';
import { FormattedMessage } from '@osd/i18n/react';
import React, { useEffect, useState } from 'react';
import { BaseDataset, Dataset, DatasetField } from '../../../common';
import { getQueryService, getIndexPatterns } from '../../services';
import { getIndexPatterns, getQueryService } from '../../services';

export const Configurator = ({
baseDataset,
Expand All @@ -35,14 +35,20 @@ export const Configurator = ({
}) => {
const queryService = getQueryService();
const queryString = queryService.queryString;
const languageService = queryService.queryString.getLanguageService();
const indexPatternsService = getIndexPatterns();
const type = queryString.getDatasetService().getType(baseDataset.type);
const languages = type?.supportedLanguages(baseDataset) || [];

const [dataset, setDataset] = useState<Dataset>(baseDataset);
const [timeFields, setTimeFields] = useState<DatasetField[]>();
const [timeField, setTimeField] = useState<string | undefined>(dataset.timeFieldName);
const [language, setLanguage] = useState<string>(languages[0]);
const [timeFieldName, setTimeFieldName] = useState<string | undefined>();
const [language, setLanguage] = useState<string>(() => {
const currentLanguage = queryString.getQuery().language;
if (languages.includes(currentLanguage)) {
return currentLanguage;
}
return languages[0];
});

useEffect(() => {
const fetchFields = async () => {
Expand Down Expand Up @@ -88,7 +94,7 @@ export const Configurator = ({
}
)}
>
<EuiFieldText disabled value={dataset.title} />
<EuiFieldText disabled value={baseDataset.title} />
</EuiFormRow>
{timeFields && timeFields.length > 0 && (
<EuiFormRow
Expand All @@ -108,11 +114,10 @@ export const Configurator = ({
{ text: '-----', value: '', disabled: true },
{ text: 'No time field', value: undefined },
]}
value={timeField}
value={timeFieldName}
onChange={(e) => {
const value = e.target.value === 'undefined' ? undefined : e.target.value;
setTimeField(value);
setDataset({ ...dataset, timeFieldName: value });
setTimeFieldName(value);
}}
/>
</EuiFormRow>
Expand All @@ -127,7 +132,7 @@ export const Configurator = ({
>
<EuiSelect
options={languages.map((languageId) => ({
text: languageId,
text: languageService.getLanguage(languageId)?.title || languageId,
value: languageId,
}))}
value={language}
Expand All @@ -149,7 +154,7 @@ export const Configurator = ({
defaultMessage="Previous"
/>
</EuiButton>
<EuiButton onClick={() => onConfirm(dataset)} fill>
<EuiButton onClick={() => onConfirm({ ...baseDataset, language, timeFieldName })} fill>
<FormattedMessage
id="data.explorer.datasetSelector.advancedSelector.confirm"
defaultMessage="Select Data"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ export const QueryLanguageSelector = (props: QueryLanguageSelectorProps) => {
iconSize="s"
onClick={onButtonClick}
className="languageSelector__button"
iconType={'arrowDown'}
iconType="arrowDown"
>
{selectedLanguage.label}
</EuiButtonEmpty>
Expand Down
Loading