From 826ea4162e78afb548cfb3764e04300bf97975c3 Mon Sep 17 00:00:00 2001 From: Prakhar Pratyush Date: Fri, 3 Feb 2023 17:51:25 +0530 Subject: [PATCH] user_topics: Refactor 'do_unmute_topic'. Replaces 'do_unmute_topic' with 'do_set_user_topic_visibility_policy' and associated minor changes. This change is made to align with the plan to use a single function 'do_set_user_topic_visibility_policy' to manage user_topic - visibility_policy changes and corresponding event generation. --- zerver/actions/message_edit.py | 12 +++++-- zerver/actions/user_topics.py | 58 +++++++++++--------------------- zerver/tests/test_events.py | 10 ++++-- zerver/tests/test_user_topics.py | 5 +-- zerver/views/user_topics.py | 6 ++-- 5 files changed, 43 insertions(+), 48 deletions(-) diff --git a/zerver/actions/message_edit.py b/zerver/actions/message_edit.py index 2102cd20b8..29a2ee56fb 100644 --- a/zerver/actions/message_edit.py +++ b/zerver/actions/message_edit.py @@ -19,7 +19,7 @@ from zerver.actions.message_send import ( render_incoming_message, ) from zerver.actions.uploads import check_attachment_reference_change -from zerver.actions.user_topics import do_set_user_topic_visibility_policy, do_unmute_topic +from zerver.actions.user_topics import do_set_user_topic_visibility_policy from zerver.lib.exceptions import JsonableError from zerver.lib.markdown import MessageRenderingResult, topic_links from zerver.lib.markdown import version as markdown_version @@ -782,15 +782,21 @@ def do_update_message( # important for security reasons; we don't want to # give users a UserTopic row in a stream they cannot # access. Unmute the topic for such users. - do_unmute_topic(muting_user, stream_being_edited, orig_topic_name) + do_set_user_topic_visibility_policy( + muting_user, + stream_being_edited, + orig_topic_name, + visibility_policy=UserTopic.VISIBILITY_POLICY_INHERIT, + ) else: # Otherwise, we move the muted topic record for the # user, but removing the old topic mute and then # creating a new one. - do_unmute_topic( + do_set_user_topic_visibility_policy( muting_user, stream_being_edited, orig_topic_name, + visibility_policy=UserTopic.VISIBILITY_POLICY_INHERIT, # do_set_user_topic_visibility_policy with visibility_policy # set to UserTopic.MUTED will send an updated muted topic # event, which contains the full set of muted diff --git a/zerver/actions/user_topics.py b/zerver/actions/user_topics.py index 2d7dfb0964..a6e17768b2 100644 --- a/zerver/actions/user_topics.py +++ b/zerver/actions/user_topics.py @@ -11,34 +11,6 @@ from zerver.models import Stream, UserProfile, UserTopic from zerver.tornado.django_api import send_event -def do_unmute_topic( - user_profile: UserProfile, stream: Stream, topic: str, *, skip_muted_topics_event: bool = False -) -> None: - try: - remove_topic_mute(user_profile, stream.id, topic) - except UserTopic.DoesNotExist: - raise JsonableError(_("Topic is not muted")) - - # This first muted_topics event is deprecated and will be removed - # once clients are migrated to handle the user_topic event type - # instead. - if not skip_muted_topics_event: - muted_topics_event = dict(type="muted_topics", muted_topics=get_topic_mutes(user_profile)) - send_event(user_profile.realm, muted_topics_event, [user_profile.id]) - - date_unmuted = timezone_now() - - user_topic_event: Dict[str, Any] = { - "type": "user_topic", - "stream_id": stream.id, - "topic_name": topic, - "last_updated": datetime_to_timestamp(date_unmuted), - "visibility_policy": UserTopic.VISIBILITY_POLICY_INHERIT, - } - - send_event(user_profile.realm, user_topic_event, [user_profile.id]) - - def do_set_user_topic_visibility_policy( user_profile: UserProfile, stream: Stream, @@ -47,25 +19,33 @@ def do_set_user_topic_visibility_policy( visibility_policy: int, last_updated: Optional[datetime.datetime] = None, ignore_duplicate: bool = False, + skip_muted_topics_event: bool = False, ) -> None: if last_updated is None: last_updated = timezone_now() - assert stream.recipient_id is not None - add_topic_mute( - user_profile, - stream.id, - stream.recipient_id, - topic, - last_updated, - ignore_duplicate=ignore_duplicate, - ) + if visibility_policy == UserTopic.VISIBILITY_POLICY_INHERIT: + try: + remove_topic_mute(user_profile, stream.id, topic) + except UserTopic.DoesNotExist: + raise JsonableError(_("Topic is not muted")) + else: + assert stream.recipient_id is not None + add_topic_mute( + user_profile, + stream.id, + stream.recipient_id, + topic, + last_updated, + ignore_duplicate=ignore_duplicate, + ) # This first muted_topics event is deprecated and will be removed # once clients are migrated to handle the user_topic event type # instead. - muted_topics_event = dict(type="muted_topics", muted_topics=get_topic_mutes(user_profile)) - send_event(user_profile.realm, muted_topics_event, [user_profile.id]) + if not skip_muted_topics_event: + muted_topics_event = dict(type="muted_topics", muted_topics=get_topic_mutes(user_profile)) + send_event(user_profile.realm, muted_topics_event, [user_profile.id]) user_topic_event: Dict[str, Any] = { "type": "user_topic", diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index acfb71c041..22e3b3356a 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -107,7 +107,7 @@ from zerver.actions.user_settings import ( do_regenerate_api_key, ) from zerver.actions.user_status import do_update_user_status -from zerver.actions.user_topics import do_set_user_topic_visibility_policy, do_unmute_topic +from zerver.actions.user_topics import do_set_user_topic_visibility_policy from zerver.actions.users import ( do_change_user_role, do_deactivate_user, @@ -1437,7 +1437,13 @@ class NormalActionsTest(BaseAction): check_user_topic("events[1]", events[1]) events = self.verify_action( - lambda: do_unmute_topic(self.user_profile, stream, "topic"), num_events=2 + lambda: do_set_user_topic_visibility_policy( + self.user_profile, + stream, + "topic", + visibility_policy=UserTopic.VISIBILITY_POLICY_INHERIT, + ), + num_events=2, ) check_muted_topics("events[0]", events[0]) check_user_topic("events[1]", events[1]) diff --git a/zerver/tests/test_user_topics.py b/zerver/tests/test_user_topics.py index c89854c6a9..87e908455f 100644 --- a/zerver/tests/test_user_topics.py +++ b/zerver/tests/test_user_topics.py @@ -4,7 +4,7 @@ from unittest import mock from django.utils.timezone import now as timezone_now -from zerver.actions.user_topics import do_set_user_topic_visibility_policy, do_unmute_topic +from zerver.actions.user_topics import do_set_user_topic_visibility_policy from zerver.lib.stream_topic import StreamTopicTarget from zerver.lib.test_classes import ZulipTestCase from zerver.lib.user_topics import ( @@ -102,10 +102,11 @@ class MutedTopicsTests(ZulipTestCase): self.assertIn((stream.name, "Verona3", mock_date_muted), get_topic_mutes(user)) self.assertTrue(topic_is_muted(user, stream.id, "verona3")) - do_unmute_topic( + do_set_user_topic_visibility_policy( user, stream, "Verona3", + visibility_policy=UserTopic.VISIBILITY_POLICY_INHERIT, ) assert stream.recipient is not None diff --git a/zerver/views/user_topics.py b/zerver/views/user_topics.py index 00d4297672..34b5f9fabe 100644 --- a/zerver/views/user_topics.py +++ b/zerver/views/user_topics.py @@ -6,7 +6,7 @@ from django.http import HttpRequest, HttpResponse from django.utils.timezone import now as timezone_now from django.utils.translation import gettext as _ -from zerver.actions.user_topics import do_set_user_topic_visibility_policy, do_unmute_topic +from zerver.actions.user_topics import do_set_user_topic_visibility_policy from zerver.lib.exceptions import JsonableError from zerver.lib.request import REQ, has_request_variables from zerver.lib.response import json_success @@ -60,7 +60,9 @@ def unmute_topic( assert stream_id is not None stream = access_stream_for_unmute_topic_by_id(user_profile, stream_id, error) - do_unmute_topic(user_profile, stream, topic_name) + do_set_user_topic_visibility_policy( + user_profile, stream, topic_name, visibility_policy=UserTopic.VISIBILITY_POLICY_INHERIT + ) @has_request_variables