fix(notifications): Re-enable sending Slacks in notify_digest() (#27969)

src/sentry/notifications/notifications/digest.py

@@ -40,16 +40,11 @@ class DigestNotification(BaseNotification):
         self.target_identifier = target_identifier
 
     def get_participants(self) -> Mapping[ExternalProviders, Set["User"]]:
-        data_by_provider = get_send_to(
+        return 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"
@@ -113,11 +108,12 @@ class DigestNotification(BaseNotification):
             return
 
         participants_by_provider = self.get_participants()
-        participants = participants_by_provider.get(ExternalProviders.EMAIL)
-        if not participants:
+        if not participants_by_provider:
             return
 
-        user_ids = {user.id for user in participants}
+        # Get every user ID for every provider as a set.
+        user_ids = {user.id for users in participants_by_provider.values() for user in users}
+
         logger.info(
             "mail.adapter.notify_digest",
             extra={
@@ -136,9 +132,12 @@ class DigestNotification(BaseNotification):
                 shared_context, self.target_type, self.target_identifier
             )
 
-        # Calculate the per-user context.
+        # Calculate the per-user context. It's fine that we're doing extra work
+        # to get personalized digests for the non-email users.
         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)
+        for provider, participants in participants_by_provider.items():
+            if provider in [ExternalProviders.EMAIL]:
+                notify(provider, self, participants, shared_context, extra_context)

tests/sentry/integrations/slack/test_notifications.py

@@ -1,10 +1,12 @@
 from urllib.parse import parse_qs
 
-import pytest
 import responses
 from django.utils import timezone
 from exam import fixture
 
+import sentry
+from sentry.digests.backends.redis import RedisBackend
+from sentry.digests.notifications import event_to_record
 from sentry.integrations.slack.notifications import send_notification_as_slack
 from sentry.mail import mail_adapter
 from sentry.models import (
@@ -37,12 +39,13 @@ from sentry.notifications.types import (
     NotificationSettingTypes,
 )
 from sentry.plugins.base import Notification
-from sentry.rules.processor import RuleFuture
+from sentry.tasks.digests import deliver_digest
 from sentry.testutils import TestCase
 from sentry.types.activity import ActivityType
 from sentry.types.integrations import ExternalProviders
 from sentry.utils import json
 from sentry.utils.compat import mock
+from sentry.utils.compat.mock import patch
 from tests.sentry.mail.activity import ActivityTestCase
 
 
@@ -799,23 +802,27 @@ class SlackActivityNotificationTest(ActivityTestCase, TestCase):
             == f"{self.project.slug} | <http://example.com/settings/account/notifications/alerts/?referrer=AlertRuleSlack|Notification Settings>"
         )
 
-    @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")
+    @patch.object(sentry, "digests")
     def test_digest_enabled(self, digests, mock_func):
         """
         Test that with digests enabled, but Slack notification settings
         (and not email settings), we send a Slack notification
         """
+        backend = RedisBackend()
+        digests.digest = backend.digest
         digests.enabled.return_value = True
+
+        rule = Rule.objects.create(project=self.project, label="my rule")
         event = self.store_event(
             data={"message": "Hello world", "level": "error"}, project_id=self.project.id
         )
-        rule = Rule.objects.create(project=self.project, label="my rule")
+        key = f"mail:p:{self.project.id}"
+        backend.add(key, event_to_record(event, [rule]), increment_delay=0, maximum_delay=0)
 
-        futures = [RuleFuture(rule, {})]
-        self.adapter.rule_notify(event, futures, ActionTargetType.MEMBER, self.user.id)
+        with self.tasks():
+            deliver_digest(key)
 
         assert digests.call_count == 0
 
@@ -823,4 +830,3 @@ class SlackActivityNotificationTest(ActivityTestCase, TestCase):
 
         assert attachment["title"] == "Hello world"
         assert attachment["text"] == ""
-        assert attachment["footer"] == event.group.qualified_short_id