Merge pull request #56124 from mihir-kandoi/pg-assets

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:

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",
 	)

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

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:

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"],
 	)
 
 

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

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)