feat(alert/report): Added optional CC and BCC fields for email notifi… (#29088)

Co-authored-by: Sivarajan Narayanan <sivarajannarayanan@Sivarajans-MacBook-Pro.local>
Co-authored-by: Sivarajan Narayanan <narayanan_sivarajan@apple.com>

superset-frontend/src/features/alerts/AlertReportModal.tsx

@@ -98,6 +98,7 @@ export interface AlertReportModalProps {
 const DEFAULT_WORKING_TIMEOUT = 3600;
 const DEFAULT_CRON_VALUE = '0 0 * * *'; // every day
 const DEFAULT_RETENTION = 90;
+const EMAIL_REGEX = /^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/;
 
 const DEFAULT_NOTIFICATION_METHODS: NotificationMethodOption[] = [
   NotificationMethodOption.Email,
@@ -372,6 +373,7 @@ export const TRANSLATIONS = {
   WORKING_TIMEOUT_ERROR_TEXT: t('working timeout'),
   RECIPIENTS_ERROR_TEXT: t('recipients'),
   EMAIL_SUBJECT_ERROR_TEXT: t('email subject'),
+  EMAIL_VALIDATION_ERROR_TEXT: t('invalid email'),
   ERROR_TOOLTIP_MESSAGE: t(
     'Not all required fields are complete. Please provide the following:',
   ),
@@ -621,6 +623,8 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
         recipients.push({
           recipient_config_json: {
             target: setting.recipients,
+            ccTarget: setting.cc,
+            bccTarget: setting.bcc,
           },
           type: setting.method,
         });
@@ -1014,6 +1018,31 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
     return hasInfo;
   };
 
+  const checkEmailFormat = () => {
+    if (!notificationSettings.length) {
+      return true;
+    }
+
+    const validateEmails = (emails: string): boolean => {
+      if (!emails) return true; // No emails to validate
+      return emails
+        .split(/[,;]/)
+        .every(email => EMAIL_REGEX.test(email.trim()));
+    };
+
+    // Use array method to check conditions
+    return notificationSettings.every(setting => {
+      if (!!setting.method && setting.method === 'Email') {
+        return (
+          (!setting.recipients?.length || validateEmails(setting.recipients)) &&
+          (!setting.cc || validateEmails(setting.cc)) &&
+          (!setting.bcc || validateEmails(setting.bcc))
+        );
+      }
+      return true; // Non-Email methods are considered valid
+    });
+  };
+
   const validateGeneralSection = () => {
     const errors = [];
     if (!currentAlert?.name?.length) {
@@ -1069,13 +1098,24 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
   };
 
   const validateNotificationSection = () => {
+    const errors = [];
     const hasErrors = !checkNotificationSettings();
-    const errors = hasErrors ? [TRANSLATIONS.RECIPIENTS_ERROR_TEXT] : [];
+
+    if (hasErrors) {
+      errors.push(TRANSLATIONS.RECIPIENTS_ERROR_TEXT);
+    } else {
+      // Check for email format errors
+      const hasValidationErrors = !checkEmailFormat();
+      if (hasValidationErrors) {
+        errors.push(TRANSLATIONS.EMAIL_VALIDATION_ERROR_TEXT);
+      }
+    }
 
     if (emailError) {
       errors.push(TRANSLATIONS.EMAIL_SUBJECT_ERROR_TEXT);
     }
 
+    // Update validation status with combined errors
     updateValidationStatus(Sections.Notification, errors);
   };
 
@@ -1132,6 +1172,8 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
       setNotificationSettings([
         {
           recipients: '',
+          cc: '',
+          bcc: '',
           options: allowedNotificationMethods,
           method: NotificationMethodOption.Email,
         },
@@ -1153,6 +1195,8 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
           // @ts-ignore: Type not assignable
           recipients: config.target || setting.recipient_config_json,
           options: allowedNotificationMethods,
+          cc: config.ccTarget || '',
+          bcc: config.bccTarget || '',
         };
       });
 

superset-frontend/src/features/alerts/components/NotificationMethod.test.tsx

@@ -80,8 +80,8 @@ describe('NotificationMethod', () => {
       />,
     );
 
-    const deleteButton = screen.getByRole('button');
+    const deleteButton = document.querySelector('.delete-button');
-    userEvent.click(deleteButton);
+    if (deleteButton) userEvent.click(deleteButton);
 
     expect(mockOnRemove).toHaveBeenCalledWith(1);
   });

superset-frontend/src/features/alerts/components/NotificationMethod.tsx

@@ -44,34 +44,75 @@ import {
 import { StyledInputContainer } from '../AlertReportModal';
 
 const StyledNotificationMethod = styled.div`
-  margin-bottom: 10px;
+  ${({ theme }) => `
+    margin-bottom: ${theme.gridUnit * 3}px;
 
-  .input-container {
+    .input-container {
-    textarea {
+      textarea {
-      height: auto;
+        height: auto;
-    }
+      }
 
-    &.error {
+      &.error {
-      input {
+        input {
-        border-color: ${({ theme }) => theme.colors.error.base};
+          border-color: ${theme.colors.error.base};
+        }
+      }
+
+      .helper {
+        margin-top: ${theme.gridUnit * 2}px;
+        font-size: ${theme.typography.sizes.s}px;
+        color: ${theme.colors.grayscale.base};
       }
     }
-  }
 
-  .inline-container {
+    .inline-container {
-    margin-bottom: 10px;
+      margin-bottom: ${theme.gridUnit * 2}px;
 
-    > div {
+      > div {
-      margin: 0;
+        margin: 0px;
+      }
+
+      .delete-button {
+        margin-left: ${theme.gridUnit * 2}px;
+        padding-top: ${theme.gridUnit}px;
+      }
     }
 
-    .delete-button {
+    .ghost-button {
-      margin-left: 10px;
+      color: ${theme.colors.primary.dark1};
-      padding-top: 3px;
+      display: inline-flex;
+      align-items: center;
+      font-size: ${theme.typography.sizes.s}px;
+      cursor: pointer;
+      margin-top: ${theme.gridUnit}px;
+
+      .icon {
+        width: ${theme.gridUnit * 3}px;
+        height: ${theme.gridUnit * 3}px;
+        font-size: ${theme.typography.sizes.s}px;
+        margin-right: ${theme.gridUnit}px;
+      }
     }
-  }
+
+    .ghost-button + .ghost-button {
+      margin-left: ${theme.gridUnit * 4}px;
+    }
+
+    .ghost-button:first-child[style*='none'] + .ghost-button {
+      margin-left: 0px; /* Remove margin when the first button is hidden */
+    }
+  `}
 `;
 
+const TRANSLATIONS = {
+  EMAIL_CC_NAME: t('CC recipients'),
+  EMAIL_BCC_NAME: t('BCC recipients'),
+  EMAIL_SUBJECT_NAME: t('Email subject name (optional)'),
+  EMAIL_SUBJECT_ERROR_TEXT: t(
+    'Please enter valid text. Spaces alone are not permitted.',
+  ),
+};
+
 interface NotificationMethodProps {
   setting?: NotificationSetting | null;
   index: number;
@@ -85,13 +126,6 @@ interface NotificationMethodProps {
   setErrorSubject: (hasError: boolean) => void;
 }
 
-const TRANSLATIONS = {
-  EMAIL_SUBJECT_NAME: t('Email subject name (optional)'),
-  EMAIL_SUBJECT_ERROR_TEXT: t(
-    'Please enter valid text. Spaces alone are not permitted.',
-  ),
-};
-
 export const mapSlackValues = ({
   method,
   recipientValue,
@@ -164,7 +198,7 @@ export const NotificationMethod: FunctionComponent<NotificationMethodProps> = ({
   defaultSubject,
   setErrorSubject,
 }) => {
-  const { method, recipients, options } = setting || {};
+  const { method, recipients, cc, bcc, options } = setting || {};
   const [recipientValue, setRecipientValue] = useState<string>(
     recipients || '',
   );
@@ -172,6 +206,10 @@ export const NotificationMethod: FunctionComponent<NotificationMethodProps> = ({
     { label: string; value: string }[]
   >([]);
   const [error, setError] = useState(false);
+  const [ccVisible, setCcVisible] = useState<boolean>(!!cc);
+  const [bccVisible, setBccVisible] = useState<boolean>(!!bcc);
+  const [ccValue, setCcValue] = useState<string>(cc || '');
+  const [bccValue, setBccValue] = useState<string>(bcc || '');
   const theme = useTheme();
   const [slackOptions, setSlackOptions] = useState<SlackOptionsType>([
     {
@@ -188,11 +226,16 @@ export const NotificationMethod: FunctionComponent<NotificationMethodProps> = ({
   }) => {
     // Since we're swapping the method, reset the recipients
     setRecipientValue('');
+    setCcValue('');
+    setBccValue('');
+
     if (onUpdate && setting) {
       const updatedSetting = {
         ...setting,
         method: selected.value,
         recipients: '',
+        cc: '',
+        bcc: '',
       };
 
       onUpdate(index, updatedSetting);
@@ -333,11 +376,49 @@ export const NotificationMethod: FunctionComponent<NotificationMethodProps> = ({
     }
   };
 
+  const onCcChange = (event: React.ChangeEvent<HTMLTextAreaElement>) => {
+    const { target } = event;
+
+    setCcValue(target.value);
+
+    if (onUpdate) {
+      const updatedSetting = {
+        ...setting,
+        cc: target.value,
+      };
+
+      onUpdate(index, updatedSetting);
+    }
+  };
+
+  const onBccChange = (event: React.ChangeEvent<HTMLTextAreaElement>) => {
+    const { target } = event;
+
+    setBccValue(target.value);
+
+    if (onUpdate) {
+      const updatedSetting = {
+        ...setting,
+        bcc: target.value,
+      };
+
+      onUpdate(index, updatedSetting);
+    }
+  };
+
   // Set recipients
   if (!!recipients && recipientValue !== recipients) {
     setRecipientValue(recipients);
   }
 
+  if (!!cc && ccValue !== cc) {
+    setCcValue(cc);
+  }
+
+  if (!!bcc && bccValue !== bcc) {
+    setBccValue(bcc);
+  }
+
   return (
     <StyledNotificationMethod>
       <div className="inline-container">
@@ -418,14 +499,16 @@ export const NotificationMethod: FunctionComponent<NotificationMethodProps> = ({
                   <>
                     <div className="input-container">
                       <textarea
-                        name="recipients"
+                        name="To"
                         data-test="recipients"
                         value={recipientValue}
                         onChange={onRecipientsChange}
                       />
                     </div>
-                    <div className="helper">
+                    <div className="input-container">
-                      {t('Recipients are separated by "," or ";"')}
+                      <div className="helper">
+                        {t('Recipients are separated by "," or ";"')}
+                      </div>
                     </div>
                   </>
                 ) : (
@@ -446,6 +529,75 @@ export const NotificationMethod: FunctionComponent<NotificationMethodProps> = ({
               </div>
             </StyledInputContainer>
           </div>
+          {method === NotificationMethodOption.Email && (
+            <StyledInputContainer>
+              {/* Render "CC" input field if ccVisible is true */}
+              {ccVisible && (
+                <>
+                  <div className="control-label">
+                    {TRANSLATIONS.EMAIL_CC_NAME}
+                  </div>
+                  <div className="input-container">
+                    <textarea
+                      name="CC"
+                      data-test="cc"
+                      value={ccValue}
+                      onChange={onCcChange}
+                    />
+                  </div>
+                  <div className="input-container">
+                    <div className="helper">
+                      {t('Recipients are separated by "," or ";"')}
+                    </div>
+                  </div>
+                </>
+              )}
+              {/* Render "BCC" input field if bccVisible is true */}
+              {bccVisible && (
+                <>
+                  <div className="control-label">
+                    {TRANSLATIONS.EMAIL_BCC_NAME}
+                  </div>
+                  <div className="input-container">
+                    <textarea
+                      name="BCC"
+                      data-test="bcc"
+                      value={bccValue}
+                      onChange={onBccChange}
+                    />
+                  </div>
+                  <div className="input-container">
+                    <div className="helper">
+                      {t('Recipients are separated by "," or ";"')}
+                    </div>
+                  </div>
+                </>
+              )}
+              {/* New buttons container */}
+              <div className="ghost-button">
+                <span
+                  className="ghost-button"
+                  role="button"
+                  tabIndex={0}
+                  onClick={() => setCcVisible(true)}
+                  style={{ display: ccVisible ? 'none' : 'inline-flex' }}
+                >
+                  <Icons.Email className="icon" />
+                  {t('Add CC Recipients')}
+                </span>
+                <span
+                  className="ghost-button"
+                  role="button"
+                  tabIndex={0}
+                  onClick={() => setBccVisible(true)}
+                  style={{ display: bccVisible ? 'none' : 'inline-flex' }}
+                >
+                  <Icons.Email className="icon" />
+                  {t('Add BCC Recipients')}
+                </span>
+              </div>
+            </StyledInputContainer>
+          )}
         </>
       ) : null}
     </StyledNotificationMethod>

superset-frontend/src/features/alerts/types.ts

@@ -50,6 +50,8 @@ export enum NotificationMethodOption {
 export type NotificationSetting = {
   method?: NotificationMethodOption;
   recipients: string;
+  cc?: string;
+  bcc?: string;
   options: NotificationMethodOption[];
 };
 
@@ -63,6 +65,8 @@ export type SlackChannel = {
 export type Recipient = {
   recipient_config_json: {
     target: string;
+    ccTarget?: string;
+    bccTarget?: string;
   };
   type: NotificationMethodOption;
 };

superset-frontend/src/features/reports/ReportModal/index.tsx

@@ -73,6 +73,8 @@ interface ReportProps {
   show: boolean;
   userId: number;
   userEmail: string;
+  ccEmail: string;
+  bccEmail: string;
   chart?: ChartState;
   chartName?: string;
   dashboardId?: number;
@@ -109,6 +111,8 @@ function ReportModal({
   chart,
   userId,
   userEmail,
+  ccEmail,
+  bccEmail,
   creationMethod,
   dashboardName,
   chartName,
@@ -184,7 +188,11 @@ function ReportModal({
       owners: [userId],
       recipients: [
         {
-          recipient_config_json: { target: userEmail },
+          recipient_config_json: {
+            target: userEmail,
+            ccTarget: ccEmail,
+            bccTarget: bccEmail,
+          },
           type: 'Email',
         },
       ],

superset-frontend/src/features/reports/types.ts

@@ -46,7 +46,14 @@ export interface ReportObject {
   name: string;
   owners: number[];
   recipients: [
-    { recipient_config_json: { target: string }; type: ReportRecipientType },
+    {
+      recipient_config_json: {
+        target: string;
+        ccTarget: string;
+        bccTarget: string;
+      };
+      type: ReportRecipientType;
+    },
   ];
   report_format: string;
   timezone: string;

superset/reports/notifications/email.py

@@ -193,11 +193,22 @@ class EmailNotification(BaseNotification):  # pylint: disable=too-few-public-met
     def _get_to(self) -> str:
         return json.loads(self._recipient.recipient_config_json)["target"]
 
+    def _get_cc(self) -> str:
+        # To accomadate backward compatability
+        return json.loads(self._recipient.recipient_config_json).get("ccTarget", "")
+
+    def _get_bcc(self) -> str:
+        # To accomadate backward compatability
+        return json.loads(self._recipient.recipient_config_json).get("bccTarget", "")
+
     @statsd_gauge("reports.email.send")
     def send(self) -> None:
         subject = self._get_subject()
         content = self._get_content()
         to = self._get_to()
+        cc = self._get_cc()
+        bcc = self._get_bcc()
+
         try:
             send_email_smtp(
                 to,
@@ -208,9 +219,10 @@ class EmailNotification(BaseNotification):  # pylint: disable=too-few-public-met
                 data=content.data,
                 pdf=content.pdf,
                 images=content.images,
-                bcc="",
                 mime_subtype="related",
                 dryrun=False,
+                cc=cc,
+                bcc=bcc,
                 header_data=content.header_data,
             )
             logger.info(

superset/reports/schemas.py

@@ -123,6 +123,8 @@ class ValidatorConfigJSONSchema(Schema):
 class ReportRecipientConfigJSONSchema(Schema):
     # TODO if email check validity
     target = fields.String()
+    ccTarget = fields.String()
+    bccTarget = fields.String()
 
 
 class ReportRecipientSchema(Schema):

superset/utils/core.py

@@ -711,7 +711,7 @@ def send_email_smtp(  # pylint: disable=invalid-name,too-many-arguments,too-many
     recipients = smtp_mail_to
     if cc:
         smtp_mail_cc = get_email_address_list(cc)
-        msg["CC"] = ", ".join(smtp_mail_cc)
+        msg["Cc"] = ", ".join(smtp_mail_cc)
         recipients = recipients + smtp_mail_cc
 
     smtp_mail_bcc = []

tests/integration_tests/reports/commands_tests.py

@@ -108,6 +108,20 @@ def get_target_from_report_schedule(report_schedule: ReportSchedule) -> list[str
     ]
 
 
+def get_cctarget_from_report_schedule(report_schedule: ReportSchedule) -> list[str]:
+    return [
+        json.loads(recipient.recipient_config_json).get("ccTarget", "")
+        for recipient in report_schedule.recipients
+    ]
+
+
+def get_bcctarget_from_report_schedule(report_schedule: ReportSchedule) -> list[str]:
+    return [
+        json.loads(recipient.recipient_config_json).get("bccTarget", "")
+        for recipient in report_schedule.recipients
+    ]
+
+
 def get_error_logs_query(report_schedule: ReportSchedule) -> BaseQuery:
     return (
         db.session.query(ReportExecutionLog)
@@ -172,6 +186,20 @@ def create_report_email_chart():
     cleanup_report_schedule(report_schedule)
 
 
+@pytest.fixture()
+def create_report_email_chart_with_cc_and_bcc():
+    chart = db.session.query(Slice).first()
+    report_schedule = create_report_notification(
+        email_target="target@email.com",
+        ccTarget="cc@email.com",
+        bccTarget="bcc@email.com",
+        chart=chart,
+    )
+    yield report_schedule
+
+    cleanup_report_schedule(report_schedule)
+
+
 @pytest.fixture()
 def create_report_email_chart_alpha_owner(get_user):
     owners = [get_user("alpha")]
@@ -617,6 +645,73 @@ def create_invalid_sql_alert_email_chart(request, app_context: AppContext):
         cleanup_report_schedule(report_schedule)
 
 
+@pytest.mark.usefixtures(
+    "load_birth_names_dashboard_with_slices",
+    "create_report_email_chart_with_cc_and_bcc",
+)
+@patch("superset.reports.notifications.email.send_email_smtp")
+@patch("superset.utils.screenshots.ChartScreenshot.get_screenshot")
+def test_email_chart_report_schedule_with_cc_bcc(
+    screenshot_mock,
+    email_mock,
+    create_report_email_chart_with_cc_and_bcc,
+):
+    """
+    ExecuteReport Command: Test chart email report schedule with screenshot and email cc, bcc options
+    """
+    # setup screenshot mock
+    screenshot_mock.return_value = SCREENSHOT_FILE
+
+    with freeze_time("2020-01-01T00:00:00Z"):
+        AsyncExecuteReportScheduleCommand(
+            TEST_ID, create_report_email_chart_with_cc_and_bcc.id, datetime.utcnow()
+        ).run()
+
+        notification_targets = get_target_from_report_schedule(
+            create_report_email_chart_with_cc_and_bcc
+        )
+
+        notification_cctargets = get_cctarget_from_report_schedule(
+            create_report_email_chart_with_cc_and_bcc
+        )
+
+        notification_bcctargets = get_bcctarget_from_report_schedule(
+            create_report_email_chart_with_cc_and_bcc
+        )
+
+        # assert that the link sent is correct
+        assert (
+            '<a href="http://0.0.0.0:8080/explore/?form_data=%7B%22slice_id%22:+'
+            f"{create_report_email_chart_with_cc_and_bcc.chart.id}"
+            '%7D&force=false">Explore in Superset</a>' in email_mock.call_args[0][2]
+        )
+        # Assert the email smtp address
+        if notification_targets:
+            assert email_mock.call_args[0][0] == notification_targets[0]
+
+        # Assert the cc recipients if provided
+        if notification_cctargets:
+            expected_cc_targets = [target.strip() for target in notification_cctargets]
+            assert (
+                email_mock.call_args[1].get("cc", "").split(",") == expected_cc_targets
+            )
+
+        if notification_bcctargets:
+            expected_bcc_targets = [
+                target.strip() for target in notification_bcctargets
+            ]
+            assert (
+                email_mock.call_args[1].get("bcc", "").split(",")
+                == expected_bcc_targets
+            )
+
+        # Assert the email inline screenshot
+        smtp_images = email_mock.call_args[1]["images"]
+        assert smtp_images[list(smtp_images.keys())[0]] == SCREENSHOT_FILE
+        # Assert logs are correct
+        assert_log(ReportState.SUCCESS)
+
+
 @pytest.mark.usefixtures(
     "load_birth_names_dashboard_with_slices", "create_report_email_chart"
 )

tests/integration_tests/reports/utils.py

@@ -116,6 +116,8 @@ def create_report_notification(
     extra: Optional[dict[str, Any]] = None,
     force_screenshot: bool = False,
     owners: Optional[list[User]] = None,
+    ccTarget: Optional[str] = None,
+    bccTarget: Optional[str] = None,
 ) -> ReportSchedule:
     if not owners:
         owners = [
@@ -138,7 +140,9 @@ def create_report_notification(
     else:
         recipient = ReportRecipients(
             type=ReportRecipientType.EMAIL,
-            recipient_config_json=json.dumps({"target": email_target}),
+            recipient_config_json=json.dumps(
+                {"target": email_target, "ccTarget": ccTarget, "bccTarget": bccTarget}
+            ),
         )
 
     if name is None: