From 535af3aaceedbf99b52fbe737b341cb22d813dbf Mon Sep 17 00:00:00 2001 From: Gil Forcada Date: Fri, 5 Dec 2014 02:09:46 +0100 Subject: [PATCH 1/4] Count acquisition wrapped comments If the View permission is not set directly on the workflow, but instead is left to be acquired, total_comments will always return 0. --- plone/app/discussion/conversation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plone/app/discussion/conversation.py b/plone/app/discussion/conversation.py index 540bf97..e73546a 100644 --- a/plone/app/discussion/conversation.py +++ b/plone/app/discussion/conversation.py @@ -80,7 +80,7 @@ class Conversation(Traversable, Persistent, Explicit): @property def total_comments(self): public_comments = [ - x for x in self._comments.values() + x for x in self.values() if user_nobody.has_permission('View', x) ] return len(public_comments) From d5b5b1c2cd269f1cfa27fb47df8fd44d18a6c4ef Mon Sep 17 00:00:00 2001 From: Gil Forcada Date: Mon, 16 Feb 2015 10:07:02 +0100 Subject: [PATCH 2/4] Fix total_comments indexer "@property removes Acquisition wrappers from 'self'. I don't remember why exactly (some implementation detail of Acquisition), but it's a fact of life." Quote from @davisagli on https://github.com/plone/plone.app.discussion/pull/58 In short: removing the @property from total_comments fix the problem. --- plone/app/discussion/catalog.py | 2 +- plone/app/discussion/conversation.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/plone/app/discussion/catalog.py b/plone/app/discussion/catalog.py index 12382df..5034928 100644 --- a/plone/app/discussion/catalog.py +++ b/plone/app/discussion/catalog.py @@ -33,7 +33,7 @@ def total_comments(object): if object.meta_type != 'Discussion Item': try: conversation = IConversation(object) - return conversation.total_comments + return conversation.total_comments() 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 e73546a..01d3182 100644 --- a/plone/app/discussion/conversation.py +++ b/plone/app/discussion/conversation.py @@ -77,7 +77,6 @@ class Conversation(Traversable, Persistent, Explicit): parent = aq_inner(self.__parent__) return parent.restrictedTraverse('@@conversation_view').enabled() - @property def total_comments(self): public_comments = [ x for x in self.values() From 4ea41aba434c59ef40bcffbd5262f13013dc5d9d Mon Sep 17 00:00:00 2001 From: Gil Forcada Date: Mon, 16 Feb 2015 16:35:43 +0100 Subject: [PATCH 3/4] Fix tests --- plone/app/discussion/tests/test_comment.py | 2 +- .../app/discussion/tests/test_conversation.py | 22 +++++++++---------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/plone/app/discussion/tests/test_comment.py b/plone/app/discussion/tests/test_comment.py index 5277f5f..467680b 100644 --- a/plone/app/discussion/tests/test_comment.py +++ b/plone/app/discussion/tests/test_comment.py @@ -433,7 +433,7 @@ class RepliesTest(unittest.TestCase): self.assertEqual(len(replies), 0) # Make sure the first comment is still in the conversation - self.assertEqual(conversation.total_comments, 1) + self.assertEqual(conversation.total_comments(), 1) def test_traversal(self): # Create a nested structure of comment replies and check the traversal diff --git a/plone/app/discussion/tests/test_conversation.py b/plone/app/discussion/tests/test_conversation.py index 8400b05..b990a54 100644 --- a/plone/app/discussion/tests/test_conversation.py +++ b/plone/app/discussion/tests/test_conversation.py @@ -78,7 +78,7 @@ class ConversationTest(unittest.TestCase): self.assertEqual(new_id, comment.comment_id) self.assertEqual(len(list(conversation.getComments())), 1) self.assertEqual(len(tuple(conversation.getThreads())), 1) - self.assertEqual(conversation.total_comments, 1) + self.assertEqual(conversation.total_comments(), 1) self.assertTrue( conversation.last_comment_date - datetime.utcnow() < timedelta(seconds=1) @@ -91,7 +91,7 @@ class ConversationTest(unittest.TestCase): comment.author_username = "nobody" conversation.addComment(comment) comment.manage_permission("View", roles=tuple()) - self.assertEqual(0, conversation.total_comments) + self.assertEqual(0, conversation.total_comments()) self.assertEqual(None, conversation.last_comment_date) self.assertEqual(["nobody"], list(conversation.commentators)) self.assertEqual([], list(conversation.public_commentators)) @@ -112,7 +112,7 @@ class ConversationTest(unittest.TestCase): # make sure the comment has been added self.assertEqual(len(list(conversation.getComments())), 1) self.assertEqual(len(tuple(conversation.getThreads())), 1) - self.assertEqual(conversation.total_comments, 1) + self.assertEqual(conversation.total_comments(), 1) # delete the comment we just created del conversation[new_id] @@ -120,7 +120,7 @@ class ConversationTest(unittest.TestCase): # make sure there is no comment left in the conversation self.assertEqual(len(list(conversation.getComments())), 0) self.assertEqual(len(tuple(conversation.getThreads())), 0) - self.assertEqual(conversation.total_comments, 0) + self.assertEqual(conversation.total_comments(), 0) def test_delete_recursive(self): # Create a conversation. In this case we doesn't assign it to an @@ -195,7 +195,7 @@ class ConversationTest(unittest.TestCase): # Make sure the comment has been deleted as well self.assertEqual(len(list(conversation.getComments())), 0) self.assertEqual(len(tuple(conversation.getThreads())), 0) - self.assertEqual(conversation.total_comments, 0) + self.assertEqual(conversation.total_comments(), 0) def test_comments_enabled_on_doc_in_subfolder(self): typetool = self.portal.portal_types @@ -422,7 +422,7 @@ class ConversationTest(unittest.TestCase): conversation.addComment(comment2) conversation.addComment(comment3) - self.assertEqual(conversation.total_comments, 3) + self.assertEqual(conversation.total_comments(), 3) def test_commentators(self): # add and remove a few comments to make sure the commentators @@ -432,7 +432,7 @@ class ConversationTest(unittest.TestCase): # object, as we just want to check the Conversation object API. conversation = IConversation(self.portal.doc1) - self.assertEqual(conversation.total_comments, 0) + self.assertEqual(conversation.total_comments(), 0) # Add a four comments from three different users # Note: in real life, we always create @@ -459,7 +459,7 @@ class ConversationTest(unittest.TestCase): new_comment4_id = conversation.addComment(comment4) # check if all commentators are in the commentators list - self.assertEqual(conversation.total_comments, 4) + self.assertEqual(conversation.total_comments(), 4) self.assertTrue('Jim' in conversation.commentators) self.assertTrue('Joe' in conversation.commentators) self.assertTrue('Jack' in conversation.commentators) @@ -472,7 +472,7 @@ class ConversationTest(unittest.TestCase): self.assertTrue('Jim' in conversation.commentators) self.assertTrue('Joe' in conversation.commentators) self.assertTrue('Jack' in conversation.commentators) - self.assertEqual(conversation.total_comments, 3) + self.assertEqual(conversation.total_comments(), 3) # remove the second comment from Jack del conversation[new_comment4_id] @@ -481,7 +481,7 @@ class ConversationTest(unittest.TestCase): self.assertTrue('Jim' in conversation.commentators) self.assertTrue('Joe' in conversation.commentators) self.assertFalse('Jack' in conversation.commentators) - self.assertEqual(conversation.total_comments, 2) + self.assertEqual(conversation.total_comments(), 2) def test_last_comment_date(self): # add and remove some comments and check if last_comment_date @@ -868,7 +868,7 @@ class RepliesTest(unittest.TestCase): # check that replies only contain the direct comments # and no comments deeper than 1 - self.assertEqual(conversation.total_comments, 6) + self.assertEqual(conversation.total_comments(), 6) self.assertEqual(len(replies), 2) self.assertEqual(len(replies_to_comment1), 2) self.assertEqual(len(replies_to_comment1_1), 1) From 2a423095abee2b6d3e73b280a84500a591c9b8e7 Mon Sep 17 00:00:00 2001 From: Gil Forcada Date: Wed, 25 Feb 2015 21:45:54 +0100 Subject: [PATCH 4/4] Add note on CHANGES.rst --- CHANGES.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index b0bbbb8..5b8ca9f 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -20,6 +20,10 @@ Changelog - Read mail settings from new (Plone 5) registry. [timo] +- Remove @property from Conversation.total_comments as @property and + Acquisition don't play well together. + [gforcada] + 2.3.3 (2014-10-23) ------------------