From 4e4c33fee2da212d67ee60eae8b893acf1f6eb29 Mon Sep 17 00:00:00 2001 From: Aditya Bansal Date: Tue, 27 Feb 2018 02:50:51 +0530 Subject: [PATCH] reminders: Make reminders setup work only for Streams. We make this change because setting up reminders in PM's didn't play really well with our current infrastructure. Basically the reminder messages from the bot can't appear in the same narrow as that of a PM between two people and therefore we disable it. Though we make an exception here where a person wants to set up reminder for himself. --- zerver/lib/actions.py | 6 ++++++ zerver/tests/test_messages.py | 17 +++++++++++++---- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index 2aeee38b70..5578ba32d9 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -1990,6 +1990,12 @@ def check_schedule_message(sender: UserProfile, client: Client, forwarder_user_profile=forwarder_user_profile) message['deliver_at'] = deliver_at message['delivery_type'] = delivery_type + + recipient = message['message'].recipient + if (delivery_type == 'remind' and (recipient.type != Recipient.STREAM and + recipient.type_id != sender.id)): + raise JsonableError(_("Reminders can only be set for streams.")) + return do_schedule_messages([message])[0] def check_stream_name(stream_name: str) -> None: diff --git a/zerver/tests/test_messages.py b/zerver/tests/test_messages.py index 4a2c6d4991..6be529d8b2 100644 --- a/zerver/tests/test_messages.py +++ b/zerver/tests/test_messages.py @@ -2016,20 +2016,29 @@ class ScheduledMessageTest(ZulipTestCase): self.assertEqual(message.scheduled_timestamp, convert_to_UTC(defer_until)) self.assertEqual(message.delivery_type, ScheduledMessage.SEND_LATER) + # Setting a reminder in PM's to other users causes a error. result = self.do_schedule_message('private', self.example_email("othello"), content + ' 4', defer_until_str, delivery_type='remind') + self.assert_json_error(result, 'Reminders can only be set for streams.') + + # Setting a reminder in PM's to ourself is successful. + # Required by reminders from message actions popover caret feature. + result = self.do_schedule_message('private', self.example_email("hamlet"), + content + ' 5', defer_until_str, + delivery_type='remind') message = self.last_scheduled_message() self.assert_json_success(result) + self.assertEqual(message.content, 'Test message 5') self.assertEqual(message.delivery_type, ScheduledMessage.REMIND) # Scheduling a message while guessing timezone. tz_guess = 'Asia/Kolkata' - result = self.do_schedule_message('stream', 'Verona', content + ' 5', + result = self.do_schedule_message('stream', 'Verona', content + ' 6', defer_until_str, tz_guess=tz_guess) message = self.last_scheduled_message() self.assert_json_success(result) - self.assertEqual(message.content, 'Test message 5') + self.assertEqual(message.content, 'Test message 6') local_tz = get_timezone(tz_guess) # Since mypy is not able to recognize localize and normalize as attributes of tzinfo we use ignore. utz_defer_until = local_tz.normalize(local_tz.localize(defer_until)) # type: ignore # Reason in comment on previous line. @@ -2043,10 +2052,10 @@ class ScheduledMessageTest(ZulipTestCase): user.timezone = 'US/Pacific' user.save(update_fields=['timezone']) result = self.do_schedule_message('stream', 'Verona', - content + ' 6', defer_until_str) + content + ' 7', defer_until_str) message = self.last_scheduled_message() self.assert_json_success(result) - self.assertEqual(message.content, 'Test message 6') + self.assertEqual(message.content, 'Test message 7') local_tz = get_timezone(user.timezone) # Since mypy is not able to recognize localize and normalize as attributes of tzinfo we use ignore. utz_defer_until = local_tz.normalize(local_tz.localize(defer_until)) # type: ignore # Reason in comment on previous line.