test: convert aggregate/pluck/positional raw SQL to ORM

A deeper re-audit (with an adversarial skeptic) of the queries left raw
in the prior commit found more that have exact ORM equivalents:

- scalar SUM/MAX  -> frappe.qb + Sum/Max .run()[0][0]
- SUM ... GROUP BY -> frappe.qb .groupby().select(Sum().as_()) run(as_dict)
- name IN (values) -> get_all(filters={'f': ['in', ...]})
- sql_list(select col) -> get_all(pluck='col')
- bulk UPDATE ... = NULL/value -> frappe.db.set_value(filters, field, val)
- positional as_list reads -> get_all(..., as_list=True) (+ sorted())

Note: get_value(dt, filters, 'sum(x)') and get_all(fields=['sum(x)'])
are rejected by frappe ('SQL functions are not allowed as strings'), so
aggregates go through frappe.qb. get_all(as_list=True) returns a tuple
(not a list), so consumers that mutate use sorted().

All affected test modules pass on MariaDB.
This commit is contained in:
Mihir Kandoi
2026-06-26 11:34:31 +05:30
parent 0776f7f7fa
commit 0583349ae4
11 changed files with 101 additions and 131 deletions

View File

@@ -2,6 +2,7 @@
# See license.txt
import frappe
from frappe.query_builder.functions import Sum
from frappe.utils import today
from erpnext.accounts.doctype.sales_invoice.test_sales_invoice import create_sales_invoice
@@ -63,13 +64,9 @@ class TestLoyaltyPointEntry(ERPNextTestSuite):
self.assertEqual(doc.loyalty_points, -7)
# Check balance
balance = frappe.db.sql(
"""
SELECT SUM(loyalty_points)
FROM `tabLoyalty Point Entry`
WHERE customer = %s
""",
(self.customer_name,),
)[0][0]
lpe = frappe.qb.DocType("Loyalty Point Entry")
balance = (
frappe.qb.from_(lpe).select(Sum(lpe.loyalty_points)).where(lpe.customer == self.customer_name)
).run()[0][0]
self.assertEqual(balance, 3) # 10 added, 7 redeemed

View File

