From 29780821e8797fee3fbcbcfacbe3e0871750d19b Mon Sep 17 00:00:00 2001
From: Maxime Beauchemin <maximebeauchemin@gmail.com>
Date: Mon, 24 Apr 2017 16:31:16 -0700
Subject: [PATCH] [druid] fixing the having clause in the explore view (#2648)

* [druid] fixing the having clause in the explore view

* Backend

* Lint

* Fixing tests
---
 .../components/DisplayQueryButton.jsx         |  3 +-
 .../explorev2/components/controls/Filter.jsx  | 38 ++++++++++++-------
 .../explorev2/components/Filter_spec.jsx      | 12 ++++--
 superset/connectors/druid/models.py           |  2 +-
 superset/views/base.py                        |  2 +-
 5 files changed, 37 insertions(+), 20 deletions(-)

diff --git a/superset/assets/javascripts/explorev2/components/DisplayQueryButton.jsx b/superset/assets/javascripts/explorev2/components/DisplayQueryButton.jsx
index 1912016b4ae77..3ac6a2eb36cdc 100644
--- a/superset/assets/javascripts/explorev2/components/DisplayQueryButton.jsx
+++ b/superset/assets/javascripts/explorev2/components/DisplayQueryButton.jsx
@@ -46,11 +46,12 @@ export default class DisplayQueryButton extends React.PureComponent {
           language: data.language,
           query: data.query,
           isLoading: false,
+          error: null,
         });
       },
       error: (data) => {
         this.setState({
-          error: data.error,
+          error: data.responseJSON ? data.responseJSON.error : 'Error...',
           isLoading: false,
         });
       },
diff --git a/superset/assets/javascripts/explorev2/components/controls/Filter.jsx b/superset/assets/javascripts/explorev2/components/controls/Filter.jsx
index a7dc1f0841514..cf29a2b084537 100644
--- a/superset/assets/javascripts/explorev2/components/controls/Filter.jsx
+++ b/superset/assets/javascripts/explorev2/components/controls/Filter.jsx
@@ -8,12 +8,12 @@ const $ = window.$ = require('jquery');
 const operatorsArr = [
   { val: 'in', type: 'array', useSelect: true, multi: true },
   { val: 'not in', type: 'array', useSelect: true, multi: true },
-  { val: '==', type: 'string', useSelect: true, multi: false },
-  { val: '!=', type: 'string', useSelect: true, multi: false },
-  { val: '>=', type: 'string' },
-  { val: '<=', type: 'string' },
-  { val: '>', type: 'string' },
-  { val: '<', type: 'string' },
+  { val: '==', type: 'string', useSelect: true, multi: false, havingOnly: true },
+  { val: '!=', type: 'string', useSelect: true, multi: false, havingOnly: true },
+  { val: '>=', type: 'string', havingOnly: true },
+  { val: '<=', type: 'string', havingOnly: true },
+  { val: '>', type: 'string', havingOnly: true },
+  { val: '<', type: 'string', havingOnly: true },
   { val: 'regex', type: 'string', datasourceTypes: ['druid'] },
   { val: 'LIKE', type: 'string', datasourceTypes: ['table'] },
 ];
