Merge lp:~yann-papouin/ocb-addons/6.1-bug-1234678-return-product-backorder-false into lp:ocb-addons/6.1

[Yann Papouin]

Automatically derived from https://code.launchpad.net/~yann-papouin/openobject-addons/6.1-bug-1234678-return-product-backorder-false for https://code.launchpad.net/~openerp/openobject-addons/6.1.

[Holger Brunn (Therp) (hbrunn)]

review: Approve (code review)

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

review: Approve (code review)

[Omar (Pexego) (omar7r)]

Hi Yann,

backorder_id is set to False in the copy method of stock.picking object in the stock module, but only when name not in default or default['name'] == '/'. Then, I think, the good fix should be in the copy method, extract this line out of this condition, not here.

Regards

review: Needs Fixing (code review)

[Yann Papouin (yann-papouin)]

Hi Omar,

I don't think like you about this. The return product action is specific and that's one of the only case where we have to set the backorder_id to False as we usually keep the backorder for any picking duplicate. Hiding this operation in the copy method could be awkward for a code reader and I don't know what condition could be used for this.

[Omar (Pexego) (omar7r)]

H Yann,

I think that don't have any sense keep the backorder_id for duplicating picking in any case, much less, like by default, when there isn't name or name == '/' Why?

[Yann Papouin (yann-papouin)]

I agree with you, backorder_id should be False by design when duplicating a picking but fixing it in ocb-addons is something "too deep" as it may require more changes in other places (built-in addons and other ones) to reintroduce the backorder_id value. (note that this sentence is just an assertion, I didn't made any code search on this behavior).

[Omar (Pexego) (omar7r)]

OK, I agree with you in this point. If any other reviewer shares this point I approve the merge. Meanwhile I abstain.

Thanks

review: Abstain (code review)

=== modified file 'stock/wizard/stock_return_picking.py'
--- stock/wizard/stock_return_picking.py	2012-06-12 16:15:51 +0000
+++ stock/wizard/stock_return_picking.py	2013-10-03 13:13:43 +0000
@@ -166,9 +166,14 @@
             new_type = 'internal'
         seq_obj_name = 'stock.picking.' + new_type
         new_pick_name = self.pool.get('ir.sequence').get(cr, uid, seq_obj_name)
-        new_picking = pick_obj.copy(cr, uid, pick.id, {'name':'%s-%s-return' % (new_pick_name, pick.name),
-                'move_lines':[], 'state':'draft', 'type':new_type,
-                'date':date_cur, 'invoice_state':data['invoice_state'],})
+        new_picking = pick_obj.copy(cr, uid, pick.id, {
+                                        'name':'%s-%s-return' % (new_pick_name, pick.name),
+                                        'move_lines':[], 
+                                        'state':'draft', 
+                                        'backorder_id': False,
+                                        'type':new_type,
+                                        'date':date_cur, 
+                                        'invoice_state':data['invoice_state'],})
         
         val_id = data['product_return_moves']
         for v in val_id: