Merge pull request #16051 from jmchilton/refactor_object_store_compon…

…ents

Refactor a few client object store components

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("<em>content</em>");
+    });
+
+    it("should allow HTML in configuration markdup explicitly set by the admin", () => {
+        wrapper = shallowMount(ConfigurationMarkdown, {
+            propsData: { markdown: "the <b>content</b>", admin: true },
+            localVue,
+        });
+        expect(wrapper.html()).toContain("<b>content</b>");
+    });
+
+    it("should escape supplied HTML for non-admin sourced content", () => {
+        wrapper = shallowMount(ConfigurationMarkdown, {
+            propsData: { markdown: "the <b>content</b>", admin: false },
+            localVue,
+        });
+        expect(wrapper.html()).not.toContain("<b>content</b>");
+    });
+});

client/src/components/ObjectStore/ConfigurationMarkdown.vue

@@ -0,0 +1,18 @@
+<script setup lang="ts">
+import { computed } from "vue";
+import { markup } from "./configurationMarkdown";
+interface Props {
+    markdown: string;
+    admin: boolean; // was the configuration specified by an admin
+}
+const props = defineProps<Props>();
+const markdownHtml = computed(() => markup(props.markdown ?? "", props.admin));
+</script>
+<template>
+    <!-- Disable v-html warning because we allow markdown generated HTML
+         in various places in the Galaxy interface. Raw HTML will be
+         excluded from markdown for all user generated content.
+    -->
+    <!-- eslint-disable-next-line vue/no-v-html -->
+    <div v-html="markdownHtml" />
+</template>

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 = "<p>My cool <strong>markdown</strong>\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,23 +52,23 @@ 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");
         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 () => {
         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.find("object-store-restriction-span-stub").props("isPrivate")).toBeTruthy();
+        expect(wrapper.vm.isPrivate).toBeTruthy();
+        const configurationMarkupEl = wrapper.find("[markdown]");
+        expect(configurationMarkupEl.attributes("markdown")).toBe(DESCRIPTION);
     });
 });

client/src/components/ObjectStore/DescribeObjectStore.vue

@@ -1,3 +1,29 @@
+<script setup lang="ts">
+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 ConfigurationMarkdown from "./ConfigurationMarkdown.vue";
+import type { ConcreteObjectStoreModel } from "./types";
+
+import { computed } from "vue";
+
+interface Props {
+    storageInfo: ConcreteObjectStoreModel;
+    what: string;
+}
+
+const props = defineProps<Props>();
+
+const quotaSourceLabel = computed(() => props.storageInfo.quota?.source);
+const isPrivate = computed(() => props.storageInfo.private);
+const badges = computed(() => props.storageInfo.badges);
+
+defineExpose({
+    isPrivate,
+});
+</script>
+
 <template>
     <div>
         <div>
@@ -23,47 +49,6 @@
             <QuotaUsageBar v-else-if="quotaUsage" :quota-usage="quotaUsage" :embedded="true" />
         </QuotaSourceUsageProvider>
         <div v-else>Galaxy has no quota configured for this object store.</div>
-        <div v-html="descriptionRendered"></div>
+        <ConfigurationMarkdown v-if="storageInfo.description" :markdown="storageInfo.description" :admin="true" />
     </div>
 </template>
-
-<script>
-import ObjectStoreRestrictionSpan from "./ObjectStoreRestrictionSpan";
-import QuotaUsageBar from "components/User/DiskUsage/Quota/QuotaUsageBar";
-import { QuotaSourceUsageProvider } from "components/User/DiskUsage/Quota/QuotaUsageProvider";
-import ObjectStoreBadges from "./ObjectStoreBadges";
-import { adminMarkup } from "./adminConfig";
-
-export default {
-    components: {
-        ObjectStoreBadges,
-        ObjectStoreRestrictionSpan,
-        QuotaSourceUsageProvider,
-        QuotaUsageBar,
-    },
-    props: {
-        storageInfo: {
-            type: Object,
-            required: true,
-        },
-        what: {
-            type: String,
-            required: true,
-        },
-    },
-    computed: {
-        quotaSourceLabel() {
-            return this.storageInfo.quota?.source;
-        },
-        descriptionRendered() {
-            return adminMarkup(this.storageInfo.description);
-        },
-        isPrivate() {
-            return this.storageInfo.private;
-        },
-        badges() {
-            return this.storageInfo.badges;
-        },
-    },
-};
-</script>

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 handle 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);
+    });
 });

