Merge lp:~openerp-dev/openobject-addons/6.0-bug_725908-ach into lp:openobject-addons/6.0

[Anup(SerpentCS)]

Hello,

   The problem with the Stock Fill Inventory has been fixed by this solution. Now all the stock moves will be imported to the Inventory with production lots.

Thanks.

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

please refer: https://code.launchpad.net/~openerp-dev/openobject-addons/trunk-bug-725908-ksa/+merge/52519

[Jay Vora (Serpent Consulting Services) (jayvora)]

Anup,

Thanks to you and Kirti for the fix. It was really a good one.

Kindly go through these important points to improve here:

1. Variable naming convention.(Do not opt for i,j,k,v).
2. Wrong indentation on your proposal Line 21.
3. Just a note : Whenever we use child_of operator, go for the right operant with [] of ids. This doesn't create any problem as this has been covered well recently in expression.py, but its a good practice.(Refer to line 31)
4. Does the order='id' give you a different result when order='parent_left'? (Refer to Line 31)
5. Line 61 should be location_dest_id = location. Think over it.
6. Use a shorthand operator. (Refer to line 63)
7. Improve the notification, it could be notification or warning(Line 70).
8. You don't need line 73.
9. Why do we need len(line_ids)? (Line 90).
10. Regarding last 2 for loops, I am not 100% sure whether its an optimized solution or not.
11. Regarding Line 75, are you sure we always have one inventory to check? I doubt because, we can use this wizard for multiple inventories. And its always wise to check before writing list[0].

Correct me If I missed anything.

I am investigating more in order to get the best suited fix.

Thanks.

review: Needs Fixing

[Jay Vora (Serpent Consulting Services) (jayvora)]

Corrections:

4. Does the order='id' give you a different result than order='parent_left'? (Refer to Line 31)

Thanks.

[Anup(SerpentCS) (anup-serpent)]

Hello Jay,

   Thanks a lot for a detailed investigation. I have fixed many of the issues mentioned by you like indentations and naming conventions, child_of, shorthand operator,notifications etc.

4.Currently order = 'parent_left' is giving the same result as id. But IMHO it's better to keep id rather than parent_left.

5. You are right we don't need that line. It saves many read calls and we don't even need that variable.

11. Well The wizard should be used with a single inventory only. You can not use it with multiple inventories. I have kept a notification if more than one inventory is selected. So it solves the issue.

We even don't need the lines 38-42. When will it be the case when ids will not be there?

Please share your views.

Thanks.

[Jay Vora (Serpent Consulting Services) (jayvora)]

Thanks for the corrections Anup.

However, I am investigating 3 changes here:
1. Return True would be good for view_init.
2. ids = ids[0] (Line 44)
3. order='id' (Line 56).

Thanks again.

review: Approve

[Jay Vora (Serpent Consulting Services) (jayvora)]

Anup,

Corrections have been made and the branch has been merged to stable 6.0.2.

Thanks for all your efforts.

=== modified file 'stock/wizard/stock_fill_inventory.py'
--- stock/wizard/stock_fill_inventory.py	2011-01-14 00:11:01 +0000
+++ stock/wizard/stock_fill_inventory.py	2011-04-18 07:33:37 +0000
@@ -39,14 +39,18 @@
          @param context: A standard dictionary
          @return: New arch of view with new columns.
         """
-        if context==None:
-            context={}
-        res = super(stock_fill_inventory, self).view_init(cr, uid, fields_list, context=context)
+        if context is None:
+            context = {}
+        super(stock_fill_inventory, self).view_init(cr, uid, fields_list, context=context)
+        
+        if len(context['active_ids']) > 1:
+            raise osv.except_osv(_('Error!'), _('You can not perform this operation on more than one Stock Inventories'))
+        
         if context.get('active_id', False):
             stock = self.pool.get('stock.inventory').browse(cr, uid, context.get('active_id', False))
+            
             if stock.state=='done':
-                raise osv.except_osv(_('Error!'), _('Stock Inventory is done'))
-        True
+                raise osv.except_osv(_('Error!'), _('Stock Inventory is already Validated.'))
 
     def fill_inventory(self, cr, uid, ids, context=None):
         """ To Import stock inventory according to products available in the selected locations.
