From 05f0e7e4d8ed8978382e1e651554b5b9611d6f84 Mon Sep 17 00:00:00 2001
From: Patrick Gerken <do3cc@patrick-gerken.de>
Date: Thu, 28 Mar 2013 14:28:22 +0100
Subject: [PATCH] Many updates to handle non public comments properly

---
 CHANGES.txt                                   | 15 +++++++++++--
 plone/app/discussion/browser/moderation.py    |  8 +++++--
 plone/app/discussion/catalog.py               |  2 +-
 plone/app/discussion/conversation.py          | 21 +++++++++++++++----
 plone/app/discussion/interfaces.py            | 10 +++++++--
 ...unctional_test_comment_review_workflow.txt | 18 ++++++++++++++++
 plone/app/discussion/tests/test_catalog.py    | 12 ++++++++++-
 .../app/discussion/tests/test_conversation.py | 14 ++++++++++++-
 8 files changed, 87 insertions(+), 13 deletions(-)

diff --git a/CHANGES.txt b/CHANGES.txt
index f9c53f6..dc04a9f 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -5,13 +5,24 @@ Changelog
 ------------------
 
 - Do not raise an error when no workflow is assigned to the comment type.
-  [timo] 
+  [timo]
+
+- Add a conversation property public_commentators that only lists
+  commentators of comments that are public.
+  The commentators indexer indexes this field now.
+  The behavior of the conversation property commentators is
+  unchanged.
+  [do3cc]
+
+- The last comment date now only returns the date of the newest
+  published comment.
+  [do3cc]
 
 
 2.2.4 (2013-03-05)
 ------------------
 
-- Check for 'checked' attribute in a way that work also for jQuery 1.7 
+- Check for 'checked' attribute in a way that work also for jQuery 1.7
   [ichimdav]
 
 - Better fix for #13037 by removing submit event trigger altogether
diff --git a/plone/app/discussion/browser/moderation.py b/plone/app/discussion/browser/moderation.py
index 91e3290..aa7bb91 100644
--- a/plone/app/discussion/browser/moderation.py
+++ b/plone/app/discussion/browser/moderation.py
@@ -95,6 +95,7 @@ class DeleteComment(BrowserView):
         conversation = aq_parent(comment)
         content_object = aq_parent(conversation)
         del conversation[comment.id]
+        content_object.reindexObject()
         IStatusMessage(self.context.REQUEST).addStatusMessage(
             _("Comment deleted."),
             type="info")
@@ -201,12 +202,13 @@ class BulkActionsView(BrowserView):
         context = aq_inner(self.context)
         for path in self.paths:
             comment = context.restrictedTraverse(path)
+            content_object = aq_parent(aq_parent(comment))
             workflowTool = getToolByName(comment, 'portal_workflow')
             current_state = workflowTool.getInfoFor(comment, 'review_state')
             if current_state != 'published':
                 workflowTool.doActionFor(comment, 'publish')
-            catalog = getToolByName(comment, 'portal_catalog')
-            catalog.reindexObject(comment)
+            comment.reindexObject()
+            content_object.reindexObject()
 
     def mark_as_spam(self):
         raise NotImplementedError
@@ -223,4 +225,6 @@ class BulkActionsView(BrowserView):
         for path in self.paths:
             comment = context.restrictedTraverse(path)
             conversation = aq_parent(comment)
+            content_object = aq_parent(conversation)
             del conversation[comment.id]
+            content_object.reindexObject()
diff --git a/plone/app/discussion/catalog.py b/plone/app/discussion/catalog.py
index e7d321b..12382df 100644
--- a/plone/app/discussion/catalog.py
+++ b/plone/app/discussion/catalog.py
@@ -61,7 +61,7 @@ def commentators(object):
     if object.meta_type != 'Discussion Item':
         try:
             conversation = IConversation(object)
-            return tuple(conversation.commentators.keys())
+            return conversation.public_commentators
         except TypeError:  # pragma: no cover
             # The item is contentish but nobody
             # implemented an adapter for it
diff --git a/plone/app/discussion/conversation.py b/plone/app/discussion/conversation.py
index ec2520c..6e529a8 100644
--- a/plone/app/discussion/conversation.py
+++ b/plone/app/discussion/conversation.py
@@ -95,15 +95,28 @@ class Conversation(Traversable, Persistent, Explicit):
 
     @property
     def last_comment_date(self):
-        try:
-            return self._comments[self._comments.maxKey()].creation_date
-        except (ValueError, KeyError, AttributeError,):
-            return None
+        # self._comments is an Instance of a btree. The keys
+        # are always ordered
+        comment_keys = self._comments.keys()
+        for comment_key in reversed(comment_keys):
+            comment = self._comments[comment_key]
+            if user_nobody.has_permission('View', comment):
+                return comment.creation_date
+        return None
 
     @property
     def commentators(self):
         return self._commentators
 
+    @property
+    def public_commentators(self):
+        retval = set()
+        for comment in self._comments.values():
+            if not user_nobody.has_permission('View', comment):
+                continue
+            retval.add(comment.author_username)
+        return tuple(retval)
+
     def objectIds(self):
         return self._comments.keys()
 
diff --git a/plone/app/discussion/interfaces.py b/plone/app/discussion/interfaces.py
index d305bcb..15c674d 100644
--- a/plone/app/discussion/interfaces.py
+++ b/plone/app/discussion/interfaces.py
@@ -31,13 +31,13 @@ class IConversation(IIterableMapping):
     """
 
     total_comments = schema.Int(
-        title=_(u"Total number of comments on this item"),
+        title=_(u"Total number of public comments on this item"),
         min=0,
         readonly=True,
         )
 
     last_comment_date = schema.Date(
-        title=_(u"Date of the most recent comment"),
+        title=_(u"Date of the most recent public comment"),
         readonly=True,
         )
 
@@ -46,6 +46,12 @@ class IConversation(IIterableMapping):
         readonly=True,
         )
 
+    public_commentators = schema.Set(
+        title=_(u"The set of unique commentators (usernames) of"
+            " published_comments"),
+        readonly=True,
+        )
+
     def addComment(comment):
         """Adds a new comment to the list of comments, and returns the
         comment id that was assigned. The comment_id property on the comment
diff --git a/plone/app/discussion/tests/functional_test_comment_review_workflow.txt b/plone/app/discussion/tests/functional_test_comment_review_workflow.txt
index 4628341..6e03a4f 100644
--- a/plone/app/discussion/tests/functional_test_comment_review_workflow.txt
+++ b/plone/app/discussion/tests/functional_test_comment_review_workflow.txt
@@ -124,6 +124,11 @@ Anonymous user can not see any posts or comment actions
     >>> 'form.button.PublishComment' in unprivileged_browser.contents
     False
 
+The catalog does not list the comments yet:
+
+    >>> portal.portal_catalog.searchResults(id='doc', total_comments=0)
+    [<Products...]
+
 Users with 'Review comment' permission can see unapproved comments and comment
 actions.
 
@@ -176,6 +181,11 @@ Make sure anonyous users see the approved comment, but not the unapproved ones.
     >>> 'First anonymous comment' in unprivileged_browser.contents
     True
 
+Make sure the catalog only lists the public comments.
+
+    >>> portal.portal_catalog.searchResults(id='doc', total_comments=1)
+    [<Products...]
+
 
 Delete a comment in the comments view
 -------------------------------------
@@ -205,4 +215,12 @@ Make sure the second comment has been deleted.
     >>> 'Second anonymous comment' in browser.contents
     False
 
+Delete the first, published comment.
+    >>> browser.open(urldoc)
+    >>> browser.getControl('Delete', index=0).click()
+
+Make sure the catalog has been updated properly.
+
+    >>> portal.portal_catalog.searchResults(id='doc', total_comments=0)
+    [<Products...]
 
diff --git a/plone/app/discussion/tests/test_catalog.py b/plone/app/discussion/tests/test_catalog.py
index 1194db2..3405e58 100644
--- a/plone/app/discussion/tests/test_catalog.py
+++ b/plone/app/discussion/tests/test_catalog.py
@@ -172,7 +172,7 @@ class ConversationCatalogTest(unittest.TestCase):
                      ))
         doc1_brain = brains[0]
 
-        self.assertEqual(doc1_brain.commentators, ('Emma', 'Jim'))
+        self.assertEqual(doc1_brain.commentators, ('Jim', 'Emma'))
 
         # remove one comments
         del self.conversation[new_comment2_id]
@@ -205,6 +205,16 @@ class ConversationCatalogTest(unittest.TestCase):
         self.assertEqual(comment1_brain.last_comment_date, None)
         self.assertEqual(comment1_brain.total_comments, None)
 
+    def test_dont_index_private_commentators(self):
+        self.comment1.manage_permission("View", roles=tuple())
+        self.portal.doc1.reindexObject()
+        brains = self.catalog.searchResults(dict(
+                     path={'query':
+                           '/'.join(self.portal.doc1.getPhysicalPath())},
+                     portal_type="Document"
+                     ))
+        doc1_brain = brains[0]
+        self.assertEqual(doc1_brain.commentators, ())
 
 class CommentCatalogTest(unittest.TestCase):
 
diff --git a/plone/app/discussion/tests/test_conversation.py b/plone/app/discussion/tests/test_conversation.py
index a424a28..36861ae 100644
--- a/plone/app/discussion/tests/test_conversation.py
+++ b/plone/app/discussion/tests/test_conversation.py
@@ -69,7 +69,7 @@ class ConversationTest(unittest.TestCase):
         self.assertTrue(isinstance(comment.comment_id, long))
         self.assertTrue(IComment.providedBy(conversation[new_id]))
         self.assertEqual(aq_base(conversation[new_id].__parent__),
-                          aq_base(conversation))
+                         aq_base(conversation))
         self.assertEqual(new_id, comment.comment_id)
         self.assertEqual(len(list(conversation.getComments())), 1)
         self.assertEqual(len(tuple(conversation.getThreads())), 1)
@@ -77,6 +77,18 @@ class ConversationTest(unittest.TestCase):
         self.assertTrue(conversation.last_comment_date - datetime.utcnow() <
                      timedelta(seconds=1))
 
+    def test_private_comment(self):
+        conversation = IConversation(self.portal.doc1)
+
+        comment = createObject('plone.Comment')
+        comment.author_username = "nobody"
+        conversation.addComment(comment)
+        comment.manage_permission("View", roles=tuple())
+        self.assertEquals(0, conversation.total_comments)
+        self.assertEquals(None, conversation.last_comment_date)
+        self.assertEquals(["nobody"], list(conversation.commentators))
+        self.assertEquals([], list(conversation.public_commentators))
+
     def test_delete_comment(self):
         # Create a conversation. In this case we doesn't assign it to an
         # object, as we just want to check the Conversation object API.