diff --git a/src/plugins/discover/public/application/main/components/layout/use_discover_histogram.ts b/src/plugins/discover/public/application/main/components/layout/use_discover_histogram.ts index 66b5be4d02ab7..dbf145dba05de 100644 --- a/src/plugins/discover/public/application/main/components/layout/use_discover_histogram.ts +++ b/src/plugins/discover/public/application/main/components/layout/use_discover_histogram.ts @@ -42,7 +42,7 @@ import type { DiscoverStateContainer } from '../../state_management/discover_sta import { addLog } from '../../../../utils/add_log'; import { useInternalStateSelector } from '../../state_management/discover_internal_state_container'; import type { DiscoverAppState } from '../../state_management/discover_app_state_container'; -import { DataDocumentsMsg } from '../../state_management/discover_data_state_container'; +import type { DataDocumentsMsg } from '../../state_management/discover_data_state_container'; import { useSavedSearch } from '../../state_management/discover_state_provider'; import { useIsEsqlMode } from '../../hooks/use_is_esql_mode'; @@ -177,7 +177,7 @@ export const useDiscoverHistogram = ({ totalHitsResult && typeof result !== 'number' ) { - // ignore the histogram initial loading state if discover state already has a total hits value + addLog(`[UnifiedHistogram] skip ${status}: total hits > 0 in Discover`, result); return; } @@ -192,6 +192,10 @@ export const useDiscoverHistogram = ({ } if (status !== UnifiedHistogramFetchStatus.complete || typeof result !== 'number') { + addLog( + '[UnifiedHistogram] ignore the histogram complete/partial state if discover state already has a total hits value', + { status, result } + ); return; } diff --git a/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx b/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx index 20ab07e1155cd..8c861c9e7ed08 100644 --- a/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx +++ b/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx @@ -8,7 +8,7 @@ */ import React, { useCallback, useEffect, useMemo, useRef } from 'react'; -import { type DataView, DataViewType } from '@kbn/data-views-plugin/public'; +import { DataViewType } from '@kbn/data-views-plugin/public'; import { DataViewPickerProps } from '@kbn/unified-search-plugin/public'; import { ENABLE_ESQL } from '@kbn/esql-utils'; import { TextBasedLanguages } from '@kbn/esql-utils'; @@ -20,7 +20,6 @@ import { useDiscoverServices } from '../../../../hooks/use_discover_services'; import type { DiscoverStateContainer } from '../../state_management/discover_state'; import { onSaveSearch } from './on_save_search'; import { useDiscoverCustomization } from '../../../../customizations'; -import { addLog } from '../../../../utils/add_log'; import { useAppStateSelector } from '../../state_management/discover_app_state_container'; import { useDiscoverTopNav } from './use_discover_topnav'; import { useIsEsqlMode } from '../../hooks/use_is_esql_mode'; @@ -47,15 +46,8 @@ export const DiscoverTopNav = ({ onCancelClick, }: DiscoverTopNavProps) => { const services = useDiscoverServices(); - const { - dataViewEditor, - navigation, - dataViewFieldEditor, - data, - uiSettings, - dataViews, - setHeaderActionMenu, - } = services; + const { dataViewEditor, navigation, dataViewFieldEditor, data, uiSettings, setHeaderActionMenu } = + services; const query = useAppStateSelector((state) => state.query); const adHocDataViews = useInternalStateSelector((state) => state.adHocDataViews); const dataView = useInternalStateSelector((state) => state.dataView!); @@ -123,23 +115,6 @@ export const DiscoverTopNav = ({ }); }, [dataViewEditor, stateContainer]); - const onEditDataView = useCallback( - async (editedDataView: DataView) => { - if (editedDataView.isPersisted()) { - // Clear the current data view from the cache and create a new instance - // of it, ensuring we have a new object reference to trigger a re-render - dataViews.clearInstanceCache(editedDataView.id); - stateContainer.actions.setDataView(await dataViews.create(editedDataView.toSpec(), true)); - } else { - await stateContainer.actions.updateAdHocDataViewId(); - } - stateContainer.actions.loadDataViewList(); - addLog('[DiscoverTopNav] onEditDataView triggers data fetching'); - stateContainer.dataState.fetch(); - }, - [dataViews, stateContainer.actions, stateContainer.dataState] - ); - const updateSavedQueryId = (newSavedQueryId: string | undefined) => { const { appState } = stateContainer; if (newSavedQueryId) { @@ -223,14 +198,13 @@ export const DiscoverTopNav = ({ textBasedLanguages: supportedTextBasedLanguages, adHocDataViews, savedDataViews, - onEditDataView, + onEditDataView: stateContainer.actions.onDataViewEdited, }; }, [ adHocDataViews, addField, createNewDataView, dataView, - onEditDataView, savedDataViews, stateContainer, uiSettings, diff --git a/src/plugins/discover/public/application/main/data_fetching/fetch_all.test.ts b/src/plugins/discover/public/application/main/data_fetching/fetch_all.test.ts index 169a644e69499..c58417f818b28 100644 --- a/src/plugins/discover/public/application/main/data_fetching/fetch_all.test.ts +++ b/src/plugins/discover/public/application/main/data_fetching/fetch_all.test.ts @@ -69,6 +69,7 @@ describe('test fetchAll', () => { getAppState: () => ({}), getInternalState: () => ({ dataView: undefined, + isLoading: false, isDataViewLoading: false, savedDataViews: [], adHocDataViews: [], @@ -262,6 +263,7 @@ describe('test fetchAll', () => { getInternalState: () => ({ dataView: undefined, isDataViewLoading: false, + isLoading: false, savedDataViews: [], adHocDataViews: [], expandedDoc: undefined, @@ -384,6 +386,7 @@ describe('test fetchAll', () => { getInternalState: () => ({ dataView: undefined, isDataViewLoading: false, + isLoading: false, savedDataViews: [], adHocDataViews: [], expandedDoc: undefined, diff --git a/src/plugins/discover/public/application/main/discover_main_route.tsx b/src/plugins/discover/public/application/main/discover_main_route.tsx index 205926cf4943b..08383d9dcac2b 100644 --- a/src/plugins/discover/public/application/main/discover_main_route.tsx +++ b/src/plugins/discover/public/application/main/discover_main_route.tsx @@ -22,6 +22,7 @@ import { reportPerformanceMetricEvent } from '@kbn/ebt-tools'; import { withSuspense } from '@kbn/shared-ux-utility'; import { getInitialESQLQuery } from '@kbn/esql-utils'; import { ESQL_TYPE } from '@kbn/data-view-utils'; +import { useInternalStateSelector } from './state_management/discover_internal_state_container'; import { useUrl } from './hooks/use_url'; import { useDiscoverStateContainer } from './hooks/use_discover_state_container'; import { MainHistoryLocationState } from '../../../common'; @@ -85,7 +86,6 @@ export function DiscoverMainRoute({ stateContainer, }); const [error, setError] = useState(); - const [loading, setLoading] = useState(true); const [noDataState, setNoDataState] = useState({ hasESData: false, hasUserDataView: false, @@ -157,10 +157,10 @@ export function DiscoverMainRoute({ initialAppState, }: { nextDataView?: DataView; initialAppState?: LoadParams['initialAppState'] } = {}) => { const loadSavedSearchStartTime = window.performance.now(); - setLoading(true); + stateContainer.actions.setIsLoading(true); const skipNoData = await skipNoDataPage(nextDataView); if (!skipNoData) { - setLoading(false); + stateContainer.actions.setIsLoading(false); return; } try { @@ -181,7 +181,7 @@ export function DiscoverMainRoute({ setBreadcrumbs({ services, titleBreadcrumbText: currentSavedSearch?.title ?? undefined }); } - setLoading(false); + stateContainer.actions.setIsLoading(false); if (services.analytics) { const loadSavedSearchDuration = window.performance.now() - loadSavedSearchStartTime; reportPerformanceMetricEvent(services.analytics, { @@ -231,7 +231,7 @@ export function DiscoverMainRoute({ useEffect(() => { if (!isCustomizationServiceInitialized) return; - setLoading(true); + stateContainer.actions.setIsLoading(true); setNoDataState({ hasESData: false, hasUserDataView: false, @@ -259,13 +259,13 @@ export function DiscoverMainRoute({ const onDataViewCreated = useCallback( async (nextDataView: unknown) => { if (nextDataView) { - setLoading(true); + stateContainer.actions.setIsLoading(true); setNoDataState((state) => ({ ...state, showNoDataPage: false })); setError(undefined); await loadSavedSearch({ nextDataView: nextDataView as DataView }); } }, - [loadSavedSearch] + [loadSavedSearch, stateContainer] ); const onESQLNavigationComplete = useCallback(async () => { @@ -326,14 +326,8 @@ export function DiscoverMainRoute({ ); } - if (loading) { - return loadingIndicator; - } - return ; }, [ - loading, - loadingIndicator, noDataDependencies, onDataViewCreated, onESQLNavigationComplete, @@ -355,11 +349,11 @@ export function DiscoverMainRoute({ - - {mainContent} @@ -368,6 +362,28 @@ export function DiscoverMainRoute({ // eslint-disable-next-line import/no-default-export export default DiscoverMainRoute; +export function DiscoverMainLoading({ + stateContainer, + showNoDataPage, + mainContent, +}: { + stateContainer: DiscoverStateContainer; + showNoDataPage: boolean; + mainContent: JSX.Element; +}) { + const loading = useInternalStateSelector((state) => state.isLoading); + + return ( + <> + + {loading && !showNoDataPage ? : mainContent} + + ); +} + function getLoadParamsForNewSearch(stateContainer: DiscoverStateContainer): { nextDataView: LoadParams['dataView']; initialAppState: LoadParams['initialAppState']; diff --git a/src/plugins/discover/public/application/main/state_management/discover_internal_state_container.ts b/src/plugins/discover/public/application/main/state_management/discover_internal_state_container.ts index d5f7deecf980a..75cdcecd336c8 100644 --- a/src/plugins/discover/public/application/main/state_management/discover_internal_state_container.ts +++ b/src/plugins/discover/public/application/main/state_management/discover_internal_state_container.ts @@ -20,6 +20,7 @@ import type { UnifiedHistogramVisContext } from '@kbn/unified-histogram-plugin/p export interface InternalState { dataView: DataView | undefined; isDataViewLoading: boolean; + isLoading: boolean; savedDataViews: DataViewListItem[]; adHocDataViews: DataView[]; expandedDoc: DataTableRecord | undefined; @@ -32,6 +33,7 @@ export interface InternalState { export interface InternalStateTransitions { setDataView: (state: InternalState) => (dataView: DataView) => InternalState; setIsDataViewLoading: (state: InternalState) => (isLoading: boolean) => InternalState; + setIsLoading: (state: InternalState) => (isLoading: boolean) => InternalState; setSavedDataViews: (state: InternalState) => (dataView: DataViewListItem[]) => InternalState; setAdHocDataViews: (state: InternalState) => (dataViews: DataView[]) => InternalState; appendAdHocDataViews: ( @@ -72,6 +74,7 @@ export function getInternalStateContainer() { { dataView: undefined, isDataViewLoading: false, + isLoading: true, adHocDataViews: [], savedDataViews: [], expandedDoc: undefined, @@ -90,6 +93,10 @@ export function getInternalStateContainer() { ...prevState, isDataViewLoading: loading, }), + setIsLoading: (prevState: InternalState) => (isLoading: boolean) => ({ + ...prevState, + isLoading, + }), setIsESQLToDataViewTransitionModalVisible: (prevState: InternalState) => (isVisible: boolean) => ({ ...prevState, diff --git a/src/plugins/discover/public/application/main/state_management/discover_state.test.ts b/src/plugins/discover/public/application/main/state_management/discover_state.test.ts index a64e36bc39097..cb5ba0dcdbced 100644 --- a/src/plugins/discover/public/application/main/state_management/discover_state.test.ts +++ b/src/plugins/discover/public/application/main/state_management/discover_state.test.ts @@ -956,6 +956,15 @@ describe('Test discover state actions', () => { expect(setTime).toHaveBeenCalledWith({ from: 'now-15d', to: 'now-10d' }); expect(setRefreshInterval).toHaveBeenCalledWith({ pause: false, value: 1000 }); }); + + test('setIsLoading', async () => { + const { state } = await getState('/'); + expect(state.internalState.getState().isLoading).toBe(true); + await state.actions.setIsLoading(false); + expect(state.internalState.getState().isLoading).toBe(false); + await state.actions.setIsLoading(true); + expect(state.internalState.getState().isLoading).toBe(true); + }); }); describe('Test discover state with embedded mode', () => { diff --git a/src/plugins/discover/public/application/main/state_management/discover_state.ts b/src/plugins/discover/public/application/main/state_management/discover_state.ts index 56d1ca44c3853..e2bf085216988 100644 --- a/src/plugins/discover/public/application/main/state_management/discover_state.ts +++ b/src/plugins/discover/public/application/main/state_management/discover_state.ts @@ -211,6 +211,11 @@ export interface DiscoverStateContainer { * @param dataView */ setDataView: (dataView: DataView) => void; + /** + * Set Discover to loading state on the highest level, this cleans up all internal UI state + * @param value + */ + setIsLoading: (value: boolean) => void; /** * Undo changes made to the saved search, e.g. when the user triggers the "Reset search" button */ @@ -412,17 +417,24 @@ export function getDiscoverStateContainer({ } }; + const setIsLoading = (value: boolean) => { + internalStateContainer.transitions.setIsLoading(value); + }; + const onDataViewEdited = async (editedDataView: DataView) => { + setIsLoading(true); + const edited = editedDataView.id; if (editedDataView.isPersisted()) { // Clear the current data view from the cache and create a new instance // of it, ensuring we have a new object reference to trigger a re-render - services.dataViews.clearInstanceCache(editedDataView.id); + services.dataViews.clearInstanceCache(edited); setDataView(await services.dataViews.create(editedDataView.toSpec(), true)); } else { await updateAdHocDataViewId(); } - loadDataViewList(); + await loadDataViewList(); addLog('[getDiscoverStateContainer] onDataViewEdited triggers data fetching'); + internalStateContainer.transitions.setIsLoading(false); fetchData(); }; @@ -612,6 +624,7 @@ export function getDiscoverStateContainer({ onDataViewCreated, onDataViewEdited, onOpenSavedSearch, + setIsLoading, transitionFromESQLToDataView, transitionFromDataViewToESQL, onUpdateQuery, diff --git a/test/functional/apps/discover/group3/_lens_vis.ts b/test/functional/apps/discover/group3/_lens_vis.ts index 03641ee5bcb41..0864382cad7a8 100644 --- a/test/functional/apps/discover/group3/_lens_vis.ts +++ b/test/functional/apps/discover/group3/_lens_vis.ts @@ -110,8 +110,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { return seriesType; } - // FLAKY: https://github.com/elastic/kibana/issues/184600 - describe.skip('discover lens vis', function () { + describe('discover lens vis', function () { before(async () => { await security.testUser.setRoles(['kibana_admin', 'test_logstash_reader']); await esArchiver.loadIfNeeded('test/functional/fixtures/es_archiver/logstash_functional');