From 9a5179c460669bf67c76b82050a60ec703474cbf Mon Sep 17 00:00:00 2001 From: paxapy Date: Wed, 28 Dec 2016 16:46:42 +0300 Subject: [PATCH] Add support for managing and deleting attachments. Modified substantially by tabbott to fix tons of issues. Fixes #454. --- .eslintrc.json | 1 + frontend_tests/node_tests/templates.js | 17 +++++ static/js/attachments_ui.js | 30 ++++++++ static/js/settings.js | 1 + static/styles/settings.css | 4 +- .../settings/attachment-item.handlebars | 24 +++++++ .../settings/attachments-settings.handlebars | 12 ++++ static/templates/settings_tab.handlebars | 2 + templates/zerver/settings_overlay.html | 4 ++ zerver/lib/attachments.py | 29 ++++++++ zerver/lib/events.py | 4 ++ zerver/models.py | 13 ++++ zerver/tests/test_attachments.py | 68 +++++++++++++++++++ zerver/tests/tests.py | 1 + zerver/views/attachments.py | 22 ++++++ zerver/views/home.py | 1 + zproject/settings.py | 1 + zproject/urls.py | 6 ++ 18 files changed, 238 insertions(+), 2 deletions(-) create mode 100644 static/js/attachments_ui.js create mode 100644 static/templates/settings/attachment-item.handlebars create mode 100644 static/templates/settings/attachments-settings.handlebars create mode 100644 zerver/lib/attachments.py create mode 100644 zerver/tests/test_attachments.py create mode 100644 zerver/views/attachments.py diff --git a/.eslintrc.json b/.eslintrc.json index 408d16af79..afd1dfa56c 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -19,6 +19,7 @@ "page_params": false, "status_classes": false, "password_quality": false, + "attachments_ui": false, "csrf_token": false, "typeahead_helper": false, "popovers": false, diff --git a/frontend_tests/node_tests/templates.js b/frontend_tests/node_tests/templates.js index 46c02087eb..961ff63b7e 100644 --- a/frontend_tests/node_tests/templates.js +++ b/frontend_tests/node_tests/templates.js @@ -42,6 +42,7 @@ function render(template_name, args) { 'notification-settings', 'bot-settings', 'alert-word-settings', + 'attachments-settings', 'ui-settings', ]); }()); @@ -252,6 +253,22 @@ function render(template_name, args) { global.write_handlebars_output("announce_stream_docs", html); }()); +(function attachment_settings_item() { + var html = '"; + global.write_handlebars_output("attachment-item", html); + var li = $(html).find("li.attachment-item:first"); + assert.equal(li.attr('data-attachment'), 42); +}()); + (function bankruptcy_modal() { var args = { unread_count: 99, diff --git a/static/js/attachments_ui.js b/static/js/attachments_ui.js new file mode 100644 index 0000000000..7ab1e45422 --- /dev/null +++ b/static/js/attachments_ui.js @@ -0,0 +1,30 @@ +var attachments_ui = (function () { + +var exports = {}; + +function delete_attachments(attachment) { + channel.del({url: '/json/attachments/' + attachment, idempotent: true}); +} + +exports.set_up_attachments = function () { + // The settings page must be rendered before this function gets called. + + var attachment_list = $('#attachments_list'); + _.each(page_params.attachments, function (attachment) { + var li = templates.render('attachment-item', {attachment: attachment}); + attachment_list.append(li); + }); + + $('#attachments_list').on('click', '.remove-attachment', function (event) { + var li = $(event.currentTarget).parents('li'); + li.remove(); + delete_attachments($(this).data('attachment')); + }); +}; + +return exports; +}()); + +if (typeof module !== 'undefined') { + module.exports = attachments_ui; +} diff --git a/static/js/settings.js b/static/js/settings.js index 63dd15b513..bbcc1c18d8 100644 --- a/static/js/settings.js +++ b/static/js/settings.js @@ -199,6 +199,7 @@ function _setup_page() { $("#ui-settings-status").hide(); alert_words_ui.set_up_alert_words(); + attachments_ui.set_up_attachments(); $("#api_key_value").text(""); $("#get_api_key_box").hide(); diff --git a/static/styles/settings.css b/static/styles/settings.css index 4d4ac4e023..524f48cf6a 100644 --- a/static/styles/settings.css +++ b/static/styles/settings.css @@ -462,7 +462,7 @@ input[type=checkbox].inline-block { position: inherit; } -#alert_words_list { +#alert_words_list, #attachments_list { list-style-type: none; margin: auto; } @@ -472,7 +472,7 @@ input[type=checkbox].inline-block { margin-top: 8px; } -#alert_words_list .edit-alert-word-buttons { +#alert_words_list .edit-alert-word-buttons, #attachments_list .edit-attachment-buttons { position: absolute; right: 20px; top: 5px; diff --git a/static/templates/settings/attachment-item.handlebars b/static/templates/settings/attachment-item.handlebars new file mode 100644 index 0000000000..77a268bafc --- /dev/null +++ b/static/templates/settings/attachment-item.handlebars @@ -0,0 +1,24 @@ + +
  • +
    +
    + {{attachment.name}} +
    +
    + +
    + {{#if attachment.messages }} + + {{/if}} +
    +
    • diff --git a/static/templates/settings/attachments-settings.handlebars b/static/templates/settings/attachments-settings.handlebars new file mode 100644 index 0000000000..deb21bf048 --- /dev/null +++ b/static/templates/settings/attachments-settings.handlebars @@ -0,0 +1,12 @@ +
    +
    + + {{t "Uploaded files" }} +
    +
    +

    + {{t 'For each file, we list any messages that link to it.' }} +

    +
    + +
    diff --git a/static/templates/settings_tab.handlebars b/static/templates/settings_tab.handlebars index 77a6d5e984..0bb3535946 100644 --- a/static/templates/settings_tab.handlebars +++ b/static/templates/settings_tab.handlebars @@ -11,5 +11,7 @@ {{ partial "alert-word-settings" }} + {{ partial "attachments-settings" }} + {{ partial "ui-settings" }} diff --git a/templates/zerver/settings_overlay.html b/templates/zerver/settings_overlay.html index 5f48351cb2..412433ff86 100644 --- a/templates/zerver/settings_overlay.html +++ b/templates/zerver/settings_overlay.html @@ -28,6 +28,10 @@
    {{ _('Custom alert words') }}
    +
  • +
    +
    {{ _('Uploaded files') }}
    +
  • {{ _('Zulip labs') }}
    diff --git a/zerver/lib/attachments.py b/zerver/lib/attachments.py new file mode 100644 index 0000000000..2c699340a2 --- /dev/null +++ b/zerver/lib/attachments.py @@ -0,0 +1,29 @@ +from __future__ import absolute_import + +from django.utils.translation import ugettext as _ +from typing import Any, Dict, List + +from zerver.lib.request import JsonableError +from zerver.lib.upload import delete_message_image +from zerver.models import Attachment, UserProfile + +def user_attachments(user_profile): + # type: (UserProfile) -> List[Dict[str, Any]] + attachments = Attachment.objects.filter(owner=user_profile).prefetch_related('messages') + return [a.to_dict() for a in attachments] + +def access_attachment_by_id(user_profile, attachment_id, needs_owner=False): + # type: (UserProfile, int, bool) -> Attachment + query = Attachment.objects.filter(id=attachment_id) + if needs_owner: + query = query.filter(owner=user_profile) + + attachment = query.first() + if attachment is None: + raise JsonableError(_("Invalid attachment")) + return attachment + +def remove_attachment(user_profile, attachment): + # type: (UserProfile, Attachment) -> None + delete_message_image(attachment.path_id) + attachment.delete() diff --git a/zerver/lib/events.py b/zerver/lib/events.py index dedd77dc09..9531236841 100644 --- a/zerver/lib/events.py +++ b/zerver/lib/events.py @@ -18,6 +18,7 @@ from typing import ( session_engine = import_module(settings.SESSION_ENGINE) from zerver.lib.alert_words import user_alert_words +from zerver.lib.attachments import user_attachments from zerver.lib.narrow import check_supported_events_narrow_filter from zerver.lib.request import JsonableError from zerver.lib.actions import validate_user_access_to_subscribers_helper, \ @@ -54,6 +55,9 @@ def fetch_initial_state_data(user_profile, event_types, queue_id): if want('alert_words'): state['alert_words'] = user_alert_words(user_profile) + if want('attachments'): + state['attachments'] = user_attachments(user_profile) + if want('message'): # The client should use get_old_messages() to fetch messages # starting with the max_message_id. They will get messages diff --git a/zerver/models.py b/zerver/models.py index d4b1a9984b..212f144be1 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -1140,6 +1140,19 @@ class Attachment(ModelReprMixin, models.Model): # type: () -> bool return self.messages.count() > 0 + def to_dict(self): + # type: () -> Dict[str, Any] + return { + 'id': self.id, + 'name': self.file_name, + 'path_id': self.path_id, + 'messages': [{ + 'id': m.id, + 'name': '{m.pub_date:%Y-%m-%d %H:%M} {recipient}/{m.subject}'.format( + recipient=get_display_recipient(m.recipient), m=m) + } for m in self.messages.all()] + } + def get_old_unclaimed_attachments(weeks_ago): # type: (int) -> Sequence[Attachment] # TODO: Change return type to QuerySet[Attachment] diff --git a/zerver/tests/test_attachments.py b/zerver/tests/test_attachments.py new file mode 100644 index 0000000000..1291cd7345 --- /dev/null +++ b/zerver/tests/test_attachments.py @@ -0,0 +1,68 @@ +# -*- coding: utf-8 -*- +from __future__ import absolute_import +from __future__ import print_function + +import mock +import ujson + +from typing import Any + +from zerver.lib.attachments import user_attachments +from zerver.lib.test_helpers import get_user_profile_by_email +from zerver.lib.test_classes import ZulipTestCase +from zerver.models import Attachment + + +class AttachmentsTests(ZulipTestCase): + def setUp(self): + # type: () -> None + user = get_user_profile_by_email("cordelia@zulip.com") + self.attachment = Attachment.objects.create( + file_name='test.txt', path_id='foo/bar/test.txt', owner=user) + + def test_list_by_user(self): + # type: () -> None + self.login("cordelia@zulip.com") + result = self.client_get('/json/attachments') + self.assert_json_success(result) + user = get_user_profile_by_email("cordelia@zulip.com") + attachments = user_attachments(user) + data = ujson.loads(result.content) + self.assertEqual(data['attachments'], attachments) + + @mock.patch('zerver.lib.attachments.delete_message_image') + def test_remove_attachment(self, ignored): + # type: (Any) -> None + self.login("cordelia@zulip.com") + result = self.client_delete('/json/attachments/{pk}'.format(pk=self.attachment.pk)) + self.assert_json_success(result) + user = get_user_profile_by_email("cordelia@zulip.com") + attachments = user_attachments(user) + self.assertEqual(attachments, []) + + def test_list_another_user(self): + # type: () -> None + self.login("iago@zulip.com") + result = self.client_get('/json/attachments') + self.assert_json_success(result) + data = ujson.loads(result.content) + self.assertEqual(data['attachments'], []) + + def test_remove_another_user(self): + # type: () -> None + self.login("iago@zulip.com") + result = self.client_delete('/json/attachments/{pk}'.format(pk=self.attachment.pk)) + self.assert_json_error(result, 'Invalid attachment') + user = get_user_profile_by_email("cordelia@zulip.com") + attachments = user_attachments(user) + self.assertEqual(attachments, [self.attachment.to_dict()]) + + def test_list_unauthenticated(self): + # type: () -> None + result = self.client_get('/json/attachments') + self.assert_json_error(result, 'Not logged in: API authentication or user session required', status_code=401) + + def test_delete_unauthenticated(self): + # type: () -> None + result = self.client_delete('/json/attachments/{pk}'.format(pk=self.attachment.pk)) + self.assert_json_error(result, 'Not logged in: API authentication or user session required', status_code=401) diff --git a/zerver/tests/tests.py b/zerver/tests/tests.py index 4efc5ee0fa..845313b4b0 100644 --- a/zerver/tests/tests.py +++ b/zerver/tests/tests.py @@ -1856,6 +1856,7 @@ class HomeTest(ZulipTestCase): # Keep this list sorted!!! expected_keys = [ "alert_words", + "attachments", "autoscroll_forever", "avatar_source", "avatar_url", diff --git a/zerver/views/attachments.py b/zerver/views/attachments.py new file mode 100644 index 0000000000..5beb094ab4 --- /dev/null +++ b/zerver/views/attachments.py @@ -0,0 +1,22 @@ +from __future__ import absolute_import +from django.http import HttpRequest, HttpResponse + +from zerver.decorator import REQ +from zerver.models import UserProfile +from zerver.lib.validator import check_int +from zerver.lib.response import json_success +from zerver.lib.attachments import user_attachments, remove_attachment, \ + access_attachment_by_id + + +def list_by_user(request, user_profile): + # type: (HttpRequest, UserProfile) -> HttpResponse + return json_success({"attachments": user_attachments(user_profile)}) + + +def remove(request, user_profile, attachment_id=REQ(validator=check_int)): + # type: (HttpRequest, UserProfile, int) -> HttpResponse + attachment = access_attachment_by_id(user_profile, attachment_id, + needs_owner=True) + remove_attachment(user_profile, attachment) + return json_success() diff --git a/zerver/views/home.py b/zerver/views/home.py index 99e6c140bc..7f7d418a14 100644 --- a/zerver/views/home.py +++ b/zerver/views/home.py @@ -264,6 +264,7 @@ def home_real(request): furthest_read_time = sent_time_in_epoch_seconds(latest_read), save_stacktraces = settings.SAVE_FRONTEND_STACKTRACES, alert_words = register_ret['alert_words'], + attachments = register_ret['attachments'], muted_topics = register_ret['muted_topics'], realm_filters = register_ret['realm_filters'], realm_default_streams = register_ret['realm_default_streams'], diff --git a/zproject/settings.py b/zproject/settings.py index 7987998528..9d368b9b09 100644 --- a/zproject/settings.py +++ b/zproject/settings.py @@ -852,6 +852,7 @@ JS_SPECS = { 'js/message_flags.js', 'js/alert_words.js', 'js/alert_words_ui.js', + 'js/attachments_ui.js', 'js/message_store.js', 'js/server_events.js', 'js/zulip.js', diff --git a/zproject/urls.py b/zproject/urls.py index b26393a189..11963bd8f4 100644 --- a/zproject/urls.py +++ b/zproject/urls.py @@ -228,6 +228,12 @@ v1_api_and_json_patterns = [ {'PUT': 'zerver.views.reactions.add_reaction_backend', 'DELETE': 'zerver.views.reactions.remove_reaction_backend'}), + # attachments -> zerver.views.attachments + url(r'^attachments$', rest_dispatch, + {'GET': 'zerver.views.attachments.list_by_user'}), + url(r'^attachments/(?P[0-9]+)$', rest_dispatch, + {'DELETE': 'zerver.views.attachments.remove'}), + # typing -> zerver.views.typing # POST sends a typing notification event to recipients url(r'^typing$', rest_dispatch,