Clarify current select behavior around null and empty selections.

jmchilton · Jul 25, 2024 · a26224b · a26224b
1 parent 565e17b
commit a26224b
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 1 deletion.
diff --git a/lib/galaxy/tool_util/parameters/models.py b/lib/galaxy/tool_util/parameters/models.py
@@ -480,7 +480,10 @@ def has_selected_static_option(self):
 
     @property
     def request_requires_value(self) -> bool:
-        return not self.optional and not self.has_selected_static_option
+        # API will allow an empty value and just grab the first static option
+        # see API Tests -> test_tools.py -> test_select_first_by_default
+        # so only require a value in the multiple case if optional is False
+        return self.multiple and not self.optional
 
 
 DiscriminatorType = Union[bool, str]

diff --git a/lib/galaxy_test/api/test_tools.py b/lib/galaxy_test/api/test_tools.py
@@ -879,6 +879,25 @@ def test_dataset_hidden_after_job_finish(self):
             output_details = self.dataset_populator.get_history_dataset_details(history_id, dataset=output, wait=True)
             assert not output_details["visible"]
 
+    @skip_without_tool("gx_select")
+    def test_select_first_by_default(self):
+        # we have a tool test for this but I wanted to verify it wasn't just the
+        # tool test framework filling in a default. Creating a raw request here
+        # verifies that currently select parameters don't require a selection.
+        with self.dataset_populator.test_history(require_new=False) as history_id:
+            inputs = {}
+            response = self._run("gx_select", history_id, inputs, assert_ok=True)
+            output = response["outputs"][0]
+            output1_content = self.dataset_populator.get_history_dataset_content(history_id, dataset=output)
+            assert output1_content.strip() == "--ex1"
+
+            inputs = {
+                "parameter": None,
+            }
+            response = self._run("gx_select", history_id, inputs, assert_ok=False)
+            assert "an invalid option" in response.text
+            self._assert_status_code_is(response, 400)
+
     @skip_without_tool("multi_select")
     def test_multi_select_as_list(self):
         with self.dataset_populator.test_history(require_new=False) as history_id:

diff --git a/test/functional/tools/parameters/gx_select.xml b/test/functional/tools/parameters/gx_select.xml
@@ -23,5 +23,15 @@ echo '$parameter' >> '$output'
                 </assert_contents>
             </output>
         </test>
+        <test>
+            <!-- Selects have an implicit first option default in tool tests.
+                 Do they in the actual API also?
+            -->
+            <output name="output">
+                <assert_contents>
+                    <has_line line="--ex1" />
+                </assert_contents>
+            </output>
+        </test>
     </tests>
 </tool>
diff --git a/test/unit/tool_util/parameter_specification.yml b/test/unit/tool_util/parameter_specification.yml
@@ -82,6 +82,8 @@ gx_select:
   request_valid:
   - parameter: "--ex1"
   - parameter: "ex2"
+  # see API Tests -> test_tools.py -> test_select_first_by_default
+  - {}
   request_invalid:
   # Not allowing selecting booleans by truevalue/falsevalue - don't allow selecting
   # selects by label.
@@ -91,7 +93,18 @@ gx_select:
   - parameter: null
   - parameter: {}
   - parameter: 5
+  request_internal_valid:
+  - parameter: "--ex1"
+  - parameter: "ex2"
+  request_internal_invalid:
+  - parameter: {}
+  test_case_valid:
+  - parameter: 'ex2'
+  - parameter: '--ex1'
   - {}
+  test_case_invalid:
+  - parameter: {}
+  - parameter: null
 
 gx_select_optional:
   request_valid: