From db47c876ee69bd034b7e131daf0a7fd5672d2221 Mon Sep 17 00:00:00 2001
From: guerler <aysam.guerler@gmail.com>
Date: Wed, 14 Mar 2018 15:38:28 -0400
Subject: [PATCH 01/25] Remove prepopulation of column selector

---
 lib/galaxy/tools/parameters/basic.py | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py
index 7265d48a344..f8651c45694 100644
--- a/lib/galaxy/tools/parameters/basic.py
+++ b/lib/galaxy/tools/parameters/basic.py
@@ -43,8 +43,6 @@ workflow_building_modes = Bunch(DISABLED=False, ENABLED=True, USE_HISTORY=1)
 
 WORKFLOW_PARAMETER_REGULAR_EXPRESSION = re.compile('''\$\{.+?\}''')
 
-MAX_DEFAULT_COLUMNS = 999
-
 
 def contains_workflow_parameter(value, search=False):
     if not isinstance(value, string_types):
@@ -1128,11 +1126,7 @@ class ColumnListParameter(SelectToolParameter):
                 return []
             # Build up possible columns for this dataset
             this_column_list = []
-            # Valid column-based datasets contain at least 1 column if that column has not been
-            # specified we prepopulate the selector assuming that the datasets is not ready yet.
-            if dataset.metadata.columns is None:
-                this_column_list = [str(i) for i in range(1, MAX_DEFAULT_COLUMNS + 1)]
-            elif self.numerical:
+            if self.numerical:
                 # If numerical was requested, filter columns based on metadata
                 for i, col in enumerate(dataset.metadata.column_types):
                     if col == 'int' or col == 'float':

From fc7beca6d1d379ddfc1ea4769edfc8c22d74fb10 Mon Sep 17 00:00:00 2001
From: guerler <aysam.guerler@gmail.com>
Date: Wed, 14 Mar 2018 23:54:30 -0400
Subject: [PATCH 02/25] Create input element combining text and select options

---
 .../scripts/mvc/form/form-parameters.js       | 17 +-------
 client/galaxy/scripts/mvc/ui/ui-misc.js       | 43 +++++++++++++++++++
 2 files changed, 45 insertions(+), 15 deletions(-)

diff --git a/client/galaxy/scripts/mvc/form/form-parameters.js b/client/galaxy/scripts/mvc/form/form-parameters.js
index bda898a9322..5c7a9abd89e 100644
--- a/client/galaxy/scripts/mvc/form/form-parameters.js
+++ b/client/galaxy/scripts/mvc/form/form-parameters.js
@@ -84,23 +84,10 @@ export default Backbone.Model.extend({
         }
 
         // identify display type
-        var SelectClass = Ui.Select;
-        switch (input_def.display) {
-            case "checkboxes":
-                SelectClass = Ui.Checkbox;
-                break;
-            case "radio":
-                SelectClass = Ui.Radio;
-                break;
-            case "radiobutton":
-                SelectClass = Ui.RadioButton;
-                break;
-        }
-
-        // create select field
-        return new SelectClass.View({
+        return new Ui.TextSelect({
             id: `field-${input_def.id}`,
             data: data,
+            display: input_def.display,
             error_text: input_def.error_text || "No options available",
             readonly: input_def.readonly,
             multiple: input_def.multiple,
diff --git a/client/galaxy/scripts/mvc/ui/ui-misc.js b/client/galaxy/scripts/mvc/ui/ui-misc.js
index 7ad33d347c3..f8188e7fce6 100644
--- a/client/galaxy/scripts/mvc/ui/ui-misc.js
+++ b/client/galaxy/scripts/mvc/ui/ui-misc.js
@@ -181,6 +181,48 @@ export var Hidden = Backbone.View.extend({
     }
 });
 
+/** Creates switcher */
+export var TextSelect = Backbone.View.extend({
+    initialize: function(options) {
+        this.text = new Input(options);
+        var SelectClass = Select;
+        switch (options.display) {
+            case "checkboxes":
+                SelectClass = Checkbox;
+                break;
+            case "radio":
+                SelectClass = Radio;
+                break;
+            case "radiobutton":
+                SelectClass = RadioButton;
+                break;
+        }
+        this.select = new SelectClass.View(options);
+        this.model = this.select.model;
+        this.setElement($("<div/>").append(this.select.$el)
+                                   .append(this.text.$el));
+        this.listenTo(this.model, "change", this.render, this);
+        this.render();
+    },
+    value: function(new_val) {
+        var element = this._check() ? this.select : this.text;
+        return element.value(new_val)
+    },
+    update: function(options) {
+        this.select.update(options);
+    },
+    render: function() {
+        var flag = this._check();
+        this.select.$el[flag ? 'show' : 'hide']();
+        this.text.$el[flag ? 'hide' : 'show']();
+        return this;
+    },
+    _check: function() {
+        var data = this.model.get('data');
+        return $.isArray(data) && data.length > 0;
+    }
+});
+
 /** Creates a upload element input field */
 export var Upload = Backbone.View.extend({
     initialize: function(options) {
@@ -263,6 +305,7 @@ export default {
     Checkbox: Options.Checkbox,
     Radio: Options.Radio,
     Select: Select,
+    TextSelect: TextSelect,
     Hidden: Hidden,
     Slider: Slider,
     Drilldown: Drilldown

From 2e8c9489bfd149d12b9498ea89c5633a07509d48 Mon Sep 17 00:00:00 2001
From: guerler <aysam.guerler@gmail.com>
Date: Thu, 15 Mar 2018 00:19:05 -0400
Subject: [PATCH 03/25] Fix value handling, remove redundant value check

---
 client/galaxy/scripts/mvc/ui/ui-misc.js | 10 +++++-----
 lib/galaxy/tools/parameters/basic.py    |  6 +-----
 2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/client/galaxy/scripts/mvc/ui/ui-misc.js b/client/galaxy/scripts/mvc/ui/ui-misc.js
index f8188e7fce6..6f371d2c8c1 100644
--- a/client/galaxy/scripts/mvc/ui/ui-misc.js
+++ b/client/galaxy/scripts/mvc/ui/ui-misc.js
@@ -181,7 +181,7 @@ export var Hidden = Backbone.View.extend({
     }
 });
 
-/** Creates switcher */
+/** Creates an input element which switches between select and text field */
 export var TextSelect = Backbone.View.extend({
     initialize: function(options) {
         this.text = new Input(options);
@@ -206,15 +206,15 @@ export var TextSelect = Backbone.View.extend({
     },
     value: function(new_val) {
         var element = this._check() ? this.select : this.text;
-        return element.value(new_val)
+        return element.value(new_val);
     },
     update: function(options) {
         this.select.update(options);
     },
     render: function() {
-        var flag = this._check();
-        this.select.$el[flag ? 'show' : 'hide']();
-        this.text.$el[flag ? 'hide' : 'show']();
+        var check = this._check();
+        this.select.$el[check ? 'show' : 'hide']();
+        this.text.$el[check ? 'hide' : 'show']();
         return this;
     },
     _check: function() {
diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py
index f8651c45694..ccbcb03ee41 100644
--- a/lib/galaxy/tools/parameters/basic.py
+++ b/lib/galaxy/tools/parameters/basic.py
@@ -866,8 +866,6 @@ class SelectToolParameter(ToolParameter):
             return value
         if (not legal_values or value is None) and self.optional:
             return None
-        if not legal_values:
-            raise ValueError("Parameter %s requires a value, but has no legal values defined." % self.name)
         if isinstance(value, list):
             if not self.multiple:
                 raise ValueError("Multiple values provided but parameter %s is not expecting multiple values." % self.name)
@@ -1122,7 +1120,7 @@ class ColumnListParameter(SelectToolParameter):
             if isinstance(dataset, trans.app.model.HistoryDatasetCollectionAssociation):
                 dataset = dataset.to_hda_representative()
             # Columns can only be identified if metadata is available
-            if not hasattr(dataset, 'metadata') or not hasattr(dataset.metadata, 'columns'):
+            if not hasattr(dataset, 'metadata') or not hasattr(dataset.metadata, 'columns') or not dataset.metadata.columns:
                 return []
             # Build up possible columns for this dataset
             this_column_list = []
@@ -1329,8 +1327,6 @@ class DrillDownSelectToolParameter(SelectToolParameter):
         if len(value) > 1 and not self.multiple:
             raise ValueError("Multiple values provided but parameter %s is not expecting multiple values." % self.name)
         rval = []
-        if not legal_values:
-            raise ValueError("Parameter %s requires a value, but has no legal values defined." % self.name)
         for val in value:
             if val not in legal_values:
                 raise ValueError("An invalid option was selected for %s, %r, please verify" % (self.name, val))

From 9fbdcd1d5ee84887c28eed727fec77d402ef95fa Mon Sep 17 00:00:00 2001
From: guerler <aysam.guerler@gmail.com>
Date: Thu, 15 Mar 2018 00:29:27 -0400
Subject: [PATCH 04/25] Relax validation conditions

---
 lib/galaxy/tools/parameters/basic.py | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py
index ccbcb03ee41..2e2a5a9a37c 100644
--- a/lib/galaxy/tools/parameters/basic.py
+++ b/lib/galaxy/tools/parameters/basic.py
@@ -843,13 +843,12 @@ class SelectToolParameter(ToolParameter):
             return self.legal_values
 
     def from_json(self, value, trans, other_values={}):
+        if not value and not self.optional:
+            raise ValueError("An invalid option was selected for %s, please verify." % (self.name))
+        if not value:
+            return None
         legal_values = self.get_legal_values(trans, other_values)
-        workflow_building_mode = trans.workflow_building_mode
-        for context_value in other_values.values():
-            if is_runtime_value(context_value):
-                workflow_building_mode = workflow_building_modes.ENABLED
-                break
-        if len(list(legal_values)) == 0 and workflow_building_mode:
+        if len(list(legal_values)) == 0:
             if self.multiple:
                 # While it is generally allowed that a select value can be '',
                 # we do not allow this to be the case in a dynamically
@@ -864,8 +863,6 @@ class SelectToolParameter(ToolParameter):
                         # use \r\n to separate lines.
                         value = value.split()
             return value
-        if (not legal_values or value is None) and self.optional:
-            return None
         if isinstance(value, list):
             if not self.multiple:
                 raise ValueError("Multiple values provided but parameter %s is not expecting multiple values." % self.name)
@@ -1311,17 +1308,17 @@ class DrillDownSelectToolParameter(SelectToolParameter):
 
     def from_json(self, value, trans, other_values={}):
         legal_values = self.get_legal_values(trans, other_values)
-        if len(list(legal_values)) == 0 and trans.workflow_building_mode:
+        if not value and not self.optional:
+            raise ValueError("An invalid option was selected for %s, please verify." % (self.name))
+        if not value:
+            return None
+        if len(list(legal_values)) == 0:
             if self.multiple:
                 if value == '':  # No option selected
                     value = None
                 else:
                     value = value.split("\n")
             return value
-        if not value and not self.optional:
-            raise ValueError("An invalid option was selected for %s, please verify." % (self.name))
-        if not value:
-            return None
         if not isinstance(value, list):
             value = [value]
         if len(value) > 1 and not self.multiple:

From ff874dc4e4f4d21bb133b2730058393c7ae7a66a Mon Sep 17 00:00:00 2001
From: guerler <aysam.guerler@gmail.com>
Date: Thu, 15 Mar 2018 09:12:41 -0400
Subject: [PATCH 05/25] Properly switch between states, transfer current value

---
 client/galaxy/scripts/mvc/ui/ui-misc.js | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/client/galaxy/scripts/mvc/ui/ui-misc.js b/client/galaxy/scripts/mvc/ui/ui-misc.js
index 6f371d2c8c1..d08e7a932f4 100644
--- a/client/galaxy/scripts/mvc/ui/ui-misc.js
+++ b/client/galaxy/scripts/mvc/ui/ui-misc.js
@@ -198,28 +198,21 @@ export var TextSelect = Backbone.View.extend({
                 break;
         }
         this.select = new SelectClass.View(options);
-        this.model = this.select.model;
         this.setElement($("<div/>").append(this.select.$el)
                                    .append(this.text.$el));
-        this.listenTo(this.model, "change", this.render, this);
-        this.render();
+        this.update(options.data);
     },
     value: function(new_val) {
-        var element = this._check() ? this.select : this.text;
+        var element = this.textmode ? this.text : this.select;
         return element.value(new_val);
     },
     update: function(options) {
+        var v = this.value();
+        this.textmode = !$.isArray(options) || options.length === 0;
+        this.text.$el[this.textmode ? "show" : "hide"]();
+        this.select.$el[this.textmode ? "hide" : "show"]();
         this.select.update(options);
-    },
-    render: function() {
-        var check = this._check();
-        this.select.$el[check ? 'show' : 'hide']();
-        this.text.$el[check ? 'hide' : 'show']();
-        return this;
-    },
-    _check: function() {
-        var data = this.model.get('data');
-        return $.isArray(data) && data.length > 0;
+        this.value(v);
     }
 });
 

From 9fff4f1615aa7ce02561d681ae20397b22b17f02 Mon Sep 17 00:00:00 2001
From: guerler <aysam.guerler@gmail.com>
Date: Thu, 15 Mar 2018 09:40:55 -0400
Subject: [PATCH 06/25] Remove redundant initial value checks, value can only
 be empty

---
 lib/galaxy/tools/parameters/basic.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py
index 2e2a5a9a37c..28180a3b795 100644
--- a/lib/galaxy/tools/parameters/basic.py
+++ b/lib/galaxy/tools/parameters/basic.py
@@ -907,7 +907,7 @@ class SelectToolParameter(ToolParameter):
 
     def get_initial_value(self, trans, other_values):
         options = list(self.get_options(trans, other_values))
-        if len(options) == 0 and trans.workflow_building_mode:
+        if len(options) == 0:
             return None
         value = [optval for _, optval, selected in options if selected]
         if len(value) == 0:
@@ -1378,7 +1378,7 @@ class DrillDownSelectToolParameter(SelectToolParameter):
                 recurse_options(initial_values, option['options'])
         # More working around dynamic options for workflow
         options = self.get_options(trans=trans, other_values=other_values)
-        if len(list(options)) == 0 and trans.workflow_building_mode:
+        if len(list(options)) == 0:
             return None
         initial_values = []
         recurse_options(initial_values, options)

From 1db1c0c77122edd183ab6f0bf5da24c94c83b476 Mon Sep 17 00:00:00 2001
From: guerler <aysam.guerler@gmail.com>
Date: Thu, 15 Mar 2018 12:37:47 -0400
Subject: [PATCH 07/25] Revise condition, check explicitly for none

---
 lib/galaxy/tools/parameters/basic.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py
index 28180a3b795..79525ef8645 100644
--- a/lib/galaxy/tools/parameters/basic.py
+++ b/lib/galaxy/tools/parameters/basic.py
@@ -843,10 +843,10 @@ class SelectToolParameter(ToolParameter):
             return self.legal_values
 
     def from_json(self, value, trans, other_values={}):
-        if not value and not self.optional:
+        if value is None:
+            if self.optional:
+                return None
             raise ValueError("An invalid option was selected for %s, please verify." % (self.name))
-        if not value:
-            return None
         legal_values = self.get_legal_values(trans, other_values)
         if len(list(legal_values)) == 0:
             if self.multiple:
@@ -1308,10 +1308,10 @@ class DrillDownSelectToolParameter(SelectToolParameter):
 
     def from_json(self, value, trans, other_values={}):
         legal_values = self.get_legal_values(trans, other_values)
-        if not value and not self.optional:
+        if value is None:
+            if self.optional:
+                return None
             raise ValueError("An invalid option was selected for %s, please verify." % (self.name))
-        if not value:
-            return None
         if len(list(legal_values)) == 0:
             if self.multiple:
                 if value == '':  # No option selected

From 608f3dc9d3ed59a21fba221f590240ecfdc1294a Mon Sep 17 00:00:00 2001
From: guerler <aysam.guerler@gmail.com>
Date: Thu, 15 Mar 2018 13:08:17 -0400
Subject: [PATCH 08/25] Add check for column index values

---
 lib/galaxy/tools/parameters/basic.py | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py
index 79525ef8645..1746729fa7c 100644
--- a/lib/galaxy/tools/parameters/basic.py
+++ b/lib/galaxy/tools/parameters/basic.py
@@ -1092,6 +1092,12 @@ class ColumnListParameter(SelectToolParameter):
         if not value and self.accept_default:
             value = self.default_value or '1'
             return [value] if self.multiple else value
+        if value is not None:
+            for v in util.listify(value):
+                try:
+                    int(value)
+                except ValueError:
+                    raise ValueError("Column indices can only be integers.")
         return super(ColumnListParameter, self).from_json(value, trans, other_values)
 
     @staticmethod

From f4f275f82aa9b1e19aafe0525b4006876905ef96 Mon Sep 17 00:00:00 2001
From: guerler <aysam.guerler@gmail.com>
Date: Thu, 15 Mar 2018 13:25:37 -0400
Subject: [PATCH 09/25] Add dataset state check to column input

---
 lib/galaxy/tools/parameters/basic.py | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py
index 1746729fa7c..9af751475f3 100644
--- a/lib/galaxy/tools/parameters/basic.py
+++ b/lib/galaxy/tools/parameters/basic.py
@@ -1122,8 +1122,12 @@ class ColumnListParameter(SelectToolParameter):
             # Use representative dataset if a dataset collection is parsed
             if isinstance(dataset, trans.app.model.HistoryDatasetCollectionAssociation):
                 dataset = dataset.to_hda_representative()
-            # Columns can only be identified if metadata is available
-            if not hasattr(dataset, 'metadata') or not hasattr(dataset.metadata, 'columns') or not dataset.metadata.columns:
+            # Columns can only be identified if the dataset is ready and metadata is available
+            if not hasattr(dataset, 'state') or \
+                dataset.state != galaxy.model.Dataset.states.OK or \
+                not hasattr(dataset, 'metadata') or \
+                not hasattr(dataset.metadata, 'columns') or \
+                not dataset.metadata.columns:
                 return []
             # Build up possible columns for this dataset
             this_column_list = []

From a144eb1fb40eee0920e8153b3e51304c10130003 Mon Sep 17 00:00:00 2001
From: guerler <aysam.guerler@gmail.com>
Date: Thu, 15 Mar 2018 15:44:39 -0400
Subject: [PATCH 10/25] Fix integer test

---
 lib/galaxy/tools/parameters/basic.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py
index 9af751475f3..340cb07c8f7 100644
--- a/lib/galaxy/tools/parameters/basic.py
+++ b/lib/galaxy/tools/parameters/basic.py
@@ -1095,7 +1095,7 @@ class ColumnListParameter(SelectToolParameter):
         if value is not None:
             for v in util.listify(value):
                 try:
-                    int(value)
+                    int(v)
                 except ValueError:
                     raise ValueError("Column indices can only be integers.")
         return super(ColumnListParameter, self).from_json(value, trans, other_values)

From 29337d2b7b1d51c44ff621049483037fd9d48d1a Mon Sep 17 00:00:00 2001
From: guerler <aysam.guerler@gmail.com>
Date: Thu, 15 Mar 2018 16:35:33 -0400
Subject: [PATCH 11/25] Simplify legal value length check

---
 lib/galaxy/tools/parameters/basic.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py
index 340cb07c8f7..0499ce6edcc 100644
--- a/lib/galaxy/tools/parameters/basic.py
+++ b/lib/galaxy/tools/parameters/basic.py
@@ -848,7 +848,7 @@ class SelectToolParameter(ToolParameter):
                 return None
             raise ValueError("An invalid option was selected for %s, please verify." % (self.name))
         legal_values = self.get_legal_values(trans, other_values)
-        if len(list(legal_values)) == 0:
+        if not legal_values:
             if self.multiple:
                 # While it is generally allowed that a select value can be '',
                 # we do not allow this to be the case in a dynamically
@@ -1322,7 +1322,7 @@ class DrillDownSelectToolParameter(SelectToolParameter):
             if self.optional:
                 return None
             raise ValueError("An invalid option was selected for %s, please verify." % (self.name))
-        if len(list(legal_values)) == 0:
+        if not legal_values:
             if self.multiple:
                 if value == '':  # No option selected
                     value = None

From 05ce42d229db08c6af7425f1afe1eea625dfd1da Mon Sep 17 00:00:00 2001
From: guerler <aysam.guerler@gmail.com>
Date: Thu, 15 Mar 2018 16:54:31 -0400
Subject: [PATCH 12/25] Provide value for column index if not an integer

---
 lib/galaxy/tools/parameters/basic.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py
index 0499ce6edcc..dad9e47139b 100644
--- a/lib/galaxy/tools/parameters/basic.py
+++ b/lib/galaxy/tools/parameters/basic.py
@@ -1097,7 +1097,7 @@ class ColumnListParameter(SelectToolParameter):
                 try:
                     int(v)
                 except ValueError:
-                    raise ValueError("Column indices can only be integers.")
+                    raise ValueError("Column index '%s' is not an integer." % v)
         return super(ColumnListParameter, self).from_json(value, trans, other_values)
 
     @staticmethod

From 9febc519944dcfb5c8907b8a302fd6aff6f0f79f Mon Sep 17 00:00:00 2001
From: guerler <aysam.guerler@gmail.com>
Date: Thu, 15 Mar 2018 19:32:48 -0400
Subject: [PATCH 13/25] Identify implicit conversions, check reference context
 for datasets

---
 lib/galaxy/tools/parameters/basic.py | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py
index dad9e47139b..84a3106376d 100644
--- a/lib/galaxy/tools/parameters/basic.py
+++ b/lib/galaxy/tools/parameters/basic.py
@@ -53,10 +53,15 @@ def contains_workflow_parameter(value, search=False):
         return True
     return False
 
-
 def is_runtime_value(value):
     return isinstance(value, RuntimeValue) or (isinstance(value, dict) and value.get('__class__') == 'RuntimeValue')
 
+def has_runtime_datasets(trans, value):
+    for v in util.listify(value):
+        if isinstance(v, trans.app.model.HistoryDatasetAssociation) and \
+            (v.state != galaxy.model.Dataset.states.OK or hasattr(v, "implicit_conversion")):
+                return True
+    return False
 
 def parse_dynamic_options(param, input_source):
     options_elem = input_source.parse_dynamic_options_elem()
@@ -848,7 +853,12 @@ class SelectToolParameter(ToolParameter):
                 return None
             raise ValueError("An invalid option was selected for %s, please verify." % (self.name))
         legal_values = self.get_legal_values(trans, other_values)
-        if not legal_values:
+        workflow_building_mode = trans.workflow_building_mode
+        for context_value in other_values.values():
+            if is_runtime_value(context_value) or has_runtime_datasets(trans, context_value):
+                workflow_building_mode = workflow_building_modes.ENABLED
+                break
+        if not legal_values and workflow_building_mode:
             if self.multiple:
                 # While it is generally allowed that a select value can be '',
                 # we do not allow this to be the case in a dynamically
@@ -1113,7 +1123,7 @@ class ColumnListParameter(SelectToolParameter):
         dataset (if found).
         """
         # Get the value of the associated data reference (a dataset)
-        dataset = other_values.get(self.data_ref, None)
+        dataset = other_values.get(self.data_ref)
         # Check if a dataset is selected
         if not dataset:
             return []
@@ -1123,9 +1133,7 @@ class ColumnListParameter(SelectToolParameter):
             if isinstance(dataset, trans.app.model.HistoryDatasetCollectionAssociation):
                 dataset = dataset.to_hda_representative()
             # Columns can only be identified if the dataset is ready and metadata is available
-            if not hasattr(dataset, 'state') or \
-                dataset.state != galaxy.model.Dataset.states.OK or \
-                not hasattr(dataset, 'metadata') or \
+            if not hasattr(dataset, 'metadata') or \
                 not hasattr(dataset.metadata, 'columns') or \
                 not dataset.metadata.columns:
                 return []
@@ -1699,6 +1707,11 @@ class DataToolParameter(BaseDataToolParameter):
                 rval = values[0]
             else:
                 raise ValueError("Invalid dataset supplied to single input dataset parameter.")
+        dataset_matcher = DatasetMatcher(trans, self, None, other_values)
+        for hda in values:
+            match = dataset_matcher.hda_match(hda, check_security=False)
+            if match and match.implicit_conversion:
+                hda.implicit_conversion = True
         return rval
 
     def to_param_dict_string(self, value, other_values={}):

From 1baafd07ffbaa42bd5071a659ccb82203591afa0 Mon Sep 17 00:00:00 2001
From: guerler <aysam.guerler@gmail.com>
Date: Thu, 15 Mar 2018 19:42:16 -0400
Subject: [PATCH 14/25] Move implicit dataset tagging to dataset state check

Check attribute
---
 lib/galaxy/tools/parameters/basic.py | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py
index 84a3106376d..c3fad0ad99b 100644
--- a/lib/galaxy/tools/parameters/basic.py
+++ b/lib/galaxy/tools/parameters/basic.py
@@ -53,16 +53,18 @@ def contains_workflow_parameter(value, search=False):
         return True
     return False
 
+
 def is_runtime_value(value):
     return isinstance(value, RuntimeValue) or (isinstance(value, dict) and value.get('__class__') == 'RuntimeValue')
 
+
 def has_runtime_datasets(trans, value):
     for v in util.listify(value):
-        if isinstance(v, trans.app.model.HistoryDatasetAssociation) and \
-            (v.state != galaxy.model.Dataset.states.OK or hasattr(v, "implicit_conversion")):
-                return True
+        if isinstance(v, trans.app.model.HistoryDatasetAssociation) and hasattr(v, "state") and (v.state != galaxy.model.Dataset.states.OK or hasattr(v, "implicit_conversion")):
+            return True
     return False
 
+
 def parse_dynamic_options(param, input_source):
     options_elem = input_source.parse_dynamic_options_elem()
     if options_elem is not None:
@@ -1690,16 +1692,17 @@ class DataToolParameter(BaseDataToolParameter):
             rval = value
         else:
             rval = trans.sa_session.query(trans.app.model.HistoryDatasetAssociation).get(value)
-        if isinstance(rval, list):
-            values = rval
-        else:
-            values = [rval]
+        values = util.listify(rval)
+        dataset_matcher = DatasetMatcher(trans, self, None, other_values)
         for v in values:
             if v:
                 if v.deleted:
                     raise ValueError("The previously selected dataset has been deleted.")
                 if hasattr(v, "dataset") and v.dataset.state in [galaxy.model.Dataset.states.ERROR, galaxy.model.Dataset.states.DISCARDED]:
                     raise ValueError("The previously selected dataset has entered an unusable state")
+                match = dataset_matcher.hda_match(v, check_security=False)
+                if match and match.implicit_conversion:
+                    v.implicit_conversion = True
         if not self.multiple:
             if len(values) > 1:
                 raise ValueError("More than one dataset supplied to single input dataset parameter.")
@@ -1707,11 +1710,6 @@ class DataToolParameter(BaseDataToolParameter):
                 rval = values[0]
             else:
                 raise ValueError("Invalid dataset supplied to single input dataset parameter.")
-        dataset_matcher = DatasetMatcher(trans, self, None, other_values)
-        for hda in values:
-            match = dataset_matcher.hda_match(hda, check_security=False)
-            if match and match.implicit_conversion:
-                hda.implicit_conversion = True
         return rval
 
     def to_param_dict_string(self, value, other_values={}):

From dae6cc7f4cb403cdee44c532f314f639d8e9f96b Mon Sep 17 00:00:00 2001
From: guerler <aysam.guerler@gmail.com>
Date: Fri, 16 Mar 2018 17:40:11 -0400
Subject: [PATCH 15/25] Fix indent

---
 lib/galaxy/tools/parameters/basic.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py
index c3fad0ad99b..fa9ce839f01 100644
--- a/lib/galaxy/tools/parameters/basic.py
+++ b/lib/galaxy/tools/parameters/basic.py
@@ -1137,7 +1137,7 @@ class ColumnListParameter(SelectToolParameter):
             # Columns can only be identified if the dataset is ready and metadata is available
             if not hasattr(dataset, 'metadata') or \
                 not hasattr(dataset.metadata, 'columns') or \
-                not dataset.metadata.columns:
+                    not dataset.metadata.columns:
                 return []
             # Build up possible columns for this dataset
             this_column_list = []

From f305c0b0ab9182af0578078c622d024a57b9b05d Mon Sep 17 00:00:00 2001
From: guerler <aysam.guerler@gmail.com>
Date: Fri, 16 Mar 2018 20:34:22 -0400
Subject: [PATCH 16/25] Reinsert legal value check handler

---
 lib/galaxy/tools/parameters/basic.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py
index fa9ce839f01..f0ab502afbc 100644
--- a/lib/galaxy/tools/parameters/basic.py
+++ b/lib/galaxy/tools/parameters/basic.py
@@ -875,6 +875,8 @@ class SelectToolParameter(ToolParameter):
                         # use \r\n to separate lines.
                         value = value.split()
             return value
+        elif not legal_values:
+            raise ValueError("Parameter %s requires a value, but has no legal values defined." % self.name)
         if isinstance(value, list):
             if not self.multiple:
                 raise ValueError("Multiple values provided but parameter %s is not expecting multiple values." % self.name)

From c7e794745b1b7699a7ac34caaa684f298f562dbc Mon Sep 17 00:00:00 2001
From: guerler <aysam.guerler@gmail.com>
Date: Mon, 19 Mar 2018 11:31:16 -0400
Subject: [PATCH 17/25] Validate value before matching dataset

---
 lib/galaxy/tools/parameters/basic.py | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py
index f0ab502afbc..9a08a498f04 100644
--- a/lib/galaxy/tools/parameters/basic.py
+++ b/lib/galaxy/tools/parameters/basic.py
@@ -55,12 +55,15 @@ def contains_workflow_parameter(value, search=False):
 
 
 def is_runtime_value(value):
-    return isinstance(value, RuntimeValue) or (isinstance(value, dict) and value.get('__class__') == 'RuntimeValue')
+    return isinstance(value, RuntimeValue) or (isinstance(value, dict)
+        and value.get("__class__") == "RuntimeValue")
 
 
 def has_runtime_datasets(trans, value):
     for v in util.listify(value):
-        if isinstance(v, trans.app.model.HistoryDatasetAssociation) and hasattr(v, "state") and (v.state != galaxy.model.Dataset.states.OK or hasattr(v, "implicit_conversion")):
+        if isinstance(v, trans.app.model.HistoryDatasetAssociation) and \
+            ((hasattr(v, "state") and v.state != galaxy.model.Dataset.states.OK) or
+                hasattr(v, "implicit_conversion")):
             return True
     return False
 
@@ -1700,11 +1703,12 @@ class DataToolParameter(BaseDataToolParameter):
             if v:
                 if v.deleted:
                     raise ValueError("The previously selected dataset has been deleted.")
-                if hasattr(v, "dataset") and v.dataset.state in [galaxy.model.Dataset.states.ERROR, galaxy.model.Dataset.states.DISCARDED]:
+                elif hasattr(v, "dataset") and v.dataset.state in [galaxy.model.Dataset.states.ERROR, galaxy.model.Dataset.states.DISCARDED]:
                     raise ValueError("The previously selected dataset has entered an unusable state")
-                match = dataset_matcher.hda_match(v, check_security=False)
-                if match and match.implicit_conversion:
-                    v.implicit_conversion = True
+                elif hasattr(v, "dataset"):
+                    match = dataset_matcher.hda_match(v, check_security=False)
+                    if match and match.implicit_conversion:
+                        v.implicit_conversion = True
         if not self.multiple:
             if len(values) > 1:
                 raise ValueError("More than one dataset supplied to single input dataset parameter.")

From 81a418fe98ee2f26b35bae81060398431b9a096e Mon Sep 17 00:00:00 2001
From: guerler <aysam.guerler@gmail.com>
Date: Wed, 21 Mar 2018 08:32:21 -0400
Subject: [PATCH 18/25] Remove explicit integer validation

---
 lib/galaxy/tools/parameters/basic.py | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py
index 9a08a498f04..a9ff9f0047f 100644
--- a/lib/galaxy/tools/parameters/basic.py
+++ b/lib/galaxy/tools/parameters/basic.py
@@ -1109,12 +1109,6 @@ class ColumnListParameter(SelectToolParameter):
         if not value and self.accept_default:
             value = self.default_value or '1'
             return [value] if self.multiple else value
-        if value is not None:
-            for v in util.listify(value):
-                try:
-                    int(v)
-                except ValueError:
-                    raise ValueError("Column index '%s' is not an integer." % v)
         return super(ColumnListParameter, self).from_json(value, trans, other_values)
 
     @staticmethod

From 187c540cd4aac7e1059fdaa6aaf56c87913f1d09 Mon Sep 17 00:00:00 2001
From: guerler <aysam.guerler@gmail.com>
Date: Wed, 21 Mar 2018 11:35:54 -0400
Subject: [PATCH 19/25] Fix test case

---
 test/api/test_tools.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/api/test_tools.py b/test/api/test_tools.py
index 6f8ff1a3ed8..0200114bab2 100644
--- a/test/api/test_tools.py
+++ b/test/api/test_tools.py
@@ -456,7 +456,7 @@ class ToolsTestCase(api.ApiTestCase):
     @skip_without_tool("column_param")
     def test_column_legal_values(self):
         history_id = self.dataset_populator.new_history()
-        new_dataset1 = self.dataset_populator.new_dataset(history_id, content='#col1\tcol2')
+        new_dataset1 = self.dataset_populator.new_dataset(history_id, content='#col1\tcol2', wait=True)
         inputs = {
             'input1': {"src": "hda", "id": new_dataset1["id"]},
             'col': "' ; echo 'moo",

From ade62bc3b15b25e76d7114c5ee9051cd5f575700 Mon Sep 17 00:00:00 2001
From: guerler <aysam.guerler@gmail.com>
Date: Thu, 22 Mar 2018 10:38:55 -0400
Subject: [PATCH 20/25] Revise select display matching

---
 client/galaxy/scripts/mvc/ui/ui-misc.js | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/client/galaxy/scripts/mvc/ui/ui-misc.js b/client/galaxy/scripts/mvc/ui/ui-misc.js
index e21a669ecf7..4ed8892847b 100644
--- a/client/galaxy/scripts/mvc/ui/ui-misc.js
+++ b/client/galaxy/scripts/mvc/ui/ui-misc.js
@@ -188,18 +188,12 @@ export var Hidden = Backbone.View.extend({
 export var TextSelect = Backbone.View.extend({
     initialize: function(options) {
         this.text = new Input(options);
-        var SelectClass = Select;
-        switch (options.display) {
-            case "checkboxes":
-                SelectClass = Checkbox;
-                break;
-            case "radio":
-                SelectClass = Radio;
-                break;
-            case "radiobutton":
-                SelectClass = RadioButton;
-                break;
+        var classes = {
+            "checkboxes": Checkbox,
+            "radio": Radio,
+            "radiobutton": RadioButton
         }
+        var SelectClass = classes[options.display] || Select;
         this.select = new SelectClass.View(options);
         this.setElement($("<div/>").append(this.select.$el)
                                    .append(this.text.$el));

From 8290da24eaf0994c018b055ba9bbc73876540f7c Mon Sep 17 00:00:00 2001
From: Nicola Soranzo <nicola.soranzo@earlham.ac.uk>
Date: Thu, 22 Mar 2018 15:47:07 +0000
Subject: [PATCH 21/25] Fix indentation

---
 lib/galaxy/tools/parameters/basic.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py
index a9ff9f0047f..9fb99a8f1fe 100644
--- a/lib/galaxy/tools/parameters/basic.py
+++ b/lib/galaxy/tools/parameters/basic.py
@@ -62,7 +62,7 @@ def is_runtime_value(value):
 def has_runtime_datasets(trans, value):
     for v in util.listify(value):
         if isinstance(v, trans.app.model.HistoryDatasetAssociation) and \
-            ((hasattr(v, "state") and v.state != galaxy.model.Dataset.states.OK) or
+                ((hasattr(v, "state") and v.state != galaxy.model.Dataset.states.OK) or
                 hasattr(v, "implicit_conversion")):
             return True
     return False
@@ -1135,7 +1135,7 @@ class ColumnListParameter(SelectToolParameter):
                 dataset = dataset.to_hda_representative()
             # Columns can only be identified if the dataset is ready and metadata is available
             if not hasattr(dataset, 'metadata') or \
-                not hasattr(dataset.metadata, 'columns') or \
+                    not hasattr(dataset.metadata, 'columns') or \
                     not dataset.metadata.columns:
                 return []
             # Build up possible columns for this dataset

From 241ade62cd740bf62d2348859493ed7326799597 Mon Sep 17 00:00:00 2001
From: guerler <aysam.guerler@gmail.com>
Date: Thu, 22 Mar 2018 12:03:31 -0400
Subject: [PATCH 22/25] Align validation order for consistency

---
 lib/galaxy/tools/parameters/basic.py | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py
index 9fb99a8f1fe..61fe9e52024 100644
--- a/lib/galaxy/tools/parameters/basic.py
+++ b/lib/galaxy/tools/parameters/basic.py
@@ -853,10 +853,6 @@ class SelectToolParameter(ToolParameter):
             return self.legal_values
 
     def from_json(self, value, trans, other_values={}):
-        if value is None:
-            if self.optional:
-                return None
-            raise ValueError("An invalid option was selected for %s, please verify." % (self.name))
         legal_values = self.get_legal_values(trans, other_values)
         workflow_building_mode = trans.workflow_building_mode
         for context_value in other_values.values():
@@ -878,6 +874,10 @@ class SelectToolParameter(ToolParameter):
                         # use \r\n to separate lines.
                         value = value.split()
             return value
+        elif value is None:
+            if self.optional:
+                return None
+            raise ValueError("An invalid option was selected for %s, please verify." % (self.name))
         elif not legal_values:
             raise ValueError("Parameter %s requires a value, but has no legal values defined." % self.name)
         if isinstance(value, list):
@@ -924,7 +924,7 @@ class SelectToolParameter(ToolParameter):
 
     def get_initial_value(self, trans, other_values):
         options = list(self.get_options(trans, other_values))
-        if len(options) == 0:
+        if not options:
             return None
         value = [optval for _, optval, selected in options if selected]
         if len(value) == 0:
@@ -1327,10 +1327,6 @@ class DrillDownSelectToolParameter(SelectToolParameter):
 
     def from_json(self, value, trans, other_values={}):
         legal_values = self.get_legal_values(trans, other_values)
-        if value is None:
-            if self.optional:
-                return None
-            raise ValueError("An invalid option was selected for %s, please verify." % (self.name))
         if not legal_values:
             if self.multiple:
                 if value == '':  # No option selected
@@ -1338,6 +1334,10 @@ class DrillDownSelectToolParameter(SelectToolParameter):
                 else:
                     value = value.split("\n")
             return value
+        elif value is None:
+            if self.optional:
+                return None
+            raise ValueError("An invalid option was selected for %s, please verify." % (self.name))
         if not isinstance(value, list):
             value = [value]
         if len(value) > 1 and not self.multiple:
@@ -1397,7 +1397,7 @@ class DrillDownSelectToolParameter(SelectToolParameter):
                 recurse_options(initial_values, option['options'])
         # More working around dynamic options for workflow
         options = self.get_options(trans=trans, other_values=other_values)
-        if len(list(options)) == 0:
+        if not options:
             return None
         initial_values = []
         recurse_options(initial_values, options)

From 2b3557ffce38a97080c9921846e06131aa43e2e9 Mon Sep 17 00:00:00 2001
From: John Chilton <jmchilton@gmail.com>
Date: Mon, 26 Mar 2018 07:55:16 -0400
Subject: [PATCH 23/25] Fix test case verifying we check column values before
 tool execution.

---
 test/api/test_tools.py | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/test/api/test_tools.py b/test/api/test_tools.py
index 0200114bab2..c441a97dae6 100644
--- a/test/api/test_tools.py
+++ b/test/api/test_tools.py
@@ -456,13 +456,18 @@ class ToolsTestCase(api.ApiTestCase):
     @skip_without_tool("column_param")
     def test_column_legal_values(self):
         history_id = self.dataset_populator.new_history()
-        new_dataset1 = self.dataset_populator.new_dataset(history_id, content='#col1\tcol2', wait=True)
+        new_dataset1 = self.dataset_populator.new_dataset(history_id, content='#col1\tcol2')
         inputs = {
             'input1': {"src": "hda", "id": new_dataset1["id"]},
             'col': "' ; echo 'moo",
         }
         response = self._run("column_param", history_id, inputs)
-        assert response.status_code != 200
+        # This needs to either fail at submit time or at job prepare time, but we have
+        # to make sure the job doesn't run.
+        if response.status_code == 200:
+            job = response.json()["jobs"][0]
+            final_job_state = self.dataset_populator.wait_for_job(job["id"])
+            assert final_job_state == "error"
 
     @skip_without_tool("collection_paired_test")
     def test_collection_parameter(self):

From 91e97b60968d6813bd7a28d0d60e778b288ac385 Mon Sep 17 00:00:00 2001
From: Nicola Soranzo <nicola.soranzo@earlham.ac.uk>
Date: Mon, 26 Mar 2018 18:26:41 +0100
Subject: [PATCH 24/25] Refactor code for clarity

---
 lib/galaxy/tools/parameters/basic.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py
index 61fe9e52024..6f2ac5f4c91 100644
--- a/lib/galaxy/tools/parameters/basic.py
+++ b/lib/galaxy/tools/parameters/basic.py
@@ -859,7 +859,9 @@ class SelectToolParameter(ToolParameter):
             if is_runtime_value(context_value) or has_runtime_datasets(trans, context_value):
                 workflow_building_mode = workflow_building_modes.ENABLED
                 break
-        if not legal_values and workflow_building_mode:
+        if not legal_values:
+            if not workflow_building_mode:
+                raise ValueError("Parameter %s requires a value, but has no legal values defined." % self.name)
             if self.multiple:
                 # While it is generally allowed that a select value can be '',
                 # we do not allow this to be the case in a dynamically
@@ -878,8 +880,6 @@ class SelectToolParameter(ToolParameter):
             if self.optional:
                 return None
             raise ValueError("An invalid option was selected for %s, please verify." % (self.name))
-        elif not legal_values:
-            raise ValueError("Parameter %s requires a value, but has no legal values defined." % self.name)
         if isinstance(value, list):
             if not self.multiple:
                 raise ValueError("Multiple values provided but parameter %s is not expecting multiple values." % self.name)

From 840e436ea1f181ebe0c8478145e2cd0e97dc1778 Mon Sep 17 00:00:00 2001
From: guerler <aysam.guerler@gmail.com>
Date: Mon, 26 Mar 2018 15:45:29 -0400
Subject: [PATCH 25/25] Add strict workflow building mode check to drill downs

---
 lib/galaxy/tools/parameters/basic.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py
index 6f2ac5f4c91..1e727537855 100644
--- a/lib/galaxy/tools/parameters/basic.py
+++ b/lib/galaxy/tools/parameters/basic.py
@@ -1328,6 +1328,8 @@ class DrillDownSelectToolParameter(SelectToolParameter):
     def from_json(self, value, trans, other_values={}):
         legal_values = self.get_legal_values(trans, other_values)
         if not legal_values:
+            if not trans.workflow_building_mode:
+                raise ValueError("Parameter %s requires a value, but has no legal values defined." % self.name)
             if self.multiple:
                 if value == '':  # No option selected
                     value = None