From 486f41349d384f8f66defb8e2aaf6ae8f8a80d89 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Sun, 16 Jun 2024 19:30:05 +0200 Subject: [PATCH] Ensure datatypes match on dragging elements into FormData This is pretty common source of cheetah templating issues where tool authors assume (orrectly, IMO) only datatypes allowed in the format attribute of an input dataset are allowed. Here's a hypothetical example: ``` # set index = $input.cram_index # set index = $input.cram_index ln -s '$index' input.index ``` This is going to fail if a user drag e.g. a txt dataset into the input. For anything but the valid datatypes this is going to fail. Another way things can fail is if an input datatypes should contain extra files, but doesn't, for instance in https://sentry.galaxyproject.org/share/issue/73d7bd51dd2d418e96d563f0ac2383f0/: ``` Exception: __action_for_transfer called on non-existent file - [None] File "galaxy/jobs/runners/pulsar.py", line 446, in queue_job external_job_id = pulsar_submit_job(client, client_job_description, remote_job_config) File "pulsar/client/staging/up.py", line 39, in submit_job file_stager = FileStager(client, client_job_description, job_config) File "pulsar/client/staging/up.py", line 148, in __init__ self.__upload_input_files() File "pulsar/client/staging/up.py", line 264, in __upload_input_files self.__upload_input_metadata_file(client_input.action_source) File "pulsar/client/staging/up.py", line 287, in __upload_input_metadata_file self.transfer_tracker.handle_transfer_source(input_action_source, path_type.INPUT, name=remote_name) File "pulsar/client/staging/up.py", line 495, in handle_transfer_source action = self.__action_for_transfer(source, type, contents) File "pulsar/client/staging/up.py", line 551, in __action_for_transfer raise Exception(message) ``` Fixes https://github.com/galaxyproject/galaxy/issues/8202 We can now easily change datatypes in batch if necessary, this shouldn't be an argument to not properly implement datatype filtering. --- .../Form/Elements/FormData/FormData.test.js | 4 ++ .../Form/Elements/FormData/FormData.vue | 56 +++++++++++++++---- .../src/components/Form/FormElement.test.js | 2 +- client/src/components/Form/FormElement.vue | 21 +++++-- 4 files changed, 65 insertions(+), 18 deletions(-) diff --git a/client/src/components/Form/Elements/FormData/FormData.test.js b/client/src/components/Form/Elements/FormData/FormData.test.js index 65b7c9dd6100..bcff5d7e5589 100644 --- a/client/src/components/Form/Elements/FormData/FormData.test.js +++ b/client/src/components/Form/Elements/FormData/FormData.test.js @@ -3,6 +3,8 @@ import { mount } from "@vue/test-utils"; import { PiniaVuePlugin } from "pinia"; import { dispatchEvent, getLocalVue } from "tests/jest/helpers"; +import { testDatatypesMapper } from "@/components/Datatypes/test_fixtures"; +import { useDatatypesMapperStore } from "@/stores/datatypesMapperStore"; import { useEventStore } from "@/stores/eventStore"; import MountTarget from "./FormData.vue"; @@ -15,6 +17,8 @@ let eventStore; function createTarget(propsData) { const pinia = createTestingPinia({ stubActions: false }); eventStore = useEventStore(); + const datatypesStore = useDatatypesMapperStore(); + datatypesStore.datatypesMapper = testDatatypesMapper; return mount(MountTarget, { localVue, propsData, diff --git a/client/src/components/Form/Elements/FormData/FormData.vue b/client/src/components/Form/Elements/FormData/FormData.vue index 123d24bb43b5..8bb48e50638d 100644 --- a/client/src/components/Form/Elements/FormData/FormData.vue +++ b/client/src/components/Form/Elements/FormData/FormData.vue @@ -7,6 +7,7 @@ import { BAlert, BButton, BButtonGroup, BCollapse, BFormCheckbox, BTooltip } fro import { computed, onMounted, type Ref, ref, watch } from "vue"; import { getGalaxyInstance } from "@/app"; +import { useDatatypesMapper } from "@/composables/datatypesMapper"; import { useUid } from "@/composables/utils/uid"; import { type EventData, useEventStore } from "@/stores/eventStore"; import { orList } from "@/utils/strings"; @@ -51,8 +52,9 @@ const props = withDefaults( ); const eventStore = useEventStore(); +const { datatypesMapper } = useDatatypesMapper(); -const $emit = defineEmits(["input"]); +const $emit = defineEmits(["input", "alert"]); // Determines wether values should be processed as linked or unlinked const currentLinked = ref(true); @@ -302,6 +304,10 @@ function getSourceType(val: DataOption) { function handleIncoming(incoming: Record, partial = true) { if (incoming) { const values = Array.isArray(incoming) ? incoming : [incoming]; + const extensions = values.map((v) => v.extension || v.elements_datatypes).filter((v) => (v ? true : false)); + if (!canAcceptDatatype(extensions)) { + return false; + } if (values.length > 0) { const incomingValues: Array = []; values.forEach((v) => { @@ -349,6 +355,7 @@ function handleIncoming(incoming: Record, partial = true) { } } } + return true; } /** @@ -372,10 +379,36 @@ function onBrowse() { } } +function canAcceptDatatype(itemDatatypes: string | Array) { + if (!(props.extensions?.length > 0)) { + return true; + } + let datatypes: Array; + if (!Array.isArray(itemDatatypes)) { + datatypes = [itemDatatypes]; + } else { + datatypes = itemDatatypes; + } + const incompatibleItem = datatypes.find( + (extension) => !datatypesMapper.value?.isSubTypeOfAny(extension, props.extensions) + ); + if (incompatibleItem) { + return false; + } + return true; +} + // Drag/Drop event handlers function onDragEnter(evt: MouseEvent) { const eventData = eventStore.getDragData(); if (eventData) { + const extensions = (eventData.extension as string) || (eventData.elements_datatypes as Array); + if (!canAcceptDatatype(extensions)) { + currentHighlighting.value = "warning"; + $emit("alert", `${extensions} is not an acceptable format for this parameter.`); + } else { + currentHighlighting.value = "success"; + } dragTarget.value = evt.target; dragData.value = eventData; } @@ -384,23 +417,24 @@ function onDragEnter(evt: MouseEvent) { function onDragLeave(evt: MouseEvent) { if (dragTarget.value === evt.target) { currentHighlighting.value = null; - } -} - -function onDragOver() { - if (dragData.value !== null) { - currentHighlighting.value = "warning"; + $emit("alert", undefined); } } function onDrop() { if (dragData.value) { + let accept = false; if (eventStore.multipleDragData) { - handleIncoming(Object.values(dragData.value) as any, false); + accept = handleIncoming(Object.values(dragData.value) as any, false); + } else { + accept = handleIncoming(dragData.value); + } + if (accept) { + currentHighlighting.value = "success"; } else { - handleIncoming(dragData.value); + currentHighlighting.value = "warning"; } - currentHighlighting.value = "success"; + $emit("alert", undefined); dragData.value = null; clearHighlighting(); } @@ -468,7 +502,7 @@ const noOptionsWarningMessage = computed(() => { :class="currentHighlighting && `ui-dragover-${currentHighlighting}`" @dragenter.prevent="onDragEnter" @dragleave.prevent="onDragLeave" - @dragover.prevent="onDragOver" + @dragover.prevent @drop.prevent="onDrop">
diff --git a/client/src/components/Form/FormElement.test.js b/client/src/components/Form/FormElement.test.js index 9aac1c55c0f6..a3cd69f50f74 100644 --- a/client/src/components/Form/FormElement.test.js +++ b/client/src/components/Form/FormElement.test.js @@ -30,7 +30,7 @@ describe("FormElement", () => { const error = wrapper.find(".ui-form-error-text"); expect(error.text()).toBe("error_text"); - await wrapper.setProps({ error: "" }); + await wrapper.setProps({ error: undefined }); const no_error = wrapper.findAll(".ui-form-error"); expect(no_error.length).toBe(0); diff --git a/client/src/components/Form/FormElement.vue b/client/src/components/Form/FormElement.vue index 34baae64c846..50bdeb5aa923 100644 --- a/client/src/components/Form/FormElement.vue +++ b/client/src/components/Form/FormElement.vue @@ -127,7 +127,7 @@ function onConnect() { const isHidden = computed(() => attrs.value["hidden"]); const elementId = computed(() => `form-element-${props.id}`); -const hasAlert = computed(() => Boolean(props.error || props.warning)); +const hasAlert = computed(() => alerts.value.length > 0); const showPreview = computed(() => (collapsed.value && attrs.value["collapsible_preview"]) || props.disabled); const showField = computed(() => !collapsed.value && !props.disabled); @@ -174,6 +174,16 @@ const isEmpty = computed(() => { const isRequired = computed(() => attrs.value["optional"] === false); const isRequiredType = computed(() => props.type !== "boolean"); const isOptional = computed(() => !isRequired.value && attrs.value["optional"] !== undefined); +const formAlert = ref(); +const alerts = computed(() => { + return [formAlert.value, props.error, props.warning] + .filter((v) => v !== undefined && v !== null) + .map((v) => linkify(sanitize(v!, { USE_PROFILES: { html: true } }))); +}); + +function onAlert(value: string | undefined) { + formAlert.value = value; +}