event_queue: Move notification trigger logic to notification_data.

This removes some complexity from the event_queue module.
To avoid code duplication, we reduce the `is_notifiable` methods to
internally just call the `trigger` methods and check their return value.

zerver/lib/notification_data.py

@@ -1,5 +1,5 @@
 from dataclasses import dataclass
-from typing import Collection, Set
+from typing import Collection, Optional, Set
 
 
 @dataclass
@@ -54,35 +54,53 @@ class UserMessageNotificationsData:
         ) or self.is_push_notifiable(private_message, sender_id, idle)
 
     def is_push_notifiable(self, private_message: bool, sender_id: int, idle: bool) -> bool:
+        return self.get_push_notification_trigger(private_message, sender_id, idle) is not None
+
+    def get_push_notification_trigger(
+        self, private_message: bool, sender_id: int, idle: bool
+    ) -> Optional[str]:
         if not idle and not self.online_push_enabled:
-            return False
+            return None
 
         if self.user_id == sender_id:
-            return False
+            return None
 
         if self.sender_is_muted:
-            return False
+            return None
 
-        return (
+        if private_message:
-            private_message
+            return "private_message"
-            or self.mentioned
+        elif self.mentioned:
-            or self.wildcard_mention_notify
+            return "mentioned"
-            or self.stream_push_notify
+        elif self.wildcard_mention_notify:
-        )
+            return "wildcard_mentioned"
+        elif self.stream_push_notify:
+            return "stream_push_notify"
+        else:
+            return None
 
     def is_email_notifiable(self, private_message: bool, sender_id: int, idle: bool) -> bool:
+        return self.get_email_notification_trigger(private_message, sender_id, idle) is not None
+
+    def get_email_notification_trigger(
+        self, private_message: bool, sender_id: int, idle: bool
+    ) -> Optional[str]:
         if not idle:
-            return False
+            return None
 
         if self.user_id == sender_id:
-            return False
+            return None
 
         if self.sender_is_muted:
-            return False
+            return None
 
-        return (
+        if private_message:
-            private_message
+            return "private_message"
-            or self.mentioned
+        elif self.mentioned:
-            or self.wildcard_mention_notify
+            return "mentioned"
-            or self.stream_email_notify
+        elif self.wildcard_mention_notify:
-        )
+            return "wildcard_mentioned"
+        elif self.stream_email_notify:
+            return "stream_email_notify"
+        else:
+            return None

zerver/tests/test_notification_data.py

@@ -8,33 +8,66 @@ class TestNotificationData(ZulipTestCase):
 
         # Boring case
         user_data = self.create_user_notifications_data_object(user_id=user_id)
+        self.assertEqual(
+            user_data.get_push_notification_trigger(
+                private_message=False, sender_id=sender_id, idle=True
+            ),
+            None,
+        )
         self.assertFalse(
             user_data.is_push_notifiable(private_message=False, sender_id=sender_id, idle=True)
         )
 
-        # Notifiable cases for PMs, mentions, stream notifications
+        # Private message
         user_data = self.create_user_notifications_data_object(user_id=user_id)
+        self.assertEqual(
+            user_data.get_push_notification_trigger(
+                private_message=True, sender_id=sender_id, idle=True
+            ),
+            "private_message",
+        )
         self.assertTrue(
             user_data.is_push_notifiable(private_message=True, sender_id=sender_id, idle=True)
         )
 
+        # Mention
         user_data = self.create_user_notifications_data_object(
             user_id=user_id, flags=["mentioned"], mentioned=True
         )
+        self.assertEqual(
+            user_data.get_push_notification_trigger(
+                private_message=False, sender_id=sender_id, idle=True
+            ),
+            "mentioned",
+        )
         self.assertTrue(
             user_data.is_push_notifiable(private_message=False, sender_id=sender_id, idle=True)
         )
 
+        # Wildcard mention
         user_data = self.create_user_notifications_data_object(
             user_id=user_id, flags=["wildcard_mentioned"], wildcard_mention_notify=True
         )
