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

settings: Add group_creator as default for can_manage_group. 

We create an unnamed user group with just the group creator as it's
member when trying to set the default. The pattern I've followed across
most of the acting_user additions is to just put the user declared
somewhere before the check_add_user_group and see if the test passes.
If it does not, then I'll look at what kind of user it needs to be set
to `acting_user`.
This commit is contained in:
Shubham Padia 2024-09-25 09:51:28 +00:00 committed by Tim Abbott
parent 91edf59873
commit 12ebd97f1f
24 changed files with 162 additions and 109 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
web/src/settings_components.ts
View File

@ -23,7 +23,7 @@ import * as scroll_util from "./scroll_util";
import * as settings_config from "./settings_config";
import * as settings_data from "./settings_data";
import type {CustomProfileField, group_setting_type_schema} from "./state_data";
import {realm} from "./state_data";
import {current_user, realm} from "./state_data";
import * as stream_data from "./stream_data";
import type {StreamSubscription} from "./sub_store";
import type {GroupSettingPillContainer} from "./typeahead_helper";
@ -1489,7 +1489,14 @@ export function create_group_setting_widget({
setting_name,
"group",
)!.default_group_name;
if (default_group_name === "group_creator") {
set_group_setting_widget_value("new_group_" + setting_name, {
direct_members: [current_user.user_id],
direct_subgroups: [],
});
} else {
const default_group_id = user_groups.get_user_group_from_name(default_group_name)!.id;
set_group_setting_widget_value("new_group_" + setting_name, default_group_id);
}
}
}

8
web/src/settings_data.ts
View File

@ -208,14 +208,6 @@ export function can_manage_user_group(group_id: number): boolean {
const group = user_groups.get_user_group_from_id(group_id);
// This is a temporary exception and this should be removed as soon
// as `group_creator` is set as a default for `can_manage_group`
// property of user groups. See this topic for more details:
// https://chat.zulip.org/#narrow/stream/3-backend/topic/Group.20creation.20-.20who.20can.20change.20the.20setting.2E/near/1943861
if (group.creator_id && group.creator_id === current_user.user_id) {
return true;
}
if (
!current_user.is_admin &&
!current_user.is_moderator &&

5
web/tests/settings_data.test.js
View File

@ -387,7 +387,10 @@ run_test("can_manage_user_group", () => {
members: new Set([1, 2]),
is_system_group: false,
direct_subgroup_ids: new Set([4, 5]),
can_manage_group: 4,
can_manage_group: {
direct_members: [4],
direct_subgroups: [],
},
can_mention_group: 3,
creator_id: 4,
};

17
zerver/lib/import_realm.py
View File

@ -760,7 +760,6 @@ def bulk_import_named_user_groups(data: TableData) -> None:
group["can_manage_group_id"],
group["can_mention_group_id"],
group["deactivated"],
group["creator_id"],
group["date_created"],
)
for group in data["zerver_namedusergroup"]
@ -768,7 +767,7 @@ def bulk_import_named_user_groups(data: TableData) -> None:
query = SQL(
"""
INSERT INTO zerver_namedusergroup (usergroup_ptr_id, realm_id, name, description, is_system_group, can_manage_group_id, can_mention_group_id, deactivated, creator_id, date_created)
INSERT INTO zerver_namedusergroup (usergroup_ptr_id, realm_id, name, description, is_system_group, can_manage_group_id, can_mention_group_id, deactivated, date_created)
VALUES %s
"""
)
@ -1200,6 +1199,7 @@ def do_import_realm(import_dir: Path, subdomain: str, processes: int = 1) -> Rea
# We need to enforce the invariant that every realm must have a PresenceSequence.
PresenceSequence.objects.create(realm=realm, last_update_id=0)
named_user_group_id_to_creator_id = {}
if "zerver_usergroup" in data:
re_map_foreign_keys(data, "zerver_usergroup", "realm", related_table="realm")
bulk_import_model(data, UserGroup)
@ -1215,6 +1215,9 @@ def do_import_realm(import_dir: Path, subdomain: str, processes: int = 1) -> Rea
re_map_foreign_keys(
data, "zerver_namedusergroup", "realm_for_sharding", related_table="realm"
)
for group in data["zerver_namedusergroup"]:
creator_id = group.pop("creator_id", None)
named_user_group_id_to_creator_id[group["id"]] = creator_id
for setting_name in NamedUserGroup.GROUP_PERMISSION_SETTINGS:
re_map_foreign_keys(
data,
@ -1307,6 +1310,16 @@ def do_import_realm(import_dir: Path, subdomain: str, processes: int = 1) -> Rea
stream.creator_id = stream_id_to_creator_id[stream.id]
Stream.objects.bulk_update(streams, ["creator_id"])
if "zerver_namedusergroup" in data:
# UserProfiles have been loaded, so now we're ready to set .creator_id
# for groups based on the mapping we saved earlier.
named_user_groups = NamedUserGroup.objects.filter(
id__in=named_user_group_id_to_creator_id.keys()
)
for group in named_user_groups:
group.creator_id = named_user_group_id_to_creator_id[group.id]
NamedUserGroup.objects.bulk_update(named_user_groups, ["creator_id"])
re_map_foreign_keys(data, "zerver_defaultstream", "stream", related_table="stream")
re_map_foreign_keys(data, "zerver_realmemoji", "author", related_table="user_profile")

19
zerver/lib/user_groups.py
View File

@ -139,13 +139,6 @@ def check_permission_for_managing_all_groups(
permission, which is a permission that requires either certain roles
or membership in the group itself to be used.
"""
# This is a temporary exception and this should be removed as soon
# as `group_creator` is set as a default for `can_manage_group`
# property of user groups. See this topic for more details:
# https://chat.zulip.org/#narrow/stream/3-backend/topic/Group.20creation.20-.20who.20can.20change.20the.20setting.2E/near/1943861
if user_group.creator and user_group.creator.id == user_profile.id:
return True
can_manage_all_groups = user_profile.can_manage_all_groups()
if can_manage_all_groups:
if user_profile.is_realm_admin or user_profile.is_moderator:
@ -789,6 +782,18 @@ def set_defaults_for_group_settings(
else:
default_group_name = permission_config.default_group_name
if default_group_name == "group_creator":
if user_group.creator:
default_group = UserGroup(
realm=user_group.realm,
)
default_group.save()
UserGroupMembership.objects.create(
user_profile=user_group.creator, user_group=default_group
)
else:
raise AssertionError("Group creator should not be None.")
else:
default_group = system_groups_name_dict[default_group_name].usergroup_ptr
setattr(user_group, setting_name, default_group)

2
zerver/models/groups.py
View File

@ -100,7 +100,7 @@ class NamedUserGroup(UserGroup): # type: ignore[django-manager-missing] # djang
allow_owners_group=True,
allow_nobody_group=True,
allow_everyone_group=False,
default_group_name=SystemGroups.NOBODY,
default_group_name="group_creator",
default_for_system_groups=SystemGroups.NOBODY,
id_field_name="can_manage_group_id",
),

2
zerver/openapi/zulip.yaml
View File

@ -23940,7 +23940,7 @@ components:
The ID of the target user group.
schema:
type: integer
example: 33
example: 34
required: true
QueueId:
name: queue_id

4
zerver/tests/test_audit_log.py
View File

@ -1201,7 +1201,7 @@ class TestRealmAuditLog(ZulipTestCase):
hamlet = self.example_user("hamlet")
cordelia = self.example_user("cordelia")
now = timezone_now()
user_group = check_add_user_group(hamlet.realm, "foo", [], acting_user=None)
user_group = check_add_user_group(hamlet.realm, "foo", [], acting_user=hamlet)
bulk_add_members_to_user_groups([user_group], [hamlet.id, cordelia.id], acting_user=hamlet)
audit_log_entries = RealmAuditLog.objects.filter(
@ -1228,7 +1228,7 @@ class TestRealmAuditLog(ZulipTestCase):
def test_change_user_group_subgroups_memberships(self) -> None:
hamlet = self.example_user("hamlet")
user_group = check_add_user_group(hamlet.realm, "main", [], acting_user=None)
user_group = check_add_user_group(hamlet.realm, "main", [], acting_user=hamlet)
subgroups = [
check_add_user_group(hamlet.realm, f"subgroup{num}", [], acting_user=hamlet)
for num in range(3)

2
zerver/tests/test_auth_backends.py
View File

@ -7804,7 +7804,7 @@ class LDAPGroupSyncTest(ZulipTestCase):
)
create_user_group_in_database(
"cool_test_group", [], realm, acting_user=None, description="Created by LDAP sync"
"cool_test_group", [], realm, acting_user=hamlet, description="Created by LDAP sync"
)
self.assertTrue(

2
zerver/tests/test_event_queue.py
View File

@ -528,7 +528,7 @@ class MissedMessageHookTest(ZulipTestCase):
self.cordelia.realm,
"hamlet_and_cordelia",
[self.user_profile, self.cordelia],
acting_user=None,
acting_user=self.user_profile,
)
msg_id = self.send_stream_message(
self.iago, "Denmark", content="@*hamlet_and_cordelia* what's up?"

12
zerver/tests/test_events.py
View File

@ -1832,13 +1832,13 @@ class NormalActionsTest(BaseAction):
othello = self.example_user("othello")
with self.verify_action() as events:
check_add_user_group(
self.user_profile.realm, "backend", [othello], "Backend team", acting_user=None
self.user_profile.realm, "backend", [othello], "Backend team", acting_user=othello
)
check_user_group_add("events[0]", events[0])
nobody_group = NamedUserGroup.objects.get(
name=SystemGroups.NOBODY, realm=self.user_profile.realm, is_system_group=True
self.assertEqual(
events[0]["group"]["can_manage_group"],
AnonymousSettingGroupDict(direct_members=[12], direct_subgroups=[]),
)
self.assertEqual(events[0]["group"]["can_manage_group"], nobody_group.id)
everyone_group = NamedUserGroup.objects.get(
name=SystemGroups.EVERYONE, realm=self.user_profile.realm, is_system_group=True
)
@ -1857,7 +1857,7 @@ class NormalActionsTest(BaseAction):
[othello],
"",
{"can_manage_group": user_group, "can_mention_group": user_group},
acting_user=None,
acting_user=othello,
)
check_user_group_add("events[0]", events[0])
self.assertEqual(
@ -1930,7 +1930,7 @@ class NormalActionsTest(BaseAction):
check_user_group_remove_members("events[0]", events[0])
api_design = check_add_user_group(
hamlet.realm, "api-design", [hamlet], description="API design team", acting_user=None
hamlet.realm, "api-design", [hamlet], description="API design team", acting_user=othello
)
# Test add subgroups

9
zerver/tests/test_import_export.py
View File

@ -503,7 +503,7 @@ class RealmImportExportTest(ExportFile):
self.assertEqual(exported_realm_user_default[0]["default_language"], "de")
exported_usergroups = data["zerver_usergroup"]
self.assert_length(exported_usergroups, 9)
self.assert_length(exported_usergroups, 10)
self.assertFalse("direct_members" in exported_usergroups[2])
self.assertFalse("direct_subgroups" in exported_usergroups[2])
@ -1406,7 +1406,12 @@ class RealmImportExportTest(ExportFile):
@getter
def get_user_group_names(r: Realm) -> set[str]:
return {group.named_user_group.name for group in UserGroup.objects.filter(realm=r)}
result = set()
for group in UserGroup.objects.filter(realm=r):
if hasattr(group, "named_user_group"):
result.add(group.named_user_group.name)
return result
@getter
def get_named_user_group_names(r: Realm) -> set[str]:

2
zerver/tests/test_markdown.py
View File

@ -2694,7 +2694,7 @@ class MarkdownMentionTest(ZulipTestCase):
def create_user_group_for_test(self, user_group_name: str) -> NamedUserGroup:
othello = self.example_user("othello")
return check_add_user_group(
get_realm("zulip"), user_group_name, [othello], acting_user=None
get_realm("zulip"), user_group_name, [othello], acting_user=othello
)
def test_user_group_mention_single(self) -> None:

4
zerver/tests/test_message_delete.py
View File

@ -411,7 +411,9 @@ class DeleteMessageTest(ZulipTestCase):
# of the group which is allowed to delete any message.
polonius = self.example_user("polonius")
hamlet = self.example_user("hamlet")
user_group = check_add_user_group(realm, "test-group", [hamlet, polonius], acting_user=None)
user_group = check_add_user_group(
realm, "test-group", [hamlet, polonius], acting_user=hamlet
)
do_change_realm_permission_group_setting(
realm,
"can_delete_any_message_group",

8
zerver/tests/test_message_edit.py
View File

@ -1460,8 +1460,10 @@ class EditMessageTest(ZulipTestCase):
self.subscribe(othello, "test_stream")
self.subscribe(cordelia, "test_stream")
leadership = check_add_user_group(othello.realm, "leadership", [othello], acting_user=None)
support = check_add_user_group(othello.realm, "support", [othello], acting_user=None)
leadership = check_add_user_group(
othello.realm, "leadership", [othello], acting_user=othello
)
support = check_add_user_group(othello.realm, "support", [othello], acting_user=othello)
moderators_system_group = NamedUserGroup.objects.get(
realm=iago.realm, name=SystemGroups.MODERATORS, is_system_group=True
@ -1526,7 +1528,7 @@ class EditMessageTest(ZulipTestCase):
)
self.assert_json_success(result)
test = check_add_user_group(shiva.realm, "test", [shiva], acting_user=None)
test = check_add_user_group(shiva.realm, "test", [shiva], acting_user=shiva)
add_subgroups_to_user_group(leadership, [test], acting_user=None)
support.can_mention_group = leadership
support.save()

14
zerver/tests/test_message_notification_emails.py
View File

@ -679,10 +679,10 @@ class TestMessageNotificationEmails(ZulipTestCase):
cordelia = self.example_user("cordelia")
hamlet_only = check_add_user_group(
get_realm("zulip"), "hamlet_only", [hamlet], acting_user=None
get_realm("zulip"), "hamlet_only", [hamlet], acting_user=hamlet
)
hamlet_and_cordelia = check_add_user_group(
get_realm("zulip"), "hamlet_and_cordelia", [hamlet, cordelia], acting_user=None
get_realm("zulip"), "hamlet_and_cordelia", [hamlet, cordelia], acting_user=hamlet
)
hamlet_only_message_id = self.send_stream_message(othello, "Denmark", "@*hamlet_only*")
@ -717,7 +717,7 @@ class TestMessageNotificationEmails(ZulipTestCase):
othello = self.example_user("othello")
hamlet_and_cordelia = check_add_user_group(
get_realm("zulip"), "hamlet_and_cordelia", [hamlet, cordelia], acting_user=None
get_realm("zulip"), "hamlet_and_cordelia", [hamlet, cordelia], acting_user=hamlet
)
user_group_mentioned_message_id = self.send_stream_message(
@ -754,7 +754,7 @@ class TestMessageNotificationEmails(ZulipTestCase):
othello = self.example_user("othello")
hamlet_and_cordelia = check_add_user_group(
get_realm("zulip"), "hamlet_and_cordelia", [hamlet, cordelia], acting_user=None
get_realm("zulip"), "hamlet_and_cordelia", [hamlet, cordelia], acting_user=hamlet
)
topic_wildcard_mentioned_in_followed_topic_message_id = self.send_stream_message(
@ -1628,14 +1628,14 @@ class TestMessageNotificationEmails(ZulipTestCase):
# user groups having upto 'MAX_GROUP_SIZE_FOR_MENTION_REACTIVATION'
# members are small user groups.
small_user_group = check_add_user_group(
zulip_realm, "small_user_group", [hamlet, othello], acting_user=None
zulip_realm, "small_user_group", [hamlet, othello], acting_user=hamlet
)
large_user_group = check_add_user_group(
zulip_realm, "large_user_group", [hamlet], acting_user=None
zulip_realm, "large_user_group", [hamlet], acting_user=hamlet
)
subgroup = check_add_user_group(
zulip_realm, "subgroup", [othello, cordelia], acting_user=None
zulip_realm, "subgroup", [othello, cordelia], acting_user=hamlet
)
add_subgroups_to_user_group(large_user_group, [subgroup], acting_user=None)

8
zerver/tests/test_message_send.py
View File

@ -2134,8 +2134,10 @@ class StreamMessagesTest(ZulipTestCase):
self.subscribe(othello, "test_stream")
self.subscribe(cordelia, "test_stream")
leadership = check_add_user_group(othello.realm, "leadership", [othello], acting_user=None)
support = check_add_user_group(othello.realm, "support", [othello], acting_user=None)
leadership = check_add_user_group(
othello.realm, "leadership", [othello], acting_user=othello
)
support = check_add_user_group(othello.realm, "support", [othello], acting_user=othello)
moderators_system_group = NamedUserGroup.objects.get(
realm=iago.realm, name=SystemGroups.MODERATORS, is_system_group=True
@ -2169,7 +2171,7 @@ class StreamMessagesTest(ZulipTestCase):
result = self.api_get(iago, "/api/v1/messages/" + str(msg_id))
self.assert_json_success(result)
test = check_add_user_group(shiva.realm, "test", [shiva], acting_user=None)
test = check_add_user_group(shiva.realm, "test", [shiva], acting_user=shiva)
add_subgroups_to_user_group(leadership, [test], acting_user=None)
support.can_mention_group = leadership
support.save()

4
zerver/tests/test_notification_data.py
View File

@ -373,9 +373,9 @@ class TestNotificationData(ZulipTestCase):
cordelia = self.example_user("cordelia")
realm = hamlet.realm
hamlet_only = check_add_user_group(realm, "hamlet_only", [hamlet], acting_user=None)
hamlet_only = check_add_user_group(realm, "hamlet_only", [hamlet], acting_user=hamlet)
hamlet_and_cordelia = check_add_user_group(
realm, "hamlet_and_cordelia", [hamlet, cordelia], acting_user=None
realm, "hamlet_and_cordelia", [hamlet, cordelia], acting_user=hamlet
)
mention_backend = MentionBackend(realm.id)

8
zerver/tests/test_push_notifications.py
View File

@ -3753,14 +3753,14 @@ class HandlePushNotificationTest(PushNotificationTest):
zulip_realm,
"small_user_group",
[self.user_profile, othello],
acting_user=None,
acting_user=othello,
)
large_user_group = check_add_user_group(
zulip_realm, "large_user_group", [self.user_profile], acting_user=None
zulip_realm, "large_user_group", [self.user_profile], acting_user=othello
)
subgroup = check_add_user_group(
zulip_realm, "subgroup", [othello, cordelia], acting_user=None
zulip_realm, "subgroup", [othello, cordelia], acting_user=othello
)
add_subgroups_to_user_group(large_user_group, [subgroup], acting_user=None)
@ -4266,7 +4266,7 @@ class TestGetAPNsPayload(PushNotificationTest):
def test_get_message_payload_apns_user_group_mention(self) -> None:
user_profile = self.example_user("othello")
user_group = check_add_user_group(
get_realm("zulip"), "test_user_group", [user_profile], acting_user=None
get_realm("zulip"), "test_user_group", [user_profile], acting_user=user_profile
)
stream = Stream.objects.filter(name="Verona").get()
message = self.get_message(Recipient.STREAM, stream.id, stream.realm_id)

5
zerver/tests/test_realm.py
View File

@ -2087,9 +2087,8 @@ class RealmAPITest(ZulipTestCase):
with self.subTest(property=prop):
self.do_test_realm_update_api(prop)
check_add_user_group(
get_realm("zulip"), "leadership", [self.example_user("hamlet")], acting_user=None
)
hamlet = self.example_user("hamlet")
check_add_user_group(get_realm("zulip"), "leadership", [hamlet], acting_user=hamlet)
for prop in Realm.REALM_PERMISSION_GROUP_SETTINGS:
with self.subTest(property=prop):
self.do_test_realm_permission_group_setting_update_api(prop)

16
zerver/tests/test_subs.py
View File

@ -2832,15 +2832,15 @@ class StreamAdminTest(ZulipTestCase):
def test_can_remove_subscribers_group(self) -> None:
realm = get_realm("zulip")
iago = self.example_user("iago")
leadership_group = check_add_user_group(
realm,
"leadership",
[self.example_user("iago"), self.example_user("shiva")],
acting_user=None,
)
managers_group = check_add_user_group(
realm, "managers", [self.example_user("hamlet")], acting_user=None
[iago, self.example_user("shiva")],
acting_user=iago,
)
hamlet = self.example_user("hamlet")
managers_group = check_add_user_group(realm, "managers", [hamlet], acting_user=hamlet)
add_subgroups_to_user_group(managers_group, [leadership_group], acting_user=None)
cordelia = self.example_user("cordelia")
@ -4469,7 +4469,9 @@ class SubscriptionAPITest(ZulipTestCase):
do_set_realm_property(realm, "waiting_period_threshold", 0, acting_user=None)
self.common_subscribe_to_streams(cordelia, ["new_stream2"], invite_only=invite_only)
leadership_group = check_add_user_group(realm, "Leadership", [desdemona], acting_user=None)
leadership_group = check_add_user_group(
realm, "Leadership", [desdemona], acting_user=desdemona
)
do_change_realm_permission_group_setting(
realm, stream_policy, leadership_group, acting_user=None
)
@ -4484,7 +4486,7 @@ class SubscriptionAPITest(ZulipTestCase):
self.common_subscribe_to_streams(desdemona, ["new_stream3"], invite_only=invite_only)
staff_group = check_add_user_group(realm, "Staff", [iago], acting_user=None)
staff_group = check_add_user_group(realm, "Staff", [iago], acting_user=iago)
setting_group = self.create_or_update_anonymous_group_for_setting([cordelia], [staff_group])
do_change_realm_permission_group_setting(
realm, stream_policy, setting_group, acting_user=None

94
zerver/tests/test_user_groups.py
View File

@ -75,7 +75,7 @@ class UserGroupTestCase(ZulipTestCase):
self.assertSetEqual(set(subgroup_ids), {member.id for member in members})
def create_user_group_for_test(
self, group_name: str, acting_user: UserProfile | None = None
self, group_name: str, acting_user: UserProfile
) -> NamedUserGroup:
members = [self.example_user("othello")]
return check_add_user_group(
@ -145,7 +145,10 @@ class UserGroupTestCase(ZulipTestCase):
self.assertEqual(user_groups[9]["name"], "newgroup")
self.assertEqual(user_groups[9]["description"], "")
self.assertEqual(user_groups[9]["members"], [])
self.assertEqual(user_groups[9]["can_manage_group"], user_group.id)
self.assertEqual(
user_groups[9]["can_manage_group"],
AnonymousSettingGroupDict(direct_members=[11], direct_subgroups=[]),
)
self.assertEqual(user_groups[9]["can_mention_group"], everyone_group.id)
self.assertFalse(user_groups[0]["deactivated"])
@ -162,7 +165,7 @@ class UserGroupTestCase(ZulipTestCase):
"can_manage_group": setting_group,
"can_mention_group": setting_group,
},
acting_user=None,
acting_user=self.example_user("desdemona"),
)
user_groups = user_groups_in_realm_serialized(realm, include_deactivated_groups=False)
self.assertEqual(user_groups[10]["id"], new_user_group.id)
@ -190,9 +193,8 @@ class UserGroupTestCase(ZulipTestCase):
)
self.assertFalse(user_groups[0]["deactivated"])
another_new_group = check_add_user_group(
realm, "newgroup3", [self.example_user("hamlet")], acting_user=None
)
hamlet = self.example_user("hamlet")
another_new_group = check_add_user_group(realm, "newgroup3", [hamlet], acting_user=hamlet)
add_subgroups_to_user_group(
new_user_group, [another_new_group, owners_system_group], acting_user=None
)
@ -220,7 +222,7 @@ class UserGroupTestCase(ZulipTestCase):
def test_get_direct_user_groups(self) -> None:
othello = self.example_user("othello")
self.create_user_group_for_test("support")
self.create_user_group_for_test("support", acting_user=self.example_user("desdemona"))
user_groups = get_direct_user_groups(othello)
self.assert_length(user_groups, 3)
# othello is a direct member of two role-based system groups also.
@ -236,12 +238,14 @@ class UserGroupTestCase(ZulipTestCase):
desdemona = self.example_user("desdemona")
shiva = self.example_user("shiva")
leadership_group = check_add_user_group(realm, "Leadership", [desdemona], acting_user=None)
leadership_group = check_add_user_group(
realm, "Leadership", [desdemona], acting_user=desdemona
)
staff_group = check_add_user_group(realm, "Staff", [iago], acting_user=None)
staff_group = check_add_user_group(realm, "Staff", [iago], acting_user=iago)
GroupGroupMembership.objects.create(supergroup=staff_group, subgroup=leadership_group)
everyone_group = check_add_user_group(realm, "Everyone", [shiva], acting_user=None)
everyone_group = check_add_user_group(realm, "Everyone", [shiva], acting_user=shiva)
GroupGroupMembership.objects.create(supergroup=everyone_group, subgroup=staff_group)
self.assertCountEqual(
@ -402,13 +406,13 @@ class UserGroupTestCase(ZulipTestCase):
def test_has_user_group_access_for_subgroup(self) -> None:
iago = self.example_user("iago")
zulip_realm = get_realm("zulip")
zulip_group = check_add_user_group(zulip_realm, "zulip", [], acting_user=None)
zulip_group = check_add_user_group(zulip_realm, "zulip", [], acting_user=iago)
moderators_group = NamedUserGroup.objects.get(
name=SystemGroups.MODERATORS, realm=zulip_realm, is_system_group=True
)
lear_realm = get_realm("lear")
lear_group = check_add_user_group(lear_realm, "test", [], acting_user=None)
lear_group = check_add_user_group(lear_realm, "test", [], acting_user=iago)
self.assertFalse(has_user_group_access_for_subgroup(lear_group, iago))
self.assertTrue(has_user_group_access_for_subgroup(zulip_group, iago))
@ -434,11 +438,8 @@ class UserGroupAPITestCase(UserGroupTestCase):
everyone_system_group = NamedUserGroup.objects.get(
name="role:everyone", realm=hamlet.realm, is_system_group=True
)
nobody_system_group = NamedUserGroup.objects.get(
name=SystemGroups.NOBODY, realm=hamlet.realm, is_system_group=True
)
support_group = NamedUserGroup.objects.get(name="support", realm=hamlet.realm)
self.assertEqual(support_group.can_manage_group, nobody_system_group.usergroup_ptr)
self.assertCountEqual(support_group.can_manage_group.direct_members.all(), [hamlet])
self.assertEqual(support_group.can_mention_group, everyone_system_group.usergroup_ptr)
# Test invalid member error
@ -521,7 +522,7 @@ class UserGroupAPITestCase(UserGroupTestCase):
permission_configuration = NamedUserGroup.GROUP_PERMISSION_SETTINGS[setting_name]
leadership_group = check_add_user_group(
hamlet.realm, "leadership", [hamlet], acting_user=None
hamlet.realm, "leadership", [hamlet], acting_user=hamlet
)
moderators_group = NamedUserGroup.objects.get(
name="role:moderators", realm=hamlet.realm, is_system_group=True
@ -802,8 +803,9 @@ class UserGroupAPITestCase(UserGroupTestCase):
self.assert_json_error(result, "Invalid user group")
lear_realm = get_realm("lear")
lear_cordelia = self.lear_user("cordelia")
lear_test_group = check_add_user_group(
lear_realm, "test", [self.lear_user("cordelia")], acting_user=None
lear_realm, "test", [lear_cordelia], acting_user=lear_cordelia
)
result = self.client_patch(f"/json/user_groups/{lear_test_group.id}", info=params)
self.assert_json_error(result, "Invalid user group")
@ -860,7 +862,7 @@ class UserGroupAPITestCase(UserGroupTestCase):
name="role:moderators", realm=hamlet.realm, is_system_group=True
)
self.login("hamlet")
self.login("desdemona")
params = {}
params[setting_name] = orjson.dumps(
{
@ -1064,7 +1066,7 @@ class UserGroupAPITestCase(UserGroupTestCase):
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)
check_add_user_group(hamlet.realm, "marketing", [hamlet], acting_user=None)
check_add_user_group(hamlet.realm, "marketing", [hamlet], acting_user=hamlet)
check_add_user_group(hamlet.realm, "leadership", [hamlet], acting_user=hamlet)
for setting_name in NamedUserGroup.GROUP_PERMISSION_SETTINGS:
@ -1089,7 +1091,7 @@ class UserGroupAPITestCase(UserGroupTestCase):
hamlet = self.example_user("hamlet")
support_group = check_add_user_group(hamlet.realm, "support", [hamlet], acting_user=hamlet)
marketing_group = check_add_user_group(
hamlet.realm, "marketing", [hamlet], acting_user=None
hamlet.realm, "marketing", [hamlet], acting_user=hamlet
)
everyone_group = NamedUserGroup.objects.get(
name="role:everyone", realm=hamlet.realm, is_system_group=True
@ -1248,8 +1250,12 @@ class UserGroupAPITestCase(UserGroupTestCase):
self.assert_json_error(result, "Invalid user group")
def test_user_group_deactivation(self) -> None:
support_group = self.create_user_group_for_test("support")
leadership_group = self.create_user_group_for_test("leadership")
support_group = self.create_user_group_for_test(
"support", acting_user=self.example_user("desdemona")
)
leadership_group = self.create_user_group_for_test(
"leadership", acting_user=self.example_user("othello")
)
add_subgroups_to_user_group(support_group, [leadership_group], acting_user=None)
realm = get_realm("zulip")
@ -1402,10 +1408,11 @@ class UserGroupAPITestCase(UserGroupTestCase):
def test_user_group_deactivation_with_group_used_for_settings(self) -> None:
realm = get_realm("zulip")
othello = self.example_user("othello")
hamlet = self.example_user("hamlet")
support_group = self.create_user_group_for_test("support", acting_user=othello)
support_group = self.create_user_group_for_test(
"support", acting_user=self.example_user("othello")
)
moderators_group = NamedUserGroup.objects.get(
name=SystemGroups.MODERATORS, realm=realm, is_system_group=True
)
@ -1500,7 +1507,9 @@ class UserGroupAPITestCase(UserGroupTestCase):
stream, setting_name, moderators_group, acting_user=None
)
leadership_group = self.create_user_group_for_test("leadership")
leadership_group = self.create_user_group_for_test(
"leadership", acting_user=self.example_user("othello")
)
for setting_name in NamedUserGroup.GROUP_PERMISSION_SETTINGS:
do_change_user_group_permission_setting(
leadership_group, setting_name, support_group, acting_user=None
@ -1592,7 +1601,7 @@ class UserGroupAPITestCase(UserGroupTestCase):
for i in range(50)
]
with self.assert_database_query_count(5):
with self.assert_database_query_count(7):
user_group = create_user_group_in_database(
name="support",
members=[hamlet, cordelia, *original_users],
@ -1619,7 +1628,7 @@ class UserGroupAPITestCase(UserGroupTestCase):
with (
mock.patch("zerver.views.user_groups.notify_for_user_group_subscription_changes"),
self.assert_database_query_count(11),
self.assert_database_query_count(14),
):
result = self.client_post(f"/json/user_groups/{user_group.id}/members", info=params)
self.assert_json_success(result)
@ -1760,7 +1769,7 @@ class UserGroupAPITestCase(UserGroupTestCase):
self.subscribe(user, stream_name)
check_add_user_group(
name=group_name, initial_members=list(support_team), realm=realm, acting_user=None
name=group_name, initial_members=list(support_team), realm=realm, acting_user=hamlet
)
payload = dict(
@ -2070,7 +2079,9 @@ class UserGroupAPITestCase(UserGroupTestCase):
check_update_user_group("help", "Troubleshooting team", "desdemona")
def test_realm_level_setting_for_updating_members(self) -> None:
user_group = self.create_user_group_for_test("support")
user_group = self.create_user_group_for_test(
"support", acting_user=self.example_user("desdemona")
)
aaron = self.example_user("aaron")
othello = self.example_user("othello")
cordelia = self.example_user("cordelia")
@ -2194,11 +2205,11 @@ class UserGroupAPITestCase(UserGroupTestCase):
def test_group_level_setting_for_updating_members(self) -> None:
othello = self.example_user("othello")
user_group = check_add_user_group(
get_realm("zulip"), "support", [othello], acting_user=None
get_realm("zulip"), "support", [othello], acting_user=self.example_user("desdemona")
)
hamlet = self.example_user("hamlet")
leadership_group = check_add_user_group(
get_realm("zulip"), "leadership", [hamlet], acting_user=None
get_realm("zulip"), "leadership", [hamlet], acting_user=hamlet
)
aaron = self.example_user("aaron")
@ -2383,12 +2394,12 @@ class UserGroupAPITestCase(UserGroupTestCase):
)
leadership_group = check_add_user_group(
realm, "leadership", [desdemona, iago, hamlet], acting_user=None
realm, "leadership", [desdemona, iago, hamlet], acting_user=desdemona
)
support_group = check_add_user_group(
realm, "support", [hamlet, othello], acting_user=hamlet
)
test_group = check_add_user_group(realm, "test", [hamlet], acting_user=None)
test_group = check_add_user_group(realm, "test", [hamlet], acting_user=hamlet)
self.login("cordelia")
# Non-admin and non-moderators who are not a member of group cannot add or remove subgroups.
@ -2470,8 +2481,9 @@ class UserGroupAPITestCase(UserGroupTestCase):
self.assert_subgroup_membership(test_group, [support_group])
lear_realm = get_realm("lear")
lear_cordelia = self.lear_user("cordelia")
lear_test_group = check_add_user_group(
lear_realm, "test", [self.lear_user("cordelia")], acting_user=None
lear_realm, "test", [lear_cordelia], acting_user=lear_cordelia
)
result = self.client_post(f"/json/user_groups/{lear_test_group.id}/subgroups", info=params)
self.assert_json_error(result, "Invalid user group")
@ -2534,7 +2546,7 @@ class UserGroupAPITestCase(UserGroupTestCase):
lear_realm = get_realm("lear")
lear_cordelia = self.lear_user("cordelia")
lear_test_group = check_add_user_group(
lear_realm, "test", [lear_cordelia], acting_user=None
lear_realm, "test", [lear_cordelia], acting_user=lear_cordelia
)
result = self.client_get(
f"/json/user_groups/{lear_test_group.id}/members/{lear_cordelia.id}"
@ -2595,8 +2607,9 @@ class UserGroupAPITestCase(UserGroupTestCase):
self.assert_json_error(result, "Invalid user group")
lear_realm = get_realm("lear")
lear_cordelia = self.lear_user("cordelia")
lear_test_group = check_add_user_group(
lear_realm, "test", [self.lear_user("cordelia")], acting_user=None
lear_realm, "test", [lear_cordelia], acting_user=lear_cordelia
)
result = self.client_get(f"/json/user_groups/{lear_test_group.id}/members")
self.assert_json_error(result, "Invalid user group")
@ -2643,8 +2656,9 @@ class UserGroupAPITestCase(UserGroupTestCase):
self.assert_json_error(result, "Invalid user group")
lear_realm = get_realm("lear")
lear_cordelia = self.lear_user("cordelia")
lear_test_group = check_add_user_group(
lear_realm, "test", [self.lear_user("cordelia")], acting_user=None
lear_realm, "test", [lear_cordelia], acting_user=lear_cordelia
)
result = self.client_get(f"/json/user_groups/{lear_test_group.id}/subgroups")
self.assert_json_error(result, "Invalid user group")
@ -2678,9 +2692,9 @@ class UserGroupAPITestCase(UserGroupTestCase):
self.assertCountEqual(result_dict["subgroups"], [admins_group.id])
def test_add_subgroup_from_wrong_realm(self) -> None:
other_realm = do_create_realm("other", "Other Realm")
other_user_group = check_add_user_group(other_realm, "user_group", [], acting_user=None)
iago = self.example_user("iago")
other_realm = do_create_realm("other", "Other Realm")
other_user_group = check_add_user_group(other_realm, "user_group", [], acting_user=iago)
realm = get_realm("zulip")
zulip_group = check_add_user_group(realm, "zulip_test", [], acting_user=iago)

8
zerver/transaction_tests/test_user_groups.py
View File

@ -64,7 +64,7 @@ def dev_update_subgroups(
class UserGroupRaceConditionTestCase(ZulipTransactionTestCase):
created_user_groups: list[UserGroup] = []
created_user_groups: list[NamedUserGroup] = []
counter = 0
CHAIN_LENGTH = 3
@ -73,7 +73,13 @@ class UserGroupRaceConditionTestCase(ZulipTransactionTestCase):
# Clean up the user groups created to minimize leakage
with transaction.atomic():
for group in self.created_user_groups:
# can_manage_group can be deleted as long as it's the
# default group_creator. If we start using non-default
# can_manage_group in this test, deleting that group
# should be reconsidered.
can_manage_group = group.can_manage_group
group.delete()
can_manage_group.delete()
transaction.on_commit(self.created_user_groups.clear)
super().tearDown()

3
zilencer/management/commands/populate_db.py
View File

@ -1414,10 +1414,11 @@ def choose_date_sent(
def create_user_groups() -> None:
zulip = get_realm("zulip")
cordelia = get_user_by_delivery_email("cordelia@zulip.com", zulip)
members = [
get_user_by_delivery_email("cordelia@zulip.com", zulip),
get_user_by_delivery_email("hamlet@zulip.com", zulip),
]
create_user_group_in_database(
"hamletcharacters", members, zulip, description="Characters of Hamlet", acting_user=None
"hamletcharacters", members, zulip, description="Characters of Hamlet", acting_user=cordelia
)