diff --git a/client/package.json b/client/package.json index 6e3f76bfc7e..da8fcd44777 100644 --- a/client/package.json +++ b/client/package.json @@ -51,6 +51,7 @@ "csv-parse": "^5.3.0", "d3": "^7.8.0", "d3-zoom": "^3.0.0", + "d3v3": "npm:d3@3", "date-fns": "^2.28.0", "date-fns-tz": "^1.3.3", "decode-uri-component": "^0.2.1", diff --git a/client/src/components/Panels/Common/ToolSection.vue b/client/src/components/Panels/Common/ToolSection.vue index c57738e49e3..39942c94cff 100644 --- a/client/src/components/Panels/Common/ToolSection.vue +++ b/client/src/components/Panels/Common/ToolSection.vue @@ -97,6 +97,10 @@ export default { type: Boolean, default: false, }, + sortItems: { + type: Boolean, + default: true, + }, }, setup() { const { config, isLoaded } = useConfig(); @@ -135,6 +139,7 @@ export default { if ( this.isLoaded && this.config.toolbox_auto_sort === true && + this.sortItems === true && !this.category.elems.some((el) => el.text !== undefined && el.text !== "") ) { const elements = [...this.category.elems]; diff --git a/client/src/components/Panels/ToolBoxWorkflow.vue b/client/src/components/Panels/ToolBoxWorkflow.vue index bcb26c53f60..251ad3c8497 100644 --- a/client/src/components/Panels/ToolBoxWorkflow.vue +++ b/client/src/components/Panels/ToolBoxWorkflow.vue @@ -59,6 +59,7 @@ :key="workflowSection.name" :category="workflowSection" section-name="workflows" + :sort-items="false" operation-icon="fa fa-files-o" operation-title="Insert individual steps." :query-filter="query" diff --git a/client/src/components/Workflow/Editor/Forms/FormOutput.vue b/client/src/components/Workflow/Editor/Forms/FormOutput.vue index 12e858b61d2..cc1b2a49c8a 100644 --- a/client/src/components/Workflow/Editor/Forms/FormOutput.vue +++ b/client/src/components/Workflow/Editor/Forms/FormOutput.vue @@ -84,7 +84,6 @@ import FormCard from "@/components/Form/FormCard"; import FormElement from "@/components/Form/FormElement"; import FormOutputLabel from "@/components/Workflow/Editor/Forms/FormOutputLabel"; -import { Step } from "@/stores/workflowStepStore"; const actions = [ "RenameDatasetAction__newname", @@ -129,7 +128,8 @@ export default { required: true, }, step: { - type: Step, + // type Step from "@/stores/workflowStepStore"; + type: Object, required: true, }, }, diff --git a/client/src/components/Workflow/Editor/Forms/FormOutputLabel.test.js b/client/src/components/Workflow/Editor/Forms/FormOutputLabel.test.js index b0f9c8fd882..bfd43ce5263 100644 --- a/client/src/components/Workflow/Editor/Forms/FormOutputLabel.test.js +++ b/client/src/components/Workflow/Editor/Forms/FormOutputLabel.test.js @@ -52,9 +52,13 @@ describe("FormOutputLabel", () => { const inputOther = wrapperOther.find("input"); await input.setValue("new-label"); expect(wrapper.find(".ui-form-error").exists()).toBe(false); + expect(wrapperOther.find(".ui-form-error").exists()).toBe(false); await inputOther.setValue("other-label"); + expect(wrapper.find(".ui-form-error").exists()).toBe(false); + expect(wrapperOther.find(".ui-form-error").exists()).toBe(false); await input.setValue("other-label"); expect(wrapper.find(".ui-form-error").text()).toBe("Duplicate output label 'other-label' will be ignored."); + expect(wrapperOther.find(".ui-form-error").exists()).toBe(false); expect(stepStore.workflowOutputs["new-label"].outputName).toBe("output-name"); }); }); diff --git a/client/src/components/Workflow/Editor/Forms/FormOutputLabel.vue b/client/src/components/Workflow/Editor/Forms/FormOutputLabel.vue index 4ff135742a1..25666521b7f 100644 --- a/client/src/components/Workflow/Editor/Forms/FormOutputLabel.vue +++ b/client/src/components/Workflow/Editor/Forms/FormOutputLabel.vue @@ -45,7 +45,8 @@ const label = computed(() => { }); function onInput(newLabel: string) { - if (!stepStore.workflowOutputs[newLabel]) { + const existingWorkflowOutput = stepStore.workflowOutputs[newLabel]; + if (!existingWorkflowOutput) { const newWorkflowOutputs = [...(props.step.workflow_outputs || [])].filter( (workflowOutput) => workflowOutput.output_name !== props.name ); @@ -55,7 +56,7 @@ function onInput(newLabel: string) { }); stepStore.updateStep({ ...props.step, workflow_outputs: newWorkflowOutputs }); error.value = undefined; - } else { + } else if (existingWorkflowOutput.stepId !== props.step.id) { error.value = `Duplicate output label '${newLabel}' will be ignored.`; } } diff --git a/client/src/components/Workflow/Editor/Forms/FormSection.vue b/client/src/components/Workflow/Editor/Forms/FormSection.vue index b924842cd71..d86ab98d969 100644 --- a/client/src/components/Workflow/Editor/Forms/FormSection.vue +++ b/client/src/components/Workflow/Editor/Forms/FormSection.vue @@ -30,7 +30,6 @@ - diff --git a/client/src/components/Workflow/Editor/Node.vue b/client/src/components/Workflow/Editor/Node.vue index 0d7abe9abd6..f10d5e629a2 100644 --- a/client/src/components/Workflow/Editor/Node.vue +++ b/client/src/components/Workflow/Editor/Node.vue @@ -176,7 +176,12 @@ const connectionStore = useConnectionStore(); const stateStore = useWorkflowStateStore(); const stepStore = useWorkflowStepStore(); const isLoading = computed(() => Boolean(stateStore.getStepLoadingState(props.id)?.loading)); -useNodePosition(el, props.id, stateStore); +useNodePosition( + el, + props.id, + stateStore, + computed(() => props.scale) +); const title = computed(() => props.step.label || props.step.name); const idString = computed(() => `wf-node-step-${props.id}`); const showRule = computed(() => props.step.inputs?.length > 0 && props.step.outputs?.length > 0); diff --git a/client/src/components/Workflow/Editor/NodeInput.vue b/client/src/components/Workflow/Editor/NodeInput.vue index c0f89069fd2..31d63a133a5 100644 --- a/client/src/components/Workflow/Editor/NodeInput.vue +++ b/client/src/components/Workflow/Editor/NodeInput.vue @@ -17,6 +17,13 @@ @click="onRemove" @keyup.delete="onRemove" /> {{ label }} + + * + @@ -25,7 +32,6 @@ import { useCoordinatePosition } from "./composables/useCoordinatePosition"; import { useConnectionStore } from "@/stores/workflowConnectionStore"; import { computed } from "vue"; import { inject, ref, toRefs, watchEffect } from "vue"; -import { UseElementBoundingReturn } from "@vueuse/core"; import { storeToRefs } from "pinia"; import { useTerminal } from "./composables/useTerminal"; import { DatatypesMapperModel } from "@/components/Datatypes/model"; @@ -51,7 +57,8 @@ export default { required: true, }, rootOffset: { - type: UseElementBoundingReturn, + // type UseElementBoundingReturn from "@vueuse/core"; + type: Object, required: true, }, scale: { @@ -214,3 +221,16 @@ export default { }, }; + + diff --git a/client/src/components/Workflow/Editor/WorkflowGraph.vue b/client/src/components/Workflow/Editor/WorkflowGraph.vue index 3dc2aa26b14..896a30ac22d 100644 --- a/client/src/components/Workflow/Editor/WorkflowGraph.vue +++ b/client/src/components/Workflow/Editor/WorkflowGraph.vue @@ -29,10 +29,10 @@ @@ -68,7 +68,7 @@ const canvas: Ref = ref(null); const elementBounding = useElementBounding(canvas, { windowResize: false, windowScroll: false }); const scroll = useScroll(canvas); -const { transform, panBy, setZoom, moveTo } = useD3Zoom(1, minZoom, maxZoom, canvas, scroll); +const { transform, panBy, setZoom, moveTo } = useD3Zoom(1, minZoom, maxZoom, canvas, scroll, { x: 20, y: 20 }); const isDragging = ref(false); provide("isDragging", isDragging); diff --git a/client/src/components/Workflow/Editor/WorkflowMinimap.vue b/client/src/components/Workflow/Editor/WorkflowMinimap.vue index 09890756cf4..4327e843f1b 100644 --- a/client/src/components/Workflow/Editor/WorkflowMinimap.vue +++ b/client/src/components/Workflow/Editor/WorkflowMinimap.vue @@ -1,171 +1,296 @@ + + - +.workflow-overview-body { + --node-color: #{$brand-primary}; + --error-color: #{$state-danger-bg}; + --selected-outline-color: #{$brand-primary}; + --view-color: #{fade-out($brand-dark, 0.8)}; + --view-outline-color: #{$brand-info}; +} + diff --git a/client/src/components/Workflow/Editor/composables/d3Zoom.ts b/client/src/components/Workflow/Editor/composables/d3Zoom.ts index 17df1fc74dd..07ecb321784 100644 --- a/client/src/components/Workflow/Editor/composables/d3Zoom.ts +++ b/client/src/components/Workflow/Editor/composables/d3Zoom.ts @@ -17,9 +17,10 @@ export function useD3Zoom( minZoom: number, maxZoom: number, targetRef: Ref, - scroll: UseScrollReturn + scroll: UseScrollReturn, + initialPan: XYPosition = { x: 0, y: 0 } ) { - const transform = ref({ x: 0, y: 0, k: k }); + const transform = ref({ x: initialPan.x, y: initialPan.y, k: k }); const d3Zoom = zoom().filter(filter).scaleExtent([minZoom, maxZoom]); watch(targetRef, () => { diff --git a/client/src/components/Workflow/Editor/composables/useNodePosition.ts b/client/src/components/Workflow/Editor/composables/useNodePosition.ts index 2ce4e85c31d..ec51e0f08a1 100644 --- a/client/src/components/Workflow/Editor/composables/useNodePosition.ts +++ b/client/src/components/Workflow/Editor/composables/useNodePosition.ts @@ -1,14 +1,33 @@ import { useElementBounding } from "@vueuse/core"; -import { onUnmounted, reactive, type Ref } from "vue"; +import { onUnmounted, unref, watch, type ComputedRef, type Ref } from "vue"; import type { useWorkflowStateStore } from "@/stores/workflowEditorStateStore"; export function useNodePosition( nodeRef: Ref, stepId: number, - workflowStateStore: ReturnType + workflowStateStore: ReturnType, + scale: ComputedRef | Ref ) { const position = useElementBounding(nodeRef, { windowResize: false }); - workflowStateStore.setStepPosition(stepId, reactive(position)); + + watch( + Object.values(position), + () => { + workflowStateStore.setStepPosition(stepId, { + height: unref(position.height) / scale.value, + width: unref(position.width) / scale.value, + left: unref(position.left) / scale.value, + right: unref(position.right) / scale.value, + top: unref(position.top) / scale.value, + bottom: unref(position.bottom) / scale.value, + x: unref(position.x) / scale.value, + y: unref(position.y) / scale.value, + update: position.update, + }); + }, + { immediate: true } + ); + onUnmounted(() => { workflowStateStore.deleteStepPosition(stepId); }); diff --git a/client/src/components/Workflow/Editor/modules/geometry.ts b/client/src/components/Workflow/Editor/modules/geometry.ts new file mode 100644 index 00000000000..b462f6e308f --- /dev/null +++ b/client/src/components/Workflow/Editor/modules/geometry.ts @@ -0,0 +1,202 @@ +/** simple rectangle without rotation */ +export interface Rectangle { + x: number; + y: number; + width: number; + height: number; +} + +/** + * Class compatible with rectangle interface. + * Provides additional properties and methods specific to bounding boxes. + * Useful to calculate the bounds of multiple rectangles, + * using the `fitRectangle` method. + */ +export class AxisAlignedBoundingBox implements Rectangle { + /** x coordinate of left edge */ + x = Infinity; + + /** y coordinate of upper edge */ + y = Infinity; + + /** x coordinate of right edge */ + endX = -Infinity; + + /** y coordinate of lower edge */ + endY = -Infinity; + + get width() { + const width = this.endX - this.x; + return width > 0 ? width : 0; + } + + set width(value) { + this.endX = this.x + value; + } + + get height() { + const height = this.endY - this.y; + return height > 0 ? height : 0; + } + + set height(value) { + this.endY = this.y + value; + } + + reset() { + this.x = Infinity; + this.y = Infinity; + this.endX = -Infinity; + this.endY = -Infinity; + } + + /** expand bounding box to fit a rectangle */ + fitRectangle(rect: Readonly) { + if (this.x > rect.x) { + this.x = rect.x; + } + + if (this.y > rect.y) { + this.y = rect.y; + } + + if (this.endX < rect.x + rect.width) { + this.endX = rect.x + rect.width; + } + + if (this.endY < rect.y + rect.height) { + this.endY = rect.y + rect.height; + } + } + + /** make width and height the same, maintaining the center of the bounding box */ + squareCenter() { + if (this.width > this.height) { + const difference = this.width - this.height; + this.y -= difference * 0.5; + this.endY += difference * 0.5; + } else { + const difference = this.height - this.width; + this.x -= difference * 0.5; + this.endX += difference * 0.5; + } + } + + /** expand bounding box in every direction */ + expand(by: number) { + this.x -= by; + this.y -= by; + this.endX += by; + this.endY += by; + } + + /** check if a point is inside the bounding box */ + isPointInBounds(point: { x: number; y: number }) { + if (point.x > this.x && point.y > this.y && point.x < this.endX && point.y < this.endY) { + return true; + } else { + return false; + } + } +} + +/* Format + [a b + c d + e f] + as used by canvas: https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/transform +*/ +// prettier-ignore +export type Matrix = [ + number, number, + number, number, + number, number, +]; + +/** vector as a tuple */ +export type Vector = [number, number]; + +/** + * Wraps basic transform operations. + * Each operation returns a new instance, so method calls can be chained + * without mutating the initial transform. + */ +export class Transform { + matrix: Matrix; + + constructor(matrix: Matrix = [1, 0, 0, 1, 0, 0]) { + this.matrix = matrix; + } + + /** returns a new transform with a translation vector added */ + translate(vector: Vector) { + // prettier-ignore + return new Transform([ + this.matrix[0], this.matrix[1], + this.matrix[2], this.matrix[3], + this.matrix[4] + vector[0], this.matrix[5] + vector[1] + ]); + } + + /** returns a new transform scaled by a given vector */ + scale(vector: Vector) { + // prettier-ignore + return new Transform([ + this.matrix[0] * vector[0], this.matrix[1] * vector[1], + this.matrix[2] * vector[0], this.matrix[3] * vector[1], + this.matrix[4], this.matrix[5] + ]); + } + + /** Returns the inverse vector. Can be used to un-transform things */ + inverse() { + const m = this.matrix; + // https://www.wolframalpha.com/input?i=Inverse+%5B%7B%7Ba%2Cc%2Ce%7D%2C%7Bb%2Cd%2Cf%7D%2C%7B0%2C0%2C1%7D%7D%5D + const denominator = m[0] * m[3] - m[1] * m[2]; + + const a = m[3] / denominator; + const b = m[1] / -denominator; + const c = m[2] / -denominator; + const d = m[0] / denominator; + const e = (m[3] * m[4] - m[2] * m[5]) / -denominator; + const f = (m[1] * m[4] - m[0] * m[5]) / denominator; + + // prettier-ignore + return new Transform([ + a, b, + c, d, + e, f + ]); + } + + /** applies this transform to a rendering context */ + applyToContext(ctx: CanvasRenderingContext2D): void { + ctx.transform(...this.matrix); + } + + /** returns a vector transformed by this transform */ + apply(vector: Vector): Vector { + return [ + this.matrix[0] * vector[0] + this.matrix[2] * vector[1] + this.matrix[4], + this.matrix[1] * vector[0] + this.matrix[3] * vector[1] + this.matrix[5], + ]; + } + + /** removes the translation portion of the transform */ + resetTranslation(): Transform { + // prettier-ignore + return new Transform ([ + this.matrix[0], this.matrix[1], + this.matrix[2], this.matrix[3], + 0, 0 + ]); + } + + get scaleX() { + return this.matrix[0]; + } + + get scaleY() { + return this.matrix[3]; + } +} diff --git a/client/src/stores/workflowStepStore.ts b/client/src/stores/workflowStepStore.ts index 20057d984f1..034a98ab1ff 100644 --- a/client/src/stores/workflowStepStore.ts +++ b/client/src/stores/workflowStepStore.ts @@ -252,7 +252,11 @@ export const useWorkflowStepStore = defineStore("workflowStepStore", { }, removeConnection(connection: Connection) { const inputStep = this.getStep(connection.input.stepId); - Vue.delete(inputStep.input_connections, connection.input.name); + if (this.getStepExtraInputs(inputStep.id).find((input) => connection.input.name === input.name)) { + inputStep.input_connections[connection.input.name] = undefined; + } else { + Vue.delete(inputStep.input_connections, connection.input.name); + } this.updateStep(inputStep); }, removeStep(this: State, stepId: number) { diff --git a/client/src/style/scss/workflow.scss b/client/src/style/scss/workflow.scss index a3ca99cf01d..c16fc0af9af 100644 --- a/client/src/style/scss/workflow.scss +++ b/client/src/style/scss/workflow.scss @@ -209,45 +209,51 @@ @extend .mr-1; } .workflow-overview { + --workflow-overview-size: 150px; + --workflow-overview-min-size: 50px; + --workflow-overview-max-size: 300px; + --workflow-overview-padding: 7px; + --workflow-overview-border: 1px; + border-top-left-radius: 0.3rem; - cursor: pointer; + cursor: nwse-resize; position: absolute; - width: 150px; - height: 150px; + width: var(--workflow-overview-size); + height: var(--workflow-overview-size); right: 0px; bottom: 0px; - border-top: solid $border-color 1px; - border-left: solid $border-color 1px; - padding: 7px 0 0 7px; + border-top: solid $border-color var(--workflow-overview-border); + border-left: solid $border-color var(--workflow-overview-border); background: $workflow-overview-bg no-repeat url("../../assets/images/resizable.png"); z-index: 20000; overflow: hidden; - max-width: 300px; - max-height: 300px; - min-width: 50px; - min-height: 50px; - .viewport { - stroke: #25537b; - } + padding: var(--workflow-overview-padding) 0 0 var(--workflow-overview-padding); + + // account for padding and border + max-width: calc( + var(--workflow-overview-max-size) + var(--workflow-overview-padding) + + var(--workflow-overview-border) + ); + max-height: calc( + var(--workflow-overview-max-size) + var(--workflow-overview-padding) + + var(--workflow-overview-border) + ); + min-width: calc( + var(--workflow-overview-min-size) + var(--workflow-overview-padding) + + var(--workflow-overview-border) + ); + min-height: calc( + var(--workflow-overview-min-size) + var(--workflow-overview-padding) + + var(--workflow-overview-border) + ); + .workflow-overview-body { + cursor: pointer; position: relative; overflow: hidden; width: 100%; height: 100%; } - .mini-node { - + .ok { - // this is $primary-brand / #25537b - filter: invert(26%) sepia(75%) saturate(489%) hue-rotate(166deg) brightness(90%) contrast(89%); - } - + .error { - // this is #e31a1e - filter: invert(16%) sepia(86%) saturate(3962%) hue-rotate(350deg) brightness(96%) contrast(97%); - } - + .highlight { - filter: invert(14%) sepia(38%) saturate(1544%) hue-rotate(321deg) brightness(84%) contrast(117%); - } - } } #input-choices-menu { color: black; diff --git a/client/src/utils/navigation/navigation.yml b/client/src/utils/navigation/navigation.yml index 7114c497544..9f614fd7579 100644 --- a/client/src/utils/navigation/navigation.yml +++ b/client/src/utils/navigation/navigation.yml @@ -615,6 +615,11 @@ workflow_editor: type: xpath selector: > //div[@id='form-element-__annotation']//textarea + step_when: + type: xpath + selector: > + //div[@id='form-element-__conditional']//input + param_type_form: '#parameter_definition\|parameter_type' configure_output: type: xpath selector: > diff --git a/client/src/viz/circster.js b/client/src/viz/circster.js index adea9a7586d..a14c2535b06 100644 --- a/client/src/viz/circster.js +++ b/client/src/viz/circster.js @@ -4,8 +4,8 @@ import Backbone from "backbone"; import { getAppRoot } from "onload/loadConfig"; import { getGalaxyInstance } from "app"; import _l from "utils/localization"; -import * as d3 from "d3"; -import { event as currentEvent } from "d3"; +import * as d3 from "d3v3"; +import { event as currentEvent } from "d3v3"; import visualization from "viz/visualization"; import mod_utils from "utils/utils"; import config from "utils/config"; diff --git a/client/src/viz/phyloviz.js b/client/src/viz/phyloviz.js index b71098e3389..616f1692fbe 100644 --- a/client/src/viz/phyloviz.js +++ b/client/src/viz/phyloviz.js @@ -1,7 +1,7 @@ import $ from "jquery"; import Backbone from "backbone"; import _l from "utils/localization"; -import * as d3 from "d3"; +import * as d3 from "d3v3"; import visualization_mod from "viz/visualization"; import { Dataset } from "mvc/dataset/data"; import mod_icon_btn from "mvc/ui/icon-button"; diff --git a/client/src/viz/sweepster.js b/client/src/viz/sweepster.js index 8578d481247..f5a65b3f9ab 100644 --- a/client/src/viz/sweepster.js +++ b/client/src/viz/sweepster.js @@ -8,7 +8,7 @@ import $ from "jquery"; import Backbone from "backbone"; import { getAppRoot } from "onload/loadConfig"; import _l from "utils/localization"; -import * as d3 from "d3"; +import * as d3 from "d3v3"; import visualization from "viz/visualization"; import tracks from "viz/trackster/tracks"; import tools from "viz/tools"; diff --git a/client/src/viz/tools.js b/client/src/viz/tools.js index 5fb54fe1fa4..2255442ec26 100644 --- a/client/src/viz/tools.js +++ b/client/src/viz/tools.js @@ -3,7 +3,7 @@ */ import _ from "underscore"; import $ from "jquery"; -import * as d3 from "d3"; +import * as d3 from "d3v3"; import Backbone from "backbone"; import { getAppRoot } from "onload/loadConfig"; import util from "viz/trackster/util"; diff --git a/client/yarn.lock b/client/yarn.lock index d7f18e64d3a..6bdce602d75 100644 --- a/client/yarn.lock +++ b/client/yarn.lock @@ -4515,6 +4515,11 @@ d3@^7.8.0: d3-transition "3" d3-zoom "3" +"d3v3@npm:d3@3": + version "3.5.17" + resolved "https://registry.yarnpkg.com/d3/-/d3-3.5.17.tgz#bc46748004378b21a360c9fc7cf5231790762fb8" + integrity sha512-yFk/2idb8OHPKkbAL8QaOaqENNoMhIaSHZerk3oQsECwkObkCpJyjYwCe+OHiq6UEdhe1m8ZGARRRO3ljFjlKg== + d@1, d@^1.0.1: version "1.0.1" resolved "https://registry.npmjs.org/d/-/d-1.0.1.tgz" diff --git a/lib/galaxy/managers/users.py b/lib/galaxy/managers/users.py index a3b31ec8843..622a91da2de 100644 --- a/lib/galaxy/managers/users.py +++ b/lib/galaxy/managers/users.py @@ -537,7 +537,7 @@ class UserManager(base.ModelManager, deletable.PurgableManagerMixin): reset_user, prt = self.get_reset_token(trans, email) if prt: host = self.__get_host(trans) - reset_url = url_for(controller="root", action="login", token=prt.token) + reset_url = url_for(controller="login", action="start", token=prt.token) body = PASSWORD_RESET_TEMPLATE % ( host, prt.expiration_time.strftime(trans.app.config.pretty_datetime_format), diff --git a/lib/galaxy/tool_util/edam_util.py b/lib/galaxy/tool_util/edam_util.py index 788cfa86c15..514a551a5c7 100644 --- a/lib/galaxy/tool_util/edam_util.py +++ b/lib/galaxy/tool_util/edam_util.py @@ -65,7 +65,7 @@ def load_edam_tree_from_tsv_stream(tsv_stream: TextIO, *included_terms: str): parents = fields[parents_column].split("|") edam[term_id] = { - "label": fields[label_column], + "label": fields[label_column].strip('"'), "definition": fields[definition_column].strip('"'), "parents": [x[len(EDAM_PREFIX) :] for x in parents if x.startswith(EDAM_PREFIX)], } diff --git a/lib/galaxy/tool_util/linters/help.py b/lib/galaxy/tool_util/linters/help.py index 1d5776a8c56..0008d465662 100644 --- a/lib/galaxy/tool_util/linters/help.py +++ b/lib/galaxy/tool_util/linters/help.py @@ -1,4 +1,7 @@ """This module contains a linting function for a tool's help.""" + +from typing import Union + from galaxy.util import ( rst_to_html, unicodify, @@ -30,10 +33,22 @@ def lint_help(tool_xml, lint_ctx): if "TODO" in help_text: lint_ctx.warn("Help contains TODO text.", node=helps[0]) - try: - rst_to_html(help_text, error=True) - except Exception as e: - lint_ctx.warn(f"Invalid reStructuredText found in help - [{unicodify(e)}].", node=helps[0]) - return + invalid_rst = rst_invalid(help_text) + if invalid_rst: + lint_ctx.warn(f"Invalid reStructuredText found in help - [{invalid_rst}].", node=helps[0]) + else: + lint_ctx.valid("Help contains valid reStructuredText.", node=helps[0]) - lint_ctx.valid("Help contains valid reStructuredText.", node=helps[0]) + +def rst_invalid(text: str) -> Union[bool, str]: + """ + Predicate to determine if text is invalid reStructuredText. + Return False if the supplied text is valid reStructuredText or + a string indicating the problem. + """ + invalid_rst: Union[bool, str] = False + try: + rst_to_html(text, error=True) + except Exception as e: + invalid_rst = unicodify(e) + return invalid_rst diff --git a/lib/galaxy/tool_util/linters/inputs.py b/lib/galaxy/tool_util/linters/inputs.py index 53cf2db0613..4721d580644 100644 --- a/lib/galaxy/tool_util/linters/inputs.py +++ b/lib/galaxy/tool_util/linters/inputs.py @@ -109,7 +109,7 @@ PARAMETER_VALIDATOR_TYPE_COMPATIBILITY = { } PARAM_TYPE_CHILD_COMBINATIONS = [ - ("./options", ["select", "drill_down"]), + ("./options", ["data", "select", "drill_down"]), ("./options/option", ["drill_down"]), ("./column", ["data_column"]), ] @@ -186,6 +186,37 @@ def lint_inputs(tool_xml, lint_ctx): lint_ctx.warn( f"Param input [{param_name}] with no format specified - 'data' format will be assumed.", node=param ) + options = param.findall("./options") + has_options_filter_attribute = False + if len(options) == 1: + for oa in options[0].attrib: + if oa == "options_filter_attribute": + has_options_filter_attribute = True + else: + lint_ctx.error(f"Data parameter [{param_name}] uses invalid attribute: {oa}", node=param) + elif len(options) > 1: + lint_ctx.error(f"Data parameter [{param_name}] contains multiple options elements.", node=options[1]) + # for data params only filters with key='build' of type='data_meta' are allowed + filters = param.findall("./options/filter") + for f in filters: + if not f.get("ref"): + lint_ctx.error( + f"Data parameter [{param_name}] filter needs to define a ref attribute", + node=f, + ) + if has_options_filter_attribute: + if f.get("type") != "data_meta": + lint_ctx.error( + f'Data parameter [{param_name}] for filters only type="data_meta" is allowed, found type="{f.get("type")}"', + node=f, + ) + else: + if f.get("key") != "dbkey" or f.get("type") != "data_meta": + lint_ctx.error( + f'Data parameter [{param_name}] for filters only type="data_meta" and key="dbkey" are allowed, found type="{f.get("type")}" and key="{f.get("key")}"', + node=f, + ) + elif param_type == "select": # get dynamic/statically defined options dynamic_options = param.get("dynamic_options", None) diff --git a/lib/galaxy/tool_util/loader_directory.py b/lib/galaxy/tool_util/loader_directory.py index fd0e73e58f5..1491f5ab047 100644 --- a/lib/galaxy/tool_util/loader_directory.py +++ b/lib/galaxy/tool_util/loader_directory.py @@ -190,7 +190,7 @@ def looks_like_a_data_manager_xml(path): def as_dict_if_looks_like_yaml_or_cwl_with_class(path, classes): """ - get a dict from yaml file if it contains `class: CLASS`, where CLASS is + get a dict from yaml file if it contains a line `class: CLASS`, where CLASS is any string given in CLASSES. must appear in the first 5k and also load properly in total. """ @@ -199,7 +199,7 @@ def as_dict_if_looks_like_yaml_or_cwl_with_class(path, classes): start_contents = f.read(5 * 1024) except UnicodeDecodeError: return False, None - if re.search(rf"\nclass:\s+({'|'.join(classes)})\s*\n", start_contents) is None: + if re.search(rf"^class:\s+{'|'.join(classes)}\s*$", start_contents, re.MULTILINE) is None: return False, None with open(path) as f: diff --git a/lib/galaxy/tool_util/xsd/galaxy.xsd b/lib/galaxy/tool_util/xsd/galaxy.xsd index 0c7ccdea0d5..c4115d9e717 100644 --- a/lib/galaxy/tool_util/xsd/galaxy.xsd +++ b/lib/galaxy/tool_util/xsd/galaxy.xsd @@ -3999,14 +3999,17 @@ dataset for the contained input of the type specified using the ``type`` tag. `` tag when the ``type`` attribute value is ``select`` or -``data`` and used to dynamically generated lists of options. This tag set -dynamically creates a list of options whose values can be -obtained from a predefined file stored locally or a dataset selected from the -current history. +``data`` and used to dynamically generated lists of options. + +For data parameters this tag can be used to restrict possible input datasets to datasets that match the ``dbkey`` of another data input by including a ``data_meta`` filter. See for +instance here: [/tools/maf/interval2maf.xml](https://github.com/galaxyproject/galaxy/blob/master/tools/maf/interval2maf.xml) + +For select parameters this tag set dynamically creates a list of options whose +values can be obtained from a predefined file stored locally or a dataset +selected from the current history. There are at least five basic ways to use this tag - four of these correspond to a ``from_XXX`` attribute on the ``options`` directive and the other is to diff --git a/lib/galaxy_test/selenium/test_workflow_editor.py b/lib/galaxy_test/selenium/test_workflow_editor.py index eb40f60ff96..24686289d39 100644 --- a/lib/galaxy_test/selenium/test_workflow_editor.py +++ b/lib/galaxy_test/selenium/test_workflow_editor.py @@ -747,6 +747,67 @@ steps: workflow = self.workflow_populator.download_workflow(workflow_id) assert len(workflow["steps"]) == 3 + @selenium_test + def test_editor_create_conditional_step(self): + editor = self.components.workflow_editor + self.workflow_create_new(annotation="simple when step definition") + # Insert a boolean parameter + self.workflow_editor_add_input(item_name="parameter_input") + param_type_element = editor.param_type_form.wait_for_present() + self.switch_param_type(param_type_element, "Boolean") + editor.label_input.wait_for_and_send_keys("param_input") + editor.tool_menu.wait_for_visible() + # Insert cat tool + self.tool_open("cat") + self.sleep_for(self.wait_types.UX_RENDER) + editor.label_input.wait_for_and_send_keys("downstream_step") + # Insert head tool + self.tool_open("head") + self.workflow_editor_click_option("Auto Layout") + self.sleep_for(self.wait_types.UX_RENDER) + editor.label_input.wait_for_and_send_keys("conditional_step") + # Connect head to cat + self.workflow_editor_connect("conditional_step#out_file1", "downstream_step#input1") + self.assert_connected("conditional_step#out_file1", "downstream_step#input1") + # Make head tool conditional + conditional_node = editor.node._(label="conditional_step") + conditional_node.input_terminal(name="input").wait_for_present() + # Assert no when input before making step conditional + conditional_node.input_terminal(name="when").wait_for_absent() + conditional_toggle = editor.step_when.wait_for_present() + self.action_chains().move_to_element(conditional_toggle).click().perform() + # Toggling conditional should cause when input to appear + conditional_node.input_terminal(name="when").wait_for_present() + self.action_chains().move_to_element(conditional_toggle).click().perform() + # Toggling conditional should cause when input to disappear + conditional_node.input_terminal(name="when").wait_for_absent() + self.action_chains().move_to_element(conditional_toggle).click().perform() + conditional_node.input_terminal(name="when").wait_for_present() + # Output connection should be invalid, as output from conditional step is potentially null + self.assert_connection_invalid("conditional_step#out_file1", "downstream_step#input1") + downstream_step = editor.node._(label="downstream_step") + downstream_step.destroy.wait_for_and_click() + downstream_step.wait_for_absent() + # Connect boolean input to when + self.workflow_editor_connect("param_input#output", "conditional_step#when") + self.assert_connected("param_input#output", "conditional_step#when") + # Change boolean input parameter to invalid parameter type + editor.node._(label="param_input").wait_for_and_click() + param_type_element = editor.param_type_form.wait_for_present() + self.switch_param_type(param_type_element, "Text") + self.assert_connection_invalid("param_input#output", "conditional_step#when") + self.workflow_editor_destroy_connection("conditional_step#when") + # Make sure the when input is still shown + conditional_node.input_terminal(name="when").wait_for_present() + # Assert save button is disabled because of disconnected when + save_button = self.components.workflow_editor.save_button + save_button.wait_for_visible() + # TODO: hook up best practice panel, disable save when "when" not connected + # assert save_button.has_class("disabled") + + def switch_param_type(self, element, param_type): + self.action_chains().move_to_element(element).click().send_keys(param_type).send_keys(Keys.ENTER).perform() + @selenium_test def test_editor_invalid_tool_state(self): workflow_populator = self.workflow_populator diff --git a/test/unit/tool_util/test_loader_directory.py b/test/unit/tool_util/test_loader_directory.py new file mode 100644 index 00000000000..08ec1f5a51c --- /dev/null +++ b/test/unit/tool_util/test_loader_directory.py @@ -0,0 +1,24 @@ +import tempfile + +from galaxy.tool_util.loader_directory import is_a_yaml_with_class + + +def test_is_a_yaml_with_class(): + with tempfile.NamedTemporaryFile("w", suffix=".yaml") as tf: + fname = tf.name + tf.write( + """class: GalaxyWorkflow +name: "Test Workflow" +inputs: + - id: input1 +outputs: + - id: wf_output_1 + outputSource: first_cat/out_file1 +steps: + - tool_id: cat + label: first_cat + in: + input1: input1""" + ) + tf.flush() + assert is_a_yaml_with_class(fname, ["GalaxyWorkflow"]) diff --git a/test/unit/tool_util/test_tool_linters.py b/test/unit/tool_util/test_tool_linters.py index 4c79f580d1e..4f420823519 100644 --- a/test/unit/tool_util/test_tool_linters.py +++ b/test/unit/tool_util/test_tool_linters.py @@ -192,6 +192,43 @@ INPUTS_DATA_PARAM = """ """ +INPUTS_DATA_PARAM_OPTIONS = """ + + + + + + + + + +""" + +INPUTS_DATA_PARAM_OPTIONS_FILTER_ATTRIBUTE = """ + + + + + + + + + +""" + +INPUTS_DATA_PARAM_INVALIDOPTIONS = """ + + + + + + + + + + +""" + INPUTS_CONDITIONAL = """ @@ -1099,6 +1136,42 @@ def test_inputs_data_param(lint_ctx): assert not lint_ctx.error_messages +def test_inputs_data_param_options(lint_ctx): + tool_source = get_xml_tool_source(INPUTS_DATA_PARAM_OPTIONS) + run_lint(lint_ctx, inputs.lint_inputs, tool_source) + assert not lint_ctx.valid_messages + assert "Found 1 input parameters." in lint_ctx.info_messages + assert len(lint_ctx.info_messages) == 1 + assert not lint_ctx.warn_messages + assert not lint_ctx.error_messages + + +def test_inputs_data_param_options_filter_attribute(lint_ctx): + tool_source = get_xml_tool_source(INPUTS_DATA_PARAM_OPTIONS_FILTER_ATTRIBUTE) + run_lint(lint_ctx, inputs.lint_inputs, tool_source) + assert not lint_ctx.valid_messages + assert "Found 1 input parameters." in lint_ctx.info_messages + assert len(lint_ctx.info_messages) == 1 + assert not lint_ctx.warn_messages + assert not lint_ctx.error_messages + + +def test_inputs_data_param_invalid_options(lint_ctx): + tool_source = get_xml_tool_source(INPUTS_DATA_PARAM_INVALIDOPTIONS) + run_lint(lint_ctx, inputs.lint_inputs, tool_source) + assert not lint_ctx.valid_messages + assert "Found 1 input parameters." in lint_ctx.info_messages + assert len(lint_ctx.info_messages) == 1 + assert not lint_ctx.warn_messages + assert "Data parameter [valid_name] contains multiple options elements." in lint_ctx.error_messages + assert "Data parameter [valid_name] filter needs to define a ref attribute" in lint_ctx.error_messages + assert ( + 'Data parameter [valid_name] for filters only type="data_meta" and key="dbkey" are allowed, found type="expression" and key="None"' + in lint_ctx.error_messages + ) + assert len(lint_ctx.error_messages) == 3 + + def test_inputs_conditional(lint_ctx): tool_source = get_xml_tool_source(INPUTS_CONDITIONAL) run_lint(lint_ctx, inputs.lint_inputs, tool_source) @@ -1284,7 +1357,7 @@ def test_inputs_type_child_combinations(lint_ctx): assert not lint_ctx.valid_messages assert not lint_ctx.warn_messages assert ( - "Parameter [text_param] './options' tags are only allowed for parameters of type ['select', 'drill_down']" + "Parameter [text_param] './options' tags are only allowed for parameters of type ['data', 'select', 'drill_down']" in lint_ctx.error_messages ) assert (