mirror of
				https://github.com/zulip/zulip.git
				synced 2025-11-03 21:43:21 +00:00 
			
		
		
		
	scheduled_messages: Set read_by_sender for self-DMs using DM group.
When using direct message group as the recipient for 1:1 or self DMs, ensure read_by_sender is set correctly when scheduling a message.
This commit is contained in:
		
				
					committed by
					
						
						Tim Abbott
					
				
			
			
				
	
			
			
			
						parent
						
							bde295806c
						
					
				
				
					commit
					9c036024bd
				
			@@ -64,8 +64,8 @@ def check_schedule_message(
 | 
			
		||||
        # Legacy default: a scheduled message you sent from a non-API client is
 | 
			
		||||
        # automatically marked as read for yourself, unless it was sent to
 | 
			
		||||
        # yourself only.
 | 
			
		||||
        read_by_sender = (
 | 
			
		||||
            client.default_read_by_sender() and send_request.message.recipient != sender.recipient
 | 
			
		||||
        read_by_sender = client.default_read_by_sender() and not addressee.is_message_to_self(
 | 
			
		||||
            sender
 | 
			
		||||
        )
 | 
			
		||||
 | 
			
		||||
    return do_schedule_messages(
 | 
			
		||||
 
 | 
			
		||||
@@ -97,6 +97,13 @@ class Addressee:
 | 
			
		||||
        assert self._topic_name is not None
 | 
			
		||||
        return self._topic_name
 | 
			
		||||
 | 
			
		||||
    def is_message_to_self(self, sender: UserProfile) -> bool:
 | 
			
		||||
        return (
 | 
			
		||||
            self.is_private()
 | 
			
		||||
            and len(self.user_profiles()) == 1
 | 
			
		||||
            and self.user_profiles()[0].id == sender.id
 | 
			
		||||
        )
 | 
			
		||||
 | 
			
		||||
    @staticmethod
 | 
			
		||||
    def legacy_build(
 | 
			
		||||
        sender: UserProfile,
 | 
			
		||||
 
 | 
			
		||||
@@ -15,7 +15,7 @@ from psycopg2.sql import SQL
 | 
			
		||||
from analytics.lib.counts import COUNT_STATS
 | 
			
		||||
from analytics.models import RealmCount
 | 
			
		||||
from zerver.lib.cache import generic_bulk_cached_fetch, to_dict_cache_key_id
 | 
			
		||||
from zerver.lib.display_recipient import get_display_recipient_by_id
 | 
			
		||||
from zerver.lib.display_recipient import get_display_recipient, get_display_recipient_by_id
 | 
			
		||||
from zerver.lib.exceptions import JsonableError, MissingAuthenticationError
 | 
			
		||||
from zerver.lib.markdown import MessageRenderingResult
 | 
			
		||||
from zerver.lib.mention import MentionData, sender_can_mention_group
 | 
			
		||||
@@ -1745,3 +1745,14 @@ def is_1_to_1_message(message: Message) -> bool:
 | 
			
		||||
        return True
 | 
			
		||||
 | 
			
		||||
    return False
 | 
			
		||||
 | 
			
		||||
 | 
			
		||||
def is_message_to_self(message: Message) -> bool:
 | 
			
		||||
    if message.recipient.type == Recipient.DIRECT_MESSAGE_GROUP:
 | 
			
		||||
        group_members = get_display_recipient(message.recipient)
 | 
			
		||||
        return len(group_members) == 1 and group_members[0]["id"] == message.sender.id
 | 
			
		||||
 | 
			
		||||
    if message.recipient.type == Recipient.PERSONAL:
 | 
			
		||||
        return message.recipient == message.sender.recipient
 | 
			
		||||
 | 
			
		||||
    return False
 | 
			
		||||
 
 | 
			
		||||
@@ -14,10 +14,12 @@ from zerver.actions.scheduled_messages import (
 | 
			
		||||
    try_deliver_one_scheduled_message,
 | 
			
		||||
)
 | 
			
		||||
from zerver.actions.users import change_user_is_active
 | 
			
		||||
from zerver.lib.message import is_message_to_self
 | 
			
		||||
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, Recipient, ScheduledMessage, UserMessage
 | 
			
		||||
from zerver.models.recipients import get_or_create_direct_message_group
 | 
			
		||||
 | 
			
		||||
if TYPE_CHECKING:
 | 
			
		||||
    from django.test.client import _MonkeyPatchedWSGIResponse as TestHttpResponse
 | 
			
		||||
@@ -104,13 +106,10 @@ class ScheduledMessageTest(ZulipTestCase):
 | 
			
		||||
        )
 | 
			
		||||
        self.assert_json_success(result)
 | 
			
		||||
 | 
			
		||||
    def test_successful_deliver_stream_scheduled_message(self) -> None:
 | 
			
		||||
        # No scheduled message
 | 
			
		||||
        result = try_deliver_one_scheduled_message()
 | 
			
		||||
        self.assertFalse(result)
 | 
			
		||||
 | 
			
		||||
        self.create_scheduled_message()
 | 
			
		||||
        scheduled_message = self.last_scheduled_message()
 | 
			
		||||
    def assert_scheduled_message_delivered(
 | 
			
		||||
        self, scheduled_message: ScheduledMessage, recipient: Recipient | None
 | 
			
		||||
    ) -> None:
 | 
			
		||||
        sender = self.example_user("hamlet")
 | 
			
		||||
 | 
			
		||||
        # mock current time to be greater than the scheduled time, so that the `scheduled_message` can be sent.
 | 
			
		||||
        more_than_scheduled_delivery_datetime = scheduled_message.scheduled_timestamp + timedelta(
 | 
			
		||||
@@ -130,23 +129,47 @@ class ScheduledMessageTest(ZulipTestCase):
 | 
			
		||||
                ],
 | 
			
		||||
            )
 | 
			
		||||
            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)
 | 
			
		||||
            self.assertEqual(scheduled_message.recipient, recipient)
 | 
			
		||||
            self.assertEqual(scheduled_message.sender, sender)
 | 
			
		||||
 | 
			
		||||
            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.topic_name(), scheduled_message.topic_name())
 | 
			
		||||
            self.assertEqual(delivered_message.date_sent, more_than_scheduled_delivery_datetime)
 | 
			
		||||
            self.assertEqual(delivered_message.recipient, scheduled_message.recipient)
 | 
			
		||||
            self.assertEqual(delivered_message.sender, scheduled_message.sender)
 | 
			
		||||
 | 
			
		||||
    def test_successful_deliver_direct_scheduled_message(self) -> None:
 | 
			
		||||
            sender_user_message = UserMessage.objects.get(
 | 
			
		||||
                message_id=scheduled_message.delivered_message_id, user_profile_id=sender.id
 | 
			
		||||
            )
 | 
			
		||||
            self.assertEqual(
 | 
			
		||||
                sender_user_message.flags.read, not is_message_to_self(delivered_message)
 | 
			
		||||
            )
 | 
			
		||||
 | 
			
		||||
    def test_successful_deliver_stream_scheduled_message(self) -> None:
 | 
			
		||||
        # No scheduled message
 | 
			
		||||
        result = try_deliver_one_scheduled_message()
 | 
			
		||||
        self.assertFalse(result)
 | 
			
		||||
 | 
			
		||||
        self.create_scheduled_message()
 | 
			
		||||
        scheduled_message = self.last_scheduled_message()
 | 
			
		||||
 | 
			
		||||
        stream_id = self.get_stream_id("Verona")
 | 
			
		||||
        recipient = Recipient.objects.get(type=Recipient.STREAM, type_id=stream_id)
 | 
			
		||||
        self.assert_scheduled_message_delivered(scheduled_message, recipient)
 | 
			
		||||
 | 
			
		||||
    def test_successful_deliver_direct_scheduled_message_to_other(self) -> None:
 | 
			
		||||
        # No scheduled message
 | 
			
		||||
        self.assertFalse(try_deliver_one_scheduled_message())
 | 
			
		||||
 | 
			
		||||
        content = "Test message"
 | 
			
		||||
        scheduled_delivery_datetime = timezone_now() + 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], f"{content} 3", scheduled_delivery_timestamp
 | 
			
		||||
