Merge lp:~therp-nl/banking-addons/6.1-no_unnecessary_raise into lp:banking-addons/6.1

[Stefan Rijnhart (Opener)]

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.

7.0 version here: https://code.launchpad.net/~therp-nl/banking-addons/7.0-no_unnecessary_raise/+merge/205648

[Yannick Vaucher @ Camptocamp (yvaucher-c2c)]

LGTM

review: Approve (code review, no tests)

[Holger Brunn (Therp) (hbrunn)]

same as https://code.launchpad.net/~therp-nl/banking-addons/7.0-no_unnecessary_raise/+merge/205648

review: Approve (code review)

=== modified file 'account_banking/account_banking.py'
--- account_banking/account_banking.py	2013-12-13 10:45:24 +0000
+++ account_banking/account_banking.py	2014-02-10 20:23:17 +0000
@@ -1321,14 +1321,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]
@@ -1404,10 +1400,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(
@@ -1491,44 +1484,45 @@
             country = country_obj.browse(
                 cursor, uid, country_ids[0], context=context)
         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, cursor, uid,
-                            info.bic or iban_acc.BIC_searchkey,
-                            name = info.bank
-                            )
-                        values['country_id'] = country_id or \
-                                               country_ids and country_ids[0] or \
-                                               False
-                        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, cursor, uid,
+                        info.bic or iban_acc.BIC_searchkey,
+                        name = info.bank
+                        )
+                    values['country_id'] = country_id or \
+                        country_ids and country_ids[0] or \
+                        False
+                    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 %(country)s')
+                        _('The account number appears to be invalid for '
+                          '%(country)s')
                         % {'country': 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 %(country)s')
-                            % {'country': 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:30:40 +0000
+++ account_banking/sepa/iban.py	2014-02-10 20:23:17 +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	2012-01-12 09:50:06 +0000
+++ account_banking/sepa/online.py	2014-02-10 20:23:17 +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'