diff --git a/erpnext/manufacturing/doctype/bom_creator/bom_creator.py b/erpnext/manufacturing/doctype/bom_creator/bom_creator.py index 55d37b3e588..54c402870a9 100644 --- a/erpnext/manufacturing/doctype/bom_creator/bom_creator.py +++ b/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: - frappe.delete_doc("BOM Creator Item", item.name) - if item.expandable: - delete_node(fg_item=item.value, parent=item.parent_id) + 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) - doc = frappe.get_doc("BOM Creator", kwargs.parent) - doc.set_rate_for_items() - doc.save() + if updated: + doc = frappe.get_doc("BOM Creator", kwargs.parent) + doc.set_rate_for_items() + doc.save() - return doc + return doc - -@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 + return {} diff --git a/erpnext/manufacturing/doctype/bom_creator/test_bom_creator.py b/erpnext/manufacturing/doctype/bom_creator/test_bom_creator.py index a835de78274..6b5c28cd7bc 100644 --- a/erpnext/manufacturing/doctype/bom_creator/test_bom_creator.py +++ b/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 = [ diff --git a/erpnext/public/js/bom_configurator/bom_configurator.bundle.js b/erpnext/public/js/bom_configurator/bom_configurator.bundle.js index d25ca212b41..9d79d2f019e 100644 --- a/erpnext/public/js/bom_configurator/bom_configurator.bundle.js +++ b/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;