display private channels

superset-frontend/src/components/Select/Select.test.tsx

@@ -188,6 +188,20 @@ test('does not add a new option if the value is already in the options', async (
  expect(options).toHaveLength(1);
 });
 
+test('does not add new options when the value is in a nested/grouped option', async () => {
+ const options = [
+ {
+ label: 'Group',
+ options: [OPTIONS[0]],
+ },
+ ];
+ render(<Select {...defaultProps} options={options} value={OPTIONS[0]} />);
+ await open();
+ expect(await findSelectOption(OPTIONS[0].label)).toBeInTheDocument();
+ const selectOptions = await findAllSelectOptions();
+ expect(selectOptions).toHaveLength(1);
+});
+
 test('inverts the selection', async () => {
  render(<Select {...defaultProps} invertSelection />);
  await open();

superset-frontend/src/components/Select/Select.tsx

@@ -182,8 +182,18 @@ const Select = forwardRef(
 
  // add selected values to options list if they are not in it
  const fullSelectOptions = useMemo(() => {
+ // check to see if selectOptions are grouped
+ let groupedOptions: SelectOptionsType;
+ if (selectOptions.some(opt => opt.options)) {
+ groupedOptions = selectOptions.reduce(
+ (acc, group) => [...acc, ...group.options],
+ [] as SelectOptionsType,
+ );
+ }
  const missingValues: SelectOptionsType = ensureIsArray(selectValue)
- .filter(opt => !hasOption(getValue(opt), selectOptions))
+ .filter(
+ opt => !hasOption(getValue(opt), groupedOptions || selectOptions),
+ )
  .map(opt =>
  isLabeledValue(opt) ? opt : { value: opt, label: String(opt) },
  );

superset-frontend/src/features/alerts/components/NotificationMethod.test.tsx

@@ -17,7 +17,7 @@
  * under the License.
  */
 import { render, screen, fireEvent } from 'spec/helpers/testing-library';
-import { NotificationMethod, mapSlackOptions } from './NotificationMethod';
+import { NotificationMethod, mapSlackValues } from './NotificationMethod';
 import { NotificationMethodOption, NotificationSetting } from '../types';
 
 const mockOnUpdate = jest.fn();
@@ -138,14 +138,12 @@ describe('NotificationMethod', () => {
  it('should correctly map recipients when method is SlackV2', () => {
  const method = 'SlackV2';
  const recipientValue = 'user1,user2';
- const slackOptions = {
- data: [
- { label: 'User One', value: 'user1' },
- { label: 'User Two', value: 'user2' },
- ],
- };
+ const slackOptions: { label: string; value: string }[] = [
+ { label: 'User One', value: 'user1' },
+ { label: 'User Two', value: 'user2' },
+ ];
 
- const result = mapSlackOptions({ method, recipientValue, slackOptions });
+ const result = mapSlackValues({ method, recipientValue, slackOptions });
 
  expect(result).toEqual([
  { label: 'User One', value: 'user1' },
@@ -156,31 +154,25 @@ describe('NotificationMethod', () => {
  it('should return an empty array when recipientValue is an empty string', () => {
  const method = 'SlackV2';
  const recipientValue = '';
- const slackOptions = {
- data: [
- { label: 'User One', value: 'user1' },
- { label: 'User Two', value: 'user2' },
- { label: 'User Three', value: 'user3' },
- ],
- };
+ const slackOptions: { label: string; value: string }[] = [
+ { label: 'User One', value: 'user1' },
+ { label: 'User Two', value: 'user2' },
+ ];
 
- const result = mapSlackOptions({ method, recipientValue, slackOptions });
+ const result = mapSlackValues({ method, recipientValue, slackOptions });
 
  expect(result).toEqual([]);
  });
- // Ensure that the mapSlackOptions function correctly maps recipients when the method is Slack with updated recipient values
+ // Ensure that the mapSlackValues function correctly maps recipients when the method is Slack with updated recipient values
  it('should correctly map recipients when method is Slack with updated recipient values', () => {
  const method = 'Slack';
  const recipientValue = 'User One,User Two';
- const slackOptions = {
- data: [
- { label: 'User One', value: 'user1' },
- { label: 'User Two', value: 'user2' },
- { label: 'User Three', value: 'user3' },
- ],
- };
-
- const result = mapSlackOptions({ method, recipientValue, slackOptions });
+ const slackOptions: { label: string; value: string }[] = [
+ { label: 'User One', value: 'user1' },
+ { label: 'User Two', value: 'user2' },
+ ];
+
+ const result = mapSlackValues({ method, recipientValue, slackOptions });
 
  expect(result).toEqual([
  { label: 'User One', value: 'user1' },

superset-frontend/src/features/alerts/components/NotificationMethod.tsx

@@ -23,9 +23,11 @@ import {
  useEffect,
  useMemo,
 } from 'react';
+import rison from 'rison';
 
 import {
  FeatureFlag,
+ JsonResponse,
  SupersetClient,
  isFeatureEnabled,
  styled,
@@ -90,27 +92,68 @@ const TRANSLATIONS = {
  ),
 };
 
-export const mapSlackOptions = ({
+export const mapSlackValues = ({
  method,
  recipientValue,
  slackOptions,
 }: {
  method: string;
  recipientValue: string;
- slackOptions: { data: { label: string; value: string }[] };
+ slackOptions: { label: string; value: string }[];
 }) => {
  const prop = method === NotificationMethodOption.SlackV2 ? 'value' : 'label';
  return recipientValue
  .split(',')
  .map(recipient =>
- slackOptions.data.find(
+ slackOptions.find(
  option =>
  option[prop].trim().toLowerCase() === recipient.trim().toLowerCase(),
  ),
  )
  .filter(val => !!val) as { label: string; value: string }[];
 };
 
+export const mapChannelsToOptions = (result: SlackChannel[]) => {
+ const publicChannels: SlackChannel[] = [];
+ const privateChannels: SlackChannel[] = [];
+
+ result.forEach(channel => {
+ if (channel.is_private) {
+ privateChannels.push(channel);
+ } else {
+ publicChannels.push(channel);
+ }
+ });
+
+ return [
+ {
+ label: 'Public Channels',
+ options: publicChannels.map((channel: SlackChannel) => ({
+ label: `${channel.name} ${
+ channel.is_member ? '' : '(Bot not in channel)'
+ }`,
+ value: channel.id,
+ key: channel.id,
+ })),
+ key: 'public',
+ },
+ {
+ label: 'Private Channels (Bot in channel)',
+ options: privateChannels.map((channel: SlackChannel) => ({
+ label: channel.name,
+ value: channel.id,
+ key: channel.id,
+ })),
+ key: 'private',
+ },
+ ];
+};
+
+type SlackOptionsType = {
+ label: string;
+ options: { label: string; value: string }[];
+}[];
+
 export const NotificationMethod: FunctionComponent<NotificationMethodProps> = ({
  setting = null,
  index,
@@ -130,21 +173,15 @@ export const NotificationMethod: FunctionComponent<NotificationMethodProps> = ({
  >([]);
  const [error, setError] = useState(false);
  const theme = useTheme();
- const [slackOptions, setSlackOptions] = useState<{
- data: { label: string; value: string }[];
- totalCount: number;
- }>({ data: [], totalCount: 0 });
+ const [slackOptions, setSlackOptions] = useState<SlackOptionsType>([
+ {
+ label: '',
+ options: [],
+ },
+ ]);
 
  const [useSlackV1, setUseSlackV1] = useState<boolean>(false);
 
- const mapChannelsToOptions = (result: SlackChannel[]) =>
- result.map((channel: SlackChannel) => ({
- label: `${channel.name} ${
- channel.is_member ? '' : '<i>(Bot not in channel)</i>'
- }`,
- value: channel.id,
- }));
-
  const onMethodChange = (selected: {
  label: string;
  value: NotificationMethodOption;
@@ -162,37 +199,50 @@ export const NotificationMethod: FunctionComponent<NotificationMethodProps> = ({
  }
  };
 
+ const fetchSlackChannels = async ({
+ searchString = '',
+ types = [],
+ exactMatch = false,
+ }: {
+ searchString?: string | undefined;
+ types?: string[];
+ exactMatch?: boolean | undefined;
+ } = {}): Promise<JsonResponse> => {
+ const queryString = rison.encode({ searchString, types, exactMatch });
+ const endpoint = `/api/v1/report/slack_channels/?q=${queryString}`;
+ return SupersetClient.get({ endpoint });
+ };
+
  useEffect(() => {
- const endpoint = '/api/v1/report/slack_channels/';
  if (
  method &&
  [
  NotificationMethodOption.Slack,
  NotificationMethodOption.SlackV2,
  ].includes(method) &&
- !slackOptions.data.length
+ !slackOptions[0].options.length
  ) {
- SupersetClient.get({ endpoint })
+ fetchSlackChannels({ types: ['public_channel', 'private_channel'] })
  .then(({ json }) => {
- const { result, count } = json;
+ const { result } = json;
 
- const options: { label: any; value: any }[] =
- mapChannelsToOptions(result);
+ const options: SlackOptionsType = mapChannelsToOptions(result);
 
- setSlackOptions({
- data: options,
- totalCount: (count ?? options.length) as number,
- });
+ setSlackOptions(options);
 
- // on first load, if the Slack channels api succeeds,
- // then convert this to SlackV2
  if (isFeatureEnabled(FeatureFlag.AlertReportSlackV2)) {
- // convert v1 names to v2 ids
+ // map existing ids to names for display
+ // or names to ids if slack v1
+ const [publicOptions, privateOptions] = options;
+
  setSlackRecipients(
- mapSlackOptions({
+ mapSlackValues({
  method,
  recipientValue,
- slackOptions: { data: options },
+ slackOptions: [
+ ...publicOptions.options,
+ ...privateOptions.options,
+ ],
  }),
  );
  if (method === NotificationMethodOption.Slack) {
@@ -210,7 +260,7 @@ export const NotificationMethod: FunctionComponent<NotificationMethodProps> = ({
  }
  }, [method]);
 
- const formattedOptions = useMemo(
+ const methodOptions = useMemo(
  () =>
  (options || [])
  .filter(
@@ -300,9 +350,9 @@ export const NotificationMethod: FunctionComponent<NotificationMethodProps> = ({
  labelInValue
  onChange={onMethodChange}
  placeholder={t('Select Delivery Method')}
- options={formattedOptions}
+ options={methodOptions}
  showSearch
- value={formattedOptions.find(option => option.value === method)}
+ value={methodOptions.find(option => option.value === method)}
  />
  {index !== 0 && !!onRemove ? (
  <span
@@ -385,7 +435,7 @@ export const NotificationMethod: FunctionComponent<NotificationMethodProps> = ({
  mode="multiple"
  name="recipients"
  value={slackRecipients}
- options={slackOptions.data}
+ options={slackOptions}
  onChange={onSlackRecipientsChange}
  allowClear
  data-test="recipients"

superset-frontend/src/features/alerts/components/RecipientIcon.test.tsx

@@ -0,0 +1,32 @@
+import { render, screen } from 'spec/helpers/testing-library';
+import RecipientIcon from './RecipientIcon';
+import { NotificationMethodOption } from '../types';
+
+describe('RecipientIcon', () => {
+ it('should render the email icon when type is Email', () => {
+ render(<RecipientIcon type={NotificationMethodOption.Email} />);
+ const regexPattern = new RegExp(NotificationMethodOption.Email, 'i');
+ const emailIcon = screen.getByLabelText(regexPattern);
+ expect(emailIcon).toBeInTheDocument();
+ });
+
+ it('should render the Slack icon when type is Slack', () => {
+ render(<RecipientIcon type={NotificationMethodOption.Slack} />);
+ const regexPattern = new RegExp(NotificationMethodOption.Slack, 'i');
+ const slackIcon = screen.getByLabelText(regexPattern);
+ expect(slackIcon).toBeInTheDocument();
+ });
+
+ it('should render the Slack icon when type is SlackV2', () => {
+ render(<RecipientIcon type={NotificationMethodOption.SlackV2} />);
+ const regexPattern = new RegExp(NotificationMethodOption.Slack, 'i');
+ const slackIcon = screen.getByLabelText(regexPattern);
+ expect(slackIcon).toBeInTheDocument();
+ });
+
+ it('should not render any icon when type is not recognized', () => {
+ render(<RecipientIcon type="unknown" />);
+ const icons = screen.queryByLabelText(/.*/);
+ expect(icons).not.toBeInTheDocument();
+ });
+});

superset-frontend/src/features/alerts/types.ts

@@ -57,6 +57,7 @@ export type SlackChannel = {
  id: string;
  name: string;
  is_member: boolean;
+ is_private: boolean;
 };
 
 export type Recipient = {

superset/commands/report/execute.py

@@ -76,7 +76,7 @@
 from superset.utils.decorators import logs_context, transaction
 from superset.utils.pdf import build_pdf_from_screenshots
 from superset.utils.screenshots import ChartScreenshot, DashboardScreenshot
-from superset.utils.slack import get_channels_with_search
+from superset.utils.slack import get_channels_with_search, SlackChannelTypes
 from superset.utils.urls import get_url_path
 
 logger = logging.getLogger(__name__)
@@ -138,8 +138,18 @@ def update_report_schedule_slack_v2(self) -> None:
  if recipient_copy.type == ReportRecipientType.SLACK:
  recipient_copy.type = ReportRecipientType.SLACKV2
  slack_recipients = json.loads(recipient_copy.recipient_config_json)
+ # we need to ensure that existing reports can also fetch
+ # ids from private channels
  recipient_copy.recipient_config_json = json.dumps(
- {"target": get_channels_with_search(slack_recipients["target"])}
+ {
+ "target": get_channels_with_search(
+ slack_recipients["target"],
+ types=[
+ SlackChannelTypes.PRIVATE,
+ SlackChannelTypes.PUBLIC,
+ ],
+ )
+ }
  )
 
  updated_recipients.append(recipient_copy)