eschutho commented on code in PR #28797:
URL: https://github.com/apache/superset/pull/28797#discussion_r1628529153
##########
superset/reports/notifications/slack.py:
##########
@@ -60,16 +61,24 @@ class SlackNotification(BaseNotification): # pylint:
disable=too-few-public-met
type = ReportRecipientType.SLACK
- def _get_channel(self) -> str:
+ def _get_channels(self, client: WebClient) -> List[str]:
"""
Get the recipient's channel(s).
- Note Slack SDK uses "channel" to refer to one or more
- channels. Multiple channels are demarcated by a comma.
- :returns: The comma separated list of channel(s)
+ :returns: A list of channel ids: "EID676L"
"""
recipient_str =
json.loads(self._recipient.recipient_config_json)["target"]
- return ",".join(get_email_address_list(recipient_str))
+ channel_recipients = get_email_address_list(recipient_str)
+
+ conversations_list_response = client.conversations_list(
+ types="public_channel,private_channel"
Review Comment:
I was thinking about this more @betodealmeida and it would be a breaking
change because the existing applications would need the `channels:read` slack
scope for this, but maybe for the next major release, it would a nice
improvement to have a multiselect ui for the channel and then also store the
id. ❤️
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
