[item-variants] added more validations, tests #2224
diff --git a/erpnext/manufacturing/doctype/bom/bom.py b/erpnext/manufacturing/doctype/bom/bom.py
index 1185403..8328fc7 100644
--- a/erpnext/manufacturing/doctype/bom/bom.py
+++ b/erpnext/manufacturing/doctype/bom/bom.py
@@ -207,6 +207,12 @@
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)
diff --git a/erpnext/stock/doctype/item/item.js b/erpnext/stock/doctype/item/item.js
index 06aae12..e274690 100644
--- a/erpnext/stock/doctype/item/item.js
+++ b/erpnext/stock/doctype/item/item.js
@@ -9,6 +9,14 @@
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 @@
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');
}
diff --git a/erpnext/stock/doctype/item/item.json b/erpnext/stock/doctype/item/item.json
index 0a74e41..c1a1fe9 100644
--- a/erpnext/stock/doctype/item/item.json
+++ b/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",
diff --git a/erpnext/stock/doctype/item/item.py b/erpnext/stock/doctype/item/item.py
index 565d24e..c7d9375 100644
--- a/erpnext/stock/doctype/item/item.py
+++ b/erpnext/stock/doctype/item/item.py
@@ -9,9 +9,11 @@
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 @@
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 @@
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 @@
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 @@
if not self.item_variants:
return []
+ self.variant_attributes = {}
variant_dict = {}
variant_item_codes = []
@@ -178,31 +193,34 @@
# 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 @@
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
diff --git a/erpnext/stock/doctype/item/test_item.py b/erpnext/stock/doctype/item/test_item.py
index c040548..2807ca8 100644
--- a/erpnext/stock/doctype/item/test_item.py
+++ b/erpnext/stock/doctype/item/test_item.py
@@ -6,21 +6,47 @@
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 = frappe.copy_doc(test_records[11])
- item.insert()
+ item = self.get_item(11)
+
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 @@
'_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):
- frappe.delete_doc("Item", test_records[11].get("item_code"))
- item = frappe.copy_doc(test_records[11])
- item.insert()
+ self.assertEqual(item.variant_attributes['_Test Variant Item-L-R'], [['Test Size', 'Large'], ['Test Colour', 'Red']])
+ self.assertEqual(item.variant_attributes['_Test Variant Item-S-G'], [['Test Size', 'Small'], ['Test Colour', 'Green']])
+
+ # 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 @@
"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,
diff --git a/erpnext/stock/doctype/stock_entry/stock_entry.py b/erpnext/stock/doctype/stock_entry/stock_entry.py
index a2a156f..a2d8c26 100644
--- a/erpnext/stock/doctype/stock_entry/stock_entry.py
+++ b/erpnext/stock/doctype/stock_entry/stock_entry.py
@@ -11,7 +11,7 @@
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 @@
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 @@
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")
- if not item.uom:
- item.uom = item.stock_uom
- if not item.conversion_factor:
- item.conversion_factor = 1
+
+ item_details = self.get_item_details(frappe._dict({"item_code": item.item_code,
+ "company": self.company, "project_name": self.project_name}))
+
+ for f in ("uom", "stock_uom", "description", "item_name", "expense_account",
+ "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 @@
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 '',
- 'stock_uom' : item and item[0]['stock_uom'] or '',
- 'description' : item and item[0]['description'] or '',
- 'item_name' : item and item[0]['item_name'] or '',
+ 'uom' : item.stock_uom,
+ 'stock_uom' : item.stock_uom,
+ 'description' : item.description,
+ '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,
diff --git a/erpnext/stock/doctype/stock_ledger_entry/stock_ledger_entry.py b/erpnext/stock/doctype/stock_ledger_entry/stock_ledger_entry.py
index 0e92b5d..1f7f9e5 100644
--- a/erpnext/stock/doctype/stock_ledger_entry/stock_ledger_entry.py
+++ b/erpnext/stock/doctype/stock_ledger_entry/stock_ledger_entry.py
@@ -8,6 +8,7 @@
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 @@
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 @@
{"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
diff --git a/erpnext/stock/get_item_details.py b/erpnext/stock/get_item_details.py
index 19f9724..fd728e6 100644
--- a/erpnext/stock/get_item_details.py
+++ b/erpnext/stock/get_item_details.py
@@ -40,7 +40,7 @@
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 @@
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):
- item = item_doc
+def get_basic_details(args, item):
+ 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 @@
"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
- 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")),
- "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")),
+ "income_account": get_default_income_account(args, item),
+ "expense_account": get_default_expense_account(args, item),
+ "cost_center": get_default_cost_center(args, item),
"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,
- "stock_qty": 1.0,
+ "qty": 0.0,
+ "stock_qty": 0.0,
"price_list_rate": 0.0,
"base_price_list_rate": 0.0,
"rate": 0.0,
@@ -177,6 +169,24 @@
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 @@
validate_conversion_rate(args, meta)
- price_list_rate = frappe.db.get_value("Item Price",
- {"price_list": args.price_list, "item_code": args.item_code}, "price_list_rate")
+ price_list_rate = get_price_list_rate_for(args, item_doc.name)
+ 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 @@
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 @@
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()