From 19b239fba099d852fd1fbc5f9680f9f224ad0c4f Mon Sep 17 00:00:00 2001
From: Sonia Aguilar <33540275+soniaAguilarPeiron@users.noreply.github.com>
Date: Tue, 25 Jul 2023 22:34:14 +0200
Subject: [PATCH] Alerting: Fix inconsistencies in alert rule form depending on
 alert type (#72287)

* Fix inconsistencies in alert rule form depending on alert type

* Fix default annotations when comming from dashboard panel

* Update texts following pr review comments

* Fix texts

---------

Co-authored-by: Virginia Cepeda <virginia.cepeda@grafana.com>
---
 .../components/rule-editor/AlertRuleForm.tsx        | 11 +++++++++--
 .../rule-editor/CloudEvaluationBehavior.tsx         | 13 +++++++++++--
 .../unified/components/rule-editor/DetailsStep.tsx  |  7 ++++---
 .../components/rule-editor/FolderAndGroup.tsx       |  2 +-
 .../rule-editor/GrafanaEvaluationBehavior.tsx       |  2 +-
 .../components/rule-editor/NotificationsStep.tsx    |  2 +-
 .../QueryAndExpressionsStep.tsx                     |  9 ++++++---
 .../features/alerting/unified/utils/constants.ts    |  6 +++---
 .../features/alerting/unified/utils/rule-form.ts    |  5 +++--
 9 files changed, 39 insertions(+), 18 deletions(-)

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 e8715f33eb0..038a776fb35 100644
--- a/public/app/features/alerting/unified/components/rule-editor/AlertRuleForm.tsx
+++ b/public/app/features/alerting/unified/components/rule-editor/AlertRuleForm.tsx
@@ -21,7 +21,13 @@ import { useUnifiedAlertingSelector } from '../../hooks/useUnifiedAlertingSelect
 import { deleteRuleAction, saveRuleFormAction } from '../../state/actions';
 import { RuleFormType, RuleFormValues } from '../../types/rule-form';
 import { initialAsyncRequestState } from '../../utils/redux';
-import { getDefaultFormValues, getDefaultQueries, MINUTE, rulerRuleToFormValues } from '../../utils/rule-form';
+import {
+  getDefaultFormValues,
+  getDefaultQueries,
+  MINUTE,
+  normalizeDefaultAnnotations,
+  rulerRuleToFormValues,
+} from '../../utils/rule-form';
 import * as ruleId from '../../utils/rule-id';
 
 import { CloudEvaluationBehavior } from './CloudEvaluationBehavior';
@@ -50,7 +56,7 @@ const AlertRuleNameInput = () => {
 
   const ruleFormType = watch('type');
   return (
-    <RuleEditorSection stepNo={1} title="Set alert rule name">
+    <RuleEditorSection stepNo={1} title="Set alert rule name.">
       <Field
         className={styles.formInput}
         label="Rule name"
@@ -323,6 +329,7 @@ function formValuesFromQueryParams(ruleDefinition: string, type: RuleFormType):
   return ignoreHiddenQueries({
     ...getDefaultFormValues(),
     ...ruleFromQueryParams,
+    annotations: normalizeDefaultAnnotations(ruleFromQueryParams.annotations ?? []),
     queries: ruleFromQueryParams.queries ?? getDefaultQueries(),
     type: type || RuleFormType.grafana,
     evaluateEvery: MINUTE,
diff --git a/public/app/features/alerting/unified/components/rule-editor/CloudEvaluationBehavior.tsx b/public/app/features/alerting/unified/components/rule-editor/CloudEvaluationBehavior.tsx
index c74eff45c59..07b8c0bd3ae 100644
--- a/public/app/features/alerting/unified/components/rule-editor/CloudEvaluationBehavior.tsx
+++ b/public/app/features/alerting/unified/components/rule-editor/CloudEvaluationBehavior.tsx
@@ -8,6 +8,7 @@ import { Field, Input, InputControl, Select, useStyles2 } from '@grafana/ui';
 import { RuleFormType, RuleFormValues } from '../../types/rule-form';
 import { timeOptions } from '../../utils/time';
 
+import { GroupAndNamespaceFields } from './GroupAndNamespaceFields';
 import { PreviewRule } from './PreviewRule';
 import { RuleEditorSection } from './RuleEditorSection';
 
@@ -21,6 +22,7 @@ export const CloudEvaluationBehavior = () => {
   } = useFormContext<RuleFormValues>();
 
   const type = watch('type');
+  const dataSourceName = watch('dataSourceName');
 
   // cloud recording rules do not have alert conditions
   if (type === RuleFormType.cloudRecording) {
@@ -28,8 +30,11 @@ export const CloudEvaluationBehavior = () => {
   }
 
   return (
-    <RuleEditorSection stepNo={3} title="Alert evaluation behavior">
-      <Field label="For" description="Expression has to be true for this long for the alert to be fired.">
+    <RuleEditorSection stepNo={3} title="Set alert evaluation behavior">
+      <Field
+        label="Pending period"
+        description="Period in which an alert rule can be in breach of the condition until the alert rule fires."
+      >
         <div className={styles.flexRow}>
           <Field invalid={!!errors.forTime?.message} error={errors.forTime?.message} className={styles.inlineField}>
             <Input
@@ -52,6 +57,10 @@ export const CloudEvaluationBehavior = () => {
           />
         </div>
       </Field>
+      {type === RuleFormType.cloudAlerting && dataSourceName && (
+        <GroupAndNamespaceFields rulesSourceName={dataSourceName} />
+      )}
+
       <PreviewRule />
     </RuleEditorSection>
   );
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 ad777757ee6..be14d996704 100644
--- a/public/app/features/alerting/unified/components/rule-editor/DetailsStep.tsx
+++ b/public/app/features/alerting/unified/components/rule-editor/DetailsStep.tsx
@@ -56,11 +56,12 @@ export function DetailsStep() {
   return (
     <RuleEditorSection
       stepNo={type === RuleFormType.cloudRecording ? 3 : 4}
-      title={type === RuleFormType.cloudRecording ? 'Folder and group' : 'Add annotations'}
+      title={type === RuleFormType.cloudRecording ? 'Add namespace and group' : 'Add annotations'}
       description={getDescription(type, styles)}
     >
-      {(ruleFormType === RuleFormType.cloudRecording || ruleFormType === RuleFormType.cloudAlerting) &&
-        dataSourceName && <GroupAndNamespaceFields rulesSourceName={dataSourceName} />}
+      {ruleFormType === RuleFormType.cloudRecording && dataSourceName && (
+        <GroupAndNamespaceFields rulesSourceName={dataSourceName} />
+      )}
 
       {type !== RuleFormType.cloudRecording && <AnnotationsField />}
     </RuleEditorSection>
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 64bb430b056..4b88fed73e9 100644
--- a/public/app/features/alerting/unified/components/rule-editor/FolderAndGroup.tsx
+++ b/public/app/features/alerting/unified/components/rule-editor/FolderAndGroup.tsx
@@ -182,7 +182,7 @@ export function FolderAndGroup({ groupfoldersForGrafana }: { groupfoldersForGraf
         <Field
           label="Evaluation group"
           data-testid="group-picker"
-          description="Rules within the same group are evaluated sequentially over the same time interval"
+          description="Rules within the same group are evaluated sequentially over the same time interval."
           className={styles.formInput}
           error={errors.group?.message}
           invalid={!!errors.group?.message}
diff --git a/public/app/features/alerting/unified/components/rule-editor/GrafanaEvaluationBehavior.tsx b/public/app/features/alerting/unified/components/rule-editor/GrafanaEvaluationBehavior.tsx
index 2870a696e8d..0d40e3de5b3 100644
--- a/public/app/features/alerting/unified/components/rule-editor/GrafanaEvaluationBehavior.tsx
+++ b/public/app/features/alerting/unified/components/rule-editor/GrafanaEvaluationBehavior.tsx
@@ -168,7 +168,7 @@ function ForInput({ evaluateEvery }: { evaluateEvery: string }) {
         label={
           <Label
             htmlFor="evaluateFor"
-            description="Period in which an alert rule can be in breach of the condition until the alert rule fires"
+            description="Period in which an alert rule can be in breach of the condition until the alert rule fires."
           >
             Pending period
           </Label>
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 0b0c6780189..f3c4cf41984 100644
--- a/public/app/features/alerting/unified/components/rule-editor/NotificationsStep.tsx
+++ b/public/app/features/alerting/unified/components/rule-editor/NotificationsStep.tsx
@@ -86,7 +86,7 @@ export const NotificationsStep = ({ alertUid }: NotificationsStepProps) => {
   return (
     <RuleEditorSection
       stepNo={type === RuleFormType.cloudRecording ? 4 : 5}
-      title={type === RuleFormType.cloudRecording ? 'Labels' : 'Configure notifications'}
+      title={type === RuleFormType.cloudRecording ? 'Add labels' : 'Configure notifications'}
       description={
         type === RuleFormType.cloudRecording ? (
           'Add labels to help you better manage your recording rules'
diff --git a/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndExpressionsStep.tsx b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndExpressionsStep.tsx
index 997e2dccb8d..294438609da 100644
--- a/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndExpressionsStep.tsx
+++ b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndExpressionsStep.tsx
@@ -25,7 +25,7 @@ import { NeedHelpInfo } from '../NeedHelpInfo';
 import { QueryEditor } from '../QueryEditor';
 import { RecordingRuleEditor } from '../RecordingRuleEditor';
 import { RuleEditorSection } from '../RuleEditorSection';
-import { errorFromSeries, refIdExists, findRenamedDataQueryReferences } from '../util';
+import { errorFromSeries, findRenamedDataQueryReferences, refIdExists } from '../util';
 
 import { CloudDataSourceSelector } from './CloudDataSourceSelector';
 import { SmartAlertTypeDetector } from './SmartAlertTypeDetector';
@@ -334,7 +334,10 @@ export const QueryAndExpressionsStep = ({ editingExistingRule, onDataChange }: P
   ]);
 
   return (
-    <RuleEditorSection stepNo={2} title="Define query and alert condition">
+    <RuleEditorSection
+      stepNo={2}
+      title={type !== RuleFormType.cloudRecording ? 'Define query and alert condition' : 'Define query'}
+    >
       {/* This is the cloud data source selector */}
       {(type === RuleFormType.cloudRecording || type === RuleFormType.cloudAlerting) && (
         <CloudDataSourceSelector onChangeCloudDatasource={onChangeCloudDatasource} />
@@ -436,7 +439,7 @@ export const QueryAndExpressionsStep = ({ editingExistingRule, onDataChange }: P
           />
           {/* Expression Queries */}
           <Text element="h5">Expressions</Text>
-          <div className={styles.mutedText}>Manipulate data returned from queries with math and other operations</div>
+          <div className={styles.mutedText}>Manipulate data returned from queries with math and other operations.</div>
           <ExpressionsEditor
             queries={queries}
             panelData={queryPreviewData}
diff --git a/public/app/features/alerting/unified/utils/constants.ts b/public/app/features/alerting/unified/utils/constants.ts
index 0e4160965f9..b78593480d4 100644
--- a/public/app/features/alerting/unified/utils/constants.ts
+++ b/public/app/features/alerting/unified/utils/constants.ts
@@ -31,9 +31,9 @@ export const annotationLabels: Record<Annotation, string> = {
 };
 
 export const annotationDescriptions: Record<Annotation, string> = {
-  [Annotation.description]: 'Description of what the alert rule does',
-  [Annotation.summary]: 'Short summary of what happened and why',
-  [Annotation.runbookURL]: 'Webpage where you keep your runbook for the alert',
+  [Annotation.description]: 'Description of what the alert rule does.',
+  [Annotation.summary]: 'Short summary of what happened and why.',
+  [Annotation.runbookURL]: 'Webpage where you keep your runbook for the alert.',
   [Annotation.dashboardUID]: '',
   [Annotation.panelID]: '',
   [Annotation.alertId]: '',
diff --git a/public/app/features/alerting/unified/utils/rule-form.ts b/public/app/features/alerting/unified/utils/rule-form.ts
index bb5f3af1ad6..63eb52d6a68 100644
--- a/public/app/features/alerting/unified/utils/rule-form.ts
+++ b/public/app/features/alerting/unified/utils/rule-form.ts
@@ -94,7 +94,7 @@ export function formValuesToRulerRuleDTO(values: RuleFormValues): RulerRuleDTO {
   throw new Error(`unexpected rule type: ${type}`);
 }
 
-function listifyLabelsOrAnnotations(
+export function listifyLabelsOrAnnotations(
   item: Labels | Annotations | undefined,
   addEmpty: boolean
 ): Array<{ key: string; value: string }> {
@@ -106,7 +106,7 @@ function listifyLabelsOrAnnotations(
 }
 
 //make sure default annotations are always shown in order even if empty
-function normalizeDefaultAnnotations(annotations: Array<{ key: string; value: string }>) {
+export function normalizeDefaultAnnotations(annotations: Array<{ key: string; value: string }>) {
   const orderedAnnotations = [...annotations];
   const defaultAnnotationKeys = defaultAnnotations.map((annotation) => annotation.key);
 
@@ -179,6 +179,7 @@ export function rulerRuleToFormValues(ruleWithLocation: RuleWithLocation): RuleF
       return {
         ...defaultFormValues,
         ...alertingRuleValues,
+        annotations: normalizeDefaultAnnotations(listifyLabelsOrAnnotations(rule.annotations, false)),
         type: RuleFormType.cloudAlerting,
         dataSourceName: ruleSourceName,
         namespace,