From e9bca57af56848854d75984ef6795c576cf4d262 Mon Sep 17 00:00:00 2001 From: S Sakthivel Murugan Date: Sun, 5 Jul 2026 00:14:02 +0530 Subject: [PATCH] test: add regression test for trends chart total row (cherry picked from commit b72ecdda0ddb477913e5514fd6bf545b72dbca2b) # Conflicts: # erpnext/buying/report/purchase_order_trends/test_purchase_order_trends.py # erpnext/selling/report/quotation_trends/test_quotation_trends.py # erpnext/selling/report/sales_order_trends/test_sales_order_trends.py --- .../purchase_order_trends.py | 15 +- .../test_purchase_order_trends.py | 198 ++++++++++++++++ erpnext/controllers/trends.py | 13 +- .../quotation_trends/quotation_trends.py | 15 +- .../quotation_trends/test_quotation_trends.py | 180 +++++++++++++++ .../sales_order_trends/sales_order_trends.py | 14 +- .../test_sales_order_trends.py | 213 ++++++++++++++++++ .../delivery_note_trends.py | 8 +- .../landed_cost_report/landed_cost_report.py | 12 +- .../purchase_receipt_trends.py | 8 +- 10 files changed, 643 insertions(+), 33 deletions(-) create mode 100644 erpnext/buying/report/purchase_order_trends/test_purchase_order_trends.py create mode 100644 erpnext/selling/report/quotation_trends/test_quotation_trends.py create mode 100644 erpnext/selling/report/sales_order_trends/test_sales_order_trends.py diff --git a/erpnext/buying/report/purchase_order_trends/purchase_order_trends.py b/erpnext/buying/report/purchase_order_trends/purchase_order_trends.py index d59051407d2..dcfc9ad7076 100644 --- a/erpnext/buying/report/purchase_order_trends/purchase_order_trends.py +++ b/erpnext/buying/report/purchase_order_trends/purchase_order_trends.py @@ -4,7 +4,6 @@ from frappe import _ -import erpnext from erpnext.controllers.trends import get_columns, get_data @@ -15,7 +14,6 @@ def execute(filters=None): conditions = get_columns(filters, "Purchase Order") data = get_data(filters, conditions) chart_data = get_chart_data(data, conditions, filters) - return conditions["columns"], data, None, chart_data @@ -40,9 +38,15 @@ def get_chart_data(data, conditions, filters): labels = [column.split(":")[0] for column in columns] datapoints = [0] * len(labels) + group_by_col_idx = None + if filters.get("group_by"): + group_by_col_idx = conditions["columns"].index(conditions["grbc"][0]) + for row in data: - # If group by filter, don't add first row of group (it's already summed) - if not row[start]: + # Skip the final grand-total row + if row[0] == f"'{_('Total')}'": + continue + if group_by_col_idx is not None and row[group_by_col_idx] == "": continue # Remove None values and compute only periodic data row = [x if x else 0 for x in row[start:-2]] @@ -51,7 +55,6 @@ def get_chart_data(data, conditions, filters): for i in range(len(row)): datapoints[i] += row[i] - company_currency = erpnext.get_company_currency(filters.get("company")) return { "data": { "labels": labels, @@ -63,5 +66,5 @@ def get_chart_data(data, conditions, filters): "lineOptions": {"regionFill": 1}, "fieldtype": "Currency", "options": "currency", - "currency": company_currency, + "currency": conditions.get("company_currency"), } diff --git a/erpnext/buying/report/purchase_order_trends/test_purchase_order_trends.py b/erpnext/buying/report/purchase_order_trends/test_purchase_order_trends.py new file mode 100644 index 00000000000..d11ad290120 --- /dev/null +++ b/erpnext/buying/report/purchase_order_trends/test_purchase_order_trends.py @@ -0,0 +1,198 @@ +# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors +# License: GNU General Public License v3. See license.txt + +import frappe +from frappe import _ +from frappe.utils import today + +from erpnext.accounts.utils import get_fiscal_year +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) + + def test_total_row_not_double_counted_in_chart(self): + # Regression test for the fix in trends.calculate_total_row that populates the + # Total row's Currency column. Before the fix in get_chart_data (skipping the + # Total row by label instead of `if not row[start]`), that populated Currency + # cell made the Total-row-skip guard falsy, so the already-summed Total row got + # added into the chart a second time (a PO of qty=3, rate=100 -> 300 read as 600). + 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, transaction_date=today()) + + fiscal_year = get_fiscal_year(today())[0] + + filters = frappe._dict( + { + "company": "_Test Company", + "fiscal_year": fiscal_year, + "period": "Monthly", + "based_on": "Item", + } + ) + + columns, data, _message, chart = execute(filters) + + self.assertTrue(columns) + self.assertTrue(data) + + # The Total row (present in `data`) must not be re-summed into the chart's datapoints. + total_row = next(row for row in data if row[0] == f"'{_('Total')}'") + expected_total = total_row[-1] # Total(Amt) is the last column + + chart_total = sum(chart["data"]["datasets"][0]["values"]) + + self.assertEqual(chart_total, expected_total) + self.assertEqual(chart_total, 300) + + def test_chart_currency_matches_company_currency(self): + # Regression test: the chart's "currency" key should reflect the transacting + # company's currency (conditions["company_currency"]), not a stale global default. + 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=1, rate=100, transaction_date=today()) + + fiscal_year = get_fiscal_year(today())[0] + + filters = frappe._dict( + { + "company": "_Test Company", + "fiscal_year": fiscal_year, + "period": "Monthly", + "based_on": "Item", + } + ) + + _columns, _data, _message, chart = execute(filters) + + expected_currency = frappe.get_cached_value("Company", "_Test Company", "default_currency") + self.assertEqual(chart["currency"], expected_currency) + + def test_group_by_chart_matches_table_total_with_mixed_group_sizes(self): + # _Test Item is split across two suppliers -> two detail rows under one header row. + # _Test Item 2 has only one supplier -> exactly one detail row under its header row. + # A regression that double-counts header rows would inflate the chart above 600; + # a regression that zeroes single-group rows would report less than 600. + 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( + item_code="_Test Item", supplier="_Test Supplier", qty=3, rate=100, transaction_date=today() + ) + create_purchase_order( + item_code="_Test Item", supplier="_Test Supplier 1", qty=2, rate=100, transaction_date=today() + ) + create_purchase_order( + item_code="_Test Item 2", supplier="_Test Supplier", qty=1, rate=100, transaction_date=today() + ) + + fiscal_year = get_fiscal_year(today())[0] + filters = frappe._dict( + { + "company": "_Test Company", + "fiscal_year": fiscal_year, + "period": "Monthly", + "based_on": "Item", + "group_by": "Supplier", + } + ) + + columns, data, _message, chart = execute(filters) + self.assertTrue(columns) + self.assertTrue(data) + + total_row = next(row for row in data if row[0] == f"'{_('Total')}'") + expected_total = total_row[-1] + chart_total = sum(chart["data"]["datasets"][0]["values"]) + + # 300 (item/supplier) + 200 (item/supplier1) + 100 (item2/supplier) = 600 + self.assertEqual(expected_total, 600) + self.assertEqual(chart_total, expected_total) + + def test_group_by_swapped_roles_based_on_supplier_group_by_item(self): + # Same regression, opposite role assignment: based_on="Supplier" with group_by="Item". + # Supplier's based_on_cols (Supplier, Supplier Name, Supplier Group, Currency) put the + # group_by placeholder at a different column index than the Item-based_on case above, + # exercising the alternate `inc`/`ind` arithmetic. + 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( + item_code="_Test Item", supplier="_Test Supplier", qty=3, rate=100, transaction_date=today() + ) + create_purchase_order( + item_code="_Test Item 2", supplier="_Test Supplier", qty=1, rate=100, transaction_date=today() + ) + + fiscal_year = get_fiscal_year(today())[0] + filters = frappe._dict( + { + "company": "_Test Company", + "fiscal_year": fiscal_year, + "period": "Monthly", + "based_on": "Supplier", + "group_by": "Item", + } + ) + + columns, data, _message, chart = execute(filters) + total_row = next(row for row in data if row[0] == f"'{_('Total')}'") + expected_total = total_row[-1] + chart_total = sum(chart["data"]["datasets"][0]["values"]) + + # 300 + 100 = 400 + self.assertEqual(expected_total, 400) + self.assertEqual(chart_total, expected_total) + + def test_group_by_single_group_value_not_zeroed(self): + # Isolates the specific failure mode flagged in review: a based_on value with exactly + # one associated group value must still contribute its real amount to the chart, not 0. + 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( + item_code="_Test Item", supplier="_Test Supplier", qty=2, rate=150, transaction_date=today() + ) + + fiscal_year = get_fiscal_year(today())[0] + filters = frappe._dict( + { + "company": "_Test Company", + "fiscal_year": fiscal_year, + "period": "Monthly", + "based_on": "Item", + "group_by": "Supplier", + } + ) + + columns, data, _message, chart = execute(filters) + chart_total = sum(chart["data"]["datasets"][0]["values"]) + + self.assertGreater(chart_total, 0) + self.assertEqual(chart_total, 300) diff --git a/erpnext/controllers/trends.py b/erpnext/controllers/trends.py index fed7bec1351..38e37f6ddbe 100644 --- a/erpnext/controllers/trends.py +++ b/erpnext/controllers/trends.py @@ -43,6 +43,9 @@ def get_columns(filters, trans): "addl_tables": based_on_details["addl_tables"], "addl_tables_relational_cond": based_on_details.get("addl_tables_relational_cond", ""), } + conditions["company_currency"] = ( + erpnext.get_company_currency(filters.get("company")) if filters.get("company") else None + ) return conditions @@ -207,7 +210,7 @@ def get_data(filters, conditions): data.append(des) - total_row = calculate_total_row(data1, conditions["columns"], filters.get("company")) + total_row = calculate_total_row(data1, conditions["columns"], conditions.get("company_currency")) data.append(total_row) else: data = frappe.db.sql( @@ -232,13 +235,13 @@ def get_data(filters, conditions): as_list=1, ) - total_row = calculate_total_row(data, conditions["columns"], filters.get("company")) + total_row = calculate_total_row(data, conditions["columns"], conditions.get("company_currency")) data.append(total_row) return data -def calculate_total_row(data, columns, company=None): +def calculate_total_row(data, columns, company_currency=None): def wrap_in_quotes(label): return f"'{label}'" @@ -247,7 +250,7 @@ def calculate_total_row(data, columns, company=None): for i, col in enumerate(columns): if "Float" in col or "Currency/currency" in col: total_values[i] = 0 - if col.split(":")[0] == "Currency": + if "Link/Currency" in col: currency_col_idx = i for row in data: @@ -259,7 +262,7 @@ def calculate_total_row(data, columns, company=None): total_row.append(total_values.get(i, None)) if currency_col_idx is not None: - total_row[currency_col_idx] = company and erpnext.get_company_currency(company) + total_row[currency_col_idx] = company_currency return total_row diff --git a/erpnext/selling/report/quotation_trends/quotation_trends.py b/erpnext/selling/report/quotation_trends/quotation_trends.py index 57c6cc4e2e3..e5b62569394 100644 --- a/erpnext/selling/report/quotation_trends/quotation_trends.py +++ b/erpnext/selling/report/quotation_trends/quotation_trends.py @@ -3,7 +3,6 @@ from frappe import _ -import erpnext from erpnext.controllers.trends import get_columns, get_data @@ -40,9 +39,15 @@ def get_chart_data(data, conditions, filters): labels = [column.split(":")[0] for column in columns] datapoints = [0] * len(labels) + group_by_col_idx = None + if filters.get("group_by"): + group_by_col_idx = conditions["columns"].index(conditions["grbc"][0]) + for row in data: - # If group by filter, don't add first row of group (it's already summed) - if not row[start]: + # Skip the final grand-total row + if row[0] == f"'{_('Total')}'": + continue + if group_by_col_idx is not None and row[group_by_col_idx] == "": continue # Remove None values and compute only periodic data row = [x if x else 0 for x in row[start:-2]] @@ -50,7 +55,7 @@ def get_chart_data(data, conditions, filters): for i in range(len(row)): datapoints[i] += row[i] - company_currency = erpnext.get_company_currency(filters.get("company")) + return { "data": { "labels": labels, @@ -60,5 +65,5 @@ def get_chart_data(data, conditions, filters): "lineOptions": {"regionFill": 1}, "fieldtype": "Currency", "options": "currency", - "currency": company_currency, + "currency": conditions.get("company_currency"), } diff --git a/erpnext/selling/report/quotation_trends/test_quotation_trends.py b/erpnext/selling/report/quotation_trends/test_quotation_trends.py new file mode 100644 index 00000000000..95ba6dd50bc --- /dev/null +++ b/erpnext/selling/report/quotation_trends/test_quotation_trends.py @@ -0,0 +1,180 @@ +# Copyright (c) 2026, Frappe Technologies Pvt. Ltd. and Contributors +# See license.txt + +import frappe +from frappe import _ + +from erpnext.selling.doctype.quotation.test_quotation import make_quotation +from erpnext.selling.report.quotation_trends.quotation_trends import execute +from erpnext.tests.utils import ERPNextTestSuite + +FISCAL_YEAR = "_Test Fiscal Year 2026" +TXN_DATE = "2026-06-01" + + +class TestQuotationTrends(ERPNextTestSuite): + """The trends report buckets submitted Quotation quantities/amounts by period + (Yearly/Monthly) for the chosen `based_on` dimension (Item, Customer, ...).""" + + def run_report(self, **extra): + filters = frappe._dict( + { + "company": "_Test Company", + "fiscal_year": FISCAL_YEAR, + "based_on": "Item", + "period": "Yearly", + } + ) + filters.update(extra) + result = execute(filters) + columns, data = result[0], result[1] + labels = [c.split(":")[0] if isinstance(c, str) else c.get("label") for c in columns] + return labels, data + + def _cell(self, data, key_label, key_value, col_label, labels): + """Value at column `col_label` for the row whose `key_label` column equals + `key_value`, or 0 when that row doesn't exist yet.""" + key_idx = labels.index(key_label) + col_idx = labels.index(col_label) + for row in data: + if row[key_idx] == key_value: + return row[col_idx] or 0 + return 0 + + def test_yearly_item_amount_and_total(self): + # Yearly period => a single " (Qty)"/"(Amt)" bucket plus Total(Qty)/Total(Amt). + labels, before = self.run_report() + qty_col = f"{FISCAL_YEAR} (Qty)" + amt_col = f"{FISCAL_YEAR} (Amt)" + before_qty = self._cell(before, "Item", "_Test Item", qty_col, labels) + before_amt = self._cell(before, "Item", "_Test Item", amt_col, labels) + before_tot_qty = self._cell(before, "Item", "_Test Item", "Total(Qty)", labels) + before_tot_amt = self._cell(before, "Item", "_Test Item", "Total(Amt)", labels) + + make_quotation(item="_Test Item", qty=4, rate=200, transaction_date=TXN_DATE) + + labels, after = self.run_report() + self.assertEqual(self._cell(after, "Item", "_Test Item", qty_col, labels) - before_qty, 4) + self.assertEqual(self._cell(after, "Item", "_Test Item", amt_col, labels) - before_amt, 800) + self.assertEqual(self._cell(after, "Item", "_Test Item", "Total(Qty)", labels) - before_tot_qty, 4) + self.assertEqual(self._cell(after, "Item", "_Test Item", "Total(Amt)", labels) - before_tot_amt, 800) + + def test_monthly_lands_in_june_bucket(self): + # Monthly period => one bucket per month; a 2026-06-01 quotation hits "Jun (Qty)"/"(Amt)". + labels, before = self.run_report(period="Monthly") + before_jun_qty = self._cell(before, "Item", "_Test Item", "Jun (Qty)", labels) + before_jun_amt = self._cell(before, "Item", "_Test Item", "Jun (Amt)", labels) + before_may_qty = self._cell(before, "Item", "_Test Item", "May (Qty)", labels) + + make_quotation(item="_Test Item", qty=3, rate=100, transaction_date=TXN_DATE) + + labels, after = self.run_report(period="Monthly") + self.assertEqual(self._cell(after, "Item", "_Test Item", "Jun (Qty)", labels) - before_jun_qty, 3) + # the amount path is a separate SUM(base_net_amount) case, so assert it too + self.assertEqual(self._cell(after, "Item", "_Test Item", "Jun (Amt)", labels) - before_jun_amt, 300) + # nothing was quoted in May, so that bucket is unchanged + self.assertEqual(self._cell(after, "Item", "_Test Item", "May (Qty)", labels) - before_may_qty, 0) + + def test_based_on_customer_groups_amount_by_party(self): + # based_on Customer keys rows on the "Party" column (the customer id) + labels, before = self.run_report(based_on="Customer") + amt_col = f"{FISCAL_YEAR} (Amt)" + before_amt = self._cell(before, "Party", "_Test Customer", amt_col, labels) + + make_quotation( + party_name="_Test Customer", item="_Test Item", qty=2, rate=150, transaction_date=TXN_DATE + ) + + labels, after = self.run_report(based_on="Customer") + self.assertEqual(self._cell(after, "Party", "_Test Customer", amt_col, labels) - before_amt, 300) + + def test_group_by_chart_matches_table_total_with_mixed_group_sizes(self): + # _Test Item is quoted to two customers -> two detail rows under one header row. + # _Test Item 2 is quoted to only one customer -> exactly one detail row under its + # header row. A regression that double-counts header rows would inflate the chart + # above 800; a regression that zeroes single-group rows would report less than 800. + filters = frappe._dict( + { + "company": "_Test Company", + "fiscal_year": FISCAL_YEAR, + "period": "Yearly", + "based_on": "Item", + "group_by": "Customer", + } + ) + + make_quotation( + item="_Test Item", party_name="_Test Customer", qty=4, rate=100, transaction_date=TXN_DATE + ) + make_quotation( + item="_Test Item", party_name="_Test Customer 1", qty=1, rate=100, transaction_date=TXN_DATE + ) + make_quotation( + item="_Test Item 2", party_name="_Test Customer", qty=3, rate=100, transaction_date=TXN_DATE + ) + + columns, data, _message, chart = execute(filters) + self.assertTrue(columns) + self.assertTrue(data) + + total_row = next(row for row in data if row[0] == f"'{_('Total')}'") + expected_total = total_row[-1] + chart_total = sum(chart["data"]["datasets"][0]["values"]) + + # 400 (item/customer) + 100 (item/customer1) + 300 (item2/customer) = 800 + self.assertEqual(expected_total, 800) + self.assertEqual(chart_total, expected_total) + + def test_group_by_swapped_roles_based_on_customer_group_by_item(self): + # Same regression, opposite role assignment: based_on="Customer" with group_by="Item". + # Customer's based_on_cols for Quotation (Party, Party Name, Territory, Currency) put + # the group_by placeholder at a different column index than the Item-based_on case + # above, exercising the alternate `inc`/`ind` arithmetic. + filters = frappe._dict( + { + "company": "_Test Company", + "fiscal_year": FISCAL_YEAR, + "period": "Yearly", + "based_on": "Customer", + "group_by": "Item", + } + ) + + make_quotation( + party_name="_Test Customer", item="_Test Item", qty=3, rate=100, transaction_date=TXN_DATE + ) + make_quotation( + party_name="_Test Customer", item="_Test Item 2", qty=1, rate=100, transaction_date=TXN_DATE + ) + + columns, data, _message, chart = execute(filters) + total_row = next(row for row in data if row[0] == f"'{_('Total')}'") + expected_total = total_row[-1] + chart_total = sum(chart["data"]["datasets"][0]["values"]) + + # 300 + 100 = 400 + self.assertEqual(expected_total, 400) + self.assertEqual(chart_total, expected_total) + + def test_group_by_single_group_value_not_zeroed(self): + # Isolates the specific failure mode flagged in review: a based_on value with exactly + # one associated group value must still contribute its real amount to the chart, not 0. + filters = frappe._dict( + { + "company": "_Test Company", + "fiscal_year": FISCAL_YEAR, + "period": "Yearly", + "based_on": "Item", + "group_by": "Customer", + } + ) + + make_quotation( + item="_Test Item", party_name="_Test Customer", qty=2, rate=150, transaction_date=TXN_DATE + ) + + columns, data, _message, chart = execute(filters) + chart_total = sum(chart["data"]["datasets"][0]["values"]) + + self.assertGreater(chart_total, 0) + self.assertEqual(chart_total, 300) diff --git a/erpnext/selling/report/sales_order_trends/sales_order_trends.py b/erpnext/selling/report/sales_order_trends/sales_order_trends.py index e819ab949c3..fb35df14a65 100644 --- a/erpnext/selling/report/sales_order_trends/sales_order_trends.py +++ b/erpnext/selling/report/sales_order_trends/sales_order_trends.py @@ -4,7 +4,6 @@ from frappe import _ -import erpnext from erpnext.controllers.trends import get_columns, get_data @@ -40,9 +39,15 @@ def get_chart_data(data, conditions, filters): labels = [column.split(":")[0] for column in columns] datapoints = [0] * len(labels) + group_by_col_idx = None + if filters.get("group_by"): + group_by_col_idx = conditions["columns"].index(conditions["grbc"][0]) + for row in data: - # If group by filter, don't add first row of group (it's already summed) - if not row[start]: + # Skip the final grand-total row + if row[0] == f"'{_('Total')}'": + continue + if group_by_col_idx is not None and row[group_by_col_idx] == "": continue # Remove None values and compute only periodic data row = [x if x else 0 for x in row[start:-2]] @@ -51,7 +56,6 @@ def get_chart_data(data, conditions, filters): for i in range(len(row)): datapoints[i] += row[i] - company_currency = erpnext.get_company_currency(filters.get("company")) return { "data": { "labels": labels, @@ -61,5 +65,5 @@ def get_chart_data(data, conditions, filters): "lineOptions": {"regionFill": 1}, "fieldtype": "Currency", "options": "currency", - "currency": company_currency, + "currency": conditions.get("company_currency"), } diff --git a/erpnext/selling/report/sales_order_trends/test_sales_order_trends.py b/erpnext/selling/report/sales_order_trends/test_sales_order_trends.py new file mode 100644 index 00000000000..47a1c9679f8 --- /dev/null +++ b/erpnext/selling/report/sales_order_trends/test_sales_order_trends.py @@ -0,0 +1,213 @@ +# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors +# License: GNU General Public License v3. See license.txt + +import frappe +from frappe import _ +from frappe.utils import today + +from erpnext.accounts.utils import get_fiscal_year +from erpnext.tests.utils import ERPNextTestSuite + + +class TestSalesOrderTrends(ERPNextTestSuite): + def test_report_executes_with_group_by(self): + # trends.get_data builds per-period SUM(CASE ...) aggregates (converted from MySQL SUM(IF)), + # groups by the based-on KEY only (non-key descriptive columns like item_name/territory are + # MAX()-aggregated so the report stays one row per key on both engines), and uses a based_on_key + # for the group-by detail subqueries. Setting group_by exercises that full path on both engines. + 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(item_code="_Test Item", qty=3, rate=100) + + filters = { + "company": "_Test Company", + "period": "Monthly", + "based_on": "Item", + "group_by": "Customer", + } + columns, data, _chart_none, _chart = execute(filters) + + 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) + + def test_total_row_not_double_counted_in_chart(self): + # Regression test for the fix in trends.calculate_total_row that populates the + # Total row's Currency column. Before the fix in get_chart_data (skipping the + # Total row by label instead of `if not row[start]`), that populated Currency + # cell made the Total-row-skip guard falsy, so the already-summed Total row got + # added into the chart a second time (an SO of qty=3, rate=100 -> 300 read as 600). + 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(item_code="_Test Item", qty=3, rate=100, transaction_date=today()) + + fiscal_year = get_fiscal_year(today())[0] + filters = frappe._dict( + { + "company": "_Test Company", + "fiscal_year": fiscal_year, + "period": "Monthly", + "based_on": "Item", + } + ) + + columns, data, _message, chart = execute(filters) + self.assertTrue(columns) + self.assertTrue(data) + + total_row = next(row for row in data if row[0] == f"'{_('Total')}'") + expected_total = total_row[-1] # Total(Amt) is the last column + + chart_total = sum(chart["data"]["datasets"][0]["values"]) + self.assertEqual(chart_total, expected_total) + self.assertEqual(chart_total, 300) + + def test_chart_currency_matches_company_currency(self): + # Regression test: the chart's "currency" key should reflect the transacting + # company's currency (conditions["company_currency"]), not a stale global default. + 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(item_code="_Test Item", qty=1, rate=100, transaction_date=today()) + + fiscal_year = get_fiscal_year(today())[0] + filters = frappe._dict( + { + "company": "_Test Company", + "fiscal_year": fiscal_year, + "period": "Monthly", + "based_on": "Item", + } + ) + + _columns, _data, _message, chart = execute(filters) + expected_currency = frappe.get_cached_value("Company", "_Test Company", "default_currency") + self.assertEqual(chart["currency"], expected_currency) + + def test_group_by_chart_matches_table_total_with_mixed_group_sizes(self): + # _Test Item is split across two customers -> two detail rows under one header row. + # _Test Item 2 has only one customer -> exactly one detail row under its header row. + # A regression that double-counts header rows would inflate the chart above 600; + # a regression that zeroes single-group rows would report less than 600. + 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( + item_code="_Test Item", customer="_Test Customer", qty=3, rate=100, transaction_date=today() + ) + make_sales_order( + item_code="_Test Item", customer="_Test Customer 1", qty=2, rate=100, transaction_date=today() + ) + make_sales_order( + item_code="_Test Item 2", customer="_Test Customer", qty=1, rate=100, transaction_date=today() + ) + + fiscal_year = get_fiscal_year(today())[0] + filters = frappe._dict( + { + "company": "_Test Company", + "fiscal_year": fiscal_year, + "period": "Monthly", + "based_on": "Item", + "group_by": "Customer", + } + ) + + columns, data, _message, chart = execute(filters) + self.assertTrue(columns) + self.assertTrue(data) + + total_row = next(row for row in data if row[0] == f"'{_('Total')}'") + expected_total = total_row[-1] + chart_total = sum(chart["data"]["datasets"][0]["values"]) + + # 300 (item/customer) + 200 (item/customer1) + 100 (item2/customer) = 600 + self.assertEqual(expected_total, 600) + self.assertEqual(chart_total, expected_total) + + def test_group_by_swapped_roles_based_on_customer_group_by_item(self): + # Same regression, opposite role assignment: based_on="Customer" with group_by="Item". + # Customer's based_on_cols (Customer, Customer Name, Territory, Currency) put the + # group_by placeholder at a different column index than the Item-based_on case above, + # exercising the alternate `inc`/`ind` arithmetic. + 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( + item_code="_Test Item", customer="_Test Customer", qty=3, rate=100, transaction_date=today() + ) + make_sales_order( + item_code="_Test Item 2", customer="_Test Customer", qty=1, rate=100, transaction_date=today() + ) + + fiscal_year = get_fiscal_year(today())[0] + filters = frappe._dict( + { + "company": "_Test Company", + "fiscal_year": fiscal_year, + "period": "Monthly", + "based_on": "Customer", + "group_by": "Item", + } + ) + + columns, data, _message, chart = execute(filters) + total_row = next(row for row in data if row[0] == f"'{_('Total')}'") + expected_total = total_row[-1] + chart_total = sum(chart["data"]["datasets"][0]["values"]) + + # 300 + 100 = 400 + self.assertEqual(expected_total, 400) + self.assertEqual(chart_total, expected_total) + + def test_group_by_single_group_value_not_zeroed(self): + # Isolates the specific failure mode flagged in review: a based_on value with exactly + # one associated group value must still contribute its real amount to the chart, not 0. + 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( + item_code="_Test Item", customer="_Test Customer", qty=2, rate=150, transaction_date=today() + ) + + fiscal_year = get_fiscal_year(today())[0] + filters = frappe._dict( + { + "company": "_Test Company", + "fiscal_year": fiscal_year, + "period": "Monthly", + "based_on": "Item", + "group_by": "Customer", + } + ) + + columns, data, _message, chart = execute(filters) + chart_total = sum(chart["data"]["datasets"][0]["values"]) + + self.assertGreater(chart_total, 0) + self.assertEqual(chart_total, 300) diff --git a/erpnext/stock/report/delivery_note_trends/delivery_note_trends.py b/erpnext/stock/report/delivery_note_trends/delivery_note_trends.py index 8e98a6832e5..1365a02ba25 100644 --- a/erpnext/stock/report/delivery_note_trends/delivery_note_trends.py +++ b/erpnext/stock/report/delivery_note_trends/delivery_note_trends.py @@ -4,7 +4,6 @@ from frappe import _ -import erpnext from erpnext.controllers.trends import get_columns, get_data @@ -15,12 +14,12 @@ def execute(filters=None): conditions = get_columns(filters, "Delivery Note") data = get_data(filters, conditions) - chart_data = get_chart_data(data, filters) + chart_data = get_chart_data(data, conditions, filters) return conditions["columns"], data, None, chart_data -def get_chart_data(data, filters): +def get_chart_data(data, conditions, filters): def wrap_in_quotes(label): return f"'{label}'" @@ -46,7 +45,6 @@ def get_chart_data(data, filters): labels.append(row[0]) datapoints.append(row[-1]) - company_currency = erpnext.get_company_currency(filters.get("company")) return { "data": { "labels": labels, @@ -55,5 +53,5 @@ def get_chart_data(data, filters): "type": "bar", "fieldtype": "Currency", "options": "currency", - "currency": company_currency, + "currency": conditions.get("company_currency"), } diff --git a/erpnext/stock/report/landed_cost_report/landed_cost_report.py b/erpnext/stock/report/landed_cost_report/landed_cost_report.py index 7b8503e8537..18473c51b24 100644 --- a/erpnext/stock/report/landed_cost_report/landed_cost_report.py +++ b/erpnext/stock/report/landed_cost_report/landed_cost_report.py @@ -4,6 +4,8 @@ import frappe from frappe import _ +import erpnext + def execute(filters: dict | None = None): columns = get_columns() @@ -26,6 +28,13 @@ def get_columns() -> list[dict]: "fieldtype": "Currency", "options": "currency", }, + { + "label": _("Currency"), + "fieldname": "currency", + "fieldtype": "Link", + "options": "Currency", + "hidden": 1, + }, { "label": _("Purchase Voucher Type"), "fieldname": "voucher_type", @@ -50,8 +59,7 @@ def get_columns() -> list[dict]: def get_data(filters) -> list[list]: - company_currency = frappe.get_cached_value("Company", filters.company, "default_currency") - + company_currency = erpnext.get_company_currency(filters.get("company")) landed_cost_vouchers = get_landed_cost_vouchers(filters) or {} landed_vouchers = list(landed_cost_vouchers.keys()) vendor_invoices = {} diff --git a/erpnext/stock/report/purchase_receipt_trends/purchase_receipt_trends.py b/erpnext/stock/report/purchase_receipt_trends/purchase_receipt_trends.py index 1f7098ba806..4210d1a3604 100644 --- a/erpnext/stock/report/purchase_receipt_trends/purchase_receipt_trends.py +++ b/erpnext/stock/report/purchase_receipt_trends/purchase_receipt_trends.py @@ -4,7 +4,6 @@ from frappe import _ -import erpnext from erpnext.controllers.trends import get_columns, get_data @@ -15,12 +14,12 @@ def execute(filters=None): conditions = get_columns(filters, "Purchase Receipt") data = get_data(filters, conditions) - chart_data = get_chart_data(data, filters) + chart_data = get_chart_data(data, conditions, filters) return conditions["columns"], data, None, chart_data -def get_chart_data(data, filters): +def get_chart_data(data, conditions, filters): def wrap_in_quotes(label): return f"'{label}'" @@ -45,7 +44,6 @@ def get_chart_data(data, filters): labels.append(row[0]) datapoints.append(row[-1]) - company_currency = erpnext.get_company_currency(filters.get("company")) return { "data": { @@ -56,5 +54,5 @@ def get_chart_data(data, filters): "colors": ["#5e64ff"], "fieldtype": "Currency", "options": "currency", - "currency": company_currency, + "currency": conditions.get("company_currency"), }