openerp-community-reviewer team mailing list archive

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

Review: Needs Fixing code review

I have made some comments inline, and I wonder if the module name wouldn't be better as stock_picking_product_customer_code to follow current core conventions (more or less, you know that there is no written rules).

Regards.

Diff comments:

> === added directory 'product_customer_code_picking'
> === added file 'product_customer_code_picking/__init__.py'
> --- product_customer_code_picking/__init__.py	1970-01-01 00:00:00 +0000
> +++ product_customer_code_picking/__init__.py	2014-01-21 14:55:23 +0000
> @@ -0,0 +1,21 @@
> +# -*- coding: utf-8 -*-
> +##############################################################################
> +#
> +#    Copyright (C) 2013 Agile Business Group sagl (<http://www.agilebg.com>)
> +#    Author: Nicola Malcontenti <nicola.malcontenti@xxxxxxxxxxx>
> +#
> +#    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 stock_picking
> 
> === added file 'product_customer_code_picking/__openerp__.py'
> --- product_customer_code_picking/__openerp__.py	1970-01-01 00:00:00 +0000
> +++ product_customer_code_picking/__openerp__.py	2014-01-21 14:55:23 +0000
> @@ -0,0 +1,44 @@
> +# -*- coding: utf-8 -*-
> +##############################################################################
> +#
> +#    Copyright (C) 2013 Agile Business Group sagl (<http://www.agilebg.com>)
> +#    Author: Nicola Malcontenti <nicola.malcontenti@xxxxxxxxxxx>
> +#
> +#    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/>.
> +#
> +##############################################################################
> +{
> +    "name" : "Product Customer code for stock picking",
> +    "version" : "1.0",
> +    "author" : "Agile Business Group",
> +    "website" : "http://www.agilebg.com";,
> +    "category" : "Stock",
> +    "depends" : [
> +        'base',
> +        'product',
> +        'stock',
> +        'product_customer_code'
> +    ],
> +    "description": """
> +    Based on product_customer_code,
> +    this module loads in every stock picking
> +    the customer code defined in the product,
> +    """,
> +    "demo" : [],
> +    "data" : [
> +        'stock_picking_view.xml',
> +    ],
> +    'installable' : True,
> +    'active' : False,
> +}
> 
> === added directory 'product_customer_code_picking/i18n'
> === added file 'product_customer_code_picking/i18n/it.po'
> --- product_customer_code_picking/i18n/it.po	1970-01-01 00:00:00 +0000
> +++ product_customer_code_picking/i18n/it.po	2014-01-21 14:55:23 +0000
> @@ -0,0 +1,33 @@
> +# Translation of OpenERP Server.
> +# This file contains the translation of the following modules:
> +# 	* product_customer_code_picking
> +#
> +msgid ""
> +msgstr ""
> +"Project-Id-Version: OpenERP Server 7.0\n"
> +"Report-Msgid-Bugs-To: \n"
> +"POT-Creation-Date: 2014-01-15 14:28+0000\n"
> +"PO-Revision-Date: 2014-01-15 15:29+0100\n"
> +"Last-Translator: <>\n"
> +"Language-Team: \n"
> +"MIME-Version: 1.0\n"
> +"Content-Type: text/plain; charset=UTF-8\n"
> +"Content-Transfer-Encoding: 8bit\n"
> +"Plural-Forms: \n"
> +"Language: it\n"
> +"X-Generator: Poedit 1.6.2\n"
> +
> +#. module: product_customer_code_picking
> +#: field:stock.move,product_customer_code:0
> +msgid "Product Customer Code"
> +msgstr "Codice Prodotto Cliente"
> +
> +#. module: product_customer_code_picking
> +#: model:ir.model,name:product_customer_code_picking.model_stock_move
> +msgid "Stock Move"
> +msgstr "Movimento di magazzino"
> +
> +#. module: product_customer_code_picking
> +#: model:ir.model,name:product_customer_code_picking.model_stock_picking
> +msgid "Picking List"
> +msgstr "Documento di trasporto"
> 
> === added file 'product_customer_code_picking/i18n/product_customer_code_picking.pot'
> --- product_customer_code_picking/i18n/product_customer_code_picking.pot	1970-01-01 00:00:00 +0000
> +++ product_customer_code_picking/i18n/product_customer_code_picking.pot	2014-01-21 14:55:23 +0000
> @@ -0,0 +1,32 @@
> +# Translation of OpenERP Server.
> +# This file contains the translation of the following modules:
> +#	* product_customer_code_picking
> +#
> +msgid ""
> +msgstr ""
> +"Project-Id-Version: OpenERP Server 7.0\n"
> +"Report-Msgid-Bugs-To: \n"
> +"POT-Creation-Date: 2014-01-15 14:28+0000\n"
> +"PO-Revision-Date: 2014-01-15 14:28+0000\n"
> +"Last-Translator: <>\n"
> +"Language-Team: \n"
> +"MIME-Version: 1.0\n"
> +"Content-Type: text/plain; charset=UTF-8\n"
> +"Content-Transfer-Encoding: \n"
> +"Plural-Forms: \n"
> +
> +#. module: product_customer_code_picking
> +#: field:stock.move,product_customer_code:0
> +msgid "Product Customer Code"
> +msgstr ""
> +
> +#. module: product_customer_code_picking
> +#: model:ir.model,name:product_customer_code_picking.model_stock_move
> +msgid "Stock Move"
> +msgstr ""
> +
> +#. module: product_customer_code_picking
> +#: model:ir.model,name:product_customer_code_picking.model_stock_picking
> +msgid "Picking List"
> +msgstr ""
> +
> 
> === added file 'product_customer_code_picking/stock_picking.py'
> --- product_customer_code_picking/stock_picking.py	1970-01-01 00:00:00 +0000
> +++ product_customer_code_picking/stock_picking.py	2014-01-21 14:55:23 +0000
> @@ -0,0 +1,54 @@
> +# -*- coding: utf-8 -*-
> +##############################################################################
> +#
> +#    Copyright (C) 2013 Agile Business Group sagl (<http://www.agilebg.com>)
> +#    Author: Nicola Malcontenti <nicola.malcontenti@xxxxxxxxxxx>
> +#
> +#    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 openerp.osv import fields, orm
> +
> +
> +class stock_move(orm.Model):
> +    _inherit = 'stock.move'
> +
> +    def _get_product_customer_code(
> +            self, cr, uid, ids,
> +            name, args, context=None):
> +        if context is None:

You don't need to instantiate context, because it's not used in any place.

> +            context = {}
> +        res = {}
> +        product_customer_code_obj = self.pool.get('product.customer.code')

You can put self.pool['product.customer.code'] for better practices.

> +        for move in self.browse(cr, uid, ids, context=context):
> +            res[move.id] = ''
> +            partner = move.partner_id
> +            product = move.product_id
> +            if product and partner:
> +                code_ids = product_customer_code_obj.search(cr, uid, [
> +                    ('product_id', '=', product.id),
> +                    ('partner_id', '=', partner.id),
> +                    ], limit=1, context=context)
> +                if code_ids:
> +                    code = product_customer_code_obj.browse(

Better to use a read method to improve performance, due to there is only one field retrieved (product_code).

> +                        cr, uid,
> +                        code_ids[0], context=context).product_code or ''
> +                    res[move.id] = code
> +        return res
> +
> +    _columns = {
> +        'product_customer_code': fields.function(
> +            _get_product_customer_code,
> +            string='Product Customer Code', type='char', size=64),
> +    }
> 
> === added file 'product_customer_code_picking/stock_picking_view.xml'
> --- product_customer_code_picking/stock_picking_view.xml	1970-01-01 00:00:00 +0000
> +++ product_customer_code_picking/stock_picking_view.xml	2014-01-21 14:55:23 +0000
> @@ -0,0 +1,36 @@
> +<?xml version="1.0" encoding="utf-8"?>
> +<!--
> +  product customer code picking for OpenERP
> +  Copyright (C) 2013 Agile Business Group sagl (<http://www.agilebg.com>).
> +    Authors, Nicola Malcontenti, nicola.malcontenti@xxxxxxxxxxx
> +  The licence is in the file __openerp__.py
> +-->
> +<openerp>
> +    <data>
> +
> +        <record id="view_move_picking_tree" model="ir.ui.view">
> +            <field name="name">stock.move.product.code.tree</field>
> +            <field name="model">stock.move</field>
> +            <field name="inherit_id" ref="stock.view_move_picking_tree"/>
> +            <field eval="16" name="priority"/>
> +            <field name="arch" type="xml">
> +                <xpath expr="//field[@name='product_id']" position="after">

You can use <field name="product_id" position="after"> for better readibility if you want (although this is also correct).

> +                    <field name="product_customer_code" />
> +                </xpath>
> +            </field>
> +        </record>
> +
> +        <record id="view_move_picking_form" model="ir.ui.view">
> +            <field name="name">stock.move.product.code.form</field>
> +            <field name="model">stock.move</field>
> +            <field name="inherit_id" ref="stock.view_move_picking_form"/>
> +            <field eval="16" name="priority"/>
> +            <field name="arch" type="xml">
> +                <xpath expr="//field[@name='product_id']" position="after">
> +                    <field name="product_customer_code" />
> +                </xpath>
> +            </field>
> +        </record>
> +
> +    </data>
> +</openerp>
> 


-- 
https://code.launchpad.net/~agilebg/stock-logistic-flows/adding_product_customer_code_picking/+merge/202472
Your team Stock and Logistic Core Editors is subscribed to branch lp:stock-logistic-flows.