diff --git a/web/src/settings_data.ts b/web/src/settings_data.ts index 094526dc95..fa6440fd81 100644 --- a/web/src/settings_data.ts +++ b/web/src/settings_data.ts @@ -162,7 +162,7 @@ export function user_can_move_messages_between_streams(): boolean { return user_has_permission(realm.realm_move_messages_between_streams_policy); } -export function user_can_edit_user_groups(): boolean { +export function user_can_edit_all_user_groups(): boolean { return user_has_permission(realm.realm_user_group_edit_policy); } @@ -171,18 +171,22 @@ export function can_edit_user_group(group_id: number): boolean { return false; } - if (!user_can_edit_user_groups()) { - return false; + let can_edit_all_user_groups = user_can_edit_all_user_groups(); + + if ( + !current_user.is_admin && + !current_user.is_moderator && + !user_groups.is_direct_member_of(current_user.user_id, group_id) + ) { + can_edit_all_user_groups = false; } - // Admins and moderators are allowed to edit user groups even if they - // are not a member of that user group. Members can edit user groups - // only if they belong to that group. - if (current_user.is_admin || current_user.is_moderator) { + if (can_edit_all_user_groups) { return true; } - return user_groups.is_direct_member_of(current_user.user_id, group_id); + const group = user_groups.get_user_group_from_id(group_id); + return user_groups.is_user_in_group(group.can_manage_group, current_user.user_id); } export function user_can_create_user_groups(): boolean { diff --git a/web/tests/settings_data.test.js b/web/tests/settings_data.test.js index 56f19ae143..7dd658788e 100644 --- a/web/tests/settings_data.test.js +++ b/web/tests/settings_data.test.js @@ -162,9 +162,9 @@ test_policy( settings_data.user_can_create_user_groups, ); test_policy( - "user_can_edit_user_groups", + "user_can_edit_all_user_groups", "realm_user_group_edit_policy", - settings_data.user_can_edit_user_groups, + settings_data.user_can_edit_all_user_groups, ); test_policy( "user_can_add_custom_emoji", @@ -335,36 +335,96 @@ run_test("user_can_create_multiuse_invite", () => { }); run_test("can_edit_user_group", () => { + const admins = { + description: "Administrators", + name: "role:administrators", + id: 1, + members: new Set([1]), + is_system_group: true, + direct_subgroup_ids: new Set([]), + can_manage_group: 4, + can_mention_group: 1, + }; + const moderators = { + description: "Moderators", + name: "role:moderators", + id: 2, + members: new Set([2]), + is_system_group: true, + direct_subgroup_ids: new Set([1]), + can_manage_group: 4, + can_mention_group: 1, + }; + const members = { + description: "Members", + name: "role:members", + id: 3, + members: new Set([3]), + is_system_group: true, + direct_subgroup_ids: new Set([1, 2]), + can_manage_group: 4, + can_mention_group: 4, + }; + const nobody = { + description: "Nobody", + name: "role:nobody", + id: 4, + members: new Set([]), + is_system_group: true, + direct_subgroup_ids: new Set([]), + can_manage_group: 4, + can_mention_group: 2, + }; const students = { description: "Students group", name: "Students", - id: 0, + id: 5, members: new Set([1, 2]), is_system_group: false, direct_subgroup_ids: new Set([4, 5]), - can_mention_group: 2, + can_manage_group: 4, + can_mention_group: 3, }; user_groups.initialize({ - realm_user_groups: [students], + realm_user_groups: [admins, moderators, members, nobody, students], }); page_params.is_spectator = true; assert.ok(!settings_data.can_edit_user_group(students.id)); page_params.is_spectator = false; + realm.realm_user_group_edit_policy = settings_config.common_policy_values.by_admins_only.code; current_user.user_id = 3; - current_user.is_guest = true; assert.ok(!settings_data.can_edit_user_group(students.id)); - current_user.is_guest = false; - current_user.is_moderator = true; + current_user.is_admin = true; assert.ok(settings_data.can_edit_user_group(students.id)); + current_user.is_admin = false; + current_user.is_moderator = true; + assert.ok(!settings_data.can_edit_user_group(students.id)); + + realm.realm_user_group_edit_policy = settings_config.common_policy_values.by_members.code; current_user.is_moderator = false; + current_user.is_guest = false; assert.ok(!settings_data.can_edit_user_group(students.id)); current_user.user_id = 2; - realm.realm_waiting_period_threshold = 0; + assert.ok(settings_data.can_edit_user_group(students.id)); + + realm.realm_user_group_edit_policy = settings_config.common_policy_values.by_admins_only.code; + assert.ok(!settings_data.can_edit_user_group(students.id)); + + const event = { + group_id: students.id, + data: { + can_manage_group: members.id, + }, + }; + user_groups.update(event); + assert.ok(settings_data.can_edit_user_group(students.id)); + + current_user.user_id = 3; assert.ok(settings_data.can_edit_user_group(students.id)); });