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

erpnext/__init__.py

@@ -4,7 +4,7 @@ import frappe
 
 from erpnext.hooks import regional_overrides
 
-__version__ = "13.55.2"
+__version__ = "13.54.4"
 
 
 def get_default_company(user=None):

erpnext/accounts/doctype/pos_invoice/pos_invoice.py

@@ -704,7 +704,7 @@ def get_pos_reserved_qty(item_code, warehouse):
 	reserved_qty = (
 		frappe.qb.from_(p_inv)
 		.from_(p_item)
-		.select(Sum(p_item.qty).as_("qty"))
+		.select(Sum(p_item.stock_qty).as_("stock_qty"))
 		.where(
 			(p_inv.name == p_item.parent)
 			& (IfNull(p_inv.consolidated_invoice, "") == "")
@@ -715,7 +715,7 @@ def get_pos_reserved_qty(item_code, warehouse):
 		)
 	).run(as_dict=True)
 
-	return reserved_qty[0].qty or 0 if reserved_qty else 0
+	return flt(reserved_qty[0].stock_qty) if reserved_qty else 0
 
 
 @frappe.whitelist()

erpnext/manufacturing/doctype/bom/bom.py

@@ -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):

erpnext/manufacturing/doctype/bom/test_bom.py

@@ -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()
 

erpnext/manufacturing/doctype/production_plan/test_production_plan.py

@@ -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)

erpnext/manufacturing/doctype/work_order/test_work_order.py

@@ -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"