From b7a9c220e14c6e85840568da4bf87be84b246749 Mon Sep 17 00:00:00 2001 From: Ross Mabbett <92495987+rtexelm@users.noreply.github.com> Date: Fri, 1 Dec 2023 19:32:08 -0500 Subject: [PATCH] fix(Alerts/Reports): allow use of ";" separator in slack recipient entry (#25894) Co-authored-by: John Bodley <4567245+john-bodley@users.noreply.github.com> --- superset/reports/notifications/slack.py | 11 +++- .../reports/notifications/slack_tests.py | 58 +++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 tests/unit_tests/reports/notifications/slack_tests.py diff --git a/superset/reports/notifications/slack.py b/superset/reports/notifications/slack.py index a769622b5..fbae398bc 100644 --- a/superset/reports/notifications/slack.py +++ b/superset/reports/notifications/slack.py @@ -44,6 +44,7 @@ from superset.reports.notifications.exceptions import ( NotificationParamException, NotificationUnprocessableException, ) +from superset.utils.core import get_email_address_list from superset.utils.decorators import statsd_gauge logger = logging.getLogger(__name__) @@ -60,7 +61,15 @@ class SlackNotification(BaseNotification): # pylint: disable=too-few-public-met type = ReportRecipientType.SLACK def _get_channel(self) -> str: - return json.loads(self._recipient.recipient_config_json)["target"] + """ + 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) + """ + recipient_str = json.loads(self._recipient.recipient_config_json)["target"] + + return ",".join(get_email_address_list(recipient_str)) def _message_template(self, table: str = "") -> str: return __( diff --git a/tests/unit_tests/reports/notifications/slack_tests.py b/tests/unit_tests/reports/notifications/slack_tests.py new file mode 100644 index 000000000..0a5e9baa4 --- /dev/null +++ b/tests/unit_tests/reports/notifications/slack_tests.py @@ -0,0 +1,58 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +import pandas as pd + + +def test_get_channel_with_multi_recipients() -> None: + """ + Test the _get_channel function to ensure it will return a string + with recipients separated by commas without interstitial spacing + """ + from superset.reports.models import ReportRecipients, ReportRecipientType + from superset.reports.notifications.base import NotificationContent + from superset.reports.notifications.slack import SlackNotification + + content = NotificationContent( + name="test alert", + header_data={ + "notification_format": "PNG", + "notification_type": "Alert", + "owners": [1], + "notification_source": None, + "chart_id": None, + "dashboard_id": None, + }, + embedded_data=pd.DataFrame( + { + "A": [1, 2, 3], + "B": [4, 5, 6], + "C": ["111", "222", '333'], + } + ), + description='

This is a test alert


', + ) + slack_notification = SlackNotification( + recipient=ReportRecipients( + type=ReportRecipientType.SLACK, + recipient_config_json='{"target": "some_channel; second_channel, third_channel"}', + ), + content=content, + ) + + result = slack_notification._get_channel() + + assert result == "some_channel,second_channel,third_channel"