NestedFolderPicker: Seperate state from Browse Dashboards (#82672)

* initial very very early stab at isolated state for nested folder picker

* more

* complete state rework. still need to do search

* tidy up some comments

* split api hook into seperate file, start to try and get search results back (its not working)

* Fix loading status

* Reset files

* cleanup

* fix tests

* return object

* restore hiding items

* restore

* restore

* remove those comments

* rename hooks

* rename hooks

* simplify selectors - thanks ash!!!

* more ci please?

public/app/core/components/NestedFolderPicker/NestedFolderPicker.test.tsx

@@ -46,14 +46,37 @@ describe('NestedFolderPicker', () => {
 
   beforeAll(() => {
     window.HTMLElement.prototype.scrollIntoView = function () {};
+
     server = setupServer(
       http.get('/api/folders/:uid', () => {
         return HttpResponse.json({
           title: folderA.item.title,
           uid: folderA.item.uid,
         });
+      }),
+
+      http.get('/api/folders', ({ request }) => {
+        const url = new URL(request.url);
+        const parentUid = url.searchParams.get('parentUid') ?? undefined;
+
+        const limit = parseInt(url.searchParams.get('limit') ?? '1000', 10);
+        const page = parseInt(url.searchParams.get('page') ?? '1', 10);
+
+        // reconstruct a folder API response from the flat tree fixture
+        const folders = mockTree
+          .filter((v) => v.item.kind === 'folder' && v.item.parentUID === parentUid)
+          .map((folder) => {
+            return {
+              uid: folder.item.uid,
+              title: folder.item.kind === 'folder' ? folder.item.title : "invalid - this shouldn't happen",
+            };
+          })
+          .slice(limit * (page - 1), limit * page);
+
+        return HttpResponse.json(folders);
       })
     );
+
     server.listen();
   });
 
@@ -127,6 +150,40 @@ describe('NestedFolderPicker', () => {
     expect(mockOnChange).toHaveBeenCalledWith(folderA.item.uid, folderA.item.title);
   });
 
+  it('shows the root folder by default', async () => {
+    render(<NestedFolderPicker onChange={mockOnChange} />);
+
+    // Open the picker and wait for children to load
+    const button = await screen.findByRole('button', { name: 'Select folder' });
+    await userEvent.click(button);
+    await screen.findByLabelText(folderA.item.title);
+
+    await userEvent.click(screen.getByLabelText('Dashboards'));
+    expect(mockOnChange).toHaveBeenCalledWith('', 'Dashboards');
+  });
+
+  it('hides the root folder if the prop says so', async () => {
+    render(<NestedFolderPicker showRootFolder={false} onChange={mockOnChange} />);
+
+    // Open the picker and wait for children to load
+    const button = await screen.findByRole('button', { name: 'Select folder' });
+    await userEvent.click(button);
+    await screen.findByLabelText(folderA.item.title);
+
+    expect(screen.queryByLabelText('Dashboards')).not.toBeInTheDocument();
+  });
+
+  it('hides folders specififed by UID', async () => {
+    render(<NestedFolderPicker excludeUIDs={[folderA.item.uid]} onChange={mockOnChange} />);
+
+    // Open the picker and wait for children to load
+    const button = await screen.findByRole('button', { name: 'Select folder' });
+    await userEvent.click(button);
+    await screen.findByLabelText(folderB.item.title);
+
+    expect(screen.queryByLabelText(folderA.item.title)).not.toBeInTheDocument();
+  });
+
   describe('when nestedFolders is enabled', () => {
     let originalToggles = { ...config.featureToggles };
 

public/app/core/components/NestedFolderPicker/NestedFolderPicker.tsx

@@ -8,25 +8,15 @@ import { config } from '@grafana/runtime';
 import { Alert, Icon, Input, LoadingBar, useStyles2 } from '@grafana/ui';
 import { t } from 'app/core/internationalization';
 import { skipToken, useGetFolderQuery } from 'app/features/browse-dashboards/api/browseDashboardsAPI';
-import { PAGE_SIZE } from 'app/features/browse-dashboards/api/services';
+import { DashboardViewItemWithUIItems, DashboardsTreeItem } from 'app/features/browse-dashboards/types';
-import {
-  childrenByParentUIDSelector,
-  createFlatTree,
-  fetchNextChildrenPage,
-  rootItemsSelector,
-  useBrowseLoadingStatus,
-  useLoadNextChildrenPage,
-} from 'app/features/browse-dashboards/state';
-import { getPaginationPlaceholders } from 'app/features/browse-dashboards/state/utils';
-import { DashboardViewItemCollection } from 'app/features/browse-dashboards/types';
 import { QueryResponse, getGrafanaSearcher } from 'app/features/search/service';
 import { queryResultToViewItem } from 'app/features/search/service/utils';
 import { DashboardViewItem } from 'app/features/search/types';
-import { useDispatch, useSelector } from 'app/types/store';
 
 import { getDOMId, NestedFolderList } from './NestedFolderList';
 import Trigger from './Trigger';
-import { useTreeInteractions } from './hooks';
+import { ROOT_FOLDER_ITEM, useFoldersQuery } from './useFoldersQuery';
+import { useTreeInteractions } from './useTreeInteractions';
 
 export interface NestedFolderPickerProps {
   /* Folder UID to show as selected */
@@ -48,8 +38,6 @@ export interface NestedFolderPickerProps {
   clearable?: boolean;
 }
 
-const EXCLUDED_KINDS = ['empty-folder' as const, 'dashboard' as const];
-
 const debouncedSearch = debounce(getSearchResults, 300);
 
 async function getSearchResults(searchQuery: string) {
@@ -72,10 +60,8 @@ export function NestedFolderPicker({
   onChange,
 }: NestedFolderPickerProps) {
   const styles = useStyles2(getStyles);
-  const dispatch = useDispatch();
   const selectedFolder = useGetFolderQuery(value || skipToken);
 
-  const rootStatus = useBrowseLoadingStatus(undefined);
   const nestedFoldersEnabled = Boolean(config.featureToggles.nestedFolders);
 
   const [search, setSearch] = useState('');
@@ -83,18 +69,27 @@ export function NestedFolderPicker({
   const [isFetchingSearchResults, setIsFetchingSearchResults] = useState(false);
   const [autoFocusButton, setAutoFocusButton] = useState(false);
   const [overlayOpen, setOverlayOpen] = useState(false);
-  const [folderOpenState, setFolderOpenState] = useState<Record<string, boolean>>({});
+  const [foldersOpenState, setFoldersOpenState] = useState<Record<string, boolean>>({});
   const overlayId = useId();
   const [error] = useState<Error | undefined>(undefined); // TODO: error not populated anymore
   const lastSearchTimestamp = useRef<number>(0);
 
+  const isBrowsing = Boolean(overlayOpen && !(search && searchResults));
+  const {
+    items: browseFlatTree,
+    isLoading: isBrowseLoading,
+    requestNextPage: fetchFolderPage,
+  } = useFoldersQuery(isBrowsing, foldersOpenState);
+
   useEffect(() => {
     if (!search) {
       setSearchResults(null);
       return;
     }
+
     const timestamp = Date.now();
     setIsFetchingSearchResults(true);
+
     debouncedSearch(search).then((queryResponse) => {
       // Only keep the results if it's was issued after the most recently resolved search.
       // This prevents results showing out of order if first request is slower than later ones.
@@ -109,9 +104,6 @@ export function NestedFolderPicker({
     });
   }, [search]);
 
-  const rootCollection = useSelector(rootItemsSelector);
-  const childrenCollections = useSelector(childrenByParentUIDSelector);
-
   // the order of middleware is important!
   const middleware = [
     flip({
@@ -143,13 +135,13 @@ export function NestedFolderPicker({
 
   const handleFolderExpand = useCallback(
     async (uid: string, newOpenState: boolean) => {
-      setFolderOpenState((old) => ({ ...old, [uid]: newOpenState }));
+      setFoldersOpenState((old) => ({ ...old, [uid]: newOpenState }));
 
-      if (newOpenState && !folderOpenState[uid]) {
+      if (newOpenState && !foldersOpenState[uid]) {
-        dispatch(fetchNextChildrenPage({ parentUID: uid, pageSize: PAGE_SIZE, excludeKinds: EXCLUDED_KINDS }));
+        fetchFolderPage(uid);
       }
     },
-    [dispatch, folderOpenState]
+    [fetchFolderPage, foldersOpenState]
   );
 
   const handleFolderSelect = useCallback(
@@ -175,69 +167,53 @@ export function NestedFolderPicker({
 
   const handleCloseOverlay = useCallback(() => setOverlayOpen(false), [setOverlayOpen]);
 
-  const baseHandleLoadMore = useLoadNextChildrenPage(EXCLUDED_KINDS);
   const handleLoadMore = useCallback(
     (folderUID: string | undefined) => {
       if (search) {
         return;
       }
 
-      baseHandleLoadMore(folderUID);
+      fetchFolderPage(folderUID);
     },
-    [search, baseHandleLoadMore]
+    [search, fetchFolderPage]
   );
 
   const flatTree = useMemo(() => {
-    if (search && searchResults) {
+    let flatTree: Array<DashboardsTreeItem<DashboardViewItemWithUIItems>> = [];
-      const searchCollection: DashboardViewItemCollection = {
-        isFullyLoaded: true, //searchResults.items.length === searchResults.totalRows,
-        lastKindHasMoreItems: false, // TODO: paginate search
-        lastFetchedKind: 'folder', // TODO: paginate search
-        lastFetchedPage: 1, // TODO: paginate search
-        items: searchResults.items ?? [],
-      };
 
-      return createFlatTree(undefined, searchCollection, childrenCollections, {}, 0, EXCLUDED_KINDS, excludeUIDs);
+    if (isBrowsing) {
+      flatTree = browseFlatTree;
+    } else {
+      flatTree =
+        searchResults?.items.map((item) => ({
+          isOpen: false,
+          level: 0,
+          item: {
+            kind: 'folder' as const,
+            title: item.title,
+            uid: item.uid,
+          },
+        })) ?? [];
     }
 
-    const allExcludedUIDs = config.sharedWithMeFolderUID
+    // It's not super optimal to filter these in an additional iteration, but
-      ? [...(excludeUIDs || []), config.sharedWithMeFolderUID]
+    // these options are used infrequently that its not a big deal
-      : excludeUIDs;
+    if (!showRootFolder || excludeUIDs?.length) {
+      flatTree = flatTree.filter((item) => {
+        if (!showRootFolder && item === ROOT_FOLDER_ITEM) {
+          return false;
+        }
 
-    let flatTree = createFlatTree(
+        if (excludeUIDs?.includes(item.item.uid)) {
-      undefined,
+          return false;
-      rootCollection,
+        }
-      childrenCollections,
-      folderOpenState,
-      0,
-      EXCLUDED_KINDS,
-      allExcludedUIDs
-    );
 
-    if (showRootFolder) {
+        return true;
-      // Increase the level of each item to 'make way' for the fake root Dashboards item
-      for (const item of flatTree) {
-        item.level += 1;
-      }
-
-      flatTree.unshift({
-        isOpen: true,
-        level: 0,
-        item: {
-          kind: 'folder',
-          title: 'Dashboards',
-          uid: '',
-        },
       });
     }
 
-    // If the root collection hasn't loaded yet, create loading placeholders
-    if (!rootCollection) {
-      flatTree = flatTree.concat(getPaginationPlaceholders(PAGE_SIZE, undefined, 0));
-    }
-
     return flatTree;
-  }, [search, searchResults, rootCollection, childrenCollections, folderOpenState, excludeUIDs, showRootFolder]);
+  }, [browseFlatTree, excludeUIDs, isBrowsing, searchResults?.items, showRootFolder]);
 
   const isItemLoaded = useCallback(
     (itemIndex: number) => {
@@ -245,6 +221,7 @@ export function NestedFolderPicker({
       if (!treeItem) {
         return false;
       }
+
       const item = treeItem.item;
       const result = !(item.kind === 'ui' && item.uiKind === 'pagination-placeholder');
 
@@ -253,7 +230,7 @@ export function NestedFolderPicker({
     [flatTree]
   );
 
-  const isLoading = rootStatus === 'pending' || isFetchingSearchResults;
+  const isLoading = isBrowseLoading || isFetchingSearchResults;
 
   const { focusedItemIndex, handleKeyDown } = useTreeInteractions({
     tree: flatTree,

public/app/core/components/NestedFolderPicker/useFoldersQuery.ts

@@ -0,0 +1,201 @@
+import { createSelector } from '@reduxjs/toolkit';
+import { QueryDefinition, BaseQueryFn } from '@reduxjs/toolkit/dist/query';
+import { QueryActionCreatorResult } from '@reduxjs/toolkit/dist/query/core/buildInitiate';
+import { RequestOptions } from 'http';
+import { useCallback, useEffect, useMemo, useRef } from 'react';
+
+import { ListFolderQueryArgs, browseDashboardsAPI } from 'app/features/browse-dashboards/api/browseDashboardsAPI';
+import { PAGE_SIZE } from 'app/features/browse-dashboards/api/services';
+import { getPaginationPlaceholders } from 'app/features/browse-dashboards/state/utils';
+import { DashboardViewItemWithUIItems, DashboardsTreeItem } from 'app/features/browse-dashboards/types';
+import { RootState } from 'app/store/configureStore';
+import { FolderListItemDTO } from 'app/types';
+import { useDispatch, useSelector } from 'app/types/store';
+
+type ListFoldersQuery = ReturnType<ReturnType<typeof browseDashboardsAPI.endpoints.listFolders.select>>;
+type ListFoldersRequest = QueryActionCreatorResult<
+  QueryDefinition<
+    ListFolderQueryArgs,
+    BaseQueryFn<RequestOptions>,
+    'getFolder',
+    FolderListItemDTO[],
+    'browseDashboardsAPI'
+  >
+>;
+
+const listFoldersSelector = createSelector(
+  (state: RootState) => state,
+  (
+    state: RootState,
+    parentUid: ListFolderQueryArgs['parentUid'],
+    page: ListFolderQueryArgs['page'],
+    limit: ListFolderQueryArgs['limit']
+  ) => browseDashboardsAPI.endpoints.listFolders.select({ parentUid, page, limit }),
+  (state, selectFolderList) => selectFolderList(state)
+);
+
+const listAllFoldersSelector = createSelector(
+  [(state: RootState) => state, (state: RootState, requests: ListFoldersRequest[]) => requests],
+  (state: RootState, requests: ListFoldersRequest[]) => {
+    const seenRequests = new Set<string>();
+
+    const rootPages: ListFoldersQuery[] = [];
+    const pagesByParent: Record<string, ListFoldersQuery[]> = {};
+    let isLoading = false;
+
+    for (const req of requests) {
+      if (seenRequests.has(req.requestId)) {
+        continue;
+      }
+
+      const page = listFoldersSelector(state, req.arg.parentUid, req.arg.page, req.arg.limit);
+      if (page.status === 'pending') {
+        isLoading = true;
+      }
+
+      const parentUid = page.originalArgs?.parentUid;
+      if (parentUid) {
+        if (!pagesByParent[parentUid]) {
+          pagesByParent[parentUid] = [];
+        }
+
+        pagesByParent[parentUid].push(page);
+      } else {
+        rootPages.push(page);
+      }
+    }
+
+    return {
+      isLoading,
+      rootPages,
+      pagesByParent,
+    };
+  }
+);
+
+/**
+ * Returns the whether the set of pages are 'fully loaded', and the last page number
+ */
+function getPagesLoadStatus(pages: ListFoldersQuery[]): [boolean, number | undefined] {
+  const lastPage = pages.at(-1);
+  const lastPageNumber = lastPage?.originalArgs?.page;
+
+  if (!lastPage?.data) {
+    // If there's no pages yet, or the last page is still loading
+    return [false, lastPageNumber];
+  } else {
+    return [lastPage.data.length < lastPage.originalArgs.limit, lastPageNumber];
+  }
+}
+
+/**
+ * Returns a loaded folder hierarchy as a flat list and a function to load more pages.
+ */
+export function useFoldersQuery(isBrowsing: boolean, openFolders: Record<string, boolean>) {
+  const dispatch = useDispatch();
+
+  // Keep a list of all requests so we can
+  //   a) unsubscribe from them when the component is unmounted
+  //   b) use them to select the responses out of the state
+  const requestsRef = useRef<ListFoldersRequest[]>([]);
+
+  const state = useSelector((rootState: RootState) => {
+    return listAllFoldersSelector(rootState, requestsRef.current);
+  });
+
+  // Loads the next page of folders for the given parent UID by inspecting the
+  // state to determine what the next page is
+  const requestNextPage = useCallback(
+    (parentUid: string | undefined) => {
+      const pages = parentUid ? state.pagesByParent[parentUid] : state.rootPages;
+      const [fullyLoaded, pageNumber] = getPagesLoadStatus(pages ?? []);
+      if (fullyLoaded) {
+        return;
+      }
+
+      const args = { parentUid, page: (pageNumber ?? 0) + 1, limit: PAGE_SIZE };
+      const promise = dispatch(browseDashboardsAPI.endpoints.listFolders.initiate(args));
+
+      // It's important that we create a new array so we can correctly memoize with it
+      requestsRef.current = requestsRef.current.concat([promise]);
+    },
+    [state, dispatch]
+  );
+
+  // Unsubscribe from all requests when the component is unmounted
+  useEffect(() => {
+    return () => {
+      for (const req of requestsRef.current) {
+        req.unsubscribe();
+      }
+    };
+  }, []);
+
+  // Convert the individual responses into a flat list of folders, with level indicating
+  // the depth in the hierarchy.
+  const treeList = useMemo(() => {
+    if (!isBrowsing) {
+      return [];
+    }
+
+    function createFlatList(
+      parentUid: string | undefined,
+      pages: ListFoldersQuery[],
+      level: number
+    ): Array<DashboardsTreeItem<DashboardViewItemWithUIItems>> {
+      const flatList = pages.flatMap((page) => {
+        const pageItems = page.data ?? [];
+
+        return pageItems.flatMap((item) => {
+          const folderIsOpen = openFolders[item.uid];
+
+          const flatItem: DashboardsTreeItem<DashboardViewItemWithUIItems> = {
+            isOpen: Boolean(folderIsOpen),
+            level: level,
+            item: {
+              kind: 'folder' as const,
+              title: item.title,
+              uid: item.uid,
+            },
+          };
+
+          const childPages = folderIsOpen && state.pagesByParent[item.uid];
+          if (childPages) {
+            const childFlatItems = createFlatList(item.uid, childPages, level + 1);
+            return [flatItem, ...childFlatItems];
+          }
+
+          return flatItem;
+        });
+      });
+
+      const [fullyLoaded] = getPagesLoadStatus(pages);
+      if (!fullyLoaded) {
+        flatList.push(...getPaginationPlaceholders(PAGE_SIZE, parentUid, level));
+      }
+
+      return flatList;
+    }
+
+    const rootFlatTree = createFlatList(undefined, state.rootPages, 1);
+    rootFlatTree.unshift(ROOT_FOLDER_ITEM);
+
+    return rootFlatTree;
+  }, [state, isBrowsing, openFolders]);
+
+  return {
+    items: treeList,
+    isLoading: state.isLoading,
+    requestNextPage,
+  };
+}
+
+export const ROOT_FOLDER_ITEM = {
+  isOpen: true,
+  level: 0,
+  item: {
+    kind: 'folder' as const,
+    title: 'Dashboards',
+    uid: '',
+  },
+};

public/app/features/browse-dashboards/api/browseDashboardsAPI.ts

@@ -14,6 +14,7 @@ import {
   DescendantCount,
   DescendantCountDTO,
   FolderDTO,
+  FolderListItemDTO,
   ImportDashboardResponseDTO,
   SaveDashboardResponseDTO,
 } from 'app/types';
@@ -69,11 +70,22 @@ function createBackendSrvBaseQuery({ baseURL }: { baseURL: string }): BaseQueryF
   return backendSrvBaseQuery;
 }
 
+export interface ListFolderQueryArgs {
+  page: number;
+  parentUid: string | undefined;
+  limit: number;
+}
+
 export const browseDashboardsAPI = createApi({
   tagTypes: ['getFolder'],
   reducerPath: 'browseDashboardsAPI',
   baseQuery: createBackendSrvBaseQuery({ baseURL: '/api' }),
   endpoints: (builder) => ({
+    listFolders: builder.query<FolderListItemDTO[], ListFolderQueryArgs>({
+      providesTags: (result) => result?.map((folder) => ({ type: 'getFolder', id: folder.uid })) ?? [],
+      query: ({ page, parentUid, limit }) => ({ url: '/folders', params: { page, parentUid, limit } }),
+    }),
+
     // get folder info (e.g. title, parents) but *not* children
     getFolder: builder.query<FolderDTO, string>({
       providesTags: (_result, _error, folderUID) => [{ type: 'getFolder', id: folderUID }],
@@ -360,4 +372,5 @@ export const {
   useSaveDashboardMutation,
   useSaveFolderMutation,
 } = browseDashboardsAPI;
+
 export { skipToken } from '@reduxjs/toolkit/query/react';

public/app/types/folders.ts

@@ -1,5 +1,10 @@
 import { WithAccessControlMetadata } from '@grafana/data';
 
+export interface FolderListItemDTO {
+  uid: string;
+  title: string;
+}
+
 export interface FolderDTO extends WithAccessControlMetadata {
   canAdmin: boolean;
   canDelete: boolean;