diff --git a/erpnext/assets/doctype/asset/asset.py b/erpnext/assets/doctype/asset/asset.py index 1c9f3871d47..1f0bee0f021 100644 --- a/erpnext/assets/doctype/asset/asset.py +++ b/erpnext/assets/doctype/asset/asset.py @@ -735,12 +735,16 @@ class Asset(AccountsController): frappe.throw(_("Asset cannot be cancelled, as it is already {0}").format(self.status)) def cancel_movement_entries(self): - movements = frappe.db.sql( - """SELECT asm.name, asm.docstatus - FROM `tabAsset Movement` asm, `tabAsset Movement Item` asm_item - WHERE asm_item.parent=asm.name and asm_item.asset=%s and asm.docstatus=1""", - self.name, - as_dict=1, + # filter the parent Asset Movement's docstatus (as the original SQL did), not the child row's + asm = frappe.qb.DocType("Asset Movement") + asm_item = frappe.qb.DocType("Asset Movement Item") + movements = ( + frappe.qb.from_(asm_item) + .inner_join(asm) + .on(asm_item.parent == asm.name) + .select(asm.name) + .where((asm_item.asset == self.name) & (asm.docstatus == 1)) + .run(as_dict=True) ) for movement in movements: @@ -860,15 +864,18 @@ class Asset(AccountsController): cwip_enabled = is_cwip_accounting_enabled(self.asset_category) 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: # 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 is already booked from invoice then do not make gl entries regardless of cwip enabled/disabled 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 is booked from invoice then make gl entries regardless of cwip enabled/disabled return True @@ -878,10 +885,11 @@ class Asset(AccountsController): # if cwip account isn't available do not make gl entries 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 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): asset_bought_with_invoice = self.purchase_invoice and frappe.db.get_value( @@ -1074,11 +1082,15 @@ def make_post_gl_entry(): for asset_category in asset_categories: if cint(asset_category.enable_cwip_accounting): - assets = frappe.db.sql_list( - """ select name from `tabAsset` - where asset_category = %s and ifnull(booked_fixed_asset, 0) = 0 - and available_for_use_date = %s and docstatus = 1""", - (asset_category.name, nowdate()), + assets = frappe.get_all( + "Asset", + filters={ + "asset_category": asset_category.name, + "booked_fixed_asset": 0, + "available_for_use_date": nowdate(), + "docstatus": 1, + }, + pluck="name", ) for asset in assets: diff --git a/erpnext/assets/doctype/asset_maintenance/asset_maintenance.py b/erpnext/assets/doctype/asset_maintenance/asset_maintenance.py index 76b2047812b..9c2b7682ade 100644 --- a/erpnext/assets/doctype/asset_maintenance/asset_maintenance.py +++ b/erpnext/assets/doctype/asset_maintenance/asset_maintenance.py @@ -79,11 +79,14 @@ def assign_tasks(asset_maintenance_name, assign_to_member, maintenance_task, nex "description": maintenance_task, "date": next_due_date, } - if not frappe.db.sql( - """select owner from `tabToDo` - where reference_type=%(doctype)s and reference_name=%(name)s and status='Open' - and owner=%(assign_to)s""", - args, + if not frappe.db.exists( + "ToDo", + { + "reference_type": args["doctype"], + "reference_name": args["name"], + "status": "Open", + "owner": args["assign_to"], + }, ): # assign_to function expects a list args["assign_to"] = [args["assign_to"]] @@ -187,13 +190,9 @@ def get_team_members( @frappe.whitelist() def get_maintenance_log(asset_name: str): - return frappe.db.sql( - """ - select maintenance_status, count(asset_name) as count, asset_name - from `tabAsset Maintenance Log` - where asset_name=%s - group by maintenance_status - """, - (asset_name,), - as_dict=1, + return frappe.get_all( + "Asset Maintenance Log", + filters={"asset_name": asset_name}, + fields=["maintenance_status", {"COUNT": "asset_name", "as": "count"}, "asset_name"], + group_by="maintenance_status, asset_name", ) diff --git a/erpnext/assets/doctype/asset_maintenance/test_asset_maintenance.py b/erpnext/assets/doctype/asset_maintenance/test_asset_maintenance.py index 068c6cff9c9..7411349b029 100644 --- a/erpnext/assets/doctype/asset_maintenance/test_asset_maintenance.py +++ b/erpnext/assets/doctype/asset_maintenance/test_asset_maintenance.py @@ -18,6 +18,36 @@ class TestAssetMaintenance(ERPNextTestSuite): self.asset_name = frappe.db.get_value("Asset", {"purchase_receipt": self.pr.name}, "name") self.asset_doc = frappe.get_doc("Asset", self.asset_name) + def test_get_maintenance_log_counts_by_status(self): + """get_maintenance_log uses a v16 dict aggregate field spec + ({"COUNT": "asset_name", "as": "count"}); confirm it runs and returns correct per-status counts + on both engines (the whitelisted endpoint was previously untested).""" + from erpnext.assets.doctype.asset_maintenance.asset_maintenance import get_maintenance_log + + self.asset_doc.available_for_use_date = nowdate() + self.asset_doc.purchase_date = nowdate() + self.asset_doc.save() + + frappe.get_doc( + { + "doctype": "Asset Maintenance", + "asset_name": self.asset_name, + "maintenance_team": "Team Awesome", + "company": "_Test Company", + "asset_maintenance_tasks": get_maintenance_tasks(), + } + ).insert() + + rows = get_maintenance_log(self.asset_name) + # the dict aggregate spec did not crash and returned grouped rows... + self.assertTrue(rows) + self.assertTrue(all("maintenance_status" in r for r in rows)) + # ...and the per-status counts sum to the total number of logs for this asset + self.assertEqual( + sum(r["count"] for r in rows), + frappe.db.count("Asset Maintenance Log", {"asset_name": self.asset_name}), + ) + def test_create_asset_maintenance_with_log(self): month_end_date = get_last_day(nowdate()) diff --git a/erpnext/assets/doctype/asset_movement/asset_movement.py b/erpnext/assets/doctype/asset_movement/asset_movement.py index c3248563440..674be5c65b3 100644 --- a/erpnext/assets/doctype/asset_movement/asset_movement.py +++ b/erpnext/assets/doctype/asset_movement/asset_movement.py @@ -127,24 +127,20 @@ class AssetMovement(Document): def get_latest_location_and_custodian(self, asset): current_location, current_employee = "", "" - cond = "1=1" # 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 - args = {"asset": asset, "company": self.company} - latest_movement_entry = frappe.db.sql( - f""" - SELECT asm_item.target_location, asm_item.to_employee - FROM `tabAsset Movement Item` asm_item - JOIN `tabAsset Movement` asm ON asm_item.parent = asm.name - WHERE - asm_item.asset = %(asset)s AND - asm.company = %(company)s AND - asm.docstatus = 1 AND {cond} - ORDER BY asm.transaction_date DESC - LIMIT 1 - """, - args, + asm = frappe.qb.DocType("Asset Movement") + asm_item = frappe.qb.DocType("Asset Movement Item") + latest_movement_entry = ( + frappe.qb.from_(asm_item) + .inner_join(asm) + .on(asm_item.parent == asm.name) + .select(asm_item.target_location, asm_item.to_employee) + .where((asm_item.asset == asset) & (asm.company == self.company) & (asm.docstatus == 1)) + .orderby(asm.transaction_date, order=frappe.qb.desc) + .limit(1) + .run() ) if latest_movement_entry: diff --git a/erpnext/assets/doctype/location/location.py b/erpnext/assets/doctype/location/location.py index c2c3d5a2f1e..c6c999c4dbd 100644 --- a/erpnext/assets/doctype/location/location.py +++ b/erpnext/assets/doctype/location/location.py @@ -215,17 +215,12 @@ def get_children(doctype: str, parent: str | None = None, location: str | None = if parent is None or parent == "All Locations": parent = "" - return frappe.db.sql( - f""" - select - name as value, - is_group as expandable - from - `tabLocation` comp - where - ifnull(parent_location, "")={frappe.db.escape(parent)} - """, - as_dict=1, + filters = {"parent_location": parent} if parent else {"parent_location": ["is", "not set"]} + + return frappe.get_all( + "Location", + filters=filters, + fields=["name as value", "is_group as expandable"], ) diff --git a/erpnext/assets/report/fixed_asset_register/fixed_asset_register.py b/erpnext/assets/report/fixed_asset_register/fixed_asset_register.py index 9b2cadaabe5..1dfa24da40c 100644 --- a/erpnext/assets/report/fixed_asset_register/fixed_asset_register.py +++ b/erpnext/assets/report/fixed_asset_register/fixed_asset_register.py @@ -395,32 +395,30 @@ def get_group_by_data( def get_purchase_receipt_supplier_map(): + pr = frappe.qb.DocType("Purchase Receipt") + pri = frappe.qb.DocType("Purchase Receipt Item") return frappe._dict( - frappe.db.sql( - """ Select - pr.name, pr.supplier - FROM `tabPurchase Receipt` pr, `tabPurchase Receipt Item` pri - WHERE - pri.parent = pr.name - AND pri.is_fixed_asset=1 - AND pr.docstatus=1 - AND pr.is_return=0""" - ) + frappe.qb.from_(pr) + .inner_join(pri) + .on(pri.parent == pr.name) + .select(pr.name, pr.supplier) + .distinct() + .where((pri.is_fixed_asset == 1) & (pr.docstatus == 1) & (pr.is_return == 0)) + .run() ) def get_purchase_invoice_supplier_map(): + pi = frappe.qb.DocType("Purchase Invoice") + pii = frappe.qb.DocType("Purchase Invoice Item") return frappe._dict( - frappe.db.sql( - """ Select - pi.name, pi.supplier - FROM `tabPurchase Invoice` pi, `tabPurchase Invoice Item` pii - WHERE - pii.parent = pi.name - AND pii.is_fixed_asset=1 - AND pi.docstatus=1 - AND pi.is_return=0""" - ) + frappe.qb.from_(pi) + .inner_join(pii) + .on(pii.parent == pi.name) + .select(pi.name, pi.supplier) + .distinct() + .where((pii.is_fixed_asset == 1) & (pi.docstatus == 1) & (pi.is_return == 0)) + .run() ) diff --git a/erpnext/assets/report/fixed_asset_register/test_fixed_asset_register.py b/erpnext/assets/report/fixed_asset_register/test_fixed_asset_register.py new file mode 100644 index 00000000000..ee91c77f919 --- /dev/null +++ b/erpnext/assets/report/fixed_asset_register/test_fixed_asset_register.py @@ -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)