Added validation and fixes UX for asset (#14727)

erpnext/assets/doctype/asset/asset.py

@@ -337,7 +337,7 @@ class Asset(AccountsController):
 
 		if self.get('default_finance_book'):
 			for d in self.get('finance_books'):
-				if d.finance_books == self.default_finance_book:
+				if d.finance_book == self.default_finance_book:
 					return cint(d.idx) - 1
 
 	def update_stock_movement(self):

erpnext/assets/doctype/asset_movement/asset_movement.js

@@ -2,7 +2,27 @@
 // For license information, please see license.txt
 
 frappe.ui.form.on('Asset Movement', {
-	onload: function(frm) {
-		//
+	select_serial_no: function(frm) {
+		if (frm.doc.select_serial_no) {
+			let serial_no = frm.doc.serial_no
+				? frm.doc.serial_no + '\n' + frm.doc.select_serial_no : frm.doc.select_serial_no;
+			frm.set_value("serial_no", serial_no);
+			frm.set_value("quantity", serial_no.split('\n').length);
+		}
+	},
+
+	serial_no: function(frm) {
+		const qty = frm.doc.serial_no ? frm.doc.serial_no.split('\n').length : 0;
+		frm.set_value("quantity", qty);
+	},
+
+	setup: function(frm) {
+		frm.set_query("select_serial_no", function() {
+			return {
+				filters: {
+					"asset": frm.doc.asset
+				}
+			};
+		});
 	}
 });

erpnext/assets/doctype/asset_movement/asset_movement.json

@@ -207,6 +207,39 @@
    "translatable": 0, 
    "unique": 0
   }, 
+  {
+   "allow_bulk_edit": 0, 
+   "allow_in_quick_entry": 0, 
+   "allow_on_submit": 0, 
+   "bold": 0, 
+   "collapsible": 0, 
+   "columns": 0, 
+   "fieldname": "select_serial_no", 
+   "fieldtype": "Link", 
+   "hidden": 0, 
+   "ignore_user_permissions": 0, 
+   "ignore_xss_filter": 0, 
+   "in_filter": 0, 
+   "in_global_search": 0, 
+   "in_list_view": 0, 
+   "in_standard_filter": 0, 
+   "label": "Select Serial No", 
+   "length": 0, 
+   "no_copy": 0, 
+   "options": "Serial No", 
+   "permlevel": 0, 
+   "precision": "", 
+   "print_hide": 0, 
+   "print_hide_if_no_value": 0, 
+   "read_only": 0, 
+   "remember_last_selected_value": 0, 
+   "report_hide": 0, 
+   "reqd": 0, 
+   "search_index": 0, 
+   "set_only_once": 0, 
+   "translatable": 0, 
+   "unique": 0
+  }, 
   {
    "allow_bulk_edit": 0, 
    "allow_in_quick_entry": 0, 
@@ -575,7 +608,7 @@
  "issingle": 0, 
  "istable": 0, 
  "max_attachments": 0, 
- "modified": "2018-06-26 15:02:35.822022", 
+ "modified": "2018-06-28 12:07:16.531921", 
  "modified_by": "Administrator", 
  "module": "Assets", 
  "name": "Asset Movement", 
@@ -640,12 +673,13 @@
    "write": 1
   }
  ], 
- "quick_entry": 1, 
+ "quick_entry": 0, 
  "read_only": 0, 
  "read_only_onload": 0, 
  "show_name_in_global_search": 0, 
  "sort_field": "modified", 
  "sort_order": "DESC", 
  "track_changes": 0, 
- "track_seen": 0
+ "track_seen": 0, 
+ "track_views": 0
 }

erpnext/assets/doctype/asset_movement/asset_movement.py

@@ -36,7 +36,7 @@ class AssetMovement(Document):
 			if not self.serial_no and not (self.from_employee or self.to_employee):
 				self.source_location = frappe.db.get_value("Asset", self.asset, "location")
 
-			if self.purpose == 'Issue' and not self.source_location:
+			if self.purpose == 'Issue' and not (self.source_location or self.from_employee):
 				frappe.throw(_("Source Location is required for the asset {0}").format(self.asset))
 
 			if self.serial_no and self.source_location:
@@ -51,7 +51,7 @@ class AssetMovement(Document):
 		if self.source_location and self.source_location == self.target_location and self.purpose == 'Transfer':
 			frappe.throw(_("Source and Target Location cannot be same"))
 
-		if self.purpose == 'Receipt' and not self.target_location:
+		if self.purpose == 'Receipt' and not (self.target_location or self.to_employee):
 			frappe.throw(_("Target Location is required for the asset {0}").format(self.asset))
 
 	def on_submit(self):
@@ -93,10 +93,13 @@ class AssetMovement(Document):
 		if not self.serial_no:
 			frappe.db.set_value("Asset", self.asset, "location", location)
 
+		if not employee and self.purpose in ['Receipt', 'Transfer']:
+			employee = self.to_employee
+
 		if self.serial_no:
 			for d in get_serial_nos(self.serial_no):
-				if (location or self.purpose == 'Issue'):
+				if (location or (self.purpose == 'Issue' and self.source_location)):
 					frappe.db.set_value('Serial No', d, 'location', location)
 
-				if employee:
+				if employee or self.docstatus==2 or self.purpose == 'Issue':
 					frappe.db.set_value('Serial No', d, 'employee', employee)

erpnext/stock/doctype/item/item.py

@@ -45,7 +45,7 @@ class Item(WebsiteGenerator):
 		self.set_onload('stock_exists', self.stock_ledger_created())
 		self.set_asset_naming_series()
 		if self.is_fixed_asset:
-			asset = frappe.db.get_all("Asset", filters={"item_code": self.name, "docstatus": 1}, limit=1)
+			asset = self.asset_exists()
 			self.set_onload("asset_exists", True if asset else False)
 
 	def set_asset_naming_series(self):
@@ -113,6 +113,7 @@ class Item(WebsiteGenerator):
 
 		self.validate_has_variants()
 		self.validate_stock_exists_for_template_item()
+		self.validate_asset_exists_for_serialized_asset()
 		self.validate_attributes()
 		self.validate_variant_attributes()
 		self.validate_website_image()
@@ -693,6 +694,18 @@ class Item(WebsiteGenerator):
 					frappe.throw(
 						_('Cannot change Attributes after stock transaction. Make a new Item and transfer stock to the new Item'))
 
+	def validate_asset_exists_for_serialized_asset(self):
+		if (not self.get("__islocal") and self.asset_exists() and
+			cint(self.has_serial_no) != cint(frappe.db.get_value('Item', self.name, 'has_serial_no'))):
+			frappe.throw(_("Asset is already exists against the item {0}, you cannot change the has serial no value")
+				.format(self.name))
+
+	def asset_exists(self):
+		if not hasattr(self, '_asset_created'):
+			self._asset_created = frappe.db.get_all("Asset",
+				filters={"item_code": self.name, "docstatus": 1}, limit=1)
+		return self._asset_created
+
 	def validate_uom(self):
 		if not self.get("__islocal"):
 			check_stock_uom_with_bin(self.name, self.stock_uom)

erpnext/stock/doctype/purchase_receipt/purchase_receipt.js

@@ -11,6 +11,14 @@ frappe.ui.form.on("Purchase Receipt", {
 			'Stock Entry': 'Return',
 			'Purchase Invoice': 'Invoice'
 		}
+
+		frm.set_query("asset", "items", function() {
+			return {
+				filters: {
+					"purchase_receipt": frm.doc.name
+				}
+			}
+		})
 	},
 	onload: function(frm) {
 		$.each(["warehouse", "rejected_warehouse"], function(i, field) {