From 7a0c5421e3f20846f509f2b6c51f6beda9a73cd1 Mon Sep 17 00:00:00 2001
From: Puridach wutthihathaithamrong
 <76474644+puridach-w@users.noreply.github.com>
Date: Thu, 16 May 2024 20:04:42 +0700
Subject: [PATCH] feat: Customizable email subject name (#26327)

Co-authored-by: Puridach Wutthihathaithamrong <>
---
 .../src/features/alerts/AlertReportModal.tsx  |  46 ++++++++
 .../alerts/components/NotificationMethod.tsx  | 104 +++++++++++++++---
 .../src/features/alerts/types.ts              |   1 +
 superset/commands/report/execute.py           |  21 ++--
 ...b_add_subject_column_to_report_schedule.py |  41 +++++++
 superset/reports/api.py                       |   1 +
 superset/reports/models.py                    |   2 +
 superset/reports/schemas.py                   |  17 +++
 8 files changed, 209 insertions(+), 24 deletions(-)
 create mode 100644 superset/migrations/versions/2024-05-10_11-09_9621c6d56ffb_add_subject_column_to_report_schedule.py

diff --git a/superset-frontend/src/features/alerts/AlertReportModal.tsx b/superset-frontend/src/features/alerts/AlertReportModal.tsx
index fd0cf7eda9592..ac2f6a56500b3 100644
--- a/superset-frontend/src/features/alerts/AlertReportModal.tsx
+++ b/superset-frontend/src/features/alerts/AlertReportModal.tsx
@@ -367,6 +367,7 @@ export const TRANSLATIONS = {
   CRONTAB_ERROR_TEXT: t('crontab'),
   WORKING_TIMEOUT_ERROR_TEXT: t('working timeout'),
   RECIPIENTS_ERROR_TEXT: t('recipients'),
+  EMAIL_SUBJECT_ERROR_TEXT: t('email subject'),
   ERROR_TOOLTIP_MESSAGE: t(
     'Not all required fields are complete. Please provide the following:',
   ),
@@ -491,6 +492,9 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
   const [notificationSettings, setNotificationSettings] = useState<
     NotificationSetting[]
   >([]);
+  const [emailSubject, setEmailSubject] = useState<string>('');
+  const [emailError, setEmailError] = useState(false);
+
   const onNotificationAdd = () => {
     setNotificationSettings([
       ...notificationSettings,
@@ -543,6 +547,7 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
     owners: [],
     recipients: [],
     sql: '',
+    email_subject: '',
     validator_config_json: {},
     validator_type: '',
     force_screenshot: false,
@@ -888,6 +893,10 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
     const parsedValue = type === 'number' ? parseInt(value, 10) || null : value;
 
     updateAlertState(name, parsedValue);
+
+    if (name === 'name') {
+      updateEmailSubject();
+    }
   };
 
   const onCustomWidthChange = (value: number | null | undefined) => {
@@ -1058,6 +1067,11 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
   const validateNotificationSection = () => {
     const hasErrors = !checkNotificationSettings();
     const errors = hasErrors ? [TRANSLATIONS.RECIPIENTS_ERROR_TEXT] : [];
+
+    if (emailError) {
+      errors.push(TRANSLATIONS.EMAIL_SUBJECT_ERROR_TEXT);
+    }
+
     updateValidationStatus(Sections.Notification, errors);
   };
 
@@ -1199,6 +1213,7 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
   const currentAlertSafe = currentAlert || {};
   useEffect(() => {
     validateAll();
+    updateEmailSubject();
   }, [
     currentAlertSafe.name,
     currentAlertSafe.owners,
@@ -1212,6 +1227,7 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
     contentType,
     notificationSettings,
     conditionNotNull,
+    emailError,
   ]);
   useEffect(() => {
     enforceValidation();
@@ -1243,6 +1259,32 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
     return titleText;
   };
 
+  const updateEmailSubject = () => {
+    if (contentType === 'chart') {
+      if (currentAlert?.name || currentAlert?.chart?.label) {
+        setEmailSubject(
+          `${currentAlert?.name}: ${currentAlert?.chart?.label || ''}`,
+        );
+      } else {
+        setEmailSubject('');
+      }
+    } else if (contentType === 'dashboard') {
+      if (currentAlert?.name || currentAlert?.dashboard?.label) {
+        setEmailSubject(
+          `${currentAlert?.name}: ${currentAlert?.dashboard?.label || ''}`,
+        );
+      } else {
+        setEmailSubject('');
+      }
+    } else {
+      setEmailSubject('');
+    }
+  };
+
+  const handleErrorUpdate = (hasError: boolean) => {
+    setEmailError(hasError);
+  };
+
   return (
     <StyledModal
       className="no-content-padding"
@@ -1690,6 +1732,10 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
                 key={`NotificationMethod-${i}`}
                 onUpdate={updateNotificationSetting}
                 onRemove={removeNotificationSetting}
+                onInputChange={onInputChange}
+                email_subject={currentAlert?.email_subject || ''}
+                defaultSubject={emailSubject || ''}
+                setErrorSubject={handleErrorUpdate}
               />
             </StyledNotificationMethodWrapper>
           ))}
diff --git a/superset-frontend/src/features/alerts/components/NotificationMethod.tsx b/superset-frontend/src/features/alerts/components/NotificationMethod.tsx
index b50988700ef4e..614018c12a171 100644
--- a/superset-frontend/src/features/alerts/components/NotificationMethod.tsx
+++ b/superset-frontend/src/features/alerts/components/NotificationMethod.tsx
@@ -30,6 +30,12 @@ const StyledNotificationMethod = styled.div`
     textarea {
       height: auto;
     }
+
+    &.error {
+      input {
+        border-color: ${({ theme }) => theme.colors.error.base};
+      }
+    }
   }
 
   .inline-container {
@@ -51,18 +57,36 @@ interface NotificationMethodProps {
   index: number;
   onUpdate?: (index: number, updatedSetting: NotificationSetting) => void;
   onRemove?: (index: number) => void;
+  onInputChange?: (
+    event: React.ChangeEvent<HTMLTextAreaElement | HTMLInputElement>,
+  ) => void;
+  email_subject: string;
+  defaultSubject: string;
+  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 NotificationMethod: FunctionComponent<NotificationMethodProps> = ({
   setting = null,
   index,
   onUpdate,
   onRemove,
+  onInputChange,
+  email_subject,
+  defaultSubject,
+  setErrorSubject,
 }) => {
   const { method, recipients, options } = setting || {};
   const [recipientValue, setRecipientValue] = useState<string>(
     recipients || '',
   );
+  const [error, setError] = useState(false);
   const theme = useTheme();
 
   if (!setting) {
@@ -100,6 +124,22 @@ export const NotificationMethod: FunctionComponent<NotificationMethodProps> = ({
     }
   };
 
+  const onSubjectChange = (
+    event: React.ChangeEvent<HTMLTextAreaElement | HTMLInputElement>,
+  ) => {
+    const { value } = event.target;
+
+    if (onInputChange) {
+      onInputChange(event);
+    }
+
+    const hasError = value.length > 0 && value.trim().length === 0;
+    setError(hasError);
+    if (setErrorSubject) {
+      setErrorSubject(hasError);
+    }
+  };
+
   // Set recipients
   if (!!recipients && recipientValue !== recipients) {
     setRecipientValue(recipients);
@@ -138,23 +178,57 @@ export const NotificationMethod: FunctionComponent<NotificationMethodProps> = ({
         </StyledInputContainer>
       </div>
       {method !== undefined ? (
-        <StyledInputContainer>
-          <div className="control-label">
-            {t('%s recipients', method)}
-            <span className="required">*</span>
+        <>
+          <div className="inline-container">
+            <StyledInputContainer>
+              {method === 'Email' ? (
+                <>
+                  <div className="control-label">
+                    {TRANSLATIONS.EMAIL_SUBJECT_NAME}
+                  </div>
+                  <div className={`input-container ${error ? 'error' : ''}`}>
+                    <input
+                      type="text"
+                      name="email_subject"
+                      value={email_subject}
+                      placeholder={defaultSubject}
+                      onChange={onSubjectChange}
+                    />
+                  </div>
+                  {error && (
+                    <div
+                      style={{
+                        color: theme.colors.error.base,
+                        fontSize: theme.gridUnit * 3,
+                      }}
+                    >
+                      {TRANSLATIONS.EMAIL_SUBJECT_ERROR_TEXT}
+                    </div>
+                  )}
+                </>
+              ) : null}
+            </StyledInputContainer>
           </div>
-          <div className="input-container">
-            <textarea
-              name="recipients"
-              data-test="recipients"
-              value={recipientValue}
-              onChange={onRecipientsChange}
-            />
+          <div className="inline-container">
+            <StyledInputContainer>
+              <div className="control-label">
+                {t('%s recipients', method)}
+                <span className="required">*</span>
+              </div>
+              <div className="input-container">
+                <textarea
+                  name="recipients"
+                  data-test="recipients"
+                  value={recipientValue}
+                  onChange={onRecipientsChange}
+                />
+              </div>
+              <div className="helper">
+                {t('Recipients are separated by "," or ";"')}
+              </div>
+            </StyledInputContainer>
           </div>
-          <div className="helper">
-            {t('Recipients are separated by "," or ";"')}
-          </div>
-        </StyledInputContainer>
+        </>
       ) : null}
     </StyledNotificationMethod>
   );
diff --git a/superset-frontend/src/features/alerts/types.ts b/superset-frontend/src/features/alerts/types.ts
index 6dd57ccf0a3f1..932a744663d8f 100644
--- a/superset-frontend/src/features/alerts/types.ts
+++ b/superset-frontend/src/features/alerts/types.ts
@@ -79,6 +79,7 @@ export type AlertObject = {
   dashboard_id?: number;
   database?: MetaObject;
   description?: string;
+  email_subject?: string;
   error?: string;
   force_screenshot: boolean;
   grace_period?: number;
diff --git a/superset/commands/report/execute.py b/superset/commands/report/execute.py
index 42563c72d7904..d521ac161f612 100644
--- a/superset/commands/report/execute.py
+++ b/superset/commands/report/execute.py
@@ -391,16 +391,19 @@ def _get_notification_content(self) -> NotificationContent:
         ):
             embedded_data = self._get_embedded_data()
 
-        if self._report_schedule.chart:
-            name = (
-                f"{self._report_schedule.name}: "
-                f"{self._report_schedule.chart.slice_name}"
-            )
+        if self._report_schedule.email_subject:
+            name = self._report_schedule.email_subject
         else:
-            name = (
-                f"{self._report_schedule.name}: "
-                f"{self._report_schedule.dashboard.dashboard_title}"
-            )
+            if self._report_schedule.chart:
+                name = (
+                    f"{self._report_schedule.name}: "
+                    f"{self._report_schedule.chart.slice_name}"
+                )
+            else:
+                name = (
+                    f"{self._report_schedule.name}: "
+                    f"{self._report_schedule.dashboard.dashboard_title}"
+                )
 
         return NotificationContent(
             name=name,
diff --git a/superset/migrations/versions/2024-05-10_11-09_9621c6d56ffb_add_subject_column_to_report_schedule.py b/superset/migrations/versions/2024-05-10_11-09_9621c6d56ffb_add_subject_column_to_report_schedule.py
new file mode 100644
index 0000000000000..7af2038f64d50
--- /dev/null
+++ b/superset/migrations/versions/2024-05-10_11-09_9621c6d56ffb_add_subject_column_to_report_schedule.py
@@ -0,0 +1,41 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""add subject column to report schedule
+
+Revision ID: 9621c6d56ffb
+Revises: 4081be5b6b74
+Create Date: 2024-05-10 11:09:12.046862
+
+"""
+
+import sqlalchemy as sa
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision = "9621c6d56ffb"
+down_revision = "4081be5b6b74"
+
+
+def upgrade():
+    op.add_column(
+        "report_schedule",
+        sa.Column("email_subject", sa.String(length=255), nullable=True),
+    )
+
+
+def downgrade():
+    op.drop_column("report_schedule", "email_subject")
diff --git a/superset/reports/api.py b/superset/reports/api.py
index 8238213fefd72..4a298b564dd8c 100644
--- a/superset/reports/api.py
+++ b/superset/reports/api.py
@@ -119,6 +119,7 @@ def ensure_alert_reports_enabled(self) -> Optional[Response]:
         "validator_config_json",
         "validator_type",
         "working_timeout",
+        "email_subject",
     ]
     show_select_columns = show_columns + [
         "chart.datasource_id",
diff --git a/superset/reports/models.py b/superset/reports/models.py
index 63f904876f58e..3627a2ebf46ea 100644
--- a/superset/reports/models.py
+++ b/superset/reports/models.py
@@ -173,6 +173,8 @@ class ReportSchedule(AuditMixinNullable, ExtraJSONMixin, Model):
 
     extra: ReportScheduleExtra  # type: ignore
 
+    email_subject = Column(String(255))
+
     def __repr__(self) -> str:
         return str(self.name)
 
diff --git a/superset/reports/schemas.py b/superset/reports/schemas.py
index 64b56ec35a0be..f57a663e53de4 100644
--- a/superset/reports/schemas.py
+++ b/superset/reports/schemas.py
@@ -54,6 +54,7 @@
 name_description = "The report schedule name."
 # :)
 description_description = "Use a nice description to give context to this Alert/Report"
+email_subject_description = "The report schedule subject line"
 context_markdown_description = "Markdown description"
 crontab_description = (
     "A CRON expression."
@@ -146,6 +147,14 @@ class ReportSchedulePostSchema(Schema):
         allow_none=True,
         required=False,
     )
+    email_subject = fields.String(
+        metadata={
+            "description": email_subject_description,
+            "example": "[Report]  Report name: Dashboard or chart name",
+        },
+        allow_none=True,
+        required=False,
+    )
     context_markdown = fields.String(
         metadata={"description": context_markdown_description},
         allow_none=True,
@@ -272,6 +281,14 @@ class ReportSchedulePutSchema(Schema):
         allow_none=True,
         required=False,
     )
+    email_subject = fields.String(
+        metadata={
+            "description": email_subject_description,
+            "example": "[Report]  Report name: Dashboard or chart name",
+        },
+        allow_none=True,
+        required=False,
+    )
     context_markdown = fields.String(
         metadata={"description": context_markdown_description},
         allow_none=True,