avanzosc team mailing list archive
-
avanzosc team
-
Mailing list archive
-
Message #00626
Re: [Merge] lp:~mikelarregi/avanzosc/account_invoice_layout.py into lp:~avanzosc-security-team/avanzosc/72horas
Review: Needs Fixing code review
Varios comentarios que no son bloqueantes, pero estaría bien cambiar. Sobre lo que comentas, seguramente no haga falta la copia del context, ya que eso se hace cuando no se quiere que un valor escrito en la función llamada, se acumule en el diccionario general, pero normalmente no suele ser el caso.
Sobre las llamadas SQL, ya sabes que no es lo ideal, pero si no te quieres complicar la vida y no las ves fácilmente sustituibles por ORM, déjalas y ya está.
Un saludo.
Diff comments:
> === modified file 'nayar_basic_account_invoice_layout/__init__.py'
> --- nayar_basic_account_invoice_layout/__init__.py 2014-06-11 10:23:47 +0000
> +++ nayar_basic_account_invoice_layout/__init__.py 2014-07-04 09:28:16 +0000
> @@ -19,8 +19,7 @@
> #
> ##############################################################################
>
> -import account_invoice_layout
> -import report
> +from . import models
> +from . import report
>
> # vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4:
> -
>
> === modified file 'nayar_basic_account_invoice_layout/__openerp__.py'
> --- nayar_basic_account_invoice_layout/__openerp__.py 2014-06-11 10:23:47 +0000
> +++ nayar_basic_account_invoice_layout/__openerp__.py 2014-07-04 09:28:16 +0000
> @@ -32,17 +32,18 @@
> * add titles, comment lines, sub total lines
> * draw horizontal lines and put page breaks
>
> - Moreover, there is one option which allows you to print all the selected invoices with a given special message at the bottom of it. This feature can be very useful for printing your invoices with end-of-year wishes, special punctual conditions...
> + Moreover, there is one option which allows you to print all the selected"""
> + """ invoices with a given special message at the bottom of it. """
> + """This feature can be very useful for printing your invoices """
> + """with end-of-year wishes, special punctual conditions...
>
> """,
> 'author': 'Nayar Systems',
> 'website': 'http://www.72horas.net',
> 'depends': ['account'],
> - 'init_xml': [],
> - 'update_xml': [
> - 'account_invoice_layout_report.xml',
> + 'data': [
> + 'report/account_invoice_layout_report.xml',
> ],
> 'installable': True,
> - 'active': False,
> }
> # vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4:
>
> === added directory 'nayar_basic_account_invoice_layout/models'
> === added file 'nayar_basic_account_invoice_layout/models/__init__.py'
> --- nayar_basic_account_invoice_layout/models/__init__.py 1970-01-01 00:00:00 +0000
> +++ nayar_basic_account_invoice_layout/models/__init__.py 2014-07-04 09:28:16 +0000
> @@ -0,0 +1,22 @@
> +# -*- coding: utf-8 -*-
> +##############################################################################
> +#
> +# OpenERP, Open Source Management Solution
> +# Copyright (C) 2004-2010 Tiny SPRL (<http://tiny.be>).
> +#
> +# This program is free software: you can redistribute it and/or modify
> +# it under the terms of the GNU Affero General Public License as
> +# published by the Free Software Foundation, either version 3 of the
> +# License, or (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU Affero General Public License for more details.
> +#
> +# You should have received a copy of the GNU Affero General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +#
> +##############################################################################
> +
> +from . import account_invoice_layout
>
> === renamed file 'nayar_basic_account_invoice_layout/account_invoice_layout.py' => 'nayar_basic_account_invoice_layout/models/account_invoice_layout.py'
> --- nayar_basic_account_invoice_layout/account_invoice_layout.py 2014-06-11 10:23:47 +0000
> +++ nayar_basic_account_invoice_layout/models/account_invoice_layout.py 2014-07-04 09:28:16 +0000
> @@ -19,38 +19,50 @@
> #
> ##############################################################################
>
> -from osv import fields, osv
> -
> -class notify_message(osv.osv):
> +from openerp.osv import fields, orm
> +
> +
> +class NotifyMessage(orm.Model):
> _name = 'notify.message'
> _description = 'Notify By Messages'
> _columns = {
> 'name': fields.char('Title', size=64, required=True),
> - 'msg': fields.text('Special Message', size=128, required=True, help='This notification will appear at the bottom of the Invoices when printed.', translate=True)
> + 'msg': fields.text(
> + 'Special Message', size=128, required=True,
> + help=('This notification will appear at the bottom of the Invoices'
> + ' when printed.'), translate=True)
> }
>
> -notify_message()
>
> -class account_invoice_line(osv.osv):
> +class AccountInvoiceLine(orm.Model):
>
> def move_line_get_item(self, cr, uid, line, context=None):
> if line.state != 'article':
> return None
> - return super(account_invoice_line, self).move_line_get_item(cr, uid, line, context)
> + return super(
> + AccountInvoiceLine, self).move_line_get_item(cr, uid, line,
> + context=context)
>
> def fields_get(self, cr, uid, fields=None, context=None):
> article = {
> 'article': [('readonly', False), ('invisible', False)],
> - 'text': [('readonly', True), ('invisible', True), ('required', False)],
> - 'subtotal': [('readonly', True), ('invisible', True), ('required', False)],
> - 'title': [('readonly', True), ('invisible', True), ('required', False)],
> - 'break': [('readonly', True), ('invisible', True), ('required', False)],
> - 'line': [('readonly', True), ('invisible', True), ('required', False)],
> + 'text': [('readonly', True), ('invisible', True),
> + ('required', False)],
> + 'subtotal': [('readonly', True), ('invisible', True),
> + ('required', False)],
> + 'title': [('readonly', True), ('invisible', True),
> + ('required', False)],
> + 'break': [('readonly', True), ('invisible', True),
> + ('required', False)],
> + 'line': [('readonly', True), ('invisible', True),
> + ('required', False)],
> }
> states = {
> 'name': {
> - 'break': [('readonly', True),('required', False),('invisible', True)],
> - 'line': [('readonly', True),('required', False),('invisible', True)],
> + 'break': [('readonly', True), ('required', False),
> + ('invisible', True)],
> + 'line': [('readonly', True), ('required', False),
> + ('invisible', True)],
> },
> 'product_id': article,
> 'account_id': article,
> @@ -61,15 +73,17 @@
> 'invoice_line_tax_id': article,
> 'account_analytic_id': article,
> }
> - res = super(account_invoice_line, self).fields_get(cr, uid, fields, context)
> + res = super(AccountInvoiceLine, self).fields_get(cr, uid, fields,
> + context=context)
> for field in res:
> - if states.has_key(field):
> - for key,value in states[field].items():
> - res[field].setdefault('states',{})
> + if field in states:
> + for key, value in states[field].items():
> + res[field].setdefault('states', {})
> res[field]['states'][key] = value
> return res
>
> - def onchange_invoice_line_view(self, cr, uid, id, type, context=None, *args):
> + def onchange_invoice_line_view(self, cr, uid, id, type, context=None,
> + *args):
>
> if (not type):
> return {}
> @@ -85,7 +99,7 @@
> 'invoice_line_tax_id': False,
> 'account_analytic_id': False,
> },
> - }
> + }
> if type == 'line':
> temp['value']['name'] = ' '
> if type == 'break':
> @@ -96,45 +110,51 @@
> return {}
>
> def create(self, cr, user, vals, context=None):
> - if vals.has_key('state'):
> + if 'state' in vals:
> if vals['state'] == 'line':
> vals['name'] = ' '
> if vals['state'] == 'break':
> vals['name'] = ' '
> if vals['state'] != 'article':
> - vals['quantity']= 0
> - vals['account_id']= self._default_account(cr, user, None)
> - return super(account_invoice_line, self).create(cr, user, vals, context)
> + vals['quantity'] = 0
> + vals['account_id'] = self._default_account(cr, user, None)
> + return super(AccountInvoiceLine, self).create(cr, user, vals, context)
>
> def write(self, cr, user, ids, vals, context=None):
> - if vals.has_key('state'):
> + if 'state' in vals:
> if vals['state'] != 'article':
> - vals['product_id']= False
> - vals['uos_id']= False
> - vals['account_id']= self._default_account(cr, user, None)
> - vals['price_unit']= False
> - vals['price_subtotal']= False
> - vals['quantity']= 0
> - vals['discount']= False
> - vals['invoice_line_tax_id']= False
> - vals['account_analytic_id']= False
> + vals['product_id'] = False
> + vals['uos_id'] = False
> + vals['account_id'] = self._default_account(cr, user, None)
> + vals['price_unit'] = False
> + vals['price_subtotal'] = False
> + vals['quantity'] = 0
> + vals['discount'] = False
> + vals['invoice_line_tax_id'] = False
> + vals['account_analytic_id'] = False
> if vals['state'] == 'line':
> vals['name'] = ' '
> if vals['state'] == 'break':
> vals['name'] = ' '
> - return super(account_invoice_line, self).write(cr, user, ids, vals, context)
> + return super(
> + AccountInvoiceLine, self).write(cr, user, ids, vals,
> + context=context)
>
> def copy_data(self, cr, uid, id, default=None, context=None):
> if default is None:
> default = {}
> default['state'] = self.browse(cr, uid, id, context=context).state
> - return super(account_invoice_line, self).copy_data(cr, uid, id, default, context)
> + return super(
> + AccountInvoiceLine, self).copy_data(cr, uid, id, default,
> + context=context)
>
> def _fnct(self, cr, uid, ids, name, args, context=None):
> res = {}
> lines = self.browse(cr, uid, ids, context=context)
> account_ids = [line.account_id.id for line in lines]
> - account_names = dict(self.pool.get('account.account').name_get(cr, uid, account_ids, context=context))
> + account_names = (
> + dict(self.pool['account.account'].name_get(cr, uid, account_ids,
> + context=context)))
> for line in lines:
> if line.state != 'article':
> if line.state == 'line':
> @@ -152,23 +172,31 @@
> _description = "Invoice Line"
> _inherit = "account.invoice.line"
> _columns = {
> - 'state': fields.selection([
> - ('article','Product'),
> - ('title','Title'),
> - ('text','Note'),
> - ('subtotal','Sub Total'),
> - ('line','Separator Line'),
> - ('break','Page Break'),]
> - ,'Type', select=True, required=True),
> - 'sequence': fields.integer('Sequence Number', help="Gives the sequence order when displaying a list of invoice lines."),
> - 'functional_field': fields.function(_fnct, arg=None, fnct_inv=None, fnct_inv_arg=None, type='char', fnct_search=None, obj=None, method=True, store=False, string="Source Account"),
> + 'state': fields.selection(
> + [('article', 'Product'),
> + ('title', 'Title'),
> + ('text', 'Note'),
> + ('subtotal', 'Sub Total'),
> + ('line', 'Separator Line'),
> + ('break', 'Page Break')], 'Type', select=True, required=True),
> + 'sequence': fields.integer(
> + 'Sequence Number', help="Gives the sequence order when"
> + " displaying a list of invoice lines."),
> + 'functional_field': fields.function(
> + _fnct, arg=None, fnct_inv=None, fnct_inv_arg=None, type='char',
> + fnct_search=None, obj=None, method=True, store=False,
> + string="Source Account"),
> }
>
> def _default_account(self, cr, uid, context=None):
> if context is None:
No hace falta inicializar context
> context = {}
> - current_company = self.pool.get('res.users').browse(cr,uid,uid).company_id.id
> - account_id = self.pool.get('account.account').search(cr, uid, [('company_id','=',current_company),('parent_id','=',False)], limit=1,context=context)
> + current_company = (
> + self.pool['res.users'].browse(cr, uid, uid,
> + context=context).company_id.id)
> + account_id = (self.pool['account.account'].search(
> + cr, uid, [('company_id', '=', current_company),
> + ('parent_id', '=', False)], limit=1, context=context))
> return account_id and account_id[0] or False
>
> _defaults = {
> @@ -176,11 +204,11 @@
> 'sequence': 0,
> }
>
> -account_invoice_line()
>
> class one2many_mod2(fields.one2many):
>
> - def get(self, cr, obj, ids, name, user=None, offset=0, context=None, values=None):
> + def get(self, cr, obj, ids, name, user=None, offset=0, context=None,
> + values=None):
> if context is None:
Puede que no haga falta inicializar context
> context = {}
> if not values:
> @@ -188,25 +216,37 @@
> res = {}
> for id in ids:
> res[id] = []
> - ids2 = obj.pool.get(self._obj).search(cr, user, [(self._fields_id,'in',ids),('state','=','article')], limit=self._limit)
> - for r in obj.pool.get(self._obj)._read_flat(cr, user, ids2, [self._fields_id], context=context, load='_classic_write'):
> - res[r[self._fields_id]].append( r['id'] )
> + ids2 = (
> + obj.pool.get(self._obj).search(cr, user,
> + [(self._fields_id, 'in', ids),
> + ('state', '=', 'article')],
> + limit=self._limit))
propagar context
> + for r in obj.pool.get(
Esto puede ser obj.pool[
> + self._obj)._read_flat(cr, user, ids2, [self._fields_id],
> + context=context, load='_classic_write'):
> + res[r[self._fields_id]].append(r['id'])
> return res
>
> -class account_invoice(osv.osv):
> +
> +class AccountInvoice(orm.Model):
>
> def copy(self, cr, uid, id, default=None, context=None):
> if default is None:
> default = {}
> default['invoice_line'] = False
> - return super(account_invoice, self).copy(cr, uid, id, default, context)
> + return super(AccountInvoice, self).copy(cr, uid, id, default,
> + context=context)
>
> _inherit = "account.invoice"
> _columns = {
> - 'abstract_line_ids': fields.one2many('account.invoice.line', 'invoice_id', 'Invoice Lines',readonly=True, states={'draft':[('readonly',False)]}),
> - 'invoice_line': one2many_mod2('account.invoice.line', 'invoice_id', 'Invoice Lines',readonly=True, states={'draft':[('readonly',False)]}),
> + 'abstract_line_ids':
> + fields.one2many('account.invoice.line', 'invoice_id',
> + 'Invoice Lines', readonly=True,
> + states={'draft': [('readonly', False)]}),
> + 'invoice_line':
> + one2many_mod2('account.invoice.line', 'invoice_id',
> + 'Invoice Lines', readonly=True,
> + states={'draft': [('readonly', False)]})
> }
>
> -account_invoice()
> -
> # vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4:
>
> === modified file 'nayar_basic_account_invoice_layout/report/__init__.py'
> --- nayar_basic_account_invoice_layout/report/__init__.py 2014-06-11 10:23:47 +0000
> +++ nayar_basic_account_invoice_layout/report/__init__.py 2014-07-04 09:28:16 +0000
> @@ -19,6 +19,6 @@
> #
> ##############################################################################
>
> -import report_account_invoice_layout
> +from . import report_account_invoice_layout
> # vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4:
>
>
> === renamed file 'nayar_basic_account_invoice_layout/account_invoice_layout_report.xml' => 'nayar_basic_account_invoice_layout/report/account_invoice_layout_report.xml'
> === modified file 'nayar_basic_account_invoice_layout/report/report_account_invoice_layout.py'
> --- nayar_basic_account_invoice_layout/report/report_account_invoice_layout.py 2014-06-11 10:23:47 +0000
> +++ nayar_basic_account_invoice_layout/report/report_account_invoice_layout.py 2014-07-04 09:28:16 +0000
> @@ -20,13 +20,13 @@
> ##############################################################################
>
> import time
> -
> -from report import report_sxw
> -
> -
> -class nayar_basic_account_invoice_1(report_sxw.rml_parse):
> +from openerp.report import report_sxw
> +
> +
> +class NayarBasicAccountInvoice1(report_sxw.rml_parse):
> def __init__(self, cr, uid, name, context):
> - super(nayar_basic_account_invoice_1, self).__init__(cr, uid, name, context=context)
> + super(NayarBasicAccountInvoice1, self).__init__(cr, uid, name,
> + context=context)
> self.localcontext.update({
> 'time': time,
> 'invoice_lines': self.invoice_lines,
> @@ -36,13 +36,16 @@
> self._node = None
>
> def get_refund(self, invoice):
> - self.cr.execute("SELECT i.number, i.date_invoice from account_invoice i inner join account_invoice_refunds_rel r on r.original_invoice_id = i.id where r.refund_invoice_id = %s",(invoice.id,))
> + self.cr.execute("SELECT i.number, i.date_invoice "
> + "from account_invoice i inner join "
> + "account_invoice_refunds_rel r on "
> + "r.original_invoice_id = i.id "
> + "where r.refund_invoice_id = %s", (invoice.id,))
> res = self.cr.fetchall()
> numbers = ""
> for i in res:
> date = i[1].split(" ")[0]
> numbers += i[0] + " (" + date + ")"
> -
> return numbers
>
> def invoice_lines(self, invoice):
> @@ -52,47 +55,66 @@
> invoice_list = []
> res = {}
> list_in_seq = {}
> - ids = self.pool.get('account.invoice.line').search(self.cr, self.uid, [('invoice_id', '=', invoice.id)])
> + ids = self.pool[
> + 'account.invoice.line'].search(self.cr, self.uid,
> + [('invoice_id', '=', invoice.id)],
> + context=self.context.copy())
propagar context
> ids.sort()
> for id in range(0, len(ids)):
> - info = self.pool.get('account.invoice.line').browse(self.cr, self.uid, ids[id], self.context.copy())
> + info = self.pool[
> + 'account.invoice.line'].browse(self.cr, self.uid,
> + ids[id], self.context.copy())
propagar context
> list_in_seq[info] = info.sequence
> i = 1
> j = 0
> - final=sorted(list_in_seq.items(), lambda x, y: cmp(x[1], y[1]))
> + final = sorted(list_in_seq.items(), lambda x, y: cmp(x[1], y[1]))
> invoice_list = [x[0] for x in final]
> sum_flag = {}
> sum_flag[j] = -1
> for entry in invoice_list:
> res = {}
> if entry.state == 'article':
> - self.cr.execute('select tax_id from account_invoice_line_tax where invoice_line_id=%s', (entry.id,))
> + self.cr.execute('select tax_id from account_invoice_line_tax '
> + 'where invoice_line_id=%s', (entry.id,))
> tax_ids = self.cr.fetchall()
> if tax_ids == []:
> res['tax_types'] = ''
> else:
> tax_names_dict = {}
> for item in range(0, len(tax_ids)):
> - self.cr.execute('select name from account_tax where id=%s', (tax_ids[item][0],))
> + self.cr.execute('select name '
> + 'from account_tax where id=%s',
> + (tax_ids[item][0],))
> type = self.cr.fetchone()
> tax_names_dict[item] = type[0]
> - tax_names = ','.join([tax_names_dict[x] for x in range(0, len(tax_names_dict))])
> + tax_names = ','.join(
> + [tax_names_dict[x]
> + for x in range(0, len(tax_names_dict))])
> res['tax_types'] = tax_names
> res['name'] = entry.name
> - res['quantity'] = self.formatLang(entry.quantity, digits=self.get_digits(dp='Account'))
> - res['price_unit'] = self.formatLang(entry.price_unit, digits=self.get_digits(dp='Account'))
> - res['discount'] = self.formatLang(entry.discount, digits=self.get_digits(dp='Account'))
> - res['price_subtotal'] = self.formatLang(entry.price_subtotal, digits=self.get_digits(dp='Account'))
> + res['quantity'] = (
> + self.formatLang(entry.quantity,
> + digits=self.get_digits(dp='Account')))
> + res['price_unit'] = (
> + self.formatLang(entry.price_unit,
> + digits=self.get_digits(dp='Account')))
> + res['discount'] = (
> + self.formatLang(entry.discount,
> + digits=self.get_digits(dp='Account')))
> + res['price_subtotal'] = (
> + self.formatLang(entry.price_subtotal,
> + digits=self.get_digits(dp='Account')))
> sub_total[i] = entry.price_subtotal
> i = i + 1
> res['note'] = entry.note
> res['currency'] = invoice.currency_id.symbol
> res['type'] = entry.state
> -
> - if entry.uos_id.id == False:
> + if not entry.uos_id.id:
Se podría incluso chequear if not entry.uos_id:, ya que browse_record_null se evalúa a False.
> res['uos'] = ''
> else:
> - uos_name = self.pool.get('product.uom').read(self.cr, self.uid, entry.uos_id.id, ['name'], self.context.copy())
> + uos_name = self.pool['product.uom'].read(
> + self.cr, self.uid, entry.uos_id.id, ['name'],
> + self.context.copy())
propagar context
> res['uos'] = uos_name['name']
> else:
> res['quantity'] = ''
> @@ -102,7 +124,6 @@
> res['type'] = entry.state
> res['note'] = entry.note
> res['uos'] = ''
> -
> if entry.state == 'subtotal':
> res['name'] = entry.name
> sum = 0
> @@ -111,11 +132,9 @@
> temp = 1
> else:
> temp = sum_flag[j]
> -
> for sum_id in range(temp, len(sub_total)+1):
> sum += sub_total[sum_id]
> - sum_flag[j+1] = sum_id +1
> -
> + sum_flag[j+1] = sum_id + 1
> j = j + 1
> res['price_subtotal'] = "%.2f" % (sum)
> res['currency'] = invoice.currency_id.symbol
> @@ -138,7 +157,8 @@
> res['discount'] = '____________'
> res['tax_types'] = '____________________'
> res['uos'] = '_____'
> - res['name'] = '_______________________________________________'
> + res['name'] = (
> + '_______________________________________________')
> res['price_subtotal'] = '____________'
> res['currency'] = '____'
> elif entry.state == 'break':
> @@ -150,9 +170,10 @@
> res['name'] = entry.name
> res['price_subtotal'] = ''
> res['currency'] = invoice.currency_id.symbol
> -
> result.append(res)
> return result
> -report_sxw.report_sxw('report.nayar.basic.account.invoice.layout', 'account.invoice', 'addons/nayar_basic_account_invoice_layout/report/report_account_invoice_layout.rml', parser=nayar_basic_account_invoice_1)
> +report_sxw.report_sxw(
> + 'report.nayar.basic.account.invoice.layout', 'account.invoice',
> + 'addons/nayar_basic_account_invoice_layout/report/'
> + 'report_account_invoice_layout.rml', parser=NayarBasicAccountInvoice1)
> # vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4:
> -
>
--
https://code.launchpad.net/~mikelarregi/avanzosc/account_invoice_layout.py/+merge/225622
Your team Avanzosc_security is subscribed to branch lp:~avanzosc-security-team/avanzosc/72horas.
References
