Skip to content

Commit

Permalink
[ES|QL] Edits query in the dashboard (elastic#169911)
Browse files Browse the repository at this point in the history
## Summary

Part of elastic#165928
Closes elastic#144498

Allows the user to edit the ES|QL query from the dashboard. Also allows
the user to select one of the suggestions.

<img width="1886" alt="image"
src="https://github.com/elastic/kibana/assets/17003240/9961c154-e414-4ce1-bff5-33ec5c30db69">
<img width="1883" alt="image"
src="https://github.com/elastic/kibana/assets/17003240/6e8971d3-4a35-466f-804a-b8df58b09394">

### Testing
Navigate to Discover ES|QL mode and save a Lens chart to a dashboard.
Click the edit Visualization.

### Important notes
- We can very easily enable suggestions for the dataview panels but I am
going to do it on a follow up PR to keep this PR clean
- Creation is going to be added on a follow up PR
- Warnings are not rendered in the editor because I am using the limit 0
for performance reasons. We need to find another way to depict them via
the embeddable or store. It will be on a follow up PR.
- Errors are being displayed though. The user is not allowed to apply
the changes when an error occurs.
- Creating ES|QL charts from dashboard will happen to a follow up PR

### Running queries which don't return numeric fields
In these cases (i.e. `from logstash-* | keep clientip` we are returning
a table. I had to change the datatable logic for text based datasource
to not depend to isBucketed flag. This is something we had foreseen from
the [beginning of text based
languages](elastic#144498)

<img width="1879" alt="image"
src="https://github.com/elastic/kibana/assets/17003240/ca4b66fd-560d-4c1e-881d-b173458a06ae">

### Running queries which return a lot of fields
For queries with many fields Lens is going to suggest a huge table
trying to add the fields to the different dimensions. This is not
something we want:
- not performant
- user possibly will start removing fields from the dimensions
- this table is unreadable

For this reason we decided to select the first 5 fields and then the
user can easily adjust the dimensions with the fields they want.

<img width="1215" alt="image"
src="https://github.com/elastic/kibana/assets/17003240/07d7ee78-0085-41b1-98a0-a77eefbc0dcd">


### Checklist

- [ ] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [ ] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [ ] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
- [ ] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
stratoula and kibanamachine authored Nov 23, 2023
1 parent 3ec310b commit b55dae3
Show file tree
Hide file tree
Showing 57 changed files with 2,403 additions and 568 deletions.
114 changes: 87 additions & 27 deletions packages/kbn-text-based-editor/src/editor_footer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import {
EuiPopoverTitle,
EuiDescriptionList,
EuiDescriptionListDescription,
EuiButton,
useEuiTheme,
} from '@elastic/eui';
import { Interpolation, Theme, css } from '@emotion/react';
import { css as classNameCss } from '@emotion/css';
Expand Down Expand Up @@ -60,12 +62,14 @@ export function ErrorsWarningsPopover({
type,
setIsPopoverOpen,
onErrorClick,
isSpaceReduced,
}: {
isPopoverOpen: boolean;
items: MonacoError[];
type: 'error' | 'warning';
setIsPopoverOpen: (flag: boolean) => void;
onErrorClick: (error: MonacoError) => void;
isSpaceReduced?: boolean;
}) {
const strings = getConstsByType(type, items.length);
return (
Expand All @@ -90,7 +94,7 @@ export function ErrorsWarningsPopover({
setIsPopoverOpen(!isPopoverOpen);
}}
>
<p>{strings.message}</p>
<p>{isSpaceReduced ? items.length : strings.message}</p>
</EuiText>
}
ownFocus={false}
Expand Down Expand Up @@ -151,8 +155,11 @@ interface EditorFooterProps {
warning?: MonacoError[];
detectTimestamp: boolean;
onErrorClick: (error: MonacoError) => void;
refreshErrors: () => void;
runQuery: () => void;
hideRunQueryText?: boolean;
disableSubmitAction?: boolean;
editorIsInline?: boolean;
isSpaceReduced?: boolean;
}

export const EditorFooter = memo(function EditorFooter({
Expand All @@ -162,10 +169,15 @@ export const EditorFooter = memo(function EditorFooter({
warning,
detectTimestamp,
onErrorClick,
refreshErrors,
runQuery,
hideRunQueryText,
disableSubmitAction,
editorIsInline,
isSpaceReduced,
}: EditorFooterProps) {
const { euiTheme } = useEuiTheme();
const [isPopoverOpen, setIsPopoverOpen] = useState(false);

return (
<EuiFlexGroup
gutterSize="s"
Expand All @@ -176,24 +188,6 @@ export const EditorFooter = memo(function EditorFooter({
>
<EuiFlexItem grow={false}>
<EuiFlexGroup gutterSize="s" responsive={false} alignItems="center">
{errors && errors.length > 0 && (
<ErrorsWarningsPopover
isPopoverOpen={isPopoverOpen}
items={errors}
type="error"
setIsPopoverOpen={setIsPopoverOpen}
onErrorClick={onErrorClick}
/>
)}
{warning && warning.length > 0 && (
<ErrorsWarningsPopover
isPopoverOpen={isPopoverOpen}
items={warning}
type="warning"
setIsPopoverOpen={setIsPopoverOpen}
onErrorClick={onErrorClick}
/>
)}
<EuiFlexItem grow={false} style={{ marginRight: '8px' }}>
<EuiText size="xs" color="subdued" data-test-subj="TextBasedLangEditor-footer-lines">
<p>
Expand All @@ -206,30 +200,49 @@ export const EditorFooter = memo(function EditorFooter({
</EuiFlexItem>
<EuiFlexItem grow={false} style={{ marginRight: '16px' }}>
<EuiFlexGroup gutterSize="xs" responsive={false} alignItems="center">
<EuiFlexItem grow={false}>
<EuiIcon type="calendar" color="subdued" size="s" />
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiText size="xs" color="subdued" data-test-subj="TextBasedLangEditor-date-info">
<p>
{detectTimestamp
{isSpaceReduced
? '@timestamp'
: detectTimestamp
? i18n.translate(
'textBasedEditor.query.textBasedLanguagesEditor.timestampDetected',
{
defaultMessage: '@timestamp detected',
defaultMessage: '@timestamp found',
}
)
: i18n.translate(
'textBasedEditor.query.textBasedLanguagesEditor.timestampNotDetected',
{
defaultMessage: '@timestamp not detected',
defaultMessage: '@timestamp not found',
}
)}
</p>
</EuiText>
</EuiFlexItem>
</EuiFlexGroup>
</EuiFlexItem>
{errors && errors.length > 0 && (
<ErrorsWarningsPopover
isPopoverOpen={isPopoverOpen}
items={errors}
type="error"
setIsPopoverOpen={setIsPopoverOpen}
onErrorClick={onErrorClick}
isSpaceReduced={isSpaceReduced}
/>
)}
{warning && warning.length > 0 && (
<ErrorsWarningsPopover
isPopoverOpen={isPopoverOpen}
items={warning}
type="warning"
setIsPopoverOpen={setIsPopoverOpen}
onErrorClick={onErrorClick}
isSpaceReduced={isSpaceReduced}
/>
)}
</EuiFlexGroup>
</EuiFlexItem>
{!hideRunQueryText && (
Expand All @@ -255,6 +268,53 @@ export const EditorFooter = memo(function EditorFooter({
</EuiFlexGroup>
</EuiFlexItem>
)}
{Boolean(editorIsInline) && (
<EuiFlexItem grow={false}>
<EuiButton
color="text"
size="s"
fill
onClick={runQuery}
isDisabled={Boolean(disableSubmitAction)}
data-test-subj="TextBasedLangEditor-run-query-button"
minWidth={isSpaceReduced ? false : undefined}
>
<EuiFlexGroup
gutterSize="xs"
responsive={false}
alignItems="center"
justifyContent="spaceBetween"
>
<EuiFlexItem grow={false}>
{isSpaceReduced
? i18n.translate('textBasedEditor.query.textBasedLanguagesEditor.run', {
defaultMessage: 'Run',
})
: i18n.translate('textBasedEditor.query.textBasedLanguagesEditor.runQuery', {
defaultMessage: 'Run query',
})}
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiText
size="xs"
css={css`
border: 1px solid
${Boolean(disableSubmitAction)
? euiTheme.colors.disabled
: euiTheme.colors.emptyShade};
padding: 0 ${euiTheme.size.xs};
font-size: ${euiTheme.size.s};
margin-left: ${euiTheme.size.xs};
border-radius: ${euiTheme.size.xs};
`}
>
{COMMAND_KEY}
</EuiText>
</EuiFlexItem>
</EuiFlexGroup>
</EuiButton>
</EuiFlexItem>
)}
</EuiFlexGroup>
);
});
4 changes: 3 additions & 1 deletion packages/kbn-text-based-editor/src/resizable_button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,13 @@ import { css } from '@emotion/react';
export function ResizableButton({
onMouseDownResizeHandler,
onKeyDownResizeHandler,
editorIsInline,
}: {
onMouseDownResizeHandler: (
mouseDownEvent: React.MouseEvent<HTMLButtonElement, MouseEvent> | React.TouchEvent
) => void;
onKeyDownResizeHandler: (keyDownEvernt: React.KeyboardEvent) => void;
editorIsInline?: boolean;
}) {
return (
<EuiResizableButton
Expand All @@ -26,7 +28,7 @@ export function ResizableButton({
onKeyDown={onKeyDownResizeHandler}
onTouchStart={onMouseDownResizeHandler}
css={css`
position: absolute;
position: ${editorIsInline ? 'relative' : 'absolute'};
bottom: 0;
left: 0;
right: 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ export const textBasedLanguagedEditorStyles = (
hasErrors: boolean,
hasWarning: boolean,
isCodeEditorExpandedFocused: boolean,
hasReference: boolean
hasReference: boolean,
editorIsInline: boolean
) => {
let position = isCompactFocused ? ('absolute' as 'absolute') : ('relative' as 'relative'); // cast string to type 'relative' | 'absolute'
if (isCodeEditorExpanded) {
Expand All @@ -33,7 +34,9 @@ export const textBasedLanguagedEditorStyles = (
zIndex: isCompactFocused ? 4 : 0,
height: `${editorHeight}px`,
border: isCompactFocused ? euiTheme.border.thin : 'none',
borderTopLeftRadius: isCodeEditorExpanded ? 0 : '6px',
borderLeft: editorIsInline || !isCompactFocused ? 'none' : euiTheme.border.thin,
borderRight: editorIsInline || !isCompactFocused ? 'none' : euiTheme.border.thin,
borderTopLeftRadius: isCodeEditorExpanded ? 0 : euiTheme.border.radius.medium,
borderBottom: isCodeEditorExpanded
? 'none'
: isCompactFocused
Expand All @@ -45,8 +48,8 @@ export const textBasedLanguagedEditorStyles = (
width: isCodeEditorExpanded ? '100%' : `calc(100% - ${hasReference ? 80 : 40}px)`,
alignItems: isCompactFocused ? 'flex-start' : 'center',
border: !isCompactFocused ? euiTheme.border.thin : 'none',
borderTopLeftRadius: '6px',
borderBottomLeftRadius: '6px',
borderTopLeftRadius: euiTheme.border.radius.medium,
borderBottomLeftRadius: euiTheme.border.radius.medium,
borderBottomWidth: hasErrors ? '2px' : '1px',
borderBottomColor: hasErrors ? euiTheme.colors.danger : euiTheme.colors.lightShade,
},
Expand All @@ -66,6 +69,8 @@ export const textBasedLanguagedEditorStyles = (
},
bottomContainer: {
border: euiTheme.border.thin,
borderLeft: editorIsInline ? 'none' : euiTheme.border.thin,
borderRight: editorIsInline ? 'none' : euiTheme.border.thin,
borderTop:
isCodeEditorExpanded && !isCodeEditorExpandedFocused
? hasErrors
Expand All @@ -75,29 +80,29 @@ export const textBasedLanguagedEditorStyles = (
backgroundColor: euiTheme.colors.lightestShade,
paddingLeft: euiTheme.size.base,
paddingRight: euiTheme.size.base,
paddingTop: euiTheme.size.xs,
paddingBottom: euiTheme.size.xs,
paddingTop: editorIsInline ? euiTheme.size.s : euiTheme.size.xs,
paddingBottom: editorIsInline ? euiTheme.size.s : euiTheme.size.xs,
width: 'calc(100% + 2px)',
position: 'relative' as 'relative', // cast string to type 'relative',
marginTop: 0,
marginLeft: 0,
marginBottom: 0,
borderBottomLeftRadius: '6px',
borderBottomRightRadius: '6px',
borderBottomLeftRadius: editorIsInline ? 0 : euiTheme.border.radius.medium,
borderBottomRightRadius: editorIsInline ? 0 : euiTheme.border.radius.medium,
},
topContainer: {
border: euiTheme.border.thin,
borderTopLeftRadius: '6px',
borderTopRightRadius: '6px',
border: editorIsInline ? 'none' : euiTheme.border.thin,
borderTopLeftRadius: editorIsInline ? 0 : euiTheme.border.radius.medium,
borderTopRightRadius: editorIsInline ? 0 : euiTheme.border.radius.medium,
backgroundColor: euiTheme.colors.lightestShade,
paddingLeft: euiTheme.size.base,
paddingRight: euiTheme.size.base,
paddingTop: euiTheme.size.xs,
paddingBottom: euiTheme.size.xs,
paddingTop: editorIsInline ? euiTheme.size.s : euiTheme.size.xs,
paddingBottom: editorIsInline ? euiTheme.size.s : euiTheme.size.xs,
width: 'calc(100% + 2px)',
position: 'relative' as 'relative', // cast string to type 'relative',
marginLeft: 0,
marginTop: euiTheme.size.s,
marginTop: editorIsInline ? 0 : euiTheme.size.s,
},
dragResizeContainer: {
width: '100%',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ describe('TextBasedLanguagesEditor', () => {
});
});

it('should render the date info with no @timestamp detected', async () => {
it('should render the date info with no @timestamp found', async () => {
const newProps = {
...props,
isCodeEditorExpanded: true,
Expand All @@ -93,11 +93,11 @@ describe('TextBasedLanguagesEditor', () => {
const component = mount(renderTextBasedLanguagesEditorComponent({ ...newProps }));
expect(
component.find('[data-test-subj="TextBasedLangEditor-date-info"]').at(0).text()
).toStrictEqual('@timestamp not detected');
).toStrictEqual('@timestamp not found');
});
});

it('should render the date info with @timestamp detected if detectTimestamp is true', async () => {
it('should render the date info with @timestamp found if detectTimestamp is true', async () => {
const newProps = {
...props,
isCodeEditorExpanded: true,
Expand All @@ -107,7 +107,7 @@ describe('TextBasedLanguagesEditor', () => {
const component = mount(renderTextBasedLanguagesEditorComponent({ ...newProps }));
expect(
component.find('[data-test-subj="TextBasedLangEditor-date-info"]').at(0).text()
).toStrictEqual('@timestamp detected');
).toStrictEqual('@timestamp found');
});
});

Expand Down Expand Up @@ -265,4 +265,24 @@ describe('TextBasedLanguagesEditor', () => {
expect(component.find('[data-test-subj="TextBasedLangEditor-run-query"]').length).toBe(0);
});
});

it('should render correctly if editorIsInline prop is set to true', async () => {
const onTextLangQuerySubmit = jest.fn();
const newProps = {
...props,
isCodeEditorExpanded: true,
hideRunQueryText: true,
editorIsInline: true,
onTextLangQuerySubmit,
};
await act(async () => {
const component = mount(renderTextBasedLanguagesEditorComponent({ ...newProps }));
expect(component.find('[data-test-subj="TextBasedLangEditor-run-query"]').length).toBe(0);
expect(
component.find('[data-test-subj="TextBasedLangEditor-run-query-button"]').length
).not.toBe(1);
findTestSubject(component, 'TextBasedLangEditor-run-query-button').simulate('click');
expect(onTextLangQuerySubmit).toHaveBeenCalled();
});
});
});
Loading

0 comments on commit b55dae3

Please sign in to comment.