Alerting: Fixes Alert list panel "ungrouped" regression (#68018)

public/app/plugins/panel/alertlist/unified-alerting/GroupedView.test.tsx

@@ -0,0 +1,46 @@
+import { render, screen } from '@testing-library/react';
+import React from 'react';
+
+import { CombinedRuleWithLocation } from 'app/types/unified-alerting';
+import { PromRuleType } from 'app/types/unified-alerting-dto';
+
+import { UnifiedAlertListOptions } from '../types';
+
+import GroupedView, { UNGROUPED_KEY } from './GroupedView';
+
+describe('Grouped view', () => {
+  const rules: CombinedRuleWithLocation[] = [
+    {
+      promRule: {
+        type: PromRuleType.Alerting,
+        alerts: [
+          // @ts-ignore
+          { labels: { job: 'job-1', severity: 'high' } },
+          // @ts-ignore
+          { labels: { job: 'job-2', severity: 'low' } },
+        ],
+      },
+    },
+    {
+      promRule: {
+        type: PromRuleType.Alerting,
+        alerts: [
+          // @ts-ignore
+          { labels: { foo: 'bar', severity: 'low' } },
+        ],
+      },
+    },
+  ];
+
+  it('should group instances by label(s) correctly', () => {
+    // @ts-ignore
+    const options: UnifiedAlertListOptions = {
+      groupBy: ['job', 'severity'],
+    };
+
+    render(<GroupedView rules={rules} options={options} />);
+    expect(screen.getByTestId('job=job-1&severity=high')).toBeInTheDocument();
+    expect(screen.getByTestId('job=job-2&severity=low')).toBeInTheDocument();
+    expect(screen.getByTestId(UNGROUPED_KEY)).toBeInTheDocument();
+  });
+});

public/app/plugins/panel/alertlist/unified-alerting/GroupedView.tsx

@@ -5,7 +5,7 @@ import { AlertLabel } from 'app/features/alerting/unified/components/AlertLabel'
 import { getAlertingRule } from 'app/features/alerting/unified/utils/rules';
 import { Alert } from 'app/types/unified-alerting';
 
-import { AlertingRule, CombinedRuleWithLocation } from '../../../../types/unified-alerting';
+import { CombinedRuleWithLocation } from '../../../../types/unified-alerting';
 import { AlertInstances } from '../AlertInstances';
 import { getStyles } from '../UnifiedAlertList';
 import { GroupedRules, UnifiedAlertListOptions } from '../types';
@@ -16,9 +16,10 @@ type Props = {
   options: UnifiedAlertListOptions;
 };
 
+export const UNGROUPED_KEY = '__ungrouped__';
+
 const GroupedModeView = ({ rules, options }: Props) => {
   const styles = useStyles2(getStyles);
-
   const groupBy = options.groupBy;
 
   const groupedRules = useMemo<GroupedRules>(() => {
@@ -27,16 +28,23 @@ const GroupedModeView = ({ rules, options }: Props) => {
     const hasInstancesWithMatchingLabels = (rule: CombinedRuleWithLocation) =>
       groupBy ? alertHasEveryLabelForCombinedRules(rule, groupBy) : true;
 
-    const matchingRules = rules.filter(hasInstancesWithMatchingLabels);
-    matchingRules.forEach((rule: CombinedRuleWithLocation) => {
-      const alertingRule: AlertingRule | null = getAlertingRule(rule);
+    rules.forEach((rule) => {
+      const alertingRule = getAlertingRule(rule);
+      const hasInstancesMatching = hasInstancesWithMatchingLabels(rule);
+
       (alertingRule?.alerts ?? []).forEach((alert) => {
-        const mapKey = createMapKey(groupBy, alert.labels);
+        const mapKey = hasInstancesMatching ? createMapKey(groupBy, alert.labels) : UNGROUPED_KEY;
+
         const existingAlerts = groupedRules.get(mapKey) ?? [];
         groupedRules.set(mapKey, [...existingAlerts, alert]);
       });
     });
 
+    // move the "UNGROUPED" key to the last item in the Map, items are shown in insertion order
+    const ungrouped = groupedRules.get(UNGROUPED_KEY) ?? [];
+    groupedRules.delete(UNGROUPED_KEY);
+    groupedRules.set(UNGROUPED_KEY, ungrouped);
+
     // Remove groups having no instances
     // This is different from filtering Rules without instances that we do in UnifiedAlertList
     const filteredGroupedRules = Array.from(groupedRules.entries()).reduce((acc, [groupKey, groupAlerts]) => {
@@ -54,12 +62,13 @@ const GroupedModeView = ({ rules, options }: Props) => {
   return (
     <>
       {Array.from(groupedRules).map(([key, alerts]) => (
-        <li className={styles.alertRuleItem} key={key}>
+        <li className={styles.alertRuleItem} key={key} data-testid={key}>
           <div>
             <div className={styles.customGroupDetails}>
               <div className={styles.alertLabels}>
-                {key && parseMapKey(key).map(([key, value]) => <AlertLabel key={key} labelKey={key} value={value} />)}
-                {!key && 'No grouping'}
+                {key !== UNGROUPED_KEY &&
+                  parseMapKey(key).map(([key, value]) => <AlertLabel key={key} labelKey={key} value={value} />)}
+                {key === UNGROUPED_KEY && 'No grouping'}
               </div>
             </div>
             <AlertInstances alerts={alerts} options={options} />
@@ -79,7 +88,7 @@ function parseMapKey(key: string): Array<[string, string]> {
 }
 
 function alertHasEveryLabelForCombinedRules(rule: CombinedRuleWithLocation, groupByKeys: string[]) {
-  const alertingRule: AlertingRule | null = getAlertingRule(rule);
+  const alertingRule = getAlertingRule(rule);
   return groupByKeys.every((key) => {
     return (alertingRule?.alerts ?? []).some((alert) => alert.labels[key]);
   });