Browse Source

ref(notifications): consolidate notification logic (#30575)

This PR moves some logic and methods from the OrganizationRequestNotification class to its parent BaseNotification class. This is part of an effort to ensure greater standardization of notifications. Functionally, the only change is modifying the referrer for links to in the format of referrer_base + provider_type + recipient_type. This will improve our analytics so we know which provider and which kind of recipient are going to links in Sentry. It also changes the format slightly in some cases (AlertRuleSlackTeam becomes alert-rule-slack-team). However, this change will not impact how we look at the referrer in analytics.
Stephen Cefali 3 years ago
parent
commit
1063229777

+ 1 - 0
src/sentry/integrations/slack/analytics.py

@@ -17,6 +17,7 @@ class SlackIntegrationStatus(analytics.Event):  # type: ignore
     )
 
 
+# TODO: add field for whether target is user or team
 class SlackIntegrationNotificationSent(analytics.Event):  # type: ignore
     type = "integrations.slack.notification_sent"
 

+ 4 - 6
src/sentry/integrations/slack/message_builder/issues.py

@@ -1,6 +1,5 @@
 from __future__ import annotations
 
-import re
 from typing import Any, Callable, Mapping, Sequence
 
 from django.core.cache import cache
@@ -23,12 +22,11 @@ from sentry.models import (
 from sentry.notifications.notifications.base import BaseNotification, ProjectNotification
 from sentry.notifications.notifications.rules import AlertRuleNotification
 from sentry.notifications.utils.actions import MessageAction
+from sentry.types.integrations import ExternalProviders
 from sentry.utils import json
 from sentry.utils.dates import to_timestamp
 from sentry.utils.http import absolute_uri
 
-from ..utils import build_notification_footer
-
 STATUSES = {"resolved": "resolved", "ignored": "ignored", "unresolved": "re-opened"}
 
 
@@ -276,8 +274,8 @@ def get_title_link(
     if event and link_to_event:
         url = group.get_absolute_url(params={"referrer": "slack"}, event_id=event.event_id)
 
-    elif issue_details:
-        referrer = re.sub("Notification$", "Slack", notification.__class__.__name__)
+    elif issue_details and notification:
+        referrer = notification.get_referrer(ExternalProviders.SLACK)
         url = group.get_absolute_url(params={"referrer": referrer})
 
     else:
@@ -342,7 +340,7 @@ class SlackIssuesMessageBuilder(SlackMessageBuilder):
         color = get_color(event_for_tags, self.notification)
         fields = build_tag_fields(event_for_tags, self.tags)
         footer = (
-            build_notification_footer(self.notification, self.recipient)
+            self.notification.build_notification_footer(self.recipient)
             if self.notification and self.recipient
             else build_footer(self.group, project, self.rules)
         )

+ 1 - 6
src/sentry/integrations/slack/utils/__init__.py

@@ -1,5 +1,4 @@
 __all__ = (
-    "build_notification_footer",
     "check_signing_secret",
     "get_channel_id",
     "get_channel_id_with_timeout",
@@ -27,11 +26,7 @@ from .channel import (
     strip_channel_name,
     validate_channel_id,
 )
-from .notifications import (
-    build_notification_footer,
-    send_incident_alert_notification,
-    send_slack_response,
-)
+from .notifications import send_incident_alert_notification, send_slack_response
 from .users import get_slack_data_by_user, get_users
 
 SLACK_RATE_LIMITED_MESSAGE = "Requests to Slack exceeded the rate limit. Please try again later."

+ 1 - 54
src/sentry/integrations/slack/utils/notifications.py

@@ -1,19 +1,14 @@
 from __future__ import annotations
 
-import re
 from typing import Mapping
-from urllib.parse import urljoin
 
 from sentry.constants import ObjectStatus
 from sentry.incidents.models import AlertRuleTriggerAction, Incident
 from sentry.integrations.slack.client import SlackClient
 from sentry.integrations.slack.message_builder.incidents import SlackIncidentsMessageBuilder
-from sentry.models import Environment, Integration, Team, User
-from sentry.notifications.notifications.activity.release import ReleaseActivityNotification
-from sentry.notifications.notifications.base import BaseNotification
+from sentry.models import Integration
 from sentry.shared_integrations.exceptions import ApiError
 from sentry.utils import json
-from sentry.utils.http import absolute_uri
 
 from . import logger
 
@@ -50,54 +45,6 @@ def send_incident_alert_notification(
         logger.info("rule.fail.slack_post", extra={"error": str(e)})
 
 
-def get_referrer_qstring(notification: BaseNotification, recipient: Team | User) -> str:
-    # TODO: make a generic version that works for other notification types
-    return (
-        "?referrer="
-        + re.sub("Notification$", "Slack", notification.__class__.__name__)
-        + str(recipient.__class__.__name__)
-    )
-
-
-def get_settings_url(notification: BaseNotification, recipient: Team | User) -> str:
-    url_str = "/settings/account/notifications/"
-    if notification.fine_tuning_key:
-        url_str += f"{notification.fine_tuning_key}/"
-    return str(urljoin(absolute_uri(url_str), get_referrer_qstring(notification, recipient)))
-
-
-def build_notification_footer(notification: BaseNotification, recipient: Team | User) -> str:
-    if isinstance(recipient, Team):
-        team = Team.objects.get(id=recipient.id)
-        url_str = f"/settings/{notification.organization.slug}/teams/{team.slug}/notifications/"
-        settings_url = str(
-            urljoin(absolute_uri(url_str), get_referrer_qstring(notification, recipient))
-        )
-    else:
-        settings_url = get_settings_url(notification, recipient)
-
-    if isinstance(notification, ReleaseActivityNotification):
-        # no environment related to a deploy
-        if notification.release:
-            return f"{notification.release.projects.all()[0].slug} | <{settings_url}|Notification Settings>"
-        return f"<{settings_url}|Notification Settings>"
-
-    parent = getattr(notification, "project", notification.organization)
-    footer: str = parent.slug
-    group = getattr(notification, "group", None)
-    latest_event = group.get_latest_event() if group else None
-    environment = None
-    if latest_event:
-        try:
-            environment = latest_event.get_environment()
-        except Environment.DoesNotExist:
-            pass
-    if environment and getattr(environment, "name", None) != "":
-        footer += f" | {environment.name}"
-    footer += f" | <{settings_url}|Notification Settings>"
-    return footer
-
-
 def send_slack_response(
     integration: Integration, text: str, params: Mapping[str, str], command: str
 ) -> None:

+ 1 - 0
src/sentry/mail/analytics.py

@@ -1,6 +1,7 @@
 from sentry import analytics
 
 
+# TODO: should have same base class as SlackIntegrationNotificationSent
 class EmailNotificationSent(analytics.Event):  # type: ignore
     type = "integrations.email.notification_sent"
 

+ 7 - 0
src/sentry/notifications/notifications/activity/assigned.py

@@ -8,6 +8,8 @@ from .base import GroupActivityNotification
 
 
 class AssignedActivityNotification(GroupActivityNotification):
+    referrer_base = "assigned-activity"
+
     def get_activity_name(self) -> str:
         return "Assigned"
 
@@ -68,6 +70,8 @@ class AssignedActivityNotification(GroupActivityNotification):
         else:
             author = "Sentry"
 
+        # TODO: refactor so assignee type doesn't change
+
         # legacy Activity objects from before assignable teams
         if "assigneeType" not in data or data["assigneeType"] == "user":
             author = "themselves"
@@ -89,6 +93,9 @@ class AssignedActivityNotification(GroupActivityNotification):
                 assignee = "an unknown team"
             else:
                 assignee = f"#{assignee_team.slug}"
+        else:
+            # need this case to ensure assignee is bound
+            assignee = "unknown"
         return author, assignee
 
     def get_notification_title(self) -> str:

+ 5 - 3
src/sentry/notifications/notifications/activity/base.py

@@ -1,6 +1,5 @@
 from __future__ import annotations
 
-import re
 from abc import ABC
 from typing import TYPE_CHECKING, Any, Mapping, MutableMapping
 from urllib.parse import urlparse, urlunparse
@@ -10,6 +9,7 @@ from django.utils.safestring import SafeString, mark_safe
 
 from sentry.notifications.helpers import get_reason_context
 from sentry.notifications.notifications.base import ProjectNotification
+from sentry.notifications.types import NotificationSettingTypes
 from sentry.notifications.utils import send_activity_notification
 from sentry.notifications.utils.avatar import avatar_as_html
 from sentry.notifications.utils.participants import get_participants_for_group
@@ -20,7 +20,7 @@ if TYPE_CHECKING:
 
 
 class ActivityNotification(ProjectNotification, ABC):
-    fine_tuning_key = "workflow"
+    notification_setting_type = NotificationSettingTypes.WORKFLOW
     metrics_key = "activity"
 
     def __init__(self, activity: Activity) -> None:
@@ -88,7 +88,9 @@ class GroupActivityNotification(ActivityNotification, ABC):
         return self.get_activity_name()
 
     def get_group_link(self) -> str:
-        referrer = re.sub("Notification$", "Email", self.__class__.__name__)
+        # method only used for emails
+        # TODO: pass in recipient so we can add that to the referrer
+        referrer = self.get_referrer(ExternalProviders.EMAIL)
         return str(self.group.get_absolute_url(params={"referrer": referrer}))
 
     def get_participants_with_group_subscription_reason(

+ 2 - 0
src/sentry/notifications/notifications/activity/new_processing_issues.py

@@ -12,6 +12,8 @@ from .base import ActivityNotification
 
 
 class NewProcessingIssuesActivityNotification(ActivityNotification):
+    referrer_base = "new-processing-issues-activity"
+
     def __init__(self, activity: Activity) -> None:
         super().__init__(activity)
         self.issues = summarize_issues(self.activity.data["issues"])

+ 1 - 0
src/sentry/notifications/notifications/activity/note.py

@@ -7,6 +7,7 @@ from .base import GroupActivityNotification
 
 class NoteActivityNotification(GroupActivityNotification):
     message_builder = "SlackNotificationsMessageBuilder"
+    referrer_base = "note-activity"
 
     def get_activity_name(self) -> str:
         return "Note"

+ 2 - 0
src/sentry/notifications/notifications/activity/regression.py

@@ -9,6 +9,8 @@ from .base import GroupActivityNotification
 
 
 class RegressionActivityNotification(GroupActivityNotification):
+    referrer_base = "regression-activity"
+
     def get_activity_name(self) -> str:
         return "Regression"
 

Some files were not shown because too many files changed in this diff