From 7739703111e51dfe8b2c27c9236bb5ee831b7aca Mon Sep 17 00:00:00 2001 From: Lauryn Menard Date: Fri, 28 Apr 2023 17:42:23 +0200 Subject: [PATCH] scheduled-messages: Update scheduled message objects in the API for type. Updates the objects in the API for scheduled messages so that those for stream messages return the `to` property as an integer since it is always the unique stream ID and so that those for direct messages do not have a `topic` property since direct messages never have a topic. Also makes small update so that web app scheduled messages overlay has the correct stream ID. --- web/src/scheduled_messages_overlay_ui.js | 2 +- zerver/lib/scheduled_messages.py | 15 +++++++--- zerver/models.py | 35 ++++++++++++++++++++---- zerver/openapi/zulip.yaml | 25 +++++++++-------- zerver/tests/test_scheduled_messages.py | 2 +- 5 files changed, 56 insertions(+), 23 deletions(-) diff --git a/web/src/scheduled_messages_overlay_ui.js b/web/src/scheduled_messages_overlay_ui.js index 965d2e2c3e..ccb386f841 100644 --- a/web/src/scheduled_messages_overlay_ui.js +++ b/web/src/scheduled_messages_overlay_ui.js @@ -26,7 +26,7 @@ function format(scheduled_messages) { const msg_render_context = {...msg}; if (msg.type === "stream") { msg_render_context.is_stream = true; - msg_render_context.stream_id = msg.to[0]; + msg_render_context.stream_id = msg.to; msg_render_context.stream_name = stream_data.maybe_get_stream_name( msg_render_context.stream_id, ); diff --git a/zerver/lib/scheduled_messages.py b/zerver/lib/scheduled_messages.py index e9b4464b69..75be0296ce 100644 --- a/zerver/lib/scheduled_messages.py +++ b/zerver/lib/scheduled_messages.py @@ -1,9 +1,14 @@ -from typing import List +from typing import List, Union from django.utils.translation import gettext as _ from zerver.lib.exceptions import ResourceNotFoundError -from zerver.models import ScheduledMessage, ScheduledMessageDict, UserProfile +from zerver.models import ( + DirectScheduledMessageAPI, + ScheduledMessage, + StreamScheduledMessageAPI, + UserProfile, +) def access_scheduled_message( @@ -15,11 +20,13 @@ def access_scheduled_message( raise ResourceNotFoundError(_("Scheduled message does not exist")) -def get_undelivered_scheduled_messages(user_profile: UserProfile) -> List[ScheduledMessageDict]: +def get_undelivered_scheduled_messages( + user_profile: UserProfile, +) -> List[Union[DirectScheduledMessageAPI, StreamScheduledMessageAPI]]: scheduled_messages = ScheduledMessage.objects.filter( sender=user_profile, delivered=False, delivery_type=ScheduledMessage.SEND_LATER ).order_by("scheduled_timestamp") - scheduled_message_dicts: List[ScheduledMessageDict] = [ + scheduled_message_dicts: List[Union[DirectScheduledMessageAPI, StreamScheduledMessageAPI]] = [ scheduled_message.to_dict() for scheduled_message in scheduled_messages ] return scheduled_message_dicts diff --git a/zerver/models.py b/zerver/models.py index 3c4d96304f..9e4b530d9f 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -4281,13 +4281,22 @@ class ScheduledMessageNotificationEmail(models.Model): scheduled_timestamp = models.DateTimeField(db_index=True) -class ScheduledMessageDict(TypedDict): +class StreamScheduledMessageAPI(TypedDict): + scheduled_message_id: int + to: int + type: str + content: str + rendered_content: str + topic: str + deliver_at: int + + +class DirectScheduledMessageAPI(TypedDict): scheduled_message_id: int to: List[int] type: str content: str rendered_content: str - topic: str deliver_at: int @@ -4326,12 +4335,28 @@ class ScheduledMessage(models.Model): def set_topic_name(self, topic_name: str) -> None: self.subject = topic_name - def to_dict(self) -> ScheduledMessageDict: + def to_dict(self) -> Union[StreamScheduledMessageAPI, DirectScheduledMessageAPI]: recipient, recipient_type_str = get_recipient_ids(self.recipient, self.sender.id) - return ScheduledMessageDict( + if recipient_type_str == "private": + # The topic for direct messages should always be an empty string. + assert self.topic_name() == "" + + return DirectScheduledMessageAPI( + scheduled_message_id=self.id, + to=recipient, + type=recipient_type_str, + content=self.content, + rendered_content=self.rendered_content, + deliver_at=int(self.scheduled_timestamp.timestamp() * 1000), + ) + + # The recipient for stream messages should always just be the unique stream ID. + assert len(recipient) == 1 + + return StreamScheduledMessageAPI( scheduled_message_id=self.id, - to=recipient, + to=recipient[0], type=recipient_type_str, content=self.content, rendered_content=self.rendered_content, diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index f404169741..ab1343ad82 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -17358,21 +17358,23 @@ components: - stream - private to: - type: array + oneOf: + - type: integer + - type: array + items: + type: integer description: | - An array of the tentative target audience IDs. For "stream" - messages, this should contain exactly 1 ID, the ID of the - target stream. For private messages, this should be an array - of target user IDs. This cannot be an empty array since - scheduled messages are always addressed. - items: - type: integer + The scheduled message's tentative target audience. + + For stream messages, it will be the unique ID of the target + stream. For private messages, it will be an array with the + target users' IDs. topic: type: string description: | - For stream scheduled message, the tentative topic name. For private messages, - this will be ignored and should ideally - be the empty string. Should not contain null bytes. + Only present if `type` is `"stream"`. + + The topic for the stream message. content: type: string description: | @@ -17392,7 +17394,6 @@ components: - scheduled_message_id - type - to - - topic - content - rendered_content - deliver_at diff --git a/zerver/tests/test_scheduled_messages.py b/zerver/tests/test_scheduled_messages.py index 7271826ed1..a5c6b65bca 100644 --- a/zerver/tests/test_scheduled_messages.py +++ b/zerver/tests/test_scheduled_messages.py @@ -221,7 +221,7 @@ class ScheduledMessageTest(ZulipTestCase): scheduled_messages[0]["scheduled_message_id"], self.last_scheduled_message().id ) self.assertEqual(scheduled_messages[0]["content"], content) - self.assertEqual(scheduled_messages[0]["to"], [self.get_stream_id("Verona")]) + self.assertEqual(scheduled_messages[0]["to"], self.get_stream_id("Verona")) self.assertEqual(scheduled_messages[0]["type"], "stream") self.assertEqual(scheduled_messages[0]["topic"], "Test topic") self.assertEqual(