From 2b18d5a2e4e65ceddd551697b1ae73713e0ffab4 Mon Sep 17 00:00:00 2001 From: Timo Stollenwerk Date: Tue, 13 May 2014 07:07:37 +0200 Subject: [PATCH] Revert "Merge pull request #38 from delib/evilbungle-comment-acquisition" This reverts commit e18598e316e824795e752e7bb1c90ee37dc1cc6f, reversing changes made to fd6ac0788bf3ade982e60ede7ceda26b0c0e598e. This pull request introduces two test failures. See http://jenkins.plone.org/job/plone-5.0-python-2.7/lastCompletedBuild/testReport/ for details. --- plone/app/discussion/conversation.py | 18 +- plone/app/discussion/testing.py | 4 - plone/app/discussion/tests/configure.zcml | 16 -- plone/app/discussion/tests/profile/types.xml | 4 - .../profile/types/sample_content_type.xml | 47 ---- .../discussion/tests/profile/workflows.xml | 4 - .../definition.xml | 75 ------ .../app/discussion/tests/test_acquisition.py | 243 ------------------ 8 files changed, 6 insertions(+), 405 deletions(-) delete mode 100644 plone/app/discussion/tests/configure.zcml delete mode 100644 plone/app/discussion/tests/profile/types.xml delete mode 100644 plone/app/discussion/tests/profile/types/sample_content_type.xml delete mode 100644 plone/app/discussion/tests/profile/workflows.xml delete mode 100644 plone/app/discussion/tests/profile/workflows/comment_workflow_acquired_view/definition.xml delete mode 100644 plone/app/discussion/tests/test_acquisition.py diff --git a/plone/app/discussion/conversation.py b/plone/app/discussion/conversation.py index ca22c05..ba27c0f 100644 --- a/plone/app/discussion/conversation.py +++ b/plone/app/discussion/conversation.py @@ -50,14 +50,8 @@ from plone.app.discussion.comment import Comment from AccessControl.SpecialUsers import nobody as user_nobody -from ComputedAttribute import ComputedAttribute - ANNOTATION_KEY = 'plone.app.discussion:conversation' -def computed_attribute_decorator(level=0): - def computed_attribute_wrapper(func): - return ComputedAttribute(func, level) - return computed_attribute_wrapper class Conversation(Traversable, Persistent, Explicit): """A conversation is a container for all comments on a content object. @@ -93,21 +87,21 @@ class Conversation(Traversable, Persistent, Explicit): parent = aq_inner(self.__parent__) return parent.restrictedTraverse('@@conversation_view').enabled() - @computed_attribute_decorator(level=1) + @property def total_comments(self): public_comments = [ - x for x in self.values() + x for x in self._comments.values() if user_nobody.has_permission('View', x) ] return len(public_comments) - @computed_attribute_decorator(level=1) + @property def last_comment_date(self): # 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[comment_key] + comment = self._comments[comment_key] if user_nobody.has_permission('View', comment): return comment.creation_date return None @@ -116,10 +110,10 @@ class Conversation(Traversable, Persistent, Explicit): def commentators(self): return self._commentators - @computed_attribute_decorator(level=1) + @property def public_commentators(self): retval = set() - for comment in self.values(): + for comment in self._comments.values(): if not user_nobody.has_permission('View', comment): continue retval.add(comment.author_username) diff --git a/plone/app/discussion/testing.py b/plone/app/discussion/testing.py index e47026f..e7a397d 100644 --- a/plone/app/discussion/testing.py +++ b/plone/app/discussion/testing.py @@ -41,14 +41,10 @@ class PloneAppDiscussion(PloneSandboxLayer): xmlconfig.file('configure.zcml', plone.app.discussion, context=configurationContext) - xmlconfig.file('configure.zcml', - plone.app.discussion.tests, - context=configurationContext) def setUpPloneSite(self, portal): # Install into Plone site using portal_setup applyProfile(portal, 'plone.app.discussion:default') - applyProfile(portal, 'plone.app.discussion.tests:testing') # Creates some users acl_users = getToolByName(portal, 'acl_users') diff --git a/plone/app/discussion/tests/configure.zcml b/plone/app/discussion/tests/configure.zcml deleted file mode 100644 index ee79b95..0000000 --- a/plone/app/discussion/tests/configure.zcml +++ /dev/null @@ -1,16 +0,0 @@ - - - - - - - diff --git a/plone/app/discussion/tests/profile/types.xml b/plone/app/discussion/tests/profile/types.xml deleted file mode 100644 index 0aec569..0000000 --- a/plone/app/discussion/tests/profile/types.xml +++ /dev/null @@ -1,4 +0,0 @@ - - - - diff --git a/plone/app/discussion/tests/profile/types/sample_content_type.xml b/plone/app/discussion/tests/profile/types/sample_content_type.xml deleted file mode 100644 index 7869638..0000000 --- a/plone/app/discussion/tests/profile/types/sample_content_type.xml +++ /dev/null @@ -1,47 +0,0 @@ - - - - - sample_content_type - Sample Content - document_icon.png - True - True - - - True - - plone.dexterity.content.Item - - cmf.AddPortalContent - - - - - - view - False - - - - - - - - - - - - - - - - - - - diff --git a/plone/app/discussion/tests/profile/workflows.xml b/plone/app/discussion/tests/profile/workflows.xml deleted file mode 100644 index 500f444..0000000 --- a/plone/app/discussion/tests/profile/workflows.xml +++ /dev/null @@ -1,4 +0,0 @@ - - - - diff --git a/plone/app/discussion/tests/profile/workflows/comment_workflow_acquired_view/definition.xml b/plone/app/discussion/tests/profile/workflows/comment_workflow_acquired_view/definition.xml deleted file mode 100644 index 89a9fb2..0000000 --- a/plone/app/discussion/tests/profile/workflows/comment_workflow_acquired_view/definition.xml +++ /dev/null @@ -1,75 +0,0 @@ - - - Access contents information - Change portal events - Modify portal content - View - - Visible to everyone, editable by the owner. - - Anonymous - - - Editor - Manager - Owner - Site Administrator - - - Editor - Manager - Owner - Site Administrator - - - - - - Previous transition - - transition/getId|nothing - - - - - - The ID of the user who performed the previous transition - - user/getId - - - - - - Comment about the last transition - - python:state_change.kwargs.get('comment', '') - - - - - - Provides access to workflow history - - state_change/getHistory - - - Request review - Review portal content - - - - When the previous transition was performed - - state_change/getDateTime - - - - - diff --git a/plone/app/discussion/tests/test_acquisition.py b/plone/app/discussion/tests/test_acquisition.py deleted file mode 100644 index c6a96db..0000000 --- a/plone/app/discussion/tests/test_acquisition.py +++ /dev/null @@ -1,243 +0,0 @@ -# -*- coding: utf-8 -*- -from AccessControl.User import User # before SpecialUsers -from AccessControl.SpecialUsers import nobody as user_nobody -from AccessControl.PermissionRole import rolesForPermissionOn -from Acquisition import aq_chain, aq_base -from plone.app.discussion.testing import \ - PLONE_APP_DISCUSSION_INTEGRATION_TESTING -from plone.app.discussion.interfaces import IConversation -from plone.app.testing import TEST_USER_ID, setRoles -from Products.CMFCore.utils import getToolByName -from zope.component import createObject - -import unittest2 as unittest - - -dexterity_type_name = 'sample_content_type' -dexterity_object_id = 'instance-of-dexterity-type' -archetypes_object_id = 'instance-of-archetypes-type' -one_state_workflow = 'one_state_workflow' -comment_workflow_acquired_view = 'comment_workflow_acquired_view' - -class AcquisitionTest(unittest.TestCase): - """ Define the expected behaviour of wrapped and unwrapped comments. """ - - layer = PLONE_APP_DISCUSSION_INTEGRATION_TESTING - - def setUp(self): - self.portal = self.layer['portal'] - self.request = self.layer['request'] - setRoles(self.portal, TEST_USER_ID, ['Manager']) - self.wftool = getToolByName(self.portal, 'portal_workflow') - - # Use customized workflow for comments. - self.wftool.setChainForPortalTypes( - ['Discussion Item'], - (comment_workflow_acquired_view,), - ) - - # Use one_state_workflow for Document and sample_content_type, - # so they're always published. - self.wftool.setChainForPortalTypes( - ['Document', dexterity_type_name], - (one_state_workflow,), - ) - - # Create a dexterity item and add a comment. - self.portal.invokeFactory( - id=dexterity_object_id, - title='Instance Of Dexterity Type', - type_name=dexterity_type_name, - ) - - self.dexterity_object = self.portal.get(dexterity_object_id) - dx_conversation = IConversation(self.dexterity_object) - self.dexterity_conversation = dx_conversation - dx_comment = createObject('plone.Comment') - dx_conversation.addComment(dx_comment) - self.unwrapped_dexterity_comment = dx_comment - self.wrapped_dexterity_comment = dx_conversation[dx_comment.id] - - # Create an Archetypes item and add a comment. - self.portal.invokeFactory( - id=archetypes_object_id, - title='Instance Of Archetypes Type', - type_name='Document', - ) - - self.archetypes_object = self.portal.get(archetypes_object_id) - at_conversation = IConversation(self.archetypes_object) - self.archetypes_conversation = at_conversation - at_comment = createObject('plone.Comment') - at_conversation.addComment(at_comment) - self.unwrapped_archetypes_comment = at_comment - self.wrapped_archetypes_comment = at_conversation[at_comment.id] - - - def test_workflows_installed(self): - """Check that the new comment workflow has been installed properly. - (Just a test to check our test setup.) - """ - workflows = self.wftool.objectIds() - self.assertTrue('comment_workflow_acquired_view' in workflows) - - def test_workflows_applied(self): - """Check that all objects have the workflow that we expect. - (Just a test to check our test setup.)""" - self.assertEqual( - self.wftool.getChainFor(self.archetypes_object), - (one_state_workflow,) - ) - self.assertEqual( - self.wftool.getChainFor(self.dexterity_object), - (one_state_workflow,) - ) - self.assertEqual( - self.wftool.getChainFor(self.unwrapped_archetypes_comment), - (comment_workflow_acquired_view,) - ) - self.assertEqual( - self.wftool.getChainFor(self.unwrapped_dexterity_comment), - (comment_workflow_acquired_view,) - ) - - def test_comment_acquisition_chain(self): - """ Test that the acquisition chains for wrapped and unwrapped - comments are as expected. """ - - # Unwrapped comments rely on __parent__ attributes to determine - # parentage. Frustratingly there is no guarantee that __parent__ - # is always set, so the computed acquisition chain may be short. - # In this case the unwrapped AT and DX objects stored as the - # conversation parents don't have a __parent__, preventing the portal - # from being included in the chain. - self.assertNotIn(self.portal, - aq_chain(self.unwrapped_archetypes_comment)) - self.assertNotIn(self.portal, - aq_chain(self.unwrapped_dexterity_comment)) - - # Wrapped comments however have a complete chain and thus can find the - # portal object reliably. - self.assertIn(self.portal,aq_chain(self.wrapped_archetypes_comment)) - self.assertIn(self.portal,aq_chain(self.wrapped_dexterity_comment)) - - - def test_acquiring_comment_permissions(self): - """ Unwrapped comments should not be able to acquire permissions - controlled by unreachable objects """ - - # We use the "Allow sendto" permission as by default it is - # controlled by the portal, which is unreachable via __parent__ - # attributes on the comments. - permission = "Allow sendto" - - # Unwrapped comments can't find the portal so just return manager - self.assertNotIn("Anonymous", - rolesForPermissionOn(permission, - self.unwrapped_archetypes_comment)) - self.assertNotIn("Anonymous", - rolesForPermissionOn(permission, - self.unwrapped_dexterity_comment)) - - # Wrapped objects can find the portal and correctly return the - # anonymous role. - self.assertIn("Anonymous", - rolesForPermissionOn(permission, - self.wrapped_archetypes_comment)) - self.assertIn("Anonymous", - rolesForPermissionOn(permission, - self.wrapped_dexterity_comment)) - - def test_acquiring_comment_permissions_via_user_nobody(self): - """ The current implementation uses user_nobody.has_permission to - check whether anonymous can view comments. This confirms it also - works. """ - - # Again we want to use a permission that's not managed by any of our - # content objects so it must be acquired from the portal. - permission = "Allow sendto" - - self.assertFalse( - user_nobody.has_permission(permission, - self.unwrapped_archetypes_comment)) - - self.assertFalse( - user_nobody.has_permission(permission, - self.unwrapped_dexterity_comment)) - - self.assertTrue( - user_nobody.has_permission(permission, - self.wrapped_archetypes_comment)) - - self.assertTrue( - user_nobody.has_permission(permission, - self.wrapped_dexterity_comment)) - -class AcquiredPermissionTest(unittest.TestCase): - """ Test methods of a conversation which rely on acquired permissions """ - - layer = PLONE_APP_DISCUSSION_INTEGRATION_TESTING - - def setUp(self): - self.portal = self.layer['portal'] - self.request = self.layer['request'] - setRoles(self.portal, TEST_USER_ID, ['Manager']) - self.wftool = getToolByName(self.portal, 'portal_workflow') - - # Disable workflow for comments and content. - self.wftool.setChainForPortalTypes(["Discussion Item"],[]) - self.wftool.setChainForPortalTypes([dexterity_type_name],[]) - - # Create a dexterity item. - self.portal.invokeFactory( - id=dexterity_object_id, - title='Instance Of Dexterity Type', - type_name=dexterity_type_name, - ) - - self.content = self.portal.get(dexterity_object_id) - - # Absolutely make sure that we're replicating the case of an - # incomplete chain correctly. - aq_base(self.content).__parent__ = None - - self.conversation = IConversation(self.content) - - # Add a comment - comment = createObject('plone.Comment') - self.conversation.addComment(comment) - self.comment = comment - - def test_view_permission_is_only_available_on_portal(self): - """ Check that the test setup is correct """ - - content_roles = rolesForPermissionOn("View",aq_base(self.content)) - self.assertNotIn("Anonymous",content_roles) - - comment_roles = rolesForPermissionOn("View",aq_base(self.comment)) - self.assertNotIn("Anonymous",comment_roles) - - # This actually acquires view from the app root, but we don't really - # care, we just need to confirm that something above our content - # object will give us View. - portal_roles = rolesForPermissionOn("View",self.portal) - self.assertIn("Anonymous",portal_roles) - - # The following tests fail when the conversation uses unwrapped comment - # objects to determine whether an anonymous user has the view permission. - - def test_total_comments(self): - self.assertEqual(self.conversation.total_comments,1) - - def test_last_comment_date(self): - self.assertEqual(self.conversation.last_comment_date, - self.comment.creation_date) - - def test_public_commentators(self): - self.assertEqual(self.conversation.public_commentators, - (self.comment.author_username,)) - - - -def test_suite(): - return unittest.defaultTestLoader.loadTestsFromName(__name__)