fix(controllers): keep Supplier trends one row per supplier + add regression tests

The earlier parity fix aggregated the non-key descriptive columns for the Item
and Customer based-on paths but left Supplier grouping by all three selected
columns (supplier, supplier_name, supplier_group). supplier_name is a stored
per-transaction field, so historical purchase docs holding a divergent value for
the same supplier would split one supplier into multiple rows — diverging from
the original MariaDB output, which grouped by t1.supplier only.

Aggregate supplier_name with Max() and keep only supplier + the FD master column
supplier_group in GROUP BY, restoring one row per supplier on both engines.

Add regression tests for the Supplier (purchase) and Customer (sales) paths that
assert a single row per key even when stored descriptive fields diverge; both
fail on the pre-fix multi-column GROUP BY and pass after the fix, on MariaDB and
Postgres.
This commit is contained in:
Mihir Kandoi
2026-06-22 00:58:56 +05:30
parent dbd1388b40
commit 8f1c703871
3 changed files with 66 additions and 2 deletions

View File

@@ -0,0 +1,32 @@
# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
# License: GNU General Public License v3. See license.txt
import frappe
from erpnext.tests.utils import ERPNextTestSuite
class TestPurchaseOrderTrends(ERPNextTestSuite):
def test_supplier_with_divergent_stored_name_stays_one_row(self):
# supplier_name is a stored per-transaction field; historical purchase docs can hold a different
# value for the same supplier. trends groups by t1.supplier only and aggregates supplier_name with
# Max(), so the report stays one row per supplier on both MariaDB and Postgres. Grouping by
# supplier_name (the pre-fix behaviour) would split the supplier into two rows.
from erpnext.buying.doctype.purchase_order.test_purchase_order import create_purchase_order
from erpnext.buying.report.purchase_order_trends.purchase_order_trends import execute
create_purchase_order(supplier="_Test Supplier", qty=3, rate=100)
po2 = create_purchase_order(supplier="_Test Supplier", qty=2, rate=100)
# simulate a historical doc that stored a different supplier_name for the same supplier
frappe.db.set_value("Purchase Order", po2.name, "supplier_name", "_Test Supplier (renamed)")
filters = {
"company": "_Test Company",
"period": "Monthly",
"based_on": "Supplier",
}
columns, data, _chart_none, _chart = execute(filters)
self.assertTrue(columns)
supplier_rows = [row for row in data if row[0] == "_Test Supplier"]
self.assertEqual(len(supplier_rows), 1)

View File

@@ -418,8 +418,14 @@ def based_wise_columns_query(based_on, trans):
"Supplier Name:Data:120",
"Supplier Group:Link/Supplier Group:140",
]
based_on_details["based_on_select"] = "t1.supplier, t1.supplier_name, t3.supplier_group,"
based_on_details["based_on_group_by"] = "t1.supplier, t1.supplier_name, t3.supplier_group"
# supplier_name is a stored per-transaction field (not functionally dependent on supplier), so
# it is aggregated to keep one row per supplier — matching the prior MariaDB output, which grouped
# by t1.supplier only. supplier_group comes from the joined master and is FD on supplier, so it
# stays in GROUP BY (postgres-valid, no row split).
based_on_details[
"based_on_select"
] = "t1.supplier, Max(t1.supplier_name) as supplier_name, t3.supplier_group,"
based_on_details["based_on_group_by"] = "t1.supplier, t3.supplier_group"
based_on_details["addl_tables"] = ",`tabSupplier` t3"
based_on_details["addl_tables_relational_cond"] = " and t1.supplier = t3.name"

View File

@@ -1,6 +1,8 @@
# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
# License: GNU General Public License v3. See license.txt
import frappe
from erpnext.tests.utils import ERPNextTestSuite
@@ -25,3 +27,27 @@ class TestSalesOrderTrends(ERPNextTestSuite):
self.assertTrue(columns)
self.assertTrue(any("_Test Item" in [str(cell) for cell in row] for row in data))
def test_customer_with_divergent_stored_territory_stays_one_row(self):
# territory (and customer_name) are stored per-transaction fields; historical sales docs can hold a
# different value for the same customer. trends groups by t1.customer only and aggregates these with
# Max(), so the report stays one row per customer on both MariaDB and Postgres. Grouping by territory
# (the pre-fix behaviour) would split the customer into two rows.
from erpnext.selling.doctype.sales_order.test_sales_order import make_sales_order
from erpnext.selling.report.sales_order_trends.sales_order_trends import execute
make_sales_order(customer="_Test Customer", item_code="_Test Item", qty=3, rate=100)
so2 = make_sales_order(customer="_Test Customer", item_code="_Test Item", qty=2, rate=100)
# simulate a historical doc that stored a different territory for the same customer
frappe.db.set_value("Sales Order", so2.name, "territory", "_Test Territory Rest Of The World")
filters = {
"company": "_Test Company",
"period": "Monthly",
"based_on": "Customer",
}
columns, data, _chart_none, _chart = execute(filters)
self.assertTrue(columns)
customer_rows = [row for row in data if row[0] == "_Test Customer"]
self.assertEqual(len(customer_rows), 1)