ref(notifications): Add digest emails to Notifications hierarchy (#27928)

src/sentry/digests/__init__.py

@@ -1,4 +1,5 @@
 from collections import namedtuple
+from typing import Any, Mapping
 
 from django.conf import settings
 
@@ -24,6 +25,8 @@ ScheduleEntry = namedtuple("ScheduleEntry", "key timestamp")
 
 OPTIONS = frozenset(("increment_delay", "maximum_delay", "minimum_delay"))
 
+Digest = Mapping[str, Mapping[str, Any]]
+
 
 def get_option_key(plugin, option):
     assert option in OPTIONS

src/sentry/digests/utilities.py

@@ -1,6 +1,7 @@
 from collections import Counter, OrderedDict, defaultdict
 
 from sentry.models import ActorTuple, OrganizationMemberTeam, ProjectOwnership, Team, User
+from sentry.notifications.types import ActionTargetType
 
 
 # TODO(tkaemming): This should probably just be part of `build_digest`.
@@ -23,6 +24,13 @@ def get_digest_metadata(digest):
     return start, end, counts
 
 
+def should_get_personalized_digests(target_type: ActionTargetType, project_id: int) -> bool:
+    return (
+        target_type == ActionTargetType.ISSUE_OWNERS
+        and ProjectOwnership.objects.filter(project_id=project_id).exists()
+    )
+
+
 def get_personalized_digests(target_type, project_id, digest, user_ids):
     """
     get_personalized_digests(project_id: Int, digest: Digest, user_ids: Set[Int]) -> Iterator[user_id: Int, digest: Digest]

src/sentry/mail/adapter.py

@@ -1,33 +1,28 @@
-import itertools
 import logging
 from typing import Any, Optional, Sequence
 
-from django.utils import dateformat
-from django.utils.encoding import force_text
-
-from sentry import digests, options
+from sentry import digests
 from sentry.digests import get_option_key as get_digest_option_key
 from sentry.digests.notifications import event_to_record, unsplit_key
-from sentry.digests.utilities import get_digest_metadata, get_personalized_digests
 from sentry.models import NotificationSetting, Project, ProjectOption
 from sentry.notifications.notifications.activity import EMAIL_CLASSES_BY_TYPE
+from sentry.notifications.notifications.digest import DigestNotification
 from sentry.notifications.notifications.rules import AlertRuleNotification
 from sentry.notifications.notifications.user_report import UserReportNotification
 from sentry.notifications.types import ActionTargetType
-from sentry.notifications.utils import get_integration_link, has_alert_integration
-from sentry.notifications.utils.participants import get_send_to
 from sentry.plugins.base.structs import Notification
 from sentry.tasks.digests import deliver_digest
 from sentry.types.integrations import ExternalProviders
-from sentry.utils import json, metrics
-from sentry.utils.email import MessageBuilder
-from sentry.utils.linksign import generate_signed_link
+from sentry.utils import metrics
 
 logger = logging.getLogger(__name__)
 
 
 class MailAdapter:
-    """This class contains generic logic for notifying users via Email."""
+    """
+    This class contains generic logic for notifying users via Email.
+    TODO(mgaeta): Make an explicit interface that is shared with NotificationPlugin.
+    """
 
     mail_option_key = "mail:subject_prefix"
 
@@ -84,52 +79,6 @@ class MailAdapter:
 
         logger.info("mail.adapter.notification.%s" % log_event, extra=extra)
 
-    def _build_subject_prefix(self, project):
-        subject_prefix = ProjectOption.objects.get_value(project, self.mail_option_key, None)
-        if not subject_prefix:
-            subject_prefix = options.get("mail.subject-prefix")
-        return force_text(subject_prefix)
-
-    def _build_message(
-        self,
-        project,
-        subject,
-        template=None,
-        html_template=None,
-        body=None,
-        reference=None,
-        reply_reference=None,
-        headers=None,
-        context=None,
-        send_to=None,
-        type=None,
-    ):
-        if not send_to:
-            logger.debug("Skipping message rendering, no users to send to.")
-            return
-
-        subject_prefix = self._build_subject_prefix(project)
-        subject = force_text(subject)
-
-        msg = MessageBuilder(
-            subject=f"{subject_prefix}{subject}",
-            template=template,
-            html_template=html_template,
-            body=body,
-            headers=headers,
-            type=type,
-            context=context,
-            reference=reference,
-            reply_reference=reply_reference,
-        )
-        msg.add_users(send_to, project=project)
-        return msg
-
-    def _send_mail(self, *args, **kwargs):
-        message = self._build_message(*args, **kwargs)
-        if message is not None:
-            return message.send_async()
-
     @staticmethod
     def get_sendable_user_objects(project):
         """
@@ -162,97 +111,19 @@ class MailAdapter:
             or self.get_sendable_user_objects(group.project)
         )
 
-    def add_unsubscribe_link(self, context, user_id, project, referrer):
-        context["unsubscribe_link"] = generate_signed_link(
-            user_id,
-            "sentry-account-email-unsubscribe-project",
-            referrer,
-            kwargs={"project_id": project.id},
-        )
-
-    def notify(self, notification, target_type, target_identifier=None, **kwargs):
+    @staticmethod
+    def notify(notification, target_type, target_identifier=None, **kwargs):
         AlertRuleNotification(notification, target_type, target_identifier).send()
 
-    def get_digest_subject(self, group, counts, date):
-        return "{short_id} - {count} new {noun} since {date}".format(
-            short_id=group.qualified_short_id,
-            count=len(counts),
-            noun="alert" if len(counts) == 1 else "alerts",
-            date=dateformat.format(date, "N j, Y, P e"),
-        )
-
+    @staticmethod
     def notify_digest(
-        self,
         project: Project,
         digest: Any,
         target_type: ActionTargetType,
         target_identifier: Optional[int] = None,
     ) -> None:
         metrics.incr("mail_adapter.notify_digest")
-
-        users = get_send_to(project, target_type, target_identifier).get(ExternalProviders.EMAIL)
-        if not users:
-            return
-        user_ids = {user.id for user in users}
-
-        logger.info(
-            "mail.adapter.notify_digest",
-            extra={
-                "project_id": project.id,
-                "target_type": target_type.value,
-                "target_identifier": target_identifier,
-                "user_ids": user_ids,
-            },
-        )
-        org = project.organization
-        for user_id, digest in get_personalized_digests(target_type, project.id, digest, user_ids):
-            start, end, counts = get_digest_metadata(digest)
-
-            # If there is only one group in this digest (regardless of how many
-            # rules it appears in), we should just render this using the single
-            # notification template. If there is more than one record for a group,
-            # just choose the most recent one.
-            if len(counts) == 1:
-                group = next(iter(counts))
-                record = max(
-                    itertools.chain.from_iterable(
-                        groups.get(group, []) for groups in digest.values()
-                    ),
-                    key=lambda record: record.timestamp,
-                )
-                notification = Notification(record.value.event, rules=record.value.rules)
-                return self.notify(notification, target_type, target_identifier)
-
-            context = {
-                "start": start,
-                "end": end,
-                "project": project,
-                "digest": digest,
-                "counts": counts,
-                "slack_link": get_integration_link(org, "slack"),
-                "has_alert_integration": has_alert_integration(project),
-            }
-
-            headers = {
-                "X-Sentry-Project": project.slug,
-                "X-SMTPAPI": json.dumps({"category": "digest_email"}),
-            }
-
-            group = next(iter(counts))
-            subject = self.get_digest_subject(group, counts, start)
-
-            self.add_unsubscribe_link(context, user_id, project, "alert_digest")
-            self._send_mail(
-                subject=subject,
-                template="sentry/emails/digests/body.txt",
-                html_template="sentry/emails/digests/body.html",
-                project=project,
-                reference=project,
-                headers=headers,
-                type="notify.digest",
-                context=context,
-                send_to=[user_id],
-            )
+        return DigestNotification(project, digest, target_type, target_identifier).send()
 
     @staticmethod
     def notify_about_activity(activity):
@@ -264,7 +135,8 @@ class MailAdapter:
 
         email_cls(activity).send()
 
-    def handle_user_report(self, payload, project: Project, **kwargs):
+    @staticmethod
+    def handle_user_report(payload, project: Project, **kwargs):
         metrics.incr("mail_adapter.handle_user_report")
         return UserReportNotification(project, payload["report"]).send()
 

src/sentry/mail/notifications.py

@@ -1,8 +1,10 @@
 import logging
 from typing import Any, Mapping, Optional, Set
 
+from django.utils.encoding import force_text
+
 from sentry import options
-from sentry.models import ProjectOption, User
+from sentry.models import Project, ProjectOption, User
 from sentry.notifications.notifications.activity.base import ActivityNotification
 from sentry.notifications.notifications.base import BaseNotification
 from sentry.notifications.notifications.rules import AlertRuleNotification
@@ -34,23 +36,30 @@ def get_headers(notification: BaseNotification) -> Mapping[str, Any]:
     return headers
 
 
