[REF] stock: rounding issues

There's a certain configuration of UOM that is not correctly handled in
the v11 stock:
- create a move with an UOM having an higher factor than the product's
  UOM
- this higher UOM has a rounding of 1.0, meaning it doesn't allow
  partial reservation
- use the product's uom in this move and enter quantities that aren't
  expressible in the move's uom

A practical example is to set the rounding of dozen at 1.0, set the uom
of a product to units, create a move for a dozen and create a new move
line with 8 units.

The first issue is the `quantity_done` field, that uses an half-up
rounding on the `qty_done` field of each move line and sum them. When
applying this logic to the previous example, `quantity_done` field would
be 1.0, meaning we won't make a backorder for the missing 4 units. We
fix these issue by summing a non rounded qty_done and afterwards
rounding this value to the general products'UOM. This mean
`quantity_done` is technically not be expressible in the move's uom, but
keeping this value close to the reality will allow to handle the
backorders and extra moves correctly.

The second issue is that the backorder is always expressed in the move's
uom. In the previous example, this would mean creating a backorder of 0
dozen. In v10, an error was throwed, saying this operation is
impossible.

The `_prepare_move_split_vals` previously always received the quantity
of the backordered move in the uom of the original move. This is a
remnant of the v10 implementation where, if the quantity to backorder is
not expressible in the original move's UOM, an error was throwed. We now
pass the used uom via a context key to not update the method signature,
as it is probably legitimately overidden by custom modules. This way, in
the previous example, a backorder of 4 units is created.

The last issue is in `_action_assign`. Let's say you have 8 units in
stock and you want to reserve a dozen, the system will reserve 8 units
and then convert back 8 units in dozen, meaning a dozen was reserved.
This resulted in awkward situation where more items than available were
reserved, and the famous "cannot unreserve more than what's available in
stock" was throwed at almost each operation. To solve this, we convert
the not-yet reserved quantities back and forth in both uom. This
prevents partial reservation when not allowed, which is good. We had to
adapt some lines of `_action_assgn` that expected
`_update_reserved_quantity` always at least partially reserved the move.
It is now possible that it doesn't reserve anything.

We also normalize the quantity with a greater decimal precision when
deciding if we open the immediate transfer wizard or not (in the
original exemple, if you enter a single unit the immediate transfer
wizard would be openned because 0 was done in the UOM of the dozen). We
also prevent to enter `qty_done` on the move line if the value doesn't
honour the rounding of the UOM.

We introduce two tests, a simple  `_action_assign` in MTS and a more
complete MTO scenario with backorders, extra moves and invalid qty_done.

opw-1835186
opw-1835946

addons/stock/models/stock_move.py

@@ -247,10 +247,19 @@ class StockMove(models.Model):
 
     @api.depends('move_line_ids.qty_done', 'move_line_ids.product_uom_id')
     def _quantity_done_compute(self):
+        """ This field represents the sum of the move lines `qty_done`. It allows the user to know
+        if there is still work to do.
+
+        We take care of rounding this value at the general decimal precision and not the rounding
+        of the move's UOM to make sure this value is really close to the real sum, because this
+        field will be used in `_action_done` in order to know if the move will need a backorder or
+        an extra move.
+        """
         for move in self:
+            quantity_done = 0
             for move_line in move._get_move_lines():
-                # Transform the move_line quantity_done into the move uom.
-                move.quantity_done += move_line.product_uom_id._compute_quantity(move_line.qty_done, move.product_uom)
+                quantity_done += move_line.product_uom_id._compute_quantity(move_line.qty_done, move.product_uom, round=False)
+            move.quantity_done = quantity_done
 
     def _quantity_done_set(self):
         quantity_done = self[0].quantity_done  # any call to create will invalidate `move.quantity_done`
@@ -815,12 +824,22 @@ class StockMove(models.Model):
 
         taken_quantity = min(available_quantity, need)
 
+        # `taken_quantity` is in the quants unit of measure. There's a possibility that the move's
+        # unit of measure won't be respected if we blindly reserve this quantity, a common usecase
+        # 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')
+
         quants = []
         try:
-            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
-            )
+            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:
             # If it raises here, it means that the `available_quantity` brought by a done move line
             # is not available on the quants itself. This could be the result of an inventory
@@ -881,6 +900,8 @@ class StockMove(models.Model):
                         continue
                     need = move.product_qty - move.reserved_availability
                     taken_quantity = move._update_reserved_quantity(need, available_quantity, move.location_id, strict=False)
