Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor the queries to be stored as a dict. #994

Merged
merged 1 commit into from
Aug 23, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 9 additions & 4 deletions caravel/assets/javascripts/SqlLab/components/QueryHistory.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,17 @@ import { Alert } from 'react-bootstrap';

const QueryHistory = (props) => {
const activeQeId = props.tabHistory[props.tabHistory.length - 1];
const queries = props.queries.filter((q) => (q.sqlEditorId === activeQeId));
if (queries.length > 0) {
const queriesArray = []
for (var query_id in props.queries) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.filter would be more idiomatic, checkout $.map

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.filter doesn't work on object, $.map - wouldn't filter the values out.
Let's keep it as it is - fairly simple construct and we can change it if that gets complicated.

if (props.queries[query_id].sqlEditorId === activeQeId) {
queriesArray.push(props.queries[query_id])
}
}
if (queriesArray.length > 0) {
return (
<QueryTable
columns={['state', 'started', 'duration', 'rows', 'sql', 'actions']}
queries={queries}
queries={queriesArray}
/>
);
}
Expand All @@ -30,7 +35,7 @@ QueryHistory.defaultProps = {
};

QueryHistory.propTypes = {
queries: React.PropTypes.array,
queries: React.PropTypes.object,
tabHistory: React.PropTypes.array,
actions: React.PropTypes.object,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,11 @@ class QueryEditors extends React.Component {
render() {
const editors = this.props.queryEditors.map((qe, i) => {
let latestQuery;
this.props.queries.forEach((q) => {
if (q.id === qe.latestQueryId) {
latestQuery = q;
for (var key in this.props.queries) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need for a loop!
let latestQuery = this.props.queries[qe.latestQueryId]

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will do that

if (key === qe.latestQueryId) {
latestQuery = this.props.queries[key];
}
});
}
const state = (latestQuery) ? latestQuery.state : '';
const tabTitle = (
<div>
Expand Down Expand Up @@ -99,7 +99,7 @@ class QueryEditors extends React.Component {
}
QueryEditors.propTypes = {
actions: React.PropTypes.object,
queries: React.PropTypes.array,
queries: React.PropTypes.object,
queryEditors: React.PropTypes.array,
tabHistory: React.PropTypes.array,
};
Expand Down
40 changes: 33 additions & 7 deletions caravel/assets/javascripts/SqlLab/reducers.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,39 @@ const defaultQueryEditor = {

export const initialState = {
alerts: [],
queries: [],
queries: {},
queryEditors: [defaultQueryEditor],
tabHistory: [defaultQueryEditor.id],
tables: [],
workspaceQueries: [],
};

function removeFromObject(state, arrKey, obj, idKey = 'id') {
const newObject = {};
for (const key in state[arrKey]) {
if (!(key === obj[idKey])) {
newObject[idKey] = state[arrKey][idKey];
}
}
return Object.assign({}, state, { [arrKey]: newObject });
}

function addToObject(state, arrKey, obj) {
const newObject = Object.assign({}, state[arrKey]);
const copiedObject = Object.assign({}, obj);

if (!copiedObject.id) {
copiedObject.id = shortid.generate();
}
newObject[copiedObject.id] = copiedObject;
return Object.assign({}, state, { [arrKey]: newObject });
}

function alterInObject(state, arrKey, obj, alterations) {
const newObject = Object.assign({}, state[arrKey]);
newObject[obj.id] = (Object.assign({}, newObject[obj.id], alterations));
return Object.assign({}, state, { [arrKey]: newObject });
}

function alterInArr(state, arrKey, obj, alterations) {
// Finds an item in an array in the state and replaces it with a
Expand Down Expand Up @@ -48,7 +74,7 @@ function removeFromArr(state, arrKey, obj, idKey = 'id') {

function addToArr(state, arrKey, obj) {
const newObj = Object.assign({}, obj);
if (!(newObj.id)) {
if (!newObj.id) {
newObj.id = shortid.generate();
}
const newState = {};
Expand All @@ -74,7 +100,7 @@ export const sqlLabReducer = function (state, action) {
return newState;
},
[actions.REMOVE_QUERY]() {
return removeFromArr(state, 'queries', action.query);
return removeFromObject(state, 'queries', action.query);
},
[actions.RESET_STATE]() {
return Object.assign({}, initialState);
Expand All @@ -92,12 +118,12 @@ export const sqlLabReducer = function (state, action) {
return removeFromArr(state, 'tables', action.table);
},
[actions.START_QUERY]() {
const newState = addToArr(state, 'queries', action.query);
const newState = addToObject(state, 'queries', action.query);
const sqlEditor = { id: action.query.sqlEditorId };
return alterInArr(newState, 'queryEditors', sqlEditor, { latestQueryId: action.query.id });
},
[actions.STOP_QUERY]() {
return alterInArr(state, 'queries', action.query, { state: 'stopped' });
return alterInObject(state, 'queries', action.query, { state: 'stopped' });
},
[actions.QUERY_SUCCESS]() {
const alts = {
Expand All @@ -106,11 +132,11 @@ export const sqlLabReducer = function (state, action) {
rows: action.results.data.length,
endDttm: moment(),
};
return alterInArr(state, 'queries', action.query, alts);
return alterInObject(state, 'queries', action.query, alts);
},
[actions.QUERY_FAILED]() {
const alts = { state: 'failed', msg: action.msg, endDttm: moment() };
return alterInArr(state, 'queries', action.query, alts);
return alterInObject(state, 'queries', action.query, alts);
},
[actions.SET_ACTIVE_QUERY_EDITOR]() {
const qeIds = state.queryEditors.map((qe) => qe.id);
Expand Down