Merge pull request #55720 from frappe/mergify/bp/version-15-hotfix/pr-55627

fix(inactive_customers): add allowlist for doctype filter and migrate… (backport #55627)

erpnext/selling/report/inactive_customers/inactive_customers.py

@@ -4,6 +4,8 @@
 
 import frappe
 from frappe import _
+from frappe.query_builder import Case, CustomFunction
+from frappe.query_builder.functions import Count, Max, Sum
 from frappe.utils import cint
 
 
@@ -24,50 +26,69 @@ def execute(filters=None):
 	customers = get_sales_details(doctype)
 
 	data = []
-	for cust in customers:
+	for row in customers:
-		if cint(cust[8]) >= cint(days_since_last_order):
+		if cint(row[8]) >= cint(days_since_last_order):
-			cust.insert(7, get_last_sales_amt(cust[0], doctype))
+			row.insert(7, get_last_sales_amt(row[0], doctype))
-			data.append(cust)
+			data.append(row)
 	return columns, data
 
 
 def get_sales_details(doctype):
-	cond = """sum(so.base_net_total) as 'total_order_considered',
+	customer = frappe.qb.DocType("Customer")
-			max(so.posting_date) as 'last_order_date',
+	sales_doctype = frappe.qb.DocType(doctype)
-			DATEDIFF(CURRENT_DATE, max(so.posting_date)) as 'days_since_last_order' """
-	if doctype == "Sales Order":
-		cond = """sum(if(so.status = "Stopped",
-				so.base_net_total * so.per_delivered/100,
-				so.base_net_total)) as 'total_order_considered',
-			max(so.transaction_date) as 'last_order_date',
-			DATEDIFF(CURRENT_DATE, max(so.transaction_date)) as 'days_since_last_order'"""
 
-	return frappe.db.sql(
+	date_diff = CustomFunction("DATEDIFF", ["d1", "d2"])
-		f"""select
+	current_date = CustomFunction("CURRENT_DATE", [])
-			cust.name,
+
-			cust.customer_name,
+	if doctype == "Sales Order":
-			cust.territory,
+		total_considered = Sum(
-			cust.customer_group,
+			Case()
-			count(distinct(so.name)) as 'num_of_order',
+			.when(
-			sum(base_net_total) as 'total_order_value', {cond}
+				sales_doctype.status == "Stopped",
-		from `tabCustomer` cust, `tab{doctype}` so
+				sales_doctype.base_net_total * sales_doctype.per_delivered / 100,
-		where cust.name = so.customer and so.docstatus = 1
+			)
-		group by cust.name
+			.else_(sales_doctype.base_net_total)
-		order by 'days_since_last_order' desc """,
+		)
-		as_list=1,
+		date_col = sales_doctype.transaction_date
-	)
+	else:
+		total_considered = Sum(sales_doctype.base_net_total)
+		date_col = sales_doctype.posting_date
+
+	last_order_date = Max(date_col)
+	days_since_last_order = date_diff(current_date(), last_order_date)
+
+	return (
+		frappe.qb.from_(customer)
+		.inner_join(sales_doctype)
+		.on(customer.name == sales_doctype.customer)
+		.select(
+			customer.name,
+			customer.customer_name,
+			customer.territory,
+			customer.customer_group,
+			Count(sales_doctype.name).distinct().as_("num_of_order"),
+			Sum(sales_doctype.base_net_total).as_("total_order_value"),
+			total_considered.as_("total_order_considered"),
+			last_order_date.as_("last_order_date"),
+			days_since_last_order.as_("days_since_last_order"),
+		)
+		.where(sales_doctype.docstatus == 1)
+		.groupby(customer.name)
+		.orderby(days_since_last_order, order=frappe.qb.desc)
+	).run(as_list=True)
 
 
 def get_last_sales_amt(customer, doctype):
-	cond = "posting_date"
+	sales_doctype = frappe.qb.DocType(doctype)
-	if doctype == "Sales Order":
+	date_col = sales_doctype.transaction_date if doctype == "Sales Order" else sales_doctype.posting_date
-		cond = "transaction_date"
+
-	res = frappe.db.sql(
+	res = (
-		f"""select base_net_total from `tab{doctype}`
+		frappe.qb.from_(sales_doctype)
-		where customer = %s and docstatus = 1 order by {cond} desc
+		.select(sales_doctype.base_net_total)
-		limit 1""",
+		.where((sales_doctype.customer == customer) & (sales_doctype.docstatus == 1))
-		customer,
+		.orderby(date_col, order=frappe.qb.desc)
-	)
+		.limit(1)
+	).run()
 
 	return res and res[0][0] or 0
 

erpnext/selling/report/inactive_customers/test_inactive_customers.py

@@ -0,0 +1,55 @@
+# Copyright (c) 2024, Frappe Technologies Pvt. Ltd. and contributors
+# For license information, please see license.txt
+
+import frappe
+from frappe.tests.utils import FrappeTestCase
+from frappe.utils import add_days, getdate, today
+
+from erpnext.selling.doctype.sales_order.test_sales_order import make_sales_order
+from erpnext.selling.report.inactive_customers.inactive_customers import execute
+
+
+class TestInactiveCustomers(FrappeTestCase):
+	def setUp(self):
+		self.customer = frappe.new_doc("Customer")
+		self.customer.customer_name = "_Test Inactive Customer"
+		self.customer.customer_group = "_Test Customer Group"
+		self.customer.insert()
+		self.last_order_date = add_days(today(), -120)
+		so = make_sales_order(
+			customer=self.customer.name,
+			transaction_date=self.last_order_date,
+			qty=5,
+			rate=200,
+		)
+		so.submit()
+		self.sales_order = so
+
+	def test_invalid_doctype_is_rejected(self):
+		self.assertRaises(
+			frappe.ValidationError,
+			execute,
+			{"doctype": "Purchase Order", "days_since_last_order": 30},
+		)
+
+	def test_inactive_customer_is_listed_with_expected_columns(self):
+		columns, data = execute({"doctype": "Sales Order", "days_since_last_order": 30})
+
+		row = self.get_customer_row(data)
+		self.assertIsNotNone(row, "Inactive customer should be present in the report")
+
+		# Column contract: the report relies on positional access.
+		self.assertEqual(row[0], self.customer.name)
+		self.assertEqual(row[7], 1000)  # Last Order Amount inserted at index 7 (5 * 200)
+		self.assertEqual(getdate(row[8]), getdate(self.last_order_date))  # Last Order Date
+		self.assertGreaterEqual(row[9], 30)  # Days Since Last Order
+
+	def test_recent_customer_is_excluded(self):
+		_columns, data = execute({"doctype": "Sales Order", "days_since_last_order": 200})
+		self.assertIsNone(
+			self.get_customer_row(data),
+			"Customer ordering within the threshold must be excluded",
+		)
+
+	def get_customer_row(self, data):
+		return next((row for row in data if row[0] == self.customer.name), None)