Browse Source

types(py): Slack Endpoints (#29912)

Marcos Gaeta 3 years ago
parent
commit
f8cdf9dbc7

+ 1 - 11
mypy.ini

@@ -33,17 +33,7 @@ files = src/sentry/api/bases/external_actor.py,
         src/sentry/grouping/strategies/template.py,
         src/sentry/grouping/strategies/utils.py,
         src/sentry/integrations/base.py,
-        src/sentry/integrations/slack/analytics.py,
-        src/sentry/integrations/slack/client.py,
-        src/sentry/integrations/slack/message_builder/,
-        src/sentry/integrations/slack/notifications.py,
-        src/sentry/integrations/slack/notify_action.py,
-        src/sentry/integrations/slack/requests/,
-        src/sentry/integrations/slack/tasks.py,
-        src/sentry/integrations/slack/unfurl/,
-        src/sentry/integrations/slack/urls.py,
-        src/sentry/integrations/slack/utils/,
-        src/sentry/integrations/slack/views/,
+        src/sentry/integrations/slack/,
         src/sentry/integrations/vsts/,
         src/sentry/killswitches.py,
         src/sentry/lang/native/appconnect.py,

+ 2 - 0
src/sentry/api/helpers/group_index/__init__.py

@@ -22,9 +22,11 @@ __all__ = (
     "BULK_MUTATION_LIMIT",
     "SEARCH_MAX_HITS",
     "delete_group_list",
+    "update_groups",
 )
 
 from .delete import *  # NOQA
 from .delete import delete_group_list
 from .index import *  # NOQA
 from .update import *  # NOQA
+from .update import update_groups

+ 8 - 1
src/sentry/integrations/msteams/unlink_identity.py

@@ -1,3 +1,4 @@
+from django.core.signing import BadSignature, SignatureExpired
 from django.urls import reverse
 from django.views.decorators.cache import never_cache
 
@@ -30,7 +31,13 @@ class MsTeamsUnlinkIdentityView(BaseView):
     @transaction_start("MsTeamsUnlinkIdentityView")
     @never_cache
     def handle(self, request, signed_params):
-        params = unsign(signed_params)
+        try:
+            params = unsign(signed_params)
+        except (SignatureExpired, BadSignature):
+            return render_to_response(
+                "sentry/integrations/msteams/expired-link.html",
+                request=request,
+            )
 
         if request.method != "POST":
             return render_to_response(

+ 41 - 38
src/sentry/integrations/slack/endpoints/action.py

@@ -2,8 +2,8 @@ from __future__ import annotations
 
 from typing import Any, Mapping, MutableMapping
 
+import requests as requests_
 from django.urls import reverse
-from requests import post
 from rest_framework.request import Request
 from rest_framework.response import Response
 
@@ -14,6 +14,8 @@ from sentry.api.helpers.group_index import update_groups
 from sentry.auth.access import from_member
 from sentry.exceptions import UnableToAcceptMemberInvitationException
 from sentry.integrations.slack.client import SlackClient
+from sentry.integrations.slack.message_builder import SlackBody
+from sentry.integrations.slack.message_builder.issues import SlackIssuesMessageBuilder
 from sentry.integrations.slack.requests.action import SlackActionRequest
 from sentry.integrations.slack.requests.base import SlackRequestError
 from sentry.integrations.slack.views.link_identity import build_linking_url
@@ -33,8 +35,6 @@ from sentry.utils import json
 from sentry.utils.http import absolute_uri
 from sentry.web.decorators import transaction_start
 
-from ..message_builder import SlackBody
-from ..message_builder.issues import SlackIssuesMessageBuilder
 from ..utils import logger
 
 LINK_IDENTITY_MESSAGE = (
@@ -127,6 +127,9 @@ class SlackActionEndpoint(Endpoint):  # type: ignore
     def on_assign(
         self, request: Request, identity: Identity, group: Group, action: MessageAction
     ) -> None:
+        if not (action.selected_options and len(action.selected_options)):
+            # Short-circuit if action is invalid
+            return
         assignee = action.selected_options[0]["value"]
         if assignee == "none":
             assignee = None
@@ -144,7 +147,10 @@ class SlackActionEndpoint(Endpoint):  # type: ignore
         integration: Integration,
     ) -> None:
         status_data = (action.value or "").split(":", 1)
-        status = {"status": status_data[0]}
+        if not len(status_data):
+            return
+
+        status: MutableMapping[str, Any] = {"status": status_data[0]}
 
         resolve_type = status_data[-1]
 
@@ -214,14 +220,16 @@ class SlackActionEndpoint(Endpoint):  # type: ignore
         return attachment
 
     def is_message(self, data: Mapping[str, Any]) -> bool:
-        # XXX(epurkhsier): Used in coordination with construct_reply. Bot
-        # posted messages will not have the type at all.
-        return data.get("original_message", {}).get("type") == "message"
+        """
+        XXX(epurkhiser): Used in coordination with construct_reply.
+         Bot posted messages will not have the type at all.
+        """
+        # Explicitly typing to satisfy mypy.
+        is_message_: bool = data.get("original_message", {}).get("type") == "message"
+        return is_message_
 
     @transaction_start("SlackActionEndpoint")
     def post(self, request: Request) -> Response:
-        logging_data: MutableMapping[str, str] = {}
-
         try:
             slack_request = SlackActionRequest(request)
             slack_request.validate()
@@ -242,44 +250,34 @@ class SlackActionEndpoint(Endpoint):  # type: ignore
         action_list_raw = data.get("actions", [])
         action_list = [MessageAction(**action_data) for action_data in action_list_raw]
 
-        channel_id = slack_request.channel_id
-        user_id = slack_request.user_id
-        integration = slack_request.integration
-        response_url = data.get("response_url")
+        organizations = slack_request.integration.organizations.all()
 
         if action_option in ["link", "ignore"]:
             analytics.record(
                 "integrations.slack.chart_unfurl_action",
-                organization_id=integration.organizations.all()[0].id,
+                organization_id=organizations[0].id,
                 action=action_option,
             )
             payload = {"delete_original": "true"}
             try:
-                post(response_url, json=payload)
+                requests_.post(slack_request.response_url, json=payload)
             except ApiError as e:
                 logger.error("slack.action.response-error", extra={"error": str(e)})
                 return self.respond(status=403)
 
             return self.respond()
 
-        logging_data["channel_id"] = channel_id
-        logging_data["slack_user_id"] = user_id
-        logging_data["response_url"] = response_url
-        logging_data["integration_id"] = integration.id
-
         # Determine the issue group action is being taken on
         group_id = slack_request.callback_data["issue"]
-        logging_data["group_id"] = group_id
+        logging_data = {**slack_request.logging_data, "group_id": group_id}
 
         try:
             group = Group.objects.select_related("project__organization").get(
-                project__in=Project.objects.filter(
-                    organization__in=integration.organizations.all()
-                ),
+                project__in=Project.objects.filter(organization__in=organizations),
                 id=group_id,
             )
         except Group.DoesNotExist:
-            logger.info("slack.action.invalid-issue", extra=logging_data)
+            logger.info("slack.action.invalid-issue", extra={**logging_data})
             return self.respond(status=403)
 
         logging_data["organization_id"] = group.organization.id
@@ -291,7 +289,12 @@ class SlackActionEndpoint(Endpoint):  # type: ignore
             return self.respond(status=403)
 
         if not identity:
-            associate_url = build_linking_url(integration, user_id, channel_id, response_url)
+            associate_url = build_linking_url(
+                integration=slack_request.integration,
+                slack_id=slack_request.user_id,
+                channel_id=slack_request.channel_id,
+                response_url=slack_request.response_url,
+            )
             return self.respond_ephemeral(LINK_IDENTITY_MESSAGE.format(associate_url=associate_url))
 
         # Handle status dialog submission
@@ -303,7 +306,7 @@ class SlackActionEndpoint(Endpoint):  # type: ignore
             )
 
             try:
-                self.on_status(request, identity, group, action, data, integration)
+                self.on_status(request, identity, group, action, data, slack_request.integration)
             except client.ApiError as error:
                 return self.api_error(slack_request, group, identity, error, "status_dialog")
 
@@ -334,20 +337,20 @@ class SlackActionEndpoint(Endpoint):  # type: ignore
         defer_attachment_update = False
 
         # Handle interaction actions
-        action_type = None
-        try:
-            for action in action_list:
-                action_type = action.name
-
+        for action in action_list:
+            action_type = action.name
+            try:
                 if action_type == "status":
-                    self.on_status(request, identity, group, action, data, integration)
+                    self.on_status(
+                        request, identity, group, action, data, slack_request.integration
+                    )
                 elif action_type == "assign":
                     self.on_assign(request, identity, group, action)
                 elif action_type == "resolve_dialog":
-                    self.open_resolve_dialog(data, group, integration)
+                    self.open_resolve_dialog(data, group, slack_request.integration)
                     defer_attachment_update = True
-        except client.ApiError as error:
-            return self.api_error(slack_request, group, identity, error, action_type)
+            except client.ApiError as error:
+                return self.api_error(slack_request, group, identity, error, action_type)
 
         if defer_attachment_update:
             return self.respond()
@@ -362,9 +365,9 @@ class SlackActionEndpoint(Endpoint):  # type: ignore
 
         return self.respond(body)
 
-    def handle_member_approval(self, slack_request: SlackActionRequest):
+    def handle_member_approval(self, slack_request: SlackActionRequest) -> Response:
         try:
-            # get_identity can return Noone
+            # get_identity can return nobody
             identity = slack_request.get_identity()
         except IdentityProvider.DoesNotExist:
             identity = None

+ 18 - 14
src/sentry/integrations/slack/endpoints/base.py

@@ -1,11 +1,13 @@
+from __future__ import annotations
+
 import abc
-from typing import Any, Sequence, Tuple
 
+from rest_framework import status
 from rest_framework.response import Response
 
 from sentry.api.base import Endpoint
 from sentry.integrations.slack.message_builder.help import SlackHelpMessageBuilder
-from sentry.integrations.slack.requests.base import SlackRequest
+from sentry.integrations.slack.requests.base import SlackDMRequest, SlackRequestError
 from sentry.integrations.slack.views.link_identity import build_linking_url
 from sentry.integrations.slack.views.unlink_identity import build_unlinking_url
 
@@ -19,12 +21,12 @@ ALREADY_LINKED_MESSAGE = "You are already linked as `{username}`."
 
 
 class SlackDMEndpoint(Endpoint, abc.ABC):  # type: ignore
-    def post_dispatcher(self, request: SlackRequest) -> Any:
+    def post_dispatcher(self, request: SlackDMRequest) -> Response:
         """
         All Slack commands are handled by this endpoint. This block just
         validates the request and dispatches it to the right handler.
         """
-        command, args = self.get_command_and_args(request)
+        command, args = request.get_command_and_args()
 
         if command in ["help", ""]:
             return self.respond(SlackHelpMessageBuilder().build())
@@ -48,18 +50,18 @@ class SlackDMEndpoint(Endpoint, abc.ABC):  # type: ignore
         unknown_command = request_data.get("text", "").lower()
         return self.respond(SlackHelpMessageBuilder(unknown_command).build())
 
-    def get_command_and_args(self, request: SlackRequest) -> Tuple[str, Sequence[str]]:
-        raise NotImplementedError
-
-    def reply(self, slack_request: SlackRequest, message: str) -> Response:
+    def reply(self, slack_request: SlackDMRequest, message: str) -> Response:
         raise NotImplementedError
 
-    def link_user(self, slack_request: SlackRequest) -> Any:
+    def link_user(self, slack_request: SlackDMRequest) -> Response:
         if slack_request.has_identity:
             return self.reply(
                 slack_request, ALREADY_LINKED_MESSAGE.format(username=slack_request.identity_str)
             )
 
+        if not (slack_request.integration and slack_request.user_id and slack_request.channel_id):
+            raise SlackRequestError(status=status.HTTP_400_BAD_REQUEST)
+
         associate_url = build_linking_url(
             integration=slack_request.integration,
             slack_id=slack_request.user_id,
@@ -68,21 +70,23 @@ class SlackDMEndpoint(Endpoint, abc.ABC):  # type: ignore
         )
         return self.reply(slack_request, LINK_USER_MESSAGE.format(associate_url=associate_url))
 
-    def unlink_user(self, slack_request: SlackRequest) -> Any:
+    def unlink_user(self, slack_request: SlackDMRequest) -> Response:
         if not slack_request.has_identity:
             return self.reply(slack_request, NOT_LINKED_MESSAGE)
 
-        integration = slack_request.integration
+        if not (slack_request.integration and slack_request.user_id and slack_request.channel_id):
+            raise SlackRequestError(status=status.HTTP_400_BAD_REQUEST)
+
         associate_url = build_unlinking_url(
-            integration_id=integration.id,
+            integration_id=slack_request.integration.id,
             slack_id=slack_request.user_id,
             channel_id=slack_request.channel_id,
             response_url=slack_request.response_url,
         )
         return self.reply(slack_request, UNLINK_USER_MESSAGE.format(associate_url=associate_url))
 
-    def link_team(self, slack_request: SlackRequest) -> Any:
+    def link_team(self, slack_request: SlackDMRequest) -> Response:
         raise NotImplementedError
 
-    def unlink_team(self, slack_request: SlackRequest) -> Any:
+    def unlink_team(self, slack_request: SlackDMRequest) -> Response:
         raise NotImplementedError

+ 12 - 20
src/sentry/integrations/slack/endpoints/command.py

@@ -1,4 +1,4 @@
-from typing import Optional, Sequence, Tuple
+from __future__ import annotations
 
 from django.http import HttpResponse
 from rest_framework import status
@@ -6,7 +6,7 @@ from rest_framework.request import Request
 from rest_framework.response import Response
 
 from sentry.integrations.slack.message_builder.disconnected import SlackDisconnectedMessageBuilder
-from sentry.integrations.slack.requests.base import SlackRequest, SlackRequestError
+from sentry.integrations.slack.requests.base import SlackDMRequest, SlackRequestError
 from sentry.integrations.slack.requests.command import SlackCommandRequest
 from sentry.integrations.slack.utils.auth import is_valid_role
 from sentry.integrations.slack.views.link_team import build_team_linking_url
@@ -38,20 +38,20 @@ DIRECT_MESSAGE_CHANNEL_NAME = "directmessage"
 INSUFFICIENT_ROLE_MESSAGE = "You must be a Sentry admin, manager, or owner to link or unlink teams."
 
 
-def is_team_linked_to_channel(
-    organization: Organization, slack_request: SlackCommandRequest
-) -> bool:
+def is_team_linked_to_channel(organization: Organization, slack_request: SlackDMRequest) -> bool:
     """Check if a Slack channel already has a team linked to it"""
-    return ExternalActor.objects.filter(
+    # Explicitly typing to satisfy mypy.
+    is_linked: bool = ExternalActor.objects.filter(
         organization=organization,
         integration=slack_request.integration,
         provider=ExternalProviders.SLACK.value,
         external_name=slack_request.channel_name,
         external_id=slack_request.channel_id,
     ).exists()
+    return is_linked
 
 
-def get_identity(slack_request: SlackRequest) -> Optional[Identity]:
+def get_identity(slack_request: SlackDMRequest) -> Identity | None:
     try:
         identity = slack_request.get_identity()
     except IdentityProvider.DoesNotExist:
@@ -59,19 +59,11 @@ def get_identity(slack_request: SlackRequest) -> Optional[Identity]:
     return identity
 
 
-class SlackCommandsEndpoint(SlackDMEndpoint):  # type: ignore
+class SlackCommandsEndpoint(SlackDMEndpoint):
     authentication_classes = ()
     permission_classes = ()
 
-    def get_command_and_args(self, payload: SlackRequest) -> Tuple[str, Sequence[str]]:
-        payload = payload.data
-        text = payload.get("text", "").lower().split()
-        if not text:
-            return "", []
-
-        return text[0], text[1:]
-
-    def reply(self, slack_request: SlackRequest, message: str) -> Response:
+    def reply(self, slack_request: SlackDMRequest, message: str) -> Response:
         return self.respond(
             {
                 "response_type": "ephemeral",
@@ -80,7 +72,7 @@ class SlackCommandsEndpoint(SlackDMEndpoint):  # type: ignore
             }
         )
 
-    def link_team(self, slack_request: SlackCommandRequest) -> Response:
+    def link_team(self, slack_request: SlackDMRequest) -> Response:
         if slack_request.channel_name == DIRECT_MESSAGE_CHANNEL_NAME:
             return self.reply(slack_request, LINK_FROM_CHANNEL_MESSAGE)
 
@@ -113,7 +105,7 @@ class SlackCommandsEndpoint(SlackDMEndpoint):  # type: ignore
         )
         return self.reply(slack_request, LINK_TEAM_MESSAGE.format(associate_url=associate_url))
 
-    def unlink_team(self, slack_request: SlackCommandRequest) -> Response:
+    def unlink_team(self, slack_request: SlackDMRequest) -> Response:
         if slack_request.channel_name == DIRECT_MESSAGE_CHANNEL_NAME:
             return self.reply(slack_request, LINK_FROM_CHANNEL_MESSAGE)
 
@@ -126,7 +118,7 @@ class SlackCommandsEndpoint(SlackDMEndpoint):  # type: ignore
             integration, identity.user
         )
 
-        found: Optional[OrganizationMember] = None
+        found: OrganizationMember | None = None
         for organization_membership in organization_memberships:
             if is_team_linked_to_channel(organization_membership.organization, slack_request):
                 found = organization_membership

+ 62 - 98
src/sentry/integrations/slack/endpoints/event.py

@@ -1,14 +1,16 @@
+from __future__ import annotations
+
 from collections import defaultdict
-from typing import Any, Dict, List, Mapping, Optional, Sequence, Tuple
+from typing import Any, Mapping, MutableMapping
 
 from rest_framework.request import Request
 from rest_framework.response import Response
 
 from sentry import analytics, features
 from sentry.integrations.slack.client import SlackClient
-from sentry.integrations.slack.message_builder.base.block import BlockSlackMessageBuilder
-from sentry.integrations.slack.message_builder.event import SlackEventMessageBuilder
-from sentry.integrations.slack.requests.base import SlackRequest, SlackRequestError
+from sentry.integrations.slack.message_builder.help import SlackHelpMessageBuilder
+from sentry.integrations.slack.message_builder.prompt import SlackPromptLinkMessageBuilder
+from sentry.integrations.slack.requests.base import SlackDMRequest, SlackRequestError
 from sentry.integrations.slack.requests.event import COMMANDS, SlackEventRequest
 from sentry.integrations.slack.unfurl import LinkType, UnfurlableUrl, link_handlers, match_link
 from sentry.integrations.slack.views.link_identity import build_linking_url
