banking-addons-team team mailing list archive
-
banking-addons-team team
-
Mailing list archive
-
Message #01482
[Merge] lp:~therp-nl/banking-addons/7.0-no_unnecessary_raise into lp:banking-addons
Stefan Rijnhart (Therp) has proposed merging lp:~therp-nl/banking-addons/7.0-no_unnecessary_raise into lp:banking-addons.
Requested reviews:
Banking Addons Core Editors (banking-addons-team)
For more details, see:
https://code.launchpad.net/~therp-nl/banking-addons/7.0-no_unnecessary_raise/+merge/205648
Legacy code raises in two instances:
- When the BBAN of a Turkish IBAN is requested
- When no online database lookup is implemented for a region
These exceptions were not caught properly in every case. Instead of adding additional try/except blocks, I'm proposing to do away with these exceptions and return False instead. Dealing with an empty value is obvious in all cases where these methods are called.
6.1 version here: https://code.launchpad.net/~therp-nl/banking-addons/6.1-no_unnecessary_raise/+merge/205647
--
https://code.launchpad.net/~therp-nl/banking-addons/7.0-no_unnecessary_raise/+merge/205648
Your team Banking Addons Core Editors is requested to review the proposed merge of lp:~therp-nl/banking-addons/7.0-no_unnecessary_raise into lp:banking-addons.
=== modified file 'account_banking/account_banking.py'
--- account_banking/account_banking.py 2013-10-31 07:33:46 +0000
+++ account_banking/account_banking.py 2014-02-10 20:23:41 +0000
@@ -681,14 +681,10 @@
if term[0].lower() == 'acc_number' and term[1] in ('=', '=='):
iban = sepa.IBAN(term[2])
if iban.valid:
- # Some countries can't convert to BBAN
- try:
- bban = iban.localized_BBAN
- # Prevent empty search filters
- if bban:
- extra_term = ('acc_number_domestic', term[1], bban)
- except:
- pass
+ bban = iban.localized_BBAN
+ # Prevent empty search filters
+ if bban:
+ extra_term = ('acc_number_domestic', term[1], bban)
if extra_term:
return ['|', term, extra_term]
return [term]
@@ -763,10 +759,7 @@
res[record.id] = False
else:
iban_acc = sepa.IBAN(record.acc_number)
- try:
- res[record.id] = iban_acc.localized_BBAN
- except NotImplementedError:
- res[record.id] = False
+ res[record.id] = iban_acc.localized_BBAN
return res
def onchange_acc_number(
@@ -854,43 +847,43 @@
cr, uid, country_ids[0], context=context)
values['country_id'] = country_ids[0]
if country and country.code in sepa.IBAN.countries:
- try:
- info = sepa.online.account_info(country.code, acc_number)
- if info:
- iban_acc = sepa.IBAN(info.iban)
- if iban_acc.valid:
- values['acc_number_domestic'] = iban_acc.localized_BBAN
- values['acc_number'] = unicode(iban_acc)
- values['state'] = 'iban'
- bank_id, country_id = get_or_create_bank(
- self.pool, cr, uid,
- info.bic or iban_acc.BIC_searchkey,
- name = info.bank
- )
- if country_id:
- values['country_id'] = country_id
- values['bank'] = bank_id or False
- if info.bic:
- values['bank_bic'] = info.bic
- else:
- info = None
- if info is None:
- result.update(warning(
+ info = sepa.online.account_info(country.code, acc_number)
+ if info:
+ iban_acc = sepa.IBAN(info.iban)
+ if iban_acc.valid:
+ values['acc_number_domestic'] = iban_acc.localized_BBAN
+ values['acc_number'] = unicode(iban_acc)
+ values['state'] = 'iban'
+ bank_id, country_id = get_or_create_bank(
+ self.pool, cr, uid,
+ info.bic or iban_acc.BIC_searchkey,
+ name = info.bank
+ )
+ if country_id:
+ values['country_id'] = country_id
+ values['bank'] = bank_id or False
+ if info.bic:
+ values['bank_bic'] = info.bic
+ else:
+ info = None
+ if info is None:
+ result.update(warning(
_('Invalid data'),
_('The account number appears to be invalid for %s')
% country.name
- ))
- except NotImplementedError:
+ ))
+ if info is False:
if country.code in sepa.IBAN.countries:
acc_number_fmt = sepa.BBAN(acc_number, country.code)
if acc_number_fmt.valid:
values['acc_number_domestic'] = str(acc_number_fmt)
else:
result.update(warning(
- _('Invalid format'),
- _('The account number has the wrong format for %s')
- % country.name
- ))
+ _('Invalid format'),
+ _('The account number has the wrong format '
+ 'for %(country)s')
+ % {'country': country.name}
+ ))
return result
def onchange_iban(
=== modified file 'account_banking/sepa/iban.py'
--- account_banking/sepa/iban.py 2013-10-14 12:34:20 +0000
+++ account_banking/sepa/iban.py 2014-02-10 20:23:41 +0000
@@ -408,10 +408,9 @@
Localized format of local or Basic Bank Account Number, aka BBAN
'''
if self.countrycode == 'TR':
- raise NotImplementedError, (
- 'The Turkish BBAN requires information that is not in the '
- 'IBAN number.'
- )
+ # The Turkish BBAN requires information that is not in the
+ # IBAN number.
+ return False
return self.BBAN_format.BBAN(self)
@property
=== modified file 'account_banking/sepa/online.py'
--- account_banking/sepa/online.py 2014-01-05 02:07:34 +0000
+++ account_banking/sepa/online.py 2014-02-10 20:23:41 +0000
@@ -157,13 +157,13 @@
'''
Consult the online database for this country to obtain its
corresponding IBAN/BIC number and other info available.
- Raise NotImplemented when no information service could be found.
Returns None when a service was found but something went wrong.
- Returns a dictionary (struct) of information when found.
+ Returns a dictionary (struct) of information when found, or
+ False when not implemented.
'''
if iso in _account_info:
return _account_info[iso](bank_acc)
- raise NotImplementedError()
+ return False
bic_re = re.compile("[^']+'([^']*)'.*")
SWIFTlink = 'http://www.swift.com/bsl/freequery.do'
Follow ups
