From 9278a219eaa2452ab93c752bb6f8432a43434b69 Mon Sep 17 00:00:00 2001 From: Sahil Batra Date: Mon, 10 Apr 2023 16:26:36 +0530 Subject: [PATCH] user_groups: Refactor get_realm_user_groups_for_dropdown_list_widget. This commit refactors get_realm_user_groups_for_dropdown_list_widget function to use "group_permission_settings" module instead of passing variables like "allow_internet_group", etc. individually for each settings. We lose some test coverage due to this commit, but that would be fixed when we add some more group-based settings and allow groups other than system groups in the group based settings. --- tools/test-js-with-node | 2 +- web/src/stream_edit.js | 4 +- web/src/stream_settings_ui.js | 4 +- web/src/user_groups.ts | 23 +++++++++--- web/tests/user_groups.test.js | 69 ++++------------------------------- 5 files changed, 33 insertions(+), 69 deletions(-) diff --git a/tools/test-js-with-node b/tools/test-js-with-node index 0170616940..75c029558c 100755 --- a/tools/test-js-with-node +++ b/tools/test-js-with-node @@ -91,7 +91,6 @@ EXEMPT_FILES = make_set( "web/src/gear_menu.ts", "web/src/giphy.js", "web/src/global.d.ts", - "web/src/group_permission_settings.ts", "web/src/hash_util.js", "web/src/hashchange.js", "web/src/hbs.d.ts", @@ -215,6 +214,7 @@ EXEMPT_FILES = make_set( "web/src/user_group_edit.js", "web/src/user_group_edit_members.js", "web/src/user_group_ui_updates.js", + "web/src/user_groups.ts", "web/src/user_groups_settings_ui.js", "web/src/user_profile.js", "web/src/user_settings.ts", diff --git a/web/src/stream_edit.js b/web/src/stream_edit.js index 3f4183c99a..4d8c9efe67 100644 --- a/web/src/stream_edit.js +++ b/web/src/stream_edit.js @@ -210,7 +210,9 @@ export function show_settings_for(node) { const opts = { widget_name: "can_remove_subscribers_group_id", - data: user_groups.get_realm_user_groups_for_dropdown_list_widget(true, true, true), + data: user_groups.get_realm_user_groups_for_dropdown_list_widget( + "can_remove_subscribers_group", + ), default_text: $t({defaultMessage: "No user groups"}), include_current_item: false, value: sub.can_remove_subscribers_group_id, diff --git a/web/src/stream_settings_ui.js b/web/src/stream_settings_ui.js index badaaa480f..f30a664e68 100644 --- a/web/src/stream_settings_ui.js +++ b/web/src/stream_settings_ui.js @@ -660,7 +660,9 @@ export function setup_page(callback) { const opts = { widget_name: "new_stream_can_remove_subscribers_group_id", - data: user_groups.get_realm_user_groups_for_dropdown_list_widget(true, true, true), + data: user_groups.get_realm_user_groups_for_dropdown_list_widget( + "can_remove_subscribers_group", + ), default_text: $t({defaultMessage: "No user groups"}), include_current_item: false, value: user_groups.get_user_group_from_name("@role:administrators").id, diff --git a/web/src/user_groups.ts b/web/src/user_groups.ts index 453b9fcbb4..9c7d95c1b1 100644 --- a/web/src/user_groups.ts +++ b/web/src/user_groups.ts @@ -1,5 +1,6 @@ import * as blueslip from "./blueslip"; import {FoldDict} from "./fold_dict"; +import * as group_permission_settings from "./group_permission_settings"; import * as settings_config from "./settings_config"; import type {User, UserGroupUpdateEvent} from "./types"; @@ -203,17 +204,29 @@ export function is_user_in_group(user_group_id: number, user_id: number): boolea } export function get_realm_user_groups_for_dropdown_list_widget( - require_system_group: boolean, - exclude_internet_group: boolean, - exclude_owners_group: boolean, + setting_name: string, ): UserGroupForDropdownListWidget[] { + const group_setting_config = + group_permission_settings.get_group_permission_setting_config(setting_name); + + if (group_setting_config === undefined) { + return []; + } + + const {require_system_group, allow_internet_group, allow_owners_group, allow_nobody_group} = + group_setting_config; + const system_user_groups = settings_config.system_user_groups_list .filter((group) => { - if (exclude_internet_group && group.name === "@role:internet") { + if (!allow_internet_group && group.name === "@role:internet") { return false; } - if (exclude_owners_group && group.name === "@role:owners") { + if (!allow_owners_group && group.name === "@role:owners") { + return false; + } + + if (!allow_nobody_group && group.name === "@role:nobody") { return false; } diff --git a/web/tests/user_groups.test.js b/web/tests/user_groups.test.js index 2097eddc3e..d2418d2e0a 100644 --- a/web/tests/user_groups.test.js +++ b/web/tests/user_groups.test.js @@ -308,21 +308,12 @@ run_test("get_realm_user_groups_for_dropdown_list_widget", () => { direct_subgroup_ids: new Set([4, 5]), }; - assert.throws( - () => user_groups.get_realm_user_groups_for_dropdown_list_widget(true, false, false), - { - name: "Error", - message: "Unknown group name: @role:internet", - }, - ); - - let expected_groups_list = [ + const expected_groups_list = [ {name: "translated: Admins, moderators, members and guests", value: "5"}, {name: "translated: Admins, moderators and members", value: "4"}, {name: "translated: Admins, moderators and full members", value: "6"}, {name: "translated: Admins and moderators", value: "3"}, {name: "translated: Admins", value: "2"}, - {name: "translated: Owners", value: "1"}, ]; user_groups.initialize({ @@ -339,59 +330,15 @@ run_test("get_realm_user_groups_for_dropdown_list_widget", () => { }); assert.deepEqual( - user_groups.get_realm_user_groups_for_dropdown_list_widget(true, true, false), + user_groups.get_realm_user_groups_for_dropdown_list_widget("can_remove_subscribers_group"), expected_groups_list, ); - expected_groups_list = [ - {name: "translated: Everyone on the internet", value: "7"}, - {name: "translated: Admins, moderators, members and guests", value: "5"}, - {name: "translated: Admins, moderators and members", value: "4"}, - {name: "translated: Admins, moderators and full members", value: "6"}, - {name: "translated: Admins and moderators", value: "3"}, - {name: "translated: Admins", value: "2"}, - ]; - assert.deepEqual( - user_groups.get_realm_user_groups_for_dropdown_list_widget(true, false, true), - expected_groups_list, - ); - - expected_groups_list = [ - {name: "translated: Admins, moderators, members and guests", value: "5"}, - {name: "translated: Admins, moderators and members", value: "4"}, - {name: "translated: Admins, moderators and full members", value: "6"}, - {name: "translated: Admins and moderators", value: "3"}, - {name: "translated: Admins", value: "2"}, - ]; - assert.deepEqual( - user_groups.get_realm_user_groups_for_dropdown_list_widget(true, true, true), - expected_groups_list, - ); - - expected_groups_list = [ - {name: "translated: Everyone on the internet", value: "7"}, - {name: "translated: Admins, moderators, members and guests", value: "5"}, - {name: "translated: Admins, moderators and members", value: "4"}, - {name: "translated: Admins, moderators and full members", value: "6"}, - {name: "translated: Admins and moderators", value: "3"}, - {name: "translated: Admins", value: "2"}, - {name: "translated: Owners", value: "1"}, - ]; - assert.deepEqual( - user_groups.get_realm_user_groups_for_dropdown_list_widget(true, false, false), - expected_groups_list, - ); - - expected_groups_list = [ - {name: "translated: Admins, moderators, members and guests", value: "5"}, - {name: "translated: Admins, moderators and members", value: "4"}, - {name: "translated: Admins, moderators and full members", value: "6"}, - {name: "translated: Admins and moderators", value: "3"}, - {name: "translated: Admins", value: "2"}, - {name: "Students", value: "8"}, - ]; - assert.deepEqual( - user_groups.get_realm_user_groups_for_dropdown_list_widget(false, true, true), - expected_groups_list, + assert.throws( + () => user_groups.get_realm_user_groups_for_dropdown_list_widget("invalid_setting"), + { + name: "Error", + message: "Invalid setting: invalid_setting", + }, ); });