diff --git a/templates/zerver/api/changelog.md b/templates/zerver/api/changelog.md index b03a00886d..c5797a5a15 100644 --- a/templates/zerver/api/changelog.md +++ b/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** diff --git a/version.py b/version.py index 569ba4a215..18bd9cf076 100644 --- a/version.py +++ b/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 diff --git a/zerver/lib/message.py b/zerver/lib/message.py index f13b19fd49..6f0ccb542c 100644 --- a/zerver/lib/message.py +++ b/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 ( - APIEditHistoryEvent, - DisplayRecipientT, - EditHistoryEvent, - UserDisplayRecipient, -) +from zerver.lib.types import 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[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 + edit_history: List[EditHistoryEvent] = orjson.loads(edit_history_json) obj["edit_history"] = edit_history if Message.need_to_render_content( diff --git a/zerver/lib/types.py b/zerver/lib/types.py index 53fd941e7b..a765ab0adf 100644 --- a/zerver/lib/types.py +++ b/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 diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 6795c8cb21..b96d1ae5f1 100644 --- a/zerver/openapi/zulip.yaml +++ b/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) - or the topic was edited (but not the content), the snapshot object - will only contain data for the modified fields (e.g. if only the topic + Also note that each snapshot object will only contain additional data for the + modified fields for that particular edit (e.g. if only the topic or stream 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 - `prev_content`. + Only present if message's content was edited. + + 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` - objects, each one with the values of the - message after the edit. + A chronologically sorted, oldest to newest, array + of `snapshot` objects, each one with the values of + 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: | diff --git a/zerver/tests/test_message_edit.py b/zerver/tests/test_message_edit.py index 1ee93402ff..7c8ebc1235 100644 --- a/zerver/tests/test_message_edit.py +++ b/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: - # self.assertEqual( - # fetch_message_dict["edit_history"], - # orjson.loads(msg.edit_history), - # ) + if msg.edit_history: + self.assertEqual( + fetch_message_dict["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") diff --git a/zerver/views/message_edit.py b/zerver/views/message_edit.py index caa383675c..ef136a5894 100644 --- a/zerver/views/message_edit.py +++ b/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)