message_edit: Add last_moved_timestamp to API message objects.

This will allow clients to display MOVED/EDITED indicators, and their
tooltips, without interacting with the `edit_history` section of
message objects, which we plan to remove in the future.

Supporting that requires both introducing both last_moved_timestamp,
and changing the definition of last_edit_timestamp to not include
message moves, which involves recalculating it at the API layer.

The last_moved_timestamp is not present if the topic moves for the
message are for resolving or unresolving the topic. It is always
present for channel moves.

Co-authored-by: Lauryn Menard <lauryn@zulip.com>

api_docs/changelog.md

@@ -20,6 +20,25 @@ format used by the Zulip server that they are interacting with.
 
 ## Changes in Zulip 10.0
 
+**Feature level 365**
+
+* [`GET /events`](/api/get-events), [`GET /messages`](/api/get-messages),
+  [`GET /messages/{message_id}`](/api/get-message): Added
+  `last_moved_timestamp` field to message objects for when the message
+  was last moved to a different channel or topic. If the message's topic
+  has only been [resolved or unresolved](/help/resolve-a-topic), then
+  the field is not present. Clients should use this field, rather than
+  parsing the message object's `edit_history` array, to display an
+  indicator that the message has been moved.
+ * [`GET /events`](/api/get-events), [`GET /messages`](/api/get-messages),
+  [`GET /messages/{message_id}`](/api/get-message): The
+  `last_edit_timestamp` field on message objects is only present if the
+  message's content has been edited. Previously, this field was present
+  if the message's content had been edited or moved to a different
+  channel or topic. Clients should use this field, rather than parsing
+  the message object's `edit_history` array, to display an indicator
+  that the message has been edited.
+
 **Feature level 364**
 
 * [`PATCH /realm/user_settings_defaults`](/api/update-realm-user-settings-defaults),

version.py

@@ -34,7 +34,7 @@ DESKTOP_WARNING_VERSION = "5.9.3"
 # new level means in api_docs/changelog.md, as well as "**Changes**"
 # entries in the endpoint's documentation in `zulip.yaml`.
 
-API_FEATURE_LEVEL = 364
+API_FEATURE_LEVEL = 365
 
 # Bump the minor PROVISION_VERSION to indicate that folks should provision
 # only when going from an old version of the code to a newer version. Bump

zerver/lib/message.py

@@ -34,6 +34,7 @@ from zerver.lib.streams import (
 )
 from zerver.lib.topic import (
     MESSAGE__TOPIC,
+    RESOLVED_TOPIC_PREFIX,
     TOPIC_NAME,
     maybe_rename_general_chat_to_empty_topic,
     messages_for_topic,
@@ -215,8 +216,11 @@ def normalize_body_for_import(body: str) -> str:
     return truncate_content(body, settings.MAX_MESSAGE_LENGTH, "\n[message truncated]")
 
 
+TOPIC_TRUNCATION_MESSAGE = "..."
+
+
 def truncate_topic(topic_name: str) -> str:
-    return truncate_content(topic_name, MAX_TOPIC_NAME_LENGTH, "...")
+    return truncate_content(topic_name, MAX_TOPIC_NAME_LENGTH, TOPIC_TRUNCATION_MESSAGE)
 
 
 def visible_edit_history_for_message(
@@ -242,6 +246,35 @@ def visible_edit_history_for_message(
     return visible_edit_history
 
 
+# This is similar to what we do in build_message_edit_request in
+# zerver/actions/message_edit.py, but since we don't have the
+# pre-truncation topic name in the message edit history object,
+# the logic for the topic resolved case is different here.
+def topic_resolve_toggled(topic: str, prev_topic: str) -> bool:
+    resolved_prefix_len = len(RESOLVED_TOPIC_PREFIX)
+    truncation_len = len(TOPIC_TRUNCATION_MESSAGE)
+    # Topic unresolved
+    if prev_topic.startswith(RESOLVED_TOPIC_PREFIX) and not topic.startswith(RESOLVED_TOPIC_PREFIX):
+        return prev_topic[resolved_prefix_len:] == topic
+
+    # Topic resolved
+    if topic.startswith(RESOLVED_TOPIC_PREFIX) and not prev_topic.startswith(RESOLVED_TOPIC_PREFIX):
+        if len(prev_topic) <= MAX_TOPIC_NAME_LENGTH - resolved_prefix_len:
+            # When the topic was resolved, it was not truncated,
+            # so we remove the resolved prefix and compare.
+            return topic[resolved_prefix_len:] == prev_topic
+        if topic.endswith(TOPIC_TRUNCATION_MESSAGE):
+            # When the topic was resolved, it was likely truncated,
+            # so we confirm the previous topic starts with the topic
+            # without the resolved prefix and truncation message.
+            topic_without_resolved_prefix_and_truncation_message = topic[
+                resolved_prefix_len:-truncation_len
+            ]
+            return prev_topic.startswith(topic_without_resolved_prefix_and_truncation_message)
+
+    return False
+
+
 def messages_for_ids(
     message_ids: list[int],
     user_message_flags: dict[int, list[str]],
@@ -284,6 +317,29 @@ def messages_for_ids(
         if message_id in search_fields:
             msg_dict.update(search_fields[message_id])
         if "edit_history" in msg_dict:
+            # In addition to computing last_moved_timestamp, we recompute
+            # last_edit_timestamp, because the logic powering the database
+            # field updates it on moves as well, and we'd like to show the
+            # correct value for messages that had only been moved.
+            last_moved_timestamp = 0
+            last_edit_timestamp = 0
+            for item in msg_dict["edit_history"]:
+                if "prev_stream" in item:
+                    last_moved_timestamp = max(last_moved_timestamp, item["timestamp"])
+                elif "prev_topic" in item and not topic_resolve_toggled(
+                    item["topic"], item["prev_topic"]
+                ):
+                    last_moved_timestamp = max(last_moved_timestamp, item["timestamp"])
+                if "prev_content" in item:
+                    last_edit_timestamp = max(last_edit_timestamp, item["timestamp"])
+            if last_moved_timestamp != 0:
+                msg_dict["last_moved_timestamp"] = last_moved_timestamp
+            if last_edit_timestamp != 0:
+                msg_dict["last_edit_timestamp"] = last_edit_timestamp
+            else:
+                # Remove it if it was already present.
+                msg_dict.pop("last_edit_timestamp", None)
+
             if (
                 message_edit_history_visibility_policy
                 == MessageEditHistoryVisibilityPolicyEnum.none.value
@@ -294,6 +350,7 @@ def messages_for_ids(
                     message_edit_history_visibility_policy, msg_dict["edit_history"]
                 )
                 msg_dict["edit_history"] = visible_edit_history
+
         msg_dict["can_access_sender"] = msg_dict["sender_id"] not in inaccessible_sender_ids
         message_list.append(msg_dict)
 

zerver/openapi/zulip.yaml

@@ -7216,6 +7216,7 @@ paths:
                                 id: {}
                                 is_me_message: {}
                                 last_edit_timestamp: {}
+                                last_moved_timestamp: {}
                                 reactions: {}
                                 recipient_id: {}
                                 sender_email: {}
@@ -8583,6 +8584,7 @@ paths:
                               id: {}
                               is_me_message: {}
                               last_edit_timestamp: {}
+                              last_moved_timestamp: {}
                               reactions: {}
                               recipient_id: {}
                               sender_email: {}
@@ -22456,6 +22458,7 @@ paths:
                           id: {}
                           is_me_message: {}
                           last_edit_timestamp: {}
+                          last_moved_timestamp: {}
                           reactions: {}
                           recipient_id: {}
                           sender_email: {}
@@ -24114,6 +24117,7 @@ components:
             id: {}
             is_me_message: {}
             last_edit_timestamp: {}
+            last_moved_timestamp: {}
             reactions: {}
             recipient_id: {}
             sender_email: {}
@@ -24304,10 +24308,32 @@ components:
         last_edit_timestamp:
           type: integer
           description: |
-            The UNIX timestamp for when the message was last edited,
+            The UNIX timestamp for when the message's content was last edited, in
-            in UTC seconds.
+            UTC seconds.
 
-            Not present if the message has never been edited.
+            Not present if the message's content has never been edited.
+
+            Clients should use this field, rather than parsing the `edit_history`
+            array, to display an indicator that the message has been edited.
+
+            **Changes**: Prior to Zulip 10.0 (feature level 365), this was the
+            time when the message was last edited or moved.
+        last_moved_timestamp:
+          type: integer
+          description: |
+            The UNIX timestamp for when the message was last moved to a different
+            channel or topic, in UTC seconds.
+
+            Not present if the message has never been moved, or if the only topic
+            moves for the message are [resolving or unresolving](/help/resolve-a-topic)
+            the message's topic.
+
+            Clients should use this field, rather than parsing the `edit_history`
+            array, to display an indicator that the message has been moved.
+
+            **Changes**: New in Zulip 10.0 (feature level 365). Previously,
+            parsing the `edit_history` array was required in order to correctly
+            display moved message indicators.
         reactions:
           type: array
           description: |

zerver/tests/test_message_edit.py

@@ -3,6 +3,7 @@ from operator import itemgetter
 from unittest import mock
 
 import orjson
+import time_machine
 from django.utils.timezone import now as timezone_now
 
 from zerver.actions.message_edit import get_mentions_for_message_updates
@@ -18,6 +19,7 @@ from zerver.lib.message import messages_for_ids
 from zerver.lib.message_cache import MessageDict
 from zerver.lib.test_classes import ZulipTestCase
 from zerver.lib.test_helpers import most_recent_message, queries_captured
+from zerver.lib.timestamp import datetime_to_timestamp
 from zerver.lib.topic import TOPIC_NAME
 from zerver.lib.utils import assert_is_not_none
 from zerver.models import Attachment, Message, NamedUserGroup, Realm, UserProfile, UserTopic
@@ -543,13 +545,14 @@ class EditMessageTest(ZulipTestCase):
 
         # Now verify that if we fetch the message directly, there's no
         # edit history data attached.
-        messages_result = self.client_get(
+        message_fetch_result = self.client_get(
-            "/json/messages", {"anchor": msg_id_1, "num_before": 0, "num_after": 10}
+            f"/json/messages/{msg_id_1}",
         )
-        self.assert_json_success(messages_result)
+        self.assert_json_success(message_fetch_result)
-        json_messages = orjson.loads(messages_result.content)
+        message_dict = orjson.loads(message_fetch_result.content)["message"]
-        for msg in json_messages["messages"]:
+        self.assertNotIn("edit_history", message_dict)
-            self.assertNotIn("edit_history", msg)
+        # We still have a last edit timestamp present.
+        self.assertIn("last_edit_timestamp", message_dict)
 
     def test_edit_message_history(self) -> None:
         self.login("hamlet")
@@ -743,6 +746,91 @@ class EditMessageTest(ZulipTestCase):
         mention_user_ids = get_mentions_for_message_updates(message)
         self.assertEqual(mention_user_ids, {cordelia.id})
 
+    def test_edit_and_moved_timestamps(self) -> None:
+        self.login("hamlet")
+        hamlet = self.example_user("hamlet")
+        stream_1 = self.make_stream("stream 1")
+        stream_2 = self.make_stream("stream 2")
+        self.subscribe(hamlet, stream_1.name)
+        self.subscribe(hamlet, stream_2.name)
+        time_zero = timezone_now().replace(microsecond=0)
+
+        with time_machine.travel(time_zero, tick=False):
+            first_message_id = self.send_stream_message(
+                self.example_user("hamlet"),
+                "stream 1",
+                topic_name="topic 1",
+                content="content 1",
+            )
+
+        first_message_edit_time = time_zero + timedelta(seconds=1)
+        with time_machine.travel(first_message_edit_time, tick=False):
+            result = self.client_patch(
+                f"/json/messages/{first_message_id}",
+                {
+                    "content": "content 2",
+                },
+            )
+            self.assert_json_success(result)
+            message_fetch_result = self.client_get(
+                f"/json/messages/{first_message_id}",
+            )
+            self.assert_json_success(message_fetch_result)
+            message_dict = orjson.loads(message_fetch_result.content)["message"]
+            self.assertEqual(
+                message_dict["last_edit_timestamp"], datetime_to_timestamp(first_message_edit_time)
+            )
+            self.assertNotIn("last_moved_timestamp", message_dict)
+
+        first_message_move_time = time_zero + timedelta(seconds=2)
+        with time_machine.travel(first_message_move_time, tick=False):
+            result = self.client_patch(
+                f"/json/messages/{first_message_id}",
+                {
+                    "topic": "topic 2",
+                },
+            )
+            self.assert_json_success(result)
+            message_fetch_result = self.client_get(
+                f"/json/messages/{first_message_id}",
+            )
+            self.assert_json_success(message_fetch_result)
+            message_dict = orjson.loads(message_fetch_result.content)["message"]
+            self.assertEqual(
+                message_dict["last_edit_timestamp"], datetime_to_timestamp(first_message_edit_time)
+            )
+            self.assertEqual(
+                message_dict["last_moved_timestamp"], datetime_to_timestamp(first_message_move_time)
+            )
+
+        with time_machine.travel(time_zero, tick=False):
+            second_message_id = self.send_stream_message(
+                self.example_user("hamlet"),
+                "stream 2",
+                topic_name="topic 2",
+                content="content 2",
+            )
+
+        second_message_move_time = time_zero + timedelta(minutes=1)
+        with time_machine.travel(second_message_move_time, tick=False):
+            result = self.client_patch(
+                f"/json/messages/{second_message_id}",
+                {
+                    "stream_id": stream_1.id,
+                },
+            )
+            message_fetch_result = self.client_get(
+                f"/json/messages/{second_message_id}",
+            )
+            self.assert_json_success(message_fetch_result)
+            message_dict = orjson.loads(message_fetch_result.content)["message"]
+            self.assert_json_success(result)
+            self.assertNotIn("last_edit_timestamp", message_dict)
+            self.assertEqual(
+                message_dict["last_moved_timestamp"],
+                datetime_to_timestamp(second_message_move_time),
+            )
+
     def test_edit_cases(self) -> None:
         """This test verifies the accuracy of construction of Zulip's edit
         history data structures."""

zerver/tests/test_message_move_topic.py

@@ -22,6 +22,7 @@ from zerver.actions.realm_settings import (
 from zerver.actions.user_topics import do_set_user_topic_visibility_policy
 from zerver.lib.message import truncate_topic
 from zerver.lib.test_classes import ZulipTestCase, get_topic_messages
+from zerver.lib.timestamp import datetime_to_timestamp
 from zerver.lib.topic import RESOLVED_TOPIC_PREFIX, messages_for_topic
 from zerver.lib.types import StreamMessageEditRequest
 from zerver.lib.user_topics import (
@@ -1471,6 +1472,14 @@ class MessageMoveTopicTest(ZulipTestCase):
             f"@_**{user_profile.full_name}|{user_profile.id}** has marked this topic as resolved.",
         )
 
+        message_fetch_result = self.client_get(
+            f"/json/messages/{msg_id}",
+        )
+        self.assert_json_success(message_fetch_result)
+        message_dict = orjson.loads(message_fetch_result.content)["message"]
+        self.assert_json_success(result)
+        self.assertNotIn("last_moved_timestamp", message_dict)
+
         # Note that we are removing the prefix from the already truncated topic,
         # so unresolved_topic_name will not be the same as the original topic_name
         unresolved_topic_name = new_topic_name.replace(RESOLVED_TOPIC_PREFIX, "")
@@ -1490,6 +1499,14 @@ class MessageMoveTopicTest(ZulipTestCase):
             f"@_**{user_profile.full_name}|{user_profile.id}** has marked this topic as unresolved.",
         )
 
+        message_fetch_result = self.client_get(
+            f"/json/messages/{msg_id}",
+        )
+        self.assert_json_success(message_fetch_result)
+        message_dict = orjson.loads(message_fetch_result.content)["message"]
+        self.assert_json_success(result)
+        self.assertNotIn("last_moved_timestamp", message_dict)
+
     def test_notify_resolve_and_move_topic(self) -> None:
         user_profile = self.example_user("hamlet")
         self.login("hamlet")
@@ -1498,8 +1515,14 @@ class MessageMoveTopicTest(ZulipTestCase):
         self.subscribe(user_profile, stream.name)
 
         # Resolve a topic normally first
-        msg_id = self.send_stream_message(user_profile, stream.name, "foo", topic_name=topic_name)
+        time_zero = timezone_now().replace(microsecond=0)
+        with time_machine.travel(time_zero, tick=False):
+            msg_id = self.send_stream_message(
+                user_profile, stream.name, "foo", topic_name=topic_name
+            )
         resolved_topic_name = RESOLVED_TOPIC_PREFIX + topic_name
+        first_message_move_time = time_zero + timedelta(seconds=2)
+        with time_machine.travel(first_message_move_time, tick=False):
             result = self.client_patch(
                 "/json/messages/" + str(msg_id),
                 {
@@ -1516,8 +1539,18 @@ class MessageMoveTopicTest(ZulipTestCase):
             f"@_**{user_profile.full_name}|{user_profile.id}** has marked this topic as resolved.",
         )
 
+        message_fetch_result = self.client_get(
+            f"/json/messages/{msg_id}",
+        )
+        self.assert_json_success(message_fetch_result)
+        message_dict = orjson.loads(message_fetch_result.content)["message"]
+        self.assert_json_success(result)
+        self.assertNotIn("last_moved_timestamp", message_dict)
+
         # Test unresolving a topic while moving it (✔ test -> bar)
         new_topic_name = "bar"
+        second_message_move_time = time_zero + timedelta(seconds=4)
+        with time_machine.travel(second_message_move_time, tick=False):
             result = self.client_patch(
                 "/json/messages/" + str(msg_id),
                 {
@@ -1533,8 +1566,21 @@ class MessageMoveTopicTest(ZulipTestCase):
             f"This topic was moved here from #**public stream>✔ test** by @_**{user_profile.full_name}|{user_profile.id}**.",
         )
 
+        message_fetch_result = self.client_get(
+            f"/json/messages/{msg_id}",
+        )
+        self.assert_json_success(message_fetch_result)
+        message_dict = orjson.loads(message_fetch_result.content)["message"]
+        self.assert_json_success(result)
+        self.assertEqual(
+            message_dict["last_moved_timestamp"],
+            datetime_to_timestamp(second_message_move_time),
+        )
+
         # Now test moving the topic while also resolving it (bar -> ✔ baz)
         new_resolved_topic_name = RESOLVED_TOPIC_PREFIX + "baz"
+        third_message_move_time = time_zero + timedelta(seconds=6)
+        with time_machine.travel(third_message_move_time, tick=False):
             result = self.client_patch(
                 "/json/messages/" + str(msg_id),
                 {
@@ -1550,6 +1596,17 @@ class MessageMoveTopicTest(ZulipTestCase):
             f"This topic was moved here from #**public stream>{new_topic_name}** by @_**{user_profile.full_name}|{user_profile.id}**.",
         )
 
+        message_fetch_result = self.client_get(
+            f"/json/messages/{msg_id}",
+        )
+        self.assert_json_success(message_fetch_result)
+        message_dict = orjson.loads(message_fetch_result.content)["message"]
+        self.assert_json_success(result)
+        self.assertEqual(
+            message_dict["last_moved_timestamp"],
+            datetime_to_timestamp(third_message_move_time),
+        )
+
     def test_mark_topic_as_resolved(self) -> None:
         self.login("iago")
         admin_user = self.example_user("iago")