user_groups: Allow adding users who are members of subgroups.

This commit updates the code, which checks if user is member of
the group before adding them to the group, to consider only
direct members and now allows members of subgroups to be added
as direct members of the group.
This commit is contained in:
Sahil Batra 2024-10-16 14:47:58 +05:30 committed by Tim Abbott
parent aa2c7bf1d1
commit a3b7d956bc
3 changed files with 23 additions and 3 deletions

View File

@ -220,7 +220,7 @@ function add_new_members({pill_user_ids}: {pill_user_ids: number[]}): void {
deactivated_users.add(user_id); deactivated_users.add(user_id);
return false; return false;
} }
if (user_groups.is_user_in_group(group.id, user_id)) { if (user_groups.is_user_in_group(group.id, user_id, true)) {
// we filter out already added users before sending // we filter out already added users before sending
// add member request as the endpoint is not so robust and // add member request as the endpoint is not so robust and
// fails complete request if any already added member // fails complete request if any already added member
@ -235,7 +235,7 @@ function add_new_members({pill_user_ids}: {pill_user_ids: number[]}): void {
if ( if (
user_id_set.has(current_user.user_id) && user_id_set.has(current_user.user_id) &&
user_groups.is_user_in_group(group.id, current_user.user_id) user_groups.is_user_in_group(group.id, current_user.user_id, true)
) { ) {
// We don't want to send a request to add ourselves if we // We don't want to send a request to add ourselves if we
// are already added to this group. This case occurs // are already added to this group. This case occurs

View File

@ -311,7 +311,11 @@ export function get_recursive_group_members(target_user_group: UserGroup): Set<n
return members; return members;
} }
export function is_user_in_group(user_group_id: number, user_id: number): boolean { export function is_user_in_group(
user_group_id: number,
user_id: number,
direct_member_only = false,
): boolean {
const user_group = user_group_by_id_dict.get(user_group_id); const user_group = user_group_by_id_dict.get(user_group_id);
if (user_group === undefined) { if (user_group === undefined) {
blueslip.error("Could not find user group", {user_group_id}); blueslip.error("Could not find user group", {user_group_id});
@ -321,6 +325,10 @@ export function is_user_in_group(user_group_id: number, user_id: number): boolea
return true; return true;
} }
if (direct_member_only) {
return false;
}
const subgroup_ids = get_recursive_subgroups(user_group); const subgroup_ids = get_recursive_subgroups(user_group);
if (subgroup_ids === undefined) { if (subgroup_ids === undefined) {
return false; return false;

View File

@ -349,6 +349,18 @@ run_test("is_user_in_group", () => {
assert.equal(user_groups.is_user_in_group(foo.id, 6), true); assert.equal(user_groups.is_user_in_group(foo.id, 6), true);
assert.equal(user_groups.is_user_in_group(foo.id, 3), false); assert.equal(user_groups.is_user_in_group(foo.id, 3), false);
// Check when passing direct_member_only as true.
assert.equal(user_groups.is_user_in_group(admins.id, 1, true), true);
assert.equal(user_groups.is_user_in_group(admins.id, 6, true), false);
assert.equal(user_groups.is_user_in_group(all.id, 2, true), true);
assert.equal(user_groups.is_user_in_group(all.id, 1, true), false);
assert.equal(user_groups.is_user_in_group(all.id, 6, true), false);
assert.equal(user_groups.is_user_in_group(test.id, 4, true), true);
assert.equal(user_groups.is_user_in_group(test.id, 1, true), false);
assert.equal(user_groups.is_user_in_group(test.id, 6, true), false);
assert.equal(user_groups.is_user_in_setting_group(test.id, 4), true); assert.equal(user_groups.is_user_in_setting_group(test.id, 4), true);
assert.equal(user_groups.is_user_in_setting_group(test.id, 1), true); assert.equal(user_groups.is_user_in_setting_group(test.id, 1), true);
assert.equal(user_groups.is_user_in_setting_group(test.id, 6), true); assert.equal(user_groups.is_user_in_setting_group(test.id, 6), true);