fix: converted whitelist non class methods to class methods

erpnext/manufacturing/doctype/bom_creator/bom_creator.py

@@ -152,7 +152,7 @@ class BOMCreator(Document):
 
 	@frappe.whitelist()
 	def add_boms(self):
-		frappe.has_permission("BOM Creator", "submit", doc=self, throw=True)
+		self.check_permission("submit")
 		self.submit()
 
 	def set_rate_for_items(self):
@@ -214,7 +214,7 @@ class BOMCreator(Document):
 
 	@frappe.whitelist()
 	def enqueue_create_boms(self):
-		frappe.has_permission("BOM Creator", "submit", doc=self, throw=True)
+		self.check_permission("submit")
 		self.enqueue_bom_creation()
 
 	def enqueue_bom_creation(self):
@@ -288,7 +288,7 @@ class BOMCreator(Document):
 
 	@frappe.whitelist()
 	def edit_qty(self, docname: str, qty: float):
-		frappe.has_permission("BOM Creator", "write", doc=self, throw=True)
+		self.check_permission("write")
 
 		if not frappe.db.exists("BOM Creator Item", {"name": docname, "parent": self.name}):
 			frappe.throw(_("BOM Creator Item {0} does not exist").format(docname))
@@ -359,9 +359,149 @@ class BOMCreator(Document):
 
 	@frappe.whitelist()
 	def get_default_bom(self, item_code: str) -> str:
-		frappe.has_permission("BOM Creator", "read", doc=self, throw=True)
+		self.check_permission("read")
 		return frappe.get_cached_value("Item", item_code, "default_bom")
 
+	@frappe.whitelist()
+	def add_item(self, **kwargs):
+		self.check_permission("write")
+
+		if isinstance(kwargs, str):
+			kwargs = frappe.parse_json(kwargs)
+
+		if isinstance(kwargs, dict):
+			kwargs = frappe._dict(kwargs)
+
+		item_info = get_item_details(kwargs.item_code)
+
+		parent_row_no = ""
+		if kwargs.fg_reference_id and self.name != kwargs.fg_reference_id:
+			parent_row_no = get_parent_row_no(self, kwargs.fg_reference_id)
+
+		kwargs.update(
+			{
+				"uom": item_info.stock_uom,
+				"stock_uom": item_info.stock_uom,
+				"conversion_factor": 1,
+			}
+		)
+
+		if parent_row_no:
+			kwargs.update({"parent_row_no": parent_row_no})
+
+		for key in BOM_ITEM_FIELDS:
+			if key not in kwargs:
+				kwargs[key] = ""
+
+		self.append("items", kwargs)
+		self.save()
+
+		return self
+
+	@frappe.whitelist()
+	def add_sub_assembly(self, **kwargs):
+		self.check_permission("write")
+
+		if isinstance(kwargs, str):
+			kwargs = frappe.parse_json(kwargs)
+
+		if isinstance(kwargs, dict):
+			kwargs = frappe._dict(kwargs)
+
+		bom_item = frappe.parse_json(kwargs.bom_item)
+
+		name = kwargs.fg_reference_id
+		parent_row_no = ""
+		if not kwargs.convert_to_sub_assembly:
+			item_info = get_item_details(bom_item.item_code)
+			parent_row_no = get_parent_row_no(self, kwargs.fg_reference_id)
+
+			item_row = self.append(
+				"items",
+				{
+					"item_code": bom_item.item_code,
+					"qty": bom_item.qty,
+					"uom": item_info.stock_uom,
+					"fg_item": kwargs.fg_item,
+					"conversion_factor": 1,
+					"parent_row_no": parent_row_no,
+					"fg_reference_id": name,
+					"stock_qty": bom_item.qty,
+					"do_not_explode": 1,
+					"is_expandable": 1,
+					"stock_uom": item_info.stock_uom,
+					"allow_alternative_item": kwargs.allow_alternative_item,
+				},
+			)
+
+			parent_row_no = item_row.idx
+			name = ""
+		else:
+			parent_row_no = get_parent_row_no(self, kwargs.fg_reference_id)
+
+		for row in bom_item.get("items"):
+			row = frappe._dict(row)
+			item_info = get_item_details(row.item_code)
+			self.append(
+				"items",
+				{
+					"item_code": row.item_code,
+					"qty": row.qty,
+					"fg_item": bom_item.item_code,
+					"uom": item_info.stock_uom,
+					"fg_reference_id": name,
+					"parent_row_no": parent_row_no,
+					"conversion_factor": 1,
+					"do_not_explode": 1,
+					"stock_qty": row.qty,
+					"stock_uom": item_info.stock_uom,
+				},
+			)
+
+		self.save()
+
+		return self
+
+	@frappe.whitelist()
+	def delete_node(self, **kwargs):
+		self.check_permission("write")
+
+		if isinstance(kwargs, str):
+			kwargs = frappe.parse_json(kwargs)
+
+		if isinstance(kwargs, dict):
+			kwargs = frappe._dict(kwargs)
+
+		updated = False
+		if kwargs.docname:
+			row = next((row for row in self.items if row.name == kwargs.docname), None)
+			if not row:
+				frappe.throw(_("BOM Creator Item with name {0} does not exist").format(kwargs.docname))
+
+			row.delete()
+			self.remove(row)
+			updated = True
+
+		items = get_children(parent=kwargs.fg_item, parent_id=self.name)
+		if items:
+			for item in items:
+				updated = True
+				child_row = next((row for row in self.items if row.name == item.name), None)
+				if child_row:
+					child_row.delete()
+					self.remove(child_row)
+
+				if item.expandable:
+					self.delete_node(fg_item=item.value)
+
+		if updated:
+			self.set_rate_for_items()
+			self.save()
+
+			return self
+
+		return frappe._dict()
+
 
 @frappe.whitelist()
 def get_children(doctype: str | None = None, parent: str | None = None, **kwargs):
