Browse Source

ref(slack): Add types to Slack endpoints (#26885)

Marcos Gaeta 3 years ago
parent
commit
b790b36a50

+ 3 - 1
mypy.ini

@@ -21,6 +21,7 @@ files = src/sentry/api/bases/external_actor.py,
         src/sentry/integrations/slack/message_builder/**/*.py,
         src/sentry/integrations/slack/requests/*.py,
         src/sentry/integrations/slack/util/*.py,
+        src/sentry/integrations/slack/views/*.py,
         src/sentry/killswitches.py,
         src/sentry/lang/native/appconnect.py,
         src/sentry/notifications/**/*.py,
@@ -33,7 +34,8 @@ files = src/sentry/api/bases/external_actor.py,
         src/sentry/utils/dates.py,
         src/sentry/utils/kvstore,
         src/sentry/utils/snql.py,
-        src/sentry/unmerge.py
+        src/sentry/unmerge.py,
+        src/sentry/web/decorators.py
 
 ; Enable all options used with --strict
 warn_unused_configs=True

+ 36 - 12
src/sentry/integrations/slack/endpoints/action.py

@@ -1,5 +1,10 @@
+from typing import Any, Dict, Mapping
+
+from rest_framework.request import Request
+from rest_framework.response import Response
+
 from sentry import analytics
-from sentry.api import client
+from sentry.api import ApiClient, client
 from sentry.api.base import Endpoint
 from sentry.integrations.slack.client import SlackClient
 from sentry.integrations.slack.message_builder.issues import build_group_attachment
@@ -7,11 +12,12 @@ 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
 from sentry.integrations.slack.views.unlink_identity import build_unlinking_url
-from sentry.models import ApiKey, Group, Identity, IdentityProvider, Project
+from sentry.models import ApiKey, Group, Identity, IdentityProvider, Integration, Project
 from sentry.shared_integrations.exceptions import ApiError
 from sentry.utils import json
 from sentry.web.decorators import transaction_start
 
+from ..message_builder import SlackBody
 from ..utils import logger
 
 LINK_IDENTITY_MESSAGE = "Looks like you haven't linked your Sentry account with your Slack identity yet! <{associate_url}|Link your identity now> to perform actions in Sentry through Slack."
@@ -34,11 +40,17 @@ RESOLVE_SELECTOR = {
 }
 
 
-class SlackActionEndpoint(Endpoint):
+class SlackActionEndpoint(Endpoint):  # type: ignore
     authentication_classes = ()
     permission_classes = ()
 
-    def api_error(self, error, action_type, logging_data, error_text):
+    def api_error(
+        self,
+        error: ApiClient.ApiError,
+        action_type: str,
+        logging_data: Dict[str, str],
+        error_text: str,
+    ) -> Response:
         logging_data = logging_data.copy()
         logging_data["response"] = str(error.body)
         logging_data["action_type"] = action_type
@@ -49,7 +61,9 @@ class SlackActionEndpoint(Endpoint):
             {"response_type": "ephemeral", "replace_original": False, "text": error_text}
         )
 
-    def on_assign(self, request, identity, group, action):
+    def on_assign(
+        self, request: Request, identity: Identity, group: Group, action: Mapping[str, Any]
+    ) -> None:
         assignee = action["selected_options"][0]["value"]
 
         if assignee == "none":
@@ -58,7 +72,15 @@ class SlackActionEndpoint(Endpoint):
         self.update_group(group, identity, {"assignedTo": assignee})
         analytics.record("integrations.slack.assign", actor_id=identity.user_id)
 
-    def on_status(self, request, identity, group, action, data, integration):
+    def on_status(
+        self,
+        request: Request,
+        identity: Identity,
+        group: Group,
+        action: Mapping[str, Any],
+        data: Mapping[str, Any],
+        integration: Integration,
+    ) -> None:
         status = action["value"]
 
         status_data = status.split(":", 1)
@@ -80,7 +102,7 @@ class SlackActionEndpoint(Endpoint):
             actor_id=identity.user_id,
         )
 
-    def update_group(self, group, identity, data):
+    def update_group(self, group: Group, identity: Identity, data: Mapping[str, str]) -> Response:
         event_write_key = ApiKey(
             organization=group.project.organization, scope_list=["event:write"]
         )
@@ -93,7 +115,9 @@ class SlackActionEndpoint(Endpoint):
             auth=event_write_key,
         )
 
-    def open_resolve_dialog(self, data, group, integration):
+    def open_resolve_dialog(
+        self, data: Mapping[str, Any], group: Group, integration: Integration
+    ) -> None:
         # XXX(epurkhiser): In order to update the original message we have to
         # keep track of the response_url in the callback_id. Definitely hacky,
         # but seems like there's no other solutions [1]:
@@ -126,7 +150,7 @@ class SlackActionEndpoint(Endpoint):
         except ApiError as e:
             logger.error("slack.action.response-error", extra={"error": str(e)})
 
-    def construct_reply(self, attachment, is_message=False):
+    def construct_reply(self, attachment: SlackBody, is_message: bool = False) -> SlackBody:
         # XXX(epurkhiser): Slack is inconsistent about it's expected responses
         # for interactive action requests.
         #
@@ -142,14 +166,14 @@ class SlackActionEndpoint(Endpoint):
 
         return attachment
 
-    def is_message(self, data):
+    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"
 
     @transaction_start("SlackActionEndpoint")
-    def post(self, request):
-        logging_data = {}
+    def post(self, request: Request) -> Response:
+        logging_data: Dict[str, str] = {}
 
         try:
             slack_request = SlackActionRequest(request)

+ 3 - 2
src/sentry/integrations/slack/endpoints/command.py

@@ -3,6 +3,7 @@ from typing import Mapping, Sequence, Tuple
 
 from django.http import Http404, HttpResponse
 from rest_framework.request import Request
+from rest_framework.response import Response
 
 from sentry.api.base import Endpoint
 from sentry.integrations.slack.message_builder.help import SlackHelpMessageBuilder
@@ -24,11 +25,11 @@ def get_command_and_args(payload: Mapping[str, str]) -> Tuple[str, Sequence[str]
     return text[0], text[1:]
 
 
-class SlackCommandsEndpoint(Endpoint):
+class SlackCommandsEndpoint(Endpoint):  # type: ignore
     authentication_classes = ()
     permission_classes = ()
 
-    def send_ephemeral_notification(self, message):
+    def send_ephemeral_notification(self, message: str) -> Response:
         return self.respond(
             {
                 "response_type": "ephemeral",

+ 17 - 9
src/sentry/integrations/slack/endpoints/event.py

@@ -1,5 +1,8 @@
 from collections import defaultdict
-from typing import Any, Dict, List
+from typing import Any, Dict, List, Mapping, Optional
+
+from rest_framework.request import Request
+from rest_framework.response import Response
 
 from sentry.api.base import Endpoint
 from sentry.integrations.slack.client import SlackClient
@@ -7,6 +10,7 @@ from sentry.integrations.slack.message_builder.event import SlackEventMessageBui
 from sentry.integrations.slack.requests.base import SlackRequestError
 from sentry.integrations.slack.requests.event import SlackEventRequest
 from sentry.integrations.slack.unfurl import LinkType, UnfurlableUrl, link_handlers, match_link
+from sentry.models import Integration
 from sentry.shared_integrations.exceptions import ApiError
 from sentry.utils import json
 from sentry.web.decorators import transaction_start
@@ -16,20 +20,22 @@ from ..utils import logger, parse_link
 
 # XXX(dcramer): a lot of this is copied from sentry-plugins right now, and will
 # need refactored
-class SlackEventEndpoint(Endpoint):
+class SlackEventEndpoint(Endpoint):  # type: ignore
     authentication_classes = ()
     permission_classes = ()
 
-    def _get_access_token(self, integration):
+    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
         return integration.metadata.get("user_access_token") or integration.metadata["access_token"]
 
-    def on_url_verification(self, request, data):
+    def on_url_verification(self, request: Request, data: Mapping[str, str]) -> Response:
         return self.respond({"challenge": data["challenge"]})
 
-    def on_message(self, request, integration, token, data):
+    def on_message(
+        self, request: Request, integration: Integration, token: str, data: Mapping[str, Any]
+    ) -> Response:
         channel = data["channel"]
         # if it's a message posted by our bot, we don't want to respond since
         # that will cause an infinite loop of messages
@@ -46,7 +52,9 @@ class SlackEventEndpoint(Endpoint):
 
         return self.respond()
 
-    def on_link_shared(self, request, integration, token, data):
+    def on_link_shared(
+        self, request: Request, integration: Integration, token: str, data: Mapping[str, Any]
+    ) -> Optional[Response]:
         matches: Dict[LinkType, List[UnfurlableUrl]] = defaultdict(list)
         links_seen = set()
 
@@ -77,7 +85,7 @@ class SlackEventEndpoint(Endpoint):
             matches[link_type].append(UnfurlableUrl(url=item["url"], args=args))
 
         if not matches:
-            return
+            return None
 
         # Unfurl each link type
         results: Dict[str, Any] = {}
@@ -85,7 +93,7 @@ class SlackEventEndpoint(Endpoint):
             results.update(link_handlers[link_type].fn(request, integration, unfurl_data))
 
         if not results:
-            return
+            return None
 
         access_token = self._get_access_token(integration)
 
@@ -106,7 +114,7 @@ class SlackEventEndpoint(Endpoint):
 
     # TODO(dcramer): implement app_uninstalled and tokens_revoked
     @transaction_start("SlackEventEndpoint")
-    def post(self, request):
+    def post(self, request: Request) -> Response:
         try:
             slack_request = SlackEventRequest(request)
             slack_request.validate()

+ 1 - 1
src/sentry/integrations/slack/requests/base.py

@@ -35,7 +35,7 @@ class SlackRequest:
 
     def __init__(self, request: Request) -> None:
         self.request = request
-        self.integration: Optional[Any] = None
+        self.integration: Optional[Integration] = None
         self._data: MutableMapping[str, Any] = {}
         self._log_request()
 

+ 19 - 0
src/sentry/integrations/slack/views/__init__.py

@@ -0,0 +1,19 @@
+from django.urls import reverse
+from django.views.decorators.cache import never_cache as django_never_cache
+
+from sentry.utils.http import absolute_uri
+from sentry.utils.signing import sign
+from sentry.utils.types import Any
+from sentry.web.decorators import EndpointFunc
+
+
+def never_cache(view_func: EndpointFunc) -> EndpointFunc:
+    """TODO(mgaeta): Remove cast once Django has a typed version."""
+    result: EndpointFunc = django_never_cache(view_func)
+    return result
+
+
+def build_linking_url(endpoint: str, **kwargs: Any) -> str:
+    """TODO(mgaeta): Remove cast once sentry/utils/http.py is typed."""
+    url: str = absolute_uri(reverse(endpoint, kwargs={"signed_params": sign(**kwargs)}))
+    return url

+ 17 - 13
src/sentry/integrations/slack/views/link_identity.py

@@ -1,22 +1,30 @@
 from django.db import IntegrityError
-from django.urls import reverse
 from django.utils import timezone
-from django.views.decorators.cache import never_cache
+from rest_framework.request import Request
+from rest_framework.response import Response
 
-from sentry.models import Identity, IdentityStatus
+from sentry.models import Identity, IdentityStatus, Integration, Organization
 from sentry.shared_integrations.exceptions import ApiError
-from sentry.utils.http import absolute_uri
-from sentry.utils.signing import sign, unsign
+from sentry.utils.signing import unsign
 from sentry.web.decorators import transaction_start
 from sentry.web.frontend.base import BaseView
 from sentry.web.helpers import render_to_response
 
 from ..client import SlackClient
 from ..utils import get_identity, logger
+from . import build_linking_url as base_build_linking_url
+from . import never_cache
 
 
-def build_linking_url(integration, organization, slack_id, channel_id, response_url):
-    signed_params = sign(
+def build_linking_url(
+    integration: Integration,
+    organization: Organization,
+    slack_id: str,
+    channel_id: str,
+    response_url: str,
+) -> str:
+    return base_build_linking_url(
+        "sentry-integration-slack-link-identity",
         integration_id=integration.id,
         organization_id=organization.id,
         slack_id=slack_id,
@@ -24,15 +32,11 @@ def build_linking_url(integration, organization, slack_id, channel_id, response_
         response_url=response_url,
     )
 
-    return absolute_uri(
-        reverse("sentry-integration-slack-link-identity", kwargs={"signed_params": signed_params})
-    )
-
 
-class SlackLinkIdentityView(BaseView):
+class SlackLinkIdentityView(BaseView):  # type: ignore
     @transaction_start("SlackLinkIdentityView")
     @never_cache
-    def handle(self, request, signed_params):
+    def handle(self, request: Request, signed_params: str) -> Response:
         params = unsign(signed_params)
 
         organization, integration, idp = get_identity(

+ 28 - 18
src/sentry/integrations/slack/views/link_team.py

@@ -1,6 +1,8 @@
+from typing import Any, Mapping, Sequence
+
 from django import forms
-from django.urls import reverse
-from django.views.decorators.cache import never_cache
+from rest_framework.request import Request
+from rest_framework.response import Response
 
 from sentry.models import (
     ExternalActor,
@@ -14,18 +16,22 @@ from sentry.models import (
 from sentry.notifications.types import NotificationSettingOptionValues, NotificationSettingTypes
 from sentry.shared_integrations.exceptions import ApiError
 from sentry.types.integrations import ExternalProviders
-from sentry.utils.http import absolute_uri
-from sentry.utils.signing import sign, unsign
+from sentry.utils.signing import unsign
 from sentry.web.decorators import transaction_start
 from sentry.web.frontend.base import BaseView
 from sentry.web.helpers import render_to_response
 
 from ..client import SlackClient
 from ..utils import logger
+from . import build_linking_url as base_build_linking_url
+from . import never_cache
 
 
-def build_linking_url(integration, slack_id, channel_id, channel_name, response_url):
-    signed_params = sign(
+def build_linking_url(
+    integration: Integration, slack_id: str, channel_id: str, channel_name: str, response_url: str
+) -> str:
+    return base_build_linking_url(
+        "sentry-integration-slack-link-team",
         integration_id=integration.id,
         slack_id=slack_id,
         channel_id=channel_id,
@@ -33,23 +39,19 @@ def build_linking_url(integration, slack_id, channel_id, channel_name, response_
         response_url=response_url,
     )
 
-    return absolute_uri(
-        reverse("sentry-integration-slack-link-team", kwargs={"signed_params": signed_params})
-    )
-
 
-class SelectTeamForm(forms.Form):
+class SelectTeamForm(forms.Form):  # type: ignore
     team = forms.ChoiceField(label="Team")
 
-    def __init__(self, teams, *args, **kwargs):
+    def __init__(self, teams: Sequence[Team], *args: Any, **kwargs: Any):
         super().__init__(*args, **kwargs)
 
         self.fields["team"].choices = [(team.id, team.slug) for team in teams]
         self.fields["team"].widget.choices = self.fields["team"].choices
 
 
-class SlackLinkTeamView(BaseView):
-    def get_identity(self, request, integration, slack_id):
+class SlackLinkTeamView(BaseView):  # type: ignore
+    def get_identity(self, request: Request, integration: Integration, slack_id: str) -> Response:
         try:
             idp = IdentityProvider.objects.get(type="slack", external_id=integration.external_id)
         except IdentityProvider.DoesNotExist:
@@ -69,14 +71,22 @@ class SlackLinkTeamView(BaseView):
             )
         return identity
 
-    def render_error_page(self, request, body_text):
+    def render_error_page(self, request: Request, body_text: str) -> Response:
         return render_to_response(
             "sentry/integrations/slack-link-team-error.html",
             request=request,
             context={"body_text": body_text},
         )
 
-    def send_slack_message(self, request, client, token, text, channel_id, integration):
+    def send_slack_message(
+        self,
+        request: Request,
+        client: SlackClient,
+        token: str,
+        text: Mapping[str, str],
+        channel_id: str,
+        integration: Integration,
+    ) -> Response:
         payload = {
             "token": token,
             "channel": channel_id,
@@ -103,7 +113,7 @@ class SlackLinkTeamView(BaseView):
 
     @transaction_start("SlackLinkTeamView")
     @never_cache
-    def handle(self, request, signed_params):
+    def handle(self, request: Request, signed_params: str) -> Response:
         params = unsign(signed_params)
         integration = Integration.objects.get(id=params["integration_id"])
         organization = integration.organizations.all()[0]
@@ -132,7 +142,7 @@ class SlackLinkTeamView(BaseView):
 
         team = Team.objects.get(id=team_id, organization=organization)
         if not team:
-            return self.render_error_page(body_text="HTTP 404: Team does not exist")
+            return self.render_error_page(request, body_text="HTTP 404: Team does not exist")
 
         INSUFFICIENT_ROLE_MESSAGE = {
             "heading": "Insufficient role",

+ 12 - 12
src/sentry/integrations/slack/views/unlink_identity.py

@@ -1,22 +1,26 @@
 from django.db import IntegrityError
 from django.http import Http404
-from django.urls import reverse
-from django.views.decorators.cache import never_cache
+from rest_framework.request import Request
+from rest_framework.response import Response
 
 from sentry.models import Identity
 from sentry.shared_integrations.exceptions import ApiError
-from sentry.utils.http import absolute_uri
-from sentry.utils.signing import sign, unsign
+from sentry.utils.signing import unsign
 from sentry.web.decorators import transaction_start
 from sentry.web.frontend.base import BaseView
 from sentry.web.helpers import render_to_response
 
 from ..client import SlackClient
 from ..utils import get_identity, logger
+from . import build_linking_url as base_build_linking_url
+from . import never_cache
 
 
-def build_unlinking_url(integration_id, organization_id, slack_id, channel_id, response_url):
-    signed_params = sign(
+def build_unlinking_url(
+    integration_id: str, organization_id: str, slack_id: str, channel_id: str, response_url: str
+) -> str:
+    return base_build_linking_url(
+        "sentry-integration-slack-unlink-identity",
         integration_id=integration_id,
         organization_id=organization_id,
         slack_id=slack_id,
@@ -24,15 +28,11 @@ def build_unlinking_url(integration_id, organization_id, slack_id, channel_id, r
         response_url=response_url,
     )
 
-    return absolute_uri(
-        reverse("sentry-integration-slack-unlink-identity", kwargs={"signed_params": signed_params})
-    )
-
 
-class SlackUnlinkIdentityView(BaseView):
+class SlackUnlinkIdentityView(BaseView):  # type: ignore
     @transaction_start("SlackUnlinkIdentityView")
     @never_cache
-    def handle(self, request, signed_params):
+    def handle(self, request: Request, signed_params: str) -> Response:
         params = unsign(signed_params)
 
         organization, integration, idp = get_identity(

+ 16 - 11
src/sentry/web/decorators.py

@@ -1,19 +1,24 @@
 from functools import wraps
+from typing import Any, Callable
 
 from django.contrib import messages
-from django.http import HttpResponseRedirect
+from django.http import HttpResponse, HttpResponseRedirect
 from django.urls import reverse
 from django.utils.translation import ugettext_lazy as _
+from rest_framework.request import Request
 from sentry_sdk import Hub
 
 from sentry.utils import auth
 
 ERR_BAD_SIGNATURE = _("The link you followed is invalid or expired.")
 
+# TODO(mgaeta): It's not currently possible to type a Callable's args with kwargs.
+EndpointFunc = Callable[..., HttpResponse]
 
-def login_required(func):
+
+def login_required(func: EndpointFunc) -> EndpointFunc:
     @wraps(func)
-    def wrapped(request, *args, **kwargs):
+    def wrapped(request: Request, *args: Any, **kwargs: Any) -> HttpResponse:
         if not request.user.is_authenticated:
             auth.initiate_login(request, next_url=request.get_full_path())
             if "organization_slug" in kwargs:
@@ -28,9 +33,9 @@ def login_required(func):
     return wrapped
 
 
-def signed_auth_required(func):
+def signed_auth_required(func: EndpointFunc) -> EndpointFunc:
     @wraps(func)
-    def wrapped(request, *args, **kwargs):
+    def wrapped(request: Request, *args: Any, **kwargs: Any) -> HttpResponse:
         if not request.user_from_signed_request:
             messages.add_message(request, messages.ERROR, ERR_BAD_SIGNATURE)
             return HttpResponseRedirect(auth.get_login_url())
@@ -39,10 +44,10 @@ def signed_auth_required(func):
     return wrapped
 
 
-def set_referrer_policy(policy):
-    def real_decorator(func):
+def set_referrer_policy(policy: str) -> Callable[[EndpointFunc], EndpointFunc]:
+    def real_decorator(func: EndpointFunc) -> EndpointFunc:
         @wraps(func)
-        def wrapped(request, *args, **kwargs):
+        def wrapped(request: Request, *args: Any, **kwargs: Any) -> HttpResponse:
             response = func(request, *args, **kwargs)
             response["Referrer-Policy"] = policy
             return response
@@ -52,10 +57,10 @@ def set_referrer_policy(policy):
     return real_decorator
 
 
-def transaction_start(endpoint):
-    def decorator(func):
+def transaction_start(endpoint: str) -> Callable[[EndpointFunc], EndpointFunc]:
+    def decorator(func: EndpointFunc) -> EndpointFunc:
         @wraps(func)
-        def wrapped(request, *args, **kwargs):
+        def wrapped(request: Request, *args: Any, **kwargs: Any) -> HttpResponse:
             with Hub.current.start_transaction(op="http.server", name=endpoint, sampled=True):
                 return func(request, *args, **kwargs)
 

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