Merge pull request #18645 from davelopez/improve_user_types

Improve types around User in schema and client
galaxyproject · Aug 6, 2024 · 4f20fbf · 4f20fbf
2 parents db9ff9e + 85dd962
commit 4f20fbf
Show file tree

Hide file tree

Showing 28 changed files with 168 additions and 163 deletions.
diff --git a/client/src/api/index.test.ts b/client/src/api/index.test.ts
@@ -1,28 +1,24 @@
+import { getFakeRegisteredUser } from "@tests/test-data";
+
 import {
     type AnonymousUser,
     type AnyHistory,
     type HistorySummary,
     type HistorySummaryExtended,
     isRegisteredUser,
-    type User,
     userOwnsHistory,
 } from ".";
 
 const REGISTERED_USER_ID = "fake-user-id";
 const ANOTHER_USER_ID = "another-fake-user-id";
 const ANONYMOUS_USER_ID = null;
 
-const REGISTERED_USER: User = {
-    id: REGISTERED_USER_ID,
-    email: "[email protected]",
-    tags_used: [],
-    isAnonymous: false,
-    total_disk_usage: 0,
-};
+const REGISTERED_USER = getFakeRegisteredUser({ id: REGISTERED_USER_ID });
 
 const ANONYMOUS_USER: AnonymousUser = {
     isAnonymous: true,
     total_disk_usage: 0,
+    nice_total_disk_usage: "0.0 bytes",
 };
 
 const SESSIONLESS_USER = null;

diff --git a/client/src/api/index.ts b/client/src/api/index.ts
@@ -215,36 +215,31 @@ export function isHistoryItem(item: object): item is HistoryItemSummary {
     return item && "history_content_type" in item;
 }
 
-type QuotaUsageResponse = components["schemas"]["UserQuotaUsage"];
+type RegisteredUserModel = components["schemas"]["DetailedUserModel"];
+type AnonymousUserModel = components["schemas"]["AnonUserModel"];
+type UserModel = RegisteredUserModel | AnonymousUserModel;
 
