From be3f52abb19b870bea44ed05c1756a8bfb08f898 Mon Sep 17 00:00:00 2001
From: Gilles De Mey <gilles.de.mey@gmail.com>
Date: Wed, 20 Apr 2022 11:41:33 +0200
Subject: [PATCH] Alerting: grafana managed group names (#47785)

---
 .../alerting/unified/RuleEditor.test.tsx      |   7 +-
 .../alerting/unified/RuleList.test.tsx        |   8 +-
 .../unified/components/RuleLocation.tsx       |  21 +++
 .../components/rule-editor/AlertTypeStep.tsx  |  78 ++++++---
 .../rule-editor/GrafanaConditionsStep.tsx     |   6 +-
 .../unified/components/rules/GrafanaRules.tsx |  11 +-
 .../rules/RuleListGroupView.test.tsx          |   6 +-
 .../unified/components/rules/RulesFilter.tsx  |  11 +-
 .../unified/components/rules/RulesGroup.tsx   |   9 +-
 .../unified/components/rules/RulesTable.tsx   |  13 +-
 .../hooks/useCombinedRuleNamespaces.test.ts   |  57 ++++++
 .../hooks/useCombinedRuleNamespaces.ts        |  34 ++--
 .../alerting/unified/state/actions.ts         |   8 +-
 .../alerting/unified/types/rule-form.ts       |   2 +-
 .../alerting/unified/utils/rule-form.ts       |   3 +-
 .../unified/utils/rulerClient.test.ts         |  31 ----
 .../alerting/unified/utils/rulerClient.ts     | 163 ++++++++++--------
 17 files changed, 312 insertions(+), 156 deletions(-)
 create mode 100644 public/app/features/alerting/unified/components/RuleLocation.tsx
 create mode 100644 public/app/features/alerting/unified/hooks/useCombinedRuleNamespaces.test.ts
 delete mode 100644 public/app/features/alerting/unified/utils/rulerClient.test.ts

diff --git a/public/app/features/alerting/unified/RuleEditor.test.tsx b/public/app/features/alerting/unified/RuleEditor.test.tsx
index d257846c641..fc860ea5669 100644
--- a/public/app/features/alerting/unified/RuleEditor.test.tsx
+++ b/public/app/features/alerting/unified/RuleEditor.test.tsx
@@ -1,4 +1,4 @@
-import { Matcher, render, waitFor } from '@testing-library/react';
+import { Matcher, render, waitFor, screen } from '@testing-library/react';
 import { Provider } from 'react-redux';
 import { BackendSrv, locationService, setBackendSrv, setDataSourceSrv } from '@grafana/runtime';
 import { configureStore } from 'app/store/configureStore';
@@ -249,6 +249,9 @@ describe('RuleEditor', () => {
     const folderInput = await ui.inputs.folder.find();
     await clickSelectOption(folderInput, 'Folder A');
 
+    const groupInput = screen.getByRole('textbox', { name: /^Group/ });
+    userEvent.type(groupInput, 'my group');
+
     userEvent.type(ui.inputs.annotationValue(0).get(), 'some summary');
     userEvent.type(ui.inputs.annotationValue(1).get(), 'some description');
 
@@ -268,7 +271,7 @@ describe('RuleEditor', () => {
       'Folder A',
       {
         interval: '1m',
-        name: 'my great new rule',
+        name: 'my group',
         rules: [
           {
             annotations: { description: 'some description', summary: 'some summary' },
diff --git a/public/app/features/alerting/unified/RuleList.test.tsx b/public/app/features/alerting/unified/RuleList.test.tsx
index 1e55be07468..72cdcc1a3a8 100644
--- a/public/app/features/alerting/unified/RuleList.test.tsx
+++ b/public/app/features/alerting/unified/RuleList.test.tsx
@@ -192,10 +192,10 @@ describe('RuleList', () => {
     expect(groups).toHaveLength(5);
 
     expect(groups[0]).toHaveTextContent('foofolder');
-    expect(groups[1]).toHaveTextContent('default > group-1');
-    expect(groups[2]).toHaveTextContent('default > group-1');
-    expect(groups[3]).toHaveTextContent('default > group-2');
-    expect(groups[4]).toHaveTextContent('lokins > group-1');
+    expect(groups[1]).toHaveTextContent('default group-1');
+    expect(groups[2]).toHaveTextContent('default group-1');
+    expect(groups[3]).toHaveTextContent('default group-2');
+    expect(groups[4]).toHaveTextContent('lokins group-1');
 
     const errors = await ui.cloudRulesSourceErrors.find();
 
diff --git a/public/app/features/alerting/unified/components/RuleLocation.tsx b/public/app/features/alerting/unified/components/RuleLocation.tsx
new file mode 100644
index 00000000000..c1b176e231b
--- /dev/null
+++ b/public/app/features/alerting/unified/components/RuleLocation.tsx
@@ -0,0 +1,21 @@
+import { Icon } from '@grafana/ui';
+import React, { FC } from 'react';
+
+interface RuleLocationProps {
+  namespace: string;
+  group?: string;
+}
+
+const RuleLocation: FC<RuleLocationProps> = ({ namespace, group }) => {
+  if (!group) {
+    return <>{namespace}</>;
+  }
+
+  return (
+    <>
+      {namespace} <Icon name="angle-right" /> {group}
+    </>
+  );
+};
+
+export { RuleLocation };
diff --git a/public/app/features/alerting/unified/components/rule-editor/AlertTypeStep.tsx b/public/app/features/alerting/unified/components/rule-editor/AlertTypeStep.tsx
index c0ebdaa73bf..5a926a0a3f0 100644
--- a/public/app/features/alerting/unified/components/rule-editor/AlertTypeStep.tsx
+++ b/public/app/features/alerting/unified/components/rule-editor/AlertTypeStep.tsx
@@ -1,6 +1,6 @@
 import React, { FC } from 'react';
 import { DataSourceInstanceSettings, GrafanaTheme2 } from '@grafana/data';
-import { Field, Input, InputControl, useStyles2 } from '@grafana/ui';
+import { Field, Icon, Input, InputControl, Label, Tooltip, useStyles2 } from '@grafana/ui';
 import { css } from '@emotion/css';
 import { RuleEditorSection } from './RuleEditorSection';
 import { useFormContext } from 'react-hook-form';
@@ -12,6 +12,7 @@ import { checkForPathSeparator } from './util';
 import { RuleTypePicker } from './rule-types/RuleTypePicker';
 import { contextSrv } from 'app/core/services/context_srv';
 import { AccessControlAction } from 'app/types';
+import { Stack } from '@grafana/experimental';
 
 interface Props {
   editingExistingRule: boolean;
@@ -120,26 +121,60 @@ export const AlertTypeStep: FC<Props> = ({ editingExistingRule }) => {
         dataSourceName && <GroupAndNamespaceFields rulesSourceName={dataSourceName} />}
 
       {ruleFormType === RuleFormType.grafana && (
-        <Field
-          label="Folder"
-          className={styles.formInput}
-          error={errors.folder?.message}
-          invalid={!!errors.folder?.message}
-          data-testid="folder-picker"
-        >
-          <InputControl
-            render={({ field: { ref, ...field } }) => (
-              <RuleFolderPicker {...field} enableCreateNew={true} enableReset={true} />
-            )}
-            name="folder"
-            rules={{
-              required: { value: true, message: 'Please select a folder' },
-              validate: {
-                pathSeparator: (folder: Folder) => checkForPathSeparator(folder.title),
-              },
-            }}
-          />
-        </Field>
+        <div className={styles.flexRow}>
+          <Field
+            label={
+              <Label htmlFor="folder" description={'Select a folder to store your rule.'}>
+                <Stack gap={0.5}>
+                  Folder
+                  <Tooltip
+                    placement="top"
+                    content={
+                      <div>
+                        Each folder has unique folder permission. When you store multiple rules in a folder, the folder
+                        access permissions get assigned to the rules.
+                      </div>
+                    }
+                  >
+                    <Icon name="info-circle" size="xs" />
+                  </Tooltip>
+                </Stack>
+              </Label>
+            }
+            className={styles.formInput}
+            error={errors.folder?.message}
+            invalid={!!errors.folder?.message}
+            data-testid="folder-picker"
+          >
+            <InputControl
+              render={({ field: { ref, ...field } }) => (
+                <RuleFolderPicker inputId="folder" {...field} enableCreateNew={true} enableReset={true} />
+              )}
+              name="folder"
+              rules={{
+                required: { value: true, message: 'Please select a folder' },
+                validate: {
+                  pathSeparator: (folder: Folder) => checkForPathSeparator(folder.title),
+                },
+              }}
+            />
+          </Field>
+          <Field
+            label="Group"
+            data-testid="group-picker"
+            description="Rules within the same group are evaluated after the same time interval."
+            className={styles.formInput}
+            error={errors.group?.message}
+            invalid={!!errors.group?.message}
+          >
+            <Input
+              id="group"
+              {...register('group', {
+                required: { value: true, message: 'Must enter a group name' },
+              })}
+            />
+          </Field>
+        </div>
       )}
     </RuleEditorSection>
   );
@@ -172,5 +207,6 @@ const getStyles = (theme: GrafanaTheme2) => ({
     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/GrafanaConditionsStep.tsx
index cbde71df334..2d6b637eddf 100644
--- a/public/app/features/alerting/unified/components/rule-editor/GrafanaConditionsStep.tsx
+++ b/public/app/features/alerting/unified/components/rule-editor/GrafanaConditionsStep.tsx
@@ -67,10 +67,8 @@ export const GrafanaConditionsStep: FC = () => {
             Evaluate every
           </InlineLabel>
           <Field
-            className={styles.inlineField}
-            error={errors.evaluateEvery?.message}
-            invalid={!!errors.evaluateEvery?.message}
-            validationMessageHorizontalOverflow={true}
+            label="Evaluate"
+            description="Evaluation internal applies to every rule within a group. It can overwrite the interval of an existing alert rule."
           >
             <Input id={evaluateEveryId} width={8} {...register('evaluateEvery', evaluateEveryValidationOptions)} />
           </Field>
diff --git a/public/app/features/alerting/unified/components/rules/GrafanaRules.tsx b/public/app/features/alerting/unified/components/rules/GrafanaRules.tsx
index eb4e4262f16..ab37aecab6e 100644
--- a/public/app/features/alerting/unified/components/rules/GrafanaRules.tsx
+++ b/public/app/features/alerting/unified/components/rules/GrafanaRules.tsx
@@ -7,6 +7,8 @@ import { RulesGroup } from './RulesGroup';
 import { GRAFANA_RULES_SOURCE_NAME } from '../../utils/datasource';
 import { CombinedRuleNamespace } from 'app/types/unified-alerting';
 import { initialAsyncRequestState } from '../../utils/redux';
+import { useQueryParams } from 'app/core/hooks/useQueryParams';
+import { flattenGrafanaManagedRules } from '../../hooks/useCombinedRuleNamespaces';
 
 interface Props {
   namespaces: CombinedRuleNamespace[];
@@ -15,10 +17,15 @@ interface Props {
 
 export const GrafanaRules: FC<Props> = ({ namespaces, expandAll }) => {
   const styles = useStyles(getStyles);
+  const [queryParams] = useQueryParams();
+
   const { loading } = useUnifiedAlertingSelector(
     (state) => state.promRules[GRAFANA_RULES_SOURCE_NAME] || initialAsyncRequestState
   );
 
+  const wantsGroupedView = queryParams['view'] === 'grouped';
+  const namespacesFormat = wantsGroupedView ? namespaces : flattenGrafanaManagedRules(namespaces);
+
   return (
     <section className={styles.wrapper}>
       <div className={styles.sectionHeader}>
@@ -26,7 +33,7 @@ export const GrafanaRules: FC<Props> = ({ namespaces, expandAll }) => {
         {loading ? <LoadingPlaceholder className={styles.loader} text="Loading..." /> : <div />}
       </div>
 
-      {namespaces?.map((namespace) =>
+      {namespacesFormat?.map((namespace) =>
         namespace.groups.map((group) => (
           <RulesGroup
             group={group}
@@ -36,7 +43,7 @@ export const GrafanaRules: FC<Props> = ({ namespaces, expandAll }) => {
           />
         ))
       )}
-      {namespaces?.length === 0 && <p>No rules found.</p>}
+      {namespacesFormat?.length === 0 && <p>No rules found.</p>}
     </section>
   );
 };
diff --git a/public/app/features/alerting/unified/components/rules/RuleListGroupView.test.tsx b/public/app/features/alerting/unified/components/rules/RuleListGroupView.test.tsx
index e7d2192e5d8..73a3983712b 100644
--- a/public/app/features/alerting/unified/components/rules/RuleListGroupView.test.tsx
+++ b/public/app/features/alerting/unified/components/rules/RuleListGroupView.test.tsx
@@ -1,3 +1,4 @@
+import { locationService } from '@grafana/runtime';
 import { render } from '@testing-library/react';
 import { contextSrv } from 'app/core/services/context_srv';
 import { configureStore } from 'app/store/configureStore';
@@ -5,6 +6,7 @@ import { AccessControlAction } from 'app/types';
 import { CombinedRuleNamespace } from 'app/types/unified-alerting';
 import React from 'react';
 import { Provider } from 'react-redux';
+import { Router } from 'react-router-dom';
 import { byRole } from 'testing-library-selector';
 import { mockCombinedRule, mockDataSource } from '../../mocks';
 import { GRAFANA_RULES_SOURCE_NAME } from '../../utils/datasource';
@@ -76,7 +78,9 @@ function renderRuleList(namespaces: CombinedRuleNamespace[]) {
 
   render(
     <Provider store={store}>
-      <RuleListGroupView namespaces={namespaces} expandAll />
+      <Router history={locationService.getHistory()}>
+        <RuleListGroupView namespaces={namespaces} expandAll />
+      </Router>
     </Provider>
   );
 }
diff --git a/public/app/features/alerting/unified/components/rules/RulesFilter.tsx b/public/app/features/alerting/unified/components/rules/RulesFilter.tsx
index e68188b4268..f0f38384a31 100644
--- a/public/app/features/alerting/unified/components/rules/RulesFilter.tsx
+++ b/public/app/features/alerting/unified/components/rules/RulesFilter.tsx
@@ -12,10 +12,15 @@ import { alertStateToReadable } from '../../utils/rules';
 import { Stack } from '@grafana/experimental';
 
 const ViewOptions: SelectableValue[] = [
+  {
+    icon: 'list-ul',
+    label: 'List',
+    value: 'list',
+  },
   {
     icon: 'folder',
-    label: 'Groups',
-    value: 'group',
+    label: 'Grouped',
+    value: 'grouped',
   },
   {
     icon: 'heart-rate',
@@ -147,7 +152,7 @@ const RulesFilter = () => {
             <Label>View as</Label>
             <RadioButtonGroup
               options={ViewOptions}
-              value={String(queryParams['view'] || 'group')}
+              value={String(queryParams['view'] ?? ViewOptions[0].value)}
               onChange={handleViewChange}
             />
           </div>
diff --git a/public/app/features/alerting/unified/components/rules/RulesGroup.tsx b/public/app/features/alerting/unified/components/rules/RulesGroup.tsx
index ef668398c0c..b8c0c442d59 100644
--- a/public/app/features/alerting/unified/components/rules/RulesGroup.tsx
+++ b/public/app/features/alerting/unified/components/rules/RulesGroup.tsx
@@ -14,6 +14,7 @@ import { deleteRulesGroupAction } from '../../state/actions';
 import { GRAFANA_RULES_SOURCE_NAME, isCloudRulesSource } from '../../utils/datasource';
 import { isFederatedRuleGroup, isGrafanaRulerRule } from '../../utils/rules';
 import { CollapseToggle } from '../CollapseToggle';
+import { RuleLocation } from '../RuleLocation';
 import { ActionIcon } from './ActionIcon';
 import { EditCloudGroupModal } from './EditCloudGroupModal';
 import { RulesTable } from './RulesTable';
@@ -118,7 +119,13 @@ export const RulesGroup: FC<Props> = React.memo(({ group, namespace, expandAll }
     );
   }
 
-  const groupName = isCloudRulesSource(rulesSource) ? `${namespace.name} > ${group.name}` : namespace.name;
+  // ungrouped rules are rules that are in the "default" group name
+  const isUngrouped = group.name === 'default';
+  const groupName = isUngrouped ? (
+    <RuleLocation namespace={namespace.name} />
+  ) : (
+    <RuleLocation namespace={namespace.name} group={group.name} />
+  );
 
   return (
     <div className={styles.wrapper} data-testid="rule-group">
diff --git a/public/app/features/alerting/unified/components/rules/RulesTable.tsx b/public/app/features/alerting/unified/components/rules/RulesTable.tsx
index 221d238b91d..f638d258fcf 100644
--- a/public/app/features/alerting/unified/components/rules/RulesTable.tsx
+++ b/public/app/features/alerting/unified/components/rules/RulesTable.tsx
@@ -3,7 +3,6 @@ import { useStyles2 } from '@grafana/ui';
 import React, { FC, useMemo } from 'react';
 import { css, cx } from '@emotion/css';
 import { RuleDetails } from './RuleDetails';
-import { isCloudRulesSource } from '../../utils/datasource';
 import { useHasRuler } from '../../hooks/useHasRuler';
 import { CombinedRule } from 'app/types/unified-alerting';
 import { Annotation } from '../../utils/constants';
@@ -11,6 +10,7 @@ import { RuleState } from './RuleState';
 import { RuleHealth } from './RuleHealth';
 import { DynamicTable, DynamicTableColumnProps, DynamicTableItemProps } from '../DynamicTable';
 import { DynamicTableWithGuidelines } from '../DynamicTableWithGuidelines';
+import { RuleLocation } from '../RuleLocation';
 
 type RuleTableColumnProps = DynamicTableColumnProps<CombinedRule>;
 type RuleTableItemProps = DynamicTableItemProps<CombinedRule>;
@@ -137,8 +137,15 @@ function useColumns(showSummaryColumn: boolean, showGroupColumn: boolean) {
         // eslint-disable-next-line react/display-name
         renderCell: ({ data: rule }) => {
           const { namespace, group } = rule;
-          const { rulesSource } = namespace;
-          return isCloudRulesSource(rulesSource) ? `${namespace.name} > ${group.name}` : namespace.name;
+          // ungrouped rules are rules that are in the "default" group name
+          const isUngrouped = group.name === 'default';
+          const groupName = isUngrouped ? (
+            <RuleLocation namespace={namespace.name} />
+          ) : (
+            <RuleLocation namespace={namespace.name} group={group.name} />
+          );
+
+          return groupName;
         },
         size: 5,
       });
diff --git a/public/app/features/alerting/unified/hooks/useCombinedRuleNamespaces.test.ts b/public/app/features/alerting/unified/hooks/useCombinedRuleNamespaces.test.ts
new file mode 100644
index 00000000000..bbd6fb105f6
--- /dev/null
+++ b/public/app/features/alerting/unified/hooks/useCombinedRuleNamespaces.test.ts
@@ -0,0 +1,57 @@
+import { CombinedRuleGroup, CombinedRuleNamespace } from 'app/types/unified-alerting';
+import { sortRulesByName, flattenGrafanaManagedRules } from './useCombinedRuleNamespaces';
+
+describe('flattenGrafanaManagedRules', () => {
+  it('should properly transform grafana managed namespaces', () => {
+    // the rules from both ungrouped groups should go in the default group
+    const ungroupedGroup1 = {
+      name: 'my-rule',
+      rules: [{ name: 'my-rule' }],
+    } as CombinedRuleGroup;
+
+    const ungroupedGroup2 = {
+      name: 'another-rule',
+      rules: [{ name: 'another-rule' }],
+    } as CombinedRuleGroup;
+
+    // the rules from both these groups should go in their own group name
+    const group1 = {
+      name: 'group1',
+      rules: [{ name: 'rule-1' }, { name: 'rule-2' }],
+    } as CombinedRuleGroup;
+
+    const group2 = {
+      name: 'group2',
+      rules: [{ name: 'rule-1' }, { name: 'rule-2' }],
+    } as CombinedRuleGroup;
+
+    const namespace1 = {
+      rulesSource: 'grafana',
+      name: 'ns1',
+      groups: [ungroupedGroup1, ungroupedGroup2, group1, group2],
+    };
+
+    const namespace2 = {
+      rulesSource: 'grafana',
+      name: 'ns2',
+      groups: [ungroupedGroup1],
+    };
+
+    const input = [namespace1, namespace2] as CombinedRuleNamespace[];
+    const [ns1, ns2] = flattenGrafanaManagedRules(input);
+
+    expect(ns1.groups).toEqual([
+      {
+        name: 'default',
+        rules: sortRulesByName([...ungroupedGroup1.rules, ...ungroupedGroup2.rules, ...group1.rules, ...group2.rules]),
+      },
+    ]);
+
+    expect(ns2.groups).toEqual([
+      {
+        name: 'default',
+        rules: ungroupedGroup1.rules,
+      },
+    ]);
+  });
+});
diff --git a/public/app/features/alerting/unified/hooks/useCombinedRuleNamespaces.ts b/public/app/features/alerting/unified/hooks/useCombinedRuleNamespaces.ts
index 259ff2af8c3..8b7afc403f5 100644
--- a/public/app/features/alerting/unified/hooks/useCombinedRuleNamespaces.ts
+++ b/public/app/features/alerting/unified/hooks/useCombinedRuleNamespaces.ts
@@ -81,17 +81,7 @@ export function useCombinedRuleNamespaces(rulesSourceName?: string): CombinedRul
           });
 
           const result = Object.values(namespaces);
-          if (isGrafanaRulesSource(rulesSource)) {
-            // merge all groups in case of grafana managed, essentially treating namespaces (folders) as gorups
-            result.forEach((namespace) => {
-              namespace.groups = [
-                {
-                  name: 'default',
-                  rules: namespace.groups.flatMap((g) => g.rules).sort((a, b) => a.name.localeCompare(b.name)),
-                },
-              ];
-            });
-          }
+
           cache.current[rulesSourceName] = { promRules, rulerRules, result };
           return result;
         })
@@ -100,6 +90,28 @@ export function useCombinedRuleNamespaces(rulesSourceName?: string): CombinedRul
   );
 }
 
+// merge all groups in case of grafana managed, essentially treating namespaces (folders) as groups
+export function flattenGrafanaManagedRules(namespaces: CombinedRuleNamespace[]) {
+  return namespaces.map((namespace) => {
+    const newNamespace: CombinedRuleNamespace = {
+      ...namespace,
+      groups: [],
+    };
+
+    // add default group with ungrouped rules
+    newNamespace.groups.push({
+      name: 'default',
+      rules: sortRulesByName(namespace.groups.flatMap((group) => group.rules)),
+    });
+
+    return newNamespace;
+  });
+}
+
+export function sortRulesByName(rules: CombinedRule[]) {
+  return rules.sort((a, b) => a.name.localeCompare(b.name));
+}
+
 function addRulerGroupsToCombinedNamespace(namespace: CombinedRuleNamespace, groups: RulerRuleGroupDTO[]): void {
   namespace.groups = groups.map((group) => {
     const combinedGroup: CombinedRuleGroup = {
diff --git a/public/app/features/alerting/unified/state/actions.ts b/public/app/features/alerting/unified/state/actions.ts
index 92df3168f92..571debd4e0b 100644
--- a/public/app/features/alerting/unified/state/actions.ts
+++ b/public/app/features/alerting/unified/state/actions.ts
@@ -390,8 +390,14 @@ export const saveRuleFormAction = createAsyncThunk(
           if (redirectOnSave) {
             locationService.push(redirectOnSave);
           } else {
+            // if the identifier comes up empty (this happens when Grafana managed rule moves to another namespace or group)
+            const stringifiedIdentifier = ruleId.stringifyIdentifier(identifier);
+            if (!stringifiedIdentifier) {
+              locationService.push('/alerting/list');
+              return;
+            }
             // redirect to edit page
-            const newLocation = `/alerting/${encodeURIComponent(ruleId.stringifyIdentifier(identifier))}/edit`;
+            const newLocation = `/alerting/${encodeURIComponent(stringifiedIdentifier)}/edit`;
             if (locationService.getLocation().pathname !== newLocation) {
               locationService.replace(newLocation);
             }
diff --git a/public/app/features/alerting/unified/types/rule-form.ts b/public/app/features/alerting/unified/types/rule-form.ts
index 0ae36bb00b0..068dd2d3e2c 100644
--- a/public/app/features/alerting/unified/types/rule-form.ts
+++ b/public/app/features/alerting/unified/types/rule-form.ts
@@ -11,6 +11,7 @@ export interface RuleFormValues {
   name: string;
   type?: RuleFormType;
   dataSourceName: string | null;
+  group: string;
 
   labels: Array<{ key: string; value: string }>;
   annotations: Array<{ key: string; value: string }>;
@@ -26,7 +27,6 @@ export interface RuleFormValues {
 
   // cortex / loki rules
   namespace: string;
-  group: string;
   forTime: number;
   forTimeUnit: string;
   expression: string;
diff --git a/public/app/features/alerting/unified/utils/rule-form.ts b/public/app/features/alerting/unified/utils/rule-form.ts
index c288586f1dd..36ae3b7510f 100644
--- a/public/app/features/alerting/unified/utils/rule-form.ts
+++ b/public/app/features/alerting/unified/utils/rule-form.ts
@@ -45,6 +45,7 @@ export const getDefaultFormValues = (): RuleFormValues => {
     ],
     dataSourceName: null,
     type: canCreateGrafanaRules ? RuleFormType.grafana : canCreateCloudRules ? RuleFormType.cloudAlerting : undefined, // viewers can't create prom alerts
+    group: '',
 
     // grafana
     folder: null,
@@ -56,7 +57,6 @@ export const getDefaultFormValues = (): RuleFormValues => {
     evaluateFor: '5m',
 
     // cortex / loki
-    group: '',
     namespace: '',
     expression: '',
     forTime: 1,
@@ -118,6 +118,7 @@ export function rulerRuleToFormValues(ruleWithLocation: RuleWithLocation): RuleF
         ...defaultFormValues,
         name: ga.title,
         type: RuleFormType.grafana,
+        group: group.name,
         evaluateFor: rule.for || '0',
         evaluateEvery: group.interval || defaultFormValues.evaluateEvery,
         noDataState: ga.no_data_state,
diff --git a/public/app/features/alerting/unified/utils/rulerClient.test.ts b/public/app/features/alerting/unified/utils/rulerClient.test.ts
deleted file mode 100644
index ec5771833cf..00000000000
--- a/public/app/features/alerting/unified/utils/rulerClient.test.ts
+++ /dev/null
@@ -1,31 +0,0 @@
-import { RulerRuleGroupDTO } from 'app/types/unified-alerting-dto';
-import { getUniqueGroupName } from './rulerClient';
-
-describe('getUniqueGroupName', () => {
-  it('Should return the original value when there are no duplicates', () => {
-    // Arrange
-    const originalGroupName = 'file-system-out-of-space';
-    const existingGroups: RulerRuleGroupDTO[] = [];
-
-    // Act
-    const groupName = getUniqueGroupName(originalGroupName, existingGroups);
-
-    // Assert
-    expect(groupName).toBe(originalGroupName);
-  });
-
-  it('Should increment suffix counter until a unique name created', () => {
-    // Arrange
-    const originalGroupName = 'file-system-out-of-space';
-    const existingGroups: RulerRuleGroupDTO[] = [
-      { name: 'file-system-out-of-space', rules: [] },
-      { name: 'file-system-out-of-space-2', rules: [] },
-    ];
-
-    // Act
-    const groupName = getUniqueGroupName(originalGroupName, existingGroups);
-
-    // Assert
-    expect(groupName).toBe('file-system-out-of-space-3');
-  });
-});
diff --git a/public/app/features/alerting/unified/utils/rulerClient.ts b/public/app/features/alerting/unified/utils/rulerClient.ts
index 89eb8e6c286..1d9c512a3dd 100644
--- a/public/app/features/alerting/unified/utils/rulerClient.ts
+++ b/public/app/features/alerting/unified/utils/rulerClient.ts
@@ -1,15 +1,14 @@
 import { RuleIdentifier, RulerDataSourceConfig, RuleWithLocation } from 'app/types/unified-alerting';
-import { PostableRulerRuleGroupDTO, RulerGrafanaRuleDTO, RulerRuleGroupDTO } from 'app/types/unified-alerting-dto';
 import {
-  deleteRulerRulesGroup,
-  fetchRulerRulesGroup,
-  fetchRulerRulesNamespace,
-  fetchRulerRules,
-  setRulerRuleGroup,
-} from '../api/ruler';
+  PostableRuleGrafanaRuleDTO,
+  PostableRulerRuleGroupDTO,
+  RulerGrafanaRuleDTO,
+  RulerRuleGroupDTO,
+} from 'app/types/unified-alerting-dto';
+import { deleteRulerRulesGroup, fetchRulerRulesGroup, fetchRulerRules, setRulerRuleGroup } from '../api/ruler';
 import { RuleFormValues } from '../types/rule-form';
 import * as ruleId from '../utils/rule-id';
-import { GRAFANA_RULES_SOURCE_NAME, isGrafanaRulesSource } from './datasource';
+import { GRAFANA_RULES_SOURCE_NAME } from './datasource';
 import { formValuesToRulerGrafanaRuleDTO, formValuesToRulerRuleDTO } from './rule-form';
 import {
   isCloudRuleIdentifier,
@@ -25,16 +24,6 @@ export interface RulerClient {
   saveGrafanaRule(values: RuleFormValues, existing?: RuleWithLocation): Promise<RuleIdentifier>;
 }
 
-export function getUniqueGroupName(currentGroupName: string, existingGroups: RulerRuleGroupDTO[]) {
-  let newGroupName = currentGroupName;
-  let idx = 1;
-  while (!!existingGroups.find((g) => g.name === newGroupName)) {
-    newGroupName = `${currentGroupName}-${++idx}`;
-  }
-
-  return newGroupName;
-}
-
 export function getRulerClient(rulerConfig: RulerDataSourceConfig): RulerClient {
   const findEditableRule = async (ruleIdentifier: RuleIdentifier): Promise<RuleWithLocation | null> => {
     if (isGrafanaRuleIdentifier(ruleIdentifier)) {
@@ -90,13 +79,8 @@ export function getRulerClient(rulerConfig: RulerDataSourceConfig): RulerClient
   };
 
   const deleteRule = async (ruleWithLocation: RuleWithLocation): Promise<void> => {
-    const { ruleSourceName, namespace, group, rule } = ruleWithLocation;
-    // in case of GRAFANA, each group implicitly only has one rule. delete the group.
-    if (isGrafanaRulesSource(ruleSourceName)) {
-      await deleteRulerRulesGroup(rulerConfig, namespace, group.name);
-      return;
-    }
-    // in case of CLOUD
+    const { namespace, group, rule } = ruleWithLocation;
+
     // it was the last rule, delete the entire group
     if (group.rules.length === 1) {
       await deleteRulerRulesGroup(rulerConfig, namespace, group.name);
@@ -157,65 +141,104 @@ export function getRulerClient(rulerConfig: RulerDataSourceConfig): RulerClient
     }
   };
 
-  const saveGrafanaRule = async (values: RuleFormValues, existing?: RuleWithLocation): Promise<RuleIdentifier> => {
-    const { folder, evaluateEvery } = values;
-    const formRule = formValuesToRulerGrafanaRuleDTO(values);
-
+  const saveGrafanaRule = async (values: RuleFormValues, existingRule?: RuleWithLocation): Promise<RuleIdentifier> => {
+    const { folder, group, evaluateEvery } = values;
     if (!folder) {
       throw new Error('Folder must be specified');
     }
 
-    // updating an existing rule...
-    if (existing) {
-      // refetch it to be sure we have the latest
-      const freshExisting = await findEditableRule(ruleId.fromRuleWithLocation(existing));
-      if (!freshExisting) {
-        throw new Error('Rule not found.');
-      }
+    const newRule = formValuesToRulerGrafanaRuleDTO(values);
+    const namespace = folder.title;
+    const groupSpec = { name: group, interval: evaluateEvery };
 
-      // if same folder, repost the group with updated rule
-      if (freshExisting.namespace === folder.title) {
-        const uid = (freshExisting.rule as RulerGrafanaRuleDTO).grafana_alert.uid!;
-        formRule.grafana_alert.uid = uid;
-        await setRulerRuleGroup(rulerConfig, freshExisting.namespace, {
-          name: freshExisting.group.name,
-          interval: evaluateEvery,
-          rules: [formRule],
-        });
-        return { uid, ruleSourceName: 'grafana' };
-      }
+    if (!existingRule) {
+      return addRuleToNamespaceAndGroup(namespace, groupSpec, newRule);
     }
 
-    // if creating new rule or folder was changed, create rule in a new group
-    const targetFolderGroups = await fetchRulerRulesNamespace(rulerConfig, folder.title);
+    const sameNamespace = existingRule.namespace === namespace;
+    const sameGroup = existingRule.group.name === values.group;
+    const sameLocation = sameNamespace && sameGroup;
 
-    // set group name to rule name, but be super paranoid and check that this group does not already exist
-    const groupName = getUniqueGroupName(values.name, targetFolderGroups);
-    formRule.grafana_alert.title = groupName;
+    if (sameLocation) {
+      // we're update a rule in the same namespace and group
+      return updateGrafanaRule(existingRule, newRule, evaluateEvery);
+    } else {
+      // we're moving a rule to either a different group or namespace
+      return moveGrafanaRule(namespace, groupSpec, existingRule, newRule);
+    }
+  };
+
+  const addRuleToNamespaceAndGroup = async (
+    namespace: string,
+    group: { name: string; interval: string },
+    newRule: PostableRuleGrafanaRuleDTO
+  ): Promise<RuleIdentifier> => {
+    const existingGroup = await fetchRulerRulesGroup(rulerConfig, namespace, group.name);
+    if (!existingGroup) {
+      throw new Error(`No group found with name "${group.name}"`);
+    }
 
     const payload: PostableRulerRuleGroupDTO = {
-      name: groupName,
-      interval: evaluateEvery,
-      rules: [formRule],
+      name: group.name,
+      interval: group.interval,
+      rules: (existingGroup.rules ?? []).concat(newRule as RulerGrafanaRuleDTO),
     };
-    await setRulerRuleGroup(rulerConfig, folder.title, payload);
 
-    // now refetch this group to get the uid, hah
-    const result = await fetchRulerRulesGroup(rulerConfig, folder.title, groupName);
-    const newUid = (result?.rules[0] as RulerGrafanaRuleDTO)?.grafana_alert?.uid;
-    if (newUid) {
-      // if folder has changed, delete the old one
-      if (existing) {
-        const freshExisting = await findEditableRule(ruleId.fromRuleWithLocation(existing));
-        if (freshExisting && freshExisting.namespace !== folder.title) {
-          await deleteRule(freshExisting);
-        }
-      }
+    await setRulerRuleGroup(rulerConfig, namespace, payload);
 
-      return { uid: newUid, ruleSourceName: 'grafana' };
-    } else {
-      throw new Error('Failed to fetch created rule.');
+    return { uid: '', ruleSourceName: GRAFANA_RULES_SOURCE_NAME };
+  };
+
+  // we can't move the rule in a single atomic operation so we have to
+  // 1. add the rule to the new group
+  // 2. remove the rule from the old one
+  const moveGrafanaRule = async (
+    namespace: string,
+    group: { name: string; interval: string },
+    existingRule: RuleWithLocation,
+    newRule: PostableRuleGrafanaRuleDTO
+  ): Promise<RuleIdentifier> => {
+    // add the new rule to the requested namespace and group
+    const identifier = await addRuleToNamespaceAndGroup(namespace, group, newRule);
+
+    // remove the rule from the previous namespace and group
+    await deleteRule({
+      ruleSourceName: existingRule.ruleSourceName,
+      namespace: existingRule.namespace,
+      group: existingRule.group,
+      rule: newRule as RulerGrafanaRuleDTO,
+    });
+
+    return identifier;
+  };
+
+  const updateGrafanaRule = async (
+    existingRule: RuleWithLocation,
+    newRule: PostableRuleGrafanaRuleDTO,
+    interval: string
+  ): Promise<RuleIdentifier> => {
+    // type guard to make sure we're working with a Grafana managed rule
+    if (!isGrafanaRulerRule(existingRule.rule)) {
+      throw new Error('The rule is not a Grafana managed rule');
     }
+
+    // make sure our updated alert has the same UID as before
+    const uid = existingRule.rule.grafana_alert.uid;
+    newRule.grafana_alert.uid = uid;
+
+    // create the new array of rules we want to send to the group
+    const newRules = existingRule.group.rules
+      .filter((rule): rule is RulerGrafanaRuleDTO => isGrafanaRulerRule(rule))
+      .filter((rule) => rule.grafana_alert.uid !== uid)
+      .concat(newRule as RulerGrafanaRuleDTO);
+
+    await setRulerRuleGroup(rulerConfig, existingRule.namespace, {
+      name: existingRule.group.name,
+      interval: interval,
+      rules: newRules,
+    });
+
+    return { uid: '', ruleSourceName: GRAFANA_RULES_SOURCE_NAME };
   };
 
   // Would be nice to somehow align checking of ruler type between different methods