[FIX] stock: move line: write

Write is overridden on reserved move lines to make sure the quants are
correctly updated. It is divided in two parts: if you update the reserved
quantity and if you update a characteristic. If somehow both are updated
in the same call (which doesn't happen in the standard interface), then
the quants are unreserved two times, which is wrong.

This issue showcased by [0] and on databases where the reserved
quantities are made editable by customization.

We grouped the two update in one, such as the TODO indicated us since a
long, long time.
We removed `result_packaged_id` from the trigger because it should not
impact the reservation whatsoever.
We needed to remove a tricky logic in the stock move's backordering part
but it was fishy anyway.

[0] eac8c06e2233

closes odoo/odoo#53928

X-original-commit: 507101434f9221f2cfa6de66e1952676fff322fd
Signed-off-by: Simon Lejeune (sle) <sle@openerp.com>

3 files changed, 63 insertions, 37 deletions

diff --git a/addons/stock/models/stock_move.py b/addons/stock/models/stock_move.py
index 7256e54c5e6..b196ec1adc2 100644
--- a/addons/stock/models/stock_move.py
+++ b/addons/stock/models/stock_move.py
@@ -1420,15 +1420,6 @@ class StockMove(models.Model):
                 # Need to do some kind of conversion here
                 qty_split = move.product_uom._compute_quantity(move.product_uom_qty - move.quantity_done, move.product_id.uom_id, rounding_method='HALF-UP')
                 new_move = move._split(qty_split)
-                for move_line in move.move_line_ids:
-                    if move_line.product_qty and move_line.qty_done:
-                        # FIXME: there will be an issue if the move was partially available
-                        # By decreasing `product_qty`, we free the reservation.
-                        # FIXME: if qty_done > product_qty, this could raise if nothing is in stock
-                        try:
-                            move_line.write({'product_uom_qty': move_line.qty_done})
-                        except UserError:
-                            pass
                 move._unreserve_initial_demand(new_move)
                 if cancel_backorder:
                     self.env['stock.move'].browse(new_move)._action_cancel()
diff --git a/addons/stock/models/stock_move_line.py b/addons/stock/models/stock_move_line.py
index 183f4312efd..079aad39305 100644
--- a/addons/stock/models/stock_move_line.py
+++ b/addons/stock/models/stock_move_line.py
@@ -222,10 +222,6 @@ class StockMoveLine(models.Model):
         return mls
 
     def write(self, vals):
-        """ Through the interface, we allow users to change the charateristics of a move line. If a
-        quantity has been reserved for this move line, we impact the reservation directly to free
-        the old quants and allocate the new ones.
-        """
         if self.env.context.get('bypass_reservation_update'):
             return super(StockMoveLine, self).write(vals)
 
@@ -235,27 +231,11 @@ class StockMoveLine(models.Model):
         moves_to_recompute_state = self.env['stock.move']
         Quant = self.env['stock.quant']
         precision = self.env['decimal.precision'].precision_get('Product Unit of Measure')
-        # We forbid to change the reserved quantity in the interace, but it is needed in the
-        # case of stock.move's split.
-        # TODO Move me in the update
-        if 'product_uom_qty' in vals:
-            for ml in self.filtered(lambda m: m.state in ('partially_available', 'assigned') and m.product_id.type == 'product'):
-                if not ml._should_bypass_reservation(ml.location_id):
-                    qty_to_decrease = ml.product_qty - ml.product_uom_id._compute_quantity(vals['product_uom_qty'], ml.product_id.uom_id, rounding_method='HALF-UP')
-                    try:
-                        Quant._update_reserved_quantity(ml.product_id, ml.location_id, -qty_to_decrease, lot_id=ml.lot_id, package_id=ml.package_id, owner_id=ml.owner_id, strict=True)
-                    except UserError:
-                        if ml.lot_id:
-                            Quant._update_reserved_quantity(ml.product_id, ml.location_id, -qty_to_decrease, lot_id=False, package_id=ml.package_id, owner_id=ml.owner_id, strict=True)
-                        else:
-                            raise
-
         triggers = [
             ('location_id', 'stock.location'),
             ('location_dest_id', 'stock.location'),
             ('lot_id', 'stock.production.lot'),
             ('package_id', 'stock.quant.package'),
-            ('result_package_id', 'stock.quant.package'),
             ('owner_id', 'res.partner')
         ]
         updates = {}
@@ -263,34 +243,53 @@ class StockMoveLine(models.Model):
             if key in vals:
                 updates[key] = self.env[model].browse(vals[key])
 
-        if updates:
+        # When we try to write on a reserved move line any fields from `triggers` or directly
+        # `product_uom_qty` (the actual reserved quantity), we need to make sure the associated
+        # quants are correctly updated in order to not make them out of sync (i.e. the sum of the
+        # move lines `product_uom_qty` should always be equal to the sum of `reserved_quantity` on
+        # the quants). If the new charateristics are not available on the quants, we chose to
+        # reserve the maximum possible.
+        if updates or 'product_uom_qty' in vals:
             for ml in self.filtered(lambda ml: ml.state in ['partially_available', 'assigned'] and ml.product_id.type == 'product'):
+
+                if 'product_uom_qty' in vals:
+                    new_product_uom_qty = ml.product_uom_id._compute_quantity(
+                        vals['product_uom_qty'], ml.product_id.uom_id, rounding_method='HALF-UP')
+                    # Make sure `product_uom_qty` is not negative.
+                    if float_compare(new_product_uom_qty, 0, precision_rounding=ml.product_id.uom_id.rounding) < 0:
+                        raise UserError(_('Reserving a negative quantity is not allowed.'))
+                else:
+                    new_product_uom_qty = ml.product_qty
+
+                # Unreserve the old charateristics of the move line.
                 if not ml._should_bypass_reservation(ml.location_id):
                     try:
                         Quant._update_reserved_quantity(ml.product_id, ml.location_id, -ml.product_qty, lot_id=ml.lot_id, package_id=ml.package_id, owner_id=ml.owner_id, strict=True)
                     except UserError:
+                        # If we were not able to unreserve on tracked quants, we can use untracked ones.
                         if ml.lot_id:
                             Quant._update_reserved_quantity(ml.product_id, ml.location_id, -ml.product_qty, lot_id=False, package_id=ml.package_id, owner_id=ml.owner_id, strict=True)
                         else:
                             raise
 
+                # Reserve the maximum available of the new charateristics of the move line.
                 if not ml._should_bypass_reservation(updates.get('location_id', ml.location_id)):
-                    new_product_qty = 0
+                    reserved_qty = 0
                     try:
-                        q = Quant._update_reserved_quantity(ml.product_id, updates.get('location_id', ml.location_id), ml.product_qty, lot_id=updates.get('lot_id', ml.lot_id),
+                        q = Quant._update_reserved_quantity(ml.product_id, updates.get('location_id', ml.location_id), new_product_uom_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)
-                        new_product_qty = sum([x[1] for x in q])
+                        reserved_qty = sum([x[1] for x in q])
                     except UserError:
                         if updates.get('lot_id'):
                             # If we were not able to reserve on tracked quants, we can use untracked ones.
                             try:
-                                q = Quant._update_reserved_quantity(ml.product_id, updates.get('location_id', ml.location_id), ml.product_qty, lot_id=False,
+                                q = Quant._update_reserved_quantity(ml.product_id, updates.get('location_id', ml.location_id), new_product_uom_qty, lot_id=False,
                                                                      package_id=updates.get('package_id', ml.package_id), owner_id=updates.get('owner_id', ml.owner_id), strict=True)
-                                new_product_qty = sum([x[1] for x in q])
+                                reserved_qty = sum([x[1] for x in q])
                             except UserError:
                                 pass
-                    if new_product_qty != ml.product_qty:
-                        new_product_uom_qty = ml.product_id.uom_id._compute_quantity(new_product_qty, ml.product_uom_id, rounding_method='HALF-UP')
+                    if reserved_qty != new_product_uom_qty:
+                        new_product_uom_qty = ml.product_id.uom_id._compute_quantity(reserved_qty, ml.product_uom_id, rounding_method='HALF-UP')
                         moves_to_recompute_state |= ml.move_id
                         ml.with_context(bypass_reservation_update=True).product_uom_qty = new_product_uom_qty
 
diff --git a/addons/stock/tests/test_packing.py b/addons/stock/tests/test_packing.py
index 41b07798f71..acdce51ef56 100644
--- a/addons/stock/tests/test_packing.py
+++ b/addons/stock/tests/test_packing.py
@@ -630,3 +630,39 @@ class TestPacking(TestPackingCommon):
             putaway_A.location_out_id.id,
             "The move line destination location must be the one from the picking.")
         internal_transfer.button_validate()
+
+    def test_partial_put_in_pack(self):
+        """ Create a simple move in a delivery. Reserve the quantity but set as quantity done only a part.
+        Call Put In Pack button. """
+        self.productA.tracking = 'lot'
+        lot1 = self.env['stock.production.lot'].create({
+            'product_id': self.productA.id,
+            'name': '00001',
+            'company_id': self.warehouse.company_id.id
+        })
+        self.env['stock.quant']._update_available_quantity(self.productA, self.stock_location, 20.0, lot_id=lot1)
+        ship_move_a = self.env['stock.move'].create({
+            'name': 'The ship move',
+            'product_id': self.productA.id,
+            'product_uom_qty': 5.0,
+            'product_uom': self.productA.uom_id.id,
+            'location_id': self.ship_location.id,
+            'location_dest_id': self.customer_location.id,
+            'warehouse_id': self.warehouse.id,
+            'picking_type_id': self.warehouse.out_type_id.id,
+            'procure_method': 'make_to_order',
+            'state': 'draft',
+        })
+        ship_move_a._assign_picking()
+        ship_move_a._action_confirm()
+        pack_move_a = ship_move_a.move_orig_ids[0]
+        pick_move_a = pack_move_a.move_orig_ids[0]
+
+        pick_picking = pick_move_a.picking_id
+
+        pick_picking.picking_type_id.show_entire_packs = True
+
+        pick_picking.action_assign()
+
+        pick_picking.move_line_ids.qty_done = 3
+        first_pack = pick_picking.put_in_pack()