From 9d6ff93d0f5b39e939f632486a97093b59d335ef Mon Sep 17 00:00:00 2001
From: LeDungViindoo <anhdung288.viindoo@gmail.com>
Date: Tue, 25 Apr 2023 02:01:17 +0000
Subject: [PATCH] [FIX] mrp: enforce constrains check for cost share of
 byproducts in MO

Steps to reproduce:

Step 1: Create new MO
Step 2: In page by-products click three dots to show column cost_share
in tree by-products
Step 3: Create new Byproduct and input cost share >100% or <0% or input
two byproducts
        where their cost share adds up to more than 100%

Expected result:
Validation error: total byproduct cost_share cannot exceed 100
or Validation error: cost_share values must be positive

Actual result:
Values saves without issue

Issue is due to `move_byproduct_ids` being removed in the mrp_production
overrides of
`write()` or `create()` and having the `_compute_move_byproduct_ids`
populate its values.
This removal is causing the constrains to not be enforced, therefore we
set the constrains
field to `move_finished_ids` since changing of this value will ensure
that the values are
correctly checked.
Note: There will be a decrease in performance since this means the
constrains will be
called whenever the MO's product to produce is changed, but hopefully it
will be
minimal since there is no simple fix for this issue.

closes odoo/odoo#119009

Signed-off-by: Tiffany Chang <tic@odoo.com>
---
 addons/mrp/models/mrp_production.py |  2 +-
 addons/mrp/tests/test_byproduct.py  | 58 +++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/addons/mrp/models/mrp_production.py b/addons/mrp/models/mrp_production.py
index 65a4765bcfd0..80ba3b6b1212 100644
--- a/addons/mrp/models/mrp_production.py
+++ b/addons/mrp/models/mrp_production.py
@@ -725,7 +725,7 @@ class MrpProduction(models.Model):
         else:
             self.workorder_ids = False
 
-    @api.constrains('move_byproduct_ids')
+    @api.constrains('move_finished_ids')
     def _check_byproducts(self):
         for order in self:
             if any(move.cost_share < 0 for move in order.move_byproduct_ids):
diff --git a/addons/mrp/tests/test_byproduct.py b/addons/mrp/tests/test_byproduct.py
index 2002e04ad3a0..ac1147b082b9 100644
--- a/addons/mrp/tests/test_byproduct.py
+++ b/addons/mrp/tests/test_byproduct.py
@@ -3,6 +3,7 @@
 
 from odoo.tests import Form
 from odoo.tests import common
+from odoo.exceptions import ValidationError
 
 
 class TestMrpByProduct(common.TransactionCase):
@@ -205,3 +206,60 @@ class TestMrpByProduct(common.TransactionCase):
         finished_move_line = mo.move_finished_ids.filtered(lambda m: m.product_id == self.product_a).move_line_ids
         self.assertEqual(byproduct_move_line.location_dest_id, shelf2_location)
         self.assertEqual(finished_move_line.location_dest_id, shelf2_location)
+
+    def test_check_byproducts_cost_share(self):
+        """
+        Test that byproducts with total cost_share > 100% or a cost_share < 0%
+        will throw a ValidationError
+        """
+        # Create new MO
+        mo_form = Form(self.env['mrp.production'])
+        mo_form.product_id = self.product_a
+        mo_form.product_qty = 2.0
+        mo = mo_form.save()
+
+        # Create product
+        self.product_d = self.env['product.product'].create({
+                'name': 'Product D',
+                'type': 'product'})
+        self.product_e = self.env['product.product'].create({
+                'name': 'Product E',
+                'type': 'product'})
+
+        # Create byproduct
+        byproduct_1 = self.env['stock.move'].create({
+            'name': 'By Product 1',
+            'product_id': self.product_d.id,
+            'product_uom': self.ref('uom.product_uom_unit'),
+            'production_id': mo.id,
+            'location_id': self.ref('stock.stock_location_stock'),
+            'location_dest_id': self.ref('stock.stock_location_output'),
+            'product_uom_qty': 0,
+            'quantity_done': 0
+            })
+        byproduct_2 = self.env['stock.move'].create({
+            'name': 'By Product 2',
+            'product_id': self.product_e.id,
+            'product_uom': self.ref('uom.product_uom_unit'),
+            'production_id': mo.id,
+            'location_id': self.ref('stock.stock_location_stock'),
+            'location_dest_id': self.ref('stock.stock_location_output'),
+            'product_uom_qty': 0,
+            'quantity_done': 0
+            })
+
+        # Update byproduct has cost share > 100%
+        with self.assertRaises(ValidationError), self.cr.savepoint():
+            byproduct_1.cost_share = 120
+            mo.write({'move_byproduct_ids': [(4, byproduct_1.id)]})
+
+        # Update byproduct has cost share < 0%
+        with self.assertRaises(ValidationError), self.cr.savepoint():
+            byproduct_1.cost_share = -10
+            mo.write({'move_byproduct_ids': [(4, byproduct_1.id)]})
+
+        # Update byproducts have total cost share > 100%
+        with self.assertRaises(ValidationError), self.cr.savepoint():
+            byproduct_1.cost_share = 60
+            byproduct_2.cost_share = 70
+            mo.write({'move_byproduct_ids': [(6, 0, [byproduct_1.id, byproduct_2.id])]})
-- 
GitLab