From 736a20527384e502f073c4a8b4d62b212ace249c Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Mon, 9 Nov 2015 14:24:46 -0500 Subject: [PATCH 01/41] Add missing info to comment serializer Add can_edit, has_children and is_abuse fields to the API v2 comment serializer. --- api/comments/serializers.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/api/comments/serializers.py b/api/comments/serializers.py index 138e02fcf0e..adad71b31b8 100644 --- a/api/comments/serializers.py +++ b/api/comments/serializers.py @@ -41,6 +41,9 @@ class CommentSerializer(JSONAPISerializer): date_modified = ser.DateTimeField(read_only=True) modified = ser.BooleanField(read_only=True, default=False) deleted = ser.BooleanField(read_only=True, source='is_deleted', default=False) + is_abuse = ser.SerializerMethodField() + has_children = ser.SerializerMethodField() + can_edit = ser.SerializerMethodField() # LinksField.to_representation adds link to "self" links = LinksField({}) @@ -48,6 +51,17 @@ class CommentSerializer(JSONAPISerializer): class Meta: type_ = 'comments' + def get_is_abuse(self, obj): + auth = Auth(self.context['request'].user) + return auth.user and auth.user._id in obj.reports + + def get_can_edit(self, obj): + auth = Auth(self.context['request'].user) + return obj.user == auth.user + + def get_has_children(self, obj): + return bool(getattr(obj, 'commented', [])) + def create(self, validated_data): user = validated_data['user'] auth = Auth(user) From 5e0f5b15f705df709f352a5a7fb70908b921b874 Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Mon, 9 Nov 2015 14:30:42 -0500 Subject: [PATCH 02/41] Use apiV2 to fetch list of node comments Since the v2 comment serializer does not include serializied information about the commenter (and embedded requests have not yet been implemented), iterate through the serialized comments, make a request to get commenter info and then create a CommentModel based on the updated response. --- website/static/css/commentpane.css | 4 ++ website/static/js/comment.js | 65 ++++++++++++++----- .../templates/include/comment_template.mako | 2 +- 3 files changed, 54 insertions(+), 17 deletions(-) diff --git a/website/static/css/commentpane.css b/website/static/css/commentpane.css index 6d82aae7031..c15a63a90a7 100644 --- a/website/static/css/commentpane.css +++ b/website/static/css/commentpane.css @@ -53,4 +53,8 @@ color: #FFF; position: absolute; margin-left: 40px; +} + +.comment-gravatar { + height: 20px; } \ No newline at end of file diff --git a/website/static/js/comment.js b/website/static/js/comment.js index a9795fa1710..b93f5772722 100644 --- a/website/static/js/comment.js +++ b/website/static/js/comment.js @@ -82,6 +82,7 @@ var BaseComment = function() { self.submittingReply = ko.observable(false); self.comments = ko.observableArray(); + self.unreadComments = ko.observable(0); self.displayCount = ko.computed(function() { @@ -138,23 +139,42 @@ BaseComment.prototype.fetch = function() { if (self._loaded) { deferred.resolve(self.comments()); } - $.getJSON( - nodeApiUrl + 'comments/', - {target: self.id()}, - function(response) { - self.comments( - ko.utils.arrayMap(response.comments.reverse(), function(comment) { - return new CommentModel(comment, self, self.$root); - }) - ); - self.unreadComments(response.nUnread); - deferred.resolve(self.comments()); - self._loaded = true; + var url = osfHelpers.apiV2Url('nodes/' + window.contextVars.node.id + '/comments/', {}); + if (self.id() !== undefined) { + url = osfHelpers.apiV2Url('comments/' + self.id() + '/replies/', {}); + } + var request = osfHelpers.ajaxJSON( + 'GET', + url, + {'isCors': true}); + request.done(function(response) { + for (var i=0; i < response.data.length; i++) { + updateCommentUserData(response.data[i], self); } - ); +// self.unreadComments(response.nUnread); + deferred.resolve(self.comments()); + self._loaded = true; + }); return deferred; }; +var updateCommentUserData = function(commentJSON, self) { + var userRequest = osfHelpers.ajaxJSON( + 'GET', + commentJSON.relationships.user.links.related.href, + {'isCors': true}); + userRequest.done(function(response) { + commentJSON.relationships.user = response; + var commentModel = new CommentModel(commentJSON, self, self.$root); + self.comments.push(commentModel); + self.comments.sort(function (left, right) { + return left.dateCreated() === right.dateCreated() ? 0 : (left.dateCreated() > right.dateCreated() ? -1 : 1); + }); + }); + return self.comments; +}; + + BaseComment.prototype.submitReply = function() { var self = this; if (!self.replyContent()) { @@ -198,13 +218,26 @@ var CommentModel = function(data, $parent, $root) { BaseComment.prototype.constructor.call(this); var self = this; + var userData = data.relationships.user.data; self.$parent = $parent; self.$root = $root; - // Note: assigns observables: canEdit, content, dateCreated, dateModified - // hasChildren, id, isAbuse, isDeleted. Leaves out author. - $.extend(self, koHelpers.mapJStoKO(data, {exclude: ['author']})); + self.id = ko.observable(data.id); + self.content = ko.observable(data.attributes.content); + self.dateCreated = ko.observable(data.attributes.date_created); + self.dateModified = ko.observable(data.attributes.date_modified); + self.isDeleted = ko.observable(data.attributes.deleted); + self.modified = ko.observable(data.attributes.modified); + self.isAbuse = ko.observable(data.attributes.is_abuse); + self.canEdit = ko.observable(data.attributes.can_edit); + self.hasChildren = ko.observable(data.attributes.has_children); + self.author = { + 'id': data.relationships.user.data.id, + 'url': userData.links.html, + 'name': userData.attributes.full_name, + 'gravatarUrl': userData.links.profile_image + }; self.contentDisplay = ko.observable(markdown.full.render(self.content())); diff --git a/website/templates/include/comment_template.mako b/website/templates/include/comment_template.mako index 10087bf74a7..7fb0185e2f4 100644 --- a/website/templates/include/comment_template.mako +++ b/website/templates/include/comment_template.mako @@ -80,7 +80,7 @@
- + From db0809c5ddc43a16e7a12fe3331b8a1caf734eec Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Mon, 9 Nov 2015 14:58:34 -0500 Subject: [PATCH 03/41] Make creating comments/replies use v2 API --- website/static/js/comment.js | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/website/static/js/comment.js b/website/static/js/comment.js index b93f5772722..3d240eae352 100644 --- a/website/static/js/comment.js +++ b/website/static/js/comment.js @@ -186,16 +186,30 @@ BaseComment.prototype.submitReply = function() { return; } self.submittingReply(true); - osfHelpers.postJSON( - nodeApiUrl + 'comment/', + var url = osfHelpers.apiV2Url('nodes/' + window.contextVars.node.id + '/comments/', {}); + if (self.id() !== undefined) { + url = osfHelpers.apiV2Url('comments/' + self.id() + '/replies/', {}); + } + var request = osfHelpers.ajaxJSON( + 'POST', + url, { - target: self.id(), - content: self.replyContent(), - } - ).done(function(response) { + 'isCors': true, + 'data': { + 'data': { + 'type': 'comments', + 'attributes': { + 'content': self.replyContent() + } + } + } + }); + request.done(function(response) { self.cancelReply(); self.replyContent(null); - self.comments.unshift(new CommentModel(response.comment, self, self.$root)); + self.onSubmitSuccess(response); + updateCommentUserData(response.data, self); +// self.comments.unshift(new CommentModel(response.comment, self, self.$root)); if (!self.hasChildren()) { self.hasChildren(true); } @@ -206,8 +220,8 @@ BaseComment.prototype.submitReply = function() { self.$root.fetchDiscussion(); self.$root.commented(true); } - self.onSubmitSuccess(response); - }).fail(function() { + }); + request.fail(function() { self.cancelReply(); self.errorMessage('Could not submit comment'); }); From 125db9fb9ad3080a93dd197302161e8c0e7eaadc Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Mon, 9 Nov 2015 15:32:22 -0500 Subject: [PATCH 04/41] Use api v2 route to edit comment --- website/static/js/comment.js | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/website/static/js/comment.js b/website/static/js/comment.js index 3d240eae352..647942d25cf 100644 --- a/website/static/js/comment.js +++ b/website/static/js/comment.js @@ -255,7 +255,7 @@ var CommentModel = function(data, $parent, $root) { self.contentDisplay = ko.observable(markdown.full.render(self.content())); - // Update contentDisplay with rednered markdown whenever content changes + // Update contentDisplay with rendered markdown whenever content changes self.content.subscribe(function(newContent) { self.contentDisplay(markdown.full.render(newContent)); }); @@ -336,19 +336,33 @@ CommentModel.prototype.submitEdit = function(data, event) { self.errorMessage('Please enter a comment'); return; } - osfHelpers.putJSON( - nodeApiUrl + 'comment/' + self.id() + '/', - {content: self.content()} - ).done(function(response) { - self.content(response.content); - self.dateModified(response.dateModified); + var request = osfHelpers.ajaxJSON( + 'PUT', + osfHelpers.apiV2Url('comments/' + self.id() + '/', {}), + { + 'isCors': true, + 'data': { + 'data': { + 'id': self.id(), + 'type': 'comments', + 'attributes': { + 'content': self.content(), + 'deleted': false + } + } + } + }); + request.done(function(response) { + self.content(response.data.attributes.content); + self.dateModified(response.data.attributes.date_modified); self.editing(false); self.modified(true); self.editErrorMessage(''); self.$root.editors -= 1; // Refresh tooltip on date modified, if present $tips.tooltip('destroy').tooltip(); - }).fail(function() { + }); + request.fail(function() { self.cancelEdit(); self.errorMessage('Could not submit comment'); }); From bf351ad3b3eda767d642139c94171b79c42c9a37 Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Mon, 9 Nov 2015 15:40:55 -0500 Subject: [PATCH 05/41] Use v2 API to delete/undelete comments --- website/static/js/comment.js | 46 ++++++++++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 10 deletions(-) diff --git a/website/static/js/comment.js b/website/static/js/comment.js index 647942d25cf..8bcdfdcd7b6 100644 --- a/website/static/js/comment.js +++ b/website/static/js/comment.js @@ -399,13 +399,26 @@ CommentModel.prototype.startDelete = function() { CommentModel.prototype.submitDelete = function() { var self = this; - $.ajax({ - type: 'DELETE', - url: nodeApiUrl + 'comment/' + self.id() + '/', - }).done(function() { + var request = osfHelpers.ajaxJSON( + 'PATCH', + osfHelpers.apiV2Url('comments/' + self.id() + '/', {}), + { + 'isCors': true, + 'data': { + 'data': { + 'id': self.id(), + 'type': 'comments', + 'attributes': { + 'deleted': true + } + } + } + }); + request.done(function() { self.isDeleted(true); self.deleting(false); - }).fail(function() { + }); + request.fail(function() { self.deleting(false); }); }; @@ -420,12 +433,25 @@ CommentModel.prototype.startUndelete = function() { CommentModel.prototype.submitUndelete = function() { var self = this; - osfHelpers.putJSON( - nodeApiUrl + 'comment/' + self.id() + '/undelete/', - {} - ).done(function() { + var request = osfHelpers.ajaxJSON( + 'PATCH', + osfHelpers.apiV2Url('comments/' + self.id() + '/', {}), + { + 'isCors': true, + 'data': { + 'data': { + 'id': self.id(), + 'type': 'comments', + 'attributes': { + 'deleted': false + } + } + } + }); + request.done(function() { self.isDeleted(false); - }).fail(function() { + }); + request.fail(function() { self.undeleting(false); }); }; From 0bd2354b96f0a309bf86afa445733c58458ec58b Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Mon, 9 Nov 2015 16:10:11 -0500 Subject: [PATCH 06/41] Use v2 API to report/unreport comments --- website/static/js/comment.js | 37 +++++++++++++++++--------- website/templates/project/project.mako | 1 + 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/website/static/js/comment.js b/website/static/js/comment.js index 8bcdfdcd7b6..296f16bc521 100644 --- a/website/static/js/comment.js +++ b/website/static/js/comment.js @@ -380,15 +380,25 @@ CommentModel.prototype.cancelAbuse = function() { CommentModel.prototype.submitAbuse = function() { var self = this; - osfHelpers.postJSON( - nodeApiUrl + 'comment/' + self.id() + '/report/', + var request = osfHelpers.ajaxJSON( + 'POST', + osfHelpers.apiV2Url('comments/' + self.id() + '/reports/', {}), { - category: self.abuseCategory(), - text: self.abuseText() - } - ).done(function() { + 'isCors': true, + 'data': { + 'data': { + 'type': 'comment_reports', + 'attributes': { + 'category': self.abuseCategory(), + 'message': self.abuseText() + } + } + } + }); + request.done(function() { self.isAbuse(true); - }).fail(function() { + }); + request.fail(function() { self.errorMessage('Could not report abuse.'); }); }; @@ -466,12 +476,15 @@ CommentModel.prototype.startUnreportAbuse = function() { CommentModel.prototype.submitUnreportAbuse = function() { var self = this; - osfHelpers.postJSON( - nodeApiUrl + 'comment/' + self.id() + '/unreport/', - {} - ).done(function() { + var request = osfHelpers.ajaxJSON( + 'DELETE', + osfHelpers.apiV2Url('comments/' + self.id() + '/reports/' + window.contextVars.currentUser.id + '/', {}), + {'isCors': true} + ); + request.done(function() { self.isAbuse(false); - }).fail(function() { + }); + request.fail(function() { self.unreporting(false); }); }; diff --git a/website/templates/project/project.mako b/website/templates/project/project.mako index abed189ea5b..2a928a5eb6c 100644 --- a/website/templates/project/project.mako +++ b/website/templates/project/project.mako @@ -368,6 +368,7 @@ ${parent.javascript_bottom()} // Hack to allow mako variables to be accessed to JS modules window.contextVars = $.extend(true, {}, window.contextVars, { currentUser: { + id: ${user['id'] | sjson, n}, name: ${ user_full_name | sjson, n }, canComment: ${ user['can_comment'] | sjson, n }, canEdit: ${ user['can_edit'] | sjson, n } From 5f868c871b8ff9936ce228d9e13255bcb3a26b16 Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Tue, 10 Nov 2015 11:34:38 -0500 Subject: [PATCH 07/41] Add max_length to comment content field --- api/comments/serializers.py | 3 ++- .../nodes/views/test_node_comments_list.py | 21 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/api/comments/serializers.py b/api/comments/serializers.py index adad71b31b8..f542a30c54c 100644 --- a/api/comments/serializers.py +++ b/api/comments/serializers.py @@ -4,6 +4,7 @@ from rest_framework.exceptions import ValidationError, PermissionDenied from api.base.exceptions import InvalidModelValueError from api.base.utils import absolute_reverse +from api.base.settings import osf_settings from api.base.serializers import (JSONAPISerializer, JSONAPIHyperlinkedRelatedField, JSONAPIHyperlinkedGuidRelatedField, @@ -29,7 +30,7 @@ class CommentSerializer(JSONAPISerializer): id = IDField(source='_id', read_only=True) type = TypeField() - content = AuthorizedCharField(source='get_content') + content = AuthorizedCharField(source='get_content', max_length=osf_settings.COMMENT_MAXLENGTH) user = JSONAPIHyperlinkedRelatedField(view_name='users:user-detail', lookup_field='pk', lookup_url_kwarg='user_id', link_type='related', read_only=True) node = JSONAPIHyperlinkedRelatedField(view_name='nodes:node-detail', lookup_field='pk', lookup_url_kwarg='node_id', link_type='related', read_only=True) diff --git a/api_tests/nodes/views/test_node_comments_list.py b/api_tests/nodes/views/test_node_comments_list.py index 1bba3882a79..869dee4fb0e 100644 --- a/api_tests/nodes/views/test_node_comments_list.py +++ b/api_tests/nodes/views/test_node_comments_list.py @@ -4,6 +4,7 @@ from framework.auth import core from api.base.settings.defaults import API_BASE +from api.base.settings import osf_settings from tests.base import ApiTestCase from tests.factories import ( ProjectFactory, @@ -182,6 +183,26 @@ def test_create_comment_invalid_target_node(self): assert_equal(res.status_code, 404) assert_equal(res.json['errors'][0]['detail'], 'Not found.') + def test_create_comment_exceeds_max_length(self): + self._set_up_private_project() + payload = { + 'data': { + 'type': 'comments', + 'attributes': { + 'content': ('contentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontent' + + 'contentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcon' + + 'tentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentconten' + + 'tcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentco' + + 'ntentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentconte' + + 'ntcontentcontentcontentcontentcontentcontent') + } + } + } + res = self.app.post_json_api(self.private_url, payload, auth=self.user.auth, expect_errors=True) + assert_equal(res.status_code, 400) + assert_equal(res.json['errors'][0]['detail'], + 'Ensure this field has no more than ' + str(osf_settings.COMMENT_MAXLENGTH) + ' characters.') + def test_private_node_logged_in_admin_can_comment(self): self._set_up_private_project() res = self.app.post_json_api(self.private_url, self.payload, auth=self.user.auth) From 515c5b1d09e2c7571f500c32acecdbdf2f49f47c Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Tue, 10 Nov 2015 14:36:09 -0500 Subject: [PATCH 08/41] Send signal to call notify when commenting Because the v1 route is being removed, move the logic for calling notify when a comment is added to a separate function that receives the 'comment-added' signal sent from Comment.create. --- tests/test_models.py | 13 ++++++++++++- website/project/model.py | 1 + website/project/signals.py | 1 + website/project/views/comment.py | 16 +++++++++++----- website/signals.py | 1 + 5 files changed, 26 insertions(+), 6 deletions(-) diff --git a/tests/test_models.py b/tests/test_models.py index 0dfd996f429..6d7cd4f1154 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -31,7 +31,7 @@ from website import filters, language, settings, mailchimp_utils from website.exceptions import NodeStateError from website.profile.utils import serialize_user -from website.project.signals import contributor_added +from website.project.signals import contributor_added, comment_added from website.project.model import ( Comment, Node, NodeLog, Pointer, ensure_schemas, has_anonymous_link, get_pointer_parent, Embargo, @@ -4052,6 +4052,17 @@ def test_create(self): assert_equal(len(comment.node.logs), 2) assert_equal(comment.node.logs[-1].action, NodeLog.COMMENT_ADDED) + def test_create_sends_comment_added_signal(self): + with capture_signals() as mock_signals: + comment = Comment.create( + auth=self.auth, + user=self.comment.user, + node=self.comment.node, + target=self.comment.target, + is_public=True, + ) + assert_equal(mock_signals.signals_sent(), set([comment_added])) + def test_edit(self): self.comment.edit( auth=self.auth, diff --git a/website/project/model.py b/website/project/model.py index b0479c33ffa..353aec72d8e 100644 --- a/website/project/model.py +++ b/website/project/model.py @@ -223,6 +223,7 @@ def create(cls, auth, **kwargs): ) comment.node.save() + project_signals.comment_added.send(comment, auth=auth) return comment diff --git a/website/project/signals.py b/website/project/signals.py index 642f2c97227..2b22e12dc96 100644 --- a/website/project/signals.py +++ b/website/project/signals.py @@ -1,6 +1,7 @@ import blinker signals = blinker.Namespace() +comment_added = signals.signal('comment-added') contributor_added = signals.signal('contributor-added') contributor_removed = signals.signal('contributor-removed') unreg_contributor_added = signals.signal('unreg-contributor-added') diff --git a/website/project/views/comment.py b/website/project/views/comment.py index 1bc3335c5db..5a94bfe9b05 100644 --- a/website/project/views/comment.py +++ b/website/project/views/comment.py @@ -16,6 +16,7 @@ from website.filters import gravatar from website.models import Guid, Comment from website.project.decorators import must_be_contributor_or_public +from website.project.signals import comment_added from datetime import datetime from website.project.model import has_anonymous_link @@ -148,9 +149,18 @@ def add_comment(auth, node, **kwargs): ) comment.save() + return { + 'comment': serialize_comment(comment, auth) + }, http.CREATED + +@comment_added.connect +def send_comment_added_notification(comment, auth): + node = comment.node + target = comment.target + context = dict( gravatar_url=auth.user.profile_image_url(), - content=content, + content=comment.content, target_user=target.user if is_reply(target) else None, parent_comment=target.content if is_reply(target) else "", url=node.absolute_url @@ -174,10 +184,6 @@ def add_comment(auth, node, **kwargs): **context ) - return { - 'comment': serialize_comment(comment, auth) - }, http.CREATED - def is_reply(target): return isinstance(target, Comment) diff --git a/website/signals.py b/website/signals.py index 3584dc6a162..d13deb431b7 100644 --- a/website/signals.py +++ b/website/signals.py @@ -8,6 +8,7 @@ ALL_SIGNALS = [ auth.contributor_removed, auth.node_deleted, + project.comment_added, project.unreg_contributor_added, project.contributor_added, project.privacy_set_public, From 477c1d2f5e12176892a036a424b0b95ff5a09472 Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Tue, 10 Nov 2015 17:09:13 -0500 Subject: [PATCH 09/41] Get unread comments using v2 API --- website/project/model.py | 3 +++ website/static/js/comment.js | 13 +++++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/website/project/model.py b/website/project/model.py index 353aec72d8e..edcacbf3ea5 100644 --- a/website/project/model.py +++ b/website/project/model.py @@ -198,6 +198,9 @@ def find_unread(cls, user, node): default_timestamp = datetime.datetime(1970, 1, 1, 12, 0, 0) n_unread = 0 if node.is_contributor(user): + if user.comments_viewed_timestamp is None: + user.comments_viewed_timestamp = {} + user.save() view_timestamp = user.comments_viewed_timestamp.get(node._id, default_timestamp) n_unread = Comment.find(Q('node', 'eq', node) & Q('user', 'ne', user) & diff --git a/website/static/js/comment.js b/website/static/js/comment.js index 296f16bc521..f34997547bb 100644 --- a/website/static/js/comment.js +++ b/website/static/js/comment.js @@ -151,7 +151,7 @@ BaseComment.prototype.fetch = function() { for (var i=0; i < response.data.length; i++) { updateCommentUserData(response.data[i], self); } -// self.unreadComments(response.nUnread); + setUnreadCommentCount(self); deferred.resolve(self.comments()); self._loaded = true; }); @@ -174,6 +174,16 @@ var updateCommentUserData = function(commentJSON, self) { return self.comments; }; +var setUnreadCommentCount = function(self) { + var request = osfHelpers.ajaxJSON( + 'GET', + osfHelpers.apiV2Url('nodes/' + window.contextVars.node.id + '/?related_counts=True', {query: 'related_counts=True'}), + {'isCors': true}); + request.done(function(response) { + self.unreadComments(response.data.relationships.comments.links.related.meta.unread); + }); +}; + BaseComment.prototype.submitReply = function() { var self = this; @@ -209,7 +219,6 @@ BaseComment.prototype.submitReply = function() { self.replyContent(null); self.onSubmitSuccess(response); updateCommentUserData(response.data, self); -// self.comments.unshift(new CommentModel(response.comment, self, self.$root)); if (!self.hasChildren()) { self.hasChildren(true); } From 52a4129ee1f45a775ea114dd23125fd774b933e9 Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Wed, 11 Nov 2015 12:28:55 -0500 Subject: [PATCH 10/41] Fix get_is_abuse Handle case where auth is None or the user is an anonymous user. --- api/comments/serializers.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/api/comments/serializers.py b/api/comments/serializers.py index f542a30c54c..fc99ad32f45 100644 --- a/api/comments/serializers.py +++ b/api/comments/serializers.py @@ -54,6 +54,8 @@ class Meta: def get_is_abuse(self, obj): auth = Auth(self.context['request'].user) + if not auth or auth.user.is_anonymous: + return False return auth.user and auth.user._id in obj.reports def get_can_edit(self, obj): From 316e860e9e584935f27f5152e8f9a50cde9119d6 Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Wed, 11 Nov 2015 12:31:07 -0500 Subject: [PATCH 11/41] Do not allow whitespace comments Also, add a test to ensure that html tags are removed from the content. --- api/comments/serializers.py | 12 +++++- .../nodes/views/test_node_comments_list.py | 39 ++++++++++++++++--- 2 files changed, 44 insertions(+), 7 deletions(-) diff --git a/api/comments/serializers.py b/api/comments/serializers.py index fc99ad32f45..f27cb2d0ec6 100644 --- a/api/comments/serializers.py +++ b/api/comments/serializers.py @@ -70,7 +70,11 @@ def create(self, validated_data): auth = Auth(user) node = validated_data['node'] - validated_data['content'] = validated_data.pop('get_content') + content = validated_data.pop('get_content') + validated_data['content'] = content.strip() + if not validated_data['content']: + raise ValidationError('Comment cannot be empty.') + if node and node.can_comment(auth): comment = Comment.create(auth=auth, **validated_data) else: @@ -82,7 +86,11 @@ def update(self, comment, validated_data): auth = Auth(self.context['request'].user) if validated_data: if 'get_content' in validated_data: - comment.edit(validated_data['get_content'], auth=auth, save=True) + content = validated_data.pop('get_content') + content = content.strip() + if not content: + raise ValidationError('Comment cannot be empty.') + comment.edit(content, auth=auth, save=True) if validated_data.get('is_deleted', None) is True: comment.delete(auth, save=True) elif comment.is_deleted: diff --git a/api_tests/nodes/views/test_node_comments_list.py b/api_tests/nodes/views/test_node_comments_list.py index 869dee4fb0e..6cbfd9ed99e 100644 --- a/api_tests/nodes/views/test_node_comments_list.py +++ b/api_tests/nodes/views/test_node_comments_list.py @@ -12,6 +12,7 @@ AuthUserFactory, CommentFactory ) +from website.util.sanitize import strip_html class TestNodeCommentsList(ApiTestCase): @@ -177,11 +178,33 @@ def test_create_comment_no_content(self): assert_equal(res.json['errors'][0]['detail'], 'This field may not be blank.') assert_equal(res.json['errors'][0]['source']['pointer'], '/data/attributes/content') - def test_create_comment_invalid_target_node(self): - url = '/{}nodes/{}/comments/'.format(API_BASE, 'abcde') - res = self.app.post_json_api(url, self.payload, auth=self.user.auth, expect_errors=True) - assert_equal(res.status_code, 404) - assert_equal(res.json['errors'][0]['detail'], 'Not found.') + def test_create_comment_trims_whitespace(self): + self._set_up_private_project() + payload = { + 'data': { + 'type': 'comments', + 'attributes': { + 'content': ' ' + } + } + } + res = self.app.post_json_api(self.private_url, payload, auth=self.user.auth, expect_errors=True) + assert_equal(res.status_code, 400) + assert_equal(res.json['errors'][0]['detail'], 'Comment cannot be empty.') + + def test_create_comment_sanitizes_input(self): + self._set_up_private_project() + payload = { + 'data': { + 'type': 'comments', + 'attributes': { + 'content': 'Cool Comment' + } + } + } + res = self.app.post_json_api(self.private_url, payload, auth=self.user.auth) + assert_equal(res.status_code, 201) + assert_equal(res.json['data']['attributes']['content'], strip_html(payload['data']['attributes']['content'])) def test_create_comment_exceeds_max_length(self): self._set_up_private_project() @@ -203,6 +226,12 @@ def test_create_comment_exceeds_max_length(self): assert_equal(res.json['errors'][0]['detail'], 'Ensure this field has no more than ' + str(osf_settings.COMMENT_MAXLENGTH) + ' characters.') + def test_create_comment_invalid_target_node(self): + url = '/{}nodes/{}/comments/'.format(API_BASE, 'abcde') + res = self.app.post_json_api(url, self.payload, auth=self.user.auth, expect_errors=True) + assert_equal(res.status_code, 404) + assert_equal(res.json['errors'][0]['detail'], 'Not found.') + def test_private_node_logged_in_admin_can_comment(self): self._set_up_private_project() res = self.app.post_json_api(self.private_url, self.payload, auth=self.user.auth) From 5ce7747d1153aa84dbaf5f1c77e5c3e986225c70 Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Thu, 12 Nov 2015 10:08:29 -0500 Subject: [PATCH 12/41] Remove query string in apiv2 node url Remove query string since it is already passed in as a query parameter. --- website/static/js/comment.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/static/js/comment.js b/website/static/js/comment.js index f34997547bb..79bfc9b2c47 100644 --- a/website/static/js/comment.js +++ b/website/static/js/comment.js @@ -177,7 +177,7 @@ var updateCommentUserData = function(commentJSON, self) { var setUnreadCommentCount = function(self) { var request = osfHelpers.ajaxJSON( 'GET', - osfHelpers.apiV2Url('nodes/' + window.contextVars.node.id + '/?related_counts=True', {query: 'related_counts=True'}), + osfHelpers.apiV2Url('nodes/' + window.contextVars.node.id + '/', {query: 'related_counts=True'}), {'isCors': true}); request.done(function(response) { self.unreadComments(response.data.relationships.comments.links.related.meta.unread); From 2e2739bc2f728fe5589c85edb546cc73a934546b Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Thu, 12 Nov 2015 10:19:06 -0500 Subject: [PATCH 13/41] Add tests for updating comments Ensure that when a comment is edited, it cannot exceed the max length or be empty. --- .../comments/views/test_comment_detail.py | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/api_tests/comments/views/test_comment_detail.py b/api_tests/comments/views/test_comment_detail.py index 9978144e1e8..2ac996bf968 100644 --- a/api_tests/comments/views/test_comment_detail.py +++ b/api_tests/comments/views/test_comment_detail.py @@ -2,6 +2,7 @@ from nose.tools import * # flake8: noqa from api.base.settings.defaults import API_BASE +from api.base.settings import osf_settings from tests.base import ApiTestCase from tests.factories import ProjectFactory, AuthUserFactory, CommentFactory, RegistrationFactory @@ -193,6 +194,44 @@ def test_public_node_non_contributor_commenter_can_update_comment(self): assert_equal(res.status_code, 200) assert_equal(payload['data']['attributes']['content'], res.json['data']['attributes']['content']) + def test_update_comment_cannot_exceed_max_length(self): + self._set_up_private_project_with_comment() + payload = { + 'data': { + 'id': self.comment._id, + 'type': 'comments', + 'attributes': { + 'content': ('contentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontent' + + 'contentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcon' + + 'tentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentconten' + + 'tcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentco' + + 'ntentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentconte' + + 'ntcontentcontentcontentcontentcontentcontent'), + 'deleted': False + } + } + } + res = self.app.put_json_api(self.private_url, payload, auth=self.user.auth, expect_errors=True) + assert_equal(res.status_code, 400) + assert_equal(res.json['errors'][0]['detail'], + 'Ensure this field has no more than ' + str(osf_settings.COMMENT_MAXLENGTH) + ' characters.') + + def test_update_comment_cannot_be_empty(self): + self._set_up_private_project_with_comment() + payload = { + 'data': { + 'id': self.comment._id, + 'type': 'comments', + 'attributes': { + 'content': '', + 'deleted': False + } + } + } + res = self.app.put_json_api(self.private_url, payload, auth=self.user.auth, expect_errors=True) + assert_equal(res.status_code, 400) + assert_equal(res.json['errors'][0]['detail'], 'This field may not be blank.') + def test_private_node_only_logged_in_contributor_commenter_can_delete_comment(self): self._set_up_private_project_with_comment() comment = CommentFactory(node=self.private_project, target=self.private_project, user=self.user) From 2e23b2fdb90fcc0e7dae3e4a471bf025402cde2c Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Thu, 12 Nov 2015 11:30:39 -0500 Subject: [PATCH 14/41] Fix is_abuse and creating reports Ensure self.abuseText() defaults to an empty string. Also fix checking whether a user is anoymous in the is_abuse serializer method field. --- api/comments/serializers.py | 4 ++-- website/static/js/comment.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/api/comments/serializers.py b/api/comments/serializers.py index f27cb2d0ec6..6dd6cb51b5a 100644 --- a/api/comments/serializers.py +++ b/api/comments/serializers.py @@ -54,13 +54,13 @@ class Meta: def get_is_abuse(self, obj): auth = Auth(self.context['request'].user) - if not auth or auth.user.is_anonymous: + if not auth or auth.user.is_anonymous(): return False return auth.user and auth.user._id in obj.reports def get_can_edit(self, obj): auth = Auth(self.context['request'].user) - return obj.user == auth.user + return obj.user._id == auth.user._id def get_has_children(self, obj): return bool(getattr(obj, 'commented', [])) diff --git a/website/static/js/comment.js b/website/static/js/comment.js index 79bfc9b2c47..4b4cafcb678 100644 --- a/website/static/js/comment.js +++ b/website/static/js/comment.js @@ -284,7 +284,7 @@ var CommentModel = function(data, $parent, $root) { self.undeleting = ko.observable(false); self.abuseCategory = ko.observable('spam'); - self.abuseText = ko.observable(); + self.abuseText = ko.observable(''); self.editing = ko.observable(false); From 0b6d65e9b57b533689deb94579f121c42c254799 Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Thu, 12 Nov 2015 11:41:19 -0500 Subject: [PATCH 15/41] Correct/update documentation --- api/comments/views.py | 6 ++++++ api/nodes/views.py | 3 +++ website/project/model.py | 4 ++-- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/api/comments/views.py b/api/comments/views.py index ab2d40a4068..4f4e68f904b 100644 --- a/api/comments/views.py +++ b/api/comments/views.py @@ -67,6 +67,9 @@ class CommentRepliesList(generics.ListCreateAPIView, CommentMixin, ODMFilterMixi date_modified iso8601 timestamp timestamp when the comment was last updated modified boolean has this comment been edited? deleted boolean is this comment deleted? + is_abuse boolean has this comment been reported by the current user? + has_children boolean does this comment have replies? + can_edit boolean can the current user edit this comment? ##Links @@ -161,6 +164,9 @@ class CommentDetail(generics.RetrieveUpdateAPIView, CommentMixin): date_modified iso8601 timestamp timestamp when the comment was last updated modified boolean has this comment been edited? deleted boolean is this comment deleted? + is_abuse boolean has this comment been reported by the current user? + has_children boolean does this comment have replies? + can_edit boolean can the current user edit this comment? ##Relationships diff --git a/api/nodes/views.py b/api/nodes/views.py index c15caae7de5..445e9aba901 100644 --- a/api/nodes/views.py +++ b/api/nodes/views.py @@ -1416,6 +1416,9 @@ class NodeCommentsList(generics.ListCreateAPIView, ODMFilterMixin, NodeMixin): date_modified iso8601 timestamp timestamp when the comment was last updated modified boolean has this comment been edited? deleted boolean is this comment deleted? + is_abuse boolean has this comment been reported by the current user? + has_children boolean does this comment have replies? + can_edit boolean can the current user edit this comment? ##Links diff --git a/website/project/model.py b/website/project/model.py index edcacbf3ea5..22fa61d944d 100644 --- a/website/project/model.py +++ b/website/project/model.py @@ -163,8 +163,8 @@ class Comment(GuidStoredObject): # Dictionary field mapping user IDs to dictionaries of report details: # { - # 'icpnw': {'category': 'hate', 'message': 'offensive'}, - # 'cdi38': {'category': 'spam', 'message': 'godwins law'}, + # 'icpnw': {'category': 'hate', 'text': 'offensive'}, + # 'cdi38': {'category': 'spam', 'text': 'godwins law'}, # } reports = fields.DictionaryField(validate=validate_comment_reports) From b52957472a3c416fdaa29ed429a02ee53e222882 Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Thu, 12 Nov 2015 13:36:22 -0500 Subject: [PATCH 16/41] Fix get_can_edit Check if auth or if user is anonymous before comparing the commenter and user by id --- api/comments/serializers.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/api/comments/serializers.py b/api/comments/serializers.py index 6dd6cb51b5a..f0392c023e4 100644 --- a/api/comments/serializers.py +++ b/api/comments/serializers.py @@ -60,6 +60,8 @@ def get_is_abuse(self, obj): def get_can_edit(self, obj): auth = Auth(self.context['request'].user) + if not auth or auth.user.is_anonymous(): + return False return obj.user._id == auth.user._id def get_has_children(self, obj): From 5f791e3faa9f2c95ef56ec6f55027025f8f689b8 Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Thu, 12 Nov 2015 17:14:03 -0500 Subject: [PATCH 17/41] Remove v1 comment list route --- tests/test_views.py | 69 -------------------------------- website/project/views/comment.py | 38 ------------------ website/routes.py | 10 ----- 3 files changed, 117 deletions(-) diff --git a/tests/test_views.py b/tests/test_views.py index 05da735ea5d..bee4df45274 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -3792,36 +3792,6 @@ def test_report_abuse(self): {'category': 'spam', 'text': 'ads'} ) - def test_can_view_private_comments_if_contributor(self): - - self._configure_project(self.project, 'public') - CommentFactory(node=self.project, user=self.project.creator, is_public=False) - - url = self.project.api_url + 'comments/' - res = self.app.get(url, auth=self.project.creator.auth) - - assert_equal(len(res.json['comments']), 1) - - def test_view_comments_with_anonymous_link(self): - self.project.save() - self.project.set_privacy('private') - self.project.reload() - user = AuthUserFactory() - link = PrivateLinkFactory(anonymous=True) - link.nodes.append(self.project) - link.save() - - CommentFactory(node=self.project, user=self.project.creator, is_public=False) - - url = self.project.api_url + 'comments/' - res = self.app.get(url, {"view_only": link.key}, auth=user.auth) - comment = res.json['comments'][0] - author = comment['author'] - assert_in('A user', author['name']) - assert_false(author['gravatarUrl']) - assert_false(author['url']) - assert_false(author['id']) - def test_discussion_recursive(self): self._configure_project(self.project, 'public') @@ -3889,45 +3859,6 @@ def test_confirm_non_contrib_viewers_dont_have_pid_in_comments_view_timestamp(se self.non_contributor.reload() assert_not_in(self.project._id, self.non_contributor.comments_viewed_timestamp) - def test_n_unread_comments_updates_when_comment_is_added(self): - self._add_comment(self.project, auth=self.project.creator.auth) - self.project.reload() - - url = self.project.api_url_for('list_comments') - res = self.app.get(url, auth=self.user.auth) - assert_equal(res.json.get('nUnread'), 1) - - url = self.project.api_url_for('update_comments_timestamp') - res = self.app.put_json(url, auth=self.user.auth) - self.user.reload() - - url = self.project.api_url_for('list_comments') - res = self.app.get(url, auth=self.user.auth) - assert_equal(res.json.get('nUnread'), 0) - - def test_n_unread_comments_updates_when_comment_reply(self): - comment = CommentFactory(node=self.project, user=self.project.creator) - reply = CommentFactory(node=self.project, user=self.user, target=comment) - self.project.reload() - - url = self.project.api_url_for('list_comments') - res = self.app.get(url, auth=self.project.creator.auth) - assert_equal(res.json.get('nUnread'), 1) - - - def test_n_unread_comments_updates_when_comment_is_edited(self): - self.test_edit_comment() - self.project.reload() - - url = self.project.api_url_for('list_comments') - res = self.app.get(url, auth=self.user.auth) - assert_equal(res.json.get('nUnread'), 1) - - def test_n_unread_comments_is_zero_when_no_comments(self): - url = self.project.api_url_for('list_comments') - res = self.app.get(url, auth=self.project.creator.auth) - assert_equal(res.json.get('nUnread'), 0) - class TestTagViews(OsfTestCase): diff --git a/website/project/views/comment.py b/website/project/views/comment.py index 5a94bfe9b05..23a6db96ca6 100644 --- a/website/project/views/comment.py +++ b/website/project/views/comment.py @@ -102,14 +102,6 @@ def serialize_comment(comment, auth, anonymous=False): } -def serialize_comments(record, auth, anonymous=False): - - return [ - serialize_comment(comment, auth, anonymous) - for comment in getattr(record, 'commented', []) - ] - - def get_comment(cid, auth, owner=False): comment = Comment.load(cid) if comment is None: @@ -189,35 +181,6 @@ def is_reply(target): return isinstance(target, Comment) -@must_be_contributor_or_public -def list_comments(auth, node, **kwargs): - anonymous = has_anonymous_link(node, auth) - guid = request.args.get('target') - target = resolve_target(node, guid) - serialized_comments = serialize_comments(target, auth, anonymous) - n_unread = 0 - - if node.is_contributor(auth.user): - if auth.user.comments_viewed_timestamp is None: - auth.user.comments_viewed_timestamp = {} - auth.user.save() - n_unread = n_unread_comments(target, auth.user) - return { - 'comments': serialized_comments, - 'nUnread': n_unread - } - - -def n_unread_comments(node, user): - """Return the number of unread comments on a node for a user.""" - default_timestamp = datetime(1970, 1, 1, 12, 0, 0) - view_timestamp = user.comments_viewed_timestamp.get(node._id, default_timestamp) - return Comment.find(Q('node', 'eq', node) & - Q('user', 'ne', user) & - Q('date_created', 'gt', view_timestamp) & - Q('date_modified', 'gt', view_timestamp)).count() - - @must_be_logged_in @must_be_contributor_or_public def edit_comment(auth, **kwargs): @@ -269,7 +232,6 @@ def update_comments_timestamp(auth, node, **kwargs): if node.is_contributor(auth.user): auth.user.comments_viewed_timestamp[node._id] = datetime.utcnow() auth.user.save() - list_comments(**kwargs) return {node._id: auth.user.comments_viewed_timestamp[node._id].isoformat()} else: return {} diff --git a/website/routes.py b/website/routes.py index fbcd97b656b..e3245eaf298 100644 --- a/website/routes.py +++ b/website/routes.py @@ -296,16 +296,6 @@ def make_url_map(app): process_rules(app, [ - Rule( - [ - '/project//comments/', - '/project//node//comments/', - ], - 'get', - project_views.comment.list_comments, - json_renderer, - ), - Rule( [ '/project//comments/discussion/', From b171e07b756f58caff2be3f0f86c286dc0bed6ff Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Thu, 12 Nov 2015 17:24:11 -0500 Subject: [PATCH 18/41] Remove remaining comment CRUD routes Remove v1 routes for create, edit, delete and undelete for comments and tests. --- tests/test_views.py | 263 ------------------------------- website/project/views/comment.py | 122 +------------- website/routes.py | 40 ----- 3 files changed, 1 insertion(+), 424 deletions(-) diff --git a/tests/test_views.py b/tests/test_views.py index bee4df45274..bfc1f7f9210 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -3505,269 +3505,6 @@ def _configure_project(self, project, comment_level): project.comment_level = comment_level project.save() - def _add_comment(self, project, content=None, **kwargs): - - content = content if content is not None else 'hammer to fall' - url = project.api_url + 'comment/' - return self.app.post_json( - url, - { - 'content': content, - 'isPublic': 'public', - }, - **kwargs - ) - - def test_add_comment_public_contributor(self): - - self._configure_project(self.project, 'public') - res = self._add_comment( - self.project, auth=self.project.creator.auth, - ) - - self.project.reload() - - res_comment = res.json['comment'] - date_created = parse_date(str(res_comment.pop('dateCreated'))) - date_modified = parse_date(str(res_comment.pop('dateModified'))) - - serialized_comment = serialize_comment(self.project.commented[0], self.consolidated_auth) - date_created2 = parse_date(serialized_comment.pop('dateCreated')) - date_modified2 = parse_date(serialized_comment.pop('dateModified')) - - assert_datetime_equal(date_created, date_created2) - assert_datetime_equal(date_modified, date_modified2) - - assert_equal(len(self.project.commented), 1) - assert_equal(res_comment, serialized_comment) - - def test_add_comment_public_non_contributor(self): - - self._configure_project(self.project, 'public') - res = self._add_comment( - self.project, auth=self.non_contributor.auth, - ) - - self.project.reload() - - res_comment = res.json['comment'] - date_created = parse_date(res_comment.pop('dateCreated')) - date_modified = parse_date(res_comment.pop('dateModified')) - - serialized_comment = serialize_comment(self.project.commented[0], Auth(user=self.non_contributor)) - date_created2 = parse_date(serialized_comment.pop('dateCreated')) - date_modified2 = parse_date(serialized_comment.pop('dateModified')) - - assert_datetime_equal(date_created, date_created2) - assert_datetime_equal(date_modified, date_modified2) - - assert_equal(len(self.project.commented), 1) - assert_equal(res_comment, serialized_comment) - - def test_add_comment_private_contributor(self): - - self._configure_project(self.project, 'private') - res = self._add_comment( - self.project, auth=self.project.creator.auth, - ) - - self.project.reload() - - res_comment = res.json['comment'] - date_created = parse_date(str(res_comment.pop('dateCreated'))) - date_modified = parse_date(str(res_comment.pop('dateModified'))) - - serialized_comment = serialize_comment(self.project.commented[0], self.consolidated_auth) - date_created2 = parse_date(serialized_comment.pop('dateCreated')) - date_modified2 = parse_date(serialized_comment.pop('dateModified')) - - assert_datetime_equal(date_created, date_created2) - assert_datetime_equal(date_modified, date_modified2) - - assert_equal(len(self.project.commented), 1) - assert_equal(res_comment, serialized_comment) - - def test_add_comment_private_non_contributor(self): - - self._configure_project(self.project, 'private') - res = self._add_comment( - self.project, auth=self.non_contributor.auth, expect_errors=True, - ) - - assert_equal(res.status_code, http.FORBIDDEN) - - def test_add_comment_logged_out(self): - self._configure_project(self.project, 'public') - res = self._add_comment(self.project) - - assert_equal(res.status_code, 302) - assert_in('login', res.headers.get('location')) - - def test_add_comment_off(self): - - self._configure_project(self.project, None) - res = self._add_comment( - self.project, auth=self.project.creator.auth, expect_errors=True, - ) - - assert_equal(res.status_code, http.BAD_REQUEST) - - def test_add_comment_empty(self): - self._configure_project(self.project, 'public') - res = self._add_comment( - self.project, content='', - auth=self.project.creator.auth, - expect_errors=True, - ) - assert_equal(res.status_code, http.BAD_REQUEST) - assert_false(getattr(self.project, 'commented', [])) - - def test_add_comment_toolong(self): - self._configure_project(self.project, 'public') - res = self._add_comment( - self.project, content='toolong' * 500, - auth=self.project.creator.auth, - expect_errors=True, - ) - assert_equal(res.status_code, http.BAD_REQUEST) - assert_false(getattr(self.project, 'commented', [])) - - def test_add_comment_whitespace(self): - self._configure_project(self.project, 'public') - res = self._add_comment( - self.project, content=' ', - auth=self.project.creator.auth, - expect_errors=True - ) - assert_equal(res.status_code, http.BAD_REQUEST) - assert_false(getattr(self.project, 'commented', [])) - - def test_edit_comment(self): - - self._configure_project(self.project, 'public') - comment = CommentFactory(node=self.project) - - url = self.project.api_url + 'comment/{0}/'.format(comment._id) - res = self.app.put_json( - url, - { - 'content': 'edited', - 'isPublic': 'private', - }, - auth=self.project.creator.auth, - ) - - comment.reload() - - assert_equal(res.json['content'], 'edited') - - assert_equal(comment.content, 'edited') - - def test_edit_comment_short(self): - self._configure_project(self.project, 'public') - comment = CommentFactory(node=self.project, content='short') - url = self.project.api_url + 'comment/{0}/'.format(comment._id) - res = self.app.put_json( - url, - { - 'content': '', - 'isPublic': 'private', - }, - auth=self.project.creator.auth, - expect_errors=True, - ) - comment.reload() - assert_equal(res.status_code, http.BAD_REQUEST) - assert_equal(comment.content, 'short') - - def test_edit_comment_toolong(self): - self._configure_project(self.project, 'public') - comment = CommentFactory(node=self.project, content='short') - url = self.project.api_url + 'comment/{0}/'.format(comment._id) - res = self.app.put_json( - url, - { - 'content': 'toolong' * 500, - 'isPublic': 'private', - }, - auth=self.project.creator.auth, - expect_errors=True, - ) - comment.reload() - assert_equal(res.status_code, http.BAD_REQUEST) - assert_equal(comment.content, 'short') - - def test_edit_comment_non_author(self): - "Contributors who are not the comment author cannot edit." - self._configure_project(self.project, 'public') - comment = CommentFactory(node=self.project) - non_author = AuthUserFactory() - self.project.add_contributor(non_author, auth=self.consolidated_auth) - - url = self.project.api_url + 'comment/{0}/'.format(comment._id) - res = self.app.put_json( - url, - { - 'content': 'edited', - 'isPublic': 'private', - }, - auth=non_author.auth, - expect_errors=True, - ) - - assert_equal(res.status_code, http.FORBIDDEN) - - def test_edit_comment_non_contributor(self): - "Non-contributors who are not the comment author cannot edit." - self._configure_project(self.project, 'public') - comment = CommentFactory(node=self.project) - - url = self.project.api_url + 'comment/{0}/'.format(comment._id) - res = self.app.put_json( - url, - { - 'content': 'edited', - 'isPublic': 'private', - }, - auth=self.non_contributor.auth, - expect_errors=True, - ) - - assert_equal(res.status_code, http.FORBIDDEN) - - def test_delete_comment_author(self): - - self._configure_project(self.project, 'public') - comment = CommentFactory(node=self.project) - - url = self.project.api_url + 'comment/{0}/'.format(comment._id) - self.app.delete_json( - url, - auth=self.project.creator.auth, - ) - - comment.reload() - - assert_true(comment.is_deleted) - - def test_delete_comment_non_author(self): - - self._configure_project(self.project, 'public') - comment = CommentFactory(node=self.project) - - url = self.project.api_url + 'comment/{0}/'.format(comment._id) - res = self.app.delete_json( - url, - auth=self.non_contributor.auth, - expect_errors=True, - ) - - assert_equal(res.status_code, http.FORBIDDEN) - - comment.reload() - - assert_false(comment.is_deleted) - def test_report_abuse(self): self._configure_project(self.project, 'public') diff --git a/website/project/views/comment.py b/website/project/views/comment.py index 23a6db96ca6..8f44c62769f 100644 --- a/website/project/views/comment.py +++ b/website/project/views/comment.py @@ -4,33 +4,21 @@ import pytz from flask import request -from modularodm import Q from framework.exceptions import HTTPError from framework.auth.decorators import must_be_logged_in from framework.auth.utils import privacy_info_handle -from framework.forms.utils import sanitize from website import settings from website.notifications.emails import notify from website.filters import gravatar -from website.models import Guid, Comment +from website.models import Comment from website.project.decorators import must_be_contributor_or_public from website.project.signals import comment_added from datetime import datetime from website.project.model import has_anonymous_link -def resolve_target(node, guid): - - if not guid: - return node - target = Guid.load(guid) - if target is None: - raise HTTPError(http.BAD_REQUEST) - return target.referent - - def collect_discussion(target, users=None): users = users or collections.defaultdict(list) @@ -74,34 +62,6 @@ def comment_discussion(auth, node, **kwargs): } -def serialize_comment(comment, auth, anonymous=False): - return { - 'id': comment._id, - 'author': { - 'id': privacy_info_handle(comment.user._id, anonymous), - 'url': privacy_info_handle(comment.user.url, anonymous), - 'name': privacy_info_handle( - comment.user.fullname, anonymous, name=True - ), - 'gravatarUrl': privacy_info_handle( - gravatar( - comment.user, use_ssl=True, - size=settings.PROFILE_IMAGE_SMALL - ), - anonymous - ), - }, - 'dateCreated': comment.date_created.isoformat(), - 'dateModified': comment.date_modified.isoformat(), - 'content': comment.content, - 'hasChildren': bool(getattr(comment, 'commented', [])), - 'canEdit': comment.user == auth.user, - 'modified': comment.modified, - 'isDeleted': comment.is_deleted, - 'isAbuse': auth.user and auth.user._id in comment.reports, - } - - def get_comment(cid, auth, owner=False): comment = Comment.load(cid) if comment is None: @@ -111,40 +71,6 @@ def get_comment(cid, auth, owner=False): raise HTTPError(http.FORBIDDEN) return comment - -@must_be_logged_in -@must_be_contributor_or_public -def add_comment(auth, node, **kwargs): - - if not node.comment_level: - raise HTTPError(http.BAD_REQUEST) - - if not node.can_comment(auth): - raise HTTPError(http.FORBIDDEN) - - guid = request.json.get('target') - target = resolve_target(node, guid) - - content = request.json.get('content').strip() - content = sanitize(content) - if not content: - raise HTTPError(http.BAD_REQUEST) - if len(content) > settings.COMMENT_MAXLENGTH: - raise HTTPError(http.BAD_REQUEST) - - comment = Comment.create( - auth=auth, - node=node, - target=target, - user=auth.user, - content=content, - ) - comment.save() - - return { - 'comment': serialize_comment(comment, auth) - }, http.CREATED - @comment_added.connect def send_comment_added_notification(comment, auth): node = comment.node @@ -180,52 +106,6 @@ def send_comment_added_notification(comment, auth): def is_reply(target): return isinstance(target, Comment) - -@must_be_logged_in -@must_be_contributor_or_public -def edit_comment(auth, **kwargs): - - cid = kwargs.get('cid') - comment = get_comment(cid, auth, owner=True) - - content = request.json.get('content').strip() - content = sanitize(content) - if not content: - raise HTTPError(http.BAD_REQUEST) - if len(content) > settings.COMMENT_MAXLENGTH: - raise HTTPError(http.BAD_REQUEST) - - comment.edit( - content=content, - auth=auth, - save=True - ) - - return serialize_comment(comment, auth) - - -@must_be_logged_in -@must_be_contributor_or_public -def delete_comment(auth, **kwargs): - - cid = kwargs.get('cid') - comment = get_comment(cid, auth, owner=True) - comment.delete(auth=auth, save=True) - - return {} - - -@must_be_logged_in -@must_be_contributor_or_public -def undelete_comment(auth, **kwargs): - - cid = kwargs.get('cid') - comment = get_comment(cid, auth, owner=True) - comment.undelete(auth=auth, save=True) - - return {} - - @must_be_logged_in @must_be_contributor_or_public def update_comments_timestamp(auth, node, **kwargs): diff --git a/website/routes.py b/website/routes.py index e3245eaf298..26a823081ce 100644 --- a/website/routes.py +++ b/website/routes.py @@ -306,46 +306,6 @@ def make_url_map(app): json_renderer, ), - Rule( - [ - '/project//comment/', - '/project//node//comment/', - ], - 'post', - project_views.comment.add_comment, - json_renderer, - ), - - Rule( - [ - '/project//comment//', - '/project//node//comment//', - ], - 'put', - project_views.comment.edit_comment, - json_renderer, - ), - - Rule( - [ - '/project//comment//', - '/project//node//comment//', - ], - 'delete', - project_views.comment.delete_comment, - json_renderer, - ), - - Rule( - [ - '/project//comment//undelete/', - '/project//node//comment//undelete/', - ], - 'put', - project_views.comment.undelete_comment, - json_renderer, - ), - Rule( [ '/project//comments/timestamps/', From 04ea63c4a8900985047fbc020d3bc58f4ce939a5 Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Thu, 12 Nov 2015 17:26:30 -0500 Subject: [PATCH 19/41] Remove comment (un)report abuse routes --- tests/test_views.py | 26 ---------------- website/project/views/comment.py | 51 -------------------------------- website/routes.py | 20 ------------- 3 files changed, 97 deletions(-) diff --git a/tests/test_views.py b/tests/test_views.py index bfc1f7f9210..7ca402c6326 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -16,7 +16,6 @@ from modularodm import Q from modularodm.exceptions import ValidationError -from dateutil.parser import parse as parse_date from framework import auth from framework.exceptions import HTTPError @@ -42,7 +41,6 @@ from website import mails, settings from website.util import rubeus from website.project.views.node import _view_project, abbrev_authors, _should_show_wiki_widget -from website.project.views.comment import serialize_comment from website.project.decorators import check_can_access from website.project.signals import contributor_added from website.addons.github.model import AddonGitHubOauthSettings @@ -3505,30 +3503,6 @@ def _configure_project(self, project, comment_level): project.comment_level = comment_level project.save() - def test_report_abuse(self): - - self._configure_project(self.project, 'public') - comment = CommentFactory(node=self.project) - reporter = AuthUserFactory() - - url = self.project.api_url + 'comment/{0}/report/'.format(comment._id) - - self.app.post_json( - url, - { - 'category': 'spam', - 'text': 'ads', - }, - auth=reporter.auth, - ) - - comment.reload() - assert_in(reporter._id, comment.reports) - assert_equal( - comment.reports[reporter._id], - {'category': 'spam', 'text': 'ads'} - ) - def test_discussion_recursive(self): self._configure_project(self.project, 'public') diff --git a/website/project/views/comment.py b/website/project/views/comment.py index 8f44c62769f..1940d310c06 100644 --- a/website/project/views/comment.py +++ b/website/project/views/comment.py @@ -1,11 +1,7 @@ # -*- coding: utf-8 -*- import collections -import httplib as http import pytz -from flask import request - -from framework.exceptions import HTTPError from framework.auth.decorators import must_be_logged_in from framework.auth.utils import privacy_info_handle @@ -61,16 +57,6 @@ def comment_discussion(auth, node, **kwargs): ] } - -def get_comment(cid, auth, owner=False): - comment = Comment.load(cid) - if comment is None: - raise HTTPError(http.NOT_FOUND) - if owner: - if auth.user != comment.user: - raise HTTPError(http.FORBIDDEN) - return comment - @comment_added.connect def send_comment_added_notification(comment, auth): node = comment.node @@ -116,40 +102,3 @@ def update_comments_timestamp(auth, node, **kwargs): else: return {} - -@must_be_logged_in -@must_be_contributor_or_public -def report_abuse(auth, **kwargs): - - user = auth.user - - cid = kwargs.get('cid') - comment = get_comment(cid, auth) - - category = request.json.get('category') - text = request.json.get('text', '') - if not category: - raise HTTPError(http.BAD_REQUEST) - - try: - comment.report_abuse(user, save=True, category=category, text=text) - except ValueError: - raise HTTPError(http.BAD_REQUEST) - - return {} - - -@must_be_logged_in -@must_be_contributor_or_public -def unreport_abuse(auth, **kwargs): - user = auth.user - - cid = kwargs.get('cid') - comment = get_comment(cid, auth) - - try: - comment.unreport_abuse(user, save=True) - except ValueError: - raise HTTPError(http.BAD_REQUEST) - - return {} diff --git a/website/routes.py b/website/routes.py index 26a823081ce..252166d4335 100644 --- a/website/routes.py +++ b/website/routes.py @@ -316,26 +316,6 @@ def make_url_map(app): json_renderer, ), - Rule( - [ - '/project//comment//report/', - '/project//node//comment//report/', - ], - 'post', - project_views.comment.report_abuse, - json_renderer, - ), - - Rule( - [ - '/project//comment//unreport/', - '/project//node//comment//unreport/', - ], - 'post', - project_views.comment.unreport_abuse, - json_renderer, - ), - Rule( [ '/project//citation/', From 943e352b0fadb535d4d48e90aee3003de852660d Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Fri, 13 Nov 2015 09:45:55 -0500 Subject: [PATCH 20/41] Fix flake8 error --- website/project/views/comment.py | 1 - 1 file changed, 1 deletion(-) diff --git a/website/project/views/comment.py b/website/project/views/comment.py index 1940d310c06..0e8e1a10c4a 100644 --- a/website/project/views/comment.py +++ b/website/project/views/comment.py @@ -101,4 +101,3 @@ def update_comments_timestamp(auth, node, **kwargs): return {node._id: auth.user.comments_viewed_timestamp[node._id].isoformat()} else: return {} - From 40c9a275e987063e9d31235cd52444e9ccb588d4 Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Fri, 13 Nov 2015 11:54:30 -0500 Subject: [PATCH 21/41] Fix failing test --- tests/test_notifications.py | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/tests/test_notifications.py b/tests/test_notifications.py index 25d02f651d2..0781fb5a162 100644 --- a/tests/test_notifications.py +++ b/tests/test_notifications.py @@ -19,7 +19,7 @@ from website.notifications.model import NotificationSubscription from website.notifications import emails from website.notifications import utils -from website.project.model import Node +from website.project.model import Node, Comment from website import mails from website.util import api_url_for from website.util import web_url_for @@ -1089,21 +1089,18 @@ def test_check_user_comment_reply_subscription_if_email_not_sent_to_target_user( # user is not subscribed to project comment notifications project = factories.ProjectFactory() - # reply to user + # user comments on project target = factories.CommentFactory(node=project, user=user) content = 'hammer to fall' - # auth=project.creator.auth - url = project.api_url + 'comment/' - self.app.post_json( - url, - { - 'content': content, - 'isPublic': 'public', - 'target': target._id - - }, - auth=project.creator.auth + # reply to user (note: notify is called from Comment.create) + reply = Comment.create( + auth=Auth(project.creator), + user=project.creator, + node=project, + content=content, + target=target, + is_public=True, ) assert_true(mock_notify.called) assert_equal(mock_notify.call_count, 2) From 59240d0c2a1a35b55e2a7f01a26d644224fb1e6b Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Fri, 13 Nov 2015 14:17:00 -0500 Subject: [PATCH 22/41] Fix unread comments regression When finding unread comments for a user on a node, look for comments created since comments_viewed_timestamp OR edited since then (instead of AND). Also exclude any deleted comments and add tests for the model method now that the view method tests have been removed. Fixes [#OSF-5193] --- tests/test_models.py | 55 ++++++++++++++++++++++++++++++++++++++++ website/project/model.py | 5 ++-- 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/tests/test_models.py b/tests/test_models.py index 6d7cd4f1154..25d55e84b32 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -4168,6 +4168,61 @@ def test_get_content_private_project_throws_permissions_error_for_logged_out_use with assert_raises(PermissionsError): comment.get_content(auth=None) + def test_find_unread_is_zero_when_no_comments(self): + n_unread = Comment.find_unread(user=UserFactory(), node=ProjectFactory()) + assert_equal(n_unread, 0) + + def test_find_unread_new_comments(self): + project = ProjectFactory() + user = UserFactory() + project.add_contributor(user) + project.save() + comment = CommentFactory(node=project, user=project.creator) + n_unread = Comment.find_unread(user=user, node=project) + assert_equal(n_unread, 1) + + def test_find_unread_includes_comment_replies(self): + project = ProjectFactory() + user = UserFactory() + project.add_contributor(user) + project.save() + comment = CommentFactory(node=project, user=user) + reply = CommentFactory(node=project, target=comment, user=project.creator) + n_unread = Comment.find_unread(user=user, node=project) + assert_equal(n_unread, 1) + + # Regression test for https://openscience.atlassian.net/browse/OSF-5193 + def test_find_unread_includes_edited_comments(self): + project = ProjectFactory() + user = AuthUserFactory() + project.add_contributor(user) + project.save() + comment = CommentFactory(node=project, user=project.creator) + + url = project.api_url_for('update_comments_timestamp') + res = self.app.put_json(url, auth=user.auth) + user.reload() + n_unread = Comment.find_unread(user=user, node=project) + assert_equal(n_unread, 0) + + # Edit previously read comment + comment.edit( + auth=Auth(project.creator), + content='edited', + save=True + ) + n_unread = Comment.find_unread(user=user, node=project) + assert_equal(n_unread, 1) + + def test_find_unread_does_not_include_deleted_comments(self): + project = ProjectFactory() + user = AuthUserFactory() + project.add_contributor(user) + project.save() + comment = CommentFactory(node=project, user=project.creator, is_deleted=True) + n_unread = Comment.find_unread(user=user, node=project) + assert_equal(n_unread, 0) + class TestPrivateLink(OsfTestCase): diff --git a/website/project/model.py b/website/project/model.py index 22fa61d944d..d8f4037ef20 100644 --- a/website/project/model.py +++ b/website/project/model.py @@ -204,8 +204,9 @@ def find_unread(cls, user, node): view_timestamp = user.comments_viewed_timestamp.get(node._id, default_timestamp) n_unread = Comment.find(Q('node', 'eq', node) & Q('user', 'ne', user) & - Q('date_created', 'gt', view_timestamp) & - Q('date_modified', 'gt', view_timestamp)).count() + Q('is_deleted', 'eq', False) & + (Q('date_created', 'gt', view_timestamp) | + Q('date_modified', 'gt', view_timestamp))).count() return n_unread @classmethod From 348520d52ef0143be50e1bd1032d324c75ac4b2d Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Fri, 13 Nov 2015 16:43:53 -0500 Subject: [PATCH 23/41] Fix getting comment reply lists Every time comment replies are fetched instead of pushing the comment model to the array (that already contains the comment) re-assign the value of self.comments to the result of the fetch. --- website/static/js/comment.js | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/website/static/js/comment.js b/website/static/js/comment.js index 4b4cafcb678..fb92a51c778 100644 --- a/website/static/js/comment.js +++ b/website/static/js/comment.js @@ -11,7 +11,6 @@ var koHelpers = require('./koHelpers'); require('knockout.punches'); require('jquery-autosize'); ko.punches.enableAll(); -var Raven = require('raven-js'); var osfHelpers = require('js/osfHelpers'); var CommentPane = require('js/commentpane'); @@ -148,9 +147,8 @@ BaseComment.prototype.fetch = function() { url, {'isCors': true}); request.done(function(response) { - for (var i=0; i < response.data.length; i++) { - updateCommentUserData(response.data[i], self); - } + var index = 0; + updateCommentUserData(response, index, self); setUnreadCommentCount(self); deferred.resolve(self.comments()); self._loaded = true; @@ -158,20 +156,24 @@ BaseComment.prototype.fetch = function() { return deferred; }; -var updateCommentUserData = function(commentJSON, self) { +var updateCommentUserData = function(commentsData, index, self) { var userRequest = osfHelpers.ajaxJSON( 'GET', - commentJSON.relationships.user.links.related.href, + commentsData.data[index].relationships.user.links.related.href, {'isCors': true}); userRequest.done(function(response) { - commentJSON.relationships.user = response; - var commentModel = new CommentModel(commentJSON, self, self.$root); - self.comments.push(commentModel); - self.comments.sort(function (left, right) { - return left.dateCreated() === right.dateCreated() ? 0 : (left.dateCreated() > right.dateCreated() ? -1 : 1); - }); + commentsData.data[index].relationships.user = response; + if (index !== commentsData.data.length - 1) { + updateCommentUserData(commentsData, (index + 1), self); + } + else { + self.comments( + ko.utils.arrayMap(commentsData.data, function(comment) { + return new CommentModel(comment, self, self.$root); + }) + ); + } }); - return self.comments; }; var setUnreadCommentCount = function(self) { @@ -247,7 +249,7 @@ var CommentModel = function(data, $parent, $root) { self.$root = $root; self.id = ko.observable(data.id); - self.content = ko.observable(data.attributes.content); + self.content = ko.observable(data.attributes.content || ''); self.dateCreated = ko.observable(data.attributes.date_created); self.dateModified = ko.observable(data.attributes.date_modified); self.isDeleted = ko.observable(data.attributes.deleted); From 9d2d57c88f5ef4a0da2ed5bad44983eb34465255 Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Mon, 16 Nov 2015 09:49:17 -0500 Subject: [PATCH 24/41] Fix logic for creating comment Change updateCommentsUserData to handle a single comment (from when a comment is added) or a list of comments (from fetching comment data). --- website/static/js/comment.js | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/website/static/js/comment.js b/website/static/js/comment.js index fb92a51c778..b8ba06992b1 100644 --- a/website/static/js/comment.js +++ b/website/static/js/comment.js @@ -148,7 +148,9 @@ BaseComment.prototype.fetch = function() { {'isCors': true}); request.done(function(response) { var index = 0; - updateCommentUserData(response, index, self); + if (response.data.length !== 0) { + updateCommentUserData(response, index, self); + } setUnreadCommentCount(self); deferred.resolve(self.comments()); self._loaded = true; @@ -157,21 +159,31 @@ BaseComment.prototype.fetch = function() { }; var updateCommentUserData = function(commentsData, index, self) { + var commentJSON = commentsData.data[index]; + if (index === -1) { + commentJSON = commentsData.data; + } var userRequest = osfHelpers.ajaxJSON( 'GET', - commentsData.data[index].relationships.user.links.related.href, + commentJSON.relationships.user.links.related.href, {'isCors': true}); userRequest.done(function(response) { - commentsData.data[index].relationships.user = response; - if (index !== commentsData.data.length - 1) { - updateCommentUserData(commentsData, (index + 1), self); + if (index === -1) { + commentsData.data.relationships.user = response; + self.comments.unshift(new CommentModel(commentsData.data, self, self.$root)); } else { - self.comments( - ko.utils.arrayMap(commentsData.data, function(comment) { - return new CommentModel(comment, self, self.$root); - }) - ); + commentsData.data[index].relationships.user = response; + if (index !== commentsData.data.length - 1) { + updateCommentUserData(commentsData, (index + 1), self); + } + else { + self.comments( + ko.utils.arrayMap(commentsData.data, function(comment) { + return new CommentModel(comment, self, self.$root); + }) + ); + } } }); }; @@ -220,7 +232,7 @@ BaseComment.prototype.submitReply = function() { self.cancelReply(); self.replyContent(null); self.onSubmitSuccess(response); - updateCommentUserData(response.data, self); + updateCommentUserData(response, -1, self); if (!self.hasChildren()) { self.hasChildren(true); } From 4270db0d6941af0460c38765cb08ce4f4dbf2fb9 Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Sun, 22 Nov 2015 16:15:56 -0500 Subject: [PATCH 25/41] Embed user data in api requests for comments Instead of making a separate request to get the data for every comment's user, embed that information in the request for the list of nodes. --- website/static/js/comment.js | 54 +++++++++--------------------------- 1 file changed, 13 insertions(+), 41 deletions(-) diff --git a/website/static/js/comment.js b/website/static/js/comment.js index b8ba06992b1..42aee00d159 100644 --- a/website/static/js/comment.js +++ b/website/static/js/comment.js @@ -138,19 +138,20 @@ BaseComment.prototype.fetch = function() { if (self._loaded) { deferred.resolve(self.comments()); } - var url = osfHelpers.apiV2Url('nodes/' + window.contextVars.node.id + '/comments/', {}); + var url = osfHelpers.apiV2Url('nodes/' + window.contextVars.node.id + '/comments/', {query: 'embed=user'}); if (self.id() !== undefined) { - url = osfHelpers.apiV2Url('comments/' + self.id() + '/replies/', {}); + url = osfHelpers.apiV2Url('comments/' + self.id() + '/replies/', {query: 'embed=user'}); } var request = osfHelpers.ajaxJSON( 'GET', url, {'isCors': true}); request.done(function(response) { - var index = 0; - if (response.data.length !== 0) { - updateCommentUserData(response, index, self); - } + self.comments( + ko.utils.arrayMap(response.data, function(comment) { + return new CommentModel(comment, self, self.$root); + }) + ); setUnreadCommentCount(self); deferred.resolve(self.comments()); self._loaded = true; @@ -158,35 +159,6 @@ BaseComment.prototype.fetch = function() { return deferred; }; -var updateCommentUserData = function(commentsData, index, self) { - var commentJSON = commentsData.data[index]; - if (index === -1) { - commentJSON = commentsData.data; - } - var userRequest = osfHelpers.ajaxJSON( - 'GET', - commentJSON.relationships.user.links.related.href, - {'isCors': true}); - userRequest.done(function(response) { - if (index === -1) { - commentsData.data.relationships.user = response; - self.comments.unshift(new CommentModel(commentsData.data, self, self.$root)); - } - else { - commentsData.data[index].relationships.user = response; - if (index !== commentsData.data.length - 1) { - updateCommentUserData(commentsData, (index + 1), self); - } - else { - self.comments( - ko.utils.arrayMap(commentsData.data, function(comment) { - return new CommentModel(comment, self, self.$root); - }) - ); - } - } - }); -}; var setUnreadCommentCount = function(self) { var request = osfHelpers.ajaxJSON( @@ -210,9 +182,9 @@ BaseComment.prototype.submitReply = function() { return; } self.submittingReply(true); - var url = osfHelpers.apiV2Url('nodes/' + window.contextVars.node.id + '/comments/', {}); + var url = osfHelpers.apiV2Url('nodes/' + window.contextVars.node.id + '/comments/', {query: 'embed=user'}); if (self.id() !== undefined) { - url = osfHelpers.apiV2Url('comments/' + self.id() + '/replies/', {}); + url = osfHelpers.apiV2Url('comments/' + self.id() + '/replies/', {query: 'embed=user'}); } var request = osfHelpers.ajaxJSON( 'POST', @@ -231,8 +203,7 @@ BaseComment.prototype.submitReply = function() { request.done(function(response) { self.cancelReply(); self.replyContent(null); - self.onSubmitSuccess(response); - updateCommentUserData(response, -1, self); + self.comments.unshift(new CommentModel(response.data, self, self.$root)); if (!self.hasChildren()) { self.hasChildren(true); } @@ -243,6 +214,7 @@ BaseComment.prototype.submitReply = function() { self.$root.fetchDiscussion(); self.$root.commented(true); } + self.onSubmitSuccess(response); }); request.fail(function() { self.cancelReply(); @@ -255,7 +227,7 @@ var CommentModel = function(data, $parent, $root) { BaseComment.prototype.constructor.call(this); var self = this; - var userData = data.relationships.user.data; + var userData = data.embeds.user.data; self.$parent = $parent; self.$root = $root; @@ -270,7 +242,7 @@ var CommentModel = function(data, $parent, $root) { self.canEdit = ko.observable(data.attributes.can_edit); self.hasChildren = ko.observable(data.attributes.has_children); self.author = { - 'id': data.relationships.user.data.id, + 'id': userData.id, 'url': userData.links.html, 'name': userData.attributes.full_name, 'gravatarUrl': userData.links.profile_image From 9daab5aab1f2917f12b82801f7243e5b1f57e865 Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Fri, 4 Dec 2015 15:36:20 -0500 Subject: [PATCH 26/41] Update creating comments Specify the comment target and type in the POST request --- website/static/js/comment.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/website/static/js/comment.js b/website/static/js/comment.js index 42aee00d159..6b7ba02318d 100644 --- a/website/static/js/comment.js +++ b/website/static/js/comment.js @@ -183,9 +183,6 @@ BaseComment.prototype.submitReply = function() { } self.submittingReply(true); var url = osfHelpers.apiV2Url('nodes/' + window.contextVars.node.id + '/comments/', {query: 'embed=user'}); - if (self.id() !== undefined) { - url = osfHelpers.apiV2Url('comments/' + self.id() + '/replies/', {query: 'embed=user'}); - } var request = osfHelpers.ajaxJSON( 'POST', url, @@ -196,6 +193,14 @@ BaseComment.prototype.submitReply = function() { 'type': 'comments', 'attributes': { 'content': self.replyContent() + }, + 'relationships': { + 'target': { + 'data': { + 'type': self.id() === undefined ? 'nodes' : 'comments', + 'id': self.id() === undefined ? window.contextVars.node.id : self.id() + } + } } } } From 44c3424317b6dce19b50345a223ebf0822318b5a Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Fri, 11 Dec 2015 10:43:11 -0500 Subject: [PATCH 27/41] Add author info when creating comment When a POST request is made to create a new comment or reply, the commenter's info is not embedded in the response. Get the current user's author info in CommentListModel (where the comments list gets fetched) so that whenever the user makes a comment, their info can be added to the CommentModel. --- website/static/js/comment.js | 45 ++++++++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 10 deletions(-) diff --git a/website/static/js/comment.js b/website/static/js/comment.js index 6b7ba02318d..6263032033b 100644 --- a/website/static/js/comment.js +++ b/website/static/js/comment.js @@ -132,6 +132,23 @@ BaseComment.prototype.setupToolTips = function(elm) { }); }; +BaseComment.prototype.fetchUserData = function() { + var self = this; + var request = osfHelpers.ajaxJSON( + 'GET', + osfHelpers.apiV2Url('users/me/', {}), + {'isCors': true}); + request.done(function(response) { + self.author = { + 'id': response.data.id, + 'url': response.data.links.html, + 'name': response.data.attributes.full_name, + 'gravatarUrl': response.data.links.profile_image + }; + return self.author; + }); +}; + BaseComment.prototype.fetch = function() { var self = this; var deferred = $.Deferred(); @@ -159,7 +176,6 @@ BaseComment.prototype.fetch = function() { return deferred; }; - var setUnreadCommentCount = function(self) { var request = osfHelpers.ajaxJSON( 'GET', @@ -182,7 +198,10 @@ BaseComment.prototype.submitReply = function() { return; } self.submittingReply(true); - var url = osfHelpers.apiV2Url('nodes/' + window.contextVars.node.id + '/comments/', {query: 'embed=user'}); + var url = osfHelpers.apiV2Url('nodes/' + window.contextVars.node.id + '/comments/', {}); + if (self.id() !== undefined) { + url = osfHelpers.apiV2Url('comments/' + self.id() + '/replies/', {}); + } var request = osfHelpers.ajaxJSON( 'POST', url, @@ -208,7 +227,9 @@ BaseComment.prototype.submitReply = function() { request.done(function(response) { self.cancelReply(); self.replyContent(null); - self.comments.unshift(new CommentModel(response.data, self, self.$root)); + var commentModel = new CommentModel(response.data, self, self.$root); + commentModel.author = self.$root.author; + self.comments.unshift(commentModel); if (!self.hasChildren()) { self.hasChildren(true); } @@ -232,7 +253,6 @@ var CommentModel = function(data, $parent, $root) { BaseComment.prototype.constructor.call(this); var self = this; - var userData = data.embeds.user.data; self.$parent = $parent; self.$root = $root; @@ -246,12 +266,16 @@ var CommentModel = function(data, $parent, $root) { self.isAbuse = ko.observable(data.attributes.is_abuse); self.canEdit = ko.observable(data.attributes.can_edit); self.hasChildren = ko.observable(data.attributes.has_children); - self.author = { - 'id': userData.id, - 'url': userData.links.html, - 'name': userData.attributes.full_name, - 'gravatarUrl': userData.links.profile_image - }; + + if ('embeds' in data && 'user' in data.embeds) { + var userData = data.embeds.user.data; + self.author = { + 'id': userData.id, + 'url': userData.links.html, + 'name': userData.attributes.full_name, + 'gravatarUrl': userData.links.profile_image + }; + } self.contentDisplay = ko.observable(markdown.full.render(self.content())); @@ -522,6 +546,7 @@ var CommentListModel = function(userName, canComment, hasChildren) { self.hasChildren = ko.observable(hasChildren); self.discussion = ko.observableArray(); + self.fetchUserData(); self.fetch(); self.fetchDiscussion(); From 0044469b0fb2019b300df0e23b522f9732d38436 Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Tue, 15 Dec 2015 13:52:48 -0500 Subject: [PATCH 28/41] Remove unnecessary Auth in serializer methods --- api/comments/serializers.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/api/comments/serializers.py b/api/comments/serializers.py index aa58372d332..9dee970984d 100644 --- a/api/comments/serializers.py +++ b/api/comments/serializers.py @@ -53,16 +53,16 @@ class Meta: type_ = 'comments' def get_is_abuse(self, obj): - auth = Auth(self.context['request'].user) - if not auth or auth.user.is_anonymous(): + user = self.context['request'].user + if user.is_anonymous(): return False - return auth.user and auth.user._id in obj.reports + return user._id in obj.reports def get_can_edit(self, obj): - auth = Auth(self.context['request'].user) - if not auth or auth.user.is_anonymous(): + user = self.context['request'].user + if user.is_anonymous(): return False - return obj.user._id == auth.user._id + return obj.user._id == user._id def get_has_children(self, obj): return bool(getattr(obj, 'commented', [])) From da0f8e3406bbf8b5962830f63fd7743e518fb10c Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Tue, 15 Dec 2015 13:55:46 -0500 Subject: [PATCH 29/41] Check for is_deleted not equal to True in find_unread --- website/project/model.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/project/model.py b/website/project/model.py index 864bb77702b..27696228d10 100644 --- a/website/project/model.py +++ b/website/project/model.py @@ -225,7 +225,7 @@ def find_unread(cls, user, node): view_timestamp = user.comments_viewed_timestamp.get(node._id, default_timestamp) n_unread = Comment.find(Q('node', 'eq', node) & Q('user', 'ne', user) & - Q('is_deleted', 'eq', False) & + Q('is_deleted', 'ne', True) & (Q('date_created', 'gt', view_timestamp) | Q('date_modified', 'gt', view_timestamp))).count() return n_unread From d6a04238455aecfdd291b36d409f908d45e12f3f Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Tue, 15 Dec 2015 14:25:24 -0500 Subject: [PATCH 30/41] Add error logging and return promises - Make methods that do async requests return promises. - Log unexpected errors to Sentry - Make setUnreadCommentCount a method of BaseComment --- website/static/js/comment.js | 72 +++++++++++++++++++++++++++++------- 1 file changed, 58 insertions(+), 14 deletions(-) diff --git a/website/static/js/comment.js b/website/static/js/comment.js index c87397d982d..74d8f86b9cd 100644 --- a/website/static/js/comment.js +++ b/website/static/js/comment.js @@ -169,14 +169,15 @@ BaseComment.prototype.fetch = function() { return new CommentModel(comment, self, self.$root); }) ); - setUnreadCommentCount(self); + self.setUnreadCommentCount(); deferred.resolve(self.comments()); self._loaded = true; }); - return deferred; + return deferred.promise(); }; -var setUnreadCommentCount = function(self) { +BaseComment.prototype.setUnreadCommentCount = function() { + var self = this; var request = osfHelpers.ajaxJSON( 'GET', osfHelpers.apiV2Url('nodes/' + window.contextVars.node.id + '/', {query: 'related_counts=True'}), @@ -184,6 +185,7 @@ var setUnreadCommentCount = function(self) { request.done(function(response) { self.unreadComments(response.data.relationships.comments.links.related.meta.unread); }); + return request; }; @@ -239,9 +241,14 @@ BaseComment.prototype.submitReply = function() { } self.onSubmitSuccess(response); }); - request.fail(function() { + request.fail(function(xhr, status, error) { self.cancelReply(); self.errorMessage('Could not submit comment'); + Raven.captureMessage('Error creating comment', { + url: url, + status: status, + error: error + }); }); }; @@ -357,9 +364,10 @@ CommentModel.prototype.submitEdit = function(data, event) { self.errorMessage('Please enter a comment'); return; } + var url = osfHelpers.apiV2Url('comments/' + self.id() + '/', {}); var request = osfHelpers.ajaxJSON( 'PUT', - osfHelpers.apiV2Url('comments/' + self.id() + '/', {}), + url, { 'isCors': true, 'data': { @@ -383,10 +391,16 @@ CommentModel.prototype.submitEdit = function(data, event) { // Refresh tooltip on date modified, if present $tips.tooltip('destroy').tooltip(); }); - request.fail(function() { + request.fail(function(xhr, status, error) { self.cancelEdit(); self.errorMessage('Could not submit comment'); + Raven.captureMessage('Error editing comment', { + url: url, + status: status, + error: error + }); }); + return request; }; CommentModel.prototype.reportAbuse = function() { @@ -401,9 +415,10 @@ CommentModel.prototype.cancelAbuse = function() { CommentModel.prototype.submitAbuse = function() { var self = this; + var url = osfHelpers.apiV2Url('comments/' + self.id() + '/reports/', {}); var request = osfHelpers.ajaxJSON( 'POST', - osfHelpers.apiV2Url('comments/' + self.id() + '/reports/', {}), + url, { 'isCors': true, 'data': { @@ -419,9 +434,15 @@ CommentModel.prototype.submitAbuse = function() { request.done(function() { self.isAbuse(true); }); - request.fail(function() { + request.fail(function(xhr, status, error) { self.errorMessage('Could not report abuse.'); + Raven.captureMessage('Error reporting abuse', { + url: url, + status: status, + error: error + }); }); + return request; }; CommentModel.prototype.startDelete = function() { @@ -430,9 +451,10 @@ CommentModel.prototype.startDelete = function() { CommentModel.prototype.submitDelete = function() { var self = this; + var url = osfHelpers.apiV2Url('comments/' + self.id() + '/', {}); var request = osfHelpers.ajaxJSON( 'PATCH', - osfHelpers.apiV2Url('comments/' + self.id() + '/', {}), + url, { 'isCors': true, 'data': { @@ -449,9 +471,15 @@ CommentModel.prototype.submitDelete = function() { self.isDeleted(true); self.deleting(false); }); - request.fail(function() { + request.fail(function(xhr, status, error) { self.deleting(false); + Raven.captureMessage('Error deleting comment', { + url: url, + status: status, + error: error + }); }); + return request; }; CommentModel.prototype.cancelDelete = function() { @@ -464,9 +492,10 @@ CommentModel.prototype.startUndelete = function() { CommentModel.prototype.submitUndelete = function() { var self = this; + var url = osfHelpers.apiV2Url('comments/' + self.id() + '/', {}); var request = osfHelpers.ajaxJSON( 'PATCH', - osfHelpers.apiV2Url('comments/' + self.id() + '/', {}), + url, { 'isCors': true, 'data': { @@ -482,9 +511,16 @@ CommentModel.prototype.submitUndelete = function() { request.done(function() { self.isDeleted(false); }); - request.fail(function() { + request.fail(function(xhr, status, error) { self.undeleting(false); + Raven.captureMessage('Error undeleting comment', { + url: url, + status: status, + error: error + }); + }); + return request; }; CommentModel.prototype.cancelUndelete = function() { @@ -497,17 +533,25 @@ CommentModel.prototype.startUnreportAbuse = function() { CommentModel.prototype.submitUnreportAbuse = function() { var self = this; + var url = osfHelpers.apiV2Url('comments/' + self.id() + '/reports/' + window.contextVars.currentUser.id + '/', {}); var request = osfHelpers.ajaxJSON( 'DELETE', - osfHelpers.apiV2Url('comments/' + self.id() + '/reports/' + window.contextVars.currentUser.id + '/', {}), + url, {'isCors': true} ); request.done(function() { self.isAbuse(false); }); - request.fail(function() { + request.fail(function(xhr, status, error) { self.unreporting(false); + Raven.captureMessage('Error unreporting comment', { + url: url, + status: status, + error: error + }); + }); + return request; }; CommentModel.prototype.cancelUnreportAbuse = function() { From f3ca37e747436f14e4270b4bb0efbe6a6deaf59e Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Tue, 15 Dec 2015 14:32:14 -0500 Subject: [PATCH 31/41] Refactor tests --- api/comments/serializers.py | 5 ++- .../comments/views/test_comment_detail.py | 9 ++--- .../nodes/views/test_node_comments_list.py | 33 +++++++++++++++---- 3 files changed, 32 insertions(+), 15 deletions(-) diff --git a/api/comments/serializers.py b/api/comments/serializers.py index 9dee970984d..c82a741c041 100644 --- a/api/comments/serializers.py +++ b/api/comments/serializers.py @@ -134,7 +134,10 @@ def create(self, validated_data): detail='Invalid comment target \'{}\'.'.format(target_id) ) validated_data['target'] = target - validated_data['content'] = validated_data.pop('get_content') + content = validated_data.pop('get_content') + validated_data['content'] = content.strip() + if not validated_data['content']: + raise ValidationError('Comment cannot be empty.') if node and node.can_comment(auth): comment = Comment.create(auth=auth, **validated_data) else: diff --git a/api_tests/comments/views/test_comment_detail.py b/api_tests/comments/views/test_comment_detail.py index 810a804787c..4ec9b58c729 100644 --- a/api_tests/comments/views/test_comment_detail.py +++ b/api_tests/comments/views/test_comment_detail.py @@ -201,12 +201,7 @@ def test_update_comment_cannot_exceed_max_length(self): 'id': self.comment._id, 'type': 'comments', 'attributes': { - 'content': ('contentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontent' - + 'contentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcon' - + 'tentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentconten' - + 'tcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentco' - + 'ntentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentconte' - + 'ntcontentcontentcontentcontentcontentcontent'), + 'content': ''.join(['c' for c in range(osf_settings.COMMENT_MAXLENGTH + 1)]), 'deleted': False } } @@ -214,7 +209,7 @@ def test_update_comment_cannot_exceed_max_length(self): res = self.app.put_json_api(self.private_url, payload, auth=self.user.auth, expect_errors=True) assert_equal(res.status_code, 400) assert_equal(res.json['errors'][0]['detail'], - 'Ensure this field has no more than ' + str(osf_settings.COMMENT_MAXLENGTH) + ' characters.') + 'Ensure this field has no more than {} characters.'.format(str(osf_settings.COMMENT_MAXLENGTH))) def test_update_comment_cannot_be_empty(self): self._set_up_private_project_with_comment() diff --git a/api_tests/nodes/views/test_node_comments_list.py b/api_tests/nodes/views/test_node_comments_list.py index a41a452d795..f94ee1e7bc1 100644 --- a/api_tests/nodes/views/test_node_comments_list.py +++ b/api_tests/nodes/views/test_node_comments_list.py @@ -415,6 +415,14 @@ def test_create_comment_trims_whitespace(self): 'type': 'comments', 'attributes': { 'content': ' ' + }, + 'relationships': { + 'target': { + 'data': { + 'type': 'nodes', + 'id': self.private_project._id + } + } } } } @@ -429,6 +437,14 @@ def test_create_comment_sanitizes_input(self): 'type': 'comments', 'attributes': { 'content': 'Cool Comment' + }, + 'relationships': { + 'target': { + 'data': { + 'type': 'nodes', + 'id': self.private_project._id + } + } } } } @@ -442,19 +458,22 @@ def test_create_comment_exceeds_max_length(self): 'data': { 'type': 'comments', 'attributes': { - 'content': ('contentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontent' - + 'contentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcon' - + 'tentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentconten' - + 'tcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentco' - + 'ntentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentcontentconte' - + 'ntcontentcontentcontentcontentcontentcontent') + 'content': (''.join(['c' for c in range(osf_settings.COMMENT_MAXLENGTH + 1)])) + }, + 'relationships': { + 'target': { + 'data': { + 'type': 'nodes', + 'id': self.private_project._id + } + } } } } res = self.app.post_json_api(self.private_url, payload, auth=self.user.auth, expect_errors=True) assert_equal(res.status_code, 400) assert_equal(res.json['errors'][0]['detail'], - 'Ensure this field has no more than ' + str(osf_settings.COMMENT_MAXLENGTH) + ' characters.') + 'Ensure this field has no more than {} characters.'.format(str(osf_settings.COMMENT_MAXLENGTH))) def test_create_comment_invalid_target_node(self): url = '/{}nodes/{}/comments/'.format(API_BASE, 'abcde') From 7f9acece7431afe176fb6c08e0ca0be246e0b7ac Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Tue, 15 Dec 2015 16:30:15 -0500 Subject: [PATCH 32/41] Add help text to new comment serializer fields --- api/comments/serializers.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/api/comments/serializers.py b/api/comments/serializers.py index c82a741c041..d9adcb481d4 100644 --- a/api/comments/serializers.py +++ b/api/comments/serializers.py @@ -42,9 +42,9 @@ class CommentSerializer(JSONAPISerializer): date_modified = ser.DateTimeField(read_only=True) modified = ser.BooleanField(read_only=True, default=False) deleted = ser.BooleanField(read_only=True, source='is_deleted', default=False) - is_abuse = ser.SerializerMethodField() - has_children = ser.SerializerMethodField() - can_edit = ser.SerializerMethodField() + is_abuse = ser.SerializerMethodField(help_text='Whether the current user reported this comment.') + has_children = ser.SerializerMethodField(help_text='Whether this comment has any replies.') + can_edit = ser.SerializerMethodField(help_text='Whether the current user can edit this comment.') # LinksField.to_representation adds link to "self" links = LinksField({}) From e22220586058f21e2e4884fca2fb605bd85192dd Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Wed, 16 Dec 2015 11:44:00 -0500 Subject: [PATCH 33/41] Pass in user info to the comment model Instead of making a request to get the current user's information, get the info off of contextVars and pass it in to the model. --- website/static/js/comment.js | 34 ++++--------------- .../static/js/pages/project-dashboard-page.js | 9 +++-- website/templates/project/project.mako | 2 -- website/templates/project/project_base.mako | 8 +++-- 4 files changed, 20 insertions(+), 33 deletions(-) diff --git a/website/static/js/comment.js b/website/static/js/comment.js index 74d8f86b9cd..bd30a26f32b 100644 --- a/website/static/js/comment.js +++ b/website/static/js/comment.js @@ -81,7 +81,6 @@ var BaseComment = function() { self.submittingReply = ko.observable(false); self.comments = ko.observableArray(); - self.unreadComments = ko.observable(0); self.displayCount = ko.computed(function() { @@ -132,23 +131,6 @@ BaseComment.prototype.setupToolTips = function(elm) { }); }; -BaseComment.prototype.fetchUserData = function() { - var self = this; - var request = osfHelpers.ajaxJSON( - 'GET', - osfHelpers.apiV2Url('users/me/', {}), - {'isCors': true}); - request.done(function(response) { - self.author = { - 'id': response.data.id, - 'url': response.data.links.html, - 'name': response.data.attributes.full_name, - 'gravatarUrl': response.data.links.profile_image - }; - return self.author; - }); -}; - BaseComment.prototype.fetch = function() { var self = this; var deferred = $.Deferred(); @@ -226,9 +208,7 @@ BaseComment.prototype.submitReply = function() { request.done(function(response) { self.cancelReply(); self.replyContent(null); - var commentModel = new CommentModel(response.data, self, self.$root); - commentModel.author = self.$root.author; - self.comments.unshift(commentModel); + self.comments.unshift(new CommentModel(response.data, self, self.$root)); if (!self.hasChildren()) { self.hasChildren(true); } @@ -279,6 +259,8 @@ var CommentModel = function(data, $parent, $root) { 'name': userData.attributes.full_name, 'gravatarUrl': userData.links.profile_image }; + } else { + self.author = self.$root.author; } self.contentDisplay = ko.observable(markdown.full.render(self.content())); @@ -571,7 +553,7 @@ CommentModel.prototype.onSubmitSuccess = function() { /* * */ -var CommentListModel = function(userName, canComment, hasChildren) { +var CommentListModel = function(canComment, hasChildren, currentUser) { BaseComment.prototype.constructor.call(this); @@ -582,12 +564,10 @@ var CommentListModel = function(userName, canComment, hasChildren) { self.editors = 0; self.commented = ko.observable(false); - self.userName = ko.observable(userName); self.canComment = ko.observable(canComment); self.hasChildren = ko.observable(hasChildren); self.discussion = ko.observableArray(); - - self.fetchUserData(); + self.author = currentUser; self.fetch(); self.fetchDiscussion(); @@ -629,9 +609,9 @@ var onOpen = function() { }); }; -var init = function(selector, userName, canComment, hasChildren) { +var init = function(selector, canComment, hasChildren, currentUser) { new CommentPane(selector, {onOpen: onOpen}); - var viewModel = new CommentListModel(userName, canComment, hasChildren); + var viewModel = new CommentListModel(canComment, hasChildren, currentUser); var $elm = $(selector); if (!$elm.length) { throw('No results found for selector'); diff --git a/website/static/js/pages/project-dashboard-page.js b/website/static/js/pages/project-dashboard-page.js index 76cc88d5c73..7c9fa660dc6 100644 --- a/website/static/js/pages/project-dashboard-page.js +++ b/website/static/js/pages/project-dashboard-page.js @@ -48,10 +48,15 @@ $('body').on('nodeLoad', function(event, data) { // Initialize comment pane w/ its viewmodel var $comments = $('#comments'); if ($comments.length) { - var userName = window.contextVars.currentUser.name; var canComment = window.contextVars.currentUser.canComment; var hasChildren = window.contextVars.node.hasChildren; - Comment.init('#commentPane', userName, canComment, hasChildren); + var currentUser = { + id: ctx.currentUser.id, + url: ctx.currentUser.urls.profile, + name: ctx.currentUser.fullname, + gravatarUrl: ctx.currentUser.gravatarUrl + }; + Comment.init('#commentPane', canComment, hasChildren, currentUser); } $(document).ready(function () { diff --git a/website/templates/project/project.mako b/website/templates/project/project.mako index 066603c2792..9029fec43ae 100644 --- a/website/templates/project/project.mako +++ b/website/templates/project/project.mako @@ -407,8 +407,6 @@ ${parent.javascript_bottom()} // Hack to allow mako variables to be accessed to JS modules window.contextVars = $.extend(true, {}, window.contextVars, { currentUser: { - id: ${user['id'] | sjson, n}, - name: ${ user_full_name | sjson, n }, canComment: ${ user['can_comment'] | sjson, n }, canEdit: ${ user['can_edit'] | sjson, n } }, diff --git a/website/templates/project/project_base.mako b/website/templates/project/project_base.mako index 827315f32d6..36010ac6984 100644 --- a/website/templates/project/project_base.mako +++ b/website/templates/project/project_base.mako @@ -71,10 +71,14 @@ ## TODO: Abstract me username: ${ user['username'] | sjson, n }, id: ${ user_id | sjson, n }, - urls: {api: userApiUrl}, + urls: { + api: userApiUrl, + profile: ${user_url | sjson, n} + }, isContributor: ${ user.get('is_contributor', False) | sjson, n }, fullname: ${ user['fullname'] | sjson, n }, - isAdmin: ${ user.get('is_admin', False) | sjson, n} + isAdmin: ${ user.get('is_admin', False) | sjson, n}, + gravatarUrl: ${user_gravatar | sjson, n} }, node: { ## TODO: Abstract me From f54790db427cd025bb84e52881f704abed8be6f2 Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Fri, 18 Dec 2015 16:50:37 -0500 Subject: [PATCH 34/41] Validate content max_length in Comment model --- tests/test_models.py | 11 +++++++++++ website/project/model.py | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/test_models.py b/tests/test_models.py index e7d8d3d6079..2be0c04b51d 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -4126,6 +4126,17 @@ def test_create(self): assert_equal(len(comment.node.logs), 2) assert_equal(comment.node.logs[-1].action, NodeLog.COMMENT_ADDED) + def test_create_comment_content_cannot_exceed_max_length(self): + with assert_raises(ValidationValueError): + comment = Comment.create( + auth=self.auth, + user=self.comment.user, + node=self.comment.node, + target=self.comment.target, + is_public=True, + content=''.join(['c' for c in range(settings.COMMENT_MAXLENGTH + 1)]) + ) + def test_create_sends_comment_added_signal(self): with capture_signals() as mock_signals: comment = Comment.create( diff --git a/website/project/model.py b/website/project/model.py index 27696228d10..05240ed22f3 100644 --- a/website/project/model.py +++ b/website/project/model.py @@ -180,7 +180,7 @@ class Comment(GuidStoredObject): modified = fields.BooleanField(default=False) is_deleted = fields.BooleanField(default=False) - content = fields.StringField() + content = fields.StringField(validate=MaxLengthValidator(settings.COMMENT_MAXLENGTH)) # Dictionary field mapping user IDs to dictionaries of report details: # { From 84d05a7ab4085e3b6b2a2ddc57effdcb58837ef9 Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Sat, 19 Dec 2015 15:25:05 -0500 Subject: [PATCH 35/41] Make content required in model & serializer --- api/comments/serializers.py | 2 +- tests/factories.py | 4 ++++ tests/test_models.py | 2 ++ tests/test_notifications.py | 12 ++++++------ website/project/model.py | 2 +- 5 files changed, 14 insertions(+), 8 deletions(-) diff --git a/api/comments/serializers.py b/api/comments/serializers.py index d9adcb481d4..78530b260a2 100644 --- a/api/comments/serializers.py +++ b/api/comments/serializers.py @@ -30,7 +30,7 @@ class CommentSerializer(JSONAPISerializer): id = IDField(source='_id', read_only=True) type = TypeField() - content = AuthorizedCharField(source='get_content', max_length=osf_settings.COMMENT_MAXLENGTH) + content = AuthorizedCharField(source='get_content', required=True, max_length=osf_settings.COMMENT_MAXLENGTH) target = TargetField(link_type='related', meta={'type': 'get_target_type'}) user = RelationshipField(related_view='users:user-detail', related_view_kwargs={'user_id': ''}) diff --git a/tests/factories.py b/tests/factories.py index e1dec255fb5..054f6ad1703 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -480,10 +480,12 @@ def _build(cls, target_class, *args, **kwargs): node = kwargs.pop('node', None) or NodeFactory() user = kwargs.pop('user', None) or node.creator target = kwargs.pop('target', None) or node + content = kwargs.pop('content', None) or 'Test comment.' instance = target_class( node=node, user=user, target=target, + content=content, *args, **kwargs ) return instance @@ -493,10 +495,12 @@ def _create(cls, target_class, *args, **kwargs): node = kwargs.pop('node', None) or NodeFactory() user = kwargs.pop('user', None) or node.creator target = kwargs.pop('target', None) or node + content = kwargs.pop('content', None) or 'Test comment.' instance = target_class( node=node, user=user, target=target, + content=content, *args, **kwargs ) instance.save() diff --git a/tests/test_models.py b/tests/test_models.py index 2be0c04b51d..e1e5fee21cf 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -4119,6 +4119,7 @@ def test_create(self): node=self.comment.node, target=self.comment.target, is_public=True, + content='This is a comment.' ) assert_equal(comment.user, self.comment.user) assert_equal(comment.node, self.comment.node) @@ -4145,6 +4146,7 @@ def test_create_sends_comment_added_signal(self): node=self.comment.node, target=self.comment.target, is_public=True, + content='This is a comment.' ) assert_equal(mock_signals.signals_sent(), set([comment_added])) diff --git a/tests/test_notifications.py b/tests/test_notifications.py index 0781fb5a162..3cc19a39f03 100644 --- a/tests/test_notifications.py +++ b/tests/test_notifications.py @@ -1095,12 +1095,12 @@ def test_check_user_comment_reply_subscription_if_email_not_sent_to_target_user( # reply to user (note: notify is called from Comment.create) reply = Comment.create( - auth=Auth(project.creator), - user=project.creator, - node=project, - content=content, - target=target, - is_public=True, + auth=Auth(project.creator), + user=project.creator, + node=project, + content=content, + target=target, + is_public=True, ) assert_true(mock_notify.called) assert_equal(mock_notify.call_count, 2) diff --git a/website/project/model.py b/website/project/model.py index 05240ed22f3..a4741cb97c3 100644 --- a/website/project/model.py +++ b/website/project/model.py @@ -180,7 +180,7 @@ class Comment(GuidStoredObject): modified = fields.BooleanField(default=False) is_deleted = fields.BooleanField(default=False) - content = fields.StringField(validate=MaxLengthValidator(settings.COMMENT_MAXLENGTH)) + content = fields.StringField(required=True, validate=MaxLengthValidator(settings.COMMENT_MAXLENGTH)) # Dictionary field mapping user IDs to dictionaries of report details: # { From af8a143ab4a8f308c4c2a9736bd8c8761beea564 Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Sat, 19 Dec 2015 15:45:19 -0500 Subject: [PATCH 36/41] Use field validator for content Instead of checking that the value is not empty in the create and update methods, use a field validator. Note: the trim_whitespace parameter gets applied after validation, allowing whitespace comments so a custom field validator is needed. --- api/comments/serializers.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/api/comments/serializers.py b/api/comments/serializers.py index 78530b260a2..592e4522e1e 100644 --- a/api/comments/serializers.py +++ b/api/comments/serializers.py @@ -52,6 +52,11 @@ class CommentSerializer(JSONAPISerializer): class Meta: type_ = 'comments' + def validate_content(self, value): + if value is None or not value.strip(): + raise ValidationError('Comment cannot be empty.') + return value + def get_is_abuse(self, obj): user = self.context['request'].user if user.is_anonymous(): @@ -73,9 +78,6 @@ def update(self, comment, validated_data): if validated_data: if 'get_content' in validated_data: content = validated_data.pop('get_content') - content = content.strip() - if not content: - raise ValidationError('Comment cannot be empty.') comment.edit(content, auth=auth, save=True) if validated_data.get('is_deleted', None) is True: comment.delete(auth, save=True) @@ -104,7 +106,7 @@ def get_validated_target_type(self, obj): target_type = self.context['request'].data.get('target_type') expected_target_type = self.get_target_type(target) if target_type != expected_target_type: - raise Conflict('Invalid target type. Expected \"{0}\", got \"{1}.\"'.format(expected_target_type, target_type)) + raise Conflict('Invalid target type. Expected "{0}", got "{1}."'.format(expected_target_type, target_type)) return target_type def get_target(self, node_id, target_id): @@ -134,10 +136,6 @@ def create(self, validated_data): detail='Invalid comment target \'{}\'.'.format(target_id) ) validated_data['target'] = target - content = validated_data.pop('get_content') - validated_data['content'] = content.strip() - if not validated_data['content']: - raise ValidationError('Comment cannot be empty.') if node and node.can_comment(auth): comment = Comment.create(auth=auth, **validated_data) else: From 3a34bb96120e61ff6e3650b4918e5352da826af4 Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Sun, 20 Dec 2015 14:32:57 -0500 Subject: [PATCH 37/41] Add Comment model content not empty validator Since empty comments are not allowed, add validation to the comment model as well. Add tests to check that a validation error is raised when trying to create an empty or whitespace comment. Also test that comments with content=None throw a validation error. --- tests/test_models.py | 36 ++++++++++++++++++++++++++++++++++++ website/project/model.py | 5 +++-- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/tests/test_models.py b/tests/test_models.py index e1e5fee21cf..08fb6ee5f06 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -4138,6 +4138,42 @@ def test_create_comment_content_cannot_exceed_max_length(self): content=''.join(['c' for c in range(settings.COMMENT_MAXLENGTH + 1)]) ) + def test_create_comment_content_cannot_be_none(self): + with assert_raises(ValidationError) as error: + comment = Comment.create( + auth=self.auth, + user=self.comment.user, + node=self.comment.node, + target=self.comment.target, + is_public=True, + content=None + ) + assert_equal(error.exception.message, 'Value is required.') + + def test_create_comment_content_cannot_be_empty(self): + with assert_raises(ValidationValueError) as error: + comment = Comment.create( + auth=self.auth, + user=self.comment.user, + node=self.comment.node, + target=self.comment.target, + is_public=True, + content='' + ) + assert_equal(error.exception.message, 'Ensure this value is not empty.') + + def test_create_comment_content_cannot_be_whitespace(self): + with assert_raises(ValidationValueError) as error: + comment = Comment.create( + auth=self.auth, + user=self.comment.user, + node=self.comment.node, + target=self.comment.target, + is_public=True, + content=' ' + ) + assert_equal(error.exception.message, 'Ensure this value is not empty.') + def test_create_sends_comment_added_signal(self): with capture_signals() as mock_signals: comment = Comment.create( diff --git a/website/project/model.py b/website/project/model.py index a4741cb97c3..9a14de9734a 100644 --- a/website/project/model.py +++ b/website/project/model.py @@ -17,7 +17,7 @@ from modularodm import Q from modularodm import fields -from modularodm.validators import MaxLengthValidator +from modularodm.validators import MaxLengthValidator, ValueNotEmptyValidator from modularodm.exceptions import NoResultsFound from modularodm.exceptions import ValidationTypeError from modularodm.exceptions import ValidationValueError @@ -180,7 +180,8 @@ class Comment(GuidStoredObject): modified = fields.BooleanField(default=False) is_deleted = fields.BooleanField(default=False) - content = fields.StringField(required=True, validate=MaxLengthValidator(settings.COMMENT_MAXLENGTH)) + content = fields.StringField(required=True, + validate=(MaxLengthValidator(settings.COMMENT_MAXLENGTH), ValueNotEmptyValidator())) # Dictionary field mapping user IDs to dictionaries of report details: # { From 8db0c060830dfbefbd422d2410ea6cdbbf9d5be2 Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Mon, 21 Dec 2015 10:27:06 -0500 Subject: [PATCH 38/41] Use string_required validator for content field Also make the string_required validator trim whitespace and then check that the string is empty. --- framework/mongo/validators.py | 2 +- tests/test_models.py | 4 ++-- website/project/model.py | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/framework/mongo/validators.py b/framework/mongo/validators.py index 0673ad64b6a..7279bd89b9e 100644 --- a/framework/mongo/validators.py +++ b/framework/mongo/validators.py @@ -6,7 +6,7 @@ def string_required(value): - if value is None or value == '': + if value is None or value.strip() == '': raise ValidationValueError('Value must not be empty.') return True diff --git a/tests/test_models.py b/tests/test_models.py index 08fb6ee5f06..b7a0937bf56 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -4160,7 +4160,7 @@ def test_create_comment_content_cannot_be_empty(self): is_public=True, content='' ) - assert_equal(error.exception.message, 'Ensure this value is not empty.') + assert_equal(error.exception.message, 'Value must not be empty.') def test_create_comment_content_cannot_be_whitespace(self): with assert_raises(ValidationValueError) as error: @@ -4172,7 +4172,7 @@ def test_create_comment_content_cannot_be_whitespace(self): is_public=True, content=' ' ) - assert_equal(error.exception.message, 'Ensure this value is not empty.') + assert_equal(error.exception.message, 'Value must not be empty.') def test_create_sends_comment_added_signal(self): with capture_signals() as mock_signals: diff --git a/website/project/model.py b/website/project/model.py index 9a14de9734a..2558838b87e 100644 --- a/website/project/model.py +++ b/website/project/model.py @@ -17,7 +17,7 @@ from modularodm import Q from modularodm import fields -from modularodm.validators import MaxLengthValidator, ValueNotEmptyValidator +from modularodm.validators import MaxLengthValidator from modularodm.exceptions import NoResultsFound from modularodm.exceptions import ValidationTypeError from modularodm.exceptions import ValidationValueError @@ -181,7 +181,7 @@ class Comment(GuidStoredObject): is_deleted = fields.BooleanField(default=False) content = fields.StringField(required=True, - validate=(MaxLengthValidator(settings.COMMENT_MAXLENGTH), ValueNotEmptyValidator())) + validate=[MaxLengthValidator(settings.COMMENT_MAXLENGTH), validators.string_required]) # Dictionary field mapping user IDs to dictionaries of report details: # { From 37d6cdf1448f948f6507eaec6b7eb0e7f88865c3 Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Mon, 21 Dec 2015 10:37:20 -0500 Subject: [PATCH 39/41] Add permissions checks to comment model methods Handle PermissionsErrors in comment serializer methods. --- api/comments/serializers.py | 23 +++++++++++++++++------ website/project/model.py | 8 ++++++++ 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/api/comments/serializers.py b/api/comments/serializers.py index 592e4522e1e..7866d176d53 100644 --- a/api/comments/serializers.py +++ b/api/comments/serializers.py @@ -1,5 +1,6 @@ from rest_framework import serializers as ser from framework.auth.core import Auth +from framework.exceptions import PermissionsError from website.project.model import Comment, Node from rest_framework.exceptions import ValidationError, PermissionDenied from api.base.exceptions import InvalidModelValueError, Conflict @@ -78,11 +79,20 @@ def update(self, comment, validated_data): if validated_data: if 'get_content' in validated_data: content = validated_data.pop('get_content') - comment.edit(content, auth=auth, save=True) + try: + comment.edit(content, auth=auth, save=True) + except PermissionsError: + raise PermissionDenied('Not authorized to edit this comment.') if validated_data.get('is_deleted', None) is True: - comment.delete(auth, save=True) + try: + comment.delete(auth, save=True) + except PermissionsError: + raise PermissionDenied('Not authorized to delete this comment.') elif comment.is_deleted: - comment.undelete(auth, save=True) + try: + comment.undelete(auth, save=True) + except PermissionsError: + raise PermissionDenied('Not authorized to undelete this comment.') return comment def get_target_type(self, obj): @@ -136,10 +146,11 @@ def create(self, validated_data): detail='Invalid comment target \'{}\'.'.format(target_id) ) validated_data['target'] = target - if node and node.can_comment(auth): + validated_data['content'] = validated_data.pop('get_content') + try: comment = Comment.create(auth=auth, **validated_data) - else: - raise PermissionDenied("Not authorized to comment on this project.") + except PermissionsError: + raise PermissionDenied('Not authorized to comment on this project.') return comment diff --git a/website/project/model.py b/website/project/model.py index 2558838b87e..f9df595f59d 100644 --- a/website/project/model.py +++ b/website/project/model.py @@ -234,6 +234,8 @@ def find_unread(cls, user, node): @classmethod def create(cls, auth, **kwargs): comment = cls(**kwargs) + if not comment.node.can_comment(auth): + raise PermissionsError('{0!r} does not have permission to comment on this node'.format(auth.user)) comment.save() comment.node.add_log( @@ -254,6 +256,8 @@ def create(cls, auth, **kwargs): return comment def edit(self, content, auth, save=False): + if not self.node.can_comment(auth) or self.user._id != auth.user._id: + raise PermissionsError('{0!r} does not have permission to edit this comment'.format(auth.user)) self.content = content self.modified = True self.node.add_log( @@ -271,6 +275,8 @@ def edit(self, content, auth, save=False): self.save() def delete(self, auth, save=False): + if not self.node.can_comment(auth) or self.user._id != auth.user._id: + raise PermissionsError('{0!r} does not have permission to comment on this node'.format(auth.user)) self.is_deleted = True self.node.add_log( NodeLog.COMMENT_REMOVED, @@ -287,6 +293,8 @@ def delete(self, auth, save=False): self.save() def undelete(self, auth, save=False): + if not self.node.can_comment(auth) or self.user._id != auth.user._id: + raise PermissionsError('{0!r} does not have permission to comment on this node'.format(auth.user)) self.is_deleted = False self.node.add_log( NodeLog.COMMENT_ADDED, From a84919cc207ddd772ee535df3f682017110dd5f1 Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Tue, 22 Dec 2015 09:59:57 -0500 Subject: [PATCH 40/41] Handle anonymous view only links for comments If a user is viewing a project through a VOL, pass the private_key to the API request to get comments. --- .../comments/views/test_comment_detail.py | 71 ++++++++++++++++++- website/project/model.py | 4 +- website/static/js/comment.js | 23 +++++- .../templates/include/comment_template.mako | 2 +- 4 files changed, 93 insertions(+), 7 deletions(-) diff --git a/api_tests/comments/views/test_comment_detail.py b/api_tests/comments/views/test_comment_detail.py index 4ec9b58c729..1853b2a08ef 100644 --- a/api_tests/comments/views/test_comment_detail.py +++ b/api_tests/comments/views/test_comment_detail.py @@ -4,7 +4,7 @@ from api.base.settings.defaults import API_BASE from api.base.settings import osf_settings from tests.base import ApiTestCase -from tests.factories import ProjectFactory, AuthUserFactory, CommentFactory, RegistrationFactory +from tests.factories import ProjectFactory, AuthUserFactory, CommentFactory, RegistrationFactory, PrivateLinkFactory class TestCommentDetailView(ApiTestCase): @@ -68,6 +68,27 @@ def test_private_node_logged_out_user_cannot_view_comment(self): res = self.app.get(self.private_url, expect_errors=True) assert_equal(res.status_code, 401) + def test_private_node_user_with_private_link_can_see_comment(self): + self._set_up_private_project_with_comment() + private_link = PrivateLinkFactory(anonymous=False) + private_link.nodes.append(self.private_project) + private_link.save() + res = self.app.get('/{}comments/{}/'.format(API_BASE, self.comment._id), {'view_only': private_link.key}, expect_errors=True) + assert_equal(res.status_code, 200) + assert_equal(self.comment._id, res.json['data']['id']) + assert_equal(self.comment.content, res.json['data']['attributes']['content']) + + def test_private_node_user_with_anonymous_link_cannot_see_commenter_info(self): + self._set_up_private_project_with_comment() + private_link = PrivateLinkFactory(anonymous=True) + private_link.nodes.append(self.private_project) + private_link.save() + res = self.app.get('/{}comments/{}/'.format(API_BASE, self.comment._id), {'view_only': private_link.key}, expect_errors=True) + assert_equal(res.status_code, 200) + assert_equal(self.comment._id, res.json['data']['id']) + assert_equal(self.comment.content, res.json['data']['attributes']['content']) + assert_not_in('user', res.json['data']['relationships']) + def test_public_node_logged_in_contributor_can_view_comment(self): self._set_up_public_project_with_comment() res = self.app.get(self.public_url, auth=self.user.auth) @@ -89,6 +110,15 @@ def test_public_node_logged_out_user_can_view_comment(self): assert_equal(self.public_comment._id, res.json['data']['id']) assert_equal(self.public_comment.content, res.json['data']['attributes']['content']) + def test_public_node_user_with_private_link_can_view_comment(self): + self._set_up_public_project_with_comment() + private_link = PrivateLinkFactory(anonymous=False) + private_link.nodes.append(self.public_project) + private_link.save() + res = self.app.get('/{}comments/{}/'.format(API_BASE, self.public_comment._id), {'view_only': private_link.key}, expect_errors=True) + assert_equal(self.public_comment._id, res.json['data']['id']) + assert_equal(self.public_comment.content, res.json['data']['attributes']['content']) + def test_registration_logged_in_contributor_can_view_comment(self): self._set_up_registration_with_comment() res = self.app.get(self.registration_url, auth=self.user.auth) @@ -480,6 +510,32 @@ def test_private_node_logged_out_user_cannot_see_deleted_comment(self): res = self.app.get(url, expect_errors=True) assert_equal(res.status_code, 401) + def test_private_node_view_only_link_user_cannot_see_deleted_comment(self): + self._set_up_private_project_with_comment() + self.comment.is_deleted = True + self.comment.save() + + private_link = PrivateLinkFactory(anonymous=False) + private_link.nodes.append(self.private_project) + private_link.save() + + res= self.app.get('/{}comments/{}/'.format(API_BASE, self.comment._id), {'view_only': private_link.key}, expect_errors=True) + assert_equal(res.status_code, 200) + assert_is_none(res.json['data']['attributes']['content']) + + def test_private_node_anonymous_view_only_link_user_cannot_see_deleted_comment(self): + self._set_up_private_project_with_comment() + self.comment.is_deleted = True + self.comment.save() + + anonymous_link = PrivateLinkFactory(anonymous=True) + anonymous_link.nodes.append(self.private_project) + anonymous_link.save() + + res= self.app.get('/{}comments/{}/'.format(API_BASE, self.comment._id), {'view_only': anonymous_link.key}, expect_errors=True) + assert_equal(res.status_code, 200) + assert_is_none(res.json['data']['attributes']['content']) + def test_public_node_only_logged_in_commenter_can_view_deleted_comment(self): public_project = ProjectFactory(is_public=True, creator=self.user) comment = CommentFactory(node=public_project, target=public_project, user=self.user) @@ -519,3 +575,16 @@ def test_public_node_logged_out_user_cannot_view_deleted_comments(self): res = self.app.get(url) assert_equal(res.status_code, 200) assert_is_none(res.json['data']['attributes']['content']) + + def test_public_node_view_only_link_user_cannot_see_deleted_comment(self): + self._set_up_public_project_with_comment() + self.public_comment.is_deleted = True + self.public_comment.save() + + private_link = PrivateLinkFactory(anonymous=False) + private_link.nodes.append(self.public_project) + private_link.save() + + res= self.app.get('/{}comments/{}/'.format(API_BASE, self.public_comment._id), {'view_only': private_link.key}, expect_errors=True) + assert_equal(res.status_code, 200) + assert_is_none(res.json['data']['attributes']['content']) diff --git a/website/project/model.py b/website/project/model.py index f9df595f59d..241d4a4afdf 100644 --- a/website/project/model.py +++ b/website/project/model.py @@ -206,10 +206,10 @@ def get_absolute_url(self): def get_content(self, auth): """ Returns the comment content if the user is allowed to see it. Deleted comments can only be viewed by the user who created the comment.""" - if (not auth or auth.user.is_anonymous()) and not self.node.is_public: + if not auth and not self.node.is_public: raise PermissionsError - if self.is_deleted and (((not auth or auth.user.is_anonymous()) and self.node.is_public) + if self.is_deleted and ((not auth or auth.user.is_anonymous()) or (auth and not auth.user.is_anonymous() and self.user._id != auth.user._id)): return None diff --git a/website/static/js/comment.js b/website/static/js/comment.js index bd30a26f32b..58d20ed8519 100644 --- a/website/static/js/comment.js +++ b/website/static/js/comment.js @@ -137,9 +137,14 @@ BaseComment.prototype.fetch = function() { if (self._loaded) { deferred.resolve(self.comments()); } - var url = osfHelpers.apiV2Url('nodes/' + window.contextVars.node.id + '/comments/', {query: 'embed=user'}); + var query = 'embed=user'; + var urlParams = osfHelpers.urlParams(); + if (urlParams.view_only) { + query = 'view_only=' + urlParams.view_only; + } + var url = osfHelpers.apiV2Url('nodes/' + window.contextVars.node.id + '/comments/', {query: query}); if (self.id() !== undefined) { - url = osfHelpers.apiV2Url('comments/' + self.id() + '/replies/', {query: 'embed=user'}); + url = osfHelpers.apiV2Url('comments/' + self.id() + '/replies/', {query: query}); } var request = osfHelpers.ajaxJSON( 'GET', @@ -160,9 +165,14 @@ BaseComment.prototype.fetch = function() { BaseComment.prototype.setUnreadCommentCount = function() { var self = this; + var query = 'related_counts=True'; + var urlParams = osfHelpers.urlParams(); + if (urlParams.view_only) { + query = query + '&view_only=' + urlParams.view_only; + } var request = osfHelpers.ajaxJSON( 'GET', - osfHelpers.apiV2Url('nodes/' + window.contextVars.node.id + '/', {query: 'related_counts=True'}), + osfHelpers.apiV2Url('nodes/' + window.contextVars.node.id + '/', {query: query}), {'isCors': true}); request.done(function(response) { self.unreadComments(response.data.relationships.comments.links.related.meta.unread); @@ -259,6 +269,13 @@ var CommentModel = function(data, $parent, $root) { 'name': userData.attributes.full_name, 'gravatarUrl': userData.links.profile_image }; + } else if (osfHelpers.urlParams().view_only) { + self.author = { + 'id': null, + 'url': '', + 'name': 'A User', + 'gravatarUrl': '' + }; } else { self.author = self.$root.author; } diff --git a/website/templates/include/comment_template.mako b/website/templates/include/comment_template.mako index 7fb0185e2f4..121930ffaf1 100644 --- a/website/templates/include/comment_template.mako +++ b/website/templates/include/comment_template.mako @@ -80,7 +80,7 @@
- + From 300953775dc20c839889478885b68c56a6d37331 Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Tue, 22 Dec 2015 10:31:09 -0500 Subject: [PATCH 41/41] Only get unread count if not priviate link --- api_tests/comments/views/test_comment_detail.py | 2 +- website/static/js/comment.js | 15 ++++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/api_tests/comments/views/test_comment_detail.py b/api_tests/comments/views/test_comment_detail.py index 1853b2a08ef..b689f5aa137 100644 --- a/api_tests/comments/views/test_comment_detail.py +++ b/api_tests/comments/views/test_comment_detail.py @@ -585,6 +585,6 @@ def test_public_node_view_only_link_user_cannot_see_deleted_comment(self): private_link.nodes.append(self.public_project) private_link.save() - res= self.app.get('/{}comments/{}/'.format(API_BASE, self.public_comment._id), {'view_only': private_link.key}, expect_errors=True) + res = self.app.get('/{}comments/{}/'.format(API_BASE, self.public_comment._id), {'view_only': private_link.key}, expect_errors=True) assert_equal(res.status_code, 200) assert_is_none(res.json['data']['attributes']['content']) diff --git a/website/static/js/comment.js b/website/static/js/comment.js index 58d20ed8519..eb2f9e433f1 100644 --- a/website/static/js/comment.js +++ b/website/static/js/comment.js @@ -137,15 +137,19 @@ BaseComment.prototype.fetch = function() { if (self._loaded) { deferred.resolve(self.comments()); } + var hasPrivateLink = false; + var query = 'embed=user'; var urlParams = osfHelpers.urlParams(); if (urlParams.view_only) { + hasPrivateLink = true; query = 'view_only=' + urlParams.view_only; } var url = osfHelpers.apiV2Url('nodes/' + window.contextVars.node.id + '/comments/', {query: query}); if (self.id() !== undefined) { url = osfHelpers.apiV2Url('comments/' + self.id() + '/replies/', {query: query}); } + var request = osfHelpers.ajaxJSON( 'GET', url, @@ -156,7 +160,9 @@ BaseComment.prototype.fetch = function() { return new CommentModel(comment, self, self.$root); }) ); - self.setUnreadCommentCount(); + if (!hasPrivateLink) { + self.setUnreadCommentCount(); + } deferred.resolve(self.comments()); self._loaded = true; }); @@ -165,14 +171,9 @@ BaseComment.prototype.fetch = function() { BaseComment.prototype.setUnreadCommentCount = function() { var self = this; - var query = 'related_counts=True'; - var urlParams = osfHelpers.urlParams(); - if (urlParams.view_only) { - query = query + '&view_only=' + urlParams.view_only; - } var request = osfHelpers.ajaxJSON( 'GET', - osfHelpers.apiV2Url('nodes/' + window.contextVars.node.id + '/', {query: query}), + osfHelpers.apiV2Url('nodes/' + window.contextVars.node.id + '/', {query: 'related_counts=True'}), {'isCors': true}); request.done(function(response) { self.unreadComments(response.data.relationships.comments.links.related.meta.unread);