← Back to team overview

openerp-community-reviewer team mailing list archive

[Merge] lp:~camptocamp/sale-financial/7.0-port-sale_markup into lp:sale-financial

 

Yannick Vaucher @ Camptocamp has proposed merging lp:~camptocamp/sale-financial/7.0-port-sale_markup into lp:sale-financial.

Requested reviews:
  Sale Core Editors (sale-core-editors)

For more details, see:
https://code.launchpad.net/~camptocamp/sale-financial/7.0-port-sale_markup/+merge/214061

Portage of module sale_markup


Requires following module portages

https://code.launchpad.net/~camptocamp/sale-financial/7.0-port-sale_line_watcher/+merge/214058
https://code.launchpad.net/~camptocamp/sale-financial/7.0-port-sale_floor_price/+merge/214059
-- 
https://code.launchpad.net/~camptocamp/sale-financial/7.0-port-sale_markup/+merge/214061
Your team Sale Core Editors is requested to review the proposed merge of lp:~camptocamp/sale-financial/7.0-port-sale_markup into lp:sale-financial.
=== modified file 'sale_markup/__openerp__.py'
--- sale_markup/__openerp__.py	2013-11-14 08:16:22 +0000
+++ sale_markup/__openerp__.py	2014-04-03 15:35:21 +0000
@@ -23,19 +23,24 @@
  'author' : 'Camptocamp',
  'maintainer': 'Camptocamp',
  'category': 'version',
- 'complexity': "normal",  # easy, normal, expert
- 'depends' : ['base',
-              'product_get_cost_field',
-              'mrp',
-              'sale',
-              'sale_floor_price'],
- 'description': """display the product and sale markup in the appropriate views""",
+ 'complexity': "normal",
+ 'depends' : [
+     'base',
+     'product_get_cost_field',
+     'mrp',
+     'sale',
+     'sale_floor_price'],
+ 'description': """
+Markup rate on product and sales
+================================
+
+Display the product and sale markup in the appropriate views
+""",
  'website': 'http://www.camptocamp.com/',
- 'init_xml': [],
- 'update_xml': ['sale_view.xml', 'product_view.xml'],
- 'demo_xml': [],
- 'tests': [],
- 'installable': False,
+ 'data': ['sale_view.xml',
+          'product_view.xml'],
+ 'test': [],
+ 'installable': True,
  'auto_install': False,
  'license': 'AGPL-3',
  'application': True}

=== added file 'sale_markup/i18n/fr.po'
--- sale_markup/i18n/fr.po	1970-01-01 00:00:00 +0000
+++ sale_markup/i18n/fr.po	2014-04-03 15:35:21 +0000
@@ -0,0 +1,76 @@
+# Translation of OpenERP Server.
+# This file contains the translation of the following modules:
+#	* sale_markup
+#
+msgid ""
+msgstr ""
+"Project-Id-Version: OpenERP Server 7.0\n"
+"Report-Msgid-Bugs-To: \n"
+"POT-Creation-Date: 2014-04-03 14:11+0000\n"
+"PO-Revision-Date: 2014-04-03 16:32+0100\n"
+"Last-Translator: Yannick Vaucher <yannick.vaucher@xxxxxxxxxxxxxx>\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"
+
+#. module: sale_markup
+#: field:product.product,cost_price:0
+msgid "Cost Price"
+msgstr "Coût de revient"
+
+#. module: sale_markup
+#: field:sale.order.line,cost_price:0
+msgid "Historical Cost Price"
+msgstr "Coût de revient historique"
+
+#. module: sale_markup
+#: field:product.product,commercial_margin:0
+#: field:sale.order.line,commercial_margin:0
+msgid "Margin"
+msgstr "Marge"
+
+#. module: sale_markup
+#: help:product.product,commercial_margin:0
+msgid "Margin is [ sale_price - cost_price ] (not based on historical values)"
+msgstr "La marge est calculée ainsi [ prix_vente - prix_achat ] (n'est pas basé sur l'historique des valeurs)"
+
+#. module: sale_markup
+#: help:sale.order.line,commercial_margin:0
+msgid "Margin is [ sale_price - cost_price ], changing it will update the discount"
+msgstr "La marge est calculée ainsi [ prix_vente - prix_achat ], changer la marge adapte la remise."
+
+#. module: sale_markup
+#: help:sale.order.line,markup_rate:0
+msgid "Markup is [ margin / sale_price ], changing it will update the discount"
+msgstr "Le taux de marque est calculé ainsi [ marge / prix_vente ], changer le taux de marque adapte la remise."
+
+#. module: sale_markup
+#: view:sale.order:0
+#: field:sale.order,markup_rate:0
+#: field:sale.order.line,markup_rate:0
+#: field:product.product,markup_rate:0
+msgid "Markup"
+msgstr "Taux de marque"
+
+#. module: sale_markup
+#: help:product.product,markup_rate:0
+msgid "Markup is [ margin / sale_price ] (not based on historical values)"
+msgstr "Le taux de marque est [ marge / prix_vente ] (n'est pas basé sur l'historique des valeurs)"
+
+#. module: sale_markup
+#: help:product.product,cost_price:0
+msgid "The cost price is the standard price unless you install the product_cost_incl_bom module."
+msgstr "Le coût de revient est le prix standard à moins que le module product_cost_incl_bom ne soit installé."
+
+#. module: sale_markup
+#: help:sale.order.line,cost_price:0
+msgid "The cost price of the product at the time of the creation of the sale order"
+msgstr "Le coût de revient du produit au moment de la création de la commande client."
+
+#. module: sale_markup
+#: view:sale.order:0
+msgid "product_id_change(parent.pricelist_id,product_id,product_uom_qty,product_uom,product_uos_qty,product_uos,name,parent.partner_id,False,True,parent.date_order,product_packaging,parent.fiscal_position,False,price_unit,discount,context)"
+msgstr "product_id_change(parent.pricelist_id,product_id,product_uom_qty,product_uom,product_uos_qty,product_uos,name,parent.partner_id,False,True,parent.date_order,product_packaging,parent.fiscal_position,False,price_unit,discount,context)"
+