client/src/components/ObjectStore/ObjectStoreBadge.vue

@@ -1,6 +1,6 @@
 <script lang="ts" setup>
 import { computed } from "vue";
-import { adminMarkup } from "./adminConfig";
+import ConfigurationMarkdown from "./ConfigurationMarkdown.vue";
 import { FontAwesomeIcon, FontAwesomeLayers } from "@fortawesome/vue-fontawesome";
 import type { components } from "@/schema";
 import "./badgeIcons";
@@ -62,7 +62,7 @@ const shrink = computed(() => {
 });
 
 const message = computed(() => {
-    return adminMarkup(props.badge.message);
+    return props.badge.message;
 });
 </script>
 
@@ -135,7 +135,7 @@ const message = computed(() => {
             placement="bottom"
             class="object-store-badge-popover">
             <p v-localize>{{ stockMessage }}</p>
-            <div v-html="message"></div>
+            <ConfigurationMarkdown v-if="message" :markdown="message" :admin="true" />
         </b-popover>
     </span>
 </template>

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,

client/src/components/ObjectStore/ShowSelectedObjectStore.vue

@@ -1,25 +1,48 @@
 <script setup lang="ts">
 import LoadingSpan from "@/components/LoadingSpan.vue";
-import { ObjectStoreDetailsProvider } from "@/components/providers/ObjectStoreProvider";
 import DescribeObjectStore from "@/components/ObjectStore/DescribeObjectStore.vue";
+import { getObjectStoreDetails } from "./services";
+import { watch, ref } from "vue";
+import type { ConcreteObjectStoreModel } from "./types";
+import { errorMessageAsString } from "@/utils/simple-error";
 
 interface ShowSelectObjectStoreProps {
-    forWhat: String;
-    preferredObjectStoreId?: String | null;
+    forWhat: string;
+    preferredObjectStoreId: string;
 }
 
-withDefaults(defineProps<ShowSelectObjectStoreProps>(), {
-    preferredObjectStoreId: null,
-});
+const props = defineProps<ShowSelectObjectStoreProps>();
 
+const objectStore = ref<ConcreteObjectStoreModel | null>(null);
+const loading = ref(true);
+const error = ref<string | null>(null);
+
+async function fetch() {
+    loading.value = true;
+    try {
+        objectStore.value = await getObjectStoreDetails(props.preferredObjectStoreId);
+    } catch (e) {
+        error.value = errorMessageAsString(e);
+    } finally {
+        loading.value = false;
+    }
+}
+
+watch(
+    () => props.preferredObjectStoreId,
+    async () => {
+        fetch();
+    }
+);
+fetch();
 const loadingMessage = "Loading object store details";
 </script>
 
 <template>
-    <ObjectStoreDetailsProvider
-        :id="preferredObjectStoreId"
-        v-slot="{ result: storageInfo, loading: isLoadingStorageInfo }">
-        <LoadingSpan v-if="isLoadingStorageInfo" :message="loadingMessage | localize" />
-        <DescribeObjectStore v-else :what="forWhat" :storage-info="storageInfo"> </DescribeObjectStore>
-    </ObjectStoreDetailsProvider>
+    <div>
+        <LoadingSpan v-if="loading" :message="loadingMessage | localize" />
+        <DescribeObjectStore v-else-if="objectStore != null" :what="forWhat" :storage-info="objectStore">
+        </DescribeObjectStore>
+        <b-alert v-else-if="error" show variant="danger">{{ error }}</b-alert>
+    </div>
 </template>

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;
+}

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;
+}

client/src/components/ObjectStore/types.ts

@@ -0,0 +1,3 @@
+import type { components } from "@/schema";
+
+export type ConcreteObjectStoreModel = components["schemas"]["ConcreteObjectStoreModel"];