fix(stock): allocate secondary item cost from the consumption entry (#57738)

* fix(stock): allocate secondary item cost from the consumption entry

A secondary item's rate is its BOM share of the cost of the consumed
rows. With Get RM Cost From Consumption Entry enabled the consumption
happens in a separate document, so the Manufacture entry carries no
consumed rows and that cost is zero. The share evaluated to zero, and the
row fell through to the item's own valuation rate.

Only the finished good substituted the consumption entry's cost. Against
a consumption entry of 1000 and a BOM allocating 75% to the finished good
and 25% to scrap, the finished good took its 750 while the scrap took an
unrelated valuation of 100, booking 850 for 1000 consumed.

Derive the allocation base once and use it for both sides.

* test(stock): cover secondary allocation against a consumption entry

A consumption entry of 1000 splits into 750 and 250 by the BOM's shares.

(cherry picked from commit 8db8c6a83d)

# Conflicts:
#	erpnext/stock/doctype/stock_entry/stock_entry.py
#	erpnext/stock/doctype/stock_entry/test_stock_entry.py

erpnext/__init__.py

@@ -6,7 +6,7 @@ import frappe
 from frappe.model.document import Document
 from frappe.utils.user import is_website_user
 
-__version__ = "16.31.1"
+__version__ = "16.26.2"
 
 
 def get_default_company(user=None):

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/payment_reconciliation/test_payment_reconciliation.py

@@ -2405,86 +2405,6 @@ class TestPaymentReconciliation(ERPNextTestSuite):
 		self.assertEqual(flt(pr.allocation[0].get("difference_amount")), -5000.0)
 		pr.reconcile()
 
-	def test_foreign_currency_reverse_payment_entry_gain_for_supplier(self):
-		transaction_date = nowdate()
-		self.supplier = "_Test Supplier USD"
-		amount = 100
-		department = frappe.db.get_value("Department", {"company": self.company, "is_group": 0}, "name")
-
-		# Pay USD 100 at an exchange rate of 90.
-		pe = self.create_payment_entry(amount=amount, posting_date=transaction_date)
-		pe.payment_type = "Pay"
-		pe.party_type = "Supplier"
-		pe.party = self.supplier
-		pe.paid_from = self.cash
-		pe.paid_from_account_currency = "INR"
-		pe.target_exchange_rate = 90
-		pe.paid_amount = 90 * amount
-		pe.received_amount = amount
-		pe.paid_to = self.creditors_usd
-		pe.paid_to_account_currency = "USD"
-		pe.department = department
-		pe = pe.save().submit()
-
-		# Receive USD 100 from the supplier at an exchange rate of 100.
-		reverse_pe = self.create_payment_entry(amount=amount, posting_date=transaction_date)
-		reverse_pe.payment_type = "Receive"
-		reverse_pe.party_type = "Supplier"
-		reverse_pe.party = self.supplier
-		reverse_pe.paid_from = self.creditors_usd
-		reverse_pe.paid_from_account_currency = "USD"
-		reverse_pe.source_exchange_rate = 100
-		reverse_pe.paid_amount = amount
-		reverse_pe.received_amount = 100 * amount
-		reverse_pe.paid_to = self.cash
-		reverse_pe.paid_to_account_currency = "INR"
-		reverse_pe.department = department
-		reverse_pe = reverse_pe.save().submit()
-
-		pr = self.create_payment_reconciliation(party_is_customer=False)
-		pr.party = self.supplier
-		pr.receivable_payable_account = self.creditors_usd
-		pr.get_unreconciled_entries()
-		invoices = [invoice.as_dict() for invoice in pr.invoices]
-		payments = [payment.as_dict() for payment in pr.payments]
-		pr.allocate_entries(frappe._dict({"invoices": invoices, "payments": payments}))
-		for row in pr.allocation:
-			row.department = department
-
-		self.assertEqual(flt(pr.allocation[0].difference_amount), 1000)
-		pr.reconcile()
-
-		gain_loss_journal = frappe.db.get_value(
-			"Journal Entry Account",
-			{
-				"reference_type": reverse_pe.doctype,
-				"reference_name": reverse_pe.name,
-				"party": self.supplier,
-				"docstatus": 1,
-			},
-			"parent",
-		)
-		party_row = frappe.db.get_value(
-			"Journal Entry Account",
-			{"parent": gain_loss_journal, "party": self.supplier},
-			["debit", "credit"],
-			as_dict=True,
-		)
-		self.assertEqual(flt(party_row.debit), 1000)
-		self.assertEqual(flt(party_row.credit), 0)
-
-		party_gl_entries = frappe.get_all(
-			"GL Entry",
-			filters={
-				"voucher_no": ["in", [pe.name, reverse_pe.name, gain_loss_journal]],
-				"account": self.creditors_usd,
-				"party": self.supplier,
-				"is_cancelled": 0,
-			},
-			fields=["debit", "credit"],
-		)
-		self.assertEqual(flt(sum(row.debit - row.credit for row in party_gl_entries)), 0)
-
 	def test_foreign_currency_reverse_journal_entry_against_journal_entry_for_customer(self):
 		transaction_date = nowdate()
 		customer = self.customer_usd

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/accounts/party.py

@@ -854,12 +854,10 @@ def validate_account_party_type(self):
 
 
 def get_dashboard_info(party_type, party, loyalty_program=None):
-	doctype = "Sales Invoice" if party_type == "Customer" else "Purchase Invoice"
-	if not frappe.has_permission(doctype, "read"):
-		return None
-
 	current_fiscal_year = get_fiscal_year(nowdate(), as_dict=True)
 
+	doctype = "Sales Invoice" if party_type == "Customer" else "Purchase Invoice"
+
 	companies = frappe.get_list(
 		doctype, filters={"docstatus": 1, party_type.lower(): party}, distinct=1, fields=["company"]
 	)

erpnext/controllers/accounts_controller.py

@@ -1893,7 +1893,7 @@ class AccountsController(TransactionBase):
 
 	def is_payable_account(self, reference_doctype, account):
 		if reference_doctype == "Purchase Invoice" or (
-			reference_doctype in ("Journal Entry", "Payment Entry")
+			reference_doctype == "Journal Entry"
 			and frappe.get_cached_value("Account", account, "account_type") == "Payable"
 		):
 			return True

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/crm/doctype/opportunity/opportunity.py

@@ -134,7 +134,6 @@ class Opportunity(TransactionBase, CRMNote):
 		self.validate_uom_is_integer("uom", "qty")
 		self.validate_cust_name()
 		self.map_fields()
-		self.validate_qty()
 		self.set_exchange_rate()
 
 		if not self.title:
@@ -145,15 +144,6 @@ class Opportunity(TransactionBase, CRMNote):
 	def on_update(self):
 		self.update_prospect()
 
-	def validate_qty(self):
-		for item in self.items:
-			if flt(item.qty) <= 0:
-				frappe.throw(
-					_("Row #{0}: Quantity must be greater than 0 for Item {1}").format(
-						item.idx, item.item_code
-					)
-				)
-
 	def map_fields(self):
 		for field in self.meta.get_valid_columns():
 			if not self.get(field) and frappe.db.field_exists(self.opportunity_from, field):

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/bom_configurator/bom_configurator.bundle.js

@@ -32,7 +32,18 @@ class BOMConfigurator {
 	}
 
 	bind_events() {
-		frappe.views.trees["BOM Configurator"].events = this;
+		frappe.views.trees["BOM Configurator"].events = {
+			frm: this.frm,
+			add_item: this.add_item,
+			add_sub_assembly: this.add_sub_assembly,
+			set_query_for_workstation: this.set_query_for_workstation,
+			get_sub_assembly_modal_fields: this.get_sub_assembly_modal_fields,
+			convert_to_sub_assembly: this.convert_to_sub_assembly,
+			delete_node: this.delete_node,
+			edit_bom: this.edit_bom,
+			load_tree: this.load_tree,
+			set_default_qty: this.set_default_qty,
+		};
 	}
 
 	tree_options() {

erpnext/public/js/controllers/transaction.js

@@ -20,10 +20,8 @@ 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 (erpnext.stock.secondary_item_purposes.includes(purpose) && (row.type || row.is_legacy_scrap_item))
-		return false;
+	if (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/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/material_request/material_request.js

@@ -533,7 +533,7 @@ frappe.ui.form.on("Material Request", {
 				},
 			],
 			primary_action_label: __("Create"),
-			primary_action: function (values) {
+			primary_action: async function (values) {
 				const item_suppliers = (values.items || []).filter((row) => row.__checked);
 				if (!item_suppliers.length) {
 					frappe.throw(__("Select at least one Item"));
@@ -567,6 +567,10 @@ frappe.ui.form.on("Material Request", {
 					);
 				}
 
+				if (!(await erpnext.utils.confirm_if_drafts_exist(frm.doc, "Purchase Order"))) {
+					return;
+				}
+
 				frappe.call({
 					method: "erpnext.stock.doctype.material_request.material_request.make_purchase_orders_by_supplier",
 					args: { source_name: frm.doc.name, item_suppliers: item_suppliers },

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,21 @@ 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)
-
+<<<<<<< HEAD
 		items = []
 		# Set basic rate for incoming items
+		for d in self.get("items"):
+=======
+		bom_cost_allocation_per = (
+			frappe.get_cached_value("BOM", self.bom_no, "cost_allocation_per") if self.bom_no else None
+		)
+
+		secondary_items_cost_basis = self.get_secondary_items_cost_basis(outgoing_items_cost)
+
+		zero_valuation_items = []
 		finished_items_last = sorted(self.get("items"), key=lambda row: cint(row.is_finished_item))
 		for d in finished_items_last:
+>>>>>>> 8db8c6a83d (fix(stock): allocate secondary item cost from the consumption entry (#57738))
 			if d.s_warehouse or d.set_basic_rate_manually:
 				continue
 
@@ -1471,7 +1471,19 @@ class StockEntry(StockController, SubcontractingInwardController):
 				d.basic_amount = 0.0
 				continue
 
-			has_derived_rate = False
+<<<<<<< HEAD
+			rate_derived_from_consumption = False
+=======
+			self._set_incoming_item_rate(
+				d,
+				outgoing_items_cost,
+				raise_error_if_no_rate,
+				zero_valuation_items,
+				bom_cost_allocation_per,
+				has_consumption_basis,
+				secondary_items_cost_basis,
+			)
+>>>>>>> 8db8c6a83d (fix(stock): allocate secondary item cost from the consumption entry (#57738))
 
 			if d.allow_zero_valuation_rate and d.basic_rate and self.purpose != "Receive from Customer":
 				d.basic_rate = 0.0
@@ -1481,25 +1493,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
 
@@ -1534,6 +1547,20 @@ class StockEntry(StockController, SubcontractingInwardController):
 
 			frappe.msgprint(message, alert=True)
 
+	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 has_consumption_basis(self) -> bool:
 		"""Whether the cost of the consumed items is known, even when that cost is zero."""
 		if any(d.s_warehouse for d in self.get("items")):
@@ -1559,6 +1586,77 @@ class StockEntry(StockController, SubcontractingInwardController):
 			)
 		return self._consumption_entries
 
+<<<<<<< HEAD
+=======
+	def _set_incoming_item_rate(
+		self,
+		d,
+		outgoing_items_cost,
+		raise_error_if_no_rate,
+		zero_valuation_items,
+		bom_cost_allocation_per=None,
+		has_consumption_basis=False,
+		secondary_items_cost_basis=0,
+	):
+		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
+			zero_valuation_items.append(d.item_code)
+		elif d.is_finished_item:
+			if self.purpose == "Manufacture":
+				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
+			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"))
+
+			if self.bom_no:
+				d.basic_rate *= bom_cost_allocation_per / 100
+		elif d.secondary_item_type and d.bom_secondary_item:
+			cost_allocation_per = flt(
+				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
+
+		# 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:
+			d.basic_rate = get_valuation_rate(
+				d.item_code,
+				d.t_warehouse,
+				self.doctype,
+				self.name,
+				d.allow_zero_valuation_rate,
+				currency=erpnext.get_company_currency(self.company),
+				company=self.company,
+				raise_error_if_no_rate=raise_error_if_no_rate,
+				batch_no=d.batch_no,
+				serial_and_batch_bundle=d.serial_and_batch_bundle,
+			)
+
+		# do not round off basic rate to avoid precision loss
+		d.basic_rate = flt(d.basic_rate)
+		d.basic_amount = flt(flt(d.transfer_qty) * flt(d.basic_rate), d.precision("basic_amount"))
+
+	def _notify_zero_valuation_rate(self, items):
+		if len(items) > 1:
+			message = _(
+				"Items rate has been updated to zero as Allow Zero Valuation Rate is checked for the following items: {0}"
+			).format(", ".join(frappe.bold(item) for item in items))
+		else:
+			message = _(
+				"Item rate has been updated to zero as Allow Zero Valuation Rate is checked for item {0}"
+			).format(frappe.bold(items[0]))
+
+		frappe.msgprint(message, alert=True)
+
+>>>>>>> 8db8c6a83d (fix(stock): allocate secondary item cost from the consumption entry (#57738))
 	def set_rate_for_outgoing_items(self, reset_outgoing_rate=True, raise_error_if_no_rate=True):
 		outgoing_items_cost = 0.0
 		for d in self.get("items"):
@@ -1612,43 +1710,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 +1751,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 +2140,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

@@ -7,7 +7,6 @@ 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,
@@ -2738,6 +2737,38 @@ class TestStockEntry(ERPNextTestSuite):
 		self.assertEqual(fg_sle.incoming_rate, 0)
 		self.assertEqual(fg_sle.stock_value_difference, 0)
 
+<<<<<<< HEAD
+=======
+	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,
+					"secondary_item_type": secondary_item_type,
+				},
+			)
+			return se
+
+		self.assertRaises(QualityInspectionRequiredError, receipt("").submit)
+		self.assertRaises(QualityInspectionRequiredError, receipt("Scrap").submit)
+
 	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
@@ -2769,7 +2800,7 @@ class TestStockEntry(ERPNextTestSuite):
 				"item_code": scrap_item,
 				"t_warehouse": warehouse,
 				"qty": 5,
-				"type": "Scrap",
+				"secondary_item_type": "Scrap",
 				"conversion_factor": 1,
 			},
 		)
@@ -2807,7 +2838,7 @@ class TestStockEntry(ERPNextTestSuite):
 		bom.append(
 			"secondary_items",
 			{
-				"type": "Scrap",
+				"secondary_item_type": "Scrap",
 				"item_code": scrap_item,
 				"item_name": scrap_item,
 				"qty": 5,
@@ -2833,7 +2864,7 @@ class TestStockEntry(ERPNextTestSuite):
 		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)
+		scrap_row = next(d for d in se.items if d.secondary_item_type)
 
 		self.assertFalse(scrap_row.is_finished_item)
 		self.assertEqual(flt(scrap_row.basic_amount), 250.0)
@@ -2868,7 +2899,7 @@ class TestStockEntry(ERPNextTestSuite):
 		bom.append(
 			"secondary_items",
 			{
-				"type": "Scrap",
+				"secondary_item_type": "Scrap",
 				"item_code": scrap_item,
 				"item_name": scrap_item,
 				"qty": 5,
@@ -2888,7 +2919,7 @@ class TestStockEntry(ERPNextTestSuite):
 		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)
+		scrap_row = next(d for d in se.items if d.secondary_item_type)
 		fg_row = next(d for d in se.items if d.is_finished_item)
 
 		self.assertEqual(flt(scrap_row.basic_rate), 0.0)
@@ -2896,36 +2927,6 @@ class TestStockEntry(ERPNextTestSuite):
 		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}
 	)
@@ -2954,7 +2955,7 @@ class TestStockEntry(ERPNextTestSuite):
 		bom.append(
 			"secondary_items",
 			{
-				"type": "Scrap",
+				"secondary_item_type": "Scrap",
 				"item_code": scrap_item,
 				"item_name": scrap_item,
 				"qty": 5,
@@ -2979,13 +2980,14 @@ class TestStockEntry(ERPNextTestSuite):
 		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)
+		scrap_row = next(d for d in se.items if d.secondary_item_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)
 
+>>>>>>> 8db8c6a83d (fix(stock): allocate secondary item cost from the consumption entry (#57738))
 	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/report/stock_and_account_value_comparison/test_stock_and_account_value_comparison.py

@@ -6,8 +6,6 @@ from frappe.utils import today
 
 from erpnext.stock.doctype.item.test_item import make_item
 from erpnext.stock.doctype.purchase_receipt.test_purchase_receipt import make_purchase_receipt
-from erpnext.stock.doctype.warehouse.test_warehouse import create_warehouse
-from erpnext.stock.doctype.warehouse.warehouse import get_warehouses_based_on_account
 from erpnext.stock.report.stock_and_account_value_comparison.stock_and_account_value_comparison import (
 	create_reposting_entries,
 	execute,
@@ -57,22 +55,3 @@ class TestStockAndAccountValueComparison(ERPNextTestSuite):
 			filters={"based_on": "Item and Warehouse", "item_code": item},
 		)
 		self.assertFalse(item_wh_rivs, "Purchase vouchers must not be reposted Item-and-Warehouse based")
-
-	def test_child_account_override_excluded_from_group_account(self):
-		# A group warehouse carries an inventory account; a child (e.g. Goods-in-Transit) can override
-		# it with its own account. get_warehouses_based_on_account must return only warehouses whose
-		# effective account matches, excluding the overriding child.
-		group = create_warehouse("_Test SAVC Group WH", {"is_group": 1}, company=PI_COMPANY)
-		group_account = frappe.get_value("Warehouse", group, "account")
-
-		inheriting = create_warehouse(
-			"_Test SAVC Inherit WH", {"parent_warehouse": group, "account": group_account}, company=PI_COMPANY
-		)
-		overriding = create_warehouse(
-			"_Test SAVC Transit WH", {"parent_warehouse": group}, company=PI_COMPANY
-		)
-
-		warehouses = get_warehouses_based_on_account(group_account, PI_COMPANY)
-
-		self.assertIn(inheriting, warehouses)
-		self.assertNotIn(overriding, warehouses)