Merge lp:~therp-nl/openobject-addons/7.0-addons_lp985525-wrong-bom-test into lp:openobject-addons/7.0

[Ronald Portier (Therp)]

Replace checks for recursion and for product not being part (directly or indirectly) of its own bom.

[Ronald Portier (Therp) (rportier1962)]

Changed status to work in progress, because rethinking the problem the solution is not bullet proof.

My code guards against creating recursion by adding a product on a bom line that is among the ancestors (following bom_id) of that line.

But recursion can also be caused by creating a containing bill of material that has the product somewhere as a component among its descendents. I think that is the problem the original code tried (in the wrong way) to solve.

Even creating an "in between" bill of material, might cause recursion if it has some product somewhere both on the way up, or the way down.

Unmerged revisions

8690. By Ronald Portier (Therp) on 2013-06-01

[FIX] - Replace test for recursion in product or bom itself with
    code that works, is understandable, and uses ORM instead of plain SQL.

=== modified file 'mrp/mrp.py'
--- mrp/mrp.py	2012-12-20 11:47:30 +0000
+++ mrp/mrp.py	2013-06-01 17:25:55 +0000
@@ -235,29 +235,76 @@
     ]
 
     def _check_recursion(self, cr, uid, ids, context=None):
-        level = 100
-        while len(ids):
-            cr.execute('select distinct bom_id from mrp_bom where id IN %s',(tuple(ids),))
-            ids = filter(None, map(lambda x:x[0], cr.fetchall()))
-            if not level:
-                return False
-            level -= 1
+        for original_id in ids:
+            check_id = original_id
+            while check_id:
+                record = self.read(
+                    cr, uid, [check_id], ['bom_id'], context=context)[0]
+                if record and record['bom_id']:
+                    check_id = record['bom_id'][0]
+                    if check_id == original_id:
+                        return False
+                else:
+                    check_id = False
         return True
 
     def _check_product(self, cr, uid, ids, context=None):
-        all_prod = []
-        boms = self.browse(cr, uid, ids, context=context)
-        def check_bom(boms):
-            res = True
-            for bom in boms:
-                if bom.product_id.id in all_prod:
-                    res = res and False
-                all_prod.append(bom.product_id.id)
-                lines = bom.bom_lines
-                if lines:
-                    res = res and check_bom([bom_id for bom_id in lines if bom_id not in boms])
-            return res
-        return check_bom(boms)
+
+        def get_record(cr, uid, mb_id, context=None):
+            return self.read(
+                cr, uid, [mb_id], ['bom_id', 'product_id'],
+                context=context)[0]
+
+        def where_used(cr, uid, product_id, context=None):
+            # Get all the records for which the main product is
+            # a subproduct.
+            product_ids = []
+            bom_ids = self.search(
+                cr, uid, [('product_id', '=', product_id),
+                            ('bom_id', '<>', False)],
+                context=context)
+            # get all the parents of these lines
+            records = self.read(
+                cr, uid, bom_ids, ['bom_id'],
+                context=context)
+            parent_ids = []
+            for record in records:
+                parent_id = record['bom_id'][0]
+                if parent_id not in parent_ids:
+                    parent_ids.append(parent_id )
+                # now read parents for the actual products
+                records = self.read(
+                    cr, uid, parent_ids, ['product_id'],
+                    context=context)
+                for record in records:
+                    product_id = record['product_id'][0]
+                    if not product_id in product_ids:
+                        product_ids.append(product_id)
+            return product_ids
+
+        for original_id in ids:
+            to_be_checked = []
+            checked = []
+
+            record = get_record(cr, uid, original_id, context=context)
+            # If bom record not for a line (but for main product)
+            # no reason to check further.
+            if not record['bom_id']:
+                continue
+            original_product = record['product_id'][0]
+            record = get_record(cr, uid, record['bom_id'][0], context=context)
+            to_be_checked.append(record['product_id'][0])
+            while len(to_be_checked) > 0:
+                product_id = to_be_checked.pop()
+                if product_id == original_product:
+                    return False
+                checked.append(product_id)
+                maybe_check = where_used(
+                    cr, uid, product_id, context=None)
+                for product_id in maybe_check:
+                    if product_id not in checked:
+                        to_be_checked.append(product_id)
+        return True
 
     _constraints = [
         (_check_recursion, 'Error ! You cannot create recursive BoM.', ['parent_id']),