Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

user_groups: Remove "@" from name of role-based system groups. 

This commit removes "@" from name of role-based system groups
since we have added a restricion on having user group names
starting with "@" in the previous commit as they look odd in
mention syntax.

We also add a migration in this commit to update the name of
role-based system groups in existing realms to remove "@"
from the name. This migration also updates the names of
non-system user groups by removing the invalid prefixes
from their names and if there is a group already with that
name, we insted name the group as "group:{group_id}".

Fixes #26148.
This commit is contained in:
Sahil Batra 2023-07-03 12:48:44 +05:30 committed by Tim Abbott
parent 929bf1243e
commit 2e4f7f6336
12 changed files with 155 additions and 69 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
web/src/settings_config.ts
View File

@ -852,35 +852,35 @@ export const desktop_icon_count_display_values = {
export const system_user_groups_list = [
{
name: "@role:internet",
name: "role:internet",
display_name: $t({defaultMessage: "Everyone on the internet"}),
},
{
name: "@role:everyone",
name: "role:everyone",
display_name: $t({defaultMessage: "Admins, moderators, members and guests"}),
},
{
name: "@role:members",
name: "role:members",
display_name: $t({defaultMessage: "Admins, moderators and members"}),
},
{
name: "@role:fullmembers",
name: "role:fullmembers",
display_name: $t({defaultMessage: "Admins, moderators and full members"}),
},
{
name: "@role:moderators",
name: "role:moderators",
display_name: $t({defaultMessage: "Admins and moderators"}),
},
{
name: "@role:administrators",
name: "role:administrators",
display_name: $t({defaultMessage: "Admins"}),
},
{
name: "@role:owners",
name: "role:owners",
display_name: $t({defaultMessage: "Owners"}),
},
{
name: "@role:nobody",
name: "role:nobody",
display_name: $t({defaultMessage: "Nobody"}),
},
];

2
web/src/stream_settings_ui.js
View File

@ -671,7 +671,7 @@ export function setup_page(callback) {
),
default_text: $t({defaultMessage: "No user groups"}),
include_current_item: false,
value: user_groups.get_user_group_from_name("@role:administrators").id,
value: user_groups.get_user_group_from_name("role:administrators").id,
};
new_stream_can_remove_subscribers_group_widget = new DropdownListWidget(opts);

6
web/src/user_groups.ts
View File

@ -236,15 +236,15 @@ export function get_realm_user_groups_for_dropdown_list_widget(
const system_user_groups = settings_config.system_user_groups_list
.filter((group) => {
if (!allow_internet_group && group.name === "@role:internet") {
if (!allow_internet_group && group.name === "role:internet") {
return false;
}
if (!allow_owners_group && group.name === "@role:owners") {
if (!allow_owners_group && group.name === "role:owners") {
return false;
}
if (!allow_nobody_group && group.name === "@role:nobody") {
if (!allow_nobody_group && group.name === "role:nobody") {
return false;
}

16
web/tests/user_groups.test.js
View File

@ -246,7 +246,7 @@ run_test("is_user_in_group", () => {
run_test("get_realm_user_groups_for_dropdown_list_widget", () => {
const nobody = {
name: "@role:nobody",
name: "role:nobody",
description: "foo",
id: 1,
members: new Set([]),
@ -254,7 +254,7 @@ run_test("get_realm_user_groups_for_dropdown_list_widget", () => {
direct_subgroup_ids: new Set([]),
};
const owners = {
name: "@role:owners",
name: "role:owners",
description: "foo",
id: 2,
members: new Set([1]),
@ -262,7 +262,7 @@ run_test("get_realm_user_groups_for_dropdown_list_widget", () => {
direct_subgroup_ids: new Set([]),
};
const admins = {
name: "@role:administrators",
name: "role:administrators",
description: "foo",
id: 3,
members: new Set([2]),
@ -270,7 +270,7 @@ run_test("get_realm_user_groups_for_dropdown_list_widget", () => {
direct_subgroup_ids: new Set([1]),
};
const moderators = {
name: "@role:moderators",
name: "role:moderators",
description: "foo",
id: 4,
members: new Set([3]),
@ -278,7 +278,7 @@ run_test("get_realm_user_groups_for_dropdown_list_widget", () => {
direct_subgroup_ids: new Set([2]),
};
const members = {
name: "@role:members",
name: "role:members",
description: "foo",
id: 5,
members: new Set([4]),
@ -286,7 +286,7 @@ run_test("get_realm_user_groups_for_dropdown_list_widget", () => {
direct_subgroup_ids: new Set([6]),
};
const everyone = {
name: "@role:everyone",
name: "role:everyone",
description: "foo",
id: 6,
members: new Set([]),
@ -294,7 +294,7 @@ run_test("get_realm_user_groups_for_dropdown_list_widget", () => {
direct_subgroup_ids: new Set([4]),
};
const full_members = {
name: "@role:fullmembers",
name: "role:fullmembers",
description: "foo",
id: 7,
members: new Set([5]),
@ -302,7 +302,7 @@ run_test("get_realm_user_groups_for_dropdown_list_widget", () => {
direct_subgroup_ids: new Set([3]),
};
const internet = {
name: "@role:internet",
name: "role:internet",
id: 8,
members: new Set([]),
is_system_group: true,

6
zerver/lib/user_groups.py
View File

@ -85,17 +85,17 @@ def access_user_group_for_setting(
if not allow_internet_group and user_group.name == UserGroup.EVERYONE_ON_INTERNET_GROUP_NAME:
raise JsonableError(
_("'{}' setting cannot be set to '@role:internet' group.").format(setting_name)
_("'{}' setting cannot be set to 'role:internet' group.").format(setting_name)
)
if not allow_owners_group and user_group.name == UserGroup.OWNERS_GROUP_NAME:
raise JsonableError(
_("'{}' setting cannot be set to '@role:owners' group.").format(setting_name)
_("'{}' setting cannot be set to 'role:owners' group.").format(setting_name)
)
if not allow_nobody_group and user_group.name == UserGroup.NOBODY_GROUP_NAME:
raise JsonableError(
_("'{}' setting cannot be set to '@role:nobody' group.").format(setting_name)
_("'{}' setting cannot be set to 'role:nobody' group.").format(setting_name)
)
return user_group

86
zerver/migrations/0459_remove_invalid_characters_from_user_group_name.py Normal file
View File

@ -0,0 +1,86 @@
# Generated by Django 4.2.2 on 2023-07-03 06:39
from typing import Any
from django.db import migrations, transaction
from django.db.backends.base.schema import BaseDatabaseSchemaEditor
from django.db.migrations.state import StateApps
INVALID_NAME_PREFIXES = ["@", "role:", "user:", "stream:", "channel:", "group:"]
def check_group_name_starts_with_invalid_prefix(group_name: str) -> str:
for invalid_prefix in INVALID_NAME_PREFIXES:
if group_name.startswith(invalid_prefix):
return invalid_prefix
return ""
def remove_invalid_characters_from_user_group_name(
apps: StateApps, schema_editor: BaseDatabaseSchemaEditor
) -> None:
UserGroup = apps.get_model("zerver", "UserGroup")
Realm = apps.get_model("zerver", "Realm")
@transaction.atomic
def update_single_realm(realm: Any) -> None:
# Remove "@" character from name of role-based system groups.
groups_to_update = []
for group in UserGroup.objects.filter(realm=realm, is_system_group=True):
if group.name[0] != "@":
# We skip the group if its name has already been updated.
continue
old_group_name = group.name
group.name = old_group_name[1:]
groups_to_update.append(group)
# Fix the name of non-system groups as well.
existing_group_names = UserGroup.objects.filter(
realm=realm, is_system_group=False
).values_list("name", flat=True)
existing_group_names_set = set(existing_group_names)
for prefix in INVALID_NAME_PREFIXES:
groups = UserGroup.objects.filter(
realm=realm, is_system_group=False, name__startswith=prefix
)
for group in groups:
orig_group_name = group.name
group_name = group.name
while True:
matching_invalid_prefix = check_group_name_starts_with_invalid_prefix(
group_name
)
if len(matching_invalid_prefix) == 0:
break
group_name = group_name[len(matching_invalid_prefix) :]
if len(group_name) > 0 and group_name not in existing_group_names_set:
group.name = group_name
else:
group.name = f"group:{group.id}"
existing_group_names_set.add(group.name)
groups_to_update.append(group)
print(f"Renaming {orig_group_name} to {group_name} for {group.id} in {realm.id}")
UserGroup.objects.bulk_update(groups_to_update, ["name"])
for realm in Realm.objects.all():
update_single_realm(realm)
class Migration(migrations.Migration):
atomic = False
dependencies = [
("zerver", "0458_realmauditlog_modified_user_group"),
]
operations = [
migrations.RunPython(
remove_invalid_characters_from_user_group_name,
elidable=True,
reverse_code=migrations.RunPython.noop,
)
]

16
zerver/models.py
View File

@ -2256,14 +2256,14 @@ class UserGroup(models.Model): # type: ignore[django-manager-missing] # django-
can_mention_group = models.ForeignKey("self", on_delete=models.RESTRICT)
# Names for system groups.
FULL_MEMBERS_GROUP_NAME = "@role:fullmembers"
EVERYONE_ON_INTERNET_GROUP_NAME = "@role:internet"
OWNERS_GROUP_NAME = "@role:owners"
ADMINISTRATORS_GROUP_NAME = "@role:administrators"
MODERATORS_GROUP_NAME = "@role:moderators"
MEMBERS_GROUP_NAME = "@role:members"
EVERYONE_GROUP_NAME = "@role:everyone"
NOBODY_GROUP_NAME = "@role:nobody"
FULL_MEMBERS_GROUP_NAME = "role:fullmembers"
EVERYONE_ON_INTERNET_GROUP_NAME = "role:internet"
OWNERS_GROUP_NAME = "role:owners"
ADMINISTRATORS_GROUP_NAME = "role:administrators"
MODERATORS_GROUP_NAME = "role:moderators"
MEMBERS_GROUP_NAME = "role:members"
EVERYONE_GROUP_NAME = "role:everyone"
NOBODY_GROUP_NAME = "role:nobody"
# We do not have "Full members" and "Everyone on the internet"
# group here since there isn't a separate role value for full

10
zerver/openapi/zulip.yaml
View File

@ -16178,7 +16178,7 @@ paths:
ID of the user group whose members are allowed to mention the new
user group.
This setting cannot be set to `"@role:internet"` and `"@role:owners"`
This setting cannot be set to `"role:internet"` and `"role:owners"`
system groups.
**Changes**: New in Zulip 8.0 (feature level 191). Previously, groups
@ -16315,7 +16315,7 @@ paths:
ID of the new user group whose members are allowed to mention the
group.
This setting cannot be set to `"@role:internet"` and `"@role:owners"`
This setting cannot be set to `"role:internet"` and `"role:owners"`
system groups.
**Changes**: New in Zulip 8.0 (feature level 191). Previously, groups
@ -16458,7 +16458,7 @@ paths:
{
"description": "Owners of this organization",
"id": 1,
"name": "@role:owners",
"name": "role:owners",
"members": [1],
"direct_subgroup_ids": [],
"is_system_group": true,
@ -16467,7 +16467,7 @@ paths:
{
"description": "Administrators of this organization, including owners",
"id": 2,
"name": "@role:administrators",
"name": "role:administrators",
"members": [2],
"direct_subgroup_ids": [1],
"is_system_group": true,
@ -19263,7 +19263,7 @@ components:
This setting can currently only be set to user groups that are
system groups, except for the system groups named
`"@role:internet"` and `"@role:owners"`.
`"role:internet"` and `"role:owners"`.
**Changes**: New in Zulip 7.0 (feature level 161).
schema:

2
zerver/tests/test_events.py
View File

@ -1400,7 +1400,7 @@ class NormalActionsTest(BaseAction):
# Test can_mention_group setting update
moderators_group = UserGroup.objects.get(
name="@role:moderators", realm=self.user_profile.realm, is_system_group=True
name="role:moderators", realm=self.user_profile.realm, is_system_group=True
)
events = self.verify_action(
lambda: do_change_user_group_permission_setting(

6
zerver/tests/test_import_export.py
View File

@ -440,7 +440,7 @@ class RealmImportExportTest(ExportFile):
exported_usergroups = data["zerver_usergroup"]
self.assert_length(exported_usergroups, 9)
self.assertEqual(exported_usergroups[2]["name"], "@role:administrators")
self.assertEqual(exported_usergroups[2]["name"], "role:administrators")
self.assertFalse("direct_members" in exported_usergroups[2])
self.assertFalse("direct_subgroups" in exported_usergroups[2])
@ -1206,7 +1206,7 @@ class RealmImportExportTest(ExportFile):
@getter
def get_group_group_membership(r: Realm) -> Set[str]:
usergroup = UserGroup.objects.get(realm=r, name="@role:members")
usergroup = UserGroup.objects.get(realm=r, name="role:members")
group_group_membership = GroupGroupMembership.objects.filter(supergroup=usergroup)
subgroups = {membership.subgroup.name for membership in group_group_membership}
return subgroups
@ -1226,7 +1226,7 @@ class RealmImportExportTest(ExportFile):
# We already check the subgroups of the group through GroupGroupMembership
# objects, but we also want to check that direct_subgroups field is set
# correctly since we do not include this in export data.
usergroup = UserGroup.objects.get(realm=r, name="@role:members")
usergroup = UserGroup.objects.get(realm=r, name="role:members")
direct_subgroups = usergroup.direct_subgroups.all()
direct_subgroup_names = {group.name for group in direct_subgroups}
return direct_subgroup_names

32
zerver/tests/test_subs.py
View File

@ -272,7 +272,7 @@ class TestCreateStreams(ZulipTestCase):
self.assertEqual(events[0]["event"]["streams"][0]["name"], "Private stream")
moderators_system_group = UserGroup.objects.get(
name="@role:moderators", realm=realm, is_system_group=True
name="role:moderators", realm=realm, is_system_group=True
)
new_streams, existing_streams = create_streams_if_needed(
realm,
@ -478,10 +478,10 @@ class TestCreateStreams(ZulipTestCase):
realm = user.realm
self.login_user(user)
moderators_system_group = UserGroup.objects.get(
name="@role:moderators", realm=realm, is_system_group=True
name="role:moderators", realm=realm, is_system_group=True
)
admins_system_group = UserGroup.objects.get(
name="@role:administrators", realm=realm, is_system_group=True
name="role:administrators", realm=realm, is_system_group=True
)
post_data = {
@ -518,7 +518,7 @@ class TestCreateStreams(ZulipTestCase):
)
internet_group = UserGroup.objects.get(
name="@role:internet", is_system_group=True, realm=realm
name="role:internet", is_system_group=True, realm=realm
)
post_data = {
"subscriptions": orjson.dumps(
@ -529,10 +529,10 @@ class TestCreateStreams(ZulipTestCase):
result = self.api_post(user, "/api/v1/users/me/subscriptions", post_data, subdomain="zulip")
self.assert_json_error(
result,
"'can_remove_subscribers_group' setting cannot be set to '@role:internet' group.",
"'can_remove_subscribers_group' setting cannot be set to 'role:internet' group.",
)
owners_group = UserGroup.objects.get(name="@role:owners", is_system_group=True, realm=realm)
owners_group = UserGroup.objects.get(name="role:owners", is_system_group=True, realm=realm)
post_data = {
"subscriptions": orjson.dumps(
[{"name": "new_stream3", "description": "Third new stream"}]
@ -542,10 +542,10 @@ class TestCreateStreams(ZulipTestCase):
result = self.api_post(user, "/api/v1/users/me/subscriptions", post_data, subdomain="zulip")
self.assert_json_error(
result,
"'can_remove_subscribers_group' setting cannot be set to '@role:owners' group.",
"'can_remove_subscribers_group' setting cannot be set to 'role:owners' group.",
)
nobody_group = UserGroup.objects.get(name="@role:nobody", is_system_group=True, realm=realm)
nobody_group = UserGroup.objects.get(name="role:nobody", is_system_group=True, realm=realm)
post_data = {
"subscriptions": orjson.dumps(
[{"name": "new_stream3", "description": "Third new stream"}]
@ -555,7 +555,7 @@ class TestCreateStreams(ZulipTestCase):
result = self.api_post(user, "/api/v1/users/me/subscriptions", post_data, subdomain="zulip")
self.assert_json_error(
result,
"'can_remove_subscribers_group' setting cannot be set to '@role:nobody' group.",
"'can_remove_subscribers_group' setting cannot be set to 'role:nobody' group.",
)
@ -2071,7 +2071,7 @@ class StreamAdminTest(ZulipTestCase):
stream = self.subscribe(user_profile, "stream_name1")
moderators_system_group = UserGroup.objects.get(
name="@role:moderators", realm=realm, is_system_group=True
name="role:moderators", realm=realm, is_system_group=True
)
self.login("shiva")
result = self.client_patch(
@ -2100,7 +2100,7 @@ class StreamAdminTest(ZulipTestCase):
)
internet_group = UserGroup.objects.get(
name="@role:internet", is_system_group=True, realm=realm
name="role:internet", is_system_group=True, realm=realm
)
result = self.client_patch(
f"/json/streams/{stream.id}",
@ -2108,27 +2108,27 @@ class StreamAdminTest(ZulipTestCase):
)
self.assert_json_error(
result,
"'can_remove_subscribers_group' setting cannot be set to '@role:internet' group.",
"'can_remove_subscribers_group' setting cannot be set to 'role:internet' group.",
)
owners_group = UserGroup.objects.get(name="@role:owners", is_system_group=True, realm=realm)
owners_group = UserGroup.objects.get(name="role:owners", is_system_group=True, realm=realm)
result = self.client_patch(
f"/json/streams/{stream.id}",
{"can_remove_subscribers_group_id": orjson.dumps(owners_group.id).decode()},
)
self.assert_json_error(
result,
"'can_remove_subscribers_group' setting cannot be set to '@role:owners' group.",
"'can_remove_subscribers_group' setting cannot be set to 'role:owners' group.",
)
nobody_group = UserGroup.objects.get(name="@role:nobody", is_system_group=True, realm=realm)
nobody_group = UserGroup.objects.get(name="role:nobody", is_system_group=True, realm=realm)
result = self.client_patch(
f"/json/streams/{stream.id}",
{"can_remove_subscribers_group_id": orjson.dumps(nobody_group.id).decode()},
)
self.assert_json_error(
result,
"'can_remove_subscribers_group' setting cannot be set to '@role:nobody' group.",
"'can_remove_subscribers_group' setting cannot be set to 'role:nobody' group.",
)
# For private streams, even admins must be subscribed to the stream to change

26
zerver/tests/test_user_groups.py
View File

@ -242,7 +242,7 @@ class UserGroupAPITestCase(UserGroupTestCase):
# Check default value of can_mention_group setting.
everyone_system_group = UserGroup.objects.get(
name="@role:everyone", realm=hamlet.realm, is_system_group=True
name="role:everyone", realm=hamlet.realm, is_system_group=True
)
support_group = UserGroup.objects.get(name="support", realm=hamlet.realm)
self.assertEqual(support_group.can_mention_group, everyone_system_group)
@ -314,7 +314,7 @@ class UserGroupAPITestCase(UserGroupTestCase):
hamlet.realm, "leadership", [hamlet], acting_user=None
)
moderators_group = UserGroup.objects.get(
name="@role:moderators", realm=hamlet.realm, is_system_group=True
name="role:moderators", realm=hamlet.realm, is_system_group=True
)
params = {
"name": "support",
@ -339,7 +339,7 @@ class UserGroupAPITestCase(UserGroupTestCase):
self.assertEqual(test_group.can_mention_group, leadership_group)
nobody_group = UserGroup.objects.get(
name="@role:nobody", realm=hamlet.realm, is_system_group=True
name="role:nobody", realm=hamlet.realm, is_system_group=True
)
params = {
"name": "marketing",
@ -353,7 +353,7 @@ class UserGroupAPITestCase(UserGroupTestCase):
self.assertEqual(marketing_group.can_mention_group, nobody_group)
internet_group = UserGroup.objects.get(
name="@role:internet", realm=hamlet.realm, is_system_group=True
name="role:internet", realm=hamlet.realm, is_system_group=True
)
params = {
"name": "frontend",
@ -363,11 +363,11 @@ class UserGroupAPITestCase(UserGroupTestCase):
}
result = self.client_post("/json/user_groups/create", info=params)
self.assert_json_error(
result, "'can_mention_group' setting cannot be set to '@role:internet' group."
result, "'can_mention_group' setting cannot be set to 'role:internet' group."
)
owners_group = UserGroup.objects.get(
name="@role:owners", realm=hamlet.realm, is_system_group=True
name="role:owners", realm=hamlet.realm, is_system_group=True
)
params = {
"name": "frontend",
@ -377,7 +377,7 @@ class UserGroupAPITestCase(UserGroupTestCase):
}
result = self.client_post("/json/user_groups/create", info=params)
self.assert_json_error(
result, "'can_mention_group' setting cannot be set to '@role:owners' group."
result, "'can_mention_group' setting cannot be set to 'role:owners' group."
)
params = {
@ -484,7 +484,7 @@ class UserGroupAPITestCase(UserGroupTestCase):
)
moderators_group = UserGroup.objects.get(
name="@role:moderators", realm=hamlet.realm, is_system_group=True
name="role:moderators", realm=hamlet.realm, is_system_group=True
)
self.login("hamlet")
@ -505,7 +505,7 @@ class UserGroupAPITestCase(UserGroupTestCase):
self.assertEqual(support_group.can_mention_group, marketing_group)
nobody_group = UserGroup.objects.get(
name="@role:nobody", realm=hamlet.realm, is_system_group=True
name="role:nobody", realm=hamlet.realm, is_system_group=True
)
params = {
"can_mention_group_id": orjson.dumps(nobody_group.id).decode(),
@ -516,25 +516,25 @@ class UserGroupAPITestCase(UserGroupTestCase):
self.assertEqual(support_group.can_mention_group, nobody_group)
owners_group = UserGroup.objects.get(
name="@role:owners", realm=hamlet.realm, is_system_group=True
name="role:owners", realm=hamlet.realm, is_system_group=True
)
params = {
"can_mention_group_id": orjson.dumps(owners_group.id).decode(),
}
result = self.client_patch(f"/json/user_groups/{support_group.id}", info=params)
self.assert_json_error(
result, "'can_mention_group' setting cannot be set to '@role:owners' group."
result, "'can_mention_group' setting cannot be set to 'role:owners' group."
)
internet_group = UserGroup.objects.get(
name="@role:internet", realm=hamlet.realm, is_system_group=True
name="role:internet", realm=hamlet.realm, is_system_group=True
)
params = {
"can_mention_group_id": orjson.dumps(internet_group.id).decode(),
}
result = self.client_patch(f"/json/user_groups/{support_group.id}", info=params)
self.assert_json_error(
result, "'can_mention_group' setting cannot be set to '@role:internet' group."
result, "'can_mention_group' setting cannot be set to 'role:internet' group."
)
params = {