unread: Replace sender_id with other_user_id.

Note that we still send sender_id for legacy mobile
clients.
This commit is contained in:
Steve Howell 2022-03-07 15:47:49 +00:00 committed by Tim Abbott
parent 822c232e37
commit 8e05a9fcf7
7 changed files with 48 additions and 30 deletions

View File

@ -625,6 +625,8 @@ test("server_counts", () => {
page_params.unread_msgs = { page_params.unread_msgs = {
pms: [ pms: [
{ {
other_user_id: 101,
// sender_id is deprecated.
sender_id: 101, sender_id: 101,
unread_message_ids: [31, 32, 60, 61, 62, 63], unread_message_ids: [31, 32, 60, 61, 62, 63],
}, },

View File

@ -98,7 +98,7 @@ class UnreadPMCounter {
set_pms(pms) { set_pms(pms) {
for (const obj of 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); this.set_message_ids(user_ids_string, obj.unread_message_ids);
} }
} }

View File

@ -20,6 +20,14 @@ format used by the Zulip server that they are interacting with.
## Changes in Zulip 5.0 ## 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** **Feature level 118**
* [`GET /messages`](/api/get-messages), [`GET * [`GET /messages`](/api/get-messages), [`GET

View File

@ -33,7 +33,7 @@ DESKTOP_WARNING_VERSION = "5.4.3"
# Changes should be accompanied by documentation explaining what the # Changes should be accompanied by documentation explaining what the
# new level means in templates/zerver/api/changelog.md, as well as # new level means in templates/zerver/api/changelog.md, as well as
# "**Changes**" entries in the endpoint's documentation in `zulip.yaml`. # "**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 # 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 # only when going from an old version of the code to a newer version. Bump

View File

@ -72,7 +72,7 @@ class RawUnreadStreamDict(TypedDict):
class RawUnreadPrivateMessageDict(TypedDict): class RawUnreadPrivateMessageDict(TypedDict):
sender_id: int other_user_id: int
class RawUnreadHuddleDict(TypedDict): class RawUnreadHuddleDict(TypedDict):
@ -96,6 +96,8 @@ class UnreadStreamInfo(TypedDict):
class UnreadPrivateMessageInfo(TypedDict): class UnreadPrivateMessageInfo(TypedDict):
other_user_id: int
# Deprecated and misleading synonym for other_user_id
sender_id: int sender_id: int
unread_message_ids: List[int] unread_message_ids: List[int]
@ -1080,14 +1082,8 @@ def extract_unread_data_from_um_rows(
else: else:
other_user_id = sender_id 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( pm_dict[message_id] = dict(
sender_id=other_user_id, other_user_id=other_user_id,
) )
elif msg_type == Recipient.HUDDLE: elif msg_type == Recipient.HUDDLE:
@ -1148,9 +1144,13 @@ def aggregate_pms(
) -> List[UnreadPrivateMessageInfo]: ) -> List[UnreadPrivateMessageInfo]:
lookup_dict: Dict[int, UnreadPrivateMessageInfo] = {} lookup_dict: Dict[int, UnreadPrivateMessageInfo] = {}
for message_id, attribute_dict in input_dict.items(): 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: 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( obj = UnreadPrivateMessageInfo(
other_user_id=other_user_id,
sender_id=other_user_id, sender_id=other_user_id,
unread_message_ids=[], unread_message_ids=[],
) )
@ -1248,13 +1248,12 @@ def apply_unread_message_event(
elif message_type == "private": elif message_type == "private":
if len(others) == 1: if len(others) == 1:
other_id = others[0]["id"] other_user_id = others[0]["id"]
else: 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( state["pm_dict"][message_id] = RawUnreadPrivateMessageDict(
sender_id=other_id, other_user_id=other_user_id,
) )
else: else:

View File

@ -9386,14 +9386,29 @@ paths:
items: items:
type: object type: object
description: | description: |
Object containing the details of a unread private Object containing the details of a unread private message with
message with a specific user. 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 additionalProperties: false
properties: properties:
sender_id: other_user_id:
type: integer type: integer
description: | 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: message_ids:
type: array type: array
description: | description: |

View File

@ -712,7 +712,7 @@ class GetUnreadMsgsTest(ZulipTestCase):
self.assertEqual( self.assertEqual(
pm_dict[cordelia_pm_message_ids[0]], 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: 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}) 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( self.assertEqual(
pm_dict[othello_msg.id], pm_dict[othello_msg.id],
dict(sender_id=othello.id), dict(other_user_id=othello.id),
) )
cordelia = self.example_user("cordelia") cordelia = self.example_user("cordelia")
@ -778,10 +775,9 @@ class GetUnreadMsgsTest(ZulipTestCase):
{othello_msg.id, cordelia_msg.id}, {othello_msg.id, cordelia_msg.id},
) )
# Again, `sender_id` is misnamed here.
self.assertEqual( self.assertEqual(
pm_dict[cordelia_msg.id], pm_dict[cordelia_msg.id],
dict(sender_id=cordelia.id), dict(other_user_id=cordelia.id),
) )
# Send a message to ourself. # Send a message to ourself.
@ -797,10 +793,9 @@ class GetUnreadMsgsTest(ZulipTestCase):
{othello_msg.id, cordelia_msg.id, hamlet_msg.id}, {othello_msg.id, cordelia_msg.id, hamlet_msg.id},
) )
# Again, `sender_id` is misnamed here.
self.assertEqual( self.assertEqual(
pm_dict[hamlet_msg.id], pm_dict[hamlet_msg.id],
dict(sender_id=hamlet.id), dict(other_user_id=hamlet.id),
) )
# Call get_raw_unread_data again. # Call get_raw_unread_data again.
@ -814,10 +809,9 @@ class GetUnreadMsgsTest(ZulipTestCase):
{othello_msg.id, cordelia_msg.id, hamlet_msg.id}, {othello_msg.id, cordelia_msg.id, hamlet_msg.id},
) )
# Again, `sender_id` is misnamed here.
self.assertEqual( self.assertEqual(
pm_dict[hamlet_msg.id], pm_dict[hamlet_msg.id],
dict(sender_id=hamlet.id), dict(other_user_id=hamlet.id),
) )
def test_unread_msgs(self) -> None: def test_unread_msgs(self) -> None: