diff --git a/erpnext/manufacturing/doctype/bom/bom.py b/erpnext/manufacturing/doctype/bom/bom.py index 24f84e63b3..1d7d451cc8 100644 --- a/erpnext/manufacturing/doctype/bom/bom.py +++ b/erpnext/manufacturing/doctype/bom/bom.py @@ -447,25 +447,29 @@ class BOM(WebsiteGenerator): frappe.throw(_("Quantity required for Item {0} in row {1}").format(m.item_code, m.idx)) check_list.append(m) - def check_recursion(self, bom_list=[]): + def check_recursion(self, bom_list=None): """ Check whether recursion occurs in any bom""" + def _throw_error(bom_name): + frappe.throw(_("BOM recursion: {0} cannot be parent or child of {0}").format(bom_name)) + bom_list = self.traverse_tree() - bom_nos = frappe.get_all('BOM Item', fields=["bom_no"], - filters={'parent': ('in', bom_list), 'parenttype': 'BOM'}) + child_items = frappe.get_all('BOM Item', fields=["bom_no", "item_code"], + filters={'parent': ('in', bom_list), 'parenttype': 'BOM'}) or [] - raise_exception = False - if bom_nos and self.name in [d.bom_no for d in bom_nos]: - raise_exception = True + child_bom = {d.bom_no for d in child_items} + child_items_codes = {d.item_code for d in child_items} - if not raise_exception: - bom_nos = frappe.get_all('BOM Item', fields=["parent"], - filters={'bom_no': self.name, 'parenttype': 'BOM'}) + if self.name in child_bom: + _throw_error(self.name) - if self.name in [d.parent for d in bom_nos]: - raise_exception = True + if self.item in child_items_codes: + _throw_error(self.item) - if raise_exception: - frappe.throw(_("BOM recursion: {0} cannot be parent or child of {1}").format(self.name, self.name)) + bom_nos = frappe.get_all('BOM Item', fields=["parent"], + filters={'bom_no': self.name, 'parenttype': 'BOM'}) or [] + + if self.name in {d.parent for d in bom_nos}: + _throw_error(self.name) def traverse_tree(self, bom_list=None): def _get_children(bom_no): diff --git a/erpnext/manufacturing/doctype/bom/test_bom.py b/erpnext/manufacturing/doctype/bom/test_bom.py index b8f0db0de2..8408f10b18 100644 --- a/erpnext/manufacturing/doctype/bom/test_bom.py +++ b/erpnext/manufacturing/doctype/bom/test_bom.py @@ -280,6 +280,46 @@ class TestBOM(unittest.TestCase): self.assertEqual(reqd_item.qty, created_item.qty) self.assertEqual(reqd_item.exploded_qty, created_item.exploded_qty) + def test_bom_recursion_1st_level(self): + """BOM should not allow BOM item again in child""" + item_code = "_Test BOM Recursion" + make_item(item_code, {'is_stock_item': 1}) + + bom = frappe.new_doc("BOM") + bom.item = item_code + bom.append("items", frappe._dict(item_code=item_code)) + with self.assertRaises(frappe.ValidationError) as err: + bom.save() + + self.assertTrue("recursion" in str(err.exception).lower()) + frappe.delete_doc("BOM", bom.name, ignore_missing=True) + + def test_bom_recursion_transitive(self): + item1 = "_Test BOM Recursion" + item2 = "_Test BOM Recursion 2" + make_item(item1, {'is_stock_item': 1}) + make_item(item2, {'is_stock_item': 1}) + + bom1 = frappe.new_doc("BOM") + bom1.item = item1 + bom1.append("items", frappe._dict(item_code=item2)) + bom1.save() + bom1.submit() + + bom2 = frappe.new_doc("BOM") + bom2.item = item2 + bom2.append("items", frappe._dict(item_code=item1)) + + with self.assertRaises(frappe.ValidationError) as err: + bom2.save() + bom2.submit() + + self.assertTrue("recursion" in str(err.exception).lower()) + + bom1.cancel() + frappe.delete_doc("BOM", bom1.name, ignore_missing=True, force=True) + frappe.delete_doc("BOM", bom2.name, ignore_missing=True, force=True) + def test_bom_with_process_loss_item(self): fg_item_non_whole, fg_item_whole, bom_item = create_process_loss_bom_items() diff --git a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py index a5b9ff845f..78028039c4 100644 --- a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py +++ b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py @@ -288,6 +288,7 @@ class TestProductionPlan(unittest.TestCase): self.assertEqual(warehouses, expected_warehouses) def test_get_sales_order_with_variant(self): + rm_item = create_item('PIV_RM', valuation_rate = 100) if not frappe.db.exists('Item', {"item_code": 'PIV'}): item = create_item('PIV', valuation_rate = 100) variant_settings = { @@ -300,20 +301,20 @@ class TestProductionPlan(unittest.TestCase): } item.update(variant_settings) item.save() - parent_bom = make_bom(item = 'PIV', raw_materials = ['PIV']) + parent_bom = make_bom(item = 'PIV', raw_materials = [rm_item.item_code]) if not frappe.db.exists('BOM', {"item": 'PIV'}): - parent_bom = make_bom(item = 'PIV', raw_materials = ['PIV']) + parent_bom = make_bom(item = 'PIV', raw_materials = [rm_item.item_code]) else: parent_bom = frappe.get_doc('BOM', {"item": 'PIV'}) if not frappe.db.exists('Item', {"item_code": 'PIV-RED'}): variant = create_variant("PIV", {"Colour": "Red"}) variant.save() - variant_bom = make_bom(item = variant.item_code, raw_materials = [variant.item_code]) + variant_bom = make_bom(item = variant.item_code, raw_materials = [rm_item.item_code]) else: variant = frappe.get_doc('Item', 'PIV-RED') if not frappe.db.exists('BOM', {"item": 'PIV-RED'}): - variant_bom = make_bom(item = variant.item_code, raw_materials = [variant.item_code]) + variant_bom = make_bom(item = variant.item_code, raw_materials = [rm_item.item_code]) """Testing when item variant has a BOM""" so = make_sales_order(item_code="PIV-RED", qty=5) diff --git a/erpnext/manufacturing/doctype/work_order/test_work_order.py b/erpnext/manufacturing/doctype/work_order/test_work_order.py index 3a334a530c..c0ed611534 100644 --- a/erpnext/manufacturing/doctype/work_order/test_work_order.py +++ b/erpnext/manufacturing/doctype/work_order/test_work_order.py @@ -675,13 +675,18 @@ class TestWorkOrder(unittest.TestCase): def test_valuation_rate_missing_on_make_stock_entry(self): item_name = 'Test Valuation Rate Missing' + rm_item = '_Test raw material item' make_item(item_name, { "is_stock_item": 1, "include_item_in_manufacturing": 1, }) + make_item('_Test raw material item', { + "is_stock_item": 1, + "include_item_in_manufacturing": 1, + }) if not frappe.db.get_value('BOM', {'item': item_name}): - make_bom(item=item_name, raw_materials=[item_name], rm_qty=1) + make_bom(item=item_name, raw_materials=[rm_item], rm_qty=1) company = "_Test Company with perpetual inventory" source_warehouse = create_warehouse("Test Valuation Rate Missing Warehouse", company=company)