Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ import {
CompressOutlined,
HistoryOutlined,
SlackOutlined,
ApiOutlined,
} from '@ant-design/icons';
import { FC } from 'react';
import { IconType } from './types';
Expand Down Expand Up @@ -287,6 +288,7 @@ const AntdIcons = {
CompressOutlined,
HistoryOutlined,
SlackOutlined,
ApiOutlined,
} as const;

type AntdIconNames = keyof typeof AntdIcons;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export enum FeatureFlag {
AlertReports = 'ALERT_REPORTS',
AlertReportTabs = 'ALERT_REPORT_TABS',
AlertReportSlackV2 = 'ALERT_REPORT_SLACK_V2',
Comment thread
rusackas marked this conversation as resolved.
AlertReportWebhook = 'ALERT_REPORT_WEBHOOK',
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we need this feature flag. I think this won't really break anything for existing users, and only ads new tooling for users.

In fairiness, we should probably open another PR to remove ALERT_REPORT_SLACK_V2 soon and complete that migration. And probably several others 😅

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Someone in the slack chat suggested that I add this to make it similar to how slack notifications are disabled by default and only enabled when a slack API key is provided. I dont mind removing it though since i agree i also dont really see how it breaks anything. Let me know what you think!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gentle ping @rusackas 😄

AlertReportsFilter = 'ALERT_REPORTS_FILTER',
AllowFullCsvExport = 'ALLOW_FULL_CSV_EXPORT',
AvoidColorsCollision = 'AVOID_COLORS_COLLISION',
Expand Down
1 change: 1 addition & 0 deletions superset-frontend/src/features/alerts/AlertReportModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ const EMAIL_REGEX = /^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/;

const DEFAULT_NOTIFICATION_METHODS: NotificationMethodOption[] = [
NotificationMethodOption.Email,
NotificationMethodOption.Webhook,
];
const DEFAULT_NOTIFICATION_FORMAT = 'PNG';
const DEFAULT_EXTRA_DASHBOARD_OPTIONS: Extra = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,8 @@ export const NotificationMethod: FunctionComponent<NotificationMethodProps> = ({
((!isFeatureEnabled(FeatureFlag.AlertReportSlackV2) ||
useSlackV1) &&
method === NotificationMethodOption.Slack) ||
(isFeatureEnabled(FeatureFlag.AlertReportWebhook) &&
method === NotificationMethodOption.Webhook) ||
method === NotificationMethodOption.Email,
)
Comment thread
rusackas marked this conversation as resolved.
.map(method => ({
Expand All @@ -354,7 +356,9 @@ export const NotificationMethod: FunctionComponent<NotificationMethodProps> = ({
return null;
}

const onRecipientsChange = (event: ChangeEvent<HTMLTextAreaElement>) => {
const onRecipientsChange = (
event: ChangeEvent<HTMLTextAreaElement | HTMLInputElement>,
) => {
const { target } = event;

setRecipientValue(target.value);
Expand Down Expand Up @@ -476,9 +480,9 @@ export const NotificationMethod: FunctionComponent<NotificationMethodProps> = ({
</div>
{method !== undefined ? (
<>
<div className="inline-container">
<StyledInputContainer>
{method === NotificationMethodOption.Email ? (
{method === NotificationMethodOption.Email ? (
<div className="inline-container">
<StyledInputContainer>
<>
<div className="control-label">
{TRANSLATIONS.EMAIL_SUBJECT_NAME}
Expand All @@ -503,66 +507,87 @@ export const NotificationMethod: FunctionComponent<NotificationMethodProps> = ({
</div>
)}
</>
) : null}
</StyledInputContainer>
</div>
<div className="inline-container">
<StyledInputContainer>
<div className="control-label">
{t(
'%s recipients',
method === NotificationMethodOption.SlackV2
? NotificationMethodOption.Slack
: method,
)}
<span className="required">*</span>
</div>
<div>
{[
NotificationMethodOption.Email,
NotificationMethodOption.Slack,
].includes(method) ? (
<>
</StyledInputContainer>
</div>
) : null}
{method !== NotificationMethodOption.Webhook ? (
<div className="inline-container">
<StyledInputContainer>
<div className="control-label">
{t(
'%s recipients',
method === NotificationMethodOption.SlackV2
? NotificationMethodOption.Slack
: method,
)}
<span className="required">*</span>
</div>
<div>
{[
NotificationMethodOption.Email,
NotificationMethodOption.Slack,
].includes(method) ? (
<>
<div className="input-container">
<Input.TextArea
name="To"
data-test="recipients"
value={recipientValue}
onChange={onRecipientsChange}
/>
</div>
<div className="input-container">
<div className="helper">
{t('Recipients are separated by "," or ";"')}
</div>
</div>
</>
) : (
// for SlackV2
<div className="input-container">
<Input.TextArea
name="To"
<Select
ariaLabel={t('Select channels')}
mode="multiple"
name="recipients"
value={slackRecipients}
options={slackOptions}
onChange={onSlackRecipientsChange}
allowClear
data-test="recipients"
value={recipientValue}
onChange={onRecipientsChange}
loading={isSlackChannelsLoading}
allowSelectAll={false}
labelInValue
/>
<RefreshLabel
onClick={() => updateSlackOptions({ force: true })}
tooltipContent={t('Force refresh Slack channels list')}
disabled={isSlackChannelsLoading}
/>
</div>
<div className="input-container">
<div className="helper">
{t('Recipients are separated by "," or ";"')}
</div>
</div>
</>
) : (
// for SlackV2
)}
</div>
</StyledInputContainer>
</div>
) : (
<div className="inline-container">
<StyledInputContainer>
<div className="control-label">
{t('%s URL', method)}
<span className="required">*</span>
</div>
<div>
<div className="input-container">
<Select
ariaLabel={t('Select channels')}
mode="multiple"
name="recipients"
value={slackRecipients}
options={slackOptions}
onChange={onSlackRecipientsChange}
allowClear
<Input
name="To"
data-test="recipients"
loading={isSlackChannelsLoading}
allowSelectAll={false}
labelInValue
/>
<RefreshLabel
onClick={() => updateSlackOptions({ force: true })}
tooltipContent={t('Force refresh Slack channels list')}
disabled={isSlackChannelsLoading}
value={recipientValue}
onChange={onRecipientsChange}
/>
</div>
)}
</div>
</StyledInputContainer>
</div>
</div>
</StyledInputContainer>
</div>
)}
{method === NotificationMethodOption.Email && (
<StyledInputContainer>
{/* Render "CC" input field if ccVisible is true */}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,12 @@ export default function RecipientIcon({ type }: { type: string }) {
);
recipientIconConfig.label = NotificationMethodOption.Slack;
break;
case NotificationMethodOption.Webhook:
recipientIconConfig.icon = (
<Icons.ApiOutlined css={notificationStyledIcon} iconSize="l" />
Comment thread
rusackas marked this conversation as resolved.
);
recipientIconConfig.label = NotificationMethodOption.Webhook;
break;
default:
recipientIconConfig.icon = null;
recipientIconConfig.label = '';
Expand Down
2 changes: 2 additions & 0 deletions superset-frontend/src/features/alerts/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export enum NotificationMethodOption {
Email = 'Email',
Slack = 'Slack',
SlackV2 = 'SlackV2',
Webhook = 'Webhook',
}

export type SelectValue = {
Expand Down Expand Up @@ -173,6 +174,7 @@ export enum RecipientIconName {
Email = 'Email',
Slack = 'Slack',
SlackV2 = 'SlackV2',
Webhook = 'Webhook',
Comment thread
rusackas marked this conversation as resolved.
}
export interface AlertsReportsConfig {
ALERT_REPORTS_DEFAULT_WORKING_TIMEOUT: number;
Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/src/features/reports/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
export type ReportScheduleType = 'Alert' | 'Report';
export type ReportCreationMethod = 'charts' | 'dashboards' | 'alerts_reports';

export type ReportRecipientType = 'Email' | 'Slack';
export type ReportRecipientType = 'Email' | 'Slack' | 'Webhook';
Comment thread
rusackas marked this conversation as resolved.

export enum ReportType {
Dashboards = 'dashboards',
Expand Down
3 changes: 3 additions & 0 deletions superset/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,7 @@ class D3TimeFormat(TypedDict, total=False):
"ALERT_REPORT_TABS": False,
"ALERT_REPORTS_FILTER": False,
"ALERT_REPORT_SLACK_V2": False,
"ALERT_REPORT_WEBHOOK": False,
Comment thread
rusackas marked this conversation as resolved.
"DASHBOARD_RBAC": False,
"ENABLE_ADVANCED_DATA_TYPES": False,
# Enabling ALERTS_ATTACH_REPORTS, the system sends email and slack message
Expand Down Expand Up @@ -1780,6 +1781,8 @@ def EMAIL_HEADER_MUTATOR( # pylint: disable=invalid-name,unused-argument # noq
# You can also assign a function to the config that returns the expected integer
ALERT_MINIMUM_INTERVAL = int(timedelta(minutes=0).total_seconds())
REPORT_MINIMUM_INTERVAL = int(timedelta(minutes=0).total_seconds())
# Enforce HTTPS for webhook alerts/reports
ALERT_REPORTS_WEBHOOK_HTTPS_ONLY = True

# A custom prefix to use on all Alerts & Reports emails
EMAIL_REPORTS_SUBJECT_PREFIX = "[Report] "
Expand Down
1 change: 1 addition & 0 deletions superset/reports/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ class ReportRecipientType(StrEnum):
EMAIL = "Email"
SLACK = "Slack"
SLACKV2 = "SlackV2"
WEBHOOK = "Webhook"


class ReportState(StrEnum):
Expand Down
1 change: 1 addition & 0 deletions superset/reports/notifications/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from superset.reports.notifications.email import EmailNotification # noqa: F401
from superset.reports.notifications.slack import SlackNotification # noqa: F401
from superset.reports.notifications.slackv2 import SlackV2Notification # noqa: F401
from superset.reports.notifications.webhook import WebhookNotification # noqa: F401


def create_notification(
Expand Down
Loading
Loading