test(stock): cover inspection on a receipt row typed as a secondary item

The row must be blocked with or without the type set.

(cherry picked from commit fec5dae639)

erpnext/accounts/doctype/journal_entry/journal_entry.js

@@ -567,7 +567,7 @@ $.extend(erpnext.journal_entry, {
 	lock_reversal_entry: function (frm) {
 		frm.fields
 			.filter((field) => field.has_input)
-			.filter((field) => !["posting_date", "custom_remark", "remark"].includes(field.df.fieldname))
+			.filter((field) => field.df.fieldname != "posting_date")
 			.forEach((field) => frm.set_df_property(field.df.fieldname, "read_only", 1));
 		frm.set_df_property("accounts", "read_only", 1);
 	},

erpnext/accounts/doctype/pos_profile/pos_profile.py

@@ -234,18 +234,15 @@ def get_item_groups(pos_profile):
 		for data in pos_profile.get("item_groups"):
 			item_groups.extend(
 				[
-					d.name
+					"%s" % frappe.db.escape(d.name)
 					for d in get_child_nodes("Item Group", data.item_group)
 					if not permitted_item_groups or d.name in permitted_item_groups
 				]
 			)
 
 	if not item_groups and permitted_item_groups:
-		item_groups = list(permitted_item_groups)
+		item_groups = ["%s" % frappe.db.escape(d) for d in permitted_item_groups]
 
-	# Return raw Item Group names; the callers parameterize them via the query builder
-	# (item_group.isin(...)) / frappe.get_all, which escapes them once. Pre-escaping here would
-	# double-escape (item_group IN ('''X''')) and match nothing.
 	return list(set(item_groups))
 
 

erpnext/accounts/doctype/repost_accounting_ledger/repost_accounting_ledger.py

@@ -467,24 +467,19 @@ def get_child_docs(doc: list) -> list:
 
 
 def validate_docs_for_deferred_accounting(sales_docs, purchase_docs):
-	docs_with_deferred_revenue = ()
-	docs_with_deferred_expense = ()
+	docs_with_deferred_revenue = frappe.db.get_all(
+		"Sales Invoice Item",
+		filters={"parent": ["in", sales_docs], "docstatus": 1, "enable_deferred_revenue": True},
+		fields=["parent"],
+		as_list=1,
+	)
 
-	if sales_docs:
-		docs_with_deferred_revenue = frappe.db.get_all(
-			"Sales Invoice Item",
-			filters={"parent": ["in", sales_docs], "docstatus": 1, "enable_deferred_revenue": True},
-			fields=["parent"],
-			as_list=1,
-		)
-
-	if purchase_docs:
-		docs_with_deferred_expense = frappe.db.get_all(
-			"Purchase Invoice Item",
-			filters={"parent": ["in", purchase_docs], "docstatus": 1, "enable_deferred_expense": 1},
-			fields=["parent"],
-			as_list=1,
-		)
+	docs_with_deferred_expense = frappe.db.get_all(
+		"Purchase Invoice Item",
+		filters={"parent": ["in", purchase_docs], "docstatus": 1, "enable_deferred_expense": 1},
+		fields=["parent"],
+		as_list=1,
+	)
 
 	if docs_with_deferred_revenue or docs_with_deferred_expense:
 		frappe.throw(

erpnext/controllers/stock_controller.py

@@ -70,23 +70,9 @@ 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 purpose in SECONDARY_ITEM_PURPOSES and (row.get("type") or row.get("is_legacy_scrap_item")):
+	if row.get("type") or row.get("is_legacy_scrap_item"):
 		return False
 	if purpose == "Manufacture":
 		return bool(row.is_finished_item)
@@ -1618,7 +1604,7 @@ class StockController(AccountsController):
 			elif self.doctype == "Stock Entry":
 				qi_required = stock_entry_row_requires_inspection(self.purpose, row)
 
-			if is_inspection_exempt_secondary_row(self, row):
+			if row.get("type") or row.get("is_legacy_scrap_item"):
 				continue
 
 			if qi_required:  # validate row only if inspection is required on item level

erpnext/manufacturing/doctype/plant_floor/stock_summary_template.html

@@ -2,9 +2,9 @@
 <div class="row" style="border-bottom:1px solid var(--border-color); padding:4px 5px; margin-top: 3px;margin-bottom: 3px;">
 	<div class="col-sm-1">
 		{% if(row.image) { %}
-			<img style="width:50px;height:50px;" src="{{frappe.utils.escape_html(row.image)}}">
+			<img style="width:50px;height:50px;" src="{{row.image}}">
 		{% } else { %}
-			<div style="width:50px;height:50px;background-color:var(--control-bg);text-align:center;padding-top:15px">{{frappe.get_abbr(frappe.utils.escape_html(row.item_code), 2)}}</div>
+			<div style="width:50px;height:50px;background-color:var(--control-bg);text-align:center;padding-top:15px">{{frappe.get_abbr(row.item_code, 2)}}</div>
 		{% } %}
 	</div>
 	<div class="col-sm-3">
@@ -13,7 +13,7 @@
 		{% } else { %}
 			{{row.item_link}}
 			<p>
-				{{frappe.utils.escape_html(row.item_name)}}
+				{{row.item_name}}
 			</p>
 		{% } %}
 
@@ -52,10 +52,10 @@
 		</span>
 	</div>
 	<div class="col-sm-1">
-		<button style="margin-left: 7px;" class="btn btn-default btn-xs btn-add" data-item-code="{{ frappe.utils.escape_html(row.item_code) }}">{{ __("Add") }}</button>
+		<button style="margin-left: 7px;" class="btn btn-default btn-xs btn-add" data-item-code="{{ escape(row.item_code) }}">{{ __("Add") }}</button>
 	</div>
 	<div class="col-sm-1">
-		<button style="margin-left: 7px;" class="btn btn-default btn-xs btn-move" data-item-code="{{ frappe.utils.escape_html(row.item_code) }}">{{ __("Move") }}</button>
+		<button style="margin-left: 7px;" class="btn btn-default btn-xs btn-move" data-item-code="{{ escape(row.item_code) }}">{{ __("Move") }}</button>
 	</div>
 </div>
 {% }); %}

erpnext/manufacturing/doctype/workstation/workstation.py

@@ -513,7 +513,7 @@ def get_workstations(**kwargs):
 		d.color = color_map.get(d.status, "red")
 		d.workstation_link = get_url_to_form("Workstation", d.name)
 		if d.status != "Production":
-			d.status_image = frappe.utils.escape_html(d.off_status_image)
+			d.status_image = d.off_status_image
 			d.workstation_off = "workstation-off"
 
 	return data

erpnext/public/js/controllers/transaction.js

@@ -18,12 +18,19 @@ erpnext.stock.qi_outgoing_purposes = [
 	"Subcontracting Delivery",
 	"Disassemble",
 ];
+erpnext.stock.secondary_item_purposes = ["Manufacture", "Repack", "Disassemble"];
 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 (erpnext.stock.secondary_item_purposes.includes(purpose) && (row.type || row.is_legacy_scrap_item))
+<<<<<<< HEAD
+	if (row.type || row.is_legacy_scrap_item) return false;
+=======
+	if (
+		erpnext.stock.secondary_item_purposes.includes(purpose) &&
+		(row.secondary_item_type || row.is_legacy_scrap_item)
+	)
 		return false;
+>>>>>>> dfec7bd5c7 (fix(stock): stop a secondary item type from waiving quality inspection)
 	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/public/js/plant_floor_visual/visual_plant.js

@@ -168,7 +168,7 @@ class VisualPlantFloor {
 				.find(".workstation-image-container")
 				.append(
 					`<div class="workstation-image-cls workstation-abbr" style="margin:6px; height:82px">${frappe.get_abbr(
-						frappe.utils.escape_html(data.name),
+						data.name,
 						2
 					)}</div>`
 				);

erpnext/public/js/templates/item_selector.html

@@ -1,19 +1,17 @@
 <div class="app-listing item-list image-view-container item-selector">
 {% for (var i=0; i < data.length; i++) { var item = data[i]; %}
-	{% const item_name = frappe.utils.escape_html(item.name); %}
-	{% const item_title = frappe.utils.escape_html(item.item_name || item.name); %}
 	{% if (i % 4 === 0) { %}<div class="image-view-row">{% } %}
-	<div class="image-view-item" data-name="{{ item_name }}">
+	<div class="image-view-item" data-name="{{ item.name }}">
 		<div class="image-view-header doclist-row">
 			<div class="list-value">
-				<a class="grey list-id" data-name="{{ item_name }}"
-					title="{{ item_title }}">
-					{{ item_title }}</a>
+				<a class="grey list-id" data-name="{{item.name}}"
+					title="{{ item.item_name || item.name}}">
+					{{item.item_name || item.name}}</a>
 			</div>
 		</div>
 		<div class="image-view-body">
-			<a  data-item-code="{{ item_name }}"
-				title="{{ item_title }}"
+			<a  data-item-code="{{ item.name }}"
+				title="{{ item.item_name || item.name }}"
 			>
 				<div class="image-field"
 					style="
@@ -24,11 +22,11 @@
 				>
 					{% if (!item.image) { %}
 					<span class="placeholder-text">
-						{%= frappe.get_abbr(item_title) %}
+						{%= frappe.get_abbr(item.item_name || item.name) %}
 					</span>
 					{% } %}
 					{% if (item.image) { %}
-					<img src="{{ frappe.utils.escape_html(item.image) }}" alt="{{ item_title }}">
+					<img src="{{ item.image }}" alt="{{item.item_name || item.name}}">
 					{% } %}
 				</div>
 			</a>

erpnext/public/js/templates/visual_plant_floor_template.html

@@ -1,6 +1,5 @@
 {% $.each(workstations, (idx, row) => { %}
-	{% const row_workstation_name = frappe.utils.escape_html(row.name); %}
-	<div class="workstation-wrapper" data-workstation="{{row_workstation_name}}">
+	<div class="workstation-wrapper" data-workstation="{{row.name}}">
 		<div class="workstation-status text-left" style="">
 			<span class="indicator-pill no-indicator-dot whitespace-nowrap {{row.color}}" style="margin: 8px 0px 0px 8px;">
 				<span class="workstation-status-title" style="font-size:10px">{{row.status}}</span>
@@ -11,12 +10,12 @@
 				{% if(row.status_image) { %}
 					<img class="workstation-image-cls" src="{{row.status_image}}">
 				{% } else { %}
-					<div class="workstation-image-cls workstation-abbr" style="margin:6px; height:82px">{{frappe.get_abbr(row_workstation_name, 2)}}</div>
+					<div class="workstation-image-cls workstation-abbr" style="margin:6px; height:82px">{{frappe.get_abbr(row.name, 2)}}</div>
 				{% } %}
 			</div>
-			<span class="ellipsis" title="{{row_workstation_name}}">
+			<span class="ellipsis" title="{{row.name}}">
 				<div style="font-size:11px; text-align:center;padding-bottom:8px">{{row.workstation_name}}</div>
 			</span>
 		</div>
 	</div>
-{% }); %}
+{% }); %}

erpnext/stock/doctype/item/item.py

@@ -853,17 +853,7 @@ class Item(Document):
 				frappe.throw(_("Item {0} is not a template item.").format(frappe.bold(self.variant_of)))
 
 			if based_on == "Item Attribute":
-				previous_doc = self.get_doc_before_save()
-				saved_attributes = (
-					{(row.attribute, row.attribute_value) for row in previous_doc.attributes}
-					if previous_doc
-					else set()
-				)
-
 				for d in self.attributes:
-					if (d.attribute, d.attribute_value) in saved_attributes:
-						continue
-
 					if not frappe.db.exists(
 						"Item Variant Attribute", {"attribute": d.attribute, "parent": self.variant_of}
 					):

erpnext/stock/doctype/item/test_item.py

@@ -410,24 +410,6 @@ class TestItem(ERPNextTestSuite):
 
 		self.assertRaises(InvalidItemAttributeValueError, attribute.save)
 
-	def test_disabled_attribute_blocks_only_attribute_changes(self):
-		frappe.delete_doc_if_exists("Item", "_Test Variant Item-L", force=1)
-
-		variant = create_variant("_Test Variant Item", {"Test Size": "Large"})
-		variant.save()
-
-		attribute = frappe.get_doc("Item Attribute", "Test Size")
-		attribute.disabled = 1
-		attribute.save()
-
-		variant.reload()
-		variant.description = "Edited after the attribute was disabled"
-		variant.save()
-
-		variant.reload()
-		variant.attributes[0].attribute_value = "Small"
-		self.assertRaises(frappe.ValidationError, variant.save)
-
 	def test_rename_attribute_value_updates_variants(self):
 		frappe.delete_doc_if_exists("Item", "_Test Variant Item-L", force=1)
 

erpnext/stock/doctype/stock_entry/stock_entry.py

@@ -83,15 +83,6 @@ 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."""
@@ -1456,12 +1447,9 @@ 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
-		finished_items_last = sorted(self.get("items"), key=lambda row: cint(row.is_finished_item))
-		for d in finished_items_last:
+		for d in self.get("items"):
 			if d.s_warehouse or d.set_basic_rate_manually:
 				continue
 
@@ -1471,7 +1459,7 @@ class StockEntry(StockController, SubcontractingInwardController):
 				d.basic_amount = 0.0
 				continue
 
-			has_derived_rate = False
+			rate_derived_from_consumption = False
 
 			if d.allow_zero_valuation_rate and d.basic_rate and self.purpose != "Receive from Customer":
 				d.basic_rate = 0.0
@@ -1481,25 +1469,26 @@ class StockEntry(StockController, SubcontractingInwardController):
 					d.basic_rate = self.get_basic_rate_for_manufactured_item(
 						d.transfer_qty, outgoing_items_cost, has_consumption_basis
 					)
-					has_derived_rate = has_consumption_basis
+					rate_derived_from_consumption = 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
-					has_derived_rate = any(item.s_warehouse for item in self.get("items"))
+					rate_derived_from_consumption = 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 = flt(
-					frappe.get_value("BOM Secondary Item", d.bom_secondary_item, "cost_allocation_per")
+				cost_allocation_per = frappe.get_value(
+					"BOM Secondary Item", d.bom_secondary_item, "cost_allocation_per"
 				)
-				if flt(d.transfer_qty):
-					d.basic_rate = (secondary_items_cost_basis * (cost_allocation_per / 100)) / d.transfer_qty
-					has_derived_rate = True
+				# 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
 
-			# 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:
+			# 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:
 				if self.is_new():
 					raise_error_if_no_rate = False
 
@@ -1612,43 +1601,11 @@ 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 is_costed_out_of_finished_item(d)]
-		)
+		scrap_items_cost = sum([flt(d.basic_amount) for d in self.get("items") if d.is_legacy_scrap_item])
 
 		if settings.material_consumption:
 			if settings.get_rm_cost_from_consumption_entry and self.work_order:
@@ -1685,7 +1642,20 @@ class StockEntry(StockController, SubcontractingInwardController):
 							)
 						)
 
-					outgoing_items_cost = self._fetch_consumption_entry_cost()
+					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
 
 			# 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.
@@ -2061,9 +2031,7 @@ class StockEntry(StockController, SubcontractingInwardController):
 
 		for d in self.items:
 			if d.t_warehouse and not d.s_warehouse:
-				if d.type or d.is_legacy_scrap_item:
-					d.is_finished_item = 0
-				elif self.purpose == "Repack" or d.item_code == finished_item:
+				if self.purpose == "Repack" or d.item_code == finished_item:
 					d.is_finished_item = 1
 			else:
 				d.is_finished_item = 0

erpnext/stock/doctype/stock_entry/test_stock_entry.py

@@ -2738,164 +2738,6 @@ 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(
@@ -2918,7 +2760,7 @@ class TestStockEntry(ERPNextTestSuite):
 					"t_warehouse": "_Test Warehouse - _TC",
 					"qty": 10,
 					"conversion_factor": 1,
-					"type": secondary_item_type,
+					"secondary_item_type": secondary_item_type,
 				},
 			)
 			return se
@@ -2926,66 +2768,6 @@ class TestStockEntry(ERPNextTestSuite):
 		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 (

erpnext/stock/services/quality_inspection_service.py

@@ -0,0 +1,165 @@
+# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
+# License: GNU General Public License v3. See license.txt
+
+"""Quality inspection validation for stock transactions.
+
+Extracted from ``StockController``. Validates that items requiring quality
+inspection have a present / submitted / non-rejected Quality Inspection.
+"""
+
+import frappe
+from frappe import _
+
+from erpnext.exceptions import (
+	QualityInspectionNotSubmittedError,
+	QualityInspectionRejectedError,
+	QualityInspectionRequiredError,
+)
+
+# Doctype -> the document-level "inspection required" flag. Shared with
+# check_item_quality_inspection in stock_controller so the two stay in sync.
+INSPECTION_FIELDNAME_MAP = {
+	"Purchase Receipt": "inspection_required_before_purchase",
+	"Purchase Invoice": "inspection_required_before_purchase",
+	"Subcontracting Receipt": "inspection_required_before_purchase",
+	"Sales Invoice": "inspection_required_before_delivery",
+	"Delivery Note": "inspection_required_before_delivery",
+}
+
+# Purposes whose inward (t_warehouse) row is inspected.
+QI_INCOMING_PURPOSES = (
+	"Material Receipt",
+	"Repack",
+	"Receive from Customer",
+	"Subcontracting Return",
+)
+
+# Purposes whose outgoing (s_warehouse) row is inspected. This is an explicit
+# allow-list rather than "everything that isn't incoming" so a new purpose can't
+# silently start requiring a QI. Material Consumption for Manufacture is left out
+# on purpose: an inspection_required BOM inspects the manufactured output (handled
+# by the "Manufacture" finished-good rule), not each consumed raw material.
+# Keep this in sync with erpnext.stock.qi_* helpers in transaction.js.
+QI_OUTGOING_PURPOSES = (
+	"Material Issue",
+	"Material Transfer",
+	"Material Transfer for Manufacture",
+	"Send to Subcontractor",
+	"Subcontracting Delivery",
+	"Disassemble",
+)
+
+
+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("secondary_item_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 purpose in SECONDARY_ITEM_PURPOSES and (
+		row.get("secondary_item_type") or row.get("is_legacy_scrap_item")
+	):
+		return False
+	if purpose == "Manufacture":
+		return bool(row.is_finished_item)
+	if purpose in QI_INCOMING_PURPOSES:
+		return bool(row.t_warehouse)
+	if purpose in QI_OUTGOING_PURPOSES:
+		return bool(row.s_warehouse and row.s_warehouse != row.t_warehouse)
+	return False
+
+
+class QualityInspectionService:
+	def __init__(self, doc) -> None:
+		self.doc = doc
+
+	def validate_inspection(self):
+		"""Checks if quality inspection is set/ is valid for Items that require inspection."""
+		inspection_required_fieldname = INSPECTION_FIELDNAME_MAP.get(self.doc.doctype)
+
+		# return if inspection is not required on document level
+		if (
+			(not inspection_required_fieldname and self.doc.doctype != "Stock Entry")
+			or (self.doc.doctype == "Stock Entry" and not self.doc.inspection_required)
+			or (self.doc.doctype in ["Sales Invoice", "Purchase Invoice"] and not self.doc.update_stock)
+		):
+			return
+
+		for row in self.doc.get("items"):
+			qi_required = False
+			if inspection_required_fieldname and frappe.get_cached_value(
+				"Item", row.item_code, inspection_required_fieldname
+			):
+				qi_required = True
+			elif self.doc.doctype == "Stock Entry":
+				qi_required = stock_entry_row_requires_inspection(self.doc.purpose, row)
+
+			if is_inspection_exempt_secondary_row(self.doc, row):
+				continue
+
+			if qi_required:  # validate row only if inspection is required on item level
+				if self.doc.doctype in [
+					"Purchase Receipt",
+					"Purchase Invoice",
+					"Sales Invoice",
+					"Delivery Note",
+				] and frappe.get_single_value(
+					"Stock Settings", "allow_to_make_quality_inspection_after_purchase_or_delivery"
+				):
+					return
+
+				self.validate_qi_presence(row)
+				if self.doc.docstatus == 1:
+					self.validate_qi_submission(row)
+					self.validate_qi_rejection(row)
+
+	def validate_qi_presence(self, row):
+		"""Check if QI is present on row level. Warn on save and stop on submit if missing."""
+		if not row.quality_inspection:
+			msg = _("Row #{0}: Quality Inspection is required for Item {1}").format(
+				row.idx, frappe.bold(row.item_code)
+			)
+			if self.doc.docstatus == 1:
+				frappe.throw(msg, title=_("Inspection Required"), exc=QualityInspectionRequiredError)
+			else:
+				frappe.msgprint(msg, title=_("Inspection Required"), indicator="blue")
+
+	def validate_qi_submission(self, row):
+		"""Check if QI is submitted on row level, during submission"""
+		action = frappe.get_single_value("Stock Settings", "action_if_quality_inspection_is_not_submitted")
+		qa_docstatus = frappe.db.get_value("Quality Inspection", row.quality_inspection, "docstatus")
+
+		if qa_docstatus != 1:
+			link = frappe.utils.get_link_to_form("Quality Inspection", row.quality_inspection)
+			msg = _("Row #{0}: Quality Inspection {1} is not submitted for the item: {2}").format(
+				row.idx, link, row.item_code
+			)
+			if action == "Stop":
+				frappe.throw(msg, title=_("Inspection Submission"), exc=QualityInspectionNotSubmittedError)
+			else:
+				frappe.msgprint(msg, alert=True, indicator="orange")
+
+	def validate_qi_rejection(self, row):
+		"""Check if QI is rejected on row level, during submission"""
+		action = frappe.get_single_value("Stock Settings", "action_if_quality_inspection_is_rejected")
+		qa_status = frappe.db.get_value("Quality Inspection", row.quality_inspection, "status")
+
+		if qa_status == "Rejected":
+			link = frappe.utils.get_link_to_form("Quality Inspection", row.quality_inspection)
+			msg = _("Row #{0}: Quality Inspection {1} was rejected for item {2}").format(
+				row.idx, link, row.item_code
+			)
+			if action == "Stop":
+				frappe.throw(msg, title=_("Inspection Rejected"), exc=QualityInspectionRejectedError)
+			else:
+				frappe.msgprint(msg, alert=True, indicator="orange")