API for editing comments via PATCH

pillar/api/nodes/__init__.py

@@ -469,6 +469,8 @@ def setup_app(app, url_prefix):
     app.on_insert_nodes += nodes_convert_markdown
     app.on_inserted_nodes += after_inserting_nodes
 
+    app.on_update_nodes += convert_markdown
+
     app.on_deleted_item_nodes += after_deleting_node
 
     app.register_api_blueprint(blueprint, url_prefix=url_prefix)

pillar/api/nodes/custom/comment.py

@@ -1,15 +1,19 @@
 """PATCH support for comment nodes."""
+
 import logging
 
-import werkzeug.exceptions as wz_exceptions
+from eve.methods.patch import patch_internal
 from flask import current_app
+import werkzeug.exceptions as wz_exceptions
+
 from pillar.api.utils import authorization, authentication, jsonify
 
 from . import register_patch_handler
 
 log = logging.getLogger(__name__)
 ROLES_FOR_COMMENT_VOTING = {u'subscriber', u'demo'}
-VALID_COMMENT_OPERATIONS = {u'upvote', u'downvote', u'revoke'}
+COMMENT_VOTING_OPS = {u'upvote', u'downvote', u'revoke'}
+VALID_COMMENT_OPERATIONS = COMMENT_VOTING_OPS.union({u'edit'})
 
 
 @register_patch_handler(u'comment')
@@ -17,7 +21,23 @@ def patch_comment(node_id, patch):
     assert_is_valid_patch(node_id, patch)
     user_id = authentication.current_user_id()
 
-    # Find the node
+    if patch[u'op'] in COMMENT_VOTING_OPS:
+        result, node = vote_comment(user_id, node_id, patch)
+    else:
+        assert patch[u'op'] == u'edit', 'Invalid patch operation %s' % patch[u'op']
+        result, node = edit_comment(user_id, node_id, patch)
+
+    return jsonify({'_status': 'OK',
+                    'result': result,
+                    'properties': node['properties']
+                    })
+
+
+def vote_comment(user_id, node_id, patch):
+    """Performs a voting operation."""
+
+    # Find the node. Includes a query on the properties.ratings array so
+    # that we only get the current user's rating.
     nodes_coll = current_app.data.driver.db['nodes']
     node_query = {'_id': node_id,
                   '$or': [{'properties.ratings.$.user': {'$exists': False}},
@@ -25,7 +45,7 @@ def patch_comment(node_id, patch):
     node = nodes_coll.find_one(node_query,
                                projection={'properties': 1})
     if node is None:
-        log.warning('How can the node not be found?')
+        log.warning('User %s wanted to patch non-existing node %s' % (user_id, node_id))
         raise wz_exceptions.NotFound('Node %s not found' % node_id)
 
     props = node['properties']
@@ -82,6 +102,7 @@ def patch_comment(node_id, patch):
     action = actions[patch['op']]
     mongo_update = action()
 
+    nodes_coll = current_app.data.driver.db['nodes']
     if mongo_update:
         log.info('Running %s', mongo_update)
         if rating:
@@ -97,10 +118,50 @@ def patch_comment(node_id, patch):
                                projection={'properties.rating_positive': 1,
                                            'properties.rating_negative': 1})
 
-    return jsonify({'_status': 'OK',
-                    'result': result,
-                    'properties': node['properties']
-                    })
+    return result, node
+
+
+def edit_comment(user_id, node_id, patch):
+    """Edits a single comment.
+
+    Doesn't do permission checking; users are allowed to edit their own
+    comment, and this is not something you want to revoke anyway. Admins
+    can edit all comments.
+    """
+
+    # Find the node. We need to fetch some more info than we use here, so that
+    # we can pass this stuff to Eve's patch_internal; that way the validation &
+    # authorisation system has enough info to work.
+    nodes_coll = current_app.data.driver.db['nodes']
+    projection = {'user': 1,
+                  'project': 1,
+                  'node_type': 1}
+    node = nodes_coll.find_one(node_id, projection=projection)
+    if node is None:
+        log.warning('User %s wanted to patch non-existing node %s' % (user_id, node_id))
+        raise wz_exceptions.NotFound('Node %s not found' % node_id)
+
+    if node['user'] != user_id and not authorization.user_has_role(u'admin'):
+        raise wz_exceptions.Forbidden('You can only edit your own comments.')
+
+    # Use Eve to PATCH this node, as that also updates the etag.
+    r, _, _, status = patch_internal('nodes',
+                                     {'properties.content': patch['content'],
+                                      'project': node['project'],
+                                      'user': node['user'],
+                                      'node_type': node['node_type']},
+                                     concurrency_check=False,
+                                     _id=node_id)
+    if status != 200:
+        log.error('Error %i editing comment %s for user %s: %s',
+                  status, node_id, user_id, r)
+        raise wz_exceptions.InternalServerError('Internal error %i from Eve' % status)
+    else:
+        log.info('User %s edited comment %s', user_id, node_id)
+
+    # Fetch the new content, so the client can show these without querying again.
+    node = nodes_coll.find_one(node_id, projection={'properties.content_html': 1})
+    return status, node
 
 
 def assert_is_valid_patch(node_id, patch):
@@ -112,8 +173,12 @@ def assert_is_valid_patch(node_id, patch):
         raise wz_exceptions.BadRequest("PATCH should have a key 'op' indicating the operation.")
 
     if op not in VALID_COMMENT_OPERATIONS:
-        raise wz_exceptions.BadRequest('Operation should be one of %s',
-                                       ', '.join(VALID_COMMENT_OPERATIONS))
+        raise wz_exceptions.BadRequest(u'Operation should be one of %s',
+                                       u', '.join(VALID_COMMENT_OPERATIONS))
+
+    if op not in COMMENT_VOTING_OPS:
+        # We can't check here, we need the node owner for that.
+        return
 
     # See whether the user is allowed to patch
     if authorization.user_matches_roles(ROLES_FOR_COMMENT_VOTING):