Merge lp:~camptocamp/openerp-fiscal-rules/7.0-wrong-onchange-1255918-revert-hack into lp:openerp-fiscal-rules

Proposed by Guewen Baconnier @ Camptocamp
Status: Merged
Approved by: Yannick Vaucher @ Camptocamp
Approved revision: 67
Merged at revision: 66
Proposed branch: lp:~camptocamp/openerp-fiscal-rules/7.0-wrong-onchange-1255918-revert-hack
Merge into: lp:openerp-fiscal-rules
Diff against target: 20 lines (+2/-3)
1 file modified
account_fiscal_position_rule_sale/sale.py (+2/-3)
To merge this branch: bzr merge lp:~camptocamp/openerp-fiscal-rules/7.0-wrong-onchange-1255918-revert-hack
Reviewer Review Type Date Requested Status
Sandy Carter (http://www.savoirfairelinux.com) code review Approve
Raphaël Valyi - http://www.akretion.com Approve
Yannick Vaucher @ Camptocamp code review, no tests Approve
Review via email: mp+199144@code.launchpad.net

Description of the change

Follows https://code.launchpad.net/~camptocamp/openerp-fiscal-rules/7.0-wrong-onchange-1255918/+merge/197376

Since Raphaël stated that the context being mutated in kwargs is not necessary [0], I propose to revert the change.

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

[0] https://code.launchpad.net/~camptocamp/openerp-fiscal-rules/7.0-wrong-onchange-1255918/+merge/197376/comments/458163

To post a comment you must log in.
Revision history for this message
Yannick Vaucher @ Camptocamp (yvaucher-c2c) wrote :

Unless we make the hack uglier it seems necessary to do this revert.

Looking at https://code.launchpad.net/~camptocamp/openerp-fiscal-rules/7.0-wrong-onchange-1255918/+merge/197376/comments/458163 we can suspect it was approved before the change by Raphael

review: Approve (code review, no tests)
Revision history for this message
Yannick Vaucher @ Camptocamp (yvaucher-c2c) wrote :

disambiguation:
I meant
-> we can suspect it was approved by Raphael, before the change

Revision history for this message
Raphaël Valyi - http://www.akretion.com (rvalyi) wrote :

thanks guys.

In any case, for our localization we will try to wrap the extra params in the context more cleanly using:
https://code.launchpad.net/~akretion-team/server-env-tools/web-context-tunnel/+merge/198599
(You are welcome to review BTW)

And we will unwrap our extra params ourselves from the context in our localization override.

Thanks.

review: Approve
Revision history for this message
Sandy Carter (http://www.savoirfairelinux.com) (sandy-carter) wrote :

LGTM

review: Approve (code review)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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-16 15:26:04 +0000
@@ -53,14 +53,13 @@
53 return result53 return result
5454
55 values = result['value']55 values = result['value']
56 kwargs = context.copy()56 kwargs = {
57 kwargs.update({
58 'shop_id': context['shop_id'],57 'shop_id': context['shop_id'],
59 'partner_id': partner_id,58 'partner_id': partner_id,
60 'partner_invoice_id': values.get('partner_invoice_id', False),59 'partner_invoice_id': values.get('partner_invoice_id', False),
61 'partner_shipping_id': values.get('partner_shipping_id', False),60 'partner_shipping_id': values.get('partner_shipping_id', False),
62 'context': context61 'context': context
63 })62 }
64 return self._fiscal_position_map(cr, uid, result, **kwargs)63 return self._fiscal_position_map(cr, uid, result, **kwargs)
6564
66 def onchange_address_id(self, cr, uid, ids, partner_invoice_id,65 def onchange_address_id(self, cr, uid, ids, partner_invoice_id,

Subscribers

People subscribed via source and target branches