opensource/galaxy
3
0
Fork 0
mirror of https://github.com/galaxyproject/galaxy.git synced 2026-09-24 16:30:27 +08:00
Code Issues Packages Projects Releases Wiki Activity

Fix post job action getting lost when node is made active

Browse Source 
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.
This commit is contained in:
mvdbeek
2022-08-30 17:14:34 +02:00
parent c096e599ff
commit f67ca01a99
10 changed files with 270 additions and 106 deletions
Download Patch File Download Diff File Expand all files Collapse all files
client/src/components/Workflow/Editor/Forms/FormDefault.test.js
+9 -10
View File
@@ -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,
client/src/components/Workflow/Editor/Forms/FormDefault.vue
+53 -24
View File
@@ -1,5 +1,5 @@
<template>
<FormCard :title="node.name" :icon="nodeIcon">
<FormCard :title="nodeName" :icon="nodeIcon">
<template v-slot:operations>
<b-button
v-if="isSubworkflow"
@@ -27,14 +27,14 @@
<template v-slot:body>
<FormElement
id="__label"
:value="node.label"
:value="nodeLabel"
title="Label"
help="Add a step label."
:error="errorLabel"
@input="onLabel" />
<FormElement
id="__annotation"
:value="node.annotation"
:value="nodeAnnotation"
title="Step Annotation"
:area="true"
help="Add an annotation or notes to this step. Annotations are available when a workflow is viewed."
@@ -42,10 +42,10 @@
<FormDisplay :id="id" :inputs="inputs" @onChange="onChange" />
<div v-if="isSubworkflow">
<FormOutputLabel
v-for="(output, index) in node.outputs"
v-for="(output, index) in nodeOutputs"
:key="index"
:name="output.name"
:active-outputs="node.activeOutputs"
:active-outputs="nodeActiveOutputs"
:show-details="true" />
</div>
</template>
@@ -68,6 +68,42 @@ export default {
FormOutputLabel,
},
props: {
nodeName: {
type: String,
required: true,
},
nodeId: {
type: String,
required: true,
},
nodeContentId: {
type: String,
required: true,
},
nodeAnnotation: {
type: String,
required: false,
},
nodeLabel: {
type: String,
required: true,
},
nodeType: {
type: String,
required: true,
},
nodeActiveOutputs: {
type: Object,
required: true,
},
nodeOutputs: {
type: Array,
required: true,
},
configForm: {
type: Object,
required: true,
},
datatypes: {
type: Array,
required: true,
@@ -76,43 +112,36 @@ export default {
type: Function,
required: true,
},
getNode: {
type: Function,
required: true,
},
},
computed: {
node() {
return this.getNode();
},
nodeIcon() {
return WorkflowIcons[this.node.type];
return WorkflowIcons[this.nodeType];
},
workflow() {
return this.getManager();
},
id() {
return this.node.id;
return this.nodeId;
},
inputs() {
return this.node.config_form.inputs;
return this.configForm.inputs;
},
isSubworkflow() {
return this.node.type == "subworkflow";
return this.nodeType == "subworkflow";
},
errorLabel() {
return checkLabels(this.node.id, this.node.label, this.workflow.nodes);
return checkLabels(this.nodeId, this.nodeLabel, this.workflow.nodes);
},
},
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);
},
onEditSubworkflow() {
this.$emit("onEditSubworkflow", this.node.content_id);
this.$emit("onEditSubworkflow", this.nodeContentId);
},
onUpgradeSubworkflow() {
this.$emit("onAttemptRefactor", [
@@ -120,10 +149,10 @@ export default {
]);
},
onChange(values) {
this.$emit("onSetData", this.node.id, {
id: this.node.id,
type: this.node.type,
content_id: this.node.content_id,
this.$emit("onSetData", this.nodeId, {
id: this.nodeId,
type: this.nodeType,
content_id: this.nodeContentId,
inputs: values,
});
},
client/src/components/Workflow/Editor/Forms/FormSection.vue
+46 -14
View File
@@ -18,8 +18,8 @@
v-for="(output, index) in outputs"
:key="index"
:output-name="output.name"
:active-outputs="node.activeOutputs"
:inputs="node.inputs"
:active-outputs="nodeActiveOutputs"
:inputs="nodeInputs"
:datatypes="datatypes"
:form-data="formData"
@onInput="onInput"
@@ -41,14 +41,26 @@ export default {
type: String,
required: true,
},
getNode: {
type: Function,
nodeInputs: {
type: Array,
required: true,
},
nodeOutputs: {
type: Array,
required: true,
},
nodeActiveOutputs: {
type: Object,
required: true,
},
datatypes: {
type: Array,
required: true,
},
postJobActions: {
type: Object,
required: true,
},
},
data() {
return {
@@ -56,14 +68,8 @@ export default {
};
},
computed: {
node() {
return this.getNode();
},
postJobActions() {
return this.node.postJobActions;
},
outputs() {
return this.node.outputs;
return this.nodeOutputs;
},
firstOutput() {
return this.outputs.length > 0 && this.outputs[0];
@@ -87,6 +93,34 @@ export default {
created() {
this.setFormData();
},
watch: {
formData() {
// The formData shape is kind of unfortunate, but it is what we have now.
// This should be a properly nested object whose values should be retrieved and set via a store
const postJobActions = {};
Object.entries(this.formData).forEach(([key, value]) => {
const [pja, outputName, actionType, name] = key.split("__", 4);
if (pja == "pja") {
const pjaKey = `${actionType}${outputName}`;
if (!postJobActions[pjaKey]) {
postJobActions[pjaKey] = {
action_type: actionType,
output_name: outputName,
action_arguments: {},
};
}
if (name) {
if (name == "output_name") {
postJobActions[pjaKey]["output_name"] = value;
} else {
postJobActions[pjaKey]["action_arguments"][name] = value;
}
}
}
});
this.$emit("onChange", postJobActions);
},
},
methods: {
setFormData() {
const pjas = {};
@@ -104,9 +138,8 @@ export default {
if (pjas[this.emailPayloadKey]) {
pjas[this.emailActionKey] = true;
}
this.formData = pjas;
console.debug("FormSection - Setting new data.", this.postJobActions, pjas);
this.$emit("onChange", this.formData);
this.formData = pjas;
},
setEmailAction(pjas) {
if (pjas[this.emailActionKey]) {
@@ -131,7 +164,6 @@ export default {
this.setEmailAction(this.formData);
if (changed) {
this.formData = Object.assign({}, this.formData);
this.$emit("onChange", this.formData);
}
},
onDatatype(pjaKey, outputName, newDatatype) {
client/src/components/Workflow/Editor/Forms/FormTool.test.js
+17 -20
View File
@@ -14,22 +14,22 @@ describe("FormTool", () => {
propsData: {
id: "input",
datatypes: [],
getNode: () => {
return {
id: "id",
config_form: {
id: "tool_id+1.0",
name: "tool_name",
version: "1.0",
description: "description",
inputs: [],
help: "help_text",
versions: ["1.0", "2.0", "3.0"],
},
outputs: [],
postJobActions: [],
};
configForm: {
id: "tool_id+1.0",
name: "tool_name",
version: "1.0",
description: "description",
inputs: [],
help: "help_text",
versions: ["1.0", "2.0", "3.0"],
},
nodeId: "id",
nodeAnnotation: "",
nodeLabel: "",
nodeInputs: [],
nodeOutputs: [],
nodeActiveOutputs: {},
postJobActions: {},
getManager: () => {
return {};
},
@@ -46,19 +46,16 @@ describe("FormTool", () => {
it("check version change", async () => {
const dropdowns = wrapper.findAll(".dropdown-item");
let state = wrapper.emitted().onSetData[0][1];
expect(state.tool_version).toEqual("1.0");
expect(state.tool_id).toEqual("tool_id+1.0");
let version = dropdowns.at(3);
expect(version.text()).toBe("Switch to 2.0");
await version.trigger("click");
state = wrapper.emitted().onSetData[1][1];
let state = wrapper.emitted().onSetData[0][1];
expect(state.tool_version).toEqual("2.0");
expect(state.tool_id).toEqual("tool_id+2.0");
version = dropdowns.at(2);
expect(version.text()).toBe("Switch to 3.0");
await version.trigger("click");
state = wrapper.emitted().onSetData[2][1];
state = wrapper.emitted().onSetData[1][1];
expect(state.tool_version).toEqual("3.0");
expect(state.tool_id).toEqual("tool_id+3.0");
});
client/src/components/Workflow/Editor/Forms/FormTool.vue
+58 -31
View File
@@ -2,12 +2,12 @@
<CurrentUser v-slot="{ user }">
<ToolCard
v-if="hasData"
:id="node.config_form.id"
:id="configForm.id"
:user="user"
:version="node.config_form.version"
:title="node.config_form.name"
:description="node.config_form.description"
:options="node.config_form"
:version="configForm.version"
:title="configForm.name"
:description="configForm.description"
:options="configForm"
:message-text="messageText"
:message-variant="messageVariant"
@onChangeVersion="onChangeVersion"
@@ -15,14 +15,14 @@
<template v-slot:body>
<FormElement
id="__label"
:value="node.label"
:value="nodeLabel"
title="Label"
help="Add a step label."
:error="errorLabel"
@input="onLabel" />
<FormElement
id="__annotation"
:value="node.annotation"
:value="nodeAnnotation"
title="Step Annotation"
:area="true"
help="Add an annotation or notes to this step. Annotations are available when a workflow is viewed."
@@ -34,7 +34,14 @@
text-enable="Set in Advance"
text-disable="Set at Runtime"
@onChange="onChange" />
<FormSection :id="nodeId" :get-node="getNode" :datatypes="datatypes" @onChange="onChangeSection" />
<FormSection
:id="nodeId"
:node-inputs="nodeInputs"
:node-outputs="nodeOutputs"
:node-active-outputs="nodeActiveOutputs"
:datatypes="datatypes"
:post-job-actions="postJobActions"
@onChange="onChangePostJobActions" />
</template>
</ToolCard>
</CurrentUser>
@@ -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",
client/src/components/Workflow/Editor/Index.vue
+52 -2
View File
@@ -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" />
<FormDefault
v-else-if="hasActiveNodeDefault"
:node-name="activeNodeName"
:node-id="activeNodeId"
:node-content-id="activeNodeContentId"
:node-annotation="activeNodeAnnotation"
:node-label="activeNodeLabel"
:node-type="activeNodeType"
:node-outputs="activeNodeOutputs"
:node-active-outputs="activeNodeActiveOutputs"
:config-form="activeNodeConfigForm"
:get-manager="getManager"
:get-node="getNode"
:datatypes="datatypes"
@onAnnotation="onAnnotation"
@onLabel="onLabel"
@@ -299,9 +315,39 @@ export default {
showLint() {
return this.showInPanel == "lint";
},
postJobActions() {
return this.activeNode.postJobActions;
},
activeNodeId() {
return this.activeNode && this.activeNode.id;
},
activeNodeName() {
return this.activeNode?.name;
},
activeNodeContentId() {
return this.activeNode && this.activeNode.contentId;
},
activeNodeLabel() {
return this.activeNode?.label;
},
activeNodeAnnotation() {
return this.activeNode?.annotation;
},
activeNodeConfigForm() {
return this.activeNode?.config_form;
},
activeNodeInputs() {
return this.activeNode?.inputs;
},
activeNodeOutputs() {
return this.activeNode?.outputs;
},
activeNodeActiveOutputs() {
return this.activeNode?.activeOutputs;
},
activeNodeType() {
return this.activeNode?.type;
},
hasActiveNodeDefault() {
return this.activeNode && this.activeNode.type != "tool";
},
@@ -436,6 +482,10 @@ export default {
onChange() {
this.hasChanges = true;
},
onChangePostJobActions(nodeId, postJobActions) {
Vue.set(this.nodes[nodeId], "postJobActions", postJobActions);
this.onChange();
},
onRemove(node) {
delete this.nodes[node.id];
Vue.delete(this.steps, node.id);
client/src/components/Workflow/Editor/Node.vue
+5 -2
View File
@@ -140,11 +140,12 @@ export default {
data() {
return {
popoverShow: false,
node: null,
inputs: [],
outputs: [],
inputTerminals: {},
outputTerminals: {},
postJobActions: {},
activeOutputs: null,
errors: null,
label: null,
annotation: null,
@@ -321,7 +322,9 @@ export default {
const outputNames = this.outputs.map((output) => 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) {
client/src/components/Workflow/Editor/NodeOutput.vue
+5 -2
View File
@@ -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
lib/galaxy/webapps/galaxy/api/workflows.py
-1
View File
@@ -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
lib/galaxy_test/selenium/test_workflow_editor.py
+25
View File
@@ -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(
"""