Commit 123aa85e authored by Jean-Paul Smets's avatar Jean-Paul Smets

Comments to highlight code weaknesses.

git-svn-id: https://svn.erp5.org/repos/public/erp5/trunk@31328 20353a03-c40f-0410-a6d1-a30d3c3de9de
parent e96c1b8a
...@@ -87,9 +87,15 @@ class RuleMixin: ...@@ -87,9 +87,15 @@ class RuleMixin:
def test(self, *args, **kw): def test(self, *args, **kw):
""" """
If no test method is defined, return False, to prevent infinite loop If no test method is defined, return False, to prevent infinite loop
XXX-JPS - I do not understand why
""" """
if not self.getTestMethodId(): if not self.getTestMethodId():
return False return False # XXX-JPS - if people are stupid are enough not to configfure predicates,
# it is not our role to be clever for them
# Rules have a workflow - make sure applicable rule system works
# if you wish, add a test here on workflow state to prevent using
# rules which are no longer applicable
return Predicate.test(self, *args, **kw) return Predicate.test(self, *args, **kw)
def expand(self, applied_rule, **kw): def expand(self, applied_rule, **kw):
...@@ -110,7 +116,7 @@ class RuleMixin: ...@@ -110,7 +116,7 @@ class RuleMixin:
for movement in applied_rule.getMovementList(): for movement in applied_rule.getMovementList():
movement.expand(**kw) movement.expand(**kw)
# Implementation of IDivergenceController # Implementation of IDivergenceController # XXX-JPS move to IDivergenceController only mixin for
security.declareProtected( Permissions.AccessContentsInformation, security.declareProtected( Permissions.AccessContentsInformation,
'isDivergent') 'isDivergent')
def isDivergent(self, movement, ignore_list=[]): def isDivergent(self, movement, ignore_list=[]):
...@@ -170,7 +176,7 @@ class RuleMixin: ...@@ -170,7 +176,7 @@ class RuleMixin:
# of the form (prevision_movement_dict, list of decision_movement) # of the form (prevision_movement_dict, list of decision_movement)
prevision_to_decision_map = [] prevision_to_decision_map = []
# XXX First we try to match by 'order' value if possible. # XXX First we try to match by 'order' value if possible. # XXX-JPS implement as DivergenceTester, no ad-hoc here
matched_prevision_list = [] matched_prevision_list = []
matched_decision_list = [] matched_decision_list = []
prevision_order_dict = dict( prevision_order_dict = dict(
...@@ -243,7 +249,7 @@ class RuleMixin: ...@@ -243,7 +249,7 @@ class RuleMixin:
map_list = [] map_list = []
for decision_movement in decision_movement_dict.get(tester_key, ()): for decision_movement in decision_movement_dict.get(tester_key, ()):
if _compare(tester_list, prevision_movement, decision_movement): if _compare(tester_list, prevision_movement, decision_movement):
# XXX is it OK to have more than 2 decision_movements? # XXX is it OK to have more than 2 decision_movements? # XXX-JPS - I think yes
map_list.append(decision_movement) map_list.append(decision_movement)
prevision_to_decision_map.append((prevision_movement, map_list)) prevision_to_decision_map.append((prevision_movement, map_list))
...@@ -311,7 +317,7 @@ class RuleMixin: ...@@ -311,7 +317,7 @@ class RuleMixin:
quantity divergence testers quantity divergence testers
""" """
if exclude_quantity: if exclude_quantity:
return filter(lambda x:x.isTestingProvider() and \ return filter(lambda x:x.isTestingProvider() and \ # XXX-JPS name too generic - is it for divergence ? for somehting else ?
x.getTestedProperty() != 'quantity', self.objectValues( x.getTestedProperty() != 'quantity', self.objectValues(
portal_type=self.getPortalDivergenceTesterTypeList())) portal_type=self.getPortalDivergenceTesterTypeList()))
else: else:
...@@ -357,6 +363,14 @@ class RuleMixin: ...@@ -357,6 +363,14 @@ class RuleMixin:
Compares a prevision_movement to decision_movement_list which Compares a prevision_movement to decision_movement_list which
are part of the matching group and updates movement_collection_diff are part of the matching group and updates movement_collection_diff
accordingly accordingly
NOTE: this method API implicitely considers that each group of matching
movements has 1 prevision_movement (aggregated) for N decision_movement
It implies that prevision_movement are "more" aggregated than
decision_movement.
TODO:
- is this asumption appropriate ?
""" """
# Sample implementation - but it actually looks very generic # Sample implementation - but it actually looks very generic
# Case 1: movements which are not needed # Case 1: movements which are not needed
...@@ -425,8 +439,9 @@ class RuleMixin: ...@@ -425,8 +439,9 @@ class RuleMixin:
# Not Frozen can be updated # Not Frozen can be updated
kw = {} kw = {}
for tester in divergence_tester_list: for tester in divergence_tester_list:
if tester.compare(prevision_movement, decision_movement): if tester.compare(prevision_movement, decision_movement):
kw.update(tester.getUpdatablePropertyDict(prevision_movement, decision_movement)) kw.update(tester.getUpdatablePropertyDict(prevision_movement, decision_movement))
# XXX-JPS - there is a risk here that quanity is wrongly updated
if kw: if kw:
movement_collection_diff.addUpdatableMovement(decision_movement, kw) movement_collection_diff.addUpdatableMovement(decision_movement, kw)
# Second, we calculate if the total quantity is the same on both sides # Second, we calculate if the total quantity is the same on both sides
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment