Rename map_over_type to subcollection_type for consistency, add test …

…to verify that mapping mode is correctly assigned
galaxyproject · Aug 30, 2023 · 71a9391 · 71a9391
1 parent d58a166
commit 71a9391
Show file tree

Hide file tree

Showing 5 changed files with 67 additions and 39 deletions.
diff --git a/client/src/components/Form/Elements/FormData/FormData.test.js b/client/src/components/Form/Elements/FormData/FormData.test.js
@@ -29,7 +29,8 @@ const defaultOptions = {
     dce: [
         { id: "dce1", name: "dceName1", src: "dce", is_dataset: true },
         { id: "dce2", name: "dceName2", src: "dce" },
-        { id: "dce3", name: "dceName3", src: "dce", map_over_type: "mapOverType" },
+        { id: "dce3", name: "dceName3", src: "dce", subcollection_type: "subcollectionType" },
+        { id: "dce4", name: "dceName4", src: "dce", is_dataset: true },
     ],
     hda: [
         { id: "hda1", hid: 1, name: "hdaName1", src: "hda" },
@@ -54,12 +55,12 @@ describe("FormData", () => {
         const value_0 = {
             batch: false,
             product: false,
-            values: [{ id: "hda3", src: "hda", map_over_type: null }],
+            values: [{ id: "hda3", src: "hda", subcollection_type: null }],
         };
         const value_1 = {
             batch: false,
             product: false,
-            values: [{ id: "hda1", src: "hda", map_over_type: null }],
+            values: [{ id: "hda1", src: "hda", subcollection_type: null }],
         };
         const options = wrapper.find(".btn-group").findAll("button");
         expect(options.length).toBe(4);
@@ -113,8 +114,8 @@ describe("FormData", () => {
             batch: false,
             product: false,
             values: [
-                { id: "hda2", map_over_type: null, src: "hda" },
-                { id: "hda3", map_over_type: null, src: "hda" },
+                { id: "hda2", subcollection_type: null, src: "hda" },
+                { id: "hda3", subcollection_type: null, src: "hda" },
             ],
         });
         expect(wrapper.emitted().input.length).toEqual(1);
@@ -126,17 +127,25 @@ describe("FormData", () => {
             batch: false,
             product: false,
             values: [
-                { id: "hda2", map_over_type: null, src: "hda" },
-                { id: "hda3", map_over_type: null, src: "hda" },
+                { id: "hda2", subcollection_type: null, src: "hda" },
+                { id: "hda3", subcollection_type: null, src: "hda" },
             ],
         };
         expect(wrapper.emitted().input[0][0]).toEqual(value_0);
         await selectedValues.at(0).trigger("click");
-        const value_1 = { batch: false, product: false, values: [{ id: "hda2", map_over_type: null, src: "hda" }] };
+        const value_1 = {
+            batch: false,
+            product: false,
+            values: [{ id: "hda2", subcollection_type: null, src: "hda" }],
+        };
         expect(wrapper.emitted().input[1][0]).toEqual(value_1);
         await wrapper.setProps({ value: value_1 });
         await selectedValues.at(1).trigger("click");
-        const value_2 = { batch: false, product: false, values: [{ id: "hda2", map_over_type: null, src: "hda" }] };
+        const value_2 = {
+            batch: false,
+            product: false,
+            values: [{ id: "hda2", subcollection_type: null, src: "hda" }],
+        };
         expect(wrapper.emitted().input[1][0]).toEqual(value_2);
         await wrapper.setProps({ value: value_2 });
         expect(wrapper.emitted().input.length).toBe(3);
@@ -148,7 +157,11 @@ describe("FormData", () => {
             value: { values: [{ id: "dce1", src: "dce" }] },
             options: defaultOptions,
         });
-        const value_0 = { batch: false, product: false, values: [{ id: "dce1", map_over_type: null, src: "dce" }] };
+        const value_0 = {
+            batch: false,
+            product: false,
+            values: [{ id: "dce1", subcollection_type: null, src: "dce" }],
+        };
         expect(wrapper.emitted().input[0][0]).toEqual(value_0);
         expect(wrapper.emitted().input.length).toEqual(1);
         const message = wrapper.findAll(".form-data-entry-label");
@@ -159,34 +172,48 @@ describe("FormData", () => {
         expect(wrapper.emitted().input[1][0]).toEqual(null);
     });
 
-    it("dataset collection as hdca without map_over_type", async () => {
+    it("dataset collection as hdca without subcollection_type", async () => {
         const wrapper = createTarget({
             value: { values: [{ id: "dce2", src: "dce" }] },
             options: defaultOptions,
         });
-        const value_0 = { batch: true, product: false, values: [{ id: "dce2", map_over_type: null, src: "dce" }] };
+        const value_0 = { batch: true, product: false, values: [{ id: "dce2", subcollection_type: null, src: "dce" }] };
         expect(wrapper.emitted().input[0][0]).toEqual(value_0);
     });
 
-    it("dataset collection as hdca with map_over_type", async () => {
+    it("dataset collection as hdca mapped to batch field", async () => {
         const wrapper = createTarget({
             value: { values: [{ id: "dce3", src: "dce" }] },
             options: defaultOptions,
         });
         const value_0 = {
             batch: true,
             product: false,
-            values: [{ id: "dce3", map_over_type: "mapOverType", src: "dce" }],
+            values: [{ id: "dce3", subcollection_type: "subcollectionType", src: "dce" }],
+        };
+        expect(wrapper.emitted().input[0][0]).toEqual(value_0);
+    });
+
+    it("dataset collection as hdca mapped to non-batch field", async () => {
+        const wrapper = createTarget({
+            type: "data_collection",
+            value: { values: [{ id: "dce3", src: "dce" }] },
+            options: defaultOptions,
+        });
+        const value_0 = {
+            batch: false,
+            product: false,
+            values: [{ id: "dce3", subcollection_type: "subcollectionType", src: "dce" }],
         };
         expect(wrapper.emitted().input[0][0]).toEqual(value_0);
     });
 
-    it("multiple dataset collection values with varying map_over_type", async () => {
+    it("multiple dataset collection elements (as hdas)", async () => {
         const wrapper = createTarget({
             value: {
                 values: [
-                    { id: "dce2", src: "dce" },
-                    { id: "dce3", src: "dce" },
+                    { id: "dce1", src: "dce" },
+                    { id: "dce4", src: "dce" },
                 ],
             },
             options: defaultOptions,
@@ -195,8 +222,8 @@ describe("FormData", () => {
             batch: true,
             product: false,
             values: [
-                { id: "dce2", map_over_type: null, src: "dce" },
-                { id: "dce3", map_over_type: "mapOverType", src: "dce" },
+                { id: "dce1", subcollection_type: null, src: "dce" },
+                { id: "dce4", subcollection_type: null, src: "dce" },
             ],
         };
         expect(wrapper.emitted().input[0][0]).toEqual(value_0);
@@ -213,15 +240,15 @@ describe("FormData", () => {
         expect(wrapper.emitted().input[1][0]).toEqual({
             batch: true,
             product: false,
-            values: [{ id: "hdca4", map_over_type: null, src: "hdca" }],
+            values: [{ id: "hdca4", subcollection_type: null, src: "hdca" }],
         });
         eventStore.setDragData({ id: "hda2", history_content_type: "dataset" });
         dispatchEvent(wrapper, "dragenter");
         dispatchEvent(wrapper, "drop");
         expect(wrapper.emitted().input[2][0]).toEqual({
             batch: false,
             product: false,
-            values: [{ id: "hda2", map_over_type: null, src: "hda" }],
+            values: [{ id: "hda2", subcollection_type: null, src: "hda" }],
         });
     });
 
@@ -234,15 +261,15 @@ describe("FormData", () => {
         expect(wrapper.emitted().input[0][0]).toEqual({
             batch: false,
             product: false,
-            values: [{ id: "hda3", map_over_type: null, src: "hda" }],
+            values: [{ id: "hda3", subcollection_type: null, src: "hda" }],
         });
         const noCheckLinked = wrapper.find("input[type='checkbox']");
         expect(noCheckLinked.exists()).toBeFalsy();
         await wrapper.find("[title='Multiple datasets'").trigger("click");
         expect(wrapper.emitted().input[1][0]).toEqual({
             batch: true,
             product: false,
-            values: [{ id: "hda3", map_over_type: null, src: "hda" }],
+            values: [{ id: "hda3", subcollection_type: null, src: "hda" }],
         });
         const checkLinked = wrapper.find("input[type='checkbox']");
         expect(wrapper.find(".custom-switch span").text()).toBe(
@@ -256,7 +283,7 @@ describe("FormData", () => {
         expect(wrapper.emitted().input[2][0]).toEqual({
             batch: true,
             product: true,
-            values: [{ id: "hda3", map_over_type: null, src: "hda" }],
+            values: [{ id: "hda3", subcollection_type: null, src: "hda" }],
         });
     });
 });
diff --git a/client/src/components/Form/Elements/FormData/FormData.vue b/client/src/components/Form/Elements/FormData/FormData.vue
@@ -384,7 +384,6 @@ function setValue(val: Array<DataOption> | DataOption | null) {
     if (val) {
         const values = Array.isArray(val) ? val : [val];
         if (variant.value && values.length > 0 && values[0]) {
-            const hasMapOverType = values.find((v) => !!v.map_over_type);
             const isMultiple = values.length > 1;
 
             // Determine source representation
@@ -404,25 +403,27 @@ function setValue(val: Array<DataOption> | DataOption | null) {
             let batch: string = BATCH.DISABLED;
             if (variantIndex >= 0) {
                 const variantDetails = variant.value[variantIndex];
-                if ((isLDDA.value || isDCE.value) && variantDetails && variantDetails.batch) {
-                    batch = variantDetails.batch;
-                } else {
-                    // Switch to another field type if source differs from current field
-                    if (currentVariant.value && currentVariant.value.src !== sourceType) {
-                        currentField.value = variantIndex;
+                if (variantDetails) {
+                    if ((isLDDA.value || isDCE.value) && variantDetails.batch) {
+                        batch = variantDetails.batch;
+                    } else {
+                        // Switch to another field type if source differs from current field
+                        if (currentVariant.value && currentVariant.value.src !== sourceType) {
+                            currentField.value = variantIndex;
+                        }
+                        batch = (currentVariant.value && currentVariant.value.batch) || BATCH.DISABLED;
                     }
-                    batch = (currentVariant.value && currentVariant.value.batch) || BATCH.DISABLED;
                 }
             }
 
             // Emit new value
             $emit("input", {
-                batch: batch !== BATCH.DISABLED || !!hasMapOverType,
+                batch: batch !== BATCH.DISABLED,
                 product: batch === BATCH.ENABLED && !currentLinked.value,
                 values: values.map((entry) => ({
                     id: entry.id,
                     src: entry.src,
-                    map_over_type: entry.map_over_type || null,
+                    subcollection_type: entry.subcollection_type || null,
                 })),
             });
         }

diff --git a/client/src/components/Form/Elements/FormData/types.ts b/client/src/components/Form/Elements/FormData/types.ts
@@ -3,8 +3,8 @@ export interface DataOption {
     is_dataset?: boolean;
     keep: boolean;
     hid: number;
-    map_over_type?: string;
     name: string;
     src: string;
+    subcollection_type?: string;
     tags: Array<string>;
 }
diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py
@@ -2299,7 +2299,7 @@ def append(list, hda, name, src, keep=False, subcollection_type=None):
                 "keep": keep,
             }
             if subcollection_type:
-                value["map_over_type"] = subcollection_type
+                value["subcollection_type"] = subcollection_type
             return list.append(value)
 
         def append_dce(dce):
@@ -2519,7 +2519,7 @@ def to_dict(self, trans, other_values=None):
         # append DCE
         if isinstance(other_values.get(self.name), DatasetCollectionElement):
             dce = other_values[self.name]
-            d["options"]["hdca"].append(
+            d["options"]["dce"].append(
                 {
                     "id": trans.security.encode_id(dce.id),
                     "hid": -1,
@@ -2557,7 +2557,7 @@ def to_dict(self, trans, other_values=None):
                     "name": name,
                     "src": "hdca",
                     "tags": [t.user_tname if not t.value else f"{t.user_tname}:{t.value}" for t in hdca.tags],
-                    "map_over_type": subcollection_type,
+                    "subcollection_type": subcollection_type,
                 }
             )
 

diff --git a/lib/galaxy/tools/parameters/meta.py b/lib/galaxy/tools/parameters/meta.py
@@ -260,7 +260,7 @@ def __expand_collection_parameter(trans, input_key, incoming_val, collections_to
             if src != "hdca":
                 raise exceptions.ToolMetaParameterException(f"Invalid dataset collection source type {src}")
             encoded_hdc_id = incoming_val["id"]
-            subcollection_type = incoming_val.get("map_over_type", None)
+            subcollection_type = incoming_val.get("subcollection_type", None)
         except TypeError:
             encoded_hdc_id = incoming_val
             subcollection_type = None