Merge lp:~acysos-team/banking-addons/banking-addons-spain-party-identifier into lp:banking-addons

Proposed by Ignacio Ibeas (www.acysos.com) on 2014-02-13
Status: Merged
Approved by: Holger Brunn (Therp) on 2014-02-28
Approved revision: 227
Merged at revision: 229
Proposed branch: lp:~acysos-team/banking-addons/banking-addons-spain-party-identifier
Merge into: lp:banking-addons
Diff against target: 18 lines (+6/-2)
1 file modified
account_banking_pain_base/company.py (+6/-2)
To merge this branch: bzr merge lp:~acysos-team/banking-addons/banking-addons-spain-party-identifier
Reviewer Review Type Date Requested Status
Holger Brunn (Therp) code review Approve on 2014-02-28
Guewen Baconnier @ Camptocamp Approve on 2014-02-28
Pedro Manuel Baeza code review 2014-02-13 Approve on 2014-02-24
Review via email: mp+206246@code.launchpad.net

Description of the change

Hello,

Add the party identifier support for Spain.

Greetings

To post a comment you must log in.
Pedro Manuel Baeza (pedro.baeza) wrote :

As you have added another condition, and to accomodate other countries, code can be refactored to:

if company:
    country_code = company_vat[0:2].upper()
    if country_code == 'BE':
        party_identifier = company_vat[2:].replace(' ', '')
    elif country_code == 'ES':
        party_identifier = company.sepa_creditor_identifier

Regards.

review: Needs Fixing (code review)
Holger Brunn (Therp) (hbrunn) wrote :

yes, I think Pedro's right.

But I guess his first line should be

if company_vat:

though

review: Needs Fixing (code review)
Pedro Manuel Baeza (pedro.baeza) wrote :

Sorry for the typo mistake. It must be:

if company_vat:
    country_code = company_vat[0:2].upper()
    if country_code == 'BE':
        party_identifier = company_vat[2:].replace(' ', '')
    elif country_code == 'ES':
        party_identifier = company.sepa_creditor_identifier

Regards.

227. By Ignacio Ibeas (www.acysos.com) on 2014-02-24

[FIX] account_banling_pain_base: refactored

Hello,

I have made the change.

Greetings

Pedro Manuel Baeza (pedro.baeza) wrote :

LGTM

Regards.

review: Approve (code review)

LGTM
Thanks

review: Approve
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_banking_pain_base/company.py'
2--- account_banking_pain_base/company.py 2013-12-24 00:01:04 +0000
3+++ account_banking_pain_base/company.py 2014-02-24 17:57:22 +0000
4@@ -45,8 +45,12 @@
5 company = self.browse(cr, uid, company_id, context=context)
6 company_vat = company.vat
7 party_identifier = False
8- if company_vat and company_vat[0:2].upper() in ['BE']:
9- party_identifier = company_vat[2:].replace(' ', '')
10+ if company_vat:
11+ country_code = company_vat[0:2].upper()
12+ if country_code == 'BE':
13+ party_identifier = company_vat[2:].replace(' ', '')
14+ elif country_code == 'ES':
15+ party_identifier = company.sepa_creditor_identifier
16 return party_identifier
17
18 def _initiating_party_issuer_default(self, cr, uid, context=None):

Subscribers

People subscribed via source and target branches

to status/vote changes: