[FIX] product, decimal_precision, stock, mrp: Prevent quant reservation issue

One can set a Unit of Measure precision rounding more
precise than the overall Decimal Accuracy. This can trigger the error
'It is not possible to unreserve more products', as there will be
inconsistencies on reserved quantities due to decimal roundings.
We add 2 warnings on Units of Measure and Decimal Accuracy.
Therefore, users are warned when their UOM/decimal changes can
cause inconsistencies in quants reservation.

Also, we want to prevent the issue at the root by ensuring that both the move
and the quant have the same reserved quantity.
The contrary can happen e.g. when a product is defined in Liters (rounding .001)
but used in an operation in ml (rounding .01, so e.g. 187.5ml).
The conversions between the 2 UOM can cause the differences of reservations.

In _prepare_move_line_vals we make sure uom_quantity_back_to_product_uom
is not more precise than the overall Decimal Accuracy. That way, if
the reserved quantity changes between UOM conversions, the move line created
is in the quant/product UOM, and we do not have move line's reserved quantity >
quantity in stock.
We do the same for _update_reserved_quantity (stock.move), when updating
a move line.

In _update_move_lines, because of UOM conversions, new_quantity_done
can differ from ml.product_uom_qty, which triggers the creation of an extra
move line, leading to reserved quantity > quantity in stock. To prevent
that, we compare the reserved quantities in the product UOM.

opw 2206902 (Example 1)
opw 2171541
opw 2221227
opw 2233937
opw 2198775
and many more

closes odoo/odoo#61758

X-original-commit: 91318520314632655127142de674156802214a21
Signed-off-by: Arnold Moyaux <amoyaux@users.noreply.github.com>

7 files changed, 368 insertions, 3 deletions

diff --git a/addons/mrp/tests/test_order.py b/addons/mrp/tests/test_order.py
index 90f2945d6e6..65a4da48486 100644
--- a/addons/mrp/tests/test_order.py
+++ b/addons/mrp/tests/test_order.py
@@ -1403,3 +1403,107 @@ class TestMrpOrder(TestMrpCommon):
         mo_copy = mo_form.save()
         mo_copy.button_mark_done()
         self.assertEqual(mo_copy.state, 'done')
+
+    def test_product_produce_different_uom(self):
+        """ Check that for products tracked by lots,
+        with component product UOM different from UOM used in the BOM,
+        we do not create a new move line due to extra reserved quantity
+        caused by decimal rounding conversions.
+        """
+
+        # the overall decimal accuracy is set to 3 digits
+        precision = self.env.ref('product.decimal_product_uom')
+        precision.digits = 3
+
+        # define L and ml, L has rounding .001 but ml has rounding .01
+        # when producing e.g. 187.5ml, it will be rounded to .188L
+        categ_test = self.env['uom.category'].create({'name': 'Volume Test'})
+
+        uom_L = self.env['uom.uom'].create({
+            'name': 'Test Liters',
+            'category_id': categ_test.id,
+            'uom_type': 'reference',
+            'rounding': 0.001
+        })
+
+        uom_ml = self.env['uom.uom'].create({
+            'name': 'Test ml',
+            'category_id': categ_test.id,
+            'uom_type': 'smaller',
+            'rounding': 0.01,
+            'factor_inv': 0.001,
+        })
+
+        # create a product component and the final product using the component
+        product_comp = self.env['product.product'].create({
+            'name': 'Product Component',
+            'type': 'product',
+            'tracking': 'lot',
+            'categ_id': self.env.ref('product.product_category_all').id,
+            'uom_id': uom_L.id,
+            'uom_po_id': uom_L.id,
+        })
+
+        product_final = self.env['product.product'].create({
+            'name': 'Product Final',
+            'type': 'product',
+            'tracking': 'lot',
+            'categ_id': self.env.ref('product.product_category_all').id,
+            'uom_id': uom_L.id,
+            'uom_po_id': uom_L.id,
+        })
+
+        # the products are tracked by lot, so we go through _generate_consumed_move_line
+        lot_final = self.env['stock.production.lot'].create({
+            'name': 'Lot Final',
+            'product_id': product_final.id,
+            'company_id': self.env.company.id,
+        })
+
+        lot_comp = self.env['stock.production.lot'].create({
+            'name': 'Lot Component',
+            'product_id': product_comp.id,
+            'company_id': self.env.company.id,
+        })
+
+        # update the quantity on hand for Component, in a lot
+        self.stock_location = self.env.ref('stock.stock_location_stock')
+        self.env['stock.quant']._update_available_quantity(product_comp, self.stock_location, 1, lot_id=lot_comp)
+
+        # create a BOM for Final, using Component
+        test_bom = self.env['mrp.bom'].create({
+            'product_id': product_final.id,
+            'product_tmpl_id': product_final.product_tmpl_id.id,
+            'product_uom_id': uom_L.id,
+            'product_qty': 1.0,
+            'type': 'normal',
+            'bom_line_ids': [(0, 0, {
+                'product_id': product_comp.id,
+                'product_qty': 375.00,
+                'product_uom_id': uom_ml.id
+            })],
+        })
+
+        # create a MO for this BOM
+        mo_product_final_form = Form(self.env['mrp.production'])
+        mo_product_final_form.product_id = product_final
+        mo_product_final_form.product_uom_id = uom_L
+        mo_product_final_form.bom_id = test_bom
+        mo_product_final_form.product_qty = 0.5
+        mo_product_final_form = mo_product_final_form.save()
+
+        mo_product_final_form.action_confirm()
+        mo_product_final_form.action_assign()
+        self.assertEqual(mo_product_final_form.reservation_state, 'assigned')
+
+        # produce
+        res_dict = mo_product_final_form.button_mark_done()
+        self.assertEqual(res_dict.get('res_model'), 'mrp.immediate.production')
+        wizard = Form(self.env[res_dict['res_model']].with_context(res_dict['context'])).save()
+        wizard.process()
+
+        # check that in _generate_consumed_move_line,
+        # we do not create an extra move line because
+        # of a conversion 187.5ml = 0.188L
+        # thus creating an extra line with 'product_uom_qty': 0.5
+        self.assertEqual(len(mo_product_final_form.move_raw_ids.move_line_ids), 1, 'One move line should exist for the MO.')
diff --git a/addons/product/i18n/product.pot b/addons/product/i18n/product.pot
index 574530cf552..16fcc6f186a 100644
--- a/addons/product/i18n/product.pot
+++ b/addons/product/i18n/product.pot
@@ -2341,6 +2341,15 @@ msgid ""
 msgstr ""
 
 #. module: product
+#: code:addons/product/models/uom_uom.py:0
+#, python-format
+msgid ""
+"This rounding precision is higher than the Decimal Accuracy (%s digits).\n"
+"This may cause inconsistencies in computations.\n"
+"Please set a precision between %s and 1."
+msgstr ""
+
+#. module: product
 #: model:ir.model.fields,help:product.field_product_supplierinfo__product_code
 msgid ""
 "This vendor's product code will be used when printing a request for "
@@ -2662,6 +2671,13 @@ msgid "Volume unit of measure label"
 msgstr ""
 
 #. module: product
+#: code:addons/product/models/decimal_precision.py:0
+#: code:addons/product/models/uom_uom.py:0
+#, python-format
+msgid "Warning!"
+msgstr ""
+
+#. module: product
 #: model_terms:ir.ui.view,arch_db:product.product_template_search_view
 msgid "Warnings"
 msgstr ""
@@ -2695,6 +2711,15 @@ msgid "White"
 msgstr ""
 
 #. module: product
