Merge branch 'release_23.1' into dev

.github/workflows/integration_selenium.yaml

@@ -21,8 +21,11 @@ concurrency:
   group: ${{ github.workflow }}-${{ github.ref }}
   cancel-in-progress: true
 jobs:
+  setup-selenium:
+    uses: ./.github/workflows/setup_selenium.yaml
   test:
     name: Test
+    needs: setup-selenium
     runs-on: ubuntu-latest
     strategy:
       matrix:
@@ -68,7 +71,6 @@ jobs:
           node-version: '18.12.1'
           cache: 'yarn'
           cache-dependency-path: 'galaxy root/client/yarn.lock'
-      - uses: nanasess/setup-chromedriver@v1
       - name: Run tests
         run: ./run_tests.sh --coverage -integration test/integration_selenium
         working-directory: 'galaxy root'

.github/workflows/selenium.yaml

@@ -21,8 +21,11 @@ concurrency:
   group: ${{ github.workflow }}-${{ github.ref }}
   cancel-in-progress: true
 jobs:
+  setup-selenium:
+    uses: ./.github/workflows/setup_selenium.yaml
   test:
     name: Test
+    needs: setup-selenium
     runs-on: ubuntu-latest
     strategy:
       fail-fast: false
@@ -68,7 +71,6 @@ jobs:
         with:
           path: 'galaxy root/.venv'
           key: gxy-venv-${{ runner.os }}-${{ steps.full-python-version.outputs.version }}-${{ hashFiles('galaxy root/requirements.txt') }}-selenium
-      - uses: nanasess/setup-chromedriver@v1
       - name: Run tests
         run: ./run_tests.sh --coverage -selenium lib/galaxy_test/selenium -- --num-shards=3 --shard-id=${{ matrix.chunk }}
         working-directory: 'galaxy root'

.github/workflows/setup_selenium.yaml

@@ -0,0 +1,8 @@
+on:
+  workflow_call:
+jobs:
+  setup_chromedriver:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Install chromedriver
+        uses: mvdbeek/setup-chromedriver@chromedriver_puppeteer

client/src/components/Notifications/Broadcasts/BroadcastsOverlay.test.ts

@@ -0,0 +1,83 @@
+import { createTestingPinia } from "@pinia/testing";
+import { getLocalVue } from "@tests/jest/helpers";
+import { shallowMount } from "@vue/test-utils";
+import flushPromises from "flush-promises";
+import { setActivePinia } from "pinia";
+
+import { type BroadcastNotification, useBroadcastsStore } from "@/stores/broadcastsStore";
+
+import BroadcastsOverlay from "./BroadcastsOverlay.vue";
+
+const localVue = getLocalVue(true);
+
+const now = new Date();
+const inTwoMonths = new Date(now.setMonth(now.getMonth() + 2));
+
+function generateBroadcastNotification(id: string): BroadcastNotification {
+    return {
+        id: id,
+        create_time: now.toISOString(),
+        update_time: now.toISOString(),
+        publication_time: now.toISOString(),
+        expiration_time: inTwoMonths.toISOString(),
+        source: "testing",
+        variant: "info",
+        content: {
+            subject: `Test subject ${id}`,
+            message: `Test message ${id}`,
+        },
+    };
+}
+
+const FAKE_BROADCASTS: BroadcastNotification[] = [
+    generateBroadcastNotification("1"),
+    generateBroadcastNotification("2"),
+];
+
+async function mountBroadcastsOverlayWith(broadcasts: BroadcastNotification[] = []) {
+    const pinia = createTestingPinia();
+    setActivePinia(pinia);
+
+    const broadcastsStore = useBroadcastsStore();
+    broadcastsStore.broadcasts = broadcasts;
+
+    const spyOnDismissBroadcast = jest.spyOn(broadcastsStore, "dismissBroadcast");
+    spyOnDismissBroadcast.mockImplementation(async (broadcast) => {
+        broadcastsStore.broadcasts = broadcastsStore.broadcasts.filter((b) => b.id !== broadcast.id);
+    });
+
+    const wrapper = shallowMount(BroadcastsOverlay, {
+        localVue,
+        pinia,
+    });
+
+    await flushPromises();
+    return wrapper;
+}
+
+describe("BroadcastsOverlay.vue", () => {
+    it("should not render anything when there is no broadcast", async () => {
+        const wrapper = await mountBroadcastsOverlayWith();
+
+        expect(wrapper.exists()).toBe(true);
+        expect(wrapper.html()).toBe("");
+    });
+
+    it("should render only one broadcast at a time", async () => {
+        const wrapper = await mountBroadcastsOverlayWith(FAKE_BROADCASTS);
+        expect(wrapper.findAll(".broadcast-message")).toHaveLength(1);
+        expect(wrapper.find(".broadcast-message").text()).toContain("Test message 1");
+    });
+
+    it("should render the next broadcast when the current one is dismissed", async () => {
+        const wrapper = await mountBroadcastsOverlayWith(FAKE_BROADCASTS);
+        expect(wrapper.findAll(".broadcast-message")).toHaveLength(1);
+        expect(wrapper.find(".broadcast-message").text()).toContain("Test message 1");
+
+        const dismissButton = wrapper.find("#dismiss-button");
+        await dismissButton.trigger("click");
+
+        expect(wrapper.findAll(".broadcast-message")).toHaveLength(1);
+        expect(wrapper.find(".broadcast-message").text()).toContain("Test message 2");
+    });
+});

