mirror of
https://github.com/zulip/zulip.git
synced 2025-11-04 05:53:43 +00:00
get_recursive_group_members: Use DISTINCT in the query.
Having repetitions in the result can cause subtle bugs in callers that don't expect it.
This commit is contained in:
committed by
Tim Abbott
parent
b0914167eb
commit
639972b753
@@ -831,7 +831,7 @@ def get_recursive_group_members_union_for_groups(
|
|||||||
return UserProfile.objects.filter(
|
return UserProfile.objects.filter(
|
||||||
is_active=True,
|
is_active=True,
|
||||||
direct_groups__in=get_recursive_subgroups_union_for_groups(user_group_ids),
|
direct_groups__in=get_recursive_subgroups_union_for_groups(user_group_ids),
|
||||||
)
|
).distinct()
|
||||||
|
|
||||||
|
|
||||||
def get_recursive_membership_groups(user_profile: UserProfile) -> QuerySet[UserGroup]:
|
def get_recursive_membership_groups(user_profile: UserProfile) -> QuerySet[UserGroup]:
|
||||||
|
|||||||
@@ -407,6 +407,31 @@ class UserGroupTestCase(ZulipTestCase):
|
|||||||
[desdemona, shiva, aaron, prospero],
|
[desdemona, shiva, aaron, prospero],
|
||||||
)
|
)
|
||||||
|
|
||||||
|
def test_get_user_group_member_ids_distinct(self) -> None:
|
||||||
|
"""
|
||||||
|
Verify that when fetching recursive member ids of groups
|
||||||
|
we only return distinct ids - without repetitions even if
|
||||||
|
there's an overlap between the subgroups/supergroups.
|
||||||
|
"""
|
||||||
|
realm = get_realm("zulip")
|
||||||
|
hamlet = self.example_user("hamlet")
|
||||||
|
desdemona = self.example_user("desdemona")
|
||||||
|
cordelia = self.example_user("cordelia")
|
||||||
|
|
||||||
|
supergroup = check_add_user_group(realm, "S", [hamlet], acting_user=desdemona)
|
||||||
|
|
||||||
|
subgroup1 = check_add_user_group(realm, "A", [cordelia], acting_user=desdemona)
|
||||||
|
GroupGroupMembership.objects.create(supergroup=supergroup, subgroup=subgroup1)
|
||||||
|
|
||||||
|
subgroup2 = check_add_user_group(realm, "B", [hamlet], acting_user=desdemona)
|
||||||
|
GroupGroupMembership.objects.create(supergroup=supergroup, subgroup=subgroup2)
|
||||||
|
|
||||||
|
# hamlet has both a direct membership in the supergroup, as well as a recursive membership
|
||||||
|
# via subgroup2. We make sure that despite that, hamlet.id occurs only once in the result.
|
||||||
|
self.assertEqual(
|
||||||
|
sorted(get_user_group_member_ids(supergroup)), sorted([hamlet.id, cordelia.id])
|
||||||
|
)
|
||||||
|
|
||||||
def test_subgroups_of_role_based_system_groups(self) -> None:
|
def test_subgroups_of_role_based_system_groups(self) -> None:
|
||||||
realm = get_realm("zulip")
|
realm = get_realm("zulip")
|
||||||
owners_group = NamedUserGroup.objects.get(
|
owners_group = NamedUserGroup.objects.get(
|
||||||
|
|||||||
Reference in New Issue
Block a user