@@ -400,106 +540,6 @@ def get_children(doctype: str | None = None, parent: str | None = None, **kwargs
 	return frappe.get_all("BOM Creator Item", fields=fields, filters=query_filters, order_by="idx")
 
 
-@frappe.whitelist()
-def add_item(**kwargs):
-	if isinstance(kwargs, str):
-		kwargs = frappe.parse_json(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)
-
-	parent_row_no = ""
-	if kwargs.fg_reference_id and doc.name != kwargs.fg_reference_id:
-		parent_row_no = get_parent_row_no(doc, kwargs.fg_reference_id)
-
-	kwargs.update(
-		{
-			"uom": item_info.stock_uom,
-			"stock_uom": item_info.stock_uom,
-			"conversion_factor": 1,
-		}
-	)
-
-	if parent_row_no:
-		kwargs.update({"parent_row_no": parent_row_no})
-
-	doc.append("items", kwargs)
-	doc.save()
-
-	return doc
-
-
-@frappe.whitelist()
-def add_sub_assembly(**kwargs):
-	if isinstance(kwargs, str):
-		kwargs = frappe.parse_json(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)
-
-	name = kwargs.fg_reference_id
-	parent_row_no = ""
-	if not kwargs.convert_to_sub_assembly:
-		item_info = get_item_details(bom_item.item_code)
-		parent_row_no = get_parent_row_no(doc, kwargs.fg_reference_id)
-
-		item_row = doc.append(
-			"items",
-			{
-				"item_code": bom_item.item_code,
-				"qty": bom_item.qty,
-				"uom": item_info.stock_uom,
-				"fg_item": kwargs.fg_item,
-				"conversion_factor": 1,
-				"parent_row_no": parent_row_no,
-				"fg_reference_id": name,
-				"stock_qty": bom_item.qty,
-				"do_not_explode": 1,
-				"is_expandable": 1,
-				"stock_uom": item_info.stock_uom,
-				"allow_alternative_item": kwargs.allow_alternative_item,
-			},
-		)
-
-		parent_row_no = item_row.idx
-		name = ""
-	else:
-		parent_row_no = get_parent_row_no(doc, kwargs.fg_reference_id)
-
-	for row in bom_item.get("items"):
-		row = frappe._dict(row)
-		item_info = get_item_details(row.item_code)
-		doc.append(
-			"items",
-			{
-				"item_code": row.item_code,
-				"qty": row.qty,
-				"fg_item": bom_item.item_code,
-				"uom": item_info.stock_uom,
-				"fg_reference_id": name,
-				"parent_row_no": parent_row_no,
-				"conversion_factor": 1,
-				"do_not_explode": 1,
-				"stock_qty": row.qty,
-				"stock_uom": item_info.stock_uom,
-			},
-		)
-
-	doc.save()
-
-	return doc
-
-
 def get_item_details(item_code):
 	return frappe.get_cached_value(
 		"Item", item_code, ["item_name", "description", "image", "stock_uom", "default_bom"], as_dict=1
@@ -517,39 +557,3 @@ def get_parent_row_no(doc, name):
 	frappe.msgprint(_("Parent Row No not found for {0}").format(name), alert=True)
 
 	return None
-
-
-@frappe.whitelist()
-def delete_node(**kwargs):
-	if isinstance(kwargs, str):
-		kwargs = frappe.parse_json(kwargs)
-
-	if isinstance(kwargs, dict):
-		kwargs = frappe._dict(kwargs)
-
-	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
-
-	items = get_children(parent=kwargs.fg_item, parent_id=kwargs.parent)
-	if items:
-		for item in items:
-			updated = True
-			frappe.delete_doc("BOM Creator Item", item.name)
-			if item.expandable:
-				delete_node(fg_item=item.value, parent=item.parent_id)
-
-	if updated:
-		doc = frappe.get_doc("BOM Creator", kwargs.parent)
-		doc.set_rate_for_items()
-		doc.save()
-
-		return doc
-
-	return {}

erpnext/manufacturing/doctype/bom_creator/test_bom_creator.py

@@ -6,12 +6,6 @@ import random
 import frappe
 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
 
 
@@ -40,8 +34,7 @@ class TestBOMCreator(FrappeTestCase):
 			conversion_rate=1,
 		)
 
-		add_sub_assembly(
-			parent=doc.name,
+		doc.add_sub_assembly(
 			fg_item=final_product,
 			fg_reference_id=doc.name,
 			bom_item={
@@ -93,8 +86,7 @@ class TestBOMCreator(FrappeTestCase):
 			conversion_rate=1,
 		)
 
-		add_item(
-			parent=doc.name,
+		doc.add_item(
 			fg_item=final_product,
 			fg_reference_id=doc.name,
 			item_code="Pedal Assembly",
@@ -135,8 +127,7 @@ class TestBOMCreator(FrappeTestCase):
 			conversion_rate=1,
 		)
 
-		add_item(
-			parent=doc.name,
+		doc.add_item(
 			fg_item=final_product,
 			fg_reference_id=doc.name,
 			item_code="Pedal Assembly",
@@ -146,9 +137,8 @@ class TestBOMCreator(FrappeTestCase):
 		doc.reload()
 		self.assertEqual(doc.items[0].is_expandable, 0)
 
-		add_sub_assembly(
+		doc.add_sub_assembly(
 			convert_to_sub_assembly=1,
-			parent=doc.name,
 			fg_item=final_product,
 			fg_reference_id=doc.items[0].name,
 			bom_item={
@@ -201,8 +191,7 @@ class TestBOMCreator(FrappeTestCase):
 			conversion_rate=1,
 		)
 
-		add_item(
-			parent=doc.name,
+		doc.add_item(
 			fg_item=final_product,
 			fg_reference_id=doc.name,
 			item_code="Pedal Assembly",
@@ -212,9 +201,8 @@ class TestBOMCreator(FrappeTestCase):
 		doc.reload()
 		self.assertEqual(doc.items[0].is_expandable, 0)
 
-		add_sub_assembly(
+		doc.add_sub_assembly(
 			convert_to_sub_assembly=1,
-			parent=doc.name,
 			fg_item=final_product,
 			fg_reference_id=doc.items[0].name,
 			bom_item={
@@ -247,113 +235,6 @@ 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

@@ -219,14 +219,10 @@ class BOMConfigurator {
 				},
 			],
 			(data) => {
-				if (!node.data.parent_id) {
-					node.data.parent_id = this.frm.doc.name;
-				}
-
 				frappe.call({
-					method: "erpnext.manufacturing.doctype.bom_creator.bom_creator.add_item",
+					method: "add_item",
+					doc: this.frm.doc,
 					args: {
-						parent: node.data.parent_id,
 						fg_item: node.data.value,
 						item_code: data.item_code,
 						fg_reference_id: node.data.name || this.frm.doc.name,
@@ -255,14 +251,10 @@ class BOMConfigurator {
 		dialog.set_primary_action(__("Add"), () => {
 			let bom_item = dialog.get_values();
 
-			if (!node.data?.parent_id) {
-				node.data.parent_id = this.frm.doc.name;
-			}
-
 			frappe.call({
-				method: "erpnext.manufacturing.doctype.bom_creator.bom_creator.add_sub_assembly",
+				method: "add_sub_assembly",
+				doc: this.frm.doc,
 				args: {
-					parent: node.data.parent_id,
 					fg_item: node.data.value,
 					fg_reference_id: node.data.name || this.frm.doc.name,
 					bom_item: bom_item,
@@ -357,9 +349,9 @@ class BOMConfigurator {
 			let bom_item = dialog.get_values();
 
 			frappe.call({
-				method: "erpnext.manufacturing.doctype.bom_creator.bom_creator.add_sub_assembly",
+				method: "add_sub_assembly",
+				doc: this.frm.doc,
 				args: {
-					parent: node.data.parent_id,
 					fg_item: node.data.value,
 					bom_item: bom_item,
 					fg_reference_id: node.data.name || this.frm.doc.name,
@@ -389,11 +381,10 @@ class BOMConfigurator {
 	delete_node(node, view) {
 		frappe.confirm(__("Are you sure you want to delete this Item?"), () => {
 			frappe.call({
-				method: "erpnext.manufacturing.doctype.bom_creator.bom_creator.delete_node",
+				method: "delete_node",
+				doc: this.frm.doc,
 				args: {
-					parent: node.data.parent_id,
 					fg_item: node.data.value,
-					doctype: node.data.doctype,
 					docname: node.data.name,
 				},
 				callback: (r) => {