From a8e235116906b7a125c1fa9b45c15e7bcfac042b Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Tue, 11 Aug 2026 16:03:49 +0530 Subject: [PATCH 1/3] perf(stock): skip item metadata rewrite when settings unchanged Every Stock Settings save rewrote the Item naming property setters and the barcode visibility property setters. make_property_setter without a doctype fans out to every doctype that has the fieldname and clears each doctype's cache, and set_by_naming_series also backfills tabItem. Gate both on has_value_changed. Item naming behaviour is unaffected: it reads the item_naming_by default, which is still set on every save. --- .../doctype/stock_settings/stock_settings.py | 42 ++++++++++++------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/erpnext/stock/doctype/stock_settings/stock_settings.py b/erpnext/stock/doctype/stock_settings/stock_settings.py index cefb321c791..c986a4fa115 100644 --- a/erpnext/stock/doctype/stock_settings/stock_settings.py +++ b/erpnext/stock/doctype/stock_settings/stock_settings.py @@ -84,22 +84,8 @@ class StockSettings(Document): ]: frappe.db.set_default(key, self.get(key, "")) - from erpnext.utilities.naming import set_by_naming_series - - set_by_naming_series( - "Item", - "item_code", - self.get("item_naming_by") == "Naming Series", - hide_name_field=True, - make_mandatory=0, - ) - - # show/hide barcode field - for name in ["barcode", "barcodes", "scan_barcode"]: - frappe.make_property_setter( - {"fieldname": name, "property": "hidden", "value": 0 if self.show_barcode_field else 1}, - validate_fields_for_doctype=False, - ) + self.update_item_naming_settings() + self.update_barcode_field_visibility() self.validate_over_delivery_receipt_allowance() self.validate_serial_and_batch_no_settings() @@ -113,6 +99,30 @@ class StockSettings(Document): self.change_precision_for_stock_entry() self.validate_do_not_use_batchwise_valuation() + def update_item_naming_settings(self): + if not self.has_value_changed("item_naming_by"): + return + + from erpnext.utilities.naming import set_by_naming_series + + set_by_naming_series( + "Item", + "item_code", + self.get("item_naming_by") == "Naming Series", + hide_name_field=True, + make_mandatory=0, + ) + + def update_barcode_field_visibility(self): + if not self.has_value_changed("show_barcode_field"): + return + + for name in ["barcode", "barcodes", "scan_barcode"]: + frappe.make_property_setter( + {"fieldname": name, "property": "hidden", "value": 0 if self.show_barcode_field else 1}, + validate_fields_for_doctype=False, + ) + def validate_over_delivery_receipt_allowance(self): if not self.over_delivery_receipt_allowance: self.role_allowed_to_over_deliver_receive = None From 7aa6a07e7477c4d625466bff29d0e012111ead4a Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Tue, 11 Aug 2026 16:03:50 +0530 Subject: [PATCH 2/3] test(stock): cover item metadata rewrite gating --- .../stock_settings/test_stock_settings.py | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/erpnext/stock/doctype/stock_settings/test_stock_settings.py b/erpnext/stock/doctype/stock_settings/test_stock_settings.py index 112fa52401d..398dbfeac02 100644 --- a/erpnext/stock/doctype/stock_settings/test_stock_settings.py +++ b/erpnext/stock/doctype/stock_settings/test_stock_settings.py @@ -2,6 +2,8 @@ # See license.txt +from unittest.mock import patch + import frappe from erpnext.tests.utils import ERPNextTestSuite @@ -51,3 +53,32 @@ class TestStockSettings(ERPNextTestSuite): ) item.delete() + + def test_unrelated_change_does_not_update_item_metadata(self): + settings = frappe.get_single("Stock Settings") + settings.allow_partial_reservation = not settings.allow_partial_reservation + + with ( + patch("erpnext.utilities.naming.set_by_naming_series") as set_by_naming_series, + patch("frappe.make_property_setter") as make_property_setter, + ): + settings.save() + + set_by_naming_series.assert_not_called() + make_property_setter.assert_not_called() + + def test_item_metadata_updates_when_related_settings_change(self): + settings = frappe.get_single("Stock Settings") + settings.item_naming_by = ( + "Item Code" if settings.item_naming_by == "Naming Series" else "Naming Series" + ) + settings.show_barcode_field = not settings.show_barcode_field + + with ( + patch("erpnext.utilities.naming.set_by_naming_series") as set_by_naming_series, + patch("frappe.make_property_setter") as make_property_setter, + ): + settings.save() + + set_by_naming_series.assert_called_once() + self.assertEqual(make_property_setter.call_count, 3) From d6e2d7105f781bbe3d4285beffe7c890553fec22 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Tue, 11 Aug 2026 16:03:51 +0530 Subject: [PATCH 3/3] perf(tests): run sales partner summary reports on shared fixtures Both sales partner summary suites created identical submitted, draft, cancelled, and returned transactions per doctype. Run both reports against one fixture set and receive stock only for Delivery Note and POS Invoice, the doctypes that consume it. --- .../test_sales_partner_commission_summary.py | 36 ++-- .../test_sales_partner_transaction_summary.py | 160 +----------------- .../test_utils.py | 105 ++++++++++++ 3 files changed, 134 insertions(+), 167 deletions(-) create mode 100644 erpnext/selling/report/sales_partner_transaction_summary/test_utils.py diff --git a/erpnext/selling/report/sales_partner_commission_summary/test_sales_partner_commission_summary.py b/erpnext/selling/report/sales_partner_commission_summary/test_sales_partner_commission_summary.py index 32f71f12aaf..99e89520080 100644 --- a/erpnext/selling/report/sales_partner_commission_summary/test_sales_partner_commission_summary.py +++ b/erpnext/selling/report/sales_partner_commission_summary/test_sales_partner_commission_summary.py @@ -9,6 +9,9 @@ from frappe.utils.data import comma_or from erpnext.selling.report.sales_partner_commission_summary.sales_partner_commission_summary import ( SALES_TRANSACTION_DOCTYPES, ) +from erpnext.selling.report.sales_partner_transaction_summary.test_utils import ( + SalesPartnerTransactionSummaryAssertions, +) from erpnext.stock.doctype.stock_entry.test_stock_entry import make_stock_entry from erpnext.tests.utils import ERPNextTestSuite @@ -63,14 +66,15 @@ class SalesPartnerSummaryReportTestMixin(ERPNextTestSuite): self.make_transaction_func = make_transaction_funcs[doctype] - make_stock_entry( - item_code="_Test Item 2", - qty=10, - company="_Test Company", - to_warehouse="_Test Warehouse - _TC", - purpose="Material Receipt", - posting_date="2026-01-01", - ) + if doctype in {"Delivery Note", "POS Invoice"}: + make_stock_entry( + item_code="_Test Item 2", + qty=10, + company="_Test Company", + to_warehouse="_Test Warehouse - _TC", + purpose="Material Receipt", + posting_date="2026-01-01", + ) if doctype == "POS Invoice": POSInvoiceTestMixin.setUp(self) @@ -246,7 +250,9 @@ class SalesPartnerSummaryReportTestMixin(ERPNextTestSuite): self.returned_doc.submit() -class TestSalesPartnerCommissionSummary(SalesPartnerSummaryReportTestMixin): +class TestSalesPartnerSummaryReports( + SalesPartnerSummaryReportTestMixin, SalesPartnerTransactionSummaryAssertions +): def setUp(self): self.filters = { "company": "_Test Company", @@ -262,29 +268,33 @@ class TestSalesPartnerCommissionSummary(SalesPartnerSummaryReportTestMixin): def test_posting_date_column_label(self): self.assert_posting_date_label() - def test_sales_order_sp_commission_summary(self): + def test_sales_order_sp_summaries(self): self.filters["doctype"] = "Sales Order" self.create_transactions(self.filters["doctype"]) self.assert_sales_partner_commission_summary_report() + self.assert_sales_partner_transaction_summary_report() - def test_sales_invoice_sp_commission_summary(self): + def test_sales_invoice_sp_summaries(self): self.filters["doctype"] = "Sales Invoice" self.create_transactions(self.filters["doctype"]) self.assert_sales_partner_commission_summary_report() + self.assert_sales_partner_transaction_summary_report() - def test_delivery_note_sp_commission_summary(self): + def test_delivery_note_sp_summaries(self): self.filters["doctype"] = "Delivery Note" self.create_transactions(self.filters["doctype"]) self.assert_sales_partner_commission_summary_report() + self.assert_sales_partner_transaction_summary_report() - def test_pos_invoice_sp_commission_summary(self): + def test_pos_invoice_sp_summaries(self): self.filters["doctype"] = "POS Invoice" self.create_transactions(self.filters["doctype"]) self.assert_sales_partner_commission_summary_report() + self.assert_sales_partner_transaction_summary_report() def assert_sales_partner_commission_summary_report(self): report_data = run(self.report_name, self.filters) diff --git a/erpnext/selling/report/sales_partner_transaction_summary/test_sales_partner_transaction_summary.py b/erpnext/selling/report/sales_partner_transaction_summary/test_sales_partner_transaction_summary.py index 45b4efee867..b2c7604f863 100644 --- a/erpnext/selling/report/sales_partner_transaction_summary/test_sales_partner_transaction_summary.py +++ b/erpnext/selling/report/sales_partner_transaction_summary/test_sales_partner_transaction_summary.py @@ -1,14 +1,17 @@ # Copyright (c) 2026, Frappe Technologies Pvt. Ltd. and contributors # For license information, please see license.txt -from frappe.desk.query_report import run - from erpnext.selling.report.sales_partner_commission_summary.test_sales_partner_commission_summary import ( SalesPartnerSummaryReportTestMixin, ) +from erpnext.selling.report.sales_partner_transaction_summary.test_utils import ( + SalesPartnerTransactionSummaryAssertions, +) -class TestSalesPartnerTransactionSummary(SalesPartnerSummaryReportTestMixin): +class TestSalesPartnerTransactionSummary( + SalesPartnerSummaryReportTestMixin, SalesPartnerTransactionSummaryAssertions +): def setUp(self): self.filters = { "company": "_Test Company", @@ -25,159 +28,8 @@ class TestSalesPartnerTransactionSummary(SalesPartnerSummaryReportTestMixin): def test_posting_date_column_label(self): self.assert_posting_date_label() - def test_sales_order_sp_transaction_summary(self): - self.filters["doctype"] = "Sales Order" - self.create_transactions(self.filters["doctype"]) - - self.assert_sales_partner_transaction_summary_report() - def test_sales_invoice_sp_transaction_summary(self): self.filters["doctype"] = "Sales Invoice" self.create_transactions(self.filters["doctype"]) self.assert_sales_partner_transaction_summary_report() - - def test_delivery_note_sp_transaction_summary(self): - self.filters["doctype"] = "Delivery Note" - self.create_transactions(self.filters["doctype"]) - - self.assert_sales_partner_transaction_summary_report() - - def test_pos_invoice_sp_transaction_summary(self): - self.filters["doctype"] = "POS Invoice" - self.create_transactions(self.filters["doctype"]) - - self.assert_sales_partner_transaction_summary_report() - - def assert_sales_partner_transaction_summary_report(self): - report_data = run(self.report_name, self.filters) - - self.report_result = report_data.get("result") - self.report_result_without_total_row = self.report_result[:-1] - - self.assertIsNotNone(self.report_result_without_total_row) - - self.assert_7pc_commission() - self.assert_5pc_commission_with_multiple_items() - self.assert_doc_with_no_sp() - self.assert_doc_with_posting_date_out_of_range() - self.assert_doc_with_revoked_commission() - self.assert_doc_not_submitted() - self.assert_doc_cancelled() - self.assert_commission() - - if self.filters["doctype"] != "Sales Order": - self.assert_returned_doc() - - def assert_7pc_commission(self): - doc_name = self.seven_pc_doc.name - - row = next((row for row in self.report_result_without_total_row if row.get("name") == doc_name), None) - - self.assertIsNotNone(row) - - self.assertEqual(row["customer"], "_Test Customer") - self.assertEqual(row["item_code"], "_Test Item") - self.assertEqual(row["item_group"], "_Test Item Group") - self.assertEqual(row["amount"], 1000) - self.assertEqual(row["commission_rate"], 7) - self.assertEqual(row["commission"], 70) - - def assert_5pc_commission_with_multiple_items(self): - doc_name = self.five_pc_doc.name - - row1 = next( - ( - row - for row in self.report_result_without_total_row - if row.get("name") == doc_name and row.get("item_code") == "_Test Item" - ), - None, - ) - self.assertIsNotNone(row1) - - row2 = next( - ( - row - for row in self.report_result_without_total_row - if row.get("name") == doc_name and row.get("item_code") == "_Test Item 2" - ), - None, - ) - self.assertIsNotNone(row2) - - self.assertEqual(row1["amount"], 120) - self.assertEqual(row1["commission_rate"], 5) - self.assertEqual(row1["commission"], 6) - - self.assertEqual(row2["amount"], 120) - self.assertEqual(row2["commission_rate"], 5) - self.assertEqual(row2["commission"], 6) - - def assert_doc_with_no_sp(self): - doc_name = self.no_sp_doc.name - - row = next((row for row in self.report_result_without_total_row if row.get("name") == doc_name), None) - - self.assertIsNone(row) - - def assert_doc_with_posting_date_out_of_range(self): - doc_name = self.date_out_of_range_doc.name - - row = next((row for row in self.report_result_without_total_row if row.get("name") == doc_name), None) - - self.assertIsNone(row) - - def assert_doc_with_revoked_commission(self): - doc_name = self.revoked_comm_doc.name - - row = next((row for row in self.report_result_without_total_row if row.get("name") == doc_name), None) - - self.assertIsNotNone(row) - self.assertEqual(row["amount"], 800) - self.assertEqual(row["commission_rate"], 7) - self.assertEqual(row["commission"], 0) - - def assert_doc_not_submitted(self): - doc_name = self.doc_not_submitted.name - - row = next((row for row in self.report_result_without_total_row if row.get("name") == doc_name), None) - - self.assertIsNone(row) - - def assert_doc_cancelled(self): - doc_name = self.cancelled_doc.name - - row = next((row for row in self.report_result_without_total_row if row.get("name") == doc_name), None) - - self.assertIsNone(row) - - def assert_commission(self): - total_row = self.report_result[-1] - - # Total Amount - self.assertEqual(total_row[-4], 2040) - - # Total Commission - self.assertEqual(total_row[-1], 82) - - def assert_returned_doc(self): - doc_name = self.to_be_returned_doc.name - returned_doc_name = self.returned_doc.name - - outward_row = next( - (row for row in self.report_result_without_total_row if row.get("name") == doc_name), None - ) - inward_row = next( - (row for row in self.report_result_without_total_row if row.get("name") == returned_doc_name), - None, - ) - - self.assertIsNotNone(outward_row) - self.assertIsNotNone(inward_row) - - self.assertEqual(outward_row["amount"], 900) - self.assertEqual(outward_row["commission"], 45) - - self.assertEqual(inward_row["amount"], -900) - self.assertEqual(inward_row["commission"], -45) diff --git a/erpnext/selling/report/sales_partner_transaction_summary/test_utils.py b/erpnext/selling/report/sales_partner_transaction_summary/test_utils.py new file mode 100644 index 00000000000..df8f5cc76d6 --- /dev/null +++ b/erpnext/selling/report/sales_partner_transaction_summary/test_utils.py @@ -0,0 +1,105 @@ +# Copyright (c) 2026, Frappe Technologies Pvt. Ltd. and contributors +# For license information, please see license.txt + +from frappe.desk.query_report import run + + +class SalesPartnerTransactionSummaryAssertions: + def assert_sales_partner_transaction_summary_report(self): + filters = self.filters.copy() + filters["show_return_entries"] = 1 + report_data = run("Sales Partner Transaction Summary", filters) + + self.transaction_report_result = report_data.get("result") + self.transaction_report_result_without_total_row = self.transaction_report_result[:-1] + + self.assertIsNotNone(self.transaction_report_result_without_total_row) + + self.assert_transaction_7pc_commission() + self.assert_transaction_5pc_commission_with_multiple_items() + self.assert_transaction_doc_with_no_sp() + self.assert_transaction_doc_with_posting_date_out_of_range() + self.assert_transaction_doc_with_revoked_commission() + self.assert_transaction_doc_not_submitted() + self.assert_transaction_doc_cancelled() + self.assert_transaction_commission() + + if self.filters["doctype"] != "Sales Order": + self.assert_transaction_returned_doc() + + def assert_transaction_7pc_commission(self): + row = self._get_transaction_report_row(self.seven_pc_doc.name) + + self.assertIsNotNone(row) + self.assertEqual(row["customer"], "_Test Customer") + self.assertEqual(row["item_code"], "_Test Item") + self.assertEqual(row["item_group"], "_Test Item Group") + self.assertEqual(row["amount"], 1000) + self.assertEqual(row["commission_rate"], 7) + self.assertEqual(row["commission"], 70) + + def assert_transaction_5pc_commission_with_multiple_items(self): + row1 = self._get_transaction_report_row(self.five_pc_doc.name, "_Test Item") + self.assertIsNotNone(row1) + + row2 = self._get_transaction_report_row(self.five_pc_doc.name, "_Test Item 2") + self.assertIsNotNone(row2) + + self.assertEqual(row1["amount"], 120) + self.assertEqual(row1["commission_rate"], 5) + self.assertEqual(row1["commission"], 6) + + self.assertEqual(row2["amount"], 120) + self.assertEqual(row2["commission_rate"], 5) + self.assertEqual(row2["commission"], 6) + + def assert_transaction_doc_with_no_sp(self): + row = self._get_transaction_report_row(self.no_sp_doc.name) + self.assertIsNone(row) + + def assert_transaction_doc_with_posting_date_out_of_range(self): + row = self._get_transaction_report_row(self.date_out_of_range_doc.name) + self.assertIsNone(row) + + def assert_transaction_doc_with_revoked_commission(self): + row = self._get_transaction_report_row(self.revoked_comm_doc.name) + + self.assertIsNotNone(row) + self.assertEqual(row["amount"], 800) + self.assertEqual(row["commission_rate"], 7) + self.assertEqual(row["commission"], 0) + + def assert_transaction_doc_not_submitted(self): + row = self._get_transaction_report_row(self.doc_not_submitted.name) + self.assertIsNone(row) + + def assert_transaction_doc_cancelled(self): + row = self._get_transaction_report_row(self.cancelled_doc.name) + self.assertIsNone(row) + + def assert_transaction_commission(self): + total_row = self.transaction_report_result[-1] + + self.assertEqual(total_row[-4], 2040) + self.assertEqual(total_row[-1], 82) + + def assert_transaction_returned_doc(self): + outward_row = self._get_transaction_report_row(self.to_be_returned_doc.name) + inward_row = self._get_transaction_report_row(self.returned_doc.name) + + self.assertIsNotNone(outward_row) + self.assertIsNotNone(inward_row) + self.assertEqual(outward_row["amount"], 900) + self.assertEqual(outward_row["commission"], 45) + self.assertEqual(inward_row["amount"], -900) + self.assertEqual(inward_row["commission"], -45) + + def _get_transaction_report_row(self, doc_name, item_code=None): + return next( + ( + row + for row in self.transaction_report_result_without_total_row + if row.get("name") == doc_name and (not item_code or row.get("item_code") == item_code) + ), + None, + )