Merge pull request #56214 from mihir-kandoi/pg-controllers-return-stock

refactor(controllers): sales/purchase return + stock_controller raw SQL → qb/ORM (Postgres)

erpnext/controllers/sales_and_purchase_return.py

@@ -7,7 +7,7 @@ import frappe
 from frappe import _, bold
 from frappe.model.meta import get_field_precision
 from frappe.query_builder import DocType
-from frappe.query_builder.functions import Abs
+from frappe.query_builder.functions import Abs, Sum
 from frappe.utils import cint, flt, format_datetime, get_datetime
 
 import erpnext
@@ -86,26 +86,27 @@ def validate_return_against(doc):
 def validate_returned_items(doc):
 	valid_items = frappe._dict()
 
-	select_fields = "item_code, qty, stock_qty, rate, parenttype, conversion_factor, name"
+	select_fields = ["item_code", "qty", "stock_qty", "rate", "parenttype", "conversion_factor", "name"]
 	if doc.doctype != "Purchase Invoice":
-		select_fields += ",serial_no, batch_no"
+		select_fields += ["serial_no", "batch_no"]
 
 	if doc.doctype in ["Purchase Invoice", "Purchase Receipt", "Subcontracting Receipt"]:
-		select_fields += ",rejected_qty, received_qty"
+		select_fields += ["rejected_qty", "received_qty"]
 
