From f43af6624610e874e61ad3faf8701e5e6be6271a Mon Sep 17 00:00:00 2001 From: Shllokkk Date: Mon, 1 Jun 2026 21:24:42 +0530 Subject: [PATCH 1/5] fix(inactive_customers): add allowlist for doctype filter and migrate to qb (cherry picked from commit 2ecf8b0466143bca086f6e6b65dade5f4fc250b8) # Conflicts: # erpnext/selling/report/inactive_customers/inactive_customers.py --- .../inactive_customers/inactive_customers.py | 94 ++++++++++++------- 1 file changed, 58 insertions(+), 36 deletions(-) diff --git a/erpnext/selling/report/inactive_customers/inactive_customers.py b/erpnext/selling/report/inactive_customers/inactive_customers.py index d21d11b2447..32f44e7d29f 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,7 +16,11 @@ def execute(filters=None): days_since_last_order = filters.get("days_since_last_order") doctype = filters.get("doctype") +<<<<<<< HEAD if doctype not in ("Sales Order", "Sales Invoice"): +======= + if doctype not in {"Sales Order", "Sales Invoice"}: +>>>>>>> 2ecf8b0466 (fix(inactive_customers): add allowlist for doctype filter and migrate to qb) frappe.throw(_("Invalid value {0} for 'Doctype'").format(doctype)) if cint(days_since_last_order) <= 0: @@ -24,50 +30,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 7a23a9347f7878c54f38013bf50249413cec274a Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Mon, 8 Jun 2026 11:45:34 +0530 Subject: [PATCH 2/5] 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 32f44e7d29f..ed55460736a 100644 --- a/erpnext/selling/report/inactive_customers/inactive_customers.py +++ b/erpnext/selling/report/inactive_customers/inactive_customers.py @@ -30,63 +30,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 aaf2531a4e351dc09e6cb0c5c64c867c42dc5a86 Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Mon, 8 Jun 2026 11:52:56 +0530 Subject: [PATCH 3/5] 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 ed55460736a..27bb71e332c 100644 --- a/erpnext/selling/report/inactive_customers/inactive_customers.py +++ b/erpnext/selling/report/inactive_customers/inactive_customers.py @@ -39,7 +39,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", []) @@ -47,46 +47,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 fa08501045c16125749f97be0a84d8c4718ed6af Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Mon, 8 Jun 2026 11:55:32 +0530 Subject: [PATCH 4/5] 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 10cfac865ee64865d3c5e5d72d4e1bb9645dd80c Mon Sep 17 00:00:00 2001 From: rohitwaghchaure Date: Tue, 9 Jun 2026 09:48:38 +0530 Subject: [PATCH 5/5] chore: fix conflicts --- .../selling/report/inactive_customers/inactive_customers.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/erpnext/selling/report/inactive_customers/inactive_customers.py b/erpnext/selling/report/inactive_customers/inactive_customers.py index 27bb71e332c..ea0831391d3 100644 --- a/erpnext/selling/report/inactive_customers/inactive_customers.py +++ b/erpnext/selling/report/inactive_customers/inactive_customers.py @@ -16,11 +16,7 @@ def execute(filters=None): days_since_last_order = filters.get("days_since_last_order") doctype = filters.get("doctype") -<<<<<<< HEAD if doctype not in ("Sales Order", "Sales Invoice"): -======= - if doctype not in {"Sales Order", "Sales Invoice"}: ->>>>>>> 2ecf8b0466 (fix(inactive_customers): add allowlist for doctype filter and migrate to qb) frappe.throw(_("Invalid value {0} for 'Doctype'").format(doctype)) if cint(days_since_last_order) <= 0: