From 88bb5475014c4dcfac21bff3648e6d3e00d1495b Mon Sep 17 00:00:00 2001 From: John Chilton Date: Thu, 4 May 2023 16:37:26 -0400 Subject: [PATCH 1/6] Convert DescribeObjectStore to composition+ts --- .../ObjectStore/DescribeObjectStore.test.js | 4 +- .../ObjectStore/DescribeObjectStore.vue | 71 ++++++++----------- 2 files changed, 32 insertions(+), 43 deletions(-) diff --git a/client/src/components/ObjectStore/DescribeObjectStore.test.js b/client/src/components/ObjectStore/DescribeObjectStore.test.js index 1c1acff73753..95b300eec9c5 100644 --- a/client/src/components/ObjectStore/DescribeObjectStore.test.js +++ b/client/src/components/ObjectStore/DescribeObjectStore.test.js @@ -66,7 +66,7 @@ describe("DescribeObjectStore.vue", () => { expect(byIdSpan.length).toBe(1); const byNameSpan = wrapper.findAll(".display-os-by-name"); expect(byNameSpan.length).toBe(0); - expect(wrapper.find("object-store-restriction-span-stub").props("isPrivate")).toBeFalsy(); + expect(wrapper.vm.isPrivate).toBeFalsy(); }); it("test dataset storage with object store name", async () => { @@ -78,6 +78,6 @@ describe("DescribeObjectStore.vue", () => { expect(byIdSpan.length).toBe(0); const byNameSpan = wrapper.findAll(".display-os-by-name"); expect(byNameSpan.length).toBe(1); - expect(wrapper.find("object-store-restriction-span-stub").props("isPrivate")).toBeTruthy(); + expect(wrapper.vm.isPrivate).toBeTruthy(); }); }); diff --git a/client/src/components/ObjectStore/DescribeObjectStore.vue b/client/src/components/ObjectStore/DescribeObjectStore.vue index b36573c0e239..bed3aa0b1617 100644 --- a/client/src/components/ObjectStore/DescribeObjectStore.vue +++ b/client/src/components/ObjectStore/DescribeObjectStore.vue @@ -1,3 +1,33 @@ + + - - From f7ea877b53b3f30a2447dc21f284dca5dac9e91d Mon Sep 17 00:00:00 2001 From: John Chilton Date: Thu, 4 May 2023 17:43:07 -0400 Subject: [PATCH 2/6] Refactor object store configuration for reuse. --- .../ObjectStore/ConfigurationMarkdown.test.ts | 33 +++++++++++++++++++ .../ObjectStore/ConfigurationMarkdown.vue | 18 ++++++++++ .../ObjectStore/DescribeObjectStore.test.js | 23 ++++--------- .../ObjectStore/DescribeObjectStore.vue | 6 ++-- .../ObjectStore/ObjectStoreBadge.vue | 6 ++-- .../src/components/ObjectStore/adminConfig.ts | 11 ------- .../ObjectStore/configurationMarkdown.ts | 12 +++++++ 7 files changed, 75 insertions(+), 34 deletions(-) create mode 100644 client/src/components/ObjectStore/ConfigurationMarkdown.test.ts create mode 100644 client/src/components/ObjectStore/ConfigurationMarkdown.vue delete mode 100644 client/src/components/ObjectStore/adminConfig.ts create mode 100644 client/src/components/ObjectStore/configurationMarkdown.ts diff --git a/client/src/components/ObjectStore/ConfigurationMarkdown.test.ts b/client/src/components/ObjectStore/ConfigurationMarkdown.test.ts new file mode 100644 index 000000000000..1dffa81663fa --- /dev/null +++ b/client/src/components/ObjectStore/ConfigurationMarkdown.test.ts @@ -0,0 +1,33 @@ +import { shallowMount } from "@vue/test-utils"; +import { getLocalVue } from "tests/jest/helpers"; +import ConfigurationMarkdown from "./ConfigurationMarkdown.vue"; + +const localVue = getLocalVue(); + +describe("ConfigurationMarkdown.vue", () => { + let wrapper; + + it("should convert supplied configuration markup to markdown and display", () => { + wrapper = shallowMount(ConfigurationMarkdown, { + propsData: { markdown: "the *content*", admin: true }, + localVue, + }); + expect(wrapper.html()).toContain("content"); + }); + + it("should allow HTML in configuration markdup explicitly set by the admin", () => { + wrapper = shallowMount(ConfigurationMarkdown, { + propsData: { markdown: "the content", admin: true }, + localVue, + }); + expect(wrapper.html()).toContain("content"); + }); + + it("should escape supplied HTML for non-admin sourced content", () => { + wrapper = shallowMount(ConfigurationMarkdown, { + propsData: { markdown: "the content", admin: false }, + localVue, + }); + expect(wrapper.html()).not.toContain("content"); + }); +}); diff --git a/client/src/components/ObjectStore/ConfigurationMarkdown.vue b/client/src/components/ObjectStore/ConfigurationMarkdown.vue new file mode 100644 index 000000000000..d9fa3d321f6e --- /dev/null +++ b/client/src/components/ObjectStore/ConfigurationMarkdown.vue @@ -0,0 +1,18 @@ + + diff --git a/client/src/components/ObjectStore/DescribeObjectStore.test.js b/client/src/components/ObjectStore/DescribeObjectStore.test.js index 95b300eec9c5..04059f13ae9c 100644 --- a/client/src/components/ObjectStore/DescribeObjectStore.test.js +++ b/client/src/components/ObjectStore/DescribeObjectStore.test.js @@ -1,40 +1,32 @@ import { shallowMount } from "@vue/test-utils"; import DescribeObjectStore from "./DescribeObjectStore"; import { getLocalVue } from "tests/jest/helpers"; -import MarkdownIt from "markdown-it"; const localVue = getLocalVue(); +const DESCRIPTION = "My cool **markdown**"; + const TEST_STORAGE_API_RESPONSE_WITHOUT_ID = { object_store_id: null, private: false, + description: DESCRIPTION, badges: [], }; -const TEST_RENDERED_MARKDOWN_AS_HTML = "

