CMFCategory: do not store self membership of categories

CategoryTool.getCategoryList dynamically returns categories as member of
themselves, but there were cases (for example after changing id) where
the membership happens to be saved in `categories` attribute and later
caused problems after clones or renames.
To prevent these problems, CategoryTool._setCategoryList detects the
membership to self and does not save it as part of .categories

Remove no longer needed after clone script.

Remove tests which no longer makes sense now that setCategoryList does
not add membership to self.

product/CMFCategory/CategoryTool.py

@@ -1189,11 +1189,19 @@ class CategoryTool(BaseTool):
     security.declareProtected( Permissions.ModifyPortalContent, '_setCategoryList' )
     def _setCategoryList(self, context, value):
       old = set(getattr(aq_base(context), 'categories', ()))
+      relative_url = context.getRelativeUrl()
+
+      # Pure category are member of itself, but we don't store this membership
+      # (because of issues when the category is renamed/moved or cloned)
+      if getattr(context, 'isCategory', 0) and relative_url in value:
+        # note that we don't want to cast as set at this point to keep ordering (and duplicates).
+        value = [x for x in tuple(value) if x != relative_url]
+
       context.categories = value = tuple(value)
       if context.isTempDocument():
         return
+
       value = set(value)
-      relative_url = context.getRelativeUrl()
       for edit, value in ("remove", old - value), ("add", value - old):
         for path in value:
           base = self.getBaseCategoryId(path)

product/CMFCategory/tests/testCMFCategory.py

@@ -29,6 +29,7 @@
 from collections import deque
 import unittest
 
+from Acquisition import aq_base
 from Products.ERP5Type.tests.ERP5TypeTestCase import ERP5TypeTestCase
 from Products.CMFCategory.Category import NBSP_UTF8
 from Testing.ZopeTestCase.PortalTestCase import PortalTestCase
@@ -659,52 +660,24 @@ class TestCMFCategory(ERP5TypeTestCase):
 
     """
     europe = self.portal.portal_categories.resolveCategory('region/europe')
-    self.assertTrue('region/europe' in europe.getCategoryList())
+    self.assertIn('region/europe', europe.getCategoryList())
     self.assertTrue(europe.isMemberOf('region/europe'))
 
-  def test_CategoryCloneIsNotMemberOfCopiedCategory(self):
-    # This is a bug reproduction test for problems happening when membership to
-    # self was saved in .categories.
-    europe_west = self.portal.portal_categories['region']['europe']['west']
-    france = europe_west.france
-
-    # Initially, categories' getCategoryList dynamically returns the
-    # "membership to self" eventhough it's not part of .categories, but after a
-    # category ID is changed, interaction update it's .categories to set the
-    # categories.
-    france.setCategoryList(france.getCategoryList())
-
-    cb_data = europe_west.manage_copyObjects(['france'])
-    destination = self.portal.portal_categories.region
-    copy_info, = destination.manage_pasteObjects(cb_data)
-    clone = destination[copy_info['new_id']]
-    self.assertEqual([clone.getRelativeUrl()], clone.getCategoryList())
-
-  def test_CategoryMovedIsNotMemberOfCopiedCategory(self):
-    # This is a bug reproduction test for problems happening when membership to
-    # self was saved in .categories.
-    original_container = self.portal.portal_categories['region']['europe']['west']
-    new_category = original_container.newContent(id=self.id())
-
-    # This sets membership to self in .categories
-    new_category.setCategoryList(new_category.getCategoryList())
-    self.commit()
+    # this membership is not saved in .categories
+    self.assertNotIn('region/europe', getattr(aq_base(europe), 'categories', ()))
 
-    cb_data = original_container.manage_cutObjects([new_category.getId()])
-    destination = self.portal.portal_categories.region
-    copy_info, = destination.manage_pasteObjects(cb_data)
-    clone = destination[copy_info['new_id']]
-    self.assertEqual([clone.getRelativeUrl()], clone.getCategoryList())
+    # even if we set explicitly
+    europe.setCategoryList(europe.getCategoryList())
+    self.assertNotIn('region/europe', getattr(aq_base(europe), 'categories', ()))
 
-  def test_19_getCategoryList(self):
-    """
-    check that getCategoryList called on a category does not append self again
-    and again
-    """
-    region_value = self.portal.portal_categories.resolveCategory('region/%s' % self.region1)
-    category_list = region_value.getCategoryList()
-    region_value.setCategoryList(category_list)
-    self.assertEqual(category_list, region_value.getCategoryList())
+    # or if we set other categories
+    europe.setCategoryList(['subordination/person_module'])
+    self.assertItemsEqual(
+            ['region/europe', 'subordination/person_module'],
+            europe.getCategoryList())
+    self.assertEqual(
+            ('subordination/person_module',),
+            getattr(aq_base(europe), 'categories', ()))
 
   def test_19_CategoryMemberValueList(self):
     """Test strict_membership parameter to Category Member Value List """

product/ERP5/bootstrap/erp5_core/SkinTemplateItem/portal_skins/erp5_core/Category_afterClone.py

-context.setCategoryMembership(context.getBaseCategoryId(), None)

product/ERP5/bootstrap/erp5_core/SkinTemplateItem/portal_skins/erp5_core/Category_afterClone.xml

-<?xml version="1.0"?>
-<ZopeData>
-  <record id="1" aka="AAAAAAAAAAE=">
-    <pickle>
-      <global name="PythonScript" module="Products.PythonScripts.PythonScript"/>
-    </pickle>
-    <pickle>
-      <dictionary>
-        <item>
-            <key> <string>Script_magic</string> </key>
-            <value> <int>3</int> </value>
-        </item>
-        <item>
-            <key> <string>_bind_names</string> </key>
-            <value>
-              <object>
-                <klass>
-                  <global name="NameAssignments" module="Shared.DC.Scripts.Bindings"/>
-                </klass>
-                <tuple/>
-                <state>
-                  <dictionary>
-                    <item>
-                        <key> <string>_asgns</string> </key>
-                        <value>
-                          <dictionary>
-                            <item>
-                                <key> <string>name_container</string> </key>
-                                <value> <string>container</string> </value>
-                            </item>
-                            <item>
-                                <key> <string>name_context</string> </key>
-                                <value> <string>context</string> </value>
-                            </item>
-                            <item>
-                                <key> <string>name_m_self</string> </key>
-                                <value> <string>script</string> </value>
-                            </item>
-                            <item>
-                                <key> <string>name_subpath</string> </key>
-                                <value> <string>traverse_subpath</string> </value>
-                            </item>
-                          </dictionary>
-                        </value>
-                    </item>
-                  </dictionary>
-                </state>
-              </object>
-            </value>
-        </item>
-        <item>
-            <key> <string>_params</string> </key>
-            <value> <string></string> </value>
-        </item>
-        <item>
-            <key> <string>id</string> </key>
-            <value> <string>Category_afterClone</string> </value>
-        </item>
-      </dictionary>
-    </pickle>
-  </record>
-</ZopeData>