Converted comment schema from adjacency list to materialized path.

Added tests for commments.
Layed groundwork for comment moderation.

sphinx/websupport/__init__.py

@@ -157,7 +157,7 @@ class WebSupport(object):
         document['title'] = 'Search Results'
         return document
 
-    def get_comments(self, node_id, user_id=None):
+    def get_comments(self, node_id, username=None, moderator=False):
         """Get the comments and source associated with `node_id`. If 
         `user_id` is given vote information will be included with the 
         returned comments. The default CommentBackend returns dict with
@@ -191,10 +191,11 @@ class WebSupport(object):
         :param node_id: the id of the node to get comments for.
         :param user_id: the id of the user viewing the comments.
         """
-        return self.storage.get_comments(node_id, user_id)
+        return self.storage.get_comments(node_id, username, moderator)
 
-    def add_comment(self, text, node='', parent='', displayed=True, 
+    def add_comment(self, text, node_id='', parent_id='', displayed=True, 
-                    username=None, rating=0, time=None, proposal=None):
+                    username=None, rating=0, time=None, proposal=None,
+                    moderator=False):
         """Add a comment to a node or another comment. Returns the comment 
         in the same format as :meth:`get_comments`. If the comment is being 
         attached to a node, pass in the node's id (as a string) with the 
@@ -215,15 +216,16 @@ class WebSupport(object):
 
         :param parent_id: the prefixed id of the comment's parent.
         :param text: the text of the comment.
-        :param displayed: for future use...
+        :param displayed: for moderation purposes
         :param username: the username of the user making the comment.
         :param rating: the starting rating of the comment, defaults to 0.
         :param time: the time the comment was created, defaults to now.
         """
         return self.storage.add_comment(text, displayed, username, rating,
-                                        time, proposal, node, parent)
+                                        time, proposal, node_id, parent_id, 
+                                        moderator)
 
-    def process_vote(self, comment_id, user_id, value):
+    def process_vote(self, comment_id, username, value):
         """Process a user's vote. The web support package relies
         on the API user to perform authentication. The API user will 
         typically receive a comment_id and value from a form, and then
@@ -248,4 +250,6 @@ class WebSupport(object):
         :param value: 1 for an upvote, -1 for a downvote, 0 for an unvote.
         """
         value = int(value)
-        self.storage.process_vote(comment_id, user_id, value)
+        if not -1 <= value <= 1:
+            raise ValueError('vote value %s out of range (-1, 1)' % value)
+        self.storage.process_vote(comment_id, username, value)

sphinx/websupport/comments/__init__.py

@@ -40,7 +40,7 @@ class StorageBackend(object):
         """Called when a comment is being added."""
         raise NotImplementedError()
 
-    def get_comments(self, parent_id, user_id):
+    def get_comments(self, parent_id, user_id, moderator):
         """Called to retrieve all comments for a node."""
         raise NotImplementedError()
 

sphinx/websupport/comments/db.py

@@ -32,7 +32,6 @@ class Node(Base):
     document = Column(String(256), nullable=False)
     line = Column(Integer)
     source = Column(Text, nullable=False)
-    treeloc = Column(String(32), nullable=False)
 
     def __init__(self, document, line, source, treeloc):
         self.document = document
@@ -51,26 +50,32 @@ class Comment(Base):
     username = Column(String(64))
     proposal = Column(Text)
     proposal_diff = Column(Text)
+    path = Column(String(256), index=True)
 
-    node_id = Column(Integer, ForeignKey(db_prefix + 'nodes.id'))
+    #node_id = Column(Integer, ForeignKey(db_prefix + 'nodes.id'))
-    node = relation(Node, backref='comments')
+    #node = relation(Node, backref='comments')
-
-    parent_id = Column(Integer, ForeignKey(db_prefix + 'comments.id'))
-    parent = relation('Comment', backref='children', remote_side=[id])
 
     def __init__(self, text, displayed, username, rating, time, 
-                 proposal, proposal_diff, node, parent):
+                 proposal, proposal_diff):
         self.text = text
         self.displayed = displayed
         self.username = username
         self.rating = rating
         self.time = time
-        self.node = node
-        self.parent = parent
         self.proposal = proposal
         self.proposal_diff = proposal_diff
 
-    def serializable(self, user_id=None):
+    def set_path(self, node_id, parent_id):
+        if node_id:
+            self.path = '%s.%s' % (node_id, self.id)
+        else:
+            session = Session()
+            parent_path = session.query(Comment.path).\
+                filter(Comment.id == parent_id).one().path
+            session.close()
+            self.path = '%s.%s' %  (parent_path, self.id)
+
+    def serializable(self, vote=0):
         delta = datetime.now() - self.time
 
         time = {'year': self.time.year,
@@ -82,15 +87,6 @@ class Comment(Base):
                 'iso': self.time.isoformat(),
                 'delta': self.pretty_delta(delta)}
 
