diff --git a/api_docs/changelog.md b/api_docs/changelog.md index 828d11f93c..3382dfb4fb 100644 --- a/api_docs/changelog.md +++ b/api_docs/changelog.md @@ -20,6 +20,13 @@ format used by the Zulip server that they are interacting with. ## Changes in Zulip 10.0 +**Feature level 309** + +* [Group-setting values](/api/group-setting-values): Starting with + this feature level, it's now possible to use group-setting values in + production for those settings whose value is not required to be a + system group + **Feature level 308** * [`POST /register`](/api/register-queue), [`GET /events`](/api/get-events), diff --git a/api_docs/group-setting-values.md b/api_docs/group-setting-values.md index 24653c80b4..0fdece00ef 100644 --- a/api_docs/group-setting-values.md +++ b/api_docs/group-setting-values.md @@ -5,11 +5,15 @@ using [user groups](/help/user-groups), which offer much more flexible configuration than the older [roles](/api/roles-and-permissions) system. !!! warn "" + Note that many group-valued settings are configured to require + a single system group for their value via + `server_supported_permission_settings`, pending web app UI + changes to fully support group-setting values. - This API feature is under development, and currently only values that - correspond to a single named user group are permitted in - production environments, pending the web application UI supporting - displaying more complex values correctly. + **Changes**: Before Zulip 10.0 (feature level 309), only system + groups were permitted values for group-setting values in + production environments, regardless of the values in + `server_supported_permission_settings`. In the API, these settings are represented using a **group-setting value**, which can take two forms: diff --git a/version.py b/version.py index a01ab0bb90..d3b710152d 100644 --- a/version.py +++ b/version.py @@ -34,7 +34,7 @@ DESKTOP_WARNING_VERSION = "5.9.3" # new level means in api_docs/changelog.md, as well as "**Changes**" # entries in the endpoint's documentation in `zulip.yaml`. -API_FEATURE_LEVEL = 308 # Last bumped for can_leave_group. +API_FEATURE_LEVEL = 309 # Last bumped for group values in settings. # Bump the minor PROVISION_VERSION to indicate that folks should provision # only when going from an old version of the code to a newer version. Bump diff --git a/web/src/user_group_create.ts b/web/src/user_group_create.ts index aac65ad878..8426b02f80 100644 --- a/web/src/user_group_create.ts +++ b/web/src/user_group_create.ts @@ -5,7 +5,6 @@ import * as channel from "./channel"; import {$t, $t_html} from "./i18n"; import * as keydown_util from "./keydown_util"; import * as loading from "./loading"; -import {page_params} from "./page_params"; import * as settings_components from "./settings_components"; import type {GroupSettingPillContainer} from "./typeahead_helper"; import * as ui_report from "./ui_report"; @@ -133,10 +132,6 @@ export function show_new_user_group_modal(): void { user_group_create_members.build_widgets(); clear_error_display(); - - if (!page_params.development_environment) { - $("#new_group_can_manage_group_widget_container").hide(); - } } function create_user_group(): void { diff --git a/web/src/user_group_edit.js b/web/src/user_group_edit.js index 40381cecfe..664ab074ea 100644 --- a/web/src/user_group_edit.js +++ b/web/src/user_group_edit.js @@ -18,7 +18,6 @@ import {$t, $t_html} from "./i18n"; import * as ListWidget from "./list_widget"; import * as loading from "./loading"; import * as overlays from "./overlays"; -import {page_params} from "./page_params"; import * as people from "./people"; import * as scroll_util from "./scroll_util"; import * as settings_components from "./settings_components"; @@ -198,10 +197,6 @@ function show_general_settings(group) { group, }); update_general_panel_ui(group); - - if (!page_params.development_environment) { - $(".can-manage-group-container").hide(); - } } function update_general_panel_ui(group) { diff --git a/zerver/lib/user_groups.py b/zerver/lib/user_groups.py index 74f3dbde45..eb351f04ee 100644 --- a/zerver/lib/user_groups.py +++ b/zerver/lib/user_groups.py @@ -4,7 +4,6 @@ from contextlib import contextmanager from dataclasses import asdict, dataclass from typing import Any, TypedDict -from django.conf import settings from django.db import connection, transaction from django.db.models import F, Q, QuerySet from django.utils.timezone import now as timezone_now @@ -355,11 +354,6 @@ def check_setting_configuration_for_system_groups( setting_name: str, permission_configuration: GroupPermissionSetting, ) -> None: - if setting_name != "can_mention_group" and ( - not settings.ALLOW_GROUP_VALUED_SETTINGS and not setting_group.is_system_group - ): - raise SystemGroupRequiredError(setting_name) - if permission_configuration.require_system_group and not setting_group.is_system_group: raise SystemGroupRequiredError(setting_name) @@ -1018,9 +1012,6 @@ def parse_group_setting_value( if len(setting_value.direct_members) == 0 and len(setting_value.direct_subgroups) == 1: return setting_value.direct_subgroups[0] - if not settings.ALLOW_GROUP_VALUED_SETTINGS: - raise SystemGroupRequiredError(setting_name) - return setting_value diff --git a/zerver/models/realms.py b/zerver/models/realms.py index fb37a7b4cb..d56c49d114 100644 --- a/zerver/models/realms.py +++ b/zerver/models/realms.py @@ -704,7 +704,7 @@ class Realm(models.Model): # type: ignore[django-manager-missing] # django-stub allowed_system_groups=[SystemGroups.EVERYONE, SystemGroups.MEMBERS], ), can_add_custom_emoji_group=GroupPermissionSetting( - require_system_group=False, + require_system_group=not settings.ALLOW_GROUP_VALUED_SETTINGS, allow_internet_group=False, allow_owners_group=False, allow_nobody_group=False, @@ -713,7 +713,7 @@ class Realm(models.Model): # type: ignore[django-manager-missing] # django-stub id_field_name="can_add_custom_emoji_group_id", ), can_create_groups=GroupPermissionSetting( - require_system_group=False, + require_system_group=not settings.ALLOW_GROUP_VALUED_SETTINGS, allow_internet_group=False, allow_owners_group=True, allow_nobody_group=False, @@ -722,7 +722,7 @@ class Realm(models.Model): # type: ignore[django-manager-missing] # django-stub id_field_name="can_create_groups_id", ), can_create_public_channel_group=GroupPermissionSetting( - require_system_group=False, + require_system_group=not settings.ALLOW_GROUP_VALUED_SETTINGS, allow_internet_group=False, allow_owners_group=False, allow_nobody_group=False, @@ -731,7 +731,7 @@ class Realm(models.Model): # type: ignore[django-manager-missing] # django-stub id_field_name="can_create_public_channel_group_id", ), can_create_private_channel_group=GroupPermissionSetting( - require_system_group=False, + require_system_group=not settings.ALLOW_GROUP_VALUED_SETTINGS, allow_internet_group=False, allow_owners_group=False, allow_nobody_group=False, @@ -755,7 +755,7 @@ class Realm(models.Model): # type: ignore[django-manager-missing] # django-stub ], ), can_delete_any_message_group=GroupPermissionSetting( - require_system_group=False, + require_system_group=not settings.ALLOW_GROUP_VALUED_SETTINGS, allow_internet_group=False, allow_owners_group=False, allow_nobody_group=False, @@ -764,7 +764,7 @@ class Realm(models.Model): # type: ignore[django-manager-missing] # django-stub id_field_name="can_delete_any_message_group_id", ), can_delete_own_message_group=GroupPermissionSetting( - require_system_group=False, + require_system_group=not settings.ALLOW_GROUP_VALUED_SETTINGS, allow_internet_group=False, allow_owners_group=False, allow_nobody_group=False, @@ -773,7 +773,7 @@ class Realm(models.Model): # type: ignore[django-manager-missing] # django-stub id_field_name="can_delete_own_message_group_id", ), can_manage_all_groups=GroupPermissionSetting( - require_system_group=False, + require_system_group=not settings.ALLOW_GROUP_VALUED_SETTINGS, allow_internet_group=False, allow_owners_group=True, allow_nobody_group=False, @@ -782,7 +782,7 @@ class Realm(models.Model): # type: ignore[django-manager-missing] # django-stub id_field_name="can_manage_all_groups_id", ), direct_message_initiator_group=GroupPermissionSetting( - require_system_group=False, + require_system_group=not settings.ALLOW_GROUP_VALUED_SETTINGS, allow_internet_group=False, allow_owners_group=True, allow_nobody_group=True, @@ -791,7 +791,7 @@ class Realm(models.Model): # type: ignore[django-manager-missing] # django-stub id_field_name="direct_message_initiator_group_id", ), direct_message_permission_group=GroupPermissionSetting( - require_system_group=False, + require_system_group=not settings.ALLOW_GROUP_VALUED_SETTINGS, allow_internet_group=False, allow_owners_group=True, allow_nobody_group=True, diff --git a/zerver/tests/test_realm.py b/zerver/tests/test_realm.py index 5c3f1eb1b9..5f431fd4c8 100644 --- a/zerver/tests/test_realm.py +++ b/zerver/tests/test_realm.py @@ -2032,52 +2032,6 @@ class RealmAPITest(ZulipTestCase): realm = get_realm("zulip") self.assertEqual(getattr(realm, setting_name), admins_group.usergroup_ptr) - # Test case when ALLOW_GROUP_VALUED_SETTINGS is False. - with self.settings(ALLOW_GROUP_VALUED_SETTINGS=False): - result = self.client_patch( - "/json/realm", - { - setting_name: orjson.dumps( - { - "new": { - "direct_members": [othello.id], - "direct_subgroups": [moderators_group.id, leadership_group.id], - } - } - ).decode() - }, - ) - self.assert_json_error(result, f"'{setting_name}' must be a system user group.") - - result = self.client_patch( - "/json/realm", - { - setting_name: orjson.dumps( - { - "new": leadership_group.id, - } - ).decode() - }, - ) - self.assert_json_error(result, f"'{setting_name}' must be a system user group.") - - result = self.client_patch( - "/json/realm", - { - setting_name: orjson.dumps( - { - "new": { - "direct_members": [], - "direct_subgroups": [moderators_group.id], - } - } - ).decode() - }, - ) - self.assert_json_success(result) - realm = get_realm("zulip") - self.assertEqual(getattr(realm, setting_name), moderators_group.usergroup_ptr) - def test_update_realm_properties(self) -> None: for prop in Realm.property_types: # push_notifications_enabled is maintained by the server, not via the API. diff --git a/zerver/tests/test_user_groups.py b/zerver/tests/test_user_groups.py index 8bd7e5654e..f6a90ae832 100644 --- a/zerver/tests/test_user_groups.py +++ b/zerver/tests/test_user_groups.py @@ -722,51 +722,6 @@ class UserGroupAPITestCase(UserGroupTestCase): leadership_group.deactivated = False leadership_group.save() - with self.settings(ALLOW_GROUP_VALUED_SETTINGS=False): - params = { - "name": "frontend", - "members": orjson.dumps([hamlet.id]).decode(), - "description": "Frontend team", - } - params[setting_name] = orjson.dumps( - { - "direct_members": [othello.id], - "direct_subgroups": [moderators_group.id], - } - ).decode() - result = self.client_post("/json/user_groups/create", info=params) - self.assert_json_error(result, f"'{setting_name}' must be a system user group.") - - params = { - "name": "frontend", - "members": orjson.dumps([hamlet.id]).decode(), - "description": "Frontend team", - } - params[setting_name] = orjson.dumps( - { - "direct_members": [], - "direct_subgroups": [moderators_group.id], - } - ).decode() - result = self.client_post("/json/user_groups/create", info=params) - self.assert_json_success(result) - frontend_group = NamedUserGroup.objects.get(name="frontend", realm=hamlet.realm) - self.assertEqual(getattr(frontend_group, setting_name).id, moderators_group.id) - - params = { - "name": "devops", - "members": orjson.dumps([hamlet.id]).decode(), - "description": "Devops team", - } - params[setting_name] = orjson.dumps(leadership_group.id).decode() - result = self.client_post("/json/user_groups/create", info=params) - if setting_name == "can_mention_group": - self.assert_json_success(result) - devops_group = NamedUserGroup.objects.get(name="devops", realm=hamlet.realm) - self.assertEqual(getattr(devops_group, setting_name).id, leadership_group.id) - else: - self.assert_json_error(result, f"'{setting_name}' must be a system user group.") - def test_set_group_settings_during_user_group_creation(self) -> None: for setting_name in NamedUserGroup.GROUP_PERMISSION_SETTINGS: self.do_test_set_group_setting_during_user_group_creation(setting_name) @@ -1041,46 +996,6 @@ class UserGroupAPITestCase(UserGroupTestCase): leadership_group.deactivated = False leadership_group.save() - # Test case when ALLOW_GROUP_VALUED_SETTINGS is False. - with self.settings(ALLOW_GROUP_VALUED_SETTINGS=False): - params[setting_name] = orjson.dumps( - { - "new": { - "direct_members": [othello.id], - "direct_subgroups": [moderators_group.id, marketing_group.id], - } - } - ).decode() - result = self.client_patch(f"/json/user_groups/{support_group.id}", info=params) - self.assert_json_error(result, f"'{setting_name}' must be a system user group.") - - params[setting_name] = orjson.dumps( - { - "new": { - "direct_members": [], - "direct_subgroups": [moderators_group.id], - } - } - ).decode() - result = self.client_patch(f"/json/user_groups/{support_group.id}", info=params) - self.assert_json_success(result) - support_group = NamedUserGroup.objects.get(name="support", realm=hamlet.realm) - self.assertEqual(getattr(support_group, setting_name).id, moderators_group.id) - - params[setting_name] = orjson.dumps( - { - "new": marketing_group.id, - } - ).decode() - result = self.client_patch(f"/json/user_groups/{support_group.id}", info=params) - - if setting_name == "can_mention_group": - self.assert_json_success(result) - support_group = NamedUserGroup.objects.get(name="support", realm=hamlet.realm) - self.assertEqual(getattr(support_group, setting_name).id, marketing_group.id) - else: - self.assert_json_error(result, f"'{setting_name}' must be a system user group.") - def test_update_user_group_permission_settings(self) -> None: hamlet = self.example_user("hamlet") check_add_user_group(hamlet.realm, "support", [hamlet], acting_user=hamlet)