diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index 7ce92f264e..30a47db4ea 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -35,6 +35,7 @@ from zerver.lib.message import ( from zerver.lib.realm_icon import realm_icon_url from zerver.lib.retention import move_message_to_archive from zerver.lib.send_email import send_email, FromAddress +from zerver.lib.stream_topic import StreamTopicTarget from zerver.lib.topic_mutes import ( get_topic_mutes, add_topic_mute, @@ -764,8 +765,8 @@ RecipientInfoResult = TypedDict('RecipientInfoResult', { 'service_bot_tuples': List[Tuple[int, int]], }) -def get_recipient_info(recipient, sender_id): - # type: (Recipient, int) -> RecipientInfoResult +def get_recipient_info(recipient, sender_id, stream_topic): + # type: (Recipient, int, Optional[StreamTopicTarget]) -> RecipientInfoResult stream_push_user_ids = set() # type: Set[int] if recipient.type == Recipient.PERSONAL: @@ -783,16 +784,18 @@ def get_recipient_info(recipient, sender_id): 'push_notifications', 'in_home_view', ).order_by('user_profile_id') + user_ids = [ row['user_profile_id'] for row in subscription_rows ] + stream_push_user_ids = { row['user_profile_id'] for row in subscription_rows # Note: muting a stream overrides stream_push_notify if row['push_notifications'] and row['in_home_view'] - } + } - stream_topic.user_ids_muting_topic() elif recipient.type == Recipient.HUDDLE: user_ids = Subscription.objects.filter( @@ -960,8 +963,20 @@ def do_send_messages(messages_maybe_none): message['realm'] = message.get('realm', message['message'].sender.realm) for message in messages: - info = get_recipient_info(message['message'].recipient, - message['message'].sender_id) + if message['message'].recipient.type == Recipient.STREAM: + stream_id = message['message'].recipient.type_id + stream_topic = StreamTopicTarget( + stream_id=stream_id, + topic_name=message['message'].topic_name() + ) + else: + stream_topic = None + + info = get_recipient_info( + recipient=message['message'].recipient, + sender_id=message['message'].sender_id, + stream_topic=stream_topic, + ) message['active_user_ids'] = info['active_user_ids'] message['push_notify_user_ids'] = info['push_notify_user_ids'] @@ -3248,9 +3263,26 @@ def do_update_message(user_profile, message, subject, propagate_mode, if Message.content_has_attachment(prev_content) or Message.content_has_attachment(message.content): check_attachment_reference_change(prev_content, message) + if message.recipient.type == Recipient.STREAM: + if subject is not None: + new_topic_name = subject + else: + new_topic_name = message.topic_name() + + stream_topic = StreamTopicTarget( + stream_id=stream_id, + topic_name=new_topic_name, + ) + else: + stream_topic = None + # TODO: We may want a slightly leaner of this function for updates. - info = get_recipient_info(message.recipient, - message.sender_id) + info = get_recipient_info( + recipient=message.recipient, + sender_id=message.sender_id, + stream_topic=stream_topic, + ) + event['push_notify_user_ids'] = list(info['push_notify_user_ids']) event['stream_push_user_ids'] = list(info['stream_push_user_ids']) event['prior_mention_user_ids'] = list(prior_mention_user_ids) diff --git a/zerver/tests/test_event_queue.py b/zerver/tests/test_event_queue.py index 56612869a2..117790f6a7 100644 --- a/zerver/tests/test_event_queue.py +++ b/zerver/tests/test_event_queue.py @@ -219,8 +219,7 @@ class MissedMessageNotificationsTest(ZulipTestCase): args_list = mock_enqueue.call_args_list[0][0] self.assertEqual(args_list, (user_profile.id, msg_id, False, False, - # BUG: That next True should be False, see #7059 - True, "Denmark", False, True, + False, "Denmark", False, True, {'email_notified': False, 'push_notified': False})) # Clear the event queue, now repeat with stream message with stream_push_notify diff --git a/zerver/tests/test_messages.py b/zerver/tests/test_messages.py index cdb1be1384..2845fa260e 100644 --- a/zerver/tests/test_messages.py +++ b/zerver/tests/test_messages.py @@ -565,7 +565,7 @@ class StreamMessagesTest(ZulipTestCase): a lot of code to generate messages with markdown and without markdown. ''' - self.assert_length(queries, 14) + self.assert_length(queries, 15) def test_stream_message_dict(self): # type: () -> None diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py index 690e203da0..e959ddf5d2 100644 --- a/zerver/tests/test_subs.py +++ b/zerver/tests/test_subs.py @@ -1629,7 +1629,7 @@ class SubscriptionAPITest(ZulipTestCase): streams_to_sub, dict(principals=ujson.dumps([user1.email, user2.email])), ) - self.assert_length(queries, 41) + self.assert_length(queries, 42) self.assert_length(events, 7) for ev in [x for x in events if x['event']['type'] not in ('message', 'stream')]: diff --git a/zerver/tests/test_users.py b/zerver/tests/test_users.py index 5ed55d0a3d..b90f9c8ceb 100644 --- a/zerver/tests/test_users.py +++ b/zerver/tests/test_users.py @@ -32,6 +32,8 @@ from zerver.lib.actions import ( do_reactivate_user, do_change_is_admin, ) +from zerver.lib.topic_mutes import add_topic_mute +from zerver.lib.stream_topic import StreamTopicTarget from django.conf import settings @@ -359,6 +361,7 @@ class RecipientInfoTest(ZulipTestCase): realm = hamlet.realm stream_name = 'Test Stream' + topic_name = 'test topic' for user in [hamlet, cordelia, othello]: self.subscribe(user, stream_name) @@ -366,6 +369,11 @@ class RecipientInfoTest(ZulipTestCase): stream = get_stream(stream_name, realm) recipient = get_recipient(Recipient.STREAM, stream.id) + stream_topic = StreamTopicTarget( + stream_id=stream.id, + topic_name=topic_name, + ) + sub = get_subscription(stream_name, hamlet) sub.push_notifications = True sub.save() @@ -373,6 +381,7 @@ class RecipientInfoTest(ZulipTestCase): info = get_recipient_info( recipient=recipient, sender_id=hamlet.id, + stream_topic=stream_topic, ) all_user_ids = {hamlet.id, cordelia.id, othello.id} @@ -388,6 +397,22 @@ class RecipientInfoTest(ZulipTestCase): self.assertEqual(info, expected_info) + # Now mute Hamlet to omit him from stream_push_user_ids. + add_topic_mute( + user_profile=hamlet, + stream_id=stream.id, + recipient_id=recipient.id, + topic_name=topic_name, + ) + + info = get_recipient_info( + recipient=recipient, + sender_id=hamlet.id, + stream_topic=stream_topic, + ) + + self.assertEqual(info['stream_push_user_ids'], set()) + class BulkUsersTest(ZulipTestCase): def test_client_gravatar_option(self): # type: () -> None