From 586cd1fdb7370a20d794ec08afd7a31bc10824b4 Mon Sep 17 00:00:00 2001
From: Ross Patterson <me@rpatterson.net>
Date: Thu, 14 Apr 2011 16:17:29 +0000
Subject: [PATCH] Don't add the annotation unless a comment is actually being
 added. Fixes #11370.

svn path=/plone.app.discussion/trunk/; revision=48829
---
 CHANGES.txt                                   |  4 +++
 plone/app/discussion/conversation.py          | 10 ++++--
 plone/app/discussion/tests/test_catalog.py    | 35 +++++++++++++++++++
 .../app/discussion/tests/test_conversation.py |  8 +++++
 plone/app/discussion/tests/test_migration.py  | 10 ++++++
 5 files changed, 64 insertions(+), 3 deletions(-)

diff --git a/CHANGES.txt b/CHANGES.txt
index 0379751..165b205 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -4,6 +4,10 @@ Changelog
 2.0b2 (Unreleased)
 ------------------
 
+- Don't add the annotation unless a comment is actually being added.
+  Fixes #11370.
+  [rossp]
+
 - Fixed i18n of the "Commenting has been disabled." message.
   [vincentfretin]
 
diff --git a/plone/app/discussion/conversation.py b/plone/app/discussion/conversation.py
index 14f2581..e785536 100644
--- a/plone/app/discussion/conversation.py
+++ b/plone/app/discussion/conversation.py
@@ -241,6 +241,11 @@ class Conversation(Traversable, Persistent, Explicit):
             self._children[reply_to] = LLSet()
         self._children[reply_to].insert(id)
 
+        # Add the annotation if not already done
+        annotions = IAnnotations(self.__parent__)
+        if not ANNOTATION_KEY in annotions:
+            annotions[ANNOTATION_KEY] = self
+
         # Notify that the object is added. The object must here be
         # acquisition wrapped or the indexing will fail.
         notify(ObjectCreatedEvent(comment))
@@ -334,14 +339,13 @@ class Conversation(Traversable, Persistent, Explicit):
 @implementer(IConversation)
 @adapter(IAnnotatable)
 def conversationAdapterFactory(content):
-    """Adapter factory to fetch the default conversation from annotations.
-    Will create the conversation if it does not exist.
+    """
+    Adapter factory to fetch the default conversation from annotations.
     """
     annotions = IAnnotations(content)
     if not ANNOTATION_KEY in annotions:
         conversation = Conversation()
         conversation.__parent__ = aq_base(content)
-        annotions[ANNOTATION_KEY] = conversation
     else:
         conversation = annotions[ANNOTATION_KEY]
     return conversation.__of__(content)
diff --git a/plone/app/discussion/tests/test_catalog.py b/plone/app/discussion/tests/test_catalog.py
index fd48eae..37fe364 100644
--- a/plone/app/discussion/tests/test_catalog.py
+++ b/plone/app/discussion/tests/test_catalog.py
@@ -5,6 +5,7 @@ import unittest
 from datetime import datetime
 
 from zope.component import createObject
+from zope.annotation.interfaces import IAnnotations
 
 from Products.CMFCore.utils import getToolByName
 
@@ -364,5 +365,39 @@ class CommentCatalogTest(PloneTestCase):
         self.assertEquals(query['Type'], 'Comment')
         self.assertEquals(len(topic.queryCatalog()), 1)
 
+
+class NoConversationCatalogTest(PloneTestCase):
+
+    layer = DiscussionLayer
+
+    def afterSetUp(self):
+        # First we need to create some content.
+        self.loginAsPortalOwner()
+        self.portal.invokeFactory(id='doc1',
+                                  Title='Document 1',
+                                  type_name='Document')
+
+        self.catalog = getToolByName(self.portal, 'portal_catalog')
+
+        conversation = IConversation(self.portal.doc1)
+
+        brains = self.catalog.searchResults(
+                     path = {'query' :
+                             '/'.join(self.portal.doc1.getPhysicalPath()) },
+                     portal_type = "Document"
+                     )
+        self.conversation = conversation
+        self.brains = brains
+        self.doc1_brain = brains[0]
+
+    def test_total_comments(self):
+        self.failUnless(self.doc1_brain.has_key('total_comments'))
+        self.assertEquals(self.doc1_brain.total_comments, 0)
+
+        # Make sure no conversation has been created
+        self.assert_('plone.app.discussion:conversation' not in
+                     IAnnotations(self.portal.doc1))
+
+
 def test_suite():
     return unittest.defaultTestLoader.loadTestsFromName(__name__)
diff --git a/plone/app/discussion/tests/test_conversation.py b/plone/app/discussion/tests/test_conversation.py
index 1c7f7c2..2701aea 100644
--- a/plone/app/discussion/tests/test_conversation.py
+++ b/plone/app/discussion/tests/test_conversation.py
@@ -2,6 +2,7 @@ import unittest
 from datetime import datetime, timedelta
 
 from zope.component import createObject, queryUtility
+from zope.annotation.interfaces import IAnnotations
 
 from Acquisition import aq_base, aq_parent
 
@@ -698,6 +699,13 @@ class ConversationTest(PloneTestCase):
     def test_discussion_item_not_in_bad_types(self):
         self.failIf('Discussion Item' in BAD_TYPES)
 
+    def test_no_comment(self):
+        conversation = IConversation(self.portal.doc1)
+
+        # Make sure no conversation has been created
+        self.assert_('plone.app.discussion:conversation' not in
+                     IAnnotations(self.portal.doc1))
+
 
 class RepliesTest(PloneTestCase):
 
diff --git a/plone/app/discussion/tests/test_migration.py b/plone/app/discussion/tests/test_migration.py
index b1b16ac..564091e 100644
--- a/plone/app/discussion/tests/test_migration.py
+++ b/plone/app/discussion/tests/test_migration.py
@@ -207,5 +207,15 @@ class MigrationTest(PloneTestCase):
         talkback = self.discussion.getDiscussionFor(self.doc)
         self.assertEquals(len(talkback.getReplies()), 0)
 
+    def test_migrate_no_comment(self):
+
+        # Call migration script
+        self.view()
+
+        # Make sure no conversation has been created
+        self.assert_('plone.app.discussion:conversation' not in
+                     IAnnotations(self.doc))
+
+
 def test_suite():
     return unittest.defaultTestLoader.loadTestsFromName(__name__)