From 29dd6e6681d20bbacb69517d2d2c875aa929eb9e Mon Sep 17 00:00:00 2001 From: Shllokkk Date: Mon, 1 Jun 2026 21:24:42 +0530 Subject: [PATCH 1/6] fix(inactive_customers): add allowlist for doctype filter and migrate to qb (cherry picked from commit 2ecf8b0466143bca086f6e6b65dade5f4fc250b8) --- .../inactive_customers/inactive_customers.py | 93 ++++++++++++------- 1 file changed, 57 insertions(+), 36 deletions(-) diff --git a/erpnext/selling/report/inactive_customers/inactive_customers.py b/erpnext/selling/report/inactive_customers/inactive_customers.py index 7e4ddc128ac..ea3bcb43a9f 100644 --- a/erpnext/selling/report/inactive_customers/inactive_customers.py +++ b/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 @@ -14,6 +16,9 @@ def execute(filters=None): days_since_last_order = filters.get("days_since_last_order") doctype = filters.get("doctype") + if doctype not in {"Sales Order", "Sales Invoice"}: + frappe.throw(_("Invalid value {0} for 'Doctype'").format(doctype)) + if cint(days_since_last_order) <= 0: frappe.throw(_("'Days Since Last Order' must be greater than or equal to zero")) @@ -21,50 +26,66 @@ def execute(filters=None): customers = get_sales_details(doctype) data = [] - for cust in customers: - if cint(cust[8]) >= cint(days_since_last_order): - cust.insert(7, get_last_sales_amt(cust[0], doctype)) - data.append(cust) + for C in customers: + if cint(C[8]) >= cint(days_since_last_order): + C.insert(7, get_last_sales_amt(C[0], doctype)) + data.append(C) return columns, data def get_sales_details(doctype): - cond = """sum(so.base_net_total) as 'total_order_considered', - max(so.posting_date) as 'last_order_date', - 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'""" + C = frappe.qb.DocType("Customer") + DT = frappe.qb.DocType(doctype) - return frappe.db.sql( - f"""select - cust.name, - cust.customer_name, - cust.territory, - cust.customer_group, - count(distinct(so.name)) as 'num_of_order', - sum(base_net_total) as 'total_order_value', {cond} - from `tabCustomer` cust, `tab{doctype}` so - where cust.name = so.customer and so.docstatus = 1 - group by cust.name - order by 'days_since_last_order' desc """, - as_list=1, - ) + DateDiff = CustomFunction("DATEDIFF", ["d1", "d2"]) + CurDate = CustomFunction("CURRENT_DATE", []) + + if doctype == "Sales Order": + total_considered = Sum( + Case() + .when(DT.status == "Stopped", DT.base_net_total * DT.per_delivered / 100) + .else_(DT.base_net_total) + ) + date_col = DT.transaction_date + else: + total_considered = Sum(DT.base_net_total) + date_col = DT.posting_date + + last_order_date = Max(date_col) + days_since_last_order = DateDiff(CurDate(), last_order_date) + + return ( + frappe.qb.from_(C) + .inner_join(DT) + .on(C.name == DT.customer) + .select( + C.name, + C.customer_name, + C.territory, + C.customer_group, + Count(DT.name).distinct().as_("num_of_order"), + Sum(DT.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(DT.docstatus == 1) + .groupby(C.name) + .orderby(days_since_last_order, order=frappe.qb.desc) + ).run(as_list=True) def get_last_sales_amt(customer, doctype): - cond = "posting_date" - if doctype == "Sales Order": - cond = "transaction_date" - res = frappe.db.sql( - f"""select base_net_total from `tab{doctype}` - where customer = %s and docstatus = 1 order by {cond} desc - limit 1""", - customer, - ) + DT = frappe.qb.DocType(doctype) + date_col = DT.transaction_date if doctype == "Sales Order" else DT.posting_date + + res = ( + frappe.qb.from_(DT) + .select(DT.base_net_total) + .where((DT.customer == customer) & (DT.docstatus == 1)) + .orderby(date_col, order=frappe.qb.desc) + .limit(1) + ).run() return res and res[0][0] or 0 From ebb7b3755843dd8a61415ded90ec97c177836b47 Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Mon, 8 Jun 2026 11:45:34 +0530 Subject: [PATCH 2/6] refactor(inactive_customers): use descriptive aliases and add tests Rename single-letter query-builder aliases (C, DT) to readable names (customer, sales) and add report tests covering the column contract, validation guards, and the days-since-last-order threshold. (cherry picked from commit 8f15dd4d5d1288958f301d4b7144572e3a42ba0f) --- .../inactive_customers/inactive_customers.py | 60 +++++++++---------- .../test_inactive_customers.py | 59 ++++++++++++++++++ 2 files changed, 89 insertions(+), 30 deletions(-) create mode 100644 erpnext/selling/report/inactive_customers/test_inactive_customers.py diff --git a/erpnext/selling/report/inactive_customers/inactive_customers.py b/erpnext/selling/report/inactive_customers/inactive_customers.py index ea3bcb43a9f..3402a10dc4d 100644 --- a/erpnext/selling/report/inactive_customers/inactive_customers.py +++ b/erpnext/selling/report/inactive_customers/inactive_customers.py @@ -26,63 +26,63 @@ def execute(filters=None): customers = get_sales_details(doctype) data = [] - for C in customers: - if cint(C[8]) >= cint(days_since_last_order): - C.insert(7, get_last_sales_amt(C[0], doctype)) - data.append(C) + for row in customers: + if cint(row[8]) >= cint(days_since_last_order): + row.insert(7, get_last_sales_amt(row[0], doctype)) + data.append(row) return columns, data def get_sales_details(doctype): - C = frappe.qb.DocType("Customer") - DT = frappe.qb.DocType(doctype) + customer = frappe.qb.DocType("Customer") + sales = frappe.qb.DocType(doctype) - DateDiff = CustomFunction("DATEDIFF", ["d1", "d2"]) - CurDate = CustomFunction("CURRENT_DATE", []) + date_diff = CustomFunction("DATEDIFF", ["d1", "d2"]) + current_date = CustomFunction("CURRENT_DATE", []) if doctype == "Sales Order": total_considered = Sum( Case() - .when(DT.status == "Stopped", DT.base_net_total * DT.per_delivered / 100) - .else_(DT.base_net_total) + .when(sales.status == "Stopped", sales.base_net_total * sales.per_delivered / 100) + .else_(sales.base_net_total) ) - date_col = DT.transaction_date + date_col = sales.transaction_date else: - total_considered = Sum(DT.base_net_total) - date_col = DT.posting_date + total_considered = Sum(sales.base_net_total) + date_col = sales.posting_date last_order_date = Max(date_col) - days_since_last_order = DateDiff(CurDate(), last_order_date) + days_since_last_order = date_diff(current_date(), last_order_date) return ( - frappe.qb.from_(C) - .inner_join(DT) - .on(C.name == DT.customer) + frappe.qb.from_(customer) + .inner_join(sales) + .on(customer.name == sales.customer) .select( - C.name, - C.customer_name, - C.territory, - C.customer_group, - Count(DT.name).distinct().as_("num_of_order"), - Sum(DT.base_net_total).as_("total_order_value"), + customer.name, + customer.customer_name, + customer.territory, + customer.customer_group, + Count(sales.name).distinct().as_("num_of_order"), + Sum(sales.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(DT.docstatus == 1) - .groupby(C.name) + .where(sales.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): - DT = frappe.qb.DocType(doctype) - date_col = DT.transaction_date if doctype == "Sales Order" else DT.posting_date + sales = frappe.qb.DocType(doctype) + date_col = sales.transaction_date if doctype == "Sales Order" else sales.posting_date res = ( - frappe.qb.from_(DT) - .select(DT.base_net_total) - .where((DT.customer == customer) & (DT.docstatus == 1)) + frappe.qb.from_(sales) + .select(sales.base_net_total) + .where((sales.customer == customer) & (sales.docstatus == 1)) .orderby(date_col, order=frappe.qb.desc) .limit(1) ).run() diff --git a/erpnext/selling/report/inactive_customers/test_inactive_customers.py b/erpnext/selling/report/inactive_customers/test_inactive_customers.py new file mode 100644 index 00000000000..be7aa39e3be --- /dev/null +++ b/erpnext/selling/report/inactive_customers/test_inactive_customers.py @@ -0,0 +1,59 @@ +# Copyright (c) 2024, Frappe Technologies Pvt. Ltd. and contributors +# For license information, please see license.txt + +import frappe +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 +from erpnext.tests.utils import ERPNextTestSuite + + +class TestInactiveCustomers(ERPNextTestSuite): + def setUp(self): + self.customer = frappe.get_doc(doctype="Customer", customer_name="_Test Inactive 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_non_positive_days_is_rejected(self): + self.assertRaises( + frappe.ValidationError, + execute, + {"doctype": "Sales Order", "days_since_last_order": 0}, + ) + + 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) From 1cba32eb1aedf045a4c3285458b15202bb7ad260 Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Mon, 8 Jun 2026 11:52:56 +0530 Subject: [PATCH 3/6] refactor(inactive_customers): rename sales alias to sales_doctype (cherry picked from commit 8d7edafc99364995927ca633eceffafa5b857ea4) --- .../inactive_customers/inactive_customers.py | 35 ++++++++++--------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/erpnext/selling/report/inactive_customers/inactive_customers.py b/erpnext/selling/report/inactive_customers/inactive_customers.py index 3402a10dc4d..8c7c7b99a32 100644 --- a/erpnext/selling/report/inactive_customers/inactive_customers.py +++ b/erpnext/selling/report/inactive_customers/inactive_customers.py @@ -35,7 +35,7 @@ def execute(filters=None): def get_sales_details(doctype): customer = frappe.qb.DocType("Customer") - sales = frappe.qb.DocType(doctype) + sales_doctype = frappe.qb.DocType(doctype) date_diff = CustomFunction("DATEDIFF", ["d1", "d2"]) current_date = CustomFunction("CURRENT_DATE", []) @@ -43,46 +43,49 @@ def get_sales_details(doctype): if doctype == "Sales Order": total_considered = Sum( Case() - .when(sales.status == "Stopped", sales.base_net_total * sales.per_delivered / 100) - .else_(sales.base_net_total) + .when( + sales_doctype.status == "Stopped", + sales_doctype.base_net_total * sales_doctype.per_delivered / 100, + ) + .else_(sales_doctype.base_net_total) ) - date_col = sales.transaction_date + date_col = sales_doctype.transaction_date else: - total_considered = Sum(sales.base_net_total) - date_col = sales.posting_date + 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) - .on(customer.name == sales.customer) + .inner_join(sales_doctype) + .on(customer.name == sales_doctype.customer) .select( customer.name, customer.customer_name, customer.territory, customer.customer_group, - Count(sales.name).distinct().as_("num_of_order"), - Sum(sales.base_net_total).as_("total_order_value"), + 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.docstatus == 1) + .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): - sales = frappe.qb.DocType(doctype) - date_col = sales.transaction_date if doctype == "Sales Order" else sales.posting_date + sales_doctype = frappe.qb.DocType(doctype) + date_col = sales_doctype.transaction_date if doctype == "Sales Order" else sales_doctype.posting_date res = ( - frappe.qb.from_(sales) - .select(sales.base_net_total) - .where((sales.customer == customer) & (sales.docstatus == 1)) + frappe.qb.from_(sales_doctype) + .select(sales_doctype.base_net_total) + .where((sales_doctype.customer == customer) & (sales_doctype.docstatus == 1)) .orderby(date_col, order=frappe.qb.desc) .limit(1) ).run() From 30efbf8c9bf8d5c20f6ec07af3b1a54ea1c55807 Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Mon, 8 Jun 2026 11:55:32 +0530 Subject: [PATCH 4/6] test(inactive_customers): remove non-positive days test case (cherry picked from commit 601f39dda7688203e697e2ba419fe27a743a6589) --- .../report/inactive_customers/test_inactive_customers.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/erpnext/selling/report/inactive_customers/test_inactive_customers.py b/erpnext/selling/report/inactive_customers/test_inactive_customers.py index be7aa39e3be..c139f42a61f 100644 --- a/erpnext/selling/report/inactive_customers/test_inactive_customers.py +++ b/erpnext/selling/report/inactive_customers/test_inactive_customers.py @@ -29,13 +29,6 @@ class TestInactiveCustomers(ERPNextTestSuite): {"doctype": "Purchase Order", "days_since_last_order": 30}, ) - def test_non_positive_days_is_rejected(self): - self.assertRaises( - frappe.ValidationError, - execute, - {"doctype": "Sales Order", "days_since_last_order": 0}, - ) - def test_inactive_customer_is_listed_with_expected_columns(self): columns, data = execute({"doctype": "Sales Order", "days_since_last_order": 30}) From bd41e741be05911575006c45dee3d5157df948a2 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Wed, 10 Jun 2026 10:16:32 +0530 Subject: [PATCH 5/6] test(inactive customers): use FrappeTestCase --- .../report/inactive_customers/test_inactive_customers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/erpnext/selling/report/inactive_customers/test_inactive_customers.py b/erpnext/selling/report/inactive_customers/test_inactive_customers.py index c139f42a61f..b5f418c9910 100644 --- a/erpnext/selling/report/inactive_customers/test_inactive_customers.py +++ b/erpnext/selling/report/inactive_customers/test_inactive_customers.py @@ -2,14 +2,14 @@ # 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 -from erpnext.tests.utils import ERPNextTestSuite -class TestInactiveCustomers(ERPNextTestSuite): +class TestInactiveCustomers(FrappeTestCase): def setUp(self): self.customer = frappe.get_doc(doctype="Customer", customer_name="_Test Inactive Customer").insert() self.last_order_date = add_days(today(), -120) From ede2a68afd49f2b5943953afd688f429681adc66 Mon Sep 17 00:00:00 2001 From: Shllokkk Date: Sun, 12 Jul 2026 12:32:23 +0530 Subject: [PATCH 6/6] fix: set customer group in inactive customers report test setup --- .../report/inactive_customers/test_inactive_customers.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/erpnext/selling/report/inactive_customers/test_inactive_customers.py b/erpnext/selling/report/inactive_customers/test_inactive_customers.py index b5f418c9910..575b7a8fe41 100644 --- a/erpnext/selling/report/inactive_customers/test_inactive_customers.py +++ b/erpnext/selling/report/inactive_customers/test_inactive_customers.py @@ -11,7 +11,10 @@ from erpnext.selling.report.inactive_customers.inactive_customers import execute class TestInactiveCustomers(FrappeTestCase): def setUp(self): - self.customer = frappe.get_doc(doctype="Customer", customer_name="_Test Inactive Customer").insert() + 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,