Dashboard: Prevents folder change when navigating to general settings (#38103)

* Dashboard: Prevents folder change when navigating to general settings

* Tests: fixes broken tests

* Chore: changes from PR feedback
This commit is contained in:
Hugo Häggmark 2021-08-20 11:53:48 +02:00 committed by GitHub
parent 5ff3b7bf3f
commit a0773b290b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 122 additions and 79 deletions

View File

@ -4,7 +4,7 @@ import debounce from 'debounce-promise';
import { AsyncMultiSelect, Icon, resetSelectStyles, useStyles2 } from '@grafana/ui';
import { GrafanaTheme2, SelectableValue } from '@grafana/data';
import { FolderInfo } from 'app/types';
import { FolderInfo, PermissionLevelString } from 'app/types';
import { getBackendSrv } from 'app/core/services/backend_srv';
export interface FolderFilterProps {
@ -67,7 +67,7 @@ async function getFoldersAsOptions(searchString: string, setLoading: (loading: b
const params = {
query: searchString,
type: 'dash-folder',
permission: 'View',
permission: PermissionLevelString.View,
};
const searchHits = await getBackendSrv().search(params);

View File

@ -1,30 +1,49 @@
import React from 'react';
import { shallow } from 'enzyme';
import { FolderPicker } from './FolderPicker';
jest.mock('@grafana/runtime', () => ({
...((jest.requireActual('@grafana/runtime') as unknown) as object),
getBackendSrv: () => ({
search: jest.fn(() => [
{ title: 'Dash 1', id: 'A' },
{ title: 'Dash 2', id: 'B' },
]),
}),
}));
jest.mock('../../config', () => ({
getConfig: () => ({}),
}));
jest.mock('app/core/services/context_srv', () => ({
contextSrv: {
user: { orgId: 1 },
},
}));
import { FolderPicker, getInitialValues } from './FolderPicker';
import * as api from 'app/features/manage-dashboards/state/actions';
import { DashboardSearchHit } from '../../../features/search/types';
describe('FolderPicker', () => {
it('should render', () => {
jest
.spyOn(api, 'searchFolders')
.mockResolvedValue([
{ title: 'Dash 1', id: 1 } as DashboardSearchHit,
{ title: 'Dash 2', id: 2 } as DashboardSearchHit,
]);
const wrapper = shallow(<FolderPicker onChange={jest.fn()} />);
expect(wrapper).toMatchSnapshot();
});
});
describe('getInitialValues', () => {
describe('when called with folderId and title', () => {
it('then it should return folderId and title', async () => {
const getFolder = jest.fn().mockResolvedValue({});
const folder = await getInitialValues({ folderId: 0, folderName: 'Some title', getFolder });
expect(folder).toEqual({ label: 'Some title', value: 0 });
expect(getFolder).not.toHaveBeenCalled();
});
});
describe('when called with just a folderId', () => {
it('then it should call api to retrieve title', async () => {
const getFolder = jest.fn().mockResolvedValue({ id: 0, title: 'Title from api' });
const folder = await getInitialValues({ folderId: 0, getFolder });
expect(folder).toEqual({ label: 'Title from api', value: 0 });
expect(getFolder).toHaveBeenCalledTimes(1);
expect(getFolder).toHaveBeenCalledWith(0);
});
});
describe('when called without folderId', () => {
it('then it should throw an error', async () => {
const getFolder = jest.fn().mockResolvedValue({});
await expect(getInitialValues({ getFolder })).rejects.toThrow();
});
});
});

View File

@ -2,13 +2,12 @@ import React, { PureComponent } from 'react';
import { debounce } from 'lodash';
import { AsyncSelect } from '@grafana/ui';
import { AppEvents, SelectableValue } from '@grafana/data';
import { getBackendSrv } from '@grafana/runtime';
import { selectors } from '@grafana/e2e-selectors';
import appEvents from '../../app_events';
import { contextSrv } from 'app/core/services/context_srv';
import { DashboardSearchHit } from 'app/features/search/types';
import { createFolder } from 'app/features/manage-dashboards/state/actions';
import { createFolder, getFolderById, searchFolders } from 'app/features/manage-dashboards/state/actions';
import { PermissionLevelString } from '../../../types';
export interface Props {
onChange: ($folder: { title: string; id: number }) => void;
@ -18,9 +17,16 @@ export interface Props {
dashboardId?: any;
initialTitle?: string;
initialFolderId?: number;
permissionLevel?: 'View' | 'Edit';
permissionLevel?: Exclude<PermissionLevelString, PermissionLevelString.Admin>;
allowEmpty?: boolean;
showRoot?: boolean;
/**
* Skips loading all folders in order to find the folder matching
* the folder where the dashboard is stored.
* Instead initialFolderId and initialTitle will be used to display the correct folder.
* initialFolderId needs to have an value > -1 or an error will be thrown.
*/
skipInitialLoad?: boolean;
}
interface State {
@ -48,26 +54,29 @@ export class FolderPicker extends PureComponent<Props, State> {
enableReset: false,
initialTitle: '',
enableCreateNew: false,
permissionLevel: 'Edit',
permissionLevel: PermissionLevelString.Edit,
allowEmpty: false,
showRoot: true,
};
componentDidMount = async () => {
if (this.props.skipInitialLoad) {
const folder = await getInitialValues({
getFolder: getFolderById,
folderId: this.props.initialFolderId,
folderName: this.props.initialTitle,
});
this.setState({ folder });
return;
}
await this.loadInitialValue();
};
getOptions = async (query: string) => {
const { rootName, enableReset, initialTitle, permissionLevel, initialFolderId, showRoot } = this.props;
const params = {
query,
type: 'dash-folder',
permission: permissionLevel,
};
// TODO: move search to BackendSrv interface
// @ts-ignore
const searchHits = (await getBackendSrv().search(params)) as DashboardSearchHit[];
const searchHits = await searchFolders(query, permissionLevel);
const options: Array<SelectableValue<number>> = searchHits.map((hit) => ({ label: hit.title, value: hit.id }));
if (contextSrv.isEditor && rootName?.toLowerCase().startsWith(query.toLowerCase()) && showRoot) {
@ -179,3 +188,22 @@ export class FolderPicker extends PureComponent<Props, State> {
);
}
}
interface Args {
getFolder: typeof getFolderById;
folderId?: number;
folderName?: string;
}
export async function getInitialValues({ folderName, folderId, getFolder }: Args): Promise<SelectableValue<number>> {
if (folderId === null || folderId === undefined || folderId < 0) {
throw new Error('folderId should to be greater or equal to zero.');
}
if (folderName) {
return { label: folderName, value: folderId };
}
const folderDto = await getFolder(folderId);
return { label: folderDto.title, value: folderId };
}

View File

@ -1,9 +1,9 @@
import { Matcher, render, waitFor } from '@testing-library/react';
import { Provider } from 'react-redux';
import { locationService, setDataSourceSrv, setBackendSrv, BackendSrv } from '@grafana/runtime';
import { locationService, setDataSourceSrv } from '@grafana/runtime';
import { configureStore } from 'app/store/configureStore';
import RuleEditor from './RuleEditor';
import { Router, Route } from 'react-router-dom';
import { Route, Router } from 'react-router-dom';
import React from 'react';
import { byLabelText, byRole, byTestId, byText } from 'testing-library-selector';
import { selectOptionInTest } from '@grafana/ui';
@ -18,6 +18,7 @@ import { DataSourceType, GRAFANA_RULES_SOURCE_NAME } from './utils/datasource';
import { DashboardSearchHit } from 'app/features/search/types';
import { getDefaultQueries } from './utils/rule-form';
import { ExpressionEditorProps } from './components/rule-editor/ExpressionEditor';
import * as api from 'app/features/manage-dashboards/state/actions';
jest.mock('./components/rule-editor/ExpressionEditor', () => ({
// eslint-disable-next-line react/display-name
@ -163,7 +164,7 @@ describe('RuleEditor', () => {
});
it('can create new grafana managed alert', async () => {
const searchFolderMock = jest.fn().mockResolvedValue([
const searchFolderMock = jest.spyOn(api, 'searchFolders').mockResolvedValue([
{
title: 'Folder A',
id: 1,
@ -182,10 +183,6 @@ describe('RuleEditor', () => {
}),
};
const backendSrv = ({
search: searchFolderMock,
} as any) as BackendSrv;
setBackendSrv(backendSrv);
setDataSourceSrv(new MockDataSourceSrv(dataSources));
mocks.api.setRulerRuleGroup.mockResolvedValue();
mocks.api.fetchRulerRulesNamespace.mockResolvedValue([]);

View File

@ -4,25 +4,9 @@ import userEvent from '@testing-library/user-event';
import { selectOptionInTest } from '@grafana/ui';
import { byRole } from 'testing-library-selector';
import { Props, GeneralSettingsUnconnected as GeneralSettings } from './GeneralSettings';
import { GeneralSettingsUnconnected as GeneralSettings, Props } from './GeneralSettings';
import { DashboardModel } from '../../state';
jest.mock('@grafana/runtime', () => ({
...((jest.requireActual('@grafana/runtime') as unknown) as object),
getBackendSrv: () => ({
search: jest.fn(() => [
{ title: 'A', id: 'A' },
{ title: 'B', id: 'B' },
]),
}),
}));
jest.mock('app/core/services/context_srv', () => ({
contextSrv: {
user: { orgId: 1 },
},
}));
const setupTestContext = (options: Partial<Props>) => {
const defaults: Props = {
dashboard: ({
@ -33,6 +17,7 @@ const setupTestContext = (options: Partial<Props>) => {
time_options: ['5m', '15m', '1h', '6h', '12h', '24h', '2d', '7d', '30d'],
},
meta: {
folderId: 1,
folderTitle: 'test',
},
timezone: 'utc',

View File

@ -1,7 +1,7 @@
import React, { useState } from 'react';
import { connect, ConnectedProps } from 'react-redux';
import { TimeZone } from '@grafana/data';
import { TagsInput, Input, Field, CollapsableSection, RadioButtonGroup } from '@grafana/ui';
import { CollapsableSection, Field, Input, RadioButtonGroup, TagsInput } from '@grafana/ui';
import { selectors } from '@grafana/e2e-selectors';
import { FolderPicker } from 'app/core/components/Select/FolderPicker';
import { DashboardModel } from '../../state/DashboardModel';
@ -96,6 +96,7 @@ export function GeneralSettingsUnconnected({ dashboard, updateTimeZone }: Props)
onChange={onFolderChange}
enableCreateNew={true}
dashboardId={dashboard.id}
skipInitialLoad={true}
/>
</Field>

View File

@ -3,17 +3,7 @@ import { mount } from 'enzyme';
import { SaveDashboardAsForm } from './SaveDashboardAsForm';
import { DashboardModel } from 'app/features/dashboard/state';
import { act } from 'react-dom/test-utils';
jest.mock('@grafana/runtime', () => ({
...((jest.requireActual('@grafana/runtime') as unknown) as object),
getBackendSrv: () => ({ get: jest.fn().mockResolvedValue([]), search: jest.fn().mockResolvedValue([]) }),
}));
jest.mock('app/core/services/context_srv', () => ({
contextSrv: {
user: { orgId: 1 },
},
}));
import * as api from 'app/features/manage-dashboards/state/actions';
jest.mock('app/features/plugins/datasource_srv', () => ({}));
jest.mock('app/features/expressions/ExpressionDatasource', () => ({}));
@ -21,6 +11,8 @@ jest.mock('app/features/manage-dashboards/services/ValidationSrv', () => ({
validateNewDashboardName: () => true,
}));
jest.spyOn(api, 'searchFolders').mockResolvedValue([]);
const prepareDashboardMock = (panel: any) => {
const json = {
title: 'name',
@ -56,6 +48,7 @@ const renderAndSubmitForm = async (dashboard: any, submitSpy: any) => {
describe('SaveDashboardAsForm', () => {
describe('default values', () => {
it('applies default dashboard properties', async () => {
jest.spyOn(api, 'searchFolders').mockResolvedValue([]);
const spy = jest.fn();
await renderAndSubmitForm(prepareDashboardMock({}), spy);

View File

@ -2,16 +2,17 @@ import { AppEvents, DataSourceInstanceSettings, locationUtil } from '@grafana/da
import { getBackendSrv } from 'app/core/services/backend_srv';
import {
clearDashboard,
setInputs,
setGcomDashboard,
setJsonDashboard,
InputType,
ImportDashboardDTO,
InputType,
setGcomDashboard,
setInputs,
setJsonDashboard,
} from './reducers';
import { ThunkResult, FolderInfo, DashboardDTO, DashboardDataDTO } from 'app/types';
import { DashboardDataDTO, DashboardDTO, FolderInfo, PermissionLevelString, ThunkResult } from 'app/types';
import { appEvents } from '../../../core/core';
import { dashboardWatcher } from 'app/features/live/dashboard/dashboardWatcher';
import { getDataSourceSrv, locationService } from '@grafana/runtime';
import { DashboardSearchHit } from '../../search/types';
export function fetchGcomDashboard(id: string): ThunkResult<void> {
return async (dispatch) => {
@ -222,6 +223,14 @@ export function createFolder(payload: any) {
return getBackendSrv().post('/api/folders', payload);
}
export function searchFolders(query: any, permission?: PermissionLevelString): Promise<DashboardSearchHit[]> {
return getBackendSrv().search({ query, type: 'dash-folder', permission });
}
export function getFolderById(id: number): Promise<{ id: number; title: string }> {
return getBackendSrv().get(`/api/folders/id/${id}`);
}
export function deleteDashboard(uid: string, showSuccessAlert: boolean) {
return getBackendSrv().request({
method: 'DELETE',

View File

@ -4,6 +4,7 @@ import { DashListOptions } from './types';
import { FolderPicker } from 'app/core/components/Select/FolderPicker';
import React from 'react';
import { TagsInput } from '@grafana/ui';
import { PermissionLevelString } from '../../../types';
export const plugin = new PanelPlugin<DashListOptions>(DashList)
.setPanelOptions((builder) => {
@ -49,7 +50,7 @@ export const plugin = new PanelPlugin<DashListOptions>(DashList)
initialFolderId={props.value}
initialTitle="All"
enableReset={true}
permissionLevel="View"
permissionLevel={PermissionLevelString.View}
onChange={({ id }) => props.onChange(id)}
/>
);

View File

@ -63,6 +63,12 @@ export enum PermissionLevel {
Admin = 4,
}
export enum PermissionLevelString {
View = 'View',
Edit = 'Edit',
Admin = 'Admin',
}
export enum DataSourcePermissionLevel {
Query = 1,
Admin = 2,
@ -92,8 +98,12 @@ export const dashboardAclTargets: AclTargetInfo[] = [
];
export const dashboardPermissionLevels: DashboardPermissionInfo[] = [
{ value: PermissionLevel.View, label: 'View', description: 'Can view dashboards.' },
{ value: PermissionLevel.Edit, label: 'Edit', description: 'Can add, edit and delete dashboards.' },
{ value: PermissionLevel.View, label: PermissionLevelString.View, description: 'Can view dashboards.' },
{
value: PermissionLevel.Edit,
label: PermissionLevelString.Edit,
description: 'Can add, edit and delete dashboards.',
},
{
value: PermissionLevel.Admin,
label: 'Admin',