[fix] item attribute validation, fixes #5308

erpnext/controllers/item_variant.py

@@ -32,13 +32,13 @@ def validate_item_variant_attributes(item, args):
 	attribute_values = {}
 	for t in frappe.get_all("Item Attribute Value", fields=["parent", "attribute_value"],
 		filters={"parent": ["in", args.keys()]}):
-		
+
 		(attribute_values.setdefault(t.parent.lower(), [])).append(t.attribute_value)
 
 	numeric_attributes = frappe._dict((t.attribute.lower(), t) for t in \
 		frappe.db.sql("""select attribute, from_range, to_range, increment from `tabItem Variant Attribute`
 		where parent = %s and numeric_values=1""", (item), as_dict=1))
-		
+
 	for attribute, value in args.items():
 		if attribute.lower() in numeric_attributes:
 			numeric_attribute = numeric_attributes[attribute.lower()]
@@ -61,10 +61,10 @@ def validate_item_variant_attributes(item, args):
 			if not (is_in_range and is_incremental):
 				frappe.throw(_("Value for Attribute {0} must be within the range of {1} to {2} in the increments of {3}")\
 					.format(attribute, from_range, to_range, increment), InvalidItemAttributeValueError)
-		
+
 		elif value not in attribute_values.get(attribute.lower(), []):
 			frappe.throw(_("Value {0} for Attribute {1} does not exist in the list of valid Item Attribute Values").format(
-				value, attribute))
+				value, attribute), InvalidItemAttributeValueError)
 
 def find_variant(template, args, variant_item_code=None):
 	conditions = ["""(iv_attribute.attribute="{0}" and iv_attribute.attribute_value="{1}")"""\

erpnext/stock/dashboard/item_dashboard.js

@@ -65,7 +65,7 @@ erpnext.stock.ItemDashboard = Class.extend({
 		this.max_count = this.max_count;
 
 		// show more button
-		if(data.length===21) {
+		if(data && data.length===21) {
 			this.content.find('.more').removeClass('hidden');
 
 			// remove the last element

erpnext/stock/doctype/item/test_item.py

@@ -6,7 +6,8 @@ import unittest
 import frappe
 
 from frappe.test_runner import make_test_records
-from erpnext.controllers.item_variant import create_variant, ItemVariantExistsError, InvalidItemAttributeValueError
+from erpnext.controllers.item_variant import (create_variant, ItemVariantExistsError,
+	InvalidItemAttributeValueError)
 
 test_ignore = ["BOM"]
 test_dependencies = ["Warehouse"]
@@ -86,6 +87,18 @@ class TestItem(unittest.TestCase):
 		for key, value in to_check.iteritems():
 			self.assertEquals(value, details.get(key))
 
+	def test_item_attribute_change_after_variant(self):
+		frappe.delete_doc_if_exists("Item", "_Test Variant Item-L", force=1)
+
+		variant = create_variant("_Test Variant Item", {"Test Size": "Large"})
+		variant.save()
+
+		attribute = frappe.get_doc('Item Attribute', 'Test Size')
+		attribute.item_attribute_values = []
+
+		self.assertRaises(InvalidItemAttributeValueError, attribute.save)
+		frappe.db.rollback()
+
 	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

@@ -6,13 +6,27 @@ import frappe
 from frappe.model.document import Document
 from frappe import _
 
+from erpnext.controllers.item_variant import InvalidItemAttributeValueError
+
+
 class ItemAttributeIncrementError(frappe.ValidationError): pass
 
 class ItemAttribute(Document):
+	def __setup__(self):
+		self.flags.ignore_these_exceptions_in_test = [InvalidItemAttributeValueError]
+
 	def validate(self):
 		self.validate_numeric()
 		self.validate_duplication()
-		self.validate_attribute_values()
+
+	def on_update(self):
+		self.validate_exising_items()
+
+	def validate_exising_items(self):
+		'''Validate that if there are existing items with attributes, they are valid'''
+		for item in frappe.db.sql('''select i.name from `tabItem Variant Attribute` iva, `tabItem` i
+			where iva.attribute = %s and iva.parent = i.name and i.has_variants = 0''', self.name):
+			frappe.get_doc('Item', item[0]).validate_variant_attributes()
 
 	def validate_numeric(self):
 		if self.numeric_values:
@@ -39,19 +53,3 @@ class ItemAttribute(Document):
 			if d.abbr in abbrs:
 				frappe.throw(_("{0} must appear only once").format(d.abbr))
 			abbrs.append(d.abbr)
-
-	def validate_attribute_values(self):
-		# don't validate numeric values
-		if self.numeric_values:
-			return
-
-		attribute_values = []
-		for d in self.item_attribute_values:
-			attribute_values.append(d.attribute_value)
-
-		variant_attributes = frappe.db.sql("select DISTINCT attribute_value from `tabItem Variant Attribute` where attribute=%s", self.name)
-		if variant_attributes:
-			for d in variant_attributes:
-				if d[0] and d[0] not in attribute_values:
-					frappe.throw(_("Attribute Value {0} cannot be removed from {1} as Item Variants \
-						exist with this Attribute.").format(d[0], self.name))