This is an automated email from the ASF dual-hosted git repository. michaelsmolina pushed a commit to branch 3.0 in repository https://gitbox.apache.org/repos/asf/superset.git
commit fe6acfe614e6663f2cb1b1503bce8e3cc45d1972 Author: Ross Mabbett <[email protected]> AuthorDate: Fri Dec 1 19:32:08 2023 -0500 fix(Alerts/Reports): allow use of ";" separator in slack recipient entry (#25894) Co-authored-by: John Bodley <[email protected]> (cherry picked from commit b7a9c220e14c6e85840568da4bf87be84b246749) --- superset/reports/notifications/slack.py | 11 +++- superset/views/base.py | 24 ++++----- superset/views/core.py | 4 +- .../reports/notifications/slack_tests.py | 58 ++++++++++++++++++++++ 4 files changed, 82 insertions(+), 15 deletions(-) diff --git a/superset/reports/notifications/slack.py b/superset/reports/notifications/slack.py index 4c3f2ee419..c72a4f969f 100644 --- a/superset/reports/notifications/slack.py +++ b/superset/reports/notifications/slack.py @@ -43,6 +43,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__) @@ -59,7 +60,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/superset/views/base.py b/superset/views/base.py index c77a3e5c87..a897933638 100644 --- a/superset/views/base.py +++ b/superset/views/base.py @@ -22,7 +22,7 @@ import logging import os import traceback from datetime import datetime -from typing import Any, Callable, cast, Optional, Union +from typing import Any, Callable, cast import simplejson as json import yaml @@ -140,10 +140,10 @@ def get_error_msg() -> str: def json_error_response( - msg: Optional[str] = None, + msg: str | None = None, status: int = 500, - payload: Optional[dict[str, Any]] = None, - link: Optional[str] = None, + payload: dict[str, Any] | None = None, + link: str | None = None, ) -> FlaskResponse: if not payload: payload = {"error": f"{msg}"} @@ -160,7 +160,7 @@ def json_error_response( def json_errors_response( errors: list[SupersetError], status: int = 500, - payload: Optional[dict[str, Any]] = None, + payload: dict[str, Any] | None = None, ) -> FlaskResponse: if not payload: payload = {} @@ -183,7 +183,7 @@ def data_payload_response(payload_json: str, has_error: bool = False) -> FlaskRe def generate_download_headers( - extension: str, filename: Optional[str] = None + extension: str, filename: str | None = None ) -> dict[str, Any]: filename = filename if filename else datetime.now().strftime("%Y%m%d_%H%M%S") content_disp = f"attachment; filename={filename}.{extension}" @@ -193,7 +193,7 @@ def generate_download_headers( def deprecated( eol_version: str = "4.0.0", - new_target: Optional[str] = None, + new_target: str | None = None, ) -> Callable[[Callable[..., FlaskResponse]], Callable[..., FlaskResponse]]: """ A decorator to set an API endpoint from SupersetView has deprecated. @@ -201,7 +201,7 @@ def deprecated( """ def _deprecated(f: Callable[..., FlaskResponse]) -> Callable[..., FlaskResponse]: - def wraps(self: "BaseSupersetView", *args: Any, **kwargs: Any) -> FlaskResponse: + def wraps(self: BaseSupersetView, *args: Any, **kwargs: Any) -> FlaskResponse: messsage = ( "%s.%s " "This API endpoint is deprecated and will be removed in version %s" @@ -228,7 +228,7 @@ def api(f: Callable[..., FlaskResponse]) -> Callable[..., FlaskResponse]: return the response in the JSON format """ - def wraps(self: "BaseSupersetView", *args: Any, **kwargs: Any) -> FlaskResponse: + def wraps(self: BaseSupersetView, *args: Any, **kwargs: Any) -> FlaskResponse: try: return f(self, *args, **kwargs) except NoAuthorizationError: @@ -250,7 +250,7 @@ def handle_api_exception( exceptions. """ - def wraps(self: "BaseSupersetView", *args: Any, **kwargs: Any) -> FlaskResponse: + def wraps(self: BaseSupersetView, *args: Any, **kwargs: Any) -> FlaskResponse: try: return f(self, *args, **kwargs) except SupersetSecurityException as ex: @@ -593,11 +593,11 @@ class YamlExportMixin: # pylint: disable=too-few-public-methods Used on DatabaseView for cli compatibility """ - yaml_dict_key: Optional[str] = None + yaml_dict_key: str | None = None @action("yaml_export", __("Export to YAML"), __("Export to YAML?"), "fa-download") def yaml_export( - self, items: Union[ImportExportMixin, list[ImportExportMixin]] + self, items: ImportExportMixin | list[ImportExportMixin] ) -> FlaskResponse: if not isinstance(items, list): items = [items] diff --git a/superset/views/core.py b/superset/views/core.py index 22e09f9ff4..b305a57e8c 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -1011,7 +1011,7 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods abort(404) payload = { "user": bootstrap_user_data(user, include_perms=True), - "common": common_bootstrap_payload(user), + "common": common_bootstrap_payload(), } return self.render_template( @@ -1082,7 +1082,7 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods """SQL Editor""" payload = { "defaultDbId": config["SQLLAB_DEFAULT_DBID"], - "common": common_bootstrap_payload(g.user), + "common": common_bootstrap_payload(), **self._get_sqllab_tabs(get_user_id()), } 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 0000000000..0a5e9baa46 --- /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", '<a href="http://www.example.com";>333</a>'], + } + ), + description='<p>This is <a href="#">a test</a> alert</p><br />', + ) + 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"
