Merge pull request #30517 from frappe/mergify/bp/version-13-hotfix/pr…

…-30509

fix: Add non-existent Item check and cleanup in `validate_for_items` (backport #30509)

erpnext/assets/doctype/asset/test_asset.py

@@ -68,7 +68,7 @@ def test_available_for_use_date_is_after_purchase_date(self):
 	def test_item_exists(self):
 		asset = create_asset(item_code="MacBook", do_not_save=1)
 
-		self.assertRaises(frappe.DoesNotExistError, asset.save)
+		self.assertRaises(frappe.ValidationError, asset.save)
 
 	def test_validate_item(self):
 		asset = create_asset(item_code="MacBook Pro", do_not_save=1)

erpnext/buying/utils.py

@@ -3,19 +3,19 @@
 
 
 import json
+from typing import Dict
 
 import frappe
 from frappe import _
-from frappe.utils import cint, cstr, flt
+from frappe.utils import cint, cstr, flt, getdate
 
 from erpnext.stock.doctype.item.item import get_last_purchase_details, validate_end_of_life
 
 
-def update_last_purchase_rate(doc, is_submit):
+def update_last_purchase_rate(doc, is_submit) -> None:
 	"""updates last_purchase_rate in item table for each item"""
-	import frappe.utils
 
-	this_purchase_date = frappe.utils.getdate(doc.get("posting_date") or doc.get("transaction_date"))
+	this_purchase_date = getdate(doc.get("posting_date") or doc.get("transaction_date"))
 
 	for d in doc.get("items"):
 		# get last purchase details
@@ -41,48 +41,19 @@ def update_last_purchase_rate(doc, is_submit):
 		frappe.db.set_value("Item", d.item_code, "last_purchase_rate", flt(last_purchase_rate))
 
 
-def validate_for_items(doc):
+def validate_for_items(doc) -> None:
 	items = []
 	for d in doc.get("items"):
 		if not d.qty:
 			if doc.doctype == "Purchase Receipt" and d.rejected_qty:
 				continue
 			frappe.throw(_("Please enter quantity for Item {0}").format(d.item_code))
 
-		# update with latest quantities
-		bin = frappe.db.sql(
-			"""select projected_qty from `tabBin` where
-			item_code = %s and warehouse = %s""",
-			(d.item_code, d.warehouse),
-			as_dict=1,
-		)
-
-		f_lst = {
-			"projected_qty": bin and flt(bin[0]["projected_qty"]) or 0,
-			"ordered_qty": 0,
-			"received_qty": 0,
-		}
-		if d.doctype in ("Purchase Receipt Item", "Purchase Invoice Item"):
-			f_lst.pop("received_qty")
-		for x in f_lst:
-			if d.meta.get_field(x):
-				d.set(x, f_lst[x])
-
-		item = frappe.db.sql(
-			"""select is_stock_item,
-			is_sub_contracted_item, end_of_life, disabled from `tabItem` where name=%s""",
-			d.item_code,
-			as_dict=1,
-		)[0]
-
+		set_stock_levels(row=d)  # update with latest quantities
+		item = validate_item_and_get_basic_data(row=d)
+		validate_stock_item_warehouse(row=d, item=item)
 		validate_end_of_life(d.item_code, item.end_of_life, item.disabled)
 
-		# validate stock item
-		if item.is_stock_item == 1 and d.qty and not d.warehouse and not d.get("delivered_by_supplier"):
-			frappe.throw(
-				_("Warehouse is mandatory for stock Item {0} in row {1}").format(d.item_code, d.idx)
-			)
-
 		items.append(cstr(d.item_code))
 
 	if (
@@ -93,7 +64,57 @@ def validate_for_items(doc):
 		frappe.throw(_("Same item cannot be entered multiple times."))
 
 
-def check_on_hold_or_closed_status(doctype, docname):
+def set_stock_levels(row) -> None:
+	projected_qty = frappe.db.get_value(
+		"Bin",
+		{
+			"item_code": row.item_code,
+			"warehouse": row.warehouse,
+		},
+		"projected_qty",
+	)
+
+	qty_data = {
+		"projected_qty": flt(projected_qty),
+		"ordered_qty": 0,
+		"received_qty": 0,
+	}
+	if row.doctype in ("Purchase Receipt Item", "Purchase Invoice Item"):
+		qty_data.pop("received_qty")
+
+	for field in qty_data:
+		if row.meta.get_field(field):
+			row.set(field, qty_data[field])
+
+
+def validate_item_and_get_basic_data(row) -> Dict:
+	item = frappe.db.get_values(
+		"Item",
+		filters={"name": row.item_code},
+		fieldname=["is_stock_item", "is_sub_contracted_item", "end_of_life", "disabled"],
+		as_dict=1,
+	)
+	if not item:
+		frappe.throw(_("Row #{0}: Item {1} does not exist").format(row.idx, frappe.bold(row.item_code)))
+
+	return item[0]
+
+
+def validate_stock_item_warehouse(row, item) -> None:
+	if (
+		item.is_stock_item == 1
+		and row.qty
+		and not row.warehouse
+		and not row.get("delivered_by_supplier")
+	):
+		frappe.throw(
+			_("Row #{1}: Warehouse is mandatory for stock Item {0}").format(
+				frappe.bold(row.item_code), row.idx
+			)
+		)
+
+
+def check_on_hold_or_closed_status(doctype, docname) -> None:
 	status = frappe.db.get_value(doctype, docname, "status")
 
 	if status in ("Closed", "On Hold"):