Merge pull request #22246 from davelopez/26.0_fix_file_source_removal

[26.0] Fix file source removal

client/src/components/FileSources/Instances/InstanceDropdown.test.ts

@@ -0,0 +1,97 @@
+import { getLocalVue } from "@tests/vitest/helpers";
+import { shallowMount } from "@vue/test-utils";
+import flushPromises from "flush-promises";
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+import { useServerMock } from "@/api/client/__mocks__";
+import type { UserFileSourceModel } from "@/api/fileSources";
+
+import FileSourceInstanceDropdown from "./InstanceDropdown.vue";
+import ConfigurationInstanceDropdown from "@/components/ConfigTemplates/InstanceDropdown.vue";
+
+const { toastErrorMock } = vi.hoisted(() => ({
+    toastErrorMock: vi.fn(),
+}));
+
+vi.mock("@/composables/toast", () => ({
+    Toast: {
+        error: toastErrorMock,
+    },
+}));
+
+vi.mock("@/stores/fileSourceTemplatesStore", () => ({
+    useFileSourceTemplatesStore: () => ({
+        canUpgrade: () => false,
+    }),
+}));
+
+const { server, http } = useServerMock();
+
+const FILE_SOURCE: UserFileSourceModel = {
+    uuid: "test-file-source-id",
+    name: "Test File Source",
+    description: null,
+    template_id: "simple_variable",
+    template_version: 0,
+    active: true,
+    hidden: false,
+    purged: false,
+    secrets: [],
+    type: "posix",
+    uri_root: "gxfiles://test-file-source-id",
+    variables: null,
+};
+
+describe("File Source Instance Dropdown", () => {
+    const localVue = getLocalVue(true);
+
+    beforeEach(() => {
+        server.resetHandlers();
+        toastErrorMock.mockClear();
+    });
+
+    it("emits entryRemoved when remove succeeds", async () => {
+        let requestBody: Record<string, unknown> | undefined;
+        server.use(
+            http.put("/api/file_source_instances/{uuid}", async ({ request, response }) => {
+                requestBody = (await request.json()) as Record<string, unknown>;
+                return response(200).json(FILE_SOURCE);
+            }),
+        );
+
+        const wrapper = shallowMount(FileSourceInstanceDropdown as object, {
+            localVue,
+            propsData: {
+                fileSource: FILE_SOURCE,
+            },
+        });
+
+        wrapper.findComponent(ConfigurationInstanceDropdown).vm.$emit("remove");
+        await flushPromises();
+
+        expect(requestBody).toEqual({ hidden: true });
+        expect(wrapper.emitted("entryRemoved")).toBeTruthy();
+        expect(toastErrorMock).not.toHaveBeenCalled();
+    });
+
+    it("shows toast and does not emit entryRemoved when remove fails", async () => {
+        server.use(
+            http.put("/api/file_source_instances/{uuid}", ({ response }) => {
+                return response("4XX").json({ err_code: 400, err_msg: "Unable to remove" }, { status: 400 });
+            }),
+        );
+
+        const wrapper = shallowMount(FileSourceInstanceDropdown as object, {
+            localVue,
+            propsData: {
+                fileSource: FILE_SOURCE,
+            },
+        });
+
+        wrapper.findComponent(ConfigurationInstanceDropdown).vm.$emit("remove");
+        await flushPromises();
+
+        expect(toastErrorMock).toHaveBeenCalledWith("Unable to remove", "Failed to remove instance");
+        expect(wrapper.emitted("entryRemoved")).toBeFalsy();
+    });
+});

client/src/components/FileSources/Instances/InstanceDropdown.vue

@@ -3,8 +3,9 @@ import { computed } from "vue";
 
 import { GalaxyApi } from "@/api";
 import type { UserFileSourceModel } from "@/api/fileSources";
+import { Toast } from "@/composables/toast";
 import { useFileSourceTemplatesStore } from "@/stores/fileSourceTemplatesStore";
