fix(alerts): checks if group is unresolved before triggering alerts (#16715)

src/sentry/integrations/pagerduty/notify_action.py

@@ -80,7 +80,7 @@ class PagerDutyNotifyServiceAction(EventAction):
         return self.get_integrations().exists()
 
     def after(self, event, state):
-        if event.group.is_ignored():
+        if not event.group.is_unresolved():
             return
 
         integration_id = self.get_option("account")

src/sentry/integrations/slack/notify_action.py

@@ -79,7 +79,7 @@ class SlackNotifyServiceAction(EventAction):
         return self.get_integrations().exists()
 
     def after(self, event, state):
-        if event.group.is_ignored():
+        if not event.group.is_unresolved():
             return
 
         integration_id = self.get_option("workspace")

src/sentry/models/group.py

@@ -337,6 +337,9 @@ class Group(Model):
     def is_ignored(self):
         return self.get_status() == GroupStatus.IGNORED
 
+    def is_unresolved(self):
+        return self.get_status() == GroupStatus.UNRESOLVED
+
     # TODO(dcramer): remove in 9.0 / after plugins no long ref
     is_muted = is_ignored
 

src/sentry/plugins/base/notifier.py

@@ -17,7 +17,7 @@ class Notifier(object):
         """
 
     def should_notify(self, group, event):
-        if group.is_ignored():
+        if not group.is_unresolved():
             return False
 
         project = group.project

src/sentry/plugins/bases/notify.py

@@ -141,7 +141,7 @@ class NotificationPlugin(Plugin):
         if not self.is_configured(project=project):
             return False
 
-        if group.is_ignored():
+        if not group.is_unresolved():
             return False
 
         # If the plugin doesn't support digests or they are not enabled,

tests/sentry/integrations/pagerduty/test_notify_action.py

@@ -188,3 +188,14 @@ class PagerDutyNotifyActionTest(RuleTestCase):
 
         results = list(rule.after(event=event, state=self.get_state()))
         assert len(results) == 0
+
+    # this happens if an issue is marked as resolved in the next release
+    def test_dont_notify_resolved(self):
+        event = self.get_event()
+        event.group.status = GroupStatus.RESOLVED
+        event.group.save()
+
+        rule = self.get_rule(data={"account": self.integration.id, "service": self.service.id})
+
+        results = list(rule.after(event=event, state=self.get_state()))
+        assert len(results) == 0

tests/sentry/integrations/slack/test_notify_action.py

@@ -241,3 +241,14 @@ class SlackNotifyActionTest(RuleTestCase):
 
         results = list(rule.after(event=event, state=self.get_state()))
         assert len(results) == 0
+
+    # this happens if an issue is marked as resolved in the next release
+    def test_dont_notify_resolved(self):
+        event = self.get_event()
+        event.group.status = GroupStatus.RESOLVED
+        event.group.save()
+
+        rule = self.get_rule(data={"workspace": self.integration.id, "channel": "#my-channel"})
+
+        results = list(rule.after(event=event, state=self.get_state()))
+        assert len(results) == 0

tests/sentry/plugins/bases/notify/tests.py

@@ -6,6 +6,12 @@ from sentry.plugins.bases.notify import NotificationPlugin
 from sentry.plugins.base.structs import Notification
 from sentry.testutils import TestCase
 from requests.exceptions import HTTPError, SSLError
+from sentry.models import GroupStatus
+
+
+class DummyNotificationPlugin(NotificationPlugin):
+    def is_configured(self, project):
+        return True
 
 
 class NotifyPlugin(TestCase):
@@ -47,3 +53,23 @@ class NotifyPlugin(TestCase):
 
             n.notify_users = hook
             assert n.notify(notification) is False
+
+
+class DummyNotificationPluginTest(TestCase):
+    def setUp(self):
+        self.event = self.store_event(data={}, project_id=self.project.id)
+        self.group = self.event.group
+        self.plugin = DummyNotificationPlugin()
+
+    def test_should_notify(self):
+        assert self.plugin.should_notify(self.group, self.event)
+
+    def test_dont_notify_ignored(self):
+        self.group.status = GroupStatus.IGNORED
+        self.group.save()
+        assert not self.plugin.should_notify(self.group, self.event)
+
+    def test_dont_notify_resolved(self):
+        self.group.status = GroupStatus.RESOLVED
+        self.group.save()
+        assert not self.plugin.should_notify(self.group, self.event)