notifications: Fix unnecessary wildcard mention notifications.

This fixes a bug where email notifications were sent for wildcard
mentions even if the `enable_offline_email_notifications` setting was
turned off.
This was because the `notification_data` class incorrectly considered
`wildcard_mentions_notify` as an indeoendent setting, instead of a wrapper
around `enable_offline_email_notifications` and `enable_offline_push_notifications`.

Also add a test for this case.

zerver/lib/notification_data.py

@@ -13,9 +13,10 @@ class UserMessageNotificationsData:
     pm_push_notify: bool
     mention_email_notify: bool
     mention_push_notify: bool
+    wildcard_mention_email_notify: bool
+    wildcard_mention_push_notify: bool
     stream_push_notify: bool
     stream_email_notify: bool
-    wildcard_mention_notify: bool
     sender_is_muted: bool
 
     def __post_init__(self) -> None:
@@ -41,29 +42,40 @@ class UserMessageNotificationsData:
         wildcard_mention_user_ids: Set[int],
         muted_sender_user_ids: Set[int],
     ) -> "UserMessageNotificationsData":
-        wildcard_mention_notify = (
-            user_id in wildcard_mention_user_ids and "wildcard_mentioned" in flags
-        )
 
+        # `wildcard_mention_user_ids` are those user IDs for whom wildcard mentions should
+        # obey notification settings of personal mentions. Hence, it isn't an independent
+        # notification setting and acts as a wrapper.
         pm_email_notify = user_id not in pm_mention_email_disabled_user_ids and private_message
         mention_email_notify = (
             user_id not in pm_mention_email_disabled_user_ids and "mentioned" in flags
         )
+        wildcard_mention_email_notify = (
+            user_id in wildcard_mention_user_ids
+            and user_id not in pm_mention_email_disabled_user_ids
+            and "wildcard_mentioned" in flags
+        )
 
         pm_push_notify = user_id not in pm_mention_push_disabled_user_ids and private_message
         mention_push_notify = (
             user_id not in pm_mention_push_disabled_user_ids and "mentioned" in flags
         )
+        wildcard_mention_push_notify = (
+            user_id in wildcard_mention_user_ids
+            and user_id not in pm_mention_push_disabled_user_ids
+            and "wildcard_mentioned" in flags
+        )
         return cls(
             user_id=user_id,
             pm_email_notify=pm_email_notify,
             mention_email_notify=mention_email_notify,
+            wildcard_mention_email_notify=wildcard_mention_email_notify,
             pm_push_notify=pm_push_notify,
             mention_push_notify=mention_push_notify,
+            wildcard_mention_push_notify=wildcard_mention_push_notify,
             online_push_enabled=(user_id in online_push_user_ids),
             stream_push_notify=(user_id in stream_push_user_ids),
             stream_email_notify=(user_id in stream_email_user_ids),
-            wildcard_mention_notify=wildcard_mention_notify,
             sender_is_muted=(user_id in muted_sender_user_ids),
         )
 
@@ -95,7 +107,7 @@ class UserMessageNotificationsData:
             return NotificationTriggers.PRIVATE_MESSAGE
         elif self.mention_push_notify:
             return NotificationTriggers.MENTION
-        elif self.wildcard_mention_notify:
+        elif self.wildcard_mention_push_notify:
             return NotificationTriggers.WILDCARD_MENTION
         elif self.stream_push_notify:
             return NotificationTriggers.STREAM_PUSH
@@ -122,7 +134,7 @@ class UserMessageNotificationsData:
             return NotificationTriggers.PRIVATE_MESSAGE
         elif self.mention_email_notify:
             return NotificationTriggers.MENTION
-        elif self.wildcard_mention_notify:
+        elif self.wildcard_mention_email_notify:
             return NotificationTriggers.WILDCARD_MENTION
         elif self.stream_email_notify:
             return NotificationTriggers.STREAM_EMAIL

zerver/lib/test_classes.py

@@ -1344,9 +1344,10 @@ Output:
             pm_push_notify=kwargs.get("pm_push_notify", False),
             mention_email_notify=kwargs.get("mention_email_notify", False),
             mention_push_notify=kwargs.get("mention_push_notify", False),
+            wildcard_mention_email_notify=kwargs.get("wildcard_mention_email_notify", False),
+            wildcard_mention_push_notify=kwargs.get("wildcard_mention_push_notify", False),
             stream_email_notify=kwargs.get("stream_email_notify", False),
             stream_push_notify=kwargs.get("stream_push_notify", False),
-            wildcard_mention_notify=kwargs.get("wildcard_mention_notify", False),
             sender_is_muted=kwargs.get("sender_is_muted", False),
         )
 

zerver/tests/test_event_queue.py

@@ -310,7 +310,8 @@ class MissedMessageNotificationsTest(ZulipTestCase):
             assert_maybe_enqueue_notifications_call_args(
                 args_dict=args_dict,
                 message_id=msg_id,
-                wildcard_mention_notify=True,
+                wildcard_mention_email_notify=True,
+                wildcard_mention_push_notify=True,
                 already_notified={"email_notified": True, "push_notified": True},
             )
         destroy_event_queue(client_descriptor.event_queue.id)
@@ -327,7 +328,8 @@ class MissedMessageNotificationsTest(ZulipTestCase):
 
             assert_maybe_enqueue_notifications_call_args(
                 args_dict=args_dict,
-                wildcard_mention_notify=False,
+                wildcard_mention_email_notify=False,
+                wildcard_mention_push_notify=False,
                 message_id=msg_id,
                 already_notified={"email_notified": False, "push_notified": False},
             )
@@ -348,7 +350,8 @@ class MissedMessageNotificationsTest(ZulipTestCase):
             assert_maybe_enqueue_notifications_call_args(
                 args_dict=args_dict,
                 message_id=msg_id,
-                wildcard_mention_notify=False,
+                wildcard_mention_email_notify=False,
+                wildcard_mention_push_notify=False,
                 already_notified={"email_notified": False, "push_notified": False},
             )
         destroy_event_queue(client_descriptor.event_queue.id)
@@ -372,7 +375,8 @@ class MissedMessageNotificationsTest(ZulipTestCase):
             assert_maybe_enqueue_notifications_call_args(
                 args_dict=args_dict,
                 message_id=msg_id,
-                wildcard_mention_notify=True,
+                wildcard_mention_email_notify=True,
+                wildcard_mention_push_notify=True,
                 already_notified={"email_notified": True, "push_notified": True},
             )
         destroy_event_queue(client_descriptor.event_queue.id)
@@ -381,6 +385,35 @@ class MissedMessageNotificationsTest(ZulipTestCase):
         user_profile.save()
         sub.save()
 
+        # If notifications for personal mentions themselves have been turned off,
+        # even turning on `wildcard_mentions_notify` should not send notifications
+        user_profile.enable_offline_email_notifications = False
+        user_profile.wildcard_mentions_notify = True
+        user_profile.save()
+        client_descriptor = allocate_event_queue()
+        self.assertTrue(client_descriptor.event_queue.empty())
+        msg_id = self.send_stream_message(iago, "Denmark", content="@**all** what's up?")
+        with mock.patch("zerver.tornado.event_queue.maybe_enqueue_notifications") as mock_enqueue:
+            missedmessage_hook(user_profile.id, client_descriptor, True)
+            mock_enqueue.assert_called_once()
+            args_dict = mock_enqueue.call_args_list[0][1]
+
+            # We've turned off email notifications for personal mentions, but push notifications
+            # for personal mentions are still on.
+            # Because `wildcard_mentions_notify` is True, a message with `@all` should follow the
+            # personal mention settings
+            assert_maybe_enqueue_notifications_call_args(
+                args_dict=args_dict,
+                message_id=msg_id,
+                wildcard_mention_email_notify=False,
+                wildcard_mention_push_notify=True,
+                already_notified={"email_notified": False, "push_notified": True},
+            )
+        destroy_event_queue(client_descriptor.event_queue.id)
+        user_profile.enable_offline_email_notifications = True
+        user_profile.wildcard_mentions_notify = True
+        user_profile.save()
+
         # Test with a user group mention
         hamlet_and_cordelia = create_user_group(
             "hamlet_and_cordelia", [user_profile, cordelia], cordelia.realm

zerver/tests/test_message_edit_notifications.py

@@ -420,7 +420,8 @@ class EditMessageSideEffectsTest(ZulipTestCase):
             user_id=cordelia.id,
             acting_user_id=hamlet.id,
             message_id=message_id,
-            wildcard_mention_notify=True,
+            wildcard_mention_email_notify=True,
+            wildcard_mention_push_notify=True,
             already_notified={},
         )
         self.assertEqual(info["enqueue_kwargs"], expected_enqueue_kwargs)

