From ddb0bb58edbab9cf5e0d7127c4754cc60b95773c Mon Sep 17 00:00:00 2001 From: Sahil Batra Date: Thu, 11 May 2023 22:03:01 +0530 Subject: [PATCH] tests: Add tests to update visibility policy when target topic is empty. This commit adds a new test to check how the visibility policy updates when moving messages to a topic that didn't exist previously. This test also helps us adding coverage for the code which just skips setting visibility_policy if there is no need to update the value because both previous and new value of visibility policy is INHERIT. The "actions/message_edit.py" file has 100% coverage now and thus is removed from "not_yet_fully_covered" list. --- tools/test-backend | 1 - zerver/tests/test_message_edit.py | 143 ++++++++++++++++++++++++++++++ 2 files changed, 143 insertions(+), 1 deletion(-) diff --git a/tools/test-backend b/tools/test-backend index 73309e3512..00823a56f8 100755 --- a/tools/test-backend +++ b/tools/test-backend @@ -75,7 +75,6 @@ not_yet_fully_covered = [ "analytics/views/stats.py", "analytics/views/support.py", # Major lib files should have 100% coverage - "zerver/actions/message_edit.py", "zerver/actions/presence.py", "zerver/lib/addressee.py", "zerver/lib/markdown/__init__.py", diff --git a/zerver/tests/test_message_edit.py b/zerver/tests/test_message_edit.py index 19e41842a4..e5ba8f382b 100644 --- a/zerver/tests/test_message_edit.py +++ b/zerver/tests/test_message_edit.py @@ -1893,6 +1893,149 @@ class EditMessageTest(EditMessageTestCase): assert_has_visibility_policy(cordelia, target_topic, UserTopic.VisibilityPolicy.UNMUTED) assert_has_visibility_policy(aaron, target_topic, UserTopic.VisibilityPolicy.UNMUTED) + def test_user_topic_states_on_moving_to_topic_with_no_messages(self) -> None: + stream_name = "Stream 123" + stream = self.make_stream(stream_name) + + hamlet = self.example_user("hamlet") + cordelia = self.example_user("cordelia") + aaron = self.example_user("aaron") + + self.subscribe(hamlet, stream_name) + self.subscribe(cordelia, stream_name) + self.subscribe(aaron, stream_name) + + def assert_has_visibility_policy( + user_profile: UserProfile, + topic_name: str, + visibility_policy: int, + ) -> None: + self.assertTrue( + topic_has_visibility_policy(user_profile, stream.id, topic_name, visibility_policy) + ) + + # Test the case where target topic has no messages: + # + # orig_topic | final behaviour + # INHERIT INHERIT + # UNMUTED UNMUTED + # MUTED MUTED + + orig_topic = "Topic1" + target_topic = "Topic1 edited" + orig_message_id = self.send_stream_message( + hamlet, stream_name, topic_name=orig_topic, content="Hello World" + ) + + do_set_user_topic_visibility_policy( + hamlet, stream, orig_topic, visibility_policy=UserTopic.VisibilityPolicy.UNMUTED + ) + do_set_user_topic_visibility_policy( + cordelia, stream, orig_topic, visibility_policy=UserTopic.VisibilityPolicy.MUTED + ) + + check_update_message( + user_profile=hamlet, + message_id=orig_message_id, + stream_id=None, + topic_name=target_topic, + propagate_mode="change_all", + send_notification_to_old_thread=False, + send_notification_to_new_thread=False, + content=None, + ) + + assert_has_visibility_policy(hamlet, orig_topic, UserTopic.VisibilityPolicy.INHERIT) + assert_has_visibility_policy(cordelia, orig_topic, UserTopic.VisibilityPolicy.INHERIT) + assert_has_visibility_policy(aaron, orig_topic, UserTopic.VisibilityPolicy.INHERIT) + assert_has_visibility_policy(hamlet, target_topic, UserTopic.VisibilityPolicy.UNMUTED) + assert_has_visibility_policy(cordelia, target_topic, UserTopic.VisibilityPolicy.MUTED) + assert_has_visibility_policy(aaron, target_topic, UserTopic.VisibilityPolicy.INHERIT) + + def test_user_topic_state_for_messages_deleted_from_target_topic( + orig_topic: str, target_topic: str, original_topic_state: int + ) -> None: + # Test the case where target topic has no messages but has UserTopic row + # due to messages being deleted from the target topic. + orig_message_id = self.send_stream_message( + hamlet, stream_name, topic_name=orig_topic, content="Hello World" + ) + target_message_id = self.send_stream_message( + hamlet, stream_name, topic_name=target_topic, content="Hello World" + ) + + if original_topic_state != UserTopic.VisibilityPolicy.INHERIT: + users = [hamlet, cordelia, aaron] + for user in users: + do_set_user_topic_visibility_policy( + user, stream, orig_topic, visibility_policy=original_topic_state + ) + + do_set_user_topic_visibility_policy( + hamlet, stream, target_topic, visibility_policy=UserTopic.VisibilityPolicy.UNMUTED + ) + do_set_user_topic_visibility_policy( + cordelia, stream, target_topic, visibility_policy=UserTopic.VisibilityPolicy.MUTED + ) + + # Delete the message in target topic to make it empty. + self.login("hamlet") + do_set_realm_property( + hamlet.realm, + "delete_own_message_policy", + Realm.POLICY_MEMBERS_ONLY, + acting_user=None, + ) + self.client_delete(f"/json/messages/{target_message_id}") + + check_update_message( + user_profile=hamlet, + message_id=orig_message_id, + stream_id=None, + topic_name=target_topic, + propagate_mode="change_all", + send_notification_to_old_thread=False, + send_notification_to_new_thread=False, + content=None, + ) + + assert_has_visibility_policy(hamlet, orig_topic, UserTopic.VisibilityPolicy.INHERIT) + assert_has_visibility_policy(cordelia, orig_topic, UserTopic.VisibilityPolicy.INHERIT) + assert_has_visibility_policy(aaron, orig_topic, UserTopic.VisibilityPolicy.INHERIT) + assert_has_visibility_policy(hamlet, target_topic, original_topic_state) + assert_has_visibility_policy(cordelia, target_topic, original_topic_state) + assert_has_visibility_policy(aaron, target_topic, original_topic_state) + + # orig_topic | target_topic | final behaviour + # INHERIT INHERIT INHERIT + # INHERIT UNMUTED INHERIT + # INHERIT MUTED INHERIT + test_user_topic_state_for_messages_deleted_from_target_topic( + orig_topic="Topic2", + target_topic="Topic2 edited", + original_topic_state=UserTopic.VisibilityPolicy.INHERIT, + ) + + # orig_topic | target_topic | final behaviour + # MUTED INHERIT MUTED + # MUTED UNMUTED MUTED + # MUTED MUTED MUTED + test_user_topic_state_for_messages_deleted_from_target_topic( + orig_topic="Topic3", + target_topic="Topic3 edited", + original_topic_state=UserTopic.VisibilityPolicy.MUTED, + ) + + # orig_topic | target_topic | final behaviour + # UNMUTED INHERIT UNMUTED + # UNMUTED UNMUTED UNMUTED + # UNMUTED MUTED UNMUTED + test_user_topic_state_for_messages_deleted_from_target_topic( + orig_topic="Topic4", + target_topic="Topic4 edited", + original_topic_state=UserTopic.VisibilityPolicy.UNMUTED, + ) + @mock.patch("zerver.actions.message_edit.send_event") def test_wildcard_mention(self, mock_send_event: mock.MagicMock) -> None: stream_name = "Macbeth"