Merge lp:~camptocamp/ocb-addons/7.0-fix-lp-1204832-RDE into lp:ocb-addons

[Romain Deheele - Camptocamp]

Hello,

This branch fixes bug #1204832: avoids gap on stock.picking.out sequence when chained picking setup

https://code.launchpad.net/~openerp-dev/openobject-addons/7.0-opw-595832-pna/+merge/178709

Romain

[Cristian Salamea (ovnicraft)]

I see the fix (in official addons ) is still fix commited.

LGTM, diff is the same from OPW.

Regards,

review: Approve

[Christophe CHAUVET (christophe-chauvet)]

Hi

The code seem correct

Regards,

review: Approve (code review, not test)

[Stefan Rijnhart (Opener) (stefan-opener)]

I think the diff is missing this revision which was added later on: http://bazaar.launchpad.net/~openerp-dev/openobject-addons/7.0-opw-595832-pna/revision/9350. Although this is a trivial change, this will give a conflict if the OPW gets merged in a later point in time.

Note that we would not get a conflict if the OPW was cherrypicked into this branch using bzr. Please consider always using cherrypicking when you port from other branches.

BTW. the merger should attribute the original committer of the OPW.

So please adopt the change in line 12 from the OPW.

review: Needs Fixing

[Romain Deheele - Camptocamp (romaindeheele)]

I made needed changes.
Thanks for your advices,

Romain

[Stefan Rijnhart (Opener) (stefan-opener)]

Hi Romain,

thank you for taking my advice on cherrypicking! The fact that the diff is now different from that of the OPW shows that the world is not as simple as I would have liked. I think you found out (like me one moment ago) that the OPW-version now has conflicts. You seemed to have solved the conflict very well. But we have this rule in OCB that all code proposed to OCB should also be proposed verbatim to the official series. Again, this is also the best way to prevent conflicts in the synchronization process.

So could you please offer your resolution of the conflict back to lp:openobject-addons/7.0? The friendliest form I imagine is to submit a proposal onto the OPW branch itself (so not directly onto the series branch). I'll set my review to 'needs fixing' to indicate that the process still needs work, even if the code LGTM. Sorry for the hassle ;-)

review: Needs Fixing

[Serpent Consulting Services (serpent-consulting-services)]

if old_ptype == 'internal' :
17 + model_name = 'stock.picking'
18 + else :
19 + model_name = 'stock.picking.' + old_ptype

Should be optimized:

model_name = 'stock.picking' + ( old_ptype != 'internal' and ('.' + old_ptype) or '')

AND
if pick_name will always be True, so should not be needed.
Instead this should be inline:

new_pick_name = picking.type == 'out' and seq_obj.get(cr, uid, 'stock.picking.' + ptype) or pick_name

Hope this helps.

(else and : should be together..pep8)

Thanks.

review: Needs Fixing

[Stefan Rijnhart (Opener) (stefan-opener)]

Hi Serpent,

remember that this is a backport of an OPW which is proposed against the official branches. If we start fixing nits in these branches, we will run into conflicts once the OPW gets merged.

[Serpent Consulting Services (serpent-consulting-services)]

Stefan,

Agreed, but we can ask opw team to correct to the right standard.

I believe, the corrections should land into stable as well.

They will agree too.

Thanks.

[Stefan Rijnhart (Opener) (stefan-opener)]

In that case, I hope that you can understand that as the person who actually resolves the occasional conflict that occurs when the official branches are replayed on OCB, I think this branch should not be merged with OCB until the OPW has merged our nits. Which will lead to considerable delays, defeating the purpose of having an OCB branch in the first place.

[Serpent Consulting Services (serpent-consulting-services)]

Yes Stefan, this branch should not be merged until OPW team merges right code. We cannot compromise with quality.

RDE C2C should request OPW team to treat this urgently as this bug sits on stable.

Thank you.

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

Romain, what is the state of this MP?

Given the discussions, I put this MP as 'work in progress'. Please set it back to 'Needs review' once ready.

[Romain Deheele - Camptocamp (romaindeheele)]

The bug fix proposed here
https://code.launchpad.net/~openerp-dev/openobject-addons/7.0-opw-595832-pna/+merge/178709
generates a conflict when you try to merge it in official addons.
As the case is getting old, I proposed (https://code.launchpad.net/~openerp-dev/openobject-addons/7.0-opw-595832-pna/+merge/178709) to recreate a new branch based on updated addons.

Unmerged revisions

9497. By Pinakin Nayi (OpenERP) on 2013-09-13

[MRG][FIX] stock: improve sequence gap on delivery order & [IMP] nit code as there is no need to browse record

=== modified file 'stock/stock.py'
--- stock/stock.py	2013-09-10 07:21:00 +0000
+++ stock/stock.py	2013-09-13 16:34:27 +0000
@@ -2062,17 +2062,23 @@
         for picking, todo in self._chain_compute(cr, uid, moves, context=context).items():
             ptype = todo[0][1][5] and todo[0][1][5] or location_obj.picking_type_get(cr, uid, todo[0][0].location_dest_id, todo[0][1][0])
             if picking:
+                # Need to check name of old picking because it always considers picking as "OUT" when created from Sales Order
+                pick_name = picking.name
+                old_ptype = location_obj.picking_type_get(cr, uid, picking.move_lines[0].location_id, picking.move_lines[0].location_dest_id)
+                if old_ptype != picking.type:
+                    if old_ptype == 'internal' :
+                        model_name = 'stock.picking'
+                    else :
+                        model_name = 'stock.picking.' + old_ptype
+                    old_pick_name = seq_obj.get(cr, uid, model_name)
+                    self.pool.get('stock.picking').write(cr, uid, [picking.id], {'name': old_pick_name, 'type': old_ptype}, context=context)
+                
                 # name of new picking according to its type
-                if ptype == 'internal':
-                    new_pick_name = seq_obj.get(cr, uid,'stock.picking')
+                if pick_name and picking.type == 'out':
+                    new_pick_name = pick_name
                 else :
                     new_pick_name = seq_obj.get(cr, uid, 'stock.picking.' + ptype)
                 pickid = self._create_chained_picking(cr, uid, new_pick_name, picking, ptype, todo, context=context)
-                # Need to check name of old picking because it always considers picking as "OUT" when created from Sales Order
-                old_ptype = location_obj.picking_type_get(cr, uid, picking.move_lines[0].location_id, picking.move_lines[0].location_dest_id)
-                if old_ptype != picking.type:
-                    old_pick_name = seq_obj.get(cr, uid, 'stock.picking.' + old_ptype)
-                    self.pool.get('stock.picking').write(cr, uid, [picking.id], {'name': old_pick_name, 'type': old_ptype}, context=context)
             else:
                 pickid = False
             for move, (loc, dummy, delay, dummy, company_id, ptype, invoice_state) in todo: