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 8f15dd4d5d)
This commit is contained in:
Nabin Hait
2026-06-08 11:45:34 +05:30
committed by Mergify
parent 29dd6e6681
commit ebb7b37558
2 changed files with 89 additions and 30 deletions

View File

@@ -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()

View File

@@ -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)