[v10.0.x] Explore: Remove storing derived "loading" property (#70394)

Explore: Remove storing derived "loading" property (#70324)

* Remove storing derived state (loading property) and use a selector instead

* Remove redundant tests

There's no way to change the interval while live streaming

* Remove check for isLive when deriving waiting for data

It was introduced in #18804

* Remove unused props

(cherry picked from commit a94ba7784a)

public/app/features/alerting/unified/components/rule-editor/VizWrapper.tsx

@@ -3,7 +3,7 @@ import React from 'react';
 import AutoSizer from 'react-virtualized-auto-sizer';
 
 import { GrafanaTheme2, isTimeSeriesFrames, PanelData, ThresholdsConfig } from '@grafana/data';
-import { GraphTresholdsStyleMode, LoadingState } from '@grafana/schema';
+import { GraphTresholdsStyleMode } from '@grafana/schema';
 import { useStyles2 } from '@grafana/ui';
 import appEvents from 'app/core/app_events';
 import { GraphContainer } from 'app/features/explore/Graph/GraphContainer';
@@ -37,7 +37,6 @@ export const VizWrapper = ({ data, thresholds, thresholdsType }: Props) => {
           <div style={{ width }}>
             {isTimeSeriesData ? (
               <GraphContainer
-                loading={data.state === LoadingState.Loading}
                 statusMessage={statusMessage}
                 data={data.series}
                 eventBus={appEvents}

public/app/features/explore/Explore.tsx

@@ -63,6 +63,7 @@ import {
   modifyQueries,
   scanStart,
   scanStopAction,
+  selectIsWaitingForData,
   setQueries,
   setSupplementaryQueryEnabled,
 } from './state/query';
@@ -297,11 +298,10 @@ export class Explore extends React.PureComponent<Props, ExploreState> {
   }
 
   renderGraphPanel(width: number) {
-    const { graphResult, absoluteRange, timeZone, queryResponse, loading, showFlameGraph } = this.props;
+    const { graphResult, absoluteRange, timeZone, queryResponse, showFlameGraph } = this.props;
 
     return (
       <GraphContainer
-        loading={loading}
         data={graphResult!}
         height={showFlameGraph ? 180 : 400}
         width={width}
@@ -565,12 +565,12 @@ function mapStateToProps(state: StoreState, { exploreId }: ExploreProps) {
     queryResponse,
     showNodeGraph,
     showFlameGraph,
-    loading,
     isFromCompactUrl,
     showRawPrometheus,
     supplementaryQueries,
   } = item;
 
+  const loading = selectIsWaitingForData(exploreId)(state);
   const logsSample = supplementaryQueries[SupplementaryQueryType.LogsSample];
   // We want to show logs sample only if there are no log results and if there is already graph or table result
   const showLogsSample = !!(logsSample.dataProvider !== undefined && !logsResult && (graphResult || tableResult));

public/app/features/explore/ExploreQueryInspector.tsx

@@ -12,7 +12,7 @@ import { InspectStatsTab } from 'app/features/inspector/InspectStatsTab';
 import { QueryInspector } from 'app/features/inspector/QueryInspector';
 import { StoreState, ExploreItemState, ExploreId } from 'app/types';
 
-import { runQueries } from './state/query';
+import { runQueries, selectIsWaitingForData } from './state/query';
 
 interface DispatchProps {
   width: number;
@@ -91,10 +91,10 @@ export function ExploreQueryInspector(props: Props) {
 function mapStateToProps(state: StoreState, { exploreId }: { exploreId: ExploreId }) {
   const explore = state.explore;
   const item: ExploreItemState = explore[exploreId]!;
-  const { loading, queryResponse } = item;
+  const { queryResponse } = item;
 
   return {
-    loading,
+    loading: selectIsWaitingForData(exploreId)(state),
     queryResponse,
   };
 }

public/app/features/explore/ExploreToolbar.tsx

@@ -22,7 +22,7 @@ import { ExploreTimeControls } from './ExploreTimeControls';
 import { LiveTailButton } from './LiveTailButton';
 import { changeDatasource } from './state/datasource';
 import { splitClose, splitOpen, maximizePaneAction, evenPaneResizeAction } from './state/main';
-import { cancelQueries, runQueries } from './state/query';
+import { cancelQueries, runQueries, selectIsWaitingForData } from './state/query';
 import { isSplit } from './state/selectors';
 import { syncTimes, changeRefreshInterval } from './state/time';
 import { LiveTailControls } from './useLiveTailControls';
@@ -49,21 +49,14 @@ export function ExploreToolbar({ exploreId, topOfViewRef, onChangeTime }: Props)
   const splitted = useSelector(isSplit);
   const timeZone = useSelector((state: StoreState) => getTimeZone(state.user));
   const fiscalYearStartMonth = useSelector((state: StoreState) => getFiscalYearStartMonth(state.user));
-  const { refreshInterval, loading, datasourceInstance, range, isLive, isPaused, syncedTimes } = useSelector(
+  const { refreshInterval, datasourceInstance, range, isLive, isPaused, syncedTimes } = useSelector(
     (state: StoreState) => ({
-      ...pick(
-        state.explore[exploreId]!,
-        'refreshInterval',
-        'loading',
-        'datasourceInstance',
-        'range',
-        'isLive',
-        'isPaused'
-      ),
+      ...pick(state.explore[exploreId]!, 'refreshInterval', 'datasourceInstance', 'range', 'isLive', 'isPaused'),
       syncedTimes: state.explore.syncedTimes,
     }),
     shallowEqual
   );
+  const loading = useSelector(selectIsWaitingForData(exploreId));
   const isLargerPane = useSelector((state: StoreState) => state.explore.largerExploreId === exploreId);
   const showSmallTimePicker = useSelector((state) => splitted || state.explore[exploreId]!.containerWidth < 1210);
   const showSmallDataSourcePicker = useSelector(

public/app/features/explore/Graph/GraphContainer.tsx

@@ -19,7 +19,6 @@ import { ExploreGraphLabel } from './ExploreGraphLabel';
 import { loadGraphStyle } from './utils';
 
 interface Props extends Pick<PanelChromeProps, 'width' | 'height' | 'statusMessage'> {
-  loading: boolean;
   data: DataFrame[];
   annotations?: DataFrame[];
   eventBus: EventBus;

public/app/features/explore/LogsContainer.tsx

@@ -27,7 +27,13 @@ import { getTimeZone } from '../profile/state/selectors';
 
 import { LiveLogsWithTheme } from './LiveLogs';
 import { Logs } from './Logs';
-import { addResultsToCache, clearCache, loadSupplementaryQueryData, setSupplementaryQueryEnabled } from './state/query';
+import {
+  addResultsToCache,
+  clearCache,
+  loadSupplementaryQueryData,
+  selectIsWaitingForData,
+  setSupplementaryQueryEnabled,
+} from './state/query';
 import { updateTimeRange } from './state/time';
 import { LiveTailControls } from './useLiveTailControls';
 import { LogsCrossFadeTransition } from './utils/LogsCrossFadeTransition';
@@ -208,13 +214,12 @@ class LogsContainer extends PureComponent<LogsContainerProps> {
   }
 }
 
-function mapStateToProps(state: StoreState, { exploreId }: { exploreId: string }) {
+function mapStateToProps(state: StoreState, { exploreId }: { exploreId: ExploreId }) {
   const explore = state.explore;
   // @ts-ignore
   const item: ExploreItemState = explore[exploreId];
   const {
     logsResult,
-    loading,
     scanning,
     datasourceInstance,
     isLive,
@@ -224,6 +229,7 @@ function mapStateToProps(state: StoreState, { exploreId }: { exploreId: string }
     absoluteRange,
     supplementaryQueries,
   } = item;
+  const loading = selectIsWaitingForData(exploreId)(state);
   const timeZone = getTimeZone(state.user);
   const logsVolume = supplementaryQueries[SupplementaryQueryType.LogsVolume];
 

public/app/features/explore/RawPrometheusContainer.tsx

@@ -12,6 +12,7 @@ import { ExploreId, ExploreItemState, TABLE_RESULTS_STYLES, TableResultsStyle }
 
 import { MetaInfoText } from './MetaInfoText';
 import RawListContainer from './PrometheusListView/RawListContainer';
+import { selectIsWaitingForData } from './state/query';
 import { getFieldLinksForExplore } from './utils/links';
 
 interface RawPrometheusContainerProps {
@@ -31,7 +32,8 @@ interface PrometheusContainerState {
 function mapStateToProps(state: StoreState, { exploreId }: RawPrometheusContainerProps) {
   const explore = state.explore;
   const item: ExploreItemState = explore[exploreId]!;
-  const { loading: loadingInState, tableResult, rawPrometheusResult, range } = item;
+  const { tableResult, rawPrometheusResult, range } = item;
+  const loadingInState = selectIsWaitingForData(exploreId)(state);
   const rawPrometheusFrame: DataFrame[] = rawPrometheusResult ? [rawPrometheusResult] : [];
   const result = (tableResult?.length ?? false) > 0 && rawPrometheusResult ? tableResult : rawPrometheusFrame;
   const loading = result && result.length > 0 ? false : loadingInState;

public/app/features/explore/TableContainer.tsx

@@ -8,6 +8,7 @@ import { StoreState } from 'app/types';
 import { ExploreId, ExploreItemState } from 'app/types/explore';
 
 import { MetaInfoText } from './MetaInfoText';
+import { selectIsWaitingForData } from './state/query';
 import { getFieldLinksForExplore } from './utils/links';
 
 interface TableContainerProps {
@@ -23,7 +24,8 @@ function mapStateToProps(state: StoreState, { exploreId }: TableContainerProps)
   const explore = state.explore;
   // @ts-ignore
   const item: ExploreItemState = explore[exploreId];
-  const { loading: loadingInState, tableResult, range } = item;
+  const { tableResult, range } = item;
+  const loadingInState = selectIsWaitingForData(exploreId);
   const loading = tableResult && tableResult.length > 0 ? false : loadingInState;
   return { loading, tableResult, range };
 }

public/app/features/explore/state/datasource.test.ts

@@ -37,7 +37,6 @@ describe('Datasource reducer', () => {
       graphResult: null,
       logsResult: null,
       tableResult: null,
-      loading: false,
       queryResponse: {
         // When creating an empty query response we also create a timeRange object with the current time.
         // Copying the range from the reducer here prevents intermittent failures when creating them at different times.

public/app/features/explore/state/datasource.ts

@@ -103,7 +103,6 @@ export const datasourceReducer = (state: ExploreItemState, action: AnyAction): E
       logsResult: null,
       supplementaryQueries: loadSupplementaryQueries(),
       queryResponse: createEmptyQueryResponse(),
-      loading: false,
       queryKeys: [],
       history,
       datasourceMissing: false,

public/app/features/explore/state/query.ts

@@ -43,6 +43,7 @@ import {
   ExploreItemState,
   ExplorePanelData,
   QueryTransaction,
+  StoreState,
   ThunkDispatch,
   ThunkResult,
 } from 'app/types';
@@ -53,15 +54,31 @@ import { createErrorNotification } from '../../../core/copy/appNotification';
 import { runRequest } from '../../query/state/runRequest';
 import { decorateData } from '../utils/decorators';
 import {
+  getSupplementaryQueryProvider,
   storeSupplementaryQueryEnabled,
   supplementaryQueryTypes,
-  getSupplementaryQueryProvider,
 } from '../utils/supplementaryQueries';
 
 import { addHistoryItem, historyUpdatedAction, loadRichHistory } from './history';
 import { stateSave } from './main';
 import { updateTime } from './time';
-import { createCacheKey, getResultsFromCache, filterLogRowsByIndex } from './utils';
+import { createCacheKey, filterLogRowsByIndex, getResultsFromCache } from './utils';
+
+/**
+ * Derives from explore state if a given Explore pane is waiting for more data to be received
+ */
+export const selectIsWaitingForData = (exploreId: ExploreId) => {
+  return (state: StoreState) => {
+    const panelState = state.explore[exploreId];
+    if (!panelState) {
+      return false;
+    }
+    return panelState.queryResponse
+      ? panelState.queryResponse.state === LoadingState.Loading ||
+          panelState.queryResponse.state === LoadingState.Streaming
+      : false;
+  };
+};
 
 //
 // Actions and Payloads
@@ -897,7 +914,10 @@ export const queryReducer = (state: ExploreItemState, action: AnyAction): Explor
 
     return {
       ...state,
-      loading: false,
+      queryResponse: {
+        ...state.queryResponse,
+        state: LoadingState.Done,
+      },
     };
   }
 
@@ -1043,7 +1063,6 @@ export const queryReducer = (state: ExploreItemState, action: AnyAction): Explor
         ...state.queryResponse,
         state: loadingState,
       },
-      loading: loadingState === LoadingState.Loading || loadingState === LoadingState.Streaming,
     };
   }
 
@@ -1159,7 +1178,6 @@ export const processQueryResponse = (
   const { response } = action.payload;
   const {
     request,
-    state: loadingState,
     series,
     error,
     graphResult,
@@ -1173,14 +1191,8 @@ export const processQueryResponse = (
   } = response;
 
   if (error) {
-    if (error.type === DataQueryErrorType.Timeout) {
-      return {
-        ...state,
-        queryResponse: response,
-        loading: loadingState === LoadingState.Loading || loadingState === LoadingState.Streaming,
-      };
-    } else if (error.type === DataQueryErrorType.Cancelled) {
-      return state;
+    if (error.type === DataQueryErrorType.Timeout || error.type === DataQueryErrorType.Cancelled) {
+      return { ...state };
     }
 
     // Send error to Angular editors
@@ -1211,7 +1223,6 @@ export const processQueryResponse = (
       state.isLive && logsResult
         ? { ...logsResult, rows: filterLogRowsByIndex(state.clearedAtIndex, logsResult.rows) }
         : logsResult,
-    loading: loadingState === LoadingState.Loading || loadingState === LoadingState.Streaming,
     showLogs: !!logsResult,
     showMetrics: !!graphResult,
     showTable: !!tableResult?.length,

public/app/features/explore/state/time.test.ts

@@ -1,14 +1,13 @@
 import { reducerTester } from 'test/core/redux/reducerTester';
 
-import { dateTime, LoadingState } from '@grafana/data';
+import { dateTime } from '@grafana/data';
 import { configureStore } from 'app/store/configureStore';
 import { ExploreId, ExploreItemState } from 'app/types/explore';
 
 import { silenceConsoleOutput } from '../../../../test/core/utils/silenceConsoleOutput';
 
 import { createDefaultInitialState } from './helpers';
-import { changeRangeAction, changeRefreshIntervalAction, timeReducer, updateTime } from './time';
-import { makeExplorePaneState } from './utils';
+import { changeRangeAction, timeReducer, updateTime } from './time';
 
 const MOCK_TIME_RANGE = {};
 
@@ -43,50 +42,6 @@ describe('Explore item reducer', () => {
     });
   });
 
-  describe('changing refresh intervals', () => {
-    it("should result in 'streaming' state, when live-tailing is active", () => {
-      const initialState = makeExplorePaneState();
-      const expectedState = {
-        ...initialState,
-        refreshInterval: 'LIVE',
-        isLive: true,
-        loading: true,
-        logsResult: {
-          hasUniqueLabels: false,
-          rows: [],
-        },
-        queryResponse: {
-          ...initialState.queryResponse,
-          state: LoadingState.Streaming,
-        },
-      };
-      reducerTester<ExploreItemState>()
-        .givenReducer(timeReducer, initialState)
-        .whenActionIsDispatched(changeRefreshIntervalAction({ exploreId: ExploreId.left, refreshInterval: 'LIVE' }))
-        .thenStateShouldEqual(expectedState);
-    });
-
-    it("should result in 'done' state, when live-tailing is stopped", () => {
-      const initialState = makeExplorePaneState();
-      const expectedState = {
-        ...initialState,
-        refreshInterval: '',
-        logsResult: {
-          hasUniqueLabels: false,
-          rows: [],
-        },
-        queryResponse: {
-          ...initialState.queryResponse,
-          state: LoadingState.Done,
-        },
-      };
-      reducerTester<ExploreItemState>()
-        .givenReducer(timeReducer, initialState)
-        .whenActionIsDispatched(changeRefreshIntervalAction({ exploreId: ExploreId.left, refreshInterval: '' }))
-        .thenStateShouldEqual(expectedState);
-    });
-  });
-
   describe('changing range', () => {
     describe('when changeRangeAction is dispatched', () => {
       it('then it should set correct state', () => {

public/app/features/explore/state/time.ts

@@ -182,7 +182,6 @@ export const timeReducer = (state: ExploreItemState, action: AnyAction): Explore
       },
       isLive: live,
       isPaused: live ? false : state.isPaused,
-      loading: live,
       logsResult,
     };
   }

public/app/features/explore/state/utils.ts

@@ -51,7 +51,6 @@ export const makeExplorePaneState = (): ExploreItemState => ({
     to: null,
   } as any,
   scanning: false,
-  loading: false,
   queryKeys: [],
   isLive: false,
   isPaused: false,

public/app/types/explore.ts

@@ -139,7 +139,6 @@ export interface ExploreItemState {
    */
   scanRange?: RawTimeRange;
 
-  loading: boolean;
   /**
    * Table model that combines all query table results into a single table.
    */