notification: Use existing email format for missed 1:1 DM via DM group.

To maintain API compatibility, we render the email notification for
missed 1:1 direct messages using DirectMessageGroup with the same
format as messages sent to a Personal recipient.

zerver/lib/email_notifications.py

@@ -209,10 +209,13 @@ def build_message_list(
     messages_to_render: list[dict[str, Any]] = []
 
     def sender_string(message: Message) -> str:
-        if message.recipient.type in (Recipient.STREAM, Recipient.DIRECT_MESSAGE_GROUP):
+        if message.recipient.type == Recipient.STREAM:
             return message.sender.full_name
-        else:
-            return ""
+        elif message.recipient.type == Recipient.DIRECT_MESSAGE_GROUP:
+            display_recipient = get_display_recipient(message.recipient)
+            if len(display_recipient) > 2:
+                return message.sender.full_name
+        return ""
 
     def fix_plaintext_image_urls(content: str) -> str:
         # Replace image URLs in plaintext content of the form
@@ -272,7 +275,8 @@ def build_message_list(
             grouping: dict[str, Any] = {"user": message.sender_id}
             narrow_link = personal_narrow_url(
                 realm=user.realm,
-                sender=message.sender,
+                sender_id=message.sender.id,
+                sender_full_name=message.sender.full_name,
             )
             header = f"You and {message.sender.full_name}"
             header_html = Markup(
@@ -500,35 +504,37 @@ def do_send_missedmessage_events_reply_in_zulip(
         reply_to_name = "Zulip"
 
     senders = list({m["message"].sender for m in missed_messages})
-    if missed_messages[0]["message"].recipient.type == Recipient.DIRECT_MESSAGE_GROUP:
-        display_recipient = get_display_recipient(missed_messages[0]["message"].recipient)
+    message = missed_messages[0]["message"]
+    if message.recipient.type == Recipient.DIRECT_MESSAGE_GROUP:
+        display_recipient = get_display_recipient(message.recipient)
         narrow_url = direct_message_group_narrow_url(
             user=user_profile,
             display_recipient=display_recipient,
         )
         context.update(narrow_url=narrow_url)
         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:
-            direct_message_group_display_name = " and ".join(other_recipients)
-            context.update(direct_message_group_display_name=direct_message_group_display_name)
+        if len(other_recipients) <= 1:
+            context.update(group_pm=False, private_message=True)
+        elif len(other_recipients) == 2:
+            group_display_name = " and ".join(other_recipients)
+            context.update(group_pm=True, direct_message_group_display_name=group_display_name)
         elif len(other_recipients) == 3:
-            direct_message_group_display_name = (
+            group_display_name = (
                 f"{other_recipients[0]}, {other_recipients[1]}, and {other_recipients[2]}"
             )
-            context.update(direct_message_group_display_name=direct_message_group_display_name)
+            context.update(group_pm=True, direct_message_group_display_name=group_display_name)
         else:
-            direct_message_group_display_name = "{}, and {} others".format(
+            group_display_name = "{}, and {} others".format(
                 ", ".join(other_recipients[:2]), len(other_recipients) - 2
             )
-            context.update(direct_message_group_display_name=direct_message_group_display_name)
-    elif missed_messages[0]["message"].recipient.type == Recipient.PERSONAL:
+            context.update(group_pm=True, direct_message_group_display_name=group_display_name)
+    elif message.recipient.type == Recipient.PERSONAL:
         narrow_url = personal_narrow_url(
             realm=user_profile.realm,
-            sender=missed_messages[0]["message"].sender,
+            sender_id=message.sender.id,
+            sender_full_name=message.sender.full_name,
         )
-        context.update(narrow_url=narrow_url)
-        context.update(private_message=True)
+        context.update(narrow_url=narrow_url, private_message=True)
     elif (
         context["mention"]
         or context["stream_email_notify"]
@@ -550,7 +556,6 @@ def do_send_missedmessage_events_reply_in_zulip(
                     ]
                 }
             )
-        message = missed_messages[0]["message"]
         assert message.recipient.type == Recipient.STREAM
         stream = Stream.objects.only("id", "name").get(id=message.recipient.type_id)
         narrow_url = message_link_url(

zerver/lib/url_encoding.py

@@ -94,9 +94,9 @@ def encode_user_full_name_and_id(full_name: str, user_id: int, with_operator: bo
     return direct_message_slug
 
 
-def personal_narrow_url(*, realm: Realm, sender: UserProfile) -> str:
+def personal_narrow_url(*, realm: Realm, sender_id: int, sender_full_name: str) -> str:
     base_url = f"{realm.url}/#narrow/dm/"
-    direct_message_slug = encode_user_full_name_and_id(sender.full_name, sender.id)
+    direct_message_slug = encode_user_full_name_and_id(sender_full_name, sender_id)
     return base_url + direct_message_slug
 
 
@@ -104,6 +104,17 @@ def direct_message_group_narrow_url(
     *, user: UserProfile, display_recipient: list[UserDisplayRecipient]
 ) -> str:
     realm = user.realm
+    if len(display_recipient) == 1:
+        # For self-DMs, we use the personal narrow URL format.
+        return personal_narrow_url(realm=realm, sender_id=user.id, sender_full_name=user.full_name)
+    if len(display_recipient) == 2:
+        # For 1:1 DMs, we use the personal narrow URL format.
+        other_user = next(r for r in display_recipient if r["id"] != user.id)
+        return personal_narrow_url(
+            realm=realm, sender_id=other_user["id"], sender_full_name=other_user["full_name"]
+        )
+
+    # For group DMs with more than 2 users, we use other user IDs to create a slug.
     other_user_ids = [r["id"] for r in display_recipient if r["id"] != user.id]
     direct_message_slug = encode_user_ids(other_user_ids)
     base_url = f"{realm.url}/#narrow/dm/"

zerver/tests/test_message_notification_emails.py

@@ -34,6 +34,7 @@ from zerver.lib.test_classes import ZulipTestCase
 from zerver.models import Message, UserMessage, UserProfile, UserTopic
 from zerver.models.realm_emoji import get_name_keyed_dict_for_active_realm_emoji
 from zerver.models.realms import get_realm
+from zerver.models.recipients import get_or_create_direct_message_group
 from zerver.models.scheduled_jobs import NotificationTriggers
 from zerver.models.streams import get_stream
 
@@ -1279,6 +1280,42 @@ class TestMessageNotificationEmails(ZulipTestCase):
         email_subject = "DMs with Cordelia, Lear's daughter"
         self._test_cases(msg_id, verify_body_include, email_subject)
 
+    def test_pm_link_in_missed_message_header_with_multiple_user_with_the_same_name(self) -> None:
+        hamlet = self.example_user("hamlet")
+        iago = self.example_user("iago")
+        iago_2 = self.example_user("ZOE")
+        iago_2.full_name = "iago"
+        iago_2.save()
+
+        msg_id = self.send_group_direct_message(
+            iago,
+            [hamlet, iago_2],
+            "Group personal message!",
+        )
+
+        verify_body_include = ["Iago: > Group personal message! -- Reply"]
+        email_subject = "Group DMs with iago and Iago"
+        self._test_cases(msg_id, verify_body_include, email_subject)
+
+    def test_pm_link_in_missed_message_header_using_direct_message_group(self) -> None:
+        cordelia = self.example_user("cordelia")
+        hamlet = self.example_user("hamlet")
+
+        get_or_create_direct_message_group(id_list=[cordelia.id, hamlet.id])
+
+        msg_id = self.send_personal_message(
+            cordelia,
+            hamlet,
+            "Let's test a direct message link in email notifications",
+        )
+
+        encoded_name = "Cordelia,-Lear's-daughter"
+        verify_body_include = [
+            f"view it in Zulip Dev Zulip: http://zulip.testserver/#narrow/dm/{cordelia.id}-{encoded_name}"
+        ]
+        email_subject = "DMs with Cordelia, Lear's daughter"
+        self._test_cases(msg_id, verify_body_include, email_subject)
+
     def test_sender_name_in_missed_message(self) -> None:
         hamlet = self.example_user("hamlet")
         msg_id_1 = self.send_stream_message(
@@ -1323,6 +1360,50 @@ class TestMessageNotificationEmails(ZulipTestCase):
             mail.outbox[2].alternatives[0][0],
         )
 
+    def test_sender_name_in_missed_pm_using_direct_message_group(self) -> None:
+        hamlet = self.example_user("hamlet")
+        iago = self.example_user("iago")
+
+        get_or_create_direct_message_group(id_list=[hamlet.id, iago.id])
+
+        msg_id = self.send_personal_message(iago, hamlet, "Hello")
+
+        self.handle_missedmessage_emails(
+            hamlet.id,
+            {msg_id: MissedMessageData(trigger=NotificationTriggers.DIRECT_MESSAGE)},
+        )
+
+        assert isinstance(mail.outbox[0], EmailMultiAlternatives)
+        assert isinstance(mail.outbox[0].alternatives[0][0], str)
+        # Sender name is not appended for missed 1:1 direct messages
+        self.assertEqual("> Hello\n\n--\n\nReply", mail.outbox[0].body[:18])
+        self.assertIn(
+            ">\n                    \n                        <div><p>Hello</p></div>\n",
+            mail.outbox[0].alternatives[0][0],
+        )
+
+    def test_your_name_in_missed_pm_to_self_using_direct_message_group(self) -> None:
+        hamlet = self.example_user("hamlet")
+
+        get_or_create_direct_message_group(id_list=[hamlet.id])
+
+        msg_id = self.send_personal_message(hamlet, hamlet, "Hello", read_by_sender=False)
+
+        self.handle_missedmessage_emails(
+            hamlet.id,
+            {msg_id: MissedMessageData(trigger=NotificationTriggers.DIRECT_MESSAGE)},
+        )
+
+        assert isinstance(mail.outbox[0], EmailMultiAlternatives)
+        self.assertEqual(mail.outbox[0].subject, "DMs with King Hamlet")
+        assert isinstance(mail.outbox[0].alternatives[0][0], str)
+        # Sender name is not appended for missed 1:1 direct messages
+        self.assertEqual("> Hello\n\n--\n\nReply", mail.outbox[0].body[:18])
+        self.assertIn(
+            ">\n                    \n                        <div><p>Hello</p></div>\n",
+            mail.outbox[0].alternatives[0][0],
+        )
+
     def test_multiple_missed_personal_messages(self) -> None:
         hamlet = self.example_user("hamlet")
         msg_id_1 = self.send_personal_message(