diff --git a/zerver/lib/email_notifications.py b/zerver/lib/email_notifications.py index 8dfcec7ca7..ef8eb6a99e 100644 --- a/zerver/lib/email_notifications.py +++ b/zerver/lib/email_notifications.py @@ -642,7 +642,7 @@ def handle_missedmessage_emails( for msg_list in messages_by_bucket.values(): msg = min(msg_list, key=lambda msg: msg.date_sent) - if msg.is_stream_message(): + if msg.is_stream_message() and UserMessage.has_any_mentions(user_profile_id, msg.id): context_messages = get_context_for_message(msg) filtered_context_messages = bulk_access_messages(user_profile, context_messages) msg_list.extend(filtered_context_messages) diff --git a/zerver/models.py b/zerver/models.py index 20c53c1887..c464cc34fa 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -3662,6 +3662,20 @@ class UserMessage(AbstractUserMessage): """ return UserMessage.objects.select_for_update().order_by("message_id") + @staticmethod + def has_any_mentions(user_profile_id: int, message_id: int) -> bool: + # The query uses the 'zerver_usermessage_any_mentioned_message_id' index. + return UserMessage.objects.filter( + Q( + flags__andnz=UserMessage.flags.mentioned.mask + | UserMessage.flags.wildcard_mentioned.mask + | UserMessage.flags.topic_wildcard_mentioned.mask + | UserMessage.flags.group_mentioned.mask + ), + user_profile_id=user_profile_id, + message_id=message_id, + ).exists() + def get_usermessage_by_message_id( user_profile: UserProfile, message_id: int diff --git a/zerver/tests/test_message_notification_emails.py b/zerver/tests/test_message_notification_emails.py index f545ac02aa..77a18298a0 100644 --- a/zerver/tests/test_message_notification_emails.py +++ b/zerver/tests/test_message_notification_emails.py @@ -216,18 +216,36 @@ class TestMessageNotificationEmails(ZulipTestCase): ) def _extra_context_in_missed_stream_messages_topic_wildcard_mention_in_followed_topic( - self, show_message_content: bool = True + self, + show_message_content: bool = True, + *, + receiver_is_participant: bool, ) -> None: - for i in range(1, 6): + for i in range(1, 3): self.send_stream_message(self.example_user("othello"), "Denmark", content=str(i)) self.send_stream_message(self.example_user("othello"), "Denmark", "11", topic_name="test2") + + if receiver_is_participant: + self.send_stream_message(self.example_user("hamlet"), "Denmark", content="hello") + msg_id = self.send_stream_message(self.example_user("othello"), "Denmark", "@**topic**") + trigger = NotificationTriggers.TOPIC_WILDCARD_MENTION_IN_FOLLOWED_TOPIC + if not receiver_is_participant: + trigger = NotificationTriggers.STREAM_EMAIL if show_message_content: - verify_body_include = [ - "Othello, the Moor of Venice: > 1 > 2 > 3 > 4 > 5 > @**topic** -- ", - "You are receiving this because all topic participants were mentioned in #Denmark > test.", - ] + # If Hamlet (receiver) is not a topic participant, @topic doesn't mention him, + # so he won't receive added context (previous messages) in the email. + if receiver_is_participant: + verify_body_include = [ + "Othello, the Moor of Venice: > 1 > 2 King Hamlet: > hello Othello, the Moor of Venice: > @**topic** -- ", + "You are receiving this because all topic participants were mentioned in #Denmark > test.", + ] + else: + verify_body_include = [ + "Othello, the Moor of Venice: > @**topic** -- ", + "You are receiving this because you have email notifications enabled for #Denmark.", + ] email_subject = "#Denmark > test" verify_body_does_not_include: List[str] = [] else: @@ -252,7 +270,7 @@ class TestMessageNotificationEmails(ZulipTestCase): email_subject, show_message_content=show_message_content, verify_body_does_not_include=verify_body_does_not_include, - trigger=NotificationTriggers.TOPIC_WILDCARD_MENTION_IN_FOLLOWED_TOPIC, + trigger=trigger, ) def _extra_context_in_missed_stream_messages_stream_wildcard_mention_in_followed_topic( @@ -297,18 +315,36 @@ class TestMessageNotificationEmails(ZulipTestCase): ) def _extra_context_in_missed_stream_messages_topic_wildcard_mention( - self, show_message_content: bool = True + self, + show_message_content: bool = True, + *, + receiver_is_participant: bool, ) -> None: - for i in range(1, 6): + for i in range(1, 3): self.send_stream_message(self.example_user("othello"), "Denmark", content=str(i)) self.send_stream_message(self.example_user("othello"), "Denmark", "11", topic_name="test2") + + if receiver_is_participant: + self.send_stream_message(self.example_user("hamlet"), "Denmark", content="hello") + msg_id = self.send_stream_message(self.example_user("othello"), "denmark", "@**topic**") + trigger = NotificationTriggers.TOPIC_WILDCARD_MENTION + if not receiver_is_participant: + trigger = NotificationTriggers.STREAM_EMAIL if show_message_content: - verify_body_include = [ - "Othello, the Moor of Venice: > 1 > 2 > 3 > 4 > 5 > @**topic** -- ", - "You are receiving this because all topic participants were mentioned in #Denmark > test.", - ] + # If Hamlet (receiver) is not a topic participant, @topic doesn't mention him, + # so he won't receive added context (previous messages) in the email. + if receiver_is_participant: + verify_body_include = [ + "Othello, the Moor of Venice: > 1 > 2 King Hamlet: > hello Othello, the Moor of Venice: > @**topic** -- ", + "You are receiving this because all topic participants were mentioned in #Denmark > test.", + ] + else: + verify_body_include = [ + "Othello, the Moor of Venice: > @**topic** -- ", + "You are receiving this because you have email notifications enabled for #Denmark.", + ] email_subject = "#Denmark > test" verify_body_does_not_include: List[str] = [] else: @@ -333,7 +369,7 @@ class TestMessageNotificationEmails(ZulipTestCase): email_subject, show_message_content=show_message_content, verify_body_does_not_include=verify_body_does_not_include, - trigger=NotificationTriggers.TOPIC_WILDCARD_MENTION, + trigger=trigger, ) def _extra_context_in_missed_stream_messages_stream_wildcard_mention( @@ -383,7 +419,7 @@ class TestMessageNotificationEmails(ZulipTestCase): self.send_stream_message(self.example_user("othello"), "Denmark", "11", topic_name="test2") msg_id = self.send_stream_message(self.example_user("othello"), "denmark", "12") verify_body_include = [ - "Othello, the Moor of Venice: > 1 > 2 > 3 > 4 > 5 > 6 > 7 > 8 > 9 > 10 > 12 -- ", + "Othello, the Moor of Venice: > 12 -- ", "You are receiving this because you have email notifications enabled for #Denmark.", ] email_subject = "#Denmark > test" @@ -432,7 +468,7 @@ class TestMessageNotificationEmails(ZulipTestCase): self.assert_json_success(self.resolve_topic_containing_message(othello_user, msg_id)) verify_body_include = [ - "Othello, the Moor of Venice: > 0 > 1 > 2 -- ", + "Othello, the Moor of Venice: > 2 -- ", "You are receiving this because you have email notifications enabled for #Denmark.", ] email_subject = "[resolved] #Denmark > threading and so forth" @@ -987,7 +1023,8 @@ class TestMessageNotificationEmails(ZulipTestCase): self._extra_context_in_missed_stream_messages_mention(show_message_content=False) mail.outbox = [] self._extra_context_in_missed_stream_messages_topic_wildcard_mention_in_followed_topic( - show_message_content=False + show_message_content=False, + receiver_is_participant=True, ) mail.outbox = [] self._extra_context_in_missed_stream_messages_stream_wildcard_mention_in_followed_topic( @@ -995,7 +1032,8 @@ class TestMessageNotificationEmails(ZulipTestCase): ) mail.outbox = [] self._extra_context_in_missed_stream_messages_topic_wildcard_mention( - show_message_content=False + show_message_content=False, + receiver_is_participant=True, ) mail.outbox = [] self._extra_context_in_missed_stream_messages_stream_wildcard_mention( @@ -1014,7 +1052,16 @@ class TestMessageNotificationEmails(ZulipTestCase): def test_extra_context_in_missed_stream_messages_topic_wildcard_in_followed_topic( self, ) -> None: - self._extra_context_in_missed_stream_messages_topic_wildcard_mention_in_followed_topic() + self._extra_context_in_missed_stream_messages_topic_wildcard_mention_in_followed_topic( + receiver_is_participant=True + ) + + def test_extra_context_in_missed_stream_messages_topic_wildcard_in_followed_topic_receiver_not_participant( + self, + ) -> None: + self._extra_context_in_missed_stream_messages_topic_wildcard_mention_in_followed_topic( + receiver_is_participant=False + ) def test_extra_context_in_missed_stream_messages_stream_wildcard_in_followed_topic( self, @@ -1022,7 +1069,16 @@ class TestMessageNotificationEmails(ZulipTestCase): self._extra_context_in_missed_stream_messages_stream_wildcard_mention_in_followed_topic() def test_extra_context_in_missed_stream_messages_topic_wildcard(self) -> None: - self._extra_context_in_missed_stream_messages_topic_wildcard_mention() + self._extra_context_in_missed_stream_messages_topic_wildcard_mention( + receiver_is_participant=True + ) + + def test_extra_context_in_missed_stream_messages_topic_wildcard_receiver_not_participant( + self, + ) -> None: + self._extra_context_in_missed_stream_messages_topic_wildcard_mention( + receiver_is_participant=False + ) def test_extra_context_in_missed_stream_messages_stream_wildcard(self) -> None: self._extra_context_in_missed_stream_messages_stream_wildcard_mention()