From 5c7c5cd0e2cd5f60ef0df1f7dd9849d675206a8f Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Mon, 19 Feb 2024 20:59:52 +0100 Subject: [PATCH 1/8] Switch Discover to data loading state when data view is switched --- .../main/hooks/utils/build_state_subscribe.ts | 4 ++-- .../main/hooks/utils/change_data_view.test.ts | 11 ++++++++++- .../application/main/hooks/utils/change_data_view.ts | 7 +++++++ .../services/discover_data_state_container.test.ts | 2 +- .../main/services/discover_data_state_container.ts | 5 +++-- .../application/main/services/discover_state.ts | 1 + .../application/main/services/load_saved_search.ts | 2 +- 7 files changed, 25 insertions(+), 7 deletions(-) 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 27407822553bb..5857527d17f42 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 @@ -60,7 +60,7 @@ export const buildStateSubscribe = const isTextBasedQueryLangPrev = isTextBasedQuery(prevQuery); if (!isTextBasedQueryLangPrev) { savedSearchState.update({ nextState }); - dataState.reset(savedSearch); + dataState.reset(); } } // Cast to boolean to avoid false positives when comparing @@ -86,7 +86,7 @@ export const buildStateSubscribe = return; } savedSearch.searchSource.setField('index', nextDataView); - dataState.reset(savedSearch); + dataState.reset(); setDataView(nextDataView); savedSearchDataView = nextDataView; } diff --git a/src/plugins/discover/public/application/main/hooks/utils/change_data_view.test.ts b/src/plugins/discover/public/application/main/hooks/utils/change_data_view.test.ts index a0266b65ed01d..07c1a04918b62 100644 --- a/src/plugins/discover/public/application/main/hooks/utils/change_data_view.test.ts +++ b/src/plugins/discover/public/application/main/hooks/utils/change_data_view.test.ts @@ -26,7 +26,13 @@ const setupTestParams = (dataView: DataView | undefined) => { discoverState.internalState.transitions.setDataView(savedSearch.searchSource.getField('index')!); services.dataViews.get = jest.fn(() => Promise.resolve(dataView as DataView)); discoverState.appState.update = jest.fn(); - return { services, appState: discoverState.appState, internalState: discoverState.internalState }; + discoverState.dataState.reset = jest.fn(); + return { + services, + appState: discoverState.appState, + internalState: discoverState.internalState, + dataState: discoverState.dataState, + }; }; describe('changeDataView', () => { @@ -38,6 +44,7 @@ describe('changeDataView', () => { index: 'data-view-with-user-default-column-id', sort: [['@timestamp', 'desc']], }); + expect(params.dataState.reset).toHaveBeenCalled(); }); it('should set the right app state when a valid data view to switch to is given', async () => { @@ -48,11 +55,13 @@ describe('changeDataView', () => { index: 'data-view-with-various-field-types-id', sort: [['data', 'desc']], }); + expect(params.dataState.reset).toHaveBeenCalled(); }); it('should not set the app state when an invalid data view to switch to is given', async () => { const params = setupTestParams(undefined); await changeDataView('data-view-with-various-field-types', params); expect(params.appState.update).not.toHaveBeenCalled(); + expect(params.dataState.reset).toHaveBeenCalled(); }); }); diff --git a/src/plugins/discover/public/application/main/hooks/utils/change_data_view.ts b/src/plugins/discover/public/application/main/hooks/utils/change_data_view.ts index 7218ea76b2ac6..ec45bce14d8f4 100644 --- a/src/plugins/discover/public/application/main/hooks/utils/change_data_view.ts +++ b/src/plugins/discover/public/application/main/hooks/utils/change_data_view.ts @@ -18,6 +18,9 @@ import { DiscoverAppStateContainer } from '../../services/discover_app_state_con import { addLog } from '../../../../utils/add_log'; import { DiscoverServices } from '../../../../build_services'; import { getDataViewAppState } from '../../utils/get_switch_data_view_app_state'; +import { + DiscoverDataStateContainer +} from "@kbn/discover-plugin/public/application/main/services/discover_data_state_container"; /** * Function executed when switching data view in the UI @@ -28,10 +31,12 @@ export async function changeDataView( services, internalState, appState, + dataState, }: { services: DiscoverServices; internalState: DiscoverInternalStateContainer; appState: DiscoverAppStateContainer; + dataState: DiscoverDataStateContainer; } ) { addLog('[ui] changeDataView', { id }); @@ -39,6 +44,8 @@ export async function changeDataView( const dataView = internalState.getState().dataView; const state = appState.getState(); let nextDataView: DataView | null = null; + // switch to the loading state of Discover Data, to make sure loading indication is displayed when loading the new data view + dataState.reset(); try { nextDataView = typeof id === 'string' ? await dataViews.get(id, false) : id; diff --git a/src/plugins/discover/public/application/main/services/discover_data_state_container.test.ts b/src/plugins/discover/public/application/main/services/discover_data_state_container.test.ts index 516d81cc9c3f0..ee46732075496 100644 --- a/src/plugins/discover/public/application/main/services/discover_data_state_container.test.ts +++ b/src/plugins/discover/public/application/main/services/discover_data_state_container.test.ts @@ -95,7 +95,7 @@ describe('test getDataStateContainer', () => { await waitFor(() => { expect(dataState.data$.main$.value.fetchStatus).toBe(FetchStatus.COMPLETE); }); - dataState.reset(stateContainer.savedSearchState.getState()); + dataState.reset(); await waitFor(() => { expect(dataState.data$.main$.value.fetchStatus).toBe(FetchStatus.LOADING); }); diff --git a/src/plugins/discover/public/application/main/services/discover_data_state_container.ts b/src/plugins/discover/public/application/main/services/discover_data_state_container.ts index b2f8ae9f0a81d..201f1ba262781 100644 --- a/src/plugins/discover/public/application/main/services/discover_data_state_container.ts +++ b/src/plugins/discover/public/application/main/services/discover_data_state_container.ts @@ -122,7 +122,7 @@ export interface DiscoverDataStateContainer { /** * resetting all data observable to initial state */ - reset: (savedSearch: SavedSearch) => void; + reset: () => void; /** * cancels the running queries @@ -320,7 +320,8 @@ export function getDataStateContainer({ return refetch$; }; - const reset = (savedSearch: SavedSearch) => { + const reset = () => { + const savedSearch = getSavedSearch(); const recordType = getRawRecordType(savedSearch.searchSource.getField('query')); sendResetMsg(dataSubjects, getInitialFetchStatus(), recordType); }; diff --git a/src/plugins/discover/public/application/main/services/discover_state.ts b/src/plugins/discover/public/application/main/services/discover_state.ts index 1e9be0600c3af..2a0097f3e8d42 100644 --- a/src/plugins/discover/public/application/main/services/discover_state.ts +++ b/src/plugins/discover/public/application/main/services/discover_state.ts @@ -454,6 +454,7 @@ export function getDiscoverStateContainer({ services, internalState: internalStateContainer, appState: appStateContainer, + dataState: dataStateContainer, }); }; /** 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 30ed1792a50e0..3d958b6895606 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 @@ -142,7 +142,7 @@ function updateBySavedSearch(savedSearch: SavedSearch, deps: LoadSavedSearchDeps } // Finally notify dataStateContainer, data.query and filterManager about new derived state - dataStateContainer.reset(savedSearch); + dataStateContainer.reset(); // set data service filters const filters = savedSearch.searchSource.getField('filter'); if (Array.isArray(filters) && filters.length) { From 96074a286a3e94e412e4e742571c0a1674e172ed Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Mon, 19 Feb 2024 21:01:33 +0100 Subject: [PATCH 2/8] Fix linting --- .../public/application/main/hooks/utils/change_data_view.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/plugins/discover/public/application/main/hooks/utils/change_data_view.ts b/src/plugins/discover/public/application/main/hooks/utils/change_data_view.ts index ec45bce14d8f4..4b0b79fb5be47 100644 --- a/src/plugins/discover/public/application/main/hooks/utils/change_data_view.ts +++ b/src/plugins/discover/public/application/main/hooks/utils/change_data_view.ts @@ -13,14 +13,12 @@ import { SORT_DEFAULT_ORDER_SETTING, DEFAULT_COLUMNS_SETTING, } from '@kbn/discover-utils'; +import { DiscoverDataStateContainer } from '../../services/discover_data_state_container'; import { DiscoverInternalStateContainer } from '../../services/discover_internal_state_container'; import { DiscoverAppStateContainer } from '../../services/discover_app_state_container'; import { addLog } from '../../../../utils/add_log'; import { DiscoverServices } from '../../../../build_services'; import { getDataViewAppState } from '../../utils/get_switch_data_view_app_state'; -import { - DiscoverDataStateContainer -} from "@kbn/discover-plugin/public/application/main/services/discover_data_state_container"; /** * Function executed when switching data view in the UI From 2b96dfcbf0c4a76d1136eaf63bad704cdb40f731 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Tue, 20 Feb 2024 07:00:42 +0100 Subject: [PATCH 3/8] Improve code --- .../main/components/layout/discover_layout.tsx | 12 ++++++++---- .../main/hooks/utils/change_data_view.test.ts | 13 ++++++++----- .../main/hooks/utils/change_data_view.ts | 6 ++---- .../services/discover_internal_state_container.ts | 9 ++++++++- .../application/main/services/discover_state.ts | 1 - .../public/application/main/utils/fetch_all.test.ts | 3 +++ 6 files changed, 29 insertions(+), 15 deletions(-) diff --git a/src/plugins/discover/public/application/main/components/layout/discover_layout.tsx b/src/plugins/discover/public/application/main/components/layout/discover_layout.tsx index 57c5a9041dac2..69bf6d44fad4d 100644 --- a/src/plugins/discover/public/application/main/components/layout/discover_layout.tsx +++ b/src/plugins/discover/public/application/main/components/layout/discover_layout.tsx @@ -81,7 +81,10 @@ export function DiscoverLayout({ stateContainer }: DiscoverLayoutProps) { if (uiSettings.get(SHOW_FIELD_STATISTICS) !== true) return VIEW_MODE.DOCUMENT_LEVEL; return state.viewMode ?? VIEW_MODE.DOCUMENT_LEVEL; }); - const dataView = useInternalStateSelector((state) => state.dataView!); + const [dataView, dataViewLoading] = useInternalStateSelector((state) => [ + state.dataView!, + state.dataViewLoading, + ]); const dataState: DataMainMsg = useDataState(main$); const savedSearch = useSavedSearchInitial(); @@ -223,12 +226,13 @@ export function DiscoverLayout({ stateContainer }: DiscoverLayoutProps) { onDropFieldToTable={onDropFieldToTable} panelsToggle={panelsToggle} /> - {resultState === 'loading' && } + {(resultState === 'loading' || dataViewLoading) && } ); }, [ currentColumns, dataView, + dataViewLoading, docLinks, isPlainRecord, mainContainer, @@ -243,8 +247,8 @@ export function DiscoverLayout({ stateContainer }: DiscoverLayoutProps) { const isLoading = documentState.fetchStatus === FetchStatus.LOADING || - documentState.fetchStatus === FetchStatus.PARTIAL; - + documentState.fetchStatus === FetchStatus.PARTIAL || + dataViewLoading; const onCancelClick = useCallback(() => { stateContainer.dataState.cancel(); sendErrorMsg(stateContainer.dataState.data$.documents$); diff --git a/src/plugins/discover/public/application/main/hooks/utils/change_data_view.test.ts b/src/plugins/discover/public/application/main/hooks/utils/change_data_view.test.ts index 07c1a04918b62..d94f01f1a7f1e 100644 --- a/src/plugins/discover/public/application/main/hooks/utils/change_data_view.test.ts +++ b/src/plugins/discover/public/application/main/hooks/utils/change_data_view.test.ts @@ -15,6 +15,8 @@ import { savedSearchMock } from '../../../../__mocks__/saved_search'; import { discoverServiceMock } from '../../../../__mocks__/services'; import type { DataView } from '@kbn/data-views-plugin/common'; import { getDiscoverStateMock } from '../../../../__mocks__/discover_state.mock'; +import { PureTransitionsToTransitions } from '@kbn/kibana-utils-plugin/common/state_containers'; +import { InternalStateTransitions } from '../../services/discover_internal_state_container'; const setupTestParams = (dataView: DataView | undefined) => { const savedSearch = savedSearchMock; @@ -26,12 +28,13 @@ const setupTestParams = (dataView: DataView | undefined) => { discoverState.internalState.transitions.setDataView(savedSearch.searchSource.getField('index')!); services.dataViews.get = jest.fn(() => Promise.resolve(dataView as DataView)); discoverState.appState.update = jest.fn(); - discoverState.dataState.reset = jest.fn(); + discoverState.internalState.transitions = { + setDataViewLoading: jest.fn(), + } as unknown as Readonly>; return { services, appState: discoverState.appState, internalState: discoverState.internalState, - dataState: discoverState.dataState, }; }; @@ -44,7 +47,7 @@ describe('changeDataView', () => { index: 'data-view-with-user-default-column-id', sort: [['@timestamp', 'desc']], }); - expect(params.dataState.reset).toHaveBeenCalled(); + expect(params.internalState.transitions.setDataViewLoading).toBeCalledTimes(2); }); it('should set the right app state when a valid data view to switch to is given', async () => { @@ -55,13 +58,13 @@ describe('changeDataView', () => { index: 'data-view-with-various-field-types-id', sort: [['data', 'desc']], }); - expect(params.dataState.reset).toHaveBeenCalled(); + expect(params.internalState.transitions.setDataViewLoading).toBeCalledTimes(2); }); it('should not set the app state when an invalid data view to switch to is given', async () => { const params = setupTestParams(undefined); await changeDataView('data-view-with-various-field-types', params); expect(params.appState.update).not.toHaveBeenCalled(); - expect(params.dataState.reset).toHaveBeenCalled(); + expect(params.internalState.transitions.setDataViewLoading).toBeCalledTimes(2); }); }); diff --git a/src/plugins/discover/public/application/main/hooks/utils/change_data_view.ts b/src/plugins/discover/public/application/main/hooks/utils/change_data_view.ts index 4b0b79fb5be47..3754079f6a10a 100644 --- a/src/plugins/discover/public/application/main/hooks/utils/change_data_view.ts +++ b/src/plugins/discover/public/application/main/hooks/utils/change_data_view.ts @@ -13,7 +13,6 @@ import { SORT_DEFAULT_ORDER_SETTING, DEFAULT_COLUMNS_SETTING, } from '@kbn/discover-utils'; -import { DiscoverDataStateContainer } from '../../services/discover_data_state_container'; import { DiscoverInternalStateContainer } from '../../services/discover_internal_state_container'; import { DiscoverAppStateContainer } from '../../services/discover_app_state_container'; import { addLog } from '../../../../utils/add_log'; @@ -29,12 +28,10 @@ export async function changeDataView( services, internalState, appState, - dataState, }: { services: DiscoverServices; internalState: DiscoverInternalStateContainer; appState: DiscoverAppStateContainer; - dataState: DiscoverDataStateContainer; } ) { addLog('[ui] changeDataView', { id }); @@ -43,7 +40,7 @@ export async function changeDataView( const state = appState.getState(); let nextDataView: DataView | null = null; // switch to the loading state of Discover Data, to make sure loading indication is displayed when loading the new data view - dataState.reset(); + internalState.transitions.setDataViewLoading(true); try { nextDataView = typeof id === 'string' ? await dataViews.get(id, false) : id; @@ -68,4 +65,5 @@ export async function changeDataView( internalState.transitions.setExpandedDoc(undefined); } } + internalState.transitions.setDataViewLoading(false); } diff --git a/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts b/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts index ec8d7ec6c57f2..8604803a062ef 100644 --- a/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts +++ b/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts @@ -17,14 +17,16 @@ import type { DataTableRecord } from '@kbn/discover-utils/types'; export interface InternalState { dataView: DataView | undefined; + dataViewLoading: boolean; savedDataViews: DataViewListItem[]; adHocDataViews: DataView[]; expandedDoc: DataTableRecord | undefined; customFilters: Filter[]; } -interface InternalStateTransitions { +export interface InternalStateTransitions { setDataView: (state: InternalState) => (dataView: DataView) => InternalState; + setDataViewLoading: (state: InternalState) => (isLoading: boolean) => InternalState; setSavedDataViews: (state: InternalState) => (dataView: DataViewListItem[]) => InternalState; setAdHocDataViews: (state: InternalState) => (dataViews: DataView[]) => InternalState; appendAdHocDataViews: ( @@ -52,6 +54,7 @@ export function getInternalStateContainer() { return createStateContainer( { dataView: undefined, + dataViewLoading: false, adHocDataViews: [], savedDataViews: [], expandedDoc: undefined, @@ -62,6 +65,10 @@ export function getInternalStateContainer() { ...prevState, dataView: nextDataView, }), + setDataViewLoading: (prevState: InternalState) => (loading: boolean) => ({ + ...prevState, + dataViewLoading: loading, + }), setSavedDataViews: (prevState: InternalState) => (nextDataViewList: DataViewListItem[]) => ({ ...prevState, savedDataViews: nextDataViewList, diff --git a/src/plugins/discover/public/application/main/services/discover_state.ts b/src/plugins/discover/public/application/main/services/discover_state.ts index 2a0097f3e8d42..1e9be0600c3af 100644 --- a/src/plugins/discover/public/application/main/services/discover_state.ts +++ b/src/plugins/discover/public/application/main/services/discover_state.ts @@ -454,7 +454,6 @@ export function getDiscoverStateContainer({ services, internalState: internalStateContainer, appState: appStateContainer, - dataState: dataStateContainer, }); }; /** diff --git a/src/plugins/discover/public/application/main/utils/fetch_all.test.ts b/src/plugins/discover/public/application/main/utils/fetch_all.test.ts index 744ca9039d8c0..80696f8dce28e 100644 --- a/src/plugins/discover/public/application/main/utils/fetch_all.test.ts +++ b/src/plugins/discover/public/application/main/utils/fetch_all.test.ts @@ -72,6 +72,7 @@ describe('test fetchAll', () => { getAppState: () => ({}), getInternalState: () => ({ dataView: undefined, + dataViewLoading: false, savedDataViews: [], adHocDataViews: [], expandedDoc: undefined, @@ -269,6 +270,7 @@ describe('test fetchAll', () => { getAppState: () => ({ query }), getInternalState: () => ({ dataView: undefined, + dataViewLoading: false, savedDataViews: [], adHocDataViews: [], expandedDoc: undefined, @@ -394,6 +396,7 @@ describe('test fetchAll', () => { getAppState: () => ({ query }), getInternalState: () => ({ dataView: undefined, + dataViewLoading: false, savedDataViews: [], adHocDataViews: [], expandedDoc: undefined, From 39a452fd6d97be58774a0e83d80cdabe2c6005a8 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Fri, 23 Feb 2024 09:55:45 +0100 Subject: [PATCH 4/8] Add a delayed EuiProgress renderer when a DataView is being loaded --- .../components/layout/discover_layout.tsx | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/plugins/discover/public/application/main/components/layout/discover_layout.tsx b/src/plugins/discover/public/application/main/components/layout/discover_layout.tsx index 69bf6d44fad4d..8dae8a2c41eae 100644 --- a/src/plugins/discover/public/application/main/components/layout/discover_layout.tsx +++ b/src/plugins/discover/public/application/main/components/layout/discover_layout.tsx @@ -7,7 +7,14 @@ */ import './discover_layout.scss'; import React, { ReactElement, useCallback, useEffect, useMemo, useRef, useState } from 'react'; -import { EuiPage, EuiPageBody, EuiPanel, useEuiBackgroundColor } from '@elastic/eui'; +import { + EuiPage, + EuiPageBody, + EuiPanel, + EuiProgress, + useEuiBackgroundColor, + EuiDelayRender, +} from '@elastic/eui'; import { css } from '@emotion/react'; import { i18n } from '@kbn/i18n'; import { METRIC_TYPE } from '@kbn/analytics'; @@ -226,13 +233,12 @@ export function DiscoverLayout({ stateContainer }: DiscoverLayoutProps) { onDropFieldToTable={onDropFieldToTable} panelsToggle={panelsToggle} /> - {(resultState === 'loading' || dataViewLoading) && } + {resultState === 'loading' && } ); }, [ currentColumns, dataView, - dataViewLoading, docLinks, isPlainRecord, mainContainer, @@ -247,8 +253,8 @@ export function DiscoverLayout({ stateContainer }: DiscoverLayoutProps) { const isLoading = documentState.fetchStatus === FetchStatus.LOADING || - documentState.fetchStatus === FetchStatus.PARTIAL || - dataViewLoading; + documentState.fetchStatus === FetchStatus.PARTIAL; + const onCancelClick = useCallback(() => { stateContainer.dataState.cancel(); sendErrorMsg(stateContainer.dataState.data$.documents$); @@ -297,6 +303,11 @@ export function DiscoverLayout({ stateContainer }: DiscoverLayoutProps) { height: 100%; `} > + {dataViewLoading && ( + + + + )} Date: Fri, 23 Feb 2024 10:04:32 +0100 Subject: [PATCH 5/8] Cleanup --- .../application/main/hooks/utils/build_state_subscribe.ts | 4 ++-- .../public/application/main/hooks/utils/change_data_view.ts | 1 - .../main/services/discover_data_state_container.test.ts | 2 +- .../main/services/discover_data_state_container.ts | 5 ++--- 4 files changed, 5 insertions(+), 7 deletions(-) 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 5857527d17f42..27407822553bb 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 @@ -60,7 +60,7 @@ export const buildStateSubscribe = const isTextBasedQueryLangPrev = isTextBasedQuery(prevQuery); if (!isTextBasedQueryLangPrev) { savedSearchState.update({ nextState }); - dataState.reset(); + dataState.reset(savedSearch); } } // Cast to boolean to avoid false positives when comparing @@ -86,7 +86,7 @@ export const buildStateSubscribe = return; } savedSearch.searchSource.setField('index', nextDataView); - dataState.reset(); + dataState.reset(savedSearch); setDataView(nextDataView); savedSearchDataView = nextDataView; } diff --git a/src/plugins/discover/public/application/main/hooks/utils/change_data_view.ts b/src/plugins/discover/public/application/main/hooks/utils/change_data_view.ts index 3754079f6a10a..093f047cb7249 100644 --- a/src/plugins/discover/public/application/main/hooks/utils/change_data_view.ts +++ b/src/plugins/discover/public/application/main/hooks/utils/change_data_view.ts @@ -39,7 +39,6 @@ export async function changeDataView( const dataView = internalState.getState().dataView; const state = appState.getState(); let nextDataView: DataView | null = null; - // switch to the loading state of Discover Data, to make sure loading indication is displayed when loading the new data view internalState.transitions.setDataViewLoading(true); try { diff --git a/src/plugins/discover/public/application/main/services/discover_data_state_container.test.ts b/src/plugins/discover/public/application/main/services/discover_data_state_container.test.ts index ee46732075496..516d81cc9c3f0 100644 --- a/src/plugins/discover/public/application/main/services/discover_data_state_container.test.ts +++ b/src/plugins/discover/public/application/main/services/discover_data_state_container.test.ts @@ -95,7 +95,7 @@ describe('test getDataStateContainer', () => { await waitFor(() => { expect(dataState.data$.main$.value.fetchStatus).toBe(FetchStatus.COMPLETE); }); - dataState.reset(); + dataState.reset(stateContainer.savedSearchState.getState()); await waitFor(() => { expect(dataState.data$.main$.value.fetchStatus).toBe(FetchStatus.LOADING); }); diff --git a/src/plugins/discover/public/application/main/services/discover_data_state_container.ts b/src/plugins/discover/public/application/main/services/discover_data_state_container.ts index 201f1ba262781..b2f8ae9f0a81d 100644 --- a/src/plugins/discover/public/application/main/services/discover_data_state_container.ts +++ b/src/plugins/discover/public/application/main/services/discover_data_state_container.ts @@ -122,7 +122,7 @@ export interface DiscoverDataStateContainer { /** * resetting all data observable to initial state */ - reset: () => void; + reset: (savedSearch: SavedSearch) => void; /** * cancels the running queries @@ -320,8 +320,7 @@ export function getDataStateContainer({ return refetch$; }; - const reset = () => { - const savedSearch = getSavedSearch(); + const reset = (savedSearch: SavedSearch) => { const recordType = getRawRecordType(savedSearch.searchSource.getField('query')); sendResetMsg(dataSubjects, getInitialFetchStatus(), recordType); }; From 0ee71a007effc8d75051b0c710a28e1177817765 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Fri, 23 Feb 2024 10:05:57 +0100 Subject: [PATCH 6/8] Cleanup prt 2 --- .../public/application/main/services/load_saved_search.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 3d958b6895606..30ed1792a50e0 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 @@ -142,7 +142,7 @@ function updateBySavedSearch(savedSearch: SavedSearch, deps: LoadSavedSearchDeps } // Finally notify dataStateContainer, data.query and filterManager about new derived state - dataStateContainer.reset(); + dataStateContainer.reset(savedSearch); // set data service filters const filters = savedSearch.searchSource.getField('filter'); if (Array.isArray(filters) && filters.length) { From 9eb4b2075e7b5bdb5d2e16e13d958b5c4916b411 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Fri, 23 Feb 2024 17:55:46 +0100 Subject: [PATCH 7/8] Update src/plugins/discover/public/application/main/hooks/utils/change_data_view.test.ts Co-authored-by: Julia Rechkunova --- .../application/main/hooks/utils/change_data_view.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/plugins/discover/public/application/main/hooks/utils/change_data_view.test.ts b/src/plugins/discover/public/application/main/hooks/utils/change_data_view.test.ts index d94f01f1a7f1e..438ef92d00e8b 100644 --- a/src/plugins/discover/public/application/main/hooks/utils/change_data_view.test.ts +++ b/src/plugins/discover/public/application/main/hooks/utils/change_data_view.test.ts @@ -47,7 +47,8 @@ describe('changeDataView', () => { index: 'data-view-with-user-default-column-id', sort: [['@timestamp', 'desc']], }); - expect(params.internalState.transitions.setDataViewLoading).toBeCalledTimes(2); + expect(params.internalState.transitions.setDataViewLoading).toHaveBeenNthCalledWith(1, true); + expect(params.internalState.transitions.setDataViewLoading).toHaveBeenNthCalledWith(2, false); }); it('should set the right app state when a valid data view to switch to is given', async () => { From f4038750b2b54ded52fee2809da2c8bcce47d159 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Fri, 23 Feb 2024 19:58:57 +0100 Subject: [PATCH 8/8] Address review feedback --- .../main/components/layout/discover_layout.tsx | 2 +- .../main/hooks/utils/change_data_view.test.ts | 12 +++++++----- .../application/main/hooks/utils/change_data_view.ts | 4 ++-- .../services/discover_internal_state_container.ts | 10 +++++----- .../public/application/main/utils/fetch_all.test.ts | 6 +++--- 5 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/plugins/discover/public/application/main/components/layout/discover_layout.tsx b/src/plugins/discover/public/application/main/components/layout/discover_layout.tsx index 8dae8a2c41eae..db500566d3143 100644 --- a/src/plugins/discover/public/application/main/components/layout/discover_layout.tsx +++ b/src/plugins/discover/public/application/main/components/layout/discover_layout.tsx @@ -90,7 +90,7 @@ export function DiscoverLayout({ stateContainer }: DiscoverLayoutProps) { }); const [dataView, dataViewLoading] = useInternalStateSelector((state) => [ state.dataView!, - state.dataViewLoading, + state.isDataViewLoading, ]); const dataState: DataMainMsg = useDataState(main$); const savedSearch = useSavedSearchInitial(); diff --git a/src/plugins/discover/public/application/main/hooks/utils/change_data_view.test.ts b/src/plugins/discover/public/application/main/hooks/utils/change_data_view.test.ts index 438ef92d00e8b..118e6e6c0f2db 100644 --- a/src/plugins/discover/public/application/main/hooks/utils/change_data_view.test.ts +++ b/src/plugins/discover/public/application/main/hooks/utils/change_data_view.test.ts @@ -29,7 +29,7 @@ const setupTestParams = (dataView: DataView | undefined) => { services.dataViews.get = jest.fn(() => Promise.resolve(dataView as DataView)); discoverState.appState.update = jest.fn(); discoverState.internalState.transitions = { - setDataViewLoading: jest.fn(), + setIsDataViewLoading: jest.fn(), } as unknown as Readonly>; return { services, @@ -47,8 +47,8 @@ describe('changeDataView', () => { index: 'data-view-with-user-default-column-id', sort: [['@timestamp', 'desc']], }); - expect(params.internalState.transitions.setDataViewLoading).toHaveBeenNthCalledWith(1, true); - expect(params.internalState.transitions.setDataViewLoading).toHaveBeenNthCalledWith(2, false); + expect(params.internalState.transitions.setIsDataViewLoading).toHaveBeenNthCalledWith(1, true); + expect(params.internalState.transitions.setIsDataViewLoading).toHaveBeenNthCalledWith(2, false); }); it('should set the right app state when a valid data view to switch to is given', async () => { @@ -59,13 +59,15 @@ describe('changeDataView', () => { index: 'data-view-with-various-field-types-id', sort: [['data', 'desc']], }); - expect(params.internalState.transitions.setDataViewLoading).toBeCalledTimes(2); + expect(params.internalState.transitions.setIsDataViewLoading).toHaveBeenNthCalledWith(1, true); + expect(params.internalState.transitions.setIsDataViewLoading).toHaveBeenNthCalledWith(2, false); }); it('should not set the app state when an invalid data view to switch to is given', async () => { const params = setupTestParams(undefined); await changeDataView('data-view-with-various-field-types', params); expect(params.appState.update).not.toHaveBeenCalled(); - expect(params.internalState.transitions.setDataViewLoading).toBeCalledTimes(2); + expect(params.internalState.transitions.setIsDataViewLoading).toHaveBeenNthCalledWith(1, true); + expect(params.internalState.transitions.setIsDataViewLoading).toHaveBeenNthCalledWith(2, false); }); }); diff --git a/src/plugins/discover/public/application/main/hooks/utils/change_data_view.ts b/src/plugins/discover/public/application/main/hooks/utils/change_data_view.ts index 093f047cb7249..41a911295cacd 100644 --- a/src/plugins/discover/public/application/main/hooks/utils/change_data_view.ts +++ b/src/plugins/discover/public/application/main/hooks/utils/change_data_view.ts @@ -39,7 +39,7 @@ export async function changeDataView( const dataView = internalState.getState().dataView; const state = appState.getState(); let nextDataView: DataView | null = null; - internalState.transitions.setDataViewLoading(true); + internalState.transitions.setIsDataViewLoading(true); try { nextDataView = typeof id === 'string' ? await dataViews.get(id, false) : id; @@ -64,5 +64,5 @@ export async function changeDataView( internalState.transitions.setExpandedDoc(undefined); } } - internalState.transitions.setDataViewLoading(false); + internalState.transitions.setIsDataViewLoading(false); } diff --git a/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts b/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts index 8604803a062ef..5825e4d2cef6c 100644 --- a/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts +++ b/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts @@ -17,7 +17,7 @@ import type { DataTableRecord } from '@kbn/discover-utils/types'; export interface InternalState { dataView: DataView | undefined; - dataViewLoading: boolean; + isDataViewLoading: boolean; savedDataViews: DataViewListItem[]; adHocDataViews: DataView[]; expandedDoc: DataTableRecord | undefined; @@ -26,7 +26,7 @@ export interface InternalState { export interface InternalStateTransitions { setDataView: (state: InternalState) => (dataView: DataView) => InternalState; - setDataViewLoading: (state: InternalState) => (isLoading: boolean) => InternalState; + setIsDataViewLoading: (state: InternalState) => (isLoading: boolean) => InternalState; setSavedDataViews: (state: InternalState) => (dataView: DataViewListItem[]) => InternalState; setAdHocDataViews: (state: InternalState) => (dataViews: DataView[]) => InternalState; appendAdHocDataViews: ( @@ -54,7 +54,7 @@ export function getInternalStateContainer() { return createStateContainer( { dataView: undefined, - dataViewLoading: false, + isDataViewLoading: false, adHocDataViews: [], savedDataViews: [], expandedDoc: undefined, @@ -65,9 +65,9 @@ export function getInternalStateContainer() { ...prevState, dataView: nextDataView, }), - setDataViewLoading: (prevState: InternalState) => (loading: boolean) => ({ + setIsDataViewLoading: (prevState: InternalState) => (loading: boolean) => ({ ...prevState, - dataViewLoading: loading, + isDataViewLoading: loading, }), setSavedDataViews: (prevState: InternalState) => (nextDataViewList: DataViewListItem[]) => ({ ...prevState, diff --git a/src/plugins/discover/public/application/main/utils/fetch_all.test.ts b/src/plugins/discover/public/application/main/utils/fetch_all.test.ts index 80696f8dce28e..08fae23110972 100644 --- a/src/plugins/discover/public/application/main/utils/fetch_all.test.ts +++ b/src/plugins/discover/public/application/main/utils/fetch_all.test.ts @@ -72,7 +72,7 @@ describe('test fetchAll', () => { getAppState: () => ({}), getInternalState: () => ({ dataView: undefined, - dataViewLoading: false, + isDataViewLoading: false, savedDataViews: [], adHocDataViews: [], expandedDoc: undefined, @@ -270,7 +270,7 @@ describe('test fetchAll', () => { getAppState: () => ({ query }), getInternalState: () => ({ dataView: undefined, - dataViewLoading: false, + isDataViewLoading: false, savedDataViews: [], adHocDataViews: [], expandedDoc: undefined, @@ -396,7 +396,7 @@ describe('test fetchAll', () => { getAppState: () => ({ query }), getInternalState: () => ({ dataView: undefined, - dataViewLoading: false, + isDataViewLoading: false, savedDataViews: [], adHocDataViews: [], expandedDoc: undefined,