From 00af0afe522c363a714720f48d0dcbfb82db3898 Mon Sep 17 00:00:00 2001 From: Alexa V <239999+axelavargas@users.noreply.github.com> Date: Mon, 14 Oct 2024 12:11:31 +0200 Subject: [PATCH] Dashboard Scene: Fix permissions and error handling in creating alert from panel (#94521) * Dashboard Scene: Add better error handling when creating alert from panel menu * Add missing permission to createNewAlert and add unit test --- .../scene/PanelMenuBehavior.test.tsx | 121 +++++++++++++++++- .../scene/PanelMenuBehavior.tsx | 37 ++++-- 2 files changed, 146 insertions(+), 12 deletions(-) diff --git a/public/app/features/dashboard-scene/scene/PanelMenuBehavior.test.tsx b/public/app/features/dashboard-scene/scene/PanelMenuBehavior.test.tsx index 5b0fa84910a..d0aa2f56b9f 100644 --- a/public/app/features/dashboard-scene/scene/PanelMenuBehavior.test.tsx +++ b/public/app/features/dashboard-scene/scene/PanelMenuBehavior.test.tsx @@ -6,9 +6,10 @@ import { PluginExtensionTypes, getDefaultTimeRange, toDataFrame, + urlUtil, } from '@grafana/data'; import { getPanelPlugin } from '@grafana/data/test/__mocks__/pluginMocks'; -import { getPluginLinkExtensions, locationService } from '@grafana/runtime'; +import { config, getPluginLinkExtensions, locationService } from '@grafana/runtime'; import { LocalValueVariable, SceneQueryRunner, @@ -19,6 +20,10 @@ import { } from '@grafana/scenes'; import { contextSrv } from 'app/core/services/context_srv'; import { GetExploreUrlArguments } from 'app/core/utils/explore'; +import { grantUserPermissions } from 'app/features/alerting/unified/mocks'; +import { scenesPanelToRuleFormValues } from 'app/features/alerting/unified/utils/rule-form'; +import * as storeModule from 'app/store/store'; +import { AccessControlAction } from 'app/types'; import { buildPanelEditScene } from '../panel-edit/PanelEditor'; @@ -30,6 +35,7 @@ import { DefaultGridLayoutManager } from './layout-default/DefaultGridLayoutMana const mocks = { contextSrv: jest.mocked(contextSrv), getExploreUrl: jest.fn(), + notifyApp: jest.fn(), }; jest.mock('app/core/utils/explore', () => ({ @@ -47,6 +53,10 @@ jest.mock('@grafana/runtime', () => ({ getPluginLinkExtensions: jest.fn(), })); +jest.mock('app/store/store', () => ({ + dispatch: jest.fn(), +})); + const getPluginLinkExtensionsMock = jest.mocked(getPluginLinkExtensions); describe('panelMenuBehavior', () => { @@ -102,6 +112,9 @@ describe('panelMenuBehavior', () => { mocks.contextSrv.hasAccessToExplore.mockReturnValue(true); mocks.getExploreUrl.mockReturnValue(Promise.resolve('/explore')); + config.unifiedAlertingEnabled = true; + grantUserPermissions([AccessControlAction.AlertingRuleRead, AccessControlAction.AlertingRuleUpdate]); + menu.activate(); await new Promise((r) => setTimeout(r, 1)); @@ -548,6 +561,112 @@ describe('panelMenuBehavior', () => { expect(menu.state.items?.[0].text).toBe('Explore'); }); }); + + describe('onCreateAlert', () => { + beforeEach(() => { + jest.spyOn(storeModule, 'dispatch').mockImplementation(() => {}); + jest.spyOn(locationService, 'push').mockImplementation(() => {}); + jest.spyOn(urlUtil, 'renderUrl').mockImplementation((url, params) => `${url}?${JSON.stringify(params)}`); + }); + + it('should navigate to alert creation page on success', async () => { + const { menu, panel } = await buildTestScene({}); + const mockFormValues = { someKey: 'someValue' }; + + config.unifiedAlertingEnabled = true; + grantUserPermissions([AccessControlAction.AlertingRuleRead, AccessControlAction.AlertingRuleUpdate]); + + jest + .spyOn(require('app/features/alerting/unified/utils/rule-form'), 'scenesPanelToRuleFormValues') + .mockResolvedValue(mockFormValues); + + // activate the menu + menu.activate(); + // wait for the menu to be activated + await new Promise((r) => setTimeout(r, 1)); + // use userEvent mechanism to click the menu item + const moreMenu = menu.state.items?.find((i) => i.text === 'More...')?.subMenu; + const alertMenuItem = moreMenu?.find((i) => i.text === 'New alert rule')?.onClick; + expect(alertMenuItem).toBeDefined(); + + alertMenuItem?.({} as React.MouseEvent); + expect(scenesPanelToRuleFormValues).toHaveBeenCalledWith(panel); + }); + + it('should show error notification on failure', async () => { + const { menu, panel } = await buildTestScene({}); + const mockError = new Error('Test error'); + jest + .spyOn(require('app/features/alerting/unified/utils/rule-form'), 'scenesPanelToRuleFormValues') + .mockRejectedValue(mockError); + // Don't make notifyApp throw an error, just mock it + + menu.activate(); + await new Promise((r) => setTimeout(r, 1)); + + const moreMenu = menu.state.items?.find((i) => i.text === 'More...')?.subMenu; + const alertMenuItem = moreMenu?.find((i) => i.text === 'New alert rule')?.onClick; + expect(alertMenuItem).toBeDefined(); + + await alertMenuItem?.({} as React.MouseEvent); + + await new Promise((r) => setTimeout(r, 0)); + + expect(scenesPanelToRuleFormValues).toHaveBeenCalledWith(panel); + }); + + it('should render "New alert rule" menu item when user has permissions to read and update alerts', async () => { + const { menu } = await buildTestScene({}); + config.unifiedAlertingEnabled = true; + grantUserPermissions([AccessControlAction.AlertingRuleRead, AccessControlAction.AlertingRuleUpdate]); + + menu.activate(); + await new Promise((r) => setTimeout(r, 1)); + + const moreMenu = menu.state.items?.find((i) => i.text === 'More...')?.subMenu; + expect(moreMenu?.find((i) => i.text === 'New alert rule')).toBeDefined(); + }); + + it('should not contain "New alert rule" menu item when user does not have permissions to read and update alerts', async () => { + const { menu } = await buildTestScene({}); + config.unifiedAlertingEnabled = true; + grantUserPermissions([AccessControlAction.AlertingRuleRead]); + + menu.activate(); + await new Promise((r) => setTimeout(r, 1)); + + const moreMenu = menu.state.items?.find((i) => i.text === 'More...')?.subMenu; + expect(moreMenu?.find((i) => i.text === 'New alert rule')).toBeUndefined(); + }); + + it('should not contain "New alert rule" menu item when unifiedAlertingEnabled is false', async () => { + const { menu } = await buildTestScene({}); + config.unifiedAlertingEnabled = false; + + menu.activate(); + await new Promise((r) => setTimeout(r, 1)); + + const moreMenu = menu.state.items?.find((i) => i.text === 'More...')?.subMenu; + expect(moreMenu?.find((i) => i.text === 'New alert rule')).toBeUndefined(); + }); + + it('should not contain "New alert rule" menu item when user does not have permissions to read and update alerts', async () => { + const { menu } = await buildTestScene({}); + config.unifiedAlertingEnabled = true; + grantUserPermissions([AccessControlAction.AlertingRuleRead]); + + menu.activate(); + await new Promise((r) => setTimeout(r, 1)); + + const moreMenu = menu.state.items?.find((i) => i.text === 'More...')?.subMenu; + const alertMenuItem = moreMenu?.find((i) => i.text === 'New alert rule')?.onClick; + expect(alertMenuItem).toBeUndefined(); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + }); }); interface SceneOptions { diff --git a/public/app/features/dashboard-scene/scene/PanelMenuBehavior.tsx b/public/app/features/dashboard-scene/scene/PanelMenuBehavior.tsx index 982bd12b6b2..a3e1706ec3f 100644 --- a/public/app/features/dashboard-scene/scene/PanelMenuBehavior.tsx +++ b/public/app/features/dashboard-scene/scene/PanelMenuBehavior.tsx @@ -12,14 +12,19 @@ import { config, getPluginLinkExtensions, locationService } from '@grafana/runti import { LocalValueVariable, sceneGraph, SceneGridRow, VizPanel, VizPanelMenu } from '@grafana/scenes'; import { DataQuery, OptionsWithLegend } from '@grafana/schema'; import appEvents from 'app/core/app_events'; +import { createErrorNotification } from 'app/core/copy/appNotification'; import { t } from 'app/core/internationalization'; +import { notifyApp } from 'app/core/reducers/appNotification'; import { contextSrv } from 'app/core/services/context_srv'; +import { getMessageFromError } from 'app/core/utils/errors'; +import { getCreateAlertInMenuAvailability } from 'app/features/alerting/unified/utils/access-control'; import { scenesPanelToRuleFormValues } from 'app/features/alerting/unified/utils/rule-form'; import { shareDashboardType } from 'app/features/dashboard/components/ShareModal/utils'; import { InspectTab } from 'app/features/inspector/types'; import { getScenePanelLinksSupplier } from 'app/features/panel/panellinks/linkSuppliers'; import { createExtensionSubMenu } from 'app/features/plugins/extensions/utils'; import { addDataTrailPanelAction } from 'app/features/trails/Integrations/dashboardIntegration'; +import { dispatch } from 'app/store/store'; import { ShowConfirmModalEvent } from 'app/types/events'; import { ShareDrawer } from '../sharing/ShareDrawer/ShareDrawer'; @@ -214,11 +219,15 @@ export function panelMenuBehavior(menu: VizPanelMenu, isRepeat = false) { } } - moreSubMenu.push({ - text: t('panel.header-menu.new-alert-rule', `New alert rule`), - iconClassName: 'bell', - onClick: (e) => onCreateAlert(panel), - }); + const isCreateAlertMenuOptionAvailable = getCreateAlertInMenuAvailability(); + + if (isCreateAlertMenuOptionAvailable) { + moreSubMenu.push({ + text: t('panel.header-menu.new-alert-rule', `New alert rule`), + iconClassName: 'bell', + onClick: (e) => onCreateAlert(panel), + }); + } if (hasLegendOptions(panel.state.options) && !isEditingPanel) { moreSubMenu.push({ @@ -482,12 +491,18 @@ export function onRemovePanel(dashboard: DashboardScene, panel: VizPanel) { } const onCreateAlert = async (panel: VizPanel) => { - const formValues = await scenesPanelToRuleFormValues(panel); - const ruleFormUrl = urlUtil.renderUrl('/alerting/new', { - defaults: JSON.stringify(formValues), - returnTo: location.pathname + location.search, - }); - locationService.push(ruleFormUrl); + try { + const formValues = await scenesPanelToRuleFormValues(panel); + const ruleFormUrl = urlUtil.renderUrl('/alerting/new', { + defaults: JSON.stringify(formValues), + returnTo: location.pathname + location.search, + }); + locationService.push(ruleFormUrl); + } catch (err) { + const message = `Error getting rule values from the panel: ${getMessageFromError(err)}`; + dispatch(notifyApp(createErrorNotification(message))); + return; + } }; export function toggleVizPanelLegend(vizPanel: VizPanel): void {