Merge pull request #56130 from mihir-kandoi/pg-support

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

erpnext/support/doctype/issue/issue.py

@@ -266,7 +266,7 @@ def has_website_permission(doc, ptype, user, verbose=False):
 
 def update_issue(contact, method):
 	"""Called when Contact is deleted"""
-	frappe.db.sql("""UPDATE `tabIssue` set contact='' where contact=%s""", contact.name)
+	frappe.db.set_value("Issue", {"contact": contact.name}, "contact", "")
 
 
 @frappe.whitelist()

erpnext/support/doctype/warranty_claim/test_warranty_claim.py

@@ -1,8 +1,103 @@
 # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors and Contributors
 # See license.txt
 
+import frappe
+from frappe.utils.data import today
+
+from erpnext.support.doctype.warranty_claim.warranty_claim import make_maintenance_visit
 from erpnext.tests.utils import ERPNextTestSuite
 
 
 class TestWarrantyClaim(ERPNextTestSuite):
-	pass
+	def make_warranty_claim(self):
+		# Warranty Claim is not a submittable doctype; it stays at docstatus 0.
+		claim = frappe.new_doc("Warranty Claim")
+		claim.status = "Open"
+		claim.complaint_date = today()
+		claim.customer = "_Test Customer"
+		claim.item_code = "_Test Item"
+		claim.complaint = "Device stopped working under warranty"
+		claim.company = "_Test Company"
+		claim.insert(ignore_permissions=True)
+		return claim
+
+	def make_maintenance_visit_for_claim(self, claim, completion_status):
+		visit = frappe.new_doc("Maintenance Visit")
+		visit.company = "_Test Company"
+		visit.customer = "_Test Customer"
+		visit.mntc_date = today()
+		visit.maintenance_type = "Unscheduled"
+		visit.completion_status = completion_status
+		visit.append(
+			"purposes",
+			{
+				"item_code": "_Test Item",
+				"service_person": "_Test Sales Person",
+				"work_done": "Replaced the faulty component",
+				"description": "Warranty repair",
+				"prevdoc_doctype": "Warranty Claim",
+				"prevdoc_docname": claim.name,
+			},
+		)
+		visit.insert(ignore_permissions=True)
+		visit.submit()
+		return visit
+
+	def test_make_maintenance_visit_maps_new_visit_when_none_completed(self):
+		# No "Fully Completed" visit yet -> converted query returns nothing,
+		# so a fresh Maintenance Visit draft is mapped from the claim.
+		claim = self.make_warranty_claim()
+
+		target = make_maintenance_visit(claim.name)
+
+		self.assertIsNotNone(target)
+		self.assertEqual(target.doctype, "Maintenance Visit")
+		self.assertTrue(target.is_new())
+		# item_code present -> a purpose row is mapped and back-linked to the claim
+		self.assertEqual(len(target.purposes), 1)
+		row = target.purposes[0]
+		self.assertEqual(row.item_code, "_Test Item")
+		self.assertEqual(row.prevdoc_doctype, "Warranty Claim")
+		self.assertEqual(row.prevdoc_docname, claim.name)
+
+	def test_make_maintenance_visit_returns_none_when_fully_completed_exists(self):
+		# A submitted, "Fully Completed" visit pointing at the claim must be
+		# found by the converted join query -> no new visit is mapped.
+		claim = self.make_warranty_claim()
+		visit = self.make_maintenance_visit_for_claim(claim, "Fully Completed")
+
+		# Sanity: the seeded visit really is the one the query should match.
+		self.assertEqual(visit.docstatus, 1)
+		self.assertEqual(visit.completion_status, "Fully Completed")
+		self.assertEqual(visit.purposes[0].prevdoc_docname, claim.name)
+
+		self.assertIsNone(make_maintenance_visit(claim.name))
+
+	def test_make_maintenance_visit_ignores_partially_completed(self):
+		# A "Partially Completed" visit must NOT satisfy the query, so a new
+		# visit is still mapped (the completion_status filter is exercised).
+		claim = self.make_warranty_claim()
+		self.make_maintenance_visit_for_claim(claim, "Partially Completed")
+
+		target = make_maintenance_visit(claim.name)
+
+		self.assertIsNotNone(target)
+		self.assertTrue(target.is_new())
+		self.assertEqual(target.doctype, "Maintenance Visit")
+
+	def test_on_cancel_blocked_by_active_maintenance_visit(self):
+		# on_cancel's converted query joins Maintenance Visit Purpose -> Maintenance Visit and
+		# filters the PARENT visit's docstatus != 2; a submitted (non-cancelled) visit referencing
+		# the claim must block cancellation.
+		claim = self.make_warranty_claim()
+		self.make_maintenance_visit_for_claim(claim, "Partially Completed")
+
+		self.assertRaises(frappe.ValidationError, claim.on_cancel)
+
+	def test_on_cancel_allowed_when_no_active_visit(self):
+		# No referencing visit -> the query returns nothing -> the claim is marked Cancelled.
+		claim = self.make_warranty_claim()
+
+		claim.on_cancel()
+
+		self.assertEqual(frappe.db.get_value("Warranty Claim", claim.name, "status"), "Cancelled")

