From fd399ced0f296685e8864289ef4ad7eb10466342 Mon Sep 17 00:00:00 2001 From: Julia Rechkunova Date: Wed, 10 May 2023 15:29:11 +0200 Subject: [PATCH 01/32] [Discover] Allow to change current sample size and save it with a saved search --- .../components/layout/discover_documents.tsx | 18 ++-- .../components/top_nav/on_save_search.tsx | 5 ++ .../main/hooks/utils/build_state_subscribe.ts | 4 +- .../services/discover_app_state_container.ts | 4 + .../main/utils/cleanup_url_state.ts | 8 ++ .../application/main/utils/fetch_documents.ts | 5 +- .../main/utils/get_state_defaults.ts | 5 +- .../discover_grid/discover_grid.tsx | 47 ++++++---- .../discover_grid_settings_popover.tsx | 84 ++++++++++++++++++ .../common/saved_searches_utils.ts | 1 + src/plugins/saved_search/common/types.ts | 2 + .../saved_searches/saved_searches_utils.ts | 1 + .../server/saved_objects/schema.ts | 86 +++++++++++++++++++ .../server/saved_objects/search.ts | 74 +--------------- 14 files changed, 248 insertions(+), 96 deletions(-) create mode 100644 src/plugins/discover/public/components/discover_grid/discover_grid_settings_popover.tsx create mode 100644 src/plugins/saved_search/server/saved_objects/schema.ts 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 3070f447712c5..7cb1ce51845c6 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 @@ -86,8 +86,8 @@ function DiscoverDocumentsComponent({ const documents$ = stateContainer.dataState.data$.documents$; const savedSearch = useSavedSearchInitial(); const { dataViews, capabilities, uiSettings } = services; - const [query, sort, rowHeight, rowsPerPage, grid, columns, index] = useAppStateSelector( - (state) => { + const [query, sort, rowHeight, rowsPerPage, grid, columns, index, sampleSizeState] = + useAppStateSelector((state) => { return [ state.query, state.sort, @@ -96,9 +96,9 @@ function DiscoverDocumentsComponent({ state.grid, state.columns, state.index, + state.sampleSize, ]; - } - ); + }); const setExpandedDoc = useCallback( (doc: DataTableRecord | undefined) => { stateContainer.internalState.transitions.setExpandedDoc(doc); @@ -156,6 +156,13 @@ function DiscoverDocumentsComponent({ [stateContainer] ); + const onUpdateSampleSize = useCallback( + (newSampleSize: number) => { + stateContainer.appState.update({ sampleSize: newSampleSize }); + }, + [stateContainer] + ); + const onSort = useCallback( (nextSort: string[][]) => { stateContainer.appState.update({ sort: nextSort }); @@ -235,7 +242,6 @@ function DiscoverDocumentsComponent({ isLoading={isDataLoading} rows={rows} sort={(sort as SortOrder[]) || []} - sampleSize={sampleSize} searchDescription={savedSearch.description} searchTitle={savedSearch.title} setExpandedDoc={!isPlainRecord ? setExpandedDoc : undefined} @@ -254,6 +260,8 @@ function DiscoverDocumentsComponent({ isPlainRecord={isPlainRecord} rowsPerPageState={rowsPerPage} onUpdateRowsPerPage={onUpdateRowsPerPage} + sampleSizeState={sampleSizeState || sampleSize} + onUpdateSampleSize={onUpdateSampleSize} onFieldEdited={onFieldEdited} savedSearchId={savedSearch.id} DocumentView={DiscoverGridFlyout} 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 7addf5edef9f9..b43c694bf0bf8 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 @@ -114,6 +114,7 @@ export async function onSaveSearch({ const currentTitle = savedSearch.title; const currentTimeRestore = savedSearch.timeRestore; const currentRowsPerPage = savedSearch.rowsPerPage; + const currentSampleSize = savedSearch.sampleSize; const currentDescription = savedSearch.description; const currentTags = savedSearch.tags; savedSearch.title = newTitle; @@ -122,6 +123,9 @@ export async function onSaveSearch({ savedSearch.rowsPerPage = uiSettings.get(DOC_TABLE_LEGACY) ? currentRowsPerPage : state.appState.getState().rowsPerPage; + savedSearch.sampleSize = uiSettings.get(DOC_TABLE_LEGACY) + ? currentSampleSize + : state.appState.getState().sampleSize; if (savedObjectsTagging) { savedSearch.tags = newTags; } @@ -149,6 +153,7 @@ export async function onSaveSearch({ savedSearch.title = currentTitle; savedSearch.timeRestore = currentTimeRestore; savedSearch.rowsPerPage = currentRowsPerPage; + savedSearch.sampleSize = currentSampleSize; savedSearch.description = currentDescription; if (savedObjectsTagging) { savedSearch.tags = currentTags; diff --git a/src/plugins/discover/public/application/main/hooks/utils/build_state_subscribe.ts b/src/plugins/discover/public/application/main/hooks/utils/build_state_subscribe.ts index 1799e5146c803..18d753ed9db2b 100644 --- a/src/plugins/discover/public/application/main/hooks/utils/build_state_subscribe.ts +++ b/src/plugins/discover/public/application/main/hooks/utils/build_state_subscribe.ts @@ -49,12 +49,13 @@ export const buildStateSubscribe = return; } addLog('[appstate] subscribe triggered', nextState); - const { hideChart, interval, breakdownField, sort, index } = appState.getPrevious(); + const { hideChart, interval, breakdownField, sampleSize, sort, index } = appState.getPrevious(); // Cast to boolean to avoid false positives when comparing // undefined and false, which would trigger a refetch const chartDisplayChanged = Boolean(nextState.hideChart) !== Boolean(hideChart); const chartIntervalChanged = nextState.interval !== interval; const breakdownFieldChanged = nextState.breakdownField !== breakdownField; + const sampleSizeChanged = nextState.sampleSize !== sampleSize; const docTableSortChanged = !isEqual(nextState.sort, sort); const dataViewChanged = !isEqual(nextState.index, index); let savedSearchDataView; @@ -88,6 +89,7 @@ export const buildStateSubscribe = chartDisplayChanged || chartIntervalChanged || breakdownFieldChanged || + sampleSizeChanged || docTableSortChanged || dataViewChanged ) { diff --git a/src/plugins/discover/public/application/main/services/discover_app_state_container.ts b/src/plugins/discover/public/application/main/services/discover_app_state_container.ts index ba189bec26a1b..548a5c4224c61 100644 --- a/src/plugins/discover/public/application/main/services/discover_app_state_container.ts +++ b/src/plugins/discover/public/application/main/services/discover_app_state_container.ts @@ -124,6 +124,10 @@ export interface DiscoverAppState { * Number of rows in the grid per page */ rowsPerPage?: number; + /** + * Custom sample size + */ + sampleSize?: number; /** * Breakdown field of chart */ diff --git a/src/plugins/discover/public/application/main/utils/cleanup_url_state.ts b/src/plugins/discover/public/application/main/utils/cleanup_url_state.ts index 3abeed97d4cdc..464ef426ac7be 100644 --- a/src/plugins/discover/public/application/main/utils/cleanup_url_state.ts +++ b/src/plugins/discover/public/application/main/utils/cleanup_url_state.ts @@ -46,5 +46,13 @@ export function cleanupUrlState(appStateFromUrl: AppStateUrl): DiscoverAppState delete appStateFromUrl.rowsPerPage; } + if ( + appStateFromUrl?.sampleSize && + !(typeof appStateFromUrl.sampleSize === 'number' && appStateFromUrl.sampleSize > 0) + ) { + // remove the param if it's invalid + delete appStateFromUrl.sampleSize; + } + return appStateFromUrl as DiscoverAppState; } diff --git a/src/plugins/discover/public/application/main/utils/fetch_documents.ts b/src/plugins/discover/public/application/main/utils/fetch_documents.ts index db092cb449057..826eb1731aaf0 100644 --- a/src/plugins/discover/public/application/main/utils/fetch_documents.ts +++ b/src/plugins/discover/public/application/main/utils/fetch_documents.ts @@ -20,9 +20,10 @@ import { FetchDeps } from './fetch_all'; */ export const fetchDocuments = ( searchSource: ISearchSource, - { abortController, inspectorAdapters, searchSessionId, services }: FetchDeps + { abortController, inspectorAdapters, searchSessionId, services, getAppState }: FetchDeps ): Promise => { - searchSource.setField('size', services.uiSettings.get(SAMPLE_SIZE_SETTING)); + const sampleSize = getAppState().sampleSize; + searchSource.setField('size', sampleSize || services.uiSettings.get(SAMPLE_SIZE_SETTING)); searchSource.setField('trackTotalHits', false); searchSource.setField('highlightAll', true); searchSource.setField('version', true); diff --git a/src/plugins/discover/public/application/main/utils/get_state_defaults.ts b/src/plugins/discover/public/application/main/utils/get_state_defaults.ts index 1ec3be8c3ec1a..ff3821cac66b7 100644 --- a/src/plugins/discover/public/application/main/utils/get_state_defaults.ts +++ b/src/plugins/discover/public/application/main/utils/get_state_defaults.ts @@ -70,6 +70,7 @@ export function getStateDefaults({ savedQuery: undefined, rowHeight: undefined, rowsPerPage: undefined, + sampleSize: undefined, grid: undefined, breakdownField: undefined, }; @@ -94,7 +95,9 @@ export function getStateDefaults({ if (savedSearch.rowsPerPage) { defaultState.rowsPerPage = savedSearch.rowsPerPage; } - + if (savedSearch.sampleSize) { + defaultState.sampleSize = savedSearch.sampleSize; + } if (savedSearch.breakdownField) { defaultState.breakdownField = savedSearch.breakdownField; } diff --git a/src/plugins/discover/public/components/discover_grid/discover_grid.tsx b/src/plugins/discover/public/components/discover_grid/discover_grid.tsx index 35c51ab4c996c..803a4d3848438 100644 --- a/src/plugins/discover/public/components/discover_grid/discover_grid.tsx +++ b/src/plugins/discover/public/components/discover_grid/discover_grid.tsx @@ -46,6 +46,7 @@ import { SHOW_MULTIFIELDS, } from '../../../common'; import { DiscoverGridDocumentToolbarBtn } from './discover_grid_document_selection'; +import { DiscoverGridSettingsPopover } from './discover_grid_settings_popover'; import { getShouldShowFieldHandler } from '../../utils/get_should_show_field_handler'; import type { DataTableRecord, ValueToStringConverter } from '../../types'; import { useRowHeightsOptions } from '../../hooks/use_row_heights_options'; @@ -111,10 +112,6 @@ export interface DiscoverGridProps { * Array of documents provided by Elasticsearch */ rows?: DataTableRecord[]; - /** - * The max size of the documents returned by Elasticsearch - */ - sampleSize: number; /** * Function to set the expanded document, which is displayed in a flyout */ @@ -179,6 +176,14 @@ export interface DiscoverGridProps { * Update rows per page state */ onUpdateRowsPerPage?: (rowsPerPage: number) => void; + /** + * The max size of the documents returned by Elasticsearch + */ + sampleSizeState: number; + /** + * Update rows per page state + */ + onUpdateSampleSize?: (sampleSize: number) => void; /** * Callback to execute on edit runtime field */ @@ -226,7 +231,6 @@ export const DiscoverGrid = ({ onSetColumns, onSort, rows, - sampleSize, searchDescription, searchTitle, setExpandedDoc, @@ -241,6 +245,8 @@ export const DiscoverGrid = ({ className, rowHeightState, onUpdateRowHeight, + sampleSizeState, + onUpdateSampleSize, isPlainRecord = false, rowsPerPageState, onUpdateRowsPerPage, @@ -386,7 +392,7 @@ export const DiscoverGrid = ({ /** * Render variables */ - const showDisclaimer = rowCount === sampleSize && isOnLastPage; + const showDisclaimer = rowCount === sampleSizeState && isOnLastPage; const randomId = useMemo(() => htmlIdGenerator()(), []); const closeFieldEditor = useRef<() => void | undefined>(); @@ -482,17 +488,26 @@ export const DiscoverGrid = ({ ); const additionalControls = useMemo( - () => - usedSelectedDocs.length ? ( - ({ + left: { + append: usedSelectedDocs.length ? ( + + ) : null, + }, + right: onUpdateSampleSize ? ( + ) : null, - [usedSelectedDocs, isFilterActive, rows, setIsFilterActive] + }), + [usedSelectedDocs, isFilterActive, rows, sampleSizeState, setIsFilterActive, onUpdateSampleSize] ); const showDisplaySelector = useMemo( @@ -621,7 +636,7 @@ export const DiscoverGrid = ({ id="discover.gridSampleSize.limitDescription" defaultMessage="Search results are limited to {sampleSize} documents. Add more search terms to narrow your search." values={{ - sampleSize, + sampleSize: sampleSizeState, }} />

diff --git a/src/plugins/discover/public/components/discover_grid/discover_grid_settings_popover.tsx b/src/plugins/discover/public/components/discover_grid/discover_grid_settings_popover.tsx new file mode 100644 index 0000000000000..7ef3c4ec09511 --- /dev/null +++ b/src/plugins/discover/public/components/discover_grid/discover_grid_settings_popover.tsx @@ -0,0 +1,84 @@ +/* + * 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 React, { useCallback, useState } from 'react'; +import { EuiButtonIcon, EuiFormRow, EuiPopover, EuiRange, EuiToolTip } from '@elastic/eui'; +import { i18n } from '@kbn/i18n'; + +export function DiscoverGridSettingsPopover({ + sampleSize, + onChangeSampleSize, +}: { + sampleSize: number; + onChangeSampleSize: (sampleSize: number) => void; +}) { + const [isPopoverOpen, setIsPopoverOpen] = useState(false); + + const togglePopover = useCallback(() => { + setIsPopoverOpen((isOpen) => !isOpen); + }, []); + + const closePopover = useCallback(() => { + setIsPopoverOpen(false); + }, []); + + const onChangeSampleSizeInput = useCallback( + (event) => { + const newSampleSize = Number(event.target.value); + if (newSampleSize > 0) { + onChangeSampleSize(newSampleSize); + } + }, + [onChangeSampleSize] + ); + + const buttonLabel = i18n.translate('discover.grid.settingsPopover.buttonLabel', { + defaultMessage: 'Grid settings', + }); + + const sampleSizeLabel = i18n.translate('discover.grid.settingsPopover.sampleSizeLabel', { + defaultMessage: 'Sample size', + }); + + return ( + + + + } + > + + + + + ); +} diff --git a/src/plugins/saved_search/common/saved_searches_utils.ts b/src/plugins/saved_search/common/saved_searches_utils.ts index 41934b86a36d5..3210b02abe59c 100644 --- a/src/plugins/saved_search/common/saved_searches_utils.ts +++ b/src/plugins/saved_search/common/saved_searches_utils.ts @@ -32,5 +32,6 @@ export const fromSavedSearchAttributes = ( timeRange: attributes.timeRange, refreshInterval: attributes.refreshInterval, rowsPerPage: attributes.rowsPerPage, + sampleSize: attributes.sampleSize, breakdownField: attributes.breakdownField, }); diff --git a/src/plugins/saved_search/common/types.ts b/src/plugins/saved_search/common/types.ts index 319ff420c1697..ccf319c1e34bb 100644 --- a/src/plugins/saved_search/common/types.ts +++ b/src/plugins/saved_search/common/types.ts @@ -41,6 +41,7 @@ export interface SavedSearchAttributes { refreshInterval?: RefreshInterval; rowsPerPage?: number; + sampleSize?: number; breakdownField?: string; } @@ -72,5 +73,6 @@ export interface SavedSearch { refreshInterval?: RefreshInterval; rowsPerPage?: number; + sampleSize?: number; breakdownField?: string; } diff --git a/src/plugins/saved_search/public/services/saved_searches/saved_searches_utils.ts b/src/plugins/saved_search/public/services/saved_searches/saved_searches_utils.ts index fe15f630318c1..bc28fcbad6135 100644 --- a/src/plugins/saved_search/public/services/saved_searches/saved_searches_utils.ts +++ b/src/plugins/saved_search/public/services/saved_searches/saved_searches_utils.ts @@ -58,5 +58,6 @@ export const toSavedSearchAttributes = ( timeRange: savedSearch.timeRange ? pick(savedSearch.timeRange, ['from', 'to']) : undefined, refreshInterval: savedSearch.refreshInterval, rowsPerPage: savedSearch.rowsPerPage, + sampleSize: savedSearch.sampleSize, breakdownField: savedSearch.breakdownField, }); diff --git a/src/plugins/saved_search/server/saved_objects/schema.ts b/src/plugins/saved_search/server/saved_objects/schema.ts new file mode 100644 index 0000000000000..4ae325024bcc3 --- /dev/null +++ b/src/plugins/saved_search/server/saved_objects/schema.ts @@ -0,0 +1,86 @@ +/* + * 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 { schema } from '@kbn/config-schema'; +import { VIEW_MODE } from '../../common'; + +const SCHEMA_SEARCH_BASE = { + // General + title: schema.string(), + description: schema.string({ defaultValue: '' }), + + // Data grid + columns: schema.arrayOf(schema.string(), { defaultValue: [] }), + sort: schema.oneOf( + [ + schema.arrayOf(schema.arrayOf(schema.string(), { maxSize: 2 })), + schema.arrayOf(schema.string(), { maxSize: 2 }), + ], + { defaultValue: [] } + ), + grid: schema.object( + { + columns: schema.maybe( + schema.recordOf( + schema.string(), + schema.object({ + width: schema.maybe(schema.number()), + }) + ) + ), + }, + { defaultValue: {} } + ), + rowHeight: schema.maybe(schema.number()), + rowsPerPage: schema.maybe(schema.number()), + + // Chart + hideChart: schema.boolean({ defaultValue: false }), + breakdownField: schema.maybe(schema.string()), + + // Search + kibanaSavedObjectMeta: schema.object({ + searchSourceJSON: schema.string(), + }), + isTextBasedQuery: schema.boolean({ defaultValue: false }), + usesAdHocDataView: schema.maybe(schema.boolean()), + + // Time + timeRestore: schema.maybe(schema.boolean()), + timeRange: schema.maybe( + schema.object({ + from: schema.string(), + to: schema.string(), + }) + ), + refreshInterval: schema.maybe( + schema.object({ + pause: schema.boolean(), + value: schema.number(), + }) + ), + + // Display + viewMode: schema.maybe( + schema.oneOf([ + schema.literal(VIEW_MODE.DOCUMENT_LEVEL), + schema.literal(VIEW_MODE.AGGREGATED_LEVEL), + ]) + ), + hideAggregatedPreview: schema.maybe(schema.boolean()), + + // Legacy + hits: schema.maybe(schema.number()), + version: schema.maybe(schema.number()), +}; + +export const SCHEMA_SEARCH_V8_8_0 = schema.object(SCHEMA_SEARCH_BASE); +export const SCHEMA_SEARCH_V8_9_0 = schema.object({ + ...SCHEMA_SEARCH_BASE, + sampleSize: schema.maybe(schema.number()), +}); diff --git a/src/plugins/saved_search/server/saved_objects/search.ts b/src/plugins/saved_search/server/saved_objects/search.ts index 9b78f5ea4aecb..9c1e303c91da5 100644 --- a/src/plugins/saved_search/server/saved_objects/search.ts +++ b/src/plugins/saved_search/server/saved_objects/search.ts @@ -6,12 +6,11 @@ * Side Public License, v 1. */ -import { schema } from '@kbn/config-schema'; import { ANALYTICS_SAVED_OBJECT_INDEX } from '@kbn/core-saved-objects-server'; import { SavedObjectsType } from '@kbn/core/server'; import { MigrateFunctionsObject } from '@kbn/kibana-utils-plugin/common'; -import { VIEW_MODE } from '../../common'; import { getAllMigrations } from './search_migrations'; +import { SCHEMA_SEARCH_V8_8_0, SCHEMA_SEARCH_V8_9_0 } from './schema'; export function getSavedSearchObjectType( getSearchSourceMigrations: () => MigrateFunctionsObject @@ -44,75 +43,8 @@ export function getSavedSearchObjectType( }, }, schemas: { - '8.8.0': schema.object({ - // General - title: schema.string(), - description: schema.string({ defaultValue: '' }), - - // Data grid - columns: schema.arrayOf(schema.string(), { defaultValue: [] }), - sort: schema.oneOf( - [ - schema.arrayOf(schema.arrayOf(schema.string(), { maxSize: 2 })), - schema.arrayOf(schema.string(), { maxSize: 2 }), - ], - { defaultValue: [] } - ), - grid: schema.object( - { - columns: schema.maybe( - schema.recordOf( - schema.string(), - schema.object({ - width: schema.maybe(schema.number()), - }) - ) - ), - }, - { defaultValue: {} } - ), - rowHeight: schema.maybe(schema.number()), - rowsPerPage: schema.maybe(schema.number()), - - // Chart - hideChart: schema.boolean({ defaultValue: false }), - breakdownField: schema.maybe(schema.string()), - - // Search - kibanaSavedObjectMeta: schema.object({ - searchSourceJSON: schema.string(), - }), - isTextBasedQuery: schema.boolean({ defaultValue: false }), - usesAdHocDataView: schema.maybe(schema.boolean()), - - // Time - timeRestore: schema.maybe(schema.boolean()), - timeRange: schema.maybe( - schema.object({ - from: schema.string(), - to: schema.string(), - }) - ), - refreshInterval: schema.maybe( - schema.object({ - pause: schema.boolean(), - value: schema.number(), - }) - ), - - // Display - viewMode: schema.maybe( - schema.oneOf([ - schema.literal(VIEW_MODE.DOCUMENT_LEVEL), - schema.literal(VIEW_MODE.AGGREGATED_LEVEL), - ]) - ), - hideAggregatedPreview: schema.maybe(schema.boolean()), - - // Legacy - hits: schema.maybe(schema.number()), - version: schema.maybe(schema.number()), - }), + '8.8.0': SCHEMA_SEARCH_V8_8_0, + '8.9.0': SCHEMA_SEARCH_V8_9_0, }, migrations: () => getAllMigrations(getSearchSourceMigrations()), }; From 6fdae5c82932781917de7dbd18c1c8a0f74d1654 Mon Sep 17 00:00:00 2001 From: Julia Rechkunova Date: Wed, 10 May 2023 16:21:20 +0200 Subject: [PATCH 02/32] [Discover] Search embaddable supports a custom sample size --- .../discover_grid_settings_popover.tsx | 26 ++++++++++++++----- .../embeddable/saved_search_embeddable.tsx | 17 ++++++++++-- .../discover/public/embeddable/types.ts | 1 + .../embeddable/utils/update_search_source.ts | 7 ++--- 4 files changed, 40 insertions(+), 11 deletions(-) diff --git a/src/plugins/discover/public/components/discover_grid/discover_grid_settings_popover.tsx b/src/plugins/discover/public/components/discover_grid/discover_grid_settings_popover.tsx index 7ef3c4ec09511..a6889734c41fb 100644 --- a/src/plugins/discover/public/components/discover_grid/discover_grid_settings_popover.tsx +++ b/src/plugins/discover/public/components/discover_grid/discover_grid_settings_popover.tsx @@ -6,9 +6,10 @@ * Side Public License, v 1. */ -import React, { useCallback, useState } from 'react'; +import React, { useCallback, useEffect, useState } from 'react'; import { EuiButtonIcon, EuiFormRow, EuiPopover, EuiRange, EuiToolTip } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; +import useDebounce from 'react-use/lib/useDebounce'; export function DiscoverGridSettingsPopover({ sampleSize, @@ -18,6 +19,7 @@ export function DiscoverGridSettingsPopover({ onChangeSampleSize: (sampleSize: number) => void; }) { const [isPopoverOpen, setIsPopoverOpen] = useState(false); + const [activeSampleSize, setActiveSampleSize] = useState(sampleSize); const togglePopover = useCallback(() => { setIsPopoverOpen((isOpen) => !isOpen); @@ -27,14 +29,14 @@ export function DiscoverGridSettingsPopover({ setIsPopoverOpen(false); }, []); - const onChangeSampleSizeInput = useCallback( + const onChangeActiveSampleSize = useCallback( (event) => { const newSampleSize = Number(event.target.value); if (newSampleSize > 0) { - onChangeSampleSize(newSampleSize); + setActiveSampleSize(newSampleSize); } }, - [onChangeSampleSize] + [setActiveSampleSize] ); const buttonLabel = i18n.translate('discover.grid.settingsPopover.buttonLabel', { @@ -45,6 +47,18 @@ export function DiscoverGridSettingsPopover({ defaultMessage: 'Sample size', }); + useEffect(() => { + setActiveSampleSize(sampleSize); // reset local state + }, [sampleSize, setActiveSampleSize]); + + useDebounce( + () => { + onChangeSampleSize(activeSampleSize); + }, + 300, + [activeSampleSize, onChangeSampleSize] + ); + return ( diff --git a/src/plugins/discover/public/embeddable/saved_search_embeddable.tsx b/src/plugins/discover/public/embeddable/saved_search_embeddable.tsx index 1317916588184..b73f1aeadc0d8 100644 --- a/src/plugins/discover/public/embeddable/saved_search_embeddable.tsx +++ b/src/plugins/discover/public/embeddable/saved_search_embeddable.tsx @@ -78,6 +78,7 @@ export type SearchProps = Partial & onMoveColumn?: (column: string, index: number) => void; onUpdateRowHeight?: (rowHeight?: number) => void; onUpdateRowsPerPage?: (rowsPerPage?: number) => void; + onUpdateSampleSize?: (sampleSize?: number) => void; }; export interface SearchEmbeddableConfig { @@ -109,6 +110,7 @@ export class SavedSearchEmbeddable private prevQuery?: Query; private prevSort?: SortOrder[]; private prevSearchSessionId?: string; + private prevSampleSize?: number; private searchProps?: SearchProps; private node?: HTMLElement; @@ -191,9 +193,10 @@ export class SavedSearchEmbeddable searchSource, this.searchProps!.dataView, this.searchProps!.sort, + this.searchProps!.sampleSizeState, useNewFieldsApi, { - sampleSize: this.services.uiSettings.get(SAMPLE_SIZE_SETTING), + defaultSampleSize: this.services.uiSettings.get(SAMPLE_SIZE_SETTING), defaultSort: this.services.uiSettings.get(SORT_DEFAULT_ORDER_SETTING), } ); @@ -368,7 +371,6 @@ export class SavedSearchEmbeddable }); this.updateInput({ sort: sortOrderArr }); }, - sampleSize: this.services.uiSettings.get(SAMPLE_SIZE_SETTING), onFilter: async (field, value, operator) => { let filters = generateFilters( this.filterManager, @@ -399,6 +401,13 @@ export class SavedSearchEmbeddable onUpdateRowsPerPage: (rowsPerPage) => { this.updateInput({ rowsPerPage }); }, + sampleSizeState: + this.input.sampleSize || + this.savedSearch.sampleSize || + this.services.uiSettings.get(SAMPLE_SIZE_SETTING), + onUpdateSampleSize: (sampleSize) => { + this.updateInput({ sampleSize }); + }, }; const timeRangeSearchSource = searchSource.create(); @@ -441,6 +450,7 @@ export class SavedSearchEmbeddable !isEqual(this.prevQuery, this.input.query) || !isEqual(this.prevTimeRange, this.getTimeRange()) || !isEqual(this.prevSort, this.input.sort) || + this.prevSampleSize !== this.input.sampleSize || this.prevSearchSessionId !== this.input.searchSessionId ); } @@ -451,6 +461,7 @@ export class SavedSearchEmbeddable } return ( this.input.rowsPerPage !== searchProps.rowsPerPageState || + this.input.sampleSize !== searchProps.sampleSizeState || (this.input.columns && !isEqual(this.input.columns, searchProps.columns)) ); } @@ -476,6 +487,7 @@ export class SavedSearchEmbeddable searchProps.searchTitle = this.panelTitle; searchProps.rowHeightState = this.input.rowHeight || this.savedSearch.rowHeight; searchProps.rowsPerPageState = this.input.rowsPerPage || this.savedSearch.rowsPerPage; + searchProps.sampleSizeState = this.input.sampleSize || this.savedSearch.sampleSize; searchProps.filters = this.savedSearch.searchSource.getField('filter') as Filter[]; searchProps.savedSearchId = this.savedSearch.id; if (forceFetch || isFetchRequired) { @@ -492,6 +504,7 @@ export class SavedSearchEmbeddable this.prevTimeRange = this.getTimeRange(); this.prevSearchSessionId = this.input.searchSessionId; this.prevSort = this.input.sort; + this.prevSampleSize = this.input.sampleSize; this.searchProps = searchProps; await this.fetch(); } else if (this.searchProps && this.node) { diff --git a/src/plugins/discover/public/embeddable/types.ts b/src/plugins/discover/public/embeddable/types.ts index 4dd049c8de9a9..04a2302a0086c 100644 --- a/src/plugins/discover/public/embeddable/types.ts +++ b/src/plugins/discover/public/embeddable/types.ts @@ -27,6 +27,7 @@ export interface SearchInput extends EmbeddableInput { sort?: SortOrder[]; rowHeight?: number; rowsPerPage?: number; + sampleSize?: number; } export interface SearchOutput extends EmbeddableOutput { diff --git a/src/plugins/discover/public/embeddable/utils/update_search_source.ts b/src/plugins/discover/public/embeddable/utils/update_search_source.ts index 87ee137aed796..6139add885d89 100644 --- a/src/plugins/discover/public/embeddable/utils/update_search_source.ts +++ b/src/plugins/discover/public/embeddable/utils/update_search_source.ts @@ -14,14 +14,15 @@ export const updateSearchSource = ( searchSource: ISearchSource, dataView: DataView | undefined, sort: (SortOrder[] & string[][]) | undefined, + sampleSize: number | undefined, useNewFieldsApi: boolean, defaults: { - sampleSize: number; + defaultSampleSize: number; defaultSort: string; } ) => { - const { sampleSize, defaultSort } = defaults; - searchSource.setField('size', sampleSize); + const { defaultSampleSize, defaultSort } = defaults; + searchSource.setField('size', sampleSize || defaultSampleSize); searchSource.setField('sort', getSortForSearchSource(sort, dataView, defaultSort)); if (useNewFieldsApi) { searchSource.removeField('fieldsFromSource'); From 5ca785ccece3532d6d2b9bf645218fe6ca5ecc3f Mon Sep 17 00:00:00 2001 From: Julia Rechkunova Date: Thu, 11 May 2023 10:38:15 +0200 Subject: [PATCH 03/32] [Discover] Update test --- .../group2/check_registered_types.test.ts | 2 +- .../main/utils/fetch_documents.test.ts | 1 + .../main/utils/get_state_defaults.test.ts | 2 ++ .../discover_grid/discover_grid.test.tsx | 4 +-- .../utils/update_search_source.test.ts | 30 +++++++++++++++++-- .../saved_searches/get_saved_searches.test.ts | 3 ++ .../saved_searches_utils.test.ts | 2 ++ 7 files changed, 38 insertions(+), 6 deletions(-) diff --git a/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts b/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts index 4f0b65bc35c18..ff5bcad74c372 100644 --- a/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts +++ b/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts @@ -126,7 +126,7 @@ describe('checking migration metadata changes on all registered SO types', () => "query": "cfc049e1f0574fb4fdb2d653d7c10bdc970a2610", "rules-settings": "eb8d40b7d60aeffe66831f7d5d83de28d85776d8", "sample-data-telemetry": "c38daf1a49ed24f2a4fb091e6e1e833fccf19935", - "search": "ed3a9b1681b57d69560909d51933fdf17576ea68", + "search": "0f1ac4e440e3923ad1b82d15e79fd62a23c57265", "search-session": "fae0dfc63274d6a3b90ca583802c48cab8760637", "search-telemetry": "1bbaf2db531b97fa04399440fa52d46e86d54dd8", "security-rule": "151108f4906744a137ddc89f5988310c5b9ba8b6", diff --git a/src/plugins/discover/public/application/main/utils/fetch_documents.test.ts b/src/plugins/discover/public/application/main/utils/fetch_documents.test.ts index 22415aa782194..b3a7513f738e7 100644 --- a/src/plugins/discover/public/application/main/utils/fetch_documents.test.ts +++ b/src/plugins/discover/public/application/main/utils/fetch_documents.test.ts @@ -25,6 +25,7 @@ const getDeps = () => searchSessionId: '123', services: discoverServiceMock, savedSearch: savedSearchMock, + getAppState: () => ({ sampleSize: 100 }), } as unknown as FetchDeps); describe('test fetchDocuments', () => { diff --git a/src/plugins/discover/public/application/main/utils/get_state_defaults.test.ts b/src/plugins/discover/public/application/main/utils/get_state_defaults.test.ts index a994adc340f21..555f80bcaa308 100644 --- a/src/plugins/discover/public/application/main/utils/get_state_defaults.test.ts +++ b/src/plugins/discover/public/application/main/utils/get_state_defaults.test.ts @@ -36,6 +36,7 @@ describe('getStateDefaults', () => { "query": undefined, "rowHeight": undefined, "rowsPerPage": undefined, + "sampleSize": undefined, "savedQuery": undefined, "sort": Array [ Array [ @@ -70,6 +71,7 @@ describe('getStateDefaults', () => { "query": undefined, "rowHeight": undefined, "rowsPerPage": undefined, + "sampleSize": undefined, "savedQuery": undefined, "sort": Array [], "viewMode": undefined, diff --git a/src/plugins/discover/public/components/discover_grid/discover_grid.test.tsx b/src/plugins/discover/public/components/discover_grid/discover_grid.test.tsx index 97e951635288f..1f08e62150da1 100644 --- a/src/plugins/discover/public/components/discover_grid/discover_grid.test.tsx +++ b/src/plugins/discover/public/components/discover_grid/discover_grid.test.tsx @@ -20,7 +20,7 @@ import { buildDataTableRecord } from '../../utils/build_data_record'; import { getDocId } from '../../utils/get_doc_id'; import { EsHitRecord } from '../../types'; -function getProps() { +function getProps(): DiscoverGridProps { const services = discoverServiceMock; services.dataViewFieldEditor.userPermissions.editIndexPattern = jest.fn().mockReturnValue(true); @@ -37,7 +37,7 @@ function getProps() { onSetColumns: jest.fn(), onSort: jest.fn(), rows: esHits.map((hit) => buildDataTableRecord(hit, dataViewMock)), - sampleSize: 30, + sampleSizeState: 30, searchDescription: '', searchTitle: '', setExpandedDoc: jest.fn(), diff --git a/src/plugins/discover/public/embeddable/utils/update_search_source.test.ts b/src/plugins/discover/public/embeddable/utils/update_search_source.test.ts index 779fcfffde778..80a1758d1283e 100644 --- a/src/plugins/discover/public/embeddable/utils/update_search_source.test.ts +++ b/src/plugins/discover/public/embeddable/utils/update_search_source.test.ts @@ -12,22 +12,46 @@ import type { SortOrder } from '@kbn/saved-search-plugin/public'; describe('updateSearchSource', () => { const defaults = { - sampleSize: 50, + defaultSampleSize: 50, defaultSort: 'asc', }; + const customSampleSize = 70; + it('updates a given search source', async () => { const searchSource = createSearchSourceMock({}); - updateSearchSource(searchSource, dataViewMock, [] as SortOrder[], false, defaults); + updateSearchSource( + searchSource, + dataViewMock, + [] as SortOrder[], + customSampleSize, + false, + defaults + ); expect(searchSource.getField('fields')).toBe(undefined); // does not explicitly request fieldsFromSource when not using fields API expect(searchSource.getField('fieldsFromSource')).toBe(undefined); + expect(searchSource.getField('size')).toEqual(customSampleSize); }); it('updates a given search source with the usage of the new fields api', async () => { const searchSource = createSearchSourceMock({}); - updateSearchSource(searchSource, dataViewMock, [] as SortOrder[], true, defaults); + updateSearchSource( + searchSource, + dataViewMock, + [] as SortOrder[], + customSampleSize, + true, + defaults + ); expect(searchSource.getField('fields')).toEqual([{ field: '*', include_unmapped: 'true' }]); expect(searchSource.getField('fieldsFromSource')).toBe(undefined); + expect(searchSource.getField('size')).toEqual(customSampleSize); + }); + + it('updates a given search source with a default sample size', async () => { + const searchSource = createSearchSourceMock({}); + updateSearchSource(searchSource, dataViewMock, [] as SortOrder[], undefined, true, defaults); + expect(searchSource.getField('size')).toEqual(defaults.defaultSampleSize); }); }); diff --git a/src/plugins/saved_search/public/services/saved_searches/get_saved_searches.test.ts b/src/plugins/saved_search/public/services/saved_searches/get_saved_searches.test.ts index f64d0ef659f00..afc0b018fd594 100644 --- a/src/plugins/saved_search/public/services/saved_searches/get_saved_searches.test.ts +++ b/src/plugins/saved_search/public/services/saved_searches/get_saved_searches.test.ts @@ -77,6 +77,7 @@ describe('getSavedSearch', () => { description: 'description', grid: {}, hideChart: false, + sampleSize: 100, }, id: 'ccf1af80-2297-11ec-86e0-1155ffb9c7a7', type: 'search', @@ -113,6 +114,7 @@ describe('getSavedSearch', () => { "refreshInterval": undefined, "rowHeight": undefined, "rowsPerPage": undefined, + "sampleSize": 100, "searchSource": Object { "create": [MockFunction], "createChild": [MockFunction], @@ -212,6 +214,7 @@ describe('getSavedSearch', () => { "refreshInterval": undefined, "rowHeight": undefined, "rowsPerPage": undefined, + "sampleSize": undefined, "searchSource": Object { "create": [MockFunction], "createChild": [MockFunction], diff --git a/src/plugins/saved_search/public/services/saved_searches/saved_searches_utils.test.ts b/src/plugins/saved_search/public/services/saved_searches/saved_searches_utils.test.ts index 511385276ab11..e67dffb0a19bb 100644 --- a/src/plugins/saved_search/public/services/saved_searches/saved_searches_utils.test.ts +++ b/src/plugins/saved_search/public/services/saved_searches/saved_searches_utils.test.ts @@ -56,6 +56,7 @@ describe('saved_searches_utils', () => { "refreshInterval": undefined, "rowHeight": undefined, "rowsPerPage": undefined, + "sampleSize": undefined, "searchSource": SearchSource { "dependencies": Object { "aggs": Object { @@ -146,6 +147,7 @@ describe('saved_searches_utils', () => { "refreshInterval": undefined, "rowHeight": undefined, "rowsPerPage": undefined, + "sampleSize": undefined, "sort": Array [ Array [ "a", From 8d80dbe03ee240a97a5913530fa3b0cb0569a35d Mon Sep 17 00:00:00 2001 From: Julia Rechkunova Date: Thu, 11 May 2023 10:58:17 +0200 Subject: [PATCH 04/32] [Discover] Fix context page --- .../discover/public/application/context/context_app_content.tsx | 2 +- .../components/discover_grid/discover_grid_settings_popover.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugins/discover/public/application/context/context_app_content.tsx b/src/plugins/discover/public/application/context/context_app_content.tsx index 6b65bec06bdcf..f69037cdad6a9 100644 --- a/src/plugins/discover/public/application/context/context_app_content.tsx +++ b/src/plugins/discover/public/application/context/context_app_content.tsx @@ -152,7 +152,7 @@ export function ContextAppContent({ dataView={dataView} expandedDoc={expandedDoc} isLoading={isAnchorLoading} - sampleSize={0} + sampleSizeState={0} sort={sort as SortOrder[]} isSortEnabled={false} showTimeCol={showTimeCol} diff --git a/src/plugins/discover/public/components/discover_grid/discover_grid_settings_popover.tsx b/src/plugins/discover/public/components/discover_grid/discover_grid_settings_popover.tsx index a6889734c41fb..18529b034c6c5 100644 --- a/src/plugins/discover/public/components/discover_grid/discover_grid_settings_popover.tsx +++ b/src/plugins/discover/public/components/discover_grid/discover_grid_settings_popover.tsx @@ -40,7 +40,7 @@ export function DiscoverGridSettingsPopover({ ); const buttonLabel = i18n.translate('discover.grid.settingsPopover.buttonLabel', { - defaultMessage: 'Grid settings', + defaultMessage: 'Settings', }); const sampleSizeLabel = i18n.translate('discover.grid.settingsPopover.sampleSizeLabel', { From 8150dd7b5d6b8f73a9781c670059af37791b59fd Mon Sep 17 00:00:00 2001 From: Julia Rechkunova Date: Thu, 11 May 2023 11:00:15 +0200 Subject: [PATCH 05/32] [Discover] Disable in SQL mode --- .../application/main/components/layout/discover_documents.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 7cb1ce51845c6..93bbaf6dc1835 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 @@ -261,7 +261,7 @@ function DiscoverDocumentsComponent({ rowsPerPageState={rowsPerPage} onUpdateRowsPerPage={onUpdateRowsPerPage} sampleSizeState={sampleSizeState || sampleSize} - onUpdateSampleSize={onUpdateSampleSize} + onUpdateSampleSize={!isPlainRecord ? onUpdateSampleSize : undefined} onFieldEdited={onFieldEdited} savedSearchId={savedSearch.id} DocumentView={DiscoverGridFlyout} From 517cd51f8f3f0c29861192f206c438e2bfe16bde Mon Sep 17 00:00:00 2001 From: Julia Rechkunova Date: Thu, 11 May 2023 13:15:15 +0200 Subject: [PATCH 06/32] [Discover] Fix redundant state updates --- .../discover_grid_settings_popover.tsx | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/plugins/discover/public/components/discover_grid/discover_grid_settings_popover.tsx b/src/plugins/discover/public/components/discover_grid/discover_grid_settings_popover.tsx index 18529b034c6c5..8e504f0431024 100644 --- a/src/plugins/discover/public/components/discover_grid/discover_grid_settings_popover.tsx +++ b/src/plugins/discover/public/components/discover_grid/discover_grid_settings_popover.tsx @@ -6,10 +6,10 @@ * Side Public License, v 1. */ -import React, { useCallback, useEffect, useState } from 'react'; +import React, { useCallback, useEffect, useMemo, useState } from 'react'; import { EuiButtonIcon, EuiFormRow, EuiPopover, EuiRange, EuiToolTip } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; -import useDebounce from 'react-use/lib/useDebounce'; +import { debounce } from 'lodash'; export function DiscoverGridSettingsPopover({ sampleSize, @@ -29,14 +29,20 @@ export function DiscoverGridSettingsPopover({ setIsPopoverOpen(false); }, []); + const debouncedOnChangeSampleSize = useMemo( + () => debounce(onChangeSampleSize, 300, { leading: false, trailing: true }), + [onChangeSampleSize] + ); + const onChangeActiveSampleSize = useCallback( (event) => { const newSampleSize = Number(event.target.value); if (newSampleSize > 0) { setActiveSampleSize(newSampleSize); + debouncedOnChangeSampleSize(newSampleSize); } }, - [setActiveSampleSize] + [setActiveSampleSize, debouncedOnChangeSampleSize] ); const buttonLabel = i18n.translate('discover.grid.settingsPopover.buttonLabel', { @@ -51,14 +57,6 @@ export function DiscoverGridSettingsPopover({ setActiveSampleSize(sampleSize); // reset local state }, [sampleSize, setActiveSampleSize]); - useDebounce( - () => { - onChangeSampleSize(activeSampleSize); - }, - 300, - [activeSampleSize, onChangeSampleSize] - ); - return ( Date: Thu, 11 May 2023 13:21:11 +0200 Subject: [PATCH 07/32] [Discover] Fix the max value --- .../discover_grid/discover_grid_settings_popover.tsx | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/plugins/discover/public/components/discover_grid/discover_grid_settings_popover.tsx b/src/plugins/discover/public/components/discover_grid/discover_grid_settings_popover.tsx index 8e504f0431024..fe5b948b7d3a0 100644 --- a/src/plugins/discover/public/components/discover_grid/discover_grid_settings_popover.tsx +++ b/src/plugins/discover/public/components/discover_grid/discover_grid_settings_popover.tsx @@ -11,6 +11,8 @@ import { EuiButtonIcon, EuiFormRow, EuiPopover, EuiRange, EuiToolTip } from '@el import { i18n } from '@kbn/i18n'; import { debounce } from 'lodash'; +const MAX_ALLOWED_SAMPLE_SIZE = 3000; + export function DiscoverGridSettingsPopover({ sampleSize, onChangeSampleSize, @@ -39,7 +41,9 @@ export function DiscoverGridSettingsPopover({ const newSampleSize = Number(event.target.value); if (newSampleSize > 0) { setActiveSampleSize(newSampleSize); - debouncedOnChangeSampleSize(newSampleSize); + if (newSampleSize <= MAX_ALLOWED_SAMPLE_SIZE) { + debouncedOnChangeSampleSize(newSampleSize); + } } }, [setActiveSampleSize, debouncedOnChangeSampleSize] @@ -84,7 +88,7 @@ export function DiscoverGridSettingsPopover({ fullWidth showInput min={1} - max={3000} + max={MAX_ALLOWED_SAMPLE_SIZE} step={1} value={activeSampleSize} onChange={onChangeActiveSampleSize} From b25b974c1ccb4d393714507089856a97e31069d6 Mon Sep 17 00:00:00 2001 From: Julia Rechkunova Date: Thu, 11 May 2023 13:49:15 +0200 Subject: [PATCH 08/32] [Discover] Discard old value --- .../discover_grid/discover_grid_settings_popover.tsx | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/plugins/discover/public/components/discover_grid/discover_grid_settings_popover.tsx b/src/plugins/discover/public/components/discover_grid/discover_grid_settings_popover.tsx index fe5b948b7d3a0..e3de736312dfe 100644 --- a/src/plugins/discover/public/components/discover_grid/discover_grid_settings_popover.tsx +++ b/src/plugins/discover/public/components/discover_grid/discover_grid_settings_popover.tsx @@ -23,13 +23,14 @@ export function DiscoverGridSettingsPopover({ const [isPopoverOpen, setIsPopoverOpen] = useState(false); const [activeSampleSize, setActiveSampleSize] = useState(sampleSize); - const togglePopover = useCallback(() => { - setIsPopoverOpen((isOpen) => !isOpen); + const openPopover = useCallback(() => { + setIsPopoverOpen(true); }, []); const closePopover = useCallback(() => { setIsPopoverOpen(false); - }, []); + setActiveSampleSize(sampleSize); + }, [sampleSize]); const debouncedOnChangeSampleSize = useMemo( () => debounce(onChangeSampleSize, 300, { leading: false, trailing: true }), @@ -76,7 +77,7 @@ export function DiscoverGridSettingsPopover({ iconType="gear" color="text" data-test-subj="dscGridSettingsPopoverButton" - onClick={togglePopover} + onClick={isPopoverOpen ? closePopover : openPopover} aria-label={buttonLabel} /> From 1fbf4eb2f68c259f07a3ddcdef015a096d9aee61 Mon Sep 17 00:00:00 2001 From: Julia Rechkunova Date: Mon, 15 May 2023 15:39:34 +0200 Subject: [PATCH 09/32] [Discover] Fix the default case --- .../discover/public/embeddable/saved_search_embeddable.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/plugins/discover/public/embeddable/saved_search_embeddable.tsx b/src/plugins/discover/public/embeddable/saved_search_embeddable.tsx index b73f1aeadc0d8..66aafd3168140 100644 --- a/src/plugins/discover/public/embeddable/saved_search_embeddable.tsx +++ b/src/plugins/discover/public/embeddable/saved_search_embeddable.tsx @@ -487,7 +487,10 @@ export class SavedSearchEmbeddable searchProps.searchTitle = this.panelTitle; searchProps.rowHeightState = this.input.rowHeight || this.savedSearch.rowHeight; searchProps.rowsPerPageState = this.input.rowsPerPage || this.savedSearch.rowsPerPage; - searchProps.sampleSizeState = this.input.sampleSize || this.savedSearch.sampleSize; + searchProps.sampleSizeState = + this.input.sampleSize || + this.savedSearch.sampleSize || + this.services.uiSettings.get(SAMPLE_SIZE_SETTING); searchProps.filters = this.savedSearch.searchSource.getField('filter') as Filter[]; searchProps.savedSearchId = this.savedSearch.id; if (forceFetch || isFetchRequired) { From 96572bb3d573076ce429ecacf353231c1ecf3675 Mon Sep 17 00:00:00 2001 From: Julia Rechkunova Date: Mon, 22 May 2023 19:28:24 +0200 Subject: [PATCH 10/32] [Discover] Update UI --- .../discover_grid/discover_grid.tsx | 34 +++++++++++-------- .../discover_grid_settings_popover.tsx | 29 ++++++++++------ 2 files changed, 37 insertions(+), 26 deletions(-) diff --git a/src/plugins/discover/public/components/discover_grid/discover_grid.tsx b/src/plugins/discover/public/components/discover_grid/discover_grid.tsx index 803a4d3848438..61aef1c70ba3b 100644 --- a/src/plugins/discover/public/components/discover_grid/discover_grid.tsx +++ b/src/plugins/discover/public/components/discover_grid/discover_grid.tsx @@ -490,22 +490,26 @@ export const DiscoverGrid = ({ const additionalControls = useMemo( () => ({ left: { - append: usedSelectedDocs.length ? ( - - ) : null, + append: ( + <> + {onUpdateSampleSize ? ( + + ) : null} + {usedSelectedDocs.length ? ( + + ) : null} + + ), }, - right: onUpdateSampleSize ? ( - - ) : null, }), [usedSelectedDocs, isFilterActive, rows, sampleSizeState, setIsFilterActive, onUpdateSampleSize] ); diff --git a/src/plugins/discover/public/components/discover_grid/discover_grid_settings_popover.tsx b/src/plugins/discover/public/components/discover_grid/discover_grid_settings_popover.tsx index e3de736312dfe..104ab19630229 100644 --- a/src/plugins/discover/public/components/discover_grid/discover_grid_settings_popover.tsx +++ b/src/plugins/discover/public/components/discover_grid/discover_grid_settings_popover.tsx @@ -7,7 +7,7 @@ */ import React, { useCallback, useEffect, useMemo, useState } from 'react'; -import { EuiButtonIcon, EuiFormRow, EuiPopover, EuiRange, EuiToolTip } from '@elastic/eui'; +import { EuiButtonEmpty, EuiFormRow, EuiPopover, EuiRange, EuiToolTip } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { debounce } from 'lodash'; @@ -50,13 +50,19 @@ export function DiscoverGridSettingsPopover({ [setActiveSampleSize, debouncedOnChangeSampleSize] ); - const buttonLabel = i18n.translate('discover.grid.settingsPopover.buttonLabel', { - defaultMessage: 'Settings', + const buttonLabel = i18n.translate('discover.grid.sampleSizeSettingsPopover.buttonLabel', { + defaultMessage: '{sampleSize} rows max', + values: { + sampleSize, + }, }); - const sampleSizeLabel = i18n.translate('discover.grid.settingsPopover.sampleSizeLabel', { - defaultMessage: 'Sample size', - }); + const sampleSizeLabel = i18n.translate( + 'discover.grid.sampleSizeSettingsPopover.sampleSizeLabel', + { + defaultMessage: 'Sample size', + } + ); useEffect(() => { setActiveSampleSize(sampleSize); // reset local state @@ -67,19 +73,20 @@ export function DiscoverGridSettingsPopover({ data-test-subj="dscGridSettingsPopover" isOpen={isPopoverOpen} closePopover={closePopover} - anchorPosition="downRight" + anchorPosition="downCenter" panelPaddingSize="s" panelClassName="euiDataGrid__displayPopoverPanel" button={ - + > + {buttonLabel} + } > From 05a72b7785d5a3a9e69e9f2b3f65e64f3f6c84a1 Mon Sep 17 00:00:00 2001 From: Julia Rechkunova Date: Mon, 22 May 2023 20:05:44 +0200 Subject: [PATCH 11/32] [Discover] Update icon --- .../components/discover_grid/discover_grid_settings_popover.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/discover/public/components/discover_grid/discover_grid_settings_popover.tsx b/src/plugins/discover/public/components/discover_grid/discover_grid_settings_popover.tsx index 104ab19630229..b7290cd44036a 100644 --- a/src/plugins/discover/public/components/discover_grid/discover_grid_settings_popover.tsx +++ b/src/plugins/discover/public/components/discover_grid/discover_grid_settings_popover.tsx @@ -80,7 +80,7 @@ export function DiscoverGridSettingsPopover({ Date: Thu, 8 Jun 2023 13:21:27 +0200 Subject: [PATCH 12/32] [Discover] Fix tests --- .../migrations/group2/check_registered_types.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts b/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts index 7184ce0960f63..c0fca8ee321c9 100644 --- a/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts +++ b/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts @@ -129,7 +129,7 @@ describe('checking migration metadata changes on all registered SO types', () => "query": "8db5d48c62d75681d80d82a42b5642f60d068202", "rules-settings": "892a2918ebaeba809a612b8d97cec0b07c800b5f", "sample-data-telemetry": "37441b12f5b0159c2d6d5138a494c9f440e950b5", - "search": "8d5184dd5b986d57250b6ffd9ae48a1925e4c7a3", + "search": "4fb17606575043bd8b87e9ca90b1dd9e5824fc4c", "search-session": "b2fcd840e12a45039ada50b1355faeafa39876d1", "search-telemetry": "b568601618744720b5662946d3103e3fb75fe8ee", "security-rule": "07abb4d7e707d91675ec0495c73816394c7b521f", From d4684432fa77fbbbbbb723d4d4754fe85a012c08 Mon Sep 17 00:00:00 2001 From: Julia Rechkunova Date: Wed, 9 Aug 2023 15:16:42 +0200 Subject: [PATCH 13/32] [Discover] Fix reading from saved search SO --- .../common/service/saved_searches_utils.test.ts | 9 ++++++--- .../server/content_management/saved_search_storage.ts | 1 + 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/plugins/saved_search/common/service/saved_searches_utils.test.ts b/src/plugins/saved_search/common/service/saved_searches_utils.test.ts index f319ad6be4a28..33d684fbe17a0 100644 --- a/src/plugins/saved_search/common/service/saved_searches_utils.test.ts +++ b/src/plugins/saved_search/common/service/saved_searches_utils.test.ts @@ -25,6 +25,9 @@ describe('saved_searches_utils', () => { hideChart: true, isTextBasedQuery: false, usesAdHocDataView: false, + rowsPerPage: 250, + sampleSize: 1000, + breakdownField: 'extension.keyword', }; expect( @@ -38,7 +41,7 @@ describe('saved_searches_utils', () => { ) ).toMatchInlineSnapshot(` Object { - "breakdownField": undefined, + "breakdownField": "extension.keyword", "columns": Array [ "a", "b", @@ -52,8 +55,8 @@ describe('saved_searches_utils', () => { "references": Array [], "refreshInterval": undefined, "rowHeight": undefined, - "rowsPerPage": undefined, - "sampleSize": undefined, + "rowsPerPage": 250, + "sampleSize": 1000, "searchSource": SearchSource { "dependencies": Object { "aggs": Object { diff --git a/src/plugins/saved_search/server/content_management/saved_search_storage.ts b/src/plugins/saved_search/server/content_management/saved_search_storage.ts index 9d13d52db0271..dc5513388a7d9 100644 --- a/src/plugins/saved_search/server/content_management/saved_search_storage.ts +++ b/src/plugins/saved_search/server/content_management/saved_search_storage.ts @@ -36,6 +36,7 @@ export class SavedSearchStorage extends SOContentStorage { 'refreshInterval', 'rowsPerPage', 'breakdownField', + 'sampleSize', ], }); } From 5bbe61e7f3ad1f808cc048dc75c40c9bd67762ee Mon Sep 17 00:00:00 2001 From: Julia Rechkunova Date: Wed, 9 Aug 2023 16:01:15 +0200 Subject: [PATCH 14/32] [Discover] Update search hash --- .../migrations/group2/check_registered_types.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts b/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts index cf09da60183bd..c9b3b60e603af 100644 --- a/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts +++ b/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts @@ -129,7 +129,7 @@ describe('checking migration metadata changes on all registered SO types', () => "risk-engine-configuration": "1b8b175e29ea5311408125c92c6247f502b2d79d", "rules-settings": "892a2918ebaeba809a612b8d97cec0b07c800b5f", "sample-data-telemetry": "37441b12f5b0159c2d6d5138a494c9f440e950b5", - "search": "4fb17606575043bd8b87e9ca90b1dd9e5824fc4c", + "search": "99c3b2e2c3095eff791337fbea590bcc32edc700", "search-session": "b2fcd840e12a45039ada50b1355faeafa39876d1", "search-telemetry": "b568601618744720b5662946d3103e3fb75fe8ee", "security-rule": "07abb4d7e707d91675ec0495c73816394c7b521f", From 44c3da0438920360dda13407e97df1e2944fca35 Mon Sep 17 00:00:00 2001 From: Julia Rechkunova Date: Wed, 9 Aug 2023 16:08:30 +0200 Subject: [PATCH 15/32] [Discover] Add tests --- .../main/utils/cleanup_url_state.test.ts | 25 +++++++++++++++++++ .../embeddable/_saved_search_embeddable.ts | 2 ++ .../discover/group2/_data_grid_pagination.ts | 2 ++ 3 files changed, 29 insertions(+) diff --git a/src/plugins/discover/public/application/main/utils/cleanup_url_state.test.ts b/src/plugins/discover/public/application/main/utils/cleanup_url_state.test.ts index ea1af49f48e89..7ff24e8aa4ff5 100644 --- a/src/plugins/discover/public/application/main/utils/cleanup_url_state.test.ts +++ b/src/plugins/discover/public/application/main/utils/cleanup_url_state.test.ts @@ -80,4 +80,29 @@ describe('cleanupUrlState', () => { } as unknown as AppStateUrl; expect(cleanupUrlState(state)).toMatchInlineSnapshot(`Object {}`); }); + + test('should keep a valid sampleSize', async () => { + const state = { + sampleSize: 50, + } as AppStateUrl; + expect(cleanupUrlState(state)).toMatchInlineSnapshot(` + Object { + "sampleSize": 50, + } + `); + }); + + test('should remove a negative sampleSize', async () => { + const state = { + sampleSize: -50, + } as AppStateUrl; + expect(cleanupUrlState(state)).toMatchInlineSnapshot(`Object {}`); + }); + + test('should remove an invalid sampleSize', async () => { + const state = { + sampleSize: 'test', + } as unknown as AppStateUrl; + expect(cleanupUrlState(state)).toMatchInlineSnapshot(`Object {}`); + }); }); diff --git a/test/functional/apps/discover/embeddable/_saved_search_embeddable.ts b/test/functional/apps/discover/embeddable/_saved_search_embeddable.ts index 8961af57a4ad2..40f1ff96f75ed 100644 --- a/test/functional/apps/discover/embeddable/_saved_search_embeddable.ts +++ b/test/functional/apps/discover/embeddable/_saved_search_embeddable.ts @@ -86,6 +86,8 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await dataGrid.checkCurrentRowsPerPageToBe(10); }); + // TODO: add a test for sampleSize setting + it('should control columns correctly', async () => { await addSearchEmbeddableToDashboard(); await PageObjects.dashboard.switchToEditMode(); diff --git a/test/functional/apps/discover/group2/_data_grid_pagination.ts b/test/functional/apps/discover/group2/_data_grid_pagination.ts index 7da02308c73be..2362229cf473a 100644 --- a/test/functional/apps/discover/group2/_data_grid_pagination.ts +++ b/test/functional/apps/discover/group2/_data_grid_pagination.ts @@ -116,5 +116,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect((await dataGrid.getDocTableRows()).length).to.be(10); // as in the saved search await dataGrid.checkCurrentRowsPerPageToBe(10); }); + + // TODO: add a test for sampleSize setting }); } From 19e60fff6e71203824b0db09aac9bfb550e982c9 Mon Sep 17 00:00:00 2001 From: Julia Rechkunova Date: Wed, 9 Aug 2023 17:21:52 +0200 Subject: [PATCH 16/32] [Discover] Fix tests --- .../saved_searches/saved_search_attribute_service.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/plugins/saved_search/public/services/saved_searches/saved_search_attribute_service.test.ts b/src/plugins/saved_search/public/services/saved_searches/saved_search_attribute_service.test.ts index cc6a6ec79ffea..35c35e669bff8 100644 --- a/src/plugins/saved_search/public/services/saved_searches/saved_search_attribute_service.test.ts +++ b/src/plugins/saved_search/public/services/saved_searches/saved_search_attribute_service.test.ts @@ -200,6 +200,7 @@ describe('getSavedSearchAttributeService', () => { "refreshInterval": undefined, "rowHeight": undefined, "rowsPerPage": undefined, + "sampleSize": undefined, "searchSource": Object { "create": [MockFunction], "createChild": [MockFunction], From 8f2679ed83a150a361f134e8136f28082c63eb26 Mon Sep 17 00:00:00 2001 From: Julia Rechkunova Date: Thu, 28 Sep 2023 16:28:25 +0200 Subject: [PATCH 17/32] [Discover] Use new Eui props --- .../src/components/data_table.tsx | 39 ++++--- ...data_table_additional_display_settings.tsx | 67 +++++++++++ .../discover_grid_settings_popover.tsx | 108 ------------------ 3 files changed, 89 insertions(+), 125 deletions(-) create mode 100644 packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.tsx delete mode 100644 packages/kbn-unified-data-table/src/components/discover_grid_settings_popover.tsx diff --git a/packages/kbn-unified-data-table/src/components/data_table.tsx b/packages/kbn-unified-data-table/src/components/data_table.tsx index cabb6694a7af8..1a8108112b255 100644 --- a/packages/kbn-unified-data-table/src/components/data_table.tsx +++ b/packages/kbn-unified-data-table/src/components/data_table.tsx @@ -27,6 +27,7 @@ import { EuiDataGridControlColumn, EuiDataGridCustomBodyProps, EuiDataGridCellValueElementProps, + EuiDataGridToolBarVisibilityDisplaySelectorOptions, EuiDataGridStyle, } from '@elastic/eui'; import type { DataView } from '@kbn/data-views-plugin/public'; @@ -59,7 +60,7 @@ import { toolbarVisibility as toolbarVisibilityDefaults, } from '../constants'; import { UnifiedDataTableFooter } from './data_table_footer'; -import { DiscoverGridSettingsPopover } from './discover_grid_settings_popover'; +import { UnifiedDataTableAdditionalDisplaySettings } from './data_table_additional_display_settings'; export type SortOrder = [string, string]; @@ -675,12 +676,6 @@ export const UnifiedDataTable = ({ const additionalControls = useMemo( () => ( <> - {onUpdateSampleSize ? ( - - ) : null} {usedSelectedDocs.length ? ( - !!onUpdateRowHeight - ? { - allowDensity: false, - allowRowHeight: true, - } - : undefined, - [onUpdateRowHeight] - ); + const showDisplaySelector = useMemo(() => { + const options: EuiDataGridToolBarVisibilityDisplaySelectorOptions = {}; + + if (onUpdateRowHeight) { + options.allowDensity = false; + options.allowRowHeight = true; + } + + if (onUpdateSampleSize) { + options.allowResetButton = false; + options.additionalDisplaySettings = ( + + ); + } + + return Object.keys(options).length ? options : undefined; + }, [sampleSizeState, onUpdateRowHeight, onUpdateSampleSize]); const inMemory = useMemo(() => { return isPlainRecord && columns.length diff --git a/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.tsx b/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.tsx new file mode 100644 index 0000000000000..2cc1ec7860ecc --- /dev/null +++ b/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.tsx @@ -0,0 +1,67 @@ +/* + * 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 React, { useCallback, useEffect, useMemo, useState } from 'react'; +import { EuiFormRow, EuiRange } from '@elastic/eui'; +import { i18n } from '@kbn/i18n'; +import { debounce } from 'lodash'; + +const MAX_ALLOWED_SAMPLE_SIZE = 10000; + +export interface UnifiedDataTableAdditionalDisplaySettingsProps { + sampleSize: number; + onChangeSampleSize: (sampleSize: number) => void; +} + +export const UnifiedDataTableAdditionalDisplaySettings: React.FC< + UnifiedDataTableAdditionalDisplaySettingsProps +> = ({ sampleSize, onChangeSampleSize }) => { + const [activeSampleSize, setActiveSampleSize] = useState(sampleSize); + + const debouncedOnChangeSampleSize = useMemo( + () => debounce(onChangeSampleSize, 300, { leading: false, trailing: true }), + [onChangeSampleSize] + ); + + const onChangeActiveSampleSize = useCallback( + (event) => { + const newSampleSize = Number(event.target.value); + if (newSampleSize > 0) { + setActiveSampleSize(newSampleSize); + if (newSampleSize <= MAX_ALLOWED_SAMPLE_SIZE) { + debouncedOnChangeSampleSize(newSampleSize); + } + } + }, + [setActiveSampleSize, debouncedOnChangeSampleSize] + ); + + const sampleSizeLabel = i18n.translate('unifiedDataTable.sampleSizeSettings.sampleSizeLabel', { + defaultMessage: 'Sample size', + }); + + useEffect(() => { + setActiveSampleSize(sampleSize); // reset local state + }, [sampleSize, setActiveSampleSize]); + + return ( + + + + ); +}; diff --git a/packages/kbn-unified-data-table/src/components/discover_grid_settings_popover.tsx b/packages/kbn-unified-data-table/src/components/discover_grid_settings_popover.tsx deleted file mode 100644 index b7290cd44036a..0000000000000 --- a/packages/kbn-unified-data-table/src/components/discover_grid_settings_popover.tsx +++ /dev/null @@ -1,108 +0,0 @@ -/* - * 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 React, { useCallback, useEffect, useMemo, useState } from 'react'; -import { EuiButtonEmpty, EuiFormRow, EuiPopover, EuiRange, EuiToolTip } from '@elastic/eui'; -import { i18n } from '@kbn/i18n'; -import { debounce } from 'lodash'; - -const MAX_ALLOWED_SAMPLE_SIZE = 3000; - -export function DiscoverGridSettingsPopover({ - sampleSize, - onChangeSampleSize, -}: { - sampleSize: number; - onChangeSampleSize: (sampleSize: number) => void; -}) { - const [isPopoverOpen, setIsPopoverOpen] = useState(false); - const [activeSampleSize, setActiveSampleSize] = useState(sampleSize); - - const openPopover = useCallback(() => { - setIsPopoverOpen(true); - }, []); - - const closePopover = useCallback(() => { - setIsPopoverOpen(false); - setActiveSampleSize(sampleSize); - }, [sampleSize]); - - const debouncedOnChangeSampleSize = useMemo( - () => debounce(onChangeSampleSize, 300, { leading: false, trailing: true }), - [onChangeSampleSize] - ); - - const onChangeActiveSampleSize = useCallback( - (event) => { - const newSampleSize = Number(event.target.value); - if (newSampleSize > 0) { - setActiveSampleSize(newSampleSize); - if (newSampleSize <= MAX_ALLOWED_SAMPLE_SIZE) { - debouncedOnChangeSampleSize(newSampleSize); - } - } - }, - [setActiveSampleSize, debouncedOnChangeSampleSize] - ); - - const buttonLabel = i18n.translate('discover.grid.sampleSizeSettingsPopover.buttonLabel', { - defaultMessage: '{sampleSize} rows max', - values: { - sampleSize, - }, - }); - - const sampleSizeLabel = i18n.translate( - 'discover.grid.sampleSizeSettingsPopover.sampleSizeLabel', - { - defaultMessage: 'Sample size', - } - ); - - useEffect(() => { - setActiveSampleSize(sampleSize); // reset local state - }, [sampleSize, setActiveSampleSize]); - - return ( - - - {buttonLabel} - - - } - > - - - - - ); -} From 8b6cee9edb65b033c68675fd94fb47c0250fb549 Mon Sep 17 00:00:00 2001 From: Julia Rechkunova Date: Thu, 28 Sep 2023 17:07:22 +0200 Subject: [PATCH 18/32] [Discover] Update version --- src/plugins/saved_search/server/saved_objects/schema.ts | 2 +- src/plugins/saved_search/server/saved_objects/search.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/plugins/saved_search/server/saved_objects/schema.ts b/src/plugins/saved_search/server/saved_objects/schema.ts index 1a94bb79a9e3a..f144166f23ffc 100644 --- a/src/plugins/saved_search/server/saved_objects/schema.ts +++ b/src/plugins/saved_search/server/saved_objects/schema.ts @@ -80,7 +80,7 @@ const SCHEMA_SEARCH_BASE = { }; export const SCHEMA_SEARCH_V8_8_0 = schema.object(SCHEMA_SEARCH_BASE); -export const SCHEMA_SEARCH_V8_10_0 = schema.object({ +export const SCHEMA_SEARCH_V8_11_0 = schema.object({ ...SCHEMA_SEARCH_BASE, sampleSize: schema.maybe(schema.number()), }); diff --git a/src/plugins/saved_search/server/saved_objects/search.ts b/src/plugins/saved_search/server/saved_objects/search.ts index 74baedcee2452..cf51ec8b56476 100644 --- a/src/plugins/saved_search/server/saved_objects/search.ts +++ b/src/plugins/saved_search/server/saved_objects/search.ts @@ -10,7 +10,7 @@ import { ANALYTICS_SAVED_OBJECT_INDEX } from '@kbn/core-saved-objects-server'; import { SavedObjectsType } from '@kbn/core/server'; import { MigrateFunctionsObject } from '@kbn/kibana-utils-plugin/common'; import { getAllMigrations } from './search_migrations'; -import { SCHEMA_SEARCH_V8_8_0, SCHEMA_SEARCH_V8_10_0 } from './schema'; +import { SCHEMA_SEARCH_V8_8_0, SCHEMA_SEARCH_V8_11_0 } from './schema'; export function getSavedSearchObjectType( getSearchSourceMigrations: () => MigrateFunctionsObject @@ -44,7 +44,7 @@ export function getSavedSearchObjectType( }, schemas: { '8.8.0': SCHEMA_SEARCH_V8_8_0, - '8.10.0': SCHEMA_SEARCH_V8_10_0, + '8.11.0': SCHEMA_SEARCH_V8_11_0, }, migrations: () => getAllMigrations(getSearchSourceMigrations()), }; From 3b1650b78c6d83bb3e349cc32ba8ccc64eac8f96 Mon Sep 17 00:00:00 2001 From: Julia Rechkunova Date: Mon, 2 Oct 2023 18:19:48 +0200 Subject: [PATCH 19/32] [Discover] Update tests --- .../utils/update_search_source.test.ts | 20 +++++++++++++------ .../discover/group2/_data_grid_row_height.ts | 14 ++++++++----- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/src/plugins/discover/public/embeddable/utils/update_search_source.test.ts b/src/plugins/discover/public/embeddable/utils/update_search_source.test.ts index 35e94d73069d8..4ea8f3e27632c 100644 --- a/src/plugins/discover/public/embeddable/utils/update_search_source.test.ts +++ b/src/plugins/discover/public/embeddable/utils/update_search_source.test.ts @@ -62,19 +62,26 @@ describe('updateSearchSource', () => { it('updates a given search source with a default sample size', async () => { const searchSource = createSearchSourceMock({}); updateSearchSource(searchSource, dataViewMock, [] as SortOrder[], undefined, true, defaults); - expect(searchSource.getField('size')).toEqual(defaults.defaultSampleSize); + expect(searchSource.getField('size')).toEqual(defaults.sampleSize); }); it('updates a given search source with sort field', async () => { const searchSource1 = createSearchSourceMock({}); - updateSearchSource(searchSource1, dataViewMock, [] as SortOrder[], true, defaults); + updateSearchSource(searchSource1, dataViewMock, [] as SortOrder[], undefined, true, defaults); expect(searchSource1.getField('sort')).toEqual([{ _score: 'asc' }]); const searchSource2 = createSearchSourceMock({}); - updateSearchSource(searchSource2, dataViewMockWithTimeField, [] as SortOrder[], true, { - sampleSize: 50, - sortDir: 'desc', - }); + updateSearchSource( + searchSource2, + dataViewMockWithTimeField, + [] as SortOrder[], + undefined, + true, + { + sampleSize: 50, + sortDir: 'desc', + } + ); expect(searchSource2.getField('sort')).toEqual([{ _doc: 'desc' }]); const searchSource3 = createSearchSourceMock({}); @@ -82,6 +89,7 @@ describe('updateSearchSource', () => { searchSource3, dataViewMockWithTimeField, [['bytes', 'desc']] as SortOrder[], + undefined, true, defaults ); diff --git a/test/functional/apps/discover/group2/_data_grid_row_height.ts b/test/functional/apps/discover/group2/_data_grid_row_height.ts index 2c385b67aaa02..84574655cb406 100644 --- a/test/functional/apps/discover/group2/_data_grid_row_height.ts +++ b/test/functional/apps/discover/group2/_data_grid_row_height.ts @@ -14,6 +14,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { const esArchiver = getService('esArchiver'); const kibanaServer = getService('kibanaServer'); const dataGrid = getService('dataGrid'); + const testSubjects = getService('testSubjects'); const PageObjects = getPageObjects(['settings', 'common', 'discover', 'header', 'timePicker']); const defaultSettings = { defaultIndex: 'logstash-*' }; const security = getService('security'); @@ -47,7 +48,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(await dataGrid.getCurrentRowHeightValue()).to.be('Auto fit'); }); - it('should allow to change row height and reset it', async () => { + it('should allow to change row height', async () => { await dataGrid.clickGridSettings(); expect(await dataGrid.getCurrentRowHeightValue()).to.be('Auto fit'); @@ -59,13 +60,16 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(await dataGrid.getCurrentRowHeightValue()).to.be('Single'); - await dataGrid.resetRowHeightValue(); - - expect(await dataGrid.getCurrentRowHeightValue()).to.be('Auto fit'); + // we hide "Reset to default" action in Discover + await testSubjects.missingOrFail('resetDisplaySelector'); await dataGrid.changeRowHeightValue('Custom'); - await dataGrid.resetRowHeightValue(); + expect(await dataGrid.getCurrentRowHeightValue()).to.be('Custom'); + + await testSubjects.missingOrFail('resetDisplaySelector'); + + await dataGrid.changeRowHeightValue('Auto fit'); expect(await dataGrid.getCurrentRowHeightValue()).to.be('Auto fit'); }); From e2a9d30ce1487fb1a301ab9fbcc6077ee50cfe49 Mon Sep 17 00:00:00 2001 From: Julia Rechkunova Date: Thu, 5 Oct 2023 14:26:15 +0200 Subject: [PATCH 20/32] [Discover] Update scheme version --- .../migrations/group2/check_registered_types.test.ts | 2 +- src/plugins/saved_search/server/saved_objects/schema.ts | 2 +- src/plugins/saved_search/server/saved_objects/search.ts | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts b/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts index 0639d2e7c4cd7..aca3ec8bdfade 100644 --- a/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts +++ b/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts @@ -133,7 +133,7 @@ describe('checking migration metadata changes on all registered SO types', () => "risk-engine-configuration": "b105d4a3c6adce40708d729d12e5ef3c8fbd9508", "rules-settings": "892a2918ebaeba809a612b8d97cec0b07c800b5f", "sample-data-telemetry": "37441b12f5b0159c2d6d5138a494c9f440e950b5", - "search": "99c3b2e2c3095eff791337fbea590bcc32edc700", + "search": "510a911d2f53d8a14b0785c192fcbee04023547e", "search-session": "b2fcd840e12a45039ada50b1355faeafa39876d1", "search-telemetry": "b568601618744720b5662946d3103e3fb75fe8ee", "security-rule": "07abb4d7e707d91675ec0495c73816394c7b521f", diff --git a/src/plugins/saved_search/server/saved_objects/schema.ts b/src/plugins/saved_search/server/saved_objects/schema.ts index f144166f23ffc..606fb643d3f30 100644 --- a/src/plugins/saved_search/server/saved_objects/schema.ts +++ b/src/plugins/saved_search/server/saved_objects/schema.ts @@ -80,7 +80,7 @@ const SCHEMA_SEARCH_BASE = { }; export const SCHEMA_SEARCH_V8_8_0 = schema.object(SCHEMA_SEARCH_BASE); -export const SCHEMA_SEARCH_V8_11_0 = schema.object({ +export const SCHEMA_SEARCH_V8_12_0 = schema.object({ ...SCHEMA_SEARCH_BASE, sampleSize: schema.maybe(schema.number()), }); diff --git a/src/plugins/saved_search/server/saved_objects/search.ts b/src/plugins/saved_search/server/saved_objects/search.ts index cf51ec8b56476..2d3844f098c6a 100644 --- a/src/plugins/saved_search/server/saved_objects/search.ts +++ b/src/plugins/saved_search/server/saved_objects/search.ts @@ -10,7 +10,7 @@ import { ANALYTICS_SAVED_OBJECT_INDEX } from '@kbn/core-saved-objects-server'; import { SavedObjectsType } from '@kbn/core/server'; import { MigrateFunctionsObject } from '@kbn/kibana-utils-plugin/common'; import { getAllMigrations } from './search_migrations'; -import { SCHEMA_SEARCH_V8_8_0, SCHEMA_SEARCH_V8_11_0 } from './schema'; +import { SCHEMA_SEARCH_V8_8_0, SCHEMA_SEARCH_V8_12_0 } from './schema'; export function getSavedSearchObjectType( getSearchSourceMigrations: () => MigrateFunctionsObject @@ -44,7 +44,7 @@ export function getSavedSearchObjectType( }, schemas: { '8.8.0': SCHEMA_SEARCH_V8_8_0, - '8.11.0': SCHEMA_SEARCH_V8_11_0, + '8.12.0': SCHEMA_SEARCH_V8_12_0, }, migrations: () => getAllMigrations(getSearchSourceMigrations()), }; From 4f55005769812af5e9f009a745594a2f6571541a Mon Sep 17 00:00:00 2001 From: Julia Rechkunova Date: Thu, 5 Oct 2023 14:56:46 +0200 Subject: [PATCH 21/32] [Discover] Update UI --- .../components/data_table_additional_display_settings.tsx | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.tsx b/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.tsx index 2cc1ec7860ecc..31205e17aaf79 100644 --- a/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.tsx +++ b/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.tsx @@ -7,11 +7,11 @@ */ import React, { useCallback, useEffect, useMemo, useState } from 'react'; -import { EuiFormRow, EuiRange } from '@elastic/eui'; +import { EuiFormRow, EuiFieldNumber } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { debounce } from 'lodash'; -const MAX_ALLOWED_SAMPLE_SIZE = 10000; +const MAX_ALLOWED_SAMPLE_SIZE = 1000; export interface UnifiedDataTableAdditionalDisplaySettingsProps { sampleSize: number; @@ -51,10 +51,9 @@ export const UnifiedDataTableAdditionalDisplaySettings: React.FC< return ( - Date: Thu, 5 Oct 2023 14:58:43 +0200 Subject: [PATCH 22/32] [Discover] Fix props --- .../cloud_security_data_table/cloud_security_data_table.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/cloud_security_posture/public/components/cloud_security_data_table/cloud_security_data_table.tsx b/x-pack/plugins/cloud_security_posture/public/components/cloud_security_data_table/cloud_security_data_table.tsx index 2318a16f2efbb..b5a3efca64c5a 100644 --- a/x-pack/plugins/cloud_security_posture/public/components/cloud_security_data_table/cloud_security_data_table.tsx +++ b/x-pack/plugins/cloud_security_posture/public/components/cloud_security_data_table/cloud_security_data_table.tsx @@ -236,7 +236,7 @@ export const CloudSecurityDataTable = ({ onSetColumns={onSetColumns} onSort={onSort} rows={rows} - sampleSize={MAX_FINDINGS_TO_LOAD} + sampleSizeState={MAX_FINDINGS_TO_LOAD} setExpandedDoc={setExpandedDoc} renderDocumentView={renderDocumentView} sort={sort} From 7a312087365ab51f2c1b3aa664293a86557216f9 Mon Sep 17 00:00:00 2001 From: Julia Rechkunova Date: Thu, 5 Oct 2023 16:09:27 +0200 Subject: [PATCH 23/32] [Discover] Add jest tests --- .../src/components/data_table.test.tsx | 69 +++++++++++++++++ ...table_additional_display_settings.test.tsx | 74 +++++++++++++++++++ 2 files changed, 143 insertions(+) create mode 100644 packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.test.tsx diff --git a/packages/kbn-unified-data-table/src/components/data_table.test.tsx b/packages/kbn-unified-data-table/src/components/data_table.test.tsx index a5f4e0919bf50..97bcc0e2c6654 100644 --- a/packages/kbn-unified-data-table/src/components/data_table.test.tsx +++ b/packages/kbn-unified-data-table/src/components/data_table.test.tsx @@ -10,6 +10,7 @@ import { ReactWrapper } from 'enzyme'; import { EuiButton, EuiCopy, + EuiDataGrid, EuiDataGridCellValueElementProps, EuiDataGridCustomBodyProps, } from '@elastic/eui'; @@ -301,6 +302,74 @@ describe('UnifiedDataTable', () => { }); }); + describe('display settings', () => { + it('should include additional display settings if onUpdateSampleSize is provided', async () => { + const component = await getComponent({ + ...getProps(), + sampleSizeState: 150, + onUpdateSampleSize: jest.fn(), + onUpdateRowHeight: jest.fn(), + }); + + expect(component.find(EuiDataGrid).prop('toolbarVisibility')).toMatchInlineSnapshot(` + Object { + "additionalControls": , + "showColumnSelector": false, + "showDisplaySelector": Object { + "additionalDisplaySettings": , + "allowDensity": false, + "allowResetButton": false, + "allowRowHeight": true, + }, + "showFullScreenSelector": true, + "showSortSelector": true, + } + `); + }); + + it('should not include additional display settings if onUpdateSampleSize is not provided', async () => { + const component = await getComponent({ + ...getProps(), + sampleSizeState: 200, + onUpdateRowHeight: jest.fn(), + }); + + expect(component.find(EuiDataGrid).prop('toolbarVisibility')).toMatchInlineSnapshot(` + Object { + "additionalControls": , + "showColumnSelector": false, + "showDisplaySelector": Object { + "allowDensity": false, + "allowRowHeight": true, + }, + "showFullScreenSelector": true, + "showSortSelector": true, + } + `); + }); + + it('should hide display settings if no handlers provided', async () => { + const component = await getComponent({ + ...getProps(), + onUpdateRowHeight: undefined, + onUpdateSampleSize: undefined, + }); + + expect(component.find(EuiDataGrid).prop('toolbarVisibility')).toMatchInlineSnapshot(` + Object { + "additionalControls": , + "showColumnSelector": false, + "showDisplaySelector": undefined, + "showFullScreenSelector": true, + "showSortSelector": true, + } + `); + }); + }); + describe('externalControlColumns', () => { it('should render external leading control columns', async () => { const component = await getComponent({ diff --git a/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.test.tsx b/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.test.tsx new file mode 100644 index 0000000000000..8828dd5585c80 --- /dev/null +++ b/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.test.tsx @@ -0,0 +1,74 @@ +/* + * 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 React from 'react'; +import { mountWithIntl } from '@kbn/test-jest-helpers'; +import { act } from 'react-dom/test-utils'; +import { findTestSubject } from '@elastic/eui/lib/test'; +import { UnifiedDataTableAdditionalDisplaySettings } from './data_table_additional_display_settings'; +import lodash from 'lodash'; + +jest.spyOn(lodash, 'debounce').mockImplementation((fn: any) => fn); + +describe('UnifiedDataTableAdditionalDisplaySettings', function () { + describe('sampleSize', function () { + it('should work correctly', async () => { + const onChangeSampleSizeMock = jest.fn(); + + const component = mountWithIntl( + + ); + const input = findTestSubject(component, 'unifiedDataTableSampleSizeRange'); + expect(input.exists()).toBe(true); + expect(input.prop('value')).toBe(10); + + await act(async () => { + input.simulate('change', { + target: { + value: 100, + }, + }); + }); + + expect(onChangeSampleSizeMock).toHaveBeenCalledWith(100); + + await new Promise((resolve) => setTimeout(resolve, 0)); + component.update(); + + expect(findTestSubject(component, 'unifiedDataTableSampleSizeRange').prop('value')).toBe(100); + }); + + it('should render value changes correctly', async () => { + const onChangeSampleSizeMock = jest.fn(); + + const component = mountWithIntl( + + ); + + expect(findTestSubject(component, 'unifiedDataTableSampleSizeRange').prop('value')).toBe(200); + + component.setProps({ + sampleSize: 500, + onChangeSampleSize: onChangeSampleSizeMock, + }); + + await new Promise((resolve) => setTimeout(resolve, 0)); + component.update(); + + expect(findTestSubject(component, 'unifiedDataTableSampleSizeRange').prop('value')).toBe(500); + + expect(onChangeSampleSizeMock).not.toHaveBeenCalled(); + }); + }); +}); From 60baf9074b5b01487b6346d063487cb4749d24c5 Mon Sep 17 00:00:00 2001 From: Julia Rechkunova Date: Fri, 6 Oct 2023 11:18:21 +0200 Subject: [PATCH 24/32] [Discover] Update search SO hash --- .../migrations/group2/check_registered_types.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts b/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts index aca3ec8bdfade..01351ac9ea237 100644 --- a/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts +++ b/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts @@ -133,7 +133,7 @@ describe('checking migration metadata changes on all registered SO types', () => "risk-engine-configuration": "b105d4a3c6adce40708d729d12e5ef3c8fbd9508", "rules-settings": "892a2918ebaeba809a612b8d97cec0b07c800b5f", "sample-data-telemetry": "37441b12f5b0159c2d6d5138a494c9f440e950b5", - "search": "510a911d2f53d8a14b0785c192fcbee04023547e", + "search": "2c1ab8a17e6972be2fa8d3880ba2305dfd9a5a6e", "search-session": "b2fcd840e12a45039ada50b1355faeafa39876d1", "search-telemetry": "b568601618744720b5662946d3103e3fb75fe8ee", "security-rule": "07abb4d7e707d91675ec0495c73816394c7b521f", From 7194e6b5a03debd3f4014ef22eac20c395b4a0b0 Mon Sep 17 00:00:00 2001 From: Julia Rechkunova Date: Fri, 6 Oct 2023 11:24:26 +0200 Subject: [PATCH 25/32] [Discover] Extend tests suite --- ...table_additional_display_settings.test.tsx | 36 ++++++++++++++++++- ...data_table_additional_display_settings.tsx | 2 +- 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.test.tsx b/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.test.tsx index 8828dd5585c80..6046bbc186604 100644 --- a/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.test.tsx +++ b/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.test.tsx @@ -10,7 +10,10 @@ import React from 'react'; import { mountWithIntl } from '@kbn/test-jest-helpers'; import { act } from 'react-dom/test-utils'; import { findTestSubject } from '@elastic/eui/lib/test'; -import { UnifiedDataTableAdditionalDisplaySettings } from './data_table_additional_display_settings'; +import { + UnifiedDataTableAdditionalDisplaySettings, + MAX_ALLOWED_SAMPLE_SIZE, +} from './data_table_additional_display_settings'; import lodash from 'lodash'; jest.spyOn(lodash, 'debounce').mockImplementation((fn: any) => fn); @@ -46,6 +49,37 @@ describe('UnifiedDataTableAdditionalDisplaySettings', function () { expect(findTestSubject(component, 'unifiedDataTableSampleSizeRange').prop('value')).toBe(100); }); + it('should not execute the callback for an invalid input', async () => { + const invalidValue = MAX_ALLOWED_SAMPLE_SIZE + 1; + const onChangeSampleSizeMock = jest.fn(); + + const component = mountWithIntl( + + ); + const input = findTestSubject(component, 'unifiedDataTableSampleSizeRange'); + expect(input.prop('value')).toBe(5); + + await act(async () => { + input.simulate('change', { + target: { + value: invalidValue, + }, + }); + }); + + await new Promise((resolve) => setTimeout(resolve, 0)); + component.update(); + + expect(findTestSubject(component, 'unifiedDataTableSampleSizeRange').prop('value')).toBe( + invalidValue + ); + + expect(onChangeSampleSizeMock).not.toHaveBeenCalled(); + }); + it('should render value changes correctly', async () => { const onChangeSampleSizeMock = jest.fn(); diff --git a/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.tsx b/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.tsx index 31205e17aaf79..91c7de6c68f5e 100644 --- a/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.tsx +++ b/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.tsx @@ -11,7 +11,7 @@ import { EuiFormRow, EuiFieldNumber } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { debounce } from 'lodash'; -const MAX_ALLOWED_SAMPLE_SIZE = 1000; +export const MAX_ALLOWED_SAMPLE_SIZE = 1000; export interface UnifiedDataTableAdditionalDisplaySettingsProps { sampleSize: number; From 79712df9d64ac14b4807dee262c602e5d3d5ce81 Mon Sep 17 00:00:00 2001 From: Julia Rechkunova Date: Fri, 6 Oct 2023 12:50:30 +0200 Subject: [PATCH 26/32] [Discover] Add functional tests --- ...table_additional_display_settings.test.tsx | 12 +- ...data_table_additional_display_settings.tsx | 2 +- .../embeddable/_saved_search_embeddable.ts | 2 - .../discover/group2/_data_grid_pagination.ts | 2 - .../discover/group2/_data_grid_sample_size.ts | 195 ++++++++++++++++++ test/functional/apps/discover/group2/index.ts | 1 + test/functional/services/data_grid.ts | 16 ++ 7 files changed, 219 insertions(+), 11 deletions(-) create mode 100644 test/functional/apps/discover/group2/_data_grid_sample_size.ts diff --git a/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.test.tsx b/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.test.tsx index 6046bbc186604..7cecfdfcb7676 100644 --- a/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.test.tsx +++ b/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.test.tsx @@ -29,7 +29,7 @@ describe('UnifiedDataTableAdditionalDisplaySettings', function () { onChangeSampleSize={onChangeSampleSizeMock} /> ); - const input = findTestSubject(component, 'unifiedDataTableSampleSizeRange'); + const input = findTestSubject(component, 'unifiedDataTableSampleSizeInput'); expect(input.exists()).toBe(true); expect(input.prop('value')).toBe(10); @@ -46,7 +46,7 @@ describe('UnifiedDataTableAdditionalDisplaySettings', function () { await new Promise((resolve) => setTimeout(resolve, 0)); component.update(); - expect(findTestSubject(component, 'unifiedDataTableSampleSizeRange').prop('value')).toBe(100); + expect(findTestSubject(component, 'unifiedDataTableSampleSizeInput').prop('value')).toBe(100); }); it('should not execute the callback for an invalid input', async () => { @@ -59,7 +59,7 @@ describe('UnifiedDataTableAdditionalDisplaySettings', function () { onChangeSampleSize={onChangeSampleSizeMock} /> ); - const input = findTestSubject(component, 'unifiedDataTableSampleSizeRange'); + const input = findTestSubject(component, 'unifiedDataTableSampleSizeInput'); expect(input.prop('value')).toBe(5); await act(async () => { @@ -73,7 +73,7 @@ describe('UnifiedDataTableAdditionalDisplaySettings', function () { await new Promise((resolve) => setTimeout(resolve, 0)); component.update(); - expect(findTestSubject(component, 'unifiedDataTableSampleSizeRange').prop('value')).toBe( + expect(findTestSubject(component, 'unifiedDataTableSampleSizeInput').prop('value')).toBe( invalidValue ); @@ -90,7 +90,7 @@ describe('UnifiedDataTableAdditionalDisplaySettings', function () { /> ); - expect(findTestSubject(component, 'unifiedDataTableSampleSizeRange').prop('value')).toBe(200); + expect(findTestSubject(component, 'unifiedDataTableSampleSizeInput').prop('value')).toBe(200); component.setProps({ sampleSize: 500, @@ -100,7 +100,7 @@ describe('UnifiedDataTableAdditionalDisplaySettings', function () { await new Promise((resolve) => setTimeout(resolve, 0)); component.update(); - expect(findTestSubject(component, 'unifiedDataTableSampleSizeRange').prop('value')).toBe(500); + expect(findTestSubject(component, 'unifiedDataTableSampleSizeInput').prop('value')).toBe(500); expect(onChangeSampleSizeMock).not.toHaveBeenCalled(); }); diff --git a/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.tsx b/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.tsx index 91c7de6c68f5e..3cd9f2d7f8a4a 100644 --- a/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.tsx +++ b/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.tsx @@ -59,7 +59,7 @@ export const UnifiedDataTableAdditionalDisplaySettings: React.FC< step={1} value={activeSampleSize} onChange={onChangeActiveSampleSize} - data-test-subj="unifiedDataTableSampleSizeRange" + data-test-subj="unifiedDataTableSampleSizeInput" /> ); diff --git a/test/functional/apps/discover/embeddable/_saved_search_embeddable.ts b/test/functional/apps/discover/embeddable/_saved_search_embeddable.ts index 63fd699b7afc2..2fb99d9ebb43f 100644 --- a/test/functional/apps/discover/embeddable/_saved_search_embeddable.ts +++ b/test/functional/apps/discover/embeddable/_saved_search_embeddable.ts @@ -86,8 +86,6 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await dataGrid.checkCurrentRowsPerPageToBe(10); }); - // TODO: add a test for sampleSize setting - it('should control columns correctly', async () => { await addSearchEmbeddableToDashboard(); await PageObjects.dashboard.switchToEditMode(); diff --git a/test/functional/apps/discover/group2/_data_grid_pagination.ts b/test/functional/apps/discover/group2/_data_grid_pagination.ts index d7ff267b8a492..4c0a3aa53759e 100644 --- a/test/functional/apps/discover/group2/_data_grid_pagination.ts +++ b/test/functional/apps/discover/group2/_data_grid_pagination.ts @@ -119,7 +119,5 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect((await dataGrid.getDocTableRows()).length).to.be(10); // as in the saved search await dataGrid.checkCurrentRowsPerPageToBe(10); }); - - // TODO: add a test for sampleSize setting }); } diff --git a/test/functional/apps/discover/group2/_data_grid_sample_size.ts b/test/functional/apps/discover/group2/_data_grid_sample_size.ts new file mode 100644 index 0000000000000..7f133905d3de5 --- /dev/null +++ b/test/functional/apps/discover/group2/_data_grid_sample_size.ts @@ -0,0 +1,195 @@ +/* + * 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'; + +const DEFAULT_ROWS_PER_PAGE = 100; +const DEFAULT_SAMPLE_SIZE = 300; +const CUSTOM_SAMPLE_SIZE = 750; +const CUSTOM_SAMPLE_SIZE_FOR_SAVED_SEARCH = 150; +const CUSTOM_SAMPLE_SIZE_FOR_DASHBOARD_PANEL = 10; +const FOOTER_SELECTOR = 'unifiedDataTableFooter'; +const SAVED_SEARCH_NAME = 'With sample size'; + +export default function ({ getService, getPageObjects }: FtrProviderContext) { + const browser = getService('browser'); + const esArchiver = getService('esArchiver'); + const kibanaServer = getService('kibanaServer'); + const dataGrid = getService('dataGrid'); + const testSubjects = getService('testSubjects'); + const retry = getService('retry'); + const dashboardAddPanel = getService('dashboardAddPanel'); + const PageObjects = getPageObjects([ + 'settings', + 'common', + 'discover', + 'header', + 'timePicker', + 'dashboard', + ]); + const security = getService('security'); + const defaultSettings = { + defaultIndex: 'logstash-*', + 'discover:sampleSize': DEFAULT_SAMPLE_SIZE, + 'discover:rowHeightOption': 0, // single line + 'discover:sampleRowsPerPage': DEFAULT_ROWS_PER_PAGE, + hideAnnouncements: true, + }; + + describe('discover data grid sample size', function describeIndexTests() { + before(async () => { + await security.testUser.setRoles(['kibana_admin', 'test_logstash_reader']); + await esArchiver.loadIfNeeded('test/functional/fixtures/es_archiver/logstash_functional'); + await kibanaServer.importExport.load('test/functional/fixtures/kbn_archiver/discover'); + }); + + after(async () => { + await kibanaServer.importExport.unload('test/functional/fixtures/kbn_archiver/discover'); + await kibanaServer.uiSettings.replace({}); + await kibanaServer.savedObjects.cleanStandardList(); + }); + + beforeEach(async function () { + await PageObjects.timePicker.setDefaultAbsoluteRangeViaUiSettings(); + await kibanaServer.uiSettings.update(defaultSettings); + await PageObjects.common.navigateToApp('discover'); + await PageObjects.header.waitUntilLoadingHasFinished(); + await PageObjects.discover.waitUntilSearchingHasFinished(); + }); + + async function goToLastPageAndCheckFooterMessage(sampleSize: number) { + const lastPageNumber = Math.ceil(sampleSize / DEFAULT_ROWS_PER_PAGE) - 1; + + // go to the last page + await testSubjects.click(`pagination-button-${lastPageNumber}`); + // footer is shown now + await retry.try(async function () { + await testSubjects.existOrFail(FOOTER_SELECTOR); + }); + expect( + (await testSubjects.getVisibleText(FOOTER_SELECTOR)).includes(String(sampleSize)) + ).to.be(true); + } + + it('should use the default sample size', async () => { + await dataGrid.clickGridSettings(); + expect(await dataGrid.getCurrentSampleSizeValue()).to.be(DEFAULT_SAMPLE_SIZE); + await goToLastPageAndCheckFooterMessage(DEFAULT_SAMPLE_SIZE); + }); + + it('should allow to change sample size', async () => { + await dataGrid.clickGridSettings(); + expect(await dataGrid.getCurrentSampleSizeValue()).to.be(DEFAULT_SAMPLE_SIZE); + + await dataGrid.changeSampleSizeValue(CUSTOM_SAMPLE_SIZE); + + await PageObjects.header.waitUntilLoadingHasFinished(); + await PageObjects.discover.waitUntilSearchingHasFinished(); + + expect(await dataGrid.getCurrentSampleSizeValue()).to.be(CUSTOM_SAMPLE_SIZE); + await goToLastPageAndCheckFooterMessage(CUSTOM_SAMPLE_SIZE); + }); + + it('should persist the selection after reloading the page', async () => { + await dataGrid.clickGridSettings(); + expect(await dataGrid.getCurrentSampleSizeValue()).to.be(DEFAULT_SAMPLE_SIZE); + + await dataGrid.changeSampleSizeValue(CUSTOM_SAMPLE_SIZE); + + await PageObjects.header.waitUntilLoadingHasFinished(); + await PageObjects.discover.waitUntilSearchingHasFinished(); + + await browser.refresh(); + + await PageObjects.discover.waitUntilSearchingHasFinished(); + await dataGrid.clickGridSettings(); + + expect(await dataGrid.getCurrentSampleSizeValue()).to.be(CUSTOM_SAMPLE_SIZE); + await goToLastPageAndCheckFooterMessage(CUSTOM_SAMPLE_SIZE); + }); + + it('should save a custom sample size with a search', async () => { + await dataGrid.clickGridSettings(); + expect(await dataGrid.getCurrentSampleSizeValue()).to.be(DEFAULT_SAMPLE_SIZE); + + await dataGrid.changeSampleSizeValue(CUSTOM_SAMPLE_SIZE_FOR_SAVED_SEARCH); + + await PageObjects.header.waitUntilLoadingHasFinished(); + await PageObjects.discover.waitUntilSearchingHasFinished(); + + await PageObjects.discover.saveSearch(SAVED_SEARCH_NAME); + + await PageObjects.discover.waitUntilSearchingHasFinished(); + await dataGrid.clickGridSettings(); + + expect(await dataGrid.getCurrentSampleSizeValue()).to.be(CUSTOM_SAMPLE_SIZE_FOR_SAVED_SEARCH); + await goToLastPageAndCheckFooterMessage(CUSTOM_SAMPLE_SIZE_FOR_SAVED_SEARCH); + + // reset to the default value + await PageObjects.discover.clickNewSearchButton(); + await dataGrid.clickGridSettings(); + expect(await dataGrid.getCurrentSampleSizeValue()).to.be(DEFAULT_SAMPLE_SIZE); + await goToLastPageAndCheckFooterMessage(DEFAULT_SAMPLE_SIZE); + + // load the saved search again + await PageObjects.discover.loadSavedSearch(SAVED_SEARCH_NAME); + await dataGrid.clickGridSettings(); + expect(await dataGrid.getCurrentSampleSizeValue()).to.be(CUSTOM_SAMPLE_SIZE_FOR_SAVED_SEARCH); + await goToLastPageAndCheckFooterMessage(CUSTOM_SAMPLE_SIZE_FOR_SAVED_SEARCH); + + // load another saved search without a custom sample size + await PageObjects.discover.loadSavedSearch('A Saved Search'); + await dataGrid.clickGridSettings(); + expect(await dataGrid.getCurrentSampleSizeValue()).to.be(DEFAULT_SAMPLE_SIZE); + await goToLastPageAndCheckFooterMessage(DEFAULT_SAMPLE_SIZE); + }); + + it('should use the default sample size on Dashboard', async () => { + await PageObjects.common.navigateToApp('dashboard'); + await PageObjects.dashboard.clickNewDashboard(); + await dashboardAddPanel.clickOpenAddPanel(); + await dashboardAddPanel.addSavedSearch('A Saved Search'); + + await dataGrid.clickGridSettings(); + expect(await dataGrid.getCurrentSampleSizeValue()).to.be(DEFAULT_SAMPLE_SIZE); + await goToLastPageAndCheckFooterMessage(DEFAULT_SAMPLE_SIZE); + }); + + it('should use custom sample size on Dashboard when specified', async () => { + await PageObjects.common.navigateToApp('dashboard'); + await PageObjects.dashboard.clickNewDashboard(); + await dashboardAddPanel.clickOpenAddPanel(); + await dashboardAddPanel.addSavedSearch(SAVED_SEARCH_NAME); + + await dataGrid.clickGridSettings(); + expect(await dataGrid.getCurrentSampleSizeValue()).to.be(CUSTOM_SAMPLE_SIZE_FOR_SAVED_SEARCH); + + await dataGrid.changeSampleSizeValue(CUSTOM_SAMPLE_SIZE_FOR_DASHBOARD_PANEL); + + await PageObjects.header.waitUntilLoadingHasFinished(); + + await dataGrid.clickGridSettings(); + expect(await dataGrid.getCurrentSampleSizeValue()).to.be( + CUSTOM_SAMPLE_SIZE_FOR_DASHBOARD_PANEL + ); + await goToLastPageAndCheckFooterMessage(CUSTOM_SAMPLE_SIZE_FOR_DASHBOARD_PANEL); + + await PageObjects.dashboard.saveDashboard('test'); + + await browser.refresh(); + await PageObjects.header.waitUntilLoadingHasFinished(); + + await dataGrid.clickGridSettings(); + expect(await dataGrid.getCurrentSampleSizeValue()).to.be( + CUSTOM_SAMPLE_SIZE_FOR_DASHBOARD_PANEL + ); + await goToLastPageAndCheckFooterMessage(CUSTOM_SAMPLE_SIZE_FOR_DASHBOARD_PANEL); + }); + }); +} diff --git a/test/functional/apps/discover/group2/index.ts b/test/functional/apps/discover/group2/index.ts index a01110b5dc6ac..0af704bd1768f 100644 --- a/test/functional/apps/discover/group2/index.ts +++ b/test/functional/apps/discover/group2/index.ts @@ -30,6 +30,7 @@ export default function ({ getService, loadTestFile }: FtrProviderContext) { loadTestFile(require.resolve('./_data_grid_doc_table')); loadTestFile(require.resolve('./_data_grid_copy_to_clipboard')); loadTestFile(require.resolve('./_data_grid_row_height')); + loadTestFile(require.resolve('./_data_grid_sample_size')); loadTestFile(require.resolve('./_data_grid_pagination')); loadTestFile(require.resolve('./_data_grid_footer')); loadTestFile(require.resolve('./_data_grid_field_tokens')); diff --git a/test/functional/services/data_grid.ts b/test/functional/services/data_grid.ts index 337fea7c3ff45..321f1ff715dd2 100644 --- a/test/functional/services/data_grid.ts +++ b/test/functional/services/data_grid.ts @@ -7,6 +7,7 @@ */ import { chunk } from 'lodash'; +import { Key } from 'selenium-webdriver'; import { FtrService } from '../ftr_provider_context'; import { WebElementWrapper } from './lib/web_element_wrapper'; @@ -366,6 +367,21 @@ export class DataGridService extends FtrService { await this.testSubjects.click('resetDisplaySelector'); } + public async getCurrentSampleSizeValue() { + const sampleSizeInput = await this.testSubjects.find('unifiedDataTableSampleSizeInput'); + return Number(await sampleSizeInput.getAttribute('value')); + } + + public async changeSampleSizeValue(newValue: number) { + const sampleSizeInput = await this.testSubjects.find('unifiedDataTableSampleSizeInput'); + await sampleSizeInput.focus(); + await sampleSizeInput.pressKeys([ + Key[process.platform === 'darwin' ? 'COMMAND' : 'CONTROL'], + 'a', + ]); + await sampleSizeInput.type(String(newValue)); + } + public async getDetailsRow(): Promise { const detailRows = await this.getDetailsRows(); return detailRows[0]; From 6866eb461e5accc55ab54c1eb44eabf7e1d9f3ac Mon Sep 17 00:00:00 2001 From: Julia Rechkunova Date: Fri, 6 Oct 2023 15:08:19 +0200 Subject: [PATCH 27/32] [Discover] Update code style --- .../application/main/components/layout/discover_documents.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 09e7749c018a2..698cdd7ee3818 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 @@ -128,7 +128,7 @@ function DiscoverDocumentsComponent({ 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 sampleSize = useMemo(() => uiSettings.get(SAMPLE_SIZE_SETTING), [uiSettings]); + const defaultSampleSize = useMemo(() => uiSettings.get(SAMPLE_SIZE_SETTING), [uiSettings]); const documentState = useDataState(documents$); const isDataLoading = @@ -338,7 +338,7 @@ function DiscoverDocumentsComponent({ isPlainRecord={isTextBasedQuery} rowsPerPageState={rowsPerPage ?? getDefaultRowsPerPage(services.uiSettings)} onUpdateRowsPerPage={onUpdateRowsPerPage} - sampleSizeState={sampleSizeState || sampleSize} + sampleSizeState={sampleSizeState || defaultSampleSize} onUpdateSampleSize={!isTextBasedQuery ? onUpdateSampleSize : undefined} onFieldEdited={onFieldEdited} configRowHeight={uiSettings.get(ROW_HEIGHT_OPTION)} From a636a958b97a39863b2863cedc56f6074229cf1c Mon Sep 17 00:00:00 2001 From: Julia Rechkunova Date: Fri, 6 Oct 2023 15:25:33 +0200 Subject: [PATCH 28/32] [Discover] Fix messaging for legacy table embeddable --- .../components/doc_table/create_doc_table_embeddable.tsx | 1 + .../public/components/doc_table/doc_table_embeddable.tsx | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/plugins/discover/public/components/doc_table/create_doc_table_embeddable.tsx b/src/plugins/discover/public/components/doc_table/create_doc_table_embeddable.tsx index e0f24c2839113..a0a55a17a9cba 100644 --- a/src/plugins/discover/public/components/doc_table/create_doc_table_embeddable.tsx +++ b/src/plugins/discover/public/components/doc_table/create_doc_table_embeddable.tsx @@ -17,6 +17,7 @@ export function DiscoverDocTableEmbeddable(renderProps: DocTableEmbeddableProps) columns={renderProps.columns} rows={renderProps.rows} rowsPerPageState={renderProps.rowsPerPageState} + sampleSizeState={renderProps.sampleSizeState} onUpdateRowsPerPage={renderProps.onUpdateRowsPerPage} totalHitCount={renderProps.totalHitCount} dataView={renderProps.dataView} diff --git a/src/plugins/discover/public/components/doc_table/doc_table_embeddable.tsx b/src/plugins/discover/public/components/doc_table/doc_table_embeddable.tsx index fbe8ed083ebc3..7de8e9f86b226 100644 --- a/src/plugins/discover/public/components/doc_table/doc_table_embeddable.tsx +++ b/src/plugins/discover/public/components/doc_table/doc_table_embeddable.tsx @@ -23,6 +23,7 @@ import { SavedSearchEmbeddableBase } from '../../embeddable/saved_search_embedda export interface DocTableEmbeddableProps extends DocTableProps { totalHitCount?: number; rowsPerPageState?: number; + sampleSizeState?: number; interceptedWarnings?: SearchResponseInterceptedWarning[]; onUpdateRowsPerPage?: (rowsPerPage?: number) => void; } @@ -84,8 +85,8 @@ export const DocTableEmbeddable = (props: DocTableEmbeddableProps) => { ); const sampleSize = useMemo(() => { - return services.uiSettings.get(SAMPLE_SIZE_SETTING, 500); - }, [services]); + return props.sampleSizeState || services.uiSettings.get(SAMPLE_SIZE_SETTING, 500); + }, [services, props.sampleSizeState]); const renderDocTable = useCallback( (renderProps: DocTableRenderProps) => { From 7feb2aeef7f9ba34465329c40a240def16352141 Mon Sep 17 00:00:00 2001 From: Julia Rechkunova Date: Mon, 9 Oct 2023 11:10:30 +0200 Subject: [PATCH 29/32] [Discover] Switch back to EuiRange --- ...table_additional_display_settings.test.tsx | 29 +++++++++++-------- ...data_table_additional_display_settings.tsx | 19 ++++++++---- test/functional/services/data_grid.ts | 11 +++++-- 3 files changed, 40 insertions(+), 19 deletions(-) diff --git a/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.test.tsx b/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.test.tsx index 7cecfdfcb7676..b6effab9d9e9e 100644 --- a/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.test.tsx +++ b/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.test.tsx @@ -29,8 +29,7 @@ describe('UnifiedDataTableAdditionalDisplaySettings', function () { onChangeSampleSize={onChangeSampleSizeMock} /> ); - const input = findTestSubject(component, 'unifiedDataTableSampleSizeInput'); - expect(input.exists()).toBe(true); + const input = findTestSubject(component, 'unifiedDataTableSampleSizeInput').last(); expect(input.prop('value')).toBe(10); await act(async () => { @@ -46,21 +45,23 @@ describe('UnifiedDataTableAdditionalDisplaySettings', function () { await new Promise((resolve) => setTimeout(resolve, 0)); component.update(); - expect(findTestSubject(component, 'unifiedDataTableSampleSizeInput').prop('value')).toBe(100); + expect( + findTestSubject(component, 'unifiedDataTableSampleSizeInput').last().prop('value') + ).toBe(100); }); it('should not execute the callback for an invalid input', async () => { - const invalidValue = MAX_ALLOWED_SAMPLE_SIZE + 1; + const invalidValue = MAX_ALLOWED_SAMPLE_SIZE + 10; const onChangeSampleSizeMock = jest.fn(); const component = mountWithIntl( ); - const input = findTestSubject(component, 'unifiedDataTableSampleSizeInput'); - expect(input.prop('value')).toBe(5); + const input = findTestSubject(component, 'unifiedDataTableSampleSizeInput').last(); + expect(input.prop('value')).toBe(50); await act(async () => { input.simulate('change', { @@ -73,9 +74,9 @@ describe('UnifiedDataTableAdditionalDisplaySettings', function () { await new Promise((resolve) => setTimeout(resolve, 0)); component.update(); - expect(findTestSubject(component, 'unifiedDataTableSampleSizeInput').prop('value')).toBe( - invalidValue - ); + expect( + findTestSubject(component, 'unifiedDataTableSampleSizeInput').last().prop('value') + ).toBe(invalidValue); expect(onChangeSampleSizeMock).not.toHaveBeenCalled(); }); @@ -90,7 +91,9 @@ describe('UnifiedDataTableAdditionalDisplaySettings', function () { /> ); - expect(findTestSubject(component, 'unifiedDataTableSampleSizeInput').prop('value')).toBe(200); + expect( + findTestSubject(component, 'unifiedDataTableSampleSizeInput').last().prop('value') + ).toBe(200); component.setProps({ sampleSize: 500, @@ -100,7 +103,9 @@ describe('UnifiedDataTableAdditionalDisplaySettings', function () { await new Promise((resolve) => setTimeout(resolve, 0)); component.update(); - expect(findTestSubject(component, 'unifiedDataTableSampleSizeInput').prop('value')).toBe(500); + expect( + findTestSubject(component, 'unifiedDataTableSampleSizeInput').last().prop('value') + ).toBe(500); expect(onChangeSampleSizeMock).not.toHaveBeenCalled(); }); diff --git a/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.tsx b/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.tsx index 3cd9f2d7f8a4a..701145e268bb2 100644 --- a/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.tsx +++ b/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.tsx @@ -7,11 +7,13 @@ */ import React, { useCallback, useEffect, useMemo, useState } from 'react'; -import { EuiFormRow, EuiFieldNumber } from '@elastic/eui'; +import { EuiFormRow, EuiRange } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { debounce } from 'lodash'; export const MAX_ALLOWED_SAMPLE_SIZE = 1000; +export const MIN_ALLOWED_SAMPLE_SIZE = 10; +export const SAMPLE_SIZE_STEP = 10; export interface UnifiedDataTableAdditionalDisplaySettingsProps { sampleSize: number; @@ -21,7 +23,7 @@ export interface UnifiedDataTableAdditionalDisplaySettingsProps { export const UnifiedDataTableAdditionalDisplaySettings: React.FC< UnifiedDataTableAdditionalDisplaySettingsProps > = ({ sampleSize, onChangeSampleSize }) => { - const [activeSampleSize, setActiveSampleSize] = useState(sampleSize); + const [activeSampleSize, setActiveSampleSize] = useState(sampleSize); const debouncedOnChangeSampleSize = useMemo( () => debounce(onChangeSampleSize, 300, { leading: false, trailing: true }), @@ -30,7 +32,13 @@ export const UnifiedDataTableAdditionalDisplaySettings: React.FC< const onChangeActiveSampleSize = useCallback( (event) => { + if (!event.target.value) { + setActiveSampleSize(''); + return; + } + const newSampleSize = Number(event.target.value); + if (newSampleSize > 0) { setActiveSampleSize(newSampleSize); if (newSampleSize <= MAX_ALLOWED_SAMPLE_SIZE) { @@ -51,12 +59,13 @@ export const UnifiedDataTableAdditionalDisplaySettings: React.FC< return ( - Date: Fri, 13 Oct 2023 15:00:35 +0200 Subject: [PATCH 30/32] [Discover] Limit fetched sample by settings and 10_000 --- .../src/components/data_table.tsx | 8 +- ...data_table_additional_display_settings.tsx | 30 ++++--- .../components/layout/discover_documents.tsx | 9 +- .../components/top_nav/on_save_search.tsx | 13 ++- .../services/discover_app_state_container.ts | 2 +- .../main/services/load_saved_search.ts | 2 +- .../main/utils/cleanup_url_state.test.ts | 84 ++++++++++++------- .../main/utils/cleanup_url_state.ts | 14 +++- .../application/main/utils/fetch_documents.ts | 5 +- .../doc_table/doc_table_embeddable.tsx | 12 +-- .../doc_table/doc_table_infinite.tsx | 10 ++- .../embeddable/saved_search_embeddable.tsx | 31 +++---- .../saved_search_embeddable_component.tsx | 4 + .../public/embeddable/saved_search_grid.tsx | 4 +- .../utils/update_search_source.test.ts | 21 +++-- .../embeddable/utils/update_search_source.ts | 7 +- .../public/utils/get_allowed_sample_size.ts | 30 +++++++ src/plugins/saved_search/common/constants.ts | 3 + .../content_management/v1/cm_services.ts | 8 +- src/plugins/saved_search/common/index.ts | 7 +- .../server/saved_objects/schema.ts | 13 ++- 21 files changed, 218 insertions(+), 99 deletions(-) create mode 100644 src/plugins/discover/public/utils/get_allowed_sample_size.ts diff --git a/packages/kbn-unified-data-table/src/components/data_table.tsx b/packages/kbn-unified-data-table/src/components/data_table.tsx index 0e9b800ccc6d7..22a625f479e3b 100644 --- a/packages/kbn-unified-data-table/src/components/data_table.tsx +++ b/packages/kbn-unified-data-table/src/components/data_table.tsx @@ -203,6 +203,10 @@ export interface UnifiedDataTableProps { * Update rows per page state */ onUpdateRowsPerPage?: (rowsPerPage: number) => void; + /** + * Configuration option to limit sample size slider + */ + maxAllowedSampleSize?: number; /** * The max size of the documents returned by Elasticsearch */ @@ -347,6 +351,7 @@ export const UnifiedDataTable = ({ className, rowHeightState, onUpdateRowHeight, + maxAllowedSampleSize, sampleSizeState, onUpdateSampleSize, isPlainRecord = false, @@ -734,6 +739,7 @@ export const UnifiedDataTable = ({ options.allowResetButton = false; options.additionalDisplaySettings = ( @@ -741,7 +747,7 @@ export const UnifiedDataTable = ({ } return Object.keys(options).length ? options : undefined; - }, [sampleSizeState, onUpdateRowHeight, onUpdateSampleSize]); + }, [maxAllowedSampleSize, sampleSizeState, onUpdateRowHeight, onUpdateSampleSize]); const inMemory = useMemo(() => { return isPlainRecord && columns.length diff --git a/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.tsx b/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.tsx index 701145e268bb2..2555c5f253929 100644 --- a/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.tsx +++ b/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.tsx @@ -11,19 +11,29 @@ import { EuiFormRow, EuiRange } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { debounce } from 'lodash'; -export const MAX_ALLOWED_SAMPLE_SIZE = 1000; -export const MIN_ALLOWED_SAMPLE_SIZE = 10; -export const SAMPLE_SIZE_STEP = 10; +export const DEFAULT_MAX_ALLOWED_SAMPLE_SIZE = 1000; +export const MIN_ALLOWED_SAMPLE_SIZE = 1; +export const RANGE_MIN_SAMPLE_SIZE = 10; // it's necessary to be able to use `step={10}` configuration for EuiRange +export const RANGE_STEP_SAMPLE_SIZE = 10; export interface UnifiedDataTableAdditionalDisplaySettingsProps { + maxAllowedSampleSize?: number; sampleSize: number; onChangeSampleSize: (sampleSize: number) => void; } export const UnifiedDataTableAdditionalDisplaySettings: React.FC< UnifiedDataTableAdditionalDisplaySettingsProps -> = ({ sampleSize, onChangeSampleSize }) => { +> = ({ + maxAllowedSampleSize = DEFAULT_MAX_ALLOWED_SAMPLE_SIZE, + sampleSize, + onChangeSampleSize, +}) => { const [activeSampleSize, setActiveSampleSize] = useState(sampleSize); + const minRangeSampleSize = Math.max( + Math.min(RANGE_MIN_SAMPLE_SIZE, sampleSize), + MIN_ALLOWED_SAMPLE_SIZE + ); // flexible: allows to go lower than RANGE_MIN_SAMPLE_SIZE but greater than MIN_ALLOWED_SAMPLE_SIZE const debouncedOnChangeSampleSize = useMemo( () => debounce(onChangeSampleSize, 300, { leading: false, trailing: true }), @@ -39,14 +49,14 @@ export const UnifiedDataTableAdditionalDisplaySettings: React.FC< const newSampleSize = Number(event.target.value); - if (newSampleSize > 0) { + if (newSampleSize >= MIN_ALLOWED_SAMPLE_SIZE) { setActiveSampleSize(newSampleSize); - if (newSampleSize <= MAX_ALLOWED_SAMPLE_SIZE) { + if (newSampleSize <= maxAllowedSampleSize) { debouncedOnChangeSampleSize(newSampleSize); } } }, - [setActiveSampleSize, debouncedOnChangeSampleSize] + [maxAllowedSampleSize, setActiveSampleSize, debouncedOnChangeSampleSize] ); const sampleSizeLabel = i18n.translate('unifiedDataTable.sampleSizeSettings.sampleSizeLabel', { @@ -62,9 +72,9 @@ export const UnifiedDataTableAdditionalDisplaySettings: React.FC< !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 defaultSampleSize = useMemo(() => uiSettings.get(SAMPLE_SIZE_SETTING), [uiSettings]); const documentState = useDataState(documents$); const isDataLoading = @@ -338,7 +340,8 @@ function DiscoverDocumentsComponent({ isPlainRecord={isTextBasedQuery} rowsPerPageState={rowsPerPage ?? getDefaultRowsPerPage(services.uiSettings)} onUpdateRowsPerPage={onUpdateRowsPerPage} - sampleSizeState={sampleSizeState || defaultSampleSize} + maxAllowedSampleSize={getMaxAllowedSampleSize(services.uiSettings)} + sampleSizeState={getAllowedSampleSize(sampleSizeState, services.uiSettings)} onUpdateSampleSize={!isTextBasedQuery ? onUpdateSampleSize : undefined} onFieldEdited={onFieldEdited} configRowHeight={uiSettings.get(ROW_HEIGHT_OPTION)} 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 42ce5ba0b891a..abae8e83b41a1 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 @@ -15,6 +15,7 @@ import { SavedSearch, SaveSavedSearchOptions } from '@kbn/saved-search-plugin/pu import { DOC_TABLE_LEGACY } from '@kbn/discover-utils'; import { DiscoverServices } from '../../../../build_services'; import { DiscoverStateContainer } from '../../services/discover_state'; +import { getAllowedSampleSize } from '../../../../utils/get_allowed_sample_size'; async function saveDataSource({ savedSearch, @@ -119,9 +120,15 @@ export async function onSaveSearch({ savedSearch.rowsPerPage = uiSettings.get(DOC_TABLE_LEGACY) ? currentRowsPerPage : state.appState.getState().rowsPerPage; - savedSearch.sampleSize = uiSettings.get(DOC_TABLE_LEGACY) - ? currentSampleSize - : state.appState.getState().sampleSize; + + // save the custom value or reset it if it's invalid + const appStateSampleSize = state.appState.getState().sampleSize; + const allowedSampleSize = getAllowedSampleSize(appStateSampleSize, uiSettings); + savedSearch.sampleSize = + appStateSampleSize && allowedSampleSize === appStateSampleSize + ? appStateSampleSize + : undefined; + if (savedObjectsTagging) { savedSearch.tags = newTags; } diff --git a/src/plugins/discover/public/application/main/services/discover_app_state_container.ts b/src/plugins/discover/public/application/main/services/discover_app_state_container.ts index 77205152b4f02..124c83beda236 100644 --- a/src/plugins/discover/public/application/main/services/discover_app_state_container.ts +++ b/src/plugins/discover/public/application/main/services/discover_app_state_container.ts @@ -303,7 +303,7 @@ export function getInitialState( ? defaultAppState : { ...defaultAppState, - ...cleanupUrlState(stateStorageURL), + ...cleanupUrlState(stateStorageURL, services.uiSettings), }, services.uiSettings ); diff --git a/src/plugins/discover/public/application/main/services/load_saved_search.ts b/src/plugins/discover/public/application/main/services/load_saved_search.ts index 3631ca876cce6..8b8dcc2beb2f4 100644 --- a/src/plugins/discover/public/application/main/services/load_saved_search.ts +++ b/src/plugins/discover/public/application/main/services/load_saved_search.ts @@ -91,7 +91,7 @@ export const loadSavedSearch = async ( // Update app state container with the next state derived from the next saved search const nextAppState = getInitialState(undefined, nextSavedSearch, services); const mergedAppState = appState - ? { ...nextAppState, ...cleanupUrlState({ ...appState }) } + ? { ...nextAppState, ...cleanupUrlState({ ...appState }, services.uiSettings) } : nextAppState; appStateContainer.resetToState(mergedAppState); diff --git a/src/plugins/discover/public/application/main/utils/cleanup_url_state.test.ts b/src/plugins/discover/public/application/main/utils/cleanup_url_state.test.ts index 7ff24e8aa4ff5..2d49639e02884 100644 --- a/src/plugins/discover/public/application/main/utils/cleanup_url_state.test.ts +++ b/src/plugins/discover/public/application/main/utils/cleanup_url_state.test.ts @@ -8,11 +8,14 @@ import { AppStateUrl } from '../services/discover_app_state_container'; import { cleanupUrlState } from './cleanup_url_state'; +import { createDiscoverServicesMock } from '../../../__mocks__/services'; + +const services = createDiscoverServicesMock(); describe('cleanupUrlState', () => { test('cleaning up legacy sort', async () => { const state = { sort: ['batman', 'desc'] } as AppStateUrl; - expect(cleanupUrlState(state)).toMatchInlineSnapshot(` + expect(cleanupUrlState(state, services.uiSettings)).toMatchInlineSnapshot(` Object { "sort": Array [ Array [ @@ -25,7 +28,7 @@ describe('cleanupUrlState', () => { }); test('not cleaning up broken legacy sort', async () => { const state = { sort: ['batman'] } as unknown as AppStateUrl; - expect(cleanupUrlState(state)).toMatchInlineSnapshot(`Object {}`); + expect(cleanupUrlState(state, services.uiSettings)).toMatchInlineSnapshot(`Object {}`); }); test('not cleaning up regular sort', async () => { const state = { @@ -34,7 +37,7 @@ describe('cleanupUrlState', () => { ['robin', 'asc'], ], } as AppStateUrl; - expect(cleanupUrlState(state)).toMatchInlineSnapshot(` + expect(cleanupUrlState(state, services.uiSettings)).toMatchInlineSnapshot(` Object { "sort": Array [ Array [ @@ -53,14 +56,14 @@ describe('cleanupUrlState', () => { const state = { sort: [], } as AppStateUrl; - expect(cleanupUrlState(state)).toMatchInlineSnapshot(`Object {}`); + expect(cleanupUrlState(state, services.uiSettings)).toMatchInlineSnapshot(`Object {}`); }); test('should keep a valid rowsPerPage', async () => { const state = { rowsPerPage: 50, } as AppStateUrl; - expect(cleanupUrlState(state)).toMatchInlineSnapshot(` + expect(cleanupUrlState(state, services.uiSettings)).toMatchInlineSnapshot(` Object { "rowsPerPage": 50, } @@ -71,38 +74,63 @@ describe('cleanupUrlState', () => { const state = { rowsPerPage: -50, } as AppStateUrl; - expect(cleanupUrlState(state)).toMatchInlineSnapshot(`Object {}`); + expect(cleanupUrlState(state, services.uiSettings)).toMatchInlineSnapshot(`Object {}`); }); test('should remove an invalid rowsPerPage', async () => { const state = { rowsPerPage: 'test', } as unknown as AppStateUrl; - expect(cleanupUrlState(state)).toMatchInlineSnapshot(`Object {}`); + expect(cleanupUrlState(state, services.uiSettings)).toMatchInlineSnapshot(`Object {}`); }); - test('should keep a valid sampleSize', async () => { - const state = { - sampleSize: 50, - } as AppStateUrl; - expect(cleanupUrlState(state)).toMatchInlineSnapshot(` - Object { - "sampleSize": 50, - } - `); - }); + describe('sampleSize', function () { + test('should keep a valid sampleSize', async () => { + const state = { + sampleSize: 50, + } as AppStateUrl; + expect(cleanupUrlState(state, services.uiSettings)).toMatchInlineSnapshot(` + Object { + "sampleSize": 50, + } + `); + }); - test('should remove a negative sampleSize', async () => { - const state = { - sampleSize: -50, - } as AppStateUrl; - expect(cleanupUrlState(state)).toMatchInlineSnapshot(`Object {}`); - }); + test('should remove for ES|QL', async () => { + const state = { + sampleSize: 50, + query: { + esql: 'from test', + }, + } as AppStateUrl; + expect(cleanupUrlState(state, services.uiSettings)).toMatchInlineSnapshot(` + Object { + "query": Object { + "esql": "from test", + }, + } + `); + }); - test('should remove an invalid sampleSize', async () => { - const state = { - sampleSize: 'test', - } as unknown as AppStateUrl; - expect(cleanupUrlState(state)).toMatchInlineSnapshot(`Object {}`); + test('should remove a negative sampleSize', async () => { + const state = { + sampleSize: -50, + } as AppStateUrl; + expect(cleanupUrlState(state, services.uiSettings)).toMatchInlineSnapshot(`Object {}`); + }); + + test('should remove an invalid sampleSize', async () => { + const state = { + sampleSize: 'test', + } as unknown as AppStateUrl; + expect(cleanupUrlState(state, services.uiSettings)).toMatchInlineSnapshot(`Object {}`); + }); + + test('should remove a too large sampleSize', async () => { + const state = { + sampleSize: 500000, + } as AppStateUrl; + expect(cleanupUrlState(state, services.uiSettings)).toMatchInlineSnapshot(`Object {}`); + }); }); }); diff --git a/src/plugins/discover/public/application/main/utils/cleanup_url_state.ts b/src/plugins/discover/public/application/main/utils/cleanup_url_state.ts index 464ef426ac7be..cdfb95d87f134 100644 --- a/src/plugins/discover/public/application/main/utils/cleanup_url_state.ts +++ b/src/plugins/discover/public/application/main/utils/cleanup_url_state.ts @@ -6,14 +6,19 @@ * Side Public License, v 1. */ import { isOfAggregateQueryType } from '@kbn/es-query'; +import type { IUiSettingsClient } from '@kbn/core-ui-settings-browser'; import { DiscoverAppState, AppStateUrl } from '../services/discover_app_state_container'; import { migrateLegacyQuery } from '../../../utils/migrate_legacy_query'; +import { getMaxAllowedSampleSize } from '../../../utils/get_allowed_sample_size'; /** * Takes care of the given url state, migrates legacy props and cleans up empty props * @param appStateFromUrl */ -export function cleanupUrlState(appStateFromUrl: AppStateUrl): DiscoverAppState { +export function cleanupUrlState( + appStateFromUrl: AppStateUrl, + uiSettings: IUiSettingsClient +): DiscoverAppState { if ( appStateFromUrl && appStateFromUrl.query && @@ -48,7 +53,12 @@ export function cleanupUrlState(appStateFromUrl: AppStateUrl): DiscoverAppState if ( appStateFromUrl?.sampleSize && - !(typeof appStateFromUrl.sampleSize === 'number' && appStateFromUrl.sampleSize > 0) + (isOfAggregateQueryType(appStateFromUrl.query) || // not supported yet for ES|QL + !( + typeof appStateFromUrl.sampleSize === 'number' && + appStateFromUrl.sampleSize > 0 && + appStateFromUrl.sampleSize <= getMaxAllowedSampleSize(uiSettings) + )) ) { // remove the param if it's invalid delete appStateFromUrl.sampleSize; diff --git a/src/plugins/discover/public/application/main/utils/fetch_documents.ts b/src/plugins/discover/public/application/main/utils/fetch_documents.ts index eeb5ed7d4b43e..b1e18273479bf 100644 --- a/src/plugins/discover/public/application/main/utils/fetch_documents.ts +++ b/src/plugins/discover/public/application/main/utils/fetch_documents.ts @@ -9,10 +9,11 @@ import { i18n } from '@kbn/i18n'; import { filter, map } from 'rxjs/operators'; import { lastValueFrom } from 'rxjs'; import { isRunningResponse, ISearchSource } from '@kbn/data-plugin/public'; -import { SAMPLE_SIZE_SETTING, buildDataTableRecordList } from '@kbn/discover-utils'; +import { buildDataTableRecordList } from '@kbn/discover-utils'; import type { EsHitRecord } from '@kbn/discover-utils/types'; import { getSearchResponseInterceptedWarnings } from '@kbn/search-response-warnings'; import type { RecordsFetchResponse } from '../../types'; +import { getAllowedSampleSize } from '../../../utils/get_allowed_sample_size'; import { FetchDeps } from './fetch_all'; /** @@ -24,7 +25,7 @@ export const fetchDocuments = ( { abortController, inspectorAdapters, searchSessionId, services, getAppState }: FetchDeps ): Promise => { const sampleSize = getAppState().sampleSize; - searchSource.setField('size', sampleSize || services.uiSettings.get(SAMPLE_SIZE_SETTING)); + searchSource.setField('size', getAllowedSampleSize(sampleSize, services.uiSettings)); searchSource.setField('trackTotalHits', false); searchSource.setField('highlightAll', true); searchSource.setField('version', true); diff --git a/src/plugins/discover/public/components/doc_table/doc_table_embeddable.tsx b/src/plugins/discover/public/components/doc_table/doc_table_embeddable.tsx index 7de8e9f86b226..36e3629f089aa 100644 --- a/src/plugins/discover/public/components/doc_table/doc_table_embeddable.tsx +++ b/src/plugins/discover/public/components/doc_table/doc_table_embeddable.tsx @@ -10,20 +10,19 @@ import React, { memo, useCallback, useMemo, useRef } from 'react'; import './index.scss'; import { FormattedMessage } from '@kbn/i18n-react'; import { EuiText } from '@elastic/eui'; -import { SAMPLE_SIZE_SETTING, usePager } from '@kbn/discover-utils'; +import { usePager } from '@kbn/discover-utils'; import type { SearchResponseInterceptedWarning } from '@kbn/search-response-warnings'; import { ToolBarPagination, MAX_ROWS_PER_PAGE_OPTION, } from './components/pager/tool_bar_pagination'; import { DocTableProps, DocTableRenderProps, DocTableWrapper } from './doc_table_wrapper'; -import { useDiscoverServices } from '../../hooks/use_discover_services'; import { SavedSearchEmbeddableBase } from '../../embeddable/saved_search_embeddable_base'; export interface DocTableEmbeddableProps extends DocTableProps { totalHitCount?: number; rowsPerPageState?: number; - sampleSizeState?: number; + sampleSizeState: number; interceptedWarnings?: SearchResponseInterceptedWarning[]; onUpdateRowsPerPage?: (rowsPerPage?: number) => void; } @@ -31,7 +30,6 @@ export interface DocTableEmbeddableProps extends DocTableProps { const DocTableWrapperMemoized = memo(DocTableWrapper); export const DocTableEmbeddable = (props: DocTableEmbeddableProps) => { - const services = useDiscoverServices(); const onUpdateRowsPerPage = props.onUpdateRowsPerPage; const tableWrapperRef = useRef(null); const { @@ -84,10 +82,6 @@ export const DocTableEmbeddable = (props: DocTableEmbeddableProps) => { [hasNextPage, props.rows.length, props.totalHitCount] ); - const sampleSize = useMemo(() => { - return props.sampleSizeState || services.uiSettings.get(SAMPLE_SIZE_SETTING, 500); - }, [services, props.sampleSizeState]); - const renderDocTable = useCallback( (renderProps: DocTableRenderProps) => { return ( @@ -113,7 +107,7 @@ export const DocTableEmbeddable = (props: DocTableEmbeddableProps) => { ) : undefined diff --git a/src/plugins/discover/public/components/doc_table/doc_table_infinite.tsx b/src/plugins/discover/public/components/doc_table/doc_table_infinite.tsx index cb285746963ac..92265f731bf13 100644 --- a/src/plugins/discover/public/components/doc_table/doc_table_infinite.tsx +++ b/src/plugins/discover/public/components/doc_table/doc_table_infinite.tsx @@ -6,16 +6,17 @@ * Side Public License, v 1. */ -import React, { Fragment, memo, useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import React, { Fragment, memo, useCallback, useEffect, useRef, useState } from 'react'; import './index.scss'; import { FormattedMessage } from '@kbn/i18n-react'; import { debounce } from 'lodash'; import { EuiButtonEmpty } from '@elastic/eui'; -import { SAMPLE_SIZE_SETTING } from '@kbn/discover-utils'; import { DocTableProps, DocTableRenderProps, DocTableWrapper } from './doc_table_wrapper'; import { SkipBottomButton } from '../../application/main/components/skip_bottom_button'; import { shouldLoadNextDocPatch } from './utils/should_load_next_doc_patch'; import { useDiscoverServices } from '../../hooks/use_discover_services'; +import { getAllowedSampleSize } from '../../utils/get_allowed_sample_size'; +import { useAppStateSelector } from '../../application/main/services/discover_app_state_container'; const FOOTER_PADDING = { padding: 0 }; @@ -38,8 +39,9 @@ const DocTableInfiniteContent = ({ onBackToTop, }: DocTableInfiniteContentProps) => { const { uiSettings } = useDiscoverServices(); - - const sampleSize = useMemo(() => uiSettings.get(SAMPLE_SIZE_SETTING, 500), [uiSettings]); + const sampleSize = useAppStateSelector((state) => + getAllowedSampleSize(state.sampleSize, uiSettings) + ); const onSkipBottomButton = useCallback(() => { onSetMaxLimit(); diff --git a/src/plugins/discover/public/embeddable/saved_search_embeddable.tsx b/src/plugins/discover/public/embeddable/saved_search_embeddable.tsx index 6febb91a1145e..e5896215e56de 100644 --- a/src/plugins/discover/public/embeddable/saved_search_embeddable.tsx +++ b/src/plugins/discover/public/embeddable/saved_search_embeddable.tsx @@ -53,7 +53,6 @@ import type { DataTableRecord, EsHitRecord } from '@kbn/discover-utils/types'; import { DOC_HIDE_TIME_COLUMN_SETTING, DOC_TABLE_LEGACY, - SAMPLE_SIZE_SETTING, SEARCH_FIELDS_FROM_SOURCE, SHOW_FIELD_STATISTICS, SORT_DEFAULT_ORDER_SETTING, @@ -65,6 +64,7 @@ import { VIEW_MODE, getDefaultRowsPerPage } from '../../common/constants'; import type { ISearchEmbeddable, SearchInput, SearchOutput } from './types'; import type { DiscoverServices } from '../build_services'; import { getSortForEmbeddable, SortPair } from '../utils/sorting'; +import { getMaxAllowedSampleSize, getAllowedSampleSize } from '../utils/get_allowed_sample_size'; import { SEARCH_EMBEDDABLE_TYPE, SEARCH_EMBEDDABLE_CELL_ACTIONS_TRIGGER_ID } from './constants'; import { SavedSearchEmbeddableComponent } from './saved_search_embeddable_component'; import { handleSourceColumnState } from '../utils/state_helpers'; @@ -127,7 +127,7 @@ export class SavedSearchEmbeddable private prevQuery?: Query; private prevSort?: SortOrder[]; private prevSearchSessionId?: string; - private prevSampleSize?: number; + private prevSampleSizeInput?: number; private searchProps?: SearchProps; private initialized?: boolean; private node?: HTMLElement; @@ -258,6 +258,10 @@ export class SavedSearchEmbeddable return isTextBasedQuery(query); }; + private getFetchedSampleSize = (searchProps: SearchProps): number => { + return getAllowedSampleSize(searchProps.sampleSizeState, this.services.uiSettings); + }; + private fetch = async () => { const savedSearch = this.savedSearch; const searchProps = this.searchProps; @@ -278,10 +282,9 @@ export class SavedSearchEmbeddable savedSearch.searchSource, searchProps.dataView, searchProps.sort, - searchProps.sampleSizeState, + this.getFetchedSampleSize(searchProps), useNewFieldsApi, { - sampleSize: this.services.uiSettings.get(SAMPLE_SIZE_SETTING), sortDir: this.services.uiSettings.get(SORT_DEFAULT_ORDER_SETTING), } ); @@ -505,10 +508,7 @@ export class SavedSearchEmbeddable onUpdateRowsPerPage: (rowsPerPage) => { this.updateInput({ rowsPerPage }); }, - sampleSizeState: - this.input.sampleSize || - savedSearch.sampleSize || - this.services.uiSettings.get(SAMPLE_SIZE_SETTING), + sampleSizeState: this.input.sampleSize || savedSearch.sampleSize, onUpdateSampleSize: (sampleSize) => { this.updateInput({ sampleSize }); }, @@ -556,7 +556,7 @@ export class SavedSearchEmbeddable !isEqual(this.prevQuery, this.input.query) || !isEqual(this.prevTimeRange, this.getTimeRange()) || !isEqual(this.prevSort, this.input.sort) || - this.prevSampleSize !== this.input.sampleSize || + this.prevSampleSizeInput !== this.input.sampleSize || this.prevSearchSessionId !== this.input.searchSessionId ); } @@ -600,10 +600,8 @@ export class SavedSearchEmbeddable this.input.rowsPerPage || savedSearch.rowsPerPage || getDefaultRowsPerPage(this.services.uiSettings); - searchProps.sampleSizeState = - this.input.sampleSize || - savedSearch.sampleSize || - this.services.uiSettings.get(SAMPLE_SIZE_SETTING); + searchProps.maxAllowedSampleSize = getMaxAllowedSampleSize(this.services.uiSettings); + searchProps.sampleSizeState = this.input.sampleSize || savedSearch.sampleSize; searchProps.filters = savedSearch.searchSource.getField('filter') as Filter[]; searchProps.savedSearchId = savedSearch.id; @@ -622,7 +620,7 @@ export class SavedSearchEmbeddable this.prevTimeRange = this.getTimeRange(); this.prevSearchSessionId = this.input.searchSessionId; this.prevSort = this.input.sort; - this.prevSampleSize = this.input.sampleSize; + this.prevSampleSizeInput = this.input.sampleSize; this.searchProps = searchProps; await this.fetch(); @@ -708,7 +706,10 @@ export class SavedSearchEmbeddable > - + , diff --git a/src/plugins/discover/public/embeddable/saved_search_embeddable_component.tsx b/src/plugins/discover/public/embeddable/saved_search_embeddable_component.tsx index 6c499a09d4152..43085e3c0902e 100644 --- a/src/plugins/discover/public/embeddable/saved_search_embeddable_component.tsx +++ b/src/plugins/discover/public/embeddable/saved_search_embeddable_component.tsx @@ -16,6 +16,7 @@ import { isTextBasedQuery } from '../application/main/utils/is_text_based_query' import { SearchProps } from './saved_search_embeddable'; interface SavedSearchEmbeddableComponentProps { + fetchedSampleSize: number; searchProps: SearchProps; useLegacyTable: boolean; query?: AggregateQuery | Query; @@ -25,6 +26,7 @@ const DiscoverDocTableEmbeddableMemoized = React.memo(DiscoverDocTableEmbeddable const DiscoverGridEmbeddableMemoized = React.memo(DiscoverGridEmbeddable); export function SavedSearchEmbeddableComponent({ + fetchedSampleSize, searchProps, useLegacyTable, query, @@ -34,6 +36,7 @@ export function SavedSearchEmbeddableComponent({ return ( ); @@ -41,6 +44,7 @@ export function SavedSearchEmbeddableComponent({ return ( { + sampleSizeState: number; // a required prop totalHitCount?: number; query?: AggregateQuery | Query; interceptedWarnings?: SearchResponseInterceptedWarning[]; diff --git a/src/plugins/discover/public/embeddable/utils/update_search_source.test.ts b/src/plugins/discover/public/embeddable/utils/update_search_source.test.ts index 4ea8f3e27632c..0b56ea8397728 100644 --- a/src/plugins/discover/public/embeddable/utils/update_search_source.test.ts +++ b/src/plugins/discover/public/embeddable/utils/update_search_source.test.ts @@ -22,7 +22,6 @@ const dataViewMockWithTimeField = buildDataViewMock({ describe('updateSearchSource', () => { const defaults = { - sampleSize: 50, sortDir: 'asc', }; @@ -59,15 +58,16 @@ describe('updateSearchSource', () => { expect(searchSource.getField('size')).toEqual(customSampleSize); }); - it('updates a given search source with a default sample size', async () => { - const searchSource = createSearchSourceMock({}); - updateSearchSource(searchSource, dataViewMock, [] as SortOrder[], undefined, true, defaults); - expect(searchSource.getField('size')).toEqual(defaults.sampleSize); - }); - it('updates a given search source with sort field', async () => { const searchSource1 = createSearchSourceMock({}); - updateSearchSource(searchSource1, dataViewMock, [] as SortOrder[], undefined, true, defaults); + updateSearchSource( + searchSource1, + dataViewMock, + [] as SortOrder[], + customSampleSize, + true, + defaults + ); expect(searchSource1.getField('sort')).toEqual([{ _score: 'asc' }]); const searchSource2 = createSearchSourceMock({}); @@ -75,10 +75,9 @@ describe('updateSearchSource', () => { searchSource2, dataViewMockWithTimeField, [] as SortOrder[], - undefined, + customSampleSize, true, { - sampleSize: 50, sortDir: 'desc', } ); @@ -89,7 +88,7 @@ describe('updateSearchSource', () => { searchSource3, dataViewMockWithTimeField, [['bytes', 'desc']] as SortOrder[], - undefined, + customSampleSize, true, defaults ); diff --git a/src/plugins/discover/public/embeddable/utils/update_search_source.ts b/src/plugins/discover/public/embeddable/utils/update_search_source.ts index 7420b316f753e..ce2e72664e7d5 100644 --- a/src/plugins/discover/public/embeddable/utils/update_search_source.ts +++ b/src/plugins/discover/public/embeddable/utils/update_search_source.ts @@ -14,15 +14,14 @@ export const updateSearchSource = ( searchSource: ISearchSource, dataView: DataView | undefined, sort: (SortOrder[] & string[][]) | undefined, - sampleSize: number | undefined, + sampleSize: number, useNewFieldsApi: boolean, defaults: { - sampleSize: number; sortDir: string; } ) => { - const { sampleSize: defaultSampleSize, sortDir } = defaults; - searchSource.setField('size', sampleSize || defaultSampleSize); + const { sortDir } = defaults; + searchSource.setField('size', sampleSize); searchSource.setField( 'sort', getSortForSearchSource({ diff --git a/src/plugins/discover/public/utils/get_allowed_sample_size.ts b/src/plugins/discover/public/utils/get_allowed_sample_size.ts new file mode 100644 index 0000000000000..f805fb662395d --- /dev/null +++ b/src/plugins/discover/public/utils/get_allowed_sample_size.ts @@ -0,0 +1,30 @@ +/* + * 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 { SAMPLE_SIZE_SETTING } from '@kbn/discover-utils'; +import { + MIN_SAVED_SEARCH_SAMPLE_SIZE, + MAX_SAVED_SEARCH_SAMPLE_SIZE, +} from '@kbn/saved-search-plugin/common'; + +export const getMaxAllowedSampleSize = (uiSettings: IUiSettingsClient): number => { + return Math.min(uiSettings.get(SAMPLE_SIZE_SETTING), MAX_SAVED_SEARCH_SAMPLE_SIZE); +}; + +export const getAllowedSampleSize = ( + customSampleSize: number | undefined, + uiSettings: IUiSettingsClient +): number => { + if (!customSampleSize) { + return uiSettings.get(SAMPLE_SIZE_SETTING); + } + return Math.max( + Math.min(customSampleSize, getMaxAllowedSampleSize(uiSettings)), + MIN_SAVED_SEARCH_SAMPLE_SIZE + ); +}; diff --git a/src/plugins/saved_search/common/constants.ts b/src/plugins/saved_search/common/constants.ts index 57e3cfff51ebb..a980bd40e3e26 100644 --- a/src/plugins/saved_search/common/constants.ts +++ b/src/plugins/saved_search/common/constants.ts @@ -10,4 +10,7 @@ export const SavedSearchType = 'search'; export const LATEST_VERSION = 1; +export const MIN_SAVED_SEARCH_SAMPLE_SIZE = 1; +export const MAX_SAVED_SEARCH_SAMPLE_SIZE = 10000; + export type SavedSearchContentType = typeof SavedSearchType; diff --git a/src/plugins/saved_search/common/content_management/v1/cm_services.ts b/src/plugins/saved_search/common/content_management/v1/cm_services.ts index b885e51dbb04e..0cbbe69c4bfeb 100644 --- a/src/plugins/saved_search/common/content_management/v1/cm_services.ts +++ b/src/plugins/saved_search/common/content_management/v1/cm_services.ts @@ -15,6 +15,7 @@ import { updateOptionsSchema, createResultSchema, } from '@kbn/content-management-utils'; +import { MIN_SAVED_SEARCH_SAMPLE_SIZE, MAX_SAVED_SEARCH_SAMPLE_SIZE } from '../../constants'; const sortSchema = schema.arrayOf(schema.string(), { maxSize: 2 }); @@ -60,7 +61,12 @@ const savedSearchAttributesSchema = schema.object( }) ), rowsPerPage: schema.maybe(schema.number()), - sampleSize: schema.maybe(schema.number()), + sampleSize: schema.maybe( + schema.number({ + min: MIN_SAVED_SEARCH_SAMPLE_SIZE, + max: MAX_SAVED_SEARCH_SAMPLE_SIZE, + }) + ), breakdownField: schema.maybe(schema.string()), version: schema.maybe(schema.number()), }, diff --git a/src/plugins/saved_search/common/index.ts b/src/plugins/saved_search/common/index.ts index 4669ecd3bd4b9..0ac92232fb3b8 100644 --- a/src/plugins/saved_search/common/index.ts +++ b/src/plugins/saved_search/common/index.ts @@ -21,5 +21,10 @@ export enum VIEW_MODE { AGGREGATED_LEVEL = 'aggregated', } -export { SavedSearchType, LATEST_VERSION } from './constants'; +export { + SavedSearchType, + LATEST_VERSION, + MIN_SAVED_SEARCH_SAMPLE_SIZE, + MAX_SAVED_SEARCH_SAMPLE_SIZE, +} from './constants'; export { getKibanaContextFn } from './expressions/kibana_context'; diff --git a/src/plugins/saved_search/server/saved_objects/schema.ts b/src/plugins/saved_search/server/saved_objects/schema.ts index 606fb643d3f30..19dfdf5e7a11c 100644 --- a/src/plugins/saved_search/server/saved_objects/schema.ts +++ b/src/plugins/saved_search/server/saved_objects/schema.ts @@ -7,7 +7,11 @@ */ import { schema } from '@kbn/config-schema'; -import { VIEW_MODE } from '../../common'; +import { + MIN_SAVED_SEARCH_SAMPLE_SIZE, + MAX_SAVED_SEARCH_SAMPLE_SIZE, + VIEW_MODE, +} from '../../common'; const SCHEMA_SEARCH_BASE = { // General @@ -82,5 +86,10 @@ const SCHEMA_SEARCH_BASE = { export const SCHEMA_SEARCH_V8_8_0 = schema.object(SCHEMA_SEARCH_BASE); export const SCHEMA_SEARCH_V8_12_0 = schema.object({ ...SCHEMA_SEARCH_BASE, - sampleSize: schema.maybe(schema.number()), + sampleSize: schema.maybe( + schema.number({ + min: MIN_SAVED_SEARCH_SAMPLE_SIZE, + max: MAX_SAVED_SEARCH_SAMPLE_SIZE, + }) + ), }); From b398fdd762bddb67c501f756e0cca69af6e60fad Mon Sep 17 00:00:00 2001 From: Julia Rechkunova Date: Fri, 13 Oct 2023 15:22:42 +0200 Subject: [PATCH 31/32] [Discover] Add tests --- .../saved_search_embeddable.test.ts | 6 +++ .../utils/get_allowed_sample_size.test.ts | 49 +++++++++++++++++++ .../public/utils/get_allowed_sample_size.ts | 4 +- .../discover/group2/_data_grid_sample_size.ts | 4 +- 4 files changed, 59 insertions(+), 4 deletions(-) create mode 100644 src/plugins/discover/public/utils/get_allowed_sample_size.test.ts diff --git a/src/plugins/discover/public/embeddable/saved_search_embeddable.test.ts b/src/plugins/discover/public/embeddable/saved_search_embeddable.test.ts index 1473d07ba72b6..eaa7680137fe3 100644 --- a/src/plugins/discover/public/embeddable/saved_search_embeddable.test.ts +++ b/src/plugins/discover/public/embeddable/saved_search_embeddable.test.ts @@ -118,6 +118,7 @@ describe('saved search embeddable', () => { columns: ['message', 'extension'], rowHeight: 30, rowsPerPage: 50, + sampleSize: 250, }; const searchInput: SearchInput = byValue ? { ...baseInput, attributes: {} as SavedSearchByValueAttributes } @@ -194,6 +195,11 @@ describe('saved search embeddable', () => { await waitOneTick(); expect(searchProps.rowsPerPageState).toEqual(100); + expect(searchProps.sampleSizeState).toEqual(250); + searchProps.onUpdateSampleSize!(300); + await waitOneTick(); + expect(searchProps.sampleSizeState).toEqual(300); + searchProps.onFilter!({ name: 'customer_id', type: 'string', scripted: false }, [17], '+'); await waitOneTick(); expect(executeTriggerActions).toHaveBeenCalled(); diff --git a/src/plugins/discover/public/utils/get_allowed_sample_size.test.ts b/src/plugins/discover/public/utils/get_allowed_sample_size.test.ts new file mode 100644 index 0000000000000..e7431dab6d478 --- /dev/null +++ b/src/plugins/discover/public/utils/get_allowed_sample_size.test.ts @@ -0,0 +1,49 @@ +/* + * 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 { SAMPLE_SIZE_SETTING } from '@kbn/discover-utils'; +import { getAllowedSampleSize, getMaxAllowedSampleSize } from './get_allowed_sample_size'; +import { IUiSettingsClient } from '@kbn/core-ui-settings-browser'; + +describe('allowed sample size', () => { + function getUiSettingsMock(sampleSize?: number): IUiSettingsClient { + return { + get: (key: string) => { + if (key === SAMPLE_SIZE_SETTING) { + return sampleSize; + } + }, + } as IUiSettingsClient; + } + + const uiSettings = getUiSettingsMock(500); + + describe('getAllowedSampleSize', function () { + test('should work correctly for a valid input', function () { + expect(getAllowedSampleSize(1, uiSettings)).toBe(1); + expect(getAllowedSampleSize(100, uiSettings)).toBe(100); + expect(getAllowedSampleSize(500, uiSettings)).toBe(500); + }); + + test('should work correctly for an invalid input', function () { + expect(getAllowedSampleSize(-10, uiSettings)).toBe(500); + expect(getAllowedSampleSize(undefined, uiSettings)).toBe(500); + expect(getAllowedSampleSize(50_000, uiSettings)).toBe(500); + }); + }); + + describe('getMaxAllowedSampleSize', function () { + test('should work correctly', function () { + expect(getMaxAllowedSampleSize(uiSettings)).toBe(500); + expect(getMaxAllowedSampleSize(getUiSettingsMock(1000))).toBe(1000); + expect(getMaxAllowedSampleSize(getUiSettingsMock(100))).toBe(100); + expect(getMaxAllowedSampleSize(getUiSettingsMock(20_000))).toBe(10_000); + expect(getMaxAllowedSampleSize(getUiSettingsMock(undefined))).toBe(500); + }); + }); +}); diff --git a/src/plugins/discover/public/utils/get_allowed_sample_size.ts b/src/plugins/discover/public/utils/get_allowed_sample_size.ts index f805fb662395d..588a33545e2a7 100644 --- a/src/plugins/discover/public/utils/get_allowed_sample_size.ts +++ b/src/plugins/discover/public/utils/get_allowed_sample_size.ts @@ -13,14 +13,14 @@ import { } from '@kbn/saved-search-plugin/common'; export const getMaxAllowedSampleSize = (uiSettings: IUiSettingsClient): number => { - return Math.min(uiSettings.get(SAMPLE_SIZE_SETTING), MAX_SAVED_SEARCH_SAMPLE_SIZE); + return Math.min(uiSettings.get(SAMPLE_SIZE_SETTING) || 500, MAX_SAVED_SEARCH_SAMPLE_SIZE); }; export const getAllowedSampleSize = ( customSampleSize: number | undefined, uiSettings: IUiSettingsClient ): number => { - if (!customSampleSize) { + if (!customSampleSize || customSampleSize < 0) { return uiSettings.get(SAMPLE_SIZE_SETTING); } return Math.max( diff --git a/test/functional/apps/discover/group2/_data_grid_sample_size.ts b/test/functional/apps/discover/group2/_data_grid_sample_size.ts index 7f133905d3de5..891363f0868db 100644 --- a/test/functional/apps/discover/group2/_data_grid_sample_size.ts +++ b/test/functional/apps/discover/group2/_data_grid_sample_size.ts @@ -10,8 +10,8 @@ import expect from '@kbn/expect'; import { FtrProviderContext } from '../ftr_provider_context'; const DEFAULT_ROWS_PER_PAGE = 100; -const DEFAULT_SAMPLE_SIZE = 300; -const CUSTOM_SAMPLE_SIZE = 750; +const DEFAULT_SAMPLE_SIZE = 500; +const CUSTOM_SAMPLE_SIZE = 250; const CUSTOM_SAMPLE_SIZE_FOR_SAVED_SEARCH = 150; const CUSTOM_SAMPLE_SIZE_FOR_DASHBOARD_PANEL = 10; const FOOTER_SELECTOR = 'unifiedDataTableFooter'; From d6c17ad0e52d43323a2ce695fc20b41f6c2de8eb Mon Sep 17 00:00:00 2001 From: Julia Rechkunova Date: Fri, 13 Oct 2023 15:50:30 +0200 Subject: [PATCH 32/32] [Discover] Fix tests --- .../data_table_additional_display_settings.test.tsx | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.test.tsx b/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.test.tsx index b6effab9d9e9e..79e887c8c670d 100644 --- a/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.test.tsx +++ b/packages/kbn-unified-data-table/src/components/data_table_additional_display_settings.test.tsx @@ -10,10 +10,7 @@ import React from 'react'; import { mountWithIntl } from '@kbn/test-jest-helpers'; import { act } from 'react-dom/test-utils'; import { findTestSubject } from '@elastic/eui/lib/test'; -import { - UnifiedDataTableAdditionalDisplaySettings, - MAX_ALLOWED_SAMPLE_SIZE, -} from './data_table_additional_display_settings'; +import { UnifiedDataTableAdditionalDisplaySettings } from './data_table_additional_display_settings'; import lodash from 'lodash'; jest.spyOn(lodash, 'debounce').mockImplementation((fn: any) => fn); @@ -51,11 +48,12 @@ describe('UnifiedDataTableAdditionalDisplaySettings', function () { }); it('should not execute the callback for an invalid input', async () => { - const invalidValue = MAX_ALLOWED_SAMPLE_SIZE + 10; + const invalidValue = 600; const onChangeSampleSizeMock = jest.fn(); const component = mountWithIntl(