[erp5_core] Carefull refactoring of Base_edit

bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/Base_edit.py

@@ -91,7 +91,10 @@ def editListBox(listbox_field, listbox):
 
 
 def editMatrixBox(matrixbox_field, matrixbox):
-  """ Function called to edit a Matrix box
+  """Go through every field in matrix and call edit on it.
+  
+  Most of the code is just a copy&paste from ERP5Form/MatrixBox:render and 
+  should be in (non-yet-existing) MatrixBoxEditor instead of here (XXX TODO).
   """
   if matrixbox is None:
     return
@@ -170,7 +173,7 @@ def editMatrixBox(matrixbox_field, matrixbox):
         matrix_context.setCellRange(base_id=cell_base_id, *matrixbox_cell_range)
 
   for cell_index_tuple, cell_value_dict in matrixbox.items():
-    # Only update cells which still exist
+    # after constructing the cell-range we can edit all existing cells
     if not matrix_context.hasInRange(*cell_index_tuple, **kd):
       return "Cell %s does not exist" % str(cell_index_tuple)
 
@@ -204,7 +207,7 @@ MARKER = []  # placeholder for an empty value
 message = Base_translateString("Data updated.")
 
 try:
-  # extract all listbox's object form fields from the request and `edit` the object
+  # Extract all form fields from the request and call `edit` on them
   for field in form.get_fields():
     # Dispatch field either to `edit_kwargs` (in case of simple fields) or to `encapsulated_editor_list` in case of editors
     field_name = field.id if not field.has_value('alternate_name') else (field.get_value('alternate_name') or field.id)
@@ -217,6 +220,7 @@ try:
       edit_kwargs[field_name[len(field_prefix):]] = field_value if field_value != '' else None
 
     ## XXX We need to find a way not to use meta_type.
+    # Kato: can be done simply by implementing 'Editors' for fields which are here
     field_meta_type = field.meta_type
     if field_meta_type == 'ProxyField':
       field_meta_type = field.getRecursiveTemplateField().meta_type
@@ -235,6 +239,7 @@ try:
   context.edit(REQUEST=request, edit_order=edit_order, **edit_kwargs)
   for encapsulated_editor in encapsulated_editor_list:
     encapsulated_editor.edit(context)
+
 except ActivityPendingError as e:
   message = Base_translateString(str(e))
 

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

-"""
-  This script validates a form to the current REQUEST,
-  processes the REQUEST to extract form data and editors,
-  then updates the current context with the form data
-  by calling edit on it or by invoking editors.
+"""Handle form - REQUEST interaction.
+
+-  Validate a form to the current REQUEST
+-  Extract form data and editors from REQUEST,
+-  Update current context with form data by calling edit or invoking editors
+
+:param silent: int (0|1) means that the edit action is not invoked by a form
+               submit but rather by an internal code thus the return value
+               contains as much usefull info as possible
 
   TODO: split the generic form validation logic
   from the context update logic
@@ -37,7 +41,7 @@ edit_order = form.edit_order
 try:
   # Validate
   form.validate_all_to_request(request, key_prefix=key_prefix)
-except FormValidationError, validation_errors:
+except FormValidationError as validation_errors:
   # Pack errors into the request
   field_errors = form.ErrorFields(validation_errors)
   request.set('field_errors', field_errors)
@@ -48,200 +52,194 @@ except FormValidationError, validation_errors:
       value = request.get(field_id)
       if callable(value):
         value(request)
-  if silent_mode: return form(request), 'form'
+  if silent_mode:
+    return form(request), 'form'
   return form(request)
 
 def editListBox(listbox_field, listbox):
-  """ Function called to edit a listbox
-  """
-  if listbox is not None:
-    gv = {}
-    if listbox_field.has_value('global_attributes'):
-      hidden_attributes = map(lambda x:x[0], listbox_field.get_value('global_attributes'))
-      for k in hidden_attributes:
-        gv[k] = getattr(request, k, None)
-    for url, v in listbox.items():
-      v.update(gv)
-      # Form: '' -> ERP5: None
-      encapsulated_editor_list = []
-      cleaned_v = {}
-      for key, value in v.items():
-        if hasattr(value, 'edit'):
-          encapsulated_editor_list.append(value)
-        else:
-          if value == '':        
-            value = None
-          cleaned_v[key] = value
-
-      if cleaned_v:
-        if listbox_edit is None:
-          obj = context.restrictedTraverse(url)
-          obj.edit(edit_order=edit_order, **cleaned_v)
-        else:
-          listbox_edit(url, edit_order, cleaned_v)
-
-      for encapsulated_editor in encapsulated_editor_list:
-        encapsulated_editor.edit(obj)
+  """Go through every item in the listbox and call its `edit` with modified values."""
+  if listbox is None:
+    return
+
+  # extract hidden (global) attributes from request to be used in listbox's update
+  global_attr = {hidden_key: getattr(request, hidden_key, None)
+                 for hidden_key, _ in listbox_field.get_value('global_attributes')} \
+                if listbox_field.has_value('global_attributes') \
+                else {}
+
+  for item_url, item_value in listbox.items():
+    item_value.update(global_attr)
+    # Form: '' -> ERP5: None
+    editor_list = []
+    value_dict = {}
+    for key, value in item_value.items():
+      # for every value decide whether it is an attribute or an editor
+      if hasattr(value, 'edit'):
+        editor_list.append(value)
+      else:
+        value_dict[key] = value if value != '' else None
+
+    if value_dict:
+      if listbox_edit is None:
+        obj = context.restrictedTraverse(item_url)
+        obj.edit(edit_order=edit_order, **value_dict)
+        for editor in editor_list:
+          editor.edit(obj)
+      else:
+        listbox_edit(item_url, edit_order, value_dict)
 
 def editMatrixBox(matrixbox_field, matrixbox):
-  """ Function called to edit a Matrix box
+  """Go through every field in matrix and call edit on it.
+  
+  Most of the code is just a copy&paste from ERP5Form/MatrixBox:render and 
+  should be in (non-yet-existing) MatrixBoxEditor instead of here (XXX TODO).
   """
-  if matrixbox is not None:
-    cell_base_id = matrixbox_field.get_value('cell_base_id')
-    portal_type = matrixbox_field.get_value('cell_portal_type')
-    getter_method = matrixbox_field.get_value('getter_method')
-    if getter_method not in (None, ''):
-      matrix_context = getattr(context,getter_method)()
+  if matrixbox is None:
+    return
+
+  cell_base_id = matrixbox_field.get_value('cell_base_id')
+  portal_type = matrixbox_field.get_value('cell_portal_type')
+  getter_method = matrixbox_field.get_value('getter_method')
+  if getter_method not in (None, ''):
+    matrix_context = getattr(context,getter_method)()
+  else:
+    matrix_context = context
+
+  if matrix_context is None:
+    return
+
+  kd = {}
+  kd['portal_type'] = portal_type
+  kd['base_id'] = cell_base_id
+  gv = {}
+  if matrixbox_field.has_value('global_attributes'):
+    hidden_attributes = [x[0] for x in matrixbox_field.get_value('global_attributes')]
+    for k in hidden_attributes:
+      gv[k] = getattr(request, k, None)
+  if matrixbox_field.get_value('update_cell_range'):
+    as_cell_range_script_id = matrixbox_field.get_value(
+            'as_cell_range_script_id')
+    lines = []
+    columns = []
+    tabs = []
+    extra_dimension_list_list = []
+    if as_cell_range_script_id:
+      cell_range = getattr(matrix_context,
+          as_cell_range_script_id)(matrixbox=True, base_id=cell_base_id)
+      if len(cell_range) == 1:
+        lines, = cell_range
+      elif len(cell_range) == 2:
+        lines, columns = cell_range
+      elif len(cell_range) == 3:
+        lines, columns, tabs = cell_range
+      elif len(cell_range) > 3:
+        lines = cell_range[0]
+        columns = cell_range[1]
+        tabs = cell_range[2]
+        extra_dimension_list_list = cell_range[3:]
     else:
-      matrix_context = context
-    if matrix_context is not None:
-      kd = {}
-      kd['portal_type'] = portal_type
-      kd['base_id'] = cell_base_id
-      gv = {}
-      if matrixbox_field.has_value('global_attributes'):
-        hidden_attributes = [x[0] for x in matrixbox_field.get_value('global_attributes')]
-        for k in hidden_attributes:
-          gv[k] = getattr(request, k, None)
-      if matrixbox_field.get_value('update_cell_range'):
-        as_cell_range_script_id = matrixbox_field.get_value(
-                'as_cell_range_script_id')
-        lines = []
-        columns = []
-        tabs = []
-        extra_dimension_list_list = []
-        if as_cell_range_script_id:
-          cell_range = getattr(matrix_context,
-              as_cell_range_script_id)(matrixbox=True, base_id=cell_base_id)
-          if len(cell_range) == 1:
-            lines, = cell_range
-          elif len(cell_range) == 2:
-            lines, columns = cell_range
-          elif len(cell_range) == 3:
-            lines, columns, tabs = cell_range
-          elif len(cell_range) > 3:
-            lines = cell_range[0]
-            columns = cell_range[1]
-            tabs = cell_range[2]
-            extra_dimension_list_list = cell_range[3:]
-        else:
-          lines = matrixbox_field.get_value('lines')
-          columns = matrixbox_field.get_value('columns')
-          tabs = matrixbox_field.get_value('tabs')
-
-        column_ids = map(lambda x: x[0], columns)
-        line_ids = map(lambda x: x[0], lines)
-        tab_ids = map(lambda x: x[0], tabs)
-        extra_dimension_category_list_list = [[category for category, label in dimension_list] for dimension_list in extra_dimension_list_list]
-
-        # There are 3 cases
-        # Case 1: we do 1 dimensional matrix
-        # Case 2: we do 2 dimensional matrix
-        # Case 3: we do 2 dimensional matrix + tabs
-        # Case 4: we do 2 dimensional matrix + tabs + extra
-        cell_range = matrix_context.getCellRange(base_id = cell_base_id)
-        if (len(column_ids) == 0) or (column_ids[0] is None):
-          matrixbox_cell_range = [line_ids]
-          if cell_range != matrixbox_cell_range:
-            matrix_context.setCellRange(line_ids, base_id=cell_base_id)
-
-        elif (len(tab_ids) == 0) or (tab_ids[0] is None):
-          matrixbox_cell_range = [line_ids, column_ids]
-          if cell_range != matrixbox_cell_range:
-            matrix_context.setCellRange(line_ids, column_ids, base_id=cell_base_id)
-
-        else:
-          matrixbox_cell_range = [line_ids, column_ids, tab_ids]
-          if extra_dimension_category_list_list:
-            matrixbox_cell_range = matrixbox_cell_range + extra_dimension_category_list_list
-          if cell_range != matrixbox_cell_range:
-            matrix_context.setCellRange(base_id=cell_base_id, *matrixbox_cell_range)
-
-      for k,v in matrixbox.items():
-        # Only update cells which still exist
-        if matrix_context.hasInRange(*k, **kd):
-          c = matrix_context.newCell(*k, **kd)
-          if c is not None:
-            c.edit(edit_order=edit_order, **gv)  # First update globals which include the def. of property_list
-            if v.has_key('variated_property'):
-              # For Variated Properties
-              value = v['variated_property']
-              del v['variated_property']
-              if gv.has_key('mapped_value_property_list'):
-                # Change the property which is defined by the
-                # first element of mapped_value_property_list
-                # XXX May require some changes with Sets
-                key = gv['mapped_value_property_list'][0]
-                v[key] = value
-            # Form: '' -> ERP5: None
-            cleaned_v = v.copy()
-            for key, value in cleaned_v.items():
-              if value == '':
-                cleaned_v[key] = None
-            c.edit(edit_order=edit_order, **cleaned_v) # and update the cell specific values
-          else:
-            return "Could not create cell %s" % str(k)
-        else:
-          return "Cell %s does not exist" % str(k)
-
-field_prefix_len = len(field_prefix)
-
-def parseField(f):
-  """
-   Parse given form field, to put them in
-   kw or in encapsulated_editor_list
-  """
-  k = f.id
-  if f.has_value('alternate_name'):
-    k = f.get_value('alternate_name') or f.id
-  v = getattr(request, k, MARKER)
-  if hasattr(v, 'edit'):
-    # This is an encapsulated editor
-    # call it
-    encapsulated_editor_list.append(v)
-  elif v is not MARKER:
-    if k.startswith(field_prefix):
-      # We only take into account
-      # the object attributes
-      k = k[field_prefix_len:]
-      # Form: '' -> ERP5: None
-      if v == '':
-        v = None
-      kw[k] = v
-
-# Some initilizations
-kw = {}
-encapsulated_editor_list = []
-MARKER = []
+      lines = matrixbox_field.get_value('lines')
+      columns = matrixbox_field.get_value('columns')
+      tabs = matrixbox_field.get_value('tabs')
+
+    column_ids = map(lambda x: x[0], columns)
+    line_ids = map(lambda x: x[0], lines)
+    tab_ids = map(lambda x: x[0], tabs)
+    extra_dimension_category_list_list = [[category for category, label in dimension_list] for dimension_list in extra_dimension_list_list]
+
+    # There are 3 cases
+    # Case 1: we do 1 dimensional matrix
+    # Case 2: we do 2 dimensional matrix
+    # Case 3: we do 2 dimensional matrix + tabs
+    # Case 4: we do 2 dimensional matrix + tabs + extra
+    cell_range = matrix_context.getCellRange(base_id = cell_base_id)
+    if (len(column_ids) == 0) or (column_ids[0] is None):
+      matrixbox_cell_range = [line_ids]
+      if cell_range != matrixbox_cell_range:
+        matrix_context.setCellRange(line_ids, base_id=cell_base_id)
+
+    elif (len(tab_ids) == 0) or (tab_ids[0] is None):
+      matrixbox_cell_range = [line_ids, column_ids]
+      if cell_range != matrixbox_cell_range:
+        matrix_context.setCellRange(line_ids, column_ids, base_id=cell_base_id)
+
+    else:
+      matrixbox_cell_range = [line_ids, column_ids, tab_ids]
+      if extra_dimension_category_list_list:
+        matrixbox_cell_range = matrixbox_cell_range + extra_dimension_category_list_list
+      if cell_range != matrixbox_cell_range:
+        matrix_context.setCellRange(base_id=cell_base_id, *matrixbox_cell_range)
+
+  for cell_index_tuple, cell_value_dict in matrixbox.items():
+    # after constructing the cell-range we can edit all existing cells
+    if not matrix_context.hasInRange(*cell_index_tuple, **kd):
+      return "Cell %s does not exist" % str(cell_index_tuple)
+
+    cell = matrix_context.newCell(*cell_index_tuple, **kd)
+    if cell is None:
+      return "Could not create cell %s" % str(cell_index_tuple)
+
+    cell.edit(edit_order=edit_order, **gv)  # First update globals which include the def. of property_list
+    if cell_value_dict.has_key('variated_property'):
+      # For Variated Properties
+      value = cell_value_dict['variated_property']
+      del cell_value_dict['variated_property']
+      if gv.has_key('mapped_value_property_list'):
+        # Change the property which is defined by the
+        # first element of mapped_value_property_list
+        # XXX Kato: What? Why?
+        # XXX May require some changes with Sets
+        key = gv['mapped_value_property_list'][0]
+        cell_value_dict[key] = value
+    # Form: '' -> ERP5: None
+    cleaned_v = cell_value_dict.copy()
+    for key, value in cleaned_v.items():
+      if value == '':
+        cleaned_v[key] = None
+    cell.edit(edit_order=edit_order, **cleaned_v) # and update the cell specific values
+
+
+edit_kwargs = {}  # keyword arguments for `edit` function on context
+encapsulated_editor_list = []  # editors placed inside REQUEST object
+MARKER = []  # placeholder for an empty value
 message = Base_translateString("Data updated.")
 
+
 try:
-  # We process all the field in form and
-  # we check if they are in the request,
-  # then we edit them
+  # Extract all form fields from the request and call `edit` on them
   for field in form.get_fields():
-    parseField(field)
+    # Dispatch field either to `edit_kwargs` (in case of simple fields) or to `encapsulated_editor_list` in case of editors
+    field_name = field.id if not field.has_value('alternate_name') else (field.get_value('alternate_name') or field.id)
+    field_value = getattr(request, field_name, MARKER)
+    if hasattr(field_value, 'edit'):
+      # field is an encapsulated editor; call it later
+      encapsulated_editor_list.append(field_value)
+    elif field_value is not MARKER and field_name.startswith(field_prefix):
+      # object own attribute (fix value Form: '' -> ERP5: None)
+      edit_kwargs[field_name[len(field_prefix):]] = field_value if field_value != '' else None
 
     ## XXX We need to find a way not to use meta_type.
     field_meta_type = field.meta_type
     if field_meta_type == 'ProxyField':
       field_meta_type = field.getRecursiveTemplateField().meta_type
-
+    # Kato: following fields should implement 'Editors' so we can delete those lines
     if(field_meta_type == 'ListBox'):
       editListBox(field, request.get(field.id))
-    elif(field_meta_type == 'MatrixBox'):
+    if(field_meta_type == 'MatrixBox'):
       editMatrixBox(field, request.get(field.id))
 
   # Return parsed values 
-  if silent_mode: return (kw, encapsulated_editor_list), 'edit'
+  if silent_mode:
+    return (edit_kwargs, encapsulated_editor_list), 'edit'
 
   # Maybe we should build a list of objects we need
   # Update basic attributes
-  context.edit(REQUEST=request, edit_order=edit_order, **kw)
+  context.edit(REQUEST=request, edit_order=edit_order, **edit_kwargs)
   for encapsulated_editor in encapsulated_editor_list:
     encapsulated_editor.edit(context)
-except ActivityPendingError,e:
-  message = Base_translateString("%s" % e)
+
+except ActivityPendingError as e:
+  message = Base_translateString(str(e))
 
 if message_only:
   return message
@@ -283,5 +281,6 @@ else:
 
 result = request['RESPONSE'].redirect(redirect_url) 
 
-if silent_mode: return result, 'redirect'
+if silent_mode:
+  return result, 'redirect'
 return result