From 5a3d8cbdeeda2193a6eeb058fc405b6f8f607aba Mon Sep 17 00:00:00 2001
From: davelopez <46503462+davelopez@users.noreply.github.com>
Date: Mon, 5 Aug 2024 09:22:22 +0200
Subject: [PATCH 1/4] Improve quota field types in schema

---
 client/src/api/schema/schema.ts | 6 +++---
 lib/galaxy/schema/schema.py     | 6 ++++--
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/client/src/api/schema/schema.ts b/client/src/api/schema/schema.ts
index fd0b318f8b0b..2720918058d5 100644
--- 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/lib/galaxy/schema/schema.py b/lib/galaxy/schema/schema.py
index 3653890f4968..e782cdb82092 100644
--- a/lib/galaxy/schema/schema.py
+++ b/lib/galaxy/schema/schema.py
@@ -362,7 +362,7 @@ class CreatedUserModel(UserModel, DiskUsageUserModel):
 
 
 class AnonUserModel(DiskUsageUserModel):
-    quota_percent: Any = QuotaPercentField
+    quota_percent: Optional[float] = QuotaPercentField
 
 
 class DetailedUserModel(BaseUserModel, AnonUserModel):
@@ -371,7 +371,9 @@ class DetailedUserModel(BaseUserModel, AnonUserModel):
     preferences: Dict[Any, Any] = Field(default=..., title="Preferences", description="Preferences of the user")
     preferred_object_store_id: Optional[str] = PreferredObjectStoreIdField
     quota: str = Field(default=..., title="Quota", description="Quota applicable to the user")
-    quota_bytes: Any = Field(default=..., title="Quota in bytes", description="Quota applicable to the user in bytes.")
+    quota_bytes: Optional[int] = Field(
+        default=None, title="Quota in bytes", description="Quota applicable to the user in bytes."
+    )
     tags_used: List[str] = Field(default=..., title="Tags used", description="Tags used by the user")
 
 

From a99d4e576a4baeaa7b90098c9877e0c6039b8729 Mon Sep 17 00:00:00 2001
From: davelopez <46503462+davelopez@users.noreply.github.com>
Date: Mon, 5 Aug 2024 10:21:02 +0200
Subject: [PATCH 2/4] Improve client types around user + refactoring

Refactor userStore method to use matchesCurrentUsername for ownership checks.
---
 client/src/api/index.ts                       | 28 ++++++++-----------
 client/src/api/workflows.ts                   | 13 +++++++++
 client/src/components/Masthead/QuotaMeter.vue |  9 +++---
 .../Workflow/List/WorkflowActions.vue         |  7 ++---
 .../Workflow/List/WorkflowActionsExtend.vue   |  7 ++---
 .../components/Workflow/List/WorkflowCard.vue |  8 ++----
 .../Workflow/List/WorkflowIndicators.vue      | 11 ++++----
 .../components/Workflow/List/WorkflowList.vue |  2 +-
 .../Published/WorkflowInformation.vue         | 20 ++-----------
 .../Workflow/Published/WorkflowPublished.vue  | 19 ++++---------
 .../components/Workflow/Run/WorkflowRun.vue   |  5 ++--
 .../components/Workflow/workflows.services.ts |  4 +--
 client/src/stores/userStore.ts                | 17 +++++++----
 13 files changed, 67 insertions(+), 83 deletions(-)

diff --git a/client/src/api/index.ts b/client/src/api/index.ts
index b27d9746a0f3..e22be0702139 100644
--- a/client/src/api/index.ts
+++ b/client/src/api/index.ts
@@ -215,36 +215,32 @@ 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"];
 
-/** 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;
+export type GenericUser = RegisteredUser | AnonymousUser;
 
 /** Represents any user, including anonymous users or session-less (null) users.**/
 export type AnyUser = GenericUser | null;
 
