From 3708429a375a063e172cc10d329d7fe29e2f78de Mon Sep 17 00:00:00 2001
From: Ross Patterson <me@rpatterson.net>
Date: Fri, 15 Apr 2011 04:29:46 +0000
Subject: [PATCH] Move some policy out of the conversation storage adapter into
 a view, specifically "enabled()".  Prevents having to replace/migrate
 persistent objects to change policy which really only concerns the context
 and possibly the request, not the conversation storage. Fixes #11372.

svn path=/plone.app.discussion/trunk/; revision=48849
---
 CHANGES.txt                                   |  7 ++
 plone/app/discussion/browser/comments.py      | 12 +--
 plone/app/discussion/browser/configure.zcml   |  9 ++
 plone/app/discussion/browser/conversation.py  | 95 +++++++++++++++++++
 plone/app/discussion/conversation.py          | 68 +------------
 plone/app/discussion/interfaces.py            | 25 -----
 .../discussion/tests/test_comments_viewlet.py | 10 ++
 .../app/discussion/tests/test_conversation.py | 24 +++--
 8 files changed, 146 insertions(+), 104 deletions(-)
 create mode 100644 plone/app/discussion/browser/conversation.py

diff --git a/CHANGES.txt b/CHANGES.txt
index 7329c5d..7fedca1 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -4,6 +4,13 @@ Changelog
 2.0b2 (Unreleased)
 ------------------
 
+- Move some policy out of the conversation storage adapter into a
+  view, specifically "enabled()".  Prevents having to replace/migrate
+  persistent objects to change policy which really only concerns the
+  context and possibly the request, not the conversation storage.
+  Fixes #11372.
+  [rossp]
+
 - Fix unindexing of comments when deleting content resulting from
   iterating over a BTree while modifying it. Fixes #11402.
   [rossp]
diff --git a/plone/app/discussion/browser/comments.py b/plone/app/discussion/browser/comments.py
index 6627987..a6b143f 100644
--- a/plone/app/discussion/browser/comments.py
+++ b/plone/app/discussion/browser/comments.py
@@ -168,14 +168,15 @@ class CommentForm(extensible.ExtensibleForm, form.Form):
         if 'user_notification' in data:
             user_notification = data['user_notification']
 
-        # The add-comment view is called on the conversation object
-        conversation = IConversation(self.__parent__)
-
         # Check if conversation is enabled on this content object
-        if not conversation.enabled():
+        if not self.__parent__.restrictedTraverse(
+            '@@conversation_view').enabled():
             raise Unauthorized, "Discussion is not enabled for this content\
                                  object."
 
+        # The add-comment view is called on the conversation object
+        conversation = IConversation(self.__parent__)
+
         if data['in_reply_to']:
             # Fetch the comment we want to reply to
             conversation_to_reply_to = conversation.get(data['in_reply_to'])
@@ -291,8 +292,7 @@ class CommentsViewlet(ViewletBase):
 
     def is_discussion_allowed(self):
         context = aq_inner(self.context)