+                    if float_is_zero(taken_quantity, precision_rounding=move.product_id.uom_id.rounding):
+                        continue
                     if need == taken_quantity:
                         assigned_moves |= move
                     else:
@@ -937,6 +958,8 @@ class StockMove(models.Model):
                     for (location_id, lot_id, package_id, owner_id), quantity in available_move_lines.items():
                         need = move.product_qty - sum(move.move_line_ids.mapped('product_qty'))
                         taken_quantity = move._update_reserved_quantity(need, quantity, location_id, lot_id, package_id, owner_id)
+                        if float_is_zero(taken_quantity, precision_rounding=move.product_id.uom_id.rounding):
+                            continue
                         if need - taken_quantity == 0.0:
                             assigned_moves |= move
                             break
@@ -1040,8 +1063,10 @@ class StockMove(models.Model):
 
         # Split moves where necessary and move quants
         for move in moves_todo:
-            rounding = move.product_uom.rounding
-            if float_compare(move.quantity_done, move.product_uom_qty, precision_rounding=rounding) < 0:
+            # To know whether we need to create a backorder or not, round to the general product's
+            # decimal precision and not the product's UOM.
+            rounding = self.env['decimal.precision'].precision_get('Product Unit of Measure')
+            if float_compare(move.quantity_done, move.product_uom_qty, precision_digits=rounding) < 0:
                 # 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)
@@ -1085,15 +1110,17 @@ class StockMove(models.Model):
         self.mapped('move_line_ids').unlink()
         return super(StockMove, self).unlink()
 
-    def _prepare_move_split_vals(self, uom_qty):
+    def _prepare_move_split_vals(self, qty):
         vals = {
-            'product_uom_qty': uom_qty,
+            'product_uom_qty': qty,
             'procure_method': 'make_to_stock',
             'move_dest_ids': [(4, x.id) for x in self.move_dest_ids if x.state not in ('done', 'cancel')],
             'move_orig_ids': [(4, x.id) for x in self.move_orig_ids],
             'origin_returned_move_id': self.origin_returned_move_id.id,
             'price_unit': self.price_unit,
         }
+        if self.env.context.get('force_split_uom_id'):
+            vals['product_uom'] = self.env.context['force_split_uom_id']
         return vals
 
     def _split(self, qty, restrict_partner_id=False):
@@ -1112,9 +1139,18 @@ class StockMove(models.Model):
             raise UserError(_('You cannot split a draft move. It needs to be confirmed first.'))
         if float_is_zero(qty, precision_rounding=self.product_id.uom_id.rounding) or self.product_qty <= qty:
             return self.id
-        # HALF-UP rounding as only rounding errors will be because of propagation of error from default UoM
+
+        decimal_precision = self.env['decimal.precision'].precision_get('Product Unit of Measure')
+
+        # `qty` passed as argument is the quantity to backorder and is always expressed in the
+        # quants UOM. If we're able to convert back and forth this quantity in the move's and the
+        # quants UOM, the backordered move can keep the UOM of the move. Else, we'll create is in
+        # the UOM of the quants.
         uom_qty = self.product_id.uom_id._compute_quantity(qty, self.product_uom, rounding_method='HALF-UP')
-        defaults = self._prepare_move_split_vals(uom_qty)
+        if float_compare(qty, self.product_uom._compute_quantity(uom_qty, self.product_id.uom_id, rounding_method='HALF-UP'), precision_digits=decimal_precision) == 0:
+            defaults = self._prepare_move_split_vals(uom_qty)
+        else:
+            defaults = self.with_context(force_split_uom_id=self.product_id.uom_id.id)._prepare_move_split_vals(qty)
 
         if restrict_partner_id:
             defaults['restrict_partner_id'] = restrict_partner_id
@@ -1123,21 +1159,15 @@ class StockMove(models.Model):
         if self.env.context.get('source_location_id'):
             defaults['location_id'] = self.env.context['source_location_id']
         new_move = self.with_context(rounding_method='HALF-UP').copy(defaults)
-        # ctx = context.copy()
-        # TDE CLEANME: used only in write in this file, to clean
-        # ctx['do_not_propagate'] = True
 
         # FIXME: pim fix your crap
-        self.with_context(do_not_propagate=True, do_not_unreserve=True, rounding_method='HALF-UP').write({'product_uom_qty': self.product_uom_qty - uom_qty})
-
-        # if self.move_dest_id and self.propagate and self.move_dest_id.state not in ('done', 'cancel'):
-        #     new_move_prop = self.move_dest_id.split(qty)
-        #     new_move.write({'move_dest_id': new_move_prop})
-        # returning the first element of list returned by action_confirm is ok because we checked it wouldn't be exploded (and
-        # thus the result of action_confirm should always be a list of 1 element length)
-        # In this case we don't merge move since the new move with 0 quantity done will be used for the backorder.
+        # Update the original `product_qty` of the move. Use the general product's decimal
+        # precision and not the move's UOM to handle case where the `quantity_done` is not
+        # compatible with the move's UOM.
+        new_product_qty = self.product_id.uom_id._compute_quantity(self.product_qty - qty, self.product_uom, round=False)
+        new_product_qty = float_round(new_product_qty, precision_digits=self.env['decimal.precision'].precision_get('Product Unit of Measure'))
+        self.with_context(do_not_propagate=True, do_not_unreserve=True, rounding_method='HALF-UP').write({'product_uom_qty': new_product_qty})
         new_move = new_move._action_confirm(merge=False)
-        # TDE FIXME: due to action confirm change
         return new_move.id
 
     def _recompute_state(self):

addons/stock/models/stock_move_line.py

@@ -369,6 +369,15 @@ class StockMoveLine(models.Model):
         # `action_done` on the next move lines.
         ml_to_delete = self.env['stock.move.line']
         for ml in self:
+            # Check here if `ml.qty_done` respects the rounding of `ml.product_uom_id`.
+            uom_qty = float_round(ml.qty_done, precision_rounding=ml.product_uom_id.rounding, rounding_method='HALF-UP')
+            precision_digits = self.env['decimal.precision'].precision_get('Product Unit of Measure')
+            qty_done = float_round(ml.qty_done, precision_digits=precision_digits, rounding_method='HALF-UP')
+            if float_compare(uom_qty, qty_done, precision_digits=precision_digits) != 0:
+                raise UserError(_('The quantity done for the product "%s" doesn\'t respect the rounding precision \
+                                  defined on the unit of measure "%s". Please change the quantity done or the \
+                                  rounding precision of your unit of measure.') % (ml.product_id.display_name, ml.product_uom_id.name))
+
             qty_done_float_compared = float_compare(ml.qty_done, 0, precision_rounding=ml.product_uom_id.rounding)
             if qty_done_float_compared > 0:
                 if ml.product_id.tracking != 'none':

addons/stock/models/stock_picking.py

@@ -699,7 +699,8 @@ class Picking(models.Model):
 
         # If no lots when needed, raise error
         picking_type = self.picking_type_id
-        no_quantities_done = all(float_is_zero(move_line.qty_done, precision_rounding=move_line.product_uom_id.rounding) for move_line in self.move_line_ids)
+        precision_digits = self.env['decimal.precision'].precision_get('Product Unit of Measure')
+        no_quantities_done = all(float_is_zero(move_line.qty_done, precision_digits=precision_digits) for move_line in self.move_line_ids)
         no_reserved_quantities = all(float_is_zero(move_line.product_qty, precision_rounding=move_line.product_uom_id.rounding) for move_line in self.move_line_ids)
         if no_reserved_quantities and no_quantities_done:
             raise UserError(_('You cannot validate a transfer if you have not processed any quantity. You should rather cancel the transfer.'))

addons/stock/tests/test_move.py

@@ -827,6 +827,63 @@ class StockMove(TransactionCase):
 
         self.assertEqual(len(move.move_line_ids), 4.0)
 
+    def test_availability_6(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 don't make a partial reservation when the
+        quantity available is not enough to reserve the move. Check also that it is not possible
+        to set `quantity_done` with a value not honouring the UOM's rounding.
+        """
+        # on the dozen uom, set the rounding set 1.0
+        self.uom_dozen.rounding = 1
+
+        # 6 units are available in stock
+        self.env['stock.quant']._update_available_quantity(self.product1, self.stock_location, 6.0)
+
+        # the move should not be reserved
+        move = self.env['stock.move'].create({
+            'name': 'test_availability_6',
+            'location_id': self.stock_location.id,
+            'location_dest_id': self.customer_location.id,
+            'product_id': self.product1.id,
+            'product_uom': self.uom_dozen.id,
+            'product_uom_qty': 1,
+        })
+        move._action_confirm()
+        move._action_assign()
+        self.assertEqual(move.state, 'confirmed')
+
+        # the quants should be left untouched
+        self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product1, self.stock_location), 6.0)
+
+        # make 8 units available, the move should again not be reservabale
+        self.env['stock.quant']._update_available_quantity(self.product1, self.stock_location, 2.0)
+        move._action_assign()
+        self.assertEqual(move.state, 'confirmed')
+        self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product1, self.stock_location), 8.0)
+
+        # make 12 units available, this time the move should be reservable
+        self.env['stock.quant']._update_available_quantity(self.product1, self.stock_location, 4.0)
+        move._action_assign()
+        self.assertEqual(move.state, 'assigned')
+        self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product1, self.stock_location), 0.0)
+
+        # Check it isn't possible to set any value to quantity_done
+        with self.assertRaises(UserError):
+            move.quantity_done = 0.1
+            move._action_done()
+
+        with self.assertRaises(UserError):
+            move.quantity_done = 1.1
+            move._action_done()
+
+        with self.assertRaises(UserError):
+            move.quantity_done = 0.9
+            move._action_done()
+
+        move.quantity_done = 1
+        move._action_done()
+        self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product1, self.customer_location), 12.0)
+
     def test_unreserve_1(self):
         """ Check that unreserving a stock move sets the products reserved as available and
         set the state back to confirmed.
@@ -1356,6 +1413,108 @@ class StockMove(TransactionCase):
         self.assertEqual(move_stock_stock_1.state, 'assigned')
         self.assertEqual(move_stock_stock_2.state, 'waiting')
 
+    def test_link_assign_7(self):
+        # on the dozen uom, set the rounding set 1.0
+        self.uom_dozen.rounding = 1
+
+        # 6 units are available in stock
+        self.env['stock.quant']._update_available_quantity(self.product1, self.stock_location, 6.0)
+
+        # 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.product1.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.product1.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()
+
+        # the pick should not be reservable because of the rounding of the dozen
+        move_stock_pack._action_assign()
+        self.assertEqual(move_stock_pack.state, 'confirmed')
+        move_pack_cust._action_assign()
+        self.assertEqual(move_pack_cust.state, 'waiting')
+
+        # move the 6 units by adding an unreserved move line
+        move_stock_pack.write({'move_line_ids': [(0, 0, {
+            'product_id': self.product1.id,
+            'product_uom_id': self.uom_unit.id,
+            'qty_done': 6,
+            'product_uom_qty': 0,
+            'lot_id': False,
+            'package_id': False,
+            'result_package_id': False,
+            'location_id': move_stock_pack.location_id.id,
+            'location_dest_id': move_stock_pack.location_dest_id.id,
+            'picking_id': picking_stock_pack.id,
+        })]})
+
+        # the quantity done on the move should not respect the rounding of the move line
+        self.assertEqual(move_stock_pack.quantity_done, 0.5)
+
+        # create the backorder in the uom of the quants
+        backorder_wizard_dict = picking_stock_pack.button_validate()
+        backorder_wizard = self.env[backorder_wizard_dict['res_model']].browse(backorder_wizard_dict['res_id'])
+        backorder_wizard.process()
+        self.assertEqual(move_stock_pack.state, 'done')
+        self.assertEqual(move_stock_pack.quantity_done, 0.5)
+        self.assertEqual(move_stock_pack.product_uom_qty, 0.5)
+
+        # 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')
+
+        # 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)])
+        backorder.move_lines.write({'move_line_ids': [(0, 0, {
+            'product_id': self.product1.id,
+            'product_uom_id': self.uom_dozen.id,
+            'qty_done': 1,
+            'product_uom_qty': 0,
+            'lot_id': False,
+            'package_id': False,
+            'result_package_id': False,
+            'location_id': backorder.location_id.id,
+            'location_dest_id': backorder.location_dest_id.id,
+            'picking_id': backorder.id,
+        })]})
+        overprocessed_wizard_dict = backorder.button_validate()
+        overprocessed_wizard = self.env[overprocessed_wizard_dict['res_model']].browse(overprocessed_wizard_dict['res_id'])
+        overprocessed_wizard.action_confirm()
+        backorder_move = backorder.move_lines
+        self.assertEqual(backorder_move.state, 'done')
+        self.assertEqual(backorder_move.quantity_done, 12.0)
+        self.assertEqual(backorder_move.product_uom_qty, 12.0)
+        self.assertEqual(backorder_move.product_uom, self.uom_unit)
+
+        # the second move should now be reservable
+        move_pack_cust._action_assign()
+        self.assertEqual(move_pack_cust.state, 'assigned')
+
     def test_use_unreserved_move_line_1(self):
         """ Test that validating a stock move linked to an untracked product reserved by another one
         correctly unreserves the other one.