From 71a9391cffecd7316d1ef23d89d6808feb129bc1 Mon Sep 17 00:00:00 2001 From: guerler Date: Wed, 30 Aug 2023 12:39:11 +0300 Subject: [PATCH] Rename map_over_type to subcollection_type for consistency, add test to verify that mapping mode is correctly assigned --- .../Form/Elements/FormData/FormData.test.js | 75 +++++++++++++------ .../Form/Elements/FormData/FormData.vue | 21 +++--- .../Form/Elements/FormData/types.ts | 2 +- lib/galaxy/tools/parameters/basic.py | 6 +- lib/galaxy/tools/parameters/meta.py | 2 +- 5 files changed, 67 insertions(+), 39 deletions(-) diff --git a/client/src/components/Form/Elements/FormData/FormData.test.js b/client/src/components/Form/Elements/FormData/FormData.test.js index 397547fa60ec..ee46af6ef926 100644 --- 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,16 +172,16 @@ 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, @@ -176,17 +189,31 @@ describe("FormData", () => { 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,7 +240,7 @@ 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"); @@ -221,7 +248,7 @@ describe("FormData", () => { 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,7 +261,7 @@ 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(); @@ -242,7 +269,7 @@ describe("FormData", () => { 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 index e67751d6b66e..25a0cbae2711 100644 --- 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 | 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 | 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 index 95a1c6b8002a..489fef46753f 100644 --- 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; } diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index ae0598d8acdf..85212649f060 100644 --- 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 index a9f47ac0eae0..a0380f53a825 100644 --- 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