From 17a0304309e498dcd33928b766f950508d92abd5 Mon Sep 17 00:00:00 2001 From: Prakhar Pratyush Date: Mon, 9 Oct 2023 16:41:32 +0530 Subject: [PATCH] send_message: Add an optional parameter in the success response. Add an optional `automatic_new_visibility_policy` enum field in the success response to indicate the new visibility policy value due to the `automatically_follow_topics_policy` and `automatically_unmute_topics_in_muted_streams_policy` user settings during the send message action. Only present if there is a change in the visibility policy. --- api_docs/changelog.md | 7 +++ version.py | 2 +- zerver/actions/message_send.py | 51 ++++++++++++------- zerver/actions/scheduled_messages.py | 4 +- zerver/lib/message.py | 1 + zerver/lib/onboarding.py | 4 +- zerver/lib/test_classes.py | 6 ++- zerver/openapi/zulip.yaml | 33 +++++++++++- zerver/tests/test_message_send.py | 41 +++++++++++++++ zerver/views/message_send.py | 12 +++-- .../commands/add_mock_conversation.py | 4 +- 11 files changed, 136 insertions(+), 29 deletions(-) diff --git a/api_docs/changelog.md b/api_docs/changelog.md index 58c326b6f0..d8c8eec03e 100644 --- a/api_docs/changelog.md +++ b/api_docs/changelog.md @@ -20,6 +20,13 @@ format used by the Zulip server that they are interacting with. ## Changes in Zulip 8.0 +**Feature level 218** + +* [`POST /messages`](/api/send-message): Added an optional + `automatic_new_visibility_policy` enum field in the success response + to indicate the new visibility policy value due to the [visibility policy settings](/help/mute-a-topic) + during the send message action. + **Feature level 217** * [`POST /mobile_push/test_notification`](/api/test-notify): Added new endpoint diff --git a/version.py b/version.py index 5310d05d25..f395616fed 100644 --- a/version.py +++ b/version.py @@ -33,7 +33,7 @@ DESKTOP_WARNING_VERSION = "5.9.3" # Changes should be accompanied by documentation explaining what the # new level means in api_docs/changelog.md, as well as "**Changes**" # entries in the endpoint's documentation in `zulip.yaml`. -API_FEATURE_LEVEL = 217 +API_FEATURE_LEVEL = 218 # 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/actions/message_send.py b/zerver/actions/message_send.py index 0dcacf122c..40491f6ea4 100644 --- a/zerver/actions/message_send.py +++ b/zerver/actions/message_send.py @@ -196,6 +196,12 @@ class ActiveUserDict(TypedDict): bot_type: Optional[int] +@dataclass +class SentMessageResult: + message_id: int + automatic_new_visibility_policy: Optional[int] = None + + def get_recipient_info( *, realm_id: int, @@ -843,7 +849,7 @@ def do_send_messages( email_gateway: bool = False, scheduled_message_to_self: bool = False, mark_as_read: Sequence[int] = [], -) -> List[int]: +) -> List[SentMessageResult]: """See https://zulip.readthedocs.io/en/latest/subsystems/sending-messages.html for high-level documentation on this subsystem. @@ -964,6 +970,7 @@ def do_send_messages( topic=send_request.message.topic_name(), visibility_policy=new_visibility_policy, ) + send_request.automatic_new_visibility_policy = new_visibility_policy # Deliver events to the real-time push system, as well as # enqueuing any additional processing triggered by the message. @@ -1123,7 +1130,14 @@ def do_send_messages( }, ) - return [send_request.message.id for send_request in send_message_requests] + sent_message_results = [ + SentMessageResult( + message_id=send_request.message.id, + automatic_new_visibility_policy=send_request.automatic_new_visibility_policy, + ) + for send_request in send_message_requests + ] + return sent_message_results def already_sent_mirrored_message_id(message: Message) -> Optional[int]: @@ -1236,8 +1250,8 @@ def check_send_stream_message( ) -> int: addressee = Addressee.for_stream_name(stream_name, topic) message = check_message(sender, client, addressee, body, realm) - - return do_send_messages([message])[0] + sent_message_result = do_send_messages([message])[0] + return sent_message_result.message_id def check_send_stream_message_by_id( @@ -1250,8 +1264,8 @@ def check_send_stream_message_by_id( ) -> int: addressee = Addressee.for_stream_id(stream_id, topic) message = check_message(sender, client, addressee, body, realm) - - return do_send_messages([message])[0] + sent_message_result = do_send_messages([message])[0] + return sent_message_result.message_id def check_send_private_message( @@ -1259,8 +1273,8 @@ def check_send_private_message( ) -> int: addressee = Addressee.for_user_profile(receiving_user) message = check_message(sender, client, addressee, body) - - return do_send_messages([message])[0] + sent_message_result = do_send_messages([message])[0] + return sent_message_result.message_id # check_send_message: @@ -1281,7 +1295,7 @@ def check_send_message( widget_content: Optional[str] = None, *, skip_stream_access_check: bool = False, -) -> int: +) -> SentMessageResult: addressee = Addressee.legacy_build(sender, recipient_type_name, message_to, topic_name) try: message = check_message( @@ -1299,7 +1313,7 @@ def check_send_message( skip_stream_access_check=skip_stream_access_check, ) except ZephyrMessageAlreadySentError as e: - return e.message_id + return SentMessageResult(message_id=e.message_id) return do_send_messages([message])[0] @@ -1745,8 +1759,8 @@ def internal_send_private_message( ) if message is None: return None - message_ids = do_send_messages([message]) - return message_ids[0] + sent_message_result = do_send_messages([message])[0] + return sent_message_result.message_id def internal_send_stream_message( @@ -1769,8 +1783,9 @@ def internal_send_stream_message( if message is None: return None - message_ids = do_send_messages([message]) - return message_ids[0] + + sent_message_result = do_send_messages([message])[0] + return sent_message_result.message_id def internal_send_stream_message_by_name( @@ -1790,8 +1805,8 @@ def internal_send_stream_message_by_name( if message is None: return None - message_ids = do_send_messages([message]) - return message_ids[0] + sent_message_result = do_send_messages([message])[0] + return sent_message_result.message_id def internal_send_huddle_message( @@ -1806,5 +1821,5 @@ def internal_send_huddle_message( ) if message is None: return None - message_ids = do_send_messages([message]) - return message_ids[0] + sent_message_result = do_send_messages([message])[0] + return sent_message_result.message_id diff --git a/zerver/actions/scheduled_messages.py b/zerver/actions/scheduled_messages.py index 18945a0571..4a8416b4d9 100644 --- a/zerver/actions/scheduled_messages.py +++ b/zerver/actions/scheduled_messages.py @@ -302,10 +302,10 @@ def send_scheduled_message(scheduled_message: ScheduledMessage) -> None: ) scheduled_message_to_self = scheduled_message.recipient == scheduled_message.sender.recipient - message_id = do_send_messages( + sent_message_result = do_send_messages( [send_request], scheduled_message_to_self=scheduled_message_to_self )[0] - scheduled_message.delivered_message_id = message_id + scheduled_message.delivered_message_id = sent_message_result.message_id scheduled_message.delivered = True scheduled_message.save(update_fields=["delivered", "delivered_message_id"]) notify_remove_scheduled_message(scheduled_message.sender, scheduled_message.id) diff --git a/zerver/lib/message.py b/zerver/lib/message.py index db89fb5356..2ebc5038db 100644 --- a/zerver/lib/message.py +++ b/zerver/lib/message.py @@ -208,6 +208,7 @@ class SendMessageRequest: limit_unread_user_ids: Optional[Set[int]] = None service_queue_events: Optional[Dict[str, List[Dict[str, Any]]]] = None disable_external_notifications: bool = False + automatic_new_visibility_policy: Optional[int] = None # We won't try to fetch more unread message IDs from the database than diff --git a/zerver/lib/onboarding.py b/zerver/lib/onboarding.py index d863afd0d4..a5d6ded376 100644 --- a/zerver/lib/onboarding.py +++ b/zerver/lib/onboarding.py @@ -345,7 +345,9 @@ def send_initial_realm_messages(realm: Realm) -> None: ) for message in welcome_messages ] - message_ids = do_send_messages(messages) + message_ids = [ + sent_message_result.message_id for sent_message_result in do_send_messages(messages) + ] # We find the one of our just-sent messages with turtle.png in it, # and react to it. This is a bit hacky, but works and is kinda a diff --git a/zerver/lib/test_classes.py b/zerver/lib/test_classes.py index 3e1c4fc252..38987211da 100644 --- a/zerver/lib/test_classes.py +++ b/zerver/lib/test_classes.py @@ -1071,7 +1071,7 @@ Output: recipient_list = [to_user.id] (sending_client, _) = Client.objects.get_or_create(name=sending_client_name) - return check_send_message( + sent_message_result = check_send_message( from_user, sending_client, "private", @@ -1079,6 +1079,7 @@ Output: None, content, ) + return sent_message_result.message_id def send_huddle_message( self, @@ -1092,7 +1093,7 @@ Output: (sending_client, _) = Client.objects.get_or_create(name=sending_client_name) - return check_send_message( + sent_message_result = check_send_message( from_user, sending_client, "private", @@ -1100,6 +1101,7 @@ Output: None, content, ) + return sent_message_result.message_id def send_stream_message( self, diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 28e0d0d4fc..c6d21d6c88 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -6166,6 +6166,8 @@ paths: allOf: - $ref: "#/components/schemas/JsonSuccessBase" - additionalProperties: false + required: + - id properties: result: {} msg: {} @@ -6174,7 +6176,36 @@ paths: type: integer description: | The unique ID assigned to the sent message. - example: {"msg": "", "id": 42, "result": "success"} + automatic_new_visibility_policy: + type: integer + enum: + - 2 + - 3 + description: | + If the message's sender had configured their [visibility policy settings](/help/mute-a-topic) + to potentially automatically follow or unmute topics when sending messages, + and one of these policies did in fact change the user's visibility policy + for the topic where this message was sent, the new value for that user's + visibility policy for the recipient topic. + + Only present if the sender's visibility was in fact changed. + + Intended to be used by clients where it is important to explicitly display a + notice that the topic's visibility policy has changed automatically due to + the message sent. The information is also important to correctly decide whether to + display a warning or notice suggesting to unmute the topic after sending a message + to a muted stream. Such a notice might feel confusing in the event that the act of + sending the message had already resulted in the user unmuting or following the topic + in question. + + **Changes**: New in Zulip 8.0 (feature level 218). + example: + { + "msg": "", + "id": 42, + "automatic_new_visibility_policy": 2, + "result": "success", + } "400": description: Bad request. content: diff --git a/zerver/tests/test_message_send.py b/zerver/tests/test_message_send.py index e046d9d108..6d1feb0c8d 100644 --- a/zerver/tests/test_message_send.py +++ b/zerver/tests/test_message_send.py @@ -568,6 +568,47 @@ class MessagePOSTTest(ZulipTestCase): result, "Stream '&<"'><non-existent>' does not exist" ) + def test_message_to_stream_with_automatically_change_visibility_policy(self) -> None: + """ + Sending a message to a stream with the automatic follow/unmute policy + enabled results in including an extra optional parameter in the response. + """ + user = self.example_user("hamlet") + do_change_user_setting( + user, + "automatically_follow_topics_policy", + UserProfile.AUTOMATICALLY_CHANGE_VISIBILITY_POLICY_ON_SEND, + acting_user=None, + ) + result = self.api_post( + user, + "/api/v1/messages", + { + "type": "stream", + "to": orjson.dumps("Verona").decode(), + "content": "Test message", + "topic": "Test topic", + }, + ) + content = self.assert_json_success(result) + assert "automatic_new_visibility_policy" in content + self.assertEqual(content["automatic_new_visibility_policy"], 3) + + # Hamlet sends another message to the same topic. There will be no change in the visibility + # policy, so the 'automatic_new_visibility_policy' parameter should be absent in the result. + result = self.api_post( + user, + "/api/v1/messages", + { + "type": "stream", + "to": orjson.dumps("Verona").decode(), + "content": "Another Test message", + "topic": "Test topic", + }, + ) + content = self.assert_json_success(result) + assert "automatic_new_visibility_policy" not in content + def test_personal_message(self) -> None: """ Sending a personal message to a valid username is successful. diff --git a/zerver/views/message_send.py b/zerver/views/message_send.py index 1c5441b9f9..e70a6aeda4 100644 --- a/zerver/views/message_send.py +++ b/zerver/views/message_send.py @@ -1,5 +1,5 @@ from email.headerregistry import Address -from typing import Iterable, Optional, Sequence, Union, cast +from typing import Dict, Iterable, Optional, Sequence, Union, cast from django.core import validators from django.core.exceptions import ValidationError @@ -221,7 +221,8 @@ def send_message_backend( raise JsonableError(_("Invalid mirrored message")) sender = user_profile - ret = check_send_message( + data: Dict[str, int] = {} + sent_message_result = check_send_message( sender, client, recipient_type_name, @@ -236,7 +237,12 @@ def send_message_backend( sender_queue_id=queue_id, widget_content=widget_content, ) - return json_success(request, data={"id": ret}) + data["id"] = sent_message_result.message_id + if sent_message_result.automatic_new_visibility_policy: + data[ + "automatic_new_visibility_policy" + ] = sent_message_result.automatic_new_visibility_policy + return json_success(request, data=data) @has_request_variables diff --git a/zilencer/management/commands/add_mock_conversation.py b/zilencer/management/commands/add_mock_conversation.py index 4cd919434e..c4f50876cf 100644 --- a/zilencer/management/commands/add_mock_conversation.py +++ b/zilencer/management/commands/add_mock_conversation.py @@ -118,7 +118,9 @@ From image editing program: for message in staged_messages ] - message_ids = do_send_messages(messages) + message_ids = [ + sent_message_result.message_id for sent_message_result in do_send_messages(messages) + ] preview_message = Message.objects.get( id__in=message_ids, content__icontains="image previews"