Put data preview in south pane (#1486)

* Put data preview in south pane

Before: data preview of a selected table appears as a modal, but for
some cases users may want to view data and edit sql at the same time

After:
 - data preview of a selected table pops up a new tab in South Pane
 - data are saved to local state and flushed in global store in
   ResultSet component

* Moved dataPreviewId to table object

* Put back preview icon for fetching preview data

* Revert "Put back preview icon for fetching preview data"

This reverts commit b6f5dcfe64.

* Added option to retrieve preview results after refresh

caravel/assets/javascripts/SqlLab/actions.js

@@ -20,6 +20,7 @@ export const QUERY_EDITOR_SET_SQL = 'QUERY_EDITOR_SET_SQL';
 export const QUERY_EDITOR_SET_SELECTED_TEXT = 'QUERY_EDITOR_SET_SELECTED_TEXT';
 export const SET_DATABASES = 'SET_DATABASES';
 export const SET_ACTIVE_QUERY_EDITOR = 'SET_ACTIVE_QUERY_EDITOR';
+export const SET_ACTIVE_SOUTHPANE_TAB = 'SET_ACTIVE_SOUTHPANE_TAB';
 export const ADD_ALERT = 'ADD_ALERT';
 export const REMOVE_ALERT = 'REMOVE_ALERT';
 export const REFRESH_QUERIES = 'REFRESH_QUERIES';
@@ -31,7 +32,8 @@ export const REQUEST_QUERY_RESULTS = 'REQUEST_QUERY_RESULTS';
 export const QUERY_SUCCESS = 'QUERY_SUCCESS';
 export const QUERY_FAILED = 'QUERY_FAILED';
 export const CLEAR_QUERY_RESULTS = 'CLEAR_QUERY_RESULTS';
-export const HIDE_DATA_PREVIEW = 'HIDE_DATA_PREVIEW';
+export const REMOVE_DATA_PREVIEW = 'REMOVE_DATA_PREVIEW';
+export const CHANGE_DATA_PREVIEW_ID = 'CHANGE_DATA_PREVIEW_ID';
 
 export function resetState() {
   return { type: RESET_STATE };
@@ -39,10 +41,11 @@ export function resetState() {
 
 export function startQuery(query) {
   Object.assign(query, {
-    id: shortid.generate(),
+    id: query.id ? query.id : shortid.generate(),
     progress: 0,
     startDttm: now(),
     state: (query.runAsync) ? 'pending' : 'running',
+    cached: false,
   });
   return { type: START_QUERY, query };
 }
@@ -63,8 +66,8 @@ export function clearQueryResults(query) {
   return { type: CLEAR_QUERY_RESULTS, query };
 }
 
-export function hideDataPreview() {
+export function removeDataPreview(table) {
-  return { type: HIDE_DATA_PREVIEW };
+  return { type: REMOVE_DATA_PREVIEW, table };
 }
 
 export function requestQueryResults(query) {
@@ -166,6 +169,10 @@ export function setActiveQueryEditor(queryEditor) {
   return { type: SET_ACTIVE_QUERY_EDITOR, queryEditor };
 }
 
+export function setActiveSouthPaneTab(tabId) {
+  return { type: SET_ACTIVE_SOUTHPANE_TAB, tabId };
+}
+
 export function removeQueryEditor(queryEditor) {
   return { type: REMOVE_QUERY_EDITOR, queryEditor };
 }
@@ -198,22 +205,35 @@ export function queryEditorSetSelectedText(queryEditor, sql) {
   return { type: QUERY_EDITOR_SET_SELECTED_TEXT, queryEditor, sql };
 }
 
-export function mergeTable(table) {
+export function mergeTable(table, query) {
-  return { type: MERGE_TABLE, table };
+  return { type: MERGE_TABLE, table, query };
 }
 
 export function addTable(query, tableName) {
   return function (dispatch) {
     let url = `/caravel/table/${query.dbId}/${tableName}/${query.schema}/`;
     $.get(url, (data) => {
-      dispatch(
+      const dataPreviewQuery = {
-        mergeTable(Object.assign(data, {
+        id: shortid.generate(),
+        dbId: query.dbId,
+        sql: data.selectStar,
+        tableName,
+        sqlEditorId: null,
+        tab: '',
+        runAsync: false,
+        ctas: false,
+      };
+      // Merge table to tables in state
+      dispatch(mergeTable(
+        Object.assign(data, {
           dbId: query.dbId,
           queryEditorId: query.id,
           schema: query.schema,
           expanded: true,
-        }))
+        }), dataPreviewQuery)
       );
+      // Run query to get preview data for table
+      dispatch(runQuery(dataPreviewQuery));
     })
     .fail(() => {
       dispatch(
@@ -238,6 +258,27 @@ export function addTable(query, tableName) {
   };
 }
 
+export function changeDataPreviewId(oldQueryId, newQuery) {
+  return { type: CHANGE_DATA_PREVIEW_ID, oldQueryId, newQuery };
+}
+
+export function reFetchQueryResults(query) {
+  return function (dispatch) {
+    const newQuery = {
+      id: shortid.generate(),
+      dbId: query.dbId,
+      sql: query.sql,
+      tableName: query.tableName,
+      sqlEditorId: null,
+      tab: '',
+      runAsync: false,
+      ctas: false,
+    };
+    dispatch(runQuery(newQuery));
+    dispatch(changeDataPreviewId(query.id, newQuery));
+  };
+}
+
 export function expandTable(table) {
   return { type: EXPAND_TABLE, table };
 }

caravel/assets/javascripts/SqlLab/components/App.jsx

@@ -5,7 +5,6 @@ import TabbedSqlEditors from './TabbedSqlEditors';
 import QueryAutoRefresh from './QueryAutoRefresh';
 import QuerySearch from './QuerySearch';
 import Alerts from './Alerts';
-import DataPreviewModal from './DataPreviewModal';
 
 import { bindActionCreators } from 'redux';
 import { connect } from 'react-redux';
@@ -49,7 +48,6 @@ class App extends React.PureComponent {
     return (
       <div className="App SqlLab">
         <Alerts alerts={this.props.alerts} actions={this.props.actions} />
-        <DataPreviewModal />
         <div className="container-fluid">
           {content}
         </div>

caravel/assets/javascripts/SqlLab/components/ResultSet.jsx

@@ -31,8 +31,20 @@ class ResultSet extends React.PureComponent {
     this.state = {
       searchText: '',
       showModal: false,
+      results: null,
     };
   }
+  componentWillReceiveProps(nextProps) {
+    // when new results comes in, save them locally and clear in store
+    if ((!nextProps.query.cached)
+      && nextProps.query.results
+      && nextProps.query.results.data.length > 0) {
+      this.setState(
+        { results: nextProps.query.results },
+        this.clearQueryResults(nextProps.query)
+      );
+    }
+  }
   getControls() {
     if (this.props.search || this.props.visualize || this.props.csv) {
       let csvButton;
@@ -83,6 +95,9 @@ class ResultSet extends React.PureComponent {
     }
     return <div className="noControls" />;
   }
+  clearQueryResults(query) {
+    this.props.actions.clearQueryResults(query);
+  }
   popSelectStar() {
     const qe = {
       id: shortid.generate(),
@@ -105,10 +120,14 @@ class ResultSet extends React.PureComponent {
   fetchResults(query) {
     this.props.actions.fetchQueryResults(query);
   }
+  reFetchQueryResults(query) {
+    this.props.actions.reFetchQueryResults(query);
+  }
   render() {
     const query = this.props.query;
-    const results = query.results;
+    const results = (this.props.query.cached) ? this.state.results : query.results;
     let sql;
+
     if (this.props.showSql) {
       sql = <HighlightedSql sql={query.sql} />;
     }
@@ -181,6 +200,16 @@ class ResultSet extends React.PureComponent {
         );
       }
     }
+    if (query.cached) {
+      return (
+        <a
+          href="#"
+          onClick={this.reFetchQueryResults.bind(this, query)}
+        >
+          click to retrieve results
+        </a>
+      );
+    }
     return (<Alert bsStyle="warning">The query returned no data</Alert>);
   }
 }

caravel/assets/javascripts/SqlLab/components/SouthPane.jsx

@@ -1,25 +1,43 @@
 import { Alert, Tab, Tabs } from 'react-bootstrap';
 import QueryHistory from './QueryHistory';
 import ResultSet from './ResultSet';
-import { areArraysShallowEqual } from '../../reduxUtils';
+import { connect } from 'react-redux';
+import { bindActionCreators } from 'redux';
+import * as Actions from '../actions';
 import React from 'react';
+import { areArraysShallowEqual } from '../../reduxUtils';
 
 import shortid from 'shortid';
 
+/*
+    editorQueries are queries executed by users passed from SqlEditor component
+    dataPrebiewQueries are all queries executed for preview of table data (from SqlEditorLeft)
+*/
 const propTypes = {
-  queries: React.PropTypes.array.isRequired,
+  editorQueries: React.PropTypes.array.isRequired,
+  dataPreviewQueries: React.PropTypes.array.isRequired,
   actions: React.PropTypes.object.isRequired,
+  activeSouthPaneTab: React.PropTypes.string,
+};
+
+const defaultProps = {
+  activeSouthPaneTab: 'Results',
 };
 
 class SouthPane extends React.PureComponent {
+  switchTab(id) {
+    this.props.actions.setActiveSouthPaneTab(id);
+  }
   shouldComponentUpdate(nextProps) {
-    return !areArraysShallowEqual(this.props.queries, nextProps.queries);
+    return !areArraysShallowEqual(this.props.editorQueries, nextProps.editorQueries)
+      || !areArraysShallowEqual(this.props.dataPreviewQueries, nextProps.dataPreviewQueries)
+      || this.props.activeSouthPaneTab !== nextProps.activeSouthPaneTab;
   }
   render() {
     let latestQuery;
     const props = this.props;
-    if (props.queries.length > 0) {
+    if (props.editorQueries.length > 0) {
-      latestQuery = props.queries[props.queries.length - 1];
+      latestQuery = props.editorQueries[props.editorQueries.length - 1];
     }
     let results;
     if (latestQuery) {
@@ -29,22 +47,59 @@ class SouthPane extends React.PureComponent {
     } else {
       results = <Alert bsStyle="info">Run a query to display results here</Alert>;
     }
+
+    const dataPreviewTabs = props.dataPreviewQueries.map((query) => (
+      <Tab
+        title={`Preview for ${query.tableName}`}
+        eventKey={query.id}
+        key={query.id}
+      >
+        <ResultSet query={query} visualize={false} csv={false} actions={props.actions} />
+      </Tab>
+    ));
+
     return (
       <div className="SouthPane">
-        <Tabs bsStyle="tabs" id={shortid.generate()}>
+        <Tabs
-          <Tab title="Results" eventKey={1}>
+          bsStyle="tabs"
+          id={shortid.generate()}
+          activeKey={this.props.activeSouthPaneTab}
+          onSelect={this.switchTab.bind(this)}
+        >
+          <Tab
+            title="Results"
+            eventKey="Results"
+          >
             <div style={{ overflow: 'auto' }}>
               {results}
             </div>
           </Tab>
-          <Tab title="Query History" eventKey={2}>
+          <Tab
-            <QueryHistory queries={props.queries} actions={props.actions} />
+            title="Query History"
+            eventKey="History"
+          >
+            <QueryHistory queries={props.editorQueries} actions={props.actions} />
           </Tab>
+          {dataPreviewTabs}
         </Tabs>
       </div>
     );
   }
 }
-SouthPane.propTypes = propTypes;
 
-export default SouthPane;
+function mapStateToProps(state) {
+  return {
+    activeSouthPaneTab: state.activeSouthPaneTab,
+  };
+}
+
+function mapDispatchToProps(dispatch) {
+  return {
+    actions: bindActionCreators(Actions, dispatch),
+  };
+}
+
+SouthPane.propTypes = propTypes;
+SouthPane.defaultProps = defaultProps;
+
+export default connect(mapStateToProps, mapDispatchToProps)(SouthPane);

caravel/assets/javascripts/SqlLab/components/SqlEditor.jsx

@@ -25,7 +25,8 @@ const propTypes = {
   latestQuery: React.PropTypes.object,
   networkOn: React.PropTypes.bool,
   tables: React.PropTypes.array.isRequired,
-  queries: React.PropTypes.array.isRequired,
+  editorQueries: React.PropTypes.array.isRequired,
+  dataPreviewQueries: React.PropTypes.array.isRequired,
   queryEditor: React.PropTypes.object.isRequired,
 };
 
@@ -70,6 +71,7 @@ class SqlEditor extends React.PureComponent {
       ctas,
     };
     this.props.actions.runQuery(query);
+    this.props.actions.setActiveSouthPaneTab('Results');
   }
   stopQuery() {
     this.props.actions.stopQuery(this.props.latestQuery);
@@ -224,7 +226,8 @@ class SqlEditor extends React.PureComponent {
             {editorBottomBar}
             <br />
             <SouthPane
-              queries={this.props.queries}
+              editorQueries={this.props.editorQueries}
+              dataPreviewQueries={this.props.dataPreviewQueries}
               actions={this.props.actions}
             />
           </Col>

caravel/assets/javascripts/SqlLab/components/TabbedSqlEditors.jsx

@@ -118,6 +118,15 @@ class TabbedSqlEditors extends React.PureComponent {
         database = this.props.databases[qe.dbId];
       }
       const state = (latestQuery) ? latestQuery.state : '';
+
+      const dataPreviewQueries = [];
+      this.props.tables.forEach((table) => {
+        const queryId = table.dataPreviewQueryId;
+        if (queryId && this.props.queries[queryId]) {
+          dataPreviewQueries.push(this.props.queries[queryId]);
+        }
+      });
+
       const tabTitle = (
         <div>
           <div className={'circle ' + state} /> {qe.title} {' '}
@@ -152,7 +161,8 @@ class TabbedSqlEditors extends React.PureComponent {
                 <SqlEditor
                   tables={this.props.tables.filter((t) => (t.queryEditorId === qe.id))}
                   queryEditor={qe}
-                  queries={this.state.queriesArray}
+                  editorQueries={this.state.queriesArray}
+                  dataPreviewQueries={dataPreviewQueries}
                   latestQuery={latestQuery}
                   database={database}
                   actions={this.props.actions}

caravel/assets/javascripts/SqlLab/components/TableElement.jsx

@@ -51,18 +51,7 @@ class TableElement extends React.PureComponent {
 
   removeTable() {
     this.setState({ expanded: false });
-  }
+    this.props.actions.removeDataPreview(this.props.table);
-  dataPreviewModal() {
-    const query = {
-      dbId: this.props.table.dbId,
-      sql: this.props.table.selectStar,
-      tableName: this.props.table.name,
-      sqlEditorId: null,
-      tab: '',
-      runAsync: false,
-      ctas: false,
-    };
-    this.props.actions.runQuery(query);
   }
   toggleSortColumns() {
     this.setState({ sortColumns: !this.state.sortColumns });
@@ -192,12 +181,6 @@ class TableElement extends React.PureComponent {
                     'Original table column order'}
                   href="#"
                 />
-                <Link
-                  className="fa fa-search-plus pull-left m-l-2"
-                  onClick={this.dataPreviewModal.bind(this)}
-                  tooltip="Data preview"
-                  href="#"
-                />
                 {table.selectStar &&
                   <CopyToClipboard
                     copyNode={
@@ -211,7 +194,7 @@ class TableElement extends React.PureComponent {
                 <Link
                   className="fa fa-trash table-remove pull-left m-l-2"
                   onClick={this.removeTable.bind(this)}
-                  tooltip="Remove from panel"
+                  tooltip="Remove table preview"
                   href="#"
                 />
               </ButtonGroup>

caravel/assets/javascripts/SqlLab/reducers.js

@@ -16,8 +16,6 @@ export const defaultQueryEditor = {
 
 export const initialState = {
   alerts: [],
-  showDataPreviewModal: false,
-  dataPreviewQueryId: null,
   networkOn: true,
   queries: {},
   databases: {},
@@ -25,6 +23,7 @@ export const initialState = {
   tabHistory: [defaultQueryEditor.id],
   tables: [],
   queriesLastUpdate: 0,
+  activeSouthPaneTab: 'Results',
 };
 
 export const sqlLabReducer = function (state, action) {
@@ -86,19 +85,44 @@ export const sqlLabReducer = function (state, action) {
         }
       });
       if (existingTable) {
+        if (action.query) {
+          at.dataPreviewQueryId = action.query.id;
+        }
         return alterInArr(state, 'tables', existingTable, at);
       }
       at.id = shortid.generate();
-      return addToArr(state, 'tables', at);
+      // for new table, associate Id of query for data preview
+      at.dataPreviewQueryId = null;
+      let newState = addToArr(state, 'tables', at);
+      if (action.query) {
+        newState = alterInArr(newState, 'tables', at, { dataPreviewQueryId: action.query.id });
+      }
+      return newState;
     },
     [actions.EXPAND_TABLE]() {
       return alterInArr(state, 'tables', action.table, { expanded: true });
     },
-    [actions.HIDE_DATA_PREVIEW]() {
+    [actions.REMOVE_DATA_PREVIEW]() {
       const queries = Object.assign({}, state.queries);
-      delete queries[state.dataPreviewQueryId];
+      delete queries[action.table.dataPreviewQueryId];
+      const newState = alterInArr(state, 'tables', action.table, { dataPreviewQueryId: null });
       return Object.assign(
-        {}, state, { showDataPreviewModal: false, queries, dataPreviewQueryId: null });
+       {}, newState, { queries });
+    },
+    [actions.CHANGE_DATA_PREVIEW_ID]() {
+      const queries = Object.assign({}, state.queries);
+      delete queries[action.oldQueryId];
+
+      const newTables = [];
+      state.tables.forEach((t) => {
+        if (t.dataPreviewQueryId === action.oldQueryId) {
+          newTables.push(Object.assign({}, t, { dataPreviewQueryId: action.newQuery.id }));
+        } else {
+          newTables.push(t);
+        }
+      });
+      return Object.assign(
+       {}, state, { queries, tables: newTables, activeSouthPaneTab: action.newQuery.id });
     },
     [actions.COLLAPSE_TABLE]() {
       return alterInArr(state, 'tables', action.table, { expanded: false });
@@ -116,8 +140,7 @@ export const sqlLabReducer = function (state, action) {
           newState = Object.assign({}, state, { queries });
         }
       } else {
-        newState.dataPreviewQueryId = action.query.id;
+        newState.activeSouthPaneTab = action.query.id;
-        newState.showDataPreviewModal = true;
       }
       newState = addToObject(newState, 'queries', action.query);
       const sqlEditor = { id: action.query.sqlEditorId };
@@ -127,7 +150,7 @@ export const sqlLabReducer = function (state, action) {
       return alterInObject(state, 'queries', action.query, { state: 'stopped' });
     },
     [actions.CLEAR_QUERY_RESULTS]() {
-      return alterInObject(state, 'queries', action.query, { results: [] });
+      return alterInObject(state, 'queries', action.query, { results: [], cached: true });
     },
     [actions.REQUEST_QUERY_RESULTS]() {
       return alterInObject(state, 'queries', action.query, { state: 'fetching' });
@@ -144,6 +167,7 @@ export const sqlLabReducer = function (state, action) {
         rows,
         state: 'success',
         errorMessage: null,
+        cached: false,
       };
       return alterInObject(state, 'queries', action.query, alts);
     },
@@ -160,6 +184,9 @@ export const sqlLabReducer = function (state, action) {
       }
       return state;
     },
+    [actions.SET_ACTIVE_SOUTHPANE_TAB]() {
+      return Object.assign({}, state, { activeSouthPaneTab: action.tabId });
+    },
     [actions.QUERY_EDITOR_SETDB]() {
       return alterInArr(state, 'queryEditors', action.queryEditor, { dbId: action.dbId });
     },

caravel/assets/spec/javascripts/sqllab/TableElement_spec.jsx

@@ -24,9 +24,9 @@ describe('TableElement', () => {
       React.isValidElement(<TableElement {...mockedProps} />)
     ).to.equal(true);
   });
-  it('has 3 Link elements', () => {
+  it('has 2 Link elements', () => {
     const wrapper = shallow(<TableElement {...mockedProps} />);
-    expect(wrapper.find(Link)).to.have.length(3);
+    expect(wrapper.find(Link)).to.have.length(2);
   });
   it('has 14 columns', () => {
     const wrapper = shallow(<TableElement {...mockedProps} />);

caravel/assets/spec/javascripts/sqllab/fixtures.js

@@ -11,6 +11,7 @@ export const table = {
   schema: 'caravel',
   name: 'ab_user',
   id: 'r11Vgt60',
+  dataPreviewQueryId: null,
   partitions: {
     cols: ['username'],
     latest: 'bob',
@@ -183,6 +184,7 @@ export const queries = [
     tab: 'Demo',
     runAsync: false,
     ctas: false,
+    cached: false,
     id: 'BkA1CLrJg',
     progress: 100,
     startDttm: 1476910566092.96,
@@ -217,6 +219,7 @@ export const queries = [
     tab: 'Demo',
     runAsync: true,
     ctas: false,
+    cached: false,
     id: 'S1zeAISkx',
     progress: 100,
     startDttm: 1476910570802.2,
@@ -247,8 +250,6 @@ export const queries = [
 
 export const initialState = {
   alerts: [],
-  showDataPreviewModal: false,
-  dataPreviewQueryId: null,
   networkOn: true,
   queries: {},
   databases: {},
@@ -257,6 +258,7 @@ export const initialState = {
   tables: [],
   workspaceQueries: [],
   queriesLastUpdate: 0,
+  activeSouthPaneTab: 'Results',
 };
 
 export const query = {
@@ -267,4 +269,5 @@ export const query = {
   tempTableName: null,
   runAsync: false,
   ctas: false,
+  cached: false,
 };