Merge pull request #24105 from jmchilton/workbook_import_followups

Strengthen workbook import browser assertions after #23922

client/src/utils/navigation/navigation.yml

@@ -1753,6 +1753,10 @@ rule_builder:
     view_source: '.rule-builder-view-source'
     source: '.rule-source'
     table: '#rules-ag-grid .ag-root'
+    table_row: '#rules-ag-grid .ag-row'
+    table_row_cell: '#rules-ag-grid .ag-row[row-index="${row_index}"] .ag-cell'
+    # row_index is 0-based, column_number is ag-grid's 1-based aria-colindex
+    table_cell: '#rules-ag-grid .ag-row[row-index="${row_index}"] .ag-cell[aria-colindex="${column_number}"]'
     extension_select: '.rule-footer-extension-group .extension-select'
 
 charts:

lib/galaxy/selenium/navigates_galaxy.py

@@ -1235,6 +1235,21 @@ class NavigatesGalaxy(HasDriverProxy[WaitType]):
     def rule_builder_show_and_get_source_as_json(self) -> dict:
         return json.loads(self.rule_builder_show_and_get_source())
 
+    def rule_builder_column_values(self, column_index: int) -> list[str]:
+        """Text of each rendered cell in a rule builder column, in row order.
+
+        The grid only renders cells in view, so this moves keyboard focus to the
+        column to scroll it in. Rows are not scrolled, so this suits small tables.
+        """
+        rule_builder = self.components.rule_builder
+        rendered_cell = rule_builder.table_row_cell(row_index=0).wait_for_visible()
+        self.press(Key.HOME, *[Key.ARROW_RIGHT] * column_index, element=rendered_cell)
+        row_count = len(rule_builder.table_row.all())
+        return [
+            rule_builder.table_cell(row_index=row_index, column_number=column_index + 1).wait_for_visible().text
+            for row_index in range(row_count)
+        ]
+
     def rule_builder_enter_source_text(self, json):
         rule_builder = self.components.rule_builder
         text_area_elem = rule_builder.source.wait_for_visible()

lib/galaxy_test/selenium/test_workbook_mapping.py

@@ -5,6 +5,11 @@ these tests upload a workbook and assert the mapping it produced rather than
 building rules by hand.
 """
 
+from urllib.parse import (
+    parse_qs,
+    urlparse,
+)
+
 from .framework import (
     selenium_test,
     SeleniumTestCase,
@@ -30,6 +35,15 @@ class TestWorkbookMapping(SeleniumTestCase, UsesUploadActivity):
         assert entry["type"] == expected_type, f"Expected mapping {index} to be {expected_type}, got {entry}"
         assert entry["columns"] == expected_columns, f"Expected mapping {index} on {expected_columns}, got {entry}"
 
+    def _file_names(self, column_index: int) -> list[str]:
+        return [url.rsplit("/", 1)[-1] for url in self.rule_builder_column_values(column_index)]
+
+    def _assert_creating_datasets(self) -> None:
+        self.components.rule_builder.collection_name_input.assert_absent()
+
+    def _assert_creating_collections(self) -> None:
+        self.components.rule_builder.collection_name_input.wait_for_visible()
+
     def _assert_example_1_mapping(self) -> None:
         mapping = self._mapping(3)
         self._assert_mapping(mapping, 0, "name", [0])
@@ -40,6 +54,7 @@ class TestWorkbookMapping(SeleniumTestCase, UsesUploadActivity):
     def test_dataset_mapping_from_workbook(self):
         """Dataset name, URL and genome columns are recognized by their headers."""
         self.upload_context("rule").upload_workbook(self._workbook(1))
+        self._assert_creating_datasets()
         self._assert_example_1_mapping()
 
     @selenium_test
@@ -48,6 +63,7 @@ class TestWorkbookMapping(SeleniumTestCase, UsesUploadActivity):
         rule_import = self.upload_context("rule").creating("datasets").from_source("workbook")
         assert "type=datasets" in rule_import.workbook_download_url()
         rule_import.upload_workbook_from_card(self._workbook(1))
+        self._assert_creating_datasets()
         self._assert_example_1_mapping()
 
     @selenium_test
@@ -60,13 +76,15 @@ class TestWorkbookMapping(SeleniumTestCase, UsesUploadActivity):
             .workbook_for_collection_type("list:paired")
         )
         download_url = rule_import.workbook_download_url()
-        assert "type=collection" in download_url, download_url
-        assert "collection_type=list:paired" in download_url.replace("%3A", ":"), download_url
+        query = parse_qs(urlparse(download_url).query)
+        assert query["type"] == ["collection"], download_url
+        assert query["collection_type"] == ["list:paired"], download_url
 
     @selenium_test
     def test_list_collection_mapping_from_workbook(self):
         """URI and TYPE are recognized as alternate spellings of url and file type."""
         self.upload_context("rule").upload_workbook(self._workbook(2))
+        self._assert_creating_collections()
 
         mapping = self._mapping(3)
         self._assert_mapping(mapping, 0, "list_identifiers", [0])
@@ -77,6 +95,7 @@ class TestWorkbookMapping(SeleniumTestCase, UsesUploadActivity):
     def test_list_paired_collection_mapping_from_workbook(self):
         """Two URL columns per row are split into forward and reverse elements."""
         self.upload_context("rule").upload_workbook(self._workbook(3))
+        self._assert_creating_collections()
 
         mapping = self._mapping(4)
         self._assert_mapping(mapping, 0, "list_identifiers", [0])
@@ -84,12 +103,31 @@ class TestWorkbookMapping(SeleniumTestCase, UsesUploadActivity):
         self._assert_mapping(mapping, 2, "dbkey", [2])
         self._assert_mapping(mapping, 3, "paired_identifier", [3])
 
+        accessions = ["DRX036147", "DRX036148", "DRX036149"]
+        assert self.rule_builder_column_values(0) == [a for a in accessions for _ in range(2)]
+        assert self._file_names(1) == [f"{a}_{i}.fastq.gz" for a in accessions for i in "12"]
+        assert self.rule_builder_column_values(3) == ["1", "2"] * 3
+
     @selenium_test
     def test_nested_list_paired_collection_mapping_from_workbook(self):
         """Two identifier columns nest, outermost first, around the split pairs."""
         self.upload_context("rule").upload_workbook(self._workbook(4))
+        self._assert_creating_collections()
 
         mapping = self._mapping(3)
         self._assert_mapping(mapping, 0, "url", [0])
         self._assert_mapping(mapping, 1, "list_identifiers", [1, 2])
         self._assert_mapping(mapping, 2, "paired_identifier", [3])
+
+        rows = [
+            ("SRR5680996", "H3K4me3", "rep1"),
+            ("SRR5680997", "H3K27me3", "rep2"),
+            ("SRR5681007", "H3K27me3", "rep1"),
+            ("SRR5681006", "H3K4me3", "rep2"),
+            ("SRR5680998", "CTCF", "rep1"),
+            ("SRR5681008", "CTCF", "rep2"),
+        ]
+        assert self._file_names(0) == [f"{run}_{i}.fastq.gz" for run, _, _ in rows for i in "12"]
+        assert self.rule_builder_column_values(1) == [outer for _, outer, _ in rows for _ in range(2)]
+        assert self.rule_builder_column_values(2) == [inner for _, _, inner in rows for _ in range(2)]
+        assert self.rule_builder_column_values(3) == ["1", "2"] * 6