Merge lp:~kinner-vachhani/openerp-fiscal-rules/fix-lp-1276519 into lp:openerp-fiscal-rules/6.1

Proposed by Kinner Vachhani
Status: Merged
Approved by: Yannick Vaucher @ Camptocamp
Approved revision: 53
Merged at revision: 52
Proposed branch: lp:~kinner-vachhani/openerp-fiscal-rules/fix-lp-1276519
Merge into: lp:openerp-fiscal-rules/6.1
Diff against target: 12 lines (+1/-1)
1 file modified
account_fiscal_position_rule/account_fiscal_position_rule.py (+1/-1)
To merge this branch: bzr merge lp:~kinner-vachhani/openerp-fiscal-rules/fix-lp-1276519
Reviewer Review Type Date Requested Status
Guewen Baconnier @ Camptocamp code review Approve
Yannick Vaucher @ Camptocamp code review, no test Approve
Pedro Manuel Baeza code review Approve
Review via email: mp+204878@code.launchpad.net

Description of the change

bug fix lp:1276519

To post a comment you must log in.
Revision history for this message
Pedro Manuel Baeza (pedro.baeza) wrote :

I don't think that this patch is the best solution. Maybe we have to deal with it because on the core, things are not done correctly, but if it is our fault in some other place, better to fix in this place instead making a workaround.

The thing is that the line:

partner_addr = partner.address_get(['invoice'])

equals to

partner_addr = self.pool.get('res.partner').address_get(cr, uid, [partner_id], ['invoice'], context=context)

The problem here is that address_get has not declared context as keyword argument (or not declared at all). If this is the case, we cannot do anything, except accept this change, but maybe it's one inherited method that change this signature.

Regards.

review: Needs Information
Revision history for this message
Kinner Vachhani (kinner-vachhani) wrote :

I checked base signature and it seems like its not taking context at all into consideration and that explains the error.

server/openerp/addons/base/res/res_partner.py:243: def address_get(self, cr, uid, ids, adr_pref=None):

In this case this patch should do the job.

Revision history for this message
Pedro Manuel Baeza (pedro.baeza) wrote :

Yeah, I see, so there's no other possibility.

I approve then the patch.

Regards.

review: Approve (code review)
Revision history for this message
Kinner Vachhani (kinner-vachhani) wrote :

> Yeah, I see, so there's no other possibility.
>
> I approve then the patch.
>
> Regards.

Thank you Pedro.

Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote :

Can you add a space after the commas please (pep8)? Thanks

review: Needs Fixing
53. By Kinner Vachhani

[PEP8] space added after comma

Revision history for this message
Kinner Vachhani (kinner-vachhani) wrote :

Hi Guewen,
Thanks for review and appreciated for your time.

> Can you add a space after the commas please (pep8)? Thanks
Space after comma added.

Revision history for this message
Yannick Vaucher @ Camptocamp (yvaucher-c2c) wrote :

LGTM thanks for the fix

review: Approve (code review, no test)
Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote :

Thanks!

review: Approve (code review)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'account_fiscal_position_rule/account_fiscal_position_rule.py'
2--- account_fiscal_position_rule/account_fiscal_position_rule.py 2012-10-29 13:41:30 +0000
3+++ account_fiscal_position_rule/account_fiscal_position_rule.py 2014-03-30 12:22:05 +0000
4@@ -130,7 +130,7 @@
5 # In picking case the invoice_id can be empty but we need a value
6 # I only see this case, maybe we can move this code in fiscal_stock_rule
7 else:
8- partner_addr = partner.address_get(['invoice'])
9+ partner_addr = self.pool.get('res.partner').address_get(cr, uid, [partner_id], ['invoice'])
10 addr_id = partner_addr['invoice'] and partner_addr['invoice'] or None
11 if addr_id:
12 addrs['invoice'] = obj_address.browse(cr, uid, addr_id, context=context)

Subscribers

People subscribed via source and target branches

to all changes: