From 1862c3b333d7fc6a61c2c2a6a719a0effa4a7fc3 Mon Sep 17 00:00:00 2001 From: Shubham Padia Date: Mon, 14 Oct 2024 11:29:10 +0000 Subject: [PATCH] user_groups: Use can_leave_group setting to check permisison. This commit adds code to use can_leave_group setting when checking permission to leave group in webapp. --- web/src/settings_data.ts | 10 ++ web/src/state_data.ts | 1 + web/src/types.ts | 1 + web/src/user_group_edit.js | 10 +- web/src/user_groups.ts | 7 ++ web/tests/composebox_typeahead.test.js | 3 + web/tests/lib/events.js | 1 + web/tests/settings_data.test.js | 123 +++++++++++++++++++++++++ web/tests/user_groups.test.js | 4 + 9 files changed, 157 insertions(+), 3 deletions(-) diff --git a/web/src/settings_data.ts b/web/src/settings_data.ts index 466be06ca1..f36f60bab4 100644 --- a/web/src/settings_data.ts +++ b/web/src/settings_data.ts @@ -231,6 +231,16 @@ export function can_join_user_group(group_id: number): boolean { return can_manage_user_group(group_id); } +export function can_leave_user_group(group_id: number): boolean { + const group = user_groups.get_user_group_from_id(group_id); + + if (user_has_permission_for_group_setting(group.can_leave_group, "can_leave_group", "group")) { + return true; + } + + return can_manage_user_group(group_id); +} + export function user_can_create_user_groups(): boolean { return user_has_permission_for_group_setting( realm.realm_can_create_groups, diff --git a/web/src/state_data.ts b/web/src/state_data.ts index 86c742e40d..27e8dae8f7 100644 --- a/web/src/state_data.ts +++ b/web/src/state_data.ts @@ -147,6 +147,7 @@ export const user_group_schema = z.object({ direct_subgroup_ids: z.array(z.number()), can_add_members_group: group_setting_value_schema, can_join_group: group_setting_value_schema, + can_leave_group: group_setting_value_schema, can_manage_group: group_setting_value_schema, can_mention_group: z.number(), deactivated: z.boolean(), diff --git a/web/src/types.ts b/web/src/types.ts index 9c9a40d965..911687f94e 100644 --- a/web/src/types.ts +++ b/web/src/types.ts @@ -18,6 +18,7 @@ export type UserGroupUpdateEvent = { description?: string; can_add_members_group?: number; can_join_group?: number; + can_leave_group?: number; can_manage_group?: number; can_mention_group?: number; deactivated?: boolean; diff --git a/web/src/user_group_edit.js b/web/src/user_group_edit.js index 37d8db9382..2daee7c5b0 100644 --- a/web/src/user_group_edit.js +++ b/web/src/user_group_edit.js @@ -272,7 +272,7 @@ function update_group_membership_button(group_id) { } const can_join_group = settings_data.can_join_user_group(group_id); - const can_leave_group = settings_data.can_manage_user_group(group_id); + const can_leave_group = settings_data.can_leave_user_group(group_id); let can_update_membership = true; if (!is_member && !can_join_group) { @@ -339,7 +339,7 @@ export function handle_member_edit_event(group_id, user_ids) { const item = group; item.is_member = user_groups.is_user_in_group(group_id, people.my_current_user_id()); item.can_join = settings_data.can_join_user_group(item.id); - item.can_leave = settings_data.can_manage_user_group(item.id); + item.can_leave = settings_data.can_leave_user_group(item.id); const html = render_browse_user_groups_list_item(item); const $new_row = $(html); @@ -670,6 +670,10 @@ export function update_group(event) { sync_group_permission_setting("can_join_group", group); update_group_membership_button(group.id); } + if (event.data.can_leave_group !== undefined) { + sync_group_permission_setting("can_leave_group", group); + update_group_membership_button(group.id); + } } } @@ -883,7 +887,7 @@ export function setup_page(callback) { item.id, ); item.can_join = settings_data.can_join_user_group(item.id); - item.can_leave = settings_data.can_manage_user_group(item.id); + item.can_leave = settings_data.can_leave_user_group(item.id); return render_browse_user_groups_list_item(item); }, filter: { diff --git a/web/src/user_groups.ts b/web/src/user_groups.ts index ae907f31af..5a5e1876c3 100644 --- a/web/src/user_groups.ts +++ b/web/src/user_groups.ts @@ -57,6 +57,7 @@ export function add(user_group_raw: UserGroupRaw): UserGroup { direct_subgroup_ids: new Set(user_group_raw.direct_subgroup_ids), can_add_members_group: user_group_raw.can_add_members_group, can_join_group: user_group_raw.can_join_group, + can_leave_group: user_group_raw.can_leave_group, can_manage_group: user_group_raw.can_manage_group, can_mention_group: user_group_raw.can_mention_group, deactivated: user_group_raw.deactivated, @@ -126,6 +127,12 @@ export function update(event: UserGroupUpdateEvent): void { user_group_name_dict.delete(group.name); user_group_name_dict.set(group.name, group); } + + if (event.data.can_leave_group !== undefined) { + group.can_leave_group = event.data.can_leave_group; + user_group_name_dict.delete(group.name); + user_group_name_dict.set(group.name, group); + } } export function get_user_group_from_name(name: string): UserGroup | undefined { diff --git a/web/tests/composebox_typeahead.test.js b/web/tests/composebox_typeahead.test.js index f466c559b3..655ffb42af 100644 --- a/web/tests/composebox_typeahead.test.js +++ b/web/tests/composebox_typeahead.test.js @@ -450,6 +450,7 @@ const hamletcharacters = user_group_item({ direct_subgroup_ids: new Set([]), can_add_members_group: 2, can_join_group: 2, + can_leave_group: 2, can_manage_group: 2, can_mention_group: 2, deactivated: false, @@ -466,6 +467,7 @@ const backend = user_group_item({ direct_subgroup_ids: new Set([1]), can_add_members_group: 1, can_join_group: 1, + can_leave_group: 2, can_manage_group: 1, can_mention_group: 1, deactivated: false, @@ -482,6 +484,7 @@ const call_center = user_group_item({ direct_subgroup_ids: new Set([]), can_add_members_group: 2, can_join_group: 2, + can_leave_group: 2, can_manage_group: 2, can_mention_group: 2, deactivated: false, diff --git a/web/tests/lib/events.js b/web/tests/lib/events.js index a3490ba0af..4282bbe852 100644 --- a/web/tests/lib/events.js +++ b/web/tests/lib/events.js @@ -841,6 +841,7 @@ exports.fixtures = { direct_subgroup_ids: [2], can_add_members_group: 16, can_join_group: 16, + can_leave_group: 15, can_manage_group: 16, can_mention_group: 11, deactivated: false, diff --git a/web/tests/settings_data.test.js b/web/tests/settings_data.test.js index 2d2bba18cd..0e3c90a36a 100644 --- a/web/tests/settings_data.test.js +++ b/web/tests/settings_data.test.js @@ -587,6 +587,129 @@ run_test("can_join_user_group", ({override}) => { assert.ok(settings_data.can_join_user_group(students.id)); }); +run_test("can_leave_user_group", ({override}) => { + const admins = { + description: "Administrators", + name: "role:administrators", + id: 1, + members: new Set([1]), + is_system_group: true, + direct_subgroup_ids: new Set([]), + can_join_group: 4, + can_leave_group: 4, + 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_join_group: 4, + can_leave_group: 4, + can_manage_group: 4, + can_mention_group: 1, + }; + const members = { + description: "Members", + name: "role:members", + id: 3, + members: new Set([3, 4]), + is_system_group: true, + direct_subgroup_ids: new Set([1, 2]), + can_join_group: 4, + can_leave_group: 4, + 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_join_group: 4, + can_leave_group: 4, + can_manage_group: 4, + can_mention_group: 2, + }; + const students = { + description: "Students group", + name: "Students", + id: 5, + members: new Set([1, 2]), + is_system_group: false, + direct_subgroup_ids: new Set([4, 5]), + can_join_group: 1, + can_leave_group: 1, + can_manage_group: { + direct_members: [4], + direct_subgroups: [], + }, + can_mention_group: 3, + creator_id: 4, + }; + user_groups.initialize({ + realm_user_groups: [admins, moderators, members, nobody, students], + }); + override(realm, "realm_can_manage_all_groups", nobody.id); + + page_params.is_spectator = true; + assert.ok(!settings_data.can_leave_user_group(students.id)); + + page_params.is_spectator = false; + // admin user + override(current_user, "user_id", 1); + assert.ok(settings_data.can_leave_user_group(students.id)); + + // moderator user + override(current_user, "user_id", 2); + assert.ok(!settings_data.can_leave_user_group(students.id)); + + let event = { + group_id: students.id, + data: { + can_leave_group: moderators.id, + }, + }; + user_groups.update(event); + assert.ok(settings_data.can_leave_user_group(students.id)); + + override(current_user, "user_id", 1); + assert.ok(settings_data.can_leave_user_group(students.id)); + + // Some other user. + override(current_user, "user_id", 5); + assert.ok(!settings_data.can_leave_user_group(students.id)); + + event = { + group_id: students.id, + data: { + can_leave_group: { + direct_members: [5], + direct_subgroups: [admins.id], + }, + }, + }; + user_groups.update(event); + assert.ok(settings_data.can_leave_user_group(students.id)); + + override(current_user, "user_id", 2); + assert.ok(!settings_data.can_leave_user_group(students.id)); + + // User can leave the group if they can manage the group which + // depends on can_manage_group and realm.can_manage_all_groups settings. + override(current_user, "user_id", 4); + assert.ok(settings_data.can_leave_user_group(students.id)); + + override(realm, "realm_can_manage_all_groups", moderators.id); + override(current_user, "user_id", 2); + assert.ok(settings_data.can_leave_user_group(students.id)); +}); + run_test("can_add_members_user_group", () => { const admins = { description: "Administrators", diff --git a/web/tests/user_groups.test.js b/web/tests/user_groups.test.js index 1def9647b4..668c87c197 100644 --- a/web/tests/user_groups.test.js +++ b/web/tests/user_groups.test.js @@ -24,6 +24,7 @@ run_test("user_groups", () => { direct_subgroup_ids: new Set([4, 5]), can_add_members_group: 1, can_join_group: 1, + can_leave_group: 1, can_manage_group: 1, can_mention_group: 2, deactivated: false, @@ -48,6 +49,7 @@ run_test("user_groups", () => { direct_subgroup_ids: new Set([]), can_add_members_group: 1, can_join_group: 1, + can_leave_group: 1, can_manage_group: 1, can_mention_group: 2, deactivated: false, @@ -59,6 +61,7 @@ run_test("user_groups", () => { is_system_group: false, direct_subgroup_ids: new Set([4, 5, 6]), can_join_group: 1, + can_leave_group: 1, can_manage_group: 1, can_mention_group: 1, deactivated: false, @@ -70,6 +73,7 @@ run_test("user_groups", () => { is_system_group: false, direct_subgroup_ids: new Set([4, 5, 6]), can_join_group: 1, + can_leave_group: 1, can_manage_group: 1, can_mention_group: 1, deactivated: true,