diff --git a/zerver/lib/mention.py b/zerver/lib/mention.py index bc7c535d4f..5795b0be37 100644 --- a/zerver/lib/mention.py +++ b/zerver/lib/mention.py @@ -4,7 +4,7 @@ from dataclasses import dataclass from re import Match from django.conf import settings -from django.db.models import Q +from django.db.models import Prefetch, Q from zerver.lib.users import get_inaccessible_user_ids from zerver.models import NamedUserGroup, UserProfile @@ -269,9 +269,19 @@ class MentionData: self.user_group_members: dict[int, list[int]] = {} user_group_names = possible_user_group_mentions(content) if user_group_names: + # We are not directly doing 'prefetch_related("direct_members")' + # because then we would have to filter out deactivated users + # using 'group.direct_members.filter(is_active=True)', and that + # would take away the advantage of using 'prefetch_related' + # because of not using group.direct_members.all(). for group in NamedUserGroup.objects.filter( realm_id=realm_id, name__in=user_group_names, is_system_group=False - ).prefetch_related("direct_members"): + ).prefetch_related( + Prefetch( + "direct_members", + queryset=UserProfile.objects.filter(realm_id=realm_id, is_active=True), + ) + ): self.user_group_name_info[group.name.lower()] = group self.user_group_members[group.id] = [m.id for m in group.direct_members.all()] diff --git a/zerver/tests/test_markdown.py b/zerver/tests/test_markdown.py index 00490fdd68..3eee89c099 100644 --- a/zerver/tests/test_markdown.py +++ b/zerver/tests/test_markdown.py @@ -293,6 +293,15 @@ class MarkdownMiscTest(ZulipTestCase): mention_data = MentionData(mention_backend, content, message_sender=None) self.assertTrue(mention_data.message_has_topic_wildcards()) + content = "@*hamletcharacters*" + group = NamedUserGroup.objects.get(realm=realm, name="hamletcharacters") + mention_data = MentionData(mention_backend, content, message_sender=None) + self.assertCountEqual(mention_data.get_group_members(group.id), [hamlet.id, cordelia.id]) + + change_user_is_active(cordelia, False) + mention_data = MentionData(mention_backend, content, message_sender=None) + self.assertEqual(mention_data.get_group_members(group.id), [hamlet.id]) + def test_invalid_katex_path(self) -> None: with self.settings(DEPLOY_ROOT="/nonexistent"): with self.assertLogs(level="ERROR") as m: diff --git a/zerver/tests/test_notification_data.py b/zerver/tests/test_notification_data.py index 0c49e2deb6..a36f72bb27 100644 --- a/zerver/tests/test_notification_data.py +++ b/zerver/tests/test_notification_data.py @@ -1,4 +1,5 @@ from zerver.actions.user_groups import check_add_user_group +from zerver.actions.users import do_deactivate_user from zerver.lib.mention import MentionBackend, MentionData from zerver.lib.notification_data import UserMessageNotificationsData, get_user_group_mentions_data from zerver.lib.test_classes import ZulipTestCase @@ -457,3 +458,19 @@ class TestNotificationData(ZulipTestCase): cordelia.id: hamlet_and_cordelia.id, }, ) + + # Deactivated users are excluded. + do_deactivate_user(hamlet, acting_user=None) + result = get_user_group_mentions_data( + mentioned_user_ids=set(), + mentioned_user_group_ids=[hamlet_and_cordelia.id], + mention_data=MentionData( + mention_backend, "hey @*hamlet_and_cordelia*!", message_sender=None + ), + ) + self.assertDictEqual( + result, + { + cordelia.id: hamlet_and_cordelia.id, + }, + )