-        conversation = IConversation(context)
-        return conversation.enabled()
+        return context.restrictedTraverse('@@conversation_view').enabled()
 
     def comment_transform_message(self):
         """Returns the description that shows up above the comment text,
diff --git a/plone/app/discussion/browser/configure.zcml b/plone/app/discussion/browser/configure.zcml
index 95dbf2e..a240167 100644
--- a/plone/app/discussion/browser/configure.zcml
+++ b/plone/app/discussion/browser/configure.zcml
@@ -100,6 +100,15 @@
         permission="zope2.View"
         />
 
+    <!-- Conversation view -->
+    <browser:page
+        name="conversation_view"
+        for="Products.CMFCore.interfaces.IContentish"
+        layer="..interfaces.IDiscussionLayer"
+        class=".conversation.ConversationView"
+        permission="zope2.View"
+        />
+
     <!-- Comment view -->
     <browser:view
         name="view"
diff --git a/plone/app/discussion/browser/conversation.py b/plone/app/discussion/browser/conversation.py
new file mode 100644
index 0000000..d3f114e
--- /dev/null
+++ b/plone/app/discussion/browser/conversation.py
@@ -0,0 +1,95 @@
+from zope.component import queryUtility
+
+from plone.registry.interfaces import IRegistry
+
+from Acquisition import aq_inner
+from Acquisition import aq_base
+
+from Products.CMFCore.utils import getToolByName
+from Products.CMFCore.interfaces import IFolderish
+
+from Products.CMFPlone.interfaces import IPloneSiteRoot
+from Products.CMFPlone.interfaces import INonStructuralFolder
+
+from plone.app.discussion.interfaces import IDiscussionSettings
+
+
+class ConversationView(object):
+
+    def enabled(self):
+        """ Returns True if discussion is enabled for this conversation.
+
+        This method checks five different settings in order to figure out if
+        discussion is enable on a specific content object:
+
+        1) Check if discussion is enabled globally in the plone.app.discussion
+           registry/control panel.
+
+        2) If the current content object is a folder, always return
+           False, since we don't allow comments on a folder. This
+           setting is used to allow/ disallow comments for all content
+           objects inside a folder, not for the folder itself.
+
+        3) Check if the allow_discussion boolean flag on the content object is
+           set. If it is set to True or False, return the value. If it set to
+           None, try further.
+
+        4) Traverse to a folder with allow_discussion set to either True or
+           False. If allow_discussion is not set (None), traverse further until
+           we reach the PloneSiteRoot.
+
+        5) Check if discussion is allowed for the content type.
+        """
+        context = aq_inner(self.context)
+
+        # Fetch discussion registry
+        registry = queryUtility(IRegistry)
+        settings = registry.forInterface(IDiscussionSettings, check=False)
+
+        # Check if discussion is allowed globally
+        if not settings.globally_enabled:
+            return False
+
+        # Always return False if object is a folder
+        if (IFolderish.providedBy(context) and
+            not INonStructuralFolder.providedBy(context)):
+            return False
+
+        def traverse_parents(context):
+            # Run through the aq_chain of obj and check if discussion is
+            # enabled in a parent folder.
+            for obj in context.aq_chain:
+                if not IPloneSiteRoot.providedBy(obj):
+                    if (IFolderish.providedBy(obj) and
+                        not INonStructuralFolder.providedBy(obj)):
+                        flag = getattr(obj, 'allow_discussion', None)
+                        if flag is not None:
+                            return flag
+            return None
+
+        # If discussion is disabled for the object, bail out
+        obj_flag = getattr(aq_base(context), 'allow_discussion', None)
+        if obj_flag is False:
+            return False
+
+        # Check if traversal returned a folder with discussion_allowed set
+        # to True or False.
+        folder_allow_discussion = traverse_parents(context)
+
+        if folder_allow_discussion is True:
+            if not getattr(self, 'allow_discussion', None):
+                return True
+        elif folder_allow_discussion is False:
+            if obj_flag:
+                return True
+
+        # Check if discussion is allowed on the content type
+        portal_types = getToolByName(self, 'portal_types')
+        document_fti = getattr(portal_types, context.portal_type)
+        if not document_fti.getProperty('allow_discussion'):
+            # If discussion is not allowed on the content type,
+            # check if 'allow discussion' is overridden on the content object.
+            if not obj_flag:
+                return False
+
+        return True
diff --git a/plone/app/discussion/conversation.py b/plone/app/discussion/conversation.py
index c155885..5ce96ed 100644
--- a/plone/app/discussion/conversation.py
+++ b/plone/app/discussion/conversation.py
@@ -14,10 +14,9 @@ import time
 
 from persistent import Persistent
 
-from plone.registry.interfaces import IRegistry
-
 from zope.interface import implements, implementer
-from zope.component import adapts, adapter, queryUtility
+from zope.component import adapts
+from zope.component import adapter
 
 from zope.annotation.interfaces import IAnnotations, IAnnotatable
 
@@ -31,11 +30,6 @@ from OFS.Traversable import Traversable
 from OFS.event import ObjectWillBeAddedEvent
 from OFS.event import ObjectWillBeRemovedEvent
 
-from Products.CMFCore.utils import getToolByName
-from Products.CMFCore.interfaces import IFolderish
-
-from Products.CMFPlone.interfaces import IPloneSiteRoot, INonStructuralFolder
-
 from zope.container.contained import ContainerModifiedEvent
 
 from zope.lifecycleevent import ObjectCreatedEvent
@@ -49,7 +43,6 @@ from BTrees.LOBTree import LOBTree
 from BTrees.LLBTree import LLSet
 
 from plone.app.discussion.interfaces import IConversation
-from plone.app.discussion.interfaces import IDiscussionSettings
 from plone.app.discussion.interfaces import IReplies
 from plone.app.discussion.comment import Comment
 
@@ -87,63 +80,8 @@ class Conversation(Traversable, Persistent, Explicit):
         return self.id
 
     def enabled(self):
-        # Returns True if discussion is enabled on the conversation
-
-        # Fetch discussion registry
-        registry = queryUtility(IRegistry)
-        settings = registry.forInterface(IDiscussionSettings, check=False)
-
-        # Check if discussion is allowed globally
-        if not settings.globally_enabled:
-            return False
-
         parent = aq_inner(self.__parent__)
-
-        # Always return False if object is a folder
-        if (IFolderish.providedBy(parent) and
-            not INonStructuralFolder.providedBy(parent)):
-            return False
-
-        def traverse_parents(obj):
-            # Run through the aq_chain of obj and check if discussion is
-            # enabled in a parent folder.
-            for obj in self.aq_chain:
-                if not IPloneSiteRoot.providedBy(obj):
-                    if (IFolderish.providedBy(obj) and
-                        not INonStructuralFolder.providedBy(obj)):
-                        flag = getattr(obj, 'allow_discussion', None)
-                        if flag is not None:
-                            return flag
-            return None
-
-        obj = aq_parent(self)
-
-        # If discussion is disabled for the object, bail out
-        obj_flag = getattr(aq_base(obj), 'allow_discussion', None)
-        if obj_flag is False:
-            return False
-
-        # Check if traversal returned a folder with discussion_allowed set
-        # to True or False.
-        folder_allow_discussion = traverse_parents(obj)
-
-        if folder_allow_discussion is True:
-            if not getattr(self, 'allow_discussion', None):
-                return True
-        elif folder_allow_discussion is False:
-            if obj_flag:
-                return True
-
-        # Check if discussion is allowed on the content type
-        portal_types = getToolByName(self, 'portal_types')
-        document_fti = getattr(portal_types, obj.portal_type)
-        if not document_fti.getProperty('allow_discussion'):
-            # If discussion is not allowed on the content type,
-            # check if 'allow discussion' is overridden on the content object.
-            if not obj_flag:
-                return False
-
-        return True
+        return parent.restrictedTraverse('@@conversation_view').enabled()
 
     @property
     def total_comments(self):
diff --git a/plone/app/discussion/interfaces.py b/plone/app/discussion/interfaces.py
index df0ec81..1ed57f2 100644
--- a/plone/app/discussion/interfaces.py
+++ b/plone/app/discussion/interfaces.py
@@ -168,31 +168,6 @@ class IConversation(IIterableMapping):
         readonly=True,
         )
 
-    def enabled():
-        """ Returns True if discussion is enabled for this conversation.
-
-        This method checks five different settings in order to figure out if
-        discussion is enable on a specific content object:
-
-        1) Check if discussion is enabled globally in the plone.app.discussion
-           registry/control panel.
-
-        2) If the current content object is a folder, always return False, since
-           we don't allow comments on a folder. This setting is used to allow/
-           disallow comments for all content objects inside a folder, not for
-           the folder itself.
-
-        3) Check if the allow_discussion boolean flag on the content object is
-           set. If it is set to True or False, return the value. If it set to
-           None, try further.
-
-        4) Traverse to a folder with allow_discussion set to either True or
-           False. If allow_discussion is not set (None), traverse further until
-           we reach the PloneSiteRoot.
-
-        5) Check if discussion is allowed for the content type.
-        """
-
     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/test_comments_viewlet.py b/plone/app/discussion/tests/test_comments_viewlet.py
index d5a0f21..ca8c4e6 100644
--- a/plone/app/discussion/tests/test_comments_viewlet.py
+++ b/plone/app/discussion/tests/test_comments_viewlet.py
@@ -7,6 +7,7 @@ from AccessControl import Unauthorized
 
 from OFS.Image import Image
 
+from zope import interface
 from zope.interface import alsoProvides
 from zope.publisher.browser import TestRequest
 from zope.annotation.interfaces import IAttributeAnnotatable
@@ -29,6 +30,7 @@ from Products.PloneTestCase.ptc import PloneTestCase
 
 from plone.app.discussion.browser.comments import CommentsViewlet
 from plone.app.discussion.browser.comments import CommentForm
+from plone.app.discussion import interfaces
 from plone.app.discussion.interfaces import IConversation
 from plone.app.discussion.tests.layer import DiscussionLayer
 from plone.app.discussion.interfaces import IDiscussionSettings
@@ -39,6 +41,9 @@ class TestCommentForm(PloneTestCase):
     layer = DiscussionLayer
 
     def afterSetUp(self):
+        interface.alsoProvides(
+            self.portal.REQUEST, interfaces.IDiscussionLayer)
+
         self.loginAsPortalOwner()
         typetool = self.portal.portal_types
         typetool.constructContent('Document', self.portal, 'doc1')
@@ -111,6 +116,8 @@ class TestCommentForm(PloneTestCase):
         settings = registry.forInterface(IDiscussionSettings, check=False)
         settings.anonymous_comments = True
 
+        self.portal.portal_workflow.doActionFor(self.context, 'publish')
+
         # Logout
         self.logout()
 
@@ -211,6 +218,9 @@ class TestCommentsViewlet(PloneTestCase):
     layer = DiscussionLayer
 
     def afterSetUp(self):
+        interface.alsoProvides(
+            self.portal.REQUEST, interfaces.IDiscussionLayer)
+
         self.loginAsPortalOwner()
         typetool = self.portal.portal_types
         typetool.constructContent('Document', self.portal, 'doc1')
diff --git a/plone/app/discussion/tests/test_conversation.py b/plone/app/discussion/tests/test_conversation.py
index 2701aea..6cc8b30 100644
--- a/plone/app/discussion/tests/test_conversation.py
+++ b/plone/app/discussion/tests/test_conversation.py
@@ -1,6 +1,7 @@
 import unittest
 from datetime import datetime, timedelta
 
+from zope import interface
 from zope.component import createObject, queryUtility
 from zope.annotation.interfaces import IAnnotations
 
@@ -14,6 +15,7 @@ from Products.CMFCore.utils import getToolByName
 from Products.PloneTestCase.ptc import PloneTestCase
 from plone.app.discussion.tests.layer import DiscussionLayer
 
+from plone.app.discussion import interfaces
 from plone.app.discussion.interfaces import IConversation
 from plone.app.discussion.interfaces import IComment
 from plone.app.discussion.interfaces import IReplies
@@ -25,6 +27,9 @@ class ConversationTest(PloneTestCase):
     layer = DiscussionLayer
 
     def afterSetUp(self):
+        interface.alsoProvides(
+            self.portal.REQUEST, interfaces.IDiscussionLayer)
+
         # First we need to create some content.
         self.loginAsPortalOwner()
         typetool = self.portal.portal_types
@@ -244,7 +249,7 @@ class ConversationTest(PloneTestCase):
         self.assertFalse(aq_base(folder).allow_discussion)
 
         doc = self.portal.folder1.doc2
-        conversation = IConversation(doc)
+        conversation = doc.restrictedTraverse('@@conversation_view')
         self.assertEquals(conversation.enabled(), False)
 
         # We have to allow discussion on Document content type, since
@@ -258,7 +263,8 @@ class ConversationTest(PloneTestCase):
     def test_disable_commenting_globally(self):
 
         # Create a conversation.
-        conversation = IConversation(self.portal.doc1)
+        conversation = self.portal.doc1.restrictedTraverse(
+            '@@conversation_view')
 
         # We have to allow discussion on Document content type, since
         # otherwise allow_discussion will always return False
@@ -286,7 +292,7 @@ class ConversationTest(PloneTestCase):
 
         self.typetool.constructContent('News Item', self.portal, 'newsitem')
         newsitem = self.portal.newsitem
-        conversation = IConversation(newsitem)
+        conversation = newsitem.restrictedTraverse('@@conversation_view')
 
         # We have to allow discussion on Document content type, since
         # otherwise allow_discussion will always return False
@@ -312,7 +318,8 @@ class ConversationTest(PloneTestCase):
     def test_disable_commenting_for_content_type(self):
 
         # Create a conversation.
-        conversation = IConversation(self.portal.doc1)
+        conversation = self.portal.doc1.restrictedTraverse(
+            '@@conversation_view')
 
         # The Document content type is disabled by default
         self.assertEquals(conversation.enabled(), False)
@@ -337,11 +344,11 @@ class ConversationTest(PloneTestCase):
         # The enabled method should always return False for the folder
         # itself.
 
-        # Create a folder
+        # Create a folderp
         self.typetool.constructContent('Folder', self.portal, 'f1')
         f1 = self.portal.f1
         # Usually we don't create a conversation on a folder
-        conversation = IConversation(self.portal.f1)
+        conversation = self.portal.f1.restrictedTraverse('@@conversation_view')
 
         # Allow discussion for the folder
         self.portal_discussion.overrideDiscussionFor(f1, True)
@@ -369,7 +376,7 @@ class ConversationTest(PloneTestCase):
         # Create a document inside the folder
         self.typetool.constructContent('Document', f1, 'doc1')
         doc1 = self.portal.f1.doc1
-        doc1_conversation = IConversation(doc1)
+        doc1_conversation = doc1.restrictedTraverse('@@conversation_view')
 
         self.assertEquals(doc1_conversation.enabled(), False)
 
@@ -389,7 +396,8 @@ class ConversationTest(PloneTestCase):
         # Allow discussion on a single content object
 
         # Create a conversation.
-        conversation = IConversation(self.portal.doc1)
+        conversation = self.portal.doc1.restrictedTraverse(
+            '@@conversation_view')
 
         # Discussion is disallowed by default
         self.assertEquals(conversation.enabled(), False)