From ffca81dbc15956f9d47e0b784313e7be4a73a960 Mon Sep 17 00:00:00 2001 From: Anand Doshi Date: Wed, 19 Aug 2015 11:51:45 +0530 Subject: [PATCH] [fix] added test cases and fixes to Item Variants --- erpnext/stock/doctype/item/item.py | 28 +++++----- erpnext/stock/doctype/item/test_item.py | 52 +++++++++++++++---- .../doctype/item_attribute/item_attribute.py | 10 ++-- .../item_attribute/test_item_attribute.py | 22 +++++++- 4 files changed, 85 insertions(+), 27 deletions(-) diff --git a/erpnext/stock/doctype/item/item.py b/erpnext/stock/doctype/item/item.py index 20a064bc75..cd3d295979 100644 --- a/erpnext/stock/doctype/item/item.py +++ b/erpnext/stock/doctype/item/item.py @@ -14,6 +14,7 @@ from frappe.website.doctype.website_slideshow.website_slideshow import get_slide class WarehouseNotSet(frappe.ValidationError): pass class ItemTemplateCannotHaveStock(frappe.ValidationError): pass class ItemVariantExistsError(frappe.ValidationError): pass +class InvalidItemAttributeValueError(frappe.ValidationError): pass class Item(WebsiteGenerator): website = frappe._dict( @@ -351,9 +352,11 @@ class Item(WebsiteGenerator): frappe.throw(_("Please specify Attribute Value for attribute {0}").format(d.attribute)) args[d.attribute] = d.attribute_value - variant = get_variant(self.variant_of, args) - if variant and variant != self.name: - frappe.throw(_("Item variant {0} exists with same attributes").format(variant), ItemVariantExistsError) + if self.get("__islocal"): + # test this during insert because naming is based on item_code and we cannot use condition like self.name != variant + variant = get_variant(self.variant_of, args) + if variant: + frappe.throw(_("Item variant {0} exists with same attributes").format(variant), ItemVariantExistsError) def validate_end_of_life(item_code, end_of_life=None, verbose=1): if not end_of_life: @@ -510,8 +513,8 @@ def validate_item_variant_attributes(item, args): filters={"parent": ["in", args.keys()]}): (attribute_values.setdefault(t.parent, [])).append(t.attribute_value) - numeric_attributes = [t.name for t in frappe.get_list("Item Attribute", filters={"numeric_values":1, - "parent": ["in", args.keys()]})] + numeric_attributes = frappe._dict((t.name, t) for t in frappe.get_list("Item Attribute", filters={"numeric_values":1, + "name": ["in", args.keys()]}, fields=["name", "from_range", "to_range", "increment"])) template_item = frappe.get_doc("Item", item) template_item_attributes = frappe._dict((d.attribute, d) for d in template_item.attributes) @@ -519,18 +522,19 @@ def validate_item_variant_attributes(item, args): for attribute, value in args.items(): if attribute in numeric_attributes: - template_attribute = template_item_attributes[attribute] + numeric_attribute = numeric_attributes[attribute] - if template_attribute.increment == 0: + from_range = numeric_attribute.from_range + to_range = numeric_attribute.to_range + increment = numeric_attribute.increment + + if increment == 0: # defensive validation to prevent ZeroDivisionError frappe.throw(_("Increment for Attribute {0} cannot be 0").format(attribute)) - from_range = template_attribute.from_range - to_range = template_attribute.to_range - increment = template_attribute.increment if not ( (from_range <= flt(value) <= to_range) and (flt(value) - from_range) % increment == 0 ): - 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)) + 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[attribute]: frappe.throw(_("Value {0} for Attribute {1} does not exist in the list of valid Item Attribute Values").format( @@ -538,7 +542,7 @@ def validate_item_variant_attributes(item, args): def find_variant(item, args): conditions = ["""(iv_attribute.attribute="{0}" and iv_attribute.attribute_value="{1}")"""\ - .format(frappe.db.escape(key), frappe.db.escape(value)) for key, value in args.items()] + .format(frappe.db.escape(key), frappe.db.escape(cstr(value))) for key, value in args.items()] conditions = " or ".join(conditions) diff --git a/erpnext/stock/doctype/item/test_item.py b/erpnext/stock/doctype/item/test_item.py index 428bd37d00..2ba96475bd 100644 --- a/erpnext/stock/doctype/item/test_item.py +++ b/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.stock.doctype.item.item import WarehouseNotSet, ItemTemplateCannotHaveStock, create_variant, ItemVariantExistsError +from erpnext.stock.doctype.item.item import (WarehouseNotSet, ItemTemplateCannotHaveStock, create_variant, + ItemVariantExistsError, InvalidItemAttributeValueError) from erpnext.stock.doctype.stock_entry.test_stock_entry import make_stock_entry test_ignore = ["BOM"] @@ -100,21 +101,52 @@ class TestItem(unittest.TestCase): self.assertEquals(value, details.get(key)) def test_make_item_variant(self): - if frappe.db.exists("Item", "_Test Variant Item-L"): - frappe.delete_doc("Item", "_Test Variant Item-L") - frappe.delete_doc("Item", "_Test Variant Item-L2") + frappe.delete_doc_if_exists("Item", "_Test Variant Item-L") - variant = create_variant("_Test Variant Item", """{"Test Size": "Large"}""") - variant.item_code = "_Test Variant Item-L" - variant.item_name = "_Test Variant Item-L" + variant = create_variant("_Test Variant Item", {"Test Size": "Large"}) variant.save() # doing it again should raise error - variant = create_variant("_Test Variant Item", """{"Test Size": "Large"}""") - variant.item_code = "_Test Variant Item-L2" - variant.item_name = "_Test Variant Item-L2" + variant = create_variant("_Test Variant Item", {"Test Size": "Large"}) self.assertRaises(ItemVariantExistsError, variant.save) + def test_make_item_variant_with_numeric_values(self): + # cleanup + 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") + + # make item attribute + frappe.get_doc({ + "doctype": "Item Attribute", + "attribute_name": "Test Item Length", + "numeric_values": 1, + "from_range": 0.0, + "to_range": 100.0, + "increment": 0.5 + }).insert() + + # make template item + make_item("_Test Numeric Template Item", { + "attributes": [ + {"attribute": "Test Size"}, + {"attribute": "Test Item Length"} + ], + "default_warehouse": "_Test Warehouse - _TC" + }) + + 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" + self.assertRaises(InvalidItemAttributeValueError, variant.save) + + variant = create_variant("_Test Numeric Template Item", {"Test Size": "Large", "Test Item Length": 1.5}) + self.assertEquals(variant.item_code, None) + variant.item_code = "_Test Numeric Variant-L-1.5" + variant.item_name = "_Test Numeric Variant Large 1.5m" + variant.save() + def make_item_variant(): if not frappe.db.exists("Item", "_Test Variant Item-S"): variant = create_variant("_Test Variant Item", """{"Test Size": "Small"}""") diff --git a/erpnext/stock/doctype/item_attribute/item_attribute.py b/erpnext/stock/doctype/item_attribute/item_attribute.py index a82a3a8c22..8310288e47 100644 --- a/erpnext/stock/doctype/item_attribute/item_attribute.py +++ b/erpnext/stock/doctype/item_attribute/item_attribute.py @@ -6,6 +6,8 @@ import frappe from frappe.model.document import Document from frappe import _ +class ItemAttributeIncrementError(frappe.ValidationError): pass + class ItemAttribute(Document): def validate(self): self.validate_numeric() @@ -15,14 +17,14 @@ class ItemAttribute(Document): def validate_numeric(self): if self.numeric_values: self.set("item_attribute_values", []) - if not self.from_range or not self.to_range: + if self.from_range is None or self.to_range is None: frappe.throw(_("Please specify from/to range")) - elif self.from_range > self.to_range: - frappe.throw(_("From Range cannot be greater than to Range")) + elif self.from_range >= self.to_range: + frappe.throw(_("From Range has to be less than To Range")) if not self.increment: - frappe.throw(_("Increment cannot be 0")) + frappe.throw(_("Increment cannot be 0"), ItemAttributeIncrementError) else: self.from_range = self.to_range = self.increment = 0 diff --git a/erpnext/stock/doctype/item_attribute/test_item_attribute.py b/erpnext/stock/doctype/item_attribute/test_item_attribute.py index 31b3b0a977..8b73d46721 100644 --- a/erpnext/stock/doctype/item_attribute/test_item_attribute.py +++ b/erpnext/stock/doctype/item_attribute/test_item_attribute.py @@ -6,5 +6,25 @@ import unittest test_records = frappe.get_test_records('Item Attribute') +from .item_attribute import ItemAttributeIncrementError + class TestItemAttribute(unittest.TestCase): - pass + def setUp(self): + if frappe.db.exists("Item Attribute", "_Test_Length"): + frappe.delete_doc("Item Attribute", "_Test_Length") + + def test_numeric_item_attribute(self): + item_attribute = frappe.get_doc({ + "doctype": "Item Attribute", + "attribute_name": "_Test_Length", + "numeric_values": 1, + "from_range": 0.0, + "to_range": 100.0, + "increment": 0 + }) + + self.assertRaises(ItemAttributeIncrementError, item_attribute.save) + + item_attribute.increment = 0.5 + item_attribute.save() +