+        self.assertEqual(
+            user_data.get_push_notification_trigger(
+                private_message=False, sender_id=sender_id, idle=True
+            ),
+            "wildcard_mentioned",
+        )
         self.assertTrue(
             user_data.is_push_notifiable(private_message=False, sender_id=sender_id, idle=True)
         )
 
+        # Stream notification
         user_data = self.create_user_notifications_data_object(
             user_id=user_id, stream_push_notify=True
         )
+        self.assertEqual(
+            user_data.get_push_notification_trigger(
+                private_message=False, sender_id=sender_id, idle=True
+            ),
+            "stream_push_notify",
+        )
         self.assertTrue(
             user_data.is_push_notifiable(private_message=False, sender_id=sender_id, idle=True)
         )
@@ -42,6 +75,12 @@ class TestNotificationData(ZulipTestCase):
         # Now, test the `online_push_enabled` property
         # Test no notifications when not idle
         user_data = self.create_user_notifications_data_object(user_id=user_id)
+        self.assertEqual(
+            user_data.get_push_notification_trigger(
+                private_message=True, sender_id=sender_id, idle=False
+            ),
+            None,
+        )
         self.assertFalse(
             user_data.is_push_notifiable(private_message=True, sender_id=sender_id, idle=False)
         )
@@ -50,6 +89,12 @@ class TestNotificationData(ZulipTestCase):
         user_data = self.create_user_notifications_data_object(
             user_id=user_id, online_push_enabled=True
         )
+        self.assertEqual(
+            user_data.get_push_notification_trigger(
+                private_message=True, sender_id=sender_id, idle=False
+            ),
+            "private_message",
+        )
         self.assertTrue(
             user_data.is_push_notifiable(private_message=True, sender_id=sender_id, idle=False)
         )
@@ -66,6 +111,12 @@ class TestNotificationData(ZulipTestCase):
             stream_email_notify=True,
             stream_push_notify=True,
         )
+        self.assertEqual(
+            user_data.get_push_notification_trigger(
+                private_message=True, sender_id=sender_id, idle=True
+            ),
+            None,
+        )
         self.assertFalse(
             user_data.is_push_notifiable(private_message=True, sender_id=sender_id, idle=True)
         )
@@ -80,6 +131,12 @@ class TestNotificationData(ZulipTestCase):
             stream_email_notify=True,
             stream_push_notify=True,
         )
+        self.assertEqual(
+            user_data.get_push_notification_trigger(
+                private_message=True, sender_id=sender_id, idle=True
+            ),
+            None,
+        )
         self.assertFalse(
             user_data.is_push_notifiable(private_message=True, sender_id=sender_id, idle=True)
         )
@@ -90,39 +147,78 @@ class TestNotificationData(ZulipTestCase):
 
         # Boring case
         user_data = self.create_user_notifications_data_object(user_id=user_id)
+        self.assertEqual(
+            user_data.get_email_notification_trigger(
+                private_message=False, sender_id=sender_id, idle=True
+            ),
+            None,
+        )
         self.assertFalse(
             user_data.is_email_notifiable(private_message=False, sender_id=sender_id, idle=True)
         )
 
-        # Notifiable cases for PMs, mentions, stream notifications
+        # Private message
         user_data = self.create_user_notifications_data_object(user_id=user_id)
+        self.assertEqual(
+            user_data.get_email_notification_trigger(
+                private_message=True, sender_id=sender_id, idle=True
+            ),
+            "private_message",
+        )
         self.assertTrue(
             user_data.is_email_notifiable(private_message=True, sender_id=sender_id, idle=True)
         )
 
+        # Mention
         user_data = self.create_user_notifications_data_object(
             user_id=user_id, flags=["mentioned"], mentioned=True
         )
+        self.assertEqual(
+            user_data.get_email_notification_trigger(
+                private_message=False, sender_id=sender_id, idle=True
+            ),
+            "mentioned",
+        )
         self.assertTrue(
             user_data.is_email_notifiable(private_message=False, sender_id=sender_id, idle=True)
         )
 
