Unverified Commit 486f4134 authored by mvdbeek's avatar mvdbeek
Browse files

Ensure datatypes match on dragging elements into FormData 

This is pretty common source of cheetah templating issues
where tool authors assume (orrectly, IMO) only datatypes allowed in the format
attribute of an input dataset are allowed.
Here's a hypothetical example:

```
  # set index = $input.cram_index
  # set index = $input.cram_index
ln -s '$index' input.index
```
This is going to fail if a user drag e.g. a txt dataset into the input.

For anything but the valid datatypes this is going to fail.

Another way things can fail is if an input datatypes should contain
extra files, but doesn't, for instance in
https://sentry.galaxyproject.org/share/issue/73d7bd51dd2d418e96d563f0ac2383f0/:

```
Exception: __action_for_transfer called on non-existent file - [None]
  File "galaxy/jobs/runners/pulsar.py", line 446, in queue_job
    external_job_id = pulsar_submit_job(client, client_job_description, remote_job_config)
  File "pulsar/client/staging/up.py", line 39, in submit_job
    file_stager = FileStager(client, client_job_description, job_config)
  File "pulsar/client/staging/up.py", line 148, in __init__
    self.__upload_input_files()
  File "pulsar/client/staging/up.py", line 264, in __upload_input_files
    self.__upload_input_metadata_file(client_input.action_source)
  File "pulsar/client/staging/up.py", line 287, in __upload_input_metadata_file
    self.transfer_tracker.handle_transfer_source(input_action_source, path_type.INPUT, name=remote_name)
  File "pulsar/client/staging/up.py", line 495, in handle_transfer_source
    action = self.__action_for_transfer(source, type, contents)
  File "pulsar/client/staging/up.py", line 551, in __action_for_transfer
    raise Exception(message)
```

Fixes https://github.com/galaxyproject/galaxy/issues/8202

We can now easily change datatypes in batch if necessary,
this shouldn't be an argument to not properly implement
datatype filtering.
parent 4fd73fc0

client/src/components/Form/Elements/FormData/FormData.test.js

+4 −0
Original line number Diff line number Diff line
@@ -3,6 +3,8 @@ import { mount } from "@vue/test-utils"; 
import { PiniaVuePlugin } from "pinia";

import { dispatchEvent, getLocalVue } from "tests/jest/helpers";



import { testDatatypesMapper } from "@/components/Datatypes/test_fixtures";

import { useDatatypesMapperStore } from "@/stores/datatypesMapperStore";

import { useEventStore } from "@/stores/eventStore";



