Merge pull request #31175 from ankush/recursive_non_explosive_bom
fix: allow recursive BOMs as long as they don't recursively explode
This commit is contained in:
commit
2cbc665d19
@ -93,6 +93,11 @@ frappe.ui.form.on("BOM", {
|
||||
});
|
||||
}
|
||||
|
||||
frm.add_custom_button(__("New Version"), function() {
|
||||
let new_bom = frappe.model.copy_doc(frm.doc);
|
||||
frappe.set_route("Form", "BOM", new_bom.name);
|
||||
});
|
||||
|
||||
if(frm.doc.docstatus==1) {
|
||||
frm.add_custom_button(__("Work Order"), function() {
|
||||
frm.trigger("make_work_order");
|
||||
|
@ -22,6 +22,10 @@ from erpnext.stock.get_item_details import get_conversion_factor, get_price_list
|
||||
form_grid_templates = {"items": "templates/form_grid/item_grid.html"}
|
||||
|
||||
|
||||
class BOMRecursionError(frappe.ValidationError):
|
||||
pass
|
||||
|
||||
|
||||
class BOMTree:
|
||||
"""Full tree representation of a BOM"""
|
||||
|
||||
@ -251,9 +255,8 @@ class BOM(WebsiteGenerator):
|
||||
for item in self.get("items"):
|
||||
self.validate_bom_currency(item)
|
||||
|
||||
item.bom_no = ""
|
||||
if not item.do_not_explode:
|
||||
item.bom_no = item.bom_no
|
||||
if item.do_not_explode:
|
||||
item.bom_no = ""
|
||||
|
||||
ret = self.get_bom_material_detail(
|
||||
{
|
||||
@ -555,35 +558,27 @@ class BOM(WebsiteGenerator):
|
||||
"""Check whether recursion occurs in any bom"""
|
||||
|
||||
def _throw_error(bom_name):
|
||||
frappe.throw(_("BOM recursion: {0} cannot be parent or child of {0}").format(bom_name))
|
||||
frappe.throw(
|
||||
_("BOM recursion: {1} cannot be parent or child of {0}").format(self.name, bom_name),
|
||||
exc=BOMRecursionError,
|
||||
)
|
||||
|
||||
bom_list = self.traverse_tree()
|
||||
child_items = (
|
||||
frappe.get_all(
|
||||
"BOM Item",
|
||||
fields=["bom_no", "item_code"],
|
||||
filters={"parent": ("in", bom_list), "parenttype": "BOM"},
|
||||
)
|
||||
or []
|
||||
child_items = frappe.get_all(
|
||||
"BOM Item",
|
||||
fields=["bom_no", "item_code"],
|
||||
filters={"parent": ("in", bom_list), "parenttype": "BOM"},
|
||||
)
|
||||
|
||||
child_bom = {d.bom_no for d in child_items}
|
||||
child_items_codes = {d.item_code for d in child_items}
|
||||
for item in child_items:
|
||||
if self.name == item.bom_no:
|
||||
_throw_error(self.name)
|
||||
if self.item == item.item_code and item.bom_no:
|
||||
# Same item but with different BOM should not be allowed.
|
||||
# Same item can appear recursively once as long as it doesn't have BOM.
|
||||
_throw_error(item.bom_no)
|
||||
|
||||
if self.name in child_bom:
|
||||
_throw_error(self.name)
|
||||
|
||||
if self.item in child_items_codes:
|
||||
_throw_error(self.item)
|
||||
|
||||
bom_nos = (
|
||||
frappe.get_all(
|
||||
"BOM Item", fields=["parent"], filters={"bom_no": self.name, "parenttype": "BOM"}
|
||||
)
|
||||
or []
|
||||
)
|
||||
|
||||
if self.name in {d.parent for d in bom_nos}:
|
||||
if self.name in {d.bom_no for d in self.items}:
|
||||
_throw_error(self.name)
|
||||
|
||||
def traverse_tree(self, bom_list=None):
|
||||
|
@ -10,7 +10,7 @@ from frappe.tests.utils import FrappeTestCase
|
||||
from frappe.utils import cstr, flt
|
||||
|
||||
from erpnext.buying.doctype.purchase_order.test_purchase_order import create_purchase_order
|
||||
from erpnext.manufacturing.doctype.bom.bom import item_query, make_variant_bom
|
||||
from erpnext.manufacturing.doctype.bom.bom import BOMRecursionError, item_query, make_variant_bom
|
||||
from erpnext.manufacturing.doctype.bom_update_tool.bom_update_tool import update_cost
|
||||
from erpnext.stock.doctype.item.test_item import make_item
|
||||
from erpnext.stock.doctype.stock_reconciliation.test_stock_reconciliation import (
|
||||
@ -324,43 +324,36 @@ class TestBOM(FrappeTestCase):
|
||||
|
||||
def test_bom_recursion_1st_level(self):
|
||||
"""BOM should not allow BOM item again in child"""
|
||||
item_code = "_Test BOM Recursion"
|
||||
make_item(item_code, {"is_stock_item": 1})
|
||||
item_code = make_item(properties={"is_stock_item": 1}).name
|
||||
|
||||
bom = frappe.new_doc("BOM")
|
||||
bom.item = item_code
|
||||
bom.append("items", frappe._dict(item_code=item_code))
|
||||
with self.assertRaises(frappe.ValidationError) as err:
|
||||
bom.save()
|
||||
with self.assertRaises(BOMRecursionError):
|
||||
bom.items[0].bom_no = bom.name
|
||||
bom.save()
|
||||
|
||||
self.assertTrue("recursion" in str(err.exception).lower())
|
||||
frappe.delete_doc("BOM", bom.name, ignore_missing=True)
|
||||
|
||||
def test_bom_recursion_transitive(self):
|
||||
item1 = "_Test BOM Recursion"
|
||||
item2 = "_Test BOM Recursion 2"
|
||||
make_item(item1, {"is_stock_item": 1})
|
||||
make_item(item2, {"is_stock_item": 1})
|
||||
item1 = make_item(properties={"is_stock_item": 1}).name
|
||||
item2 = make_item(properties={"is_stock_item": 1}).name
|
||||
|
||||
bom1 = frappe.new_doc("BOM")
|
||||
bom1.item = item1
|
||||
bom1.append("items", frappe._dict(item_code=item2))
|
||||
bom1.save()
|
||||
bom1.submit()
|
||||
|
||||
bom2 = frappe.new_doc("BOM")
|
||||
bom2.item = item2
|
||||
bom2.append("items", frappe._dict(item_code=item1))
|
||||
bom2.save()
|
||||
|
||||
with self.assertRaises(frappe.ValidationError) as err:
|
||||
bom2.items[0].bom_no = bom1.name
|
||||
bom1.items[0].bom_no = bom2.name
|
||||
|
||||
with self.assertRaises(BOMRecursionError):
|
||||
bom1.save()
|
||||
bom2.save()
|
||||
bom2.submit()
|
||||
|
||||
self.assertTrue("recursion" in str(err.exception).lower())
|
||||
|
||||
bom1.cancel()
|
||||
frappe.delete_doc("BOM", bom1.name, ignore_missing=True, force=True)
|
||||
frappe.delete_doc("BOM", bom2.name, ignore_missing=True, force=True)
|
||||
|
||||
def test_bom_with_process_loss_item(self):
|
||||
fg_item_non_whole, fg_item_whole, bom_item = create_process_loss_bom_items()
|
||||
|
Loading…
Reference in New Issue
Block a user