From a954539b5324b1375670d98fb5c1a90897b25eb7 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Fri, 19 Jun 2026 16:28:23 +0530 Subject: [PATCH] refactor(postgres): port sales_analytics tree/order-type queries to the query builder Co-Authored-By: Claude Opus 4.8 (1M context) --- .../report/sales_analytics/sales_analytics.py | 27 +-- .../sales_analytics/test_sales_analytics.py | 174 ++++++++++++++++++ 2 files changed, 190 insertions(+), 11 deletions(-) create mode 100644 erpnext/selling/report/sales_analytics/test_sales_analytics.py diff --git a/erpnext/selling/report/sales_analytics/sales_analytics.py b/erpnext/selling/report/sales_analytics/sales_analytics.py index e36690b4384..93f1abe8222 100644 --- a/erpnext/selling/report/sales_analytics/sales_analytics.py +++ b/erpnext/selling/report/sales_analytics/sales_analytics.py @@ -510,10 +510,10 @@ class Analytics: self.depth_map = frappe._dict() - self.group_entries = frappe.db.sql( - f"""select name, lft, rgt , {parent} as parent - from `tab{self.filters.tree_type}` order by lft""", - as_dict=1, + self.group_entries = frappe.get_all( + self.filters.tree_type, + fields=["name", "lft", "rgt", f"{parent} as parent"], + order_by="lft", ) for d in self.group_entries: @@ -528,14 +528,19 @@ class Analytics: if not frappe.db.exists("DocType", self.filters.doc_type): frappe.throw(_("Invalid Document Type {0}").format(self.filters.doc_type)) - self.group_entries = frappe.db.sql( - f""" select * from (select "Order Types" as name, 0 as lft, - 2 as rgt, '' as parent union select distinct order_type as name, 1 as lft, 1 as rgt, "Order Types" as parent - from `tab{self.filters.doc_type}` where ifnull(order_type, '') != '') as b order by lft, name - """, - as_dict=1, + order_types = frappe.get_all( + self.filters.doc_type, + filters={"order_type": ["is", "set"]}, + pluck="order_type", + distinct=True, + order_by="order_type", ) + self.group_entries = [frappe._dict(name="Order Types", lft=0, rgt=2, parent="")] + self.group_entries += [ + frappe._dict(name=order_type, lft=1, rgt=1, parent="Order Types") for order_type in order_types + ] + for d in self.group_entries: if d.parent: self.depth_map.setdefault(d.name, self.depth_map.get(d.parent) + 1) @@ -544,7 +549,7 @@ class Analytics: def get_supplier_parent_child_map(self): self.parent_child_map = frappe._dict( - frappe.db.sql(""" select name, supplier_group from `tabSupplier`""") + frappe.get_all("Supplier", fields=["name", "supplier_group"], as_list=True) ) def get_chart_data(self): diff --git a/erpnext/selling/report/sales_analytics/test_sales_analytics.py b/erpnext/selling/report/sales_analytics/test_sales_analytics.py new file mode 100644 index 00000000000..b9827327b63 --- /dev/null +++ b/erpnext/selling/report/sales_analytics/test_sales_analytics.py @@ -0,0 +1,174 @@ +# Copyright (c) 2024, Frappe Technologies Pvt. Ltd. and Contributors +# License: GNU General Public License v3. See license.txt + + +import frappe +from frappe.utils import flt + +from erpnext.buying.doctype.purchase_order.test_purchase_order import create_purchase_order +from erpnext.selling.doctype.sales_order.test_sales_order import make_sales_order +from erpnext.selling.report.sales_analytics.sales_analytics import execute +from erpnext.tests.utils import ERPNextTestSuite + +# Bootstrap masters reused as-is (see erpnext/tests/utils.py): +# "_Test Customer" -> customer_group "_Test Customer Group", territory "_Test Territory" +# "_Test Supplier" -> supplier_group "_Test Supplier Group" (child of "All Supplier Groups") +# Sales Order.order_type defaults to "Sales" (reqd Select field) +COMPANY = "_Test Company" +CUSTOMER = "_Test Customer" +CUSTOMER_GROUP = "_Test Customer Group" +TERRITORY = "_Test Territory" +SUPPLIER = "_Test Supplier" +SUPPLIER_GROUP = "_Test Supplier Group" +FROM_DATE = "2019-04-01" +TO_DATE = "2019-06-30" + + +class TestSalesAnalytics(ERPNextTestSuite): + def setUp(self): + frappe.set_user("Administrator") + # Two submitted Sales Orders for the bootstrap customer inside the report window. + # These roll up into the tree roots the converted tree/order-type queries build. + self.orders = [ + make_sales_order( + company=COMPANY, + customer=CUSTOMER, + qty=5, + rate=100, + transaction_date="2019-04-10", + ), + make_sales_order( + company=COMPANY, + customer=CUSTOMER, + qty=3, + rate=100, + transaction_date="2019-05-15", + ), + ] + + def _base_filters(self, **overrides): + filters = { + "doc_type": "Sales Order", + "value_quantity": "Value", + "range": "Monthly", + "company": COMPANY, + "from_date": FROM_DATE, + "to_date": TO_DATE, + } + filters.update(overrides) + return filters + + def _expected_value_total(self): + return sum(flt(so.base_net_total) for so in self.orders) + + def _expected_qty_total(self): + return sum(flt(so.total_qty) for so in self.orders) + + def _row_by_entity(self, data): + return {row["entity"]: row for row in data} + + def test_customer_group_tree_rolls_up_to_root(self): + """tree_type='Customer Group' drives get_groups (tree get_all ordered by lft) + and get_rows_by_group, rolling child values up to the 'All Customer Groups' root.""" + columns, data, *_ = execute(self._base_filters(tree_type="Customer Group")) + + self.assertTrue(columns) + self.assertTrue(data) + + rows = self._row_by_entity(data) + # The whole tree is returned, so both the root and the customer's own group appear. + self.assertIn("All Customer Groups", rows) + self.assertIn(CUSTOMER_GROUP, rows) + + expected = self._expected_value_total() + self.assertGreater(expected, 0) + # Leaf group holds the orders; root receives the same total via roll-up. + self.assertAlmostEqual(rows[CUSTOMER_GROUP]["total"], expected, places=2) + self.assertAlmostEqual(rows["All Customer Groups"]["total"], expected, places=2) + # Roots of a tree report sit at indent 0. + self.assertEqual(rows["All Customer Groups"]["indent"], 0) + + def test_territory_tree_rolls_up_to_root(self): + """tree_type='Territory' exercises the same tree path against the Territory tree.""" + columns, data, *_ = execute(self._base_filters(tree_type="Territory")) + + self.assertTrue(columns) + rows = self._row_by_entity(data) + self.assertIn("All Territories", rows) + self.assertIn(TERRITORY, rows) + + expected = self._expected_value_total() + self.assertAlmostEqual(rows[TERRITORY]["total"], expected, places=2) + self.assertAlmostEqual(rows["All Territories"]["total"], expected, places=2) + + def test_order_type_synthetic_tree(self): + """tree_type='Order Type' drives get_teams: distinct order_type rebuilt in Python + under a synthetic 'Order Types' root, then rolled up via get_rows_by_group.""" + columns, data, *_ = execute(self._base_filters(tree_type="Order Type")) + + self.assertTrue(columns) + rows = self._row_by_entity(data) + # Synthetic root plus the default order_type the bootstrap Sales Orders carry. + self.assertIn("Order Types", rows) + self.assertIn("Sales", rows) + self.assertEqual(rows["Order Types"]["indent"], 0) + + expected = self._expected_value_total() + self.assertAlmostEqual(rows["Sales"]["total"], expected, places=2) + self.assertAlmostEqual(rows["Order Types"]["total"], expected, places=2) + + def test_customer_group_by_quantity(self): + """value_quantity='Quantity' switches the selected value column (total_qty).""" + _columns, data, *_ = execute( + self._base_filters(tree_type="Customer Group", value_quantity="Quantity") + ) + + rows = self._row_by_entity(data) + self.assertIn(CUSTOMER_GROUP, rows) + + expected_qty = self._expected_qty_total() + self.assertGreater(expected_qty, 0) + self.assertAlmostEqual(rows[CUSTOMER_GROUP]["total"], expected_qty, places=2) + self.assertAlmostEqual(rows["All Customer Groups"]["total"], expected_qty, places=2) + + def test_supplier_group_tree_maps_supplier_to_group(self): + """tree_type='Supplier Group' (doc_type='Purchase Order') exercises + get_supplier_parent_child_map: the query selects 'supplier' as entity, then + get_periodic_data remaps each supplier to its group via the parent->child map + built by frappe.get_all('Supplier', ['name', 'supplier_group'], as_list=True). + The group total then rolls up into the 'All Supplier Groups' root.""" + # Baseline the report before adding our Purchase Order so the assertion is + # robust to any pre-existing rows in the historical window. + base_filters = self._base_filters(tree_type="Supplier Group", doc_type="Purchase Order") + _columns, base_data, *_ = execute(base_filters) + base_rows = self._row_by_entity(base_data) + base_group_total = flt(base_rows.get(SUPPLIER_GROUP, {}).get("total", 0.0)) + + po = create_purchase_order( + company=COMPANY, + supplier=SUPPLIER, + qty=4, + rate=250, + transaction_date="2019-04-10", + ) + po_value = flt(po.base_net_total) + self.assertGreater(po_value, 0) + + columns, data, *_ = execute(base_filters) + + self.assertTrue(columns) + self.assertTrue(data) + + rows = self._row_by_entity(data) + # The supplier was remapped to its group; both the leaf group and the tree + # root appear as entities (no raw supplier name leaks into the output). + self.assertIn(SUPPLIER_GROUP, rows) + self.assertIn("All Supplier Groups", rows) + self.assertNotIn(SUPPLIER, rows) + # Roots of a tree report sit at indent 0. + self.assertEqual(rows["All Supplier Groups"]["indent"], 0) + + # The new PO lands in the supplier's group via the parent->child map. + self.assertAlmostEqual(rows[SUPPLIER_GROUP]["total"] - base_group_total, po_value, places=2) + # Roll-up: the root aggregates every group, so it covers at least this PO. + self.assertGreaterEqual(flt(rows["All Supplier Groups"]["total"]), po_value)