♻️ ref(slack): fix slack webhook actions typing

pyproject.toml

@@ -276,7 +276,6 @@ module = [
     "sentry.integrations.slack.unfurl.discover",
     "sentry.integrations.slack.utils.channel",
     "sentry.integrations.slack.utils.users",
-    "sentry.integrations.slack.webhooks.action",
     "sentry.integrations.slack.webhooks.command",
     "sentry.integrations.slack.webhooks.event",
     "sentry.integrations.utils.commit_context",

src/sentry/api/helpers/group_index/update.py

@@ -170,7 +170,7 @@ def update_groups(
     projects: Sequence[Project],
     organization_id: int,
     search_fn: SearchFunction | None,
-    user: User | None = None,
+    user: RpcUser | User | None = None,
     data: Mapping[str, Any] | None = None,
 ) -> Response:
     # If `user` and `data` are passed as parameters then they should override

src/sentry/integrations/discord/webhooks/message_component.py

@@ -232,7 +232,7 @@ def update_group(self, data: Mapping[str, object]) -> None:
                 projects=[self.group.project],
                 organization_id=self.group.organization.id,
                 search_fn=None,
-                user=self.user,  # type: ignore[arg-type]
+                user=self.user,
                 data=data,
             )
 

src/sentry/integrations/slack/message_builder/issues.py

@@ -51,7 +51,7 @@
 from sentry.models.rule import Rule
 from sentry.models.team import Team
 from sentry.notifications.notifications.base import ProjectNotification
-from sentry.notifications.utils.actions import MessageAction
+from sentry.notifications.utils.actions import BlockKitMessageAction, MessageAction
 from sentry.notifications.utils.participants import (
     dedupe_suggested_assignees,
     get_suspect_commit_users,
@@ -135,7 +135,9 @@ def build_assigned_text(identity: RpcIdentity, assignee: str) -> str | None:
     return f"*Issue assigned to {assignee_text} by <@{identity.external_id}>*"
 
 
-def build_action_text(identity: RpcIdentity, action: MessageAction) -> str | None:
+def build_action_text(
+    identity: RpcIdentity, action: MessageAction | BlockKitMessageAction
+) -> str | None:
     if action.name == "assign":
         selected_options = action.selected_options or []
         if not len(selected_options):
@@ -356,7 +358,7 @@ def build_actions(
     group: Group,
     project: Project,
     text: str,
-    actions: Sequence[MessageAction] | None = None,
+    actions: Sequence[MessageAction | BlockKitMessageAction] | None = None,
     identity: RpcIdentity | None = None,
 ) -> tuple[Sequence[MessageAction], str, bool]:
     """Having actions means a button will be shown on the Slack message e.g. ignore, resolve, assign."""
@@ -421,7 +423,7 @@ def __init__(
         event: Event | GroupEvent | None = None,
         tags: set[str] | None = None,
         identity: RpcIdentity | None = None,
-        actions: Sequence[MessageAction] | None = None,
+        actions: Sequence[MessageAction | BlockKitMessageAction] | None = None,
         rules: list[Rule] | None = None,
         link_to_event: bool = False,
         issue_details: bool = False,

src/sentry/integrations/slack/webhooks/action.py

@@ -208,22 +208,40 @@ def api_error(
                 channel_id = data.get("channel_id")
                 response_url = data.get("orig_response_url")
 
-        if error.status_code == 403:
+        user_id = slack_request.user_id
+        channel = channel_id or slack_request.channel_id
+        resp_url = response_url or slack_request.response_url
+
+        if user_id is None or channel is None or resp_url is None:
+            text = DEFAULT_ERROR_MESSAGE
+        # keeping this separate from above since its a different condition
+        elif error.status_code != 403:
+            text = DEFAULT_ERROR_MESSAGE
+        else:
             text = UNLINK_IDENTITY_MESSAGE.format(
                 associate_url=build_unlinking_url(
                     slack_request.integration.id,
-                    slack_request.user_id,
-                    channel_id or slack_request.channel_id,
-                    response_url or slack_request.response_url,
+                    slack_id=user_id,
+                    channel_id=channel,
+                    response_url=resp_url,
                 ),
                 user_email=user.email,
                 org_name=group.organization.name,
             )
-        else:
-            text = DEFAULT_ERROR_MESSAGE
 
         return self.respond_ephemeral(text)
 
+    @staticmethod
+    def _unpack_error_text(validation_error: serializers.ValidationError) -> str:
+        detail = validation_error.detail
+        while True:
+            if isinstance(detail, dict):
+                detail = list(detail.values())
+            element = detail[0]
+            if isinstance(element, str):
+                return element
+            detail = element
+
     def validation_error(
         self,
         slack_request: SlackActionRequest,
@@ -240,11 +258,15 @@ def validation_error(
             },
         )
 
-        text: str = list(*error.detail.values())[0]
+        text: str = self._unpack_error_text(error)
         return self.respond_ephemeral(text)
 
     def on_assign(
-        self, request: Request, user: RpcUser, group: Group, action: MessageAction
+        self,
+        request: Request,
+        user: RpcUser,
+        group: Group,
+        action: MessageAction | BlockKitMessageAction,
     ) -> None:
         if not (action.selected_options and len(action.selected_options)):
             # Short-circuit if action is invalid
@@ -269,7 +291,7 @@ def on_status(
         request: Request,
         user: RpcUser,
         group: Group,
-        action: MessageAction,
+        action: MessageAction | BlockKitMessageAction,
     ) -> None:
         status_data = (action.value or "").split(":", 2)
         if not len(status_data):
@@ -408,11 +430,11 @@ def _update_modal(
                     },
                 )
                 # The modal was not found, so we need to open a new one
-                self._open_view(slack_client, modal_payload, slack_request)
+                self._open_modal(slack_client, modal_payload, slack_request)
             else:
                 raise
 
-    def _open_view(
+    def _open_modal(
         self, slack_client: SlackSdkClient, modal_payload: View, slack_request: SlackActionRequest
     ) -> None:
         # Error handling is done in the calling function
@@ -451,10 +473,11 @@ def open_resolve_dialog(self, slack_request: SlackActionRequest, group: Group) -
             # We passed this in control when we sent the loading modal to beat the 3 second timeout
             external_id = slack_request.get_action_ts()
 
-            if options.get("send-slack-response-from-control-silo"):
-                self._update_modal(slack_client, external_id, modal_payload, slack_request)
+            if not external_id or not options.get("send-slack-response-from-control-silo"):
+                # If we don't have an external_id or option is disabled we need to open a new modal
+                self._open_modal(slack_client, modal_payload, slack_request)
             else:
-                self._open_view(slack_client, modal_payload, slack_request)
+                self._update_modal(slack_client, external_id, modal_payload, slack_request)
 
             metrics.incr(
                 SLACK_WEBHOOK_GROUP_ACTIONS_SUCCESS_DATADOG_METRIC,
@@ -503,10 +526,11 @@ def open_archive_dialog(self, slack_request: SlackActionRequest, group: Group) -
             # We passed this in control when we sent the loading modal to beat the 3 second timeout
             external_id = slack_request.get_action_ts()
 
-            if options.get("send-slack-response-from-control-silo"):
-                self._update_modal(slack_client, external_id, modal_payload, slack_request)
+            if not external_id or not options.get("send-slack-response-from-control-silo"):
+                # If we don't have an external_id or option is disabled we need to open a new modal
+                self._open_modal(slack_client, modal_payload, slack_request)
             else:
-                self._open_view(slack_client, modal_payload, slack_request)
+                self._update_modal(slack_client, external_id, modal_payload, slack_request)
 
             metrics.incr(
                 SLACK_WEBHOOK_GROUP_ACTIONS_SUCCESS_DATADOG_METRIC,
@@ -549,7 +573,7 @@ def _handle_group_actions(
         self,
         slack_request: SlackActionRequest,
         request: Request,
-        action_list: Sequence[MessageAction],
+        action_list: Sequence[BlockKitMessageAction],
     ) -> Response:
         from sentry.integrations.slack.views.link_identity import build_linking_url
 
@@ -564,6 +588,10 @@ def _handle_group_actions(
         identity_user = slack_request.get_identity_user()
 
         if not identity or not identity_user:
+            # if we don't have user_id or channel_id, we can't link the identity
+            if not slack_request.user_id or not slack_request.channel_id:
+                return self.respond_ephemeral(NO_IDENTITY_MESSAGE)
+
             associate_url = build_linking_url(
                 integration=slack_request.integration,
                 slack_id=slack_request.user_id,
@@ -594,10 +622,10 @@ def _handle_group_actions(
             if not selection:
                 return self.respond()
 
-            action = MessageAction(name="status", value=selection)
+            status_action = MessageAction(name="status", value=selection)
 
             try:
-                self.on_status(request, identity_user, group, action)
+                self.on_status(request, identity_user, group, status_action)
             except client.ApiError as error:
                 return self.api_error(slack_request, group, identity_user, error, "status_dialog")
 
@@ -608,19 +636,18 @@ def _handle_group_actions(
             blocks = SlackIssuesMessageBuilder(
                 group,
                 identity=identity,
-                actions=[action],
+                actions=[status_action],
                 tags=original_tags_from_request,
                 rules=[rule] if rule else None,
                 issue_details=True,
                 skip_fallback=True,
             ).build()
 
             # use the original response_url to update the link attachment
-            json_blocks = orjson.dumps(blocks.get("blocks")).decode()
             try:
                 webhook_client = WebhookClient(private_metadata["orig_response_url"])
                 webhook_client.send(
-                    blocks=json_blocks, delete_original=False, replace_original=True
+                    blocks=blocks.get("blocks"), delete_original=False, replace_original=True
                 )
                 metrics.incr(
                     SLACK_WEBHOOK_GROUP_ACTIONS_SUCCESS_DATADOG_METRIC,
@@ -700,18 +727,20 @@ def _handle_group_actions(
             return self.respond()
 
         response_url = slack_request.data["response_url"]
-        json_blocks = orjson.dumps(response.get("blocks")).decode()
         webhook_client = WebhookClient(response_url)
         try:
             webhook_client.send(
-                blocks=json_blocks,
+                blocks=response.get("blocks"),
                 text=response.get("text"),
                 delete_original=False,
                 replace_original=True,
             )
             _logger.info(
                 "slack.webhook.update_status.success",
-                extra={"integration_id": slack_request.integration.id, "blocks": json_blocks},
+                extra={
+                    "integration_id": slack_request.integration.id,
+                    "blocks": response.get("blocks"),
+                },
             )
             metrics.incr(
                 SLACK_WEBHOOK_GROUP_ACTIONS_SUCCESS_DATADOG_METRIC,
@@ -758,44 +787,42 @@ def get_action_option(cls, slack_request: SlackActionRequest) -> str | None:
         return action_option
 
     @classmethod
-    def get_action_list(cls, slack_request: SlackActionRequest) -> list[MessageAction]:
+    def get_action_list(cls, slack_request: SlackActionRequest) -> list[BlockKitMessageAction]:
         action_data = slack_request.data.get("actions")
-        if action_data:
-            # XXX(CEO): this is here for backwards compatibility - if a user performs an action with an "older"
-            # style issue alert but the block kit flag is enabled, we don't want to fall into this code path
-            if action_data[0].get("action_id"):
-                action_list = []
-                for action_data in action_data:
-                    if action_data.get("type") in ("static_select", "external_select"):
-                        action = BlockKitMessageAction(
-                            name=action_data["action_id"],
-                            label=action_data["selected_option"]["text"]["text"],
-                            type=action_data["type"],
-                            value=action_data["selected_option"]["value"],
-                            action_id=action_data["action_id"],
-                            block_id=action_data["block_id"],
-                            selected_options=[
-                                {"value": action_data.get("selected_option", {}).get("value")}
-                            ],
-                        )
-                        # TODO: selected_options is kinda ridiculous, I think this is built to handle multi-select?
-                    else:
-                        action = BlockKitMessageAction(
-                            name=action_data["action_id"],
-                            label=action_data["text"]["text"],
-                            type=action_data["type"],
-                            value=action_data["value"],
-                            action_id=action_data["action_id"],
-                            block_id=action_data["block_id"],
-                        )
-                    action_list.append(action)
-
-                return action_list
-        return [
-            MessageAction(**action_data)
-            for action_data in action_data or []
-            if "name" in action_data
-        ]
+        if (
+            not action_data
+            or not isinstance(action_data, list)
+            or not action_data[0].get("action_id")
+        ):
+            return []
+
+        action_list = []
+        for action_data in action_data:
+            if action_data.get("type") in ("static_select", "external_select"):
+                action = BlockKitMessageAction(
+                    name=action_data["action_id"],
+                    label=action_data["selected_option"]["text"]["text"],
+                    type=action_data["type"],
+                    value=action_data["selected_option"]["value"],
+                    action_id=action_data["action_id"],
+                    block_id=action_data["block_id"],
+                    selected_options=[
+                        {"value": action_data.get("selected_option", {}).get("value")}
+                    ],
+                )
+                # TODO: selected_options is kinda ridiculous, I think this is built to handle multi-select?
+            else:
+                action = BlockKitMessageAction(
+                    name=action_data["action_id"],
+                    label=action_data["text"]["text"],
+                    type=action_data["type"],
+                    value=action_data["value"],
+                    action_id=action_data["action_id"],
+                    block_id=action_data["block_id"],
+                )
+            action_list.append(action)
+
+        return action_list
 
     def post(self, request: Request) -> Response:
         try:
@@ -903,6 +930,7 @@ def handle_member_approval(self, slack_request: SlackActionRequest, action: str)
         except Exception:
             # shouldn't error but if it does, respond to the user
             _logger.exception(
+                "slack.action.member-invitation-error",
                 extra={
                     "organization_id": organization.id,
                     "member_id": member.id,