Improve code

elastic · Feb 20, 2024 · 2b96dfc · 2b96dfc
1 parent 96074a2
commit 2b96dfc
Show file tree

Hide file tree

Showing 6 changed files with 29 additions and 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
@@ -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' && <LoadingSpinner />}
+        {(resultState === 'loading' || dataViewLoading) && <LoadingSpinner />}
       </>
     );
   }, [
     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
@@ -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<PureTransitionsToTransitions<InternalStateTransitions>>;
   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
@@ -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
@@ -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<InternalState, InternalStateTransitions, {}>(
     {
       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
@@ -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
@@ -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,