-/** Represents a registered user.**/
-export interface User extends QuotaUsageResponse {
-    id: string;
-    email: string;
-    tags_used: string[];
+export interface RegisteredUser extends RegisteredUserModel {
     isAnonymous: false;
-    is_admin?: boolean;
-    username?: string;
 }
 
-export interface AnonymousUser extends QuotaUsageResponse {
-    id?: string;
+export interface AnonymousUser extends AnonymousUserModel {
     isAnonymous: true;
-    is_admin?: false;
-    username?: string;
 }
 
-export type GenericUser = User | AnonymousUser;
-
 /** Represents any user, including anonymous users or session-less (null) users.**/
-export type AnyUser = GenericUser | null;
+export type AnyUser = RegisteredUser | AnonymousUser | null;
+
+export function isRegisteredUser(user: AnyUser | UserModel): user is RegisteredUser {
+    return user !== null && "email" in user;
+}
 
-export function isRegisteredUser(user: AnyUser): user is User {
-    return user !== null && !user?.isAnonymous;
+export function isAnonymousUser(user: AnyUser | UserModel): user is AnonymousUser {
+    return user !== null && !isRegisteredUser(user);
 }
 
-export function isAnonymousUser(user: AnyUser): user is AnonymousUser {
-    return user !== null && user.isAnonymous;
+export function isAdminUser(user: AnyUser | UserModel): user is RegisteredUser {
+    return isRegisteredUser(user) && user.is_admin;
 }
 
 export function userOwnsHistory(user: AnyUser, history: AnyHistory) {

diff --git a/client/src/api/schema/schema.ts b/client/src/api/schema/schema.ts
@@ -2187,7 +2187,7 @@ export interface components {
              * Quota percent
              * @description Percentage of the storage quota applicable to the user.
              */
-            quota_percent?: unknown;
+            quota_percent?: number | null;
             /**
              * Total disk usage
              * @description Size of all non-purged, unique datasets of the user in bytes.
@@ -4613,12 +4613,12 @@ export interface components {
              * Quota in bytes
              * @description Quota applicable to the user in bytes.
              */
-            quota_bytes: unknown;
+            quota_bytes?: number | null;
             /**
              * Quota percent
              * @description Percentage of the storage quota applicable to the user.
              */
-            quota_percent?: unknown;
+            quota_percent?: number | null;
             /**
              * Tags used
              * @description Tags used by the user

diff --git a/client/src/api/workflows.ts b/client/src/api/workflows.ts
@@ -9,3 +9,16 @@ export const invocationCountsFetcher = fetcher.path("/api/workflows/{workflow_id
 
 export const sharing = fetcher.path("/api/workflows/{workflow_id}/sharing").method("get").create();
 export const enableLink = fetcher.path("/api/workflows/{workflow_id}/enable_link_access").method("put").create();
+
+//TODO: replace with generated schema model when available
+export interface WorkflowSummary {
+    name: string;
+    owner: string;
+    [key: string]: unknown;
+    update_time: string;
+    license?: string;
+    tags?: string[];
+    creator?: {
+        [key: string]: unknown;
+    }[];
+}
diff --git a/client/src/components/DatasetInformation/DatasetError.test.ts b/client/src/components/DatasetInformation/DatasetError.test.ts
@@ -1,3 +1,4 @@
+import { getFakeRegisteredUser } from "@tests/test-data";
 import { mount } from "@vue/test-utils";
 import flushPromises from "flush-promises";
 import { createPinia } from "pinia";
@@ -57,13 +58,7 @@ async function montDatasetError(has_duplicate_inputs = true, has_empty_inputs =
     });
 
     const userStore = useUserStore();
-    userStore.currentUser = {
-        email: user_email || "email",
-        id: "user_id",
-        tags_used: [],
-        isAnonymous: false,
-        total_disk_usage: 0,
-    };
+    userStore.currentUser = getFakeRegisteredUser({ email: user_email });
 
     await flushPromises();
 

diff --git a/client/src/components/History/CurrentHistory/HistoryMessages.vue b/client/src/components/History/CurrentHistory/HistoryMessages.vue
@@ -5,14 +5,14 @@ import { FontAwesomeIcon } from "@fortawesome/vue-fontawesome";
 import { BAlert } from "bootstrap-vue";
 import { computed, ref } from "vue";
 
-import { type GenericUser, type HistorySummary, userOwnsHistory } from "@/api";
+import { type AnyUser, type HistorySummary, userOwnsHistory } from "@/api";
 import localize from "@/utils/localization";
 
 library.add(faArchive, faBurn, faTrash);
 
 interface Props {
     history: HistorySummary;
-    currentUser: GenericUser | null;
+    currentUser: AnyUser;
 }
 
 const props = defineProps<Props>();

diff --git a/client/src/components/History/HistoryView.test.js b/client/src/components/History/HistoryView.test.js
@@ -1,3 +1,4 @@
+import { getFakeRegisteredUser } from "@tests/test-data";
 import { mount } from "@vue/test-utils";
 import axios from "axios";
 import MockAdapter from "axios-mock-adapter";
@@ -76,10 +77,7 @@ async function createWrapper(localVue, currentUserId, history) {
         pinia,
     });
     const userStore = useUserStore();
-    const userData = {
-        id: currentUserId,
-    };
-    userStore.currentUser = { ...userStore.currentUser, ...userData };
+    userStore.currentUser = getFakeRegisteredUser({ id: currentUserId });
     await flushPromises();
     return wrapper;
 }

diff --git a/client/src/components/History/Multiple/MultipleView.test.js b/client/src/components/History/Multiple/MultipleView.test.js
@@ -1,3 +1,4 @@
+import { getFakeRegisteredUser } from "@tests/test-data";
 import { mount } from "@vue/test-utils";
 import axios from "axios";
 import MockAdapter from "axios-mock-adapter";
@@ -20,7 +21,8 @@ const getFakeHistorySummaries = (num, selectedIndex) => {
         update_time: new Date().toISOString(),
     }));
 };
-const currentUser = { id: USER_ID };
+
+const currentUser = getFakeRegisteredUser({ id: USER_ID });
 
 describe("MultipleView", () => {
     let axiosMock;

diff --git a/client/src/components/JobDestinationParams/JobDestinationParams.test.ts b/client/src/components/JobDestinationParams/JobDestinationParams.test.ts
@@ -1,3 +1,4 @@
+import { getFakeRegisteredUser } from "@tests/test-data";
 import { shallowMount } from "@vue/test-utils";
 import flushPromises from "flush-promises";
 import { createPinia } from "pinia";
@@ -31,14 +32,7 @@ async function mountJobDestinationParams() {
     });
 
     const userStore = useUserStore();
-    userStore.currentUser = {
-        email: "admin@email",
-        id: "1",
-        tags_used: [],
-        isAnonymous: false,
-        total_disk_usage: 1048576,
-        is_admin: true,
-    };
+    userStore.currentUser = getFakeRegisteredUser({ is_admin: true });
 
     await flushPromises();
 

diff --git a/client/src/components/Masthead/Masthead.test.js b/client/src/components/Masthead/Masthead.test.js
@@ -1,10 +1,12 @@
 import { createTestingPinia } from "@pinia/testing";
+import { getFakeRegisteredUser } from "@tests/test-data";
 import { mount } from "@vue/test-utils";
 import { WindowManager } from "layout/window-manager";
 import { PiniaVuePlugin } from "pinia";
 import { getLocalVue } from "tests/jest/helpers";
 
 import { mockFetcher } from "@/api/schema/__mocks__";
+import { useUserStore } from "@/stores/userStore";
 
 import { loadWebhookMenuItems } from "./_webhooks";
 
@@ -18,6 +20,8 @@ jest.mock("vue-router/composables", () => ({
 }));
 jest.mock("@/api/schema");
 
+const currentUser = getFakeRegisteredUser();
+
 describe("Masthead.vue", () => {
     let wrapper;
     let localVue;
@@ -42,6 +46,10 @@ describe("Masthead.vue", () => {
 
         windowManager = new WindowManager({});
         const windowTab = windowManager.getTab();
+
+        const userStore = useUserStore();
+        userStore.currentUser = currentUser;
+
         wrapper = mount(Masthead, {
             propsData: {
                 windowTab,

diff --git a/client/src/components/Masthead/QuotaMeter.test.ts b/client/src/components/Masthead/QuotaMeter.test.ts
@@ -1,8 +1,10 @@
 import { createTestingPinia } from "@pinia/testing";
+import { getFakeRegisteredUser } from "@tests/test-data";
 import { mount } from "@vue/test-utils";
 import flushPromises from "flush-promises";
 import { getLocalVue } from "tests/jest/helpers";
 
+import { type RegisteredUser } from "@/api";
 import { useUserStore } from "@/stores/userStore";
 
 import QuotaMeter from "./QuotaMeter.vue";
@@ -20,11 +22,11 @@ jest.mock("@/composables/config", () => ({
 
 const localVue = getLocalVue();
 
-async function createQuotaMeterWrapper(config: any, userData: any) {
+async function createQuotaMeterWrapper(config: any, user: RegisteredUser) {
     configValues = { ...config };
     const pinia = createTestingPinia();
     const userStore = useUserStore();
-    userStore.currentUser = { ...userStore.currentUser, ...userData };
+    userStore.currentUser = user;
     const wrapper = mount(QuotaMeter, {
         localVue,
         pinia,
@@ -33,55 +35,49 @@ async function createQuotaMeterWrapper(config: any, userData: any) {
     return wrapper;
 }
 
+const FAKE_USER = getFakeRegisteredUser({ quota: "100 MB", total_disk_usage: 5120, quota_percent: 50 });
+
 describe("QuotaMeter.vue", () => {
     it("shows a percentage usage", async () => {
-        const user = {
-            total_disk_usage: 5120,
-            quota_percent: 50,
-            quota: "100 MB",
-        };
         const config = { enable_quotas: true };
-        const wrapper = await createQuotaMeterWrapper(config, user);
+        const wrapper = await createQuotaMeterWrapper(config, FAKE_USER);
         expect(wrapper.find(".quota-progress > span").text()).toBe("Using 50% of 100 MB");
     });
 
     it("changes appearance depending on usage", async () => {
         const config = { enable_quotas: true };
         {
-            const user = { quota_percent: 30 };
+            const user = { ...FAKE_USER, quota_percent: 30 };
             const wrapper = await createQuotaMeterWrapper(config, user);
             expect(wrapper.find(".quota-progress .progress-bar").classes()).toContain("bg-success");
         }
         {
-            const user = { quota_percent: 80 };
+            const user = { ...FAKE_USER, quota_percent: 80 };
             const wrapper = await createQuotaMeterWrapper(config, user);
             expect(wrapper.find(".quota-progress .progress-bar").classes()).toContain("bg-warning");
         }
         {
-            const user = { quota_percent: 95 };
+            const user = { ...FAKE_USER, quota_percent: 95 };
             const wrapper = await createQuotaMeterWrapper(config, user);
             expect(wrapper.find(".quota-progress .progress-bar").classes()).toContain("bg-danger");
         }
     });
 
     it("displays tooltip", async () => {
         const config = { enable_quotas: true };
-        const wrapper = await createQuotaMeterWrapper(config, {});
+        const wrapper = await createQuotaMeterWrapper(config, FAKE_USER);
         expect(wrapper.attributes("title")).toContain("Storage");
     });
 
     it("shows total usage when there is no quota", async () => {
         {
-            const user = { total_disk_usage: 7168 };
+            const user = { ...FAKE_USER, total_disk_usage: 7168 };
             const config = { enable_quotas: false };
             const wrapper = await createQuotaMeterWrapper(config, user);
             expect(wrapper.find("span").text()).toBe("Using 7 KB");
         }
         {
-            const user = {
-                total_disk_usage: 21504,
-                quota: "unlimited",
-            };
+            const user = { ...FAKE_USER, total_disk_usage: 21504, quota: "unlimited" };
             const config = { enable_quotas: true };
             const wrapper = await createQuotaMeterWrapper(config, user);
             expect(wrapper.find("span").text()).toBe("Using 21 KB");

diff --git a/client/src/components/Masthead/QuotaMeter.vue b/client/src/components/Masthead/QuotaMeter.vue
@@ -3,20 +3,21 @@ import { BLink, BProgress, BProgressBar } from "bootstrap-vue";
 import { storeToRefs } from "pinia";
 import { computed } from "vue";
 
+import { isRegisteredUser } from "@/api";
 import { useConfig } from "@/composables/config";
 import { useUserStore } from "@/stores/userStore";
 import { bytesToString } from "@/utils/utils";
 
 const { config } = useConfig();
 const { currentUser, isAnonymous } = storeToRefs(useUserStore());
 
-const hasQuota = computed(() => {
+const hasQuota = computed<boolean>(() => {
     const quotasEnabled = config.value.enable_quotas ?? false;
-    const quotaLimited = currentUser.value?.quota !== "unlimited" ?? false;
+    const quotaLimited = (isRegisteredUser(currentUser.value) && currentUser.value.quota !== "unlimited") ?? false;
     return quotasEnabled && quotaLimited;
 });
 
-const quotaLimit = computed(() => currentUser.value?.quota ?? 0);
+const quotaLimit = computed(() => (isRegisteredUser(currentUser.value) && currentUser.value.quota) ?? null);
 
 const totalUsageString = computed(() => {
     const total = currentUser.value?.total_disk_usage ?? 0;
@@ -56,7 +57,7 @@ const variant = computed(() => {
                 <span v-localize>Using</span>
                 <span v-if="hasQuota">
                     <span>{{ usage.toFixed(0) }}%</span>
-                    <span v-if="quotaLimit !== 0">of {{ quotaLimit }}</span>
+                    <span v-if="quotaLimit !== null">of {{ quotaLimit }}</span>
                 </span>
                 <span v-else>{{ totalUsageString }}</span>
             </span>

diff --git a/client/src/components/User/DiskUsage/DiskUsageSummary.test.ts b/client/src/components/User/DiskUsage/DiskUsageSummary.test.ts
@@ -1,3 +1,4 @@
+import { getFakeRegisteredUser } from "@tests/test-data";
 import { mount } from "@vue/test-utils";
 import flushPromises from "flush-promises";
 import { createPinia } from "pinia";
@@ -19,16 +20,11 @@ const localVue = getLocalVue();
 const quotaUsageClassSelector = ".quota-usage";
 const basicDiskUsageSummaryId = "#basic-disk-usage-summary";
 
-const fakeUserWithQuota = {
-    id: "fakeUser",
-    email: "fakeUserEmail",
-    tags_used: [],
-    isAnonymous: false,
+const fakeUserWithQuota = getFakeRegisteredUser({
     total_disk_usage: 1048576,
     quota_bytes: 104857600,
     quota_percent: 1,
-    quota_source_label: "Default",
-};
+});
 
 // TODO: Replace this with a mockFetcher when #16608 is merged
 const mockGetCurrentUser = getCurrentUser as jest.Mock;