diff --git a/sphinx/websupport/__init__.py b/sphinx/websupport/__init__.py index 7af8aa1cb..373622eb1 100644 --- a/sphinx/websupport/__init__.py +++ b/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, - username=None, rating=0, time=None, proposal=None): + def add_comment(self, text, node_id='', parent_id='', displayed=True, + 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) diff --git a/sphinx/websupport/comments/__init__.py b/sphinx/websupport/comments/__init__.py index 3d7f51542..10856dffd 100644 --- a/sphinx/websupport/comments/__init__.py +++ b/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() diff --git a/sphinx/websupport/comments/db.py b/sphinx/websupport/comments/db.py index ecb62afa5..91175ed38 100644 --- a/sphinx/websupport/comments/db.py +++ b/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 = relation(Node, backref='comments') - - parent_id = Column(Integer, ForeignKey(db_prefix + 'comments.id')) - parent = relation('Comment', backref='children', remote_side=[id]) + #node_id = Column(Integer, ForeignKey(db_prefix + 'nodes.id')) + #node = relation(Node, backref='comments') 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) - for child in self.children]} + '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) - # -1 if downvoted, +1 if upvoted, 0 if voted then unvoted. - value = Column(Integer, nullable=False) - + username = Column(String(64), primary_key=True) 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): - self.value = value - self.user_id = user_id + def __init__(self, comment_id, username, value): self.comment_id = comment_id + self.username = username + self.value = value diff --git a/sphinx/websupport/comments/sqlalchemystorage.py b/sphinx/websupport/comments/sqlalchemystorage.py index 294c48f44..0e11c4a09 100644 --- a/sphinx/websupport/comments/sqlalchemystorage.py +++ b/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: - 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: + if node_id 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, - node, parent) + time or datetime.now(), proposal, proposal_diff) 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() - data = {'source': node.source, - 'comments': [comment.serializable(user_id) - for comment in node.comments]} + node = session.query(Node).filter(Node.id == node_id).one() 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 diff --git a/tests/test_websupport.py b/tests/test_websupport.py index 53307fb4e..f92dbcdee 100644 --- a/tests/test_websupport.py +++ b/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() - comment = support.add_comment('First test comment', node=str(node.id)) - support.add_comment('Child test comment', parent=str(comment['id'])) - data = support.get_comments(str(node.id)) + nodes = session.query(Node).all() + first_node = nodes[0] + second_node = nodes[1] + + # 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']