From 5dbaf5b65cc43e157201b68980c8f024ba79f467 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 30 Jul 2024 17:31:35 +0200 Subject: [PATCH] Fix collection type drag and drop validation The legacy form allowed users to drag non-collection datasets into collection inputs. The selected value wouldn't actually visibly change, but if a user hits run the value would actually be submitted, leading to https://sentry.galaxyproject.org/share/issue/2d1ada39ae9e4ba4952815ac07b2df47/: ``` AttributeError: 'HistoryDatasetAssociation' object has no attribute 'collection' File "galaxy/tools/__init__.py", line 1969, in handle_single_execution rval = self.execute( File "galaxy/tools/__init__.py", line 2066, in execute return self.tool_action.execute( File "galaxy/tools/actions/__init__.py", line 426, in execute ) = self._collect_inputs(tool, trans, incoming, history, current_user_roles, collection_info) File "galaxy/tools/actions/__init__.py", line 360, in _collect_inputs inp_data, all_permissions = self._collect_input_datasets( File "galaxy/tools/actions/__init__.py", line 300, in _collect_input_datasets tool.visit_inputs(param_values, visitor) File "galaxy/tools/__init__.py", line 1793, in visit_inputs visit_input_values(self.inputs, values, callback) File "galaxy/tools/parameters/__init__.py", line 211, in visit_input_values visit_input_values( File "galaxy/tools/parameters/__init__.py", line 227, in visit_input_values callback_helper( File "galaxy/tools/parameters/__init__.py", line 162, in callback_helper new_value = callback(**args) File "galaxy/tools/actions/__init__.py", line 246, in visitor collection = value.collection ``` --- .../Form/Elements/FormData/FormData.test.js | 48 +++++++++++++ .../Form/Elements/FormData/FormData.vue | 71 ++++++++++++++++--- 2 files changed, 111 insertions(+), 8 deletions(-) diff --git a/client/src/components/Form/Elements/FormData/FormData.test.js b/client/src/components/Form/Elements/FormData/FormData.test.js index bcff5d7e5589..709f2910a791 100644 --- a/client/src/components/Form/Elements/FormData/FormData.test.js +++ b/client/src/components/Form/Elements/FormData/FormData.test.js @@ -372,6 +372,54 @@ describe("FormData", () => { }); }); + it("rejects hda on collection input", async () => { + const wrapper = createTarget({ + value: null, + options: defaultOptions, + type: "data_collection", + }); + eventStore.setDragData({ id: "whatever", history_content_type: "dataset" }); + dispatchEvent(wrapper, "dragenter"); + dispatchEvent(wrapper, "drop"); + expect(wrapper.emitted().alert[0][0]).toEqual("dataset is not a valid input for dataset collection parameter."); + }); + + it("rejects paired collection on list collection input", async () => { + const wrapper = createTarget({ + value: null, + options: defaultOptions, + type: "data_collection", + collectionTypes: ["list"], + }); + eventStore.setDragData({ + id: "whatever", + history_content_type: "dataset_collection", + collection_type: "paired", + }); + dispatchEvent(wrapper, "dragenter"); + dispatchEvent(wrapper, "drop"); + expect(wrapper.emitted().alert[0][0]).toEqual( + "paired dataset collection is not a valid input for list type dataset collection parameter." + ); + }); + + it("accepts list:list collection on list collection input", async () => { + const wrapper = createTarget({ + value: null, + options: defaultOptions, + type: "data_collection", + collectionTypes: ["list"], + }); + eventStore.setDragData({ + id: "whatever", + history_content_type: "dataset_collection", + collection_type: "list:list", + }); + dispatchEvent(wrapper, "dragenter"); + dispatchEvent(wrapper, "drop"); + expect(wrapper.emitted().alert[0][0]).toEqual(undefined); + }); + it("linked and unlinked batch mode handling", async () => { const wrapper = createTarget({ value: null, diff --git a/client/src/components/Form/Elements/FormData/FormData.vue b/client/src/components/Form/Elements/FormData/FormData.vue index b5f71c8cb2d1..f6054f28cd00 100644 --- a/client/src/components/Form/Elements/FormData/FormData.vue +++ b/client/src/components/Form/Elements/FormData/FormData.vue @@ -35,7 +35,7 @@ const props = withDefaults( values: Array; }; extensions?: Array; - type?: string; + type?: "data" | "data_collection"; collectionTypes?: Array; flavor?: string; tag?: string; @@ -310,6 +310,9 @@ function handleIncoming(incoming: Record, partial = true) { if (!canAcceptDatatype(extensions)) { return false; } + if (values.some((v) => !canAcceptSrc(v.history_content_type, v.collection_type))) { + return false; + } if (values.length > 0) { const incomingValues: Array = []; values.forEach((v) => { @@ -414,17 +417,67 @@ function canAcceptDatatype(itemDatatypes: string | Array) { return true; } +function canAcceptSrc(historyContentType: "dataset" | "dataset_collection", collectionType?: string) { + if (historyContentType === "dataset") { + // HDA can only be fed into data parameters, not collection parameters + if (props.type === "data") { + return true; + } else { + $emit("alert", "dataset is not a valid input for dataset collection parameter."); + return false; + } + } else if (historyContentType === "dataset_collection") { + if (props.type === "data") { + // collection can always be mapped over a data input ... in theory. + // One day we should also validate the map over model + return true; + } + if (!collectionType) { + // Should always be set if item is dataset collection + throw Error("Item is a dataset collection of unknown type."); + } else if (!props.collectionTypes) { + // if no collection_type is set all collections are valid + return true; + } else { + if (props.collectionTypes.includes(collectionType)) { + return true; + } + if (props.collectionTypes.some((element) => collectionType.endsWith(element))) { + return true; + } else { + $emit( + "alert", + `${collectionType} dataset collection is not a valid input for ${orList( + props.collectionTypes + )} type dataset collection parameter.` + ); + return false; + } + } + } else { + throw Error("Unknown history content type."); + } +} + // 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); + let highlightingState = "success"; if (!canAcceptDatatype(extensions)) { - currentHighlighting.value = "warning"; + highlightingState = "warning"; $emit("alert", `${extensions} is not an acceptable format for this parameter.`); - } else { - currentHighlighting.value = "success"; + } else if ( + !canAcceptSrc( + eventData.history_content_type as "dataset" | "dataset_collection", + eventData.collection_type as string + ) + ) { + highlightingState = "warning"; + $emit("alert", `${eventData.history_content_type} is not an acceptable input type for this parameter.`); } + currentHighlighting.value = highlightingState; dragTarget.value = evt.target; dragData.value = eventData; } @@ -501,12 +554,14 @@ const formatsButtonId = useUid("form-data-formats-"); const warningListAmount = 4; const noOptionsWarningMessage = computed(() => { - if (!props.extensions || props.extensions.length === 0) { - return "No datasets available"; + const itemType = props.type === "data" ? "datasets" : "dataset collections"; + const collectionTypeLabel = props.collectionTypes?.length ? `${orList(props.collectionTypes)} ` : ""; + if (!props.extensions || props.extensions.length === 0 || props.extensions.includes("data")) { + return `No ${collectionTypeLabel}${itemType} available`; } else if (props.extensions.length <= warningListAmount) { - return `No ${orList(props.extensions)} datasets available`; + return `No ${collectionTypeLabel}${itemType} with ${orList(props.extensions)} elements available`; } else { - return "No compatible datasets available"; + return `No compatible ${collectionTypeLabel}${itemType} available`; } });