user_groups: Remove unneeded fields from UserGroup model.

This commit removes name, description, is_system_group and
can_mention_group fields from UserGroup model and rename
them in NamedUserGroup model.

Fixes #29554.
This commit is contained in:
Sahil Batra 2024-04-18 22:29:50 +05:30 committed by Tim Abbott
parent 27558315a2
commit 7518d550f2
8 changed files with 80 additions and 61 deletions

View File

@ -47,16 +47,11 @@ def create_user_group_in_database(
realm=realm,
description=description,
is_system_group=is_system_group,
named_group_name=name,
named_group_description=description,
named_group_is_system_group=is_system_group,
realm_for_sharding=realm,
)
for setting_name, setting_value in group_settings_map.items():
setattr(user_group, setting_name, setting_value)
named_group_setting_name = "named_group_" + setting_name
setattr(user_group, named_group_setting_name, setting_value)
system_groups_name_dict = get_role_based_system_groups_dict(realm)
user_group = set_defaults_for_group_settings(
@ -224,8 +219,7 @@ def do_update_user_group_name(
try:
old_value = user_group.name
user_group.name = name
user_group.named_group_name = name
user_group.save(update_fields=["name", "named_group_name"])
user_group.save(update_fields=["name"])
RealmAuditLog.objects.create(
realm=user_group.realm,
modified_user_group=user_group,
@ -248,8 +242,7 @@ def do_update_user_group_description(
) -> None:
old_value = user_group.description
user_group.description = description
user_group.named_group_description = description
user_group.save(update_fields=["description", "named_group_description"])
user_group.save(update_fields=["description"])
RealmAuditLog.objects.create(
realm=user_group.realm,
modified_user_group=user_group,
@ -447,8 +440,6 @@ def do_change_user_group_permission_setting(
) -> None:
old_value = getattr(user_group, setting_name)
setattr(user_group, setting_name, setting_value_group)
named_group_setting_name = "named_group_" + setting_name
setattr(user_group, named_group_setting_name, setting_value_group)
user_group.save()
RealmAuditLog.objects.create(
realm=user_group.realm,

View File

@ -694,10 +694,10 @@ def bulk_import_named_user_groups(data: TableData) -> None:
(
group["usergroup_ptr_id"],
group["realm_for_sharding_id"],
group["named_group_name"],
group["named_group_description"],
group["named_group_is_system_group"],
group["named_group_can_mention_group_id"],
group["name"],
group["description"],
group["is_system_group"],
group["can_mention_group_id"],
)
for group in data["zerver_namedusergroup"]
]
@ -1056,10 +1056,6 @@ def do_import_realm(import_dir: Path, subdomain: str, processes: int = 1) -> Rea
if "zerver_usergroup" in data:
re_map_foreign_keys(data, "zerver_usergroup", "realm", related_table="realm")
for setting_name in NamedUserGroup.GROUP_PERMISSION_SETTINGS:
re_map_foreign_keys(
data, "zerver_usergroup", setting_name, related_table="usergroup"
)
bulk_import_model(data, UserGroup)
if "zerver_namedusergroup" in data:
@ -1070,11 +1066,10 @@ def do_import_realm(import_dir: Path, subdomain: str, processes: int = 1) -> Rea
data, "zerver_namedusergroup", "realm_for_sharding", related_table="realm"
)
for setting_name in NamedUserGroup.GROUP_PERMISSION_SETTINGS:
named_group_setting_name = "named_group_" + setting_name
re_map_foreign_keys(
data,
"zerver_namedusergroup",
named_group_setting_name,
setting_name,
related_table="usergroup",
)
bulk_import_named_user_groups(data)

View File

@ -432,9 +432,6 @@ def set_defaults_for_group_settings(
default_group = system_groups_name_dict[default_group_name].usergroup_ptr
setattr(user_group, setting_name, default_group)
setting_name_for_named_object = "named_group_" + setting_name
setattr(user_group, setting_name_for_named_object, default_group)
return user_group
@ -443,20 +440,11 @@ def bulk_create_system_user_groups(groups: List[Dict[str, str]], realm: Realm) -
# settings since we can only set them to the correct values after the groups
# are created.
initial_group_setting_value = -1
rows = [
SQL("({},{},{},{},{})").format(
Literal(realm.id),
Literal(group["name"]),
Literal(group["description"]),
Literal(True),
Literal(initial_group_setting_value),
)
for group in groups
]
rows = [SQL("({})").format(Literal(realm.id))] * len(groups)
query = SQL(
"""
INSERT INTO zerver_usergroup (realm_id, name, description, is_system_group, can_mention_group_id)
INSERT INTO zerver_usergroup (realm_id)
VALUES {rows}
RETURNING id
"""
@ -559,9 +547,7 @@ def create_system_user_groups_for_realm(realm: Realm) -> Dict[int, NamedUserGrou
for group in system_user_groups_list:
user_group = set_defaults_for_group_settings(group, {}, system_groups_name_dict)
groups_with_updated_settings.append(user_group)
NamedUserGroup.objects.bulk_update(
groups_with_updated_settings, ["can_mention_group", "named_group_can_mention_group"]
)
NamedUserGroup.objects.bulk_update(groups_with_updated_settings, ["can_mention_group"])
subgroup_objects: List[GroupGroupMembership] = []
# "Nobody" system group is not a subgroup of any user group, since it is already empty.

View File

@ -0,0 +1,56 @@
# Generated by Django 4.2.12 on 2024-04-19 04:05
from django.db import migrations
class Migration(migrations.Migration):
dependencies = [
("zerver", "0514_update_usergroup_foreign_keys_to_namedusergroup"),
]
operations = [
migrations.AlterUniqueTogether(
name="usergroup",
unique_together=set(),
),
migrations.RemoveField(
model_name="usergroup",
name="can_mention_group",
),
migrations.RemoveField(
model_name="usergroup",
name="description",
),
migrations.RemoveField(
model_name="usergroup",
name="is_system_group",
),
migrations.RemoveField(
model_name="usergroup",
name="name",
),
migrations.RenameField(
model_name="namedusergroup",
old_name="named_group_can_mention_group",
new_name="can_mention_group",
),
migrations.RenameField(
model_name="namedusergroup",
old_name="named_group_description",
new_name="description",
),
migrations.RenameField(
model_name="namedusergroup",
old_name="named_group_is_system_group",
new_name="is_system_group",
),
migrations.RenameField(
model_name="namedusergroup",
old_name="named_group_name",
new_name="name",
),
migrations.AlterUniqueTogether(
name="namedusergroup",
unique_together={("realm_for_sharding", "name")},
),
]

View File

@ -19,7 +19,6 @@ class SystemGroups:
class UserGroup(models.Model): # type: ignore[django-manager-missing] # django-stubs cannot resolve the custom CTEManager yet https://github.com/typeddjango/django-stubs/issues/1023
objects: CTEManager = CTEManager()
name = models.CharField(max_length=100)
direct_members = models.ManyToManyField(
UserProfile, through="zerver.UserGroupMembership", related_name="direct_groups"
)
@ -31,13 +30,6 @@ class UserGroup(models.Model): # type: ignore[django-manager-missing] # django-
related_name="direct_supergroups",
)
realm = models.ForeignKey("zerver.Realm", on_delete=CASCADE)
description = models.TextField(default="")
is_system_group = models.BooleanField(default=False)
can_mention_group = models.ForeignKey("self", on_delete=models.RESTRICT)
class Meta:
unique_together = (("realm", "name"),)
class NamedUserGroup(UserGroup): # type: ignore[django-manager-missing] # django-stubs cannot resolve the custom CTEManager yet https://github.com/typeddjango/django-stubs/issues/1023
@ -58,11 +50,11 @@ class NamedUserGroup(UserGroup): # type: ignore[django-manager-missing] # djang
# setting also points to a UserGroup object.
related_name="named_user_group",
)
named_group_name = models.CharField(max_length=MAX_NAME_LENGTH, db_column="name")
named_group_description = models.TextField(default="", db_column="description")
named_group_is_system_group = models.BooleanField(default=False, db_column="is_system_group")
name = models.CharField(max_length=MAX_NAME_LENGTH, db_column="name")
description = models.TextField(default="", db_column="description")
is_system_group = models.BooleanField(default=False, db_column="is_system_group")
named_group_can_mention_group = models.ForeignKey(
can_mention_group = models.ForeignKey(
UserGroup, on_delete=models.RESTRICT, db_column="can_mention_group_id"
)
@ -108,7 +100,7 @@ class NamedUserGroup(UserGroup): # type: ignore[django-manager-missing] # djang
}
class Meta:
unique_together = (("realm_for_sharding", "named_group_name"),)
unique_together = (("realm_for_sharding", "name"),)
class UserGroupMembership(models.Model):

View File

@ -266,8 +266,6 @@ def get_temp_user_group_id() -> Dict[str, object]:
name="temp",
realm=get_realm("zulip"),
can_mention_group_id=11,
named_group_name="temp",
named_group_can_mention_group_id=11,
realm_for_sharding=get_realm("zulip"),
)
return {

View File

@ -454,13 +454,12 @@ class RealmImportExportTest(ExportFile):
exported_usergroups = data["zerver_usergroup"]
self.assert_length(exported_usergroups, 9)
self.assertEqual(exported_usergroups[2]["name"], "role:administrators")
self.assertFalse("direct_members" in exported_usergroups[2])
self.assertFalse("direct_subgroups" in exported_usergroups[2])
exported_namedusergroups = data["zerver_namedusergroup"]
self.assert_length(exported_namedusergroups, 9)
self.assertEqual(exported_namedusergroups[2]["named_group_name"], "role:administrators")
self.assertEqual(exported_namedusergroups[2]["name"], "role:administrators")
self.assertTrue("usergroup_ptr" in exported_namedusergroups[2])
self.assertTrue("realm_for_sharding" in exported_namedusergroups[2])
self.assertFalse("realm" in exported_namedusergroups[2])
@ -1072,7 +1071,7 @@ class RealmImportExportTest(ExportFile):
@getter
def get_group_names_for_group_settings(r: Realm) -> Set[str]:
return {
getattr(r, permission_name).name
getattr(r, permission_name).named_user_group.name
for permission_name in Realm.REALM_PERMISSION_GROUP_SETTINGS
}
@ -1231,7 +1230,7 @@ class RealmImportExportTest(ExportFile):
@getter
def get_user_group_names(r: Realm) -> Set[str]:
return {group.name for group in UserGroup.objects.filter(realm=r)}
return {group.named_user_group.name for group in UserGroup.objects.filter(realm=r)}
@getter
def get_named_user_group_names(r: Realm) -> Set[str]:
@ -1248,7 +1247,9 @@ class RealmImportExportTest(ExportFile):
def get_group_group_membership(r: Realm) -> Set[str]:
usergroup = NamedUserGroup.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}
subgroups = {
membership.subgroup.named_user_group.name for membership in group_group_membership
}
return subgroups
@getter
@ -1268,13 +1269,13 @@ class RealmImportExportTest(ExportFile):
# correctly since we do not include this in export data.
usergroup = NamedUserGroup.objects.get(realm=r, name="role:members")
direct_subgroups = usergroup.direct_subgroups.all()
direct_subgroup_names = {group.name for group in direct_subgroups}
direct_subgroup_names = {group.named_user_group.name for group in direct_subgroups}
return direct_subgroup_names
@getter
def get_user_group_can_mention_group_setting(r: Realm) -> str:
user_group = NamedUserGroup.objects.get(realm=r, name="hamletcharacters")
return user_group.can_mention_group.name
return user_group.can_mention_group.named_user_group.name
# test botstoragedata and botconfigdata
@getter

View File

@ -104,7 +104,7 @@ class UserGroupTestCase(ZulipTestCase):
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.
user_group_names = [group.name for group in user_groups]
user_group_names = [group.named_user_group.name for group in user_groups]
self.assertEqual(
set(user_group_names),
{"support", SystemGroups.MEMBERS, SystemGroups.FULL_MEMBERS},