-        vote = ''
-        if user_id is not None:
-            session = Session()
-            vote = session.query(CommentVote).filter(
-                CommentVote.comment_id == self.id).filter(
-                CommentVote.user_id == user_id).first()
-            vote = vote.value if vote is not None else 0
-            session.close()
-
         return {'text': self.text,
                 'username': self.username or 'Anonymous',
                 'id': self.id,
@@ -98,11 +94,8 @@ class Comment(Base):
                 'age': delta.seconds,
                 'time': time,
                 'vote': vote or 0,
-                'node': self.node.id if self.node else None,
-                'parent': self.parent.id if self.parent else None,
                 'proposal_diff': self.proposal_diff,
-                'children': [child.serializable(user_id)
+                'children': []}
-                             for child in self.children]}
 
     def pretty_delta(self, delta):
         days = delta.days
@@ -120,15 +113,14 @@ class Comment(Base):
 class CommentVote(Base):
     __tablename__ = db_prefix + 'commentvote'
 
-    user_id = Column(Integer, primary_key=True)
+    username = Column(String(64), primary_key=True)
-    # -1 if downvoted, +1 if upvoted, 0 if voted then unvoted.
-    value = Column(Integer, nullable=False)
-
     comment_id = Column(Integer, ForeignKey(db_prefix + 'comments.id'),
                         primary_key=True)
     comment = relation(Comment, backref="votes")
+    # -1 if downvoted, +1 if upvoted, 0 if voted then unvoted.
+    value = Column(Integer, nullable=False)
 
-    def __init__(self, comment_id, user_id, value):
+    def __init__(self, comment_id, username, value):
-        self.value = value
-        self.user_id = user_id
         self.comment_id = comment_id
+        self.username = username
+        self.value = value

sphinx/websupport/comments/sqlalchemystorage.py

@@ -11,6 +11,7 @@
 
 from datetime import datetime
 
+from sqlalchemy.orm import aliased
 from sphinx.websupport.comments import StorageBackend
 from sphinx.websupport.comments.db import Base, Node, Comment, CommentVote,\
                                           Session
@@ -37,51 +38,89 @@ class SQLAlchemyStorage(StorageBackend):
         self.build_session.close()
 
     def add_comment(self, text, displayed, username, rating, time,
-                    proposal, node, parent):
+                    proposal, node_id, parent_id, moderator):
         session = Session()
         
-        if node:
+        if node_id and proposal:
-            node = session.query(Node).filter(Node.id == node).first()
-            parent = None
-        else:
-            node = None
-            parent = session.query(Comment).filter(
-                Comment.id == parent).first()
-        if node and proposal:
             differ = CombinedHtmlDiff()
             proposal_diff = differ.make_html(node.source, proposal)
         else:
             proposal_diff = None
 
         comment = Comment(text, displayed, username, rating, 
-                          time or datetime.now(), proposal, proposal_diff, 
+                          time or datetime.now(), proposal, proposal_diff)
-                          node, parent)
         session.add(comment)
+        session.flush()
+        comment.set_path(node_id, parent_id)
         session.commit()
         comment = comment.serializable()
         session.close()
         return comment
 
-    def get_comments(self, node_id, user_id):
+    def get_comments(self, node_id, username, moderator):
         session = Session()
-        node = session.query(Node).filter(Node.id == node_id).first()
+        node = session.query(Node).filter(Node.id == node_id).one()
-        data = {'source': node.source,
-                'comments': [comment.serializable(user_id)
-                             for comment in node.comments]}
         session.close()
-        return data
+        comments = self._serializable_list(node_id, username, moderator)
+        return {'source': node.source,
+                'comments': comments}
 