+def build_subject_prefix(project: Project, mail_option_key: Optional[str] = None) -> str:
+    key = mail_option_key or "mail:subject_prefix"
+    return force_text(
+        ProjectOption.objects.get_value(project, key) or options.get("mail.subject-prefix")
+    )
+
+
 def get_subject_with_prefix(
     notification: BaseNotification,
     context: Optional[Mapping[str, Any]] = None,
     mail_option_key: Optional[str] = None,
 ) -> bytes:
-    key = mail_option_key or "mail:subject_prefix"
-    prefix = str(
-        ProjectOption.objects.get_value(notification.project, key)
-        or options.get("mail.subject-prefix")
-    )
+
+    prefix = build_subject_prefix(notification.project, mail_option_key)
     return f"{prefix}{notification.get_subject(context)}".encode("utf-8")
 
 
-def get_unsubscribe_link(user_id: int, resource_id: int, key: str = "issue") -> str:
+def get_unsubscribe_link(
+    user_id: int, resource_id: int, key: str = "issue", referrer: Optional[str] = None
+) -> str:
     return generate_signed_link(
         user_id,
         f"sentry-account-email-unsubscribe-{key}",
+        referrer,
         kwargs={f"{key}_id": resource_id},
     )
 
@@ -93,8 +102,10 @@ def get_context(
         **notification.get_user_context(user, extra_context),
     }
     if notification.get_unsubscribe_key():
-        key, resource_id = notification.get_unsubscribe_key()
-        context.update({"unsubscribe_link": get_unsubscribe_link(user.id, resource_id, key)})
+        key, resource_id, referrer = notification.get_unsubscribe_key()
+        context.update(
+            {"unsubscribe_link": get_unsubscribe_link(user.id, resource_id, key, referrer)}
+        )
 
     return context
 

src/sentry/notifications/notifications/activity/base.py

@@ -90,8 +90,8 @@ class GroupActivityNotification(ActivityNotification, ABC):
     def get_reply_reference(self) -> Optional[Any]:
         return self.group
 
