Merge pull request galaxyproject#19866 from ahmedhamidawan/fix_dce_drag_drop

[24.2] Fix drag and drop for dataset collection elements

Author: mvdbeek

client/src/api/index.ts

@@ -194,7 +194,7 @@ export type CollectionEntry = HDCASummary | SubCollection;
 /**
  * Returns true if the given entry is a top level HDCA and false for sub-collections.
  */
-export function isHDCA(entry?: CollectionEntry): entry is HDCASummary {
+export function isHDCA(entry?: HistoryItemSummary | CollectionEntry): entry is HDCASummary {
     return (
         entry !== undefined && "history_content_type" in entry && entry.history_content_type === "dataset_collection"
     );

client/src/components/Form/Elements/FormData/FormData.vue

@@ -13,7 +13,16 @@ import { BAlert, BButton, BButtonGroup, BCollapse, BFormCheckbox, BTooltip } fro
 import { storeToRefs } from "pinia";
 import { computed, onMounted, type Ref, ref, watch } from "vue";
 
-import { isDatasetElement, isDCE } from "@/api";
+import {
+    type DCESummary,
+    type HDAObject,
+    type HistoryItemSummary,
+    isDatasetElement,
+    isDCE,
+    isHDCA,
+    isHistoryItem,
+} from "@/api";
+import type { HistoryContentType } from "@/api/datasets";
 import { getGalaxyInstance } from "@/app";
 import { useDatatypesMapper } from "@/composables/datatypesMapper";
 import { useUid } from "@/composables/utils/uid";
@@ -40,6 +49,8 @@ type SelectOption = {
     value: DataOption | null;
 };
 
+type HistoryOrCollectionItem = HistoryItemSummary | DCESummary;
+
 const props = withDefaults(
     defineProps<{
         loading?: boolean;
@@ -83,7 +94,7 @@ const currentField = ref(0);
 const currentHighlighting: Ref<string | null> = ref(null);
 
 // Drag/Drop related values
-const dragData: Ref<EventData | null> = ref(null);
+const dragData: Ref<EventData[]> = ref([]);
 const dragTarget: Ref<EventTarget | null> = ref(null);
 
 // Collection creator modal settings
@@ -331,35 +342,50 @@ function getSourceType(val: DataOption) {
 }
 
 /** Add values from drag/drop or data dialog sources */
-function handleIncoming(incoming: Record<string, unknown>, partial = true) {
+function handleIncoming(incoming: Record<string, unknown> | Record<string, unknown>[], 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));
+
+        // ensure all incoming values are isHistoryOrCollectionItem
+        if (!values.every(isHistoryOrCollectionItem)) {
+            return false;
+        }
+
+        const extensions = Array.from(
+            new Set(
+                values
+                    .map(getExtensionsForItem)
+                    .flat()
+                    .filter((v) => v !== null && v !== undefined)
+            )
+        ) as string[];
+
         if (!canAcceptDatatype(extensions)) {
             return false;
         }
-        if (values.some((v) => !canAcceptSrc(v.history_content_type, v.collection_type))) {
+        if (
+            values.some((v) => {
+                const { historyContentType } = getSrcAndContentType(v);
+                const collectionType = "collection_type" in v && v.collection_type ? v.collection_type : undefined;
+                return !canAcceptSrc(historyContentType, collectionType);
+            })
+        ) {
             return false;
         }
         if (values.length > 0) {
             const incomingValues: Array<DataOption> = [];
-            values.forEach((v) => {
+            values.forEach((currVal) => {
                 // Map incoming objects to data option values
-                let newSrc;
-                if (isDCE(v)) {
-                    if (isDatasetElement(v)) {
-                        newSrc = SOURCE.DATASET;
-                        v = v.object;
-                    } else {
-                        newSrc = SOURCE.COLLECTION_ELEMENT;
-                    }
+                const { newSrc, datasetCollectionDataset } = getSrcAndContentType(currVal);
+                let v: HistoryOrCollectionItem | HDAObject;
+                if (datasetCollectionDataset) {
+                    v = datasetCollectionDataset;
                 } else {
-                    newSrc =
-                        v.src || (v.history_content_type === "dataset_collection" ? SOURCE.COLLECTION : SOURCE.DATASET);
+                    v = currVal;
                 }
-                const newHid = v.hid;
+                const newHid = isHistoryItem(v) ? v.hid : undefined;
                 const newId = v.id;
-                const newName = v.name ? v.name : newId;
+                const newName = isHistoryItem(v) && v.name ? v.name : newId;
                 const newValue: DataOption = {
                     id: newId,
                     src: newSrc,
@@ -370,10 +396,11 @@ function handleIncoming(incoming: Record<string, unknown>, partial = true) {
                     keep: true,
                     tags: [],
                 };
-                if (v.collection_type && props.collectionTypes?.length > 0) {
-                    if (!props.collectionTypes.includes(v.collection_type)) {
+                if (isHistoryItem(v) && isHDCA(v) && props.collectionTypes?.length > 0) {
+                    const itemCollectionType = v.collection_type;
+                    if (!props.collectionTypes.includes(itemCollectionType)) {
                         const mapOverType = props.collectionTypes.find((collectionType) =>
-                            v.collection_type.endsWith(collectionType)
+                            itemCollectionType.endsWith(collectionType)
                         );
                         if (!mapOverType) {
                             return false;
@@ -437,6 +464,9 @@ function onBrowse() {
 }
 
 function canAcceptDatatype(itemDatatypes: string | Array<string>) {
+    // TODO: Shouldn't we enforce a datatype (at least "data") because of the case:
+    // What if the drop item is a `DCESummary`, then it has no extension (?) and we
+    // pass it as a valid item regardless of its elements' datatypes.
     if (!(props.extensions?.length > 0) || props.extensions.includes("data")) {
         return true;
     }
@@ -455,6 +485,39 @@ function canAcceptDatatype(itemDatatypes: string | Array<string>) {
     return true;
 }
 
+/**
+ * Given an element, determine the source and content type.
+ * Also returns the collection element dataset object if it exists.
+ */
+function getSrcAndContentType(element: HistoryOrCollectionItem): {
+    historyContentType: HistoryContentType;
+    newSrc: string;
+    datasetCollectionDataset: HDAObject | undefined;
+} {
+    let historyContentType: HistoryContentType;
+    let newSrc: string;
+    let datasetCollectionDataset: HDAObject | undefined;
+    if (isDCE(element)) {
+        if (isDatasetElement(element)) {
+            historyContentType = "dataset";
+            newSrc = SOURCE.DATASET;
+            datasetCollectionDataset = element.object;
+        } else {
+            historyContentType = "dataset_collection";
+            newSrc = SOURCE.COLLECTION_ELEMENT;
+        }
+    } else {
+        historyContentType = element.history_content_type;
+        newSrc =
+            "src" in element && typeof element.src === "string"
+                ? element.src
+                : historyContentType === "dataset_collection"
+                ? SOURCE.COLLECTION
+                : SOURCE.DATASET;
+    }
+    return { historyContentType, newSrc, datasetCollectionDataset };
+}
+
 function canAcceptSrc(historyContentType: "dataset" | "dataset_collection", collectionType?: string) {
     if (historyContentType === "dataset") {
         // HDA can only be fed into data parameters, not collection parameters
@@ -514,27 +577,39 @@ function createdCollection(collection: any) {
     handleIncoming(collection);
 }
 
+/**
+ * Get the extension(s) for a given item
+ */
+function getExtensionsForItem(item: HistoryOrCollectionItem): string | string[] | null {
+    return "extension" in item ? item.extension : "elements_datatypes" in item ? item.elements_datatypes : null;
+}
+
+function isHistoryOrCollectionItem(item: EventData): item is HistoryOrCollectionItem {
+    return isHistoryItem(item) || isDCE(item);
+}
+
 // 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<string>);
-        let highlightingState = "success";
-        if (!canAcceptDatatype(extensions)) {
-            highlightingState = "warning";
-            $emit("alert", `${extensions} is not an acceptable format for this parameter.`);
-        } 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.`);
+    const eventData = eventStore.getDragItems();
+    dragData.value = [];
+
+    for (const item of eventData) {
+        if (isHistoryOrCollectionItem(item)) {
+            const extensions = getExtensionsForItem(item);
+            const { historyContentType } = getSrcAndContentType(item);
+            const collectionType = "collection_type" in item && item.collection_type ? item.collection_type : undefined;
+            let highlightingState = "success";
+            if (extensions && !canAcceptDatatype(extensions)) {
+                highlightingState = "warning";
+                $emit("alert", `${extensions} is not an acceptable format for this parameter.`);
+            } else if (!canAcceptSrc(historyContentType, collectionType)) {
+                highlightingState = "warning";
+                $emit("alert", `${historyContentType} is not an acceptable input type for this parameter.`);
+            }
+            currentHighlighting.value = highlightingState;
+            dragTarget.value = evt.target;
+            dragData.value.push(item);
         }
-        currentHighlighting.value = highlightingState;
-        dragTarget.value = evt.target;
-        dragData.value = eventData;
     }
 }
 
@@ -546,20 +621,14 @@ function onDragLeave(evt: MouseEvent) {
 }
 
 function onDrop() {
-    if (dragData.value) {
-        let accept = false;
-        if (eventStore.multipleDragData) {
-            accept = handleIncoming(Object.values(dragData.value) as any, false);
-        } else {
-            accept = handleIncoming(dragData.value);
-        }
-        if (accept) {
+    if (dragData.value.length) {
+        if (handleIncoming(dragData.value, dragData.value.length === 1)) {
             currentHighlighting.value = "success";
         } else {
             currentHighlighting.value = "warning";
         }
         $emit("alert", undefined);
-        dragData.value = null;
+        dragData.value = [];
         clearHighlighting();
     }
 }

client/src/components/Form/Elements/FormData/types.ts

@@ -1,6 +1,6 @@
 export type DataOption = {
     id: string;
-    hid: number;
+    hid?: number;
     is_dataset?: boolean;
     keep: boolean;
     batch: boolean;