Merge pull request #16224 from jmchilton/edit_attributes

E2E Tests for Edit Dataset Attributes Page

client/src/components/DatasetInformation/DatasetAttributes.vue

@@ -1,7 +1,7 @@
 <template>
     <div aria-labelledby="dataset-attributes-heading">
         <h1 id="dataset-attributes-heading" v-localize class="h-lg">Edit Dataset Attributes</h1>
-        <b-alert v-if="messageText" :variant="messageVariant" show>
+        <b-alert v-if="messageText" class="dataset-attributes-alert" :variant="messageVariant" show>
             {{ messageText | l }}
         </b-alert>
         <DatasetAttributesProvider :id="datasetId" v-slot="{ result, loading }" @error="onError">

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

@@ -1,8 +1,10 @@
 <script setup lang="ts">
 import { computed, onMounted, watch, type ComputedRef } from "vue";
 import Multiselect from "vue-multiselect";
+import { useMultiselect } from "@/composables/useMultiselect";
 
 type SelectValue = string | null;
+const { ariaExpanded, onOpen, onClose } = useMultiselect();
 
 interface SelectOption {
     label: string;
@@ -148,9 +150,12 @@ onMounted(() => {
         :options="formattedOptions"
         :multiple="multiple"
         :selected-label="selectedLabel"
+        :aria-expanded="ariaExpanded"
         placeholder="Select value"
         select-label="Click to select"
         track-by="value"
-        label="label" />
+        label="label"
+        @open="onOpen"
+        @close="onClose" />
     <b-alert v-else v-localize variant="warning" show> No options available. </b-alert>
 </template>

client/src/components/Form/FormElement.vue

@@ -200,10 +200,12 @@ const isOptional = computed(() => !isRequired.value && attrs.value["optional"] !
                 </b-button>
 
                 <span v-if="props.title" class="ui-form-title-text ml-1">
-                    {{ props.title }}
+                    <label :for="props.id">{{ props.title }}</label>
                 </span>
             </span>
-            <span v-else-if="props.title" class="ui-form-title-text">{{ props.title }}</span>
+            <span v-else-if="props.title" class="ui-form-title-text"
+                ><label :for="props.id">{{ props.title }}</label></span
+            >
 
             <span
                 v-if="isRequired && isRequiredType && props.title"

client/src/components/TagsMultiselect/StatelessTags.vue

@@ -1,6 +1,7 @@
 <script setup lang="ts">
 import { ref, computed } from "vue";
 import Multiselect from "vue-multiselect";
+import { useMultiselect } from "@/composables/useMultiselect";
 import { FontAwesomeIcon } from "@fortawesome/vue-fontawesome";
 import { library } from "@fortawesome/fontawesome-svg-core";
 import { faTags, faCheck, faTimes, faPlus } from "@fortawesome/free-solid-svg-icons";
@@ -62,15 +63,7 @@ function onDelete(tag: string) {
     emit("input", val);
 }
 
-const editing = ref(false);
-
-function onOpen() {
-    editing.value = true;
-}
-
-function onClose() {
-    editing.value = false;
-}
+const { editing, ariaExpanded, onOpen, onClose } = useMultiselect();
 
 const multiselectElement: Ref<Multiselect | null> = ref(null);
 
@@ -127,7 +120,7 @@ function onTagClicked(tag: string) {
             :multiple="true"
             :taggable="true"
             :close-on-select="false"
-            :aria-expanded="editing ? 'true' : 'false'"
+            :aria-expanded="ariaExpanded"
             @tag="onAddTag"
             @input="onInput"
             @open="onOpen"

client/src/composables/useMultiselect.ts

@@ -0,0 +1,18 @@
+import { ref, computed } from "vue";
+
+export function useMultiselect() {
+    const editing = ref(false);
+    const ariaExpanded = computed(() => {
+        return editing.value ? "true" : "false";
+    });
+
+    function onOpen() {
+        editing.value = true;
+    }
+
+    function onClose() {
+        editing.value = false;
+    }
+
+    return { editing, ariaExpanded, onOpen, onClose };
+}

client/src/utils/navigation/navigation.yml

@@ -334,7 +334,13 @@ storage_dashboard:
 edit_dataset_attributes:
   selectors:
     database_build_dropdown: '[data-label="Database/Build"]'
-    save_btn: '#dataset-attributes-default-save'
+
+    _: '[aria-labelledby="dataset-attributes-heading"]'
+    name_input: '[aria-labelledby="dataset-attributes-heading"] input#name'
+    info_input: '[aria-labelledby="dataset-attributes-heading"] textarea#info'
+    annotation_input: '[aria-labelledby="dataset-attributes-heading"] textarea#annotation'
+    save_button: '#dataset-attributes-default-save'
+    alert: '.dataset-attributes-alert'
 
   dbkey_dropdown_results:
     selectors:

lib/galaxy/selenium/axe_results.py

@@ -19,19 +19,20 @@
 
 # check all violations of this level as a baseline 'impact'...
 BASELINE_VIOLATION_FILTER: Impact = "critical"
+FORMS_VIOLATIONS = ["duplicate-id-aria"]  # https://github.com/galaxyproject/galaxy/issues/16188
 # unless they are in this list...
-KNOWN_VIOLATIONS = [
+KNOWN_VIOLATIONS = FORMS_VIOLATIONS + [
     "aria-required-attr",
     "aria-required-children",
     "aria-required-parent",
-    "duplicate-id-aria",  # id="listbox-null" appears in a couple tests (e.g. test_data_column_input_editing)
     "image-alt",  # test_workflow_editor.py::TestWorkflowEditor::test_existing_connections
     "label",
     "button-name",
     "select-name",
 ]
 # Over time we hope known violations grows smaller until the violation
 # filter can be lowered. Next level would be "serious".
+# xref https://github.com/galaxyproject/galaxy/issues/16185
 
 
 class AxeResult:
@@ -89,7 +90,9 @@ def violations(self) -> List[Violation]:
     def violations_with_impact_of_at_least(self, impact: Impact) -> List[Violation]:
         """"""
 
-    def assert_no_violations_with_impact_of_at_least(self, impact: Impact) -> None:
+    def assert_no_violations_with_impact_of_at_least(
+        self, impact: Impact, excludes: Optional[List[str]] = None
+    ) -> None:
         """"""
 
 
@@ -116,10 +119,14 @@ def violations(self) -> List[Violation]:
     def violations_with_impact_of_at_least(self, impact: Impact) -> List[Violation]:
         return [v for v in self.violations() if v.is_impact_at_least(impact)]
 
-    def assert_no_violations_with_impact_of_at_least(self, impact: Impact) -> None:
+    def assert_no_violations_with_impact_of_at_least(
+        self, impact: Impact, excludes: Optional[List[str]] = None
+    ) -> None:
+        excludes = excludes or []
         violations = self.violations_with_impact_of_at_least(impact)
-        if violations:
-            raise AssertionError(violations[0].message)
+        filtered_violations = [v for v in violations if v.id not in excludes]
+        if filtered_violations:
+            raise AssertionError(filtered_violations[0].message)
 
 
 class NullAxeResults(AxeResults):
@@ -138,7 +145,9 @@ def violations(self) -> List[Violation]:
     def violations_with_impact_of_at_least(self, impact: Impact) -> List[Violation]:
         return []
 
-    def assert_no_violations_with_impact_of_at_least(self, impact: Impact) -> None:
+    def assert_no_violations_with_impact_of_at_least(
+        self, impact: Impact, excludes: Optional[List[str]] = None
+    ) -> None:
         pass
 
 

lib/galaxy/selenium/driver_factory.py

@@ -27,6 +27,7 @@
 DEFAULT_WINDOW_WIDTH = 1280
 DEFAULT_WINDOW_HEIGHT = 1000
 VALID_LOCAL_BROWSERS = ["CHROME", "FIREFOX"]
+PYVIRTUALDISPLAY_UNAVAILABLE_MESSAGE = "pyvirtualdisplay must be installed to run this test configuration and is not, install with 'pip install pyvirtualdisplay'"
 
 
 class ConfiguredDriver:
@@ -136,6 +137,8 @@ def is_virtual_display_available():
 
 def virtual_display_if_enabled(enabled):
     if enabled:
+        if Display is None:
+            raise Exception(PYVIRTUALDISPLAY_UNAVAILABLE_MESSAGE)
         display = Display(visible=0, size=(800, 600))
         display.start()
         return display

lib/galaxy/selenium/navigates_galaxy.py

@@ -17,6 +17,7 @@
     Any,
     cast,
     Dict,
+    NamedTuple,
     Optional,
     Union,
 )
@@ -53,6 +54,12 @@
 WaitType = collections.namedtuple("WaitType", ["name", "default_length"])
 
 
+class HistoryEntry(NamedTuple):
+    id: str
+    hid: str
+    history_content_type: str
+
+
 # Default wait times should make sense for a development server under low
 # load. Wait times for production servers can be scaled up with a multiplier.
 class WAIT_TYPES:
@@ -375,10 +382,29 @@ def current_history_publish(self):
         self.click_history_option_sharing()
         self.make_accessible_and_publishable()
 
-    def latest_history_item(self):
+    def latest_history_entry(self):
+        entry_dict = self._latest_history_item()
+        if entry_dict is None:
+            return None
+        else:
+            return HistoryEntry(
+                id=entry_dict["id"],
+                hid=entry_dict["hid"],
+                history_content_type=entry_dict["history_content_type"],
+            )
+
+    def latest_history_item(self) -> Dict[str, Any]:
+        return_value = self._latest_history_item()
+        assert return_value, "Attempted to get latest history item on empty history."
+        return return_value
+
+    def _latest_history_item(self) -> Optional[Dict[str, Any]]:
         history_contents = self.history_contents()
-        assert len(history_contents) > 0
-        return history_contents[-1]
+        if len(history_contents) > 0:
+            entry_dict = history_contents[-1]
+            return entry_dict
+        else:
+            return None
 
     def wait_for_history(self, assert_ok=True):
         def history_becomes_terminal(driver):
@@ -714,6 +740,15 @@ def hover_over(self, target):
         action_chains = self.action_chains()
         action_chains.move_to_element(target).perform()
 
+    def perform_single_upload(self, test_path, **kwd) -> HistoryEntry:
+        before_latest_history_item = self.latest_history_entry()
+        self._perform_upload(test_path=test_path, **kwd)
+        after_latest_history_item = self.latest_history_entry()
+        assert after_latest_history_item
+        if before_latest_history_item is not None:
+            assert before_latest_history_item.id != after_latest_history_item.id
+        return after_latest_history_item
+
     def perform_upload(self, test_path, **kwd):
         self._perform_upload(test_path=test_path, **kwd)
 
@@ -1748,6 +1783,11 @@ def history_multi_view_display_collection_contents(self, collection_hid, collect
         self.wait_for_and_click(dataset_selector)
         self.history_panel_wait_for_hid_state(1, "ok", multi_history_panel=True)
 
+    def history_panel_item_edit(self, hid):
+        item = self.history_panel_item_component(hid=hid)
+        item.edit_button.wait_for_and_click()
+        self.components.edit_dataset_attributes._.wait_for_visible()
+
     def history_panel_item_view_dataset_details(self, hid):
         item = self.history_panel_item_component(hid=hid)
         item.dataset_operations.wait_for_visible()

lib/galaxy/selenium/smart_components.py

@@ -1,3 +1,8 @@
+from typing import (
+    List,
+    Optional,
+)
+
 from selenium.webdriver.support.select import Select
 
 from galaxy.navigation.components import (
@@ -132,12 +137,19 @@ def has_class(self, class_name):
     def wait_for_and_send_keys(self, *text):
         self.wait_for_visible().send_keys(*text)
 
+    def wait_for_and_clear_and_send_keys(self, *text):
+        dom_element = self.wait_for_visible()
+        dom_element.clear()
+        dom_element.send_keys(*text)
+
     def axe_eval(self) -> AxeResults:
         return self._has_driver.axe_eval(context=self._target.element_locator[1])
 
-    def assert_no_axe_violations_with_impact_of_at_least(self, impact: Impact) -> None:
+    def assert_no_axe_violations_with_impact_of_at_least(
+        self, impact: Impact, excludes: Optional[List[str]] = None
+    ) -> None:
         self.wait_for_visible()
-        self.axe_eval().assert_no_violations_with_impact_of_at_least(impact)
+        self.axe_eval().assert_no_violations_with_impact_of_at_least(impact, excludes=excludes)
 
     def __str__(self):
         return f"SmartTarget[_target={self._target}]"

lib/galaxy_test/selenium/test_dataset_edit.py

@@ -0,0 +1,67 @@
+from galaxy.selenium.axe_results import FORMS_VIOLATIONS
+from .framework import (
+    managed_history,
+    selenium_test,
+    SeleniumTestCase,
+)
+
+TEST_ANNOTATION = "my cool annotation"
+TEST_INFO = "my cool info"
+
+
+class TestHistoryPanel(SeleniumTestCase):
+    ensure_registered = True
+
+    @selenium_test
+    @managed_history
+    def test_history_dataset_rename(self):
+        original_name = "1.txt"
+        new_name = "newname.txt"
+
+        history_entry = self.perform_single_upload(self.get_filename(original_name))
+        hid = history_entry.hid
+        self.wait_for_history()
+        self.history_panel_wait_for_hid_ok(hid)
+        self.history_panel_item_edit(hid=hid)
+        edit_dataset_attributes = self.components.edit_dataset_attributes
+        name_component = edit_dataset_attributes.name_input
+        assert name_component.wait_for_value() == original_name
+        edit_dataset_attributes._.assert_no_axe_violations_with_impact_of_at_least(
+            "critical", excludes=FORMS_VIOLATIONS
+        )
+        name_component.wait_for_and_clear_and_send_keys(new_name)
+        edit_dataset_attributes.save_button.wait_for_and_click()
+        edit_dataset_attributes.alert.wait_for_visible()
+
+        # assert success message, name updated in form and in history panel
+        assert edit_dataset_attributes.alert.has_class("alert-success")
+        assert name_component.wait_for_value() == new_name
+        assert self.history_panel_item_component(hid=hid).name.wait_for_text() == new_name
+
+    @selenium_test
+    @managed_history
+    def test_history_dataset_update_annotation_and_info(self):
+        history_entry = self.perform_single_upload(self.get_filename("1.txt"))
+        hid = history_entry.hid
+        self.wait_for_history()
+        self.history_panel_wait_for_hid_ok(hid)
+        self.history_panel_item_edit(hid=hid)
+        edit_dataset_attributes = self.components.edit_dataset_attributes
+        annotation_component = edit_dataset_attributes.annotation_input
+        annotation_component.wait_for_and_clear_and_send_keys(TEST_ANNOTATION)
+
+        info_component = edit_dataset_attributes.info_input
+        info_component.wait_for_and_clear_and_send_keys(TEST_INFO)
+
+        edit_dataset_attributes.save_button.wait_for_and_click()
+        edit_dataset_attributes.alert.wait_for_visible()
+
+        # assert success message, name updated in form and in history panel
+        assert edit_dataset_attributes.alert.has_class("alert-success")
+
+        # reopen and check that attributes are updated
+        self.home()
+        self.history_panel_item_edit(hid=hid)
+
+        assert annotation_component.wait_for_value() == TEST_ANNOTATION
+        assert info_component.wait_for_value() == TEST_INFO

lib/galaxy_test/selenium/test_history_dataset_state.py

@@ -54,7 +54,7 @@ def test_dataset_change_dbkey(self):
         self.components.edit_dataset_attributes.dbkey_dropdown_results.dbkey_dropdown_option(
             dbkey_text=TEST_DBKEY_TEXT
         ).wait_for_and_click()
-        self.components.edit_dataset_attributes.save_btn.wait_for_and_click()
+        self.components.edit_dataset_attributes.save_button.wait_for_and_click()
         self.sleep_for(self.wait_types.JOB_COMPLETION)
         self.history_panel_wait_for_hid_ok(FIRST_HID)
         self.assert_item_dbkey_displayed_as(FIRST_HID, "apiMel3")