diff --git a/erpnext/manufacturing/report/bom_stock_analysis/bom_stock_analysis.py b/erpnext/manufacturing/report/bom_stock_analysis/bom_stock_analysis.py index 51450599084..e787451e57b 100644 --- a/erpnext/manufacturing/report/bom_stock_analysis/bom_stock_analysis.py +++ b/erpnext/manufacturing/report/bom_stock_analysis/bom_stock_analysis.py @@ -239,17 +239,21 @@ def get_bom_data(filters): # bom_no + is_phantom_item drive whether/which sub-BOM explode_phantom_boms recurses into, so # they must come from the SAME BOM Item line. Aggregating each independently (Max) could pair a # bom_no from one line with is_phantom_item from another when an item_code repeats in the BOM. - # Take the first (lowest idx) line per item_code as the coherent representative. - first_line = {} + # Rows are grouped by item_code (one qty_per_unit total per component), so pick one coherent + # representative line: the first line, but upgrade to the first phantom line if any exists, so a + # phantom sub-BOM is never dropped just because a non-phantom line happens to be listed first. + representative = {} for line in frappe.get_all( "BOM Item", filters={"parent": filters.get("bom"), "parenttype": "BOM"}, fields=["item_code", "bom_no", "is_phantom_item"], order_by="idx", ): - first_line.setdefault(line.item_code, line) + existing = representative.get(line.item_code) + if existing is None or (line.is_phantom_item and not existing.is_phantom_item): + representative[line.item_code] = line for row in data: - line = first_line.get(row.item_code) + line = representative.get(row.item_code) if line: row.bom_no = line.bom_no row.is_phantom_item = line.is_phantom_item diff --git a/erpnext/manufacturing/report/bom_stock_analysis/test_bom_stock_analysis.py b/erpnext/manufacturing/report/bom_stock_analysis/test_bom_stock_analysis.py index e6dc760ff6b..592f577b936 100644 --- a/erpnext/manufacturing/report/bom_stock_analysis/test_bom_stock_analysis.py +++ b/erpnext/manufacturing/report/bom_stock_analysis/test_bom_stock_analysis.py @@ -79,16 +79,10 @@ class TestBOMStockAnalysis(ERPNextTestSuite): ) self.assertEqual(footer.get("description"), expected_min) - def test_phantom_explosion_picks_coherent_sub_bom(self): - """bom_no and is_phantom_item must come from the SAME BOM Item line. - - When a component is listed more than once in a BOM pointing at different sub-BOMs - (one phantom, one not), the report groups both lines into a single row by item_code. - Aggregating bom_no and is_phantom_item with independent Max() could pair the phantom - flag of one line with the bom_no of the other, so explode_phantom_boms recurses into - the wrong sub-BOM. We now take one coherent representative line (lowest idx), so the - phantom sub-BOM is the one exploded. - """ + def _build_duplicate_component_bom(self, phantom_first): + """Parent BOM that lists one `component` twice, once via a phantom sub-BOM and once via a + non-phantom sub-BOM. `phantom_first` controls which line is at idx 1. Returns the names of + (parent_bom, rm_phantom, rm_normal, component).""" rm_phantom = make_item(properties={"is_stock_item": 1, "valuation_rate": 10}).name rm_normal = make_item(properties={"is_stock_item": 1, "valuation_rate": 10}).name component = make_item(properties={"is_stock_item": 1, "valuation_rate": 10}).name @@ -101,10 +95,12 @@ class TestBOMStockAnalysis(ERPNextTestSuite): phantom_bom.submit() normal_bom = make_bom(item=component, raw_materials=[rm_normal]) - # Parent lists `component` twice: phantom line first (idx 1), non-phantom second (idx 2). fg_item = make_item(properties={"is_stock_item": 1, "valuation_rate": 10}).name + first_bom, second_bom = ( + (phantom_bom.name, normal_bom.name) if phantom_first else (normal_bom.name, phantom_bom.name) + ) parent = make_bom(item=fg_item, raw_materials=[component], do_not_save=True) - parent.items[0].bom_no = phantom_bom.name + parent.items[0].bom_no = first_bom component_doc = frappe.get_doc("Item", component) parent.append( "items", @@ -113,21 +109,43 @@ class TestBOMStockAnalysis(ERPNextTestSuite): "qty": 1, "uom": component_doc.stock_uom, "stock_uom": component_doc.stock_uom, - "bom_no": normal_bom.name, + "bom_no": second_bom, }, ) parent.save() parent.submit() + return parent.name, rm_phantom, rm_normal, component - raw_data = bom_stock_analysis_report(filters={"qty_to_make": 1, "bom": parent.name})[1] + def _assert_phantom_exploded(self, parent_bom, rm_phantom, rm_normal, component): + raw_data = bom_stock_analysis_report(filters={"qty_to_make": 1, "bom": parent_bom})[1] items = {row.get("item") for row in raw_data if row} - # Phantom sub-BOM exploded -> its raw material appears; the component row is replaced. self.assertIn(rm_phantom, items) self.assertNotIn(component, items) # The non-phantom line's sub-BOM must NOT be mis-exploded. self.assertNotIn(rm_normal, items) + def test_phantom_explosion_picks_coherent_sub_bom(self): + """bom_no and is_phantom_item must come from the SAME BOM Item line. + + When a component is listed more than once in a BOM pointing at different sub-BOMs + (one phantom, one not), the report groups both lines into a single row by item_code. + Aggregating bom_no and is_phantom_item with independent Max() could pair the phantom + flag of one line with the bom_no of the other, so explode_phantom_boms recurses into + the wrong sub-BOM. We now take one coherent representative line, so the phantom sub-BOM + is the one exploded. + """ + self._assert_phantom_exploded(*self._build_duplicate_component_bom(phantom_first=True)) + + def test_phantom_explosion_when_phantom_line_is_not_first(self): + """The phantom flag must win regardless of line order. + + If the non-phantom line is listed first (idx 1) and the phantom line second, a naive + first-line representative would drop the phantom flag and skip the sub-BOM explosion. + The representative is phantom-preferring, so the phantom sub-BOM is still exploded. + """ + self._assert_phantom_exploded(*self._build_duplicate_component_bom(phantom_first=False)) + def split_data_and_footer(raw_data): """Separate component rows from the footer row. Skips blank spacer rows."""