ref(slack): Refactor Link Identity View

src/sentry/integrations/slack/views/link_identity.py

@@ -1,14 +1,20 @@
+import logging
+
 from django.core.signing import BadSignature, SignatureExpired
-from django.http import HttpResponse
+from django.db import IntegrityError
+from django.http import Http404, HttpRequest, HttpResponse
+from django.http.response import HttpResponseBase
 from django.utils.decorators import method_decorator
-from rest_framework.request import Request
 
+from sentry.integrations.slack.views import render_error_page
+from sentry.integrations.slack.views.types import IdentityParams
 from sentry.integrations.types import ExternalProviderEnum, ExternalProviders
 from sentry.integrations.utils import get_identity_or_404
 from sentry.models.identity import Identity
 from sentry.notifications.notificationcontroller import NotificationController
 from sentry.notifications.notifications.integration_nudge import IntegrationNudgeNotification
 from sentry.services.hybrid_cloud.integration.model import RpcIntegration
+from sentry.utils import metrics
 from sentry.utils.signing import unsign
 from sentry.web.frontend.base import BaseView, control_silo_view
 from sentry.web.helpers import render_to_response
@@ -17,6 +23,8 @@
 from . import build_linking_url as base_build_linking_url
 from . import never_cache
 
+_logger = logging.getLogger(__name__)
+
 SUCCESS_LINKED_MESSAGE = (
     "Your Slack identity has been linked to your Sentry account. You're good to go!"
 )
@@ -40,47 +48,102 @@ class SlackLinkIdentityView(BaseView):
     Django view for linking user to slack account. Creates an entry on Identity table.
     """
 
+    _METRICS_SUCCESS_KEY = "sentry.integrations.slack.link_identity_view.success"
+    _METRICS_FAILURE_KEY = "sentry.integrations.slack.link_identity_view.failure"
+
     @method_decorator(never_cache)
-    def handle(self, request: Request, signed_params: str) -> HttpResponse:
+    def dispatch(self, request: HttpRequest, *args, **kwargs) -> HttpResponseBase:
         try:
+            signed_params = kwargs.pop("signed_params")
             params = unsign(signed_params)
-        except (SignatureExpired, BadSignature):
+        except (SignatureExpired, BadSignature) as e:
+            _logger.warning("dispatch.signature_error", exc_info=e)
+            metrics.incr(self._METRICS_FAILURE_KEY, tags={"error": str(e)}, sample_rate=1.0)
             return render_to_response(
                 "sentry/integrations/slack/expired-link.html",
                 request=request,
             )
+        except KeyError as e:
+            _logger.warning("dispatch.key_error", exc_info=e)
+            metrics.incr(self._METRICS_FAILURE_KEY, tags={"error": str(e)}, sample_rate=1.0)
+            return render_error_page(
+                request,
+                status=400,
+                body_text="HTTP 400: Missing required 'signed_params' parameter",
+            )
+
+        try:
+            organization, integration, idp = get_identity_or_404(
+                ExternalProviders.SLACK,
+                request.user,
+                integration_id=params["integration_id"],
+            )
+        except Http404:
+            _logger.exception(
+                "get_identity_error", extra={"integration_id": params["integration_id"]}
+            )
+            metrics.incr(self._METRICS_FAILURE_KEY + ".get_identity", sample_rate=1.0)
+            raise
+
+        _logger.info("get_identity_success", extra={"integration_id": params["integration_id"]})
+        metrics.incr(self._METRICS_SUCCESS_KEY + ".get_identity", sample_rate=1.0)
+        params.update({"organization": organization, "integration": integration, "idp": idp})
+        return super().dispatch(request, params=params)
+
+    def get(self, request: HttpRequest, *args, **kwargs) -> HttpResponse:
+        params = kwargs["params"]
+        organization, integration = params["organization"], params["integration"]
 
-        organization, integration, idp = get_identity_or_404(
-            ExternalProviders.SLACK,
-            request.user,
-            integration_id=params["integration_id"],
+        return render_to_response(
+            "sentry/auth-link-identity.html",
+            request=request,
+            context={"organization": organization, "provider": integration.get_provider()},
         )
 
-        if request.method != "POST":
-            return render_to_response(
-                "sentry/auth-link-identity.html",
-                request=request,
-                context={"organization": organization, "provider": integration.get_provider()},
+    def post(self, request: HttpRequest, *args, **kwargs) -> HttpResponse:
+        params_dict = kwargs["params"]
+        params = IdentityParams(
+            organization=params_dict["organization"],
+            integration=params_dict["integration"],
+            idp=params_dict["idp"],
+            slack_id=params_dict["slack_id"],
+            channel_id=params_dict["channel_id"],
+        )
+
+        try:
+            Identity.objects.link_identity(
+                user=request.user, idp=params.idp, external_id=params.slack_id
+            )
+        except IntegrityError:
+            _logger.exception("slack.link.integrity_error")
+            metrics.incr(
+                self._METRICS_FAILURE_KEY + ".post.identity.integrity_error",
+                sample_rate=1.0,
             )
+            raise Http404
 
-        Identity.objects.link_identity(user=request.user, idp=idp, external_id=params["slack_id"])
+        # TODO: We should use use the dataclass to send the slack response
+        send_slack_response(
+            params.integration, SUCCESS_LINKED_MESSAGE, params.__dict__, command="link"
+        )
 
-        send_slack_response(integration, SUCCESS_LINKED_MESSAGE, params, command="link")
-        has_slack_settings = None
         controller = NotificationController(
             recipients=[request.user],
-            organization_id=organization.id,
+            organization_id=params.organization.id,
             provider=ExternalProviderEnum.SLACK,
         )
         has_slack_settings = controller.user_has_any_provider_settings(ExternalProviderEnum.SLACK)
 
         if not has_slack_settings:
-            IntegrationNudgeNotification(organization, request.user, ExternalProviders.SLACK).send()
+            IntegrationNudgeNotification(
+                params.organization, request.user, ExternalProviders.SLACK
+            ).send()
+
+        _logger.info("link_identity_success", extra={"slack_id": params.slack_id})
+        metrics.incr(self._METRICS_SUCCESS_KEY + ".post.link_identity", sample_rate=1.0)
 
-        # TODO(epurkhiser): We could do some fancy slack querying here to
-        #  render a nice linking page with info about the user their linking.
         return render_to_response(
             "sentry/integrations/slack/linked.html",
             request=request,
-            context={"channel_id": params["channel_id"], "team_id": integration.external_id},
+            context={"channel_id": params.channel_id, "team_id": params.integration.external_id},
         )

src/sentry/integrations/slack/views/types.py

@@ -0,0 +1,23 @@
+from dataclasses import dataclass
+
+from sentry.models.identity import IdentityProvider
+from sentry.models.integrations.integration import Integration
+from sentry.services.hybrid_cloud.organization import RpcOrganization
+
+
+@dataclass
+class IdentityParams:
+    organization: RpcOrganization
+    integration: Integration
+    idp: IdentityProvider
+    slack_id: str
+    channel_id: str
+    response_url: str | None = None
+
+    def __init__(self, organization, integration, idp, slack_id, channel_id, response_url=None):
+        self.organization = organization
+        self.integration = integration
+        self.idp = idp
+        self.slack_id = slack_id
+        self.channel_id = channel_id
+        self.response_url = response_url

src/sentry/integrations/slack/views/unlink_identity.py

@@ -8,7 +8,8 @@
 
 from sentry.integrations.slack.utils.notifications import send_slack_response
 from sentry.integrations.slack.views import build_linking_url as base_build_linking_url
-from sentry.integrations.slack.views import never_cache
+from sentry.integrations.slack.views import never_cache, render_error_page
+from sentry.integrations.slack.views.types import IdentityParams
 from sentry.integrations.types import ExternalProviders
 from sentry.integrations.utils import get_identity_or_404
 from sentry.models.identity import Identity
@@ -54,6 +55,14 @@ def dispatch(self, request: HttpRequest, signed_params: str) -> HttpResponseBase
                 "sentry/integrations/slack/expired-link.html",
                 request=request,
             )
+        except KeyError as e:
+            _logger.warning("dispatch.key_error", exc_info=e)
+            metrics.incr(self._METRICS_FAILURE_KEY, tags={"error": str(e)}, sample_rate=1.0)
+            return render_error_page(
+                request,
+                status=400,
+                body_text="HTTP 400: Missing required 'signed_params' parameter",
+            )
 
         try:
             organization, integration, idp = get_identity_or_404(
@@ -84,26 +93,35 @@ def get(self, request: HttpRequest, *args, **kwargs) -> HttpResponse:
         )
 
     def post(self, request: HttpRequest, *args, **kwargs) -> HttpResponse:
-        params = kwargs["params"]
-        integration, idp = params["integration"], params["idp"]
+        params_dict = kwargs["params"]
+        params = IdentityParams(
+            organization=params_dict["organization"],
+            integration=params_dict["integration"],
+            idp=params_dict["idp"],
+            slack_id=params_dict["slack_id"],
+            channel_id=params_dict["channel_id"],
+        )
 
         try:
-            Identity.objects.filter(idp_id=idp.id, external_id=params["slack_id"]).delete()
+            Identity.objects.filter(idp_id=params.idp, external_id=params.slack_id).delete()
         except IntegrityError:
-            _logger.exception("slack.unlink.integrity-error")
+            _logger.exception("slack.unlink.integrity_error")
             metrics.incr(
                 self._METRICS_FAILURE_KEY + ".post.identity.integrity_error",
                 sample_rate=1.0,
             )
             raise Http404
 
-        send_slack_response(integration, SUCCESS_UNLINKED_MESSAGE, params, command="unlink")
+        # TODO: We should use use the dataclass to send the slack response
+        send_slack_response(
+            params.integration, SUCCESS_UNLINKED_MESSAGE, params.__dict__, command="unlink"
+        )
 
-        _logger.info("unlink_identity_success", extra={"slack_id": params["slack_id"]})
+        _logger.info("unlink_identity_success", extra={"slack_id": params.slack_id})
         metrics.incr(self._METRICS_SUCCESS_KEY + ".post.unlink_identity", sample_rate=1.0)
 
         return render_to_response(
             "sentry/integrations/slack/unlinked.html",
             request=request,
-            context={"channel_id": params["channel_id"], "team_id": integration.external_id},
+            context={"channel_id": params.channel_id, "team_id": params.integration.external_id},
         )