[item-variants] added more validations, tests #2224

erpnext/manufacturing/doctype/bom/bom.py

@@ -207,6 +207,12 @@ class BOM(Document):
 
 	def validate_bom_no(self, item, bom_no, idx):
 		"""Validate BOM No of sub-contracted items"""
+		bom = frappe.get_doc("BOM", bom_no)
+		if not bom.is_active:
+			frappe.throw(_("BOM {0} must be active").format(bom_no))
+		if not bom.docstatus!=1:
+			frappe.throw(_("BOM {0} must be submitted").format(bom_no))
+
 		bom = frappe.db.sql("""select name from `tabBOM` where name = %s and item = %s
 			and is_active=1 and docstatus=1""",
 			(bom_no, item), as_dict =1)

erpnext/stock/doctype/item/item.js

@@ -9,6 +9,14 @@ cur_frm.cscript.refresh = function(doc) {
 
 	cur_frm.cscript.make_dashboard();
 
+	cur_frm.set_intro();
+	if (cur_frm.doc.has_variants) {
+		cur_frm.set_intro(__("This Item is a Template and cannot be used in transactions. Item attributes will be copied over into the variants unless 'No Copy' is set"));
+	}
+	if (cur_frm.doc.variant_of) {
+		cur_frm.set_intro(__("This Item is a Variant of {0} (Template). Attributes will be copied over from the template unless 'No Copy' is set", [cur_frm.doc.variant_of]));
+	}
+
 	if (frappe.defaults.get_default("item_naming_by")!="Naming Series") {
 		cur_frm.toggle_display("naming_series", false);
 	} else {
@@ -154,7 +162,9 @@ cur_frm.cscript.add_image = function(doc, dt, dn) {
 	doc.description_html = repl('<table style="width: 100%; table-layout: fixed;">' +
 		'<tr><td style="width:110px"><img src="%(imgurl)s" width="100px"></td>' +
 		'<td>%(desc)s</td></tr>' +
-		'</table>', {imgurl: frappe.utils.get_file_link(doc.image), desc:doc.description});
+		'</table>', {
+			imgurl: frappe.utils.get_file_link(doc.image),
+			desc: doc.description.replace(/\n/g, "<br>")});
 
 	refresh_field('description_html');
 }

erpnext/stock/doctype/item/item.json

@@ -42,13 +42,15 @@
    "search_index": 0
   }, 
   {
+   "depends_on": "variant_of", 
    "description": "If item is a variant of another item then description, image, pricing, taxes etc will be set from the template unless explicitly specified", 
    "fieldname": "variant_of", 
    "fieldtype": "Link", 
    "label": "Variant Of", 
    "options": "Item", 
    "permlevel": 0, 
-   "precision": ""
+   "precision": "", 
+   "read_only": 1
   }, 
   {
    "fieldname": "item_name", 
@@ -860,7 +862,7 @@
  "icon": "icon-tag", 
  "idx": 1, 
  "max_attachments": 1, 
- "modified": "2014-09-30 14:34:50.101879", 
+ "modified": "2014-10-03 04:58:39.278047", 
  "modified_by": "Administrator", 
  "module": "Stock", 
  "name": "Item", 

erpnext/stock/doctype/item/item.py

@@ -9,9 +9,11 @@ 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
+import copy
 
 class WarehouseNotSet(frappe.ValidationError): pass
 class DuplicateVariant(frappe.ValidationError): pass
+class ItemTemplateCannotHaveStock(frappe.ValidationError): pass
 
 class Item(WebsiteGenerator):
 	page_title_field = "item_name"
@@ -24,7 +26,7 @@ class Item(WebsiteGenerator):
 		self.get("__onload").sle_exists = self.check_if_sle_exists()
 
 	def autoname(self):
-		if frappe.db.get_default("item_naming_by")=="Naming Series":
+		if frappe.db.get_default("item_naming_by")=="Naming Series" and not self.variant_of:
 			from frappe.model.naming import make_autoname
 			self.item_code = make_autoname(self.naming_series+'.#####')
 		elif not self.item_code:
@@ -51,7 +53,7 @@ class Item(WebsiteGenerator):
 		self.validate_barcode()
 		self.cant_change()
 		self.validate_item_type_for_reorder()
-		self.validate_variants_are_unique()
+		self.validate_variants()
 
 		if not self.get("__islocal"):
 			self.old_item_group = frappe.db.get_value(self.doctype, self.name, "item_group")
@@ -117,6 +119,18 @@ class Item(WebsiteGenerator):
 			if not matched:
 				frappe.throw(_("Default Unit of Measure can not be changed directly because you have already made some transaction(s) with another UOM. To change default UOM, use 'UOM Replace Utility' tool under Stock module."))
 
+	def validate_variants(self):
+		self.validate_variants_are_unique()
+		self.validate_stock_for_template_must_be_zero()
+
+	def validate_stock_for_template_must_be_zero(self):
+		if self.has_variants:
+			stock_in = frappe.db.sql_list("""select warehouse from tabBin
+				where item_code=%s and ifnull(actual_qty, 0) > 0""", self.name)
+			if stock_in:
+				frappe.throw(_("Item Template cannot have stock and varaiants. Please remove stock from warehouses {0}").format(", ".join(stock_in)),
+					ItemTemplateCannotHaveStock)
+
 	def validate_variants_are_unique(self):
 		if not self.has_variants:
 			self.item_variants = []
@@ -167,6 +181,7 @@ class Item(WebsiteGenerator):
 		if not self.item_variants:
 			return []
 
+		self.variant_attributes = {}
 		variant_dict = {}
 		variant_item_codes = []
 
@@ -178,31 +193,34 @@ class Item(WebsiteGenerator):
 		# sort attributes by their priority
 		attributes = filter(None, map(lambda d: d if d in variant_dict else None, all_attributes))
 
-		def add_attribute_suffixes(item_code, attributes):
+		def add_attribute_suffixes(item_code, my_attributes, attributes):
 			attr = frappe.get_doc("Item Attribute", attributes[0])
 			for value in attr.item_attribute_values:
 				if value.attribute_value in variant_dict[attr.name]:
+					_my_attributes = copy.deepcopy(my_attributes)
+					_my_attributes.append([attr.name, value.attribute_value])
 					if len(attributes) > 1:
-						add_attribute_suffixes(item_code + "-" + value.abbr, attributes[1:])
+						add_attribute_suffixes(item_code + "-" + value.abbr, _my_attributes, attributes[1:])
 					else:
 						variant_item_codes.append(item_code + "-" + value.abbr)
+						self.variant_attributes[item_code + "-" + value.abbr] = _my_attributes
 
-		add_attribute_suffixes(self.name, attributes)
+		add_attribute_suffixes(self.name, [], attributes)
 
 		return variant_item_codes
 
 	def make_variant(self, item_code):
 		item = frappe.new_doc("Item")
-		self.copy_attributes_to_variant(item, insert=True)
+		self.copy_attributes_to_variant(item, item_code, insert=True)
 		item.item_code = item_code
 		item.insert()
 
 	def update_variant(self, item_code):
 		item = frappe.get_doc("Item", item_code)
-		self.copy_attributes_to_variant(item)
+		self.copy_attributes_to_variant(item, item_code)
 		item.save()
 
-	def copy_attributes_to_variant(self, variant, insert=False):
+	def copy_attributes_to_variant(self, variant, item_code, insert=False):
 		from frappe.model import no_value_fields
 		for field in self.meta.fields:
 			if field.fieldtype not in no_value_fields and (insert or not field.no_copy):
@@ -210,6 +228,11 @@ class Item(WebsiteGenerator):
 					variant.set(field.fieldname, self.get(field.fieldname))
 					variant.__dirty = True
 
+		variant.description += "\n"
+		for attr in self.variant_attributes[item_code]:
+			variant.description += "\n" + attr[0] + ": " + attr[1]
+			if variant.description_html:
+				variant.description_html += "<div style='margin-top: 4px; font-size: 80%'>" + attr[0] + ": " + attr[1] + "</div>"
 		variant.variant_of = self.name
 		variant.has_variants = 0
 		variant.show_in_website = 0

erpnext/stock/doctype/item/test_item.py

@@ -6,21 +6,47 @@ import unittest
 import frappe
 
 from frappe.test_runner import make_test_records
-from erpnext.stock.doctype.item.item import WarehouseNotSet, DuplicateVariant
+from erpnext.stock.doctype.item.item import WarehouseNotSet, DuplicateVariant, ItemTemplateCannotHaveStock
 
 test_ignore = ["BOM"]
 test_dependencies = ["Warehouse"]
 
 class TestItem(unittest.TestCase):
+	def get_item(self, idx):
+		item_code = test_records[idx].get("item_code")
+		if not frappe.db.exists("Item", item_code):
+			item = frappe.copy_doc(test_records[idx])
+			item.insert()
+		else:
+			item = frappe.get_doc("Item", item_code)
+
+		return item
+
 	def test_duplicate_variant(self):
 		item = frappe.copy_doc(test_records[11])
 		item.append("item_variants", {"item_attribute": "Test Size", "item_attribute_value": "Small"})
 		self.assertRaises(DuplicateVariant, item.insert)
 
+	def test_template_cannot_have_stock(self):
+		item = self.get_item(10)
+
+		se = frappe.new_doc("Stock Entry")
+		se.purpose = "Material Receipt"
+		se.append("mtn_details", {
+			"item_code": item.name,
+			"t_warehouse": "Stores - WP",
+			"qty": 1,
+			"incoming_rate": 1
+		})
+		se.insert()
+		se.submit()
+
+		item.has_variants = 1
+		self.assertRaises(ItemTemplateCannotHaveStock, item.save)
+
 	def test_variant_item_codes(self):
-		frappe.delete_doc("Item", test_records[11].get("item_code"))
+		item = self.get_item(11)
-		item = frappe.copy_doc(test_records[11])
+
-		item.insert()
 		variants = ['_Test Variant Item-S', '_Test Variant Item-M', '_Test Variant Item-L']
 		self.assertEqual(item.get_variant_item_codes(), variants)
 		for v in variants:
@@ -36,10 +62,23 @@ class TestItem(unittest.TestCase):
 			'_Test Variant Item-M-B', '_Test Variant Item-L-R',
 			'_Test Variant Item-L-G', '_Test Variant Item-L-B'])
 
