Move selection checksum computation next to its verification

Allows getting current selection's MD5 even outside ListBox's renderer.
Signed-off-by: Julien Muchembled <jm@nexedi.com>

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

@@ -129,7 +129,6 @@ if dialog_method == \'Base_createRelation\':\n
 if dialog_method == \'Folder_delete\':\n
   return context.Folder_delete(form_id=kw[\'form_id\'],\n
                                selection_name=kw[\'selection_name\'],\n
-                               uids=kw[\'listbox_uid\'],\n
                                md5_object_uid_list=kw[\'md5_object_uid_list\'])\n
 \n
 \n

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

@@ -57,24 +57,13 @@ from Products.CMFCore.WorkflowCore import WorkflowException\n
 \n
 portal = context.getPortalObject()\n
 Base_translateString = portal.Base_translateString\n
+REQUEST = portal.REQUEST\n
 \n
-selected_uids = portal.portal_selections.updateSelectionCheckedUidList(\n
-                            selection_name,listbox_uid,uids)\n
 uids = portal.portal_selections.getSelectionCheckedUidsFor(selection_name)\n
-\n
-error = portal.portal_selections.selectionHasChanged(md5_object_uid_list,\n
-                                                      uids)\n
-\n
-REQUEST=context.REQUEST\n
-qs = \'\'\n
-ret_url = context.absolute_url() + \'/\' + form_id\n
-if error:\n
+if portal.portal_selections.selectionHasChanged(md5_object_uid_list, uids):\n
   message = Base_translateString("Sorry, your selection has changed.")\n
-  qs = \'?portal_status_message=%s\' % message\n
-elif uids is not None:\n
+elif uids:\n
   # Check if there is some related objets.\n
-  object_used = 0\n
-\n
   object_list = [x.getObject() for x in context.Folder_getDeleteObjectList(uid=uids)]\n
   object_used = sum([x.getRelationCountForDeletion() and 1 for x in object_list])\n
 \n
@@ -84,7 +73,6 @@ elif uids is not None:\n
     else:\n
       message = Base_translateString("Sorry, ${count} items are in use.",\n
                                      mapping={\'count\': repr(object_used)})\n
-    qs = \'?portal_status_message=%s\' % message  \n
   else:\n
 \n
     # Do not delete objects which have a workflow history    \n
@@ -106,7 +94,7 @@ elif uids is not None:\n
 \n
     # Remove some objects\n
     try:\n
-      if object_to_remove_list != []:\n
+      if object_to_remove_list:\n
         if context.portal_type == \'Preference\':\n
           # Templates inside preference are not indexed, so we cannot pass\n
           # uids= to manage_delObjects and have to use ids=\n
@@ -121,7 +109,7 @@ elif uids is not None:\n
     except ConflictError:\n
       raise\n
     except Exception, message:\n
-      qs = \'?portal_status_message=%s\' % message\n
+      pass\n
     else:\n
       object_ids = [x.getId() for x in object_to_remove_list]\n
       comment = Base_translateString(\'Deleted objects: ${object_ids}\',\n
@@ -147,7 +135,6 @@ elif uids is not None:\n
           raise\n
         except:\n
           not_deleted_count += 1\n
-          pass\n
 \n
       # Generate message\n
       if not_deleted_count == 1:\n
@@ -158,20 +145,20 @@ elif uids is not None:\n
                                        mapping={\'count\': not_deleted_count})\n
       qs = \'?portal_status_message=%s\' % message\n
 \n
+    # make sure nothing is checked after\n
+    portal.portal_selections.setSelectionCheckedUidsFor(selection_name, ())\n
 else:\n
   message = Base_translateString("Please select one or more items first.")\n
-  qs = \'?portal_status_message=%s\' % message\n
 \n
-# make sure nothing is checked after\n
-portal.portal_selections.setSelectionCheckedUidsFor(selection_name, [])\n
-return REQUEST.RESPONSE.redirect("%s%s" % (ret_url, qs))\n
+return REQUEST.RESPONSE.redirect("%s/%s?portal_status_message=%s"\n
+  % (context.absolute_url_path(), form_id, message))\n
 
 
 ]]></string> </value>
         </item>
         <item>
             <key> <string>_params</string> </key>
-            <value> <string>form_id=\'\',selection_index=None,object_uid=None,selection_name=None,field_id=None,uids=None,cancel_url=\'\',listbox_uid=[],md5_object_uid_list=\'\'</string> </value>
+            <value> <string>form_id=\'\',selection_index=None,object_uid=None,selection_name=None,field_id=None,cancel_url=\'\',md5_object_uid_list=\'\'</string> </value>
         </item>
         <item>
             <key> <string>id</string> </key>

product/ERP5/tests/testERP5Core.py

@@ -405,20 +405,23 @@ class TestERP5Core(ERP5TypeTestCase, ZopeTestCase.Functional):
     response = self.publish(self.portal_id, self.auth)
     self.assertEquals(HTTP_OK, response.getStatus())
 
+  def _Folder_delete(self, *object_list):
+    selection_name = 'test_selection'
+    portal_selections = self.portal.portal_selections
+    portal_selections.setSelectionCheckedUidsFor(selection_name,
+      [x.getUid() for x in object_list])
+    md5_string = portal_selections.getSelectionChecksum(selection_name)
+    return object_list[0].getParentValue().Folder_delete(
+      selection_name=selection_name, md5_object_uid_list=md5_string)
+
   def test_Folder_delete(self):
     module = self.portal.newContent(portal_type='Folder', id='test_folder')
     document_1 = module.newContent(portal_type='Folder', id='1')
     document_2 = module.newContent(portal_type='Folder', id='2')
-    uid_list = [document_1.getUid(), document_2.getUid()]
-    self.portal.portal_selections.setSelectionParamsFor(
-          'test_selection', dict(uids=uid_list))
     self.tic()
-    md5_string = md5(str(sorted(map(str, uid_list)))).hexdigest()
-    redirect = module.Folder_delete(selection_name='test_selection',
-                                    uids=uid_list,
-                                    md5_object_uid_list=md5_string)
+    redirect = self._Folder_delete(document_1, document_2)
     self.assert_('Deleted.' in redirect, redirect)
-    self.assertEquals(len(module.objectValues()), 0)
+    self.assertEqual(module.objectCount(), 0)
 
   def test_Folder_delete_related_object(self):
     # deletion is refused if there are related objects
@@ -435,12 +438,7 @@ class TestERP5Core(ERP5TypeTestCase, ZopeTestCase.Functional):
     self.assertEqual(2, organisation.getRelationCountForDeletion())
     self.assertEqual(0, person.getRelationCountForDeletion())
     def delete(assert_deleted, obj):
-      uid_list = [obj.getUid()]
-      self.portal.portal_selections.setSelectionParamsFor(
-                          'test_selection', dict(uids=uid_list))
-      md5_string = md5(str(sorted(map(str, uid_list)))).hexdigest()
-      redirect = obj.getParentValue().Folder_delete(uids=uid_list,
-        selection_name='test_selection', md5_object_uid_list=md5_string)
+      redirect = self._Folder_delete(obj)
       self.assertTrue(('Sorry, 1 item is in use.', 'Deleted.')[assert_deleted]
                       in redirect, redirect)
       self.tic()
@@ -463,21 +461,15 @@ class TestERP5Core(ERP5TypeTestCase, ZopeTestCase.Functional):
                                 context=document_1,
                                 base_category_list=('source',),
                                 category_list=document_2.getRelativeUrl())
-    uid_list = [document_2.getUid(), ]
-    self.portal.portal_selections.setSelectionParamsFor(
-                          'test_selection', dict(uids=uid_list))
     self.tic()
     self.assertEquals([document_1],
         self.portal.portal_categories.getRelatedValueList(document_2))
-    md5_string = md5(str(sorted(map(str, uid_list)))).hexdigest()
 
     document_1.manage_permission('View', [], acquire=0)
     document_1.manage_permission('Access contents information', [], acquire=0)
-    redirect = module.Folder_delete(selection_name='test_selection',
-                                    uids=uid_list,
-                                    md5_object_uid_list=md5_string)
+    redirect = self._Folder_delete(document_2)
     self.assert_('Sorry, 1 item is in use.' in redirect, redirect)
-    self.assertEquals(len(module.objectValues()), 2)
+    self.assertEqual(module.objectCount(), 2)
 
   def test_getPropertyForUid(self):
     error_list = []

product/ERP5Form/ListBox.py

@@ -50,7 +50,6 @@ from Products.ERP5Type.Globals import InitializeClass, get_request
 from Products.PythonScripts.Utility import allow_class
 from Products.PageTemplates.PageTemplateFile import PageTemplateFile
 from warnings import warn
-from hashlib import md5
 import cgi
 
 DEFAULT_LISTBOX_DISPLAY_STYLE = 'table'
@@ -2560,20 +2559,14 @@ class ListBoxHTMLRenderer(ListBoxRenderer):
     """Generate a MD5 checksum against checked uids. This is used to confirm
     that selected values do not change between a display of a dialog and an execution.
 
-    FIXME: this should only use getCheckedUidList, but Folder_deleteObjectList does not use
-    the feature that checked uids are used when no list method is specified.
+    FIXME: SelectionTool.getSelectionChecksum's uid_list parameter is
+    deprecated, but Folder_deleteObjectList does not use the feature that
+    checked uids are used when no list method is specified.
     """
-    checked_uid_list = self.request.get('uids')
-    if checked_uid_list is None:
-      checked_uid_list = self.getCheckedUidList()
-    if checked_uid_list is not None:
-      checked_uid_list = [str(uid) for uid in checked_uid_list]
-      checked_uid_list.sort()
-      md5_string = md5(str(checked_uid_list)).hexdigest()
-    else:
-      md5_string = None
-
-    return md5_string
+    return self.getSelectionTool().getSelectionChecksum(
+      self.getSelectionName(),
+      uid_list=self.request.get('uids'),
+    )
 
   def getPhysicalRoot(self):
     """Return the physical root (an Application object). This method is required for

product/ERP5Form/Tool/SelectionTool.py

@@ -1170,13 +1170,26 @@ class SelectionTool( BaseTool, SimpleItem ):
       """
         We want to be sure that the selection did not change
       """
+      return md5_string != self._getUIDListChecksum(object_uid_list)
+
+    security.declareProtected(ERP5Permissions.View, 'getSelectionChecksum')
+    def getSelectionChecksum(self, selection_name, uid_list=None):
+      """Generate an MD5 checksum against checked uids. This is used to confirm
+      that selected values do not change between a display of a dialog and an
+      execution.
+      uid_list (deprecated)
+        For backward compatibility with code not updating selected uids.
+      """
+      if uid_list is None:
+        uid_list = self.getSelectionCheckedUidsFor(selection_name)
+      return self._getUIDListChecksum(uid_list)
+
+    def _getUIDListChecksum(self, uid_list):
+      if uid_list is None:
+          return None
       # XXX To avoid the difference of the string representations of int and long,
       # convert each element to a string.
-      object_uid_list = [str(x) for x in object_uid_list]
-      object_uid_list.sort()
-      new_md5_string = md5(str(object_uid_list)).hexdigest()
-      return md5_string != new_md5_string
-
+      return md5(str(sorted(map(str, uid_list)))).hexdigest()
 
     # Related document searching
     def viewSearchRelatedDocumentDialog(self, index, form_id,