From dc56bb5071c83f72938de7b57ef21a9dfd73fb1f Mon Sep 17 00:00:00 2001 From: Simon Lejeune Date: Mon, 26 Mar 2018 14:20:49 +0200 Subject: [PATCH] [FIX] stock: stock.move.line: `_free_reservation` In a situation where `_action_done` is called on a batch of move line when one of them is forced and there isn't enough quantity available to reserve it, a call to `_free_reservation` is made in order to not have move lines reserved on quantity now unavailable. The issue is that the method looks for move line to unlink by making a search on move line having a quantity reserved. As the `_action_done` method defer the update of the reserved quantity at the end of the batch, `_free_reservation` could try to find and unreserve move lines that were juste processed, resulting in a traceback because there isn't enough stock to unreserved (because the move line was processed and thus moved at the next location). We fix this issue by explicitely passing the move lines to ignore. --- addons/stock/models/stock_move_line.py | 14 +++++++--- addons/stock/tests/test_move.py | 37 +++++++++++++++++++++++++- 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/addons/stock/models/stock_move_line.py b/addons/stock/models/stock_move_line.py index ba192f4b8a5..bb93b1c224b 100644 --- a/addons/stock/models/stock_move_line.py +++ b/addons/stock/models/stock_move_line.py @@ -403,6 +403,7 @@ class StockMoveLine(models.Model): ml_to_delete.unlink() # Now, we can actually move the quant. + done_ml = self.env['stock.move.line'] for ml in self - ml_to_delete: if ml.product_id.type == 'product': Quant = self.env['stock.quant'] @@ -411,7 +412,7 @@ class StockMoveLine(models.Model): # if this move line is force assigned, unreserve elsewhere if needed if not ml.location_id.should_bypass_reservation() and float_compare(ml.qty_done, ml.product_qty, precision_rounding=rounding) > 0: extra_qty = ml.qty_done - ml.product_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._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_to_ignore=done_ml) # unreserve what's been reserved if not ml.location_id.should_bypass_reservation() and ml.product_id.type == 'product' and ml.product_qty: try: @@ -430,6 +431,7 @@ class StockMoveLine(models.Model): Quant._update_available_quantity(ml.product_id, ml.location_id, -taken_from_untracked_qty, lot_id=False, package_id=ml.package_id, owner_id=ml.owner_id) Quant._update_available_quantity(ml.product_id, ml.location_id, taken_from_untracked_qty, lot_id=ml.lot_id, package_id=ml.package_id, owner_id=ml.owner_id) 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) + done_ml |= ml # Reset the reserved quantity as we just moved it to the destination location. (self - ml_to_delete).with_context(bypass_reservation_update=True).write({ 'product_uom_qty': 0.00, @@ -452,13 +454,19 @@ class StockMoveLine(models.Model): data['owner_name'] = self.env['res.partner'].browse(vals.get('owner_id')).name record.message_post_with_view(template, values={'move': move, 'vals': dict(vals, **data)}, subtype_id=self.env.ref('mail.mt_note').id) - def _free_reservation(self, product_id, location_id, quantity, lot_id=None, package_id=None, owner_id=None): + def _free_reservation(self, product_id, location_id, quantity, lot_id=None, package_id=None, owner_id=None, ml_to_ignore=None): """ When editing a done move line or validating one with some forced quantities, it is possible to impact quants that were not reserved. It is therefore necessary to edit or unlink the move lines that reserved a quantity now unavailable. + + :param ml_to_ignore: recordset of `stock.move.line` that should NOT be unreserved """ self.ensure_one() + if ml_to_ignore is None: + ml_to_ignore = self.env['stock.move.line'] + ml_to_ignore |= self + # Check the available quantity, with the `strict` kw set to `True`. If the available # quantity is greather than the quantity now unavailable, there is nothing to do. available_quantity = self.env['stock.quant']._get_available_quantity( @@ -475,7 +483,7 @@ class StockMoveLine(models.Model): ('owner_id', '=', owner_id.id if owner_id else False), ('package_id', '=', package_id.id if package_id else False), ('product_qty', '>', 0.0), - ('id', '!=', self.id), + ('id', 'not in', ml_to_ignore.ids), ] oudated_candidates = self.env['stock.move.line'].search(oudated_move_lines_domain) diff --git a/addons/stock/tests/test_move.py b/addons/stock/tests/test_move.py index 609bf0aa334..032260d44b0 100644 --- a/addons/stock/tests/test_move.py +++ b/addons/stock/tests/test_move.py @@ -478,7 +478,7 @@ class StockMove(TransactionCase): 'product_id': self.product2.id, }) - move1.with_context(debug=True).move_line_ids[1].lot_id = lot3 + move1.move_line_ids[1].lot_id = lot3 self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product2, self.stock_location), 1.0) self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product2, self.stock_location, lot_id=lot1, strict=True), 0.0) @@ -1473,6 +1473,41 @@ class StockMove(TransactionCase): self.assertEqual(move1.state, 'confirmed') self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product1, self.stock_location, lot_id=lot1), 0.0) + def test_use_unreserved_move_line_3(self): + """ Test the behavior of `_free_reservation` when ran on a recordset of move lines where + some are assigned and some are force assigned. `_free_reservation` should not use an + already processed move line when looking for a move line candidate to unreserve. + """ + self.env['stock.quant']._update_available_quantity(self.product1, self.stock_location, 1.0) + + move1 = self.env['stock.move'].create({ + 'name': 'test_use_unreserved_move_line_3', + 'location_id': self.stock_location.id, + 'location_dest_id': self.customer_location.id, + 'product_id': self.product1.id, + 'product_uom': self.uom_unit.id, + 'product_uom_qty': 3.0, + }) + move1._action_confirm() + move1._action_assign() + move1.quantity_done = 1 + + # add a forced move line in `move1` + move1.write({'move_line_ids': [(0, 0, { + 'product_id': self.product1.id, + 'product_uom_id': self.uom_unit.id, + 'qty_done': 2, + 'product_uom_qty': 0, + 'lot_id': False, + 'package_id': False, + 'result_package_id': False, + 'location_id': move1.location_id.id, + 'location_dest_id': move1.location_dest_id.id, + })]}) + move1._action_done() + + self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product1, self.customer_location), 3.0) + def test_edit_reserved_move_line_1(self): """ Test that editing a stock move line linked to an untracked product correctly and directly adapts the reservation. In this case, we edit the sublocation where we take the