diff --git a/client/src/app/monitor.js b/client/src/app/monitor.js index e4e86d2d830..f74015a9219 100644 --- a/client/src/app/monitor.js +++ b/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()); diff --git a/client/src/components/History/Content/ContentItem.vue b/client/src/components/History/Content/ContentItem.vue index 090dbf5bad1..79e4cf5aad0 100644 --- a/client/src/components/History/Content/ContentItem.vue +++ b/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"; diff --git a/client/src/components/History/Content/Dataset/DatasetActions.vue b/client/src/components/History/Content/Dataset/DatasetActions.vue index bf0f4bad48e..f0a3697ae24 100644 --- a/client/src/components/History/Content/Dataset/DatasetActions.vue +++ b/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"); diff --git a/client/src/entry/analysis/router-push.js b/client/src/entry/analysis/router-push.js new file mode 100644 index 00000000000..e4f6b201483 --- /dev/null +++ b/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; + } + }); + }; +} diff --git a/client/src/entry/analysis/router-push.test.js b/client/src/entry/analysis/router-push.test.js new file mode 100644 index 00000000000..91b027a897d --- /dev/null +++ b/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.*`)); + }); +}); diff --git a/client/src/entry/analysis/router.js b/client/src/entry/analysis/router.js index d6ed64b85a5..0a9dd734ea2 100644 --- a/client/src/entry/analysis/router.js +++ b/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() { diff --git a/client/src/utils/url.js b/client/src/utils/url.js index 4c8b7598f71..98e16a2e56f 100644 --- a/client/src/utils/url.js +++ b/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; +} diff --git a/client/src/utils/url.test.js b/client/src/utils/url.test.js new file mode 100644 index 00000000000..6fcaf8bab99 --- /dev/null +++ b/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"); + }); +});