sent_by_human: Add "test suite" to the set of Zulip UI-style clients.

Earlier, when we used 'self.send_message()' in the backend tests,
the sent message was not marked as read for the sender.

Reason: To set the read flag, we have to check if
'message.sent_by_human()'. It returns False because the
'sending_client' for tests is "test suite" and the 'sent_by_human'
function doesn't enlist the "test suite" client name as a human client.

This commit adds "test suite" to that list.

Also fixes a bug in when apply_unread_message_event was called that
was revealed by this change.
This commit is contained in:
Prakhar Pratyush 2023-09-30 16:10:39 +05:30 committed by Tim Abbott
parent 68abc6af21
commit c349d1137c
7 changed files with 87 additions and 63 deletions

View File

@ -718,7 +718,7 @@ def apply_event(
) -> None: ) -> None:
if event["type"] == "message": if event["type"] == "message":
state["max_message_id"] = max(state["max_message_id"], event["message"]["id"]) state["max_message_id"] = max(state["max_message_id"], event["message"]["id"])
if "raw_unread_msgs" in state: if "raw_unread_msgs" in state and "read" not in event["flags"]:
apply_unread_message_event( apply_unread_message_event(
user_profile, user_profile,
state["raw_unread_msgs"], state["raw_unread_msgs"],

View File

@ -3111,20 +3111,26 @@ class Message(AbstractMessage):
sending_client = self.sending_client.name.lower() sending_client = self.sending_client.name.lower()
return ( return (
sending_client (
in ( sending_client
"zulipandroid", in (
"zulipios", "zulipandroid",
"zulipdesktop", "zulipios",
"zulipmobile", "zulipdesktop",
"zulipelectron", "zulipmobile",
"zulipterminal", "zulipelectron",
"snipe", "zulipterminal",
"website", "snipe",
"ios", "website",
"android", "ios",
"android",
)
) )
) or ("desktop app" in sending_client) or ("desktop app" in sending_client)
# Since the vast majority of messages are sent by humans
# in Zulip, treat test suite messages as such.
or (sending_client == "test suite" and settings.TEST_SUITE)
)
@staticmethod @staticmethod
def is_status_message(content: str, rendered_content: str) -> bool: def is_status_message(content: str, rendered_content: str) -> bool:

View File

@ -577,7 +577,8 @@ class NormalActionsTest(BaseAction):
) )
def test_stream_update_message_events(self) -> None: def test_stream_update_message_events(self) -> None:
self.send_stream_message(self.example_user("hamlet"), "Verona", "hello") iago = self.example_user("iago")
self.send_stream_message(iago, "Verona", "hello")
# Verify stream message editing - content only # Verify stream message editing - content only
message = Message.objects.order_by("-id")[0] message = Message.objects.order_by("-id")[0]
@ -666,7 +667,7 @@ class NormalActionsTest(BaseAction):
# Verify move topic to different stream. # Verify move topic to different stream.
self.subscribe(self.user_profile, "Verona") self.subscribe(self.user_profile, "Verona")
self.subscribe(self.user_profile, "Denmark") self.subscribe(self.user_profile, "Denmark")
self.send_stream_message(self.user_profile, "Verona") self.send_stream_message(iago, "Verona")
message_id = self.send_stream_message(self.user_profile, "Verona") message_id = self.send_stream_message(self.user_profile, "Verona")
message = Message.objects.get(id=message_id) message = Message.objects.get(id=message_id)
stream = get_stream("Denmark", self.user_profile.realm) stream = get_stream("Denmark", self.user_profile.realm)
@ -788,7 +789,7 @@ class NormalActionsTest(BaseAction):
check_update_message_flags_remove("events[0]", events[0]) check_update_message_flags_remove("events[0]", events[0])
personal_message = self.send_personal_message( personal_message = self.send_personal_message(
from_user=user_profile, to_user=self.example_user("cordelia"), content=content from_user=self.example_user("cordelia"), to_user=user_profile, content=content
) )
self.verify_action( self.verify_action(
partial(do_update_message_flags, user_profile, "add", "read", [personal_message]), partial(do_update_message_flags, user_profile, "add", "read", [personal_message]),

View File

@ -1124,7 +1124,7 @@ class HomeTest(ZulipTestCase):
self.send_test_message(message) self.send_test_message(message)
self.login_user(long_term_idle_user) self.login_user(long_term_idle_user)
with queries_captured() as queries: with queries_captured() as queries:
self.assertEqual(self.soft_activate_and_get_unread_count(), 2) self.assertEqual(self.soft_activate_and_get_unread_count(), 1)
query_count = len(queries) query_count = len(queries)
long_term_idle_user.refresh_from_db() long_term_idle_user.refresh_from_db()
self.assertFalse(long_term_idle_user.long_term_idle) self.assertFalse(long_term_idle_user.long_term_idle)
@ -1134,7 +1134,7 @@ class HomeTest(ZulipTestCase):
message = "Test message 2" message = "Test message 2"
self.send_test_message(message) self.send_test_message(message)
with queries_captured() as queries: with queries_captured() as queries:
self.assertEqual(self.soft_activate_and_get_unread_count(), 3) self.assertEqual(self.soft_activate_and_get_unread_count(), 2)
# Test here for query count to be at least 5 less than previous count. # Test here for query count to be at least 5 less than previous count.
# This will assure add_missing_messages() isn't repeatedly called. # This will assure add_missing_messages() isn't repeatedly called.
self.assertGreaterEqual(query_count - len(queries), 5) self.assertGreaterEqual(query_count - len(queries), 5)
@ -1156,7 +1156,7 @@ class HomeTest(ZulipTestCase):
self.send_test_message(message) self.send_test_message(message)
self.login_user(long_term_idle_user) self.login_user(long_term_idle_user)
with queries_captured() as queries: with queries_captured() as queries:
self.assertEqual(self.soft_activate_and_get_unread_count(), 4) self.assertEqual(self.soft_activate_and_get_unread_count(), 3)
query_count = len(queries) query_count = len(queries)
long_term_idle_user.refresh_from_db() long_term_idle_user.refresh_from_db()
self.assertFalse(long_term_idle_user.long_term_idle) self.assertFalse(long_term_idle_user.long_term_idle)
@ -1166,7 +1166,7 @@ class HomeTest(ZulipTestCase):
message = "Test message 4" message = "Test message 4"
self.send_test_message(message) self.send_test_message(message)
with queries_captured() as queries: with queries_captured() as queries:
self.assertEqual(self.soft_activate_and_get_unread_count(), 5) self.assertEqual(self.soft_activate_and_get_unread_count(), 4)
self.assertGreaterEqual(query_count - len(queries), 5) self.assertGreaterEqual(query_count - len(queries), 5)
idle_user_msg_list = get_user_messages(long_term_idle_user) idle_user_msg_list = get_user_messages(long_term_idle_user)
self.assertEqual(idle_user_msg_list[-1].content, message) self.assertEqual(idle_user_msg_list[-1].content, message)

View File

@ -335,7 +335,7 @@ class EditMessageTest(EditMessageTestCase):
response_dict = self.assert_json_success(result) response_dict = self.assert_json_success(result)
self.assertEqual(response_dict["raw_content"], "Personal message") self.assertEqual(response_dict["raw_content"], "Personal message")
self.assertEqual(response_dict["message"]["id"], msg_id) self.assertEqual(response_dict["message"]["id"], msg_id)
self.assertEqual(response_dict["message"]["flags"], []) self.assertEqual(response_dict["message"]["flags"], ["read"])
# Send message to web-public stream where hamlet is not subscribed. # Send message to web-public stream where hamlet is not subscribed.
# This will test case of user having no `UserMessage` but having access # This will test case of user having no `UserMessage` but having access
@ -2127,7 +2127,7 @@ class EditMessageTest(EditMessageTestCase):
[ [
{ {
"id": hamlet.id, "id": hamlet.id,
"flags": ["wildcard_mentioned"], "flags": ["read", "wildcard_mentioned"],
}, },
{ {
"id": cordelia.id, "id": cordelia.id,
@ -2181,13 +2181,19 @@ class EditMessageTest(EditMessageTestCase):
) )
message_id = self.send_stream_message(hamlet, stream_name, "Hello everyone") message_id = self.send_stream_message(hamlet, stream_name, "Hello everyone")
def notify(user_id: int) -> Dict[str, Any]: users_to_be_notified = sorted(
return { [
"id": user_id, {
"flags": ["wildcard_mentioned"], "id": hamlet.id,
} "flags": ["read", "wildcard_mentioned"],
},
users_to_be_notified = sorted(map(notify, [cordelia.id, hamlet.id]), key=itemgetter("id")) {
"id": cordelia.id,
"flags": ["wildcard_mentioned"],
},
],
key=itemgetter("id"),
)
result = self.client_patch( result = self.client_patch(
f"/json/messages/{message_id}", f"/json/messages/{message_id}",
{ {
@ -2228,7 +2234,7 @@ class EditMessageTest(EditMessageTestCase):
[ [
{ {
"id": hamlet.id, "id": hamlet.id,
"flags": ["wildcard_mentioned"], "flags": ["read", "wildcard_mentioned"],
}, },
{ {
"id": cordelia.id, "id": cordelia.id,
@ -2319,13 +2325,19 @@ class EditMessageTest(EditMessageTestCase):
self.login_user(hamlet) self.login_user(hamlet)
message_id = self.send_stream_message(hamlet, stream_name, "Hello everyone") message_id = self.send_stream_message(hamlet, stream_name, "Hello everyone")
def notify(user_id: int) -> Dict[str, Any]: users_to_be_notified = sorted(
return { [
"id": user_id, {
"flags": ["wildcard_mentioned"], "id": hamlet.id,
} "flags": ["read", "wildcard_mentioned"],
},
users_to_be_notified = sorted(map(notify, [cordelia.id, hamlet.id]), key=itemgetter("id")) {
"id": cordelia.id,
"flags": ["wildcard_mentioned"],
},
],
key=itemgetter("id"),
)
result = self.client_patch( result = self.client_patch(
f"/json/messages/{message_id}", f"/json/messages/{message_id}",
{ {

View File

@ -182,6 +182,7 @@ class UnreadCountTests(ZulipTestCase):
mock_push_notifications_enabled.assert_called() mock_push_notifications_enabled.assert_called()
# Sending a new message results in unread UserMessages being created # Sending a new message results in unread UserMessages being created
# for users other than sender.
def test_new_message(self) -> None: def test_new_message(self) -> None:
self.login("hamlet") self.login("hamlet")
content = "Test message for unset read bit" content = "Test message for unset read bit"
@ -190,8 +191,10 @@ class UnreadCountTests(ZulipTestCase):
self.assertGreater(len(user_messages), 0) self.assertGreater(len(user_messages), 0)
for um in user_messages: for um in user_messages:
self.assertEqual(um.message.content, content) self.assertEqual(um.message.content, content)
if um.user_profile.email != self.example_email("hamlet"): if um.user_profile.delivery_email != self.example_email("hamlet"):
self.assertFalse(um.flags.read) self.assertFalse(um.flags.read)
else:
self.assertTrue(um.flags.read)
def test_update_flags(self) -> None: def test_update_flags(self) -> None:
self.login("hamlet") self.login("hamlet")
@ -315,14 +318,16 @@ class UnreadCountTests(ZulipTestCase):
def test_mark_all_in_stream_read(self) -> None: def test_mark_all_in_stream_read(self) -> None:
self.login("hamlet") self.login("hamlet")
user_profile = self.example_user("hamlet") hamlet = self.example_user("hamlet")
stream = self.subscribe(user_profile, "test_stream") cordelia = self.example_user("cordelia")
self.subscribe(self.example_user("cordelia"), "test_stream") iago = self.example_user("iago")
for user in [hamlet, cordelia, iago]:
self.subscribe(user, "test_stream")
self.subscribe(user, "Denmark")
stream = get_stream("test_stream", hamlet.realm)
message_id = self.send_stream_message(self.example_user("hamlet"), "test_stream", "hello") message_id = self.send_stream_message(cordelia, "test_stream", "hello")
unrelated_message_id = self.send_stream_message( unrelated_message_id = self.send_stream_message(cordelia, "Denmark", "hello")
self.example_user("hamlet"), "Denmark", "hello"
)
with self.capture_send_event_calls(expected_num_events=1) as events: with self.capture_send_event_calls(expected_num_events=1) as events:
result = self.client_post( result = self.client_post(
@ -346,17 +351,18 @@ class UnreadCountTests(ZulipTestCase):
differences = [key for key in expected if expected[key] != event[key]] differences = [key for key in expected if expected[key] != event[key]]
self.assert_length(differences, 0) self.assert_length(differences, 0)
hamlet = self.example_user("hamlet") # cordelia sent the message and hamlet marked it as read.
um = list(UserMessage.objects.filter(message=message_id)) um = list(UserMessage.objects.filter(message=message_id))
for msg in um: for msg in um:
if msg.user_profile.email == hamlet.email: if msg.user_profile.email in [hamlet.email, cordelia.email]:
self.assertTrue(msg.flags.read) self.assertTrue(msg.flags.read)
else: else:
self.assertFalse(msg.flags.read) self.assertFalse(msg.flags.read)
# cordelia sent the message, so marked as read only for him.
unrelated_messages = list(UserMessage.objects.filter(message=unrelated_message_id)) unrelated_messages = list(UserMessage.objects.filter(message=unrelated_message_id))
for msg in unrelated_messages: for msg in unrelated_messages:
if msg.user_profile.email == hamlet.email: if msg.user_profile.email in [hamlet.email, iago.email]:
self.assertFalse(msg.flags.read) self.assertFalse(msg.flags.read)
def test_mark_all_in_invalid_stream_read(self) -> None: def test_mark_all_in_invalid_stream_read(self) -> None:
@ -384,20 +390,19 @@ class UnreadCountTests(ZulipTestCase):
def test_mark_all_in_stream_topic_read(self) -> None: def test_mark_all_in_stream_topic_read(self) -> None:
self.login("hamlet") self.login("hamlet")
user_profile = self.example_user("hamlet") hamlet = self.example_user("hamlet")
self.subscribe(user_profile, "test_stream") cordelia = self.example_user("cordelia")
for user in [hamlet, cordelia]:
self.subscribe(user, "test_stream")
self.subscribe(user, "Denmark")
message_id = self.send_stream_message( message_id = self.send_stream_message(cordelia, "test_stream", "hello", "test_topic")
self.example_user("hamlet"), "test_stream", "hello", "test_topic" unrelated_message_id = self.send_stream_message(cordelia, "Denmark", "hello", "Denmark2")
)
unrelated_message_id = self.send_stream_message(
self.example_user("hamlet"), "Denmark", "hello", "Denmark2"
)
with self.capture_send_event_calls(expected_num_events=1) as events: with self.capture_send_event_calls(expected_num_events=1) as events:
result = self.client_post( result = self.client_post(
"/json/mark_topic_as_read", "/json/mark_topic_as_read",
{ {
"stream_id": get_stream("test_stream", user_profile.realm).id, "stream_id": get_stream("test_stream", hamlet.realm).id,
"topic_name": "test_topic", "topic_name": "test_topic",
}, },
) )
@ -418,12 +423,12 @@ class UnreadCountTests(ZulipTestCase):
um = list(UserMessage.objects.filter(message=message_id)) um = list(UserMessage.objects.filter(message=message_id))
for msg in um: for msg in um:
if msg.user_profile_id == user_profile.id: if msg.user_profile_id == hamlet.id:
self.assertTrue(msg.flags.read) self.assertTrue(msg.flags.read)
unrelated_messages = list(UserMessage.objects.filter(message=unrelated_message_id)) unrelated_messages = list(UserMessage.objects.filter(message=unrelated_message_id))
for msg in unrelated_messages: for msg in unrelated_messages:
if msg.user_profile_id == user_profile.id: if msg.user_profile_id == hamlet.id:
self.assertFalse(msg.flags.read) self.assertFalse(msg.flags.read)
def test_mark_all_in_invalid_topic_read(self) -> None: def test_mark_all_in_invalid_topic_read(self) -> None:
@ -1221,7 +1226,7 @@ class MessageAccessTests(ZulipTestCase):
for msg in self.get_messages(): for msg in self.get_messages():
if msg["id"] in message_ids: if msg["id"] in message_ids:
check_flags(msg["flags"], {"starred"}) check_flags(msg["flags"], {"read", "starred"})
else: else:
check_flags(msg["flags"], {"read"}) check_flags(msg["flags"], {"read"})
@ -1231,7 +1236,7 @@ class MessageAccessTests(ZulipTestCase):
for msg in self.get_messages(): for msg in self.get_messages():
if msg["id"] in message_ids: if msg["id"] in message_ids:
check_flags(msg["flags"], set()) check_flags(msg["flags"], {"read"})
def test_change_collapsed_public_stream_historical(self) -> None: def test_change_collapsed_public_stream_historical(self) -> None:
hamlet = self.example_user("hamlet") hamlet = self.example_user("hamlet")

View File

@ -336,7 +336,7 @@ class AddNewUserHistoryTest(ZulipTestCase):
user_message = most_recent_usermessage(user_profile) user_message = most_recent_usermessage(user_profile)
self.assertEqual( self.assertEqual(
repr(user_message), repr(user_message),
f"<UserMessage: recip / {user_profile.email} ([])>", f"<UserMessage: recip / {user_profile.email} (['read'])>",
) )