From c520a963540d67186253aa10a90bf28d7726f9e8 Mon Sep 17 00:00:00 2001 From: Prakhar Pratyush Date: Thu, 12 Oct 2023 12:43:34 +0530 Subject: [PATCH] typing_indicator: Add a 'stream_id' parameter to 'POST /typing'. This commit adds a 'stream_id' parameter to the 'POST /typing' endpoint. Now, 'to' is used only for "direct" type. In the case of "stream" type, 'stream_id' and 'topic' are used. --- api_docs/changelog.md | 9 +++--- web/src/typing.js | 2 +- zerver/openapi/python_examples.py | 4 +-- zerver/openapi/zulip.yaml | 39 +++++++++++++----------- zerver/tests/test_typing.py | 50 ++++++++++++++++++++----------- zerver/views/typing.py | 19 ++++++++---- 6 files changed, 76 insertions(+), 47 deletions(-) diff --git a/api_docs/changelog.md b/api_docs/changelog.md index 030ce16bf7..d15a954b74 100644 --- a/api_docs/changelog.md +++ b/api_docs/changelog.md @@ -29,10 +29,11 @@ format used by the Zulip server that they are interacting with. * [`POST /typing`](/api/set-typing-status): Stopped supporting `private` as a valid value for the `type` parameter. -* [`POST /typing`](/api/set-typing-status): When the `type` of the message - being composed is `"stream"`, changed the `to` parameter to accept the - ID of the stream in which the message is being typed. Previously, it - accepted a single-element list containing the ID of the stream. +* [`POST /typing`](/api/set-typing-status): Stopped using the `to` parameter + for the `"stream"` type. Previously, in the case of the `"stream"` type, it + accepted a single-element list containing the ID of the stream. Added an + optional parameter, `stream_id`. Now, `to` is used only for `"direct"` type. + In the case of `"stream"` type, `stream_id` and `topic` are used. * Note that stream typing notifications were not enabled in any Zulip client prior to feature level 215. diff --git a/web/src/typing.js b/web/src/typing.js index 4eee60a07f..37a0a6bebc 100644 --- a/web/src/typing.js +++ b/web/src/typing.js @@ -47,7 +47,7 @@ function send_direct_message_typing_notification(user_ids_array, operation) { function send_stream_typing_notification(stream_id, topic, operation) { const data = { type: "stream", - to: JSON.stringify(stream_id), + stream_id: JSON.stringify(stream_id), topic, op: operation, }; diff --git a/zerver/openapi/python_examples.py b/zerver/openapi/python_examples.py index 9b1bb13953..dda66ed850 100644 --- a/zerver/openapi/python_examples.py +++ b/zerver/openapi/python_examples.py @@ -1351,7 +1351,7 @@ def set_typing_status(client: Client) -> None: request = { "type": "stream", "op": "start", - "to": stream_id, + "stream_id": stream_id, "topic": topic, } result = client.set_typing_status(request) @@ -1369,7 +1369,7 @@ def set_typing_status(client: Client) -> None: request = { "type": "stream", "op": "stop", - "to": stream_id, + "stream_id": stream_id, "topic": topic, } result = client.set_typing_status(request) diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 73a584b15a..ccfdca0ed6 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -16689,17 +16689,14 @@ paths: - name: to in: query description: | - For `"direct"` type it is the user IDs of the recipients of the message - being typed. Send a JSON-encoded list of user IDs. (Use a list even if - there is only one recipient.) + User IDs of the recipients of the message being typed. Required for the + `"direct"` type. Ignored in the case of `"stream"` type. Send a JSON-encoded + list of user IDs. (Use a list even if there is only one recipient.) - For `"stream"` type it is the ID of the stream in which the message is - being typed. - - **Changes**: In Zulip 8.0 (feature level 215), for the `"stream"` `type`, - changed this parameter to accept the ID of the stream in which the message - is being typed. Previously, it accepted a single-element list containing - the ID of the stream. + **Changes**: In Zulip 8.0 (feature level 215), stopped using this parameter + for the `"stream"` type. Previously, in the case of the `"stream"` type, it + accepted a single-element list containing the ID of the stream. A new parameter, + `stream_id`, is now used for this. Support for typing notifications for stream messages is new in Zulip 4.0 (feature level 58). Previously, typing @@ -16711,14 +16708,22 @@ paths: content: application/json: schema: - oneOf: - - type: integer - - type: array - items: - type: integer - minLength: 1 + type: array + items: + type: integer + minLength: 1 example: [9, 10] - required: true + - name: stream_id + in: query + description: | + ID of the stream in which the message is being typed. Required for the `"stream"` + type. Ignored in the case of `"direct"` type. + + **Changes**: New in Zulip 8.0 (feature level 215). Previously, a single-element + list containing the ID of the stream was passed in `to` parameter. + schema: + type: integer + example: 7 - name: topic in: query description: | diff --git a/zerver/tests/test_typing.py b/zerver/tests/test_typing.py index 9577e40e9a..e59cf81903 100644 --- a/zerver/tests/test_typing.py +++ b/zerver/tests/test_typing.py @@ -56,17 +56,7 @@ class TypingValidateToArgumentsTest(ZulipTestCase): """ sender = self.example_user("hamlet") result = self.api_post(sender, "/api/v1/typing", {"op": "start", "to": "2"}) - self.assert_json_error(result, "Invalid data type for recipients") - - def test_invalid_to_for_stream_messages(self) -> None: - """ - Sending stream typing notifications without 'to' as an integer fails. - """ - sender = self.example_user("hamlet") - result = self.api_post( - sender, "/api/v1/typing", {"type": "stream", "op": "start", "to": "invalid"} - ) - self.assert_json_error(result, "Invalid data type for stream ID") + self.assert_json_error(result, "to is not a list") def test_invalid_user_id_for_direct_messages(self) -> None: """ @@ -75,7 +65,7 @@ class TypingValidateToArgumentsTest(ZulipTestCase): sender = self.example_user("hamlet") invalid_user_ids = orjson.dumps([2, "a", 4]).decode() result = self.api_post(sender, "/api/v1/typing", {"op": "start", "to": invalid_user_ids}) - self.assert_json_error(result, "Recipient list may only contain user IDs") + self.assert_json_error(result, "to[1] is not an integer") def test_empty_to_array_direct_messages(self) -> None: """ @@ -102,6 +92,32 @@ class TypingValidateToArgumentsTest(ZulipTestCase): result = self.api_post(sender, "/api/v1/typing", {"op": "start", "to": invalid}) self.assert_json_error(result, "Invalid user ID 9999999") + +class TypingValidateStreamIdTopicArgumentsTest(ZulipTestCase): + def test_missing_stream_id(self) -> None: + """ + Sending stream typing notifications without 'stream_id' fails. + """ + sender = self.example_user("hamlet") + result = self.api_post( + sender, + "/api/v1/typing", + {"type": "stream", "op": "start", "topic": "test"}, + ) + self.assert_json_error(result, "Missing stream_id") + + def test_invalid_stream_id(self) -> None: + """ + Sending stream typing notifications without 'stream_id' as an integer fails. + """ + sender = self.example_user("hamlet") + result = self.api_post( + sender, + "/api/v1/typing", + {"type": "stream", "op": "start", "stream_id": "invalid", "topic": "test"}, + ) + self.assert_json_error(result, 'Argument "stream_id" is not valid JSON.') + def test_includes_stream_id_but_not_topic(self) -> None: sender = self.example_user("hamlet") stream_id = self.get_stream_id("general") @@ -109,7 +125,7 @@ class TypingValidateToArgumentsTest(ZulipTestCase): result = self.api_post( sender, "/api/v1/typing", - {"type": "stream", "op": "start", "to": str(stream_id)}, + {"type": "stream", "op": "start", "stream_id": str(stream_id)}, ) self.assert_json_error(result, "Missing topic") @@ -124,7 +140,7 @@ class TypingValidateToArgumentsTest(ZulipTestCase): { "type": "stream", "op": "start", - "to": str(stream_id), + "stream_id": str(stream_id), "topic": topic, }, ) @@ -364,7 +380,7 @@ class TypingHappyPathTestStreams(ZulipTestCase): params = dict( type="stream", op="start", - to=str(stream_id), + stream_id=str(stream_id), topic=topic, ) @@ -398,7 +414,7 @@ class TypingHappyPathTestStreams(ZulipTestCase): params = dict( type="stream", op="stop", - to=str(stream_id), + stream_id=str(stream_id), topic=topic, ) @@ -467,7 +483,7 @@ class TestSendTypingNotificationsSettings(ZulipTestCase): params = dict( type="stream", op="start", - to=str(stream_id), + stream_id=str(stream_id), topic=topic, ) diff --git a/zerver/views/typing.py b/zerver/views/typing.py index 51c78efe0a..428f0a4ae8 100644 --- a/zerver/views/typing.py +++ b/zerver/views/typing.py @@ -1,15 +1,14 @@ -from typing import Optional +from typing import List, Optional from django.http import HttpRequest, HttpResponse from django.utils.translation import gettext as _ from zerver.actions.typing import check_send_typing_notification, do_send_stream_typing_notification from zerver.lib.exceptions import JsonableError -from zerver.lib.recipient_parsing import extract_direct_message_recipient_ids, extract_stream_id from zerver.lib.request import REQ, has_request_variables from zerver.lib.response import json_success from zerver.lib.streams import access_stream_by_id, access_stream_for_send_message -from zerver.lib.validator import check_string_in +from zerver.lib.validator import check_int, check_list, check_string_in from zerver.models import UserProfile VALID_OPERATOR_TYPES = ["start", "stop"] @@ -24,12 +23,17 @@ def send_notification_backend( "type", str_validator=check_string_in(VALID_RECIPIENT_TYPES), default="direct" ), operator: str = REQ("op", str_validator=check_string_in(VALID_OPERATOR_TYPES)), - notification_to: str = REQ("to"), + notification_to: Optional[List[int]] = REQ( + "to", json_validator=check_list(check_int), default=None + ), + stream_id: Optional[int] = REQ(json_validator=check_int, default=None), topic: Optional[str] = REQ("topic", default=None), ) -> HttpResponse: recipient_type_name = req_type if recipient_type_name == "stream": - stream_id = extract_stream_id(notification_to) + if stream_id is None: + raise JsonableError(_("Missing stream_id")) + if topic is None: raise JsonableError(_("Missing topic")) @@ -42,7 +46,10 @@ def send_notification_backend( access_stream_for_send_message(user_profile, stream, forwarder_user_profile=None) do_send_stream_typing_notification(user_profile, operator, stream, topic) else: - user_ids = extract_direct_message_recipient_ids(notification_to) + if notification_to is None: + raise JsonableError(_("Missing 'to' argument")) + + user_ids = notification_to to_length = len(user_ids) if to_length == 0: raise JsonableError(_("Empty 'to' list"))