+#: code:addons/product/models/decimal_precision.py:34
+#, python-format
+msgid "You are setting a Decimal Accuracy less precise than the UOMs:\n"
+"%s\n"
+"This may cause inconsistencies in computations.\n"
+"Please increase the rounding of those units of measure, or the digits of this Decimal Accuracy."
+msgstr ""
+
+#. module: product
 #: model_terms:ir.actions.act_window,help:product.product_pricelist_action2
 msgid ""
 "You can assign pricelists to your customers or select one when creating a "
diff --git a/addons/product/models/__init__.py b/addons/product/models/__init__.py
index 57917450de3..6e6333f642d 100644
--- a/addons/product/models/__init__.py
+++ b/addons/product/models/__init__.py
@@ -5,6 +5,7 @@
 
 from . import res_config_settings
 from . import decimal_precision
+from . import uom_uom
 
 # don't try to be a good boy and sort imports alphabetically.
 # `product.template` should be initialised before `product.product`
diff --git a/addons/product/models/decimal_precision.py b/addons/product/models/decimal_precision.py
index e8a46a90e2f..0539fbb32dc 100644
--- a/addons/product/models/decimal_precision.py
+++ b/addons/product/models/decimal_precision.py
@@ -15,3 +15,26 @@ class DecimalPrecision(models.Model):
                 for precision in self):
             raise ValidationError(_("You cannot define the decimal precision of 'Account' as greater than the rounding factor of the company's main currency"))
         return True
+
+    @api.onchange('digits')
+    def _onchange_digits(self):
+        if self.name != "Product Unit of Measure":  # precision_get() relies on this name
+            return
+        # We are changing the precision of UOM fields; check whether the
+        # precision is equal or higher than existing units of measure.
+        rounding = 1.0 / 10.0**self.digits
+        dangerous_uom = self.env['uom.uom'].search([('rounding', '<', rounding)])
+        if dangerous_uom:
+            uom_descriptions = [
+                " - %s (id=%s, precision=%s)" % (uom.name, uom.id, uom.rounding)
+                for uom in dangerous_uom
+            ]
+            return {'warning': {
+                'title': _('Warning!'),
+                'message': _(
+                    "You are setting a Decimal Accuracy less precise than the UOMs:\n"
+                    "%s\n"
+                    "This may cause inconsistencies in computations.\n"
+                    "Please increase the rounding of those units of measure, or the digits of this Decimal Accuracy."
+                 ) % ('\n'.join(uom_descriptions)),
+            }}
diff --git a/addons/product/models/uom_uom.py b/addons/product/models/uom_uom.py
new file mode 100644
index 00000000000..cae84ba1ba0
--- /dev/null
+++ b/addons/product/models/uom_uom.py
@@ -0,0 +1,21 @@
+# -*- coding: utf-8 -*-
+# Part of Odoo. See LICENSE file for full copyright and licensing details.
+
+from odoo import api, models, _
+
+
+class UoM(models.Model):
+    _inherit = 'uom.uom'
+
+    @api.onchange('rounding')
+    def _onchange_rounding(self):
+        precision = self.env['decimal.precision'].precision_get('Product Unit of Measure')
+        if self.rounding < 1.0 / 10.0**precision:
+            return {'warning': {
+                'title': _('Warning!'),
+                'message': _(
+                    "This rounding precision is higher than the Decimal Accuracy"
+                    " (%s digits).\nThis may cause inconsistencies in computations.\n"
+                    "Please set a precision between %s and 1."
+                ) % (precision, 1.0 / 10.0**precision),
+            }}
diff --git a/addons/stock/models/stock_move.py b/addons/stock/models/stock_move.py
index 2e35f883e9b..a60eb156dc0 100644
--- a/addons/stock/models/stock_move.py
+++ b/addons/stock/models/stock_move.py
@@ -1164,9 +1164,10 @@ class StockMove(models.Model):
             'company_id': self.company_id.id,
         }
         if quantity:
+            rounding = self.env['decimal.precision'].precision_get('Product Unit of Measure')
             uom_quantity = self.product_id.uom_id._compute_quantity(quantity, self.product_uom, rounding_method='HALF-UP')
