From 7d9d6e8834c18ed9fa2189aed53b64ae9df1c024 Mon Sep 17 00:00:00 2001
From: Arnold Moyaux <arm@odoo.com>
Date: Wed, 20 Jul 2022 09:05:20 +0000
Subject: [PATCH] [FIX] stock: create/write on `stock.move.line`responssible
 for quant reservation

A lot of issue with "It is not possible to unreserve more products of ... than you have in stock".
It's the result of a desynchronisation between
`stock.move.line`.`reserved_qty` and `stock.quant`.`reserved_quantity` fields.

It should never happens in theory. However we already faced it a lot due
to bugs/custom code/server actions... It's not trivial to fix because
the issue come from data corruption. So it is hard to spot the different
main issues.

In order to avoid it, this commit try to modify the structure of stock
module. Before the operations was made in this order:
- The `stock.move` checks the quantity available on `stock.quant`
- The `stock.move` write the quantity reserved on the `stock.quant` and
  save it in a variable
- The `stock.move` create a `stock.move.line` with the quanity reserved.
The main issue is that a `stock.move.line` could be easily created with
a reserved quantity while the `stock.quant` are not update nor checked.

After this commit the operations will be:
- The `stock.move` checks the quantity available
- The `stock.move` dispatch the available quantity among the `stock.move.line`
  based on create or write calls.
- The `stock.move.line` reserve the quantity on `stock.quant`
- If the quantity is bigger than available, we write the max available on
  `stock.move.line`

The idea is to respect the different layers
`stock.move` <-> `stock.move.line` <-> `stock.quant`.
Avoid the interactions bewteen `stock.move` and `stock.quant`

This behavior is already well managed in other use cases.
E.g. the real quantity itself, `_do_unreserve` of `stock.move`,...

opw - a lot

Part-of: odoo/odoo#115328
---
 addons/mrp/models/mrp_production.py           | 10 +-
 addons/mrp/models/stock_move.py               |  5 -
 addons/mrp/tests/test_order.py                |  8 +-
 .../test_warehouse_multistep_manufacturing.py |  3 +-
 addons/mrp/wizard/change_production_qty.py    |  3 +-
 .../models/mrp_production.py                  |  4 +-
 .../models/stock_picking.py                   |  1 +
 addons/sale_stock/tests/test_sale_stock.py    |  3 +-
 addons/stock/models/stock_move.py             | 24 ++---
 addons/stock/models/stock_move_line.py        | 73 ++++++++------
 addons/stock/models/stock_quant.py            | 99 ++++++++++++-------
 .../tests/test_generate_serial_numbers.py     |  1 +
 addons/stock/tests/test_inventory.py          |  1 +
 addons/stock/tests/test_move.py               |  7 +-
 addons/stock/tests/test_quant.py              | 11 +--
 addons/stock/tests/test_stock_flow.py         |  6 +-
 16 files changed, 146 insertions(+), 113 deletions(-)

diff --git a/addons/mrp/models/mrp_production.py b/addons/mrp/models/mrp_production.py
index dee698355d1..cddccb861a6 100644
--- a/addons/mrp/models/mrp_production.py
+++ b/addons/mrp/models/mrp_production.py
@@ -1695,7 +1695,7 @@ class MrpProduction(models.Model):
                 # Unreserve the quantity removed from initial `stock.move.line` and
                 # not assigned to a move anymore. In case of a split smaller than initial
                 # quantity and fully reserved
