Skip to content

Commit

Permalink
Enable "Run Query in New Tab" in SQL Lab (#1343)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
Nick Barnwell authored and mistercrunch committed Oct 18, 2016
1 parent 8f29944 commit 2edce5b
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 4 deletions.
5 changes: 5 additions & 0 deletions caravel/assets/javascripts/SqlLab/actions.js
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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 };
}
Expand Down
9 changes: 5 additions & 4 deletions caravel/assets/javascripts/SqlLab/components/QueryTable.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -112,7 +113,7 @@ class QueryTable extends React.Component {
/>
<Link
className="fa fa-plus-circle m-r-3"
onClick={self.notImplemented}
onClick={this.openQueryInNewTab.bind(this, query)}
tooltip="Run query in a new tab"
placement="top"
/>
Expand Down
14 changes: 14 additions & 0 deletions caravel/assets/javascripts/SqlLab/reducers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
28 changes: 28 additions & 0 deletions caravel/assets/spec/javascripts/sqllab/reducers_spec.js
Original file line number Diff line number Diff line change
@@ -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);
});
});
});

0 comments on commit 2edce5b

Please sign in to comment.