From 9f9d5b2f98a88f28200504da01a6efaf072a64f4 Mon Sep 17 00:00:00 2001 From: Shubham Padia Date: Tue, 22 Oct 2024 10:14:59 +0000 Subject: [PATCH] groups: Accept anonymous groups for can_access_all_users_group. On the frontend, the selection is still a dropdown of system groups but on the API level, we have started accepting anonymous groups similar to other settings We've kept require system groups true for now until we switch to group picker on the frontend. --- api_docs/changelog.md | 5 +++++ web/src/settings_components.ts | 1 + zerver/lib/event_schema.py | 2 +- zerver/models/realms.py | 3 ++- zerver/openapi/zulip.yaml | 32 ++++++++++++++++++-------------- zerver/tests/test_realm.py | 14 ++++++++++---- zerver/views/realm.py | 6 ++---- 7 files changed, 39 insertions(+), 24 deletions(-) diff --git a/api_docs/changelog.md b/api_docs/changelog.md index 32c4f49af8..c85add7db9 100644 --- a/api_docs/changelog.md +++ b/api_docs/changelog.md @@ -27,6 +27,11 @@ format used by the Zulip server that they are interacting with. by `create_multiuse_invite_group` realm setting, which is a now a [group-setting value](/api/group-setting-values) instead of an integer ID of the group. +* `PATCH /realm`, [`POST /register`](/api/register-queue), + [`GET /events`](/api/get-events): Anonymous groups are now accepted + by `can_access_all_users_group` realm setting, which is a now a + [group-setting value](/api/group-setting-values) instead of an + integer ID of the group. **Feature level 313** diff --git a/web/src/settings_components.ts b/web/src/settings_components.ts index e4a5f5a79a..a6a2344e76 100644 --- a/web/src/settings_components.ts +++ b/web/src/settings_components.ts @@ -1044,6 +1044,7 @@ export function populate_data_for_realm_settings_request( } const realm_group_settings_using_new_api_format = new Set([ + "can_access_all_users_group", "can_add_custom_emoji_group", "can_create_groups", "can_create_private_channel_group", diff --git a/zerver/lib/event_schema.py b/zerver/lib/event_schema.py index 72d4cafb9c..18d413c9e8 100644 --- a/zerver/lib/event_schema.py +++ b/zerver/lib/event_schema.py @@ -1067,7 +1067,7 @@ group_setting_update_data_type = DictType( required_keys=[], optional_keys=[ ("create_multiuse_invite_group", group_setting_type), - ("can_access_all_users_group", int), + ("can_access_all_users_group", group_setting_type), ("can_add_custom_emoji_group", group_setting_type), ("can_create_groups", group_setting_type), ("can_create_public_channel_group", group_setting_type), diff --git a/zerver/models/realms.py b/zerver/models/realms.py index f50e9f6748..997f846702 100644 --- a/zerver/models/realms.py +++ b/zerver/models/realms.py @@ -809,6 +809,7 @@ class Realm(models.Model): # type: ignore[django-manager-missing] # django-stub REALM_PERMISSION_GROUP_SETTINGS_WITH_NEW_API_FORMAT = [ "create_multiuse_invite_group", + "can_access_all_users_group", "can_add_custom_emoji_group", "can_create_groups", "can_create_private_channel_group", @@ -1190,10 +1191,10 @@ def get_realm_by_id(realm_id: int) -> Realm: def get_realm_with_settings(realm_id: int) -> Realm: # Prefetch the following settings: - # * All the settings that can be set to anonymous groups. # This also prefetches can_access_all_users_group setting, # even when it cannot be set to anonymous groups because # the setting is used when fetching users in the realm. + # * All the settings that can be set to anonymous groups. # * Announcements streams. return Realm.objects.select_related( "create_multiuse_invite_group", diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index e303f5b9ec..d3e18a888f 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -4401,15 +4401,17 @@ paths: The [policy](/api/roles-and-permissions#permission-levels) for which users can create bot users in this organization. can_access_all_users_group: - type: integer - description: | - The ID of the [user group](/api/get-user-groups) whose members - are allowed to access all users in the organization. + allOf: + - $ref: "#/components/schemas/GroupSettingValue" + - description: | + A [group-setting value](/api/group-setting-values) defining the + set of users who are allowed to access all users in the + organization. - This setting can currently only be set to `"role:everyone"` - system group. + **Changes**: Prior to Zulip 10.0 (feature level 314), this value used + to be of type integer and did not accept anonymous user groups. - **Changes**: New in Zulip 8.0 (feature level 225). + New in Zulip 8.0 (feature level 225). can_create_groups: allOf: - $ref: "#/components/schemas/GroupSettingValue" @@ -17227,15 +17229,17 @@ paths: **Changes**: New in Zulip 8.0 (feature level 216). realm_can_access_all_users_group: - type: integer - description: | - The ID of the [user group](/api/get-user-groups) whose members - are allowed to access all users in the organization. + allOf: + - $ref: "#/components/schemas/GroupSettingValue" + - description: | + A [group-setting value](/api/group-setting-values) defining the + set of users who are allowed to access all users in the + organization. - This setting can currently only be set to `"role:members"` - and `"role:everyone"` system groups. + **Changes**: Prior to Zulip 10.0 (feature level 314), this value used + to be of type integer and did not accept anonymous user groups. - **Changes**: New in Zulip 8.0 (feature level 225). + New in Zulip 8.0 (feature level 225). zulip_plan_is_not_limited: type: boolean description: | diff --git a/zerver/tests/test_realm.py b/zerver/tests/test_realm.py index 4b7cd4d0b8..bb8abe749d 100644 --- a/zerver/tests/test_realm.py +++ b/zerver/tests/test_realm.py @@ -1828,16 +1828,22 @@ class RealmAPITest(ZulipTestCase): result = self.client_patch("/json/realm", {setting_name: value}) self.assert_json_error(result, f"'{setting_name}' must be a system user group.") + group = admins_group + if setting_permission_configuration.allowed_system_groups: + group = NamedUserGroup.objects.get( + name=setting_permission_configuration.allowed_system_groups[0], realm=realm + ) + value = orjson.dumps( { "new": { "direct_members": [], - "direct_subgroups": [admins_group.id], + "direct_subgroups": [group.id], } } ).decode() realm = self.update_with_api(setting_name, value) - self.assertEqual(getattr(realm, setting_name), admins_group.usergroup_ptr) + self.assertEqual(getattr(realm, setting_name), group.usergroup_ptr) def do_test_realm_permission_group_setting_update_api_with_anonymous_groups( self, setting_name: str @@ -2377,12 +2383,12 @@ class RealmAPITest(ZulipTestCase): self.login("iago") members_group = NamedUserGroup.objects.get(name="role:members", realm=realm) - req = {"can_access_all_users_group": orjson.dumps(members_group.id).decode()} + req = {"can_access_all_users_group": orjson.dumps({"new": members_group.id}).decode()} result = self.client_patch("/json/realm", req) self.assert_json_error(result, "Available on Zulip Cloud Plus. Upgrade to access.") do_change_realm_plan_type(realm, Realm.PLAN_TYPE_STANDARD, acting_user=None) - req = {"can_access_all_users_group": orjson.dumps(members_group.id).decode()} + req = {"can_access_all_users_group": orjson.dumps({"new": members_group.id}).decode()} result = self.client_patch("/json/realm", req) self.assert_json_error(result, "Available on Zulip Cloud Plus. Upgrade to access.") diff --git a/zerver/views/realm.py b/zerver/views/realm.py index 8f64a438ba..e84664ebb5 100644 --- a/zerver/views/realm.py +++ b/zerver/views/realm.py @@ -172,9 +172,7 @@ def update_realm( ApiParamConfig("move_messages_between_streams_limit_seconds"), ] = None, enable_guest_user_indicator: Json[bool] | None = None, - can_access_all_users_group_id: Annotated[ - Json[int] | None, ApiParamConfig("can_access_all_users_group") - ] = None, + can_access_all_users_group: Json[GroupSettingChangeRequest] | None = None, ) -> HttpResponse: # Realm object is being refetched here to make sure that we # do not use stale object from cache which can happen when a @@ -241,7 +239,7 @@ def update_realm( if enable_spectator_access: realm.ensure_not_on_limited_plan() - if can_access_all_users_group_id is not None: + if can_access_all_users_group is not None: realm.can_enable_restricted_user_access_for_guests() data: dict[str, Any] = {}