From 69f0a4e1cb2c7775617a15a5ba709836a568210c Mon Sep 17 00:00:00 2001 From: vera-liu Date: Fri, 4 Nov 2016 14:21:51 -0700 Subject: [PATCH] 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 b6f5dcfe64f280e26479792d99c895d38936dc1f. * Added option to retrieve preview results after refresh --- caravel/assets/javascripts/SqlLab/actions.js | 59 +++++++++++--- .../javascripts/SqlLab/components/App.jsx | 2 - .../SqlLab/components/ResultSet.jsx | 31 +++++++- .../SqlLab/components/SouthPane.jsx | 77 ++++++++++++++++--- .../SqlLab/components/SqlEditor.jsx | 7 +- .../SqlLab/components/TabbedSqlEditors.jsx | 12 ++- .../SqlLab/components/TableElement.jsx | 21 +---- caravel/assets/javascripts/SqlLab/reducers.js | 45 ++++++++--- .../javascripts/sqllab/TableElement_spec.jsx | 4 +- .../spec/javascripts/sqllab/fixtures.js | 7 +- 10 files changed, 207 insertions(+), 58 deletions(-) diff --git a/caravel/assets/javascripts/SqlLab/actions.js b/caravel/assets/javascripts/SqlLab/actions.js index fa7f6e346e..12de5b32ba 100644 --- a/caravel/assets/javascripts/SqlLab/actions.js +++ b/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() { - return { type: HIDE_DATA_PREVIEW }; +export function removeDataPreview(table) { + 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) { - return { type: MERGE_TABLE, table }; +export function mergeTable(table, query) { + 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( - mergeTable(Object.assign(data, { + const dataPreviewQuery = { + 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 }; } diff --git a/caravel/assets/javascripts/SqlLab/components/App.jsx b/caravel/assets/javascripts/SqlLab/components/App.jsx index a43751156d..7eba77fd5a 100644 --- a/caravel/assets/javascripts/SqlLab/components/App.jsx +++ b/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 (
-
{content}
diff --git a/caravel/assets/javascripts/SqlLab/components/ResultSet.jsx b/caravel/assets/javascripts/SqlLab/components/ResultSet.jsx index a6aade429c..24b8e5fa19 100644 --- a/caravel/assets/javascripts/SqlLab/components/ResultSet.jsx +++ b/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
; } + 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 = ; } @@ -181,6 +200,16 @@ class ResultSet extends React.PureComponent { ); } } + if (query.cached) { + return ( + + click to retrieve results + + ); + } return (The query returned no data); } } diff --git a/caravel/assets/javascripts/SqlLab/components/SouthPane.jsx b/caravel/assets/javascripts/SqlLab/components/SouthPane.jsx index 9b0cbd554d..6f57bc8df5 100644 --- a/caravel/assets/javascripts/SqlLab/components/SouthPane.jsx +++ b/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) { - latestQuery = props.queries[props.queries.length - 1]; + if (props.editorQueries.length > 0) { + latestQuery = props.editorQueries[props.editorQueries.length - 1]; } let results; if (latestQuery) { @@ -29,22 +47,59 @@ class SouthPane extends React.PureComponent { } else { results = Run a query to display results here; } + + const dataPreviewTabs = props.dataPreviewQueries.map((query) => ( + + + + )); + return (
- - + +
{results}
- - + + + {dataPreviewTabs}
); } } -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); diff --git a/caravel/assets/javascripts/SqlLab/components/SqlEditor.jsx b/caravel/assets/javascripts/SqlLab/components/SqlEditor.jsx index 5ae681e58c..102e9f6101 100644 --- a/caravel/assets/javascripts/SqlLab/components/SqlEditor.jsx +++ b/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}
diff --git a/caravel/assets/javascripts/SqlLab/components/TabbedSqlEditors.jsx b/caravel/assets/javascripts/SqlLab/components/TabbedSqlEditors.jsx index d22ebc7cd4..4fc6539782 100644 --- a/caravel/assets/javascripts/SqlLab/components/TabbedSqlEditors.jsx +++ b/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 = (
{qe.title} {' '} @@ -152,7 +161,8 @@ class TabbedSqlEditors extends React.PureComponent { (t.queryEditorId === qe.id))} queryEditor={qe} - queries={this.state.queriesArray} + editorQueries={this.state.queriesArray} + dataPreviewQueries={dataPreviewQueries} latestQuery={latestQuery} database={database} actions={this.props.actions} diff --git a/caravel/assets/javascripts/SqlLab/components/TableElement.jsx b/caravel/assets/javascripts/SqlLab/components/TableElement.jsx index 52e7d2a651..2e9e6a6b78 100644 --- a/caravel/assets/javascripts/SqlLab/components/TableElement.jsx +++ b/caravel/assets/javascripts/SqlLab/components/TableElement.jsx @@ -51,18 +51,7 @@ class TableElement extends React.PureComponent { removeTable() { this.setState({ expanded: false }); - } - 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); + this.props.actions.removeDataPreview(this.props.table); } toggleSortColumns() { this.setState({ sortColumns: !this.state.sortColumns }); @@ -192,12 +181,6 @@ class TableElement extends React.PureComponent { 'Original table column order'} href="#" /> - {table.selectStar && diff --git a/caravel/assets/javascripts/SqlLab/reducers.js b/caravel/assets/javascripts/SqlLab/reducers.js index 7881b98eb7..4d0c97fb91 100644 --- a/caravel/assets/javascripts/SqlLab/reducers.js +++ b/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.showDataPreviewModal = true; + newState.activeSouthPaneTab = action.query.id; } 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 }); }, diff --git a/caravel/assets/spec/javascripts/sqllab/TableElement_spec.jsx b/caravel/assets/spec/javascripts/sqllab/TableElement_spec.jsx index 9859785547..90a11682b7 100644 --- a/caravel/assets/spec/javascripts/sqllab/TableElement_spec.jsx +++ b/caravel/assets/spec/javascripts/sqllab/TableElement_spec.jsx @@ -24,9 +24,9 @@ describe('TableElement', () => { React.isValidElement() ).to.equal(true); }); - it('has 3 Link elements', () => { + it('has 2 Link elements', () => { const wrapper = shallow(); - expect(wrapper.find(Link)).to.have.length(3); + expect(wrapper.find(Link)).to.have.length(2); }); it('has 14 columns', () => { const wrapper = shallow(); diff --git a/caravel/assets/spec/javascripts/sqllab/fixtures.js b/caravel/assets/spec/javascripts/sqllab/fixtures.js index 3ff6d922f6..2c5fb53b51 100644 --- a/caravel/assets/spec/javascripts/sqllab/fixtures.js +++ b/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, };