diff --git a/erpnext/manufacturing/doctype/bom_creator/bom_creator.py b/erpnext/manufacturing/doctype/bom_creator/bom_creator.py index 54c402870a9..71ba95cc8a6 100644 --- a/erpnext/manufacturing/doctype/bom_creator/bom_creator.py +++ b/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 {} diff --git a/erpnext/manufacturing/doctype/bom_creator/test_bom_creator.py b/erpnext/manufacturing/doctype/bom_creator/test_bom_creator.py index 6b5c28cd7bc..331fdf61768 100644 --- a/erpnext/manufacturing/doctype/bom_creator/test_bom_creator.py +++ b/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 = [ diff --git a/erpnext/public/js/bom_configurator/bom_configurator.bundle.js b/erpnext/public/js/bom_configurator/bom_configurator.bundle.js index 9d79d2f019e..240c86a90d9 100644 --- a/erpnext/public/js/bom_configurator/bom_configurator.bundle.js +++ b/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) => {