avanzosc team mailing list archive

[Pedro Manuel Baeza <pedro.baeza@xxxxxxxxx>]

Review: Needs Fixing code review

Comentarios inline

Diff comments:

> === modified file 'dos_production_lots_import/wizard/production_lots_import.py'
> --- dos_production_lots_import/wizard/production_lots_import.py	2014-07-03 14:50:51 +0000
> +++ dos_production_lots_import/wizard/production_lots_import.py	2014-07-09 15:24:30 +0000
> @@ -22,10 +22,11 @@
>  import base64
>  import logging
>  import csv
> +from datetime import datetime
>  import time
>  from openerp.tools.translate import _
>  from tempfile import TemporaryFile
> -from openerp.orm import orm, fields
> +from openerp.osv import orm, fields
>  
>  
>  class ProductionLotsImport(orm.TransientModel):
> @@ -33,7 +34,6 @@
>  
>      _name = "production.lots.import"
>      _description = "Production Lots Import"
> -    _inherit = "ir.wizard.screen"
>  
>      _columns = {
>          'product_type': fields.selection([('sim', 'Tarjeta SIM'),
> @@ -44,8 +44,7 @@
>  
>      def is_date(self, date):
>          try:
> -            if date != '':
> -                valid_date = time.strptime(date, '%Y-%m-%d')
> +            datetime.datetime.strptime(date, '%Y-%m-%d')

Esto va a cascar también, ya que debería ser:

datetime.strptime(date, '%Y-%m-%d')

porque has hecho:

from datetime import datetime).

>              return True
>          except ValueError:
>              return False
> @@ -61,7 +60,7 @@
>          return True
>  
>      def get_production_lot_id(self, cr, uid, dic, location_id,
> -                              location_dest_id):
> +                              location_dest_id, context=None):
>          production_lot_obj = self.pool['stock.production.lot']
>          stock_move_obj = self.pool['stock.move']
>          name = None
> @@ -95,7 +94,6 @@
>          stock_picking_obj = self.pool['stock.picking']
>          stock_move_obj = self.pool['stock.move']
>          stock_prod_lot_obj = self.pool['stock.production.lot']
> -        product_obj = self.pool['product.product']
>          try:
>              uid = 1
>              # Empezamos a leer fichero
> @@ -113,7 +111,7 @@
>                  stock_warehouse_obj.search(cr, uid, [], context=context)[0])
>              stock_warehouse = stock_warehouse_obj.browse(cr, uid,
>                                                           stock_warehouse_ids,
> -                                                         context):
> +                                                         context)
>              if not stock_warehouse:
>                  raise orm.except_orm(_('Stock Warehouse not defined !'),
>                                       _('Stock Warehouse not defined.'))
> @@ -169,9 +167,7 @@
>                                                                          uid,
>                                                                          vals)
>                          # Actualizamos datos de lote de produccion
> -                        result = stock_prod_lot_obj.write(cr, uid,
> -                                                          [prod_lot_id],
> -                                                          dic)
> +                        stock_prod_lot_obj.write(cr, uid, [prod_lot_id], dic)
>                          # Traspaso de ubicación
>                          stock_prod_lot = stock_prod_lot_obj.browse(cr, uid,
>                                                                     prod_lot_id,
> @@ -197,7 +193,7 @@
>                              'priority': '1',
>                              'state': 'done',
>                              }
> -                        stock_move_id = stock_move_obj.create(cr, uid, vals)
> +                        stock_move_obj.create(cr, uid, vals)

¿Tenemos el context disponible?

>                      else:
>                          succesfull = False
>                          filename = ('[production_lot_import][format: %s]'
> @@ -247,7 +243,6 @@
>              fileformat = 'csv'
>          else:
>              header_line_columns = ""
> -            file_format = 'no_csv'
>          fileobj.seek(0)
>          self.load_data(cr, fileobj, fileformat, header_line_columns, context)
>          fileobj.close()
> 
> === modified file 'dos_production_lots_import/wizard/production_lots_import_view.xml'
> --- dos_production_lots_import/wizard/production_lots_import_view.xml	2014-06-24 13:05:55 +0000
> +++ dos_production_lots_import/wizard/production_lots_import_view.xml	2014-07-09 15:24:30 +0000
> @@ -6,31 +6,27 @@
>              <field name="name">Activation of SIM Importer</field>
>              <field name="model">production.lots.import</field>
>              <field name="arch" type="xml">
> -                <form string="Activation of SIM Importer">
> +                <form string="Activation of SIM Importer" version="7.0">
>                      <group col="8">
>                          <group colspan="3">
> -                            <field name="config_logo" widget="image" width="220" height="130" nolabel="1" colspan="1"/>
> -                            <newline/>
> -                            <label colspan="4" width="220" string="Supported file format: *.csv (Comma-separated values)"/>
> -                            <label colspan="4" width="220" string="Please double-check that the file encoding is set to UTF-8 (sometimes called Unicode) when the translator exports it."/>
> -                            <label colspan="4" width="220"/>
> -                            <label colspan="4" width="220" string="When using CSV format, please also check that the first line of your file is one of the following:"/>
> -                            <label colspan="4" width="220"/>
> -							<label colspan="4" width="220" string="icc;telefono;pin;puk;fecha_alta;operador;"/>
> -							<label colspan="4" width="220" string="descripcion;albaran_entrada"/>
> +                            <p colspan="4" width="220" string="Supported file format: *.csv (Comma-separated values)"/>

Puedes quitar colspan y width, ya que eso pone atributos raros. Por defecto, los <p> ya se colocan como deben ser según el group donde estén. También se pone <p>Texto</p>

> +                            <p colspan="4" width="220" string="Please double-check that the file encoding is set to UTF-8 (sometimes called Unicode) when the translator exports it."/>

Ídem para las siguientes líneas

> +                            <p colspan="4" width="220" string="When using CSV format, please also check that the first line of your file is one of the following:"/>
> +                            <p colspan="4" width="220" string="icc;telefono;pin;puk;fecha_alta;operador;"/>
> +                            <p colspan="4" width="220" string="descripcion;albaran_entrada"/>
>                          </group>
>                          <separator orientation="vertical" rowspan="15"/>
>                          <group colspan="4" col="4">
>                              <separator string="Activation of SIM Importer" colspan="4"/>
> -                            <newline/>
>                              <field name="product_type" width="200" invisible="1"/>
>                              <field name="data" colspan="4"/>
>                          </group>
>                          <group colspan="8" col="8">
>                              <separator string="" colspan="8"/>
> -                            <label colspan="6" width="220"/>
> -                                <button special="cancel" string="_Close" icon="gtk-cancel"/>
> +                            <footer>
>                                  <button name="import_file" string="_Import" type="object" icon="gtk-ok"/>
> +                                <button class="oe_link" string="_Close" special="cancel"/>

La palabra "or"

> +                            </footer>
>                          </group>
>                      </group>
>                  </form>
> 


-- 
https://code.launchpad.net/~avanzosc-security-team/avanzosc/72h_dos_production_lots_import/+merge/226093
Your team Avanzosc_security is subscribed to branch lp:~avanzosc-security-team/avanzosc/72horas.