@@ -154,33 +177,7 @@ class ScheduledMessageTest(ZulipTestCase):
 | 
			
		||||
        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 + timedelta(minutes=1)
 | 
			
		||||
 | 
			
		||||
        with (
 | 
			
		||||
            time_machine.travel(more_than_scheduled_delivery_datetime, tick=False),
 | 
			
		||||
            self.assertLogs(level="INFO") as logs,
 | 
			
		||||
        ):
 | 
			
		||||
            result = try_deliver_one_scheduled_message()
 | 
			
		||||
            self.assertTrue(result)
 | 
			
		||||
            self.assertEqual(
 | 
			
		||||
                logs.output,
 | 
			
		||||
                [
 | 
			
		||||
                    f"INFO:root:Sending scheduled message {scheduled_message.id} with date {scheduled_message.scheduled_timestamp} (sender: {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.assertTrue(sender_user_message.flags.read)
 | 
			
		||||
        self.assert_scheduled_message_delivered(scheduled_message, recipient=othello.recipient)
 | 
			
		||||
 | 
			
		||||
        # Check error is sent if an edit happens after the scheduled
 | 
			
		||||
        # message is successfully sent.
 | 
			
		||||
@@ -210,33 +207,31 @@ class ScheduledMessageTest(ZulipTestCase):
 | 
			
		||||
        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 + timedelta(minutes=1)
 | 
			
		||||
        self.assert_scheduled_message_delivered(scheduled_message, recipient=sender.recipient)
 | 
			
		||||
 | 
			
		||||
        with (
 | 
			
		||||
            time_machine.travel(more_than_scheduled_delivery_datetime, tick=False),
 | 
			
		||||
            self.assertLogs(level="INFO") as logs,
 | 
			
		||||
        ):
 | 
			
		||||
            result = try_deliver_one_scheduled_message()
 | 
			
		||||
            self.assertTrue(result)
 | 
			
		||||
            self.assertEqual(
 | 
			
		||||
                logs.output,
 | 
			
		||||
                [
 | 
			
		||||
                    f"INFO:root:Sending scheduled message {scheduled_message.id} with date {scheduled_message.scheduled_timestamp} (sender: {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 test_successful_deliver_direct_scheduled_message_to_self_using_direct_message_group(
 | 
			
		||||
        self,
 | 
			
		||||
    ) -> None:
 | 
			
		||||
        # No scheduled message
 | 
			
		||||
        self.assertFalse(try_deliver_one_scheduled_message())
 | 
			
		||||
 | 
			
		||||
        content = "Test message to self"
 | 
			
		||||
        scheduled_delivery_datetime = timezone_now() + timedelta(minutes=5)
 | 
			
		||||
        scheduled_delivery_timestamp = int(scheduled_delivery_datetime.timestamp())
 | 
			
		||||
        sender = self.example_user("hamlet")
 | 
			
		||||
 | 
			
		||||
        # Create a direct message group for the sender.
 | 
			
		||||
        direct_message_group = get_or_create_direct_message_group(id_list=[sender.id])
 | 
			
		||||
 | 
			
		||||
        response = self.do_schedule_message(
 | 
			
		||||
            "direct", [sender.id], content, scheduled_delivery_timestamp
 | 
			
		||||
        )
 | 
			
		||||
        self.assert_json_success(response)
 | 
			
		||||
        scheduled_message = self.last_scheduled_message()
 | 
			
		||||
 | 
			
		||||
        self.assert_scheduled_message_delivered(
 | 
			
		||||
            scheduled_message, recipient=direct_message_group.recipient
 | 
			
		||||
        )
 | 
			
		||||
 | 
			
		||||
    def verify_deliver_scheduled_message_failure(
 | 
			
		||||
        self, scheduled_message: ScheduledMessage, expected_failure_message: str
 | 
			
		||||
 
 | 
			
		||||
		Reference in New Issue
	
	Block a user