Merge lp:~openerp-dev/openobject-addons/6.1-opw-575875-rha into lp:openobject-addons/6.1

[Rifakat Husen (OpenERP)]

Hello,

When filling the inventory line with product and does not provide production lot then totol qty
should return qty withouht production lot(prodlot_id==Null) but currently it does not distinguish
and retund all qty with and without lot. It leads to wrong creation of stock moves.

Also I have put check on inventory line for production lot, when product's incoming tracking is
required so one must have to provide production lot in inventory line.

One should have general practice to fill the inventory lines using 'Fill Inventory' wizard in
order to avoid problem for stock.

Please review this fix.

Regards,
Rifakat

[Numérigraphe (numerigraphe)]

Since this bug was a bit hard to illustrate and explain, can you please add an automatic test for this fix, so we make sure it does not come back as a regression later on?

review: Needs Fixing (automatic test)

[Numérigraphe (numerigraphe)]

Your revision 6842 is dubious: that's quite a functional change, and I don't think it's consistent with the button on the product form that lets quickly enter a new quantity.

review: Needs Fixing (functional)

[Numérigraphe (numerigraphe)]

review: Disapprove

[Olivier Dony (Odoo) (odo-openerp)]

Hello,

On a functional aspect this change looks fine to me. I double-checked our addons and there is no place where we put prodlot_id: False value in the context for get_product_available's, except here. Therefore we can safely introduce this new semantics: if prodlot_id is False in context, it means we're explicitly looking for products that have *no* production lot. When we mean to count all products regardless of their production lot, the key should be omitted instead.

I also agree with Lionel's remarks:
1. We could really use an extra test to cover this fix. It should be as simple as adding 1-2 extra steps at the end of stock/test/opening_stock.yml to perform an extra product qty change with Ice Cream and prod_lot_id = False. The result before and after the fix should be different, so you can assert that the final stock quantity for Ice Cream is correct.
2. I don't think the change on l.32-34 is needed, appropriate exceptions will be triggered when confirming the stock moves anyway, if the production lot tracking conditions are not met, so this change is unnecessary.

Now one small technical remark on the patch:
- l.9 and 19: the code is becoming quite unreadable due to this overlong inline condition. Please make it a separate variable (e.g. prodlot_condition) that is computed above the call to cr.execute() and inserted in the query string. In fact you need to update your code with the latest 6.1 changes, as the code was refactored a little bit.

Thanks!

review: Needs Fixing

[Numérigraphe (numerigraphe)]

That looks fine, thanks a lot.
Lionel

review: Approve

=== modified file 'stock/product.py'
--- stock/product.py	2012-06-19 07:40:07 +0000
+++ stock/product.py	2012-10-22 09:39:22 +0000
@@ -256,6 +256,8 @@
         if prodlot_id:
             prodlot_clause = ' and prodlot_id = %s '
             where += [prodlot_id]
+        elif 'prodlot_id' in context and not prodlot_id:
+            prodlot_clause = 'and prodlot_id is null '
 
         # TODO: perhaps merge in one query.
         if 'in' in what:

=== modified file 'stock/test/opening_stock.yml'
--- stock/test/opening_stock.yml	2011-12-23 11:06:43 +0000
+++ stock/test/opening_stock.yml	2012-10-22 09:39:22 +0000
@@ -33,6 +33,36 @@
     product = self.browse(cr, uid, ref('product_icecream'), context=context)
     assert product.qty_available == 10, "Stock is not updated."
 -
+  I update the current stock of the Ice-cream with 10 kgm in Small Refrigerator without Production Lot.
+-
+  !record {model: stock.change.product.qty, id: change_qty_nolot}:
+    location_id: location_refrigerator_small
+    new_quantity: 10
+    product_id: product_icecream
+-
+  !python {model: stock.change.product.qty}: |
+    self.change_product_qty(cr, uid, [ref('change_qty_nolot')], context=context)
+-
+  I check available stock of Ice-cream after update stock.
+-
+  !python {model: product.product}: |
+    product = self.browse(cr, uid, ref('product_icecream'), context=context)
+    assert product.qty_available == 20, "Real stock is not updated."
+-
+  I revert 10kgm updated stock again with no production lot in order to level the stock
+-
+  !record {model: stock.change.product.qty, id: change_qty_nolot_1}:
+    location_id: location_refrigerator_small
+    new_quantity: 0
+    product_id: product_icecream
+-
+  !python {model: stock.change.product.qty}: |
+    self.change_product_qty(cr, uid, [ref('change_qty_nolot_1')], context=context)
+-
+  !python {model: product.product}: |
+    product = self.browse(cr, uid, ref('product_icecream'), context=context)
+    assert product.qty_available == 10, "Real stock is not updated."
+-
   I merge inventory.
 -
   !python {model: stock.inventory.merge }: |