Editing comments via PATCH on pillar-web, and some other comment fixes

pillar/web/nodes/custom/comments.py

@@ -70,26 +70,19 @@ def comments_create():
 @blueprint.route('/comments/<string(length=24):comment_id>', methods=['POST'])
 @login_required
 def comment_edit(comment_id):
-    """Allows a user to edit their comment (or any they have PUT access to)."""
+    """Allows a user to edit their comment."""
 
     api = system_util.pillar_api()
 
-    # Fetch the old comment.
+    comment = Node({'_id': comment_id})
-    comment_node = Node.find(comment_id, api=api)
+    result = comment.patch({'op': 'edit', 'content': request.form['content']}, api=api)
-    if comment_node.node_type != 'comment':
+    assert result['_status'] == 'OK'
-        log.info('POST to %s node %s done as if it were a comment edit; rejected.',
-                 comment_node.node_type, comment_id)
-        raise wz_exceptions.BadRequest('Node ID is not a comment.')
 
-    # Update the node.
+    return jsonify({
-    comment_node.properties.content = request.form['content']
+        'status': 'success',
-    update_ok = comment_node.update(api=api)
+        'data': {
-    if not update_ok:
+            'content_html': result.properties.content_html,
-        log.warning('Unable to update comment node %s: %s',
+        }})
-                    comment_id, comment_node.error)
-        raise wz_exceptions.InternalServerError('Unable to update comment node, unknown why.')
-
-    return '', 204
 
 
 def format_comment(comment, is_reply=False, is_team=False, replies=None):
@@ -233,13 +226,8 @@ def comments_rate(comment_id, operation):
 
     api = system_util.pillar_api()
 
-    comment = Node.find(comment_id, {'projection': {'_id': 1}}, api=api)
-    if not comment:
-        log.info('Node %i not found; how could someone click on the upvote/downvote button?',
-                 comment_id)
-        raise wz_exceptions.NotFound()
-
     # PATCH the node and return the result.
+    comment = Node({'_id': comment_id})
     result = comment.patch({'op': operation}, api=api)
     assert result['_status'] == 'OK'
 

src/scripts/tutti/2_comments.js

@@ -210,6 +210,9 @@ function comment_mode(clicked_item, mode)
 		$edit_buttons.find('.edit_mode').show();
 		$edit_buttons.find('.edit_cancel').hide();
 		$edit_buttons.find('.edit_save').hide();
+
+		$container.find('.comment-content').removeClass('editing');
+		$container.find('.comment-content-preview').html('').hide();
 	}
 }
 
@@ -218,22 +221,20 @@ function comment_mode(clicked_item, mode)
  *
  * clicked_item: save/cancel button.
  *
- * Returns a promise on the comment loading.
+ * Returns a promise on the comment loading if reload_comment=true.
  */
-function commentEditCancel(clicked_item) {
+function commentEditCancel(clicked_item, reload_comment) {
+	comment_mode(clicked_item, 'view');
+
 	var comment_container = $(clicked_item).closest('.comment-container');
 	var comment_id = comment_container.data('node-id');
 
+	if (!reload_comment) return;
+
 	return loadComment(comment_id, {'properties.content': 1})
 	.done(function(data) {
-		var comment_raw = data['properties']['content'];
+		var comment_html = data['properties']['content_html'];
-		var comment_html = convert(comment_raw);
+		comment_container.find('.comment-content').html(comment_html);
-
-		comment_mode(clicked_item, 'view');
-		comment_container.find('.comment-content')
-			.removeClass('editing')
-			.html(comment_html);
-		comment_container.find('.comment-content-preview').html('').hide();
 	})
 	.fail(function(data) {
 		if (console) console.log('Error fetching comment: ', xhr);
@@ -288,7 +289,9 @@ function save_comment(is_new_comment, $commentContainer)
 		$.post('/nodes/comments/' + commentId,
 			{'content': comment})
 		.fail(promise.reject)
-		.done(function(data) { promise.resolve(commentId, comment); });
+		.done(function(resp) {
+			promise.resolve(commentId, resp.data.content_html);
+		});
 	}
 
 	return promise;

src/templates/nodes/custom/comment/list_embed.jade

@@ -158,7 +158,7 @@ script.
 	});
 
 	$(document).on('click','body .comment-action-edit span.edit_cancel',function(e){
-		commentEditCancel(this);
+		commentEditCancel(this, true);
 	});
 
 	/* Save edited comment */
@@ -181,14 +181,12 @@ script.
 				show_comment_edit_button_error($button, "Houston! Try again?");
 			}
 		})
-		.done(function(comment_id, comment) {
+		.done(function(comment_id, comment_html) {
-			commentEditCancel($button)
+			commentEditCancel($button, false)
-			.done(function() {
+
-				// TODO: reload just this comment's HTML from the back-end,
+			$container.find('.comment-content')
-				// rather than relying on our JS-converted Markdown.
+				.html(comment_html)
-				$container.find('.comment-content').html(convert(comment));
+				.flashOnce();
-				$container.flashOnce();
-			});
 
 			$button
 				.html('<i class="pi-check"></i> save changes')

tests/test_api/test_patch.py

@@ -24,12 +24,12 @@ class AbstractPatchCommentTest(AbstractPillarTest):
         resp = self.post('/api/nodes', json=asset,
                          auth_token='owner-token',
                          expected_status=201)
-        asset_id = resp.json()['_id']
+        self.asset_id = resp.json()['_id']
 
         # Create the comment
         comment = {'description': '',
                    'project': self.project_id,
-                   'parent': asset_id,
+                   'parent': self.asset_id,
                    'node_type': 'comment',
                    'user': self.owner_id,
                    'properties': {'rating_positive': 0,
@@ -201,3 +201,19 @@ class EditCommentTest(AbstractPatchCommentTest):
                          patched_node['properties']['content'])
         self.assertEqual(u'<p>Purrrr kittycat</p>\n',
                          patched_node['properties']['content_html'])
+
+    def test_edit_noncomment_node(self):
+        url = '/api/nodes/%s' % self.asset_id
+
+        self.patch(url,
+                   json={'op': 'edit', 'content': 'Je moeder is niet je vader.'},
+                   auth_token='owner-token',
+                   expected_status=405)
+
+    def test_edit_nonexistant_node(self):
+        url = '/api/nodes/%s' % ('0' * 24)
+
+        self.patch(url,
+                   json={'op': 'edit', 'content': 'Je moeder is niet je vader.'},
+                   auth_token='owner-token',
+                   expected_status=404)