+            uom_quantity = float_round(uom_quantity, precision_digits=rounding)
             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:
@@ -1208,9 +1209,9 @@ class StockMove(models.Model):
             taken_quantity = self.product_uom._compute_quantity(taken_quantity_move_uom, self.product_id.uom_id, rounding_method='HALF-UP')
 
         quants = []
+        rounding = self.env['decimal.precision'].precision_get('Product Unit of Measure')
 
         if self.product_id.tracking == 'serial':
-            rounding = self.env['decimal.precision'].precision_get('Product Unit of Measure')
             if float_compare(taken_quantity, int(taken_quantity), precision_digits=rounding) != 0:
                 taken_quantity = 0
 
@@ -1228,7 +1229,11 @@ class StockMove(models.Model):
         for reserved_quant, quantity in quants:
             to_update = self.move_line_ids.filtered(lambda ml: ml._reservation_is_updatable(quantity, reserved_quant))
             if to_update:
-                to_update[0].with_context(bypass_reservation_update=True).product_uom_qty += self.product_id.uom_id._compute_quantity(quantity, to_update[0].product_uom_id, rounding_method='HALF-UP')
+                uom_quantity = self.product_id.uom_id._compute_quantity(quantity, to_update[0].product_uom_id, rounding_method='HALF-UP')
+                uom_quantity = float_round(uom_quantity, precision_digits=rounding)
+                uom_quantity_back_to_product_uom = to_update[0].product_uom_id._compute_quantity(uom_quantity, self.product_id.uom_id, rounding_method='HALF-UP')
+            if to_update and float_compare(quantity, uom_quantity_back_to_product_uom, precision_digits=rounding) == 0:
+                to_update[0].with_context(bypass_reservation_update=True).product_uom_qty += uom_quantity
             else:
                 if self.product_id.tracking == 'serial':
                     for i in range(0, int(quantity)):
diff --git a/addons/stock/tests/test_move2.py b/addons/stock/tests/test_move2.py
index e8a4f5337a0..4328b8b9593 100644
--- a/addons/stock/tests/test_move2.py
+++ b/addons/stock/tests/test_move2.py
@@ -6,6 +6,9 @@ from datetime import timedelta
 from odoo.addons.stock.tests.common import TestStockCommon
 from odoo.exceptions import UserError
 
+from odoo.tests import Form
+from odoo.tools import float_is_zero, float_compare
+
 from odoo.tests.common import Form
 
 class TestPickShip(TestStockCommon):
@@ -2095,6 +2098,189 @@ class TestStockUOM(TestStockCommon):
         self.assertEqual(len(back_order_in), 1.00, 'There should be one back order created')
         self.assertEqual(back_order_in.move_lines.product_qty, 91640.00, 'There should be one back order created')
 
+    def test_move_product_with_different_uom(self):
+        """ Product defined in g with 0.01 rounding
+        Decimal Accuracy (DA) 3 digits.
+        Quantity on hand: 149.88g
+        Picking of 1kg
+        kg has 0.0001 rounding
+        Due to conversions, we may end up reserving 150g
+        (more than the quantity in stock), we check that
+        we reserve less than the quantity in stock
+        """
+        precision = self.env.ref('product.decimal_product_uom')
+        precision.digits = 3
+        precision_digits = precision.digits
+
+        self.uom_kg.rounding = 0.0001
+        self.uom_gm.rounding = 0.01
+
+        product_G = self.env['product.product'].create({
+            'name': 'Product G',
+            'type': 'product',
+            'categ_id': self.env.ref('product.product_category_all').id,
+            'uom_id': self.uom_gm.id,
+            'uom_po_id': self.uom_gm.id,
+        })
+
+        stock_location = self.env['stock.location'].browse(self.stock_location)
+        self.env['stock.quant']._update_available_quantity(product_G, stock_location, 149.88)
+        self.assertEqual(len(product_G.stock_quant_ids), 1, 'One quant should exist for the product.')
+        quant = product_G.stock_quant_ids
+
+        # transfer 1kg of product_G
+        picking = self.env['stock.picking'].create({
+            'location_id': self.stock_location,
+            'location_dest_id': self.customer_location,
+            'picking_type_id': self.env.ref('stock.picking_type_out').id,
+        })
+
+        move = self.env['stock.move'].create({
+            'name': 'test_reserve_product_G',
+            'location_id': self.stock_location,
+            'location_dest_id': self.customer_location,
+            'picking_id': picking.id,
+            'product_id': product_G.id,
+            'product_uom': self.uom_kg.id,
+            'product_uom_qty': 1,
+        })
+
+        self.assertEqual(move.product_uom.id, self.uom_kg.id)
+        self.assertEqual(move.product_uom_qty, 1.0)
+
+        picking.action_confirm()
+        picking.action_assign()
+
+        self.assertEqual(product_G.uom_id.rounding, 0.01)
+        self.assertEqual(move.product_uom.rounding, 0.0001)
+
+        self.assertEqual(len(picking.move_line_ids), 1, 'One move line should exist for the picking.')
+        move_line = picking.move_line_ids
+        # check that we do not reserve more (in the same UOM) than the quantity in stock
+        self.assertEqual(float_compare(move_line.product_qty, quant.quantity, precision_digits=precision_digits), -1, "We do not reserve more (in the same UOM) than the quantity in stock")
+        # check that we reserve the same quantity in the ml and the quant
+        self.assertTrue(float_is_zero(move_line.product_qty - quant.reserved_quantity, precision_digits=precision_digits))
+
+    def test_update_product_move_line_with_different_uom(self):
+        """ Check that when the move line and corresponding
+        product have different UOM with possibly conflicting
+        precisions, we do not reserve more than the quantity
+        in stock. Similar initial configuration as
+        test_move_product_with_different_uom.
+        """
+        precision = self.env.ref('product.decimal_product_uom')
+        precision.digits = 3
+        precision_digits = precision.digits
+
+        self.uom_kg.rounding = 0.0001
+        self.uom_gm.rounding = 0.01
+
+        product_LtDA = self.env['product.product'].create({
+            'name': 'Product Less than DA',
+            'type': 'product',
+            'categ_id': self.env.ref('product.product_category_all').id,
+            'uom_id': self.uom_gm.id,
+            'uom_po_id': self.uom_gm.id,
+        })
+
+        product_GtDA = self.env['product.product'].create({
+            'name': 'Product Greater than DA',
+            'type': 'product',
+            'categ_id': self.env.ref('product.product_category_all').id,
+            'uom_id': self.uom_gm.id,
+            'uom_po_id': self.uom_gm.id,
+        })
+
+        stock_location = self.env['stock.location'].browse(self.stock_location)
+
+        # quantity in hand converted to kg is not more precise than the DA
+        self.env['stock.quant']._update_available_quantity(product_LtDA, stock_location, 149)
+        # quantity in hand converted to kg is more precise than the DA
+        self.env['stock.quant']._update_available_quantity(product_GtDA, stock_location, 149.88)
+
+        self.assertEqual(len(product_LtDA.stock_quant_ids), 1, 'One quant should exist for the product.')
+        self.assertEqual(len(product_GtDA.stock_quant_ids), 1, 'One quant should exist for the product.')
+        quant_LtDA = product_LtDA.stock_quant_ids
+        quant_GtDA = product_GtDA.stock_quant_ids
+
+        # create 2 moves of 1kg
+        move_LtDA = self.env['stock.move'].create({
+            'name': 'test_reserve_product_LtDA',
+            'location_id': self.stock_location,
+            'location_dest_id': self.customer_location,
+            'product_id': product_LtDA.id,
+            'product_uom': self.uom_kg.id,
+            'product_uom_qty': 1,
+        })
+
+        move_GtDA = self.env['stock.move'].create({
+            'name': 'test_reserve_product_GtDA',
+            'location_id': self.stock_location,
+            'location_dest_id': self.customer_location,
+            'product_id': product_GtDA.id,
+            'product_uom': self.uom_kg.id,
+            'product_uom_qty': 1,
+        })
+
+        self.assertEqual(move_LtDA.state, 'draft')
+        self.assertEqual(move_GtDA.state, 'draft')
+        move_LtDA._action_confirm()
+        move_GtDA._action_confirm()
+        self.assertEqual(move_LtDA.state, 'confirmed')
+        self.assertEqual(move_GtDA.state, 'confirmed')
+        # check availability, less than initial demand
+        move_LtDA._action_assign()
+        move_GtDA._action_assign()
+        self.assertEqual(move_LtDA.state, 'partially_available')
+        self.assertEqual(move_GtDA.state, 'partially_available')
+        # the initial demand is 1kg
+        self.assertEqual(move_LtDA.product_uom.id, self.uom_kg.id)
+        self.assertEqual(move_GtDA.product_uom.id, self.uom_kg.id)
+        self.assertEqual(move_LtDA.product_uom_qty, 1.0)
+        self.assertEqual(move_GtDA.product_uom_qty, 1.0)
+        # one move line is created
+        self.assertEqual(len(move_LtDA.move_line_ids), 1)
+        self.assertEqual(len(move_GtDA.move_line_ids), 1)
+
+        # increase quantity by 0.14988 kg (more precise than DA)
+        self.env['stock.quant']._update_available_quantity(product_LtDA, stock_location, 149.88)
+        self.env['stock.quant']._update_available_quantity(product_GtDA, stock_location, 149.88)
+
+        # _update_reserved_quantity is called on a move only in _action_assign
+        move_LtDA._action_assign()
+        move_GtDA._action_assign()
+
+        # as the move line for LtDA and its corresponding quant can be
+        # in different UOMs, a new move line can be created
+        # from _update_reserved_quantity
+        move_lines_LtDA = self.env["stock.move.line"].search([
+            ('product_id', '=', quant_LtDA.product_id.id),
+            ('location_id', '=', quant_LtDA.location_id.id),
+            ('lot_id', '=', quant_LtDA.lot_id.id),
+            ('package_id', '=', quant_LtDA.package_id.id),
+            ('owner_id', '=', quant_LtDA.owner_id.id),
+            ('product_qty', '!=', 0)
+        ])
+        reserved_on_move_lines_LtDA = sum(move_lines_LtDA.mapped('product_qty'))
+
+        move_lines_GtDA = self.env["stock.move.line"].search([
+            ('product_id', '=', quant_GtDA.product_id.id),
+            ('location_id', '=', quant_GtDA.location_id.id),
+            ('lot_id', '=', quant_GtDA.lot_id.id),
+            ('package_id', '=', quant_GtDA.package_id.id),
+            ('owner_id', '=', quant_GtDA.owner_id.id),
+            ('product_qty', '!=', 0)
+        ])
+        reserved_on_move_lines_GtDA = sum(move_lines_GtDA.mapped('product_qty'))
+
+        # check that we do not reserve more (in the same UOM) than the quantity in stock
+        self.assertEqual(float_compare(reserved_on_move_lines_LtDA, quant_LtDA.quantity, precision_digits=precision_digits), -1, "We do not reserve more (in the same UOM) than the quantity in stock")
+        self.assertEqual(float_compare(reserved_on_move_lines_GtDA, quant_GtDA.quantity, precision_digits=precision_digits), -1, "We do not reserve more (in the same UOM) than the quantity in stock")
+
+        # check that we reserve the same quantity in the ml and the quant
+        self.assertTrue(float_is_zero(reserved_on_move_lines_LtDA - quant_LtDA.reserved_quantity, precision_digits=precision_digits))
+        self.assertTrue(float_is_zero(reserved_on_move_lines_GtDA - quant_GtDA.reserved_quantity, precision_digits=precision_digits))
+
 
 class TestRoutes(TestStockCommon):
     def setUp(self):