Alerting: don't copy original rule uid when cloning (#70645)

* Prevent using original rule uid when cloning

* Use uid from url instead of a new param in the alert rule form

* Add function to clone rule and tests for it

* Fix lint & tests

---------

Co-authored-by: Sonia Aguilar <soniaaguilarpeiron@gmail.com>

public/app/features/alerting/unified/CloneRuleEditor.test.tsx

@@ -10,12 +10,25 @@ import { config, setBackendSrv, setDataSourceSrv } from '@grafana/runtime';
 import { backendSrv } from 'app/core/services/backend_srv';
 import { AlertManagerCortexConfig } from 'app/plugins/datasource/alertmanager/types';
 import 'whatwg-fetch';
+import { RuleWithLocation } from 'app/types/unified-alerting';
 
-import { RulerGrafanaRuleDTO } from '../../../types/unified-alerting-dto';
+import {
+  RulerAlertingRuleDTO,
+  RulerGrafanaRuleDTO,
+  RulerRecordingRuleDTO,
+  RulerRuleDTO,
+} from '../../../types/unified-alerting-dto';
 
-import { CloneRuleEditor } from './CloneRuleEditor';
+import { cloneRuleDefinition, CloneRuleEditor } from './CloneRuleEditor';
 import { ExpressionEditorProps } from './components/rule-editor/ExpressionEditor';
-import { mockDataSource, MockDataSourceSrv, mockRulerAlertingRule, mockRulerGrafanaRule, mockStore } from './mocks';
+import {
+  mockDataSource,
+  MockDataSourceSrv,
+  mockRulerAlertingRule,
+  mockRulerGrafanaRule,
+  mockRulerRuleGroup,
+  mockStore,
+} from './mocks';
 import { mockAlertmanagerConfigResponse } from './mocks/alertmanagerApi';
 import { mockSearchApiResponse } from './mocks/grafanaApi';
 import { mockRulerRulesApiResponse, mockRulerRulesGroupApiResponse } from './mocks/rulerApi';
@@ -222,4 +235,125 @@ describe('CloneRuleEditor', function () {
       });
     });
   });
+
+  describe('cloneRuleDefinition', () => {
+    it("Should change the cloned rule's name accordingly for Grafana rules", () => {
+      const rule: RulerGrafanaRuleDTO = mockRulerGrafanaRule(
+        {
+          for: '1m',
+          labels: { severity: 'critical', region: 'nasa' },
+          annotations: { [Annotation.summary]: 'This is a very important alert rule' },
+        },
+        { uid: 'grafana-rule-1', title: 'First Grafana Rule', data: [] }
+      );
+
+      const originalRule: RuleWithLocation<RulerGrafanaRuleDTO> = {
+        ruleSourceName: 'my-prom-ds',
+        namespace: 'namespace-one',
+        group: mockRulerRuleGroup(),
+        rule,
+      };
+
+      const clonedRule: RuleWithLocation<RulerRuleDTO> = cloneRuleDefinition(originalRule);
+
+      const grafanaRule: RulerGrafanaRuleDTO = clonedRule.rule as RulerGrafanaRuleDTO;
+
+      expect(originalRule.rule.grafana_alert.title).toEqual('First Grafana Rule');
+      expect(grafanaRule.grafana_alert.title).toEqual('First Grafana Rule (copy)');
+    });
+
+    it("Should change the cloned rule's name accordingly for Ruler rules", () => {
+      const rule: RulerAlertingRuleDTO = mockRulerAlertingRule({
+        for: '1m',
+        alert: 'First Ruler Rule',
+        expr: 'vector(1) > 0',
+        labels: { severity: 'critical', region: 'nasa' },
+        annotations: { [Annotation.summary]: 'This is a very important alert rule' },
+      });
+
+      const originalRule: RuleWithLocation<RulerAlertingRuleDTO> = {
+        ruleSourceName: 'my-prom-ds',
+        namespace: 'namespace-one',
+        group: mockRulerRuleGroup(),
+        rule,
+      };
+
+      const clonedRule: RuleWithLocation<RulerRuleDTO> = cloneRuleDefinition(originalRule);
+
+      const alertingRule: RulerAlertingRuleDTO = clonedRule.rule as RulerAlertingRuleDTO;
+
+      expect(originalRule.rule.alert).toEqual('First Ruler Rule');
+      expect(alertingRule.alert).toEqual('First Ruler Rule (copy)');
+    });
+
+    it("Should change the cloned rule's name accordingly for Recording rules", () => {
+      const rule: RulerRecordingRuleDTO = {
+        record: 'instance:node_num_cpu:sum',
+        expr: 'count without (cpu) (count without (mode) (node_cpu_seconds_total{job="integrations/node_exporter"}))',
+        labels: { type: 'cpu' },
+      };
+
+      const originalRule: RuleWithLocation<RulerRecordingRuleDTO> = {
+        ruleSourceName: 'my-prom-ds',
+        namespace: 'namespace-one',
+        group: mockRulerRuleGroup(),
+        rule,
+      };
+
+      const clonedRule: RuleWithLocation<RulerRuleDTO> = cloneRuleDefinition(originalRule);
+
+      const recordingRule: RulerRecordingRuleDTO = clonedRule.rule as RulerRecordingRuleDTO;
+
+      expect(originalRule.rule.record).toEqual('instance:node_num_cpu:sum');
+      expect(recordingRule.record).toEqual('instance:node_num_cpu:sum (copy)');
+    });
+
+    it('Should remove the group for provisioned Grafana rules', () => {
+      const rule: RulerGrafanaRuleDTO = mockRulerGrafanaRule(
+        {
+          for: '1m',
+          labels: { severity: 'critical', region: 'nasa' },
+          annotations: { [Annotation.summary]: 'This is a very important alert rule' },
+        },
+        { uid: 'grafana-rule-1', title: 'First Grafana Rule', data: [], provenance: 'foo' }
+      );
+
+      const originalRule: RuleWithLocation<RulerGrafanaRuleDTO> = {
+        ruleSourceName: 'my-prom-ds',
+        namespace: 'namespace-one',
+        group: mockRulerRuleGroup(),
+        rule,
+      };
+
+      const clonedRule: RuleWithLocation<RulerRuleDTO> = cloneRuleDefinition(originalRule);
+
+      expect(originalRule.group.name).toEqual('group1');
+      expect(clonedRule.group.name).toEqual('');
+    });
+
+    it('The cloned rule should not contain a UID property', () => {
+      const rule: RulerGrafanaRuleDTO = mockRulerGrafanaRule(
+        {
+          for: '1m',
+          labels: { severity: 'critical', region: 'nasa' },
+          annotations: { [Annotation.summary]: 'This is a very important alert rule' },
+        },
+        { uid: 'grafana-rule-1', title: 'First Grafana Rule', data: [] }
+      );
+
+      const originalRule: RuleWithLocation<RulerGrafanaRuleDTO> = {
+        ruleSourceName: 'my-prom-ds',
+        namespace: 'namespace-one',
+        group: mockRulerRuleGroup(),
+        rule,
+      };
+
+      const clonedRule: RuleWithLocation<RulerRuleDTO> = cloneRuleDefinition(originalRule);
+
+      const grafanaRule: RulerGrafanaRuleDTO = clonedRule.rule as RulerGrafanaRuleDTO;
+
+      expect(originalRule.rule.grafana_alert.uid).toEqual('grafana-rule-1');
+      expect(grafanaRule.grafana_alert.uid).toEqual('');
+    });
+  });
 });

public/app/features/alerting/unified/CloneRuleEditor.tsx

@@ -6,7 +6,7 @@ import { locationService } from '@grafana/runtime/src';
 import { Alert, LoadingPlaceholder } from '@grafana/ui/src';
 
 import { useDispatch } from '../../../types';
-import { RuleIdentifier } from '../../../types/unified-alerting';
+import { RuleIdentifier, RuleWithLocation } from '../../../types/unified-alerting';
 import { RulerRuleDTO } from '../../../types/unified-alerting-dto';
 
 import { AlertRuleForm } from './components/rule-editor/AlertRuleForm';
@@ -30,18 +30,9 @@ export function CloneRuleEditor({ sourceRuleId }: { sourceRuleId: RuleIdentifier
   }
 
   if (rule) {
-    const ruleClone = cloneDeep(rule);
-    changeRuleName(
-      ruleClone.rule,
-      generateCopiedName(getRuleName(ruleClone.rule), ruleClone.group.rules.map(getRuleName))
-    );
+    const ruleClone = cloneRuleDefinition(rule);
     const formPrefill = rulerRuleToFormValues(ruleClone);
 
-    // Provisioned alert rules have provisioned alert group which cannot be used in UI
-    if (isGrafanaRulerRule(rule.rule) && Boolean(rule.rule.grafana_alert.provenance)) {
-      formPrefill.group = '';
-    }
-
     return <AlertRuleForm prefill={formPrefill} />;
   }
 
@@ -74,3 +65,22 @@ function changeRuleName(rule: RulerRuleDTO, newName: string) {
     rule.record = newName;
   }
 }
