diff --git a/superset-frontend/src/features/alerts/components/NotificationMethod.test.tsx b/superset-frontend/src/features/alerts/components/NotificationMethod.test.tsx
index 292e8d9af..39d9b2625 100644
--- a/superset-frontend/src/features/alerts/components/NotificationMethod.test.tsx
+++ b/superset-frontend/src/features/alerts/components/NotificationMethod.test.tsx
@@ -16,9 +16,21 @@
* specific language governing permissions and limitations
* under the License.
*/
-import { fireEvent, render, screen } from 'spec/helpers/testing-library';
+import {
+ cleanup,
+ fireEvent,
+ render,
+ screen,
+ waitFor,
+} from 'spec/helpers/testing-library';
import userEvent from '@testing-library/user-event';
+import {
+ FeatureFlag,
+ JsonResponse,
+ SupersetClient,
+ TextResponse,
+} from '@superset-ui/core';
import { NotificationMethod, mapSlackValues } from './NotificationMethod';
import { NotificationMethodOption, NotificationSetting } from '../types';
@@ -43,6 +55,7 @@ const mockDefaultSubject = 'Default Subject';
describe('NotificationMethod', () => {
beforeEach(() => {
jest.clearAllMocks();
+ cleanup();
});
it('should render the component', () => {
@@ -180,4 +193,291 @@ describe('NotificationMethod', () => {
{ label: 'User Two', value: 'user2' },
]);
});
+ it('should render CC and BCC fields when method is Email and visibility flags are true', () => {
+ const defaultProps = {
+ setting: {
+ method: NotificationMethodOption.Email,
+ recipients: 'recipient1@example.com, recipient2@example.com',
+ cc: 'cc1@example.com',
+ bcc: 'bcc1@example.com',
+ options: [
+ NotificationMethodOption.Email,
+ NotificationMethodOption.Slack,
+ ],
+ },
+ index: 0,
+ onUpdate: jest.fn(),
+ onRemove: jest.fn(),
+ onInputChange: jest.fn(),
+ email_subject: 'Test Subject',
+ defaultSubject: 'Default Subject',
+ setErrorSubject: jest.fn(),
+ };
+
+ const { getByTestId } = render();
+
+ // Check if CC and BCC fields are rendered
+ expect(getByTestId('cc')).toBeInTheDocument();
+ expect(getByTestId('bcc')).toBeInTheDocument();
+ });
+ it('should render CC and BCC fields with correct values when method is Email', () => {
+ const defaultProps = {
+ setting: {
+ method: NotificationMethodOption.Email,
+ recipients: 'recipient1@example.com, recipient2@example.com',
+ cc: 'cc1@example.com',
+ bcc: 'bcc1@example.com',
+ options: [
+ NotificationMethodOption.Email,
+ NotificationMethodOption.Slack,
+ ],
+ },
+ index: 0,
+ onUpdate: jest.fn(),
+ onRemove: jest.fn(),
+ onInputChange: jest.fn(),
+ email_subject: 'Test Subject',
+ defaultSubject: 'Default Subject',
+ setErrorSubject: jest.fn(),
+ };
+
+ const { getByTestId } = render();
+
+ // Check if CC and BCC fields are rendered with correct values
+ expect(getByTestId('cc')).toHaveValue('cc1@example.com');
+ expect(getByTestId('bcc')).toHaveValue('bcc1@example.com');
+ });
+ it('should not render CC and BCC fields when method is not Email', () => {
+ const defaultProps = {
+ setting: {
+ method: NotificationMethodOption.Slack,
+ recipients: 'recipient1@example.com, recipient2@example.com',
+ cc: 'cc1@example.com',
+ bcc: 'bcc1@example.com',
+ options: [
+ NotificationMethodOption.Email,
+ NotificationMethodOption.Slack,
+ ],
+ },
+ index: 0,
+ onUpdate: jest.fn(),
+ onRemove: jest.fn(),
+ onInputChange: jest.fn(),
+ email_subject: 'Test Subject',
+ defaultSubject: 'Default Subject',
+ setErrorSubject: jest.fn(),
+ };
+
+ const { queryByTestId } = render();
+
+ // Check if CC and BCC fields are not rendered
+ expect(queryByTestId('cc')).not.toBeInTheDocument();
+ expect(queryByTestId('bcc')).not.toBeInTheDocument();
+ });
+ // Handle empty recipients list gracefully
+ it('should handle empty recipients list gracefully', () => {
+ const defaultProps = {
+ setting: {
+ method: NotificationMethodOption.Email,
+ recipients: '',
+ cc: '',
+ bcc: '',
+ options: [
+ NotificationMethodOption.Email,
+ NotificationMethodOption.Slack,
+ ],
+ },
+ index: 0,
+ onUpdate: jest.fn(),
+ onRemove: jest.fn(),
+ onInputChange: jest.fn(),
+ email_subject: 'Test Subject',
+ defaultSubject: 'Default Subject',
+ setErrorSubject: jest.fn(),
+ };
+
+ const { queryByTestId } = render();
+
+ // Check if CC and BCC fields are not rendered
+ expect(queryByTestId('cc')).not.toBeInTheDocument();
+ expect(queryByTestId('bcc')).not.toBeInTheDocument();
+ });
+ it('shows the right combo when ff is false', async () => {
+ /* should show the div with "Recipients are separated by"
+ when FeatureFlag.AlertReportSlackV2 is false and fetchSlackChannels errors
+ */
+ // Mock the feature flag to be false
+ window.featureFlags = { [FeatureFlag.AlertReportSlackV2]: false };
+
+ // Mock the SupersetClient.get to simulate an error
+ jest.spyOn(SupersetClient, 'get').mockImplementation(() => {
+ throw new Error('Error fetching Slack channels');
+ });
+
+ render(
+ ,
+ );
+
+ // Wait for the component to handle the error and render the expected div
+ await waitFor(() => {
+ expect(
+ screen.getByText('Recipients are separated by "," or ";"'),
+ ).toBeInTheDocument();
+ });
+ });
+ it('shows the textbox when the fetch fails', async () => {
+ /* should show the div with "Recipients are separated by"
+ when FeatureFlag.AlertReportSlackV2 is true and fetchSlackChannels errors
+ */
+
+ // Mock the feature flag to be false
+ window.featureFlags = { [FeatureFlag.AlertReportSlackV2]: false };
+
+ // Mock the SupersetClient.get to simulate an error
+ jest.spyOn(SupersetClient, 'get').mockImplementation(() => {
+ throw new Error('Error fetching Slack channels');
+ });
+
+ render(
+ ,
+ );
+
+ // Wait for the component to handle the error and render the expected div
+ await waitFor(() => {
+ expect(
+ screen.getByText('Recipients are separated by "," or ";"'),
+ ).toBeInTheDocument();
+ });
+ });
+ it('shows the dropdown when ff is true and slackChannels succeed', async () => {
+ /* should show the Select channels dropdown
+ when FeatureFlag.AlertReportSlackV2 is true and fetchSlackChannels succeeds
+ */
+ // Mock the feature flag to be false
+ window.featureFlags = { [FeatureFlag.AlertReportSlackV2]: true };
+
+ // Mock the SupersetClient.get to simulate an error
+ jest
+ .spyOn(SupersetClient, 'get')
+ .mockImplementation(
+ () =>
+ Promise.resolve({ json: { result: [] } }) as unknown as Promise<
+ Response | JsonResponse | TextResponse
+ >,
+ );
+
+ render(
+ ,
+ );
+
+ // Wait for the component to handle the error and render the expected div
+ await waitFor(() => {
+ expect(screen.getByTitle('Slack')).toBeInTheDocument();
+ });
+ });
+ it('shows the textarea when ff is true and slackChannels fail', async () => {
+ /* should show the Select channels dropdown
+ when FeatureFlag.AlertReportSlackV2 is true and fetchSlackChannels succeeds
+ */
+ // Mock the feature flag to be false
+ window.featureFlags = { [FeatureFlag.AlertReportSlackV2]: true };
+
+ // Mock the SupersetClient.get to simulate an error
+ jest.spyOn(SupersetClient, 'get').mockImplementation(() => {
+ throw new Error('Error fetching Slack channels');
+ });
+
+ render(
+ ,
+ );
+
+ // Wait for the component to handle the error and render the expected div
+ expect(
+ screen.getByText('Recipients are separated by "," or ";"'),
+ ).toBeInTheDocument();
+ });
+ it('shows the textarea when ff is true and slackChannels fail and slack is selected', async () => {
+ /* should show the Select channels dropdown
+ when FeatureFlag.AlertReportSlackV2 is true and fetchSlackChannels succeeds
+ */
+ // Mock the feature flag to be false
+ window.featureFlags = { [FeatureFlag.AlertReportSlackV2]: true };
+
+ // Mock the SupersetClient.get to simulate an error
+ jest.spyOn(SupersetClient, 'get').mockImplementation(() => {
+ throw new Error('Error fetching Slack channels');
+ });
+
+ render(
+ ,
+ );
+
+ // Wait for the component to handle the error and render the expected div
+ expect(
+ screen.getByText('Recipients are separated by "," or ";"'),
+ ).toBeInTheDocument();
+ });
});
diff --git a/superset-frontend/src/features/alerts/components/NotificationMethod.tsx b/superset-frontend/src/features/alerts/components/NotificationMethod.tsx
index 5028c14f1..dff2b9e96 100644
--- a/superset-frontend/src/features/alerts/components/NotificationMethod.tsx
+++ b/superset-frontend/src/features/alerts/components/NotificationMethod.tsx
@@ -211,6 +211,8 @@ export const NotificationMethod: FunctionComponent = ({
const [ccValue, setCcValue] = useState(cc || '');
const [bccValue, setBccValue] = useState(bcc || '');
const theme = useTheme();
+ const [methodOptionsLoading, setMethodOptionsLoading] =
+ useState(true);
const [slackOptions, setSlackOptions] = useState([
{
label: '',
@@ -257,51 +259,47 @@ export const NotificationMethod: FunctionComponent = ({
};
useEffect(() => {
- if (
- method &&
- [
- NotificationMethodOption.Slack,
- NotificationMethodOption.SlackV2,
- ].includes(method) &&
- !slackOptions[0]?.options.length
- ) {
+ if (!slackOptions[0]?.options.length) {
fetchSlackChannels({ types: ['public_channel', 'private_channel'] })
.then(({ json }) => {
const { result } = json;
-
const options: SlackOptionsType = mapChannelsToOptions(result);
setSlackOptions(options);
if (isFeatureEnabled(FeatureFlag.AlertReportSlackV2)) {
- // map existing ids to names for display
+ // for edit mode, map existing ids to names for display if slack v2
// or names to ids if slack v1
const [publicOptions, privateOptions] = options;
-
- setSlackRecipients(
- mapSlackValues({
- method,
- recipientValue,
- slackOptions: [
- ...publicOptions.options,
- ...privateOptions.options,
- ],
- }),
- );
- if (method === NotificationMethodOption.Slack) {
- onMethodChange({
- label: NotificationMethodOption.Slack,
- value: NotificationMethodOption.SlackV2,
- });
+ if (
+ method &&
+ [
+ NotificationMethodOption.SlackV2,
+ NotificationMethodOption.Slack,
+ ].includes(method)
+ ) {
+ setSlackRecipients(
+ mapSlackValues({
+ method,
+ recipientValue,
+ slackOptions: [
+ ...publicOptions.options,
+ ...privateOptions.options,
+ ],
+ }),
+ );
}
}
})
- .catch(() => {
+ .catch(e => {
// Fallback to slack v1 if slack v2 is not compatible
setUseSlackV1(true);
+ })
+ .finally(() => {
+ setMethodOptionsLoading(false);
});
}
- }, [method]);
+ }, []);
const methodOptions = useMemo(
() =>
@@ -323,7 +321,7 @@ export const NotificationMethod: FunctionComponent = ({
: method,
value: method,
})),
- [options],
+ [options, useSlackV1],
);
if (!setting) {
@@ -434,8 +432,10 @@ export const NotificationMethod: FunctionComponent = ({
options={methodOptions}
showSearch
value={methodOptions.find(option => option.value === method)}
+ loading={methodOptionsLoading}
/>
{index !== 0 && !!onRemove ? (
+ // eslint-disable-next-line jsx-a11y/control-has-associated-label