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

[Data Explorer] Adding PPL to query language selector #5623

Closed
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- [Discover] Display inner properties in the left navigation bar [#5429](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5429)
- [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))
- [Data Explorer] Adding PPL to query language selector ([#5623](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5623))

### 🐛 Bug Fixes

Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
@import "./query_bar";
@import "./language_switcher"
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// TODO: removed the following CSS
// when https://github.com/opensearch-project/OpenSearch-Dashboards/issues/5628 is completed
.languageSwitcher {
abbyhu2000 marked this conversation as resolved.
Show resolved Hide resolved
max-width: 150px;
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ describe('LanguageSwitcher', () => {
const services = {
uiSettings: startMock.uiSettings,
docLinks: startMock.docLinks,
application: startMock.application,
};

return (
Expand Down
181 changes: 121 additions & 60 deletions src/plugins/data/public/ui/query_string_input/language_switcher.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@

import {
EuiButtonEmpty,
EuiComboBox,
EuiComboBoxOptionOption,
EuiForm,
EuiFormRow,
EuiLink,
Expand All @@ -42,7 +44,9 @@
} from '@elastic/eui';
import { FormattedMessage } from '@osd/i18n/react';
import React, { useState } from 'react';
import { useObservable } from 'react-use';
import { useOpenSearchDashboards } from '../../../../opensearch_dashboards_react/public';
import { IDataPluginServices } from '../../types';

interface Props {
language: string;
Expand All @@ -51,6 +55,34 @@
}

export function QueryLanguageSwitcher(props: Props) {
const opensearchDashboards = useOpenSearchDashboards<IDataPluginServices>();
const { application } = opensearchDashboards.services;
const currentApp$ = application?.currentAppId$;
let useNewQuerySelector;
application?.applications$.subscribe((applications) => {
Copy link
Member

Choose a reason for hiding this comment

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

This subscribe should be wrapped inside a useEffect I suppose?

Copy link
Member

Choose a reason for hiding this comment

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

+1

applications.forEach((applicationEntry) => {
if (applicationEntry.id === 'observability-dashboards') {
useNewQuerySelector = true;
return;

Check warning on line 66 in src/plugins/data/public/ui/query_string_input/language_switcher.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/public/ui/query_string_input/language_switcher.tsx#L65-L66

Added lines #L65 - L66 were not covered by tests
}
});
});

const dataExplorerOptions = [
Copy link
Member

Choose a reason for hiding this comment

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

We also need SQL. Also can we use the internationalized version of these labels from below? i.e. luceneLabel and dqlLabel?

Copy link
Member Author

Choose a reason for hiding this comment

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

Added SQL which will also redirect to observability page.

dataExplorerOptions is used by <EuiComboBox> to show the dropdown list. Do we want to change how the label is shown on the dropdown from lucene to luceneLabel etc? It looks kinda crowded to me.
Screenshot 2024-01-03 at 12 15 18 PM

Copy link
Member

Choose a reason for hiding this comment

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

oh no, i meant that on line 90 you have an internationalized version of these labels. Can we reuse them? and add new labels for the missing languages?

Copy link
Member

@SuZhou-Joe SuZhou-Joe Dec 21, 2023

Choose a reason for hiding this comment

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

Can we add an enum for these language options like

enum ENUM_LANGUAGES { DQL = "DQL" }

? And I suggest adding an id field inside options though under context of language, label no need to wrapped by i18n.

Copy link
Member

Choose a reason for hiding this comment

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

We will still need i18n since its purpose is to individually identify the language name. e.g. data.query.queryBar.dqlLanguageName

{
label: 'DQL',
},
{
label: 'lucene',
},
{
label: 'PPL',
},
{
label: 'SQL',
},
];

const osdDQLDocs = useOpenSearchDashboards().services.docLinks?.links.opensearchDashboards.dql
abbyhu2000 marked this conversation as resolved.
Show resolved Hide resolved
.base;
const [isPopoverOpen, setIsPopoverOpen] = useState(false);
Expand Down Expand Up @@ -78,66 +110,95 @@
</EuiButtonEmpty>
);

return (
<EuiPopover
id="queryLanguageSwitcherPopover"
anchorClassName="euiFormControlLayout__append"
ownFocus
anchorPosition={props.anchorPosition || 'downRight'}
button={button}
isOpen={isPopoverOpen}
closePopover={() => setIsPopoverOpen(false)}
repositionOnScroll
>
<EuiPopoverTitle>
<FormattedMessage
id="data.query.queryBar.syntaxOptionsTitle"
defaultMessage="Syntax options"
/>
</EuiPopoverTitle>
<div style={{ width: '350px' }}>
<EuiText>
<p>
<FormattedMessage
id="data.query.queryBar.syntaxOptionsDescription"
defaultMessage="The {docsLink} (DQL) offers a simplified query
syntax and support for scripted fields. If you turn off DQL,
OpenSearch Dashboards uses Lucene."
values={{
docsLink: (
<EuiLink href={osdDQLDocs} target="_blank">
{dqlFullName}
</EuiLink>
),
}}
/>
</p>
</EuiText>
const handleLanguageChange = (newLanguage: EuiComboBoxOptionOption[]) => {
if (['PPL', 'SQL'].includes(newLanguage[0].label)) {
application?.navigateToUrl('../observability-logs#/explorer');
return;

Check warning on line 116 in src/plugins/data/public/ui/query_string_input/language_switcher.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/public/ui/query_string_input/language_switcher.tsx#L115-L116

Added lines #L115 - L116 were not covered by tests
}
const queryLanguage = newLanguage[0].label === 'DQL' ? 'kuery' : newLanguage[0].label;
props.onSelectLanguage(queryLanguage);

Check warning on line 119 in src/plugins/data/public/ui/query_string_input/language_switcher.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/public/ui/query_string_input/language_switcher.tsx#L119

Added line #L119 was not covered by tests
};

<EuiSpacer size="m" />
// The following is a temporary solution for adding PPL navigation, and should be replaced once final solution is determined.
// Follow-up issue: https://github.com/opensearch-project/OpenSearch-Dashboards/issues/5628
if (useObservable(currentApp$!, '') === 'data-explorer' && useNewQuerySelector) {
const selectedLanguage = {

Check warning on line 125 in src/plugins/data/public/ui/query_string_input/language_switcher.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/public/ui/query_string_input/language_switcher.tsx#L125

Added line #L125 was not covered by tests
label: props.language === 'kuery' ? 'DQL' : props.language,
};
return (

Check warning on line 128 in src/plugins/data/public/ui/query_string_input/language_switcher.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/public/ui/query_string_input/language_switcher.tsx#L128

Added line #L128 was not covered by tests
<EuiComboBox
className="languageSwitcher"
data-test-subj="languageSelect"
options={dataExplorerOptions}
selectedOptions={[selectedLanguage]}
onChange={handleLanguageChange}
singleSelection={{ asPlainText: true }}
isClearable={false}
async
/>
);
} else {
return (
<EuiPopover
id="queryLanguageSwitcherPopover"
anchorClassName="euiFormControlLayout__append"
ownFocus
anchorPosition={props.anchorPosition || 'downRight'}
button={button}
isOpen={isPopoverOpen}
closePopover={() => setIsPopoverOpen(false)}

Check warning on line 149 in src/plugins/data/public/ui/query_string_input/language_switcher.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/public/ui/query_string_input/language_switcher.tsx#L149

Added line #L149 was not covered by tests
repositionOnScroll
>
<EuiPopoverTitle>
<FormattedMessage
id="data.query.queryBar.syntaxOptionsTitle"
defaultMessage="Syntax options"
/>
</EuiPopoverTitle>
<div style={{ width: '350px' }}>
<EuiText>
<p>
<FormattedMessage
id="data.query.queryBar.syntaxOptionsDescription"
defaultMessage="The {docsLink} (DQL) offers a simplified query
syntax and support for scripted fields. If you turn off DQL,
OpenSearch Dashboards uses Lucene."
values={{
docsLink: (
<EuiLink href={osdDQLDocs} target="_blank">
{dqlFullName}
</EuiLink>
),
}}
/>
</p>
</EuiText>

<EuiForm>
<EuiFormRow label={dqlFullName}>
<EuiSwitch
id="queryEnhancementOptIn"
name="popswitch"
label={
props.language === 'kuery' ? (
<FormattedMessage id="data.query.queryBar.dqlOnLabel" defaultMessage="On" />
) : (
<FormattedMessage id="data.query.queryBar.dqlOffLabel" defaultMessage="Off" />
)
}
checked={props.language === 'kuery'}
onChange={() => {
const newLanguage = props.language === 'lucene' ? 'kuery' : 'lucene';
props.onSelectLanguage(newLanguage);
}}
data-test-subj="languageToggle"
/>
</EuiFormRow>
</EuiForm>
</div>
</EuiPopover>
);
<EuiSpacer size="m" />

<EuiForm>
<EuiFormRow label={dqlFullName}>
<EuiSwitch
id="queryEnhancementOptIn"
name="popswitch"
label={
props.language === 'kuery' ? (
<FormattedMessage id="data.query.queryBar.dqlOnLabel" defaultMessage="On" />
) : (
<FormattedMessage id="data.query.queryBar.dqlOffLabel" defaultMessage="Off" />
)
}
checked={props.language === 'kuery'}
onChange={() => {
const newLanguage = props.language === 'lucene' ? 'kuery' : 'lucene';
props.onSelectLanguage(newLanguage);

Check warning on line 194 in src/plugins/data/public/ui/query_string_input/language_switcher.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/public/ui/query_string_input/language_switcher.tsx#L194

Added line #L194 was not covered by tests
}}
data-test-subj="languageToggle"
/>
</EuiFormRow>
</EuiForm>
</div>
</EuiPopover>
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,6 @@ export default class QueryStringInputUI extends Component<Props, State> {
if (this.persistedLog) {
this.persistedLog.add(query.query);
}

this.props.onSubmit({ query: fromUser(query.query), language: query.language });
}
};
Expand Down
Loading