From 339bb0b4ea20958c642a26c304e0f609e90746a3 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Tue, 28 Jul 2026 11:25:29 +0530 Subject: [PATCH] fix(manufacturing): scope BOM Creator tree children to the parent row The BOM Creator tree identified a node by the parent's item code (fg_item) instead of the specific BOM Creator Item row, so every occurrence of a repeated sub-assembly shared one child set: expanding any one of them listed the raw materials of all of them, and deleting one wiped the raw materials of its siblings. Key the tree on fg_reference_id and make the node value the row name, matching the framework convention that a tree node's value is its docname. Item code now travels as its own field for the label and for the fg_item argument sent back on add/convert. Fixes #57311 (cherry picked from commit b37152752ffe10bb510ee90d2ec4697034c54543) --- .../doctype/bom_creator/bom_creator.py | 29 +++++--- .../doctype/bom_creator/test_bom_creator.py | 69 +++++++++++++++++++ .../bom_configurator.bundle.js | 32 +++++++-- 3 files changed, 112 insertions(+), 18 deletions(-) diff --git a/erpnext/manufacturing/doctype/bom_creator/bom_creator.py b/erpnext/manufacturing/doctype/bom_creator/bom_creator.py index 16198e653c6..f589ec1acc4 100644 --- a/erpnext/manufacturing/doctype/bom_creator/bom_creator.py +++ b/erpnext/manufacturing/doctype/bom_creator/bom_creator.py @@ -537,15 +537,8 @@ class BOMCreator(Document): row.delete() 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() - if item.expandable: - self.delete_node(fg_item=item.value) + if self.delete_child_nodes(kwargs.docname or self.name): + updated = True if updated: self.set_rate_for_items() @@ -555,6 +548,19 @@ class BOMCreator(Document): return frappe._dict() + def delete_child_nodes(self, fg_reference_id: str): + deleted = False + for item in get_children(parent=fg_reference_id, parent_id=self.name): + child_row = next((row for row in self.items if row.name == item.name), None) + if child_row: + child_row.delete() + + deleted = True + if item.expandable: + self.delete_child_nodes(item.name) + + return deleted + @frappe.whitelist() def get_children(doctype: str | None = None, parent: str | None = None, **kwargs): @@ -567,7 +573,7 @@ def get_children(doctype: str | None = None, parent: str | None = None, **kwargs kwargs = frappe._dict(kwargs) fields = [ - "item_code as value", + "name as value", "item_name as title", "is_expandable as expandable", "parent as parent_id", @@ -575,6 +581,7 @@ def get_children(doctype: str | None = None, parent: str | None = None, **kwargs "idx", ValueWrapper("BOM Creator Item").as_("doctype"), "name", + "item_code", "uom", "rate", "amount", @@ -583,7 +590,7 @@ def get_children(doctype: str | None = None, parent: str | None = None, **kwargs ] query_filters = { - "fg_item": parent, + "fg_reference_id": parent, "parent": kwargs.parent_id, } diff --git a/erpnext/manufacturing/doctype/bom_creator/test_bom_creator.py b/erpnext/manufacturing/doctype/bom_creator/test_bom_creator.py index 94a8b0b607b..e11f43e97e6 100644 --- a/erpnext/manufacturing/doctype/bom_creator/test_bom_creator.py +++ b/erpnext/manufacturing/doctype/bom_creator/test_bom_creator.py @@ -241,6 +241,26 @@ class TestBOMCreator(ERPNextTestSuite): data = frappe.get_all("BOM", filters={"bom_creator": doc.name, "docstatus": 1}) self.assertEqual(len(data), 2) + def test_repeated_sub_assembly_keeps_own_raw_materials(self): + doc, first_wheel, second_wheel = make_repeated_sub_assembly_bom( + "Bicycle BOM with Repeated Sub Assembly" + ) + + self.assertEqual(child_items(doc, doc.name), ["Frame Assembly", "Seat Assembly"]) + self.assertEqual(child_items(doc, first_wheel), ["Rim", "Spokes"]) + self.assertEqual(child_items(doc, second_wheel), ["Hub"]) + + def test_delete_repeated_sub_assembly_keeps_sibling_raw_materials(self): + doc, first_wheel, second_wheel = make_repeated_sub_assembly_bom( + "Bicycle BOM with Deleted Sub Assembly" + ) + + doc.delete_node(doctype="BOM Creator Item", docname=second_wheel) + doc.reload() + + self.assertEqual(child_items(doc, first_wheel), ["Rim", "Spokes"]) + self.assertFalse([row for row in doc.items if row.item_code == "Hub"]) + def test_edit_and_delete_reject_unknown_item(self): final_product = "Bicycle" make_item( @@ -327,6 +347,55 @@ def create_items(): ) +def make_repeated_sub_assembly_bom(name): + """Bicycle > (Frame Assembly > Wheel Assembly > Rim, Spokes), (Seat Assembly > Wheel Assembly > Hub)""" + final_product = "Bicycle" + 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, + ) + + def add_sub_assembly(fg_item, fg_reference_id, item_code, raw_materials): + doc.add_sub_assembly( + fg_item=fg_item, + fg_reference_id=fg_reference_id, + bom_item={ + "item_code": item_code, + "qty": 1, + "items": [{"item_code": item, "qty": 1} for item in raw_materials], + }, + ) + doc.reload() + + return next( + row.name + for row in doc.items + if row.item_code == item_code and row.fg_reference_id == fg_reference_id + ) + + frame = add_sub_assembly(final_product, doc.name, "Frame Assembly", ["Frame"]) + first_wheel = add_sub_assembly("Frame Assembly", frame, "Wheel Assembly", ["Rim", "Spokes"]) + + seat = add_sub_assembly(final_product, doc.name, "Seat Assembly", ["Seat"]) + second_wheel = add_sub_assembly("Seat Assembly", seat, "Wheel Assembly", ["Hub"]) + + return doc, first_wheel, second_wheel + + +def child_items(doc, parent): + from erpnext.manufacturing.doctype.bom_creator.bom_creator import get_children + + return sorted(row.item_code for row in get_children(parent=parent, parent_id=doc.name)) + + def make_bom_creator(**kwargs): if isinstance(kwargs, str) or isinstance(kwargs, dict): kwargs = frappe.parse_json(kwargs) diff --git a/erpnext/public/js/bom_configurator/bom_configurator.bundle.js b/erpnext/public/js/bom_configurator/bom_configurator.bundle.js index 07871687006..297f6be4ba3 100644 --- a/erpnext/public/js/bom_configurator/bom_configurator.bundle.js +++ b/erpnext/public/js/bom_configurator/bom_configurator.bundle.js @@ -57,6 +57,7 @@ class BOMConfigurator { breadcrumb: "Manufacturing", get_tree_nodes: "erpnext.manufacturing.doctype.bom_creator.bom_creator.get_children", root_label: this.frm.doc.item_code, + get_label: (node) => this.get_node_label(node), disable_add_node: true, get_tree_root: false, show_expand_all: false, @@ -66,6 +67,23 @@ class BOMConfigurator { }; } + get_node_label(node) { + const item_code = this.get_item_code(node); + const item_name = node.data?.title || item_code; + + if (item_name === item_code) { + return frappe.utils.escape_html(item_code); + } + + return `${frappe.utils.escape_html(item_name)} (${frappe.utils.escape_html( + item_code + )})`; + } + + get_item_code(node) { + return node.data?.item_code || this.frm.doc.item_code; + } + tree_methods() { let frm_obj = this; let view = frappe.views.trees["BOM Configurator"]; @@ -73,7 +91,8 @@ class BOMConfigurator { return { onload: function (me) { me.args["parent_id"] = frm_obj.frm.doc.name; - me.args["parent"] = frm_obj.frm.doc.item_code; + me.args["parent"] = frm_obj.frm.doc.name; + me.root_value = frm_obj.frm.doc.name; me.parent = frm_obj.$wrapper.get(0); me.body = frm_obj.$wrapper.get(0); me.make_tree(); @@ -83,7 +102,7 @@ class BOMConfigurator { const uom = node.data.uom || frm_obj.frm.doc.uom; const docname = node.data.name || frm_obj.frm.doc.name; let amount = node.data.amount; - if (node.data.value === frm_obj.frm.doc.item_code) { + if (node.is_root) { amount = frm_obj.frm.doc.raw_material_cost; } @@ -243,7 +262,7 @@ class BOMConfigurator { method: "add_item", doc: this.frm.doc, args: { - fg_item: node.data.value, + fg_item: this.get_item_code(node), item_code: data.item_code, fg_reference_id: node.data.name || this.frm.doc.name, qty: data.qty, @@ -298,7 +317,7 @@ class BOMConfigurator { method: "add_sub_assembly", doc: this.frm.doc, args: { - fg_item: node.data.value, + fg_item: this.get_item_code(node), fg_reference_id: node.data.name || this.frm.doc.name, bom_item: bom_item, operation: node.data.operation, @@ -417,7 +436,7 @@ class BOMConfigurator { }); dialog.set_values({ - item_code: node.data.value, + item_code: this.get_item_code(node), qty: node.data.qty, }); @@ -445,7 +464,7 @@ class BOMConfigurator { method: "add_sub_assembly", doc: this.frm.doc, args: { - fg_item: node.data.value, + fg_item: this.get_item_code(node), bom_item: bom_item, fg_reference_id: node.data.name || this.frm.doc.name, convert_to_sub_assembly: true, @@ -482,7 +501,6 @@ class BOMConfigurator { method: "delete_node", doc: this.frm.doc, args: { - fg_item: node.data.value, doctype: node.data.doctype, docname: node.data.name, },