Skip to content

Commit

Permalink
feat: Customizable email subject name (apache#26327)
Browse files Browse the repository at this point in the history
Co-authored-by: Puridach Wutthihathaithamrong <>
  • Loading branch information
puridach-w authored and EnxDev committed May 31, 2024
1 parent 718a988 commit 7a0c542
Show file tree
Hide file tree
Showing 8 changed files with 209 additions and 24 deletions.
46 changes: 46 additions & 0 deletions superset-frontend/src/features/alerts/AlertReportModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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:',
),
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -543,6 +547,7 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
owners: [],
recipients: [],
sql: '',
email_subject: '',
validator_config_json: {},
validator_type: '',
force_screenshot: false,
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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);
};

Expand Down Expand Up @@ -1199,6 +1213,7 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
const currentAlertSafe = currentAlert || {};
useEffect(() => {
validateAll();
updateEmailSubject();
}, [
currentAlertSafe.name,
currentAlertSafe.owners,
Expand All @@ -1212,6 +1227,7 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
contentType,
notificationSettings,
conditionNotNull,
emailError,
]);
useEffect(() => {
enforceValidation();
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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>
))}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ const StyledNotificationMethod = styled.div`
textarea {
height: auto;
}
&.error {
input {
border-color: ${({ theme }) => theme.colors.error.base};
}
}
}
.inline-container {
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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>
);
Expand Down
1 change: 1 addition & 0 deletions superset-frontend/src/features/alerts/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
21 changes: 12 additions & 9 deletions superset/commands/report/execute.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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")
1 change: 1 addition & 0 deletions superset/reports/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 2 additions & 0 deletions superset/reports/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
17 changes: 17 additions & 0 deletions superset/reports/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 7a0c542

Please sign in to comment.