erp5_core: Fixes in Folder delete

See merge request nexedi/erp5!1381

bt5/erp5_l10n_fr/MessageTranslationTemplateItem/fr/erp5_ui/translation.po

@@ -8254,6 +8254,9 @@ msgstr "Sans comptes non mouvementés"
 msgid "On <a href=\"${decision_url}\">${decision_type} ${decision_title}</a> of <a href=\"${delivery_url}\">${delivery_title}</a> :"
 msgstr "<a href=\"${decision_url}\">${decision_type} ${decision_title}</a> de <a href=\"${delivery_url}\">${delivery_title}</a>: "
 
+msgid "One or more documents could not be deleted."
+msgstr "Un ou plusieurs documents n'ont pas pu être supprimés."
+
 msgid "Online Content:"
 msgstr "Contenu en ligne:"
 
@@ -13894,6 +13897,9 @@ msgstr "Effectuez une recherche pour afficher le contenu."
 msgid "To reset your password, please enter your login below. An email will be sent to your address with a link to enter your new password."
 msgstr "Afin de changer votre mot de passe, entrez votre nom d'utilisateur ci-dessous. Un email contenant des explications sur la marche à suivre sera envoyé à votre adresse."
 
+msgid "Too many documents selected."
+msgstr "Trop de documents sélectionnés."
+
 msgid "Too many documents were found."
 msgstr "Trop de possibilités trouvées."
 

bt5/erp5_ui_test/PathTemplateItem/portal_tests/listbox_zuite/testDelete.zpt

@@ -187,9 +187,8 @@ It contains the same columns
 </tr>
 <tr>
   <td>assertTextPresent</td>
-  <td>Deleted.</td>
+  <td>One or more documents could not be deleted.</td>
   <td></td>
-  <td>We used to report error on user deleting deleted document ... but user does not care, does she?</td>
 </tr>
 <tr>
   <td>clickAndWait</td>

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

