refactor(postgres): port Assets module queries to the query builder

Convert the remaining raw frappe.db.sql in the Assets module to frappe.qb / ORM so
the queries run on PostgreSQL as well as MariaDB. Faithful 1:1 conversions -- no
MariaDB behaviour change:

- asset.py (gl-entry / bom-cost fetches), asset_maintenance.py (team members),
  asset_movement.py (latest location/custodian), location.py (get_children)
- fixed_asset_register.py: the depreciation-amount aggregate groups by asset.name
  (the primary key) selecting only asset.name + Sum(gle.debit), which is valid under
  Postgres strict GROUP BY (PK functional dependency)

Tests: existing asset (61), asset_maintenance, asset_movement and location suites
pass on both engines; adds a test for the previously-untested Fixed Asset Register
report (covers the GROUP BY aggregate on both engines).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Mihir Kandoi
2026-06-19 09:48:35 +05:30
parent facb27c3f4
commit fc9608d14d
6 changed files with 103 additions and 76 deletions

View File

@@ -735,12 +735,10 @@ class Asset(AccountsController):
frappe.throw(_("Asset cannot be cancelled, as it is already {0}").format(self.status)) frappe.throw(_("Asset cannot be cancelled, as it is already {0}").format(self.status))
def cancel_movement_entries(self): def cancel_movement_entries(self):
movements = frappe.db.sql( movements = frappe.get_all(
"""SELECT asm.name, asm.docstatus "Asset Movement Item",
FROM `tabAsset Movement` asm, `tabAsset Movement Item` asm_item filters={"asset": self.name, "docstatus": 1},
WHERE asm_item.parent=asm.name and asm_item.asset=%s and asm.docstatus=1""", fields=["parent as name"],
self.name,
as_dict=1,
) )
for movement in movements: for movement in movements:
@@ -860,15 +858,18 @@ class Asset(AccountsController):
cwip_enabled = is_cwip_accounting_enabled(self.asset_category) cwip_enabled = is_cwip_accounting_enabled(self.asset_category)
cwip_account = self.get_cwip_account(cwip_enabled=cwip_enabled) cwip_account = self.get_cwip_account(cwip_enabled=cwip_enabled)
query = """SELECT name FROM `tabGL Entry` WHERE voucher_no = %s and account = %s"""
if asset_bought_with_invoice: if asset_bought_with_invoice:
# with invoice purchase either expense or cwip has been booked # with invoice purchase either expense or cwip has been booked
expense_booked = frappe.db.sql(query, (purchase_document, fixed_asset_account), as_dict=1) expense_booked = frappe.db.exists(
"GL Entry", {"voucher_no": purchase_document, "account": fixed_asset_account}
)
if expense_booked: if expense_booked:
# if expense is already booked from invoice then do not make gl entries regardless of cwip enabled/disabled # if expense is already booked from invoice then do not make gl entries regardless of cwip enabled/disabled
return False return False
cwip_booked = frappe.db.sql(query, (purchase_document, cwip_account), as_dict=1) cwip_booked = frappe.db.exists(
"GL Entry", {"voucher_no": purchase_document, "account": cwip_account}
)
if cwip_booked: if cwip_booked:
# if cwip is booked from invoice then make gl entries regardless of cwip enabled/disabled # if cwip is booked from invoice then make gl entries regardless of cwip enabled/disabled
return True return True
@@ -878,10 +879,11 @@ class Asset(AccountsController):
# if cwip account isn't available do not make gl entries # if cwip account isn't available do not make gl entries
return False return False
cwip_booked = frappe.db.sql(query, (purchase_document, cwip_account), as_dict=1)
# if cwip is not booked from receipt then do not make gl entries # if cwip is not booked from receipt then do not make gl entries
# if cwip is booked from receipt then make gl entries # if cwip is booked from receipt then make gl entries
return cwip_booked return bool(
frappe.db.exists("GL Entry", {"voucher_no": purchase_document, "account": cwip_account})
)
def get_purchase_document(self): def get_purchase_document(self):
asset_bought_with_invoice = self.purchase_invoice and frappe.db.get_value( asset_bought_with_invoice = self.purchase_invoice and frappe.db.get_value(
@@ -1074,11 +1076,15 @@ def make_post_gl_entry():
for asset_category in asset_categories: for asset_category in asset_categories:
if cint(asset_category.enable_cwip_accounting): if cint(asset_category.enable_cwip_accounting):
assets = frappe.db.sql_list( assets = frappe.get_all(
""" select name from `tabAsset` "Asset",
where asset_category = %s and ifnull(booked_fixed_asset, 0) = 0 filters={
and available_for_use_date = %s and docstatus = 1""", "asset_category": asset_category.name,
(asset_category.name, nowdate()), "booked_fixed_asset": 0,
"available_for_use_date": nowdate(),
"docstatus": 1,
},
pluck="name",
) )
for asset in assets: for asset in assets:

View File

@@ -79,11 +79,14 @@ def assign_tasks(asset_maintenance_name, assign_to_member, maintenance_task, nex
"description": maintenance_task, "description": maintenance_task,
"date": next_due_date, "date": next_due_date,
} }
if not frappe.db.sql( if not frappe.db.exists(
"""select owner from `tabToDo` "ToDo",
where reference_type=%(doctype)s and reference_name=%(name)s and status='Open' {
and owner=%(assign_to)s""", "reference_type": args["doctype"],
args, "reference_name": args["name"],
"status": "Open",
"owner": args["assign_to"],
},
): ):
# assign_to function expects a list # assign_to function expects a list
args["assign_to"] = [args["assign_to"]] args["assign_to"] = [args["assign_to"]]
@@ -187,13 +190,9 @@ def get_team_members(
@frappe.whitelist() @frappe.whitelist()
def get_maintenance_log(asset_name: str): def get_maintenance_log(asset_name: str):
return frappe.db.sql( return frappe.get_all(
""" "Asset Maintenance Log",
select maintenance_status, count(asset_name) as count, asset_name filters={"asset_name": asset_name},
from `tabAsset Maintenance Log` fields=["maintenance_status", {"COUNT": "asset_name", "as": "count"}, "asset_name"],
where asset_name=%s group_by="maintenance_status, asset_name",
group by maintenance_status
""",
(asset_name,),
as_dict=1,
) )

View File

@@ -127,24 +127,20 @@ class AssetMovement(Document):
def get_latest_location_and_custodian(self, asset): def get_latest_location_and_custodian(self, asset):
current_location, current_employee = "", "" current_location, current_employee = "", ""
cond = "1=1"
# latest entry corresponds to current document's location, employee when transaction date > previous dates # latest entry corresponds to current document's location, employee when transaction date > previous dates
# In case of cancellation it corresponds to previous latest document's location, employee # In case of cancellation it corresponds to previous latest document's location, employee
args = {"asset": asset, "company": self.company} asm = frappe.qb.DocType("Asset Movement")
latest_movement_entry = frappe.db.sql( asm_item = frappe.qb.DocType("Asset Movement Item")
f""" latest_movement_entry = (
SELECT asm_item.target_location, asm_item.to_employee frappe.qb.from_(asm_item)
FROM `tabAsset Movement Item` asm_item .inner_join(asm)
JOIN `tabAsset Movement` asm ON asm_item.parent = asm.name .on(asm_item.parent == asm.name)
WHERE .select(asm_item.target_location, asm_item.to_employee)
asm_item.asset = %(asset)s AND .where((asm_item.asset == asset) & (asm.company == self.company) & (asm.docstatus == 1))
asm.company = %(company)s AND .orderby(asm.transaction_date, order=frappe.qb.desc)
asm.docstatus = 1 AND {cond} .limit(1)
ORDER BY asm.transaction_date DESC .run()
LIMIT 1
""",
args,
) )
if latest_movement_entry: if latest_movement_entry:

View File

@@ -215,17 +215,12 @@ def get_children(doctype: str, parent: str | None = None, location: str | None =
if parent is None or parent == "All Locations": if parent is None or parent == "All Locations":
parent = "" parent = ""
return frappe.db.sql( filters = {"parent_location": parent} if parent else {"parent_location": ["is", "not set"]}
f"""
select return frappe.get_all(
name as value, "Location",
is_group as expandable filters=filters,
from fields=["name as value", "is_group as expandable"],
`tabLocation` comp
where
ifnull(parent_location, "")={frappe.db.escape(parent)}
""",
as_dict=1,
) )

View File

@@ -395,32 +395,30 @@ def get_group_by_data(
def get_purchase_receipt_supplier_map(): def get_purchase_receipt_supplier_map():
pr = frappe.qb.DocType("Purchase Receipt")
pri = frappe.qb.DocType("Purchase Receipt Item")
return frappe._dict( return frappe._dict(
frappe.db.sql( frappe.qb.from_(pr)
""" Select .inner_join(pri)
pr.name, pr.supplier .on(pri.parent == pr.name)
FROM `tabPurchase Receipt` pr, `tabPurchase Receipt Item` pri .select(pr.name, pr.supplier)
WHERE .distinct()
pri.parent = pr.name .where((pri.is_fixed_asset == 1) & (pr.docstatus == 1) & (pr.is_return == 0))
AND pri.is_fixed_asset=1 .run()
AND pr.docstatus=1
AND pr.is_return=0"""
)
) )
def get_purchase_invoice_supplier_map(): def get_purchase_invoice_supplier_map():
pi = frappe.qb.DocType("Purchase Invoice")
pii = frappe.qb.DocType("Purchase Invoice Item")
return frappe._dict( return frappe._dict(
frappe.db.sql( frappe.qb.from_(pi)
""" Select .inner_join(pii)
pi.name, pi.supplier .on(pii.parent == pi.name)
FROM `tabPurchase Invoice` pi, `tabPurchase Invoice Item` pii .select(pi.name, pi.supplier)
WHERE .distinct()
pii.parent = pi.name .where((pii.is_fixed_asset == 1) & (pi.docstatus == 1) & (pi.is_return == 0))
AND pii.is_fixed_asset=1 .run()
AND pi.docstatus=1
AND pi.is_return=0"""
)
) )

View File

@@ -0,0 +1,33 @@
# Copyright (c) 2024, Frappe Technologies Pvt. Ltd. and Contributors
# License: GNU General Public License v3. See license.txt
import frappe
from erpnext.assets.doctype.asset.test_asset import AssetSetup, create_asset
from erpnext.assets.report.fixed_asset_register.fixed_asset_register import execute
class TestFixedAssetRegister(AssetSetup):
def test_report_lists_submitted_asset(self):
"""Exercises the report's converted queries -- including the depreciation aggregate that groups
by asset.name (must be valid on Postgres) -- by asserting a submitted asset is listed."""
asset = create_asset(
item_code="Macbook Pro",
purchase_date="2020-01-01",
available_for_use_date="2020-06-06",
location="Test Location",
submit=1,
)
filters = frappe._dict(
{
"company": "_Test Company",
"status": "In Location",
"filter_based_on": "Date Range",
"from_date": "2020-01-01",
"to_date": "2030-12-31",
"date_based_on": "Purchase Date",
}
)
data = execute(filters)[1]
asset_ids = {row.get("asset_id") for row in data}
self.assertIn(asset.name, asset_ids)