@@ -23,7 +25,7 @@ from .base import SlackDMEndpoint
 from .command import LINK_FROM_CHANNEL_MESSAGE
 
 
-class SlackEventEndpoint(SlackDMEndpoint):  # type: ignore
+class SlackEventEndpoint(SlackDMEndpoint):
     """
     XXX(dcramer): a lot of this is copied from sentry-plugins right now, and will need refactoring
     """
@@ -31,26 +33,10 @@ class SlackEventEndpoint(SlackDMEndpoint):  # type: ignore
     authentication_classes = ()
     permission_classes = ()
 
-    def is_bot(self, data: Mapping[str, Any]) -> bool:
-        """
-        If it's a message posted by our bot, we don't want to respond since that
-        will cause an infinite loop of messages.
-        """
-        return bool(data.get("bot_id"))
-
-    def get_command_and_args(self, slack_request: SlackRequest) -> Tuple[str, Sequence[str]]:
-        data = slack_request.data.get("event")
-        command = data["text"].lower().split()
-        return command[0], command[1:]
-
-    def reply(self, slack_request: SlackRequest, message: str) -> Response:
+    def reply(self, slack_request: SlackDMRequest, message: str) -> Response:
+        headers = {"Authorization": f"Bearer {self._get_access_token(slack_request.integration)}"}
+        payload = {"channel": slack_request.channel_name, "text": message}
         client = SlackClient()
-        access_token = self._get_access_token(slack_request.integration)
-        headers = {"Authorization": f"Bearer {access_token}"}
-        data = slack_request.data.get("event")
-        channel = data["channel"]
-        payload = {"channel": channel, "text": message}
-
         try:
             client.post("/chat.postMessage", headers=headers, data=payload, json=True)
         except ApiError as e:
@@ -58,49 +44,40 @@ class SlackEventEndpoint(SlackDMEndpoint):  # type: ignore
 
         return self.respond()
 
-    def link_team(self, slack_request: SlackRequest) -> Any:
+    def link_team(self, slack_request: SlackDMRequest) -> Response:
         return self.reply(slack_request, LINK_FROM_CHANNEL_MESSAGE)
 
-    def unlink_team(self, slack_request: SlackRequest) -> Any:
+    def unlink_team(self, slack_request: SlackDMRequest) -> Response:
         return self.reply(slack_request, LINK_FROM_CHANNEL_MESSAGE)
 
-    def _get_access_token(self, integration: Integration) -> Any:
-        # the classic bot tokens must use the user auth token for URL unfurling
-        # we stored the user_access_token there
-        # but for workspace apps and new slack bot tokens, we can just use access_token
-        return integration.metadata.get("user_access_token") or integration.metadata["access_token"]
+    def _get_access_token(self, integration: Integration) -> str:
+        """
+        The classic bot tokens must use the user auth token for URL unfurling we
+        stored the user_access_token there but for workspace apps and new Slack
+        bot tokens, we can just use access_token.
+        """
+        access_token: str = (
+            integration.metadata.get("user_access_token") or integration.metadata["access_token"]
+        )
+        return access_token
 
     def on_url_verification(self, request: Request, data: Mapping[str, str]) -> Response:
         return self.respond({"challenge": data["challenge"]})
 
-    def prompt_link(
-        self,
-        data: Mapping[str, Any],
-        slack_request: SlackRequest,
-        integration: Integration,
-    ):
+    def prompt_link(self, slack_request: SlackDMRequest) -> None:
         associate_url = build_linking_url(
-            integration=integration,
+            integration=slack_request.integration,
             slack_id=slack_request.user_id,
             channel_id=slack_request.channel_id,
             response_url=slack_request.response_url,
         )
 
-        builder = BlockSlackMessageBuilder()
-
-        blocks = [
-            builder.get_markdown_block(
-                "Link your Slack identity to Sentry to unfurl Discover charts."
-            ),
-            builder.get_action_block([("Link", associate_url, "link"), ("Cancel", None, "ignore")]),
-        ]
-
         payload = {
-            "token": self._get_access_token(integration),
-            "channel": data["channel"],
-            "user": data["user"],
+            "token": self._get_access_token(slack_request.integration),
+            "channel": slack_request.channel_name,
+            "user": slack_request.user_id,
             "text": "Link your Slack identity to Sentry to unfurl Discover charts.",
-            "blocks": json.dumps(blocks),
+            **SlackPromptLinkMessageBuilder(associate_url).as_payload(),
         }
 
         client = SlackClient()
@@ -109,16 +86,16 @@ class SlackEventEndpoint(SlackDMEndpoint):  # type: ignore
         except ApiError as e:
             logger.error("slack.event.unfurl-error", extra={"error": str(e)}, exc_info=True)
 
-    def on_message(
-        self, request: Request, integration: Integration, token: str, data: Mapping[str, Any]
-    ) -> Response:
-        channel = data["channel"]
+    def on_message(self, request: Request, slack_request: SlackDMRequest) -> Response:
         command = request.data.get("event", {}).get("text", "").lower()
-        if self.is_bot(data) or not command:
+        if slack_request.is_bot() or not command:
             return self.respond()
-        access_token = self._get_access_token(integration)
-        headers = {"Authorization": f"Bearer {access_token}"}
-        payload = {"channel": channel, **SlackEventMessageBuilder(integration, command).build()}
+
+        headers = {"Authorization": f"Bearer {self._get_access_token(slack_request.integration)}"}
+        payload = {
+            "channel": slack_request.channel_name,
+            **SlackHelpMessageBuilder(command).as_payload(),
+        }
         client = SlackClient()
         try:
             client.post("/chat.postMessage", headers=headers, data=payload, json=True)
@@ -127,19 +104,15 @@ class SlackEventEndpoint(SlackDMEndpoint):  # type: ignore
 
         return self.respond()
 
-    def on_link_shared(
-        self,
-        request: Request,
-        slack_request: SlackRequest,
-    ) -> Optional[Response]:
-        matches: Dict[LinkType, List[UnfurlableUrl]] = defaultdict(list)
+    def on_link_shared(self, request: Request, slack_request: SlackDMRequest) -> bool:
+        """Returns true on success"""
+        matches: MutableMapping[LinkType, list[UnfurlableUrl]] = defaultdict(list)
         links_seen = set()
 
-        integration = slack_request.integration
-        data = slack_request.data.get("event")
+        data = slack_request.data.get("event", {})
 
         # An unfurl may have multiple links to unfurl
-        for item in data["links"]:
+        for item in data.get("links", []):
             try:
                 url = item["url"]
                 slack_shared_link = parse_link(url)
@@ -157,22 +130,19 @@ class SlackEventEndpoint(SlackDMEndpoint):  # type: ignore
             if link_type is None or args is None:
                 continue
 
+            organization = slack_request.integration.organizations.all()[0]
             if (
                 link_type == LinkType.DISCOVER
                 and not slack_request.has_identity
-                and features.has(
-                    "organizations:chart-unfurls",
-                    slack_request.integration.organizations.all()[0],
-                    actor=request.user,
-                )
+                and features.has("organizations:chart-unfurls", organization, actor=request.user)
             ):
                 analytics.record(
                     "integrations.slack.chart_unfurl",
-                    organization_id=integration.organizations.all()[0].id,
+                    organization_id=organization.id,
                     unfurls_count=0,
                 )
-                self.prompt_link(data, slack_request, integration)
-                return self.respond()
+                self.prompt_link(slack_request)
+                return True
 
             # Don't unfurl the same thing multiple times
             seen_marker = hash(json.dumps((link_type, args), sort_keys=True))
@@ -183,19 +153,24 @@ class SlackEventEndpoint(SlackDMEndpoint):  # type: ignore
             matches[link_type].append(UnfurlableUrl(url=url, args=args))
 
         if not matches:
-            return None
+            return False
 
         # Unfurl each link type
-        results: Dict[str, Any] = {}
+        results: MutableMapping[str, Any] = {}
         for link_type, unfurl_data in matches.items():
             results.update(
-                link_handlers[link_type].fn(request, integration, unfurl_data, slack_request.user)
+                link_handlers[link_type].fn(
+                    request,
+                    slack_request.integration,
+                    unfurl_data,
+                    slack_request.user,
+                )
             )
 
         if not results:
-            return None
+            return False
 
-        access_token = self._get_access_token(integration)
+        access_token = self._get_access_token(slack_request.integration)
 
         payload = {
             "token": access_token,
@@ -210,7 +185,7 @@ class SlackEventEndpoint(SlackDMEndpoint):  # type: ignore
         except ApiError as e:
             logger.error("slack.event.unfurl-error", extra={"error": str(e)}, exc_info=True)
 
-        return self.respond()
+        return True
 
     # TODO(dcramer): implement app_uninstalled and tokens_revoked
     @transaction_start("SlackEventEndpoint")
@@ -225,30 +200,19 @@ class SlackEventEndpoint(SlackDMEndpoint):  # type: ignore
             return self.on_url_verification(request, slack_request.data)
 
         if slack_request.type == "link_shared":
-            resp = self.on_link_shared(
-                request,
-                slack_request,
-            )
-
-            if resp:
-                return resp
+            if self.on_link_shared(request, slack_request):
+                return self.respond()
 
         if slack_request.type == "message":
-            data = slack_request.data.get("event")
-            if self.is_bot(data):
+            if slack_request.is_bot():
                 return self.respond()
 
-            command = data.get("text")
+            command, _ = slack_request.get_command_and_args()
             if command in COMMANDS:
                 resp = super().post_dispatcher(slack_request)
 
             else:
-                resp = self.on_message(
-                    request,
-                    slack_request.integration,
-                    slack_request.data.get("token"),
-                    slack_request.data.get("event"),
-                )
+                resp = self.on_message(request, slack_request)
 
             if resp:
                 return resp

+ 5 - 2
src/sentry/integrations/slack/message_builder/base/block.py

@@ -1,5 +1,5 @@
 from abc import ABC
-from typing import Any, Dict, List, MutableMapping, Optional, Sequence, Tuple, TypedDict
+from typing import Any, Dict, List, Mapping, MutableMapping, Optional, Sequence, Tuple, TypedDict
 
 from sentry.integrations.slack.message_builder import SlackBlock, SlackBody
 from sentry.integrations.slack.message_builder.base.base import SlackMessageBuilder
@@ -58,4 +58,7 @@ class BlockSlackMessageBuilder(SlackMessageBuilder, ABC):
 
     @staticmethod
     def _build_blocks(*args: SlackBlock) -> SlackBody:
-        return {"blocks": args}
+        return {"blocks": list(args)}
+
+    def as_payload(self) -> Mapping[str, Any]:
+        return self.build()  # type: ignore

+ 0 - 5
src/sentry/integrations/slack/message_builder/discover.py

@@ -12,8 +12,3 @@ class SlackDiscoverMessageBuilder(BlockSlackMessageBuilder):
         return self._build_blocks(
             self.get_image_block(self.chart_url, title=self.title, alt="Discover Chart")
         )
-
-
-def build_discover_attachment(title: str, chart_url: str) -> SlackBody:
-    """@deprecated"""
-    return SlackDiscoverMessageBuilder(title, chart_url).build()

+ 6 - 14
src/sentry/integrations/slack/message_builder/event.py

@@ -1,27 +1,19 @@
-from typing import Iterable, List, Optional
+from typing import Sequence
 
-from sentry.integrations.slack.message_builder import SlackBlock, SlackBody
+from sentry.integrations.slack.message_builder import SlackBlock
 from sentry.integrations.slack.message_builder.help import SlackHelpMessageBuilder
-from sentry.models import Integration
 
 from ..utils import logger
 from .help import UNKNOWN_COMMAND_MESSAGE
 
 
 class SlackEventMessageBuilder(SlackHelpMessageBuilder):
-    def __init__(self, integration: Integration, command: Optional[str] = None) -> None:
-        super().__init__()
-        self.integration = integration
-        self.command = command
-
-    def get_header_block(self) -> Iterable[SlackBlock]:
-        blocks: List[SlackBlock] = []
-        if self.command != "help":
+    def get_header_blocks(self) -> Sequence[SlackBlock]:
+        blocks = []
+        if self.command and self.command != "help":
             logger.info("slack.event.unknown-command", extra={"command": self.command})
             blocks.append(
                 self.get_markdown_block(UNKNOWN_COMMAND_MESSAGE.format(command=self.command))
             )
-        return blocks
 
-    def build(self) -> SlackBody:
-        return super().build()
+        return blocks

Some files were not shown because too many files changed in this diff