From ebb7b3755843dd8a61415ded90ec97c177836b47 Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Mon, 8 Jun 2026 11:45:34 +0530 Subject: [PATCH] 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)