Skip to content

Commit

Permalink
add slackv2 notification
Browse files Browse the repository at this point in the history
  • Loading branch information
eschutho committed Jun 21, 2024
1 parent fc9bc17 commit eccc201
Show file tree
Hide file tree
Showing 25 changed files with 694 additions and 444 deletions.
1 change: 1 addition & 0 deletions UPDATING.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ assists people when migrating to a new version.
translations inside the python package. This includes the .mo files needed by pybabel on the
backend, as well as the .json files used by the frontend. If you were doing anything before
as part of your bundling to expose translation packages, it's probably not needed anymore.
- [29264](https://github.com/apache/superset/pull/29264) Slack has updated its file upload api, and we are now supporting this new api in Superset, although the Slack api is not backward compatible. The original Slack integration is deprecated and we will require a new Slack scope `channels:read` to be added to Slack workspaces in order to use this new api. In an upcoming release, we will make this new Slack scope mandatory and remove the old Slack functionality.

### Potential Downtime

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export enum FeatureFlag {
AlertsAttachReports = 'ALERTS_ATTACH_REPORTS',
AlertReports = 'ALERT_REPORTS',
AlertReportTabs = 'ALERT_REPORT_TABS',
AlertReportSlackV2 = 'ALERT_REPORT_SLACK_V2',
AllowFullCsvExport = 'ALLOW_FULL_CSV_EXPORT',
AvoidColorsCollision = 'AVOID_COLORS_COLLISION',
ChartPluginsExperimental = 'CHART_PLUGINS_EXPERIMENTAL',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ jest.mock('@superset-ui/core', () => ({

jest.mock('src/features/databases/state.ts', () => ({
useCommonConf: () => ({
ALERT_REPORTS_NOTIFICATION_METHODS: ['Email', 'Slack'],
ALERT_REPORTS_NOTIFICATION_METHODS: ['Email', 'Slack', 'SlackV2'],
}),
}));

Expand Down
18 changes: 16 additions & 2 deletions superset-frontend/src/features/alerts/AlertReportModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,7 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
]);

setNotificationAddState(
notificationSettings.length === allowedNotificationMethods.length
notificationSettings.length === allowedNotificationMethodsCount
? 'hidden'
: 'disabled',
);
Expand Down Expand Up @@ -1235,6 +1235,20 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
enforceValidation();
}, [validationStatus]);

const allowedNotificationMethodsCount = useMemo(
() =>
allowedNotificationMethods.reduce((accum: string[], setting: string) => {
if (
accum.some(nm => nm.includes('slack')) &&
setting.toLowerCase().includes('slack')
) {
return accum;
}
return [...accum, setting.toLowerCase()];
}, []).length,
[allowedNotificationMethods],
);

// Show/hide
if (isHidden && show) {
setIsHidden(false);
Expand Down Expand Up @@ -1743,7 +1757,7 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
))}
{
// Prohibit 'add notification method' button if only one present
allowedNotificationMethods.length > notificationSettings.length && (
allowedNotificationMethodsCount > notificationSettings.length && (
<NotificationMethodAdd
data-test="notification-add"
status={notificationAddState}
Expand Down
171 changes: 144 additions & 27 deletions superset-frontend/src/features/alerts/components/NotificationMethod.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,24 @@
* specific language governing permissions and limitations
* under the License.
*/
import { FunctionComponent, useState, ChangeEvent } from 'react';
import {
FunctionComponent,
useState,
ChangeEvent,
useEffect,
useMemo,
} from 'react';

import { styled, t, useTheme } from '@superset-ui/core';
import { Select } from 'src/components';
import {
FeatureFlag,
SupersetClient,
isFeatureEnabled,
styled,
t,
useTheme,
} from '@superset-ui/core';
import rison from 'rison';
import { AsyncSelect, Select } from 'src/components';
import Icons from 'src/components/Icons';
import { NotificationMethodOption, NotificationSetting } from '../types';
import { StyledInputContainer } from '../AlertReportModal';
Expand Down Expand Up @@ -87,27 +101,100 @@ export const NotificationMethod: FunctionComponent<NotificationMethodProps> = ({
const [recipientValue, setRecipientValue] = useState<string>(
recipients || '',
);
const [slackRecipients, setSlackRecipients] = useState<
{ label: string; value: string }[]
>([]);
const [error, setError] = useState(false);
const theme = useTheme();

if (!setting) {
return null;
}
const [useSlackV1, setUseSlackV1] = useState<boolean>(false);

const mapChannelsToOptions = (result: { name: any; id: any }[]) =>
result.map((result: { name: any; id: any }) => ({
label: result.name,
value: result.id,
}));

const onMethodChange = (method: NotificationMethodOption) => {
const loadChannels = async (
search_string: string | undefined = '',
): Promise<{
data: { label: any; value: any }[];
totalCount: number;
}> => {
const query = rison.encode({ search_string });
const endpoint = `/api/v1/report/slack_channels/?q=${query}`;
const noResults = { data: [], totalCount: 0 };
return SupersetClient.get({ endpoint })
.then(({ json }) => {
const { result, count } = json;

const options: { label: any; value: any }[] =
mapChannelsToOptions(result);

return {
data: options,
totalCount: (count ?? options.length) as number,
};
})
.catch(() => {
// Fallback to slack v1 if slack v2 is not compatible
setUseSlackV1(true);
return noResults;
});
};
const onMethodChange = (selected: {
label: string;
value: NotificationMethodOption;
}) => {
// Since we're swapping the method, reset the recipients
setRecipientValue('');
if (onUpdate) {
if (onUpdate && setting) {
const updatedSetting = {
...setting,
method,
method: selected.value,
recipients: '',
};

onUpdate(index, updatedSetting);
}
};

useEffect(() => {
// fetch slack channel names from
// ids on first load
if (method && ['Slack', 'SlackV2'].includes(method)) {
loadChannels(recipients).then(response => {
setSlackRecipients(response.data || []);
// if fetch succeeds, set the method to SlackV2
onMethodChange({ label: 'Slack', value: 'SlackV2' });
});
}
}, []);

const formattedOptions = useMemo(
() =>
(options || [])
.filter(
method =>
(isFeatureEnabled(FeatureFlag.AlertReportSlackV2) &&
!useSlackV1 &&
method === 'SlackV2') ||
((!isFeatureEnabled(FeatureFlag.AlertReportSlackV2) ||
useSlackV1) &&
method === 'Slack') ||
method === 'Email',
)
.map(method => ({
label: method === 'SlackV2' ? 'Slack' : method,
value: method,
})),
[options],
);

if (!setting) {
return null;
}

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

Expand All @@ -123,6 +210,21 @@ export const NotificationMethod: FunctionComponent<NotificationMethodProps> = ({
}
};

const onSlackRecipientsChange = (
recipients: { label: string; value: string }[],
) => {
setSlackRecipients(recipients);

if (onUpdate) {
const updatedSetting = {
...setting,
recipients: recipients?.map(obj => obj.value).join(','),
};

onUpdate(index, updatedSetting);
}
};

const onSubjectChange = (
event: ChangeEvent<HTMLTextAreaElement | HTMLInputElement>,
) => {
Expand Down Expand Up @@ -153,15 +255,12 @@ export const NotificationMethod: FunctionComponent<NotificationMethodProps> = ({
<Select
ariaLabel={t('Delivery method')}
data-test="select-delivery-method"
labelInValue
onChange={onMethodChange}
placeholder={t('Select Delivery Method')}
options={(options || []).map(
(method: NotificationMethodOption) => ({
label: method,
value: method,
}),
)}
value={method}
options={formattedOptions}
showSearch
value={formattedOptions.find(option => option.value === method)}
/>
{index !== 0 && !!onRemove ? (
<span
Expand Down Expand Up @@ -211,19 +310,37 @@ export const NotificationMethod: FunctionComponent<NotificationMethodProps> = ({
<div className="inline-container">
<StyledInputContainer>
<div className="control-label">
{t('%s recipients', method)}
{t('%s recipients', method === 'SlackV2' ? 'Slack' : 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>
{['Email', 'Slack'].includes(method) ? (
<>
<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>
</>
) : (
// for SlackV2
<AsyncSelect
ariaLabel={t('Select channels')}
mode="multiple"
name="recipients"
value={slackRecipients}
options={loadChannels}
onChange={onSlackRecipientsChange}
allowClear
data-test="recipients"
/>
)}
</div>
</StyledInputContainer>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ export default function RecipientIcon({ type }: { type: string }) {
recipientIconConfig.icon = <Icons.Slack css={StyledIcon} />;
recipientIconConfig.label = RecipientIconName.Slack;
break;
case RecipientIconName.SlackV2:
recipientIconConfig.icon = <Icons.Slack css={StyledIcon} />;
recipientIconConfig.label = RecipientIconName.Slack;
break;
default:
recipientIconConfig.icon = null;
recipientIconConfig.label = '';
Expand Down
3 changes: 2 additions & 1 deletion superset-frontend/src/features/alerts/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export type DatabaseObject = {
id: number;
};

export type NotificationMethodOption = 'Email' | 'Slack';
export type NotificationMethodOption = 'Email' | 'Slack' | 'SlackV2';

export type NotificationSetting = {
method?: NotificationMethodOption;
Expand Down Expand Up @@ -124,6 +124,7 @@ export enum AlertState {
export enum RecipientIconName {
Email = 'Email',
Slack = 'Slack',
SlackV2 = 'SlackV2',
}
export interface AlertsReportsConfig {
ALERT_REPORTS_DEFAULT_WORKING_TIMEOUT: number;
Expand Down
Loading

0 comments on commit eccc201

Please sign in to comment.