From 82837304ec70a6f7742fa84d350f6451a3fc2957 Mon Sep 17 00:00:00 2001 From: Aman Agrawal Date: Wed, 2 Mar 2022 04:33:20 +0000 Subject: [PATCH] api: Send full message in GET /messages/{message_id} response. Previously, this URL just returned the `raw_content` field. It seems cleanest to just make it a single-message variant of GET /messages, deprecating the only format. --- templates/zerver/api/changelog.md | 6 + .../zerver/help/include/rest-endpoints.md | 2 +- version.py | 2 +- zerver/openapi/zulip.yaml | 119 ++++++++++++++++-- zerver/tests/test_message_edit.py | 47 ++++++- zerver/views/message_edit.py | 31 ++++- 6 files changed, 191 insertions(+), 16 deletions(-) diff --git a/templates/zerver/api/changelog.md b/templates/zerver/api/changelog.md index 8390f7a73c..50726fe44e 100644 --- a/templates/zerver/api/changelog.md +++ b/templates/zerver/api/changelog.md @@ -20,6 +20,12 @@ format used by the Zulip server that they are interacting with. ## Changes in Zulip 5.0 +**Feature level 120** + +* [`GET /messages/{message_id}`](/api/get-message): This endpoint + now sends the full message details. Previously, it only returned + the message's raw Markdown content. + **Feature level 119** * [`POST /register`](/api/register-queue): The `unread_msgs` section diff --git a/templates/zerver/help/include/rest-endpoints.md b/templates/zerver/help/include/rest-endpoints.md index 9353c8e439..e93f5a1800 100644 --- a/templates/zerver/help/include/rest-endpoints.md +++ b/templates/zerver/help/include/rest-endpoints.md @@ -9,7 +9,7 @@ * [Add an emoji reaction](/api/add-reaction) * [Remove an emoji reaction](/api/remove-reaction) * [Render a message](/api/render-message) -* [Get a message's raw Markdown](/api/get-raw-message) +* [Fetch a single message](/api/get-message) * [Check if messages match narrow](/api/check-messages-match-narrow) * [Get a message's edit history](/api/get-message-history) * [Update personal message flags](/api/update-message-flags) diff --git a/version.py b/version.py index 8f459080b6..1621c00c83 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 = 119 +API_FEATURE_LEVEL = 120 # 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/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 94694049ed..8f1ef4a6c9 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -5524,20 +5524,37 @@ paths: } /messages/{message_id}: get: - operationId: get-raw-message - summary: Get a message's raw Markdown + operationId: get-message + summary: Fetch a single message. tags: ["messages"] description: | - Get the raw content of a message. + Given a message ID, return the message object. `GET {{ api_url }}/v1/messages/{msg_id}` - This is a rarely-used endpoint relevant for clients that primarily - work with HTML-rendered messages but might need to occasionally fetch - the message's raw Markdown (e.g. for pre-filling a message-editing - UI). + Additionally, a `raw_content` field is included. This field is + useful for clients that primarily work with HTML-rendered + messages but might need to occasionally fetch the message's + raw Markdown (e.g. for [view + source](/help/view-the-markdown-source-of-a-message) or + prefilling a message edit textarea). + + **Changes**: Before Zulip 5.0 (feature level 120), this + endpoint only returned the `raw_content` field. parameters: - $ref: "#/components/parameters/MessageId" + - name: apply_markdown + in: query + description: | + If `true`, message content is returned in the rendered HTML + format. If `false`, message content is returned in the raw + Markdown-format text that user entered. + + **Changes**: New in Zulip 5.0 (feature level 120). + schema: + type: boolean + default: true + example: false responses: "200": description: Success. @@ -5553,13 +5570,99 @@ paths: msg: {} raw_content: type: string + deprecated: true description: | - The raw content of the message. + The raw Markdown content of the message. + + **Deprecated** and to be removed once no longer required for + legacy clients. Modern clients should prefer passing + `apply_markdown=false` to request raw message content. + message: + description: | + An object containing details of the message. + + **Changes**: New in Zulip 5.0 (feature level 120). + allOf: + - $ref: "#/components/schemas/MessagesBase" + - additionalProperties: false + properties: + avatar_url: + nullable: true + client: {} + content: {} + content_type: {} + display_recipient: {} + edit_history: {} + id: {} + is_me_message: {} + last_edit_timestamp: {} + reactions: {} + recipient_id: {} + sender_email: {} + sender_full_name: {} + sender_id: {} + sender_realm_str: {} + stream_id: {} + subject: {} + submessages: {} + timestamp: {} + topic_links: {} + type: {} + flags: + type: array + description: | + The user's [message flags][message-flags] for the message. + + [message-flags]: /api/update-message-flags#available-flags + items: + type: string example: { "raw_content": "**Don't** forget your towel!", "result": "success", "msg": "", + "message": + { + "subject": "", + "sender_realm_str": "zulip", + "type": "private", + "content": "

Security experts agree that relational algorithms are an interesting new topic in the field of networking, and scholars concur.

