mirror of
				https://github.com/zulip/zulip.git
				synced 2025-11-04 05:53:43 +00:00 
			
		
		
		
	message_send: Don't mark scheduled messages to self as read.
The only reasonable intent for such a scheduled message is to remind oneself of something at that time, which requires it being unread. Fixes #25523.
This commit is contained in:
		@@ -590,6 +590,7 @@ def create_user_messages(
 | 
			
		||||
    mentioned_user_ids: AbstractSet[int],
 | 
			
		||||
    mark_as_read_user_ids: Set[int],
 | 
			
		||||
    limit_unread_user_ids: Optional[Set[int]],
 | 
			
		||||
    scheduled_message_to_self: bool,
 | 
			
		||||
) -> List[UserMessageLite]:
 | 
			
		||||
    # These properties on the Message are set via
 | 
			
		||||
    # render_markdown by code in the Markdown inline patterns
 | 
			
		||||
@@ -626,7 +627,14 @@ def create_user_messages(
 | 
			
		||||
    for user_profile_id in um_eligible_user_ids:
 | 
			
		||||
        flags = base_flags
 | 
			
		||||
        if (
 | 
			
		||||
            (user_profile_id == sender_id and message.sent_by_human())
 | 
			
		||||
            (
 | 
			
		||||
                # Messages you sent from a non-API client are
 | 
			
		||||
                # automatically marked as read for yourself; scheduled
 | 
			
		||||
                # messages to yourself only are not.
 | 
			
		||||
                user_profile_id == sender_id
 | 
			
		||||
                and message.sent_by_human()
 | 
			
		||||
                and not scheduled_message_to_self
 | 
			
		||||
            )
 | 
			
		||||
            or user_profile_id in mark_as_read_user_ids
 | 
			
		||||
            or (limit_unread_user_ids is not None and user_profile_id not in limit_unread_user_ids)
 | 
			
		||||
        ):
 | 
			
		||||
@@ -707,6 +715,7 @@ def do_send_messages(
 | 
			
		||||
    send_message_requests_maybe_none: Sequence[Optional[SendMessageRequest]],
 | 
			
		||||
    *,
 | 
			
		||||
    email_gateway: bool = False,
 | 
			
		||||
    scheduled_message_to_self: bool = False,
 | 
			
		||||
    mark_as_read: Sequence[int] = [],
 | 
			
		||||
) -> List[int]:
 | 
			
		||||
    """See
 | 
			
		||||
@@ -754,6 +763,7 @@ def do_send_messages(
 | 
			
		||||
                mentioned_user_ids=mentioned_user_ids,
 | 
			
		||||
                mark_as_read_user_ids=mark_as_read_user_ids,
 | 
			
		||||
                limit_unread_user_ids=send_request.limit_unread_user_ids,
 | 
			
		||||
                scheduled_message_to_self=scheduled_message_to_self,
 | 
			
		||||
            )
 | 
			
		||||
 | 
			
		||||
            for um in user_messages:
 | 
			
		||||
 
 | 
			
		||||
@@ -244,7 +244,10 @@ def send_scheduled_message(scheduled_message: ScheduledMessage) -> None:
 | 
			
		||||
        scheduled_message.realm,
 | 
			
		||||
    )
 | 
			
		||||
 | 
			
		||||
    message_id = do_send_messages([send_request])[0]
 | 
			
		||||
    scheduled_message_to_self = scheduled_message.recipient == scheduled_message.sender.recipient
 | 
			
		||||
    message_id = do_send_messages(
 | 
			
		||||
        [send_request], scheduled_message_to_self=scheduled_message_to_self
 | 
			
		||||
    )[0]
 | 
			
		||||
    scheduled_message.delivered_message_id = message_id
 | 
			
		||||
    scheduled_message.delivered = True
 | 
			
		||||
    scheduled_message.save(update_fields=["delivered", "delivered_message_id"])
 | 
			
		||||
 
 | 
			
		||||
@@ -19,7 +19,7 @@ from zerver.lib.exceptions import JsonableError
 | 
			
		||||
from zerver.lib.test_classes import ZulipTestCase
 | 
			
		||||
from zerver.lib.test_helpers import most_recent_message
 | 
			
		||||
from zerver.lib.timestamp import timestamp_to_datetime
 | 
			
		||||
from zerver.models import Attachment, Message, ScheduledMessage
 | 
			
		||||
from zerver.models import Attachment, Message, ScheduledMessage, UserMessage
 | 
			
		||||
 | 
			
		||||
if TYPE_CHECKING:
 | 
			
		||||
    from django.test.client import _MonkeyPatchedWSGIResponse as TestHttpResponse
 | 
			
		||||
@@ -153,6 +153,7 @@ class ScheduledMessageTest(ZulipTestCase):
 | 
			
		||||
        content = "Test message"
 | 
			
		||||
        scheduled_delivery_datetime = timezone_now() + datetime.timedelta(minutes=5)
 | 
			
		||||
        scheduled_delivery_timestamp = int(scheduled_delivery_datetime.timestamp())
 | 
			
		||||
        sender = self.example_user("hamlet")
 | 
			
		||||
        othello = self.example_user("othello")
 | 
			
		||||
        response = self.do_schedule_message(
 | 
			
		||||
            "direct", [othello.id], content + " 3", scheduled_delivery_timestamp
 | 
			
		||||
@@ -190,6 +191,10 @@ class ScheduledMessageTest(ZulipTestCase):
 | 
			
		||||
                    delivered_message.rendered_content, scheduled_message.rendered_content
 | 
			
		||||
                )
 | 
			
		||||
                self.assertEqual(delivered_message.date_sent, more_than_scheduled_delivery_datetime)
 | 
			
		||||
                sender_user_message = UserMessage.objects.get(
 | 
			
		||||
                    message_id=scheduled_message.delivered_message_id, user_profile_id=sender.id
 | 
			
		||||
                )
 | 
			
		||||
                self.assertTrue(sender_user_message.flags.read)
 | 
			
		||||
 | 
			
		||||
        # Check error is sent if an edit happens after the scheduled
 | 
			
		||||
        # message is successfully sent.
 | 
			
		||||
@@ -204,6 +209,56 @@ class ScheduledMessageTest(ZulipTestCase):
 | 
			
		||||
        )
 | 
			
		||||
        self.assert_json_error(updated_response, "Scheduled message was already sent")
 | 
			
		||||
 | 
			
		||||
    def test_successful_deliver_private_scheduled_message_to_self(self) -> None:
 | 
			
		||||
        logger = mock.Mock()
 | 
			
		||||
        # No scheduled message
 | 
			
		||||
        self.assertFalse(try_deliver_one_scheduled_message(logger))
 | 
			
		||||
 | 
			
		||||
        content = "Test message to self"
 | 
			
		||||
        scheduled_delivery_datetime = timezone_now() + datetime.timedelta(minutes=5)
 | 
			
		||||
        scheduled_delivery_timestamp = int(scheduled_delivery_datetime.timestamp())
 | 
			
		||||
        sender = self.example_user("hamlet")
 | 
			
		||||
        response = self.do_schedule_message(
 | 
			
		||||
            "direct", [sender.id], content, scheduled_delivery_timestamp
 | 
			
		||||
        )
 | 
			
		||||
        self.assert_json_success(response)
 | 
			
		||||
        scheduled_message = self.last_scheduled_message()
 | 
			
		||||
 | 
			
		||||
        # mock current time to be greater than the scheduled time.
 | 
			
		||||
        more_than_scheduled_delivery_datetime = scheduled_delivery_datetime + datetime.timedelta(
 | 
			
		||||
            minutes=1
 | 
			
		||||
        )
 | 
			
		||||
        with mock.patch(
 | 
			
		||||
            "zerver.actions.scheduled_messages.timezone_now",
 | 
			
		||||
            return_value=more_than_scheduled_delivery_datetime,
 | 
			
		||||
        ):
 | 
			
		||||
            with mock.patch(
 | 
			
		||||
                "zerver.actions.message_send.timezone_now",
 | 
			
		||||
                return_value=more_than_scheduled_delivery_datetime,
 | 
			
		||||
            ):
 | 
			
		||||
                result = try_deliver_one_scheduled_message(logger)
 | 
			
		||||
                self.assertTrue(result)
 | 
			
		||||
                logger.info.assert_called_once_with(
 | 
			
		||||
                    "Sending scheduled message %s with date %s (sender: %s)",
 | 
			
		||||
                    scheduled_message.id,
 | 
			
		||||
                    scheduled_message.scheduled_timestamp,
 | 
			
		||||
                    scheduled_message.sender_id,
 | 
			
		||||
                )
 | 
			
		||||
                scheduled_message.refresh_from_db()
 | 
			
		||||
                assert isinstance(scheduled_message.delivered_message_id, int)
 | 
			
		||||
                self.assertEqual(scheduled_message.delivered, True)
 | 
			
		||||
                self.assertEqual(scheduled_message.failed, False)
 | 
			
		||||
                delivered_message = Message.objects.get(id=scheduled_message.delivered_message_id)
 | 
			
		||||
                self.assertEqual(delivered_message.content, scheduled_message.content)
 | 
			
		||||
                self.assertEqual(
 | 
			
		||||
                    delivered_message.rendered_content, scheduled_message.rendered_content
 | 
			
		||||
                )
 | 
			
		||||
                self.assertEqual(delivered_message.date_sent, more_than_scheduled_delivery_datetime)
 | 
			
		||||
                sender_user_message = UserMessage.objects.get(
 | 
			
		||||
                    message_id=scheduled_message.delivered_message_id, user_profile_id=sender.id
 | 
			
		||||
                )
 | 
			
		||||
                self.assertFalse(sender_user_message.flags.read)
 | 
			
		||||
 | 
			
		||||
    def verify_deliver_scheduled_message_failure(
 | 
			
		||||
        self, scheduled_message: ScheduledMessage, logger: mock.Mock, expected_failure_message: str
 | 
			
		||||
    ) -> None:
 | 
			
		||||
 
 | 
			
		||||
		Reference in New Issue
	
	Block a user