zerver/tests/test_notification_data.py

@@ -37,7 +37,7 @@ class TestNotificationData(ZulipTestCase):
 
         # Wildcard mention
         user_data = self.create_user_notifications_data_object(
-            user_id=user_id, wildcard_mention_notify=True
+            user_id=user_id, wildcard_mention_push_notify=True
         )
         self.assertEqual(
             user_data.get_push_notification_trigger(acting_user_id=acting_user_id, idle=True),
@@ -79,11 +79,12 @@ class TestNotificationData(ZulipTestCase):
         user_data = self.create_user_notifications_data_object(
             user_id=user_id,
             sender_is_muted=True,
-            wildcard_mention_notify=True,
             pm_push_notify=True,
             pm_email_notify=True,
             mention_push_notify=True,
             mention_email_notify=True,
+            wildcard_mention_push_notify=True,
+            wildcard_mention_email_notify=True,
         )
         self.assertEqual(
             user_data.get_push_notification_trigger(acting_user_id=acting_user_id, idle=True),
@@ -94,11 +95,12 @@ class TestNotificationData(ZulipTestCase):
         # Message sender is the user the object corresponds to.
         user_data = self.create_user_notifications_data_object(
             user_id=acting_user_id,
-            wildcard_mention_notify=True,
             pm_push_notify=True,
             pm_email_notify=True,
             mention_push_notify=True,
             mention_email_notify=True,
+            wildcard_mention_push_notify=True,
+            wildcard_mention_email_notify=True,
         )
         self.assertEqual(
             user_data.get_push_notification_trigger(acting_user_id=acting_user_id, idle=True),
@@ -140,7 +142,7 @@ class TestNotificationData(ZulipTestCase):
 
         # Wildcard mention
         user_data = self.create_user_notifications_data_object(
-            user_id=user_id, wildcard_mention_notify=True
+            user_id=user_id, wildcard_mention_email_notify=True
         )
         self.assertEqual(
             user_data.get_email_notification_trigger(acting_user_id=acting_user_id, idle=True),
@@ -173,11 +175,12 @@ class TestNotificationData(ZulipTestCase):
         user_data = self.create_user_notifications_data_object(
             user_id=user_id,
             sender_is_muted=True,
-            wildcard_mention_notify=True,
             pm_push_notify=True,
             pm_email_notify=True,
             mention_push_notify=True,
             mention_email_notify=True,
+            wildcard_mention_push_notify=True,
+            wildcard_mention_email_notify=True,
         )
         self.assertEqual(
             user_data.get_email_notification_trigger(acting_user_id=acting_user_id, idle=True),
@@ -188,11 +191,12 @@ class TestNotificationData(ZulipTestCase):
         # Message sender is the user the object corresponds to.
         user_data = self.create_user_notifications_data_object(
             user_id=acting_user_id,
-            wildcard_mention_notify=True,
             pm_push_notify=True,
             pm_email_notify=True,
             mention_push_notify=True,
             mention_email_notify=True,
+            wildcard_mention_push_notify=True,
+            wildcard_mention_email_notify=True,
         )
         self.assertEqual(
             user_data.get_email_notification_trigger(acting_user_id=acting_user_id, idle=True),

zerver/tornado/event_queue.py

@@ -762,9 +762,10 @@ def missedmessage_hook(
             pm_email_notify=internal_data.get("pm_email_notify", False),
             mention_push_notify=internal_data.get("mention_push_notify", False),
             mention_email_notify=internal_data.get("mention_email_notify", False),
+            wildcard_mention_push_notify=internal_data.get("wildcard_mention_push_notify", False),
+            wildcard_mention_email_notify=internal_data.get("wildcard_mention_email_notify", False),
             stream_push_notify=internal_data.get("stream_push_notify", False),
             stream_email_notify=internal_data.get("stream_email_notify", False),
-            wildcard_mention_notify=internal_data.get("wildcard_mention_notify", False),
             # Since one is by definition idle, we don't need to check online_push_enabled
             online_push_enabled=False,
         )