Fix directory expansion logic to ensure unique parent directories are fetched and sorted by depth (#822)

Signed-off-by: achour94 <[email protected]>

Author: achour94

src/components/directoryItemSelector/DirectoryItemSelector.tsx

@@ -260,21 +260,21 @@ export function DirectoryItemSelector({
     }, [convertRoots, types, snackError]);
 
     const fetchDirectoryChildren = useCallback(
-        (nodeId: UUID): void => {
+        (nodeId: UUID): Promise<void> => {
             const typeList = types.includes(ElementType.DIRECTORY) ? [] : types;
-            fetchDirectoryContent(nodeId, typeList)
+            return fetchDirectoryContent(nodeId, typeList)
                 .then((children) => {
                     const childrenMatchedTypes = children.filter((item: ElementAttributes) =>
                         contentFilter().has(item.type)
                     );
 
                     if (childrenMatchedTypes.length > 0 && equipmentTypes && equipmentTypes.length > 0) {
-                        fetchElementsInfos(
+                        return fetchElementsInfos(
                             childrenMatchedTypes.map((e: ElementAttributes) => e.elementUuid),
                             types,
                             equipmentTypes
                         ).then((childrenWithMetadata: ElementAttributes[]) => {
-                            const filtredChildren = itemFilter
+                            const filteredChildren = itemFilter
                                 ? childrenWithMetadata.filter((val: ElementAttributes) => {
                                       // Accept every directory
                                       if (val.type === ElementType.DIRECTORY) {
@@ -285,12 +285,12 @@ export function DirectoryItemSelector({
                                   })
                                 : childrenWithMetadata;
                             // update directory content
-                            addToDirectory(nodeId, filtredChildren);
+                            addToDirectory(nodeId, filteredChildren);
                         });
-                    } else {
-                        // update directory content
-                        addToDirectory(nodeId, childrenMatchedTypes);
                     }
+                    // update directory content
+                    addToDirectory(nodeId, childrenMatchedTypes);
+                    return Promise.resolve();
                 })
                 .catch((error) => {
                     console.warn(`Could not update subs (and content) of '${nodeId}' : ${error.message}`);
@@ -301,13 +301,11 @@ export function DirectoryItemSelector({
 
     // Helper function to fetch children for a node if not already loaded
     const fetchNodeChildrenIfNeeded = useCallback(
-        (nodeId: UUID, delay: number = 0) => {
-            setTimeout(() => {
-                const node = nodeMap.current[nodeId];
-                if (node && (!node.children || node.children.length === 0) && node.type === ElementType.DIRECTORY) {
-                    fetchDirectoryChildren(nodeId);
-                }
-            }, delay);
+        async (nodeId: UUID): Promise<void> => {
+            const node = nodeMap.current[nodeId];
+            if (node && (!node.children || node.children.length === 0) && node.type === ElementType.DIRECTORY) {
+                await fetchDirectoryChildren(nodeId);
+            }
         },
         [fetchDirectoryChildren]
     );
@@ -320,18 +318,18 @@ export function DirectoryItemSelector({
 
         const expandedArray = await getExpansionPathsForSelected(selected, expanded);
         setAutoExpandedNodes(expandedArray);
-        fetchChildrenForExpandedNodes(expandedArray, fetchNodeChildrenIfNeeded);
+        await fetchChildrenForExpandedNodes(expandedArray, fetchNodeChildrenIfNeeded);
         return true;
     }, [selected, expanded, fetchNodeChildrenIfNeeded]);
 
     // Handle expansion from provided expanded prop
-    const handleProvidedExpansion = useCallback((): boolean => {
+    const handleProvidedExpansion = useCallback(async (): Promise<boolean> => {
         if (!expanded || expanded.length === 0) {
             return false;
         }
 
         setAutoExpandedNodes(expanded);
-        fetchChildrenForExpandedNodes(expanded, fetchNodeChildrenIfNeeded);
+        await fetchChildrenForExpandedNodes(expanded, fetchNodeChildrenIfNeeded);
 
         return true;
     }, [expanded, fetchNodeChildrenIfNeeded]);
@@ -345,7 +343,7 @@ export function DirectoryItemSelector({
         }
 
         setAutoExpandedNodes(expandPath);
-        fetchChildrenForExpandedNodes(expandPath, fetchNodeChildrenIfNeeded);
+        await fetchChildrenForExpandedNodes(expandPath, fetchNodeChildrenIfNeeded);
 
         return true;
     }, [fetchNodeChildrenIfNeeded]);
@@ -354,11 +352,15 @@ export function DirectoryItemSelector({
     const initializeExpansion = useCallback(async () => {
         // Priority 1: Handle selected items
         const selectedSuccess = await handleSelectedExpansion();
-        if (selectedSuccess) return;
+        if (selectedSuccess) {
+            return;
+        }
 
         // Priority 2: Handle provided expanded items
-        const expandedSuccess = handleProvidedExpansion();
-        if (expandedSuccess) return;
+        const expandedSuccess = await handleProvidedExpansion();
+        if (expandedSuccess) {
+            return;
+        }
 
         // Priority 3: Fall back to last selected directory
         await handleLastSelectedExpansion();

src/components/directoryItemSelector/utils.ts

@@ -92,29 +92,40 @@ export async function initializeFromLastSelected(): Promise<UUID[] | null> {
 }
 
 /**
- * Fetches expansion paths for multiple selected items
+ * Fetches expansion paths for multiple selected items, collecting unique parent directories
+ * (excluding the selected items themselves) and sorting them by their minimum depth across
+ * all paths. This ensures parents appear before their descendants in the returned array,
+ * which is crucial for sequential fetching to avoid loading children before parents are
+ * fully populated in the node map.
  * @param selectedIds Array of selected item UUIDs
  * @param expanded Optional existing expanded nodes
- * @returns Promise resolving to combined expansion array
+ * @returns Promise resolving to combined expansion array sorted by depth
  */
 export async function getExpansionPathsForSelected(selectedIds: UUID[], expanded: UUID[] = []): Promise<UUID[]> {
     const expandedSet = new Set<UUID>(expanded);
+    const idToMinIndex = new Map<UUID, number>();
 
-    const fetchPromises = selectedIds.map(async (selectedId) => {
-        const path = await fetchDirectoryPathSafe(selectedId);
+    const paths = await Promise.all(selectedIds.map(fetchDirectoryPathSafe));
 
-        if (path && path.length > 0) {
-            // Add all parent directories to the expanded set (exclude the item itself)
+    paths
+        .filter((p): p is ElementAttributes[] => !!p && p.length > 0)
+        .forEach((path) => {
             path.forEach((element, index) => {
                 if (index < path.length - 1) {
-                    expandedSet.add(element.elementUuid);
+                    const id = element.elementUuid;
+                    expandedSet.add(id);
+                    if (!idToMinIndex.has(id) || index < idToMinIndex.get(id)!) {
+                        idToMinIndex.set(id, index);
+                    }
                 }
             });
-        }
-    });
+        });
+
+    const expandedArray = Array.from(expandedSet).sort(
+        (a, b) => (idToMinIndex.get(a) ?? Infinity) - (idToMinIndex.get(b) ?? Infinity)
+    );
 
-    await Promise.all(fetchPromises);
-    return Array.from(expandedSet);
+    return expandedArray;
 }
 
 /**
@@ -142,17 +153,16 @@ export async function saveLastSelectedDirectoryFromNode(node: {
 }
 
 /**
- * Fetches children for expanded nodes with staggered delay to avoid overwhelming the server
+ * Fetches children for expanded nodes sequentially to ensure parent nodes are loaded before children
  * @param expandedNodes Array of node UUIDs to fetch children for
  * @param fetchChildrenCallback Function to fetch children for a single node
- * @param delayBetweenRequests Delay in milliseconds between requests (default: 100ms)
  */
-export function fetchChildrenForExpandedNodes(
+export async function fetchChildrenForExpandedNodes(
     expandedNodes: UUID[],
-    fetchChildrenCallback: (nodeId: UUID, delay: number) => void,
-    delayBetweenRequests: number = 100
-): void {
-    expandedNodes.forEach((nodeId, index) => {
-        fetchChildrenCallback(nodeId, index * delayBetweenRequests);
-    });
+    fetchChildrenCallback: (nodeId: UUID) => Promise<void>
+): Promise<void> {
+    await expandedNodes.reduce(async (promise, nodeId) => {
+        await promise;
+        return fetchChildrenCallback(nodeId);
+    }, Promise.resolve());
 }