Browse Source

ref(slack): refactor issues message builder (#70201)

Cathy Teng 10 months ago
parent
commit
2316baedde
1 changed files with 123 additions and 102 deletions
  1. 123 102
      src/sentry/integrations/slack/message_builder/issues.py

+ 123 - 102
src/sentry/integrations/slack/message_builder/issues.py

@@ -213,27 +213,33 @@ def get_tags(
 def get_context(group: Group) -> str:
     context_text = ""
 
-    context = group.issue_type.notification_config.context
-    context_dict = {}
-
-    for c in context:
-        if c in SUPPORTED_CONTEXT_DATA:
-            context_dict[c] = SUPPORTED_CONTEXT_DATA[c](group)
+    context = group.issue_type.notification_config.context.copy()
 
     # for errors, non-regression performance, and rage click issues
     # always show state and first seen
     # only show event count and user count if event count > 1 or state != new
 
-    event_count = context_dict.get("Events")
-    state = context_dict.get("State")
-    if (event_count and int(event_count) <= 1) or (state and state == "New"):
-        # filter out event count and users count
-        context_dict.pop("Events", None)
-        context_dict.pop("Users Affected", None)
+    state = None
+    event_count = None
+    if "State" in context:
+        state = SUPPORTED_CONTEXT_DATA["State"](group)
+    if "Events" in context:
+        event_count = SUPPORTED_CONTEXT_DATA["Events"](group)
+
+    if (state and state == "New") or (event_count and int(event_count) <= 1):
+        if "Events" in context:
+            context.remove("Events")
+
+        # avoid hitting Snuba for user count if we don't need it
+        if "Users Affected" in context:
+            context.remove("Users Affected")
+
+    for c in context:
+        if c in SUPPORTED_CONTEXT_DATA:
+            v = SUPPORTED_CONTEXT_DATA[c](group)
+            if v:
+                context_text += f"{c}: *{v}*   "
 
-    for k, v in context_dict.items():
-        if v:
-            context_text += f"{k}: *{v}*   "
     return context_text.rstrip()
 
 
@@ -361,7 +367,7 @@ def get_suspect_commit_text(
     return suspect_commit_text
 
 
-def get_action_text(text: str, actions: Sequence[Any], identity: RpcIdentity) -> str:
+def get_action_text(actions: Sequence[Any], identity: RpcIdentity) -> str:
     action_text = "\n".join(
         [
             action_text
@@ -381,7 +387,7 @@ def build_actions(
 ) -> tuple[Sequence[MessageAction], str, bool]:
     """Having actions means a button will be shown on the Slack message e.g. ignore, resolve, assign."""
     if actions and identity:
-        text = get_action_text(text, actions, identity)
+        text = get_action_text(actions, identity)
         if features.has("organizations:slack-improvements", project.organization):
             # if actions are taken, return True at the end to show the white circle emoji
             return [], text, True
@@ -478,6 +484,97 @@ class SlackIssuesMessageBuilder(BlockSlackMessageBuilder):
         """
         return True
 
+    def get_title_block(
+        self,
+        rule_id: int,
+        notification_uuid: str,
+        event_or_group: GroupEvent | Group,
+        has_action: bool,
+    ) -> SlackBlock:
+        title_link = get_title_link(
+            self.group,
+            self.event,
+            self.link_to_event,
+            self.issue_details,
+            self.notification,
+            ExternalProviders.SLACK,
+            rule_id,
+            notification_uuid=notification_uuid,
+        )
+        title = build_attachment_title(event_or_group)
+
+        is_error_issue = self.group.issue_category == GroupCategory.ERROR
+        title_emoji = None
+        if has_action:
+            # if issue is resolved, archived, or assigned, replace circle emojis with white circle
+            title_emoji = (
+                ACTION_EMOJI
+                if is_error_issue
+                else ACTIONED_CATEGORY_TO_EMOJI.get(self.group.issue_category)
+            )
+        elif is_error_issue:
+            level_text = None
+            for k, v in LOG_LEVELS_MAP.items():
+                if self.group.level == v:
+                    level_text = k
+
+            title_emoji = LEVEL_TO_EMOJI.get(level_text)
+        else:
+            title_emoji = CATEGORY_TO_EMOJI.get(self.group.issue_category)
+
+        title_text = (title_emoji + " " or "") + f"<{title_link}|*{escape_slack_text(title)}*>"
+
+        return self.get_markdown_block(title_text)
+
+    def get_text_block(self, text) -> SlackBlock:
+        if self.group.issue_category == GroupCategory.FEEDBACK:
+            max_block_text_length = USER_FEEDBACK_MAX_BLOCK_TEXT_LENGTH
+        else:
+            max_block_text_length = MAX_BLOCK_TEXT_LENGTH
+
+        return self.get_markdown_quote_block(text, max_block_text_length)
+
+    def get_suggested_assignees_block(self, suggested_assignees: list[str]) -> SlackBlock:
+        suggested_assignee_text = "Suggested Assignees: "
+        for assignee in suggested_assignees:
+            suggested_assignee_text += assignee + ", "
+        return self.get_context_block(suggested_assignee_text[:-2])  # get rid of comma at the end
+
+    def get_footer(self) -> SlackBlock:
+        # This link does not contain user input (it's a static label and a url), must not escape it.
+        replay_link = build_attachment_replay_link(self.group, self.event)
+
+        timestamp = None
+        if not self.issue_details:
+            ts = self.group.last_seen
+            timestamp = max(ts, self.event.datetime) if self.event else ts
+
+        project = Project.objects.get_from_cache(id=self.group.project_id)
+        footer = (
+            self.notification.build_notification_footer(self.recipient, ExternalProviders.SLACK)
+            if self.notification and self.recipient
+            else build_footer(self.group, project, self.rules, SLACK_URL_FORMAT)
+        )
+
+        if not self.notification:
+            # the footer content differs if it's a workflow notification, so we must check for that
+            footer_data = {
+                "Project": f"<{project.get_absolute_url()}|{escape_slack_text(project.slug)}>",
+                "Alert": footer,
+                "Short ID": self.group.qualified_short_id,
+            }
+            footer_text = ""
+            for k, v in footer_data.items():
+                footer_text += f"{k}: {v}    "
+
+            if replay_link:
+                footer_text += replay_link
+            else:
+                footer_text = footer_text[:-4]  # chop off the empty space
+            return self.get_context_block(text=footer_text)
+        else:
+            return self.get_context_block(text=footer, timestamp=timestamp)
+
     def build(self, notification_uuid: str | None = None) -> SlackBlock:
         # XXX(dcramer): options are limited to 100 choices, even when nested
         text = build_attachment_text(self.group, self.event) or ""
@@ -485,17 +582,11 @@ class SlackIssuesMessageBuilder(BlockSlackMessageBuilder):
 
         text = escape_slack_markdown_text(text)
 
-        # This link does not contain user input (it's a static label and a url), must not escape it.
-        replay_link = build_attachment_replay_link(self.group, self.event)
         project = Project.objects.get_from_cache(id=self.group.project_id)
 
         # If an event is unspecified, use the tags of the latest event (if one exists).
         event_for_tags = self.event or self.group.get_latest_event()
-        footer = (
-            self.notification.build_notification_footer(self.recipient, ExternalProviders.SLACK)
-            if self.notification and self.recipient
-            else build_footer(self.group, project, self.rules, SLACK_URL_FORMAT)
-        )
+
         obj = self.event if self.event is not None else self.group
         action_text = ""
 
@@ -513,62 +604,20 @@ class SlackIssuesMessageBuilder(BlockSlackMessageBuilder):
         if self.rules:
             rule_id = self.rules[0].id
 
-        title_link = get_title_link(
-            self.group,
-            self.event,
-            self.link_to_event,
-            self.issue_details,
-            self.notification,
-            ExternalProviders.SLACK,
-            rule_id,
-            notification_uuid=notification_uuid,
-        )
-        title = build_attachment_title(obj)
-
-        # build up the blocks for newer issue alert formatting #
-
-        # build title block
-        title_text = f"<{title_link}|*{escape_slack_text(title)}*>"
-
         # build up actions text
         if self.actions and self.identity and not action_text:
-            action_text = get_action_text(text, self.actions, self.identity)
+            # this means somebody is interacting with the message
+            action_text = get_action_text(self.actions, self.identity)
             if features.has("organizations:slack-improvements", self.group.project.organization):
                 has_action = True
 
-        is_error_issue = self.group.issue_category == GroupCategory.ERROR
-        if has_action:
-            # if issue is resolved, archived, or assigned, replace circle emojis with white circle
-            title_emoji = (
-                ACTION_EMOJI
-                if is_error_issue
-                else ACTIONED_CATEGORY_TO_EMOJI.get(self.group.issue_category)
-            )
-        elif is_error_issue:
-            level_text = None
-            for k, v in LOG_LEVELS_MAP.items():
-                if self.group.level == v:
-                    level_text = k
-
-            title_emoji = LEVEL_TO_EMOJI.get(level_text)
-        else:
-            title_emoji = CATEGORY_TO_EMOJI.get(self.group.issue_category)
-
-        if title_emoji:
-            title_text = f"{title_emoji} {title_text}"
-        blocks = [self.get_markdown_block(title_text)]
+        blocks = [self.get_title_block(rule_id, notification_uuid, obj, has_action)]
 
         # build up text block
+        text = text.lstrip(" ")
+        # XXX(CEO): sometimes text is " " and slack will error if we pass an empty string (now "")
         if text:
-            text = text.lstrip(" ")
-            # XXX(CEO): sometimes text is " " and slack will error if we pass an empty string (now "")
-            if text:
-                if self.group.issue_category == GroupCategory.FEEDBACK:
-                    max_block_text_length = USER_FEEDBACK_MAX_BLOCK_TEXT_LENGTH
-                else:
-                    max_block_text_length = MAX_BLOCK_TEXT_LENGTH
-
-                blocks.append(self.get_markdown_quote_block(text, max_block_text_length))
+            blocks.append(self.get_text_block(text))
 
         if self.actions:
             blocks.append(self.get_markdown_block(action_text))
@@ -615,12 +664,7 @@ class SlackIssuesMessageBuilder(BlockSlackMessageBuilder):
                 self.group.project, event_for_tags, assignee
             )
         if len(suggested_assignees) > 0:
-            suggested_assignee_text = "Suggested Assignees: "
-            for assignee in suggested_assignees:
-                suggested_assignee_text += assignee + ", "
-            blocks.append(
-                self.get_context_block(suggested_assignee_text[:-2])
-            )  # get rid of comma at the end
+            blocks.append(self.get_suggested_assignees_block(suggested_assignees))
 
         # add suspect commit info
         if event_for_tags:
@@ -634,30 +678,7 @@ class SlackIssuesMessageBuilder(BlockSlackMessageBuilder):
             blocks.append(self.get_markdown_block(notes_text))
 
         # build footer block
-        timestamp = None
-        if not self.issue_details:
-            ts = self.group.last_seen
-            timestamp = max(ts, self.event.datetime) if self.event else ts
-
-        if not self.notification:
-            # the footer content differs if it's a workflow notification, so we must check for that
-            footer_data = {
-                "Project": f"<{project.get_absolute_url()}|{escape_slack_text(project.slug)}>",
-                "Alert": footer,
-                "Short ID": self.group.qualified_short_id,
-            }
-            footer_text = ""
-            for k, v in footer_data.items():
-                footer_text += f"{k}: {v}    "
-
-            if replay_link:
-                footer_text += replay_link
-            else:
-                footer_text = footer_text[:-4]  # chop off the empty space
-            blocks.append(self.get_context_block(text=footer_text))
-        else:
-            blocks.append(self.get_context_block(text=footer, timestamp=timestamp))
-
+        blocks.append(self.get_footer())
         blocks.append(self.get_divider())
 
         block_id = {"issue": self.group.id}