From 95509f20008684470a2c02ca1bb1295f395d4f29 Mon Sep 17 00:00:00 2001
From: Maxime Beauchemin <maximebeauchemin@gmail.com>
Date: Tue, 25 Jul 2017 20:50:41 -0700
Subject: [PATCH] [bugfix] only filterable columns should show up in FilterBox
 list (#3105)

* [bugfix] only filterable columns should show up in FilterBox list

* Touchups
---
 .../explore/components/ControlPanelsContainer.jsx    | 12 ++++++++++--
 .../explore/components/controls/SelectControl.jsx    |  3 ++-
 .../assets/javascripts/explore/stores/controls.jsx   |  2 +-
 .../assets/javascripts/explore/stores/visTypes.js    | 11 ++++++++---
 superset/connectors/base/models.py                   |  4 +++-
 5 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/superset/assets/javascripts/explore/components/ControlPanelsContainer.jsx b/superset/assets/javascripts/explore/components/ControlPanelsContainer.jsx
index 9227390d12..ac82bb490c 100644
--- a/superset/assets/javascripts/explore/components/ControlPanelsContainer.jsx
+++ b/superset/assets/javascripts/explore/components/ControlPanelsContainer.jsx
@@ -4,7 +4,7 @@ import PropTypes from 'prop-types';
 import { bindActionCreators } from 'redux';
 import { connect } from 'react-redux';
 import { Alert } from 'react-bootstrap';
-import { sectionsToRender } from '../stores/visTypes';
+import { sectionsToRender, visTypes } from '../stores/visTypes';
 import ControlPanelSection from './ControlPanelSection';
 import ControlRow from './ControlRow';
 import Control from './Control';
@@ -28,7 +28,15 @@ class ControlPanelsContainer extends React.Component {
     this.getControlData = this.getControlData.bind(this);
   }
   getControlData(controlName) {
-    const mapF = controls[controlName].mapStateToProps;
+    // Identifying mapStateToProps function to apply (logic can't be in store)
+    let mapF = controls[controlName].mapStateToProps;
+
+    // Looking to find mapStateToProps override for this viz type
+    const controlOverrides = visTypes[this.props.controls.viz_type.value].controlOverrides || {};
+    if (controlOverrides[controlName] && controlOverrides[controlName].mapStateToProps) {
+      mapF = controlOverrides[controlName].mapStateToProps;
+    }
+    // Applying mapStateToProps if needed
     if (mapF) {
       return Object.assign({}, this.props.controls[controlName], mapF(this.props.exploreState));
     }
diff --git a/superset/assets/javascripts/explore/components/controls/SelectControl.jsx b/superset/assets/javascripts/explore/components/controls/SelectControl.jsx
index 6998c071b0..312fced55b 100644
--- a/superset/assets/javascripts/explore/components/controls/SelectControl.jsx
+++ b/superset/assets/javascripts/explore/components/controls/SelectControl.jsx
@@ -43,7 +43,8 @@ export default class SelectControl extends React.PureComponent {
     this.onChange = this.onChange.bind(this);
   }
   componentWillReceiveProps(nextProps) {
-    if (nextProps.choices !== this.props.choices) {
+    if (nextProps.choices !== this.props.choices ||
+        nextProps.options !== this.props.options) {
       const options = this.getOptions(nextProps);
       this.setState({ options });
     }
diff --git a/superset/assets/javascripts/explore/stores/controls.jsx b/superset/assets/javascripts/explore/stores/controls.jsx
index 13306932ce..ecfbfe9e64 100644
--- a/superset/assets/javascripts/explore/stores/controls.jsx
+++ b/superset/assets/javascripts/explore/stores/controls.jsx
@@ -326,7 +326,7 @@ export const controls = {
     valueRenderer: c => <ColumnOption column={c} />,
     valueKey: 'column_name',
     mapStateToProps: state => ({
-      options: (state.datasource) ? state.datasource.columns : [],
+      options: (state.datasource) ? state.datasource.columns.filter(c => c.groupby) : [],
     }),
   },
 
diff --git a/superset/assets/javascripts/explore/stores/visTypes.js b/superset/assets/javascripts/explore/stores/visTypes.js
index bdebf07642..7d3f26284b 100644
--- a/superset/assets/javascripts/explore/stores/visTypes.js
+++ b/superset/assets/javascripts/explore/stores/visTypes.js
@@ -75,7 +75,7 @@ export const sections = {
   ],
 };
 
-const visTypes = {
+export const visTypes = {
   dist_bar: {
     label: 'Distribution - Bar Chart',
     controlPanelSections: [
@@ -742,8 +742,13 @@ const visTypes = {
     controlOverrides: {
       groupby: {
         label: 'Filter controls',
-        description: 'The controls you want to filter on',
-        default: [],
+        description: (
+          'The controls you want to filter on. Note that only columns ' +
+          'checked as "filterable" will show up on this list.'
+        ),
+        mapStateToProps: state => ({
+          options: (state.datasource) ? state.datasource.columns.filter(c => c.filterable) : [],
+        }),
       },
     },
   },
diff --git a/superset/connectors/base/models.py b/superset/connectors/base/models.py
index e203ef4401..b32bb928a3 100644
--- a/superset/connectors/base/models.py
+++ b/superset/connectors/base/models.py
@@ -222,7 +222,9 @@ class BaseColumn(AuditMixinNullable, ImportMixin):
 
     @property
     def data(self):
-        attrs = ('column_name', 'verbose_name', 'description', 'expression')
+        attrs = (
+            'column_name', 'verbose_name', 'description', 'expression',
+            'filterable', 'groupby')
         return {s: getattr(self, s) for s in attrs}