+
+export function cloneRuleDefinition(rule: RuleWithLocation<RulerRuleDTO>) {
+  const ruleClone = cloneDeep(rule);
+  changeRuleName(
+    ruleClone.rule,
+    generateCopiedName(getRuleName(ruleClone.rule), ruleClone.group.rules.map(getRuleName))
+  );
+
+  if (isGrafanaRulerRule(ruleClone.rule)) {
+    ruleClone.rule.grafana_alert.uid = '';
+
+    // Provisioned alert rules have provisioned alert group which cannot be used in UI
+    if (Boolean(ruleClone.rule.grafana_alert.provenance)) {
+      ruleClone.group = { name: '', rules: ruleClone.group.rules };
+    }
+  }
+
+  return ruleClone;
+}

public/app/features/alerting/unified/ExistingRuleEditor.tsx

@@ -62,5 +62,5 @@ export function ExistingRuleEditor({ identifier, id }: ExistingRuleEditorProps)
     return <AlertWarning title="Cannot edit rule">Sorry! You do not have permission to edit this rule.</AlertWarning>;
   }
 
-  return <AlertRuleForm existing={result} id={id} />;
+  return <AlertRuleForm existing={result} />;
 }

public/app/features/alerting/unified/RuleEditorGrafanaRules.test.tsx

@@ -163,7 +163,6 @@ describe('RuleEditor grafana managed rules', () => {
               is_paused: false,
               no_data_state: 'NoData',
               title: 'my great new rule',
-              uid: '',
             },
           },
         ],

public/app/features/alerting/unified/components/rule-editor/AlertRuleForm.tsx

@@ -72,10 +72,9 @@ const AlertRuleNameInput = () => {
 type Props = {
   existing?: RuleWithLocation;
   prefill?: Partial<RuleFormValues>; // Existing implies we modify existing rule. Prefill only provides default form values
-  id?: string;
 };
 
-export const AlertRuleForm = ({ existing, prefill, id }: Props) => {
+export const AlertRuleForm = ({ existing, prefill }: Props) => {
   const styles = useStyles2(getStyles);
   const dispatch = useDispatch();
   const notifyApp = useAppNotification();
@@ -83,8 +82,9 @@ export const AlertRuleForm = ({ existing, prefill, id }: Props) => {
   const [showEditYaml, setShowEditYaml] = useState(false);
   const [evaluateEvery, setEvaluateEvery] = useState(existing?.group.interval ?? MINUTE);
 
-  const routeParams = useParams<{ type: string }>();
+  const routeParams = useParams<{ type: string; id: string }>();
   const ruleType = translateRouteParamToRuleType(routeParams.type);
+  const uidFromParams = routeParams.id;
 
   const returnTo: string = (queryParams['returnTo'] as string | undefined) ?? '/alerting/list';
   const [showDeleteModal, setShowDeleteModal] = useState<boolean>(false);
@@ -252,7 +252,7 @@ export const AlertRuleForm = ({ existing, prefill, id }: Props) => {
                     <CloudEvaluationBehavior />
                   )}
                   <DetailsStep />
-                  <NotificationsStep alertUid={id} />
+                  <NotificationsStep alertUid={uidFromParams} />
                 </>
               )}
             </div>

public/app/features/alerting/unified/types/rule-form.ts

@@ -11,7 +11,6 @@ export enum RuleFormType {
 export interface RuleFormValues {
   // common
   name: string;
-  uid: string;
   type?: RuleFormType;
   dataSourceName: string | null;
   group: string;

public/app/features/alerting/unified/utils/__snapshots__/rule-form.test.ts.snap

@@ -15,7 +15,6 @@ exports[`formValuesToRulerGrafanaRuleDTO should correctly convert rule form valu
     "is_paused": false,
     "no_data_state": "NoData",
     "title": "",
-    "uid": "",
   },
   "labels": {
     "": "",
@@ -54,7 +53,6 @@ exports[`formValuesToRulerGrafanaRuleDTO should not save both instant and range
     "is_paused": false,
     "no_data_state": "NoData",
     "title": "",
-    "uid": "",
   },
   "labels": {
     "": "",

public/app/features/alerting/unified/utils/rule-form.ts

@@ -131,7 +131,6 @@ export function formValuesToRulerGrafanaRuleDTO(values: RuleFormValues): Postabl
     return {
       grafana_alert: {
         title: name,
-        uid: values.uid,
         condition,
         no_data_state: noDataState,
         exec_err_state: execErrState,
@@ -157,7 +156,6 @@ export function rulerRuleToFormValues(ruleWithLocation: RuleWithLocation): RuleF
       return {
         ...defaultFormValues,
         name: ga.title,
-        uid: ga.uid,
         type: RuleFormType.grafana,
         group: group.name,
         evaluateEvery: group.interval || defaultFormValues.evaluateEvery,