fix(alerts/reports): removing duplicate notification method options (a…

…pache#27239)
EnxDev · Apr 15, 2024 · e059634 · e059634
1 parent bde267e
commit e059634
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 26 deletions.
diff --git a/superset-frontend/src/features/alerts/AlertReportModal.tsx b/superset-frontend/src/features/alerts/AlertReportModal.tsx
@@ -50,6 +50,7 @@ import { useCommonConf } from 'src/features/databases/state';
 import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls';
 import {
   NotificationMethodOption,
+  NotificationSetting,
   AlertObject,
   ChartObject,
   DashboardObject,
@@ -395,12 +396,6 @@ const NotificationMethodAdd: FunctionComponent<NotificationMethodAddProps> = ({
   );
 };
 
-type NotificationSetting = {
-  method?: NotificationMethodOption;
-  recipients: string;
-  options: NotificationMethodOption[];
-};
-
 const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
   addDangerToast,
   onAdd,
@@ -497,15 +492,26 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
     NotificationSetting[]
   >([]);
   const onNotificationAdd = () => {
-    const settings: NotificationSetting[] = notificationSettings.slice();
-    settings.push({
-      recipients: '',
-      options: allowedNotificationMethods,
-    });
+    setNotificationSettings([
+      ...notificationSettings,
+      {
+        recipients: '',
+        // options shown in the newly added notification method
+        options: allowedNotificationMethods.filter(
+          // are filtered such that
+          option =>
+            // options are not included
+            !notificationSettings.reduce(
+              // when it exists in previous notificationSettings
+              (accum, setting) => accum || option === setting.method,
+              false,
+            ),
+        ),
+      },
+    ]);
 
-    setNotificationSettings(settings);
     setNotificationAddState(
-      settings.length === allowedNotificationMethods.length
+      notificationSettings.length === allowedNotificationMethods.length
         ? 'hidden'
         : 'disabled',
     );
@@ -547,13 +553,20 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
     index: number,
     setting: NotificationSetting,
   ) => {
-    const settings = notificationSettings.slice();
+    // if you've changed notification method
+    if (notificationSettings[index].method !== setting.method) {
+      notificationSettings[index] = setting;
 
-    settings[index] = setting;
-    setNotificationSettings(settings);
+      setNotificationSettings(
+        notificationSettings.filter((_, idx) => idx <= index),
+      );
+      if (notificationSettings.length - 1 > index) {
+        setNotificationAddState('active');
+      }
 
-    if (setting.method !== undefined && notificationAddState !== 'hidden') {
-      setNotificationAddState('active');
+      if (setting.method !== undefined && notificationAddState !== 'hidden') {
+        setNotificationAddState('active');
+      }
     }
   };
 

diff --git a/superset-frontend/src/features/alerts/components/NotificationMethod.tsx b/superset-frontend/src/features/alerts/components/NotificationMethod.tsx
@@ -20,7 +20,7 @@ import React, { FunctionComponent, useState } from 'react';
 import { styled, t, useTheme } from '@superset-ui/core';
 import { Select } from 'src/components';
 import Icons from 'src/components/Icons';
-import { NotificationMethodOption } from '../types';
+import { NotificationMethodOption, NotificationSetting } from '../types';
 import { StyledInputContainer } from '../AlertReportModal';
 
 const StyledNotificationMethod = styled.div`
@@ -46,12 +46,6 @@ const StyledNotificationMethod = styled.div`
   }
 `;
 
-type NotificationSetting = {
-  method?: NotificationMethodOption;
-  recipients: string;
-  options: NotificationMethodOption[];
-};
-
 interface NotificationMethodProps {
   setting?: NotificationSetting | null;
   index: number;
@@ -130,7 +124,7 @@ export const NotificationMethod: FunctionComponent<NotificationMethodProps> = ({
               )}
               value={method}
             />
-            {method !== undefined && index !== 0 && !!onRemove ? (
+            {index !== 0 && !!onRemove ? (
               <span
                 role="button"
                 tabIndex={0}

diff --git a/superset-frontend/src/features/alerts/types.ts b/superset-frontend/src/features/alerts/types.ts
@@ -43,6 +43,12 @@ export type DatabaseObject = {
 
 export type NotificationMethodOption = 'Email' | 'Slack';
 
+export type NotificationSetting = {
+  method?: NotificationMethodOption;
+  recipients: string;
+  options: NotificationMethodOption[];
+};
+
 export type Recipient = {
   recipient_config_json: {
     target: string;