From f98975f51a62d611f536368671c3dbaf81d61eb9 Mon Sep 17 00:00:00 2001 From: diptanilsaha Date: Thu, 21 May 2026 12:16:01 +0530 Subject: [PATCH] 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