Browse Source

fix(notifications platform): Account for empty get_send_to result (#25603)

* fix(notifications platform): Account for empty get_send_to result
Colleen O'Rourke 3 years ago
parent
commit
e81aec5c81
2 changed files with 51 additions and 9 deletions
  1. 12 9
      src/sentry/mail/adapter.py
  2. 39 0
      tests/sentry/mail/test_adapter.py

+ 12 - 9
src/sentry/mail/adapter.py

@@ -285,10 +285,12 @@ class MailAdapter:
             team = Team.objects.get(id=int(target_identifier), projectteam__project=project)
         except Team.DoesNotExist:
             return set()
-        return (
-            set(team.member_set.values_list("user_id", flat=True))
-            - self.disabled_users_from_project(project)[ExternalProviders.EMAIL]
-        )
+
+        disabled_users = self.disabled_users_from_project(project).get(ExternalProviders.EMAIL)
+        if disabled_users:
+            return set(team.member_set.values_list("user_id", flat=True)) - disabled_users
+        else:
+            return set(team.member_set.values_list("user_id", flat=True))
 
     def get_send_to_member(self, project, target_identifier):
         """
@@ -474,9 +476,10 @@ class MailAdapter:
 
     def notify_digest(self, project, digest, target_type, target_identifier=None):
         metrics.incr("mail_adapter.notify_digest")
-        user_ids = self.get_send_to(project, target_type, target_identifier)[
+        user_ids = self.get_send_to(project, target_type, target_identifier).get(
             ExternalProviders.EMAIL
-        ]
+        )
+
         logger.info(
             "mail.adapter.notify_digest",
             extra={
@@ -547,9 +550,9 @@ class MailAdapter:
         metrics.incr("mail_adapter.handle_user_report")
         group = Group.objects.get(id=payload["report"]["issue"]["id"])
 
-        participants = GroupSubscription.objects.get_participants(group=group)
-        if participants:
-            participants = participants[ExternalProviders.EMAIL]
+        participants = GroupSubscription.objects.get_participants(group=group).get(
+            ExternalProviders.EMAIL
+        )
 
         if not participants:
             return

+ 39 - 0
tests/sentry/mail/test_adapter.py

@@ -554,6 +554,45 @@ class MailAdapterNotifyDigestTest(BaseMailAdapterTest, TestCase):
 
         assert msg.subject.startswith("[Example prefix]")
 
+    @mock.patch.object(mail_adapter, "notify", side_effect=mail_adapter.notify, autospec=True)
+    def test_notify_digest_user_does_not_exist(self, notify):
+        """Test that in the event a rule has been created with an action to send to a user who
+        no longer exists, we don't blow up when getting users in get_send_to
+        """
+        project = self.project
+        event = self.store_event(
+            data={"timestamp": iso_format(before_now(minutes=1)), "fingerprint": ["group-1"]},
+            project_id=project.id,
+        )
+        event2 = self.store_event(
+            data={"timestamp": iso_format(before_now(minutes=1)), "fingerprint": ["group-2"]},
+            project_id=project.id,
+        )
+
+        action_data = {
+            "id": "sentry.mail.actions.NotifyEmailAction",
+            "targetType": "Member",
+            "targetIdentifier": str(444),
+        }
+        rule = Rule.objects.create(
+            project=self.project,
+            label="a rule",
+            data={
+                "match": "all",
+                "actions": [action_data],
+            },
+        )
+
+        digest = build_digest(
+            project, (event_to_record(event, (rule,)), event_to_record(event2, (rule,)))
+        )
+
+        with self.tasks():
+            self.adapter.notify_digest(project, digest, ActionTargetType.MEMBER, 444)
+
+        assert notify.call_count == 0
+        assert len(mail.outbox) == 0
+
 
 class MailAdapterRuleNotifyTest(BaseMailAdapterTest, TestCase):
     def test_normal(self):