-                if quantity:
+                if quantity and not move_line.move_id._should_bypass_reservation():
                     self.env['stock.quant']._update_reserved_quantity(
                         move_line.product_id, move_line.location_id, -quantity,
                         lot_id=move_line.lot_id, package_id=move_line.package_id,
@@ -1712,7 +1712,7 @@ class MrpProduction(models.Model):
         # Avoid triggering a useless _recompute_state
         self.env['stock.move.line'].browse(move_lines_to_unlink).write({'move_id': False})
         self.env['stock.move.line'].browse(move_lines_to_unlink).unlink()
-        self.env['stock.move.line'].create(move_lines_vals)
+        self.env['stock.move.line'].with_context(bypass_reservation_update=True).create(move_lines_vals)
 
         workorders_to_cancel = self.env['mrp.workorder']
         for production in self:
@@ -1851,7 +1851,7 @@ class MrpProduction(models.Model):
             order._check_sn_uniqueness()
 
     def do_unreserve(self):
-        self.move_raw_ids.filtered(lambda x: x.state not in ('done', 'cancel'))._do_unreserve()
+        (self.move_finished_ids | self.move_raw_ids).filtered(lambda x: x.state not in ('done', 'cancel'))._do_unreserve()
 
     def button_scrap(self):
         self.ensure_one()
@@ -2083,6 +2083,8 @@ class MrpProduction(models.Model):
             if move.has_tracking != 'serial' or move.product_id == self.product_id:
                 continue
             for move_line in move.move_line_ids:
+                if float_is_zero(move_line.qty_done, precision_rounding=move_line.product_uom_id.rounding):
+                    continue
                 if self._is_finished_sn_already_produced(move_line.lot_id, excluded_sml=move_line):
                     raise UserError(_('The serial number %(number)s used for byproduct %(product_name)s has already been produced',
                                       number=move_line.lot_id.name, product_name=move_line.product_id.name))
@@ -2129,6 +2131,8 @@ class MrpProduction(models.Model):
                     raise UserError(message)
 
     def _is_finished_sn_already_produced(self, lot, excluded_sml=None):
+        if not lot:
+            return False
         excluded_sml = excluded_sml or self.env['stock.move.line']
         domain = [
             ('lot_id', '=', lot.id),
diff --git a/addons/mrp/models/stock_move.py b/addons/mrp/models/stock_move.py
index 1bd45236019..9e5537c9310 100644
--- a/addons/mrp/models/stock_move.py
+++ b/addons/mrp/models/stock_move.py
@@ -478,11 +478,6 @@ class StockMove(models.Model):
             return True
         return False
 
-    def _should_bypass_reservation(self, forced_location=False):
-        res = super(StockMove, self)._should_bypass_reservation(
-            forced_location=forced_location)
-        return bool(res and not self.production_id)
-
     def _key_assign_picking(self):
         keys = super(StockMove, self)._key_assign_picking()
         return keys + (self.created_production_id,)
diff --git a/addons/mrp/tests/test_order.py b/addons/mrp/tests/test_order.py
index 5e4f968a938..80ec4aad926 100644
--- a/addons/mrp/tests/test_order.py
+++ b/addons/mrp/tests/test_order.py
@@ -1005,19 +1005,19 @@ class TestMrpOrder(TestMrpCommon):
         self.assertEqual(len(move_byproduct_1), 1)
         self.assertEqual(move_byproduct_1.product_uom_qty, 2.0)
         self.assertEqual(move_byproduct_1.quantity_done, 0)
-        self.assertEqual(len(move_byproduct_1.move_line_ids), 0)
+        self.assertEqual(len(move_byproduct_1.move_line_ids), 2)
 
         move_byproduct_2 = mo.move_finished_ids.filtered(lambda l: l.product_id == self.byproduct2)
         self.assertEqual(len(move_byproduct_2), 1)
         self.assertEqual(move_byproduct_2.product_uom_qty, 4.0)
         self.assertEqual(move_byproduct_2.quantity_done, 0)
-        self.assertEqual(len(move_byproduct_2.move_line_ids), 0)
+        self.assertEqual(len(move_byproduct_2.move_line_ids), 1)
 
         move_byproduct_3 = mo.move_finished_ids.filtered(lambda l: l.product_id == self.byproduct3)
         self.assertEqual(move_byproduct_3.product_uom_qty, 4.0)
         self.assertEqual(move_byproduct_3.quantity_done, 0)
         self.assertEqual(move_byproduct_3.product_uom, dozen)
-        self.assertEqual(len(move_byproduct_3.move_line_ids), 0)
+        self.assertEqual(len(move_byproduct_3.move_line_ids), 1)
 
         mo_form = Form(mo)
         mo_form.qty_producing = 1.0
@@ -1043,7 +1043,7 @@ class TestMrpOrder(TestMrpCommon):
             ml.qty_done = 1
         details_operation_form.save()
         details_operation_form = Form(move_byproduct_2, view=self.env.ref('stock.view_stock_move_operations'))
-        with details_operation_form.move_line_ids.new() as ml:
+        with details_operation_form.move_line_ids.edit(0) as ml:
             ml.lot_id = self.lot_1
             ml.qty_done = 2
         details_operation_form.save()
diff --git a/addons/mrp/tests/test_warehouse_multistep_manufacturing.py b/addons/mrp/tests/test_warehouse_multistep_manufacturing.py
index 6e760a60435..4abcd0382dd 100644
--- a/addons/mrp/tests/test_warehouse_multistep_manufacturing.py
+++ b/addons/mrp/tests/test_warehouse_multistep_manufacturing.py
@@ -485,7 +485,7 @@ class TestMultistepManufacturingWarehouse(TestMrpCommon):
         """
 
         self.warehouse.mto_pull_id.route_id.active = True
-        # Creating complex product which trigger another manifacture
+        # Creating complex product which trigger another manufacture
 
         routes = self.warehouse.manufacture_pull_id.route_id + self.warehouse.mto_pull_id.route_id
         self.complex_product = self.env['product.product'].create({
@@ -525,6 +525,7 @@ class TestMultistepManufacturingWarehouse(TestMrpCommon):
         production_form.picking_type_id = self.warehouse.manu_type_id
         production = production_form.save()
         production.action_confirm()
+        self.env.invalidate_all()
 
         move_raw_ids = production.move_raw_ids
         self.assertEqual(len(move_raw_ids), 2)
diff --git a/addons/mrp/wizard/change_production_qty.py b/addons/mrp/wizard/change_production_qty.py
index dffb9673c9a..73da124d9cb 100644
--- a/addons/mrp/wizard/change_production_qty.py
+++ b/addons/mrp/wizard/change_production_qty.py
@@ -45,7 +45,8 @@ class ChangeProductionQty(models.TransientModel):
                 move.write({'product_uom_qty': move.product_uom_qty + qty})
 
         if push_moves:
-            push_moves._action_confirm()._action_assign()
+            push_moves._action_confirm()
+        production.move_finished_ids._action_assign()
 
         return modification
 
diff --git a/addons/mrp_subcontracting/models/mrp_production.py b/addons/mrp_subcontracting/models/mrp_production.py
index 60b554bad83..2ec044f77f7 100644
--- a/addons/mrp_subcontracting/models/mrp_production.py
+++ b/addons/mrp_subcontracting/models/mrp_production.py
@@ -137,11 +137,11 @@ class MrpProduction(models.Model):
                         'qty_done': new_quantity_done,
                         'lot_id': self.lot_producing_id and self.lot_producing_id.id,
                     }
-                    ml.copy(default=default)
-                    ml.with_context(bypass_reservation_update=True).write({
+                    ml.write({
                         'reserved_uom_qty': new_qty_reserved,
                         'qty_done': 0
                     })
+                    ml.copy(default=default)
 
             if float_compare(quantity, 0, precision_rounding=self.product_uom_id.rounding) > 0:
                 self.env['stock.move.line'].create({
diff --git a/addons/mrp_subcontracting/models/stock_picking.py b/addons/mrp_subcontracting/models/stock_picking.py
index b41aeb23fa9..6f08c82cd50 100644
--- a/addons/mrp_subcontracting/models/stock_picking.py
+++ b/addons/mrp_subcontracting/models/stock_picking.py
@@ -70,6 +70,7 @@ class StockPicking(models.Model):
             amounts = [move_line.qty_done for move_line in move.move_line_ids]
             len_amounts = len(amounts)
             productions = production._split_productions({production: amounts}, set_consumed_qty=True)
+            productions.move_finished_ids.move_line_ids.write({'qty_done': 0})
             for production, move_line in zip(productions, move.move_line_ids):
                 if move_line.lot_id:
                     production.lot_producing_id = move_line.lot_id
diff --git a/addons/sale_stock/tests/test_sale_stock.py b/addons/sale_stock/tests/test_sale_stock.py
index 1d5db1b20ed..e6bd2700823 100644
--- a/addons/sale_stock/tests/test_sale_stock.py
+++ b/addons/sale_stock/tests/test_sale_stock.py
@@ -1262,10 +1262,9 @@ class TestSaleStock(TestSaleCommon, ValuationReconciliationTestCommon):
         so = so_form.save()
         so.action_confirm()
 
-        pick_picking, pack_picking, _ = so.picking_ids
+        _, pack_picking, pick_picking = so.picking_ids
         (pick_picking + pack_picking).move_ids.quantity_done = 5
         (pick_picking + pack_picking).button_validate()
-
         with Form(so) as so_form:
             with so_form.order_line.edit(0) as line:
                 line.product_uom_qty = 3
diff --git a/addons/stock/models/stock_move.py b/addons/stock/models/stock_move.py
index a9ef6fc724d..335db29f46f 100644
--- a/addons/stock/models/stock_move.py
+++ b/addons/stock/models/stock_move.py
@@ -1445,6 +1445,7 @@ Please change the quantity done or the rounding precision of your unit of measur
             'company_id': self.company_id.id,
         }
         if quantity:
+            # TODO could be also move in create/write
             rounding = self.env['decimal.precision'].precision_get('Product Unit of Measure')
             uom_quantity = self.product_id.uom_id._compute_quantity(quantity, self.product_uom, rounding_method='HALF-UP')
             uom_quantity = float_round(uom_quantity, precision_digits=rounding)
@@ -1502,18 +1503,13 @@ Please change the quantity done or the rounding precision of your unit of measur
             if float_compare(taken_quantity, int(taken_quantity), precision_digits=rounding) != 0:
                 taken_quantity = 0
 
-        try:
-            with self.env.cr.savepoint():
-                if not float_is_zero(taken_quantity, precision_rounding=self.product_id.uom_id.rounding):
-                    quants = self.env['stock.quant']._update_reserved_quantity(
-                        self.product_id, location_id, taken_quantity, lot_id=lot_id,
-                        package_id=package_id, owner_id=owner_id, strict=strict
-                    )
-        except UserError:
-            taken_quantity = 0
+        if not float_is_zero(taken_quantity, precision_rounding=self.product_id.uom_id.rounding):
+            quants = self.env['stock.quant']._get_reserve_quantity(
+                self.product_id, location_id, taken_quantity, lot_id=lot_id,
+                package_id=package_id, owner_id=owner_id, strict=strict
+            )
 
         # Find a candidate move line to update or create a new one.
-        serial_move_line_vals = []
         for reserved_quant, quantity in quants:
             to_update = next((line for line in self.move_line_ids if line._reservation_is_updatable(quantity, reserved_quant)), False)
             if to_update:
@@ -1521,14 +1517,12 @@ Please change the quantity done or the rounding precision of your unit of measur
                 uom_quantity = float_round(uom_quantity, precision_digits=rounding)
                 uom_quantity_back_to_product_uom = to_update.product_uom_id._compute_quantity(uom_quantity, self.product_id.uom_id, rounding_method='HALF-UP')
             if to_update and float_compare(quantity, uom_quantity_back_to_product_uom, precision_digits=rounding) == 0:
-                to_update.with_context(bypass_reservation_update=True).reserved_uom_qty += uom_quantity
+                to_update.with_context(reserved_quant=reserved_quant).reserved_uom_qty += uom_quantity
             else:
                 if self.product_id.tracking == 'serial':
-                    # Move lines with serial tracked product_id cannot be to-update candidates. Delay the creation to speed up candidates search + create.
-                    serial_move_line_vals.extend([self._prepare_move_line_vals(quantity=1, reserved_quant=reserved_quant) for i in range(int(quantity))])
+                    self.env['stock.move.line'].with_context(reserved_quant=reserved_quant).create([self._prepare_move_line_vals(quantity=1, reserved_quant=reserved_quant) for i in range(int(quantity))])
                 else:
-                    self.env['stock.move.line'].create(self._prepare_move_line_vals(quantity=quantity, reserved_quant=reserved_quant))
-        self.env['stock.move.line'].create(serial_move_line_vals)
+                    self.env['stock.move.line'].with_context(reserved_quant=reserved_quant).create(self._prepare_move_line_vals(quantity=quantity, reserved_quant=reserved_quant))
         return taken_quantity
 
     def _should_bypass_reservation(self, forced_location=False):
diff --git a/addons/stock/models/stock_move_line.py b/addons/stock/models/stock_move_line.py
index aff746e0d23..9287a3667bb 100644
--- a/addons/stock/models/stock_move_line.py
+++ b/addons/stock/models/stock_move_line.py
@@ -311,6 +311,26 @@ class StockMoveLine(models.Model):
             else:
                 create_move(move_line)
 
+        for move_line in mls:
+            reserved_uom_qty = move_line.reserved_uom_qty
+            location = move_line.location_id
+            product = move_line.product_id
+            move = move_line.move_id
+            if move:
+                bypass_reservation = not move._should_bypass_reservation()
+            else:
+                bypass_reservation = product.type == 'product' and not location.should_bypass_reservation()
+            if reserved_uom_qty and not self.env.context.get('bypass_reservation_update') and bypass_reservation:
+                ml_uom = move_line.product_uom_id
+
+                reserved_qty = ml_uom._compute_quantity(reserved_uom_qty, product.uom_id, rounding_method='HALF-UP')
+                reserved_quants = self.env.context.get('reserved_quant', self.env['stock.quant'])._update_reserved_quantity(
+                    product, location, reserved_qty, lot_id=move_line.lot_id, package_id=move_line.package_id, owner_id=move_line.owner_id, strict=True)
+
+                if not reserved_quants or float_compare(reserved_qty, sum(map(lambda q: q[1], reserved_quants)), product.uom_id.rounding) != 0:
+                    reserved_uom_qty = product.uom_id._compute_quantity(reserved_qty, ml_uom, rounding_method='HALF-UP')
+                    move_line.with_context(bypass_reservation=True).reserved_uom_qty = reserved_uom_qty
+
         for ml, vals in zip(mls, vals_list):
             if ml.move_id and \
                     ml.move_id.picking_id and \
@@ -340,9 +360,10 @@ class StockMoveLine(models.Model):
         return mls
 
     def write(self, vals):
+        # Very dangerous key. It could create some desynchronization between `stock.move.line` and `stock.quant`.
+        # It exists for performances purposes. Only use it if you handle `stock.quant` reservation manualy.
         if self.env.context.get('bypass_reservation_update'):
-            return super(StockMoveLine, self).write(vals)
-
+            return super().write(vals)
         if 'product_id' in vals and any(vals.get('state', ml.state) != 'draft' and vals['product_id'] != ml.product_id.id for ml in self):
             raise UserError(_("Changing the product is only allowed in 'Draft' state."))
 
@@ -382,40 +403,35 @@ class StockMoveLine(models.Model):
         # the quants). If the new charateristics are not available on the quants, we chose to
         # reserve the maximum possible.
         if updates or 'reserved_uom_qty' in vals:
-            for ml in self.filtered(lambda ml: ml.state in ['partially_available', 'assigned', 'confirmed'] and ml.product_id.type == 'product'):
-
+            for ml in self:
+                if ml.product_id.type != 'product':
+                    continue
                 if 'reserved_uom_qty' in vals:
-                    new_reserved_uom_qty = ml.product_uom_id._compute_quantity(
+                    new_reserved_qty = ml.product_uom_id._compute_quantity(
                         vals['reserved_uom_qty'], ml.product_id.uom_id, rounding_method='HALF-UP')
                     # Make sure `reserved_uom_qty` is not negative.
-                    if float_compare(new_reserved_uom_qty, 0, precision_rounding=ml.product_id.uom_id.rounding) < 0:
+                    if float_compare(new_reserved_qty, 0, precision_rounding=ml.product_id.uom_id.rounding) < 0:
                         raise UserError(_('Reserving a negative quantity is not allowed.'))
                 else:
-                    new_reserved_uom_qty = ml.reserved_qty
+                    new_reserved_qty = ml.reserved_qty
 
                 # Unreserve the old charateristics of the move line.
-                if not ml.move_id._should_bypass_reservation(ml.location_id):
-                    Quant._update_reserved_quantity(ml.product_id, ml.location_id, -ml.reserved_qty, lot_id=ml.lot_id, package_id=ml.package_id, owner_id=ml.owner_id, strict=True)
+                if not float_is_zero(ml.reserved_qty, precision_rounding=ml.product_uom_id.rounding):
+                    if not ml.move_id._should_bypass_reservation(ml.location_id):
+                        Quant._update_reserved_quantity(ml.product_id, ml.location_id, -ml.reserved_qty, lot_id=ml.lot_id, package_id=ml.package_id, owner_id=ml.owner_id, strict=True)
 
                 # Reserve the maximum available of the new charateristics of the move line.
+                reserved_qty = new_reserved_qty
                 if not ml.move_id._should_bypass_reservation(updates.get('location_id', ml.location_id)):
-                    reserved_qty = 0
-                    try:
-                        available_qty = Quant._get_available_quantity(ml.product_id, updates.get('location_id', ml.location_id), lot_id=updates.get('lot_id', ml.lot_id),
-                                                                      package_id=updates.get('package_id', ml.package_id), owner_id=updates.get('owner_id', ml.owner_id), strict=True)
-                        to_reserve = min(available_qty, new_reserved_uom_qty)
-                        q = []
-                        if to_reserve:
-                            q = Quant._update_reserved_quantity(ml.product_id, updates.get('location_id', ml.location_id), to_reserve, lot_id=updates.get('lot_id', ml.lot_id),
-                                                                package_id=updates.get('package_id', ml.package_id), owner_id=updates.get('owner_id', ml.owner_id), strict=True)
-                        reserved_qty = sum([x[1] for x in q])
-                    except UserError:
-                        pass
-                    if reserved_qty != new_reserved_uom_qty:
-                        new_reserved_uom_qty = ml.product_id.uom_id._compute_quantity(reserved_qty, ml.product_uom_id, rounding_method='HALF-UP')
-                        ml.with_context(bypass_reservation_update=True).reserved_uom_qty = new_reserved_uom_qty
-                        # we don't want to override the new reserved quantity
-                        vals.pop('reserved_uom_qty', None)
+                    q = Quant._update_reserved_quantity(ml.product_id, updates.get('location_id', ml.location_id), new_reserved_qty, lot_id=updates.get('lot_id', ml.lot_id),
+                                                        package_id=updates.get('package_id', ml.package_id), owner_id=updates.get('owner_id', ml.owner_id), strict=True)
+                    reserved_qty = sum([x[1] for x in q])
+
+                if reserved_qty != new_reserved_qty:
+                    reserved_uom_qty = ml.product_id.uom_id._compute_quantity(reserved_qty, ml.product_uom_id, rounding_method='HALF-UP')
+                    vals['reserved_uom_qty'] = reserved_uom_qty
+
+                if 'reserved_uom_qty' in vals and vals['reserved_uom_qty'] != ml.reserved_uom_qty:
                     moves_to_recompute_state |= ml.move_id
 
         # When editing a done move line, the reserved availability of a potential chained move is impacted. Take care of running again `_action_assign` on the concerned moves.
@@ -593,9 +609,6 @@ class StockMoveLine(models.Model):
                     qty_done_product_uom = ml.product_uom_id._compute_quantity(ml.qty_done, ml.product_id.uom_id, rounding_method='HALF-UP')
                     extra_qty = qty_done_product_uom - ml.reserved_qty
                     ml._free_reservation(ml.product_id, ml.location_id, extra_qty, lot_id=ml.lot_id, package_id=ml.package_id, owner_id=ml.owner_id, ml_ids_to_ignore=ml_ids_to_ignore)
-                # unreserve what's been reserved
-                if not ml.move_id._should_bypass_reservation(ml.location_id) and ml.product_id.type == 'product' and ml.reserved_qty:
-                    Quant._update_reserved_quantity(ml.product_id, ml.location_id, -ml.reserved_qty, lot_id=ml.lot_id, package_id=ml.package_id, owner_id=ml.owner_id, strict=True)
 
                 # move what's been actually done
                 quantity = ml.product_uom_id._compute_quantity(ml.qty_done, ml.move_id.product_id.uom_id, rounding_method='HALF-UP')
@@ -610,7 +623,7 @@ class StockMoveLine(models.Model):
                 Quant._update_available_quantity(ml.product_id, ml.location_dest_id, quantity, lot_id=ml.lot_id, package_id=ml.result_package_id, owner_id=ml.owner_id, in_date=in_date)
             ml_ids_to_ignore.add(ml.id)
         # Reset the reserved quantity as we just moved it to the destination location.
-        mls_todo.with_context(bypass_reservation_update=True).write({
+        mls_todo.write({
             'reserved_uom_qty': 0.00,
             'date': fields.Datetime.now(),
         })
diff --git a/addons/stock/models/stock_quant.py b/addons/stock/models/stock_quant.py
index 78855a45e51..67a51966e2f 100644
--- a/addons/stock/models/stock_quant.py
+++ b/addons/stock/models/stock_quant.py
@@ -4,6 +4,7 @@
 import logging
 
 from ast import literal_eval
+from collections import defaultdict
 from psycopg2 import Error
 
 from odoo import _, api, fields, models
@@ -625,6 +626,62 @@ class StockQuant(models.Model):
             else:
                 return sum([available_quantity for available_quantity in availaible_quantities.values() if float_compare(available_quantity, 0, precision_rounding=rounding) > 0])
 
+    def _get_reserve_quantity(self, product_id, location_id, quantity, lot_id=None, package_id=None, owner_id=None, strict=False):
+        """ Get the quantity available to reserve for the set of quants
+        sharing the combination of `product_id, location_id` if `strict` is set to False or sharing
+        the *exact same characteristics* otherwise. Typically, this method is called before the
+        `stock.move.line` creation to know the reserved_qty that could be use. It's also call by
+        `_update_reserve_quantity` to find the quant to reserve.
+
+        :return: a list of tuples (quant, quantity_reserved) showing on which quant the reservation
+            could be done and how much the system is able to reserve on it
+        """
+        self = self.sudo()
+        rounding = product_id.uom_id.rounding
+        quants = self or self._gather(product_id, location_id, lot_id=lot_id, package_id=package_id, owner_id=owner_id, strict=strict)
+        reserved_quants = []
+
+        if float_compare(quantity, 0, precision_rounding=rounding) > 0:
+            # if we want to reserve
+            available_quantity = sum(quants.filtered(lambda q: float_compare(q.quantity, 0, precision_rounding=rounding) > 0).mapped('quantity')) - sum(quants.mapped('reserved_quantity'))
+        elif float_compare(quantity, 0, precision_rounding=rounding) < 0:
+            # if we want to unreserve
+            available_quantity = sum(quants.mapped('reserved_quantity'))
+            if float_compare(abs(quantity), available_quantity, precision_rounding=rounding) > 0:
+                raise UserError(_('It is not possible to unreserve more products of %s than you have in stock.', product_id.display_name))
+        else:
+            return reserved_quants
+
+        negative_reserved_quantity = defaultdict(float)
+        for quant in quants:
+            if float_compare(quant.quantity - quant.reserved_quantity, 0, precision_rounding=rounding) < 0:
+                negative_reserved_quantity[(quant.location_id, quant.lot_id, quant.package_id, quant.owner_id)] += quant.quantity - quant.reserved_quantity
+        for quant in quants:
+            if float_compare(quantity, 0, precision_rounding=rounding) > 0:
+                max_quantity_on_quant = quant.quantity - quant.reserved_quantity
+                if float_compare(max_quantity_on_quant, 0, precision_rounding=rounding) <= 0:
+                    continue
+                negative_quantity = negative_reserved_quantity[(quant.location_id, quant.lot_id, quant.package_id, quant.owner_id)]
+                if negative_quantity:
+                    negative_qty_to_remove = min(abs(negative_quantity), max_quantity_on_quant)
+                    negative_reserved_quantity[(quant.location_id, quant.lot_id, quant.package_id, quant.owner_id)] += negative_qty_to_remove
+                    max_quantity_on_quant -= negative_qty_to_remove
+                if float_compare(max_quantity_on_quant, 0, precision_rounding=rounding) <= 0:
+                    continue
+                max_quantity_on_quant = min(max_quantity_on_quant, quantity)
+                reserved_quants.append((quant, max_quantity_on_quant))
+                quantity -= max_quantity_on_quant
+                available_quantity -= max_quantity_on_quant
+            else:
+                max_quantity_on_quant = min(quant.reserved_quantity, abs(quantity))
+                reserved_quants.append((quant, -max_quantity_on_quant))
+                quantity += max_quantity_on_quant
+                available_quantity += max_quantity_on_quant
+
+            if float_is_zero(quantity, precision_rounding=rounding) or float_is_zero(available_quantity, precision_rounding=rounding):
+                break
+        return reserved_quants
+
     @api.onchange('location_id', 'product_id', 'lot_id', 'package_id', 'owner_id')
     def _onchange_location_or_product_id(self):
         vals = {}
@@ -773,7 +830,6 @@ class StockQuant(models.Model):
             })
         return self._get_available_quantity(product_id, location_id, lot_id=lot_id, package_id=package_id, owner_id=owner_id, strict=False, allow_negative=True), in_date
 
-    @api.model
     def _update_reserved_quantity(self, product_id, location_id, quantity, lot_id=None, package_id=None, owner_id=None, strict=False):
         """ Increase the reserved quantity, i.e. increase `reserved_quantity` for the set of quants
         sharing the combination of `product_id, location_id` if `strict` is set to False or sharing
@@ -787,43 +843,10 @@ class StockQuant(models.Model):
             was done and how much the system was able to reserve on it
         """
         self = self.sudo()
-        rounding = product_id.uom_id.rounding
-        quants = self._gather(product_id, location_id, lot_id=lot_id, package_id=package_id, owner_id=owner_id, strict=strict)
-        reserved_quants = []
-
-        if float_compare(quantity, 0, precision_rounding=rounding) > 0:
-            # if we want to reserve
-            available_quantity = sum(quants.filtered(lambda q: float_compare(q.quantity, 0, precision_rounding=rounding) > 0).mapped('quantity')) - sum(quants.mapped('reserved_quantity'))
-            if float_compare(quantity, available_quantity, precision_rounding=rounding) > 0:
-                raise UserError(_('It is not possible to reserve more products of %s than you have in stock.', product_id.display_name))
-        elif float_compare(quantity, 0, precision_rounding=rounding) < 0:
-            # if we want to unreserve
-            available_quantity = sum(quants.mapped('reserved_quantity'))
-            if float_compare(abs(quantity), available_quantity, precision_rounding=rounding) > 0:
-                raise UserError(_('It is not possible to unreserve more products of %s than you have in stock.', product_id.display_name))
-        else:
-            return reserved_quants
-
-        for quant in quants:
-            if float_compare(quantity, 0, precision_rounding=rounding) > 0:
-                max_quantity_on_quant = quant.quantity - quant.reserved_quantity
-                if float_compare(max_quantity_on_quant, 0, precision_rounding=rounding) <= 0:
-                    continue
-                max_quantity_on_quant = min(max_quantity_on_quant, quantity)
-                quant.reserved_quantity += max_quantity_on_quant
-                reserved_quants.append((quant, max_quantity_on_quant))
-                quantity -= max_quantity_on_quant
-                available_quantity -= max_quantity_on_quant
-            else:
-                max_quantity_on_quant = min(quant.reserved_quantity, abs(quantity))
-                quant.reserved_quantity -= max_quantity_on_quant
-                reserved_quants.append((quant, -max_quantity_on_quant))
-                quantity += max_quantity_on_quant
-                available_quantity += max_quantity_on_quant
-
-            if float_is_zero(quantity, precision_rounding=rounding) or float_is_zero(available_quantity, precision_rounding=rounding):
-                break
-        return reserved_quants
+        quants_to_reserve = self._get_reserve_quantity(product_id, location_id, quantity, lot_id=lot_id, package_id=package_id, owner_id=owner_id, strict=strict)
+        for quant, qty_to_reserve in quants_to_reserve:
+            quant.reserved_quantity += qty_to_reserve
+        return quants_to_reserve
 
     @api.model
     def _unlink_zero_quants(self):
diff --git a/addons/stock/tests/test_generate_serial_numbers.py b/addons/stock/tests/test_generate_serial_numbers.py
index 377c6a9a837..ce7a180749a 100644
--- a/addons/stock/tests/test_generate_serial_numbers.py
+++ b/addons/stock/tests/test_generate_serial_numbers.py
@@ -36,6 +36,7 @@ class StockGenerate(TransactionCase):
 
     def get_new_move(self, nbre_of_lines):
         move_lines_val = []
+        self.env['stock.quant']._update_available_quantity(self.product_serial, self.location, nbre_of_lines)
         for i in range(nbre_of_lines):
             move_lines_val.append({
                 'product_id': self.product_serial.id,
diff --git a/addons/stock/tests/test_inventory.py b/addons/stock/tests/test_inventory.py
index 5a8a585bc1b..0ae2941910b 100644
--- a/addons/stock/tests/test_inventory.py
+++ b/addons/stock/tests/test_inventory.py
@@ -393,6 +393,7 @@ class TestInventory(TransactionCase):
             'product_uom': self.uom_unit.id,
             'product_uom_qty': 4.0,
         })
+        quant.invalidate_recordset()
         move_out._action_confirm()
         move_out._action_assign()
         move_out.move_line_ids.qty_done = 4
diff --git a/addons/stock/tests/test_move.py b/addons/stock/tests/test_move.py
index 4f277e44262..682619157a4 100644
--- a/addons/stock/tests/test_move.py
+++ b/addons/stock/tests/test_move.py
@@ -3612,8 +3612,9 @@ class StockMove(TransactionCase):
     def test_edit_reserved_move_line_9(self):
         """
         When writing on the reserved quantity on the SML, a process tries to
-        reserve the quants with that new quantity. If the written quantity is
-        more than actually available, this quantity should be set to the available quantity.
+        reserve the quants with that new quantity. If it fails (for instance
+        because the written quantity is more than actually available), it should
+        take the maximum available.
         """
         self.env['stock.quant']._update_available_quantity(self.product, self.stock_location, 1.0)
 
@@ -3632,7 +3633,7 @@ class StockMove(TransactionCase):
         out_move.move_line_ids.reserved_uom_qty = 2
 
         self.assertTrue(out_move.move_line_ids)
-        self.assertEqual(out_move.move_line_ids.reserved_uom_qty, 1, "The reserved quantity should be what is available")
+        self.assertEqual(out_move.move_line_ids.reserved_uom_qty, 1, "The maximum available still one")
 
     def test_edit_done_move_line_1(self):
         """ Test that editing a done stock move line linked to an untracked product correctly and
diff --git a/addons/stock/tests/test_quant.py b/addons/stock/tests/test_quant.py
index b233788a6fd..c70f746a1c6 100644
--- a/addons/stock/tests/test_quant.py
+++ b/addons/stock/tests/test_quant.py
@@ -410,15 +410,15 @@ class StockQuant(TransactionCase):
         })
         self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product, self.stock_location), 0.0)
         self.assertEqual(len(self.gather_relevant(self.product, self.stock_location)), 2)
-        with self.assertRaises(UserError):
-            self.env['stock.quant']._update_reserved_quantity(self.product, self.stock_location, 10.0)
+        reserved_quants = self.env['stock.quant']._update_reserved_quantity(self.product, self.stock_location, 10.0)
+        self.assertFalse(reserved_quants)
         self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product, self.stock_location), 0.0)
 
     def test_increase_reserved_quantity_5(self):
         """ Decrease the available quantity when no quant are in a location.
         """
-        with self.assertRaises(UserError):
-            self.env['stock.quant']._update_reserved_quantity(self.product, self.stock_location, 1.0)
+        reserved_quants = self.env['stock.quant']._update_reserved_quantity(self.product, self.stock_location, 1.0)
+        self.assertFalse(reserved_quants)
         self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product, self.stock_location), 0.0)
 
     def test_decrease_reserved_quantity_1(self):
@@ -437,8 +437,7 @@ class StockQuant(TransactionCase):
     def test_increase_decrease_reserved_quantity_1(self):
         """ Decrease then increase reserved quantity when no quant are in a location.
         """
-        with self.assertRaises(UserError):
-            self.env['stock.quant']._update_reserved_quantity(self.product, self.stock_location, 1.0)
+        self.env['stock.quant']._update_reserved_quantity(self.product, self.stock_location, 1.0)
         self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product, self.stock_location), 0.0)
         with self.assertRaises(UserError):
             self.env['stock.quant']._update_reserved_quantity(self.product, self.stock_location, -1.0, strict=True)
