[FIX] stock: do not unreserve proccessed move line

When a user unreserve a picking or a move it will
drop all the linked move lines. However it could
happens that some move line already have a done quantity
and the user won't lose this information.

This commit filters move lines in order to only drop
the move line without quantity done and remove the
reserved quantity on others.

Joint work with William Henrotin <whe@odoo.com>

task-2069646

addons/stock/models/stock_move.py

@@ -621,18 +621,30 @@ def action_assign_serial(self):
 
     def _do_unreserve(self):
         moves_to_unreserve = self.env['stock.move']
+        moves_not_to_recompute = self.env['stock.move']
         for move in self:
-            if move.state == 'cancel':
+            if move.state == 'cancel' or (move.state == 'done' and move.scrapped):
                 # We may have cancelled move in an open picking in a "propagate_cancel" scenario.
+                # We may have done move in an open picking in a scrap scenario.
                 continue
-            if move.state == 'done':
-                if move.scrapped:
-                    # We may have done move in an open picking in a scrap scenario.
-                    continue
-                else:
-                    raise UserError(_('You cannot unreserve a stock move that has been set to \'Done\'.'))
+            elif move.state == 'done':
+                raise UserError(_("You cannot unreserve a stock move that has been set to 'Done'."))
             moves_to_unreserve |= move
-        moves_to_unreserve.mapped('move_line_ids').unlink()
+
+        ml_to_update = self.env['stock.move.line']
+        ml_to_unlink = self.env['stock.move.line']
+        for ml in moves_to_unreserve.move_line_ids:
+            if ml.qty_done:
+                ml_to_update |= ml
+            else:
+                ml_to_unlink |= ml
+                moves_not_to_recompute |= ml.move_id
+
+        ml_to_update.write({'product_uom_qty': 0})
+        ml_to_unlink.unlink()
+        # `write` on `stock.move.line` doesn't call `_recompute_state` (unlike to `unlink`),
+        # so it must be called for each move where no move line has been deleted.
+        (moves_to_unreserve - moves_not_to_recompute)._recompute_state()
         return True
 
     def _generate_serial_numbers(self, next_serial_count=False):

addons/stock/models/stock_move_line.py

@@ -296,8 +296,8 @@ def write(self, vals):
                         ml.with_context(bypass_reservation_update=True).product_uom_qty = new_product_uom_qty
 
         # 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.
-        next_moves = self.env['stock.move']
         if updates or 'qty_done' in vals:
+            next_moves = self.env['stock.move']
             mls = self.filtered(lambda ml: ml.move_id.state == 'done' and ml.product_id.type == 'product')
             if not updates:  # we can skip those where qty_done is already good up to UoM rounding
                 mls = mls.filtered(lambda ml: not float_is_zero(ml.qty_done - vals['qty_done'], precision_rounding=ml.product_uom_id.rounding))
@@ -356,8 +356,8 @@ def write(self, vals):
             moves = self.filtered(lambda ml: ml.move_id.state == 'done' or ml.move_id.picking_id and ml.move_id.picking_id.immediate_transfer).mapped('move_id')
             for move in moves:
                 move.product_uom_qty = move.quantity_done
-        next_moves._do_unreserve()
-        next_moves._action_assign()
+            next_moves._do_unreserve()
+            next_moves._action_assign()
         return res
 
     def unlink(self):

addons/stock/tests/test_move.py

@@ -1543,6 +1543,62 @@ def test_unreserve_6(self):
         self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product, self.stock_location), 10.0)
         self.assertEqual(q2.reserved_quantity, 10)
 
+    def test_unreserve_7(self):
+        """ Check the unreservation of a stock move delete only stock move lines
+        without quantity done.
+        """
+        product = self.env['product.product'].create({
+            'name': 'product',
+            'tracking': 'serial',
+            'type': 'product',
+        })
+
+        serial_numbers = self.env['stock.production.lot'].create([{
+            'name': str(x),
+            'product_id': product.id,
+            'company_id': self.env.company.id,
+        } for x in range(5)])
+
+        for serial in serial_numbers:
+            self.env['stock.quant'].create({
+                'product_id': product.id,
+                'location_id': self.stock_location.id,
+                'quantity': 1.0,
+                'lot_id': serial.id,
+                'reserved_quantity': 0.0,
+            })
+
+        move1 = self.env['stock.move'].create({
+            'name': 'test_unreserve_7',
+            'location_id': self.stock_location.id,
+            'location_dest_id': self.customer_location.id,
+            'product_id': product.id,
+            'product_uom': product.uom_id.id,
+            'product_uom_qty': 5.0,
+        })
+        move1._action_confirm()
+        move1._action_assign()
+        self.assertEqual(move1.state, 'assigned')
+        self.assertEqual(len(move1.move_line_ids), 5)
+        self.assertEqual(self.env['stock.quant']._get_available_quantity(product, self.stock_location), 0.0)
+
+        # Check state is changed even with 0 move lines unlinked
+        move1.move_line_ids.write({'qty_done': 1})
+        move1._do_unreserve()
+        self.assertEqual(len(move1.move_line_ids), 5)
+        self.assertEqual(move1.state, 'confirmed')
+        move1._action_assign()
+        # set a quantity done on the two first move lines
+        move1.move_line_ids.write({'qty_done': 0})
+        move1.move_line_ids[0].qty_done = 1
+        move1.move_line_ids[1].qty_done = 1
+
+        move1._do_unreserve()
+        self.assertEqual(move1.state, 'confirmed')
+        self.assertEqual(len(move1.move_line_ids), 2)
+        self.assertEqual(move1.move_line_ids.mapped('qty_done'), [1, 1])
+        self.assertEqual(move1.move_line_ids.mapped('product_uom_qty'), [0, 0])
+
     def test_link_assign_1(self):
         """ Test the assignment mechanism when two chained stock moves try to move one unit of an
         untracked product.