diff --git a/frontend_tests/node_tests/unread.js b/frontend_tests/node_tests/unread.js index 9481ef1974..c58d51d251 100644 --- a/frontend_tests/node_tests/unread.js +++ b/frontend_tests/node_tests/unread.js @@ -625,6 +625,8 @@ test("server_counts", () => { page_params.unread_msgs = { pms: [ { + other_user_id: 101, + // sender_id is deprecated. sender_id: 101, unread_message_ids: [31, 32, 60, 61, 62, 63], }, diff --git a/static/js/unread.js b/static/js/unread.js index 05dca060d9..cf3c81b20c 100644 --- a/static/js/unread.js +++ b/static/js/unread.js @@ -98,7 +98,7 @@ class UnreadPMCounter { set_pms(pms) { for (const obj of pms) { - const user_ids_string = obj.sender_id.toString(); + const user_ids_string = obj.other_user_id.toString(); this.set_message_ids(user_ids_string, obj.unread_message_ids); } } diff --git a/templates/zerver/api/changelog.md b/templates/zerver/api/changelog.md index c5797a5a15..8390f7a73c 100644 --- a/templates/zerver/api/changelog.md +++ b/templates/zerver/api/changelog.md @@ -20,6 +20,14 @@ format used by the Zulip server that they are interacting with. ## Changes in Zulip 5.0 +**Feature level 119** + +* [`POST /register`](/api/register-queue): The `unread_msgs` section + of the response now prefers `other_user_id` over the poorly named + `sender_id` field in the `pms` dictionaries. This change is + motivated by the possibility that a message you yourself sent to + another user could be marked as unread. + **Feature level 118** * [`GET /messages`](/api/get-messages), [`GET diff --git a/version.py b/version.py index 18bd9cf076..8f459080b6 100644 --- a/version.py +++ b/version.py @@ -33,7 +33,7 @@ DESKTOP_WARNING_VERSION = "5.4.3" # Changes should be accompanied by documentation explaining what the # new level means in templates/zerver/api/changelog.md, as well as # "**Changes**" entries in the endpoint's documentation in `zulip.yaml`. -API_FEATURE_LEVEL = 118 +API_FEATURE_LEVEL = 119 # 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 diff --git a/zerver/lib/message.py b/zerver/lib/message.py index 322edc8b5f..b049d184c2 100644 --- a/zerver/lib/message.py +++ b/zerver/lib/message.py @@ -72,7 +72,7 @@ class RawUnreadStreamDict(TypedDict): class RawUnreadPrivateMessageDict(TypedDict): - sender_id: int + other_user_id: int class RawUnreadHuddleDict(TypedDict): @@ -96,6 +96,8 @@ class UnreadStreamInfo(TypedDict): class UnreadPrivateMessageInfo(TypedDict): + other_user_id: int + # Deprecated and misleading synonym for other_user_id sender_id: int unread_message_ids: List[int] @@ -1080,14 +1082,8 @@ def extract_unread_data_from_um_rows( else: other_user_id = sender_id - # The `sender_id` field here is misnamed. It's really - # just the other participant in a PM conversation. For - # most unread PM messages, the other user is also the sender, - # but that's not true for certain messages sent from the - # API. Unfortunately, it's difficult now to rename the - # field without breaking mobile. pm_dict[message_id] = dict( - sender_id=other_user_id, + other_user_id=other_user_id, ) elif msg_type == Recipient.HUDDLE: @@ -1148,9 +1144,13 @@ def aggregate_pms( ) -> List[UnreadPrivateMessageInfo]: lookup_dict: Dict[int, UnreadPrivateMessageInfo] = {} for message_id, attribute_dict in input_dict.items(): - other_user_id = attribute_dict["sender_id"] + other_user_id = attribute_dict["other_user_id"] if other_user_id not in lookup_dict: + # The `sender_id` field here is only supported for + # legacy mobile clients. Its actual semantics are the same + # as `other_user_id`. obj = UnreadPrivateMessageInfo( + other_user_id=other_user_id, sender_id=other_user_id, unread_message_ids=[], ) @@ -1248,13 +1248,12 @@ def apply_unread_message_event( elif message_type == "private": if len(others) == 1: - other_id = others[0]["id"] + other_user_id = others[0]["id"] else: - other_id = user_profile.id + other_user_id = user_profile.id - # The `sender_id` field here is misnamed. state["pm_dict"][message_id] = RawUnreadPrivateMessageDict( - sender_id=other_id, + other_user_id=other_user_id, ) else: diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 4e2404019c..94694049ed 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -9386,14 +9386,29 @@ paths: items: type: object description: | - Object containing the details of a unread private - message with a specific user. + Object containing the details of a unread private message with + a specific user. Note that in rare situations, it is possible + for a message that you sent to another user to be marked as + unread and thus appear here. additionalProperties: false properties: - sender_id: + other_user_id: type: integer description: | - The user id of the other participant in a PM conversation. + The user id of the other participant in this non-group private + message conversation. Will be your own user ID for messages + that you sent to only yourself. + sender_id: + deprecated: true + type: integer + description: | + Old name for `other_user_id`. Clients should access this + field in Zulip server versions that do not yet support + `other_user_id`. + + **Changes**: Deprecated in Zulip 5.0 (feature level 119). + We expect to provide a next version of the full `unread_msgs` + API before removing this legacy name. message_ids: type: array description: | diff --git a/zerver/tests/test_message_flags.py b/zerver/tests/test_message_flags.py index 493fd8706e..96ed2754c0 100644 --- a/zerver/tests/test_message_flags.py +++ b/zerver/tests/test_message_flags.py @@ -712,7 +712,7 @@ class GetUnreadMsgsTest(ZulipTestCase): self.assertEqual( pm_dict[cordelia_pm_message_ids[0]], - dict(sender_id=cordelia.id), + dict(other_user_id=cordelia.id), ) def test_raw_unread_personal_from_self(self) -> None: @@ -756,12 +756,9 @@ class GetUnreadMsgsTest(ZulipTestCase): self.assertEqual(set(pm_dict.keys()), {othello_msg.id}) - # For legacy reason we call the field `sender_id` here, - # but it really refers to the other user id in the conversation, - # which is Othello. self.assertEqual( pm_dict[othello_msg.id], - dict(sender_id=othello.id), + dict(other_user_id=othello.id), ) cordelia = self.example_user("cordelia") @@ -778,10 +775,9 @@ class GetUnreadMsgsTest(ZulipTestCase): {othello_msg.id, cordelia_msg.id}, ) - # Again, `sender_id` is misnamed here. self.assertEqual( pm_dict[cordelia_msg.id], - dict(sender_id=cordelia.id), + dict(other_user_id=cordelia.id), ) # Send a message to ourself. @@ -797,10 +793,9 @@ class GetUnreadMsgsTest(ZulipTestCase): {othello_msg.id, cordelia_msg.id, hamlet_msg.id}, ) - # Again, `sender_id` is misnamed here. self.assertEqual( pm_dict[hamlet_msg.id], - dict(sender_id=hamlet.id), + dict(other_user_id=hamlet.id), ) # Call get_raw_unread_data again. @@ -814,10 +809,9 @@ class GetUnreadMsgsTest(ZulipTestCase): {othello_msg.id, cordelia_msg.id, hamlet_msg.id}, ) - # Again, `sender_id` is misnamed here. self.assertEqual( pm_dict[hamlet_msg.id], - dict(sender_id=hamlet.id), + dict(other_user_id=hamlet.id), ) def test_unread_msgs(self) -> None: