From 60c42769978d5952a3ea1b3ff952acfdde01e3fa Mon Sep 17 00:00:00 2001
From: Tomas Peterka <tomas.peterka@nexedi.com>
Date: Fri, 4 May 2018 16:08:04 +0200
Subject: [PATCH] [hal_json] Improve extra_param (aka keep_items) persistency
 between requests

---
 .../Base_callDialogMethod.py                  | 36 ++++++++++++------
 .../erp5_hal_json_style/Base_renderForm.py    | 19 ++++++----
 .../ERP5Document_getHateoas.py                |  5 ---
 .../erp5_ui_test/Folder_doNothing.py          | 17 ++++++++-
 .../erp5_ui_test/Folder_doNothing.xml         |  2 +-
 .../erp5_ui_test/Folder_doNothingDialog.xml   |  2 +-
 .../renderjs_ui_zuite/testSelectAll.zpt       | 38 ++++++++++++++++++-
 7 files changed, 90 insertions(+), 29 deletions(-)

diff --git a/bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/Base_callDialogMethod.py b/bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/Base_callDialogMethod.py
index c5bc432180..f848f4d127 100644
--- a/bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/Base_callDialogMethod.py
+++ b/bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/Base_callDialogMethod.py
@@ -5,10 +5,11 @@ Responsible for validating form data and redirecting to the form action.
 Please note that the new UI has deprecated use of Selections. Your scripts
 will no longer receive `selection_name` nor `selection` in their arguments.
 
-There are runtime values hidden in every form (injected by getHateoas Script):
+There are runtime values hidden in every dialog form (injected by getHateoas Script):
   form_id - previous form ID (backward compatibility reasons)
   dialog_id - current form dialog ID
   dialog_method - method to be called - can be either update_method or dialog_method of the Dialog Form
+  extra_param_json - JSON serialized extra parameters for the dialog script
 """
 
 from Products.ERP5Type.Log import log, DEBUG, INFO, WARNING, ERROR
@@ -110,6 +111,21 @@ try:
   editable_mode = request.get('editable_mode', 1)
   request.set('editable_mode', 1)
   form_data = form.validate_all_to_request(request)
+  # Notify the underlying script whether user did modifications
+  form_hash = form.hash_validated_data(form_data)
+  # Inject `has_changed` parameter to arguments
+  if "form_hash" in extra_param:
+    kw['has_changed'] = (form_hash != extra_param.get('form_hash'))
+  # update form_hash here so we do not rely on developer/Dialog Script
+  # to pass it correctly
+  extra_param["form_hash"] = form_hash
+
+  # Put extra_param into request so we can pass it behind developers back
+  # it is picked up at Base_renderForm so the developer does not need to
+  # know that they should pass it.
+  # We cannot use **kwargs because all form fields get injected there so
+  # from our point of view it contains random garbage
+  request.set('extra_param', extra_param)
 
   request.set('editable_mode', editable_mode)
   default_skin = portal.portal_skins.getDefaultSkin()
@@ -187,7 +203,7 @@ if len(listbox_id_list):
 # First check for an query in form parameters - if they are there
 # that means previous view was a listbox with selected stuff so recover here
 query = extra_param.get("query", None)
-select_all = int(extra_param.pop("_select_all", "0"))
+select_all = extra_param.get("_select_all", 0)
 
 # inject `uids` into Scripts **kwargs when we got any `query` (empty or filled)
 if query is not None:
@@ -200,11 +216,12 @@ if query is not None:
 
 # early-stop if user selected all documents
 if query == "" and select_all == 0 and dialog_method != update_method:  # do not interrupt on UPDATE
+  extra_param["_select_all"] = 1
   return context.Base_renderForm(
     dialog_id,
     message=translate("All documents are selected! Submit again to proceed or Cancel and narrow down your search."),
     level=WARNING,
-    keep_items={'_select_all': 1},
+    keep_items=extra_param,
     query=query,
     form_data=form_data)
 
@@ -215,13 +232,9 @@ if query == "" and select_all == 0 and dialog_method != update_method:  # do not
 if dialog_category == "object_search" :
   portal.portal_selections.setSelectionParamsFor(kw['selection_name'], kw)
 
-# Notify the underlying script whether user did modifications
-form_hash = form.hash_validated_data(form_data)
-if "form_hash" in extra_param:
-  kw['has_changed'] = (form_hash != extra_param.pop('form_hash'))
-
 # Add rest of extra param into arguments of the target method
-kw.update(extra_param)
+kw.update(**extra_param)
+kw.update(keep_items=extra_param)  # better backward compatibility
 
 # Finally we will call the Dialog Method
 # Handle deferred style, unless we are executing the update action
@@ -239,7 +252,8 @@ if dialog_method != update_method and kw.get('deferred_style', 0):
       return context.Base_renderForm(dialog_id,
         message=translate('Deferred reports are possible only with preference '\
                           '"Report Style" set to "ODT" or "ODS"'),
-        level=WARNING)
+        level=WARNING,
+        keep_items=extra_param)
 
   # If the action form has report_view as it's method, it
   if page_template != 'report_view':
@@ -304,7 +318,7 @@ if True:
     meta_type = ""
 
   if meta_type in ("ERP5 Form", "ERP5 Report"):
-    return context.ERP5Document_getHateoas(REQUEST=request, form=dialog_form, mode="form", form_data=form_data)
+    return context.ERP5Document_getHateoas(REQUEST=request, form=dialog_form, mode="form", form_data=form_data, extra_param_json=extra_param)
 
   return dialog_form(**kw)
 
diff --git a/bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/Base_renderForm.py b/bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/Base_renderForm.py
index 0ed9e08378..095a940731 100644
--- a/bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/Base_renderForm.py
+++ b/bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/Base_renderForm.py
@@ -9,16 +9,21 @@ This script differs from Base_redirect that it keeps the form values in place.
 :param REQUEST: request
 :param **kwargs: should contain parameters to ERP5Document_getHateoas such as 'query' to replace Selections
 """
-
-keep_items = keep_items or {}
+REQUEST = REQUEST or context.REQUEST
 
 form = getattr(context, form_id)
 
-if not message and "portal_status_message" in keep_items:
-  message = keep_items.pop("portal_status_message")
+# recover "hidden field" from HAL_JSON interface behind developers back (sorry)
+extra_param_json = REQUEST.get('extra_param', {})
+
+if keep_items is not None:
+  if not message and "portal_status_message" in keep_items:
+    message = keep_items.pop("portal_status_message")
+
+  if not level and "portal_status_level" in keep_items:
+    level = keep_items.pop("portal_status_level")
 
-if not level and "portal_status_level" in keep_items:
-  level = keep_items.pop("portal_status_level")
+  extra_param_json.update(keep_items)
 
-return context.ERP5Document_getHateoas(form=form, mode='form', REQUEST=REQUEST, extra_param_json=keep_items,
+return context.ERP5Document_getHateoas(form=form, mode='form', REQUEST=REQUEST, extra_param_json=extra_param_json,
                                        portal_status_message=message, portal_status_level=level, **kwargs)
diff --git a/bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/ERP5Document_getHateoas.py b/bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/ERP5Document_getHateoas.py
index 301dbfcc93..34702249b7 100644
--- a/bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/ERP5Document_getHateoas.py
+++ b/bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/ERP5Document_getHateoas.py
@@ -1243,11 +1243,6 @@ def renderForm(traversed_document, form, response_dict, key_prefix=None, selecti
   # end-if report_section
 
   if form.pt == "form_dialog":
-    # Insert hash of current values into the form so scripts can see whether data has
-    # changed if they provide multi-step process
-    if form_data is not None:
-      extra_param_json["form_hash"] = form.hash_validated_data(form_data)
-
     # extra_param_json is a special field in forms (just like form_id). extra_param_json field holds JSON
     # metadata about the form (its hash and dynamic fields)
     renderHiddenField(response_dict, 'extra_param_json', json.dumps(extra_param_json))
diff --git a/bt5/erp5_ui_test/SkinTemplateItem/portal_skins/erp5_ui_test/Folder_doNothing.py b/bt5/erp5_ui_test/SkinTemplateItem/portal_skins/erp5_ui_test/Folder_doNothing.py
index a969dfaf69..c71916c579 100644
--- a/bt5/erp5_ui_test/SkinTemplateItem/portal_skins/erp5_ui_test/Folder_doNothing.py
+++ b/bt5/erp5_ui_test/SkinTemplateItem/portal_skins/erp5_ui_test/Folder_doNothing.py
@@ -2,12 +2,25 @@ from Products.ERP5Type.Log import log
 
 log("Folder method received dialog_id, form_id, uids and {!s}".format(kwargs.keys()))
 
-if 'has_changed' not in kwargs or kwargs['has_changed'] is None:
-  message = "Did nothing."
+if kwargs.get('has_changed', None) is None:
+  message = "First submission."
 else:
   if kwargs['has_changed']:
     message = "Data has changed."
   else:
     message = "Data the same."
 
+if kwargs.get("update_method", ""):
+  return context.Base_renderForm(dialog_id, message="Updated. " + message)
+
+if _my_confirmation == 0:
+  # Here is an example of unfriendly confirmation Script which takes
+  # whole keep_item for itself!
+  # It should take keep_items from parameters, update it and pass it
+  # along. But no programmer will ever comply with that so we are ready!
+  return context.Base_renderForm(dialog_id,
+                                 message="Submit again to confirm. " + message,
+                                 level='warning',
+                                 keep_items={'_my_confirmation': 1})
+
 return context.Base_redirect(form_id, keep_items={"portal_status_message": message})
diff --git a/bt5/erp5_ui_test/SkinTemplateItem/portal_skins/erp5_ui_test/Folder_doNothing.xml b/bt5/erp5_ui_test/SkinTemplateItem/portal_skins/erp5_ui_test/Folder_doNothing.xml
index 63e2bbc669..d84510574c 100644
--- a/bt5/erp5_ui_test/SkinTemplateItem/portal_skins/erp5_ui_test/Folder_doNothing.xml
+++ b/bt5/erp5_ui_test/SkinTemplateItem/portal_skins/erp5_ui_test/Folder_doNothing.xml
@@ -50,7 +50,7 @@
         </item>
         <item>
             <key> <string>_params</string> </key>
-            <value> <string>dialog_id, form_id, uids, **kwargs</string> </value>
+            <value> <string>dialog_id, form_id, uids, _my_confirmation=0, **kwargs</string> </value>
         </item>
         <item>
             <key> <string>id</string> </key>
diff --git a/bt5/erp5_ui_test/SkinTemplateItem/portal_skins/erp5_ui_test/Folder_doNothingDialog.xml b/bt5/erp5_ui_test/SkinTemplateItem/portal_skins/erp5_ui_test/Folder_doNothingDialog.xml
index cfafd4dc71..a6bd5173b1 100644
--- a/bt5/erp5_ui_test/SkinTemplateItem/portal_skins/erp5_ui_test/Folder_doNothingDialog.xml
+++ b/bt5/erp5_ui_test/SkinTemplateItem/portal_skins/erp5_ui_test/Folder_doNothingDialog.xml
@@ -121,7 +121,7 @@
         </item>
         <item>
             <key> <string>update_action</string> </key>
-            <value> <string>Folder_doNothingDialog</string> </value>
+            <value> <string>Folder_doNothing</string> </value>
         </item>
         <item>
             <key> <string>update_action_title</string> </key>
diff --git a/bt5/erp5_web_renderjs_ui_test/PathTemplateItem/portal_tests/renderjs_ui_zuite/testSelectAll.zpt b/bt5/erp5_web_renderjs_ui_test/PathTemplateItem/portal_tests/renderjs_ui_zuite/testSelectAll.zpt
index b63c154e3c..c9b13cf60c 100644
--- a/bt5/erp5_web_renderjs_ui_test/PathTemplateItem/portal_tests/renderjs_ui_zuite/testSelectAll.zpt
+++ b/bt5/erp5_web_renderjs_ui_test/PathTemplateItem/portal_tests/renderjs_ui_zuite/testSelectAll.zpt
@@ -95,7 +95,7 @@
 <tr><th rowspan="1" colspan="3">Updating the dialog must not trigger warning about all selected</th></tr>
 <tal:block metal:use-macro="here/Zuite_CommonTemplateForRenderjsUi/macros/update_dialog" />
 <tal:block metal:use-macro="here/Zuite_CommonTemplateForRenderjsUi/macros/wait_for_content_loaded" />
-<tal:block tal:define="notification_configuration python: {'class': 'success', 'text': 'Data received.'}">
+<tal:block tal:define="notification_configuration python: {'class': 'success', 'text': 'Updated. First submission.'}">
   <tal:block metal:use-macro="here/Zuite_CommonTemplateForRenderjsUi/macros/wait_for_notification" />
 </tal:block>
 
@@ -106,10 +106,25 @@
   <tal:block metal:use-macro="here/Zuite_CommonTemplateForRenderjsUi/macros/wait_for_notification" />
 </tal:block>
 
+<tr><td>assertValue</td>
+    <td>//input[@name="field_custom_variable"]</td>
+    <td>couscous</td></tr>
+
 <tr><td>type</td>
     <td>//input[@name="field_custom_variable"]</td>
     <td>couscous</td></tr>
 
+<tr><th rowspan="1" colspan="3">Submitting, however, must warn the user</th></tr>
+<tal:block metal:use-macro="here/Zuite_CommonTemplateForRenderjsUi/macros/submit_dialog" />
+<tal:block metal:use-macro="here/Zuite_CommonTemplateForRenderjsUi/macros/wait_for_content_loaded" />
+<tal:block tal:define="notification_configuration python: {'class': 'error', 'text': 'Submit again to confirm. Data the same.'}">
+  <tal:block metal:use-macro="here/Zuite_CommonTemplateForRenderjsUi/macros/wait_for_notification" />
+</tal:block>
+
+<tr><td>assertValue</td>
+    <td>//input[@name="field_custom_variable"]</td>
+    <td>couscous</td></tr>
+
 <tr><th rowspan="1" colspan="3">Second submission must work as advertised</th></tr>
 <tal:block metal:use-macro="here/Zuite_CommonTemplateForRenderjsUi/macros/submit_dialog" />
 <tal:block metal:use-macro="here/Zuite_CommonTemplateForRenderjsUi/macros/wait_for_content_loaded" />
@@ -141,6 +156,19 @@
   <tal:block metal:use-macro="here/Zuite_CommonTemplateForRenderjsUi/macros/wait_for_notification" />
 </tal:block>
 
+<tr><td>waitForElementPresent</td>
+    <td>//input[@name="field_custom_variable"]</td><td></td></tr>
+<tr><td>type</td>
+    <td>//input[@name="field_custom_variable"]</td>
+    <td>musaka</td></tr>
+
+<tr><th rowspan="1" colspan="3">Now the user-script confirmation should kick in</th></tr>
+<tal:block metal:use-macro="here/Zuite_CommonTemplateForRenderjsUi/macros/submit_dialog" />
+<tal:block metal:use-macro="here/Zuite_CommonTemplateForRenderjsUi/macros/wait_for_content_loaded" />
+<tal:block tal:define="notification_configuration python: {'class': 'error', 'text': 'Submit again to confirm. Data has changed.'}">
+  <tal:block metal:use-macro="here/Zuite_CommonTemplateForRenderjsUi/macros/wait_for_notification" />
+</tal:block>
+
 <tr><td>waitForElementPresent</td>
     <td>//input[@name="field_custom_variable"]</td><td></td></tr>
 <tr><td>type</td>
@@ -189,7 +217,13 @@
 <tr><th rowspan="1" colspan="3">Second submission must work as advertised</th></tr>
 <tal:block metal:use-macro="here/Zuite_CommonTemplateForRenderjsUi/macros/submit_dialog" />
 <tal:block metal:use-macro="here/Zuite_CommonTemplateForRenderjsUi/macros/wait_for_content_loaded" />
-<tal:block tal:define="notification_configuration python: {'class': 'success', 'text': 'Did nothing.'}">
+<tal:block tal:define="notification_configuration python: {'class': 'error', 'text': 'Submit again to confirm. First submission.'}">
+  <tal:block metal:use-macro="here/Zuite_CommonTemplateForRenderjsUi/macros/wait_for_notification" />
+</tal:block>
+
+<tal:block metal:use-macro="here/Zuite_CommonTemplateForRenderjsUi/macros/submit_dialog" />
+<tal:block metal:use-macro="here/Zuite_CommonTemplateForRenderjsUi/macros/wait_for_content_loaded" />
+<tal:block tal:define="notification_configuration python: {'class': 'success', 'text': 'Data the same.'}">
   <tal:block metal:use-macro="here/Zuite_CommonTemplateForRenderjsUi/macros/wait_for_notification" />
 </tal:block>
 
-- 
2.30.9