From 7cbf5ae78d2a010af79b40aee927f148c1ef18ef Mon Sep 17 00:00:00 2001 From: Gilles De Mey Date: Thu, 30 Nov 2023 13:37:14 +0100 Subject: [PATCH] Alerting: Remove old contact points view (#78704) --- .betterer.results | 2 +- .../feature-toggles/index.md | 1 - .../src/types/featureToggles.gen.ts | 1 - pkg/services/featuremgmt/registry.go | 7 - pkg/services/featuremgmt/toggles_gen.csv | 1 - pkg/services/featuremgmt/toggles_gen.go | 4 - .../features/alerting/unified/Receivers.tsx | 42 +- ...nts.v2.test.tsx => ContactPoints.test.tsx} | 147 +++- ...ContactPoints.v2.tsx => ContactPoints.tsx} | 12 +- .../contact-points/ContactPoints.v1.test.tsx | 699 ------------------ .../contact-points/ContactPoints.v1.tsx | 107 --- .../contact-points/NewContactPoint.test.tsx | 116 +++ .../contact-points/NewContactPoint.tsx | 3 +- .../__mocks__/alertmanager.config.mock.json | 8 + .../alertmanager.mimir.config.mock.json | 8 +- .../__mocks__/alertmanager.vanilla.mock.json | 70 ++ .../__mocks__/grafanaManagedServer.ts | 32 +- .../contact-points/__mocks__/server.ts | 24 - .../__mocks__/vanillaAlertmanagerServer.ts | 21 + .../NewContactPoint.test.tsx.snap | 147 ++++ .../useContactPoints.test.tsx.snap | 4 +- .../{ => components}/ContactPointsFilter.tsx | 2 +- .../{ => components}/GlobalConfig.tsx | 6 +- .../components/GlobalConfigAlert.tsx | 30 + .../{ => components}/Modals.tsx | 0 .../contact-points/components/UnusedBadge.tsx | 13 + .../receivers/ReceiversAndTemplatesView.tsx | 73 -- .../receivers/ReceiversTable.test.tsx | 242 ------ .../components/receivers/ReceiversTable.tsx | 522 ------------- .../grafanaAppReceivers/grafanaApp.ts | 12 +- .../receivers/grafanaAppReceivers/types.ts | 4 - .../useReceiversMetadata.ts | 34 +- .../receivers/useAlertmanagerConfigHealth.ts | 35 - .../ContactPointSelector.tsx | 2 +- .../alerting/unified/utils/alertmanager.ts | 12 - .../alerting/unified/utils/receivers.ts | 45 +- .../plugins/datasource/alertmanager/types.ts | 24 +- 37 files changed, 627 insertions(+), 1885 deletions(-) rename public/app/features/alerting/unified/components/contact-points/{ContactPoints.v2.test.tsx => ContactPoints.test.tsx} (57%) rename public/app/features/alerting/unified/components/contact-points/{ContactPoints.v2.tsx => ContactPoints.tsx} (98%) delete mode 100644 public/app/features/alerting/unified/components/contact-points/ContactPoints.v1.test.tsx delete mode 100644 public/app/features/alerting/unified/components/contact-points/ContactPoints.v1.tsx create mode 100644 public/app/features/alerting/unified/components/contact-points/NewContactPoint.test.tsx create mode 100644 public/app/features/alerting/unified/components/contact-points/__mocks__/alertmanager.vanilla.mock.json delete mode 100644 public/app/features/alerting/unified/components/contact-points/__mocks__/server.ts create mode 100644 public/app/features/alerting/unified/components/contact-points/__mocks__/vanillaAlertmanagerServer.ts create mode 100644 public/app/features/alerting/unified/components/contact-points/__snapshots__/NewContactPoint.test.tsx.snap rename public/app/features/alerting/unified/components/contact-points/{ => components}/ContactPointsFilter.tsx (95%) rename public/app/features/alerting/unified/components/contact-points/{ => components}/GlobalConfig.tsx (74%) create mode 100644 public/app/features/alerting/unified/components/contact-points/components/GlobalConfigAlert.tsx rename public/app/features/alerting/unified/components/contact-points/{ => components}/Modals.tsx (100%) create mode 100644 public/app/features/alerting/unified/components/contact-points/components/UnusedBadge.tsx delete mode 100644 public/app/features/alerting/unified/components/receivers/ReceiversAndTemplatesView.tsx delete mode 100644 public/app/features/alerting/unified/components/receivers/ReceiversTable.test.tsx delete mode 100644 public/app/features/alerting/unified/components/receivers/ReceiversTable.tsx delete mode 100644 public/app/features/alerting/unified/components/receivers/useAlertmanagerConfigHealth.ts diff --git a/.betterer.results b/.betterer.results index 515f8d40135..bed4d33a06e 100644 --- a/.betterer.results +++ b/.betterer.results @@ -1830,7 +1830,7 @@ exports[`better eslint`] = { [0, 0, 0, "Styles should be written using objects.", "0"], [0, 0, 0, "Styles should be written using objects.", "1"] ], - "public/app/features/alerting/unified/components/contact-points/ContactPoints.v2.tsx:5381": [ + "public/app/features/alerting/unified/components/contact-points/ContactPoints.tsx:5381": [ [0, 0, 0, "Do not use any type assertions.", "0"] ], "public/app/features/alerting/unified/components/export/FileExportPreview.tsx:5381": [ diff --git a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md index 0fdd2893cf0..979ac9330ff 100644 --- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md +++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md @@ -74,7 +74,6 @@ Some features are enabled by default. You can disable these feature by setting t | `splitScopes` | Support faster dashboard and folder search by splitting permission scopes into parts | | `dashgpt` | Enable AI powered features in dashboards | | `reportingRetries` | Enables rendering retries for the reporting feature | -| `alertingContactPointsV2` | Show the new contacpoints list view | | `transformationsVariableSupport` | Allows using variables in transformations | | `cloudWatchBatchQueries` | Runs CloudWatch metrics queries as separate batches | diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index 33da24411d5..6eb6c218fb4 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -124,7 +124,6 @@ export interface FeatureToggles { lokiRunQueriesInParallel?: boolean; wargamesTesting?: boolean; alertingInsights?: boolean; - alertingContactPointsV2?: boolean; externalCorePlugins?: boolean; pluginsAPIMetrics?: boolean; httpSLOLevels?: boolean; diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index d10f921d735..22004e59248 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -791,13 +791,6 @@ var ( AllowSelfServe: falsePtr, HideFromAdminPage: true, // This is moving away from being a feature toggle. }, - { - Name: "alertingContactPointsV2", - Description: "Show the new contacpoints list view", - FrontendOnly: true, - Stage: FeatureStagePublicPreview, - Owner: grafanaAlertingSquad, - }, { Name: "externalCorePlugins", Description: "Allow core plugins to be loaded as external", diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index 956556ac376..34fd3bc6048 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -105,7 +105,6 @@ libraryPanelRBAC,experimental,@grafana/dashboards-squad,false,false,true,false lokiRunQueriesInParallel,privatePreview,@grafana/observability-logs,false,false,false,false wargamesTesting,experimental,@grafana/hosted-grafana-team,false,false,false,false alertingInsights,GA,@grafana/alerting-squad,false,false,false,true -alertingContactPointsV2,preview,@grafana/alerting-squad,false,false,false,true externalCorePlugins,experimental,@grafana/plugins-platform-backend,false,false,false,false pluginsAPIMetrics,experimental,@grafana/plugins-platform-backend,false,false,false,true httpSLOLevels,experimental,@grafana/hosted-grafana-team,false,false,true,false diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index a37c72933dc..900645c168a 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -431,10 +431,6 @@ const ( // Show the new alerting insights landing page FlagAlertingInsights = "alertingInsights" - // FlagAlertingContactPointsV2 - // Show the new contacpoints list view - FlagAlertingContactPointsV2 = "alertingContactPointsV2" - // FlagExternalCorePlugins // Allow core plugins to be loaded as external FlagExternalCorePlugins = "externalCorePlugins" diff --git a/public/app/features/alerting/unified/Receivers.tsx b/public/app/features/alerting/unified/Receivers.tsx index ccaa9128c96..6de84d52b62 100644 --- a/public/app/features/alerting/unified/Receivers.tsx +++ b/public/app/features/alerting/unified/Receivers.tsx @@ -1,15 +1,13 @@ import React from 'react'; import { Route, Switch } from 'react-router-dom'; -import { config } from '@grafana/runtime'; import { withErrorBoundary } from '@grafana/ui'; -const ContactPointsV1 = SafeDynamicImport(() => import('./components/contact-points/ContactPoints.v1')); -const ContactPointsV2 = SafeDynamicImport(() => import('./components/contact-points/ContactPoints.v2')); +const ContactPointsV2 = SafeDynamicImport(() => import('./components/contact-points/ContactPoints')); const EditContactPoint = SafeDynamicImport(() => import('./components/contact-points/EditContactPoint')); const NewContactPoint = SafeDynamicImport(() => import('./components/contact-points/NewContactPoint')); const EditMessageTemplate = SafeDynamicImport(() => import('./components/contact-points/EditMessageTemplate')); const NewMessageTemplate = SafeDynamicImport(() => import('./components/contact-points/NewMessageTemplate')); -const GlobalConfig = SafeDynamicImport(() => import('./components/contact-points/GlobalConfig')); +const GlobalConfig = SafeDynamicImport(() => import('./components/contact-points/components/GlobalConfig')); const DuplicateMessageTemplate = SafeDynamicImport( () => import('./components/contact-points/DuplicateMessageTemplate') ); @@ -18,29 +16,21 @@ import { GrafanaRouteComponentProps } from 'app/core/navigation/types'; import { AlertmanagerPageWrapper } from './components/AlertingPageWrapper'; -const newContactPointsListView = config.featureToggles.alertingContactPointsV2 ?? false; - -// TODO add pagenav back in – that way we have correct breadcrumbs and page title -const ContactPoints = (props: GrafanaRouteComponentProps): JSX.Element => ( +const ContactPoints = (_props: GrafanaRouteComponentProps): JSX.Element => ( - {/* TODO do we want a "routes" component for each Alerting entity? */} - {newContactPointsListView ? ( - - - - - - - - - - ) : ( - - )} + + + + + + + + + ); diff --git a/public/app/features/alerting/unified/components/contact-points/ContactPoints.v2.test.tsx b/public/app/features/alerting/unified/components/contact-points/ContactPoints.test.tsx similarity index 57% rename from public/app/features/alerting/unified/components/contact-points/ContactPoints.v2.test.tsx rename to public/app/features/alerting/unified/components/contact-points/ContactPoints.test.tsx index 28420a80172..fde20c0fadc 100644 --- a/public/app/features/alerting/unified/components/contact-points/ContactPoints.v2.test.tsx +++ b/public/app/features/alerting/unified/components/contact-points/ContactPoints.test.tsx @@ -5,6 +5,7 @@ import React, { PropsWithChildren } from 'react'; import { TestProvider } from 'test/helpers/TestProvider'; import { selectors } from '@grafana/e2e-selectors'; +import { AlertManagerDataSourceJsonData, AlertManagerImplementation } from 'app/plugins/datasource/alertmanager/types'; import { AccessControlAction } from 'app/types'; import { setupMswServer } from '../../mockApi'; @@ -13,9 +14,12 @@ import { AlertmanagerProvider } from '../../state/AlertmanagerContext'; import { setupDataSources } from '../../testSetup/datasources'; import { DataSourceType } from '../../utils/datasource'; -import ContactPoints, { ContactPoint } from './ContactPoints.v2'; +import ContactPoints, { ContactPoint } from './ContactPoints'; import setupGrafanaManagedServer from './__mocks__/grafanaManagedServer'; import setupMimirFlavoredServer, { MIMIR_DATASOURCE_UID } from './__mocks__/mimirFlavoredServer'; +import setupVanillaAlertmanagerFlavoredServer, { + VANILLA_ALERTMANAGER_DATASOURCE_UID, +} from './__mocks__/vanillaAlertmanagerServer'; /** * There are lots of ways in which we test our pages and components. Here's my opinionated approach to testing them. @@ -38,17 +42,15 @@ const server = setupMswServer(); describe('contact points', () => { describe('Contact points with Grafana managed alertmanager', () => { beforeEach(() => { - setupGrafanaManagedServer(server); - }); - - beforeAll(() => { grantUserPermissions([ AccessControlAction.AlertingNotificationsRead, AccessControlAction.AlertingNotificationsWrite, ]); + + setupGrafanaManagedServer(server); }); - it('should show / hide loading states', async () => { + it('should show / hide loading states, have all actions enabled', async () => { render( @@ -64,6 +66,67 @@ describe('contact points', () => { expect(screen.getByText('grafana-default-email')).toBeInTheDocument(); expect(screen.getAllByTestId('contact-point')).toHaveLength(4); + + // check for available actions – our mock 4 contact points, 1 of them is provisioned + expect(screen.getByRole('link', { name: 'add contact point' })).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'export all' })).toBeInTheDocument(); + + // 2 of them are unused by routes in the mock response + const unusedBadge = screen.getAllByLabelText('unused'); + expect(unusedBadge).toHaveLength(2); + + const viewProvisioned = screen.getByRole('link', { name: 'view-action' }); + expect(viewProvisioned).toBeInTheDocument(); + expect(viewProvisioned).not.toBeDisabled(); + + const editButtons = screen.getAllByRole('link', { name: 'edit-action' }); + expect(editButtons).toHaveLength(3); + editButtons.forEach((button) => { + expect(button).not.toBeDisabled(); + }); + + const moreActionsButtons = screen.getAllByRole('button', { name: 'more-actions' }); + expect(moreActionsButtons).toHaveLength(4); + moreActionsButtons.forEach((button) => { + expect(button).not.toBeDisabled(); + }); + }); + + it('should disable certain actions if the user has no write permissions', async () => { + grantUserPermissions([AccessControlAction.AlertingNotificationsRead]); + + render( + + + , + { wrapper: TestProvider } + ); + + // wait for loading to be done + await waitFor(async () => { + expect(screen.queryByText('Loading...')).not.toBeInTheDocument(); + }); + + // should disable create contact point + expect(screen.getByRole('link', { name: 'add contact point' })).toHaveAttribute('aria-disabled', 'true'); + + // there should be no edit buttons + expect(screen.queryAllByRole('link', { name: 'edit-action' })).toHaveLength(0); + + // there should be view buttons though + const viewButtons = screen.getAllByRole('link', { name: 'view-action' }); + expect(viewButtons).toHaveLength(4); + + // delete should be disabled in the "more" actions + const moreButtons = screen.queryAllByRole('button', { name: 'more-actions' }); + expect(moreButtons).toHaveLength(4); + + // check if all of the delete buttons are disabled + for await (const button of moreButtons) { + await userEvent.click(button); + const deleteButton = await screen.queryByRole('menuitem', { name: 'delete' }); + expect(deleteButton).toBeDisabled(); + } }); it('should call delete when clicked and not disabled', async () => { @@ -175,7 +238,7 @@ describe('contact points', () => { ); }); - it('should show / hide loading states', async () => { + it('should show / hide loading states, have the right actions enabled', async () => { render( @@ -193,6 +256,76 @@ describe('contact points', () => { expect(screen.getByText('mixed')).toBeInTheDocument(); expect(screen.getByText('some webhook')).toBeInTheDocument(); expect(screen.getAllByTestId('contact-point')).toHaveLength(2); + + // check for available actions – export should be disabled + expect(screen.getByRole('link', { name: 'add contact point' })).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: 'export all' })).not.toBeInTheDocument(); + + // 1 of them is used by a route in the mock response + const unusedBadge = screen.getAllByLabelText('unused'); + expect(unusedBadge).toHaveLength(1); + + const editButtons = screen.getAllByRole('link', { name: 'edit-action' }); + expect(editButtons).toHaveLength(2); + editButtons.forEach((button) => { + expect(button).not.toBeDisabled(); + }); + + const moreActionsButtons = screen.getAllByRole('button', { name: 'more-actions' }); + expect(moreActionsButtons).toHaveLength(2); + moreActionsButtons.forEach((button) => { + expect(button).not.toBeDisabled(); + }); + }); + }); + + describe('Vanilla Alertmanager ', () => { + beforeEach(() => { + setupVanillaAlertmanagerFlavoredServer(server); + }); + + beforeAll(() => { + grantUserPermissions([ + AccessControlAction.AlertingNotificationsExternalRead, + AccessControlAction.AlertingNotificationsExternalWrite, + ]); + + const alertManager = mockDataSource({ + name: VANILLA_ALERTMANAGER_DATASOURCE_UID, + uid: VANILLA_ALERTMANAGER_DATASOURCE_UID, + type: DataSourceType.Alertmanager, + jsonData: { + implementation: AlertManagerImplementation.prometheus, + handleGrafanaManagedAlerts: true, + }, + }); + + setupDataSources(alertManager); + }); + + it("should not allow any editing because it's not supported", async () => { + render( + + + + + + ); + + await waitFor(async () => { + expect(screen.getByText('Loading...')).toBeInTheDocument(); + await waitForElementToBeRemoved(screen.getByText('Loading...')); + expect(screen.queryByTestId(selectors.components.Alert.alertV2('error'))).not.toBeInTheDocument(); + }); + + expect(screen.queryByRole('link', { name: 'add contact point' })).not.toBeInTheDocument(); + + const viewProvisioned = screen.getByRole('link', { name: 'view-action' }); + expect(viewProvisioned).toBeInTheDocument(); + expect(viewProvisioned).not.toBeDisabled(); }); }); }); diff --git a/public/app/features/alerting/unified/components/contact-points/ContactPoints.v2.tsx b/public/app/features/alerting/unified/components/contact-points/ContactPoints.tsx similarity index 98% rename from public/app/features/alerting/unified/components/contact-points/ContactPoints.v2.tsx rename to public/app/features/alerting/unified/components/contact-points/ContactPoints.tsx index c6e8cc2fe97..368cd23aa5f 100644 --- a/public/app/features/alerting/unified/components/contact-points/ContactPoints.v2.tsx +++ b/public/app/features/alerting/unified/components/contact-points/ContactPoints.tsx @@ -45,14 +45,14 @@ import { Spacer } from '../Spacer'; import { Strong } from '../Strong'; import { GrafanaReceiverExporter } from '../export/GrafanaReceiverExporter'; import { GrafanaReceiversExporter } from '../export/GrafanaReceiversExporter'; -import { GlobalConfigAlert } from '../receivers/ReceiversAndTemplatesView'; -import { UnusedContactPointBadge } from '../receivers/ReceiversTable'; import { ReceiverMetadataBadge } from '../receivers/grafanaAppReceivers/ReceiverMetadataBadge'; import { ReceiverPluginMetadata } from '../receivers/grafanaAppReceivers/useReceiversMetadata'; -import { ContactPointsFilter } from './ContactPointsFilter'; -import { useDeleteContactPointModal } from './Modals'; import { NotificationTemplates } from './NotificationTemplates'; +import { ContactPointsFilter } from './components/ContactPointsFilter'; +import { GlobalConfigAlert } from './components/GlobalConfigAlert'; +import { useDeleteContactPointModal } from './components/Modals'; +import { UnusedContactPointBadge } from './components/UnusedBadge'; import { RECEIVER_META_KEY, RECEIVER_PLUGIN_META_KEY, @@ -135,6 +135,7 @@ const ContactPoints = () => { {addContactPointSupported && ( {