Merge pull request #14539 from mvdbeek/post_job_action_state_bug_fix

[22.05] Fix post job action getting lost when node is made active
This commit is contained in:
Dannon
2022-08-30 22:30:08 -04:00
committed by GitHub
10 changed files with 270 additions and 106 deletions
@@ -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,
@@ -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,
});
},
@@ -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) {
@@ -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");
});
@@ -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",
@@ -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);
@@ -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) {
@@ -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
@@ -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
@@ -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(
"""