fix: load slack channels earlier (#29846)

This commit is contained in:
Elizabeth Thompson 2024-08-06 15:40:30 -07:00 committed by GitHub
parent 226b755798
commit 0c3aa7d8fe
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 330 additions and 30 deletions

View File

@ -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(<NotificationMethod {...defaultProps} />);
// 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(<NotificationMethod {...defaultProps} />);
// 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(<NotificationMethod {...defaultProps} />);
// 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(<NotificationMethod {...defaultProps} />);
// 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(
<NotificationMethod
setting={{
...mockSetting,
method: NotificationMethodOption.Slack,
}}
index={0}
onUpdate={mockOnUpdate}
onRemove={mockOnRemove}
onInputChange={mockOnInputChange}
email_subject={mockEmailSubject}
defaultSubject={mockDefaultSubject}
setErrorSubject={mockSetErrorSubject}
/>,
);
// 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(
<NotificationMethod
setting={{
...mockSetting,
method: NotificationMethodOption.Slack,
}}
index={0}
onUpdate={mockOnUpdate}
onRemove={mockOnRemove}
onInputChange={mockOnInputChange}
email_subject={mockEmailSubject}
defaultSubject={mockDefaultSubject}
setErrorSubject={mockSetErrorSubject}
/>,
);
// 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(
<NotificationMethod
setting={{
...mockSetting,
method: NotificationMethodOption.SlackV2,
recipients: 'slack-channel',
}}
index={0}
onUpdate={mockOnUpdate}
onRemove={mockOnRemove}
onInputChange={mockOnInputChange}
email_subject={mockEmailSubject}
defaultSubject={mockDefaultSubject}
setErrorSubject={mockSetErrorSubject}
/>,
);
// 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(
<NotificationMethod
setting={{
...mockSetting,
method: NotificationMethodOption.Slack,
recipients: 'slack-channel',
}}
index={0}
onUpdate={mockOnUpdate}
onRemove={mockOnRemove}
onInputChange={mockOnInputChange}
email_subject={mockEmailSubject}
defaultSubject={mockDefaultSubject}
setErrorSubject={mockSetErrorSubject}
/>,
);
// 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(
<NotificationMethod
setting={{
...mockSetting,
method: NotificationMethodOption.Slack,
recipients: 'slack-channel',
}}
index={0}
onUpdate={mockOnUpdate}
onRemove={mockOnRemove}
onInputChange={mockOnInputChange}
email_subject={mockEmailSubject}
defaultSubject={mockDefaultSubject}
setErrorSubject={mockSetErrorSubject}
/>,
);
// Wait for the component to handle the error and render the expected div
expect(
screen.getByText('Recipients are separated by "," or ";"'),
).toBeInTheDocument();
});
});

View File

@ -211,6 +211,8 @@ export const NotificationMethod: FunctionComponent<NotificationMethodProps> = ({
const [ccValue, setCcValue] = useState<string>(cc || '');
const [bccValue, setBccValue] = useState<string>(bcc || '');
const theme = useTheme();
const [methodOptionsLoading, setMethodOptionsLoading] =
useState<boolean>(true);
const [slackOptions, setSlackOptions] = useState<SlackOptionsType>([
{
label: '',
@ -257,27 +259,25 @@ export const NotificationMethod: FunctionComponent<NotificationMethodProps> = ({
};
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;
if (
method &&
[
NotificationMethodOption.SlackV2,
NotificationMethodOption.Slack,
].includes(method)
) {
setSlackRecipients(
mapSlackValues({
method,
@ -288,20 +288,18 @@ export const NotificationMethod: FunctionComponent<NotificationMethodProps> = ({
],
}),
);
if (method === NotificationMethodOption.Slack) {
onMethodChange({
label: NotificationMethodOption.Slack,
value: NotificationMethodOption.SlackV2,
});
}
}
})
.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<NotificationMethodProps> = ({
: method,
value: method,
})),
[options],
[options, useSlackV1],
);
if (!setting) {
@ -434,8 +432,10 @@ export const NotificationMethod: FunctionComponent<NotificationMethodProps> = ({
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
<span
role="button"
tabIndex={0}