chore(slack): remove non block kit part 2 (#68831)

Author: cathteng

src/sentry/integrations/message_builder.py

@@ -192,10 +192,7 @@ def build_footer(
         text = rules[0].label if rules[0].label else "Test Alert"
         footer += f" via {url_format.format(text=text, url=rule_url)}"
 
-        if (
-            features.has("organizations:slack-block-kit", project.organization)
-            and url_format == SLACK_URL_FORMAT
-        ):
+        if url_format == SLACK_URL_FORMAT:
             footer = url_format.format(text=text, url=rule_url)
 
         if len(rules) > 1:

src/sentry/integrations/slack/actions/notification.py

@@ -41,13 +41,10 @@ class SlackNotifyServiceAction(IntegrationEventAction):
     prompt = "Send a Slack notification"
     provider = "slack"
     integration_key = "workspace"
+    label = "Send a notification to the {workspace} Slack workspace to {channel} (optionally, an ID: {channel_id}) and show tags {tags} and notes {notes} in notification"
 
     def __init__(self, *args: Any, **kwargs: Any) -> None:
         super().__init__(*args, **kwargs)
-        # XXX(CEO): when removing the feature flag, put `label` back up as a class var
-        self.label = "Send a notification to the {workspace} Slack workspace to {channel} (optionally, an ID: {channel_id}) and show tags {tags} in notification"  # type: ignore[misc]
-        if features.has("organizations:slack-block-kit", self.project.organization):
-            self.label = "Send a notification to the {workspace} Slack workspace to {channel} (optionally, an ID: {channel_id}) and show tags {tags} and notes {notes} in notification"  # type: ignore[misc]
         self.form_fields = {
             "workspace": {
                 "type": "choice",
@@ -57,11 +54,10 @@ def __init__(self, *args: Any, **kwargs: Any) -> None:
             "channel_id": {"type": "string", "placeholder": "e.g., CA2FRA079 or UA1J9RTE1"},
             "tags": {"type": "string", "placeholder": "e.g., environment,user,my_tag"},
         }
-        if features.has("organizations:slack-block-kit", self.project.organization):
-            self.form_fields["notes"] = {
-                "type": "string",
-                "placeholder": "e.g. @jane, @on-call-team",
-            }
+        self.form_fields["notes"] = {
+            "type": "string",
+            "placeholder": "e.g. @jane, @on-call-team",
+        }
 
         self._repository: IssueAlertNotificationMessageRepository = (
             get_default_issue_alert_repository()
@@ -86,45 +82,25 @@ def send_notification(event: GroupEvent, futures: Sequence[RuleFuture]) -> None:
             additional_attachment = get_additional_attachment(
                 integration, self.project.organization
             )
-            payload = {}
-            if features.has("organizations:slack-block-kit", event.group.project.organization):
-                blocks = SlackIssuesMessageBuilder(
-                    group=event.group,
-                    event=event,
-                    tags=tags,
-                    rules=rules,
-                    notes=self.get_option("notes", ""),
-                ).build(notification_uuid=notification_uuid)
-
-                if additional_attachment:
-                    for block in additional_attachment:
-                        blocks["blocks"].append(block)
+            blocks = SlackIssuesMessageBuilder(
+                group=event.group,
+                event=event,
+                tags=tags,
+                rules=rules,
+                notes=self.get_option("notes", ""),
+            ).build(notification_uuid=notification_uuid)
 
-                if blocks.get("blocks"):
-                    payload = {
-                        "text": blocks.get("text"),
-                        "blocks": json.dumps(blocks.get("blocks")),
-                        "channel": channel,
-                        "unfurl_links": False,
-                        "unfurl_media": False,
-                    }
-            else:
-                attachments = [
-                    SlackIssuesMessageBuilder(
-                        group=event.group,
-                        event=event,
-                        tags=tags,
-                        rules=rules,
-                    ).build(notification_uuid=notification_uuid)
-                ]
-                # getsentry might add a billing related attachment
-                if additional_attachment:
-                    attachments.append(additional_attachment)
+            if additional_attachment:
+                for block in additional_attachment:
+                    blocks["blocks"].append(block)
 
+            if payload_blocks := blocks.get("blocks"):
                 payload = {
+                    "text": blocks.get("text"),
+                    "blocks": json.dumps(payload_blocks),
                     "channel": channel,
-                    "link_names": 1,
-                    "attachments": json.dumps(attachments),
+                    "unfurl_links": False,
+                    "unfurl_media": False,
                 }
                 self.logger.info(
                     "rule.slack_post.attachments",
@@ -203,9 +179,8 @@ def send_notification(event: GroupEvent, futures: Sequence[RuleFuture]) -> None:
                     "event_id": event.event_id,
                     "channel_name": self.get_option("channel"),
                 }
-                if features.has("organizations:slack-block-kit", event.group.project.organization):
-                    # temporarily log the payload so we can debug message failures
-                    log_params["payload"] = json.dumps(payload)
+                # temporarily log the payload so we can debug message failures
+                log_params["payload"] = json.dumps(payload)
 
                 self.logger.info(
                     "rule.fail.slack_post",
@@ -297,20 +272,12 @@ def send_confirmation_notification(
     def render_label(self) -> str:
         tags = self.get_tags_list()
 
-        if features.has("organizations:slack-block-kit", self.project.organization):
-            return self.label.format(
-                workspace=self.get_integration_name(),
-                channel=self.get_option("channel"),
-                channel_id=self.get_option("channel_id"),
-                tags="[{}]".format(", ".join(tags)),
-                notes=self.get_option("notes", ""),
-            )
-
         return self.label.format(
             workspace=self.get_integration_name(),
             channel=self.get_option("channel"),
             channel_id=self.get_option("channel_id"),
             tags="[{}]".format(", ".join(tags)),
+            notes=self.get_option("notes", ""),
         )
 
     def get_tags_list(self) -> Sequence[str]:

src/sentry/integrations/slack/message_builder/notifications/base.py

@@ -65,11 +65,7 @@ def build(self) -> SlackAttachment | SlackBlock:
 
         actions_block = []
         for action in actions:
-            if (
-                action.label in ["Turn on personal notifications", "Approve", "Reject"]
-                or action.url
-            ):
-                actions_block.append(self.get_button_action(action))
+            actions_block.append(self.get_button_action(action))
 
         if actions_block:
             blocks.append({"type": "actions", "elements": [action for action in actions_block]})

src/sentry/integrations/slack/notifications.py

@@ -7,10 +7,9 @@
 
 import sentry_sdk
 
-from sentry import features
 from sentry.integrations.mixins import NotifyBasicMixin
 from sentry.integrations.notifications import get_context, get_integrations_by_channel_by_recipient
-from sentry.integrations.slack.message_builder import SlackAttachment, SlackBlock
+from sentry.integrations.slack.message_builder import SlackBlock
 from sentry.integrations.slack.message_builder.base.block import BlockSlackMessageBuilder
 from sentry.integrations.slack.message_builder.notifications import get_message_builder
 from sentry.models.integrations.integration import Integration
@@ -47,24 +46,20 @@ def _get_attachments(
     recipient: RpcActor,
     shared_context: Mapping[str, Any],
     extra_context_by_actor: Mapping[RpcActor, Mapping[str, Any]] | None,
-) -> list[SlackAttachment] | SlackBlock:
+) -> SlackBlock:
     extra_context = (
         extra_context_by_actor[recipient] if extra_context_by_actor and recipient else {}
     )
     context = get_context(notification, recipient, shared_context, extra_context)
     cls = get_message_builder(notification.message_builder)
     attachments = cls(notification, context, recipient).build()
-    if isinstance(attachments, list) or features.has(
-        "organizations:slack-block-kit", notification.organization
-    ):
-        return attachments
-    return [attachments]
+    return attachments
 
 
 def _notify_recipient(
     notification: BaseNotification,
     recipient: RpcActor,
-    attachments: list[SlackAttachment] | SlackBlock,
+    attachments: SlackBlock,
     channel: str,
     integration: Integration,
     shared_context: Mapping[str, Any],
@@ -75,69 +70,39 @@ def _notify_recipient(
 
         text = notification.get_notification_title(ExternalProviders.SLACK, shared_context)
 
-        # NOTE(isabella): we check that attachments consists of blocks in case the flag is turned on
-        # while a notification with the legacy format is being sent
-        if features.has("organizations:slack-block-kit", notification.organization) and isinstance(
-            local_attachments, dict
-        ):
-            blocks = []
-            if text:
-                # NOTE(isabella): with legacy attachments, the notification title was
-                # automatically rendered based on the `text` field in the payload; in the block
-                # system, that payload field is used strictly as preview/fallback text, so we need
-                # to add this block to render the title in the notification ie. "Issue marked as
-                # regression", "New comment by <user>"
-                blocks.append(BlockSlackMessageBuilder.get_markdown_block(text))
-            attachment_blocks = local_attachments.get("blocks")
-            if attachment_blocks:
-                for attachment in attachment_blocks:
-                    blocks.append(attachment)
-            if len(blocks) >= 2 and blocks[1].get("block_id"):
-                # block id needs to be in the first block
-                blocks[0]["block_id"] = blocks[1]["block_id"]
-                del blocks[1]["block_id"]
-            additional_attachment = get_additional_attachment(
-                integration, notification.organization
-            )
-            if additional_attachment:
-                for block in additional_attachment:
-                    blocks.append(block)
-            if (
-                not text
-            ):  # if there isn't a notification title, try using message description as fallback
-                text = notification.get_message_description(recipient, ExternalProviders.SLACK)
-            payload = {
-                "channel": channel,
-                "unfurl_links": False,
-                "unfurl_media": False,
-                "text": text if text else "",
-                "blocks": json.dumps(blocks),
-            }
-            callback_id = local_attachments.get("callback_id")
-            if callback_id:
-                # callback_id is now at the same level as blocks, rather than within attachments
-                if isinstance(callback_id, str):
-                    payload["callback_id"] = callback_id
-                else:
-                    payload["callback_id"] = json.dumps(local_attachments.get("callback_id"))
-        else:
-            # Add optional billing related attachment.
-            additional_attachment = get_additional_attachment(
-                integration, notification.organization
-            )
-            if additional_attachment:
-                local_attachments.append(additional_attachment)
-
-            # unfurl_links and unfurl_media are needed to preserve the intended message format
-            # and prevent the app from replying with help text to the unfurl
-            payload = {
-                "channel": channel,
-                "link_names": 1,
-                "unfurl_links": False,
-                "unfurl_media": False,
-                "text": text,
-                "attachments": json.dumps(local_attachments),
-            }
+        blocks = []
+        if text:
+            blocks.append(BlockSlackMessageBuilder.get_markdown_block(text))
+        attachment_blocks = local_attachments.get("blocks")
+        if attachment_blocks:
+            for attachment in attachment_blocks:
+                blocks.append(attachment)
+        if len(blocks) >= 2 and blocks[1].get("block_id"):
+            # block id needs to be in the first block
+            blocks[0]["block_id"] = blocks[1]["block_id"]
+            del blocks[1]["block_id"]
+        additional_attachment = get_additional_attachment(integration, notification.organization)
+        if additional_attachment:
+            for block in additional_attachment:
+                blocks.append(block)
+        if (
+            not text
+        ):  # if there isn't a notification title, try using message description as fallback
+            text = notification.get_message_description(recipient, ExternalProviders.SLACK)
+        payload = {
+            "channel": channel,
+            "unfurl_links": False,
+            "unfurl_media": False,
+            "text": text if text else "",
+            "blocks": json.dumps(blocks),
+        }
+        callback_id = local_attachments.get("callback_id")
+        if callback_id:
+            # callback_id is now at the same level as blocks, rather than within attachments
+            if isinstance(callback_id, str):
+                payload["callback_id"] = callback_id
+            else:
+                payload["callback_id"] = json.dumps(local_attachments.get("callback_id"))
 
         post_message_task = post_message
         if SiloMode.get_current_mode() == SiloMode.CONTROL:

tests/sentry/api/endpoints/test_organization_invite_request_index.py

@@ -9,8 +9,7 @@
 from sentry.models.organizationmember import InviteStatus, OrganizationMember
 from sentry.models.organizationmemberteam import OrganizationMemberTeam
 from sentry.testutils.cases import APITestCase, SlackActivityNotificationTest
-from sentry.testutils.helpers.features import with_feature
-from sentry.testutils.helpers.slack import get_attachment_no_text
+from sentry.testutils.helpers.slack import get_blocks_and_fallback_text
 from sentry.testutils.hybrid_cloud import HybridCloudTestMixin
 from sentry.testutils.outbox import outbox_runner
 from sentry.utils import json
@@ -188,9 +187,7 @@ def test_request_to_invite_email(self):
         assert "eric@localhost" in mail.outbox[0].body
 
     @responses.activate
-    @with_feature({"organizations:slack-block-kit": False})
     def test_request_to_invite_slack(self):
-        # TODO: make this a block kit test
         with self.tasks():
             self.get_success_response(
                 self.organization.slug,
@@ -200,44 +197,44 @@ def test_request_to_invite_slack(self):
                 status_code=201,
             )
 
-        attachment = get_attachment_no_text()
+        blocks, fallback_text = get_blocks_and_fallback_text()
         assert (
-            attachment["text"]
+            fallback_text
             == f"foo@localhost is requesting to invite eric@localhost into {self.organization.name}"
         )
-        notification_uuid = parse_qs(urlparse(attachment["actions"][2]["url"]).query)[
-            "notification_uuid"
-        ][0]
-        assert attachment["actions"] == [
+        query_params = parse_qs(urlparse(blocks[1]["elements"][0]["text"]).query)
+        notification_uuid = query_params["notification_uuid"][0]
+        notification_uuid = notification_uuid.split("|")[
+            0
+        ]  # remove method of hyperlinking in slack
+        assert blocks[2]["elements"] == [
             {
-                "text": "Approve",
-                "name": "Approve",
-                "style": "primary",
                 "type": "button",
-                "value": "approve_member",
+                "text": {"type": "plain_text", "text": "Approve"},
                 "action_id": "approve_request",
+                "value": "approve_member",
             },
             {
-                "text": "Reject",
-                "name": "Reject",
-                "style": "danger",
                 "type": "button",
-                "value": "reject_member",
+                "text": {"type": "plain_text", "text": "Reject"},
                 "action_id": "approve_request",
+                "value": "reject_member",
             },
             {
-                "text": "See Members & Requests",
-                "name": "See Members & Requests",
-                "url": f"http://testserver/settings/{self.organization.slug}/members/?referrer=invite_request-slack-user&notification_uuid={notification_uuid}",
                 "type": "button",
+                "text": {"type": "plain_text", "text": "See Members & Requests"},
+                "url": f"http://testserver/settings/{self.organization.slug}/members/?referrer=invite_request-slack-user&notification_uuid={notification_uuid}",
+                "value": "link_clicked",
             },
         ]
+        footer = blocks[1]["elements"][0]["text"]
         assert (
-            attachment["footer"]
+            footer
             == f"You are receiving this notification because you have the scope member:write | <http://testserver/settings/account/notifications/approval/?referrer=invite_request-slack-user&notification_uuid={notification_uuid}|Notification Settings>"
         )
         member = OrganizationMember.objects.get(email="eric@localhost")
-        assert json.loads(attachment["callback_id"]) == {
+        data = parse_qs(responses.calls[0].request.body)
+        assert json.loads(data["callback_id"][0]) == {
             "member_id": member.id,
             "member_email": "eric@localhost",
         }

tests/sentry/api/endpoints/test_project_rule_details.py

@@ -1002,9 +1002,7 @@ def test_update_channel_slack(self, mock_send_confirmation_notification):
 
     @responses.activate
     @with_feature("organizations:rule-create-edit-confirm-notification")
-    @with_feature({"organizations:slack-block-kit": False})
     def test_slack_confirmation_notification_contents(self):
-        # TODO: make this a block kit test
         conditions = [
             {"id": "sentry.rules.conditions.first_seen_event.FirstSeenEventCondition"},
         ]
@@ -1093,7 +1091,7 @@ def test_slack_confirmation_notification_contents(self):
         assert rendered_blocks[0]["text"]["text"] == message
         changes = "*Changes*\n"
         changes += "• Added condition 'The issue's category is equal to Performance'\n"
-        changes += "• Changed action from *Send a notification to the Awesome Team Slack workspace to #old_channel_name (optionally, an ID: old_channel_id) and show tags [] in notification* to *Send a notification to the Awesome Team Slack workspace to new_channel_name (optionally, an ID: new_channel_id) and show tags [] in notification*\n"
+        changes += "• Changed action from *Send a notification to the Awesome Team Slack workspace to #old_channel_name (optionally, an ID: old_channel_id) and show tags [] and notes  in notification* to *Send a notification to the Awesome Team Slack workspace to new_channel_name (optionally, an ID: new_channel_id) and show tags [] and notes  in notification*\n"
         changes += "• Changed frequency from *5 minutes* to *3 hours*\n"
         changes += f"• Added *{staging_env.name}* environment\n"
         changes += "• Changed rule name from *my rule* to *new rule*\n"