Fix flaky histogram when an adhoc data view is being edited

src/plugins/discover/public/application/main/components/layout/use_discover_histogram.ts

@@ -42,13 +42,19 @@ 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';
 
 const EMPTY_ESQL_COLUMNS: DatatableColumn[] = [];
 const EMPTY_FILTERS: Filter[] = [];
 
+// enabled for debugging
+if (window) {
+  // @ts-ignore
+  window.ELASTIC_DISCOVER_LOGGER = `debug`;
+}
+
 export interface UseDiscoverHistogramProps {
   stateContainer: DiscoverStateContainer;
   inspectorAdapters: InspectorAdapters;
@@ -180,17 +186,45 @@ export const useDiscoverHistogram = ({
           totalHitsResult &&
           typeof result !== 'number'
         ) {
-          // ignore the histogram initial loading state if discover state already has a total hits value
+          addLog(
+            '[UnifiedHistogram] ignore the histogram initial loading state if discover state already has a total hits value',
+            { status, result }
+          );
           return;
         }
 
+        const fetchStatus = status.toString() as FetchStatus;
+        if (
+          fetchStatus === FetchStatus.COMPLETE &&
+          savedSearchData$.totalHits$.getValue().fetchStatus === FetchStatus.COMPLETE &&
+          result !== savedSearchData$.totalHits$.getValue().result
+        ) {
+          // this is a workaround to make sure the new total hits value is displayed
+          // a different value without a loading state in between would lead to be ignored by useDataState
+          addLog(
+            '[UnifiedHistogram] send loading state to total hits$ to make sure the new value is displayed',
+            { status, result }
+          );
+          savedSearchData$.totalHits$.next({
+            fetchStatus: FetchStatus.LOADING,
+          });
+        }
+
         // Sync the totalHits$ observable with the unified histogram state
         savedSearchData$.totalHits$.next({
-          fetchStatus: status.toString() as FetchStatus,
+          fetchStatus,
           result,
         });
 
-        if (status !== UnifiedHistogramFetchStatus.complete || typeof result !== 'number') {
+        if (
+          (status !== UnifiedHistogramFetchStatus.complete &&
+            status !== UnifiedHistogramFetchStatus.partial) ||
+          typeof result !== 'number'
+        ) {
+          addLog(
+            '[UnifiedHistogram] ignore the histogram complete/partial state if discover state already has a total hits value',
+            { status, result }
+          );
           return;
         }
 

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';
@@ -46,15 +45,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!);
@@ -122,23 +114,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) {
@@ -221,14 +196,13 @@ export const DiscoverTopNav = ({
       textBasedLanguages: supportedTextBasedLanguages,
       adHocDataViews,
       savedDataViews,
-      onEditDataView,
+      onEditDataView: stateContainer.actions.onDataViewEdited,
     };
   }, [
     adHocDataViews,
     addField,
     createNewDataView,
     dataView,
-    onEditDataView,
     savedDataViews,
     stateContainer,
     uiSettings,

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 [hasESData, setHasESData] = useState(false);
   const [hasUserDataView, setHasUserDataView] = useState(false);
   const [showNoDataPage, setShowNoDataPage] = useState<boolean>(false);
@@ -156,9 +156,9 @@ export function DiscoverMainRoute({
       initialAppState,
     }: { nextDataView?: DataView; initialAppState?: LoadParams['initialAppState'] } = {}) => {
       const loadSavedSearchStartTime = window.performance.now();
-      setLoading(true);
+      stateContainer.internalState.transitions.setIsLoading(true);
       if (!nextDataView && !(await checkData())) {
-        setLoading(false);
+        stateContainer.internalState.transitions.setIsLoading(false);
         return;
       }
       try {
@@ -181,7 +181,7 @@ export function DiscoverMainRoute({
 
           setBreadcrumbs({ services, titleBreadcrumbText: currentSavedSearch?.title ?? undefined });
         }
-        setLoading(false);
+        stateContainer.internalState.transitions.setIsLoading(false);
         if (services.analytics) {
           const loadSavedSearchDuration = window.performance.now() - loadSavedSearchStartTime;
           reportPerformanceMetricEvent(services.analytics, {
@@ -215,7 +215,7 @@ export function DiscoverMainRoute({
     },
     [
       checkData,
-      stateContainer.actions,
+      stateContainer,
       savedSearchId,
       historyLocationState?.dataViewSpec,
       customizationContext.displayMode,
@@ -231,8 +231,7 @@ export function DiscoverMainRoute({
 
   useEffect(() => {
     if (!isCustomizationServiceInitialized) return;
-
-    setLoading(true);
+    stateContainer.internalState.transitions.setIsLoading(true);
     setHasESData(false);
     setHasUserDataView(false);
     setShowNoDataPage(false);
@@ -258,13 +257,13 @@ export function DiscoverMainRoute({
   const onDataViewCreated = useCallback(
     async (nextDataView: unknown) => {
       if (nextDataView) {
-        setLoading(true);
+        stateContainer.internalState.transitions.setIsLoading(true);
         setShowNoDataPage(false);
         setError(undefined);
         await loadSavedSearch({ nextDataView: nextDataView as DataView });
       }
     },
-    [loadSavedSearch]
+    [loadSavedSearch, stateContainer]
   );
 
   const onESQLNavigationComplete = useCallback(async () => {
@@ -325,14 +324,8 @@ export function DiscoverMainRoute({
       );
     }
 
-    if (loading) {
-      return loadingIndicator;
-    }
-
     return <DiscoverMainAppMemoized stateContainer={stateContainer} />;
   }, [
-    loading,
-    loadingIndicator,
     noDataDependencies,
     onDataViewCreated,
     onESQLNavigationComplete,
@@ -354,20 +347,40 @@ export function DiscoverMainRoute({
   return (
     <DiscoverCustomizationProvider value={customizationService}>
       <DiscoverMainProvider value={stateContainer}>
-        <>
-          <DiscoverTopNavInline
-            stateContainer={stateContainer}
-            hideNavMenuItems={loading || showNoDataPage}
-          />
-          {mainContent}
-        </>
+        <DiscoverMainLoading
+          mainContent={mainContent}
+          showNoDataPage={showNoDataPage}
+          stateContainer={stateContainer}
+        />
       </DiscoverMainProvider>
     </DiscoverCustomizationProvider>
   );
 }
 // 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 (
+    <>
+      <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,6 +19,7 @@ import type { UnifiedHistogramVisContext } from '@kbn/unified-histogram-plugin/p
 
 export interface InternalState {
   dataView: DataView | undefined;
+  isLoading: boolean;
   isDataViewLoading: boolean;
   savedDataViews: DataViewListItem[];
   adHocDataViews: DataView[];
@@ -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: (
@@ -71,6 +73,7 @@ export function getInternalStateContainer() {
   return createStateContainer<InternalState, InternalStateTransitions, {}>(
     {
       dataView: undefined,
+      isLoading: true,
       isDataViewLoading: false,
       adHocDataViews: [],
       savedDataViews: [],
@@ -90,6 +93,10 @@ export function getInternalStateContainer() {
         ...prevState,
         isDataViewLoading: loading,
       }),
+      setIsLoading: (prevState: InternalState) => (isLoading: boolean) => ({
+        ...prevState,
+        isLoading,
+      }),
       setIsESQLToDataViewTransitionModalVisible:
         (prevState: InternalState) => (isVisible: boolean) => ({
           ...prevState,

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

@@ -408,16 +408,19 @@ export function getDiscoverStateContainer({
   };
 
   const onDataViewEdited = async (editedDataView: DataView) => {
+    internalStateContainer.transitions.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();
   };
 

test/functional/apps/discover/group3/_lens_vis.ts

@@ -110,8 +110,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
     return seriesType;
   }
 
-  // Failing: See 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');