groups: Enable group-settings value on groups in production.

The main change is redefining ALLOW_GROUP_VALUED_SETTINGS to not
control code, but instead to instead control the configuration for
whether settings that have not been converted to use our modern UI
patterns should require system groups.

Fundamentally, it's the same for the realm/stream group-valued
settings, which don't have the new UI patterns yet.

We remove the visual hiding of the "can manage group" setting, which
was hidden only due to transitions being incomplete.
This commit is contained in:
Tim Abbott 2024-10-15 11:31:55 -07:00
parent 86b27f09f8
commit 7e7113ad84
9 changed files with 25 additions and 164 deletions

View File

@ -20,6 +20,13 @@ format used by the Zulip server that they are interacting with.
## Changes in Zulip 10.0 ## 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** **Feature level 308**
* [`POST /register`](/api/register-queue), [`GET /events`](/api/get-events), * [`POST /register`](/api/register-queue), [`GET /events`](/api/get-events),

View File

@ -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. configuration than the older [roles](/api/roles-and-permissions) system.
!!! warn "" !!! 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 **Changes**: Before Zulip 10.0 (feature level 309), only system
correspond to a single named user group are permitted in groups were permitted values for group-setting values in
production environments, pending the web application UI supporting production environments, regardless of the values in
displaying more complex values correctly. `server_supported_permission_settings`.
In the API, these settings are represented using a **group-setting In the API, these settings are represented using a **group-setting
value**, which can take two forms: value**, which can take two forms:

View File

@ -34,7 +34,7 @@ DESKTOP_WARNING_VERSION = "5.9.3"
# new level means in api_docs/changelog.md, as well as "**Changes**" # new level means in api_docs/changelog.md, as well as "**Changes**"
# entries in the endpoint's documentation in `zulip.yaml`. # 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 # 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 # only when going from an old version of the code to a newer version. Bump

View File

@ -5,7 +5,6 @@ import * as channel from "./channel";
import {$t, $t_html} from "./i18n"; import {$t, $t_html} from "./i18n";
import * as keydown_util from "./keydown_util"; import * as keydown_util from "./keydown_util";
import * as loading from "./loading"; import * as loading from "./loading";
import {page_params} from "./page_params";
import * as settings_components from "./settings_components"; import * as settings_components from "./settings_components";
import type {GroupSettingPillContainer} from "./typeahead_helper"; import type {GroupSettingPillContainer} from "./typeahead_helper";
import * as ui_report from "./ui_report"; 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(); user_group_create_members.build_widgets();
clear_error_display(); clear_error_display();
if (!page_params.development_environment) {
$("#new_group_can_manage_group_widget_container").hide();
}
} }
function create_user_group(): void { function create_user_group(): void {

View File

@ -18,7 +18,6 @@ import {$t, $t_html} from "./i18n";
import * as ListWidget from "./list_widget"; import * as ListWidget from "./list_widget";
import * as loading from "./loading"; import * as loading from "./loading";
import * as overlays from "./overlays"; import * as overlays from "./overlays";
import {page_params} from "./page_params";
import * as people from "./people"; import * as people from "./people";
import * as scroll_util from "./scroll_util"; import * as scroll_util from "./scroll_util";
import * as settings_components from "./settings_components"; import * as settings_components from "./settings_components";
@ -198,10 +197,6 @@ function show_general_settings(group) {
group, group,
}); });
update_general_panel_ui(group); update_general_panel_ui(group);
if (!page_params.development_environment) {
$(".can-manage-group-container").hide();
}
} }
function update_general_panel_ui(group) { function update_general_panel_ui(group) {

View File

@ -4,7 +4,6 @@ from contextlib import contextmanager
from dataclasses import asdict, dataclass from dataclasses import asdict, dataclass
from typing import Any, TypedDict from typing import Any, TypedDict
from django.conf import settings
from django.db import connection, transaction from django.db import connection, transaction
from django.db.models import F, Q, QuerySet from django.db.models import F, Q, QuerySet
from django.utils.timezone import now as timezone_now from django.utils.timezone import now as timezone_now
@ -355,11 +354,6 @@ def check_setting_configuration_for_system_groups(
setting_name: str, setting_name: str,
permission_configuration: GroupPermissionSetting, permission_configuration: GroupPermissionSetting,
) -> None: ) -> 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: if permission_configuration.require_system_group and not setting_group.is_system_group:
raise SystemGroupRequiredError(setting_name) 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: if len(setting_value.direct_members) == 0 and len(setting_value.direct_subgroups) == 1:
return setting_value.direct_subgroups[0] return setting_value.direct_subgroups[0]
if not settings.ALLOW_GROUP_VALUED_SETTINGS:
raise SystemGroupRequiredError(setting_name)
return setting_value return setting_value

View File

@ -704,7 +704,7 @@ class Realm(models.Model): # type: ignore[django-manager-missing] # django-stub
allowed_system_groups=[SystemGroups.EVERYONE, SystemGroups.MEMBERS], allowed_system_groups=[SystemGroups.EVERYONE, SystemGroups.MEMBERS],
), ),
can_add_custom_emoji_group=GroupPermissionSetting( can_add_custom_emoji_group=GroupPermissionSetting(
require_system_group=False, require_system_group=not settings.ALLOW_GROUP_VALUED_SETTINGS,
allow_internet_group=False, allow_internet_group=False,
allow_owners_group=False, allow_owners_group=False,
allow_nobody_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", id_field_name="can_add_custom_emoji_group_id",
), ),
can_create_groups=GroupPermissionSetting( can_create_groups=GroupPermissionSetting(
require_system_group=False, require_system_group=not settings.ALLOW_GROUP_VALUED_SETTINGS,
allow_internet_group=False, allow_internet_group=False,
allow_owners_group=True, allow_owners_group=True,
allow_nobody_group=False, 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", id_field_name="can_create_groups_id",
), ),
can_create_public_channel_group=GroupPermissionSetting( can_create_public_channel_group=GroupPermissionSetting(
require_system_group=False, require_system_group=not settings.ALLOW_GROUP_VALUED_SETTINGS,
allow_internet_group=False, allow_internet_group=False,
allow_owners_group=False, allow_owners_group=False,
allow_nobody_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", id_field_name="can_create_public_channel_group_id",
), ),
can_create_private_channel_group=GroupPermissionSetting( can_create_private_channel_group=GroupPermissionSetting(
require_system_group=False, require_system_group=not settings.ALLOW_GROUP_VALUED_SETTINGS,
allow_internet_group=False, allow_internet_group=False,
allow_owners_group=False, allow_owners_group=False,
allow_nobody_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( can_delete_any_message_group=GroupPermissionSetting(
require_system_group=False, require_system_group=not settings.ALLOW_GROUP_VALUED_SETTINGS,
allow_internet_group=False, allow_internet_group=False,
allow_owners_group=False, allow_owners_group=False,
allow_nobody_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", id_field_name="can_delete_any_message_group_id",
), ),
can_delete_own_message_group=GroupPermissionSetting( can_delete_own_message_group=GroupPermissionSetting(
require_system_group=False, require_system_group=not settings.ALLOW_GROUP_VALUED_SETTINGS,
allow_internet_group=False, allow_internet_group=False,
allow_owners_group=False, allow_owners_group=False,
allow_nobody_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", id_field_name="can_delete_own_message_group_id",
), ),
can_manage_all_groups=GroupPermissionSetting( can_manage_all_groups=GroupPermissionSetting(
require_system_group=False, require_system_group=not settings.ALLOW_GROUP_VALUED_SETTINGS,
allow_internet_group=False, allow_internet_group=False,
allow_owners_group=True, allow_owners_group=True,
allow_nobody_group=False, 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", id_field_name="can_manage_all_groups_id",
), ),
direct_message_initiator_group=GroupPermissionSetting( direct_message_initiator_group=GroupPermissionSetting(
require_system_group=False, require_system_group=not settings.ALLOW_GROUP_VALUED_SETTINGS,
allow_internet_group=False, allow_internet_group=False,
allow_owners_group=True, allow_owners_group=True,
allow_nobody_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", id_field_name="direct_message_initiator_group_id",
), ),
direct_message_permission_group=GroupPermissionSetting( direct_message_permission_group=GroupPermissionSetting(
require_system_group=False, require_system_group=not settings.ALLOW_GROUP_VALUED_SETTINGS,
allow_internet_group=False, allow_internet_group=False,
allow_owners_group=True, allow_owners_group=True,
allow_nobody_group=True, allow_nobody_group=True,

View File

@ -2032,52 +2032,6 @@ class RealmAPITest(ZulipTestCase):
realm = get_realm("zulip") realm = get_realm("zulip")
self.assertEqual(getattr(realm, setting_name), admins_group.usergroup_ptr) 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: def test_update_realm_properties(self) -> None:
for prop in Realm.property_types: for prop in Realm.property_types:
# push_notifications_enabled is maintained by the server, not via the API. # push_notifications_enabled is maintained by the server, not via the API.

View File

@ -722,51 +722,6 @@ class UserGroupAPITestCase(UserGroupTestCase):
leadership_group.deactivated = False leadership_group.deactivated = False
leadership_group.save() 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: def test_set_group_settings_during_user_group_creation(self) -> None:
for setting_name in NamedUserGroup.GROUP_PERMISSION_SETTINGS: for setting_name in NamedUserGroup.GROUP_PERMISSION_SETTINGS:
self.do_test_set_group_setting_during_user_group_creation(setting_name) 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.deactivated = False
leadership_group.save() 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: def test_update_user_group_permission_settings(self) -> None:
hamlet = self.example_user("hamlet") hamlet = self.example_user("hamlet")
check_add_user_group(hamlet.realm, "support", [hamlet], acting_user=hamlet) check_add_user_group(hamlet.realm, "support", [hamlet], acting_user=hamlet)