@@ -58,61 +62,65 @@
         @return:
         """
         if context is None:
-            context = {}        
+            context = {}
+
         inventory_line_obj = self.pool.get('stock.inventory.line')
         location_obj = self.pool.get('stock.location')
         product_obj = self.pool.get('product.product')
-        stock_location_obj = self.pool.get('stock.location')
-        if ids and len(ids): 
-            ids = ids[0]
-        else:
-             return {'type': 'ir.actions.act_window_close'}    
+        move_obj = self.pool.get('stock.move')
+        ids = ids[0]
+        
         fill_inventory = self.browse(cr, uid, ids, context=context)
         res = {}
         res_location = {}
+        
         if fill_inventory.recursive :
             location_ids = location_obj.search(cr, uid, [('location_id',
-                             'child_of', fill_inventory.location_id.id)])
-            for location in location_ids :
-                res = location_obj._product_get(cr, uid, location)
-                res_location[location] = res
+                             'child_of', [fill_inventory.location_id.id])], order="id")
         else:
-            context.update({'compute_child': False})
-            res = location_obj._product_get(cr, uid,
-                        fill_inventory.location_id.id, context=context)
-            res_location[fill_inventory.location_id.id] = res
-    
-        product_ids = []
-        for location in res_location.keys():
-            res = res_location[location]
-            for product_id in res.keys():
-                prod = product_obj.browse(cr, uid, product_id, context=context)
-                uom = prod.uom_id.id
-                context.update(uom=uom, compute_child=False)
-                amount = stock_location_obj._product_get(cr, uid,
-                         location, [product_id], context=context)[product_id]
-                if(amount):
-                    if fill_inventory.set_stock_zero:
-                        amount = 0                    
-                    line_ids=inventory_line_obj.search(cr, uid,
-                        [('inventory_id', '=', context['active_ids']),
-                         ('location_id', '=', location),
-                         ('product_id', '=', product_id),
-                         ('product_uom', '=', uom),
-                        ('product_qty', '=', amount)])
-                    if not len(line_ids):
-                        inventory_line = {
-                            'inventory_id': context['active_ids'][0],
-                            'location_id': location,
-                            'product_id': product_id,
-                            'product_uom': uom,
-                            'product_qty': amount
-                        }
-                        inventory_line_obj.create(cr, uid, inventory_line)
-                    product_ids.append(product_id)
-
-        if(len(product_ids) == 0):
-            raise osv.except_osv(_('Message !'), _('No product in this location.'))
+            location_ids = [fill_inventory.location_id.id]
+
+        res = {}
+        flag = False
+        
+        for location in location_ids:
+            datas = {}
+            res[location] = {}
+            move_ids = move_obj.search(cr, uid, [('location_dest_id','=',location),('state','=','done')], context=context)
+
+            for move in move_obj.browse(cr, uid, move_ids, context=context):
+                lot_id = move.prodlot_id.id
+                prod_id = move.product_id.id
+                qty = move.product_qty
+                
+                if datas.get((prod_id, lot_id)):
+                    qty += datas[(prod_id, lot_id)]['product_qty']
+                
+                datas[(prod_id, lot_id)] = {'product_id': prod_id, 'location_id': location, 'product_qty': qty, 'product_uom': move.product_id.uom_id.id, 'prod_lot_id': lot_id}
+            
+            if datas:
+                flag = True
+                res[location] = datas
+
+        if not flag:
+            raise osv.except_osv(_('Warning !'), _('No product in this location.'))
+
+        for stock_move in res.values():
+            for stock_move_details in stock_move.values():
+                stock_move_details.update({'inventory_id': context['active_ids'][0]})
+                domain = []
+                
+                if fill_inventory.set_stock_zero:
+                    stock_move_details.update({'product_qty': 0})
+
+                for field, value in stock_move_details.items():
+                    domain.append((field, '=', value))
+
+                line_ids = inventory_line_obj.search(cr, uid, domain, context=context)
+
+                if not line_ids:
+                    inventory_line_obj.create(cr, uid, stock_move_details, context=context)
+
         return {'type': 'ir.actions.act_window_close'}
 
 stock_fill_inventory()