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

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

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

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(
+				cost_allocation_per = flt(
-					"BOM Secondary Item", d.bom_secondary_item, "cost_allocation_per"
+					frappe.get_value("BOM Secondary Item", d.bom_secondary_item, "cost_allocation_per")
 				)
-				# Only recalculate when cost is actually allocated; otherwise preserve the
+				if flt(d.transfer_qty):
-				# user-entered rate (or fall through to get_valuation_rate below)
+					d.basic_rate = (secondary_items_cost_basis * (cost_allocation_per / 100)) / d.transfer_qty
-				if cost_allocation_per and flt(d.transfer_qty):
+					has_derived_rate = True
-					d.basic_rate = (outgoing_items_cost * (cost_allocation_per / 100)) / d.transfer_qty
 
-			# A rate of zero derived from the consumed items is their actual cost, not a missing
+			# A rate of zero that was derived rather than left unset is a real cost. Falling back to
-			# rate. Falling back to the item's valuation here would value free inputs as output.
+			# 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 rate_derived_from_consumption:
+			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")
+					outgoing_items_cost = self._fetch_consumption_entry_cost()
-					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
 
 			# 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