fix: Disable Slack notification method if no api token (#16367)

* feat: allow company choose alert/report notification methods

* fix: disable Slack as notification method if no api token

* fix unit test

* improve typing

superset-frontend/src/views/CRUD/alert/AlertReportModal.test.jsx

@@ -326,9 +326,10 @@ describe('AlertReportModal', () => {
     });
     await waitForComponentToPaint(wrapper);
 
+    // use default config: only show Email as notification option.
     expect(
       wrapper.find('[data-test="notification-add"]').props().status,
-    ).toEqual('disabled');
+    ).toEqual('hidden');
     act(() => {
       wrapper
         .find('[data-test="select-delivery-method"]')

superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx

@@ -43,10 +43,9 @@ import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags';
 import withToasts from 'src/messageToasts/enhancers/withToasts';
 import Owner from 'src/types/Owner';
 import TextAreaControl from 'src/explore/components/controls/TextAreaControl';
-import { AlertReportCronScheduler } from './components/AlertReportCronScheduler';
+import { useCommonConf } from 'src/views/CRUD/data/database/state';
-import { NotificationMethod } from './components/NotificationMethod';
-
 import {
+  NotificationMethodOption,
   AlertObject,
   ChartObject,
   DashboardObject,
@@ -54,7 +53,9 @@ import {
   MetaObject,
   Operator,
   Recipient,
-} from './types';
+} from 'src/views/CRUD/alert/types';
+import { AlertReportCronScheduler } from './components/AlertReportCronScheduler';
+import { NotificationMethod } from './components/NotificationMethod';
 
 const TIMEOUT_MIN = 1;
 const TEXT_BASED_VISUALIZATION_TYPES = [
@@ -78,7 +79,7 @@ interface AlertReportModalProps {
   show: boolean;
 }
 
-const NOTIFICATION_METHODS: NotificationMethod[] = ['Email', 'Slack'];
+const DEFAULT_NOTIFICATION_METHODS: NotificationMethodOption[] = ['Email'];
 const DEFAULT_NOTIFICATION_FORMAT = 'PNG';
 const CONDITIONS = [
   {
@@ -381,12 +382,10 @@ const NotificationMethodAdd: FunctionComponent<NotificationMethodAddProps> = ({
   );
 };
 
-type NotificationMethod = 'Email' | 'Slack';
-
 type NotificationSetting = {
-  method?: NotificationMethod;
+  method?: NotificationMethodOption;
   recipients: string;
-  options: NotificationMethod[];
+  options: NotificationMethodOption[];
 };
 
 const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
@@ -397,6 +396,10 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
   alert = null,
   isReport = false,
 }) => {
+  const conf = useCommonConf();
+  const allowedNotificationMethods: NotificationMethodOption[] =
+    conf?.ALERT_REPORTS_NOTIFICATION_METHODS || DEFAULT_NOTIFICATION_METHODS;
+
   const [disableSave, setDisableSave] = useState<boolean>(true);
   const [
     currentAlert,
@@ -435,12 +438,14 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
 
     settings.push({
       recipients: '',
-      options: NOTIFICATION_METHODS, // TODO: Need better logic for this
+      options: allowedNotificationMethods,
     });
 
     setNotificationSettings(settings);
     setNotificationAddState(
-      settings.length === NOTIFICATION_METHODS.length ? 'hidden' : 'disabled',
+      settings.length === allowedNotificationMethods.length
+        ? 'hidden'
+        : 'disabled',
     );
   };
 
@@ -910,16 +915,18 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
             ? JSON.parse(setting.recipient_config_json)
             : {};
         return {
-          method: setting.type as NotificationMethod,
+          method: setting.type,
           // @ts-ignore: Type not assignable
           recipients: config.target || setting.recipient_config_json,
-          options: NOTIFICATION_METHODS as NotificationMethod[], // Need better logic for this
+          options: allowedNotificationMethods,
         };
       });
 
       setNotificationSettings(settings);
       setNotificationAddState(
-        settings.length === NOTIFICATION_METHODS.length ? 'hidden' : 'active',
+        settings.length === allowedNotificationMethods.length
+          ? 'hidden'
+          : 'active',
       );
       setContentType(resource.chart ? 'chart' : 'dashboard');
       setReportFormat(
@@ -1309,6 +1316,7 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
               <NotificationMethod
                 setting={notificationSetting}
                 index={i}
+                key={`NotificationMethod-${i}`}
                 onUpdate={updateNotificationSetting}
                 onRemove={removeNotificationSetting}
               />

superset-frontend/src/views/CRUD/alert/components/NotificationMethod.tsx

@@ -20,6 +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 'src/views/CRUD/alert/types';
 import { StyledInputContainer } from '../AlertReportModal';
 
 const StyledNotificationMethod = styled.div`
@@ -49,12 +50,10 @@ const StyledNotificationMethod = styled.div`
   }
 `;
 
-type NotificationMethod = 'Email' | 'Slack';
-
 type NotificationSetting = {
-  method?: NotificationMethod;
+  method?: NotificationMethodOption;
   recipients: string;
-  options: NotificationMethod[];
+  options: NotificationMethodOption[];
 };
 
 interface NotificationMethodProps {
@@ -80,7 +79,7 @@ export const NotificationMethod: FunctionComponent<NotificationMethodProps> = ({
     return null;
   }
 
-  const onMethodChange = (method: NotificationMethod) => {
+  const onMethodChange = (method: NotificationMethodOption) => {
     // Since we're swapping the method, reset the recipients
     setRecipientValue('');
     if (onUpdate) {
@@ -126,10 +125,12 @@ export const NotificationMethod: FunctionComponent<NotificationMethodProps> = ({
               data-test="select-delivery-method"
               onChange={onMethodChange}
               placeholder={t('Select Delivery Method')}
-              options={(options || []).map((method: NotificationMethod) => ({
+              options={(options || []).map(
-                label: method,
+                (method: NotificationMethodOption) => ({
-                value: method,
+                  label: method,
-              }))}
+                  value: method,
+                }),
+              )}
               value={method}
             />
           </div>

superset-frontend/src/views/CRUD/alert/types.ts

@@ -40,11 +40,13 @@ export type DatabaseObject = {
   id: number;
 };
 
+export type NotificationMethodOption = 'Email' | 'Slack';
+
 export type Recipient = {
   recipient_config_json: {
     target: string;
   };
-  type: string;
+  type: NotificationMethodOption;
 };
 
 export type MetaObject = {

superset-frontend/src/views/CRUD/data/database/state.ts

@@ -21,5 +21,5 @@ import { useSelector } from 'react-redux';
 import { ViewState } from 'src/views/types';
 
 export function useCommonConf() {
-  return useSelector((state: ViewState) => state.common.conf);
+  return useSelector((state: ViewState) => state?.common?.conf);
 }

superset-frontend/src/views/types.ts

@@ -16,11 +16,14 @@
  * specific language governing permissions and limitations
  * under the License.
  */
+import { NotificationMethodOption } from './CRUD/alert/types';
+
 export interface ViewState {
   common: {
     conf: {
       SQLALCHEMY_DOCS_URL: string;
       SQLALCHEMY_DISPLAY_TEXT: string;
+      ALERT_REPORTS_NOTIFICATION_METHODS: NotificationMethodOption[];
     };
   };
   messageToast: Array<Object>;

superset/views/base.py

@@ -70,6 +70,7 @@ from superset.exceptions import (
     SupersetSecurityException,
 )
 from superset.models.helpers import ImportExportMixin
+from superset.models.reports import ReportRecipientType
 from superset.translations.utils import get_language_pack
 from superset.typing import FlaskResponse
 from superset.utils import core as utils
@@ -346,9 +347,21 @@ def common_bootstrap_payload() -> Dict[str, Any]:
     messages = get_flashed_messages(with_categories=True)
     locale = str(get_locale())
 
+    # should not expose API TOKEN to frontend
+    frontend_config = {k: conf.get(k) for k in FRONTEND_CONF_KEYS}
+    if conf.get("SLACK_API_TOKEN"):
+        frontend_config["ALERT_REPORTS_NOTIFICATION_METHODS"] = [
+            ReportRecipientType.EMAIL,
+            ReportRecipientType.SLACK,
+        ]
+    else:
+        frontend_config["ALERT_REPORTS_NOTIFICATION_METHODS"] = [
+            ReportRecipientType.EMAIL,
+        ]
+
     bootstrap_data = {
         "flash_messages": messages,
-        "conf": {k: conf.get(k) for k in FRONTEND_CONF_KEYS},
+        "conf": frontend_config,
         "locale": locale,
         "language_pack": get_language_pack(locale),
         "feature_flags": get_feature_flags(),