[REF] stock: action_done on stock.move.line

Before this commit, `action_done` on stock.move.line was intended to
receive the recordset of the move lines of a single move where the move
lines having a null `qty_done` were already unlinked.

This part of code was in stock.picking's `button_validate` and not
copied in mrp, and weird things happened when mrp tried to run
`action_done` on a stock.move.line with a `qty_done` of 0.

We move this check directly in `action_done`, as well as the check on
the picking type that was done in stock.move's `action_done`.

We also adapts rev[0] that was not working anymore since the new stock,
and add some tests.

[0] e79eaecc46

addons/stock/models/stock_move.py

@@ -883,17 +883,7 @@ class StockMove(models.Model):
                 # If you were already putting stock.move.lots on the next one in the work order, transfer those to the new move
                 move.move_line_ids.filtered(lambda x: x.qty_done == 0.0).write({'move_id': new_move})
                 self.browse(new_move).quantity_done = 0.0
-            for packop in move.move_line_ids:
-                if float_compare(packop.qty_done, 0, precision_rounding=rounding) > 0:
-                    if move.has_tracking != 'none' and (move.picking_type_id.use_create_lots or move.picking_type_id.use_existing_lots):
-                        if packop.lot_name and not packop.lot_id:
-                            lot = self.env['stock.production.lot'].create(
-                                {'name': packop.lot_name, 'product_id': packop.product_id.id})
-                            packop.write({'lot_id': lot.id})
-                        if not packop.lot_id:
-                            raise UserError(_('You need to supply a lot/serial number.'))
-                # execute `action_done` even if nothing was done, to free the reservation
-                packop.action_done()
+            move.move_line_ids.action_done()
         picking = self and self[0].picking_id or False
         moves_todo.write({'state': 'done', 'date': fields.Datetime.now()})
         moves_todo.mapped('move_dest_ids').action_assign()

addons/stock/models/stock_move_line.py

@@ -237,10 +237,56 @@ class StockMoveLine(models.Model):
         return super(StockMoveLine, self).unlink()
 
     def action_done(self):
-        """ This method will finalize the work with a move line by "moving" quants to the
-        destination location.
+        """ This method is called during a move's `action_done`. It'll actually move a quant from
+        the source location to the destination location, and unreserve if needed in the source
+        location.
+
+        This method is intended to be called on all the move lines of a move. This method is not
+        intended to be called when editing a `done` move (that's what the override of `write` here
+        is done.
         """
+
+        # First, we loop over all the move lines to do a preliminary check: `qty_done` should not
+        # be negative and, according to the presence of a picking type or a linked inventory
+        # adjustment, enforce some rules on the `lot_id` field. If `qty_done` is null, we unlink
+        # the line. It is mandatory in order to free the reservation and correctly apply
+        # `action_done` on the next move lines.
+        ml_to_delete = self.env['stock.move.line']
         for ml in self:
+            qty_done_float_compared = float_compare(ml.qty_done, 0, precision_rounding=ml.product_uom_id.rounding)
+            if qty_done_float_compared > 0:
+                if ml.product_id.tracking != 'none':
+                    picking_type_id = ml.move_id.picking_type_id
+                    if picking_type_id:
+                        if picking_type_id.use_create_lots:
+                            # If a picking type is linked, we may have to create a production lot on
+                            # the fly before assigning it to the move line if the user checked both
+                            # `use_create_lots` and `use_existing_lots`.
+                            if ml.lot_name and not ml.lot_id:
+                                lot = self.env['stock.production.lot'].create(
+                                    {'name': ml.lot_name, 'product_id': ml.product_id.id}
+                                )
+                                ml.write({'lot_id': lot.id})
+                        elif not picking_type_id.use_create_lots and not picking_type_id.use_existing_lots:
+                            # If the user disabled both `use_create_lots` and `use_existing_lots`
+                            # checkboxes on the picking type, he's allowed to enter tracked
+                            # products without a `lot_id`.
+                            continue
+                    elif ml.move_id.inventory_id:
+                        # If an inventory adjustment is linked, the user is allowed to enter
+                        # tracked products without a `lot_id`.
+                        continue
+
+                    if not ml.lot_id:
+                        raise UserError(_('You need to supply a lot/serial number.'))
+            elif qty_done_float_compared < 0:
+                raise UserError(_('No negative quantities allowed'))
+            else:
+                ml_to_delete |= ml
+        ml_to_delete.unlink()
+
+        # Now, we can actually move the quant.
+        for ml in self - ml_to_delete:
             if ml.product_id.type != 'consu':
                 Quant = self.env['stock.quant']
                 rounding = ml.product_uom_id.rounding

addons/stock/models/stock_picking.py

@@ -591,7 +591,6 @@ class Picking(models.Model):
     @api.multi
     def button_validate(self):
         self.ensure_one()
-        move_line_delete = self.env['stock.move.line']
         if not self.move_lines and not self.move_line_ids:
             raise UserError(_('Please add some lines to move'))
         # In draft or with no pack operations edited yet, ask if we can just do everything
@@ -633,16 +632,6 @@ class Picking(models.Model):
                 'res_id': wiz.id,
                 'context': self.env.context,
             }
-        for operation in self.move_line_ids:
-            if operation.qty_done < 0:
-                raise UserError(_('No negative quantities allowed'))
-            if operation.qty_done > 0:
-                pass
-                #operation.write({'product_qty': operation.qty_done})
-            else:
-                move_line_delete |= operation
-        if move_line_delete:
-            move_line_delete.unlink()
         self.action_done()
         return
 

addons/stock/models/stock_production_lot.py

@@ -30,10 +30,10 @@ class ProductionLot(models.Model):
 
     @api.model
     def create(self, vals):
-        pack_id = self.env.context.get('active_pack_operation', False)
-        if pack_id:
-            pack = self.env['stock.move.line'].browse(pack_id)
-            if pack.picking_id and not pack.picking_id.picking_type_id.use_create_lots:
+        active_picking_id = self.env.context.get('active_picking_id', False)
+        if active_picking_id:
+            picking_id = self.env['stock.picking'].browse(active_picking_id)
+            if picking_id and not picking_id.picking_type_id.use_create_lots:
                 raise UserError(_("You are not allowed to create a lot for this picking type"))
         return super(ProductionLot, self).create(vals)
 

addons/stock/tests/test_move2.py

@@ -2,6 +2,7 @@
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
 from odoo.addons.stock.tests.common import TestStockCommon
+from odoo.exceptions import UserError
 
 
 class TestPickShip(TestStockCommon):
@@ -746,3 +747,177 @@ class TestSinglePicking(TestStockCommon):
 
         self.assertEqual(move2.state, 'assigned')
         self.assertEqual(delivery_order.state, 'assigned')
+
+    def test_use_create_lot_use_existing_lot_1(self):
+        """ Check the behavior of a picking when `use_create_lot` and `use_existing_lot` are
+        set to False and there's a move for a tracked product.
+        """
+        self.env['stock.picking.type']\
+            .browse(self.picking_type_out)\
+            .write({
+                'use_create_lots': False,
+                'use_existing_lots': False,
+            })
+        self.productA.tracking = 'lot'
+
+        delivery_order = self.env['stock.picking'].create({
+            'location_id': self.pack_location,
+            'location_dest_id': self.customer_location,
+            'partner_id': self.partner_delta_id,
+            'picking_type_id': self.picking_type_out,
+        })
+        self.MoveObj.create({
+            'name': self.productA.name,
+            'product_id': self.productA.id,
+            'product_uom_qty': 2,
+            'product_uom': self.productA.uom_id.id,
+            'picking_id': delivery_order.id,
+            'picking_type_id': self.picking_type_out,
+            'location_id': self.pack_location,
+            'location_dest_id': self.customer_location,
+        })
+
+        delivery_order.action_confirm()
+        delivery_order.force_assign()
+        delivery_order.move_lines.quantity_done = 2
+        # do not set a lot_id or lot_name, it should work
+        delivery_order.action_done()
+
+    def test_use_create_lot_use_existing_lot_2(self):
+        """ Check the behavior of a picking when `use_create_lot` and `use_existing_lot` are
+        set to True and there's a move for a tracked product.
+        """
+        self.env['stock.picking.type']\
+            .browse(self.picking_type_out)\
+            .write({
+                'use_create_lots': True,
+                'use_existing_lots': True,
+            })
+        self.productA.tracking = 'lot'
+
+        delivery_order = self.env['stock.picking'].create({
+            'location_id': self.pack_location,
+            'location_dest_id': self.customer_location,
+            'partner_id': self.partner_delta_id,
+            'picking_type_id': self.picking_type_out,
+        })
+        self.MoveObj.create({
+            'name': self.productA.name,
+            'product_id': self.productA.id,
+            'product_uom_qty': 2,
+            'product_uom': self.productA.uom_id.id,
+            'picking_id': delivery_order.id,
+            'picking_type_id': self.picking_type_out,
+            'location_id': self.pack_location,
+            'location_dest_id': self.customer_location,
+        })
+
+        delivery_order.action_confirm()
+        delivery_order.force_assign()
+        delivery_order.move_lines.quantity_done = 2
+        move_line = delivery_order.move_lines.move_line_ids
+
+        # not lot_name set, should raise
+        with self.assertRaises(UserError):
+            delivery_order.action_done()
+
+        # enter a new lot name, should work
+        move_line.lot_name = 'newlot'
+        delivery_order.action_done()
+
+    def test_use_create_lot_use_existing_lot_3(self):
+        """ Check the behavior of a picking when `use_create_lot` is set to True and
+        `use_existing_lot` is set to False and there's a move for a tracked product.
+        """
+        self.env['stock.picking.type']\
+            .browse(self.picking_type_out)\
+            .write({
+                'use_create_lots': True,
+                'use_existing_lots': False,
+            })
+        self.productA.tracking = 'lot'
+
+        delivery_order = self.env['stock.picking'].create({
+            'location_id': self.pack_location,
+            'location_dest_id': self.customer_location,
+            'partner_id': self.partner_delta_id,
+            'picking_type_id': self.picking_type_out,
+        })
+        self.MoveObj.create({
+            'name': self.productA.name,
+            'product_id': self.productA.id,
+            'product_uom_qty': 2,
+            'product_uom': self.productA.uom_id.id,
+            'picking_id': delivery_order.id,
+            'picking_type_id': self.picking_type_out,
+            'location_id': self.pack_location,
+            'location_dest_id': self.customer_location,
+        })
+
+        delivery_order.action_confirm()
+        delivery_order.force_assign()
+        delivery_order.move_lines.quantity_done = 2
+        move_line = delivery_order.move_lines.move_line_ids
+
+        # not lot_name set, should raise
+        with self.assertRaises(UserError):
+            delivery_order.action_done()
+
+        # enter a new lot name, should work
+        move_line.lot_name = 'newlot'
+        delivery_order.action_done()
+
+    def test_use_create_lot_use_existing_lot_4(self):
+        """ Check the behavior of a picking when `use_create_lot` is set to False and
+        `use_existing_lot` is set to True and there's a move for a tracked product.
+        """
+        self.env['stock.picking.type']\
+            .browse(self.picking_type_out)\
+            .write({
+                'use_create_lots': False,
+                'use_existing_lots': True,
+            })
+        self.productA.tracking = 'lot'
+
+        delivery_order = self.env['stock.picking'].create({
+            'location_id': self.pack_location,
+            'location_dest_id': self.customer_location,
+            'partner_id': self.partner_delta_id,
+            'picking_type_id': self.picking_type_out,
+        })
+        self.MoveObj.create({
+            'name': self.productA.name,
+            'product_id': self.productA.id,
+            'product_uom_qty': 2,
+            'product_uom': self.productA.uom_id.id,
+            'picking_id': delivery_order.id,
+            'picking_type_id': self.picking_type_out,
+            'location_id': self.pack_location,
+            'location_dest_id': self.customer_location,
+        })
+
+        delivery_order.action_confirm()
+        delivery_order.force_assign()
+        delivery_order.move_lines.quantity_done = 2
+        move_line = delivery_order.move_lines.move_line_ids
+
+        # not lot_name set, should raise
+        with self.assertRaises(UserError):
+            delivery_order.action_done()
+
+        # creating a lot from the view should raise
+        with self.assertRaises(UserError):
+            self.env['stock.production.lot']\
+                .with_context(active_picking_id=delivery_order.id)\
+                .create({
+                    'name': 'lot1',
+                    'product_id': self.productA.id,
+                })
+
+        # enter an existing lot_id, should work
+        lot1 = self.env['stock.production.lot'].create({
+            'name': 'lot1',
+            'product_id': self.productA.id,
+        })
+        move_line.lot_id = lot1
+        delivery_order.action_done()

addons/stock/tests/test_stock_flow.py

@@ -1237,7 +1237,6 @@ class TestStockFlow(TestStockCommon):
 
     def test_30_check_with_no_incoming_lot(self):
         """ Picking in without lots and picking out with"""
-
         # Change basic operation type not to get lots
         # Create product with lot tracking
         picking_in = self.env['stock.picking.type'].browse(self.picking_type_in)
@@ -1254,6 +1253,7 @@ class TestStockFlow(TestStockCommon):
             'product_uom_qty': 4,
             'product_uom': self.productA.uom_id.id,
             'picking_id': picking_in.id,
+            'picking_type_id': self.picking_type_in,
             'location_id': self.supplier_location,
             'location_dest_id': self.stock_location})
 

addons/stock/views/stock_move_views.xml

@@ -171,7 +171,7 @@
                             <field name="product_id" invisible="1"/>
                             <field name="location_id" invisible="not context.get('show_source_location')" domain="[('id', 'child_of', parent.location_id)])" groups="stock.group_stock_multi_locations"/>
                             <field name="location_dest_id" invisible="not context.get('show_destination_location')" domain="[('id', 'child_of', parent.location_dest_id)]" groups="stock.group_stock_multi_locations"/>
-                            <field name="lot_id" invisible="not context.get('show_lots_m2o')" domain="[('product_id', '=', parent.product_id)]" groups="stock.group_production_lot" context="{'default_product_id': parent.product_id}"/>
+                            <field name="lot_id" invisible="not context.get('show_lots_m2o')" domain="[('product_id', '=', parent.product_id)]" groups="stock.group_production_lot" context="{'default_product_id': parent.product_id, 'active_picking_id': picking_id}"/>
                             <field name="lot_name" invisible="not context.get('show_lots_text')" groups="stock.group_production_lot"/>
                             <field name="package_id" invisible="not context.get('show_package')" groups="stock.group_tracking_lot"/>
                             <field name="result_package_id" groups="stock.group_tracking_lot"/>