Browse Source

Remove should_email() (#33384)

Marcos Gaeta 2 years ago
parent
commit
4a2759f20f

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

@@ -1,6 +1,6 @@
 from __future__ import annotations
 
-from abc import ABC
+import abc
 from typing import TYPE_CHECKING, Any, Mapping, MutableMapping
 from urllib.parse import urlparse, urlunparse
 
@@ -19,7 +19,7 @@ if TYPE_CHECKING:
     from sentry.models import Activity, Team, User
 
 
-class ActivityNotification(ProjectNotification, ABC):
+class ActivityNotification(ProjectNotification, abc.ABC):
     notification_setting_type = NotificationSettingTypes.WORKFLOW
     metrics_key = "activity"
 
@@ -56,13 +56,15 @@ class ActivityNotification(ProjectNotification, ABC):
     def get_type(self) -> str:
         return f"notify.activity.{self.activity.get_type_display()}"
 
+    @abc.abstractmethod
     def get_context(self) -> MutableMapping[str, Any]:
-        raise NotImplementedError
+        pass
 
+    @abc.abstractmethod
     def get_participants_with_group_subscription_reason(
         self,
     ) -> Mapping[ExternalProviders, Mapping[Team | User, int]]:
-        raise NotImplementedError
+        pass
 
     def send(self) -> None:
         return send_activity_notification(self)
@@ -71,7 +73,7 @@ class ActivityNotification(ProjectNotification, ABC):
         return {"activity": self.activity, **super().get_log_params(recipient)}
 
 
-class GroupActivityNotification(ActivityNotification, ABC):
+class GroupActivityNotification(ActivityNotification, abc.ABC):
     message_builder = "IssueNotificationMessageBuilder"
 
     def __init__(self, activity: Activity) -> None:

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

@@ -63,9 +63,6 @@ class ReleaseActivityNotification(ActivityNotification):
         self.version = self.release.version
         self.version_parsed = parse_release(self.version)["description"]
 
-    def should_email(self) -> bool:
-        return bool(self.release and self.deploy)
-
     def get_participants_with_group_subscription_reason(
         self,
     ) -> Mapping[ExternalProviders, Mapping[Team | User, int]]:
@@ -178,3 +175,8 @@ class ReleaseActivityNotification(ActivityNotification):
         if self.release:
             return f"{self.release.projects.all()[0].slug} | <{settings_url}|Notification Settings>"
         return f"<{settings_url}|Notification Settings>"
+
+    def send(self) -> None:
+        # Don't create a message when the Activity doesn't have a release and deploy.
+        if bool(self.release and self.deploy):
+            return super().send()

+ 0 - 3
src/sentry/notifications/notifications/base.py

@@ -72,9 +72,6 @@ class BaseNotification(abc.ABC):
     def get_reply_reference(self) -> Any | None:
         return None
 
-    def should_email(self) -> bool:
-        return True
-
     def get_template(self) -> str:
         return f"sentry/emails/{self.get_filename()}.txt"
 

+ 0 - 3
src/sentry/notifications/notifications/digest.py

@@ -128,9 +128,6 @@ class DigestNotification(ProjectNotification):
         }
 
     def send(self) -> None:
-        if not self.should_email():
-            return
-
         # Only calculate shared context once.
         shared_context = self.get_context()
 

+ 0 - 3
src/sentry/notifications/utils/__init__.py

@@ -268,9 +268,6 @@ def get_interface_list(event: Event) -> Sequence[tuple[str, str, str]]:
 
 
 def send_activity_notification(notification: ActivityNotification | UserReportNotification) -> None:
-    if not notification.should_email():
-        return
-
     participants_by_provider = notification.get_participants_with_group_subscription_reason()
     if not participants_by_provider:
         return

+ 0 - 1
tests/sentry/mail/activity/test_release.py

@@ -130,7 +130,6 @@ class ReleaseTestCase(ActivityTestCase):
         )
 
         assert email.release is None
-        assert not email.should_email()
 
     def test_no_committers(self):
         release, deploy = self.another_release("b")