[explorev2] adding support for client side validators on controls (#1920)

* Adding support for client side validators on controls

* Applying validators to more fields

* Addressing comments

superset/assets/javascripts/explore/components/QueryAndSaveBtns.jsx

@@ -1,5 +1,6 @@
 import React, { PropTypes } from 'react';
-import { Button, ButtonGroup } from 'react-bootstrap';
+import { ButtonGroup, OverlayTrigger, Tooltip } from 'react-bootstrap';
+import Button from '../../components/Button';
 import classnames from 'classnames';
 
 const propTypes = {
@@ -7,6 +8,7 @@ const propTypes = {
   onQuery: PropTypes.func.isRequired,
   onSave: PropTypes.func,
   disabled: PropTypes.bool,
+  errorMessage: PropTypes.string,
 };
 
 const defaultProps = {
@@ -14,33 +16,49 @@ const defaultProps = {
   disabled: false,
 };
 
-export default function QueryAndSaveBtns({ canAdd, onQuery, onSave, disabled }) {
+export default function QueryAndSaveBtns({ canAdd, onQuery, onSave, disabled, errorMessage }) {
   const saveClasses = classnames({
     'disabled disabledButton': canAdd !== 'True',
   });
+  const qryButtonStyle = errorMessage ? 'danger' : 'primary';
+  const qryButtonDisabled = errorMessage ? true : disabled;
 
   return (
-    <ButtonGroup className="query-and-save">
-      <Button
-        id="query_button"
-        onClick={onQuery}
-        bsSize="small"
-        disabled={disabled}
-        bsStyle="primary"
-      >
-        <i className="fa fa-bolt" /> Query
-      </Button>
-      <Button
-        className={saveClasses}
-        bsSize="small"
-        data-target="#save_modal"
-        data-toggle="modal"
-        disabled={disabled}
-        onClick={onSave}
-      >
-        <i className="fa fa-plus-circle"></i> Save as
-      </Button>
-    </ButtonGroup>
+    <div>
+      <ButtonGroup className="query-and-save">
+        <Button
+          id="query_button"
+          onClick={onQuery}
+          disabled={qryButtonDisabled}
+          bsStyle={qryButtonStyle}
+        >
+          <i className="fa fa-bolt" /> Query
+        </Button>
+        <Button
+          className={saveClasses}
+          data-target="#save_modal"
+          data-toggle="modal"
+          disabled={qryButtonDisabled}
+          onClick={onSave}
+        >
+          <i className="fa fa-plus-circle"></i> Save as
+        </Button>
+      </ButtonGroup>
+      {errorMessage &&
+        <span>
+          {' '}
+          <OverlayTrigger
+            placement="right"
+            overlay={
+              <Tooltip id={'query-error-tooltip'}>
+                {errorMessage}
+              </Tooltip>}
+          >
+            <i className="fa fa-exclamation-circle text-danger fa-lg" />
+          </OverlayTrigger>
+        </span>
+      }
+    </div>
   );
 }
 

superset/assets/javascripts/explorev2/actions/exploreActions.js

@@ -114,8 +114,8 @@ export function fetchFilterValues(datasource_type, datasource_id, filter, col) {
 }
 
 export const SET_FIELD_VALUE = 'SET_FIELD_VALUE';
-export function setFieldValue(datasource_type, key, value, label) {
-  return { type: SET_FIELD_VALUE, datasource_type, key, value, label };
+export function setFieldValue(fieldName, value, validationErrors) {
+  return { type: SET_FIELD_VALUE, fieldName, value, validationErrors };
 }
 
 export const CHART_UPDATE_STARTED = 'CHART_UPDATE_STARTED';

superset/assets/javascripts/explorev2/components/CheckboxField.jsx

@@ -11,14 +11,12 @@ const propTypes = {
 
 const defaultProps = {
   value: false,
-  label: null,
-  description: null,
   onChange: () => {},
 };
 
 export default class CheckboxField extends React.Component {
   onToggle() {
-    this.props.onChange(this.props.name);
+    this.props.onChange(!this.props.value);
   }
   render() {
     return (

superset/assets/javascripts/explorev2/components/ControlHeader.jsx

@@ -0,0 +1,48 @@
+import React, { PropTypes } from 'react';
+import { ControlLabel, OverlayTrigger, Tooltip } from 'react-bootstrap';
+import InfoTooltipWithTrigger from '../../components/InfoTooltipWithTrigger';
+
+const propTypes = {
+  label: PropTypes.string.isRequired,
+  description: PropTypes.string,
+  validationErrors: PropTypes.array,
+};
+
+const defaultProps = {
+  description: null,
+  validationErrors: [],
+};
+
+export default function ControlHeader({ label, description, validationErrors }) {
+  const hasError = (validationErrors.length > 0);
+  return (
+    <ControlLabel>
+      {hasError ?
+        <strong className="text-danger">{label}</strong> :
+        <span>{label}</span>
+      }
+      {' '}
+      {(validationErrors.length > 0) &&
+        <span>
+          <OverlayTrigger
+            placement="right"
+            overlay={
+              <Tooltip id={'error-tooltip'}>
+                {validationErrors.join(' ')}
+              </Tooltip>
+            }
+          >
+            <i className="fa fa-exclamation-circle text-danger" />
+          </OverlayTrigger>
+          {' '}
+        </span>
+      }
+      {description &&
+        <InfoTooltipWithTrigger label={label} tooltip={description} />
+      }
+    </ControlLabel>
+  );
+}
+
+ControlHeader.propTypes = propTypes;
+ControlHeader.defaultProps = defaultProps;

superset/assets/javascripts/explorev2/components/ControlLabelWithTooltip.jsx

@@ -1,26 +0,0 @@
-import React, { PropTypes } from 'react';
-import { ControlLabel } from 'react-bootstrap';
-import InfoTooltipWithTrigger from '../../components/InfoTooltipWithTrigger';
-
-const propTypes = {
-  label: PropTypes.string.isRequired,
-  description: PropTypes.string,
-};
-
-const defaultProps = {
-  description: null,
-};
-
-export default function ControlLabelWithTooltip({ label, description }) {
-  return (
-    <ControlLabel>
-      {label} &nbsp;
-      {description &&
-        <InfoTooltipWithTrigger label={label} tooltip={description} />
-      }
-    </ControlLabel>
-  );
-}
-
-ControlLabelWithTooltip.propTypes = propTypes;
-ControlLabelWithTooltip.defaultProps = defaultProps;

superset/assets/javascripts/explorev2/components/ControlPanelsContainer.jsx

@@ -27,7 +27,6 @@ class ControlPanelsContainer extends React.Component {
     this.fieldOverrides = this.fieldOverrides.bind(this);
     this.getFieldData = this.getFieldData.bind(this);
     this.removeAlert = this.removeAlert.bind(this);
-    this.onChange = this.onChange.bind(this);
   }
   componentWillMount() {
     const datasource_id = this.props.form_data.datasource;
@@ -44,14 +43,8 @@ class ControlPanelsContainer extends React.Component {
       }
     }
   }
-  onChange(name, value) {
-    this.props.actions.setFieldValue(this.props.datasource_type, name, value);
-  }
   getFieldData(fs) {
     const fieldOverrides = this.fieldOverrides();
-    if (!this.props.fields) {
-      return null;
-    }
     let fieldData = this.props.fields[fs] || {};
     if (fieldOverrides.hasOwnProperty(fs)) {
       const overrideData = fieldOverrides[fs];
@@ -100,13 +93,14 @@ class ControlPanelsContainer extends React.Component {
               {section.fieldSetRows.map((fieldSets, i) => (
                 <FieldSetRow
                   key={`fieldsetrow-${i}`}
-                  fields={fieldSets.map(field => (
+                  fields={fieldSets.map(fieldName => (
                     <FieldSet
-                      name={field}
-                      key={`field-${field}`}
-                      onChange={this.onChange}
-                      value={this.props.form_data[field]}
-                      {...this.getFieldData(field)}
+                      name={fieldName}
+                      key={`field-${fieldName}`}
+                      value={this.props.form_data[fieldName]}
+                      validationErrors={this.props.fields[fieldName].validationErrors}
+                      actions={this.props.actions}
+                      {...this.getFieldData(fieldName)}
                     />
                   ))}
                 />

superset/assets/javascripts/explorev2/components/ExploreViewContainer.jsx

@@ -15,6 +15,7 @@ const propTypes = {
   actions: React.PropTypes.object.isRequired,
   datasource_type: React.PropTypes.string.isRequired,
   chartStatus: React.PropTypes.string.isRequired,
+  fields: React.PropTypes.object.isRequired,
 };
 
 
@@ -72,6 +73,28 @@ class ExploreViewContainer extends React.Component {
   toggleModal() {
     this.setState({ showModal: !this.state.showModal });
   }
+  renderErrorMessage() {
+    // Returns an error message as a node if any errors are in the store
+    const errors = [];
+    for (const fieldName in this.props.fields) {
+      const field = this.props.fields[fieldName];
+      if (field.validationErrors && field.validationErrors.length > 0) {
+        errors.push(
+          <div key={fieldName}>
+            <strong>{`[ ${field.label} ] `}</strong>
+            {field.validationErrors.join('. ')}
+          </div>
+        );
+      }
+    }
+    let errorMessage;
+    if (errors.length > 0) {
+      errorMessage = (
+        <div style={{ textAlign: 'left' }}>{errors}</div>
+      );
+    }
+    return errorMessage;
+  }
 
   render() {
     return (
@@ -98,8 +121,9 @@ class ExploreViewContainer extends React.Component {
               onQuery={this.onQuery.bind(this, this.props.form_data)}
               onSave={this.toggleModal.bind(this)}
               disabled={this.props.chartStatus === 'loading'}
+              errorMessage={this.renderErrorMessage()}
             />
-            <br /><br />
+            <br />
             <ControlPanelsContainer
               actions={this.props.actions}
               form_data={this.props.form_data}
@@ -126,6 +150,7 @@ function mapStateToProps(state) {
     datasource_type: state.datasource_type,
     form_data: state.viz.form_data,
     chartStatus: state.chartStatus,
+    fields: state.fields,
   };
 }
 

superset/assets/javascripts/explorev2/components/FieldSet.jsx

@@ -4,7 +4,7 @@ import CheckboxField from './CheckboxField';
 import TextAreaField from './TextAreaField';
 import SelectField from './SelectField';
 
-import ControlLabelWithTooltip from './ControlLabelWithTooltip';
+import ControlHeader from './ControlHeader';
 
 const fieldMap = {
   TextField,
@@ -15,14 +15,15 @@ const fieldMap = {
 const fieldTypes = Object.keys(fieldMap);
 
 const propTypes = {
+  actions: PropTypes.object.isRequired,
   name: PropTypes.string.isRequired,
   type: PropTypes.oneOf(fieldTypes).isRequired,
   label: PropTypes.string.isRequired,
   choices: PropTypes.arrayOf(PropTypes.array),
   description: PropTypes.string,
   places: PropTypes.number,
-  validators: PropTypes.any,
-  onChange: React.PropTypes.func,
+  validators: PropTypes.array,
+  validationErrors: PropTypes.array,
   value: PropTypes.oneOfType([
     PropTypes.string,
     PropTypes.number,
@@ -31,16 +32,46 @@ const propTypes = {
 };
 
 const defaultProps = {
-  onChange: () => {},
+  validators: [],
+  validationErrors: [],
 };
 
 export default class FieldSet extends React.PureComponent {
+  constructor(props) {
+    super(props);
+    this.validate = this.validate.bind(this);
+    this.onChange = this.onChange.bind(this);
+  }
+  onChange(value) {
+    const validationErrors = this.validate(value);
+    this.props.actions.setFieldValue(this.props.name, value, validationErrors);
+  }
+  validate(value) {
+    const validators = this.props.validators;
+    const validationErrors = [];
+    if (validators && validators.length > 0) {
+      validators.forEach(f => {
+        const v = f(value);
+        if (v) {
+          validationErrors.push(v);
+        }
+      });
+    }
+    return validationErrors;
+  }
   render() {
     const FieldType = fieldMap[this.props.type];
     return (
       <div>
-        <ControlLabelWithTooltip label={this.props.label} description={this.props.description} />
-        <FieldType {...this.props} />
+        <ControlHeader
+          label={this.props.label}
+          description={this.props.description}
+          validationErrors={this.props.validationErrors}
+        />
+        <FieldType
+          onChange={this.onChange}
+          {...this.props}
+        />
       </div>
     );
   }

superset/assets/javascripts/explorev2/components/SelectField.jsx

@@ -33,11 +33,7 @@ export default class SelectField extends React.Component {
     if (this.props.multi) {
       optionValue = opt ? opt.map((o) => o.value) : null;
     }
-    if (this.props.name === 'datasource' && optionValue !== null) {
-      this.props.onChange(this.props.name, optionValue, opt.label);
-    } else {
-      this.props.onChange(this.props.name, optionValue);
-    }
+    this.props.onChange(optionValue);
   }
   renderOption(opt) {
     if (this.props.name === 'viz_type') {

superset/assets/javascripts/explorev2/components/TextAreaField.jsx

@@ -18,7 +18,7 @@ const defaultProps = {
 
 export default class TextAreaField extends React.Component {
   onChange(event) {
-    this.props.onChange(this.props.name, event.target.value);
+    this.props.onChange(event.target.value);
   }
   render() {
     return (

superset/assets/javascripts/explorev2/components/TextField.jsx

@@ -18,16 +18,17 @@ const defaultProps = {
 
 export default class TextField extends React.Component {
   onChange(event) {
-    this.props.onChange(this.props.name, event.target.value);
+    this.props.onChange(event.target.value);
   }
   render() {
+    const value = this.props.value || '';
     return (
       <FormGroup controlId="formInlineName" bsSize="small">
         <FormControl
           type="text"
           placeholder=""
           onChange={this.onChange.bind(this)}
-          value={this.props.value}
+          value={value}
         />
       </FormGroup>
     );

superset/assets/javascripts/explorev2/reducers/exploreReducer.js

@@ -30,7 +30,6 @@ export const exploreReducer = function (state, action) {
     [actions.REMOVE_CONTROL_PANEL_ALERT]() {
       return Object.assign({}, state, { controlPanelAlert: null });
     },
-
     [actions.FETCH_DASHBOARDS_SUCCEEDED]() {
       return Object.assign({}, state, { dashboards: action.choices });
     },
@@ -74,24 +73,27 @@ export const exploreReducer = function (state, action) {
       );
     },
     [actions.SET_FIELD_VALUE]() {
-      const newFormData = action.key === 'datasource' ?
-        defaultFormData(state.viz.form_data.viz_type, action.datasource_type) :
-        Object.assign({}, state.viz.form_data);
-      if (action.key === 'datasource') {
+      let newFormData = Object.assign({}, state.viz.form_data);
+      if (action.fieldName === 'datasource') {
+        newFormData = defaultFormData(state.viz.form_data.viz_type, action.datasource_type);
         newFormData.datasource_name = action.label;
         newFormData.slice_id = state.viz.form_data.slice_id;
         newFormData.slice_name = state.viz.form_data.slice_name;
         newFormData.viz_type = state.viz.form_data.viz_type;
       }
-      if (action.key === 'viz_type') {
-        newFormData.previous_viz_type = state.viz.form_data.viz_type;
-      }
-      newFormData[action.key] = (action.value !== undefined)
-        ? action.value : (!state.viz.form_data[action.key]);
+      newFormData[action.fieldName] = action.value;
+
+      const fields = Object.assign({}, state.fields);
+      const field = fields[action.fieldName];
+      field.value = action.value;
+      field.validationErrors = action.validationErrors;
       return Object.assign(
         {},
         state,
-        { viz: Object.assign({}, state.viz, { form_data: newFormData }) }
+        {
+          fields,
+          viz: Object.assign({}, state.viz, { form_data: newFormData }),
+        }
       );
     },
     [actions.CHART_UPDATE_SUCCEEDED]() {

superset/assets/javascripts/explorev2/stores/fields.js

@@ -1,5 +1,6 @@
 import { formatSelectOptionsForRange, formatSelectOptions } from '../../modules/utils';
 import visTypes from './visTypes';
+import * as v from '../validators';
 
 const D3_FORMAT_DOCS = 'D3 format syntax: https://github.com/d3/d3-format';
 
@@ -51,6 +52,7 @@ export const fields = {
     type: 'SelectField',
     multi: true,
     label: 'Metrics',
+    validators: [v.nonEmpty],
     mapStateToProps: (state) => ({
       choices: (state.datasource) ? state.datasource.metrics_combo : [],
     }),
@@ -508,6 +510,7 @@ export const fields = {
   treemap_ratio: {
     type: 'TextField',
     label: 'Ratio',
+    validators: [v.numeric],
     default: 0.5 * (1 + Math.sqrt(5)),  // d3 default, golden ratio
     description: 'Target aspect ratio for treemap tiles.',
   },
@@ -560,7 +563,7 @@ export const fields = {
   rolling_periods: {
     type: 'TextField',
     label: 'Periods',
-    validators: [],
+    validators: [v.integer],
     description: 'Defines the size of the rolling window function, ' +
                  'relative to the time granularity selected',
   },
@@ -659,6 +662,7 @@ export const fields = {
   compare_lag: {
     type: 'TextField',
     label: 'Comparison Period Lag',
+    validators: [v.integer],
     description: 'Based on granularity, number of time periods to compare against',
   },
 
@@ -785,6 +789,7 @@ export const fields = {
 
   size_from: {
     type: 'TextField',
+    validators: [v.integer],
     label: 'Font Size From',
     default: '20',
     description: 'Font size for the smallest value in the list',
@@ -792,6 +797,7 @@ export const fields = {
 
   size_to: {
     type: 'TextField',
+    validators: [v.integer],
     label: 'Font Size To',
     default: '150',
     description: 'Font size for the biggest value in the list',
@@ -907,7 +913,7 @@ export const fields = {
     type: 'TextField',
     label: 'Period Ratio',
     default: '',
-    validators: [],
+    validators: [v.integer],
     description: '[integer] Number of period to compare against, ' +
                  'this is relative to the granularity selected',
   },
@@ -924,6 +930,7 @@ export const fields = {
   time_compare: {
     type: 'TextField',
     label: 'Time Shift',
+    validators: [v.integer],
     default: null,
     description: 'Overlay a timeseries from a ' +
                  'relative time period. Expects relative time delta ' +
@@ -1011,6 +1018,7 @@ export const fields = {
     type: 'TextField',
     label: 'Opacity',
     default: 1,
+    validators: [v.numeric],
     description: 'Opacity of all clusters, points, and labels. ' +
                  'Between 0 and 1.',
   },
@@ -1018,8 +1026,8 @@ export const fields = {
   viewport_zoom: {
     type: 'TextField',
     label: 'Zoom',
+    validators: [v.numeric],
     default: 11,
-    validators: [],
     description: 'Zoom level of the map',
     places: 8,
   },
@@ -1028,6 +1036,7 @@ export const fields = {
     type: 'TextField',
     label: 'Default latitude',
     default: 37.772123,
+    validators: [v.numeric],
     description: 'Latitude of default viewport',
     places: 8,
   },
@@ -1036,6 +1045,7 @@ export const fields = {
     type: 'TextField',
     label: 'Default longitude',
     default: -122.405293,
+    validators: [v.numeric],
     description: 'Longitude of default viewport',
     places: 8,
   },

superset/assets/javascripts/explorev2/validators.js

@@ -0,0 +1,32 @@
+/* Reusable validator functions used in controls definitions
+ *
+ * validator functions receive the v and the configuration of the field
+ * as arguments and return something that evals to false if v is valid,
+ * and an error message if not valid.
+ * */
+
+export function numeric(v) {
+  if (v && isNaN(v)) {
+    return 'is expected to be a number';
+  }
+  return false;
+}
+
+export function integer(v) {
+  if (v && (isNaN(v) || parseInt(v, 10) !== +(v))) {
+    return 'is expected to be an integer';
+  }
+  return false;
+}
+
+export function nonEmpty(v) {
+  if (
+      v === null ||
+      v === undefined ||
+      v === '' ||
+      (Array.isArray(v) && v.length === 0)
+  ) {
+    return 'cannot be empty';
+  }
+  return false;
+}