Merge pull request #16819 from davelopez/fix_collection_drilling

Fix collection drilling

client/src/components/History/CurrentCollection/CollectionPanel.vue

@@ -7,7 +7,7 @@ import ExpandedItems from "@/components/History/Content/ExpandedItems";
 import { updateContentFields } from "@/components/History/model/queries";
 import { useCollectionElementsStore } from "@/stores/collectionElementsStore";
 import { HistorySummary } from "@/stores/historyStore";
-import { DCESummary, DCObject, HDCASummary } from "@/stores/services";
+import { CollectionEntry, DCESummary, isCollectionElement, isHDCA, SubCollection } from "@/stores/services";
 
 import CollectionDetails from "./CollectionDetails.vue";
 import CollectionNavigation from "./CollectionNavigation.vue";
@@ -17,7 +17,7 @@ import ListingLayout from "@/components/History/Layout/ListingLayout.vue";
 
 interface Props {
     history: HistorySummary;
-    selectedCollections: HDCASummary[];
+    selectedCollections: CollectionEntry[];
     showControls?: boolean;
     filterable?: boolean;
 }
@@ -30,17 +30,29 @@ const props = withDefaults(defineProps<Props>(), {
 const collectionElementsStore = useCollectionElementsStore();
 
 const emit = defineEmits<{
-    (e: "view-collection", collection: HDCASummary): void;
-    (e: "update:selected-collections", collections: HDCASummary[]): void;
+    (e: "view-collection", collection: CollectionEntry): void;
+    (e: "update:selected-collections", collections: CollectionEntry[]): void;
 }>();
 
 const offset = ref(0);
 
-const dsc = computed(() => props.selectedCollections[props.selectedCollections.length - 1] as HDCASummary);
+const dsc = computed(() => {
+    const currentCollection = props.selectedCollections[props.selectedCollections.length - 1];
+    if (currentCollection === undefined) {
+        throw new Error("No collection selected");
+    }
+    return currentCollection;
+});
 const collectionElements = computed(() => collectionElementsStore.getCollectionElements(dsc.value, offset.value));
 const loading = computed(() => collectionElementsStore.isLoadingCollectionElements(dsc.value));
-const jobState = computed(() => dsc.value?.job_state_summary);
-const rootCollection = computed(() => props.selectedCollections[0]);
+const jobState = computed(() => ("job_state_summary" in dsc.value ? dsc.value.job_state_summary : undefined));
+const rootCollection = computed(() => {
+    if (isHDCA(props.selectedCollections[0])) {
+        return props.selectedCollections[0];
+    } else {
+        throw new Error("Root collection must be an HistoryDatasetCollectionAssociation");
+    }
+});
 const isRoot = computed(() => dsc.value == rootCollection.value);
 
 function updateDsc(collection: any, fields: Object | undefined) {
@@ -59,8 +71,15 @@ function onScroll(newOffset: number) {
     offset.value = newOffset;
 }
 
-async function onViewSubCollection(itemObject: DCObject) {
-    const collection = await collectionElementsStore.getCollection(itemObject.id);
+async function onViewDatasetCollectionElement(element: DCESummary) {
+    if (!isCollectionElement(element)) {
+        return;
+    }
+    const collection: SubCollection = {
+        ...element.object,
+        name: element.element_identifier,
+        hdca_id: rootCollection.value.id,
+    };
     emit("view-collection", collection);
 }
 
@@ -106,7 +125,7 @@ watch(
                                 :is-dataset="item.element_type == 'hda'"
                                 :filterable="filterable"
                                 @update:expand-dataset="setExpanded(item, $event)"
-                                @view-collection="onViewSubCollection" />
+                                @view-collection="onViewDatasetCollectionElement(item)" />
                         </template>
                     </ListingLayout>
                 </div>

client/src/stores/collectionElementsStore.test.ts

@@ -46,7 +46,8 @@ describe("useCollectionElementsStore", () => {
         expect(store.isLoadingCollectionElements(collection1)).toEqual(false);
         expect(fetchCollectionElements).toHaveBeenCalled();
 
-        const elements = store.storedCollectionElements[collection1.id];
+        const collection1Key = store.getCollectionKey(collection1);
+        const elements = store.storedCollectionElements[collection1Key];
         expect(elements).toBeDefined();
         expect(elements).toHaveLength(limit);
     });
@@ -55,8 +56,9 @@ describe("useCollectionElementsStore", () => {
         const store = useCollectionElementsStore();
         const storedCount = 5;
         const expectedStoredElements = Array.from({ length: storedCount }, (_, i) => mockElement(collection1.id, i));
-        store.storedCollectionElements[collection1.id] = expectedStoredElements;
-        expect(store.storedCollectionElements[collection1.id]).toHaveLength(storedCount);
+        const collection1Key = store.getCollectionKey(collection1);
+        store.storedCollectionElements[collection1Key] = expectedStoredElements;
+        expect(store.storedCollectionElements[collection1Key]).toHaveLength(storedCount);
 
         const offset = 0;
         const limit = 5;
@@ -70,8 +72,9 @@ describe("useCollectionElementsStore", () => {
         const store = useCollectionElementsStore();
         const storedCount = 3;
         const expectedStoredElements = Array.from({ length: storedCount }, (_, i) => mockElement(collection1.id, i));
-        store.storedCollectionElements[collection1.id] = expectedStoredElements;
-        expect(store.storedCollectionElements[collection1.id]).toHaveLength(storedCount);
+        const collection1Key = store.getCollectionKey(collection1);
+        store.storedCollectionElements[collection1Key] = expectedStoredElements;
+        expect(store.storedCollectionElements[collection1Key]).toHaveLength(storedCount);
 
         const offset = 2;
         const limit = 5;
@@ -82,7 +85,7 @@ describe("useCollectionElementsStore", () => {
         expect(store.isLoadingCollectionElements(collection1)).toEqual(false);
         expect(fetchCollectionElements).toHaveBeenCalled();
 
-        const elements = store.storedCollectionElements[collection1.id];
+        const elements = store.storedCollectionElements[collection1Key];
         expect(elements).toBeDefined();
         // The offset was overlapping with the stored elements, so it was increased by the number of stored elements
         // so it fetches the next "limit" number of elements

client/src/stores/collectionElementsStore.ts

@@ -1,32 +1,45 @@
 import { defineStore } from "pinia";
 import Vue, { computed, ref } from "vue";
 
-import { DCESummary, HDCASummary, HistoryContentItemBase } from "./services";
+import { CollectionEntry, DCESummary, HDCASummary, HistoryContentItemBase, isHDCA } from "./services";
 import * as Service from "./services/datasetCollection.service";
 
 export const useCollectionElementsStore = defineStore("collectionElementsStore", () => {
     const storedCollections = ref<{ [key: string]: HDCASummary }>({});
     const loadingCollectionElements = ref<{ [key: string]: boolean }>({});
     const storedCollectionElements = ref<{ [key: string]: DCESummary[] }>({});
 
+    /**
+     * Returns a key that can be used to store or retrieve the elements of a collection in the store.
+     *
+     * It consistently returns a DatasetCollection ID for (top level) HDCAs or sub-collections.
+     */
+    function getCollectionKey(collection: CollectionEntry): string {
+        if (isHDCA(collection)) {
+            return collection.collection_id;
+        }
+        return collection.id;
+    }
+
     const getCollectionElements = computed(() => {
-        return (collection: HDCASummary, offset = 0, limit = 50) => {
-            const elements = storedCollectionElements.value[collection.id] ?? [];
+        return (collection: CollectionEntry, offset = 0, limit = 50) => {
+            const elements = storedCollectionElements.value[getCollectionKey(collection)] ?? [];
             fetchMissingElements({ collection, offset, limit });
             return elements ?? null;
         };
     });
 
     const isLoadingCollectionElements = computed(() => {
-        return (collection: HDCASummary) => {
-            return loadingCollectionElements.value[collection.id] ?? false;
+        return (collection: CollectionEntry) => {
+            return loadingCollectionElements.value[getCollectionKey(collection)] ?? false;
         };
     });
 
-    async function fetchMissingElements(params: { collection: HDCASummary; offset: number; limit: number }) {
+    async function fetchMissingElements(params: { collection: CollectionEntry; offset: number; limit: number }) {
+        const collectionKey = getCollectionKey(params.collection);
         try {
             const maxElementCountInCollection = params.collection.element_count ?? 0;
-            const storedElements = storedCollectionElements.value[params.collection.id] ?? [];
+            const storedElements = storedCollectionElements.value[collectionKey] ?? [];
             // Collections are immutable, so there is no need to fetch elements if the range we want is already stored
             if (params.offset + params.limit <= storedElements.length) {
                 return;
@@ -38,22 +51,22 @@ export const useCollectionElementsStore = defineStore("collectionElementsStore",
                 return;
             }
 
-            Vue.set(loadingCollectionElements.value, params.collection.id, true);
-            const fetchedElements = await Service.fetchElementsFromHDCA({
-                hdca: params.collection,
+            Vue.set(loadingCollectionElements.value, collectionKey, true);
+            const fetchedElements = await Service.fetchElementsFromCollection({
+                entry: params.collection,
                 offset: params.offset,
                 limit: params.limit,
             });
             const updatedElements = [...storedElements, ...fetchedElements];
-            Vue.set(storedCollectionElements.value, params.collection.id, updatedElements);
+            Vue.set(storedCollectionElements.value, collectionKey, updatedElements);
         } finally {
-            Vue.delete(loadingCollectionElements.value, params.collection.id);
+            Vue.delete(loadingCollectionElements.value, collectionKey);
         }
     }
 
-    async function loadCollectionElements(collection: HDCASummary) {
-        const elements = await Service.fetchElementsFromHDCA({ hdca: collection });
-        Vue.set(storedCollectionElements.value, collection.id, elements);
+    async function loadCollectionElements(collection: CollectionEntry) {
+        const elements = await Service.fetchElementsFromCollection({ entry: collection });
+        Vue.set(storedCollectionElements.value, getCollectionKey(collection), elements);
     }
 
     function saveCollections(historyContentsPayload: HistoryContentItemBase[]) {
@@ -93,5 +106,6 @@ export const useCollectionElementsStore = defineStore("collectionElementsStore",
         fetchCollection,
         loadCollectionElements,
         saveCollections,
+        getCollectionKey,
     };
 });

client/src/stores/services/datasetCollection.service.ts

@@ -1,6 +1,6 @@
 import { fetcher } from "@/schema";
 
-import { DCESummary, HDCADetailed, HDCASummary } from ".";
+import { CollectionEntry, DCESummary, HDCADetailed, isHDCA } from ".";
 
 const DEFAULT_LIMIT = 50;
 
@@ -17,9 +17,13 @@ const getCollectionContents = fetcher
     .create();
 
 export async function fetchCollectionElements(params: {
+    /** The ID of the top level HDCA that associates this collection with the History it belongs to. */
     hdcaId: string;
+    /** The ID of the collection itself. */
     collectionId: string;
+    /** The offset to start fetching elements from. */
     offset?: number;
+    /** The maximum number of elements to fetch. */
     limit?: number;
 }): Promise<DCESummary[]> {
     const { data } = await getCollectionContents({
@@ -32,14 +36,19 @@ export async function fetchCollectionElements(params: {
     return data;
 }
 
-export async function fetchElementsFromHDCA(params: {
-    hdca: HDCASummary;
+export async function fetchElementsFromCollection(params: {
+    /** The HDCA or sub-collection to fetch elements from. */
+    entry: CollectionEntry;
+    /** The offset to start fetching elements from. */
     offset?: number;
+    /** The maximum number of elements to fetch. */
     limit?: number;
 }): Promise<DCESummary[]> {
+    const hdcaId = isHDCA(params.entry) ? params.entry.id : params.entry.hdca_id;
+    const collectionId = isHDCA(params.entry) ? params.entry.collection_id : params.entry.id;
     return fetchCollectionElements({
-        hdcaId: params.hdca.id,
-        collectionId: params.hdca.collection_id,
+        hdcaId: hdcaId,
+        collectionId: collectionId,
         offset: params.offset ?? 0,
         limit: params.limit ?? DEFAULT_LIMIT,
     });

client/src/stores/services/index.ts

@@ -1,12 +1,94 @@
 import { components } from "@/schema";
 
+/**
+ * Contains minimal information about a HistoryContentItem.
+ */
+export type HistoryContentItemBase = components["schemas"]["EncodedHistoryContentItem"];
+
+/**
+ * Contains summary information about a HistoryDatasetAssociation.
+ */
 export type DatasetSummary = components["schemas"]["HDASummary"];
+
+/**
+ * Contains additional details about a HistoryDatasetAssociation.
+ */
 export type DatasetDetails = components["schemas"]["HDADetailed"];
+
+/**
+ * Represents a HistoryDatasetAssociation with either summary or detailed information.
+ */
+export type DatasetEntry = DatasetSummary | DatasetDetails;
+
+/**
+ * Contains summary information about a DCE (DatasetCollectionElement).
+ *
+ * DCEs associate a parent collection to its elements. Those elements can be either
+ * HDAs or other DCs (DatasetCollections).
+ * The type of the element is indicated by the `element_type` field and the element
+ * itself is contained in the `object` field.
+ */
 export type DCESummary = components["schemas"]["DCESummary"];
+
+/**
+ * DatasetCollectionElement specific type for collections.
+ */
+export interface DCECollection extends DCESummary {
+    element_type: "dataset_collection";
+    object: DCObject;
+}
+
+/**
+ * Contains summary information about a HDCA (HistoryDatasetCollectionAssociation).
+ *
+ * HDCAs are (top level only) history items that contains information about the association
+ * between a History and a DatasetCollection.
+ */
 export type HDCASummary = components["schemas"]["HDCASummary"];
+
+/**
+ * Contains additional details about a HistoryDatasetCollectionAssociation.
+ */
 export type HDCADetailed = components["schemas"]["HDCADetailed"];
+
+/**
+ * Contains information about a DatasetCollection.
+ *
+ * DatasetCollections are immutable and contain one or more DCEs.
+ */
 export type DCObject = components["schemas"]["DCObject"];
 
-export type HistoryContentItemBase = components["schemas"]["EncodedHistoryContentItem"];
+/**
+ * A SubCollection is a DatasetCollectionElement of type `dataset_collection`
+ * with additional information to simplify its handling.
+ *
+ * This is used to be able to distinguish between top level HDCAs and sub-collections.
+ * It helps simplify both, the representation of sub-collections in the UI, and fetching of elements.
+ */
+export interface SubCollection extends DCObject {
+    /** The name of the collection. Usually corresponds to the DCE identifier. */
+    name: string;
+    /** The ID of the top level HDCA that associates this collection with the History it belongs to. */
+    hdca_id: string;
+}
 
-export type DatasetEntry = DatasetSummary | DatasetDetails;
+/**
+ * Represents either a top level HDCASummary or a sub-collection.
+ */
+export type CollectionEntry = HDCASummary | SubCollection;
+
+/**
+ * Returns true if the given entry is a top level HDCA and false for sub-collections.
+ */
+export function isHDCA(entry?: CollectionEntry): entry is HDCASummary {
+    return (
+        entry !== undefined && "history_content_type" in entry && entry.history_content_type === "dataset_collection"
+    );
+}
+
+/**
+ * Returns true if the given element of a collection is a DatasetCollection.
+ */
+export function isCollectionElement(element: DCESummary): element is DCECollection {
+    return element.element_type === "dataset_collection";
+}