From 34d204afe881fa1e7db46fea56ced2016c38b5f1 Mon Sep 17 00:00:00 2001 From: Julia Rechkunova Date: Tue, 9 Apr 2024 14:14:15 +0200 Subject: [PATCH 1/3] [Discover][ES|QL] No legacy table for ES|QL mode --- packages/kbn-discover-utils/index.ts | 1 + .../kbn-discover-utils/src/utils/index.ts | 1 + .../src/utils/is_legacy_table_enabled.ts | 24 +++++++++++++++++++ .../components/layout/discover_documents.tsx | 10 +++++--- .../components/top_nav/on_save_search.tsx | 8 +++++-- .../view_mode_toggle/view_mode_toggle.tsx | 7 ++++-- .../embeddable/saved_search_embeddable.tsx | 10 +++++--- .../components/doc_viewer_source/source.tsx | 7 ++++-- .../unified_doc_viewer/public/plugin.tsx | 10 ++++++-- 9 files changed, 64 insertions(+), 14 deletions(-) create mode 100644 packages/kbn-discover-utils/src/utils/is_legacy_table_enabled.ts diff --git a/packages/kbn-discover-utils/index.ts b/packages/kbn-discover-utils/index.ts index 5eb2650482611..a409962230ce6 100644 --- a/packages/kbn-discover-utils/index.ts +++ b/packages/kbn-discover-utils/index.ts @@ -37,5 +37,6 @@ export { getIgnoredReason, getShouldShowFieldHandler, isNestedFieldParent, + isLegacyTableEnabled, usePager, } from './src'; diff --git a/packages/kbn-discover-utils/src/utils/index.ts b/packages/kbn-discover-utils/src/utils/index.ts index 4828fcf82a447..8415fc7df0710 100644 --- a/packages/kbn-discover-utils/src/utils/index.ts +++ b/packages/kbn-discover-utils/src/utils/index.ts @@ -13,3 +13,4 @@ export * from './get_doc_id'; export * from './get_ignored_reason'; export * from './get_should_show_field_handler'; export * from './nested_fields'; +export { isLegacyTableEnabled } from './is_legacy_table_enabled'; diff --git a/packages/kbn-discover-utils/src/utils/is_legacy_table_enabled.ts b/packages/kbn-discover-utils/src/utils/is_legacy_table_enabled.ts new file mode 100644 index 0000000000000..7e575cf80dbfb --- /dev/null +++ b/packages/kbn-discover-utils/src/utils/is_legacy_table_enabled.ts @@ -0,0 +1,24 @@ +/* + * 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 type { IUiSettingsClient } from '@kbn/core-ui-settings-browser'; +import { DOC_TABLE_LEGACY } from '../constants'; + +export function isLegacyTableEnabled({ + uiSettings, + isTextBasedQueryMode, +}: { + uiSettings: IUiSettingsClient; + isTextBasedQueryMode: boolean; +}): boolean { + if (isTextBasedQueryMode) { + return false; // only show the new data grid + } + + return uiSettings.get(DOC_TABLE_LEGACY); +} diff --git a/src/plugins/discover/public/application/main/components/layout/discover_documents.tsx b/src/plugins/discover/public/application/main/components/layout/discover_documents.tsx index 4c0f01d04b6eb..6d123ab057deb 100644 --- a/src/plugins/discover/public/application/main/components/layout/discover_documents.tsx +++ b/src/plugins/discover/public/application/main/components/layout/discover_documents.tsx @@ -29,8 +29,8 @@ import { } from '@kbn/unified-data-table'; import { DOC_HIDE_TIME_COLUMN_SETTING, - DOC_TABLE_LEGACY, HIDE_ANNOUNCEMENTS, + isLegacyTableEnabled, MAX_DOC_FIELDS_DISPLAYED, ROW_HEIGHT_OPTION, SEARCH_FIELDS_FROM_SOURCE, @@ -140,15 +140,19 @@ function DiscoverDocumentsComponent({ const expandedDoc = useInternalStateSelector((state) => state.expandedDoc); + const isTextBasedQuery = useMemo(() => getRawRecordType(query) === RecordRawType.PLAIN, [query]); const useNewFieldsApi = useMemo(() => !uiSettings.get(SEARCH_FIELDS_FROM_SOURCE), [uiSettings]); const hideAnnouncements = useMemo(() => uiSettings.get(HIDE_ANNOUNCEMENTS), [uiSettings]); - const isLegacy = useMemo(() => uiSettings.get(DOC_TABLE_LEGACY), [uiSettings]); + const isLegacy = useMemo( + () => isLegacyTableEnabled({ uiSettings, isTextBasedQueryMode: isTextBasedQuery }), + [uiSettings, isTextBasedQuery] + ); const documentState = useDataState(documents$); const isDataLoading = documentState.fetchStatus === FetchStatus.LOADING || documentState.fetchStatus === FetchStatus.PARTIAL; - const isTextBasedQuery = useMemo(() => getRawRecordType(query) === RecordRawType.PLAIN, [query]); + // This is needed to prevent EuiDataGrid pushing onSort because the data view has been switched. // It's just necessary for non-text-based query lang requests since they don't have a partial result state, that's // considered as loading state in the Component. diff --git a/src/plugins/discover/public/application/main/components/top_nav/on_save_search.tsx b/src/plugins/discover/public/application/main/components/top_nav/on_save_search.tsx index f22d07b4d4d89..6414d7b5685fa 100644 --- a/src/plugins/discover/public/application/main/components/top_nav/on_save_search.tsx +++ b/src/plugins/discover/public/application/main/components/top_nav/on_save_search.tsx @@ -10,9 +10,10 @@ import React, { useState } from 'react'; import { i18n } from '@kbn/i18n'; import { EuiFormRow, EuiSwitch } from '@elastic/eui'; import { FormattedMessage } from '@kbn/i18n-react'; +import { isOfAggregateQueryType } from '@kbn/es-query'; import { SavedObjectSaveModal, showSaveModal, OnSaveProps } from '@kbn/saved-objects-plugin/public'; import { SavedSearch, SaveSavedSearchOptions } from '@kbn/saved-search-plugin/public'; -import { DOC_TABLE_LEGACY } from '@kbn/discover-utils'; +import { isLegacyTableEnabled } from '@kbn/discover-utils'; import { DiscoverServices } from '../../../../build_services'; import { DiscoverStateContainer } from '../../services/discover_state'; import { getAllowedSampleSize } from '../../../../utils/get_allowed_sample_size'; @@ -123,7 +124,10 @@ export async function onSaveSearch({ savedSearch.title = newTitle; savedSearch.description = newDescription; savedSearch.timeRestore = newTimeRestore; - savedSearch.rowsPerPage = uiSettings.get(DOC_TABLE_LEGACY) + savedSearch.rowsPerPage = isLegacyTableEnabled({ + uiSettings, + isTextBasedQueryMode: isOfAggregateQueryType(savedSearch.searchSource.getField('query')), + }) ? currentRowsPerPage : state.appState.getState().rowsPerPage; diff --git a/src/plugins/discover/public/components/view_mode_toggle/view_mode_toggle.tsx b/src/plugins/discover/public/components/view_mode_toggle/view_mode_toggle.tsx index 147486ac6dc6e..98b9ac12ef056 100644 --- a/src/plugins/discover/public/components/view_mode_toggle/view_mode_toggle.tsx +++ b/src/plugins/discover/public/components/view_mode_toggle/view_mode_toggle.tsx @@ -10,7 +10,7 @@ import React, { useMemo, ReactElement } from 'react'; import { EuiFlexGroup, EuiFlexItem, EuiTab, EuiTabs, useEuiTheme } from '@elastic/eui'; import { FormattedMessage } from '@kbn/i18n-react'; import { css } from '@emotion/react'; -import { DOC_TABLE_LEGACY, SHOW_FIELD_STATISTICS } from '@kbn/discover-utils'; +import { isLegacyTableEnabled, SHOW_FIELD_STATISTICS } from '@kbn/discover-utils'; import { VIEW_MODE } from '../../../common/constants'; import { useDiscoverServices } from '../../hooks/use_discover_services'; import { DiscoverStateContainer } from '../../application/main/services/discover_state'; @@ -31,7 +31,10 @@ export const DocumentViewModeToggle = ({ }) => { const { euiTheme } = useEuiTheme(); const { uiSettings } = useDiscoverServices(); - const isLegacy = useMemo(() => uiSettings.get(DOC_TABLE_LEGACY), [uiSettings]); + const isLegacy = useMemo( + () => isLegacyTableEnabled({ uiSettings, isTextBasedQueryMode: isTextBasedQuery }), + [uiSettings, isTextBasedQuery] + ); const includesNormalTabsStyle = viewMode === VIEW_MODE.AGGREGATED_LEVEL || isLegacy; const containerPadding = includesNormalTabsStyle ? euiTheme.size.s : 0; diff --git a/src/plugins/discover/public/embeddable/saved_search_embeddable.tsx b/src/plugins/discover/public/embeddable/saved_search_embeddable.tsx index 76fda6fee1a98..e8b0163923162 100644 --- a/src/plugins/discover/public/embeddable/saved_search_embeddable.tsx +++ b/src/plugins/discover/public/embeddable/saved_search_embeddable.tsx @@ -49,11 +49,11 @@ import type { SearchResponseWarning } from '@kbn/search-response-warnings'; import type { EsHitRecord } from '@kbn/discover-utils/types'; import { DOC_HIDE_TIME_COLUMN_SETTING, - DOC_TABLE_LEGACY, SEARCH_FIELDS_FROM_SOURCE, SHOW_FIELD_STATISTICS, SORT_DEFAULT_ORDER_SETTING, buildDataTableRecord, + isLegacyTableEnabled, } from '@kbn/discover-utils'; import { columnActions, getTextBasedColumnsMeta } from '@kbn/unified-data-table'; import { VIEW_MODE, getDefaultRowsPerPage } from '../../common/constants'; @@ -629,9 +629,10 @@ export class SavedSearchEmbeddable return; } + const isTextBasedQueryMode = this.isTextBasedSearch(savedSearch); const viewMode = getValidViewMode({ viewMode: savedSearch.viewMode, - isTextBasedQueryMode: this.isTextBasedSearch(savedSearch), + isTextBasedQueryMode, }); if ( @@ -669,7 +670,10 @@ export class SavedSearchEmbeddable return; } - const useLegacyTable = this.services.uiSettings.get(DOC_TABLE_LEGACY); + const useLegacyTable = isLegacyTableEnabled({ + uiSettings: this.services.uiSettings, + isTextBasedQueryMode, + }); const query = savedSearch.searchSource.getField('query'); const props = { savedSearch, diff --git a/src/plugins/unified_doc_viewer/public/components/doc_viewer_source/source.tsx b/src/plugins/unified_doc_viewer/public/components/doc_viewer_source/source.tsx index 189cbd2b5be4a..26ac6cb01b71e 100644 --- a/src/plugins/unified_doc_viewer/public/components/doc_viewer_source/source.tsx +++ b/src/plugins/unified_doc_viewer/public/components/doc_viewer_source/source.tsx @@ -15,7 +15,7 @@ import { i18n } from '@kbn/i18n'; import type { DataView } from '@kbn/data-views-plugin/public'; import type { DataTableRecord } from '@kbn/discover-utils/types'; import { ElasticRequestState } from '@kbn/unified-doc-viewer'; -import { DOC_TABLE_LEGACY, SEARCH_FIELDS_FROM_SOURCE } from '@kbn/discover-utils'; +import { isLegacyTableEnabled, SEARCH_FIELDS_FROM_SOURCE } from '@kbn/discover-utils'; import { getUnifiedDocViewerServices } from '../../plugin'; import { useEsDocSearch } from '../../hooks'; import { getHeight } from './get_height'; @@ -54,7 +54,10 @@ export const DocViewerSource = ({ const [jsonValue, setJsonValue] = useState(''); const { uiSettings } = getUnifiedDocViewerServices(); const useNewFieldsApi = !uiSettings.get(SEARCH_FIELDS_FROM_SOURCE); - const useDocExplorer = !uiSettings.get(DOC_TABLE_LEGACY); + const useDocExplorer = isLegacyTableEnabled({ + uiSettings, + isTextBasedQueryMode: Array.isArray(textBasedHits), + }); const [requestState, hit] = useEsDocSearch({ id, index, diff --git a/src/plugins/unified_doc_viewer/public/plugin.tsx b/src/plugins/unified_doc_viewer/public/plugin.tsx index 4b3793872b454..6dc4527441822 100644 --- a/src/plugins/unified_doc_viewer/public/plugin.tsx +++ b/src/plugins/unified_doc_viewer/public/plugin.tsx @@ -8,7 +8,7 @@ import React from 'react'; import type { CoreSetup, Plugin } from '@kbn/core/public'; -import { DOC_TABLE_LEGACY } from '@kbn/discover-utils'; +import { isLegacyTableEnabled } from '@kbn/discover-utils'; import { i18n } from '@kbn/i18n'; import { DocViewsRegistry } from '@kbn/unified-doc-viewer'; import { EuiDelayRender, EuiSkeletonText } from '@elastic/eui'; @@ -51,8 +51,14 @@ export class UnifiedDocViewerPublicPlugin }), order: 10, component: (props) => { + const { textBasedHits } = props; const { uiSettings } = getUnifiedDocViewerServices(); - const DocView = uiSettings.get(DOC_TABLE_LEGACY) ? DocViewerLegacyTable : DocViewerTable; + const DocView = isLegacyTableEnabled({ + uiSettings, + isTextBasedQueryMode: Array.isArray(textBasedHits), + }) + ? DocViewerLegacyTable + : DocViewerTable; return ( Date: Tue, 9 Apr 2024 15:10:42 +0200 Subject: [PATCH 2/3] [Discover][ES|QL] Add tests --- .../apps/discover/classic/_esql_grid.ts | 95 +++++++++++++++++++ .../functional/apps/discover/classic/index.ts | 1 + .../discover/group3/_time_field_column.ts | 22 ++--- 3 files changed, 102 insertions(+), 16 deletions(-) create mode 100644 test/functional/apps/discover/classic/_esql_grid.ts diff --git a/test/functional/apps/discover/classic/_esql_grid.ts b/test/functional/apps/discover/classic/_esql_grid.ts new file mode 100644 index 0000000000000..7dfd331acec12 --- /dev/null +++ b/test/functional/apps/discover/classic/_esql_grid.ts @@ -0,0 +1,95 @@ +/* + * 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 { FtrProviderContext } from '../ftr_provider_context'; + +export default function ({ getService, getPageObjects }: FtrProviderContext) { + const esArchiver = getService('esArchiver'); + const kibanaServer = getService('kibanaServer'); + const testSubjects = getService('testSubjects'); + const security = getService('security'); + const dataGrid = getService('dataGrid'); + const dashboardAddPanel = getService('dashboardAddPanel'); + const dashboardPanelActions = getService('dashboardPanelActions'); + const PageObjects = getPageObjects([ + 'common', + 'discover', + 'dashboard', + 'header', + 'timePicker', + 'unifiedFieldList', + ]); + + const defaultSettings = { + defaultIndex: 'logstash-*', + 'doc_table:legacy': true, + }; + + describe('discover esql grid with legacy setting', async function () { + before(async () => { + await security.testUser.setRoles(['kibana_admin', 'test_logstash_reader']); + await kibanaServer.importExport.load('test/functional/fixtures/kbn_archiver/discover'); + await esArchiver.loadIfNeeded('test/functional/fixtures/es_archiver/logstash_functional'); + await kibanaServer.uiSettings.replace(defaultSettings); + await PageObjects.common.navigateToApp('discover'); + await PageObjects.timePicker.setDefaultAbsoluteRange(); + }); + + after(async () => { + await kibanaServer.savedObjects.cleanStandardList(); + await kibanaServer.importExport.unload('test/functional/fixtures/kbn_archiver/discover'); + await esArchiver.unload('test/functional/fixtures/es_archiver/logstash_functional'); + await kibanaServer.uiSettings.replace({}); + }); + + it('should render esql view correctly', async function () { + const savedSearchESQL = 'testESQLWithLegacySetting'; + await PageObjects.header.waitUntilLoadingHasFinished(); + await PageObjects.discover.waitUntilSearchingHasFinished(); + + await testSubjects.existOrFail('docTableHeader'); + await testSubjects.missingOrFail('euiDataGridBody'); + + await PageObjects.discover.selectTextBaseLang(); + + await PageObjects.header.waitUntilLoadingHasFinished(); + await PageObjects.discover.waitUntilSearchingHasFinished(); + + await testSubjects.missingOrFail('docTableHeader'); + await testSubjects.existOrFail('euiDataGridBody'); + + await dataGrid.clickRowToggle({ rowIndex: 0 }); + + await testSubjects.existOrFail('docTableDetailsFlyout'); + + await PageObjects.discover.saveSearch(savedSearchESQL); + + await PageObjects.common.navigateToApp('dashboard'); + await PageObjects.dashboard.clickNewDashboard(); + await PageObjects.timePicker.setDefaultAbsoluteRange(); + await dashboardAddPanel.clickOpenAddPanel(); + await dashboardAddPanel.addSavedSearch(savedSearchESQL); + await PageObjects.header.waitUntilLoadingHasFinished(); + + await testSubjects.missingOrFail('docTableHeader'); + await testSubjects.existOrFail('euiDataGridBody'); + + await dataGrid.clickRowToggle({ rowIndex: 0 }); + + await testSubjects.existOrFail('docTableDetailsFlyout'); + + await dashboardPanelActions.removePanelByTitle(savedSearchESQL); + + await dashboardAddPanel.addSavedSearch('A Saved Search'); + + await PageObjects.header.waitUntilLoadingHasFinished(); + await testSubjects.existOrFail('docTableHeader'); + await testSubjects.missingOrFail('euiDataGridBody'); + }); + }); +} diff --git a/test/functional/apps/discover/classic/index.ts b/test/functional/apps/discover/classic/index.ts index 845b5e2ee5ece..e6dc2f17c7699 100644 --- a/test/functional/apps/discover/classic/index.ts +++ b/test/functional/apps/discover/classic/index.ts @@ -27,5 +27,6 @@ export default function ({ getService, loadTestFile }: FtrProviderContext) { loadTestFile(require.resolve('./_field_data_with_fields_api')); loadTestFile(require.resolve('./_classic_table_doc_navigation')); loadTestFile(require.resolve('./_hide_announcements')); + loadTestFile(require.resolve('./_esql_grid')); }); } diff --git a/test/functional/apps/discover/group3/_time_field_column.ts b/test/functional/apps/discover/group3/_time_field_column.ts index 8ed5188151bad..01bd81bddc444 100644 --- a/test/functional/apps/discover/group3/_time_field_column.ts +++ b/test/functional/apps/discover/group3/_time_field_column.ts @@ -346,7 +346,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { `${SEARCH_NO_COLUMNS}${savedSearchSuffix}ESQL` ); await PageObjects.discover.waitUntilSearchingHasFinished(); - expect(await docTable.getHeaderFields()).to.eql( + expect(await dataGrid.getHeaderFields()).to.eql( hideTimeFieldColumnSetting ? ['Document'] : ['@timestamp', 'Document'] ); @@ -354,9 +354,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { `${SEARCH_NO_COLUMNS}${savedSearchSuffix}ESQLdrop` ); await PageObjects.discover.waitUntilSearchingHasFinished(); - expect(await docTable.getHeaderFields()).to.eql( - hideTimeFieldColumnSetting ? ['Document'] : ['@timestamp', 'Document'] - ); + expect(await dataGrid.getHeaderFields()).to.eql(['Document']); // only @timestamp is selected await PageObjects.discover.loadSavedSearch( @@ -377,8 +375,8 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { `${SEARCH_WITH_ONLY_TIMESTAMP}${savedSearchSuffix}ESQL` ); await PageObjects.discover.waitUntilSearchingHasFinished(); - expect(await docTable.getHeaderFields()).to.eql( - hideTimeFieldColumnSetting ? ['@timestamp'] : ['@timestamp', '@timestamp'] + expect(await dataGrid.getHeaderFields()).to.eql( + hideTimeFieldColumnSetting ? ['@timestamp'] : ['@timestamp', 'Document'] ); }); @@ -404,11 +402,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { `${SEARCH_WITH_SELECTED_COLUMNS}${savedSearchSuffix}ESQL` ); await PageObjects.discover.waitUntilSearchingHasFinished(); - expect(await docTable.getHeaderFields()).to.eql( - hideTimeFieldColumnSetting - ? ['bytes', 'extension'] - : ['@timestamp', 'bytes', 'extension'] - ); + expect(await dataGrid.getHeaderFields()).to.eql(['bytes', 'extension']); // with selected columns and @timestamp await PageObjects.discover.loadSavedSearch( @@ -431,11 +425,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { `${SEARCH_WITH_SELECTED_COLUMNS_AND_TIMESTAMP}${savedSearchSuffix}ESQL` ); await PageObjects.discover.waitUntilSearchingHasFinished(); - expect(await docTable.getHeaderFields()).to.eql( - hideTimeFieldColumnSetting - ? ['bytes', 'extension', '@timestamp'] - : ['@timestamp', 'bytes', 'extension', '@timestamp'] - ); + expect(await dataGrid.getHeaderFields()).to.eql(['bytes', 'extension', '@timestamp']); }); }); }); From 0eba55a0310c5064632845ab71704818fc1b58da Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Tue, 9 Apr 2024 13:21:54 +0000 Subject: [PATCH 3/3] [CI] Auto-commit changed files from 'node scripts/lint_ts_projects --fix' --- packages/kbn-discover-utils/tsconfig.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/kbn-discover-utils/tsconfig.json b/packages/kbn-discover-utils/tsconfig.json index 0051ad5b2a00a..64453f8245afe 100644 --- a/packages/kbn-discover-utils/tsconfig.json +++ b/packages/kbn-discover-utils/tsconfig.json @@ -21,6 +21,7 @@ "@kbn/es-query", "@kbn/field-formats-plugin", "@kbn/field-types", - "@kbn/i18n" + "@kbn/i18n", + "@kbn/core-ui-settings-browser" ] }