resolve_topic: Prevent incorrect notification during message move.

This commit fixes the bug where the "topic unresolved" notification
is wrongly triggered when moving a message between a resolved and
unresolved topic, except for when the topics have the same name.

To resolve this issue, the commit ensures that resolved/unresolved
notifications are not sent if a message has been moved to a new
topic. This is achieved by comparing the names of the old and new
topics without considering the "resolved prefix".

The commit also accounts for the scenario where `new_topic_name`
has been truncated, indicating that it was resolved and the name
had to change to accommodate the "resolved prefix".

This solution does not try to specially handle the possible case that
a stream has two topics with the same name, even if one is resolved
and another unresolved.

Fixes #29007.

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,
             )
         )
 

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)