Annotations: Migrate dashboardId to dashboardUID (#52588)

* Annotations: replaced dashboardId with dashboardUID when fetching annotations

* Annotations: Replaced dashboardID with dashboardUID when adding and updating annotations

* Annotations: Used dashboardUID for PanelChrome related methods

* Annotations: Used dashboarduid in annolistpanel

* Annotations: Used dashboardUID in graph plugin

* used boolean in canEditDashboard
This commit is contained in:
Leo 2022-08-01 09:21:41 +02:00 committed by GitHub
parent 046a2602ff
commit 1fc9f6f1c6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 210 additions and 79 deletions

View File

@ -33,6 +33,7 @@ export interface AnnotationEvent {
id?: string;
annotation?: any;
dashboardId?: number;
dashboardUID?: string;
panelId?: number;
userId?: number;
login?: string;

View File

@ -32,8 +32,8 @@ export interface PanelContext {
onToggleSeriesVisibility?: (label: string, mode: SeriesVisibilityChangeMode) => void;
canAddAnnotations?: () => boolean;
canEditAnnotations?: (dashboardId: number) => boolean;
canDeleteAnnotations?: (dashboardId: number) => boolean;
canEditAnnotations?: (dashboardUID?: string) => boolean;
canDeleteAnnotations?: (dashboardUID?: string) => boolean;
onAnnotationCreate?: (annotation: AnnotationEventUIModel) => void;
onAnnotationUpdate?: (annotation: AnnotationEventUIModel) => void;
onAnnotationDelete?: (id: string) => void;

View File

@ -125,6 +125,7 @@ const alertEventAndAnnotationFields: AnnotationFieldInfo[] = [
{ key: 'panelId' },
{ key: 'alertId' },
{ key: 'dashboardId' },
{ key: 'dashboardUID' },
];
export function getAnnotationsFromData(

View File

@ -47,6 +47,9 @@ function setupTestContext(options: Partial<Props>) {
panelInitialized: jest.fn(),
getTimezone: () => 'browser',
events: new EventBusSrv(),
canAddAnnotations: jest.fn(),
canEditAnnotations: jest.fn(),
canDeleteAnnotations: jest.fn(),
meta: {
isPublic: false,
},

View File

@ -29,7 +29,6 @@ import { applyPanelTimeOverrides } from 'app/features/dashboard/utils/panel';
import { changeSeriesColorConfigFactory } from 'app/plugins/panel/timeseries/overrides/colorSeriesConfigFactory';
import { RenderEvent } from 'app/types/events';
import { contextSrv } from '../../../core/services/context_srv';
import { isSoloRoute } from '../../../routes/utils';
import { deleteAnnotation, saveAnnotation, updateAnnotation } from '../../annotations/api';
import { getDashboardQueryRunner } from '../../query/state/DashboardQueryRunner/DashboardQueryRunner';
@ -89,53 +88,16 @@ export class PanelChrome extends PureComponent<Props, State> {
onAnnotationCreate: this.onAnnotationCreate,
onAnnotationUpdate: this.onAnnotationUpdate,
onAnnotationDelete: this.onAnnotationDelete,
canAddAnnotations: this.canAddAnnotation,
onInstanceStateChange: this.onInstanceStateChange,
onToggleLegendSort: this.onToggleLegendSort,
canEditAnnotations: this.canEditAnnotation,
canDeleteAnnotations: this.canDeleteAnnotation,
canAddAnnotations: props.dashboard.canAddAnnotations.bind(props.dashboard),
canEditAnnotations: props.dashboard.canEditAnnotations.bind(props.dashboard),
canDeleteAnnotations: props.dashboard.canDeleteAnnotations.bind(props.dashboard),
},
data: this.getInitialPanelDataState(),
};
}
canEditDashboard = () => Boolean(this.props.dashboard.meta.canEdit || this.props.dashboard.meta.canMakeEditable);
canAddAnnotation = () => {
let canAdd = true;
if (contextSrv.accessControlEnabled()) {
canAdd = !!this.props.dashboard.meta.annotationsPermissions?.dashboard.canAdd;
}
return canAdd && this.canEditDashboard();
};
canEditAnnotation = (dashboardId: number) => {
let canEdit = true;
if (contextSrv.accessControlEnabled()) {
if (dashboardId !== 0) {
canEdit = !!this.props.dashboard.meta.annotationsPermissions?.dashboard.canEdit;
} else {
canEdit = !!this.props.dashboard.meta.annotationsPermissions?.organization.canEdit;
}
}
return canEdit && this.canEditDashboard();
};
canDeleteAnnotation = (dashboardId: number) => {
let canDelete = true;
if (contextSrv.accessControlEnabled()) {
if (dashboardId !== 0) {
canDelete = !!this.props.dashboard.meta.annotationsPermissions?.dashboard.canDelete;
} else {
canDelete = !!this.props.dashboard.meta.annotationsPermissions?.organization.canDelete;
}
}
return canDelete && this.canEditDashboard();
};
// Due to a mutable panel model we get the sync settings via function that proactively reads from the model
getSync = () => (this.props.isEditing ? DashboardCursorSync.Off : this.props.dashboard.graphTooltip);
@ -400,7 +362,7 @@ export class PanelChrome extends PureComponent<Props, State> {
onAnnotationCreate = async (event: AnnotationEventUIModel) => {
const isRegion = event.from !== event.to;
const anno = {
dashboardId: this.props.dashboard.id,
dashboardUID: this.props.dashboard.uid,
panelId: this.props.panel.id,
isRegion,
time: event.from,
@ -423,7 +385,7 @@ export class PanelChrome extends PureComponent<Props, State> {
const isRegion = event.from !== event.to;
const anno = {
id: event.id,
dashboardId: this.props.dashboard.id,
dashboardUID: this.props.dashboard.uid,
panelId: this.props.panel.id,
isRegion,
time: event.from,

View File

@ -1,5 +1,7 @@
import { keys as _keys } from 'lodash';
import { contextSrv } from 'app/core/services/context_srv';
import { getDashboardModel } from '../../../../test/helpers/getDashboardModel';
import { variableAdapters } from '../../variables/adapters';
import { createAdHocVariableAdapter } from '../../variables/adhoc/adapter';
@ -9,7 +11,9 @@ import { setTimeSrv, TimeSrv } from '../services/TimeSrv';
import { DashboardModel } from '../state/DashboardModel';
import { PanelModel } from '../state/PanelModel';
jest.mock('app/core/services/context_srv', () => ({}));
jest.mock('app/core/services/context_srv');
const mockContextSrv = jest.mocked(contextSrv, true);
variableAdapters.setInit(() => [
createQueryVariableAdapter(),
@ -860,20 +864,160 @@ describe('DashboardModel', () => {
describe('canAddAnnotations', () => {
it.each`
canEdit | canMakeEditable | expected
${false} | ${false} | ${false}
${false} | ${true} | ${true}
${true} | ${false} | ${true}
${true} | ${true} | ${true}
canEdit | canMakeEditable | canAdd | expected
${false} | ${true} | ${true} | ${true}
${true} | ${false} | ${true} | ${true}
${true} | ${true} | ${true} | ${true}
${false} | ${false} | ${true} | ${false}
${false} | ${true} | ${false} | ${false}
${true} | ${false} | ${false} | ${false}
${true} | ${true} | ${false} | ${false}
${false} | ${false} | ${false} | ${false}
`(
'when called with canEdit:{$canEdit}, canMakeEditable:{$canMakeEditable} and expected:{$expected}',
({ canEdit, canMakeEditable, expected }) => {
const dashboard = new DashboardModel({});
'when called with canEdit:{$canEdit}, canMakeEditable:{$canMakeEditable}, canAdd:{$canAdd} and expected:{$expected}',
({ canEdit, canMakeEditable, canAdd, expected }) => {
const dashboard = new DashboardModel(
{},
{
annotationsPermissions: {
dashboard: { canAdd, canEdit: true, canDelete: true },
organization: { canAdd: false, canEdit: false, canDelete: false },
},
}
);
dashboard.meta.canEdit = canEdit;
dashboard.meta.canMakeEditable = canMakeEditable;
mockContextSrv.accessControlEnabled.mockReturnValue(true);
const result = dashboard.canAddAnnotations();
expect(result).toBe(expected);
}
);
});
const result = dashboard.canEditDashboard();
describe('canEditAnnotations', () => {
it.each`
canEdit | canMakeEditable | canEditWithOrgPermission | expected
${false} | ${true} | ${true} | ${true}
${true} | ${false} | ${true} | ${true}
${true} | ${true} | ${true} | ${true}
${false} | ${false} | ${true} | ${false}
${false} | ${true} | ${false} | ${false}
${true} | ${false} | ${false} | ${false}
${true} | ${true} | ${false} | ${false}
${false} | ${false} | ${false} | ${false}
`(
'when called with canEdit:{$canEdit}, canMakeEditable:{$canMakeEditable}, canEditWithOrgPermission:{$canEditWithOrgPermission} and expected:{$expected}',
({ canEdit, canMakeEditable, canEditWithOrgPermission, expected }) => {
const dashboard = new DashboardModel(
{},
{
annotationsPermissions: {
dashboard: { canAdd: false, canEdit: false, canDelete: true },
organization: { canAdd: false, canEdit: canEditWithOrgPermission, canDelete: false },
},
}
);
dashboard.meta.canEdit = canEdit;
dashboard.meta.canMakeEditable = canMakeEditable;
mockContextSrv.accessControlEnabled.mockReturnValue(true);
const result = dashboard.canEditAnnotations();
expect(result).toBe(expected);
}
);
it.each`
canEdit | canMakeEditable | canEditWithDashboardPermission | expected
${false} | ${true} | ${true} | ${true}
${true} | ${false} | ${true} | ${true}
${true} | ${true} | ${true} | ${true}
${false} | ${false} | ${true} | ${false}
${false} | ${true} | ${false} | ${false}
${true} | ${false} | ${false} | ${false}
${true} | ${true} | ${false} | ${false}
${false} | ${false} | ${false} | ${false}
`(
'when called with canEdit:{$canEdit}, canMakeEditable:{$canMakeEditable}, canEditWithDashboardPermission:{$canEditWithDashboardPermission} and expected:{$expected}',
({ canEdit, canMakeEditable, canEditWithDashboardPermission, expected }) => {
const dashboard = new DashboardModel(
{},
{
annotationsPermissions: {
dashboard: { canAdd: false, canEdit: canEditWithDashboardPermission, canDelete: true },
organization: { canAdd: false, canEdit: false, canDelete: false },
},
}
);
dashboard.meta.canEdit = canEdit;
dashboard.meta.canMakeEditable = canMakeEditable;
mockContextSrv.accessControlEnabled.mockReturnValue(true);
const result = dashboard.canEditAnnotations('testDashboardUID');
expect(result).toBe(expected);
}
);
});
describe('canDeleteAnnotations', () => {
it.each`
canEdit | canMakeEditable | canDeleteWithOrgPermission | expected
${false} | ${true} | ${true} | ${true}
${true} | ${false} | ${true} | ${true}
${true} | ${true} | ${true} | ${true}
${false} | ${false} | ${true} | ${false}
${false} | ${true} | ${false} | ${false}
${true} | ${false} | ${false} | ${false}
${true} | ${true} | ${false} | ${false}
${false} | ${false} | ${false} | ${false}
`(
'when called with canEdit:{$canEdit}, canMakeEditable:{$canMakeEditable}, canDeleteWithOrgPermission:{$canDeleteWithOrgPermission} and expected:{$expected}',
({ canEdit, canMakeEditable, canDeleteWithOrgPermission, expected }) => {
const dashboard = new DashboardModel(
{},
{
annotationsPermissions: {
dashboard: { canAdd: false, canEdit: false, canDelete: false },
organization: { canAdd: false, canEdit: false, canDelete: canDeleteWithOrgPermission },
},
}
);
dashboard.meta.canEdit = canEdit;
dashboard.meta.canMakeEditable = canMakeEditable;
mockContextSrv.accessControlEnabled.mockReturnValue(true);
const result = dashboard.canDeleteAnnotations();
expect(result).toBe(expected);
}
);
it.each`
canEdit | canMakeEditable | canDeleteWithDashboardPermission | expected
${false} | ${true} | ${true} | ${true}
${true} | ${false} | ${true} | ${true}
${true} | ${true} | ${true} | ${true}
${false} | ${false} | ${true} | ${false}
${false} | ${true} | ${false} | ${false}
${true} | ${false} | ${false} | ${false}
${true} | ${true} | ${false} | ${false}
${false} | ${false} | ${false} | ${false}
`(
'when called with canEdit:{$canEdit}, canMakeEditable:{$canMakeEditable}, canDeleteWithDashboardPermission:{$canDeleteWithDashboardPermission} and expected:{$expected}',
({ canEdit, canMakeEditable, canDeleteWithDashboardPermission, expected }) => {
const dashboard = new DashboardModel(
{},
{
annotationsPermissions: {
dashboard: { canAdd: false, canEdit: false, canDelete: canDeleteWithDashboardPermission },
organization: { canAdd: false, canEdit: false, canDelete: false },
},
}
);
dashboard.meta.canEdit = canEdit;
dashboard.meta.canMakeEditable = canMakeEditable;
mockContextSrv.accessControlEnabled.mockReturnValue(true);
const result = dashboard.canDeleteAnnotations('testDashboardUID');
expect(result).toBe(expected);
}
);

View File

@ -1073,12 +1073,13 @@ export class DashboardModel implements TimeModel {
return this.getVariablesFromState(this.uid);
}
canEditAnnotations(dashboardId: number) {
canEditAnnotations(dashboardUID?: string) {
let canEdit = true;
// if RBAC is enabled there are additional conditions to check
if (contextSrv.accessControlEnabled()) {
if (dashboardId === 0) {
// dashboardUID is falsy when it is an organizational annotation
if (!dashboardUID) {
canEdit = !!this.meta.annotationsPermissions?.organization.canEdit;
} else {
canEdit = !!this.meta.annotationsPermissions?.dashboard.canEdit;
@ -1087,14 +1088,29 @@ export class DashboardModel implements TimeModel {
return this.canEditDashboard() && canEdit;
}
canDeleteAnnotations(dashboardUID?: string) {
let canDelete = true;
if (contextSrv.accessControlEnabled()) {
// dashboardUID is falsy when it is an organizational annotation
if (!dashboardUID) {
canDelete = !!this.meta.annotationsPermissions?.organization.canDelete;
} else {
canDelete = !!this.meta.annotationsPermissions?.dashboard.canDelete;
}
}
return canDelete && this.canEditDashboard();
}
canAddAnnotations() {
// If RBAC is enabled there are additional conditions to check
const canAdd = !contextSrv.accessControlEnabled() || this.meta.annotationsPermissions?.dashboard.canAdd;
// If RBAC is enabled there are additional conditions to check.
const canAdd = !contextSrv.accessControlEnabled() || Boolean(this.meta.annotationsPermissions?.dashboard.canAdd);
return this.canEditDashboard() && canAdd;
}
canEditDashboard() {
return this.meta.canEdit || this.meta.canMakeEditable;
return Boolean(this.meta.canEdit || this.meta.canMakeEditable);
}
shouldUpdateDashboardPanelFromJSON(updatedPanel: PanelModel, panel: PanelModel) {

View File

@ -64,7 +64,7 @@ describe('grafana data source', () => {
type: GrafanaAnnotationType.Dashboard,
tags: ['tag1'],
},
{ id: 1 }
{ uid: 'DSNdW0gVk' }
);
beforeEach(() => {
@ -78,7 +78,7 @@ describe('grafana data source', () => {
});
});
function setupAnnotationQueryOptions(annotation: Partial<GrafanaAnnotationQuery>, dashboard?: { id: number }) {
function setupAnnotationQueryOptions(annotation: Partial<GrafanaAnnotationQuery>, dashboard?: { uid: string }) {
return {
annotation: {
target: annotation,

View File

@ -170,11 +170,11 @@ export class GrafanaDatasource extends DataSourceWithBackend<GrafanaQuery> {
if (target.type === GrafanaAnnotationType.Dashboard) {
// if no dashboard id yet return
if (!options.dashboard.id) {
if (!options.dashboard.uid) {
return Promise.resolve({ data: [] });
}
// filter by dashboard id
params.dashboardId = options.dashboard.id;
params.dashboardUID = options.dashboard.uid;
// remove tags filter if any
delete params.tags;
} else {
@ -202,7 +202,7 @@ export class GrafanaDatasource extends DataSourceWithBackend<GrafanaQuery> {
const annotations = await getBackendSrv().get(
'/api/annotations',
params,
`grafana-data-source-annotations-${annotation.name}-${options.dashboard?.id}`
`grafana-data-source-annotations-${annotation.name}-${options.dashboard?.uid}`
);
return { data: [toDataFrame(annotations)] };
}

View File

@ -41,6 +41,8 @@ const defaultResult: any = {
panelId: 13,
dashboardId: 14, // deliberately different from panelId
id: 14,
uid: '7MeksYbmk',
dashboardUID: '7MeksYbmk',
url: '/d/asdkjhajksd/some-dash',
};
@ -53,7 +55,7 @@ async function setupTestContext({
const getMock = jest.spyOn(backendSrv, 'get');
getMock.mockResolvedValue(results);
const dash: any = { id: 1, formatDate: (time: number) => new Date(time).toISOString() };
const dash: any = { uid: 'srx16xR4z', formatDate: (time: number) => new Date(time).toISOString() };
const dashSrv: any = { getCurrent: () => dash };
setDashboardSrv(dashSrv);
const pushSpy = jest.spyOn(locationService, 'push');
@ -99,7 +101,7 @@ describe('AnnoListPanel', () => {
expect(getMock).toHaveBeenCalledWith(
'/api/annotations',
{
dashboardId: 1,
dashboardUID: 'srx16xR4z',
limit: 10,
tags: ['tag A', 'tag B'],
type: 'annotation',
@ -205,7 +207,7 @@ describe('AnnoListPanel', () => {
await userEvent.click(screen.getByText(/result text/i));
await waitFor(() => expect(getMock).toHaveBeenCalledTimes(1));
expect(getMock).toHaveBeenCalledWith('/api/search', { dashboardIds: 14 });
expect(getMock).toHaveBeenCalledWith('/api/search', { dashboardUIDs: '7MeksYbmk' });
expect(pushSpy).toHaveBeenCalledTimes(1);
expect(pushSpy).toHaveBeenCalledWith('/d/asdkjhajksd/some-dash?from=1609458600000&to=1609459800000');
});
@ -223,7 +225,7 @@ describe('AnnoListPanel', () => {
expect(getMock).toHaveBeenCalledWith(
'/api/annotations',
{
dashboardId: 1,
dashboardUID: 'srx16xR4z',
limit: 10,
tags: ['tag A', 'tag B', 'Result tag B'],
type: 'annotation',
@ -247,7 +249,7 @@ describe('AnnoListPanel', () => {
expect(getMock).toHaveBeenCalledWith(
'/api/annotations',
{
dashboardId: 1,
dashboardUID: 'srx16xR4z',
limit: 10,
tags: ['tag A', 'tag B'],
type: 'annotation',

View File

@ -97,7 +97,7 @@ export class AnnoListPanel extends PureComponent<Props, State> {
};
if (options.onlyFromThisDashboard) {
params.dashboardId = getDashboardSrv().getCurrent()?.id;
params.dashboardUID = getDashboardSrv().getCurrent()?.uid;
}
let timeInfo = '';
@ -148,13 +148,13 @@ export class AnnoListPanel extends PureComponent<Props, State> {
params.viewPanel = anno.panelId;
}
if (current?.id === anno.dashboardId) {
if (current?.uid === anno.dashboardUID) {
locationService.partial(params);
return;
}
const result = await getBackendSrv().get('/api/search', { dashboardIds: anno.dashboardId });
if (result && result.length && result[0].id === anno.dashboardId) {
const result = await getBackendSrv().get('/api/search', { dashboardUIDs: anno.dashboardUID });
if (result && result.length && result[0].uid === anno.dashboardUID) {
const dash = result[0];
const url = new URL(dash.url, window.location.origin);
url.searchParams.set('from', params.from);
@ -162,7 +162,7 @@ export class AnnoListPanel extends PureComponent<Props, State> {
locationService.push(locationUtil.stripBaseFromUrl(url.toString()));
return;
}
appEvents.emit(AppEvents.alertWarning, ['Unknown Dashboard: ' + anno.dashboardId]);
appEvents.emit(AppEvents.alertWarning, ['Unknown Dashboard: ' + anno.dashboardUID]);
};
_timeOffset(time: number, offset: string, subtract = false): number {

View File

@ -23,7 +23,7 @@ export class EventEditorCtrl {
$onInit() {
this.event.panelId = this.panelCtrl.panel.id; // set correct id if in panel edit
this.event.dashboardId = this.panelCtrl.dashboard.id;
this.event.dashboardUID = this.panelCtrl.dashboard.uid;
// Annotations query returns time as Unix timestamp in milliseconds
this.event.time = tryEpochToMoment(this.event.time);

View File

@ -31,7 +31,7 @@ export class EventManager {
updateTime(range: { from: any; to: any }) {
if (!this.event) {
this.event = {};
this.event.dashboardId = this.panelCtrl.dashboard.id;
this.event.dashboardUID = this.panelCtrl.dashboard.uid;
this.event.panelId = this.panelCtrl.panel.id;
}

View File

@ -95,8 +95,8 @@ export function AnnotationMarker({ annotation, timeZone, width }: Props) {
timeFormatter={timeFormatter}
onEdit={onAnnotationEdit}
onDelete={onAnnotationDelete}
canEdit={canEditAnnotations!(annotation.dashboardId)}
canDelete={canDeleteAnnotations!(annotation.dashboardId)}
canEdit={canEditAnnotations!(annotation.dashboardUID)}
canDelete={canDeleteAnnotations!(annotation.dashboardUID)}
/>
);
}, [canEditAnnotations, canDeleteAnnotations, onAnnotationDelete, onAnnotationEdit, timeFormatter, annotation]);

View File

@ -1,6 +1,8 @@
interface AnnotationsDataFrameViewDTO {
id: string;
/** @deprecate */
dashboardId: number;
dashboardUID: string;
time: number;
timeEnd: number;
text: string;