refactor(controllers): convert sales/purchase return lookups to qb/ORM

validate_returned_items used a raw frappe.db.sql with a string-built column
list (and a separate Packed Item select); get_already_returned_items used a
raw GROUP BY sum. Convert both to frappe.get_all / frappe.qb (Sum(Abs(...))
with an explicit groupby). The qb GROUP BY mirrors the original
`group by item_code, <field>`, so it is parity-preserving (not a behaviour
change) and valid on Postgres.

Surgical re-apply: develop's `is_debit_note = 0` credit-note fix in
make_return_doc is preserved (the staging branch predated and would have
reverted it).

Adds a test (Delivery Note -> sales return) exercising validate_returned_items
and get_already_returned_items on both engines.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Mihir Kandoi
2026-06-21 05:23:31 +05:30
parent 4255059846
commit 465446bb79
3 changed files with 93 additions and 53 deletions

View File

@@ -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 = {}

View File

@@ -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):
@@ -632,7 +630,7 @@ def check_item_quality_inspection(doctype: str, docstatus: str | int, items: str
inspection_fieldname = INSPECTION_FIELDNAME_MAP.get(doctype)
if inspection_fieldname is None:
return items if doctype == "Stock Entry" else []
return []
allow_after_transaction = cint(docstatus) == 1 and frappe.get_single_value(
"Stock Settings", "allow_to_make_quality_inspection_after_purchase_or_delivery"
@@ -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

View File

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