[FIX] stock: update location depending on state

maybe we should compute

addons/stock/models/stock_package_level.py

@@ -13,7 +13,7 @@ class StockPackageLevel(models.Model):
     picking_id = fields.Many2one('stock.picking', 'Picking')
     move_ids = fields.One2many('stock.move', 'package_level_id')
     move_line_ids = fields.One2many('stock.move.line', 'package_level_id')
-    location_id = fields.Many2one('stock.location', 'From')
+    location_id = fields.Many2one('stock.location', 'From', compute='_compute_location_id')
     location_dest_id = fields.Many2one('stock.location', 'To')
     is_done = fields.Boolean('Done', compute='_compute_is_done', inverse='_set_is_done')
     state = fields.Selection([
@@ -51,8 +51,8 @@ class StockPackageLevel(models.Model):
                         else:
                             corresponding_move = package_level.move_ids.filtered(lambda m: m.product_id == quant.product_id)[:1]
                             self.env['stock.move.line'].create({
-                                'location_id': package_level.picking_id.location_id.id,
-                                'location_dest_id': package_level.picking_id.location_dest_id.id,
+                                'location_id': package_level.location_id.id,
+                                'location_dest_id': package_level.location_dest_id.id,
                                 'picking_id': package_level.picking_id.id,
                                 'product_id': quant.product_id.id,
                                 'qty_done': quant.quantity,
@@ -110,12 +110,6 @@ class StockPackageLevel(models.Model):
                 package_level.show_lots_m2o = False
                 package_level.show_lots_text = False
 
-
-    @api.onchange('package_id')
-    def _onchange_package_id(self):
-        if not self.is_fresh_package:
-            self.location_id = self.package_id.location_id
-
     def _generate_moves(self):
         for package_level in self:
             if package_level.package_id:
@@ -136,9 +130,6 @@ class StockPackageLevel(models.Model):
         if vals.get('location_dest_id'):
             result.mapped('move_line_ids').write({'location_dest_id': vals['location_dest_id']})
             result.mapped('move_ids').write({'location_dest_id': vals['location_dest_id']})
-        if not result.is_fresh_package and vals.get('location_id'):
-            result.mapped('move_line_ids').write({'location_id': vals['location_id']})
-            result.mapped('move_ids').write({'location_id': vals['location_id']})
         if result.picking_id.state != 'draft' and result.location_id and result.location_dest_id and not result.move_ids and not result.move_line_ids:
             result._generate_moves()
         return result
@@ -148,13 +139,6 @@ class StockPackageLevel(models.Model):
         if vals.get('location_dest_id'):
             self.mapped('move_line_ids').write({'location_dest_id': vals['location_dest_id']})
             self.mapped('move_ids').write({'location_dest_id': vals['location_dest_id']})
-        if vals.get('location_id'):
-            fresh_packages = self.env['stock.package_level']
-            for package_level in self:
-                if package_level.is_fresh_package:
-                    fresh_packages |= package_level
-            fresh_packages.mapped('move_line_ids').write({'location_id': vals['location_id']})
-            fresh_packages.mapped('move_ids').write({'location_id': vals['location_id']})
         return result
 
     def unlink(self):
@@ -183,3 +167,15 @@ class StockPackageLevel(models.Model):
                 or any(grouped_ops.get(key, 0) - grouped_quants.get(key, 0) != 0 for key in grouped_ops):
             all_in = False
         return all_in
+
+    @api.depends('state', 'move_line_ids')
+    def _compute_location_id(self):
+        for pl in self:
+            if pl.state == 'new' or pl.is_fresh_package:
+                pl.location = False
+            elif pl.state in ('draft', 'cancel'):
+                pl.location_id = pl.picking_id.location_id
+            elif pl.state == 'confirmed' and pl.move_ids:
+                pl.location_id = pl.move_ids[0].location_id
+            elif pl.state in ('assigned', 'done') and pl.move_line_ids:
+                pl.location_id = pl.move_line_ids[0].location_id

addons/stock/models/stock_picking.py

@@ -661,7 +661,7 @@ class Picking(models.Model):
                             'result_package_id': pack.id,
                         })
                     else:
-                        move_lines_in_package_level = move_lines_to_pack.filtered(lambda ml: ml.move_id.package_level_id != False)
+                        move_lines_in_package_level = move_lines_to_pack.filtered(lambda ml: ml.move_id.package_level_id)
                         move_lines_without_package_level = move_lines_to_pack - move_lines_in_package_level
                         for ml in move_lines_in_package_level:
                             ml.write({

addons/stock/tests/test_packing.py

@@ -94,7 +94,6 @@ class TestPacking(TransactionCase):
         package_level = self.env['stock.package_level'].create({
             'package_id': pack.id,
             'picking_id': picking.id,
-            'location_id': self.stock_location.id,
             'location_dest_id': self.stock_location.id,
         })
         self.assertEquals(package_level.state, 'draft',
@@ -133,3 +132,44 @@ class TestPacking(TransactionCase):
                           'The quant package must be in the destination location')
         self.assertEquals(pack.quant_ids[0].location_id.id, picking.location_dest_id.id,
                           'The quant must be in the destination location')
+
+    def test_multi_pack_reservation(self):
+        """ When we move entire packages, it is possible to have a multiple times
+            the same package in package level list, we make sure that only one is reserved,
+            and that the location_id of the package is the one where the package is once it
+            is reserved.
+        """
+        pack = self.env['stock.quant.package'].create({'name': 'The pack to pick'})
+        shelf1_location = self.env['stock.location'].create({
+            'name': 'shelf1',
+            'usage': 'internal',
+            'location_id': self.stock_location.id,
+        })
+        self.env['stock.quant']._update_available_quantity(self.productA, shelf1_location, 20.0, package_id=pack)
+        picking = self.env['stock.picking'].create({
+            'picking_type_id': self.warehouse.int_type_id.id,
+            'location_id': self.stock_location.id,
+            'location_dest_id': self.stock_location.id,
+            'state': 'draft',
+        })
+        package_level = self.env['stock.package_level'].create({
+            'package_id': pack.id,
+            'picking_id': picking.id,
+            'location_dest_id': self.stock_location.id,
+        })
+        package_level = self.env['stock.package_level'].create({
+            'package_id': pack.id,
+            'picking_id': picking.id,
+            'location_dest_id': self.stock_location.id,
+        })
+        picking.action_confirm()
+        self.assertEqual(picking.package_level_ids.mapped('location_id.id'), [self.stock_location.id],
+                         'The package levels should still in the same location after confirmation.')
+        picking.action_assign()
+        package_level_reserved = picking.package_level_ids.filtered(lambda pl: pl.state == 'assigned')
+        package_level_confirmed = picking.package_level_ids.filtered(lambda pl: pl.state == 'confirmed')
+        self.assertEqual(package_level_reserved.location_id.id, shelf1_location.id, 'The reserved package level must be reserved in shelf1')
+        self.assertEqual(package_level_confirmed.location_id.id, self.stock_location.id, 'The not reserved package should keep its location')
+        picking.do_unreserve()
+        self.assertEqual(picking.package_level_ids.mapped('location_id.id'), [self.stock_location.id],
+                         'The package levels should have back the original location.')

addons/stock/views/stock_package_level_views.xml

@@ -53,7 +53,7 @@
             <tree editable="bottom" decoration-muted="state == 'done'">
                 <field name="is_fresh_package" invisible="1"/>
                 <field name="package_id" attrs="{'readonly': [('state', 'in', ('confirmed', 'assigned', 'done', 'cancel'))]}" domain="[('location_id', 'child_of', parent.location_id)]" options="{'no_create': True}"/>
-                <field name="location_id" domain="[('id', 'child_of', parent.location_id)]" attrs="{'readonly': [('state', 'in', ('confirmed', 'assigned', 'done', 'cancel'))], 'column_invisible': [('parent.picking_type_code', '=', 'incoming')]}" groups="stock.group_stock_multi_locations" force_save="1"/>
+                <field name="location_id" attrs="{'column_invisible': [('parent.picking_type_code', '=', 'incoming')]}" groups="stock.group_stock_multi_locations"/>
                 <field name="location_dest_id" domain="[('id', 'child_of', parent.location_dest_id)]" attrs="{'column_invisible': [('parent.picking_type_code', '=', 'outgoing')]}" groups="stock.group_stock_multi_locations"/>
                 <field name="state"/>
                 <field name="is_done" attrs="{'readonly': [('parent.state', 'in', ('draft', 'new', 'done'))]}"/>