[new feature] Product Configurator (via Item Quick Entry) (#11535)

* [Feature] Item Variant Creation from Quick Entry

* [minor] formatted js

* [minor] set 3 attribute per page instead of 5 in template

* [fix] fixed codecy issue

* [fix] label translation

* [minor] changed trigger event of item template

* [fix] moved item ui tests under stock

* [UI test] added test for item attribute

* [UI test] added test for creation of item variant from quick entry

* [fix] item variant ui test fixes

* [wip]

* [cleanup] item quick entry

* [remove] tests, fixtures were missing

* [refactor] use set_only_once in item

erpnext/stock/dashboard/item_dashboard.js

@@ -80,7 +80,7 @@ erpnext.stock.ItemDashboard = Class.extend({
 			$(frappe.render_template('item_dashboard_list', context)).appendTo(this.result);
 		} else {
 			var message = __(" Currently no stock available in any warehouse")
-			$("<span class='small'> <i class='fa fa-exclamation-triangle' aria-hidden='true'></i>"+message+"</span>").appendTo(this.result);
+			$("<span class='text-muted small'>"+message+"</span>").appendTo(this.result);
 		}
 	},
 	get_item_dashboard_data: function(data, max_count, show_item) {

erpnext/stock/doctype/item/item.js

@@ -74,19 +74,8 @@ frappe.ui.form.on("Item", {
 		}
 
 		erpnext.item.edit_prices_button(frm);
-
-		// make sensitive fields(has_serial_no, is_stock_item, valuation_method, has_batch_no)
-		// read only if any stock ledger entry exists
-		if (!frm.doc.__islocal && frm.doc.is_stock_item) {
-			frm.toggle_enable(['has_serial_no', 'is_stock_item', 'valuation_method', 'has_batch_no'],
-				(frm.doc.__onload && frm.doc.__onload.sle_exists=="exists") ? false : true);
-		}
-
 		erpnext.item.toggle_attributes(frm);
 
-		frm.toggle_enable("is_fixed_asset", (frm.doc.__islocal || (!frm.doc.is_stock_item &&
-			((frm.doc.__onload && frm.doc.__onload.asset_exists) ? false : true))));
-
 		frm.add_custom_button(__('Duplicate'), function() {
 			var new_item = frappe.model.copy_doc(frm.doc);
 			if(new_item.item_name===new_item.item_code) {
@@ -103,25 +92,18 @@ frappe.ui.form.on("Item", {
 				frappe.set_route("Form", "Item Variant Settings");
 			}, __("View"));
 		}
-
-		if(frm.doc.__onload && frm.doc.__onload.stock_exists) {
-			// Hide variants section if stock exists
-			frm.toggle_display("variants_section", 0);
-		}
 	},
 
 	validate: function(frm){
 		erpnext.item.weight_to_validate(frm);
 	},
 
-	image: function(frm) {
+	image: function() {
 		refresh_field("image_view");
 	},
 
 	is_fixed_asset: function(frm) {
-		if (frm.doc.is_fixed_asset) {
-			frm.set_value("is_stock_item", 0);
-		}
+		frm.set_value("is_stock_item", frm.doc.is_fixed_asset ? 0 : 1);
 	},
 
 	page_name: frappe.utils.warn_page_name_change,
@@ -469,5 +451,6 @@ $.extend(erpnext.item, {
 			// nothing to do with attributes, hide it
 			frm.toggle_display("attributes", false);
 		}
+		frm.layout.refresh_sections();
 	}
 });

erpnext/stock/doctype/item/item.json

@@ -138,7 +138,7 @@
    "report_hide": 0, 
    "reqd": 0, 
    "search_index": 0, 
-   "set_only_once": 0, 
+   "set_only_once": 1, 
    "unique": 0
   }, 
   {
@@ -358,7 +358,7 @@
   {
    "allow_bulk_edit": 0, 
    "allow_on_submit": 0, 
-   "bold": 0, 
+   "bold": 1, 
    "collapsible": 0, 
    "columns": 0, 
    "default": "1", 
@@ -384,9 +384,9 @@
    "read_only": 0, 
    "remember_last_selected_value": 0, 
    "report_hide": 0, 
-   "reqd": 1, 
+   "reqd": 0, 
    "search_index": 0, 
-   "set_only_once": 0, 
+   "set_only_once": 1, 
    "unique": 0
   }, 
   {
@@ -508,7 +508,7 @@
    "report_hide": 0, 
    "reqd": 0, 
    "search_index": 0, 
-   "set_only_once": 0, 
+   "set_only_once": 1, 
    "unique": 0
   }, 
   {
@@ -891,7 +891,7 @@
    "report_hide": 0, 
    "reqd": 0, 
    "search_index": 0, 
-   "set_only_once": 0, 
+   "set_only_once": 1, 
    "unique": 0
   }, 
   {
@@ -1180,7 +1180,7 @@
    "report_hide": 0, 
    "reqd": 0, 
    "search_index": 0, 
-   "set_only_once": 0, 
+   "set_only_once": 1, 
    "unique": 0
   }, 
   {
@@ -1307,7 +1307,7 @@
    "report_hide": 0, 
    "reqd": 0, 
    "search_index": 0, 
-   "set_only_once": 0, 
+   "set_only_once": 1, 
    "unique": 0
   }, 
   {
@@ -1437,7 +1437,7 @@
    "report_hide": 0, 
    "reqd": 0, 
    "search_index": 0, 
-   "set_only_once": 0, 
+   "set_only_once": 1, 
    "unique": 0
   }, 
   {
@@ -1446,7 +1446,7 @@
    "bold": 0, 
    "collapsible": 0, 
    "columns": 0, 
-   "depends_on": "eval:doc.has_variants && doc.variant_based_on==='Item Attribute'", 
+   "depends_on": "eval:(doc.has_variants || doc.variant_of) && doc.variant_based_on==='Item Attribute'", 
    "fieldname": "attributes", 
    "fieldtype": "Table", 
    "hidden": 1, 
@@ -1469,7 +1469,7 @@
    "report_hide": 0, 
    "reqd": 0, 
    "search_index": 0, 
-   "set_only_once": 0, 
+   "set_only_once": 1, 
    "unique": 0
   }, 
   {
@@ -3360,7 +3360,7 @@
  "issingle": 0, 
  "istable": 0, 
  "max_attachments": 1, 
- "modified": "2017-11-01 11:53:52.060505", 
+ "modified": "2017-11-13 15:49:13.213990", 
  "modified_by": "Administrator", 
  "module": "Stock", 
  "name": "Item", 

erpnext/stock/doctype/item/item.py

@@ -30,14 +30,11 @@ class Item(WebsiteGenerator):
 	def onload(self):
 		super(Item, self).onload()
 
-		self.set_onload('sle_exists', self.check_if_sle_exists())
+		self.set_onload('stock_exists', self.stock_ledger_created())
 		if self.is_fixed_asset:
 			asset = frappe.db.get_all("Asset", filters={"item_code": self.name, "docstatus": 1}, limit=1)
 			self.set_onload("asset_exists", True if asset else False)
 
-		if frappe.db.get_value('Stock Ledger Entry', {'item_code': self.name}):
-			self.set_onload('stock_exists', True)
-
 	def autoname(self):
 		if frappe.db.get_default("item_naming_by")=="Naming Series":
 			if self.variant_of:
@@ -60,6 +57,9 @@ class Item(WebsiteGenerator):
 		if self.is_sales_item and not self.get('is_item_from_hub'):
 			self.publish_in_hub = 1
 
+	def before_save(self):
+		self.get_doc_before_save()
+
 	def after_insert(self):
 		'''set opening stock and item price'''
 		if self.standard_rate:
@@ -89,7 +89,6 @@ class Item(WebsiteGenerator):
 		self.fill_customer_code()
 		self.check_item_tax()
 		self.validate_barcode()
-		self.cant_change()
 		self.validate_warehouse_for_reorder()
 		self.update_bom_item_desc()
 		self.synced_with_hub = 0
@@ -247,6 +246,14 @@ class Item(WebsiteGenerator):
 			if not self.asset_category:
 				frappe.throw(_("Asset Category is mandatory for Fixed Asset item"))
 
+			if self.stock_ledger_created():
+				frappe.throw(_("Cannot be a fixed asset item as Stock Ledger is created."))
+
+		if not self.is_fixed_asset:
+			asset = frappe.db.get_all("Asset", filters={"item_code": self.name, "docstatus": 1}, limit=1)
+			if asset:
+				frappe.throw(_('"Is Fixed Asset" cannot be unchecked, as Asset record exists against the item'))
+
 	def get_context(self, context):
 		context.show_search=True
 		context.search_link = '/product_search'
@@ -456,62 +463,27 @@ class Item(WebsiteGenerator):
 			if duplicate:
 				frappe.throw(_("Barcode {0} already used in Item {1}").format(self.barcode, duplicate[0][0]))
 
-	def cant_change(self):
-		if not self.get("__islocal"):
-			to_check = ("has_serial_no", "is_stock_item",
-				"valuation_method", "has_batch_no", "is_fixed_asset")
-
-			vals = frappe.db.get_value("Item", self.name, to_check, as_dict=True)
-			if not vals.get('valuation_method') and self.get('valuation_method'):
-				vals['valuation_method'] = frappe.db.get_single_value("Stock Settings", "valuation_method") or "FIFO"
-
-			if vals:
-				for key in to_check:
-					if cstr(self.get(key)) != cstr(vals.get(key)):
-						if not self.check_if_linked_document_exists(key):
-							break # no linked document, allowed
-						else:
-							frappe.throw(_("As there are existing transactions against item {0}, you can not change the value of {1}").format(self.name, frappe.bold(self.meta.get_label(key))))
-
-			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:
-					frappe.throw(_('"Is Fixed Asset" cannot be unchecked, as Asset record exists against the item'))
-
-	def check_if_linked_document_exists(self, key):
-		linked_doctypes = ["Delivery Note Item", "Sales Invoice Item", "Purchase Receipt Item",
-			"Purchase Invoice Item", "Stock Entry Detail", "Stock Reconciliation Item"]
-
-		# For "Is Stock Item", following doctypes is important
-		# because reserved_qty, ordered_qty and requested_qty updated from these doctypes
-		if key == "is_stock_item":
-			linked_doctypes += ["Sales Order Item", "Purchase Order Item", "Material Request Item"]
-
-		for doctype in linked_doctypes:
-			if frappe.db.get_value(doctype, filters={"item_code": self.name, "docstatus": 1}) or \
-				frappe.db.get_value("Production Order",
-					filters={"production_item": self.name, "docstatus": 1}):
-				return True
 
+	def validate_warehouse_for_reorder(self):
+		'''Validate Reorder level table for duplicate and conditional mandatory'''
+		warehouse = []
 		for d in self.get("reorder_levels"):
+			if not d.warehouse_group:
+				d.warehouse_group = d.warehouse
+			if d.get("warehouse") and d.get("warehouse") not in warehouse:
+				warehouse += [d.get("warehouse")]
+			else:
+				frappe.throw(_("Row {0}: An Reorder entry already exists for this warehouse {1}")
+					.format(d.idx, d.warehouse), DuplicateReorderRows)
+
 			if d.warehouse_reorder_level and not d.warehouse_reorder_qty:
 				frappe.throw(_("Row #{0}: Please set reorder quantity").format(d.idx))
 
-	def validate_warehouse_for_reorder(self):
-		warehouse = []
-		for i in self.get("reorder_levels"):
-			if not i.warehouse_group:
-				i.warehouse_group = i.warehouse
-			if i.get("warehouse") and i.get("warehouse") not in warehouse:
-				warehouse += [i.get("warehouse")]
-			else:
-				frappe.throw(_("Row {0}: An Reorder entry already exists for this warehouse {1}")
-					.format(i.idx, i.warehouse), DuplicateReorderRows)
-
-	def check_if_sle_exists(self):
-		sle = frappe.db.sql("""select name from `tabStock Ledger Entry`
-			where item_code = %s""", self.name)
-		return sle and 'exists' or 'not exists'
+	def stock_ledger_created(self):
+		if not hasattr(self, '_stock_ledger_created'):
+			self._stock_ledger_created = len(frappe.db.sql("""select name from `tabStock Ledger Entry`
+				where item_code = %s limit 1""", self.name))
+		return self._stock_ledger_created
 
 	def validate_name_with_item_group(self):
 		# causes problem with tree build
@@ -638,19 +610,19 @@ class Item(WebsiteGenerator):
 				template_item.save()
 
 	def update_variants(self):
-			if self.flags.dont_update_variants or \
-				frappe.db.get_single_value('Item Variant Settings', 'do_not_update_variants'):
-				return
-			if self.has_variants:
-				updated = []
-				variants = frappe.db.get_all("Item", fields=["item_code"], filters={"variant_of": self.name })
-				for d in variants:
-					variant = frappe.get_doc("Item", d)
-					copy_attributes_to_variant(self, variant)
-					variant.save()
-					updated.append(d.item_code)
-				if updated:
-					frappe.msgprint(_("Item Variants {0} updated").format(", ".join(updated)))
+		if self.flags.dont_update_variants or \
+			frappe.db.get_single_value('Item Variant Settings', 'do_not_update_variants'):
+			return
+		if self.has_variants:
+			updated = []
+			variants = frappe.db.get_all("Item", fields=["item_code"], filters={"variant_of": self.name })
+			for d in variants:
+				variant = frappe.get_doc("Item", d)
+				copy_attributes_to_variant(self, variant)
+				variant.save()
+				updated.append(d.item_code)
+			if updated:
+				frappe.msgprint(_("Item Variants {0} updated").format(", ".join(updated)))
 
 	def validate_has_variants(self):
 		if not self.has_variants and frappe.db.get_value("Item", self.name, "has_variants"):
@@ -658,10 +630,15 @@ class Item(WebsiteGenerator):
 				frappe.throw(_("Item has variants."))
 
 	def validate_stock_exists_for_template_item(self):
-		if self.has_variants and \
-			frappe.db.get_value('Stock Ledger Entry', {'item_code': self.name}):
-			frappe.throw(_("As stock exists against an item {0}, you can not enable has variants property")
-				.format(self.name), StockExistsForTemplate)
+		if self.stock_ledger_created():
+			if (self._doc_before_save.has_variants != self.has_variants or
+				self.variant_of != self._doc_before_save.variant_of):
+				frappe.throw(_("Cannot change Variant properties after stock transction. You will have to make a new Item to do this.").format(self.name),
+					StockExistsForTemplate)
+
+			if self.has_variants or self.variant_of:
+				if not self.is_child_table_same('attributes'):
+					frappe.throw(_('Cannot change Attributes after stock transaction. Make a new Item and transfer stock to the new Item'))
 
 	def validate_uom(self):
 		if not self.get("__islocal"):

erpnext/stock/doctype/item/tests/test_item.js

@@ -0,0 +1,121 @@
+QUnit.module('stock');
+QUnit.test("test: item", function (assert) {
+	assert.expect(6);
+	let done = assert.async();
+	let keyboard_cost  = 800;
+	let screen_cost  = 4000;
+	let CPU_cost  = 15000;
+	let scrap_cost = 100;
+	let no_of_items_to_stock = 100;
+	let is_stock_item = 1;
+	frappe.run_serially([
+		// test item creation
+		() => frappe.set_route("List", "Item"),
+
+		// Create a keyboard item
+		() => frappe.tests.make(
+			"Item", [
+				{item_code: "Keyboard"},
+				{item_group: "Products"},
+				{is_stock_item: is_stock_item},
+				{standard_rate: keyboard_cost},
+				{opening_stock: no_of_items_to_stock},
+				{default_warehouse: "Stores - FT"}
+			]
+		),
+		() => {
+			assert.ok(cur_frm.doc.item_name.includes('Keyboard'),
+				'Item Keyboard created correctly');
+			assert.ok(cur_frm.doc.item_code.includes('Keyboard'),
+				'item_code for Keyboard set correctly');
+			assert.ok(cur_frm.doc.item_group.includes('Products'),
+				'item_group for Keyboard set correctly');
+			assert.equal(cur_frm.doc.is_stock_item, is_stock_item,
+				'is_stock_item for Keyboard set correctly');
+			assert.equal(cur_frm.doc.standard_rate, keyboard_cost,
+				'standard_rate for Keyboard set correctly');
+			assert.equal(cur_frm.doc.opening_stock, no_of_items_to_stock,
+				'opening_stock for Keyboard set correctly');
+		},
+
+		// Create a Screen item
+		() => frappe.tests.make(
+			"Item", [
+				{item_code: "Screen"},
+				{item_group: "Products"},
+				{is_stock_item: is_stock_item},
+				{standard_rate: screen_cost},
+				{opening_stock: no_of_items_to_stock},
+				{default_warehouse: "Stores - FT"}
+			]
+		),
+
+		// Create a CPU item
+		() => frappe.tests.make(
+			"Item", [
+				{item_code: "CPU"},
+				{item_group: "Products"},
+				{is_stock_item: is_stock_item},
+				{standard_rate: CPU_cost},
+				{opening_stock: no_of_items_to_stock},
+				{default_warehouse: "Stores - FT"}
+			]
+		),
+
+		// Create a laptop item
+		() => frappe.tests.make(
+			"Item", [
+				{item_code: "Laptop"},
+				{item_group: "Products"},
+				{default_warehouse: "Stores - FT"}
+			]
+		),
+		() => frappe.tests.make(
+			"Item", [
+				{item_code: "Computer"},
+				{item_group: "Products"},
+				{is_stock_item: 0},
+			]
+		),
+
+		// Create a scrap item
+		() => frappe.tests.make(
+			"Item", [
+				{item_code: "Scrap item"},
+				{item_group: "Products"},
+				{is_stock_item: is_stock_item},
+				{standard_rate: scrap_cost},
+				{opening_stock: no_of_items_to_stock},
+				{default_warehouse: "Stores - FT"}
+			]
+		),
+		() => frappe.tests.make(
+			"Item", [
+				{item_code: "Test Product 4"},
+				{item_group: "Products"},
+				{is_stock_item: 1},
+				{has_batch_no: 1},
+				{create_new_batch: 1},
+				{uoms:
+					[
+						[
+							{uom:"Unit"},
+							{conversion_factor: 10},
+						]
+					]
+				},
+				{taxes:
+					[
+						[
+							{tax_type:"SGST - "+frappe.get_abbr(frappe.defaults.get_default("Company"))},
+							{tax_rate: 0},
+						]
+					]},
+				{has_serial_no: 1},
+				{standard_rate: 100},
+				{opening_stock: 100},
+			]
+		),
+		() => done()
+	]);
+});

erpnext/stock/doctype/warehouse/warehouse.py

@@ -57,11 +57,11 @@ class Warehouse(NestedSet):
 
 	def check_if_sle_exists(self):
 		return frappe.db.sql("""select name from `tabStock Ledger Entry`
-			where warehouse = %s""", self.name)
+			where warehouse = %s limit 1""", self.name)
 
 	def check_if_child_exists(self):
 		return frappe.db.sql("""select name from `tabWarehouse`
-			where parent_warehouse = %s""", self.name)
+			where parent_warehouse = %s limit 1""", self.name)
 
 	def before_rename(self, old_name, new_name, merge=False):
 		super(Warehouse, self).before_rename(old_name, new_name, merge)