This is an automated email from the ASF dual-hosted git repository.
vavila pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new d2e0e2b79c fix(Slack): Fix Slack recipients migration to V2 (#32336)
d2e0e2b79c is described below
commit d2e0e2b79caa25562d139032655141732b08a24c
Author: Vitor Avila <[email protected]>
AuthorDate: Thu Mar 6 08:52:15 2025 -0300
fix(Slack): Fix Slack recipients migration to V2 (#32336)
---
superset/commands/report/execute.py | 78 +++++++-----
superset/reports/notifications/slack.py | 4 +-
superset/reports/notifications/slackv2.py | 4 +-
superset/reports/schemas.py | 16 ++-
superset/utils/core.py | 14 ++-
superset/utils/slack.py | 25 ++--
tests/integration_tests/reports/commands_tests.py | 140 +++++++++++++++++++---
tests/integration_tests/reports/utils.py | 6 +-
tests/integration_tests/utils_tests.py | 14 +--
tests/unit_tests/commands/report/execute_test.py | 77 ++++++++++++
tests/unit_tests/utils/slack_test.py | 16 ++-
11 files changed, 317 insertions(+), 77 deletions(-)
diff --git a/superset/commands/report/execute.py
b/superset/commands/report/execute.py
index 0c57d55bb5..2e94b8c4b8 100644
--- a/superset/commands/report/execute.py
+++ b/superset/commands/report/execute.py
@@ -72,7 +72,7 @@ from superset.reports.notifications.exceptions import (
)
from superset.tasks.utils import get_executor
from superset.utils import json
-from superset.utils.core import HeaderDataType, override_user
+from superset.utils.core import HeaderDataType, override_user,
recipients_string_to_list
from superset.utils.csv import get_chart_csv_data, get_chart_dataframe
from superset.utils.decorators import logs_context, transaction
from superset.utils.pdf import build_pdf_from_screenshots
@@ -137,24 +137,40 @@ class BaseReportState:
if recipient.type == ReportRecipientType.SLACK:
recipient.type = ReportRecipientType.SLACKV2
slack_recipients =
json.loads(recipient.recipient_config_json)
+ # V1 method allowed to use leading `#` in the channel name
+ channel_names = (slack_recipients["target"] or
"").replace("#", "")
# we need to ensure that existing reports can also fetch
# ids from private channels
+ channels = get_channels_with_search(
+ search_string=channel_names,
+ types=[
+ SlackChannelTypes.PRIVATE,
+ SlackChannelTypes.PUBLIC,
+ ],
+ exact_match=True,
+ )
+ channels_list = recipients_string_to_list(channel_names)
+ if len(channels_list) != len(channels):
+ missing_channels = set(channels_list) - {
+ channel["name"] for channel in channels
+ }
+ msg = (
+ "Could not find the following channels: "
+ f"{', '.join(missing_channels)}"
+ )
+ raise UpdateFailedError(msg)
+ channel_ids = ",".join(channel["id"] for channel in
channels)
recipient.recipient_config_json = json.dumps(
{
- "target": get_channels_with_search(
- slack_recipients["target"],
- types=[
- SlackChannelTypes.PRIVATE,
- SlackChannelTypes.PUBLIC,
- ],
- )
+ "target": channel_ids,
}
)
except Exception as ex:
- logger.warning(
- "Failed to update slack recipients to v2: %s", str(ex),
exc_info=True
- )
- raise UpdateFailedError from ex
+ # Revert to v1 to preserve configuration (requires manual fix)
+ recipient.type = ReportRecipientType.SLACK
+ msg = f"Failed to update slack recipients to v2: {str(ex)}"
+ logger.exception(msg)
+ raise UpdateFailedError(msg) from ex
def create_log(self, error_message: Optional[str] = None) -> None:
"""
@@ -553,30 +569,32 @@ class BaseReportState:
for recipient in recipients:
notification = create_notification(recipient, notification_content)
try:
- if app.config["ALERT_REPORTS_NOTIFICATION_DRY_RUN"]:
+ try:
+ if app.config["ALERT_REPORTS_NOTIFICATION_DRY_RUN"]:
+ logger.info(
+ "Would send notification for alert %s, to %s. "
+ "ALERT_REPORTS_NOTIFICATION_DRY_RUN is enabled, "
+ "set it to False to send notifications.",
+ self._report_schedule.name,
+ recipient.recipient_config_json,
+ )
+ else:
+ notification.send()
+ except SlackV1NotificationError as ex:
+ # The slack notification should be sent with the v2 api
logger.info(
- "Would send notification for alert %s, to %s. "
- "ALERT_REPORTS_NOTIFICATION_DRY_RUN is enabled, "
- "set it to False to send notifications.",
- self._report_schedule.name,
- recipient.recipient_config_json,
+ "Attempting to upgrade the report to Slackv2: %s",
str(ex)
)
- else:
- notification.send()
- except SlackV1NotificationError as ex:
- # The slack notification should be sent with the v2 api
- logger.info("Attempting to upgrade the report to Slackv2: %s",
str(ex))
- try:
self.update_report_schedule_slack_v2()
recipient.type = ReportRecipientType.SLACKV2
notification = create_notification(recipient,
notification_content)
notification.send()
- except (UpdateFailedError, NotificationParamException) as err:
- # log the error but keep processing the report with SlackV1
- logger.warning(
- "Failed to update slack recipients to v2: %s", str(err)
- )
- except (NotificationError, SupersetException) as ex:
+ except (
+ UpdateFailedError,
+ NotificationParamException,
+ NotificationError,
+ SupersetException,
+ ) as ex:
# collect errors but keep processing them
notification_errors.append(
SupersetError(
diff --git a/superset/reports/notifications/slack.py
b/superset/reports/notifications/slack.py
index 4622a45606..589fddb9aa 100644
--- a/superset/reports/notifications/slack.py
+++ b/superset/reports/notifications/slack.py
@@ -43,7 +43,7 @@ from superset.reports.notifications.exceptions import (
)
from superset.reports.notifications.slack_mixin import SlackMixin
from superset.utils import json
-from superset.utils.core import get_email_address_list
+from superset.utils.core import recipients_string_to_list
from superset.utils.decorators import statsd_gauge
from superset.utils.slack import (
get_slack_client,
@@ -70,7 +70,7 @@ class SlackNotification(SlackMixin, BaseNotification): #
pylint: disable=too-fe
"""
recipient_str =
json.loads(self._recipient.recipient_config_json)["target"]
- return ",".join(get_email_address_list(recipient_str))
+ return ",".join(recipients_string_to_list(recipient_str))
def _get_inline_files(
self,
diff --git a/superset/reports/notifications/slackv2.py
b/superset/reports/notifications/slackv2.py
index 824d4bd326..19b5d98e00 100644
--- a/superset/reports/notifications/slackv2.py
+++ b/superset/reports/notifications/slackv2.py
@@ -42,7 +42,7 @@ from superset.reports.notifications.exceptions import (
)
from superset.reports.notifications.slack_mixin import SlackMixin
from superset.utils import json
-from superset.utils.core import get_email_address_list
+from superset.utils.core import recipients_string_to_list
from superset.utils.decorators import statsd_gauge
from superset.utils.slack import get_slack_client
@@ -64,7 +64,7 @@ class SlackV2Notification(SlackMixin, BaseNotification): #
pylint: disable=too-
""" # noqa: E501
recipient_str =
json.loads(self._recipient.recipient_config_json)["target"]
- return get_email_address_list(recipient_str)
+ return recipients_string_to_list(recipient_str)
def _get_inline_files(
self,
diff --git a/superset/reports/schemas.py b/superset/reports/schemas.py
index 7078970b38..cfccc579bc 100644
--- a/superset/reports/schemas.py
+++ b/superset/reports/schemas.py
@@ -19,7 +19,7 @@ from typing import Any, Optional, Union
from croniter import croniter
from flask import current_app
from flask_babel import gettext as _
-from marshmallow import fields, Schema, validate, validates, validates_schema
+from marshmallow import EXCLUDE, fields, Schema, validate, validates,
validates_schema
from marshmallow.validate import Length, Range, ValidationError
from pytz import all_timezones
@@ -399,3 +399,17 @@ class ReportSchedulePutSchema(Schema):
max=max_width,
)
)
+
+
+class SlackChannelSchema(Schema):
+ """
+ Schema to load Slack channels, set to ignore any fields not used by
Superset.
+ """
+
+ class Meta:
+ unknown = EXCLUDE
+
+ id = fields.String()
+ name = fields.String()
+ is_member = fields.Boolean()
+ is_private = fields.Boolean()
diff --git a/superset/utils/core.py b/superset/utils/core.py
index 2aaf912229..9e1bee5ecb 100644
--- a/superset/utils/core.py
+++ b/superset/utils/core.py
@@ -702,7 +702,7 @@ def send_email_smtp( # pylint:
disable=invalid-name,too-many-arguments,too-many
'[email protected]', 'foo', '<b>Foo</b> bar',['/dev/null'], dryrun=True)
"""
smtp_mail_from = config["SMTP_MAIL_FROM"]
- smtp_mail_to = get_email_address_list(to)
+ smtp_mail_to = recipients_string_to_list(to)
msg = MIMEMultipart(mime_subtype)
msg["Subject"] = subject
@@ -713,14 +713,14 @@ def send_email_smtp( # pylint:
disable=invalid-name,too-many-arguments,too-many
recipients = smtp_mail_to
if cc:
- smtp_mail_cc = get_email_address_list(cc)
+ smtp_mail_cc = recipients_string_to_list(cc)
msg["Cc"] = ", ".join(smtp_mail_cc)
recipients = recipients + smtp_mail_cc
smtp_mail_bcc = []
if bcc:
# don't add bcc in header
- smtp_mail_bcc = get_email_address_list(bcc)
+ smtp_mail_bcc = recipients_string_to_list(bcc)
recipients = recipients + smtp_mail_bcc
msg["Date"] = formatdate(localtime=True)
@@ -813,7 +813,13 @@ def send_mime_email(
smtp.quit()
-def get_email_address_list(address_string: str) -> list[str]:
+def recipients_string_to_list(address_string: str | None) -> list[str]:
+ """
+ Returns the list of target recipients for alerts and reports.
+
+ Strips values and converts a comma/semicolon separated
+ string into a list.
+ """
address_string_list: list[str] = []
if isinstance(address_string, str):
address_string_list = re.split(r",|\s|;", address_string)
diff --git a/superset/utils/slack.py b/superset/utils/slack.py
index b4c8018ffc..6d51f2765e 100644
--- a/superset/utils/slack.py
+++ b/superset/utils/slack.py
@@ -26,7 +26,9 @@ from slack_sdk.http_retry.builtin_handlers import
RateLimitErrorRetryHandler
from superset import feature_flag_manager
from superset.exceptions import SupersetException
+from superset.reports.schemas import SlackChannelSchema
from superset.utils.backports import StrEnum
+from superset.utils.core import recipients_string_to_list
logger = logging.getLogger(__name__)
@@ -57,7 +59,7 @@ def get_channels_with_search(
limit: int = 999,
types: Optional[list[SlackChannelTypes]] = None,
exact_match: bool = False,
-) -> list[str]:
+) -> list[SlackChannelSchema]:
"""
The slack api is paginated but does not include search, so we need to fetch
all channels and filter them ourselves
@@ -66,7 +68,8 @@ def get_channels_with_search(
try:
client = get_slack_client()
- channels = []
+ channel_schema = SlackChannelSchema()
+ channels: list[SlackChannelSchema] = []
cursor = None
extra_params = {}
extra_params["types"] = ",".join(types) if types else None
@@ -75,29 +78,27 @@ def get_channels_with_search(
response = client.conversations_list(
limit=limit, cursor=cursor, exclude_archived=True,
**extra_params
)
- channels.extend(response.data["channels"])
+ channels.extend(
+ channel_schema.load(channel) for channel in
response.data["channels"]
+ )
cursor = response.data.get("response_metadata",
{}).get("next_cursor")
if not cursor:
break
# The search string can be multiple channels separated by commas
if search_string:
- search_array = [
- search.lower()
- for search in (search_string.split(",") if search_string else
[])
- ]
-
+ search_array = recipients_string_to_list(search_string)
channels = [
channel
for channel in channels
if any(
(
- search == channel["name"].lower()
- or search == channel["id"].lower()
+ search.lower() == channel["name"].lower()
+ or search.lower() == channel["id"].lower()
if exact_match
else (
- search in channel["name"].lower()
- or search in channel["id"].lower()
+ search.lower() in channel["name"].lower()
+ or search.lower() in channel["id"].lower()
)
)
for search in search_array
diff --git a/tests/integration_tests/reports/commands_tests.py
b/tests/integration_tests/reports/commands_tests.py
index 7b22b38fc0..d3798d05ec 100644
--- a/tests/integration_tests/reports/commands_tests.py
+++ b/tests/integration_tests/reports/commands_tests.py
@@ -308,7 +308,10 @@ def create_report_slack_chart():
def create_report_slack_chartv2():
chart = db.session.query(Slice).first()
report_schedule = create_report_notification(
- slack_channel="slack_channel_id", chart=chart,
name="report_slack_chartv2"
+ slack_channel="slack_channel_id",
+ chart=chart,
+ name="report_slack_chartv2",
+ use_slack_v2=True,
)
yield report_schedule
@@ -1300,9 +1303,7 @@ def test_email_dashboard_report_schedule_force_screenshot(
assert_log(ReportState.SUCCESS)
[email protected](
- "load_birth_names_dashboard_with_slices", "create_report_slack_chart"
-)
[email protected]("create_report_slack_chart")
@patch("superset.commands.report.execute.get_channels_with_search")
@patch("superset.reports.notifications.slack.should_use_v2_api",
return_value=True)
@patch("superset.reports.notifications.slackv2.get_slack_client")
@@ -1316,13 +1317,19 @@ def test_slack_chart_report_schedule_converts_to_v2(
):
"""
ExecuteReport Command: Test chart slack report schedule
+ while converting the recipients list to SlackV2.
"""
# setup screenshot mock
screenshot_mock.return_value = SCREENSHOT_FILE
-
channel_id = "slack_channel_id"
-
- get_channels_with_search_mock.return_value = channel_id
+ get_channels_with_search_mock.return_value = [
+ {
+ "id": channel_id,
+ "name": "slack_channel",
+ "is_member": True,
+ "is_private": False,
+ },
+ ]
with freeze_time("2020-01-01T00:00:00Z"):
with patch.object(current_app.config["STATS_LOGGER"], "gauge") as
statsd_mock:
@@ -1357,33 +1364,40 @@ def test_slack_chart_report_schedule_converts_to_v2(
assert statsd_mock.call_args_list[1] ==
call("reports.slack.send.ok", 1)
[email protected](
- "load_birth_names_dashboard_with_slices", "create_report_slack_chartv2"
-)
@patch("superset.commands.report.execute.get_channels_with_search")
@patch("superset.reports.notifications.slack.should_use_v2_api",
return_value=True)
@patch("superset.reports.notifications.slackv2.get_slack_client")
@patch("superset.utils.screenshots.ChartScreenshot.get_screenshot")
-def test_slack_chart_report_schedule_v2(
+def test_slack_chart_report_schedule_converts_to_v2_channel_with_hash(
screenshot_mock,
slack_client_mock,
slack_should_use_v2_api_mock,
get_channels_with_search_mock,
- create_report_slack_chart,
):
"""
- ExecuteReport Command: Test chart slack report schedule
+ ExecuteReport Command: Test converting a Slack report to v2 when
+ the channel name includes the leading hash (supported in v1).
"""
# setup screenshot mock
screenshot_mock.return_value = SCREENSHOT_FILE
channel_id = "slack_channel_id"
-
- get_channels_with_search_mock.return_value = channel_id
+ chart = db.session.query(Slice).first()
+ report_schedule = create_report_notification(
+ slack_channel="#slack_channel", chart=chart
+ )
+ get_channels_with_search_mock.return_value = [
+ {
+ "id": channel_id,
+ "name": "slack_channel",
+ "is_member": True,
+ "is_private": False,
+ },
+ ]
with freeze_time("2020-01-01T00:00:00Z"):
with patch.object(current_app.config["STATS_LOGGER"], "gauge") as
statsd_mock:
AsyncExecuteReportScheduleCommand(
- TEST_ID, create_report_slack_chart.id, datetime.utcnow()
+ TEST_ID, report_schedule.id, datetime.utcnow()
).run()
assert (
@@ -1395,6 +1409,12 @@ def test_slack_chart_report_schedule_v2(
== SCREENSHOT_FILE
)
+ # Assert that the report recipients were updated
+ assert report_schedule.recipients[0].recipient_config_json ==
json.dumps(
+ {"target": channel_id}
+ )
+ assert report_schedule.recipients[0].type ==
ReportRecipientType.SLACKV2
+
# Assert logs are correct
assert_log(ReportState.SUCCESS)
# this will send a warning
@@ -1403,6 +1423,94 @@ def test_slack_chart_report_schedule_v2(
)
assert statsd_mock.call_args_list[1] ==
call("reports.slack.send.ok", 1)
+ cleanup_report_schedule(report_schedule)
+
+
+@patch("superset.commands.report.execute.get_channels_with_search")
+@patch("superset.reports.notifications.slack.should_use_v2_api",
return_value=True)
+@patch("superset.reports.notifications.slackv2.get_slack_client")
+@patch("superset.utils.screenshots.ChartScreenshot.get_screenshot")
+def test_slack_chart_report_schedule_fails_to_converts_to_v2(
+ screenshot_mock,
+ slack_client_mock,
+ slack_should_use_v2_api_mock,
+ get_channels_with_search_mock,
+):
+ """
+ ExecuteReport Command: Test converting a Slack report to v2 fails.
+ """
+ # setup screenshot mock
+ screenshot_mock.return_value = SCREENSHOT_FILE
+ channel_id = "slack_channel_id"
+ chart = db.session.query(Slice).first()
+ report_schedule = create_report_notification(
+ slack_channel="#slack_channel,my_member_ID", chart=chart
+ )
+ get_channels_with_search_mock.return_value = [
+ {
+ "id": channel_id,
+ "name": "slack_channel",
+ "is_member": True,
+ "is_private": False,
+ },
+ ]
+
+ with pytest.raises(ReportScheduleSystemErrorsException):
+ AsyncExecuteReportScheduleCommand(
+ TEST_ID, report_schedule.id, datetime.utcnow()
+ ).run()
+
+ # Assert failuer with proper log
+ expected_message = (
+ "Failed to update slack recipients to v2: "
+ "Could not find the following channels: my_member_ID"
+ )
+ assert_log(ReportState.ERROR, error_message=expected_message)
+
+ # Assert that previous configuration was kept for manual correction
+ assert report_schedule.recipients[0].recipient_config_json == json.dumps(
+ {"target": "#slack_channel,my_member_ID"}
+ )
+ assert report_schedule.recipients[0].type == ReportRecipientType.SLACK
+
+ cleanup_report_schedule(report_schedule)
+
+
[email protected]("create_report_slack_chartv2")
+@patch("superset.reports.notifications.slack.should_use_v2_api",
return_value=True)
+@patch("superset.reports.notifications.slackv2.get_slack_client")
+@patch("superset.utils.screenshots.ChartScreenshot.get_screenshot")
+def test_slack_chart_report_schedule_v2(
+ screenshot_mock,
+ slack_client_mock,
+ slack_should_use_v2_api_mock,
+ create_report_slack_chartv2,
+):
+ """
+ ExecuteReport Command: Test chart slack report schedule using Slack v2.
+ """
+ # setup screenshot mock
+ screenshot_mock.return_value = SCREENSHOT_FILE
+
+ with freeze_time("2020-01-01T00:00:00Z"):
+ with patch.object(current_app.config["STATS_LOGGER"], "gauge") as
statsd_mock:
+ AsyncExecuteReportScheduleCommand(
+ TEST_ID, create_report_slack_chartv2.id, datetime.utcnow()
+ ).run()
+
+ assert (
+
slack_client_mock.return_value.files_upload_v2.call_args[1]["channel"]
+ == "slack_channel_id"
+ )
+ assert (
+
slack_client_mock.return_value.files_upload_v2.call_args[1]["file"]
+ == SCREENSHOT_FILE
+ )
+
+ # Assert logs are correct
+ assert_log(ReportState.SUCCESS)
+ assert statsd_mock.call_args_list[0] ==
call("reports.slack.send.ok", 1)
+
@pytest.mark.usefixtures(
"load_birth_names_dashboard_with_slices", "create_report_slack_chart"
diff --git a/tests/integration_tests/reports/utils.py
b/tests/integration_tests/reports/utils.py
index 3ab5c46c4e..e11ecc653b 100644
--- a/tests/integration_tests/reports/utils.py
+++ b/tests/integration_tests/reports/utils.py
@@ -119,6 +119,7 @@ def create_report_notification(
owners: Optional[list[User]] = None,
ccTarget: Optional[str] = None, # noqa: N803
bccTarget: Optional[str] = None, # noqa: N803
+ use_slack_v2: bool = False,
) -> ReportSchedule:
if not owners:
owners = [
@@ -130,8 +131,11 @@ def create_report_notification(
]
if slack_channel:
+ type = (
+ ReportRecipientType.SLACKV2 if use_slack_v2 else
ReportRecipientType.SLACK
+ )
recipient = ReportRecipients(
- type=ReportRecipientType.SLACK,
+ type=type,
recipient_config_json=json.dumps(
{
"target": slack_channel,
diff --git a/tests/integration_tests/utils_tests.py
b/tests/integration_tests/utils_tests.py
index ca49d65fc0..8a09a23f9f 100644
--- a/tests/integration_tests/utils_tests.py
+++ b/tests/integration_tests/utils_tests.py
@@ -52,7 +52,7 @@ from superset.utils.core import (
GenericDataType,
get_form_data_token,
as_list,
- get_email_address_list,
+ recipients_string_to_list,
get_stacktrace,
merge_extra_filters,
merge_extra_form_data,
@@ -809,12 +809,12 @@ class TestUtils(SupersetTestCase):
assert expected_filename in path
assert os.path.exists(path)
- def test_get_email_address_list(self):
- assert get_email_address_list("a@a") == ["a@a"]
- assert get_email_address_list(" a@a ") == ["a@a"]
- assert get_email_address_list("a@a\n") == ["a@a"]
- assert get_email_address_list(",a@a;") == ["a@a"]
- assert get_email_address_list(",a@a; b@b c@c a-c@c; d@d, f@f") == [
+ def test_recipients_string_to_list(self):
+ assert recipients_string_to_list("a@a") == ["a@a"]
+ assert recipients_string_to_list(" a@a ") == ["a@a"]
+ assert recipients_string_to_list("a@a\n") == ["a@a"]
+ assert recipients_string_to_list(",a@a;") == ["a@a"]
+ assert recipients_string_to_list(",a@a; b@b c@c a-c@c; d@d, f@f") == [
"a@a",
"b@b",
"c@c",
diff --git a/tests/unit_tests/commands/report/execute_test.py
b/tests/unit_tests/commands/report/execute_test.py
index 7c04f2f8ab..0f24577729 100644
--- a/tests/unit_tests/commands/report/execute_test.py
+++ b/tests/unit_tests/commands/report/execute_test.py
@@ -24,9 +24,11 @@ import pytest
from pytest_mock import MockerFixture
from superset.app import SupersetApp
+from superset.commands.exceptions import UpdateFailedError
from superset.commands.report.execute import BaseReportState
from superset.dashboards.permalink.types import DashboardPermalinkState
from superset.reports.models import (
+ ReportRecipients,
ReportRecipientType,
ReportSchedule,
ReportScheduleType,
@@ -480,3 +482,78 @@ def test_screenshot_width_calculation(
f"Test {test_id}: Expected width {expected_width}, "
f"but got {kwargs['window_size'][0]}"
)
+
+
+def test_update_recipient_to_slack_v2(mocker: MockerFixture):
+ """
+ Test converting a Slack recipient to Slack v2 format.
+ """
+ mocker.patch(
+ "superset.commands.report.execute.get_channels_with_search",
+ return_value=[
+ {
+ "id": "abc124f",
+ "name": "channel-1",
+ "is_member": True,
+ "is_private": False,
+ },
+ {
+ "id": "blah_!channel_2",
+ "name": "Channel_2",
+ "is_member": True,
+ "is_private": False,
+ },
+ ],
+ )
+ mock_report_schedule = ReportSchedule(
+ recipients=[
+ ReportRecipients(
+ type=ReportRecipientType.SLACK,
+ recipient_config_json=json.dumps({"target": "Channel-1,
Channel_2"}),
+ ),
+ ],
+ )
+
+ mock_cmmd: BaseReportState = BaseReportState(
+ mock_report_schedule, "January 1, 2021", "execution_id_example"
+ )
+ mock_cmmd.update_report_schedule_slack_v2()
+
+ assert (
+ mock_cmmd._report_schedule.recipients[0].recipient_config_json
+ == '{"target": "abc124f,blah_!channel_2"}'
+ )
+ assert mock_cmmd._report_schedule.recipients[0].type ==
ReportRecipientType.SLACKV2
+
+
+def test_update_recipient_to_slack_v2_missing_channels(mocker: MockerFixture):
+ """
+ Test converting a Slack recipient to Slack v2 format raises an error
+ in case it can't find all channels.
+ """
+ mocker.patch(
+ "superset.commands.report.execute.get_channels_with_search",
+ return_value=[
+ {
+ "id": "blah_!channel_2",
+ "name": "Channel 2",
+ "is_member": True,
+ "is_private": False,
+ },
+ ],
+ )
+ mock_report_schedule = ReportSchedule(
+ name="Test Report",
+ recipients=[
+ ReportRecipients(
+ type=ReportRecipientType.SLACK,
+ recipient_config_json=json.dumps({"target": "Channel 1,
Channel 2"}),
+ ),
+ ],
+ )
+
+ mock_cmmd: BaseReportState = BaseReportState(
+ mock_report_schedule, "January 1, 2021", "execution_id_example"
+ )
+ with pytest.raises(UpdateFailedError):
+ mock_cmmd.update_report_schedule_slack_v2()
diff --git a/tests/unit_tests/utils/slack_test.py
b/tests/unit_tests/utils/slack_test.py
index 42c3d3d4ce..ed7a82c220 100644
--- a/tests/unit_tests/utils/slack_test.py
+++ b/tests/unit_tests/utils/slack_test.py
@@ -153,7 +153,12 @@ The server responded with: missing scope: channels:read"""
def test_filter_channels_by_specified_types(self, mocker):
mock_data = {
"channels": [
- {"name": "general", "id": "C12345", "type": "public"},
+ {
+ "id": "C12345",
+ "name": "general",
+ "is_member": False,
+ "is_private": False,
+ },
],
"response_metadata": {"next_cursor": None},
}
@@ -164,7 +169,14 @@ The server responded with: missing scope: channels:read"""
mocker.patch("superset.utils.slack.get_slack_client",
return_value=mock_client)
result = get_channels_with_search(types=["public"])
- assert result == [{"name": "general", "id": "C12345", "type":
"public"}]
+ assert result == [
+ {
+ "id": "C12345",
+ "name": "general",
+ "is_member": False,
+ "is_private": False,
+ }
+ ]
def test_handle_pagination_multiple_pages(self, mocker):
mock_data_page1 = {