-    def process_vote(self, comment_id, user_id, value):
+    def _serializable_list(self, node_id, username, moderator):
+        session = Session()
+
+        if username:
+            # If a username is provided, create a subquery to retrieve all
+            # votes by this user. We will outerjoin with the comment query
+            # with this subquery so we have a user's voting information.
+            sq = session.query(CommentVote).\
+                filter(CommentVote.username == username).subquery()
+            cvalias = aliased(CommentVote, sq)
+            q = session.query(Comment, cvalias.value).outerjoin(cvalias)
+        else:
+            q = session.query(Comment)
+
+        # Filter out all comments not descending from this node.
+        q = q.filter(Comment.path.like(node_id + '.%'))
+        # Filter out non-displayed comments if this isn't a moderator.
+        if not moderator:
+            q = q.filter(Comment.displayed == True)
+        # Retrieve all results. Results must be ordered by Comment.path
+        # so that we can easily transform them from a flat list to a tree.
+        results = q.order_by(Comment.path).all()
+        session.close()
+
+        # We now need to convert the flat list of results to a nested
+        # lists to form the comment tree. Results will by ordered by
+        # the materialized path.
+        comments = []
+        list_stack = [comments]
+        for r in results:
+            comment, vote = r if username else (r, 0)
+
+            inheritance_chain = comment.path.split('.')[1:]
+            
+            if len(inheritance_chain) == len(list_stack) + 1:
+                parent = list_stack[-1][-1]
+                list_stack.append(parent['children'])
+            elif len(inheritance_chain) < len(list_stack):
+                while len(inheritance_chain) < len(list_stack):
+                    list_stack.pop()
+
+            list_stack[-1].append(comment.serializable(vote=vote))
+        
+        return comments
+
+    def process_vote(self, comment_id, username, value):
         session = Session()
         vote = session.query(CommentVote).filter(
             CommentVote.comment_id == comment_id).filter(
-            CommentVote.user_id == user_id).first()
+            CommentVote.username == username).first()
 
         comment = session.query(Comment).filter(
             Comment.id == comment_id).first()
 
         if vote is None:
-            vote = CommentVote(comment_id, user_id, value)
+            vote = CommentVote(comment_id, username, value)
             comment.rating += value
         else:
             comment.rating += value - vote.value

tests/test_websupport.py

@@ -36,6 +36,7 @@ def clear_builddir():
 
 
 def teardown_module():
+    (test_root / 'generated').rmtree(True)
     clear_builddir()
 
 
@@ -130,13 +131,61 @@ def test_whoosh():
 @with_support()
 def test_comments(support):
     session = Session()
-    node = session.query(Node).first()
+    nodes = session.query(Node).all()
-    comment = support.add_comment('First test comment', node=str(node.id))
+    first_node = nodes[0]
-    support.add_comment('Child test comment', parent=str(comment['id']))
+    second_node = nodes[1]
-    data = support.get_comments(str(node.id))
+
+    # Create a displayed comment and a non displayed comment.
+    comment = support.add_comment('First test comment', 
+                                  node_id=str(first_node.id))
+    support.add_comment('Hidden comment', node_id=str(first_node.id), 
+                        displayed=False)
+    # Add a displayed and not displayed child to the displayed comment.
+    support.add_comment('Child test comment', parent_id=str(comment['id']))
+    support.add_comment('Hidden child test comment', 
+                        parent_id=str(comment['id']), displayed=False)
+    # Add a comment to another node to make sure it isn't returned later.
+    support.add_comment('Second test comment', 
+                        node_id=str(second_node.id))
+    
+    # Access the comments as a moderator.
+    data = support.get_comments(str(first_node.id), moderator=True)
+    comments = data['comments']
+    children = comments[0]['children']
+    assert len(comments) == 2
+    assert comments[1]['text'] == 'Hidden comment'
+    assert len(children) == 2
+    assert children[1]['text'] == 'Hidden child test comment'
+
+    # Access the comments without being a moderator.
+    data = support.get_comments(str(first_node.id))
     comments = data['comments']
     children = comments[0]['children']
     assert len(comments) == 1
     assert comments[0]['text'] == 'First test comment'
     assert len(children) == 1
     assert children[0]['text'] == 'Child test comment'
+
+    def check_rating(val):
+        data = support.get_comments(str(first_node.id))
+        comment = data['comments'][0]
+        assert comment['rating'] == val, '%s != %s' % (comment['rating'], val)
+        
+    support.process_vote(comment['id'], 'user_one', '1')
+    support.process_vote(comment['id'], 'user_two', '1')
+    support.process_vote(comment['id'], 'user_three', '1')
+    check_rating(3)
+    support.process_vote(comment['id'], 'user_one', '-1')
+    check_rating(1)
+    support.process_vote(comment['id'], 'user_one', '0')
+    check_rating(2)
+
+    # Make sure a vote with value > 1 or < -1 can't be cast.
+    raises(ValueError, support.process_vote, comment['id'], 'user_one', '2')
+    raises(ValueError, support.process_vote, comment['id'], 'user_one', '-2')
+
+    # Make sure past voting data is associated with comments when they are
+    # fetched.
+    data = support.get_comments(str(first_node.id), username='user_two')
+    comment = data['comments'][0]
+    assert comment['vote'] == 1, '%s != 1' % comment['vote']