From 4ed03748fe6adeca30407d0c7ea6a45beac7330a Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Mon, 3 Aug 2026 16:52:51 +0530 Subject: [PATCH 1/2] fix(stock): correct secondary item valuation across stock entry purposes Backport of five fixes merged to develop, adapted to this branch, where the field is still named `type` and the stock entry rate logic has not been split out of set_basic_rate. - A secondary row with no BOM link is costed out of the finished good, as legacy scrap was. Finished goods are rated last so a single validate pass sees the secondary rows' amounts. (#57732) - Repack no longer flags secondary rows as finished goods, so each side takes the share the BOM declares instead of the scrap absorbing the finished good's percentage. (#57735) - A BOM allocation of 0% means the row carries no cost, rather than falling through to the item's own valuation rate. (#57736) - Secondary Item Type no longer waives a quality inspection on purposes that do not produce secondary items. (#57737) - The BOM allocation applies to the consumption entry's cost when the raw material cost comes from one. (#57738) Replaces the individual backports, which could not be cherry-picked cleanly: every hunk needed rewriting against the pre-rename field and the un-refactored rate logic. --- erpnext/controllers/stock_controller.py | 18 +++- erpnext/public/js/controllers/transaction.js | 4 +- .../stock/doctype/stock_entry/stock_entry.py | 90 +++++++++++++------ 3 files changed, 80 insertions(+), 32 deletions(-) diff --git a/erpnext/controllers/stock_controller.py b/erpnext/controllers/stock_controller.py index 7a292b078b6..f10706b2ac6 100644 --- a/erpnext/controllers/stock_controller.py +++ b/erpnext/controllers/stock_controller.py @@ -70,9 +70,23 @@ QI_OUTGOING_PURPOSES = ( ) +SECONDARY_ITEM_PURPOSES = ("Manufacture", "Repack", "Disassemble") + + +def is_inspection_exempt_secondary_row(doc, row) -> bool: + """Whether the row is a secondary item on a document that produces secondary items.""" + if not (row.get("type") or row.get("is_legacy_scrap_item")): + return False + + if doc.doctype == "Stock Entry": + return doc.purpose in SECONDARY_ITEM_PURPOSES + + return True + + def stock_entry_row_requires_inspection(purpose, row): """Check if this Stock Entry row need a Quality Inspection.""" - if row.get("type") or row.get("is_legacy_scrap_item"): + if purpose in SECONDARY_ITEM_PURPOSES and (row.get("type") or row.get("is_legacy_scrap_item")): return False if purpose == "Manufacture": return bool(row.is_finished_item) @@ -1604,7 +1618,7 @@ class StockController(AccountsController): elif self.doctype == "Stock Entry": qi_required = stock_entry_row_requires_inspection(self.purpose, row) - if row.get("type") or row.get("is_legacy_scrap_item"): + if is_inspection_exempt_secondary_row(self, row): continue if qi_required: # validate row only if inspection is required on item level diff --git a/erpnext/public/js/controllers/transaction.js b/erpnext/public/js/controllers/transaction.js index ad110712d71..8f62a139005 100644 --- a/erpnext/public/js/controllers/transaction.js +++ b/erpnext/public/js/controllers/transaction.js @@ -20,8 +20,10 @@ erpnext.stock.qi_outgoing_purposes = [ ]; erpnext.stock.is_incoming_qi_purpose = (purpose) => purpose === "Manufacture" || erpnext.stock.qi_incoming_purposes.includes(purpose); +erpnext.stock.secondary_item_purposes = ["Manufacture", "Repack", "Disassemble"]; erpnext.stock.row_requires_quality_inspection = (purpose, row) => { - if (row.type || row.is_legacy_scrap_item) return false; + if (erpnext.stock.secondary_item_purposes.includes(purpose) && (row.type || row.is_legacy_scrap_item)) + return false; if (purpose === "Manufacture") return !!row.is_finished_item; if (erpnext.stock.qi_incoming_purposes.includes(purpose)) return !!row.t_warehouse; if (erpnext.stock.qi_outgoing_purposes.includes(purpose)) diff --git a/erpnext/stock/doctype/stock_entry/stock_entry.py b/erpnext/stock/doctype/stock_entry/stock_entry.py index ad22e5c3c08..6f8a4644a56 100644 --- a/erpnext/stock/doctype/stock_entry/stock_entry.py +++ b/erpnext/stock/doctype/stock_entry/stock_entry.py @@ -83,6 +83,15 @@ from erpnext.controllers.subcontracting_inward_controller import SubcontractingI form_grid_templates = {"items": "templates/form_grid/stock_entry_grid.html"} +def is_costed_out_of_finished_item(row) -> bool: + """Whether the row takes its value out of the finished good instead of adding to it. + + A secondary item that is not linked to a BOM has no cost allocation of its own, so it is + valued the way the legacy scrap item was: its cost is deducted from the finished good. + """ + return bool(row.is_legacy_scrap_item or (row.type and not row.bom_secondary_item)) + + def _qty_tolerance(precision: int) -> float: """One unit at the column's precision -- absorbs float rounding without letting a real (whole-unit) quantity divergence slip through.""" @@ -1447,9 +1456,12 @@ class StockEntry(StockController, SubcontractingInwardController): outgoing_items_cost = self.set_rate_for_outgoing_items(reset_outgoing_rate, raise_error_if_no_rate) has_consumption_basis = self.has_consumption_basis() + secondary_items_cost_basis = self.get_secondary_items_cost_basis(outgoing_items_cost) + items = [] # Set basic rate for incoming items - for d in self.get("items"): + finished_items_last = sorted(self.get("items"), key=lambda row: cint(row.is_finished_item)) + for d in finished_items_last: if d.s_warehouse or d.set_basic_rate_manually: continue @@ -1459,7 +1471,7 @@ class StockEntry(StockController, SubcontractingInwardController): d.basic_amount = 0.0 continue - rate_derived_from_consumption = False + has_derived_rate = False if d.allow_zero_valuation_rate and d.basic_rate and self.purpose != "Receive from Customer": d.basic_rate = 0.0 @@ -1469,26 +1481,25 @@ class StockEntry(StockController, SubcontractingInwardController): d.basic_rate = self.get_basic_rate_for_manufactured_item( d.transfer_qty, outgoing_items_cost, has_consumption_basis ) - rate_derived_from_consumption = has_consumption_basis + has_derived_rate = has_consumption_basis elif self.purpose == "Repack": d.basic_rate = self.get_basic_rate_for_repacked_items(d.transfer_qty, outgoing_items_cost) # Repack rate comes from consumed source-warehouse rows, not consumption entries - rate_derived_from_consumption = any(item.s_warehouse for item in self.get("items")) + has_derived_rate = any(item.s_warehouse for item in self.get("items")) if self.bom_no: d.basic_rate *= frappe.get_value("BOM", self.bom_no, "cost_allocation_per") / 100 elif d.type and d.bom_secondary_item: - cost_allocation_per = frappe.get_value( - "BOM Secondary Item", d.bom_secondary_item, "cost_allocation_per" + cost_allocation_per = flt( + frappe.get_value("BOM Secondary Item", d.bom_secondary_item, "cost_allocation_per") ) - # Only recalculate when cost is actually allocated; otherwise preserve the - # user-entered rate (or fall through to get_valuation_rate below) - if cost_allocation_per and flt(d.transfer_qty): - d.basic_rate = (outgoing_items_cost * (cost_allocation_per / 100)) / d.transfer_qty + if flt(d.transfer_qty): + d.basic_rate = (secondary_items_cost_basis * (cost_allocation_per / 100)) / d.transfer_qty + has_derived_rate = True - # A rate of zero derived from the consumed items is their actual cost, not a missing - # rate. Falling back to the item's valuation here would value free inputs as output. - if not d.basic_rate and not d.allow_zero_valuation_rate and not rate_derived_from_consumption: + # A rate of zero that was derived rather than left unset is a real cost. Falling back to + # the item's valuation here would value free inputs, or an unallocated row, as output. + if not d.basic_rate and not d.allow_zero_valuation_rate and not has_derived_rate: if self.is_new(): raise_error_if_no_rate = False @@ -1601,11 +1612,43 @@ class StockEntry(StockController, SubcontractingInwardController): ) return flt(outgoing_items_cost / total_fg_qty) + def get_secondary_items_cost_basis(self, outgoing_items_cost) -> float: + """The cost a BOM allocation splits: the consumed rows, or the entry that replaced them.""" + if outgoing_items_cost or self.purpose != "Manufacture" or not self.work_order: + return outgoing_items_cost + + settings = frappe.get_single("Manufacturing Settings") + if not (settings.material_consumption and settings.get_rm_cost_from_consumption_entry): + return outgoing_items_cost + + if not self.get_consumption_entries(): + return outgoing_items_cost + + return self._fetch_consumption_entry_cost() + + def _fetch_consumption_entry_cost(self): + SE = frappe.qb.DocType("Stock Entry") + SE_ITEM = frappe.qb.DocType("Stock Entry Detail") + + return ( + frappe.qb.from_(SE) + .left_join(SE_ITEM) + .on(SE.name == SE_ITEM.parent) + .select(Sum(SE_ITEM.valuation_rate * SE_ITEM.transfer_qty)) + .where( + (SE.docstatus == 1) + & (SE.work_order == self.work_order) + & (SE.purpose == "Material Consumption for Manufacture") + ) + ).run()[0][0] or 0 + def get_basic_rate_for_manufactured_item( self, finished_item_qty, outgoing_items_cost=0, has_consumption_basis=False ) -> float: settings = frappe.get_single("Manufacturing Settings") - scrap_items_cost = sum([flt(d.basic_amount) for d in self.get("items") if d.is_legacy_scrap_item]) + scrap_items_cost = sum( + [flt(d.basic_amount) for d in self.get("items") if is_costed_out_of_finished_item(d)] + ) if settings.material_consumption: if settings.get_rm_cost_from_consumption_entry and self.work_order: @@ -1642,20 +1685,7 @@ class StockEntry(StockController, SubcontractingInwardController): ) ) - SE = frappe.qb.DocType("Stock Entry") - SE_ITEM = frappe.qb.DocType("Stock Entry Detail") - - outgoing_items_cost = ( - frappe.qb.from_(SE) - .left_join(SE_ITEM) - .on(SE.name == SE_ITEM.parent) - .select(Sum(SE_ITEM.valuation_rate * SE_ITEM.transfer_qty)) - .where( - (SE.docstatus == 1) - & (SE.work_order == self.work_order) - & (SE.purpose == "Material Consumption for Manufacture") - ) - ).run()[0][0] or 0 + outgoing_items_cost = self._fetch_consumption_entry_cost() # Estimate from the BOM only when nothing was consumed. A consumed cost of zero is a # real cost, so substituting BOM rates would value free inputs as output. @@ -2031,7 +2061,9 @@ class StockEntry(StockController, SubcontractingInwardController): for d in self.items: if d.t_warehouse and not d.s_warehouse: - if self.purpose == "Repack" or d.item_code == finished_item: + if d.type or d.is_legacy_scrap_item: + d.is_finished_item = 0 + elif self.purpose == "Repack" or d.item_code == finished_item: d.is_finished_item = 1 else: d.is_finished_item = 0 From aaa99f775db3ea2dbf4e890ab2ea3c4d2700b4ea Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Mon, 3 Aug 2026 16:53:20 +0530 Subject: [PATCH 2/2] test(stock): cover secondary item valuation across stock entry purposes Ports the five regression tests to this branch's `type` field name. --- .../doctype/stock_entry/test_stock_entry.py | 249 ++++++++++++++++++ 1 file changed, 249 insertions(+) diff --git a/erpnext/stock/doctype/stock_entry/test_stock_entry.py b/erpnext/stock/doctype/stock_entry/test_stock_entry.py index d91ec89d7dd..ff64b88c26d 100644 --- a/erpnext/stock/doctype/stock_entry/test_stock_entry.py +++ b/erpnext/stock/doctype/stock_entry/test_stock_entry.py @@ -7,6 +7,7 @@ from frappe.utils import add_days, cstr, flt, get_time, getdate, nowtime, today from erpnext.accounts.doctype.account.test_account import get_inventory_account from erpnext.controllers.accounts_controller import InvalidQtyError +from erpnext.exceptions import QualityInspectionRequiredError from erpnext.stock.doctype.item.test_item import ( create_item, make_item, @@ -2737,6 +2738,254 @@ class TestStockEntry(ERPNextTestSuite): self.assertEqual(fg_sle.incoming_rate, 0) self.assertEqual(fg_sle.stock_value_difference, 0) + def test_manufacture_balances_secondary_item_added_without_a_bom(self): + """A secondary item with no BOM link is costed out of the finished good, as legacy scrap was.""" + rm_item = make_item(properties={"is_stock_item": 1}).name + fg_item = make_item(properties={"is_stock_item": 1}).name + scrap_item = make_item(properties={"is_stock_item": 1, "valuation_rate": 20}).name + warehouse = "_Test Warehouse - _TC" + + make_stock_entry(item_code=rm_item, target=warehouse, qty=10, basic_rate=100) + + se = frappe.new_doc("Stock Entry") + se.purpose = se.stock_entry_type = "Manufacture" + se.company = "_Test Company" + se.append( + "items", {"item_code": rm_item, "s_warehouse": warehouse, "qty": 10, "conversion_factor": 1} + ) + se.append( + "items", + { + "item_code": fg_item, + "t_warehouse": warehouse, + "qty": 10, + "is_finished_item": 1, + "conversion_factor": 1, + }, + ) + se.append( + "items", + { + "item_code": scrap_item, + "t_warehouse": warehouse, + "qty": 5, + "type": "Scrap", + "conversion_factor": 1, + }, + ) + se.save() + + scrap_row = se.items[2] + self.assertEqual(flt(scrap_row.basic_rate), 20.0) + self.assertEqual(flt(scrap_row.basic_amount), 100.0) + + fg_row = se.items[1] + self.assertEqual(flt(fg_row.basic_rate), 90.0) + self.assertEqual(flt(fg_row.basic_amount), 900.0) + + self.assertEqual(flt(se.total_incoming_value), 1000.0) + self.assertEqual(flt(se.total_outgoing_value), 1000.0) + self.assertEqual(flt(se.value_difference), 0.0) + + def test_repack_allocates_cost_to_secondary_item(self): + """A Repack secondary item takes its own BOM share, not the finished good's.""" + rm_item = make_item(properties={"is_stock_item": 1, "valuation_rate": 100}).name + fg_item = make_item(properties={"is_stock_item": 1}).name + scrap_item = make_item(properties={"is_stock_item": 1, "valuation_rate": 20}).name + warehouse = "_Test Warehouse - _TC" + + bom = frappe.get_doc( + { + "doctype": "BOM", + "item": fg_item, + "currency": "INR", + "quantity": 10, + "company": "_Test Company", + } + ) + bom.append("items", {"item_code": rm_item, "qty": 10}) + bom.append( + "secondary_items", + { + "type": "Scrap", + "item_code": scrap_item, + "item_name": scrap_item, + "qty": 5, + "cost_allocation_per": 25, + "process_loss_per": 0, + }, + ) + bom.insert() + bom.submit() + self.assertEqual(flt(bom.cost_allocation_per), 75.0) + + make_stock_entry(item_code=rm_item, target=warehouse, qty=100, basic_rate=100) + + se = frappe.new_doc("Stock Entry") + se.purpose = se.stock_entry_type = "Repack" + se.company = "_Test Company" + se.from_bom = 1 + se.bom_no = bom.name + se.fg_completed_qty = 10 + se.from_warehouse = warehouse + se.to_warehouse = warehouse + se.get_items() + se.save() + + fg_row = next(d for d in se.items if d.is_finished_item) + scrap_row = next(d for d in se.items if d.type) + + self.assertFalse(scrap_row.is_finished_item) + self.assertEqual(flt(scrap_row.basic_amount), 250.0) + self.assertEqual(flt(fg_row.basic_amount), 750.0) + + self.assertEqual(flt(se.total_incoming_value), 1000.0) + self.assertEqual(flt(se.total_outgoing_value), 1000.0) + self.assertEqual(flt(se.value_difference), 0.0) + + def test_secondary_item_with_zero_cost_allocation_carries_no_value(self): + """A BOM that allocates 0% to a secondary item gives the finished good everything.""" + from erpnext.manufacturing.doctype.work_order.test_work_order import make_wo_order_test_record + from erpnext.manufacturing.doctype.work_order.work_order import ( + make_stock_entry as make_stock_entry_from_wo, + ) + + rm_item = make_item(properties={"is_stock_item": 1, "valuation_rate": 100}).name + fg_item = make_item(properties={"is_stock_item": 1}).name + scrap_item = make_item(properties={"is_stock_item": 1, "valuation_rate": 20}).name + warehouse = "_Test Warehouse - _TC" + + bom = frappe.get_doc( + { + "doctype": "BOM", + "item": fg_item, + "currency": "INR", + "quantity": 10, + "company": "_Test Company", + } + ) + bom.append("items", {"item_code": rm_item, "qty": 10}) + bom.append( + "secondary_items", + { + "type": "Scrap", + "item_code": scrap_item, + "item_name": scrap_item, + "qty": 5, + "cost_allocation_per": 0, + "process_loss_per": 0, + }, + ) + bom.insert() + bom.submit() + self.assertEqual(flt(bom.cost_allocation_per), 100.0) + + make_stock_entry(item_code=rm_item, target=warehouse, qty=100, basic_rate=100) + wo = make_wo_order_test_record( + production_item=fg_item, bom_no=bom.name, qty=10, skip_transfer=1, source_warehouse=warehouse + ) + + se = frappe.get_doc(make_stock_entry_from_wo(wo.name, "Manufacture", 10)) + se.save() + + scrap_row = next(d for d in se.items if d.type) + fg_row = next(d for d in se.items if d.is_finished_item) + + self.assertEqual(flt(scrap_row.basic_rate), 0.0) + self.assertEqual(flt(scrap_row.basic_amount), 0.0) + self.assertEqual(flt(fg_row.basic_amount), 1000.0) + self.assertEqual(flt(se.value_difference), 0.0) + + def test_secondary_item_type_does_not_waive_inspection_outside_manufacturing(self): + """A stray secondary item type must not let a QI-required item through a receipt.""" + item = make_item( + properties={ + "is_stock_item": 1, + "valuation_rate": 50, + "inspection_required_before_purchase": 1, + } + ).name + + def receipt(secondary_item_type): + se = frappe.new_doc("Stock Entry") + se.purpose = se.stock_entry_type = "Material Receipt" + se.company = "_Test Company" + se.inspection_required = 1 + se.append( + "items", + { + "item_code": item, + "t_warehouse": "_Test Warehouse - _TC", + "qty": 10, + "conversion_factor": 1, + "type": secondary_item_type, + }, + ) + return se + + self.assertRaises(QualityInspectionRequiredError, receipt("").submit) + self.assertRaises(QualityInspectionRequiredError, receipt("Scrap").submit) + + @ERPNextTestSuite.change_settings( + "Manufacturing Settings", {"material_consumption": 1, "get_rm_cost_from_consumption_entry": 1} + ) + def test_secondary_item_allocation_uses_consumption_entry_cost(self): + """A BOM allocation splits the consumption entry's cost, not an empty set of consumed rows.""" + from erpnext.manufacturing.doctype.work_order.test_work_order import make_wo_order_test_record + from erpnext.manufacturing.doctype.work_order.work_order import ( + make_stock_entry as make_stock_entry_from_wo, + ) + + rm_item = make_item(properties={"is_stock_item": 1, "valuation_rate": 100}).name + fg_item = make_item(properties={"is_stock_item": 1}).name + scrap_item = make_item(properties={"is_stock_item": 1, "valuation_rate": 20}).name + warehouse = "_Test Warehouse - _TC" + + bom = frappe.get_doc( + { + "doctype": "BOM", + "item": fg_item, + "currency": "INR", + "quantity": 10, + "company": "_Test Company", + } + ) + bom.append("items", {"item_code": rm_item, "qty": 10}) + bom.append( + "secondary_items", + { + "type": "Scrap", + "item_code": scrap_item, + "item_name": scrap_item, + "qty": 5, + "cost_allocation_per": 25, + "process_loss_per": 0, + }, + ) + bom.insert() + bom.submit() + + make_stock_entry(item_code=rm_item, target=warehouse, qty=100, basic_rate=100) + wo = make_wo_order_test_record( + production_item=fg_item, bom_no=bom.name, qty=10, skip_transfer=1, source_warehouse=warehouse + ) + + consumption = frappe.get_doc( + make_stock_entry_from_wo(wo.name, "Material Consumption for Manufacture", 10) + ) + consumption.submit() + self.assertEqual(flt(consumption.total_outgoing_value), 1000.0) + + se = frappe.get_doc(make_stock_entry_from_wo(wo.name, "Manufacture", 10)) + se.save() + + scrap_row = next(d for d in se.items if d.type) + fg_row = next(d for d in se.items if d.is_finished_item) + + self.assertEqual(flt(fg_row.basic_amount), 750.0) + self.assertEqual(flt(scrap_row.basic_amount), 250.0) + self.assertEqual(flt(se.total_incoming_value), 1000.0) + def _make_wo_for_free_raw_material(self, rm_item, fg_item, bom_no): from erpnext.manufacturing.doctype.work_order.test_work_order import make_wo_order_test_record from erpnext.manufacturing.doctype.work_order.work_order import (