[Discover] Prevent showing the ES|QL transition modal when a saved search without unsaved changes is open (#177107)

## Summary

This PR prevents the ES|QL transition modal from showing when switching
to a data view from a saved search that has no unsaved changes.

Resolves #176772.

### 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)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [x] [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
- [x] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [ ] 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))
- [ ] If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)
- [ ] 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)

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
This commit is contained in:
Davis McPhee 2024-02-22 11:47:13 -04:00 committed by GitHub
parent cc915bd4d9
commit 36365bd911
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 258 additions and 86 deletions

View file

@ -13,4 +13,5 @@ export {
getLimitFromESQLQuery,
removeDropCommandsFromESQLQuery,
getIndexForESQLQuery,
TextBasedLanguages,
} from './src';

View file

@ -6,6 +6,7 @@
* Side Public License, v 1.
*/
export { TextBasedLanguages } from './types';
export { getESQLAdHocDataview, getIndexForESQLQuery } from './utils/get_esql_adhoc_dataview';
export {
getIndexPatternFromSQLQuery,

View file

@ -0,0 +1,12 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
export enum TextBasedLanguages {
SQL = 'SQL',
ESQL = 'ESQL',
}

View file

@ -9,13 +9,18 @@
import React, { useCallback, useEffect, useMemo, useRef } from 'react';
import type { AggregateQuery, Query, TimeRange } from '@kbn/es-query';
import { type DataView, DataViewType } from '@kbn/data-views-plugin/public';
import type { DataViewPickerProps } from '@kbn/unified-search-plugin/public';
import { DataViewPickerProps } from '@kbn/unified-search-plugin/public';
import { ENABLE_ESQL } from '@kbn/discover-utils';
import { useSavedSearchInitial } from '../../services/discover_state_provider';
import { TextBasedLanguages } from '@kbn/esql-utils';
import {
useSavedSearch,
useSavedSearchHasChanged,
useSavedSearchInitial,
} from '../../services/discover_state_provider';
import { useInternalStateSelector } from '../../services/discover_internal_state_container';
import { useDiscoverServices } from '../../../../hooks/use_discover_services';
import { getHeaderActionMenuMounter } from '../../../../kibana_services';
import { DiscoverStateContainer } from '../../services/discover_state';
import type { DiscoverStateContainer } from '../../services/discover_state';
import { onSaveSearch } from './on_save_search';
import { useDiscoverCustomization } from '../../../../customizations';
import { addLog } from '../../../../utils/add_log';
@ -47,30 +52,25 @@ export const DiscoverTopNav = ({
isLoading,
onCancelClick,
}: DiscoverTopNavProps) => {
const services = useDiscoverServices();
const { dataViewEditor, navigation, dataViewFieldEditor, data, uiSettings, dataViews } = services;
const query = useAppStateSelector((state) => state.query);
const adHocDataViews = useInternalStateSelector((state) => state.adHocDataViews);
const dataView = useInternalStateSelector((state) => state.dataView!);
const savedDataViews = useInternalStateSelector((state) => state.savedDataViews);
const savedSearch = useSavedSearchInitial();
const isTextBased = useMemo(() => isTextBasedQuery(query), [query]);
const showDatePicker = useMemo(() => {
// always show the timepicker for text based languages
const isTextBased = isTextBasedQuery(query);
return (
isTextBased ||
(!isTextBased && dataView.isTimeBased() && dataView.type !== DataViewType.ROLLUP)
);
}, [dataView, isTextBased]);
const services = useDiscoverServices();
const { dataViewEditor, navigation, dataViewFieldEditor, data, uiSettings, dataViews } = services;
const canEditDataView =
Boolean(dataViewEditor?.userPermissions.editDataView()) || !dataView.isPersisted();
}, [dataView, query]);
const closeFieldEditor = useRef<() => void | undefined>();
const closeDataViewEditor = useRef<() => void | undefined>();
const { AggregateQueryTopNavMenu } = navigation.ui;
useEffect(() => {
return () => {
// Make sure to close the editors when unmounting
@ -83,6 +83,9 @@ export const DiscoverTopNav = ({
};
}, []);
const canEditDataView =
Boolean(dataViewEditor?.userPermissions.editDataView()) || !dataView.isPersisted();
const editField = useMemo(
() =>
canEditDataView
@ -116,19 +119,22 @@ export const DiscoverTopNav = ({
});
}, [dataViewEditor, stateContainer]);
const onEditDataView = async (editedDataView: DataView) => {
if (editedDataView.isPersisted()) {
// Clear the current data view from the cache and create a new instance
// of it, ensuring we have a new object reference to trigger a re-render
dataViews.clearInstanceCache(editedDataView.id);
stateContainer.actions.setDataView(await dataViews.create(editedDataView.toSpec(), true));
} else {
await stateContainer.actions.updateAdHocDataViewId();
}
stateContainer.actions.loadDataViewList();
addLog('[DiscoverTopNav] onEditDataView triggers data fetching');
stateContainer.dataState.fetch();
};
const onEditDataView = useCallback(
async (editedDataView: DataView) => {
if (editedDataView.isPersisted()) {
// Clear the current data view from the cache and create a new instance
// of it, ensuring we have a new object reference to trigger a re-render
dataViews.clearInstanceCache(editedDataView.id);
stateContainer.actions.setDataView(await dataViews.create(editedDataView.toSpec(), true));
} else {
await stateContainer.actions.updateAdHocDataViewId();
}
stateContainer.actions.loadDataViewList();
addLog('[DiscoverTopNav] onEditDataView triggers data fetching');
stateContainer.dataState.fetch();
},
[dataViews, stateContainer.actions, stateContainer.dataState]
);
const updateSavedQueryId = (newSavedQueryId: string | undefined) => {
const { appState } = stateContainer;
@ -143,41 +149,6 @@ export const DiscoverTopNav = ({
appState.set(newState);
}
};
const setMenuMountPoint = useMemo(() => {
return getHeaderActionMenuMounter();
}, []);
const isESQLModeEnabled = uiSettings.get(ENABLE_ESQL);
const supportedTextBasedLanguages = [];
if (isESQLModeEnabled) {
supportedTextBasedLanguages.push('ESQL');
}
const searchBarCustomization = useDiscoverCustomization('search_bar');
const SearchBar = useMemo(
() => searchBarCustomization?.CustomSearchBar ?? AggregateQueryTopNavMenu,
[searchBarCustomization?.CustomSearchBar, AggregateQueryTopNavMenu]
);
const shouldHideDefaultDataviewPicker =
!!searchBarCustomization?.CustomDataViewPicker || !!searchBarCustomization?.hideDataViewPicker;
const dataViewPickerProps: DataViewPickerProps = {
trigger: {
label: dataView?.getName() || '',
'data-test-subj': 'discover-dataView-switch-link',
title: dataView?.getIndexPattern() || '',
},
currentDataViewId: dataView?.id,
onAddField: addField,
onDataViewCreated: createNewDataView,
onCreateDefaultAdHocDataView: stateContainer.actions.createAndAppendAdHocDataView,
onChangeDataView: stateContainer.actions.onChangeDataView,
textBasedLanguages: supportedTextBasedLanguages as DataViewPickerProps['textBasedLanguages'],
adHocDataViews,
savedDataViews,
onEditDataView,
};
const onTextBasedSavedAndExit = useCallback(
({ onSave, onCancel }) => {
@ -193,11 +164,66 @@ export const DiscoverTopNav = ({
);
const { topNavBadges, topNavMenu } = useDiscoverTopNav({ stateContainer });
const topNavProps = !services.serverless && {
badges: topNavBadges,
config: topNavMenu,
setMenuMountPoint,
};
const setMenuMountPoint = getHeaderActionMenuMounter();
const topNavProps = useMemo(() => {
if (services.serverless) {
return undefined;
}
return {
badges: topNavBadges,
config: topNavMenu,
setMenuMountPoint,
};
}, [services.serverless, setMenuMountPoint, topNavBadges, topNavMenu]);
const savedSearchId = useSavedSearch().id;
const savedSearchHasChanged = useSavedSearchHasChanged();
const dataViewPickerProps: DataViewPickerProps = useMemo(() => {
const isESQLModeEnabled = uiSettings.get(ENABLE_ESQL);
const supportedTextBasedLanguages: DataViewPickerProps['textBasedLanguages'] = isESQLModeEnabled
? [TextBasedLanguages.ESQL]
: [];
return {
trigger: {
label: dataView?.getName() || '',
'data-test-subj': 'discover-dataView-switch-link',
title: dataView?.getIndexPattern() || '',
},
currentDataViewId: dataView?.id,
onAddField: addField,
onDataViewCreated: createNewDataView,
onCreateDefaultAdHocDataView: stateContainer.actions.createAndAppendAdHocDataView,
onChangeDataView: stateContainer.actions.onChangeDataView,
textBasedLanguages: supportedTextBasedLanguages,
shouldShowTextBasedLanguageTransitionModal: !savedSearchId || savedSearchHasChanged,
adHocDataViews,
savedDataViews,
onEditDataView,
};
}, [
adHocDataViews,
addField,
createNewDataView,
dataView,
onEditDataView,
savedDataViews,
savedSearchHasChanged,
savedSearchId,
stateContainer,
uiSettings,
]);
const searchBarCustomization = useDiscoverCustomization('search_bar');
const SearchBar = useMemo(
() => searchBarCustomization?.CustomSearchBar ?? navigation.ui.AggregateQueryTopNavMenu,
[searchBarCustomization?.CustomSearchBar, navigation.ui.AggregateQueryTopNavMenu]
);
const shouldHideDefaultDataviewPicker =
!!searchBarCustomization?.CustomDataViewPicker || !!searchBarCustomization?.hideDataViewPicker;
return (
<SearchBar

View file

@ -30,10 +30,19 @@ function createStateHelpers() {
container!.savedSearchState.getInitial$().getValue()
);
};
const useSavedSearchHasChanged = () => {
const container = useContainer();
return useObservable<boolean>(
container!.savedSearchState.getHasChanged$(),
container!.savedSearchState.getHasChanged$().getValue()
);
};
return {
Provider: context.Provider,
useSavedSearch,
useSavedSearchInitial,
useSavedSearchHasChanged,
};
}
@ -41,6 +50,7 @@ export const {
Provider: DiscoverStateProvider,
useSavedSearchInitial,
useSavedSearch,
useSavedSearchHasChanged,
} = createStateHelpers();
export const DiscoverMainProvider = ({

View file

@ -14,10 +14,13 @@ import { findTestSubject } from '@elastic/eui/lib/test';
import { KibanaContextProvider } from '@kbn/kibana-react-plugin/public';
import { dataPluginMock } from '@kbn/data-plugin/public/mocks';
import { indexPatternEditorPluginMock as dataViewEditorPluginMock } from '@kbn/data-view-editor-plugin/public/mocks';
import { TextBasedLanguages } from '@kbn/esql-utils';
import { ChangeDataView } from './change_dataview';
import { DataViewSelector } from './data_view_selector';
import { dataViewMock } from './mocks/dataview';
import { DataViewPickerPropsExtended, TextBasedLanguages } from './data_view_picker';
import { DataViewPickerPropsExtended } from './data_view_picker';
import { render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
describe('DataView component', () => {
const createMockWebStorage = () => ({
@ -224,4 +227,44 @@ describe('DataView component', () => {
},
]);
});
describe('test based language switch warning icon', () => {
beforeAll(() => {
// Enzyme doesn't clean the DOM between tests, so we need to do it manually
document.body.innerHTML = '';
});
it('should show text based language switch warning icon', () => {
render(
wrapDataViewComponentInContext(
{
...props,
onDataViewCreated: jest.fn(),
textBasedLanguages: [TextBasedLanguages.ESQL],
textBasedLanguage: TextBasedLanguages.ESQL,
},
false
)
);
userEvent.click(screen.getByTestId('dataview-trigger'));
expect(screen.queryByTestId('textBasedLang-warning')).toBeInTheDocument();
});
it('should not show text based language switch warning icon when shouldShowTextBasedLanguageTransitionModal is false', () => {
render(
wrapDataViewComponentInContext(
{
...props,
onDataViewCreated: jest.fn(),
textBasedLanguages: [TextBasedLanguages.ESQL],
textBasedLanguage: TextBasedLanguages.ESQL,
shouldShowTextBasedLanguageTransitionModal: false,
},
false
)
);
userEvent.click(screen.getByTestId('dataview-trigger'));
expect(screen.queryByTestId('textBasedLang-warning')).not.toBeInTheDocument();
});
});
});

View file

@ -78,6 +78,7 @@ export function ChangeDataView({
onSaveTextLanguageQuery,
onTextLangQuerySubmit,
textBasedLanguage,
shouldShowTextBasedLanguageTransitionModal = true,
isDisabled,
onEditDataView,
onCreateDefaultAdHocDataView,
@ -238,7 +239,7 @@ export function ChangeDataView({
<EuiFlexItem grow={false}>
<EuiFlexGroup alignItems="center" gutterSize="xs" responsive={false}>
<EuiFlexItem grow={false}>
{Boolean(isTextBasedLangSelected) ? (
{isTextBasedLangSelected && shouldShowTextBasedLanguageTransitionModal ? (
<EuiToolTip
position="top"
content={i18n.translate(
@ -309,17 +310,23 @@ export function ChangeDataView({
onChangeDataView={async (newId) => {
setSelectedDataViewId(newId);
setPopoverIsOpen(false);
if (isTextBasedLangSelected && !isTextLangTransitionModalDismissed) {
setIsTextLangTransitionModalVisible(true);
} else if (isTextBasedLangSelected && isTextLangTransitionModalDismissed) {
setIsTextBasedLangSelected(false);
// clean up the Text based language query
onTextLangQuerySubmit?.({
language: 'kuery',
query: '',
});
onChangeDataView(newId);
setTriggerLabel(trigger.label);
if (isTextBasedLangSelected) {
const showTransitionModal =
!isTextLangTransitionModalDismissed && shouldShowTextBasedLanguageTransitionModal;
if (showTransitionModal) {
setIsTextLangTransitionModalVisible(true);
} else {
setIsTextBasedLangSelected(false);
// clean up the Text based language query
onTextLangQuerySubmit?.({
language: 'kuery',
query: '',
});
onChangeDataView(newId);
setTriggerLabel(trigger.label);
}
} else {
onChangeDataView(newId);
}

View file

@ -10,6 +10,7 @@ import React from 'react';
import type { EuiButtonProps, EuiSelectableProps } from '@elastic/eui';
import type { DataView, DataViewListItem, DataViewSpec } from '@kbn/data-views-plugin/public';
import type { AggregateQuery, Query } from '@kbn/es-query';
import { TextBasedLanguages } from '@kbn/esql-utils';
import { ChangeDataView } from './change_dataview';
export type ChangeDataViewTriggerProps = EuiButtonProps & {
@ -17,11 +18,6 @@ export type ChangeDataViewTriggerProps = EuiButtonProps & {
title?: string;
};
export enum TextBasedLanguages {
SQL = 'SQL',
ESQL = 'ESQL',
}
export interface OnSaveTextLanguageQueryProps {
onSave: () => void;
onCancel: () => void;
@ -84,6 +80,11 @@ export interface DataViewPickerProps {
* Callback that is called when the user clicks the Save and switch transition modal button
*/
onSaveTextLanguageQuery?: ({ onSave, onCancel }: OnSaveTextLanguageQueryProps) => void;
/**
* Determines if the text based language transition
* modal should be shown when switching data views
*/
shouldShowTextBasedLanguageTransitionModal?: boolean;
/**
* Makes the picker disabled by disabling the popover trigger
@ -117,6 +118,7 @@ export const DataViewPicker = ({
onSaveTextLanguageQuery,
onTextLangQuerySubmit,
textBasedLanguage,
shouldShowTextBasedLanguageTransitionModal,
onCreateDefaultAdHocDataView,
isDisabled,
}: DataViewPickerPropsExtended) => {
@ -137,6 +139,7 @@ export const DataViewPicker = ({
onSaveTextLanguageQuery={onSaveTextLanguageQuery}
onTextLangQuerySubmit={onTextLangQuerySubmit}
textBasedLanguage={textBasedLanguage}
shouldShowTextBasedLanguageTransitionModal={shouldShowTextBasedLanguageTransitionModal}
isDisabled={isDisabled}
/>
);

View file

@ -41,7 +41,11 @@ export default function TextBasedLanguagesTransitionModal({
const language = getLanguageDisplayName(textBasedLanguage);
return (
<EuiModal onClose={() => setIsTextLangTransitionModalVisible(false)} style={{ width: 700 }}>
<EuiModal
onClose={() => setIsTextLangTransitionModalVisible(false)}
style={{ width: 700 }}
data-test-subj="unifiedSearch_switch_modal"
>
<EuiModalHeader>
<EuiModalHeaderTitle>
{i18n.translate(

View file

@ -45,6 +45,7 @@
"@kbn/code-editor",
"@kbn/calculate-width-from-char-count",
"@kbn/react-kibana-context-render",
"@kbn/esql-utils"
],
"exclude": [
"target/**/*",

View file

@ -18,6 +18,8 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
const testSubjects = getService('testSubjects');
const monacoEditor = getService('monacoEditor');
const security = getService('security');
const retry = getService('retry');
const find = getService('find');
const PageObjects = getPageObjects([
'common',
'discover',
@ -33,6 +35,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
describe('discover esql view', async function () {
before(async () => {
await kibanaServer.savedObjects.cleanStandardList();
await security.testUser.setRoles(['kibana_admin', 'test_logstash_reader']);
log.debug('load kibana index with default index pattern');
await kibanaServer.importExport.load('test/functional/fixtures/kbn_archiver/discover');
@ -152,6 +155,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
]);
});
});
describe('errors', () => {
it('should show error messages for syntax errors in query', async function () {
await PageObjects.discover.selectTextBaseLang();
@ -179,5 +183,60 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
}
});
});
describe('switch modal', () => {
beforeEach(async () => {
await PageObjects.common.navigateToApp('discover');
await PageObjects.timePicker.setDefaultAbsoluteRange();
});
it('should show switch modal when switching to a data view', async () => {
await PageObjects.discover.selectTextBaseLang();
await PageObjects.header.waitUntilLoadingHasFinished();
await PageObjects.discover.waitUntilSearchingHasFinished();
await PageObjects.discover.selectIndexPattern('logstash-*', false);
await retry.try(async () => {
await testSubjects.existOrFail('unifiedSearch_switch_modal');
});
});
it('should not show switch modal when switching to a data view while a saved search is open', async () => {
await PageObjects.discover.selectTextBaseLang();
const testQuery = 'from logstash-* | limit 100 | drop @timestamp';
await monacoEditor.setCodeEditorValue(testQuery);
await testSubjects.click('querySubmitButton');
await PageObjects.header.waitUntilLoadingHasFinished();
await PageObjects.discover.waitUntilSearchingHasFinished();
await PageObjects.discover.selectIndexPattern('logstash-*', false);
await retry.try(async () => {
await testSubjects.existOrFail('unifiedSearch_switch_modal');
});
await find.clickByCssSelector(
'[data-test-subj="unifiedSearch_switch_modal"] .euiModal__closeIcon'
);
await retry.try(async () => {
await testSubjects.missingOrFail('unifiedSearch_switch_modal');
});
await PageObjects.discover.saveSearch('esql_test');
await PageObjects.discover.selectIndexPattern('logstash-*');
await testSubjects.missingOrFail('unifiedSearch_switch_modal');
});
it('should show switch modal when switching to a data view while a saved search with unsaved changes is open', async () => {
await PageObjects.discover.selectTextBaseLang();
await PageObjects.header.waitUntilLoadingHasFinished();
await PageObjects.discover.waitUntilSearchingHasFinished();
await PageObjects.discover.saveSearch('esql_test2');
const testQuery = 'from logstash-* | limit 100 | drop @timestamp';
await monacoEditor.setCodeEditorValue(testQuery);
await testSubjects.click('querySubmitButton');
await PageObjects.header.waitUntilLoadingHasFinished();
await PageObjects.discover.waitUntilSearchingHasFinished();
await PageObjects.discover.selectIndexPattern('logstash-*', false);
await retry.try(async () => {
await testSubjects.existOrFail('unifiedSearch_switch_modal');
});
});
});
});
}

View file

@ -551,13 +551,18 @@ export class DiscoverPageObject extends FtrService {
return hasBadge;
}
public async selectIndexPattern(indexPattern: string) {
public async selectIndexPattern(
indexPattern: string,
waitUntilLoadingHasFinished: boolean = true
) {
await this.testSubjects.click('discover-dataView-switch-link');
await this.find.setValue('[data-test-subj="indexPattern-switcher"] input', indexPattern);
await this.find.clickByCssSelector(
`[data-test-subj="indexPattern-switcher"] [title="${indexPattern}"]`
);
await this.header.waitUntilLoadingHasFinished();
if (waitUntilLoadingHasFinished) {
await this.header.waitUntilLoadingHasFinished();
}
}
public async getIndexPatterns() {