", + "flags": ["read"], + "id": 16, + "display_recipient": + [ + { + "id": 4, + "is_mirror_dummy": false, + "email": "hamlet@zulip.com", + "full_name": "King Hamlet", + }, + { + "id": 5, + "is_mirror_dummy": false, + "email": "iago@zulip.com", + "full_name": "Iago", + }, + { + "id": 8, + "is_mirror_dummy": false, + "email": "prospero@zulip.com", + "full_name": "Prospero from The Tempest", + }, + ], + "content_type": "text/html", + "is_me_message": false, + "timestamp": 1527921326, + "sender_id": 4, + "sender_full_name": "King Hamlet", + "recipient_id": 27, + "topic_links": [], + "client": "populate_db", + "avatar_url": "https://secure.gravatar.com/avatar/6d8cad0fd00256e7b40691d27ddfd466?d=identicon&version=1", + "submessages": [], + "sender_email": "hamlet@zulip.com", + "reactions": [], + }, } "400": description: Bad request. diff --git a/zerver/tests/test_message_edit.py b/zerver/tests/test_message_edit.py index 7c8ebc1235..b6199e57f8 100644 --- a/zerver/tests/test_message_edit.py +++ b/zerver/tests/test_message_edit.py @@ -304,18 +304,57 @@ class EditMessageTest(EditMessageTestCase): self.assert_json_success(result) self.check_topic(msg_id, topic_name="edited") - def test_fetch_raw_message(self) -> None: + def test_fetch_message_from_id(self) -> None: self.login("hamlet") msg_id = self.send_personal_message( from_user=self.example_user("hamlet"), to_user=self.example_user("cordelia"), - content="**before** edit", + content="Personal message", ) - result = self.client_get(f"/json/messages/{msg_id}") + result = self.client_get("/json/messages/" + str(msg_id)) self.assert_json_success(result) - self.assertEqual(result.json()["raw_content"], "**before** edit") + self.assertEqual(result.json()["raw_content"], "Personal message") + self.assertEqual(result.json()["message"]["id"], msg_id) + + # Send message to web public stream where hamlet is not subscribed. + # This will test case of user having no `UserMessage` but having access + # to message. + web_public_stream = self.make_stream("web-public-stream", is_web_public=True) + self.subscribe(self.example_user("cordelia"), web_public_stream.name) + web_public_stream_msg_id = self.send_stream_message( + self.example_user("cordelia"), web_public_stream.name, content="web-public message" + ) + result = self.client_get("/json/messages/" + str(web_public_stream_msg_id)) + self.assert_json_success(result) + self.assertEqual(result.json()["raw_content"], "web-public message") + self.assertEqual(result.json()["message"]["id"], web_public_stream_msg_id) + + # Spectator should be able to fetch message in web public stream. + self.logout() + result = self.client_get("/json/messages/" + str(web_public_stream_msg_id)) + self.assert_json_success(result) + self.assertEqual(result.json()["raw_content"], "web-public message") + self.assertEqual(result.json()["message"]["id"], web_public_stream_msg_id) + + # Verify default is apply_markdown=True + self.assertEqual(result.json()["message"]["content"], "

web-public message

") + + # Verify apply_markdown=False works correctly. + result = self.client_get( + "/json/messages/" + str(web_public_stream_msg_id), {"apply_markdown": "false"} + ) + self.assert_json_success(result) + self.assertEqual(result.json()["raw_content"], "web-public message") + self.assertEqual(result.json()["message"]["content"], "web-public message") + + with self.settings(WEB_PUBLIC_STREAMS_ENABLED=False): + result = self.client_get("/json/messages/" + str(web_public_stream_msg_id)) + self.assert_json_error( + result, "Not logged in: API authentication or user session required", status_code=401 + ) # Test error cases + self.login("hamlet") result = self.client_get("/json/messages/999999") self.assert_json_error(result, "Invalid message(s)") diff --git a/zerver/views/message_edit.py b/zerver/views/message_edit.py index ef136a5894..d4743ef580 100644 --- a/zerver/views/message_edit.py +++ b/zerver/views/message_edit.py @@ -12,7 +12,7 @@ from zerver.context_processors import get_valid_realm_from_request from zerver.lib.actions import check_update_message, do_delete_messages from zerver.lib.exceptions import JsonableError from zerver.lib.html_diff import highlight_html_differences -from zerver.lib.message import access_message, access_web_public_message +from zerver.lib.message import access_message, access_web_public_message, messages_for_ids 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 @@ -190,6 +190,7 @@ def json_fetch_raw_message( request: HttpRequest, maybe_user_profile: Union[UserProfile, AnonymousUser], message_id: int = REQ(converter=to_non_negative_int, path_only=True), + apply_markdown: bool = REQ(json_validator=check_bool, default=True), ) -> HttpResponse: if not maybe_user_profile.is_authenticated: @@ -198,4 +199,30 @@ def json_fetch_raw_message( else: (message, user_message) = access_message(maybe_user_profile, message_id) - return json_success(request, data={"raw_content": message.content}) + flags = ["read"] + if not maybe_user_profile.is_authenticated: + allow_edit_history = realm.allow_edit_history + else: + if user_message: + flags = user_message.flags_list() + allow_edit_history = maybe_user_profile.realm.allow_edit_history + + # Security note: It's important that we call this only with a + # message already fetched via `access_message` type methods, + # as we do above. + message_dict_list = messages_for_ids( + message_ids=[message.id], + user_message_flags={message_id: flags}, + search_fields={}, + apply_markdown=apply_markdown, + client_gravatar=True, + allow_edit_history=allow_edit_history, + ) + response = dict( + message=message_dict_list[0], + # raw_content is deprecated; we will need to wait until + # clients have been fully migrated to using the modern API + # before removing this, probably in 2023. + raw_content=message.content, + ) + return json_success(request, response)