import MountTarget from "./FormData.vue";
@@ -15,6 +17,8 @@ let eventStore; 
function createTarget(propsData) {

    const pinia = createTestingPinia({ stubActions: false });

    eventStore = useEventStore();

    const datatypesStore = useDatatypesMapperStore();

    datatypesStore.datatypesMapper = testDatatypesMapper;

    return mount(MountTarget, {

        localVue,

        propsData,

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

+45 −11
Original line number Diff line number Diff line
@@ -7,6 +7,7 @@ import { BAlert, BButton, BButtonGroup, BCollapse, BFormCheckbox, BTooltip } fro 
import { computed, onMounted, type Ref, ref, watch } from "vue";



import { getGalaxyInstance } from "@/app";

import { useDatatypesMapper } from "@/composables/datatypesMapper";

import { useUid } from "@/composables/utils/uid";

import { type EventData, useEventStore } from "@/stores/eventStore";

import { orList } from "@/utils/strings";
@@ -51,8 +52,9 @@ const props = withDefaults( 
);



const eventStore = useEventStore();

const { datatypesMapper } = useDatatypesMapper();



const $emit = defineEmits(["input"]);

const $emit = defineEmits(["input", "alert"]);



// Determines wether values should be processed as linked or unlinked

const currentLinked = ref(true);
@@ -302,6 +304,10 @@ function getSourceType(val: DataOption) { 
function handleIncoming(incoming: 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));

        if (!canAcceptDatatype(extensions)) {

            return false;

        }

        if (values.length > 0) {

            const incomingValues: Array<DataOption> = [];

            values.forEach((v) => {
@@ -349,6 +355,7 @@ function handleIncoming(incoming: Record<string, unknown>, partial = true) { 
            }

        }

    }

    return true;

}



/**
@@ -372,10 +379,36 @@ function onBrowse() { 
    }

}



function canAcceptDatatype(itemDatatypes: string | Array<string>) {

    if (!(props.extensions?.length > 0)) {

        return true;

    }

    let datatypes: Array<string>;

    if (!Array.isArray(itemDatatypes)) {

        datatypes = [itemDatatypes];

    } else {

        datatypes = itemDatatypes;

    }

    const incompatibleItem = datatypes.find(

        (extension) => !datatypesMapper.value?.isSubTypeOfAny(extension, props.extensions)

    );

    if (incompatibleItem) {

        return false;

    }

    return true;

}



// 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>);

        if (!canAcceptDatatype(extensions)) {

            currentHighlighting.value = "warning";

            $emit("alert", `${extensions} is not an acceptable format for this parameter.`);

        } else {

            currentHighlighting.value = "success";

        }

        dragTarget.value = evt.target;

        dragData.value = eventData;

    }
@@ -384,23 +417,24 @@ function onDragEnter(evt: MouseEvent) { 
function onDragLeave(evt: MouseEvent) {

    if (dragTarget.value === evt.target) {

        currentHighlighting.value = null;

    }

}



function onDragOver() {

    if (dragData.value !== null) {

        currentHighlighting.value = "warning";

        $emit("alert", undefined);

    }

}



function onDrop() {

    if (dragData.value) {

        let accept = false;

        if (eventStore.multipleDragData) {

            handleIncoming(Object.values(dragData.value) as any, false);

            accept = handleIncoming(Object.values(dragData.value) as any, false);

        } else {

            handleIncoming(dragData.value);

            accept = handleIncoming(dragData.value);

        }

        if (accept) {

            currentHighlighting.value = "success";

        } else {

            currentHighlighting.value = "warning";

        }

        $emit("alert", undefined);

        dragData.value = null;

        clearHighlighting();

    }
@@ -468,7 +502,7 @@ const noOptionsWarningMessage = computed(() => { 
        :class="currentHighlighting && `ui-dragover-${currentHighlighting}`"

        @dragenter.prevent="onDragEnter"

        @dragleave.prevent="onDragLeave"

        @dragover.prevent="onDragOver"

        @dragover.prevent

        @drop.prevent="onDrop">

        <div class="d-flex flex-column">

            <BButtonGroup v-if="variant && variant.length > 1" buttons class="align-self-start">

client/src/components/Form/FormElement.test.js

+1 −1
Original line number Diff line number Diff line
@@ -30,7 +30,7 @@ describe("FormElement", () => { 
        const error = wrapper.find(".ui-form-error-text");

        expect(error.text()).toBe("error_text");



        await wrapper.setProps({ error: "" });

        await wrapper.setProps({ error: undefined });

        const no_error = wrapper.findAll(".ui-form-error");

        expect(no_error.length).toBe(0);

client/src/components/Form/FormElement.vue

+15 −6
Original line number Diff line number Diff line
@@ -127,7 +127,7 @@ function onConnect() { 


const isHidden = computed(() => attrs.value["hidden"]);

const elementId = computed(() => `form-element-${props.id}`);

const hasAlert = computed(() => Boolean(props.error || props.warning));

const hasAlert = computed(() => alerts.value.length > 0);

const showPreview = computed(() => (collapsed.value && attrs.value["collapsible_preview"]) || props.disabled);

const showField = computed(() => !collapsed.value && !props.disabled);
@@ -174,6 +174,16 @@ const isEmpty = computed(() => { 
const isRequired = computed(() => attrs.value["optional"] === false);

const isRequiredType = computed(() => props.type !== "boolean");

const isOptional = computed(() => !isRequired.value && attrs.value["optional"] !== undefined);

const formAlert = ref<string>();

const alerts = computed(() => {

    return [formAlert.value, props.error, props.warning]

        .filter((v) => v !== undefined && v !== null)

        .map((v) => linkify(sanitize(v!, { USE_PROFILES: { html: true } })));

});



function onAlert(value: string | undefined) {

    formAlert.value = value;

}

</script>



<template>
@@ -182,11 +192,9 @@ const isOptional = computed(() => !isRequired.value && attrs.value["optional"] ! 
        :id="elementId"

        class="ui-form-element section-row"

        :class="{ alert: hasAlert, 'alert-info': hasAlert }">

        <div v-if="hasAlert" class="ui-form-error">

        <div v-for="(alert, index) in alerts" :key="index" class="ui-form-error">

            <FontAwesomeIcon class="mr-1" icon="fa-exclamation" />

            <span

                class="ui-form-error-text"

                v-html="linkify(sanitize(props.error || props.warning, { USE_PROFILES: { html: true } }))" />

            <span class="ui-form-error-text" v-html="alert" />

        </div>



        <div class="ui-form-title">
@@ -288,7 +296,8 @@ const isOptional = computed(() => !isRequired.value && attrs.value["optional"] ! 
                :optional="attrs.optional"

                :options="attrs.options"

                :tag="attrs.tag"

                :type="props.type" />

                :type="props.type"

                @alert="onAlert" />

            <FormDrilldown

                v-else-if="props.type === 'drill_down'"

                :id="id"