Skip to content

Commit

Permalink
[Discover] Fix "New" link in ES|QL mode (elastic#177038)
Browse files Browse the repository at this point in the history
- Closes elastic#176873

## Summary

This PR makes sure that when user presses "New" top nav link in
Discover, they will stay in ES|QL mode if the previous mode was ES|QL
too. The ES|QL query will be reset to the initial one `from <index
pattern> | limit 10`. For this query I created a new util
`getInitialESQLQuery()`.

### Checklist

- [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] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)

---------

Co-authored-by: kibanamachine <[email protected]>
  • Loading branch information
jughosta and kibanamachine authored Feb 23, 2024
1 parent c2b3173 commit a786612
Show file tree
Hide file tree
Showing 13 changed files with 218 additions and 13 deletions.
1 change: 1 addition & 0 deletions packages/kbn-esql-utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,6 @@ export {
getLimitFromESQLQuery,
removeDropCommandsFromESQLQuery,
getIndexForESQLQuery,
getInitialESQLQuery,
TextBasedLanguages,
} from './src';
1 change: 1 addition & 0 deletions packages/kbn-esql-utils/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

export { TextBasedLanguages } from './types';
export { getESQLAdHocDataview, getIndexForESQLQuery } from './utils/get_esql_adhoc_dataview';
export { getInitialESQLQuery } from './utils/get_initial_esql_query';
export {
getIndexPatternFromSQLQuery,
getIndexPatternFromESQLQuery,
Expand Down
15 changes: 15 additions & 0 deletions packages/kbn-esql-utils/src/utils/get_initial_esql_query.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/*
* 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.
*/

import { getInitialESQLQuery } from './get_initial_esql_query';

describe('getInitialESQLQuery', () => {
it('should work correctly', () => {
expect(getInitialESQLQuery('logs*')).toBe('from logs* | limit 10');
});
});
15 changes: 15 additions & 0 deletions packages/kbn-esql-utils/src/utils/get_initial_esql_query.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/*
* 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.
*/

/**
* Builds an ES|QL query for the provided index or index pattern
* @param indexOrIndexPattern
*/
export function getInitialESQLQuery(indexOrIndexPattern: string): string {
return `from ${indexOrIndexPattern} | limit 10`;
}
4 changes: 2 additions & 2 deletions src/plugins/discover/common/esql_locator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import { DISCOVER_ESQL_LOCATOR } from '@kbn/deeplinks-analytics';
import { LocatorDefinition, LocatorPublic } from '@kbn/share-plugin/common';
import { SerializableRecord } from '@kbn/utility-types';
import { getIndexForESQLQuery } from '@kbn/esql-utils';
import { getIndexForESQLQuery, getInitialESQLQuery } from '@kbn/esql-utils';
import type { DataViewsPublicPluginStart } from '@kbn/data-views-plugin/public';

export type DiscoverESQLLocatorParams = SerializableRecord;
Expand All @@ -30,7 +30,7 @@ export class DiscoverESQLLocatorDefinition implements LocatorDefinition<Discover
const { discoverAppLocator, getIndices } = this.deps;

const indexName = await getIndexForESQLQuery({ dataViews: { getIndices } });
const esql = `from ${indexName ?? '*'} | limit 10`;
const esql = getInitialESQLQuery(indexName ?? '*');

const params = {
query: { esql },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import useObservable from 'react-use/lib/useObservable';
import { reportPerformanceMetricEvent } from '@kbn/ebt-tools';
import { withSuspense } from '@kbn/shared-ux-utility';
import { isOfEsqlQueryType } from '@kbn/es-query';
import { getInitialESQLQuery } from '@kbn/esql-utils';
import { ESQL_TYPE } from '@kbn/data-view-utils';
import { useUrl } from './hooks/use_url';
import { useDiscoverStateContainer } from './hooks/use_discover_state_container';
import { MainHistoryLocationState } from '../../../common';
Expand All @@ -38,6 +40,8 @@ import {
} from '../../customizations';
import type { DiscoverCustomizationContext } from '../types';
import { DiscoverTopNavServerless } from './components/top_nav/discover_topnav_serverless';
import { isTextBasedQuery } from './utils/is_text_based_query';
import { DiscoverStateContainer, LoadParams } from './services/discover_state';

const DiscoverMainAppMemoized = memo(DiscoverMainApp);

Expand Down Expand Up @@ -148,7 +152,10 @@ export function DiscoverMainRoute({
}, [data.dataViews, savedSearchId, stateContainer.appState]);

const loadSavedSearch = useCallback(
async (nextDataView?: DataView) => {
async ({
nextDataView,
initialAppState,
}: { nextDataView?: DataView; initialAppState?: LoadParams['initialAppState'] } = {}) => {
const loadSavedSearchStartTime = window.performance.now();
setLoading(true);
if (!nextDataView && !(await checkData())) {
Expand All @@ -162,6 +169,7 @@ export function DiscoverMainRoute({
savedSearchId,
dataView: nextDataView,
dataViewSpec: historyLocationState?.dataViewSpec,
initialAppState,
});
if (customizationContext.displayMode === 'standalone') {
if (currentSavedSearch?.id) {
Expand Down Expand Up @@ -230,8 +238,12 @@ export function DiscoverMainRoute({
setHasUserDataView(false);
setShowNoDataPage(false);
setError(undefined);
// restore the previously selected data view for a new state
loadSavedSearch(!savedSearchId ? stateContainer.internalState.getState().dataView : undefined);
if (savedSearchId) {
loadSavedSearch();
} else {
// restore the previously selected data view for a new state (when a saved search was open)
loadSavedSearch(getLoadParamsForNewSearch(stateContainer));
}
}, [isCustomizationServiceInitialized, loadSavedSearch, savedSearchId, stateContainer]);

// secondary fetch: in case URL is set to `/`, used to reset to 'new' state, keeping the current data view
Expand All @@ -240,8 +252,7 @@ export function DiscoverMainRoute({
savedSearchId,
onNewUrl: () => {
// restore the previously selected data view for a new state
const dataView = stateContainer.internalState.getState().dataView;
loadSavedSearch(dataView);
loadSavedSearch(getLoadParamsForNewSearch(stateContainer));
},
});

Expand All @@ -251,7 +262,7 @@ export function DiscoverMainRoute({
setLoading(true);
setShowNoDataPage(false);
setError(undefined);
await loadSavedSearch(nextDataView as DataView);
await loadSavedSearch({ nextDataView: nextDataView as DataView });
}
},
[loadSavedSearch]
Expand Down Expand Up @@ -351,3 +362,27 @@ export function DiscoverMainRoute({
}
// eslint-disable-next-line import/no-default-export
export default DiscoverMainRoute;

function getLoadParamsForNewSearch(stateContainer: DiscoverStateContainer): {
nextDataView: LoadParams['dataView'];
initialAppState: LoadParams['initialAppState'];
} {
const prevAppState = stateContainer.appState.getState();
const prevDataView = stateContainer.internalState.getState().dataView;
const initialAppState =
prevAppState?.query &&
isTextBasedQuery(prevAppState.query) &&
prevDataView &&
prevDataView.type === ESQL_TYPE
? {
// reset to a default ES|QL query
query: {
esql: getInitialESQLQuery(prevDataView.getIndexPattern()),
},
}
: undefined;
return {
nextDataView: prevDataView,
initialAppState,
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ export interface LoadParams {
* the data view to use, if undefined, the saved search's data view will be used
*/
dataView?: DataView;
/**
* Custom initial app state for loading a saved search
*/
initialAppState?: DiscoverAppState;
/**
* the data view spec to use, if undefined, the saved search's data view will be used
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export const loadSavedSearch = async (
deps: LoadSavedSearchDeps
): Promise<SavedSearch> => {
addLog('[discoverState] loadSavedSearch');
const { savedSearchId } = params ?? {};
const { savedSearchId, initialAppState } = params ?? {};
const {
appStateContainer,
internalStateContainer,
Expand All @@ -54,7 +54,7 @@ export const loadSavedSearch = async (
services,
} = deps;
const appStateExists = !appStateContainer.isEmptyURL();
const appState = appStateExists ? appStateContainer.getState() : undefined;
const appState = appStateExists ? appStateContainer.getState() : initialAppState;

// Loading the saved search or creating a new one
let nextSavedSearch = savedSearchId
Expand Down
3 changes: 2 additions & 1 deletion src/plugins/discover/public/global_search/search_provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { i18n } from '@kbn/i18n';
import { DEFAULT_APP_CATEGORIES } from '@kbn/core/public';
import type { GlobalSearchResultProvider } from '@kbn/global-search-plugin/public';
import type { DataPublicPluginStart } from '@kbn/data-plugin/public';
import { getInitialESQLQuery } from '@kbn/esql-utils';
import type { DiscoverAppLocator } from '../../common';

/**
Expand Down Expand Up @@ -49,7 +50,7 @@ export const getESQLSearchProvider: (

const params = {
query: {
esql: `from ${defaultDataView?.getIndexPattern()} | limit 10`,
esql: getInitialESQLQuery(defaultDataView?.getIndexPattern()),
},
dataViewSpec: defaultDataView?.toSpec(),
};
Expand Down
3 changes: 2 additions & 1 deletion src/plugins/discover/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@
"@kbn/esql-utils",
"@kbn/managed-content-badge",
"@kbn/deeplinks-analytics",
"@kbn/shared-ux-markdown"
"@kbn/shared-ux-markdown",
"@kbn/data-view-utils"
],
"exclude": ["target/**/*"]
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
import { METRIC_TYPE } from '@kbn/analytics';
import { useKibana } from '@kbn/kibana-react-plugin/public';
import { AggregateQuery, getLanguageDisplayName } from '@kbn/es-query';
import { getInitialESQLQuery } from '@kbn/esql-utils';
import type { DataView } from '@kbn/data-views-plugin/public';
import type { IUnifiedSearchPluginServices } from '../types';
import { type DataViewPickerPropsExtended } from './data_view_picker';
Expand Down Expand Up @@ -348,7 +349,7 @@ export function ChangeDataView({
color="success"
size="s"
fullWidth
onClick={() => onTextBasedSubmit({ esql: `from ${trigger.title} | limit 10` })}
onClick={() => onTextBasedSubmit({ esql: getInitialESQLQuery(trigger.title!) })}
data-test-subj="select-text-based-language-panel"
contentProps={{
css: {
Expand Down
130 changes: 130 additions & 0 deletions test/functional/apps/discover/group4/_new_search.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
/*
* 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.
*/

import expect from '@kbn/expect';
import { FtrProviderContext } from '../ftr_provider_context';

export default function ({ getService, getPageObjects }: FtrProviderContext) {
const esArchiver = getService('esArchiver');
const PageObjects = getPageObjects([
'common',
'discover',
'timePicker',
'header',
'unifiedSearch',
]);
const kibanaServer = getService('kibanaServer');
const filterBar = getService('filterBar');
const queryBar = getService('queryBar');
const monacoEditor = getService('monacoEditor');
const testSubjects = getService('testSubjects');
const security = getService('security');

describe('discover new search action', function () {
before(async function () {
await security.testUser.setRoles(['kibana_admin', 'test_logstash_reader']);
await kibanaServer.importExport.load('test/functional/fixtures/kbn_archiver/discover.json');
await esArchiver.loadIfNeeded('test/functional/fixtures/es_archiver/logstash_functional');
await kibanaServer.uiSettings.replace({ defaultIndex: 'logstash-*' });
await PageObjects.common.navigateToApp('discover');
await PageObjects.timePicker.setDefaultAbsoluteRange();
});

after(async () => {
await kibanaServer.importExport.unload('test/functional/fixtures/kbn_archiver/discover.json');
await esArchiver.unload('test/functional/fixtures/es_archiver/logstash_functional');
await kibanaServer.uiSettings.replace({});
await kibanaServer.savedObjects.cleanStandardList();
});

it('should work correctly for data view mode', async function () {
await filterBar.addFilter({ field: 'extension', operation: 'is', value: 'png' });
await PageObjects.header.waitUntilLoadingHasFinished();
await queryBar.setQuery('bytes > 15000');
await queryBar.submitQuery();
await PageObjects.header.waitUntilLoadingHasFinished();
await PageObjects.discover.waitUntilSearchingHasFinished();
expect(await PageObjects.discover.getHitCount()).to.be('353');
expect(await filterBar.hasFilter('extension', 'png')).to.be(true);
expect(await queryBar.getQueryString()).to.be('bytes > 15000');

await PageObjects.discover.clickNewSearchButton();
await PageObjects.discover.waitUntilSearchingHasFinished();
expect(await PageObjects.discover.getHitCount()).to.be('14,004');
expect(await filterBar.hasFilter('extension', 'png')).to.be(false);
expect(await queryBar.getQueryString()).to.be('');
});

it('should work correctly for a saved search in data view mode', async function () {
await PageObjects.discover.createAdHocDataView('logs*', true);
await filterBar.addFilter({ field: 'extension', operation: 'is', value: 'css' });
await PageObjects.header.waitUntilLoadingHasFinished();
await queryBar.setQuery('bytes > 100');
await queryBar.submitQuery();
await PageObjects.header.waitUntilLoadingHasFinished();
await PageObjects.discover.waitUntilSearchingHasFinished();
expect(await PageObjects.discover.getHitCount()).to.be('2,108');
expect(await filterBar.hasFilter('extension', 'css')).to.be(true);
expect(await queryBar.getQueryString()).to.be('bytes > 100');

await PageObjects.discover.saveSearch('adHoc');
await PageObjects.discover.waitUntilSearchingHasFinished();
expect(await PageObjects.discover.getHitCount()).to.be('2,108');

await PageObjects.discover.clickNewSearchButton();
await PageObjects.discover.waitUntilSearchingHasFinished();
expect(await PageObjects.discover.getHitCount()).to.be('14,004');
expect(await filterBar.hasFilter('extension', 'css')).to.be(false);
expect(await queryBar.getQueryString()).to.be('');
expect(
await PageObjects.unifiedSearch.getSelectedDataView('discover-dataView-switch-link')
).to.be('logs**');
expect(await PageObjects.discover.isAdHocDataViewSelected()).to.be(true);
});

it('should work correctly for ESQL mode', async () => {
await PageObjects.discover.selectTextBaseLang();

const testQuery = `from logstash-* | limit 100 | stats countB = count(bytes) by geo.dest | sort countB`;
await monacoEditor.setCodeEditorValue(testQuery);
await testSubjects.click('querySubmitButton');
await PageObjects.header.waitUntilLoadingHasFinished();
await PageObjects.discover.waitUntilSearchingHasFinished();
expect(await PageObjects.discover.getHitCountInt()).to.greaterThan(10);
await testSubjects.existOrFail('unifiedHistogramSuggestionSelector');

await PageObjects.discover.clickNewSearchButton();
await PageObjects.discover.waitUntilSearchingHasFinished();
expect(await monacoEditor.getCodeEditorValue()).to.be('from logstash-* | limit 10');
await testSubjects.missingOrFail('unifiedHistogramSuggestionSelector'); // histogram also updated
expect(await PageObjects.discover.getHitCount()).to.be('10');
});

it('should work correctly for a saved search in ESQL mode', async () => {
await PageObjects.discover.selectTextBaseLang();

const testQuery = `from logstash-* | limit 100 | stats countB = count(bytes) by geo.dest | sort countB`;
await monacoEditor.setCodeEditorValue(testQuery);
await testSubjects.click('querySubmitButton');
await PageObjects.header.waitUntilLoadingHasFinished();
await PageObjects.discover.waitUntilSearchingHasFinished();
expect(await PageObjects.discover.getHitCountInt()).to.greaterThan(10);
await testSubjects.existOrFail('unifiedHistogramSuggestionSelector');

await PageObjects.discover.saveSearch('esql');
await PageObjects.discover.waitUntilSearchingHasFinished();
expect(await PageObjects.discover.getHitCountInt()).to.greaterThan(10);

await PageObjects.discover.clickNewSearchButton();
await PageObjects.discover.waitUntilSearchingHasFinished();
expect(await monacoEditor.getCodeEditorValue()).to.be('from logstash-* | limit 10');
await testSubjects.missingOrFail('unifiedHistogramSuggestionSelector'); // histogram also updated
expect(await PageObjects.discover.getHitCount()).to.be('10');
});
});
}
1 change: 1 addition & 0 deletions test/functional/apps/discover/group4/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,6 @@ export default function ({ getService, loadTestFile }: FtrProviderContext) {
loadTestFile(require.resolve('./_data_view_edit'));
loadTestFile(require.resolve('./_field_list_new_fields'));
loadTestFile(require.resolve('./_request_cancellation'));
loadTestFile(require.resolve('./_new_search'));
});
}

0 comments on commit a786612

Please sign in to comment.