Merge pull request #12120 from mvdbeek/fix_and_enhance_subworkflow_step_upgrade_messages

[21.05] Fix and enhance subworkflow step upgrade messages

lib/galaxy/managers/workflows.py

@@ -617,7 +617,7 @@ class WorkflowContentsManager(UsesAnnotations):
                 continue
             if step.upgrade_messages:
                 has_upgrade_messages = True
-            if step.type == 'tool' or step.type is None:
+            if step.type in ('tool', 'subworkflow', None):
                 if step.module.version_changes:
                     step_version_changes.extend(step.module.version_changes)
                 step_errors = step.module.get_errors()
@@ -805,7 +805,7 @@ class WorkflowContentsManager(UsesAnnotations):
             # Fix any missing parameters
             upgrade_message_dict = module.check_and_update_state() or {}
             if hasattr(module, "version_changes") and module.version_changes:
-                upgrade_message_dict[module.tool.name] = "\n".join(module.version_changes)
+                upgrade_message_dict[module.get_name()] = "\n".join(module.version_changes)
             # Get user annotation.
             config_form = module.get_config_form(step=step)
             annotation_str = self.get_item_annotation_str(trans.sa_session, trans.user, step) or ''

lib/galaxy/tools/toolbox/base.py

@@ -761,21 +761,6 @@ class AbstractToolBox(Dictifiable, ManagesIntegratedToolPanelMixin):
             return True
         return False
 
-    def get_tool_id(self, tool_id):
-        """ Take a tool id - potentially from a different Galaxy instance or that
-        is no longer loaded - and find the closest match to the currently loaded
-        tools (using get_tool for inexact matches which currently returns the oldest
-        tool shed installed tool with the same short id).
-        """
-        if tool_id not in self._tools_by_id:
-            tool = self.get_tool(tool_id)
-            if tool:
-                tool_id = tool.id
-            else:
-                tool_id = None
-        # else exact match - leave unmodified.
-        return tool_id
-
     def get_loaded_tools_by_lineage(self, tool_id):
         """Get all loaded tools associated by lineage to the tool whose id is tool_id."""
         tool_lineage = self._lineage_map.get(tool_id)

lib/galaxy/workflow/modules.py

@@ -398,6 +398,7 @@ class SubWorkflowModule(WorkflowModule):
     # - Second pass actually turn RuntimeInputs into inputs if possible.
     type = "subworkflow"
     name = "Subworkflow"
+    _modules = None
 
     @classmethod
     def from_dict(Class, trans, d, **kwds):
@@ -455,7 +456,21 @@ class SubWorkflowModule(WorkflowModule):
         return inputs
 
     def get_modules(self):
-        return [module_factory.from_workflow_step(self.trans, step) for step in self.subworkflow.steps]
+        if self._modules is None:
+            self._modules = [module_factory.from_workflow_step(self.trans, step) for step in self.subworkflow.steps]
+        return self._modules
+
+    @property
+    def version_changes(self):
+        version_changes = []
+        for m in self.get_modules():
+            if hasattr(m, 'version_changes'):
+                version_changes.extend(m.version_changes)
+        return version_changes
+
+    def check_and_update_state(self):
+        states = (m.check_and_update_state() for m in self.get_modules())
+        return [upgrade_message for upgrade_message in states if upgrade_message] or None
 
     def get_errors(self, **kwargs):
         errors = (module.get_errors(include_tool_id=True) for module in self.get_modules())
@@ -1244,13 +1259,15 @@ class ToolModule(WorkflowModule):
     def __init__(self, trans, tool_id, tool_version=None, exact_tools=True, tool_uuid=None, **kwds):
         super().__init__(trans, content_id=tool_id, **kwds)
         self.tool_id = tool_id
-        self.tool_version = str(tool_version)
+        self.tool_version = str(tool_version) if tool_version else None
         self.tool_uuid = tool_uuid
         self.tool = trans.app.toolbox.get_tool(tool_id, tool_version=tool_version, exact=exact_tools, tool_uuid=tool_uuid)
         if self.tool:
-            if tool_version and exact_tools and str(self.tool.version) != tool_version:
-                safe_version = WORKFLOW_SAFE_TOOL_VERSION_UPDATES.get(tool_id)
-                if safe_version and safe_version.current_version >= packaging.version.parse(tool_version) >= safe_version.min_version:
+            current_tool_id = self.tool.id
+            current_tool_version = str(self.tool.version)
+            if tool_version and exact_tools and self.tool_version != current_tool_version:
+                safe_version = WORKFLOW_SAFE_TOOL_VERSION_UPDATES.get(current_tool_id)
+                if safe_version and safe_version.current_version >= packaging.version.parse(current_tool_id) >= safe_version.min_version:
                     self.tool = trans.app.toolbox.get_tool(tool_id, tool_version=tool_version, exact=False, tool_uuid=tool_uuid)
                 else:
                     log.info(f"Exact tool specified during workflow module creation for [{tool_id}] but couldn't find correct version [{tool_version}].")
@@ -1299,22 +1316,19 @@ class ToolModule(WorkflowModule):
 
     @classmethod
     def from_workflow_step(Class, trans, step, **kwds):
-        if step.tool_id is not None:
-            tool_id = trans.app.toolbox.get_tool_id(step.tool_id) or step.tool_id
-        else:
-            tool_id = None
         tool_version = step.tool_version
         tool_uuid = step.tool_uuid
-        module = super().from_workflow_step(trans, step, tool_id=tool_id, tool_version=tool_version, tool_uuid=tool_uuid, **kwds)
+        kwds['exact_tools'] = False
+        module = super().from_workflow_step(trans, step, tool_id=step.tool_id, tool_version=tool_version, tool_uuid=tool_uuid, **kwds)
         module.workflow_outputs = step.workflow_outputs
         module.post_job_actions = {}
         for pja in step.post_job_actions:
             module.post_job_actions[pja.action_type] = pja
         if module.tool:
             message = ""
-            if step.tool_id and step.tool_id != module.tool_id:  # This means the exact version of the tool is not installed. We inform the user.
+            if step.tool_id and step.tool_id != module.tool.id or step.tool_version and step.tool_version != module.tool.version:  # This means the exact version of the tool is not installed. We inform the user.
                 old_tool_shed = step.tool_id.split("/repos/")[0]
-                if old_tool_shed not in tool_id:  # Only display the following warning if the tool comes from a different tool shed
+                if old_tool_shed not in module.tool.id:  # Only display the following warning if the tool comes from a different tool shed
                     old_tool_shed_url = get_tool_shed_url_from_tool_shed_registry(trans.app, old_tool_shed)
                     if not old_tool_shed_url:  # a tool from a different tool_shed has been found, but the original tool shed has been deactivated
                         old_tool_shed_url = "http://" + old_tool_shed  # let's just assume it's either http, or a http is forwarded to https.
@@ -1324,12 +1338,12 @@ class ToolModule(WorkflowModule):
                     message += f"The tool \'{module.tool.name}\', version {tool_version} by the owner {module.tool.repository_owner} installed from <a href=\"{old_url}\" target=\"_blank\">{old_tool_shed_url}</a> is not available. "
                     message += f"A derivation of this tool installed from <a href=\"{new_url}\" target=\"_blank\">{new_tool_shed_url}</a> will be used instead. "
             if step.tool_version and (step.tool_version != module.tool.version):
-                message += f"<span title=\"tool id '{tool_id}'\">Using version '{module.tool.version}' instead of version '{step.tool_version}' specified in this workflow. "
+                message += f"<span title=\"tool id '{step.tool_id}'\">Using version '{module.tool.version}' instead of version '{step.tool_version}' specified in this workflow. "
             if message:
                 log.debug(message)
                 module.version_changes.append(message)
         else:
