From 4322985a8aa7c5d007ebef4d227b5ef4c4d971fe Mon Sep 17 00:00:00 2001 From: Simon Lejeune Date: Wed, 25 Apr 2018 14:13:56 +0200 Subject: [PATCH] [FIX] stock: rounding issues Followup of rev 383947646f5652804bd6bbc764380a890e6f4a8d Let's say you have 6 units and you try to reserve a dozen while the rounding of the dozen is 1.0. The previous commit correctly only allows to reserve 12 units, and handle correctly forced quantities in other compatible UOM, all that with extra moves/backoders. But we missed an important issue: if now you have 12 units, you will reserve them. If these 12 units are split across quants, i.e. some are in another sublocations or are assigned a special lot_id, then we'll again have rounding issue because the reservation was always expressed in the move's uom while sometimes it is not possible to go back and forth between the move uom and and the product's uom. In other word, we'll reserve 12 times 0 dozen and get stuck at the next step. This patch now only reserve a quantity if the quantity is expressible in the rounding's UOM. If this quantity is split across multiple move lines, we'll make the move line reserved in the UOM of the quants if necessary. We also do not apply the rounding down -> halfup dance if `_update_reserved_quantity` is called with the force argument set as True (meaning, in an mto scenario). The change in `_prepare_move_line_vals` will make the move lines in the uom of the qants and everything should be good. With the last change, we need to adapt the test added by the previous patch, else we won't be able to make a chain of 2 move moving producst tracked by serial number if the rounding of the dozen is set at 1.0. opw-1835186 opw-1835946 --- addons/stock/models/stock_move.py | 19 +++-- addons/stock/tests/test_move.py | 111 +++++++++++++++++++++++++++++- 2 files changed, 124 insertions(+), 6 deletions(-) diff --git a/addons/stock/models/stock_move.py b/addons/stock/models/stock_move.py index d729b2f7a0f..9914f790bbf 100644 --- a/addons/stock/models/stock_move.py +++ b/addons/stock/models/stock_move.py @@ -809,7 +809,12 @@ class StockMove(models.Model): } if quantity: uom_quantity = self.product_id.uom_id._compute_quantity(quantity, self.product_uom, rounding_method='HALF-UP') - vals = dict(vals, product_uom_qty=uom_quantity) + uom_quantity_back_to_product_uom = self.product_uom._compute_quantity(uom_quantity, self.product_id.uom_id, rounding_method='HALF-UP') + rounding = self.env['decimal.precision'].precision_get('Product Unit of Measure') + if float_compare(quantity, uom_quantity_back_to_product_uom, precision_digits=rounding) == 0: + vals = dict(vals, product_uom_qty=uom_quantity) + else: + vals = dict(vals, product_uom_qty=quantity, product_uom_id=self.product_id.uom_id.id) if reserved_quant: vals = dict( vals, @@ -839,9 +844,12 @@ class StockMove(models.Model): # is if the move's unit of measure's rounding does not allow fractional reservation. We chose # to convert `taken_quantity` to the move's unit of measure with a down rounding method and # then get it back in the quants unit of measure with an half-up rounding_method. This - # way, we'll never reserve more than allowed. - taken_quantity_move_uom = self.product_id.uom_id._compute_quantity(taken_quantity, self.product_uom, rounding_method='DOWN') - taken_quantity = self.product_uom._compute_quantity(taken_quantity_move_uom, self.product_id.uom_id, rounding_method='HALF-UP') + # way, we'll never reserve more than allowed. We do not apply this logic if + # `available_quantity` is brought by a chained move line. In this case, `_prepare_move_line_vals` + # will take care of changing the UOM to the UOM of the product. + if not strict: + taken_quantity_move_uom = self.product_id.uom_id._compute_quantity(taken_quantity, self.product_uom, rounding_method='DOWN') + taken_quantity = self.product_uom._compute_quantity(taken_quantity_move_uom, self.product_id.uom_id, rounding_method='HALF-UP') quants = [] try: @@ -859,11 +867,12 @@ class StockMove(models.Model): taken_quantity = 0 # Find a candidate move line to update or create a new one. + for reserved_quant, quantity in quants: to_update = self.move_line_ids.filtered(lambda m: m.product_id.tracking != 'serial' and m.location_id.id == reserved_quant.location_id.id and m.lot_id.id == reserved_quant.lot_id.id and m.package_id.id == reserved_quant.package_id.id and m.owner_id.id == reserved_quant.owner_id.id) if to_update: - to_update[0].with_context(bypass_reservation_update=True).product_uom_qty += self.product_id.uom_id._compute_quantity(quantity, self.product_uom, rounding_method='HALF-UP') + to_update[0].with_context(bypass_reservation_update=True).product_uom_qty += self.product_id.uom_id._compute_quantity(quantity, to_update.product_uom_id, rounding_method='HALF-UP') else: if self.product_id.tracking == 'serial': for i in range(0, int(quantity)): diff --git a/addons/stock/tests/test_move.py b/addons/stock/tests/test_move.py index c7f24401108..269a4913d72 100644 --- a/addons/stock/tests/test_move.py +++ b/addons/stock/tests/test_move.py @@ -884,6 +884,47 @@ class StockMove(TransactionCase): move._action_done() self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product1, self.customer_location), 12.0) + def test_availability_7(self): + """ Check that, in the scenario where a move is in a bigger uom than the uom of the quants + and this uom only allows entire numbers, we only reserve quantity honouring the uom's + rounding even if the quantity is set across multiple quants. + """ + # on the dozen uom, set the rounding set 1.0 + self.uom_dozen.rounding = 1 + + # make 12 quants of 1 + for i in range(1, 13): + lot_id = self.env['stock.production.lot'].create({ + 'name': 'lot%s' % str(i), + 'product_id': self.product2.id, + }) + self.env['stock.quant']._update_available_quantity(self.product2, self.stock_location, 1.0, lot_id=lot_id) + + # the move should be reserved + move = self.env['stock.move'].create({ + 'name': 'test_availability_7', + 'location_id': self.stock_location.id, + 'location_dest_id': self.customer_location.id, + 'product_id': self.product2.id, + 'product_uom': self.uom_dozen.id, + 'product_uom_qty': 1, + }) + move._action_confirm() + move._action_assign() + self.assertEqual(move.state, 'assigned') + self.assertEqual(len(move.move_line_ids.mapped('product_uom_id')), 1) + self.assertEqual(move.move_line_ids.mapped('product_uom_id'), self.uom_unit) + + for move_line in move.move_line_ids: + move_line.qty_done = 1 + move._action_done() + + self.assertEqual(move.product_uom_qty, 1) + self.assertEqual(move.product_uom.id, self.uom_dozen.id) + self.assertEqual(move.state, 'done') + self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product2, self.customer_location), 12.0) + self.assertEqual(len(self.env['stock.quant']._gather(self.product2, self.customer_location)), 12) + def test_unreserve_1(self): """ Check that unreserving a stock move sets the products reserved as available and set the state back to confirmed. @@ -1486,7 +1527,10 @@ class StockMove(TransactionCase): # the second move should not be reservable because of the rounding on the dozen move_pack_cust._action_assign() - self.assertEqual(move_pack_cust.state, 'waiting') + self.assertEqual(move_pack_cust.state, 'partially_available') + move_line_pack_cust = move_pack_cust.move_line_ids + self.assertEqual(move_line_pack_cust.product_uom_qty, 6) + self.assertEqual(move_line_pack_cust.product_uom_id.id, self.uom_unit.id) # move a dozen on the backorder to see how we handle the extra move backorder = self.env['stock.picking'].search([('backorder_id', '=', picking_stock_pack.id)]) @@ -1514,6 +1558,71 @@ class StockMove(TransactionCase): # the second move should now be reservable move_pack_cust._action_assign() self.assertEqual(move_pack_cust.state, 'assigned') + self.assertEqual(move_line_pack_cust.product_uom_qty, 12) + self.assertEqual(move_line_pack_cust.product_uom_id.id, self.uom_unit.id) + self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product1, move_stock_pack.location_dest_id), 6) + + def test_link_assign_8(self): + """ Set the rounding of the dozen to 1.0, create a chain of two move for a dozen, the product + concerned is tracked by serial number. Check that the flow is ok. + """ + # on the dozen uom, set the rounding set 1.0 + self.uom_dozen.rounding = 1 + + # 6 units are available in stock + for i in range(1, 13): + lot_id = self.env['stock.production.lot'].create({ + 'name': 'lot%s' % str(i), + 'product_id': self.product2.id, + }) + self.env['stock.quant']._update_available_quantity(self.product2, self.stock_location, 1.0, lot_id=lot_id) + + # create pickings and moves for a pick -> pack mto scenario + picking_stock_pack = self.env['stock.picking'].create({ + 'location_id': self.stock_location.id, + 'location_dest_id': self.pack_location.id, + 'picking_type_id': self.env.ref('stock.picking_type_internal').id, + }) + move_stock_pack = self.env['stock.move'].create({ + 'name': 'test_link_assign_7', + 'location_id': self.stock_location.id, + 'location_dest_id': self.pack_location.id, + 'product_id': self.product2.id, + 'product_uom': self.uom_dozen.id, + 'product_uom_qty': 1.0, + 'picking_id': picking_stock_pack.id, + }) + picking_pack_cust = self.env['stock.picking'].create({ + 'location_id': self.pack_location.id, + 'location_dest_id': self.customer_location.id, + 'picking_type_id': self.env.ref('stock.picking_type_out').id, + }) + move_pack_cust = self.env['stock.move'].create({ + 'name': 'test_link_assign_7', + 'location_id': self.pack_location.id, + 'location_dest_id': self.customer_location.id, + 'product_id': self.product2.id, + 'product_uom': self.uom_dozen.id, + 'product_uom_qty': 1.0, + 'picking_id': picking_pack_cust.id, + }) + move_stock_pack.write({'move_dest_ids': [(4, move_pack_cust.id, 0)]}) + move_pack_cust.write({'move_orig_ids': [(4, move_stock_pack.id, 0)]}) + (move_stock_pack + move_pack_cust)._action_confirm() + + move_stock_pack._action_assign() + self.assertEqual(move_stock_pack.state, 'assigned') + move_pack_cust._action_assign() + self.assertEqual(move_pack_cust.state, 'waiting') + + for ml in move_stock_pack.move_line_ids: + ml.qty_done = 1 + picking_stock_pack.button_validate() + self.assertEqual(move_pack_cust.state, 'assigned') + for ml in move_pack_cust.move_line_ids: + self.assertEqual(ml.product_uom_qty, 1) + self.assertEqual(ml.product_uom_id.id, self.uom_unit.id) + self.assertTrue(bool(ml.lot_id.id)) def test_use_unreserved_move_line_1(self): """ Test that validating a stock move linked to an untracked product reserved by another one