feat(notifications): Move Email Digest to `send_notification_as_email` (#25781)

src/sentry/mail/adapter.py

@@ -5,16 +5,13 @@ 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.digests import get_option_key as get_digest_option_key
-from sentry.digests.notifications import event_to_record, unsplit_key
+from sentry import options
 from sentry.digests.utilities import get_digest_metadata, get_personalized_digests
 from sentry.models import Group, GroupSubscription, NotificationSetting, Project, ProjectOption
 from sentry.notifications.activity import EMAIL_CLASSES_BY_TYPE
 from sentry.notifications.rules import AlertRuleNotification, get_send_to
 from sentry.notifications.types import ActionTargetType, GroupSubscriptionReason
 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
@@ -38,49 +35,15 @@ class MailAdapter:
     ) -> None:
         metrics.incr("mail_adapter.rule_notify")
         rules = []
-        extra = {
-            "event_id": event.event_id,
-            "group_id": event.group_id,
-            "is_from_mail_action_adapter": True,
-            "target_type": target_type.value,
-            "target_identifier": target_identifier,
-        }
-        log_event = "dispatched"
         for future in futures:
             rules.append(future.rule)
-            extra["rule_id"] = future.rule.id
-            if not future.kwargs:
-                continue
-            raise NotImplementedError(
-                "The default behavior for notification de-duplication does not support args"
-            )
-
-        project = event.group.project
-        extra["project_id"] = project.id
-
-        if digests.enabled(project):
-
-            def get_digest_option(key):
-                return ProjectOption.objects.get_value(project, get_digest_option_key("mail", key))
-
-            digest_key = unsplit_key(event.group.project, target_type, target_identifier)
-            extra["digest_key"] = digest_key
-            immediate_delivery = digests.add(
-                digest_key,
-                event_to_record(event, rules),
-                increment_delay=get_digest_option("increment_delay"),
-                maximum_delay=get_digest_option("maximum_delay"),
-            )
-            if immediate_delivery:
-                deliver_digest.delay(digest_key)
-            else:
-                log_event = "digested"
-
-        else:
-            notification = Notification(event=event, rules=rules)
-            self.notify(notification, target_type, target_identifier)
+            if future.kwargs:
+                raise NotImplementedError(
+                    "The default behavior for notification de-duplication does not support args"
+                )
 
-        logger.info("mail.adapter.notification.%s" % log_event, extra=extra)
+        notification = Notification(event=event, rules=rules)
+        self.notify(notification, target_type, target_identifier)
 
     def _build_subject_prefix(self, project):
         subject_prefix = ProjectOption.objects.get_value(project, self.mail_option_key, None)

src/sentry/mail/notifications.py

@@ -1,12 +1,15 @@
 import logging
 from typing import Any, Mapping, Optional, Set
 
-from sentry import options
+from sentry import digests, options
+from sentry.digests import get_option_key as get_digest_option_key
+from sentry.digests.notifications import event_to_record, unsplit_key
 from sentry.models import ProjectOption, User
 from sentry.notifications.activity.base import ActivityNotification
 from sentry.notifications.base import BaseNotification
 from sentry.notifications.notify import register_notification_provider
 from sentry.notifications.rules import AlertRuleNotification
+from sentry.tasks.digests import deliver_digest
 from sentry.types.integrations import ExternalProviders
 from sentry.utils import json
 from sentry.utils.email import MessageBuilder, group_id_to_email
@@ -70,7 +73,7 @@ def log_message(notification: BaseNotification, user: User) -> None:
         "user_id": user.id,
     }
     if notification.group:
-        extra.update({"group": notification.group.id})
+        extra.update({"group_id": notification.group.id})
 
     if isinstance(notification, AlertRuleNotification):
         extra.update(
@@ -79,6 +82,9 @@ def log_message(notification: BaseNotification, user: User) -> None:
                 "target_identifier": notification.target_identifier,
             }
         )
+        if len(notification.rules):
+            extra.update({"rule_id": notification.rules[0].id})
+
     elif isinstance(notification, ActivityNotification):
         extra.update({"activity": notification.activity})
 
@@ -106,6 +112,41 @@ def get_context(
     return context
 
 
+def get_digest_option(notification: AlertRuleNotification, key: str) -> str:
+    return ProjectOption.objects.get_value(notification.project, get_digest_option_key("mail", key))
+
+
+def get_digest_key(notification: AlertRuleNotification) -> str:
+    return unsplit_key(
+        notification.project, notification.target_type, notification.target_identifier
+    )
+
+
+def add_to_digest(notification: AlertRuleNotification) -> None:
+    digest_key = get_digest_key(notification)
+    immediate_delivery = digests.add(
+        digest_key,
+        event_to_record(notification.event, notification.rules),
+        increment_delay=get_digest_option(notification, "increment_delay"),
+        maximum_delay=get_digest_option(notification, "maximum_delay"),
+    )
+    if immediate_delivery:
+        deliver_digest.delay(digest_key)
+
+    action_key = "dispatched" if immediate_delivery else "digested"
+    logger.info(
+        f"mail.adapter.notification.{action_key}",
+        extra={
+            "group": notification.group.id,
+            "project_id": notification.project.id,
+            "is_from_mail_action_adapter": True,
+            "target_type": notification.target_type.value,
+            "target_identifier": notification.target_identifier,
+            "rule_id": notification.rules[0].id,
+        },
+    )
+
+
 @register_notification_provider(ExternalProviders.EMAIL)
 def send_notification_as_email(
     notification: BaseNotification,
@@ -113,6 +154,9 @@ def send_notification_as_email(
     shared_context: Mapping[str, Any],
     extra_context_by_user_id: Optional[Mapping[int, Mapping[str, Any]]],
 ) -> None:
+    if isinstance(notification, AlertRuleNotification) and digests.enabled(notification.project):
+        return add_to_digest(notification)
+
     headers = get_headers(notification)
     subject = get_subject_with_prefix(notification)
     type = get_email_type(notification)

src/sentry/notifications/helpers.py

@@ -1,5 +1,5 @@
 from collections import defaultdict
-from typing import Any, Dict, Iterable, List, Mapping, MutableMapping, Optional, Set, Tuple, Union
+from typing import Any, Dict, Iterable, List, Mapping, Optional, Set, Tuple, Union
 
 from sentry.notifications.types import (
     NOTIFICATION_SETTING_DEFAULTS,
@@ -305,21 +305,3 @@ def get_user_subscriptions_for_groups(
             results[group.id] = (is_disabled, is_active, subscription)
 
     return results
-
-
-def get_settings_by_provider(
-    settings: Mapping[
-        NotificationScopeType, Mapping[ExternalProviders, NotificationSettingOptionValues]
-    ]
-) -> MutableMapping[
-    ExternalProviders, MutableMapping[NotificationScopeType, NotificationSettingOptionValues]
-]:
-    output: MutableMapping[
-        ExternalProviders, MutableMapping[NotificationScopeType, NotificationSettingOptionValues]
-    ] = defaultdict(dict)
-
-    for scope_type in settings:
-        for provider, value in settings[scope_type].items():
-            output[provider][scope_type] = value
-
-    return output

src/sentry/notifications/rules.py

@@ -14,7 +14,6 @@ from sentry.notifications.utils import (
 from sentry.notifications.utils.participants import get_send_to
 from sentry.plugins.base.structs import Notification
 from sentry.types.integrations import ExternalProviders
-from sentry.utils import metrics
 
 logger = logging.getLogger(__name__)
 
@@ -80,17 +79,6 @@ class AlertRuleNotification(BaseNotification):
     def send(self) -> None:
         from sentry.notifications.notify import notify
 
-        metrics.incr("mail_adapter.notify")
-        logger.info(
-            "mail.adapter.notify",
-            extra={
-                "target_type": self.target_type.value,
-                "target_identifier": self.target_identifier,
-                "group": self.group.id,
-                "project_id": self.project.id,
-            },
-        )
-
         participants_by_provider = self.get_participants()
         if not participants_by_provider:
             return

src/sentry/notifications/utils/participants.py

@@ -15,14 +15,12 @@ from sentry.models import (
 )
 from sentry.notifications.helpers import (
     get_deploy_values_by_provider,
-    get_settings_by_provider,
     transform_to_notification_settings_by_user,
 )
 from sentry.notifications.notify import notification_providers
 from sentry.notifications.types import (
     ActionTargetType,
     GroupSubscriptionReason,
-    NotificationScopeType,
     NotificationSettingOptionValues,
     NotificationSettingTypes,
 )
@@ -221,34 +219,6 @@ def get_users_for_teams_to_resolve(teams_to_resolve: Set[int]) -> Set[User]:
     )
 
 
-def disabled_users_from_project(project: Project) -> Mapping[ExternalProviders, Set[User]]:
-    """ Get a set of users that have disabled Issue Alert notifications for a given project. """
-    user_ids = project.member_set.values_list("user", flat=True)
-    users = User.objects.filter(id__in=user_ids)
-    notification_settings = NotificationSetting.objects.get_for_users_by_parent(
-        type=NotificationSettingTypes.ISSUE_ALERTS,
-        parent=project,
-        users=users,
-    )
-    notification_settings_by_user = transform_to_notification_settings_by_user(
-        notification_settings, users
-    )
-    # Although this can be done with dict comprehension, looping for clarity.
-    output = defaultdict(set)
-    for user in users:
-        settings = notification_settings_by_user.get(user)
-        if settings:
-            settings_by_provider = get_settings_by_provider(settings)
-            for provider, settings_value_by_scope in settings_by_provider.items():
-                project_setting = settings_value_by_scope.get(NotificationScopeType.PROJECT)
-                user_setting = settings_value_by_scope.get(NotificationScopeType.USER)
-                if project_setting == NotificationSettingOptionValues.NEVER or (
-                    not project_setting and user_setting == NotificationSettingOptionValues.NEVER
-                ):
-                    output[provider].add(user)
-    return output
-
-
 def get_send_to_team(
     project: Project, target_identifier: Optional[Union[str, int]]
 ) -> Mapping[ExternalProviders, Set[User]]:

tests/sentry/integrations/slack/test_notifications.py

@@ -1,6 +1,5 @@
 from urllib.parse import parse_qs
 
-import pytest
 import responses
 from django.utils import timezone
 from exam import fixture
@@ -470,10 +469,9 @@ class SlackActivityNotificationTest(ActivityTestCase, TestCase):
         assert attachments[0]["text"] == ""
         assert attachments[0]["footer"] == event.group.qualified_short_id
 
-    @pytest.mark.skip(reason="will be needed soon but not yet")
     @responses.activate
     @mock.patch("sentry.notifications.notify.notify", side_effect=send_notification)
-    @mock.patch("sentry.mail.adapter.digests")
+    @mock.patch("sentry.mail.notifications.digests")
     def test_digest_enabled(self, digests, mock_func):
         """
         Test that with digests enabled, but Slack notification settings

tests/sentry/mail/test_adapter.py

@@ -611,7 +611,7 @@ class MailAdapterRuleNotifyTest(BaseMailAdapterTest, TestCase):
             self.adapter.rule_notify(event, futures, ActionTargetType.ISSUE_OWNERS)
             assert notify.call_count == 1
 
-    @mock.patch("sentry.mail.adapter.digests")
+    @mock.patch("sentry.mail.notifications.digests")
     def test_digest(self, digests):
         digests.enabled.return_value = True
 

tests/sentry/notifications/test_utils.py

@@ -6,7 +6,6 @@ from sentry.notifications.helpers import (
     get_groups_for_query,
     get_scope,
     get_scope_type,
-    get_settings_by_provider,
     get_subscription_from_attributes,
     get_target_id,
     get_user_subscriptions_for_groups,
@@ -29,7 +28,7 @@ from sentry.types.integrations import ExternalProviders
 
 class NotificationHelpersTest(TestCase):
     def setUp(self):
-        super(TestCase, self).setUp()
+        super().setUp()
 
         NotificationSetting.objects.update_settings(
             ExternalProviders.SLACK,
@@ -319,15 +318,3 @@ class NotificationHelpersTest(TestCase):
             global_default_workflow_option,
         )
         assert subscriptions == {self.group.id: (False, True, None)}
-
-    def test_get_settings_by_provider(self):
-        settings = {
-            NotificationScopeType.USER: {
-                ExternalProviders.EMAIL: NotificationSettingOptionValues.NEVER
-            }
-        }
-        assert get_settings_by_provider(settings) == {
-            ExternalProviders.EMAIL: {
-                NotificationScopeType.USER: NotificationSettingOptionValues.NEVER
-            }
-        }