-import { rethrowSimple } from "@/utils/simple-error";
+import { errorMessageAsString } from "@/utils/simple-error";
 
 import InstanceDropdown from "@/components/ConfigTemplates/InstanceDropdown.vue";
 
@@ -30,7 +31,8 @@ async function onRemove() {
     });
 
     if (error) {
-        rethrowSimple(error);
+        Toast.error(errorMessageAsString(error, "Failed to remove instance."), "Failed to remove instance");
+        return;
     }
 
     emit("entryRemoved");

client/src/components/ObjectStore/Instances/InstanceDropdown.test.ts

@@ -0,0 +1,103 @@
+import { getLocalVue } from "@tests/vitest/helpers";
+import { shallowMount } from "@vue/test-utils";
+import flushPromises from "flush-promises";
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+import { useServerMock } from "@/api/client/__mocks__";
+
+import type { UserConcreteObjectStore } from "./types";
+
+import ObjectStoreInstanceDropdown from "./InstanceDropdown.vue";
+import ConfigurationInstanceDropdown from "@/components/ConfigTemplates/InstanceDropdown.vue";
+
+const { toastErrorMock } = vi.hoisted(() => ({
+    toastErrorMock: vi.fn(),
+}));
+
+vi.mock("@/composables/toast", () => ({
+    Toast: {
+        error: toastErrorMock,
+    },
+}));
+
+vi.mock("@/stores/objectStoreTemplatesStore", () => ({
+    useObjectStoreTemplatesStore: () => ({
+        canUpgrade: () => false,
+    }),
+}));
+
+const { server, http } = useServerMock();
+
+const OBJECT_STORE: UserConcreteObjectStore = {
+    uuid: "test-object-store-id",
+    name: "Test Object Store",
+    description: null,
+    template_id: "simple_variable",
+    template_version: 0,
+    active: true,
+    hidden: false,
+    purged: false,
+    private: false,
+    object_store_id: null,
+    device: null,
+    object_expires_after_days: null,
+    badges: [],
+    quota: { enabled: false },
+    secrets: [],
+    type: "disk",
+    variables: null,
+};
+
+describe("Object Store Instance Dropdown", () => {
+    const localVue = getLocalVue(true);
+
+    beforeEach(() => {
+        server.resetHandlers();
+        toastErrorMock.mockClear();
+    });
+
+    it("emits entryRemoved when remove succeeds", async () => {
+        let requestBody: Record<string, unknown> | undefined;
+        server.use(
+            http.put("/api/object_store_instances/{uuid}", async ({ request, response }) => {
+                requestBody = (await request.json()) as Record<string, unknown>;
+                return response(200).json(OBJECT_STORE);
+            }),
+        );
+
+        const wrapper = shallowMount(ObjectStoreInstanceDropdown as object, {
+            localVue,
+            propsData: {
+                objectStore: OBJECT_STORE,
+            },
+        });
+
+        wrapper.findComponent(ConfigurationInstanceDropdown).vm.$emit("remove");
+        await flushPromises();
+
+        expect(requestBody).toEqual({ hidden: true });
+        expect(wrapper.emitted("entryRemoved")).toBeTruthy();
+        expect(toastErrorMock).not.toHaveBeenCalled();
+    });
+
+    it("shows toast and does not emit entryRemoved when remove fails", async () => {
+        server.use(
+            http.put("/api/object_store_instances/{uuid}", ({ response }) => {
+                return response("4XX").json({ err_code: 400, err_msg: "Unable to remove" }, { status: 400 });
+            }),
+        );
+
+        const wrapper = shallowMount(ObjectStoreInstanceDropdown as object, {
+            localVue,
+            propsData: {
+                objectStore: OBJECT_STORE,
+            },
+        });
+
+        wrapper.findComponent(ConfigurationInstanceDropdown).vm.$emit("remove");
+        await flushPromises();
+
+        expect(toastErrorMock).toHaveBeenCalledWith("Unable to remove", "Failed to remove instance");
+        expect(wrapper.emitted("entryRemoved")).toBeFalsy();
+    });
+});

client/src/components/ObjectStore/Instances/InstanceDropdown.vue

