fix(setup): reject negative sales targets

(cherry picked from commit 16e6c794b5)

erpnext/setup/doctype/sales_partner/sales_partner.py

@@ -7,7 +7,7 @@ from frappe.contacts.address_and_contact import load_address_and_contact
 from frappe.utils import cstr, filter_strip_join
 from frappe.website.website_generator import WebsiteGenerator
 
-from erpnext.setup.doctype.target_detail.target_detail import validate_duplicate_targets
+from erpnext.setup.doctype.target_detail.target_detail import validate_targets
 
 
 class SalesPartner(WebsiteGenerator):
@@ -49,7 +49,7 @@ class SalesPartner(WebsiteGenerator):
 		self.name = self.partner_name
 
 	def validate(self):
-		validate_duplicate_targets(self.get("targets"))
+		validate_targets(self.get("targets"))
 		if not self.route:
 			self.route = "partners/" + self.scrub(self.partner_name)
 		super().validate()

erpnext/setup/doctype/sales_person/sales_person.py

@@ -14,7 +14,7 @@ from frappe.utils.data import get_url_to_list
 from frappe.utils.nestedset import NestedSet, get_root_of
 
 from erpnext import get_default_currency
-from erpnext.setup.doctype.target_detail.target_detail import validate_duplicate_targets
+from erpnext.setup.doctype.target_detail.target_detail import validate_targets
 
 
 class SalesPerson(NestedSet):
@@ -44,7 +44,7 @@ class SalesPerson(NestedSet):
 	nsm_parent_field = "parent_sales_person"
 
 	def validate(self):
-		validate_duplicate_targets(self.get("targets"))
+		validate_targets(self.get("targets"))
 		if not self.enabled:
 			self.validate_sales_person()
 

erpnext/setup/doctype/target_detail/target_detail.py

@@ -5,6 +5,7 @@
 import frappe
 from frappe import _
 from frappe.model.document import Document
+from frappe.utils import flt
 
 
 class TargetDetail(Document):
@@ -29,9 +30,11 @@ class TargetDetail(Document):
 	pass
 
 
-def validate_duplicate_targets(targets):
+def validate_targets(targets):
 	seen = set()
 	for target in targets or []:
+		if flt(target.target_qty) < 0 or flt(target.target_amount) < 0:
+			frappe.throw(_("Row {0}: Target Qty and Target Amount cannot be negative").format(target.idx))
 		key = (target.fiscal_year, target.item_group or None)
 		if key in seen:
 			frappe.throw(

erpnext/setup/doctype/target_detail/test_target_detail.py

@@ -41,3 +41,21 @@ class TestTargetDetail(ERPNextTestSuite):
 				"targets", dict(holder.targets[0].as_dict(), name=None, item_group="_Test Item Group A")
 			)
 			holder.save()
+
+	def test_negative_targets_are_rejected(self):
+		fiscal_year = get_fiscal_year(nowdate())[0]
+		distribution = create_target_distribution(fiscal_year)
+		for doctype, name_field in (
+			("Territory", "territory_name"),
+			("Sales Person", "sales_person_name"),
+			("Sales Partner", "partner_name"),
+		):
+			holder = create_sales_target_doc(
+				doctype, name_field, frappe.generate_hash(length=10), fiscal_year, distribution.name
+			)
+			for field in ("target_qty", "target_amount"):
+				with self.subTest(doctype=doctype, field=field):
+					holder.reload()
+					holder.targets[0].set(field, -1)
+					with self.assertRaisesRegex(frappe.ValidationError, "cannot be negative"):
+						holder.save()

erpnext/setup/doctype/territory/territory.py

@@ -7,7 +7,7 @@ from frappe import _
 from frappe.utils import flt
 from frappe.utils.nestedset import NestedSet, get_root_of
 
-from erpnext.setup.doctype.target_detail.target_detail import validate_duplicate_targets
+from erpnext.setup.doctype.target_detail.target_detail import validate_targets
 
 
 class Territory(NestedSet):
@@ -34,7 +34,7 @@ class Territory(NestedSet):
 	nsm_parent_field = "parent_territory"
 
 	def validate(self):
-		validate_duplicate_targets(self.get("targets"))
+		validate_targets(self.get("targets"))
 		if not self.parent_territory:
 			self.parent_territory = get_root_of("Territory")