You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by va...@apache.org on 2025/02/20 22:04:51 UTC
(superset) branch fix/slack-v2-channels-migration created (now 206a73e4e1)
This is an automated email from the ASF dual-hosted git repository.
vavila pushed a change to branch fix/slack-v2-channels-migration
in repository https://gitbox.apache.org/repos/asf/superset.git
at 206a73e4e1 fix(Slack): Fix Slack recipients migration to V2
This branch includes the following new commits:
new 206a73e4e1 fix(Slack): Fix Slack recipients migration to V2
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(superset) 01/01: fix(Slack): Fix Slack recipients migration to V2
Posted by va...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
vavila pushed a commit to branch fix/slack-v2-channels-migration
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 206a73e4e18bbfd035d213d1e91ce5d9a4d84297
Author: Vitor Avila <vi...@preset.io>
AuthorDate: Thu Feb 20 19:04:33 2025 -0300
fix(Slack): Fix Slack recipients migration to V2
---
superset/commands/report/execute.py | 20 +++---
superset/reports/schemas.py | 16 ++++-
superset/utils/slack.py | 12 ++--
tests/unit_tests/commands/report/execute_test.py | 77 ++++++++++++++++++++++++
4 files changed, 113 insertions(+), 12 deletions(-)
diff --git a/superset/commands/report/execute.py b/superset/commands/report/execute.py
index 0c57d55bb5..839c1496f8 100644
--- a/superset/commands/report/execute.py
+++ b/superset/commands/report/execute.py
@@ -139,15 +139,21 @@ class BaseReportState:
slack_recipients = json.loads(recipient.recipient_config_json)
# we need to ensure that existing reports can also fetch
# ids from private channels
+ current_target_count = slack_recipients["target"].split(",")
+ channels = get_channels_with_search(
+ slack_recipients["target"],
+ types=[
+ SlackChannelTypes.PRIVATE,
+ SlackChannelTypes.PUBLIC,
+ ],
+ exact_match=True,
+ )
+ if len(current_target_count) != len(channels):
+ raise UpdateFailedError("Not all channels could be found")
+ 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:
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/slack.py b/superset/utils/slack.py
index 468429fb60..b788d96630 100644
--- a/superset/utils/slack.py
+++ b/superset/utils/slack.py
@@ -25,6 +25,7 @@ from slack_sdk.errors import SlackApiError
from superset import feature_flag_manager
from superset.exceptions import SupersetException
+from superset.reports.schemas import SlackChannelSchema
from superset.utils.backports import StrEnum
logger = logging.getLogger(__name__)
@@ -51,7 +52,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
@@ -60,7 +61,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
@@ -69,7 +71,9 @@ 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
@@ -77,7 +81,7 @@ def get_channels_with_search(
# The search string can be multiple channels separated by commas
if search_string:
search_array = [
- search.lower()
+ search.strip().lower()
for search in (search_string.split(",") if search_string else [])
]
diff --git a/tests/unit_tests/commands/report/execute_test.py b/tests/unit_tests/commands/report/execute_test.py
index 7c04f2f8ab..69bcc03074 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()