emails: Make sender name go in-line with message body.

Making sender name go in-line with message body only if
the html starts with <p> tag since it won't look good
if the message starts with a code snippet, ul, etc.
If message starts with p tag we can safely assume that
it can go in-line with sender name.

templates/zerver/emails/digest.source.html

@@ -9,7 +9,6 @@
                     <div class='hot_convo_recipient_header'>{{ recipient_block.header.html|safe }}</div>
                     <div class='hot_convo_message_content'>
                         {% for sender_block in recipient_block.senders %}
-                            {% if sender_block.sender %} <div class="hot_convo_message_sender">{{ sender_block.sender }}</div>{% endif %}
                             {% for message_block in sender_block.content %}
                             <div class='hot_convo_message_content_block'>
                                 {{ message_block.html|safe }}

templates/zerver/emails/digest.txt

@@ -1,7 +1,6 @@
 {% if hot_conversations %}
 {% for convo in hot_conversations %}{% for recipient_block in convo.first_few_messages %}{{ recipient_block.header.plain }}
-{% for sender_block in recipient_block.senders %}{% if sender_block.sender %}{{ sender_block.sender }}
+{% for sender_block in recipient_block.senders %}
-{% endif %}
 {% for message_block in sender_block.content %}{{ message_block.plain }}
 {% endfor %}{% endfor %}
 {% if convo.count > 0 %}+ {{ convo.count }} more message{{ convo.count|pluralize }} by {{ convo.participants|display_list(4) }}.{% endif %}{% endfor %}{% endfor %}{% endif %}

templates/zerver/emails/missed_message.source.html

@@ -15,7 +15,6 @@
         {% for recipient_block in messages %}
             {% for sender_block in recipient_block.senders %}
                 <div class="missed_message">
-                    {% if sender_block.sender %}<div class="missed_message_sender">{{ sender_block.sender }}</div>{% endif %}
                     {% for message_block in sender_block.content %}
                         {{ message_block.html|safe }}
                     {% endfor %}

templates/zerver/emails/missed_message.txt

@@ -1,10 +1,6 @@
 {% if show_message_content %}
 {% for recipient_block in messages %}
 {% for sender_block in recipient_block.senders %}
-{% if sender_block.sender %}
-{{ sender_block.sender }}
----
-{% endif %}
 {% for message_block in sender_block.content %}
 {{ message_block.plain }}
 {% endfor %}

zerver/lib/email_notifications.py

@@ -33,6 +33,7 @@ import re
 import subprocess
 from collections import defaultdict
 import pytz
+from bs4 import BeautifulSoup
 
 def relative_to_full_url(base_url: str, content: str) -> str:
     # Convert relative URLs to absolute URLs.
@@ -138,7 +139,18 @@ def build_message_list(user_profile: UserProfile, messages: List[Message]) -> Li
         # with a simple hyperlink.
         return re.sub(r"\[(\S*)\]\((\S*)\)", r"\2", content)
 
-    def build_message_payload(message: Message) -> Dict[str, str]:
+    def append_sender_to_message(message_plain: str, message_html: str, sender: str) -> Tuple[str, str]:
+        message_plain = "{}: {}".format(sender, message_plain)
+        message_soup = BeautifulSoup(message_html, "html.parser")
+        sender_name_soup = BeautifulSoup("<b>{}</b>: ".format(sender), "html.parser")
+        first_tag = message_soup.find()
+        if first_tag.name == "p":
+            first_tag.insert(0, sender_name_soup)
+        else:
+            message_soup.insert(0, sender_name_soup)
+        return message_plain, str(message_soup)
+
+    def build_message_payload(message: Message, sender: Optional[str]=None) -> Dict[str, str]:
         plain = message.content
         plain = fix_plaintext_image_urls(plain)
         # There's a small chance of colliding with non-Zulip URLs containing
@@ -154,13 +166,14 @@ def build_message_list(user_profile: UserProfile, messages: List[Message]) -> Li
         html = message.rendered_content
         html = relative_to_full_url(user_profile.realm.uri, html)
         html = fix_emojis(html, user_profile.realm.uri, user_profile.emojiset)
-
+        if sender:
+            plain, html = append_sender_to_message(plain, html, sender)
         return {'plain': plain, 'html': html}
 
     def build_sender_payload(message: Message) -> Dict[str, Any]:
         sender = sender_string(message)
         return {'sender': sender,
-                'content': [build_message_payload(message)]}
+                'content': [build_message_payload(message, sender)]}
 
     def message_header(user_profile: UserProfile, message: Message) -> Dict[str, Any]:
         if message.recipient.type == Recipient.PERSONAL:

zerver/tests/test_email_notifications.py

@@ -241,7 +241,7 @@ class TestMissedMessages(ZulipTestCase):
             '@**King Hamlet**')
 
         if show_message_content:
-            body = ("Othello, the Moor of Venice --- 1 2 3 4 5 6 7 8 9 10 @**King Hamlet** "
+            body = ("Othello, the Moor of Venice: 1 2 3 4 5 6 7 8 9 10 @**King Hamlet** "
                     "You are receiving this email because you were mentioned")
             email_subject = '#Denmark > test'
             verify_body_does_not_include = []  # type: List[str]
@@ -271,7 +271,7 @@ class TestMissedMessages(ZulipTestCase):
         msg_id = self.send_stream_message(
             self.example_email('othello'), "denmark",
             '12')
-        body = ("Othello, the Moor of Venice --- 1 2 3 4 5 6 7 8 9 10 12 "
+        body = ("Othello, the Moor of Venice: 1 2 3 4 5 6 7 8 9 10 12 "
                 "You are receiving this email "
                 "because you have email notifications enabled for this stream.")
         email_subject = '#Denmark > test'
@@ -288,7 +288,7 @@ class TestMissedMessages(ZulipTestCase):
         msg_id = self.send_stream_message(
             self.example_email('othello'), "Denmark",
             '@**King Hamlet**')
-        body = ("Cordelia Lear --- 0 1 2 Othello, the Moor of Venice --- @**King Hamlet** "
+        body = ("Cordelia Lear: 0 1 2 Othello, the Moor of Venice: @**King Hamlet** "
                 "You are receiving this email because you were mentioned")
         email_subject = '#Denmark > test'
         self._test_cases(tokens, msg_id, body, email_subject, send_as_user, trigger='mentioned')
@@ -366,7 +366,7 @@ class TestMissedMessages(ZulipTestCase):
         )
 
         if show_message_content:
-            body = 'Othello, the Moor of Venice --- Group personal message! Manage email preferences:'
+            body = 'Othello, the Moor of Venice: Group personal message! Manage email preferences:'
             email_subject = 'Group PMs with Iago and Othello, the Moor of Venice'
             verify_body_does_not_include = []  # type: List[str]
         else:
@@ -395,7 +395,7 @@ class TestMissedMessages(ZulipTestCase):
             'Group personal message!',
         )
 
-        body = 'Othello, the Moor of Venice --- Group personal message! Manage email preferences'
+        body = 'Othello, the Moor of Venice: Group personal message! Manage email preferences'
         email_subject = 'Group PMs with Cordelia Lear, Iago, and Othello, the Moor of Venice'
         self._test_cases(tokens, msg_id, body, email_subject, send_as_user)
 
@@ -412,7 +412,7 @@ class TestMissedMessages(ZulipTestCase):
                                            self.example_email('prospero')],
                                           'Group personal message!')
 
-        body = 'Othello, the Moor of Venice --- Group personal message! Manage email preferences'
+        body = 'Othello, the Moor of Venice: Group personal message! Manage email preferences'
         email_subject = 'Group PMs with Cordelia Lear, Iago, and 2 others'
         self._test_cases(tokens, msg_id, body, email_subject, send_as_user)
 
@@ -658,6 +658,42 @@ class TestMissedMessages(ZulipTestCase):
         email_subject = 'PMs with Othello, the Moor of Venice'
         self._test_cases(tokens, msg_id, body, email_subject, send_as_user=False, verify_html_body=True)
 
+    @patch('zerver.lib.email_mirror.generate_random_token')
+    def test_sender_name_in_missed_message(self, mock_random_token: MagicMock) -> None:
+        tokens = self._get_tokens()
+        mock_random_token.side_effect = tokens
+
+        hamlet = self.example_user('hamlet')
+        msg_id_1 = self.send_stream_message(self.example_email('iago'),
+                                            "Denmark",
+                                            '@**King Hamlet**')
+        msg_id_2 = self.send_stream_message(self.example_email('iago'),
+                                            "Verona",
+                                            '* 1\n *2')
+        msg_id_3 = self.send_personal_message(self.example_email('iago'),
+                                              hamlet.email,
+                                              'Hello')
+
+        handle_missedmessage_emails(hamlet.id, [
+            {'message_id': msg_id_1, "trigger": "mentioned"},
+            {'message_id': msg_id_2, "trigger": "stream_email_notify"},
+            {'message_id': msg_id_3},
+        ])
+
+        self.assertIn('Iago: @**King Hamlet**\n\nYou are', mail.outbox[0].body)
+        # If message content starts with <p> tag the sender name is appended inside the <p> tag.
+        self.assertIn('<p><b>Iago</b>: <span class="user-mention"', mail.outbox[0].alternatives[0][0])
+
+        self.assertIn('Iago: * 1\n *2\n\nYou are receiving', mail.outbox[1].body)
+        # If message content does not starts with <p> tag sender name is appended before the <p> tag
+        self.assertIn('       <b>Iago</b>: <ul>\n<li>1<br/>\n *2</li>\n</ul>\n',
+                      mail.outbox[1].alternatives[0][0])
+
+        self.assertEqual('Hello\n\n\nManage email preferences:', mail.outbox[2].body[:33])
+        # Sender name is not appended to message for PM missed messages
+        self.assertIn('>\n                    \n                        <p>Hello</p>\n',
+                      mail.outbox[2].alternatives[0][0])
+
     @patch('zerver.lib.email_mirror.generate_random_token')
     def test_multiple_missed_personal_messages(self, mock_random_token: MagicMock) -> None:
         tokens = self._get_tokens()