+        # Wildcard mention
         user_data = self.create_user_notifications_data_object(
             user_id=user_id, flags=["wildcard_mentioned"], wildcard_mention_notify=True
         )
+        self.assertEqual(
+            user_data.get_email_notification_trigger(
+                private_message=False, sender_id=sender_id, idle=True
+            ),
+            "wildcard_mentioned",
+        )
         self.assertTrue(
             user_data.is_email_notifiable(private_message=False, sender_id=sender_id, idle=True)
         )
 
+        # Stream notification
         user_data = self.create_user_notifications_data_object(
             user_id=user_id, stream_email_notify=True
         )
+        self.assertEqual(
+            user_data.get_email_notification_trigger(
+                private_message=False, sender_id=sender_id, idle=True
+            ),
+            "stream_email_notify",
+        )
         self.assertTrue(
             user_data.is_email_notifiable(private_message=False, sender_id=sender_id, idle=True)
         )
 
         # Test no notifications when not idle
         user_data = self.create_user_notifications_data_object(user_id=user_id)
+        self.assertEqual(
+            user_data.get_email_notification_trigger(
+                private_message=True, sender_id=sender_id, idle=False
+            ),
+            None,
+        )
         self.assertFalse(
             user_data.is_email_notifiable(private_message=True, sender_id=sender_id, idle=False)
         )
@@ -139,6 +235,12 @@ class TestNotificationData(ZulipTestCase):
             stream_email_notify=True,
             stream_push_notify=True,
         )
+        self.assertEqual(
+            user_data.get_email_notification_trigger(
+                private_message=True, sender_id=sender_id, idle=True
+            ),
+            None,
+        )
         self.assertFalse(
             user_data.is_email_notifiable(private_message=True, sender_id=sender_id, idle=True)
         )
@@ -153,6 +255,12 @@ class TestNotificationData(ZulipTestCase):
             stream_email_notify=True,
             stream_push_notify=True,
         )
+        self.assertEqual(
+            user_data.get_email_notification_trigger(
+                private_message=True, sender_id=sender_id, idle=True
+            ),
+            None,
+        )
         self.assertFalse(
             user_data.is_email_notifiable(private_message=True, sender_id=sender_id, idle=True)
         )

zerver/tornado/event_queue.py

@@ -809,16 +809,9 @@ def maybe_enqueue_notifications(
 
     if user_data.is_push_notifiable(private_message, sender_id, idle):
         notice = build_offline_notification(user_data.user_id, message_id)
-        if private_message:
+        notice["trigger"] = user_data.get_push_notification_trigger(
-            notice["trigger"] = "private_message"
+            private_message, sender_id, idle
-        elif user_data.mentioned:
+        )
-            notice["trigger"] = "mentioned"
-        elif user_data.wildcard_mention_notify:
-            notice["trigger"] = "wildcard_mentioned"
-        elif user_data.stream_push_notify:
-            notice["trigger"] = "stream_push_notify"
-        else:
-            raise AssertionError("Unknown notification trigger!")
         notice["stream_name"] = stream_name
         if not already_notified.get("push_notified"):
             queue_json_publish("missedmessage_mobile_notifications", notice)
@@ -830,16 +823,9 @@ def maybe_enqueue_notifications(
     # above.
     if user_data.is_email_notifiable(private_message, sender_id, idle):
         notice = build_offline_notification(user_data.user_id, message_id)
-        if private_message:
+        notice["trigger"] = user_data.get_email_notification_trigger(
-            notice["trigger"] = "private_message"
+            private_message, sender_id, idle
-        elif user_data.mentioned:
+        )
-            notice["trigger"] = "mentioned"
-        elif user_data.wildcard_mention_notify:
-            notice["trigger"] = "wildcard_mentioned"
-        elif user_data.stream_email_notify:
-            notice["trigger"] = "stream_email_notify"
-        else:
-            raise AssertionError("Unknown notification trigger!")
         notice["stream_name"] = stream_name
         if not already_notified.get("email_notified"):
             queue_json_publish("missedmessage_emails", notice, lambda notice: None)