diff --git a/zerver/lib/streams.py b/zerver/lib/streams.py index 78fd87443b..57015e8eda 100644 --- a/zerver/lib/streams.py +++ b/zerver/lib/streams.py @@ -1033,32 +1033,34 @@ def get_group_setting_value_dict_for_streams( def get_setting_values_for_group_settings( group_ids: list[int], ) -> dict[int, int | AnonymousSettingGroupDict]: + user_groups = UserGroup.objects.filter(id__in=group_ids).select_related("named_user_group") + + setting_groups_dict: dict[int, int | AnonymousSettingGroupDict] = dict() + anonymous_group_ids = [] + for group in user_groups: + if hasattr(group, "named_user_group"): + setting_groups_dict[group.id] = group.id + else: + anonymous_group_ids.append(group.id) + user_members = ( - UserGroupMembership.objects.filter(user_group_id__in=group_ids) + UserGroupMembership.objects.filter(user_group_id__in=anonymous_group_ids) .annotate( member_type=Value("user"), - is_named_group=Q(user_group__named_user_group__isnull=False), ) - .values_list("member_type", "is_named_group", "user_group_id", "user_profile_id") + .values_list("member_type", "user_group_id", "user_profile_id") ) group_subgroups = ( - GroupGroupMembership.objects.filter(supergroup_id__in=group_ids) + GroupGroupMembership.objects.filter(supergroup_id__in=anonymous_group_ids) .annotate( member_type=Value("group"), - is_named_group=Q(supergroup__named_user_group__isnull=False), ) - .values_list("member_type", "is_named_group", "supergroup_id", "subgroup_id") + .values_list("member_type", "supergroup_id", "subgroup_id") ) all_members = user_members.union(group_subgroups) - setting_groups_dict: dict[int, int | AnonymousSettingGroupDict] = dict() - for member_type, is_named_group, group_id, member_id in all_members: - if is_named_group: - if group_id not in setting_groups_dict: - setting_groups_dict[group_id] = group_id - continue - + for member_type, group_id, member_id in all_members: if group_id not in setting_groups_dict: setting_groups_dict[group_id] = AnonymousSettingGroupDict( direct_members=[], diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py index d676588f7a..6ebee9a17c 100644 --- a/zerver/tests/test_subs.py +++ b/zerver/tests/test_subs.py @@ -99,6 +99,7 @@ from zerver.lib.test_helpers import ( reset_email_visibility_to_everyone_in_zulip_realm, ) from zerver.lib.types import ( + AnonymousSettingGroupDict, APIStreamDict, APISubscriptionDict, NeverSubscribedStreamDict, @@ -6213,6 +6214,59 @@ class GetSubscribersTest(ZulipTestCase): continue self.assert_length(sub["subscribers"], len(users_to_subscribe)) + # Test query count when setting is set to anonymous group. + realm = hamlet.realm + stream = get_stream("stream_1", realm) + admins_group = NamedUserGroup.objects.get( + name=SystemGroups.ADMINISTRATORS, realm=realm, is_system_group=True + ) + setting_group = self.create_or_update_anonymous_group_for_setting([hamlet], [admins_group]) + do_change_stream_group_based_setting( + stream, + "can_remove_subscribers_group", + setting_group, + acting_user=None, + ) + stream = get_stream("stream_2", realm) + setting_group = self.create_or_update_anonymous_group_for_setting( + [cordelia], + [admins_group], + ) + do_change_stream_group_based_setting( + stream, + "can_remove_subscribers_group", + setting_group, + acting_user=None, + ) + + with self.assert_database_query_count(6): + subscribed_streams, _ = gather_subscriptions( + self.user_profile, include_subscribers=True + ) + self.assertGreaterEqual(len(subscribed_streams), 11) + for sub in subscribed_streams: + if not sub["name"].startswith("stream_"): + continue + self.assert_length(sub["subscribers"], len(users_to_subscribe)) + if sub["name"] == "stream_1": + self.assertEqual( + sub["can_remove_subscribers_group"], + AnonymousSettingGroupDict( + direct_members=[hamlet.id], + direct_subgroups=[admins_group.id], + ), + ) + elif sub["name"] == "stream_2": + self.assertEqual( + sub["can_remove_subscribers_group"], + AnonymousSettingGroupDict( + direct_members=[cordelia.id], + direct_subgroups=[admins_group.id], + ), + ) + else: + self.assertEqual(sub["can_remove_subscribers_group"], admins_group.id) + def test_never_subscribed_streams(self) -> None: """ Check never_subscribed streams are fetched correctly and not include invite_only streams,