From 0929feba06f829351368d3ad4aed2938f2b8c049 Mon Sep 17 00:00:00 2001 From: Gilles De Mey Date: Tue, 19 Nov 2024 11:41:31 +0100 Subject: [PATCH] Alerting: Use NestedFolderPicker (#96398) Co-authored-by: Tom Ratcliffe --- .betterer.results | 3 - .../alerting/unified/CloneRuleEditor.test.tsx | 47 +----------- .../RuleEditorCloudOnlyAllowed.test.tsx | 10 +-- .../unified/RuleEditorCloudRules.test.tsx | 10 +-- .../unified/RuleEditorGrafanaRules.test.tsx | 69 +++-------------- .../PanelAlertTabContent.test.tsx.snap | 1 + .../alerting/unified/api/alertRuleApi.ts | 2 +- .../components/rule-editor/FolderAndGroup.tsx | 53 ++++++------- .../rule-editor/RuleFolderPicker.tsx | 76 ------------------- .../SimplifiedRuleEditor.test.tsx | 19 ++--- .../NotificationPreview.test.tsx | 2 +- .../NotificationPreview.tsx | 4 +- .../unified/mocks/server/handlers/folders.ts | 41 +++++++++- .../alerting/unified/types/rule-form.ts | 6 +- .../alerting/unified/utils/rule-form.ts | 36 +++++---- .../api/browseDashboardsAPI.ts | 4 - .../components/CreateNewButton.tsx | 20 ++++- public/app/types/acl.ts | 1 - 18 files changed, 133 insertions(+), 271 deletions(-) delete mode 100644 public/app/features/alerting/unified/components/rule-editor/RuleFolderPicker.tsx diff --git a/.betterer.results b/.betterer.results index 2af5d4872d0..6f16c0ac58a 100644 --- a/.betterer.results +++ b/.betterer.results @@ -1682,9 +1682,6 @@ exports[`better eslint`] = { "public/app/features/alerting/unified/components/rule-editor/RuleEditorSection.tsx:5381": [ [0, 0, 0, "No untranslated strings. Wrap text with ", "0"] ], - "public/app/features/alerting/unified/components/rule-editor/RuleFolderPicker.tsx:5381": [ - [0, 0, 0, "No untranslated strings. Wrap text with ", "0"] - ], "public/app/features/alerting/unified/components/rule-editor/RuleInspector.tsx:5381": [ [0, 0, 0, "Do not use any type assertions.", "0"], [0, 0, 0, "No untranslated strings. Wrap text with ", "1"], diff --git a/public/app/features/alerting/unified/CloneRuleEditor.test.tsx b/public/app/features/alerting/unified/CloneRuleEditor.test.tsx index 38155aa917a..a34dde92bc8 100644 --- a/public/app/features/alerting/unified/CloneRuleEditor.test.tsx +++ b/public/app/features/alerting/unified/CloneRuleEditor.test.tsx @@ -3,9 +3,7 @@ import { FormProvider, useForm } from 'react-hook-form'; import { getWrapper, render, waitFor, waitForElementToBeRemoved, within } from 'test/test-utils'; import { byRole, byTestId, byText } from 'testing-library-selector'; -import { selectors } from '@grafana/e2e-selectors/src'; import { setDataSourceSrv } from '@grafana/runtime'; -import { DashboardSearchItem, DashboardSearchItemType } from 'app/features/search/types'; import { RuleWithLocation } from 'app/types/unified-alerting'; import { AccessControlAction } from '../../../types'; @@ -18,7 +16,7 @@ import { import { cloneRuleDefinition, CloneRuleEditor } from './CloneRuleEditor'; import { ExpressionEditorProps } from './components/rule-editor/ExpressionEditor'; -import { mockFeatureDiscoveryApi, mockSearchApi, setupMswServer } from './mockApi'; +import { mockFeatureDiscoveryApi, setupMswServer } from './mockApi'; import { grantUserPermissions, mockDataSource, @@ -44,12 +42,6 @@ jest.mock('./components/rule-editor/ExpressionEditor', () => ({ ), })); -// For simplicity of the test we mock the NotificationPreview component -// Otherwise we would need to mock a few more HTTP api calls which are not relevant for these tests -jest.mock('./components/rule-editor/notificaton-preview/NotificationPreview', () => ({ - NotificationPreview: () =>
, -})); - jest.spyOn(AlertingQueryRunner.prototype, 'run').mockImplementation(() => Promise.resolve()); const server = setupMswServer(); @@ -58,7 +50,7 @@ const ui = { inputs: { name: byRole('textbox', { name: 'name' }), expr: byTestId('expr'), - folderContainer: byTestId(selectors.components.FolderPicker.containerV2), + folderContainer: byTestId('folder-picker'), namespace: byTestId('namespace-picker'), group: byTestId('group-picker'), annotationValue: (idx: number) => byTestId(`annotation-value-${idx}`), @@ -84,14 +76,6 @@ describe('CloneRuleEditor', function () { it('should populate form values from the existing alert rule', async function () { setDataSourceSrv(new MockDataSourceSrv({})); - mockSearchApi(server).search([ - mockDashboardSearchItem({ - title: 'folder-one', - uid: grafanaRulerRule.grafana_alert.namespace_uid, - type: DashboardSearchItemType.DashDB, - }), - ]); - render( , { wrapper: Wrapper } @@ -105,7 +89,7 @@ describe('CloneRuleEditor', function () { await waitFor(() => { expect(ui.inputs.name.get()).toHaveValue(`${grafanaRulerRule.grafana_alert.title} (copy)`); }); - expect(ui.inputs.folderContainer.get()).toHaveTextContent('folder-one'); + expect(ui.inputs.folderContainer.get()).toHaveTextContent('Folder A'); expect(ui.inputs.group.get()).toHaveTextContent(grafanaRulerRule.grafana_alert.rule_group); expect( byRole('listitem', { @@ -147,16 +131,6 @@ describe('CloneRuleEditor', function () { rules: [originRule], }); - mockSearchApi(server).search([ - mockDashboardSearchItem({ - title: 'folder-one', - uid: '123', - type: DashboardSearchItemType.DashDB, - folderTitle: 'folder-one', - folderUid: '123', - }), - ]); - render( ) { - return { - title: '', - uid: '', - type: DashboardSearchItemType.DashDB, - url: '', - uri: '', - items: [], - tags: [], - slug: '', - isStarred: false, - ...searchItem, - }; -} diff --git a/public/app/features/alerting/unified/RuleEditorCloudOnlyAllowed.test.tsx b/public/app/features/alerting/unified/RuleEditorCloudOnlyAllowed.test.tsx index cfc05cee73d..e99c56856d2 100644 --- a/public/app/features/alerting/unified/RuleEditorCloudOnlyAllowed.test.tsx +++ b/public/app/features/alerting/unified/RuleEditorCloudOnlyAllowed.test.tsx @@ -1,13 +1,11 @@ import { renderRuleEditor, ui } from 'test/helpers/alertingRuleEditor'; -import { screen, waitForElementToBeRemoved } from 'test/test-utils'; +import { screen } from 'test/test-utils'; import { byText } from 'testing-library-selector'; import { contextSrv } from 'app/core/services/context_srv'; import { AccessControlAction } from 'app/types'; import { PromApiFeatures, PromApplication } from 'app/types/unified-alerting-dto'; -import { searchFolders } from '../../manage-dashboards/state/actions'; - import { discoverFeaturesByUid } from './api/buildInfo'; import { fetchRulerRulesGroup } from './api/ruler'; import { ExpressionEditorProps } from './components/rule-editor/ExpressionEditor'; @@ -30,7 +28,6 @@ jest.mock('./api/ruler', () => ({ fetchRulerRulesGroup: jest.fn(), fetchRulerRulesNamespace: jest.fn(), })); -jest.mock('../../../../app/features/manage-dashboards/state/actions'); // there's no angular scope in test and things go terribly wrong when trying to render the query editor row. // lets just skip it @@ -116,8 +113,6 @@ const dataSources = { setupDataSources(...Object.values(dataSources)); const mocks = { - // getAllDataSources: jest.mocked(config.getAllDataSources), - searchFolders: jest.mocked(searchFolders), api: { discoverFeaturesByUid: jest.mocked(discoverFeaturesByUid), fetchRulerRulesGroup: jest.mocked(fetchRulerRulesGroup), @@ -181,11 +176,8 @@ describe('RuleEditor cloud: checking editable data sources', () => { return null; }); - mocks.searchFolders.mockResolvedValue([]); - // render rule editor, select mimir/loki managed alerts const { user } = renderRuleEditor(); - await waitForElementToBeRemoved(screen.queryAllByTestId('Spinner')); await ui.inputs.name.find(); diff --git a/public/app/features/alerting/unified/RuleEditorCloudRules.test.tsx b/public/app/features/alerting/unified/RuleEditorCloudRules.test.tsx index 6465ba48b86..a3398383671 100644 --- a/public/app/features/alerting/unified/RuleEditorCloudRules.test.tsx +++ b/public/app/features/alerting/unified/RuleEditorCloudRules.test.tsx @@ -1,7 +1,6 @@ -import userEvent from '@testing-library/user-event'; import { renderRuleEditor, ui } from 'test/helpers/alertingRuleEditor'; import { clickSelectOption } from 'test/helpers/selectOptionInTest'; -import { screen, waitForElementToBeRemoved } from 'test/test-utils'; +import { screen } from 'test/test-utils'; import { selectors } from '@grafana/e2e-selectors'; import { AccessControlAction } from 'app/types'; @@ -21,8 +20,6 @@ jest.mock('./components/rule-editor/ExpressionEditor', () => ({ ), })); -jest.mock('../../../../app/features/manage-dashboards/state/actions'); - jest.mock('app/core/components/AppChrome/AppChromeUpdate', () => ({ AppChromeUpdate: ({ actions }: { actions: React.ReactNode }) =>
{actions}
, })); @@ -46,10 +43,7 @@ describe('RuleEditor cloud', () => { }); it('can create a new cloud alert', async () => { - const user = userEvent.setup(); - - renderRuleEditor(); - await waitForElementToBeRemoved(screen.queryAllByTestId('Spinner')); + const { user } = renderRuleEditor(); const removeExpressionsButtons = screen.getAllByLabelText('Remove expression'); expect(removeExpressionsButtons).toHaveLength(2); diff --git a/public/app/features/alerting/unified/RuleEditorGrafanaRules.test.tsx b/public/app/features/alerting/unified/RuleEditorGrafanaRules.test.tsx index d290b8f58e3..4f7776594fd 100644 --- a/public/app/features/alerting/unified/RuleEditorGrafanaRules.test.tsx +++ b/public/app/features/alerting/unified/RuleEditorGrafanaRules.test.tsx @@ -1,51 +1,24 @@ -import { screen, waitForElementToBeRemoved } from '@testing-library/react'; -import userEvent from '@testing-library/user-event'; import * as React from 'react'; import { renderRuleEditor, ui } from 'test/helpers/alertingRuleEditor'; import { clickSelectOption } from 'test/helpers/selectOptionInTest'; +import { screen } from 'test/test-utils'; import { byRole } from 'testing-library-selector'; import { contextSrv } from 'app/core/services/context_srv'; import { mockFeatureDiscoveryApi, setupMswServer } from 'app/features/alerting/unified/mockApi'; -import { DashboardSearchHit, DashboardSearchItemType } from 'app/features/search/types'; import { AccessControlAction } from 'app/types'; -import { searchFolders } from '../../../../app/features/manage-dashboards/state/actions'; - -import { ExpressionEditorProps } from './components/rule-editor/ExpressionEditor'; import { grantUserPermissions, mockDataSource } from './mocks'; -import { grafanaRulerGroup, grafanaRulerRule } from './mocks/grafanaRulerApi'; +import { grafanaRulerGroup } from './mocks/grafanaRulerApi'; import { setupDataSources } from './testSetup/datasources'; import { buildInfoResponse } from './testSetup/featureDiscovery'; -import * as config from './utils/config'; - -jest.mock('./components/rule-editor/ExpressionEditor', () => ({ - ExpressionEditor: ({ value, onChange }: ExpressionEditorProps) => ( - onChange(e.target.value)} /> - ), -})); - -jest.mock('../../../../app/features/manage-dashboards/state/actions'); jest.mock('app/core/components/AppChrome/AppChromeUpdate', () => ({ AppChromeUpdate: ({ actions }: { actions: React.ReactNode }) =>
{actions}
, })); -// there's no angular scope in test and things go terribly wrong when trying to render the query editor row. -// lets just skip it -jest.mock('app/features/query/components/QueryEditorRow', () => ({ - QueryEditorRow: () =>

hi

, -})); - -jest.spyOn(config, 'getAllDataSources'); - jest.setTimeout(60 * 1000); -const mocks = { - getAllDataSources: jest.mocked(config.getAllDataSources), - searchFolders: jest.mocked(searchFolders), -}; - const server = setupMswServer(); describe('RuleEditor grafana managed rules', () => { @@ -83,41 +56,17 @@ describe('RuleEditor grafana managed rules', () => { setupDataSources(dataSources.default); mockFeatureDiscoveryApi(server).discoverDsFeatures(dataSources.default, buildInfoResponse.mimir); - mocks.getAllDataSources.mockReturnValue(Object.values(dataSources)); - mocks.searchFolders.mockResolvedValue([ - { - title: 'Folder A', - uid: grafanaRulerRule.grafana_alert.namespace_uid, - id: 1, - type: DashboardSearchItemType.DashDB, - }, - { - title: 'Folder B', - id: 2, - uid: 'b', - type: DashboardSearchItemType.DashDB, - }, - { - title: 'Folder / with slash', - uid: 'c', - id: 2, - type: DashboardSearchItemType.DashDB, - }, - ] as DashboardSearchHit[]); + const { user } = renderRuleEditor(); - renderRuleEditor(); - await waitForElementToBeRemoved(screen.queryAllByTestId('Spinner')); - - await userEvent.type(await ui.inputs.name.find(), 'my great new rule'); - - const folderInput = await ui.inputs.folder.find(); - await clickSelectOption(folderInput, 'Folder A'); + await user.type(await ui.inputs.name.find(), 'my great new rule'); + await user.click(await screen.findByRole('button', { name: /select folder/i })); + await user.click(await screen.findByLabelText(/folder a/i)); const groupInput = await ui.inputs.group.find(); - await userEvent.click(await byRole('combobox').find(groupInput)); + await user.click(await byRole('combobox').find(groupInput)); await clickSelectOption(groupInput, grafanaRulerGroup.name); - await userEvent.type(ui.inputs.annotationValue(1).get(), 'some description'); + await user.type(ui.inputs.annotationValue(1).get(), 'some description'); - await userEvent.click(ui.buttons.saveAndExit.get()); + await user.click(ui.buttons.saveAndExit.get()); expect(await screen.findByRole('status')).toHaveTextContent('Rule added successfully'); }); diff --git a/public/app/features/alerting/unified/__snapshots__/PanelAlertTabContent.test.tsx.snap b/public/app/features/alerting/unified/__snapshots__/PanelAlertTabContent.test.tsx.snap index bb0d4a46ec3..4c1b0bb8b65 100644 --- a/public/app/features/alerting/unified/__snapshots__/PanelAlertTabContent.test.tsx.snap +++ b/public/app/features/alerting/unified/__snapshots__/PanelAlertTabContent.test.tsx.snap @@ -14,6 +14,7 @@ exports[`PanelAlertTabContent Will render alerts belonging to panel and a button ], "condition": "C", "folder": { + "kind": "folder", "title": "super folder", "uid": "abc", }, diff --git a/public/app/features/alerting/unified/api/alertRuleApi.ts b/public/app/features/alerting/unified/api/alertRuleApi.ts index 84d6d15bad9..ef657ef857a 100644 --- a/public/app/features/alerting/unified/api/alertRuleApi.ts +++ b/public/app/features/alerting/unified/api/alertRuleApi.ts @@ -17,7 +17,7 @@ import { } from 'app/types/unified-alerting-dto'; import { ExportFormats } from '../components/export/providers'; -import { Folder } from '../components/rule-editor/RuleFolderPicker'; +import { Folder } from '../types/rule-form'; import { getDatasourceAPIUid, GRAFANA_RULES_SOURCE_NAME, isGrafanaRulesSource } from '../utils/datasource'; import { arrayKeyValuesToObject } from '../utils/labels'; import { isCloudRuleIdentifier, isPrometheusRuleIdentifier } from '../utils/rules'; diff --git a/public/app/features/alerting/unified/components/rule-editor/FolderAndGroup.tsx b/public/app/features/alerting/unified/components/rule-editor/FolderAndGroup.tsx index 165817ccf3e..aec8020e993 100644 --- a/public/app/features/alerting/unified/components/rule-editor/FolderAndGroup.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/FolderAndGroup.tsx @@ -4,26 +4,26 @@ import * as React from 'react'; import { useCallback, useMemo, useState } from 'react'; import { Controller, FormProvider, useForm, useFormContext } from 'react-hook-form'; -import { AppEvents, GrafanaTheme2, SelectableValue } from '@grafana/data'; +import { GrafanaTheme2, SelectableValue } from '@grafana/data'; import { selectors } from '@grafana/e2e-selectors'; import { AsyncSelect, Box, Button, Field, Input, Label, Modal, Stack, Text, useStyles2 } from '@grafana/ui'; -import appEvents from 'app/core/app_events'; +import { NestedFolderPicker } from 'app/core/components/NestedFolderPicker/NestedFolderPicker'; +import { useAppNotification } from 'app/core/copy/appNotification'; import { t } from 'app/core/internationalization'; import { contextSrv } from 'app/core/services/context_srv'; -import { createFolder } from 'app/features/manage-dashboards/state/actions'; +import { useNewFolderMutation } from 'app/features/browse-dashboards/api/browseDashboardsAPI'; import { AccessControlAction } from 'app/types'; import { RulerRuleGroupDTO, RulerRulesConfigDTO } from 'app/types/unified-alerting-dto'; import { alertRuleApi } from '../../api/alertRuleApi'; import { GRAFANA_RULER_CONFIG } from '../../api/featureDiscoveryApi'; -import { RuleFormValues } from '../../types/rule-form'; +import { Folder, RuleFormValues } from '../../types/rule-form'; import { DEFAULT_GROUP_EVALUATION_INTERVAL } from '../../utils/rule-form'; import { isGrafanaRecordingRuleByType, isGrafanaRulerRule } from '../../utils/rules'; import { ProvisioningBadge } from '../Provisioning'; import { evaluateEveryValidationOptions } from '../rules/EditRuleGroupModal'; import { EvaluationGroupQuickPick } from './EvaluationGroupQuickPick'; -import { containsSlashes, Folder, RuleFolderPicker } from './RuleFolderPicker'; export const MAX_GROUP_RESULTS = 1000; @@ -164,13 +164,18 @@ export function FolderAndGroup({ (
- { - field.onChange({ title, uid }); + value={folder?.uid} + onChange={(uid, title) => { + if (uid && title) { + setValue('folder', { title, uid }); + } else { + setValue('folder', undefined); + } + resetGroup(); }} /> @@ -288,31 +293,27 @@ function FolderCreationModal({ }): React.ReactElement { const styles = useStyles2(getStyles); + const notifyApp = useAppNotification(); const [title, setTitle] = useState(''); + const [createFolder] = useNewFolderMutation(); + const onSubmit = async () => { - const newFolder = await createFolder({ title: title }); - if (!newFolder.uid) { - appEvents.emit(AppEvents.alertError, ['Folder could not be created']); - return; + const { data, error } = await createFolder({ title }); + + if (error) { + notifyApp.error('Failed to create folder'); + } else if (data) { + onCreate({ title: data.title, uid: data.uid }); + notifyApp.success('Folder created'); } - - const folder: Folder = { title: newFolder.title, uid: newFolder.uid }; - onCreate(folder); - appEvents.emit(AppEvents.alertSuccess, ['Folder Created', 'OK']); }; - const error = containsSlashes(title); - return (
Create a new folder to store your rule
- Folder name} - error={"The folder name can't contain slashes"} - invalid={error} - > + Folder name}>