mirror of
https://github.com/zulip/zulip.git
synced 2026-06-27 21:01:32 +08:00
personal_recipient: Handle cases with none values.
When migrating personal messages to use direct message groups, we need to properly handle cases where the personal recipient is None. This ensuressmooth transition as we move towards using direct message groups for all 1:1 conversations. The changes handle: - Cases where recipient.recipient is None during message sending - Migration path from personal recipients to direct message groups - Tests to verify proper handling of None recipients
This commit is contained in:
parent
a072058ec5
commit
fffbbdf4c7
@ -99,8 +99,8 @@ def do_delete_user(user_profile: UserProfile, *, acting_user: UserProfile | None
|
||||
# Recipient objects don't get deleted through CASCADE, so we need to handle
|
||||
# the user's personal recipient manually. This will also delete all Messages pointing
|
||||
# to this recipient (all direct messages sent to the user).
|
||||
assert personal_recipient is not None
|
||||
personal_recipient.delete()
|
||||
if personal_recipient is not None:
|
||||
personal_recipient.delete()
|
||||
replacement_user = create_user(
|
||||
force_id=user_id,
|
||||
email=Address(
|
||||
|
||||
@ -20,7 +20,6 @@ from contextvars import ContextVar
|
||||
from dataclasses import dataclass
|
||||
from datetime import datetime
|
||||
from email.headerregistry import Address
|
||||
from functools import cache
|
||||
from itertools import chain, islice
|
||||
from typing import TYPE_CHECKING, Any, Optional, TypeAlias, TypedDict, TypeVar, cast
|
||||
from urllib.parse import urlsplit
|
||||
@ -39,6 +38,7 @@ import zerver.lib.upload
|
||||
from analytics.models import RealmCount, StreamCount, UserCount
|
||||
from version import ZULIP_VERSION
|
||||
from zerver.lib.avatar_hash import user_avatar_base_path_from_ids
|
||||
from zerver.lib.display_recipient import get_display_recipient
|
||||
from zerver.lib.migration_status import MigrationStatusJson, parse_migration_status
|
||||
from zerver.lib.parallel import run_parallel, run_parallel_queue
|
||||
from zerver.lib.pysa import mark_sanitized
|
||||
@ -93,7 +93,7 @@ from zerver.models.presence import PresenceSequence
|
||||
from zerver.models.realm_audit_logs import AuditLogEventType
|
||||
from zerver.models.realms import get_fake_email_domain, get_realm
|
||||
from zerver.models.saved_snippets import SavedSnippet
|
||||
from zerver.models.users import ExternalAuthID, get_system_bot
|
||||
from zerver.models.users import ExternalAuthID, get_system_bot, is_cross_realm_bot_email
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from mypy_boto3_s3.service_resource import Bucket, Object
|
||||
@ -1452,7 +1452,11 @@ def custom_fetch_user_profile_cross_realm(response: TableData, context: Context)
|
||||
bot_default_email = bot_name_to_default_email[bot_name]
|
||||
bot_user_id = get_system_bot(bot_email, internal_realm.id).id
|
||||
|
||||
recipient_id = Recipient.objects.get(type_id=bot_user_id, type=Recipient.PERSONAL).id
|
||||
try:
|
||||
recipient_id = Recipient.objects.get(type_id=bot_user_id, type=Recipient.PERSONAL).id
|
||||
except Recipient.DoesNotExist:
|
||||
recipient_id = None
|
||||
|
||||
crossrealm_bots.append(
|
||||
dict(
|
||||
email=bot_default_email,
|
||||
@ -1537,6 +1541,7 @@ def custom_fetch_direct_message_groups(response: TableData, context: Context) ->
|
||||
r["id"]
|
||||
for r in list(response["zerver_userprofile"])
|
||||
+ list(response["zerver_userprofile_mirrordummy"])
|
||||
+ list(response["zerver_userprofile_crossrealm"])
|
||||
}
|
||||
|
||||
recipient_filter = Q()
|
||||
@ -1573,17 +1578,21 @@ def custom_fetch_direct_message_groups(response: TableData, context: Context) ->
|
||||
for sub in Subscription.objects.select_related("user_profile").filter(
|
||||
recipient__in=realm_direct_message_group_recipient_ids
|
||||
):
|
||||
if sub.user_profile.realm_id != realm.id:
|
||||
if sub.user_profile.realm_id != realm.id and not is_cross_realm_bot_email(
|
||||
sub.user_profile.delivery_email
|
||||
):
|
||||
# In almost every case the other realm will be zulip.com
|
||||
unsafe_direct_message_group_recipient_ids.add(sub.recipient_id)
|
||||
|
||||
# Now filter down to just those direct message groups that are
|
||||
# entirely within the realm.
|
||||
#
|
||||
# This is important for ensuring that the User objects needed
|
||||
# to import it on the other end exist (since we're only
|
||||
# exporting the users from this realm), at the cost of losing
|
||||
# some of these cross-realm messages.
|
||||
# This is important for ensuring that the User objects needed to
|
||||
# import it on the other end exist (since we're only exporting the
|
||||
# users from this realm), at the cost of losing any true
|
||||
# cross-realm messages. (As of 2025, true cross-realm messages,
|
||||
# not involving system bots cannot exist without a bug or fork of
|
||||
# Zulip).
|
||||
direct_message_group_subs = [
|
||||
sub
|
||||
for sub in realm_direct_message_group_subs
|
||||
@ -2821,22 +2830,18 @@ def batched(iterable: Iterable[T], n: int) -> Iterable[tuple[T, ...]]:
|
||||
def export_messages_single_user(
|
||||
user_profile: UserProfile, *, output_dir: Path, reaction_message_ids: set[int]
|
||||
) -> None:
|
||||
@cache
|
||||
def get_recipient(recipient_id: int) -> str:
|
||||
recipient = Recipient.objects.get(id=recipient_id)
|
||||
|
||||
def get_recipient(recipient: Recipient, sender: UserProfile) -> str:
|
||||
if recipient.type == Recipient.STREAM:
|
||||
stream = Stream.objects.values("name").get(id=recipient.type_id)
|
||||
return stream["name"]
|
||||
|
||||
user_names = (
|
||||
UserProfile.objects.filter(
|
||||
subscription__recipient_id=recipient.id,
|
||||
)
|
||||
.order_by("full_name")
|
||||
.values_list("full_name", flat=True)
|
||||
)
|
||||
display_recipients = get_display_recipient(recipient)
|
||||
|
||||
if len(display_recipients) == 2:
|
||||
other_user = next(user for user in display_recipients if user["id"] != sender.id)
|
||||
return other_user["full_name"]
|
||||
|
||||
user_names = [user["full_name"] for user in display_recipients]
|
||||
return ", ".join(user_names)
|
||||
|
||||
messages_from_me = Message.objects.filter(
|
||||
@ -2878,7 +2883,9 @@ def export_messages_single_user(
|
||||
item["flags_mask"] = user_message.flags.mask
|
||||
# Add a few nice, human-readable details
|
||||
item["sending_client_name"] = user_message.message.sending_client.name
|
||||
item["recipient_name"] = get_recipient(user_message.message.recipient_id)
|
||||
item["recipient_name"] = get_recipient(
|
||||
user_message.message.recipient, user_message.message.sender
|
||||
)
|
||||
return floatify_datetime_fields(item, "zerver_message")
|
||||
|
||||
message_filename = os.path.join(output_dir, f"messages-{dump_file_id:06}.json")
|
||||
|
||||
@ -1244,8 +1244,16 @@ def do_import_realm(import_dir: Path, subdomain: str, processes: int = 1) -> Rea
|
||||
new_user_id = get_system_bot(item["email"], internal_realm.id).id
|
||||
update_id_map(table="user_profile", old_id=item["id"], new_id=new_user_id)
|
||||
crossrealm_user_ids.add(new_user_id)
|
||||
new_recipient_id = Recipient.objects.get(type=Recipient.PERSONAL, type_id=new_user_id).id
|
||||
update_id_map(table="recipient", old_id=item["recipient_id"], new_id=new_recipient_id)
|
||||
try:
|
||||
new_recipient_id = Recipient.objects.get(
|
||||
type=Recipient.PERSONAL, type_id=new_user_id
|
||||
).id
|
||||
update_id_map(table="recipient", old_id=item["recipient_id"], new_id=new_recipient_id)
|
||||
except Recipient.DoesNotExist:
|
||||
# This can happen if the pre-import server used DirectMessageGroup
|
||||
# for cross-realm bots exactly when the post-import server does.
|
||||
# The personal recipients shouldn't exist in both cases.
|
||||
assert item["recipient_id"] is None
|
||||
|
||||
# We first do a pass of updating model IDs for the cluster of
|
||||
# major models that have foreign keys into each other.
|
||||
|
||||
@ -1399,7 +1399,9 @@ def get_recent_conversations_recipient_id(
|
||||
return recipient_id
|
||||
|
||||
|
||||
def get_recent_private_conversations(user_profile: UserProfile) -> dict[int, dict[str, Any]]:
|
||||
def _get_recent_conversations_via_legacy_personal_recipient(
|
||||
user_profile_id: int, recipient_id: int
|
||||
) -> list[tuple[int, int]]:
|
||||
"""This function uses some carefully optimized SQL queries, designed
|
||||
to use the UserMessage index on private_messages. It is
|
||||
somewhat complicated by the fact that for 1:1 direct
|
||||
@ -1412,17 +1414,9 @@ def get_recent_private_conversations(user_profile: UserProfile) -> dict[int, dic
|
||||
It may be possible to write this query directly in Django, however
|
||||
it is made much easier by using CTEs, which Django does not
|
||||
natively support.
|
||||
|
||||
We return a dictionary structure for convenient modification
|
||||
below; this structure is converted into its final form by
|
||||
post_process.
|
||||
|
||||
"""
|
||||
RECENT_CONVERSATIONS_LIMIT = 1000
|
||||
|
||||
recipient_map = {}
|
||||
my_recipient_id = user_profile.recipient_id
|
||||
|
||||
query = SQL(
|
||||
"""
|
||||
WITH personals AS (
|
||||
@ -1435,12 +1429,12 @@ def get_recent_private_conversations(user_profile: UserProfile) -> dict[int, dic
|
||||
message AS (
|
||||
SELECT message_id,
|
||||
CASE
|
||||
WHEN m.recipient_id = %(my_recipient_id)s
|
||||
WHEN m.recipient_id = %(recipient_id)s
|
||||
THEN m.sender_id
|
||||
ELSE NULL
|
||||
END AS sender_id,
|
||||
CASE
|
||||
WHEN m.recipient_id <> %(my_recipient_id)s
|
||||
WHEN m.recipient_id <> %(recipient_id)s
|
||||
THEN m.recipient_id
|
||||
ELSE NULL
|
||||
END AS outgoing_recipient_id
|
||||
@ -1466,29 +1460,64 @@ def get_recent_private_conversations(user_profile: UserProfile) -> dict[int, dic
|
||||
cursor.execute(
|
||||
query,
|
||||
{
|
||||
"user_profile_id": user_profile.id,
|
||||
"user_profile_id": user_profile_id,
|
||||
"conversation_limit": RECENT_CONVERSATIONS_LIMIT,
|
||||
"my_recipient_id": my_recipient_id,
|
||||
"recipient_id": recipient_id,
|
||||
},
|
||||
)
|
||||
rows = cursor.fetchall()
|
||||
return cursor.fetchall()
|
||||
|
||||
# The resulting rows will be (recipient_id, max_message_id)
|
||||
# objects for all parties we've had recent (group?) private
|
||||
# message conversations with, including direct messages with
|
||||
# yourself (those will generate an empty list of user_ids).
|
||||
for recipient_id, max_message_id in rows:
|
||||
recipient_map[recipient_id] = dict(
|
||||
max_message_id=max_message_id,
|
||||
user_ids=[],
|
||||
|
||||
def _get_recent_conversations_via_direct_message_group(
|
||||
user_profile_id: int,
|
||||
) -> list[tuple[int, int]]:
|
||||
"""
|
||||
This functions fetches the most recent conversations given that all
|
||||
private messages of this user are through direct message groups.
|
||||
"""
|
||||
RECENT_CONVERSATIONS_LIMIT = 1000
|
||||
|
||||
recent_pm_message_ids = (
|
||||
UserMessage.objects.filter(user_profile_id=user_profile_id)
|
||||
.extra(where=[UserMessage.where_flag_is_present(UserMessage.flags.is_private)]) # noqa: S610
|
||||
.order_by("-message_id")
|
||||
.values_list("message_id", flat=True)[:RECENT_CONVERSATIONS_LIMIT]
|
||||
)
|
||||
|
||||
return list(
|
||||
Message.objects.filter(id__in=recent_pm_message_ids)
|
||||
.values("recipient_id")
|
||||
.annotate(max_message_id=Max("id"))
|
||||
.values_list("recipient_id", "max_message_id")
|
||||
)
|
||||
|
||||
|
||||
def get_recent_private_conversations(user_profile: UserProfile) -> dict[int, dict[str, Any]]:
|
||||
"""
|
||||
We return a dictionary structure for convenient modification
|
||||
below; this structure is converted into its final form by
|
||||
post_process.
|
||||
"""
|
||||
# Step 1: Collect recent message info
|
||||
if user_profile.recipient_id is not None:
|
||||
recent_conversations = _get_recent_conversations_via_legacy_personal_recipient(
|
||||
user_profile.id, user_profile.recipient_id
|
||||
)
|
||||
else:
|
||||
recent_conversations = _get_recent_conversations_via_direct_message_group(user_profile.id)
|
||||
|
||||
recipient_map: dict[int, dict[str, Any]] = {
|
||||
recipient_id: {"max_message_id": max_message_id, "user_ids": []}
|
||||
for recipient_id, max_message_id in recent_conversations
|
||||
}
|
||||
|
||||
# Now we need to map all the recipient_id objects to lists of user IDs
|
||||
for recipient_id, user_profile_id in (
|
||||
subscriptions = (
|
||||
Subscription.objects.filter(recipient_id__in=recipient_map.keys())
|
||||
.exclude(user_profile_id=user_profile.id)
|
||||
.values_list("recipient_id", "user_profile_id")
|
||||
):
|
||||
)
|
||||
for recipient_id, user_profile_id in subscriptions:
|
||||
recipient_map[recipient_id]["user_ids"].append(user_profile_id)
|
||||
|
||||
# Sort to prevent test flakes and client bugs.
|
||||
|
||||
@ -654,13 +654,17 @@ class NarrowBuilder:
|
||||
narrow_user_profile
|
||||
)
|
||||
|
||||
private_flag = column("flags", Integer).op("&")(UserMessage.flags.is_private.mask) != 0
|
||||
realm_match = column("realm_id", Integer) == self.realm.id
|
||||
direct_message_group_cond = column("recipient_id", Integer).in_(
|
||||
direct_message_group_recipient_ids
|
||||
)
|
||||
|
||||
self_recipient_id = self.user_profile.recipient_id
|
||||
# See note above in `by_dm` about needing bidirectional messages
|
||||
# for direct messages with another person.
|
||||
cond = and_(
|
||||
column("flags", Integer).op("&")(UserMessage.flags.is_private.mask) != 0,
|
||||
column("realm_id", Integer) == self.realm.id,
|
||||
or_(
|
||||
if self_recipient_id is not None and narrow_user_profile.recipient_id is not None:
|
||||
# See note above in `by_dm` about needing bidirectional messages
|
||||
# for direct messages with another person.
|
||||
dm_conditions = or_(
|
||||
and_(
|
||||
column("sender_id", Integer) == narrow_user_profile.id,
|
||||
column("recipient_id", Integer) == self_recipient_id,
|
||||
@ -669,11 +673,13 @@ class NarrowBuilder:
|
||||
column("sender_id", Integer) == self.user_profile.id,
|
||||
column("recipient_id", Integer) == narrow_user_profile.recipient_id,
|
||||
),
|
||||
and_(
|
||||
column("recipient_id", Integer).in_(direct_message_group_recipient_ids),
|
||||
),
|
||||
),
|
||||
)
|
||||
direct_message_group_cond,
|
||||
)
|
||||
cond = and_(private_flag, realm_match, dm_conditions)
|
||||
else:
|
||||
# If the user does not have a recipient_id, then they only rely on
|
||||
# direct group message for their conversations.
|
||||
cond = and_(private_flag, realm_match, direct_message_group_cond)
|
||||
return query.where(maybe_negate(cond))
|
||||
|
||||
def by_group_pm_with(
|
||||
|
||||
@ -58,19 +58,23 @@ def get_recipient_from_user_profiles(
|
||||
type_id=direct_message_group.id,
|
||||
)
|
||||
|
||||
# If no direct message group recipient exists, or if the setting
|
||||
# is disabled, we fall back to using personal recipients.
|
||||
del recipient_profiles_map[sender.id]
|
||||
if len(recipient_profiles_map) == 1:
|
||||
[recipient_user_profile] = recipient_profiles_map.values()
|
||||
else:
|
||||
recipient_user_profile = sender
|
||||
return Recipient(
|
||||
id=recipient_user_profile.recipient_id,
|
||||
type=Recipient.PERSONAL,
|
||||
type_id=recipient_user_profile.id,
|
||||
# Making sure we have personal recipients for all users,
|
||||
# otherwise, fall back to the direct message group.
|
||||
has_personal_recipient = all(
|
||||
user_profile.recipient_id is not None
|
||||
for user_profile in recipient_profiles_map.values()
|
||||
)
|
||||
|
||||
if has_personal_recipient:
|
||||
del recipient_profiles_map[sender.id]
|
||||
if len(recipient_profiles_map) == 1:
|
||||
[recipient_user_profile] = recipient_profiles_map.values()
|
||||
else:
|
||||
recipient_user_profile = sender
|
||||
return Recipient(
|
||||
id=recipient_user_profile.recipient_id,
|
||||
type=Recipient.PERSONAL,
|
||||
type_id=recipient_user_profile.id,
|
||||
)
|
||||
if create:
|
||||
direct_message_group = get_or_create_direct_message_group(user_ids)
|
||||
else:
|
||||
|
||||
@ -757,8 +757,10 @@ def check_can_access_user(
|
||||
).exists():
|
||||
return True
|
||||
|
||||
assert user_profile.recipient_id is not None
|
||||
assert target_user.recipient_id is not None
|
||||
if user_profile.recipient_id is None or target_user.recipient_id is None:
|
||||
# If either user does not have a recipient_id, they rely on
|
||||
# direct message groups for 1:1 or self DMs.
|
||||
return False
|
||||
|
||||
# Querying the "Message" table is expensive so we do this last.
|
||||
direct_message_query = Message.objects.filter(
|
||||
@ -810,6 +812,11 @@ def get_inaccessible_user_ids(
|
||||
if not possible_inaccessible_user_ids:
|
||||
return set()
|
||||
|
||||
if not acting_user.recipient_id:
|
||||
# If the acting user does not have a recipient_id, they only rely on
|
||||
# direct message groups for 1:1 or self DMs.
|
||||
return possible_inaccessible_user_ids
|
||||
|
||||
target_user_recipient_ids = UserProfile.objects.filter(
|
||||
id__in=possible_inaccessible_user_ids
|
||||
).values_list("recipient_id", flat=True)
|
||||
@ -948,7 +955,9 @@ def get_users_involved_in_dms_with_target_users(
|
||||
|
||||
direct_message_participants_dict[sender_id] = recipient_user_ids
|
||||
|
||||
personal_recipient_ids_for_target_users = [user.recipient_id for user in target_users]
|
||||
personal_recipient_ids_for_target_users = [
|
||||
user.recipient_id for user in target_users if user.recipient_id is not None
|
||||
]
|
||||
direct_message_senders_query = Message.objects.filter(
|
||||
realm=realm,
|
||||
recipient_id__in=personal_recipient_ids_for_target_users,
|
||||
|
||||
@ -14,6 +14,7 @@ from django.core.exceptions import ValidationError
|
||||
from django.core.management.base import CommandError
|
||||
from django.db.models import Q, QuerySet
|
||||
from django.forms.models import model_to_dict
|
||||
from django.test import override_settings
|
||||
from django.utils.timezone import now as timezone_now
|
||||
from typing_extensions import override
|
||||
|
||||
@ -112,7 +113,10 @@ from zerver.models.messages import ImageAttachment
|
||||
from zerver.models.presence import PresenceSequence
|
||||
from zerver.models.realm_audit_logs import AuditLogEventType
|
||||
from zerver.models.realms import get_realm
|
||||
from zerver.models.recipients import get_direct_message_group_hash
|
||||
from zerver.models.recipients import (
|
||||
get_direct_message_group_hash,
|
||||
get_or_create_direct_message_group,
|
||||
)
|
||||
from zerver.models.streams import get_active_streams, get_stream
|
||||
from zerver.models.users import ExternalAuthID, get_system_bot, get_user_by_delivery_email
|
||||
|
||||
@ -1015,6 +1019,12 @@ class RealmImportExportTest(ExportFile):
|
||||
|
||||
self.assertEqual(get_consented_user_ids(realm), consented_user_ids)
|
||||
|
||||
# Remove the recipient of the welcome bot to test exporting bots without personal recipients.
|
||||
internal_realm = get_realm(settings.SYSTEM_BOT_REALM)
|
||||
welcome_bot = get_system_bot(settings.WELCOME_BOT, internal_realm.id)
|
||||
welcome_bot.recipient = None
|
||||
welcome_bot.save(update_fields=["recipient"])
|
||||
|
||||
self.export_realm_and_create_auditlog(
|
||||
realm,
|
||||
export_type=RealmExport.EXPORT_FULL_WITH_CONSENT,
|
||||
@ -2992,6 +3002,55 @@ class SingleUserExportTest(ExportFile):
|
||||
],
|
||||
)
|
||||
|
||||
@override_settings(PREFER_DIRECT_MESSAGE_GROUP=True)
|
||||
def test_1_to_1_message_data_using_direct_message_group(self) -> None:
|
||||
hamlet = self.example_user("hamlet")
|
||||
cordelia = self.example_user("cordelia")
|
||||
othello = self.example_user("othello")
|
||||
bot = self.create_test_bot("test-bot", hamlet)
|
||||
|
||||
get_or_create_direct_message_group([hamlet.id, cordelia.id])
|
||||
get_or_create_direct_message_group([hamlet.id, bot.id])
|
||||
get_or_create_direct_message_group([hamlet.id])
|
||||
|
||||
hi_hamlet_message_id = self.send_personal_message(othello, hamlet, "hi hamlet")
|
||||
hi_cordelia_message_id = self.send_personal_message(hamlet, cordelia, "hi cordelia")
|
||||
bye_hamlet_message_id = self.send_personal_message(cordelia, hamlet, "bye hamlet")
|
||||
test_bot_message_id = self.send_personal_message(hamlet, bot, "test bot message")
|
||||
self.send_personal_message(othello, cordelia, "an irrelevant message")
|
||||
bye_peeps_message_id = self.send_group_direct_message(
|
||||
othello, [cordelia, hamlet], "bye peeps"
|
||||
)
|
||||
self_message_id = self.send_personal_message(hamlet, hamlet, "hi myself")
|
||||
|
||||
output_dir = make_export_output_dir()
|
||||
hamlet = self.example_user("hamlet")
|
||||
|
||||
with self.assertLogs(level="INFO"):
|
||||
do_export_user(hamlet, output_dir)
|
||||
|
||||
messages = read_json("messages-000001.json")
|
||||
|
||||
excerpt = [
|
||||
(rec["id"], rec["content"], rec["recipient_name"])
|
||||
for rec in messages["zerver_message"][-6:]
|
||||
]
|
||||
self.assertEqual(
|
||||
excerpt,
|
||||
[
|
||||
(hi_hamlet_message_id, "hi hamlet", hamlet.full_name),
|
||||
(hi_cordelia_message_id, "hi cordelia", cordelia.full_name),
|
||||
(bye_hamlet_message_id, "bye hamlet", hamlet.full_name),
|
||||
(test_bot_message_id, "test bot message", bot.full_name),
|
||||
(
|
||||
bye_peeps_message_id,
|
||||
"bye peeps",
|
||||
f"{cordelia.full_name}, {hamlet.full_name}, {othello.full_name}",
|
||||
),
|
||||
(self_message_id, "hi myself", hamlet.full_name),
|
||||
],
|
||||
)
|
||||
|
||||
def test_user_data(self) -> None:
|
||||
# We register checkers during test setup, and then we call them at the end.
|
||||
checkers = {}
|
||||
|
||||
@ -562,6 +562,18 @@ class NarrowBuilderTest(ZulipTestCase):
|
||||
"WHERE NOT ((flags & %(flags_1)s) != %(param_1)s AND realm_id = %(realm_id_1)s AND (sender_id = %(sender_id_1)s AND recipient_id = %(recipient_id_1)s OR sender_id = %(sender_id_2)s AND recipient_id = %(recipient_id_2)s OR recipient_id IN (__[POSTCOMPILE_recipient_id_3])))",
|
||||
)
|
||||
|
||||
def test_add_term_using_dm_including_operator_without_personal_recipient(self) -> None:
|
||||
# Dropping the personal recipient for Othello
|
||||
othello = self.example_user("othello")
|
||||
othello.recipient = None
|
||||
othello.save()
|
||||
|
||||
term = NarrowParameter(operator="dm-including", operand=self.othello_email)
|
||||
self._do_add_term_test(
|
||||
term,
|
||||
"WHERE (flags & %(flags_1)s) != %(param_1)s AND realm_id = %(realm_id_1)s AND recipient_id IN (__[POSTCOMPILE_recipient_id_1])",
|
||||
)
|
||||
|
||||
def test_add_term_using_id_operator_integer(self) -> None:
|
||||
term = NarrowParameter(operator="id", operand=555)
|
||||
self._do_add_term_test(term, "WHERE id = %(param_1)s")
|
||||
|
||||
@ -2449,6 +2449,28 @@ class StreamMessagesTest(ZulipTestCase):
|
||||
)
|
||||
self.assertEqual(recent_conversation["max_message_id"], message2_id)
|
||||
|
||||
def test_get_raw_unread_data_for_1_to_1_dms_using_group_direct_message(self) -> None:
|
||||
sender = self.example_user("hamlet")
|
||||
receiver = self.example_user("cordelia")
|
||||
receiver.recipient = None
|
||||
receiver.save()
|
||||
|
||||
message1_id = self.send_personal_message(sender, receiver, "test content 1")
|
||||
message2_id = self.send_personal_message(sender, receiver, "test content 2")
|
||||
|
||||
msg_data = get_raw_unread_data(receiver)
|
||||
|
||||
self.assert_length(msg_data["pm_dict"].keys(), 2)
|
||||
self.assert_length(msg_data["huddle_dict"].keys(), 0)
|
||||
|
||||
self.assertIn(message1_id, msg_data["pm_dict"].keys())
|
||||
self.assertIn(message2_id, msg_data["pm_dict"].keys())
|
||||
|
||||
recent_conversations = get_recent_private_conversations(receiver)
|
||||
[recent_conversation] = recent_conversations.values()
|
||||
self.assertEqual(set(recent_conversation["user_ids"]), {sender.id})
|
||||
self.assertEqual(recent_conversation["max_message_id"], message2_id)
|
||||
|
||||
def test_stream_becomes_active_on_message_send(self) -> None:
|
||||
# Mark a stream as inactive
|
||||
stream = self.make_stream("inactive_stream")
|
||||
@ -2544,10 +2566,11 @@ class PersonalMessageSendTest(ZulipTestCase):
|
||||
self.assertEqual(message_stream_count(receiver), receiver_messages + 1)
|
||||
|
||||
direct_message_group = get_direct_message_group([sender.id, receiver.id])
|
||||
if direct_message_group and settings.PREFER_DIRECT_MESSAGE_GROUP:
|
||||
recipient = Recipient.objects.get(
|
||||
type_id=direct_message_group.id, type=Recipient.DIRECT_MESSAGE_GROUP
|
||||
)
|
||||
has_none_recipient = receiver.recipient is None or sender.recipient is None
|
||||
if has_none_recipient:
|
||||
recipient = get_or_create_direct_message_group([sender.id, receiver.id]).recipient
|
||||
elif settings.PREFER_DIRECT_MESSAGE_GROUP and direct_message_group:
|
||||
recipient = direct_message_group.recipient
|
||||
else:
|
||||
recipient = Recipient.objects.get(type_id=receiver.id, type=Recipient.PERSONAL)
|
||||
|
||||
@ -2582,6 +2605,26 @@ class PersonalMessageSendTest(ZulipTestCase):
|
||||
receiver=receiver,
|
||||
)
|
||||
|
||||
def test_personal_when_personal_recipient_is_none(self) -> None:
|
||||
"""
|
||||
If you send a personal using direct_message_group, only you and the recipient see it.
|
||||
"""
|
||||
sender = self.example_user("hamlet")
|
||||
receiver = self.example_user("othello")
|
||||
|
||||
# Removing the personal recipient to ensure a new direct message group is created.
|
||||
receiver.recipient = None
|
||||
receiver.save()
|
||||
|
||||
self.login("hamlet")
|
||||
self.assert_personal(
|
||||
sender=sender,
|
||||
receiver=receiver,
|
||||
)
|
||||
|
||||
message = most_recent_message(sender)
|
||||
self.assertEqual(message.recipient.type, Recipient.DIRECT_MESSAGE_GROUP)
|
||||
|
||||
def test_direct_message_initiator_group_setting(self) -> None:
|
||||
"""
|
||||
Tests that direct_message_initiator_group_setting works correctly.
|
||||
|
||||
@ -18,7 +18,10 @@ from zerver.actions.create_user import do_create_user, do_reactivate_user
|
||||
from zerver.actions.invites import do_create_multiuse_invite_link, do_invite_users
|
||||
from zerver.actions.message_send import RecipientInfoResult, get_recipient_info
|
||||
from zerver.actions.muted_users import do_mute_user
|
||||
from zerver.actions.realm_settings import do_set_realm_property
|
||||
from zerver.actions.realm_settings import (
|
||||
do_change_realm_permission_group_setting,
|
||||
do_set_realm_property,
|
||||
)
|
||||
from zerver.actions.user_settings import bulk_regenerate_api_keys, do_change_user_setting
|
||||
from zerver.actions.user_topics import do_set_user_topic_visibility_policy
|
||||
from zerver.actions.users import (
|
||||
@ -76,7 +79,7 @@ from zerver.models import (
|
||||
)
|
||||
from zerver.models.clients import get_client
|
||||
from zerver.models.custom_profile_fields import check_valid_user_ids
|
||||
from zerver.models.groups import SystemGroups
|
||||
from zerver.models.groups import NamedUserGroup, SystemGroups
|
||||
from zerver.models.prereg_users import filter_to_valid_prereg_users
|
||||
from zerver.models.realm_audit_logs import AuditLogEventType
|
||||
from zerver.models.realms import InvalidFakeEmailDomainError, get_fake_email_domain, get_realm
|
||||
@ -590,6 +593,29 @@ class PermissionTest(ZulipTestCase):
|
||||
self.example_user("cordelia"), self.example_user("aaron").id, for_admin=False
|
||||
)
|
||||
|
||||
def test_access_user_by_id_when_personal_recipient_is_none(self) -> None:
|
||||
self.set_up_db_for_testing_user_access()
|
||||
polonius = self.example_user("polonius")
|
||||
|
||||
# Removing the personal recipient to ensure a new direct message group is used for 1:1 dms.
|
||||
polonius.recipient = None
|
||||
polonius.save()
|
||||
|
||||
# Restricting the "Members" system group to not allow access to all users.
|
||||
realm = get_realm("zulip")
|
||||
members_system_group = NamedUserGroup.objects.get(name=SystemGroups.MEMBERS, realm=realm)
|
||||
do_change_realm_permission_group_setting(
|
||||
realm, "can_access_all_users_group", members_system_group, acting_user=None
|
||||
)
|
||||
|
||||
aaron = self.example_user("aaron")
|
||||
target_user = access_user_by_id(polonius, aaron.id, for_admin=False)
|
||||
self.assertEqual(target_user, aaron)
|
||||
|
||||
othello = self.example_user("othello")
|
||||
with self.assertRaises(JsonableError):
|
||||
access_user_by_id(polonius, othello.id, for_admin=False)
|
||||
|
||||
def check_property_for_role(self, user_profile: UserProfile, role: int) -> bool:
|
||||
if role == UserProfile.ROLE_REALM_ADMINISTRATOR:
|
||||
return (
|
||||
@ -3206,6 +3232,26 @@ class GetProfileTest(ZulipTestCase):
|
||||
)
|
||||
self.assertEqual(inaccessible_user_ids, {othello.id})
|
||||
|
||||
def test_get_inaccessible_user_ids_when_personal_recipient_is_none(self) -> None:
|
||||
polonius = self.example_user("polonius")
|
||||
|
||||
# Removing the personal recipient to ensure we use direct message group.
|
||||
polonius.recipient = None
|
||||
polonius.save()
|
||||
|
||||
bot = self.example_user("default_bot")
|
||||
hamlet = self.example_user("hamlet")
|
||||
othello = self.example_user("othello")
|
||||
|
||||
inaccessible_user_ids = get_inaccessible_user_ids([bot.id, hamlet.id, othello.id], polonius)
|
||||
self.assert_length(inaccessible_user_ids, 0)
|
||||
|
||||
self.set_up_db_for_testing_user_access()
|
||||
polonius = self.example_user("polonius")
|
||||
|
||||
inaccessible_user_ids = get_inaccessible_user_ids([bot.id, hamlet.id, othello.id], polonius)
|
||||
self.assertEqual(inaccessible_user_ids, {othello.id})
|
||||
|
||||
def test_get_users_for_spectators(self) -> None:
|
||||
# Checks that spectators can fetch users data.
|
||||
hamlet = self.example_user("hamlet")
|
||||
|
||||
Loading…
Reference in New Issue
Block a user