fix conversation.__delitem__ function and add test for deleting comments from a conversation.

svn path=/plone.app.discussion/trunk/; revision=27062

plone/app/discussion/conversation.py

@@ -51,167 +51,168 @@ ANNOTATION_KEY = 'plone.app.discussion:conversation'
 
 class Conversation(Traversable, Persistent, Explicit):
     """A conversation is a container for all comments on a content object.
-    
+
     It manages internal data structures for comment threading and efficient
     comment lookup.
     """
-    
+
     implements(IConversation)
-    
+
     __allow_access_to_unprotected_subobjects__ = True
-    
+
     def __init__(self, id="++conversation++default"):
         self.id = id
-        
+
         # username -> count of comments; key is removed when count reaches 0
         self._commentators = OIBTree()
-        
+
         # id -> comment - find comment by id
-        self._comments = LOBTree()  
+        self._comments = LOBTree()
-        
+
         # id -> IISet (children) - find all children for a given comment. 0 signifies root.
         self._children = LOBTree()
-        
+
     def getId(self):
         """Get the id of the conversation. This is used to construct a
         URL.
         """
         return self.id
-    
+
     @property
     def enabled(self):
         # TODO - check __parent__'s settings + global settings
         return True
-    
+
     @property
     def total_comments(self):
         return len(self._comments)
-    
+
     @property
     def last_comment_date(self):
         try:
             return self._comments[self._comments.maxKey()].creation_date
         except (ValueError, KeyError, AttributeError,):
             return None
-    
+
     @property
     def commentators(self):
         return self._commentators.keys()
-    
+
     def getComments(self, start=0, size=None):
         """Get unthreaded comments
         """
         # TODO - batching
         return self._comments.values()
-    
+
     def getThreads(self, start=0, size=None, root=None, depth=None):
         """Get threaded comments
         """
         # TODO - build threads
         return self._comments.values()
-    
+
     def addComment(self, comment):
         """Add a new comment. The parent id should have been set already. The
         comment id may be modified to find a free key. The id used will be
         returned.
         """
-        
+
         # Make sure we don't have a wrapped object
-        
+
         comment = aq_base(comment)
-        
+
         id = long(time.time() * 1e6)
         while id in self._comments:
             id += 1
-        
+
         comment.comment_id = id
         notify(ObjectWillBeAddedEvent(comment, self, id))
         self._comments[id] = comment
-        
+
         comment.__parent__ = self
-        
+
         # Record unique users who've commented (for logged in users only)
         commentator = comment.author_username
         if commentator:
             if not commentator in self._commentators:
                 self._commentators[commentator] = 0
             self._commentators[commentator] += 1
-        
+
         reply_to = comment.in_reply_to
         if not reply_to:
             # top level comments are in reply to the faux id 0
             comment.in_reply_to = reply_to = 0
-        
+
         if not reply_to in self._children:
             self._children[reply_to] = LLSet()
         self._children[reply_to].insert(id)
-        
+
         # Notify that the object is added. The object must here be
         # acquisition wrapped or the indexing will fail.
         notify(ObjectAddedEvent(comment.__of__(self), self, id))
         notify(ContainerModifiedEvent(self))
-        
+
         return id
-        
+
     # Dict API
-    
+
     def __len__(self):
         return len(self._comments)
-    
+
     def __contains__(self, key):
         return long(key) in self._comments
-    
+
     def __getitem__(self, key):
         """Get an item by its long key
         """
         return self._comments[long(key)].__of__(self)
-    
+
     def __delitem__(self, key):
         """Delete an item by its long key
         """
-        
+
         key = long(key)
-        
+
         comment = self[key].__of__(self)
         commentator = comment.author_username
-        
+
         notify(ObjectWillBeRemovedEvent(comment, self, key))
-        self._comments.remove(key)
+
+        self._comments.pop(key)
         notify(ObjectRemovedEvent(comment, self, key))
-        
+
         if commentator and commentator in self._commentators:
             if self._commentators[commentator] <= 1:
                 del self._commentators[commentator]
             else:
                 self._commentators[commentator] -= 1
-        
+
         notify(ContainerModifiedEvent(self))
-    
+
     def __iter__(self):
         return iter(self._comments)
-    
+
     def get(self, key, default=None):
         comment = self._comments.get(long(key), default)
         if comment is default:
             return default
         return comment.__of__(self)
-    
+
     def keys(self):
         return self._comments.keys()
-    
+
     def items(self):
         return [(i[0], i[1].__of__(self),) for i in self._comments.items()]
-    
+
     def values(self):
         return [v.__of__(self) for v in self._comments.values()]
-    
+
     def iterkeys(self):
         return self._comments.iterkeys()
-    
+
     def itervalues(self):
         for v in self._comments.itervalues():
             yield v.__of__(self)
-    
+
     def iteritems(self):
         for k, v in self._comments.iteritems():
             yield (k, v.__of__(self),)
@@ -231,31 +232,31 @@ def conversationAdapterFactory(content):
 
 class ConversationReplies(object):
     """An IReplies adapter for conversations.
-    
+
     This makes it easy to work with top-level comments.
     """
-    
+
     implements(IReplies)
     adapts(Conversation) # relies on implementation details
-    
+
     def __init__(self, context):
         self.conversation = context
         self.children = self.conversation._children.get(0, LLSet())
-    
+
     def addComment(self, comment):
         comment.in_reply_to = None
         return self.conversation.addComment(comment)
-        
+
     # Dict API
-    
+
     def __len__(self):
         return len(self.children)
-    
+
     def __contains__(self, key):
         return long(key) in self.children
-    
+
     # TODO: Should __getitem__, get, __iter__, values(), items() and iter* return aq-wrapped comments?
-    
+
     def __getitem__(self, key):
         """Get an item by its long key
         """
@@ -263,7 +264,7 @@ class ConversationReplies(object):
         if key not in self.children:
             raise KeyError(key)
         return self.conversation[key]
-    
+
     def __delitem__(self, key):
         """Delete an item by its long key
         """
@@ -271,42 +272,42 @@ class ConversationReplies(object):
         if key not in self.children:
             raise KeyError(key)
         del self.conversation[key]
-    
+
     def __iter__(self):
         return iter(self.children)
-    
+
     def get(self, key, default=None):
         key = long(key)
         if key not in self.children:
             return default
         return self.conversation.get(key)
-    
+
     def keys(self):
         return self.children
-    
+
     def items(self):
         return [(k, self.conversation[k]) for k in self.children]
-    
+
     def values(self):
         return [self.conversation[k] for k in self.children]
-    
+
     def iterkeys(self):
         return iter(self.children)
-    
+
     def itervalues(self):
         for key in self.children:
             yield self.conversation[key]
-    
+
     def iteritems(self):
         for key in self.children:
             yield (key, self.conversation[key],)
 
 class CommentReplies(ConversationReplies):
     """An IReplies adapter for comments.
-    
+
     This makes it easy to work with replies to specific comments.
     """
-    
+
     implements(IReplies)
 
     # depends on implementation details of conversation
@@ -314,20 +315,20 @@ class CommentReplies(ConversationReplies):
     # have a different type of Comment
 
     adapts(Comment)
-    
+
-    
+
     def __init__(self, context):
         self.comment = context
         self.conversation = self.comment.__parent__
-        
+
         if self.conversation is None or not hasattr(self.conversation, '_children'):
             raise TypeError("This adapter doesn't know what to do with the parent conversation")
-        
+
         self.comment_id = self.comment.comment_id
         self.children = self.conversation._children.get(self.comment_id, LLSet())
-    
+
     def addComment(self, comment):
         comment.in_reply_to = self.comment_id
         return self.conversation.addComment(comment)
-    
+
     # Dict API is inherited, written in terms of self.conversation and self.children

plone/app/discussion/tests/test_conversation.py

@@ -48,7 +48,29 @@ class ConversationTest(PloneTestCase):
         self.assert_(conversation.last_comment_date - datetime.now() < timedelta(seconds=1))
 
     def test_delete_comment(self):
-        pass
+        # Create a conversation. In this case we doesn't assign it to an
+        # object, as we just want to check the Conversation object API.
+        conversation = IConversation(self.portal.doc1)
+
+        # Pretend that we have traversed to the comment by aq wrapping it.
+        conversation = conversation.__of__(self.portal.doc1)
+
+        # Add a comment. Note: in real life, we always create comments via the factory
+        # to allow different factories to be swapped in
+
+        comment = createObject('plone.Comment')
+        comment.title = 'Comment 1'
+        comment.text = 'Comment text'
+
+        new_id = conversation.addComment(comment)
+
+        # delete the comment we just created
+        conversation.__delitem__(new_id)
+
+        # make sure there is no comment left in the conversation
+        self.assertEquals(len(conversation.getComments()), 0)
+        self.assertEquals(len(conversation.getThreads()), 0)
+        self.assertEquals(conversation.total_comments, 0)
 
     def test_dict_operations(self):
         # test dict operations and acquisition wrapping
@@ -103,13 +125,13 @@ class ConversationTest(PloneTestCase):
 
     def test_get_threads_batched(self):
         pass
-    
+
     def test_traversal(self):
         # make sure we can traverse to conversations and get a URL and path
-        
+
-        conversation = self.portal.doc1.restrictedTraverse('++conversation++default')        
+        conversation = self.portal.doc1.restrictedTraverse('++conversation++default')
         self.assert_(IConversation.providedBy(conversation))
-        
+
         self.assertEquals(('', 'plone', 'doc1', '++conversation++default'), conversation.getPhysicalPath())
         self.assertEquals('plone/doc1/%2B%2Bconversation%2B%2Bdefault', conversation.absolute_url())