From e23f3ac89b6674edcb7b4ccc4b87481ee742fa10 Mon Sep 17 00:00:00 2001 From: Prakhar Pratyush Date: Sat, 7 Oct 2023 12:52:12 +0530 Subject: [PATCH] typing_indicator: Update the 'to' parameter to accept stream ID. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the `type` of the message being composed is "stream", this commit updates the `to` parameter to accept the ID of the stream in which the message is being typed. Earlier, it accepted a single-element list containing the ID of the stream. Sending the element instead of a list containing the single element makes more sense. --- api_docs/changelog.md | 8 ++++ zerver/openapi/python_examples.py | 4 +- zerver/openapi/zulip.yaml | 26 +++++++++---- zerver/tests/test_typing.py | 64 +++++++++++++++---------------- zerver/views/typing.py | 23 +++++------ 5 files changed, 71 insertions(+), 54 deletions(-) diff --git a/api_docs/changelog.md b/api_docs/changelog.md index 30d97b5d2e..030ce16bf7 100644 --- a/api_docs/changelog.md +++ b/api_docs/changelog.md @@ -29,6 +29,14 @@ 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. + +* Note that stream typing notifications were not enabled in any Zulip client + prior to feature level 215. + **Feature level 214** * [`PATCH /realm/user_settings_defaults`](/api/update-realm-user-settings-defaults), diff --git a/zerver/openapi/python_examples.py b/zerver/openapi/python_examples.py index 161c065237..9b1bb13953 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], + "to": 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], + "to": stream_id, "topic": topic, } result = client.set_typing_status(request) diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index a56eea463e..73a584b15a 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -16630,7 +16630,11 @@ paths: See the subsystems documentation on [typing indicators][typing-protocol-docs] for additional design details on Zulip's typing notifications protocol. - **Changes**: Support for displaying stream typing notifications was new + **Changes**: Clients shouldn't care about the APIs prior to Zulip 8.0 (feature level 215) + for stream typing notifications, as no client actually implemented + the previous API for those. + + Support for displaying stream typing notifications was new in Zulip 4.0 (feature level 58). Clients should indicate they support processing stream typing notifications via the `stream_typing_notifications` value in the `client_capabilities` parameter of the @@ -16689,10 +16693,15 @@ paths: being typed. Send a JSON-encoded list of user IDs. (Use a list even if there is only one recipient.) - For `"stream"` type it is a single element list containing ID of stream in - which the message is being typed. + For `"stream"` type it is the ID of the stream in which the message is + being typed. - **Changes**: Support for typing notifications for stream messages + **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. + + Support for typing notifications for stream messages is new in Zulip 4.0 (feature level 58). Previously, typing notifications were only for direct messages. @@ -16702,9 +16711,12 @@ paths: content: application/json: schema: - type: array - items: - type: integer + oneOf: + - type: integer + - type: array + items: + type: integer + minLength: 1 example: [9, 10] required: true - name: topic diff --git a/zerver/tests/test_typing.py b/zerver/tests/test_typing.py index 3defc455b7..9577e40e9a 100644 --- a/zerver/tests/test_typing.py +++ b/zerver/tests/test_typing.py @@ -50,6 +50,33 @@ class TypingMessagetypeTest(ZulipTestCase): class TypingValidateToArgumentsTest(ZulipTestCase): + def test_invalid_to_for_direct_messages(self) -> None: + """ + Sending dms typing notifications without 'to' as a list fails. + """ + 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") + + def test_invalid_user_id_for_direct_messages(self) -> None: + """ + Sending dms typing notifications with invalid user_id fails. + """ + 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") + def test_empty_to_array_direct_messages(self) -> None: """ Sending dms typing notification without recipient fails @@ -58,16 +85,6 @@ class TypingValidateToArgumentsTest(ZulipTestCase): result = self.api_post(sender, "/api/v1/typing", {"op": "start", "to": "[]"}) self.assert_json_error(result, "Empty 'to' list") - def test_empty_to_array_stream(self) -> None: - """ - Sending stream typing notification without recipient fails - """ - sender = self.example_user("hamlet") - result = self.api_post( - sender, "/api/v1/typing", {"type": "stream", "op": "start", "to": "[]"} - ) - self.assert_json_error(result, "Empty 'to' list") - def test_missing_recipient(self) -> None: """ Sending typing notification without recipient fails @@ -76,15 +93,6 @@ class TypingValidateToArgumentsTest(ZulipTestCase): result = self.api_post(sender, "/api/v1/typing", {"op": "start"}) self.assert_json_error(result, "Missing 'to' argument") - def test_argument_to_is_not_valid_json(self) -> None: - """ - Sending typing notification to invalid recipient fails - """ - sender = self.example_user("hamlet") - invalid = "bad email" - result = self.api_post(sender, "/api/v1/typing", {"op": "start", "to": invalid}) - self.assert_json_error(result, 'Argument "to" is not valid JSON.') - def test_bogus_user_id(self) -> None: """ Sending typing notification to invalid recipient fails @@ -94,14 +102,6 @@ class TypingValidateToArgumentsTest(ZulipTestCase): result = self.api_post(sender, "/api/v1/typing", {"op": "start", "to": invalid}) self.assert_json_error(result, "Invalid user ID 9999999") - def test_send_multiple_stream_ids(self) -> None: - sender = self.example_user("hamlet") - - result = self.api_post( - sender, "/api/v1/typing", {"type": "stream", "op": "stop", "to": "[1, 2, 3]"} - ) - self.assert_json_error(result, "Cannot send to multiple streams") - def test_includes_stream_id_but_not_topic(self) -> None: sender = self.example_user("hamlet") stream_id = self.get_stream_id("general") @@ -109,7 +109,7 @@ class TypingValidateToArgumentsTest(ZulipTestCase): result = self.api_post( sender, "/api/v1/typing", - {"type": "stream", "op": "start", "to": orjson.dumps([stream_id]).decode()}, + {"type": "stream", "op": "start", "to": str(stream_id)}, ) self.assert_json_error(result, "Missing topic") @@ -124,7 +124,7 @@ class TypingValidateToArgumentsTest(ZulipTestCase): { "type": "stream", "op": "start", - "to": orjson.dumps([stream_id]).decode(), + "to": str(stream_id), "topic": topic, }, ) @@ -364,7 +364,7 @@ class TypingHappyPathTestStreams(ZulipTestCase): params = dict( type="stream", op="start", - to=orjson.dumps([stream_id]).decode(), + to=str(stream_id), topic=topic, ) @@ -398,7 +398,7 @@ class TypingHappyPathTestStreams(ZulipTestCase): params = dict( type="stream", op="stop", - to=orjson.dumps([stream_id]).decode(), + to=str(stream_id), topic=topic, ) @@ -467,7 +467,7 @@ class TestSendTypingNotificationsSettings(ZulipTestCase): params = dict( type="stream", op="start", - to=orjson.dumps([stream_id]).decode(), + to=str(stream_id), topic=topic, ) diff --git a/zerver/views/typing.py b/zerver/views/typing.py index e8ff7ce947..51c78efe0a 100644 --- a/zerver/views/typing.py +++ b/zerver/views/typing.py @@ -1,14 +1,15 @@ -from typing import List, Optional +from typing import 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_int, check_list, check_string_in +from zerver.lib.validator import check_string_in from zerver.models import UserProfile VALID_OPERATOR_TYPES = ["start", "stop"] @@ -23,36 +24,32 @@ 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: List[int] = REQ("to", json_validator=check_list(check_int)), + notification_to: str = REQ("to"), topic: Optional[str] = REQ("topic", default=None), ) -> HttpResponse: - to_length = len(notification_to) - - if to_length == 0: - raise JsonableError(_("Empty 'to' list")) - recipient_type_name = req_type if recipient_type_name == "stream": - if to_length > 1: - raise JsonableError(_("Cannot send to multiple streams")) - + stream_id = extract_stream_id(notification_to) if topic is None: raise JsonableError(_("Missing topic")) if not user_profile.send_stream_typing_notifications: raise JsonableError(_("User has disabled typing notifications for stream messages")) - stream_id = notification_to[0] # Verify that the user has access to the stream and has # permission to send messages to it. stream = access_stream_by_id(user_profile, stream_id)[0] 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) + to_length = len(user_ids) + if to_length == 0: + raise JsonableError(_("Empty 'to' list")) + if not user_profile.send_private_typing_notifications: raise JsonableError(_("User has disabled typing notifications for direct messages")) - user_ids = notification_to check_send_typing_notification(user_profile, user_ids, operator) return json_success(request)