-export function isRegisteredUser(user: AnyUser): user is User {
-    return user !== null && !user?.isAnonymous;
+export function isRegisteredUser(user: AnyUser): user is RegisteredUser {
+    return user !== null && "email" in user;
 }
 
 export function isAnonymousUser(user: AnyUser): user is AnonymousUser {
-    return user !== null && user.isAnonymous;
+    return user !== null && !isRegisteredUser(user);
+}
+
+export function isAdminUser(user: AnyUser): user is RegisteredUser {
+    return isRegisteredUser(user) && user.is_admin;
 }
 
 export function userOwnsHistory(user: AnyUser, history: AnyHistory) {
diff --git a/client/src/api/workflows.ts b/client/src/api/workflows.ts
index 4d676cefcc94..af3fbd325502 100644
--- 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/Masthead/QuotaMeter.vue b/client/src/components/Masthead/QuotaMeter.vue
index fe6ad9e1fcff..5d3dc01ae72b 100644
--- a/client/src/components/Masthead/QuotaMeter.vue
+++ b/client/src/components/Masthead/QuotaMeter.vue
@@ -3,6 +3,7 @@ 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";
@@ -10,13 +11,13 @@ 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/Workflow/List/WorkflowActions.vue b/client/src/components/Workflow/List/WorkflowActions.vue
index a6acda9a75f6..91c5524c1ac4 100644
--- a/client/src/components/Workflow/List/WorkflowActions.vue
+++ b/client/src/components/Workflow/List/WorkflowActions.vue
@@ -44,12 +44,9 @@ const { confirm } = useConfirmDialog();
 const bookmarkLoading = ref(false);
 
 const shared = computed(() => {
-    if (userStore.currentUser) {
-        return userStore.currentUser.username !== props.workflow.owner;
-    } else {
-        return false;
-    }
+    return !userStore.matchesCurrentUsername(props.workflow.owner);
 });
+
 const sourceType = computed(() => {
     if (props.workflow.source_metadata?.url) {
         return "url";
diff --git a/client/src/components/Workflow/List/WorkflowActionsExtend.vue b/client/src/components/Workflow/List/WorkflowActionsExtend.vue
index 6968cc745ef7..ae5878a44e7f 100644
--- a/client/src/components/Workflow/List/WorkflowActionsExtend.vue
+++ b/client/src/components/Workflow/List/WorkflowActionsExtend.vue
@@ -36,12 +36,9 @@ const { isAnonymous } = storeToRefs(useUserStore());
 const downloadUrl = computed(() => {
     return withPrefix(`/api/workflows/${props.workflow.id}/download?format=json-download`);
 });
+
 const shared = computed(() => {
-    if (userStore.currentUser) {
-        return userStore.currentUser.username !== props.workflow.owner;
-    } else {
-        return false;
-    }
+    return !userStore.matchesCurrentUsername(props.workflow.owner);
 });
 
 async function onCopy() {
diff --git a/client/src/components/Workflow/List/WorkflowCard.vue b/client/src/components/Workflow/List/WorkflowCard.vue
index 076ec2c271e6..f217a1a76ec2 100644
--- a/client/src/components/Workflow/List/WorkflowCard.vue
+++ b/client/src/components/Workflow/List/WorkflowCard.vue
@@ -48,13 +48,11 @@ const showRename = ref(false);
 const showPreview = ref(false);
 
 const workflow = computed(() => props.workflow);
+
 const shared = computed(() => {
-    if (userStore.currentUser) {
-        return userStore.currentUser.username !== workflow.value.owner;
-    } else {
-        return false;
-    }
+    return !userStore.matchesCurrentUsername(workflow.value.owner);
 });
+
 const description = computed(() => {
     if (workflow.value.annotations && workflow.value.annotations.length > 0) {
         return workflow.value.annotations[0].trim();
diff --git a/client/src/components/Workflow/List/WorkflowIndicators.vue b/client/src/components/Workflow/List/WorkflowIndicators.vue
index 676e56932a03..92ccac96c4ae 100644
--- a/client/src/components/Workflow/List/WorkflowIndicators.vue
+++ b/client/src/components/Workflow/List/WorkflowIndicators.vue
@@ -29,19 +29,17 @@ const router = useRouter();
 const userStore = useUserStore();
 
 const publishedTitle = computed(() => {
-    if (userStore.currentUser?.username === props.workflow.owner) {
+    if (userStore.matchesCurrentUsername(props.workflow.owner)) {
         return "Published by you. Click to view all published workflows by you";
     } else {
         return `Published by '${props.workflow.owner}'. Click to view all published workflows by '${props.workflow.owner}'`;
     }
 });
+
 const shared = computed(() => {
-    if (userStore.currentUser) {
-        return userStore.currentUser.username !== props.workflow.owner;
-    } else {
-        return false;
-    }
+    return !userStore.matchesCurrentUsername(props.workflow.owner);
 });
+
 const sourceType = computed(() => {
     if (props.workflow.source_metadata?.url) {
         return "url";
@@ -51,6 +49,7 @@ const sourceType = computed(() => {
         return "";
     }
 });
+
 const sourceTitle = computed(() => {
     if (sourceType.value.includes("trs")) {
         return `Imported from TRS ID (version: ${props.workflow.source_metadata.trs_version_id}). Click to copy ID`;
diff --git a/client/src/components/Workflow/List/WorkflowList.vue b/client/src/components/Workflow/List/WorkflowList.vue
index b730a12f002c..70467677c548 100644
--- a/client/src/components/Workflow/List/WorkflowList.vue
+++ b/client/src/components/Workflow/List/WorkflowList.vue
@@ -145,7 +145,7 @@ async function load(overlayLoading = false, silent = false) {
             : data;
 
         if (props.activeList === "my") {
-            filteredWorkflows = filter(filteredWorkflows, (w: any) => w.owner === userStore.currentUser?.username);
+            filteredWorkflows = filter(filteredWorkflows, (w: any) => userStore.matchesCurrentUsername(w.owner));
         }
 
         workflowsLoaded.value = filteredWorkflows;
diff --git a/client/src/components/Workflow/Published/WorkflowInformation.vue b/client/src/components/Workflow/Published/WorkflowInformation.vue
index b9d9afb1acb5..dc41bdd27c85 100644
--- a/client/src/components/Workflow/Published/WorkflowInformation.vue
+++ b/client/src/components/Workflow/Published/WorkflowInformation.vue
@@ -5,6 +5,7 @@ import { FontAwesomeIcon } from "@fortawesome/vue-fontawesome";
 import { computed } from "vue";
 import { RouterLink } from "vue-router";
 
+import { type WorkflowSummary } from "@/api/workflows";
 import { useUserStore } from "@/stores/userStore";
 import { getFullAppUrl } from "@/utils/utils";
 
@@ -16,19 +17,8 @@ import UtcDate from "@/components/UtcDate.vue";
 
 library.add(faBuilding, faUser);
 
-interface WorkflowInformation {
-    name: string;
-    [key: string]: unknown;
-    update_time: string;
-    license?: string;
-    tags?: string[];
-    creator?: {
-        [key: string]: unknown;
-    }[];
-}
-
 interface Props {
-    workflowInfo: WorkflowInformation;
+    workflowInfo: WorkflowSummary;
     embedded?: boolean;
 }
 
@@ -51,11 +41,7 @@ const fullLink = computed(() => {
 });
 
 const userOwned = computed(() => {
-    if (userStore.currentUser) {
-        return userStore.currentUser.username === props.workflowInfo.owner;
-    } else {
-        return false;
-    }
+    return userStore.matchesCurrentUsername(props.workflowInfo.owner);
 });
 </script>
 
diff --git a/client/src/components/Workflow/Published/WorkflowPublished.vue b/client/src/components/Workflow/Published/WorkflowPublished.vue
index d2c14b7d18b3..524623314ab7 100644
--- a/client/src/components/Workflow/Published/WorkflowPublished.vue
+++ b/client/src/components/Workflow/Published/WorkflowPublished.vue
@@ -6,6 +6,7 @@ import { type AxiosError } from "axios";
 import { BAlert, BButton, BCard } from "bootstrap-vue";
 import { computed, onMounted, ref, watch } from "vue";
 
+import { type WorkflowSummary } from "@/api/workflows";
 import { fromSimple } from "@/components/Workflow/Editor/modules/model";
 import { getWorkflowFull, getWorkflowInfo } from "@/components/Workflow/workflows.services";
 import { useDatatypesMapper } from "@/composables/datatypesMapper";
@@ -22,14 +23,6 @@ import WorkflowInformation from "@/components/Workflow/Published/WorkflowInforma
 
 library.add(faBuilding, faDownload, faEdit, faPlay, faSpinner, faUser);
 
-type WorkflowInfo = {
-    name: string;
-    [key: string]: unknown;
-    license?: string;
-    tags?: string[];
-    update_time: string;
-};
-
 interface Props {
     id: string;
     zoom?: number;
@@ -65,7 +58,7 @@ const { stateStore } = provideScopedWorkflowStores(props.id);
 
 const loading = ref(true);
 const errorMessage = ref("");
-const workflowInfo = ref<WorkflowInfo>();
+const workflowInfo = ref<WorkflowSummary>();
 const workflow = ref<Workflow | null>(null);
 
 const hasError = computed(() => !!errorMessage.value);
@@ -80,13 +73,11 @@ const initialPosition = computed(() => ({
 }));
 
 const viewUrl = computed(() => withPrefix(`/published/workflow?id=${props.id}`));
+
 const sharedWorkflow = computed(() => {
-    if (userStore.currentUser) {
-        return userStore.currentUser.username !== workflowInfo.value?.owner;
-    } else {
-        return false;
-    }
+    return !userStore.matchesCurrentUsername(workflowInfo.value?.owner);
 });
+
 const editButtonTitle = computed(() => {
     if (userStore.isAnonymous) {
         return "Log in to edit Workflow";
diff --git a/client/src/components/Workflow/Run/WorkflowRun.vue b/client/src/components/Workflow/Run/WorkflowRun.vue
index 5f3a97e7dad0..4190257e3cdd 100644
--- a/client/src/components/Workflow/Run/WorkflowRun.vue
+++ b/client/src/components/Workflow/Run/WorkflowRun.vue
@@ -1,6 +1,5 @@
 <script setup lang="ts">
 import { BAlert, BLink } from "bootstrap-vue";
-import { storeToRefs } from "pinia";
 import { computed, onMounted, ref, watch } from "vue";
 import { RouterLink } from "vue-router";
 import { useRouter } from "vue-router/composables";
@@ -22,7 +21,7 @@ import WorkflowRunSuccess from "@/components/Workflow/Run/WorkflowRunSuccess.vue
 
 const historyStore = useHistoryStore();
 const historyItemsStore = useHistoryItemsStore();
-const { currentUser } = storeToRefs(useUserStore());
+const userStore = useUserStore();
 const router = useRouter();
 
 interface Props {
@@ -55,7 +54,7 @@ const editorLink = computed(
     () => `/workflows/edit?id=${props.workflowId}${props.version ? `&version=${props.version}` : ""}`
 );
 const historyStatusKey = computed(() => `${currentHistoryId.value}_${lastUpdateTime.value}`);
-const isOwner = computed(() => currentUser.value?.username === workflowModel.value.runData.owner);
+const isOwner = computed(() => userStore.matchesCurrentUsername(workflowModel.value.runData.owner));
 const lastUpdateTime = computed(() => historyItemsStore.lastUpdateTime);
 const canRunOnHistory = computed(() => {
     if (!currentHistoryId.value) {
diff --git a/client/src/components/Workflow/workflows.services.ts b/client/src/components/Workflow/workflows.services.ts
index e0c9c7d328c9..ade0f1f1b40d 100644
--- a/client/src/components/Workflow/workflows.services.ts
+++ b/client/src/components/Workflow/workflows.services.ts
@@ -53,9 +53,9 @@ export async function copyWorkflow(id: string, currentOwner: string, version?: s
     const { data: workflowData } = await axios.get(withPrefix(path));
 
     workflowData.name = `Copy of ${workflowData.name}`;
-    const currentUsername = useUserStore().currentUser?.username;
+    const userStore = useUserStore();
 
-    if (currentUsername !== currentOwner) {
+    if (!userStore.matchesCurrentUsername(currentOwner)) {
         workflowData.name += ` shared by user ${currentOwner}`;
     }
 
diff --git a/client/src/stores/userStore.ts b/client/src/stores/userStore.ts
index 9e53f5f111ac..3691848ff1f0 100644
--- a/client/src/stores/userStore.ts
+++ b/client/src/stores/userStore.ts
@@ -1,7 +1,7 @@
 import { defineStore } from "pinia";
 import { computed, ref } from "vue";
 
-import { type AnonymousUser, type User } from "@/api";
+import { type AnyUser, isAdminUser, isAnonymousUser, isRegisteredUser, type RegisteredUser } from "@/api";
 import { useUserLocalStorage } from "@/composables/userLocalStorage";
 import { useHistoryStore } from "@/stores/historyStore";
 import {
@@ -19,7 +19,7 @@ interface Preferences {
 type ListViewMode = "grid" | "list";
 
 export const useUserStore = defineStore("userStore", () => {
-    const currentUser = ref<User | AnonymousUser | null>(null);
+    const currentUser = ref<AnyUser>(null);
     const currentPreferences = ref<Preferences | null>(null);
 
     // explicitly pass current User, because userStore might not exist yet
@@ -35,11 +35,11 @@ export const useUserStore = defineStore("userStore", () => {
     }
 
     const isAdmin = computed(() => {
-        return currentUser.value?.is_admin ?? false;
+        return isAdminUser(currentUser.value);
     });
 
     const isAnonymous = computed(() => {
-        return !("email" in (currentUser.value || []));
+        return isAnonymousUser(currentUser.value);
     });
 
     const currentTheme = computed(() => {
@@ -54,7 +54,13 @@ export const useUserStore = defineStore("userStore", () => {
         }
     });
 
-    function setCurrentUser(user: User) {
+    const matchesCurrentUsername = computed(() => {
+        return (username?: string) => {
+            return isRegisteredUser(currentUser.value) && currentUser.value.username === username;
+        };
+    });
+
+    function setCurrentUser(user: RegisteredUser) {
         currentUser.value = user;
     }
 
@@ -132,6 +138,7 @@ export const useUserStore = defineStore("userStore", () => {
         toggledSideBar,
         preferredListViewMode,
         loadUser,
+        matchesCurrentUsername,
         setCurrentUser,
         setCurrentTheme,
         setPreferredListViewMode,

From 83c7d0e808feacb6890ab5aed1e0412178cd3497 Mon Sep 17 00:00:00 2001
From: davelopez <46503462+davelopez@users.noreply.github.com>
Date: Mon, 5 Aug 2024 16:22:49 +0200
Subject: [PATCH 3/4] Use correct user types in client tests

---
 client/src/api/index.test.ts                  | 12 +++-----
 .../DatasetInformation/DatasetError.test.ts   |  9 ++----
 .../components/History/HistoryView.test.js    |  6 ++--
 .../History/Multiple/MultipleView.test.js     |  4 ++-
 .../JobDestinationParams.test.ts              | 10 ++-----
 .../src/components/Masthead/Masthead.test.js  |  8 +++++
 .../components/Masthead/QuotaMeter.test.ts    | 30 ++++++++-----------
 .../User/DiskUsage/DiskUsageSummary.test.ts   | 10 ++-----
 .../Workflow/List/WorkflowList.test.ts        |  8 ++---
 client/tests/test-data/index.ts               | 19 ++++++++++++
 10 files changed, 59 insertions(+), 57 deletions(-)
 create mode 100644 client/tests/test-data/index.ts

diff --git a/client/src/api/index.test.ts b/client/src/api/index.test.ts
index fb16bc95edc2..6d82145560ef 100644
--- a/client/src/api/index.test.ts
+++ b/client/src/api/index.test.ts
@@ -1,10 +1,11 @@
+import { getFakeRegisteredUser } from "@tests/test-data";
+
 import {
     type AnonymousUser,
     type AnyHistory,
     type HistorySummary,
     type HistorySummaryExtended,
     isRegisteredUser,
-    type User,
     userOwnsHistory,
 } from ".";
 
@@ -12,17 +13,12 @@ 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: "test@mail.test",
-    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/components/DatasetInformation/DatasetError.test.ts b/client/src/components/DatasetInformation/DatasetError.test.ts
index c3213e8dc6fc..3d0dac642ffc 100644
--- 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/HistoryView.test.js b/client/src/components/History/HistoryView.test.js
index d74ce6d885a2..d0d371c80e36 100644
--- 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
index c8acf9c4fa4e..580314ec4e33 100644
--- 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
index 5486f481f91d..aa0e241a9cd1 100644
--- 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
index f299b5b2500f..b5332ed65084 100644
--- 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
index 3599cc2d754a..b405a5b36a12 100644
--- 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,32 +35,29 @@ 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");
         }
@@ -66,22 +65,19 @@ describe("QuotaMeter.vue", () => {
 
     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/User/DiskUsage/DiskUsageSummary.test.ts b/client/src/components/User/DiskUsage/DiskUsageSummary.test.ts
index 5c81ffef4cf8..62e6e1d6b90b 100644
--- 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;
diff --git a/client/src/components/Workflow/List/WorkflowList.test.ts b/client/src/components/Workflow/List/WorkflowList.test.ts
index b4b71d02f185..29e165b42e0b 100644
--- a/client/src/components/Workflow/List/WorkflowList.test.ts
+++ b/client/src/components/Workflow/List/WorkflowList.test.ts
@@ -1,5 +1,6 @@
 import { createTestingPinia } from "@pinia/testing";
 import { getLocalVue } from "@tests/jest/helpers";
+import { getFakeRegisteredUser } from "@tests/test-data";
 import { mount } from "@vue/test-utils";
 import flushPromises from "flush-promises";
 import { setActivePinia } from "pinia";
@@ -18,14 +19,11 @@ const localVue = getLocalVue();
 const FAKE_USER_ID = "fake_user_id";
 const FAKE_USERNAME = "fake_username";
 const FAKE_USER_EMAIL = "fake_user_email";
-const FAKE_USER = {
+const FAKE_USER = getFakeRegisteredUser({
     id: FAKE_USER_ID,
     email: FAKE_USER_EMAIL,
-    tags_used: [],
-    isAnonymous: false,
     username: FAKE_USERNAME,
-    total_disk_usage: 0,
-};
+});
 
 async function mountWorkflowList() {
     const pinia = createTestingPinia();
diff --git a/client/tests/test-data/index.ts b/client/tests/test-data/index.ts
new file mode 100644
index 000000000000..b4fa53c750de
--- /dev/null
+++ b/client/tests/test-data/index.ts
@@ -0,0 +1,19 @@
+import { type RegisteredUser } from "@/api";
+
+export function getFakeRegisteredUser(data: Partial<RegisteredUser> = {}): RegisteredUser {
+    return {
+        id: "fake_user_id",
+        email: "fake_user_email",
+        tags_used: [],
+        isAnonymous: false,
+        username: "fake_username",
+        total_disk_usage: 0,
+        nice_total_disk_usage: "0.0 bytes",
+        deleted: false,
+        purged: false,
+        is_admin: false,
+        preferences: {},
+        quota: "default",
+        ...data,
+    };
+}

From 85dd962a5f751c396943db64f087ae7f438e6bf6 Mon Sep 17 00:00:00 2001
From: davelopez <46503462+davelopez@users.noreply.github.com>
Date: Mon, 5 Aug 2024 18:27:51 +0200
Subject: [PATCH 4/4] More type fixes for user-related stuff

---
 client/src/api/index.ts                       | 11 +++---
 .../CurrentHistory/HistoryMessages.vue        |  4 +--
 client/src/composables/hashedUserId.ts        |  6 ++--
 client/src/composables/userLocalStorage.ts    |  4 +--
 client/src/stores/userStore.ts                | 34 ++++++++++++++-----
 5 files changed, 38 insertions(+), 21 deletions(-)

diff --git a/client/src/api/index.ts b/client/src/api/index.ts
index e22be0702139..3f3c1c689144 100644
--- a/client/src/api/index.ts
+++ b/client/src/api/index.ts
@@ -217,6 +217,7 @@ export function isHistoryItem(item: object): item is HistoryItemSummary {
 
 type RegisteredUserModel = components["schemas"]["DetailedUserModel"];
 type AnonymousUserModel = components["schemas"]["AnonUserModel"];
+type UserModel = RegisteredUserModel | AnonymousUserModel;
 
 export interface RegisteredUser extends RegisteredUserModel {
     isAnonymous: false;
@@ -226,20 +227,18 @@ export interface AnonymousUser extends AnonymousUserModel {
     isAnonymous: true;
 }
 
-export type GenericUser = RegisteredUser | 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): user is RegisteredUser {
+export function isRegisteredUser(user: AnyUser | UserModel): user is RegisteredUser {
     return user !== null && "email" in user;
 }
 
-export function isAnonymousUser(user: AnyUser): user is AnonymousUser {
+export function isAnonymousUser(user: AnyUser | UserModel): user is AnonymousUser {
     return user !== null && !isRegisteredUser(user);
 }
 
-export function isAdminUser(user: AnyUser): user is RegisteredUser {
+export function isAdminUser(user: AnyUser | UserModel): user is RegisteredUser {
     return isRegisteredUser(user) && user.is_admin;
 }
 
diff --git a/client/src/components/History/CurrentHistory/HistoryMessages.vue b/client/src/components/History/CurrentHistory/HistoryMessages.vue
index b078e41f5e73..f3c87772568e 100644
--- 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/composables/hashedUserId.ts b/client/src/composables/hashedUserId.ts
index ef832d774d47..5d86b2dd8c42 100644
--- a/client/src/composables/hashedUserId.ts
+++ b/client/src/composables/hashedUserId.ts
@@ -2,7 +2,7 @@ import { useLocalStorage } from "@vueuse/core";
 import { storeToRefs } from "pinia";
 import { computed, type Ref, ref, watch } from "vue";
 
-import { type GenericUser } from "@/api";
+import { type AnyUser } from "@/api";
 import { useUserStore } from "@/stores/userStore";
 
 async function hash32(value: string): Promise<string> {
@@ -32,8 +32,8 @@ let unhashedId: string | null = null;
 /**
  * One way hashed ID of the current User
  */
-export function useHashedUserId(user?: Ref<GenericUser | null>) {
-    let currentUser: Ref<GenericUser | null>;
+export function useHashedUserId(user?: Ref<AnyUser>) {
+    let currentUser: Ref<AnyUser>;
 
     if (user) {
         currentUser = user;
diff --git a/client/src/composables/userLocalStorage.ts b/client/src/composables/userLocalStorage.ts
index 16e8a7ce00ea..f0787732a12d 100644
--- a/client/src/composables/userLocalStorage.ts
+++ b/client/src/composables/userLocalStorage.ts
@@ -1,7 +1,7 @@
 import { useLocalStorage } from "@vueuse/core";
 import { computed, customRef, type Ref, ref } from "vue";
 
-import { type GenericUser } from "@/api";
+import { type AnyUser } from "@/api";
 
 import { useHashedUserId } from "./hashedUserId";
 
@@ -10,7 +10,7 @@ import { useHashedUserId } from "./hashedUserId";
  * @param key
  * @param initialValue
  */
-export function useUserLocalStorage<T>(key: string, initialValue: T, user?: Ref<GenericUser | null>) {
+export function useUserLocalStorage<T>(key: string, initialValue: T, user?: Ref<AnyUser>) {
     const { hashedUserId } = useHashedUserId(user);
 
     const storedRef = computed(() => {
diff --git a/client/src/stores/userStore.ts b/client/src/stores/userStore.ts
index 3691848ff1f0..55a49d491209 100644
--- a/client/src/stores/userStore.ts
+++ b/client/src/stores/userStore.ts
@@ -11,9 +11,14 @@ import {
     setCurrentThemeQuery,
 } from "@/stores/users/queries";
 
+interface FavoriteTools {
+    tools: string[];
+}
+
 interface Preferences {
-    theme: string;
-    favorites: { tools: string[] };
+    theme?: string;
+    favorites: FavoriteTools;
+    [key: string]: unknown;
 }
 
 type ListViewMode = "grid" | "list";
@@ -68,12 +73,15 @@ export const useUserStore = defineStore("userStore", () => {
         if (!loadPromise) {
             loadPromise = getCurrentUser()
                 .then(async (user) => {
-                    currentUser.value = { ...user, isAnonymous: !user.email };
-                    currentPreferences.value = user?.preferences ?? null;
-                    // TODO: This is a hack to get around the fact that the API returns a string
-                    if (currentPreferences.value?.favorites) {
-                        currentPreferences.value.favorites = JSON.parse(user?.preferences?.favorites ?? { tools: [] });
+                    if (isRegisteredUser(user)) {
+                        currentUser.value = user;
+                        currentPreferences.value = processUserPreferences(user);
+                    } else if (isAnonymousUser(user)) {
+                        currentUser.value = user;
+                    } else if (user === null) {
+                        currentUser.value = null;
                     }
+
                     if (includeHistories) {
                         const historyStore = useHistoryStore();
                         // load first few histories for user to start pagination
@@ -116,7 +124,7 @@ export const useUserStore = defineStore("userStore", () => {
 
     function setFavoriteTools(tools: string[]) {
         if (currentPreferences.value) {
-            currentPreferences.value.favorites.tools = tools ?? { tools: [] };
+            currentPreferences.value.favorites.tools = tools;
         }
     }
 
@@ -128,6 +136,16 @@ export const useUserStore = defineStore("userStore", () => {
         toggledSideBar.value = toggledSideBar.value === currentOpen ? "" : currentOpen;
     }
 
+    function processUserPreferences(user: RegisteredUser): Preferences {
+        // Favorites are returned as a JSON string by the API
+        const favorites =
+            typeof user.preferences.favorites === "string" ? JSON.parse(user.preferences.favorites) : { tools: [] };
+        return {
+            ...user.preferences,
+            favorites,
+        };
+    }
+
     return {
         currentUser,
         currentPreferences,