messages: Add support for admins deleting messages.

This makes it possible for Zulip administrators to delete messages.
This is primarily intended for use in deleting early test messages,
but it can solve other problems as well.

Later we'll want to play with the permissions model for this, but for
now, the goal is just to integrate the feature.

Note that it saves the deleted messages for some time using the same
approach as Zulip's message retention policy feature.

Fixes #135.
This commit is contained in:
K.Kanakhin 2017-05-15 01:14:26 +06:00 committed by Tim Abbott
parent 9b13b19006
commit 2434f2d96c
18 changed files with 377 additions and 8 deletions

View File

@ -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();
});

View File

@ -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) {

View File

@ -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);
}

View File

@ -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;
}
}

View File

@ -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) {

View File

@ -28,7 +28,14 @@
</a>
</li>
{{/if}}
{{#if should_display_delete_option}}
<li>
<a href="#" class="delete_message" data-message-id="{{message.id}}">
<i class="icon-vector-remove"></i>
{{t "Delete message" }}
</a>
</li>
{{/if}}
{{#if can_mute_topic}}
<li>
<a href="#" class="popover_mute_topic" data-msg-stream="{{message.stream}}" data-msg-topic="{{message.subject}}">

View File

@ -0,0 +1,16 @@
<div id="delete_message_modal" class="modal hide fade new-style" tabindex="-1" role="dialog" aria-labelledby="delete_message_modal_label" aria-hidden="true">
<div class="modal-header">
<button type="button" class="close" data-dismiss="modal" aria-hidden="true">×</button>
<h3 id="delete_message_modal_label">{{ _("Delete message") }} </h3>
</div>
<div class="modal-body">
<div class="content">
<p><strong>{{ _("Are you sure you want to delete this message?") }}</strong></p>
</div>
<div id="delete-message-error"></div>
</div>
<div class="modal-footer">
<button class="button white rounded" data-dismiss="modal" aria-hidden="true">{{ _("Cancel") }}</button>
<button class="button white rounded btn-danger" id="do_delete_message_button">{{ _("Yes, delete this message") }}</button>
</div>
</div>

View File

@ -91,6 +91,7 @@
{% include "zerver/message_history.html" %}
{% include "zerver/delete_message.html" %}
{% include "zerver/compose.html" %}
<div id="notifications-area">
</div>

View File

@ -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)

View File

@ -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

View File

@ -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()

View File

@ -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):

View File

@ -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)")

View File

@ -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)

View File

@ -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',

View File

@ -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))

View File

@ -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)):

View File

@ -247,7 +247,8 @@ v1_api_and_json_patterns = [
'POST': 'zerver.views.messages.send_message_backend'}),
url(r'^messages/(?P<message_id>[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,