topics: Apply automatic follow/unmute when message moved to new topic.

This commit adds a feature, wherein when someone moves a user's
message to be the first message in a topic, and the user has the
Automatically follow topics initiated enabled, then the new topic will
also be followed by the user.

Similarly, if the user has Automatically unmute topics initiated
enabled, the moved topic would also be unmuted.

Fixes #28408.
This commit is contained in:
roanster007
2024-02-18 16:04:58 +05:30
committed by Tim Abbott
parent 93913dd6e7
commit 0b2cb835f5
2 changed files with 353 additions and 1 deletions

View File

@@ -586,6 +586,41 @@ def update_message_content(
) )
def apply_automatic_unmute_follow_topics_policy(
user_profile: UserProfile,
target_stream: Stream,
target_topic_name: str,
) -> None:
if (
user_profile.automatically_follow_topics_policy
== UserProfile.AUTOMATICALLY_CHANGE_VISIBILITY_POLICY_ON_INITIATION
):
bulk_do_set_user_topic_visibility_policy(
[user_profile],
target_stream,
target_topic_name,
visibility_policy=UserTopic.VisibilityPolicy.FOLLOWED,
)
elif (
user_profile.automatically_unmute_topics_in_muted_streams_policy
== UserProfile.AUTOMATICALLY_CHANGE_VISIBILITY_POLICY_ON_INITIATION
):
subscription = Subscription.objects.filter(
recipient=target_stream.recipient,
user_profile=user_profile,
active=True,
is_user_active=True,
).first()
if subscription is not None and subscription.is_muted:
bulk_do_set_user_topic_visibility_policy(
[user_profile],
target_stream,
target_topic_name,
visibility_policy=UserTopic.VisibilityPolicy.UNMUTED,
)
# This must be called already in a transaction, with a write lock on # This must be called already in a transaction, with a write lock on
# the target_message. # the target_message.
@transaction.atomic(savepoint=False) @transaction.atomic(savepoint=False)
@@ -1152,6 +1187,42 @@ def do_update_message(
visibility_policy=new_visibility_policy, visibility_policy=new_visibility_policy,
) )
elif message_edit_request.is_stream_edited or message_edit_request.is_topic_edited:
sender = target_message.sender
target_stream = message_edit_request.target_stream
target_topic = message_edit_request.target_topic_name
assert target_stream.recipient_id is not None
messages_in_target_topic = messages_for_topic(
realm.id, target_stream.recipient_id, target_topic
).exclude(id__in=[*changed_message_ids])
# All behavior in channels with protected history depends on
# the permissions of users who might glean information about
# whether the topic previously existed. So we need to look at
# whether this message is becoming the first message in the
# target topic, as seen by the user who we might change topic
# visibility policy for in this code path.
first_message_in_target_topic = bulk_access_stream_messages_query(
sender, messages_in_target_topic, target_stream
).first()
is_target_message_first = False
# the target_message would be the first message in the moved topic
# either if the moved topic doesn't have any messages, or if the
# target_message is sent before the first message in the moved
# topic.
if (
first_message_in_target_topic is None
or target_message.id < first_message_in_target_topic.id
):
is_target_message_first = True
if not sender.is_bot and sender not in users_losing_access and is_target_message_first:
apply_automatic_unmute_follow_topics_policy(sender, target_stream, target_topic)
send_event_on_commit(user_profile.realm, event, users_to_be_notified) send_event_on_commit(user_profile.realm, event, users_to_be_notified)
resolved_topic_message_id = None resolved_topic_message_id = None

View File

@@ -1,5 +1,6 @@
from datetime import timedelta from datetime import timedelta
from operator import itemgetter from operator import itemgetter
from typing import Literal
from unittest import mock from unittest import mock
import orjson import orjson
@@ -14,16 +15,26 @@ from zerver.actions.realm_settings import (
) )
from zerver.actions.streams import do_change_stream_group_based_setting, do_deactivate_stream from zerver.actions.streams import do_change_stream_group_based_setting, do_deactivate_stream
from zerver.actions.user_groups import add_subgroups_to_user_group, check_add_user_group from zerver.actions.user_groups import add_subgroups_to_user_group, check_add_user_group
from zerver.actions.user_settings import do_change_user_setting
from zerver.actions.user_topics import do_set_user_topic_visibility_policy from zerver.actions.user_topics import do_set_user_topic_visibility_policy
from zerver.lib import utils from zerver.lib import utils
from zerver.lib.message import messages_for_ids from zerver.lib.message import messages_for_ids
from zerver.lib.message_cache import MessageDict from zerver.lib.message_cache import MessageDict
from zerver.lib.stream_topic import StreamTopicTarget
from zerver.lib.test_classes import ZulipTestCase from zerver.lib.test_classes import ZulipTestCase
from zerver.lib.test_helpers import most_recent_message, queries_captured from zerver.lib.test_helpers import most_recent_message, queries_captured
from zerver.lib.timestamp import datetime_to_timestamp from zerver.lib.timestamp import datetime_to_timestamp
from zerver.lib.topic import TOPIC_NAME from zerver.lib.topic import TOPIC_NAME
from zerver.lib.utils import assert_is_not_none from zerver.lib.utils import assert_is_not_none
from zerver.models import Attachment, Message, NamedUserGroup, Realm, UserProfile, UserTopic from zerver.models import (
Attachment,
Message,
NamedUserGroup,
Realm,
Subscription,
UserProfile,
UserTopic,
)
from zerver.models.groups import SystemGroups from zerver.models.groups import SystemGroups
from zerver.models.messages import UserMessage from zerver.models.messages import UserMessage
from zerver.models.realms import MessageEditHistoryVisibilityPolicyEnum, get_realm from zerver.models.realms import MessageEditHistoryVisibilityPolicyEnum, get_realm
@@ -2335,3 +2346,273 @@ class EditMessageTest(ZulipTestCase):
"'prev_content_sha256' value does not match the expected value.", "'prev_content_sha256' value does not match the expected value.",
) )
self.check_message(msg_id, topic_name="editing", content="First user edit") self.check_message(msg_id, topic_name="editing", content="First user edit")
def check_automatic_change_visibility_policy_on_initiation_during_moving_messages(
self,
message_id: int,
sender_id: int,
stream_topic_target: StreamTopicTarget,
visibility_policy: Literal[
UserTopic.VisibilityPolicy.FOLLOWED, UserTopic.VisibilityPolicy.UNMUTED
],
expected_follow_or_unmute_target_topic: bool | None = None,
) -> None:
result = self.client_patch(
f"/json/messages/{message_id}",
{
"stream_id": stream_topic_target.stream_id,
"topic": stream_topic_target.topic_name,
},
)
self.assert_json_success(result)
user_ids = stream_topic_target.user_ids_with_visibility_policy(visibility_policy)
if expected_follow_or_unmute_target_topic:
self.assertIn(sender_id, user_ids)
else:
self.assertNotIn(sender_id, user_ids)
def test_move_message_to_new_topic_with_automatic_follow_policy(self) -> None:
self.login("iago")
iago = self.example_user("iago")
cordelia = self.example_user("cordelia")
hamlet = self.example_user("hamlet")
shiva = self.example_user("shiva")
users = [iago, cordelia, hamlet, shiva]
stream = self.make_stream("new_stream")
original_topic = "original"
post_move = "post-move"
for user in users:
self.subscribe(user, stream.name)
do_change_user_setting(
user,
"automatically_follow_topics_policy",
UserProfile.AUTOMATICALLY_CHANGE_VISIBILITY_POLICY_ON_INITIATION,
acting_user=None,
)
msg_ids = [
self.send_stream_message(
sender=sender,
stream_name=stream.name,
topic_name=original_topic,
content=f"Message sent by {sender.full_name}",
)
for sender in users
]
stream_topic_target_original = StreamTopicTarget(
stream_id=stream.id,
topic_name=original_topic,
)
user_ids = stream_topic_target_original.user_ids_with_visibility_policy(
UserTopic.VisibilityPolicy.FOLLOWED
)
self.assertEqual(user_ids, {iago.id})
stream_topic_target_post_move = StreamTopicTarget(
stream_id=stream.id,
topic_name=post_move,
)
# If the target topic has no message, then the visibility policy
# of the sender of first message being moved to the topic is set
# to FOLLOWED.
self.check_automatic_change_visibility_policy_on_initiation_during_moving_messages(
msg_ids[2],
hamlet.id,
stream_topic_target_post_move,
UserTopic.VisibilityPolicy.FOLLOWED,
True,
)
# If the target topic already has messages in it, then the visibility
# policy of the sender of first message being moved to topic is only
# set if it is sent before the first preexisting message of target
# topic
self.check_automatic_change_visibility_policy_on_initiation_during_moving_messages(
msg_ids[3],
shiva.id,
stream_topic_target_post_move,
UserTopic.VisibilityPolicy.FOLLOWED,
False,
)
self.check_automatic_change_visibility_policy_on_initiation_during_moving_messages(
msg_ids[1],
cordelia.id,
stream_topic_target_post_move,
UserTopic.VisibilityPolicy.FOLLOWED,
True,
)
# If the message is moved to a topic in a new private stream with
# protected history, then visibility policy of sender is set to
# FOLLOWED only if it can be accessed by the sender.
private_stream = self.make_stream(
"private", invite_only=True, history_public_to_subscribers=False
)
self.subscribe(iago, private_stream.name)
self.subscribe(cordelia, private_stream.name)
stream_topic_target_post_move = StreamTopicTarget(
stream_id=private_stream.id,
topic_name=post_move,
)
user_ids = stream_topic_target_post_move.user_ids_with_visibility_policy(
UserTopic.VisibilityPolicy.FOLLOWED
)
self.assertEqual(user_ids, set())
self.check_automatic_change_visibility_policy_on_initiation_during_moving_messages(
msg_ids[2],
hamlet.id,
stream_topic_target_post_move,
UserTopic.VisibilityPolicy.FOLLOWED,
False,
)
self.check_automatic_change_visibility_policy_on_initiation_during_moving_messages(
msg_ids[1],
cordelia.id,
stream_topic_target_post_move,
UserTopic.VisibilityPolicy.FOLLOWED,
True,
)
def test_move_message_to_new_topic_with_automatic_unmute_policy(self) -> None:
self.login("iago")
iago = self.example_user("iago")
cordelia = self.example_user("cordelia")
hamlet = self.example_user("hamlet")
shiva = self.example_user("shiva")
users = [iago, cordelia, hamlet, shiva]
stream = self.make_stream("new_stream")
recipient = stream.recipient
original_topic = "original"
post_move = "post-move"
for user in users:
self.subscribe(user, stream.name)
do_change_user_setting(
user,
"automatically_unmute_topics_in_muted_streams_policy",
UserProfile.AUTOMATICALLY_CHANGE_VISIBILITY_POLICY_ON_INITIATION,
acting_user=None,
)
subscription = Subscription.objects.get(recipient=recipient, user_profile=user)
subscription.is_muted = True
subscription.save()
msg_ids = [
self.send_stream_message(
sender=sender,
stream_name=stream.name,
topic_name=original_topic,
content=f"Message sent by {sender.full_name}",
)
for sender in users
]
stream_topic_target_original = StreamTopicTarget(
stream_id=stream.id,
topic_name=original_topic,
)
user_ids = stream_topic_target_original.user_ids_with_visibility_policy(
UserTopic.VisibilityPolicy.UNMUTED
)
self.assertEqual(user_ids, {iago.id})
stream_topic_target_post_move = StreamTopicTarget(
stream_id=stream.id,
topic_name=post_move,
)
# If the target topic has no message, then the visibility policy
# of the sender of first message being moved to the topic is set
# to UNMUTED.
self.check_automatic_change_visibility_policy_on_initiation_during_moving_messages(
msg_ids[2],
hamlet.id,
stream_topic_target_post_move,
UserTopic.VisibilityPolicy.UNMUTED,
True,
)
# If the target topic already has messages in it, then the visibility
# policy of the sender of first message being moved to topic is only
# set if it is sent before the first preexisting message of target
# topic
self.check_automatic_change_visibility_policy_on_initiation_during_moving_messages(
msg_ids[3],
shiva.id,
stream_topic_target_post_move,
UserTopic.VisibilityPolicy.UNMUTED,
False,
)
self.check_automatic_change_visibility_policy_on_initiation_during_moving_messages(
msg_ids[1],
cordelia.id,
stream_topic_target_post_move,
UserTopic.VisibilityPolicy.UNMUTED,
True,
)
def test_automatic_follow_policy_in_channel_with_protected_history(self) -> None:
self.login("iago")
iago = self.example_user("iago")
hamlet = self.example_user("hamlet")
do_change_user_setting(
hamlet,
"automatically_follow_topics_policy",
UserProfile.AUTOMATICALLY_CHANGE_VISIBILITY_POLICY_ON_INITIATION,
acting_user=None,
)
# Iago's message to "test topic" is not visible to Hamlet.
core = self.make_stream("core", iago.realm, True, history_public_to_subscribers=False)
self.subscribe(iago, "core")
self.send_stream_message(iago, "core", topic_name="test topic")
self.subscribe(hamlet, "core")
self.send_stream_message(iago, "core", topic_name="#general")
msg_id = self.send_stream_message(hamlet, "core", topic_name="#general")
stream_topic_target_post_move = StreamTopicTarget(
stream_id=core.id,
topic_name="test topic",
)
self.assertEqual(
stream_topic_target_post_move.user_ids_with_visibility_policy(
UserTopic.VisibilityPolicy.FOLLOWED
),
set(),
)
# Verify that Hamlet follows the topic after moving his
# message, because as far as he knows, his message is now the
# first message in "test topic".
self.check_automatic_change_visibility_policy_on_initiation_during_moving_messages(
msg_id,
hamlet.id,
stream_topic_target_post_move,
UserTopic.VisibilityPolicy.FOLLOWED,
True,
)
self.assertEqual(
stream_topic_target_post_move.user_ids_with_visibility_policy(
UserTopic.VisibilityPolicy.FOLLOWED
),
{hamlet.id},
)