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) <noreply@anthropic.com>
This commit is contained in:
Mihir Kandoi
2026-06-21 04:49:03 +05:30
parent 9fb08153d6
commit 7fe79b115d
2 changed files with 28 additions and 12 deletions

View File

@@ -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:

View File

@@ -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)