[fix] item attribute validation and tests
diff --git a/erpnext/controllers/item_variant.py b/erpnext/controllers/item_variant.py
index 3ff3d88..8294b64 100644
--- a/erpnext/controllers/item_variant.py
+++ b/erpnext/controllers/item_variant.py
@@ -24,22 +24,23 @@
if not args:
frappe.throw(_("Please specify at least one attribute in the Attributes table"))
- validate_item_variant_attributes(template, args)
-
return find_variant(template, args, variant)
def validate_item_variant_attributes(item, args=None):
+ if isinstance(item, basestring):
+ item = frappe.get_doc('Item', item)
+
if not args:
args = {d.attribute.lower():d.attribute_value for d in item.attributes}
- attribute_values = get_attribute_values()
-
- numeric_attributes = frappe._dict({d.attribute.lower(): d for d
- in item.attributes if d.numeric_values==1})
+ attribute_values, numeric_values = get_attribute_values()
for attribute, value in args.items():
- if attribute.lower() in numeric_attributes:
- numeric_attribute = numeric_attributes[attribute.lower()]
+ if not value:
+ continue
+
+ if attribute.lower() in numeric_values:
+ numeric_attribute = numeric_values[attribute.lower()]
from_range = numeric_attribute.from_range
to_range = numeric_attribute.to_range
@@ -57,22 +58,28 @@
is_incremental = remainder==0 or remainder==increment
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)
+ frappe.throw(_("Value for Attribute {0} must be within the range of {1} to {2} in the increments of {3} for Item {2}")\
+ .format(attribute, from_range, to_range, increment, item.name), 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), InvalidItemAttributeValueError)
+ frappe.throw(_("Value {0} for Attribute {1} does not exist in the list of valid Item Attribute Values for Item {2}").format(
+ value, attribute, item.name), InvalidItemAttributeValueError)
def get_attribute_values():
if not frappe.flags.attribute_values:
attribute_values = {}
+ numeric_values = {}
for t in frappe.get_all("Item Attribute Value", fields=["parent", "attribute_value"]):
- (attribute_values.setdefault(t.parent.lower(), [])).append(t.attribute_value)
+ attribute_values.setdefault(t.parent.lower(), []).append(t.attribute_value)
+
+ for t in frappe.get_all('Item Attribute',
+ fields=["name", "from_range", "to_range", "increment"], filters={'numeric_values': 1}):
+ numeric_values[t.name.lower()] = t
frappe.flags.attribute_values = attribute_values
+ frappe.flags.numeric_values = numeric_values
- return frappe.flags.attribute_values
+ return frappe.flags.attribute_values, frappe.flags.numeric_values
def find_variant(template, args, variant_item_code=None):
conditions = ["""(iv_attribute.attribute="{0}" and iv_attribute.attribute_value="{1}")"""\
@@ -126,7 +133,7 @@
variant.set("attributes", variant_attributes)
copy_attributes_to_variant(template, variant)
- make_variant_item_code(template, variant)
+ make_variant_item_code(template.item_code, variant)
return variant
@@ -144,7 +151,7 @@
for d in variant.attributes:
variant.description += "<p>" + d.attribute + ": " + cstr(d.attribute_value) + "</p>"
-def make_variant_item_code(template, variant):
+def make_variant_item_code(template_item_code, variant):
"""Uses template's item code and abbreviations to make variant's item code"""
if variant.item_code:
return
@@ -160,8 +167,10 @@
}, as_dict=True)
if not item_attribute:
- # somehow an invalid item attribute got used
return
+ # frappe.throw(_('Invalid attribute {0} {1}').format(frappe.bold(attr.attribute),
+ # frappe.bold(attr.attribute_value)), title=_('Invalid Attribute'),
+ # exc=InvalidItemAttributeValueError)
if item_attribute[0].numeric_values:
# don't generate item code if one of the attributes is numeric
@@ -170,7 +179,7 @@
abbreviations.append(item_attribute[0].abbr)
if abbreviations:
- variant.item_code = "{0}-{1}".format(template.item_code, "-".join(abbreviations))
+ variant.item_code = "{0}-{1}".format(template_item_code, "-".join(abbreviations))
if variant.item_code:
variant.item_name = variant.item_code
diff --git a/erpnext/stock/doctype/item/item.py b/erpnext/stock/doctype/item/item.py
index 355503e..de5d545 100644
--- a/erpnext/stock/doctype/item/item.py
+++ b/erpnext/stock/doctype/item/item.py
@@ -13,7 +13,8 @@
from erpnext.setup.doctype.item_group.item_group import invalidate_cache_for, get_parent_item_groups
from frappe.website.render import clear_cache
from frappe.website.doctype.website_slideshow.website_slideshow import get_slideshow
-from erpnext.controllers.item_variant import get_variant, copy_attributes_to_variant, ItemVariantExistsError
+from erpnext.controllers.item_variant import (get_variant, copy_attributes_to_variant,
+ make_variant_item_code, validate_item_variant_attributes, ItemVariantExistsError)
class DuplicateReorderRows(frappe.ValidationError): pass
@@ -36,12 +37,7 @@
if frappe.db.get_default("item_naming_by")=="Naming Series":
if self.variant_of:
if not self.item_code:
- item_code_suffix = ""
- for attribute in self.attributes:
- attribute_abbr = frappe.db.get_value("Item Attribute Value",
- {"parent": attribute.attribute, "attribute_value": attribute.attribute_value}, "abbr")
- item_code_suffix += "-" + str(attribute_abbr or attribute.attribute_value)
- self.item_code = str(self.variant_of) + item_code_suffix
+ self.item_code = make_variant_item_code(self.variant_of, self)
else:
from frappe.model.naming import make_autoname
self.item_code = make_autoname(self.naming_series+'.#####')
@@ -123,8 +119,8 @@
def set_opening_stock(self):
'''set opening stock'''
if not self.valuation_rate:
- frappe.throw(_("Valuation Rate is mandatory if Opening Stock entered"))
-
+ frappe.throw(_("Valuation Rate is mandatory if Opening Stock entered"))
+
from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry
# default warehouse, or Stores
@@ -224,12 +220,12 @@
file_doc.make_thumbnail()
self.thumbnail = file_doc.thumbnail_url
-
+
def validate_fixed_asset(self):
if self.is_fixed_asset:
if self.is_stock_item:
frappe.throw(_("Fixed Asset Item must be a non-stock item."))
-
+
if not self.asset_category:
frappe.throw(_("Asset Category is mandatory for Fixed Asset item"))
@@ -453,7 +449,7 @@
def cant_change(self):
if not self.get("__islocal"):
- vals = frappe.db.get_value("Item", self.name, ["has_serial_no", "is_stock_item",
+ vals = frappe.db.get_value("Item", self.name, ["has_serial_no", "is_stock_item",
"valuation_method", "has_batch_no", "is_fixed_asset"], as_dict=True)
if vals and ((self.is_stock_item != vals.is_stock_item) or
@@ -463,7 +459,7 @@
if self.check_if_linked_document_exists():
frappe.throw(_("As there are existing transactions for this item, \
you can not change the values of 'Has Serial No', 'Has Batch No', 'Is Stock Item' and 'Valuation Method'"))
-
+
if vals and not self.is_fixed_asset and self.is_fixed_asset != vals.is_fixed_asset:
asset = frappe.db.get_all("Asset", filters={"item_code": self.name, "docstatus": 1}, limit=1)
if asset:
@@ -650,6 +646,8 @@
frappe.throw(_("Item variant {0} exists with same attributes")
.format(variant), ItemVariantExistsError)
+ validate_item_variant_attributes(self, args)
+
def get_timeline_data(doctype, name):
'''returns timeline data based on stock ledger entry'''
return dict(frappe.db.sql('''select unix_timestamp(posting_date), count(*)
diff --git a/erpnext/stock/doctype/item/test_item.py b/erpnext/stock/doctype/item/test_item.py
index 47f5162..bd617c6 100644
--- a/erpnext/stock/doctype/item/test_item.py
+++ b/erpnext/stock/doctype/item/test_item.py
@@ -36,6 +36,9 @@
return item
class TestItem(unittest.TestCase):
+ def setUp(self):
+ frappe.flags.attribute_values = None
+
def get_item(self, idx):
item_code = test_records[idx].get("item_code")
if not frappe.db.exists("Item", item_code):
@@ -96,6 +99,9 @@
attribute = frappe.get_doc('Item Attribute', 'Test Size')
attribute.item_attribute_values = []
+ # reset flags
+ frappe.flags.attribute_values = None
+
self.assertRaises(InvalidItemAttributeValueError, attribute.save)
frappe.db.rollback()
@@ -112,10 +118,18 @@
def test_make_item_variant_with_numeric_values(self):
# cleanup
+ for d in frappe.db.get_all('Item', filters={'variant_of':
+ '_Test Numeric Template Item'}):
+ frappe.delete_doc_if_exists("Item", d.name)
+
frappe.delete_doc_if_exists("Item", "_Test Numeric Template Item")
- frappe.delete_doc_if_exists("Item", "_Test Numeric Variant-L-1.5")
frappe.delete_doc_if_exists("Item Attribute", "Test Item Length")
+ frappe.db.sql('''delete from `tabItem Variant Attribute`
+ where attribute="Test Item Length"''')
+
+ frappe.flags.attribute_values = None
+
# make item attribute
frappe.get_doc({
"doctype": "Item Attribute",
@@ -143,7 +157,8 @@
"default_warehouse": "_Test Warehouse - _TC"
})
- variant = create_variant("_Test Numeric Template Item", {"Test Size": "Large", "Test Item Length": 1.1})
+ variant = create_variant("_Test Numeric Template Item",
+ {"Test Size": "Large", "Test Item Length": 1.1})
self.assertEquals(variant.item_code, None)
variant.item_code = "_Test Numeric Variant-L-1.1"
variant.item_name = "_Test Numeric Variant Large 1.1m"
diff --git a/erpnext/stock/doctype/item_attribute/item_attribute.py b/erpnext/stock/doctype/item_attribute/item_attribute.py
index 7529ef6..3220bc5 100644
--- a/erpnext/stock/doctype/item_attribute/item_attribute.py
+++ b/erpnext/stock/doctype/item_attribute/item_attribute.py
@@ -16,6 +16,7 @@
self.flags.ignore_these_exceptions_in_test = [InvalidItemAttributeValueError]
def validate(self):
+ frappe.flags.attribute_values = None
self.validate_numeric()
self.validate_duplication()
@@ -26,7 +27,7 @@
'''Validate that if there are existing items with attributes, they are valid'''
for item in frappe.db.sql('''select distinct 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):
- validate_item_variant_attributes(frappe.get_doc('Item', item[0]))
+ validate_item_variant_attributes(item[0])
def validate_numeric(self):
if self.numeric_values: