From 90d9f005878240621db2e9d8a6d935b0201dd6bf Mon Sep 17 00:00:00 2001 From: Raj Joshi Date: Wed, 26 Mar 2025 14:09:51 -0700 Subject: [PATCH 1/2] :recycle: ref: refactor notification building utils to prevent circular dependency --- .../discord/message_builder/issues.py | 7 ++++- .../integrations/messaging/message_builder.py | 9 +++---- src/sentry/integrations/messaging/types.py | 11 ++++++++ .../msteams/card_builder/issues.py | 7 ++++- .../slack/message_builder/incidents.py | 7 ++--- .../slack/message_builder/issues.py | 14 +++++++--- .../slack/message_builder/metric_alerts.py | 7 ++--- .../slack/message_builder/types.py | 11 -------- .../slack/message_builder/util.py | 27 +++++++++++++++++++ src/sentry_plugins/slack/plugin.py | 2 +- .../integrations/discord/test_issue_alert.py | 2 +- .../slack/test_message_builder.py | 2 +- 12 files changed, 71 insertions(+), 35 deletions(-) create mode 100644 src/sentry/integrations/messaging/types.py create mode 100644 src/sentry/integrations/slack/message_builder/util.py diff --git a/src/sentry/integrations/discord/message_builder/issues.py b/src/sentry/integrations/discord/message_builder/issues.py index 8be7cf5cddfa74..e685d8854d0e47 100644 --- a/src/sentry/integrations/discord/message_builder/issues.py +++ b/src/sentry/integrations/discord/message_builder/issues.py @@ -73,7 +73,12 @@ def build(self, notification_uuid: str | None = None) -> dict[str, object]: color=LEVEL_TO_COLOR[get_color(event_for_tags, self.notification, self.group)], # We can't embed urls in Discord embed footers. footer=DiscordMessageEmbedFooter( - build_footer(self.group, project, self.rules, "{text}") + build_footer( + group=self.group, + project=project, + url_format="{text}", + rules=self.rules, + ) ), fields=build_tag_fields(event_for_tags, self.tags), timestamp=timestamp, diff --git a/src/sentry/integrations/messaging/message_builder.py b/src/sentry/integrations/messaging/message_builder.py index 6567c0c6670aba..891a3918765a1c 100644 --- a/src/sentry/integrations/messaging/message_builder.py +++ b/src/sentry/integrations/messaging/message_builder.py @@ -5,7 +5,7 @@ from sentry import features from sentry.eventstore.models import Event, GroupEvent -from sentry.integrations.slack.message_builder.types import LEVEL_TO_COLOR, SLACK_URL_FORMAT +from sentry.integrations.messaging.types import LEVEL_TO_COLOR from sentry.integrations.types import EXTERNAL_PROVIDERS, ExternalProviders from sentry.issues.grouptype import GroupCategory from sentry.models.environment import Environment @@ -173,7 +173,7 @@ def build_attachment_text(group: Group, event: Event | GroupEvent | None = None) def build_attachment_replay_link( - group: Group, event: Event | GroupEvent | None = None, url_format: str = SLACK_URL_FORMAT + group: Group, url_format: str, event: Event | GroupEvent | None = None ) -> str | None: has_replay = features.has("organizations:session-replay", group.organization) has_slack_links = features.has( @@ -199,8 +199,8 @@ def build_rule_url(rule: Any, group: Group, project: Project) -> str: def build_footer( group: Group, project: Project, + url_format: str, rules: Sequence[Rule] | None = None, - url_format: str = SLACK_URL_FORMAT, ) -> str: footer = f"{group.qualified_short_id}" if rules: @@ -210,9 +210,6 @@ 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 url_format == SLACK_URL_FORMAT: - footer = url_format.format(text=text, url=rule_url) - if len(rules) > 1: footer += f" (+{len(rules) - 1} other)" diff --git a/src/sentry/integrations/messaging/types.py b/src/sentry/integrations/messaging/types.py new file mode 100644 index 00000000000000..90c21a968c2dd3 --- /dev/null +++ b/src/sentry/integrations/messaging/types.py @@ -0,0 +1,11 @@ +# Attachment colors used for issues with no actions take. + +LEVEL_TO_COLOR = { + "_actioned_issue": "#EDEEEF", + "_incident_resolved": "#4DC771", + "debug": "#FBE14F", + "error": "#E03E2F", + "fatal": "#FA4747", + "info": "#2788CE", + "warning": "#FFC227", +} diff --git a/src/sentry/integrations/msteams/card_builder/issues.py b/src/sentry/integrations/msteams/card_builder/issues.py index 56b879f74de19e..320519b815ebfc 100644 --- a/src/sentry/integrations/msteams/card_builder/issues.py +++ b/src/sentry/integrations/msteams/card_builder/issues.py @@ -129,7 +129,12 @@ def build_group_footer(self) -> ColumnSetBlock: image_column = create_footer_logo_block() - text = build_footer(self.group, project, self.rules, MSTEAMS_URL_FORMAT) + text = build_footer( + group=self.group, + project=project, + url_format=MSTEAMS_URL_FORMAT, + rules=self.rules, + ) text_column = create_footer_column_block(create_footer_text_block(text)) diff --git a/src/sentry/integrations/slack/message_builder/incidents.py b/src/sentry/integrations/slack/message_builder/incidents.py index 8bdcd9a52fc5df..a2f2c71078f7fa 100644 --- a/src/sentry/integrations/slack/message_builder/incidents.py +++ b/src/sentry/integrations/slack/message_builder/incidents.py @@ -1,13 +1,10 @@ from datetime import datetime from sentry.incidents.typings.metric_detector import AlertContext, MetricIssueContext +from sentry.integrations.messaging.types import LEVEL_TO_COLOR from sentry.integrations.metric_alerts import incident_attachment_info from sentry.integrations.slack.message_builder.base.block import BlockSlackMessageBuilder -from sentry.integrations.slack.message_builder.types import ( - INCIDENT_COLOR_MAPPING, - LEVEL_TO_COLOR, - SlackBody, -) +from sentry.integrations.slack.message_builder.types import INCIDENT_COLOR_MAPPING, SlackBody from sentry.integrations.slack.utils.escape import escape_slack_text from sentry.models.organization import Organization diff --git a/src/sentry/integrations/slack/message_builder/issues.py b/src/sentry/integrations/slack/message_builder/issues.py index c64a37013de558..3514d2c22c1006 100644 --- a/src/sentry/integrations/slack/message_builder/issues.py +++ b/src/sentry/integrations/slack/message_builder/issues.py @@ -17,7 +17,6 @@ build_attachment_replay_link, build_attachment_text, build_attachment_title, - build_footer, format_actor_option_slack, format_actor_options_slack, get_title_link, @@ -32,6 +31,7 @@ SLACK_URL_FORMAT, SlackBlock, ) +from sentry.integrations.slack.message_builder.util import build_slack_footer from sentry.integrations.slack.utils.escape import escape_slack_markdown_text, escape_slack_text from sentry.integrations.time_utils import get_approx_start_time, time_since from sentry.integrations.types import ExternalProviders @@ -506,7 +506,11 @@ def get_suggested_assignees_block(self, suggested_assignees: list[str]) -> Slack 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) + replay_link = build_attachment_replay_link( + group=self.group, + url_format=SLACK_URL_FORMAT, + event=self.event, + ) timestamp = None if not self.issue_details: @@ -517,7 +521,11 @@ def get_footer(self) -> SlackBlock: 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) + else build_slack_footer( + group=self.group, + project=project, + rules=self.rules, + ) ) if not self.notification: diff --git a/src/sentry/integrations/slack/message_builder/metric_alerts.py b/src/sentry/integrations/slack/message_builder/metric_alerts.py index 9022aeec248e8b..e229d006cd1154 100644 --- a/src/sentry/integrations/slack/message_builder/metric_alerts.py +++ b/src/sentry/integrations/slack/message_builder/metric_alerts.py @@ -1,12 +1,9 @@ from sentry.incidents.models.alert_rule import AlertRule from sentry.incidents.models.incident import Incident, IncidentStatus +from sentry.integrations.messaging.types import LEVEL_TO_COLOR from sentry.integrations.metric_alerts import metric_alert_unfurl_attachment_info from sentry.integrations.slack.message_builder.base.block import BlockSlackMessageBuilder -from sentry.integrations.slack.message_builder.types import ( - INCIDENT_COLOR_MAPPING, - LEVEL_TO_COLOR, - SlackBody, -) +from sentry.integrations.slack.message_builder.types import INCIDENT_COLOR_MAPPING, SlackBody from sentry.integrations.slack.utils.escape import escape_slack_text diff --git a/src/sentry/integrations/slack/message_builder/types.py b/src/sentry/integrations/slack/message_builder/types.py index 1aabe8d29f236d..d243ceeea0d11c 100644 --- a/src/sentry/integrations/slack/message_builder/types.py +++ b/src/sentry/integrations/slack/message_builder/types.py @@ -7,17 +7,6 @@ SlackBlock = dict[str, Any] SlackBody = Union[SlackAttachment, SlackBlock] -# Attachment colors used for issues with no actions take. -LEVEL_TO_COLOR = { - "_actioned_issue": "#EDEEEF", - "_incident_resolved": "#4DC771", - "debug": "#FBE14F", - "error": "#E03E2F", - "fatal": "#FA4747", - "info": "#2788CE", - "warning": "#FFC227", -} - INCIDENT_COLOR_MAPPING = { "Resolved": "_incident_resolved", "Warning": "warning", diff --git a/src/sentry/integrations/slack/message_builder/util.py b/src/sentry/integrations/slack/message_builder/util.py new file mode 100644 index 00000000000000..666d64091ad0b4 --- /dev/null +++ b/src/sentry/integrations/slack/message_builder/util.py @@ -0,0 +1,27 @@ +from collections.abc import Sequence + +from sentry.integrations.messaging.message_builder import build_rule_url +from sentry.integrations.slack.message_builder.types import SLACK_URL_FORMAT +from sentry.models.group import Group +from sentry.models.project import Project +from sentry.models.rule import Rule + + +def build_slack_footer( + group: Group, + project: Project, + rules: Sequence[Rule] | None = None, +) -> str: + footer = f"{group.qualified_short_id}" + + if rules: + rule_url = build_rule_url(rules[0], group, project) + # If this notification is triggered via the "Send Test Notification" + # button then the label is not defined, but the url works. + text = rules[0].label if rules[0].label else "Test Alert" + footer = SLACK_URL_FORMAT.format(text=text, url=rule_url) + + if len(rules) > 1: + footer += f" (+{len(rules) - 1} other)" + + return footer diff --git a/src/sentry_plugins/slack/plugin.py b/src/sentry_plugins/slack/plugin.py index eb2d7789d60c1c..f329d6e6e976a9 100644 --- a/src/sentry_plugins/slack/plugin.py +++ b/src/sentry_plugins/slack/plugin.py @@ -2,7 +2,7 @@ from sentry import tagstore from sentry.integrations.base import FeatureDescription, IntegrationFeatures -from sentry.integrations.slack.message_builder.types import LEVEL_TO_COLOR +from sentry.integrations.messaging.types import LEVEL_TO_COLOR from sentry.plugins.base.structs import Notification from sentry.plugins.bases import notify from sentry.shared_integrations.exceptions import ApiError diff --git a/tests/sentry/integrations/discord/test_issue_alert.py b/tests/sentry/integrations/discord/test_issue_alert.py index 2fc7950d83c8fb..3813f5cc30f7cc 100644 --- a/tests/sentry/integrations/discord/test_issue_alert.py +++ b/tests/sentry/integrations/discord/test_issue_alert.py @@ -140,7 +140,7 @@ def test_basic(self, mock_record): notification_uuid=notification_uuid, ), "color": LEVEL_TO_COLOR["error"], - "footer": {"text": build_footer(self.event.group, self.event.project, None, "{text}")}, + "footer": {"text": build_footer(self.event.group, self.event.project, "{text}", None)}, "fields": [], "timestamp": self.event.timestamp, } diff --git a/tests/sentry/integrations/slack/test_message_builder.py b/tests/sentry/integrations/slack/test_message_builder.py index c35d4fafc5655c..05c7ba2c8987d9 100644 --- a/tests/sentry/integrations/slack/test_message_builder.py +++ b/tests/sentry/integrations/slack/test_message_builder.py @@ -22,6 +22,7 @@ build_attachment_text, build_attachment_title, ) +from sentry.integrations.messaging.types import LEVEL_TO_COLOR from sentry.integrations.slack.message_builder.incidents import SlackIncidentsMessageBuilder from sentry.integrations.slack.message_builder.issues import ( SlackIssuesMessageBuilder, @@ -32,7 +33,6 @@ get_tags, ) from sentry.integrations.slack.message_builder.metric_alerts import SlackMetricAlertMessageBuilder -from sentry.integrations.slack.message_builder.types import LEVEL_TO_COLOR from sentry.integrations.time_utils import time_since from sentry.issues.grouptype import ( FeedbackGroup, From 829e30c3257bb0112ec7506813e65ce8b2c885f8 Mon Sep 17 00:00:00 2001 From: Raj Joshi Date: Wed, 26 Mar 2025 14:17:42 -0700 Subject: [PATCH 2/2] :wrench: chore: fix test --- tests/sentry_plugins/slack/test_plugin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/sentry_plugins/slack/test_plugin.py b/tests/sentry_plugins/slack/test_plugin.py index f440c14064e450..efba90a826385d 100644 --- a/tests/sentry_plugins/slack/test_plugin.py +++ b/tests/sentry_plugins/slack/test_plugin.py @@ -5,7 +5,7 @@ import pytest import responses -from sentry.integrations.slack.message_builder.types import LEVEL_TO_COLOR +from sentry.integrations.messaging.types import LEVEL_TO_COLOR from sentry.models.rule import Rule from sentry.plugins.base import Notification from sentry.shared_integrations.exceptions import ApiError