From 189718dc646ff2d8f58c016e89bc7856405a5913 Mon Sep 17 00:00:00 2001 From: Sahil Batra Date: Thu, 23 Nov 2023 14:44:35 +0530 Subject: [PATCH] settings: Add support to change user-access setting in development. This commit updates the backend code to allow changing can_access_all_users_group setting in development environment and also adds a dropdown in webapp UI which is only shown in development environment. --- web/src/settings_components.js | 7 +++++ web/src/settings_org.js | 28 +++++++++++++++++++ web/src/types.ts | 1 + web/src/user_groups.ts | 5 ++++ web/styles/settings.css | 3 +- .../organization_permissions_admin.hbs | 7 +++++ web/tests/user_groups.test.js | 27 +++++++++++++++++- zerver/models.py | 2 +- zerver/tests/test_realm.py | 24 ++++++++++++++++ zerver/views/realm.py | 5 ++++ 10 files changed, 106 insertions(+), 3 deletions(-) diff --git a/web/src/settings_components.js b/web/src/settings_components.js index ec09b47cb8..c12b9ec1c0 100644 --- a/web/src/settings_components.js +++ b/web/src/settings_components.js @@ -239,6 +239,7 @@ export let notifications_stream_widget = null; export let signup_notifications_stream_widget = null; export let create_multiuse_invite_group_widget = null; export let can_remove_subscribers_group_widget = null; +export let can_access_all_users_group_widget = null; export function get_widget_for_dropdown_list_settings(property_name) { switch (property_name) { @@ -252,6 +253,8 @@ export function get_widget_for_dropdown_list_settings(property_name) { return create_multiuse_invite_group_widget; case "can_remove_subscribers_group": return can_remove_subscribers_group_widget; + case "realm_can_access_all_users_group": + return can_access_all_users_group_widget; default: blueslip.error("No dropdown list widget for property", {property_name}); return null; @@ -278,6 +281,10 @@ export function set_can_remove_subscribers_group_widget(widget) { can_remove_subscribers_group_widget = widget; } +export function set_can_access_all_users_group_widget(widget) { + can_access_all_users_group_widget = widget; +} + export function set_dropdown_list_widget_setting_value(property_name, value) { const widget = get_widget_for_dropdown_list_settings(property_name); widget.render(value); diff --git a/web/src/settings_org.js b/web/src/settings_org.js index 7cd029c680..75781f0e27 100644 --- a/web/src/settings_org.js +++ b/web/src/settings_org.js @@ -477,6 +477,7 @@ export function discard_property_element_changes(elem, for_realm_default_setting case "realm_default_code_block_language": case "can_remove_subscribers_group": case "realm_create_multiuse_invite_group": + case "realm_can_access_all_users_group": settings_components.set_dropdown_list_widget_setting_value( property_name, property_value, @@ -737,6 +738,33 @@ export function init_dropdown_widgets() { create_multiuse_invite_group_widget, ); create_multiuse_invite_group_widget.setup(); + + const can_access_all_users_group_widget = new dropdown_widget.DropdownWidget({ + widget_name: "realm_can_access_all_users_group", + get_options: () => + user_groups.get_realm_user_groups_for_dropdown_list_widget( + "can_access_all_users_group", + "realm", + ), + $events_container: $("#settings_overlay_container #organization-permissions"), + item_click_callback(event, dropdown) { + dropdown.hide(); + event.preventDefault(); + event.stopPropagation(); + settings_components.can_access_all_users_group_widget.render(); + settings_components.save_discard_widget_status_handler($("#org-guest-settings")); + }, + tippy_props: { + placement: "bottom-start", + }, + default_id: page_params.realm_can_access_all_users_group, + unique_id_type: dropdown_widget.DATA_TYPES.NUMBER, + on_mount_callback(dropdown) { + $(dropdown.popper).css("min-width", "300px"); + }, + }); + settings_components.set_can_access_all_users_group_widget(can_access_all_users_group_widget); + can_access_all_users_group_widget.setup(); } export function populate_data_for_request(subsection, for_realm_default_settings, sub) { diff --git a/web/src/types.ts b/web/src/types.ts index 571e8a7311..462b4f8ab0 100644 --- a/web/src/types.ts +++ b/web/src/types.ts @@ -203,4 +203,5 @@ export type GroupPermissionSetting = { default_group_name: string; id_field_name: string; default_for_system_groups: string | null; + allowed_system_groups: string[]; }; diff --git a/web/src/user_groups.ts b/web/src/user_groups.ts index a5fe080a9c..84f7e6e461 100644 --- a/web/src/user_groups.ts +++ b/web/src/user_groups.ts @@ -240,6 +240,7 @@ export function get_realm_user_groups_for_dropdown_list_widget( allow_owners_group, allow_nobody_group, allow_everyone_group, + allowed_system_groups, } = group_setting_config; const system_user_groups = settings_config.system_user_groups_list @@ -260,6 +261,10 @@ export function get_realm_user_groups_for_dropdown_list_widget( return false; } + if (allowed_system_groups.length && !allowed_system_groups.includes(group.name)) { + return false; + } + return true; }) .map((group) => { diff --git a/web/styles/settings.css b/web/styles/settings.css index 241a5904c6..3d7bea5e66 100644 --- a/web/styles/settings.css +++ b/web/styles/settings.css @@ -821,7 +821,8 @@ input[type="checkbox"] { margin: 20px 0 0; } -#org-join-settings { +#org-join-settings, +#org-guest-settings { .dropdown-widget-button { width: 325px; color: hsl(0deg 0% 33%); diff --git a/web/templates/settings/organization_permissions_admin.hbs b/web/templates/settings/organization_permissions_admin.hbs index 7ea6ac5dbe..12cf4b3582 100644 --- a/web/templates/settings/organization_permissions_admin.hbs +++ b/web/templates/settings/organization_permissions_admin.hbs @@ -295,6 +295,13 @@ is_checked=realm_enable_guest_user_indicator label=admin_settings_label.realm_enable_guest_user_indicator}} + + {{#if development}} + {{> ../dropdown_widget_with_label + widget_name="realm_can_access_all_users_group" + label=(t 'Which users have access to all other users in the organization') + value_type="number"}} + {{/if}}
diff --git a/web/tests/user_groups.test.js b/web/tests/user_groups.test.js index 34438a9048..9e5315a730 100644 --- a/web/tests/user_groups.test.js +++ b/web/tests/user_groups.test.js @@ -328,6 +328,7 @@ run_test("get_realm_user_groups_for_dropdown_list_widget", () => { allow_everyone_group: true, default_group_name: "role:administrators", id_field_name: "can_remove_subscribers_group_id", + allowed_system_groups: [], }, }, realm: { @@ -339,11 +340,22 @@ run_test("get_realm_user_groups_for_dropdown_list_widget", () => { allow_everyone_group: false, default_group_name: "role:administrators", id_field_name: "create_multiuse_invite_group_id", + allowed_system_groups: [], + }, + can_access_all_users_group: { + require_system_group: true, + allow_internet_group: false, + allow_owners_group: false, + allow_nobody_group: false, + allow_everyone_group: true, + default_group_name: "role:everyone", + id_field_name: "can_access_all_users_group_id", + allowed_system_groups: ["role:everyone", "role:members"], }, }, }; - const expected_groups_list = [ + let expected_groups_list = [ {name: "translated: Admins, moderators, members and guests", unique_id: 6}, {name: "translated: Admins, moderators and members", unique_id: 5}, {name: "translated: Admins, moderators and full members", unique_id: 7}, @@ -373,6 +385,19 @@ run_test("get_realm_user_groups_for_dropdown_list_widget", () => { expected_groups_list, ); + expected_groups_list = [ + {name: "translated: Admins, moderators, members and guests", unique_id: 6}, + {name: "translated: Admins, moderators and members", unique_id: 5}, + ]; + + assert.deepEqual( + user_groups.get_realm_user_groups_for_dropdown_list_widget( + "can_access_all_users_group", + "realm", + ), + expected_groups_list, + ); + assert.throws( () => user_groups.get_realm_user_groups_for_dropdown_list_widget("invalid_setting", "stream"), diff --git a/zerver/models.py b/zerver/models.py index 3ab6b9fb50..d269de31d7 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -830,7 +830,7 @@ class Realm(models.Model): # type: ignore[django-manager-missing] # django-stub allow_everyone_group=True, default_group_name=SystemGroups.EVERYONE, id_field_name="can_access_all_users_group_id", - allowed_system_groups=[SystemGroups.EVERYONE], + allowed_system_groups=[SystemGroups.EVERYONE, SystemGroups.MEMBERS], ), ) diff --git a/zerver/tests/test_realm.py b/zerver/tests/test_realm.py index ad2fc6c632..c8c38e71f3 100644 --- a/zerver/tests/test_realm.py +++ b/zerver/tests/test_realm.py @@ -1337,6 +1337,30 @@ class RealmAPITest(ZulipTestCase): with self.subTest(property=prop): self.do_test_realm_permission_group_setting_update_api(prop) + def test_update_can_access_all_users_group_setting(self) -> None: + realm = get_realm("zulip") + self.login("iago") + members_group = UserGroup.objects.get(realm=realm, name=SystemGroups.MEMBERS) + + with self.settings(DEVELOPMENT=False): + with self.assertRaises(AssertionError), self.assertLogs( + "django.request", "ERROR" + ) as error_log: + self.client_patch("/json/realm", {"can_access_all_users_group": members_group.id}) + + self.assertTrue( + "ERROR:django.request:Internal Server Error: /json/realm" in error_log.output[0] + ) + self.assertTrue("AssertionError" in error_log.output[0]) + + with self.settings(DEVELOPMENT=True): + result = self.client_patch( + "/json/realm", {"can_access_all_users_group": members_group.id} + ) + self.assert_json_success(result) + realm = get_realm("zulip") + self.assertEqual(realm.can_access_all_users_group_id, members_group.id) + # Not in Realm.property_types because org_type has # a unique RealmAuditLog event_type. def test_update_realm_org_type(self) -> None: diff --git a/zerver/views/realm.py b/zerver/views/realm.py index 419a7cd9c5..81ad10f270 100644 --- a/zerver/views/realm.py +++ b/zerver/views/realm.py @@ -1,5 +1,6 @@ from typing import Any, Dict, Mapping, Optional, Union +from django.conf import settings from django.core.exceptions import ValidationError from django.http import HttpRequest, HttpResponse from django.shortcuts import render @@ -232,6 +233,10 @@ def update_realm( if enable_spectator_access: realm.ensure_not_on_limited_plan() + if can_access_all_users_group_id is not None: + # Remove this when the feature is ready for production. + assert settings.DEVELOPMENT + data: Dict[str, Any] = {} message_content_delete_limit_seconds: Optional[int] = None