diff --git a/UPDATING.md b/UPDATING.md index 2327fcfa3fdb4..a23aec5907731 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -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 diff --git a/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts b/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts index b3af431d52bba..8ffc4f845d97b 100644 --- a/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts +++ b/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts @@ -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', diff --git a/superset-frontend/src/features/alerts/AlertReportModal.test.tsx b/superset-frontend/src/features/alerts/AlertReportModal.test.tsx index e32d13ab635f2..17047b7a404e4 100644 --- a/superset-frontend/src/features/alerts/AlertReportModal.test.tsx +++ b/superset-frontend/src/features/alerts/AlertReportModal.test.tsx @@ -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'], }), })); diff --git a/superset-frontend/src/features/alerts/AlertReportModal.tsx b/superset-frontend/src/features/alerts/AlertReportModal.tsx index 633138e5796c2..78f2a8d4db53d 100644 --- a/superset-frontend/src/features/alerts/AlertReportModal.tsx +++ b/superset-frontend/src/features/alerts/AlertReportModal.tsx @@ -517,7 +517,7 @@ const AlertReportModal: FunctionComponent = ({ ]); setNotificationAddState( - notificationSettings.length === allowedNotificationMethods.length + notificationSettings.length === allowedNotificationMethodsCount ? 'hidden' : 'disabled', ); @@ -1235,6 +1235,20 @@ const AlertReportModal: FunctionComponent = ({ 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); @@ -1743,7 +1757,7 @@ const AlertReportModal: FunctionComponent = ({ ))} { // Prohibit 'add notification method' button if only one present - allowedNotificationMethods.length > notificationSettings.length && ( + allowedNotificationMethodsCount > notificationSettings.length && ( { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('should render the component', () => { + render( + , + ); + + expect(screen.getByText('Notification Method')).toBeInTheDocument(); + expect( + screen.getByText('Email subject name (optional)'), + ).toBeInTheDocument(); + expect(screen.getByText('Email recipients')).toBeInTheDocument(); + }); + + it('should call onUpdate when the method is changed', () => { + render( + , + ); + + const select = screen.getByLabelText('Delivery method'); + fireEvent.change(select, { target: { value: 'Slack' } }); + + expect(mockOnUpdate).toHaveBeenCalledWith(0, { + ...mockSetting, + method: 'Slack', + recipients: '', + }); + }); + + it('should call onRemove when the delete button is clicked', () => { + render( + , + ); + + const deleteButton = screen.getByRole('button'); + fireEvent.click(deleteButton); + + expect(mockOnRemove).toHaveBeenCalledWith(1); + }); + + it('should call onRecipientsChange when the recipients value is changed', () => { + render( + , + ); + + const recipientsInput = screen.getByTestId('recipients'); + fireEvent.change(recipientsInput, { + target: { value: 'test1@example.com' }, + }); + + expect(mockOnUpdate).toHaveBeenCalledWith(0, { + ...mockSetting, + recipients: 'test1@example.com', + }); + }); + + it('should call onSlackRecipientsChange when the slack recipients value is changed', () => { + render( + , + ); + + const slackRecipientsInput = screen.getByLabelText('Select owners'); + fireEvent.change(slackRecipientsInput, [ + { label: 'User 1', value: 'user1' }, + { label: 'User 2', value: 'user2' }, + ]); + + expect(mockOnUpdate).toHaveBeenCalledWith(0, { + ...mockSetting, + recipients: 'user1,user2', + }); + }); + + it('should call onSubjectChange when the email subject value is changed', () => { + render( + , + ); + + const subjectInput = screen.getByPlaceholderText('Default Subject'); + fireEvent.change(subjectInput, { target: { value: 'Test Subject' } }); + + expect(mockOnInputChange).toHaveBeenCalled(); + }); +}); diff --git a/superset-frontend/src/features/alerts/components/NotificationMethod.tsx b/superset-frontend/src/features/alerts/components/NotificationMethod.tsx index b2d780423bc13..c38d649c7828d 100644 --- a/superset-frontend/src/features/alerts/components/NotificationMethod.tsx +++ b/superset-frontend/src/features/alerts/components/NotificationMethod.tsx @@ -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'; @@ -87,20 +101,57 @@ export const NotificationMethod: FunctionComponent = ({ const [recipientValue, setRecipientValue] = useState( 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(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: '', }; @@ -108,6 +159,42 @@ export const NotificationMethod: FunctionComponent = ({ } }; + 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) => { const { target } = event; @@ -123,6 +210,21 @@ export const NotificationMethod: FunctionComponent = ({ } }; + 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, ) => { @@ -153,15 +255,12 @@ export const NotificationMethod: FunctionComponent = ({