-	def test_item_creation(self):
+		self.assertEqual(item.variant_attributes['_Test Variant Item-L-R'], [['Test Size', 'Large'], ['Test Colour', 'Red']])
-		frappe.delete_doc("Item", test_records[11].get("item_code"))
+		self.assertEqual(item.variant_attributes['_Test Variant Item-S-G'], [['Test Size', 'Small'], ['Test Colour', 'Green']])
-		item = frappe.copy_doc(test_records[11])
+
-		item.insert()
+		# check stock entry cannot be made
+	def test_stock_entry_cannot_be_made_for_template(self):
+		item = self.get_item(11)
+
+		se = frappe.new_doc("Stock Entry")
+		se.purpose = "Material Receipt"
+		se.append("mtn_details", {
+			"item_code": item.name,
+			"t_warehouse": "Stores - WP",
+			"qty": 1,
+			"incoming_rate": 1
+		})
+		se.insert()
+		self.assertRaises(ItemTemplateCannotHaveStock, se.submit)
 
 	def test_default_warehouse(self):
 		item = frappe.copy_doc(test_records[0])
@@ -57,7 +96,7 @@ class TestItem(unittest.TestCase):
 			"income_account": "Sales - _TC",
 			"expense_account": "_Test Account Cost for Goods Sold - _TC",
 			"cost_center": "_Test Cost Center 2 - _TC",
