From f67ca01a99e4afeef944bff8bc480cc8096ebadc Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 30 Aug 2022 14:56:22 +0200 Subject: [PATCH] Fix post job action getting lost when node is made active And many more reactivity fixes. The core of the issue was that the FormElement would emit a onUpdate event, which triggers a request to `api/workflows/build_module`. The FormSection that controls the post job action data is not ready at this point, so when the data comes back in we have to break the connection because the post job action that sets the datatype is missing. We really don't need to call `build_module` though when change post job actions, they don't require the backend at all, we just need to bubble them up to the Index component, which can set the postJobAction on the active node (let's ignore that we should manipulate the data, not the vue instance ...). In order for that to be properly reactive I had to remove the majority of the getNode hacks. We do still rely on this in the Terminals and Connectors, so those can get out of sync still, but it fixes an annoying issue where connections are dropped without notice. --- .../Workflow/Editor/Forms/FormDefault.test.js | 19 ++-- .../Workflow/Editor/Forms/FormDefault.vue | 77 +++++++++++----- .../Workflow/Editor/Forms/FormSection.vue | 60 ++++++++++--- .../Workflow/Editor/Forms/FormTool.test.js | 37 ++++---- .../Workflow/Editor/Forms/FormTool.vue | 89 ++++++++++++------- .../src/components/Workflow/Editor/Index.vue | 54 ++++++++++- .../src/components/Workflow/Editor/Node.vue | 7 +- .../components/Workflow/Editor/NodeOutput.vue | 7 +- lib/galaxy/webapps/galaxy/api/workflows.py | 1 - .../selenium/test_workflow_editor.py | 25 ++++++ 10 files changed, 270 insertions(+), 106 deletions(-) diff --git a/client/src/components/Workflow/Editor/Forms/FormDefault.test.js b/client/src/components/Workflow/Editor/Forms/FormDefault.test.js index 91ff8e38948..12b0a9bb637 100644 --- a/client/src/components/Workflow/Editor/Forms/FormDefault.test.js +++ b/client/src/components/Workflow/Editor/Forms/FormDefault.test.js @@ -23,16 +23,15 @@ describe("FormDefault", () => { nodes: [], }; }, - getNode: () => { - return { - name: "node-title", - type: "subworkflow", - outputs: outputs, - activeOutputs: activeOutputs, - config_form: { - inputs: [], - }, - }; + nodeId: "id", + nodeContentId: "id", + nodeLabel: "label", + nodeName: "node-title", + nodeType: "subworkflow", + nodeOutputs: outputs, + nodeActiveOutputs: activeOutputs, + configForm: { + inputs: [], }, }, localVue, diff --git a/client/src/components/Workflow/Editor/Forms/FormDefault.vue b/client/src/components/Workflow/Editor/Forms/FormDefault.vue index 5434c434f0c..dedb6decf64 100644 --- a/client/src/components/Workflow/Editor/Forms/FormDefault.vue +++ b/client/src/components/Workflow/Editor/Forms/FormDefault.vue @@ -1,5 +1,5 @@ @@ -58,6 +65,34 @@ export default { FormSection, }, props: { + nodeId: { + type: String, + required: true, + }, + nodeAnnotation: { + type: String, + required: true, + }, + nodeLabel: { + type: String, + required: true, + }, + nodeInputs: { + type: Array, + required: true, + }, + nodeOutputs: { + type: Array, + required: true, + }, + nodeActiveOutputs: { + type: Object, + required: true, + }, + configForm: { + type: Object, + required: true, + }, datatypes: { type: Array, required: true, @@ -66,40 +101,33 @@ export default { type: Function, required: true, }, - getNode: { - type: Function, + postJobActions: { + type: Object, required: true, }, }, data() { return { mainValues: {}, - sectionValues: {}, messageText: "", messageVariant: "success", }; }, computed: { - node() { - return this.getNode(); - }, workflow() { return this.getManager(); }, id() { - return `${this.node.id}:${this.node.config_form.id}`; - }, - nodeId() { - return this.node.id; + return `${this.nodeId}:${this.configForm.id}`; }, hasData() { - return !!this.node.config_form; + return !!this.configForm; }, errorLabel() { - return checkLabels(this.node.id, this.node.label, this.workflow.nodes); + return checkLabels(this.nodeId, this.nodeLabel, this.workflow.nodes); }, inputs() { - const inputs = this.node.config_form.inputs; + const inputs = this.configForm.inputs; Utils.deepeach(inputs, (input) => { if (input.type) { if (["data", "data_collection"].indexOf(input.type) != -1) { @@ -126,35 +154,34 @@ export default { return inputs; }, errors() { - return this.node.config_form.errors; + return this.configForm.errors; }, }, methods: { onAnnotation(newAnnotation) { - this.$emit("onAnnotation", this.node.id, newAnnotation); + this.$emit("onAnnotation", this.nodeId, newAnnotation); }, onLabel(newLabel) { - this.$emit("onLabel", this.node.id, newLabel); + this.$emit("onLabel", this.nodeId, newLabel); }, onChange(values) { this.mainValues = values; this.postChanges(); }, - onChangeSection(values) { - this.sectionValues = values; - this.postChanges(); + onChangePostJobActions(postJobActions) { + this.$emit("onChangePostJobActions", this.nodeId, postJobActions); }, onChangeVersion(newVersion) { - this.messageText = `Now you are using '${this.node.config_form.name}' version ${newVersion}.`; + this.messageText = `Now you are using '${this.configForm.name}' version ${newVersion}.`; this.postChanges(newVersion); }, onUpdateFavorites(user, newFavorites) { user.preferences["favorites"] = newFavorites; }, postChanges(newVersion) { - const payload = Object.assign({}, this.mainValues, this.sectionValues); + const payload = Object.assign({}, this.mainValues); console.debug("FormTool - Posting changes.", payload); - const options = this.node.config_form; + const options = this.configForm; let toolId = options.id; let toolVersion = options.version; if (newVersion) { @@ -162,7 +189,7 @@ export default { toolVersion = newVersion; console.debug("FormTool - Tool version changed.", toolId, toolVersion); } - this.$emit("onSetData", this.node.id, { + this.$emit("onSetData", this.nodeId, { tool_id: toolId, tool_version: toolVersion, type: "tool", diff --git a/client/src/components/Workflow/Editor/Index.vue b/client/src/components/Workflow/Editor/Index.vue index e571efb4ea9..87a98a57204 100644 --- a/client/src/components/Workflow/Editor/Index.vue +++ b/client/src/components/Workflow/Editor/Index.vue @@ -135,15 +135,31 @@ v-if="hasActiveNodeTool" :key="activeNodeId" :get-manager="getManager" - :get-node="getNode" + :node-id="activeNodeId" + :node-annotation="activeNodeAnnotation" + :node-label="activeNodeLabel" + :node-inputs="activeNodeInputs" + :node-outputs="activeNodeOutputs" + :node-active-outputs="activeNodeActiveOutputs" + :config-form="activeNodeConfigForm" :datatypes="datatypes" + :post-job-actions="postJobActions" + @onChangePostJobActions="onChangePostJobActions" @onAnnotation="onAnnotation" @onLabel="onLabel" @onSetData="onSetData" /> output.name); this.activeOutputs.initialize(this.outputs, data.workflow_outputs); this.activeOutputs.filterOutputs(outputNames); - this.postJobActions = data.post_job_actions || {}; + // data coming from the workflow editor API has post job actions, + // data coming from the build_module call does not (and should not) + this.postJobActions = data.post_job_actions || this.postJobActions; this.config_form = data.config_form; }, initData(data) { diff --git a/client/src/components/Workflow/Editor/NodeOutput.vue b/client/src/components/Workflow/Editor/NodeOutput.vue index 0a28967de1a..3209904d101 100644 --- a/client/src/components/Workflow/Editor/NodeOutput.vue +++ b/client/src/components/Workflow/Editor/NodeOutput.vue @@ -80,6 +80,9 @@ export default { } return extensions; }, + effectiveOutput() { + return { ...this.output, extensions: this.extensions }; + }, }, watch: { label() { @@ -88,10 +91,10 @@ export default { this.$emit("onChange"); }); }, - output(newOutput) { + effectiveOutput(newOutput) { const oldTerminal = this.terminal; if (oldTerminal instanceof this.terminalClassForOutput(newOutput)) { - oldTerminal.update({ ...newOutput, extensions: this.extensions }); + oldTerminal.update(newOutput); oldTerminal.destroyInvalidConnections(); } else { // create new terminal, connect like old terminal, destroy old terminal diff --git a/lib/galaxy/webapps/galaxy/api/workflows.py b/lib/galaxy/webapps/galaxy/api/workflows.py index 3a4583acd8d..45a10417cbe 100644 --- a/lib/galaxy/webapps/galaxy/api/workflows.py +++ b/lib/galaxy/webapps/galaxy/api/workflows.py @@ -670,7 +670,6 @@ class WorkflowsAPIController( "inputs": module.get_all_inputs(connectable_only=True), "outputs": module.get_all_outputs(), "config_form": module.get_config_form(), - "post_job_actions": module.get_post_job_actions(inputs), } @expose_api diff --git a/lib/galaxy_test/selenium/test_workflow_editor.py b/lib/galaxy_test/selenium/test_workflow_editor.py index f39b4357011..831a499847f 100644 --- a/lib/galaxy_test/selenium/test_workflow_editor.py +++ b/lib/galaxy_test/selenium/test_workflow_editor.py @@ -558,6 +558,31 @@ steps: editor.node.output_data_row(output_name="out_file1", extension="bam").wait_for_visible() self.assert_not_connected("create_2#out_file1", "checksum#input") + @selenium_test + def test_change_datatype_post_job_action_lost_regression(self): + self.open_in_workflow_editor( + """ +class: GalaxyWorkflow +inputs: [] +steps: + - tool_id: create_2 + label: create_2 + outputs: + out_file1: + change_datatype: bam + - tool_id: metadata_bam + label: metadata_bam + in: + input_bam: create_2/out_file1 +""" + ) + self.assert_connected("create_2#out_file1", "metadata_bam#input_bam") + editor = self.components.workflow_editor + node = editor.node._(label="create_2") + node.wait_for_and_click() + self.assert_connected("create_2#out_file1", "metadata_bam#input_bam") + + @selenium_test def test_change_datatype_in_subworkflow(self): self.open_in_workflow_editor( """