From 24f0989ac1f6d3054643d6377e8c126ee8ca0cf7 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Sat, 11 Jul 2026 15:28:23 +0530 Subject: [PATCH] fix: correct filter handling in Sales Person-wise Transaction Summary + tests (backport #56783) (#56908) Co-authored-by: Nabin Hait --- .../sales_person_wise_transaction_summary.py | 67 +++++++++--------- ...t_sales_person_wise_transaction_summary.py | 69 +++++++++++++++++++ 2 files changed, 100 insertions(+), 36 deletions(-) create mode 100644 erpnext/selling/report/sales_person_wise_transaction_summary/test_sales_person_wise_transaction_summary.py diff --git a/erpnext/selling/report/sales_person_wise_transaction_summary/sales_person_wise_transaction_summary.py b/erpnext/selling/report/sales_person_wise_transaction_summary/sales_person_wise_transaction_summary.py index 23ed83cca84..bc3bce3da4a 100644 --- a/erpnext/selling/report/sales_person_wise_transaction_summary/sales_person_wise_transaction_summary.py +++ b/erpnext/selling/report/sales_person_wise_transaction_summary/sales_person_wise_transaction_summary.py @@ -183,8 +183,22 @@ def get_entries(filters): .as_("contribution_amt") ) + # Only pass valid document-field filters to get_query; report-specific keys such as + # doc_type / sales_person / item_group are handled separately below. + doc_filters = {"docstatus": 1} + for field in ["company", "customer", "territory"]: + if filters.get(field): + doc_filters[field] = filters.get(field) + + if filters.get("from_date") and filters.get("to_date"): + doc_filters[date_field] = ["between", [filters.get("from_date"), filters.get("to_date")]] + elif filters.get("from_date"): + doc_filters[date_field] = [">=", filters.get("from_date")] + elif filters.get("to_date"): + doc_filters[date_field] = ["<=", filters.get("to_date")] + query = ( - frappe.get_query(dt, filters=filters, ignore_permissions=False) + frappe.get_query(dt, filters=doc_filters, ignore_permissions=False) .join(dt_item) .on(dt.name == dt_item.parent) .join(st) @@ -203,48 +217,29 @@ def get_entries(filters): contribution_amt_case, ) .where(st.parenttype == doc_type) - .where(dt.docstatus == 1) ) + if filters.get("sales_person"): + lft, rgt = frappe.db.get_value("Sales Person", filters.get("sales_person"), ["lft", "rgt"]) + sp = frappe.qb.DocType("Sales Person") + query = query.where( + st.sales_person.isin(frappe.qb.from_(sp).select(sp.name).where((sp.lft >= lft) & (sp.rgt <= rgt))) + ) + + # only resolve items when an item_group/brand filter is set; otherwise get_items + # would return every item in the system and add a huge IN() clause on each run + if filters.get("item_group") or filters.get("brand"): + items = get_items(filters) + if not items: + # the item_group/brand filter matched nothing -> no rows + return [] + query = query.where(dt_item.item_code.isin([d[0] for d in items])) + query = query.orderby(st.sales_person).orderby(dt.name, order=frappe.qb.desc) return query.run(as_dict=True) -def get_conditions(filters, date_field): - conditions = [""] - values = [] - - for field in ["company", "customer", "territory"]: - if filters.get(field): - conditions.append(f"dt.{field}=%s") - values.append(filters[field]) - - if filters.get("sales_person"): - lft, rgt = frappe.get_value("Sales Person", filters.get("sales_person"), ["lft", "rgt"]) - conditions.append( - f"exists(select name from `tabSales Person` where lft >= {lft} and rgt <= {rgt} and name=st.sales_person)" - ) - - if filters.get("from_date"): - conditions.append(f"dt.{date_field}>=%s") - values.append(filters["from_date"]) - - if filters.get("to_date"): - conditions.append(f"dt.{date_field}<=%s") - values.append(filters["to_date"]) - - items = get_items(filters) - if items: - conditions.append("dt_item.item_code in (%s)" % ", ".join(["%s"] * len(items))) - values += items - else: - # return empty result, if no items are fetched after filtering on 'item group' and 'brand' - conditions.append("dt_item.item_code = Null") - - return " and ".join(conditions), values - - def get_items(filters): item = qb.DocType("Item") diff --git a/erpnext/selling/report/sales_person_wise_transaction_summary/test_sales_person_wise_transaction_summary.py b/erpnext/selling/report/sales_person_wise_transaction_summary/test_sales_person_wise_transaction_summary.py new file mode 100644 index 00000000000..2dbe8fee822 --- /dev/null +++ b/erpnext/selling/report/sales_person_wise_transaction_summary/test_sales_person_wise_transaction_summary.py @@ -0,0 +1,69 @@ +# Copyright (c) 2026, Frappe Technologies Pvt. Ltd. and Contributors +# See license.txt + +import frappe + +from erpnext.accounts.doctype.sales_invoice.test_sales_invoice import create_sales_invoice +from erpnext.selling.report.sales_person_wise_transaction_summary.sales_person_wise_transaction_summary import ( + execute, +) +from erpnext.tests.utils import ERPNextTestSuite + + +class TestSalesPersonWiseTransactionSummary(ERPNextTestSuite): + """Item-level summary joining a sales document with its Sales Team rows, showing + each sales person's contributed qty and amount per item line.""" + + def setUp(self): + self.sales_person = "_Test Sales Person" + + def make_invoice_with_commission(self, qty=5, rate=200, percentage=100): + si = create_sales_invoice( + item="_Test Item", qty=qty, rate=rate, do_not_save=True, posting_date="2026-06-01" + ) + si.append("sales_team", {"sales_person": self.sales_person, "allocated_percentage": percentage}) + si.insert() + si.submit() + return si + + def run_report(self, **extra): + filters = frappe._dict( + {"company": "_Test Company", "doc_type": "Sales Invoice", "sales_person": self.sales_person} + ) + filters.update(extra) + return execute(filters)[1] + + def test_doc_type_is_mandatory(self): + self.assertRaises(frappe.ValidationError, execute, frappe._dict({"company": "_Test Company"})) + + def test_invalid_doc_type_throws(self): + self.assertRaises( + frappe.ValidationError, + execute, + frappe._dict({"company": "_Test Company", "doc_type": "Purchase Invoice"}), + ) + + def test_item_line_contribution(self): + si = self.make_invoice_with_commission(qty=5, rate=200, percentage=100) + item = si.items[0] + + rows = self.run_report() + row = next((r for r in rows if r[0] == si.name and r[5] == "_Test Item"), None) + self.assertIsNotNone(row, "Invoice item line missing from report") + + # row: name, customer, territory, warehouse, posting_date, item_code, item_group, + # brand, stock_qty, base_net_amount, sales_person, allocated_percentage, + # contributed_qty, contribution_amt, currency + self.assertEqual(row[1], si.customer) + self.assertEqual(row[8], item.stock_qty) + self.assertEqual(row[9], item.base_net_amount) + self.assertEqual(row[10], self.sales_person) + self.assertEqual(row[11], 100) + self.assertEqual(row[12], item.stock_qty * 100 / 100) # contributed qty + self.assertEqual(row[13], item.base_net_amount * 100 / 100) # contribution amount + + def test_appends_total_row(self): + self.make_invoice_with_commission() + rows = self.run_report() + self.assertTrue(rows) + self.assertEqual(rows[-1], [""] * len(rows[0]))