fix: multiple issues related to BOM Creator

erpnext/manufacturing/doctype/bom_creator/bom_creator.py

@@ -152,6 +152,7 @@ class BOMCreator(Document):
 
 	@frappe.whitelist()
 	def add_boms(self):
+		frappe.has_permission("BOM Creator", "submit", doc=self, throw=True)
 		self.submit()
 
 	def set_rate_for_items(self):
@@ -209,10 +210,14 @@ class BOMCreator(Document):
 				frappe.throw(_("Please set {0} in BOM Creator {1}").format(_(label), self.name))
 
 	def on_submit(self):
-		self.enqueue_create_boms()
+		self.enqueue_bom_creation()
 
 	@frappe.whitelist()
 	def enqueue_create_boms(self):
+		frappe.has_permission("BOM Creator", "submit", doc=self, throw=True)
+		self.enqueue_bom_creation()
+
+	def enqueue_bom_creation(self):
 		frappe.enqueue(
 			self.create_boms,
 			queue="short",
@@ -281,6 +286,23 @@ class BOMCreator(Document):
 
 			frappe.msgprint(_("BOMs creation failed"))
 
+	@frappe.whitelist()
+	def edit_qty(self, docname: str, qty: float):
+		frappe.has_permission("BOM Creator", "write", doc=self, throw=True)
+
+		if not frappe.db.exists("BOM Creator Item", {"name": docname, "parent": self.name}):
+			frappe.throw(_("BOM Creator Item {0} does not exist").format(docname))
+
+		for row in self.items:
+			if row.name == docname:
+				row.qty = flt(qty)
+				break
+
+		self.set_rate_for_items()
+		self.save()
+
+		return self
+
 	def create_bom(self, row, production_item_wise_rm):
 		bom_creator_item = row.name if row.name != self.name else ""
 		if frappe.db.exists(
@@ -336,18 +358,23 @@ class BOMCreator(Document):
 		production_item_wise_rm[(row.item_code, row.name)].bom_no = bom.name
 
 	@frappe.whitelist()
-	def get_default_bom(self, item_code) -> str:
+	def get_default_bom(self, item_code: str) -> str:
+		frappe.has_permission("BOM Creator", "read", doc=self, throw=True)
 		return frappe.get_cached_value("Item", item_code, "default_bom")
 
 
 @frappe.whitelist()
-def get_children(doctype=None, parent=None, **kwargs):
+def get_children(doctype: str | None = None, parent: str | None = None, **kwargs):
+	# by default get_children takes first parameter as doctype, so added in the function
+
 	if isinstance(kwargs, str):
 		kwargs = frappe.parse_json(kwargs)
 
 	if isinstance(kwargs, dict):
 		kwargs = frappe._dict(kwargs)
 
+	frappe.has_permission("BOM Creator", "read", doc=kwargs.parent_id, throw=True)
+
 	fields = [
 		"item_code as value",
 		"item_name as title",
@@ -381,6 +408,8 @@ def add_item(**kwargs):
 	if isinstance(kwargs, dict):
 		kwargs = frappe._dict(kwargs)
 
+	frappe.has_permission("BOM Creator", "write", doc=kwargs.parent, throw=True)
+
 	doc = frappe.get_doc("BOM Creator", kwargs.parent)
 	item_info = get_item_details(kwargs.item_code)
 
@@ -413,6 +442,8 @@ def add_sub_assembly(**kwargs):
 	if isinstance(kwargs, dict):
 		kwargs = frappe._dict(kwargs)
 
+	frappe.has_permission("BOM Creator", "write", doc=kwargs.parent, throw=True)
+
 	doc = frappe.get_doc("BOM Creator", kwargs.parent)
 	bom_item = frappe.parse_json(kwargs.bom_item)
 
@@ -496,27 +527,29 @@ def delete_node(**kwargs):
 	if isinstance(kwargs, dict):
 		kwargs = frappe._dict(kwargs)
 
-	items = get_children(parent=kwargs.fg_item, parent_id=kwargs.parent)
+	frappe.has_permission("BOM Creator", "write", doc=kwargs.parent, throw=True)
+
+	updated = False
 	if kwargs.docname:
+		if not frappe.db.exists("BOM Creator Item", {"name": kwargs.docname, "parent": kwargs.parent}):
+			frappe.throw(_("BOM Creator Item with name {0} does not exist").format(kwargs.docname))
+
 		frappe.delete_doc("BOM Creator Item", kwargs.docname)
+		updated = True
 
-	for item in items:
+	items = get_children(parent=kwargs.fg_item, parent_id=kwargs.parent)
-		frappe.delete_doc("BOM Creator Item", item.name)
+	if items:
-		if item.expandable:
+		for item in items:
-			delete_node(fg_item=item.value, parent=item.parent_id)
+			updated = True
+			frappe.delete_doc("BOM Creator Item", item.name)
+			if item.expandable:
+				delete_node(fg_item=item.value, parent=item.parent_id)
 
-	doc = frappe.get_doc("BOM Creator", kwargs.parent)
+	if updated:
-	doc.set_rate_for_items()
+		doc = frappe.get_doc("BOM Creator", kwargs.parent)
-	doc.save()
+		doc.set_rate_for_items()
+		doc.save()
 
-	return doc
+		return doc
 
-
+	return {}
-@frappe.whitelist()
-def edit_qty(doctype, docname, qty, parent):
-	frappe.db.set_value(doctype, docname, "qty", qty)
-	doc = frappe.get_doc("BOM Creator", parent)
-	doc.set_rate_for_items()
-	doc.save()
-
-	return doc

erpnext/manufacturing/doctype/bom_creator/test_bom_creator.py

@@ -9,6 +9,8 @@ from frappe.tests.utils import FrappeTestCase
 from erpnext.manufacturing.doctype.bom_creator.bom_creator import (
 	add_item,
 	add_sub_assembly,
+	delete_node,
+	edit_qty,
 )
 from erpnext.stock.doctype.item.test_item import make_item
 
@@ -245,6 +247,113 @@ class TestBOMCreator(FrappeTestCase):
 		data = frappe.get_all("BOM", filters={"bom_creator": doc.name, "docstatus": 1})
 		self.assertEqual(len(data), 2)
 
+	def test_edit_qty_on_item_row(self):
+		doc = make_bom_creator_with_item("Bicycle Edit Qty", "Pedal Assembly", qty=2)
+		row_name = doc.items[0].name
+
+		edit_qty(docname=row_name, qty=5, parent=doc.name)
+
+		doc.reload()
+		self.assertEqual(doc.items[0].qty, 5.0)
+
+	def test_edit_qty_rejects_foreign_item(self):
+		# A BOM Creator Item belonging to a different BOM Creator must not be
+		# editable through a BOM Creator the user owns.
+		doc = make_bom_creator_with_item("Bicycle Owner BOM", "Pedal Assembly", qty=1)
+		other = make_bom_creator_with_item("Bicycle Foreign BOM", "Frame Assembly", qty=1)
+		foreign_row = other.items[0].name
+
+		self.assertRaises(frappe.ValidationError, edit_qty, docname=foreign_row, qty=5, parent=doc.name)
+
+		# The foreign row must be left untouched.
+		self.assertEqual(frappe.db.get_value("BOM Creator Item", foreign_row, "qty"), 1.0)
+
+	def test_delete_node_removes_item(self):
+		doc = make_bom_creator_with_item("Bicycle Delete Node", "Pedal Assembly", qty=1)
+		row_name = doc.items[0].name
+
+		delete_node(parent=doc.name, fg_item="Bicycle Delete Node Item", docname=row_name)
+
+		self.assertFalse(frappe.db.exists("BOM Creator Item", row_name))
+
+	def test_delete_node_rejects_foreign_item(self):
+		doc = make_bom_creator_with_item("Bicycle Delete Owner", "Pedal Assembly", qty=1)
+		other = make_bom_creator_with_item("Bicycle Delete Foreign", "Frame Assembly", qty=1)
+		foreign_row = other.items[0].name
+
+		self.assertRaises(
+			frappe.ValidationError,
+			delete_node,
+			parent=doc.name,
+			fg_item="Bicycle Delete Owner Item",
+			docname=foreign_row,
+		)
+
+		# The foreign row must still exist.
+		self.assertTrue(frappe.db.exists("BOM Creator Item", foreign_row))
+
+	def test_whitelisted_methods_require_write_permission(self):
+		doc = make_bom_creator_with_item("Bicycle Perm Check", "Pedal Assembly", qty=1)
+		row_name = doc.items[0].name
+
+		user = create_user_without_bom_access()
+		frappe.set_user(user)
+		try:
+			self.assertRaises(frappe.PermissionError, edit_qty, docname=row_name, qty=3, parent=doc.name)
+			self.assertRaises(
+				frappe.PermissionError,
+				delete_node,
+				parent=doc.name,
+				fg_item="Bicycle Perm Check Item",
+				docname=row_name,
+			)
+		finally:
+			frappe.set_user("Administrator")
+
+
+def make_bom_creator_with_item(name, item_code, qty=1):
+	final_product = f"{name} Item"
+	make_item(final_product, {"item_group": "Raw Material", "stock_uom": "Nos"})
+
+	doc = make_bom_creator(
+		name=name,
+		company="_Test Company",
+		item_code=final_product,
+		qty=1,
+		rm_cosy_as_per="Valuation Rate",
+		currency="INR",
+		plc_conversion_rate=1,
+		conversion_rate=1,
+	)
+
+	add_item(
+		parent=doc.name,
+		fg_item=final_product,
+		fg_reference_id=doc.name,
+		item_code=item_code,
+		qty=qty,
+	)
+
+	doc.reload()
+	return doc
+
+
+def create_user_without_bom_access():
+	user = "bom_creator_no_access@example.com"
+	if not frappe.db.exists("User", user):
+		frappe.get_doc(
+			{
+				"doctype": "User",
+				"email": user,
+				"first_name": "BOM No Access",
+				"send_welcome_email": 0,
+				# Stock User has no read/write permission on BOM Creator.
+				"roles": [{"role": "Stock User"}],
+			}
+		).insert(ignore_permissions=True)
+
+	return user
+
 
 def create_items():
 	raw_materials = [

erpnext/public/js/bom_configurator/bom_configurator.bundle.js

@@ -408,16 +408,14 @@ class BOMConfigurator {
 		frappe.prompt(
 			[{ label: __("Qty"), fieldname: "qty", default: qty, fieldtype: "Float", reqd: 1 }],
 			(data) => {
-				let doctype = node.data.doctype || this.frm.doc.doctype;
 				let docname = node.data.name || this.frm.doc.name;
 
 				frappe.call({
-					method: "erpnext.manufacturing.doctype.bom_creator.bom_creator.edit_qty",
+					method: "edit_qty",
+					doc: this.frm.doc,
 					args: {
-						doctype: doctype,
 						docname: docname,
 						qty: data.qty,
-						parent: node.data.parent_id ? node.data.parent_id : this.frm.doc.name,
 					},
 					callback: (r) => {
 						node.data.qty = data.qty;