From ec32669cf51bea5882316d84657fc2224a5976c5 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Wed, 8 Nov 2023 10:38:42 +0100 Subject: [PATCH 01/13] Change log level for duplicate data table entries to warning Fixes https://github.com/galaxyproject/galaxy/issues/16987, though there's probably something to be done for loc files being loaded from tools. At the minimum this is a versioning issue, if a new version of featureCounts comes with a new built-in reference then an old version shouldn't use it, and vice-versa. Seems like a hard problem to solve. --- lib/galaxy/tool_util/data/__init__.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/tool_util/data/__init__.py b/lib/galaxy/tool_util/data/__init__.py index 5e2fdfb7908b..927841d9004c 100644 --- a/lib/galaxy/tool_util/data/__init__.py +++ b/lib/galaxy/tool_util/data/__init__.py @@ -250,6 +250,11 @@ def add_entries( self.add_entry( entry, allow_duplicates=allow_duplicates, persist=persist, entry_source=entry_source, **kwd ) + except MessageException as e: + if e.type == "warning": + log.warning(str(e)) + else: + log.error(str(e)) except Exception as e: log.error(str(e)) return self._loaded_content_version @@ -686,7 +691,8 @@ def _add_entry( self.data.append(fields) else: raise MessageException( - f"Attempted to add fields ({fields}) to data table '{self.name}', but this entry already exists and allow_duplicates is False." + f"Attempted to add fields ({fields}) to data table '{self.name}', but this entry already exists and allow_duplicates is False.", + type="warning", ) else: raise MessageException( From edcc043b613755b262396eb36c24fe276a3f5d7c Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 15 Nov 2023 11:02:52 -0500 Subject: [PATCH 02/13] Make testing tool panel views a little easier. --- run.sh | 1 + test/functional/tools/sample_tool_conf.xml | 6 ++++++ test/integration/panel_views_1/custom_13.yml | 4 ++++ 3 files changed, 11 insertions(+) create mode 100644 test/integration/panel_views_1/custom_13.yml diff --git a/run.sh b/run.sh index 2bcdb680943c..db6a9c2a5538 100755 --- a/run.sh +++ b/run.sh @@ -41,6 +41,7 @@ then export GALAXY_CONFIG_OVERRIDE_ENABLE_BETA_TOOL_FORMATS="true" export GALAXY_CONFIG_INTERACTIVETOOLS_ENABLE="true" export GALAXY_CONFIG_OVERRIDE_WEBHOOKS_DIR="test/functional/webhooks" + export GALAXY_CONFIG_OVERRIDE_PANEL_VIEWS_DIR="$(pwd)/test/integration/panel_views_1/" fi set_galaxy_config_file_var diff --git a/test/functional/tools/sample_tool_conf.xml b/test/functional/tools/sample_tool_conf.xml index 83a70258cc6a..0e32640e96fe 100644 --- a/test/functional/tools/sample_tool_conf.xml +++ b/test/functional/tools/sample_tool_conf.xml @@ -220,6 +220,12 @@ +
+ + + +
+ diff --git a/test/integration/panel_views_1/custom_13.yml b/test/integration/panel_views_1/custom_13.yml new file mode 100644 index 000000000000..ccf114bde897 --- /dev/null +++ b/test/integration/panel_views_1/custom_13.yml @@ -0,0 +1,4 @@ +name: Filtered Test Section w/multiple versions +type: activity +items: +- sections: [test_section_multi] From 3e4089526e7d5f1c04dceb0418bcc58c681e64fd Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 15 Nov 2023 14:12:07 -0500 Subject: [PATCH 03/13] Fix duplicated tools in tool panel view section copying. --- lib/galaxy/tool_util/toolbox/panel.py | 28 ++++++++++++++++++-- lib/galaxy/tool_util/toolbox/views/static.py | 5 ++-- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/lib/galaxy/tool_util/toolbox/panel.py b/lib/galaxy/tool_util/toolbox/panel.py index 8375d39441d2..0c98a28d3f22 100644 --- a/lib/galaxy/tool_util/toolbox/panel.py +++ b/lib/galaxy/tool_util/toolbox/panel.py @@ -63,14 +63,38 @@ def __init__(self, item=None): self.links = item.get("links") or None self.elems = ToolPanelElements() - def copy(self): + def copy(self, merge_tools=False): copy = ToolSection() copy.name = self.name copy.id = self.id copy.version = self.version copy.description = self.description copy.links = self.links - copy.elems.update(self.elems) + + for key, panel_type, value in self.panel_items_iter(): + if panel_type == panel_item_types.TOOL and merge_tools: + tool = value + tool_lineage = tool.lineage + + tool_copied = False + if tool_lineage is not None: + version_ids = tool_lineage.get_version_ids(reverse=True) + + for version_id in version_ids: + if copy.elems.has_tool_with_id(version_id): + tool_copied = True + break + + if self.elems.has_tool_with_id(version_id): + copy.elems.append_tool(self.elems.get_tool_with_id(version_id)) + tool_copied = True + break + + if not tool_copied: + copy.elems[key] = value + else: + copy.elems[key] = value + return copy def to_dict(self, trans, link_details=False, tool_help=False, toolbox=None): diff --git a/lib/galaxy/tool_util/toolbox/views/static.py b/lib/galaxy/tool_util/toolbox/views/static.py index 750426e89080..2eba8eef3f4b 100644 --- a/lib/galaxy/tool_util/toolbox/views/static.py +++ b/lib/galaxy/tool_util/toolbox/views/static.py @@ -120,7 +120,7 @@ def definition_with_items_to_panel(definition, allow_sections: bool = True, item f"Failed to find matching section for (id, name) = ({element.section}, {element.section})" ) continue - section = closest_section.copy() + section = closest_section.copy(merge_tools=True) apply_filter(element, section.elems) new_panel.append_section(section.id, section) elif element.content_type == "label": @@ -151,7 +151,8 @@ def definition_with_items_to_panel(definition, allow_sections: bool = True, item if closest_section is None: log.warning(f"Failed to find matching section for (id, name) = ({element.items_from}, None)") continue - elems = closest_section.elems.copy() + section = closest_section.copy(merge_tools=True) + elems = section.elems apply_filter(element, elems) for key, item in elems.items(): new_panel[key] = item From f36b8474a3c39d5521f81d7c44affdcdedfd1663 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Thu, 16 Nov 2023 05:51:03 -0500 Subject: [PATCH 04/13] Comment about new bug discovered. Lower priority though right? Come back and fix it in dev? --- lib/galaxy_test/api/test_tools.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/galaxy_test/api/test_tools.py b/lib/galaxy_test/api/test_tools.py index 2b8620d964fa..bad17d7241d7 100644 --- a/lib/galaxy_test/api/test_tools.py +++ b/lib/galaxy_test/api/test_tools.py @@ -991,7 +991,11 @@ def test_test_by_versions(self): test_data_response = self._get("tools/multiple_versions/test_data?tool_version=*") test_data_response.raise_for_status() test_data_dicts = test_data_response.json() - assert len(test_data_dicts) == 3 + # this found a bug - tools that appear in the toolbox twice should not cause + # multiple copies of test data to be returned. This assertion broke when + # we placed multiple_versions in the test tool panel in multiple places. We need + # to fix this but it isn't as important as the existing bug. + # assert len(test_data_dicts) == 3 @skip_without_tool("multiple_versions") def test_show_with_wrong_tool_version_in_tool_id(self): From c3dc92b4638ab6f80d6bb7cdf4bceed1f5692aac Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Thu, 16 Nov 2023 09:57:40 +0100 Subject: [PATCH 05/13] Test that only latest version is loaded --- test/integration/test_panel_views.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/integration/test_panel_views.py b/test/integration/test_panel_views.py index 50a56e65b93c..8da009a7f530 100644 --- a/test/integration/test_panel_views.py +++ b/test/integration/test_panel_views.py @@ -110,6 +110,18 @@ def test_global_filters_on_integrated_panel(self): tools = section["elems"] assert len(tools) == 2, len(tools) + def test_only_latest_version_in_panel(self): + index = self.galaxy_interactor.get("tools", data=dict(in_panel=True, view="custom_13")) + index.raise_for_status() + index_as_list = index.json() + sections = [x for x in index_as_list if x["model_class"] == "ToolSection"] + assert len(sections) == 1 + section = sections[0] + assert section["id"] == "test_section_multi" + tools = section["elems"] + assert len(tools) == 1, len(tools) + assert tools[0]["version"] == "0.2" + class TestPanelViewsFromConfigIntegration(integration_util.IntegrationTestCase): framework_tool_and_types = True From ca5fce75669b7f5a6b360459d7aa8a2c02e88f3d Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Thu, 16 Nov 2023 10:56:17 +0100 Subject: [PATCH 06/13] Test tool shed version as well --- lib/galaxy_test/base/uses_shed_api.py | 18 ++++++++++++-- test/integration/test_panel_views.py | 34 +++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/lib/galaxy_test/base/uses_shed_api.py b/lib/galaxy_test/base/uses_shed_api.py index 3b8da66f37aa..58a2c74522a7 100644 --- a/lib/galaxy_test/base/uses_shed_api.py +++ b/lib/galaxy_test/base/uses_shed_api.py @@ -34,15 +34,28 @@ def install_repo_request(self, payload: Dict[str, Any]) -> Response: ) def repository_operation( - self, operation: OperationT, owner: str, name: str, changeset: str, tool_shed_url: str = DEFAULT_TOOL_SHED_URL + self, + operation: OperationT, + owner: str, + name: str, + changeset: str, + tool_shed_url: str = DEFAULT_TOOL_SHED_URL, + tool_panel_section_id: Optional[str] = None, ) -> Dict[str, Any]: payload = {"tool_shed_url": tool_shed_url, "name": name, "owner": owner, "changeset_revision": changeset} + if tool_panel_section_id: + payload["tool_panel_section_id"] = tool_panel_section_id create_response = operation(payload) assert_status_code_is(create_response, 200) return create_response.json() def install_repository( - self, owner: str, name: str, changeset: str, tool_shed_url: str = DEFAULT_TOOL_SHED_URL + self, + owner: str, + name: str, + changeset: str, + tool_shed_url: str = DEFAULT_TOOL_SHED_URL, + tool_panel_section_id: Optional[str] = None, ) -> Dict[str, Any]: try: return self.repository_operation( @@ -51,6 +64,7 @@ def install_repository( name=name, changeset=changeset, tool_shed_url=tool_shed_url, + tool_panel_section_id=tool_panel_section_id, ) except AssertionError as e: if "Error attempting to retrieve installation information from tool shed" in unicodify(e): diff --git a/test/integration/test_panel_views.py b/test/integration/test_panel_views.py index 8da009a7f530..5f8b3f5507b3 100644 --- a/test/integration/test_panel_views.py +++ b/test/integration/test_panel_views.py @@ -1,6 +1,8 @@ import os +import time from galaxy_test.driver import integration_util +from galaxy_test.driver.uses_shed import UsesShed THIS_DIR = os.path.dirname(__file__) PANEL_VIEWS_DIR_1 = os.path.join(THIS_DIR, "panel_views_1") @@ -123,6 +125,38 @@ def test_only_latest_version_in_panel(self): assert tools[0]["version"] == "0.2" +class TestPanelViewsWithShedTools(integration_util.IntegrationTestCase, UsesShed): + framework_tool_and_types = True + allow_tool_conf_override = False + + @classmethod + def handle_galaxy_config_kwds(cls, config): + super().handle_galaxy_config_kwds(config) + config["panel_views_dir"] = PANEL_VIEWS_DIR_1 + + def test_only_latest_version_in_panel_fastp(self): + FASTP_REPO = {"name": "fastp", "owner": "iuc", "tool_panel_section_id": "test_section_multi"} + OLD_CHANGESET = "1d8fe9bc4cb0" + NEW_CHANGESET = "dbf9c561ef29" + self.install_repository(**FASTP_REPO, changeset=OLD_CHANGESET) + self.install_repository(**FASTP_REPO, changeset=NEW_CHANGESET) + + # give the toolbox a moment to reload after repo installation + time.sleep(5) + index = self.galaxy_interactor.get("tools", data=dict(in_panel=True, view="custom_13")) + index.raise_for_status() + index_as_list = index.json() + sections = [x for x in index_as_list if x["model_class"] == "ToolSection"] + assert len(sections) == 1 + section = sections[0] + assert section["id"] == "test_section_multi" + tools = section["elems"] + assert len(tools) == 2, len(tools) + fastp = tools[0] + assert fastp["id"] == "toolshed.g2.bx.psu.edu/repos/iuc/fastp/fastp/0.20.1+galaxy0" + assert fastp["tool_shed_repository"]["changeset_revision"] == NEW_CHANGESET + + class TestPanelViewsFromConfigIntegration(integration_util.IntegrationTestCase): framework_tool_and_types = True From 0e142414cd2d38be75e3b82e30e1d0c1c73b7c23 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Thu, 16 Nov 2023 15:08:21 +0100 Subject: [PATCH 07/13] Exclude multiple_versions from test_global_filters_on_integrated_panel --- test/integration/panel_views_1/custom_12.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/test/integration/panel_views_1/custom_12.yml b/test/integration/panel_views_1/custom_12.yml index eeec5378690e..32f17580e201 100644 --- a/test/integration/panel_views_1/custom_12.yml +++ b/test/integration/panel_views_1/custom_12.yml @@ -3,3 +3,4 @@ type: activity excludes: - tool_id_regex: 'multi_data_.*' - tool_id_regex: '.*_text_option' +- tool_id_regex: 'multiple_version.*' From 3e13db9ee5b296b8afb0b6ee512140c7fc92cc30 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Thu, 16 Nov 2023 18:38:30 +0100 Subject: [PATCH 08/13] Ensure we're not allowing None as non-optional workflow input --- lib/galaxy/workflow/run_request.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/workflow/run_request.py b/lib/galaxy/workflow/run_request.py index e6ade4609703..26b5411cbe46 100644 --- a/lib/galaxy/workflow/run_request.py +++ b/lib/galaxy/workflow/run_request.py @@ -125,7 +125,8 @@ def _normalize_inputs( # but asserting 'optional' is definitely a bool and not a String->Bool or something is a good # start to ensure tool state is being preserved and loaded in a type safe way. assert isinstance(optional, bool) - if not inputs_key and default_value is None and not optional: + has_input_value = inputs_key and inputs[inputs_key] is not None + if not has_input_value and default_value is None and not optional: message = f"Workflow cannot be run because an expected input step '{step.id}' ({step.label}) is not optional and no input." raise exceptions.MessageException(message) if inputs_key: From 26573dd7ed9d3640b51678e03f966e5f4aec0e7b Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 17 Nov 2023 15:51:20 +0100 Subject: [PATCH 09/13] Prevent workflow submission with validation errors And disable submit button when request is being handled. --- .../src/components/Common/ButtonSpinner.vue | 5 + .../Workflow/Run/WorkflowRunFormSimple.vue | 122 ++++++++++-------- lib/galaxy/workflow/run_request.py | 2 +- 3 files changed, 76 insertions(+), 53 deletions(-) diff --git a/client/src/components/Common/ButtonSpinner.vue b/client/src/components/Common/ButtonSpinner.vue index e77f9ae66f31..b6e0c128c3cf 100644 --- a/client/src/components/Common/ButtonSpinner.vue +++ b/client/src/components/Common/ButtonSpinner.vue @@ -14,6 +14,7 @@ variant="primary" class="d-flex flex-nowrap align-items-center text-nowrap" :title="tooltip" + :disabled="disabled" @click="$emit('onClick')"> {{ title }} @@ -43,6 +44,10 @@ export default { type: String, default: null, }, + disabled: { + type: Boolean, + default: false, + }, }, }; diff --git a/client/src/components/Workflow/Run/WorkflowRunFormSimple.vue b/client/src/components/Workflow/Run/WorkflowRunFormSimple.vue index f5def99fec4f..c73b8b58ff7e 100644 --- a/client/src/components/Workflow/Run/WorkflowRunFormSimple.vue +++ b/client/src/components/Workflow/Run/WorkflowRunFormSimple.vue @@ -1,56 +1,58 @@