From 1a80885180a38018d6b5f41cb462958fe519bd31 Mon Sep 17 00:00:00 2001
From: Marcus Efraimsson <marcus.efraimsson@gmail.com>
Date: Fri, 17 May 2019 12:45:11 +0200
Subject: [PATCH] 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
---
 .../app/features/explore/GraphContainer.tsx   | 28 +++----
 public/app/features/explore/LogsContainer.tsx | 15 +---
 public/app/features/explore/Panel.tsx         | 17 +++-
 .../app/features/explore/TableContainer.tsx   |  8 +-
 .../app/features/explore/state/actionTypes.ts |  9 ---
 public/app/features/explore/state/actions.ts  | 22 +----
 .../features/explore/state/reducers.test.ts   | 81 ++++++++-----------
 public/app/features/explore/state/reducers.ts |  5 +-
 .../app/features/explore/state/selectors.ts   |  3 +-
 public/app/types/explore.ts                   |  4 -
 public/sass/pages/_explore.scss               | 19 ++++-
 11 files changed, 86 insertions(+), 125 deletions(-)

diff --git a/public/app/features/explore/GraphContainer.tsx b/public/app/features/explore/GraphContainer.tsx
index 7033473a33b..0fba2ae6ded 100644
--- a/public/app/features/explore/GraphContainer.tsx
+++ b/public/app/features/explore/GraphContainer.tsx
@@ -46,22 +46,20 @@ 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}>
-        <Graph
-          data={graphResult}
-          height={graphHeight}
-          id={`explore-graph-${exploreId}`}
-          onChangeTime={this.onChangeTime}
-          range={timeRange}
-          timeZone={timeZone}
-          split={split}
-          width={width}
-        />
+      <Panel label="Graph" collapsible isOpen={showingGraph} loading={loading} onToggle={this.onClickGraphButton}>
+        {graphResult && (
+          <Graph
+            data={graphResult}
+            height={graphHeight}
+            id={`explore-graph-${exploreId}`}
+            onChangeTime={this.onChangeTime}
+            range={timeRange}
+            timeZone={timeZone}
+            split={split}
+            width={width}
+          />
+        )}
       </Panel>
     );
   }
diff --git a/public/app/features/explore/LogsContainer.tsx b/public/app/features/explore/LogsContainer.tsx
index 83cce42c7d1..7356fb15c17 100644
--- 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,
diff --git a/public/app/features/explore/Panel.tsx b/public/app/features/explore/Panel.tsx
index dc75cb0ecca..841a19dba2a 100644
--- 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} />
diff --git a/public/app/features/explore/TableContainer.tsx b/public/app/features/explore/TableContainer.tsx
index 78f190a05cb..18ee70d8ee2 100644
--- 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}>
-        <Table data={tableResult} loading={loading} onClickCell={onClickCell} />
+      <Panel label="Table" loading={loading} collapsible isOpen={showingTable} onToggle={this.onClickTableButton}>
+        {tableResult && <Table data={tableResult} loading={loading} onClickCell={onClickCell} />}
       </Panel>
     );
   }
diff --git a/public/app/features/explore/state/actionTypes.ts b/public/app/features/explore/state/actionTypes.ts
index 225a672ae2e..ff7fdcb55de 100644
--- 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;
 }
@@ -412,11 +408,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
  */
diff --git a/public/app/features/explore/state/actions.ts b/public/app/features/explore/state/actions.ts
index 310f310e671..c1157e01c6a 100644
--- 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<ToggleGraphPayload>
-    | ActionCreator<ToggleLogsPayload>
-    | ActionCreator<ToggleTablePayload>
+  actionCreator: ActionCreator<ToggleGraphPayload> | 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;
@@ -766,11 +757,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.
  */
diff --git a/public/app/features/explore/state/reducers.test.ts b/public/app/features/explore/state/reducers.test.ts
index c5ee8dbb779..5af71c29f8d 100644
--- 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: {},
-              logs: {},
-              tables: {},
+              metrics: true,
+              logs: true,
+              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 };
 
diff --git a/public/app/features/explore/state/reducers.ts b/public/app/features/explore/state/reducers.ts
index f0847360cbe..743294f0e74 100644
--- 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,
       };
     },
   })
diff --git a/public/app/features/explore/state/selectors.ts b/public/app/features/explore/state/selectors.ts
index fff52651646..6925e706d4f 100644
--- 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,
diff --git a/public/app/types/explore.ts b/public/app/types/explore.ts
index a828cbf9d3a..6f70ecaa25b 100644
--- a/public/app/types/explore.ts
+++ b/public/app/types/explore.ts
@@ -204,10 +204,6 @@ export interface ExploreItemState {
    * True if graph result viewer is expanded. Query runs will contain graph queries.
    */
   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.
    */
diff --git a/public/sass/pages/_explore.scss b/public/sass/pages/_explore.scss
index d7799203832..26401522118 100644
--- 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 {
-  margin-right: $space-sm;
-  font-size: $font-size-lg;
-  line-height: $font-size-h6;
+  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 {