From 97477163bef06068de694f8a4b286fff736f2555 Mon Sep 17 00:00:00 2001 From: Maurits van Rees Date: Tue, 10 Jan 2017 18:02:03 +0100 Subject: [PATCH 1/2] Moved all upgrade steps to upgrades.zcml. Until now some were in configure.zcml and some in upgrades.zcml. --- plone/app/discussion/configure.zcml | 21 +-------------------- plone/app/discussion/upgrades.zcml | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/plone/app/discussion/configure.zcml b/plone/app/discussion/configure.zcml index 4e634ba..86051d0 100644 --- a/plone/app/discussion/configure.zcml +++ b/plone/app/discussion/configure.zcml @@ -40,26 +40,7 @@ provides="Products.GenericSetup.interfaces.EXTENSION" for="Products.CMFPlone.interfaces.IPloneSiteRoot" /> - - - - + diff --git a/plone/app/discussion/upgrades.zcml b/plone/app/discussion/upgrades.zcml index b1854d0..7c1753a 100644 --- a/plone/app/discussion/upgrades.zcml +++ b/plone/app/discussion/upgrades.zcml @@ -11,4 +11,24 @@ handler=".upgrades.update_registry" /> + + + + From 802e3ec04c5cb87b3cdb0500d27661c56b29ce67 Mon Sep 17 00:00:00 2001 From: Maurits van Rees Date: Tue, 10 Jan 2017 18:09:01 +0100 Subject: [PATCH 2/2] Make comment on private content not publicly available in search results. This is part of PloneHotfix20161129. Updated metadata.xml version to 1000. This leaves more room for profile version increases in earlier releases. We apply the rolemap step again to avoid accidentally missing it. --- CHANGES.rst | 3 +- plone/app/discussion/browser/controlpanel.py | 8 +- .../discussion/profiles/default/metadata.xml | 4 +- .../discussion/profiles/default/workflows.xml | 3 +- .../comment_one_state_workflow/definition.xml | 80 +++++++++++++++++++ .../comment_review_workflow/definition.xml | 6 +- plone/app/discussion/tests/test_catalog.py | 4 + plone/app/discussion/tests/test_comment.py | 6 ++ .../discussion/tests/test_comments_viewlet.py | 2 +- .../app/discussion/tests/test_controlpanel.py | 8 +- .../app/discussion/tests/test_conversation.py | 6 ++ plone/app/discussion/tests/test_indexers.py | 3 + .../discussion/tests/test_moderation_view.py | 4 +- plone/app/discussion/tests/test_workflow.py | 73 +++++++++++++++-- plone/app/discussion/upgrades.py | 47 +++++++++++ plone/app/discussion/upgrades.zcml | 20 +++++ 16 files changed, 251 insertions(+), 26 deletions(-) create mode 100644 plone/app/discussion/profiles/default/workflows/comment_one_state_workflow/definition.xml diff --git a/CHANGES.rst b/CHANGES.rst index de6bf47..63d0829 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -14,7 +14,8 @@ New features: Bug fixes: -- *add item here* +- Make comment on private content not publicly available in search results. + Part of PloneHotfix20161129. [vangheem, maurits] 2.4.19 (2017-01-02) diff --git a/plone/app/discussion/browser/controlpanel.py b/plone/app/discussion/browser/controlpanel.py index 3cc8747..4bb3ba0 100644 --- a/plone/app/discussion/browser/controlpanel.py +++ b/plone/app/discussion/browser/controlpanel.py @@ -125,7 +125,7 @@ class DiscussionSettingsControlPanel(controlpanel.ControlPanelFormWrapper): output.append('globally_enabled') # Comment moderation - one_state_worklow_disabled = 'one_state_workflow' not in workflow_chain + one_state_worklow_disabled = 'comment_one_state_workflow' not in workflow_chain comment_review_workflow_disabled = \ 'comment_review_workflow' not in workflow_chain if one_state_worklow_disabled and comment_review_workflow_disabled: @@ -174,7 +174,7 @@ class DiscussionSettingsControlPanel(controlpanel.ControlPanelFormWrapper): """ wftool = getToolByName(self.context, 'portal_workflow', None) workflow_chain = wftool.getChainForPortalType('Discussion Item') - one_state_workflow_enabled = 'one_state_workflow' in workflow_chain + one_state_workflow_enabled = 'comment_one_state_workflow' in workflow_chain comment_review_workflow_enabled = \ 'comment_review_workflow' in workflow_chain if one_state_workflow_enabled or comment_review_workflow_enabled: @@ -199,7 +199,7 @@ def notify_configuration_changed(event): else: # Disable moderation workflow wftool.setChainForPortalTypes(('Discussion Item',), - 'one_state_workflow') + 'comment_one_state_workflow') if IConfigurationChangedEvent.providedBy(event): # Types control panel setting changed @@ -209,7 +209,7 @@ def notify_configuration_changed(event): workflow_chain = wftool.getChainForPortalType('Discussion Item') if workflow_chain: workflow = workflow_chain[0] - if workflow == 'one_state_workflow': + if workflow == 'comment_one_state_workflow': settings.moderation_enabled = False elif workflow == 'comment_review_workflow': settings.moderation_enabled = True diff --git a/plone/app/discussion/profiles/default/metadata.xml b/plone/app/discussion/profiles/default/metadata.xml index ce1f445..f14a707 100644 --- a/plone/app/discussion/profiles/default/metadata.xml +++ b/plone/app/discussion/profiles/default/metadata.xml @@ -1,6 +1,6 @@ - 102 + 1000 profile-plone.app.registry:default - \ No newline at end of file + diff --git a/plone/app/discussion/profiles/default/workflows.xml b/plone/app/discussion/profiles/default/workflows.xml index 9738f7c..328a7ac 100644 --- a/plone/app/discussion/profiles/default/workflows.xml +++ b/plone/app/discussion/profiles/default/workflows.xml @@ -1,9 +1,10 @@ + - + diff --git a/plone/app/discussion/profiles/default/workflows/comment_one_state_workflow/definition.xml b/plone/app/discussion/profiles/default/workflows/comment_one_state_workflow/definition.xml new file mode 100644 index 0000000..fd541fa --- /dev/null +++ b/plone/app/discussion/profiles/default/workflows/comment_one_state_workflow/definition.xml @@ -0,0 +1,80 @@ + + + Access contents information + Change portal events + Modify portal content + View + + Visible to everyone, editable by the owner. + + + Anonymous + + + Manager + Owner + Editor + Site Administrator + + + Manager + Owner + Editor + 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/profiles/default/workflows/comment_review_workflow/definition.xml b/plone/app/discussion/profiles/default/workflows/comment_review_workflow/definition.xml index 917021e..6936319 100644 --- a/plone/app/discussion/profiles/default/workflows/comment_review_workflow/definition.xml +++ b/plone/app/discussion/profiles/default/workflows/comment_review_workflow/definition.xml @@ -41,14 +41,12 @@ Visible to everyone, non-editable. - - Anonymous + Manager - - Anonymous + diff --git a/plone/app/discussion/tests/test_catalog.py b/plone/app/discussion/tests/test_catalog.py index 91b4c62..7cd9be4 100644 --- a/plone/app/discussion/tests/test_catalog.py +++ b/plone/app/discussion/tests/test_catalog.py @@ -59,6 +59,10 @@ class ConversationCatalogTest(unittest.TestCase): def setUp(self): self.portal = self.layer['portal'] setRoles(self.portal, TEST_USER_ID, ['Manager']) + + workflow = self.portal.portal_workflow + workflow.doActionFor(self.portal.doc1, 'publish') + self.catalog = getToolByName(self.portal, 'portal_catalog') conversation = IConversation(self.portal.doc1) comment1 = createObject('plone.Comment') diff --git a/plone/app/discussion/tests/test_comment.py b/plone/app/discussion/tests/test_comment.py index daded8b..98cf0b0 100644 --- a/plone/app/discussion/tests/test_comment.py +++ b/plone/app/discussion/tests/test_comment.py @@ -27,6 +27,9 @@ class CommentTest(unittest.TestCase): self.portal = self.layer['portal'] self.request = self.layer['request'] + workflow = self.portal.portal_workflow + workflow.doActionFor(self.portal.doc1, 'publish') + setRoles(self.portal, TEST_USER_ID, ['Manager']) self.catalog = getToolByName(self.portal, 'portal_catalog') self.document_brain = self.catalog.searchResults( @@ -351,6 +354,9 @@ class RepliesTest(unittest.TestCase): self.portal = self.layer['portal'] setRoles(self.portal, TEST_USER_ID, ['Manager']) + workflow = self.portal.portal_workflow + workflow.doActionFor(self.portal.doc1, 'publish') + def test_add_comment(self): # Add comments to a CommentReplies adapter diff --git a/plone/app/discussion/tests/test_comments_viewlet.py b/plone/app/discussion/tests/test_comments_viewlet.py index 8d91e38..cf1b29f 100644 --- a/plone/app/discussion/tests/test_comments_viewlet.py +++ b/plone/app/discussion/tests/test_comments_viewlet.py @@ -458,7 +458,7 @@ class TestCommentsViewlet(unittest.TestCase): ) self.workflowTool = getToolByName(self.portal, 'portal_workflow') - self.workflowTool.setDefaultChain('one_state_workflow') + self.workflowTool.setDefaultChain('comment_one_state_workflow') self.membershipTool = getToolByName(self.folder, 'portal_membership') self.memberdata = self.portal.portal_memberdata diff --git a/plone/app/discussion/tests/test_controlpanel.py b/plone/app/discussion/tests/test_controlpanel.py index 64ae7d4..4c09594 100644 --- a/plone/app/discussion/tests/test_controlpanel.py +++ b/plone/app/discussion/tests/test_controlpanel.py @@ -163,9 +163,9 @@ class ConfigurationChangedSubscriberTest(unittest.TestCase): the 'comment_moderation' setting in the discussion control panel changes. """ - # By default the one_state_workflow without moderation is enabled + # By default the comment_one_state_workflow without moderation is enabled self.assertEqual( - ('one_state_workflow',), + ('comment_one_state_workflow',), self.portal.portal_workflow.getChainForPortalType( 'Discussion Item' ) @@ -185,7 +185,7 @@ class ConfigurationChangedSubscriberTest(unittest.TestCase): # And back self.settings.moderation_enabled = False self.assertEqual( - ('one_state_workflow',), + ('comment_one_state_workflow',), self.portal.portal_workflow.getChainForPortalType( 'Discussion Item' ) @@ -211,7 +211,7 @@ class ConfigurationChangedSubscriberTest(unittest.TestCase): # Enable the 'comment_review_workflow' with moderation enabled self.portal.portal_workflow.setChainForPortalTypes( ('Discussion Item',), - ('one_state_workflow',) + ('comment_one_state_workflow',) ) self.settings.moderation_enabled = True diff --git a/plone/app/discussion/tests/test_conversation.py b/plone/app/discussion/tests/test_conversation.py index 2720927..7880d58 100644 --- a/plone/app/discussion/tests/test_conversation.py +++ b/plone/app/discussion/tests/test_conversation.py @@ -50,6 +50,9 @@ class ConversationTest(unittest.TestCase): settings = registry.forInterface(IDiscussionSettings) settings.globally_enabled = True + workflow = self.portal.portal_workflow + workflow.doActionFor(self.portal.doc1, 'publish') + def test_add_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. @@ -744,6 +747,9 @@ class RepliesTest(unittest.TestCase): self.portal = self.layer['portal'] setRoles(self.portal, TEST_USER_ID, ['Manager']) + workflow = self.portal.portal_workflow + workflow.doActionFor(self.portal.doc1, 'publish') + def test_add_comment(self): # Add comments to a ConversationReplies adapter diff --git a/plone/app/discussion/tests/test_indexers.py b/plone/app/discussion/tests/test_indexers.py index a7d0d96..ee5c02b 100644 --- a/plone/app/discussion/tests/test_indexers.py +++ b/plone/app/discussion/tests/test_indexers.py @@ -35,6 +35,9 @@ class ConversationIndexersTest(unittest.TestCase): self.portal = self.layer['portal'] setRoles(self.portal, TEST_USER_ID, ['Manager']) + workflow = self.portal.portal_workflow + workflow.doActionFor(self.portal.doc1, 'publish') + # Create a conversation. conversation = IConversation(self.portal.doc1) diff --git a/plone/app/discussion/tests/test_moderation_view.py b/plone/app/discussion/tests/test_moderation_view.py index 849a0d0..4f44d57 100644 --- a/plone/app/discussion/tests/test_moderation_view.py +++ b/plone/app/discussion/tests/test_moderation_view.py @@ -46,9 +46,9 @@ class ModerationViewTest(unittest.TestCase): # If workflow is not set, enabled must return False self.wf_tool.setChainForPortalTypes(('Discussion Item',), ()) self.assertEqual(self.view.moderation_enabled(), False) - # The one_state_workflow does not have a 'pending' state + # The comment_one_state_workflow does not have a 'pending' state self.wf_tool.setChainForPortalTypes(('Discussion Item',), - ('one_state_workflow,')) + ('comment_one_state_workflow,')) self.assertEqual(self.view.moderation_enabled(), False) # The comment_review_workflow does have a 'pending' state self.wf_tool.setChainForPortalTypes(('Discussion Item',), diff --git a/plone/app/discussion/tests/test_workflow.py b/plone/app/discussion/tests/test_workflow.py index 2597593..3f5f19f 100644 --- a/plone/app/discussion/tests/test_workflow.py +++ b/plone/app/discussion/tests/test_workflow.py @@ -9,6 +9,7 @@ from plone.app.testing import login from plone.app.testing import logout from plone.app.testing import setRoles from plone.app.testing import TEST_USER_ID +from plone.app.testing import TEST_USER_NAME from Products.CMFCore.permissions import View from Products.CMFCore.utils import _checkPermission as checkPerm from zope.component import createObject @@ -35,7 +36,7 @@ class WorkflowSetupTest(unittest.TestCase): def test_workflows_installed(self): """Make sure both comment workflows have been installed properly. """ - self.assertTrue('one_state_workflow' in + self.assertTrue('comment_one_state_workflow' in self.portal.portal_workflow.objectIds()) self.assertTrue('comment_review_workflow' in self.portal.portal_workflow.objectIds()) @@ -44,7 +45,7 @@ class WorkflowSetupTest(unittest.TestCase): """Make sure one_state_workflow is the default workflow. """ self.assertEqual( - ('one_state_workflow',), + ('comment_one_state_workflow',), self.portal.portal_workflow.getChainForPortalType( 'Discussion Item' ) @@ -102,7 +103,7 @@ class PermissionsSetupTest(unittest.TestCase): class CommentOneStateWorkflowTest(unittest.TestCase): - """Test the one_state_workflow that ships with plone.app.discussion. + """Test the comment_one_state_workflow that ships with plone.app.discussion. """ layer = PLONE_APP_DISCUSSION_INTEGRATION_TESTING @@ -114,8 +115,6 @@ class CommentOneStateWorkflowTest(unittest.TestCase): self.folder = self.portal['test-folder'] self.catalog = self.portal.portal_catalog self.workflow = self.portal.portal_workflow - self.workflow.setChainForPortalTypes(['Document'], - 'one_state_workflow') self.folder.invokeFactory('Document', 'doc1') self.doc = self.folder.doc1 @@ -137,10 +136,10 @@ class CommentOneStateWorkflowTest(unittest.TestCase): self.portal.acl_users._doAddUser('reader', 'secret', ['Reader'], []) def test_initial_workflow_state(self): - """Make sure the initial workflow state of a comment is 'published'. + """Make sure the initial workflow state of a comment is 'private'. """ self.assertEqual(self.workflow.getInfoFor(self.doc, 'review_state'), - 'published') + 'private') def test_view_comments(self): """Make sure published comments can be viewed by everyone. @@ -149,6 +148,10 @@ class CommentOneStateWorkflowTest(unittest.TestCase): # self.login(default_user) # self.assertTrue(checkPerm(View, self.doc)) # Member is allowed + login(self.portal, TEST_USER_NAME) + workflow = self.portal.portal_workflow + workflow.doActionFor(self.doc, 'publish') + login(self.portal, 'member') self.assertTrue(checkPerm(View, self.comment)) # Reviewer is allowed @@ -164,6 +167,30 @@ class CommentOneStateWorkflowTest(unittest.TestCase): login(self.portal, 'reader') self.assertTrue(checkPerm(View, self.comment)) + def test_comment_on_private_content_not_visible_to_world(self): + logout() + self.assertFalse(checkPerm(View, self.comment)) + + def test_migration(self): + from plone.app.discussion.upgrades import upgrade_comment_workflows + # Fake permission according to earlier one_comment_workflow. + self.comment._View_Permission = ('Anonymous',) + # Anonymous can see the comment. + logout() + self.assertTrue(checkPerm(View, self.comment)) + # Run the upgrade. + login(self.portal, TEST_USER_NAME) + upgrade_comment_workflows(self.portal.portal_setup) + # The workflow chain is still what we want. + self.assertEqual( + self.portal.portal_workflow.getChainFor('Discussion Item'), + ('comment_one_state_workflow',)) + # A Manager can still see the comment. + self.assertTrue(checkPerm(View, self.comment)) + # Anonymous cannot see the comment. + logout() + self.assertFalse(checkPerm(View, self.comment)) + class CommentReviewWorkflowTest(unittest.TestCase): """Test the comment_review_workflow that ships with plone.app.discussion. @@ -269,3 +296,35 @@ class CommentReviewWorkflowTest(unittest.TestCase): 'review_state' ) ) + + def test_publish_comment_on_private_content_not_visible_to_world(self): + logout() + self.assertFalse(checkPerm(View, self.comment)) + + # publish comment and check again + login(self.portal, TEST_USER_NAME) + workflow = self.portal.portal_workflow + workflow.doActionFor(self.comment, 'publish') + + logout() + self.assertFalse(checkPerm(View, self.comment)) + + def test_migration(self): + from plone.app.discussion.upgrades import upgrade_comment_workflows + # Fake permission according to earlier comment_review_workflow. + self.comment._View_Permission = ('Anonymous',) + # Anonymous can see the comment. + logout() + self.assertTrue(checkPerm(View, self.comment)) + # Run the upgrade. + login(self.portal, TEST_USER_NAME) + upgrade_comment_workflows(self.portal.portal_setup) + # The workflow chain is still what we want. + self.assertEqual( + self.portal.portal_workflow.getChainFor('Discussion Item'), + ('comment_review_workflow',)) + # A Manager can still see the comment. + self.assertTrue(checkPerm(View, self.comment)) + # Anonymous cannot see the comment. + logout() + self.assertFalse(checkPerm(View, self.comment)) diff --git a/plone/app/discussion/upgrades.py b/plone/app/discussion/upgrades.py index 1019dc0..8d4083e 100644 --- a/plone/app/discussion/upgrades.py +++ b/plone/app/discussion/upgrades.py @@ -1,10 +1,14 @@ # -*- coding: utf-8 -*- from plone.app.discussion.interfaces import IDiscussionSettings from plone.registry.interfaces import IRegistry +from Products.CMFCore.utils import getToolByName from zope.component import getUtility +import logging + default_profile = 'profile-plone.app.discussion:default' +logger = logging.getLogger('plone.app.discussion') def update_registry(context): @@ -14,3 +18,46 @@ def update_registry(context): def update_rolemap(context): context.runImportStepFromProfile(default_profile, 'rolemap') + + +def upgrade_comment_workflows(context): + # If the current comment workflow is the one_state_workflow, running our + # import step will change it to comment_one_state_workflow. This is good. + # If it was anything else, we should restore this. So get the original + # chain. + portal_type = 'Discussion Item' + wf_tool = getToolByName(context, 'portal_workflow') + orig_chain = list(wf_tool.getChainFor(portal_type)) + + # Run the workflow step. This sets the chain to + # comment_one_state_workflow. + context.runImportStepFromProfile(default_profile, 'workflow') + + # Restore original workflow chain if needed. + old_workflow = 'one_state_workflow' + if old_workflow not in orig_chain: + # Restore the chain. Probably comment_review_workflow. + wf_tool.setChainForPortalTypes([portal_type], orig_chain) + elif len(orig_chain) > 1: + # This is strange, but I guess it could happen. + if old_workflow in orig_chain: + # Replace with new one. + idx = orig_chain.index(old_workflow) + orig_chain[idx] = 'comment_one_state_workflow' + # Restore the chain. + wf_tool.setChainForPortalTypes([portal_type], orig_chain) + new_chain = list(wf_tool.getChainFor(portal_type)) + workflows = [wf_tool.getWorkflowById(wf_id) + for wf_id in new_chain] + + # Now go over the comments, update their role mappings, and reindex the + # allowedRolesAndUsers index. + catalog = getToolByName(context, 'portal_catalog') + for brain in catalog.unrestrictedSearchResults(portal_type=portal_type): + try: + comment = brain.getObject() + for wf in workflows: + wf.updateRoleMappingsFor(comment) + comment.reindexObjectSecurity() + except (AttributeError, KeyError): + logger.info('Could not reindex comment %s' % brain.getURL()) diff --git a/plone/app/discussion/upgrades.zcml b/plone/app/discussion/upgrades.zcml index 7c1753a..9fa258c 100644 --- a/plone/app/discussion/upgrades.zcml +++ b/plone/app/discussion/upgrades.zcml @@ -31,4 +31,24 @@ profile="plone.app.discussion:default" /> + + + + + +