[v11.0.x] Alerting: Allow deleting contact points referenced only by auto-generated policies (#87115)

Alerting: Allow deleting contact points referenced only by auto-generated policies (#86800) (cherry picked from commit b679a32fad)
2024-04-30 10:43:31 +01:00
parent a24940bfd7
commit 02586f82d4
6 changed files with 103 additions and 31 deletions
@@ -20,6 +20,7 @@ import setupMimirFlavoredServer, { MIMIR_DATASOURCE_UID } from './__mocks__/mimi
 import setupVanillaAlertmanagerFlavoredServer, {
  VANILLA_ALERTMANAGER_DATASOURCE_UID,
 } from './__mocks__/vanillaAlertmanagerServer';
+import { RouteReference } from './utils';

 /**
 * There are lots of ways in which we test our pages and components. Here's my opinionated approach to testing them.
@@ -185,13 +186,19 @@ describe('contact points', () => {
      expect(deleteButton).toBeDisabled();
    });

-    it('should disable delete when contact point is linked to at least one notification policy', async () => {
-      render(
-        <ContactPoint name={'my-contact-point'} provisioned={true} receivers={[]} policies={1} onDelete={noop} />,
+    it('should disable delete when contact point is linked to at least one normal notification policy', async () => {
+      const policies: RouteReference[] = [
        {
-          wrapper,
-        }
-      );
+          receiver: 'my-contact-point',
+          route: {
+            type: 'normal',
+          },
+        },
+      ];
+
+      render(<ContactPoint name={'my-contact-point'} receivers={[]} policies={policies} onDelete={noop} />, {
+        wrapper,
+      });

      expect(screen.getByRole('link', { name: 'is used by 1 notification policy' })).toBeInTheDocument();

@@ -202,6 +209,27 @@ describe('contact points', () => {
      expect(deleteButton).toBeDisabled();
    });

+    it('should not disable delete when contact point is linked only to auto-generated notification policy', async () => {
+      const policies: RouteReference[] = [
+        {
+          receiver: 'my-contact-point',
+          route: {
+            type: 'auto-generated',
+          },
+        },
+      ];
+
+      render(<ContactPoint name={'my-contact-point'} receivers={[]} policies={policies} onDelete={noop} />, {
+        wrapper,
+      });
+
+      const moreActions = screen.getByRole('button', { name: 'more-actions' });
+      await userEvent.click(moreActions);
+
+      const deleteButton = screen.getByRole('menuitem', { name: /delete/i });
+      expect(deleteButton).not.toBeDisabled();
+    });
+
    it('should be able to search', async () => {
      render(
        <AlertmanagerProvider accessType={'notification'}>
@@ -60,7 +60,13 @@ import {
  useContactPointsWithStatus,
  useDeleteContactPoint,
 } from './useContactPoints';
-import { ContactPointWithMetadata, getReceiverDescription, isProvisioned, ReceiverConfigWithMetadata } from './utils';
+import {
+  ContactPointWithMetadata,
+  getReceiverDescription,
+  isProvisioned,
+  ReceiverConfigWithMetadata,
+  RouteReference,
+} from './utils';

 enum ActiveTab {
  ContactPoints,
@@ -226,7 +232,7 @@ const ContactPointsList = ({
    <>
      {pageItems.map((contactPoint, index) => {
        const provisioned = isProvisioned(contactPoint);
-        const policies = contactPoint.numberOfPolicies;
+        const policies = contactPoint.policies ?? [];
        const key = `${contactPoint.name}-${index}`;

        return (
@@ -287,7 +293,7 @@ interface ContactPointProps {
  disabled?: boolean;
  provisioned?: boolean;
  receivers: ReceiverConfigWithMetadata[];
-  policies?: number;
+  policies?: RouteReference[];
  onDelete: (name: string) => void;
 }

@@ -296,7 +302,7 @@ export const ContactPoint = ({
  disabled = false,
  provisioned = false,
  receivers,
-  policies = 0,
+  policies = [],
  onDelete,
 }: ContactPointProps) => {
  const styles = useStyles2(getStyles);
@@ -350,12 +356,12 @@ interface ContactPointHeaderProps {
  name: string;
  disabled?: boolean;
  provisioned?: boolean;
-  policies?: number;
+  policies?: RouteReference[];
  onDelete: (name: string) => void;
 }

 const ContactPointHeader = (props: ContactPointHeaderProps) => {
-  const { name, disabled = false, provisioned = false, policies = 0, onDelete } = props;
+  const { name, disabled = false, provisioned = false, policies = [], onDelete } = props;
  const styles = useStyles2(getStyles);

  const [exportSupported, exportAllowed] = useAlertmanagerAbility(AlertmanagerAction.ExportContactPoint);
@@ -364,9 +370,12 @@ const ContactPointHeader = (props: ContactPointHeaderProps) => {

  const [ExportDrawer, openExportDrawer] = useExportContactPoint();

-  const isReferencedByPolicies = policies > 0;
+  const numberOfPolicies = policies.length;
+  const isReferencedByAnyPolicy = numberOfPolicies > 0;
+  const isReferencedByRegularPolicies = policies.some((ref) => ref.route.type !== 'auto-generated');
+
  const canEdit = editSupported && editAllowed && !provisioned;
-  const canDelete = deleteSupported && deleteAllowed && !provisioned && policies === 0;
+  const canDelete = deleteSupported && deleteAllowed && !provisioned && !isReferencedByRegularPolicies;

  const menuActions: JSX.Element[] = [];

@@ -390,7 +399,7 @@ const ContactPointHeader = (props: ContactPointHeaderProps) => {
    menuActions.push(
      <ConditionalWrap
        key="delete-contact-point"
-        shouldWrap={isReferencedByPolicies}
+        shouldWrap={!canDelete}
        wrap={(children) => (
          <Tooltip content="Contact point is currently in use by one or more notification policies" placement="top">
            <span>{children}</span>
@@ -417,15 +426,15 @@ const ContactPointHeader = (props: ContactPointHeaderProps) => {
            {name}
          </Text>
        </Stack>
-        {isReferencedByPolicies && (
+        {isReferencedByAnyPolicy && (
          <MetaText>
            <Link to={createUrl('/alerting/routes', { contactPoint: name })}>
-              is used by <Strong>{policies}</Strong> {pluralize('notification policy', policies)}
+              is used by <Strong>{numberOfPolicies}</Strong> {pluralize('notification policy', numberOfPolicies)}
            </Link>
          </MetaText>
        )}
        {provisioned && <ProvisioningBadge />}
-        {!isReferencedByPolicies && <UnusedContactPointBadge />}
+        {!isReferencedByAnyPolicy && <UnusedContactPointBadge />}
        <Spacer />
        <LinkButton
          tooltipPlacement="top"
@@ -30,7 +30,14 @@ exports[`useContactPoints should return contact points with status 1`] = `
        },
      ],
      "name": "grafana-default-email",
-      "numberOfPolicies": 1,
+      "policies": [
+        {
+          "receiver": "grafana-default-email",
+          "route": {
+            "type": "normal",
+          },
+        },
+      ],
    },
    {
      "grafana_managed_receiver_configs": [
@@ -58,7 +65,7 @@ exports[`useContactPoints should return contact points with status 1`] = `
        },
      ],
      "name": "lotsa-emails",
-      "numberOfPolicies": 0,
+      "policies": [],
    },
    {
      "grafana_managed_receiver_configs": [
@@ -87,7 +94,14 @@ exports[`useContactPoints should return contact points with status 1`] = `
        },
      ],
      "name": "provisioned-contact-point",
-      "numberOfPolicies": 1,
+      "policies": [
+        {
+          "receiver": "provisioned-contact-point",
+          "route": {
+            "type": "normal",
+          },
+        },
+      ],
    },
    {
      "grafana_managed_receiver_configs": [
@@ -139,7 +153,7 @@ exports[`useContactPoints should return contact points with status 1`] = `
        },
      ],
      "name": "Slack with multiple channels",
-      "numberOfPolicies": 0,
+      "policies": [],
    },
  ],
  "error": undefined,
@@ -2,6 +2,8 @@ import React, { useCallback, useMemo, useState } from 'react';

 import { Button, Modal, ModalProps } from '@grafana/ui';

+import { stringifyErrorLike } from '../../../utils/misc';
+
 type ModalHook<T = undefined> = [JSX.Element, (item: T) => void, () => void];

 /**
@@ -83,7 +85,9 @@ const ErrorModal = ({ isOpen, onDismiss, error }: ErrorModalProps) => (
  >
    <p>Failed to update your configuration:</p>
    <p>
-      <code>{String(error)}</code>
+      <pre>
+        <code>{stringifyErrorLike(error)}</code>
+      </pre>
    </p>
  </Modal>
 );
@@ -1,4 +1,4 @@
-import { countBy, difference, take, trim, upperFirst } from 'lodash';
+import { difference, groupBy, take, trim, upperFirst } from 'lodash';
 import { ReactNode } from 'react';

 import { config } from '@grafana/runtime';
@@ -99,7 +99,7 @@ export interface ReceiverConfigWithMetadata extends GrafanaManagedReceiverConfig
 }

 export interface ContactPointWithMetadata extends GrafanaManagedContactPoint {
-  numberOfPolicies?: number; // now is optional as we don't have the data from the read-only endpoint
+  policies?: RouteReference[]; // now is optional as we don't have the data from the read-only endpoint
  grafana_managed_receiver_configs: ReceiverConfigWithMetadata[];
 }

@@ -121,7 +121,7 @@ export function enhanceContactPointsWithMetadata(
  // compute the entire inherited tree before finding what notification policies are using a particular contact point
  const fullyInheritedTree = computeInheritedTree(alertmanagerConfiguration?.alertmanager_config?.route ?? {});
  const usedContactPoints = getUsedContactPoints(fullyInheritedTree);
-  const usedContactPointsByName = countBy(usedContactPoints);
+  const usedContactPointsByName = groupBy(usedContactPoints, 'receiver');

  const contactPointsList = alertmanagerConfiguration
    ? alertmanagerConfiguration?.alertmanager_config.receivers ?? []
@@ -133,8 +133,8 @@ export function enhanceContactPointsWithMetadata(

    return {
      ...contactPoint,
-      numberOfPolicies:
-        alertmanagerConfiguration && usedContactPointsByName && (usedContactPointsByName[contactPoint.name] ?? 0),
+      policies:
+        alertmanagerConfiguration && usedContactPointsByName && (usedContactPointsByName[contactPoint.name] ?? []),
      grafana_managed_receiver_configs: receivers.map((receiver, index) => {
        const isOnCallReceiver = receiver.type === ReceiverTypes.OnCall;
        return {
@@ -166,10 +166,26 @@ export function isAutoGeneratedPolicy(route: Route) {
  );
 }

-export function getUsedContactPoints(route: Route): string[] {
+export interface RouteReference {
+  receiver: string;
+  route: {
+    type: 'auto-generated' | 'normal';
+  };
+}
+
+export function getUsedContactPoints(route: Route): RouteReference[] {
  const childrenContactPoints = route.routes?.flatMap((route) => getUsedContactPoints(route)) ?? [];
+
  if (route.receiver) {
-    return [route.receiver, ...childrenContactPoints];
+    return [
+      {
+        receiver: route.receiver,
+        route: {
+          type: isAutoGeneratedPolicy(route) ? 'auto-generated' : 'normal',
+        },
+      },
+      ...childrenContactPoints,
+    ];
  }

  return childrenContactPoints;
@@ -259,6 +259,7 @@ describe('Can create a new grafana managed alert unsing simplified routing', ()
      expect(mocks.api.setRulerRuleGroup).not.toHaveBeenCalled();
    });
  });
+
  it('can create new grafana managed alert when using simplified routing and selecting a contact point', async () => {
    const contactPointsAvailable: ContactPointWithMetadata[] = [
      {
@@ -275,7 +276,7 @@ describe('Can create a new grafana managed alert unsing simplified routing', ()
            settings: {},
          },
        ],
-        numberOfPolicies: 0,
+        policies: [],
      },
    ];
    mocks.useContactPointsWithStatus.mockReturnValue({