@@ -3,6 +3,7 @@
import unittest
import frappe
from frappe.query_builder.functions import Sum
from frappe.utils import cint, flt, getdate, today
from erpnext.accounts.doctype.loyalty_program.loyalty_program import (
@@ -262,14 +263,12 @@ class TestLoyaltyProgram(ERPNextTestSuite):
def get_points_earned(self):
def get_returned_amount():
returned_amount = frappe.db.sql(
"""
select sum(grand_total)
from `tabSales Invoice`
where docstatus=1 and is_return=1 and ifnull(return_against, '')=%s
""",
self.name,
)
si = frappe.qb.DocType("Sales Invoice")
returned_amount = (
frappe.qb.from_(si)
.select(Sum(si.grand_total))
.where((si.docstatus == 1) & (si.is_return == 1) & (si.return_against == self.name))
).run()
return abs(flt(returned_amount[0][0])) if returned_amount else 0
lp_details = get_loyalty_program_details_with_points(

View File

@@ -79,7 +79,6 @@ class TestPOSProfile(ERPNextTestSuite):
def get_customers_list(pos_profile=None):
if pos_profile is None:
pos_profile = {}
cond = "1=1"
customer_groups = []
if pos_profile.get("customer_groups"):
# Get customers based on the customer groups defined in the POS profile
@@ -87,14 +86,16 @@ def get_customers_list(pos_profile=None):
customer_groups.extend(
[d.get("name") for d in get_child_nodes("Customer Group", d.get("customer_group"))]
)
cond = "customer_group in ({})".format(", ".join(["%s"] * len(customer_groups)))
filters = {"disabled": 0}
if customer_groups:
filters["customer_group"] = ["in", customer_groups]
return (
frappe.db.sql(
f""" select name, customer_name, customer_group, territory from tabCustomer where disabled = 0
and {cond}""",
tuple(customer_groups),
as_dict=1,
frappe.get_all(
"Customer",
filters=filters,
fields=["name", "customer_name", "customer_group", "territory"],
)
or {}
)

View File

@@ -91,7 +91,9 @@ class TestPricingRule(ERPNextTestSuite):
details = get_item_details(args)
self.assertEqual(details.get("discount_percentage"), 5)
frappe.db.sql("update `tabPricing Rule` set priority=NULL where campaign='_Test Campaign'")
frappe.db.set_value(
"Pricing Rule", {"campaign": "_Test Campaign"}, "priority", None, update_modified=False
)
from erpnext.accounts.doctype.pricing_rule.utils import MultiplePricingRuleConflict
self.assertRaises(MultiplePricingRuleConflict, get_item_details, args)

View File

@@ -238,22 +238,13 @@ class TestAssetRepair(ERPNextTestSuite):
submit=1,
)
gl_entries = frappe.db.sql(
"""
select
account,
sum(debit) as debit,
sum(credit) as credit
from `tabGL Entry`
where
voucher_type='Asset Repair'
and voucher_no=%s
group by
account
""",
asset_repair.name,
as_dict=1,
)
gle = frappe.qb.DocType("GL Entry")
gl_entries = (
frappe.qb.from_(gle)
.select(gle.account, Sum(gle.debit).as_("debit"), Sum(gle.credit).as_("credit"))
.where((gle.voucher_type == "Asset Repair") & (gle.voucher_no == asset_repair.name))
.groupby(gle.account)
).run(as_dict=True)
self.assertTrue(gl_entries)
@@ -287,22 +278,13 @@ class TestAssetRepair(ERPNextTestSuite):
submit=1,
)
gl_entries = frappe.db.sql(
"""
select
account,
sum(debit) as debit,
sum(credit) as credit
from `tabGL Entry`
where
voucher_type='Asset Repair'
and voucher_no=%s
group by
account
""",
asset_repair.name,
as_dict=1,
)
gle = frappe.qb.DocType("GL Entry")
gl_entries = (
frappe.qb.from_(gle)
.select(gle.account, Sum(gle.debit).as_("debit"), Sum(gle.credit).as_("credit"))
.where((gle.voucher_type == "Asset Repair") & (gle.voucher_no == asset_repair.name))
.groupby(gle.account)
).run(as_dict=True)
self.assertTrue(gl_entries)

View File

@@ -94,11 +94,12 @@ class TestAssetValueAdjustment(ERPNextTestSuite):
("_Test Fixed Asset - _TC", 0.0, 4625.29),
)
gle = frappe.db.sql(
"""select account, debit, credit from `tabGL Entry`
where voucher_type='Journal Entry' and voucher_no = %s
order by account""",
adj_doc.journal_entry,
gle = frappe.get_all(
"GL Entry",
filters={"voucher_type": "Journal Entry", "voucher_no": adj_doc.journal_entry},
fields=["account", "debit", "credit"],
order_by="account",
as_list=True,
)
self.assertSequenceEqual(gle, expected_gle)
@@ -184,11 +185,12 @@ class TestAssetValueAdjustment(ERPNextTestSuite):
("_Test Fixed Asset - _TC", 0.0, 5175.29),
)
gle = frappe.db.sql(
"""select account, debit, credit from `tabGL Entry`
where voucher_type='Journal Entry' and voucher_no = %s
order by account""",
adj_doc.journal_entry,
gle = frappe.get_all(
"GL Entry",
filters={"voucher_type": "Journal Entry", "voucher_no": adj_doc.journal_entry},
fields=["account", "debit", "credit"],
order_by="account",
as_list=True,
)
self.assertSequenceEqual(gle, expected_gle)

View File

@@ -881,12 +881,8 @@ def reset_item_valuation_rate(item_code, warehouse_list=None, qty=None, rate=Non
warehouse_list = [warehouse_list]
if not warehouse_list:
warehouse_list = frappe.db.sql_list(
"""
select warehouse from `tabBin`
where item_code=%s and actual_qty > 0
""",
item_code,
warehouse_list = frappe.get_all(
"Bin", filters={"item_code": item_code, "actual_qty": [">", 0]}, pluck="warehouse"
)
if not warehouse_list:

View File

@@ -7,6 +7,7 @@ from unittest.mock import patch
import frappe
import frappe.permissions
from frappe.core.doctype.user_permission.test_user_permission import create_user
from frappe.query_builder.functions import Sum
from frappe.tests import change_settings
from frappe.utils import add_days, flt, getdate, nowdate, today
@@ -907,10 +908,12 @@ class TestSalesOrder(ERPNextTestSuite):
item_doc.save()
else:
# update valid from
frappe.db.sql(
"""UPDATE `tabItem Tax` set valid_from = CURRENT_DATE
where parent = %(item)s and item_tax_template = %(tax)s""",
{"item": item, "tax": tax_template},
frappe.db.set_value(
"Item Tax",
{"parent": item, "item_tax_template": tax_template},
"valid_from",
today(),
update_modified=False,
)
so = make_sales_order(item_code=item, qty=1, do_not_save=1)
@@ -960,10 +963,12 @@ class TestSalesOrder(ERPNextTestSuite):
self.assertEqual(so.taxes[1].total, 480)
# teardown
frappe.db.sql(
"""UPDATE `tabItem Tax` set valid_from = NULL
where parent = %(item)s and item_tax_template = %(tax)s""",
{"item": item, "tax": tax_template},
frappe.db.set_value(
"Item Tax",
{"parent": item, "item_tax_template": tax_template},
"valid_from",
None,
update_modified=False,
)
so.cancel()
so.delete()
@@ -1559,10 +1564,12 @@ class TestSalesOrder(ERPNextTestSuite):
# Check if Work Orders were raised
for item in so_item_name:
wo_qty = frappe.db.sql(
"select sum(qty) from `tabWork Order` where sales_order=%s and sales_order_item=%s",
(so.name, item),
)
wo = frappe.qb.DocType("Work Order")
wo_qty = (
frappe.qb.from_(wo)
.select(Sum(wo.qty))
.where((wo.sales_order == so.name) & (wo.sales_order_item == item))
).run()
self.assertEqual(wo_qty[0][0], so_item_name.get(item))
def test_advance_payment_entry_unlink_against_sales_order(self):

View File

@@ -2,6 +2,7 @@
# License: GNU General Public License v3. See license.txt
import frappe
from frappe.query_builder.functions import Max
from frappe.utils.nestedset import (
NestedSetChildExistsError,
NestedSetInvalidMergeError,
@@ -20,7 +21,8 @@ class TestItemGroup(ERPNextTestSuite):
def test_basic_tree(self, records=None):
min_lft = 1
max_rgt = frappe.db.sql("select max(rgt) from `tabItem Group`")[0][0]
ig = frappe.qb.DocType("Item Group")
max_rgt = frappe.qb.from_(ig).select(Max(ig.rgt)).run()[0][0]
if not records:
records = self.globalTestRecords["Item Group"][2:]
@@ -131,12 +133,7 @@ class TestItemGroup(ERPNextTestSuite):
frappe.db.get_value("Item Group", parent_item_group, "rgt")
ancestors = get_ancestors_of("Item Group", "_Test Item Group B - 3")
ancestors = frappe.db.sql(
"""select name, rgt from `tabItem Group`
where name in ({})""".format(", ".join(["%s"] * len(ancestors))),
tuple(ancestors),
as_dict=True,
)
ancestors = frappe.get_all("Item Group", filters={"name": ["in", ancestors]}, fields=["name", "rgt"])
frappe.delete_doc("Item Group", "_Test Item Group B - 3")
records_to_test = self.globalTestRecords["Item Group"][2:]
@@ -168,9 +165,8 @@ class TestItemGroup(ERPNextTestSuite):
self.test_basic_tree()
# move its children back
for name in frappe.db.sql_list(
"""select name from `tabItem Group`
where parent_item_group='_Test Item Group C'"""
for name in frappe.get_all(
"Item Group", filters={"parent_item_group": "_Test Item Group C"}, pluck="name"
):
doc = frappe.get_doc("Item Group", name)
doc.parent_item_group = "_Test Item Group B"
@@ -218,11 +214,7 @@ class TestItemGroup(ERPNextTestSuite):
def get_no_of_children(item_groups, no_of_children):
children = []
for ig in item_groups:
children += frappe.db.sql_list(
"""select name from `tabItem Group`
where ifnull(parent_item_group, '')=%s""",
ig or "",
)
children += frappe.get_all("Item Group", filters={"parent_item_group": ig}, pluck="name")
if len(children):
return get_no_of_children(children, no_of_children + len(children))

View File

@@ -40,19 +40,12 @@ from erpnext.tests.utils import ERPNextTestSuite
def get_sle(**args):
condition, values = "", []
for key, value in args.items():
condition += " and " if condition else " where "
condition += f"`{key}`=%s"
values.append(value)
return frappe.db.sql(
# posting_datetime is the precomputed date+time column; MySQL-only timestamp(date,time) errors on Postgres
"""select * from `tabStock Ledger Entry` %s
order by posting_datetime desc, creation desc limit 1"""
% condition,
values,
as_dict=1,
return frappe.get_all(
"Stock Ledger Entry",
filters=args,
fields=["*"],
order_by="posting_datetime desc, creation desc",
limit=1,
)
@@ -581,15 +574,15 @@ class TestStockEntry(ERPNextTestSuite):
expected_sle.sort(key=lambda x: x[1])
# check stock ledger entries
sle = frappe.db.sql(
"""select item_code, warehouse, actual_qty
from `tabStock Ledger Entry` where voucher_type = %s
and voucher_no = %s order by item_code, warehouse, actual_qty""",
(voucher_type, voucher_no),
as_list=1,
sle = frappe.get_all(
"Stock Ledger Entry",
filters={"voucher_type": voucher_type, "voucher_no": voucher_no},
fields=["item_code", "warehouse", "actual_qty"],
order_by="item_code, warehouse, actual_qty",
as_list=True,
)
self.assertTrue(sle)
sle.sort(key=lambda x: x[1])
sle = sorted(sle, key=lambda x: x[1])
for i, sle_value in enumerate(sle):
self.assertEqual(expected_sle[i][0], sle_value[0])
@@ -599,16 +592,16 @@ class TestStockEntry(ERPNextTestSuite):
def check_gl_entries(self, voucher_type, voucher_no, expected_gl_entries):
expected_gl_entries.sort(key=lambda x: x[0])
gl_entries = frappe.db.sql(
"""select account, debit, credit
from `tabGL Entry` where voucher_type=%s and voucher_no=%s
order by account asc, debit asc""",
(voucher_type, voucher_no),
as_list=1,
gl_entries = frappe.get_all(
"GL Entry",
filters={"voucher_type": voucher_type, "voucher_no": voucher_no},
fields=["account", "debit", "credit"],
order_by="account asc, debit asc",
as_list=True,
)
self.assertTrue(gl_entries)
gl_entries.sort(key=lambda x: x[0])
gl_entries = sorted(gl_entries, key=lambda x: x[0])
for i, gle in enumerate(gl_entries):
self.assertEqual(expected_gl_entries[i][0], gle[0])
self.assertEqual(expected_gl_entries[i][1], gle[1])

View File

@@ -85,11 +85,10 @@ class TestStockReconciliation(ERPNextTestSuite, StockTestMixin):
)
# check stock value
sle = frappe.db.sql(
"""select * from `tabStock Ledger Entry`
where voucher_type='Stock Reconciliation' and voucher_no=%s""",
stock_reco.name,
as_dict=1,
sle = frappe.get_all(
"Stock Ledger Entry",
filters={"voucher_type": "Stock Reconciliation", "voucher_no": stock_reco.name},
fields=["qty_after_transaction", "stock_value"],
)
qty_after_transaction = flt(d[0]) if d[0] != "" else flt(last_sle.get("qty_after_transaction"))