-    def get_unsubscribe_key(self) -> Optional[Tuple[str, int]]:
-        return "issue", self.group.id
+    def get_unsubscribe_key(self) -> Optional[Tuple[str, int, Optional[str]]]:
+        return "issue", self.group.id, None
 
     def get_base_context(self) -> MutableMapping[str, Any]:
         return {

src/sentry/notifications/notifications/base.py

@@ -1,14 +1,16 @@
-from typing import Any, Mapping, MutableMapping, Optional, Tuple
+from typing import TYPE_CHECKING, Any, Mapping, MutableMapping, Optional, Tuple
 
-from sentry.models import Project, User
 from sentry.utils.http import absolute_uri
 
+if TYPE_CHECKING:
+    from sentry.models import Project, User
+
 
 class BaseNotification:
     fine_tuning_key: Optional[str] = None
     is_message_issue_unfurl = False
 
-    def __init__(self, project: Project) -> None:
+    def __init__(self, project: "Project") -> None:
         self.project = project
         self.organization = self.project.organization
 
@@ -41,9 +43,10 @@ class BaseNotification:
         return str(absolute_uri(f"/{self.organization.slug}/{self.project.slug}/"))
 
     def get_user_context(
-        self, user: User, extra_context: Mapping[str, Any]
+        self, user: "User", extra_context: Mapping[str, Any]
     ) -> MutableMapping[str, Any]:
-        return {}
+        # Basically a noop.
+        return {**extra_context}
 
     def get_notification_title(self) -> str:
         raise NotImplementedError
@@ -55,5 +58,5 @@ class BaseNotification:
     def get_type(self) -> str:
         raise NotImplementedError
 
-    def get_unsubscribe_key(self) -> Optional[Tuple[str, int]]:
+    def get_unsubscribe_key(self) -> Optional[Tuple[str, int, Optional[str]]]:
         return None

src/sentry/notifications/notifications/digest.py

@@ -0,0 +1,144 @@
+import logging
+from typing import TYPE_CHECKING, Any, Iterable, Mapping, MutableMapping, Optional, Set, Tuple
+
+from sentry.digests import Digest
+from sentry.digests.utilities import (
+    get_digest_metadata,
+    get_personalized_digests,
+    should_get_personalized_digests,
+)
+from sentry.notifications.notifications.base import BaseNotification
+from sentry.notifications.notify import notify
+from sentry.notifications.types import ActionTargetType
+from sentry.notifications.utils import get_integration_link, has_alert_integration
+from sentry.notifications.utils.digest import (
+    get_digest_subject,
+    send_as_alert_notification,
+    should_send_as_alert_notification,
+)
+from sentry.notifications.utils.participants import get_send_to
+from sentry.types.integrations import ExternalProviders
+
+if TYPE_CHECKING:
+    from sentry.models import Project, User
+
+
+logger = logging.getLogger(__name__)
+
+
+class DigestNotification(BaseNotification):
+    def __init__(
+        self,
+        project: "Project",
+        digest: Digest,
+        target_type: ActionTargetType,
+        target_identifier: Optional[int] = None,
+    ) -> None:
+        super().__init__(project)
+        self.digest = digest
+        self.target_type = target_type
+        self.target_identifier = target_identifier
+
+    def get_participants(self) -> Mapping[ExternalProviders, Set["User"]]:
+        data_by_provider = get_send_to(
+            project=self.project,
+            target_type=self.target_type,
+            target_identifier=self.target_identifier,
+        )
+        return {
+            provider: data
+            for provider, data in data_by_provider.items()
+            if provider in [ExternalProviders.EMAIL]
+        }
+
+    def get_filename(self) -> str:
+        return "digests/body"
+
+    def get_category(self) -> str:
+        return "digest_email"
+
+    def get_type(self) -> str:
+        return "notify.digest"
+
+    def get_unsubscribe_key(self) -> Optional[Tuple[str, int, Optional[str]]]:
+        return "project", self.project.id, "alert_digest"
+
+    def get_subject(self, context: Optional[Mapping[str, Any]] = None) -> str:
+        if not context:
+            # This shouldn't be possible but adding a message just in case.
+            return "Digest Report"
+        return get_digest_subject(context["group"], context["counts"], context["start"])
+
+    def get_notification_title(self) -> str:
+        # This shouldn't be possible but adding a message just in case.
+        return "Digest Report"
+
+    def get_reference(self) -> Any:
+        return self.project
+
+    def get_context(self) -> MutableMapping[str, Any]:
+        start, end, counts = get_digest_metadata(self.digest)
+        group = next(iter(counts))
+        return {
+            "counts": counts,
+            "digest": self.digest,
+            "end": end,
+            "group": group,
+            "has_alert_integration": has_alert_integration(self.project),
+            "project": self.project,
+            "slack_link": get_integration_link(self.organization, "slack"),
+            "start": start,
+        }
+
+    def get_extra_context(self, recipient_ids: Iterable[int]) -> Mapping[int, Mapping[str, Any]]:
+        extra_context = {}
+        for user_id, digest in get_personalized_digests(
+            self.target_type, self.project.id, self.digest, recipient_ids
+        ):
+            start, end, counts = get_digest_metadata(digest)
+            group = next(iter(counts))
+
+            extra_context[user_id] = {
+                "counts": counts,
+                "digest": digest,
+                "group": group,
+                "end": end,
+                "start": start,
+            }
+
+        return extra_context
+
+    def send(self) -> None:
+        if not self.should_email():
+            return
+
+        participants_by_provider = self.get_participants()
+        participants = participants_by_provider.get(ExternalProviders.EMAIL)
+        if not participants:
+            return
+
+        user_ids = {user.id for user in participants}
+        logger.info(
+            "mail.adapter.notify_digest",
+            extra={
+                "project_id": self.project.id,
+                "target_type": self.target_type.value,
+                "target_identifier": self.target_identifier,
+                "user_ids": user_ids,
+            },
+        )
+
+        # Only calculate shared context once.
+        shared_context = self.get_context()
+
+        if should_send_as_alert_notification(shared_context):
+            return send_as_alert_notification(
+                shared_context, self.target_type, self.target_identifier
+            )
+
+        # Calculate the per-user context.
+        extra_context: Mapping[int, Mapping[str, Any]] = {}
+        if should_get_personalized_digests(self.target_type, self.project.id):
+            extra_context = self.get_extra_context(user_ids)
+
+        notify(ExternalProviders.EMAIL, self, participants, shared_context, extra_context)

src/sentry/notifications/utils/digest.py

@@ -0,0 +1,53 @@
+import itertools
+from datetime import datetime
+from typing import TYPE_CHECKING, Any, Counter, Mapping, Optional
+
+from django.utils import dateformat
+
+from sentry.notifications.types import ActionTargetType
+from sentry.plugins.base import Notification
+
+if TYPE_CHECKING:
+    from sentry.models import Group
+
+
+def get_digest_subject(group: "Group", counts: Counter["Group"], date: datetime) -> str:
+    return "{short_id} - {count} new {noun} since {date}".format(
+        short_id=group.qualified_short_id,
+        count=len(counts),
+        noun="alert" if len(counts) == 1 else "alerts",
+        date=dateformat.format(date, "N j, Y, P e"),
+    )
+
+
+def should_send_as_alert_notification(context: Mapping[str, Any]) -> bool:
+    """
+    If there is only one group in this digest (regardless of how many rules it
+    appears in), then short-circuit and just render this using the single
+    notification template.
+    """
+    return len(context["counts"]) == 1
+
+
+def get_timestamp(record_param: Any) -> float:
+    # Explicitly typing to satisfy mypy.
+    time: float = record_param.timestamp
+    return time
+
+
+def send_as_alert_notification(
+    context: Mapping[str, Any],
+    target_type: ActionTargetType,
+    target_identifier: Optional[int] = None,
+) -> None:
+    """If there is more than one record for a group, just choose the most recent one."""
+    from sentry.mail import mail_adapter
+
+    record = max(
+        itertools.chain.from_iterable(
+            groups.get(context["group"], []) for groups in context["digest"].values()
+        ),
+        key=get_timestamp,
+    )
+    notification = Notification(record.value.event, rules=record.value.rules)
+    mail_adapter.notify(notification, target_type, target_identifier)

tests/sentry/mail/test_adapter.py

@@ -1,3 +1,4 @@
+from collections import Counter
 from datetime import datetime
 
 import pytz
@@ -11,7 +12,7 @@ from sentry.api.serializers import serialize
 from sentry.api.serializers.models.userreport import UserReportWithGroupSerializer
 from sentry.digests.notifications import build_digest, event_to_record
 from sentry.event_manager import EventManager, get_event_type
-from sentry.mail import mail_adapter, send_notification_as_email
+from sentry.mail import build_subject_prefix, mail_adapter, send_notification_as_email
 from sentry.mail.adapter import ActionTargetType
 from sentry.models import (
     Activity,
@@ -31,6 +32,7 @@ from sentry.models import (
 )
 from sentry.notifications.notifications.rules import AlertRuleNotification
 from sentry.notifications.types import NotificationSettingOptionValues, NotificationSettingTypes
+from sentry.notifications.utils.digest import get_digest_subject
 from sentry.notifications.utils.participants import (
     get_send_to,
     get_send_to_member,
@@ -218,37 +220,14 @@ class MailAdapterGetSendableUsersTest(BaseMailAdapterTest, TestCase):
 
 class MailAdapterBuildSubjectPrefixTest(BaseMailAdapterTest, TestCase):
     def test_default_prefix(self):
-        assert self.adapter._build_subject_prefix(self.project) == "[Sentry] "
+        assert build_subject_prefix(self.project) == "[Sentry] "
 
     def test_project_level_prefix(self):
         prefix = "[Example prefix] "
         ProjectOption.objects.set_value(
             project=self.project, key="mail:subject_prefix", value=prefix
         )
-        assert self.adapter._build_subject_prefix(self.project) == prefix
-
-
-class MailAdapterBuildMessageTest(BaseMailAdapterTest, TestCase):
-    def test(self):
-        subject = "hello"
-        assert self.adapter._build_message(self.project, subject) is None
-
-    def test_specify_send_to(self):
-        subject = "hello"
-        send_to_user = self.create_user("hello@timecube.com")
-        msg = self.adapter._build_message(self.project, subject, send_to=[send_to_user.id])
-        assert msg._send_to == {send_to_user.email}
-        assert msg.subject.endswith(subject)
-
-
-class MailAdapterSendMailTest(BaseMailAdapterTest, TestCase):
-    def test(self):
-        subject = "hello"
-        with self.tasks():
-            self.adapter._send_mail(self.project, subject, body="hi", send_to=[self.user.id])
-            msg = mail.outbox[0]
-            assert msg.subject.endswith(subject)
-            assert msg.recipients() == [self.user.email]
+        assert build_subject_prefix(self.project) == prefix
 
 
 class MailAdapterNotifyTest(BaseMailAdapterTest, TestCase):
@@ -584,9 +563,9 @@ class MailAdapterNotifyTest(BaseMailAdapterTest, TestCase):
 class MailAdapterGetDigestSubjectTest(BaseMailAdapterTest, TestCase):
     def test_get_digest_subject(self):
         assert (
-            self.adapter.get_digest_subject(
+            get_digest_subject(
                 mock.Mock(qualified_short_id="BAR-1"),
-                {mock.sentinel.group: 3},
+                Counter({mock.sentinel.group: 3}),
                 datetime(2016, 9, 19, 1, 2, 3, tzinfo=pytz.utc),
             )
             == "BAR-1 - 1 new alert since Sept. 19, 2016, 1:02 a.m. UTC"