Merge lp:~openerp-dev/openobject-addons/trunk-bug-707287-aag into lp:openobject-addons

[Atik Agewan(OpenERP)]

 Hello,

  Product :User can not change the existing UOM category ,Old Uom and New Uom category should be same.

 Thanks,
  Atik

[Rucha (Open ERP) (rpa-openerp)]

please improve the code,

29 + product = self.pool.get('product.product')
30 + uom_obj = self.pool.get('product.uom')
31 + product_uom1 = product.browse(cr, uid, ids)[0]
32 + category_1 = uom_obj.browse(cr, uid, product_uom1.uom_id.id).category_id

you are in product class, can't you simply do it by self instead of self.pool.get(..)?
you don't need to use uom_obj,
product_uom1.uom_id.category_id will give you the category,
also make sure to use proper variable name, product_uom1 doesn't suits for browse record for product,
simplify the other code please,

thanks

review: Needs Fixing

[Atik Agewan(OpenERP) (aag-openerp)]

 Hello,

  Changes is Done as per your suggestion.
  But we need uom_obj in product write method to browse new uom's category
  like ..
     new_category = uom_obj.browse(cr, uid, vals['uom_po_id']).category_id

 Thanks,
  Atik

review: Needs Resubmitting

[Rucha (Open ERP) (rpa-openerp)]

I have made improvement in code,
Thanks

review: Approve

[qdp (OpenERP) (qdp)]

Hi,

line 22: update_check=True? really? what for? a wrong copy/paste?
line 25: new_uom definition can be remvoed from the loop and placed two lines above
lines 22 to 30: i don't know if you noticed it, but the same problem could appear if we change the default uom of a product that already has stock moves.. but fortunately there is a constraint that ensure that the uom and the purchase uom of a product belong to the same group, so it will never be the case and this test is enough. Still, as explicit is better that implicit, you should check that case also in your code.
e.g: the line 23 should be replaced by something like
 if 'uom_po_id' in vals or 'uom_id' in vals:
 ...

Thanks

review: Needs Fixing

[Rucha (Open ERP) (rpa-openerp)]

I have checked all possible cases, for a product, changing both uom_id and uom_po_id, only uom_po_id , only uom_id, and its working fine with checking only "if 'uom_po_id' in vals" without any fail (the reason is changing uom_id will change uom_po_id too, and if not there will be difference in categ of current uom_id/uom_po_id, so again it will raise constraint error),
can you check again please, or I am missing something?
thanks for others.

review: Needs Information

[Atik Agewan(OpenERP) (aag-openerp)]

 Hello ,

  update_check=True is removed.
  new_uom definition removed from loop.

 Thanks

=== modified file 'product/product.py'
--- product/product.py	2011-10-11 14:03:35 +0000
+++ product/product.py	2011-11-04 09:58:25 +0000
@@ -166,6 +166,13 @@
         if value == 'reference':
             return {'value': {'factor': 1, 'factor_inv': 1}}
         return {}
+    
+    def write(self, cr, uid, ids, vals, context=None):
+        if 'category_id' in vals:
+            for uom in self.browse(cr, uid, ids, context=context):
+                if uom.category_id != vals['category_id']:
+                    raise osv.except_osv(_('Warning'),_("Cannot change the category of existing UoM '%s'.") % (uom.name,))
+        return super(product_uom, self).write(cr, uid, ids, vals, context=context)
 
 product_uom()
 
@@ -328,6 +335,15 @@
             return {'value': {'uom_po_id': uom_id}}
         return False
 
+    def write(self, cr, uid, ids, vals, context=None):
+        if 'uom_po_id' in vals:
+            new_uom = self.pool.get('product.uom').browse(cr, uid, vals['uom_po_id'], context=context)
+            for product in self.browse(cr, uid, ids, context=context):
+                old_uom = product.uom_po_id
+                if old_uom.category_id.id != new_uom.category_id.id:
+                    raise osv.except_osv(_('UoM categories Mismatch!'), _("New UoM '%s' must belongs to same UoM category '%s' as of old UoM '%s'.") % (new_uom.name, old_uom.category_id.name, old_uom.name,))
+        return super(product_template, self).write(cr, uid, ids, vals, context=context)
+
     _defaults = {
         'company_id': lambda s,cr,uid,c: s.pool.get('res.company')._company_default_get(cr, uid, 'product.template', context=c),
         'list_price': lambda *a: 1,