From 2d9d12380c2d6d9e4335285e2d60c9e59e713e82 Mon Sep 17 00:00:00 2001 From: Gilles De Mey Date: Tue, 3 May 2022 11:27:34 +0200 Subject: [PATCH] Alerting: updated alerting creation order (#48548) --- .../components/rule-editor/AlertRuleForm.tsx | 12 +- .../components/rule-editor/AlertTypeStep.tsx | 215 ------------------ ...nsStep.tsx => CloudEvaluationBehavior.tsx} | 4 +- .../components/rule-editor/ConditionField.tsx | 12 +- .../components/rule-editor/DetailsStep.tsx | 125 +++++++++- ...Step.tsx => GrafanaEvaluationBehavior.tsx} | 7 +- .../components/rule-editor/QueryEditor.tsx | 66 ++---- .../AlertType.test.tsx} | 4 +- .../query-and-alert-condition/AlertType.tsx | 116 ++++++++++ .../Query.tsx} | 35 +-- .../QueryAndAlertConditionStep.tsx | 28 +++ 11 files changed, 331 insertions(+), 293 deletions(-) delete mode 100644 public/app/features/alerting/unified/components/rule-editor/AlertTypeStep.tsx rename public/app/features/alerting/unified/components/rule-editor/{CloudConditionsStep.tsx => CloudEvaluationBehavior.tsx} (94%) rename public/app/features/alerting/unified/components/rule-editor/{GrafanaConditionsStep.tsx => GrafanaEvaluationBehavior.tsx} (96%) rename public/app/features/alerting/unified/components/rule-editor/{AlertTypeStep.test.tsx => query-and-alert-condition/AlertType.test.tsx} (96%) create mode 100644 public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/AlertType.tsx rename public/app/features/alerting/unified/components/rule-editor/{QueryStep.tsx => query-and-alert-condition/Query.tsx} (56%) create mode 100644 public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndAlertConditionStep.tsx diff --git a/public/app/features/alerting/unified/components/rule-editor/AlertRuleForm.tsx b/public/app/features/alerting/unified/components/rule-editor/AlertRuleForm.tsx index cd536a9f80b..da479ea0ef8 100644 --- a/public/app/features/alerting/unified/components/rule-editor/AlertRuleForm.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/AlertRuleForm.tsx @@ -18,12 +18,11 @@ import { initialAsyncRequestState } from '../../utils/redux'; import { rulerRuleToFormValues, getDefaultFormValues, getDefaultQueries } from '../../utils/rule-form'; import * as ruleId from '../../utils/rule-id'; -import { AlertTypeStep } from './AlertTypeStep'; -import { CloudConditionsStep } from './CloudConditionsStep'; +import { CloudEvaluationBehavior } from './CloudEvaluationBehavior'; import { DetailsStep } from './DetailsStep'; -import { GrafanaConditionsStep } from './GrafanaConditionsStep'; -import { QueryStep } from './QueryStep'; +import { GrafanaEvaluationBehavior } from './GrafanaEvaluationBehavior'; import { RuleInspector } from './RuleInspector'; +import { QueryAndAlertConditionStep } from './query-and-alert-condition/QueryAndAlertConditionStep'; type Props = { existing?: RuleWithLocation; @@ -151,11 +150,10 @@ export const AlertRuleForm: FC = ({ existing }) => {
- + {showStep2 && ( <> - - {type === RuleFormType.grafana ? : } + {type === RuleFormType.grafana ? : } )} diff --git a/public/app/features/alerting/unified/components/rule-editor/AlertTypeStep.tsx b/public/app/features/alerting/unified/components/rule-editor/AlertTypeStep.tsx deleted file mode 100644 index e1d6963590b..00000000000 --- a/public/app/features/alerting/unified/components/rule-editor/AlertTypeStep.tsx +++ /dev/null @@ -1,215 +0,0 @@ -import { css } from '@emotion/css'; -import React, { FC } from 'react'; -import { useFormContext } from 'react-hook-form'; - -import { DataSourceInstanceSettings, GrafanaTheme2 } from '@grafana/data'; -import { Stack } from '@grafana/experimental'; -import { Field, Icon, Input, InputControl, Label, Tooltip, useStyles2 } from '@grafana/ui'; -import { contextSrv } from 'app/core/services/context_srv'; -import { AccessControlAction } from 'app/types'; - -import { RuleFormType, RuleFormValues } from '../../types/rule-form'; - -import { CloudRulesSourcePicker } from './CloudRulesSourcePicker'; -import { GroupAndNamespaceFields } from './GroupAndNamespaceFields'; -import { RuleEditorSection } from './RuleEditorSection'; -import { Folder, RuleFolderPicker } from './RuleFolderPicker'; -import { RuleTypePicker } from './rule-types/RuleTypePicker'; -import { checkForPathSeparator } from './util'; - -interface Props { - editingExistingRule: boolean; -} - -const recordingRuleNameValidationPattern = { - message: - 'Recording rule name must be valid metric name. It may only contain letters, numbers, and colons. It may not contain whitespace.', - value: /^[a-zA-Z_:][a-zA-Z0-9_:]*$/, -}; - -export const AlertTypeStep: FC = ({ editingExistingRule }) => { - const styles = useStyles2(getStyles); - - const { enabledRuleTypes, defaultRuleType } = getAvailableRuleTypes(); - - const { - register, - control, - watch, - formState: { errors }, - setValue, - getValues, - } = useFormContext(); - - const ruleFormType = watch('type'); - const dataSourceName = watch('dataSourceName'); - - return ( - - {!editingExistingRule && ( - - ( - - )} - name="type" - control={control} - rules={{ - required: { value: true, message: 'Please select alert type' }, - }} - /> - - )} - - - { - // we use the alert rule name as the "groupname" for Grafana managed alerts, so we can't allow path separators - if (ruleFormType === RuleFormType.grafana) { - return checkForPathSeparator(value); - } - - return true; - }, - }, - })} - autoFocus={true} - /> - -
- {(ruleFormType === RuleFormType.cloudRecording || ruleFormType === RuleFormType.cloudAlerting) && ( - - ( - { - // reset location if switching data sources, as different rules source will have different groups and namespaces - setValue('location', undefined); - onChange(ds?.name ?? null); - }} - /> - )} - name="dataSourceName" - control={control} - rules={{ - required: { value: true, message: 'Please select a data source' }, - }} - /> - - )} -
- {(ruleFormType === RuleFormType.cloudRecording || ruleFormType === RuleFormType.cloudAlerting) && - dataSourceName && } - - {ruleFormType === RuleFormType.grafana && ( -
- - - Folder - - Each folder has unique folder permission. When you store multiple rules in a folder, the folder - access permissions get assigned to the rules. -
- } - > - - - - - } - className={styles.formInput} - error={errors.folder?.message} - invalid={!!errors.folder?.message} - data-testid="folder-picker" - > - ( - - )} - name="folder" - rules={{ - required: { value: true, message: 'Please select a folder' }, - validate: { - pathSeparator: (folder: Folder) => checkForPathSeparator(folder.title), - }, - }} - /> - - - - -
- )} - - ); -}; - -function getAvailableRuleTypes() { - const canCreateGrafanaRules = contextSrv.hasPermission(AccessControlAction.AlertingRuleCreate); - const canCreateCloudRules = contextSrv.hasPermission(AccessControlAction.AlertingRuleExternalWrite); - const defaultRuleType = canCreateGrafanaRules ? RuleFormType.grafana : RuleFormType.cloudAlerting; - - const enabledRuleTypes: RuleFormType[] = []; - if (canCreateGrafanaRules) { - enabledRuleTypes.push(RuleFormType.grafana); - } - if (canCreateCloudRules) { - enabledRuleTypes.push(RuleFormType.cloudAlerting, RuleFormType.cloudRecording); - } - - return { enabledRuleTypes, defaultRuleType }; -} - -const getStyles = (theme: GrafanaTheme2) => ({ - formInput: css` - width: 330px; - & + & { - margin-left: ${theme.spacing(3)}; - } - `, - flexRow: css` - display: flex; - flex-direction: row; - justify-content: flex-start; - align-items: flex-end; - `, -}); diff --git a/public/app/features/alerting/unified/components/rule-editor/CloudConditionsStep.tsx b/public/app/features/alerting/unified/components/rule-editor/CloudEvaluationBehavior.tsx similarity index 94% rename from public/app/features/alerting/unified/components/rule-editor/CloudConditionsStep.tsx rename to public/app/features/alerting/unified/components/rule-editor/CloudEvaluationBehavior.tsx index 3323d9b4c15..aba1f9905c1 100644 --- a/public/app/features/alerting/unified/components/rule-editor/CloudConditionsStep.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/CloudEvaluationBehavior.tsx @@ -11,7 +11,7 @@ import { timeOptions } from '../../utils/time'; import { PreviewRule } from './PreviewRule'; import { RuleEditorSection } from './RuleEditorSection'; -export const CloudConditionsStep: FC = () => { +export const CloudEvaluationBehavior: FC = () => { const styles = useStyles(getStyles); const { register, @@ -28,7 +28,7 @@ export const CloudConditionsStep: FC = () => { } return ( - +
diff --git a/public/app/features/alerting/unified/components/rule-editor/ConditionField.tsx b/public/app/features/alerting/unified/components/rule-editor/ConditionField.tsx index 9c45e41bf51..8d78dbe9c2f 100644 --- a/public/app/features/alerting/unified/components/rule-editor/ConditionField.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/ConditionField.tsx @@ -1,3 +1,4 @@ +import { last } from 'lodash'; import React, { FC, useEffect, useMemo } from 'react'; import { useFormContext } from 'react-hook-form'; @@ -31,10 +32,13 @@ export const ConditionField: FC = () => { // reset condition if option no longer exists or if it is unset, but there are options available useEffect(() => { const expressions = queries.filter((query) => query.datasourceUid === ExpressionDatasourceUID); - if (condition && !options.find(({ value }) => value === condition)) { - setValue('condition', expressions.length ? expressions[expressions.length - 1].refId : null); - } else if (!condition && expressions.length) { - setValue('condition', expressions[expressions.length - 1].refId); + const lastExpression = last(expressions); + const conditionExists = options.find(({ value }) => value === condition); + + if (condition && !conditionExists) { + setValue('condition', lastExpression?.refId ?? null); + } else if (!condition && lastExpression) { + setValue('condition', lastExpression.refId, { shouldValidate: true }); } }, [condition, options, queries, setValue]); diff --git a/public/app/features/alerting/unified/components/rule-editor/DetailsStep.tsx b/public/app/features/alerting/unified/components/rule-editor/DetailsStep.tsx index 6cecff78b31..0462b64845f 100644 --- a/public/app/features/alerting/unified/components/rule-editor/DetailsStep.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/DetailsStep.tsx @@ -1,20 +1,42 @@ +import { css } from '@emotion/css'; import React, { FC } from 'react'; import { useFormContext } from 'react-hook-form'; +import { GrafanaTheme2 } from '@grafana/data'; +import { Stack } from '@grafana/experimental'; +import { useStyles2, Field, Input, InputControl, Label, Tooltip, Icon } from '@grafana/ui'; + import { RuleFormType, RuleFormValues } from '../../types/rule-form'; import AnnotationsField from './AnnotationsField'; +import { GroupAndNamespaceFields } from './GroupAndNamespaceFields'; import LabelsField from './LabelsField'; import { RuleEditorSection } from './RuleEditorSection'; +import { RuleFolderPicker, Folder } from './RuleFolderPicker'; +import { checkForPathSeparator } from './util'; + +const recordingRuleNameValidationPattern = { + message: + 'Recording rule name must be valid metric name. It may only contain letters, numbers, and colons. It may not contain whitespace.', + value: /^[a-zA-Z_:][a-zA-Z0-9_:]*$/, +}; export const DetailsStep: FC = () => { - const { watch } = useFormContext(); + const { + register, + watch, + formState: { errors }, + } = useFormContext(); + const styles = useStyles2(getStyles); + + const ruleFormType = watch('type'); + const dataSourceName = watch('dataSourceName'); const type = watch('type'); return ( { : 'Write a summary and add labels to help you better manage your alerts' } > + + { + // we use the alert rule name as the "groupname" for Grafana managed alerts, so we can't allow path separators + if (ruleFormType === RuleFormType.grafana) { + return checkForPathSeparator(value); + } + + return true; + }, + }, + })} + /> + + + {(ruleFormType === RuleFormType.cloudRecording || ruleFormType === RuleFormType.cloudAlerting) && + dataSourceName && } + + {ruleFormType === RuleFormType.grafana && ( +
+ + + Folder + + Each folder has unique folder permission. When you store multiple rules in a folder, the folder + access permissions get assigned to the rules. +
+ } + > + + + + + } + className={styles.formInput} + error={errors.folder?.message} + invalid={!!errors.folder?.message} + data-testid="folder-picker" + > + ( + + )} + name="folder" + rules={{ + required: { value: true, message: 'Please select a folder' }, + validate: { + pathSeparator: (folder: Folder) => checkForPathSeparator(folder.title), + }, + }} + /> + + + + +
+ )} {type !== RuleFormType.cloudRecording && } ); }; + +const getStyles = (theme: GrafanaTheme2) => ({ + formInput: css` + width: 330px; + & + & { + margin-left: ${theme.spacing(3)}; + } + `, + flexRow: css` + display: flex; + flex-direction: row; + justify-content: flex-start; + align-items: flex-end; + `, +}); diff --git a/public/app/features/alerting/unified/components/rule-editor/GrafanaConditionsStep.tsx b/public/app/features/alerting/unified/components/rule-editor/GrafanaEvaluationBehavior.tsx similarity index 96% rename from public/app/features/alerting/unified/components/rule-editor/GrafanaConditionsStep.tsx rename to public/app/features/alerting/unified/components/rule-editor/GrafanaEvaluationBehavior.tsx index f5cd8533662..056c6f7fcb6 100644 --- a/public/app/features/alerting/unified/components/rule-editor/GrafanaConditionsStep.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/GrafanaEvaluationBehavior.tsx @@ -9,7 +9,6 @@ import { RuleFormValues } from '../../types/rule-form'; import { positiveDurationValidationPattern, durationValidationPattern } from '../../utils/time'; import { CollapseToggle } from '../CollapseToggle'; -import { ConditionField } from './ConditionField'; import { GrafanaAlertStatePicker } from './GrafanaAlertStatePicker'; import { GrafanaConditionEvalWarning } from './GrafanaConditionEvalWarning'; import { PreviewRule } from './PreviewRule'; @@ -46,7 +45,7 @@ const evaluateEveryValidationOptions: RegisterOptions = { }, }; -export const GrafanaConditionsStep: FC = () => { +export const GrafanaEvaluationBehavior: FC = () => { const styles = useStyles2(getStyles); const [showErrorHandling, setShowErrorHandling] = useState(false); const { @@ -58,8 +57,8 @@ export const GrafanaConditionsStep: FC = () => { const evaluateForId = 'eval-for-input'; return ( - - + // TODO remove "and alert condition" for recording rules + { ); }; - renderAddQueryRow(styles: ReturnType) { - return ( - - - {config.expressionsEnabled && ( - - )} - - ); - } - isRunning() { const data = Object.values(this.state.panelDataByRefId).find((d) => Boolean(d)); return data?.state === LoadingState.Loading; @@ -145,24 +118,19 @@ export class QueryEditor extends PureComponent { renderRunQueryButton() { const isRunning = this.isRunning(); - const styles = getStyles(config.theme2); if (isRunning) { return ( -
- -
+ ); } return ( -
- -
+ ); } @@ -180,8 +148,23 @@ export class QueryEditor extends PureComponent { onDuplicateQuery={this.onDuplicateQuery} onRunQueries={this.onRunQueries} /> - {this.renderAddQueryRow(styles)} - {this.renderRunQueryButton()} + + + {config.expressionsEnabled && ( + + )} + {this.renderRunQueryButton()} +
); } @@ -227,8 +210,5 @@ const getStyles = stylesFactory((theme: GrafanaTheme2) => { border: 1px solid ${theme.colors.border.medium}; border-radius: ${theme.shape.borderRadius()}; `, - expressionButton: css` - margin-right: ${theme.spacing(0.5)}; - `, }; }); diff --git a/public/app/features/alerting/unified/components/rule-editor/AlertTypeStep.test.tsx b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/AlertType.test.tsx similarity index 96% rename from public/app/features/alerting/unified/components/rule-editor/AlertTypeStep.test.tsx rename to public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/AlertType.test.tsx index 8ddf07be1c1..851e4dc79a8 100644 --- a/public/app/features/alerting/unified/components/rule-editor/AlertTypeStep.test.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/AlertType.test.tsx @@ -8,7 +8,7 @@ import { contextSrv } from 'app/core/services/context_srv'; import { configureStore } from 'app/store/configureStore'; import { AccessControlAction } from 'app/types'; -import { AlertTypeStep } from './AlertTypeStep'; +import { AlertType } from './AlertType'; const ui = { ruleTypePicker: { @@ -28,7 +28,7 @@ function renderAlertTypeStep() { render( - + , { wrapper: FormProviderWrapper } ); diff --git a/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/AlertType.tsx b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/AlertType.tsx new file mode 100644 index 00000000000..f022b78d33a --- /dev/null +++ b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/AlertType.tsx @@ -0,0 +1,116 @@ +import { css } from '@emotion/css'; +import React, { FC } from 'react'; +import { useFormContext } from 'react-hook-form'; + +import { DataSourceInstanceSettings, GrafanaTheme2 } from '@grafana/data'; +import { Field, InputControl, useStyles2 } from '@grafana/ui'; +import { contextSrv } from 'app/core/services/context_srv'; +import { AccessControlAction } from 'app/types'; + +import { RuleFormType, RuleFormValues } from '../../../types/rule-form'; +import { CloudRulesSourcePicker } from '../CloudRulesSourcePicker'; +import { RuleTypePicker } from '../rule-types/RuleTypePicker'; + +interface Props { + editingExistingRule: boolean; +} + +export const AlertType: FC = ({ editingExistingRule }) => { + const { enabledRuleTypes, defaultRuleType } = getAvailableRuleTypes(); + + const { + control, + formState: { errors }, + getValues, + setValue, + watch, + } = useFormContext(); + + const styles = useStyles2(getStyles); + const ruleFormType = watch('type'); + + return ( + <> + {!editingExistingRule && ( + + ( + + )} + name="type" + control={control} + rules={{ + required: { value: true, message: 'Please select alert type' }, + }} + /> + + )} + +
+ {(ruleFormType === RuleFormType.cloudRecording || ruleFormType === RuleFormType.cloudAlerting) && ( + + ( + { + // reset location if switching data sources, as different rules source will have different groups and namespaces + setValue('location', undefined); + onChange(ds?.name ?? null); + }} + /> + )} + name="dataSourceName" + control={control} + rules={{ + required: { value: true, message: 'Please select a data source' }, + }} + /> + + )} +
+ + ); +}; + +function getAvailableRuleTypes() { + const canCreateGrafanaRules = contextSrv.hasPermission(AccessControlAction.AlertingRuleCreate); + const canCreateCloudRules = contextSrv.hasPermission(AccessControlAction.AlertingRuleExternalWrite); + const defaultRuleType = canCreateGrafanaRules ? RuleFormType.grafana : RuleFormType.cloudAlerting; + + const enabledRuleTypes: RuleFormType[] = []; + if (canCreateGrafanaRules) { + enabledRuleTypes.push(RuleFormType.grafana); + } + if (canCreateCloudRules) { + enabledRuleTypes.push(RuleFormType.cloudAlerting, RuleFormType.cloudRecording); + } + + return { enabledRuleTypes, defaultRuleType }; +} + +const getStyles = (theme: GrafanaTheme2) => ({ + formInput: css` + width: 330px; + & + & { + margin-left: ${theme.spacing(3)}; + } + `, + flexRow: css` + display: flex; + flex-direction: row; + justify-content: flex-start; + align-items: flex-end; + `, +}); diff --git a/public/app/features/alerting/unified/components/rule-editor/QueryStep.tsx b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/Query.tsx similarity index 56% rename from public/app/features/alerting/unified/components/rule-editor/QueryStep.tsx rename to public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/Query.tsx index 3f896e9a18e..0e027c81048 100644 --- a/public/app/features/alerting/unified/components/rule-editor/QueryStep.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/Query.tsx @@ -3,31 +3,36 @@ import { useFormContext } from 'react-hook-form'; import { Field, InputControl } from '@grafana/ui'; -import { RuleFormType, RuleFormValues } from '../../types/rule-form'; +import { RuleFormType, RuleFormValues } from '../../../types/rule-form'; +import { ExpressionEditor } from '../ExpressionEditor'; +import { QueryEditor } from '../QueryEditor'; -import { ExpressionEditor } from './ExpressionEditor'; -import { QueryEditor } from './QueryEditor'; -import { RuleEditorSection } from './RuleEditorSection'; - -export const QueryStep: FC = () => { +export const Query: FC = () => { const { control, watch, formState: { errors }, } = useFormContext(); + const type = watch('type'); const dataSourceName = watch('dataSourceName'); + const isGrafanaManagedType = type === RuleFormType.grafana; + const isCloudAlertRuleType = type === RuleFormType.cloudAlerting; + const isRecordingRuleType = type === RuleFormType.cloudRecording; + + const showCloudExpressionEditor = (isRecordingRuleType || isCloudAlertRuleType) && dataSourceName; + return ( - - {(type === RuleFormType.cloudRecording || type === RuleFormType.cloudAlerting) && dataSourceName && ( +
+ {/* This is the PromQL Editor for Cloud rules and recording rules */} + {showCloudExpressionEditor && ( } + render={({ field: { ref, ...field } }) => { + return ; + }} control={control} rules={{ required: { value: true, message: 'A valid expression is required' }, @@ -35,7 +40,9 @@ export const QueryStep: FC = () => { /> )} - {type === RuleFormType.grafana && ( + + {/* This is the editor for Grafana managed rules */} + {isGrafanaManagedType && ( { /> )} - +
); }; diff --git a/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndAlertConditionStep.tsx b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndAlertConditionStep.tsx new file mode 100644 index 00000000000..4e61cda9024 --- /dev/null +++ b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndAlertConditionStep.tsx @@ -0,0 +1,28 @@ +import React, { FC } from 'react'; +import { useFormContext } from 'react-hook-form'; + +import { RuleFormType, RuleFormValues } from '../../../types/rule-form'; +import { ConditionField } from '../ConditionField'; +import { RuleEditorSection } from '../RuleEditorSection'; + +import { AlertType } from './AlertType'; +import { Query } from './Query'; + +interface Props { + editingExistingRule: boolean; +} + +export const QueryAndAlertConditionStep: FC = ({ editingExistingRule }) => { + const { watch } = useFormContext(); + + const type = watch('type'); + const isGrafanaManagedType = type === RuleFormType.grafana; + + return ( + + + {type && } + {isGrafanaManagedType && } + + ); +};