Merge pull request #56909 from aerele/fix/sync-variant-item-code-on-abbr-rename

fix(stock): rename variant item_code/item_name when attribute abbreviation changes

erpnext/controllers/item_variant.py

@@ -186,6 +186,68 @@ def update_variant_attribute_values(item_attribute):
 	frappe.flags.attribute_values = None
 
 
+def get_attribute_abbr_renames(item_attribute):
+	"""Return the set of (current) attribute values whose abbreviation was renamed."""
+	if item_attribute.numeric_values:
+		return set()
+
+	db_value = item_attribute.get_doc_before_save()
+	if not db_value:
+		return set()
+
+	old_abbrs = {d.name: d.abbr for d in db_value.item_attribute_values}
+	changed_values = set()
+
+	for row in item_attribute.item_attribute_values:
+		if row.name in old_abbrs and old_abbrs[row.name] != row.abbr:
+			changed_values.add(row.attribute_value)
+
+	return changed_values
+
+
+def update_variant_item_codes_for_abbr_renames(item_attribute):
+	"""Rebuild item_code/item_name of variant Items affected by a renamed Item Attribute abbreviation."""
+	changed_values = get_attribute_abbr_renames(item_attribute)
+	if not changed_values:
+		return
+
+	item_variant_table = frappe.qb.DocType("Item Variant Attribute")
+	variant_names = (
+		frappe.qb.from_(item_variant_table)
+		.select(item_variant_table.parent)
+		.where(item_variant_table.attribute == item_attribute.name)
+		.where(item_variant_table.attribute_value.isin(list(changed_values)))
+		.distinct()
+		.run(pluck=True)
+	)
+
+	for variant_name in variant_names:
+		rename_variant_item_code(variant_name)
+
+
+def rename_variant_item_code(variant_name):
+	"""Recompute a variant's item_code/item_name from its template and current attribute abbreviations,
+	renaming the Item if it has changed."""
+	variant = frappe.get_doc("Item", variant_name)
+	if not variant.variant_of:
+		return
+
+	template = frappe.get_cached_doc("Item", variant.variant_of)
+
+	new_code = frappe._dict({"item_code": None, "item_name": None, "attributes": variant.attributes})
+	make_variant_item_code(template.item_code, template.item_name, new_code)
+
+	if not new_code.item_code or new_code.item_code == variant.item_code:
+		return
+
+	frappe.rename_doc("Item", variant.item_code, new_code.item_code)
+
+	# Keep item_name in lockstep with item_code: both are derived from the same abbreviation, so
+	# item_name is always rebuilt here too, even if it had since been customized away from that pattern.
+	if new_code.item_name and new_code.item_name != variant.item_name:
+		frappe.db.set_value("Item", new_code.item_code, "item_name", new_code.item_name)
+
+
 def validate_item_attribute_value(attributes_list, attribute, attribute_value, item, from_variant=True):
 	allow_rename_attribute_value = frappe.db.get_single_value(
 		"Item Variant Settings", "allow_rename_attribute_value"

erpnext/stock/doctype/item/test_item.py

@@ -506,6 +506,100 @@ class TestItem(ERPNextTestSuite):
 			"Large",
 		)
 
+	def test_rename_attribute_abbr_updates_variant_item_code(self):
+		frappe.delete_doc_if_exists("Item", "_Test Variant Item-L", force=1)
+		frappe.delete_doc_if_exists("Item", "_Test Variant Item-LRG", force=1)
+
+		variant = create_variant("_Test Variant Item", {"Test Size": "Large"})
+		variant.save()
+
+		attribute = frappe.get_doc("Item Attribute", "Test Size")
+		for row in attribute.item_attribute_values:
+			if row.attribute_value == "Large":
+				row.abbr = "LRG"
+				break
+
+		def restore_test_size_abbr():
+			doc = frappe.get_doc("Item Attribute", "Test Size")
+			for row in doc.item_attribute_values:
+				if row.attribute_value == "Large":
+					row.abbr = "L"
+					break
+			frappe.flags.attribute_values = None
+			doc.save()
+
+		self.addCleanup(restore_test_size_abbr)
+		self.addCleanup(lambda: frappe.delete_doc_if_exists("Item", "_Test Variant Item-LRG", force=1))
+
+		frappe.flags.attribute_values = None
+		attribute.save()
+
+		self.assertFalse(frappe.db.exists("Item", "_Test Variant Item-L"))
+		self.assertTrue(frappe.db.exists("Item", "_Test Variant Item-LRG"))
+		self.assertEqual(
+			frappe.db.get_value("Item", "_Test Variant Item-LRG", "item_name"),
+			"_Test Variant Item-LRG",
+		)
+
+	def test_rename_attribute_abbr_updates_variant_item_name_from_template_name(self):
+		# item_name can be derived from the template's item_name, which may differ from its
+		# item_code (e.g. a friendly display name vs. a SKU-style code). The variant's item_name
+		# must follow the abbreviation rename the same way item_code does.
+		frappe.delete_doc_if_exists("Item", "_Test Variant Item Diff-L", force=1)
+		frappe.delete_doc_if_exists("Item", "_Test Variant Item Diff-LRG", force=1)
+		frappe.delete_doc_if_exists("Item", "_Test Variant Item Diff", force=1)
+
+		template = frappe.get_doc("Item", "_Test Variant Item").as_dict()
+		template = frappe.get_doc(
+			{
+				"doctype": "Item",
+				"item_code": "_Test Variant Item Diff",
+				"item_name": "Test Variant Friendly Name",
+				"item_group": template.item_group,
+				"stock_uom": template.stock_uom,
+				"has_variants": 1,
+				"attributes": [{"attribute": "Test Size"}],
+			}
+		)
+		template.insert()
+		self.addCleanup(lambda: frappe.delete_doc_if_exists("Item", "_Test Variant Item Diff", force=1))
+
+		variant = create_variant("_Test Variant Item Diff", {"Test Size": "Large"})
+		variant.save()
+		self.assertEqual(variant.item_code, "_Test Variant Item Diff-L")
+		self.assertEqual(variant.item_name, "Test Variant Friendly Name-L")
+
+		# even a manually customized item_name (unrelated to the auto-generated pattern) must be
+		# rebuilt on abbreviation rename, since item_code and item_name are meant to stay in lockstep.
+		frappe.db.set_value("Item", variant.name, "item_name", "Custom Friendly Large Shirt Name")
+
+		attribute = frappe.get_doc("Item Attribute", "Test Size")
+		for row in attribute.item_attribute_values:
+			if row.attribute_value == "Large":
+				row.abbr = "LRG"
+				break
+
+		def restore_test_size_abbr():
+			doc = frappe.get_doc("Item Attribute", "Test Size")
+			for row in doc.item_attribute_values:
+				if row.attribute_value == "Large":
+					row.abbr = "L"
+					break
+			frappe.flags.attribute_values = None
+			doc.save()
+
+		self.addCleanup(restore_test_size_abbr)
+		self.addCleanup(lambda: frappe.delete_doc_if_exists("Item", "_Test Variant Item Diff-LRG", force=1))
+
+		frappe.flags.attribute_values = None
+		attribute.save()
+
+		self.assertFalse(frappe.db.exists("Item", "_Test Variant Item Diff-L"))
+		self.assertEqual(
+			frappe.db.get_value("Item", "_Test Variant Item Diff-LRG", "item_name"),
+			"Test Variant Friendly Name-LRG",
+		)
+
 	def test_make_item_variant(self):
 		frappe.delete_doc_if_exists("Item", "_Test Variant Item-L", force=1)
 

erpnext/stock/doctype/item_attribute/item_attribute.py

@@ -10,6 +10,7 @@ from frappe.utils import flt
 from erpnext.controllers.item_variant import (
 	InvalidItemAttributeValueError,
 	update_variant_attribute_values,
+	update_variant_item_codes_for_abbr_renames,
 	validate_is_incremental,
 	validate_item_attribute_value,
 )
@@ -46,6 +47,7 @@ class ItemAttribute(Document):
 
 	def on_update(self):
 		update_variant_attribute_values(self)
+		update_variant_item_codes_for_abbr_renames(self)
 		self.validate_exising_items()
 		self.set_enabled_disabled_in_items()