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/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/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 b1854d0..9fa258c 100644 --- a/plone/app/discussion/upgrades.zcml +++ b/plone/app/discussion/upgrades.zcml @@ -11,4 +11,44 @@ handler=".upgrades.update_registry" /> + + + + + + + + + +