From 2f56f8d0ed5d7cdca3a5640919972a22b974aaf8 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Bodas Date: Sat, 27 Mar 2021 18:15:49 +0530 Subject: [PATCH] mute user: Mark as read new messages. Messages sent by muted users are marked as read as soon as they are sent (or, more accurately, while creating the database entries itself), regardless of type (stream/huddle/PM). ede73ee4cda2ce995df066361bb195914a922023, makes it easy to pass a list to `do_send_messages` containing user-ids for whom the message should be marked as read. We add the contents of this list to the set of muter IDs, and then pass it on to `create_user_messages`. This benefits from the caching behaviour of `get_muting_users` and should not cause performance issues long term. The consequence is that messages sent by muted users will not contribute to unread counts and notifications. This commit does not affect the unread messages (if any) present just before muting, but only handles subsequent messages. Old unreads will be handled in further commits. --- zerver/lib/actions.py | 13 +++++++--- zerver/tests/test_message_edit.py | 4 +-- zerver/tests/test_message_send.py | 2 +- zerver/tests/test_muting_users.py | 41 ++++++++++++++++++++++++++++++- zerver/tests/test_signup.py | 4 +-- zerver/tests/test_subs.py | 8 +++--- zerver/tests/test_users.py | 4 +-- 7 files changed, 60 insertions(+), 16 deletions(-) diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index 0a25dc7a42..7bdd334ddf 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -167,7 +167,7 @@ from zerver.lib.upload import ( upload_emoji_image, ) from zerver.lib.user_groups import access_user_group_by_id, create_user_group -from zerver.lib.user_mutes import add_user_mute, get_user_mutes +from zerver.lib.user_mutes import add_user_mute, get_muting_users, get_user_mutes from zerver.lib.user_status import update_user_status from zerver.lib.users import ( check_bot_name_available, @@ -1848,6 +1848,11 @@ def do_send_messages( # Service bots (outgoing webhook bots and embedded bots) don't store UserMessage rows; # they will be processed later. mentioned_user_ids = send_request.message.mentions_user_ids + + # Extend the set with users who have muted the sender. + mark_as_read_for_users = get_muting_users(send_request.message.sender) + mark_as_read_for_users.update(mark_as_read) + user_messages = create_user_messages( message=send_request.message, um_eligible_user_ids=send_request.um_eligible_user_ids, @@ -1855,7 +1860,7 @@ def do_send_messages( stream_push_user_ids=send_request.stream_push_user_ids, stream_email_user_ids=send_request.stream_email_user_ids, mentioned_user_ids=mentioned_user_ids, - mark_as_read=mark_as_read, + mark_as_read_for_users=mark_as_read_for_users, ) for um in user_messages: @@ -2030,7 +2035,7 @@ def create_user_messages( stream_push_user_ids: AbstractSet[int], stream_email_user_ids: AbstractSet[int], mentioned_user_ids: AbstractSet[int], - mark_as_read: Sequence[int] = [], + mark_as_read_for_users: Set[int], ) -> List[UserMessageLite]: ums_to_create = [] for user_profile_id in um_eligible_user_ids: @@ -2049,7 +2054,7 @@ def create_user_messages( for um in ums_to_create: if ( um.user_profile_id == message.sender.id and message.sent_by_human() - ) or um.user_profile_id in mark_as_read: + ) or um.user_profile_id in mark_as_read_for_users: um.flags |= UserMessage.flags.read if wildcard: um.flags |= UserMessage.flags.wildcard_mentioned diff --git a/zerver/tests/test_message_edit.py b/zerver/tests/test_message_edit.py index 122202eb1d..fdccd3ce05 100644 --- a/zerver/tests/test_message_edit.py +++ b/zerver/tests/test_message_edit.py @@ -1171,8 +1171,8 @@ class EditMessageTest(ZulipTestCase): "topic": "new topic", }, ) - self.assertEqual(len(queries), 47) - self.assertEqual(len(cache_tries), 11) + self.assertEqual(len(queries), 48) + self.assertEqual(len(cache_tries), 13) messages = get_topic_messages(user_profile, old_stream, "test") self.assertEqual(len(messages), 1) diff --git a/zerver/tests/test_message_send.py b/zerver/tests/test_message_send.py index fbe03e043d..62b97be8e3 100644 --- a/zerver/tests/test_message_send.py +++ b/zerver/tests/test_message_send.py @@ -1557,7 +1557,7 @@ class StreamMessagesTest(ZulipTestCase): body=content, ) - self.assert_length(queries, 12) + self.assert_length(queries, 13) def test_stream_message_dict(self) -> None: user_profile = self.example_user("iago") diff --git a/zerver/tests/test_muting_users.py b/zerver/tests/test_muting_users.py index ac0aa2a0eb..be0000ff27 100644 --- a/zerver/tests/test_muting_users.py +++ b/zerver/tests/test_muting_users.py @@ -7,7 +7,7 @@ from zerver.lib.cache import cache_get, get_muting_users_cache_key from zerver.lib.test_classes import ZulipTestCase from zerver.lib.timestamp import datetime_to_timestamp from zerver.lib.user_mutes import get_mute_object, get_muting_users, get_user_mutes -from zerver.models import RealmAuditLog +from zerver.models import RealmAuditLog, UserMessage, UserProfile class MutedUsersTests(ZulipTestCase): @@ -185,3 +185,42 @@ class MutedUsersTests(ZulipTestCase): self.assertEqual(None, cache_get(get_muting_users_cache_key(cordelia))) self.assertEqual(set(), get_muting_users(cordelia)) self.assertEqual(set(), cache_get(get_muting_users_cache_key(cordelia))[0]) + + def assert_usermessage_read_flag(self, user: UserProfile, message: int, flag: bool) -> None: + usermesaage = UserMessage.objects.get( + user_profile=user, + message=message, + ) + self.assertTrue(usermesaage.flags.read == flag) + + def test_new_messages_from_muted_user_marked_as_read(self) -> None: + hamlet = self.example_user("hamlet") + self.login_user(hamlet) + cordelia = self.example_user("cordelia") + othello = self.example_user("othello") + + self.make_stream("general") + self.subscribe(hamlet, "general") + self.subscribe(cordelia, "general") + self.subscribe(othello, "general") + + # Hamlet mutes Cordelia. + url = "/api/v1/users/me/muted_users/{}".format(cordelia.id) + result = self.api_post(hamlet, url) + self.assert_json_success(result) + + # Have Cordelia send messages to Hamlet and Othello. + stream_message = self.send_stream_message(cordelia, "general", "Spam in stream") + huddle_message = self.send_huddle_message(cordelia, [hamlet, othello], "Spam in huddle") + pm_to_hamlet = self.send_personal_message(cordelia, hamlet, "Spam in PM") + pm_to_othello = self.send_personal_message(cordelia, othello, "Spam in PM") + + # These should be marked as read for Hamlet, since he has muted Cordelia. + self.assert_usermessage_read_flag(hamlet, stream_message, True) + self.assert_usermessage_read_flag(hamlet, huddle_message, True) + self.assert_usermessage_read_flag(hamlet, pm_to_hamlet, True) + + # These messages should be unreads for Othello, since he hasn't muted Cordelia. + self.assert_usermessage_read_flag(othello, stream_message, False) + self.assert_usermessage_read_flag(othello, huddle_message, False) + self.assert_usermessage_read_flag(othello, pm_to_othello, False) diff --git a/zerver/tests/test_signup.py b/zerver/tests/test_signup.py index 3cc0eece2f..4c37066ce7 100644 --- a/zerver/tests/test_signup.py +++ b/zerver/tests/test_signup.py @@ -746,13 +746,13 @@ class LoginTest(ZulipTestCase): with queries_captured() as queries, cache_tries_captured() as cache_tries: self.register(self.nonreg_email("test"), "test") # Ensure the number of queries we make is not O(streams) - self.assertEqual(len(queries), 70) + self.assertEqual(len(queries), 71) # We can probably avoid a couple cache hits here, but there doesn't # seem to be any O(N) behavior. Some of the cache hits are related # to sending messages, such as getting the welcome bot, looking up # the alert words for a realm, etc. - self.assertEqual(len(cache_tries), 15) + self.assertEqual(len(cache_tries), 16) user_profile = self.nonreg_user("test") self.assert_logged_in_user_id(user_profile.id) diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py index 4a2ff18385..ac10cd50e3 100644 --- a/zerver/tests/test_subs.py +++ b/zerver/tests/test_subs.py @@ -3531,7 +3531,7 @@ class SubscriptionAPITest(ZulipTestCase): streams_to_sub, dict(principals=orjson.dumps([user1.id, user2.id]).decode()), ) - self.assert_length(queries, 34) + self.assert_length(queries, 35) self.assert_length(events, 5) for ev in [x for x in events if x["event"]["type"] not in ("message", "stream")]: @@ -4421,7 +4421,7 @@ class SubscriptionAPITest(ZulipTestCase): [new_streams[0]], dict(principals=orjson.dumps([user1.id, user2.id]).decode()), ) - self.assert_length(queries, 34) + self.assert_length(queries, 35) # Test creating private stream. with queries_captured() as queries: @@ -4431,7 +4431,7 @@ class SubscriptionAPITest(ZulipTestCase): dict(principals=orjson.dumps([user1.id, user2.id]).decode()), invite_only=True, ) - self.assert_length(queries, 36) + self.assert_length(queries, 37) # Test creating a public stream with announce when realm has a notification stream. notifications_stream = get_stream(self.streams[0], self.test_realm) @@ -4446,7 +4446,7 @@ class SubscriptionAPITest(ZulipTestCase): principals=orjson.dumps([user1.id, user2.id]).decode(), ), ) - self.assert_length(queries, 42) + self.assert_length(queries, 43) class GetStreamsTest(ZulipTestCase): diff --git a/zerver/tests/test_users.py b/zerver/tests/test_users.py index 5bffe1f9e8..e6815d9467 100644 --- a/zerver/tests/test_users.py +++ b/zerver/tests/test_users.py @@ -819,8 +819,8 @@ class QueryCountTest(ZulipTestCase): acting_user=None, ) - self.assert_length(queries, 68) - self.assert_length(cache_tries, 20) + self.assert_length(queries, 70) + self.assert_length(cache_tries, 22) self.assert_length(events, 7) peer_add_events = [event for event in events if event["event"].get("op") == "peer_add"]