@@ -2,8 +2,9 @@
 import { computed } from "vue";
 
 import { GalaxyApi } from "@/api";
+import { Toast } from "@/composables/toast";
 import { useObjectStoreTemplatesStore } from "@/stores/objectStoreTemplatesStore";
-import { rethrowSimple } from "@/utils/simple-error";
+import { errorMessageAsString } from "@/utils/simple-error";
 
 import type { UserConcreteObjectStore } from "./types";
 
@@ -29,7 +30,8 @@ async function onRemove() {
     });
 
     if (error) {
-        rethrowSimple(error);
+        Toast.error(errorMessageAsString(error, "Failed to remove instance."), "Failed to remove instance");
+        return;
     }
 
     emit("entryRemoved");

lib/galaxy/managers/_config_templates.py

@@ -296,8 +296,9 @@ def update_template_instance(
     payload: UpdateInstancePayload,
     template: Template,
 ):
-    validate_specified_datatypes_variables(payload.variables or {}, template)
-    validate_no_extra_variables_defined(payload.variables or {}, template)
+    if payload.variables is not None:
+        validate_specified_datatypes_variables(payload.variables, template)
+        validate_no_extra_variables_defined(payload.variables, template)
     if payload.name is not None:
         template_instance.name = payload.name
     if payload.description is not None:

test/unit/app/managers/test_user_file_sources.py

@@ -480,6 +480,26 @@ class TestFileSourcesTestCase(BaseTestCase):
         assert user_file_source_showed.variables
         assert user_file_source_showed.variables["var1"] == "newval"
 
+    def test_hide_without_variables_update_on_required_variable_template(self, tmp_path):
+        self._init_managers(tmp_path, config_dict=simple_variable_template(tmp_path))
+        create_payload = CreateInstancePayload(
+            name=SIMPLE_FILE_SOURCE_NAME,
+            description=SIMPLE_FILE_SOURCE_DESCRIPTION,
+            template_id="simple_variable",
+            template_version=0,
+            variables={"var1": "requiredval"},
+            secrets={},
+        )
+        user_file_source = self._create_instance(create_payload)
+
+        hide = UpdateInstancePayload(hidden=True)
+        self._modify(user_file_source, hide)
+
+        user_file_source_showed = self.manager.show(self.trans, user_file_source.uuid)
+        assert user_file_source_showed.hidden
+        assert user_file_source_showed.variables
+        assert user_file_source_showed.variables["var1"] == "requiredval"
+
     def test_hide(self, tmp_path):
         user_file_source = self._init_and_create_simple(tmp_path)
 

test/unit/app/managers/test_user_object_stores.py

@@ -1,6 +1,4 @@
-from typing import (
-    Optional,
-)
+from typing import Optional
 
 from yaml import safe_load
 
@@ -150,6 +148,26 @@ class TestUserObjectStoreTestCase(BaseTestCase):
         assert user_object_store_showed.variables
         assert user_object_store_showed.variables["var1"] == "newval"
 
+    def test_hide_without_variables_update_on_required_variable_template(self, tmp_path):
+        self._init_managers(tmp_path, config_dict=simple_variable_template(tmp_path))
+        create_payload = CreateInstancePayload(
+            name=SIMPLE_FILE_SOURCE_NAME,
+            description=SIMPLE_FILE_SOURCE_DESCRIPTION,
+            template_id="simple_variable",
+            template_version=0,
+            variables={"var1": "requiredval"},
+            secrets={},
+        )
+        user_object_store = self._create_instance(create_payload)
+
+        hide = UpdateInstancePayload(hidden=True)
+        self._modify(user_object_store, hide)
+
+        user_object_store_showed = self.manager.show(self.trans, user_object_store.uuid)
+        assert user_object_store_showed.hidden
+        assert user_object_store_showed.variables
+        assert user_object_store_showed.variables["var1"] == "requiredval"
+
     def test_update_errors_on_extra_variables(self, tmp_path):
         self._init_managers(tmp_path, config_dict=simple_variable_template(tmp_path))
         create_payload = CreateInstancePayload(