Merge lp:~elbati/openobject-addons/7.0_fix_action_rule_processing into lp:openobject-addons/7.0

[Lorenzo Battistini]

[Yannick Vaucher @ Camptocamp (yvaucher-c2c)]

I admire the way you did use the ternary operator but wouldn't it be more readable that way?

if last_run and (last_run <= action_dt < now)
   or not last_run and (action_dt < now):

I think using it in a comparison make things a bit confusing

Or maybe just add indentation to the second line to keep clear it is still the comparison.

[Leonardo Pistone (lepistone)]

Yannick,

There is some explanation of why I suggest to use the ternary operator on the bug linked to this MP:
https://bugs.launchpad.net/openobject-addons/+bug/1190592

In short, the and-or trick doesn't behave as expected when the second condition is false, and that is what happens here. The inline if-else is unaffected by that.

review: Approve

[Lorenzo Battistini (elbati)]

review: Approve

[Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c)]

I'd really like if a more readable / maintainable formulation was used.

I made another proposal with this in mind (https://code.launchpad.net/~camptocamp/openobject-addons/7.0-fix_action_rule_processing_lp1190592-afe/+merge/209077).

review: Abstain

[Leonardo Pistone (lepistone)]

Alexandre, no problem for me to take your version.

I change my review to abstain.

review: Abstain

[Lorenzo Battistini (elbati)]

review: Abstain

Unmerged revisions

9193. By Lorenzo Battistini on 2013-06-13

[FIX] rule processing time

=== modified file 'base_action_rule/base_action_rule.py'
--- base_action_rule/base_action_rule.py	2013-05-29 10:42:02 +0000
+++ base_action_rule/base_action_rule.py	2013-06-13 13:07:42 +0000
@@ -256,7 +256,8 @@
                 if not record_dt:
                     continue
                 action_dt = get_datetime(record_dt) + delay
-                if last_run and (last_run <= action_dt < now) or (action_dt < now):
+                if ((last_run <= action_dt < now) if last_run
+                    else action_dt < now):
                     try:
                         self._process(cr, uid, action, [record.id], context=context)
                     except Exception: