bug fix: Fix unread counts for certain API messages.

If I send a message from a normal Zulip client, it is
considered to be "read" by me.  But if I send it via
an API program (using my human account), the message
is not immediately "read" by me.

Now we handle this correctly in `get_raw_unread_data`.

The symptom of this was that these messages would get
"stuck" in "Private Messages" narrows until the next
time you reloaded your app.
This commit is contained in:
Steve Howell 2020-03-17 22:17:12 +00:00 committed by Tim Abbott
parent e916d0b733
commit ca74cd6e37
2 changed files with 127 additions and 4 deletions

View File

@ -853,8 +853,19 @@ def get_raw_unread_data(user_profile: UserProfile) -> RawUnreadMessagesResult:
unmuted_stream_msgs.add(message_id) unmuted_stream_msgs.add(message_id)
elif msg_type == Recipient.PERSONAL: elif msg_type == Recipient.PERSONAL:
if sender_id == user_profile.id:
other_user_id = row['message__recipient__type_id']
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( pm_dict[message_id] = dict(
sender_id=sender_id, sender_id=other_user_id,
) )
elif msg_type == Recipient.HUDDLE: elif msg_type == Recipient.HUDDLE:
@ -940,7 +951,7 @@ def apply_unread_message_event(user_profile: UserProfile,
elif message['type'] == 'private': elif message['type'] == 'private':
others = [ others = [
recip for recip in message['display_recipient'] recip for recip in message['display_recipient']
if recip['id'] != message['sender_id'] if recip['id'] != user_profile.id
] ]
if len(others) <= 1: if len(others) <= 1:
message_type = 'private' message_type = 'private'
@ -967,9 +978,14 @@ def apply_unread_message_event(user_profile: UserProfile,
state['unmuted_stream_msgs'].add(message_id) state['unmuted_stream_msgs'].add(message_id)
elif message_type == 'private': elif message_type == 'private':
sender_id = message['sender_id'] if len(others) == 1:
other_id = others[0]['id']
else:
other_id = user_profile.id
# The `sender_id` field here is misnamed.
new_row = dict( new_row = dict(
sender_id=sender_id, sender_id=other_id,
) )
state['pm_dict'][message_id] = new_row state['pm_dict'][message_id] = new_row

View File

@ -112,8 +112,10 @@ from zerver.lib.events import (
) )
from zerver.lib.message import ( from zerver.lib.message import (
aggregate_unread_data, aggregate_unread_data,
apply_unread_message_event,
get_raw_unread_data, get_raw_unread_data,
render_markdown, render_markdown,
MessageDict,
UnreadMessagesResult, UnreadMessagesResult,
) )
from zerver.lib.test_helpers import POSTRequestMock, get_subscription, \ from zerver.lib.test_helpers import POSTRequestMock, get_subscription, \
@ -3065,6 +3067,111 @@ class GetUnreadMsgsTest(ZulipTestCase):
dict(sender_id=cordelia.id), dict(sender_id=cordelia.id),
) )
def test_raw_unread_personal_from_self(self) -> None:
hamlet = self.example_user('hamlet')
def send_unread_pm(other_user: UserProfile) -> Message:
# It is rare to send a message from Hamlet to Othello
# (or any other user) and have it be unread for
# Hamlet himself, but that is actually normal
# behavior for most API clients.
message_id = self.send_personal_message(
from_user=hamlet,
to_user=other_user,
sending_client_name='some_api_program',
)
# Check our test setup is correct--the message should
# not have looked like it was sent by a human.
message = Message.objects.get(id=message_id)
self.assertFalse(message.sent_by_human())
# And since it was not sent by a human, it should not
# be read, not even by the sender (Hamlet).
um = UserMessage.objects.get(
user_profile_id=hamlet.id,
message_id=message_id,
)
self.assertFalse(um.flags.read)
return message
othello = self.example_user('othello')
othello_msg = send_unread_pm(other_user=othello)
# And now check the unread data structure...
raw_unread_data = get_raw_unread_data(
user_profile=hamlet,
)
pm_dict = raw_unread_data['pm_dict']
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),
)
cordelia = self.example_user('cordelia')
cordelia_msg = send_unread_pm(other_user=cordelia)
apply_unread_message_event(
user_profile=hamlet,
state=raw_unread_data,
message=MessageDict.wide_dict(cordelia_msg),
flags=[],
)
self.assertEqual(
set(pm_dict.keys()),
{othello_msg.id, cordelia_msg.id}
)
# Again, `sender_id` is misnamed here.
self.assertEqual(
pm_dict[cordelia_msg.id],
dict(sender_id=cordelia.id),
)
# Send a message to ourself.
hamlet_msg = send_unread_pm(other_user=hamlet)
apply_unread_message_event(
user_profile=hamlet,
state=raw_unread_data,
message=MessageDict.wide_dict(hamlet_msg),
flags=[],
)
self.assertEqual(
set(pm_dict.keys()),
{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),
)
# Call get_raw_unread_data again.
raw_unread_data = get_raw_unread_data(
user_profile=hamlet,
)
pm_dict = raw_unread_data['pm_dict']
self.assertEqual(
set(pm_dict.keys()),
{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),
)
def test_unread_msgs(self) -> None: def test_unread_msgs(self) -> None:
sender = self.example_user('cordelia') sender = self.example_user('cordelia')
sender_id = sender.id sender_id = sender.id