fix: don't allow BOM's item code at any level of child items (#27157)

* refactor: bom recursion checking

* fix: dont allow bom recursion

if same item_code is added in child items at any level, it shouldn't be allowed.

* test: add test for bom recursion

* test: fix broken prodplan test using recursive bom

* test: fix recursive bom in tests

(cherry picked from commit c07dce940e)

# Conflicts:
#	erpnext/manufacturing/doctype/bom/bom.py
#	erpnext/manufacturing/doctype/production_plan/test_production_plan.py
#	erpnext/manufacturing/doctype/work_order/test_work_order.py
This commit is contained in:
Ankush Menat
2021-08-26 18:33:42 +05:30
committed by Mergify
parent f5160dc83d
commit b001f0db9e
4 changed files with 99 additions and 0 deletions

View File

@@ -536,6 +536,7 @@ class BOM(WebsiteGenerator):
check_list.append(m)
def check_recursion(self, bom_list=None):
<<<<<<< HEAD
"""Check whether recursion occurs in any bom"""
def _throw_error(bom_name):
@@ -560,6 +561,29 @@ class BOM(WebsiteGenerator):
_throw_error(item.bom_no)
if self.name in {d.bom_no for d in self.items}:
=======
""" 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()
child_items = frappe.get_all('BOM Item', fields=["bom_no", "item_code"],
filters={'parent': ('in', bom_list), 'parenttype': 'BOM'}) or []
child_bom = {d.bom_no for d in child_items}
child_items_codes = {d.item_code for d in child_items}
if self.name in child_bom:
_throw_error(self.name)
if self.item in child_items_codes:
_throw_error(self.item)
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}:
>>>>>>> c07dce940e (fix: don't allow BOM's item code at any level of child items (#27157))
_throw_error(self.name)
def traverse_tree(self, bom_list=None):

View File

@@ -297,6 +297,46 @@ class TestBOM(FrappeTestCase):
bom1.save()
bom2.save()
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()

View File

@@ -376,10 +376,16 @@ class TestProductionPlan(FrappeTestCase):
self.assertEqual(warehouses, expected_warehouses)
def test_get_sales_order_with_variant(self):
<<<<<<< HEAD
"Check if Template BOM is fetched in absence of Variant BOM."
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)
=======
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)
>>>>>>> c07dce940e (fix: don't allow BOM's item code at any level of child items (#27157))
variant_settings = {
"attributes": [
{"attribute": "Colour"},
@@ -388,20 +394,34 @@ class TestProductionPlan(FrappeTestCase):
}
item.update(variant_settings)
item.save()
<<<<<<< HEAD
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=[rm_item.item_code])
=======
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 = [rm_item.item_code])
>>>>>>> c07dce940e (fix: don't allow BOM's item code at any level of child items (#27157))
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()
<<<<<<< HEAD
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=[rm_item.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 = [rm_item.item_code])
>>>>>>> c07dce940e (fix: don't allow BOM's item code at any level of child items (#27157))
"""Testing when item variant has a BOM"""
so = make_sales_order(item_code="PIV-RED", qty=5)

View File

@@ -801,6 +801,7 @@ class TestWorkOrder(FrappeTestCase):
self.assertEqual(item.valuation_rate, 0)
def test_valuation_rate_missing_on_make_stock_entry(self):
<<<<<<< HEAD
item_name = "Test Valuation Rate Missing"
rm_item = "_Test raw material item"
make_item(
@@ -819,6 +820,20 @@ class TestWorkOrder(FrappeTestCase):
)
if not frappe.db.get_value("BOM", {"item": item_name}):
=======
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}):
>>>>>>> c07dce940e (fix: don't allow BOM's item code at any level of child items (#27157))
make_bom(item=item_name, raw_materials=[rm_item], rm_qty=1)
company = "_Test Company with perpetual inventory"