fix: use channel id with new slack api for file uploads

superset/reports/notifications/slack.py

@@ -17,12 +17,14 @@
 import logging
 from collections.abc import Sequence
 from io import IOBase
-from typing import Union
+from typing import List, Union
 
 import backoff
 import pandas as pd
+from deprecation import deprecated
 from flask import g
 from flask_babel import gettext as __
+from slack_sdk import WebClient
 from slack_sdk.errors import (
     BotUserAccessError,
     SlackApiError,
@@ -60,16 +62,25 @@ 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"
+        :raises SlackApiError: If the API call fails
         """
         recipient_str = json.loads(self._recipient.recipient_config_json)["target"]
 
-        return ",".join(get_email_address_list(recipient_str))
+        channel_recipients: List[str] = get_email_address_list(recipient_str)
+
+        conversations_list_response = client.conversations_list(
+            types="public_channel,private_channel"
+        )
+
+        return [
+            c["id"]
+            for c in conversations_list_response["channels"]
+            if c["name"] in channel_recipients
+        ]
 
     def _message_template(self, table: str = "") -> str:
         return __(
@@ -115,15 +126,19 @@ def _get_body(self) -> str:
 
         # Flatten columns/index so they show up nicely in the table
         df.columns = [
-            " ".join(str(name) for name in column).strip()
-            if isinstance(column, tuple)
-            else column
+            (
+                " ".join(str(name) for name in column).strip()
+                if isinstance(column, tuple)
+                else column
+            )
             for column in df.columns
         ]
         df.index = [
-            " ".join(str(name) for name in index).strip()
-            if isinstance(index, tuple)
-            else index
+            (
+                " ".join(str(name) for name in index).strip()
+                if isinstance(index, tuple)
+                else index
+            )
             for index in df.index
         ]
 
@@ -162,29 +177,40 @@ def _get_body(self) -> str:
 
     def _get_inline_files(
         self,
-    ) -> tuple[Union[str, None], Sequence[Union[str, IOBase, bytes]]]:
+    ) -> Sequence[Union[str, IOBase, bytes]]:
         if self._content.csv:
-            return ("csv", [self._content.csv])
+            return [self._content.csv]
         if self._content.screenshots:
-            return ("png", self._content.screenshots)
+            return self._content.screenshots
         if self._content.pdf:
-            return ("pdf", [self._content.pdf])
-        return (None, [])
+            return [self._content.pdf]
+        return []
 
-    @backoff.on_exception(backoff.expo, SlackApiError, factor=10, base=2, max_tries=5)
-    @statsd_gauge("reports.slack.send")
-    def send(self) -> None:
-        file_type, files = self._get_inline_files()
-        title = self._content.name
-        channel = self._get_channel()
-        body = self._get_body()
-        global_logs_context = getattr(g, "logs_context", {}) or {}
-        try:
-            client = get_slack_client()
-            # files_upload returns SlackResponse as we run it in sync mode.
-            if files:
+    @deprecated(deprecated_in="4.1")
+    def _deprecated_upload_files(
+        self, client: WebClient, title: str, body: str
+    ) -> None:
+        """
+        Deprecated method to upload files to slack
+        Should only be used if the new method fails
+        To be removed in the next major release
+        """
+        file_type, files = (None, [])
+        if self._content.csv:
+            file_type, files = ("csv", [self._content.csv])
+        if self._content.screenshots:
+            file_type, files = ("png", self._content.screenshots)
+        if self._content.pdf:
+            file_type, files = ("pdf", [self._content.pdf])
+
+        recipient_str = json.loads(self._recipient.recipient_config_json)["target"]
+
+        recipients = get_email_address_list(recipient_str)
+
+        for channel in recipients:
+            if len(files) > 0:
                 for file in files:
-                    client.files_upload_v2(
+                    client.files_upload(
                         channels=channel,
                         file=file,
                         initial_comment=body,
@@ -193,6 +219,46 @@ def send(self) -> None:
                     )
             else:
                 client.chat_postMessage(channel=channel, text=body)
+
+    @backoff.on_exception(backoff.expo, SlackApiError, factor=10, base=2, max_tries=5)
+    @statsd_gauge("reports.slack.send")
+    def send(self) -> None:
+        global_logs_context = getattr(g, "logs_context", {}) or {}
+        try:
+            client = get_slack_client()
+            title = self._content.name
+            body = self._get_body()
+
+            try:
+                channels = self._get_channels(client)
+            except SlackApiError:
+                logger.warning(
+                    "Slack scope missing. Using deprecated API to get channels. Please update your Slack app to use the new API.",
+                    extra={
+                        "execution_id": global_logs_context.get("execution_id"),
+                    },
+                )
+                self._deprecated_upload_files(client, title, body)
+                return
+
+            if channels == []:
+                raise NotificationParamException("No valid channel found")
+
+            files = self._get_inline_files()
+
+            # files_upload returns SlackResponse as we run it in sync mode.
+            for channel in channels:
+                if len(files) > 0:
+                    for file in files:
+                        client.files_upload_v2(
+                            channel=channel,
+                            file=file,
+                            initial_comment=body,
+                            title=title,
+                        )
+                else:
+                    client.chat_postMessage(channel=channel, text=body)
+
             logger.info(
                 "Report sent to slack",
                 extra={

superset/utils/json.py

@@ -24,7 +24,7 @@
 import pandas as pd
 import simplejson
 from flask_babel.speaklater import LazyString
-from simplejson import JSONDecodeError  # noqa: F401 # pylint: disable=unused-import
+from simplejson import JSONDecodeError
 
 from superset.utils.dates import datetime_to_epoch, EPOCH
 

tests/integration_tests/fixtures/world_bank_dashboard.py

@@ -28,6 +28,7 @@
 from superset.models.core import Database
 from superset.models.dashboard import Dashboard
 from superset.models.slice import Slice
+from superset.reports.models import ReportSchedule
 from superset.utils import json
 from superset.utils.core import get_example_default_schema
 from superset.utils.database import get_example_database
@@ -81,6 +82,7 @@ def load_world_bank_dashboard_with_slices_module_scope(load_world_bank_data):
     with app.app_context():
         dash_id_to_delete, slices_ids_to_delete = create_dashboard_for_loaded_data()
         yield
+        _cleanup_reports(dash_id_to_delete, slices_ids_to_delete)
         _cleanup(dash_id_to_delete, slices_ids_to_delete)
 
 
@@ -143,6 +145,21 @@ def _cleanup(dash_id: int, slices_ids: list[int]) -> None:
     db.session.commit()
 
 
+def _cleanup_reports(dash_id: int, slices_ids: list[int]) -> None:
+    reports_with_dash = (
+        db.session.query(ReportSchedule).filter_by(dashboard_id=dash_id).all()
+    )
+    reports_with_slices = (
+        db.session.query(ReportSchedule)
+        .filter(ReportSchedule.chart_id.in_(slices_ids))
+        .all()
+    )
+
+    for report in reports_with_dash + reports_with_slices:
+        db.session.delete(report)
+    db.session.commit()
+
+
 def _get_dataframe(database: Database) -> DataFrame:
     data = _get_world_bank_data()
     df = pd.DataFrame.from_dict(data)