api: Add additional fields to `edit_history` entries.

Since we've changed the database to contain these new fields, we just
need to stop dropping them in the API code.

This also changes the public API to match the database format again
by removing `prev_subject` from edit history API.

Adds an API changelog feature update for the renamed `prev_subject`
field (to `prev_topic`) and new fields (`topic` and `stream`)
in the message `edit_history`.

Also, documents said `edit_history` in the `MessagesBase` schema
in the api documentation, which is used by the `/get-messages`,
`/get-events` and `/zulip-outgoing-webhooks` endpoints.

Fixes #21076.

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

templates/zerver/api/changelog.md

@@ -20,6 +20,21 @@ format used by the Zulip server that they are interacting with.
 
 ## Changes in Zulip 5.0
 
+**Feature level 118**
+
+* [`GET /messages`](/api/get-messages), [`GET
+  /events`](/api/get-events): Improved the format of the
+  `edit_history` object within message objects. Entries for stream
+  edits now include a both a `prev_stream` and `stream` field to
+  indicate the previous and current stream IDs. Entries for topic
+  edits now include both a `prev_topic` and `topic` field to indicate
+  the previous and current topic, replacing the `prev_subject`
+  field. These changes substantially simplify client complexity for
+  processing historical message edits.
+
+* [`GET messages/{message_id}/history`](/api/get-message-history):
+  Added `stream` field to message history `snapshot` indicating
+  the updated stream ID of messages moved to a new stream.
 
 **Feature level 117**
 

version.py

@@ -33,7 +33,7 @@ DESKTOP_WARNING_VERSION = "5.4.3"
 # Changes should be accompanied by documentation explaining what the
 # new level means in templates/zerver/api/changelog.md, as well as
 # "**Changes**" entries in the endpoint's documentation in `zulip.yaml`.
-API_FEATURE_LEVEL = 117
+API_FEATURE_LEVEL = 118
 
 # 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

@@ -38,12 +38,7 @@ from zerver.lib.streams import get_web_public_streams_queryset
 from zerver.lib.timestamp import datetime_to_timestamp
 from zerver.lib.topic import DB_TOPIC_NAME, MESSAGE__TOPIC, TOPIC_LINKS, TOPIC_NAME
 from zerver.lib.topic_mutes import build_topic_mute_checker, topic_is_muted
-from zerver.lib.types import (
+from zerver.lib.types import DisplayRecipientT, EditHistoryEvent, UserDisplayRecipient
-    APIEditHistoryEvent,
-    DisplayRecipientT,
-    EditHistoryEvent,
-    UserDisplayRecipient,
-)
 from zerver.models import (
     MAX_TOPIC_NAME_LENGTH,
     Message,
@@ -507,25 +502,7 @@ class MessageDict:
         if last_edit_time is not None:
             obj["last_edit_timestamp"] = datetime_to_timestamp(last_edit_time)
             assert edit_history_json is not None
-            raw_edit_history: List[EditHistoryEvent] = orjson.loads(edit_history_json)
+            edit_history: List[EditHistoryEvent] = orjson.loads(edit_history_json)
-            edit_history: List[APIEditHistoryEvent] = []
-            for edit_history_event in raw_edit_history:
-                # Drop fields we're not yet ready to have appear in the API
-                if "prev_topic" in edit_history_event:
-                    # The prev_subject field has been renamed in the
-                    # database, but not the API.
-                    edit_history_event["prev_subject"] = edit_history_event["prev_topic"]  # type: ignore # Temporary type-checking violation
-
-                # New fields not consistently available, and thus
-                # intentionally not exposed to the API.
-                if "prev_topic" in edit_history_event:
-                    del edit_history_event["prev_topic"]
-                if "stream" in edit_history_event:
-                    del edit_history_event["stream"]
-                if "topic" in edit_history_event:
-                    del edit_history_event["topic"]
-
-                edit_history.append(edit_history_event)  # type: ignore # Temporary type-checking violation
             obj["edit_history"] = edit_history
 
         if Message.need_to_render_content(

zerver/lib/types.py

@@ -91,32 +91,11 @@ class UnspecifiedValue:
     pass
 
 
-class APIEditHistoryEvent(TypedDict, total=False):
-    """Format of legacy edit history events in the API. Contains legacy
-    fields like LEGACY_PREV_TOPIC that we intend to remove from the
-    API eventually.
-    """
-
-    # Commented fields are fields we plan to add.
-    user_id: Optional[int]
-    timestamp: int
-    prev_stream: int
-    # stream: int
-    # TODO: Remove prev_subject from the API.
-    prev_subject: str
-    # prev_topic: str
-    # topic: str
-    prev_content: str
-    prev_rendered_content: Optional[str]
-    prev_rendered_content_version: Optional[int]
-
-
 class EditHistoryEvent(TypedDict, total=False):
     """
     Database format for edit history events.
     """
 
-    # Commented fields are fields we plan to add.
     user_id: Optional[int]
     timestamp: int
     prev_stream: int

zerver/openapi/zulip.yaml

@@ -4963,9 +4963,8 @@ paths:
         `topic`, `content`, `rendered_content`, `timestamp` and `user_id`. This
         snapshot will be the only one present if the message has never been edited.
 
-        Also note that if a message's content was edited (but not the topic)
+        Also note that each snapshot object will only contain additional data for the
-        or the topic was edited (but not the content), the snapshot object
+        modified fields for that particular edit (e.g. if only the topic or stream
-        will only contain data for the modified fields (e.g. if only the topic
         was edited, `prev_content`, `prev_rendered_content`, and
         `content_html_diff` will not appear).
       responses:
@@ -4990,51 +4989,80 @@ paths:
                             topic:
                               type: string
                               description: |
-                                the topic for the message.
+                                The topic of the message immediately
+                                after this edit event.
                             prev_topic:
                               type: string
                               description: |
-                                the topic for the message before being edited.
+                                Only present if message's topic was edited.
+
+                                The topic of the message immediately
+                                prior to this edit event.
+                            stream:
+                              type: integer
+                              description: |
+                                Only present if message's stream was edited.
+
+                                The ID of the stream containing the message
+                                immediately after this edit event.
+
+                                **Changes**: New in Zulip 5.0 (feature level 118).
                             prev_stream:
                               type: integer
                               description: |
                                 Only present if message's stream was edited.
 
-                                The stream ID of the message prior to being edited.
+                                The ID of the stream containing the message immediately
+                                prior to this edit event.
                             content:
                               type: string
                               description: |
-                                the body of the message.
+                                The raw Markdown content of the message
+                                immediately after this edit event.
                             rendered_content:
                               type: string
                               description: |
-                                the already rendered, HTML version of `content`.
+                                The rendered HTML representation of `content`.
                             prev_content:
                               type: string
                               description: |
-                                the body of the message before being edited.
+                                Only present if message's content was edited.
+
+                                The raw Markdown content of the message immediately
+                                prior to this edit event.
                             prev_rendered_content:
                               type: string
                               description: |
-                                the already rendered, HTML version of
+                                Only present if message's content was edited.
-                                `prev_content`.
+
+                                The rendered HTML representation of `prev_content`.
                             user_id:
                               type: integer
+                              nullable: true
                               description: |
-                                the ID of the user that made the edit.
+                                The ID of the user that made the edit.
+
+                                Will be null only for edit history
+                                events predating March 2017.
+
+                                Clients can display edit history events where this
+                                is null as modified by either the sender (for content
+                                edits) or an unknown user (for topic edits).
                             content_html_diff:
                               type: string
                               description: |
-                                an HTML diff between this version of the message
+                                Only present if message's content was edited.
+
+                                An HTML diff between this version of the message
                                 and the previous one.
                             timestamp:
                               type: integer
                               description: |
-                                the UNIX timestamp for this edit.
+                                The UNIX timestamp for this edit.
                         description: |
-                          A chronologically sorted array of `snapshot`
+                          A chronologically sorted, oldest to newest, array
-                          objects, each one with the values of the
+                          of `snapshot` objects, each one with the values of
-                          message after the edit.
+                          the message after the edit.
                     example:
                       {
                         "message_history":
@@ -14625,6 +14653,106 @@ components:
             Data on the recipient of the message;
             either the name of a stream or a dictionary containing basic data on
             the users who received the message.
+        edit_history:
+          type: array
+          items:
+            type: object
+            additionalProperties: false
+            properties:
+              prev_content:
+                type: string
+                description: |
+                  Only present if message's content was edited.
+
+                  The content of the message immediately prior to this
+                  edit event.
+              prev_rendered_content:
+                type: string
+                description: |
+                  Only present if message's content was edited.
+
+                  The rendered HTML representation of `prev_content`.
+              prev_rendered_content_version:
+                type: string
+                description: |
+                  Only present if message's content was edited.
+
+                  The Markdown processor version number for the message
+                  immediately prior to this edit event.
+              prev_stream:
+                type: integer
+                description: |
+                  Only present if message's stream was edited.
+
+                  The stream ID of the message immediately prior to this
+                  edit event.
+              prev_topic:
+                type: string
+                description: |
+                  Only present if message's topic was edited.
+
+                  The topic of the message immediately prior to this
+                  edit event.
+
+                  **Changes**: New in Zulip 5.0 (feature level 118).
+                  Previously, this field was called `prev_subject`;
+                  clients are recommended to rename `prev_subject` to
+                  `prev_topic` if present for compatibility with
+                  older Zulip servers.
+              stream:
+                type: integer
+                description: |
+                  Only present if message's stream was edited.
+
+                  The ID of the stream containing the message
+                  immediately after this edit event.
+
+                  **Changes**: New in Zulip 5.0 (feature level 118).
+              timestamp:
+                type: integer
+                description: |
+                  The UNIX timestamp for the edit.
+              topic:
+                type: string
+                description: |
+                  Only present if message's topic was edited.
+
+                  The topic of the message immediately after this edit event.
+
+                  **Changes**: New in Zulip 5.0 (feature level 118).
+              user_id:
+                type: integer
+                nullable: true
+                description: |
+                  The ID of the user that made the edit.
+
+                  Will be null only for edit history
+                  events predating March 2017.
+
+                  Clients can display edit history events where this
+                  is null as modified by either the sender (for content
+                  edits) or an unknown user (for topic edits).
+            required:
+              - user_id
+              - timestamp
+          description: |
+            An array of objects, with each object documenting the
+            changes made in a previous edit made to the the message,
+            ordered chronologically from most recent to least recent
+            edit.
+
+            Not present if the message has never been edited or if the realm has
+            [disabled viewing of message edit history][disable-edit-history].
+
+            Every object will contain `user_id` and `timestamp`.
+
+            The other fields are optional, and will be present or not
+            depending on which of the stream, topic, and message
+            content were modified in the edit event. For example, if
+            only the topic was edited, only `prev_topic` and `topic`
+            will be present in addition to `user_id` and `timestamp`.
+
+            [disable-edit-history]: /help/disable-message-edit-history
         id:
           type: integer
           description: |

zerver/tests/test_message_edit.py

@@ -73,12 +73,11 @@ class EditMessageTestCase(ZulipTestCase):
             msg.sender_id,
         )
 
-        # TODO: uncomment assertion when edit_history fields in API match fields in database.
+        if msg.edit_history:
-        # if msg.edit_history:
+            self.assertEqual(
-        #     self.assertEqual(
+                fetch_message_dict["edit_history"],
-        #         fetch_message_dict["edit_history"],
+                orjson.loads(msg.edit_history),
-        #         orjson.loads(msg.edit_history),
+            )
-        #     )
 
     def prepare_move_topics(
         self,
@@ -879,14 +878,13 @@ class EditMessageTest(EditMessageTestCase):
                 expected_entries.add("content_html_diff")
             if i in {0, 3}:
                 expected_entries.add("prev_stream")
-                # TODO: uncomment when stream field is added to API
+                expected_entries.add("stream")
-                # expected_entries.add("stream")
             i += 1
             self.assertEqual(expected_entries, set(entry.keys()))
         self.assert_length(message_history, 7)
         self.assertEqual(message_history[0]["topic"], "topic 4")
         self.assertEqual(message_history[0]["prev_topic"], "topic 3")
-        # self.assertEqual(message_history[0]["stream"], stream_3.id)
+        self.assertEqual(message_history[0]["stream"], stream_3.id)
         self.assertEqual(message_history[0]["prev_stream"], stream_2.id)
         self.assertEqual(message_history[0]["content"], "content 4")
 
@@ -900,7 +898,7 @@ class EditMessageTest(EditMessageTestCase):
         self.assertEqual(message_history[2]["prev_content"], "content 2")
 
         self.assertEqual(message_history[3]["topic"], "topic 2")
-        # self.assertEqual(message_history[3]["stream"], stream_2.id)
+        self.assertEqual(message_history[3]["stream"], stream_2.id)
         self.assertEqual(message_history[3]["prev_stream"], stream_1.id)
         self.assertEqual(message_history[3]["content"], "content 2")
 

zerver/views/message_edit.py

@@ -16,7 +16,7 @@ from zerver.lib.message import access_message, access_web_public_message
 from zerver.lib.request import REQ, RequestNotes, has_request_variables
 from zerver.lib.response import json_success
 from zerver.lib.timestamp import datetime_to_timestamp
-from zerver.lib.topic import LEGACY_PREV_TOPIC, REQ_topic
+from zerver.lib.topic import REQ_topic
 from zerver.lib.types import EditHistoryEvent, FormattedEditHistoryEvent
 from zerver.lib.validator import check_bool, check_string_in, to_non_negative_int
 from zerver.models import Message, UserProfile
@@ -70,7 +70,7 @@ def fill_edit_history_entries(
 
         if "prev_stream" in edit_history_event:
             formatted_entry["prev_stream"] = edit_history_event["prev_stream"]
-            # TODO: Include current stream here.
+            formatted_entry["stream"] = edit_history_event["stream"]
 
         formatted_edit_history.append(formatted_entry)