From 7fe79b115d88e35d4bd03cfd156886b2e0002d5e Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Sun, 21 Jun 2026 04:49:03 +0530 Subject: [PATCH] refactor(stock): convert ItemAttribute.validate_exising_items to query builder validate_exising_items() used a raw frappe.db.sql implicit-join to find variant items using the attribute. Convert it to a frappe.qb inner join (engine-portable, MariaDB-identical) so it no longer relies on raw SQL. Only this query is converted; develop's update_variant_attribute_values on_update hook and its imports are left intact (the staging branch's whole-file version predated and would have reverted them). Adds a focused test that creates a variant and asserts validate_exising_items finds it (the validation only raises if the converted query returned the variant row). Passes on MariaDB and Postgres. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../doctype/item_attribute/item_attribute.py | 21 ++++++++----------- .../item_attribute/test_item_attribute.py | 19 +++++++++++++++++ 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/erpnext/stock/doctype/item_attribute/item_attribute.py b/erpnext/stock/doctype/item_attribute/item_attribute.py index 2e50479b409..822d257e050 100644 --- a/erpnext/stock/doctype/item_attribute/item_attribute.py +++ b/erpnext/stock/doctype/item_attribute/item_attribute.py @@ -66,18 +66,15 @@ class ItemAttribute(Document): attributes_list = [d.attribute_value for d in self.item_attribute_values] # Get Item Variant Attribute details of variant items - items = frappe.db.sql( - """ - select - i.name, iva.attribute_value as value - from - `tabItem Variant Attribute` iva, `tabItem` i - where - iva.attribute = %(attribute)s - and iva.parent = i.name and - i.variant_of is not null and i.variant_of != ''""", - {"attribute": self.name}, - as_dict=1, + iva = frappe.qb.DocType("Item Variant Attribute") + i = frappe.qb.DocType("Item") + items = ( + frappe.qb.from_(iva) + .inner_join(i) + .on(iva.parent == i.name) + .select(i.name, iva.attribute_value.as_("value")) + .where((iva.attribute == self.name) & i.variant_of.isnotnull() & (i.variant_of != "")) + .run(as_dict=1) ) for item in items: diff --git a/erpnext/stock/doctype/item_attribute/test_item_attribute.py b/erpnext/stock/doctype/item_attribute/test_item_attribute.py index 63958733eba..2d45e94a4fc 100644 --- a/erpnext/stock/doctype/item_attribute/test_item_attribute.py +++ b/erpnext/stock/doctype/item_attribute/test_item_attribute.py @@ -30,3 +30,22 @@ class TestItemAttribute(ERPNextTestSuite): item_attribute.increment = 0.5 item_attribute.save() + + def test_validate_existing_items_finds_variants(self): + # validate_exising_items() joins Item Variant Attribute to Item to find variants using this + # attribute. Exercises the converted query builder version on both engines and asserts it + # finds the variant (the raise only fires if the query returned the variant row). + from erpnext.controllers.item_variant import InvalidItemAttributeValueError, create_variant + + frappe.delete_doc_if_exists("Item", "_Test Variant Item-L", force=1) + variant = create_variant("_Test Variant Item", {"Test Size": "Large"}) + variant.save() + self.addCleanup(frappe.delete_doc_if_exists, "Item", "_Test Variant Item-L", force=1) + + attribute = frappe.get_doc("Item Attribute", "Test Size") + attribute.item_attribute_values = [] + frappe.flags.attribute_values = None + + # "Large" is no longer a permitted value, so the variant found by validate_exising_items + # is invalid; the save must abort (and so never persists the cleared values). + self.assertRaises(InvalidItemAttributeValueError, attribute.save)