Merge pull request #14725 from guerler/fix_rerun_reload

Allow to force push client routes

client/src/app/monitor.js

@@ -19,7 +19,7 @@ if (!window.Galaxy) {
             if (!config.testBuild === true) {
                 console.warn("accessing (get) window.Galaxy", serverPath());
             }
-            return getGalaxyInstance() || galaxyStub;
+            return (getGalaxyInstance && getGalaxyInstance()) || galaxyStub;
         },
         set: function (newValue) {
             console.warn("accessing (set) window.Galaxy", serverPath());

client/src/components/History/Content/ContentItem.vue

@@ -203,7 +203,7 @@ export default {
             }
         },
         onDisplay() {
-            this.$router.push(this.itemUrls.display, this.name);
+            this.$router.push(this.itemUrls.display, { title: this.name });
         },
         onDragStart(evt) {
             evt.dataTransfer.dropEffect = "move";

client/src/components/History/Content/Dataset/DatasetActions.vue

@@ -136,11 +136,11 @@ export default {
             this.$router.push(this.itemUrls.showDetails);
         },
         onRerun() {
-            this.$router.push(`/root?job_id=${this.item.creating_job}`);
+            this.$router.push(`/root?job_id=${this.item.creating_job}`, { force: true });
         },
         onVisualize() {
             const title = `Visualization of ${this.item.name || ""}`;
-            this.$router.push(this.itemUrls.visualize, title);
+            this.$router.push(this.itemUrls.visualize, { title });
         },
         onHighlight() {
             this.$emit("toggleHighlights");

client/src/entry/analysis/router-push.js

@@ -0,0 +1,45 @@
+import { getGalaxyInstance } from "app";
+import { addSearchParams } from "utils/url";
+
+/**
+ * Is called before the regular router.push() and allows us to provide logs,
+ * handle the window manager, avoid duplication warnings, and force a component
+ * refresh if needed.
+ *
+ * @param {String} Location as parsed to original router.push()
+ * @param {Object} Custom options, to provide a title and/or force reload
+ */
+export function patchRouterPush(VueRouter) {
+    const originalPush = VueRouter.prototype.push;
+    VueRouter.prototype.push = function push(location, options = {}) {
+        // add key to location to force component refresh
+        const { title, force } = options;
+        if (force) {
+            location = addSearchParams(location, { __vkey__: Date.now() });
+        }
+        // log upcoming location
+        console.debug("VueRouter - push: ", location);
+        // verify if confirmation is required
+        if (this.confirmation) {
+            if (confirm("There are unsaved changes which will be lost.")) {
+                this.confirmation = undefined;
+            } else {
+                return;
+            }
+        }
+        // show location in window manager
+        const Galaxy = getGalaxyInstance();
+        if (title && Galaxy.frame && Galaxy.frame.active) {
+            Galaxy.frame.add({ title: title, url: location });
+            return;
+        }
+        // always emit event, even when a duplicate route is pushed
+        this.app.$emit("router-push");
+        // avoid console warning when user clicks to revisit same route
+        return originalPush.call(this, location).catch((err) => {
+            if (err.name !== "NavigationDuplicated") {
+                throw err;
+            }
+        });
+    };
+}

client/src/entry/analysis/router-push.test.js

@@ -0,0 +1,72 @@
+import { getGalaxyInstance } from "app/singleton";
+import { patchRouterPush } from "./router-push";
+
+// mock Galaxy object
+jest.mock("app/singleton");
+const mockGalaxy = {
+    frame: {
+        active: false,
+        add: jest.fn(),
+    },
+};
+getGalaxyInstance.mockImplementation(() => mockGalaxy);
+
+// router push handling tests
+describe("router push changes", () => {
+    it("pushing routes", async () => {
+        window.confirm = jest.fn();
+        let currentLocation = null;
+        const mockComplete = jest.fn();
+        const mockContext = {
+            confirmation: true,
+            app: {
+                $emit: jest.fn(),
+            },
+        };
+        const mockRouter = {
+            prototype: {
+                push: (location) => {
+                    currentLocation = location;
+                    return { catch: mockComplete };
+                },
+            },
+        };
+        patchRouterPush(mockRouter);
+        const push = mockRouter.prototype.push;
+        const results = mockComplete.mock.results;
+        const windowResults = mockGalaxy.frame.add.mock.results;
+        // route will be rejected while confirmation is required
+        push.call(mockContext, "/test/name");
+        expect(currentLocation).toBe(null);
+        expect(results.length).toBe(0);
+        expect(window.confirm.mock.calls[0][0]).toBe("There are unsaved changes which will be lost.");
+        // route should properly parse to original push
+        mockContext.confirmation = false;
+        push.call(mockContext, "/test/other");
+        expect(currentLocation).toBe("/test/other");
+        expect(results.length).toBe(1);
+        // route should properly parse to original push despite title
+        const title = "test title";
+        push.call(mockContext, "/test/something", { title });
+        expect(currentLocation).toBe("/test/something");
+        expect(results.length).toBe(2);
+        // route should be handled by calling the window manager
+        mockGalaxy.frame.active = true;
+        push.call(mockContext, "/test/tryagain", { title });
+        expect(currentLocation).toBe("/test/something");
+        push.call(mockContext, "/test/openanotherone", { title });
+        expect(results.length).toBe(2);
+        expect(windowResults.length).toBe(2);
+        // route should be handled by router again
+        mockGalaxy.frame.active = false;
+        push.call(mockContext, "/test/regularagain", { title });
+        expect(currentLocation).toBe("/test/regularagain");
+        push.call(mockContext, "/test/openanotherone", { title });
+        expect(results.length).toBe(4);
+        expect(windowResults.length).toBe(2);
+        // force route should modify location by adding key
+        mockGalaxy.frame.active = false;
+        push.call(mockContext, "/test/forceroute", { force: true });
+        expect(currentLocation).toMatch(new RegExp(`/test/forceroute?.*vkey.*`));
+    });
+});

client/src/entry/analysis/router.js

@@ -2,6 +2,7 @@ import Vue from "vue";
 import VueRouter from "vue-router";
 import { getAppRoot } from "onload/loadConfig";
 import { getGalaxyInstance } from "app";
+import { patchRouterPush } from "./router-push";
 
 // these modules are mounted below the masthead.
 import Analysis from "entry/analysis/modules/Analysis";
@@ -59,32 +60,7 @@ import { StorageDashboardRouter } from "components/User/DiskUsage";
 Vue.use(VueRouter);
 
 // patches $router.push() to trigger an event and hide duplication warnings
-const originalPush = VueRouter.prototype.push;
-VueRouter.prototype.push = function push(location, windowManagerTitle = null) {
-    // verify if confirmation is required
-    console.debug("VueRouter - push: ", location);
-    if (this.confirmation) {
-        if (confirm("There are unsaved changes which will be lost.")) {
-            this.confirmation = undefined;
-        } else {
-            return;
-        }
-    }
-    // show location in window manager
-    const Galaxy = getGalaxyInstance();
-    if (windowManagerTitle && Galaxy.frame && Galaxy.frame.active) {
-        Galaxy.frame.add({ title: windowManagerTitle, url: location });
-        return;
-    }
-    // always emit event when a route is pushed
-    this.app.$emit("router-push");
-    // avoid console warning when user clicks to revisit same route
-    return originalPush.call(this, location).catch((err) => {
-        if (err.name !== "NavigationDuplicated") {
-            throw err;
-        }
-    });
-};
+patchRouterPush(VueRouter);
 
 // redirect anon users
 function redirectAnon() {

client/src/utils/url.js

@@ -13,3 +13,17 @@ export async function urlData({ url, headers, params }) {
         rethrowSimple(e);
     }
 }
+
+/**
+ * Adds search parameters to url.
+ *
+ * @param {String} original url
+ * @param {Object} params which will be added to the url
+ * @returns
+ */
+export function addSearchParams(url, params) {
+    const placeholder = url.indexOf("?") == -1 ? "?" : "&";
+    const searchParams = new URLSearchParams(params);
+    const searchString = searchParams.toString();
+    return searchString ? `${url}${placeholder}${searchString}` : url;
+}

client/src/utils/url.test.js

@@ -0,0 +1,9 @@
+import { addSearchParams } from "./url";
+
+describe("test url utilities", () => {
+    it("adding parameters to url", async () => {
+        expect(addSearchParams("/test?name=value")).toBe("/test?name=value");
+        expect(addSearchParams("/test", { name: "value", and: "this" })).toBe("/test?name=value&and=this");
+        expect(addSearchParams("/test?exists=value", { name: "value" })).toBe("/test?exists=value&name=value");
+    });
+});