diff --git a/pillar/api/nodes/__init__.py b/pillar/api/nodes/__init__.py
index 4648a40a..d34ce484 100644
--- a/pillar/api/nodes/__init__.py
+++ b/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)
diff --git a/pillar/api/nodes/custom/comment.py b/pillar/api/nodes/custom/comment.py
index e3794850..4842ce30 100644
--- a/pillar/api/nodes/custom/comment.py
+++ b/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):
diff --git a/tests/test_api/test_patch.py b/tests/test_api/test_patch.py
index 2b8f7f8c..8f1ecfa6 100644
--- a/tests/test_api/test_patch.py
+++ b/tests/test_api/test_patch.py
@@ -1,7 +1,7 @@
from pillar.tests import AbstractPillarTest
-class PatchCommentTest(AbstractPillarTest):
+class AbstractPatchCommentTest(AbstractPillarTest):
def setUp(self, **kwargs):
AbstractPillarTest.setUp(self, **kwargs)
@@ -46,6 +46,8 @@ class PatchCommentTest(AbstractPillarTest):
comment_info = resp.json()
self.node_url = '/api/nodes/%s' % comment_info['_id']
+
+class VoteCommentTest(AbstractPatchCommentTest):
def test_upvote_other_comment(self):
# Patch the node
res = self.patch(self.node_url,
@@ -161,3 +163,41 @@ class PatchCommentTest(AbstractPillarTest):
{u'user': unicode(other_user_ids[4]), u'is_positive': False},
{u'user': unicode(self.user_id), u'is_positive': False},
], patched_node['properties'].get('ratings', []))
+
+
+class EditCommentTest(AbstractPatchCommentTest):
+ def test_comment_edit_happy(self, token='owner-token'):
+ pre_node = self.get(self.node_url, auth_token=token).json()
+
+ res = self.patch(self.node_url,
+ json={'op': 'edit', 'content': 'Je moeder is niet je vader.'},
+ auth_token=token).json()
+ self.assertEqual(u'Je moeder is niet je vader.
\n',
+ res['properties']['content_html'])
+
+ # Get the node again, to inspect its changed state.
+ patched_node = self.get(self.node_url, auth_token=token).json()
+ self.assertEqual(u'Je moeder is niet je vader.',
+ patched_node['properties']['content'])
+ self.assertEqual(u'Je moeder is niet je vader.
\n',
+ patched_node['properties']['content_html'])
+ self.assertNotEqual(pre_node['_etag'], patched_node['_etag'])
+
+ def test_comment_edit_other_user_admin(self):
+ admin_id = self.create_user(user_id=24 * 'c', roles={u'admin'})
+ self.create_valid_auth_token(admin_id, 'admin-token')
+
+ self.test_comment_edit_happy(token='admin-token')
+
+ def test_comment_edit_other_user_nonadmin(self):
+ self.patch(self.node_url,
+ json={'op': 'edit', 'content': 'Je moeder is niet je vader.'},
+ auth_token='token',
+ expected_status=403)
+
+ # Get the node again, to inspect its old state.
+ patched_node = self.get(self.node_url, auth_token='token').json()
+ self.assertEqual(u'Purrrr kittycat',
+ patched_node['properties']['content'])
+ self.assertEqual(u'Purrrr kittycat
\n',
+ patched_node['properties']['content_html'])