diff --git a/zerver/lib/user_groups.py b/zerver/lib/user_groups.py index dbc91e5994..acb31a7f28 100644 --- a/zerver/lib/user_groups.py +++ b/zerver/lib/user_groups.py @@ -831,7 +831,7 @@ def get_recursive_group_members_union_for_groups( return UserProfile.objects.filter( is_active=True, direct_groups__in=get_recursive_subgroups_union_for_groups(user_group_ids), - ) + ).distinct() def get_recursive_membership_groups(user_profile: UserProfile) -> QuerySet[UserGroup]: diff --git a/zerver/tests/test_user_groups.py b/zerver/tests/test_user_groups.py index c3c3dc77cc..1eb8a3381d 100644 --- a/zerver/tests/test_user_groups.py +++ b/zerver/tests/test_user_groups.py @@ -407,6 +407,31 @@ class UserGroupTestCase(ZulipTestCase): [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: realm = get_realm("zulip") owners_group = NamedUserGroup.objects.get(