Repository: getredash/redash
Pull request: Support multi-select in parameters
## What type of PR is this? (check all applicable)
<!-- Please leave only what's applicable -->
- [x] Feature

## Description
This one is to add an option to allow multiple values in Dropdown parameters (see preview below :)).

- [x] Make 'quote each value' an option (and let the user customize the separator?)
- [x] Apply this to Query based Dropdown parameters
- [x] Styling updates (select `maxWidth`, etc)
- [x] Add tests 
- [x] Refresh Schedule

## Related Tickets & Documents
Closes #3053

## Mobile & Desktop Screenshots/Recordings (if there are UI changes)
**Dropdown**
![multi-select-enum](https://user-images.githubusercontent.com/3356951/61306018-3a975100-a7c2-11e9-84c6-2c1dc0ff739e.gif)

**Query Based Dropdown**
![multi-select-query](https://user-images.githubusercontent.com/3356951/61306411-ec368200-a7c2-11e9-9513-bd4ebe54d881.gif)


Problem statement:
Support for multi select in parameters
When using a Dropdown List type of parameter, the user should be able to define the parameter to take multiple values.

This will require some additional configuration of how to serialize the values. We can probably do away with adding a checkbox for "quote each value".

Hints from issue discussion:

Commit message: Allow multi-select for Query dropdown parameters

Patch to review:
diff --git a/client/app/components/EditParameterSettingsDialog.jsx b/client/app/components/EditParameterSettingsDialog.jsx
index 1805f9ed6c..0566302bdf 100644
--- a/client/app/components/EditParameterSettingsDialog.jsx
+++ b/client/app/components/EditParameterSettingsDialog.jsx
@@ -201,6 +201,16 @@ function EditParameterSettingsDialog(props) {
             />
           </Form.Item>
         )}
+        {(param.type === 'enum' || param.type === 'query') && (
+          <Form.Item label=" " colon={false} {...formItemProps}>
+            <Checkbox
+              defaultChecked={param.allowMultipleValues}
+              onChange={e => setParam({ ...param, allowMultipleValues: e.target.checked })}
+            >
+            Allow multiple values
+            </Checkbox>
+          </Form.Item>
+        )}
       </Form>
     </Modal>
   );
diff --git a/client/app/components/ParameterValueInput.jsx b/client/app/components/ParameterValueInput.jsx
index e4f0475ef5..f05e4eb563 100644
--- a/client/app/components/ParameterValueInput.jsx
+++ b/client/app/components/ParameterValueInput.jsx
@@ -23,6 +23,7 @@ export class ParameterValueInput extends React.Component {
     enumOptions: PropTypes.string,
     queryId: PropTypes.number,
     parameter: PropTypes.any, // eslint-disable-line react/forbid-prop-types
+    allowMultipleValues: PropTypes.bool,
     applyButton: PropTypes.bool,
     onSelect: PropTypes.func,
     className: PropTypes.string,
@@ -34,6 +35,7 @@ export class ParameterValueInput extends React.Component {
     enumOptions: '',
     queryId: null,
     parameter: null,
+    allowMultipleValues: false,
     applyButton: false,
     onSelect: () => {},
     className: '',
@@ -128,16 +130,21 @@ export class ParameterValueInput extends React.Component {
   }
 
   renderEnumInput() {
-    const { value, onSelect, enumOptions } = this.props;
+    const { value, onSelect, enumOptions, allowMultipleValues } = this.props;
     const enumOptionsArray = enumOptions.split('\n').filter(v => v !== '');
     return (
       <Select
         className={this.props.className}
+        mode={allowMultipleValues ? 'multiple' : 'default'}
+        optionFilterProp="children"
         disabled={enumOptionsArray.length === 0}
         defaultValue={value}
         onChange={onSelect}
         dropdownMatchSelectWidth={false}
         dropdownClassName="ant-dropdown-in-bootstrap-modal"
+        maxTagCount={3}
+        maxTagTextLength={10}
+        maxTagPlaceholder={num => `+${num.length} more`}
       >
         {enumOptionsArray.map(option => (<Option key={option} value={option}>{ option }</Option>))}
       </Select>
@@ -145,14 +152,19 @@ export class ParameterValueInput extends React.Component {
   }
 
   renderQueryBasedInput() {
-    const { value, onSelect, queryId, parameter } = this.props;
+    const { value, onSelect, queryId, allowMultipleValues, parameter } = this.props;
     return (
       <QueryBasedParameterInput
         className={this.props.className}
+        mode={allowMultipleValues ? 'multiple' : 'default'}
+        optionFilterProp="children"
         parameter={parameter}
         value={value}
         queryId={queryId}
         onSelect={onSelect}
+        maxTagCount={3}
+        maxTagTextLength={10}
+        maxTagPlaceholder={num => `+${num.length} more`}
       />
     );
   }
@@ -243,6 +255,7 @@ export default function init(ngModule) {
         parameter="$ctrl.param"
         enum-options="$ctrl.param.enumOptions"
         query-id="$ctrl.param.queryId"
+        allow-multiple-values="$ctrl.param.allowMultipleValues"
         on-select="$ctrl.setValue"
         apply-button="$ctrl.applyButton"
       ></parameter-value-input-impl>
diff --git a/client/app/components/QueryBasedParameterInput.jsx b/client/app/components/QueryBasedParameterInput.jsx
index 7273a34619..506c98ad36 100644
--- a/client/app/components/QueryBasedParameterInput.jsx
+++ b/client/app/components/QueryBasedParameterInput.jsx
@@ -1,4 +1,4 @@
-import { find, isFunction, toString } from 'lodash';
+import { find, isFunction, isArray, toString, map, intersection } from 'lodash';
 import React from 'react';
 import PropTypes from 'prop-types';
 import { react2angular } from 'react2angular';
@@ -10,6 +10,7 @@ export class QueryBasedParameterInput extends React.Component {
   static propTypes = {
     parameter: PropTypes.any, // eslint-disable-line react/forbid-prop-types
     value: PropTypes.any, // eslint-disable-line react/forbid-prop-types
+    mode: PropTypes.string,
     queryId: PropTypes.number,
     onSelect: PropTypes.func,
     className: PropTypes.string,
@@ -17,6 +18,7 @@ export class QueryBasedParameterInput extends React.Component {
 
   static defaultProps = {
     value: null,
+    mode: 'default',
     parameter: null,
     queryId: null,
     onSelect: () => {},
@@ -50,16 +52,24 @@ export class QueryBasedParameterInput extends React.Component {
       if (this.props.queryId === queryId) {
         this.setState({ options, loading: false });
 
-        const found = find(options, option => option.value === this.props.value) !== undefined;
-        if (!found && isFunction(this.props.onSelect)) {
-          this.props.onSelect(options[0].value);
+        if (this.props.mode === 'multiple' && isArray(this.props.value)) {
+          const optionValues = map(options, option => option.value);
+          const validValues = intersection(this.props.value, optionValues);
+          if (validValues.length !== this.props.value.length) {
+            this.props.onSelect(validValues);
+          }
+        } else {
+          const found = find(options, option => option.value === this.props.value) !== undefined;
+          if (!found && isFunction(this.props.onSelect)) {
+            this.props.onSelect(options[0].value);
+          }
         }
       }
     }
   }
 
   render() {
-    const { className, value, onSelect } = this.props;
+    const { className, value, mode, onSelect, ...otherProps } = this.props;
     const { loading, options } = this.state;
     return (
       <span>
@@ -67,10 +77,12 @@ export class QueryBasedParameterInput extends React.Component {
           className={className}
           disabled={loading || (options.length === 0)}
           loading={loading}
-          value={toString(value)}
+          mode={mode}
+          value={isArray(value) ? value : toString(value)}
           onChange={onSelect}
           dropdownMatchSelectWidth={false}
           dropdownClassName="ant-dropdown-in-bootstrap-modal"
+          {...otherProps}
         >
           {options.map(option => (<Option value={option.value} key={option.value}>{option.name}</Option>))}
         </Select>
diff --git a/client/app/services/query.js b/client/app/services/query.js
index 8c00829ca1..4ff1fef3d0 100644
--- a/client/app/services/query.js
+++ b/client/app/services/query.js
@@ -55,6 +55,7 @@ export class Parameter {
     this.name = parameter.name;
     this.type = parameter.type;
     this.useCurrentDateTime = parameter.useCurrentDateTime;
+    this.allowMultipleValues = parameter.allowMultipleValues;
     this.global = parameter.global; // backward compatibility in Widget service
     this.enumOptions = parameter.enumOptions;
     this.queryId = parameter.queryId;
@@ -174,6 +175,9 @@ export class Parameter {
         [`${prefix}${this.name}.end`]: this.value.end,
       };
     }
+    if (this.allowMultipleValues && isArray(this.value)) {
+      return { [`${prefix}${this.name}`]: JSON.stringify(this.value) };
+    }
     return {
       [`${prefix}${this.name}`]: this.value,
     };
@@ -190,7 +194,15 @@ export class Parameter {
     } else {
       const key = `${prefix}${this.name}`;
       if (has(query, key)) {
-        this.setValue(query[key]);
+        if (this.allowMultipleValues) {
+          try {
+            this.setValue(JSON.parse(query[key]));
+          } catch (e) {
+            this.setValue(query[key]);
+          }
+        } else {
+          this.setValue(query[key]);
+        }
       }
     }
   }
diff --git a/redash/models/parameterized_query.py b/redash/models/parameterized_query.py
index 333749627c..019090f75b 100644
--- a/redash/models/parameterized_query.py
+++ b/redash/models/parameterized_query.py
@@ -36,6 +36,13 @@ def dropdown_values(query_id):
     return map(pluck, data["rows"])
 
 
+def resolve_parameter_values(parameters):
+    resolved_parameters = {}
+    for (key, value) in parameters.iteritems():
+        resolved_parameters[key] = ','.join(value) if isinstance(value, list) else value
+    return resolved_parameters
+
+
 def _collect_key_names(nodes):
     keys = []
     for node in nodes._parse_tree:
@@ -92,6 +99,12 @@ def _is_date_range(obj):
         return False
 
 
+def _is_dropdown_value_valid(value, dropdown_options):
+    if isinstance(value, list):
+        return all(unicode(v) in dropdown_options for v in value)
+    return unicode(value) in dropdown_options
+
+
 class ParameterizedQuery(object):
     def __init__(self, template, schema=None):
         self.schema = schema or []
@@ -105,7 +118,7 @@ def apply(self, parameters):
             raise InvalidParameterError(invalid_parameter_names)
         else:
             self.parameters.update(parameters)
-            self.query = mustache_render(self.template, self.parameters)
+            self.query = mustache_render(self.template, resolve_parameter_values(parameters))
 
         return self
 
@@ -121,8 +134,8 @@ def _valid(self, name, value):
         validators = {
             "text": lambda value: isinstance(value, basestring),
             "number": _is_number,
-            "enum": lambda value: value in definition["enumOptions"],
-            "query": lambda value: unicode(value) in [v["value"] for v in dropdown_values(definition["queryId"])],
+            "enum": lambda value: _is_dropdown_value_valid(value, definition["enumOptions"]),
+            "query": lambda value: _is_dropdown_value_valid(value, [v["value"] for v in dropdown_values(definition["queryId"])]),
             "date": _is_date,
             "datetime-local": _is_date,
             "datetime-with-seconds": _is_date,


Code review comments:
diff_hunk: @@ -92,6 +99,12 @@ def _is_date_range(obj):
         return False
 
 
+def _is_dropdown_value_valid(value, dropdown_options):
path: redash/models/parameterized_query.py
text: @user1:
Consider renaming this function to explain what it does. (i.e. less general terms such as `is_valid` and more `is_date` - for example, `_is_value_within_options`

@author:
Thanks, this makes a lot of sense :)

diff_hunk: @@ -36,6 +36,13 @@ def dropdown_values(query_id):
     return map(pluck, data["rows"])
 
 
+def resolve_parameter_values(parameters):
path: redash/models/parameterized_query.py
text: @user1:
Again, I feel that this function name is too generic for what it does. There's no "resolving" here, but rather flattening of multiple values.