Public
Snippet $106 authored by Jérome Perrin

interdiff 1 for nexedi/erp5!116

diff
diff -u b/product/ERP5/Document/WebSite.py b/product/ERP5/Document/WebSite.py
--- b/product/ERP5/Document/WebSite.py
+++ b/product/ERP5/Document/WebSite.py
@@ -210,8 +210,8 @@
       # unregister all before traversal hooks that do not belong to us.
       my_handle = self.meta_type + '/' + self.getId()
       handle_to_unregister_list = []
-      for priority, handle in self.__before_traverse__.keys():
-        if handle != my_handle:
+      for (priority, handle), hook in self.__before_traverse__.items():
+        if isinstance(hook, WebSiteTraversalHook) and handle != my_handle:
           handle_to_unregister_list.append(handle)
       for handle in handle_to_unregister_list:
         BeforeTraverse.unregisterBeforeTraverse(self, handle)
diff -u b/product/ERP5/tests/testERP5Web.py b/product/ERP5/tests/testERP5Web.py
--- b/product/ERP5/tests/testERP5Web.py
+++ b/product/ERP5/tests/testERP5Web.py
@@ -43,6 +43,85 @@
 MOVED_TEMPORARILY = 302
   
   
+class DummyTraversalHook(object):
+  def __call__(self, container, request):
+    return
+  
+class WebTraversalHookTestMixin(object):
+  """Mixin class to test the WebSiteTraversalHook on both websection and website.
+  """
+  def getBusinessTemplateList(self):
+    return ('erp5_base', 'erp5_web', )
+  
+  def test_TraversalHook_on_newContent(self):
+    """a WebSiteTraversalHook is added on websections and websites automatically.
+    """
+    self.assertEquals(1, len(self.web_section.__before_traverse__))
+    self.assertIsInstance(
+      self.web_section.__before_traverse__.values()[0],
+      self.traversal_hook_class)
+  
+  def test_TraversalHook_on_clone(self):
+    """a WebSiteTraversalHook is correctly updated after cloning a websection/website.
+    """
+    cloned_web_section = self.web_section.Base_createCloneDocument(batch_mode=True)
+    self.assertEquals(1, len(cloned_web_section.__before_traverse__))
+  
+  def test_TraversalHook_on_change_id(self):
+    """a TraversalHook is correctly updated after changing websection id.
+    """
+    self.tic()
+    self.web_section.setId("new_id")
+    self.assertEquals(1, len(self.web_section.__before_traverse__))
+  
+  def test_TraversalHook_cleanup_on_edit(self):
+    """Old traversal hooks from cloned objects are automatically cleaned up
+    when section is edited.
+    """
+    # artificially put this websection in a similar state that websection were before
+    # we fix the bug keeping traversal hooks on clone.
+    from ZPublisher import BeforeTraverse
+    handle = '%s/different_id_than_%s' % (self.web_section.meta_type, self.web_section.getId())
+    BeforeTraverse.registerBeforeTraverse(self.web_section, self.traversal_hook_class(), handle)
+  
+    BeforeTraverse.registerBeforeTraverse(
+      self.web_section,
+      DummyTraversalHook(),
+      'unrelated_traversal_hook_that_should_be_kept')
+    self.assertEquals(3, len(self.web_section.__before_traverse__))
+  
+    self.tic()
+    self.web_section.edit(title=self.id())
+    # We have cleaned up the useless before traversal hook, but keep the unrelated one
+    self.assertEquals(2, len(self.web_section.__before_traverse__))
+    self.assertEquals(1, len([hook for hook in
+      self.web_section.__before_traverse__.values() if isinstance(hook, self.traversal_hook_class)]))
+    self.assertEquals(1, len([hook for hook in
+      self.web_section.__before_traverse__.values() if isinstance(hook, DummyTraversalHook)]))
+  
+  
+class TestWebSiteTraversalHook(WebTraversalHookTestMixin, ERP5TypeTestCase):
+  def afterSetUp(self):
+    super(TestWebSiteTraversalHook, self).afterSetUp()
+    self.web_section = self.portal.web_site_module.newContent(
+      portal_type='Web Site',
+    )
+    from Products.ERP5Type.Document.WebSite import WebSiteTraversalHook
+    self.traversal_hook_class = WebSiteTraversalHook
+  
+class TestWebSectionTraversalHook(WebTraversalHookTestMixin, ERP5TypeTestCase):
+  def afterSetUp(self):
+    super(TestWebSectionTraversalHook, self).afterSetUp()
+    web_site = self.portal.web_site_module.newContent(
+      portal_type='Web Site',
+    )
+    self.web_section = web_site.newContent(portal_type='Web Section')
+  
+    from Products.ERP5Type.Document.WebSection import WebSectionTraversalHook
+    self.traversal_hook_class = WebSectionTraversalHook
+  
+  
+  
 class TestERP5Web(ERP5TypeTestCase):
   """Test for erp5_web business template.
   """
@@ -169,32 +248,7 @@
     except:
       self.fail('Cataloging of the Web Site failed.')
   
-  def test_WebSiteTraversalHook_on_newContent(self):
-    """a WebSiteTraversalHook is added on websites automatically.
-    """
-    web_site = self.portal.web_site_module.newContent(
-      portal_type='Web Site',
-    )
-    self.assertEquals(1, len(web_site.__before_traverse__))
-  
-  def test_WebSiteTraversalHook_on_clone(self):
-    """a WebSiteTraversalHook is correctly updated after cloning a website.
-    """
-    web_site = self.portal.web_site_module.newContent(
-      portal_type='Web Site',
-    )
-    cloned_web_site = web_site.Base_createCloneDocument(batch_mode=True)
-    self.assertEquals(1, len(cloned_web_site.__before_traverse__))
   
-  def test_WebSiteTraversalHook_on_change_id(self):
-    """a WebSiteTraversalHook is correctly updated after changing website id.
-    """
-    web_site = self.portal.web_site_module.newContent(
-      portal_type='Web Site',
-    )
-    self.tic()
-    web_site.setId("new_id")
-    self.assertEquals(1, len(web_site.__before_traverse__))
   
   def test_02_EditSimpleWebPage(self):
     """
@@ -1961,2 +2015,4 @@
   suite.addTest(unittest.makeSuite(TestERP5WebCategoryPublicationWorkflow))
+  suite.addTest(unittest.makeSuite(TestWebSiteTraversalHook))
+  suite.addTest(unittest.makeSuite(TestWebSectionTraversalHook))
   return suite
only in patch2:
unchanged:
--- a/product/ERP5/Document/WebSection.py
+++ b/product/ERP5/Document/WebSection.py
@@ -151,6 +151,21 @@ class WebSection(Domain, DocumentExtensibleTraversableMixin):
         BeforeTraverse.registerBeforeTraverse(item, WebSectionTraversalHook(), handle)
       super(WebSection, self).manage_afterAdd(item, container)
   
+    security.declarePrivate( 'manage_afterClone' )
+    def manage_afterClone(self, item):
+      self._cleanupBeforeTraverseHooks()
+      super(WebSection, self).manage_afterClone(item)
+  
+    def _cleanupBeforeTraverseHooks(self):
+      # unregister all before traversal hooks that do not belong to us.
+      my_handle = self.meta_type + '/' + self.getId()
+      handle_to_unregister_list = []
+      for (priority, handle), hook in self.__before_traverse__.items():
+        if isinstance(hook, WebSectionTraversalHook) and handle != my_handle:
+          handle_to_unregister_list.append(handle)
+      for handle in handle_to_unregister_list:
+        BeforeTraverse.unregisterBeforeTraverse(self, handle)
+  
     security.declareProtected(Permissions.AccessContentsInformation, 'getLayoutProperty')
     def getLayoutProperty(self, key, default=None):
       """
@@ -427,8 +442,12 @@ class WebSection(Domain, DocumentExtensibleTraversableMixin):
       return result
   
     def _edit(self, **kw):
-      # migrate beforeTraverse hook if missing
-      if getattr(self, '__before_traverse__', None) is None and self.getPortalType() == 'Web Section':
-        handle = self.meta_type + '/' + self.getId()
-        BeforeTraverse.registerBeforeTraverse(self, WebSectionTraversalHook(), handle)
+      if self.getPortalType() == 'Web Section':
+        if getattr(self, '__before_traverse__', None) is None:
+          # migrate beforeTraverse hook if missing
+          handle = self.meta_type + '/' + self.getId()
+          BeforeTraverse.registerBeforeTraverse(self, WebSectionTraversalHook(), handle)
+        else:
+          # cleanup beforeTraverse hooks that may exist after this document was cloned.
+          self._cleanupBeforeTraverseHooks()
       super(WebSection, self)._edit(**kw)