resolve topic: Add resolve topic undo grace period.

Currently we send a notification to the topic if
it has been resolved or unresolved even if there
is an immediate event of resolving and then unresolving
or vice-versa. This adds a setting of
RESOLVE_TOPIC_UNDO_GRACE_PERIOD_SECONDS under which
if a topic has been unresolved after being resolved
immediately and the last message was the notification
of resolving, then delete the last message and don't
send a new notification and vice-versa.

Fixes zulip#19181.

zerver/lib/actions.py

@@ -156,6 +156,7 @@
 from zerver.lib.timestamp import datetime_to_timestamp, timestamp_to_datetime
 from zerver.lib.timezone import canonicalize_timezone
 from zerver.lib.topic import (
+    DB_TOPIC_NAME,
     LEGACY_PREV_TOPIC,
     ORIG_TOPIC,
     TOPIC_LINKS,
@@ -5700,6 +5701,28 @@ class MessageUpdateUserInfoResult(TypedDict):
 RESOLVED_TOPIC_PREFIX = "✔ "
 
 
+def confirm_topic_resolve_grace_period(stream: Stream, topic: str) -> bool:
+    message_args = {"recipient": stream.recipient, DB_TOPIC_NAME: topic}
+    last_message = Message.objects.filter(**message_args).last()
+    current_time = timezone_now()
+
+    if last_message:
+        time_difference = (current_time - last_message.date_sent).total_seconds()
+        is_last_message_resolve_notif = False
+        last_sender = get_system_bot(settings.NOTIFICATION_BOT)
+
+        if last_message.content.endswith("resolved.") and last_message.sender == last_sender:
+            is_last_message_resolve_notif = True
+
+        if (
+            time_difference < settings.RESOLVE_TOPIC_UNDO_GRACE_PERIOD_SECONDS
+            and is_last_message_resolve_notif
+        ):
+            do_delete_messages(last_sender.realm, [last_message])
+            return True
+    return False
+
+
 def maybe_send_resolve_topic_notifications(
     *,
     user_profile: UserProfile,
@@ -5738,6 +5761,9 @@ def maybe_send_resolve_topic_notifications(
         # not a bug with the "resolve topics" feature.
         return
 
+    if confirm_topic_resolve_grace_period(stream, new_topic):
+        return
+
     sender = get_system_bot(settings.NOTIFICATION_BOT)
     user_mention = f"@_**{user_profile.full_name}|{user_profile.id}**"
     with override_language(stream.realm.default_language):

zerver/tests/test_message_edit.py

@@ -2031,15 +2031,88 @@ def test_mark_topic_as_resolved(self) -> None:
         )
 
         unresolved_topic = original_topic
+
+        with mock.patch(
+            "zerver.lib.actions.timezone_now",
+            return_value=messages[2].date_sent + datetime.timedelta(seconds=5),
+        ):
+            result = self.client_patch(
+                "/json/messages/" + str(id1),
+                {
+                    "message_id": id1,
+                    "topic": unresolved_topic,
+                    "propagate_mode": "change_all",
+                },
+            )
+
+        self.assert_json_success(result)
+        for msg_id in [id1, id2]:
+            msg = Message.objects.get(id=msg_id)
+            self.assertEqual(
+                unresolved_topic,
+                msg.topic_name(),
+            )
+
+        messages = get_topic_messages(admin_user, stream, unresolved_topic)
+        self.assert_length(messages, 4)
+        self.assertEqual(
+            messages[3].content,
+            f"@_**Iago|{admin_user.id}** has marked this topic as unresolved.",
+        )
+
+    def test_topic_resolve_within_grace_period(self) -> None:
+        self.login("iago")
+        admin_user = self.example_user("iago")
+        stream = self.make_stream("new")
+        self.subscribe(admin_user, stream.name)
+        original_topic = "topic 1"
+        id1 = self.send_stream_message(
+            self.example_user("hamlet"), "new", content="message 1", topic_name=original_topic
+        )
+        id2 = self.send_stream_message(
+            admin_user, "new", content="message 2", topic_name=original_topic
+        )
+
+        resolved_topic = RESOLVED_TOPIC_PREFIX + original_topic
         result = self.client_patch(
             "/json/messages/" + str(id1),
             {
                 "message_id": id1,
-                "topic": unresolved_topic,
+                "topic": resolved_topic,
                 "propagate_mode": "change_all",
             },
         )
 
+        self.assert_json_success(result)
+        for msg_id in [id1, id2]:
+            msg = Message.objects.get(id=msg_id)
+            self.assertEqual(
+                resolved_topic,
+                msg.topic_name(),
+            )
+
+        messages = get_topic_messages(admin_user, stream, resolved_topic)
+        self.assert_length(messages, 3)
+        self.assertEqual(
+            messages[2].content,
+            f"@_**Iago|{admin_user.id}** has marked this topic as resolved.",
+        )
+
+        unresolved_topic = original_topic
+
+        with mock.patch(
+            "zerver.lib.actions.timezone_now",
+            return_value=messages[2].date_sent + datetime.timedelta(seconds=4),
+        ):
+            result = self.client_patch(
+                "/json/messages/" + str(id1),
+                {
+                    "message_id": id1,
+                    "topic": unresolved_topic,
+                    "propagate_mode": "change_all",
+                },
+            )
+
         self.assert_json_success(result)
         for msg_id in [id1, id2]:
             msg = Message.objects.get(id=msg_id)
@@ -2049,10 +2122,10 @@ def test_mark_topic_as_resolved(self) -> None:
             )
 
         messages = get_topic_messages(admin_user, stream, unresolved_topic)
-        self.assert_length(messages, 4)
+        self.assert_length(messages, 2)
         self.assertEqual(
-            messages[3].content,
-            f"@_**Iago|{admin_user.id}** has marked this topic as unresolved.",
+            messages[1].content,
+            "message 2",
         )
 
 

zproject/default_settings.py

@@ -455,3 +455,8 @@ class JwtAuthKey(TypedDict):
 # Any message content exceeding this limit will be truncated.
 # See: `_internal_prep_message` function in zerver/lib/actions.py.
 MAX_MESSAGE_LENGTH = 10000
+
+# Grace period before which we don't send a resolve/unresolve
+# notification to stream and also delete the previous counter
+# notification.
+RESOLVE_TOPIC_UNDO_GRACE_PERIOD_SECONDS = 60

zproject/dev_settings.py

@@ -176,3 +176,5 @@
 SOCIAL_AUTH_SAML_SP_ENTITY_ID = "http://localhost:9991"
 
 MEMCACHED_USERNAME = None
+
+RESOLVE_TOPIC_UNDO_GRACE_PERIOD_SECONDS = 5