From 1bc8d02cef32d8df29fa988deca3dd037466d1c5 Mon Sep 17 00:00:00 2001 From: Loic Oberle Date: Fri, 22 May 2026 11:42:06 +0200 Subject: [PATCH] refactor(queries): migrate item_query to Query Builder (#54834) * refactor(queries): migrate item_query to Query Builder Use Frappe Query Builder to ensure compatibility with PostgreSQL. The implementation still relies on raw SQL for fcond and mcond through LiteralValue to maintain compatibility with legacy filter builders. * refactor(queries): migrate item_query to Query Builder Fix the bugs found by coderabbit. For the eol condition: PostgreSQL raises DatetimeFieldOverflow when evaluating '0000-00-00' as a date literal, even inside NULLIF(). Added a db_type guard to skip the zero-date condition on PostgreSQL, where it can never be stored anyway. No generic cross-db solution found for this case; open to revisiting * refactor(queries): Rework item_query to use get_query Rework the item_query method to use get_query with the ignore_permissions flag at False * refactor(controller): Fix the query builder Fix the build query in item_query according to coderabbit * refactor(queries): explicitly add has_variants Explicitely add has_variants==0 to the query according to coderabbit feedback --- erpnext/controllers/queries.py | 157 +++++++++++++++++++-------------- 1 file changed, 93 insertions(+), 64 deletions(-) diff --git a/erpnext/controllers/queries.py b/erpnext/controllers/queries.py index 5be68e60591..07dbc43f0e1 100644 --- a/erpnext/controllers/queries.py +++ b/erpnext/controllers/queries.py @@ -9,10 +9,11 @@ import frappe from frappe import qb, scrub from frappe.desk.reportview import get_filters_cond, get_match_cond from frappe.permissions import has_permission -from frappe.query_builder import Criterion, CustomFunction -from frappe.query_builder.functions import Concat, Locate, Sum +from frappe.query_builder import Case, Criterion, DocType, Field +from frappe.query_builder.functions import Concat, CustomFunction, Length, Locate, Substring, Sum from frappe.utils import nowdate, today, unique from pypika import Order +from pypika.terms import LiteralValue import erpnext from erpnext.accounts.utils import build_qb_match_conditions @@ -187,38 +188,14 @@ def item_query( filters: dict | str | None = None, as_dict: bool = False, ): + """ + Fetch items for link fields + """ doctype = "Item" - conditions = [] if isinstance(filters, str): filters = json.loads(filters) - # Get searchfields from meta and use in Item Link field query - meta = frappe.get_meta(doctype, cached=True) - searchfields = meta.get_search_fields() - - columns = "" - extra_searchfields = [field for field in searchfields if field not in ["name", "description"]] - - if extra_searchfields: - columns += ", " + ", ".join(extra_searchfields) - - if "description" in searchfields: - columns += """, if(length(tabItem.description) > 40, \ - concat(substr(tabItem.description, 1, 40), "..."), description) as description""" - - searchfields = searchfields + [ - field - for field in [ - searchfield or "name", - "item_code", - "item_group", - "item_name", - ] - if field not in searchfields - ] - searchfields = " or ".join([field + " like %(txt)s" for field in searchfields]) - if filters and isinstance(filters, dict): if filters.get("customer") or filters.get("supplier"): party = filters.get("customer") or filters.get("supplier") @@ -251,43 +228,95 @@ def item_query( filters.pop("customer", None) filters.pop("supplier", None) - description_cond = "" - if frappe.db.estimate_count(doctype) < 50000: - # scan description only if items are less than 50000 - description_cond = "or tabItem.description LIKE %(txt)s" + item = DocType(doctype) - return frappe.db.sql( - """select - tabItem.name {columns} - from tabItem - where tabItem.docstatus < 2 - and tabItem.disabled=0 - and tabItem.has_variants=0 - and (tabItem.end_of_life > %(today)s or ifnull(tabItem.end_of_life, '0000-00-00')='0000-00-00') - and ({scond} or tabItem.item_code IN (select parent from `tabItem Barcode` where barcode LIKE %(txt)s) - {description_cond}) - {fcond} {mcond} - order by - if(locate(%(_txt)s, name), locate(%(_txt)s, name), 99999), - if(locate(%(_txt)s, item_name), locate(%(_txt)s, item_name), 99999), - idx desc, - name, item_name - limit %(start)s, %(page_len)s """.format( - columns=columns, - scond=searchfields, - fcond=get_filters_cond(doctype, filters, conditions).replace("%", "%%"), - mcond=get_match_cond(doctype).replace("%", "%%"), - description_cond=description_cond, - ), - { - "today": nowdate(), - "txt": "%%%s%%" % txt, - "_txt": txt.replace("%", ""), - "start": start, - "page_len": page_len, - }, - as_dict=as_dict, + # Condition for the date + eol = item.end_of_life + date_conditions = [eol > nowdate(), eol.isnull()] + # Add the condition if the db can evaluate it + if frappe.db.db_type not in ["postgres"]: + date_conditions.append(eol == "0000-00-00") + + date_condition = Criterion.any(date_conditions) + + # Condition for the searchfields + meta = frappe.get_meta("Item", cached=True) + searchfields = meta.get_search_fields() + query_select = [] + + extra_searchfields = [field for field in searchfields if field not in ["name", "description"]] + + for field in extra_searchfields: + query_select.append(item[field]) + + if "description" in searchfields: + description_col = ( + Case() + .when(Length(item.description) > 40, Concat(Substring(item.description, 1, 40), "...")) + .else_(item.description) + ).as_("description") + + query_select.append(description_col) + + fields_to_process = list( + dict.fromkeys( + searchfields + + [ + field + for field in [ + searchfield or "name", + "item_code", + "item_group", + "item_name", + ] + if field not in searchfields + ] + ) ) + db_fields = [f.fieldname for f in meta.fields] + ["name"] + search_str = f"%{txt}%" + search_conditions = [] + for fieldname in fields_to_process: + if fieldname in db_fields: + search_conditions.append(item[fieldname].like(search_str)) + + barcode_tbl = DocType("Item Barcode") + barcode_subquery = ( + frappe.qb.from_(barcode_tbl).select(barcode_tbl.parent).where(barcode_tbl.barcode.like(search_str)) + ) + search_conditions.append(item.item_code.isin(barcode_subquery)) + + # Condition for the description + if frappe.db.estimate_count("Item") < 50000 and "description" not in fields_to_process: + search_conditions.append(item.description.like(search_str)) + + txt_no_percent = txt.replace("%", "") + + # Building the query + query = ( + frappe.get_query(doctype, filters=filters, ignore_permissions=False) + .select(*query_select) + .where(item.docstatus < 2) + .where(item.disabled == 0) + .where(item.has_variants == 0) + .where(date_condition) + .where(Criterion.any(search_conditions)) + .orderby( + Case().when(Locate(txt_no_percent, item.name) > 0, Locate(txt_no_percent, item.name)).else_(99999) + ) + .orderby( + Case() + .when(Locate(txt_no_percent, item.item_name) > 0, Locate(txt_no_percent, item.item_name)) + .else_(99999) + ) + .orderby(item.idx, order=Order.desc) + .orderby(item.name) + .orderby(item.item_name) + .limit(page_len) + .offset(start) + ) + + return query.run(as_dict=as_dict) @frappe.whitelist()