-			"qty": 1.0,
+			"qty": 0.0,
 			"price_list_rate": 100.0,
 			"base_price_list_rate": 0.0,
 			"discount_percentage": 0.0,

erpnext/stock/doctype/stock_entry/stock_entry.py

@@ -11,7 +11,7 @@ from frappe import _
 from erpnext.stock.utils import get_incoming_rate
 from erpnext.stock.stock_ledger import get_previous_sle
 from erpnext.controllers.queries import get_match_cond
-from erpnext.stock.get_item_details import get_available_qty
+from erpnext.stock.get_item_details import get_available_qty, get_default_cost_center
 
 class NotUpdateStockError(frappe.ValidationError): pass
 class StockOverReturnError(frappe.ValidationError): pass
@@ -42,8 +42,8 @@ class StockEntry(StockController):
 		pro_obj = self.production_order and \
 			frappe.get_doc('Production Order', self.production_order) or None
 
-		self.set_transfer_qty()
 		self.validate_item()
+		self.set_transfer_qty()
 		self.validate_uom_is_integer("uom", "qty")
 		self.validate_uom_is_integer("stock_uom", "transfer_qty")
 		self.validate_warehouse(pro_obj)
@@ -96,21 +96,23 @@ class StockEntry(StockController):
 		for item in self.get("mtn_details"):
 			if item.item_code not in stock_items:
 				frappe.throw(_("{0} is not a stock Item").format(item.item_code))
-			if not item.stock_uom:
+
-				item.stock_uom = frappe.db.get_value("Item", item.item_code, "stock_uom")
+			item_details = self.get_item_details(frappe._dict({"item_code": item.item_code,
-			if not item.uom:
+				"company": self.company, "project_name": self.project_name}))
-				item.uom = item.stock_uom
+
-			if not item.conversion_factor:
+			for f in ("uom", "stock_uom", "description", "item_name", "expense_account",
-				item.conversion_factor = 1
+				"cost_center", "conversion_factor"):
+				item.set(f, item_details.get(f))
+
 			if not item.transfer_qty:
 				item.transfer_qty = item.qty * item.conversion_factor
+
 			if (self.purpose in ("Material Transfer", "Sales Return", "Purchase Return")
 				and not item.serial_no
 				and item.item_code in serialized_items):
 				frappe.throw(_("Row #{0}: Please specify Serial No for Item {1}").format(item.idx, item.item_code),
 					frappe.MandatoryError)
 
-
 	def validate_warehouse(self, pro_obj):
 		"""perform various (sometimes conditional) validations on warehouse"""
 
@@ -408,20 +410,21 @@ class StockEntry(StockController):
 
 	def get_item_details(self, args):
 		item = frappe.db.sql("""select stock_uom, description, item_name,
-			expense_account, buying_cost_center from `tabItem`
+			expense_account, buying_cost_center, item_group from `tabItem`
 			where name = %s and (ifnull(end_of_life,'0000-00-00')='0000-00-00' or end_of_life > now())""",
 			(args.get('item_code')), as_dict = 1)
 		if not item:
 			frappe.throw(_("Item {0} is not active or end of life has been reached").format(args.get("item_code")))
+		item = item[0]
 
 		ret = {
-			'uom'			      	: item and item[0]['stock_uom'] or '',
+			'uom'			      	: item.stock_uom,
-			'stock_uom'			  	: item and item[0]['stock_uom'] or '',
+			'stock_uom'			  	: item.stock_uom,
-			'description'		  	: item and item[0]['description'] or '',
+			'description'		  	: item.description,
-			'item_name' 		  	: item and item[0]['item_name'] or '',
+			'item_name' 		  	: item.item_name,
 			'expense_account'		: args.get("expense_account") \
 				or frappe.db.get_value("Company", args.get("company"), "stock_adjustment_account"),
-			'cost_center'			: item and item[0]['buying_cost_center'] or args.get("cost_center"),
+			'cost_center'			: get_default_cost_center(args, item),
 			'qty'					: 0,
 			'transfer_qty'			: 0,
 			'conversion_factor'		: 1,

erpnext/stock/doctype/stock_ledger_entry/stock_ledger_entry.py

@@ -8,6 +8,7 @@ from frappe import _
 from frappe.utils import flt, getdate, add_days, formatdate
 from frappe.model.document import Document
 from datetime import date
+from erpnext.stock.doctype.item.item import ItemTemplateCannotHaveStock
 
 class StockFreezeError(frappe.ValidationError): pass
 
@@ -50,7 +51,8 @@ class StockLedgerEntry(Document):
 				frappe.throw(_("{0} is required").format(self.meta.get_label(k)))
 
 	def validate_item(self):
-		item_det = frappe.db.sql("""select name, has_batch_no, docstatus, is_stock_item
+		item_det = frappe.db.sql("""select name, has_batch_no, docstatus,
+			is_stock_item, has_variants
 			from tabItem where name=%s""", self.item_code, as_dict=True)[0]
 
 		if item_det.is_stock_item != 'Yes':
@@ -66,6 +68,10 @@ class StockLedgerEntry(Document):
 					{"item": self.item_code, "name": self.batch_no}):
 				frappe.throw(_("{0} is not a valid Batch Number for Item {1}").format(self.batch_no, self.item_code))
 
+		if item_det.has_variants:
+			frappe.throw(_("Stock cannot exist for Item {0} since has variants").format(self.item_code),
+				ItemTemplateCannotHaveStock)
+
 		if not self.stock_uom:
 			self.stock_uom = item_det.stock_uom
 

erpnext/stock/get_item_details.py

@@ -40,7 +40,7 @@ def get_item_details(args):
 
 	validate_item_details(args, item)
 
-	out = get_basic_details(args, item_doc)
+	out = get_basic_details(args, item)
 
 	get_party_item_code(args, item_doc, out)
 
@@ -129,8 +129,9 @@ def validate_item_details(args, item):
 		if args.get("is_subcontracted") == "Yes" and item.is_sub_contracted_item != "Yes":
 			throw(_("Item {0} must be a Sub-contracted Item").format(item.name))
 
-def get_basic_details(args, item_doc):
+def get_basic_details(args, item):
-	item = item_doc
+	if not item:
+		item = frappe.get_doc("Item", args.get("item_code"))
 
 	from frappe.defaults import get_user_default_as_list
 	user_default_warehouse_list = get_user_default_as_list('warehouse')
@@ -143,26 +144,17 @@ def get_basic_details(args, item_doc):
 		"item_name": item.item_name,
 		"description": item.description_html or item.description,
 		"warehouse": user_default_warehouse or args.warehouse or item.default_warehouse,
-		"income_account": (item.income_account
+		"income_account": get_default_income_account(args, item),
-			or args.income_account
+		"expense_account": get_default_expense_account(args, item),
-			or frappe.db.get_value("Item Group", item.item_group, "default_income_account")
+		"cost_center": get_default_cost_center(args, item),
-			or frappe.db.get_value("Company", args.company, "default_income_account")),
-		"expense_account": (item.expense_account
-			or args.expense_account
-			or frappe.db.get_value("Item Group", item.item_group, "default_expense_account")
-			or frappe.db.get_value("Company", args.company, "default_expense_account")),
-		"cost_center": (frappe.db.get_value("Project", args.project_name, "cost_center")
-			or (item.selling_cost_center if args.transaction_type == "selling" else item.buying_cost_center)
-			or frappe.db.get_value("Item Group", item.item_group, "default_cost_center")
-			or frappe.db.get_value("Company", args.company, "cost_center")),
 		"batch_no": None,
 		"item_tax_rate": json.dumps(dict(([d.tax_type, d.tax_rate] for d in
-			item_doc.get("item_tax")))),
+			item.get("item_tax")))),
 		"uom": item.stock_uom,
 		"min_order_qty": flt(item.min_order_qty) if args.parenttype == "Material Request" else "",
 		"conversion_factor": 1.0,
-		"qty": 1.0,
+		"qty": 0.0,
-		"stock_qty": 1.0,
+		"stock_qty": 0.0,
 		"price_list_rate": 0.0,
 		"base_price_list_rate": 0.0,
 		"rate": 0.0,
@@ -177,6 +169,24 @@ def get_basic_details(args, item_doc):
 
 	return out
 
+def get_default_income_account(args, item):
+	return (item.income_account
+		or args.income_account
+		or frappe.db.get_value("Item Group", item.item_group, "default_income_account")
+		or frappe.db.get_value("Company", args.company, "default_income_account"))
+
+def get_default_expense_account(args, item):
+	return (item.expense_account
+		or args.expense_account
+		or frappe.db.get_value("Item Group", item.item_group, "default_expense_account")
+		or frappe.db.get_value("Company", args.company, "default_expense_account"))
+
+def get_default_cost_center(args, item):
+	return (frappe.db.get_value("Project", args.project_name, "cost_center")
+		or (item.selling_cost_center if args.transaction_type == "selling" else item.buying_cost_center)
+		or frappe.db.get_value("Item Group", item.item_group, "default_cost_center")
+		or frappe.db.get_value("Company", args.company, "cost_center"))
+
 def get_price_list_rate(args, item_doc, out):
 	meta = frappe.get_meta(args.parenttype)
 
@@ -185,10 +195,12 @@ def get_price_list_rate(args, item_doc, out):
 		validate_conversion_rate(args, meta)
 
 
-		price_list_rate = frappe.db.get_value("Item Price",
+		price_list_rate = get_price_list_rate_for(args, item_doc.name)
-			{"price_list": args.price_list, "item_code": args.item_code}, "price_list_rate")
+		if not price_list_rate and item_doc.variant_of:
+			price_list_rate = get_price_list_rate_for(args, item_doc.variant_of)
 
-		if not price_list_rate: return {}
+		if not price_list_rate:
+			return {}
 
 		out.price_list_rate = flt(price_list_rate) * flt(args.plc_conversion_rate) \
 			/ flt(args.conversion_rate)
@@ -198,6 +210,10 @@ def get_price_list_rate(args, item_doc, out):
 			out.update(get_last_purchase_details(item_doc.name,
 				args.parent, args.conversion_rate))
 
+def get_price_list_rate_for(args, item_code):
+	return frappe.db.get_value("Item Price",
+			{"price_list": args.price_list, "item_code": item_code}, "price_list_rate")
+
 def validate_price_list(args):
 	if args.get("price_list"):
 		if not frappe.db.get_value("Price List",
@@ -236,7 +252,6 @@ def get_party_item_code(args, item_doc, out):
 		item_supplier = item_doc.get("item_supplier_details", {"supplier": args.supplier})
 		out.supplier_part_no = item_supplier[0].supplier_part_no if item_supplier else None
 
-
 def get_pos_settings_item_details(company, args, pos_settings=None):
 	res = frappe._dict()