Skip to content

Commit

Permalink
[24.1] Add error handling for SwitchToHistoryLink
Browse files Browse the repository at this point in the history
Yet another case where errors were not being handled; in this case if the user clicks on the `SwitchToHistoryLink` when the history isn't shared/importable, we now `Toast` an error message.

Error currently reproducable if you click on the history name in this invocation: https://usegalaxy.org/workflows/invocations/a11d3f54288f2b34?from_panel=true
  • Loading branch information
ahmedhamidawan committed Sep 3, 2024
1 parent 478e989 commit 987bbf9
Show file tree
Hide file tree
Showing 3 changed files with 124 additions and 18 deletions.
98 changes: 89 additions & 9 deletions client/src/components/History/SwitchToHistoryLink.test.ts
Original file line number Diff line number Diff line change
@@ -1,27 +1,45 @@
import { createTestingPinia } from "@pinia/testing";
import { getLocalVue } from "@tests/jest/helpers";
import { shallowMount } from "@vue/test-utils";
import { mount } from "@vue/test-utils";
import axios from "axios";
import MockAdapter from "axios-mock-adapter";
import flushPromises from "flush-promises";

import { type HistorySummary } from "@/api";
import { type HistorySummaryExtended } from "@/api";
import { Toast } from "@/composables/toast";
import { useUserStore } from "@/stores/userStore";

import SwitchToHistoryLink from "./SwitchToHistoryLink.vue";

jest.mock("@/composables/toast", () => ({
Toast: {
error: jest.fn(),
},
}));
jest.mock("vue-router/composables", () => ({
useRouter: jest.fn(() => ({
resolve: jest.fn((route) => ({
href: route,
})),
})),
}));

const localVue = getLocalVue(true);

const selectors = {
historyLink: ".history-link",
historyLinkClick: ".history-link > a",
} as const;

function mountSwitchToHistoryLinkForHistory(history: HistorySummary) {
function mountSwitchToHistoryLinkForHistory(history: HistorySummaryExtended) {
const windowSpy = jest.spyOn(window, "open");
windowSpy.mockImplementation(() => null);
const pinia = createTestingPinia();

const axiosMock = new MockAdapter(axios);
axiosMock.onGet(`/api/histories/${history.id}`).reply(200, history);

const wrapper = shallowMount(SwitchToHistoryLink as object, {
const wrapper = mount(SwitchToHistoryLink as object, {
propsData: {
historyId: history.id,
},
Expand All @@ -31,17 +49,28 @@ function mountSwitchToHistoryLinkForHistory(history: HistorySummary) {
FontAwesomeIcon: true,
},
});

const userStore = useUserStore();
userStore.currentUser = {
email: "email",
id: "user_id",
tags_used: [],
isAnonymous: false,
total_disk_usage: 0,
};
return wrapper;
}

async function expectOptionForHistory(option: string, history: HistorySummary) {
async function expectOptionForHistory(option: string, history: HistorySummaryExtended) {
const wrapper = mountSwitchToHistoryLinkForHistory(history);

// Wait for the history to be loaded
await flushPromises();

expect(wrapper.html()).toContain(option);
expect(wrapper.text()).toContain(history.name);

return wrapper;
}

describe("SwitchToHistoryLink", () => {
Expand All @@ -52,7 +81,7 @@ describe("SwitchToHistoryLink", () => {
deleted: false,
archived: false,
purged: false,
} as HistorySummary;
} as HistorySummaryExtended;
const wrapper = mountSwitchToHistoryLinkForHistory(history);

expect(wrapper.find(selectors.historyLink).exists()).toBe(false);
Expand All @@ -72,7 +101,8 @@ describe("SwitchToHistoryLink", () => {
deleted: false,
purged: false,
archived: false,
} as HistorySummary;
user_id: "user_id",
} as HistorySummaryExtended;
await expectOptionForHistory("Switch", history);
});

Expand All @@ -83,7 +113,8 @@ describe("SwitchToHistoryLink", () => {
deleted: false,
purged: true,
archived: false,
} as HistorySummary;
user_id: "user_id",
} as HistorySummaryExtended;
await expectOptionForHistory("View", history);
});

Expand All @@ -94,7 +125,56 @@ describe("SwitchToHistoryLink", () => {
deleted: false,
purged: false,
archived: true,
} as HistorySummary;
user_id: "user_id",
} as HistorySummaryExtended;
await expectOptionForHistory("View", history);
});

it("should view in new tab when the history is unowned and published", async () => {
const history = {
id: "unowned-history-id",
name: "History Published",
deleted: false,
purged: false,
archived: false,
published: true,
user_id: "other_user_id",
} as HistorySummaryExtended;
const wrapper = await expectOptionForHistory("View", history);

// Wait for the history to be loaded
await flushPromises();

// Click on the history link
const link = wrapper.find(selectors.historyLinkClick);
await link.trigger("click");
await wrapper.vm.$nextTick();

// History opens in a new tab
expect(Toast.error).toHaveBeenCalledTimes(0);
});

it("should display View option and show error on click when history is unowned and not published", async () => {
const history = {
id: "published-history-id",
name: "History Unowned",
deleted: false,
purged: false,
archived: false,
user_id: "other_user_id",
} as HistorySummaryExtended;
const wrapper = await expectOptionForHistory("View", history);

// Wait for the history to be loaded
await flushPromises();

// Click on the history link
const link = wrapper.find(selectors.historyLinkClick);
await link.trigger("click");
await wrapper.vm.$nextTick();

// History does not open in a new tab and we get a Toast error
expect(Toast.error).toHaveBeenCalledTimes(1);
expect(Toast.error).toHaveBeenCalledWith("You do not have permission to view this history.");
});
});
34 changes: 28 additions & 6 deletions client/src/components/History/SwitchToHistoryLink.vue
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,19 @@ import { BLink } from "bootstrap-vue";
import { computed } from "vue";
import { useRouter } from "vue-router/composables";
import { HistorySummary } from "@/api";
import { type HistorySummary, userOwnsHistory } from "@/api";
import { Toast } from "@/composables/toast";
import { useHistoryStore } from "@/stores/historyStore";
import { useUserStore } from "@/stores/userStore";
import { errorMessageAsString } from "@/utils/simple-error";
import LoadingSpan from "@/components/LoadingSpan.vue";
library.add(faArchive, faBurn);
const router = useRouter();
const historyStore = useHistoryStore();
const userStore = useUserStore();
interface Props {
historyId: string;
Expand All @@ -25,7 +29,13 @@ const props = defineProps<Props>();
const history = computed(() => historyStore.getHistoryById(props.historyId));
const canSwitch = computed(() => !!history.value && !history.value.archived && !history.value.purged);
const canSwitch = computed(
() =>
!!history.value &&
!history.value.archived &&
!history.value.purged &&
userOwnsHistory(userStore.currentUser, history.value)
);
const actionText = computed(() => {
if (canSwitch.value) {
Expand All @@ -37,21 +47,33 @@ const actionText = computed(() => {
return "View in new tab";
});
function onClick(history: HistorySummary) {
async function onClick(history: HistorySummary) {
if (canSwitch.value) {
if (props.filters) {
historyStore.applyFilters(history.id, props.filters);
} else {
historyStore.setCurrentHistory(history.id);
try {
await historyStore.setCurrentHistory(history.id);
} catch (error) {
Toast.error(errorMessageAsString(error));
}
}
return;
}
viewHistoryInNewTab(history);
}
function viewHistoryInNewTab(history: HistorySummary) {
const routeData = router.resolve(`/histories/view?id=${history.id}`);
window.open(routeData.href, "_blank");
if (
userOwnsHistory(userStore.currentUser, history) ||
history.published ||
("importable" in history && history.importable)
) {
const routeData = router.resolve(`/histories/view?id=${history.id}`);
window.open(routeData.href, "_blank");
} else {
Toast.error("You do not have permission to view this history.");
}
}
</script>

Expand Down
10 changes: 7 additions & 3 deletions client/src/stores/historyStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,13 @@ export const useHistoryStore = defineStore("historyStore", () => {
});

async function setCurrentHistory(historyId: string) {
const currentHistory = (await setCurrentHistoryOnServer(historyId)) as HistoryDevDetailed;
selectHistory(currentHistory);
setFilterText(historyId, "");
try {
const currentHistory = (await setCurrentHistoryOnServer(historyId)) as HistoryDevDetailed;
selectHistory(currentHistory);
setFilterText(historyId, "");
} catch (error) {
rethrowSimple(error);
}
}

function setCurrentHistoryId(historyId: string) {
Expand Down

0 comments on commit 987bbf9

Please sign in to comment.