Code review comment for lp:~therp-nl/banking-addons/6.1-bank_statement_instant_voucher

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

Hi,

319+from osv import osv, fields
320+from tools.translate import _
The full import pathes should now be used as per the new Model classes
Example
from openerp.osv import orm, osv, fields
from openerp.tools.translate import _

class instant_voucher(orm.TransientModel):

277+ context['active_id'] = ids[0]
285+ 'context': context,
you probably want to use the local_context created just upper (line 276)

332+ instant = self.browse(cr, uid, ids[0], context=context)
361+ instant = self.browse(cr, uid, ids[0], context=context)
496+ instant = self.browse(cr, uid, ids[0], context=context)
As the first item only of the ids argument is used, it would be a good idea
to check that only 1 id have been given. ie. use an assert

345+ for (key, val) in vals.items():
Better have to use vals.iteritems() to avoid the generation of a unnecessary list.

340+ def update_voucher_defaults(
It should not return a None value because it is callable from XML/RPC.
I had been mingled by the vals which is a mutable argument and modified in place.
I would have returned only the default vals in this method and updated the vals in the callee method.

« Back to merge proposal