client/src/components/Notifications/Broadcasts/BroadcastsOverlay.vue

@@ -4,6 +4,7 @@ import { faInfoCircle, faTimes } from "@fortawesome/free-solid-svg-icons";
 import { FontAwesomeIcon } from "@fortawesome/vue-fontawesome";
 import { BButton } from "bootstrap-vue";
 import { storeToRefs } from "pinia";
+import { computed } from "vue";
 import { useRouter } from "vue-router/composables";
 
 import { useMarkdown } from "@/composables/markdown";
@@ -19,6 +20,21 @@ const broadcastsStore = useBroadcastsStore();
 const { activeBroadcasts } = storeToRefs(useBroadcastsStore());
 const { renderMarkdown } = useMarkdown({ openLinksInNewPage: true });
 
+const currentBroadcast = computed(() => getNextActiveBroadcast());
+
+const remainingBroadcastsCountText = computed(() => {
+    const count = activeBroadcasts.value.length - 1;
+    return count > 0 ? `${count} more` : "";
+});
+
+function getNextActiveBroadcast(): BroadcastNotification | undefined {
+    return activeBroadcasts.value.sort(sortByPublicationTime).at(0);
+}
+
+function sortByPublicationTime(a: BroadcastNotification, b: BroadcastNotification) {
+    return new Date(a.publication_time).getTime() - new Date(b.publication_time).getTime();
+}
+
 function getBroadcastVariant(item: BroadcastNotification) {
     switch (item.variant) {
         case "urgent":
@@ -34,58 +50,64 @@ function onActionClick(item: BroadcastNotification, link: string) {
     } else {
         window.open(link, "_blank");
     }
+    onDismiss(item);
+}
+
+function onDismiss(item: BroadcastNotification) {
     broadcastsStore.dismissBroadcast(item);
 }
 </script>
 
 <template>
-    <div v-if="activeBroadcasts.length > 0">
-        <div v-for="broadcast in activeBroadcasts" :key="broadcast.id">
-            <BRow
-                align-v="center"
-                class="broadcast-banner m-0"
-                :class="{ 'non-urgent': broadcast.variant !== 'urgent' }">
-                <BCol cols="auto">
-                    <FontAwesomeIcon
-                        class="mx-2"
-                        fade
-                        size="2xl"
-                        :class="`text-${getBroadcastVariant(broadcast)}`"
-                        :icon="faInfoCircle" />
-                </BCol>
-                <BCol>
-                    <BRow align-v="center">
-                        <Heading size="md" bold>
-                            {{ broadcast.content.subject }}
-                        </Heading>
-                    </BRow>
-                    <BRow align-v="center">
-                        <span class="broadcast-message" v-html="renderMarkdown(broadcast.content.message)" />
-                    </BRow>
-                    <BRow>
-                        <div v-if="broadcast.content.action_links">
-                            <BButton
-                                v-for="actionLink in broadcast.content.action_links"
-                                :key="actionLink.action_name"
-                                :title="actionLink.action_name"
-                                variant="primary"
-                                @click="onActionClick(broadcast, actionLink.link)">
-                                {{ actionLink.action_name }}
-                            </BButton>
-                        </div>
-                    </BRow>
-                </BCol>
-                <BCol cols="auto" align-self="center" class="p-0">
-                    <BButton
-                        variant="light"
-                        class="align-items-center d-flex"
-                        @click="broadcastsStore.dismissBroadcast(broadcast)">
-                        <FontAwesomeIcon class="mx-1" icon="times" />
-                        Dismiss
-                    </BButton>
-                </BCol>
-            </BRow>
-        </div>
+    <div v-if="currentBroadcast">
+        <BRow
+            align-v="center"
+            class="broadcast-banner m-0"
+            :class="{ 'non-urgent': currentBroadcast.variant !== 'urgent' }">
+            <BCol cols="auto">
+                <FontAwesomeIcon
+                    class="mx-2"
+                    fade
+                    size="2xl"
+                    :class="`text-${getBroadcastVariant(currentBroadcast)}`"
+                    :icon="faInfoCircle" />
+            </BCol>
+            <BCol>
+                <BRow align-v="center">
+                    <Heading size="md" bold>
+                        {{ currentBroadcast.content.subject }}
+                    </Heading>
+                </BRow>
+                <BRow align-v="center">
+                    <span class="broadcast-message" v-html="renderMarkdown(currentBroadcast.content.message)" />
+                </BRow>
+                <BRow>
+                    <div v-if="currentBroadcast.content.action_links">
+                        <BButton
+                            v-for="actionLink in currentBroadcast.content.action_links"
+                            :key="actionLink.action_name"
+                            :title="actionLink.action_name"
+                            variant="primary"
+                            @click="onActionClick(currentBroadcast, actionLink.link)">
+                            {{ actionLink.action_name }}
+                        </BButton>
+                    </div>
+                </BRow>
+            </BCol>
+            <BCol cols="auto" align-self="center" class="p-0">
+                <BButton
+                    id="dismiss-button"
+                    variant="light"
+                    class="align-items-center d-flex"
+                    @click="onDismiss(currentBroadcast)">
+                    <FontAwesomeIcon class="mx-1" icon="times" />
+                    Dismiss
+                </BButton>
+                <div v-if="remainingBroadcastsCountText" class="text-center mt-2">
+                    {{ remainingBroadcastsCountText }}...
+                </div>
+            </BCol>
+        </BRow>
     </div>
 </template>
 

config/plugins/webhooks/news/script.js

@@ -6,11 +6,11 @@
         }
     }
 
-    function newsSeen() {
+    function newsSeen(currentGalaxyVersion) {
         // When it's seen, remove fa, add far.
         const newsIconSpan = document.querySelector("#news .fa-bullhorn");
         newsIconSpan.classList.remove("fa-fade");
-        window.localStorage.setItem("galaxy-news-seen-release", Galaxy.config.version_major);
+        window.localStorage.setItem("galaxy-news-seen-release", currentGalaxyVersion);
     }
 
     function newsUnseen() {
@@ -50,23 +50,26 @@
         el.parentNode.replaceChild(clean, el);
 
         let currentGalaxyVersion = Galaxy.config.version_major;
+        const lastSeenVersion = window.localStorage.getItem("galaxy-news-seen-release");
 
-        // If we're at the 23.1 release candidate, we want to show the 23.0 release notes still.
-        // This should be the last release using this hack -- new notification
-        // system will provide notes moving forward
-
-        if (currentGalaxyVersion == "23.1" && Galaxy.config.version_minor.startsWith("rc")) {
-            currentGalaxyVersion = "23.0";
+        // If we're at a deployed release candidate, just mark it seen and show
+        // the previous notes if someone clicks the link.  RC notes won't exist.
+        if (Galaxy.config.version_minor.startsWith("rc")) {
+            // If we, for whatever reason, need to do this again just add
+            // another case here.  It's not worth parsing and doing version
+            // math, and we should be able to drop preferring notifications
+            // framework moving forward in 23.2
+            if (currentGalaxyVersion == "23.1") {
+                currentGalaxyVersion = "23.0";
+            }
+            newsSeen(currentGalaxyVersion);
+        } else if (lastSeenVersion != currentGalaxyVersion) {
+            newsUnseen();
+        } else {
+            newsSeen(currentGalaxyVersion);
         }
 
         const releaseNotes = `https://docs.galaxyproject.org/en/latest/releases/${currentGalaxyVersion}_announce_user.html`;
-        const lastSeenVersion = window.localStorage.getItem("galaxy-news-seen-release");
-        // Check that they've seen the current version's release notes.
-        if (lastSeenVersion != currentGalaxyVersion) {
-            newsUnseen();
-        } else {
-            newsSeen();
-        }
 
         clean.addEventListener("click", (e) => {
             e.preventDefault();
@@ -92,7 +95,7 @@
                 });
             }
             document.getElementById("news-container").style.visibility = "visible";
-            newsSeen();
+            newsSeen(currentGalaxyVersion);
         });
     });
 

lib/galaxy/tool_util/linters/outputs.py

