[24.0] Fix tool version switch in editor #17858
Conversation
This only happens for non-ts tools which don't have the version in their tool id.
that can result form upgrading tools.
mvdbeek
force-pushed
the
fix_tool_version_switch_in_editor
branch
from
2649b34 to
8e8d7fa
Compare
| @@ -1472,7 +1478,7 @@ def _workflow_to_dict_export(self, trans, stored=None, workflow=None, internal=F | |||
| "tool_version": module.get_version() if allow_upgrade else step.tool_version, | |||
| "name": module.get_name(), | |||
| "tool_state": json.dumps(tool_state), | |||
| "errors": module.get_errors(), | |||
| "errors": errors or module.get_errors(), | |||
There was a problem hiding this comment.
I suppose a user would like to see the errors from module.get_errors() also when errors is not empty, i.e. we could merge them?
But currently get_errors() can return None, a list or a string, depending on the module type, which is not great in itself, so I suppose we need to decide what format this should have.
There was a problem hiding this comment.
errors will contain the upgrade messages, I don't think there's a need to merge them ?
There was a problem hiding this comment.
Then maybe put them in a "messages" key?
There was a problem hiding this comment.
Then I have to redo the editor and potentially other things. I don't disagree that this should be modernized, but it doesn't seem like the right moment ?
There was a problem hiding this comment.
And sorry, it's not messages, these are actually errors, for the current state of the module.
There was a problem hiding this comment.
Does the editor work with something that can be None, list, string or dictionary?
Anyway, my main concern was about losing the errors from module.get_errors() when errors is not empty.
There was a problem hiding this comment.
Yes, the editor just dumps them as a string, so anything's fine. I don't think we really ever display them for more than a second inside the node body, and the refactoring action just uses step.upgrade_messages.
Fixes #17464
How to test the changes?
(Select all options that apply)
License