Merge pull request #23408 from kevingdc/fix/item-price-duplicate-checking

fix: Item price duplicate checking

erpnext/stock/doctype/item_price/item_price.py

@@ -4,14 +4,13 @@
 from __future__ import unicode_literals
 import frappe
 from frappe import _
-
-
-class ItemPriceDuplicateItem(frappe.ValidationError): pass
-
-
 from frappe.model.document import Document
 
 
+class ItemPriceDuplicateItem(frappe.ValidationError):
+	pass
+
+
 class ItemPrice(Document):
 
 	def validate(self):
@@ -23,7 +22,7 @@ class ItemPrice(Document):
 
 	def validate_item(self):
 		if not frappe.db.exists("Item", self.item_code):
-			frappe.throw(_("Item {0} not found").format(self.item_code))
+			frappe.throw(_("Item {0} not found.").format(self.item_code))
 
 	def validate_dates(self):
 		if self.valid_from and self.valid_upto:
@@ -38,40 +37,45 @@ class ItemPrice(Document):
 
 			if not price_list_details:
 				link = frappe.utils.get_link_to_form('Price List', self.price_list)
-				frappe.throw("The price list {0} does not exists or disabled".
-					format(link))
+				frappe.throw("The price list {0} does not exist or is disabled".format(link))
 
 			self.buying, self.selling, self.currency = price_list_details
 
 	def update_item_details(self):
 		if self.item_code:
-			self.item_name, self.item_description = frappe.db.get_value("Item",
-				self.item_code,["item_name", "description"])
+			self.item_name, self.item_description = frappe.db.get_value("Item", self.item_code,["item_name", "description"])
 
 	def check_duplicates(self):
-		conditions = "where item_code=%(item_code)s and price_list=%(price_list)s and name != %(name)s"
-		condition_data_dict = dict(item_code=self.item_code, price_list=self.price_list, name=self.name)
+		conditions = """where item_code = %(item_code)s and price_list = %(price_list)s and name != %(name)s"""
 
-		for field in ['uom', 'valid_from',
-			'valid_upto', 'packing_unit', 'customer', 'supplier']:
+		for field in [
+			"uom",
+			"valid_from",
+			"valid_upto",
+			"packing_unit",
+			"customer",
+			"supplier",]:
 			if self.get(field):
-				conditions += " and {0} = %({1})s".format(field, field)
-				condition_data_dict[field] = self.get(field)
+				conditions += " and {0} = %({0})s ".format(field)
+			else:
+				conditions += "and (isnull({0}) or {0} = '')".format(field)
 
 		price_list_rate = frappe.db.sql("""
-			SELECT price_list_rate
-			FROM `tabItem Price`
-			  {conditions} """.format(conditions=conditions), condition_data_dict)
+				select price_list_rate
+				from `tabItem Price`
+				{conditions}
+			""".format(conditions=conditions),
+			self.as_dict(),)
 
-		if price_list_rate :
-			frappe.throw(_("Item Price appears multiple times based on Price List, Supplier/Customer, Currency, Item, UOM, Qty and Dates."), ItemPriceDuplicateItem)
+		if price_list_rate:
+			frappe.throw(_("Item Price appears multiple times based on Price List, Supplier/Customer, Currency, Item, UOM, Qty, and Dates."), ItemPriceDuplicateItem,)
 
 	def before_save(self):
 		if self.selling:
 			self.reference = self.customer
 		if self.buying:
 			self.reference = self.supplier
-		
+
 		if self.selling and not self.buying:
 			# if only selling then remove supplier
 			self.supplier = None

erpnext/stock/doctype/item_price/test_item_price.py

@@ -138,4 +138,23 @@ class TestItemPrice(unittest.TestCase):
 		# Valid price list must already exist
 		self.assertRaises(frappe.ValidationError, doc.save)
 
+	def test_empty_duplicate_validation(self):
+		# Check if none/empty values are not compared during insert validation
+		doc = frappe.copy_doc(test_records[2])
+		doc.customer = None
+		doc.price_list_rate = 21
+		doc.insert()
+
+		args = {
+			"price_list": doc.price_list,
+			"uom": "_Test UOM",
+			"transaction_date": '2017-04-18',
+			"qty": 7
+		}
+
+		price = get_price_list_rate_for(args, doc.item_code)
+		frappe.db.rollback()
+
+		self.assertEqual(price, 21)
+
 test_records = frappe.get_test_records('Item Price')