@@ -45,7 +45,14 @@ def lint_output(tool_xml, lint_ctx):
 
         label = output.attrib.get("label", "${tool.name} on ${on_string}")
         if label in labels:
-            lint_ctx.error(f"Tool output [{name}] uses duplicated label '{label}'", node=output)
+            filter_node = output.find(".//filter")
+            if filter_node is not None:
+                lint_ctx.warn(
+                    f"Tool output [{name}] uses duplicated label '{label}', double check if filters imply disjoint cases",
+                    node=output,
+                )
+            else:
+                lint_ctx.warn(f"Tool output [{name}] uses duplicated label '{label}'", node=output)
         labels.add(label)
 
         format_set = False

lib/galaxy/web/framework/base.py

@@ -543,9 +543,9 @@ def send_file(start_response, trans, body):
         start = None
         end = None
         if trans.request.range:
-            start = trans.request.range.start
-            end = trans.request.range.end
-            file_size = trans.response.headers["content-length"]
+            start = int(trans.request.range.start)
+            file_size = int(trans.response.headers["content-length"])
+            end = int(file_size if end is None else trans.request.range.end)
             trans.response.headers["content-length"] = str(end - start)
             trans.response.headers["content-range"] = f"bytes {start}-{end - 1}/{file_size}"
             trans.response.status = 206

lib/galaxy/web/framework/middleware/static.py

@@ -18,7 +18,10 @@ class CacheableStaticURLParser(StaticURLParser):
 
     def __call__(self, environ, start_response):
         path_info = environ.get("PATH_INFO", "")
-        if not path_info:
+        script_name = environ.get("SCRIPT_NAME", "")
+        if script_name == "/robots.txt" or script_name == "/favicon.ico":
+            filename = script_name.replace("/", "")
+        elif not path_info:
             # See if this is a static file hackishly mapped.
             if os.path.exists(self.directory) and os.path.isfile(self.directory):
                 app = FileApp(self.directory)
@@ -26,7 +29,7 @@ class CacheableStaticURLParser(StaticURLParser):
                     app.cache_control(max_age=int(self.cache_seconds))
                 return app(environ, start_response)
             return self.add_slash(environ, start_response)
-        if path_info == "/":
+        elif path_info == "/":
             # @@: This should obviously be configurable
             filename = "index.html"
         else:
@@ -36,7 +39,7 @@ class CacheableStaticURLParser(StaticURLParser):
         host = environ.get("HTTP_HOST")
         if self.directory_per_host and host:
             for host_key, host_val in self.directory_per_host.items():
-                if host_key in host:
+                if host_key == host:
                     directory = host_val
                     break
 

test/unit/tool_util/test_tool_linters.py

@@ -585,6 +585,12 @@ OUTPUTS_DUPLICATED_NAME_LABEL = """
     <outputs>
         <data name="valid_name" format="fasta"/>
         <data name="valid_name" format="fasta"/>
+        <data name="another_valid_name" format="fasta" label="same label may be OK if there is a filter">
+            <filter>a condition</filter>
+        </data>
+        <data name="yet_another_valid_name" format="fasta" label="same label may be OK if there is a filter">
+            <filter>another condition</filter>
+        </data>
     </outputs>
 </tool>
 """
@@ -1525,13 +1531,17 @@ def test_outputs_discover_tool_provided_metadata(lint_ctx):
 def test_outputs_duplicated_name_label(lint_ctx):
     tool_xml_tree = get_xml_tree(OUTPUTS_DUPLICATED_NAME_LABEL)
     run_lint(lint_ctx, outputs.lint_output, tool_xml_tree)
-    assert "2 outputs found." in lint_ctx.info_messages
+    assert "4 outputs found." in lint_ctx.info_messages
     assert len(lint_ctx.info_messages) == 1
     assert not lint_ctx.valid_messages
-    assert not lint_ctx.warn_messages
+    assert len(lint_ctx.warn_messages) == 2
+    assert "Tool output [valid_name] uses duplicated label '${tool.name} on ${on_string}'" in lint_ctx.warn_messages
+    assert (
+        "Tool output [yet_another_valid_name] uses duplicated label 'same label may be OK if there is a filter', double check if filters imply disjoint cases"
+        in lint_ctx.warn_messages
+    )
     assert "Tool output [valid_name] has duplicated name" in lint_ctx.error_messages
-    assert "Tool output [valid_name] uses duplicated label '${tool.name} on ${on_string}'" in lint_ctx.error_messages
-    assert len(lint_ctx.error_messages) == 2
+    assert len(lint_ctx.error_messages) == 1
 
 
 def test_stdio_default_for_default_profile(lint_ctx):