-	for d in frappe.db.sql(
-		f"""select {select_fields} from `tab{doc.doctype} Item` where parent = %s""",
-		doc.return_against,
-		as_dict=1,
+	for d in frappe.get_all(
+		f"{doc.doctype} Item",
+		filters={"parent": doc.return_against},
+		fields=select_fields,
+		limit_page_length=0,  # all item rows of the reference document are needed (no default 20 cap)
 	):
 		valid_items = get_ref_item_dict(valid_items, d)
 
 	if doc.doctype in ("Delivery Note", "Sales Invoice"):
-		for d in frappe.db.sql(
-			"""select item_code, qty, serial_no, batch_no from `tabPacked Item`
-			where parent = %s""",
-			doc.return_against,
-			as_dict=1,
+		for d in frappe.get_all(
+			"Packed Item",
+			filters={"parent": doc.return_against},
+			fields=["item_code", "qty", "serial_no", "batch_no"],
+			limit_page_length=0,  # all packed-item rows are needed (no default 20 cap)
 		):
 			valid_items = get_ref_item_dict(valid_items, d)
 
@@ -271,29 +272,35 @@ def get_ref_item_dict(valid_items, ref_item_row):
 
 
 def get_already_returned_items(doc):
-	column = "child.item_code, sum(abs(child.qty)) as qty, sum(abs(child.stock_qty)) as stock_qty"
-	if doc.doctype in ["Purchase Invoice", "Purchase Receipt", "Subcontracting Receipt"]:
-		column += """, sum(abs(child.rejected_qty) * child.conversion_factor) as rejected_qty,
-			sum(abs(child.received_qty) * child.conversion_factor) as received_qty"""
+	child = DocType(f"{doc.doctype} Item")
+	par = DocType(doc.doctype)
 
 	field = (
 		frappe.scrub(doc.doctype) + "_item"
 		if doc.doctype in ["Purchase Invoice", "Purchase Receipt", "Sales Invoice", "POS Invoice"]
 		else "dn_detail"
 	)
-	data = frappe.db.sql(
-		f"""
-		select {column}, child.{field}
-		from
-			`tab{doc.doctype} Item` child, `tab{doc.doctype}` par
-		where
-			child.parent = par.name and par.docstatus = 1
-			and par.is_return = 1 and par.return_against = %s
-		group by item_code, {field}
-	""",
-		doc.return_against,
-		as_dict=1,
+
+	query = (
+		frappe.qb.from_(child)
+		.inner_join(par)
+		.on(child.parent == par.name)
+		.select(
+			child.item_code,
+			Sum(Abs(child.qty)).as_("qty"),
+			Sum(Abs(child.stock_qty)).as_("stock_qty"),
+			child[field],
+		)
+		.where((par.docstatus == 1) & (par.is_return == 1) & (par.return_against == doc.return_against))
+		.groupby(child.item_code, child[field])
 	)
+	if doc.doctype in ["Purchase Invoice", "Purchase Receipt", "Subcontracting Receipt"]:
+		query = query.select(
+			Sum(Abs(child.rejected_qty) * child.conversion_factor).as_("rejected_qty"),
+			Sum(Abs(child.received_qty) * child.conversion_factor).as_("received_qty"),
+		)
+
+	data = query.run(as_dict=1)
 
 	items = {}
 

erpnext/controllers/stock_controller.py

@@ -5,6 +5,8 @@ import json
 
 import frappe
 from frappe import _, bold
+from frappe.query_builder import Criterion
+from frappe.query_builder.functions import Count
 from frappe.utils import cint, cstr, flt, get_link_to_form, getdate
 
 import erpnext
@@ -279,11 +281,7 @@ class StockController(AccountsController):
 	def make_gl_entries_on_cancel(self, from_repost=False):
 		if not from_repost:
 			cancel_exchange_gain_loss_journal(frappe._dict(doctype=self.doctype, name=self.name))
-		if frappe.db.sql(
-			"""select name from `tabGL Entry` where voucher_type=%s
-			and voucher_no=%s""",
-			(self.doctype, self.name),
-		):
+		if frappe.db.exists("GL Entry", {"voucher_type": self.doctype, "voucher_no": self.name}):
 			self.make_gl_entries()
 
 	def validate_warehouse(self):
@@ -725,20 +723,18 @@ def future_sle_exists(args, sl_entries=None):
 
 	args["posting_datetime"] = get_combine_datetime(args["posting_date"], args["posting_time"])
 
-	data = frappe.db.sql(
-		"""
-		select item_code, warehouse, count(name) as total_row
-		from `tabStock Ledger Entry`
-		where
-			({})
-			and posting_datetime >= %(posting_datetime)s
-			and voucher_no != %(voucher_no)s
-			and is_cancelled = 0
-		GROUP BY
-			item_code, warehouse
-		""".format(" or ".join(or_conditions)),
-		args,
-		as_dict=1,
+	sle = frappe.qb.DocType("Stock Ledger Entry")
+	data = (
+		frappe.qb.from_(sle)
+		.select(sle.item_code, sle.warehouse, Count(sle.name).as_("total_row"))
+		.where(
+			Criterion.any(or_conditions)
+			& (sle.posting_datetime >= args["posting_datetime"])
+			& (sle.voucher_no != args["voucher_no"])
+			& (sle.is_cancelled == 0)
+		)
+		.groupby(sle.item_code, sle.warehouse)
+		.run(as_dict=1)
 	)
 
 	for d in data:
@@ -792,12 +788,10 @@ def get_conditions_to_validate_future_sle(sl_entries):
 
 		warehouse_items_map[entry.warehouse].add(entry.item_code)
 
+	sle = frappe.qb.DocType("Stock Ledger Entry")
 	or_conditions = []
 	for warehouse, items in warehouse_items_map.items():
-		or_conditions.append(
-			f"""warehouse = {frappe.db.escape(warehouse)}
-				and item_code in ({", ".join(frappe.db.escape(item) for item in items)})"""
-		)
+		or_conditions.append((sle.warehouse == warehouse) & sle.item_code.isin(list(items)))
 
 	return or_conditions
 

erpnext/controllers/tests/test_sales_and_purchase_return.py

@@ -0,0 +1,39 @@
+# Copyright (c) 2025, Frappe Technologies Pvt. Ltd. and Contributors
+# See license.txt
+
+import frappe
+
+from erpnext.tests.utils import ERPNextTestSuite
+
+
+class TestSalesAndPurchaseReturn(ERPNextTestSuite):
+	@staticmethod
+	def _cancel_and_delete(doctype, name):
+		if not frappe.db.exists(doctype, name):
+			return
+		doc = frappe.get_doc(doctype, name)
+		if doc.docstatus == 1:
+			doc.cancel()
+		frappe.delete_doc(doctype, name, force=1)
+
+	def test_sales_return_validates_against_original(self):
+		# Submitting a return Delivery Note runs validate_returned_items (Item / Packed Item lookups
+		# via frappe.get_all) and get_already_returned_items (qb GROUP BY of the returned qty) -- both
+		# converted from raw SQL here. Exercises them on both engines.
+		from erpnext.stock.doctype.delivery_note.mapper import make_sales_return
+		from erpnext.stock.doctype.delivery_note.test_delivery_note import create_delivery_note
+		from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry
+
+		se = make_stock_entry(item_code="_Test Item", target="_Test Warehouse - _TC", qty=20, basic_rate=100)
+		self.addCleanup(self._cancel_and_delete, "Stock Entry", se.name)
+
+		dn = create_delivery_note(qty=5)
+		self.addCleanup(self._cancel_and_delete, "Delivery Note", dn.name)
+
+		return_dn = make_sales_return(dn.name)
+		return_dn.insert()
+		return_dn.submit()
+		self.addCleanup(self._cancel_and_delete, "Delivery Note", return_dn.name)
+
+		self.assertEqual(return_dn.is_return, 1)
+		self.assertEqual(return_dn.items[0].qty, -5)

erpnext/controllers/tests/test_stock_controller.py

@@ -0,0 +1,42 @@
+# Copyright (c) 2025, Frappe Technologies Pvt. Ltd. and Contributors
+# See license.txt
+
+import frappe
+from frappe.utils import add_days, today
+
+from erpnext.tests.utils import ERPNextTestSuite
+
+
+class TestStockControllerConversions(ERPNextTestSuite):
+	@staticmethod
+	def _cancel_and_delete(doctype, name):
+		if not frappe.db.exists(doctype, name):
+			return
+		doc = frappe.get_doc(doctype, name)
+		if doc.docstatus == 1:
+			doc.cancel()
+		frappe.delete_doc(doctype, name, force=1)
+
+	def test_future_sle_exists_detects_later_entries(self):
+		# future_sle_exists / get_conditions_to_validate_future_sle were converted to query builder
+		# (Count + Criterion.any). A later SLE for the same item+warehouse must be detected, which
+		# exercises the converted GROUP BY query on both engines.
+		from erpnext.controllers.stock_controller import future_sle_exists
+		from erpnext.stock.doctype.item.test_item import make_item
+		from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry
+
+		item = make_item("_Test Future SLE Item", {"is_stock_item": 1}).name
+		se = make_stock_entry(item_code=item, target="_Test Warehouse - _TC", qty=10, basic_rate=100)
+		self.addCleanup(self._cancel_and_delete, "Stock Entry", se.name)
+
+		# Pretend a different voucher posts a day earlier for the same item/warehouse: the existing
+		# (later) SLE must be reported as a future entry.
+		args = frappe._dict(
+			voucher_type="Stock Entry",
+			voucher_no="_TEST-NONEXISTENT-SE",
+			posting_date=add_days(today(), -1),
+			posting_time="00:00:00",
+		)
+		sl_entries = [frappe._dict(item_code=item, warehouse="_Test Warehouse - _TC")]
+
+		self.assertTrue(future_sle_exists(args, sl_entries))