erpnext/support/doctype/warranty_claim/warranty_claim.py

@@ -62,14 +62,20 @@ class WarrantyClaim(TransactionBase):
 			self.resolution_date = now_datetime()
 
 	def on_cancel(self):
-		lst = frappe.db.sql(
-			"""select t1.name
-			from `tabMaintenance Visit` t1, `tabMaintenance Visit Purpose` t2
-			where t2.parent = t1.name and t2.prevdoc_docname = %s and	t1.docstatus!=2""",
-			(self.name),
+		mv = frappe.qb.DocType("Maintenance Visit")
+		mvp = frappe.qb.DocType("Maintenance Visit Purpose")
+		# filter the parent Maintenance Visit's docstatus (as the original SQL did), not the child row's
+		visits = (
+			frappe.qb.from_(mvp)
+			.inner_join(mv)
+			.on(mvp.parent == mv.name)
+			.select(mv.name)
+			.where((mvp.prevdoc_docname == self.name) & (mv.docstatus != 2))
+			.limit(500)
+			.run()
 		)
-		if lst:
-			lst1 = ",".join(x[0] for x in lst)
+		if visits:
+			lst1 = ",".join(x[0] for x in visits)
 			frappe.throw(_("Cancel Material Visit {0} before cancelling this Warranty Claim").format(lst1))
 		else:
 			self.db_set("status", "Cancelled")
@@ -86,12 +92,19 @@ def make_maintenance_visit(source_name: str, target_doc: str | Document | None =
 		target_doc.prevdoc_doctype = source_parent.doctype
 		target_doc.prevdoc_docname = source_parent.name
 
-	visit = frappe.db.sql(
-		"""select t1.name
-		from `tabMaintenance Visit` t1, `tabMaintenance Visit Purpose` t2
-		where t2.parent=t1.name and t2.prevdoc_docname=%s
-		and t1.docstatus=1 and t1.completion_status='Fully Completed'""",
-		source_name,
+	mv = frappe.qb.DocType("Maintenance Visit")
+	mvp = frappe.qb.DocType("Maintenance Visit Purpose")
+	visit = (
+		frappe.qb.from_(mv)
+		.inner_join(mvp)
+		.on(mvp.parent == mv.name)
+		.select(mv.name)
+		.where(
+			(mvp.prevdoc_docname == source_name)
+			& (mv.docstatus == 1)
+			& (mv.completion_status == "Fully Completed")
+		)
+		.run()
 	)
 
 	if not visit:

erpnext/support/report/first_response_time_for_issues/first_response_time_for_issues.py

@@ -4,6 +4,7 @@
 
 import frappe
 from frappe import _
+from frappe.query_builder.functions import Avg, Date
 
 
 def execute(filters=None):
@@ -17,19 +18,19 @@ def execute(filters=None):
 		},
 	]
 
-	data = frappe.db.sql(
-		"""
-		SELECT
-			date(creation) as creation_date,
-			avg(first_response_time) as avg_response_time
-		FROM tabIssue
-		WHERE
-			date(creation) between %s and %s
-			and first_response_time > 0
-		GROUP BY creation_date
-		ORDER BY creation_date desc
-	""",
-		(filters.from_date, filters.to_date),
+	issue = frappe.qb.DocType("Issue")
+	data = (
+		frappe.qb.from_(issue)
+		.select(
+			Date(issue.creation).as_("creation_date"),
+			Avg(issue.first_response_time).as_("avg_response_time"),
+		)
+		.where(
+			Date(issue.creation).between(filters.from_date, filters.to_date) & (issue.first_response_time > 0)
+		)
+		.groupby(Date(issue.creation))
+		.orderby(Date(issue.creation), order=frappe.qb.desc)
+		.run()
 	)
 
 	return columns, data

erpnext/support/report/first_response_time_for_issues/test_first_response_time_for_issues.py

@@ -0,0 +1,50 @@
+# Copyright (c) 2013, Frappe Technologies Pvt. Ltd. and contributors
+# For license information, please see license.txt
+
+import frappe
+from frappe.utils import add_days, getdate, now_datetime
+
+from erpnext.support.report.first_response_time_for_issues.first_response_time_for_issues import (
+	execute,
+)
+from erpnext.tests.utils import ERPNextTestSuite
+
+
+class TestFirstResponseTimeForIssues(ERPNextTestSuite):
+	def test_avg_first_response_time_grouped_by_creation_date(self):
+		today = getdate()
+
+		# Isolate today's group: any pre-existing Issue created today (from other
+		# fixtures running in the same transaction) would pollute the average.
+		# Rolled back automatically in tearDown.
+		frappe.db.delete("Issue", {"creation": (">=", today)})
+
+		# Seed exactly one Issue created today.
+		issue = frappe.get_doc(
+			{
+				"doctype": "Issue",
+				"subject": "First Response Time Report Issue",
+				"raised_by": "test_frt_report@example.com",
+				"description": "First Response Time Report Issue",
+				"company": "_Test Company",
+				"creation": now_datetime(),
+			}
+		).insert(ignore_permissions=True)
+
+		# first_response_time is a read-only computed Duration (seconds); set directly.
+		response_time = 3600
+		frappe.db.set_value("Issue", issue.name, "first_response_time", response_time)
+
+		columns, data = execute(frappe._dict(from_date=add_days(today, -1), to_date=add_days(today, 1)))
+
+		# Rows are tuples: (creation_date, avg_response_time) -- report uses .run() w/o as_dict.
+		rows_for_today = [row for row in data if getdate(row[0]) == today]
+		self.assertEqual(
+			len(rows_for_today),
+			1,
+			f"expected exactly one grouped row for {today}, got {rows_for_today}",
+		)
+
+		creation_date, avg_response_time = rows_for_today[0]
+		self.assertEqual(getdate(creation_date), today)
+		self.assertEqual(float(avg_response_time), float(response_time))

erpnext/support/report/issue_summary/issue_summary.py

@@ -6,6 +6,7 @@ import json
 
 import frappe
 from frappe import _, scrub
+from frappe.query_builder.functions import Avg
 from frappe.utils import flt
 
 
@@ -264,21 +265,20 @@ class IssueSummary:
 						self.issue_summary_data[entry][metric] /= flt(assignment_map.get(entry))
 
 			else:
-				data = frappe.db.sql(
-					f"""
-					SELECT
-						{field}, AVG(first_response_time) as avg_frt,
-						AVG(avg_response_time) as avg_resp_time,
-						AVG(total_hold_time) as avg_hold_time,
-						AVG(resolution_time) as avg_resolution_time,
-						AVG(user_resolution_time) as avg_user_resolution_time
-					FROM `tabIssue`
-					WHERE
-						name IN %(issues)s
-					GROUP BY {field}
-				""",
-					{"issues": issues},
-					as_dict=1,
+				issue = frappe.qb.DocType("Issue")
+				data = (
+					frappe.qb.from_(issue)
+					.select(
+						issue[field],
+						Avg(issue.first_response_time).as_("avg_frt"),
+						Avg(issue.avg_response_time).as_("avg_resp_time"),
+						Avg(issue.total_hold_time).as_("avg_hold_time"),
+						Avg(issue.resolution_time).as_("avg_resolution_time"),
+						Avg(issue.user_resolution_time).as_("avg_user_resolution_time"),
+					)
+					.where(issue.name.isin(issues))
+					.groupby(issue[field])
+					.run(as_dict=1)
 				)
 
 				for entry in data:

erpnext/support/report/issue_summary/test_issue_summary.py

@@ -0,0 +1,49 @@
+# Copyright (c) 2024, Frappe Technologies Pvt. Ltd. and contributors
+# For license information, please see license.txt
+
+import frappe
+from frappe.utils import add_days, today
+
+from erpnext.support.report.issue_summary.issue_summary import execute
+from erpnext.tests.utils import ERPNextTestSuite
+
+
+class TestIssueSummary(ERPNextTestSuite):
+	def test_count_grouped_by_issue_priority(self):
+		# Unique Issue Priority so this group is isolated from any pre-existing data.
+		priority = "__Test Issue Summary Priority"
+		if not frappe.db.exists("Issue Priority", priority):
+			frappe.get_doc({"doctype": "Issue Priority", "name": priority}).insert()
+
+		opening_date = today()
+		for subject in ("__Test Issue Summary A", "__Test Issue Summary B"):
+			frappe.get_doc(
+				{
+					"doctype": "Issue",
+					"subject": subject,
+					"priority": priority,
+					"status": "Open",
+					"opening_date": opening_date,
+				}
+			).insert()
+
+		filters = frappe._dict(
+			{
+				"based_on": "Issue Priority",
+				"from_date": add_days(opening_date, -1),
+				"to_date": add_days(opening_date, 1),
+			}
+		)
+
+		columns, data, _msg, _chart, _summary = execute(filters)
+
+		# get_rows() emits one row per group keyed on "priority" for based_on == "Issue Priority".
+		seeded_row = next((row for row in data if row.get("priority") == priority), None)
+		self.assertIsNotNone(
+			seeded_row,
+			f"expected a summary row for priority {priority!r}, got {[r.get('priority') for r in data]}",
+		)
+
+		# Two seeded Open issues -> total_issues == 2 and the "open" status bucket == 2.
+		self.assertEqual(seeded_row["total_issues"], 2)
+		self.assertEqual(seeded_row["open"], 2)

erpnext/support/report/support_hour_distribution/support_hour_distribution.py

@@ -51,17 +51,7 @@ def get_data(filters):
 
 
 def get_hours_count(start_time, end_time):
-	data = (
-		frappe.db.sql(
-			""" select count(*) from `tabIssue` where creation
-		between %(start_time)s and %(end_time)s""",
-			{"start_time": start_time, "end_time": end_time},
-			as_list=1,
-		)
-		or []
-	)
-
-	return data[0][0] if len(data) > 0 else 0
+	return frappe.db.count("Issue", {"creation": ["between", [start_time, end_time]]})
 
 
 def get_columns():

erpnext/support/report/support_hour_distribution/test_support_hour_distribution.py

@@ -0,0 +1,53 @@
+import frappe
+from frappe.utils import get_datetime, getdate
+
+from erpnext.support.doctype.issue.test_issue import make_issue
+from erpnext.support.report.support_hour_distribution.support_hour_distribution import execute
+from erpnext.tests.utils import ERPNextTestSuite
+
+
+class TestSupportHourDistribution(ERPNextTestSuite):
+	def test_issue_buckets_into_expected_time_slot(self):
+		# The report buckets Issues by `creation` into 3-hour slots over the
+		# from_date..to_date range. `creation` is auto-stamped on insert, so we
+		# force it afterwards to a known time. 14:00 sits squarely inside the
+		# "12PM - 3PM" slot (12:00:00 - 15:00:00), away from any slot boundary,
+		# so the bucket assignment is unambiguous.
+		report_date = getdate()
+		issue = make_issue(customer="_Test Customer", index=1)
+		creation = get_datetime(f"{report_date.strftime('%Y-%m-%d')} 14:00:00")
+		frappe.db.set_value("Issue", issue.name, "creation", creation, update_modified=False)
+
+		filters = frappe._dict(
+			{
+				"from_date": report_date,
+				"to_date": report_date,
+				"periodicity": "Daily",
+			}
+		)
+
+		columns, data, _, chart = execute(filters)
+
+		# Single day in range -> exactly one row.
+		self.assertEqual(len(data), 1)
+		row = data[0]
+		self.assertEqual(row["date"], report_date)
+
+		# Real-state check: the report must count exactly the Issues whose
+		# `creation` falls in the 12PM-3PM window (inclusive `between`), which
+		# now includes our seeded record. Compare against an independent count.
+		slot_start = get_datetime(f"{report_date.strftime('%Y-%m-%d')} 12:00:00")
+		slot_end = get_datetime(f"{report_date.strftime('%Y-%m-%d')} 15:00:00")
+		expected = frappe.db.count("Issue", {"creation": ["between", [slot_start, slot_end]]})
+		self.assertGreaterEqual(expected, 1)
+		self.assertEqual(row["12PM - 3PM"], expected)
+
+		# Columns: Date + 8 time slots.
+		self.assertEqual(len(columns), 9)
+
+		# Chart aggregates per-slot totals across the range; the 12PM-3PM
+		# (5th) label must include our seeded record.
+		labels = chart["data"]["labels"]
+		values = chart["data"]["datasets"][0]["values"]
+		self.assertGreaterEqual(values[labels.index("12PM - 3PM")], 1)
+		self.assertEqual(row["12PM - 3PM"], values[labels.index("12PM - 3PM")])