fix: Respect user notification settings when sending notifications related to issue owners. (SEN-411)

Whenever we send a notification for an event we check whether there is ownership information for the
project, and whether it relates to the event. If it does, then we go down a different path to decide
which users to send notifications to. This path doesn't check the user's notification preferences
for the project.

This looks like it has been broken ever since we first implemented issue owners. We've heard more
reports of it recently, possibly we actually configured issue ownership for various projects in our
org.

Fix the issue by checking the user's alert settings and filtering out users that have alerts
disabled for the given project.

src/sentry/models/project.py

@@ -257,16 +257,22 @@ class Project(Model, PendingDeletionMixin):
     def get_full_name(self):
         return self.slug
 
-    def get_notification_recipients(self, user_option):
+    def get_member_alert_settings(self, user_option):
+        """
+        Returns a list of users who have alert notifications explicitly
+        enabled/disabled.
+        :param user_option: alert option key, typically 'mail:alert'
+        :return: A dictionary in format {<user_id>: <int_alert_value>}
+        """
         from sentry.models import UserOption
-        alert_settings = dict(
-            (o.user_id, int(o.value))
-            for o in UserOption.objects.filter(
-                project=self,
-                key=user_option,
-            )
-        )
+        return {o.user_id: int(o.value) for o in UserOption.objects.filter(
+            project=self,
+            key=user_option,
+        )}
 
+    def get_notification_recipients(self, user_option):
+        from sentry.models import UserOption
+        alert_settings = self.get_member_alert_settings(user_option)
         disabled = set(u for u, v in six.iteritems(alert_settings) if v == 0)
 
         member_set = set(

src/sentry/plugins/bases/notify.py

@@ -136,13 +136,16 @@ class NotificationPlugin(Plugin):
     def notify_about_activity(self, activity):
         pass
 
+    @property
+    def alert_option_key(self):
+        return '%s:alert' % self.get_conf_key()
+
     def get_sendable_users(self, project):
         """
         Return a collection of user IDs that are eligible to receive
         notifications for the provided project.
         """
-        user_option = '%s:alert' % self.get_conf_key()
-        return project.get_notification_recipients(user_option)
+        return project.get_notification_recipients(self.alert_option_key)
 
     def __is_rate_limited(self, group, event):
         return ratelimits.is_limited(

src/sentry/plugins/sentry_mail/models.py

@@ -148,21 +148,26 @@ class MailPlugin(NotificationPlugin):
                     },
                     skip_internal=True,
                 )
-                send_to_list = []
-                teams_to_resolve = []
+                send_to_list = set()
+                teams_to_resolve = set()
                 for owner in owners:
                     if owner.type == User:
-                        send_to_list.append(owner.id)
+                        send_to_list.add(owner.id)
                     else:
-                        teams_to_resolve.append(owner.id)
+                        teams_to_resolve.add(owner.id)
 
                 # get all users in teams
                 if teams_to_resolve:
-                    send_to_list += User.objects.filter(
+                    send_to_list |= set(User.objects.filter(
                         is_active=True,
                         sentry_orgmember_set__organizationmemberteam__team__id__in=teams_to_resolve,
-                    ).values_list('id', flat=True)
-                return send_to_list
+                    ).values_list('id', flat=True))
+
+                alert_settings = project.get_member_alert_settings(self.alert_option_key)
+                disabled_users = set(
+                    user for user, setting in alert_settings.items() if setting == 0
+                )
+                return send_to_list - disabled_users
             else:
                 metrics.incr(
                     'features.owners.send_to',

tests/sentry/plugins/mail/tests.py

@@ -64,7 +64,9 @@ class MailPluginTest(TestCase):
     @mock.patch('sentry.interfaces.stacktrace.Stacktrace.get_title')
     @mock.patch('sentry.interfaces.stacktrace.Stacktrace.to_email_html')
     @mock.patch('sentry.plugins.sentry_mail.models.MailPlugin._send_mail')
-    def test_notify_users_renders_interfaces_with_utf8(self, _send_mail, _to_email_html, _get_title):
+    def test_notify_users_renders_interfaces_with_utf8(
+        self, _send_mail, _to_email_html, _get_title,
+    ):
         group = self.create_group(
             first_seen=timezone.now(),
             last_seen=timezone.now(),
@@ -603,6 +605,12 @@ class MailPluginOwnersTest(TestCase):
         assert (sorted(set([self.user.pk, self.user2.pk])) == sorted(
             self.plugin.get_send_to(self.project, event.data)))
 
+        # Make sure that disabling mail alerts works as expected
+        UserOption.objects.set_value(
+            user=self.user2, key='mail:alert', value=0, project=self.project
+        )
+        assert set([self.user.pk]) == self.plugin.get_send_to(self.project, event.data)
+
     def test_get_send_to_with_user_owners(self):
         event = Event(
             group=self.group,
@@ -614,6 +622,12 @@ class MailPluginOwnersTest(TestCase):
         assert (sorted(set([self.user.pk, self.user2.pk])) == sorted(
             self.plugin.get_send_to(self.project, event.data)))
 
+        # Make sure that disabling mail alerts works as expected
+        UserOption.objects.set_value(
+            user=self.user2, key='mail:alert', value=0, project=self.project
+        )
+        assert set([self.user.pk]) == self.plugin.get_send_to(self.project, event.data)
+
     def test_get_send_to_with_user_owner(self):
         event = Event(
             group=self.group,
@@ -622,8 +636,7 @@ class MailPluginOwnersTest(TestCase):
             datetime=self.group.last_seen,
             data=self.make_event_data('foo.jx')
         )
-        assert (sorted(set([self.user2.pk])) == sorted(
-            self.plugin.get_send_to(self.project, event.data)))
+        assert set([self.user2.pk]) == self.plugin.get_send_to(self.project, event.data)
 
     def test_get_send_to_with_fallthrough(self):
         event = Event(
@@ -633,8 +646,7 @@ class MailPluginOwnersTest(TestCase):
             datetime=self.group.last_seen,
             data=self.make_event_data('foo.jx')
         )
-        assert (sorted(set([self.user2.pk])) == sorted(
-            self.plugin.get_send_to(self.project, event.data)))
+        assert set([self.user2.pk]) == self.plugin.get_send_to(self.project, event.data)
 
     def test_get_send_to_without_fallthrough(self):
         ProjectOwnership.objects.get(project_id=self.project.id).update(fallthrough=False)
@@ -645,7 +657,7 @@ class MailPluginOwnersTest(TestCase):
             datetime=self.group.last_seen,
             data=self.make_event_data('foo.cpp')
         )
-        assert [] == sorted(self.plugin.get_send_to(self.project, event.data))
+        assert [] == self.plugin.get_send_to(self.project, event.data)
 
     def test_notify_users_with_owners(self):
         event_all_users = Event(
@@ -674,3 +686,16 @@ class MailPluginOwnersTest(TestCase):
             data=self.make_event_data('foo.jx'),
         )
         self.assert_notify(event_single_user, [self.user2.email])
+
+        # Make sure that disabling mail alerts works as expected
+        UserOption.objects.set_value(
+            user=self.user2, key='mail:alert', value=0, project=self.project
+        )
+        event_all_users = Event(
+            group=self.group,
+            message=self.group.message,
+            project=self.project,
+            datetime=self.group.last_seen,
+            data=self.make_event_data('foo.cbl'),
+        )
+        self.assert_notify(event_all_users, [self.user.email])