Skip to content

Commit

Permalink
Fix collection type drag and drop validation
Browse files Browse the repository at this point in the history
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
```
  • Loading branch information
mvdbeek committed Aug 8, 2024
1 parent 85c4f65 commit 5dbaf5b
Show file tree
Hide file tree
Showing 2 changed files with 111 additions and 8 deletions.
48 changes: 48 additions & 0 deletions client/src/components/Form/Elements/FormData/FormData.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
71 changes: 63 additions & 8 deletions client/src/components/Form/Elements/FormData/FormData.vue
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const props = withDefaults(
values: Array<DataOption>;
};
extensions?: Array<string>;
type?: string;
type?: "data" | "data_collection";
collectionTypes?: Array<string>;
flavor?: string;
tag?: string;
Expand Down Expand Up @@ -310,6 +310,9 @@ function handleIncoming(incoming: Record<string, unknown>, 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<DataOption> = [];
values.forEach((v) => {
Expand Down Expand Up @@ -414,17 +417,67 @@ function canAcceptDatatype(itemDatatypes: string | Array<string>) {
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<string>);
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;
}
Expand Down Expand Up @@ -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`;
}
});
</script>
Expand Down

0 comments on commit 5dbaf5b

Please sign in to comment.