From 56b08b2247cbaf440f223074f404f0fefc03e900 Mon Sep 17 00:00:00 2001 From: Gaudenz Steinlin Date: Mon, 17 Sep 2012 16:43:19 +0200 Subject: [PATCH] Fix catalog updates for IObjectMovedEvent If comments were moved because an object on a higher level than the commented object was moved, these comments were wrongly reindexed. The commented object also got a wrong __parent__ pointer. This fixes https://dev.plone.org/ticket/13172. --- plone/app/discussion/comment.py | 13 +++++-- plone/app/discussion/tests/test_catalog.py | 45 ++++++++++++++++++++++ 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/plone/app/discussion/comment.py b/plone/app/discussion/comment.py index 171f4df..847b99b 100644 --- a/plone/app/discussion/comment.py +++ b/plone/app/discussion/comment.py @@ -243,9 +243,17 @@ def notify_content_object_moved(obj, event): if event.oldParent is None or event.newParent is None \ or event.oldName is None or event.newName is None: return + + # This method is also called for sublocations of moved objects. We therefore can't + # assume that event.object == obj and event.{old,new}{Parent,Name} may refer to + # the actually moved object further up in the object hierarchy. + # The object is already moved at this point. so obj.getPhysicalPath retruns the new path. + # get the part of the path that was moved. + moved_path = obj.getPhysicalPath()[len(event.newParent.getPhysicalPath()) + 1:] + # Remove comments at the old location from catalog catalog = getToolByName(obj, 'portal_catalog') - old_path = '/'.join(event.oldParent.getPhysicalPath() + (event.oldName,)) + old_path = '/'.join(event.oldParent.getPhysicalPath() + (event.oldName,) + moved_path) brains = catalog.searchResults(dict( path={'query': old_path}, portal_type="Discussion Item" @@ -256,8 +264,7 @@ def notify_content_object_moved(obj, event): conversation = IConversation(obj, None) if conversation is not None: for comment in conversation.getComments(): - aq_base(comment).__parent__.__parent__.__parent__ = event.newParent - catalog.reindexObject(aq_base(comment)) + comment.reindexObject() def notify_user(obj, event): diff --git a/plone/app/discussion/tests/test_catalog.py b/plone/app/discussion/tests/test_catalog.py index 3042843..1194db2 100644 --- a/plone/app/discussion/tests/test_catalog.py +++ b/plone/app/discussion/tests/test_catalog.py @@ -331,6 +331,51 @@ class CommentCatalogTest(unittest.TestCase): '/plone/folder2/moveme/++conversation++default/' + str(comment_id)) + + def test_move_upper_level_folder(self): + # create a folder with a nested structure + self.portal.invokeFactory(id='sourcefolder', + title='Source Folder', + type_name='Folder') + self.portal.sourcefolder.invokeFactory(id='moveme', + title='Move Me', + type_name='Folder') + self.portal.sourcefolder.moveme.invokeFactory(id='mydocument', + title='My Document', + type_name='Folder') + self.portal.invokeFactory(id='targetfolder', + title='Target Folder', + type_name='Folder') + + # create comment on my-document + conversation = IConversation(self.portal.sourcefolder.moveme.mydocument) + comment = createObject('plone.Comment') + comment_id = conversation.addComment(comment) + + # We need to commit here so that _p_jar isn't None and move will work + transaction.savepoint(optimistic=True) + + # Move moveme from folder1 to folder2 + cp = self.portal.sourcefolder.manage_cutObjects(ids=('moveme',)) + self.portal.targetfolder.manage_pasteObjects(cp) + + # Make sure no old comment brains are left + brains = self.catalog.searchResults(dict( + portal_type="Discussion Item", + path={'query': '/plone/sourcefolder/moveme'} + )) + self.assertEqual(len(brains), 0) + + # make sure comments are correctly index on the target + brains = self.catalog.searchResults(dict( + portal_type="Discussion Item", + path={'query': '/plone/targetfolder/moveme'} + )) + self.assertEqual(len(brains), 1) + self.assertEqual(brains[0].getPath(), + '/plone/targetfolder/moveme/mydocument/++conversation++default/' + + str(comment_id)) + def test_update_comments_when_content_object_is_renamed(self): # We need to commit here so that _p_jar isn't None and move will work transaction.savepoint(optimistic=True)