From 621eb1f6100769bac914ae8d54a162ae0e92db66 Mon Sep 17 00:00:00 2001 From: Prakhar Pratyush Date: Thu, 19 Dec 2024 11:13:41 +0530 Subject: [PATCH] prior_mention_user_ids: Exclude user who no longer has message access. This commit updates the 'get_mentions_for_message_updates' function to use the generic 'event_recipient_ids_for_action_on_messages' function to determine users having access to the message and perform an intersection with the mentioned users to filter out the users who no longer can access the message. It helps to add hardening such that if the invariant "no usermessage row corresponding to a message exists if the user loses access to the message" is violated due to some bug, it has minimal user impact. --- zerver/actions/message_edit.py | 11 +++++++---- zerver/tests/test_message_edit.py | 3 ++- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/zerver/actions/message_edit.py b/zerver/actions/message_edit.py index 839cd4d669..fff81e3cba 100644 --- a/zerver/actions/message_edit.py +++ b/zerver/actions/message_edit.py @@ -310,7 +310,7 @@ def send_message_moved_breadcrumbs( ) -def get_mentions_for_message_updates(message_id: int) -> set[int]: +def get_mentions_for_message_updates(message: Message) -> set[int]: # We exclude UserMessage.flags.historical rows since those # users did not receive the message originally, and thus # probably are not relevant for reprocessed alert_words, @@ -318,7 +318,7 @@ def get_mentions_for_message_updates(message_id: int) -> set[int]: # decision we change in the future. mentioned_user_ids = ( UserMessage.objects.filter( - message=message_id, + message=message.id, flags=~UserMessage.flags.historical, ) .filter( @@ -331,7 +331,10 @@ def get_mentions_for_message_updates(message_id: int) -> set[int]: ) .values_list("user_profile_id", flat=True) ) - return set(mentioned_user_ids) + + user_ids_having_message_access = event_recipient_ids_for_action_on_messages([message]) + + return set(mentioned_user_ids) & user_ids_having_message_access def update_user_message_flags( @@ -1410,7 +1413,7 @@ def check_update_message( content=message_edit_request.content, message_sender=message.sender, ) - prior_mention_user_ids = get_mentions_for_message_updates(message.id) + prior_mention_user_ids = get_mentions_for_message_updates(message) # We render the message using the current user's realm; since # the cross-realm bots never edit messages, this should be diff --git a/zerver/tests/test_message_edit.py b/zerver/tests/test_message_edit.py index b8ae8fa4f7..9974733811 100644 --- a/zerver/tests/test_message_edit.py +++ b/zerver/tests/test_message_edit.py @@ -694,8 +694,9 @@ class EditMessageTest(ZulipTestCase): msg_id = self.send_stream_message( hamlet, "Denmark", content="@**Cordelia, Lear's daughter**" ) + message = Message.objects.get(id=msg_id) - mention_user_ids = get_mentions_for_message_updates(msg_id) + mention_user_ids = get_mentions_for_message_updates(message) self.assertEqual(mention_user_ids, {cordelia.id}) def test_edit_cases(self) -> None: