streams: Fix handling empty groups when computing setting values.

Since "Nobody" and other user-defined empty groups has no
members or subgroups, we need a UserGroup query.

Since we are doing UserGroup query, updated the code to
check whether the setting is set to anonymous group or
not just after doing UserGroup query and not in membership
queries.

Also, added tests to check query count when setting is set
to anonymous group.
This commit is contained in:
Sahil Batra
2024-11-22 12:15:07 +05:30
committed by Tim Abbott
parent 90e42ca8ed
commit f197d881ca
2 changed files with 69 additions and 13 deletions

View File

@@ -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=[],

View File

@@ -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,