From b0ef76bf276fecbf3dff605fdd85610af6b7075e Mon Sep 17 00:00:00 2001 From: Prakhar Pratyush Date: Thu, 19 Oct 2023 21:41:53 +0530 Subject: [PATCH] topic_mentions: Set 'topic_wildcard_mentioned' flag on @topic mention. Earlier, the 'wildcard_mentioned' flag was set for both the stream and topic wildcard mentions. Now, the 'topic_wildcard_mentioned' flag is set for topic wildcard mentions, and the 'wildcard_mentioned' flag is set for stream wildcard mentions. We will rename the 'wildcard_mentioned' flag to 'stream_wildcard_mentioned' in a later commit. --- zerver/actions/message_edit.py | 11 +++++++++-- zerver/actions/message_send.py | 2 +- zerver/lib/message.py | 9 +++++++-- zerver/lib/narrow.py | 10 +++++++--- zerver/lib/notification_data.py | 8 ++++---- zerver/tests/test_message_edit.py | 4 ++-- zerver/tests/test_message_fetch.py | 12 ++++++++---- zerver/tests/test_message_flags.py | 27 +++++++++++++++++++++++++++ zerver/tests/test_message_send.py | 10 +++++----- 9 files changed, 70 insertions(+), 23 deletions(-) diff --git a/zerver/actions/message_edit.py b/zerver/actions/message_edit.py index 801351b615..d7d768bc00 100644 --- a/zerver/actions/message_edit.py +++ b/zerver/actions/message_edit.py @@ -247,7 +247,14 @@ def get_mentions_for_message_updates(message_id: int) -> Set[int]: message=message_id, flags=~UserMessage.flags.historical, ) - .filter(Q(flags__andnz=UserMessage.flags.mentioned | UserMessage.flags.wildcard_mentioned)) + .filter( + Q( + flags__andnz=UserMessage.flags.mentioned + | UserMessage.flags.wildcard_mentioned + | UserMessage.flags.topic_wildcard_mentioned + | UserMessage.flags.group_mentioned + ) + ) .values_list("user_profile_id", flat=True) ) return set(mentioned_user_ids) @@ -283,7 +290,7 @@ def update_user_message_flags( update_flag(um, True, UserMessage.flags.wildcard_mentioned) elif rendering_result.mentions_topic_wildcard: topic_wildcard_mentioned = um.user_profile_id in topic_participant_user_ids - update_flag(um, topic_wildcard_mentioned, UserMessage.flags.wildcard_mentioned) + update_flag(um, topic_wildcard_mentioned, UserMessage.flags.topic_wildcard_mentioned) for um in changed_ums: um.save(update_fields=["flags"]) diff --git a/zerver/actions/message_send.py b/zerver/actions/message_send.py index 40491f6ea4..d46002f0dc 100644 --- a/zerver/actions/message_send.py +++ b/zerver/actions/message_send.py @@ -772,7 +772,7 @@ def create_user_messages( rendering_result.mentions_topic_wildcard and user_profile_id in topic_participant_user_ids ): - flags |= UserMessage.flags.wildcard_mentioned + flags |= UserMessage.flags.topic_wildcard_mentioned if ( user_profile_id in long_term_idle_user_ids diff --git a/zerver/lib/message.py b/zerver/lib/message.py index 2ebc5038db..304f74c1f9 100644 --- a/zerver/lib/message.py +++ b/zerver/lib/message.py @@ -1190,9 +1190,12 @@ def extract_unread_data_from_um_rows( # TODO: Add support for alert words here as well. is_mentioned = (row["flags"] & UserMessage.flags.mentioned) != 0 is_wildcard_mentioned = (row["flags"] & UserMessage.flags.wildcard_mentioned) != 0 + is_topic_wildcard_mentioned = ( + row["flags"] & UserMessage.flags.topic_wildcard_mentioned + ) != 0 if is_mentioned: mentions.add(message_id) - if is_wildcard_mentioned: + if is_wildcard_mentioned or is_topic_wildcard_mentioned: if msg_type == Recipient.STREAM: stream_id = row["message__recipient__type_id"] topic = row[MESSAGE__TOPIC] @@ -1369,7 +1372,9 @@ def apply_unread_message_event( if "mentioned" in flags: state["mentions"].add(message_id) - if "wildcard_mentioned" in flags and message_id in state["unmuted_stream_msgs"]: + if ( + "wildcard_mentioned" in flags or "topic_wildcard_mentioned" in flags + ) and message_id in state["unmuted_stream_msgs"]: state["mentions"].add(message_id) diff --git a/zerver/lib/narrow.py b/zerver/lib/narrow.py index 4f261bc0e3..7d0efead5a 100644 --- a/zerver/lib/narrow.py +++ b/zerver/lib/narrow.py @@ -362,9 +362,13 @@ class NarrowBuilder: cond = column("flags", Integer).op("&")(UserMessage.flags.read.mask) == 0 return query.where(maybe_negate(cond)) elif operand == "mentioned": - cond1 = column("flags", Integer).op("&")(UserMessage.flags.mentioned.mask) != 0 - cond2 = column("flags", Integer).op("&")(UserMessage.flags.wildcard_mentioned.mask) != 0 - cond = or_(cond1, cond2) + mention_flags_mask = ( + UserMessage.flags.mentioned.mask + | UserMessage.flags.wildcard_mentioned.mask + | UserMessage.flags.topic_wildcard_mentioned.mask + | UserMessage.flags.group_mentioned.mask + ) + cond = column("flags", Integer).op("&")(mention_flags_mask) != 0 return query.where(maybe_negate(cond)) elif operand == "alerted": cond = column("flags", Integer).op("&")(UserMessage.flags.has_alert_word.mask) != 0 diff --git a/zerver/lib/notification_data.py b/zerver/lib/notification_data.py index 71560dea02..24d39c21c0 100644 --- a/zerver/lib/notification_data.py +++ b/zerver/lib/notification_data.py @@ -106,7 +106,7 @@ class UserMessageNotificationsData: topic_wildcard_mention_email_notify = ( user_id in topic_wildcard_mention_user_ids and user_id not in dm_mention_email_disabled_user_ids - and "wildcard_mentioned" in flags + and "topic_wildcard_mentioned" in flags ) stream_wildcard_mention_email_notify = ( user_id in stream_wildcard_mention_user_ids @@ -116,7 +116,7 @@ class UserMessageNotificationsData: topic_wildcard_mention_in_followed_topic_email_notify = ( user_id in topic_wildcard_mention_in_followed_topic_user_ids and user_id not in dm_mention_email_disabled_user_ids - and "wildcard_mentioned" in flags + and "topic_wildcard_mentioned" in flags ) stream_wildcard_mention_in_followed_topic_email_notify = ( user_id in stream_wildcard_mention_in_followed_topic_user_ids @@ -131,7 +131,7 @@ class UserMessageNotificationsData: topic_wildcard_mention_push_notify = ( user_id in topic_wildcard_mention_user_ids and user_id not in dm_mention_push_disabled_user_ids - and "wildcard_mentioned" in flags + and "topic_wildcard_mentioned" in flags ) stream_wildcard_mention_push_notify = ( user_id in stream_wildcard_mention_user_ids @@ -141,7 +141,7 @@ class UserMessageNotificationsData: topic_wildcard_mention_in_followed_topic_push_notify = ( user_id in topic_wildcard_mention_in_followed_topic_user_ids and user_id not in dm_mention_push_disabled_user_ids - and "wildcard_mentioned" in flags + and "topic_wildcard_mentioned" in flags ) stream_wildcard_mention_in_followed_topic_push_notify = ( user_id in stream_wildcard_mention_in_followed_topic_user_ids diff --git a/zerver/tests/test_message_edit.py b/zerver/tests/test_message_edit.py index 5d6f2cdf96..496b23a3d9 100644 --- a/zerver/tests/test_message_edit.py +++ b/zerver/tests/test_message_edit.py @@ -2128,7 +2128,7 @@ class EditMessageTest(EditMessageTestCase): [ { "id": hamlet.id, - "flags": ["read", "wildcard_mentioned"], + "flags": ["read", "topic_wildcard_mentioned"], }, { "id": cordelia.id, @@ -2235,7 +2235,7 @@ class EditMessageTest(EditMessageTestCase): [ { "id": hamlet.id, - "flags": ["read", "wildcard_mentioned"], + "flags": ["read", "topic_wildcard_mentioned"], }, { "id": cordelia.id, diff --git a/zerver/tests/test_message_fetch.py b/zerver/tests/test_message_fetch.py index f954b8f49f..f99a75a384 100644 --- a/zerver/tests/test_message_fetch.py +++ b/zerver/tests/test_message_fetch.py @@ -247,12 +247,16 @@ class NarrowBuilderTest(ZulipTestCase): self._do_add_term_test(term, where_clause, params) term = dict(operator="is", operand="mentioned", negated=True) - where_clause = "WHERE NOT ((flags & %(flags_1)s) != %(param_1)s OR (flags & %(flags_2)s) != %(param_2)s)" + where_clause = "WHERE (flags & %(flags_1)s) = %(param_1)s" + mention_flags_mask = ( + UserMessage.flags.mentioned.mask + | UserMessage.flags.wildcard_mentioned.mask + | UserMessage.flags.topic_wildcard_mentioned.mask + | UserMessage.flags.group_mentioned.mask + ) params = dict( - flags_1=UserMessage.flags.mentioned.mask, + flags_1=mention_flags_mask, param_1=0, - flags_2=UserMessage.flags.wildcard_mentioned.mask, - param_2=0, ) self._do_add_term_test(term, where_clause, params) diff --git a/zerver/tests/test_message_flags.py b/zerver/tests/test_message_flags.py index 2e7371f22a..e1ed8b43fd 100644 --- a/zerver/tests/test_message_flags.py +++ b/zerver/tests/test_message_flags.py @@ -1083,6 +1083,11 @@ class GetUnreadMsgsTest(ZulipTestCase): result = get_unread_data() self.assertEqual(result["mentions"], [stream_message_id]) + um.flags = UserMessage.flags.topic_wildcard_mentioned + um.save() + result = get_unread_data() + self.assertEqual(result["mentions"], [stream_message_id]) + um.flags = 0 um.save() result = get_unread_data() @@ -1093,6 +1098,7 @@ class GetUnreadMsgsTest(ZulipTestCase): user_profile_id=user_profile.id, message_id=muted_stream_message_id, ) + # personal mention takes precedence over mutedness in a muted stream um.flags = UserMessage.flags.mentioned um.save() result = get_unread_data() @@ -1103,11 +1109,18 @@ class GetUnreadMsgsTest(ZulipTestCase): result = get_unread_data() self.assertEqual(result["mentions"], []) + # wildcard mentions don't take precedence over mutedness in + # a normal or muted topic within a muted stream um.flags = UserMessage.flags.wildcard_mentioned um.save() result = get_unread_data() self.assertEqual(result["mentions"], []) + um.flags = UserMessage.flags.topic_wildcard_mentioned + um.save() + result = get_unread_data() + self.assertEqual(result["mentions"], []) + um.flags = 0 um.save() result = get_unread_data() @@ -1118,11 +1131,18 @@ class GetUnreadMsgsTest(ZulipTestCase): user_profile_id=user_profile.id, message_id=unmuted_topic_muted_stream_message_id, ) + # wildcard mentions take precedence over mutedness in an unmuted + # or a followed topic within a muted stream. um.flags = UserMessage.flags.wildcard_mentioned um.save() result = get_unread_data() self.assertEqual(result["mentions"], [unmuted_topic_muted_stream_message_id]) + um.flags = UserMessage.flags.topic_wildcard_mentioned + um.save() + result = get_unread_data() + self.assertEqual(result["mentions"], [unmuted_topic_muted_stream_message_id]) + um.flags = 0 um.save() result = get_unread_data() @@ -1133,6 +1153,7 @@ class GetUnreadMsgsTest(ZulipTestCase): user_profile_id=user_profile.id, message_id=muted_topic_message_id, ) + # personal mention takes precedence over mutedness in a muted topic um.flags = UserMessage.flags.mentioned um.save() result = get_unread_data() @@ -1143,11 +1164,17 @@ class GetUnreadMsgsTest(ZulipTestCase): result = get_unread_data() self.assertEqual(result["mentions"], []) + # wildcard mentions don't take precedence over mutedness in a muted topic. um.flags = UserMessage.flags.wildcard_mentioned um.save() result = get_unread_data() self.assertEqual(result["mentions"], []) + um.flags = UserMessage.flags.topic_wildcard_mentioned + um.save() + result = get_unread_data() + self.assertEqual(result["mentions"], []) + um.flags = 0 um.save() result = get_unread_data() diff --git a/zerver/tests/test_message_send.py b/zerver/tests/test_message_send.py index 81d8d140bf..82a07f9a7b 100644 --- a/zerver/tests/test_message_send.py +++ b/zerver/tests/test_message_send.py @@ -1911,8 +1911,8 @@ class StreamMessagesTest(ZulipTestCase): self.send_and_verify_stream_wildcard_mention_message("iago", test_fails=True) self.send_and_verify_stream_wildcard_mention_message("iago", sub_count=10) - def test_wildcard_mentioned_flag_topic_wildcard_mention(self) -> None: - # For topic wildcard mentions, the 'wildcard_mentioned' flag should be + def test_topic_wildcard_mentioned_flag(self) -> None: + # For topic wildcard mentions, the 'topic_wildcard_mentioned' flag should be # set for all the user messages for topic participants, irrespective of # their notifications settings. cordelia = self.example_user("cordelia") @@ -1934,7 +1934,7 @@ class StreamMessagesTest(ZulipTestCase): self.assertTrue( UserMessage.objects.get( user_profile=cordelia, message=message - ).flags.wildcard_mentioned.is_set + ).flags.topic_wildcard_mentioned.is_set ) self.send_stream_message(hamlet, "Denmark", content="test", topic_name="topic-2") @@ -1944,7 +1944,7 @@ class StreamMessagesTest(ZulipTestCase): self.assertTrue( UserMessage.objects.get( user_profile=hamlet, message=message - ).flags.wildcard_mentioned.is_set + ).flags.topic_wildcard_mentioned.is_set ) do_change_user_setting(iago, "wildcard_mentions_notify", True, acting_user=None) @@ -1953,7 +1953,7 @@ class StreamMessagesTest(ZulipTestCase): self.assertFalse( UserMessage.objects.get( user_profile=iago, message=message - ).flags.wildcard_mentioned.is_set + ).flags.topic_wildcard_mentioned.is_set ) def test_invalid_wildcard_mention_policy(self) -> None: