Explore: Cleanup redundant state variables and unused actions (#20837)

c4c031ef · Andrej Ocenas · GitHub · 880fbcb0 · c4c031ef · c4c031ef
Commit c4c031ef authored Dec 05, 2019 by Andrej Ocenas Committed by GitHub Dec 05, 2019
9 changed files
--- a/public/app/features/explore/Explore.tsx
+++ b/public/app/features/explore/Explore.tsx
 // Libraries
-import React, { ComponentType } from 'react';
+import React from 'react';
 import { hot } from 'react-hot-loader';
 import { css } from 'emotion';
 import { connect } from 'react-redux';
@@ -27,13 +27,13 @@ import {
 // Types
 import {
  DataQuery,
-  ExploreStartPageProps,
  DataSourceApi,
  PanelData,
  RawTimeRange,
  GraphSeriesXY,
  TimeZone,
  AbsoluteTimeRange,
+  LoadingState,
 } from '@grafana/data';
 import {
@@ -71,7 +71,6 @@ const getStyles = memoizeOne(() => {
 });
 interface ExploreProps {
-  StartPage?: ComponentType<ExploreStartPageProps>;
  changeSize: typeof changeSize;
  datasourceInstance: DataSourceApi;
  datasourceMissing: boolean;
@@ -87,7 +86,6 @@ interface ExploreProps {
  scanStopAction: typeof scanStopAction;
  setQueries: typeof setQueries;
  split: boolean;
-  showingStartPage?: boolean;
  queryKeys: string[];
  initialDatasource: string;
  initialQueries: DataQuery[];
@@ -251,11 +249,9 @@ export class Explore extends React.PureComponent<ExploreProps> {
  render() {
    const {
-      StartPage,
      datasourceInstance,
      datasourceMissing,
      exploreId,
-      showingStartPage,
      split,
      queryKeys,
      mode,
@@ -270,6 +266,8 @@ export class Explore extends React.PureComponent<ExploreProps> {
    } = this.props;
    const exploreClass = split ? 'explore explore-split' : 'explore';
    const styles = getStyles();
+    const StartPage = datasourceInstance?.components?.ExploreStartPage;
+    const showStartPage = !queryResponse || queryResponse.state === LoadingState.NotStarted;
    return (
      <div className={exploreClass} ref={this.getRef}>
@@ -288,7 +286,7 @@ export class Explore extends React.PureComponent<ExploreProps> {
                return (
                  <main className={`m-t-2 ${styles.logsMain}`} style={{ width }}>
                    <ErrorBoundaryAlert>
-                      {showingStartPage && StartPage && (
+                      {showStartPage && StartPage && (
                        <div className="grafana-info-box grafana-info-box--max-lg">
                          <StartPage
                            onClickExample={this.onClickExample}
@@ -297,7 +295,7 @@ export class Explore extends React.PureComponent<ExploreProps> {
                          />
                        </div>
                      )}
-                      {!showingStartPage && (
+                      {!showStartPage && (
                        <>
                          {mode === ExploreMode.Metrics && (
                            <ExploreGraphPanel
@@ -353,11 +351,9 @@ function mapStateToProps(state: StoreState, { exploreId }: ExploreProps): Partia
  const item: ExploreItemState = explore[exploreId];
  const timeZone = getTimeZone(state.user);
  const {
-    StartPage,
    datasourceInstance,
    datasourceMissing,
    initialized,
-    showingStartPage,
    queryKeys,
    urlState,
    update,
@@ -398,11 +394,9 @@ function mapStateToProps(state: StoreState, { exploreId }: ExploreProps): Partia
  const initialUI = ui || DEFAULT_UI_STATE;
  return {
-    StartPage,
    datasourceInstance,
    datasourceMissing,
    initialized,
-    showingStartPage,
    split,
    queryKeys,
    update,

--- a/public/app/features/explore/ExploreToolbar.tsx
+++ b/public/app/features/explore/ExploreToolbar.tsx
@@ -8,7 +8,7 @@ import { css } from 'emotion';
 import { ExploreId, ExploreItemState, ExploreMode } from 'app/types/explore';
 import { ToggleButtonGroup, ToggleButton, Tooltip, ButtonSelect, SetInterval } from '@grafana/ui';
-import { RawTimeRange, TimeZone, TimeRange, DataSourceSelectItem, DataQuery } from '@grafana/data';
+import { RawTimeRange, TimeZone, TimeRange, DataQuery } from '@grafana/data';
 import { DataSourcePicker } from 'app/core/components/Select/DataSourcePicker';
 import { StoreState } from 'app/types/store';
 import {
@@ -31,6 +31,7 @@ import { LiveTailButton } from './LiveTailButton';
 import { ResponsiveButton } from './ResponsiveButton';
 import { RunButton } from './RunButton';
 import { LiveTailControls } from './useLiveTailControls';
+import { getExploreDatasources } from './state/selectors';
 const getStyles = memoizeOne(() => {
  return {
@@ -50,11 +51,9 @@ interface OwnProps {
 interface StateProps {
  datasourceMissing: boolean;
-  exploreDatasources: DataSourceSelectItem[];
  loading: boolean;
  range: TimeRange;
  timeZone: TimeZone;
-  selectedDatasource?: DataSourceSelectItem;
  splitted: boolean;
  syncedTimes: boolean;
  refreshInterval?: string;
@@ -67,6 +66,7 @@ interface StateProps {
  queries: DataQuery[];
  datasourceLoading?: boolean;
  containerWidth: number;
+  datasourceName?: string;
 }
 interface DispatchProps {
@@ -137,16 +137,20 @@ export class UnConnectedExploreToolbar extends PureComponent<Props> {
    });
  };
+  getSelectedDatasource = () => {
+    const { datasourceName } = this.props;
+    const exploreDatasources = getExploreDatasources();
+    return datasourceName ? exploreDatasources.find(datasource => datasource.name === datasourceName) : undefined;
+  };
  render() {
    const {
      datasourceMissing,
-      exploreDatasources,
      closeSplit,
      exploreId,
      loading,
      range,
      timeZone,
-      selectedDatasource,
      splitted,
      syncedTimes,
      refreshInterval,
@@ -203,8 +207,8 @@ export class UnConnectedExploreToolbar extends PureComponent<Props> {
                >
                  <DataSourcePicker
                    onChange={this.onChangeDatasource}
-                    datasources={exploreDatasources}
+                    datasources={getExploreDatasources()}
-                    current={selectedDatasource}
+                    current={this.getSelectedDatasource()}
                    showLoading={datasourceLoading}
                    hideTextValue={showSmallDataSourcePicker}
                  />
@@ -331,7 +335,6 @@ const mapStateToProps = (state: StoreState, { exploreId }: OwnProps): StateProps
  const {
    datasourceInstance,
    datasourceMissing,
-    exploreDatasources,
    range,
    refreshInterval,
    loading,
@@ -344,21 +347,15 @@ const mapStateToProps = (state: StoreState, { exploreId }: OwnProps): StateProps
    datasourceLoading,
    containerWidth,
  } = exploreItem;
-  const selectedDatasource = datasourceInstance
-    ? exploreDatasources.find(datasource => datasource.name === datasourceInstance.name)
+  const hasLiveOption = datasourceInstance?.meta?.streaming && mode === ExploreMode.Logs;
-    : undefined;
-  const hasLiveOption =
-    datasourceInstance && datasourceInstance.meta && datasourceInstance.meta.streaming && mode === ExploreMode.Logs
-      ? true
-      : false;
  return {
    datasourceMissing,
-    exploreDatasources,
+    datasourceName: datasourceInstance?.name,
    loading,
    range,
    timeZone: getTimeZone(state.user),
-    selectedDatasource,
    splitted,
    refreshInterval,
    supportedModes,

--- a/public/app/features/explore/state/actionTypes.ts
+++ b/public/app/features/explore/state/actionTypes.ts
@@ -4,7 +4,6 @@ import { Emitter } from 'app/core/core';
 import {
  DataQuery,
-  DataSourceSelectItem,
  DataSourceApi,
  QueryFixAction,
  PanelData,
@@ -81,10 +80,6 @@ export interface ClearOriginPayload {
  exploreId: ExploreId;
 }
-export interface ClearRefreshIntervalPayload {
-  exploreId: ExploreId;
-}
 export interface HighlightLogsExpressionPayload {
  exploreId: ExploreId;
  expressions: string[];
@@ -122,10 +117,6 @@ export interface ModifyQueriesPayload {
  modifier: (query: DataQuery, modification: QueryFixAction) => DataQuery;
 }
-export interface QueryStartPayload {
-  exploreId: ExploreId;
-}
 export interface QueryEndedPayload {
  exploreId: ExploreId;
  response: PanelData;
@@ -199,11 +190,6 @@ export interface QueriesImportedPayload {
  queries: DataQuery[];
 }
-export interface LoadExploreDataSourcesPayload {
-  exploreId: ExploreId;
-  exploreDatasources: DataSourceSelectItem[];
-}
 export interface SetUrlReplacedPayload {
  exploreId: ExploreId;
 }
@@ -312,10 +298,6 @@ export const loadDatasourceReadyAction = actionCreatorFactory<LoadDatasourceRead
 */
 export const modifyQueriesAction = actionCreatorFactory<ModifyQueriesPayload>('explore/MODIFY_QUERIES').create();
-export const queryStartAction = actionCreatorFactory<QueryStartPayload>('explore/QUERY_START').create();
-export const queryEndedAction = actionCreatorFactory<QueryEndedPayload>('explore/QUERY_ENDED').create();
 export const queryStreamUpdatedAction = actionCreatorFactory<QueryEndedPayload>(
  'explore/QUERY_STREAM_UPDATED'
 ).create();
@@ -389,9 +371,6 @@ export const toggleLogLevelAction = actionCreatorFactory<ToggleLogLevelPayload>(
 */
 export const resetExploreAction = actionCreatorFactory<ResetExplorePayload>('explore/RESET_EXPLORE').create();
 export const queriesImportedAction = actionCreatorFactory<QueriesImportedPayload>('explore/QueriesImported').create();
-export const loadExploreDatasources = actionCreatorFactory<LoadExploreDataSourcesPayload>(
-  'explore/LOAD_EXPLORE_DATASOURCES'
-).create();
 export const historyUpdatedAction = actionCreatorFactory<HistoryUpdatedPayload>('explore/HISTORY_UPDATED').create();

--- a/public/app/features/explore/state/actions.test.ts
+++ b/public/app/features/explore/state/actions.test.ts
@@ -107,7 +107,7 @@ describe('refreshExplore', () => {
          .givenThunk(refreshExplore)
          .whenThunkIsDispatched(exploreId);
-        const initializeExplore = dispatchedActions[2] as ActionOf<InitializeExplorePayload>;
+        const initializeExplore = dispatchedActions[1] as ActionOf<InitializeExplorePayload>;
        const { type, payload } = initializeExplore;
        expect(type).toEqual(initializeExploreAction.type);

--- a/public/app/features/explore/state/actions.ts
+++ b/public/app/features/explore/state/actions.ts
@@ -29,7 +29,6 @@ import {
  AbsoluteTimeRange,
  DataQuery,
  DataSourceApi,
-  DataSourceSelectItem,
  dateTimeForTimeZone,
  isDateTime,
  LoadingState,
@@ -57,7 +56,6 @@ import {
  loadDatasourcePendingAction,
  loadDatasourceReadyAction,
  LoadDatasourceReadyPayload,
-  loadExploreDatasources,
  modifyQueriesAction,
  queriesImportedAction,
  queryStoreSubscriptionAction,
@@ -84,6 +82,7 @@ import { getTimeSrv, TimeSrv } from '../../dashboard/services/TimeSrv';
 import { preProcessPanelData, runRequest } from '../../dashboard/state/runRequest';
 import { PanelModel } from 'app/features/dashboard/state';
 import { DataSourceSrv } from '@grafana/runtime';
+import { getExploreDatasources } from './selectors';
 /**
 * Updates UI state and save it to the URL
@@ -244,18 +243,7 @@ export function loadExploreDatasourcesAndSetDatasource(
  datasourceName: string
 ): ThunkResult<void> {
  return dispatch => {
-    const exploreDatasources: DataSourceSelectItem[] = getDatasourceSrv()
+    const exploreDatasources = getExploreDatasources();
-      .getExternal()
-      .map(
-        (ds: any) =>
-          ({
-            value: ds.name,
-            name: ds.name,
-            meta: ds.meta,
-          } as DataSourceSelectItem)
-      );
-    dispatch(loadExploreDatasources({ exploreId, exploreDatasources }));
    if (exploreDatasources.length >= 1) {
      dispatch(changeDatasource(exploreId, datasourceName));
@@ -320,6 +308,14 @@ export const loadDatasourceReady = (
  });
 };
+/**
+ * Import queries from previous datasource if possible eg Loki and Prometheus have similar query language so the
+ * labels part can be reused to get similar data.
+ * @param exploreId
+ * @param queries
+ * @param sourceDataSource
+ * @param targetDataSource
+ */
 export function importQueries(
  exploreId: ExploreId,
  queries: DataQuery[],
@@ -464,6 +460,8 @@ export function runQueries(exploreId: ExploreId): ThunkResult<void> {
          // Side-effect: Saving history in localstorage
          const nextHistory = updateHistory(history, datasourceId, queries);
          dispatch(historyUpdatedAction({ exploreId, history: nextHistory }));
+          // We save queries to the URL here so that only successfully run queries change the URL.
          dispatch(stateSave());
        }
@@ -505,6 +503,10 @@ const toRawTimeRange = (range: TimeRange): RawTimeRange => {
  };
 };
+/**
+ * Save local redux state back to the URL. Should be called when there is some change that should affect the URL.
+ * Not all of the redux state is reflected in URL though.
+ */
 export const stateSave = (): ThunkResult<void> => {
  return (dispatch, getState) => {
    const { left, right, split } = getState().explore;
@@ -714,6 +716,11 @@ export const changeDedupStrategy = (exploreId: ExploreId, dedupStrategy: LogsDed
  };
 };
+/**
+ * Reacts to changes in URL state that we need to sync back to our redux state. Checks the internal update variable
+ * to see which parts change and need to be synced.
+ * @param exploreId
+ */
 export function refreshExplore(exploreId: ExploreId): ThunkResult<void> {
  return (dispatch, getState) => {
    const itemState = getState().explore[exploreId];

--- a/public/app/features/explore/state/reducers.test.ts
+++ b/public/app/features/explore/state/reducers.test.ts
@@ -90,15 +90,11 @@ describe('Explore item reducer', () => {
          const queryKeys: string[] = [];
          const initalState: Partial<ExploreItemState> = {
            datasourceInstance: null,
-            StartPage: null,
-            showingStartPage: false,
            queries,
            queryKeys,
          };
          const expectedState: any = {
            datasourceInstance,
-            StartPage,
-            showingStartPage: true,
            queries,
            queryKeys,
            graphResult: null,

--- a/public/app/features/explore/state/reducers.ts
+++ b/public/app/features/explore/state/reducers.ts
@@ -27,12 +27,10 @@ import {
  ActionTypes,
  splitCloseAction,
  SplitCloseActionPayload,
-  loadExploreDatasources,
  historyUpdatedAction,
  changeModeAction,
  setUrlReplacedAction,
  scanStopAction,
-  queryStartAction,
  changeRangeAction,
  clearOriginAction,
  addQueryRowAction,
@@ -84,13 +82,11 @@ export const makeInitialUpdateState = (): ExploreUpdateState => ({
 * Returns a fresh Explore area state
 */
 export const makeExploreItemState = (): ExploreItemState => ({
-  StartPage: undefined,
  containerWidth: 0,
  datasourceInstance: null,
  requestedDatasourceName: null,
  datasourceLoading: null,
  datasourceMissing: false,
-  exploreDatasources: [],
  history: [],
  queries: [],
  initialized: false,
@@ -229,7 +225,6 @@ export const itemReducer = reducerFactory<ExploreItemState>({} as ExploreItemSta
        graphResult: null,
        tableResult: null,
        logsResult: null,
-        showingStartPage: Boolean(state.StartPage),
        queryKeys: getQueryKeys(queries, state.datasourceInstance),
        queryResponse: createEmptyQueryResponse(),
        loading: false,
@@ -277,7 +272,6 @@ export const itemReducer = reducerFactory<ExploreItemState>({} as ExploreItemSta
      const { datasourceInstance, version } = action.payload;
      // Custom components
-      const StartPage = datasourceInstance.components.ExploreStartPage;
      stopQueryState(state.querySubscription);
      let newMetadata = datasourceInstance.meta;
@@ -308,8 +302,6 @@ export const itemReducer = reducerFactory<ExploreItemState>({} as ExploreItemSta
        latency: 0,
        queryResponse: createEmptyQueryResponse(),
        loading: false,
-        StartPage: datasourceInstance.components.ExploreStartPage,
-        showingStartPage: Boolean(StartPage),
        queryKeys: [],
        supportedModes,
        mode,
@@ -383,22 +375,6 @@ export const itemReducer = reducerFactory<ExploreItemState>({} as ExploreItemSta
    },
  })
  .addMapper({
-    filter: queryStartAction,
-    mapper: (state): ExploreItemState => {
-      return {
-        ...state,
-        latency: 0,
-        queryResponse: {
-          ...state.queryResponse,
-          state: LoadingState.Loading,
-          error: null,
-        },
-        loading: true,
-        update: makeInitialUpdateState(),
-      };
-    },
-  })
-  .addMapper({
    filter: removeQueryRowAction,
    mapper: (state, action): ExploreItemState => {
      const { queries, queryKeys } = state;
@@ -497,15 +473,6 @@ export const itemReducer = reducerFactory<ExploreItemState>({} as ExploreItemSta
    },
  })
  .addMapper({
-    filter: loadExploreDatasources,
-    mapper: (state, action): ExploreItemState => {
-      return {
-        ...state,
-        exploreDatasources: action.payload.exploreDatasources,
-      };
-    },
-  })
-  .addMapper({
    filter: historyUpdatedAction,
    mapper: (state, action): ExploreItemState => {
      return {
@@ -599,7 +566,6 @@ export const processQueryResponse = (
      graphResult: null,
      tableResult: null,
      logsResult: null,
-      showingStartPage: false,
      update: makeInitialUpdateState(),
    };
  }
@@ -625,7 +591,6 @@ export const processQueryResponse = (
    tableResult,
    logsResult,
    loading: loadingState === LoadingState.Loading || loadingState === LoadingState.Streaming,
-    showingStartPage: false,
    update: makeInitialUpdateState(),
  };
 };

--- a/public/app/features/explore/state/selectors.ts
+++ b/public/app/features/explore/state/selectors.ts
 import { createSelector } from 'reselect';
 import { ExploreItemState } from 'app/types';
 import { filterLogLevels, dedupLogRows } from 'app/core/logs_model';
+import { getDatasourceSrv } from '../../plugins/datasource_srv';
+import { DataSourceSelectItem } from '@grafana/data';
 const logsRowsSelector = (state: ExploreItemState) => state.logsResult && state.logsResult.rows;
 const hiddenLogLevelsSelector = (state: ExploreItemState) => state.hiddenLogLevels;
@@ -17,3 +19,16 @@ export const deduplicatedRowsSelector = createSelector(
    return dedupLogRows(filteredRows, dedupStrategy);
  }
 );
+export const getExploreDatasources = (): DataSourceSelectItem[] => {
+  return getDatasourceSrv()
+    .getExternal()
+    .map(
+      (ds: any) =>
+        ({
+          value: ds.name,
+          name: ds.name,
+          meta: ds.meta,
+        } as DataSourceSelectItem)
+    );
+};
--- a/public/app/types/explore.ts
+++ b/public/app/types/explore.ts
 import { Unsubscribable } from 'rxjs';
-import { ComponentType } from 'react';
 import {
  HistoryItem,
  DataQuery,
-  DataSourceSelectItem,
  DataSourceApi,
  QueryHint,
-  ExploreStartPageProps,
  PanelData,
  DataQueryRequest,
  RawTimeRange,
@@ -55,10 +52,6 @@ export interface ExploreState {
 export interface ExploreItemState {
  /**
-   * React component to be shown when no queries have been run yet, e.g., for a query language cheat sheet.
-   */
-  StartPage?: ComponentType<ExploreStartPageProps>;
-  /**
   * Width used for calculating the graph interval (can't have more datapoints than pixels)
   */
  containerWidth: number;
@@ -83,10 +76,6 @@ export interface ExploreItemState {
   */
  eventBridge?: Emitter;
  /**
-   * List of datasources to be shown in the datasource selector.
-   */
-  exploreDatasources: DataSourceSelectItem[];
-  /**
   * List of timeseries to be shown in the Explore graph result viewer.
   */
  graphResult?: GraphSeriesXY[];
@@ -133,10 +122,6 @@ export interface ExploreItemState {
   */
  showingGraph: boolean;
  /**
-   * True StartPage needs to be shown. Typically set to `false` once queries have been run.
-   */
-  showingStartPage?: boolean;
-  /**
   * True if table result viewer is expanded. Query runs will contain table queries.
   */
  showingTable: boolean;
@@ -167,8 +152,15 @@ export interface ExploreItemState {
   */
  refreshInterval?: string;
+  /**
+   * Copy of the state of the URL which is in store.location.query. This is duplicated here so we can diff the two
+   * after a change to see if we need to sync url state back to redux store (like on clicking Back in browser).
+   */
  urlState: ExploreUrlState;
+  /**
+   * Map of what changed between real url and local urlState so we can partially update just the things that are needed.
+   */
  update: ExploreUpdateState;
  latency: number;
@@ -189,6 +181,11 @@ export interface ExploreItemState {
  querySubscription?: Unsubscribable;
  queryResponse: PanelData;
+  /**
+   * Panel Id that is set if we come to explore from a penel. Used so we can get back to it and optionally modify the
+   * query of that panel.
+   */
  originPanelId?: number;
 }
@@ -217,11 +214,6 @@ export interface ExploreUrlState {
  context?: string;
 }
-export interface QueryIntervals {
-  interval: string;
-  intervalMs: number;
-}
 export interface QueryOptions {
  minInterval: string;
  maxDataPoints?: number;