diff --git a/addons/stock/tests/test_stock_flow.py b/addons/stock/tests/test_stock_flow.py
index 8a3c5d7ab57..463e63cfa9e 100644
--- a/addons/stock/tests/test_stock_flow.py
+++ b/addons/stock/tests/test_stock_flow.py
@@ -1319,7 +1319,7 @@ class TestStockFlow(TestStockCommon):
         pack2 = pack_obj.create({'name': 'PACKINOUTTEST2'})
         picking_in.move_line_ids[0].result_package_id = pack1
         picking_in.move_line_ids[0].qty_done = 4
-        packop2 = picking_in.move_line_ids[0].with_context(bypass_reservation_update=True).copy({'reserved_uom_qty': 0})
+        packop2 = picking_in.move_line_ids[0].copy({'reserved_uom_qty': 0})
         packop2.qty_done = 6
         packop2.result_package_id = pack2
         picking_in._action_done()
@@ -1343,7 +1343,7 @@ class TestStockFlow(TestStockCommon):
         picking_out.action_confirm()
         picking_out.action_assign()
         packout1 = picking_out.move_line_ids[0]
-        packout2 = picking_out.move_line_ids[0].with_context(bypass_reservation_update=True).copy({'reserved_uom_qty': 0})
+        packout2 = picking_out.move_line_ids[0].copy({'reserved_uom_qty': 0})
         packout1.qty_done = 2
         packout1.package_id = pack1
         packout2.package_id = pack2
@@ -1377,7 +1377,7 @@ class TestStockFlow(TestStockCommon):
         pack2 = pack_obj.create({'name': 'PACKINOUTTEST2'})
         picking_in.move_line_ids[0].result_package_id = pack1
         picking_in.move_line_ids[0].qty_done = 120
-        packop2 = picking_in.move_line_ids[0].with_context(bypass_reservation_update=True).copy({'reserved_uom_qty': 0})
+        packop2 = picking_in.move_line_ids[0].copy({'reserved_uom_qty': 0})
         packop2.qty_done = 80
         packop2.result_package_id = pack2
         picking_in._action_done()