Major revision of comment system.

- Comments are stored in HTML as well as Markdown, so that conversion
  only happens when saving (rather than when viewing).
- Added 'markdown' Jinja filter for easy development. This is quite
  a heavy filter, so it shouldn't be used (much) in production.
- Added CLI command to update schemas on existing node types.

pillar/api/node_types/comment.py

@@ -6,6 +6,11 @@ node_type_comment = {
         'content': {
             'type': 'string',
             'minlength': 5,
+            'required': True,
+        },
+        # The converted-to-HTML content.
+        'content_html': {
+            'type': 'string',
         },
         'status': {
             'type': 'string',

pillar/api/node_types/group.py

@@ -1,6 +1,6 @@
 node_type_group = {
     'name': 'group',
-    'description': 'Generic group node type edited',
+    'description': 'Folder node type',
     'parent': ['group', 'project'],
     'dyn_schema': {
         # Used for sorting within the context of a group

pillar/api/nodes/__init__.py

@@ -8,6 +8,8 @@ import rsa.randnum
 import werkzeug.exceptions as wz_exceptions
 from bson import ObjectId
 from flask import current_app, g, Blueprint, request
+
+import pillar.markdown
 from pillar.api import file_storage
 from pillar.api.activities import activity_subscribe, activity_object_add
 from pillar.api.utils.algolia import algolia_index_node_delete
@@ -26,6 +28,11 @@ def only_for_node_type_decorator(required_node_type_name):
 
     If the node type is not of the required node type, returns None,
     otherwise calls the wrapped function.
+
+    >>> deco = only_for_node_type_decorator('comment')
+    >>> @deco
+    ... def handle_comment(node): pass
+
     """
 
     def only_for_node_type(wrapped):
@@ -35,6 +42,7 @@ def only_for_node_type_decorator(required_node_type_name):
                 return
 
             return wrapped(node, *args, **kwargs)
+
         return wrapper
 
     only_for_node_type.__doc__ = "Decorator, immediately returns when " \
@@ -415,8 +423,31 @@ def after_deleting_node(item):
                     item.get('_id'), ex)
 
 
-def setup_app(app, url_prefix):
+only_for_comments = only_for_node_type_decorator('comment')
 
+
+@only_for_comments
+def convert_markdown(node, original=None):
+    """Converts comments from Markdown to HTML.
+
+    Always does this on save, even when the original Markdown hasn't changed,
+    because our Markdown -> HTML conversion rules might have.
+    """
+
+    try:
+        content = node['properties']['content']
+    except KeyError:
+        node['properties']['content_html'] = ''
+    else:
+        node['properties']['content_html'] = pillar.markdown.markdown(content)
+
+
+def nodes_convert_markdown(nodes):
+    for node in nodes:
+        convert_markdown(node)
+
+
+def setup_app(app, url_prefix):
     from . import patch
     patch.setup_app(app, url_prefix=url_prefix)
 
@@ -427,6 +458,7 @@ def setup_app(app, url_prefix):
     app.on_fetched_resource_nodes += resource_parse_attachments
 
     app.on_replace_nodes += before_replacing_node
+    app.on_replace_nodes += convert_markdown
     app.on_replace_nodes += deduct_content_type
     app.on_replace_nodes += node_set_default_picture
     app.on_replaced_nodes += after_replacing_node
@@ -434,6 +466,7 @@ def setup_app(app, url_prefix):
     app.on_insert_nodes += before_inserting_nodes
     app.on_insert_nodes += nodes_deduct_content_type
     app.on_insert_nodes += nodes_set_default_picture
+    app.on_insert_nodes += nodes_convert_markdown
     app.on_inserted_nodes += after_inserting_nodes
 
     app.on_deleted_item_nodes += after_deleting_node

pillar/cli.py

@@ -5,9 +5,12 @@ Run commands with 'flask <command>'
 
 from __future__ import print_function, division
 
+import copy
 import logging
 
 from bson.objectid import ObjectId, InvalidId
+from eve.methods.put import put_internal
+
 from flask import current_app
 from flask_script import Manager
 
@@ -502,3 +505,125 @@ def move_group_node_project(node_uuid, dest_proj_url, force=False, skip_gcs=Fals
     mover.change_project(node, dest_proj)
 
     log.info('Done moving.')
+
+
+@manager.command
+@manager.option('-p', '--project', dest='proj_url', nargs='?',
+                help='Project URL')
+@manager.option('-a', '--all', dest='all_projects', action='store_true', default=False,
+                help='Replace on all projects.')
+def replace_pillar_node_type_schemas(proj_url=None, all_projects=False):
+    """Replaces the project's node type schemas with the standard Pillar ones.
+
+    Non-standard node types are left alone.
+    """
+
+    if bool(proj_url) == all_projects:
+        log.error('Use either --project or --all.')
+        return 1
+
+    from pillar.api.utils.authentication import force_cli_user
+    force_cli_user()
+
+    from pillar.api.node_types.asset import node_type_asset
+    from pillar.api.node_types.blog import node_type_blog
+    from pillar.api.node_types.comment import node_type_comment
+    from pillar.api.node_types.group import node_type_group
+    from pillar.api.node_types.group_hdri import node_type_group_hdri
+    from pillar.api.node_types.group_texture import node_type_group_texture
+    from pillar.api.node_types.hdri import node_type_hdri
+    from pillar.api.node_types.page import node_type_page
+    from pillar.api.node_types.post import node_type_post
+    from pillar.api.node_types.storage import node_type_storage
+    from pillar.api.node_types.text import node_type_text
+    from pillar.api.node_types.texture import node_type_texture
+    from pillar.api.utils import remove_private_keys
+
+    node_types = [node_type_asset, node_type_blog, node_type_comment, node_type_group,
+                  node_type_group_hdri, node_type_group_texture, node_type_hdri, node_type_page,
+                  node_type_post, node_type_storage, node_type_text, node_type_texture]
+    name_to_nt = {nt['name']: nt for nt in node_types}
+
+    projects_collection = current_app.db()['projects']
+
+    def handle_project(project):
+        log.info('Handling project %s', project['url'])
+
+        for proj_nt in project['node_types']:
+            nt_name = proj_nt['name']
+            try:
+                pillar_nt = name_to_nt[nt_name]
+            except KeyError:
+                log.info('   - skipping non-standard node type "%s"', nt_name)
+                continue
+
+            log.info('   - replacing schema on node type "%s"', nt_name)
+
+            # This leaves node type keys intact that aren't in Pillar's node_type_xxx definitions,
+            # such as permissions.
+            proj_nt.update(copy.deepcopy(pillar_nt))
+
+        # Use Eve to PUT, so we have schema checking.
+        db_proj = remove_private_keys(project)
+        r, _, _, status = put_internal('projects', db_proj, _id=project['_id'])
+        if status != 200:
+            log.error('Error %i storing altered project %s: %s', status, project['_id'], r)
+            return 4
+        log.info('Project saved succesfully.')
+
+    if all_projects:
+        for project in projects_collection.find():
+            handle_project(project)
+        return
+
+    project = projects_collection.find_one({'url': proj_url})
+    if not project:
+        log.error('Project url=%s not found', proj_url)
+        return 3
+
+    handle_project(project)
+
+
+@manager.command
+def remarkdown_comments():
+    """Retranslates all Markdown to HTML for all comment nodes.
+    """
+
+    from pillar.api.nodes import convert_markdown
+    from pprint import pformat
+
+    nodes_collection = current_app.db()['nodes']
+    comments = nodes_collection.find({'node_type': 'comment'},
+                                     projection={'properties.content': 1,
+                                                 'node_type': 1})
+
+    updated = identical = skipped = errors = 0
+    for node in comments:
+        convert_markdown(node)
+        node_id = node['_id']
+
+        try:
+            content_html = node['properties']['content_html']
+        except KeyError:
+            log.warning('Node %s has no content_html', node_id)
+            skipped += 1
+            continue
+
+        result = nodes_collection.update_one(
+            {'_id': node_id},
+            {'$set': {'properties.content_html': content_html}}
+        )
+        if result.matched_count != 1:
+            log.error('Unable to update node %s', node_id)
+            errors += 1
+            continue
+
+        if result.modified_count:
+            updated += 1
+        else:
+            identical += 1
+
+    log.info('updated  : %i', updated)
+    log.info('identical: %i', identical)
+    log.info('skipped  : %i', skipped)
+    log.info('errors   : %i', errors)

pillar/markdown.py

@@ -0,0 +1,44 @@
+"""Bleached Markdown functionality.
+
+This is for user-generated stuff, like comments.
+"""
+
+from __future__ import absolute_import
+
+import bleach
+import markdown as _markdown
+
+ALLOWED_TAGS = [
+    'a',
+    'abbr',
+    'acronym',
+    'b', 'strong',
+    'i', 'em',
+    'blockquote',
+    'code',
+    'li', 'ol', 'ul',
+    'h1', 'h2', 'h3', 'h4', 'h5', 'h6',
+    'p',
+    'img',
+]
+
+ALLOWED_ATTRIBUTES = {
+    'a': ['href', 'title', 'target'],
+    'abbr': ['title'],
+    'acronym': ['title'],
+    'img': ['src', 'alt', 'width', 'height', 'title'],
+    '*': ['style'],
+}
+
+ALLOWED_STYLES = [
+    'color', 'font-weight', 'background-color',
+]
+
+
+def markdown(s):
+    tainted_html = _markdown.markdown(s)
+    safe_html = bleach.clean(tainted_html,
+                             tags=ALLOWED_TAGS,
+                             attributes=ALLOWED_ATTRIBUTES,
+                             styles=ALLOWED_STYLES)
+    return safe_html

pillar/web/jinja.py

@@ -3,9 +3,12 @@
 from __future__ import absolute_import
 
 import jinja2.filters
+import jinja2.utils
 
+import pillar.api.utils
 from pillar.web.utils import pretty_date
 from pillar.web.nodes.routes import url_for_node
+import pillar.markdown
 
 
 def format_pretty_date(d):
@@ -37,9 +40,64 @@ def do_hide_none(s):
     return s
 
 
+# Source: Django, django/template/defaultfilters.py
+def do_pluralize(value, arg='s'):
+    """
+    Returns a plural suffix if the value is not 1. By default, 's' is used as
+    the suffix:
+
+    * If value is 0, vote{{ value|pluralize }} displays "0 votes".
+    * If value is 1, vote{{ value|pluralize }} displays "1 vote".
+    * If value is 2, vote{{ value|pluralize }} displays "2 votes".
+
+    If an argument is provided, that string is used instead:
+
+    * If value is 0, class{{ value|pluralize:"es" }} displays "0 classes".
+    * If value is 1, class{{ value|pluralize:"es" }} displays "1 class".
+    * If value is 2, class{{ value|pluralize:"es" }} displays "2 classes".
+
+    If the provided argument contains a comma, the text before the comma is
+    used for the singular case and the text after the comma is used for the
+    plural case:
+
+    * If value is 0, cand{{ value|pluralize:"y,ies" }} displays "0 candies".
+    * If value is 1, cand{{ value|pluralize:"y,ies" }} displays "1 candy".
+    * If value is 2, cand{{ value|pluralize:"y,ies" }} displays "2 candies".
+    """
+
+    if ',' not in arg:
+        arg = ',' + arg
+    bits = arg.split(',')
+    if len(bits) > 2:
+        return ''
+    singular_suffix, plural_suffix = bits[:2]
+
+    try:
+        if float(value) != 1:
+            return plural_suffix
+    except ValueError:  # Invalid string that's not a number.
+        pass
+    except TypeError:  # Value isn't a string or a number; maybe it's a list?
+        try:
+            if len(value) != 1:
+                return plural_suffix
+        except TypeError:  # len() of unsized object.
+            pass
+    return singular_suffix
+
+
+def do_markdown(s):
+    # FIXME: get rid of this filter altogether and cache HTML of comments.
+    safe_html = pillar.markdown.markdown(s)
+    return jinja2.utils.Markup(safe_html)
+
+
 def setup_jinja_env(jinja_env):
     jinja_env.filters['pretty_date'] = format_pretty_date
     jinja_env.filters['pretty_date_time'] = format_pretty_date_time
     jinja_env.filters['undertitle'] = format_undertitle
     jinja_env.filters['hide_none'] = do_hide_none
+    jinja_env.filters['pluralize'] = do_pluralize
+    jinja_env.filters['gravatar'] = pillar.api.utils.gravatar
+    jinja_env.filters['markdown'] = do_markdown
     jinja_env.globals['url_for_node'] = url_for_node

pillar/web/nodes/custom/comments.py

@@ -1,4 +1,7 @@
 import logging
+import warnings
+
+import flask
 from flask import current_app
 from flask import request
 from flask import jsonify
@@ -7,6 +10,8 @@ from flask_login import login_required, current_user
 from pillarsdk import Node
 from pillarsdk import Project
 import werkzeug.exceptions as wz_exceptions
+
+from pillar.web import subquery
 from pillar.web.nodes.routes import blueprint
 from pillar.web.utils import gravatar
 from pillar.web.utils import pretty_date
@@ -20,10 +25,22 @@ log = logging.getLogger(__name__)
 def comments_create():
     content = request.form['content']
     parent_id = request.form.get('parent_id')
+
+    if not parent_id:
+        log.warning('User %s tried to create comment without parent_id', current_user.objectid)
+        raise wz_exceptions.UnprocessableEntity()
+
     api = system_util.pillar_api()
     parent_node = Node.find(parent_id, api=api)
+    if not parent_node:
+        log.warning('Unable to create comment for user %s, parent node %r not found',
+                    current_user.objectid, parent_id)
+        raise wz_exceptions.UnprocessableEntity()
 
-    node_asset_props = dict(
+    log.warning('Creating comment for user %s on parent node %r',
+                current_user.objectid, parent_id)
+
+    comment_props = dict(
         project=parent_node.project,
         name='Comment',
         user=current_user.objectid,
@@ -36,20 +53,18 @@ def comments_create():
             rating_negative=0))
 
     if parent_id:
-        node_asset_props['parent'] = parent_id
+        comment_props['parent'] = parent_id
 
     # Get the parent node and check if it's a comment. In which case we flag
     # the current comment as a reply.
     parent_node = Node.find(parent_id, api=api)
     if parent_node.node_type == 'comment':
-        node_asset_props['properties']['is_reply'] = True
+        comment_props['properties']['is_reply'] = True
 
-    node_asset = Node(node_asset_props)
-    node_asset.create(api=api)
+    comment = Node(comment_props)
+    comment.create(api=api)
 
-    return jsonify(
-        asset_id=node_asset._id,
-        content=node_asset.properties.content)
+    return jsonify({'node_id': comment._id}), 201
 
 
 @blueprint.route('/comments/<string(length=24):comment_id>', methods=['POST'])
@@ -119,6 +134,8 @@ def format_comment(comment, is_reply=False, is_team=False, replies=None):
 
 @blueprint.route("/comments/")
 def comments_index():
+    warnings.warn('comments_index() is deprecated in favour of comments_for_node()')
+
     parent_id = request.args.get('parent_id')
     # Get data only if we format it
     api = system_util.pillar_api()
@@ -152,6 +169,50 @@ def comments_index():
     return return_content
 
 
+@blueprint.route('/<string(length=24):node_id>/comments')
+def comments_for_node(node_id):
+    """Shows the comments attached to the given node."""
+
+    api = system_util.pillar_api()
+
+    node = Node.find(node_id, api=api)
+    project = Project({'_id': node.project})
+    can_post_comments = project.node_type_has_method('comment', 'POST', api=api)
+
+    # Query for all children, i.e. comments on the node.
+    comments = Node.all({
+        'where': {'node_type': 'comment', 'parent': node_id},
+    }, api=api)
+
+    def enrich(some_comment):
+        some_comment['_user'] = subquery.get_user_info(some_comment['user'])
+        some_comment['_is_own'] = some_comment['user'] == current_user.objectid
+        some_comment['_current_user_rating'] = None  # tri-state boolean
+
+        if current_user.is_authenticated:
+            for rating in comment.properties.ratings or ():
+                if rating.user != current_user.objectid:
+                    continue
+
+                some_comment['_current_user_rating'] = rating.is_positive
+
+    for comment in comments['_items']:
+        # Query for all grandchildren, i.e. replies to comments on the node.
+        comment['_replies'] = Node.all({
+            'where': {'node_type': 'comment', 'parent': comment['_id']},
+        }, api=api)
+
+        enrich(comment)
+        for reply in comment['_replies']['_items']:
+            enrich(reply)
+
+    # Data will be requested via javascript
+    return render_template('nodes/custom/comment/list_embed.html',
+                           node_id=node_id,
+                           comments=comments,
+                           can_post_comments=can_post_comments)
+
+
 @blueprint.route("/comments/<comment_id>/rate/<operation>", methods=['POST'])
 @login_required
 def comments_rate(comment_id, operation):