From a13265125206af796d647d4023f7faeca9fbb953 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Fri, 22 Mar 2024 11:07:15 +0100 Subject: [PATCH 01/16] Refactor async calls to avoid mixing `await` and `then` --- client/src/stores/historyStore.ts | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/client/src/stores/historyStore.ts b/client/src/stores/historyStore.ts index 0ceb16cbb053..4d6dcfd6ef2e 100644 --- a/client/src/stores/historyStore.ts +++ b/client/src/stores/historyStore.ts @@ -207,7 +207,7 @@ export const useHistoryStore = defineStore("historyStore", () => { } async function loadTotalHistoryCount() { - await getHistoryCount().then((count) => (totalHistoryCount.value = count)); + totalHistoryCount.value = await getHistoryCount(); } /** TODO: @@ -231,15 +231,17 @@ export const useHistoryStore = defineStore("historyStore", () => { } } const offset = queryString ? 0 : historiesOffset.value; - await getHistoryList(offset, limit, queryString) - .then(async (histories) => { - setHistories(histories); - if (paginate && !queryString && historiesOffset.value == offset) { - await handleTotalCountChange(histories.length); - } - }) - .catch((error) => console.warn(error)) - .finally(() => setHistoriesLoading(false)); + try { + const histories = await getHistoryList(offset, limit, queryString); + setHistories(histories); + if (paginate && !queryString && historiesOffset.value == offset) { + await handleTotalCountChange(histories.length); + } + } catch (error) { + console.warn(error); + } finally { + setHistoriesLoading(false); + } } } From 4a466f9aa05d383648bcfa329080c1adbbfaabc9 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Fri, 22 Mar 2024 11:43:09 +0100 Subject: [PATCH 02/16] Reduce redundant detailed history load The current history details will be loaded by the history watcher anyway. --- client/src/stores/userStore.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/client/src/stores/userStore.ts b/client/src/stores/userStore.ts index 26783f6609fc..10b9fdf11fcd 100644 --- a/client/src/stores/userStore.ts +++ b/client/src/stores/userStore.ts @@ -67,7 +67,6 @@ export const useUserStore = defineStore("userStore", () => { } if (includeHistories) { const historyStore = useHistoryStore(); - await historyStore.loadCurrentHistory(); // load first few histories for user to start pagination await historyStore.loadHistories(); } From 73872865401959f533abd0b14b9c2660153f5fd2 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Fri, 22 Mar 2024 13:30:09 +0100 Subject: [PATCH 03/16] Add HistoryDevDetailed schema model Technically this model is used by the "client API" in the old controllers so the current API is not using it, but it is useful to have the schema in the client to know what we are getting when requesting something from the server. --- client/src/api/schema/schema.ts | 180 ++++++++++++++++++++++++++++++++ lib/galaxy/schema/schema.py | 93 +++++++++++------ 2 files changed, 241 insertions(+), 32 deletions(-) diff --git a/client/src/api/schema/schema.ts b/client/src/api/schema/schema.ts index cb7b0dc21217..2456d9a42c25 100644 --- a/client/src/api/schema/schema.ts +++ b/client/src/api/schema/schema.ts @@ -2232,6 +2232,16 @@ export interface components { * @description Whether this item is marked as deleted. */ deleted: boolean; + /** + * Encoded Email + * @description Encoded owner email. + */ + email_hash?: string | null; + /** + * Empty + * @description Whether this history is empty. + */ + empty: boolean; /** * Export Record Data * @description The export record data associated with this archived history. Used to recover the history. @@ -2243,6 +2253,11 @@ export interface components { * @default ? */ genome_build?: string | null; + /** + * HID Counter + * @description The current HID counter for this History. + */ + hid_counter: number; /** * History ID * @example 0123456789ABCDEF @@ -3629,11 +3644,26 @@ export interface components { * @description Whether this item is marked as deleted. */ deleted?: boolean | null; + /** + * Encoded Email + * @description Encoded owner email. + */ + email_hash?: string | null; + /** + * Empty + * @description Whether this history is empty. + */ + empty?: boolean | null; /** * Genome Build * @description TODO */ genome_build?: string | null; + /** + * HID Counter + * @description The current HID counter for this History. + */ + hid_counter?: number | null; /** * History ID * @example 0123456789ABCDEF @@ -6985,12 +7015,27 @@ export interface components { * @description Whether this item is marked as deleted. */ deleted: boolean; + /** + * Encoded Email + * @description Encoded owner email. + */ + email_hash?: string | null; + /** + * Empty + * @description Whether this history is empty. + */ + empty: boolean; /** * Genome Build * @description TODO * @default ? */ genome_build?: string | null; + /** + * HID Counter + * @description The current HID counter for this History. + */ + hid_counter: number; /** * History ID * @example 0123456789ABCDEF @@ -7085,6 +7130,127 @@ export interface components { */ username_and_slug?: string | null; }; + /** + * HistoryDevDetailed + * @description View used by the client to display "some" detailed history information. + * + * Currently used by the `HistoryController` endpoints. + */ + HistoryDevDetailed: { + /** + * Annotation + * @description An annotation to provide details or to help understand the purpose and usage of this item. + */ + annotation: string | null; + /** + * Archived + * @description Whether this item has been archived and is no longer active. + */ + archived: boolean; + /** Contents Active */ + contents_active: components["schemas"]["HistoryActiveContentCounts"]; + /** + * Contents URL + * @description The relative URL to access the contents of this History. + */ + contents_url: string; + /** + * Count + * @description The number of items in the history. + */ + count: number; + /** + * Create Time + * Format: date-time + * @description The time and date this item was created. + */ + create_time: string; + /** + * Deleted + * @description Whether this item is marked as deleted. + */ + deleted: boolean; + /** + * Genome Build + * @description TODO + * @default ? + */ + genome_build?: string | null; + /** + * HID Counter + * @description The current HID counter for this History. + */ + hid_counter: number; + /** + * History ID + * @example 0123456789ABCDEF + */ + id: string; + /** + * Importable + * @description Whether this History can be imported by other users with a shared link. + */ + importable: boolean; + /** + * Model class + * @description The name of the database model class. + * @constant + */ + model_class: "History"; + /** + * Name + * @description The name of the history. + */ + name: string; + /** + * Preferred Object Store ID + * @description The ID of the object store that should be used to store new datasets in this history. + */ + preferred_object_store_id?: string | null; + /** + * Published + * @description Whether this resource is currently publicly available to all users. + */ + published: boolean; + /** + * Purged + * @description Whether this item has been permanently removed. + */ + purged: boolean; + /** + * Size + * @description The total size of the contents of this history in bytes. + */ + size: number; + /** + * Slug + * @description Part of the URL to uniquely identify this History by link in a readable way. + */ + slug?: string | null; + tags: components["schemas"]["TagCollection"]; + /** + * Update Time + * Format: date-time + * @description The last time and date this item was updated. + */ + update_time: string; + /** + * URL + * @deprecated + * @description The relative URL to access this item. + */ + url: string; + /** + * User ID + * @description The encoded ID of the user that owns this History. + */ + user_id?: string | null; + /** + * Username and slug + * @description The relative URL in the form of /u/{username}/h/{slug} + */ + username_and_slug?: string | null; + }; /** * HistorySummary * @description History summary information. @@ -15592,6 +15758,7 @@ export interface operations { content: { "application/json": ( | components["schemas"]["CustomHistoryView"] + | components["schemas"]["HistoryDevDetailed"] | components["schemas"]["HistoryDetailed"] | components["schemas"]["HistorySummary"] )[]; @@ -15634,6 +15801,7 @@ export interface operations { "application/json": | components["schemas"]["JobImportHistoryResponse"] | components["schemas"]["CustomHistoryView"] + | components["schemas"]["HistoryDevDetailed"] | components["schemas"]["HistoryDetailed"] | components["schemas"]["HistorySummary"]; }; @@ -15720,6 +15888,7 @@ export interface operations { content: { "application/json": ( | components["schemas"]["CustomHistoryView"] + | components["schemas"]["HistoryDevDetailed"] | components["schemas"]["HistoryDetailed"] | components["schemas"]["HistorySummary"] )[]; @@ -15758,6 +15927,7 @@ export interface operations { content: { "application/json": ( | components["schemas"]["CustomHistoryView"] + | components["schemas"]["HistoryDevDetailed"] | components["schemas"]["HistoryDetailed"] | components["schemas"]["HistorySummary"] )[]; @@ -15826,6 +15996,7 @@ export interface operations { content: { "application/json": ( | components["schemas"]["CustomHistoryView"] + | components["schemas"]["HistoryDevDetailed"] | components["schemas"]["HistoryDetailed"] | components["schemas"]["HistorySummary"] )[]; @@ -15863,6 +16034,7 @@ export interface operations { content: { "application/json": | components["schemas"]["CustomHistoryView"] + | components["schemas"]["HistoryDevDetailed"] | components["schemas"]["HistoryDetailed"] | components["schemas"]["HistorySummary"]; }; @@ -15900,6 +16072,7 @@ export interface operations { content: { "application/json": | components["schemas"]["CustomHistoryView"] + | components["schemas"]["HistoryDevDetailed"] | components["schemas"]["HistoryDetailed"] | components["schemas"]["HistorySummary"]; }; @@ -15960,6 +16133,7 @@ export interface operations { content: { "application/json": | components["schemas"]["CustomHistoryView"] + | components["schemas"]["HistoryDevDetailed"] | components["schemas"]["HistoryDetailed"] | components["schemas"]["HistorySummary"]; }; @@ -16002,6 +16176,7 @@ export interface operations { content: { "application/json": ( | components["schemas"]["CustomHistoryView"] + | components["schemas"]["HistoryDevDetailed"] | components["schemas"]["HistoryDetailed"] | components["schemas"]["HistorySummary"] )[]; @@ -16045,6 +16220,7 @@ export interface operations { content: { "application/json": ( | components["schemas"]["CustomHistoryView"] + | components["schemas"]["HistoryDevDetailed"] | components["schemas"]["HistoryDetailed"] | components["schemas"]["HistorySummary"] )[]; @@ -16082,6 +16258,7 @@ export interface operations { content: { "application/json": | components["schemas"]["CustomHistoryView"] + | components["schemas"]["HistoryDevDetailed"] | components["schemas"]["HistoryDetailed"] | components["schemas"]["HistorySummary"]; }; @@ -16123,6 +16300,7 @@ export interface operations { content: { "application/json": | components["schemas"]["CustomHistoryView"] + | components["schemas"]["HistoryDevDetailed"] | components["schemas"]["HistoryDetailed"] | components["schemas"]["HistorySummary"]; }; @@ -16165,6 +16343,7 @@ export interface operations { content: { "application/json": | components["schemas"]["CustomHistoryView"] + | components["schemas"]["HistoryDevDetailed"] | components["schemas"]["HistoryDetailed"] | components["schemas"]["HistorySummary"]; }; @@ -16259,6 +16438,7 @@ export interface operations { content: { "application/json": | components["schemas"]["CustomHistoryView"] + | components["schemas"]["HistoryDevDetailed"] | components["schemas"]["HistoryDetailed"] | components["schemas"]["HistorySummary"]; }; diff --git a/lib/galaxy/schema/schema.py b/lib/galaxy/schema/schema.py index 34fe24988095..64211ffa08f2 100644 --- a/lib/galaxy/schema/schema.py +++ b/lib/galaxy/schema/schema.py @@ -1303,6 +1303,44 @@ class HistorySummary(Model, WithModelClass): preferred_object_store_id: Optional[str] = PreferredObjectStoreIdField +class HistoryCommonDetailed(HistorySummary): + """Common detailed information provided by a History.""" + + contents_url: ContentsUrlField + size: int = Field( + ..., + title="Size", + description="The total size of the contents of this history in bytes.", + ) + user_id: Optional[EncodedDatabaseIdField] = Field( + None, + title="User ID", + description="The encoded ID of the user that owns this History.", + ) + create_time: datetime = CreateTimeField + importable: bool = Field( + ..., + title="Importable", + description="Whether this History can be imported by other users with a shared link.", + ) + slug: Optional[str] = Field( + None, + title="Slug", + description="Part of the URL to uniquely identify this History by link in a readable way.", + ) + username_and_slug: Optional[str] = Field( + None, + title="Username and slug", + description="The relative URL in the form of /u/{username}/h/{slug}", + ) + genome_build: Optional[str] = GenomeBuildField + hid_counter: int = Field( + ..., + title="HID Counter", + description="The current HID counter for this History.", + ) + + class HistoryActiveContentCounts(Model): """Contains the number of active, deleted or hidden items in a History.""" @@ -1331,42 +1369,24 @@ class HistoryActiveContentCounts(Model): HistoryContentStateCounts = Dict[HistoryContentStates, int] -class HistoryDetailed(HistorySummary): # Equivalent to 'dev-detailed' view, which seems the default +class HistoryDetailed(HistoryCommonDetailed): """History detailed information.""" - contents_url: ContentsUrlField - size: int = Field( - ..., - title="Size", - description="The total size of the contents of this history in bytes.", - ) - user_id: Optional[EncodedDatabaseIdField] = Field( + email_hash: Optional[str] = Field( None, - title="User ID", - description="The encoded ID of the user that owns this History.", + title="Encoded Email", + description="Encoded owner email.", ) - create_time: datetime = CreateTimeField - importable: bool = Field( + empty: bool = Field( ..., - title="Importable", - description="Whether this History can be imported by other users with a shared link.", - ) - slug: Optional[str] = Field( - None, - title="Slug", - description="Part of the URL to uniquely identify this History by link in a readable way.", + title="Empty", + description="Whether this history is empty.", ) username: Optional[str] = Field( None, title="Username", description="Owner of the history", ) - username_and_slug: Optional[str] = Field( - None, - title="Username and slug", - description="The relative URL in the form of /u/{username}/h/{slug}", - ) - genome_build: Optional[str] = GenomeBuildField state: DatasetState = Field( ..., title="State", @@ -1390,8 +1410,21 @@ class HistoryDetailed(HistorySummary): # Equivalent to 'dev-detailed' view, whi ) +class HistoryDevDetailed(HistoryCommonDetailed): + """View used by the client to display "some" detailed history information. + + Currently used by the `HistoryController` endpoints. + """ + + contents_active: HistoryActiveContentCounts = Field( + ..., + title="Contents Active", + description=("Contains the number of active, deleted or hidden items in a History."), + ) + + @partial_model() -class CustomHistoryView(HistoryDetailed): +class CustomHistoryView(HistoryDetailed, HistoryDevDetailed): """History Response with all optional fields. It is used for serializing only specific attributes using the "keys" @@ -1399,12 +1432,7 @@ class CustomHistoryView(HistoryDetailed): will be requested, so we have to allow all fields to be optional. """ - # Define a few more useful fields to be optional that are not part of HistoryDetailed - contents_active: Optional[HistoryActiveContentCounts] = Field( - default=None, - title="Contents Active", - description=("Contains the number of active, deleted or hidden items in a History."), - ) + # Define a few more useful fields to be optional that are not part of other views contents_states: Optional[HistoryContentStateCounts] = Field( default=None, title="Contents States", @@ -1420,6 +1448,7 @@ class CustomHistoryView(HistoryDetailed): AnyHistoryView = Annotated[ Union[ CustomHistoryView, + HistoryDevDetailed, HistoryDetailed, HistorySummary, ], From 1361af872c49f8f2c7a01ef1172b01bc8231a44d Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Fri, 22 Mar 2024 13:41:54 +0100 Subject: [PATCH 04/16] Load current history through the historyStore in watcher This will make the history data available for all the app and reduce duplicated requests. --- client/src/store/historyStore/model/watchHistory.js | 3 +-- client/src/stores/historyStore.ts | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/client/src/store/historyStore/model/watchHistory.js b/client/src/store/historyStore/model/watchHistory.js index f04ceb582a69..74dfcce033c3 100644 --- a/client/src/store/historyStore/model/watchHistory.js +++ b/client/src/store/historyStore/model/watchHistory.js @@ -9,7 +9,6 @@ import { getGalaxyInstance } from "app"; import { storeToRefs } from "pinia"; import { useHistoryItemsStore } from "stores/historyItemsStore"; import { useHistoryStore } from "stores/historyStore"; -import { getCurrentHistoryFromServer } from "stores/services/history.services"; import { loadSet } from "utils/setCache"; import { urlData } from "utils/url"; @@ -55,7 +54,7 @@ export async function watchHistoryOnce() { // get current history const checkForUpdate = new Date(); - const history = await getCurrentHistoryFromServer(lastUpdateTime); + const history = await historyStore.loadCurrentHistory(lastUpdateTime); const { lastCheckedTime } = storeToRefs(historyItemsStore); lastCheckedTime.value = checkForUpdate; if (!history || !history.id) { diff --git a/client/src/stores/historyStore.ts b/client/src/stores/historyStore.ts index 4d6dcfd6ef2e..b3d0c047eb0e 100644 --- a/client/src/stores/historyStore.ts +++ b/client/src/stores/historyStore.ts @@ -189,8 +189,8 @@ export const useHistoryStore = defineStore("historyStore", () => { await handleTotalCountChange(1, true); } - async function loadCurrentHistory() { - const history = await getCurrentHistoryFromServer(); + async function loadCurrentHistory(since?: string) { + const history = await getCurrentHistoryFromServer(since); selectHistory(history as HistorySummary); } From 841a2812bb8de2f92bfd821ee6bfecbfffcde213 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Fri, 22 Mar 2024 14:17:11 +0100 Subject: [PATCH 05/16] Use proper types for history requests in the store This should give us an idea of what information is being stored and retrieved each time. --- client/src/api/index.ts | 25 ++++++++++++- client/src/stores/historyStore.ts | 59 ++++++++++++++++++------------- 2 files changed, 58 insertions(+), 26 deletions(-) diff --git a/client/src/api/index.ts b/client/src/api/index.ts index 4048356ef219..720c5dd8f343 100644 --- a/client/src/api/index.ts +++ b/client/src/api/index.ts @@ -7,6 +7,10 @@ import { components } from "@/api/schema"; */ export type HistorySummary = components["schemas"]["HistorySummary"]; +/** + * Contains additional details about the contents and owner of a History. + * This is used by the client API to simplify the handling of History objects. + */ export interface HistorySummaryExtended extends HistorySummary { size: number; contents_active: components["schemas"]["HistoryActiveContentCounts"]; @@ -18,7 +22,26 @@ export interface HistorySummaryExtended extends HistorySummary { */ export type HistoryDetailed = components["schemas"]["HistoryDetailed"]; -export type AnyHistory = HistorySummary | HistorySummaryExtended | HistoryDetailed; +/** + * Alternative representation of history details used by the client API. + * Shares most of the fields with HistoryDetailed but not all and adds some additional fields. + */ +export type HistoryDevDetailed = components["schemas"]["HistoryDevDetailed"]; + +/** + * Contains all available information about a History. + */ +export type HistoryExtended = HistoryDevDetailed & HistoryDetailed; + +/** + * Represents any amount of information about a History with the minimal being a HistorySummary. + */ +export type AnyHistory = + | HistorySummary + | HistorySummaryExtended + | HistoryDetailed + | HistoryDevDetailed + | HistoryExtended; /** * Contains minimal information about a HistoryContentItem. diff --git a/client/src/stores/historyStore.ts b/client/src/stores/historyStore.ts index b3d0c047eb0e..87d80b14f884 100644 --- a/client/src/stores/historyStore.ts +++ b/client/src/stores/historyStore.ts @@ -1,7 +1,7 @@ import { defineStore } from "pinia"; import { computed, del, ref, set } from "vue"; -import type { HistorySummary } from "@/api"; +import type { AnyHistory, HistoryDevDetailed, HistorySummary, HistorySummaryExtended } from "@/api"; import { archiveHistory, unarchiveHistory } from "@/api/histories.archived"; import { HistoryFilters } from "@/components/History/HistoryFilters"; import { useUserLocalStorage } from "@/composables/userLocalStorage"; @@ -29,7 +29,7 @@ export const useHistoryStore = defineStore("historyStore", () => { const pinnedHistories = useUserLocalStorage<{ id: string }[]>("history-store-pinned-histories", []); const storedCurrentHistoryId = ref(null); const storedFilterTexts = ref<{ [key: string]: string }>({}); - const storedHistories = ref<{ [key: string]: HistorySummary }>({}); + const storedHistories = ref<{ [key: string]: AnyHistory }>({}); const histories = computed(() => { return Object.values(storedHistories.value) @@ -41,9 +41,9 @@ export const useHistoryStore = defineStore("historyStore", () => { return histories.value[0]?.id ?? null; }); - const currentHistory = computed(() => { + const currentHistory = computed(() => { if (storedCurrentHistoryId.value !== null) { - return getHistoryById.value(storedCurrentHistoryId.value); + return getHistoryById.value(storedCurrentHistoryId.value) as HistorySummaryExtended; } return null; }); @@ -86,8 +86,8 @@ export const useHistoryStore = defineStore("historyStore", () => { }); async function setCurrentHistory(historyId: string) { - const currentHistory = await setCurrentHistoryOnServer(historyId); - selectHistory(currentHistory as HistorySummary); + const currentHistory = (await setCurrentHistoryOnServer(historyId)) as HistoryDevDetailed; + selectHistory(currentHistory); setFilterText(historyId, ""); } @@ -99,7 +99,7 @@ export const useHistoryStore = defineStore("historyStore", () => { set(storedFilterTexts.value, historyId, filterText); } - function setHistory(history: HistorySummary) { + function setHistory(history: AnyHistory) { if (storedHistories.value[history.id] !== undefined) { // Merge the incoming history with existing one to keep additional information Object.assign(storedHistories.value[history.id]!, history); @@ -108,7 +108,7 @@ export const useHistoryStore = defineStore("historyStore", () => { } } - function setHistories(histories: HistorySummary[]) { + function setHistories(histories: AnyHistory[]) { // The incoming history list may contain less information than the already stored // histories, so we ensure that already available details are not getting lost. const enrichedHistories = histories.map((history) => { @@ -117,7 +117,7 @@ export const useHistoryStore = defineStore("historyStore", () => { }); // Histories are provided as list but stored as map. const newMap = enrichedHistories.reduce((acc, h) => ({ ...acc, [h.id]: h }), {}) as { - [key: string]: HistorySummary; + [key: string]: AnyHistory; }; // Ensure that already stored histories, which are not available in the incoming array, // are not lost. This happens e.g. with shared histories since they have different owners. @@ -165,9 +165,9 @@ export const useHistoryStore = defineStore("historyStore", () => { } async function createNewHistory() { - const newHistory = await createAndSelectNewHistory(); + const newHistory = (await createAndSelectNewHistory()) as HistoryDevDetailed; await handleTotalCountChange(1); - return selectHistory(newHistory as HistorySummary); + return selectHistory(newHistory); } function getNextAvailableHistoryId(excludedIds: string[]) { @@ -189,9 +189,17 @@ export const useHistoryStore = defineStore("historyStore", () => { await handleTotalCountChange(1, true); } - async function loadCurrentHistory(since?: string) { - const history = await getCurrentHistoryFromServer(since); - selectHistory(history as HistorySummary); + async function loadCurrentHistory(since?: string): Promise { + try { + const history = (await getCurrentHistoryFromServer(since)) as HistoryDevDetailed; + if (!history) { + return; // There are no changes to the current history, nothing to set + } + selectHistory(history); + return history; + } catch (error) { + console.error(error); + } } /** @@ -232,25 +240,26 @@ export const useHistoryStore = defineStore("historyStore", () => { } const offset = queryString ? 0 : historiesOffset.value; try { - const histories = await getHistoryList(offset, limit, queryString); + const histories = (await getHistoryList(offset, limit, queryString)) as HistorySummary[]; setHistories(histories); if (paginate && !queryString && historiesOffset.value == offset) { await handleTotalCountChange(histories.length); } } catch (error) { - console.warn(error); + console.error(error); } finally { setHistoriesLoading(false); } } } - async function loadHistoryById(historyId: string) { + async function loadHistoryById(historyId: string): Promise { if (!isLoadingHistory.has(historyId)) { isLoadingHistory.add(historyId); try { - const history = await getHistoryByIdFromServer(historyId); - setHistory(history as HistorySummary); + const history = (await getHistoryByIdFromServer(historyId)) as HistorySummaryExtended; + setHistory(history); + return history; } catch (error) { console.error(error); } finally { @@ -260,13 +269,13 @@ export const useHistoryStore = defineStore("historyStore", () => { } async function secureHistory(history: HistorySummary) { - const securedHistory = await secureHistoryOnServer(history); - setHistory(securedHistory as HistorySummary); + const securedHistory = (await secureHistoryOnServer(history)) as HistorySummaryExtended; + setHistory(securedHistory); } async function archiveHistoryById(historyId: string, archiveExportId?: string, purgeHistory = false) { const history = await archiveHistory(historyId, archiveExportId, purgeHistory); - setHistory(history as HistorySummary); + setHistory(history); if (!history.archived) { return; } @@ -282,13 +291,13 @@ export const useHistoryStore = defineStore("historyStore", () => { async function unarchiveHistoryById(historyId: string, force?: boolean) { const history = await unarchiveHistory(historyId, force); - setHistory(history as HistorySummary); + setHistory(history); return history; } async function updateHistory({ id, ...update }: HistorySummary) { - const savedHistory = await updateHistoryFields(id, update); - setHistory(savedHistory as HistorySummary); + const savedHistory = (await updateHistoryFields(id, update)) as HistorySummaryExtended; + setHistory(savedHistory); } return { From 33cca2e0449aa3d16d6d83a6e71dbc478e242a7c Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Fri, 22 Mar 2024 14:18:43 +0100 Subject: [PATCH 06/16] Add history placeholder to prevent multiple requests And avoid reactivity issues with lost references --- client/src/stores/historyStore.ts | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/client/src/stores/historyStore.ts b/client/src/stores/historyStore.ts index 87d80b14f884..49ec6b45e04c 100644 --- a/client/src/stores/historyStore.ts +++ b/client/src/stores/historyStore.ts @@ -68,12 +68,26 @@ export const useHistoryStore = defineStore("historyStore", () => { const getHistoryById = computed(() => { return (historyId: string) => { if (!storedHistories.value[historyId]) { + // Create a placeholder to avoid multiple requests for the same history + // and reuse the reference to avoid reactivity issues + storedHistories.value[historyId] = createHistoryPlaceholder(historyId); + + // TODO: Try to remove this as it can cause computed side effects loadHistoryById(historyId); } - return storedHistories.value[historyId] ?? null; + return storedHistories.value[historyId]!; }; }); + function createHistoryPlaceholder(historyId: string) { + const history = { + id: historyId, + name: "Loading...", + contents_active: { active: 0, deleted: 0, hidden: 0 }, + } as AnyHistory; + return history; + } + const getHistoryNameById = computed(() => { return (historyId: string) => { const history = getHistoryById.value(historyId); From 23b2da38fb80e1c40a83efae6901c2ff15317ee2 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Fri, 22 Mar 2024 15:16:52 +0100 Subject: [PATCH 07/16] Fix multi-history view stats Add composables to deal with "extended" versions of histories when needed. Use proper types for history references in components. --- .../History/CurrentHistory/HistoryCounter.vue | 11 ++++++----- .../HistoryOperations/DefaultOperations.vue | 8 ++++---- .../HistoryOperations/HistoryOperations.vue | 4 ++-- .../History/CurrentHistory/HistoryPanel.vue | 4 ++-- .../CurrentHistory/usesDetailedHistory.js | 15 --------------- .../History/Multiple/MultipleViewItem.vue | 16 ++++++---------- client/src/composables/detailedHistory.ts | 18 ++++++++++++++++++ client/src/composables/historyContentStats.ts | 17 +++++++++++++++++ 8 files changed, 55 insertions(+), 38 deletions(-) delete mode 100644 client/src/components/History/CurrentHistory/usesDetailedHistory.js create mode 100644 client/src/composables/detailedHistory.ts create mode 100644 client/src/composables/historyContentStats.ts diff --git a/client/src/components/History/CurrentHistory/HistoryCounter.vue b/client/src/components/History/CurrentHistory/HistoryCounter.vue index 98efa0bf11b8..ce02cca585b4 100644 --- a/client/src/components/History/CurrentHistory/HistoryCounter.vue +++ b/client/src/components/History/CurrentHistory/HistoryCounter.vue @@ -9,14 +9,13 @@ import prettyBytes from "pretty-bytes"; import { computed, onMounted, ref, toRef } from "vue"; import { useRouter } from "vue-router/composables"; -import type { HistorySummary } from "@/api"; +import type { HistorySummaryExtended } from "@/api"; import { HistoryFilters } from "@/components/History/HistoryFilters.js"; import { useConfig } from "@/composables/config"; +import { useHistoryContentStats } from "@/composables/historyContentStats"; import { useStorageLocationConfiguration } from "@/composables/storageLocation"; import { useUserStore } from "@/stores/userStore"; -import { useDetailedHistory } from "./usesDetailedHistory"; - import PreferredStorePopover from "./PreferredStorePopover.vue"; import SelectPreferredStore from "./SelectPreferredStore.vue"; @@ -26,7 +25,7 @@ library.add(faDatabase, faEyeSlash, faHdd, faMapMarker, faSync, faTrash); const props = withDefaults( defineProps<{ - history: HistorySummary; + history: HistorySummaryExtended; isWatching?: boolean; lastChecked: Date; filterText?: string; @@ -47,7 +46,9 @@ const emit = defineEmits(["update:filter-text", "reloadContents"]); const router = useRouter(); const { config } = useConfig(); const { currentUser } = storeToRefs(useUserStore()); -const { historySize, numItemsActive, numItemsDeleted, numItemsHidden } = useDetailedHistory(toRef(props, "history")); +const { historySize, numItemsActive, numItemsDeleted, numItemsHidden } = useHistoryContentStats( + toRef(props, "history") +); const reloadButtonLoading = ref(false); const reloadButtonTitle = ref(""); diff --git a/client/src/components/History/CurrentHistory/HistoryOperations/DefaultOperations.vue b/client/src/components/History/CurrentHistory/HistoryOperations/DefaultOperations.vue index ad3f94fda94c..07bb357e4c3a 100644 --- a/client/src/components/History/CurrentHistory/HistoryOperations/DefaultOperations.vue +++ b/client/src/components/History/CurrentHistory/HistoryOperations/DefaultOperations.vue @@ -5,26 +5,26 @@ import { FontAwesomeIcon } from "@fortawesome/vue-fontawesome"; import { BDropdown, BDropdownItem, BDropdownText, BModal } from "bootstrap-vue"; import { toRef } from "vue"; -import type { HistorySummary } from "@/api"; -import { useDetailedHistory } from "@/components/History/CurrentHistory/usesDetailedHistory"; +import type { HistorySummary, HistorySummaryExtended } from "@/api"; import { deleteAllHiddenContent, purgeAllDeletedContent, unhideAllHiddenContent, } from "@/components/History/model/crud"; import { iframeRedirect } from "@/components/plugins/legacyNavigation"; +import { useHistoryContentStats } from "@/composables/historyContentStats"; library.add(faCog); interface Props { - history: HistorySummary; + history: HistorySummaryExtended; } const props = defineProps(); const emit = defineEmits(["update:operation-running"]); -const { numItemsDeleted, numItemsHidden } = useDetailedHistory(toRef(props, "history")); +const { numItemsDeleted, numItemsHidden } = useHistoryContentStats(toRef(props, "history")); function onCopy() { iframeRedirect("/dataset/copy_datasets"); diff --git a/client/src/components/History/CurrentHistory/HistoryOperations/HistoryOperations.vue b/client/src/components/History/CurrentHistory/HistoryOperations/HistoryOperations.vue index cddb848018d5..95189d85bd49 100644 --- a/client/src/components/History/CurrentHistory/HistoryOperations/HistoryOperations.vue +++ b/client/src/components/History/CurrentHistory/HistoryOperations/HistoryOperations.vue @@ -3,14 +3,14 @@ import { library } from "@fortawesome/fontawesome-svg-core"; import { faCheckSquare, faCompress } from "@fortawesome/free-solid-svg-icons"; import { FontAwesomeIcon } from "@fortawesome/vue-fontawesome"; -import type { HistorySummary } from "@/api"; +import type { HistorySummaryExtended } from "@/api"; import DefaultOperations from "@/components/History/CurrentHistory/HistoryOperations/DefaultOperations.vue"; library.add(faCheckSquare, faCompress); interface Props { - history: HistorySummary; + history: HistorySummaryExtended; hasMatches: boolean; expandedCount: number; showSelection: boolean; diff --git a/client/src/components/History/CurrentHistory/HistoryPanel.vue b/client/src/components/History/CurrentHistory/HistoryPanel.vue index 5e287c961ec2..3c3d2c6638a0 100644 --- a/client/src/components/History/CurrentHistory/HistoryPanel.vue +++ b/client/src/components/History/CurrentHistory/HistoryPanel.vue @@ -3,7 +3,7 @@ import { BAlert } from "bootstrap-vue"; import { storeToRefs } from "pinia"; import { computed, onMounted, type Ref, ref, set as VueSet, unref, watch } from "vue"; -import type { HistorySummary } from "@/api"; +import type { HistorySummaryExtended } from "@/api"; import { copyDataset } from "@/api/datasets"; import ExpandedItems from "@/components/History/Content/ExpandedItems"; import SelectedItems from "@/components/History/Content/SelectedItems"; @@ -47,7 +47,7 @@ interface BackendFilterError { interface Props { listOffset?: number; - history: HistorySummary; + history: HistorySummaryExtended; filter?: string; canEditHistory?: boolean; filterable?: boolean; diff --git a/client/src/components/History/CurrentHistory/usesDetailedHistory.js b/client/src/components/History/CurrentHistory/usesDetailedHistory.js deleted file mode 100644 index 78c0c7256756..000000000000 --- a/client/src/components/History/CurrentHistory/usesDetailedHistory.js +++ /dev/null @@ -1,15 +0,0 @@ -import { computed } from "vue"; - -export function useDetailedHistory(history) { - const historySize = computed(() => history.value.size || 0); - const numItemsActive = computed(() => history.value.contents_active?.active || 0); - const numItemsDeleted = computed(() => history.value.contents_active?.deleted || 0); - const numItemsHidden = computed(() => history.value.contents_active?.hidden || 0); - - return { - historySize, - numItemsActive, - numItemsDeleted, - numItemsHidden, - }; -} diff --git a/client/src/components/History/Multiple/MultipleViewItem.vue b/client/src/components/History/Multiple/MultipleViewItem.vue index de19d9af530b..4d3faacd4e5c 100644 --- a/client/src/components/History/Multiple/MultipleViewItem.vue +++ b/client/src/components/History/Multiple/MultipleViewItem.vue @@ -1,8 +1,9 @@