diff --git a/erpnext/manufacturing/doctype/bom_creator/bom_creator.py b/erpnext/manufacturing/doctype/bom_creator/bom_creator.py index 54d4fd48611..296fc597aad 100644 --- a/erpnext/manufacturing/doctype/bom_creator/bom_creator.py +++ b/erpnext/manufacturing/doctype/bom_creator/bom_creator.py @@ -245,10 +245,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): + self.check_permission("submit") + self.enqueue_bom_creation() + + def enqueue_bom_creation(self): frappe.enqueue( self.create_boms, queue="short", @@ -395,7 +399,13 @@ class BOMCreator(Document): @frappe.whitelist() +<<<<<<< HEAD def get_children(doctype=None, parent=None, **kwargs): +======= +def get_children(parent: str | None = None, **kwargs): + frappe.has_permission("BOM Creator", "read", throw=True) + +>>>>>>> daf3f2e142 (fix: multiple issues related to BOM Creator) if isinstance(kwargs, str): kwargs = frappe.parse_json(kwargs) @@ -431,6 +441,8 @@ def get_children(doctype=None, parent=None, **kwargs): @frappe.whitelist() def add_item(**kwargs): + frappe.has_permission("BOM Creator", "write", throw=True) + if isinstance(kwargs, str): kwargs = frappe.parse_json(kwargs) @@ -463,6 +475,8 @@ def add_item(**kwargs): @frappe.whitelist() def add_sub_assembly(**kwargs): + frappe.has_permission("BOM Creator", "write", throw=True) + if isinstance(kwargs, str): kwargs = frappe.parse_json(kwargs) @@ -552,39 +566,58 @@ def get_parent_row_no(doc, name): @frappe.whitelist() def delete_node(**kwargs): + frappe.has_permission("BOM Creator", "write", throw=True) + if isinstance(kwargs, str): kwargs = frappe.parse_json(kwargs) if isinstance(kwargs, dict): kwargs = frappe._dict(kwargs) - items = get_children(parent=kwargs.fg_item, parent_id=kwargs.parent) + 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 + + return frappe._dict() @frappe.whitelist() -def edit_bom_creator(doctype: str, docname: str, data: str | dict, parent: str): - if not frappe.has_permission(doctype=doctype, ptype="write", parent_doctype="BOM Creator"): - frappe.throw(_("You do not have permission to edit this document"), frappe.PermissionError) +def edit_bom_creator(docname: str, data: str | dict, parent: str): + frappe.has_permission("BOM Creator", "write", throw=True) + + if not frappe.db.exists("BOM Creator Item", {"parent": parent, "name": docname}): + frappe.throw(_("BOM Creator Item with name {0} does not exist").format(docname)) if isinstance(data, str): data = frappe.parse_json(data) - frappe.db.set_value(doctype, docname, data) - doc = frappe.get_doc("BOM Creator", parent) + for row in doc.items: + if row.name == docname: + for key, value in data.items(): + if key in BOM_ITEM_FIELDS: + row.set(key, value) + break + doc.set_rate_for_items() doc.save() diff --git a/erpnext/manufacturing/doctype/bom_creator/test_bom_creator.py b/erpnext/manufacturing/doctype/bom_creator/test_bom_creator.py index f7e7623b471..3c280f6baae 100644 --- a/erpnext/manufacturing/doctype/bom_creator/test_bom_creator.py +++ b/erpnext/manufacturing/doctype/bom_creator/test_bom_creator.py @@ -8,6 +8,8 @@ import frappe from erpnext.manufacturing.doctype.bom_creator.bom_creator import ( add_item, add_sub_assembly, + delete_node, + edit_bom_creator, ) from erpnext.stock.doctype.item.test_item import make_item from erpnext.tests.utils import ERPNextTestSuite @@ -251,6 +253,45 @@ class TestBOMCreator(ERPNextTestSuite): data = frappe.get_all("BOM", filters={"bom_creator": doc.name, "docstatus": 1}) self.assertEqual(len(data), 2) + def test_edit_and_delete_reject_unknown_item(self): + final_product = "Bicycle" + make_item( + final_product, + { + "item_group": "Raw Material", + "stock_uom": "Nos", + }, + ) + + doc = make_bom_creator( + name="Bicycle BOM Guarded", + company="_Test Company", + item_code=final_product, + qty=1, + rm_cosy_as_per="Valuation Rate", + currency="INR", + plc_conversion_rate=1, + conversion_rate=1, + ) + + # Editing a row that does not belong to this BOM Creator must be rejected. + self.assertRaises( + frappe.ValidationError, + edit_bom_creator, + docname="non-existent-row", + data={"qty": 5}, + parent=doc.name, + ) + + # Deleting a row that does not belong to this BOM Creator must be rejected. + self.assertRaises( + frappe.ValidationError, + delete_node, + parent=doc.name, + fg_item=final_product, + docname="non-existent-row", + ) + 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 49eee62e14d..33cf6f6f574 100644 --- a/erpnext/public/js/bom_configurator/bom_configurator.bundle.js +++ b/erpnext/public/js/bom_configurator/bom_configurator.bundle.js @@ -74,6 +74,7 @@ class BOMConfigurator { onload: function (me) { me.args["parent_id"] = frm_obj.frm.doc.name; me.args["parent"] = frm_obj.frm.doc.item_code; + delete me.args["doctype"]; me.parent = frm_obj.$wrapper.get(0); me.body = frm_obj.$wrapper.get(0); me.make_tree(); @@ -507,7 +508,6 @@ class BOMConfigurator { frappe.call({ method: "erpnext.manufacturing.doctype.bom_creator.bom_creator.edit_bom_creator", args: { - doctype: doctype, docname: docname, data: data, parent: node.data.parent_id || this.frm.doc.name, @@ -540,6 +540,13 @@ class BOMConfigurator { } load_tree(response, node) { + // delete_node returns an empty response when nothing was removed; just + // refresh the node and bail out so we don't read undefined fields below. + if (!response?.message?.items) { + frappe.views.trees["BOM Configurator"].tree.load_children(node); + return; + } + let item_row = ""; let parent_dom = ""; let total_amount = response.message.raw_material_cost;