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.
This commit is contained in:
roanster007 2024-07-05 16:43:40 +05:30 committed by Tim Abbott
parent 79f858b4b8
commit 02d0566dc5
15 changed files with 94 additions and 58 deletions

View File

@ -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.lib.utils import assert_is_not_none
from zerver.models import ( from zerver.models import (
Client, Client,
Huddle, DirectMessageGroup,
Message, Message,
NamedUserGroup, NamedUserGroup,
PreregistrationUser, PreregistrationUser,
@ -174,12 +174,12 @@ class AnalyticsTestCase(ZulipTestCase):
stream.save(update_fields=["recipient"]) stream.save(update_fields=["recipient"])
return stream, 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 self.name_counter += 1
defaults = {"huddle_hash": f"hash{self.name_counter}"} defaults = {"huddle_hash": f"hash{self.name_counter}"}
for key, value in defaults.items(): for key, value in defaults.items():
kwargs[key] = kwargs.get(key, value) 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) recipient = Recipient.objects.create(type_id=huddle.id, type=Recipient.DIRECT_MESSAGE_GROUP)
huddle.recipient = recipient huddle.recipient = recipient
huddle.save(update_fields=["recipient"]) huddle.save(update_fields=["recipient"])

View File

@ -37,7 +37,7 @@ create_zulip_test
./manage.py dumpdata \ ./manage.py dumpdata \
zerver.UserProfile zerver.Stream zerver.Recipient \ 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.UserMessage zerver.Client \
zerver.DefaultStream >zerver/tests/fixtures/messages.json zerver.DefaultStream >zerver/tests/fixtures/messages.json

View File

@ -32,7 +32,7 @@ from zerver.lib.partial import partial
from zerver.lib.stream_color import STREAM_ASSIGNMENT_COLORS as STREAM_COLORS from zerver.lib.stream_color import STREAM_ASSIGNMENT_COLORS as STREAM_COLORS
from zerver.models import ( from zerver.models import (
Attachment, Attachment,
Huddle, DirectMessageGroup,
Message, Message,
Realm, Realm,
RealmEmoji, RealmEmoji,
@ -486,7 +486,7 @@ def build_stream(
def build_direct_message_group(direct_message_group_id: int) -> ZerverFieldsT: def build_direct_message_group(direct_message_group_id: int) -> ZerverFieldsT:
direct_message_group = Huddle( direct_message_group = DirectMessageGroup(
id=direct_message_group_id, id=direct_message_group_id,
) )
return model_to_dict(direct_message_group) return model_to_dict(direct_message_group)

View File

@ -41,8 +41,8 @@ from zerver.models import (
CustomProfileField, CustomProfileField,
CustomProfileFieldValue, CustomProfileFieldValue,
DefaultStream, DefaultStream,
DirectMessageGroup,
GroupGroupMembership, GroupGroupMembership,
Huddle,
Message, Message,
MutedUser, MutedUser,
NamedUserGroup, 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} 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() unsafe_huddle_recipient_ids = set()
for sub in Subscription.objects.select_related("user_profile").filter( for sub in Subscription.objects.select_related("user_profile").filter(
recipient__in=realm_huddle_recipient_ids 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_recipient"] = huddle_recipients
response["_huddle_subscription"] = huddle_subscription_dicts 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: def custom_fetch_scheduled_messages(response: TableData, context: Context) -> None:

View File

@ -50,8 +50,8 @@ from zerver.models import (
CustomProfileField, CustomProfileField,
CustomProfileFieldValue, CustomProfileFieldValue,
DefaultStream, DefaultStream,
DirectMessageGroup,
GroupGroupMembership, GroupGroupMembership,
Huddle,
Message, Message,
MutedUser, MutedUser,
NamedUserGroup, NamedUserGroup,
@ -999,7 +999,7 @@ def disable_restricted_authentication_methods(data: TableData) -> None:
# * Realm's announcements_streams and group_permissions # * Realm's announcements_streams and group_permissions
# * UserProfile, in order by ID to avoid bot loop issues # * UserProfile, in order by ID to avoid bot loop issues
# * Now can do all realm_tables # * Now can do all realm_tables
# * Huddle # * DirectMessageGroup
# * Recipient # * Recipient
# * Subscription # * Subscription
# * Message # * 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"]) realm_emoji.save(update_fields=["author_id"])
if "zerver_huddle" in data: if "zerver_huddle" in data:
update_model_ids(Huddle, data, "huddle") update_model_ids(DirectMessageGroup, data, "huddle")
# We don't import Huddle yet, since we don't have the data to # We don't import DirectMessageGroup yet, since we don't have
# compute direct message group hashes until we've imported some # the data to compute direct message group hashes until we've
# of the tables below. # imported some of the tables below.
# We can't get direct message group hashes without processing # We can't get direct message group hashes without processing
# subscriptions first, during which # subscriptions first, during which
# get_direct_message_groups_from_subscription is called. # 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: if "zerver_huddle" in data:
process_direct_message_group_hash(data, "zerver_huddle") process_direct_message_group_hash(data, "zerver_huddle")
bulk_import_model(data, Huddle) bulk_import_model(data, DirectMessageGroup)
for direct_message_group in Huddle.objects.filter(recipient=None): for direct_message_group in DirectMessageGroup.objects.filter(recipient=None):
recipient = Recipient.objects.get( recipient = Recipient.objects.get(
type=Recipient.DIRECT_MESSAGE_GROUP, type_id=direct_message_group.id type=Recipient.DIRECT_MESSAGE_GROUP, type_id=direct_message_group.id
) )

View File

@ -72,7 +72,7 @@ from zerver.lib.validator import (
check_string_or_int_list, check_string_or_int_list,
) )
from zerver.models import ( from zerver.models import (
Huddle, DirectMessageGroup,
Message, Message,
Realm, Realm,
Recipient, Recipient,
@ -597,7 +597,7 @@ class NarrowBuilder:
) )
except (JsonableError, ValidationError): except (JsonableError, ValidationError):
raise BadNarrowOperatorError("unknown user in " + str(operand)) raise BadNarrowOperatorError("unknown user in " + str(operand))
except Huddle.DoesNotExist: except DirectMessageGroup.DoesNotExist:
# Group DM where huddle doesn't exist # Group DM where huddle doesn't exist
return query.where(maybe_negate(false())) return query.where(maybe_negate(false()))

View File

@ -3,7 +3,7 @@ from typing import Dict, Optional, Sequence
from django.core.exceptions import ValidationError from django.core.exceptions import ValidationError
from django.utils.translation import gettext as _ 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 ( from zerver.models.recipients import (
get_direct_message_group_hash, get_direct_message_group_hash,
get_or_create_direct_message_group, get_or_create_direct_message_group,
@ -56,10 +56,10 @@ def get_recipient_from_user_profiles(
if create: if create:
direct_message_group = get_or_create_direct_message_group(user_ids) direct_message_group = get_or_create_direct_message_group(user_ids)
else: else:
# We intentionally let the Huddle.DoesNotExist escape, in the # We intentionally let the DirectMessageGroup.DoesNotExist escape,
# case that there is no such direct message group, and the user # in the case that there is no such direct message group, and the
# passed create=False # user passed create=False
direct_message_group = Huddle.objects.get( direct_message_group = DirectMessageGroup.objects.get(
huddle_hash=get_direct_message_group_hash(user_ids) huddle_hash=get_direct_message_group_hash(user_ids)
) )
return Recipient( return Recipient(

View File

@ -601,7 +601,7 @@ def use_db_models(
DefaultStream = apps.get_model("zerver", "DefaultStream") DefaultStream = apps.get_model("zerver", "DefaultStream")
DefaultStreamGroup = apps.get_model("zerver", "DefaultStreamGroup") DefaultStreamGroup = apps.get_model("zerver", "DefaultStreamGroup")
EmailChangeStatus = apps.get_model("zerver", "EmailChangeStatus") EmailChangeStatus = apps.get_model("zerver", "EmailChangeStatus")
Huddle = apps.get_model("zerver", "Huddle") DirectMessageGroup = apps.get_model("zerver", "DirectMessageGroup")
Message = apps.get_model("zerver", "Message") Message = apps.get_model("zerver", "Message")
MultiuseInvite = apps.get_model("zerver", "MultiuseInvite") MultiuseInvite = apps.get_model("zerver", "MultiuseInvite")
OnboardingStep = apps.get_model("zerver", "OnboardingStep") OnboardingStep = apps.get_model("zerver", "OnboardingStep")
@ -645,7 +645,7 @@ def use_db_models(
DefaultStream=DefaultStream, DefaultStream=DefaultStream,
DefaultStreamGroup=DefaultStreamGroup, DefaultStreamGroup=DefaultStreamGroup,
EmailChangeStatus=EmailChangeStatus, EmailChangeStatus=EmailChangeStatus,
Huddle=Huddle, DirectMessageGroup=DirectMessageGroup,
Message=Message, Message=Message,
MultiuseInvite=MultiuseInvite, MultiuseInvite=MultiuseInvite,
UserTopic=UserTopic, UserTopic=UserTopic,

View File

@ -59,9 +59,9 @@ realms used for testing; consider using deactivate_realm instead."""
# storage; failing to do this leaves dangling files # storage; failing to do this leaves dangling files
do_delete_all_realm_attachments(realm) do_delete_all_realm_attachments(realm)
# TODO: This approach leaks Recipient and Huddle objects, # TODO: This approach leaks Recipient and DirectMessageGroup
# because those don't have a foreign key to the Realm or any # objects, because those don't have a foreign key to the Realm
# other model it cascades to (Realm/Stream/UserProfile/etc.). # or any other model it cascades to (Realm/Stream/UserProfile/etc.).
realm.delete() realm.delete()
print("Realm has been successfully permanently deleted.") print("Realm has been successfully permanently deleted.")

View File

@ -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"),
],
)
]

View File

@ -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 Realm as Realm
from zerver.models.realms import RealmAuthenticationMethod as RealmAuthenticationMethod from zerver.models.realms import RealmAuthenticationMethod as RealmAuthenticationMethod
from zerver.models.realms import RealmDomain as RealmDomain 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.recipients import Recipient as Recipient
from zerver.models.scheduled_jobs import AbstractScheduledJob as AbstractScheduledJob from zerver.models.scheduled_jobs import AbstractScheduledJob as AbstractScheduledJob
from zerver.models.scheduled_jobs import MissedMessageEmailAddress as MissedMessageEmailAddress from zerver.models.scheduled_jobs import MissedMessageEmailAddress as MissedMessageEmailAddress

View File

@ -21,18 +21,20 @@ class Recipient(models.Model):
of audiences Zulip supports for a message. of audiences Zulip supports for a message.
Recipient has just two attributes: The enum type, and a type_id, Recipient has just two attributes: The enum type, and a type_id,
which is the ID of the UserProfile/Stream/Huddle object containing which is the ID of the UserProfile/Stream/DirectMessageGroup object
all the metadata for the audience. There are 3 recipient types: 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 1. 1:1 direct message: The type_id is the ID of the UserProfile
who will receive any message to this Recipient. The sender who will receive any message to this Recipient. The sender
of such a message is represented separately. of such a message is represented separately.
2. Stream message: The type_id is the ID of the associated Stream. 2. Stream message: The type_id is the ID of the associated Stream.
3. Group direct message: In Zulip, group direct messages are 3. Group direct message: In Zulip, group direct messages are
represented by Huddle objects, which encode the set of users represented by DirectMessageGroup objects, which encode the set of
in the conversation. The type_id is the ID of the associated Huddle users in the conversation. The type_id is the ID of the associated
object; the set of users is usually retrieved via the Subscription DirectMessageGroup object; the set of users is usually retrieved
table. See the Huddle model for details. via the Subscription table. See the DirectMessageGroup model for
details.
See also the Subscription model, which stores which UserProfile See also the Subscription model, which stores which UserProfile
objects are subscribed to which Recipient objects. 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 return result_dict
class Huddle(models.Model): class DirectMessageGroup(models.Model):
""" """
Represents a group of individuals who may have a Represents a group of individuals who may have a
group direct message conversation together. group direct message conversation together.
The membership of the Huddle is stored in the Subscription table just like with The membership of the DirectMessageGroup is stored in the Subscription
Streams - for each user in the Huddle, there is a Subscription object table just like with Streams - for each user in the DirectMessageGroup,
tied to the UserProfile and the Huddle's recipient object. 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 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 below, to support efficiently mapping from a set of users to the
corresponding Huddle object. corresponding DirectMessageGroup object.
""" """
# TODO: We should consider whether using # TODO: We should consider whether using
# CommaSeparatedIntegerField would be better. # CommaSeparatedIntegerField would be better.
huddle_hash = models.CharField(max_length=40, db_index=True, unique=True) 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) 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: def get_direct_message_group_hash(id_list: List[int]) -> str:
id_list = sorted(set(id_list)) 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() 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 Takes a list of user IDs and returns the DirectMessageGroup
group consisting of these users. If the Huddle object does not object for the group consisting of these users. If the
yet exist, it will be transparently created. DirectMessageGroup object does not yet exist, it will be
transparently created.
""" """
from zerver.models import Subscription, UserProfile from zerver.models import Subscription, UserProfile
direct_message_group_hash = get_direct_message_group_hash(id_list) direct_message_group_hash = get_direct_message_group_hash(id_list)
with transaction.atomic(): 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 huddle_hash=direct_message_group_hash
) )
if created: if created:

View File

@ -62,8 +62,8 @@ from zerver.models import (
BotStorageData, BotStorageData,
CustomProfileField, CustomProfileField,
CustomProfileFieldValue, CustomProfileFieldValue,
DirectMessageGroup,
GroupGroupMembership, GroupGroupMembership,
Huddle,
Message, Message,
MutedUser, MutedUser,
NamedUserGroup, NamedUserGroup,
@ -584,12 +584,12 @@ class RealmImportExportTest(ExportFile):
self.send_group_direct_message( self.send_group_direct_message(
self.example_user("iago"), [self.example_user("cordelia"), self.example_user("AARON")] 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.send_group_direct_message(
self.example_user("ZOE"), self.example_user("ZOE"),
[self.example_user("hamlet"), self.example_user("AARON"), self.example_user("othello")], [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( direct_message_group_c_message_id = self.send_group_direct_message(
self.example_user("AARON"), self.example_user("AARON"),
@ -1043,8 +1043,9 @@ class RealmImportExportTest(ExportFile):
Recipient.objects.get(type=Recipient.STREAM, type_id=stream.id).id, Recipient.objects.get(type=Recipient.STREAM, type_id=stream.id).id,
) )
for dm_group in Huddle.objects.all(): for dm_group in DirectMessageGroup.objects.all():
# Huddles don't have a realm column, so we just test all Huddles for simplicity. # Direct Message groups don't have a realm column, so we just test all
# Direct Message groups for simplicity.
self.assertEqual( self.assertEqual(
dm_group.recipient_id, dm_group.recipient_id,
Recipient.objects.get(type=Recipient.DIRECT_MESSAGE_GROUP, type_id=dm_group.id).id, Recipient.objects.get(type=Recipient.DIRECT_MESSAGE_GROUP, type_id=dm_group.id).id,
@ -1272,7 +1273,9 @@ class RealmImportExportTest(ExportFile):
@getter @getter
def get_group_direct_message(r: Realm) -> str: def get_group_direct_message(r: Realm) -> str:
direct_message_group_hash = get_direct_message_group_hashes(r) 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( direct_message_group_recipient = Recipient.objects.get(
type_id=direct_message_group_id, type=3 type_id=direct_message_group_id, type=3
) )

View File

@ -1,7 +1,7 @@
import orjson import orjson
from zerver.lib.test_classes import ZulipTestCase 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 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} expected_recipient_ids = {user.id for user in expected_recipients}
direct_message_group_hash = get_direct_message_group_hash(list(expected_recipient_ids)) 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( params = dict(
to=orjson.dumps([user.id for user in recipient_users]).decode(), 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 # We should not be adding new Direct Message groups
# just because a user started typing in the compose # just because a user started typing in the compose
# box. Let's wait till they send an actual message. # 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 = events[0]["event"]
event_recipient_emails = {user["email"] for user in event["recipients"]} event_recipient_emails = {user["email"] for user in event["recipients"]}

View File

@ -51,8 +51,8 @@ from zerver.models import (
Client, Client,
CustomProfileField, CustomProfileField,
DefaultStream, DefaultStream,
DirectMessageGroup,
Draft, Draft,
Huddle,
Message, Message,
Reaction, Reaction,
Realm, Realm,
@ -127,7 +127,7 @@ def clear_database() -> None:
Recipient, Recipient,
Realm, Realm,
Subscription, Subscription,
Huddle, DirectMessageGroup,
UserMessage, UserMessage,
Client, Client,
DefaultStream, DefaultStream,