My cool markdown\n"; const TEST_STORAGE_API_RESPONSE_WITH_ID = { object_store_id: "foobar", private: false, + description: DESCRIPTION, badges: [], }; const TEST_STORAGE_API_RESPONSE_WITH_NAME = { object_store_id: "foobar", name: "my cool storage", - description: "My cool **markdown**", + description: DESCRIPTION, private: true, badges: [], }; -// works fine without mocking but I guess it is more JS unit-y with the mock? -jest.mock("markdown-it"); -MarkdownIt.mockImplementation(() => { - return { - render(markdown) { - return TEST_RENDERED_MARKDOWN_AS_HTML; - }, - }; -}); - describe("DescribeObjectStore.vue", () => { let wrapper; @@ -48,7 +40,6 @@ describe("DescribeObjectStore.vue", () => { it("test dataset storage with object store without id", async () => { await mountWithResponse(TEST_STORAGE_API_RESPONSE_WITHOUT_ID); expect(wrapper.findAll("loading-span-stub").length).toBe(0); - expect(wrapper.vm.descriptionRendered).toBeNull(); const byIdSpan = wrapper.findAll(".display-os-by-id"); expect(byIdSpan.length).toBe(0); const byNameSpan = wrapper.findAll(".display-os-by-name"); @@ -61,7 +52,6 @@ describe("DescribeObjectStore.vue", () => { await mountWithResponse(TEST_STORAGE_API_RESPONSE_WITH_ID); expect(wrapper.findAll("loading-span-stub").length).toBe(0); expect(wrapper.vm.storageInfo.object_store_id).toBe("foobar"); - expect(wrapper.vm.descriptionRendered).toBeNull(); const byIdSpan = wrapper.findAll(".display-os-by-id"); expect(byIdSpan.length).toBe(1); const byNameSpan = wrapper.findAll(".display-os-by-name"); @@ -73,11 +63,12 @@ describe("DescribeObjectStore.vue", () => { await mountWithResponse(TEST_STORAGE_API_RESPONSE_WITH_NAME); expect(wrapper.findAll("loading-span-stub").length).toBe(0); expect(wrapper.vm.storageInfo.object_store_id).toBe("foobar"); - expect(wrapper.vm.descriptionRendered).toBe(TEST_RENDERED_MARKDOWN_AS_HTML); const byIdSpan = wrapper.findAll(".display-os-by-id"); expect(byIdSpan.length).toBe(0); const byNameSpan = wrapper.findAll(".display-os-by-name"); expect(byNameSpan.length).toBe(1); expect(wrapper.vm.isPrivate).toBeTruthy(); + const configurationMarkupEl = wrapper.find("[markdown]"); + expect(configurationMarkupEl.attributes("markdown")).toBe(DESCRIPTION); }); }); diff --git a/client/src/components/ObjectStore/DescribeObjectStore.vue b/client/src/components/ObjectStore/DescribeObjectStore.vue index bed3aa0b1617..fbcbf409bad4 100644 --- a/client/src/components/ObjectStore/DescribeObjectStore.vue +++ b/client/src/components/ObjectStore/DescribeObjectStore.vue @@ -3,7 +3,7 @@ import ObjectStoreRestrictionSpan from "./ObjectStoreRestrictionSpan.vue"; import QuotaUsageBar from "@/components/User/DiskUsage/Quota/QuotaUsageBar.vue"; import { QuotaSourceUsageProvider } from "@/components/User/DiskUsage/Quota/QuotaUsageProvider.js"; import ObjectStoreBadges from "./ObjectStoreBadges.vue"; -import { adminMarkup } from "./adminConfig"; +import ConfigurationMarkdown from "./ConfigurationMarkdown.vue"; import type { components } from "@/schema"; import { computed } from "vue"; @@ -18,12 +18,10 @@ interface Props { const props = defineProps(); const quotaSourceLabel = computed(() => props.storageInfo.quota?.source); -const descriptionRendered = computed(() => adminMarkup(props.storageInfo.description || "")); const isPrivate = computed(() => props.storageInfo.private); const badges = computed(() => props.storageInfo.badges); defineExpose({ - descriptionRendered, isPrivate, }); @@ -53,6 +51,6 @@ defineExpose({

Galaxy has no quota configured for this object store.
-
+ diff --git a/client/src/components/ObjectStore/ObjectStoreBadge.vue b/client/src/components/ObjectStore/ObjectStoreBadge.vue index 2d1e1e77da0c..af0c3d4b66d1 100644 --- a/client/src/components/ObjectStore/ObjectStoreBadge.vue +++ b/client/src/components/ObjectStore/ObjectStoreBadge.vue @@ -1,6 +1,6 @@ @@ -135,7 +135,7 @@ const message = computed(() => { placement="bottom" class="object-store-badge-popover">

{{ stockMessage }}

-
+ diff --git a/client/src/components/ObjectStore/adminConfig.ts b/client/src/components/ObjectStore/adminConfig.ts deleted file mode 100644 index 5230ef7c7aa3..000000000000 --- a/client/src/components/ObjectStore/adminConfig.ts +++ /dev/null @@ -1,11 +0,0 @@ -import MarkdownIt from "markdown-it"; - -export function adminMarkup(markup: string): string | null { - let markupHtml; - if (markup) { - markupHtml = MarkdownIt({ html: true }).render(markup); - } else { - markupHtml = null; - } - return markupHtml; -} diff --git a/client/src/components/ObjectStore/configurationMarkdown.ts b/client/src/components/ObjectStore/configurationMarkdown.ts new file mode 100644 index 000000000000..7fd29651681d --- /dev/null +++ b/client/src/components/ObjectStore/configurationMarkdown.ts @@ -0,0 +1,12 @@ +import MarkdownIt from "markdown-it"; + +export function markup(markup: string, adminConfigured: boolean): string | null { + let markupHtml; + const allowHtml = adminConfigured ? true : false; + if (markup) { + markupHtml = MarkdownIt({ html: allowHtml }).render(markup); + } else { + markupHtml = null; + } + return markupHtml; +} From f649dc9a669e50e60517155a60f52909fec46b7b Mon Sep 17 00:00:00 2001 From: John Chilton Date: Fri, 5 May 2023 10:30:39 -0400 Subject: [PATCH 3/6] Small refactorings toward composables and away from providers in object store stuff. --- .../ObjectStore/DescribeObjectStore.vue | 6 +-- .../ShowSelectedObjectStore.test.js | 17 ++----- .../ObjectStore/ShowSelectedObjectStore.vue | 47 ++++++++++++++----- client/src/components/ObjectStore/services.ts | 7 +++ client/src/components/ObjectStore/types.ts | 3 ++ .../providers/ObjectStoreProvider.js | 16 ------- 6 files changed, 52 insertions(+), 44 deletions(-) create mode 100644 client/src/components/ObjectStore/types.ts delete mode 100644 client/src/components/providers/ObjectStoreProvider.js diff --git a/client/src/components/ObjectStore/DescribeObjectStore.vue b/client/src/components/ObjectStore/DescribeObjectStore.vue index fbcbf409bad4..21774c6908ab 100644 --- a/client/src/components/ObjectStore/DescribeObjectStore.vue +++ b/client/src/components/ObjectStore/DescribeObjectStore.vue @@ -4,12 +4,10 @@ import QuotaUsageBar from "@/components/User/DiskUsage/Quota/QuotaUsageBar.vue"; import { QuotaSourceUsageProvider } from "@/components/User/DiskUsage/Quota/QuotaUsageProvider.js"; import ObjectStoreBadges from "./ObjectStoreBadges.vue"; import ConfigurationMarkdown from "./ConfigurationMarkdown.vue"; -import type { components } from "@/schema"; +import type { ConcreteObjectStoreModel } from "./types"; import { computed } from "vue"; -type ConcreteObjectStoreModel = components["schemas"]["ConcreteObjectStoreModel"]; - interface Props { storageInfo: ConcreteObjectStoreModel; what: string; @@ -51,6 +49,6 @@ defineExpose({
Galaxy has no quota configured for this object store.
- + diff --git a/client/src/components/ObjectStore/ShowSelectedObjectStore.test.js b/client/src/components/ObjectStore/ShowSelectedObjectStore.test.js index 38fbb9a8a71f..757fca263e01 100644 --- a/client/src/components/ObjectStore/ShowSelectedObjectStore.test.js +++ b/client/src/components/ObjectStore/ShowSelectedObjectStore.test.js @@ -3,31 +3,24 @@ import { getLocalVue } from "tests/jest/helpers"; import ShowSelectedObjectStore from "./ShowSelectedObjectStore"; import LoadingSpan from "@/components/LoadingSpan.vue"; import DescribeObjectStore from "@/components/ObjectStore/DescribeObjectStore.vue"; -import axios from "axios"; -import MockAdapter from "axios-mock-adapter"; import flushPromises from "flush-promises"; +import { mockFetcher } from "@/schema/__mocks__"; + +jest.mock("@/schema"); const localVue = getLocalVue(true); const TEST_OBJECT_ID = "os123"; const OBJECT_STORE_DATA = { + description: null, object_store_id: TEST_OBJECT_ID, badges: [], }; describe("ShowSelectedObjectStore", () => { let wrapper; - let axiosMock; - - beforeEach(async () => { - axiosMock = new MockAdapter(axios); - }); - - afterEach(async () => { - axiosMock.restore(); - }); it("should show a loading message and then a DescribeObjectStore component", async () => { - axiosMock.onGet(`/api/object_stores/${TEST_OBJECT_ID}`).reply(200, OBJECT_STORE_DATA); + mockFetcher.path("/api/object_stores/{object_store_id}").method("get").mock({ data: OBJECT_STORE_DATA }); wrapper = mount(ShowSelectedObjectStore, { propsData: { preferredObjectStoreId: TEST_OBJECT_ID, forWhat: "Data goes into..." }, localVue, diff --git a/client/src/components/ObjectStore/ShowSelectedObjectStore.vue b/client/src/components/ObjectStore/ShowSelectedObjectStore.vue index c8a6e01a6700..c3b52aaf351b 100644 --- a/client/src/components/ObjectStore/ShowSelectedObjectStore.vue +++ b/client/src/components/ObjectStore/ShowSelectedObjectStore.vue @@ -1,25 +1,48 @@ diff --git a/client/src/components/ObjectStore/services.ts b/client/src/components/ObjectStore/services.ts index 23c0f4e3b3fc..cd9e48068b3d 100644 --- a/client/src/components/ObjectStore/services.ts +++ b/client/src/components/ObjectStore/services.ts @@ -6,3 +6,10 @@ export async function getSelectableObjectStores() { const { data } = await getObjectStores({ selectable: true }); return data; } + +const getObjectStore = fetcher.path("/api/object_stores/{object_store_id}").method("get").create(); + +export async function getObjectStoreDetails(id: string) { + const { data } = await getObjectStore({ object_store_id: id }); + return data; +} diff --git a/client/src/components/ObjectStore/types.ts b/client/src/components/ObjectStore/types.ts new file mode 100644 index 000000000000..4d7f943eba20 --- /dev/null +++ b/client/src/components/ObjectStore/types.ts @@ -0,0 +1,3 @@ +import type { components } from "@/schema"; + +export type ConcreteObjectStoreModel = components["schemas"]["ConcreteObjectStoreModel"]; diff --git a/client/src/components/providers/ObjectStoreProvider.js b/client/src/components/providers/ObjectStoreProvider.js deleted file mode 100644 index f9c797dc8a3b..000000000000 --- a/client/src/components/providers/ObjectStoreProvider.js +++ /dev/null @@ -1,16 +0,0 @@ -import axios from "axios"; -import { getAppRoot } from "onload/loadConfig"; -import { SingleQueryProvider } from "components/providers/SingleQueryProvider"; -import { rethrowSimple } from "utils/simple-error"; - -async function objectStoreDetails({ id }) { - const url = `${getAppRoot()}api/object_stores/${id}`; - try { - const { data } = await axios.get(url); - return data; - } catch (e) { - rethrowSimple(e); - } -} - -export const ObjectStoreDetailsProvider = SingleQueryProvider(objectStoreDetails); From 348fe3561f754dd4fc0fd99914b013e937f581b0 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Fri, 5 May 2023 10:58:05 -0400 Subject: [PATCH 4/6] Bug fix related to initialization in UserPreferences.vue. --- client/src/components/User/UserPreferences.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/src/components/User/UserPreferences.vue b/client/src/components/User/UserPreferences.vue index aaed1a89f8a9..40847c225e8f 100644 --- a/client/src/components/User/UserPreferences.vue +++ b/client/src/components/User/UserPreferences.vue @@ -88,7 +88,7 @@ From 31a295ca1b50e6d893b7859c0a76a0bddb9deb9b Mon Sep 17 00:00:00 2001 From: John Chilton Date: Fri, 5 May 2023 11:16:22 -0400 Subject: [PATCH 5/6] Fix for potentially empty badge messages. --- .../src/components/ObjectStore/ObjectStoreBadge.test.js | 9 +++++++++ client/src/components/ObjectStore/ObjectStoreBadge.vue | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/client/src/components/ObjectStore/ObjectStoreBadge.test.js b/client/src/components/ObjectStore/ObjectStoreBadge.test.js index d3db27538c93..fdfc6080dbfc 100644 --- a/client/src/components/ObjectStore/ObjectStoreBadge.test.js +++ b/client/src/components/ObjectStore/ObjectStoreBadge.test.js @@ -39,4 +39,13 @@ describe("ObjectStoreBadge", () => { expect(popoverText).toContain(TEST_MESSAGE); expect(popoverText).toContain("less secure by the Galaxy administrator"); }); + + it("should gracefully unspecified badge messages", async () => { + mountBadge({ type: "more_secure", message: null }); + const selector = ROOT_COMPONENT.object_store_details.badge_of_type({ type: "more_secure" }).selector; + const iconEl = wrapper.find(selector); + expect(iconEl.exists()).toBeTruthy(); + const popoverStub = wrapper.find("b-popover-stub"); + expect(popoverStub.exists()).toBe(true); + }); }); diff --git a/client/src/components/ObjectStore/ObjectStoreBadge.vue b/client/src/components/ObjectStore/ObjectStoreBadge.vue index af0c3d4b66d1..af7830f7ca7a 100644 --- a/client/src/components/ObjectStore/ObjectStoreBadge.vue +++ b/client/src/components/ObjectStore/ObjectStoreBadge.vue @@ -135,7 +135,7 @@ const message = computed(() => { placement="bottom" class="object-store-badge-popover">

{{ stockMessage }}

- + From 671e8191ee677c46ddfbffa1681d1639aa89e45e Mon Sep 17 00:00:00 2001 From: John Chilton Date: Mon, 15 May 2023 09:33:14 -0400 Subject: [PATCH 6/6] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: David López <46503462+davelopez@users.noreply.github.com> --- client/src/components/ObjectStore/ConfigurationMarkdown.vue | 2 +- client/src/components/ObjectStore/ObjectStoreBadge.test.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/client/src/components/ObjectStore/ConfigurationMarkdown.vue b/client/src/components/ObjectStore/ConfigurationMarkdown.vue index d9fa3d321f6e..15c870afb037 100644 --- a/client/src/components/ObjectStore/ConfigurationMarkdown.vue +++ b/client/src/components/ObjectStore/ConfigurationMarkdown.vue @@ -6,7 +6,7 @@ interface Props { admin: boolean; // was the configuration specified by an admin } const props = defineProps(); -const markdownHtml = computed(() => markup(props.markdown || "", props.admin)); +const markdownHtml = computed(() => markup(props.markdown ?? "", props.admin));