explore: fix issues when loading and both graph/table are collapsed (#17113)

Removes the functionality of being able to collapse/expand the logs container. When both graph and table are collapsed and you reload the page then the start page should not be displayed. When both graph and table are collapsed and you reload the page then the graph and table panels should be displayed. Fix so that reducer tests are run. On of the test used fit() instead of it() which had the consequence of only 1 reducer test was executed and the rest skipped. There was some failing tests that now is updated and now passes. Fixes #17098

explore: fix issues when loading and both graph/table are collapsed (#17113)
Removes the functionality of being able to collapse/expand the logs container. When both graph and table are collapsed and you reload the page then the start page should not be displayed. When both graph and table are collapsed and you reload the page then the graph and table panels should be displayed. Fix so that reducer tests are run. On of the test used fit() instead of it() which had the consequence of only 1 reducer test was executed and the rest skipped. There was some failing tests that now is updated and now passes. Fixes #17098
1a808851 · Marcus Efraimsson · GitHub · 32cdab4c · 1a808851 · 1a808851
Commit 1a808851 authored May 17, 2019 by Marcus Efraimsson Committed by GitHub May 17, 2019
11 changed files
--- a/public/app/features/explore/GraphContainer.tsx
+++ b/public/app/features/explore/GraphContainer.tsx
@@ -46,12 +46,9 @@ export class GraphContainer extends PureComponent<GraphContainerProps> {
    const graphHeight = showingGraph && showingTable ? 200 : 400;
    const timeRange = { from: range.from.valueOf(), to: range.to.valueOf() };
-    if (!graphResult) {
-      return null;
-    }
    return (
-      <Panel label="Graph" isOpen={showingGraph} loading={loading} onToggle={this.onClickGraphButton}>
+      <Panel label="Graph" collapsible isOpen={showingGraph} loading={loading} onToggle={this.onClickGraphButton}>
+        {graphResult && (
          <Graph
            data={graphResult}
            height={graphHeight}
@@ -62,6 +59,7 @@ export class GraphContainer extends PureComponent<GraphContainerProps> {
            split={split}
            width={width}
          />
+        )}
      </Panel>
    );
  }

--- a/public/app/features/explore/LogsContainer.tsx
+++ b/public/app/features/explore/LogsContainer.tsx
@@ -7,7 +7,7 @@ import { ExploreId, ExploreItemState } from 'app/types/explore';
 import { LogsModel, LogsDedupStrategy } from 'app/core/logs_model';
 import { StoreState } from 'app/types';
-import { toggleLogs, changeDedupStrategy, changeTime } from './state/actions';
+import { changeDedupStrategy, changeTime } from './state/actions';
 import Logs from './Logs';
 import Panel from './Panel';
 import { toggleLogLevelAction } from 'app/features/explore/state/actionTypes';
@@ -27,8 +27,6 @@ interface LogsContainerProps {
  timeZone: TimeZone;
  scanning?: boolean;
  scanRange?: RawTimeRange;
-  showingLogs: boolean;
-  toggleLogs: typeof toggleLogs;
  toggleLogLevelAction: typeof toggleLogLevelAction;
  changeDedupStrategy: typeof changeDedupStrategy;
  dedupStrategy: LogsDedupStrategy;
@@ -48,10 +46,6 @@ export class LogsContainer extends PureComponent<LogsContainerProps> {
    changeTime(exploreId, range);
  };
-  onClickLogsButton = () => {
-    this.props.toggleLogs(this.props.exploreId, this.props.showingLogs);
-  };
  handleDedupStrategyChange = (dedupStrategy: LogsDedupStrategy) => {
    this.props.changeDedupStrategy(this.props.exploreId, dedupStrategy);
  };
@@ -76,7 +70,6 @@ export class LogsContainer extends PureComponent<LogsContainerProps> {
      onStopScanning,
      range,
      timeZone,
-      showingLogs,
      scanning,
      scanRange,
      width,
@@ -84,7 +77,7 @@ export class LogsContainer extends PureComponent<LogsContainerProps> {
    } = this.props;
    return (
-      <Panel label="Logs" loading={loading} isOpen={showingLogs} onToggle={this.onClickLogsButton}>
+      <Panel label="Logs" loading={loading} isOpen>
        <Logs
          dedupStrategy={this.props.dedupStrategy || LogsDedupStrategy.none}
          data={logsResult}
@@ -115,7 +108,7 @@ function mapStateToProps(state: StoreState, { exploreId }) {
  const item: ExploreItemState = explore[exploreId];
  const { logsHighlighterExpressions, logsResult, logIsLoading, scanning, scanRange, range } = item;
  const loading = logIsLoading;
-  const { showingLogs, dedupStrategy } = exploreItemUIStateSelector(item);
+  const { dedupStrategy } = exploreItemUIStateSelector(item);
  const hiddenLogLevels = new Set(item.hiddenLogLevels);
  const dedupedResult = deduplicatedLogsSelector(item);
  const timeZone = getTimeZone(state.user);
@@ -126,7 +119,6 @@ function mapStateToProps(state: StoreState, { exploreId }) {
    logsResult,
    scanning,
    scanRange,
-    showingLogs,
    range,
    timeZone,
    dedupStrategy,
@@ -136,7 +128,6 @@ function mapStateToProps(state: StoreState, { exploreId }) {
 }
 const mapDispatchToProps = {
-  toggleLogs,
  changeDedupStrategy,
  toggleLogLevelAction,
  changeTime,

--- a/public/app/features/explore/Panel.tsx
+++ b/public/app/features/explore/Panel.tsx
@@ -4,18 +4,27 @@ interface Props {
  isOpen: boolean;
  label: string;
  loading?: boolean;
-  onToggle: (isOpen: boolean) => void;
+  collapsible?: boolean;
+  onToggle?: (isOpen: boolean) => void;
 }
 export default class Panel extends PureComponent<Props> {
-  onClickToggle = () => this.props.onToggle(!this.props.isOpen);
+  onClickToggle = () => {
+    const { onToggle, isOpen } = this.props;
+    if (onToggle) {
+      onToggle(!isOpen);
+    }
+  };
  render() {
-    const { isOpen, loading } = this.props;
+    const { isOpen, loading, collapsible } = this.props;
+    const panelClass = collapsible
+      ? 'explore-panel explore-panel--collapsible panel-container'
+      : 'explore-panel panel-container';
    const iconClass = isOpen ? 'fa fa-caret-up' : 'fa fa-caret-down';
    const loaderClass = loading ? 'explore-panel__loader explore-panel__loader--active' : 'explore-panel__loader';
    return (
-      <div className="explore-panel panel-container">
+      <div className={panelClass}>
        <div className="explore-panel__header" onClick={this.onClickToggle}>
          <div className="explore-panel__header-buttons">
            <span className={iconClass} />

--- a/public/app/features/explore/TableContainer.tsx
+++ b/public/app/features/explore/TableContainer.tsx
@@ -27,13 +27,9 @@ export class TableContainer extends PureComponent<TableContainerProps> {
  render() {
    const { loading, onClickCell, showingTable, tableResult } = this.props;
-    if (!tableResult) {
-      return null;
-    }
    return (
-      <Panel label="Table" loading={loading} isOpen={showingTable} onToggle={this.onClickTableButton}>
+      <Panel label="Table" loading={loading} collapsible isOpen={showingTable} onToggle={this.onClickTableButton}>
-        <Table data={tableResult} loading={loading} onClickCell={onClickCell} />
+        {tableResult && <Table data={tableResult} loading={loading} onClickCell={onClickCell} />}
      </Panel>
    );
  }

--- a/public/app/features/explore/state/actionTypes.ts
+++ b/public/app/features/explore/state/actionTypes.ts
@@ -204,10 +204,6 @@ export interface ToggleGraphPayload {
  exploreId: ExploreId;
 }
-export interface ToggleLogsPayload {
-  exploreId: ExploreId;
-}
 export interface UpdateUIStatePayload extends Partial<ExploreUIState> {
  exploreId: ExploreId;
 }
@@ -413,11 +409,6 @@ export const toggleTableAction = actionCreatorFactory<ToggleTablePayload>('explo
 export const toggleGraphAction = actionCreatorFactory<ToggleGraphPayload>('explore/TOGGLE_GRAPH').create();
 /**
- * Expand/collapse the logs result viewer. When collapsed, log queries won't be run.
- */
-export const toggleLogsAction = actionCreatorFactory<ToggleLogsPayload>('explore/TOGGLE_LOGS').create();
-/**
 * Updates datasource instance before datasouce loading has started
 */
 export const updateDatasourceInstanceAction = actionCreatorFactory<UpdateDatasourceInstancePayload>(

--- a/public/app/features/explore/state/actions.ts
+++ b/public/app/features/explore/state/actions.ts
@@ -72,10 +72,8 @@ import {
  splitOpenAction,
  addQueryRowAction,
  toggleGraphAction,
-  toggleLogsAction,
  toggleTableAction,
  ToggleGraphPayload,
-  ToggleLogsPayload,
  ToggleTablePayload,
  updateUIStateAction,
  runQueriesAction,
@@ -517,7 +515,6 @@ export function runQueries(exploreId: ExploreId, ignoreUIState = false, replaceU
    const {
      datasourceInstance,
      queries,
-      showingLogs,
      showingGraph,
      showingTable,
      datasourceError,
@@ -562,7 +559,7 @@ export function runQueries(exploreId: ExploreId, ignoreUIState = false, replaceU
        })
      );
    }
-    if ((ignoreUIState || showingLogs) && mode === ExploreMode.Logs) {
+    if (mode === ExploreMode.Logs) {
      dispatch(runQueriesForType(exploreId, 'Logs', { interval, format: 'logs' }));
    }
@@ -700,7 +697,7 @@ export function stateSave(replaceUrl = false): ThunkResult<void> {
      range: toRawTimeRange(left.range),
      ui: {
        showingGraph: left.showingGraph,
-        showingLogs: left.showingLogs,
+        showingLogs: true,
        showingTable: left.showingTable,
        dedupStrategy: left.dedupStrategy,
      },
@@ -713,7 +710,7 @@ export function stateSave(replaceUrl = false): ThunkResult<void> {
        range: toRawTimeRange(right.range),
        ui: {
          showingGraph: right.showingGraph,
-          showingLogs: right.showingLogs,
+          showingLogs: true,
          showingTable: right.showingTable,
          dedupStrategy: right.dedupStrategy,
        },
@@ -731,10 +728,7 @@ export function stateSave(replaceUrl = false): ThunkResult<void> {
 * queries won't be run
 */
 const togglePanelActionCreator = (
-  actionCreator:
+  actionCreator: ActionCreator<ToggleGraphPayload> | ActionCreator<ToggleTablePayload>
-    | ActionCreator<ToggleGraphPayload>
-    | ActionCreator<ToggleLogsPayload>
-    | ActionCreator<ToggleTablePayload>
 ) => (exploreId: ExploreId, isPanelVisible: boolean): ThunkResult<void> => {
  return dispatch => {
    let uiFragmentStateUpdate: Partial<ExploreUIState>;
@@ -744,9 +738,6 @@ const togglePanelActionCreator = (
      case toggleGraphAction.type:
        uiFragmentStateUpdate = { showingGraph: !isPanelVisible };
        break;
-      case toggleLogsAction.type:
-        uiFragmentStateUpdate = { showingLogs: !isPanelVisible };
-        break;
      case toggleTableAction.type:
        uiFragmentStateUpdate = { showingTable: !isPanelVisible };
        break;
@@ -767,11 +758,6 @@ const togglePanelActionCreator = (
 export const toggleGraph = togglePanelActionCreator(toggleGraphAction);
 /**
- * Expand/collapse the logs result viewer. When collapsed, log queries won't be run.
- */
-export const toggleLogs = togglePanelActionCreator(toggleLogsAction);
-/**
 * Expand/collapse the table result viewer. When collapsed, table queries won't be run.
 */
 export const toggleTable = togglePanelActionCreator(toggleTableAction);

--- a/public/app/features/explore/state/reducers.test.ts
+++ b/public/app/features/explore/state/reducers.test.ts
@@ -10,7 +10,6 @@ import {
  ExploreItemState,
  ExploreUrlState,
  ExploreState,
-  QueryTransaction,
  RangeScanner,
  ExploreMode,
 } from 'app/types/explore';
@@ -25,6 +24,7 @@ import {
  splitOpenAction,
  splitCloseAction,
  changeModeAction,
+  runQueriesAction,
 } from './actionTypes';
 import { Reducer } from 'redux';
 import { ActionOf } from 'app/core/redux/actionCreatorFactory';
@@ -36,7 +36,7 @@ import { DataSourceApi, DataQuery } from '@grafana/ui';
 describe('Explore item reducer', () => {
  describe('scanning', () => {
-    test('should start scanning', () => {
+    it('should start scanning', () => {
      const scanner = jest.fn();
      const initalState = {
        ...makeExploreItemState(),
@@ -53,7 +53,7 @@ describe('Explore item reducer', () => {
          scanner,
        });
    });
-    test('should stop scanning', () => {
+    it('should stop scanning', () => {
      const scanner = jest.fn();
      const initalState = {
        ...makeExploreItemState(),
@@ -96,7 +96,6 @@ describe('Explore item reducer', () => {
    describe('when testDataSourceFailureAction is dispatched', () => {
      it('then it should set correct state', () => {
        const error = 'some error';
-        const queryTransactions: QueryTransaction[] = [];
        const initalState: Partial<ExploreItemState> = {
          datasourceError: null,
          graphResult: [],
@@ -111,7 +110,6 @@ describe('Explore item reducer', () => {
        };
        const expectedState = {
          datasourceError: error,
-          queryTransactions,
          graphResult: undefined as any[],
          tableResult: undefined as TableModel,
          logsResult: undefined as LogsModel,
@@ -144,9 +142,9 @@ describe('Explore item reducer', () => {
          const StartPage = {};
          const datasourceInstance = {
            meta: {
-              metrics: {},
+              metrics: true,
-              logs: {},
+              logs: true,
-              tables: {},
+              tables: true,
            },
            components: {
              ExploreStartPage: StartPage,
@@ -175,6 +173,11 @@ describe('Explore item reducer', () => {
            queryKeys,
            supportedModes: [ExploreMode.Metrics, ExploreMode.Logs],
            mode: ExploreMode.Metrics,
+            graphIsLoading: false,
+            tableIsLoading: false,
+            logIsLoading: false,
+            latency: 0,
+            queryErrors: [],
          };
          reducerTester()
@@ -185,6 +188,28 @@ describe('Explore item reducer', () => {
      });
    });
  });
+  describe('run queries', () => {
+    describe('when runQueriesAction is dispatched', () => {
+      it('then it should set correct state', () => {
+        const initalState: Partial<ExploreItemState> = {
+          showingStartPage: true,
+        };
+        const expectedState = {
+          queryIntervals: {
+            interval: '1s',
+            intervalMs: 1000,
+          },
+          showingStartPage: false,
+        };
+        reducerTester()
+          .givenReducer(itemReducer, initalState)
+          .whenActionIsDispatched(runQueriesAction({ exploreId: ExploreId.left }))
+          .thenStateShouldEqual(expectedState);
+      });
+    });
+  });
 });
 export const setup = (urlStateOverrides?: any) => {
@@ -529,46 +554,8 @@ describe('Explore reducer', () => {
            });
          });
-          describe('and refreshInterval differs', () => {
-            it('then it should return update refreshInterval', () => {
-              const { initalState, serializedUrlState } = setup();
-              const expectedState = {
-                ...initalState,
-                left: {
-                  ...initalState.left,
-                  update: {
-                    ...initalState.left.update,
-                    refreshInterval: true,
-                  },
-                },
-              };
-              const stateWithDifferentDataSource = {
-                ...initalState,
-                left: {
-                  ...initalState.left,
-                  urlState: {
-                    ...initalState.left.urlState,
-                    refreshInterval: '5s',
-                  },
-                },
-              };
-              reducerTester()
-                .givenReducer(exploreReducer, stateWithDifferentDataSource)
-                .whenActionIsDispatched(
-                  updateLocation({
-                    query: {
-                      left: serializedUrlState,
-                    },
-                    path: '/explore',
-                  })
-                )
-                .thenStateShouldEqual(expectedState);
-            });
-          });
          describe('and nothing differs', () => {
-            fit('then it should return update ui', () => {
+            it('then it should return update ui', () => {
              const { initalState, serializedUrlState } = setup();
              const expectedState = { ...initalState };

--- a/public/app/features/explore/state/reducers.ts
+++ b/public/app/features/explore/state/reducers.ts
@@ -95,7 +95,6 @@ export const makeExploreItemState = (): ExploreItemState => ({
  scanning: false,
  scanRange: null,
  showingGraph: true,
-  showingLogs: true,
  showingTable: true,
  graphIsLoading: false,
  logIsLoading: false,
@@ -351,7 +350,6 @@ export const itemReducer = reducerFactory<ExploreItemState>({} as ExploreItemSta
        logsResult: resultType === 'Logs' ? null : state.logsResult,
        latency: 0,
        queryErrors,
-        showingStartPage: false,
        graphIsLoading: resultType === 'Graph' ? false : state.graphIsLoading,
        logIsLoading: resultType === 'Logs' ? false : state.logIsLoading,
        tableIsLoading: resultType === 'Table' ? false : state.tableIsLoading,
@@ -371,7 +369,6 @@ export const itemReducer = reducerFactory<ExploreItemState>({} as ExploreItemSta
        graphIsLoading: resultType === 'Graph' ? true : state.graphIsLoading,
        logIsLoading: resultType === 'Logs' ? true : state.logIsLoading,
        tableIsLoading: resultType === 'Table' ? true : state.tableIsLoading,
-        showingStartPage: false,
        update: makeInitialUpdateState(),
      };
    },
@@ -392,7 +389,6 @@ export const itemReducer = reducerFactory<ExploreItemState>({} as ExploreItemSta
        graphIsLoading: false,
        logIsLoading: false,
        tableIsLoading: false,
-        showingStartPage: false,
        update: makeInitialUpdateState(),
      };
    },
@@ -543,6 +539,7 @@ export const itemReducer = reducerFactory<ExploreItemState>({} as ExploreItemSta
      return {
        ...state,
        queryIntervals,
+        showingStartPage: false,
      };
    },
  })

--- a/public/app/features/explore/state/selectors.ts
+++ b/public/app/features/explore/state/selectors.ts
@@ -3,10 +3,9 @@ import { ExploreItemState } from 'app/types';
 import { filterLogLevels, dedupLogRows } from 'app/core/logs_model';
 export const exploreItemUIStateSelector = (itemState: ExploreItemState) => {
-  const { showingGraph, showingLogs, showingTable, showingStartPage, dedupStrategy } = itemState;
+  const { showingGraph, showingTable, showingStartPage, dedupStrategy } = itemState;
  return {
    showingGraph,
-    showingLogs,
    showingTable,
    showingStartPage,
    dedupStrategy,

--- a/public/app/types/explore.ts
+++ b/public/app/types/explore.ts
@@ -205,10 +205,6 @@ export interface ExploreItemState {
   */
  showingGraph: boolean;
  /**
-   * True if logs result viewer is expanded. Query runs will contain logs queries.
-   */
-  showingLogs: boolean;
-  /**
   * True StartPage needs to be shown. Typically set to `false` once queries have been run.
   */
  showingStartPage?: boolean;

--- a/public/sass/pages/_explore.scss
+++ b/public/sass/pages/_explore.scss
@@ -164,7 +164,7 @@
 .explore-panel__header {
  padding: $space-sm $space-md 0 $space-md;
  display: flex;
-  cursor: pointer;
+  cursor: inherit;
  transition: all 0.1s linear;
 }
@@ -176,9 +176,20 @@
 }
 .explore-panel__header-buttons {
+  display: none;
+}
+.explore-panel--collapsible {
+  .explore-panel__header {
+    cursor: pointer;
+  }
+  .explore-panel__header-buttons {
    margin-right: $space-sm;
    font-size: $font-size-lg;
    line-height: $font-size-h6;
+    display: inherit;
+  }
 }
 .time-series-disclaimer {