=== added file 'sale_markup/i18n/sale_markup.pot'
--- sale_markup/i18n/sale_markup.pot	1970-01-01 00:00:00 +0000
+++ sale_markup/i18n/sale_markup.pot	2014-04-03 15:35:21 +0000
@@ -0,0 +1,76 @@
+# Translation of OpenERP Server.
+# This file contains the translation of the following modules:
+#	* sale_markup
+#
+msgid ""
+msgstr ""
+"Project-Id-Version: OpenERP Server 7.0\n"
+"Report-Msgid-Bugs-To: \n"
+"POT-Creation-Date: 2014-04-03 14:11+0000\n"
+"PO-Revision-Date: 2014-04-03 14:11+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: sale_markup
+#: field:product.product,cost_price:0
+msgid "Cost Price"
+msgstr ""
+
+#. module: sale_markup
+#: field:sale.order.line,cost_price:0
+msgid "Historical Cost Price"
+msgstr ""
+
+#. module: sale_markup
+#: field:product.product,commercial_margin:0
+#: field:sale.order.line,commercial_margin:0
+msgid "Margin"
+msgstr ""
+
+#. module: sale_markup
+#: help:product.product,commercial_margin:0
+msgid "Margin is [ sale_price - cost_price ] (not based on historical values)"
+msgstr ""
+
+#. module: sale_markup
+#: help:sale.order.line,commercial_margin:0
+msgid "Margin is [ sale_price - cost_price ], changing it will update the discount"
+msgstr ""
+
+#. module: sale_markup
+#: help:sale.order.line,markup_rate:0
+msgid "Markup is [ margin / sale_price ], changing it will update the discount"
+msgstr ""
+
+#. module: sale_markup
+#: view:sale.order:0
+#: field:sale.order,markup_rate:0
+#: field:sale.order.line,markup_rate:0
+#: field:product.product,markup_rate:0
+msgid "Markup"
+msgstr ""
+
+#. module: sale_markup
+#: help:product.product,markup_rate:0
+msgid "Markup is [ margin / sale_price ] (not based on historical values)"
+msgstr ""
+
+#. module: sale_markup
+#: help:product.product,cost_price:0
+msgid "The cost price is the standard price unless you install the product_cost_incl_bom module."
+msgstr ""
+
+#. module: sale_markup
+#: help:sale.order.line,cost_price:0
+msgid "The cost price of the product at the time of the creation of the sale order"
+msgstr ""
+
+#. module: sale_markup
+#: view:sale.order:0
+msgid "product_id_change(parent.pricelist_id,product_id,product_uom_qty,product_uom,product_uos_qty,product_uos,name,parent.partner_id,False,True,parent.date_order,product_packaging,parent.fiscal_position,False,price_unit,discount,context)"
+msgstr ""
+

=== modified file 'sale_markup/product_markup.py'
--- sale_markup/product_markup.py	2013-04-30 12:33:48 +0000
+++ sale_markup/product_markup.py	2014-04-03 15:35:21 +0000
@@ -29,72 +29,91 @@
 class Product(Model):
     _inherit = 'product.product'
 
-    def _convert_to_foreign_currency(self, cursor, user, pricelist, amount_dict, context=None):
-        if context is None:
-            context = {}
+    def _convert_to_foreign_currency(self, cr, uid, pricelist,
+                                     amount_dict, context=None):
+        """
+        Apply purchase pricelist
+        """
         if not pricelist:
             return amount_dict
-        pricelist_obj = self.pool.get('product.pricelist')
-        currency_obj = self.pool.get('res.currency')
-        user_obj = self.pool.get('res.users')
-        pricelist = pricelist_obj.browse(cursor, user, pricelist)
-        company_currency_id = user_obj.browse(cursor, user, user).company_id.currency_id.id
-        converted_price = {}
+        pricelist_obj = self.pool['product.pricelist']
+        currency_obj = self.pool['res.currency']
+        user_obj = self.pool['res.users']
+        pricelist = pricelist_obj.browse(cr, uid, pricelist, context=context)
+        user = user_obj.browse(cr, uid, uid, context=context)
+        company_currency_id = user.company_id.currency_id.id
+        converted_prices = {}
         for product_id, amount in amount_dict.iteritems():
-            converted_price[product_id] = currency_obj.compute(cursor, user,
-                                                               company_currency_id,
-                                                               pricelist.currency_id.id,
-                                                               amount,
-                                                               round=False)
-        return converted_price
-
-    def compute_markup(self, cursor, user, ids,
-                       product_uom = None,
-                       pricelist   = None,
-                       sale_price  = None,
-                       properties  = None,
-                       context     = None):
-        '''
+            converted_prices[product_id] = currency_obj.compute(
+                cr, uid, company_currency_id, pricelist.currency_id.id, amount,
+                round=False)
+        return converted_prices
+
+    @staticmethod
+    def _compute_markup(sale_price, purchase_price):
+        """
+        Return markup as a rate
+
+        Markup = SP - PP / SP
+
+        Where SP = Sale price
+              PP = Purchase price
+        """
+        if not sale_price:
+            return 0.0
+        return sale_price - purchase_price / sale_price * 100
+
+    def compute_markup(self, cr, uid, ids,
+                       product_uom=None, pricelist=None, sale_price=None,
+                       properties=None, context=None):
+        """
         compute markup
 
         If properties, pricelist and sale_price arguments are set, it
         will be used to compute all results
-        '''
+        """
         properties = properties or []
         pricelist = pricelist or []
         if context is None:
             context = {}
         if isinstance(ids, (int, long)):
-            ids =  [ids]
+            ids = [ids]
         res = {}
 
-        # cost_price_context will be used by product_get_cost_field if it is installed
-        cost_price_context = context.copy().update({'produc_uom': product_uom,
-                                                    'properties': properties})
-        purchase_prices = self.get_cost_field(cursor, user, ids, cost_price_context)
+        # cost_price_context will be used by product_get_cost_field if it is
+        # installed
+        cost_price_context = context.copy()
+        cost_price_context.update({
+            'produc_uom': product_uom,
+            'properties': properties})
+        purchase_prices = self.get_cost_field(cr, uid, ids, cost_price_context)
         # if purchase prices failed returned a dict of default values
-        if not purchase_prices: return dict([(id, {'commercial_margin': 0.0,
-                                                   'markup_rate': 0.0,
-                                                   'cost_price': 0.0,}) for id in ids])
+        if not purchase_prices:
+            return dict([(id, {'commercial_margin': 0.0,
+                               'markup_rate': 0.0,
+                               'cost_price': 0.0,
+                               }) for id in ids])
 
-        purchase_price = self._convert_to_foreign_currency(cursor, user, pricelist, purchase_prices)
-        for pr in self.browse(cursor, user, ids):
+        purchase_prices = self._convert_to_foreign_currency(cr, uid, pricelist,
+                                                            purchase_prices,
+                                                            context=context)
+        for pr in self.browse(cr, uid, ids, context=context):
             res[pr.id] = {}
             if sale_price is None:
                 catalog_price = pr.list_price
             else:
                 catalog_price = sale_price
 
-            res[pr.id]['commercial_margin'] = catalog_price - purchase_prices[pr.id]
-
-            res[pr.id]['markup_rate'] = (catalog_price and
-                                         (catalog_price - purchase_prices[pr.id]) / catalog_price * 100 or 0.0)
-
-            res[pr.id]['cost_price'] = purchase_prices[pr.id]
+            res[pr.id].update({
+                'commercial_margin': catalog_price - purchase_prices[pr.id],
+                'markup_rate': self._compute_markup(catalog_price,
+                                                    purchase_prices[pr.id]),
+                'cost_price': purchase_prices[pr.id]
+            })
 
         return res
 
-    def _get_bom_product(self,cursor, user, ids, context=None):
+    def _get_bom_product(self, cr, uid, ids, context=None):
         """return ids of modified product and ids of all product that use
         as sub-product one of this ids. Ex:
         BoM :
@@ -103,63 +122,61 @@
                 -   Product C
         => If we change standard_price of product B, we want to update Product
         A as well..."""
-        if context is None:
-            context = {}
         def _get_parent_bom(bom_record):
             """Recursvely find the parent bom"""
-            result=[]
+            result = []
             if bom_record.bom_id:
                 result.append(bom_record.bom_id.id)
                 result.extend(_get_parent_bom(bom_record.bom_id))
             return result
         res = []
-        bom_obj = self.pool.get('mrp.bom')
-        bom_ids = bom_obj.search(cursor, user, [('product_id','in',ids)])
-        for bom in bom_obj.browse(cursor, user, bom_ids):
+        bom_obj = self.pool['mrp.bom']
+        bom_ids = bom_obj.search(cr, uid, [('product_id', 'in', ids)],
+                                 context=context)
+        for bom in bom_obj.browse(cr, uid, bom_ids, context=context):
             res += _get_parent_bom(bom)
         final_bom_ids = list(set(res + bom_ids))
-        return list(set(ids + self._get_product(cursor, user, final_bom_ids, context)))
+        return list(set(ids + self._get_product(cr, uid, final_bom_ids,
+                                                context=context)))
 
-    def _get_product(self, cursor, user, ids, context = None):
-        if context is None:
-            context = {}
-        bom_obj = self.pool.get('mrp.bom')
+    def _get_product(self, cr, uid, ids, context=None):
+        bom_obj = self.pool['mrp.bom']
 
         res = {}
-        for bom in bom_obj.browse(cursor, user, ids, context=context):
+        for bom in bom_obj.browse(cr, uid, ids, context=context):
             res[bom.product_id.id] = True
         return res.keys()
 
-    def _compute_all_markup(self, cursor, user, ids, field_name, arg, context = None):
-        '''
+    def _compute_all_markup(self, cr, uid, ids, field_name, arg,
+                            context=None):
+        """
         method for product function field on multi 'markup'
