From 030b83d8f2278a7d46283c9c474636d530ff348c Mon Sep 17 00:00:00 2001 From: Josh Hunt Date: Wed, 21 Feb 2024 18:02:37 +0000 Subject: [PATCH] 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? --- .../NestedFolderPicker.test.tsx | 57 +++++ .../NestedFolderPicker/NestedFolderPicker.tsx | 117 ++++------ .../NestedFolderPicker/useFoldersQuery.ts | 201 ++++++++++++++++++ .../{hooks.ts => useTreeInteractions.ts} | 0 .../api/browseDashboardsAPI.ts | 13 ++ public/app/types/folders.ts | 5 + 6 files changed, 323 insertions(+), 70 deletions(-) create mode 100644 public/app/core/components/NestedFolderPicker/useFoldersQuery.ts rename public/app/core/components/NestedFolderPicker/{hooks.ts => useTreeInteractions.ts} (100%) diff --git a/public/app/core/components/NestedFolderPicker/NestedFolderPicker.test.tsx b/public/app/core/components/NestedFolderPicker/NestedFolderPicker.test.tsx index 73368d51752..a6cb752fb52 100644 --- a/public/app/core/components/NestedFolderPicker/NestedFolderPicker.test.tsx +++ b/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(); + + // 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(); + + // 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(); + + // 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 }; diff --git a/public/app/core/components/NestedFolderPicker/NestedFolderPicker.tsx b/public/app/core/components/NestedFolderPicker/NestedFolderPicker.tsx index 7dd455e0780..1f2ab2739d8 100644 --- a/public/app/core/components/NestedFolderPicker/NestedFolderPicker.tsx +++ b/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 { - 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 { DashboardViewItemWithUIItems, DashboardsTreeItem } 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>({}); + const [foldersOpenState, setFoldersOpenState] = useState>({}); const overlayId = useId(); const [error] = useState(undefined); // TODO: error not populated anymore const lastSearchTimestamp = useRef(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]) { - dispatch(fetchNextChildrenPage({ parentUID: uid, pageSize: PAGE_SIZE, excludeKinds: EXCLUDED_KINDS })); + if (newOpenState && !foldersOpenState[uid]) { + 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) { - 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 ?? [], - }; + let flatTree: Array> = []; - 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 - ? [...(excludeUIDs || []), config.sharedWithMeFolderUID] - : excludeUIDs; + // It's not super optimal to filter these in an additional iteration, but + // these options are used infrequently that its not a big deal + if (!showRootFolder || excludeUIDs?.length) { + flatTree = flatTree.filter((item) => { + if (!showRootFolder && item === ROOT_FOLDER_ITEM) { + return false; + } - let flatTree = createFlatTree( - undefined, - rootCollection, - childrenCollections, - folderOpenState, - 0, - EXCLUDED_KINDS, - allExcludedUIDs - ); + if (excludeUIDs?.includes(item.item.uid)) { + return false; + } - if (showRootFolder) { - // 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: '', - }, + return true; }); } - // 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, diff --git a/public/app/core/components/NestedFolderPicker/useFoldersQuery.ts b/public/app/core/components/NestedFolderPicker/useFoldersQuery.ts new file mode 100644 index 00000000000..528ca80ab1c --- /dev/null +++ b/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>; +type ListFoldersRequest = QueryActionCreatorResult< + QueryDefinition< + ListFolderQueryArgs, + BaseQueryFn, + '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(); + + const rootPages: ListFoldersQuery[] = []; + const pagesByParent: Record = {}; + 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) { + 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([]); + + 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> { + const flatList = pages.flatMap((page) => { + const pageItems = page.data ?? []; + + return pageItems.flatMap((item) => { + const folderIsOpen = openFolders[item.uid]; + + const flatItem: DashboardsTreeItem = { + 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: '', + }, +}; diff --git a/public/app/core/components/NestedFolderPicker/hooks.ts b/public/app/core/components/NestedFolderPicker/useTreeInteractions.ts similarity index 100% rename from public/app/core/components/NestedFolderPicker/hooks.ts rename to public/app/core/components/NestedFolderPicker/useTreeInteractions.ts diff --git a/public/app/features/browse-dashboards/api/browseDashboardsAPI.ts b/public/app/features/browse-dashboards/api/browseDashboardsAPI.ts index be06306717b..4e60a5c2a4e 100644 --- a/public/app/features/browse-dashboards/api/browseDashboardsAPI.ts +++ b/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({ + 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({ providesTags: (_result, _error, folderUID) => [{ type: 'getFolder', id: folderUID }], @@ -360,4 +372,5 @@ export const { useSaveDashboardMutation, useSaveFolderMutation, } = browseDashboardsAPI; + export { skipToken } from '@reduxjs/toolkit/query/react'; diff --git a/public/app/types/folders.ts b/public/app/types/folders.ts index 6c80fe7ca0f..83d01c5c273 100644 --- a/public/app/types/folders.ts +++ b/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;