Alerting: Separate overlapping legacy and UA alerting routes (grafana#76517)

JacobsonMT · web-flow · commit c18da48e50c1 · 2024-01-04T18:01:57.000-05:00
* Separate overlapping legacy and UA alerting routes

api/alert-notifiers, alerting/list, and alerting/notifications existed in both
legacy and UA.
Rename legacy route paths and nav ids to be independent of UA ones.
diff --git a/pkg/api/alerting.go b/pkg/api/alerting.go
@@ -239,17 +239,18 @@ func (hs *HTTPServer) GetAlert(c *contextmodel.ReqContext) response.Response {
 	return response.JSON(http.StatusOK, &res)
 }
 
-func (hs *HTTPServer) GetAlertNotifiers(ngalertEnabled bool) func(*contextmodel.ReqContext) response.Response {
+func (hs *HTTPServer) GetLegacyAlertNotifiers() func(*contextmodel.ReqContext) response.Response {
 	return func(_ *contextmodel.ReqContext) response.Response {
-		if ngalertEnabled {
-			return response.JSON(http.StatusOK, channels_config.GetAvailableNotifiers())
-		}
-		// TODO(codesome): This wont be required in 8.0 since ngalert
-		// will be enabled by default with no disabling. This is to be removed later.
 		return response.JSON(http.StatusOK, alerting.GetNotifiers())
 	}
 }
 
+func (hs *HTTPServer) GetAlertNotifiers() func(*contextmodel.ReqContext) response.Response {
+	return func(_ *contextmodel.ReqContext) response.Response {
+		return response.JSON(http.StatusOK, channels_config.GetAvailableNotifiers())
+	}
+}
+
 // swagger:route GET /alert-notifications/lookup legacy_alerts_notification_channels getAlertNotificationLookup
 //
 // Get all notification channels (lookup).
diff --git a/pkg/api/api.go b/pkg/api/api.go
@@ -47,7 +47,6 @@ import (
 	"github.com/grafana/grafana/pkg/services/serviceaccounts"
 	"github.com/grafana/grafana/pkg/services/user"
 	"github.com/grafana/grafana/pkg/setting"
-	"github.com/grafana/grafana/pkg/web"
 )
 
 var plog = log.New("api")
@@ -514,15 +513,14 @@ func (hs *HTTPServer) registerRoutes() {
 			alertsRoute.Get("/states-for-dashboard", routing.Wrap(hs.GetAlertStatesForDashboard))
 		}, requestmeta.SetOwner(requestmeta.TeamAlerting))
 
-		var notifiersAuthHandler web.Handler
-		if hs.Cfg.UnifiedAlerting.IsEnabled() {
-			notifiersAuthHandler = reqSignedIn
-		} else {
-			notifiersAuthHandler = reqEditorRole
-		}
+		// Unified Alerting
+		apiRoute.Get("/alert-notifiers", reqSignedIn, requestmeta.SetOwner(requestmeta.TeamAlerting), routing.Wrap(
+			hs.GetAlertNotifiers()),
+		)
 
-		apiRoute.Get("/alert-notifiers", notifiersAuthHandler, requestmeta.SetOwner(requestmeta.TeamAlerting), routing.Wrap(
-			hs.GetAlertNotifiers(hs.Cfg.UnifiedAlerting.IsEnabled())),
+		// Legacy
+		apiRoute.Get("/alert-notifiers-legacy", reqEditorRole, requestmeta.SetOwner(requestmeta.TeamAlerting), routing.Wrap(
+			hs.GetLegacyAlertNotifiers()),
 		)
 
 		apiRoute.Group("/alert-notifications", func(alertNotifications routing.RouteRegister) {
diff --git a/pkg/services/navtree/navtreeimpl/navtree.go b/pkg/services/navtree/navtreeimpl/navtree.go
@@ -394,12 +394,12 @@ func (s *ServiceImpl) buildDashboardNavLinks(c *contextmodel.ReqContext) []*navt
 func (s *ServiceImpl) buildLegacyAlertNavLinks(c *contextmodel.ReqContext) *navtree.NavLink {
 	var alertChildNavs []*navtree.NavLink
 	alertChildNavs = append(alertChildNavs, &navtree.NavLink{
-		Text: "Alert rules", Id: "alert-list", Url: s.cfg.AppSubURL + "/alerting/list", Icon: "list-ul",
+		Text: "Alert rules", Id: "alert-list-legacy", Url: s.cfg.AppSubURL + "/alerting-legacy/list", Icon: "list-ul",
 	})
 
 	if c.SignedInUser.HasRole(roletype.RoleEditor) {
 		alertChildNavs = append(alertChildNavs, &navtree.NavLink{
-			Text: "Notification channels", Id: "channels", Url: s.cfg.AppSubURL + "/alerting/notifications",
+			Text: "Notification channels", Id: "channels", Url: s.cfg.AppSubURL + "/alerting-legacy/notifications",
 			Icon: "comment-alt-share",
 		})
 	}
@@ -411,7 +411,7 @@ func (s *ServiceImpl) buildLegacyAlertNavLinks(c *contextmodel.ReqContext) *navt
 		Icon:       "bell",
 		Children:   alertChildNavs,
 		SortWeight: navtree.WeightAlerting,
-		Url:        s.cfg.AppSubURL + "/alerting",
+		Url:        s.cfg.AppSubURL + "/alerting-legacy",
 	}
 
 	return &alertNav
diff --git a/public/app/core/utils/navBarItem-translations.ts b/public/app/core/utils/navBarItem-translations.ts
@@ -61,6 +61,8 @@ export function getNavTitle(navId: string | undefined) {
       return t('nav.alerting-home.title', 'Home');
     case 'alert-list':
       return t('nav.alerting-list.title', 'Alert rules');
+    case 'alert-list-legacy':
+      return t('nav.alert-list-legacy.title', 'Alert rules');
     case 'receivers':
       return t('nav.alerting-receivers.title', 'Contact points');
     case 'am-routes':
diff --git a/public/app/features/alerting/AlertRuleList.tsx b/public/app/features/alerting/AlertRuleList.tsx
@@ -95,7 +95,7 @@ export class AlertRuleListUnconnected extends PureComponent<Props> {
     const { alertRules, search, isLoading } = this.props;
 
     return (
-      <Page navId="alert-list">
+      <Page navId="alert-list-legacy">
         <Page.Contents isLoading={isLoading}>
           <div className="page-action-bar">
             <div className="gf-form gf-form--grow">
@@ -117,7 +117,7 @@ export class AlertRuleListUnconnected extends PureComponent<Props> {
             </div>
             <div className="page-action-bar__spacer" />
             {config.unifiedAlertingEnabled && (
-              <LinkButton variant="primary" href="alerting/ng/new">
+              <LinkButton variant="primary" href="alerting-legacy/ng/new">
                 Add NG Alert
               </LinkButton>
             )}
diff --git a/public/app/features/alerting/NotificationsListPage.tsx b/public/app/features/alerting/NotificationsListPage.tsx
@@ -57,7 +57,7 @@ const NotificationsListPage: FC = () => {
           <>
             <div className="page-action-bar">
               <div className="page-action-bar__spacer" />
-              <LinkButton icon="channel-add" href="alerting/notification/new">
+              <LinkButton icon="channel-add" href="alerting-legacy/notification/new">
                 New channel
               </LinkButton>
             </div>
@@ -75,10 +75,10 @@ const NotificationsListPage: FC = () => {
                 {notifications.map((notification) => (
                   <tr key={notification.id}>
                     <td className="link-td">
-                      <a href={`alerting/notification/${notification.id}/edit`}>{notification.name}</a>
+                      <a href={`alerting-legacy/notification/${notification.id}/edit`}>{notification.name}</a>
                     </td>
                     <td className="link-td">
-                      <a href={`alerting/notification/${notification.id}/edit`}>{notification.type}</a>
+                      <a href={`alerting-legacy/notification/${notification.id}/edit`}>{notification.type}</a>
                     </td>
                     <td className="text-right">
                       <HorizontalGroup justify="flex-end">
@@ -108,7 +108,7 @@ const NotificationsListPage: FC = () => {
           <EmptyListCTA
             title="There are no notification channels defined yet"
             buttonIcon="channel-add"
-            buttonLink="alerting/notification/new"
+            buttonLink="alerting-legacy/notification/new"
             buttonTitle="Add channel"
             proTip="You can include images in your alert notifications."
             proTipLink="http://docs.grafana.org/alerting/notifications/"
diff --git a/public/app/features/alerting/components/NotificationChannelForm.tsx b/public/app/features/alerting/components/NotificationChannelForm.tsx
@@ -106,7 +106,7 @@ export const NotificationChannelForm = ({
           <Button type="button" variant="secondary" onClick={() => onTestChannel(getValues())}>
             Test
           </Button>
-          <a href={`${config.appSubUrl}/alerting/notifications`}>
+          <a href={`${config.appSubUrl}/alerting-legacy/notifications`}>
             <Button type="button" variant="secondary">
               Back
             </Button>
diff --git a/public/app/features/alerting/routes.tsx b/public/app/features/alerting/routes.tsx
@@ -1,5 +1,6 @@
 import { uniq } from 'lodash';
 import React from 'react';
+import { Redirect, RouteComponentProps } from 'react-router-dom';
 
 import { SafeDynamicImport } from 'app/core/components/DynamicImports/SafeDynamicImport';
 import { NavLandingPage } from 'app/core/components/NavLandingPage/NavLandingPage';
@@ -14,73 +15,79 @@ const commonRoutes: RouteDescriptor[] = [];
 const legacyRoutes: RouteDescriptor[] = [
   ...commonRoutes,
   {
-    path: '/alerting',
+    path: '/alerting-legacy',
     component: () => <NavLandingPage navId="alerting-legacy" />,
   },
   {
-    path: '/alerting/list',
+    path: '/alerting-legacy/list',
     component: SafeDynamicImport(
-      () => import(/* webpackChunkName: "AlertRuleListIndex" */ 'app/features/alerting/AlertRuleList')
+      () => import(/* webpackChunkName: "AlertRuleListLegacyIndex" */ 'app/features/alerting/AlertRuleList')
     ),
   },
   {
-    path: '/alerting/ng/list',
+    path: '/alerting-legacy/ng/list',
     component: SafeDynamicImport(
-      () => import(/* webpackChunkName: "AlertRuleList" */ 'app/features/alerting/AlertRuleList')
+      () => import(/* webpackChunkName: "AlertRuleListLegacy" */ 'app/features/alerting/AlertRuleList')
     ),
   },
   {
-    path: '/alerting/notifications',
+    path: '/alerting-legacy/notifications',
     roles: config.unifiedAlertingEnabled ? () => ['Editor', 'Admin'] : undefined,
     component: SafeDynamicImport(
-      () => import(/* webpackChunkName: "NotificationsListPage" */ 'app/features/alerting/NotificationsListPage')
+      () => import(/* webpackChunkName: "NotificationsListLegacyPage" */ 'app/features/alerting/NotificationsListPage')
     ),
   },
   {
-    path: '/alerting/notifications/templates/new',
+    path: '/alerting-legacy/notifications/templates/new',
     roles: () => ['Editor', 'Admin'],
     component: SafeDynamicImport(
-      () => import(/* webpackChunkName: "NotificationsListPage" */ 'app/features/alerting/NotificationsListPage')
+      () => import(/* webpackChunkName: "NotificationsListLegacyPage" */ 'app/features/alerting/NotificationsListPage')
     ),
   },
   {
-    path: '/alerting/notifications/templates/:id/edit',
+    path: '/alerting-legacy/notifications/templates/:id/edit',
     roles: () => ['Editor', 'Admin'],
     component: SafeDynamicImport(
-      () => import(/* webpackChunkName: "NotificationsListPage" */ 'app/features/alerting/NotificationsListPage')
+      () => import(/* webpackChunkName: "NotificationsListLegacyPage" */ 'app/features/alerting/NotificationsListPage')
     ),
   },
   {
-    path: '/alerting/notifications/receivers/new',
+    path: '/alerting-legacy/notifications/receivers/new',
     roles: () => ['Editor', 'Admin'],
     component: SafeDynamicImport(
-      () => import(/* webpackChunkName: "NotificationsListPage" */ 'app/features/alerting/NotificationsListPage')
+      () => import(/* webpackChunkName: "NotificationsListLegacyPage" */ 'app/features/alerting/NotificationsListPage')
     ),
   },
   {
-    path: '/alerting/notifications/receivers/:id/edit',
+    path: '/alerting-legacy/notifications/receivers/:id/edit',
     roles: () => ['Editor', 'Admin'],
     component: SafeDynamicImport(
-      () => import(/* webpackChunkName: "NotificationsListPage" */ 'app/features/alerting/NotificationsListPage')
+      () => import(/* webpackChunkName: "NotificationsListLegacyPage" */ 'app/features/alerting/NotificationsListPage')
     ),
   },
   {
-    path: '/alerting/notifications/global-config',
+    path: '/alerting-legacy/notifications/global-config',
     roles: () => ['Admin', 'Editor'],
     component: SafeDynamicImport(
-      () => import(/* webpackChunkName: "NotificationsListPage" */ 'app/features/alerting/NotificationsListPage')
+      () => import(/* webpackChunkName: "NotificationsListLegacyPage" */ 'app/features/alerting/NotificationsListPage')
     ),
   },
   {
-    path: '/alerting/notification/new',
+    path: '/alerting-legacy/notification/new',
     component: SafeDynamicImport(
-      () => import(/* webpackChunkName: "NewNotificationChannel" */ 'app/features/alerting/NewNotificationChannelPage')
+      () =>
+        import(
+          /* webpackChunkName: "NewNotificationChannelLegacy" */ 'app/features/alerting/NewNotificationChannelPage'
+        )
     ),
   },
   {
-    path: '/alerting/notification/:id/edit',
+    path: '/alerting-legacy/notification/:id/edit',
     component: SafeDynamicImport(
-      () => import(/* webpackChunkName: "EditNotificationChannel"*/ 'app/features/alerting/EditNotificationChannelPage')
+      () =>
+        import(
+          /* webpackChunkName: "EditNotificationChannelLegacy"*/ 'app/features/alerting/EditNotificationChannelPage'
+        )
     ),
   },
 ];
@@ -295,7 +302,32 @@ export function getAlertingRoutes(cfg = config): RouteDescriptor[] {
   if (cfg.unifiedAlertingEnabled) {
     return unifiedRoutes;
   } else if (cfg.alertingEnabled) {
-    return legacyRoutes;
+    // Redirect old overlapping legacy routes to new separate ones to minimize unintended 404s.
+    const redirects = [
+      {
+        path: '/alerting',
+        component: () => <Redirect to={'/alerting-legacy'} />,
+      },
+      {
+        path: '/alerting/list',
+        component: () => <Redirect to={'/alerting-legacy/list'} />,
+      },
+      {
+        path: '/alerting/notifications',
+        component: () => <Redirect to={'/alerting-legacy/notifications'} />,
+      },
+      {
+        path: '/alerting/notification/new',
+        component: () => <Redirect to={'/alerting-legacy/notification/new'} />,
+      },
+      {
+        path: '/alerting/notification/:id/edit',
+        component: (props: RouteComponentProps<{ id: string }>) => (
+          <Redirect to={'/alerting-legacy/notification/:id/edit'.replace(':id', props.match.params.id)} />
+        ),
+      },
+    ];
+    return [...legacyRoutes, ...redirects];
   }
 
   const uniquePaths = uniq([...legacyRoutes, ...unifiedRoutes].map((route) => route.path));
diff --git a/public/app/features/alerting/state/actions.ts b/public/app/features/alerting/state/actions.ts
@@ -26,7 +26,7 @@ export function createNotificationChannel(data: any): ThunkResult<Promise<void>>
     try {
       await getBackendSrv().post(`/api/alert-notifications`, data);
       dispatch(notifyApp(createSuccessNotification('Notification created')));
-      locationService.push('/alerting/notifications');
+      locationService.push('/alerting-legacy/notifications');
     } catch (error) {
       if (isFetchError(error)) {
         dispatch(notifyApp(createErrorNotification(error.data.error)));
@@ -57,7 +57,7 @@ export function testNotificationChannel(data: any): ThunkResult<void> {
 
 export function loadNotificationTypes(): ThunkResult<void> {
   return async (dispatch) => {
-    const alertNotifiers: NotifierDTO[] = await getBackendSrv().get(`/api/alert-notifiers`);
+    const alertNotifiers: NotifierDTO[] = await getBackendSrv().get(`/api/alert-notifiers-legacy`);
 
     const notificationTypes = alertNotifiers.sort((o1, o2) => {
       if (o1.name > o2.name) {
diff --git a/public/locales/de-DE/grafana.json b/public/locales/de-DE/grafana.json
@@ -671,6 +671,9 @@
       "subtitle": "Serverweite Einstellungen und Zugriff auf Ressourcen wie Organisationen, Benutzer und Lizenzen verwalten",
       "title": "Server-Administrator"
     },
+    "alert-list-legacy": {
+      "title": ""
+    },
     "alerting": {
       "subtitle": "Informiere dich über Probleme in deinen Systemen kurz nach deren Auftreten",
       "title": "Meldungen"
diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json
@@ -671,6 +671,9 @@
       "subtitle": "Manage server-wide settings and access to resources such as organizations, users, and licenses",
       "title": "Server admin"
     },
+    "alert-list-legacy": {
+      "title": "Alert rules"
+    },
     "alerting": {
       "subtitle": "Learn about problems in your systems moments after they occur",
       "title": "Alerting"
diff --git a/public/locales/es-ES/grafana.json b/public/locales/es-ES/grafana.json
@@ -677,6 +677,9 @@
       "subtitle": "Administrar la configuración de todo el servidor y el acceso a recursos como organizaciones, usuarios y licencias",
       "title": "Administrador del servidor"
     },
+    "alert-list-legacy": {
+      "title": ""
+    },
     "alerting": {
       "subtitle": "Conozca los problemas de sus sistemas justo después de que se produzcan",
       "title": "Alertas"
diff --git a/public/locales/fr-FR/grafana.json b/public/locales/fr-FR/grafana.json
@@ -677,6 +677,9 @@
       "subtitle": "Gérer les paramètres à l'échelle du serveur et l'accès aux ressources telles que les organisations, les utilisateurs et les licences",
       "title": "Administrateur de serveur"
     },
+    "alert-list-legacy": {
+      "title": ""
+    },
     "alerting": {
       "subtitle": "En savoir plus sur les problèmes dans vos systèmes quelques instants après qu'ils se produisent",
       "title": "Alertes"
diff --git a/public/locales/pseudo-LOCALE/grafana.json b/public/locales/pseudo-LOCALE/grafana.json
@@ -671,6 +671,9 @@
       "subtitle": "Mäŉäģę şęřvęř-ŵįđę şęŧŧįŉģş äŉđ äččęşş ŧő řęşőūřčęş şūčĥ äş őřģäŉįžäŧįőŉş, ūşęřş, äŉđ ľįčęŉşęş",
       "title": "Ŝęřvęř äđmįŉ"
     },
+    "alert-list-legacy": {
+      "title": "Åľęřŧ řūľęş"
+    },
     "alerting": {
       "subtitle": "Ŀęäřŉ äþőūŧ přőþľęmş įŉ yőūř şyşŧęmş mőmęŉŧş äƒŧęř ŧĥęy őččūř",
       "title": "Åľęřŧįŉģ"
diff --git a/public/locales/zh-Hans/grafana.json b/public/locales/zh-Hans/grafana.json
@@ -665,6 +665,9 @@
       "subtitle": "管理整个服务器范围的设置，以及对组织、用户和许可证等资源的访问权限",
       "title": "服务器管理员"
     },
+    "alert-list-legacy": {
+      "title": ""
+    },
     "alerting": {
       "subtitle": "在系统发生问题后立即获悉",
       "title": "警报"
