Merge lp:~savoirfairelinux-openerp/openerp-fiscal-rules/7.0-fix-bug-1265067 into lp:openerp-fiscal-rules

[Maxime Chambreuil (http://www.savoirfairelinux.com)]

[MRG] Fix _fiscal_position_map() got multiple values for keyword argument 'uid'

[FIX] _fiscal_position_map() got multiple values for keyword argument 'uid'

[Raphaël Valyi - http://www.akretion.com (rvalyi)]

Hello Maxime,

I've hit the bug too. But I suggest we apply this MP instead which fixes it also:
https://code.launchpad.net/~camptocamp/openerp-fiscal-rules/7.0-wrong-onchange-1255918-revert-hack/+merge/199144
but in a cleaner way.

In fact, after much investigation, we concluded that the way we were propagating extra arguments in the context, was specific to our localization and wasn't quite clean (it was brutaly overriding the context instead of etending it).
In the future, we will rely on web_context_tunnel in our localization to pass more extra params to these methods and we will get them back from the context at the rght place in some specific localization override. Still I advise you have a look to hit, because I propose refactoring a bit the fiscal rules to ensure we can leave on_change methods with the original signature, that play it nicer with the other modules
https://code.launchpad.net/~akretion-team/server-env-tools/web-context-tunnel/+merge/198599

Hope this helps

review: Disapprove

[Maxime Chambreuil (http://www.savoirfairelinux.com) (max3903)]

Hello Raphaël,

I have no problem with this MP being rejected as long as your solution fixes the bug, which it doesn't seem to do:

"Note that the current code is failing is some conditions, when a 'uid' key is present in the context for instance."

[Raphaël Valyi - http://www.akretion.com (rvalyi)]

Hello Maxime,

are you sure?
I had the same bug as you reported but when I merged https://code.launchpad.net/~camptocamp/openerp-fiscal-rules/7.0-wrong-onchange-1255918-revert-hack/+merge/199144 it disappeared. Look how kwargs is defined in the MP, there is no way you'll have a uid key left.
As for web_context_tunnel, I was commenting only about how it's useful to inject extra args in the context, but in the right way (instead of just replacing the context as we were doing before), not sure you understand that, in any case what fixes the bug is this MP https://code.launchpad.net/~camptocamp/openerp-fiscal-rules/7.0-wrong-onchange-1255918-revert-hack/+merge/199144 not web_context_tunnel itself.

review: Needs Information

[Raphaël Valyi - http://www.akretion.com (rvalyi)]

Hello Maxime,

so I merged https://code.launchpad.net/~camptocamp/openerp-fiscal-rules/7.0-wrong-onchange-1255918-revert-hack/+merge/199144 and the bug doesn't occur anymore.

can we cancel this MP then?

review: Needs Information

Unmerged revisions

66. By Maxime Chambreuil (http://www.savoirfairelinux.com) on 2013-12-30

[FIX] _fiscal_position_map() got multiple values for keyword argument 'uid'

=== modified file 'account_fiscal_position_rule_sale/sale.py'
--- account_fiscal_position_rule_sale/sale.py	2013-12-04 07:52:33 +0000
+++ account_fiscal_position_rule_sale/sale.py	2013-12-30 22:14:10 +0000
@@ -54,6 +54,7 @@
 
         values = result['value']
         kwargs = context.copy()
+        del kwargs['uid']
         kwargs.update({
             'shop_id': context['shop_id'],
             'partner_id': partner_id,