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

Proposed by Stefan Rijnhart (Opener) on 2014-02-10
Status: Merged
Merged at revision: 194
Proposed branch: lp:~therp-nl/banking-addons/6.1-no_unnecessary_raise
Merge into: lp:banking-addons/6.1
Diff against target: 149 lines (+42/-49)
3 files modified
account_banking/account_banking.py (+36/-42)
account_banking/sepa/iban.py (+3/-4)
account_banking/sepa/online.py (+3/-3)
To merge this branch: bzr merge lp:~therp-nl/banking-addons/6.1-no_unnecessary_raise
Reviewer Review Type Date Requested Status
Holger Brunn (Therp) code review Approve on 2014-02-24
Yannick Vaucher @ Camptocamp code review, no tests 2014-02-10 Approve on 2014-02-24
Review via email: mp+205647@code.launchpad.net

Description of the change

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

To post a comment you must log in.

LGTM

review: Approve (code review, no tests)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'account_banking/account_banking.py'
2--- account_banking/account_banking.py 2013-12-13 10:45:24 +0000
3+++ account_banking/account_banking.py 2014-02-10 20:23:17 +0000
4@@ -1321,14 +1321,10 @@
5 if term[0].lower() == 'acc_number' and term[1] in ('=', '=='):
6 iban = sepa.IBAN(term[2])
7 if iban.valid:
8- # Some countries can't convert to BBAN
9- try:
10- bban = iban.localized_BBAN
11- # Prevent empty search filters
12- if bban:
13- extra_term = ('acc_number_domestic', term[1], bban)
14- except:
15- pass
16+ bban = iban.localized_BBAN
17+ # Prevent empty search filters
18+ if bban:
19+ extra_term = ('acc_number_domestic', term[1], bban)
20 if extra_term:
21 return ['|', term, extra_term]
22 return [term]
23@@ -1404,10 +1400,7 @@
24 res[record.id] = False
25 else:
26 iban_acc = sepa.IBAN(record.acc_number)
27- try:
28- res[record.id] = iban_acc.localized_BBAN
29- except NotImplementedError:
30- res[record.id] = False
31+ res[record.id] = iban_acc.localized_BBAN
32 return res
33
34 def onchange_acc_number(
35@@ -1491,44 +1484,45 @@
36 country = country_obj.browse(
37 cursor, uid, country_ids[0], context=context)
38 if country and country.code in sepa.IBAN.countries:
39- try:
40- info = sepa.online.account_info(country.code, acc_number)
41- if info:
42- iban_acc = sepa.IBAN(info.iban)
43- if iban_acc.valid:
44- values['acc_number_domestic'] = iban_acc.localized_BBAN
45- values['acc_number'] = unicode(iban_acc)
46- values['state'] = 'iban'
47- bank_id, country_id = get_or_create_bank(
48- self.pool, cursor, uid,
49- info.bic or iban_acc.BIC_searchkey,
50- name = info.bank
51- )
52- values['country_id'] = country_id or \
53- country_ids and country_ids[0] or \
54- False
55- values['bank'] = bank_id or False
56- if info.bic:
57- values['bank_bic'] = info.bic
58- else:
59- info = None
60- if info is None:
61- result.update(warning(
62+ info = sepa.online.account_info(country.code, acc_number)
63+ if info:
64+ iban_acc = sepa.IBAN(info.iban)
65+ if iban_acc.valid:
66+ values['acc_number_domestic'] = iban_acc.localized_BBAN
67+ values['acc_number'] = unicode(iban_acc)
68+ values['state'] = 'iban'
69+ bank_id, country_id = get_or_create_bank(
70+ self.pool, cursor, uid,
71+ info.bic or iban_acc.BIC_searchkey,
72+ name = info.bank
73+ )
74+ values['country_id'] = country_id or \
75+ country_ids and country_ids[0] or \
76+ False
77+ values['bank'] = bank_id or False
78+ if info.bic:
79+ values['bank_bic'] = info.bic
80+ else:
81+ info = None
82+ if info is None:
83+ result.update(warning(
84 _('Invalid data'),
85- _('The account number appears to be invalid for %(country)s')
86+ _('The account number appears to be invalid for '
87+ '%(country)s')
88 % {'country': country.name}
89- ))
90- except NotImplementedError:
91+ ))
92+ if info is False:
93 if country.code in sepa.IBAN.countries:
94 acc_number_fmt = sepa.BBAN(acc_number, country.code)
95 if acc_number_fmt.valid:
96 values['acc_number_domestic'] = str(acc_number_fmt)
97 else:
98 result.update(warning(
99- _('Invalid format'),
100- _('The account number has the wrong format for %(country)s')
101- % {'country': country.name}
102- ))
103+ _('Invalid format'),
104+ _('The account number has the wrong format '
105+ 'for %(country)s')
106+ % {'country': country.name}
107+ ))
108 return result
109
110 def onchange_iban(
111
112=== modified file 'account_banking/sepa/iban.py'
113--- account_banking/sepa/iban.py 2013-10-14 12:30:40 +0000
114+++ account_banking/sepa/iban.py 2014-02-10 20:23:17 +0000
115@@ -408,10 +408,9 @@
116 Localized format of local or Basic Bank Account Number, aka BBAN
117 '''
118 if self.countrycode == 'TR':
119- raise NotImplementedError, (
120- 'The Turkish BBAN requires information that is not in the '
121- 'IBAN number.'
122- )
123+ # The Turkish BBAN requires information that is not in the
124+ # IBAN number.
125+ return False
126 return self.BBAN_format.BBAN(self)
127
128 @property
129
130=== modified file 'account_banking/sepa/online.py'
131--- account_banking/sepa/online.py 2012-01-12 09:50:06 +0000
132+++ account_banking/sepa/online.py 2014-02-10 20:23:17 +0000
133@@ -157,13 +157,13 @@
134 '''
135 Consult the online database for this country to obtain its
136 corresponding IBAN/BIC number and other info available.
137- Raise NotImplemented when no information service could be found.
138 Returns None when a service was found but something went wrong.
139- Returns a dictionary (struct) of information when found.
140+ Returns a dictionary (struct) of information when found, or
141+ False when not implemented.
142 '''
143 if iso in _account_info:
144 return _account_info[iso](bank_acc)
145- raise NotImplementedError()
146+ return False
147
148 bic_re = re.compile("[^']+'([^']*)'.*")
149 SWIFTlink = 'http://www.swift.com/bsl/freequery.do'

Subscribers

People subscribed via source and target branches