From 8959d02e1e5ecd66b4cbf1dade2682a63cbe9a79 Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Tue, 4 Apr 2017 00:18:41 -0700 Subject: [PATCH 1/2] [explore] force control validation before runQuery --- .../javascripts/explorev2/components/ChartContainer.jsx | 1 + .../assets/javascripts/explorev2/components/Control.jsx | 1 + .../explorev2/components/ExploreViewContainer.jsx | 8 +++++++- .../javascripts/explorev2/components/QueryAndSaveBtns.jsx | 3 ++- superset/assets/javascripts/explorev2/index.jsx | 2 +- superset/assets/javascripts/explorev2/stores/visTypes.js | 4 ---- superset/viz.py | 2 +- 7 files changed, 13 insertions(+), 8 deletions(-) diff --git a/superset/assets/javascripts/explorev2/components/ChartContainer.jsx b/superset/assets/javascripts/explorev2/components/ChartContainer.jsx index 1053222dac4a..75cb64c99eff 100644 --- a/superset/assets/javascripts/explorev2/components/ChartContainer.jsx +++ b/superset/assets/javascripts/explorev2/components/ChartContainer.jsx @@ -58,6 +58,7 @@ class ChartContainer extends React.PureComponent { componentDidUpdate(prevProps) { if ( + this.props.queryResponse && ( prevProps.queryResponse !== this.props.queryResponse || prevProps.height !== this.props.height || diff --git a/superset/assets/javascripts/explorev2/components/Control.jsx b/superset/assets/javascripts/explorev2/components/Control.jsx index ca8ead8609cf..51a1fc6c8fbe 100644 --- a/superset/assets/javascripts/explorev2/components/Control.jsx +++ b/superset/assets/javascripts/explorev2/components/Control.jsx @@ -48,6 +48,7 @@ export default class Control extends React.PureComponent { super(props); this.validate = this.validate.bind(this); this.onChange = this.onChange.bind(this); + this.onChange(props.value, []); } onChange(value, errors) { let validationErrors = this.validate(value); diff --git a/superset/assets/javascripts/explorev2/components/ExploreViewContainer.jsx b/superset/assets/javascripts/explorev2/components/ExploreViewContainer.jsx index b57b6e6ea576..d1893ea7031c 100644 --- a/superset/assets/javascripts/explorev2/components/ExploreViewContainer.jsx +++ b/superset/assets/javascripts/explorev2/components/ExploreViewContainer.jsx @@ -48,7 +48,9 @@ class ExploreViewContainer extends React.Component { componentDidUpdate() { if (this.props.triggerQuery) { - this.runQuery(); + if (this.hasErrors()) { + this.runQuery(); + } } } @@ -95,6 +97,10 @@ class ExploreViewContainer extends React.Component { toggleModal() { this.setState({ showModal: !this.state.showModal }); } + hasErrors() { + const ctrls = this.props.controls; + Object.keys(ctrls).some(k => ctrls[k].validationErrors.length > 0); + } renderErrorMessage() { // Returns an error message as a node if any errors are in the store const errors = []; diff --git a/superset/assets/javascripts/explorev2/components/QueryAndSaveBtns.jsx b/superset/assets/javascripts/explorev2/components/QueryAndSaveBtns.jsx index 812e670f48f8..506ca28d6e68 100644 --- a/superset/assets/javascripts/explorev2/components/QueryAndSaveBtns.jsx +++ b/superset/assets/javascripts/explorev2/components/QueryAndSaveBtns.jsx @@ -9,7 +9,7 @@ const propTypes = { onSave: PropTypes.func, onStop: PropTypes.func, loading: PropTypes.bool, - errorMessage: PropTypes.string, + errorMessage: PropTypes.node, }; const defaultProps = { @@ -37,6 +37,7 @@ export default function QueryAndSaveBtns( className="query" onClick={onQuery} bsStyle={qryButtonStyle} + disabled={errorMessage} > Query diff --git a/superset/assets/javascripts/explorev2/index.jsx b/superset/assets/javascripts/explorev2/index.jsx index f2757eb5e698..e72b8c39412c 100644 --- a/superset/assets/javascripts/explorev2/index.jsx +++ b/superset/assets/javascripts/explorev2/index.jsx @@ -29,7 +29,7 @@ import { exploreReducer } from './reducers/exploreReducer'; // Initial state const bootstrappedState = Object.assign( bootstrapData, { - chartStatus: 'loading', + chartStatus: null, chartUpdateEndTime: null, chartUpdateStartTime: now(), dashboards: [], diff --git a/superset/assets/javascripts/explorev2/stores/visTypes.js b/superset/assets/javascripts/explorev2/stores/visTypes.js index a82665dcb4f6..e28a4ccdcfb8 100644 --- a/superset/assets/javascripts/explorev2/stores/visTypes.js +++ b/superset/assets/javascripts/explorev2/stores/visTypes.js @@ -255,10 +255,6 @@ const visTypes = { }, ], controlOverrides: { - metrics: { - default: null, - validators: null, - }, time_grain_sqla: { default: null, }, diff --git a/superset/viz.py b/superset/viz.py index ae440ef949a1..6538a25e4346 100755 --- a/superset/viz.py +++ b/superset/viz.py @@ -112,7 +112,7 @@ def query_obj(self): """Building a query object""" form_data = self.form_data groupby = form_data.get("groupby") or [] - metrics = form_data.get("metrics") or ['count'] + metrics = form_data.get("metrics") or [] # extra_filters are temporary/contextual filters that are external # to the slice definition. We use those for dynamic interactive From 04c933caa699b7a4542201f451e2d9acb7dfcb13 Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Mon, 10 Apr 2017 15:22:55 -0700 Subject: [PATCH 2/2] Addressing comments --- .../assets/javascripts/explorev2/components/Control.jsx | 5 ++++- .../explorev2/components/ExploreViewContainer.jsx | 8 +++----- .../javascripts/explorev2/components/QueryAndSaveBtns.jsx | 2 +- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/superset/assets/javascripts/explorev2/components/Control.jsx b/superset/assets/javascripts/explorev2/components/Control.jsx index 51a1fc6c8fbe..2c503ff32766 100644 --- a/superset/assets/javascripts/explorev2/components/Control.jsx +++ b/superset/assets/javascripts/explorev2/components/Control.jsx @@ -48,9 +48,12 @@ export default class Control extends React.PureComponent { super(props); this.validate = this.validate.bind(this); this.onChange = this.onChange.bind(this); - this.onChange(props.value, []); + this.validateAndSetValue(props.value, []); } onChange(value, errors) { + this.validateAndSetValue(value, errors); + } + validateAndSetValue(value, errors) { let validationErrors = this.validate(value); if (errors && errors.length > 0) { validationErrors = validationErrors.concat(errors); diff --git a/superset/assets/javascripts/explorev2/components/ExploreViewContainer.jsx b/superset/assets/javascripts/explorev2/components/ExploreViewContainer.jsx index d1893ea7031c..093f26db4a6f 100644 --- a/superset/assets/javascripts/explorev2/components/ExploreViewContainer.jsx +++ b/superset/assets/javascripts/explorev2/components/ExploreViewContainer.jsx @@ -47,10 +47,8 @@ class ExploreViewContainer extends React.Component { } componentDidUpdate() { - if (this.props.triggerQuery) { - if (this.hasErrors()) { - this.runQuery(); - } + if (this.props.triggerQuery && !this.hasErrors()) { + this.runQuery(); } } @@ -99,7 +97,7 @@ class ExploreViewContainer extends React.Component { } hasErrors() { const ctrls = this.props.controls; - Object.keys(ctrls).some(k => ctrls[k].validationErrors.length > 0); + return Object.keys(ctrls).some(k => ctrls[k].validationErrors.length > 0); } renderErrorMessage() { // Returns an error message as a node if any errors are in the store diff --git a/superset/assets/javascripts/explorev2/components/QueryAndSaveBtns.jsx b/superset/assets/javascripts/explorev2/components/QueryAndSaveBtns.jsx index 506ca28d6e68..023ce98c3841 100644 --- a/superset/assets/javascripts/explorev2/components/QueryAndSaveBtns.jsx +++ b/superset/assets/javascripts/explorev2/components/QueryAndSaveBtns.jsx @@ -37,7 +37,7 @@ export default function QueryAndSaveBtns( className="query" onClick={onQuery} bsStyle={qryButtonStyle} - disabled={errorMessage} + disabled={!!errorMessage} > Query