NestedFolders: Permission for creating and editing (#67406)

* Initial permissions

* Show/hide checkboxes

* Fix kinds from main

* CreateNewButton tests

* Update DashboardsTree test

* Mock folder permissions

* Rename showCheckBoxes to canSelect

* Make column ordering look better
This commit is contained in:
Tobias Skarhed 2023-04-28 18:00:56 +02:00 committed by GitHub
parent 7338164612
commit 166641d66d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 160 additions and 43 deletions

View File

@ -9,6 +9,7 @@ import { getRouteComponentProps } from 'app/core/navigation/__mocks__/routeProps
import BrowseDashboardsPage, { Props } from './BrowseDashboardsPage'; import BrowseDashboardsPage, { Props } from './BrowseDashboardsPage';
import { wellFormedTree } from './fixtures/dashboardsTreeItem.fixture'; import { wellFormedTree } from './fixtures/dashboardsTreeItem.fixture';
import * as permissions from './permissions';
const [mockTree, { dashbdD }] = wellFormedTree(); const [mockTree, { dashbdD }] = wellFormedTree();
jest.mock('react-virtualized-auto-sizer', () => { jest.mock('react-virtualized-auto-sizer', () => {
@ -57,6 +58,14 @@ describe('browse-dashboards BrowseDashboardsPage', () => {
props = { props = {
...getRouteComponentProps(), ...getRouteComponentProps(),
}; };
jest.spyOn(permissions, 'getFolderPermissions').mockImplementation(() => {
return {
canEditInFolder: true,
canCreateDashboards: true,
canCreateFolder: true,
};
});
}); });
it('displays a search input', async () => { it('displays a search input', async () => {

View File

@ -17,6 +17,7 @@ import { BrowseFilters } from './components/BrowseFilters';
import { BrowseView } from './components/BrowseView'; import { BrowseView } from './components/BrowseView';
import { CreateNewButton } from './components/CreateNewButton'; import { CreateNewButton } from './components/CreateNewButton';
import { SearchView } from './components/SearchView'; import { SearchView } from './components/SearchView';
import { getFolderPermissions } from './permissions';
import { useHasSelection } from './state'; import { useHasSelection } from './state';
export interface BrowseDashboardsPageRouteParams { export interface BrowseDashboardsPageRouteParams {
@ -52,8 +53,22 @@ const BrowseDashboardsPage = memo(({ match }: Props) => {
const navModel = useMemo(() => (folderDTO ? buildNavModel(folderDTO) : undefined), [folderDTO]); const navModel = useMemo(() => (folderDTO ? buildNavModel(folderDTO) : undefined), [folderDTO]);
const hasSelection = useHasSelection(); const hasSelection = useHasSelection();
const { canEditInFolder, canCreateDashboards, canCreateFolder } = getFolderPermissions(folderDTO);
return ( return (
<Page navId="dashboards/browse" pageNav={navModel} actions={<CreateNewButton inFolder={folderUID} />}> <Page
navId="dashboards/browse"
pageNav={navModel}
actions={
(canCreateDashboards || canCreateFolder) && (
<CreateNewButton
inFolder={folderUID}
canCreateDashboard={canCreateDashboards}
canCreateFolder={canCreateFolder}
/>
)
}
>
<Page.Contents className={styles.pageContents}> <Page.Contents className={styles.pageContents}>
<FilterInput <FilterInput
placeholder={getSearchPlaceholder(searchState.includePanels)} placeholder={getSearchPlaceholder(searchState.includePanels)}
@ -68,9 +83,15 @@ const BrowseDashboardsPage = memo(({ match }: Props) => {
<AutoSizer> <AutoSizer>
{({ width, height }) => {({ width, height }) =>
isSearching ? ( isSearching ? (
<SearchView key={rerender} width={width} height={height} /> <SearchView key={rerender} canSelect={canEditInFolder} width={width} height={height} />
) : ( ) : (
<BrowseView key={rerender} width={width} height={height} folderUID={folderUID} /> <BrowseView
key={rerender}
canSelect={canEditInFolder}
width={width}
height={height}
folderUID={folderUID}
/>
) )
} }
</AutoSizer> </AutoSizer>

View File

@ -32,7 +32,7 @@ describe('browse-dashboards BrowseView', () => {
const HEIGHT = 600; const HEIGHT = 600;
it('expands and collapses a folder', async () => { it('expands and collapses a folder', async () => {
render(<BrowseView folderUID={undefined} width={WIDTH} height={HEIGHT} />); render(<BrowseView canSelect folderUID={undefined} width={WIDTH} height={HEIGHT} />);
await screen.findByText(folderA.item.title); await screen.findByText(folderA.item.title);
await expandFolder(folderA.item.uid); await expandFolder(folderA.item.uid);
@ -43,7 +43,7 @@ describe('browse-dashboards BrowseView', () => {
}); });
it('checks items when selected', async () => { it('checks items when selected', async () => {
render(<BrowseView folderUID={undefined} width={WIDTH} height={HEIGHT} />); render(<BrowseView canSelect folderUID={undefined} width={WIDTH} height={HEIGHT} />);
const checkbox = await screen.findByTestId(selectors.pages.BrowseDashbards.table.checkbox(dashbdD.item.uid)); const checkbox = await screen.findByTestId(selectors.pages.BrowseDashbards.table.checkbox(dashbdD.item.uid));
expect(checkbox).not.toBeChecked(); expect(checkbox).not.toBeChecked();
@ -53,7 +53,7 @@ describe('browse-dashboards BrowseView', () => {
}); });
it('checks all descendants when a folder is selected', async () => { it('checks all descendants when a folder is selected', async () => {
render(<BrowseView folderUID={undefined} width={WIDTH} height={HEIGHT} />); render(<BrowseView canSelect folderUID={undefined} width={WIDTH} height={HEIGHT} />);
await screen.findByText(folderA.item.title); await screen.findByText(folderA.item.title);
// First expand then click folderA // First expand then click folderA
@ -72,7 +72,7 @@ describe('browse-dashboards BrowseView', () => {
}); });
it('checks descendants loaded after a folder is selected', async () => { it('checks descendants loaded after a folder is selected', async () => {
render(<BrowseView folderUID={undefined} width={WIDTH} height={HEIGHT} />); render(<BrowseView canSelect folderUID={undefined} width={WIDTH} height={HEIGHT} />);
await screen.findByText(folderA.item.title); await screen.findByText(folderA.item.title);
// First expand then click folderA // First expand then click folderA
@ -94,7 +94,7 @@ describe('browse-dashboards BrowseView', () => {
}); });
it('unchecks ancestors when unselecting an item', async () => { it('unchecks ancestors when unselecting an item', async () => {
render(<BrowseView folderUID={undefined} width={WIDTH} height={HEIGHT} />); render(<BrowseView canSelect folderUID={undefined} width={WIDTH} height={HEIGHT} />);
await screen.findByText(folderA.item.title); await screen.findByText(folderA.item.title);
await expandFolder(folderA.item.uid); await expandFolder(folderA.item.uid);

View File

@ -18,9 +18,10 @@ interface BrowseViewProps {
height: number; height: number;
width: number; width: number;
folderUID: string | undefined; folderUID: string | undefined;
canSelect: boolean;
} }
export function BrowseView({ folderUID, width, height }: BrowseViewProps) { export function BrowseView({ folderUID, width, height, canSelect }: BrowseViewProps) {
const dispatch = useDispatch(); const dispatch = useDispatch();
const flatTree = useFlatTreeState(folderUID); const flatTree = useFlatTreeState(folderUID);
const selectedItems = useCheckboxSelectionState(); const selectedItems = useCheckboxSelectionState();
@ -49,6 +50,7 @@ export function BrowseView({ folderUID, width, height }: BrowseViewProps) {
return ( return (
<DashboardsTree <DashboardsTree
canSelect={canSelect}
items={flatTree} items={flatTree}
width={width} width={width}
height={height} height={height}

View File

@ -5,7 +5,7 @@ import React from 'react';
import { CreateNewButton } from './CreateNewButton'; import { CreateNewButton } from './CreateNewButton';
async function renderAndOpen(folderUID?: string) { async function renderAndOpen(folderUID?: string) {
render(<CreateNewButton inFolder={folderUID} />); render(<CreateNewButton canCreateDashboard canCreateFolder inFolder={folderUID} />);
const newButton = screen.getByText('New'); const newButton = screen.getByText('New');
await userEvent.click(newButton); await userEvent.click(newButton);
} }
@ -26,4 +26,24 @@ describe('NewActionsButton', () => {
expect(screen.getByText('New Folder')).toHaveAttribute('href', '/dashboards/folder/new'); expect(screen.getByText('New Folder')).toHaveAttribute('href', '/dashboards/folder/new');
expect(screen.getByText('Import')).toHaveAttribute('href', '/dashboard/import'); expect(screen.getByText('Import')).toHaveAttribute('href', '/dashboard/import');
}); });
it('should only render dashboard items when folder creation is disabled', async () => {
render(<CreateNewButton canCreateDashboard canCreateFolder={false} />);
const newButton = screen.getByText('New');
await userEvent.click(newButton);
expect(screen.getByText('New Dashboard')).toBeInTheDocument();
expect(screen.getByText('Import')).toBeInTheDocument();
expect(screen.queryByText('New Folder')).not.toBeInTheDocument();
});
it('should only render folder item when dashboard creation is disabled', async () => {
render(<CreateNewButton canCreateDashboard={false} canCreateFolder />);
const newButton = screen.getByText('New');
await userEvent.click(newButton);
expect(screen.queryByText('New Dashboard')).not.toBeInTheDocument();
expect(screen.queryByText('Import')).not.toBeInTheDocument();
expect(screen.getByText('New Folder')).toBeInTheDocument();
});
}); });

View File

@ -13,14 +13,22 @@ interface Props {
* Pass a folder UID in which the dashboard or folder will be created * Pass a folder UID in which the dashboard or folder will be created
*/ */
inFolder?: string; inFolder?: string;
canCreateFolder: boolean;
canCreateDashboard: boolean;
} }
export function CreateNewButton({ inFolder }: Props) { export function CreateNewButton({ inFolder, canCreateDashboard, canCreateFolder }: Props) {
const newMenu = ( const newMenu = (
<Menu> <Menu>
<MenuItem url={addFolderUidToUrl('/dashboard/new', inFolder)} label={getNewDashboardPhrase()} /> {canCreateDashboard && (
<MenuItem url={addFolderUidToUrl('/dashboards/folder/new', inFolder)} label={getNewFolderPhrase()} /> <MenuItem url={addFolderUidToUrl('/dashboard/new', inFolder)} label={getNewDashboardPhrase()} />
<MenuItem url={addFolderUidToUrl('/dashboard/import', inFolder)} label={getImportPhrase()} /> )}
{canCreateFolder && (
<MenuItem url={addFolderUidToUrl('/dashboards/folder/new', inFolder)} label={getNewFolderPhrase()} />
)}
{canCreateDashboard && (
<MenuItem url={addFolderUidToUrl('/dashboard/import', inFolder)} label={getImportPhrase()} />
)}
</Menu> </Menu>
); );

View File

@ -4,6 +4,8 @@ import React from 'react';
import { TestProvider } from 'test/helpers/TestProvider'; import { TestProvider } from 'test/helpers/TestProvider';
import { assertIsDefined } from 'test/helpers/asserts'; import { assertIsDefined } from 'test/helpers/asserts';
import { selectors } from '@grafana/e2e-selectors';
import { wellFormedDashboard, wellFormedEmptyFolder, wellFormedFolder } from '../fixtures/dashboardsTreeItem.fixture'; import { wellFormedDashboard, wellFormedEmptyFolder, wellFormedFolder } from '../fixtures/dashboardsTreeItem.fixture';
import { DashboardsTree } from './DashboardsTree'; import { DashboardsTree } from './DashboardsTree';
@ -30,6 +32,7 @@ describe('browse-dashboards DashboardsTree', () => {
it('renders a dashboard item', () => { it('renders a dashboard item', () => {
render( render(
<DashboardsTree <DashboardsTree
canSelect
items={[dashboard]} items={[dashboard]}
selectedItems={selectedItems} selectedItems={selectedItems}
width={WIDTH} width={WIDTH}
@ -42,11 +45,31 @@ describe('browse-dashboards DashboardsTree', () => {
expect(screen.queryByText(dashboard.item.title)).toBeInTheDocument(); expect(screen.queryByText(dashboard.item.title)).toBeInTheDocument();
expect(screen.queryByText('Dashboard')).toBeInTheDocument(); expect(screen.queryByText('Dashboard')).toBeInTheDocument();
expect(screen.queryByText(assertIsDefined(dashboard.item.tags)[0])).toBeInTheDocument(); expect(screen.queryByText(assertIsDefined(dashboard.item.tags)[0])).toBeInTheDocument();
expect(screen.getByTestId(selectors.pages.BrowseDashbards.table.checkbox(dashboard.item.uid))).toBeInTheDocument();
});
it('does not render checkbox when disabled', () => {
render(
<DashboardsTree
canSelect={false}
items={[dashboard]}
selectedItems={selectedItems}
width={WIDTH}
height={HEIGHT}
onFolderClick={noop}
onItemSelectionChange={noop}
onAllSelectionChange={noop}
/>
);
expect(
screen.queryByTestId(selectors.pages.BrowseDashbards.table.checkbox(dashboard.item.uid))
).not.toBeInTheDocument();
}); });
it('renders a folder item', () => { it('renders a folder item', () => {
render( render(
<DashboardsTree <DashboardsTree
canSelect
items={[folder]} items={[folder]}
selectedItems={selectedItems} selectedItems={selectedItems}
width={WIDTH} width={WIDTH}
@ -64,6 +87,7 @@ describe('browse-dashboards DashboardsTree', () => {
const handler = jest.fn(); const handler = jest.fn();
render( render(
<DashboardsTree <DashboardsTree
canSelect
items={[folder]} items={[folder]}
selectedItems={selectedItems} selectedItems={selectedItems}
width={WIDTH} width={WIDTH}
@ -82,6 +106,7 @@ describe('browse-dashboards DashboardsTree', () => {
it('renders empty folder indicators', () => { it('renders empty folder indicators', () => {
render( render(
<DashboardsTree <DashboardsTree
canSelect
items={[emptyFolderIndicator]} items={[emptyFolderIndicator]}
selectedItems={selectedItems} selectedItems={selectedItems}
width={WIDTH} width={WIDTH}

View File

@ -3,7 +3,7 @@ import React, { useMemo } from 'react';
import { CellProps, Column, HeaderProps, TableInstance, useTable } from 'react-table'; import { CellProps, Column, HeaderProps, TableInstance, useTable } from 'react-table';
import { FixedSizeList as List } from 'react-window'; import { FixedSizeList as List } from 'react-window';
import { GrafanaTheme2 } from '@grafana/data'; import { GrafanaTheme2, isTruthy } from '@grafana/data';
import { selectors } from '@grafana/e2e-selectors'; import { selectors } from '@grafana/e2e-selectors';
import { Checkbox, useStyles2 } from '@grafana/ui'; import { Checkbox, useStyles2 } from '@grafana/ui';
import { DashboardViewItem, DashboardViewItemKind } from 'app/features/search/types'; import { DashboardViewItem, DashboardViewItemKind } from 'app/features/search/types';
@ -23,6 +23,7 @@ interface DashboardsTreeProps {
onFolderClick: (uid: string, newOpenState: boolean) => void; onFolderClick: (uid: string, newOpenState: boolean) => void;
onAllSelectionChange: (newState: boolean) => void; onAllSelectionChange: (newState: boolean) => void;
onItemSelectionChange: (item: DashboardViewItem, newState: boolean) => void; onItemSelectionChange: (item: DashboardViewItem, newState: boolean) => void;
canSelect: boolean;
} }
type DashboardsTreeColumn = Column<DashboardsTreeItem>; type DashboardsTreeColumn = Column<DashboardsTreeItem>;
@ -46,34 +47,37 @@ export function DashboardsTree({
onFolderClick, onFolderClick,
onAllSelectionChange, onAllSelectionChange,
onItemSelectionChange, onItemSelectionChange,
canSelect = false,
}: DashboardsTreeProps) { }: DashboardsTreeProps) {
const styles = useStyles2(getStyles); const styles = useStyles2(getStyles);
const tableColumns = useMemo(() => { const tableColumns = useMemo(() => {
const checkboxColumn: DashboardsTreeColumn = { const checkboxColumn: DashboardsTreeColumn | null = canSelect
id: 'checkbox', ? {
width: 0, id: 'checkbox',
Header: ({ selectedItems }: DashboardTreeHeaderProps) => { width: 0,
const isAllSelected = selectedItems?.$all ?? false; Header: ({ selectedItems }: DashboardTreeHeaderProps) => {
return <Checkbox value={isAllSelected} onChange={(ev) => onAllSelectionChange(ev.currentTarget.checked)} />; const isAllSelected = selectedItems?.$all ?? false;
}, return <Checkbox value={isAllSelected} onChange={(ev) => onAllSelectionChange(ev.currentTarget.checked)} />;
Cell: ({ row: { original: row }, selectedItems }: DashboardsTreeCellProps) => { },
const item = row.item; Cell: ({ row: { original: row }, selectedItems }: DashboardsTreeCellProps) => {
if (item.kind === 'ui-empty-folder' || !selectedItems) { const item = row.item;
return <></>; if (item.kind === 'ui-empty-folder' || !selectedItems) {
return <></>;
}
const isSelected = selectedItems?.[item.kind][item.uid] ?? false;
return (
<Checkbox
data-testid={selectors.pages.BrowseDashbards.table.checkbox(item.uid)}
value={isSelected}
onChange={(ev) => onItemSelectionChange(item, ev.currentTarget.checked)}
/>
);
},
} }
: null;
const isSelected = selectedItems?.[item.kind][item.uid] ?? false;
return (
<Checkbox
data-testid={selectors.pages.BrowseDashbards.table.checkbox(item.uid)}
value={isSelected}
onChange={(ev) => onItemSelectionChange(item, ev.currentTarget.checked)}
/>
);
},
};
const nameColumn: DashboardsTreeColumn = { const nameColumn: DashboardsTreeColumn = {
id: 'name', id: 'name',
@ -95,9 +99,10 @@ export function DashboardsTree({
Header: 'Tags', Header: 'Tags',
Cell: TagsCell, Cell: TagsCell,
}; };
const columns = [canSelect && checkboxColumn, nameColumn, typeColumn, tagsColumns].filter(isTruthy);
return [checkboxColumn, nameColumn, typeColumn, tagsColumns]; return columns;
}, [onItemSelectionChange, onAllSelectionChange, onFolderClick]); }, [onItemSelectionChange, onAllSelectionChange, onFolderClick, canSelect]);
const table = useTable({ columns: tableColumns, data: items }, useCustomFlexLayout); const table = useTable({ columns: tableColumns, data: items }, useCustomFlexLayout);
const { getTableProps, getTableBodyProps, headerGroups } = table; const { getTableProps, getTableBodyProps, headerGroups } = table;

View File

@ -12,9 +12,10 @@ import { setAllSelection, setItemSelectionState, useHasSelection } from '../stat
interface SearchViewProps { interface SearchViewProps {
height: number; height: number;
width: number; width: number;
canSelect: boolean;
} }
export function SearchView({ width, height }: SearchViewProps) { export function SearchView({ width, height, canSelect }: SearchViewProps) {
const dispatch = useDispatch(); const dispatch = useDispatch();
const selectedItems = useSelector((wholeState) => wholeState.browseDashboards.selectedItems); const selectedItems = useSelector((wholeState) => wholeState.browseDashboards.selectedItems);
const hasSelection = useHasSelection(); const hasSelection = useHasSelection();
@ -73,8 +74,8 @@ export function SearchView({ width, height }: SearchViewProps) {
const props: SearchResultsProps = { const props: SearchResultsProps = {
response: value, response: value,
selection: selectionChecker, selection: canSelect ? selectionChecker : undefined,
selectionToggle: handleItemSelectionChange, selectionToggle: canSelect ? handleItemSelectionChange : undefined,
clearSelection, clearSelection,
width: width, width: width,
height: height, height: height,

View File

@ -0,0 +1,26 @@
import { contextSrv } from 'app/core/core';
import { AccessControlAction, FolderDTO } from 'app/types';
function checkFolderPermission(action: AccessControlAction, fallback: boolean, folderDTO?: FolderDTO) {
return folderDTO
? contextSrv.hasAccessInMetadata(action, folderDTO, fallback)
: contextSrv.hasAccess(action, fallback);
}
export function getFolderPermissions(folderDTO?: FolderDTO) {
// It is possible to have edit permissions for folders and dashboards, without being able to save, hence 'canSave'
const canEditInFolderFallback = folderDTO ? folderDTO.canSave : contextSrv.hasEditPermissionInFolders;
const canEditInFolder = checkFolderPermission(AccessControlAction.FoldersWrite, canEditInFolderFallback, folderDTO);
const canCreateFolder = checkFolderPermission(AccessControlAction.FoldersCreate, contextSrv.isEditor);
const canCreateDashboards = checkFolderPermission(
AccessControlAction.DashboardsCreate,
canEditInFolderFallback || !!folderDTO?.canSave
);
return {
canEditInFolder,
canCreateDashboards,
canCreateFolder,
};
}