-        '''
-        if context is None:
-            context = {}
-        res = self.compute_markup(cursor, user, ids, context=context)
-        return res
+        """
+        return self.compute_markup(cr, uid, ids, context=context)
 
-    _store_cfg = {'product.product': (_get_bom_product, ['list_price' ,'standard_price'], 20),
+    _store_cfg = {'product.product': (_get_bom_product,
+                                      ['list_price', 'standard_price'], 20),
                   'mrp.bom': (_get_product,
-                             ['bom_id', 'bom_lines', 'product_id', 'product_uom',
-                             'product_qty', 'product_uos', 'product_uos_qty',
-                             'property_ids'], 20)}
-
-
+                              ['bom_id', 'bom_lines', 'product_id',
+                               'product_uom', 'product_qty', 'product_uos',
+                               'product_uos_qty', 'property_ids'], 20)
+                  }
 
     _columns = {
-        'commercial_margin' : fields.function(_compute_all_markup,
-                                              method=True,
-                                              string='Margin',
-                                              digits_compute=dp.get_precision('Sale Price'),
-                                              store =_store_cfg,
-                                              multi ='markup',
-                                              help='Margin is [ sale_price - cost_price ] (not based on historical values)'),
-        'markup_rate' : fields.function(_compute_all_markup,
-                                        method=True,
-                                        string='Markup rate (%)',
-                                        digits_compute=dp.get_precision('Sale Price'),
-                                        store=_store_cfg,
-                                        multi='markup',
-                                        help='Markup rate is [ margin / sale_price ] (not based on historical values)'),
-        }
+        'commercial_margin': fields.function(
+            _compute_all_markup,
+            string='Margin',
+            digits_compute=dp.get_precision('Sale Price'),
+            store=_store_cfg,
+            multi='markup',
+            help='Margin is [ sale_price - cost_price ] (not based on '
+                 'historical values)'),
+        'markup_rate': fields.function(
+            _compute_all_markup,
+            string='Markup',
+            digits_compute=dp.get_precision('Sale Price'),
+            store=_store_cfg,
+            multi='markup',
+            help='Markup is [ margin / sale_price ] (not based on '
+                 'historical values)'),
+    }

=== modified file 'sale_markup/product_view.xml'
--- sale_markup/product_view.xml	2012-05-15 15:56:19 +0000
+++ sale_markup/product_view.xml	2014-04-03 15:35:21 +0000
@@ -3,28 +3,26 @@
   <data>
     <record model="ir.ui.view" id="sale_markup_product_form">
       <field name="name">product.markup.view.form</field>
-      <field name="type">form</field>
       <field name="model">product.product</field>
       <field name="inherit_id" ref="product.product_normal_form_view" />
       <field name="arch" type="xml">
         <field name="list_price" position="after">
           <field name="markup_rate"
-                 groups="sale.group_sale_manager"/>
+                 groups="base.group_sale_manager"/>
           <field name="commercial_margin"
-                 groups="sale.group_sale_manager"/>
+                 groups="base.group_sale_manager"/>
         </field>
       </field>
     </record>
 
     <record model="ir.ui.view" id="sale_markup_product_tree">
       <field name="name">product.markup.view.tree</field>
-      <field name="type">tree</field>
       <field name="model">product.product</field>
       <field name="inherit_id" ref="product.product_product_tree_view" />
       <field name="arch" type="xml">
         <field name="standard_price" position="after">
           <field name="markup_rate"
-                 groups="sale.group_sale_manager"/>
+                 groups="base.group_sale_manager"/>
         </field>
       </field>
     </record>

=== modified file 'sale_markup/sale_markup.py'
--- sale_markup/sale_markup.py	2012-07-12 13:56:17 +0000
+++ sale_markup/sale_markup.py	2014-04-03 15:35:21 +0000
@@ -24,200 +24,216 @@
 from openerp.osv.orm import Model, fields
 import decimal_precision as dp
 
+
 def _prec(obj, cr, uid, mode=None):
     # This function use orm cache it should be efficient
     mode = mode or 'Sale Price'
-    return obj.pool.get('decimal.precision').precision_get(cr, uid, mode)
-
-class SaleOrder(Model):
+    return obj.pool['decimal.precision'].precision_get(cr, uid, mode)
+
+
+class SalesOrder(Model):
     _inherit = 'sale.order'
 
-    def _amount_all(self, cursor, user, ids, field_name, arg, context = None):
-        '''Calculate the markup rate based on sums'''
-
-        if context is None:
-            context = {}
-        res = {}
-        res = super(SaleOrder, self)._amount_all(cursor, user, ids, field_name, arg, context)
-
-        for sale_order in self.browse(cursor, user, ids):
+    def _amount_all(self, cr, user, ids, field_name, arg, context=None):
+        """Calculate the markup rate based on sums"""
+
+        res = super(SalesOrder, self
+                    )._amount_all(cr, user, ids, field_name, arg,
+                                  context=context)
+
+        for sale_order in self.browse(cr, user, ids):
             cost_sum = 0.0
             sale_sum = 0.0
             for line in sale_order.order_line:
                 cost_sum += line.cost_price
                 sale_sum += line.price_unit * (100 - line.discount) / 100.0
-            res[sale_order.id]['markup_rate'] = sale_sum and (sale_sum - cost_sum) / sale_sum * 100 or 0.0
+            markup_rate = ((sale_sum - cost_sum) / sale_sum * 100 if sale_sum
+                           else 0.0)
+            res[sale_order.id]['markup_rate'] = markup_rate
         return res
 
-
     def _get_order(self, cr, uid, ids, context=None):
-        if context is None:
-            context = {}
+        sale_order_line_obj = self.pool['sale.order.line']
+        sale_order_lines = sale_order_line_obj.browse(cr, uid, ids,
+                                                      context=context)
         result = set()
-        for line in self.pool.get('sale.order.line').browse(cr, uid, ids, context=context):
+        for line in sale_order_lines:
             result.add(line.order_id.id)
         return list(result)
 
     _store_sums = {
-        'sale.order': (lambda self, cr, uid, ids, c={}: ids, ['order_line'], 10),
-        'sale.order.line': (_get_order, ['price_unit', 'tax_id', 'discount', 'product_uom_qty',
-                'product_id','commercial_margin', 'markup_rate'], 10)}
-
-
-    _columns = {'markup_rate': fields.function(_amount_all,
-                                               method = True,
-                                               string = 'Markup Rate',
-                                               digits_compute=dp.get_precision('Sale Price'),
-                                               store = _store_sums,
-                                               multi='sums')}
-
-
-class SaleOrderLine(Model):
+        'sale.order': (lambda self, cr, uid, ids, c={}: ids,
+                       ['order_line'], 10),
+        'sale.order.line': (_get_order,
+                            ['price_unit', 'tax_id', 'discount',
+                             'product_uom_qty', 'product_id',
+                             'commercial_margin', 'markup_rate'], 10)
+    }
+
+    _columns = {
+        'markup_rate': fields.function(
+            _amount_all,
+            string='Markup',
+            digits_compute=dp.get_precision('Sale Price'),
+            store=_store_sums,
+            multi='sums'),
+    }
+
+
+class SalesOrderLine(Model):
 
     _inherit = 'sale.order.line'
 
-    _columns = {'commercial_margin': fields.float('Margin',
-                                                digits_compute=dp.get_precision('Sale Price'),
-                                                help='Margin is [ sale_price - cost_price ],'
-                                                     ' changing it will update the discount'),
-                'markup_rate': fields.float('Markup Rate (%)',
-                                            digits_compute=dp.get_precision('Sale Price'),
-                                            help='Margin rate is [ margin / sale_price ],'
-                                                 'changing it will update the discount'),
-                'cost_price': fields.float('Historical Cost Price',
-                                              digits_compute=dp.get_precision('Sale Price'),
-                                              help="The cost price of the product at the time of the creation of the sale order"),
-                 }
-             
-
-
-    def onchange_price_unit(self, cursor, uid, ids, price_unit, product_id, discount,
-                            product_uom, pricelist, **kwargs):
-        '''
-        If price unit change, compute the new markup rate.
-        '''
-        res = super(SaleOrderLine,self).onchange_price_unit(cursor, uid, ids,
-                                                            price_unit,
-                                                            product_id,
-                                                            discount,
-                                                            product_uom,
-                                                            pricelist)
-
+    _columns = {
+        'commercial_margin': fields.float(
+            'Margin',
+            digits_compute=dp.get_precision('Sale Price'),
+            help='Margin is [ sale_price - cost_price ], changing it will '
+                 'update the discount'),
+        'markup_rate': fields.float(
+            'Markup',
+            digits_compute=dp.get_precision('Sale Price'),
+            help='Markup is [ margin / sale_price ], changing it will '
+                 'update the discount'),
+        'cost_price': fields.float(
+            'Historical Cost Price',
+            digits_compute=dp.get_precision('Sale Price'),
+            help='The cost price of the product at the time of the creation '
+                 'of the sale order'),
+    }
+
+    def onchange_price_unit(self, cr, uid, ids, price_unit, product_id,
+                            discount, product_uom, pricelist, **kwargs):
+        """
+        If price unit changes, compute the new markup rate and
+        commercial margin
+        """
+        res = super(SaleOrderLine, self
+                    ).onchange_price_unit(cr, uid, ids, price_unit, product_id,
+                                          discount, product_uom, pricelist)
         if product_id:
-            product_obj = self.pool.get('product.product')
-            if res['value'].has_key('price_unit'):
+            product_obj = self.pool['product.product']
+            if 'price_unit' in res['value']:
                 price_unit = res['value']['price_unit']
             sale_price = price_unit * (100 - discount) / 100.0
-            markup_res = product_obj.compute_markup(cursor, uid,
+            markup_res = product_obj.compute_markup(cr, uid,
                                                     product_id,
                                                     product_uom,
                                                     pricelist,
                                                     sale_price)[product_id]
 
-
-            res['value']['commercial_margin'] = round(markup_res['commercial_margin'], _prec(self, cursor, uid))
-            res['value']['markup_rate'] = round(markup_res['markup_rate'],  _prec(self, cursor, uid))
+            res['value'].update({
+                'commercial_margin': round(
+                    markup_res['commercial_margin'], _prec(self, cr, uid)),
+                'markup_rate': round(
+                    markup_res['markup_rate'], _prec(self, cr, uid))
+            })
         return res
 
-
-
-    def onchange_discount(self, cursor, uid, ids,
-                          price_unit, product_id, discount, product_uom, pricelist, **kwargs):
-        '''
-        If discount change, compute the new markup rate
-        '''
-        res = super(SaleOrderLine,self).onchange_discount(cursor, uid, ids,
-                                                          price_unit,
-                                                          product_id,
-                                                          discount,
-                                                          product_uom,
-                                                          pricelist)
-        
+    def onchange_discount(self, cr, uid, ids,
+                          price_unit, product_id, discount, product_uom,
+                          pricelist, **kwargs):
+        """
+        If discount changes, compute the new markup rate and commercial margin.
+        """
+        res = super(SaleOrderLine, self
+                    ).onchange_discount(cr, uid, ids,
+                                        price_unit, product_id, discount,
+                                        product_uom, pricelist)
         if product_id:
-            product_obj = self.pool.get('product.product')
-            if res['value'].has_key('price_unit'):
+            product_obj = self.pool['product.product']
+            if 'price_unit' in res['value']:
                 price_unit = res['value']['price_unit']
-            if res['value'].has_key('discount'):
+            if 'discount' in res['value']:
                 discount = res['value']['discount']
             sale_price = price_unit * (100 - discount) / 100.0
-            markup_res = product_obj.compute_markup(cursor, uid,
+            markup_res = product_obj.compute_markup(cr, uid,
                                                     product_id,
                                                     product_uom,
                                                     pricelist,
                                                     sale_price)[product_id]
 
-
-            res['value']['commercial_margin'] = round(markup_res['commercial_margin'] , _prec(self, cursor, uid))
-            res['value']['markup_rate'] = round(markup_res['markup_rate'],  _prec(self, cursor, uid))
+            res['value'].update({
+                'commercial_margin': round(
+                    markup_res['commercial_margin'], _prec(self, cr, uid)),
+                'markup_rate': round(
+                    markup_res['markup_rate'], _prec(self, cr, uid))
+            })
         return res
 
-
-    def product_id_change(self, cursor, uid, ids, pricelist, product, qty=0,
-                          uom=False, qty_uos=0, uos=False, name='', partner_id=False,
-                          lang=False, update_tax=True, date_order=False, packaging=False,
-                          fiscal_position=False, flag=False, discount=None, price_unit=None, context=None):
-        '''
+    def product_id_change(self, cr, uid, ids, pricelist, product, qty=0,
+                          uom=False, qty_uos=0, uos=False, name='',
+                          partner_id=False, lang=False, update_tax=True,
+                          date_order=False, packaging=False,
+                          fiscal_position=False, flag=False, discount=None,
+                          price_unit=None, context=None):
+        """
         Overload method
-        If product change, compute the new markup.
+        If product changes, compute the new markup, cost_price and
+        commercial_margin.
         Added params : - price_unit,
                        - discount
-                       - properties
-        '''
-        if context is None:
-            context = {}
+        """
         discount = discount or 0.0
         price_unit = price_unit or 0.0
