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
=== modified file 'account_statement_base_completion/statement.py'
--- account_statement_base_completion/statement.py 2012-12-20 13:40:22 +0000
+++ account_statement_base_completion/statement.py 2013-01-28 13:04:25 +0000
@@ -286,6 +286,8 @@
286 pattern = ".*%s.*" % st_line.label286 pattern = ".*%s.*" % st_line.label
287 cr.execute(sql, (pattern,))287 cr.execute(sql, (pattern,))
288 result = cr.fetchall()288 result = cr.fetchall()
289 if not result:
290 return res
289 if len(result) > 1:291 if len(result) > 1:
290 raise ErrorTooManyPartner(292 raise ErrorTooManyPartner(
291 _('Line named "%s" (Ref:%s) was matched by more '293 _('Line named "%s" (Ref:%s) was matched by more '
@@ -442,9 +444,9 @@
442 if res:444 if res:
443 compl_lines += 1445 compl_lines += 1
444 except ErrorTooManyPartner, exc:446 except ErrorTooManyPartner, exc:
445 msg += exc.value + "\n"447 msg += repr(exc) + "\n"
446 except Exception, exc:448 except Exception, exc:
447 msg += exc.value + "\n"449 msg += repr(exc) + "\n"
448 # vals = res and res.keys() or False450 # vals = res and res.keys() or False
449 if res:451 if res:
450 vals = res[line.id]452 vals = res[line.id]

Subscribers

People subscribed via source and target branches