diff --git a/zerver/actions/message_edit.py b/zerver/actions/message_edit.py index 95d290faae..a6398b2ad0 100644 --- a/zerver/actions/message_edit.py +++ b/zerver/actions/message_edit.py @@ -147,18 +147,21 @@ def maybe_send_resolve_topic_notifications( old_topic_name: str, new_topic_name: str, changed_messages: QuerySet[Message], + pre_truncation_new_topic_name: str, ) -> Tuple[Optional[int], bool]: """Returns resolved_topic_message_id if resolve topic notifications were in fact sent.""" # Note that topics will have already been stripped in check_update_message. - # - # This logic is designed to treat removing a weird "✔ ✔✔ " - # prefix as unresolving the topic. - topic_resolved: bool = new_topic_name.startswith( - RESOLVED_TOPIC_PREFIX - ) and not old_topic_name.startswith(RESOLVED_TOPIC_PREFIX) - topic_unresolved: bool = old_topic_name.startswith( - RESOLVED_TOPIC_PREFIX - ) and not new_topic_name.startswith(RESOLVED_TOPIC_PREFIX) + resolved_prefix_len = len(RESOLVED_TOPIC_PREFIX) + topic_resolved: bool = ( + new_topic_name.startswith(RESOLVED_TOPIC_PREFIX) + and not old_topic_name.startswith(RESOLVED_TOPIC_PREFIX) + and pre_truncation_new_topic_name[resolved_prefix_len:] == old_topic_name + ) + topic_unresolved: bool = ( + old_topic_name.startswith(RESOLVED_TOPIC_PREFIX) + and not new_topic_name.startswith(RESOLVED_TOPIC_PREFIX) + and old_topic_name.lstrip(RESOLVED_TOPIC_PREFIX) == new_topic_name + ) if not topic_resolved and not topic_unresolved: # If there's some other weird topic that does not toggle the @@ -171,7 +174,7 @@ def maybe_send_resolve_topic_notifications( # notifications in a row: one can send new messages to the # pre-resolve topic and then resolve the topic created that # way to get multiple in the resolved topic. And then an - # administrator can the messages in between. We consider this + # administrator can delete the messages in between. We consider this # to be a fundamental risk of irresponsible message deletion, # not a bug with the "resolve topics" feature. return None, False @@ -1041,6 +1044,7 @@ def do_update_message( resolved_topic_message_deleted = False if topic_name is not None and content is None and new_stream is None: assert stream_being_edited is not None + assert pre_truncation_topic_name is not None resolved_topic_message_id, resolved_topic_message_deleted = ( maybe_send_resolve_topic_notifications( user_profile=user_profile, @@ -1048,6 +1052,7 @@ def do_update_message( old_topic_name=orig_topic_name, new_topic_name=topic_name, changed_messages=changed_messages, + pre_truncation_new_topic_name=pre_truncation_topic_name, ) ) diff --git a/zerver/tests/test_message_move_topic.py b/zerver/tests/test_message_move_topic.py index af547c9cfe..e5b5ac5441 100644 --- a/zerver/tests/test_message_move_topic.py +++ b/zerver/tests/test_message_move_topic.py @@ -1456,13 +1456,9 @@ class MessageMoveTopicTest(ZulipTestCase): ) self.assert_json_success(result) messages = get_topic_messages(user_profile, stream, new_topic_name) - self.assert_length(messages, 4) + self.assert_length(messages, 3) self.assertEqual( messages[2].content, - f"@_**{user_profile.full_name}|{user_profile.id}** has marked this topic as unresolved.", - ) - self.assertEqual( - messages[3].content, f"This topic was moved here from #**public stream>✔ test** by @_**{user_profile.full_name}|{user_profile.id}**.", ) @@ -1477,13 +1473,9 @@ class MessageMoveTopicTest(ZulipTestCase): ) self.assert_json_success(result) messages = get_topic_messages(user_profile, stream, new_resolved_topic_name) - self.assert_length(messages, 6) + self.assert_length(messages, 4) self.assertEqual( - messages[4].content, - f"@_**{user_profile.full_name}|{user_profile.id}** has marked this topic as resolved.", - ) - self.assertEqual( - messages[5].content, + messages[3].content, f"This topic was moved here from #**public stream>{new_topic_name}** by @_**{user_profile.full_name}|{user_profile.id}**.", ) @@ -1752,6 +1744,7 @@ class MessageMoveTopicTest(ZulipTestCase): old_topic_name=original_topic, new_topic_name=resolve_topic, changed_messages=changed_messages, + pre_truncation_new_topic_name=resolve_topic, ) topic_messages = get_topic_messages(admin_user, stream, resolve_topic)