This is an automated email from the ASF dual-hosted git repository. vavila pushed a commit to branch chore/cache-slack-channels-list in repository https://gitbox.apache.org/repos/asf/superset.git
commit 5720ca6878a2fe6d70fe85bf6c87814bb13ca19e Author: Elizabeth Thompson <[email protected]> AuthorDate: Wed Mar 5 18:04:21 2025 -0800 add refresh button --- .../alerts/components/NotificationMethod.test.tsx | 77 +++++++++++++ .../alerts/components/NotificationMethod.tsx | 127 ++++++++++++--------- 2 files changed, 153 insertions(+), 51 deletions(-) diff --git a/superset-frontend/src/features/alerts/components/NotificationMethod.test.tsx b/superset-frontend/src/features/alerts/components/NotificationMethod.test.tsx index b0c9186e29..eec2a434f6 100644 --- a/superset-frontend/src/features/alerts/components/NotificationMethod.test.tsx +++ b/superset-frontend/src/features/alerts/components/NotificationMethod.test.tsx @@ -52,6 +52,16 @@ const mockSetting: NotificationSetting = { const mockEmailSubject = 'Test Subject'; const mockDefaultSubject = 'Default Subject'; +const mockSettingSlackV2: NotificationSetting = { + method: NotificationMethodOption.SlackV2, + recipients: 'slack-channel', + options: [ + NotificationMethodOption.Email, + NotificationMethodOption.Slack, + NotificationMethodOption.SlackV2, + ], +}; + describe('NotificationMethod', () => { beforeEach(() => { jest.clearAllMocks(); @@ -480,4 +490,71 @@ describe('NotificationMethod', () => { screen.getByText('Recipients are separated by "," or ";"'), ).toBeInTheDocument(); }); + + it('shows the textarea when ff is true, slackChannels fail and slack is selected', async () => { + window.featureFlags = { [FeatureFlag.AlertReportSlackV2]: true }; + jest.spyOn(SupersetClient, 'get').mockImplementation(() => { + throw new Error('Error fetching Slack channels'); + }); + + render( + <NotificationMethod + setting={{ + ...mockSettingSlackV2, + method: NotificationMethodOption.Slack, + }} + index={0} + onUpdate={mockOnUpdate} + onRemove={mockOnRemove} + onInputChange={mockOnInputChange} + email_subject={mockEmailSubject} + defaultSubject={mockDefaultSubject} + setErrorSubject={mockSetErrorSubject} + />, + ); + + expect( + screen.getByText('Recipients are separated by "," or ";"'), + ).toBeInTheDocument(); + }); + + describe('RefreshLabel functionality', () => { + it('should call updateSlackOptions with force true when RefreshLabel is clicked', async () => { + // Set feature flag so that SlackV2 branch renders RefreshLabel + window.featureFlags = { [FeatureFlag.AlertReportSlackV2]: true }; + // Spy on SupersetClient.get which is called by updateSlackOptions + const supersetClientSpy = jest + .spyOn(SupersetClient, 'get') + .mockImplementation( + () => + Promise.resolve({ json: { result: [] } }) as unknown as Promise< + Response | JsonResponse | TextResponse + >, + ); + + render( + <NotificationMethod + setting={mockSettingSlackV2} + index={0} + onUpdate={mockOnUpdate} + onRemove={mockOnRemove} + onInputChange={mockOnInputChange} + email_subject={mockEmailSubject} + defaultSubject={mockDefaultSubject} + setErrorSubject={mockSetErrorSubject} + />, + ); + + // Wait for RefreshLabel to be rendered (it may have a tooltip with the provided content) + const refreshLabel = await waitFor(() => + screen.getByLabelText('refresh'), + ); + // Simulate a click on the RefreshLabel + userEvent.click(refreshLabel); + // Verify that the SupersetClient.get was called indicating that updateSlackOptions executed + await waitFor(() => { + expect(supersetClientSpy).toHaveBeenCalled(); + }); + }); + }); }); diff --git a/superset-frontend/src/features/alerts/components/NotificationMethod.tsx b/superset-frontend/src/features/alerts/components/NotificationMethod.tsx index 17a78697c9..60797929d1 100644 --- a/superset-frontend/src/features/alerts/components/NotificationMethod.tsx +++ b/superset-frontend/src/features/alerts/components/NotificationMethod.tsx @@ -36,6 +36,7 @@ import { } from '@superset-ui/core'; import { Select } from 'src/components'; import Icons from 'src/components/Icons'; +import RefreshLabel from 'src/components/RefreshLabel'; import { NotificationMethodOption, NotificationSetting, @@ -76,6 +77,9 @@ const StyledNotificationMethod = styled.div` margin-left: ${theme.gridUnit * 2}px; padding-top: ${theme.gridUnit}px; } + .anticon { + margin-left: ${theme.gridUnit}px; + } } .ghost-button { @@ -248,16 +252,68 @@ export const NotificationMethod: FunctionComponent<NotificationMethodProps> = ({ searchString = '', types = [], exactMatch = false, + force = false, }: { searchString?: string | undefined; types?: string[]; exactMatch?: boolean | undefined; + force?: boolean | undefined; } = {}): Promise<JsonResponse> => { - const queryString = rison.encode({ searchString, types, exactMatch }); + const queryString = rison.encode({ + searchString, + types, + exactMatch, + force, + }); const endpoint = `/api/v1/report/slack_channels/?q=${queryString}`; return SupersetClient.get({ endpoint }); }; + const updateSlackOptions = async ({ + force, + }: { + force?: boolean | undefined; + } = {}) => { + fetchSlackChannels({ types: ['public_channel', 'private_channel'], force }) + .then(({ json }) => { + const { result } = json; + const options: SlackOptionsType = mapChannelsToOptions(result); + + setSlackOptions(options); + + if (isFeatureEnabled(FeatureFlag.AlertReportSlackV2)) { + // 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, + recipientValue, + slackOptions: [ + ...publicOptions.options, + ...privateOptions.options, + ], + }), + ); + } + } + }) + .catch(e => { + // Fallback to slack v1 if slack v2 is not compatible + setUseSlackV1(true); + }) + .finally(() => { + setMethodOptionsLoading(false); + }); + }; + useEffect(() => { const slackEnabled = options?.some( option => @@ -265,44 +321,7 @@ export const NotificationMethod: FunctionComponent<NotificationMethodProps> = ({ option === NotificationMethodOption.SlackV2, ); if (slackEnabled && !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)) { - // 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, - recipientValue, - slackOptions: [ - ...publicOptions.options, - ...privateOptions.options, - ], - }), - ); - } - } - }) - .catch(e => { - // Fallback to slack v1 if slack v2 is not compatible - setUseSlackV1(true); - }) - .finally(() => { - setMethodOptionsLoading(false); - }); + updateSlackOptions(); } }, []); @@ -518,18 +537,24 @@ export const NotificationMethod: FunctionComponent<NotificationMethodProps> = ({ </> ) : ( // for SlackV2 - <Select - ariaLabel={t('Select channels')} - mode="multiple" - name="recipients" - value={slackRecipients} - options={slackOptions} - onChange={onSlackRecipientsChange} - allowClear - data-test="recipients" - allowSelectAll={false} - labelInValue - /> + <div className="input-container"> + <Select + ariaLabel={t('Select channels')} + mode="multiple" + name="recipients" + value={slackRecipients} + options={slackOptions} + onChange={onSlackRecipientsChange} + allowClear + data-test="recipients" + allowSelectAll={false} + labelInValue + /> + <RefreshLabel + onClick={() => updateSlackOptions({ force: true })} + tooltipContent={t('Force refresh Slack channels list')} + /> + </div> )} </div> </StyledInputContainer>
