From 02d0566dc5965c2682dbf037132f3736f8314fda Mon Sep 17 00:00:00 2001 From: roanster007 Date: Fri, 5 Jul 2024 16:43:40 +0530 Subject: [PATCH] refactor: Rename `Huddle` Django model class to `DirectMessageGroup`. This commit renames the "Huddle" Django model class to "DirectMessageGroup", while maintaining the same table -- "zerver_huddle". Fixes part of #28640. --- analytics/tests/test_counts.py | 6 +-- tools/rebuild-test-database | 2 +- zerver/data_import/import_util.py | 4 +- zerver/lib/export.py | 6 +-- zerver/lib/import_realm.py | 16 +++---- zerver/lib/narrow.py | 4 +- zerver/lib/recipient_users.py | 10 ++--- zerver/lib/test_helpers.py | 4 +- zerver/management/commands/delete_realm.py | 6 +-- ...name_huddle_directmessagegroup_and_more.py | 19 ++++++++ zerver/models/__init__.py | 2 +- zerver/models/recipients.py | 44 ++++++++++++------- zerver/tests/test_import_export.py | 15 ++++--- zerver/tests/test_typing.py | 10 +++-- zilencer/management/commands/populate_db.py | 4 +- 15 files changed, 94 insertions(+), 58 deletions(-) create mode 100644 zerver/migrations/0546_rename_huddle_directmessagegroup_and_more.py diff --git a/analytics/tests/test_counts.py b/analytics/tests/test_counts.py index 0d9b9b37ad..8145fde78d 100644 --- a/analytics/tests/test_counts.py +++ b/analytics/tests/test_counts.py @@ -63,7 +63,7 @@ from zerver.lib.user_counts import realm_user_count_by_role from zerver.lib.utils import assert_is_not_none from zerver.models import ( Client, - Huddle, + DirectMessageGroup, Message, NamedUserGroup, PreregistrationUser, @@ -174,12 +174,12 @@ class AnalyticsTestCase(ZulipTestCase): stream.save(update_fields=["recipient"]) return stream, recipient - def create_huddle_with_recipient(self, **kwargs: Any) -> Tuple[Huddle, Recipient]: + def create_huddle_with_recipient(self, **kwargs: Any) -> Tuple[DirectMessageGroup, Recipient]: self.name_counter += 1 defaults = {"huddle_hash": f"hash{self.name_counter}"} for key, value in defaults.items(): kwargs[key] = kwargs.get(key, value) - huddle = Huddle.objects.create(**kwargs) + huddle = DirectMessageGroup.objects.create(**kwargs) recipient = Recipient.objects.create(type_id=huddle.id, type=Recipient.DIRECT_MESSAGE_GROUP) huddle.recipient = recipient huddle.save(update_fields=["recipient"]) diff --git a/tools/rebuild-test-database b/tools/rebuild-test-database index 73cdad017d..70ca2550a2 100755 --- a/tools/rebuild-test-database +++ b/tools/rebuild-test-database @@ -37,7 +37,7 @@ create_zulip_test ./manage.py dumpdata \ zerver.UserProfile zerver.Stream zerver.Recipient \ - zerver.Subscription zerver.Message zerver.Huddle zerver.Realm \ + zerver.Subscription zerver.Message zerver.DirectMessageGroup zerver.Realm \ zerver.UserMessage zerver.Client \ zerver.DefaultStream >zerver/tests/fixtures/messages.json diff --git a/zerver/data_import/import_util.py b/zerver/data_import/import_util.py index 7cc5cb7377..f053a5b1c2 100644 --- a/zerver/data_import/import_util.py +++ b/zerver/data_import/import_util.py @@ -32,7 +32,7 @@ from zerver.lib.partial import partial from zerver.lib.stream_color import STREAM_ASSIGNMENT_COLORS as STREAM_COLORS from zerver.models import ( Attachment, - Huddle, + DirectMessageGroup, Message, Realm, RealmEmoji, @@ -486,7 +486,7 @@ def build_stream( def build_direct_message_group(direct_message_group_id: int) -> ZerverFieldsT: - direct_message_group = Huddle( + direct_message_group = DirectMessageGroup( id=direct_message_group_id, ) return model_to_dict(direct_message_group) diff --git a/zerver/lib/export.py b/zerver/lib/export.py index 149a406c76..9113ef2c64 100644 --- a/zerver/lib/export.py +++ b/zerver/lib/export.py @@ -41,8 +41,8 @@ from zerver.models import ( CustomProfileField, CustomProfileFieldValue, DefaultStream, + DirectMessageGroup, GroupGroupMembership, - Huddle, Message, MutedUser, NamedUserGroup, @@ -1131,7 +1131,7 @@ def custom_fetch_direct_message_groups(response: TableData, context: Context) -> ) realm_huddle_recipient_ids = {sub.recipient_id for sub in realm_huddle_subs} - # Mark all Huddles whose recipient ID contains a cross-realm user. + # Mark all Direct Message groups whose recipient ID contains a cross-realm user. unsafe_huddle_recipient_ids = set() for sub in Subscription.objects.select_related("user_profile").filter( recipient__in=realm_huddle_recipient_ids @@ -1157,7 +1157,7 @@ def custom_fetch_direct_message_groups(response: TableData, context: Context) -> response["_huddle_recipient"] = huddle_recipients response["_huddle_subscription"] = huddle_subscription_dicts - response["zerver_huddle"] = make_raw(Huddle.objects.filter(id__in=huddle_ids)) + response["zerver_huddle"] = make_raw(DirectMessageGroup.objects.filter(id__in=huddle_ids)) def custom_fetch_scheduled_messages(response: TableData, context: Context) -> None: diff --git a/zerver/lib/import_realm.py b/zerver/lib/import_realm.py index 0ed611f37d..085efb316d 100644 --- a/zerver/lib/import_realm.py +++ b/zerver/lib/import_realm.py @@ -50,8 +50,8 @@ from zerver.models import ( CustomProfileField, CustomProfileFieldValue, DefaultStream, + DirectMessageGroup, GroupGroupMembership, - Huddle, Message, MutedUser, NamedUserGroup, @@ -999,7 +999,7 @@ def disable_restricted_authentication_methods(data: TableData) -> None: # * Realm's announcements_streams and group_permissions # * UserProfile, in order by ID to avoid bot loop issues # * Now can do all realm_tables -# * Huddle +# * DirectMessageGroup # * Recipient # * Subscription # * Message @@ -1232,10 +1232,10 @@ def do_import_realm(import_dir: Path, subdomain: str, processes: int = 1) -> Rea realm_emoji.save(update_fields=["author_id"]) if "zerver_huddle" in data: - update_model_ids(Huddle, data, "huddle") - # We don't import Huddle yet, since we don't have the data to - # compute direct message group hashes until we've imported some - # of the tables below. + update_model_ids(DirectMessageGroup, data, "huddle") + # We don't import DirectMessageGroup yet, since we don't have + # the data to compute direct message group hashes until we've + # imported some of the tables below. # We can't get direct message group hashes without processing # subscriptions first, during which # get_direct_message_groups_from_subscription is called. @@ -1317,8 +1317,8 @@ def do_import_realm(import_dir: Path, subdomain: str, processes: int = 1) -> Rea if "zerver_huddle" in data: process_direct_message_group_hash(data, "zerver_huddle") - bulk_import_model(data, Huddle) - for direct_message_group in Huddle.objects.filter(recipient=None): + bulk_import_model(data, DirectMessageGroup) + for direct_message_group in DirectMessageGroup.objects.filter(recipient=None): recipient = Recipient.objects.get( type=Recipient.DIRECT_MESSAGE_GROUP, type_id=direct_message_group.id ) diff --git a/zerver/lib/narrow.py b/zerver/lib/narrow.py index c26d96fef5..8ce03ebe0d 100644 --- a/zerver/lib/narrow.py +++ b/zerver/lib/narrow.py @@ -72,7 +72,7 @@ from zerver.lib.validator import ( check_string_or_int_list, ) from zerver.models import ( - Huddle, + DirectMessageGroup, Message, Realm, Recipient, @@ -597,7 +597,7 @@ class NarrowBuilder: ) except (JsonableError, ValidationError): raise BadNarrowOperatorError("unknown user in " + str(operand)) - except Huddle.DoesNotExist: + except DirectMessageGroup.DoesNotExist: # Group DM where huddle doesn't exist return query.where(maybe_negate(false())) diff --git a/zerver/lib/recipient_users.py b/zerver/lib/recipient_users.py index 0c42e1a0f0..9f44e249b7 100644 --- a/zerver/lib/recipient_users.py +++ b/zerver/lib/recipient_users.py @@ -3,7 +3,7 @@ from typing import Dict, Optional, Sequence from django.core.exceptions import ValidationError from django.utils.translation import gettext as _ -from zerver.models import Huddle, Recipient, UserProfile +from zerver.models import DirectMessageGroup, Recipient, UserProfile from zerver.models.recipients import ( get_direct_message_group_hash, get_or_create_direct_message_group, @@ -56,10 +56,10 @@ def get_recipient_from_user_profiles( if create: direct_message_group = get_or_create_direct_message_group(user_ids) else: - # We intentionally let the Huddle.DoesNotExist escape, in the - # case that there is no such direct message group, and the user - # passed create=False - direct_message_group = Huddle.objects.get( + # We intentionally let the DirectMessageGroup.DoesNotExist escape, + # in the case that there is no such direct message group, and the + # user passed create=False + direct_message_group = DirectMessageGroup.objects.get( huddle_hash=get_direct_message_group_hash(user_ids) ) return Recipient( diff --git a/zerver/lib/test_helpers.py b/zerver/lib/test_helpers.py index 6f8797c683..888cc629bd 100644 --- a/zerver/lib/test_helpers.py +++ b/zerver/lib/test_helpers.py @@ -601,7 +601,7 @@ def use_db_models( DefaultStream = apps.get_model("zerver", "DefaultStream") DefaultStreamGroup = apps.get_model("zerver", "DefaultStreamGroup") EmailChangeStatus = apps.get_model("zerver", "EmailChangeStatus") - Huddle = apps.get_model("zerver", "Huddle") + DirectMessageGroup = apps.get_model("zerver", "DirectMessageGroup") Message = apps.get_model("zerver", "Message") MultiuseInvite = apps.get_model("zerver", "MultiuseInvite") OnboardingStep = apps.get_model("zerver", "OnboardingStep") @@ -645,7 +645,7 @@ def use_db_models( DefaultStream=DefaultStream, DefaultStreamGroup=DefaultStreamGroup, EmailChangeStatus=EmailChangeStatus, - Huddle=Huddle, + DirectMessageGroup=DirectMessageGroup, Message=Message, MultiuseInvite=MultiuseInvite, UserTopic=UserTopic, diff --git a/zerver/management/commands/delete_realm.py b/zerver/management/commands/delete_realm.py index 8d59c80af8..6b7dedb22f 100644 --- a/zerver/management/commands/delete_realm.py +++ b/zerver/management/commands/delete_realm.py @@ -59,9 +59,9 @@ realms used for testing; consider using deactivate_realm instead.""" # storage; failing to do this leaves dangling files do_delete_all_realm_attachments(realm) - # TODO: This approach leaks Recipient and Huddle objects, - # because those don't have a foreign key to the Realm or any - # other model it cascades to (Realm/Stream/UserProfile/etc.). + # TODO: This approach leaks Recipient and DirectMessageGroup + # objects, because those don't have a foreign key to the Realm + # or any other model it cascades to (Realm/Stream/UserProfile/etc.). realm.delete() print("Realm has been successfully permanently deleted.") diff --git a/zerver/migrations/0546_rename_huddle_directmessagegroup_and_more.py b/zerver/migrations/0546_rename_huddle_directmessagegroup_and_more.py new file mode 100644 index 0000000000..af757a7bf9 --- /dev/null +++ b/zerver/migrations/0546_rename_huddle_directmessagegroup_and_more.py @@ -0,0 +1,19 @@ +# Generated by Django 5.0.6 on 2024-07-05 10:44 + +from django.db import migrations + + +class Migration(migrations.Migration): + dependencies = [ + ("zerver", "0545_attachment_content_type"), + ] + + operations = [ + migrations.SeparateDatabaseAndState( + database_operations=[], + state_operations=[ + migrations.RenameModel(old_name="Huddle", new_name="DirectMessageGroup"), + migrations.AlterModelTable(name="directmessagegroup", table="zerver_huddle"), + ], + ) + ] diff --git a/zerver/models/__init__.py b/zerver/models/__init__.py index 56cf65838c..d2a5954c7d 100644 --- a/zerver/models/__init__.py +++ b/zerver/models/__init__.py @@ -48,7 +48,7 @@ from zerver.models.realm_playgrounds import RealmPlayground as RealmPlayground from zerver.models.realms import Realm as Realm from zerver.models.realms import RealmAuthenticationMethod as RealmAuthenticationMethod from zerver.models.realms import RealmDomain as RealmDomain -from zerver.models.recipients import Huddle as Huddle +from zerver.models.recipients import DirectMessageGroup as DirectMessageGroup from zerver.models.recipients import Recipient as Recipient from zerver.models.scheduled_jobs import AbstractScheduledJob as AbstractScheduledJob from zerver.models.scheduled_jobs import MissedMessageEmailAddress as MissedMessageEmailAddress diff --git a/zerver/models/recipients.py b/zerver/models/recipients.py index 6ecc2f2632..553ede0253 100644 --- a/zerver/models/recipients.py +++ b/zerver/models/recipients.py @@ -21,18 +21,20 @@ class Recipient(models.Model): of audiences Zulip supports for a message. Recipient has just two attributes: The enum type, and a type_id, - which is the ID of the UserProfile/Stream/Huddle object containing - all the metadata for the audience. There are 3 recipient types: + which is the ID of the UserProfile/Stream/DirectMessageGroup object + containing all the metadata for the audience. There are 3 recipient + types: 1. 1:1 direct message: The type_id is the ID of the UserProfile who will receive any message to this Recipient. The sender of such a message is represented separately. 2. Stream message: The type_id is the ID of the associated Stream. 3. Group direct message: In Zulip, group direct messages are - represented by Huddle objects, which encode the set of users - in the conversation. The type_id is the ID of the associated Huddle - object; the set of users is usually retrieved via the Subscription - table. See the Huddle model for details. + represented by DirectMessageGroup objects, which encode the set of + users in the conversation. The type_id is the ID of the associated + DirectMessageGroup object; the set of users is usually retrieved + via the Subscription table. See the DirectMessageGroup model for + details. See also the Subscription model, which stores which UserProfile objects are subscribed to which Recipient objects. @@ -112,26 +114,33 @@ def bulk_get_direct_message_group_user_ids(recipient_ids: List[int]) -> Dict[int return result_dict -class Huddle(models.Model): +class DirectMessageGroup(models.Model): """ Represents a group of individuals who may have a group direct message conversation together. - The membership of the Huddle is stored in the Subscription table just like with - Streams - for each user in the Huddle, there is a Subscription object - tied to the UserProfile and the Huddle's recipient object. + The membership of the DirectMessageGroup is stored in the Subscription + table just like with Streams - for each user in the DirectMessageGroup, + there is a Subscription object tied to the UserProfile and the + DirectMessageGroup's recipient object. A hash of the list of user IDs is stored in the huddle_hash field below, to support efficiently mapping from a set of users to the - corresponding Huddle object. + corresponding DirectMessageGroup object. """ # TODO: We should consider whether using # CommaSeparatedIntegerField would be better. huddle_hash = models.CharField(max_length=40, db_index=True, unique=True) - # Foreign key to the Recipient object for this Huddle. + # Foreign key to the Recipient object for this DirectMessageGroup. recipient = models.ForeignKey(Recipient, null=True, on_delete=models.SET_NULL) + # TODO: The model still uses the old "zerver_huddle" database table. + # As a part of the migration of "Huddle" to "DirectMessageGroup" + # it needs to be renamed to "zerver_directmessagegroup". + class Meta: + db_table = "zerver_huddle" + def get_direct_message_group_hash(id_list: List[int]) -> str: id_list = sorted(set(id_list)) @@ -139,17 +148,18 @@ def get_direct_message_group_hash(id_list: List[int]) -> str: return hashlib.sha1(hash_key.encode()).hexdigest() -def get_or_create_direct_message_group(id_list: List[int]) -> Huddle: +def get_or_create_direct_message_group(id_list: List[int]) -> DirectMessageGroup: """ - Takes a list of user IDs and returns the Huddle object for the - group consisting of these users. If the Huddle object does not - yet exist, it will be transparently created. + Takes a list of user IDs and returns the DirectMessageGroup + object for the group consisting of these users. If the + DirectMessageGroup object does not yet exist, it will be + transparently created. """ from zerver.models import Subscription, UserProfile direct_message_group_hash = get_direct_message_group_hash(id_list) with transaction.atomic(): - (direct_message_group, created) = Huddle.objects.get_or_create( + (direct_message_group, created) = DirectMessageGroup.objects.get_or_create( huddle_hash=direct_message_group_hash ) if created: diff --git a/zerver/tests/test_import_export.py b/zerver/tests/test_import_export.py index 5720fb934d..39eff0dc9e 100644 --- a/zerver/tests/test_import_export.py +++ b/zerver/tests/test_import_export.py @@ -62,8 +62,8 @@ from zerver.models import ( BotStorageData, CustomProfileField, CustomProfileFieldValue, + DirectMessageGroup, GroupGroupMembership, - Huddle, Message, MutedUser, NamedUserGroup, @@ -584,12 +584,12 @@ class RealmImportExportTest(ExportFile): self.send_group_direct_message( self.example_user("iago"), [self.example_user("cordelia"), self.example_user("AARON")] ) - direct_message_group_a = Huddle.objects.last() + direct_message_group_a = DirectMessageGroup.objects.last() self.send_group_direct_message( self.example_user("ZOE"), [self.example_user("hamlet"), self.example_user("AARON"), self.example_user("othello")], ) - direct_message_group_b = Huddle.objects.last() + direct_message_group_b = DirectMessageGroup.objects.last() direct_message_group_c_message_id = self.send_group_direct_message( self.example_user("AARON"), @@ -1043,8 +1043,9 @@ class RealmImportExportTest(ExportFile): Recipient.objects.get(type=Recipient.STREAM, type_id=stream.id).id, ) - for dm_group in Huddle.objects.all(): - # Huddles don't have a realm column, so we just test all Huddles for simplicity. + for dm_group in DirectMessageGroup.objects.all(): + # Direct Message groups don't have a realm column, so we just test all + # Direct Message groups for simplicity. self.assertEqual( dm_group.recipient_id, Recipient.objects.get(type=Recipient.DIRECT_MESSAGE_GROUP, type_id=dm_group.id).id, @@ -1272,7 +1273,9 @@ class RealmImportExportTest(ExportFile): @getter def get_group_direct_message(r: Realm) -> str: direct_message_group_hash = get_direct_message_group_hashes(r) - direct_message_group_id = Huddle.objects.get(huddle_hash=direct_message_group_hash).id + direct_message_group_id = DirectMessageGroup.objects.get( + huddle_hash=direct_message_group_hash + ).id direct_message_group_recipient = Recipient.objects.get( type_id=direct_message_group_id, type=3 ) diff --git a/zerver/tests/test_typing.py b/zerver/tests/test_typing.py index 4be05abdef..4526653939 100644 --- a/zerver/tests/test_typing.py +++ b/zerver/tests/test_typing.py @@ -1,7 +1,7 @@ import orjson from zerver.lib.test_classes import ZulipTestCase -from zerver.models import Huddle +from zerver.models import DirectMessageGroup from zerver.models.recipients import get_direct_message_group_hash @@ -203,7 +203,9 @@ class TypingHappyPathTestDirectMessages(ZulipTestCase): expected_recipient_ids = {user.id for user in expected_recipients} direct_message_group_hash = get_direct_message_group_hash(list(expected_recipient_ids)) - self.assertFalse(Huddle.objects.filter(huddle_hash=direct_message_group_hash).exists()) + self.assertFalse( + DirectMessageGroup.objects.filter(huddle_hash=direct_message_group_hash).exists() + ) params = dict( to=orjson.dumps([user.id for user in recipient_users]).decode(), @@ -219,7 +221,9 @@ class TypingHappyPathTestDirectMessages(ZulipTestCase): # We should not be adding new Direct Message groups # just because a user started typing in the compose # box. Let's wait till they send an actual message. - self.assertFalse(Huddle.objects.filter(huddle_hash=direct_message_group_hash).exists()) + self.assertFalse( + DirectMessageGroup.objects.filter(huddle_hash=direct_message_group_hash).exists() + ) event = events[0]["event"] event_recipient_emails = {user["email"] for user in event["recipients"]} diff --git a/zilencer/management/commands/populate_db.py b/zilencer/management/commands/populate_db.py index 60b98e9efd..14df8a66f8 100644 --- a/zilencer/management/commands/populate_db.py +++ b/zilencer/management/commands/populate_db.py @@ -51,8 +51,8 @@ from zerver.models import ( Client, CustomProfileField, DefaultStream, + DirectMessageGroup, Draft, - Huddle, Message, Reaction, Realm, @@ -127,7 +127,7 @@ def clear_database() -> None: Recipient, Realm, Subscription, - Huddle, + DirectMessageGroup, UserMessage, Client, DefaultStream,