-        res = {}
-        res = super(SaleOrderLine, self).product_id_change(cursor, uid, ids, pricelist, product, qty,
-                                                           uom, qty_uos, uos, name, partner_id,
-                                                           lang, update_tax, date_order, packaging,
-                                                           fiscal_position, flag, context)
-
+        res = super(SalesOrderLine, self
+                    ).product_id_change(cr, uid, ids, pricelist, product, qty,
+                                        uom, qty_uos, uos, name, partner_id,
+                                        lang, update_tax, date_order,
+                                        packaging, fiscal_position, flag,
+                                        context=context)
         if product:
-            if res['value'].has_key('price_unit'):
+            if 'price_unit' in res['value']:
                 price_unit = res['value']['price_unit']
             sale_price = price_unit * (100 - discount) / 100.0
 
-            product_obj = self.pool.get('product.product')
-            markup_res = product_obj.compute_markup(cursor, uid,
+            product_obj = self.pool['product.product']
+            markup_res = product_obj.compute_markup(cr, uid,
                                                     product,
                                                     uom,
                                                     pricelist,
                                                     sale_price)[product]
 
-            res['value']['commercial_margin'] = round(markup_res['commercial_margin'],  _prec(self, cursor, uid))
-            res['value']['markup_rate'] = round(int(markup_res['markup_rate'] * 100) / 100.0, _prec(self, cursor, uid))
-            res['value']['cost_price'] = round(markup_res['cost_price'],  _prec(self, cursor, uid))
+            res['value'].update({
+                'commercial_margin': round(
+                    markup_res['commercial_margin'], _prec(self, cr, uid)),
+                'markup_rate': round(
+                    int(markup_res['markup_rate'] * 100) / 100.0,
+                    _prec(self, cr, uid)),
+                'cost_price': round(
+                    markup_res['cost_price'], _prec(self, cr, uid))
+            })
 
         return res
 
-
-    def onchange_markup_rate(self, cursor, uid, ids,
+    def onchange_markup_rate(self, cr, uid, ids,
                              markup, cost_price, price_unit, context=None):
-        ''' If markup rate change compute the discount '''
-        if context is None:
-            context = {}
-        res = {}
-        res['value'] = {}
+        """ If markup rate changes compute the discount """
+        res = {'value': {}}
         markup = markup / 100.0
-        if not price_unit or markup == 1: return {'value': {}}
-        discount = 1 + cost_price / (markup - 1) / price_unit
-        sale_price = price_unit * (1 - discount)
-        res['value']['discount'] =  round(discount * 100,  _prec(self, cursor, uid))
-        res['value']['commercial_margin'] = round(sale_price - cost_price, _prec(self, cursor, uid))
+        if price_unit and not markup == 1:
+            discount = 1 + cost_price / (markup - 1) / price_unit
+            sale_price = price_unit * (1 - discount)
+            res['value'].update({
+                'discount': round(
+                    discount * 100,  _prec(self, cr, uid)),
+                'commercial_margin': round(
+                    sale_price - cost_price, _prec(self, cr, uid))
+            })
         return res
 
-
-    def onchange_commercial_margin(self, cursor, uid, ids,
-                                   margin, cost_price, price_unit, context=None):
-        ''' If markup rate change compute the discount '''
-        if context is None:
-            context = {}
-        res = {}
-        res['value'] = {}
-        if not price_unit: return {'value': {}}
-        discount = 1 - ((cost_price + margin) / price_unit)
-        sale_price = price_unit * (1 - discount)
-        res['value']['discount'] = round(discount * 100,  _prec(self, cursor, uid))
-        res['value']['markup_rate'] = round(margin / (sale_price or 1.0) * 100, _prec(self, cursor, uid))
+    def onchange_commercial_margin(self, cr, uid, ids,
+                                   margin, cost_price, price_unit,
+                                   context=None):
+        """ If commercial margin changes compute the discount """
+        res = {'value': {}}
+        if price_unit:
+            discount = 1 - ((cost_price + margin) / price_unit)
+            sale_price = price_unit * (1 - discount)
+            res['value'].update({
+                'discount': round(
+                    discount * 100,  _prec(self, cr, uid)),
+                'markup_rate': round(
+                    margin / (sale_price or 1.0) * 100, _prec(self, cr, uid))
+            })
         return res

=== modified file 'sale_markup/sale_view.xml'
--- sale_markup/sale_view.xml	2012-07-23 12:01:18 +0000
+++ sale_markup/sale_view.xml	2014-04-03 15:35:21 +0000
@@ -7,13 +7,15 @@
     <!-- Add markup rate of the order in form view after Compute button -->
     <record model="ir.ui.view" id="sale_markup_sale_order_view">
       <field name="name">sale.order.markup.view.form</field>
-      <field name="type">form</field>
       <field name="model">sale.order</field>
       <field name="inherit_id" ref="sale.view_order_form" />
       <field name="arch" type="xml">
-        <xpath expr="//button[@name='button_dummy']" position="before">
-          <field name="markup_rate"
-                 groups="base.group_sale_manager"/>
+        <xpath expr="//group[@name='sale_total']" position="after">
+          <group class="oe_subtotal_footer oe_right" colspan="2" name="sale_markup"
+                 groups="base.group_sale_manager">
+            <field name="markup_rate"
+                   groups="base.group_sale_manager"/>
+          </group>
         </xpath>
       </field>
     </record>
@@ -21,7 +23,6 @@
     <!-- Add markup rate of the order in tree view after state field -->
     <record model="ir.ui.view" id="sale_markup_sale_order_tree">
       <field name="name">sale.order.markup.view.tree</field>
-      <field name="type">tree</field>
       <field name="model">sale.order</field>
       <field name="inherit_id" ref="sale.view_order_tree" />
       <field name="arch" type="xml">
@@ -34,7 +35,6 @@
 
     <record model="ir.ui.view" id="sale_markup_sale_order_line_tree">
       <field name="name">sale.order.line.markup.view.tree</field>
-      <field name="type">tree</field>
       <field name="model">sale.order.line</field>
       <field name="inherit_id" ref="sale.view_order_line_tree" />
       <field name="arch" type="xml">
@@ -52,25 +52,20 @@
     <!-- Add onchanges on unit price and discount to update margin and markup -->
     <record model="ir.ui.view" id="sale_markup_sale_order_form_line_form">
       <field name="name">sale.order.markup.view.form2</field>
-      <field name="type">form</field>
       <field name="model">sale.order</field>
       <field name="inherit_id" ref="sale.view_order_form" />
       <field name="arch" type="xml">
         <xpath expr="//field[@name='product_id']"
-               position="replace">
-            <field colspan="4"
-                   name="product_id"
-                   context="{'partner_id':parent.partner_id, 'quantity':product_uom_qty, 'pricelist':parent.pricelist_id, 'shop':parent.shop_id, 'uom':product_uom}"
-                   on_change="product_id_change(parent.pricelist_id,product_id,product_uom_qty,product_uom,product_uos_qty,product_uos,name,parent.partner_id, False, True, parent.date_order, product_packaging, parent.fiscal_position, False, price_unit, discount, context)" />
-            <newline/>
-        </xpath>
-        <xpath expr="/form/notebook/page/field[@name='order_line']/form/notebook/page/group/field[@name='name']"
-               position="attributes">
-            <attribute name="colspan">5</attribute>
-        </xpath>
-        <xpath expr="/form/notebook/page/field[@name='order_line']/form/notebook/page/group/field[@name='name']"
-               position="after">
-            <separator string="Markup" colspan="5" groups="base.group_sale_manager"/>
+          position="replace">
+          <field name="product_id"
+                 context="{'partner_id':parent.partner_id, 'quantity':product_uom_qty, 'pricelist':parent.pricelist_id, 'shop':parent.shop_id, 'uom':product_uom}"
+                 groups="base.group_user"
+                 on_change="product_id_change(parent.pricelist_id, product_id, product_uom_qty, product_uom, product_uos_qty, product_uos, name, parent.partner_id, False, True, parent.date_order, product_packaging, parent.fiscal_position, False, price_unit, discount, context)" />
+          <newline/>
+        </xpath>
+        <xpath expr="//field[@name='order_line']/form//field[@name='name']"
+          position="after">
+          <group string="Markup" groups="base.group_sale_manager">
             <field name="commercial_margin"
                    on_change="onchange_commercial_margin(commercial_margin, cost_price, price_unit)"
                    groups="base.group_sale_manager"/>
@@ -79,10 +74,10 @@
                    groups="base.group_sale_manager"/>
             <field name="cost_price"
                    groups="base.group_sale_manager" invisible="1"/>
-            <newline/>
+          </group>
         </xpath>
         <xpath expr="//field[@name='product_uom_qty']" position="attributes">
-            <attribute name="on_change">product_id_change(parent.pricelist_id,product_id,product_uom_qty,product_uom,product_uos_qty,product_uos,name,parent.partner_id,False,True,parent.date_order,product_packaging,parent.fiscal_position,False,price_unit,discount,context)</attribute>
+          <attribute name="on_change">product_id_change(parent.pricelist_id,product_id,product_uom_qty,product_uom,product_uos_qty,product_uos,name,parent.partner_id,False,True,parent.date_order,product_packaging,parent.fiscal_position,False,price_unit,discount,context)</attribute>
         </xpath>
       </field>
     </record>
@@ -90,12 +85,11 @@
     <!-- Add Markup in Sales Orders form's Sale order lines tree -->
     <record model="ir.ui.view" id="sale_markup_sale_order_form_line_tree">
       <field name="name">sale.order.markup.view.form</field>
-      <field name="type">form</field>
       <field name="model">sale.order</field>
       <field name="inherit_id" ref="sale.view_order_form" />
       <field name="arch" type="xml">
         <xpath expr="//field[@name='order_line']/tree/field[@name='price_subtotal']"
-               position="after">
+          position="after">
           <field name="cost_price"
                  groups="base.group_sale_manager"/>
           <field name="markup_rate"


Follow ups