diff --git a/erpnext/controllers/item_variant.py b/erpnext/controllers/item_variant.py index 3ff3d88050b..8294b64da92 100644 --- a/erpnext/controllers/item_variant.py +++ b/erpnext/controllers/item_variant.py @@ -24,22 +24,23 @@ def get_variant(template, args, variant=None): 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 @@ def validate_item_variant_attributes(item, args=None): 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 @@ def create_variant(item, args): 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 @@ def copy_attributes_to_variant(item, variant): for d in variant.attributes: variant.description += "

" + d.attribute + ": " + cstr(d.attribute_value) + "

" -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 @@ def make_variant_item_code(template, variant): }, 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 @@ def make_variant_item_code(template, variant): 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 355503e5e56..de5d54598ce 100644 --- a/erpnext/stock/doctype/item/item.py +++ b/erpnext/stock/doctype/item/item.py @@ -13,7 +13,8 @@ from frappe.website.website_generator import WebsiteGenerator 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 @@ class Item(WebsiteGenerator): 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 @@ class Item(WebsiteGenerator): 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 @@ class Item(WebsiteGenerator): 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 @@ class Item(WebsiteGenerator): 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 @@ class Item(WebsiteGenerator): 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 @@ class Item(WebsiteGenerator): 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 47f5162332d..bd617c6b853 100644 --- a/erpnext/stock/doctype/item/test_item.py +++ b/erpnext/stock/doctype/item/test_item.py @@ -36,6 +36,9 @@ def make_item(item_code, properties=None): 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 @@ class TestItem(unittest.TestCase): 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 @@ class TestItem(unittest.TestCase): 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 @@ class TestItem(unittest.TestCase): "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 7529ef6fc03..3220bc5ba24 100644 --- a/erpnext/stock/doctype/item_attribute/item_attribute.py +++ b/erpnext/stock/doctype/item_attribute/item_attribute.py @@ -16,6 +16,7 @@ class ItemAttribute(Document): 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 @@ class ItemAttribute(Document): '''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: