From 2edce5bf8afa7b74fdafd8ab8a2e6394d46e6391 Mon Sep 17 00:00:00 2001 From: Nick Barnwell Date: Tue, 18 Oct 2016 16:02:29 -0700 Subject: [PATCH] Enable "Run Query in New Tab" in SQL Lab (#1343) * Enable "Clone to New Tab" btn in QueryHistoryTable Method #1; doesn't feel very clean. Going to attempt to reimplement using an action and changing state directly through the reducer rather than creating a new QueryEditor object directly from the QueryTable * Move Clone Logic to Action * Implement PR feedback * Clean up reducer action; fix bug Bug => Attempting to clone anything other than the most recent Query for a given TabbedQueryEditor would throw an exception, because we depended on lastQueryId to find the title of the QueryEditor to clone. Since you can only activate a clone from the currently active tab, we can instead fetch the ID of the Editor to copy the Title of from the tip of tabHistory. * Tests for Reducer Action * Fix CodeClimate feedback --- caravel/assets/javascripts/SqlLab/actions.js | 5 ++++ .../SqlLab/components/QueryTable.jsx | 9 +++--- caravel/assets/javascripts/SqlLab/reducers.js | 14 ++++++++++ .../spec/javascripts/sqllab/reducers_spec.js | 28 +++++++++++++++++++ 4 files changed, 52 insertions(+), 4 deletions(-) create mode 100644 caravel/assets/spec/javascripts/sqllab/reducers_spec.js diff --git a/caravel/assets/javascripts/SqlLab/actions.js b/caravel/assets/javascripts/SqlLab/actions.js index 74889528e2eb9..dab04aace6bad 100644 --- a/caravel/assets/javascripts/SqlLab/actions.js +++ b/caravel/assets/javascripts/SqlLab/actions.js @@ -1,5 +1,6 @@ export const RESET_STATE = 'RESET_STATE'; export const ADD_QUERY_EDITOR = 'ADD_QUERY_EDITOR'; +export const CLONE_QUERY_TO_NEW_TAB = 'CLONE_QUERY_TO_NEW_TAB'; export const REMOVE_QUERY_EDITOR = 'REMOVE_QUERY_EDITOR'; export const ADD_TABLE = 'ADD_TABLE'; export const REMOVE_TABLE = 'REMOVE_TABLE'; @@ -37,6 +38,10 @@ export function addQueryEditor(queryEditor) { return { type: ADD_QUERY_EDITOR, queryEditor }; } +export function cloneQueryToNewTab(query) { + return { type: CLONE_QUERY_TO_NEW_TAB, query }; +} + export function setNetworkStatus(networkOn) { return { type: SET_NETWORK_STATUS, networkOn }; } diff --git a/caravel/assets/javascripts/SqlLab/components/QueryTable.jsx b/caravel/assets/javascripts/SqlLab/components/QueryTable.jsx index 77ac6715fee79..3f80058be6db0 100644 --- a/caravel/assets/javascripts/SqlLab/components/QueryTable.jsx +++ b/caravel/assets/javascripts/SqlLab/components/QueryTable.jsx @@ -41,10 +41,11 @@ class QueryTable extends React.Component { restoreSql(query) { this.props.actions.queryEditorSetSql({ id: query.sqlEditorId }, query.sql); } - notImplemented() { - /* eslint no-alert: 0 */ - alert('Not implemented yet!'); + + openQueryInNewTab(query) { + this.props.actions.cloneQueryToNewTab(query); } + render() { const data = this.props.queries.map((query) => { const q = Object.assign({}, query); @@ -112,7 +113,7 @@ class QueryTable extends React.Component { /> diff --git a/caravel/assets/javascripts/SqlLab/reducers.js b/caravel/assets/javascripts/SqlLab/reducers.js index d711d64525c4e..ff1dabd9e92a0 100644 --- a/caravel/assets/javascripts/SqlLab/reducers.js +++ b/caravel/assets/javascripts/SqlLab/reducers.js @@ -34,6 +34,20 @@ export const sqlLabReducer = function (state, action) { const newState = Object.assign({}, state, { tabHistory }); return addToArr(newState, 'queryEditors', action.queryEditor); }, + [actions.CLONE_QUERY_TO_NEW_TAB]() { + const progenitor = state.queryEditors.find((qe) => + qe.id === state.tabHistory[state.tabHistory.length - 1]); + const qe = { + id: shortid.generate(), + title: `Copy of ${progenitor.title}`, + dbId: (action.query.dbId) ? action.query.dbId : null, + schema: (action.query.schema) ? action.query.schema : null, + autorun: true, + sql: action.query.sql, + }; + + return sqlLabReducer(state, actions.addQueryEditor(qe)); + }, [actions.REMOVE_QUERY_EDITOR]() { let newState = removeFromArr(state, 'queryEditors', action.queryEditor); // List of remaining queryEditor ids diff --git a/caravel/assets/spec/javascripts/sqllab/reducers_spec.js b/caravel/assets/spec/javascripts/sqllab/reducers_spec.js new file mode 100644 index 0000000000000..15b40720485a4 --- /dev/null +++ b/caravel/assets/spec/javascripts/sqllab/reducers_spec.js @@ -0,0 +1,28 @@ +import * as r from '../../../javascripts/SqlLab/reducers'; +import * as actions from '../../../javascripts/SqlLab/actions'; +import { describe, it } from 'mocha'; +import { expect } from 'chai'; + +describe('sqlLabReducer', () => { + describe('CLONE_QUERY_TO_NEW_TAB', () => { + const testQuery = { sql: 'SELECT * FROM...', dbId: 1, id: 1 }; + const state = Object.assign(r.initialState, { queries: [testQuery] }); + const newState = r.sqlLabReducer(state, actions.cloneQueryToNewTab(testQuery)); + + it('should have at most one more tab', () => { + expect(newState.queryEditors).have.length(2); + }); + + it('should have the same SQL as the cloned query', () => { + expect(newState.queryEditors[1].sql).to.equal(testQuery.sql); + }); + + it('should prefix the new tab title with "Copy of"', () => { + expect(newState.queryEditors[1].title).to.include('Copy of'); + }); + + it('should push the cloned tab onto tab history stack', () => { + expect(newState.tabHistory[1]).to.eq(newState.queryEditors[1].id); + }); + }); +});