nsinnovations/erpnext
2
0
Fork 0
mirror of https://github.com/frappe/erpnext.git synced 2026-08-17 00:18:39 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #57755 from mihir-kandoi/backport/secondary-item-valuation-fixes

Browse Source 
fix(stock): correct secondary item valuation across stock entry purposes
This commit is contained in:
Mihir Kandoi
2026-08-03 17:05:54 +05:30
committed by GitHub
parent 667b012065 aaa99f775d
commit 4babce436f
4 changed files with 329 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
erpnext/controllers/stock_controller.py
View File

@@ -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

4
erpnext/public/js/controllers/transaction.js
View File

@@ -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))

90
erpnext/stock/doctype/stock_entry/stock_entry.py
View File

@@ -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

249
erpnext/stock/doctype/stock_entry/test_stock_entry.py
View File

@@ -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 (