@@ -15,6 +15,7 @@ the selection.
 """
 from ZODB.POSException import ConflictError
 from Products.CMFCore.WorkflowCore import WorkflowException
+from Products.ERP5Type.Message import Message
 
 portal = context.getPortalObject()
 Base_translateString = portal.Base_translateString
@@ -32,28 +33,32 @@ if not listbox_uid:
     'portal_status_level': "warning"})
 
 
-if True:
-  # already filters out documents with relations that cannot be deleted
-  object_list = context.Folder_getDeleteObjectList(uid=listbox_uid)
-  object_not_deletable_len = len(listbox_uid) - len(object_list)
+# already filters out documents with relations that cannot be deleted
+object_list = context.Folder_getDeleteObjectList(uid=listbox_uid)
+if len(object_list) > 1000:
+  return context.Base_redirect(
+    keep_items={
+    'portal_status_message': translate("Too many documents selected."),
+    'portal_status_level': "warning",
+    }
+  )
+object_not_deletable_len = len(listbox_uid) - len(object_list)
 
-  # some documents cannot be deleted thus we stop and warn the user
-  if object_not_deletable_len == 1:
+# some documents cannot be deleted thus we stop and warn the user
+if object_not_deletable_len == 1:
   return context.Base_redirect(keep_items={
     'portal_status_message': translate("Sorry, 1 item is in use."),
     'portal_status_level': "warning"})
-  elif object_not_deletable_len > 1:
+elif object_not_deletable_len > 1:
   return context.Base_redirect(keep_items={
     'portal_status_message': translate("Sorry, ${count} items are in use.", mapping={'count': str(object_not_deletable_len)}),
     'portal_status_level': "warning"})
 
-  if True:
+# Do not delete objects which have a workflow history
+object_to_remove_list = []
+object_to_delete_list = []
 
-    # Do not delete objects which have a workflow history    
-    object_to_remove_list = []
-    object_to_delete_list = []
-
-    for obj in object_list:
+for obj in object_list:
 
   history_dict = obj.Base_getWorkflowHistory()
   history_dict.pop('edit_workflow', None)
@@ -66,52 +71,81 @@ if True:
     # object should not be removed, but only put in state deleted
     object_to_delete_list.append(obj)
 
-    # Remove some objects
+# Remove some objects
+if object_to_remove_list:
+  if context.getPortalType() != 'Preference':
+    # Use uids so that we can delete from listboxs showing documents
+    # that are not sub-objects of the current document
     try:
-      if object_to_remove_list:
-        if context.portal_type == 'Preference':
+      context.manage_delObjects(
+        uids=[x.getUid() for x in object_to_remove_list],
+        REQUEST=REQUEST,
+      )
+    except ConflictError:
+      raise
+    except Exception as error:
+      # Note, this does not rollback transaction, so in case manage_delObjects
+      # did delete some objects these will be deleted.
+      # This may be considered a feature of user point of view
+      # (something went wrong with some documents, but some where deleted)
+      return context.Base_renderMessage(str(error), "error")
+  else:
     # Templates inside preference are not indexed, so we cannot pass
     # uids= to manage_delObjects and have to use ids=
-          context.manage_delObjects(
-                        ids=[x.getId() for x in object_to_remove_list],
-                        REQUEST=REQUEST)
+    for document in object_to_remove_list:
+      try:
+        document.getParentValue().manage_delObjects(
+          ids=[document.getId()]
+        )
+        # Clear cache to recalculate the list of possible templates in case one was deleted
         portal.portal_caches.clearCacheFactory('erp5_ui_medium')
-        else:
-          context.manage_delObjects(
-                        uids=[x.getUid() for x in object_to_remove_list],
-                        REQUEST=REQUEST)
       except ConflictError:
         raise
       except Exception as error:
+        # Note, this does not rollback transaction, so in case manage_delObjects
+        # did delete some objects these will be deleted.
+        # This may be considered a feature of user point of view
+        # (something went wrong with some documents, but some where deleted)
         return context.Base_renderMessage(str(error), "error")
-    else: # in the case of no exception raised report sucess
-      object_ids = [x.getId() for x in object_to_remove_list]
-      comment = Base_translateString('Deleted objects: ${object_ids}',
-                                     mapping={'object_ids': object_ids})
   try:
     # record object deletion in workflow history
-        portal.portal_workflow.doActionFor(context, 'edit_action',
-                                           comment=comment)
+    portal.portal_workflow.doActionFor(
+      context,
+      'edit_action',
+      comment=Message(
+        domain='ui',
+        message='Deleted objects: ${object_ids}',
+        mapping={'object_ids': [x.getId() for x in object_to_remove_list]},
+      ),
+    )
   except WorkflowException:
     # no 'edit_action' transition for this container
     pass
 
-      message = Base_translateString("Deleted.")
+message = Base_translateString("Deleted.")
+portal_status_level = "success"
 
-      # Try to call "delete_action" workflow transition on documents which defined it
-      # Failure of such a call is not a failure globally. The document was deleted anyway
-      for obj in object_to_delete_list:
+# Try to call "delete_action" workflow transition on documents which defined it
+for obj in object_to_delete_list:
   # Hidden transition (without a message displayed)
   # are not returned by getActionsFor
   try:
     portal.portal_workflow.doActionFor(obj, 'delete_action')
-        except ConflictError:
-          raise
-        except Exception:
-          pass
+  except WorkflowException:
+    # The user does not have rights to invoke delete action.
+    # Do not raise since delete_action may succeed for other selected documents
+    # Do give a warning though
+    message = Base_translateString("One or more documents could not be deleted.")
+    portal_status_level = "warning"
 
-    # make sure nothing is checked after
-    if selection_name:
+# make sure nothing is checked after
+if selection_name:
   portal.portal_selections.setSelectionCheckedUidsFor(selection_name, ())
 
-return context.Base_redirect(form_id, keep_items={"portal_status_message": str(message)})
+return context.Base_redirect(
+  form_id,
+  keep_items={
+    "portal_status_message": str(message),
+    "portal_status_level": portal_status_level,
+  },
+)