Alerting: Fix rules deleting when reordering whilst filtered (#88221)

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

@@ -1,9 +1,9 @@
 import { SerializedError } from '@reduxjs/toolkit';
-import { prettyDOM, render, screen, waitFor } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import { setupServer } from 'msw/node';
 import React from 'react';
 import { TestProvider } from 'test/helpers/TestProvider';
+import { prettyDOM, render, screen, waitFor, within } from 'test/test-utils';
 import { byRole, byTestId, byText } from 'testing-library-selector';
 
 import { PluginExtensionTypes, PluginMeta } from '@grafana/data';
@@ -632,6 +632,63 @@ describe('RuleList', () => {
     await waitFor(() => expect(ui.ruleGroup.get()).toHaveTextContent('group-2'));
   });
 
+  it('uses entire group when reordering after filtering', async () => {
+    const user = userEvent.setup();
+
+    mocks.getAllDataSourcesMock.mockReturnValue([dataSources.prom]);
+
+    setDataSourceSrv(new MockDataSourceSrv({ prom: dataSources.prom }));
+
+    mocks.api.discoverFeatures.mockResolvedValue({
+      application: PromApplication.Cortex,
+      features: {
+        rulerApiEnabled: true,
+      },
+    });
+
+    mocks.api.fetchRulerRules.mockImplementation(() => Promise.resolve(someRulerRules));
+    mocks.api.fetchRules.mockImplementation((dataSourceName: string) => {
+      if (dataSourceName === GRAFANA_RULES_SOURCE_NAME) {
+        return Promise.resolve([
+          mockPromRuleNamespace({
+            name: 'foofolder',
+            dataSourceName: GRAFANA_RULES_SOURCE_NAME,
+            groups: [
+              mockPromRuleGroup({
+                name: 'grafana-group',
+                rules: [
+                  mockPromAlertingRule({
+                    query: '[]',
+                  }),
+                ],
+              }),
+            ],
+          }),
+        ]);
+      } else {
+        return Promise.resolve([]);
+      }
+    });
+
+    renderRuleList();
+
+    const [firstReorderButton] = await screen.findAllByLabelText(/reorder/i);
+
+    const filterInput = ui.rulesFilterInput.get();
+    await userEvent.type(filterInput, 'alert1a{Enter}');
+
+    await user.click(firstReorderButton);
+
+    const reorderDialog = await screen.findByRole('dialog');
+
+    const alertsInReorder = within(reorderDialog).getAllByTestId('reorder-alert-rule');
+
+    // We've filtered down to one rule, but the reorder dialog should still
+    // have everything in the group visible for reordering
+    // If this were not the case, rules could be deleted ⚠️
+    expect(alertsInReorder).toHaveLength(2);
+  });
+
   describe('pausing rules', () => {
     beforeEach(() => {
       grantUserPermissions([

public/app/features/alerting/unified/components/rules/ReorderRuleGroupModal.tsx

@@ -13,6 +13,7 @@ import {
 
 import { GrafanaTheme2 } from '@grafana/data';
 import { Badge, Icon, Modal, Tooltip, useStyles2 } from '@grafana/ui';
+import { useCombinedRuleNamespaces } from 'app/features/alerting/unified/hooks/useCombinedRuleNamespaces';
 import { dispatch } from 'app/store/store';
 import { CombinedRule, CombinedRuleGroup, CombinedRuleNamespace } from 'app/types/unified-alerting';
 
@@ -34,8 +35,18 @@ type CombinedRuleWithUID = { uid: string } & CombinedRule;
 
 export const ReorderCloudGroupModal = (props: ModalProps) => {
   const { group, namespace, onClose, folderUid } = props;
+
+  // The list of rules might have been filtered before we get to this reordering modal
+  // We need to grab the full (unfiltered) list so we are able to reorder via the API without
+  // deleting any rules (as they otherwise would have been omitted from the payload)
+  const unfilteredNamespaces = useCombinedRuleNamespaces();
+  const matchedNamespace = unfilteredNamespaces.find(
+    (ns) => ns.rulesSource === namespace.rulesSource && ns.name === namespace.name
+  );
+  const matchedGroup = matchedNamespace?.groups.find((g) => g.name === group.name);
+
   const [pending, setPending] = useState<boolean>(false);
-  const [rulesList, setRulesList] = useState<CombinedRule[]>(group.rules);
+  const [rulesList, setRulesList] = useState<CombinedRule[]>(matchedGroup?.rules || []);
 
   const styles = useStyles2(getStyles);
 
@@ -129,6 +140,7 @@ const ListItem = ({ provided, rule, isClone = false, isDragging = false }: ListI
 
   return (
     <div
+      data-testid="reorder-alert-rule"
       className={cx(styles.listItem, isClone && 'isClone', isDragging && 'isDragging')}
       ref={provided.innerRef}
       {...provided.draggableProps}

public/app/features/alerting/unified/components/rules/RulesGroup.tsx

@@ -105,7 +105,6 @@ export const RulesGroup = React.memo(({ group, namespace, expandAll, viewMode }:
           );
           actionIcons.push(
             <ActionIcon
-              aria-label="re-order rules"
               data-testid="reorder-group"
               key="reorder"
               icon="exchange-alt"
@@ -181,11 +180,10 @@ export const RulesGroup = React.memo(({ group, namespace, expandAll, viewMode }:
       );
       actionIcons.push(
         <ActionIcon
-          aria-label="re-order rules"
           data-testid="reorder-group"
           key="reorder"
           icon="exchange-alt"
-          tooltip="re-order rules"
+          tooltip="reorder rules"
           className={styles.rotate90}
           onClick={() => setIsReorderingGroup(true)}
         />

public/app/features/alerting/unified/mocks.ts

@@ -561,7 +561,10 @@ export const somePromRules = (dataSourceName = 'Prometheus'): RuleNamespace[] =>
 
 export const someRulerRules: RulerRulesConfigDTO = {
   namespace1: [
-    mockRulerRuleGroup({ name: 'group1', rules: [mockRulerAlertingRule({ alert: 'alert1' })] }),
+    mockRulerRuleGroup({
+      name: 'group1',
+      rules: [mockRulerAlertingRule({ alert: 'alert1' }), mockRulerAlertingRule({ alert: 'alert1a' })],
+    }),
     mockRulerRuleGroup({ name: 'group2', rules: [mockRulerAlertingRule({ alert: 'alert2' })] }),
   ],
   namespace2: [mockRulerRuleGroup({ name: 'group3', rules: [mockRulerAlertingRule({ alert: 'alert3' })] })],

public/app/features/alerting/unified/state/actions.ts

@@ -30,6 +30,7 @@ import {
 
 import { backendSrv } from '../../../../core/services/backend_srv';
 import {
+  logError,
   logInfo,
   LogMessages,
   trackSwitchToPoliciesRouting,
@@ -852,6 +853,16 @@ export const updateRulesOrder = createAsyncThunk(
             throw new Error(`Group "${groupName}" not found.`);
           }
 
+          // We're unlikely to have this happen, as any user of this action should have already ensured
+          // that the entire group was fetched before sending a new order.
+          // But as a final safeguard we should fail if we somehow ended up here with a mismatched rules count
+          // This would indicate an accidental deletion of rules following a frontend bug
+          if (existingGroup.rules.length !== newRules.length) {
+            const err = new Error('Rules count mismatch. Please refresh the page and try again.');
+            logError(err, { namespaceName, groupName });
+            throw err;
+          }
+
           const payload: PostableRulerRuleGroupDTO = {
             name: existingGroup.name,
             interval: existingGroup.interval,