diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index 6e7d18ae51..4a1ed07b8c 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -7147,7 +7147,7 @@ def do_update_message( # moved_all_visible_messages below for related details. # # So for now, we require new_stream=None for this feature. - if topic_name is not None and propagate_mode != "change_one" and new_stream is None: + if propagate_mode != "change_one" and (topic_name is not None or new_stream is not None): assert stream_being_edited is not None for muting_user in get_users_muting_topic(stream_being_edited.id, orig_topic_name): # TODO: Ideally, this would be a bulk update operation, @@ -7159,12 +7159,24 @@ def do_update_message( # writing, no individual topic in Zulip Cloud had been # muted by more than 100 users. - # We call remove_topic_mute rather than do_unmute_topic to - # avoid sending two events with new muted topics in - # immediate succession; this is correct only because - # muted_topics events always send the full set of topics. - remove_topic_mute(muting_user, stream_being_edited.id, orig_topic_name) - do_mute_topic(muting_user, stream_being_edited, topic_name) + if new_stream is not None and muting_user.id in delete_event_notify_user_ids: + # If the messages are being moved to a stream the user + # cannot access, then we treat this as the + # messages/topic being deleted for this user. Unmute + # the topic for such users. + do_unmute_topic(muting_user, stream_being_edited, orig_topic_name) + else: + # Otherwise, we move the muted topic record for the user. + # We call remove_topic_mute rather than do_unmute_topic to + # avoid sending two events with new muted topics in + # immediate succession; this is correct only because + # muted_topics events always send the full set of topics. + remove_topic_mute(muting_user, stream_being_edited.id, orig_topic_name) + do_mute_topic( + muting_user, + new_stream if new_stream is not None else stream_being_edited, + topic_name if topic_name is not None else orig_topic_name, + ) send_event(user_profile.realm, event, users_to_be_notified) diff --git a/zerver/tests/test_message_edit.py b/zerver/tests/test_message_edit.py index 29ecb86d41..2f2a82cee9 100644 --- a/zerver/tests/test_message_edit.py +++ b/zerver/tests/test_message_edit.py @@ -1345,6 +1345,106 @@ class EditMessageTest(EditMessageTestCase): self.assertFalse(topic_is_muted(hamlet, stream.id, change_one_topic_name)) self.assertTrue(topic_is_muted(hamlet, stream.id, change_later_topic_name)) + # Move topic between two public streams. + desdemona = self.example_user("desdemona") + message_id = self.send_stream_message( + hamlet, stream_name, topic_name="New topic", content="Hello World" + ) + new_public_stream = self.make_stream("New public stream") + self.subscribe(desdemona, new_public_stream.name) + self.login_user(desdemona) + muted_topics = [ + [stream_name, "New topic"], + ] + set_topic_mutes(desdemona, muted_topics) + set_topic_mutes(cordelia, muted_topics) + + with queries_captured() as queries: + check_update_message( + user_profile=desdemona, + message_id=message_id, + stream_id=new_public_stream.id, + propagate_mode="change_all", + send_notification_to_old_thread=False, + send_notification_to_new_thread=False, + content=None, + ) + self.assert_length(queries, 31) + + self.assertFalse(topic_is_muted(desdemona, stream.id, "New topic")) + self.assertFalse(topic_is_muted(cordelia, stream.id, "New topic")) + self.assertFalse(topic_is_muted(aaron, stream.id, "New topic")) + self.assertTrue(topic_is_muted(desdemona, new_public_stream.id, "New topic")) + self.assertTrue(topic_is_muted(cordelia, new_public_stream.id, "New topic")) + self.assertFalse(topic_is_muted(aaron, new_public_stream.id, "New topic")) + + # Move topic to a private stream. + message_id = self.send_stream_message( + hamlet, stream_name, topic_name="New topic", content="Hello World" + ) + new_private_stream = self.make_stream("New private stream", invite_only=True) + self.subscribe(desdemona, new_private_stream.name) + self.login_user(desdemona) + muted_topics = [ + [stream_name, "New topic"], + ] + set_topic_mutes(desdemona, muted_topics) + set_topic_mutes(cordelia, muted_topics) + + with queries_captured() as queries: + check_update_message( + user_profile=desdemona, + message_id=message_id, + stream_id=new_private_stream.id, + propagate_mode="change_all", + send_notification_to_old_thread=False, + send_notification_to_new_thread=False, + content=None, + ) + self.assert_length(queries, 31) + + # Cordelia is not subscribed to the private stream, so + # Cordelia should have had the topic unmuted, while Desdemona + # should have had her muted topic record moved. + self.assertFalse(topic_is_muted(desdemona, stream.id, "New topic")) + self.assertFalse(topic_is_muted(cordelia, stream.id, "New topic")) + self.assertFalse(topic_is_muted(aaron, stream.id, "New topic")) + self.assertTrue(topic_is_muted(desdemona, new_private_stream.id, "New topic")) + self.assertFalse(topic_is_muted(cordelia, new_private_stream.id, "New topic")) + self.assertFalse(topic_is_muted(aaron, new_private_stream.id, "New topic")) + + # Move topic between two public streams with change in topic name. + desdemona = self.example_user("desdemona") + message_id = self.send_stream_message( + hamlet, stream_name, topic_name="New topic 2", content="Hello World" + ) + self.login_user(desdemona) + muted_topics = [ + [stream_name, "New topic 2"], + ] + set_topic_mutes(desdemona, muted_topics) + set_topic_mutes(cordelia, muted_topics) + + with queries_captured() as queries: + check_update_message( + user_profile=desdemona, + message_id=message_id, + stream_id=new_public_stream.id, + topic_name="changed topic name", + propagate_mode="change_all", + send_notification_to_old_thread=False, + send_notification_to_new_thread=False, + content=None, + ) + self.assert_length(queries, 31) + + self.assertFalse(topic_is_muted(desdemona, stream.id, "New topic 2")) + self.assertFalse(topic_is_muted(cordelia, stream.id, "New topic 2")) + self.assertFalse(topic_is_muted(aaron, stream.id, "New topic 2")) + self.assertTrue(topic_is_muted(desdemona, new_public_stream.id, "changed topic name")) + self.assertTrue(topic_is_muted(cordelia, new_public_stream.id, "changed topic name")) + self.assertFalse(topic_is_muted(aaron, new_public_stream.id, "changed topic name")) + @mock.patch("zerver.lib.actions.send_event") def test_wildcard_mention(self, mock_send_event: mock.MagicMock) -> None: stream_name = "Macbeth" @@ -2090,7 +2190,7 @@ class EditMessageTest(EditMessageTestCase): "topic": "new topic", }, ) - self.assert_length(queries, 52) + self.assert_length(queries, 53) self.assert_length(cache_tries, 13) messages = get_topic_messages(user_profile, old_stream, "test")