message_cache: Use the sender’s recipient_id for incoming 1:1 DMs.

For an incoming 1:1 DM, the recipient’s own recipient_id is useless to
the recipient themselves. Substitute the sender’s recipient_id, so the
recipient can use recipient_id as documented to uniquely represent the
set of 2 users in this conversation.

Signed-off-by: Anders Kaseorg <[email protected]>

Author: andersk

api_docs/changelog.md

@@ -20,6 +20,13 @@ format used by the Zulip server that they are interacting with.
 
 ## Changes in Zulip 10.0
 
+**Feature level 327**
+
+* [`GET /messages`](/api/get-messages), [`GET
+  /messages/{message_id}`](/api/get-message), [`GET /events`](/api/get-events):
+  Adjusted the `recipient_id` field of an incoming 1:1 direct message to use the
+  same value that would be used for an outgoing message in that conversation.
+
 **Feature level 326**
 
 * [`POST /register`](/api/register-queue): Removed `allow_owners_group`

version.py

@@ -34,7 +34,7 @@
 # new level means in api_docs/changelog.md, as well as "**Changes**"
 # entries in the endpoint's documentation in `zulip.yaml`.
 
-API_FEATURE_LEVEL = 326  # Last bumped for updating fields in server_supported_permission_settings
+API_FEATURE_LEVEL = 327  # Last bumped to adjust recipient_id in incoming 1:1 DMs
 
 # Bump the minor PROVISION_VERSION to indicate that folks should provision
 # only when going from an old version of the code to a newer version. Bump

zerver/lib/message.py

@@ -253,7 +253,13 @@ def messages_for_ids(
         msg_dict["can_access_sender"] = msg_dict["sender_id"] not in inaccessible_sender_ids
         message_list.append(msg_dict)
 
-    MessageDict.post_process_dicts(message_list, apply_markdown, client_gravatar, realm)
+    MessageDict.post_process_dicts(
+        message_list,
+        apply_markdown=apply_markdown,
+        client_gravatar=client_gravatar,
+        realm=realm,
+        user_recipient_id=None if user_profile is None else user_profile.recipient_id,
+    )
 
     return message_list
 

zerver/lib/message_cache.py

@@ -175,9 +175,11 @@ def wide_dict(message: Message, realm_id: int | None = None) -> dict[str, Any]:
     @staticmethod
     def post_process_dicts(
         objs: list[dict[str, Any]],
+        *,
         apply_markdown: bool,
         client_gravatar: bool,
         realm: Realm,
+        user_recipient_id: int | None,
     ) -> None:
         """
         NOTE: This function mutates the objects in
@@ -199,6 +201,7 @@ def post_process_dicts(
                 skip_copy=True,
                 can_access_sender=can_access_sender,
                 realm_host=realm.host,
+                is_incoming_1_to_1=obj["recipient_id"] == user_recipient_id,
             )
 
     @staticmethod
@@ -211,6 +214,7 @@ def finalize_payload(
         skip_copy: bool = False,
         can_access_sender: bool,
         realm_host: str,
+        is_incoming_1_to_1: bool,
     ) -> dict[str, Any]:
         """
         By default, we make a shallow copy of the incoming dict to avoid
@@ -247,12 +251,20 @@ def finalize_payload(
         else:
             obj["content_type"] = "text/x-markdown"
 
+        if is_incoming_1_to_1:
+            # For an incoming 1:1 DM, the recipient’s own recipient_id is
+            # useless to the recipient themselves. Substitute the sender’s
+            # recipient_id, so the recipient can use recipient_id as documented
+            # to uniquely represent the set of 2 users in this conversation.
+            obj["recipient_id"] = obj["sender_recipient_id"]
+
         for item in obj.get("edit_history", []):
             if "prev_rendered_content_version" in item:
                 del item["prev_rendered_content_version"]
 
         if not keep_rendered_content:
             del obj["rendered_content"]
+        del obj["sender_recipient_id"]
         del obj["sender_realm_id"]
         del obj["sender_avatar_source"]
         del obj["sender_delivery_email"]
@@ -472,6 +484,7 @@ def bulk_hydrate_sender_info(objs: list[dict[str, Any]]) -> None:
             "full_name",
             "delivery_email",
             "email",
+            "recipient_id",
             "realm__string_id",
             "avatar_source",
             "avatar_version",
@@ -486,6 +499,7 @@ def bulk_hydrate_sender_info(objs: list[dict[str, Any]]) -> None:
         for obj in objs:
             sender_id = obj["sender_id"]
             user_row = sender_dict[sender_id]
+            obj["sender_recipient_id"] = user_row["recipient_id"]
             obj["sender_full_name"] = user_row["full_name"]
             obj["sender_email"] = user_row["email"]
             obj["sender_delivery_email"] = user_row["delivery_email"]

zerver/lib/outgoing_webhook.py

@@ -68,6 +68,7 @@ def make_request(self, base_url: str, event: dict[str, Any], realm: Realm) -> Re
                 get_user_profile_by_id(event["message"]["sender_id"]), self.user_profile
             ),
             realm_host=realm.host,
+            is_incoming_1_to_1=event["message"]["recipient_id"] == self.user_profile.recipient_id,
         )
 
         request_data = {

zerver/openapi/zulip.yaml

@@ -23510,6 +23510,11 @@ components:
             A unique ID for the set of users receiving the
             message (either a channel or group of users). Useful primarily
             for hashing.
+
+            **Changes**: Before Zulip 10.0 (feature level 327), `recipient_id`
+            was the same across all incoming 1:1 direct messages. Now, each
+            incoming message uniquely shares a `recipient_id` with outgoing
+            messages in the same conversation.
         sender_email:
           type: string
           description: |

zerver/tests/test_event_system.py

@@ -375,6 +375,7 @@ def test_get_events(self) -> None:
         self.assertEqual(events[0]["local_message_id"], local_id)
         self.assertEqual(events[0]["message"]["display_recipient"][0]["is_mirror_dummy"], False)
         self.assertEqual(events[0]["message"]["display_recipient"][1]["is_mirror_dummy"], False)
+        self.assertEqual(events[0]["message"]["recipient_id"], recipient_user_profile.recipient_id)
 
         last_event_id = events[0]["id"]
         local_id = "10.02"
@@ -407,6 +408,7 @@ def test_get_events(self) -> None:
         self.assertEqual(events[0]["type"], "message")
         self.assertEqual(events[0]["message"]["sender_email"], email)
         self.assertEqual(events[0]["local_message_id"], local_id)
+        self.assertEqual(events[0]["message"]["recipient_id"], recipient_user_profile.recipient_id)
 
         # Test that the received message in the receiver's event queue
         # exists and does not contain a local id
@@ -426,9 +428,12 @@ def test_get_events(self) -> None:
         self.assertEqual(recipient_events[0]["type"], "message")
         self.assertEqual(recipient_events[0]["message"]["sender_email"], email)
         self.assertTrue("local_message_id" not in recipient_events[0])
+        # Incoming DMs show the recipient_id that outgoing DMs would.
+        self.assertEqual(recipient_events[0]["message"]["recipient_id"], user_profile.recipient_id)
         self.assertEqual(recipient_events[1]["type"], "message")
         self.assertEqual(recipient_events[1]["message"]["sender_email"], email)
         self.assertTrue("local_message_id" not in recipient_events[1])
+        self.assertEqual(recipient_events[1]["message"]["recipient_id"], user_profile.recipient_id)
 
     def test_get_events_narrow(self) -> None:
         user_profile = self.example_user("hamlet")
@@ -861,6 +866,7 @@ def test_get_client_info_for_all_public_streams(self) -> None:
             queue_timeout=0,
             realm_id=realm.id,
             user_profile_id=hamlet.id,
+            user_recipient_id=hamlet.recipient_id,
         )
 
         client = allocate_client_descriptor(queue_data)
@@ -915,6 +921,7 @@ def test_get_info(apply_markdown: bool, client_gravatar: bool) -> None:
                 queue_timeout=0,
                 realm_id=realm.id,
                 user_profile_id=hamlet.id,
+                user_recipient_id=hamlet.recipient_id,
             )
 
             client = allocate_client_descriptor(queue_data)
@@ -959,9 +966,15 @@ def test_process_message_event_with_mocked_client_info(self) -> None:
 
         class MockClient:
             def __init__(
-                self, user_profile_id: int, apply_markdown: bool, client_gravatar: bool
+                self,
+                *,
+                user_profile_id: int,
+                user_recipient_id: int | None,
+                apply_markdown: bool,
+                client_gravatar: bool,
             ) -> None:
                 self.user_profile_id = user_profile_id
+                self.user_recipient_id = user_recipient_id
                 self.apply_markdown = apply_markdown
                 self.client_gravatar = client_gravatar
                 self.client_type_name = "whatever"
@@ -979,24 +992,28 @@ def add_event(self, event: dict[str, Any]) -> None:
 
         client1 = MockClient(
             user_profile_id=hamlet.id,
+            user_recipient_id=hamlet.recipient_id,
             apply_markdown=True,
             client_gravatar=False,
         )
 
         client2 = MockClient(
             user_profile_id=hamlet.id,
+            user_recipient_id=hamlet.recipient_id,
             apply_markdown=False,
             client_gravatar=False,
         )
 
         client3 = MockClient(
             user_profile_id=hamlet.id,
+            user_recipient_id=hamlet.recipient_id,
             apply_markdown=True,
             client_gravatar=True,
         )
 
         client4 = MockClient(
             user_profile_id=hamlet.id,
+            user_recipient_id=hamlet.recipient_id,
             apply_markdown=False,
             client_gravatar=True,
         )
@@ -1028,11 +1045,13 @@ def add_event(self, event: dict[str, Any]) -> None:
                 content="**hello**",
                 rendered_content="<b>hello</b>",
                 sender_id=sender.id,
+                recipient_id=1111,
                 type="stream",
                 client="website",
                 # NOTE: Some of these fields are clutter, but some
                 #       will be useful when we let clients specify
                 #       that they can compute their own gravatar URLs.
+                sender_recipient_id=sender.recipient_id,
                 sender_email=sender.email,
                 sender_delivery_email=sender.delivery_email,
                 sender_realm_id=sender.realm_id,
@@ -1073,6 +1092,7 @@ def add_event(self, event: dict[str, Any]) -> None:
                         sender_id=sender.id,
                         sender_email=sender.email,
                         id=999,
+                        recipient_id=1111,
                         content="<b>hello</b>",
                         content_type="text/html",
                         client="website",
@@ -1092,6 +1112,7 @@ def add_event(self, event: dict[str, Any]) -> None:
                         sender_id=sender.id,
                         sender_email=sender.email,
                         id=999,
+                        recipient_id=1111,
                         content="**hello**",
                         content_type="text/x-markdown",
                         client="website",
@@ -1112,6 +1133,7 @@ def add_event(self, event: dict[str, Any]) -> None:
                         sender_email=sender.email,
                         avatar_url=None,
                         id=999,
+                        recipient_id=1111,
                         content="<b>hello</b>",
                         content_type="text/html",
                         client="website",
@@ -1132,6 +1154,7 @@ def add_event(self, event: dict[str, Any]) -> None:
                         sender_email=sender.email,
                         avatar_url=None,
                         id=999,
+                        recipient_id=1111,
                         content="**hello**",
                         content_type="text/x-markdown",
                         client="website",
@@ -1159,6 +1182,7 @@ def test_web_reload_clients(self) -> None:
             queue_timeout=0,
             realm_id=realm.id,
             user_profile_id=hamlet.id,
+            user_recipient_id=hamlet.recipient_id,
         )
         client = allocate_client_descriptor(queue_data)
 

zerver/tests/test_message_dict.py

@@ -71,7 +71,7 @@ def reload_message(msg_id: int) -> Message:
             return msg
 
         def get_send_message_payload(
-            msg_id: int, apply_markdown: bool, client_gravatar: bool
+            msg_id: int, *, apply_markdown: bool, client_gravatar: bool
         ) -> dict[str, Any]:
             msg = reload_message(msg_id)
             wide_dict = MessageDict.wide_dict(msg)
@@ -82,11 +82,12 @@ def get_send_message_payload(
                 client_gravatar=client_gravatar,
                 can_access_sender=True,
                 realm_host=get_realm("zulip").host,
+                is_incoming_1_to_1=False,
             )
             return narrow_dict
 
         def get_fetch_payload(
-            msg_id: int, apply_markdown: bool, client_gravatar: bool
+            msg_id: int, *, apply_markdown: bool, client_gravatar: bool
         ) -> dict[str, Any]:
             msg = reload_message(msg_id)
             unhydrated_dict = MessageDict.messages_to_encoded_cache_helper([msg])[0]
@@ -97,6 +98,7 @@ def get_fetch_payload(
                 apply_markdown=apply_markdown,
                 client_gravatar=client_gravatar,
                 realm=get_realm("zulip"),
+                user_recipient_id=None,
             )
             final_dict = unhydrated_dict
             return final_dict
@@ -175,7 +177,11 @@ def test_bulk_message_fetching(self) -> None:
         with self.assert_database_query_count(7):
             objs = MessageDict.ids_to_dict(ids)
             MessageDict.post_process_dicts(
-                objs, apply_markdown=False, client_gravatar=False, realm=realm
+                objs,
+                apply_markdown=False,
+                client_gravatar=False,
+                realm=realm,
+                user_recipient_id=None,
             )
 
         self.assert_length(objs, num_ids)

zerver/tests/test_message_edit.py

@@ -171,18 +171,31 @@ def test_save_message(self) -> None:
         self.assertEqual(Message.objects.get(id=msg_id).topic_name(), "edited")
 
     def test_fetch_message_from_id(self) -> None:
-        self.login("hamlet")
+        hamlet = self.example_user("hamlet")
+        cordelia = self.example_user("cordelia")
+        self.login_user(hamlet)
+
         msg_id = self.send_personal_message(
-            from_user=self.example_user("hamlet"),
-            to_user=self.example_user("cordelia"),
-            content="Personal message",
+            from_user=hamlet, to_user=cordelia, content="Outgoing direct message"
         )
         result = self.client_get("/json/messages/" + str(msg_id))
         response_dict = self.assert_json_success(result)
-        self.assertEqual(response_dict["raw_content"], "Personal message")
+        self.assertEqual(response_dict["raw_content"], "Outgoing direct message")
         self.assertEqual(response_dict["message"]["id"], msg_id)
+        self.assertEqual(response_dict["message"]["recipient_id"], cordelia.recipient_id)
         self.assertEqual(response_dict["message"]["flags"], ["read"])
 
+        msg_id = self.send_personal_message(
+            from_user=cordelia, to_user=hamlet, content="Incoming direct message"
+        )
+        result = self.client_get("/json/messages/" + str(msg_id))
+        response_dict = self.assert_json_success(result)
+        self.assertEqual(response_dict["raw_content"], "Incoming direct message")
+        self.assertEqual(response_dict["message"]["id"], msg_id)
+        # Incoming DMs show the recipient_id that outgoing DMs would.
+        self.assertEqual(response_dict["message"]["recipient_id"], cordelia.recipient_id)
+        self.assertEqual(response_dict["message"]["flags"], [])
+
         # Send message to web-public stream where hamlet is not subscribed.
         # This will test case of user having no `UserMessage` but having access
         # to message.

zerver/tests/test_message_fetch.py

@@ -4089,6 +4089,7 @@ def test_message_without_rendered_content(self) -> None:
             client_gravatar=False,
             can_access_sender=True,
             realm_host=get_realm("zulip").host,
+            is_incoming_1_to_1=False,
         )
         self.assertEqual(final_dict["content"], "<p>test content</p>")
 
@@ -4884,6 +4885,24 @@ def test_get_messages_with_search_using_email(self) -> None:
             '@<span class="highlight">Othello</span>, the Moor of Venice</span>?</p>',
         )
 
+    def test_dm_recipient_id(self) -> None:
+        hamlet = self.example_user("hamlet")
+        othello = self.example_user("othello")
+        self.login_user(hamlet)
+
+        outgoing_message_id = self.send_personal_message(hamlet, othello)
+        incoming_message_id = self.send_personal_message(othello, hamlet)
+
+        result = self.get_and_check_messages(dict(anchor="newest", num_before=2))
+        self.assert_length(result["messages"], 2)
+        self.assertEqual(result["messages"][0]["id"], outgoing_message_id)
+        self.assertEqual(result["messages"][0]["sender_id"], hamlet.id)
+        self.assertEqual(result["messages"][0]["recipient_id"], othello.recipient_id)
+        self.assertEqual(result["messages"][1]["id"], incoming_message_id)
+        self.assertEqual(result["messages"][1]["sender_id"], othello.id)
+        # Incoming DMs show the recipient_id that outgoing DMs would.
+        self.assertEqual(result["messages"][1]["recipient_id"], othello.recipient_id)
+
 
 class MessageHasKeywordsTest(ZulipTestCase):
     """Test for keywords like has_link, has_image, has_attachment."""

zerver/tests/test_message_send.py

@@ -1750,6 +1750,7 @@ def test_stream_message_dict(self) -> None:
             apply_markdown=True,
             client_gravatar=False,
             realm=user_profile.realm,
+            user_recipient_id=None,
         )
         self.assertEqual(dct["display_recipient"], "Denmark")
 

zerver/tests/test_outgoing_webhook_system.py

@@ -53,7 +53,9 @@ def mock_event(self, bot_user: UserProfile) -> dict[str, Any]:
             "message": {
                 "display_recipient": "Verona",
                 "stream_id": 999,
+                "recipient_id": 1111,
                 "sender_id": bot_user.id,
+                "sender_recipient_id": bot_user.recipient_id,
                 "sender_email": bot_user.email,
                 "sender_realm_id": bot_user.realm.id,
                 "sender_realm_str": bot_user.realm.string_id,