Discover loading state

src/plugins/discover/public/application/main/components/layout/discover_documents.tsx

@@ -156,17 +156,6 @@ function DiscoverDocumentsComponent({
     documentState.fetchStatus === FetchStatus.LOADING ||
     documentState.fetchStatus === FetchStatus.PARTIAL;
 
-  // This is needed to prevent EuiDataGrid pushing onSort because the data view has been switched.
-  // It's just necessary for non ES|QL requests since they don't have a partial result state, that's
-  // considered as loading state in the Component.
-  // 1. When switching the data view, the sorting in the URL is reset to the default sorting of the selected data view.
-  // 2. The new sort param is already available in this component and propagated to the EuiDataGrid.
-  // 3. currentColumns are still referring to the old state
-  // 4. since the new sort by field isn't available in currentColumns EuiDataGrid is emitting a 'onSort', which is unsorting the grid
-  // 5. this is propagated to Discover's URL and causes an unwanted change of state to an unsorted state
-  // This solution switches to the loading state in this component when the URL index doesn't match the dataView.id
-  const isDataViewLoading =
-    useInternalStateSelector((state) => state.isDataViewLoading) && !isEsqlMode;
   const isEmptyDataResult =
     isEsqlMode || !documentState.result || documentState.result.length === 0;
   const rows = useMemo(() => documentState.result || [], [documentState.result]);
@@ -404,7 +393,7 @@ function DiscoverDocumentsComponent({
     [viewModeToggle, callouts, loadingIndicator]
   );
 
-  if (isDataViewLoading || (isEmptyDataResult && isDataLoading)) {
+  if (isEmptyDataResult && isDataLoading) {
     return (
       <div className="dscDocuments__loading">
         <EuiText size="xs" color="subdued">

src/plugins/discover/public/application/main/components/layout/discover_layout.tsx

@@ -9,14 +9,7 @@
 
 import './discover_layout.scss';
 import React, { ReactElement, useCallback, useEffect, useMemo, useRef, useState } from 'react';
-import {
-  EuiPage,
-  EuiPageBody,
-  EuiPanel,
-  EuiProgress,
-  useEuiBackgroundColor,
-  EuiDelayRender,
-} from '@elastic/eui';
+import { EuiPage, EuiPageBody, EuiPanel, useEuiBackgroundColor } from '@elastic/eui';
 import { css } from '@emotion/react';
 import { i18n } from '@kbn/i18n';
 import { isOfAggregateQueryType } from '@kbn/es-query';
@@ -101,10 +94,7 @@ export function DiscoverLayout({ stateContainer }: DiscoverLayoutProps) {
     }
     return state.viewMode ?? VIEW_MODE.DOCUMENT_LEVEL;
   });
-  const [dataView, dataViewLoading] = useInternalStateSelector((state) => [
-    state.dataView!,
-    state.isDataViewLoading,
-  ]);
+  const [dataView] = useInternalStateSelector((state) => [state.dataView!]);
   const customFilters = useInternalStateSelector((state) => state.customFilters);
 
   const dataState: DataMainMsg = useDataState(main$);
@@ -276,12 +266,9 @@ export function DiscoverLayout({ stateContainer }: DiscoverLayoutProps) {
       if (removedFieldName && currentColumns.includes(removedFieldName)) {
         onRemoveColumn(removedFieldName);
       }
-      if (!dataView.isPersisted()) {
-        await stateContainer.actions.updateAdHocDataViewId();
-      }
-      stateContainer.dataState.refetch$.next('reset');
+      await stateContainer.actions.onDataViewFieldEdited();
     },
-    [dataView, stateContainer, currentColumns, onRemoveColumn]
+    [stateContainer, currentColumns, onRemoveColumn]
   );
 
   const onDisableFilters = useCallback(() => {
@@ -423,11 +410,6 @@ export function DiscoverLayout({ stateContainer }: DiscoverLayoutProps) {
             height: 100%;
           `}
         >
-          {dataViewLoading && (
-            <EuiDelayRender delay={300}>
-              <EuiProgress size="xs" color="accent" position="absolute" />
-            </EuiDelayRender>
-          )}
           <SavedSearchURLConflictCallout
             savedSearch={savedSearch}
             spaces={spaces}

src/plugins/discover/public/application/main/components/top_nav/on_save_search.tsx

@@ -68,7 +68,7 @@ async function saveDataSource({
   }
 
   try {
-    const response = await state.savedSearchState.persist(savedSearch, saveOptions);
+    const response = await state.actions.persistSavedSearch(savedSearch, saveOptions);
     if (response?.id) {
       onSuccess(response.id!);
     }
@@ -156,10 +156,6 @@ export async function onSaveSearch({
       isTitleDuplicateConfirmed,
     };
 
-    if (newCopyOnSave) {
-      await state.actions.updateAdHocDataViewId();
-    }
-
     const navigateOrReloadSavedSearch = !Boolean(onSaveCb);
     const response = await saveDataSource({
       saveOptions,

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,

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<Error>();
-  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 <DiscoverMainAppMemoized stateContainer={stateContainer} />;
   }, [
-    loading,
-    loadingIndicator,
     noDataDependencies,
     onDataViewCreated,
     onESQLNavigationComplete,
@@ -355,11 +349,12 @@ export function DiscoverMainRoute({
     <DiscoverCustomizationProvider value={customizationService}>
       <DiscoverMainProvider value={stateContainer}>
         <rootProfileState.AppWrapper>
-          <DiscoverTopNavInline
+          <DiscoverMainLoading
+            mainContent={mainContent}
+            showNoDataPage={noDataState.showNoDataPage}
             stateContainer={stateContainer}
-            hideNavMenuItems={loading || noDataState.showNoDataPage}
+            loadingIndicator={loadingIndicator}
           />
-          {mainContent}
         </rootProfileState.AppWrapper>
       </DiscoverMainProvider>
     </DiscoverCustomizationProvider>
@@ -368,6 +363,30 @@ export function DiscoverMainRoute({
 // eslint-disable-next-line import/no-default-export
 export default DiscoverMainRoute;
 
+export function DiscoverMainLoading({
+  stateContainer,
+  showNoDataPage,
+  mainContent,
+  loadingIndicator,
+}: {
+  stateContainer: DiscoverStateContainer;
+  showNoDataPage: boolean;
+  mainContent: React.ReactNode;
+  loadingIndicator: React.ReactNode;
+}) {
+  const loading = useInternalStateSelector((state) => state.isLoading);
+
+  return (
+    <>
+      <DiscoverTopNavInline
+        stateContainer={stateContainer}
+        hideNavMenuItems={showNoDataPage || loading}
+      />
+      {loading && !showNoDataPage ? loadingIndicator : mainContent}
+    </>
+  );
+}
+
 function getLoadParamsForNewSearch(stateContainer: DiscoverStateContainer): {
   nextDataView: LoadParams['dataView'];
   initialAppState: LoadParams['initialAppState'];

src/plugins/discover/public/application/main/state_management/discover_internal_state_container.ts

@@ -19,7 +19,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;
@@ -31,7 +31,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: (
@@ -71,7 +71,7 @@ export function getInternalStateContainer() {
   return createStateContainer<InternalState, InternalStateTransitions, {}>(
     {
       dataView: undefined,
-      isDataViewLoading: false,
+      isLoading: true,
       adHocDataViews: [],
       savedDataViews: [],
       expandedDoc: undefined,
@@ -86,9 +86,9 @@ export function getInternalStateContainer() {
         expandedDoc:
           nextDataView?.id !== prevState.dataView?.id ? undefined : prevState.expandedDoc,
       }),
-      setIsDataViewLoading: (prevState: InternalState) => (loading: boolean) => ({
+      setIsLoading: (prevState: InternalState) => (isLoading: boolean) => ({
         ...prevState,
-        isDataViewLoading: loading,
+        isLoading,
       }),
       setIsESQLToDataViewTransitionModalVisible:
         (prevState: InternalState) => (isVisible: boolean) => ({

src/plugins/discover/public/application/main/state_management/discover_state.test.ts

@@ -851,10 +851,8 @@ describe('Test discover state actions', () => {
     });
     const unsubscribe = state.actions.initializeAndSync();
     await state.actions.onDataViewEdited(dataViewMock);
-
-    await waitFor(() => {
-      expect(state.internalState.getState().dataView).not.toBe(selectedDataView);
-    });
+    expect(state.internalState.getState().dataView?.id).toBe(dataViewMock.id);
+    expect(state.dataState.fetch).toHaveBeenCalledTimes(1);
     unsubscribe();
   });
   test('onDataViewEdited - ad-hoc data view', async () => {
@@ -866,6 +864,7 @@ describe('Test discover state actions', () => {
     await waitFor(() => {
       expect(state.internalState.getState().dataView?.id).not.toBe(previousId);
     });
+    expect(state.dataState.fetch).toHaveBeenCalledTimes(1);
     unsubscribe();
   });
 
@@ -956,6 +955,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', () => {