From 91a2a7b0a0e1015b9725df0bea4e9f4aa5a54a73 Mon Sep 17 00:00:00 2001 From: diptanilsaha Date: Thu, 21 May 2026 12:15:15 +0530 Subject: [PATCH 1/5] refactor: migrate get_parent_customer_groups to query builder Co-Authored-By: Claude Sonnet 4.6 --- .../setup/doctype/customer_group/customer_group.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/erpnext/setup/doctype/customer_group/customer_group.py b/erpnext/setup/doctype/customer_group/customer_group.py index 9a05760ec29..d0af3af7ba3 100644 --- a/erpnext/setup/doctype/customer_group/customer_group.py +++ b/erpnext/setup/doctype/customer_group/customer_group.py @@ -75,13 +75,11 @@ class CustomerGroup(NestedSet): def get_parent_customer_groups(customer_group): lft, rgt = frappe.db.get_value("Customer Group", customer_group, ["lft", "rgt"]) - - return frappe.db.sql( - """select name from `tabCustomer Group` - where lft <= %s and rgt >= %s - order by lft asc""", - (lft, rgt), - as_dict=True, + return frappe.get_all( + "Customer Group", + filters=[["lft", "<=", lft], ["rgt", ">=", rgt]], + fields=["name"], + order_by="lft asc", ) From cb610b79d2156e8b9c3d78a00ce0f63e4adfbbe1 Mon Sep 17 00:00:00 2001 From: diptanilsaha Date: Thu, 21 May 2026 12:15:26 +0530 Subject: [PATCH 2/5] feat: add get_parent_supplier_groups using query builder Co-Authored-By: Claude Sonnet 4.6 --- erpnext/setup/doctype/supplier_group/supplier_group.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/erpnext/setup/doctype/supplier_group/supplier_group.py b/erpnext/setup/doctype/supplier_group/supplier_group.py index 5e1cab40450..a5ff5b8b6b4 100644 --- a/erpnext/setup/doctype/supplier_group/supplier_group.py +++ b/erpnext/setup/doctype/supplier_group/supplier_group.py @@ -70,3 +70,13 @@ class SupplierGroup(NestedSet): def on_trash(self): NestedSet.validate_if_child_exists(self) frappe.utils.nestedset.update_nsm(self) + + +def get_parent_supplier_groups(supplier_group): + lft, rgt = frappe.db.get_value("Supplier Group", supplier_group, ["lft", "rgt"]) + return frappe.get_all( + "Supplier Group", + filters=[["lft", "<=", lft], ["rgt", ">=", rgt]], + fields=["name"], + order_by="lft asc", + ) From f98975f51a62d611f536368671c3dbaf81d61eb9 Mon Sep 17 00:00:00 2001 From: diptanilsaha Date: Thu, 21 May 2026 12:16:01 +0530 Subject: [PATCH 3/5] refactor: rewrite get_tax_template using query builder Migrates from raw frappe.db.sql with string interpolation to frappe.qb. Adds hierarchical supplier_group matching (mirrors customer_group behaviour). Removes unused get_customer_group_condition helper. Co-Authored-By: Claude Sonnet 4.6 --- erpnext/accounts/doctype/tax_rule/tax_rule.py | 59 +++++++++---------- 1 file changed, 29 insertions(+), 30 deletions(-) diff --git a/erpnext/accounts/doctype/tax_rule/tax_rule.py b/erpnext/accounts/doctype/tax_rule/tax_rule.py index c7eb11e071a..fb91fef654e 100644 --- a/erpnext/accounts/doctype/tax_rule/tax_rule.py +++ b/erpnext/accounts/doctype/tax_rule/tax_rule.py @@ -14,6 +14,7 @@ from frappe.utils import cstr from frappe.utils.nestedset import get_root_of from erpnext.setup.doctype.customer_group.customer_group import get_parent_customer_groups +from erpnext.setup.doctype.supplier_group.supplier_group import get_parent_supplier_groups class IncorrectCustomerGroup(frappe.ValidationError): @@ -176,38 +177,44 @@ def get_party_details(party: str | None, party_type: str, args: dict | None = No def get_tax_template(posting_date, args): """Get matching tax rule""" args = frappe._dict(args) - conditions = [] + + TaxRule = DocType("Tax Rule") + query = frappe.qb.from_(TaxRule).select("*") if posting_date: - conditions.append( - f"""(from_date is null or from_date <= '{posting_date}') - and (to_date is null or to_date >= '{posting_date}')""" + query = query.where( + (TaxRule.from_date.isnull() | (TaxRule.from_date <= posting_date)) + & (TaxRule.to_date.isnull() | (TaxRule.to_date >= posting_date)) ) else: - conditions.append("(from_date is null) and (to_date is null)") + query = query.where(TaxRule.from_date.isnull() & TaxRule.to_date.isnull()) - conditions.append( - "ifnull(tax_category, '') = {}".format(frappe.db.escape(cstr(args.get("tax_category")), False)) - ) - if "tax_category" in args.keys(): - del args["tax_category"] + def get_group_ancestors(doctype, get_parents, value): + if not value: + value = get_root_of(doctype) + return [""] + [d.name for d in get_parents(value)] + + group_fields = { + "customer_group": ("Customer Group", get_parent_customer_groups), + "supplier_group": ("Supplier Group", get_parent_supplier_groups), + } + + args.setdefault("tax_category", "") for key, value in args.items(): if key == "use_for_shopping_cart": - conditions.append(f"use_for_shopping_cart = {1 if value else 0}") - elif key == "customer_group": - if not value: - value = get_root_of("Customer Group") - customer_group_condition = get_customer_group_condition(value) - conditions.append(f"ifnull({key}, '') in ('', {customer_group_condition})") + query = query.where(TaxRule.use_for_shopping_cart == value) + elif key == "tax_category": + query = query.where(IfNull(TaxRule.tax_category, "") == (value or "")) + elif key in group_fields: + doctype, get_parents = group_fields[key] + query = query.where( + IfNull(TaxRule[key], "").isin(get_group_ancestors(doctype, get_parents, value)) + ) else: - conditions.append(f"ifnull({key}, '') in ('', {frappe.db.escape(cstr(value))})") + query = query.where(IfNull(TaxRule[key], "").isin(["", value or ""])) - tax_rule = frappe.db.sql( - """select * from `tabTax Rule` - where {}""".format(" and ".join(conditions)), - as_dict=True, - ) + tax_rule = query.run(as_dict=True) if not tax_rule: return None @@ -236,11 +243,3 @@ def get_tax_template(posting_date, args): return None return tax_template - - -def get_customer_group_condition(customer_group): - condition = "" - customer_groups = ["%s" % (frappe.db.escape(d.name)) for d in get_parent_customer_groups(customer_group)] - if customer_groups: - condition = ",".join(["%s"] * len(customer_groups)) % (tuple(customer_groups)) - return condition From 4d43c74f5f7ebf3cb23ae64f178ba74f26988581 Mon Sep 17 00:00:00 2001 From: diptanilsaha Date: Thu, 21 May 2026 12:16:22 +0530 Subject: [PATCH 4/5] fix: default use_for_shopping_cart to 0 in set_taxes Ensures regular transactions only match tax rules where use_for_shopping_cart = 0, preventing webshop-specific rules from applying to standard documents. Co-Authored-By: Claude Sonnet 4.6 --- erpnext/accounts/party.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/accounts/party.py b/erpnext/accounts/party.py index bb603c7bb1b..fa82b3e9188 100644 --- a/erpnext/accounts/party.py +++ b/erpnext/accounts/party.py @@ -762,7 +762,7 @@ def set_taxes( args.update({"tax_type": "Purchase"}) if use_for_shopping_cart: - args.update({"use_for_shopping_cart": use_for_shopping_cart}) + args.update({"use_for_shopping_cart": cint(use_for_shopping_cart)}) return get_tax_template(posting_date, args) From 8c4311872569d38ee8c989eff46905825a5728fe Mon Sep 17 00:00:00 2001 From: diptanilsaha Date: Thu, 21 May 2026 12:27:39 +0530 Subject: [PATCH 5/5] test: add tests for supplier group hierarchy and use_for_shopping_cart filter Co-Authored-By: Claude Sonnet 4.6 --- .../doctype/tax_rule/test_tax_rule.py | 111 ++++++++++++++++++ 1 file changed, 111 insertions(+) diff --git a/erpnext/accounts/doctype/tax_rule/test_tax_rule.py b/erpnext/accounts/doctype/tax_rule/test_tax_rule.py index cd47d689c60..d36011bc5ff 100644 --- a/erpnext/accounts/doctype/tax_rule/test_tax_rule.py +++ b/erpnext/accounts/doctype/tax_rule/test_tax_rule.py @@ -61,6 +61,117 @@ class TestTaxRule(ERPNextTestSuite): "_Test Sales Taxes and Charges Template - _TC", ) + def test_for_parent_supplier_group(self): + purchase_template = "_Test Purchase Taxes and Charges Template - _TC" + if not frappe.db.exists("Purchase Taxes and Charges Template", purchase_template): + frappe.get_doc( + { + "doctype": "Purchase Taxes and Charges Template", + "title": "_Test Purchase Taxes and Charges Template", + "company": "_Test Company", + "taxes": [ + { + "account_head": "_Test Account VAT - _TC", + "charge_type": "On Net Total", + "description": "VAT", + "doctype": "Purchase Taxes and Charges", + "cost_center": "Main - _TC", + "rate": 6, + } + ], + } + ).insert() + + make_tax_rule( + supplier_group="All Supplier Groups", + tax_type="Purchase", + purchase_tax_template=purchase_template, + priority=1, + use_for_shopping_cart=0, + from_date="2015-01-01", + save=1, + ) + + # "_Test Supplier Group" has "All Supplier Groups" as its parent — should match hierarchically + self.assertEqual( + get_tax_template( + "2015-01-01", + { + "supplier_group": "_Test Supplier Group", + "tax_type": "Purchase", + "use_for_shopping_cart": 0, + }, + ), + purchase_template, + ) + + def test_use_for_shopping_cart_filter(self): + city = "Test Cart City" + # higher priority ensures this rule wins when use_for_shopping_cart is not filtered + make_tax_rule( + customer="_Test Customer", + billing_city=city, + sales_tax_template="_Test Sales Taxes and Charges Template - _TC", + use_for_shopping_cart=0, + priority=2, + save=1, + ) + make_tax_rule( + customer="_Test Customer", + billing_city=city, + sales_tax_template="_Test Sales Taxes and Charges Template 1 - _TC", + use_for_shopping_cart=1, + priority=1, + save=1, + ) + + # Cart request (use_for_shopping_cart=1) filters to cart rules only + self.assertEqual( + get_tax_template( + "2015-01-01", + {"customer": "_Test Customer", "billing_city": city, "use_for_shopping_cart": 1}, + ), + "_Test Sales Taxes and Charges Template 1 - _TC", + ) + + # Non-cart request omits use_for_shopping_cart — no filter is applied, both rules + # are candidates; non-cart rule wins by higher priority + self.assertEqual( + get_tax_template( + "2015-01-01", + {"customer": "_Test Customer", "billing_city": city}, + ), + "_Test Sales Taxes and Charges Template - _TC", + ) + + def test_use_for_shopping_cart_default(self): + city = "Test Default Cart City" + # use_for_shopping_cart not set — Check field defaults to 0 + make_tax_rule( + customer="_Test Customer", + billing_city=city, + sales_tax_template="_Test Sales Taxes and Charges Template - _TC", + use_for_shopping_cart=0, # Default is set to 1. + save=1, + ) + + # Non-cart request (no use_for_shopping_cart in args) matches the rule + self.assertEqual( + get_tax_template( + "2015-01-01", + {"customer": "_Test Customer", "billing_city": city}, + ), + "_Test Sales Taxes and Charges Template - _TC", + ) + + # Cart request (use_for_shopping_cart=1) does not match — rule has default 0 + self.assertIsNone( + get_tax_template( + "2015-01-01", + {"customer": "_Test Customer", "billing_city": city, "use_for_shopping_cart": 1}, + ) + ) + def test_conflict_with_overlapping_dates(self): tax_rule1 = make_tax_rule( customer="_Test Customer",