builder: ignore property updates by movement groups if no document can be updated

This fixes how the builder processes the result of IMovementGroup.test.
Testing deliveries to find one that can be updated should have no impact
on properties if the builder ends up creating a delivery.

bt5/erp5_base/DocumentTemplateItem/portal_components/document.erp5.OrderMovementGroup.py

@@ -43,8 +43,8 @@ class OrderMovementGroup(MovementGroup):
 
   def test(self, movement, property_dict, **kw):
     if set(property_dict['causality_list']).issubset(movement.getCausalityList()):
-      property_dict['causality_list'] = movement.getCausalityList()
-      return True, property_dict
+      return True, dict(property_dict,
+        causality_list=movement.getCausalityList())
     else:
       return False, property_dict
 

bt5/erp5_base/MixinTemplateItem/portal_components/mixin.erp5.BuilderMixin.py

@@ -323,18 +323,6 @@ class BuilderMixin(XMLObject, Amount, Predicate):
     root_group_node.append(movement_list)
     return root_group_node
 
-  def _test(self, instance, movement_group_node_list,
-                    divergence_list):
-    result = True
-    new_property_dict_list = []
-    for movement_group_node in movement_group_node_list:
-      tmp_result, tmp_property_dict = movement_group_node.test(
-        instance, divergence_list)
-      if not tmp_result:
-        result = tmp_result
-      new_property_dict_list.append(tmp_property_dict)
-    return result, new_property_dict_list
-
   @staticmethod
   def _getSortedPropertyDict(property_dict_list):
     # Sort the edit keywords according to the order of their movement
@@ -357,7 +345,6 @@ class BuilderMixin(XMLObject, Amount, Predicate):
 
   def _findUpdatableObject(self, instance_list, movement_group_node_list,
                            divergence_list):
-    instance = None
     if instance_list:
       # we want to check the original delivery first.
       # so sort instance_list by that current is exists or not.
@@ -371,16 +358,19 @@ class BuilderMixin(XMLObject, Amount, Predicate):
           current = current.getParentValue()
       except AttributeError:
         pass
-      for instance_to_update in instance_list:
-        result, property_dict_list = self._test(
-          instance_to_update, movement_group_node_list, divergence_list)
-        if result:
-          instance = instance_to_update
-          break
-    else:
-      property_dict_list = [movement_group_node.getGroupEditDict()
-                            for movement_group_node in movement_group_node_list]
-    return instance, self._getSortedPropertyDict(property_dict_list)
+      for instance in instance_list:
+        property_dict_list = []
+        for movement_group_node in movement_group_node_list:
+          result, property_dict = movement_group_node.test(
+            instance, divergence_list)
+          if not result:
+            break
+          property_dict_list.append(property_dict)
+        else:
+          return instance, self._getSortedPropertyDict(property_dict_list)
+    return None, self._getSortedPropertyDict(
+      movement_group_node.getGroupEditDict()
+      for movement_group_node in movement_group_node_list)
 
   security.declarePrivate('buildDeliveryList')
   @UnrestrictedMethod

product/ERP5/bootstrap/erp5_core/ModuleComponentTemplateItem/portal_components/module.erp5.MovementGroup.py

@@ -164,8 +164,15 @@ class MovementGroupNode:
           if not property_list:
             return True, {}
         # else update anyway (eg. CausalityAssignmentMovementGroup etc.)
-      return self._movement_group.test(movement, self._property_dict,
-                                               property_list=property_list)
+      result, property_dict = self._movement_group.test(
+        movement, self._property_dict, property_list=property_list)
+      # The following check is partial because it does not check mutable values
+      # recursively.
+      if property_dict is self._property_dict != property_dict:
+        raise ValueError(
+          "Movement Group must not modify the passed 'property_dict':"
+          " copy it, deeply if necessary, before editing properties")
+      return result, property_dict
     else:
       return True, {}