From c07f85250de38c531fe588eebecf5816a53e244b Mon Sep 17 00:00:00 2001 From: kunal-mohta Date: Fri, 18 Jan 2019 15:07:59 +0530 Subject: [PATCH] messages: Extend do_delete_message to do_delete_messages. do_delete_message has been renamed to do_delete_messages and all occurrences of the function replaced with the appropriate new version. --- zerver/lib/actions.py | 42 ++++++++++++++----------- zerver/tests/test_events.py | 8 ++--- zerver/tests/test_push_notifications.py | 4 +-- zerver/views/messages.py | 4 +-- 4 files changed, 32 insertions(+), 26 deletions(-) diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index 84456555d3..9787107122 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -4190,26 +4190,32 @@ def do_update_message(user_profile: UserProfile, message: Message, topic_name: O return len(changed_messages) -def do_delete_message(user_profile: UserProfile, message: Message) -> None: - message_type = "stream" - if not message.is_stream_message(): - message_type = "private" +def do_delete_messages(user_profile: UserProfile, messages: Iterable[Message]) -> None: + message_ids = [] + for message in messages: + message_ids.append(message.id) + message_type = "stream" + if not message.is_stream_message(): + message_type = "private" - event = { - 'type': 'delete_message', - 'sender': user_profile.email, - 'message_id': message.id, - 'message_type': message_type, } # type: Dict[str, Any] - if message_type == "stream": - event['stream_id'] = message.recipient.type_id - event['topic'] = message.topic_name() - else: - event['recipient_user_ids'] = message.recipient.type_id + event = { + 'type': 'delete_message', + 'sender': user_profile.email, + 'message_id': message.id, + 'message_type': message_type, } # type: Dict[str, Any] + if message_type == "stream": + event['stream_id'] = message.recipient.type_id + event['topic'] = message.topic_name() + else: + event['recipient_user_ids'] = message.recipient.type_id - ums = [{'id': um.user_profile_id} for um in - UserMessage.objects.filter(message=message.id)] - move_messages_to_archive([message.id]) - send_event(user_profile.realm, event, ums) + # TODO: Each part of the following should be changed to bulk + # queries, since right now if you delete 1000 messages, you'll + # end up doing 1000 database queries in a loop and timing out. + ums = [{'id': um.user_profile_id} for um in + UserMessage.objects.filter(message=message.id)] + move_messages_to_archive([message.id]) + send_event(user_profile.realm, event, ums) def do_delete_messages_by_sender(user: UserProfile) -> None: message_ids = Message.objects.filter(sender=user).values_list('id', flat=True).order_by('id') diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index 730c45dcd6..935b34c2fe 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -53,7 +53,7 @@ from zerver.lib.actions import ( do_create_default_stream_group, do_deactivate_stream, do_deactivate_user, - do_delete_message, + do_delete_messages, do_invite_users, do_mark_hotspot_as_read, do_mute_topic, @@ -2328,7 +2328,7 @@ class EventsRegisterTest(ZulipTestCase): msg_id = self.send_stream_message("hamlet@zulip.com", "Verona") message = Message.objects.get(id=msg_id) events = self.do_test( - lambda: do_delete_message(self.user_profile, message), + lambda: do_delete_messages(self.user_profile, [message]), state_change_expected=True, ) error = schema_checker('events[0]', events[0]) @@ -2349,7 +2349,7 @@ class EventsRegisterTest(ZulipTestCase): ) message = Message.objects.get(id=msg_id) events = self.do_test( - lambda: do_delete_message(self.user_profile, message), + lambda: do_delete_messages(self.user_profile, [message]), state_change_expected=True, ) error = schema_checker('events[0]', events[0]) @@ -2363,7 +2363,7 @@ class EventsRegisterTest(ZulipTestCase): msg_id = self.send_stream_message("hamlet@zulip.com", "Verona") message = Message.objects.get(id=msg_id) self.do_test( - lambda: do_delete_message(self.user_profile, message), + lambda: do_delete_messages(self.user_profile, [message]), state_change_expected=True, ) result = fetch_initial_state_data(user_profile, None, "", client_gravatar=False) diff --git a/zerver/tests/test_push_notifications.py b/zerver/tests/test_push_notifications.py index 26a8ea3ffc..56e82058f9 100644 --- a/zerver/tests/test_push_notifications.py +++ b/zerver/tests/test_push_notifications.py @@ -34,7 +34,7 @@ from zerver.models import ( Stream, Subscription, ) -from zerver.lib.actions import do_delete_message +from zerver.lib.actions import do_delete_messages from zerver.lib.soft_deactivation import do_soft_deactivate_users from zerver.lib import push_notifications as apn from zerver.lib.push_notifications import get_mobile_push_content, \ @@ -493,7 +493,7 @@ class HandlePushNotificationTest(PushNotificationTest): 'trigger': 'private_message', } # Now, delete the message the normal way - do_delete_message(user_profile, message) + do_delete_messages(user_profile, [message]) with mock.patch('zerver.lib.push_notifications.uses_notification_bouncer') as mock_check, \ mock.patch('logging.error') as mock_logging_error, \ diff --git a/zerver/views/messages.py b/zerver/views/messages.py index c7f54388fd..e1b2461cc5 100644 --- a/zerver/views/messages.py +++ b/zerver/views/messages.py @@ -17,7 +17,7 @@ from zerver.lib.zcommand import process_zcommands from zerver.lib.actions import recipient_for_emails, do_update_message_flags, \ compute_irc_user_fullname, compute_jabber_user_fullname, \ create_mirror_user_if_needed, check_send_message, do_update_message, \ - extract_recipients, truncate_body, render_incoming_message, do_delete_message, \ + extract_recipients, truncate_body, render_incoming_message, do_delete_messages, \ do_mark_all_as_read, do_mark_stream_messages_as_read, \ get_user_info_for_message_updates, check_schedule_message from zerver.lib.addressee import raw_pm_with_emails @@ -1479,7 +1479,7 @@ def delete_message_backend(request: HttpRequest, user_profile: UserProfile, message_id: int=REQ(converter=to_non_negative_int)) -> HttpResponse: message, ignored_user_message = access_message(user_profile, message_id) validate_can_delete_message(user_profile, message) - do_delete_message(user_profile, message) + do_delete_messages(user_profile, [message]) return json_success() @has_request_variables