Graph: Fixes so only users with correct permissions can add annotations (#30419)

* Graph: Fixes so only users with edit permissions can add annotations

* Tests: corrects test message text

* Chore: changes after PR comments

public/app/features/annotations/annotation_tooltip.ts

@@ -59,7 +59,7 @@ export function annotationTooltipDirective(
       `;
 
       // Show edit icon only for users with at least Editor role
-      if (event.id && dashboard.meta.canEdit) {
+      if (event.id && dashboard.canAddAnnotations()) {
         header += `
           <span class="pointer graph-annotation__edit-icon" ng-click="onEdit()">
             <i class="fa fa-pencil-square"></i>

public/app/features/dashboard/state/DashboardModel.test.ts

@@ -684,4 +684,25 @@ describe('DashboardModel', () => {
       expect(legendsOn.length).toBe(0);
     });
   });
+
+  describe('canAddAnnotations', () => {
+    it.each`
+      canEdit  | canMakeEditable | expected
+      ${false} | ${false}        | ${false}
+      ${false} | ${true}         | ${true}
+      ${true}  | ${false}        | ${true}
+      ${true}  | ${true}         | ${true}
+    `(
+      'when called with canEdit:{$canEdit}, canMakeEditable:{$canMakeEditable} and expected:{$expected}',
+      ({ canEdit, canMakeEditable, expected }) => {
+        const dashboard = new DashboardModel({});
+        dashboard.meta.canEdit = canEdit;
+        dashboard.meta.canMakeEditable = canMakeEditable;
+
+        const result = dashboard.canAddAnnotations();
+
+        expect(result).toBe(expected);
+      }
+    );
+  });
 });

public/app/features/dashboard/state/DashboardModel.ts

@@ -1016,6 +1016,10 @@ export class DashboardModel {
     return this.getVariablesFromState();
   };
 
+  canAddAnnotations() {
+    return this.meta.canEdit || this.meta.canMakeEditable;
+  }
+
   private getPanelRepeatVariable(panel: PanelModel) {
     return this.getVariablesFromState().find((variable) => variable.name === panel.repeat);
   }

public/app/plugins/panel/graph/graph.ts

@@ -180,7 +180,7 @@ class GraphElement {
       return;
     }
 
-    if ((ranges.ctrlKey || ranges.metaKey) && (this.dashboard.meta.canEdit || this.dashboard.meta.canMakeEditable)) {
+    if ((ranges.ctrlKey || ranges.metaKey) && this.dashboard.canAddAnnotations()) {
       // Add annotation
       setTimeout(() => {
         this.eventManager.updateTime(ranges.xaxis);
@@ -201,7 +201,7 @@ class GraphElement {
   ): (() => MenuItemsGroup[]) => {
     return () => {
       // Fixed context menu items
-      const items: MenuItemsGroup[] = this.dashboard?.editable
+      const items: MenuItemsGroup[] = this.dashboard.canAddAnnotations()
         ? [
             {
               items: [
@@ -253,7 +253,7 @@ class GraphElement {
       }
 
       // skip if dashboard is not saved yet (exists in db) or user cannot edit
-      if (!this.dashboard.id || (!this.dashboard.meta.canEdit && !this.dashboard.meta.canMakeEditable)) {
+      if (!this.dashboard.id || !this.dashboard.canAddAnnotations()) {
         return;
       }
 

public/app/plugins/panel/graph/specs/graph.test.ts

+import '../module';
+import { GraphCtrl } from '../module';
+import { MetricsPanelCtrl } from 'app/features/panel/metrics_panel_ctrl';
+import { PanelCtrl } from 'app/features/panel/panel_ctrl';
+import config from 'app/core/config';
+
+import TimeSeries from 'app/core/time_series2';
+import $ from 'jquery';
+import { graphDirective, GraphElement } from '../graph';
+import { dateTime, EventBusSrv } from '@grafana/data';
+import { DashboardModel } from '../../../../features/dashboard/state';
+
 jest.mock('app/features/annotations/all', () => ({
   EventManager: () => {
     return {
@@ -16,17 +28,6 @@ jest.mock('app/core/core', () => ({
   },
 }));
 
-import '../module';
-import { GraphCtrl } from '../module';
-import { MetricsPanelCtrl } from 'app/features/panel/metrics_panel_ctrl';
-import { PanelCtrl } from 'app/features/panel/panel_ctrl';
-import config from 'app/core/config';
-
-import TimeSeries from 'app/core/time_series2';
-import $ from 'jquery';
-import { graphDirective, GraphElement } from '../graph';
-import { dateTime, EventBusSrv } from '@grafana/data';
-
 const ctx = {} as any;
 let ctrl: any;
 const scope = {
@@ -1288,25 +1289,23 @@ describe('grafanaGraph', () => {
   });
 
   describe('getContextMenuItemsSupplier', () => {
-    function getGraphElement({ editable }: { editable?: boolean } = {}) {
-      const element = new GraphElement(
-        {
-          ctrl: {
-            contextMenuCtrl: {},
-            dashboard: { editable, events: { on: jest.fn() } },
-            events: { on: jest.fn() },
-          },
-        },
-        { mouseleave: jest.fn(), bind: jest.fn() } as any,
-        {} as any
-      );
+    describe('when called and user can edit the dashboard', () => {
+      it('then the correct menu items should be returned', () => {
+        const element = getGraphElement({ canEdit: true, canMakeEditable: false });
 
-      return element;
-    }
+        const result = element.getContextMenuItemsSupplier({ x: 1, y: 1 })();
 
-    describe('when called and dashboard is editable', () => {
+        expect(result.length).toEqual(1);
+        expect(result[0].items.length).toEqual(1);
+        expect(result[0].items[0].label).toEqual('Add annotation');
+        expect(result[0].items[0].icon).toEqual('comment-alt');
+        expect(result[0].items[0].onClick).toBeDefined();
+      });
+    });
+
+    describe('when called and user can make the dashboard editable', () => {
       it('then the correct menu items should be returned', () => {
-        const element = getGraphElement({ editable: true });
+        const element = getGraphElement({ canEdit: false, canMakeEditable: true });
 
         const result = element.getContextMenuItemsSupplier({ x: 1, y: 1 })();
 
@@ -1318,9 +1317,9 @@ describe('grafanaGraph', () => {
       });
     });
 
-    describe('when called and dashboard is not editable', () => {
+    describe('when called and user can not edit the dashboard and can not make the dashboard editable', () => {
       it('then the correct menu items should be returned', () => {
-        const element = getGraphElement({ editable: false });
+        const element = getGraphElement({ canEdit: false, canMakeEditable: false });
 
         const result = element.getContextMenuItemsSupplier({ x: 1, y: 1 })();
 
@@ -1329,3 +1328,23 @@ describe('grafanaGraph', () => {
     });
   });
 });
+
+function getGraphElement({ canEdit, canMakeEditable }: { canEdit?: boolean; canMakeEditable?: boolean } = {}) {
+  const dashboard = new DashboardModel({});
+  dashboard.events.on = jest.fn();
+  dashboard.meta.canEdit = canEdit;
+  dashboard.meta.canMakeEditable = canMakeEditable;
+  const element = new GraphElement(
+    {
+      ctrl: {
+        contextMenuCtrl: {},
+        dashboard,
+        events: { on: jest.fn() },
+      },
+    },
+    { mouseleave: jest.fn(), bind: jest.fn() } as any,
+    {} as any
+  );
+
+  return element;
+}