message_send: Handle notifications for UNMUTED topic in a muted stream.

This commit adds 'visibility_policy' as a
parameter to user_allows_notifications_in_StreamTopic
function.

This adds logic inside the user_allows_notifications_in_StreamTopic
function, to not return False when a stream is muted
but the topic is UNMUTED.

Adds a method `user_id_to_visibility_policy_dict`
to 'StreamTopicTarget' class to fetch
(user_id => visibility_policy) in single db query.

Co-authored-by: Kartik Srivastava <kaushiksri0908@gmail.com>
Co-authored-by: Prakhar Pratyush <prakhar841301@gmail.com>

zerver/actions/message_send.py

@@ -210,7 +210,6 @@ def get_recipient_info(
         # stream_topic.  We may eventually want to have different versions
         # of this function for different message types.
         assert stream_topic is not None
-        user_ids_muting_topic = stream_topic.user_ids_with_visibility_policy(UserTopic.MUTED)
 
         subscription_rows = (
             get_subscriptions_for_send_message(
@@ -240,6 +239,7 @@ def get_recipient_info(
         )
 
         message_to_user_ids = [row["user_profile_id"] for row in subscription_rows]
+        user_id_to_visibility_policy = stream_topic.user_id_to_visibility_policy_dict()
 
         def notification_recipients(setting: str) -> Set[int]:
             return {
@@ -247,7 +247,9 @@ def get_recipient_info(
                 for row in subscription_rows
                 if user_allows_notifications_in_StreamTopic(
                     row["is_muted"],
-                    row["user_profile_id"] in user_ids_muting_topic,
+                    user_id_to_visibility_policy.get(
+                        row["user_profile_id"], UserTopic.VISIBILITY_POLICY_INHERIT
+                    ),
                     row[setting],
                     row["user_profile_" + setting],
                 )

zerver/lib/notification_data.py

@@ -4,7 +4,7 @@ from typing import Any, Collection, Dict, List, Optional, Set
 
 from zerver.lib.mention import MentionData
 from zerver.lib.user_groups import get_user_group_direct_member_ids
-from zerver.models import NotificationTriggers, UserGroup, UserProfile
+from zerver.models import NotificationTriggers, UserGroup, UserProfile, UserTopic
 
 
 @dataclass
@@ -176,15 +176,18 @@ class UserMessageNotificationsData:
 
 def user_allows_notifications_in_StreamTopic(
     stream_is_muted: bool,
-    topic_is_muted: bool,
+    visibility_policy: int,
     stream_specific_setting: Optional[bool],
     global_setting: bool,
 ) -> bool:
     """
-    Captures the hierarchy of notification settings, where muting is considered first, followed
+    Captures the hierarchy of notification settings, where visibility policy is considered first,
-    by stream-specific settings, and the global-setting in the UserProfile is the fallback.
+    followed by stream-specific settings, and the global-setting in the UserProfile is the fallback.
     """
-    if stream_is_muted or topic_is_muted:
+    if stream_is_muted and visibility_policy != UserTopic.UNMUTED:
+        return False
+
+    if visibility_policy == UserTopic.MUTED:
         return False
 
     if stream_specific_setting is not None:

zerver/lib/stream_topic.py

@@ -1,4 +1,4 @@
-from typing import Set
+from typing import Dict, Set
 
 from zerver.models import UserTopic
 
@@ -24,3 +24,16 @@ class StreamTopicTarget:
             "user_profile_id",
         )
         return {row["user_profile_id"] for row in query}
+
+    def user_id_to_visibility_policy_dict(self) -> Dict[int, int]:
+        user_id_to_visibility_policy: Dict[int, int] = {}
+
+        query = UserTopic.objects.filter(
+            stream_id=self.stream_id, topic_name__iexact=self.topic_name
+        ).values(
+            "visibility_policy",
+            "user_profile_id",
+        )
+        for row in query:
+            user_id_to_visibility_policy[row["user_profile_id"]] = row["visibility_policy"]
+        return user_id_to_visibility_policy

zerver/tests/test_event_queue.py

@@ -633,6 +633,133 @@ class MissedMessageHookTest(ZulipTestCase):
                 already_notified={"email_notified": False, "push_notified": False},
             )
 
+    def test_stream_email_and_push_notify_unmuted_topic_muted_stream_with_all_notifications_turned_off(
+        self,
+    ) -> None:
+        # Both push and email notifications should not be sent
+        self.change_subscription_properties({"is_muted": True})
+        do_set_user_topic_visibility_policy(
+            self.user_profile,
+            get_stream("Denmark", self.user_profile.realm),
+            "unmutingtest",
+            visibility_policy=UserTopic.UNMUTED,
+        )
+        msg_id = self.send_stream_message(
+            self.iago,
+            "Denmark",
+            content="what's up everyone?",
+            topic_name="unmutingtest",
+        )
+        with mock.patch("zerver.tornado.event_queue.maybe_enqueue_notifications") as mock_enqueue:
+            missedmessage_hook(self.user_profile.id, self.client_descriptor, True)
+            mock_enqueue.assert_called_once()
+            args_dict = mock_enqueue.call_args_list[0][1]
+
+            self.assert_maybe_enqueue_notifications_call_args(
+                args_dict=args_dict,
+                message_id=msg_id,
+                user_id=self.user_profile.id,
+                already_notified={"email_notified": False, "push_notified": False},
+            )
+
+    def test_stream_email_and_push_notify_unmuted_topic_muted_stream_with_global_setting_turned_on(
+        self,
+    ) -> None:
+        # Both push and email notifications should be sent
+        do_change_user_setting(
+            self.user_profile, "enable_stream_push_notifications", True, acting_user=None
+        )
+        do_change_user_setting(
+            self.user_profile, "enable_stream_email_notifications", True, acting_user=None
+        )
+        self.change_subscription_properties({"is_muted": True})
+        do_set_user_topic_visibility_policy(
+            self.user_profile,
+            get_stream("Denmark", self.user_profile.realm),
+            "unmutingtest",
+            visibility_policy=UserTopic.UNMUTED,
+        )
+        msg_id = self.send_stream_message(
+            self.iago,
+            "Denmark",
+            content="what's up everyone?",
+            topic_name="unmutingtest",
+        )
+        with mock.patch("zerver.tornado.event_queue.maybe_enqueue_notifications") as mock_enqueue:
+            missedmessage_hook(self.user_profile.id, self.client_descriptor, True)
+            mock_enqueue.assert_called_once()
+            args_dict = mock_enqueue.call_args_list[0][1]
+
+            self.assert_maybe_enqueue_notifications_call_args(
+                args_dict=args_dict,
+                message_id=msg_id,
+                user_id=self.user_profile.id,
+                stream_push_notify=True,
+                stream_email_notify=True,
+                already_notified={"email_notified": True, "push_notified": True},
+            )
+
+    def test_stream_email_and_push_notify_unmuted_topic_muted_stream_with_stream_setting_turned_on(
+        self,
+    ) -> None:
+        # Both push and email notifications should be sent
+        self.change_subscription_properties(
+            {"push_notifications": True, "email_notifications": True, "is_muted": True}
+        )
+        do_set_user_topic_visibility_policy(
+            self.user_profile,
+            get_stream("Denmark", self.user_profile.realm),
+            "unmutingtest",
+            visibility_policy=UserTopic.UNMUTED,
+        )
+        msg_id = self.send_stream_message(
+            self.iago,
+            "Denmark",
+            content="what's up everyone?",
+            topic_name="unmutingtest",
+        )
+        with mock.patch("zerver.tornado.event_queue.maybe_enqueue_notifications") as mock_enqueue:
+            missedmessage_hook(self.user_profile.id, self.client_descriptor, True)
+            mock_enqueue.assert_called_once()
+            args_dict = mock_enqueue.call_args_list[0][1]
+
+            self.assert_maybe_enqueue_notifications_call_args(
+                args_dict=args_dict,
+                message_id=msg_id,
+                user_id=self.user_profile.id,
+                stream_push_notify=True,
+                stream_email_notify=True,
+                already_notified={"email_notified": True, "push_notified": True},
+            )
+
+    def test_stream_email_and_push_notify_unmuted_topic_and_unmuted_stream(
+        self,
+    ) -> None:
+        # Both push and email notifications should be not sent
+        do_set_user_topic_visibility_policy(
+            self.user_profile,
+            get_stream("Denmark", self.user_profile.realm),
+            "unmutingtest",
+            visibility_policy=UserTopic.UNMUTED,
+        )
+        msg_id = self.send_stream_message(
+            self.iago,
+            "Denmark",
+            content="what's up everyone?",
+            topic_name="unmutingtest",
+        )
+        with mock.patch("zerver.tornado.event_queue.maybe_enqueue_notifications") as mock_enqueue:
+            missedmessage_hook(self.user_profile.id, self.client_descriptor, True)
+            mock_enqueue.assert_called_once()
+            args_dict = mock_enqueue.call_args_list[0][1]
+
+            self.assert_maybe_enqueue_notifications_call_args(
+                args_dict=args_dict,
+                message_id=msg_id,
+                user_id=self.user_profile.id,
+                already_notified={"email_notified": False, "push_notified": False},
+            )
+
     def test_muted_sender(self) -> None:
         do_mute_user(self.user_profile, self.iago)
         msg_id = self.send_personal_message(self.iago, self.user_profile)

zerver/tests/test_users.py

@@ -1841,6 +1841,33 @@ class RecipientInfoTest(ZulipTestCase):
         )
         self.assertEqual(info.stream_push_user_ids, {hamlet.id})
 
+        # Now have Hamlet mute the stream and unmute the topic,
+        # which shouldn't omit him from stream_push_user_ids.
+        sub.is_muted = True
+        sub.save()
+
+        do_set_user_topic_visibility_policy(
+            hamlet,
+            stream,
+            topic_name,
+            visibility_policy=UserTopic.UNMUTED,
+        )
+
+        info = get_recipient_info(
+            realm_id=realm.id,
+            recipient=recipient,
+            sender_id=hamlet.id,
+            stream_topic=stream_topic,
+        )
+        self.assertEqual(info.stream_push_user_ids, {hamlet.id})
+
+        # Now unmute the stream and remove topic visibility_policy.
+        sub.is_muted = False
+        sub.save()
+        do_set_user_topic_visibility_policy(
+            hamlet, stream, topic_name, visibility_policy=UserTopic.VISIBILITY_POLICY_INHERIT
+        )
+
         # Now have Hamlet mute the topic to omit him from stream_push_user_ids.
         do_set_user_topic_visibility_policy(
             hamlet,