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

[explore] fix query text modal while loading #2596

Merged
merged 4 commits into from
Apr 13, 2017
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion superset/assets/javascripts/components/ModalTrigger.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import cx from 'classnames';
import Button from './Button';

const propTypes = {
animation: PropTypes.bool,
triggerNode: PropTypes.node.isRequired,
modalTitle: PropTypes.node.isRequired,
modalBody: PropTypes.node, // not required because it can be generated by beforeOpen
Expand All @@ -17,6 +18,7 @@ const propTypes = {
};

const defaultProps = {
animation: true,
beforeOpen: () => {},
onExit: () => {},
isButton: false,
Expand Down Expand Up @@ -46,6 +48,7 @@ export default class ModalTrigger extends React.Component {
renderModal() {
return (
<Modal
animation={this.props.animation}
show={this.state.showModal}
onHide={this.close}
onExit={this.props.onExit}
Expand Down Expand Up @@ -73,7 +76,11 @@ export default class ModalTrigger extends React.Component {
});
if (this.props.isButton) {
return (
<Button tooltip={this.props.tooltip} onClick={this.open}>
<Button
className="modal-trigger"
tooltip={this.props.tooltip}
onClick={this.open}
>
{this.props.triggerNode}
{this.renderModal()}
</Button>
Expand Down
20 changes: 10 additions & 10 deletions superset/assets/javascripts/explorev2/components/ChartContainer.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,6 @@ class ChartContainer extends React.PureComponent {
return this.renderChart();
}
const queryResponse = this.props.queryResponse;
const query = queryResponse && queryResponse.query ? queryResponse.query : null;
return (
<div className="chart-container">
<Panel
Expand Down Expand Up @@ -266,14 +265,14 @@ class ChartContainer extends React.PureComponent {
{this.props.chartStatus === 'success' &&
this.props.queryResponse &&
this.props.queryResponse.is_cached &&
<TooltipWrapper
tooltip="Loaded from cache. Click to force refresh"
label="cache-desc"
>
<Label
style={{ fontSize: '10px', marginRight: '5px', cursor: 'pointer' }}
onClick={this.runQuery.bind(this)}
>
<TooltipWrapper
tooltip="Loaded from cache. Click to force refresh"
label="cache-desc"
>
<Label
style={{ fontSize: '10px', marginRight: '5px', cursor: 'pointer' }}
onClick={this.runQuery.bind(this)}
>
cached
</Label>
</TooltipWrapper>
Expand All @@ -288,7 +287,8 @@ class ChartContainer extends React.PureComponent {
<ExploreActionButtons
slice={this.state.mockSlice}
canDownload={this.props.can_download}
query={query}
chartStatus={this.props.chartStatus}
queryResponse={queryResponse}
queryEndpoint={getExploreUrl(this.props.latestQueryFormData, 'query')}
/>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,62 +7,91 @@ import ModalTrigger from './../../components/ModalTrigger';
const $ = window.$ = require('jquery');

const propTypes = {
query: PropTypes.string,
animation: PropTypes.bool,
queryResponse: PropTypes.object,
chartStatus: PropTypes.string,
queryEndpoint: PropTypes.string.isRequired,
};
const defaultProps = {
animation: true,
};

export default class DisplayQueryButton extends React.PureComponent {
constructor(props) {
super(props);
this.state = {
modalBody: <pre />,
language: null,
query: null,
isLoading: false,
error: null,
};
this.beforeOpen = this.beforeOpen.bind(this);
this.fetchQuery = this.fetchQuery.bind(this);
}
beforeOpen() {
fetchQuery() {
this.setState({ isLoading: true });
$.ajax({
type: 'GET',
url: this.props.queryEndpoint,
success: data => {
this.setState({
language: data.language,
query: data.query,
isLoading: false,
});
},
error: data => {
this.setState({
error: data.error,
isLoading: false,
});
},
});
}
setStateFromQueryResponse() {
const qr = this.props.queryResponse;
this.setState({
modalBody:
(<img
className="loading"
alt="Loading..."
src="/static/assets/images/loading.gif"
/>),
language: qr.language,
query: qr.query,
isLoading: false,
});
if (this.props.query) {
const modalBody = (
<pre>{this.props.query}</pre>
);
this.setState({ modalBody });
}
beforeOpen() {
if (this.props.chartStatus === 'loading' || this.props.chartStatus === null) {
this.fetchQuery();
} else {
$.ajax({
type: 'GET',
url: this.props.queryEndpoint,
success: (data) => {
const modalBody = data.language ?
(<SyntaxHighlighter language={data.language} style={github}>
{data.query}
</SyntaxHighlighter>)
:
<pre>{data.query}</pre>;
this.setState({ modalBody });
},
error(data) {
this.setState({ modalBody: (<pre>{data.error}</pre>) });
},
});
this.setStateFromQueryResponse();
}
}
renderModalBody() {
if (this.state.isLoading) {
return (<img
className="loading"
alt="Loading..."
src="/static/assets/images/loading.gif"
/>);
} else if (this.state.error) {
return <pre>{this.state.error}</pre>;
}
return (
<SyntaxHighlighter language={this.state.language} style={github}>
{this.state.query}
</SyntaxHighlighter>);
}
render() {
return (
<ModalTrigger
animation={this.props.animation}
isButton
triggerNode={<span>Query</span>}
modalTitle="Query"
bsSize="large"
beforeOpen={this.beforeOpen.bind(this)}
modalBody={this.state.modalBody}
beforeOpen={this.beforeOpen}
modalBody={this.renderModalBody()}
/>
);
}
}

DisplayQueryButton.propTypes = propTypes;
DisplayQueryButton.defaultProps = defaultProps;
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@ const propTypes = {
canDownload: PropTypes.oneOfType([PropTypes.string, PropTypes.bool]).isRequired,
slice: PropTypes.object,
queryEndpoint: PropTypes.string,
query: PropTypes.string,
queryResponse: PropTypes.object,
chartStatus: PropTypes.string,
};

export default function ExploreActionButtons({ canDownload, slice, query, queryEndpoint }) {
export default function ExploreActionButtons({
chartStatus, canDownload, slice, queryResponse, queryEndpoint }) {
const exportToCSVClasses = cx('btn btn-default btn-sm', {
'disabled disabledButton': !canDownload,
});
Expand Down Expand Up @@ -43,8 +45,9 @@ export default function ExploreActionButtons({ canDownload, slice, query, queryE
</a>

<DisplayQueryButton
query={query}
queryResponse={queryResponse}
queryEndpoint={queryEndpoint}
chartStatus={chartStatus}
/>
</div>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,30 @@
import React from 'react';
import { expect } from 'chai';
import { describe, it } from 'mocha';
import { mount } from 'enzyme';
import { Modal } from 'react-bootstrap';
import ModalTrigger from './../../../../javascripts/components/ModalTrigger.jsx';

import DisplayQueryButton from '../../../../javascripts/explorev2/components/DisplayQueryButton';

describe('DisplayQueryButton', () => {
const defaultProps = {
slice: {
viewSqlQuery: 'sql query string',
animation: false,
queryResponse: {
query: 'SELECT * FROM foo',
language: 'sql',
},
chartStatus: 'success',
queryEndpoint: 'localhost',
};

it('renders', () => {
it('is valid', () => {
expect(React.isValidElement(<DisplayQueryButton {...defaultProps} />)).to.equal(true);
});
it('renders a button and a modal', () => {
const wrapper = mount(<DisplayQueryButton {...defaultProps} />);
expect(wrapper.find(ModalTrigger)).to.have.lengthOf(1);
wrapper.find('.modal-trigger').simulate('click');
expect(wrapper.find(Modal)).to.have.lengthOf(1);
});
});
22 changes: 22 additions & 0 deletions superset/connectors/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,28 @@ def data(self):

return d

def get_query_str(self, query_obj):
"""Returns a query as a string

This is used to be displayed to the user so that she/he can
understand what is taking place behind the scene"""
raise NotImplementedError()

def query(self, query_obj):
"""Executes the query and returns a dataframe

query_obj is a dictionary representing Superset's query interface.
Should return a ``superset.models.helpers.QueryResult``
"""
raise NotImplementedError()

def values_for_column(self, column_name, limit=10000):
"""Given a column, returns an iterable of distinct values

This is used to populate the dropdown showing a list of
values in filters in the explore view"""
raise NotImplementedError()


class BaseColumn(AuditMixinNullable, ImportMixin):
"""Interface for column"""
Expand Down
10 changes: 6 additions & 4 deletions superset/connectors/druid/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -696,7 +696,10 @@ def values_for_column(self,
df = client.export_pandas()
return [row[column_name] for row in df.to_records(index=False)]

def get_query_str( # noqa / druid
def get_query_str(self, query_obj, phase=1, client=None):
return self.run_query(client=client, phase=phase, **query_obj)

def run_query( # noqa / druid
self,
groupby, metrics,
granularity,
Expand All @@ -712,8 +715,6 @@ def get_query_str( # noqa / druid
select=None, # noqa
columns=None, phase=2, client=None, form_data=None):
"""Runs a query against Druid and returns a dataframe.

This query interface is common to SqlAlchemy and Druid
"""
# TODO refactor into using a TBD Query object
client = client or self.cluster.get_pydruid_client()
Expand Down Expand Up @@ -917,7 +918,8 @@ def recursive_get_fields(_conf):
def query(self, query_obj):
qry_start_dttm = datetime.now()
client = self.cluster.get_pydruid_client()
query_str = self.get_query_str(client=client, **query_obj)
query_str = self.get_query_str(
client=client, query_obj=query_obj, phase=2)
df = client.export_pandas()

if df is None or df.size == 0:
Expand Down
6 changes: 3 additions & 3 deletions superset/connectors/sqla/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -312,9 +312,9 @@ def get_template_processor(self, **kwargs):
return get_template_processor(
table=self, database=self.database, **kwargs)

def get_query_str(self, **kwargs):
def get_query_str(self, query_obj):
engine = self.database.get_sqla_engine()
qry = self.get_sqla_query(**kwargs)
qry = self.get_sqla_query(**query_obj)
sql = str(
qry.compile(
engine,
Expand Down Expand Up @@ -538,7 +538,7 @@ def query(self, query_obj):
qry_start_dttm = datetime.now()
engine = self.database.get_sqla_engine()
qry = self.get_sqla_query(**query_obj)
sql = self.get_query_str(**query_obj)
sql = self.get_query_str(query_obj)
status = QueryStatus.SUCCESS
error_message = None
df = None
Expand Down
6 changes: 1 addition & 5 deletions superset/views/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -923,11 +923,7 @@ def explore_json(self, datasource_type, datasource_id):
if request.args.get("query") == "true":
try:
query_obj = viz_obj.query_obj()
if datasource_type == 'druid':
# only retrive first phase query for druid
query_obj['phase'] = 1
query = viz_obj.datasource.get_query_str(
datetime.now(), **query_obj)
query = viz_obj.datasource.get_query_str(query_obj)
except Exception as e:
return json_error_response(e)
return Response(
Expand Down