@@ -27,12 +27,14 @@ const propTypes = {
   removeFilter: PropTypes.func,
   filter: PropTypes.object.isRequired,
   datasource: PropTypes.object,
+  having: PropTypes.bool,
 };
 
 const defaultProps = {
   changeFilter: () => {},
   removeFilter: () => {},
   datasource: null,
+  having: false,
 };
 
 export default class Filter extends React.Component {
@@ -47,7 +49,7 @@ export default class Filter extends React.Component {
   }
   fetchFilterValues(col) {
     const datasource = this.props.datasource;
-    if (col && this.props.datasource && this.props.datasource.filter_select) {
+    if (col && this.props.datasource && this.props.datasource.filter_select && !this.props.having) {
       this.setState({ valuesLoading: true });
       $.ajax({
         type: 'GET',
@@ -90,7 +92,7 @@ export default class Filter extends React.Component {
   }
   renderFilterFormControl(filter) {
     const operator = operators[filter.op];
-    if (operator.useSelect) {
+    if (operator.useSelect && !this.props.having) {
       return (
         <SelectControl
           multi={operator.multi}
@@ -117,18 +119,28 @@ export default class Filter extends React.Component {
     const datasource = this.props.datasource;
     const filter = this.props.filter;
     const opsChoices = operatorsArr
-    .filter(o => !o.datasourceTypes || o.datasourceTypes.indexOf(datasource.type) >= 0)
+    .filter((o) => {
+      if (this.props.having) {
+        return !!o.havingOnly;
+      }
+      return (!o.datasourceTypes || o.datasourceTypes.indexOf(datasource.type) >= 0);
+    })
     .map(o => ({ value: o.val, label: o.val }));
-    const colChoices = datasource ?
-      datasource.filterable_cols.map(c => ({ value: c[0], label: c[1] })) :
-      null;
+    let colChoices;
+    if (datasource) {
+      if (this.props.having) {
+        colChoices = datasource.metrics_combo.map(c => ({ value: c[0], label: c[1] }));
+      } else {
+        colChoices = datasource.filterable_cols.map(c => ({ value: c[0], label: c[1] }));
+      }
+    }
     return (
       <div>
         <Row className="space-1">
           <Col md={12}>
             <Select
               id="select-col"
-              placeholder="Select column"
+              placeholder={this.props.having ? 'Select metric' : 'Select column'}
               clearable={false}
               options={colChoices}
               value={filter.col}
diff --git a/superset/assets/spec/javascripts/explorev2/components/Filter_spec.jsx b/superset/assets/spec/javascripts/explorev2/components/Filter_spec.jsx
index e265d6f3f2cb5..070631e1cac14 100644
--- a/superset/assets/spec/javascripts/explorev2/components/Filter_spec.jsx
+++ b/superset/assets/spec/javascripts/explorev2/components/Filter_spec.jsx
@@ -22,6 +22,10 @@ const defaultProps = {
     type: 'qtable',
     filter_select: false,
     filterable_cols: ['col1', 'col2'],
+    metrics_combo: [
+      ['m1', 'v1'],
+      ['m2', 'v2'],
+    ],
   },
 };
 
@@ -46,7 +50,7 @@ describe('Filter', () => {
   });
 
   it('renders five op choices for table datasource', () => {
-    const props = defaultProps;
+    const props = Object.assign({}, defaultProps);
     props.datasource = {
       id: 1,
       type: 'druid',
@@ -58,10 +62,10 @@ describe('Filter', () => {
   });
 
   it('renders six op choices for having filter', () => {
-    const props = defaultProps;
+    const props = Object.assign({}, defaultProps);
     props.having = true;
     const havingWrapper = shallow(<Filter {...props} />);
-    expect(havingWrapper.find('#select-op').prop('options')).to.have.lengthOf(9);
+    expect(havingWrapper.find('#select-op').prop('options')).to.have.lengthOf(6);
   });
 
   it('calls changeFilter when select is changed', () => {
@@ -75,7 +79,7 @@ describe('Filter', () => {
   });
 
   it('renders input for regex filters', () => {
-    const props = defaultProps;
+    const props = Object.assign({}, defaultProps);
     props.filter = {
       col: null,
       op: 'regex',
diff --git a/superset/connectors/druid/models.py b/superset/connectors/druid/models.py
index 9af9b08eabcbc..b063b67ee8323 100644
--- a/superset/connectors/druid/models.py
+++ b/superset/connectors/druid/models.py
@@ -835,7 +835,7 @@ def recursive_get_fields(_conf):
             qry['having'] = having_filters
 
         orig_filters = filters
-        if len(groupby) == 0:
+        if len(groupby) == 0 and not having_filters:
             del qry['dimensions']
             client.timeseries(**qry)
         if not having_filters and len(groupby) == 1:
diff --git a/superset/views/base.py b/superset/views/base.py
index 132ba82b8cb4f..6593eec7e62a5 100644
--- a/superset/views/base.py
+++ b/superset/views/base.py
@@ -29,7 +29,7 @@ def get_error_msg():
 
 
 def json_error_response(msg, status=None, stacktrace=None):
-    data = {'error': msg}
+    data = {'error': str(msg)}
     if stacktrace:
         data['stacktrace'] = stacktrace
     status = status if status else 500