accounting: use Base_redirect instead of lower level RESPONSE.redirect

This API takes care of building query string and redirecting to the same
form_id, this is a cleanup.

bt5/erp5_accounting/SkinTemplateItem/portal_skins/erp5_accounting/AccountingTransactionModule_createRelatedPaymentTransactionList.py

 from Products.ERP5Type.Message import translateString
-from zExceptions import Redirect
 
 portal = context.getPortalObject()
 countMessage = portal.portal_activities.countMessage
@@ -21,10 +20,10 @@ portal.portal_selections.setSelectionParamsFor('accounting_create_related_paymen
 
 # XXX prevent to call this on the whole module:
 if len(object_list) >= 1000:
-  return context.REQUEST.RESPONSE.redirect(
-    "%s/view?portal_status_message=%s" % (
-        context.absolute_url(), translateString(
-         'Refusing to process more than 1000 objects, check your selection.')))
+  return context.Base_redirect(
+      form_id,
+      keep_items={'portal_status_message': translateString(
+          'Refusing to process more than 1000 objects, check your selection.')})
 
 tag = 'payment_creation_%s' % random.randint(0, 1000)
 activated = 0
@@ -33,9 +32,11 @@ for obj in object_list:
     obj = obj.getObject()
     if countMessage(path=obj.getPath(),
                     method_id='Invoice_createRelatedPaymentTransaction'):
-      raise Redirect, "%s/view?portal_status_message=%s" % (
-                context.absolute_url(), translateString(
-        'Payment creation already in progress, abandon.'))
+      return context.Base_redirect(
+          form_id,
+          abort_transaction=True,
+          keep_items={'portal_status_message': translateString(
+              'Payment creation already in progress, abandon.')})
     obj.activate(tag=tag).Invoice_createRelatedPaymentTransaction(
                                                   node=node,
                                                   payment_mode=payment_mode,
@@ -44,17 +45,18 @@ for obj in object_list:
     activated += 1
 
 if not activated:
-  return context.REQUEST.RESPONSE.redirect(
-    "%s/view?portal_status_message=%s" % (
-        context.absolute_url(), translateString('No invoice in your selection.')))
+  return context.Base_redirect(
+      form_id,
+      keep_items={'portal_status_message': translateString(
+          'No invoice in your selection.')})
 
 # activate something on the folder
 context.activate(after_tag=tag).getTitle()
 
-return context.REQUEST.RESPONSE.redirect(
-    "%s/view?portal_status_message=%s" % (
-        context.absolute_url(), translateString(
-          'Payments creation for ${activated_invoice_count} on'
-          ' ${total_selection_count} invoices in progress.',
-          mapping=dict(activated_invoice_count=activated,
-                       total_selection_count=len(object_list)))))
+return context.Base_redirect(
+    form_id,
+    keep_items={'portal_status_message': translateString(
+        'Payments creation for ${activated_invoice_count} on'
+        ' ${total_selection_count} invoices in progress.',
+        mapping={'activated_invoice_count': activated,
+                 'total_selection_count': len(object_list)})})

bt5/erp5_accounting/SkinTemplateItem/portal_skins/erp5_accounting/AccountingTransactionModule_createRelatedPaymentTransactionList.xml

@@ -50,7 +50,7 @@
         </item>
         <item>
             <key> <string>_params</string> </key>
-            <value> <string>node, payment_mode, payment, selection_index=None, uids=(), listbox_uid=(),selection_name=\'\', **kw</string> </value>
+            <value> <string>node, payment_mode, payment, selection_index=None, uids=(), listbox_uid=(),selection_name=\'\', form_id=\'\', **kw</string> </value>
         </item>
         <item>
             <key> <string>id</string> </key>

bt5/erp5_accounting/SkinTemplateItem/portal_skins/erp5_accounting/Invoice_createRelatedPaymentTransaction.py

@@ -30,9 +30,9 @@ total_payable_price_details = \
 # if there's nothing more to pay, don't create an empty transaction
 if sum(total_payable_price_details.values()) == 0:
   if not batch_mode:
-    return context.REQUEST.RESPONSE.redirect(
-      "%s/view?portal_status_message=%s" % (
-      context.absolute_url(), Base_translateString('Nothing more to pay.')))
+    return context.Base_redirect(
+      form_id,
+      keep_items={'portal_status_message': Base_translateString('Nothing more to pay.')})
   return None
 
 related_payment = portal.accounting_module.newContent(

bt5/erp5_accounting/SkinTemplateItem/portal_skins/erp5_accounting/Invoice_createRelatedPaymentTransaction.xml

@@ -50,7 +50,7 @@
         </item>
         <item>
             <key> <string>_params</string> </key>
-            <value> <string>node, payment_mode, payment, date=None, plan=False, batch_mode=0, **kw</string> </value>
+            <value> <string>node, payment_mode, payment, date=None, plan=False, batch_mode=0, form_id=\'\', **kw</string> </value>
         </item>
         <item>
             <key> <string>id</string> </key>

[Georgios Dagkakis]

detail, but in case a method is used several times, doesn't it worth it to access it once? I.e. in the start of the script:

redirect = context.Base_redirect

and then

return redirect(...)

[Nicolas Wavrant]

In that case the Acquisition is calculated only once, as Base_redirect are into exclusive branches of the script. So from a performance point of view there is no difference, and the change you propose is mainly cosmetics

[Jérome Perrin]

Thanks for your comment.

That's a good point, because generally speaking, in restricted python it's better to avoid getattr, because guarded gettattr is slow.

The difference between

for i in range(1000):
   context.Base_randomScript(i)

and

randomScript = context.Base_randomScript
for i in range(1000):
   randomScript(i)

is really big, so in such a case, it's worth making a variable to save getattr. We should probably add this to our page of perfomance tips/crimes.

In the case of this script, redirect will be called only once, because we return from the script, so from performance point of view it should not make any difference. From code style point of view, I find it more readable to use context.Base_redirect directly.

I would prefer to leave it like this if that's ok with you.

[Georgios Dagkakis]

In that case the Acquisition is calculated only once, as Base_redirect are into exclusive branches of the script

indeed and ok to leave as is