From 365fd2cb721d326f011e57ecaf9dba57cd9a16dc Mon Sep 17 00:00:00 2001 From: Josh Hunt Date: Tue, 19 Mar 2024 11:44:52 +0000 Subject: [PATCH] FolderPicker: Add permission filter to nested folder picker (#84644) * FolderPicker: Add permission filter to nested folder picker * Add permission filtering to Searcher * make default edit only * dashlist view folders * update tests --- .../NestedFolderPicker.test.tsx | 61 ++++++++++++------- .../NestedFolderPicker/NestedFolderPicker.tsx | 14 +++-- .../NestedFolderPicker/useFoldersQuery.ts | 19 +++--- .../api/browseDashboardsAPI.ts | 7 ++- .../fixtures/dashboardsTreeItem.fixture.ts | 12 ++++ public/app/features/search/service/sql.ts | 3 + public/app/features/search/service/types.ts | 2 + public/app/plugins/panel/dashlist/module.tsx | 10 ++- 8 files changed, 92 insertions(+), 36 deletions(-) diff --git a/public/app/core/components/NestedFolderPicker/NestedFolderPicker.test.tsx b/public/app/core/components/NestedFolderPicker/NestedFolderPicker.test.tsx index a6cb752fb52..c19e7221517 100644 --- a/public/app/core/components/NestedFolderPicker/NestedFolderPicker.test.tsx +++ b/public/app/core/components/NestedFolderPicker/NestedFolderPicker.test.tsx @@ -8,33 +8,23 @@ import { TestProvider } from 'test/helpers/TestProvider'; import { config } from '@grafana/runtime'; import { backendSrv } from 'app/core/services/backend_srv'; +import { PermissionLevelString } from 'app/types'; -import { wellFormedTree } from '../../../features/browse-dashboards/fixtures/dashboardsTreeItem.fixture'; +import { + treeViewersCanEdit, + wellFormedTree, +} from '../../../features/browse-dashboards/fixtures/dashboardsTreeItem.fixture'; import { NestedFolderPicker } from './NestedFolderPicker'; const [mockTree, { folderA, folderB, folderC, folderA_folderA, folderA_folderB }] = wellFormedTree(); +const [mockTreeThatViewersCanEdit /* shares folders with wellFormedTree */] = treeViewersCanEdit(); jest.mock('@grafana/runtime', () => ({ ...jest.requireActual('@grafana/runtime'), getBackendSrv: () => backendSrv, })); -jest.mock('app/features/browse-dashboards/api/services', () => { - const orig = jest.requireActual('app/features/browse-dashboards/api/services'); - - return { - ...orig, - listFolders(parentUID?: string) { - const childrenForUID = mockTree - .filter((v) => v.item.kind === 'folder' && v.item.parentUID === parentUID) - .map((v) => v.item); - - return Promise.resolve(childrenForUID); - }, - }; -}); - function render(...[ui, options]: Parameters) { rtlRender({ui}, options); } @@ -58,12 +48,15 @@ describe('NestedFolderPicker', () => { http.get('/api/folders', ({ request }) => { const url = new URL(request.url); const parentUid = url.searchParams.get('parentUid') ?? undefined; + const permission = url.searchParams.get('permission'); const limit = parseInt(url.searchParams.get('limit') ?? '1000', 10); const page = parseInt(url.searchParams.get('page') ?? '1', 10); + const tree = permission === 'Edit' ? mockTreeThatViewersCanEdit : mockTree; + // reconstruct a folder API response from the flat tree fixture - const folders = mockTree + const folders = tree .filter((v) => v.item.kind === 'folder' && v.item.parentUID === parentUid) .map((folder) => { return { @@ -117,7 +110,7 @@ describe('NestedFolderPicker', () => { expect(screen.getByPlaceholderText('Search folders')).toBeInTheDocument(); expect(screen.getByLabelText('Dashboards')).toBeInTheDocument(); expect(screen.getByLabelText(folderA.item.title)).toBeInTheDocument(); - expect(screen.getByLabelText(folderB.item.title)).toBeInTheDocument(); + // expect(screen.getByLabelText(folderB.item.title)).toBeInTheDocument(); expect(screen.getByLabelText(folderC.item.title)).toBeInTheDocument(); }); @@ -174,14 +167,36 @@ describe('NestedFolderPicker', () => { }); it('hides folders specififed by UID', async () => { - render(); + 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); + await screen.findByLabelText(folderA.item.title); - expect(screen.queryByLabelText(folderA.item.title)).not.toBeInTheDocument(); + expect(screen.queryByLabelText(folderC.item.title)).not.toBeInTheDocument(); + }); + + it('by default only shows items the user can edit', async () => { + render(); + + const button = await screen.findByRole('button', { name: 'Select folder' }); + await userEvent.click(button); + await screen.findByLabelText(folderA.item.title); + + expect(screen.queryByLabelText(folderB.item.title)).not.toBeInTheDocument(); // folderB is not editable + expect(screen.getByLabelText(folderC.item.title)).toBeInTheDocument(); // but folderC is + }); + + it('shows items the user can view, with the prop', async () => { + render(); + + const button = await screen.findByRole('button', { name: 'Select folder' }); + await userEvent.click(button); + await screen.findByLabelText(folderA.item.title); + + expect(screen.getByLabelText(folderB.item.title)).toBeInTheDocument(); + expect(screen.getByLabelText(folderC.item.title)).toBeInTheDocument(); }); describe('when nestedFolders is enabled', () => { @@ -196,7 +211,7 @@ describe('NestedFolderPicker', () => { }); it('can expand and collapse a folder to show its children', async () => { - render(); + render(); // Open the picker and wait for children to load const button = await screen.findByRole('button', { name: 'Select folder' }); @@ -227,7 +242,7 @@ describe('NestedFolderPicker', () => { }); it('can expand and collapse a folder to show its children with the keyboard', async () => { - render(); + render(); const button = await screen.findByRole('button', { name: 'Select folder' }); await userEvent.click(button); diff --git a/public/app/core/components/NestedFolderPicker/NestedFolderPicker.tsx b/public/app/core/components/NestedFolderPicker/NestedFolderPicker.tsx index 1f2ab2739d8..e9ee8eb9574 100644 --- a/public/app/core/components/NestedFolderPicker/NestedFolderPicker.tsx +++ b/public/app/core/components/NestedFolderPicker/NestedFolderPicker.tsx @@ -12,6 +12,7 @@ import { DashboardViewItemWithUIItems, DashboardsTreeItem } from 'app/features/b import { QueryResponse, getGrafanaSearcher } from 'app/features/search/service'; import { queryResultToViewItem } from 'app/features/search/service/utils'; import { DashboardViewItem } from 'app/features/search/types'; +import { PermissionLevelString } from 'app/types'; import { getDOMId, NestedFolderList } from './NestedFolderList'; import Trigger from './Trigger'; @@ -31,6 +32,9 @@ export interface NestedFolderPickerProps { /* Folder UIDs to exclude from the picker, to prevent invalid operations */ excludeUIDs?: string[]; + /* Show folders matching this permission, mainly used to also show folders user can view. Defaults to showing only folders user has Edit */ + permission?: PermissionLevelString.View | PermissionLevelString.Edit; + /* Callback for when the user selects a folder */ onChange?: (folderUID: string | undefined, folderName: string | undefined) => void; @@ -40,11 +44,12 @@ export interface NestedFolderPickerProps { const debouncedSearch = debounce(getSearchResults, 300); -async function getSearchResults(searchQuery: string) { +async function getSearchResults(searchQuery: string, permission?: PermissionLevelString) { const queryResponse = await getGrafanaSearcher().search({ query: searchQuery, kind: ['folder'], limit: 100, + permission: permission, }); const items = queryResponse.view.map((v) => queryResultToViewItem(v, queryResponse.view)); @@ -57,6 +62,7 @@ export function NestedFolderPicker({ showRootFolder = true, clearable = false, excludeUIDs, + permission = PermissionLevelString.Edit, onChange, }: NestedFolderPickerProps) { const styles = useStyles2(getStyles); @@ -79,7 +85,7 @@ export function NestedFolderPicker({ items: browseFlatTree, isLoading: isBrowseLoading, requestNextPage: fetchFolderPage, - } = useFoldersQuery(isBrowsing, foldersOpenState); + } = useFoldersQuery(isBrowsing, foldersOpenState, permission); useEffect(() => { if (!search) { @@ -90,7 +96,7 @@ export function NestedFolderPicker({ const timestamp = Date.now(); setIsFetchingSearchResults(true); - debouncedSearch(search).then((queryResponse) => { + debouncedSearch(search, permission).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. // We don't need to worry about clearing the isFetching state either - if there's a later @@ -102,7 +108,7 @@ export function NestedFolderPicker({ lastSearchTimestamp.current = timestamp; } }); - }, [search]); + }, [search, permission]); // the order of middleware is important! const middleware = [ diff --git a/public/app/core/components/NestedFolderPicker/useFoldersQuery.ts b/public/app/core/components/NestedFolderPicker/useFoldersQuery.ts index 528ca80ab1c..6a89b5e1be9 100644 --- a/public/app/core/components/NestedFolderPicker/useFoldersQuery.ts +++ b/public/app/core/components/NestedFolderPicker/useFoldersQuery.ts @@ -9,7 +9,7 @@ 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 { FolderListItemDTO, PermissionLevelString } from 'app/types'; import { useDispatch, useSelector } from 'app/types/store'; type ListFoldersQuery = ReturnType>; @@ -29,8 +29,9 @@ const listFoldersSelector = createSelector( state: RootState, parentUid: ListFolderQueryArgs['parentUid'], page: ListFolderQueryArgs['page'], - limit: ListFolderQueryArgs['limit'] - ) => browseDashboardsAPI.endpoints.listFolders.select({ parentUid, page, limit }), + limit: ListFolderQueryArgs['limit'], + permission: ListFolderQueryArgs['permission'] + ) => browseDashboardsAPI.endpoints.listFolders.select({ parentUid, page, limit, permission }), (state, selectFolderList) => selectFolderList(state) ); @@ -48,7 +49,7 @@ const listAllFoldersSelector = createSelector( continue; } - const page = listFoldersSelector(state, req.arg.parentUid, req.arg.page, req.arg.limit); + const page = listFoldersSelector(state, req.arg.parentUid, req.arg.page, req.arg.limit, req.arg.permission); if (page.status === 'pending') { isLoading = true; } @@ -91,7 +92,11 @@ function getPagesLoadStatus(pages: ListFoldersQuery[]): [boolean, number | undef /** * Returns a loaded folder hierarchy as a flat list and a function to load more pages. */ -export function useFoldersQuery(isBrowsing: boolean, openFolders: Record) { +export function useFoldersQuery( + isBrowsing: boolean, + openFolders: Record, + permission?: PermissionLevelString +) { const dispatch = useDispatch(); // Keep a list of all requests so we can @@ -113,13 +118,13 @@ export function useFoldersQuery(isBrowsing: boolean, openFolders: Record ({ listFolders: builder.query({ providesTags: (result) => result?.map((folder) => ({ type: 'getFolder', id: folder.uid })) ?? [], - query: ({ page, parentUid, limit }) => ({ url: '/folders', params: { page, parentUid, limit } }), + query: ({ parentUid, limit, page, permission }) => ({ + url: '/folders', + params: { parentUid, limit, page, permission }, + }), }), // get folder info (e.g. title, parents) but *not* children diff --git a/public/app/features/browse-dashboards/fixtures/dashboardsTreeItem.fixture.ts b/public/app/features/browse-dashboards/fixtures/dashboardsTreeItem.fixture.ts index 9de6e59c06a..4ac6515c726 100644 --- a/public/app/features/browse-dashboards/fixtures/dashboardsTreeItem.fixture.ts +++ b/public/app/features/browse-dashboards/fixtures/dashboardsTreeItem.fixture.ts @@ -73,6 +73,18 @@ export function sharedWithMeFolder(seed = 1): DashboardsTreeItem(DashList) id: 'folderUID', defaultValue: undefined, editor: function RenderFolderPicker({ value, onChange }) { - return onChange(folderUID)} />; + return ( + onChange(folderUID)} + /> + ); }, }) .addCustomEditor({