Changes sender and subject lines for missed message emails.

Now, in the event of messages between two other members of a huddle,
the missed message emails are threaded in "Group PMs with name1 and
name2" and not in separate threads by sender.

Also, now the order of recipients in get_display_recipient consistent
with the order of names that appears in the list of personal messages
on the left sidebar.

Fixes most of #4553.

templates/zerver/emails/missed_message.subject

@@ -1 +1,4 @@
-Missed Zulips from {{ sender_str }}
+{% if group_pm %} Group PMs with {{ huddle_display_name }} in {{ realm_str }}
+{% elif at_mention %} {{ sender_str }} mentioned you in {{ realm_str }}
+{% elif private_message %} {{ sender_str }} sent you a message in {{ realm_str }}
+{% endif %}

zerver/lib/notifications.py

@@ -274,13 +274,36 @@ def do_send_missedmessage_events_reply_in_zulip(user_profile, missed_messages, m
     from zerver.lib.email_mirror import create_missed_message_address
     address = create_missed_message_address(user_profile, missed_messages[0])
 
+    # TODO: For the mention case, senders should just be the people
+    # 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):
+        display_recipient = get_display_recipient(missed_messages[0].recipient)
+        # Make sure that this is a list of strings, not a string.
+        assert not isinstance(display_recipient, Text)
+        other_recipients = [r['full_name'] for r in display_recipient
+                            if r['id'] != user_profile.id]
+        context.update({'group_pm': True})
+        if len(other_recipients) == 2:
+            huddle_display_name = u"%s" % (" and ".join(other_recipients))
+            context.update({'huddle_display_name': huddle_display_name})
+        elif len(other_recipients) == 3:
+            huddle_display_name = u"%s, %s, and %s" % (other_recipients[0], other_recipients[1], other_recipients[2])
+            context.update({'huddle_display_name': huddle_display_name})
+        else:
+            huddle_display_name = u"%s, and %s others" % (', '.join(other_recipients[:2]), len(other_recipients) - 2)
+            context.update({'huddle_display_name': huddle_display_name})
+    elif (missed_messages[0].recipient.type == Recipient.PERSONAL):
+        context.update({'private_message': True})
+    else:
+        context.update({'at_mention': True})
     from_email = None
+
     if len(senders) == 1 and settings.SEND_MISSED_MESSAGE_EMAILS_AS_USER:
         # If this setting is enabled, you can reply to the Zulip
         # missed message emails directly back to the original sender.

zerver/models.py

@@ -93,7 +93,7 @@ def get_display_recipient_remote_cache(recipient_id, recipient_type, recipient_t
     # We don't really care what the ordering is, just that it's deterministic.
     user_profile_list = (UserProfile.objects.filter(subscription__recipient_id=recipient_id)
                                             .select_related()
-                                            .order_by('email'))
+                                            .order_by('id'))
     return [{'email': user_profile.email,
              'full_name': user_profile.full_name,
              'short_name': user_profile.short_name,

zerver/tests/test_notifications.py

@@ -33,8 +33,8 @@ class TestMissedMessages(ZulipTestCase):
         # type: () -> List[str]
         return [str(random.getrandbits(32)) for _ in range(30)]
 
-    def _test_cases(self, tokens, msg_id, body, send_as_user):
+    def _test_cases(self, tokens, msg_id, body, subject, send_as_user):
-        # type: (List[str], int, str, bool) -> None
+        # type: (List[str], int, str, str, bool) -> None
         othello = self.example_user('othello')
         hamlet = self.example_user('hamlet')
         handle_missedmessage_emails(hamlet.id, [{'message_id': msg_id}])
@@ -49,6 +49,7 @@ class TestMissedMessages(ZulipTestCase):
         if send_as_user:
             from_email = '"%s" <%s>' % (othello.full_name, othello.email)
         self.assertEqual(msg.from_email, from_email)
+        self.assertEqual(msg.subject, subject)
         self.assertEqual(len(msg.reply_to), 1)
         self.assertIn(msg.reply_to[0], reply_to_addresses)
         self.assertIn(body, self.normalize_string(msg.body))
@@ -64,7 +65,8 @@ class TestMissedMessages(ZulipTestCase):
         self.send_message("othello@zulip.com", "Denmark", Recipient.STREAM, '11', subject='test2')
         msg_id = self.send_message("othello@zulip.com", "denmark", Recipient.STREAM, '@**hamlet**')
         body = 'Denmark > test Othello, the Moor of Venice 1 2 3 4 5 6 7 8 9 10 @**hamlet**'
-        self._test_cases(tokens, msg_id, body, send_as_user)
+        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')
     def _extra_context_in_personal_missed_stream_messages(self, send_as_user, mock_random_token):
@@ -76,7 +78,8 @@ class TestMissedMessages(ZulipTestCase):
                                    Recipient.PERSONAL,
                                    'Extremely personal message!')
         body = 'You and Othello, the Moor of Venice Extremely personal message!'
-        self._test_cases(tokens, msg_id, body, send_as_user)
+        subject = 'Othello, the Moor of Venice sent you a message in Zulip Dev'
+        self._test_cases(tokens, msg_id, body, subject, send_as_user)
 
     @patch('zerver.lib.email_mirror.generate_random_token')
     def _reply_to_email_in_personal_missed_stream_messages(self, send_as_user, mock_random_token):
@@ -88,7 +91,8 @@ class TestMissedMessages(ZulipTestCase):
                                    Recipient.PERSONAL,
                                    'Extremely personal message!')
         body = 'Or just reply to this email.'
-        self._test_cases(tokens, msg_id, body, send_as_user)
+        subject = 'Othello, the Moor of Venice sent you a message in Zulip Dev'
+        self._test_cases(tokens, msg_id, body, subject, send_as_user)
 
     @patch('zerver.lib.email_mirror.generate_random_token')
     def _reply_warning_in_personal_missed_stream_messages(self, send_as_user, mock_random_token):
@@ -100,22 +104,56 @@ class TestMissedMessages(ZulipTestCase):
                                    Recipient.PERSONAL,
                                    'Extremely personal message!')
         body = 'Please do not reply to this automated message.'
-        self._test_cases(tokens, msg_id, body, send_as_user)
+        subject = 'Othello, the Moor of Venice sent you a message in Zulip Dev'
+        self._test_cases(tokens, msg_id, body, subject, send_as_user)
 
     @patch('zerver.lib.email_mirror.generate_random_token')
-    def _extra_context_in_huddle_missed_stream_messages(self, send_as_user, mock_random_token):
+    def _extra_context_in_huddle_missed_stream_messages_two_others(self, send_as_user, mock_random_token):
         # type: (bool, MagicMock) -> None
         tokens = self._get_tokens()
         mock_random_token.side_effect = tokens
 
         msg_id = self.send_message("othello@zulip.com",
                                    ["hamlet@zulip.com", "iago@zulip.com"],
-                                   Recipient.PERSONAL,
+                                   Recipient.HUDDLE,
                                    'Group personal message!')
 
         body = ('You and Iago, Othello, the Moor of Venice Othello,'
                 ' the Moor of Venice Group personal message')
-        self._test_cases(tokens, msg_id, body, send_as_user)
+        subject = 'Group PMs with Iago and Othello, the Moor of Venice in Zulip Dev'
+        self._test_cases(tokens, msg_id, body, subject, send_as_user)
+
+    @patch('zerver.lib.email_mirror.generate_random_token')
+    def _extra_context_in_huddle_missed_stream_messages_three_others(self, send_as_user, mock_random_token):
+        # type: (bool, MagicMock) -> None
+        tokens = self._get_tokens()
+        mock_random_token.side_effect = tokens
+
+        msg_id = self.send_message("othello@zulip.com",
+                                   ["hamlet@zulip.com", "iago@zulip.com", "cordelia@zulip.com"],
+                                   Recipient.HUDDLE,
+                                   'Group personal message!')
+
+        body = ('You and Cordelia Lear, Iago, Othello, the Moor of Venice Othello,'
+                ' the Moor of Venice Group personal message')
+        subject = 'Group PMs with Cordelia Lear, Iago, and Othello, the Moor of Venice in Zulip Dev'
+        self._test_cases(tokens, msg_id, body, subject, send_as_user)
+
+    @patch('zerver.lib.email_mirror.generate_random_token')
+    def _extra_context_in_huddle_missed_stream_messages_many_others(self, send_as_user, mock_random_token):
+        # type: (bool, MagicMock) -> None
+        tokens = self._get_tokens()
+        mock_random_token.side_effect = tokens
+
+        msg_id = self.send_message("othello@zulip.com",
+                                   ["hamlet@zulip.com", "iago@zulip.com", "cordelia@zulip.com", "prospero@zulip.com"],
+                                   Recipient.HUDDLE,
+                                   'Group personal message!')
+
+        body = ('You and Cordelia Lear, Iago, Othello, the Moor of Venice, Prospero from The Tempest'
+                ' Othello, the Moor of Venice Group personal message')
+        subject = 'Group PMs with Cordelia Lear, Iago, and 2 others in Zulip Dev'
+        self._test_cases(tokens, msg_id, body, subject, send_as_user)
 
     @patch('zerver.lib.email_mirror.generate_random_token')
     def _deleted_message_in_missed_stream_messages(self, send_as_user, mock_random_token):
@@ -199,13 +237,31 @@ class TestMissedMessages(ZulipTestCase):
         self._extra_context_in_personal_missed_stream_messages(False)
 
     @override_settings(SEND_MISSED_MESSAGE_EMAILS_AS_USER=True)
-    def test_extra_context_in_huddle_missed_stream_messages_as_user(self):
+    def test_extra_context_in_huddle_missed_stream_messages_two_others_as_user(self):
         # type: () -> None
-        self._extra_context_in_huddle_missed_stream_messages(True)
+        self._extra_context_in_huddle_missed_stream_messages_two_others(True)
 
-    def test_extra_context_in_huddle_missed_stream_messages(self):
+    def test_extra_context_in_huddle_missed_stream_messages_two_others(self):
         # type: () -> None
-        self._extra_context_in_huddle_missed_stream_messages(False)
+        self._extra_context_in_huddle_missed_stream_messages_two_others(False)
+
+    @override_settings(SEND_MISSED_MESSAGE_EMAILS_AS_USER=True)
+    def test_extra_context_in_huddle_missed_stream_messages_three_others_as_user(self):
+        # type: () -> None
+        self._extra_context_in_huddle_missed_stream_messages_three_others(True)
+
+    def test_extra_context_in_huddle_missed_stream_messages_three_others(self):
+        # type: () -> None
+        self._extra_context_in_huddle_missed_stream_messages_three_others(False)
+
+    @override_settings(SEND_MISSED_MESSAGE_EMAILS_AS_USER=True)
+    def test_extra_context_in_huddle_missed_stream_messages_many_others_as_user(self):
+        # type: () -> None
+        self._extra_context_in_huddle_missed_stream_messages_many_others(True)
+
+    def test_extra_context_in_huddle_missed_stream_messages_many_others(self):
+        # type: () -> None
+        self._extra_context_in_huddle_missed_stream_messages_many_others(False)
 
     @override_settings(SEND_MISSED_MESSAGE_EMAILS_AS_USER=True)
     def test_deleted_message_in_missed_stream_messages_as_user(self):