Merge lp:~camptocamp/banking-addons/bank-statement-reconcile-70-fix-completion-error-message into lp:banking-addons/bank-statement-reconcile-70

Proposed by Nicolas Bessi - Camptocamp
Status: Superseded
Proposed branch: lp:~camptocamp/banking-addons/bank-statement-reconcile-70-fix-completion-error-message
Merge into: lp:banking-addons/bank-statement-reconcile-70
Diff against target: 24 lines (+4/-2)
1 file modified
account_statement_base_completion/statement.py (+4/-2)
To merge this branch: bzr merge lp:~camptocamp/banking-addons/bank-statement-reconcile-70-fix-completion-error-message
Reviewer Review Type Date Requested Status
Guewen Baconnier @ Camptocamp Approve
Alexandre Fayolle - camptocamp code review, no test Needs Fixing
Review via email: mp+145146@code.launchpad.net

This proposal has been superseded by a proposal from 2013-01-28.

Description of the change

Fix of error message + index out of range

To post a comment you must log in.
Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote :

Why do you call exc.__repr__() and not repr(exc)?

Revision history for this message
Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount) wrote :

Just the rush. But you right repr(x) will ensure return type in string. It will avoid concatenation error

79. By Nicolas Bessi - Camptocamp

[IMP] better call string repr

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

LGTM

review: Approve
Revision history for this message
Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) wrote :

LGTM

review: Approve (code review, no test)
Revision history for this message
Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) wrote :

I suggest changing the code of button_auto_completion in the following way:

* change msg from string to a list of strings for efficient appending
* reinit the msg_list for each id of the outer loop

=== modified file 'account_statement_base_completion/statement.py'
--- account_statement_base_completion/statement.py 2013-01-28 12:56:53 +0000
+++ account_statement_base_completion/statement.py 2013-01-30 09:55:41 +0000
@@ -432,9 +432,9 @@
         if context is None:
             context = {}
         stat_line_obj = self.pool.get('account.bank.statement.line')
- msg = ""
         compl_lines = 0
         for stat in self.browse(cr, uid, ids, context=context):
+ msg_lines = []
             ctx = context.copy()
             for line in stat.line_ids:
                 res = {}
@@ -444,14 +444,15 @@
                     if res:
                         compl_lines += 1
                 except ErrorTooManyPartner, exc:
- msg += repr(exc) + "\n"
+ msg_lines.append(repr(exc))
                 except Exception, exc:
- msg += repr(exc) + "\n"
+ msg_lines.append(repr(exc))
                 # vals = res and res.keys() or False
                 if res:
                     vals = res[line.id]
                     vals['already_completed'] = True
                     stat_line_obj.write(cr, uid, [line.id], vals, context=ctx)
- self.write_completion_log(
- cr, uid, stat.id, msg, compl_lines, context=context)
+ msg = u'\n'.join(msg_lines)
+ self.write_completion_log(cr, uid, stat.id,
+ msg, compl_lines, context=context)
         return True

review: Needs Fixing (code review, no test)
80. By Alexandre Fayolle - camptocamp

[IMP] use a list of lines and join for efficient concatenation, reinit the msg list at the correct place

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

I agree with your change Alexandre

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

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'account_statement_base_completion/statement.py'
2--- account_statement_base_completion/statement.py 2012-12-20 13:40:22 +0000
3+++ account_statement_base_completion/statement.py 2013-01-28 13:04:25 +0000
4@@ -286,6 +286,8 @@
5 pattern = ".*%s.*" % st_line.label
6 cr.execute(sql, (pattern,))
7 result = cr.fetchall()
8+ if not result:
9+ return res
10 if len(result) > 1:
11 raise ErrorTooManyPartner(
12 _('Line named "%s" (Ref:%s) was matched by more '
13@@ -442,9 +444,9 @@
14 if res:
15 compl_lines += 1
16 except ErrorTooManyPartner, exc:
17- msg += exc.value + "\n"
18+ msg += repr(exc) + "\n"
19 except Exception, exc:
20- msg += exc.value + "\n"
21+ msg += repr(exc) + "\n"
22 # vals = res and res.keys() or False
23 if res:
24 vals = res[line.id]

Subscribers

People subscribed via source and target branches