From 11dec592014598e83d35988c4ef79f720639eacd Mon Sep 17 00:00:00 2001 From: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Date: Tue, 22 Jun 2021 14:30:31 -0400 Subject: [PATCH] [Discover][Main] Improve state related code (#102028) (#102959) Co-authored-by: Matthias Wilhelm --- .../layout/discover_layout.test.tsx | 4 +- .../components/layout/discover_layout.tsx | 91 ++------- .../apps/main/components/layout/types.ts | 8 +- .../components/sidebar/discover_sidebar.tsx | 22 +- .../discover_sidebar_responsive.test.tsx | 11 - .../sidebar/discover_sidebar_responsive.tsx | 9 - .../sidebar/lib/group_fields.test.ts | 6 +- .../components/sidebar/lib/group_fields.tsx | 4 +- .../apps/main/discover_main_app.tsx | 46 +---- .../apps/main/services/discover_state.ts | 68 ++++++- .../main/services/use_discover_state.test.ts | 4 - .../apps/main/services/use_discover_state.ts | 188 ++++++++++++------ .../main/services/use_saved_search.test.ts | 52 ++++- .../apps/main/services/use_saved_search.ts | 135 +++++-------- 14 files changed, 323 insertions(+), 325 deletions(-) diff --git a/src/plugins/discover/public/application/apps/main/components/layout/discover_layout.test.tsx b/src/plugins/discover/public/application/apps/main/components/layout/discover_layout.test.tsx index 2fd394d98281b7..57a9d518f838ef 100644 --- a/src/plugins/discover/public/application/apps/main/components/layout/discover_layout.test.tsx +++ b/src/plugins/discover/public/application/apps/main/components/layout/discover_layout.test.tsx @@ -18,7 +18,6 @@ import { createSearchSourceMock } from '../../../../../../../data/common/search/ import { IndexPattern, IndexPatternAttributes } from '../../../../../../../data/common'; import { SavedObject } from '../../../../../../../../core/types'; import { indexPatternWithTimefieldMock } from '../../../../../__mocks__/index_pattern_with_timefield'; -import { DiscoverSearchSessionManager } from '../../services/discover_search_session'; import { GetStateReturn } from '../../services/discover_state'; import { DiscoverLayoutProps } from './types'; import { SavedSearchDataSubject } from '../../services/use_saved_search'; @@ -50,11 +49,12 @@ function getProps(indexPattern: IndexPattern): DiscoverLayoutProps { indexPattern, indexPatternList, navigateTo: jest.fn(), + onChangeIndexPattern: jest.fn(), + onUpdateQuery: jest.fn(), resetQuery: jest.fn(), savedSearch: savedSearchMock, savedSearchData$: savedSearch$, savedSearchRefetch$: new Subject(), - searchSessionManager: {} as DiscoverSearchSessionManager, searchSource: searchSourceMock, services, state: { columns: [] }, diff --git a/src/plugins/discover/public/application/apps/main/components/layout/discover_layout.tsx b/src/plugins/discover/public/application/apps/main/components/layout/discover_layout.tsx index 0430614d413b6b..a10674323e5cbf 100644 --- a/src/plugins/discover/public/application/apps/main/components/layout/discover_layout.tsx +++ b/src/plugins/discover/public/application/apps/main/components/layout/discover_layout.tsx @@ -36,10 +36,8 @@ import { SortPairArr } from '../../../../angular/doc_table/lib/get_sort'; import { DOC_HIDE_TIME_COLUMN_SETTING, DOC_TABLE_LEGACY, - MODIFY_COLUMNS_ON_SWITCH, SAMPLE_SIZE_SETTING, SEARCH_FIELDS_FROM_SOURCE, - SORT_DEFAULT_ORDER_SETTING, } from '../../../../../../common'; import { popularizeField } from '../../../../helpers/popularize_field'; import { DocViewFilterFn } from '../../../../doc_views/doc_views_types'; @@ -52,7 +50,6 @@ import { InspectorSession } from '../../../../../../../inspector/public'; import { DiscoverUninitialized } from '../uninitialized/uninitialized'; import { SavedSearchDataMessage } from '../../services/use_saved_search'; import { useDataGridColumns } from '../../../../helpers/use_data_grid_columns'; -import { getSwitchIndexPatternAppState } from '../../utils/get_switch_index_pattern_app_state'; import { FetchStatus } from '../../../../types'; const DocTableLegacyMemoized = React.memo(DocTableLegacy); @@ -72,26 +69,20 @@ export function DiscoverLayout({ indexPattern, indexPatternList, navigateTo, + onChangeIndexPattern, + onUpdateQuery, savedSearchRefetch$, resetQuery, savedSearchData$, savedSearch, - searchSessionManager, searchSource, services, state, stateContainer, }: DiscoverLayoutProps) { - const { - trackUiMetric, - capabilities, - indexPatterns, - data, - uiSettings: config, - filterManager, - } = services; + const { trackUiMetric, capabilities, indexPatterns, data, uiSettings, filterManager } = services; - const sampleSize = useMemo(() => config.get(SAMPLE_SIZE_SETTING), [config]); + const sampleSize = useMemo(() => uiSettings.get(SAMPLE_SIZE_SETTING), [uiSettings]); const [expandedDoc, setExpandedDoc] = useState(undefined); const [inspectorSession, setInspectorSession] = useState(undefined); const scrollableDesktop = useRef(null); @@ -121,42 +112,21 @@ export function DiscoverLayout({ }; }, [savedSearchData$, fetchState]); - const isMobile = () => { - // collapse icon isn't displayed in mobile view, use it to detect which view is displayed - return collapseIcon && !collapseIcon.current; - }; + // collapse icon isn't displayed in mobile view, use it to detect which view is displayed + const isMobile = () => collapseIcon && !collapseIcon.current; const timeField = useMemo(() => { return indexPatternsUtils.isDefault(indexPattern) ? indexPattern.timeFieldName : undefined; }, [indexPattern]); const [isSidebarClosed, setIsSidebarClosed] = useState(false); - const isLegacy = useMemo(() => services.uiSettings.get(DOC_TABLE_LEGACY), [services]); - const useNewFieldsApi = useMemo(() => !services.uiSettings.get(SEARCH_FIELDS_FROM_SOURCE), [ - services, - ]); - - const unmappedFieldsConfig = useMemo( - () => ({ - showUnmappedFields: useNewFieldsApi, - }), - [useNewFieldsApi] - ); + const isLegacy = useMemo(() => uiSettings.get(DOC_TABLE_LEGACY), [uiSettings]); + const useNewFieldsApi = useMemo(() => !uiSettings.get(SEARCH_FIELDS_FROM_SOURCE), [uiSettings]); const resultState = useMemo(() => getResultState(fetchStatus, rows!), [fetchStatus, rows]); - const updateQuery = useCallback( - (_payload, isUpdate?: boolean) => { - if (isUpdate === false) { - searchSessionManager.removeSearchSessionIdFromURL({ replace: false }); - savedSearchRefetch$.next(); - } - }, - [savedSearchRefetch$, searchSessionManager] - ); - const { columns, onAddColumn, onRemoveColumn, onMoveColumn, onSetColumns } = useDataGridColumns({ capabilities, - config, + config: uiSettings, indexPattern, indexPatterns, setAppState: stateContainer.setAppState, @@ -243,42 +213,8 @@ export function DiscoverLayout({ const contentCentered = resultState === 'uninitialized'; const showTimeCol = useMemo( - () => !config.get(DOC_HIDE_TIME_COLUMN_SETTING, false) && !!indexPattern.timeFieldName, - [config, indexPattern.timeFieldName] - ); - - const onChangeIndexPattern = useCallback( - async (id: string) => { - const nextIndexPattern = await indexPatterns.get(id); - if (nextIndexPattern && indexPattern) { - /** - * Without resetting the fetch state, e.g. a time column would be displayed when switching - * from a index pattern without to a index pattern with time filter for a brief moment - * That's because appState is updated before savedSearchData$ - * The following line of code catches this, but should be improved - */ - savedSearchData$.next({ rows: [], state: FetchStatus.LOADING, fieldCounts: {} }); - - const nextAppState = getSwitchIndexPatternAppState( - indexPattern, - nextIndexPattern, - state.columns || [], - (state.sort || []) as SortPairArr[], - config.get(MODIFY_COLUMNS_ON_SWITCH), - config.get(SORT_DEFAULT_ORDER_SETTING) - ); - stateContainer.setAppState(nextAppState); - } - }, - [ - config, - indexPattern, - indexPatterns, - savedSearchData$, - state.columns, - state.sort, - stateContainer, - ] + () => !uiSettings.get(DOC_HIDE_TIME_COLUMN_SETTING, false) && !!indexPattern.timeFieldName, + [uiSettings, indexPattern.timeFieldName] ); return ( @@ -294,7 +230,7 @@ export function DiscoverLayout({ searchSource={searchSource} services={services} stateContainer={stateContainer} - updateQuery={updateQuery} + updateQuery={onUpdateQuery} />

@@ -316,7 +252,6 @@ export function DiscoverLayout({ state={state} isClosed={isSidebarClosed} trackUiMetric={trackUiMetric} - unmappedFieldsConfig={unmappedFieldsConfig} useNewFieldsApi={useNewFieldsApi} onEditRuntimeField={onEditRuntimeField} /> @@ -373,7 +308,7 @@ export function DiscoverLayout({ > >; - resetQuery: () => void; navigateTo: (url: string) => void; + onChangeIndexPattern: (id: string) => void; + onUpdateQuery: (payload: { dateRange: TimeRange; query?: Query }, isUpdate?: boolean) => void; + resetQuery: () => void; savedSearch: SavedSearch; savedSearchData$: SavedSearchDataSubject; savedSearchRefetch$: SavedSearchRefetchSubject; - searchSessionManager: DiscoverSearchSessionManager; searchSource: ISearchSource; services: DiscoverServices; state: AppState; diff --git a/src/plugins/discover/public/application/apps/main/components/sidebar/discover_sidebar.tsx b/src/plugins/discover/public/application/apps/main/components/sidebar/discover_sidebar.tsx index 7fbbf6fd3ffdc7..7f8866a2ee369d 100644 --- a/src/plugins/discover/public/application/apps/main/components/sidebar/discover_sidebar.tsx +++ b/src/plugins/discover/public/application/apps/main/components/sidebar/discover_sidebar.tsx @@ -82,7 +82,6 @@ export function DiscoverSidebar({ trackUiMetric, useNewFieldsApi = false, useFlyout = false, - unmappedFieldsConfig, onEditRuntimeField, onChangeIndexPattern, setFieldEditorRef, @@ -129,25 +128,8 @@ export function DiscoverSidebar({ popular: popularFields, unpopular: unpopularFields, } = useMemo( - () => - groupFields( - fields, - columns, - popularLimit, - fieldCounts, - fieldFilter, - useNewFieldsApi, - !!unmappedFieldsConfig?.showUnmappedFields - ), - [ - fields, - columns, - popularLimit, - fieldCounts, - fieldFilter, - useNewFieldsApi, - unmappedFieldsConfig?.showUnmappedFields, - ] + () => groupFields(fields, columns, popularLimit, fieldCounts, fieldFilter, useNewFieldsApi), + [fields, columns, popularLimit, fieldCounts, fieldFilter, useNewFieldsApi] ); const paginate = useCallback(() => { diff --git a/src/plugins/discover/public/application/apps/main/components/sidebar/discover_sidebar_responsive.test.tsx b/src/plugins/discover/public/application/apps/main/components/sidebar/discover_sidebar_responsive.test.tsx index 2ad75806173eb0..6973221fd36248 100644 --- a/src/plugins/discover/public/application/apps/main/components/sidebar/discover_sidebar_responsive.test.tsx +++ b/src/plugins/discover/public/application/apps/main/components/sidebar/discover_sidebar_responsive.test.tsx @@ -25,7 +25,6 @@ import { } from './discover_sidebar_responsive'; import { DiscoverServices } from '../../../../../build_services'; import { ElasticSearchHit } from '../../../../doc_views/doc_views_types'; -import { DiscoverSidebar } from './discover_sidebar'; const mockServices = ({ history: () => ({ @@ -132,14 +131,4 @@ describe('discover responsive sidebar', function () { findTestSubject(comp, 'plus-extension-gif').simulate('click'); expect(props.onAddFilter).toHaveBeenCalled(); }); - it('renders sidebar with unmapped fields config', function () { - const unmappedFieldsConfig = { - showUnmappedFields: false, - }; - const componentProps = { ...props, unmappedFieldsConfig }; - const component = mountWithIntl(); - const discoverSidebar = component.find(DiscoverSidebar); - expect(discoverSidebar).toHaveLength(1); - expect(discoverSidebar.props().unmappedFieldsConfig).toEqual(unmappedFieldsConfig); - }); }); diff --git a/src/plugins/discover/public/application/apps/main/components/sidebar/discover_sidebar_responsive.tsx b/src/plugins/discover/public/application/apps/main/components/sidebar/discover_sidebar_responsive.tsx index cc33601f77728f..003bb22599e480 100644 --- a/src/plugins/discover/public/application/apps/main/components/sidebar/discover_sidebar_responsive.tsx +++ b/src/plugins/discover/public/application/apps/main/components/sidebar/discover_sidebar_responsive.tsx @@ -105,15 +105,6 @@ export interface DiscoverSidebarResponsiveProps { * Read from the Fields API */ useNewFieldsApi?: boolean; - /** - * an object containing properties for proper handling of unmapped fields - */ - unmappedFieldsConfig?: { - /** - * determines whether to display unmapped fields - */ - showUnmappedFields: boolean; - }; /** * callback to execute on edit runtime field */ diff --git a/src/plugins/discover/public/application/apps/main/components/sidebar/lib/group_fields.test.ts b/src/plugins/discover/public/application/apps/main/components/sidebar/lib/group_fields.test.ts index 58697206356214..cd9f6b3cac4a51 100644 --- a/src/plugins/discover/public/application/apps/main/components/sidebar/lib/group_fields.test.ts +++ b/src/plugins/discover/public/application/apps/main/components/sidebar/lib/group_fields.test.ts @@ -244,8 +244,7 @@ describe('group_fields', function () { 5, fieldCounts, fieldFilterState, - true, - false + true ); expect(actual.unpopular).toEqual([]); }); @@ -270,8 +269,7 @@ describe('group_fields', function () { 5, fieldCounts, fieldFilterState, - false, - undefined + false ); expect(actual.unpopular.map((field) => field.name)).toEqual(['unknown_field']); }); diff --git a/src/plugins/discover/public/application/apps/main/components/sidebar/lib/group_fields.tsx b/src/plugins/discover/public/application/apps/main/components/sidebar/lib/group_fields.tsx index dc6cbcedc80864..2007d32fe84bee 100644 --- a/src/plugins/discover/public/application/apps/main/components/sidebar/lib/group_fields.tsx +++ b/src/plugins/discover/public/application/apps/main/components/sidebar/lib/group_fields.tsx @@ -24,9 +24,9 @@ export function groupFields( popularLimit: number, fieldCounts: Record, fieldFilterState: FieldFilterState, - useNewFieldsApi: boolean, - showUnmappedFields = true + useNewFieldsApi: boolean ): GroupedFields { + const showUnmappedFields = useNewFieldsApi; const result: GroupedFields = { selected: [], popular: [], diff --git a/src/plugins/discover/public/application/apps/main/discover_main_app.tsx b/src/plugins/discover/public/application/apps/main/discover_main_app.tsx index 5cc7147b49ff98..07939fff6e7f48 100644 --- a/src/plugins/discover/public/application/apps/main/discover_main_app.tsx +++ b/src/plugins/discover/public/application/apps/main/discover_main_app.tsx @@ -5,15 +5,12 @@ * in compliance with, at your election, the Elastic License 2.0 or the Server * Side Public License, v 1. */ -import React, { useMemo, useCallback, useEffect } from 'react'; +import React, { useCallback, useEffect } from 'react'; import { History } from 'history'; import { DiscoverLayout } from './components/layout'; -import { SEARCH_FIELDS_FROM_SOURCE } from '../../../../common'; -import { useSavedSearch as useSavedSearchData } from './services/use_saved_search'; import { setBreadcrumbsTitle } from '../../helpers/breadcrumbs'; import { addHelpMenuToAppChrome } from '../../components/help_menu/help_menu_util'; import { useDiscoverState } from './services/use_discover_state'; -import { useSearchSession } from './services/use_search_session'; import { useUrl } from './services/use_url'; import { IndexPattern, IndexPatternAttributes, SavedObject } from '../../../../../data/common'; import { DiscoverServices } from '../../../build_services'; @@ -55,18 +52,20 @@ export function DiscoverMainApp(props: DiscoverMainProps) { const { services, history, navigateTo, indexPatternList } = props.opts; const { chrome, docLinks, uiSettings: config, data } = services; - const useNewFieldsApi = useMemo(() => !config.get(SEARCH_FIELDS_FROM_SOURCE), [config]); - /** * State related logic */ const { - stateContainer, - state, + data$, indexPattern, - searchSource, - savedSearch, + onChangeIndexPattern, + onUpdateQuery, + refetch$, resetSavedSearch, + savedSearch, + searchSource, + state, + stateContainer, } = useDiscoverState({ services, history, @@ -79,25 +78,6 @@ export function DiscoverMainApp(props: DiscoverMainProps) { */ useUrl({ history, resetSavedSearch }); - /** - * Search session logic - */ - const searchSessionManager = useSearchSession({ services, history, stateContainer, savedSearch }); - - /** - * Data fetching logic - */ - const { data$, refetch$ } = useSavedSearchData({ - indexPattern, - savedSearch, - searchSessionManager, - searchSource, - services, - state, - stateContainer, - useNewFieldsApi, - }); - /** * SavedSearch depended initializing */ @@ -115,11 +95,6 @@ export function DiscoverMainApp(props: DiscoverMainProps) { */ useEffect(() => { addHelpMenuToAppChrome(chrome, docLinks); - stateContainer.replaceUrlAppState({}).then(() => { - stateContainer.startSync(); - }); - - return () => stateContainer.stopSync(); }, [stateContainer, chrome, docLinks]); const resetQuery = useCallback(() => { @@ -130,12 +105,13 @@ export function DiscoverMainApp(props: DiscoverMainProps) { ; + /** + * Function starting state sync when Discover main is loaded + */ + initializeAndSync: ( + indexPattern: IndexPattern, + filterManager: FilterManager, + data: DataPublicPluginStart + ) => () => void; /** * Start sync between state and URL */ @@ -204,16 +216,18 @@ export function getState({ stateStorage, }); + const replaceUrlAppState = async (newPartial: AppState = {}) => { + const state = { ...appStateContainer.getState(), ...newPartial }; + await stateStorage.set(APP_STATE_URL_KEY, state, { replace: true }); + }; + return { kbnUrlStateStorage: stateStorage, appStateContainer: appStateContainerModified, startSync: start, stopSync: stop, setAppState: (newPartial: AppState) => setState(appStateContainerModified, newPartial), - replaceUrlAppState: async (newPartial: AppState = {}) => { - const state = { ...appStateContainer.getState(), ...newPartial }; - await stateStorage.set(APP_STATE_URL_KEY, state, { replace: true }); - }, + replaceUrlAppState, resetInitialAppState: () => { initialAppState = appStateContainer.getState(); }, @@ -224,6 +238,50 @@ export function getState({ getPreviousAppState: () => previousAppState, flushToUrl: () => stateStorage.kbnUrlControls.flush(), isAppStateDirty: () => !isEqualState(initialAppState, appStateContainer.getState()), + initializeAndSync: ( + indexPattern: IndexPattern, + filterManager: FilterManager, + data: DataPublicPluginStart + ) => { + if (appStateContainer.getState().index !== indexPattern.id) { + // used index pattern is different than the given by url/state which is invalid + setState(appStateContainerModified, { index: indexPattern.id }); + } + // sync initial app filters from state to filterManager + const filters = appStateContainer.getState().filters; + if (filters) { + filterManager.setAppFilters(cloneDeep(filters)); + } + const query = appStateContainer.getState().query; + if (query) { + data.query.queryString.setQuery(query); + } + + const stopSyncingQueryAppStateWithStateContainer = connectToQueryState( + data.query, + appStateContainer, + { + filters: esFilters.FilterStateStore.APP_STATE, + query: true, + } + ); + + // syncs `_g` portion of url with query services + const { stop: stopSyncingGlobalStateWithUrl } = syncQueryStateWithUrl( + data.query, + stateStorage + ); + + replaceUrlAppState({}).then(() => { + start(); + }); + + return () => { + stopSyncingQueryAppStateWithStateContainer(); + stopSyncingGlobalStateWithUrl(); + stop(); + }; + }, }; } diff --git a/src/plugins/discover/public/application/apps/main/services/use_discover_state.test.ts b/src/plugins/discover/public/application/apps/main/services/use_discover_state.test.ts index 051a2d2dcd9cc7..4c3d819f063a0d 100644 --- a/src/plugins/discover/public/application/apps/main/services/use_discover_state.test.ts +++ b/src/plugins/discover/public/application/apps/main/services/use_discover_state.test.ts @@ -62,10 +62,6 @@ describe('test useDiscoverState', () => { }); }); - await act(async () => { - result.current.stateContainer.startSync(); - }); - const initialColumns = result.current.state.columns; await act(async () => { result.current.setState({ columns: ['123'] }); diff --git a/src/plugins/discover/public/application/apps/main/services/use_discover_state.ts b/src/plugins/discover/public/application/apps/main/services/use_discover_state.ts index a3546d54cd4932..3c736f09a82967 100644 --- a/src/plugins/discover/public/application/apps/main/services/use_discover_state.ts +++ b/src/plugins/discover/public/application/apps/main/services/use_discover_state.ts @@ -6,19 +6,25 @@ * Side Public License, v 1. */ import { useMemo, useEffect, useState, useCallback } from 'react'; -import { cloneDeep } from 'lodash'; +import { isEqual } from 'lodash'; import { History } from 'history'; import { getState } from './discover_state'; import { getStateDefaults } from '../utils/get_state_defaults'; -import { - esFilters, - connectToQueryState, - syncQueryStateWithUrl, - IndexPattern, -} from '../../../../../../data/public'; +import { IndexPattern } from '../../../../../../data/public'; import { DiscoverServices } from '../../../../build_services'; import { SavedSearch } from '../../../../saved_searches'; import { loadIndexPattern } from '../utils/resolve_index_pattern'; +import { useSavedSearch as useSavedSearchData } from './use_saved_search'; +import { + MODIFY_COLUMNS_ON_SWITCH, + SEARCH_FIELDS_FROM_SOURCE, + SEARCH_ON_PAGE_LOAD_SETTING, + SORT_DEFAULT_ORDER_SETTING, +} from '../../../../../common'; +import { useSearchSession } from './use_search_session'; +import { FetchStatus } from '../../../types'; +import { getSwitchIndexPatternAppState } from '../utils/get_switch_index_pattern_app_state'; +import { SortPairArr } from '../../../angular/doc_table/lib/get_sort'; export function useDiscoverState({ services, @@ -31,9 +37,11 @@ export function useDiscoverState({ history: History; initialIndexPattern: IndexPattern; }) { - const { uiSettings: config, data, filterManager } = services; + const { uiSettings: config, data, filterManager, indexPatterns } = services; const [indexPattern, setIndexPattern] = useState(initialIndexPattern); const [savedSearch, setSavedSearch] = useState(initialSavedSearch); + const useNewFieldsApi = useMemo(() => !config.get(SEARCH_FIELDS_FROM_SOURCE), [config]); + const timefilter = data.query.timefilter.timefilter; const searchSource = useMemo(() => { savedSearch.searchSource.setField('index', indexPattern); @@ -57,73 +65,80 @@ export function useDiscoverState({ [config, data, history, savedSearch, services.core.notifications.toasts] ); - const { appStateContainer, getPreviousAppState } = stateContainer; + const { appStateContainer } = stateContainer; const [state, setState] = useState(appStateContainer.getState()); - useEffect(() => { - if (stateContainer.appStateContainer.getState().index !== indexPattern.id) { - // used index pattern is different than the given by url/state which is invalid - stateContainer.setAppState({ index: indexPattern.id }); - } - // sync initial app filters from state to filterManager - const filters = appStateContainer.getState().filters; - if (filters) { - filterManager.setAppFilters(cloneDeep(filters)); - } - const query = appStateContainer.getState().query; - if (query) { - data.query.queryString.setQuery(query); - } + /** + * Search session logic + */ + const searchSessionManager = useSearchSession({ services, history, stateContainer, savedSearch }); - const stopSyncingQueryAppStateWithStateContainer = connectToQueryState( - data.query, - appStateContainer, - { - filters: esFilters.FilterStateStore.APP_STATE, - query: true, - } - ); + const initialFetchStatus: FetchStatus = useMemo(() => { + // A saved search is created on every page load, so we check the ID to see if we're loading a + // previously saved search or if it is just transient + const shouldSearchOnPageLoad = + config.get(SEARCH_ON_PAGE_LOAD_SETTING) || + savedSearch.id !== undefined || + timefilter.getRefreshInterval().pause === false || + searchSessionManager.hasSearchSessionIdInURL(); + return shouldSearchOnPageLoad ? FetchStatus.LOADING : FetchStatus.UNINITIALIZED; + }, [config, savedSearch.id, searchSessionManager, timefilter]); - // syncs `_g` portion of url with query services - const { stop: stopSyncingGlobalStateWithUrl } = syncQueryStateWithUrl( - data.query, - stateContainer.kbnUrlStateStorage - ); + /** + * Data fetching logic + */ + const { data$, refetch$, reset } = useSavedSearchData({ + indexPattern, + initialFetchStatus, + searchSessionManager, + searchSource, + services, + stateContainer, + useNewFieldsApi, + }); + + useEffect(() => { + const stopSync = stateContainer.initializeAndSync(indexPattern, filterManager, data); return () => { - stopSyncingQueryAppStateWithStateContainer(); - stopSyncingGlobalStateWithUrl(); + stopSync(); }; - }, [ - appStateContainer, - config, - data.query, - data.search.session, - getPreviousAppState, - indexPattern.id, - filterManager, - services.indexPatterns, - stateContainer, - ]); + }, [stateContainer, filterManager, data, indexPattern]); + /** + * Track state changes that should trigger a fetch + */ useEffect(() => { - const unsubscribe = stateContainer.appStateContainer.subscribe(async (nextState) => { + const unsubscribe = appStateContainer.subscribe(async (nextState) => { + const { hideChart, interval, sort, index } = state; + // chart was hidden, now it should be displayed, so data is needed + const chartDisplayChanged = nextState.hideChart !== hideChart && hideChart; + const chartIntervalChanged = nextState.interval !== interval; + const docTableSortChanged = !isEqual(nextState.sort, sort); + const indexPatternChanged = !isEqual(nextState.index, index); // NOTE: this is also called when navigating from discover app to context app - if (nextState.index && state.index !== nextState.index) { - const nextIndexPattern = await loadIndexPattern( - nextState.index, - services.indexPatterns, - config - ); + if (nextState.index && indexPatternChanged) { + /** + * Without resetting the fetch state, e.g. a time column would be displayed when switching + * from a index pattern without to a index pattern with time filter for a brief moment + * That's because appState is updated before savedSearchData$ + * The following line of code catches this, but should be improved + */ + reset(); + const nextIndexPattern = await loadIndexPattern(nextState.index, indexPatterns, config); if (nextIndexPattern) { setIndexPattern(nextIndexPattern.loaded); } } + + if (chartDisplayChanged || chartIntervalChanged || docTableSortChanged) { + refetch$.next(); + } setState(nextState); }); return () => unsubscribe(); - }, [config, services.indexPatterns, state.index, stateContainer.appStateContainer, setState]); + }, [config, indexPatterns, appStateContainer, setState, state, refetch$, data$, reset]); const resetSavedSearch = useCallback( async (id?: string) => { @@ -143,13 +158,62 @@ export function useDiscoverState({ [services, indexPattern, config, data, stateContainer, savedSearch.id] ); + /** + * Function triggered when user changes index pattern in the sidebar + */ + const onChangeIndexPattern = useCallback( + async (id: string) => { + const nextIndexPattern = await indexPatterns.get(id); + if (nextIndexPattern && indexPattern) { + const nextAppState = getSwitchIndexPatternAppState( + indexPattern, + nextIndexPattern, + state.columns || [], + (state.sort || []) as SortPairArr[], + config.get(MODIFY_COLUMNS_ON_SWITCH), + config.get(SORT_DEFAULT_ORDER_SETTING) + ); + stateContainer.setAppState(nextAppState); + } + }, + [config, indexPattern, indexPatterns, state.columns, state.sort, stateContainer] + ); + /** + * Function triggered when the user changes the query in the search bar + */ + const onUpdateQuery = useCallback( + (_payload, isUpdate?: boolean) => { + if (isUpdate === false) { + searchSessionManager.removeSearchSessionIdFromURL({ replace: false }); + refetch$.next(); + } + }, + [refetch$, searchSessionManager] + ); + + /** + * Initial data fetching, also triggered when index pattern changes + */ + useEffect(() => { + if (!indexPattern) { + return; + } + if (initialFetchStatus === FetchStatus.LOADING) { + refetch$.next(); + } + }, [initialFetchStatus, refetch$, indexPattern, data$]); + return { - state, - setState, - stateContainer, + data$, indexPattern, - searchSource, - savedSearch, + refetch$, resetSavedSearch, + onChangeIndexPattern, + onUpdateQuery, + savedSearch, + searchSource, + setState, + state, + stateContainer, }; } diff --git a/src/plugins/discover/public/application/apps/main/services/use_saved_search.test.ts b/src/plugins/discover/public/application/apps/main/services/use_saved_search.test.ts index 5976c8fea5ea4f..128c94f284f56c 100644 --- a/src/plugins/discover/public/application/apps/main/services/use_saved_search.test.ts +++ b/src/plugins/discover/public/application/apps/main/services/use_saved_search.test.ts @@ -12,9 +12,10 @@ import { discoverServiceMock } from '../../../../__mocks__/services'; import { savedSearchMock } from '../../../../__mocks__/saved_search'; import { indexPatternMock } from '../../../../__mocks__/index_pattern'; import { useSavedSearch } from './use_saved_search'; -import { AppState, getState } from './discover_state'; +import { getState } from './discover_state'; import { uiSettingsMock } from '../../../../__mocks__/ui_settings'; import { useDiscoverState } from './use_discover_state'; +import { FetchStatus } from '../../../types'; describe('test useSavedSearch', () => { test('useSavedSearch return is valid', async () => { @@ -28,11 +29,10 @@ describe('test useSavedSearch', () => { const { result } = renderHook(() => { return useSavedSearch({ indexPattern: indexPatternMock, - savedSearch: savedSearchMock, + initialFetchStatus: FetchStatus.LOADING, searchSessionManager, searchSource: savedSearchMock.searchSource.createCopy(), services: discoverServiceMock, - state: {} as AppState, stateContainer, useNewFieldsApi: true, }); @@ -69,11 +69,10 @@ describe('test useSavedSearch', () => { const { result, waitForValueToChange } = renderHook(() => { return useSavedSearch({ indexPattern: indexPatternMock, - savedSearch: savedSearchMock, + initialFetchStatus: FetchStatus.LOADING, searchSessionManager, searchSource: resultState.current.searchSource, services: discoverServiceMock, - state: {} as AppState, stateContainer, useNewFieldsApi: true, }); @@ -88,4 +87,47 @@ describe('test useSavedSearch', () => { expect(result.current.data$.value.hits).toBe(0); expect(result.current.data$.value.rows).toEqual([]); }); + + test('reset sets back to initial state', async () => { + const { history, searchSessionManager } = createSearchSessionMock(); + const stateContainer = getState({ + getStateDefaults: () => ({ index: 'the-index-pattern-id' }), + history, + uiSettings: uiSettingsMock, + }); + + discoverServiceMock.data.query.timefilter.timefilter.getTime = jest.fn(() => { + return { from: '2021-05-01T20:00:00Z', to: '2021-05-02T20:00:00Z' }; + }); + + const { result: resultState } = renderHook(() => { + return useDiscoverState({ + services: discoverServiceMock, + history, + initialIndexPattern: indexPatternMock, + initialSavedSearch: savedSearchMock, + }); + }); + + const { result, waitForValueToChange } = renderHook(() => { + return useSavedSearch({ + indexPattern: indexPatternMock, + initialFetchStatus: FetchStatus.LOADING, + searchSessionManager, + searchSource: resultState.current.searchSource, + services: discoverServiceMock, + stateContainer, + useNewFieldsApi: true, + }); + }); + + result.current.refetch$.next(); + + await waitForValueToChange(() => { + return result.current.data$.value.state === FetchStatus.COMPLETE; + }); + + result.current.reset(); + expect(result.current.data$.value.state).toBe(FetchStatus.LOADING); + }); }); diff --git a/src/plugins/discover/public/application/apps/main/services/use_saved_search.ts b/src/plugins/discover/public/application/apps/main/services/use_saved_search.ts index 2b0d9517248694..8c847b54078eb7 100644 --- a/src/plugins/discover/public/application/apps/main/services/use_saved_search.ts +++ b/src/plugins/discover/public/application/apps/main/services/use_saved_search.ts @@ -5,11 +5,10 @@ * in compliance with, at your election, the Elastic License 2.0 or the Server * Side Public License, v 1. */ -import { useEffect, useRef, useCallback, useMemo } from 'react'; +import { useEffect, useRef, useCallback } from 'react'; import { i18n } from '@kbn/i18n'; import { merge, Subject, BehaviorSubject } from 'rxjs'; import { debounceTime, tap, filter } from 'rxjs/operators'; -import { isEqual } from 'lodash'; import { DiscoverServices } from '../../../../build_services'; import { DiscoverSearchSessionManager } from './discover_search_session'; import { @@ -18,13 +17,11 @@ import { SearchSource, tabifyAggResponse, } from '../../../../../../data/common'; -import { SavedSearch } from '../../../../saved_searches'; -import { AppState, GetStateReturn } from './discover_state'; +import { GetStateReturn } from './discover_state'; import { ElasticSearchHit } from '../../../doc_views/doc_views_types'; import { RequestAdapter } from '../../../../../../inspector/public'; import { AutoRefreshDoneFn, search } from '../../../../../../data/public'; import { calcFieldCounts } from '../utils/calc_field_counts'; -import { SEARCH_ON_PAGE_LOAD_SETTING } from '../../../../../common'; import { validateTimeRange } from '../utils/validate_time_range'; import { updateSearchSource } from '../utils/update_search_source'; import { SortOrder } from '../../../../saved_searches/types'; @@ -40,6 +37,7 @@ export type SavedSearchRefetchSubject = Subject; export interface UseSavedSearch { refetch$: SavedSearchRefetchSubject; data$: SavedSearchDataSubject; + reset: () => void; } export type SavedSearchRefetchMsg = 'reset' | undefined; @@ -59,48 +57,27 @@ export interface SavedSearchDataMessage { /** * This hook return 2 observables, refetch$ allows to trigger data fetching, data$ to subscribe * to the data fetching - * @param indexPattern - * @param savedSearch - * @param searchSessionManager - * @param searchSource - * @param services - * @param state - * @param stateContainer - * @param useNewFieldsApi */ export const useSavedSearch = ({ indexPattern, - savedSearch, + initialFetchStatus, searchSessionManager, searchSource, services, - state, stateContainer, useNewFieldsApi, }: { indexPattern: IndexPattern; - savedSearch: SavedSearch; + initialFetchStatus: FetchStatus; searchSessionManager: DiscoverSearchSessionManager; searchSource: SearchSource; services: DiscoverServices; - state: AppState; stateContainer: GetStateReturn; useNewFieldsApi: boolean; }): UseSavedSearch => { - const { data, filterManager, uiSettings } = services; + const { data, filterManager } = services; const timefilter = data.query.timefilter.timefilter; - const initFetchState: FetchStatus = useMemo(() => { - // A saved search is created on every page load, so we check the ID to see if we're loading a - // previously saved search or if it is just transient - const shouldSearchOnPageLoad = - uiSettings.get(SEARCH_ON_PAGE_LOAD_SETTING) || - savedSearch.id !== undefined || - timefilter.getRefreshInterval().pause === false || - searchSessionManager.hasSearchSessionIdInURL(); - return shouldSearchOnPageLoad ? FetchStatus.LOADING : FetchStatus.UNINITIALIZED; - }, [uiSettings, savedSearch.id, searchSessionManager, timefilter]); - /** * The observable the UI (aka React component) subscribes to get notified about * the changes in the data fetching process (high level: fetching started, data was received) @@ -108,7 +85,7 @@ export const useSavedSearch = ({ const data$: SavedSearchDataSubject = useSingleton( () => new BehaviorSubject({ - state: initFetchState, + state: initialFetchStatus, }) ); /** @@ -123,15 +100,14 @@ export const useSavedSearch = ({ */ const refs = useRef<{ abortController?: AbortController; - /** - * used to compare a new state against an old one, to evaluate if data needs to be fetched - */ - appState: AppState; /** * handler emitted by `timefilter.getAutoRefreshFetch$()` * to notify when data completed loading and to start a new autorefresh loop */ autoRefreshDoneCb?: AutoRefreshDoneFn; + /** + * Number of fetches used for functional testing + */ fetchCounter: number; /** * needed to right auto refresh behavior, a new auto refresh shouldnt be triggered when @@ -144,12 +120,34 @@ export const useSavedSearch = ({ */ fieldCounts: Record; }>({ - appState: state, fetchCounter: 0, fieldCounts: {}, - fetchStatus: initFetchState, + fetchStatus: initialFetchStatus, }); + /** + * Resets the fieldCounts cache and sends a reset message + * It is set to initial state (no documents, fetchCounter to 0) + * Needed when index pattern is switched or a new runtime field is added + */ + const sendResetMsg = useCallback( + (fetchStatus?: FetchStatus) => { + refs.current.fieldCounts = {}; + refs.current.fetchStatus = fetchStatus ?? initialFetchStatus; + data$.next({ + state: initialFetchStatus, + fetchCounter: 0, + rows: [], + fieldCounts: {}, + chartData: undefined, + bucketInterval: undefined, + }); + }, + [data$, initialFetchStatus] + ); + /** + * Function to fetch data from ElasticSearch + */ const fetchAll = useCallback( (reset = false) => { if (!validateTimeRange(timefilter.getTime(), services.toastNotifications)) { @@ -161,23 +159,18 @@ export const useSavedSearch = ({ refs.current.abortController = new AbortController(); const sessionId = searchSessionManager.getNextSearchSessionId(); - // Let the UI know, data fetching started - const loadingMessage: SavedSearchDataMessage = { - state: FetchStatus.LOADING, - fetchCounter: ++refs.current.fetchCounter, - }; - if (reset) { - // when runtime field was added, changed, deleted, index pattern was switched - loadingMessage.rows = []; - loadingMessage.fieldCounts = {}; - loadingMessage.chartData = undefined; - loadingMessage.bucketInterval = undefined; + sendResetMsg(FetchStatus.LOADING); + } else { + // Let the UI know, data fetching started + data$.next({ + state: FetchStatus.LOADING, + fetchCounter: ++refs.current.fetchCounter, + }); + refs.current.fetchStatus = FetchStatus.LOADING; } - data$.next(loadingMessage); - refs.current.fetchStatus = loadingMessage.state; - const { sort } = stateContainer.appStateContainer.getState(); + const { sort, hideChart, interval } = stateContainer.appStateContainer.getState(); updateSearchSource(searchSource, false, { indexPattern, services, @@ -185,8 +178,8 @@ export const useSavedSearch = ({ useNewFieldsApi, }); const chartAggConfigs = - indexPattern.timeFieldName && !state.hideChart && state.interval - ? getChartAggConfigs(searchSource, state.interval, data) + indexPattern.timeFieldName && !hideChart && interval + ? getChartAggConfigs(searchSource, interval, data) : undefined; if (!chartAggConfigs) { @@ -217,16 +210,12 @@ export const useSavedSearch = ({ state: FetchStatus.COMPLETE, rows: documents, inspectorAdapters, - fieldCounts: calcFieldCounts( - reset ? {} : refs.current.fieldCounts, - documents, - indexPattern - ), + fieldCounts: calcFieldCounts(refs.current.fieldCounts, documents, indexPattern), hits: res.rawResponse.hits.total as number, }; if (chartAggConfigs) { - const bucketAggConfig = chartAggConfigs!.aggs[1]; + const bucketAggConfig = chartAggConfigs.aggs[1]; const tabifiedData = tabifyAggResponse(chartAggConfigs, res.rawResponse); const dimensions = getDimensions(chartAggConfigs, data); if (dimensions) { @@ -259,14 +248,13 @@ export const useSavedSearch = ({ [ timefilter, services, + searchSessionManager, stateContainer.appStateContainer, searchSource, indexPattern, useNewFieldsApi, - state.hideChart, - state.interval, data, - searchSessionManager, + sendResetMsg, data$, ] ); @@ -306,32 +294,9 @@ export const useSavedSearch = ({ fetchAll, ]); - /** - * Track state changes that should trigger a fetch - */ - useEffect(() => { - const prevAppState = refs.current.appState; - - // chart was hidden, now it should be displayed, so data is needed - const chartDisplayChanged = state.hideChart !== prevAppState.hideChart && !state.hideChart; - const chartIntervalChanged = state.interval !== prevAppState.interval; - const docTableSortChanged = !isEqual(state.sort, prevAppState.sort); - const indexPatternChanged = !isEqual(state.index, prevAppState.index); - - refs.current.appState = state; - if (chartDisplayChanged || chartIntervalChanged || docTableSortChanged || indexPatternChanged) { - refetch$.next(indexPatternChanged ? 'reset' : undefined); - } - }, [refetch$, state.interval, state.sort, state]); - - useEffect(() => { - if (initFetchState === FetchStatus.LOADING) { - refetch$.next(); - } - }, [initFetchState, refetch$]); - return { refetch$, data$, + reset: sendResetMsg, }; };