mirror of
				https://github.com/zulip/zulip.git
				synced 2025-11-04 05:53:43 +00:00 
			
		
		
		
	mentions: Fix subject line and sender for missed-message mentions.
This fixes 2 issues: * The term "@-mentioned" is simplified to "mentioned". * We would incorrectly list other people who sent context messages as among the people who mentioned you.
This commit is contained in:
		@@ -274,14 +274,7 @@ def do_send_missedmessage_events_reply_in_zulip(user_profile, missed_messages, m
 | 
				
			|||||||
    from zerver.lib.email_mirror import create_missed_message_address
 | 
					    from zerver.lib.email_mirror import create_missed_message_address
 | 
				
			||||||
    address = create_missed_message_address(user_profile, missed_messages[0])
 | 
					    address = create_missed_message_address(user_profile, missed_messages[0])
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    # TODO: For the mention case, senders should just be the people
 | 
					    senders = list(set(m.sender for m in missed_messages))
 | 
				
			||||||
    # who mentioned you, not the other senders.
 | 
					 | 
				
			||||||
    senders = set(m.sender.full_name for m in missed_messages)
 | 
					 | 
				
			||||||
    sender_str = ", ".join(senders)
 | 
					 | 
				
			||||||
    context.update({
 | 
					 | 
				
			||||||
        'sender_str': sender_str,
 | 
					 | 
				
			||||||
        'realm_str': user_profile.realm.name,
 | 
					 | 
				
			||||||
    })
 | 
					 | 
				
			||||||
    if (missed_messages[0].recipient.type == Recipient.HUDDLE):
 | 
					    if (missed_messages[0].recipient.type == Recipient.HUDDLE):
 | 
				
			||||||
        display_recipient = get_display_recipient(missed_messages[0].recipient)
 | 
					        display_recipient = get_display_recipient(missed_messages[0].recipient)
 | 
				
			||||||
        # Make sure that this is a list of strings, not a string.
 | 
					        # Make sure that this is a list of strings, not a string.
 | 
				
			||||||
@@ -301,7 +294,20 @@ def do_send_missedmessage_events_reply_in_zulip(user_profile, missed_messages, m
 | 
				
			|||||||
    elif (missed_messages[0].recipient.type == Recipient.PERSONAL):
 | 
					    elif (missed_messages[0].recipient.type == Recipient.PERSONAL):
 | 
				
			||||||
        context.update({'private_message': True})
 | 
					        context.update({'private_message': True})
 | 
				
			||||||
    else:
 | 
					    else:
 | 
				
			||||||
 | 
					        # Keep only the senders who actually mentioned the user
 | 
				
			||||||
 | 
					        #
 | 
				
			||||||
 | 
					        # TODO: When we add wildcard mentions that send emails, add
 | 
				
			||||||
 | 
					        # them to the filter here.
 | 
				
			||||||
 | 
					        senders = list(set(m.sender for m in missed_messages if
 | 
				
			||||||
 | 
					                           UserMessage.objects.filter(message=m, user_profile=user_profile,
 | 
				
			||||||
 | 
					                                                      flags=UserMessage.flags.mentioned).exists()))
 | 
				
			||||||
        context.update({'at_mention': True})
 | 
					        context.update({'at_mention': True})
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    context.update({
 | 
				
			||||||
 | 
					        'sender_str': ", ".join(sender.full_name for sender in senders),
 | 
				
			||||||
 | 
					        'realm_str': user_profile.realm.name,
 | 
				
			||||||
 | 
					    })
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    from_email = None
 | 
					    from_email = None
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    if len(senders) == 1 and settings.SEND_MISSED_MESSAGE_EMAILS_AS_USER:
 | 
					    if len(senders) == 1 and settings.SEND_MISSED_MESSAGE_EMAILS_AS_USER:
 | 
				
			||||||
@@ -310,8 +316,8 @@ def do_send_missedmessage_events_reply_in_zulip(user_profile, missed_messages, m
 | 
				
			|||||||
        # However, one must ensure the Zulip server is in the SPF
 | 
					        # However, one must ensure the Zulip server is in the SPF
 | 
				
			||||||
        # record for the domain, or there will be spam/deliverability
 | 
					        # record for the domain, or there will be spam/deliverability
 | 
				
			||||||
        # problems.
 | 
					        # problems.
 | 
				
			||||||
        sender = missed_messages[0].sender
 | 
					        sender = senders[0]
 | 
				
			||||||
        from_email = '"%s" <%s>' % (sender_str, sender.email)
 | 
					        from_email = '"%s" <%s>' % (sender.full_name, sender.email)
 | 
				
			||||||
        context.update({
 | 
					        context.update({
 | 
				
			||||||
            'reply_warning': False,
 | 
					            'reply_warning': False,
 | 
				
			||||||
            'reply_to_zulip': False,
 | 
					            'reply_to_zulip': False,
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -55,7 +55,7 @@ class TestMissedMessages(ZulipTestCase):
 | 
				
			|||||||
        self.assertIn(body, self.normalize_string(msg.body))
 | 
					        self.assertIn(body, self.normalize_string(msg.body))
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    @patch('zerver.lib.email_mirror.generate_random_token')
 | 
					    @patch('zerver.lib.email_mirror.generate_random_token')
 | 
				
			||||||
    def _extra_context_in_missed_stream_messages(self, send_as_user, mock_random_token):
 | 
					    def _extra_context_in_missed_stream_messages_mention(self, send_as_user, mock_random_token):
 | 
				
			||||||
        # type: (bool, MagicMock) -> None
 | 
					        # type: (bool, MagicMock) -> None
 | 
				
			||||||
        tokens = self._get_tokens()
 | 
					        tokens = self._get_tokens()
 | 
				
			||||||
        mock_random_token.side_effect = tokens
 | 
					        mock_random_token.side_effect = tokens
 | 
				
			||||||
@@ -68,6 +68,19 @@ class TestMissedMessages(ZulipTestCase):
 | 
				
			|||||||
        subject = 'Othello, the Moor of Venice mentioned you in Zulip Dev'
 | 
					        subject = 'Othello, the Moor of Venice mentioned you in Zulip Dev'
 | 
				
			||||||
        self._test_cases(tokens, msg_id, body, subject, send_as_user)
 | 
					        self._test_cases(tokens, msg_id, body, subject, send_as_user)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    @patch('zerver.lib.email_mirror.generate_random_token')
 | 
				
			||||||
 | 
					    def _extra_context_in_missed_stream_messages_mention_two_senders(self, send_as_user, mock_random_token):
 | 
				
			||||||
 | 
					        # type: (bool, MagicMock) -> None
 | 
				
			||||||
 | 
					        tokens = self._get_tokens()
 | 
				
			||||||
 | 
					        mock_random_token.side_effect = tokens
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        for i in range(0, 3):
 | 
				
			||||||
 | 
					            self.send_message("cordelia@zulip.com", "Denmark", Recipient.STREAM, str(i))
 | 
				
			||||||
 | 
					        msg_id = self.send_message("othello@zulip.com", "Denmark", Recipient.STREAM, '@**hamlet**')
 | 
				
			||||||
 | 
					        body = 'Denmark > test Cordelia Lear 0 1 2 Othello, the Moor of Venice @**hamlet**'
 | 
				
			||||||
 | 
					        subject = 'Othello, the Moor of Venice mentioned you in Zulip Dev'
 | 
				
			||||||
 | 
					        self._test_cases(tokens, msg_id, body, subject, send_as_user)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    @patch('zerver.lib.email_mirror.generate_random_token')
 | 
					    @patch('zerver.lib.email_mirror.generate_random_token')
 | 
				
			||||||
    def _extra_context_in_personal_missed_stream_messages(self, send_as_user, mock_random_token):
 | 
					    def _extra_context_in_personal_missed_stream_messages(self, send_as_user, mock_random_token):
 | 
				
			||||||
        # type: (bool, MagicMock) -> None
 | 
					        # type: (bool, MagicMock) -> None
 | 
				
			||||||
@@ -212,11 +225,20 @@ class TestMissedMessages(ZulipTestCase):
 | 
				
			|||||||
    @override_settings(SEND_MISSED_MESSAGE_EMAILS_AS_USER=True)
 | 
					    @override_settings(SEND_MISSED_MESSAGE_EMAILS_AS_USER=True)
 | 
				
			||||||
    def test_extra_context_in_missed_stream_messages_as_user(self):
 | 
					    def test_extra_context_in_missed_stream_messages_as_user(self):
 | 
				
			||||||
        # type: () -> None
 | 
					        # type: () -> None
 | 
				
			||||||
        self._extra_context_in_missed_stream_messages(True)
 | 
					        self._extra_context_in_missed_stream_messages_mention(True)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    def test_extra_context_in_missed_stream_messages(self):
 | 
					    def test_extra_context_in_missed_stream_messages(self):
 | 
				
			||||||
        # type: () -> None
 | 
					        # type: () -> None
 | 
				
			||||||
        self._extra_context_in_missed_stream_messages(False)
 | 
					        self._extra_context_in_missed_stream_messages_mention(False)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    @override_settings(SEND_MISSED_MESSAGE_EMAILS_AS_USER=True)
 | 
				
			||||||
 | 
					    def test_extra_context_in_missed_stream_messages_as_user_two_senders(self):
 | 
				
			||||||
 | 
					        # type: () -> None
 | 
				
			||||||
 | 
					        self._extra_context_in_missed_stream_messages_mention_two_senders(True)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    def test_extra_context_in_missed_stream_messages_two_senders(self):
 | 
				
			||||||
 | 
					        # type: () -> None
 | 
				
			||||||
 | 
					        self._extra_context_in_missed_stream_messages_mention_two_senders(False)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    def test_reply_to_email_in_personal_missed_stream_messages(self):
 | 
					    def test_reply_to_email_in_personal_missed_stream_messages(self):
 | 
				
			||||||
        # type: () -> None
 | 
					        # type: () -> None
 | 
				
			||||||
 
 | 
				
			|||||||
		Reference in New Issue
	
	Block a user