diff --git a/frontend_tests/casper_tests/15-delete-message.js b/frontend_tests/casper_tests/15-delete-message.js new file mode 100644 index 0000000000..ae6969122d --- /dev/null +++ b/frontend_tests/casper_tests/15-delete-message.js @@ -0,0 +1,43 @@ +var common = require('../casper_lib/common.js').common; + +common.start_and_log_in(); + +var last_message_id; +var msgs_qty; + + +casper.then(function () { + casper.waitUntilVisible("#zhome"); +}); + +casper.then(function () { + msgs_qty = this.evaluate(function () { + return $('#zhome .message_row').length; + }); + last_message_id = this.evaluate(function () { + var msg = $('#zhome .message_row:last'); + msg.find('.info').click(); + $('.delete_message').click(); + return msg.attr('id'); + }); +}); + +casper.then(function () { + casper.waitUntilVisible("#delete_message_modal", function () { + casper.click('#do_delete_message_button'); + }); +}); + +casper.then(function () { + casper.waitWhileVisible(last_message_id, function () { + var msgs_after_deleting = casper.evaluate(function () { + return $('#zhome .message_row').length; + }); + casper.test.assertEquals(msgs_qty - 1, msgs_after_deleting); + casper.test.assertDoesntExist(last_message_id); + }); +}); + +casper.run(function () { + casper.test.done(); +}); diff --git a/static/js/message_edit.js b/static/js/message_edit.js index 107bbf7855..14b1b3e2a6 100644 --- a/static/js/message_edit.js +++ b/static/js/message_edit.js @@ -468,6 +468,26 @@ exports.show_history = function (message) { }); }; +exports.delete_message = function (msg_id) { + $("#delete-message-error").html(''); + $('#delete_message_modal').modal("show"); + $('#do_delete_message_button').off().on('click', function (e) { + e.stopPropagation(); + e.preventDefault(); + channel.del({ + url: "/json/messages/" + msg_id, + success: function () { + $('#delete_message_modal').modal("hide"); + }, + error: function (xhr) { + ui_report.error(i18n.t("Error deleting message."), xhr, + $("#delete-message-error")); + }, + }); + + }); +}; + $(document).on('narrow_deactivated.zulip', function () { _.each(currently_editing_messages, function (elem, idx) { if (current_msg_list.get(idx) !== undefined) { diff --git a/static/js/popovers.js b/static/js/popovers.js index c028159502..51d09f4e36 100644 --- a/static/js/popovers.js +++ b/static/js/popovers.js @@ -147,7 +147,7 @@ exports.toggle_actions_popover = function (element, id) { var should_display_edit_history_option = _.any(message.edit_history, function (entry) { return entry.prev_content !== undefined; }); - + var should_display_delete_option = page_params.is_admin; var args = { message: message, use_edit_icon: use_edit_icon, @@ -158,6 +158,7 @@ exports.toggle_actions_popover = function (element, id) { should_display_edit_history_option: should_display_edit_history_option, conversation_time_uri: narrow.by_conversation_and_time_uri(message, true), narrowed: narrow_state.active(), + should_display_delete_option: should_display_delete_option, }; var ypos = elt.offset().top; @@ -505,6 +506,14 @@ exports.register_click_handlers = function () { e.preventDefault(); }); + $('body').on('click', '.delete_message', function (e) { + var msgid = $(e.currentTarget).data('message-id'); + popovers.hide_actions_popover(); + message_edit.delete_message(msgid); + e.stopPropagation(); + e.preventDefault(); + }); + function initClipboard(selector) { return new Clipboard(selector); } diff --git a/static/js/server_events.js b/static/js/server_events.js index 42f57a10cb..e32552343f 100644 --- a/static/js/server_events.js +++ b/static/js/server_events.js @@ -346,6 +346,12 @@ function dispatch_normal_event(event) { break; } break; + + case 'delete_message': + var msg_id = event.message_id; + ui.remove_message(msg_id); + break; + } } diff --git a/static/js/ui.js b/static/js/ui.js index 99be4aa2fa..7d8b226d2b 100644 --- a/static/js/ui.js +++ b/static/js/ui.js @@ -98,6 +98,18 @@ exports.show_message_failed = function (message_id, failed_msg) { }); }; +exports.remove_message = function (message_id) { + _.each([message_list.all, home_msg_list, message_list.narrowed], function (list) { + if (list === undefined) { + return; + } + var row = list.get_row(message_id); + if (row !== undefined) { + list.remove_and_rerender([{id: message_id}]); + } + }); +}; + exports.show_failed_message_success = function (message_id) { // Previously failed message succeeded update_message_in_all_views(message_id, function update_row(row) { diff --git a/static/templates/actions_popover_content.handlebars b/static/templates/actions_popover_content.handlebars index 6bdd895724..314d90a8a6 100644 --- a/static/templates/actions_popover_content.handlebars +++ b/static/templates/actions_popover_content.handlebars @@ -28,7 +28,14 @@ {{/if}} - + {{#if should_display_delete_option}} +
  • + + + {{t "Delete message" }} + +
    • + {{/if}} {{#if can_mute_topic}}
  • diff --git a/templates/zerver/delete_message.html b/templates/zerver/delete_message.html new file mode 100644 index 0000000000..49275e7100 --- /dev/null +++ b/templates/zerver/delete_message.html @@ -0,0 +1,16 @@ + diff --git a/templates/zerver/home.html b/templates/zerver/home.html index 72fc22bead..f4cababc31 100644 --- a/templates/zerver/home.html +++ b/templates/zerver/home.html @@ -91,6 +91,7 @@ {% include "zerver/message_history.html" %} +{% include "zerver/delete_message.html" %} {% include "zerver/compose.html" %}
    diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index 4536847540..c2c6a2c2d8 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -28,6 +28,7 @@ from zerver.lib.message import ( render_markdown, ) from zerver.lib.realm_icon import realm_icon_url +from zerver.lib.retention import move_message_to_archive from zerver.models import Realm, RealmEmoji, Stream, UserProfile, UserActivity, \ RealmDomain, \ Subscription, Recipient, Message, Attachment, UserMessage, RealmAuditLog, \ @@ -2821,6 +2822,19 @@ def do_update_message(user_profile, message, subject, propagate_mode, content, r send_event(event, list(map(user_info, ums))) return len(changed_messages) + +def do_delete_message(user_profile, message): + # type: (UserProfile, Message) -> None + event = { + 'type': 'delete_message', + 'sender': user_profile.email, + 'message_id': message.id} # type: Dict[str, Any] + ums = [{'id': um.user_profile_id} for um in + UserMessage.objects.filter(message=message.id)] + move_message_to_archive(message.id) + send_event(event, ums) + + def encode_email_address(stream): # type: (Stream) -> Text return encode_email_address_helper(stream.name, stream.email_token) diff --git a/zerver/lib/events.py b/zerver/lib/events.py index 61abf25748..e58b2ba1c9 100644 --- a/zerver/lib/events.py +++ b/zerver/lib/events.py @@ -402,6 +402,13 @@ def apply_event(state, event, user_profile, include_subscribers): elif event['type'] == "update_message": # The client will get the updated message directly pass + elif event['type'] == "delete_message": + max_message = Message.objects.filter( + usermessage__user_profile=user_profile).order_by('-id').first() + if max_message: + state['max_message_id'] = max_message.id + else: + state['max_message_id'] = -1 elif event['type'] == "reaction": # The client will get the message with the reactions directly pass diff --git a/zerver/lib/retention.py b/zerver/lib/retention.py index 775d7d9b69..854852ddfa 100644 --- a/zerver/lib/retention.py +++ b/zerver/lib/retention.py @@ -2,8 +2,12 @@ from __future__ import absolute_import from __future__ import print_function from datetime import timedelta + +from django.db import connection, transaction +from django.forms.models import model_to_dict from django.utils.timezone import now as timezone_now -from zerver.models import Realm, Message +from zerver.models import Realm, Message, UserMessage, ArchivedMessage, ArchivedUserMessage, \ + Attachment, ArchivedAttachment from typing import Any, Dict, Optional, Generator @@ -27,3 +31,55 @@ def get_expired_messages(): realm_expired_messages = get_realm_expired_messages(realm) if realm_expired_messages: yield realm_expired_messages + + +def move_attachment_message_to_archive_by_message(message_id): + # type: (int) -> None + # Move attachments messages relation table data to archive. + query = """ + INSERT INTO zerver_archivedattachment_messages (id, archivedattachment_id, + archivedmessage_id) + SELECT zerver_attachment_messages.id, zerver_attachment_messages.attachment_id, + zerver_attachment_messages.message_id + FROM zerver_attachment_messages + LEFT JOIN zerver_archivedattachment_messages + ON zerver_archivedattachment_messages.id = zerver_attachment_messages.id + WHERE zerver_attachment_messages.message_id = {message_id} + AND zerver_archivedattachment_messages.id IS NULL + """ + with connection.cursor() as cursor: + cursor.execute(query.format(message_id=message_id)) + + +@transaction.atomic +def move_message_to_archive(message_id): + # type: (int) -> None + msg = list(Message.objects.filter(id=message_id).values()) + if not msg: + raise Message.DoesNotExist + arc_message = ArchivedMessage(**msg[0]) + arc_message.save() + + # Move user_messages to the archive. + user_messages = UserMessage.objects.filter( + message_id=message_id).exclude(id__in=ArchivedUserMessage.objects.all()) + archiving_messages = [] + for user_message in user_messages.values(): + archiving_messages.append(ArchivedUserMessage(**user_message)) + ArchivedUserMessage.objects.bulk_create(archiving_messages) + + # Move attachments to archive + attachments = Attachment.objects.filter(messages__id=message_id).exclude( + id__in=ArchivedAttachment.objects.all()) + archiving_attachments = [] + for attachment in attachments.values(): + archiving_attachments.append(ArchivedAttachment(**attachment)) + ArchivedAttachment.objects.bulk_create(archiving_attachments) + move_attachment_message_to_archive_by_message(message_id) + + # Remove data from main tables + Message.objects.get(id=message_id).delete() + user_messages.filter(id__in=ArchivedUserMessage.objects.all(), + message_id__isnull=True).delete() + archived_attachments = ArchivedAttachment.objects.filter(messages__id=message_id) + Attachment.objects.filter(messages__isnull=True, id__in=archived_attachments).delete() diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index c6aeae3ee4..c4e55020c7 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -67,6 +67,7 @@ from zerver.lib.actions import ( do_remove_realm_domain, do_change_icon_source, log_event, + do_delete_message, ) from zerver.lib.events import ( apply_events, @@ -1510,6 +1511,37 @@ class EventsRegisterTest(ZulipTestCase): error = add_schema_checker('events[1]', events[1]) self.assert_on_error(error) + def test_do_delete_message(self): + # type: () -> None + schema_checker = self.check_events_dict([ + ('type', equals('delete_message')), + ('message_id', check_int), + ('sender', check_string), + ]) + msg_id = self.send_message("hamlet@zulip.com", "Verona", Recipient.STREAM) + message = Message.objects.get(id=msg_id) + events = self.do_test( + lambda: do_delete_message(self.user_profile, message), + state_change_expected=True, + ) + error = schema_checker('events[0]', events[0]) + self.assert_on_error(error) + + def test_do_delete_message_no_max_id(self): + # type: () -> None + user_profile = self.example_user('aaron') + # Delete all historical messages for this user + user_profile = self.example_user('hamlet') + UserMessage.objects.filter(user_profile=user_profile).delete() + msg_id = self.send_message("hamlet@zulip.com", "Verona", Recipient.STREAM) + message = Message.objects.get(id=msg_id) + self.do_test( + lambda: do_delete_message(self.user_profile, message), + state_change_expected=True, + ) + result = fetch_initial_state_data(user_profile, None, "") + self.assertEqual(result['max_message_id'], -1) + class FetchInitialStateDataTest(ZulipTestCase): # Non-admin users don't have access to all bots def test_realm_bots_non_admin(self): diff --git a/zerver/tests/test_messages.py b/zerver/tests/test_messages.py index a72e069957..077d928d8f 100644 --- a/zerver/tests/test_messages.py +++ b/zerver/tests/test_messages.py @@ -1963,3 +1963,29 @@ class CheckMessageTest(ZulipTestCase): self.assertEqual(new_count, old_count + 2) self.assertEqual(ret['message'].sender.email, 'othello-bot@zulip.com') self.assertIn("there are no subscribers to that stream", most_recent_message(parent).content) + + +class DeleteMessageTest(ZulipTestCase): + + def test_delete_message_by_owner(self): + # type: () -> None + self.login("hamlet@zulip.com") + msg_id = self.send_message("hamlet@zulip.com", "Scotland", Recipient.STREAM) + result = self.client_delete('/json/messages/{msg_id}'.format(msg_id=msg_id)) + self.assert_json_error(result, "You don't have permission to edit this message") + + def test_delete_message_by_realm_admin(self): + # type: () -> None + self.login("iago@zulip.com") + msg_id = self.send_message("hamlet@zulip.com", "Scotland", Recipient.STREAM) + result = self.client_delete('/json/messages/{msg_id}'.format(msg_id=msg_id)) + self.assert_json_success(result) + + def test_delete_message_second_time(self): + # type: () -> None + self.login("iago@zulip.com") + msg_id = self.send_message("hamlet@zulip.com", "Scotland", Recipient.STREAM, + subject="editing", content="before edit") + self.client_delete('/json/messages/{msg_id}'.format(msg_id=msg_id)) + result = self.client_delete('/json/messages/{msg_id}'.format(msg_id=msg_id)) + self.assert_json_error(result, "Invalid message(s)") diff --git a/zerver/tests/test_retention.py b/zerver/tests/test_retention.py index b575954e4a..97514f283f 100644 --- a/zerver/tests/test_retention.py +++ b/zerver/tests/test_retention.py @@ -5,8 +5,10 @@ from datetime import datetime, timedelta from django.utils.timezone import now as timezone_now from zerver.lib.test_classes import ZulipTestCase -from zerver.models import Message, Realm, Recipient, UserProfile -from zerver.lib.retention import get_expired_messages +from zerver.lib.upload import create_attachment +from zerver.models import Message, Realm, Recipient, UserProfile, UserMessage, ArchivedUserMessage, \ + ArchivedMessage, Attachment, ArchivedAttachment +from zerver.lib.retention import get_expired_messages, move_message_to_archive from typing import Any, List @@ -113,3 +115,109 @@ class TestRetentionLib(ZulipTestCase): set(actual_mit_messages_ids) - set(expired_mit_messages_ids), set(actual_mit_messages_ids) ) + + +class TestMoveMessageToArchive(ZulipTestCase): + + def setUp(self): + # type: () -> None + super(TestMoveMessageToArchive, self).setUp() + self.sender = 'hamlet@zulip.com' + self.recipient = 'cordelia@zulip.com' + + def _create_attachments(self): + # type: () -> None + sample_size = 10 + dummy_files = [ + ('zulip.txt', '1/31/4CBjtTLYZhk66pZrF8hnYGwc/zulip.txt', sample_size), + ('temp_file.py', '1/31/4CBjtTLYZhk66pZrF8hnYGwc/temp_file.py', sample_size), + ('abc.py', '1/31/4CBjtTLYZhk66pZrF8hnYGwc/abc.py', sample_size) + ] + user_profile = self.example_user('hamlet') + for file_name, path_id, size in dummy_files: + create_attachment(file_name, path_id, user_profile, size) + + def _check_messages_before_archiving(self, msg_id): + # type: (int) -> List + user_messages_ids_before = list(UserMessage.objects.filter( + message_id=msg_id).order_by('id').values_list('id', flat=True)) + self.assertEqual(ArchivedUserMessage.objects.count(), 0) + self.assertEqual(ArchivedMessage.objects.count(), 0) + return user_messages_ids_before + + def _check_messages_after_archiving(self, msg_id, user_msgs_ids_before): + # type: (int, List[int]) -> None + self.assertEqual(ArchivedMessage.objects.filter(id=msg_id).count(), 1) + self.assertEqual(Message.objects.filter(id=msg_id).count(), 0) + self.assertEqual(UserMessage.objects.filter(message_id=msg_id).count(), 0) + arc_user_messages_ids_after = list(ArchivedUserMessage.objects.filter( + message_id=msg_id).order_by('id').values_list('id', flat=True)) + self.assertEqual(arc_user_messages_ids_after, user_msgs_ids_before) + + def test_personal_message_archiving(self): + # type: ()-> None + msg_id = self.send_message(self.sender, [self.recipient], Recipient.PERSONAL) + user_messages_ids_before = self._check_messages_before_archiving(msg_id) + move_message_to_archive(message_id=msg_id) + self._check_messages_after_archiving(msg_id, user_messages_ids_before) + + def test_stream_message_archiving(self): + # type: ()-> None + msg_id = self.send_message(self.sender, "Verona", Recipient.STREAM) + user_messages_ids_before = self._check_messages_before_archiving(msg_id) + move_message_to_archive(message_id=msg_id) + self._check_messages_after_archiving(msg_id, user_messages_ids_before) + + def test_archiving_message_second_time(self): + # type: ()-> None + msg_id = self.send_message(self.sender, "Verona", Recipient.STREAM) + user_messages_ids_before = self._check_messages_before_archiving(msg_id) + move_message_to_archive(message_id=msg_id) + self._check_messages_after_archiving(msg_id, user_messages_ids_before) + with self.assertRaises(Message.DoesNotExist): + move_message_to_archive(message_id=msg_id) + + def test_archiving_message_with_attachment(self): + # type: () -> None + self._create_attachments() + body = """Some files here ...[zulip.txt]( + http://localhost:9991/user_uploads/1/31/4CBjtTLYZhk66pZrF8hnYGwc/zulip.txt) + http://localhost:9991/user_uploads/1/31/4CBjtTLYZhk66pZrF8hnYGwc/temp_file.py .... + Some more.... http://localhost:9991/user_uploads/1/31/4CBjtTLYZhk66pZrF8hnYGwc/abc.py + """ + msg_id = self.send_message(self.sender, [self.recipient], Recipient.PERSONAL, body) + user_messages_ids_before = self._check_messages_before_archiving(msg_id) + attachments_ids_before = list(Attachment.objects.filter( + messages__id=msg_id).order_by("id").values_list("id", flat=True)) + self.assertEqual(ArchivedAttachment.objects.count(), 0) + move_message_to_archive(message_id=msg_id) + self._check_messages_after_archiving(msg_id, user_messages_ids_before) + self.assertEqual(Attachment.objects.count(), 0) + arc_attachments_ids_after = list(ArchivedAttachment.objects.filter( + messages__id=msg_id).order_by("id").values_list("id", flat=True)) + self.assertEqual(attachments_ids_before, arc_attachments_ids_after) + + def test_archiving_message_with_shared_attachment(self): + # type: () -> None + # Check do not removing attachments which is used in other messages. + self._create_attachments() + body = """Some files here ...[zulip.txt]( + http://localhost:9991/user_uploads/1/31/4CBjtTLYZhk66pZrF8hnYGwc/zulip.txt) + http://localhost:9991/user_uploads/1/31/4CBjtTLYZhk66pZrF8hnYGwc/temp_file.py .... + Some more.... http://localhost:9991/user_uploads/1/31/4CBjtTLYZhk66pZrF8hnYGwc/abc.py + """ + msg_id = self.send_message(self.sender, [self.recipient], Recipient.PERSONAL, body) + msg_id_shared_attachments = self.send_message(self.recipient, [self.sender], + Recipient.PERSONAL, body) + user_messages_ids_before = self._check_messages_before_archiving(msg_id) + attachments_ids_before = list(Attachment.objects.filter( + messages__id=msg_id).order_by("id").values_list("id", flat=True)) + self.assertEqual(ArchivedAttachment.objects.count(), 0) + move_message_to_archive(message_id=msg_id) + self._check_messages_after_archiving(msg_id, user_messages_ids_before) + self.assertEqual(Attachment.objects.count(), 3) + arc_attachments_ids_after = list(ArchivedAttachment.objects.filter( + messages__id=msg_id).order_by("id").values_list("id", flat=True)) + self.assertEqual(attachments_ids_before, arc_attachments_ids_after) + move_message_to_archive(message_id=msg_id_shared_attachments) + self.assertEqual(Attachment.objects.count(), 0) diff --git a/zerver/tests/test_templates.py b/zerver/tests/test_templates.py index 78ead37594..45f2e043fa 100644 --- a/zerver/tests/test_templates.py +++ b/zerver/tests/test_templates.py @@ -83,6 +83,7 @@ class TemplateTestCase(ZulipTestCase): 'zerver/subscriptions.html', 'zerver/tutorial_finale.html', 'zerver/message_history.html', + 'zerver/delete_message.html', ] unusual = [ 'zerver/emails/confirm_registration_mit.txt', diff --git a/zerver/tornado/event_queue.py b/zerver/tornado/event_queue.py index cdde60888a..fd7c796b12 100644 --- a/zerver/tornado/event_queue.py +++ b/zerver/tornado/event_queue.py @@ -799,7 +799,7 @@ def process_notification(notice): # type: (Mapping[str, Any]) -> None event = notice['event'] # type: Mapping[str, Any] users = notice['users'] # type: Union[Iterable[int], Iterable[Mapping[str, Any]]] - if event['type'] in ["update_message"]: + if event['type'] in ["update_message", "delete_message"]: process_userdata_event(event, cast(Iterable[Mapping[str, Any]], users)) elif event['type'] == "message": process_message_event(event, cast(Iterable[Mapping[str, Any]], users)) diff --git a/zerver/views/messages.py b/zerver/views/messages.py index 2a6a2882df..97d3a0fe7c 100644 --- a/zerver/views/messages.py +++ b/zerver/views/messages.py @@ -21,7 +21,7 @@ from zerver.lib import bugdown from zerver.lib.actions import recipient_for_emails, do_update_message_flags, \ compute_mit_user_fullname, 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 + extract_recipients, truncate_body, render_incoming_message, do_delete_message from zerver.lib.queue import queue_json_publish from zerver.lib.cache import ( generic_bulk_cached_fetch, @@ -1071,6 +1071,16 @@ def update_message_backend(request, user_profile, queue_json_publish('embed_links', event_data, lambda x: None) return json_success() + +@has_request_variables +def delete_message_backend(request, user_profile, message_id=REQ(converter=to_non_negative_int)): + # type: (HttpRequest, UserProfile, int) -> HttpResponse + message, ignored_user_message = access_message(user_profile, message_id) + if not user_profile.is_realm_admin: + raise JsonableError(_("You don't have permission to edit this message")) + do_delete_message(user_profile, message) + return json_success() + @has_request_variables def json_fetch_raw_message(request, user_profile, message_id=REQ(converter=to_non_negative_int)): diff --git a/zproject/urls.py b/zproject/urls.py index e5e6ea3f9f..2e519b737c 100644 --- a/zproject/urls.py +++ b/zproject/urls.py @@ -247,7 +247,8 @@ v1_api_and_json_patterns = [ 'POST': 'zerver.views.messages.send_message_backend'}), url(r'^messages/(?P[0-9]+)$', rest_dispatch, {'GET': 'zerver.views.messages.json_fetch_raw_message', - 'PATCH': 'zerver.views.messages.update_message_backend'}), + 'PATCH': 'zerver.views.messages.update_message_backend', + 'DELETE': 'zerver.views.messages.delete_message_backend'}), url(r'^messages/render$', rest_dispatch, {'POST': 'zerver.views.messages.render_message_backend'}), url(r'^messages/flags$', rest_dispatch,