-            log.warning("The tool '%s' is missing. Cannot build workflow module." % tool_id)
+            log.warning("The tool '%s' is missing. Cannot build workflow module." % step.tool_id)
         return module
 
     # ---- Saving in various forms ------------------------------------------

lib/galaxy_test/selenium/test_workflow_editor.py

@@ -1,5 +1,6 @@
 import json
 
+import yaml
 from selenium.webdriver.common.keys import Keys
 
 from galaxy_test.base.workflow_fixtures import (
@@ -404,6 +405,30 @@ steps:
         self.components.workflow_editor.modal_button_continue.wait_for_and_click()
         self.assert_has_changes_and_save()
 
+    @selenium_test
+    def test_editor_subworkflow_tool_upgrade_message(self):
+        workflow_populator = self.workflow_populator
+        embedded_workflow = yaml.safe_load(WORKFLOW_WITH_OLD_TOOL_VERSION)
+        outer_workflow = yaml.safe_load("""
+class: GalaxyWorkflow
+inputs:
+  outer_input: data
+steps:
+  nested_workflow:
+    run: {}
+    in:
+      input1: outer_input
+        """)
+        outer_workflow['steps']['nested_workflow']['run'] = embedded_workflow
+        workflow_populator.upload_yaml_workflow(json.dumps(outer_workflow), exact_tools=True)
+        self.workflow_index_open()
+        self.workflow_index_click_option("Edit")
+        self.sleep_for(self.wait_types.UX_RENDER)
+        self.assert_modal_has_text("Using version '0.2' instead of version '0.0.1'")
+        self.screenshot("workflow_editor_subworkflow_tool_upgrade")
+        self.components.workflow_editor.modal_button_continue.wait_for_and_click()
+        self.assert_has_changes_and_save()
+
     @staticmethod
     def set_text_element(element, value):
         # Try both, no harm here

test/unit/tools/test_toolbox.py

@@ -389,17 +389,17 @@ class ToolBoxTestCase(BaseToolBoxTestCase):
         assert section_by_label is section
         assert tool_panel_section_key == 'tid'
 
-    def test_get_tool_id(self):
+    def test_get_tool(self):
         self._init_tool()
         self._setup_two_versions_in_config()
         self._setup_two_versions()
-        assert self.toolbox.get_tool_id("test_tool") in [
+        assert self.toolbox.get_tool("test_tool").id in [
             "github.com/galaxyproject/example/test_tool/0.1",
             "github.com/galaxyproject/example/test_tool/0.2"
         ]
-        assert self.toolbox.get_tool_id("github.com/galaxyproject/example/test_tool/0.1") == "github.com/galaxyproject/example/test_tool/0.1"
-        assert self.toolbox.get_tool_id("github.com/galaxyproject/example/test_tool/0.2") == "github.com/galaxyproject/example/test_tool/0.2"
-        assert self.toolbox.get_tool_id("github.com/galaxyproject/example/test_tool/0.3") != "github.com/galaxyproject/example/test_tool/0.3"
+        assert self.toolbox.get_tool("github.com/galaxyproject/example/test_tool/0.1").id == "github.com/galaxyproject/example/test_tool/0.1"
+        assert self.toolbox.get_tool("github.com/galaxyproject/example/test_tool/0.2").id == "github.com/galaxyproject/example/test_tool/0.2"
+        assert self.toolbox.get_tool("github.com/galaxyproject/example/test_tool/0.3").id != "github.com/galaxyproject/example/test_tool/0.3"
 
     def test_tool_dir(self):
         self._init_tool()

test/unit/workflows/workflow_support.py

@@ -75,10 +75,6 @@ class TestToolbox:
         # Real tool box returns None of missing tool also
         return self.tools.get(tool_id, None)
 
-    def get_tool_id(self, tool_id):
-        tool = self.get_tool(tool_id)
-        return tool and tool.id
-
 
 def yaml_to_model(has_dict, id_offset=100):
     if isinstance(has_dict, str):