From e5cb1ceae0c6becbd545fe97c4977b0d72d4447c Mon Sep 17 00:00:00 2001 From: Virginia Cepeda Date: Fri, 11 Nov 2022 10:29:59 -0300 Subject: [PATCH] Alerting: Suggest previously entered custom labels (#57783) * [Alerting] - replace label inputs with dropdowns (#57019) * Add AlertLabelDropdown component It will be used to pick from or create new labels * Adapt LabelsField component to use AlertLabelDropdown instead of inputs * Add tests for LabelsField component Plus a few other tests were adapted to work with the label dropdowns * Use ref in component * Fix showing placeholders in the label dropdowns * Minor syntax change * Remove unneeded import after rebase * Display custom labels When a label key is selected, its corresponding values are shown in the dropdown * Add tooltip explaining where labels in the dropdowns come from * Fix import of Stack component * Avoid duplicated values * Improvements based on review * Display labels for currently selected datasource only * Refactor AlertsField to allow to choose whether to suggest labels or not * Suggest labels for NotificationStep and tests * Don't suggest labels in TestContactPointModal * [LabelsField] - refactor: get dataSourceName as a parameter * [LabelsField] - extract common code into reusable components * Display loading spinner while fetching rules * LabelsField - refactor Removing the suggest prop and the default dataSource 'grafana'. Instead, the component now relies on the dataSourceName param. If it's set it means we want to show suggestions so we fetch the labels, otherwise, if not set, we show the plain input texts without suggestions. * Add test for LabelsField without suggestions * Show custom labels for grafana managed alerts When the dataSourceName in the NotificationsStep component has a null value, we can assume it's because we're dealing with grafana managed alerts. In that case we set the correct value. * Fix tests after latest changes Since we removed the combobox from the TestContactPoints modal, tests had to be adjusted * Update texts * initialize all new added inputs with empty data --- .../alerting/unified/RuleEditor.test.tsx | 26 +- .../unified/components/AlertLabelDropdown.tsx | 38 +++ .../rule-editor/LabelsField.test.tsx | 110 ++++++ .../components/rule-editor/LabelsField.tsx | 315 ++++++++++++++---- .../rule-editor/NotificationsStep.tsx | 10 +- 5 files changed, 422 insertions(+), 77 deletions(-) create mode 100644 public/app/features/alerting/unified/components/AlertLabelDropdown.tsx create mode 100644 public/app/features/alerting/unified/components/rule-editor/LabelsField.test.tsx diff --git a/public/app/features/alerting/unified/RuleEditor.test.tsx b/public/app/features/alerting/unified/RuleEditor.test.tsx index 3273427ce06..9854c4b403e 100644 --- a/public/app/features/alerting/unified/RuleEditor.test.tsx +++ b/public/app/features/alerting/unified/RuleEditor.test.tsx @@ -100,6 +100,8 @@ const ui = { }, }; +const getLabelInput = (selector: HTMLElement) => within(selector).getByRole('combobox'); + describe('RuleEditor', () => { beforeEach(() => { jest.clearAllMocks(); @@ -175,10 +177,10 @@ describe('RuleEditor', () => { // TODO remove skipPointerEventsCheck once https://github.com/jsdom/jsdom/issues/3232 is fixed await userEvent.click(ui.buttons.addLabel.get(), { pointerEventsCheck: PointerEventsCheckLevel.Never }); - await userEvent.type(ui.inputs.labelKey(0).get(), 'severity'); - await userEvent.type(ui.inputs.labelValue(0).get(), 'warn'); - await userEvent.type(ui.inputs.labelKey(1).get(), 'team'); - await userEvent.type(ui.inputs.labelValue(1).get(), 'the a-team'); + await userEvent.type(getLabelInput(ui.inputs.labelKey(0).get()), 'severity{enter}'); + await userEvent.type(getLabelInput(ui.inputs.labelValue(0).get()), 'warn{enter}'); + await userEvent.type(getLabelInput(ui.inputs.labelKey(1).get()), 'team{enter}'); + await userEvent.type(getLabelInput(ui.inputs.labelValue(1).get()), 'the a-team{enter}'); // save and check what was sent to backend await userEvent.click(ui.buttons.save.get()); @@ -276,10 +278,10 @@ describe('RuleEditor', () => { // TODO remove skipPointerEventsCheck once https://github.com/jsdom/jsdom/issues/3232 is fixed await userEvent.click(ui.buttons.addLabel.get(), { pointerEventsCheck: PointerEventsCheckLevel.Never }); - await userEvent.type(ui.inputs.labelKey(0).get(), 'severity'); - await userEvent.type(ui.inputs.labelValue(0).get(), 'warn'); - await userEvent.type(ui.inputs.labelKey(1).get(), 'team'); - await userEvent.type(ui.inputs.labelValue(1).get(), 'the a-team'); + await userEvent.type(getLabelInput(ui.inputs.labelKey(0).get()), 'severity{enter}'); + await userEvent.type(getLabelInput(ui.inputs.labelValue(0).get()), 'warn{enter}'); + await userEvent.type(getLabelInput(ui.inputs.labelKey(1).get()), 'team{enter}'); + await userEvent.type(getLabelInput(ui.inputs.labelValue(1).get()), 'the a-team{enter}'); // save and check what was sent to backend await userEvent.click(ui.buttons.save.get()); @@ -370,8 +372,8 @@ describe('RuleEditor', () => { // TODO remove skipPointerEventsCheck once https://github.com/jsdom/jsdom/issues/3232 is fixed await userEvent.click(ui.buttons.addLabel.get(), { pointerEventsCheck: PointerEventsCheckLevel.Never }); - await userEvent.type(ui.inputs.labelKey(1).get(), 'team'); - await userEvent.type(ui.inputs.labelValue(1).get(), 'the a-team'); + await userEvent.type(getLabelInput(ui.inputs.labelKey(1).get()), 'team{enter}'); + await userEvent.type(getLabelInput(ui.inputs.labelValue(1).get()), 'the a-team{enter}'); // try to save, find out that recording rule name is invalid await userEvent.click(ui.buttons.save.get()); @@ -502,8 +504,8 @@ describe('RuleEditor', () => { await userEvent.type(ui.inputs.annotationValue(2).get(), 'value'); //add a label - await userEvent.type(ui.inputs.labelKey(2).get(), 'custom'); - await userEvent.type(ui.inputs.labelValue(2).get(), 'value'); + await userEvent.type(getLabelInput(ui.inputs.labelKey(2).get()), 'custom{enter}'); + await userEvent.type(getLabelInput(ui.inputs.labelValue(2).get()), 'value{enter}'); // save and check what was sent to backend await userEvent.click(ui.buttons.save.get()); diff --git a/public/app/features/alerting/unified/components/AlertLabelDropdown.tsx b/public/app/features/alerting/unified/components/AlertLabelDropdown.tsx new file mode 100644 index 00000000000..21d369b5672 --- /dev/null +++ b/public/app/features/alerting/unified/components/AlertLabelDropdown.tsx @@ -0,0 +1,38 @@ +import React, { FC } from 'react'; + +import { SelectableValue } from '@grafana/data'; +import { Select, Field } from '@grafana/ui'; + +export interface AlertLabelDropdownProps { + onChange: (newValue: SelectableValue) => void; + onOpenMenu?: () => void; + options: SelectableValue[]; + defaultValue?: SelectableValue; + type: 'key' | 'value'; +} + +const AlertLabelDropdown: FC = React.forwardRef( + function labelPicker({ onChange, options, defaultValue, type, onOpenMenu = () => {} }, ref) { + return ( +
+ + + + = + + + + +
+ + ); + })} + + + ); +}; + +const LabelsField: FC = ({ className, dataSourceName }) => { + const styles = useStyles2(getStyles); return (
- +
+ } + > + + + + <>
Labels
- {fields.map((field, index) => { - return ( -
-
- - - - = - - - -
-
- ); - })} - + {dataSourceName && } + {!dataSourceName && }
@@ -96,6 +280,9 @@ const LabelsField: FC = ({ className }) => { const getStyles = (theme: GrafanaTheme2) => { return { + icon: css` + margin-right: ${theme.spacing(0.5)}; + `, wrapper: css` margin-bottom: ${theme.spacing(4)}; `, diff --git a/public/app/features/alerting/unified/components/rule-editor/NotificationsStep.tsx b/public/app/features/alerting/unified/components/rule-editor/NotificationsStep.tsx index 288675efdc0..8c9cedac09c 100644 --- a/public/app/features/alerting/unified/components/rule-editor/NotificationsStep.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/NotificationsStep.tsx @@ -1,9 +1,13 @@ import { css } from '@emotion/css'; import React, { useState } from 'react'; +import { useFormContext } from 'react-hook-form'; import { GrafanaTheme2 } from '@grafana/data'; import { Card, Link, useStyles2, useTheme2 } from '@grafana/ui'; +import { RuleFormValues } from '../../types/rule-form'; +import { GRAFANA_RULES_SOURCE_NAME } from '../../utils/datasource'; + import LabelsField from './LabelsField'; import { RuleEditorSection } from './RuleEditorSection'; @@ -12,6 +16,10 @@ export const NotificationsStep = () => { const styles = useStyles2(getStyles); const theme = useTheme2(); + const { watch } = useFormContext(); + + const dataSourceName = watch('dataSourceName') ?? GRAFANA_RULES_SOURCE_NAME; + return ( { /> )}
- + Root route – default for all alerts